From afd09cc9fed8aa942bc03afbdeab72524d85fa9b Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 12 Sep 2026 17:26:06 -0700 Subject: [PATCH 1/2] Explain grant-mismatch asks with a prompt notice --- src/permission/auto-shell-policy.test.ts | 29 +++++ src/permission/auto-shell-policy.ts | 8 +- src/permission/gate.test.ts | 129 +++++++++++++++++++++++ src/permission/gate.ts | 66 +++++++++++- 4 files changed, 228 insertions(+), 4 deletions(-) diff --git a/src/permission/auto-shell-policy.test.ts b/src/permission/auto-shell-policy.test.ts index cc3140cf5..b28c4ab19 100644 --- a/src/permission/auto-shell-policy.test.ts +++ b/src/permission/auto-shell-policy.test.ts @@ -96,3 +96,32 @@ describe("read-only git commands pass through the policy", () => { ).toBeUndefined(); }); }); + +describe("git worktree --force= asks (CL-6824)", () => { + // Control: a contained non-force add is not flagged at all. + test("contained non-force add stays unflagged", () => { + expect( + autoShellRuleForCall(shellCall("git worktree add ./wt-plain")), + ).toBeUndefined(); + }); + + // Git has no --force= form — real git dies with + // "error: option `force' takes no value" (exit 129) — but the spelling + // still expresses force intent, so the policy asks rather than letting the + // --flag=value skip swallow it the way the old exact-match check did. + test("--force= spellings hit the worktree ask rule", () => { + for (const flag of ["--force=true", "--force=1", "--force="]) { + expect( + autoShellRuleForCall(shellCall(`git worktree add ${flag} ./wt-eq`)) + ?.name, + ).toBe("git-worktree"); + } + }); + + // The negation is not force: --no-force must not be caught by the prefix. + test("--no-force stays unflagged", () => { + expect( + autoShellRuleForCall(shellCall("git worktree add --no-force ./wt-no")), + ).toBeUndefined(); + }); +}); diff --git a/src/permission/auto-shell-policy.ts b/src/permission/auto-shell-policy.ts index d05176bef..987e7c958 100644 --- a/src/permission/auto-shell-policy.ts +++ b/src/permission/auto-shell-policy.ts @@ -346,8 +346,12 @@ const WORKTREE_PRUNE_FLAGS = new Set(["-n", "--dry-run", "-v", "--verbose"]); // Flags that take a following value on `git worktree add` (branch name, lock reason). const WORKTREE_ADD_VALUE_FLAGS = new Set(["-b", "-B", "--reason"]); -function isWorktreeForceFlag(arg: string): boolean { - return arg === "-f" || arg === "--force"; +export function isWorktreeForceFlag(arg: string): boolean { + // Git's --force takes no value (real git rejects --force= with + // "error: option `force' takes no value"), but the spelling still expresses + // force intent, so the policy treats it as force rather than letting the + // --flag=value path skip swallow it. + return arg === "-f" || arg === "--force" || arg.startsWith("--force="); } // True when the path is safe for unattended worktree add/remove: inside the diff --git a/src/permission/gate.test.ts b/src/permission/gate.test.ts index 1bcb68637..b206fa2c5 100644 --- a/src/permission/gate.test.ts +++ b/src/permission/gate.test.ts @@ -286,3 +286,132 @@ describe("standing grant covers a later git worktree command (CL-5638)", () => { expect(prompts).toBe(1); }); }); + +// CL-6824: when a standing grant covers a command but a pre-grant guard still +// forces an ask, the prompt carries PermissionRequest.notice naming the +// guard's reason. Matching semantics are unchanged — every case below still +// asks (and stays deniable); only the prompt gains the why. +describe("grant-mismatch asks carry the guard reason as a notice (CL-6824)", () => { + const root = mkdtempSync(join(tmpdir(), "gate-mismatch-notice-")); + const sessionCwd = join(root, "main"); + const git = (args: string[], cwd: string) => + execFileSync("git", args, { cwd, stdio: "ignore" }); + mkdirSync(sessionCwd); + initTemporaryGitRepo(sessionCwd, { initArgs: ["-q"] }); + writeFileSync(join(sessionCwd, "seed.txt"), "seed\n"); + git(["add", "."], sessionCwd); + git(["commit", "-qm", "seed"], sessionCwd); + + async function askWithGrants(command: string, approvals: Approval[]) { + const seen: PermissionRequest[] = []; + const gate = createPermissionGate({ + approvals, + interactive: true, + skipPermissions: false, + reactorGated: false, + cwd: sessionCwd, + rootsProvider: () => [], + requestApproval: async (request) => { + seen.push(request); + return { allow: false }; + }, + }); + const verdict = await gate.evaluate(shellCall(command)); + return { verdict, seen }; + } + + const worktreeGrant: Approval[] = [ + { tool: "run_shell", pattern: "git worktree *" }, + ]; + const catGrant: Approval[] = [{ tool: "run_shell", pattern: "cat *" }]; + + test("force worktree names --force in the notice", async () => { + const { verdict, seen } = await askWithGrants( + "git worktree add --force ../sib-force", + worktreeGrant, + ); + expect(verdict.allowed).toBe(false); + expect(seen).toHaveLength(1); + expect(seen[0]?.notice).toBe( + "A standing grant matches this command, but it uses --force, so it still needs approval.", + ); + }); + + test("--force= is still force in the notice", async () => { + const { verdict, seen } = await askWithGrants( + "git worktree add --force=true ../sib-force-eq", + worktreeGrant, + ); + expect(verdict.allowed).toBe(false); + expect(seen).toHaveLength(1); + expect(seen[0]?.notice).toBe( + "A standing grant matches this command, but it uses --force, so it still needs approval.", + ); + }); + + test("uncontained destination names the approved locations", async () => { + // A direct child of tmpdir() is not a permitted sibling of sessionCwd + // (only direct children of root/ are), so the restricted guard trips. + const outside = join(tmpdir(), "gate-6824-outside"); + const { verdict, seen } = await askWithGrants( + `git worktree add ${outside}`, + worktreeGrant, + ); + expect(verdict.allowed).toBe(false); + expect(seen).toHaveLength(1); + expect(seen[0]?.notice).toBe( + "A standing grant matches this command, but the worktree destination is outside the approved locations, so it still needs approval.", + ); + }); + + test("secret reference names the sensitive path", async () => { + const { verdict, seen } = await askWithGrants("cat .env", catGrant); + expect(verdict.allowed).toBe(false); + expect(seen).toHaveLength(1); + expect(seen[0]?.notice).toBe( + "A standing grant matches this command, but it references a sensitive path, so it still needs approval.", + ); + // The secret ask still strips grant scopes; the notice survives it. + expect(seen[0]?.scopes).toEqual([]); + }); + + test("restricted target names the workspace", async () => { + const { verdict, seen } = await askWithGrants("cat /etc/passwd", catGrant); + expect(verdict.allowed).toBe(false); + expect(seen).toHaveLength(1); + expect(seen[0]?.notice).toBe( + "A standing grant matches this command, but it targets a path outside the workspace, so it still needs approval.", + ); + }); + + test("an ask with no matching grant carries no notice", async () => { + const { verdict, seen } = await askWithGrants( + "git worktree add --force ../sib-nogrant", + [], + ); + expect(verdict.allowed).toBe(false); + expect(seen).toHaveLength(1); + expect(seen[0]?.notice).toBeUndefined(); + }); + + test("a covered command still allows with no prompt and no notice", async () => { + const seen: PermissionRequest[] = []; + const gate = createPermissionGate({ + approvals: worktreeGrant, + interactive: true, + skipPermissions: false, + reactorGated: false, + cwd: sessionCwd, + rootsProvider: () => [], + requestApproval: async (request) => { + seen.push(request); + return { allow: false }; + }, + }); + const verdict = await gate.evaluate( + shellCall("git worktree add ../sib-plain -b br-plain"), + ); + expect(verdict.allowed).toBe(true); + expect(seen).toHaveLength(0); + }); +}); diff --git a/src/permission/gate.ts b/src/permission/gate.ts index a3810da64..8c593232f 100644 --- a/src/permission/gate.ts +++ b/src/permission/gate.ts @@ -18,6 +18,7 @@ import { import { autoShellRuleForCall, safeWorktreeCommand, + isWorktreeForceFlag, } from "./auto-shell-policy.js"; import { commandReferencesSensitivePath } from "../plugins/secret-guard-plugin.js"; import { @@ -34,6 +35,7 @@ import { import { splitChainedCommand, isShellCommentOnly, + tokenize, stripCommentLines, } from "./command.js"; import { @@ -145,6 +147,39 @@ function segmentGuard( return undefined; } +// Classifies a guarded `git worktree add/remove` segment so a grant-mismatch +// notice can name the operative reason: a force flag, or a destination the +// containment authority did not approve. Returns undefined for anything else. +// Display-only refinement — the guard decision itself is unchanged. +function worktreeMismatchKind( + segment: string, +): "force" | "destination" | undefined { + const tokens = tokenize(segment); + if (tokens[0] !== "git" || tokens[1] !== "worktree") return undefined; + if (tokens[2] !== "add" && tokens[2] !== "remove") return undefined; + return tokens.slice(3).some(isWorktreeForceFlag) ? "force" : "destination"; +} + +// Explains a grant mismatch: a standing grant covers the segment, but the +// pre-grant guard still forced an ask. The wording names the guard's reason, +// never the grant — matching semantics are untouched. +function grantMismatchNotice( + segment: string, + kind: "secret" | "restricted", +): string { + if (kind === "secret") { + return "A standing grant matches this command, but it references a sensitive path, so it still needs approval."; + } + const worktreeKind = worktreeMismatchKind(segment); + if (worktreeKind === "force") { + return "A standing grant matches this command, but it uses --force, so it still needs approval."; + } + if (worktreeKind === "destination") { + return "A standing grant matches this command, but the worktree destination is outside the approved locations, so it still needs approval."; + } + return "A standing grant matches this command, but it targets a path outside the workspace, so it still needs approval."; +} + // Relative path tokens in a shell command resolve against the process cwd of the // agent that issued the call — not the session cwd that built the gate. Absolute // paths pass through unchanged so createPathRestriction still judges them against @@ -731,6 +766,7 @@ export function createPermissionGate( let needsOperator = false; let anySecret = false; + let mismatchNotice: string | undefined; for (const segment of segments) { // A secret-path reference or restricted target always requires the // operator, whether the segment would otherwise auto-allow or match @@ -747,6 +783,22 @@ export function createPermissionGate( if (guard !== undefined) { if (guard.kind === "secret") anySecret = true; needsOperator = true; + // A standing grant may still cover this segment even though the + // pre-grant guard forces an ask — record why so the prompt can say + // so. Matching semantics are untouched; this only annotates the ask. + if ( + mismatchNotice === undefined && + (await evaluateApprovals({ + tool: request.tool, + subject: segment, + approvals, + activeProviderModel, + requestCwd: effectiveCwd, + workspace: grantWorkspace(), + })) + ) { + mismatchNotice = grantMismatchNotice(segment, guard.kind); + } continue; } if ( @@ -788,9 +840,19 @@ export function createPermissionGate( // Secret-path shell must never mint a stored grant — even an exact match // would be misleading because future secret-path shell always re-asks. + // A grant mismatch carries the guard's reason on the prompt so the + // operator sees why the standing grant did not apply. const requestForOperator = anySecret - ? { ...request, scopes: [] } - : request; + ? { + ...request, + scopes: [], + ...(mismatchNotice !== undefined + ? { notice: mismatchNotice } + : null), + } + : mismatchNotice !== undefined + ? { ...request, notice: mismatchNotice } + : request; return { kind: "ask", request: requestForOperator, From 86da3fae8059328d1fea329399f7f3bd5eb799cc Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 12 Sep 2026 17:36:44 -0700 Subject: [PATCH 2/2] Treat short -f spellings as force and fix remove notice wording Short -f takes no value: real git rejects -f= and glued -f the way it rejects --force=, so the worktree policy treats them as force instead of letting the generic-flag skip swallow them. The grant-mismatch notice keeps the destination noun for add and names the worktree itself for remove. --- src/permission/auto-shell-policy.test.ts | 24 ++++++++- src/permission/auto-shell-policy.ts | 8 ++- src/permission/gate.test.ts | 62 +++++++++++++++++++++++- src/permission/gate.ts | 10 +++- 4 files changed, 99 insertions(+), 5 deletions(-) diff --git a/src/permission/auto-shell-policy.test.ts b/src/permission/auto-shell-policy.test.ts index b28c4ab19..b2e8eae4b 100644 --- a/src/permission/auto-shell-policy.test.ts +++ b/src/permission/auto-shell-policy.test.ts @@ -97,7 +97,7 @@ describe("read-only git commands pass through the policy", () => { }); }); -describe("git worktree --force= asks (CL-6824)", () => { +describe("git worktree force spellings ask (CL-6824)", () => { // Control: a contained non-force add is not flagged at all. test("contained non-force add stays unflagged", () => { expect( @@ -118,10 +118,32 @@ describe("git worktree --force= asks (CL-6824)", () => { } }); + // Short -f takes no value either — real git dies with + // "error: unknown switch `='" for `-f=` and + // "error: unknown switch `'" for glued `-f` (exit 129 both) — + // but the spellings still express force intent, so the policy asks rather + // than letting the generic-flag skip swallow them. + test("-f= and glued -f spellings hit the worktree ask rule", () => { + for (const flag of ["-f=true", "-f=", "-ftrue", "-ff"]) { + expect( + autoShellRuleForCall(shellCall(`git worktree add ${flag} ./wt-short`)) + ?.name, + ).toBe("git-worktree"); + expect( + autoShellRuleForCall( + shellCall(`git worktree remove ${flag} ./wt-short`), + )?.name, + ).toBe("git-worktree"); + } + }); + // The negation is not force: --no-force must not be caught by the prefix. test("--no-force stays unflagged", () => { expect( autoShellRuleForCall(shellCall("git worktree add --no-force ./wt-no")), ).toBeUndefined(); + expect( + autoShellRuleForCall(shellCall("git worktree remove --no-force ./wt-no")), + ).toBeUndefined(); }); }); diff --git a/src/permission/auto-shell-policy.ts b/src/permission/auto-shell-policy.ts index 987e7c958..edf997708 100644 --- a/src/permission/auto-shell-policy.ts +++ b/src/permission/auto-shell-policy.ts @@ -351,7 +351,13 @@ export function isWorktreeForceFlag(arg: string): boolean { // "error: option `force' takes no value"), but the spelling still expresses // force intent, so the policy treats it as force rather than letting the // --flag=value path skip swallow it. - return arg === "-f" || arg === "--force" || arg.startsWith("--force="); + if (arg === "--force" || arg.startsWith("--force=")) return true; + // Short -f takes no value either (real git rejects `-f=` with + // "error: unknown switch `='" and glued `-f` with + // "error: unknown switch `'"); the same fail-closed reasoning applies. + // Any `-f`-prefixed token expresses force intent. `--no-force` negations are + // unaffected: they start with "--n", not "-f". + return arg.startsWith("-f"); } // True when the path is safe for unattended worktree add/remove: inside the diff --git a/src/permission/gate.test.ts b/src/permission/gate.test.ts index b206fa2c5..070239ae9 100644 --- a/src/permission/gate.test.ts +++ b/src/permission/gate.test.ts @@ -349,7 +349,31 @@ describe("grant-mismatch asks carry the guard reason as a notice (CL-6824)", () ); }); - test("uncontained destination names the approved locations", async () => { + test("short -f= is still force in the notice", async () => { + const { verdict, seen } = await askWithGrants( + "git worktree add -f=true ../sib-force-short-eq", + worktreeGrant, + ); + expect(verdict.allowed).toBe(false); + expect(seen).toHaveLength(1); + expect(seen[0]?.notice).toBe( + "A standing grant matches this command, but it uses --force, so it still needs approval.", + ); + }); + + test("glued -f is still force in the notice", async () => { + const { verdict, seen } = await askWithGrants( + "git worktree remove -ftrue ../sib-force-glued", + worktreeGrant, + ); + expect(verdict.allowed).toBe(false); + expect(seen).toHaveLength(1); + expect(seen[0]?.notice).toBe( + "A standing grant matches this command, but it uses --force, so it still needs approval.", + ); + }); + + test("uncontained add destination names the approved locations", async () => { // A direct child of tmpdir() is not a permitted sibling of sessionCwd // (only direct children of root/ are), so the restricted guard trips. const outside = join(tmpdir(), "gate-6824-outside"); @@ -364,6 +388,21 @@ describe("grant-mismatch asks carry the guard reason as a notice (CL-6824)", () ); }); + test("uncontained remove names the worktree, not a destination", async () => { + // `remove` names an existing worktree — there is no destination — so the + // notice drops the destination noun the `add` case uses. + const outside = join(tmpdir(), "gate-6824-outside-remove"); + const { verdict, seen } = await askWithGrants( + `git worktree remove ${outside}`, + worktreeGrant, + ); + expect(verdict.allowed).toBe(false); + expect(seen).toHaveLength(1); + expect(seen[0]?.notice).toBe( + "A standing grant matches this command, but the worktree is outside the approved locations, so it still needs approval.", + ); + }); + test("secret reference names the sensitive path", async () => { const { verdict, seen } = await askWithGrants("cat .env", catGrant); expect(verdict.allowed).toBe(false); @@ -414,4 +453,25 @@ describe("grant-mismatch asks carry the guard reason as a notice (CL-6824)", () expect(verdict.allowed).toBe(true); expect(seen).toHaveLength(0); }); + + test("a --no-force command still allows with no prompt and no notice", async () => { + const seen: PermissionRequest[] = []; + const gate = createPermissionGate({ + approvals: worktreeGrant, + interactive: true, + skipPermissions: false, + reactorGated: false, + cwd: sessionCwd, + rootsProvider: () => [], + requestApproval: async (request) => { + seen.push(request); + return { allow: false }; + }, + }); + const verdict = await gate.evaluate( + shellCall("git worktree add --no-force ../sib-noforce -b br-noforce"), + ); + expect(verdict.allowed).toBe(true); + expect(seen).toHaveLength(0); + }); }); diff --git a/src/permission/gate.ts b/src/permission/gate.ts index 8c593232f..2b0bbbec2 100644 --- a/src/permission/gate.ts +++ b/src/permission/gate.ts @@ -153,11 +153,14 @@ function segmentGuard( // Display-only refinement — the guard decision itself is unchanged. function worktreeMismatchKind( segment: string, -): "force" | "destination" | undefined { +): "force" | "destination" | "worktree" | undefined { const tokens = tokenize(segment); if (tokens[0] !== "git" || tokens[1] !== "worktree") return undefined; if (tokens[2] !== "add" && tokens[2] !== "remove") return undefined; - return tokens.slice(3).some(isWorktreeForceFlag) ? "force" : "destination"; + if (tokens.slice(3).some(isWorktreeForceFlag)) return "force"; + // `add` takes a destination for the new worktree; `remove` names an + // existing worktree, so only `add` gets the destination noun. + return tokens[2] === "remove" ? "worktree" : "destination"; } // Explains a grant mismatch: a standing grant covers the segment, but the @@ -177,6 +180,9 @@ function grantMismatchNotice( if (worktreeKind === "destination") { return "A standing grant matches this command, but the worktree destination is outside the approved locations, so it still needs approval."; } + if (worktreeKind === "worktree") { + return "A standing grant matches this command, but the worktree is outside the approved locations, so it still needs approval."; + } return "A standing grant matches this command, but it targets a path outside the workspace, so it still needs approval."; }