diff --git a/.agents/notes/implemented/architecture/2026-07-02-fs-per-session-cwd.md b/.agents/notes/implemented/architecture/2026-07-02-fs-per-session-cwd.md index 9d72611d63..0a6d9b85b1 100644 --- a/.agents/notes/implemented/architecture/2026-07-02-fs-per-session-cwd.md +++ b/.agents/notes/implemented/architecture/2026-07-02-fs-per-session-cwd.md @@ -10,13 +10,15 @@ Filesystem resolution used one plugin-load cwd while bash used the session proje A valid absolute cwd can itself have two apparent parents: when it contains `symlink/..`, filesystem lookup follows the symlink before applying `..`, while `path.resolve()` erases both components lexically. Resolving sandbox policy lexically while launching bash from the raw cwd granted the unrelated lexical parent, denied writes in the real workspace, and let filesystem tools resolve relative paths into the wrong directory. +An ordinary symlink cwd exposes the same distinction when the requested relative path contains `..`: a process traverses from the symlink's physical target, while `path.resolve(cwd, path)` traverses from its lexical spelling. Reads would therefore select a different file than bash or a sandboxed mutation for the same model-supplied path. + ## Decision -Thread the caller's session cwd into path resolution, exactly as `dsh-tool-bash` already does for `workdir`. When the cwd contains a parent segment, resolve it to its native filesystem identity before any lexical join; ordinary cwd spellings stay stable for display. Reuse the resolved sandbox-policy root for mutations and sandboxed bash calls so one call has one workspace identity. The **caller** (the tool) supplies the cwd; the provider does not read a session or agent. +Thread the caller's session cwd into path resolution, exactly as `dsh-tool-bash` already does for `workdir`. When either the cwd or the requested path contains a parent segment, resolve the cwd to its native filesystem identity before any lexical join; ordinary cwd spellings stay stable for display when no traversal makes their identity observable. Reuse the resolved sandbox-policy root for mutations and sandboxed bash calls so one call has one workspace identity. The **caller** (the tool) supplies the cwd; the provider does not read a session or agent. - `FileSystem.resolve` accepts `resolve(path: string, opts?: { cwd?: string; signal?: AbortSignal }): Promise`. `opts.cwd` is the base a RELATIVE `path` resolves against; an absolute `path` ignores it; omitting `opts.cwd` uses the backend's own default. `opts.signal` cancels resolution when the backend performs I/O. The options object keeps both caller-owned resolution controls together without positional growth. - `dsh-fs-local.resolve` uses `resolveLocalTarget(opts?.cwd ?? this.config.cwd, path)`. `config.cwd` stays the default for a caller that supplies none (non-ACP / no-session use, and the single-session stdio demo where `process.cwd()` IS the workspace). -- `dsh-tool-fs`'s `read`/`write`/`edit` derive the session cwd through a shared `sessionCwd(exec)` helper (`exec.agent?.session.header.cwd`, mirroring bash's `resolveWorkdir`) and pass it to `resolve`. The helper uses native realpath semantics when a parent segment could cross a symlink while retaining ordinary spellings; a sandboxed mutation reuses the complete policy's `workspaceRoot`; a non-agent / headerless caller yields `undefined`, so the backend applies its default. +- `dsh-tool-fs`'s `read`/`write`/`edit` derive the session cwd through a shared `sessionCwd(exec, requestedPath)` helper (`exec.agent?.session.header.cwd`, mirroring bash's `resolveWorkdir`) and pass it to `resolve`. The helper uses native realpath semantics when a parent segment in either value could cross a symlink while retaining ordinary spellings otherwise; a sandboxed mutation reuses the complete policy's `workspaceRoot`; a non-agent / headerless caller yields `undefined`, so the backend applies its default. ## Alternatives considered @@ -29,7 +31,7 @@ The default lives in ONE place — the provider's `config.cwd`. `sessionCwd` ret ## Consequences - In the ACP demo the fs tools and bash now agree on each session's workspace; an editor can open any project folder and both tool families act on it. -- A session cwd containing `symlink/..` resolves to the same physical workspace for bash launch, relative filesystem paths, and the sandbox grant; the lexical parent receives no grant. +- A session cwd containing `symlink/..`, or an ordinary symlink cwd paired with a parent-traversing relative path, resolves from the same physical workspace for bash, filesystem tools, and the sandbox grant; the lexical parent receives no grant. - No change to `FsTarget` identity: `targetKey` is still the realpath of the resolved absolute path, so observed-state keying and symlink identity are unaffected — a correct per-session cwd produces the same key bash targets. - Backward compatible: every existing `resolve(path)` call (all in tests) keeps working; the new argument is optional. - The single-session stdio demo is unaffected: it supplies no session cwd (its agent's session has no `cwd`), so resolution falls back to `config.cwd = process.cwd()`, which is the workspace. diff --git a/packages/examples/agent-spine-demo/tests/multi-project-sandbox.e2e.ts b/packages/examples/agent-spine-demo/tests/multi-project-sandbox.e2e.ts index a1726943a4..6019f047c2 100644 --- a/packages/examples/agent-spine-demo/tests/multi-project-sandbox.e2e.ts +++ b/packages/examples/agent-spine-demo/tests/multi-project-sandbox.e2e.ts @@ -1,5 +1,5 @@ import { spawnSync } from 'node:child_process' -import { mkdir, mkdtemp, readFile, rm, symlink } from 'node:fs/promises' +import { mkdir, mkdtemp, readFile, rm, symlink, writeFile } from 'node:fs/promises' import { homedir } from 'node:os' import { basename, join } from 'node:path' import { afterEach, beforeEach, describe, expect, it } from 'vitest' @@ -192,4 +192,38 @@ describe('one-context multi-project sandbox', () => { await expectMissing(join(lexicalRoot, 'bash-escaped.txt')) await expectMissing(join(lexicalRoot, 'fs-escaped.txt')) }) + + it.skipIf(!processSandboxUsable)('resolves parent traversal from a symlinked session root consistently', async () => { + const active = ctx as Context + const lexicalRoot = await projectDir('lexical-parent') + const physicalRoot = await projectDir('physical-parent') + const physicalChild = join(physicalRoot, 'child') + await mkdir(physicalChild) + const link = join(lexicalRoot, 'link') + await symlink(physicalChild, link, process.platform === 'win32' ? 'junction' : 'dir') + await writeFile(join(lexicalRoot, 'shared.txt'), 'from-lexical-parent') + await writeFile(join(physicalRoot, 'shared.txt'), 'from-physical-parent') + const handle = await active.agents.create({ + sessionId: SessionId('symlink-root-parent-path-session'), + meta: { cwd: link }, + }) + + const [bashRead, fsRead] = await Promise.all([ + active.tools.execute({ + callId: CallId('bash-symlink-parent-read'), name: 'bash', agent: handle.agent, + arguments: { command: 'cat ../shared.txt', description: 'Read through the physical parent' }, + }), + active.tools.execute({ + callId: CallId('fs-symlink-parent-read'), name: 'read', agent: handle.agent, + arguments: { file_path: '../shared.txt' }, + }), + ]) + + expect(bashRead.isError).toBe(false) + expect(fsRead.isError).toBe(false) + expect(resultText(bashRead)).toContain('from-physical-parent') + expect(resultText(fsRead)).toContain('from-physical-parent') + expect(resultText(bashRead)).not.toContain('from-lexical-parent') + expect(resultText(fsRead)).not.toContain('from-lexical-parent') + }) }) diff --git a/packages/fs/tool-fs/src/edit.ts b/packages/fs/tool-fs/src/edit.ts index ec7c44902b..d3bb9a2803 100644 --- a/packages/fs/tool-fs/src/edit.ts +++ b/packages/fs/tool-fs/src/edit.ts @@ -95,7 +95,7 @@ export function applyEditTool(ctx: Context, sandbox: FsSandboxSurface): void { // Resolve the per-call sandbox policy (approved mode > session override // > backend default, plus the session cwd root) BEFORE anything executes. const sandboxPolicy = await sandbox.resolvePolicy('edit', args, exec) - const target = await ctx.fs.resolve(input.filePath, sessionResolveOptions(exec, sandboxPolicy?.workspaceRoot)) + const target = await ctx.fs.resolve(input.filePath, sessionResolveOptions(exec, input.filePath, sandboxPolicy?.workspaceRoot)) // Single-slot decision: the policy plugin returns { version: vObserved } or // throws FS_NOT_OBSERVED; the bare default is undefined (unconditional edit). // No stat — the bare default never manufactures a version basis. diff --git a/packages/fs/tool-fs/src/read.ts b/packages/fs/tool-fs/src/read.ts index a19b073514..cb1409987d 100644 --- a/packages/fs/tool-fs/src/read.ts +++ b/packages/fs/tool-fs/src/read.ts @@ -88,7 +88,7 @@ export function applyReadTool(ctx: Context, caps: ReadToolCaps): void { isConcurrencySafe: () => true, async execute(args, exec): Promise { const input = parseReadArgs(args, caps.limit) - const target = await ctx.fs.resolve(input.filePath, sessionResolveOptions(exec)) + const target = await ctx.fs.resolve(input.filePath, sessionResolveOptions(exec, input.filePath)) // One stat: type check + size routing + the version recorded as observed. // A concurrent write can only make a later guarded mutation fail stale and require reread. diff --git a/packages/fs/tool-fs/src/session-cwd.ts b/packages/fs/tool-fs/src/session-cwd.ts index b38f4109b5..aa64193a13 100644 --- a/packages/fs/tool-fs/src/session-cwd.ts +++ b/packages/fs/tool-fs/src/session-cwd.ts @@ -16,22 +16,29 @@ const PARENT_PATH_SEGMENT = /(?:^|[\\/])\.\.(?:[\\/]|$)/ /** * The session workspace cwd for this call, or `undefined` when none applies. * @param exec - the tool-execution context; only its optional `agent` is read. + * @param requestedPath - the path the provider will resolve; parent traversal + * makes a symlinked cwd's filesystem identity observable. * @returns the calling agent's session cwd, or undefined for a non-agent caller (the backend then applies its own default). */ -export function sessionCwd(exec: ToolExecution): string | undefined { +export function sessionCwd(exec: ToolExecution, requestedPath: string): string | undefined { const cwd = exec.agent?.session.header.cwd - if (cwd === undefined || !PARENT_PATH_SEGMENT.test(cwd)) return cwd + if (cwd === undefined || (!PARENT_PATH_SEGMENT.test(cwd) && !PARENT_PATH_SEGMENT.test(requestedPath))) return cwd return canonicalPath(cwd) } /** * Resolution options shared by all model-facing filesystem tools. * @param exec - the tool-execution context supplying session cwd and cancellation. + * @param requestedPath - the path the provider will resolve. * @param policyWorkspaceRoot - resolved per-call root, when a mutation carries sandbox policy. * @returns provider resolution options for the current tool call. */ -export function sessionResolveOptions(exec: ToolExecution, policyWorkspaceRoot?: string): { cwd?: string; signal?: AbortSignal } { - const cwd = policyWorkspaceRoot ?? sessionCwd(exec) +export function sessionResolveOptions( + exec: ToolExecution, + requestedPath: string, + policyWorkspaceRoot?: string, +): { cwd?: string; signal?: AbortSignal } { + const cwd = policyWorkspaceRoot ?? sessionCwd(exec, requestedPath) return { ...cwd !== undefined ? { cwd } : {}, ...exec.signal !== undefined ? { signal: exec.signal } : {}, diff --git a/packages/fs/tool-fs/src/write.ts b/packages/fs/tool-fs/src/write.ts index 38a01faa83..1e92b66612 100644 --- a/packages/fs/tool-fs/src/write.ts +++ b/packages/fs/tool-fs/src/write.ts @@ -80,7 +80,7 @@ export function applyWriteTool(ctx: Context, sandbox: FsSandboxSurface): void { // > backend default, plus the session cwd root) BEFORE anything executes; // an escalating call throws its distinct text on any non-grant. const sandboxPolicy = await sandbox.resolvePolicy('write', args, exec) - const target = await ctx.fs.resolve(input.filePath, sessionResolveOptions(exec, sandboxPolicy?.workspaceRoot)) + const target = await ctx.fs.resolve(input.filePath, sessionResolveOptions(exec, input.filePath, sandboxPolicy?.workspaceRoot)) // Single-slot decision: the policy plugin produces createIfAbsent/ // replaceIfVersion; the bare default is undefined (unconditional). No stat. const intent = await ctx.waterfall('fs/write-intent', target, exec, () => undefined) diff --git a/packages/fs/tool-fs/tests/tools.spec.ts b/packages/fs/tool-fs/tests/tools.spec.ts index ecf33e4d6d..166ea0ab0e 100644 --- a/packages/fs/tool-fs/tests/tools.spec.ts +++ b/packages/fs/tool-fs/tests/tools.spec.ts @@ -5,8 +5,9 @@ import { describe, expect, it, vi } from 'vitest' import { Context } from 'cordis' -import { realpathSync } from 'node:fs' -import { sep } from 'node:path' +import { mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join, sep } from 'node:path' import { CallId } from '@deepseek-ai/dsh-llm' import SystemPrompt, { renderPrompt } from '@deepseek-ai/dsh-system-prompt' import ToolRegistry from '@deepseek-ai/dsh-tools' @@ -113,12 +114,24 @@ describe('session cwd resolution', () => { ? {} : { agent: { session: { header: { cwd } } } } - it('retains ordinary spelling but resolves parent segments before lexical use', () => { + it('retains ordinary spelling but resolves the cwd before parent traversal', () => { const cwd = process.cwd() const throughParent = `${cwd}${sep}..` - expect(sessionCwd(execution() as never)).toBeUndefined() - expect(sessionCwd(execution(cwd) as never)).toBe(cwd) - expect(sessionCwd(execution(throughParent) as never)).toBe(realpathSync.native(throughParent)) + expect(sessionCwd(execution() as never, 'file.txt')).toBeUndefined() + expect(sessionCwd(execution(cwd) as never, 'file.txt')).toBe(cwd) + expect(sessionCwd(execution(throughParent) as never, 'file.txt')).toBe(realpathSync.native(throughParent)) + + const root = mkdtempSync(join(tmpdir(), 'dsh-tool-fs-session-cwd-')) + const physical = join(root, 'physical') + const link = join(root, 'link') + try { + mkdirSync(physical) + symlinkSync(physical, link, process.platform === 'win32' ? 'junction' : 'dir') + expect(sessionCwd(execution(link) as never, 'child.txt')).toBe(link) + expect(sessionCwd(execution(link) as never, `..${sep}parent.txt`)).toBe(realpathSync.native(link)) + } finally { + rmSync(root, { recursive: true, force: true }) + } }) })