Skip to content

Commit e42d1d2

Browse files
Explain grant-mismatch asks with a prompt notice (#960)
* Explain grant-mismatch asks with a prompt notice * Treat short -f spellings as force and fix remove notice wording Short -f takes no value: real git rejects -f=<value> and glued -f<val> the way it rejects --force=<value>, 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.
1 parent 910427f commit e42d1d2

4 files changed

Lines changed: 322 additions & 4 deletions

File tree

‎src/permission/auto-shell-policy.test.ts‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,3 +96,54 @@ describe("read-only git commands pass through the policy", () => {
9696
).toBeUndefined();
9797
});
9898
});
99+
100+
describe("git worktree force spellings ask (CL-6824)", () => {
101+
// Control: a contained non-force add is not flagged at all.
102+
test("contained non-force add stays unflagged", () => {
103+
expect(
104+
autoShellRuleForCall(shellCall("git worktree add ./wt-plain")),
105+
).toBeUndefined();
106+
});
107+
108+
// Git has no --force=<value> form — real git dies with
109+
// "error: option `force' takes no value" (exit 129) — but the spelling
110+
// still expresses force intent, so the policy asks rather than letting the
111+
// --flag=value skip swallow it the way the old exact-match check did.
112+
test("--force=<value> spellings hit the worktree ask rule", () => {
113+
for (const flag of ["--force=true", "--force=1", "--force="]) {
114+
expect(
115+
autoShellRuleForCall(shellCall(`git worktree add ${flag} ./wt-eq`))
116+
?.name,
117+
).toBe("git-worktree");
118+
}
119+
});
120+
121+
// Short -f takes no value either — real git dies with
122+
// "error: unknown switch `='" for `-f=<value>` and
123+
// "error: unknown switch `<char>'" for glued `-f<val>` (exit 129 both) —
124+
// but the spellings still express force intent, so the policy asks rather
125+
// than letting the generic-flag skip swallow them.
126+
test("-f=<value> and glued -f<val> spellings hit the worktree ask rule", () => {
127+
for (const flag of ["-f=true", "-f=", "-ftrue", "-ff"]) {
128+
expect(
129+
autoShellRuleForCall(shellCall(`git worktree add ${flag} ./wt-short`))
130+
?.name,
131+
).toBe("git-worktree");
132+
expect(
133+
autoShellRuleForCall(
134+
shellCall(`git worktree remove ${flag} ./wt-short`),
135+
)?.name,
136+
).toBe("git-worktree");
137+
}
138+
});
139+
140+
// The negation is not force: --no-force must not be caught by the prefix.
141+
test("--no-force stays unflagged", () => {
142+
expect(
143+
autoShellRuleForCall(shellCall("git worktree add --no-force ./wt-no")),
144+
).toBeUndefined();
145+
expect(
146+
autoShellRuleForCall(shellCall("git worktree remove --no-force ./wt-no")),
147+
).toBeUndefined();
148+
});
149+
});

‎src/permission/auto-shell-policy.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -346,8 +346,18 @@ const WORKTREE_PRUNE_FLAGS = new Set(["-n", "--dry-run", "-v", "--verbose"]);
346346
// Flags that take a following value on `git worktree add` (branch name, lock reason).
347347
const WORKTREE_ADD_VALUE_FLAGS = new Set(["-b", "-B", "--reason"]);
348348

349-
function isWorktreeForceFlag(arg: string): boolean {
350-
return arg === "-f" || arg === "--force";
349+
export function isWorktreeForceFlag(arg: string): boolean {
350+
// Git's --force takes no value (real git rejects --force=<value> with
351+
// "error: option `force' takes no value"), but the spelling still expresses
352+
// force intent, so the policy treats it as force rather than letting the
353+
// --flag=value path skip swallow it.
354+
if (arg === "--force" || arg.startsWith("--force=")) return true;
355+
// Short -f takes no value either (real git rejects `-f=<value>` with
356+
// "error: unknown switch `='" and glued `-f<val>` with
357+
// "error: unknown switch `<char>'"); the same fail-closed reasoning applies.
358+
// Any `-f`-prefixed token expresses force intent. `--no-force` negations are
359+
// unaffected: they start with "--n", not "-f".
360+
return arg.startsWith("-f");
351361
}
352362

353363
// True when the path is safe for unattended worktree add/remove: inside the

‎src/permission/gate.test.ts‎

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,3 +286,192 @@ describe("standing grant covers a later git worktree command (CL-5638)", () => {
286286
expect(prompts).toBe(1);
287287
});
288288
});
289+
290+
// CL-6824: when a standing grant covers a command but a pre-grant guard still
291+
// forces an ask, the prompt carries PermissionRequest.notice naming the
292+
// guard's reason. Matching semantics are unchanged — every case below still
293+
// asks (and stays deniable); only the prompt gains the why.
294+
describe("grant-mismatch asks carry the guard reason as a notice (CL-6824)", () => {
295+
const root = mkdtempSync(join(tmpdir(), "gate-mismatch-notice-"));
296+
const sessionCwd = join(root, "main");
297+
const git = (args: string[], cwd: string) =>
298+
execFileSync("git", args, { cwd, stdio: "ignore" });
299+
mkdirSync(sessionCwd);
300+
initTemporaryGitRepo(sessionCwd, { initArgs: ["-q"] });
301+
writeFileSync(join(sessionCwd, "seed.txt"), "seed\n");
302+
git(["add", "."], sessionCwd);
303+
git(["commit", "-qm", "seed"], sessionCwd);
304+
305+
async function askWithGrants(command: string, approvals: Approval[]) {
306+
const seen: PermissionRequest[] = [];
307+
const gate = createPermissionGate({
308+
approvals,
309+
interactive: true,
310+
skipPermissions: false,
311+
reactorGated: false,
312+
cwd: sessionCwd,
313+
rootsProvider: () => [],
314+
requestApproval: async (request) => {
315+
seen.push(request);
316+
return { allow: false };
317+
},
318+
});
319+
const verdict = await gate.evaluate(shellCall(command));
320+
return { verdict, seen };
321+
}
322+
323+
const worktreeGrant: Approval[] = [
324+
{ tool: "run_shell", pattern: "git worktree *" },
325+
];
326+
const catGrant: Approval[] = [{ tool: "run_shell", pattern: "cat *" }];
327+
328+
test("force worktree names --force in the notice", async () => {
329+
const { verdict, seen } = await askWithGrants(
330+
"git worktree add --force ../sib-force",
331+
worktreeGrant,
332+
);
333+
expect(verdict.allowed).toBe(false);
334+
expect(seen).toHaveLength(1);
335+
expect(seen[0]?.notice).toBe(
336+
"A standing grant matches this command, but it uses --force, so it still needs approval.",
337+
);
338+
});
339+
340+
test("--force=<value> is still force in the notice", async () => {
341+
const { verdict, seen } = await askWithGrants(
342+
"git worktree add --force=true ../sib-force-eq",
343+
worktreeGrant,
344+
);
345+
expect(verdict.allowed).toBe(false);
346+
expect(seen).toHaveLength(1);
347+
expect(seen[0]?.notice).toBe(
348+
"A standing grant matches this command, but it uses --force, so it still needs approval.",
349+
);
350+
});
351+
352+
test("short -f=<value> is still force in the notice", async () => {
353+
const { verdict, seen } = await askWithGrants(
354+
"git worktree add -f=true ../sib-force-short-eq",
355+
worktreeGrant,
356+
);
357+
expect(verdict.allowed).toBe(false);
358+
expect(seen).toHaveLength(1);
359+
expect(seen[0]?.notice).toBe(
360+
"A standing grant matches this command, but it uses --force, so it still needs approval.",
361+
);
362+
});
363+
364+
test("glued -f<val> is still force in the notice", async () => {
365+
const { verdict, seen } = await askWithGrants(
366+
"git worktree remove -ftrue ../sib-force-glued",
367+
worktreeGrant,
368+
);
369+
expect(verdict.allowed).toBe(false);
370+
expect(seen).toHaveLength(1);
371+
expect(seen[0]?.notice).toBe(
372+
"A standing grant matches this command, but it uses --force, so it still needs approval.",
373+
);
374+
});
375+
376+
test("uncontained add destination names the approved locations", async () => {
377+
// A direct child of tmpdir() is not a permitted sibling of sessionCwd
378+
// (only direct children of root/ are), so the restricted guard trips.
379+
const outside = join(tmpdir(), "gate-6824-outside");
380+
const { verdict, seen } = await askWithGrants(
381+
`git worktree add ${outside}`,
382+
worktreeGrant,
383+
);
384+
expect(verdict.allowed).toBe(false);
385+
expect(seen).toHaveLength(1);
386+
expect(seen[0]?.notice).toBe(
387+
"A standing grant matches this command, but the worktree destination is outside the approved locations, so it still needs approval.",
388+
);
389+
});
390+
391+
test("uncontained remove names the worktree, not a destination", async () => {
392+
// `remove` names an existing worktree — there is no destination — so the
393+
// notice drops the destination noun the `add` case uses.
394+
const outside = join(tmpdir(), "gate-6824-outside-remove");
395+
const { verdict, seen } = await askWithGrants(
396+
`git worktree remove ${outside}`,
397+
worktreeGrant,
398+
);
399+
expect(verdict.allowed).toBe(false);
400+
expect(seen).toHaveLength(1);
401+
expect(seen[0]?.notice).toBe(
402+
"A standing grant matches this command, but the worktree is outside the approved locations, so it still needs approval.",
403+
);
404+
});
405+
406+
test("secret reference names the sensitive path", async () => {
407+
const { verdict, seen } = await askWithGrants("cat .env", catGrant);
408+
expect(verdict.allowed).toBe(false);
409+
expect(seen).toHaveLength(1);
410+
expect(seen[0]?.notice).toBe(
411+
"A standing grant matches this command, but it references a sensitive path, so it still needs approval.",
412+
);
413+
// The secret ask still strips grant scopes; the notice survives it.
414+
expect(seen[0]?.scopes).toEqual([]);
415+
});
416+
417+
test("restricted target names the workspace", async () => {
418+
const { verdict, seen } = await askWithGrants("cat /etc/passwd", catGrant);
419+
expect(verdict.allowed).toBe(false);
420+
expect(seen).toHaveLength(1);
421+
expect(seen[0]?.notice).toBe(
422+
"A standing grant matches this command, but it targets a path outside the workspace, so it still needs approval.",
423+
);
424+
});
425+
426+
test("an ask with no matching grant carries no notice", async () => {
427+
const { verdict, seen } = await askWithGrants(
428+
"git worktree add --force ../sib-nogrant",
429+
[],
430+
);
431+
expect(verdict.allowed).toBe(false);
432+
expect(seen).toHaveLength(1);
433+
expect(seen[0]?.notice).toBeUndefined();
434+
});
435+
436+
test("a covered command still allows with no prompt and no notice", async () => {
437+
const seen: PermissionRequest[] = [];
438+
const gate = createPermissionGate({
439+
approvals: worktreeGrant,
440+
interactive: true,
441+
skipPermissions: false,
442+
reactorGated: false,
443+
cwd: sessionCwd,
444+
rootsProvider: () => [],
445+
requestApproval: async (request) => {
446+
seen.push(request);
447+
return { allow: false };
448+
},
449+
});
450+
const verdict = await gate.evaluate(
451+
shellCall("git worktree add ../sib-plain -b br-plain"),
452+
);
453+
expect(verdict.allowed).toBe(true);
454+
expect(seen).toHaveLength(0);
455+
});
456+
457+
test("a --no-force command still allows with no prompt and no notice", async () => {
458+
const seen: PermissionRequest[] = [];
459+
const gate = createPermissionGate({
460+
approvals: worktreeGrant,
461+
interactive: true,
462+
skipPermissions: false,
463+
reactorGated: false,
464+
cwd: sessionCwd,
465+
rootsProvider: () => [],
466+
requestApproval: async (request) => {
467+
seen.push(request);
468+
return { allow: false };
469+
},
470+
});
471+
const verdict = await gate.evaluate(
472+
shellCall("git worktree add --no-force ../sib-noforce -b br-noforce"),
473+
);
474+
expect(verdict.allowed).toBe(true);
475+
expect(seen).toHaveLength(0);
476+
});
477+
});

‎src/permission/gate.ts‎

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
import {
1919
autoShellRuleForCall,
2020
safeWorktreeCommand,
21+
isWorktreeForceFlag,
2122
} from "./auto-shell-policy.js";
2223
import { commandReferencesSensitivePath } from "../plugins/secret-guard-plugin.js";
2324
import {
@@ -34,6 +35,7 @@ import {
3435
import {
3536
splitChainedCommand,
3637
isShellCommentOnly,
38+
tokenize,
3739
stripCommentLines,
3840
} from "./command.js";
3941
import {
@@ -145,6 +147,45 @@ function segmentGuard(
145147
return undefined;
146148
}
147149

150+
// Classifies a guarded `git worktree add/remove` segment so a grant-mismatch
151+
// notice can name the operative reason: a force flag, or a destination the
152+
// containment authority did not approve. Returns undefined for anything else.
153+
// Display-only refinement — the guard decision itself is unchanged.
154+
function worktreeMismatchKind(
155+
segment: string,
156+
): "force" | "destination" | "worktree" | undefined {
157+
const tokens = tokenize(segment);
158+
if (tokens[0] !== "git" || tokens[1] !== "worktree") return undefined;
159+
if (tokens[2] !== "add" && tokens[2] !== "remove") return undefined;
160+
if (tokens.slice(3).some(isWorktreeForceFlag)) return "force";
161+
// `add` takes a destination for the new worktree; `remove` names an
162+
// existing worktree, so only `add` gets the destination noun.
163+
return tokens[2] === "remove" ? "worktree" : "destination";
164+
}
165+
166+
// Explains a grant mismatch: a standing grant covers the segment, but the
167+
// pre-grant guard still forced an ask. The wording names the guard's reason,
168+
// never the grant — matching semantics are untouched.
169+
function grantMismatchNotice(
170+
segment: string,
171+
kind: "secret" | "restricted",
172+
): string {
173+
if (kind === "secret") {
174+
return "A standing grant matches this command, but it references a sensitive path, so it still needs approval.";
175+
}
176+
const worktreeKind = worktreeMismatchKind(segment);
177+
if (worktreeKind === "force") {
178+
return "A standing grant matches this command, but it uses --force, so it still needs approval.";
179+
}
180+
if (worktreeKind === "destination") {
181+
return "A standing grant matches this command, but the worktree destination is outside the approved locations, so it still needs approval.";
182+
}
183+
if (worktreeKind === "worktree") {
184+
return "A standing grant matches this command, but the worktree is outside the approved locations, so it still needs approval.";
185+
}
186+
return "A standing grant matches this command, but it targets a path outside the workspace, so it still needs approval.";
187+
}
188+
148189
// Relative path tokens in a shell command resolve against the process cwd of the
149190
// agent that issued the call — not the session cwd that built the gate. Absolute
150191
// paths pass through unchanged so createPathRestriction still judges them against
@@ -731,6 +772,7 @@ export function createPermissionGate(
731772

732773
let needsOperator = false;
733774
let anySecret = false;
775+
let mismatchNotice: string | undefined;
734776
for (const segment of segments) {
735777
// A secret-path reference or restricted target always requires the
736778
// operator, whether the segment would otherwise auto-allow or match
@@ -747,6 +789,22 @@ export function createPermissionGate(
747789
if (guard !== undefined) {
748790
if (guard.kind === "secret") anySecret = true;
749791
needsOperator = true;
792+
// A standing grant may still cover this segment even though the
793+
// pre-grant guard forces an ask — record why so the prompt can say
794+
// so. Matching semantics are untouched; this only annotates the ask.
795+
if (
796+
mismatchNotice === undefined &&
797+
(await evaluateApprovals({
798+
tool: request.tool,
799+
subject: segment,
800+
approvals,
801+
activeProviderModel,
802+
requestCwd: effectiveCwd,
803+
workspace: grantWorkspace(),
804+
}))
805+
) {
806+
mismatchNotice = grantMismatchNotice(segment, guard.kind);
807+
}
750808
continue;
751809
}
752810
if (
@@ -788,9 +846,19 @@ export function createPermissionGate(
788846

789847
// Secret-path shell must never mint a stored grant — even an exact match
790848
// would be misleading because future secret-path shell always re-asks.
849+
// A grant mismatch carries the guard's reason on the prompt so the
850+
// operator sees why the standing grant did not apply.
791851
const requestForOperator = anySecret
792-
? { ...request, scopes: [] }
793-
: request;
852+
? {
853+
...request,
854+
scopes: [],
855+
...(mismatchNotice !== undefined
856+
? { notice: mismatchNotice }
857+
: null),
858+
}
859+
: mismatchNotice !== undefined
860+
? { ...request, notice: mismatchNotice }
861+
: request;
794862
return {
795863
kind: "ask",
796864
request: requestForOperator,

0 commit comments

Comments
 (0)