diff --git a/packages/acp-bridge/src/bridge.ts b/packages/acp-bridge/src/bridge.ts index 3a461d201e4..8010bd79370 100644 --- a/packages/acp-bridge/src/bridge.ts +++ b/packages/acp-bridge/src/bridge.ts @@ -5564,6 +5564,7 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { { sessionId, path: req.path, + ...(req.allowedRoots ? { allowedRoots: req.allowedRoots } : {}), }, ); const extResult = raw as { @@ -5624,6 +5625,13 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { return { sessionId, ...result }; }, + setSessionWorktree(sessionId, worktree) { + const entry = byId.get(sessionId); + if (entry) { + entry.worktree = worktree; + } + }, + async closeSession(sessionId, context, closeOpts) { return closeSessionImpl(sessionId, context, closeOpts); }, diff --git a/packages/acp-bridge/src/bridgeTypes.ts b/packages/acp-bridge/src/bridgeTypes.ts index 2f0c4343539..e307f29d43b 100644 --- a/packages/acp-bridge/src/bridgeTypes.ts +++ b/packages/acp-bridge/src/bridgeTypes.ts @@ -270,6 +270,14 @@ export interface BridgeForkAgentResult { export interface ChangeSessionCwdRequest { path: string; + /** + * Server-controlled containment roots. When present, the agent-side + * sessionCd handler verifies (after its own realpath) that the + * canonical target is under one of these roots. Only set by the + * daemon's worktree create/restore paths; direct user cd omits this + * field, preserving existing behavior. + */ + allowedRoots?: string[]; } export interface ChangeSessionCwdResult { @@ -723,6 +731,17 @@ export interface AcpSessionBridge { context?: BridgeClientRequestContext, ): Promise; + /** + * Set worktree metadata on an existing session entry. Used when + * restoring a worktree session after daemon restart — the sidecar + * file provides the metadata, and this populates the in-memory entry + * so `getSessionSummary` returns it. + */ + setSessionWorktree( + sessionId: string, + worktree: { slug: string; path: string; branch: string }, + ): void; + /** * Forward a prompt to the agent. Concurrent prompts against the same * session FIFO-serialize through a per-session queue. diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index 75f1953346f..b5ff845979a 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -7571,6 +7571,25 @@ class QwenAgent implements Agent { // Canonicalize path const canonicalPath = await fs.realpath(targetPath); + // Server-controlled containment check (worktree create/restore). + // Must run BEFORE the no-op check: a no-op cd to a directory + // outside the allowed roots must still be rejected. + const allowedRoots = params['allowedRoots']; + if (Array.isArray(allowedRoots) && allowedRoots.length > 0) { + const contained = allowedRoots.some((root: unknown) => { + if (typeof root !== 'string') return false; + const rel = path.relative(root, canonicalPath); + return !rel.startsWith('..') && !path.isAbsolute(rel); + }); + if (!contained) { + throw new RequestError( + -32004, + `Path outside allowed roots: ${canonicalPath}`, + { errorKind: 'containment_violation', path: canonicalPath }, + ); + } + } + // Noop check const previousCwd = config.getTargetDir(); if (canonicalPath === previousCwd) { diff --git a/packages/cli/src/serve/routes/session.ts b/packages/cli/src/serve/routes/session.ts index 5cfc148f069..e3b4960be4d 100644 --- a/packages/cli/src/serve/routes/session.ts +++ b/packages/cli/src/serve/routes/session.ts @@ -5,6 +5,7 @@ */ import * as crypto from 'node:crypto'; +import * as fs from 'node:fs'; import * as path from 'node:path'; import { APPROVAL_MODES, @@ -23,6 +24,7 @@ import { runWithoutDebugLogSession, writeWorktreeSessionMarker, writeWorktreeSession, + readWorktreeSession, type ApprovalMode, type SessionGroupColor, type SessionGroupPresetColor, @@ -1376,8 +1378,28 @@ export function registerSessionRoutes( // before any subsequent prompt is processed. if (worktreeMeta) { try { + // Compute allowed roots for the sessionCd containment check. + // Narrow to /.qwen/worktrees (not the whole repo) so a + // symlink .qwen/worktrees/task -> /src is rejected. + const createAllowedRoots = [ + path.join(workspaceCwd, '.qwen', 'worktrees'), + ]; + let createRepoTop: string | null = null; + try { + createRepoTop = await new GitWorktreeService( + workspaceCwd, + ).getRepoTopLevel(); + } catch { + // Not a git repo or getRepoTopLevel unavailable. + } + if (createRepoTop && createRepoTop !== workspaceCwd) { + createAllowedRoots.push( + path.join(createRepoTop, '.qwen', 'worktrees'), + ); + } await runtime.bridge.changeSessionCwd(session.sessionId, { path: worktreeMeta.path, + allowedRoots: createAllowedRoots, }); await writeWorktreeSessionMarker( worktreeMeta.path, @@ -1551,6 +1573,93 @@ export function registerSessionRoutes( } return; } + // Restore worktree isolation. Read the sidecar AFTER load/resume + // so we inherit the ACP layer's verdict: #restoreWorktreeOnResume + // clears the sidecar on dead-worktree / containment-failure paths, + // so a post-read naturally skips those cases. On the healthy path + // the sidecar is untouched and we relocate + populate the entry. + // Note: the !res.writable early-return above skips this restore; + // a client that disconnects mid-load leaves the session parked in + // the main workspace (pre-existing shape, low frequency). + if (!session.worktree) { + const sidecar = await readWorktreeSession( + new SessionService(workspaceCwd).getWorktreeSessionPath(sessionId), + ).catch(() => null); + if (sidecar) { + // Defense-in-depth: resolve symlinks on both the target and + // the expected worktrees root, then verify containment. This + // defeats both `..` traversal and symlink escapes (e.g. + // .qwen/worktrees/escape -> /etc). The allowed root is always + // derived from the server (never from the sidecar, which is + // attacker-writable). The canonical realTarget is passed to + // changeSessionCwd to eliminate the TOCTOU window between + // validation and relocation. + // For monorepo subdirectory workspaces, worktrees live under + // the repo top-level, not the workspace cwd. Try workspaceCwd + // first, then fall back to the git repo top-level. + let realTarget: string | undefined; + const candidateRoots = [ + path.join(workspaceCwd, '.qwen', 'worktrees'), + ]; + try { + realTarget = fs.realpathSync(sidecar.worktreePath); + let repoTop: string | null = null; + try { + repoTop = await new GitWorktreeService( + workspaceCwd, + ).getRepoTopLevel(); + } catch { + // Not a git repo or getRepoTopLevel unavailable. + } + if (repoTop && repoTop !== workspaceCwd) { + candidateRoots.push(path.join(repoTop, '.qwen', 'worktrees')); + } + const contained = candidateRoots.some((root) => { + try { + const realRoot = fs.realpathSync(root); + const rel = path.relative(realRoot, realTarget!); + return !rel.startsWith('..') && !path.isAbsolute(rel); + } catch { + return false; + } + }); + if (!contained) { + realTarget = undefined; + } + } catch { + realTarget = undefined; + } + if (!realTarget) { + daemonLog?.warn('worktree sidecar path failed containment', { + sessionId, + path: sidecar.worktreePath, + }); + } else { + const wt = { + slug: sidecar.slug, + path: realTarget, + branch: sidecar.worktreeBranch, + }; + try { + await runtime.bridge.changeSessionCwd(sessionId, { + path: wt.path, + allowedRoots: candidateRoots, + }); + runtime.bridge.setSessionWorktree(sessionId, wt); + session.worktree = wt; + } catch (restoreErr) { + daemonLog?.warn('worktree restore failed on load/resume', { + sessionId, + worktreePath: wt.path, + error: + restoreErr instanceof Error + ? restoreErr.message + : String(restoreErr), + }); + } + } + } + } res.status(200).json(session); } catch (err) { sendBridgeError(res, err, { diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index 819565ded47..c953ad4f33d 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -159,12 +159,32 @@ import { // `mockWt.impl` to control instance behaviour. const mockWt = vi.hoisted(() => ({ impl: undefined as (() => Record) | undefined, + readSidecar: undefined as (() => Promise) | undefined, + realpath: undefined as ((p: string) => string) | undefined, })); +vi.mock('node:fs', async (importOriginal) => { + const original = await importOriginal(); + const wrapped = ((p: fs.PathLike) => + mockWt.realpath + ? mockWt.realpath(String(p)) + : original.realpathSync(p)) as typeof original.realpathSync; + wrapped.native = original.realpathSync.native; + return { + ...original, + realpathSync: wrapped, + }; +}); vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => { const original = await importOriginal(); return { ...original, + readWorktreeSession: (...args: unknown[]) => + mockWt.readSidecar + ? mockWt.readSidecar() + : (original.readWorktreeSession as (...a: unknown[]) => unknown)( + ...args, + ), GitWorktreeService: class MockGitWorktreeService { static validateUserWorktreeSlug = original.GitWorktreeService.validateUserWorktreeSlug; @@ -500,6 +520,15 @@ interface FakeBridgeOpts { promptId: string, ) => { removed: boolean }; spawnImpl?: (req: BridgeSpawnRequest) => Promise; + changeSessionCwdImpl?: ( + sessionId: string, + req: { path: string }, + ) => Promise<{ + sessionId: string; + previousCwd: string; + newCwd: string; + warnings: string[]; + }>; loadImpl?: ( req: BridgeRestoreSessionRequest, ) => Promise; @@ -754,6 +783,10 @@ interface FakeBridge extends AcpSessionBridge { }>; detachCalls: Array<{ sessionId: string; clientId?: string }>; changeSessionCwdCalls: Array<{ sessionId: string; path: string }>; + setSessionWorktreeCalls: Array<{ + sessionId: string; + worktree: { slug: string; path: string; branch: string }; + }>; enqueueMidTurnCalls: Array<{ sessionId: string; message: string; @@ -932,6 +965,10 @@ function fakeBridge(opts: FakeBridgeOpts = {}): FakeBridge { }> = []; const detachCalls: FakeBridge['detachCalls'] = []; const changeSessionCwdCalls: Array<{ sessionId: string; path: string }> = []; + const setSessionWorktreeCalls: Array<{ + sessionId: string; + worktree: { slug: string; path: string; branch: string }; + }> = []; const enqueueMidTurnCalls: FakeBridge['enqueueMidTurnCalls'] = []; const enqueueMidTurnImpl = opts.enqueueMidTurnImpl ?? (() => ({ accepted: true })); @@ -1497,6 +1534,7 @@ function fakeBridge(opts: FakeBridgeOpts = {}): FakeBridge { killCalls, detachCalls, changeSessionCwdCalls, + setSessionWorktreeCalls, enqueueMidTurnCalls, permissionVotes, sessionPermissionVotes, @@ -1976,6 +2014,9 @@ function fakeBridge(opts: FakeBridgeOpts = {}): FakeBridge { }, async changeSessionCwd(sessionId, req) { changeSessionCwdCalls.push({ sessionId, path: req.path }); + if (opts.changeSessionCwdImpl) { + return opts.changeSessionCwdImpl(sessionId, req); + } return { sessionId, previousCwd: '/fake/previous', @@ -1983,6 +2024,9 @@ function fakeBridge(opts: FakeBridgeOpts = {}): FakeBridge { warnings: [], }; }, + setSessionWorktree(sessionId, worktree) { + setSessionWorktreeCalls.push({ sessionId, worktree }); + }, isChannelLive() { return false; }, @@ -8842,6 +8886,141 @@ describe('createServeApp', () => { // CI. The same constraint applies here. The cleanup behavior // is exercised manually via the route handler closure shared // between both routes in `restoreSessionHandler`. + + it('restores worktree isolation on load when sidecar exists', async () => { + const bridge = fakeBridge(); + const app = createServeApp( + { ...baseOpts, workspace: WS_BOUND }, + undefined, + { bridge }, + ); + // Identity realpath so the containment check passes in the test env. + mockWt.realpath = (p) => p; + mockWt.readSidecar = () => + Promise.resolve({ + slug: 'my-task', + worktreePath: `${WS_BOUND}/.qwen/worktrees/my-task`, + worktreeBranch: 'worktree-my-task', + originalCwd: WS_BOUND, + originalBranch: 'main', + originalHeadCommit: 'abc123', + }); + + try { + const res = await request(app) + .post('/session/wt-session/load') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({ cwd: WS_BOUND }); + + expect(res.status).toBe(200); + expect(res.body.worktree).toEqual({ + slug: 'my-task', + path: `${WS_BOUND}/.qwen/worktrees/my-task`, + branch: 'worktree-my-task', + }); + expect(bridge.changeSessionCwdCalls).toHaveLength(1); + expect(bridge.changeSessionCwdCalls[0].path).toBe( + `${WS_BOUND}/.qwen/worktrees/my-task`, + ); + expect(bridge.setSessionWorktreeCalls).toHaveLength(1); + } finally { + mockWt.readSidecar = undefined; + mockWt.realpath = undefined; + } + }); + + it('skips worktree restore when no sidecar exists', async () => { + const bridge = fakeBridge(); + const app = createServeApp( + { ...baseOpts, workspace: WS_BOUND }, + undefined, + { bridge }, + ); + mockWt.readSidecar = () => Promise.resolve(null); + + try { + const res = await request(app) + .post('/session/plain-session/load') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({ cwd: WS_BOUND }); + + expect(res.status).toBe(200); + expect(res.body.worktree).toBeUndefined(); + expect(bridge.changeSessionCwdCalls).toHaveLength(0); + expect(bridge.setSessionWorktreeCalls).toHaveLength(0); + } finally { + mockWt.readSidecar = undefined; + } + }); + + it('returns 200 without worktree when changeSessionCwd fails', async () => { + const bridge = fakeBridge({ + changeSessionCwdImpl: async () => { + throw new Error('cd failed'); + }, + }); + const app = createServeApp( + { ...baseOpts, workspace: WS_BOUND }, + undefined, + { bridge }, + ); + mockWt.realpath = (p) => p; + mockWt.readSidecar = () => + Promise.resolve({ + slug: 'dead-task', + worktreePath: `${WS_BOUND}/.qwen/worktrees/dead-task`, + worktreeBranch: 'worktree-dead-task', + originalCwd: WS_BOUND, + originalBranch: 'main', + originalHeadCommit: 'abc123', + }); + + try { + const res = await request(app) + .post('/session/dead-session/load') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({ cwd: WS_BOUND }); + + expect(res.status).toBe(200); + expect(res.body.worktree).toBeUndefined(); + expect(bridge.setSessionWorktreeCalls).toHaveLength(0); + } finally { + mockWt.readSidecar = undefined; + mockWt.realpath = undefined; + } + }); + + it('skips restore when sidecar path fails containment check', async () => { + const bridge = fakeBridge(); + const app = createServeApp( + { ...baseOpts, workspace: WS_BOUND }, + undefined, + { bridge }, + ); + mockWt.readSidecar = () => + Promise.resolve({ + slug: 'escape', + worktreePath: '/etc/passwd', + worktreeBranch: 'worktree-escape', + originalCwd: WS_BOUND, + originalBranch: 'main', + originalHeadCommit: 'abc123', + }); + + try { + const res = await request(app) + .post('/session/escape-session/load') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({ cwd: WS_BOUND }); + + expect(res.status).toBe(200); + expect(res.body.worktree).toBeUndefined(); + expect(bridge.changeSessionCwdCalls).toHaveLength(0); + expect(bridge.setSessionWorktreeCalls).toHaveLength(0); + } finally { + mockWt.readSidecar = undefined; + } + }); }); describe('POST /session/:id/prompt', () => { diff --git a/packages/core/src/services/sessionService.test.ts b/packages/core/src/services/sessionService.test.ts index 2ad5ba8eefc..6768e081e2d 100644 --- a/packages/core/src/services/sessionService.test.ts +++ b/packages/core/src/services/sessionService.test.ts @@ -3896,6 +3896,56 @@ describe('SessionService', () => { }); }); + describe('listSessions worktree membership', () => { + const worktreeSessionId = '7ca8c920-e29b-41d4-a716-446655440001'; + + it('includes a session whose transcript cwd is a worktree under this project', async () => { + (path as unknown as Record)['sep'] = '/'; + readdirSyncSpy.mockReturnValue([ + `${worktreeSessionId}.jsonl`, + ] as unknown as Array>); + vi.mocked(jsonl.readLines).mockResolvedValue([ + { + ...recordA1, + sessionId: worktreeSessionId, + cwd: '/test/project/root/.qwen/worktrees/my-task', + }, + ]); + // The full worktree cwd hashes differently from the repo root, + // so the first getProjectHash(recordCwd) check fails and the + // marker-based inference branch is exercised. + vi.mocked(getProjectHash).mockImplementation((p: string) => + p === '/test/project/root' ? 'test-project-hash' : 'worktree-hash', + ); + + const result = await sessionService.listSessions(); + + expect(result.items).toHaveLength(1); + expect(result.items[0].sessionId).toBe(worktreeSessionId); + }); + + it('excludes a session whose worktree belongs to a different project', async () => { + (path as unknown as Record)['sep'] = '/'; + readdirSyncSpy.mockReturnValue([ + `${worktreeSessionId}.jsonl`, + ] as unknown as Array>); + vi.mocked(jsonl.readLines).mockResolvedValue([ + { + ...recordA1, + sessionId: worktreeSessionId, + cwd: '/other/repo/.qwen/worktrees/my-task', + }, + ]); + vi.mocked(getProjectHash).mockImplementation((p: string) => + p.startsWith('/other/repo') ? 'other-hash' : 'test-project-hash', + ); + + const result = await sessionService.listSessions(); + + expect(result.items).toHaveLength(0); + }); + }); + describe('listSessions parentSessionId round-trip', () => { // Uses real disk like findSessionTitlesByPrefix — readParentSessionIdFromFile // does a synchronous tail/head scan of the file, so the mocked diff --git a/packages/core/src/services/sessionService.ts b/packages/core/src/services/sessionService.ts index 90e947d6783..dbeca44d18f 100644 --- a/packages/core/src/services/sessionService.ts +++ b/packages/core/src/services/sessionService.ts @@ -381,6 +381,24 @@ export class SessionService { return true; } + // Worktree sessions record cwd as the worktree path + // (/.qwen/worktrees/), which has a different project + // hash. Infer the repo root from the path and check its hash. This + // is durable — it doesn't depend on the sidecar file, which is + // transient and cleared when the worktree is removed. Pure string + // ops, so check before the file-read runtime status below. + // Use lastIndexOf to handle nested worktrees: for + // /repo/.qwen/worktrees/parent/.qwen/worktrees/child, the innermost + // marker gives repoRoot = /repo/.qwen/worktrees/parent (the workspace). + const worktreesMarker = `${path.sep}.qwen${path.sep}worktrees${path.sep}`; + const markerIdx = recordCwd.lastIndexOf(worktreesMarker); + if (markerIdx > 0) { + const repoRoot = recordCwd.substring(0, markerIdx); + if (getProjectHash(repoRoot) === this.projectHash) { + return true; + } + } + const status = await readRuntimeStatus( this.storage.getRuntimeStatusPath(sessionId), );