Skip to content

Commit 86da3fa

Browse files
committed
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 afd09cc commit 86da3fa

4 files changed

Lines changed: 99 additions & 5 deletions

File tree

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

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ describe("read-only git commands pass through the policy", () => {
9797
});
9898
});
9999

100-
describe("git worktree --force=<value> asks (CL-6824)", () => {
100+
describe("git worktree force spellings ask (CL-6824)", () => {
101101
// Control: a contained non-force add is not flagged at all.
102102
test("contained non-force add stays unflagged", () => {
103103
expect(
@@ -118,10 +118,32 @@ describe("git worktree --force=<value> asks (CL-6824)", () => {
118118
}
119119
});
120120

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+
121140
// The negation is not force: --no-force must not be caught by the prefix.
122141
test("--no-force stays unflagged", () => {
123142
expect(
124143
autoShellRuleForCall(shellCall("git worktree add --no-force ./wt-no")),
125144
).toBeUndefined();
145+
expect(
146+
autoShellRuleForCall(shellCall("git worktree remove --no-force ./wt-no")),
147+
).toBeUndefined();
126148
});
127149
});

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,13 @@ export function isWorktreeForceFlag(arg: string): boolean {
351351
// "error: option `force' takes no value"), but the spelling still expresses
352352
// force intent, so the policy treats it as force rather than letting the
353353
// --flag=value path skip swallow it.
354-
return arg === "-f" || arg === "--force" || arg.startsWith("--force=");
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");
355361
}
356362

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

‎src/permission/gate.test.ts‎

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,31 @@ describe("grant-mismatch asks carry the guard reason as a notice (CL-6824)", ()
349349
);
350350
});
351351

352-
test("uncontained destination names the approved locations", async () => {
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 () => {
353377
// A direct child of tmpdir() is not a permitted sibling of sessionCwd
354378
// (only direct children of root/ are), so the restricted guard trips.
355379
const outside = join(tmpdir(), "gate-6824-outside");
@@ -364,6 +388,21 @@ describe("grant-mismatch asks carry the guard reason as a notice (CL-6824)", ()
364388
);
365389
});
366390

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+
367406
test("secret reference names the sensitive path", async () => {
368407
const { verdict, seen } = await askWithGrants("cat .env", catGrant);
369408
expect(verdict.allowed).toBe(false);
@@ -414,4 +453,25 @@ describe("grant-mismatch asks carry the guard reason as a notice (CL-6824)", ()
414453
expect(verdict.allowed).toBe(true);
415454
expect(seen).toHaveLength(0);
416455
});
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+
});
417477
});

‎src/permission/gate.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,11 +153,14 @@ function segmentGuard(
153153
// Display-only refinement — the guard decision itself is unchanged.
154154
function worktreeMismatchKind(
155155
segment: string,
156-
): "force" | "destination" | undefined {
156+
): "force" | "destination" | "worktree" | undefined {
157157
const tokens = tokenize(segment);
158158
if (tokens[0] !== "git" || tokens[1] !== "worktree") return undefined;
159159
if (tokens[2] !== "add" && tokens[2] !== "remove") return undefined;
160-
return tokens.slice(3).some(isWorktreeForceFlag) ? "force" : "destination";
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";
161164
}
162165

163166
// Explains a grant mismatch: a standing grant covers the segment, but the
@@ -177,6 +180,9 @@ function grantMismatchNotice(
177180
if (worktreeKind === "destination") {
178181
return "A standing grant matches this command, but the worktree destination is outside the approved locations, so it still needs approval.";
179182
}
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+
}
180186
return "A standing grant matches this command, but it targets a path outside the workspace, so it still needs approval.";
181187
}
182188

0 commit comments

Comments
 (0)