diff --git a/src/permission/authz-grants.ts b/src/permission/authz-grants.ts index 1bc5a7420..985df5e03 100644 --- a/src/permission/authz-grants.ts +++ b/src/permission/authz-grants.ts @@ -2,6 +2,7 @@ import { evaluateGrants, type GrantRule } from "@intx/authz"; import type { Approval } from "./types.js"; import { matchesPattern } from "./matcher.js"; +import { realpathOr } from "./worktree-roots.js"; // Exact-escaped patterns (backslash before metacharacters) cannot round-trip // through @intx/authz matchPattern, so those grants are filtered out of the @@ -25,12 +26,48 @@ export function approvalToGrantRule(approval: Approval, index: number): GrantRul }; } +// The gate's own project boundary: the session root it was constructed with, +// plus every git worktree registered against that root (which, per CL-4929, +// may live outside the root entirely — a sibling directory, not a +// subdirectory). Built once per gate from its closed-over resolvedCwd and +// rootsProvider and threaded through — never accept one built anywhere else, +// or "same project" quietly stops meaning "same gate's project." +export type GrantWorkspace = { resolvedCwd: string; roots: readonly string[] }; + +// A project-scoped grant (Approval.cwd set) is confined to the session that +// minted it: it may replay only for a request whose cwd is that same session +// root, or one of the root's registered worktrees. A worktree cwd never +// equals the session root by string identity (that's the bug this closes), +// so membership is resolved through `workspace` instead of a bare `===`. +// +// `grantCwd !== workspace.resolvedCwd` is the boundary: a grant stamped with +// some OTHER project's root is rejected before roots are ever consulted, so +// a request cwd that happens to coincide with a different project's worktree +// can never match. Membership within a matching project is exact equality +// against the resolved roots, never a path-prefix — a prefix check would let +// a maliciously named sibling directory (`/repo/wt-1-evil`) match a +// legitimate root (`/repo/wt-1`). `workspace.roots` already comes back +// realpath-resolved (see worktree-roots.ts); `requestCwd` is realpath'd here +// so a symlinked checkout (macOS /tmp vs /private/tmp) still compares equal. +export function cwdMatchesGrant( + grantCwd: string | undefined, + requestCwd: string | undefined, + workspace: GrantWorkspace, +): boolean { + if (grantCwd === undefined) return true; + if (requestCwd === undefined) return false; + if (grantCwd === requestCwd) return true; + if (grantCwd !== workspace.resolvedCwd) return false; + return workspace.roots.includes(realpathOr(requestCwd)); +} + export type EvaluateApprovalsInput = { tool: string; subject: string; approvals: readonly Approval[]; activeProviderModel?: string | undefined; requestCwd?: string | undefined; + workspace: GrantWorkspace; }; // Grant-store evaluation via @intx/authz. Filters provider-model and cwd the @@ -39,12 +76,12 @@ export type EvaluateApprovalsInput = { // matchesPattern (equality after unescape) first so a stored exact command is // never lost. export async function evaluateApprovals(input: EvaluateApprovalsInput): Promise { - const { tool, subject, approvals, activeProviderModel, requestCwd } = input; + const { tool, subject, approvals, activeProviderModel, requestCwd, workspace } = input; const scoped = approvals.filter( (a) => a.tool === tool && (a.providerModel === undefined || a.providerModel === activeProviderModel) && - (a.cwd === undefined || a.cwd === requestCwd), + cwdMatchesGrant(a.cwd, requestCwd, workspace), ); if (scoped.length === 0) return false; diff --git a/src/permission/gate.test.ts b/src/permission/gate.test.ts index fefed1d5d..8b769a212 100644 --- a/src/permission/gate.test.ts +++ b/src/permission/gate.test.ts @@ -60,7 +60,9 @@ describe("preGrantGuardReason / isRequestCoveredByGrant guard parity", () => { cwd, }; const grant: Approval = { tool: "run_shell", pattern: command }; - expect(isRequestCoveredByGrant(request, grant, undefined, isRestricted)).toBe(false); + expect( + isRequestCoveredByGrant(request, grant, undefined, isRestricted, { resolvedCwd: cwd, roots: [] }), + ).toBe(false); }); test(`${name}: evaluate() never allows outright`, async () => { @@ -85,7 +87,9 @@ describe("preGrantGuardReason / isRequestCoveredByGrant guard parity", () => { }; expect(preGrantGuardReason(request, isRestricted)).toBeUndefined(); const grant: Approval = { tool: "run_shell", pattern: "npm test" }; - expect(isRequestCoveredByGrant(request, grant, undefined, isRestricted)).toBe(true); + expect( + isRequestCoveredByGrant(request, grant, undefined, isRestricted, { resolvedCwd: cwd, roots: [] }), + ).toBe(true); }); }); @@ -122,7 +126,12 @@ describe("grant coverage rebinds relative paths to the request process cwd", () cwd: agentCwd, }; const grant: Approval = { tool: "run_shell", pattern: "cat *" }; - expect(isRequestCoveredByGrant(request, grant, undefined, sessionRestricted)).toBe(false); + expect( + isRequestCoveredByGrant(request, grant, undefined, sessionRestricted, { + resolvedCwd: sessionCwd, + roots: [], + }), + ).toBe(false); }); test("a relative path inside the registered worktree is not forced-restricted", () => { @@ -135,6 +144,11 @@ describe("grant coverage rebinds relative paths to the request process cwd", () cwd: agentCwd, }; const grant: Approval = { tool: "run_shell", pattern: "cat *" }; - expect(isRequestCoveredByGrant(request, grant, undefined, sessionRestricted)).toBe(true); + expect( + isRequestCoveredByGrant(request, grant, undefined, sessionRestricted, { + resolvedCwd: sessionCwd, + roots: [], + }), + ).toBe(true); }); }); diff --git a/src/permission/gate.ts b/src/permission/gate.ts index 0b3929f34..5d0fb7713 100644 --- a/src/permission/gate.ts +++ b/src/permission/gate.ts @@ -14,7 +14,7 @@ import { autoShellRuleForCall } from "./auto-shell-policy.js"; import { commandReferencesSensitivePath } from "../plugins/secret-guard-plugin.js"; import { runShellAuthzBlockReason } from "../shell/run-shell-authz.js"; import { matchesPattern, escapeGlobLiteral } from "./matcher.js"; -import { evaluateApprovals } from "./authz-grants.js"; +import { evaluateApprovals, cwdMatchesGrant, type GrantWorkspace } from "./authz-grants.js"; import { splitChainedCommand, tokenize, isShellCommentOnly, stripCommentLines } from "./command.js"; import { createPathRestriction } from "./path-restriction.js"; import { createWorktreeRootsProvider, type RootsProvider } from "./worktree-roots.js"; @@ -51,6 +51,7 @@ function hasExactFullCommandGrant( approvals: readonly Approval[], activeProviderModel: string | undefined, requestCwd: string | undefined, + workspace: GrantWorkspace, ): boolean { // Comment-insensitive: a model-authored "# why" line prepended to an // otherwise-identical command must still replay against a grant minted @@ -62,7 +63,7 @@ function hasExactFullCommandGrant( a.tool === tool && a.pattern === normalized && (a.providerModel === undefined || a.providerModel === activeProviderModel) && - (a.cwd === undefined || a.cwd === requestCwd), + cwdMatchesGrant(a.cwd, requestCwd, workspace), ); } @@ -142,9 +143,10 @@ export function isRequestCoveredByGrant( approval: Approval, activeProviderModel: string | undefined, isRestricted: (path: string, isWrite: boolean) => boolean, + workspace: GrantWorkspace, ): boolean { if (request.tool !== approval.tool) return false; - if (approval.cwd !== undefined && approval.cwd !== request.cwd) return false; + if (!cwdMatchesGrant(approval.cwd, request.cwd, workspace)) return false; if ( approval.providerModel !== undefined && approval.providerModel !== activeProviderModel @@ -268,11 +270,15 @@ export function createPermissionGate(options: PermissionGateOptions): Permission const { requestApproval, persist, interactive, skipPermissions, providerName, model, cwd } = options; const mcpTiers = options.mcpTiers ?? createMcpToolPermissionRegistry(); const resolvedCwd = cwd ?? process.cwd(); - const pathRestriction = createPathRestriction( - resolvedCwd, - options.rootsProvider ?? createWorktreeRootsProvider(resolvedCwd), - ); + const rootsProvider = options.rootsProvider ?? createWorktreeRootsProvider(resolvedCwd); + const pathRestriction = createPathRestriction(resolvedCwd, rootsProvider); const isRestricted = pathRestriction.isRestricted; + // This gate's project boundary for grant matching (see cwdMatchesGrant): + // this session's root plus its currently-known registered worktrees. Built + // fresh per read from the same rootsProvider the gate already uses for + // path containment, so "same project" for a grant and "inside the + // workspace" for a path share one authority. + const grantWorkspace = (): GrantWorkspace => ({ resolvedCwd, roots: rootsProvider() }); let auto = options.auto; // Own a private copy so evaluating a grant never mutates the caller's array. const approvals: Approval[] = [...options.approvals]; @@ -309,7 +315,7 @@ export function createPermissionGate(options: PermissionGateOptions): Permission persist?.(approval, grant); } options.onGrant?.(approval, (request) => - isRequestCoveredByGrant(request, approval, activeProviderModel, isRestricted), + isRequestCoveredByGrant(request, approval, activeProviderModel, isRestricted, grantWorkspace()), ); }; @@ -405,7 +411,14 @@ export function createPermissionGate(options: PermissionGateOptions): Permission !fullReferencesSecret && !commandTargetsRestricted(fullCommand, isRestrictedHere) && segments.length > 1 && - hasExactFullCommandGrant(request.tool, fullCommand, approvals, activeProviderModel, effectiveCwd) + hasExactFullCommandGrant( + request.tool, + fullCommand, + approvals, + activeProviderModel, + effectiveCwd, + grantWorkspace(), + ) ) { continue; } @@ -432,6 +445,7 @@ export function createPermissionGate(options: PermissionGateOptions): Permission approvals, activeProviderModel, requestCwd: effectiveCwd, + workspace: grantWorkspace(), }) ) { continue; @@ -502,6 +516,7 @@ export function createPermissionGate(options: PermissionGateOptions): Permission approvals, activeProviderModel, requestCwd: effectiveCwd, + workspace: grantWorkspace(), }); if (alreadyApproved) { continue; diff --git a/src/permission/permission.test.ts b/src/permission/permission.test.ts index 22b03dd0a..4a546738a 100644 --- a/src/permission/permission.test.ts +++ b/src/permission/permission.test.ts @@ -260,16 +260,22 @@ describe("evaluateApprovals (@intx/authz evaluateGrants)", () => { { tool: "write_file", pattern: "src/*" }, { tool: "run_shell", pattern: "rm -rf build/\\*" }, ]; + // No approval in these fixtures carries a cwd, so the workspace passed here + // is never actually consulted (cwdMatchesGrant short-circuits on + // grantCwd === undefined) — an explicit no-op value is threaded through + // instead of an optional param, so a future call site can't silently + // narrow the security check by forgetting to pass one. + const noWorkspace = { resolvedCwd: "/unused", roots: [] }; test("allows package-compatible wildcard grants", async () => { expect( - await evaluateApprovals({ tool: "run_shell", subject: "npm test", approvals }), + await evaluateApprovals({ tool: "run_shell", subject: "npm test", approvals, workspace: noWorkspace }), ).toBe(true); expect( - await evaluateApprovals({ tool: "run_shell", subject: "curl x", approvals }), + await evaluateApprovals({ tool: "run_shell", subject: "curl x", approvals, workspace: noWorkspace }), ).toBe(false); expect( - await evaluateApprovals({ tool: "write_file", subject: "src/a.ts", approvals }), + await evaluateApprovals({ tool: "write_file", subject: "src/a.ts", approvals, workspace: noWorkspace }), ).toBe(true); }); @@ -279,6 +285,7 @@ describe("evaluateApprovals (@intx/authz evaluateGrants)", () => { tool: "run_shell", subject: "rm -rf build/*", approvals, + workspace: noWorkspace, }), ).toBe(true); expect( @@ -286,6 +293,7 @@ describe("evaluateApprovals (@intx/authz evaluateGrants)", () => { tool: "run_shell", subject: "rm -rf build/../../etc", approvals, + workspace: noWorkspace, }), ).toBe(false); }); @@ -301,6 +309,7 @@ describe("evaluateApprovals (@intx/authz evaluateGrants)", () => { subject: "npm test", approvals: scoped, activeProviderModel: "openai:gpt-4o", + workspace: noWorkspace, }), ).toBe(true); expect( @@ -309,6 +318,7 @@ describe("evaluateApprovals (@intx/authz evaluateGrants)", () => { subject: "npm test", approvals: scoped, activeProviderModel: "anthropic:opus", + workspace: noWorkspace, }), ).toBe(false); expect( @@ -317,6 +327,7 @@ describe("evaluateApprovals (@intx/authz evaluateGrants)", () => { subject: "git status", approvals: scoped, requestCwd: "/repo-a", + workspace: noWorkspace, }), ).toBe(true); expect( @@ -325,9 +336,69 @@ describe("evaluateApprovals (@intx/authz evaluateGrants)", () => { subject: "git status", approvals: scoped, requestCwd: "/repo-b", + workspace: noWorkspace, }), ).toBe(false); }); + + test("a project grant minted at the session root matches a request whose cwd is a registered worktree of that root", async () => { + const scoped: Approval[] = [{ tool: "run_shell", pattern: "git *", cwd: "/session-root" }]; + const workspace = { resolvedCwd: "/session-root", roots: ["/sibling-dispatch-wts/agent-1"] }; + expect( + await evaluateApprovals({ + tool: "run_shell", + subject: "git status", + approvals: scoped, + requestCwd: "/sibling-dispatch-wts/agent-1", + workspace, + }), + ).toBe(true); + }); + + // Security test: a grant minted for one project must never authorize a + // request whose cwd belongs to a completely different project, even when + // that other project also happens to be a git worktree somewhere. Must + // pass both before and after the worktree-matching fix. + test("a project grant does not match a request from an unrelated project root", async () => { + const scoped: Approval[] = [{ tool: "run_shell", pattern: "git *", cwd: "/session-root" }]; + const workspace = { resolvedCwd: "/session-root", roots: ["/sibling-dispatch-wts/agent-1"] }; + expect( + await evaluateApprovals({ + tool: "run_shell", + subject: "git status", + approvals: scoped, + requestCwd: "/some-other-unrelated-project", + workspace, + }), + ).toBe(false); + }); + + test("session and provider-model scopes (no cwd) are unaffected by workspace membership", async () => { + const scoped: Approval[] = [ + { tool: "run_shell", pattern: "npm *" }, + { tool: "run_shell", pattern: "git *", providerModel: "openai:gpt-4o" }, + ]; + const workspace = { resolvedCwd: "/session-root", roots: ["/sibling-dispatch-wts/agent-1"] }; + expect( + await evaluateApprovals({ + tool: "run_shell", + subject: "npm test", + approvals: scoped, + requestCwd: "/anywhere-at-all", + workspace, + }), + ).toBe(true); + expect( + await evaluateApprovals({ + tool: "run_shell", + subject: "git status", + approvals: scoped, + activeProviderModel: "openai:gpt-4o", + requestCwd: "/anywhere-at-all", + workspace, + }), + ).toBe(true); + }); }); describe("classifyTool", () => { @@ -2916,6 +2987,124 @@ describe("sub-agent identity on permission requests", () => { }); }); +describe("project-scoped grants match sub-agent worktree requests (CL-5662)", () => { + const git = (cwd: string, ...args: string[]): void => { + execFileSync("git", args, { cwd, stdio: "ignore" }); + }; + + // A sibling worktree, not nested under the session root — mirrors CL-4929's + // real-world layout where a sub-agent's worktree lives outside the repo + // entirely (e.g. a dispatch worktrees directory next to the checkout). + const createRepoWithSiblingWorktree = (): { repo: string; worktree: string } => { + const base = mkdtempSync(join(tmpdir(), "corbits-project-grant-")); + const repo = join(base, "repo"); + const worktree = join(base, "sibling-worktree"); + mkdirSync(repo); + git(repo, "init", "-b", "main"); + git(repo, "-c", "user.email=t@t", "-c", "user.name=t", "commit", "--allow-empty", "-m", "init"); + git(repo, "worktree", "add", worktree); + return { repo, worktree }; + }; + + test("a project grant minted at the session root matches a sub-agent request whose cwd is a worktree under that root", async () => { + const { runWithSubAgentIdentity } = await import("../subagent/identity-context.js"); + const { repo, worktree } = createRepoWithSiblingWorktree(); + let asked = 0; + const gate = createPermissionGate({ + approvals: [], + cwd: repo, + requestApproval: async () => { + asked++; + return { allow: true, persist: { id: "exact", label: "Always allow", pattern: "npm *", grant: "project" } }; + }, + interactive: true, + skipPermissions: false, + }); + + // First call, from the session root, mints the project grant. + const first = await gate.evaluate(shellCall("npm test")); + expect(first.allowed).toBe(true); + expect(asked).toBe(1); + + // Second call, from a sub-agent running in the sibling worktree, must + // replay the same project grant instead of asking again. + const second = await runWithSubAgentIdentity({ description: "Worker", cwd: worktree }, () => + gate.evaluate(shellCall("npm run build")), + ); + expect(second.allowed).toBe(true); + expect(asked).toBe(1); + }); + + // Security test: a project grant must never leak to a request from a + // genuinely unrelated project's directory, even though that directory is + // just as "foreign" on disk as a legitimate worktree would look to a naive + // check. Must pass both before and after the worktree-matching fix. + test("a project grant does not match a request from an unrelated project root", async () => { + const { runWithSubAgentIdentity } = await import("../subagent/identity-context.js"); + const { repo } = createRepoWithSiblingWorktree(); + const unrelated = mkdtempSync(join(tmpdir(), "corbits-unrelated-project-")); + let asked = 0; + const gate = createPermissionGate({ + approvals: [], + cwd: repo, + requestApproval: async () => { + asked++; + return { allow: true, persist: { id: "exact", label: "Always allow", pattern: "npm *", grant: "project" } }; + }, + interactive: true, + skipPermissions: false, + }); + + const first = await gate.evaluate(shellCall("npm test")); + expect(first.allowed).toBe(true); + expect(asked).toBe(1); + + const second = await runWithSubAgentIdentity({ description: "Worker", cwd: unrelated }, () => + gate.evaluate(shellCall("npm run build")), + ); + expect(second.allowed).toBe(true); + // The unrelated cwd must still ask — the grant did not leak across + // projects — even though the operator happens to approve it again here. + expect(asked).toBe(2); + }); + + // Uses write_file rather than run_shell: every bare shell token is itself + // judged for path containment against the calling agent's cwd (a + // pre-existing, unrelated restriction — see classify.ts's + // commandTargetsRestricted), so a shell command issued from a genuinely + // foreign cwd always asks regardless of any grant. write_file's subject is + // the target path, not the agent's cwd, so it isolates the thing this test + // actually checks: that an unscoped (no-cwd) grant matches irrespective of + // where the request originated. + test("session and provider-model grants still match a sub-agent request regardless of cwd", async () => { + const { runWithSubAgentIdentity } = await import("../subagent/identity-context.js"); + const { repo } = createRepoWithSiblingWorktree(); + const unrelated = mkdtempSync(join(tmpdir(), "corbits-unrelated-project-")); + const target = join(repo, "notes.md"); + let asked = 0; + const gate = createPermissionGate({ + approvals: [], + cwd: repo, + requestApproval: async () => { + asked++; + return { allow: true, persist: { id: "exact", label: "Always allow", pattern: target, grant: "session" } }; + }, + interactive: true, + skipPermissions: false, + }); + + const first = await gate.evaluate({ id: "a", name: "write_file", arguments: { path: target } }); + expect(first.allowed).toBe(true); + expect(asked).toBe(1); + + const second = await runWithSubAgentIdentity({ description: "Worker", cwd: unrelated }, () => + gate.evaluate({ id: "b", name: "write_file", arguments: { path: target } }), + ); + expect(second.allowed).toBe(true); + expect(asked).toBe(1); + }); +}); + describe("sub-agent auto-allow uses the process cwd, not the session cwd", () => { test("a relative read inside the worktree auto-allows under the process cwd", async () => { // Nested worktree under the session so absolute paths stay inside the diff --git a/src/permission/worktree-roots.ts b/src/permission/worktree-roots.ts index 50ff13765..b174b3094 100644 --- a/src/permission/worktree-roots.ts +++ b/src/permission/worktree-roots.ts @@ -7,8 +7,10 @@ const execFileAsync = promisify(execFile); // Git prints realpaths; the caller's cwd may reach the same directory through a // symlink (e.g. macOS /tmp, /var). Canonicalize both sides so self-exclusion -// compares like with like. -function realpathOr(path: string): string { +// compares like with like. Exported so other permission-layer code comparing +// a path against these roots (e.g. grant cwd matching in authz-grants.ts) +// normalizes through the same function rather than reimplementing it. +export function realpathOr(path: string): string { try { return realpathSync(path); } catch {