mirror of
https://github.com/stablyai/orca.git
synced 2026-10-24 08:03:58 +00:00
fix(native-chat): refuse target-less resumes onto a structured session (#15161)
`isResumeCommandFor` checked id-containment first, so `codex resume --last` — which carries no session id — never matched the owned session and the legacy writer guard admitted it. A TUI then attached to the very provider session the structured runtime owns. No race: it reproduces every time. Bare `claude --resume`, `-r`, `--continue` and `-c` have the same shape. Resume invocations are now parsed into (provider, target) and the guard fails closed: a resume is refused unless it names a target provably different from the owned session. `--continue`/`-c` never take a session id, so a trailing token is a prompt and they are always target-less — reading it as a target would have admitted a writer onto the owned thread. Verified: 10 ownership tests, node typecheck, oxlint clean. Mutation: dropping the target-less branch turns 4 assertions red.
This commit is contained in:
@@ -52,6 +52,35 @@ describe('structured AI Vault ownership', () => {
|
||||
)
|
||||
).rejects.toThrow('agent_session_ownership_unknown')
|
||||
})
|
||||
|
||||
it.each([
|
||||
`codex resume --last`,
|
||||
`claude --resume`,
|
||||
`claude -r`,
|
||||
`claude --continue`,
|
||||
`claude -c`,
|
||||
// `--continue` takes no session id, so the trailing token is a prompt —
|
||||
// reading it as a target would admit a writer onto the owned session.
|
||||
`claude --continue "keep going"`,
|
||||
`claude -c 019fd532-7c11-7a90-b6de-4e1a2c3d5f61`
|
||||
])('refuses resume commands without a provably different target: %s', async (command) => {
|
||||
installOwnership(command.startsWith('claude') ? { provider: 'claude' } : {})
|
||||
|
||||
await expect(
|
||||
assertLegacyAiVaultResumeCommandAllowed(command, async () => undefined)
|
||||
).rejects.toThrow('agent_session_conflict')
|
||||
})
|
||||
|
||||
it('allows a resume command that names a different provider session', async () => {
|
||||
installOwnership()
|
||||
|
||||
await expect(
|
||||
assertLegacyAiVaultResumeCommandAllowed(
|
||||
'codex resume 019fd532-7c11-7a90-b6de-4e1a2c3d5f61',
|
||||
async () => undefined
|
||||
)
|
||||
).resolves.toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
function installOwnership(overrides: Partial<StructuredProviderSessionOwnership> = {}): void {
|
||||
@@ -76,7 +105,7 @@ function installOwnership(overrides: Partial<StructuredProviderSessionOwnership>
|
||||
providerHandleChain: [
|
||||
{
|
||||
...record.providerHandleChain[0]!,
|
||||
handle: { provider: 'codex', threadId: ownership.providerSessionId }
|
||||
handle: { provider: ownership.provider, threadId: ownership.providerSessionId }
|
||||
}
|
||||
],
|
||||
lease: { ...ownership.lease, sessionId: ownership.sessionId }
|
||||
|
||||
@@ -66,10 +66,7 @@ export async function assertLegacyAiVaultResumeCommandAllowed(
|
||||
}
|
||||
|
||||
function isPotentialStructuredResumeCommand(command: string): boolean {
|
||||
return (
|
||||
/\bcodex(?:\.exe)?\b[\s\S]*\bresume\b/i.test(command) ||
|
||||
/\bclaude(?:\.exe)?\b[\s\S]*--resume\b/i.test(command)
|
||||
)
|
||||
return parseResumeInvocation(command) !== null
|
||||
}
|
||||
|
||||
function findSessionOwnership(session: AiVaultSession): StructuredProviderSessionOwnership | null {
|
||||
@@ -123,12 +120,61 @@ function isResumeCommandFor(
|
||||
command: string,
|
||||
ownership: StructuredProviderSessionOwnership
|
||||
): boolean {
|
||||
if (!command.includes(ownership.providerSessionId)) {
|
||||
const invocation = parseResumeInvocation(command)
|
||||
if (!invocation || invocation.provider !== ownership.provider) {
|
||||
return false
|
||||
}
|
||||
return ownership.provider === 'codex'
|
||||
? /\bcodex(?:\.exe)?\b[\s\S]*\bresume\b/i.test(command)
|
||||
: /\bclaude(?:\.exe)?\b[\s\S]*--resume\b/i.test(command)
|
||||
// A target-less resume (--last, --continue, or a bare --resume/-r) may pick
|
||||
// any provider session, so it cannot be admitted while one is structured.
|
||||
// Only an explicit target that differs from this owned session is safe.
|
||||
return invocation.target === null || invocation.target === ownership.providerSessionId
|
||||
}
|
||||
|
||||
type ResumeInvocation = {
|
||||
provider: 'codex' | 'claude'
|
||||
target: string | null
|
||||
}
|
||||
|
||||
function parseResumeInvocation(command: string): ResumeInvocation | null {
|
||||
// Keep this deliberately conservative: shell quoting is normalized only
|
||||
// enough to identify executable/flag tokens; an unrecognized shape is not
|
||||
// treated as proof that a different session is being resumed.
|
||||
const tokens = command.match(/"[^"\\]*(?:\\.[^"\\]*)*"|'[^']*'|[^\s]+/g) ?? []
|
||||
const normalized = tokens.map((token) => token.replace(/^['"]|['"]$/g, ''))
|
||||
const executableIndex = normalized.findIndex((token) =>
|
||||
/(?:^|[\\/])(?:codex|claude)(?:\.exe)?$/i.test(token)
|
||||
)
|
||||
if (executableIndex === -1) {
|
||||
return null
|
||||
}
|
||||
const provider = /codex(?:\.exe)?$/i.test(normalized[executableIndex]!) ? 'codex' : 'claude'
|
||||
const args = normalized.slice(executableIndex + 1)
|
||||
// `--continue`/`-c` resume the most recent session and never take an id, so a
|
||||
// following token is a prompt, not a target — they are always target-less.
|
||||
const targetlessFlags = provider === 'codex' ? [] : ['--continue', '-c']
|
||||
const targetlessIndex = args.findIndex((token) => targetlessFlags.includes(token.toLowerCase()))
|
||||
if (targetlessIndex !== -1) {
|
||||
return { provider, target: null }
|
||||
}
|
||||
const resumeFlags = provider === 'codex' ? ['resume'] : ['--resume', '-r']
|
||||
const inlineIndex = args.findIndex(
|
||||
(token) =>
|
||||
provider === 'claude' &&
|
||||
(token.toLowerCase().startsWith('--resume=') || token.toLowerCase().startsWith('-r='))
|
||||
)
|
||||
if (inlineIndex !== -1) {
|
||||
const target = args[inlineIndex]!.slice(args[inlineIndex]!.indexOf('=') + 1)
|
||||
return { provider, target: target.length > 0 ? target : null }
|
||||
}
|
||||
const markerIndex = args.findIndex((token) => resumeFlags.includes(token.toLowerCase()))
|
||||
if (markerIndex === -1) {
|
||||
return null
|
||||
}
|
||||
const candidate = args[markerIndex + 1]
|
||||
return {
|
||||
provider,
|
||||
target: candidate && !candidate.startsWith('-') ? candidate : null
|
||||
}
|
||||
}
|
||||
|
||||
function refuseLegacyWriter(ownership: StructuredProviderSessionOwnership): never {
|
||||
|
||||
Reference in New Issue
Block a user