From e28dce8bb743d6df6f8edddbfda426f0bd8e269d Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 25 Sep 2026 05:15:43 -0700 Subject: [PATCH 1/4] fix(permissions): make yolo override auto mode --- docs/ARCHITECTURE.md | 8 +- docs/IMPLEMENTATION.md | 42 +++++----- docs/PRODUCT.md | 12 +-- src/config.test.ts | 44 ++++++++++- src/config/index.ts | 27 ++++--- src/exec/runner.ts | 3 +- src/permission/permission.test.ts | 41 +++++++--- src/permission/saved-skip-warning.ts | 5 ++ src/tui/commands/built-in.test.ts | 78 +++++++++++++++++++ src/tui/commands/built-in.ts | 2 +- src/tui/commands/registry.ts | 2 +- src/tui/runner/session.ts | 14 +--- src/tui/runner/settings-writers.ts | 20 +++++ .../wiring.skip-permissions-warning.test.ts | 45 +++++++++++ src/tui/runner/wiring.ts | 23 ++++-- tests/unit/exec/runner.test.ts | 56 +++++++++++-- 16 files changed, 344 insertions(+), 78 deletions(-) create mode 100644 src/permission/saved-skip-warning.ts create mode 100644 src/tui/runner/settings-writers.ts create mode 100644 src/tui/runner/wiring.skip-permissions-warning.test.ts diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 0d264445c..cb15d8954 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -87,8 +87,8 @@ In TUI chat mode there is no completion gate — the session stays open across t - `--config ` replaces the global settings file as the provider source (useful for CI per-run injection). A provider must be defined in a settings file; there is no env fallback. - `settings.ts` owns the schema, validators (the per-repo file rejects credentials), file loaders, and the pure `resolveProvider` precedence function. - `providers.ts` defines the `ProviderCatalogEntry` type and helpers for building TUI provider lists; `profiles.ts` handles profile-level selection logic. -- `loadConfig` is async (it reads settings files). Parses a leading `exec`/`run` subcommand, flags `--cwd`, `--config`, `--provider`, `--model`, `--dangerously-skip-permissions` (forces this process; TUI `/yolo` persists as the user-global default), `--auto` / `--no-auto` (auto mode defaults on); collects positional arguments as the optional initial task for the TUI or the required prompt for exec. -- Both settings files and the project/global grant store (`.corbits/permissions.json`) are on the secret-guard denylist for path-keyed tools, so the agent cannot `read_file` its own credentials or persist standing auto-approvals. Shell commands that reference them still require explicit operator approval. +- `loadConfig` is async (it reads settings files). Parses a leading `exec`/`run` subcommand, flags `--cwd`, `--config`, `--provider`, `--model`, `--dangerously-skip-permissions` and its `--yolo` alias (both force only this process and never persist), `--auto` / `--no-auto` (auto mode defaults on); collects positional arguments as the optional initial task for the TUI or the required prompt for exec. Authorization precedence remains catastrophic authorization denial → skip → normal tier/grant/auto, so `--auto --yolo` behaves as yolo. TUI `/yolo` persists to the active settings file: with the default settings source this is the machine-wide default file, while explicit `--config ` selects that path as the active source. An ordinary TUI started without the same `--config` does not modify a custom settings file. +- The default user-global settings path, the per-repo `.corbits/settings.json`, and the project/global grant store (`.corbits/permissions.json`) are on the static secret-guard denylist for path-keyed tools, so the agent cannot `read_file` credentials there or persist standing auto-approvals. An arbitrary path selected with `--config ` is not added to that denylist at runtime. In normal and auto modes, shell commands that reference a statically protected path require explicit operator approval; yolo/skip-permissions allows those shell references after catastrophic authorization checks, while path-keyed access to statically protected paths remains hard-denied. - Credential-surface ownership: each auth store module enumerates its own files (`*_AUTH_FILENAME` / `MCP_AUTH_DIRNAME`), the data-only registry in `src/auth/credential-surface.ts` turns them into denylist patterns, `secret-guard-plugin.ts` owns matching (lexical plus realpath), and `@mention` resolution consumes the resolved check — never the registry directly. A new `*-auth.json` token store is denied only once its store module exports its filename and the registry lists it; the coverage test scans store sources for `*-auth.json` literals (registered dirnames get an includes-check instead) and fails the build until both exist. ### TUI Runner (`src/tui/runner/`) @@ -397,7 +397,7 @@ tool call - **Path Escape** (`path-escape-plugin.ts`) — Canonicalizes path-like arguments against `cwd` and blocks `..` escapes, except into a root the permission layer's worktree-roots provider allowlists (e.g. a sibling git worktree of the same repo). `tool-output://` and `archive:///` refs pass through unresolved. Runs first so later plugins see resolved paths. - **Evidence archive** (`evidence-archive-search-plugin.ts`, `evidence-archive-path-guard.ts`) — Primary-session compaction evidence is a first-class search/read surface on `search_files` / `read_file` / `grep` via `archive:///` refs. Dump paths (`evidence-archive/`, `tool-output/archive-*`) stay blocked so the on-disk sidecar is not the retrieval API. Blob keys reject `/` so they cannot nest under `tool-output`. - **Tool-output URI** (`tool-output-uri-plugin.ts`) — Normalizes mistaken `read_file` blob URIs to `tool-output:///id` (corbits-only; interchange stays unpatched). -- **Secret Guard** (`secret-guard-plugin.ts`) — Hard-denies path-keyed tool calls (`read_file`, `write_file`, …) that would put a sensitive file into (or write it from) the model context. Runs before the permission plugin, so the path-arg deny holds even under `--dangerously-skip-permissions`. Shell commands that _reference_ a sensitive path (tokenized so `cat .env`, `bun --env-file=.env run …`, and quote/env-assignment forms are detected) are not hard-denied here: they require operator approval via the permission gate, and auto mode forces an ask through the auto-shell policy (`sensitive-path` rule). Once the operator approves, the command runs. Shell detection is best-effort: token matching defeats quoting and env-assignment/redirection forms but not dynamic path construction (variable indirection, `printf` assembly). Tool-result secret scrub still redacts credential-shaped output. +- **Secret Guard** (`secret-guard-plugin.ts`) — Hard-denies path-keyed tool calls (`read_file`, `write_file`, …) that would put a sensitive file into (or write it from) the model context. Runs before the permission plugin, so the path-arg deny holds even under `--dangerously-skip-permissions`. Shell commands that _reference_ a sensitive path (tokenized so `cat .env`, `bun --env-file=.env run …`, and quote/env-assignment forms are detected) are not hard-denied here. Normal mode requires operator approval via the permission gate, and auto mode forces the same ask through the auto-shell policy (`sensitive-path` rule). Yolo/skip-permissions bypasses that prompt after catastrophic authorization checks, but path-keyed access to statically protected secret paths remains hard-denied. Shell detection is best-effort: token matching defeats quoting and env-assignment/redirection forms but not dynamic path construction (variable indirection, `printf` assembly). Tool-result secret scrub still redacts credential-shaped output. - **Authorization** (`run-shell-authz.ts`, enforced by the permission gate) — Denies catastrophic shell command patterns by regex, and hard-blocks shell `find`, head-position `rg`, and recursive `grep -r` (they can walk huge trees and OOM the host). Bounded `grep`/`search_files` tools remain practical alternatives (timeout + output caps); the patterns match those three command shapes only — an `ls -R`, `fd`, or scripted `os.walk` is just as unbounded and is not caught, so the block message tells the model not to substitute one. The gate hard-denies these at the top of its verdict path — before auto-allow, prompting, grants, and skipPermissions — so no mode or stored grant can admit them. - **Permission** (`permission-plugin.ts`) — Delegates consequential calls to the permission gate. - **Shell Guard** (`shell-guard-plugin.ts`) — Corbits Code-only replacement for stock `run_shell` (interchange stays unpatched): 120s foreground default (`settings.shell.timeoutMs` overrides that default only; a positive per-call `timeout` is the bound with no ceiling — `maxTimeoutMs` does not clamp it), background `run_shell` has no default (only a per-call timeout arms a timer), 512KB display cap with head+tail retention (the process keeps running when the cap is hit), process-group kill on timeout, abort, and plugin dispose (live children tracked in the plugin and reaped by `posixTools.dispose`), and `background: true` — the call returns a `shell_id` at once (registry in `src/shell/background-shell.ts`), the process group keeps running past the turn, completion is process-exit (stdio-close is not required), delivered on a later turn via `buildShellBackgroundMessage`, and `shell_collect` retrieves or cancels with a 300s wait cap (schema advertised by `advertiseShellGuardTimeout` only when `shell_collect` is mounted; evaluated by the permission chain at start time like any shell call). Foreground expiry is exit 124 + `timedOut:true` plus a nudge to retry with `background:true`. Also applies a 10s wall-clock budget to `grep`/`search_files`. Ripgrep detached spawns are not tracked. @@ -412,7 +412,7 @@ tool call - **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. - **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). - **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`. -- **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`, `spawn_agent`, `wait_agents`, …) 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 on path-arg tools are denied at authorize time (the same sandbox path-escape enforces at execution, so the gate does not show an Accept overlay that cannot succeed). Writes under the in-workspace session state root (legacy `.agent-state`) still ask under auto mode. Under `--dangerously-skip-permissions` (forces this process) or `/yolo` (persists as the user-global default via `setSkipPermissions`), the gate auto-allows those same cases, and pre-gate sandboxes (path-escape, shell session cwd retention, `list_dir` / `delete_file` workspace bounds) honor `getSkipPermissions()` live so outside-workspace access is not hard-denied after the gate already allowed it — without rebuilding the plugin stack. Secret-guard path denies and authorization hard blocks still apply. Mutating MCP and unknown built-ins are not blanket-allowed outside skip. Newly granted scopes are appended in memory and persisted. +- **gate** — Evaluates a call in order: catastrophic authorization denial, `skipPermissions`, then the normal tier, grant, and auto policies. Catastrophic commands remain denied under every mode. Skip mode, enabled for the process by `--dangerously-skip-permissions` or `--yolo` and persisted to the active settings source by TUI `/yolo` (machine-wide only for the default user-global source, not an explicit `--config` source), allows the call before normal policy evaluation. Therefore `--auto --yolo` behaves as yolo: auto-mode file-mutation asks and denials do not run. Otherwise, `allow`-tier passes; for `ask`-tier, the gate checks persisted approvals and then 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`, `spawn_agent`, `wait_agents`, …) 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 on path-arg tools are denied at authorize time (the same sandbox path-escape enforces at execution, so the gate does not show an Accept overlay that cannot succeed). Writes under the in-workspace session state root (legacy `.agent-state`) still ask under auto mode. Under process-only `--dangerously-skip-permissions` / `--yolo` or TUI `/yolo` persisted in the active settings source (machine-wide only for the default user-global source), the gate auto-allows those same cases, and pre-gate sandboxes (path-escape, shell session cwd retention, `list_dir` / `delete_file` workspace bounds) honor `getSkipPermissions()` live so outside-workspace access is not hard-denied after the gate already allowed it — without rebuilding the plugin stack. Static secret-guard denies for path-keyed tools and authorization hard blocks still apply. Sensitive shell references require approval in normal and auto modes, but skip mode allows them without approval after catastrophic authorization checks. Mutating MCP and unknown built-ins are not blanket-allowed outside skip. Newly granted scopes are appended in memory and persisted. - **Reactor-gated sessions (main session; `reactorGated: true`).** The gate's decision logic lives in one `decide()` used by both consumers: `evaluate()` (the middleware path below, still used by sub-agents) and `authorizeCall()`, which expresses the decision as the vendored reactor's before-tool authz effect (`src/permission/reactor-authorize.ts` bridges it into `env.authorize`). An `ask` there suspends the call as a reactor `PendingOperation` keyed by a correlationId (persisted through the context store's existing `pendingOperations`); `send()` settles as `suspended` and `src/session/approval-resume.ts` rebuilds the operator request from the approval snapshot, resolves it through the same `requestApproval` seam the TUI overlay uses, and delivers the decision to the reactor on the correlationId signal channel — an approved decision grants a one-shot bypass and the exact parked call re-dispatches; a rejected one answers it with an error result. `inFlight` occupancy owns idle rebuild: the TUI stays busy across the overlay and waits until the correlated resume is accepted (`message.received` / `message.correlated`) or a generation bump `settleAll`s the waiter. Delivery generation owns session identity: interrupt, `/clear`, and `/new` abort the outstanding overlay, skip minting a grant, drop the decision, and surface an operator notice rather than delivering into a rebuilt agent. Under reactor gating the middleware/MCP `gateToolCall` is an execution backstop, not a second copy of `env.authorize`: it consumes the `authorizeCall` verdict only when id, name, and arguments match, and does not re-decide. Deny still blocks and does not call `next`; an `ask` or `allow` skips the middleware prompt so an approved re-dispatch never re-asks. A reused `codex-proxy` id cannot apply an outer `shell` allow to an inner `run_shell` deny. Inner posix runs whose outer tool is not `run_shell` (Codex `apply_patch` proxy) never pass `env.authorize`, so `gateToolCall` decides on that cache miss and still blocks a deny. The headless denial and the stricter chained-command hard-deny are preserved as deny effects (upstream `block`s) decided inside the same `decide()`. - **Approval resume identity.** Before opening the operator gate, resume captures the session generation and agent/store pair and resolves the correlation exactly once through `ContextStore.load().pendingOperations` to one approval operation's `suspendedCall.id`. Missing or duplicate mappings produce no gate or delivery; store errors propagate with registration cleanup. Every decision path first checks captured history for an exact-call approval timeout, and operator decisions check again after the gate. While the overlay is open, resume watches history for that exact-call timeout and aborts the overlay signal so the gate auto-denies, occupancy (`inFlight`) unsticks, and no late decision is delivered. Identical tool names and arguments never establish identity. Cancellation during lookup cannot deliver a rejection. This suppresses observed exact timeouts, not all expired correlations: the reactor removes correlation state before the queued timeout result publishes, and expiration can also race the final history check or TUI delivery queue. Atomic stale-decision admission remains reactor-owned work tracked separately in CL-8000. - **Worker reactor ownership.** `workerPermissionGate` is a reactor-gated view over the parent's live permission gate: grants and policy are shared, not copied or toggled. Worker posix plugins and inherited MCP tools are bound to that view at worker start, so they take the reactor-gated `gateToolCall` path because the view reports `isReactorGated()` — they do not close over the parent's middleware-gated `isReactorGated()`. Deny still blocks; ask/allow skip the middleware prompt. `authorizeCall` on the view never emits `ask` — unresolved approvals become denials that name the permission subject, without invoking an approval callback or suspending, even with an interactive parent; the parent can obtain a grant and retry. Worker control-plane tools (`submit_result`, `ask_director`, and nested fleet verbs other than `spawn_agent`) allow without a parent grant. Authorization and tool execution run under the same async-local worker identity and cwd. Fleet authority remains an independent restriction, not an alternative permission grant. diff --git a/docs/IMPLEMENTATION.md b/docs/IMPLEMENTATION.md index ae42aa2b8..c89685079 100644 --- a/docs/IMPLEMENTATION.md +++ b/docs/IMPLEMENTATION.md @@ -174,7 +174,7 @@ Intent defaults: `intent=implement` → director `builder`; `explore` → `explo ### Auto Mode -Auto mode defaults **on** (`config.auto = true` from `loadConfig`; pass `--no-auto` to start off, or `--auto` to force on). It is toggled only via those CLI flags — there is currently no in-session key bound to it. The permission gate reads the flag (`getAuto`/`setAuto` in `src/permission/gate.ts`) on the next tool call. `--dangerously-skip-permissions` still forces this process. `/yolo [on|off|toggle]` (bare `/yolo` also toggles) persists as the user-global default and wires `getSkipPermissions`/`setSkipPermissions` so the gate and pre-gate sandboxes honor the change on the next tool call without rebuilding plugins. `/yolo` writes the same `config.globalSettingsPath` target as the other `/settings`-style toggles, including a `--config` override. Secret-guard and authz still apply. `loadConfig` tracks `skipPermissionsFromSettings` (true only when the effective value came from persisted settings, not the CLI flag) so `runTUI` can show a startup notice and `exec` can print an equivalent stderr warning for the otherwise-silent persisted default. +Auto mode defaults **on** (`config.auto = true` from `loadConfig`; pass `--no-auto` to start off, or `--auto` to force on). It is toggled only via those CLI flags — there is currently no in-session key bound to it. The permission gate reads the flag (`getAuto`/`setAuto` in `src/permission/gate.ts`) on the next tool call. `--dangerously-skip-permissions` and its `--yolo` alias force skip-permissions for this process only; skip-permissions wins when combined with auto, so `--auto --yolo` runs in yolo mode. `/yolo [on|off|toggle]` (bare `/yolo` also toggles) persists the skip-permissions setting to the active settings file and wires `getSkipPermissions`/`setSkipPermissions` so the gate and pre-gate sandboxes honor the change on the next tool call without rebuilding plugins. The active source defaults to the user-global `~/.corbits/settings.json`, making that default machine-wide. Explicit `--config ` selects that file instead, so `/yolo` writes the custom file; a later ordinary TUI launch without the same `--config` uses the user-global source and does not modify the custom file. Secret-guard and authz still apply. `loadConfig` tracks `skipPermissionsFromSettings` (true only when the effective value came from persisted settings, not either CLI flag) so `runTUI` can show a startup notice and `exec` can print an equivalent stderr warning for the otherwise-silent persisted setting. When auto is on, the gate auto-allows workspace file tools in `AUTO_ALLOWED_TOOLS` and any `run_shell` that does not match the auto-shell policy. The policy (`autoShellRuleForCall` / `AUTO_SHELL_RULES` in `src/permission/auto-shell-policy.ts`) peels wrappers via `expandShellSubjects` (`bash`/`sh`/`zsh -c`, `xargs`, transparent prefixes), then applies: @@ -338,7 +338,7 @@ Credentials and provider definitions come exclusively from the settings files. E OpenAI-compatible `baseURL` values are normalized during provider resolution. A plain base URL such as `https://provider.example.com/v1` is preserved, a trailing slash is removed, and a pasted full chat-completions endpoint such as `https://provider.example.com/v1/chat/completions` is reduced to `https://provider.example.com/v1` before the runtime appends `/chat/completions`. Invalid non-URL values fail with an explicit baseURL error. -`--config ` replaces the global settings file as the provider source (useful for CI to inject a provider per run). The per-repo `.corbits/settings.json` selection still applies on top of a `--config` source (definitions come from `--config`, selection from the local file; CLI `--provider`/`--model` override both). A provider must be defined in one of these settings files; there is no environment-variable fallback. +`--config ` replaces the user-global settings file as the active settings source (useful for CI to inject a provider per run); TUI settings persistence, including `/yolo`, writes that active file. The per-repo `.corbits/settings.json` selection still applies on top of a `--config` source (definitions come from `--config`, selection from the local file; CLI `--provider`/`--model` override both). A provider must be defined in one of these settings files; there is no environment-variable fallback. `--config` composes with, rather than replaces, the home-level OAuth profile catalog: codex/xai credentials live in `~/.corbits/codex-auth.json` and `xai-auth.json`, entirely separate from settings.json, and are merged into the resolved provider catalog on every run regardless of `--config` (CL-6973). A `--config` file that names a `codex/*` or `xai/*` provider by ID does not by itself grant that provider's credentials — those come from the OAuth store whenever a matching profile exists there, independent of which settings file supplied the provider definitions. The only way to fully exclude the home OAuth catalog is the programmatic `globalSettingsPath` option to `loadConfig`, used by tests for full isolation; it is not exposed as a CLI flag. @@ -389,25 +389,25 @@ Providers and credentials are read exclusively from settings files: the global ` Printed by `corbits --help` / `-h` from `CLI_HELP_TEXT` in `src/config/index.ts` (that constant is the source of truth; keep this table in sync when flags change). -| Verb / Flag | Default | Description | -| -------------------------------- | -------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| _(no verb)_ | — | Interactive session; optional trailing task text | -| `exec` / `run` / `-p` | — | Run a prompt (non-interactive / one-shot). `-p` is the same path as `exec` and may appear in any flag position. | -| `resume` / `continue` | — | Open the session picker for this folder (project-keyed to this checkout's git toplevel). Lists the 10 most recently persisted sessions, completed included. Type to filter. | -| `--resume []` | — | Interactive: open the session picker, or reopen a specific session when an id is given. With `exec` / `-p` the id is required, that session is continued, and the new prompt is sent (no picker). | -| `resume ` | — | Reopen a specific session in the TUI (does not auto-send a prompt) | -| `exec --resume ` | — | Headless: load that session and send ``, then exit. Missing or unreadable ids error and do not create a session. `--resume` without an id errors. | -| `-p --resume ` | — | Same headless continue path as `exec --resume` | -| `resume --pick` / `--list` | — | Interactive session picker | -| `--cwd ` | `process.cwd()` | Working directory | -| `--config ` | `~/.corbits/settings.json` | Settings file to use for provider definitions; composes with (does not exclude) home-level codex/xai OAuth credentials | -| `--provider ` | from settings | Select a configured provider | -| `--model ` | provider default | Select a model for the active provider | -| `--profile ` | — | Settings profile | -| `--dangerously-skip-permissions` | false | Auto-allow anything not denied by the authorization layer (gate + pre-gate workspace sandboxes; secret-guard / authz hard denies remain). This launch flag still forces this process; `/yolo [on\|off\|toggle]` persists as the user-global default via `setSkipPermissions` | -| `--auto` | true (default) | Force auto mode on (workspace writes + unconstrained shell without prompts) | -| `--no-auto` | false | Start with auto mode off (ask on every consequential action); no in-session key toggles it | -| `--help`, `-h` | — | Show help (exit 0 via `CliHelpError`) | +| Verb / Flag | Default | Description | +| ------------------------------------------ | -------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| _(no verb)_ | — | Interactive session; optional trailing task text | +| `exec` / `run` / `-p` | — | Run a prompt (non-interactive / one-shot). `-p` is the same path as `exec` and may appear in any flag position. | +| `resume` / `continue` | — | Open the session picker for this folder (project-keyed to this checkout's git toplevel). Lists the 10 most recently persisted sessions, completed included. Type to filter. | +| `--resume []` | — | Interactive: open the session picker, or reopen a specific session when an id is given. With `exec` / `-p` the id is required, that session is continued, and the new prompt is sent (no picker). | +| `resume ` | — | Reopen a specific session in the TUI (does not auto-send a prompt) | +| `exec --resume ` | — | Headless: load that session and send ``, then exit. Missing or unreadable ids error and do not create a session. `--resume` without an id errors. | +| `-p --resume ` | — | Same headless continue path as `exec --resume` | +| `resume --pick` / `--list` | — | Interactive session picker | +| `--cwd ` | `process.cwd()` | Working directory | +| `--config ` | `~/.corbits/settings.json` | Active settings file for provider definitions and TUI persistence such as `/yolo`; composes with (does not exclude) home-level codex/xai OAuth credentials | +| `--provider ` | from settings | Select a configured provider | +| `--model ` | provider default | Select a model for the active provider | +| `--profile ` | — | Settings profile | +| `--dangerously-skip-permissions`, `--yolo` | false | Process-only aliases that auto-allow anything not denied by the authorization layer (gate + pre-gate workspace sandboxes; secret-guard / authz hard denies remain). `/yolo [on\|off\|toggle]` persists to the active settings file instead. | +| `--auto` | true (default) | Force auto mode on (workspace writes + unconstrained shell without prompts) | +| `--no-auto` | false | Start with auto mode off (ask on every consequential action); no in-session key toggles it | +| `--help`, `-h` | — | Show help (exit 0 via `CliHelpError`) | Positional arguments after flags are joined into the optional initial task delivered when the TUI mounts. With no positional task, the operator starts from an empty prompt. diff --git a/docs/PRODUCT.md b/docs/PRODUCT.md index e22a39d6b..d4573d83d 100644 --- a/docs/PRODUCT.md +++ b/docs/PRODUCT.md @@ -66,7 +66,7 @@ $ corbits exec "Add JWT auth to the API" $ corbits run "Add JWT auth to the API" ``` -Same directors, tools, permissions, MCP, plugins, and hooks as the TUI — without the OpenTUI shell. Bootstrap shares `src/session/assemble-runtime.ts` with the TUI; see `docs/ARCHITECTURE.md` “Exec Runner” for intentional deltas (no workflow controller; single primary send; non-interactive permission gate; `ask_operator` unmounted when non-TTY). Compaction continuation matches TUI so long runs do not stall after compact. Streams assistant text to stdout for scripts and CI. Non-interactive by default: actions that need operator approval are denied unless `--dangerously-skip-permissions` is set, a persisted `/yolo` default is on, or auto mode covers them. `--dangerously-skip-permissions` still forces this process; secret-guard and authz still apply. `ask_operator` is not advertised on non-TTY exec; TTY exec still reads a single line from stdin. +Same directors, tools, permissions, MCP, plugins, and hooks as the TUI — without the OpenTUI shell. Bootstrap shares `src/session/assemble-runtime.ts` with the TUI; see `docs/ARCHITECTURE.md` “Exec Runner” for intentional deltas (no workflow controller; single primary send; non-interactive permission gate; `ask_operator` unmounted when non-TTY). Compaction continuation matches TUI so long runs do not stall after compact. Streams assistant text to stdout for scripts and CI. Non-interactive by default: actions that need operator approval are denied unless `--dangerously-skip-permissions` or its `--yolo` alias is set, the active settings file has persisted yolo on, or auto mode covers them. Both CLI flags force skip-permissions for this process only; skip-permissions wins when combined with auto, so `--auto --yolo` runs in yolo mode. Secret-guard and authz still apply. `ask_operator` is not advertised on non-TTY exec; TTY exec still reads a single line from stdin. Local multi-model capability checks use this path (`bun run eval:capability`); see `evals/capability/README.md`. @@ -92,7 +92,7 @@ the file path and parse details. ## Safety Model - **Tiered permission gate** — Read-only tools (`read_file`, `search_files`, `grep`, `list_dir`) run freely. Every consequential tool (`write_file`, `edit_file`, `run_shell`, …) is gated. The operator can Allow Once or Allow Always (scoped to a file, a directory, or a command shape). Allow Always applies for the rest of the session; Corbits also tries to remember it on disk so later sessions don't re-ask. If that write fails, the grant still holds this session and the operator is told remember did not stick. -- **Secret guard** — Path-keyed tools (`read_file`, `write_file`, …) hard-deny sensitive files (`.env`, `id_rsa`, `*.pem`, `.aws/credentials`, `.ssh/*`, `.git-credentials`, and similar), even with approval, `--dangerously-skip-permissions`, or `/yolo`. Template files like `.env.example` are exempt. Shell commands that _reference_ those paths (e.g. `bun --env-file=.env.staging run …`, `cat .env`) require explicit operator approval and never auto-run in auto mode; once approved, they proceed. Tool-result scrubbing still redacts credential-shaped output that reaches the transcript. +- **Secret guard** — Path-keyed tools (`read_file`, `write_file`, …) hard-deny sensitive files (`.env`, `id_rsa`, `*.pem`, `.aws/credentials`, `.ssh/*`, `.git-credentials`, and similar), even with approval, `--dangerously-skip-permissions`, `--yolo`, or `/yolo`. Template files like `.env.example` are exempt. In normal and auto modes, shell commands that _reference_ sensitive paths (e.g. `bun --env-file=.env.staging run …`, `cat .env`) require explicit operator approval. Yolo/skip-permissions modes allow those shell references without approval after catastrophic authorization checks; the path-keyed secret guard remains a hard deny. Tool-result scrubbing still redacts credential-shaped output that reaches the transcript. - **Catastrophic-command deny** — Destructive shell patterns that target system roots (`rm -rf /`, home, `/etc`, …), plus `mkfs`, `dd`, `sudo`, fork bombs, `curl | bash`, force-push, … are blocked before they run. Recursive delete of ordinary workspace paths is not hard-denied but requires operator approval (never auto in auto mode). - **Constrained auto mode** — Default is on (`auto = true`). Pass `--no-auto` to start in ask mode, or `--auto` to force it on; there is currently no in-session key to toggle it. Auto mode auto-approves workspace file writes/edits/deletes and unconstrained shell without per-action prompts, but it is not a free-for-all: - **Denied** (must use `write_file` / `edit_file`): shell file mutations via output redirection, `tee`, `sed -i` / `perl -i`, interpreter inline programs or heredocs. @@ -100,12 +100,12 @@ the file path and parse details. - **Wrapper peel**: `bash`/`sh`/`zsh -c`, `xargs`, and transparent prefixes (`env`, `nice`, `timeout`, …) are expanded so the same deny/ask rules see the inner payload. - Path-arg tools that escape the workspace are denied at authorize time (yolo still allows them). Writes under the in-workspace session state root still ask; mutating MCP and unknown tools still prompt. Shell that targets an outside path still asks. -- **Path sandboxing** — Tool path arguments are resolved against the working directory; paths that escape it are blocked unless `--dangerously-skip-permissions` / `/yolo` is on (secret-guard and authz hard denies still apply). +- **Path sandboxing** — Tool path arguments are resolved against the working directory; paths that escape it are blocked unless `--dangerously-skip-permissions`, `--yolo`, or persisted `/yolo` is on (secret-guard and authz hard denies still apply). - **Write verification** — After every write/edit the file is re-read and compared to confirm the change actually landed; the result returned to the model (and shown to the operator) includes a bounded diff of the changed region — `write_file`, `edit_file`, `delete_file`, and each op inside `apply_patch` — so a follow-up `read_file` is never needed just to confirm an edit landed. A whole-file rewrite's diff is truncated (and says so) rather than blowing the result size cap. ## Slash Commands (TUI) -The TUI has an extensible slash-command framework. Built-ins include `/help` (shortcut + command overlay), `/model` (models-only picker for connected accounts; **Alt+A** or `/connect` adds a provider), `/settings`, `/permissions`, `/plugins`, `/clear`, `/new`, `/compact` (fold conversation context now, optional trailing instructions to the summarizer; does not wait for the 60% occupancy governor; idle success shows the fold and does not start a new turn), `/mcp` (enable, disable, or remove servers), `/handoff [optional instructions]` (folds context through the shared operator pipeline, then immediately starts the next turn with the instructions as the inbound content — default copy when omitted; unlike `/compact`, which stops after the fold, handoff always re-infers, so the operator can pivot goals without `/clear`; a handoff issued mid-tool-batch queues behind the in-flight batch and whichever boundary fires first runs the single fold), and `/yolo` (persists as the user-global skip-permissions default; `--dangerously-skip-permissions` still forces this process; secret-guard and authz still apply; `/yolo [on|off|toggle]`, bare `/yolo` toggles), plus a `/` command per available workflow. When a session starts with the persisted default already on, the TUI shows a startup notice ("Permission prompts are disabled by your saved default…") so the silent machine-wide default is never invisible; `corbits exec` prints the equivalent warning to stderr. Plugins can register additional commands. +The TUI has an extensible slash-command framework. Built-ins include `/help` (shortcut + command overlay), `/model` (models-only picker for connected accounts; **Alt+A** or `/connect` adds a provider), `/settings`, `/permissions`, `/plugins`, `/clear`, `/new`, `/compact` (fold conversation context now, optional trailing instructions to the summarizer; does not wait for the 60% occupancy governor; idle success shows the fold and does not start a new turn), `/mcp` (enable, disable, or remove servers), `/handoff [optional instructions]` (folds context through the shared operator pipeline, then immediately starts the next turn with the instructions as the inbound content — default copy when omitted; unlike `/compact`, which stops after the fold, handoff always re-infers, so the operator can pivot goals without `/clear`; a handoff issued mid-tool-batch queues behind the in-flight batch and whichever boundary fires first runs the single fold), and `/yolo` (`/yolo [on|off|toggle]`, bare `/yolo` toggles), plus a `/` command per available workflow. `/yolo` persists skip-permissions to the active settings file. That file is the user-global `~/.corbits/settings.json` by default, making the setting machine-wide; explicit `--config ` selects a different active file, and `/yolo` writes that file. An ordinary TUI launch without the same `--config` returns to the user-global source and does not modify the custom file. `--dangerously-skip-permissions` and `--yolo` are process-only aliases. Secret-guard and authz still apply. When a session starts with its active persisted setting already on, the TUI and `corbits exec` warn that permission prompts are disabled by saved settings at the active settings path and direct the operator to edit that file to re-enable them. Plugins can register additional commands. **Default skills** exist out of the gate as first-party slash **actions**, not director names: `/implement`, `/plan`, `/refactor`, `/review`, `/pull-request-review`, `/create-issue`, `/scribe`, `/interview`, `/ast-grep`, `/lexicon`. Each one is a how-to playbook — the slash sends the skill body to the primary, which follows the steps. Skills do not assign identity or route the fleet; that stays on director system prompts. `/review` classifies the target first, then dispatches a selected fleet; `/pull-request-review` is worktree checkout plus a surface pass, loading `/review` for quality rules only; `/scribe` is how to maintain PRODUCT / ARCHITECTURE / IMPLEMENTATION; `/implement` is the per-commit review/build/critique loop — it does not steal planning from `/plan`. Substantial Builder work consumes a counsel / `/plan` plan first; tiny parent-DIY stays plan-optional. `/plan` authors an eng change plan (files, AC, non-goals, risks, ordered steps) and does not implement. `/create-issue` remains the tracker command: Linear MCP when available; otherwise it `ask_operator`s for the platform (GitHub etc.) and persists `Preferred issue tracker` in `.corbits/MEMORY.md` (GitHub via `gh issue create`). `/lexicon` owns director-prompt drift and size against the agents repo at a pinned commit. There is no first-party dispatch skill — Skywalker orchestrates natively. `git-rebase`, `linear-issue-workflow`, `style`, `philosophy`, `native-integration`, `typescript`, `ponytail`, and `opsh` stay `use_skill` only (`user-invocable: false`). Bake-only bars such as `idiot-proof` and `native-runtime` are not slashes and are not listed for `use_skill`. Draper and emil are not slashes; they remain closed directors via `spawn_agent(agent=…)`. There is no catch-all worker. Slash names are also available to the model via `skill_search` (descriptions) then `use_skill` (body). Disable the catalog in `/plugins` (`corbits-skills`) if you want them gone. @@ -133,7 +133,7 @@ The exact turn threshold is model-family-dependent (tighter for models with obse **What the user sees:** In a non-interactive `corbits exec` run, a consequential action that needs approval returns a tool error explaining that approval is unavailable. -**Recovery:** Re-run interactively (TUI), pre-approve via persisted approvals, narrow the action, re-run with `--dangerously-skip-permissions`, or use `/yolo` in the TUI (persists as the user-global default). +**Recovery:** Re-run interactively (TUI), pre-approve via persisted approvals, narrow the action, re-run with process-only `--dangerously-skip-permissions` / `--yolo`, or use `/yolo` in the TUI to persist to the active settings file. ### Resume after interruption @@ -145,7 +145,7 @@ line instead of a path dump. ## Configuration -Providers and models are configured in `~/.corbits/settings.json` (holds providers + credentials), with a selection-only per-repo `.corbits/settings.json` override. Select at launch with `--provider` / `--model`, or point at an alternate file with `--config `. `--config` only overrides where provider _definitions_ come from; it composes with, rather than replaces, credentials for codex/xai OAuth-profile providers, which live in separate home-level auth stores (`~/.corbits/codex-auth.json`, `xai-auth.json`) and are merged into the catalog regardless of `--config`. Credentials are read only from these settings files and the OAuth auth stores — there is no environment-variable override and `.env` files are not loaded, so a stale or exported key can't shadow the configured provider. The agent is denied read access to both settings files. +Providers and models are configured in the active settings file, which defaults to `~/.corbits/settings.json` (providers + credentials), with a selection-only per-repo `.corbits/settings.json` override. Select at launch with `--provider` / `--model`, or use `--config ` to select an alternate active file for provider definitions and TUI settings persistence such as `/yolo`. `--config` composes with, rather than replaces, credentials for codex/xai OAuth-profile providers, which live in separate home-level auth stores (`~/.corbits/codex-auth.json`, `xai-auth.json`) and are merged into the catalog regardless of `--config`. Credentials are read only from these settings files and the OAuth auth stores — there is no environment-variable override and `.env` files are not loaded, so a stale or exported key can't shadow the configured provider. Static secret-guard protection denies path-keyed read access to the default user-global `~/.corbits/settings.json` and per-repo `.corbits/settings.json`. An arbitrary active path selected with `--config ` is not added to that denylist at runtime. ## Optional Capabilities (plugins) diff --git a/src/config.test.ts b/src/config.test.ts index a7a135134..24ecd9e32 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -1,6 +1,13 @@ import { defined } from "../tests/helpers/defined.js"; import { afterEach, beforeEach, describe, test, expect } from "bun:test"; -import { mkdtemp, mkdir, writeFile, rm, readdir } from "node:fs/promises"; +import { + mkdtemp, + mkdir, + readFile, + writeFile, + rm, + readdir, +} from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; @@ -1225,6 +1232,19 @@ describe("loadConfig", () => { await expectCliHelp(["-h"]); }); + test("--help explains process-only yolo and its precedence over auto", () => { + expect(CLI_HELP_TEXT).toContain("--dangerously-skip-permissions, --yolo"); + expect(CLI_HELP_TEXT).toContain("this process only (--yolo alias)"); + expect(CLI_HELP_TEXT).toContain("--auto --yolo uses yolo mode"); + expect(CLI_HELP_TEXT).toContain( + "/yolo in the TUI persists the active settings file", + ); + expect(CLI_HELP_TEXT).toContain( + "the default settings file is machine-wide", + ); + expect(CLI_HELP_TEXT).toContain("--config selects another source"); + }); + test("--help after flags throws CliHelpError", async () => { await expectCliHelp(["--auto", "--help"]); await expectCliHelp(["--auto", "-h"]); @@ -1349,6 +1369,28 @@ describe("loadConfig", () => { } }); + test("exec --auto --yolo enables process-only skip without changing settings", async () => { + const cwd = await emptyCwd(); + try { + const globalPath = await writeGlobalSettings(cwd); + const settingsBefore = await readFile(globalPath); + const config = await loadConfig( + ["exec", "--cwd", cwd, "--auto", "--yolo", "ship", "it"], + { globalSettingsPath: globalPath }, + ); + + assertConfigured(config); + expect(config.command).toBe("exec"); + expect(config.task).toBe("ship it"); + expect(config.auto).toBe(true); + expect(config.dangerouslySkipPermissions).toBe(true); + expect(config.skipPermissionsFromSettings).toBe(false); + expect(await readFile(globalPath)).toEqual(settingsBefore); + } finally { + await rm(cwd, { recursive: true, force: true }); + } + }); + test("seeds dangerouslySkipPermissions from global settings without the CLI flag", async () => { const cwd = await emptyCwd(); try { diff --git a/src/config/index.ts b/src/config/index.ts index cbcca2f94..c99dc9572 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -579,9 +579,9 @@ export interface Config { // Experimental prompt shrink after an Anthropic cache expiry. Off unless // settings set anthropicCachePrompt. anthropicCachePrompt: boolean; - // True when dangerouslySkipPermissions came from the persisted global - // default rather than this invocation's CLI flag. Entry points use this to - // surface a startup notice since the persisted default is otherwise silent. + // True when dangerouslySkipPermissions came from the active settings source + // rather than this invocation's CLI flag. Entry points use this to surface a + // startup notice since the persisted value is otherwise silent. skipPermissionsFromSettings: boolean; auto: boolean; /** @@ -594,6 +594,7 @@ export interface Config { * product agent path (`corbits exec "prompt"`). Same directors/tools/permissions. */ command: "tui" | "exec"; + /** Active settings source, including an explicit --config path. */ globalSettingsPath: string; globalDefaultProvider?: string; // Every provider available to switch to at runtime. From the settings file @@ -699,10 +700,12 @@ Flags: -p one-shot prompt (same as exec / run) --resume [] interactive picker, or reopen a session; with exec/-p the id is required --director exec-only: run as this director (default: skywalker) - --dangerously-skip-permissions - skip permission prompts for this run only; - /yolo in the TUI instead persists the default - machine-wide in ~/.corbits/settings.json + --dangerously-skip-permissions, --yolo + skip permission prompts for this process only (--yolo alias); + --auto --yolo uses yolo mode (catastrophic denials remain); + /yolo in the TUI persists the active settings file; + the default settings file is machine-wide; + --config selects another source --auto / --no-auto auto mode on/off --help, -h show this help `; @@ -734,7 +737,7 @@ export class CliUserError extends Error { } export interface LoadConfigOptions { - // Override the global settings file location (for tests / non-standard homes). + // Override the default settings source (for tests / non-standard homes). globalSettingsPath?: string; // Override the home directory used for project-key session roots (tests). // Production callers leave this unset so sessions resolve under ~/.corbits. @@ -883,7 +886,7 @@ export async function loadConfig( continue; } - if (arg === "--dangerously-skip-permissions") { + if (arg === "--dangerously-skip-permissions" || arg === "--yolo") { dangerouslySkipPermissions = true; continue; } @@ -950,8 +953,8 @@ export async function loadConfig( ...options.pricing, }); - // Resolve both settings targets from the same effective global path. The - // local schema must never be read from or written to that global target. + // Resolve both settings targets from the same active source. The local schema + // must never be read from or written to the active global-schema target. const effectiveSettingsPath = configPath ?? options.globalSettingsPath ?? globalSettingsPath(); const localSettingsFile = resolveLocalSettingsPath( @@ -995,7 +998,7 @@ export async function loadConfig( { persist: true }, ); - // Track whether the effective value came from the persisted global default + // Track whether the effective value came from the active settings source // rather than this invocation's --dangerously-skip-permissions flag, so the // TUI/exec entry points can surface a startup notice for the silent case. const skipPermissionsFromSettings = diff --git a/src/exec/runner.ts b/src/exec/runner.ts index 8757ae19c..0a7e42e48 100644 --- a/src/exec/runner.ts +++ b/src/exec/runner.ts @@ -67,6 +67,7 @@ import type { ApprovalOutcome, PermissionRequest, } from "../permission/types.js"; +import { savedSkipPermissionsWarning } from "../permission/saved-skip-warning.js"; import { createAgentToolset, type AgentToolset, @@ -598,7 +599,7 @@ export async function runExec(config: Config): Promise { if (config.skipPermissionsFromSettings) { stderr.write( - "Warning: permission prompts are disabled by your saved default (/yolo off to re-enable).\n", + `${savedSkipPermissionsWarning(config.globalSettingsPath)}\n`, ); } diff --git a/src/permission/permission.test.ts b/src/permission/permission.test.ts index 5fb6b946c..e44eda2d5 100644 --- a/src/permission/permission.test.ts +++ b/src/permission/permission.test.ts @@ -2401,6 +2401,28 @@ describe("createPermissionGate", () => { expect(asked).toBe(0); }); + test("skipPermissions overrides auto shell policy but not catastrophic denial", async () => { + let asked = 0; + const gate = createPermissionGate({ + approvals: [], + requestApproval: async () => { + asked++; + return { allow: false }; + }, + interactive: true, + skipPermissions: true, + reactorGated: false, + auto: true, + }); + + expect((await gate.evaluate(shellCall("echo hi > src/a.ts"))).allowed).toBe( + true, + ); + expect((await gate.evaluate(shellCall("cat .env"))).allowed).toBe(true); + expect((await gate.evaluate(shellCall("rm -rf /"))).allowed).toBe(false); + expect(asked).toBe(0); + }); + // SECURITY: headless (interactive=false, no requestApproval) with an unapproved // ask-tier tool must produce a hard denial. Silent allow would be catastrophic // because automated pipelines often run headless and must not silently gain @@ -3614,15 +3636,16 @@ describe("createPermissionGate restricted paths", () => { expect(asked).toBe(0); }); - // Auto-allowing gitignored reads at the gate does not widen what the model can - // see via path-keyed tools: the secret-guard plugin hard-blocks sensitive-file - // reads/writes independent of any gate decision. Shell commands that mention - // those paths are ask-gated instead (see classify-security tests). - test(".env reads are still hard-blocked by the secret-guard plugin even though the gate auto-allows gitignored reads", async () => { - const gate = restrictedGate(() => { - throw new Error( - "the plugin should block before the gate is ever consulted for approval", - ); + // Skip mode does not widen what the model can see via path-keyed tools: the + // secret-guard plugin hard-blocks sensitive-file reads/writes independent of + // the gate decision. + test(".env path reads remain hard-blocked by the secret-guard plugin under skipPermissions", async () => { + const gate = createPermissionGate({ + approvals: [], + cwd, + interactive: false, + skipPermissions: true, + reactorGated: false, }); const gateVerdict = await gate.evaluate({ id: "c", diff --git a/src/permission/saved-skip-warning.ts b/src/permission/saved-skip-warning.ts new file mode 100644 index 000000000..307357acf --- /dev/null +++ b/src/permission/saved-skip-warning.ts @@ -0,0 +1,5 @@ +export function savedSkipPermissionsWarning( + globalSettingsPath: string, +): string { + return `Warning: permission prompts are disabled by saved settings at ${globalSettingsPath}; edit that file to re-enable.`; +} diff --git a/src/tui/commands/built-in.test.ts b/src/tui/commands/built-in.test.ts index dad510051..5aafa1aa2 100644 --- a/src/tui/commands/built-in.test.ts +++ b/src/tui/commands/built-in.test.ts @@ -1,5 +1,13 @@ import { describe, it, expect } from "bun:test"; +import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; import { defined } from "../../../tests/helpers/defined.js"; +import { loadConfig } from "../../config/index.js"; +import { globalSettingsPath } from "../../config/settings.js"; +import { createCommandLayer } from "../runner/commands.js"; +import { createTUISettingsWriters } from "../runner/settings-writers.js"; +import type { RunnerServices, RunnerState } from "../runner/state.js"; import { getCommand } from "./registry.js"; import type { CommandContext } from "./registry.js"; import { registerBuiltInCommands } from "./built-in.js"; @@ -110,6 +118,76 @@ describe("/yolo command", () => { expect(getCommand("yolo")).toBeDefined(); }); + it("persists through the command layer to the active custom settings file", async () => { + const home = await mkdtemp(join(tmpdir(), "corbits-yolo-command-")); + const customSettingsPath = join(home, "custom-settings.json"); + const defaultSettingsPath = globalSettingsPath(home); + const defaultBytes = + '{\n "providers": {},\n "showPromptCost": false\n}\n'; + let skipPermissions = false; + + try { + await writeFile( + customSettingsPath, + JSON.stringify({ + defaultProvider: "test", + providers: { + test: { + baseURL: "https://example.test/v1", + apiKey: "test-key", + models: ["test-model"], + }, + }, + }), + ); + await mkdir(dirname(defaultSettingsPath), { recursive: true }); + await writeFile(defaultSettingsPath, defaultBytes); + const config = await loadConfig( + ["--cwd", home, "--config", customSettingsPath], + { + globalSettingsPath: defaultSettingsPath, + pricing: { + fetchImpl: (() => + Promise.reject(new Error("offline"))) as unknown as typeof fetch, + }, + }, + ); + expect(config.globalSettingsPath).toBe(customSettingsPath); + const { globalSettingsWriter } = createTUISettingsWriters(config); + const state = { + config, + host: { shell: { modelLabel: "test · test-model · yolo" } }, + } as unknown as RunnerState; + const services = { + globalSettingsWriter, + permissionGate: { + getSkipPermissions: () => skipPermissions, + setSkipPermissions: (value: boolean) => { + skipPermissions = value; + }, + }, + } as unknown as RunnerServices; + const { commandContext } = createCommandLayer(state, services); + + expect( + defined(getCommand("yolo"), "yolo").handler("on", commandContext), + ).toEqual({ + type: "message", + text: "Yolo mode on — permission prompts skipped. Saved as the default.", + }); + await globalSettingsWriter.enqueue(async () => undefined); + + expect( + JSON.parse(await readFile(customSettingsPath, "utf8")), + ).toMatchObject({ + dangerouslySkipPermissions: true, + }); + expect(await readFile(defaultSettingsPath, "utf8")).toBe(defaultBytes); + } finally { + await rm(home, { recursive: true, force: true }); + } + }); + it("toggles skip-permissions when invoked bare", () => { let skip = false; const ctx: CommandContext = { diff --git a/src/tui/commands/built-in.ts b/src/tui/commands/built-in.ts index 35b427365..4341bee0e 100644 --- a/src/tui/commands/built-in.ts +++ b/src/tui/commands/built-in.ts @@ -256,7 +256,7 @@ export function registerBuiltInCommands(): void { }, }); - // Persist as user-global default, not session-only. + // Persist to the active settings source, not only the running session. registerCommand({ name: "yolo", description: "Skip permission prompts (persists as the default)", diff --git a/src/tui/commands/registry.ts b/src/tui/commands/registry.ts index 5003ec895..67bb128e2 100644 --- a/src/tui/commands/registry.ts +++ b/src/tui/commands/registry.ts @@ -26,7 +26,7 @@ export interface CommandContext { beginFeedbackCapture?: () => void; /** Whether skip-permissions (yolo) is active for this session. */ getSkipPermissions?: () => boolean; - /** Live-flip skip-permissions and persist `/yolo` as the user-global default. */ + /** Live-flip skip-permissions and persist `/yolo` to the active settings source. */ setSkipPermissions?: (value: boolean) => void; /** * Fold conversation context now, bypassing the occupancy governor. diff --git a/src/tui/runner/session.ts b/src/tui/runner/session.ts index 5cfb73f61..542ee3818 100644 --- a/src/tui/runner/session.ts +++ b/src/tui/runner/session.ts @@ -12,16 +12,11 @@ import { join } from "node:path"; import { randomUUID } from "node:crypto"; import { EventEmitter } from "node:events"; import { - localSettingsPath, shellTimeoutFromSettings, toolWatchdogFromSettings, } from "../../config/settings.js"; import { isCodexProviderName } from "../../config/codex-providers.js"; import { peekSourceCredentialSecret } from "../../config/source-credentials.js"; -import { - createGlobalSettingsWriter, - createLocalSettingsWriter, -} from "../../mcp/add-server.js"; import { getProcessAdmissionQueue } from "../../subagent/admission.js"; import { createSubAgentSessionStore } from "../../subagent/index.js"; import { @@ -130,6 +125,7 @@ import { type TUIStart, } from "./state.js"; import { createParkedOverlayAbortBinding } from "./parked-overlay-abort.js"; +import { createTUISettingsWriters } from "./settings-writers.js"; export async function assembleTUISession( state: RunnerState, @@ -138,12 +134,8 @@ export async function assembleTUISession( ): Promise { const config = state.config; const emitter = new EventEmitter(); - const globalSettingsWriter = createGlobalSettingsWriter( - config.globalSettingsPath, - ); - const localSettingsWriter = createLocalSettingsWriter( - localSettingsPath(config.cwd), - ); + const { globalSettingsWriter, localSettingsWriter } = + createTUISettingsWriters(config); const initialHookEnabled: Record = Object.fromEntries( Object.entries(config.settings?.hooks ?? {}).map(([id, v]) => [ id, diff --git a/src/tui/runner/settings-writers.ts b/src/tui/runner/settings-writers.ts new file mode 100644 index 000000000..a0cf2e6c6 --- /dev/null +++ b/src/tui/runner/settings-writers.ts @@ -0,0 +1,20 @@ +import type { Config } from "../../config/index.js"; +import { localSettingsPath } from "../../config/settings.js"; +import { + createGlobalSettingsWriter, + createLocalSettingsWriter, +} from "../../mcp/add-server.js"; + +export function createTUISettingsWriters( + config: Pick, +): { + globalSettingsWriter: ReturnType; + localSettingsWriter: ReturnType; +} { + return { + globalSettingsWriter: createGlobalSettingsWriter(config.globalSettingsPath), + localSettingsWriter: createLocalSettingsWriter( + localSettingsPath(config.cwd), + ), + }; +} diff --git a/src/tui/runner/wiring.skip-permissions-warning.test.ts b/src/tui/runner/wiring.skip-permissions-warning.test.ts new file mode 100644 index 000000000..d5fa9962a --- /dev/null +++ b/src/tui/runner/wiring.skip-permissions-warning.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, test } from "bun:test"; +import { createAppShell } from "../shell/index.js"; +import { shellInternals } from "../shell/internals.js"; +import { withTestRenderer } from "../harness.js"; +import { surfaceSavedSkipPermissionsWarning } from "./wiring.js"; + +const OPTIONS = { + terminal: { columns: 80, rows: 24 }, + wireKeys: false, + run: "idle" as const, +}; + +async function surfacedWarning(globalSettingsPath: string): Promise { + return withTestRenderer(async (h) => { + const shell = createAppShell(h.renderer, OPTIONS); + try { + surfaceSavedSkipPermissionsWarning(shell, { + globalSettingsPath, + skipPermissionsFromSettings: true, + }); + return shellInternals(shell)?.landingDeferredRows.at(-1)?.text ?? ""; + } finally { + shell.dispose(); + } + }); +} + +describe("saved skip-permissions startup warning", () => { + test("identifies a custom config path without false default provenance", async () => { + const warning = await surfacedWarning("/tmp/custom-corbits-settings.json"); + + expect(warning).toContain("/tmp/custom-corbits-settings.json"); + expect(warning).toContain("edit that file to re-enable"); + expect(warning).not.toMatch(/machine-wide|saved default|\/yolo off/i); + }); + + test("identifies the default settings path", async () => { + const warning = await surfacedWarning( + "/home/operator/.corbits/settings.json", + ); + + expect(warning).toContain("/home/operator/.corbits/settings.json"); + expect(warning).toContain("edit that file to re-enable"); + }); +}); diff --git a/src/tui/runner/wiring.ts b/src/tui/runner/wiring.ts index 473aaead3..fd6aa0837 100644 --- a/src/tui/runner/wiring.ts +++ b/src/tui/runner/wiring.ts @@ -52,6 +52,7 @@ import { setEffortCycleHandler, setMentionSuggestionSource, setPromptRecognitionSource, + type AppShell, } from "../shell/internals.js"; import { setPromptModelLabel, @@ -77,6 +78,7 @@ import { type RunnerState, } from "./state.js"; import { LOG_NAMESPACE_ROOT } from "../../branding.js"; +import { savedSkipPermissionsWarning } from "../../permission/saved-skip-warning.js"; import { buildFleetDryContinuationMessage, buildMailboxMailMessage, @@ -84,6 +86,20 @@ import { const tuiLogger = getLogger([LOG_NAMESPACE_ROOT, "tui"]); +export function surfaceSavedSkipPermissionsWarning( + shell: AppShell, + config: Pick< + RunnerState["config"], + "globalSettingsPath" | "skipPermissionsFromSettings" + >, +): void { + if (!config.skipPermissionsFromSettings) return; + surfaceSystemNotice( + shell, + savedSkipPermissionsWarning(config.globalSettingsPath), + ); +} + /** * One tick of the periodic fleet stall poll. * @@ -590,12 +606,7 @@ export function wirePostStartup( // The persisted /yolo default is otherwise silent: nothing on screen would // otherwise tell the operator that permission prompts are off for a repo // they never ran --dangerously-skip-permissions or /yolo in. - if (state.config.skipPermissionsFromSettings) { - surfaceSystemNotice( - hostOf(state).shell, - "Permission prompts are disabled by your saved default (/yolo off to re-enable).", - ); - } + surfaceSavedSkipPermissionsWarning(hostOf(state).shell, state.config); // Soft upgrade check: never blocks startup; offline / rate-limit is a quiet skip. // surfaceSystemNotice keeps the landing hero up and flushes into the transcript diff --git a/tests/unit/exec/runner.test.ts b/tests/unit/exec/runner.test.ts index 7206f4be5..26149fabd 100644 --- a/tests/unit/exec/runner.test.ts +++ b/tests/unit/exec/runner.test.ts @@ -1,9 +1,10 @@ +import { writeFile } from "node:fs/promises"; import { join } from "node:path"; import { describe, expect, test } from "bun:test"; import type { AgentTool } from "@intx/agent"; import type { InferenceSource } from "@intx/types/runtime"; -import type { Config } from "../../../src/config/index.js"; +import { loadConfig, type Config } from "../../../src/config/index.js"; import { disposeExecRuntime, formatCaughtError, @@ -351,7 +352,7 @@ describe("runExec", () => { } }); - test("dispose failure after toolset exists is once-only and forces a nonzero exit", async () => { + test("dispose failure is once-only and warns with the settings source", async () => { const previous = getActiveRun(); clearActiveRun(); const { cwd, home, cleanup } = createTempDirs( @@ -416,24 +417,69 @@ describe("runExec", () => { async () => { const { runExec: runExecUnderMock } = await import("../../../src/exec/runner.js"); + const defaultSettingsPath = join(home, "settings.json"); const result = await runExecUnderMock({ ...bareConfig("do the thing"), cwd, sessionId, director: "builder", - globalSettingsPath: join(home, "settings.json"), + globalSettingsPath: defaultSettingsPath, providers: [], + skipPermissionsFromSettings: true, }); expect(result.exitCode).toBe(1); expect(result.status).toBe("failed"); expect(result.error).toMatch( /plugin dispose failed|runtime dispose failed/i, ); - expect(stderrChunks.join("")).toMatch( - /runtime dispose failed/i, + const stderrOutput = stderrChunks.join(""); + expect(stderrOutput).toContain( + `Warning: permission prompts are disabled by saved settings at ${defaultSettingsPath}; edit that file to re-enable.\n`, ); + expect(stderrOutput).not.toContain("/yolo"); + expect(stderrOutput).toMatch(/runtime dispose failed/i); expect(disposeCalls).toBe(1); expect(getActiveDisposeHost()).toBeNull(); + + stderrChunks.length = 0; + const customSettingsPath = join(home, "custom-settings.json"); + await writeFile( + customSettingsPath, + JSON.stringify({ + defaultProvider: "test", + providers: { + test: { + baseURL: "https://example.test/v1", + apiKey: "test-key", + models: ["test"], + }, + }, + dangerouslySkipPermissions: true, + }), + ); + const customConfig = await loadConfig([ + "exec", + "--cwd", + cwd, + "--config", + customSettingsPath, + "do the thing", + ]); + const customResult = await runExecUnderMock({ + ...customConfig, + sessionId: `${sessionId}-custom`, + director: "builder", + }); + expect(customResult.exitCode).toBe(1); + const customStderrOutput = stderrChunks.join(""); + expect(customStderrOutput).toContain( + `Warning: permission prompts are disabled by saved settings at ${customSettingsPath}; edit that file to re-enable.\n`, + ); + expect(customStderrOutput).not.toContain("machine-wide"); + expect(customStderrOutput).not.toContain("TUI"); + expect(customStderrOutput).not.toContain("/yolo"); + expect(disposeCalls).toBe(2); + expect(getActiveDisposeHost()).toBeNull(); }, ); }, From be9dca1d94258d43ad8034e7b48d91c97aaf2806 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 25 Sep 2026 07:45:51 -0700 Subject: [PATCH 2/4] test(reads): pin continuation recovery contract for truncated reads Red tests for CL-8980: verbatim handle-following must yield the next window across session resume, dead handles must name source plus offset instead of a bare missing blob, never-handles must stay missing-blob errors, compaction stubs must preserve the resume recipe, spent replays stay single-next-call errors, and guard denials stay isError results outside decline classification. --- .../read-file-continuation-recovery.test.ts | 317 ++++++++++++++++++ .../compaction-continuation-recipe.test.ts | 192 +++++++++++ 2 files changed, 509 insertions(+) create mode 100644 src/plugins/read-file-continuation-recovery.test.ts create mode 100644 src/session/compaction-continuation-recipe.test.ts diff --git a/src/plugins/read-file-continuation-recovery.test.ts b/src/plugins/read-file-continuation-recovery.test.ts new file mode 100644 index 000000000..3b1265256 --- /dev/null +++ b/src/plugins/read-file-continuation-recovery.test.ts @@ -0,0 +1,317 @@ +import { afterAll, beforeAll, describe, expect, test } from "bun:test"; +import { mkdtemp, rm, unlink, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import type { ToolCall, ToolResult } from "@intx/types/runtime"; +import { defined } from "../../tests/helpers/defined.js"; +import { + APPROVER_REJECTION_MARKER, + BLOCKED_BY_POLICY_PREFIX, + DENIED_BY_POLICY_MARKER, + NO_MATCHING_GRANTS_MARKER, + OPERATOR_DECLINED_MARKER, +} from "../permission/decline-markers.js"; +import type { PermissionGate } from "../permission/gate.js"; +import { gateToolCall } from "./permission-plugin.js"; +import { readFileGuardPlugin } from "./read-file-guard-plugin.js"; + +// CL-8980 RED: continuation recovery. Truncated reads mint a one-shot +// in-memory handle; after resume (fresh plugin instance), prune, or +// compaction the record is dropped and the URI is indistinguishable from a +// missing spill, with no source/offset named. These tests pin the recovery +// contract: verbatim handle-following yields the next window across resume, +// dead handles name source + offset (never a bare missing-blob), never-handles +// still fail as missing blobs, spent replays stay errors with one followable +// next call, and guard denials stay isError results (never throws, never a +// decline classification). + +const neverAbort = () => new AbortController().signal; + +let dir: string; + +beforeAll(async () => { + dir = await mkdtemp(join(tmpdir(), "read-continuation-8980-")); +}); + +afterAll(async () => { + await rm(dir, { recursive: true, force: true }); +}); + +async function fixture(name: string, content: string): Promise { + const p = join(dir, name); + await writeFile(p, content); + return p; +} + +const fallback = async (call: ToolCall): Promise => ({ + callId: call.id, + content: "FALLBACK", +}); + +function freshGuard(blobReader?: { + read: (uri: string) => Promise; +}) { + const plugin = readFileGuardPlugin( + dir, + blobReader !== undefined ? { blobReader } : {}, + ); + const middleware = defined(plugin.middleware)(fallback); + return (call: ToolCall) => middleware(call, neverAbort()); +} + +function extractHandle(content: string): string { + const match = /Use path="(tool-output:\/\/\/[^"]+)"/.exec(content); + expect(match).not.toBeNull(); + return (match as RegExpExecArray)[1] as string; +} + +function tenLines(name: string): string { + return Array.from({ length: 10 }, (_, i) => `${name}-line-${i}`).join("\n"); +} + +// Decline-classification markers the director matches by substring. No +// continuation isError may carry any of them, or a recoverable read failure +// would be misread as an operator decision (src/agent/director.ts must stay +// out of this path). +const DECLINE_MARKERS = [ + DENIED_BY_POLICY_MARKER, + NO_MATCHING_GRANTS_MARKER, + OPERATOR_DECLINED_MARKER, + APPROVER_REJECTION_MARKER, +]; + +function expectNotDeclined(content: string): void { + for (const marker of DECLINE_MARKERS) { + expect(content).not.toContain(marker); + } +} + +describe("CL-8980 continuation recovery (file source)", () => { + test("verbatim follow after session resume yields the next window", async () => { + const absolutePath = await fixture("resume.txt", tenLines("resume")); + const first = await freshGuard()({ + id: "r1", + name: "read_file", + arguments: { path: "resume.txt", limit: 4 }, + }); + expect(first.isError).toBeFalsy(); + const handle = extractHandle(String(first.content)); + + // A resumed session rebuilds the plugin: brand-new instance, empty + // in-memory cursor map. Following the notice verbatim must still yield + // the next window, not a missing-blob dead end. + const resumed = await freshGuard()({ + id: "r2", + name: "read_file", + arguments: { path: handle }, + }); + expect(resumed.isError).toBeFalsy(); + expect(String(resumed.content)).toContain("resume-line-4"); + expect(String(resumed.content)).not.toContain(absolutePath); + expectNotDeclined(String(resumed.content)); + }); + + test("verbatim follow chains across a second hop after resume", async () => { + await fixture("chain.txt", tenLines("chain")); + const first = await freshGuard()({ + id: "c1", + name: "read_file", + arguments: { path: "chain.txt", limit: 4 }, + }); + const handle1 = extractHandle(String(first.content)); + + const second = await freshGuard()({ + id: "c2", + name: "read_file", + arguments: { path: handle1 }, + }); + expect(second.isError).toBeFalsy(); + expect(String(second.content)).toContain("chain-line-4"); + const handle2 = extractHandle(String(second.content)); + expect(handle2).not.toBe(handle1); + + const third = await freshGuard()({ + id: "c3", + name: "read_file", + arguments: { path: handle2 }, + }); + expect(third.isError).toBeFalsy(); + expect(String(third.content)).toContain("chain-line-8"); + }); + + test("dead file handle names the source and offset, never a bare missing blob", async () => { + const absolutePath = await fixture("gone.txt", tenLines("gone")); + const first = await freshGuard()({ + id: "d1", + name: "read_file", + arguments: { path: "gone.txt", limit: 4 }, + }); + const handle = extractHandle(String(first.content)); + await unlink(absolutePath); + + const dead = await freshGuard()({ + id: "d2", + name: "read_file", + arguments: { path: handle }, + }); + expect(dead.isError).toBe(true); + const text = String(dead.content); + expect(text).toContain(absolutePath); + expect(text).toMatch(/offset=4\b/); + expect(text).not.toContain("Blob not found"); + expect(text).not.toContain("no blob reader is configured"); + expectNotDeclined(text); + }); + + test("spent-handle replay stays an error with one followable next call", async () => { + const absolutePath = await fixture("spent.txt", tenLines("spent")); + const plugin = readFileGuardPlugin(dir, {}); + const middleware = defined(plugin.middleware)(fallback); + const first = await middleware( + { id: "s1", name: "read_file", arguments: { path: "spent.txt", limit: 4 } }, + neverAbort(), + ); + const handle = extractHandle(String(first.content)); + const second = await middleware( + { id: "s2", name: "read_file", arguments: { path: handle } }, + neverAbort(), + ); + expect(second.isError).toBeFalsy(); + + const replay = await middleware( + { id: "s3", name: "read_file", arguments: { path: handle } }, + neverAbort(), + ); + expect(replay.isError).toBe(true); + const text = String(replay.content); + expect(text).toContain("already used"); + expect(text).toContain(absolutePath); + expect(text).toMatch(/offset=4\b/); + // Exactly one followable next call, not a menu of guesses. + expect(text.match(/offset=/g)).toHaveLength(1); + expectNotDeclined(text); + }); +}); + +describe("CL-8980 continuation recovery (blob source)", () => { + const enc = new TextEncoder(); + const rows = Array.from({ length: 8_000 }, (_, i) => `row-${i}`).join("\n"); + + test("verbatim follow after resume yields the next window without the old map", async () => { + const store = new Map([["spill-resume", enc.encode(rows)]]); + const opening = freshGuard({ + read: async (uri: string) => { + const key = uri.slice("tool-output:///".length); + const bytes = store.get(key); + if (bytes === undefined) throw new Error(`Blob not found for key: ${uri}`); + return bytes; + }, + }); + const first = await opening({ + id: "b1", + name: "read_file", + arguments: { path: "tool-output:///spill-resume", limit: 5 }, + }); + expect(first.isError).toBeFalsy(); + const handle = extractHandle(String(first.content)); + + // Resumed session: new plugin instance, same durable spill store. + const resumed = freshGuard({ + read: async (uri: string) => { + const key = uri.slice("tool-output:///".length); + const bytes = store.get(key); + if (bytes === undefined) throw new Error(`Blob not found for key: ${uri}`); + return bytes; + }, + }); + const second = await resumed({ + id: "b2", + name: "read_file", + arguments: { path: handle }, + }); + expect(second.isError).toBeFalsy(); + expect(String(second.content)).toContain("row-5"); + }); + + test("dead spill handle names the spill URI and offset, never a bare missing blob", async () => { + const live = new Map([["spill-dead", enc.encode(rows)]]); + const opening = freshGuard({ + read: async (uri: string) => { + const key = uri.slice("tool-output:///".length); + const bytes = live.get(key); + if (bytes === undefined) throw new Error(`Blob not found for key: ${uri}`); + return bytes; + }, + }); + const first = await opening({ + id: "e1", + name: "read_file", + arguments: { path: "tool-output:///spill-dead", limit: 5 }, + }); + const handle = extractHandle(String(first.content)); + + // The spill is gone (pruned store) by the time the handle is followed. + const pruned = freshGuard({ + read: async (uri: string) => { + throw new Error(`Blob not found for key: ${uri}`); + }, + }); + const dead = await pruned({ + id: "e2", + name: "read_file", + arguments: { path: handle }, + }); + expect(dead.isError).toBe(true); + const text = String(dead.content); + expect(text).toContain("tool-output:///spill-dead"); + expect(text).toMatch(/offset=\d+\b/); + expect(text).not.toMatch(/Blob not found for key: tool-output:\/\/\/[0-9a-f-]+/); + expectNotDeclined(text); + }); + + test("a URI that was never a continuation handle still fails as a missing blob", async () => { + const result = await freshGuard({ + read: async (uri: string) => { + throw new Error(`Blob not found for key: ${uri}`); + }, + })({ + id: "u1", + name: "read_file", + arguments: { path: "tool-output:///never-minted" }, + }); + expect(result.isError).toBe(true); + expect(String(result.content)).toContain("Blob not found for key"); + expect(String(result.content)).not.toContain("already used"); + expectNotDeclined(String(result.content)); + }); +}); + +describe("CL-8980 guard-denied continuation stays isError (never throws)", () => { + test("a denied continuation follow returns isError with the reason", async () => { + const gate = { + isReactorGated: () => false, + evaluate: async () => ({ + allowed: false as const, + reason: "test policy: cursor follows need approval", + }), + } as unknown as PermissionGate; + const call: ToolCall = { + id: "g1", + name: "read_file", + arguments: { path: "tool-output:///cursor-deadbeef" }, + }; + let result: ToolResult | undefined; + await expect( + (async () => { + result = await gateToolCall(gate, call, neverAbort(), async () => { + throw new Error("must not reach the tool when denied"); + }); + })(), + ).resolves.toBeUndefined(); + expect(defined(result).isError).toBe(true); + expect(String(defined(result).content)).toContain( + `${BLOCKED_BY_POLICY_PREFIX}test policy: cursor follows need approval`, + ); + expectNotDeclined(String(defined(result).content)); + }); +}); diff --git a/src/session/compaction-continuation-recipe.test.ts b/src/session/compaction-continuation-recipe.test.ts new file mode 100644 index 000000000..2e93bd135 --- /dev/null +++ b/src/session/compaction-continuation-recipe.test.ts @@ -0,0 +1,192 @@ +import { describe, expect, test } from "bun:test"; +import type { + ConversationTurn, + ReactorState, + StrategyContext, +} from "@intx/types/runtime"; +import { createPruningCompactor } from "./compactor.js"; + +// CL-8980 RED: compaction must not delete the only working resume recipe for +// an unfinished large read. When the same file is read twice among the kept +// turns, the older result is stubbed — but if that older result carries a +// truncated-read continuation notice (an unconsumed cursor), the stub must +// preserve the resume recipe (handle, or source + offset). Today the stub +// drops it, leaving the model with no way forward. + +const mockStrategyCtx: StrategyContext = { + state: {} as ReactorState, + trigger: "test", +}; + +function makeTurn( + overrides: Partial & { role: ConversationTurn["role"] }, +): ConversationTurn { + return { + content: [{ type: "text", text: "" }], + timestamp: Date.now(), + ...overrides, + }; +} + +const CURSOR_HANDLE = "tool-output:///cursor-aaaabbbbccccdddd"; + +function truncatedResultBody(): string { + return [ + "huge-line-0", + "huge-line-1", + "huge-line-2", + "huge-line-3", + "", + `[Showing lines 1-4; stopped at the 4-line limit. Use path="${CURSOR_HANDLE}" (same tool, no offset needed) — not the original path. Safe to retry after any truncation warning.]`, + ].join("\n"); +} + +function identicalReadPair(): ConversationTurn[] { + const first: ConversationTurn[] = [ + makeTurn({ + role: "assistant", + content: [ + { + type: "tool_call", + id: "old-read", + name: "read_file", + arguments: { path: "huge.txt", limit: 4 }, + }, + ], + }), + makeTurn({ + role: "user", + content: [ + { + type: "tool_result", + callId: "old-read", + content: [{ type: "text", text: truncatedResultBody() }], + }, + ], + }), + ]; + const second: ConversationTurn[] = [ + makeTurn({ + role: "assistant", + content: [ + { + type: "tool_call", + id: "new-read", + name: "read_file", + arguments: { path: "huge.txt", limit: 4 }, + }, + ], + }), + makeTurn({ + role: "user", + content: [ + { + type: "tool_result", + callId: "new-read", + content: [{ type: "text", text: truncatedResultBody() }], + }, + ], + }), + ]; + return [...first, ...second]; +} + +function turnText(turn: ConversationTurn): string { + return turn.content + .map((block) => + block.type === "text" + ? block.text + : block.type === "tool_result" + ? block.content.map((c) => (c.type === "text" ? c.text : "")).join("") + : "", + ) + .join(""); +} + +describe("CL-8980 compaction preserves the resume recipe", () => { + // Both identical reads sit inside the kept recent window (so the older + // stubs) while plain filler turns ahead of them summarize away (so the + // fold actually runs instead of no-op'ing). + function auditTranscript(): ConversationTurn[] { + return [ + makeTurn({ + role: "user", + content: [ + { type: "text", text: "goal: audit the huge export file end to end" }, + ], + }), + makeTurn({ + role: "user", + content: [{ type: "text", text: "background note one for the audit" }], + }), + makeTurn({ + role: "assistant", + content: [{ type: "text", text: "background note two for the audit" }], + }), + makeTurn({ + role: "user", + content: [{ type: "text", text: "background note three, then start" }], + }), + ...identicalReadPair(), + makeTurn({ + role: "user", + content: [{ type: "text", text: "noted, keep going with the audit" }], + }), + makeTurn({ + role: "user", + content: [{ type: "text", text: "keep going with the audit" }], + }), + makeTurn({ + role: "assistant", + content: [{ type: "text", text: "continuing the audit" }], + }), + makeTurn({ + role: "user", + content: [{ type: "text", text: "anything else in the export?" }], + }), + makeTurn({ + role: "assistant", + content: [{ type: "text", text: "still auditing" }], + }), + ]; + } + + test("stubbing a superseded truncated read keeps the continuation handle", async () => { + const compactor = createPruningCompactor({ + keepRecentTurns: 8, + summaryMaxChars: 4000, + maxAnchorTurns: 2, + }); + const applied = await compactor.apply(auditTranscript(), mockStrategyCtx); + expect(applied.record.decisions.supersededReadCount).toBe(1); + + const oldResult = applied.output.find((turn) => + turn.content.some( + (block) => block.type === "tool_result" && block.callId === "old-read", + ), + ); + expect(oldResult).toBeDefined(); + const stub = turnText(oldResult as ConversationTurn); + expect(stub).not.toContain("huge-line-0"); + // The only working resume recipe for this unfinished read must survive. + expect(stub).toContain(CURSOR_HANDLE); + }); + + test("the newest truncated read stays whole so its notice keeps working", async () => { + const compactor = createPruningCompactor({ + keepRecentTurns: 8, + summaryMaxChars: 4000, + maxAnchorTurns: 2, + }); + const applied = await compactor.apply(auditTranscript(), mockStrategyCtx); + const newResult = applied.output.find((turn) => + turn.content.some( + (block) => block.type === "tool_result" && block.callId === "new-read", + ), + ); + expect(newResult).toBeDefined(); + const body = turnText(newResult as ConversationTurn); + expect(body).toContain("huge-line-3"); + expect(body).toContain(CURSOR_HANDLE); + }); +}); From d34ea6f96f9207fe071d3af48862bf59cbafaf86 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 25 Sep 2026 07:58:39 -0700 Subject: [PATCH 3/4] feat(reads): make truncated-read continuations durable across resume Truncated reads minted opaque one-shot handles that died with the plugin instance: after resume, prune, or compaction the URI was a bare missing blob with no source or offset. Handles are now self-describing (source, offset, window, nonce ride in the URI), so a verbatim follow serves the next window with no in-memory record; dead handles fail as isError naming source plus offset; never-handles still fail as missing blobs; spent replays stay single-use errors; notices carry a plain path plus offset fallback; compaction stubs preserve resume recipes. --- .../read-file-continuation-recovery.test.ts | 41 +++- src/plugins/read-file-guard-plugin.test.ts | 11 +- src/plugins/read-file-guard-plugin.ts | 207 ++++++++++++++++-- src/session/compactor.ts | 20 +- src/util/tool-output-uri.ts | 102 +++++++++ 5 files changed, 343 insertions(+), 38 deletions(-) diff --git a/src/plugins/read-file-continuation-recovery.test.ts b/src/plugins/read-file-continuation-recovery.test.ts index 3b1265256..b816fdf55 100644 --- a/src/plugins/read-file-continuation-recovery.test.ts +++ b/src/plugins/read-file-continuation-recovery.test.ts @@ -99,15 +99,19 @@ describe("CL-8980 continuation recovery (file source)", () => { // A resumed session rebuilds the plugin: brand-new instance, empty // in-memory cursor map. Following the notice verbatim must still yield - // the next window, not a missing-blob dead end. + // the next window, not a missing-blob dead end. Same window size so the + // read stays paged and mints the next handle. const resumed = await freshGuard()({ id: "r2", name: "read_file", - arguments: { path: handle }, + arguments: { path: handle, limit: 4 }, }); expect(resumed.isError).toBeFalsy(); expect(String(resumed.content)).toContain("resume-line-4"); - expect(String(resumed.content)).not.toContain(absolutePath); + // CL-8980 keeps a plain path+offset fallback alongside the handle so a + // lost handle is never a dead end; verbatim follows still use the handle. + expect(String(resumed.content)).toContain(`path="${absolutePath}"`); + expect(String(resumed.content)).not.toMatch(/Use path="[^"]*resume\.txt"/); expectNotDeclined(String(resumed.content)); }); @@ -123,7 +127,7 @@ describe("CL-8980 continuation recovery (file source)", () => { const second = await freshGuard()({ id: "c2", name: "read_file", - arguments: { path: handle1 }, + arguments: { path: handle1, limit: 4 }, }); expect(second.isError).toBeFalsy(); expect(String(second.content)).toContain("chain-line-4"); @@ -133,7 +137,7 @@ describe("CL-8980 continuation recovery (file source)", () => { const third = await freshGuard()({ id: "c3", name: "read_file", - arguments: { path: handle2 }, + arguments: { path: handle2, limit: 4 }, }); expect(third.isError).toBeFalsy(); expect(String(third.content)).toContain("chain-line-8"); @@ -168,7 +172,11 @@ describe("CL-8980 continuation recovery (file source)", () => { const plugin = readFileGuardPlugin(dir, {}); const middleware = defined(plugin.middleware)(fallback); const first = await middleware( - { id: "s1", name: "read_file", arguments: { path: "spent.txt", limit: 4 } }, + { + id: "s1", + name: "read_file", + arguments: { path: "spent.txt", limit: 4 }, + }, neverAbort(), ); const handle = extractHandle(String(first.content)); @@ -198,12 +206,15 @@ describe("CL-8980 continuation recovery (blob source)", () => { const rows = Array.from({ length: 8_000 }, (_, i) => `row-${i}`).join("\n"); test("verbatim follow after resume yields the next window without the old map", async () => { - const store = new Map([["spill-resume", enc.encode(rows)]]); + const store = new Map([ + ["spill-resume", enc.encode(rows)], + ]); const opening = freshGuard({ read: async (uri: string) => { const key = uri.slice("tool-output:///".length); const bytes = store.get(key); - if (bytes === undefined) throw new Error(`Blob not found for key: ${uri}`); + if (bytes === undefined) + throw new Error(`Blob not found for key: ${uri}`); return bytes; }, }); @@ -220,7 +231,8 @@ describe("CL-8980 continuation recovery (blob source)", () => { read: async (uri: string) => { const key = uri.slice("tool-output:///".length); const bytes = store.get(key); - if (bytes === undefined) throw new Error(`Blob not found for key: ${uri}`); + if (bytes === undefined) + throw new Error(`Blob not found for key: ${uri}`); return bytes; }, }); @@ -234,12 +246,15 @@ describe("CL-8980 continuation recovery (blob source)", () => { }); test("dead spill handle names the spill URI and offset, never a bare missing blob", async () => { - const live = new Map([["spill-dead", enc.encode(rows)]]); + const live = new Map([ + ["spill-dead", enc.encode(rows)], + ]); const opening = freshGuard({ read: async (uri: string) => { const key = uri.slice("tool-output:///".length); const bytes = live.get(key); - if (bytes === undefined) throw new Error(`Blob not found for key: ${uri}`); + if (bytes === undefined) + throw new Error(`Blob not found for key: ${uri}`); return bytes; }, }); @@ -265,7 +280,9 @@ describe("CL-8980 continuation recovery (blob source)", () => { const text = String(dead.content); expect(text).toContain("tool-output:///spill-dead"); expect(text).toMatch(/offset=\d+\b/); - expect(text).not.toMatch(/Blob not found for key: tool-output:\/\/\/[0-9a-f-]+/); + expect(text).not.toMatch( + /Blob not found for key: tool-output:\/\/\/[0-9a-f-]+/, + ); expectNotDeclined(text); }); diff --git a/src/plugins/read-file-guard-plugin.test.ts b/src/plugins/read-file-guard-plugin.test.ts index 5a3e920fe..65682403a 100644 --- a/src/plugins/read-file-guard-plugin.test.ts +++ b/src/plugins/read-file-guard-plugin.test.ts @@ -507,8 +507,15 @@ describe("readFileGuardPlugin", () => { ); expect(result.content).not.toContain("Use offset="); expect(String(result.content)).toContain('Use path="tool-output:///'); - // The literal source path never reappears as the thing to read next. - expect(String(result.content)).not.toContain("many-lines.txt"); + // CL-8980 keeps a plain path+offset fallback alongside the handle so a + // lost handle is never a dead end — but the primary next call is the + // handle, never the original path. + expect(String(result.content)).toMatch( + /\(Fallback: read_file path="[^"]*many-lines\.txt" offset=4\.\)/, + ); + expect(String(result.content)).not.toMatch( + /Use path="[^"]*many-lines\.txt"/, + ); }); test("following the minted cursor resumes and eventually reads a large file to completion without any repeat call on the original path (CL-6961)", async () => { diff --git a/src/plugins/read-file-guard-plugin.ts b/src/plugins/read-file-guard-plugin.ts index e5f556d83..cb0bcf87d 100644 --- a/src/plugins/read-file-guard-plugin.ts +++ b/src/plugins/read-file-guard-plugin.ts @@ -8,8 +8,11 @@ import type { ToolPlugin } from "@intx/tools-posix"; import type { BlobReader } from "@intx/types/runtime"; import { canonicalToolOutputUri, + decodeResumeCursor, + encodeResumeCursor, isToolOutputLike, TOOL_OUTPUT_URI_PREFIX, + type ResumeCursor, } from "../util/tool-output-uri.js"; import { formatReadFileTimeoutMessage } from "./tool-time-budget.js"; @@ -58,6 +61,11 @@ export interface ReadFileGuardPluginOptions { // promise result-truncation-plugin.ts's comment forbids, since nothing here // claims discarded bytes are retrievable; it just remembers where to resume // a fresh bounded read. +// +// Handles are self-describing (CL-8980): the URI embeds the resume recipe, +// so following it needs no in-memory record and survives session resume, +// prune, and compaction. The per-instance map below only tracks which handles +// this instance already served, to keep the single-use replay contract. type ReadCursor = | { kind: "file"; absolutePath: string; offset: number; consumed: boolean } | { kind: "blob"; uri: string; offset: number; consumed: boolean }; @@ -86,13 +94,24 @@ function mintCursor( source: | { kind: "file"; absolutePath: string } | { kind: "blob"; uri: string }, + windowLimit: number, ): string { const match = CONTINUE_OFFSET_RE.exec(content); if (match === null) return content; const offset = Number(match[1]); - const cursorId = randomUUID(); + const resume: ResumeCursor = { + source: + source.kind === "file" + ? { kind: "file", path: source.absolutePath } + : { kind: "blob", uri: source.uri }, + offset, + limit: windowLimit, + nonce: randomUUID(), + }; + const handle = encodeResumeCursor(resume); + const cursorKey = handle.slice(`${TOOL_OUTPUT_URI_PREFIX}///`.length); cursors.set( - cursorId, + cursorKey, source.kind === "file" ? { kind: "file", @@ -103,9 +122,11 @@ function mintCursor( : { kind: "blob", uri: source.uri, offset, consumed: false }, ); pruneCursorHistory(cursors); + const fallbackSource = + source.kind === "file" ? source.absolutePath : source.uri; return content.replace( CONTINUE_OFFSET_RE, - `Use path="${TOOL_OUTPUT_URI_PREFIX}///${cursorId}" (same tool, no offset needed) to continue reading the remainder — a fresh, working handle, not the original path.]`, + `Use path="${handle}" (same tool, no offset needed) to continue reading the remainder — a fresh, working handle, not the original path. Safe to retry after any truncation warning. (Fallback: read_file path="${displaySource(fallbackSource)}" offset=${offset}.)]`, ); } @@ -133,6 +154,24 @@ function staleCursorMessage(cursor: ReadCursor): string { ); } +/** + * Message for a continuation handle whose source can no longer be re-read + * (file deleted, spill pruned, no blob reader). Never a bare missing-blob: + * it names the original source and the exact offset so recovery stays one + * targeted call. + */ +function deadCursorMessage( + source: string, + offset: number, + cause: unknown, +): string { + const detail = cause instanceof Error ? cause.message : String(cause); + return ( + `this read_file continuation handle expired before its source could be re-read (${detail}). ` + + `Resume with read_file, path="${displaySource(source)}", offset=${offset}.` + ); +} + function numArg(value: unknown): number | undefined { return typeof value === "number" && Number.isFinite(value) ? value @@ -456,9 +495,11 @@ export function readFileGuardPlugin( options: ReadFileGuardPluginOptions = {}, ): ToolPlugin { const { blobReader } = options; - // Single-use resumption pointers minted by mintCursor(); scoped to this - // plugin instance (one per session/agent, per buildCorePosixToolPlugins), so - // it never outlives the session and never crosses sessions. + // Single-use resumption pointers minted by mintCursor(); the map is scoped + // to this plugin instance (one per session/agent, per + // buildCorePosixToolPlugins). The handles themselves are self-describing + // (CL-8980), so they outlive the session — the map only tracks which ones + // this instance already served, to keep the single-use replay contract. const cursors = new Map(); const cursorUriPrefix = `${TOOL_OUTPUT_URI_PREFIX}///`; return { @@ -513,10 +554,15 @@ export function readFileGuardPlugin( ? { callId: call.id, content: res.content, isError: true } : { callId: call.id, - content: mintCursor(res.content, cursors, { - kind: "file", - absolutePath: cursor.absolutePath, - }), + content: mintCursor( + res.content, + cursors, + { + kind: "file", + absolutePath: cursor.absolutePath, + }, + limit, + ), }; } if (blobReader === undefined) { @@ -538,15 +584,120 @@ export function readFileGuardPlugin( ? { callId: call.id, content: res.content, isError: true } : { callId: call.id, - content: mintCursor(res.content, cursors, { - kind: "blob", - uri: cursor.uri, - }), + content: mintCursor( + res.content, + cursors, + { + kind: "blob", + uri: cursor.uri, + }, + blobLimit, + ), }; } catch (err) { + if (signal.aborted) { + return { + callId: call.id, + content: err instanceof Error ? err.message : String(err), + isError: true, + }; + } + // The record survived but the source did not (file deleted, spill + // pruned): a dead handle, not a missing blob — name source+offset. + const knownSource = + cursor.kind === "file" ? cursor.absolutePath : cursor.uri; + return { + callId: call.id, + content: deadCursorMessage(knownSource, cursor.offset, err), + isError: true, + }; + } + } + + const resumed = decodeResumeCursor(uri); + if (resumed !== undefined) { + // Self-describing handle with no in-memory record here: a resumed + // session, a pruned map, or a compacted transcript. The recipe rides + // in the handle, so serve it exactly as a known cursor would — and + // mark it consumed on success so a verbatim re-follow is the same + // stale-cursor error as a spent handle, not a second serving. + const resumedSource = + resumed.source.kind === "file" + ? resumed.source.path + : resumed.source.uri; + const resumedKey = uri.slice(cursorUriPrefix.length); + try { + signal.throwIfAborted(); + if (resumed.source.kind === "file") { + const res = await readFileBounded( + resumed.source.path, + resumed.offset, + limit, + signal, + ); + if (res.isError) { + return { callId: call.id, content: res.content, isError: true }; + } + cursors.set(resumedKey, { + kind: "file", + absolutePath: resumed.source.path, + offset: resumed.offset, + consumed: true, + }); + pruneCursorHistory(cursors); + return { + callId: call.id, + content: mintCursor( + res.content, + cursors, + { + kind: "file", + absolutePath: resumed.source.path, + }, + limit, + ), + }; + } + if (blobReader === undefined) { + throw new Error( + `cannot read ${resumedSource}: no blob reader is configured for tool-output spills`, + ); + } + const bytes = await blobReader.read(resumed.source.uri); + const res = await readBytesBounded( + bytes, + resumed.offset, + blobLimit, + signal, + resumed.source.uri, + ); + if (res.isError) { + return { callId: call.id, content: res.content, isError: true }; + } + cursors.set(resumedKey, { + kind: "blob", + uri: resumed.source.uri, + offset: resumed.offset, + consumed: true, + }); + pruneCursorHistory(cursors); + return { + callId: call.id, + content: mintCursor( + res.content, + cursors, + { + kind: "blob", + uri: resumed.source.uri, + }, + blobLimit, + ), + }; + } catch (err) { + if (signal.aborted) throw err; return { callId: call.id, - content: err instanceof Error ? err.message : String(err), + content: deadCursorMessage(resumedSource, resumed.offset, err), isError: true, }; } @@ -573,10 +724,15 @@ export function readFileGuardPlugin( ? { callId: call.id, content: res.content, isError: true } : { callId: call.id, - content: mintCursor(res.content, cursors, { - kind: "blob", - uri, - }), + content: mintCursor( + res.content, + cursors, + { + kind: "blob", + uri, + }, + blobLimit, + ), }; } catch (err) { return { @@ -602,10 +758,15 @@ export function readFileGuardPlugin( ? { callId: call.id, content: res.content, isError: true } : { callId: call.id, - content: mintCursor(res.content, cursors, { - kind: "file", - absolutePath, - }), + content: mintCursor( + res.content, + cursors, + { + kind: "file", + absolutePath, + }, + limit, + ), }; } catch (err) { return { diff --git a/src/session/compactor.ts b/src/session/compactor.ts index 325c7b226..40960829f 100644 --- a/src/session/compactor.ts +++ b/src/session/compactor.ts @@ -672,7 +672,25 @@ function buildResultStub( const spillHint = path.startsWith("tool-output://") ? " Re-read with read_file offset/limit or grep on that URI." : ""; - return `[${name} ${path} — ${size} chars omitted from context; source unchanged.${spillHint}]`; + // A stubbed truncated read may carry the only working resume recipe for + // an unfinished large read (CL-8980): continuation handles and spill URIs + // live in the body being hollowed, so they ride the stub instead. + const bodyText = block.content + .filter((c) => c.type === "text") + .map((c) => c.text) + .join("\n"); + const recipes = Array.from( + new Set( + Array.from(bodyText.matchAll(/tool-output:\/\/\/[^\s"\]]+/g)).map((m) => + m[0].replace(/[.,;)\]]+$/, ""), + ), + ), + ).filter((recipe) => recipe.length > "tool-output:///".length); + const resume = + recipes.length > 0 + ? ` Resume: ${recipes.map((recipe) => `Use path="${recipe}" (same tool, no offset needed).`).join(" ")}` + : ""; + return `[${name} ${path} — ${size} chars omitted from context; source unchanged.${spillHint}${resume}]`; } return `[${name} — ${size} chars, omitted]`; } diff --git a/src/util/tool-output-uri.ts b/src/util/tool-output-uri.ts index 3b7a383a0..707a0141e 100644 --- a/src/util/tool-output-uri.ts +++ b/src/util/tool-output-uri.ts @@ -24,3 +24,105 @@ export function canonicalToolOutputUri(path: string): string | undefined { if (callId.length === 0) return undefined; return normalized; } + +// Self-describing read_file continuation handles (CL-8980). A truncated read +// mints `tool-output:///cursor/` instead of an opaque UUID: the +// handle itself carries the resume recipe (source + next offset + window +// limit + nonce), so following it needs no in-memory record and keeps working +// across session resume, prune, and compaction. The nonce keeps every mint a +// distinct one-shot even for identical windows, preserving spent-handle +// replay semantics. +export const CURSOR_HANDLE_PREFIX = "tool-output:///cursor/"; + +export type ResumeSource = + | { kind: "file"; path: string } + | { kind: "blob"; uri: string }; + +export interface ResumeCursor { + source: ResumeSource; + offset: number; + limit: number; + nonce: string; +} + +const CURSOR_CODEC_VERSION = 1; + +export function encodeResumeCursor(cursor: ResumeCursor): string { + const payload = JSON.stringify({ + v: CURSOR_CODEC_VERSION, + source: cursor.source, + offset: cursor.offset, + limit: cursor.limit, + nonce: cursor.nonce, + }); + return `${CURSOR_HANDLE_PREFIX}${Buffer.from(payload, "utf8").toString("base64url")}`; +} + +/** Decode a self-describing handle. Never throws: anything malformed (or any + * older opaque handle / never-a-handle URI) decodes to undefined. */ +export function decodeResumeCursor( + canonical: string | undefined, +): ResumeCursor | undefined { + try { + if (canonical === undefined) return undefined; + if (!canonical.startsWith(CURSOR_HANDLE_PREFIX)) return undefined; + const payload = JSON.parse( + Buffer.from( + canonical.slice(CURSOR_HANDLE_PREFIX.length), + "base64url", + ).toString("utf8"), + ) as { + v?: unknown; + source?: unknown; + offset?: unknown; + limit?: unknown; + nonce?: unknown; + }; + if (payload.v !== CURSOR_CODEC_VERSION) return undefined; + const { source, offset, limit, nonce } = payload; + if ( + typeof offset !== "number" || + !Number.isInteger(offset) || + offset < 0 || + typeof limit !== "number" || + !Number.isInteger(limit) || + limit <= 0 || + typeof nonce !== "string" || + nonce.length === 0 + ) { + return undefined; + } + if (typeof source !== "object" || source === null || !("kind" in source)) { + return undefined; + } + if ( + source.kind === "file" && + "path" in source && + typeof source.path === "string" && + source.path.length > 0 + ) { + return { + source: { kind: "file", path: source.path }, + offset, + limit, + nonce, + }; + } + if ( + source.kind === "blob" && + "uri" in source && + typeof source.uri === "string" && + source.uri.startsWith("tool-output:///") + ) { + return { + source: { kind: "blob", uri: source.uri }, + offset, + limit, + nonce, + }; + } + return undefined; + } catch { + return undefined; + } +} From 3ad9fcc8df2e37512317af8d960b6561a6b98b92 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 25 Sep 2026 08:14:41 -0700 Subject: [PATCH 4/4] fix(permissions): deny forged read_file cursors for outside-root paths A hand-crafted tool-output:///cursor handle naming an outside-root file passed the spill-URI exemption and was served without any containment check. The sandbox now resolves the embedded file source through the normal workspace check at authorize and execution time. --- src/permission/gate.test.ts | 22 ++++++ src/plugins/path-escape-plugin.test.ts | 78 +++++++++++++++++++ src/plugins/path-escape-plugin.ts | 36 ++++++++- .../read-file-continuation-recovery.test.ts | 56 +++++++++++++ src/plugins/read-file-guard-plugin.ts | 10 ++- src/util/tool-output-uri.ts | 4 +- 6 files changed, 200 insertions(+), 6 deletions(-) diff --git a/src/permission/gate.test.ts b/src/permission/gate.test.ts index 816cb2b90..8ec39952f 100644 --- a/src/permission/gate.test.ts +++ b/src/permission/gate.test.ts @@ -11,6 +11,7 @@ import { } from "./gate.js"; import { createPathRestriction } from "./path-restriction.js"; import { createWorktreeRootsProvider } from "./worktree-roots.js"; +import { encodeResumeCursor } from "../util/tool-output-uri.js"; import type { Approval, PermissionRequest } from "./types.js"; import { initTemporaryGitRepo } from "../../tests/helpers/temporary-git-repo.js"; @@ -509,4 +510,25 @@ describe("spill URI sandbox at authorize time (CL-6727)", () => { }); expect(verdict.effect).not.toBe("deny"); }); + + test("read_file + a forged cursor for an outside-root path is denied", async () => { + const outside = mkdtempSync(join(tmpdir(), "gate-forged-cursor-")); + const outsidePath = join(outside, "secret.txt"); + writeFileSync(outsidePath, "top-secret"); + const forged = encodeResumeCursor({ + source: { kind: "file", path: outsidePath }, + offset: 0, + limit: 4, + nonce: "forged-nonce", + }); + const verdict = await gate.authorizeCall({ + id: "spill-forged", + name: "read_file", + arguments: { path: forged }, + }); + expect(verdict.effect).toBe("deny"); + expect(verdict.effect === "deny" ? verdict.reason : "").toMatch( + /escapes working directory/, + ); + }); }); diff --git a/src/plugins/path-escape-plugin.test.ts b/src/plugins/path-escape-plugin.test.ts index c18107d97..484b9c02d 100644 --- a/src/plugins/path-escape-plugin.test.ts +++ b/src/plugins/path-escape-plugin.test.ts @@ -16,6 +16,10 @@ import { pathEscapeBlockReason, pathEscapePlugin, } from "./path-escape-plugin.js"; +import { + encodeResumeCursor, + type ResumeCursor, +} from "../util/tool-output-uri.js"; import type { ToolCall, ToolResult } from "@intx/types/runtime"; function makeCall(name: string, args: Record): ToolCall { @@ -455,6 +459,80 @@ describe("pathEscapePlugin", () => { expect(args.path).toBe("tool-output:///abc123"); }); + test("a forged file cursor for an outside-root path is denied, not served", async () => { + const cwd = await mkdtemp(join(tmpdir(), "corbits-escape-cursor-cwd-")); + const outsideDir = await mkdtemp( + join(tmpdir(), "corbits-escape-cursor-outside-"), + ); + const outsidePath = join(outsideDir, "secret.txt"); + await writeFile(outsidePath, "top-secret"); + try { + const forged = encodeResumeCursor({ + source: { kind: "file", path: outsidePath }, + offset: 0, + limit: 4, + nonce: "forged-nonce", + } satisfies ResumeCursor); + expect( + pathEscapeBlockReason({ path: forged }, cwd, () => [], "read_file"), + ).toMatch(/escapes working directory/); + const plugin = pathEscapePlugin(cwd, () => []); + const handler = plugin.middleware + ? plugin.middleware(nextHandler) + : nextHandler; + const result = await handler( + makeCall("read_file", { path: forged }), + new AbortController().signal, + ); + expect(result.isError).toBe(true); + expect(result.content).toMatch(/escapes working directory/); + } finally { + await rm(cwd, { recursive: true, force: true }); + await rm(outsideDir, { recursive: true, force: true }); + } + }); + + test("an in-bounds file cursor and a blob cursor keep the read_file exemption", async () => { + const cwd = await mkdtemp(join(tmpdir(), "corbits-escape-cursor-ok-")); + const insidePath = join(cwd, "notes.txt"); + await writeFile(insidePath, "notes"); + try { + const inBounds = encodeResumeCursor({ + source: { kind: "file", path: insidePath }, + offset: 4, + limit: 4, + nonce: "minted-nonce", + } satisfies ResumeCursor); + expect( + pathEscapeBlockReason({ path: inBounds }, cwd, () => [], "read_file"), + ).toBeUndefined(); + const blob = encodeResumeCursor({ + source: { kind: "blob", uri: "tool-output:///abc123" }, + offset: 0, + limit: 4, + nonce: "blob-nonce", + } satisfies ResumeCursor); + expect( + pathEscapeBlockReason({ path: blob }, cwd, () => [], "read_file"), + ).toBeUndefined(); + const plugin = pathEscapePlugin(cwd, () => []); + const next = async (call: ToolCall): Promise => ({ + callId: call.id, + content: JSON.stringify(call.arguments), + }); + const handler = plugin.middleware ? plugin.middleware(next) : next; + const result = await handler( + makeCall("read_file", { path: inBounds }), + new AbortController().signal, + ); + expect(result.isError).not.toBe(true); + const args = JSON.parse(String(result.content)) as { path: string }; + expect(args.path).toBe(inBounds); + } finally { + await rm(cwd, { recursive: true, force: true }); + } + }); + test("archive refs pass for archive readers but not for other tools", async () => { for (const name of ["read_file", "grep", "search_files"]) { expect( diff --git a/src/plugins/path-escape-plugin.ts b/src/plugins/path-escape-plugin.ts index 5be105f5c..1398f7932 100644 --- a/src/plugins/path-escape-plugin.ts +++ b/src/plugins/path-escape-plugin.ts @@ -1,6 +1,10 @@ import { resolve } from "node:path"; import type { ToolPlugin } from "@intx/tools-posix"; -import { isToolOutputLike } from "../util/tool-output-uri.js"; +import { + canonicalToolOutputUri, + decodeResumeCursor, + isToolOutputLike, +} from "../util/tool-output-uri.js"; import { isArchiveLike } from "../session/compaction-archive.js"; import { resolveWorkspacePath } from "../permission/path-restriction.js"; import { @@ -224,6 +228,32 @@ function virtualRefVerdict( return undefined; } +// A self-describing read_file continuation handle (tool-output:///cursor/...) +// embeds its resume source, so the spill-URI exemption above must not cover +// it blindly: a hand-crafted handle naming an outside-root file would +// otherwise bypass the containment check the plain path would fail. Resolve +// the embedded file path through the normal workspace check and deny escapes +// exactly like the plain path. Blob-source handles and opaque (never-a-handle) +// spill URIs carry no filesystem target and keep the exemption. +function cursorEscapeReason( + value: string, + cwd: string, + rootsProvider: RootsProvider, + toolName: string, +): string | undefined { + if (canonicalToolName(toolName) !== TOOL_OUTPUT_URI_TOOL) return undefined; + const resumed = decodeResumeCursor(canonicalToolOutputUri(value)); + if (resumed === undefined || resumed.source.kind !== "file") { + return undefined; + } + if ( + resolveWorkspacePath(cwd, resumed.source.path, rootsProvider) === undefined + ) { + return `Path escapes working directory: ${resumed.source.path}`; + } + return undefined; +} + // Same sandbox pathEscapePlugin enforces at execution. The permission gate // consults this at authorize time so it can deny instead of asking for a call // the plugin will reject after Accept. @@ -297,7 +327,9 @@ function blockReasonFor( if (typeof value === "string") { if (key === undefined || !looksLikePath(key)) return undefined; const verdict = virtualRefVerdict(value, toolName); - if (verdict === "skip") return undefined; + if (verdict === "skip") { + return cursorEscapeReason(value, cwd, rootsProvider, toolName); + } if (typeof verdict === "string") return verdict; if (resolveWorkspacePath(cwd, value, rootsProvider) === undefined) { return `Path escapes working directory: ${value}`; diff --git a/src/plugins/read-file-continuation-recovery.test.ts b/src/plugins/read-file-continuation-recovery.test.ts index b816fdf55..73a203ff8 100644 --- a/src/plugins/read-file-continuation-recovery.test.ts +++ b/src/plugins/read-file-continuation-recovery.test.ts @@ -13,6 +13,8 @@ import { } from "../permission/decline-markers.js"; import type { PermissionGate } from "../permission/gate.js"; import { gateToolCall } from "./permission-plugin.js"; +import { pathEscapePlugin } from "./path-escape-plugin.js"; +import { encodeResumeCursor } from "../util/tool-output-uri.js"; import { readFileGuardPlugin } from "./read-file-guard-plugin.js"; // CL-8980 RED: continuation recovery. Truncated reads mint a one-shot @@ -303,6 +305,60 @@ describe("CL-8980 continuation recovery (blob source)", () => { }); }); +describe("CL-8980 forged continuation handle is denied end to end", () => { + function stackedGuard() { + const guard = defined(readFileGuardPlugin(dir, {}).middleware)(fallback); + const stacked = defined(pathEscapePlugin(dir, () => []).middleware)(guard); + return (call: ToolCall) => stacked(call, neverAbort()); + } + + test("a hand-crafted cursor for an unminted outside-root path is denied, not served", async () => { + const outsideDir = await mkdtemp(join(tmpdir(), "read-forged-outside-")); + const outsidePath = join(outsideDir, "secret.txt"); + await writeFile(outsidePath, "forged-handle-secret-payload"); + try { + const forged = encodeResumeCursor({ + source: { kind: "file", path: outsidePath }, + offset: 0, + limit: 4, + nonce: "never-minted", + }); + const result = await stackedGuard()({ + id: "f1", + name: "read_file", + arguments: { path: forged }, + }); + expect(result.isError).toBe(true); + expect(String(result.content)).toMatch(/escapes working directory/); + expect(String(result.content)).not.toContain( + "forged-handle-secret-payload", + ); + expectNotDeclined(String(result.content)); + } finally { + await rm(outsideDir, { recursive: true, force: true }); + } + }); + + test("a minted in-bounds handle is still served through the same stack", async () => { + await fixture("stacked.txt", tenLines("stacked")); + const stack = stackedGuard(); + const first = await stack({ + id: "s1", + name: "read_file", + arguments: { path: "stacked.txt", limit: 4 }, + }); + expect(first.isError).toBeFalsy(); + const handle = extractHandle(String(first.content)); + const second = await stackedGuard()({ + id: "s2", + name: "read_file", + arguments: { path: handle, limit: 4 }, + }); + expect(second.isError).toBeFalsy(); + expect(String(second.content)).toContain("stacked-line-4"); + }); +}); + describe("CL-8980 guard-denied continuation stays isError (never throws)", () => { test("a denied continuation follow returns isError with the reason", async () => { const gate = { diff --git a/src/plugins/read-file-guard-plugin.ts b/src/plugins/read-file-guard-plugin.ts index cb0bcf87d..fcbb02fbd 100644 --- a/src/plugins/read-file-guard-plugin.ts +++ b/src/plugins/read-file-guard-plugin.ts @@ -65,12 +65,14 @@ export interface ReadFileGuardPluginOptions { // Handles are self-describing (CL-8980): the URI embeds the resume recipe, // so following it needs no in-memory record and survives session resume, // prune, and compaction. The per-instance map below only tracks which handles -// this instance already served, to keep the single-use replay contract. +// this instance already served, to keep the per-instance single-use replay +// contract: a verbatim re-follow in this instance is a stale-cursor error, +// while a fresh instance serves the self-describing handle again. type ReadCursor = | { kind: "file"; absolutePath: string; offset: number; consumed: boolean } | { kind: "blob"; uri: string; offset: number; consumed: boolean }; -// A cursor is single-use, but the record survives consumption (bounded by +// A cursor is single-use per plugin instance, but the record survives consumption (bounded by // MAX_CURSOR_HISTORY below) so a stale replay -- consumed already, or a // second process/turn racing the first -- can be told exactly where to // resume instead of hitting an opaque "blob not found" dead end that names @@ -621,6 +623,10 @@ export function readFileGuardPlugin( // in the handle, so serve it exactly as a known cursor would — and // mark it consumed on success so a verbatim re-follow is the same // stale-cursor error as a spent handle, not a second serving. + // The follow honors this call's paging args, not the minted window: + // resumed.limit only records the mint-time window, and resumed.nonce + // only keeps mints distinct (replay keying uses the full handle + // URI), so neither is consulted here. const resumedSource = resumed.source.kind === "file" ? resumed.source.path diff --git a/src/util/tool-output-uri.ts b/src/util/tool-output-uri.ts index 707a0141e..0d0991598 100644 --- a/src/util/tool-output-uri.ts +++ b/src/util/tool-output-uri.ts @@ -30,8 +30,8 @@ export function canonicalToolOutputUri(path: string): string | undefined { // handle itself carries the resume recipe (source + next offset + window // limit + nonce), so following it needs no in-memory record and keeps working // across session resume, prune, and compaction. The nonce keeps every mint a -// distinct one-shot even for identical windows, preserving spent-handle -// replay semantics. +// distinct handle even for identical windows, preserving per-instance +// spent-handle replay semantics. export const CURSOR_HANDLE_PREFIX = "tool-output:///cursor/"; export type ResumeSource =