From 7c2debc13064c77bdf00b8093adb431febac5fbd Mon Sep 17 00:00:00 2001 From: kunsanglee <85242378+kunsanglee@users.noreply.github.com> Date: Tue, 28 Jul 2026 14:28:17 +0900 Subject: [PATCH 01/12] fix(agent-hooks): drop hook status whose cwd disproves its pane Agent CLIs increasingly pre-warm a shared background daemon, and that daemon keeps the Orca pane env of whichever pane happened to spawn it. Every session it later hosts inherits ORCA_PANE_KEY/ORCA_WORKTREE_ID from that first pane, so a session running in workspace A reports workspace B's pane and its prompt, state, and subagents render on B's sidebar card while A shows nothing. Cross-check the session cwd the agent reports in its own hook payload against the worktree path already encoded in the reported worktreeId, and refuse the event when the two are disjoint. Ambiguity keeps the old behavior: a missing cwd, a non-path worktree id, mixed WSL/UNC notation, or either path nested in the other all stay attributed, since dropping a real status row is the worse failure. The relay forwards cwd alongside the normalized payload so remote sessions get the same check. --- .../server-cwd-attribution.test.ts | 114 ++++++++++++++++++ src/main/agent-hooks/server.ts | 34 +++++- src/relay/agent-hook-server.test.ts | 5 +- src/relay/agent-hook-server.ts | 1 + src/shared/agent-hook-cwd-attribution.test.ts | 81 +++++++++++++ src/shared/agent-hook-cwd-attribution.ts | 70 +++++++++++ src/shared/agent-hook-listener.ts | 4 + src/shared/agent-hook-relay.ts | 3 + src/shared/telemetry-events.ts | 2 +- 9 files changed, 311 insertions(+), 3 deletions(-) create mode 100644 src/main/agent-hooks/server-cwd-attribution.test.ts create mode 100644 src/shared/agent-hook-cwd-attribution.test.ts create mode 100644 src/shared/agent-hook-cwd-attribution.ts diff --git a/src/main/agent-hooks/server-cwd-attribution.test.ts b/src/main/agent-hooks/server-cwd-attribution.test.ts new file mode 100644 index 000000000000..0d2015b7cc17 --- /dev/null +++ b/src/main/agent-hooks/server-cwd-attribution.test.ts @@ -0,0 +1,114 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { AgentHookServer } from './server' +import { makePaneKey } from '../../shared/stable-pane-id' + +const { trackMock } = vi.hoisted(() => ({ trackMock: vi.fn() })) + +vi.mock('../telemetry/client', () => ({ track: trackMock })) +vi.mock('../telemetry/cohort-classifier', () => ({ getCohortAtEmit: vi.fn() })) + +const AGENT_PANE = makePaneKey('tab-agent', '11111111-1111-4111-8111-111111111111') +const AGENT_WORKTREE = 'repo-agent::/Users/dev/workspace/agent' + +// Why: reproduces the shared-daemon leak — a session running in one project posts the +// pane identity it inherited from the pane that first spawned the agent daemon. +function buildBody(payload: Record): Record { + return { + paneKey: AGENT_PANE, + tabId: 'tab-agent', + worktreeId: AGENT_WORKTREE, + env: 'production', + payload + } +} + +beforeEach(() => { + trackMock.mockReset() +}) + +describe('AgentHookServer cwd attribution guard', () => { + it('drops an HTTP hook whose session cwd belongs to another workspace', async () => { + const server = new AgentHookServer() + await server.start({ env: 'production' }) + try { + const env = server.buildPtyEnv() + const postHook = (payload: Record): Promise => + fetch(`http://127.0.0.1:${env.ORCA_AGENT_HOOK_PORT}/hook/claude`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-Orca-Agent-Hook-Token': env.ORCA_AGENT_HOOK_TOKEN + }, + body: JSON.stringify(buildBody(payload)) + }) + + await expect( + postHook({ + hook_event_name: 'UserPromptSubmit', + prompt: 'own session', + cwd: '/Users/dev/workspace/agent' + }) + ).resolves.toMatchObject({ status: 204 }) + expect(server.getStatusSnapshot()).toEqual([ + expect.objectContaining({ paneKey: AGENT_PANE, prompt: 'own session' }) + ]) + + await expect( + postHook({ + hook_event_name: 'UserPromptSubmit', + prompt: 'foreign session', + cwd: '/Users/dev/projects/api' + }) + ).resolves.toMatchObject({ status: 204 }) + + expect(server.getStatusSnapshot()).toEqual([ + expect.objectContaining({ paneKey: AGENT_PANE, prompt: 'own session' }) + ]) + expect(trackMock).toHaveBeenCalledWith('agent_hook_unattributed', { + reason: 'cwd_worktree_mismatch' + }) + } finally { + server.stop() + } + }) + + it('drops a relayed hook whose session cwd belongs to another workspace', () => { + const server = new AgentHookServer() + server.ingestRemote( + { + paneKey: AGENT_PANE, + tabId: 'tab-agent', + worktreeId: AGENT_WORKTREE, + // Why: the relay forwards cwd beside the payload — normalization strips it from the payload itself. + sourceCwd: '/srv/other-project', + payload: { state: 'working', prompt: 'foreign session' } + }, + 'conn-1' + ) + + expect(server.getStatusSnapshot()).toEqual([]) + expect(trackMock).toHaveBeenCalledWith('agent_hook_unattributed', { + reason: 'cwd_worktree_mismatch' + }) + }) + + it('keeps hooks that report no cwd, so sources without one stay attributed', () => { + const server = new AgentHookServer() + server.ingestRemote( + { + paneKey: AGENT_PANE, + tabId: 'tab-agent', + worktreeId: AGENT_WORKTREE, + payload: { state: 'working', prompt: 'no cwd reported' } + }, + 'conn-1' + ) + + expect(server.getStatusSnapshot()).toEqual([ + expect.objectContaining({ paneKey: AGENT_PANE, prompt: 'no cwd reported' }) + ]) + expect(trackMock).not.toHaveBeenCalledWith('agent_hook_unattributed', { + reason: 'cwd_worktree_mismatch' + }) + }) +}) diff --git a/src/main/agent-hooks/server.ts b/src/main/agent-hooks/server.ts index a9395cf44de7..253ff64e2711 100644 --- a/src/main/agent-hooks/server.ts +++ b/src/main/agent-hooks/server.ts @@ -9,6 +9,7 @@ import { track } from '../telemetry/client' import { getCohortAtEmit } from '../telemetry/cohort-classifier' import { AGENT_KIND_VALUES, type AgentKind } from '../../shared/telemetry-events' import { ORCA_HOOK_PROTOCOL_VERSION } from '../../shared/agent-hook-types' +import { hookCwdContradictsWorktree } from '../../shared/agent-hook-cwd-attribution' import { clearAllListenerCaches, clearPaneCacheState, @@ -489,6 +490,8 @@ export class AgentHookServer { private closedAgentStatusTabIds = new Set() private closedAgentStatusPaneKeys = new Set() private connectionTimestampWatermarkById = new Map() + // Why: one console line per runtime — a mis-attributing daemon fires on every hook of every session it hosts. + private warnedForeignCwdStatus = false // Why: skip disk writes when the JSON exactly matches the last write; guards against re-firing trailing timers when nothing changed. private lastWrittenJson: string | null = null @@ -771,6 +774,22 @@ export class AgentHookServer { return this.closedAgentStatusTabIds.has(tabId) } + /** Refuse a hook whose reported worktree is disproven by the session cwd it carries. */ + private shouldSuppressForeignCwdStatus(event: AgentHookEventPayload): boolean { + if (!hookCwdContradictsWorktree(event.worktreeId, event.sourceCwd)) { + return false + } + track('agent_hook_unattributed', { reason: 'cwd_worktree_mismatch' }) + if (!this.warnedForeignCwdStatus) { + this.warnedForeignCwdStatus = true + console.warn( + '[agent-hooks] dropping status: reported worktree does not own the reporting session', + { paneKey: event.paneKey, worktreeId: event.worktreeId, sourceCwd: event.sourceCwd } + ) + } + return true + } + private markPaneClosedForAgentStatus(paneKey: string): void { this.closedAgentStatusPaneKeys.delete(paneKey) this.closedAgentStatusPaneKeys.add(paneKey) @@ -1442,6 +1461,7 @@ export class AgentHookServer { toolAgentType?: string providerSession?: unknown providerSessionOnly?: unknown + sourceCwd?: string isReplay?: boolean payload: unknown }, @@ -1552,8 +1572,15 @@ export class AgentHookServer { providerSession, providerSessionOnly: envelope.providerSessionOnly === true ? true : undefined, isReplay: envelope.isReplay === true ? true : undefined, + sourceCwd: + typeof envelope.sourceCwd === 'string' && envelope.sourceCwd.trim().length > 0 + ? envelope.sourceCwd.trim() + : undefined, payload: normalizedPayload } + if (this.shouldSuppressForeignCwdStatus(event)) { + return + } this.applyNormalizedStatus(event) } @@ -1624,7 +1651,11 @@ export class AgentHookServer { trackEmptyPaneKeyHook(body) const aliasedBody = this.normalizeHookBodyPaneKeyAlias(body) const normalized = normalizeHookPayload(this.state, source, aliasedBody, this.env) - if (normalized && !this.shouldSuppressClosedTabStatus(normalized.paneKey)) { + if ( + normalized && + !this.shouldSuppressClosedTabStatus(normalized.paneKey) && + !this.shouldSuppressForeignCwdStatus(normalized) + ) { const enriched = this.applyNormalizedStatus(normalized) this.scheduleAssistantMessageRetry(source, aliasedBody, enriched) } @@ -1682,6 +1713,7 @@ export class AgentHookServer { this.lastStatusFilePath = null this.lastWrittenJson = null this.runtimeObservedStatusPaneKeys.clear() + this.warnedForeignCwdStatus = false this.promptSentDedupeByPaneKey.clear() this.closedAgentStatusTabIds.clear() this.closedAgentStatusPaneKeys.clear() diff --git a/src/relay/agent-hook-server.test.ts b/src/relay/agent-hook-server.test.ts index 3d73506c7bf6..d107f9846bb8 100644 --- a/src/relay/agent-hook-server.test.ts +++ b/src/relay/agent-hook-server.test.ts @@ -53,7 +53,7 @@ describe('RelayAgentHookServer', () => { worktreeId: 'wt-1', env: 'remote', version: '1', - payload: { hook_event_name: 'UserPromptSubmit', prompt: 'hi' } + payload: { hook_event_name: 'UserPromptSubmit', prompt: 'hi', cwd: '/srv/app' } }) }) expect(res.status).toBe(204) @@ -65,6 +65,9 @@ describe('RelayAgentHookServer', () => { expect(envelope.connectionId).toBeNull() expect(envelope.payload.state).toBe('working') expect(envelope.payload.prompt).toBe('hi') + // Why: normalization strips cwd from the payload, so Orca can only re-check + // the remote pane attribution if the relay forwards it alongside. + expect(envelope.sourceCwd).toBe('/srv/app') // Why: the relay forwards body env/version so Orca's warn-once // protocol diagnostics and remote-location marker survive the wire. expect(envelope.env).toBe('remote') diff --git a/src/relay/agent-hook-server.ts b/src/relay/agent-hook-server.ts index 40775b5bd6ae..57e118eae1d1 100644 --- a/src/relay/agent-hook-server.ts +++ b/src/relay/agent-hook-server.ts @@ -312,6 +312,7 @@ export class RelayAgentHookServer { isReplay: options.isReplay === true ? true : undefined, env, version, + sourceCwd: event.sourceCwd, payload: event.payload } this.forward(envelope) diff --git a/src/shared/agent-hook-cwd-attribution.test.ts b/src/shared/agent-hook-cwd-attribution.test.ts new file mode 100644 index 000000000000..8206656e917c --- /dev/null +++ b/src/shared/agent-hook-cwd-attribution.test.ts @@ -0,0 +1,81 @@ +import { describe, expect, it } from 'vitest' +import { hookCwdContradictsWorktree, readHookPayloadCwd } from './agent-hook-cwd-attribution' +import { FOLDER_WORKSPACE_INSTANCE_SEPARATOR } from './worktree-id' + +const REPO = 'repo-1' +const WORKTREE = `${REPO}::/Users/dev/projects/api` + +describe('readHookPayloadCwd', () => { + it('reads the cwd variants agents report, and nothing else', () => { + expect(readHookPayloadCwd({ cwd: '/Users/dev/projects/api' })).toBe('/Users/dev/projects/api') + expect(readHookPayloadCwd({ workspaceRoot: '/srv/app' })).toBe('/srv/app') + expect(readHookPayloadCwd({ workspace_root: '/srv/app' })).toBe('/srv/app') + expect(readHookPayloadCwd({ cwd: ' ' })).toBeUndefined() + expect(readHookPayloadCwd({ cwd: 42 })).toBeUndefined() + expect(readHookPayloadCwd(null)).toBeUndefined() + expect(readHookPayloadCwd('/srv/app')).toBeUndefined() + }) +}) + +describe('hookCwdContradictsWorktree', () => { + it('accepts a session running at or under the reported worktree', () => { + expect(hookCwdContradictsWorktree(WORKTREE, '/Users/dev/projects/api')).toBe(false) + expect(hookCwdContradictsWorktree(WORKTREE, '/Users/dev/projects/api/')).toBe(false) + expect(hookCwdContradictsWorktree(WORKTREE, '/Users/dev/projects/api/src/server')).toBe(false) + }) + + it('rejects a session whose cwd lies outside the reported worktree', () => { + // Why: the daemon-inherited env case — a session in one project reporting another's pane. + expect(hookCwdContradictsWorktree(WORKTREE, '/Users/dev/projects/agent')).toBe(true) + expect(hookCwdContradictsWorktree(WORKTREE, '/Users/dev/projects/api-two')).toBe(true) + }) + + it('accepts a worktree nested under the session cwd', () => { + // Why: folder workspaces can sit below the directory the agent was started in. + expect(hookCwdContradictsWorktree(WORKTREE, '/Users/dev/projects')).toBe(false) + expect(hookCwdContradictsWorktree(WORKTREE, '/')).toBe(false) + }) + + it('stays quiet when either side is missing or unparseable', () => { + expect(hookCwdContradictsWorktree(undefined, '/Users/dev/projects/agent')).toBe(false) + expect(hookCwdContradictsWorktree(WORKTREE, undefined)).toBe(false) + expect(hookCwdContradictsWorktree(WORKTREE, '')).toBe(false) + // Worktree ids without the `::` separator carry no path to compare. + expect(hookCwdContradictsWorktree('onboarding-inline-terminal', '/Users/dev/x')).toBe(false) + // Relative cwd has no comparable root. + expect(hookCwdContradictsWorktree(WORKTREE, 'projects/agent')).toBe(false) + }) + + it('ignores case and trailing separators when comparing', () => { + expect(hookCwdContradictsWorktree(WORKTREE, '/users/dev/projects/API/src')).toBe(false) + expect( + hookCwdContradictsWorktree(`${REPO}::/Users/dev/projects/api/`, '/Users/dev/projects/api') + ).toBe(false) + }) + + it('compares Windows worktrees against Windows cwds', () => { + const windowsWorktree = `${REPO}::C:\\Users\\dev\\api` + expect(hookCwdContradictsWorktree(windowsWorktree, 'C:\\Users\\dev\\api\\src')).toBe(false) + expect(hookCwdContradictsWorktree(windowsWorktree, 'C:/Users/dev/agent')).toBe(true) + }) + + it('refuses to judge across notations a single host can express two ways', () => { + // WSL reports the POSIX mount for a drive-rooted worktree, and UNC paths name the distro. + expect(hookCwdContradictsWorktree(`${REPO}::C:\\Users\\dev\\api`, '/mnt/c/Users/dev/api')).toBe( + false + ) + expect( + hookCwdContradictsWorktree(`${REPO}::\\\\wsl$\\Ubuntu\\home\\dev\\api`, '/home/dev/api') + ).toBe(false) + expect( + hookCwdContradictsWorktree(`${REPO}::/home/dev/api`, '\\\\wsl$\\Ubuntu\\home\\dev\\x') + ).toBe(false) + }) + + it('compares the real folder path of a folder-workspace instance id', () => { + const instanceId = '11111111-2222-4333-8444-555555555555' + const folderWorkspace = `${REPO}::/Users/dev/notes${FOLDER_WORKSPACE_INSTANCE_SEPARATOR}${instanceId}` + expect(hookCwdContradictsWorktree(folderWorkspace, '/Users/dev/notes/inbox')).toBe(false) + expect(hookCwdContradictsWorktree(folderWorkspace, '/Users/dev/projects/api')).toBe(true) + }) +}) diff --git a/src/shared/agent-hook-cwd-attribution.ts b/src/shared/agent-hook-cwd-attribution.ts new file mode 100644 index 000000000000..d84ec77afb8f --- /dev/null +++ b/src/shared/agent-hook-cwd-attribution.ts @@ -0,0 +1,70 @@ +import { splitWorktreeIdForFilesystem } from './worktree-id' + +const DRIVE_ROOTED = /^[a-z]:\// +const CWD_PAYLOAD_KEYS = ['cwd', 'workspaceRoot', 'workspace_root'] as const + +/** Session cwd the agent reported in its own hook payload, for sources that expose one. */ +export function readHookPayloadCwd(hookPayload: unknown): string | undefined { + if (typeof hookPayload !== 'object' || hookPayload === null) { + return undefined + } + const record = hookPayload as Record + for (const key of CWD_PAYLOAD_KEYS) { + const value = record[key] + if (typeof value === 'string' && value.trim().length > 0) { + return value.trim() + } + } + return undefined +} + +function normalizeComparablePath(raw: string): string | null { + const slashed = raw.trim().replace(/\\/g, '/') + const lowered = slashed.toLowerCase() + // Why: UNC (`//wsl$/…`) and relative paths have no comparable root — unknown, not conflicting. + if (lowered.startsWith('//') || (!lowered.startsWith('/') && !DRIVE_ROOTED.test(lowered))) { + return null + } + const trimmed = lowered.replace(/\/+$/, '') + return trimmed.length > 0 ? trimmed : '/' +} + +function isSameOrInside(inner: string, outer: string): boolean { + return inner === outer || inner.startsWith(outer === '/' ? '/' : `${outer}/`) +} + +/** + * True when the worktree a hook claims cannot own the session that sent it, judged + * against the cwd that session reported in its own payload. + * + * Why: paneKey and worktreeId ride in PTY env, and env is inherited. A shared agent + * daemon pre-warmed from one pane hands its Orca identity to every session it later + * hosts, so a session in worktree A reports pane B and its status lands on the wrong + * workspace card. Refuse only when both paths are absolute, written in the same + * notation, and disjoint; anything unclear stays attributed, since dropping a real + * status row is the worse failure. + */ +export function hookCwdContradictsWorktree( + worktreeId: string | undefined, + cwd: string | undefined +): boolean { + if (!worktreeId || !cwd) { + return false + } + const worktreePath = splitWorktreeIdForFilesystem(worktreeId)?.worktreePath + if (!worktreePath) { + return false + } + const workspace = normalizeComparablePath(worktreePath) + const session = normalizeComparablePath(cwd) + if (!workspace || !session) { + return false + } + // Why: a WSL session reports /mnt/c/… for a C:\… worktree; mixed notations aren't comparable. + if (DRIVE_ROOTED.test(workspace) !== DRIVE_ROOTED.test(session)) { + return false + } + // Why: a session started in a subdirectory is normal, and so is a workspace nested + // under the session root (folder workspaces); only fully disjoint paths are proof. + return !isSameOrInside(session, workspace) && !isSameOrInside(workspace, session) +} diff --git a/src/shared/agent-hook-listener.ts b/src/shared/agent-hook-listener.ts index d26268d120e7..163b5e0a4b01 100644 --- a/src/shared/agent-hook-listener.ts +++ b/src/shared/agent-hook-listener.ts @@ -25,6 +25,7 @@ import { type ParsedAgentStatusPayload } from './agent-status-types' import { normalizeOptionalField } from './agent-status-field-normalization' +import { readHookPayloadCwd } from './agent-hook-cwd-attribution' import { isAskUserQuestionTool } from './agent-question-answered-intent' import { claudeRosterHasWorkingSubagent, @@ -297,6 +298,8 @@ export type AgentHookEventPayload = { providerSessionOnly?: boolean /** True when this event is a relay cache replay rather than a live hook. */ isReplay?: boolean + /** Session cwd from the agent's own payload; cross-checks the env-derived pane attribution. */ + sourceCwd?: string payload: ParsedAgentStatusPayload } @@ -4019,6 +4022,7 @@ export function normalizeHookPayload( toolAgentType: readString(hookPayloadRecord, 'agent_type'), ...(providerSession ? { providerSession } : {}), ...(providerSessionOnly ? { providerSessionOnly: true } : {}), + sourceCwd: readHookPayloadCwd(hookPayloadRecord), payload: transportPayload } : null diff --git a/src/shared/agent-hook-relay.ts b/src/shared/agent-hook-relay.ts index 9cbd6c18faaa..c298b289e183 100644 --- a/src/shared/agent-hook-relay.ts +++ b/src/shared/agent-hook-relay.ts @@ -90,6 +90,9 @@ export type AgentHookRelayEnvelope = { /** Forwarded verbatim from the agent CLI POST body. Lets Orca's warn-once * protocol-version diagnostic fire on remote events the same as on local. */ version?: string + /** Session cwd the remote agent reported, used to disprove a pane attribution + * the PTY env got wrong. The normalized `payload` no longer carries it. */ + sourceCwd?: string /** Pre-normalized status payload from the relay's `normalizeHookPayload`. * Orca's `ingestRemote` validates it again at the SSH trust boundary. */ payload: ParsedAgentStatusPayload diff --git a/src/shared/telemetry-events.ts b/src/shared/telemetry-events.ts index 9cbfaac20e2b..e62e57b5dc7d 100644 --- a/src/shared/telemetry-events.ts +++ b/src/shared/telemetry-events.ts @@ -714,7 +714,7 @@ const agentHookInstallFailedSchema = z // Why: regression signal for paneKey attribution — a hook event that can't route to a pane. See docs/cli-terminal-hook-pane-key.md. const agentHookUnattributedSchema = z - .object({ reason: z.enum(['empty_pane_key', 'unknown_tab_id']) }) + .object({ reason: z.enum(['empty_pane_key', 'unknown_tab_id', 'cwd_worktree_mismatch']) }) .strict() // ── Onboarding ────────────────────────────────────────────────────────── From e2488d1b3790a2ff0f3d3b73df5597f6ae498faa Mon Sep 17 00:00:00 2001 From: kunsanglee <85242378+kunsanglee@users.noreply.github.com> Date: Tue, 28 Jul 2026 14:41:25 +0900 Subject: [PATCH 02/12] fix(agent-hooks): resolve dot segments before comparing hook cwd `/repo/../other` starts with `/repo` as plain text, so the disjoint-path check read it as living inside the worktree and let a foreign session keep the pane it had inherited. Collapse `.` and `..` lexically first, for POSIX and drive-rooted paths alike, with an over-popping prefix landing on the root rather than escaping it. --- src/shared/agent-hook-cwd-attribution.test.ts | 16 ++++++++++++ src/shared/agent-hook-cwd-attribution.ts | 26 ++++++++++++++++--- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/shared/agent-hook-cwd-attribution.test.ts b/src/shared/agent-hook-cwd-attribution.test.ts index 8206656e917c..74259408c277 100644 --- a/src/shared/agent-hook-cwd-attribution.test.ts +++ b/src/shared/agent-hook-cwd-attribution.test.ts @@ -59,6 +59,22 @@ describe('hookCwdContradictsWorktree', () => { expect(hookCwdContradictsWorktree(windowsWorktree, 'C:/Users/dev/agent')).toBe(true) }) + it('resolves dot segments so a path cannot pose as being inside the worktree', () => { + // Why: `…/api/../agent` starts with the worktree path as plain text but resolves outside it. + expect(hookCwdContradictsWorktree(WORKTREE, '/Users/dev/projects/api/../agent')).toBe(true) + expect( + hookCwdContradictsWorktree(`${REPO}::C:\\Users\\dev\\api`, 'C:\\Users\\dev\\api\\..\\agent') + ).toBe(true) + // Dot segments that stay inside, or that only spell the worktree a longer way, are not conflicts. + expect(hookCwdContradictsWorktree(WORKTREE, '/Users/dev/projects/api/./src')).toBe(false) + expect(hookCwdContradictsWorktree(WORKTREE, '/Users/dev/projects/api/src/../lib')).toBe(false) + expect( + hookCwdContradictsWorktree(`${REPO}::/Users/dev/projects/x/../api`, '/Users/dev/projects/api') + ).toBe(false) + // Popping past the root lands on the root, which every worktree sits under. + expect(hookCwdContradictsWorktree(WORKTREE, '/../../..')).toBe(false) + }) + it('refuses to judge across notations a single host can express two ways', () => { // WSL reports the POSIX mount for a drive-rooted worktree, and UNC paths name the distro. expect(hookCwdContradictsWorktree(`${REPO}::C:\\Users\\dev\\api`, '/mnt/c/Users/dev/api')).toBe( diff --git a/src/shared/agent-hook-cwd-attribution.ts b/src/shared/agent-hook-cwd-attribution.ts index d84ec77afb8f..8ffe7d7fd86c 100644 --- a/src/shared/agent-hook-cwd-attribution.ts +++ b/src/shared/agent-hook-cwd-attribution.ts @@ -18,17 +18,35 @@ export function readHookPayloadCwd(hookPayload: unknown): string | undefined { return undefined } +/** Resolve `.` and `..` lexically so a path can't pose as living under another. */ +function collapseDotSegments(absolutePath: string): string { + const driveRoot = DRIVE_ROOTED.test(absolutePath) ? absolutePath.slice(0, 2) : '' + const segments: string[] = [] + for (const segment of absolutePath.slice(driveRoot.length).split('/')) { + if (segment === '' || segment === '.') { + continue + } + if (segment === '..') { + // Why: `/..` is the root itself, so an over-popping prefix must not escape it. + segments.pop() + continue + } + segments.push(segment) + } + return segments.length > 0 ? `${driveRoot}/${segments.join('/')}` : driveRoot || '/' +} + +/** Reduce a path to a comparable form, or null when it has no root this can compare. */ function normalizeComparablePath(raw: string): string | null { - const slashed = raw.trim().replace(/\\/g, '/') - const lowered = slashed.toLowerCase() + const lowered = raw.trim().replace(/\\/g, '/').toLowerCase() // Why: UNC (`//wsl$/…`) and relative paths have no comparable root — unknown, not conflicting. if (lowered.startsWith('//') || (!lowered.startsWith('/') && !DRIVE_ROOTED.test(lowered))) { return null } - const trimmed = lowered.replace(/\/+$/, '') - return trimmed.length > 0 ? trimmed : '/' + return collapseDotSegments(lowered) } +/** True when `inner` is `outer` itself or sits beneath it. */ function isSameOrInside(inner: string, outer: string): boolean { return inner === outer || inner.startsWith(outer === '/' ? '/' : `${outer}/`) } From 897e9c384385a42af0976d6b53dac91e0e644536 Mon Sep 17 00:00:00 2001 From: kunsanglee <85242378+kunsanglee@users.noreply.github.com> Date: Tue, 28 Jul 2026 14:52:55 +0900 Subject: [PATCH 03/12] docs(agent-hooks): document the cwd-attribution test fixture --- src/main/agent-hooks/server-cwd-attribution.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/agent-hooks/server-cwd-attribution.test.ts b/src/main/agent-hooks/server-cwd-attribution.test.ts index 0d2015b7cc17..e2e6ae1f5c53 100644 --- a/src/main/agent-hooks/server-cwd-attribution.test.ts +++ b/src/main/agent-hooks/server-cwd-attribution.test.ts @@ -10,8 +10,10 @@ vi.mock('../telemetry/cohort-classifier', () => ({ getCohortAtEmit: vi.fn() })) const AGENT_PANE = makePaneKey('tab-agent', '11111111-1111-4111-8111-111111111111') const AGENT_WORKTREE = 'repo-agent::/Users/dev/workspace/agent' -// Why: reproduces the shared-daemon leak — a session running in one project posts the -// pane identity it inherited from the pane that first spawned the agent daemon. +/** + * Hook body reproducing the shared-daemon leak: a session running in one project posts + * the pane identity it inherited from the pane that first spawned the agent daemon. + */ function buildBody(payload: Record): Record { return { paneKey: AGENT_PANE, From 683e17983ab7885f857c23043d393eaa3ed7af24 Mon Sep 17 00:00:00 2001 From: kunsanglee <85242378+kunsanglee@users.noreply.github.com> Date: Tue, 28 Jul 2026 15:40:04 +0900 Subject: [PATCH 04/12] fix(agent-hooks): keep a collapsed drive root readable as Windows notation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `C:\..\..` collapsed to a slashless `c:`, which no longer matched the drive-rooted test. A worktree or cwd in that shape would then be compared against a POSIX path as if both used the same notation, and the mixed-notation bail-out that keeps unclear pairs attributed could not fire — the one way this guard could drop a legitimate status row. Terminate collapsed drive roots with their slash and let the containment check accept any root-terminated prefix. --- src/shared/agent-hook-cwd-attribution.test.ts | 10 ++++++++++ src/shared/agent-hook-cwd-attribution.ts | 10 ++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/shared/agent-hook-cwd-attribution.test.ts b/src/shared/agent-hook-cwd-attribution.test.ts index 74259408c277..f8c416f1a06f 100644 --- a/src/shared/agent-hook-cwd-attribution.test.ts +++ b/src/shared/agent-hook-cwd-attribution.test.ts @@ -75,6 +75,16 @@ describe('hookCwdContradictsWorktree', () => { expect(hookCwdContradictsWorktree(WORKTREE, '/../../..')).toBe(false) }) + it('keeps a fully collapsed drive root recognizable as Windows notation', () => { + // Why: if `C:\..\..` collapsed to a slashless `c:`, it would stop reading as a drive + // path and get compared against POSIX paths, which can only produce a false conflict. + expect(hookCwdContradictsWorktree(`${REPO}::C:\\..\\..`, '/Users/dev/projects/api')).toBe(false) + expect(hookCwdContradictsWorktree(WORKTREE, 'C:\\..\\..')).toBe(false) + // Within Windows notation the drive root is above every path on that drive. + expect(hookCwdContradictsWorktree(`${REPO}::C:\\Users\\dev`, 'C:\\..')).toBe(false) + expect(hookCwdContradictsWorktree(`${REPO}::C:\\`, 'C:\\Users\\dev')).toBe(false) + }) + it('refuses to judge across notations a single host can express two ways', () => { // WSL reports the POSIX mount for a drive-rooted worktree, and UNC paths name the distro. expect(hookCwdContradictsWorktree(`${REPO}::C:\\Users\\dev\\api`, '/mnt/c/Users/dev/api')).toBe( diff --git a/src/shared/agent-hook-cwd-attribution.ts b/src/shared/agent-hook-cwd-attribution.ts index 8ffe7d7fd86c..5e3b57b6fdea 100644 --- a/src/shared/agent-hook-cwd-attribution.ts +++ b/src/shared/agent-hook-cwd-attribution.ts @@ -33,7 +33,13 @@ function collapseDotSegments(absolutePath: string): string { } segments.push(segment) } - return segments.length > 0 ? `${driveRoot}/${segments.join('/')}` : driveRoot || '/' + // Why: a collapsed drive root keeps its slash, else `c:` fails DRIVE_ROOTED and a + // Windows path would be compared against a POSIX one as if the notations matched. + return segments.length > 0 + ? `${driveRoot}/${segments.join('/')}` + : driveRoot + ? `${driveRoot}/` + : '/' } /** Reduce a path to a comparable form, or null when it has no root this can compare. */ @@ -48,7 +54,7 @@ function normalizeComparablePath(raw: string): string | null { /** True when `inner` is `outer` itself or sits beneath it. */ function isSameOrInside(inner: string, outer: string): boolean { - return inner === outer || inner.startsWith(outer === '/' ? '/' : `${outer}/`) + return inner === outer || inner.startsWith(outer.endsWith('/') ? outer : `${outer}/`) } /** From 3989d46b2a05d3dcd0ce17deea696d9fc61d8ee1 Mon Sep 17 00:00:00 2001 From: kunsanglee <85242378+kunsanglee@users.noreply.github.com> Date: Tue, 28 Jul 2026 17:00:18 +0900 Subject: [PATCH 05/12] refactor(agent-hooks): reuse the shared path layer for the cwd guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The guard hand-rolled path normalization that `cross-platform-path.ts` already provides. Two of this branch's earlier commits fixed bugs the reuse never had, and the copy missed NFC folding, so a non-ASCII workspace path dropped every status from that workspace (#10832). Delegate to `resolveRuntimePath` and `isPathInsideOrEqual`, keep the notation bails as guard policy, and drop `readHookPayloadCwd` in favour of `readBoundedString` — which also restores the 4096 cwd bound the hand-rolled reader lost. Stop persisting `sourceCwd`, a transport-only field the hydrate whitelist drops. --- src/main/agent-hooks/server.test.ts | 8 +- src/main/agent-hooks/server.ts | 13 ++- src/shared/agent-hook-cwd-attribution.test.ts | 56 ++++++------ src/shared/agent-hook-cwd-attribution.ts | 89 ++++++------------- src/shared/agent-hook-listener.ts | 19 ++-- 5 files changed, 82 insertions(+), 103 deletions(-) diff --git a/src/main/agent-hooks/server.test.ts b/src/main/agent-hooks/server.test.ts index e05e0c173121..92ef766edeb1 100644 --- a/src/main/agent-hooks/server.test.ts +++ b/src/main/agent-hooks/server.test.ts @@ -6339,7 +6339,7 @@ describe('Last-status persistence', () => { } }) - it('does not write prompt interaction keys to last-status.json', async () => { + it('does not write transport-only fields to last-status.json', async () => { const server = new AgentHookServer() await server.start({ env: 'production', @@ -6352,7 +6352,8 @@ describe('Last-status persistence', () => { hook_event_name: 'MessagePart', role: 'user', text: 'persist status only', - messageID: 'opencode-local-message-id' + messageID: 'opencode-local-message-id', + cwd: '/srv/session' }), '/hook/opencode' ) @@ -6360,6 +6361,9 @@ describe('Last-status persistence', () => { const file = JSON.parse(readFileSync(lastStatusPath(), 'utf8')) expect(file.entries[PANE].payload.prompt).toBe('persist status only') expect(file.entries[PANE].promptInteractionKey).toBeUndefined() + // Why: hydrate's whitelist drops both, so persisting them leaves the write dedupe + // comparing against bytes hydration can never reproduce. + expect(file.entries[PANE].sourceCwd).toBeUndefined() } finally { server.stop() } diff --git a/src/main/agent-hooks/server.ts b/src/main/agent-hooks/server.ts index 253ff64e2711..6c0942352ad1 100644 --- a/src/main/agent-hooks/server.ts +++ b/src/main/agent-hooks/server.ts @@ -17,6 +17,7 @@ import { createHookListenerState, getEndpointFileName, hasPendingAgentResultText, + HOOK_CWD_MAX_LENGTH, HOOK_REQUEST_SLOWLORIS_MS, markClaudeLeadTurnInterrupted, markCodexLeadTurnInterrupted, @@ -1573,8 +1574,8 @@ export class AgentHookServer { providerSessionOnly: envelope.providerSessionOnly === true ? true : undefined, isReplay: envelope.isReplay === true ? true : undefined, sourceCwd: - typeof envelope.sourceCwd === 'string' && envelope.sourceCwd.trim().length > 0 - ? envelope.sourceCwd.trim() + typeof envelope.sourceCwd === 'string' && envelope.sourceCwd.length <= HOOK_CWD_MAX_LENGTH + ? envelope.sourceCwd.trim() || undefined : undefined, payload: normalizedPayload } @@ -2034,7 +2035,13 @@ export class AgentHookServer { if (!isValidPaneKey(paneKey)) { continue } - const { promptInteractionKey: _promptInteractionKey, ...persistedPayload } = payload + // Why: transport-only fields the hydrate whitelist drops — persisting them would + // make hydration lossy and defeat the lastWrittenJson write dedupe. + const { + promptInteractionKey: _promptInteractionKey, + sourceCwd: _sourceCwd, + ...persistedPayload + } = payload entries[paneKey] = persistedPayload as EnrichedAgentHookEventPayload } const file: LastStatusFile = { version: LAST_STATUS_FILE_VERSION, entries } diff --git a/src/shared/agent-hook-cwd-attribution.test.ts b/src/shared/agent-hook-cwd-attribution.test.ts index f8c416f1a06f..7aedf1b89b04 100644 --- a/src/shared/agent-hook-cwd-attribution.test.ts +++ b/src/shared/agent-hook-cwd-attribution.test.ts @@ -1,22 +1,10 @@ import { describe, expect, it } from 'vitest' -import { hookCwdContradictsWorktree, readHookPayloadCwd } from './agent-hook-cwd-attribution' +import { hookCwdContradictsWorktree } from './agent-hook-cwd-attribution' import { FOLDER_WORKSPACE_INSTANCE_SEPARATOR } from './worktree-id' const REPO = 'repo-1' const WORKTREE = `${REPO}::/Users/dev/projects/api` -describe('readHookPayloadCwd', () => { - it('reads the cwd variants agents report, and nothing else', () => { - expect(readHookPayloadCwd({ cwd: '/Users/dev/projects/api' })).toBe('/Users/dev/projects/api') - expect(readHookPayloadCwd({ workspaceRoot: '/srv/app' })).toBe('/srv/app') - expect(readHookPayloadCwd({ workspace_root: '/srv/app' })).toBe('/srv/app') - expect(readHookPayloadCwd({ cwd: ' ' })).toBeUndefined() - expect(readHookPayloadCwd({ cwd: 42 })).toBeUndefined() - expect(readHookPayloadCwd(null)).toBeUndefined() - expect(readHookPayloadCwd('/srv/app')).toBeUndefined() - }) -}) - describe('hookCwdContradictsWorktree', () => { it('accepts a session running at or under the reported worktree', () => { expect(hookCwdContradictsWorktree(WORKTREE, '/Users/dev/projects/api')).toBe(false) @@ -42,8 +30,10 @@ describe('hookCwdContradictsWorktree', () => { expect(hookCwdContradictsWorktree(WORKTREE, '')).toBe(false) // Worktree ids without the `::` separator carry no path to compare. expect(hookCwdContradictsWorktree('onboarding-inline-terminal', '/Users/dev/x')).toBe(false) - // Relative cwd has no comparable root. + expect(hookCwdContradictsWorktree(`${REPO}::`, '/Users/dev/x')).toBe(false) + // Relative and untrimmed cwds have no comparable root. expect(hookCwdContradictsWorktree(WORKTREE, 'projects/agent')).toBe(false) + expect(hookCwdContradictsWorktree(WORKTREE, ' /Users/dev/projects/agent')).toBe(false) }) it('ignores case and trailing separators when comparing', () => { @@ -53,12 +43,29 @@ describe('hookCwdContradictsWorktree', () => { ).toBe(false) }) + it('treats canonically equivalent non-ASCII paths as the same directory', () => { + // Why: the folder picker stores NFD on macOS while agents report cwd in NFC, so a + // byte comparison would drop every status from a non-ASCII workspace (#10832). + const folder = '/Users/dev/projects/한글' + expect( + hookCwdContradictsWorktree( + `${REPO}::${folder.normalize('NFD')}`, + `${folder.normalize('NFC')}/src` + ) + ).toBe(false) + }) + it('compares Windows worktrees against Windows cwds', () => { const windowsWorktree = `${REPO}::C:\\Users\\dev\\api` expect(hookCwdContradictsWorktree(windowsWorktree, 'C:\\Users\\dev\\api\\src')).toBe(false) expect(hookCwdContradictsWorktree(windowsWorktree, 'C:/Users/dev/agent')).toBe(true) }) + it('treats a drive root as sitting above every path on that drive', () => { + expect(hookCwdContradictsWorktree(`${REPO}::C:\\Users\\dev`, 'C:\\..')).toBe(false) + expect(hookCwdContradictsWorktree(`${REPO}::C:\\`, 'C:\\Users\\dev')).toBe(false) + }) + it('resolves dot segments so a path cannot pose as being inside the worktree', () => { // Why: `…/api/../agent` starts with the worktree path as plain text but resolves outside it. expect(hookCwdContradictsWorktree(WORKTREE, '/Users/dev/projects/api/../agent')).toBe(true) @@ -75,27 +82,26 @@ describe('hookCwdContradictsWorktree', () => { expect(hookCwdContradictsWorktree(WORKTREE, '/../../..')).toBe(false) }) - it('keeps a fully collapsed drive root recognizable as Windows notation', () => { - // Why: if `C:\..\..` collapsed to a slashless `c:`, it would stop reading as a drive - // path and get compared against POSIX paths, which can only produce a false conflict. - expect(hookCwdContradictsWorktree(`${REPO}::C:\\..\\..`, '/Users/dev/projects/api')).toBe(false) - expect(hookCwdContradictsWorktree(WORKTREE, 'C:\\..\\..')).toBe(false) - // Within Windows notation the drive root is above every path on that drive. - expect(hookCwdContradictsWorktree(`${REPO}::C:\\Users\\dev`, 'C:\\..')).toBe(false) - expect(hookCwdContradictsWorktree(`${REPO}::C:\\`, 'C:\\Users\\dev')).toBe(false) - }) - it('refuses to judge across notations a single host can express two ways', () => { - // WSL reports the POSIX mount for a drive-rooted worktree, and UNC paths name the distro. + // WSL reports the POSIX mount for a drive-rooted worktree, and UNC names the same + // directory a third way — a mapped drive or a distro share. expect(hookCwdContradictsWorktree(`${REPO}::C:\\Users\\dev\\api`, '/mnt/c/Users/dev/api')).toBe( false ) + expect( + hookCwdContradictsWorktree( + `${REPO}::C:\\Users\\dev\\api`, + '\\\\wsl.localhost\\Ubuntu\\mnt\\c\\Users\\dev\\api' + ) + ).toBe(false) expect( hookCwdContradictsWorktree(`${REPO}::\\\\wsl$\\Ubuntu\\home\\dev\\api`, '/home/dev/api') ).toBe(false) expect( hookCwdContradictsWorktree(`${REPO}::/home/dev/api`, '\\\\wsl$\\Ubuntu\\home\\dev\\x') ).toBe(false) + expect(hookCwdContradictsWorktree(`${REPO}::C:\\..\\..`, '/Users/dev/projects/api')).toBe(false) + expect(hookCwdContradictsWorktree(WORKTREE, 'C:\\..\\..')).toBe(false) }) it('compares the real folder path of a folder-workspace instance id', () => { diff --git a/src/shared/agent-hook-cwd-attribution.ts b/src/shared/agent-hook-cwd-attribution.ts index 5e3b57b6fdea..e5526c2fc46b 100644 --- a/src/shared/agent-hook-cwd-attribution.ts +++ b/src/shared/agent-hook-cwd-attribution.ts @@ -1,61 +1,12 @@ +import { + isPathInsideOrEqual, + isRuntimePathAbsolute, + isWindowsAbsolutePathLike, + resolveRuntimePath +} from './cross-platform-path' import { splitWorktreeIdForFilesystem } from './worktree-id' -const DRIVE_ROOTED = /^[a-z]:\// -const CWD_PAYLOAD_KEYS = ['cwd', 'workspaceRoot', 'workspace_root'] as const - -/** Session cwd the agent reported in its own hook payload, for sources that expose one. */ -export function readHookPayloadCwd(hookPayload: unknown): string | undefined { - if (typeof hookPayload !== 'object' || hookPayload === null) { - return undefined - } - const record = hookPayload as Record - for (const key of CWD_PAYLOAD_KEYS) { - const value = record[key] - if (typeof value === 'string' && value.trim().length > 0) { - return value.trim() - } - } - return undefined -} - -/** Resolve `.` and `..` lexically so a path can't pose as living under another. */ -function collapseDotSegments(absolutePath: string): string { - const driveRoot = DRIVE_ROOTED.test(absolutePath) ? absolutePath.slice(0, 2) : '' - const segments: string[] = [] - for (const segment of absolutePath.slice(driveRoot.length).split('/')) { - if (segment === '' || segment === '.') { - continue - } - if (segment === '..') { - // Why: `/..` is the root itself, so an over-popping prefix must not escape it. - segments.pop() - continue - } - segments.push(segment) - } - // Why: a collapsed drive root keeps its slash, else `c:` fails DRIVE_ROOTED and a - // Windows path would be compared against a POSIX one as if the notations matched. - return segments.length > 0 - ? `${driveRoot}/${segments.join('/')}` - : driveRoot - ? `${driveRoot}/` - : '/' -} - -/** Reduce a path to a comparable form, or null when it has no root this can compare. */ -function normalizeComparablePath(raw: string): string | null { - const lowered = raw.trim().replace(/\\/g, '/').toLowerCase() - // Why: UNC (`//wsl$/…`) and relative paths have no comparable root — unknown, not conflicting. - if (lowered.startsWith('//') || (!lowered.startsWith('/') && !DRIVE_ROOTED.test(lowered))) { - return null - } - return collapseDotSegments(lowered) -} - -/** True when `inner` is `outer` itself or sits beneath it. */ -function isSameOrInside(inner: string, outer: string): boolean { - return inner === outer || inner.startsWith(outer.endsWith('/') ? outer : `${outer}/`) -} +const UNC_NOTATION = /^(?:\/\/|\\\\)/ /** * True when the worktree a hook claims cannot own the session that sent it, judged @@ -72,23 +23,33 @@ export function hookCwdContradictsWorktree( worktreeId: string | undefined, cwd: string | undefined ): boolean { - if (!worktreeId || !cwd) { + const worktreePath = worktreeId + ? splitWorktreeIdForFilesystem(worktreeId)?.worktreePath + : undefined + if (!worktreePath || !cwd) { return false } - const worktreePath = splitWorktreeIdForFilesystem(worktreeId)?.worktreePath - if (!worktreePath) { + // Why: a relative path has no root, so containment either way is unknowable. + if (!isRuntimePathAbsolute(worktreePath) || !isRuntimePathAbsolute(cwd)) { return false } - const workspace = normalizeComparablePath(worktreePath) - const session = normalizeComparablePath(cwd) - if (!workspace || !session) { + // Why: UNC aliases another notation — \\wsl$\Ubuntu\mnt\c\x is C:\x, and \\server\share + // is a mapped drive — so it can never disprove the other side. + if (UNC_NOTATION.test(worktreePath) || UNC_NOTATION.test(cwd)) { return false } // Why: a WSL session reports /mnt/c/… for a C:\… worktree; mixed notations aren't comparable. - if (DRIVE_ROOTED.test(workspace) !== DRIVE_ROOTED.test(session)) { + // Together with the UNC bail this leaves the guard inert across WSL, since translating + // either way needs the session's execution host, which the hook server doesn't know. + if (isWindowsAbsolutePathLike(worktreePath) !== isWindowsAbsolutePathLike(cwd)) { return false } + // Why: fold case even on POSIX, which normalizeRuntimePathForComparison deliberately + // won't. Its callers pick candidates, so a missed match costs them nothing; here a + // missed match drops a live status row, and macOS and Windows are case-insensitive. + const workspace = resolveRuntimePath(worktreePath.toLowerCase(), '.') + const session = resolveRuntimePath(cwd.toLowerCase(), '.') // Why: a session started in a subdirectory is normal, and so is a workspace nested // under the session root (folder workspaces); only fully disjoint paths are proof. - return !isSameOrInside(session, workspace) && !isSameOrInside(workspace, session) + return !isPathInsideOrEqual(workspace, session) && !isPathInsideOrEqual(session, workspace) } diff --git a/src/shared/agent-hook-listener.ts b/src/shared/agent-hook-listener.ts index 163b5e0a4b01..13cd4a1c7d65 100644 --- a/src/shared/agent-hook-listener.ts +++ b/src/shared/agent-hook-listener.ts @@ -25,7 +25,6 @@ import { type ParsedAgentStatusPayload } from './agent-status-types' import { normalizeOptionalField } from './agent-status-field-normalization' -import { readHookPayloadCwd } from './agent-hook-cwd-attribution' import { isAskUserQuestionTool } from './agent-question-answered-intent' import { claudeRosterHasWorkingSubagent, @@ -298,7 +297,9 @@ export type AgentHookEventPayload = { providerSessionOnly?: boolean /** True when this event is a relay cache replay rather than a live hook. */ isReplay?: boolean - /** Session cwd from the agent's own payload; cross-checks the env-derived pane attribution. */ + /** Session cwd from the agent's own payload; cross-checks the env-derived pane attribution. + * Whoever populates this must run `hookCwdContradictsWorktree` before applying the status — + * an ingest path that carries the field but skips the guard re-opens the mis-attribution. */ sourceCwd?: string payload: ParsedAgentStatusPayload } @@ -841,7 +842,9 @@ const TRANSCRIPT_MAX_SCAN_BYTES = 4 * 1024 * 1024 const EMPTY_TRANSCRIPT_REGION = Buffer.alloc(0) const AMP_THREAD_ID_MAX_LENGTH = 256 const AMP_MAX_SCOPED_THREAD_CACHE_KEYS = 32 -const GROK_SESSION_CWD_MAX_LENGTH = 4096 +/** Keys agents use for the session cwd; Grok spells it two extra ways. */ +const HOOK_CWD_KEYS = ['cwd', 'workspaceRoot', 'workspace_root'] as const +export const HOOK_CWD_MAX_LENGTH = 4096 const GROK_HOME_ENVELOPE_MAX_LENGTH = 4096 function extractAssistantTextFromLine(line: string): string | undefined { @@ -1184,11 +1187,7 @@ function readGrokSessionMetadata( if (!sessionId || !isSafeGrokSessionId(sessionId)) { return undefined } - const cwd = readBoundedString( - hookPayload, - ['cwd', 'workspaceRoot', 'workspace_root'], - GROK_SESSION_CWD_MAX_LENGTH - ) + const cwd = readBoundedString(hookPayload, HOOK_CWD_KEYS, HOOK_CWD_MAX_LENGTH) // Why: hook scripts report the effective per-PTY/remote Grok home; old scripts fall back to the runtime's for compatibility. const sessionsDir = grokHome ? join(grokHome, 'sessions') @@ -4022,7 +4021,9 @@ export function normalizeHookPayload( toolAgentType: readString(hookPayloadRecord, 'agent_type'), ...(providerSession ? { providerSession } : {}), ...(providerSessionOnly ? { providerSessionOnly: true } : {}), - sourceCwd: readHookPayloadCwd(hookPayloadRecord), + sourceCwd: + readBoundedString(hookPayloadRecord, HOOK_CWD_KEYS, HOOK_CWD_MAX_LENGTH)?.trim() || + undefined, payload: transportPayload } : null From 36cd6e28c0a2ad0ca22128397a204d2c1d05c353 Mon Sep 17 00:00:00 2001 From: kunsanglee <85242378+kunsanglee@users.noreply.github.com> Date: Tue, 28 Jul 2026 17:06:17 +0900 Subject: [PATCH 06/12] fix(agent-hooks): report a mis-attributing daemon once per runtime MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cwd guard emitted telemetry on every dropped hook. A daemon that mis-attributes one pane fires on every hook of every session it hosts, and the per-session telemetry ceiling is shared by all events and never refills — so the guard could silence the rest of the session's telemetry within the hour. Move the track call inside the warn-once gate that already existed beside it; `reason` carries no pane, so repeat events add no signal. --- .../server-cwd-attribution.test.ts | 23 +++++++++++++++++++ src/main/agent-hooks/server.ts | 13 ++++++----- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/main/agent-hooks/server-cwd-attribution.test.ts b/src/main/agent-hooks/server-cwd-attribution.test.ts index e2e6ae1f5c53..147e82c71195 100644 --- a/src/main/agent-hooks/server-cwd-attribution.test.ts +++ b/src/main/agent-hooks/server-cwd-attribution.test.ts @@ -94,6 +94,29 @@ describe('AgentHookServer cwd attribution guard', () => { }) }) + it('reports a mis-attributing daemon once per runtime', () => { + // Why: the telemetry per-session ceiling never refills, so a daemon that mis-attributes + // every hook it hosts would otherwise silence every other event in the session. + const server = new AgentHookServer() + for (const prompt of ['first', 'second', 'third']) { + server.ingestRemote( + { + paneKey: AGENT_PANE, + tabId: 'tab-agent', + worktreeId: AGENT_WORKTREE, + sourceCwd: '/srv/other-project', + payload: { state: 'working', prompt } + }, + 'conn-1' + ) + } + + expect(server.getStatusSnapshot()).toEqual([]) + expect( + trackMock.mock.calls.filter(([name]) => name === 'agent_hook_unattributed') + ).toHaveLength(1) + }) + it('keeps hooks that report no cwd, so sources without one stay attributed', () => { const server = new AgentHookServer() server.ingestRemote( diff --git a/src/main/agent-hooks/server.ts b/src/main/agent-hooks/server.ts index 6c0942352ad1..37b8b94b0ce3 100644 --- a/src/main/agent-hooks/server.ts +++ b/src/main/agent-hooks/server.ts @@ -491,8 +491,9 @@ export class AgentHookServer { private closedAgentStatusTabIds = new Set() private closedAgentStatusPaneKeys = new Set() private connectionTimestampWatermarkById = new Map() - // Why: one console line per runtime — a mis-attributing daemon fires on every hook of every session it hosts. - private warnedForeignCwdStatus = false + // Why: one warn and one telemetry event per runtime — a mis-attributing daemon fires on every + // hook of every session it hosts, and the per-session telemetry ceiling never refills. + private reportedForeignCwdStatus = false // Why: skip disk writes when the JSON exactly matches the last write; guards against re-firing trailing timers when nothing changed. private lastWrittenJson: string | null = null @@ -780,9 +781,9 @@ export class AgentHookServer { if (!hookCwdContradictsWorktree(event.worktreeId, event.sourceCwd)) { return false } - track('agent_hook_unattributed', { reason: 'cwd_worktree_mismatch' }) - if (!this.warnedForeignCwdStatus) { - this.warnedForeignCwdStatus = true + if (!this.reportedForeignCwdStatus) { + this.reportedForeignCwdStatus = true + track('agent_hook_unattributed', { reason: 'cwd_worktree_mismatch' }) console.warn( '[agent-hooks] dropping status: reported worktree does not own the reporting session', { paneKey: event.paneKey, worktreeId: event.worktreeId, sourceCwd: event.sourceCwd } @@ -1714,7 +1715,7 @@ export class AgentHookServer { this.lastStatusFilePath = null this.lastWrittenJson = null this.runtimeObservedStatusPaneKeys.clear() - this.warnedForeignCwdStatus = false + this.reportedForeignCwdStatus = false this.promptSentDedupeByPaneKey.clear() this.closedAgentStatusTabIds.clear() this.closedAgentStatusPaneKeys.clear() From ddbced31d117f89ccb29f0888c540af6390f153c Mon Sep 17 00:00:00 2001 From: kunsanglee <85242378+kunsanglee@users.noreply.github.com> Date: Tue, 28 Jul 2026 18:14:04 +0900 Subject: [PATCH 07/12] test(agent-hooks): pin the per-runtime telemetry reset The once-per-runtime cap was covered but the `stop()` reset that lifts it was not, so removing that line failed no test. Add a restart case, and fold the thrice-repeated relay envelope into one local builder. --- .../server-cwd-attribution.test.ts | 55 +++++++++++-------- 1 file changed, 31 insertions(+), 24 deletions(-) diff --git a/src/main/agent-hooks/server-cwd-attribution.test.ts b/src/main/agent-hooks/server-cwd-attribution.test.ts index 147e82c71195..b45c3698f273 100644 --- a/src/main/agent-hooks/server-cwd-attribution.test.ts +++ b/src/main/agent-hooks/server-cwd-attribution.test.ts @@ -24,6 +24,25 @@ function buildBody(payload: Record): Record { } } +/** Relay a hook for the agent pane from a session running somewhere else entirely. */ +function ingestForeign(server: AgentHookServer, prompt: string): void { + server.ingestRemote( + { + paneKey: AGENT_PANE, + tabId: 'tab-agent', + worktreeId: AGENT_WORKTREE, + // Why: the relay forwards cwd beside the payload — normalization strips it from the payload itself. + sourceCwd: '/srv/other-project', + payload: { state: 'working', prompt } + }, + 'conn-1' + ) +} + +function unattributedCallCount(): number { + return trackMock.mock.calls.filter(([name]) => name === 'agent_hook_unattributed').length +} + beforeEach(() => { trackMock.mockReset() }) @@ -76,17 +95,7 @@ describe('AgentHookServer cwd attribution guard', () => { it('drops a relayed hook whose session cwd belongs to another workspace', () => { const server = new AgentHookServer() - server.ingestRemote( - { - paneKey: AGENT_PANE, - tabId: 'tab-agent', - worktreeId: AGENT_WORKTREE, - // Why: the relay forwards cwd beside the payload — normalization strips it from the payload itself. - sourceCwd: '/srv/other-project', - payload: { state: 'working', prompt: 'foreign session' } - }, - 'conn-1' - ) + ingestForeign(server, 'foreign session') expect(server.getStatusSnapshot()).toEqual([]) expect(trackMock).toHaveBeenCalledWith('agent_hook_unattributed', { @@ -99,22 +108,20 @@ describe('AgentHookServer cwd attribution guard', () => { // every hook it hosts would otherwise silence every other event in the session. const server = new AgentHookServer() for (const prompt of ['first', 'second', 'third']) { - server.ingestRemote( - { - paneKey: AGENT_PANE, - tabId: 'tab-agent', - worktreeId: AGENT_WORKTREE, - sourceCwd: '/srv/other-project', - payload: { state: 'working', prompt } - }, - 'conn-1' - ) + ingestForeign(server, prompt) } expect(server.getStatusSnapshot()).toEqual([]) - expect( - trackMock.mock.calls.filter(([name]) => name === 'agent_hook_unattributed') - ).toHaveLength(1) + expect(unattributedCallCount()).toBe(1) + }) + + it('reports again after a restart, so one runtime does not silence the next', () => { + const server = new AgentHookServer() + ingestForeign(server, 'before restart') + server.stop() + ingestForeign(server, 'after restart') + + expect(unattributedCallCount()).toBe(2) }) it('keeps hooks that report no cwd, so sources without one stay attributed', () => { From 43c2034218a1151d0d83249703490a45fe979e70 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 30 Jul 2026 08:30:35 -0700 Subject: [PATCH 08/12] fix(agent-hooks): forward sourceCwd across the SSH relay ingest hop The mux-notification handler rebuilds the ingestRemote envelope field by field and omitted sourceCwd, leaving the cwd attribution guard inert for every SSH event (live and replay). --- .../ssh/ssh-relay-session-agent-hooks.integration.test.ts | 3 +++ src/main/ssh/ssh-relay-session.ts | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/src/main/ssh/ssh-relay-session-agent-hooks.integration.test.ts b/src/main/ssh/ssh-relay-session-agent-hooks.integration.test.ts index bf0a1891cb4a..258dbb97fae4 100644 --- a/src/main/ssh/ssh-relay-session-agent-hooks.integration.test.ts +++ b/src/main/ssh/ssh-relay-session-agent-hooks.integration.test.ts @@ -478,6 +478,8 @@ describe('SshRelaySession agent hooks over a fake relay transport', () => { toolUseId: 'toolu-1', toolAgentId: 'agent-subagent-a', toolAgentType: 'Review', + // Why: the cwd attribution guard is inert for every SSH event if this hop drops sourceCwd. + sourceCwd: '/srv/remote-session', providerSessionOnly: true, providerSession: { key: 'session_id', @@ -500,6 +502,7 @@ describe('SshRelaySession agent hooks over a fake relay transport', () => { toolUseId: 'toolu-1', toolAgentId: 'agent-subagent-a', toolAgentType: 'Review', + sourceCwd: '/srv/remote-session', providerSessionOnly: true, providerSession: { key: 'session_id', diff --git a/src/main/ssh/ssh-relay-session.ts b/src/main/ssh/ssh-relay-session.ts index aef993f8eef5..39367745dde2 100644 --- a/src/main/ssh/ssh-relay-session.ts +++ b/src/main/ssh/ssh-relay-session.ts @@ -1159,6 +1159,7 @@ export class SshRelaySession { isReplay?: unknown providerSession?: unknown providerSessionOnly?: unknown + sourceCwd?: unknown payload?: unknown } if (typeof envelope.paneKey !== 'string') { @@ -1187,6 +1188,9 @@ export class SshRelaySession { isReplay: envelope.isReplay === true ? true : undefined, providerSession: envelope.providerSession, providerSessionOnly: envelope.providerSessionOnly === true ? true : undefined, + // Why: without this pass-through the cwd attribution guard in ingestRemote sees no + // cwd and stays inert for every SSH event; ingestRemote re-bounds and trims it. + sourceCwd: typeof envelope.sourceCwd === 'string' ? envelope.sourceCwd : undefined, payload: envelope.payload }, this.targetId From 30bf75c32518297d71703216ecd6a8c44a58be96 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 30 Jul 2026 08:30:45 -0700 Subject: [PATCH 09/12] fix(agent-hooks): refuse foreign-cwd hooks before listener state, resolving symlink aliases - Re-judge a would-drop on locally resolved paths so one directory spelled two ways (macOS /tmp vs /private/tmp, symlinked roots) is not read as a foreign session; the relay strips sourceCwd it proved alias-clean so Orca's raw re-guard cannot re-drop the row. - Run the guard before normalizeHookPayload on both local and relay HTTP ingest so a foreign daemon-hosted session cannot seed per-pane subagent rosters or the replay cache that later legitimate events re-emit. - Warn per pane (bounded) instead of once per runtime; telemetry stays latched. Exclude sourceCwd from the persisted payload type. --- .../server-cwd-attribution.test.ts | 93 +++++++++++++++++++ src/main/agent-hooks/server.ts | 59 +++++++++--- src/relay/agent-hook-server.test.ts | 84 ++++++++++++++++- src/relay/agent-hook-server.ts | 46 +++++++++ src/shared/agent-hook-cwd-attribution.test.ts | 63 ++++++++++++- src/shared/agent-hook-cwd-attribution.ts | 39 +++++++- src/shared/agent-hook-listener.ts | 22 +++++ 7 files changed, 390 insertions(+), 16 deletions(-) diff --git a/src/main/agent-hooks/server-cwd-attribution.test.ts b/src/main/agent-hooks/server-cwd-attribution.test.ts index b45c3698f273..9a8f7ca1e1c4 100644 --- a/src/main/agent-hooks/server-cwd-attribution.test.ts +++ b/src/main/agent-hooks/server-cwd-attribution.test.ts @@ -1,3 +1,6 @@ +import { mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' import { beforeEach, describe, expect, it, vi } from 'vitest' import { AgentHookServer } from './server' import { makePaneKey } from '../../shared/stable-pane-id' @@ -124,6 +127,96 @@ describe('AgentHookServer cwd attribution guard', () => { expect(unattributedCallCount()).toBe(2) }) + it('refuses a foreign hook before it can seed the pane subagent roster', async () => { + // Why: normalization mutates per-pane listener state; if the drop happens after it, a + // foreign SubagentStart still plants a roster row that the pane's own next event re-emits. + const server = new AgentHookServer() + await server.start({ env: 'production' }) + try { + const env = server.buildPtyEnv() + const postHook = (payload: Record): Promise => + fetch(`http://127.0.0.1:${env.ORCA_AGENT_HOOK_PORT}/hook/claude`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-Orca-Agent-Hook-Token': env.ORCA_AGENT_HOOK_TOKEN + }, + body: JSON.stringify(buildBody(payload)) + }) + + await expect( + postHook({ + hook_event_name: 'SubagentStart', + agent_id: 'sa-foreign', + cwd: '/Users/dev/projects/api' + }) + ).resolves.toMatchObject({ status: 204 }) + await expect( + postHook({ + hook_event_name: 'UserPromptSubmit', + prompt: 'own session', + cwd: '/Users/dev/workspace/agent' + }) + ).resolves.toMatchObject({ status: 204 }) + + const rows = server.getStatusSnapshot() + expect(rows).toEqual([ + expect.objectContaining({ paneKey: AGENT_PANE, prompt: 'own session' }) + ]) + expect(rows[0]?.subagents ?? []).toEqual([]) + } finally { + server.stop() + } + }) + + it('keeps a status whose cwd is only a symlink alias of the worktree path', async () => { + // Why: Orca stores the workspace path as picked while agents report physical getcwd + // (macOS /tmp is /private/tmp); one directory spelled two ways is not a foreign session. + const base = mkdtempSync(join(tmpdir(), 'orca-hook-cwd-')) + const real = join(base, 'real-workspace') + mkdirSync(real, { recursive: true }) + const link = join(base, 'linked-workspace') + try { + symlinkSync(real, link, process.platform === 'win32' ? 'junction' : 'dir') + } catch { + rmSync(base, { recursive: true, force: true }) + return // Restricted hosts that cannot create links have nothing to verify here. + } + const server = new AgentHookServer() + await server.start({ env: 'production' }) + try { + const env = server.buildPtyEnv() + const res = await fetch(`http://127.0.0.1:${env.ORCA_AGENT_HOOK_PORT}/hook/claude`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-Orca-Agent-Hook-Token': env.ORCA_AGENT_HOOK_TOKEN + }, + body: JSON.stringify({ + paneKey: AGENT_PANE, + tabId: 'tab-agent', + worktreeId: `repo-agent::${link}`, + env: 'production', + payload: { + hook_event_name: 'UserPromptSubmit', + prompt: 'aliased session', + cwd: realpathSync(real) + } + }) + }) + expect(res.status).toBe(204) + expect(server.getStatusSnapshot()).toEqual([ + expect.objectContaining({ paneKey: AGENT_PANE, prompt: 'aliased session' }) + ]) + expect(trackMock).not.toHaveBeenCalledWith('agent_hook_unattributed', { + reason: 'cwd_worktree_mismatch' + }) + } finally { + server.stop() + rmSync(base, { recursive: true, force: true }) + } + }) + it('keeps hooks that report no cwd, so sources without one stay attributed', () => { const server = new AgentHookServer() server.ingestRemote( diff --git a/src/main/agent-hooks/server.ts b/src/main/agent-hooks/server.ts index bb66e2771837..ef0663a70afd 100644 --- a/src/main/agent-hooks/server.ts +++ b/src/main/agent-hooks/server.ts @@ -9,7 +9,10 @@ import { track } from '../telemetry/client' import { getCohortAtEmit } from '../telemetry/cohort-classifier' import { AGENT_KIND_VALUES, type AgentKind } from '../../shared/telemetry-events' import { ORCA_HOOK_PROTOCOL_VERSION } from '../../shared/agent-hook-types' -import { hookCwdContradictsWorktree } from '../../shared/agent-hook-cwd-attribution' +import { + hookCwdContradictsWorktree, + hookCwdContradictsWorktreeAfterLocalResolve +} from '../../shared/agent-hook-cwd-attribution' import { clearAllListenerCaches, clearPaneCacheState, @@ -26,6 +29,7 @@ import { movePaneCacheState, normalizeHookPayload, parseFormEncodedBody, + readHookBodyCwdAttribution, readRequestBody, reapRestoredClaudeSubagentsForDeadPane, reconcileRemoteCodexState, @@ -89,7 +93,7 @@ type EnrichedAgentHookEventPayload = AgentHookEventPayload & { type PersistedAgentHookEventPayload = Omit< EnrichedAgentHookEventPayload, - 'launchToken' | 'promptInteractionKey' + 'launchToken' | 'promptInteractionKey' | 'sourceCwd' > & { launchTokenHash?: string } @@ -159,6 +163,9 @@ const AGENT_PROMPT_SENT_AGENT_KINDS = new Set(AGENT_KIND_VALUES) // Why: bound file growth from PTYs that never re-attach; 7 days is the "still relevant?" horizon beyond which entries shouldn't resurrect on hydrate. const HYDRATE_MAX_AGE_MS = 7 * 24 * 60 * 60 * 1000 +// Why: paneKey in the warn set is attacker-influenced pre-validation input; cap membership so it can't grow unboundedly. +const FOREIGN_CWD_WARN_PANE_CAP = 64 + // Why: a long-closed tab can't receive status events; bound the set so it can't grow one entry per close for the whole session. export const CLOSED_AGENT_STATUS_TAB_IDS_MAX = 1024 export const CLOSED_AGENT_STATUS_PANE_KEYS_MAX = 1024 @@ -592,9 +599,11 @@ export class AgentHookServer { private closedAgentStatusTabIds = new Set() private closedAgentStatusPaneKeys = new Set() private connectionTimestampWatermarkById = new Map() - // Why: one warn and one telemetry event per runtime — a mis-attributing daemon fires on every - // hook of every session it hosts, and the per-session telemetry ceiling never refills. - private reportedForeignCwdStatus = false + // Why: telemetry once per runtime — a mis-attributing daemon fires on every hook of every + // session it hosts, and the per-session telemetry ceiling never refills. The warn is per + // pane (bounded) so a second workspace going dark is still diagnosable from the log. + private reportedForeignCwdTelemetry = false + private warnedForeignCwdPaneKeys = new Set() // Why: skip disk writes when the JSON exactly matches the last write; guards against re-firing trailing timers when nothing changed. private lastWrittenJson: string | null = null @@ -924,14 +933,28 @@ export class AgentHookServer { return this.closedAgentStatusTabIds.has(tabId) } - /** Refuse a hook whose reported worktree is disproven by the session cwd it carries. */ - private shouldSuppressForeignCwdStatus(event: AgentHookEventPayload): boolean { - if (!hookCwdContradictsWorktree(event.worktreeId, event.sourceCwd)) { + /** Refuse a hook whose reported worktree is disproven by the session cwd it carries. + * `resolveLocalAliases` is true only where this host owns both paths (local HTTP ingest); + * remote events were already alias-resolved by the relay that owns them. */ + private shouldSuppressForeignCwdStatus( + event: Pick, + resolveLocalAliases: boolean + ): boolean { + const contradicts = resolveLocalAliases + ? hookCwdContradictsWorktreeAfterLocalResolve(event.worktreeId, event.sourceCwd) + : hookCwdContradictsWorktree(event.worktreeId, event.sourceCwd) + if (!contradicts) { return false } - if (!this.reportedForeignCwdStatus) { - this.reportedForeignCwdStatus = true + if (!this.reportedForeignCwdTelemetry) { + this.reportedForeignCwdTelemetry = true track('agent_hook_unattributed', { reason: 'cwd_worktree_mismatch' }) + } + if ( + !this.warnedForeignCwdPaneKeys.has(event.paneKey) && + this.warnedForeignCwdPaneKeys.size < FOREIGN_CWD_WARN_PANE_CAP + ) { + this.warnedForeignCwdPaneKeys.add(event.paneKey) console.warn( '[agent-hooks] dropping status: reported worktree does not own the reporting session', { paneKey: event.paneKey, worktreeId: event.worktreeId, sourceCwd: event.sourceCwd } @@ -1855,7 +1878,7 @@ export class AgentHookServer { : undefined, payload: normalizedPayload } - if (this.shouldSuppressForeignCwdStatus(event)) { + if (this.shouldSuppressForeignCwdStatus(event, false)) { return } this.recordCurrentAuthorityObservation(event) @@ -1929,11 +1952,20 @@ export class AgentHookServer { trackEmptyPaneKeyHook(body) const aliasedBody = this.normalizeHookBodyPaneKeyAlias(body) + // Why: refuse before normalization — a foreign event must not seed per-pane listener + // state (subagent rosters, lead-turn records) that later legitimate events re-emit. + if (this.shouldSuppressForeignCwdStatus(readHookBodyCwdAttribution(aliasedBody), true)) { + res.writeHead(204) + res.end() + return + } const normalized = normalizeHookPayload(this.state, source, aliasedBody, this.env) if ( normalized && !this.shouldSuppressClosedTabStatus(normalized.paneKey) && - !this.shouldSuppressForeignCwdStatus(normalized) + // Why: redundant with the pre-normalization refusal by construction; kept as drift + // armor should the raw-body read and the normalizer ever disagree on a field. + !this.shouldSuppressForeignCwdStatus(normalized, true) ) { this.recordCurrentAuthorityObservation(normalized) const enriched = this.applyNormalizedStatus(normalized) @@ -2002,7 +2034,8 @@ export class AgentHookServer { this.lastStatusFilePath = null this.lastWrittenJson = null this.runtimeObservedStatusPaneKeys.clear() - this.reportedForeignCwdStatus = false + this.reportedForeignCwdTelemetry = false + this.warnedForeignCwdPaneKeys.clear() this.hydratedAuthorityCommitments = Object.freeze([]) this.hydratedLaunchTokenHashByPaneKey.clear() this.persistedAuthorityCommitmentsByPaneKey.clear() diff --git a/src/relay/agent-hook-server.test.ts b/src/relay/agent-hook-server.test.ts index d107f9846bb8..f2f136b1b678 100644 --- a/src/relay/agent-hook-server.test.ts +++ b/src/relay/agent-hook-server.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync, writeFileSync } from 'node:fs' import { homedir, tmpdir } from 'node:os' import { join } from 'node:path' import { endpointDirForRelaySocket, RelayAgentHookServer } from './agent-hook-server' @@ -77,6 +77,88 @@ describe('RelayAgentHookServer', () => { } }) + it('refuses a hook whose cwd disproves the reported worktree before caching or forwarding', async () => { + // Why: this host owns the session's paths, so the relay is the authoritative place to + // refuse a daemon-inherited pane identity — before it can seed listener state, occupy + // the one-slot replay cache, or reach Orca at all. + const forward = vi.fn<(envelope: AgentHookRelayEnvelope) => void>() + const server = new RelayAgentHookServer({ endpointDir: dir, forward }) + await server.start() + try { + const { port, token } = server.getCoordinates() + const res = await fetch(`http://127.0.0.1:${port}/hook/claude`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-Orca-Agent-Hook-Token': token + }, + body: JSON.stringify({ + paneKey: PANE_KEY, + tabId: 'tab-1', + worktreeId: 'repo-1::/nonexistent-orca-relay/worktree-a', + env: 'remote', + version: '1', + payload: { + hook_event_name: 'UserPromptSubmit', + prompt: 'foreign session', + cwd: '/nonexistent-orca-relay/session-b' + } + }) + }) + expect(res.status).toBe(204) + expect(forward).not.toHaveBeenCalled() + expect(server.replayCachedPayloadsForPanes()).toBe(0) + } finally { + server.stop() + } + }) + + it('strips sourceCwd when the contradiction is only symlink aliasing of the worktree', async () => { + // Why: raw disjoint but resolved nested means the same directory spelled two ways. + // Orca's re-guard cannot resolve this host's paths, so forwarding the cwd would make + // it re-drop a proven-legitimate row. + const real = join(dir, 'real-workspace') + mkdirSync(real, { recursive: true }) + const link = join(dir, 'linked-workspace') + try { + symlinkSync(real, link, process.platform === 'win32' ? 'junction' : 'dir') + } catch { + return // Restricted hosts that cannot create links have nothing to verify here. + } + const forward = vi.fn<(envelope: AgentHookRelayEnvelope) => void>() + const server = new RelayAgentHookServer({ endpointDir: dir, forward }) + await server.start() + try { + const { port, token } = server.getCoordinates() + const res = await fetch(`http://127.0.0.1:${port}/hook/claude`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-Orca-Agent-Hook-Token': token + }, + body: JSON.stringify({ + paneKey: PANE_KEY, + tabId: 'tab-1', + worktreeId: `repo-1::${link}`, + env: 'remote', + version: '1', + payload: { + hook_event_name: 'UserPromptSubmit', + prompt: 'aliased session', + cwd: realpathSync(real) + } + }) + }) + expect(res.status).toBe(204) + expect(forward).toHaveBeenCalledTimes(1) + const envelope = forward.mock.calls[0][0] + expect(envelope.payload.prompt).toBe('aliased session') + expect(envelope.sourceCwd).toBeUndefined() + } finally { + server.stop() + } + }) + it('rejects requests with the wrong bearer token (403)', async () => { const forward = vi.fn() const server = new RelayAgentHookServer({ endpointDir: dir, forward }) diff --git a/src/relay/agent-hook-server.ts b/src/relay/agent-hook-server.ts index 812b21108037..72fb3afdb696 100644 --- a/src/relay/agent-hook-server.ts +++ b/src/relay/agent-hook-server.ts @@ -9,6 +9,10 @@ import { basename, dirname, join } from 'node:path' import { homedir } from 'node:os' import { ORCA_HOOK_PROTOCOL_VERSION } from '../shared/agent-hook-types' +import { + hookCwdContradictsWorktree, + hookCwdContradictsWorktreeAfterLocalResolve +} from '../shared/agent-hook-cwd-attribution' import { clearAllListenerCaches, clearPaneCacheState, @@ -19,6 +23,7 @@ import { HOOK_REQUEST_SLOWLORIS_MS, normalizeHookPayload, preparePendingGrokResultDiscovery, + readHookBodyCwdAttribution, readRequestBody, resolveHookSource, writeEndpointFile, @@ -42,6 +47,7 @@ const CODEX_SUBAGENT_POLL_MS = 1_000 // Why: cap env/version at 64 chars so a misbehaving agent CLI can't grow the meta cache unboundedly; canonical values are short. const MAX_HOOK_META_LEN = 64 +const FOREIGN_CWD_WARN_PANE_CAP = 64 // Why: WSL relay has no per-pane teardown (PTYs live on the Windows host), so the replay cache would grow forever without a recency cap. const MAX_CACHED_PANES = 256 @@ -108,6 +114,9 @@ export class RelayAgentHookServer { private fixedToken: string | undefined private preferredPort: number private portFallbackApplied = false + // Why: one stderr line per pane — a mis-attributing daemon fires on every hook it hosts. + // Bounded because paneKey here is pre-validation input. + private warnedForeignCwdPaneKeys = new Set() constructor(options: RelayHookServerOptions) { this.env = options.env ?? REMOTE_AGENT_HOOK_ENV @@ -202,6 +211,20 @@ export class RelayAgentHookServer { this.codexSubagentPollTimers.clear() clearAllListenerCaches(this.state) this.lastEnvelopeMetaByPaneKey.clear() + this.warnedForeignCwdPaneKeys.clear() + } + + private warnForeignCwdOnce(paneKey: string, worktreeId?: string, sourceCwd?: string): void { + if ( + this.warnedForeignCwdPaneKeys.has(paneKey) || + this.warnedForeignCwdPaneKeys.size >= FOREIGN_CWD_WARN_PANE_CAP + ) { + return + } + this.warnedForeignCwdPaneKeys.add(paneKey) + process.stderr.write( + `[relay-hook-server] dropping status: reported worktree does not own the reporting session (paneKey=${paneKey} worktreeId=${worktreeId ?? ''} sourceCwd=${sourceCwd ?? ''})\n` + ) } /** Request-driven replay: re-forwards each cached paneKey payload as a fresh notification. Forwards are @@ -275,8 +298,31 @@ export class RelayAgentHookServer { res.end() return } + // Why: same refusal as Orca's local ingest, run on the host that owns the session's + // paths — before normalization, so a foreign daemon-hosted session cannot seed + // per-pane listener state (subagent rosters, lead-turn records) or the replay cache. + const attribution = readHookBodyCwdAttribution(body) + const rawCwdContradiction = hookCwdContradictsWorktree( + attribution.worktreeId, + attribution.sourceCwd + ) + if ( + rawCwdContradiction && + hookCwdContradictsWorktreeAfterLocalResolve(attribution.worktreeId, attribution.sourceCwd) + ) { + this.warnForeignCwdOnce(attribution.paneKey, attribution.worktreeId, attribution.sourceCwd) + res.writeHead(204) + res.end() + return + } const event = normalizeHookPayload(this.state, source, body, this.env) if (event) { + if (rawCwdContradiction) { + // Why: this host proved the contradiction is symlink aliasing (raw disjoint, resolved + // nested). Orca's re-guard cannot resolve remote paths, so forwarding the cwd would + // make it re-drop a proven-legitimate row. + event.sourceCwd = undefined + } // TODO: once normalizeHookPayload returns validated env/version, drop bodyEnv/bodyVersion and source them from the listener result. const env = this.bodyEnv(body) const version = this.bodyVersion(body) diff --git a/src/shared/agent-hook-cwd-attribution.test.ts b/src/shared/agent-hook-cwd-attribution.test.ts index 7aedf1b89b04..fc17ede57d04 100644 --- a/src/shared/agent-hook-cwd-attribution.test.ts +++ b/src/shared/agent-hook-cwd-attribution.test.ts @@ -1,5 +1,11 @@ +import { mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' import { describe, expect, it } from 'vitest' -import { hookCwdContradictsWorktree } from './agent-hook-cwd-attribution' +import { + hookCwdContradictsWorktree, + hookCwdContradictsWorktreeAfterLocalResolve +} from './agent-hook-cwd-attribution' import { FOLDER_WORKSPACE_INSTANCE_SEPARATOR } from './worktree-id' const REPO = 'repo-1' @@ -111,3 +117,58 @@ describe('hookCwdContradictsWorktree', () => { expect(hookCwdContradictsWorktree(folderWorkspace, '/Users/dev/projects/api')).toBe(true) }) }) + +describe('hookCwdContradictsWorktreeAfterLocalResolve', () => { + it('clears a contradiction that is only symlink aliasing of the same directory', () => { + // Why: Orca stores the path as picked while agents report physical getcwd — macOS /tmp + // is /private/tmp, project roots sit behind symlinks — so raw strings can be fully + // disjoint for one directory and the guard must not read that as a foreign session. + const base = mkdtempSync(join(tmpdir(), 'orca-cwd-attr-')) + try { + const real = join(base, 'real-workspace') + mkdirSync(join(real, 'src'), { recursive: true }) + const link = join(base, 'linked-workspace') + try { + symlinkSync(real, link, process.platform === 'win32' ? 'junction' : 'dir') + } catch { + return // Restricted hosts that cannot create links have nothing to verify here. + } + const physicalSessionCwd = join(realpathSync(real), 'src') + expect(hookCwdContradictsWorktree(`${REPO}::${link}`, physicalSessionCwd)).toBe(true) + expect( + hookCwdContradictsWorktreeAfterLocalResolve(`${REPO}::${link}`, physicalSessionCwd) + ).toBe(false) + } finally { + rmSync(base, { recursive: true, force: true }) + } + }) + + it('still refuses genuinely disjoint real directories', () => { + const base = mkdtempSync(join(tmpdir(), 'orca-cwd-attr-')) + try { + const workspace = join(base, 'workspace-a') + const session = join(base, 'session-b') + mkdirSync(workspace) + mkdirSync(session) + expect(hookCwdContradictsWorktreeAfterLocalResolve(`${REPO}::${workspace}`, session)).toBe( + true + ) + } finally { + rmSync(base, { recursive: true, force: true }) + } + }) + + it('keeps the raw verdict for paths that do not exist on this host', () => { + // Why: remote (SSH/WSL) paths fail existsSync here, so the string verdict must stand — + // the relay that owns those paths runs its own resolved check before forwarding. + expect( + hookCwdContradictsWorktreeAfterLocalResolve( + `${REPO}::/nonexistent-orca-guard-test/worktree-a`, + '/nonexistent-orca-guard-test/session-b' + ) + ).toBe(true) + expect( + hookCwdContradictsWorktreeAfterLocalResolve(WORKTREE, '/Users/dev/projects/api/src') + ).toBe(false) + }) +}) diff --git a/src/shared/agent-hook-cwd-attribution.ts b/src/shared/agent-hook-cwd-attribution.ts index e5526c2fc46b..ec6ce1834b9f 100644 --- a/src/shared/agent-hook-cwd-attribution.ts +++ b/src/shared/agent-hook-cwd-attribution.ts @@ -1,10 +1,11 @@ +import { existsSync, realpathSync } from 'node:fs' import { isPathInsideOrEqual, isRuntimePathAbsolute, isWindowsAbsolutePathLike, resolveRuntimePath } from './cross-platform-path' -import { splitWorktreeIdForFilesystem } from './worktree-id' +import { splitWorktreeIdForFilesystem, WORKTREE_ID_SEPARATOR } from './worktree-id' const UNC_NOTATION = /^(?:\/\/|\\\\)/ @@ -53,3 +54,39 @@ export function hookCwdContradictsWorktree( // under the session root (folder workspaces); only fully disjoint paths are proof. return !isPathInsideOrEqual(workspace, session) && !isPathInsideOrEqual(session, workspace) } + +/** Paths this host does not have stay raw, so foreign-host paths keep the string verdict. */ +function realpathIfExists(path: string): string { + try { + if (existsSync(path)) { + return realpathSync.native(path) + } + } catch { + // Fall through to the raw spelling. + } + return path +} + +/** + * `hookCwdContradictsWorktree`, re-judged on locally resolved paths before trusting a drop. + * + * Why: one directory can spell two fully disjoint strings — macOS /tmp is /private/tmp, + * symlinked project roots, subst drives — and agents report physical getcwd while Orca + * stores the path as picked, so a raw contradiction is only proof once symlink aliasing + * is ruled out. Call this only where the current process runs on the host that owns both + * paths (Orca's local HTTP ingest, the relay's own hook server). + */ +export function hookCwdContradictsWorktreeAfterLocalResolve( + worktreeId: string | undefined, + cwd: string | undefined +): boolean { + if (!hookCwdContradictsWorktree(worktreeId, cwd)) { + return false + } + const parsed = worktreeId ? splitWorktreeIdForFilesystem(worktreeId) : undefined + if (!parsed || !cwd) { + return true + } + const resolvedWorktreeId = `${parsed.repoId}${WORKTREE_ID_SEPARATOR}${realpathIfExists(parsed.worktreePath)}` + return hookCwdContradictsWorktree(resolvedWorktreeId, realpathIfExists(cwd)) +} diff --git a/src/shared/agent-hook-listener.ts b/src/shared/agent-hook-listener.ts index 6f051fe1ebd3..8405d02533fa 100644 --- a/src/shared/agent-hook-listener.ts +++ b/src/shared/agent-hook-listener.ts @@ -1158,6 +1158,28 @@ function parseHookBodyPayloadRecord(body: unknown): Record | nu : null } +/** Pre-normalization read of the fields the cwd attribution guard needs, so ingest can + * refuse a foreign event before `normalizeHookPayload` seeds per-pane listener state + * (subagent rosters, lead-turn records) that later legitimate events would re-emit. */ +export function readHookBodyCwdAttribution(body: unknown): { + paneKey: string + worktreeId?: string + sourceCwd?: string +} { + if (typeof body !== 'object' || body === null) { + return { paneKey: '' } + } + const record = body as Record + const hookPayload = parseHookBodyPayloadRecord(body) + return { + paneKey: typeof record.paneKey === 'string' ? record.paneKey.trim() : '', + worktreeId: readStringField(record, 'worktreeId'), + sourceCwd: hookPayload + ? readBoundedString(hookPayload, HOOK_CWD_KEYS, HOOK_CWD_MAX_LENGTH)?.trim() || undefined + : undefined + } +} + function readBoundedString( record: Record, keys: readonly string[], From 79dc97afd29493af6cb873acb6ef1e9b523dd3b4 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 30 Jul 2026 09:00:56 -0700 Subject: [PATCH 10/12] fix(agent-hooks): strip alias-cleared cwd at the relay egress choke point MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The assistant-message retry and codex subagent poll re-normalize the raw body and re-attached the contradicting cwd, so main's raw re-guard dropped enrichment updates and poisoned the replay cache for symlink-aliased remote workspaces; move the strip into applyEvent, which every cache/forward leg funnels through. - Keep, not drop, when a local path exists but cannot be resolved, and fold the macOS data-volume firmlink prefix — both only rescue rows. - Bound warn-set keys and logged values on both hook servers. - Pin the retry-leg strip, the relay guard-before-normalize ordering, and cwd-side symlink resolution with discriminating tests. --- src/main/agent-hooks/server.ts | 13 ++- src/relay/agent-hook-server.test.ts | 99 +++++++++++++++++++ src/relay/agent-hook-server.ts | 33 ++++--- src/shared/agent-hook-cwd-attribution.test.ts | 51 +++++++++- src/shared/agent-hook-cwd-attribution.ts | 30 +++++- 5 files changed, 203 insertions(+), 23 deletions(-) diff --git a/src/main/agent-hooks/server.ts b/src/main/agent-hooks/server.ts index ef0663a70afd..776aa0e06dc0 100644 --- a/src/main/agent-hooks/server.ts +++ b/src/main/agent-hooks/server.ts @@ -950,14 +950,21 @@ export class AgentHookServer { this.reportedForeignCwdTelemetry = true track('agent_hook_unattributed', { reason: 'cwd_worktree_mismatch' }) } + // Why: the early guard runs pre-validation, so slice before retaining/logging — a + // token-holding client could otherwise park 1MB strings in the set and in the log. + const boundedPaneKey = event.paneKey.slice(0, MAX_PANE_KEY_LEN) if ( - !this.warnedForeignCwdPaneKeys.has(event.paneKey) && + !this.warnedForeignCwdPaneKeys.has(boundedPaneKey) && this.warnedForeignCwdPaneKeys.size < FOREIGN_CWD_WARN_PANE_CAP ) { - this.warnedForeignCwdPaneKeys.add(event.paneKey) + this.warnedForeignCwdPaneKeys.add(boundedPaneKey) console.warn( '[agent-hooks] dropping status: reported worktree does not own the reporting session', - { paneKey: event.paneKey, worktreeId: event.worktreeId, sourceCwd: event.sourceCwd } + { + paneKey: boundedPaneKey, + worktreeId: event.worktreeId?.slice(0, HOOK_CWD_MAX_LENGTH), + sourceCwd: event.sourceCwd + } ) } return true diff --git a/src/relay/agent-hook-server.test.ts b/src/relay/agent-hook-server.test.ts index f2f136b1b678..3eaeb621ebef 100644 --- a/src/relay/agent-hook-server.test.ts +++ b/src/relay/agent-hook-server.test.ts @@ -113,6 +113,105 @@ describe('RelayAgentHookServer', () => { } }) + it('refuses the foreign event before it can seed relay listener state', async () => { + // Why: normalization mutates per-pane listener state; a refusal placed after it would + // let a foreign SubagentStart plant roster rows that the pane's own next event re-emits. + const forward = vi.fn<(envelope: AgentHookRelayEnvelope) => void>() + const server = new RelayAgentHookServer({ endpointDir: dir, forward }) + await server.start() + try { + const { port, token } = server.getCoordinates() + const postHook = (payload: Record): Promise => + fetch(`http://127.0.0.1:${port}/hook/claude`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-Orca-Agent-Hook-Token': token + }, + body: JSON.stringify({ + paneKey: PANE_KEY, + tabId: 'tab-1', + worktreeId: 'repo-1::/nonexistent-orca-relay/worktree-a', + env: 'remote', + version: '1', + payload + }) + }) + await postHook({ + hook_event_name: 'SubagentStart', + agent_id: 'sa-foreign', + cwd: '/nonexistent-orca-relay/session-b' + }) + expect(forward).not.toHaveBeenCalled() + await postHook({ + hook_event_name: 'UserPromptSubmit', + prompt: 'own session', + cwd: '/nonexistent-orca-relay/worktree-a' + }) + expect(forward).toHaveBeenCalledTimes(1) + expect(forward.mock.calls[0][0].payload.prompt).toBe('own session') + expect(forward.mock.calls[0][0].payload.subagents ?? []).toEqual([]) + } finally { + server.stop() + } + }) + + it('keeps the alias strip on the assistant-message retry leg', async () => { + // Why: the retry re-normalizes the raw body, which re-attaches the contradicting cwd; + // the strip must hold at the cache/forward choke point or Orca's raw re-guard drops the + // enriched row (and the poisoned replay cache drops it again after reconnect). + const real = join(dir, 'real-workspace') + mkdirSync(real, { recursive: true }) + const link = join(dir, 'linked-workspace') + try { + symlinkSync(real, link, process.platform === 'win32' ? 'junction' : 'dir') + } catch { + return // Restricted hosts that cannot create links have nothing to verify here. + } + const forward = vi.fn<(envelope: AgentHookRelayEnvelope) => void>() + const server = new RelayAgentHookServer({ endpointDir: dir, forward }) + const transcriptPath = join(dir, 'events.jsonl') + writeFileSync(transcriptPath, '') + await server.start() + try { + const { port, token } = server.getCoordinates() + const res = await fetch(`http://127.0.0.1:${port}/hook/copilot`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-Orca-Agent-Hook-Token': token + }, + body: JSON.stringify({ + paneKey: PANE_KEY, + tabId: 'tab-1', + worktreeId: `repo-1::${link}`, + env: 'remote', + version: '1', + payload: { hook_event_name: 'Stop', transcriptPath, cwd: realpathSync(real) } + }) + }) + expect(res.status).toBe(204) + expect(forward.mock.calls[0]?.[0].sourceCwd).toBeUndefined() + + // Let the first 50ms retry miss, then land the transcript for a later attempt. + await new Promise((resolve) => setTimeout(resolve, 70)) + writeFileSync( + transcriptPath, + `${JSON.stringify({ + type: 'assistant.message', + data: { content: 'Retry leg completed.' } + })}\n` + ) + await new Promise((resolve) => setTimeout(resolve, 120)) + + const last = forward.mock.calls.at(-1)?.[0] + expect(last?.payload.lastAssistantMessage).toBe('Retry leg completed.') + expect(last?.sourceCwd).toBeUndefined() + } finally { + server.stop() + } + }) + it('strips sourceCwd when the contradiction is only symlink aliasing of the worktree', async () => { // Why: raw disjoint but resolved nested means the same directory spelled two ways. // Orca's re-guard cannot resolve this host's paths, so forwarding the cwd would make diff --git a/src/relay/agent-hook-server.ts b/src/relay/agent-hook-server.ts index 72fb3afdb696..5152d53ca62e 100644 --- a/src/relay/agent-hook-server.ts +++ b/src/relay/agent-hook-server.ts @@ -20,7 +20,9 @@ import { getEndpointFileName, hasCodexTranscriptSubagents, hasPendingAgentResultText, + HOOK_CWD_MAX_LENGTH, HOOK_REQUEST_SLOWLORIS_MS, + MAX_PANE_KEY_LEN, normalizeHookPayload, preparePendingGrokResultDiscovery, readHookBodyCwdAttribution, @@ -215,15 +217,18 @@ export class RelayAgentHookServer { } private warnForeignCwdOnce(paneKey: string, worktreeId?: string, sourceCwd?: string): void { + // Why: these run pre-validation, so slice before retaining/logging — a token-holding + // client could otherwise park 1MB strings in the set and emit multi-MB log lines. + const boundedPaneKey = paneKey.slice(0, MAX_PANE_KEY_LEN) if ( - this.warnedForeignCwdPaneKeys.has(paneKey) || + this.warnedForeignCwdPaneKeys.has(boundedPaneKey) || this.warnedForeignCwdPaneKeys.size >= FOREIGN_CWD_WARN_PANE_CAP ) { return } - this.warnedForeignCwdPaneKeys.add(paneKey) + this.warnedForeignCwdPaneKeys.add(boundedPaneKey) process.stderr.write( - `[relay-hook-server] dropping status: reported worktree does not own the reporting session (paneKey=${paneKey} worktreeId=${worktreeId ?? ''} sourceCwd=${sourceCwd ?? ''})\n` + `[relay-hook-server] dropping status: reported worktree does not own the reporting session (paneKey=${boundedPaneKey} worktreeId=${(worktreeId ?? '').slice(0, HOOK_CWD_MAX_LENGTH)} sourceCwd=${sourceCwd ?? ''})\n` ) } @@ -302,12 +307,7 @@ export class RelayAgentHookServer { // paths — before normalization, so a foreign daemon-hosted session cannot seed // per-pane listener state (subagent rosters, lead-turn records) or the replay cache. const attribution = readHookBodyCwdAttribution(body) - const rawCwdContradiction = hookCwdContradictsWorktree( - attribution.worktreeId, - attribution.sourceCwd - ) if ( - rawCwdContradiction && hookCwdContradictsWorktreeAfterLocalResolve(attribution.worktreeId, attribution.sourceCwd) ) { this.warnForeignCwdOnce(attribution.paneKey, attribution.worktreeId, attribution.sourceCwd) @@ -317,12 +317,6 @@ export class RelayAgentHookServer { } const event = normalizeHookPayload(this.state, source, body, this.env) if (event) { - if (rawCwdContradiction) { - // Why: this host proved the contradiction is symlink aliasing (raw disjoint, resolved - // nested). Orca's re-guard cannot resolve remote paths, so forwarding the cwd would - // make it re-drop a proven-legitimate row. - event.sourceCwd = undefined - } // TODO: once normalizeHookPayload returns validated env/version, drop bodyEnv/bodyVersion and source them from the listener result. const env = this.bodyEnv(body) const version = this.bodyVersion(body) @@ -379,6 +373,17 @@ export class RelayAgentHookServer { env?: string, version?: string ): void { + if ( + event.sourceCwd !== undefined && + hookCwdContradictsWorktree(event.worktreeId, event.sourceCwd) + ) { + // Why: every leg that caches/forwards funnels here — live ingest, the assistant-message + // retry, and the codex subagent poll re-normalize the raw body and re-attach the cwd. + // An event only reaches this point after the pre-normalize refusal, so a surviving raw + // contradiction is proven symlink aliasing; forwarding the cwd would make Orca's raw + // re-guard re-drop the row, and caching it would poison replay after reconnect. + event.sourceCwd = undefined + } if (event.payload.state !== 'done' || event.payload.lastAssistantMessage) { this.clearAssistantMessageRetry(event.paneKey) } diff --git a/src/shared/agent-hook-cwd-attribution.test.ts b/src/shared/agent-hook-cwd-attribution.test.ts index fc17ede57d04..4017e83a8f21 100644 --- a/src/shared/agent-hook-cwd-attribution.test.ts +++ b/src/shared/agent-hook-cwd-attribution.test.ts @@ -1,4 +1,4 @@ -import { mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync } from 'node:fs' +import { existsSync, mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { describe, expect, it } from 'vitest' @@ -143,6 +143,55 @@ describe('hookCwdContradictsWorktreeAfterLocalResolve', () => { } }) + it('clears the mirror alias where the session cwd is the symlinked spelling', () => { + // Why: the alias can sit on either side — a logical $PWD-style cwd against a physically + // stored worktree needs the cwd side resolved, not just the worktree side. + const base = mkdtempSync(join(tmpdir(), 'orca-cwd-attr-')) + try { + const real = join(base, 'real-workspace') + mkdirSync(join(real, 'src'), { recursive: true }) + const link = join(base, 'linked-workspace') + try { + symlinkSync(real, link, process.platform === 'win32' ? 'junction' : 'dir') + } catch { + return // Restricted hosts that cannot create links have nothing to verify here. + } + const physicalWorktree = realpathSync(real) + const linkSpelledCwd = join(link, 'src') + expect(hookCwdContradictsWorktree(`${REPO}::${physicalWorktree}`, linkSpelledCwd)).toBe(true) + expect( + hookCwdContradictsWorktreeAfterLocalResolve(`${REPO}::${physicalWorktree}`, linkSpelledCwd) + ).toBe(false) + } finally { + rmSync(base, { recursive: true, force: true }) + } + }) + + it('treats the macOS data-volume firmlink spelling as the same directory', () => { + // Why: firmlinks are not symlinks — realpath keeps /System/Volumes/Data/… even though it + // names the same directory, so resolution alone cannot rescue a firmlink-spelled workspace. + const base = mkdtempSync(join(tmpdir(), 'orca-cwd-attr-')) + try { + const physical = realpathSync(base) + const firmlinkSpelled = `/System/Volumes/Data${physical}` + if (process.platform !== 'darwin' || !existsSync(firmlinkSpelled)) { + return // The alias only exists on macOS data-volume layouts. + } + mkdirSync(join(physical, 'ws', 'src'), { recursive: true }) + expect( + hookCwdContradictsWorktree(`${REPO}::${firmlinkSpelled}/ws`, join(physical, 'ws', 'src')) + ).toBe(true) + expect( + hookCwdContradictsWorktreeAfterLocalResolve( + `${REPO}::${firmlinkSpelled}/ws`, + join(physical, 'ws', 'src') + ) + ).toBe(false) + } finally { + rmSync(base, { recursive: true, force: true }) + } + }) + it('still refuses genuinely disjoint real directories', () => { const base = mkdtempSync(join(tmpdir(), 'orca-cwd-attr-')) try { diff --git a/src/shared/agent-hook-cwd-attribution.ts b/src/shared/agent-hook-cwd-attribution.ts index ec6ce1834b9f..53f8e1414645 100644 --- a/src/shared/agent-hook-cwd-attribution.ts +++ b/src/shared/agent-hook-cwd-attribution.ts @@ -55,18 +55,31 @@ export function hookCwdContradictsWorktree( return !isPathInsideOrEqual(workspace, session) && !isPathInsideOrEqual(session, workspace) } -/** Paths this host does not have stay raw, so foreign-host paths keep the string verdict. */ -function realpathIfExists(path: string): string { +/** Paths this host does not have stay raw, so foreign-host paths keep the string verdict. + * `null` means the path exists here but cannot be resolved (EPERM-style mounts, deleted + * mid-check) — with the alias question unanswerable on a local path, the caller keeps. */ +function realpathIfExists(path: string): string | null { try { if (existsSync(path)) { return realpathSync.native(path) } } catch { - // Fall through to the raw spelling. + return null } return path } +const MACOS_DATA_VOLUME_PREFIX = '/System/Volumes/Data' + +/** Firmlinks are not symlinks — realpath keeps /System/Volumes/Data/Users/… even though it + * names /Users/…. Folding the data-volume prefix runs only when the raw verdict already + * said drop, so it can only rescue rows. */ +function foldMacOsDataVolumePrefix(path: string): string { + return process.platform === 'darwin' && path.startsWith(`${MACOS_DATA_VOLUME_PREFIX}/`) + ? path.slice(MACOS_DATA_VOLUME_PREFIX.length) + : path +} + /** * `hookCwdContradictsWorktree`, re-judged on locally resolved paths before trusting a drop. * @@ -87,6 +100,13 @@ export function hookCwdContradictsWorktreeAfterLocalResolve( if (!parsed || !cwd) { return true } - const resolvedWorktreeId = `${parsed.repoId}${WORKTREE_ID_SEPARATOR}${realpathIfExists(parsed.worktreePath)}` - return hookCwdContradictsWorktree(resolvedWorktreeId, realpathIfExists(cwd)) + const resolvedWorktreePath = realpathIfExists(parsed.worktreePath) + const resolvedCwd = realpathIfExists(cwd) + if (resolvedWorktreePath === null || resolvedCwd === null) { + return false + } + return hookCwdContradictsWorktree( + `${parsed.repoId}${WORKTREE_ID_SEPARATOR}${foldMacOsDataVolumePrefix(resolvedWorktreePath)}`, + foldMacOsDataVolumePrefix(resolvedCwd) + ) } From 85058e5b27359332cf8206b6f8012d8877165a2e Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 30 Jul 2026 09:21:29 -0700 Subject: [PATCH 11/12] fix(agent-hooks): keep the row when only one side of a cwd contradiction is stat-able locally A recorded worktree spelling that cannot be stat-ed on its own host (renamed, deleted, whitespace-mangled by the transit trim) cannot prove a foreign session; both-nonexistent still drops so foreign-host verdicts stand. Pins the exists-but-unresolvable keep with deterministic fs stubs. --- ...hook-cwd-attribution-local-resolve.test.ts | 58 +++++++++++++++++++ src/shared/agent-hook-cwd-attribution.ts | 33 ++++++----- 2 files changed, 78 insertions(+), 13 deletions(-) create mode 100644 src/shared/agent-hook-cwd-attribution-local-resolve.test.ts diff --git a/src/shared/agent-hook-cwd-attribution-local-resolve.test.ts b/src/shared/agent-hook-cwd-attribution-local-resolve.test.ts new file mode 100644 index 000000000000..c35dc0f3d291 --- /dev/null +++ b/src/shared/agent-hook-cwd-attribution-local-resolve.test.ts @@ -0,0 +1,58 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { existsSyncMock, realpathNativeMock } = vi.hoisted(() => ({ + existsSyncMock: vi.fn<(path: string) => boolean>(), + realpathNativeMock: vi.fn<(path: string) => string>() +})) + +// Why: the fault semantics (exists-but-unresolvable, one-side-stat-able) cannot be produced +// with real fixtures on stock hosts; deterministic fs stubs pin them instead. +vi.mock('node:fs', () => ({ + existsSync: existsSyncMock, + realpathSync: Object.assign(vi.fn(), { native: realpathNativeMock }) +})) + +import { hookCwdContradictsWorktreeAfterLocalResolve } from './agent-hook-cwd-attribution' + +const WORKTREE = 'repo-1::/data/workspace-a' +const FOREIGN_CWD = '/data/session-b' + +describe('hookCwdContradictsWorktreeAfterLocalResolve local-resolve semantics', () => { + beforeEach(() => { + existsSyncMock.mockReset() + realpathNativeMock.mockReset() + }) + + it('still refuses when both sides resolve to disjoint directories', () => { + existsSyncMock.mockReturnValue(true) + realpathNativeMock.mockImplementation((path) => path) + expect(hookCwdContradictsWorktreeAfterLocalResolve(WORKTREE, FOREIGN_CWD)).toBe(true) + }) + + it('keeps when a local path exists but cannot be resolved', () => { + // Why: EPERM-style mounts or a directory deleted mid-check — the alias question is + // unanswerable for a path this host owns, and unclear keeps. + existsSyncMock.mockReturnValue(true) + realpathNativeMock.mockImplementation(() => { + throw Object.assign(new Error('operation not permitted'), { code: 'EPERM' }) + }) + expect(hookCwdContradictsWorktreeAfterLocalResolve(WORKTREE, FOREIGN_CWD)).toBe(false) + }) + + it('keeps when only one side is stat-able locally', () => { + // Why: a recorded worktree spelling that cannot be stat-ed on the host that owns it + // (renamed, deleted, whitespace-mangled in transit) cannot prove a foreign session. + existsSyncMock.mockImplementation((path) => path === FOREIGN_CWD) + realpathNativeMock.mockImplementation((path) => path) + expect(hookCwdContradictsWorktreeAfterLocalResolve(WORKTREE, FOREIGN_CWD)).toBe(false) + + existsSyncMock.mockImplementation((path) => path === '/data/workspace-a') + expect(hookCwdContradictsWorktreeAfterLocalResolve(WORKTREE, FOREIGN_CWD)).toBe(false) + }) + + it('still refuses when neither side exists locally, preserving the foreign-host verdict', () => { + existsSyncMock.mockReturnValue(false) + expect(hookCwdContradictsWorktreeAfterLocalResolve(WORKTREE, FOREIGN_CWD)).toBe(true) + expect(realpathNativeMock).not.toHaveBeenCalled() + }) +}) diff --git a/src/shared/agent-hook-cwd-attribution.ts b/src/shared/agent-hook-cwd-attribution.ts index 53f8e1414645..314c9321468b 100644 --- a/src/shared/agent-hook-cwd-attribution.ts +++ b/src/shared/agent-hook-cwd-attribution.ts @@ -55,18 +55,18 @@ export function hookCwdContradictsWorktree( return !isPathInsideOrEqual(workspace, session) && !isPathInsideOrEqual(session, workspace) } -/** Paths this host does not have stay raw, so foreign-host paths keep the string verdict. - * `null` means the path exists here but cannot be resolved (EPERM-style mounts, deleted - * mid-check) — with the alias question unanswerable on a local path, the caller keeps. */ -function realpathIfExists(path: string): string | null { +/** `resolved: null` means the path exists here but cannot be resolved (EPERM-style mounts, + * deleted mid-check) — with the alias question unanswerable on a local path, callers keep. + * Nonexistent paths keep their raw spelling so foreign-host paths keep the string verdict. */ +function resolveLocalPath(path: string): { exists: boolean; resolved: string | null } { try { - if (existsSync(path)) { - return realpathSync.native(path) + if (!existsSync(path)) { + return { exists: false, resolved: path } } + return { exists: true, resolved: realpathSync.native(path) } } catch { - return null + return { exists: true, resolved: null } } - return path } const MACOS_DATA_VOLUME_PREFIX = '/System/Volumes/Data' @@ -100,13 +100,20 @@ export function hookCwdContradictsWorktreeAfterLocalResolve( if (!parsed || !cwd) { return true } - const resolvedWorktreePath = realpathIfExists(parsed.worktreePath) - const resolvedCwd = realpathIfExists(cwd) - if (resolvedWorktreePath === null || resolvedCwd === null) { + const worktree = resolveLocalPath(parsed.worktreePath) + const session = resolveLocalPath(cwd) + if (worktree.resolved === null || session.resolved === null) { + return false + } + // Why: exactly one side stat-able means the other side's recorded spelling cannot be + // trusted on the host that owns it (renamed, deleted, or whitespace-mangled in transit) — + // unclear keeps. Both-nonexistent still drops on the raw strings: that is the foreign-host + // shape, where this process is not the judge of existence. + if (worktree.exists !== session.exists) { return false } return hookCwdContradictsWorktree( - `${parsed.repoId}${WORKTREE_ID_SEPARATOR}${foldMacOsDataVolumePrefix(resolvedWorktreePath)}`, - foldMacOsDataVolumePrefix(resolvedCwd) + `${parsed.repoId}${WORKTREE_ID_SEPARATOR}${foldMacOsDataVolumePrefix(worktree.resolved)}`, + foldMacOsDataVolumePrefix(session.resolved) ) } From 4853bb41fb80affd25bef150b87fb0d9073ab4a6 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 30 Jul 2026 09:36:24 -0700 Subject: [PATCH 12/12] fix(agent-hooks): correct the relay strip invariant comment One-side-stat-able and unresolvable keeps also survive the refusal now; the surviving raw contradiction is host-judged keep-worthy, not always symlink aliasing. --- src/relay/agent-hook-server.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/relay/agent-hook-server.ts b/src/relay/agent-hook-server.ts index 5152d53ca62e..256fa8604913 100644 --- a/src/relay/agent-hook-server.ts +++ b/src/relay/agent-hook-server.ts @@ -379,9 +379,10 @@ export class RelayAgentHookServer { ) { // Why: every leg that caches/forwards funnels here — live ingest, the assistant-message // retry, and the codex subagent poll re-normalize the raw body and re-attach the cwd. - // An event only reaches this point after the pre-normalize refusal, so a surviving raw - // contradiction is proven symlink aliasing; forwarding the cwd would make Orca's raw - // re-guard re-drop the row, and caching it would poison replay after reconnect. + // An event only reaches this point after the pre-normalize refusal, so this host already + // judged a surviving raw contradiction keep-worthy (symlink aliasing, or an unresolvable + // or one-side-stat-able spelling); forwarding the cwd would make Orca's raw re-guard + // re-drop the proven-keep row, and caching it would poison replay after reconnect. event.sourceCwd = undefined } if (event.payload.state !== 'done' || event.payload.lastAssistantMessage) {