Skip to content

Commit afd09cc

Browse files
committed
Explain grant-mismatch asks with a prompt notice
1 parent bf79cc0 commit afd09cc

4 files changed

Lines changed: 228 additions & 4 deletions

File tree

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,3 +96,32 @@ describe("read-only git commands pass through the policy", () => {
9696
).toBeUndefined();
9797
});
9898
});
99+
100+
describe("git worktree --force=<value> asks (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+
// The negation is not force: --no-force must not be caught by the prefix.
122+
test("--no-force stays unflagged", () => {
123+
expect(
124+
autoShellRuleForCall(shellCall("git worktree add --no-force ./wt-no")),
125+
).toBeUndefined();
126+
});
127+
});

src/permission/auto-shell-policy.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -346,8 +346,12 @@ 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+
return arg === "-f" || arg === "--force" || arg.startsWith("--force=");
351355
}
352356

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

src/permission/gate.test.ts

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,3 +286,132 @@ 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("uncontained destination names the approved locations", async () => {
353+
// A direct child of tmpdir() is not a permitted sibling of sessionCwd
354+
// (only direct children of root/ are), so the restricted guard trips.
355+
const outside = join(tmpdir(), "gate-6824-outside");
356+
const { verdict, seen } = await askWithGrants(
357+
`git worktree add ${outside}`,
358+
worktreeGrant,
359+
);
360+
expect(verdict.allowed).toBe(false);
361+
expect(seen).toHaveLength(1);
362+
expect(seen[0]?.notice).toBe(
363+
"A standing grant matches this command, but the worktree destination is outside the approved locations, so it still needs approval.",
364+
);
365+
});
366+
367+
test("secret reference names the sensitive path", async () => {
368+
const { verdict, seen } = await askWithGrants("cat .env", catGrant);
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 references a sensitive path, so it still needs approval.",
373+
);
374+
// The secret ask still strips grant scopes; the notice survives it.
375+
expect(seen[0]?.scopes).toEqual([]);
376+
});
377+
378+
test("restricted target names the workspace", async () => {
379+
const { verdict, seen } = await askWithGrants("cat /etc/passwd", catGrant);
380+
expect(verdict.allowed).toBe(false);
381+
expect(seen).toHaveLength(1);
382+
expect(seen[0]?.notice).toBe(
383+
"A standing grant matches this command, but it targets a path outside the workspace, so it still needs approval.",
384+
);
385+
});
386+
387+
test("an ask with no matching grant carries no notice", async () => {
388+
const { verdict, seen } = await askWithGrants(
389+
"git worktree add --force ../sib-nogrant",
390+
[],
391+
);
392+
expect(verdict.allowed).toBe(false);
393+
expect(seen).toHaveLength(1);
394+
expect(seen[0]?.notice).toBeUndefined();
395+
});
396+
397+
test("a covered command still allows with no prompt and no notice", async () => {
398+
const seen: PermissionRequest[] = [];
399+
const gate = createPermissionGate({
400+
approvals: worktreeGrant,
401+
interactive: true,
402+
skipPermissions: false,
403+
reactorGated: false,
404+
cwd: sessionCwd,
405+
rootsProvider: () => [],
406+
requestApproval: async (request) => {
407+
seen.push(request);
408+
return { allow: false };
409+
},
410+
});
411+
const verdict = await gate.evaluate(
412+
shellCall("git worktree add ../sib-plain -b br-plain"),
413+
);
414+
expect(verdict.allowed).toBe(true);
415+
expect(seen).toHaveLength(0);
416+
});
417+
});

src/permission/gate.ts

Lines changed: 64 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,39 @@ 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" | 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+
return tokens.slice(3).some(isWorktreeForceFlag) ? "force" : "destination";
161+
}
162+
163+
// Explains a grant mismatch: a standing grant covers the segment, but the
164+
// pre-grant guard still forced an ask. The wording names the guard's reason,
165+
// never the grant — matching semantics are untouched.
166+
function grantMismatchNotice(
167+
segment: string,
168+
kind: "secret" | "restricted",
169+
): string {
170+
if (kind === "secret") {
171+
return "A standing grant matches this command, but it references a sensitive path, so it still needs approval.";
172+
}
173+
const worktreeKind = worktreeMismatchKind(segment);
174+
if (worktreeKind === "force") {
175+
return "A standing grant matches this command, but it uses --force, so it still needs approval.";
176+
}
177+
if (worktreeKind === "destination") {
178+
return "A standing grant matches this command, but the worktree destination is outside the approved locations, so it still needs approval.";
179+
}
180+
return "A standing grant matches this command, but it targets a path outside the workspace, so it still needs approval.";
181+
}
182+
148183
// Relative path tokens in a shell command resolve against the process cwd of the
149184
// agent that issued the call — not the session cwd that built the gate. Absolute
150185
// paths pass through unchanged so createPathRestriction still judges them against
@@ -731,6 +766,7 @@ export function createPermissionGate(
731766

732767
let needsOperator = false;
733768
let anySecret = false;
769+
let mismatchNotice: string | undefined;
734770
for (const segment of segments) {
735771
// A secret-path reference or restricted target always requires the
736772
// operator, whether the segment would otherwise auto-allow or match
@@ -747,6 +783,22 @@ export function createPermissionGate(
747783
if (guard !== undefined) {
748784
if (guard.kind === "secret") anySecret = true;
749785
needsOperator = true;
786+
// A standing grant may still cover this segment even though the
787+
// pre-grant guard forces an ask — record why so the prompt can say
788+
// so. Matching semantics are untouched; this only annotates the ask.
789+
if (
790+
mismatchNotice === undefined &&
791+
(await evaluateApprovals({
792+
tool: request.tool,
793+
subject: segment,
794+
approvals,
795+
activeProviderModel,
796+
requestCwd: effectiveCwd,
797+
workspace: grantWorkspace(),
798+
}))
799+
) {
800+
mismatchNotice = grantMismatchNotice(segment, guard.kind);
801+
}
750802
continue;
751803
}
752804
if (
@@ -788,9 +840,19 @@ export function createPermissionGate(
788840

789841
// Secret-path shell must never mint a stored grant — even an exact match
790842
// would be misleading because future secret-path shell always re-asks.
843+
// A grant mismatch carries the guard's reason on the prompt so the
844+
// operator sees why the standing grant did not apply.
791845
const requestForOperator = anySecret
792-
? { ...request, scopes: [] }
793-
: request;
846+
? {
847+
...request,
848+
scopes: [],
849+
...(mismatchNotice !== undefined
850+
? { notice: mismatchNotice }
851+
: null),
852+
}
853+
: mismatchNotice !== undefined
854+
? { ...request, notice: mismatchNotice }
855+
: request;
794856
return {
795857
kind: "ask",
796858
request: requestForOperator,

0 commit comments

Comments
 (0)