Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/permission/auto-shell-policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,12 @@ function worktreePathArgs(
// Contained non-force add/remove and ordinary prune/list auto-allow so dispatch
// can create sibling worktrees without a human click; force flags, uncontained
// paths, and uncommon subcommands still ask.
function safeWorktreeCommand(
// Exported for gate.ts's pre-grant restricted-path guard: a contained or
// permitted-sibling `git worktree add/remove` destination must not force an
// operator ask ahead of grant matching (CL-5638) the same way it already
// skips the auto-mode ask below — one authority for "is this worktree
// destination safe," used by both auto mode and the standing-grant guard.
export function safeWorktreeCommand(
command: string,
isRestricted: (path: string, isWrite: boolean) => boolean,
cwd: string,
Expand Down
38 changes: 38 additions & 0 deletions src/permission/gate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,3 +219,41 @@ describe("director writePaths authz on evaluate", () => {
expect(verdict.allowed).toBe(true);
});
});

// CL-5638: an Always-allow grant minted for `git worktree *` must cover a later
// worktree command whose destination is a sibling directory the operator has
// already implicitly approved under that pattern, without a second prompt.
describe("standing grant covers a later git worktree command (CL-5638)", () => {
const root = mkdtempSync(join(tmpdir(), "gate-worktree-grant-"));
const sessionCwd = join(root, "main");
const git = (args: string[], cwd: string) => execFileSync("git", args, { cwd, stdio: "ignore" });
mkdirSync(sessionCwd);
git(["init", "-q"], sessionCwd);
git(["config", "user.email", "t@example.com"], sessionCwd);
git(["config", "user.name", "t"], sessionCwd);
writeFileSync(join(sessionCwd, "seed.txt"), "seed\n");
git(["add", "."], sessionCwd);
git(["commit", "-qm", "seed"], sessionCwd);

test("second sibling worktree add is not re-prompted after Always-allow", async () => {
let prompts = 0;
const gate = createPermissionGate({
approvals: [],
interactive: true,
skipPermissions: false,
cwd: sessionCwd,
requestApproval: async () => {
prompts += 1;
return { allow: true, persist: { id: "always", label: "Always allow", pattern: "git worktree *", grant: "project" } };
},
});

const first = await gate.evaluate(shellCall("git worktree add ../sibling-a -b br-a"));
expect(first.allowed).toBe(true);
expect(prompts).toBe(1);

const second = await gate.evaluate(shellCall("git worktree add ../sibling-b -b br-b"));
expect(second.allowed).toBe(true);
expect(prompts).toBe(1);
});
});
34 changes: 28 additions & 6 deletions src/permission/gate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
commandTargetsRestricted,
MEGA_CHAIN_SEGMENT_THRESHOLD,
} from "./classify.js";
import { autoShellRuleForCall } from "./auto-shell-policy.js";
import { autoShellRuleForCall, safeWorktreeCommand } 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";
Expand Down Expand Up @@ -84,8 +84,28 @@ function hasExactFullCommandGrant(
// preGrantGuardReason (which only needs to know whether one tripped).
type SegmentGuard = { kind: "secret" | "restricted" };

function segmentGuard(segment: string, isRestricted: (path: string, isWrite: boolean) => boolean): SegmentGuard | undefined {
// `cwd`/`rootsProvider`, when both supplied, let a contained or
// permitted-sibling `git worktree add/remove` destination (see
// safeWorktreeCommand) skip the generic restricted-path scan below — the
// same exemption auto mode already applies (autoShellRuleForCall) — so a
// standing `git worktree *` grant gets a chance to match instead of the
// destination forcing an ask on every call regardless of any grant. Omitted
// (as from call sites with no cwd on hand) simply skips the exemption and
// falls back to today's behavior.
function segmentGuard(
segment: string,
isRestricted: (path: string, isWrite: boolean) => boolean,
cwd?: string,
rootsProvider?: RootsProvider,
): SegmentGuard | undefined {
if (commandReferencesSensitivePath(segment) !== undefined) return { kind: "secret" };
if (
cwd !== undefined &&
rootsProvider !== undefined &&
safeWorktreeCommand(segment, isRestricted, cwd, rootsProvider) === true
) {
return undefined;
}
if (commandTargetsRestricted(segment, isRestricted)) return { kind: "restricted" };
return undefined;
}
Expand Down Expand Up @@ -119,6 +139,7 @@ function bindRestrictedToProcessCwd(
export function preGrantGuardReason(
request: PermissionRequest,
isRestricted: (path: string, isWrite: boolean) => boolean,
rootsProvider?: RootsProvider,
): string | undefined {
if (request.tool !== "run_shell") return undefined;
const fullCommand = request.subject;
Expand All @@ -131,7 +152,7 @@ export function preGrantGuardReason(
const restricted =
request.cwd !== undefined ? bindRestrictedToProcessCwd(isRestricted, request.cwd) : isRestricted;
for (const segment of segments) {
const guard = segmentGuard(segment, restricted);
const guard = segmentGuard(segment, restricted, request.cwd, rootsProvider);
if (guard !== undefined) {
return guard.kind === "secret"
? `${segment} references a sensitive path`
Expand All @@ -154,12 +175,13 @@ export function isRequestCoveredByGrant(
activeProviderModel: string | undefined,
isRestricted: (path: string, isWrite: boolean) => boolean,
workspace: GrantWorkspace,
rootsProvider?: RootsProvider,
): boolean {
if (!grantScopeMatches(approval, request.tool, activeProviderModel, request.cwd, workspace)) return false;
if (request.tool !== "run_shell") {
return matchesPattern(request.subject, approval.pattern);
}
if (preGrantGuardReason(request, isRestricted) !== undefined) return false;
if (preGrantGuardReason(request, isRestricted, rootsProvider) !== undefined) return false;
const segments = splitChainedCommand(request.subject).filter((s) => !isShellCommentOnly(s));
if (segments.length === 0) return false;
if (segments.length > 1) {
Expand Down Expand Up @@ -334,7 +356,7 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
persist?.(approval, grant);
}
options.onGrant?.(approval, (request) =>
isRequestCoveredByGrant(request, approval, activeProviderModel, isRestricted, grantWorkspace()),
isRequestCoveredByGrant(request, approval, activeProviderModel, isRestricted, grantWorkspace(), rootsProvider),
);
};

Expand Down Expand Up @@ -473,7 +495,7 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
// replay for a guarded one just because the pattern also matches it.
// segmentGuard is the same guard preGrantGuardReason applies before
// isRequestCoveredByGrant lets a queued request skip the prompt.
const guard = segmentGuard(segment, isRestrictedHere);
const guard = segmentGuard(segment, isRestrictedHere, effectiveCwd, rootsProvider);
if (guard !== undefined) {
if (guard.kind === "secret") anySecret = true;
needsOperator = true;
Expand Down
Loading