Skip to content

Commit 662a3db

Browse files
Merge pull request #378 from corbitsdev/cl-5637-auto-allow-contained-git-worktree-ops-in-auto-mode
Auto-allow contained git worktree ops in auto mode
2 parents fbc2c69 + 4a8c8f7 commit 662a3db

5 files changed

Lines changed: 253 additions & 35 deletions

File tree

docs/ARCHITECTURE.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ In TUI chat mode there is no completion gate — the session stays open across t
106106

107107
Two directors, selected by role:
108108

109-
- **ChatDirector** (interactive, `src/agent/director.ts`) — Extends `DefaultDirector` with task list tracking, workflow nudges, LSP auto-activation, multi-turn chat semantics, and an optional **goal governor** (session-scoped auto-continue until every acceptance criterion is done). It never terminates the session: operator declines are surfaced as replies and the reactor stays alive for the next message. Auto mode is toggled by CLI flags (`--auto` / `--no-auto`); there is currently no in-session key to toggle it (default on; constrained envelope — workspace writes and unconstrained shell auto-allow; installs, recursive rm, worktree changes, sensitive-path and opaque-wrapper shell still ask; shell file-mutation denied). It is not a separate edit/plan mode.
109+
- **ChatDirector** (interactive, `src/agent/director.ts`) — Extends `DefaultDirector` with task list tracking, workflow nudges, LSP auto-activation, multi-turn chat semantics, and an optional **goal governor** (session-scoped auto-continue until every acceptance criterion is done). It never terminates the session: operator declines are surfaced as replies and the reactor stays alive for the next message. Auto mode is toggled by CLI flags (`--auto` / `--no-auto`); there is currently no in-session key to toggle it (default on; constrained envelope — workspace writes and unconstrained shell auto-allow; installs, recursive rm, force/uncontained worktree changes, sensitive-path and opaque-wrapper shell still ask; contained non-force `git worktree add`/`remove`/`prune` and `list` auto-allow; shell file-mutation denied). It is not a separate edit/plan mode.
110110
- **SubAgentDirector** (delegated work, `src/subagent/index.ts`) — Drives a dispatched worker until a turn arrives with no tool calls, then replies with the final assistant text and ends the run. A tool-less completion with **zero tool calls in the entire run** is returned as a **never-acted** salvage report (not a successful implement); explore/read-only workers that used tools then replied with findings remain normal completes. Hard stops also fire after 2 consecutive identical tool-call fingerprints (**no-progress**), on progressive re-read pressure (**thrash** — the same path re-read past a limit amid enough tool volume, tracked by `src/subagent/thrash.ts`), or after the leaf turn budget (**turn-budget**, default 30, overridable via `task(maxTurns)`, agent profile `maxTurns`, or `settings.subagentMaxTurns`, capped at 100), each returning a structured salvage report (reason, partial findings, blockers) so a thrashing child cannot burn tokens indefinitely. A fourth hard stop, **repetition**, is detected outside the director entirely: `runSubAgent`'s stream sink watches the streamed text of the in-flight cycle for degenerate token loops (`src/subagent/repetition.ts`) — whitespace-collapsed raw text, a smallest-period KMP check over the probe tail, default window >= 16 chars repeated >= 8 times, evaluated every 256 streamed chars — and on a hit aborts the run controller mid-cycle, returning a `repetition` salvage report that leads with the looped window and warns the parent against re-dispatching the identical brief. Because directors only see completed turns, this is the only stop that can catch a loop inside a single turn that never finishes. A one-shot **report-forced** signal fires a few turns before the cap while the leaf is still tooling — it is not a stop: the director injects a wrap-up nudge and lets the leaf finish on its own, so turn-budget stays reachable for a leaf still making progress. Operator/parent cancel after any progress likewise returns a **cancelled** salvage report (partial findings + tool activity) instead of a bare cancel string; cancel before progress still surfaces as cancelled-by-operator. Optional `task(tier=)` (`fast` | `standard` | `clever`) overrides profile inference, profile tier, and the parent provider for that spawn only, and fails closed when the tier is unconfigured. The parent `task` tool keeps a session-scoped brief-dispatch ledger (`src/subagent/brief-dispatch.ts`): fingerprints cover prompt + agent + intent + success_criteria + do_not (not maxTurns/description/tier). After thrash / no-progress / repetition / never-acted salvage, an identical re-dispatch is hard-blocked for the rest of the parent chat; change at least one fingerprint field to force a re-run. Turn-budget salvage still invites a higher maxTurns for a few same-brief retries without a successful complete, then flips the parent hint to stop and change approach (soft — further identical dispatches are still admitted). A successful complete resets the same-brief retry budget.
111111

112112

@@ -274,7 +274,7 @@ tool call
274274

275275
- **classify** — Read-only tools (`read_file`, `search_files`, `grep`, `list_dir`) are tier `allow`; everything else is tier `ask`. Builds approval requests: shell yields one request for the full command the model asked to run (security still splits under the gate); file tools keyed on the target path; other tools keyed on tool name.
276276
- **command** — Splits chained commands for security classification and derives command-shape approval scopes. Multi-segment chains only offer an exact-command persist pattern (a prefix like `npm *` must not cover `npm i && rm -rf /` later).
277-
- **auto-shell-policy** — Constrains `run_shell` even when auto mode would otherwise rubber-stamp it. Before matching, `expandShellSubjects` peels `bash`/`sh`/`zsh -c`, `xargs` utility tails, and transparent prefixes (`env`, `nice`, `timeout`, …) so rules see the real payload; an unparseable wrapper (variable expansion or command substitution) sets an opaque flag that forces `ask`. Effects: `deny` blocks outright (file mutations through ad-hoc tooling — output redirection, `tee`, `sed -i`/`perl -i`, interpreter inline programs or heredocs — which must instead go through `write_file`/`edit_file`); `ask` declines to auto-allow and falls through to the operator prompt (recursive `rm`, dependency installs and remote runners: npm/yarn/pnpm/bun, pip, cargo, go, brew, npx/bunx, …, git worktree add/remove/prune, shell that references a sensitive path such as `.env` or a private key, and opaque wrappers). Deny beats ask when multiple subjects match. Quoted spans are stripped before pattern matching so a quoted `>` or install word in an argument is not flagged, and program names are matched only in command position. Adding a table category is a one-line rule append in `AUTO_SHELL_RULES`.
277+
- **auto-shell-policy** — Constrains `run_shell` even when auto mode would otherwise rubber-stamp it. Before matching, `expandShellSubjects` peels `bash`/`sh`/`zsh -c`, `xargs` utility tails, and transparent prefixes (`env`, `nice`, `timeout`, …) so rules see the real payload; an unparseable wrapper (variable expansion or command substitution) sets an opaque flag that forces `ask`. Effects: `deny` blocks outright (file mutations through ad-hoc tooling — output redirection, `tee`, `sed -i`/`perl -i`, interpreter inline programs or heredocs — which must instead go through `write_file`/`edit_file`); `ask` declines to auto-allow and falls through to the operator prompt (recursive `rm`, dependency installs and remote runners: npm/yarn/pnpm/bun, pip, cargo, go, brew, npx/bunx, …, force or uncontained `git worktree` ops, shell that references a sensitive path such as `.env` or a private key, and opaque wrappers). Contained non-force `git worktree add`/`remove`/`prune` and read-only `list` auto-allow (sibling destinations like `../corbits-dispatch-wts/…` included; absolute outside, `~`, globs, and credential basenames still ask). Deny beats ask when multiple subjects match. Quoted spans are stripped before pattern matching so a quoted `>` or install word in an argument is not flagged, and program names are matched only in command position. Adding a table category is a one-line rule append in `AUTO_SHELL_RULES`.
278278
- **gate** — Evaluates a call: `skipPermissions` allows everything; `allow`-tier passes; for `ask`-tier, checks persisted approvals, otherwise requests operator approval. Shell security classifies each chain segment (`||` / `&&` / `|` / `;` / newlines), but the operator is prompted once for the full command block — any unapproved segment fails the whole block, and execution always runs the unsplit original. Safe pipeline tails and pure shell no-ops (`true` / `false` / `:` and bare control-flow keywords stranded by chain-splitting) skip without a prompt. In a non-interactive run an unresolved `ask` becomes a denial. In auto mode: non-shell built-ins in `AUTO_ALLOWED_TOOLS` (writes/edits/deletes, `manage_tasks`, `task`, …) auto-allow when not path-restricted; for `run_shell` the gate consults the auto-shell policy — a `deny` rule fails the call, an `ask` rule skips the auto-allow shortcut and proceeds to the normal approval flow, and anything unmatched is auto-allowed. Paths outside the workspace and writes under the session state root (`~/.corbits/projects/...` and legacy `.agent-state`) still ask. Mutating MCP and unknown built-ins are not blanket-allowed. Newly granted scopes are appended in memory and persisted.
279279

280280
- **matcher** — Approval pattern matching via `@intx/authz` `matchPattern` (`*` wildcards). Exact-command grants store a backslash before each metacharacter; those patterns match by equality after unescape (the package has no escape syntax).

src/permission/auto-shell-policy.ts

Lines changed: 119 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import { commandHasRecursiveRm, expandShellSubjects } from "../shell/run-shell-a
33
import { commandReferencesSensitivePath } from "../plugins/secret-guard-plugin.js";
44
import { commandHasUnboundedDirectoryListing, commandTargetsRestricted } from "./classify.js";
55
import { splitChainedCommand, tokenize } from "./command.js";
6+
import { isPermittedSiblingWorktreePath } from "./path-restriction.js";
7+
import type { RootsProvider } from "./worktree-roots.js";
68

79
// Auto-mode shell policy: a flat table of rules that constrain what a run_shell
810
// command may do when auto mode is on. Auto mode otherwise rubber-stamps every
@@ -232,7 +234,7 @@ const WORKTREE_ASK_RULE: AutoShellRule = {
232234
name: "git-worktree",
233235
effect: "ask",
234236
reason:
235-
"Git worktree add, remove, and prune change the workspace boundary and need explicit operator approval in auto mode. Only read-only git worktree list can run unattended.",
237+
"This git worktree command uses a force flag, an uncontained path, or a subcommand that still needs explicit operator approval in auto mode. Contained non-force add/remove/prune and read-only list can run unattended.",
236238
patterns: [],
237239
};
238240

@@ -283,8 +285,88 @@ const UNBOUNDED_LISTING_ASK_RULE: AutoShellRule = {
283285
};
284286

285287
const WORKTREE_LIST_FLAGS = new Set(["--porcelain", "-v", "--verbose", "-z"]);
288+
const WORKTREE_PRUNE_FLAGS = new Set(["-n", "--dry-run", "-v", "--verbose"]);
289+
// Flags that take a following value on `git worktree add` (branch name, lock reason).
290+
const WORKTREE_ADD_VALUE_FLAGS = new Set(["-b", "-B", "--reason"]);
286291

287-
function safeWorktreeCommand(command: string): boolean | undefined {
292+
function isWorktreeForceFlag(arg: string): boolean {
293+
return arg === "-f" || arg === "--force";
294+
}
295+
296+
// True when the path is safe for unattended worktree add/remove: inside the
297+
// session workspace (the unified containment authority's normal notion), or a
298+
// not-yet-registered sibling location the same authority's narrow
299+
// isPermittedSiblingWorktreePath rule allows (path-restriction.ts). No
300+
// bespoke denylist or depth counter here — everything routes through that one
301+
// authority so a path is never judged "contained" under a looser or stricter
302+
// rule than the one gate.ts uses to decide restriction.
303+
function isContainedWorktreePath(
304+
pathArg: string,
305+
isRestricted: (path: string, isWrite: boolean) => boolean,
306+
cwd: string,
307+
rootsProvider: RootsProvider,
308+
): boolean {
309+
if (!pathArg) return false;
310+
// Shell-syntax `isRestricted` below cannot resolve correctly: `resolve()`
311+
// treats a leading `~` as a literal path segment rather than expanding it,
312+
// so a home-relative path would otherwise read as "inside cwd" on the very
313+
// next line; a glob is not a single concrete destination at all. This
314+
// duplicates isPermittedSiblingWorktreePath's own guard against the same
315+
// two forms, but that duplication is required, not incidental: this check
316+
// has to run before the isRestricted() shortcut below even executes, while
317+
// isPermittedSiblingWorktreePath's copy protects direct/standalone callers
318+
// of that exported function.
319+
if (/[*?[]/.test(pathArg)) return false;
320+
if (pathArg.startsWith("~")) return false;
321+
322+
// Workspace (cwd + registered worktree roots) — always contained.
323+
if (!isRestricted(pathArg, true)) return true;
324+
325+
return isPermittedSiblingWorktreePath(cwd, pathArg, rootsProvider);
326+
}
327+
328+
// Walks worktree args, recording force and every positional path. Value-taking
329+
// flags consume the next token so branch names are not mistaken for paths.
330+
function worktreePathArgs(
331+
args: string[],
332+
valueFlags: Set<string>,
333+
): { force: boolean; paths: string[] } {
334+
const paths: string[] = [];
335+
let force = false;
336+
for (let i = 0; i < args.length; i++) {
337+
const arg = args[i]!;
338+
if (arg === "--") {
339+
paths.push(...args.slice(i + 1));
340+
break;
341+
}
342+
if (isWorktreeForceFlag(arg)) {
343+
force = true;
344+
continue;
345+
}
346+
if (arg.startsWith("-") && arg !== "-") {
347+
// `--flag=value` carries its value inline; no following token to skip.
348+
if (arg.includes("=")) continue;
349+
if (valueFlags.has(arg)) {
350+
i += 1;
351+
continue;
352+
}
353+
continue;
354+
}
355+
paths.push(arg);
356+
}
357+
return { force, paths };
358+
}
359+
360+
// `true` = auto-allow, `false` = ask, `undefined` = not a worktree command.
361+
// Contained non-force add/remove and ordinary prune/list auto-allow so dispatch
362+
// can create sibling worktrees without a human click; force flags, uncontained
363+
// paths, and uncommon subcommands still ask.
364+
function safeWorktreeCommand(
365+
command: string,
366+
isRestricted: (path: string, isWrite: boolean) => boolean,
367+
cwd: string,
368+
rootsProvider: RootsProvider,
369+
): boolean | undefined {
288370
const tokens = tokenize(command);
289371
if (tokens[0] !== "git" || !tokens.slice(1).includes("worktree")) return undefined;
290372
// Worktree policy applies only to one plain command with no git cwd override;
@@ -294,7 +376,32 @@ function safeWorktreeCommand(command: string): boolean | undefined {
294376
const args = tokens.slice(3);
295377

296378
if (subcommand === "list") return args.every((arg) => WORKTREE_LIST_FLAGS.has(arg));
297-
// Boundary-changing subcommands always route to ask, even when the destination is inside cwd.
379+
380+
if (subcommand === "prune") {
381+
for (let i = 0; i < args.length; i++) {
382+
const arg = args[i]!;
383+
if (WORKTREE_PRUNE_FLAGS.has(arg)) continue;
384+
if (arg.startsWith("--expire=")) continue;
385+
if (arg === "--expire") {
386+
i += 1;
387+
continue;
388+
}
389+
return false;
390+
}
391+
return true;
392+
}
393+
394+
if (subcommand === "add" || subcommand === "remove") {
395+
const valueFlags = subcommand === "add" ? WORKTREE_ADD_VALUE_FLAGS : new Set<string>();
396+
const { force, paths } = worktreePathArgs(args, valueFlags);
397+
if (force) return false;
398+
// add/remove require a path; no path → ask rather than guess.
399+
if (paths.length === 0) return false;
400+
// First positional is the worktree path; later tokens on add are commit-ish.
401+
return isContainedWorktreePath(paths[0]!, isRestricted, cwd, rootsProvider);
402+
}
403+
404+
// move / lock / unlock / repair / unknown — still ask until proven safe.
298405
return false;
299406
}
300407

@@ -307,9 +414,13 @@ function preferRule(a: AutoShellRule | undefined, b: AutoShellRule | undefined):
307414
return a;
308415
}
309416

417+
const NO_ROOTS: RootsProvider = () => [];
418+
310419
export function autoShellRuleForCall(
311420
call: ToolCall,
312421
isRestricted: (path: string, isWrite: boolean) => boolean = () => false,
422+
cwd: string = process.cwd(),
423+
rootsProvider: RootsProvider = NO_ROOTS,
313424
): AutoShellRule | undefined {
314425
if (call.name !== "run_shell") return undefined;
315426
const command = call.arguments.command;
@@ -349,14 +460,16 @@ export function autoShellRuleForCall(
349460

350461
// Containment: a command whose path arguments resolve outside the workspace
351462
// (including through a symlink) must ask rather than auto-run, the same way
352-
// path-arg tool calls already do. Checked per expanded subject so a wrapped
353-
// payload (bash -c, xargs, env -S) is judged on its real target, not the wrapper.
463+
// path-arg tool calls already do. Contained worktree ops are exempt — their
464+
// destinations are often intentional siblings (`../corbits-dispatch-wts/…`)
465+
// and are judged by the worktree path policy below instead.
354466
for (const subject of subjects) {
467+
if (safeWorktreeCommand(subject, isRestricted, cwd, rootsProvider) === true) continue;
355468
if (commandTargetsRestricted(subject, isRestricted)) return OUTSIDE_WORKSPACE_ASK_RULE;
356469
}
357470

358471
for (const subject of subjects) {
359-
if (safeWorktreeCommand(subject) === false) return WORKTREE_ASK_RULE;
472+
if (safeWorktreeCommand(subject, isRestricted, cwd, rootsProvider) === false) return WORKTREE_ASK_RULE;
360473
}
361474

362475
if (matched !== undefined) return matched;

src/permission/gate.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
334334
// operator prompt. Everything else auto-allows. Path-keyed secret
335335
// reads stay hard-denied by secret-guard; shell that only *mentions*
336336
// a secret path is ask so an explicit one-time approval can pass it.
337-
const shellRule = autoShellRuleForCall(call, isRestrictedHere);
337+
const shellRule = autoShellRuleForCall(call, isRestrictedHere, effectiveCwd, rootsProvider);
338338
if (shellRule?.effect === "deny") return { allowed: false, reason: shellRule.reason };
339339
if (shellRule === undefined) return { allowed: true };
340340
} else if (!restricted && AUTO_ALLOWED_TOOLS.has(call.name)) {

src/permission/path-restriction.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,40 @@ export function resolveWorkspacePath(
8686
return undefined;
8787
}
8888

89+
// Whether `path` (relative to `cwd`) names a not-yet-created sibling worktree
90+
// location: a direct child of the parent directory of `cwd` or of a currently
91+
// registered root — the "one new dir next to something already trusted" shape
92+
// `git worktree add ../name` uses. This is the single containment authority's
93+
// answer to "can auto mode create a brand-new worktree that isn't a registered
94+
// root yet"; there is deliberately no separate basename denylist or `..` depth
95+
// counter — the parent-directory equality check *is* the depth bound (a path
96+
// with any extra segment resolves to a different, non-matching parent), and
97+
// the home-directory guard below is the one home-config bag it purpose-built
98+
// against ($HOME's own children — .ssh, .aws, .config, … must never qualify).
99+
export function isPermittedSiblingWorktreePath(
100+
cwd: string,
101+
path: string,
102+
rootsProvider: RootsProvider = () => [],
103+
home: string = homedir(),
104+
): boolean {
105+
if (path.length === 0) return false;
106+
if (/[*?[]/.test(path)) return false;
107+
if (path.startsWith("~")) return false;
108+
if (path.startsWith("/") || /^[A-Za-z]:[\\/]/.test(path)) return false;
109+
110+
const abs = resolve(cwd, path);
111+
const realParent = realpathOr(dirname(abs));
112+
const realHome = realpathOr(resolve(home));
113+
if (realParent === realHome) return false;
114+
115+
const knownRoots = [...rootsProvider(), ...rootsProvider(true)];
116+
const trustedParents = new Set<string>([
117+
realpathOr(resolve(cwd, "..")),
118+
...knownRoots.map((root) => realpathOr(dirname(root))),
119+
]);
120+
return trustedParents.has(realParent);
121+
}
122+
89123
function underRoot(abs: string, root: string): boolean {
90124
// realpathNearestOr on both sides so a not-yet-created state root still
91125
// compares equal to paths under it (realpathOr alone leaves the root

0 commit comments

Comments
 (0)