diff --git a/src/lib/chat-project-access.test.ts b/src/lib/chat-project-access.test.ts index 82e8d1111..bd5106c6e 100644 --- a/src/lib/chat-project-access.test.ts +++ b/src/lib/chat-project-access.test.ts @@ -1,5 +1,8 @@ // @ts-nocheck import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; import { chatProjectAccessId, taskWorktreeProjectAccessId } from "./chat-project-access.ts"; @@ -211,3 +214,64 @@ assert.equal( ); console.log("chat-project-access tests passed"); + +// SECURITY: the requested root is client-supplied, so a `.worktrees/` +// symlink pointing outside the project would otherwise borrow the parent +// project's grant while the harness ran elsewhere. The realpathed cwd must +// land under the same prefix. +assert.equal( + chatProjectAccessId({ + projects, + requestedProjectRoot: "/Users/me/dev/cave/.worktrees/evil", + resolvedCwd: "/Users/me/victim-ungranted-project", + }), + "unregistered:/Users/me/dev/cave/.worktrees/evil", + "a symlinked worktree request whose real cwd escapes the parent project fails closed", +); + +// REGRESSION: `resolvedCwd` arrives realpath-resolved from +// resolveLocalRuntimeCwd, so a lexically-resolved prefix built from a +// symlink-registered project root could never match it, and every legitimate +// worktree chat under that project fail-closed as unregistered. +{ + const realBase = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "cpa-"))); + const realProject = path.join(realBase, "real", "cave"); + fs.mkdirSync(path.join(realProject, ".worktrees", "feat-x"), { recursive: true }); + const linkedProject = path.join(realBase, "linked-cave"); + fs.symlinkSync(realProject, linkedProject); + + const linkedProjects = [ + { + id: "proj-linked", + name: "Cave", + root: linkedProject, + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + }, + ]; + + assert.equal( + chatProjectAccessId({ + projects: linkedProjects, + requestedProjectRoot: path.join(linkedProject, ".worktrees", "feat-x"), + resolvedCwd: path.join(realProject, ".worktrees", "feat-x"), + }), + "proj-linked", + "a worktree under a symlink-registered project still authorizes against that project", + ); + + fs.mkdirSync(path.join(realBase, "elsewhere"), { recursive: true }); + assert.equal( + chatProjectAccessId({ + projects: linkedProjects, + requestedProjectRoot: path.join(linkedProject, ".worktrees", "feat-x"), + resolvedCwd: path.join(realBase, "elsewhere"), + }), + `unregistered:${path.join(linkedProject, ".worktrees", "feat-x")}`, + "canonicalizing the project root does not widen containment", + ); + + fs.rmSync(realBase, { recursive: true, force: true }); +} + +console.log("chat-project-access worktree cwd containment tests passed"); diff --git a/src/lib/chat-project-access.ts b/src/lib/chat-project-access.ts index 09e7737b7..86207d064 100644 --- a/src/lib/chat-project-access.ts +++ b/src/lib/chat-project-access.ts @@ -2,6 +2,7 @@ import path from "node:path"; import type { CaveProject } from "./cave-projects-types.ts"; import { projectById, projectForRoot } from "./cave-projects.ts"; +import { realpathOrResolve } from "./server/canonical-path.ts"; export type ChatProjectAccessArgs = { projects: CaveProject[]; @@ -16,17 +17,48 @@ export type ChatProjectAccessArgs = { }; /** - * The registered project whose `.worktrees/` directory contains `root`, if - * any. Separator-exact and traversal-safe: the candidate is `path.resolve`d - * (collapsing `..` escapes) and must sit strictly BELOW - * `/.worktrees/`, so `/proj-evil/...`, `/proj/.worktrees` itself, - * and `/proj/.worktrees/../..` all miss. + * The registered project whose `.worktrees/` directory contains BOTH the + * requested root and the cwd the runtime actually resolved for this turn. + * + * Separator-exact and traversal-safe: every path is canonicalized with + * `realpathOrResolve` (collapsing `..` escapes AND symlinks) and must sit + * strictly BELOW `/.worktrees/`, so `/proj-evil/...`, + * `/proj/.worktrees` itself, and `/proj/.worktrees/../..` all miss. + * + * Checking the resolved cwd is the security half: the requested root is + * client-supplied, so on its own it only proves the CLIENT spelled a path + * under a registered project. A symlink at `/.worktrees/` + * pointing anywhere else would otherwise hand the caller the parent project's + * grant while the harness ran outside it. `resolvedCwd` arrives realpathed + * from `resolveLocalRuntimeCwd`, so requiring it under the same prefix pins + * authorization to where the work will actually happen. + * + * Canonicalizing the project root is the correctness half, and it is required + * for the check above to be usable: `project.root` is stored as registered, + * so a project registered through a symlinked path — or any root with a + * symlinked ancestor, `/var -> /private/var` on macOS being the everyday case + * — builds a prefix the realpathed cwd can never start with, and every + * legitimate `.worktrees/` chat fails closed as unregistered. Putting + * both sides in one namespace never widens containment: a symlink that + * escapes the project resolves outside the prefix and still misses. */ -function worktreeParentProject(root: string, projects: CaveProject[]): CaveProject | null { - const resolved = path.resolve(root); +function worktreeParentProject( + root: string, + resolvedCwd: string, + projects: CaveProject[], +): CaveProject | null { + const requested = realpathOrResolve(root); + const realCwd = realpathOrResolve(resolvedCwd); for (const project of projects) { - const prefix = path.resolve(project.root) + path.sep + ".worktrees" + path.sep; - if (resolved.startsWith(prefix) && resolved.length > prefix.length) return project; + const prefix = realpathOrResolve(project.root) + path.sep + ".worktrees" + path.sep; + if ( + requested.startsWith(prefix) && + requested.length > prefix.length && + realCwd.startsWith(prefix) && + realCwd.length > prefix.length + ) { + return project; + } } return null; } @@ -111,7 +143,7 @@ export function chatProjectAccessId(args: ChatProjectAccessArgs): string | null projectForRoot(args.resolvedCwd, args.projects); if (project) return project.id; - const worktreeParent = worktreeParentProject(projectRoot, args.projects); + const worktreeParent = worktreeParentProject(projectRoot, args.resolvedCwd, args.projects); if (worktreeParent) return worktreeParent.id; return `unregistered:${projectRoot}`;