From 9dc2f49497998da0bb8a977096eab86e560f77ca Mon Sep 17 00:00:00 2001 From: corbits-builder Date: Fri, 25 Sep 2026 19:55:24 -0700 Subject: [PATCH 1/4] test(secret-guard): active custom --config holding skip must be denied --- .../secret-guard-config-denylist.test.ts | 174 ++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100644 src/plugins/secret-guard-config-denylist.test.ts diff --git a/src/plugins/secret-guard-config-denylist.test.ts b/src/plugins/secret-guard-config-denylist.test.ts new file mode 100644 index 000000000..5f31c040f --- /dev/null +++ b/src/plugins/secret-guard-config-denylist.test.ts @@ -0,0 +1,174 @@ +import { describe, expect, test } from "bun:test"; +import { mkdir, mkdtemp, rm, symlink, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { createPosixTools } from "@intx/tools-posix"; +import { createPermissionGate } from "../permission/gate.js"; +import { buildCorePosixToolPlugins } from "../agent/posix-tool-plugins.js"; + +/** + * CL-9386: an operator-chosen --config path inside the workspace is + * model-readable/writable while carrying standing skip-permissions (persisted + * there by /yolo, which writes the active settings source). The static + * secret-guard denylist only covers the default .corbits/settings.json shapes, + * so the active custom path must be runtime-denylisted for the path-keyed + * tools — reads and writes — even under skip-permissions. + */ + +const SKIP_PAYLOAD = JSON.stringify( + { dangerouslySkipPermissions: true }, + null, + 2, +); + +async function withFixture( + run: (paths: { + cwd: string; + customConfig: string; + configLink: string; + }) => Promise, +): Promise { + const parent = await mkdtemp(join(tmpdir(), "cl9386-config-denylist-")); + const cwd = join(parent, "ws"); + await mkdir(cwd, { recursive: true }); + // Standing skip-permissions persisted into the operator-chosen --config file, + // exactly what /yolo writes when --config points inside the workspace. + const customConfig = join(cwd, "operator-config.json"); + await writeFile(customConfig, `${SKIP_PAYLOAD}\n`); + // An innocuous symlink name for the same file: the resolve leg must hold. + const configLink = join(cwd, "notes.txt"); + await symlink(customConfig, configLink); + await writeFile(join(cwd, "scratch.txt"), "ordinary workspace file\n"); + try { + return await run({ cwd, customConfig, configLink }); + } finally { + await rm(parent, { recursive: true, force: true }); + } +} + +function runner( + cwd: string, + skipPermissions: boolean, + activeConfigPath?: string, +) { + const gate = createPermissionGate({ + approvals: [], + interactive: false, + skipPermissions, + reactorGated: false, + auto: false, + cwd, + }); + const args = { cwd, permissionGate: gate }; + if (activeConfigPath !== undefined) { + // CL-9386 seam: entry points thread the active --config path into the tool + // stack here. Until the option exists this assignment is ignored and the + // custom-config tests below fail (red). + (args as { secretGuardExtraDeniedPaths?: string[] }) + .secretGuardExtraDeniedPaths = [activeConfigPath]; + } + return { + gate, + tools: createPosixTools({ + cwd, + plugins: buildCorePosixToolPlugins(args), + }), + }; +} + +describe("CL-9386 runtime-denylist the active --config path holding skip", () => { + for (const skipPermissions of [true, false] as const) { + const mode = skipPermissions ? "yolo" : "normal"; + + test(`${mode}: active custom --config is blocked for read_file`, async () => { + await withFixture(async ({ cwd, customConfig }) => { + const { tools } = runner(cwd, skipPermissions, customConfig); + const result = await tools.run( + { + id: "1", + name: "read_file", + arguments: { path: "operator-config.json" }, + }, + new AbortController().signal, + ); + expect(result.isError).toBe(true); + expect(String(result.content)).toMatch(/sensitive file/i); + expect(String(result.content)).not.toContain( + "dangerouslySkipPermissions", + ); + }); + }); + + test(`${mode}: active custom --config is blocked for write_file`, async () => { + await withFixture(async ({ cwd, customConfig }) => { + const before = await Bun.file(customConfig).text(); + const { tools } = runner(cwd, skipPermissions, customConfig); + const result = await tools.run( + { + id: "1", + name: "write_file", + arguments: { + path: "operator-config.json", + content: '{"dangerouslySkipPermissions":false}\n', + }, + }, + new AbortController().signal, + ); + expect(result.isError).toBe(true); + expect(String(result.content)).toMatch(/sensitive file/i); + expect(await Bun.file(customConfig).text()).toBe(before); + }); + }); + + test(`${mode}: active custom --config via symlink name is blocked for read_file`, async () => { + await withFixture(async ({ cwd, customConfig }) => { + const { tools } = runner(cwd, skipPermissions, customConfig); + const result = await tools.run( + { id: "1", name: "read_file", arguments: { path: "notes.txt" } }, + new AbortController().signal, + ); + expect(result.isError).toBe(true); + expect(String(result.content)).toMatch(/sensitive file/i); + expect(String(result.content)).not.toContain( + "dangerouslySkipPermissions", + ); + }); + }); + } + + test("pin: default settings-shaped file stays statically denied without extras", async () => { + await withFixture(async ({ cwd }) => { + const defaultShaped = join(cwd, ".corbits", "settings.json"); + await mkdir(join(cwd, ".corbits"), { recursive: true }); + await writeFile(defaultShaped, `${SKIP_PAYLOAD}\n`); + for (const skipPermissions of [true, false] as const) { + const { tools } = runner(cwd, skipPermissions); + const result = await tools.run( + { + id: "1", + name: "read_file", + arguments: { path: ".corbits/settings.json" }, + }, + new AbortController().signal, + ); + expect(result.isError).toBe(true); + expect(String(result.content)).toMatch(/sensitive file/i); + expect(String(result.content)).not.toContain( + "dangerouslySkipPermissions", + ); + } + }); + }); + + test("pin: unrelated workspace file stays readable with extras set", async () => { + await withFixture(async ({ cwd, customConfig }) => { + const { tools } = runner(cwd, true, customConfig); + const result = await tools.run( + { id: "1", name: "read_file", arguments: { path: "scratch.txt" } }, + new AbortController().signal, + ); + expect(result.isError !== true).toBe(true); + expect(String(result.content)).toContain("ordinary workspace file"); + }); + }); +}); From cfd9d2e4b0a295e7b64666cd906f1f4b703811ca Mon Sep 17 00:00:00 2001 From: corbits-builder Date: Fri, 25 Sep 2026 20:02:54 -0700 Subject: [PATCH 2/4] fix(secret-guard): runtime-denylist the active --config path holding skip --- docs/ARCHITECTURE.md | 4 +- docs/IMPLEMENTATION.md | 2 +- docs/PRODUCT.md | 2 +- src/agent/posix-tool-plugins.ts | 14 ++++- src/agent/tools.ts | 14 +++++ src/exec/runner.ts | 3 ++ .../secret-guard-config-denylist.test.ts | 52 ++++++++++++++---- src/plugins/secret-guard-plugin.ts | 53 +++++++++++++++++-- src/subagent/agent-fleet.ts | 10 ++++ src/subagent/run.ts | 6 +++ src/subagent/types.ts | 6 +++ src/tui/runner/session.ts | 3 ++ 12 files changed, 152 insertions(+), 17 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 17ffe2dce..da16a4412 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -88,7 +88,7 @@ In TUI chat mode there is no completion gate — the session stays open across t - `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` 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. +- 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. The active settings source (`config.globalSettingsPath`, including a `--config` override pointing inside the workspace) is runtime-denylisted with the same force: path-keyed reads and writes are hard-denied even under skip-permissions, so a `/yolo`-persisted skip there cannot be silently leveraged, and workers inherit the denylist. 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. ### Inference credential recovery @@ -407,7 +407,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. 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. +- **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`. An operator-chosen `--config` path cannot be covered by static patterns, so entry points pass the resolved active settings path as `extraDeniedPaths` (exact match, lexical plus realpath). 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. diff --git a/docs/IMPLEMENTATION.md b/docs/IMPLEMENTATION.md index 1ef347fb5..9365fb122 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` 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. +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 — and the active file itself is runtime-denylisted for path-keyed tools, reads and writes, even under skip-permissions, so the persisted skip cannot be silently leveraged; the startup notice remains as disclosure, not the enforcement. `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: diff --git a/docs/PRODUCT.md b/docs/PRODUCT.md index d4573d83d..6ae4b42af 100644 --- a/docs/PRODUCT.md +++ b/docs/PRODUCT.md @@ -145,7 +145,7 @@ line instead of a path dump. ## Configuration -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. +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`. A `--config` alternate file gets the same denial at runtime: the active settings source is denylisted for the agent's file tools even when it lives inside the workspace, so `/yolo`-persisted skip-permissions there cannot be silently leveraged. ## Optional Capabilities (plugins) diff --git a/src/agent/posix-tool-plugins.ts b/src/agent/posix-tool-plugins.ts index 76a0eb33a..f75bb6b90 100644 --- a/src/agent/posix-tool-plugins.ts +++ b/src/agent/posix-tool-plugins.ts @@ -52,6 +52,13 @@ export interface CorePosixToolPluginsArgs { getShellOutputFeeds?: () => ShellOutputFeedMap | undefined; /** Primary-only evidence archive; workers omit this getter. */ getEvidenceArchive?: () => CompactionArchive | undefined; + /** + * CL-9386: runtime secret-guard denylist for the active --config path. + * Entry points pass [config.globalSettingsPath]; workers inherit their + * parent's list. Omitted (tests, ad-hoc stacks) keeps the static denylist + * only — the default settings file stays covered either way. + */ + secretGuardExtraDeniedPaths?: readonly string[]; } // Middleware order matches docs/ARCHITECTURE.md: path escape through truncation, @@ -91,6 +98,7 @@ export function buildCorePosixToolPlugins( getBackgroundShellRegistry, getShellOutputFeeds, getEvidenceArchive, + secretGuardExtraDeniedPaths, } = args; // Pre-gate sandboxes honor yolo mode so outside-workspace path tools and shell // cwd are not hard-denied after the gate already auto-allows. Pass a live @@ -121,7 +129,11 @@ export function buildCorePosixToolPlugins( evidenceArchivePathGuardPlugin(), deleteFilePlugin(cwd, { allowOutside, rootsProvider }), toolOutputUriPlugin(), - secretGuardPlugin(), + secretGuardPlugin( + secretGuardExtraDeniedPaths !== undefined + ? { extraDeniedPaths: secretGuardExtraDeniedPaths } + : undefined, + ), permissionPlugin(permissionGate), shellGuardPlugin(cwd, shellTimeout, shellEnv, { allowOutsideCwd: allowOutside, diff --git a/src/agent/tools.ts b/src/agent/tools.ts index 2a82a1d9e..f76267638 100644 --- a/src/agent/tools.ts +++ b/src/agent/tools.ts @@ -213,6 +213,13 @@ export interface AgentToolsetArgs { getContextDir?: () => string | undefined; // Per-project settings.env, merged into the run_shell tool's spawn environment. shellEnv?: Record; + /** + * CL-9386: runtime secret-guard denylist for the active --config path. + * Entry points pass [config.globalSettingsPath]; forwarded to the posix + * plugin stack and inherited by workers via the fleet deps below. Omitted + * keeps the static denylist only. + */ + secretGuardExtraDeniedPaths?: readonly string[]; // Called when a background run_shell (background: true) process exits. Hosts // deliver the exit as a system message so the reactor re-enters on a later // turn; omit it and background runs still start/collect but never notify. @@ -424,6 +431,7 @@ export async function createAgentToolset( getEvidenceArchive, sessionMode = "orchestrator", shellEnv, + secretGuardExtraDeniedPaths, toolAvailability = { languageServerAvailable: true }, } = args; let mcpServersSource = args.mcpServersSource ?? "none"; @@ -524,6 +532,9 @@ export async function createAgentToolset( permissionGate, ...(shellTimeout !== undefined ? { shellTimeout } : {}), extraToolPlugins, + ...(secretGuardExtraDeniedPaths !== undefined + ? { secretGuardExtraDeniedPaths } + : {}), ...(sessionBlobReader !== undefined ? { readFileGuard: { blobReader: sessionBlobReader } } : {}), @@ -573,6 +584,9 @@ export async function createAgentToolset( gateAgentTools(inheritedMcpTools, gate), ...(shellTimeout !== undefined ? { shellTimeout } : {}), ...(shellEnv !== undefined ? { shellEnv } : {}), + ...(secretGuardExtraDeniedPaths !== undefined + ? { secretGuardExtraDeniedPaths } + : {}), ...(skillDirs.length > 0 ? { skillDirs } : {}), ...(extraToolPlugins.length > 0 ? { extraToolPlugins } : {}), cwd, diff --git a/src/exec/runner.ts b/src/exec/runner.ts index ebf21dfd0..883128c0b 100644 --- a/src/exec/runner.ts +++ b/src/exec/runner.ts @@ -660,6 +660,9 @@ export async function runExec(config: Config): Promise { skillDirs, telemetry: liveTelemetry, isCodex: isCodexProviderName(config.providerName), + // CL-9386: the active settings source (including a --config override) + // is model-unreadable/unwritable, like the default settings file. + secretGuardExtraDeniedPaths: [config.globalSettingsPath], ...(shellTimeout !== undefined ? { shellTimeout } : {}), ...(toolWatchdog !== undefined ? { toolWatchdog } : {}), ...(localSettingsForMode?.env !== undefined diff --git a/src/plugins/secret-guard-config-denylist.test.ts b/src/plugins/secret-guard-config-denylist.test.ts index 5f31c040f..e7c167f1c 100644 --- a/src/plugins/secret-guard-config-denylist.test.ts +++ b/src/plugins/secret-guard-config-denylist.test.ts @@ -5,6 +5,7 @@ import { join } from "node:path"; import { createPosixTools } from "@intx/tools-posix"; import { createPermissionGate } from "../permission/gate.js"; import { buildCorePosixToolPlugins } from "../agent/posix-tool-plugins.js"; +import { createExtraDeniedPathMatcher } from "./secret-guard-plugin.js"; /** * CL-9386: an operator-chosen --config path inside the workspace is @@ -59,19 +60,17 @@ function runner( auto: false, cwd, }); - const args = { cwd, permissionGate: gate }; - if (activeConfigPath !== undefined) { - // CL-9386 seam: entry points thread the active --config path into the tool - // stack here. Until the option exists this assignment is ignored and the - // custom-config tests below fail (red). - (args as { secretGuardExtraDeniedPaths?: string[] }) - .secretGuardExtraDeniedPaths = [activeConfigPath]; - } return { gate, tools: createPosixTools({ cwd, - plugins: buildCorePosixToolPlugins(args), + plugins: buildCorePosixToolPlugins({ + cwd, + permissionGate: gate, + ...(activeConfigPath !== undefined + ? { secretGuardExtraDeniedPaths: [activeConfigPath] } + : {}), + }), }), }; } @@ -172,3 +171,38 @@ describe("CL-9386 runtime-denylist the active --config path holding skip", () => }); }); }); + +describe("CL-9386 createExtraDeniedPathMatcher", () => { + test("empty list never matches", () => { + expect(createExtraDeniedPathMatcher([])("/any/path.json")).toBe(false); + }); + + test("exact and dot-segment-normalized paths match; others do not", () => { + const root = join(tmpdir(), "cl9386-normalize"); + const isDenied = createExtraDeniedPathMatcher([ + `${root}/sub/../custom.json`, + ]); + expect(isDenied(`${root}/custom.json`)).toBe(true); + expect(isDenied(`${root}/sub/../custom.json`)).toBe(true); + expect(isDenied(`${root}/sub/../other.json`)).toBe(false); + expect(isDenied(`${root}/custom.json.bak`)).toBe(false); + }); + + test("symlink name resolves to the denied target; sibling links do not", async () => { + const parent = await mkdtemp(join(tmpdir(), "cl9386-matcher-")); + try { + const target = join(parent, "operator-config.json"); + await writeFile(target, `${SKIP_PAYLOAD}\n`); + const link = join(parent, "looks-safe.txt"); + await symlink(target, link); + const other = join(parent, "other.txt"); + await writeFile(other, "other\n"); + const isDenied = createExtraDeniedPathMatcher([target]); + expect(isDenied(link)).toBe(true); + expect(isDenied(target)).toBe(true); + expect(isDenied(other)).toBe(false); + } finally { + await rm(parent, { recursive: true, force: true }); + } + }); +}); diff --git a/src/plugins/secret-guard-plugin.ts b/src/plugins/secret-guard-plugin.ts index 10cc3fc3c..11cbbeb5c 100644 --- a/src/plugins/secret-guard-plugin.ts +++ b/src/plugins/secret-guard-plugin.ts @@ -115,6 +115,48 @@ export function isSensitivePathResolved(value: string): boolean { return real !== UNRESOLVABLE && isSensitivePath(real); } +// CL-9386: the active --config path is an operator-chosen settings source that +// can live anywhere — including inside the workspace, where the static +// .corbits/settings.json patterns above never match — while carrying standing +// skip-permissions (/yolo persists the active settings source). Static +// patterns cannot cover an arbitrary runtime path, so entry points thread the +// resolved active path in here and path-keyed tools hard-deny it exactly like +// the default settings file: reads and writes, lexical and realpath legs, +// even under --dangerously-skip-permissions. +export interface SecretGuardPluginOptions { + extraDeniedPaths?: readonly string[]; +} + +// Exact-path matcher over runtime-denied paths. Mirrors +// isSensitivePathResolved's two legs: the lexical form (covers a value passed +// as the identical string, including a target that does not exist yet) and +// the realpath form (covers access through a symlink name, the CL-6971 +// floor). Relative entries match lexically only — production entries are +// absolute (--config is resolved at parse; globalSettingsPath() is absolute). +export function createExtraDeniedPathMatcher( + extraDeniedPaths: readonly string[], +): (value: string) => boolean { + const lexical = new Set(); + const resolved = new Set(); + for (const entry of extraDeniedPaths) { + const normalized = entry.replace(/\\/g, "/"); + lexical.add(normalized); + if (isAbsolute(entry)) { + const absolute = resolvePath(entry).replace(/\\/g, "/"); + lexical.add(absolute); + const real = realpathNearestOr(entry); + if (real !== UNRESOLVABLE) resolved.add(real.replace(/\\/g, "/")); + } + } + if (lexical.size === 0) return () => false; + return (value: string) => { + if (lexical.has(value.replace(/\\/g, "/"))) return true; + if (!isAbsolute(value)) return false; + const real = realpathNearestOr(value); + return real !== UNRESOLVABLE && resolved.has(real.replace(/\\/g, "/")); + }; +} + // Break a shell command into the bare path-like tokens it references so each can // be matched against the secret-file denylist. Quote, backtick and backslash // characters are stripped first so split obfuscations (`.e''nv`, `'.env'`, @@ -276,14 +318,19 @@ export function commandReferencesSensitivePath( // Path-arg hard deny runs before the permission plugin, so it holds even under // --dangerously-skip-permissions. Symlink resolution is part of that floor // (CL-6971): yolo must not let an innocuous link name defeat the denylist. -export function secretGuardPlugin(): ToolPlugin { +export function secretGuardPlugin( + options?: SecretGuardPluginOptions, +): ToolPlugin { + const isExtraDenied = createExtraDeniedPathMatcher( + options?.extraDeniedPaths ?? [], + ); return { middleware: (next) => async (call, signal) => { for (const [key, value] of Object.entries(call.arguments)) { if ( typeof value === "string" && looksLikePath(key) && - isSensitivePathResolved(value) + (isSensitivePathResolved(value) || isExtraDenied(value)) ) { return { callId: call.id, @@ -293,7 +340,7 @@ export function secretGuardPlugin(): ToolPlugin { } } for (const path of productMutationPaths(call.name, call.arguments)) { - if (isSensitivePathResolved(path)) { + if (isSensitivePathResolved(path) || isExtraDenied(path)) { return { callId: call.id, content: `Access to sensitive file blocked by policy: ${path}`, diff --git a/src/subagent/agent-fleet.ts b/src/subagent/agent-fleet.ts index faca14e67..2da7c9b91 100644 --- a/src/subagent/agent-fleet.ts +++ b/src/subagent/agent-fleet.ts @@ -1185,6 +1185,11 @@ export function createSpawnAgentTool(deps: AgentFleetDeps): AgentTool { ? { shellTimeout: deps.shellTimeout } : {}), ...(deps.shellEnv !== undefined ? { shellEnv: deps.shellEnv } : {}), + ...(deps.secretGuardExtraDeniedPaths !== undefined + ? { + secretGuardExtraDeniedPaths: deps.secretGuardExtraDeniedPaths, + } + : {}), ...(deps.skillDirs !== undefined ? { skillDirs: deps.skillDirs } : {}), @@ -1366,6 +1371,11 @@ export function createSpawnAgentTool(deps: AgentFleetDeps): AgentTool { ? { shellTimeout: deps.shellTimeout } : {}), ...(deps.shellEnv !== undefined ? { shellEnv: deps.shellEnv } : {}), + ...(deps.secretGuardExtraDeniedPaths !== undefined + ? { + secretGuardExtraDeniedPaths: deps.secretGuardExtraDeniedPaths, + } + : {}), ...(deps.extraToolPlugins !== undefined ? { extraToolPlugins: deps.extraToolPlugins } : {}), diff --git a/src/subagent/run.ts b/src/subagent/run.ts index 1cc1807d3..e35b395b5 100644 --- a/src/subagent/run.ts +++ b/src/subagent/run.ts @@ -644,6 +644,9 @@ async function runSubAgentInner( ? { shellTimeout: params.shellTimeout } : {}), ...(params.shellEnv !== undefined ? { shellEnv: params.shellEnv } : {}), + ...(params.secretGuardExtraDeniedPaths !== undefined + ? { secretGuardExtraDeniedPaths: params.secretGuardExtraDeniedPaths } + : {}), readFileGuard: { blobReader: sessionBlobReader }, getBackgroundShellRegistry: () => backgroundCollectMounted ? backgroundShells : undefined, @@ -935,6 +938,9 @@ async function runSubAgentInner( ? { shellTimeout: nd.shellTimeout } : {}), ...(nd.shellEnv !== undefined ? { shellEnv: nd.shellEnv } : {}), + ...(nd.secretGuardExtraDeniedPaths !== undefined + ? { secretGuardExtraDeniedPaths: nd.secretGuardExtraDeniedPaths } + : {}), ...(nd.skillDirs !== undefined ? { skillDirs: nd.skillDirs } : {}), ...(nd.extraToolPlugins !== undefined ? { extraToolPlugins: nd.extraToolPlugins } diff --git a/src/subagent/types.ts b/src/subagent/types.ts index c6fdbb2d6..bb17fd35a 100644 --- a/src/subagent/types.ts +++ b/src/subagent/types.ts @@ -49,6 +49,12 @@ export interface SubAgentSandboxDeps { getBlobReader?: () => BlobReader | undefined; /** Project settings.env, merged into the sub-agent's run_shell spawn environment. */ shellEnv?: Record; + /** + * CL-9386: parent's secret-guard runtime denylist (the active --config + * path), so workers cannot silently read/write standing skip-permissions + * the primary itself is denied. Inherited down the dispatch chain. + */ + secretGuardExtraDeniedPaths?: readonly string[]; /** * Plugin skill dirs, same list the primary passes to createUseSkillTool. * Workers resolve attached/optional skill bodies through these dirs. diff --git a/src/tui/runner/session.ts b/src/tui/runner/session.ts index 6c5a37d1c..739c7b3f6 100644 --- a/src/tui/runner/session.ts +++ b/src/tui/runner/session.ts @@ -344,6 +344,9 @@ export async function assembleTUISession( skillDirs, telemetry: liveTelemetry, isCodex: isCodexProviderName(config.providerName), + // CL-9386: the active settings source (including a --config override) + // is model-unreadable/unwritable, like the default settings file. + secretGuardExtraDeniedPaths: [config.globalSettingsPath], ...(shellTimeout !== undefined ? { shellTimeout } : {}), ...(localSettingsForEnv?.env !== undefined ? { shellEnv: localSettingsForEnv.env } From 9daa425f9891bcfa07cb58fa0c05cf3882a11dc8 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 25 Sep 2026 20:28:25 -0700 Subject: [PATCH 3/4] fix(secret-guard): extras-deny dir-scoped grep and shell ask legs --- src/agent/posix-tool-plugins.ts | 11 +- src/permission/auto-shell-policy.ts | 5 +- src/permission/classify.ts | 21 ++- src/permission/gate.ts | 81 +++++++- src/plugins/ripgrep-plugin.ts | 78 +++++++- .../secret-guard-config-denylist.test.ts | 176 ++++++++++++++++++ src/plugins/secret-guard-plugin.ts | 27 ++- 7 files changed, 370 insertions(+), 29 deletions(-) diff --git a/src/agent/posix-tool-plugins.ts b/src/agent/posix-tool-plugins.ts index f75bb6b90..ad4caaf1f 100644 --- a/src/agent/posix-tool-plugins.ts +++ b/src/agent/posix-tool-plugins.ts @@ -109,6 +109,15 @@ export function buildCorePosixToolPlugins( // One shared workspace-roots provider for every bound in this stack, so // pathEscape and delete_file admit the same registered sibling worktrees. const rootsProvider = createWorktreeRootsProvider(cwd); + // CL-1187 finding 2: the gate's shell legs (segmentGuard, auto-allow, auto + // policy) must treat the extras-denied paths as sensitive exactly like the + // secret-guard plugin below does. The gate is built before this stack and + // shared across stacks, so forward the list here — the single funnel every + // entry point (exec, TUI) and worker flows through — rather than wiring + // each runner's gate construction separately. + if (secretGuardExtraDeniedPaths !== undefined) { + permissionGate.setSensitiveExtraDeniedPaths?.(secretGuardExtraDeniedPaths); + } const truncationOptions = getBlobWriter !== undefined || getContextDir !== undefined || @@ -146,7 +155,7 @@ export function buildCorePosixToolPlugins( ? [evidenceArchiveSearchPlugin(getEvidenceArchive)] : []), readFileGuardPlugin(cwd, readFileGuard), - ripgrepPlugin(cwd), + ripgrepPlugin(cwd, {}, undefined, secretGuardExtraDeniedPaths ?? []), // Verify wraps the line-range short-circuit (composeMiddleware runs plugins // outer-to-inner in array order) so its before/after check still covers // start_line/end_line edits instead of only substring-mode edit_file calls. diff --git a/src/permission/auto-shell-policy.ts b/src/permission/auto-shell-policy.ts index edf997708..0f6a9e882 100644 --- a/src/permission/auto-shell-policy.ts +++ b/src/permission/auto-shell-policy.ts @@ -504,6 +504,7 @@ export function autoShellRuleForCall( isRestricted: (path: string, isWrite: boolean) => boolean = () => false, cwd: string = process.cwd(), rootsProvider: RootsProvider = NO_ROOTS, + isExtraDenied: (value: string) => boolean = () => false, ): AutoShellRule | undefined { if (call.name !== "run_shell") return undefined; const command = call.arguments.command; @@ -533,7 +534,9 @@ export function autoShellRuleForCall( } for (const subject of subjects) { - if (commandReferencesSensitivePath(subject, cwd) !== undefined) + if ( + commandReferencesSensitivePath(subject, cwd, isExtraDenied) !== undefined + ) return SENSITIVE_PATH_ASK_RULE; } diff --git a/src/permission/classify.ts b/src/permission/classify.ts index e0c2ccba2..5ca653af9 100644 --- a/src/permission/classify.ts +++ b/src/permission/classify.ts @@ -420,6 +420,7 @@ export function isAutoAllowedShellSegment( segment: string, cwd: string = process.cwd(), rootsProvider: RootsProvider = NO_ROOTS, + isExtraDenied: (value: string) => boolean = () => false, ): boolean { const trimmed = segment.trim(); // Empty is not auto-allowed as a "command"; full-line comments and pure shell @@ -427,18 +428,19 @@ export function isAutoAllowedShellSegment( if (trimmed.length === 0) return false; if (isShellCommentOnly(trimmed) || isShellNoOp(trimmed)) return true; if (runShellAuthzSegmentBlockReason(trimmed) !== undefined) return false; - return isAutoAllowedSegment(segment, cwd, rootsProvider); + return isAutoAllowedSegment(segment, cwd, rootsProvider, isExtraDenied); } function isAutoAllowedSegment( segment: string, cwd: string, rootsProvider: RootsProvider, + isExtraDenied: (value: string) => boolean = () => false, ): boolean { const trimmed = segment.trim(); if (trimmed.length === 0) return false; if (isShellCommentOnly(trimmed) || isShellNoOp(trimmed)) return true; - if (commandReferencesSensitivePath(trimmed, cwd)) return false; + if (commandReferencesSensitivePath(trimmed, cwd, isExtraDenied)) return false; // Same metacharacter gate as isAutoAllowedShellCommand: this classifier also // runs standalone per pipeline/chain segment (see isAutoAllowedShellSegment), // so a segment carrying its own command substitution or redirect must not @@ -466,7 +468,11 @@ function isAutoAllowedSegment( // symlink into a secret file (notes.txt -> .env) asks exactly like the // secret name itself. Pure name-listings skip the resolve leg: `ls // notes.txt` lists freely (CL-5420), and an impure listing fails above. - if (args.some((token) => isSensitiveShellToken(token, cwd, !pureListing))) + if ( + args.some((token) => + isSensitiveShellToken(token, cwd, !pureListing, isExtraDenied), + ) + ) return false; // Pure directory listing may target outside-workspace paths (names only). // Content readers must stay inside the workspace. @@ -483,6 +489,7 @@ export function isAutoAllowedShellCommand( command: string, cwd: string = process.cwd(), rootsProvider: RootsProvider = NO_ROOTS, + isExtraDenied: (value: string) => boolean = () => false, ): boolean { const trimmed = command.trim(); if (trimmed.length === 0) return false; @@ -495,7 +502,7 @@ export function isAutoAllowedShellCommand( (isShellCommentOnly(trimmed) || isShellNoOp(trimmed)) ) return true; - if (commandReferencesSensitivePath(trimmed, cwd)) return false; + if (commandReferencesSensitivePath(trimmed, cwd, isExtraDenied)) return false; // Never auto-allow a command the authz layer would hard-deny at execution. if (runShellAuthzBlockReason(trimmed) !== undefined) return false; // Reject anything with metacharacters that compose or redirect (& ; < > ` $ etc). @@ -504,19 +511,23 @@ export function isAutoAllowedShellCommand( // Split on pipe and require every segment to be a safe read-only program. const segments = trimmed.split("|"); - return segments.every((seg) => isAutoAllowedSegment(seg, cwd, rootsProvider)); + return segments.every((seg) => + isAutoAllowedSegment(seg, cwd, rootsProvider, isExtraDenied), + ); } export function isAutoAllowedShellCall( call: ToolCall, cwd: string = process.cwd(), rootsProvider: RootsProvider = NO_ROOTS, + isExtraDenied: (value: string) => boolean = () => false, ): boolean { if (canonicalToolName(call.name) !== "run_shell") return false; return isAutoAllowedShellCommand( stringArg(call, "command"), cwd, rootsProvider, + isExtraDenied, ); } diff --git a/src/permission/gate.ts b/src/permission/gate.ts index f44a28ccc..178c403f5 100644 --- a/src/permission/gate.ts +++ b/src/permission/gate.ts @@ -20,7 +20,10 @@ import { safeWorktreeCommand, isWorktreeForceFlag, } from "./auto-shell-policy.js"; -import { commandReferencesSensitivePath } from "../plugins/secret-guard-plugin.js"; +import { + commandReferencesSensitivePath, + createExtraDeniedPathMatcher, +} from "../plugins/secret-guard-plugin.js"; import { normalizePathArguments, pathEscapeBlockReason, @@ -133,8 +136,9 @@ function segmentGuard( isRestricted: (path: string, isWrite: boolean) => boolean, cwd?: string, rootsProvider?: RootsProvider, + isExtraDenied: (value: string) => boolean = () => false, ): SegmentGuard | undefined { - if (commandReferencesSensitivePath(segment, cwd) !== undefined) + if (commandReferencesSensitivePath(segment, cwd, isExtraDenied) !== undefined) return { kind: "secret" }; if ( cwd !== undefined && @@ -223,6 +227,7 @@ export function preGrantGuardReason( request: PermissionRequest, isRestricted: (path: string, isWrite: boolean) => boolean, rootsProvider?: RootsProvider, + isExtraDenied: (value: string) => boolean = () => false, ): string | undefined { if (request.tool !== "run_shell") return undefined; const fullCommand = request.subject; @@ -239,7 +244,13 @@ export function preGrantGuardReason( ? bindRestrictedToProcessCwd(isRestricted, request.cwd) : isRestricted; for (const segment of segments) { - const guard = segmentGuard(segment, restricted, request.cwd, rootsProvider); + const guard = segmentGuard( + segment, + restricted, + request.cwd, + rootsProvider, + isExtraDenied, + ); if (guard !== undefined) { return guard.kind === "secret" ? `${segment} references a sensitive path` @@ -268,6 +279,7 @@ export function isRequestCoveredByGrant( isRestricted: (path: string, isWrite: boolean) => boolean, workspace: GrantWorkspace, rootsProvider?: RootsProvider, + isExtraDenied: (value: string) => boolean = () => false, ): boolean { return isRequestCoveredByApprovals( request, @@ -276,6 +288,7 @@ export function isRequestCoveredByGrant( isRestricted, workspace, rootsProvider, + isExtraDenied, ); } @@ -290,6 +303,7 @@ function isRequestCoveredByApprovals( isRestricted: (path: string, isWrite: boolean) => boolean, workspace: GrantWorkspace, rootsProvider?: RootsProvider, + isExtraDenied: (value: string) => boolean = () => false, ): boolean { const scoped = approvals.filter((a) => grantScopeMatches( @@ -304,7 +318,10 @@ function isRequestCoveredByApprovals( if (request.tool !== "run_shell") { return scoped.some((a) => matchesPattern(request.subject, a.pattern)); } - if (preGrantGuardReason(request, isRestricted, rootsProvider) !== undefined) + if ( + preGrantGuardReason(request, isRestricted, rootsProvider, isExtraDenied) !== + undefined + ) return false; const segments = splitChainedCommand(request.subject).filter( (s) => !isShellCommentOnly(s), @@ -313,7 +330,12 @@ function isRequestCoveredByApprovals( const cwd = request.cwd ?? workspace.resolvedCwd; return segments.every((segment) => { if (scoped.some((a) => matchesPattern(segment, a.pattern))) return true; - return isAutoAllowedShellSegment(segment, cwd, rootsProvider); + return isAutoAllowedShellSegment( + segment, + cwd, + rootsProvider, + isExtraDenied, + ); }); } @@ -382,6 +404,13 @@ export interface PermissionGateOptions { // Writes and deletes remain path-escape denies; plugin trust is not write // consent. trustedPluginRoots?: RootsProvider; + // Extras-denied config paths the shell legs treat as sensitive (CL-9386): + // the active settings source, including a --config override. Mirrors the + // secret-guard plugin's extraDeniedPaths so a custom config path asks in + // shell commands exactly like the default settings file. May also be set + // after construction via setSensitiveExtraDeniedPaths when the paths are + // learned later (the toolset builder forwards them). + sensitiveExtraDeniedPaths?: readonly string[]; // Tiers learned from connected MCP servers (tools/list annotations). Tests may // inject a shared registry; production gates create one when omitted. mcpTiers?: McpToolPermissionRegistry; @@ -488,6 +517,11 @@ export interface PermissionGate { // posix plugin stack reads this so authorize-time and execution-time // containment share one list. getTrustedPluginRoots: () => readonly string[]; + // Replace the extras-denied config paths the shell legs treat as sensitive + // (CL-9386). The toolset builder calls this to forward the active settings + // source after gate construction, so the gate and the secret-guard plugin + // share one list. Optional so test doubles of this interface keep compiling. + setSensitiveExtraDeniedPaths?: (paths: readonly string[]) => void; } // True when splitChainedCommand can be trusted to yield only real segments for @@ -570,6 +604,14 @@ export function createPermissionGate( // Session grants live only in this array; persisted grants are seeded in via // options.approvals and re-routed to a store by the persist callback. const sessionGrants: Approval[] = []; + // CL-9386: the extras-denied config paths (the active settings source) the + // shell legs consult, mirrored from the secret-guard plugin's own matcher so + // both agree on what "denied" means. Mutable via + // setSensitiveExtraDeniedPaths because the toolset builder learns the paths + // after the gate is constructed. + let isExtraDenied = createExtraDeniedPathMatcher( + options.sensitiveExtraDeniedPaths ?? [], + ); // Record an operator-granted approval in the live list and route it to the // scope-appropriate home: session grants stay in memory, everything else is @@ -638,6 +680,7 @@ export function createPermissionGate( isRestricted, grantWorkspace(), rootsProvider, + isExtraDenied, ), ); } @@ -798,14 +841,15 @@ export function createPermissionGate( // segment mentions a secret path. const shellReferencesSecret = shellCmd !== undefined && - commandReferencesSensitivePath(shellCmd, effectiveCwd) !== undefined; + commandReferencesSensitivePath(shellCmd, effectiveCwd, isExtraDenied) !== + undefined; if (!restricted && classifyTool(call.name, mcpTiers) === "allow") { return { kind: "allow" }; } if ( !restricted && !shellReferencesSecret && - isAutoAllowedShellCall(call, effectiveCwd, rootsProvider) + isAutoAllowedShellCall(call, effectiveCwd, rootsProvider, isExtraDenied) ) { return { kind: "allow" }; } @@ -821,6 +865,7 @@ export function createPermissionGate( isRestrictedHere, effectiveCwd, rootsProvider, + isExtraDenied, ); if (shellRule?.effect === "deny") { recordAutoDecision(call.name, shellRule.name, "auto-deny"); @@ -876,6 +921,7 @@ export function createPermissionGate( isRestrictedHere, effectiveCwd, rootsProvider, + isExtraDenied, ); if (guard !== undefined) { if (guard.kind === "secret") anySecret = true; @@ -912,7 +958,14 @@ export function createPermissionGate( } // Safe pipeline tails (`| sort`) and pure no-ops (`|| true`) skip. // Containment is judged against the process cwd, not the session cwd. - if (isAutoAllowedShellSegment(segment, effectiveCwd, rootsProvider)) { + if ( + isAutoAllowedShellSegment( + segment, + effectiveCwd, + rootsProvider, + isExtraDenied, + ) + ) { continue; } needsOperator = true; @@ -1141,8 +1194,11 @@ export function createPermissionGate( ) => { const anySecret = request.tool === "run_shell" && - commandReferencesSensitivePath(request.subject, request.cwd) !== - undefined; + commandReferencesSensitivePath( + request.subject, + request.cwd, + isExtraDenied, + ) !== undefined; const decision = { kind: "ask" as const, request, @@ -1256,5 +1312,10 @@ export function createPermissionGate( registerMcpClient, unregisterMcpServer, getTrustedPluginRoots: () => trustedPluginRoots(), + setSensitiveExtraDeniedPaths: (paths: readonly string[]) => { + isExtraDenied = createExtraDeniedPathMatcher(paths); + // The denied set changed — cached denies re-evaluate. + denialMemory.clear(); + }, }; } diff --git a/src/plugins/ripgrep-plugin.ts b/src/plugins/ripgrep-plugin.ts index d33d3f2e3..fc89b1548 100644 --- a/src/plugins/ripgrep-plugin.ts +++ b/src/plugins/ripgrep-plugin.ts @@ -1,5 +1,5 @@ import { statSync } from "node:fs"; -import { dirname, basename } from "node:path"; +import { dirname, basename, resolve as resolvePath } from "node:path"; import type { ToolPlugin } from "@intx/tools-posix"; import { @@ -14,6 +14,7 @@ import { type RgLimits, type SpawnRg, } from "./rg-run.js"; +import { createExtraDeniedPathMatcher } from "./secret-guard-plugin.js"; // A grep over a large tree with the pure-TypeScript walker enumerates the whole // directory (node_modules, build output, the lot) before searching, which stalls @@ -97,8 +98,51 @@ export function ripgrepPlugin( cwd: string, limits: RgLimits = {}, spawnChild?: SpawnRg, + // Extras-denied config paths (CL-9386, CL-1187 finding 1): the active + // settings source, including a --config override. The secret-guard plugin + // denies single-file grep of these paths, but a directory-scoped grep would + // still print their matches — both the rg and fallback legs below drop + // matches under denied paths so directory scope cannot exfiltrate them. + // Empty by default, which keeps every existing behavior unchanged. + extraDeniedPaths: readonly string[] = [], ): ToolPlugin { const maxBytes = limits.maxOutputBytes ?? MAX_OUTPUT_BYTES; + const isExtraDenied = createExtraDeniedPathMatcher(extraDeniedPaths); + + // The file a `file:line:match` grep line came from, resolved so it can be + // tested against the extras-denied set. Context separators (`--`) and our + // own `...` notice lines carry no file and are never matches. + const grepLineDeniedFile = (line: string, rgCwd: string): boolean => { + if (line.startsWith("...") || line === "--") return false; + const match = /^(.*?)[:-]\d+[:-]/.exec(line); + if (match?.[1] === undefined) return false; + const candidate = match[1].replace(/^\.\//, ""); + if (candidate.length === 0) return false; + return isExtraDenied(resolvePath(rgCwd, candidate)); + }; + + // Drop extras-denied matches from grep output (both legs). Filtering before + // the caps keeps the "showing first N" counts honest. + const filterGrepStdout = (stdout: string, rgCwd: string): string => + stdout + .split("\n") + .filter((line) => line.length > 0 && !grepLineDeniedFile(line, rgCwd)) + .join("\n"); + + // Drop extras-denied paths from search_files output (both legs). Filtering + // the name as well as the content: confirming the file's existence is part + // of what the denial withholds. + const filterSearchStdout = (stdout: string, rgCwd: string): string => + stdout + .split("\n") + .filter( + (line) => + line.length > 0 && + !line.startsWith("...") && + !isExtraDenied(resolvePath(rgCwd, line.replace(/^\.\//, ""))), + ) + .join("\n"); + return { middleware: (next) => async (call, signal) => { if (call.name === "grep") { @@ -130,7 +174,11 @@ export function ripgrepPlugin( const content = await runBoundedGrep(boundedArgs, signal, rgCwd); return { callId: call.id, - content: boundedContent(content, maxResults, maxBytes), + content: boundedContent( + filterGrepStdout(content, rgCwd), + maxResults, + maxBytes, + ), }; } catch (err) { return { @@ -149,12 +197,16 @@ export function ripgrepPlugin( if (result.kind === "partial") { return { callId: call.id, - content: partialContent(result.stdout, maxResults, result.notice), + content: partialContent( + filterGrepStdout(result.stdout, rgCwd), + maxResults, + result.notice, + ), }; } return { callId: call.id, - content: capLines(result.stdout, maxResults), + content: capLines(filterGrepStdout(result.stdout, rgCwd), maxResults), }; } @@ -171,6 +223,7 @@ export function ripgrepPlugin( rgCwd, signal, limits, + spawnChild, ); if (result.kind === "unavailable") { try { @@ -181,7 +234,11 @@ export function ripgrepPlugin( ); return { callId: call.id, - content: boundedContent(content, maxResults, maxBytes), + content: boundedContent( + filterSearchStdout(content, rgCwd), + maxResults, + maxBytes, + ), }; } catch (err) { return { @@ -200,12 +257,19 @@ export function ripgrepPlugin( if (result.kind === "partial") { return { callId: call.id, - content: partialContent(result.stdout, maxResults, result.notice), + content: partialContent( + filterSearchStdout(result.stdout, rgCwd), + maxResults, + result.notice, + ), }; } return { callId: call.id, - content: capLines(result.stdout, maxResults), + content: capLines( + filterSearchStdout(result.stdout, rgCwd), + maxResults, + ), }; } diff --git a/src/plugins/secret-guard-config-denylist.test.ts b/src/plugins/secret-guard-config-denylist.test.ts index e7c167f1c..d3e7dbc4d 100644 --- a/src/plugins/secret-guard-config-denylist.test.ts +++ b/src/plugins/secret-guard-config-denylist.test.ts @@ -3,9 +3,12 @@ import { mkdir, mkdtemp, rm, symlink, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { createPosixTools } from "@intx/tools-posix"; +import type { ToolCall } from "@intx/types/runtime"; import { createPermissionGate } from "../permission/gate.js"; import { buildCorePosixToolPlugins } from "../agent/posix-tool-plugins.js"; import { createExtraDeniedPathMatcher } from "./secret-guard-plugin.js"; +import { ripgrepPlugin } from "./ripgrep-plugin.js"; +import type { RgChild, SpawnRg } from "./rg-run.js"; /** * CL-9386: an operator-chosen --config path inside the workspace is @@ -133,6 +136,43 @@ describe("CL-9386 runtime-denylist the active --config path holding skip", () => ); }); }); + + test(`${mode}: dir-scoped grep does not surface extras-denied content`, async () => { + await withFixture(async ({ cwd, customConfig }) => { + const { tools } = runner(cwd, skipPermissions, customConfig); + const result = await tools.run( + { + id: "1", + name: "grep", + arguments: { + pattern: "dangerouslySkipPermissions", + path: ".", + }, + }, + new AbortController().signal, + ); + expect(result.isError !== true).toBe(true); + expect(String(result.content)).not.toContain( + "dangerouslySkipPermissions", + ); + }); + }); + + test(`${mode}: dir-scoped search_files does not surface extras-denied names`, async () => { + await withFixture(async ({ cwd, customConfig }) => { + const { tools } = runner(cwd, skipPermissions, customConfig); + const result = await tools.run( + { + id: "1", + name: "search_files", + arguments: { pattern: "*config*", path: "." }, + }, + new AbortController().signal, + ); + expect(result.isError !== true).toBe(true); + expect(String(result.content)).not.toContain("operator-config.json"); + }); + }); } test("pin: default settings-shaped file stays statically denied without extras", async () => { @@ -170,6 +210,142 @@ describe("CL-9386 runtime-denylist the active --config path holding skip", () => expect(String(result.content)).toContain("ordinary workspace file"); }); }); + + // ripgrep absent: the pure-TypeScript fallback walker leg gets the same + // extras filter. The spawn below fails exactly the way runRg treats as + // "rg not installed" (ENOENT error event -> { kind: "unavailable" }). + // Paths are absolute: the outer stack absolutizes tool paths before this + // plugin sees them, and searchLocation resolves relative paths against the + // process cwd, so a bare "." here would search the repo instead of the + // fixture (pre-existing behavior, out of scope for this change). + const rgMissingSpawn: SpawnRg = () => ({ + pid: undefined, + stdout: { on: () => undefined }, + stderr: { on: () => undefined }, + on: ((event: string, listener: (arg: never) => void) => { + if (event === "error") { + const err = Object.assign(new Error("spawn rg ENOENT"), { + code: "ENOENT", + }); + queueMicrotask(() => listener(err as never)); + } + }) as RgChild["on"], + kill: () => undefined, + }); + + test("rg missing: fallback grep does not surface extras-denied content", async () => { + await withFixture(async ({ cwd, customConfig }) => { + const tools = createPosixTools({ + cwd, + plugins: [ripgrepPlugin(cwd, {}, rgMissingSpawn, [customConfig])], + }); + const result = await tools.run( + { + id: "1", + name: "grep", + arguments: { pattern: "dangerouslySkipPermissions", path: cwd }, + }, + new AbortController().signal, + ); + expect(result.isError !== true).toBe(true); + expect(String(result.content)).not.toContain( + "dangerouslySkipPermissions", + ); + }); + }); + + test("rg missing: fallback search_files does not surface extras-denied names", async () => { + await withFixture(async ({ cwd, customConfig }) => { + const tools = createPosixTools({ + cwd, + plugins: [ripgrepPlugin(cwd, {}, rgMissingSpawn, [customConfig])], + }); + const result = await tools.run( + { + id: "1", + name: "search_files", + arguments: { pattern: "*config*", path: cwd }, + }, + new AbortController().signal, + ); + expect(result.isError !== true).toBe(true); + expect(String(result.content)).not.toContain("operator-config.json"); + }); + }); + + // CL-1187 finding 2: the shell ask-leg must treat the extras-denied active + // config like the default settings file. Pre-fix, `cat operator-config.json` + // ran with zero approval clicks in non-yolo where the default file needs + // one; the builder now forwards the extras into the gate's shell legs. + async function shellVerdict( + command: string, + cwd: string, + activeConfigPath?: string, + ): Promise<{ allowed: boolean; prompts: number }> { + let prompts = 0; + const gate = createPermissionGate({ + approvals: [], + interactive: true, + skipPermissions: false, + reactorGated: false, + auto: false, + cwd, + requestApproval: async () => { + prompts += 1; + return { allow: false }; + }, + }); + createPosixTools({ + cwd, + plugins: buildCorePosixToolPlugins({ + cwd, + permissionGate: gate, + ...(activeConfigPath !== undefined + ? { secretGuardExtraDeniedPaths: [activeConfigPath] } + : {}), + }), + }); + const call: ToolCall = { + id: "1", + name: "run_shell", + arguments: { command }, + }; + const verdict = await gate.evaluate(call); + return { allowed: verdict.allowed, prompts }; + } + + test("pin: shell reading the default settings file asks without extras", async () => { + await withFixture(async ({ cwd }) => { + await mkdir(join(cwd, ".corbits"), { recursive: true }); + await writeFile( + join(cwd, ".corbits", "settings.json"), + `${SKIP_PAYLOAD}\n`, + ); + const verdict = await shellVerdict("cat .corbits/settings.json", cwd); + expect(verdict.prompts).toBe(1); + expect(verdict.allowed).toBe(false); + }); + }); + + test("normal: shell reading the active custom --config asks with extras", async () => { + await withFixture(async ({ cwd, customConfig }) => { + const verdict = await shellVerdict( + "cat operator-config.json", + cwd, + customConfig, + ); + expect(verdict.prompts).toBe(1); + expect(verdict.allowed).toBe(false); + }); + }); + + test("pin: shell reading the custom path without extras stays auto-allowed", async () => { + await withFixture(async ({ cwd }) => { + const verdict = await shellVerdict("cat operator-config.json", cwd); + expect(verdict.prompts).toBe(0); + expect(verdict.allowed).toBe(true); + }); + }); }); describe("CL-9386 createExtraDeniedPathMatcher", () => { diff --git a/src/plugins/secret-guard-plugin.ts b/src/plugins/secret-guard-plugin.ts index 11cbbeb5c..148926763 100644 --- a/src/plugins/secret-guard-plugin.ts +++ b/src/plugins/secret-guard-plugin.ts @@ -269,17 +269,32 @@ function isBareProbeCandidate(token: string): boolean { // Pass resolveSymlinks=false for pure name-listings: listing a name is not // dumping its contents (CL-5420), so `ls notes.txt` still lists freely while // `cat notes.txt` asks. +// +// `isExtraDenied` extends both legs to the extras-denied config paths +// (CL-9386): the custom config path asks in shell commands exactly like the +// default settings file. The listing leg resolves cwd-relative tokens because +// extras entries are exact paths, not name patterns — a lexical match alone +// would miss `ls operator-config.json` while catching the absolute form. export function isSensitiveShellToken( token: string, cwd: string = process.cwd(), resolveSymlinks = true, + isExtraDenied: (value: string) => boolean = () => false, ): boolean { const expanded = expandHome(token); if (isSensitivePath(expanded)) return true; - if (!resolveSymlinks) return false; + if (isExtraDenied(expanded)) return true; + if (!resolveSymlinks) { + if (!isBareProbeCandidate(expanded)) return false; + return isExtraDenied( + isAbsolute(expanded) ? expanded : resolvePath(cwd, expanded), + ); + } if (isPathLikeShellToken(expanded)) { - if (isAbsolute(expanded)) return isSensitivePathResolved(expanded); - return isSensitivePathResolved(resolvePath(cwd, expanded)); + if (isAbsolute(expanded)) + return isSensitivePathResolved(expanded) || isExtraDenied(expanded); + const abs = resolvePath(cwd, expanded); + return isSensitivePathResolved(abs) || isExtraDenied(abs); } if (!isBareProbeCandidate(expanded)) return false; const abs = isAbsolute(expanded) ? expanded : resolvePath(cwd, expanded); @@ -288,12 +303,13 @@ export function isSensitiveShellToken( } catch { return false; } - return isSensitivePathResolved(abs); + return isSensitivePathResolved(abs) || isExtraDenied(abs); } export function commandReferencesSensitivePath( command: string, cwd: string = process.cwd(), + isExtraDenied: (value: string) => boolean = () => false, ): string | undefined { const tokens = shellPathTokens(command); // Dump vs list: a lone name-listing never dumps file contents, so only the @@ -305,7 +321,8 @@ export function commandReferencesSensitivePath( PURE_DIRECTORY_LISTING_PROGRAMS.has(program) && !/[;&|()<>\n]/.test(command); for (const token of tokens) { - if (isSensitiveShellToken(token, cwd, !listingOnly)) return token; + if (isSensitiveShellToken(token, cwd, !listingOnly, isExtraDenied)) + return token; } return undefined; } From b600e994e6f3b0f9114a0576dff7fbc1fb50a645 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 26 Sep 2026 08:32:21 -0700 Subject: [PATCH 4/4] fix(secret-guard): extras-deny hyphen-digit grep and resume scopes --- src/exec/runner.ts | 2 + src/plugins/ripgrep-plugin.ts | 24 ++++--- .../secret-guard-config-denylist.test.ts | 66 ++++++++++++++++++ src/session/approval-resume.test.ts | 67 +++++++++++++++++++ src/session/approval-resume.ts | 29 +++++++- src/tui/runner/session.ts | 2 + 6 files changed, 179 insertions(+), 11 deletions(-) diff --git a/src/exec/runner.ts b/src/exec/runner.ts index 883128c0b..ecbd598fe 100644 --- a/src/exec/runner.ts +++ b/src/exec/runner.ts @@ -1105,6 +1105,8 @@ export async function runExec(config: Config): Promise { resolveParkedCallId: (correlationId) => resolveParkedCallIdFromStore(activeStorage, correlationId), gate: permissionGate, + cwd: config.cwd, + extraDeniedPaths: [config.globalSettingsPath], deliver: async (message, stillCurrent) => { if (!stillCurrent()) return; await approvalDeliverer.deliver(message); diff --git a/src/plugins/ripgrep-plugin.ts b/src/plugins/ripgrep-plugin.ts index fc89b1548..206931fee 100644 --- a/src/plugins/ripgrep-plugin.ts +++ b/src/plugins/ripgrep-plugin.ts @@ -109,16 +109,24 @@ export function ripgrepPlugin( const maxBytes = limits.maxOutputBytes ?? MAX_OUTPUT_BYTES; const isExtraDenied = createExtraDeniedPathMatcher(extraDeniedPaths); - // The file a `file:line:match` grep line came from, resolved so it can be - // tested against the extras-denied set. Context separators (`--`) and our - // own `...` notice lines carry no file and are never matches. + // The file a `file:line:match` (or context `file-line-`) grep line came from, + // resolved so it can be tested against the extras-denied set. Context + // separators (`--`) and our own `...` notice lines carry no file and are + // never matches. + // + // rg's separator is `:-digits-:` / `:digits:`. A non-greedy first match + // treats `--` inside a dated or versioned path (`2026-09-26-config.json`, + // `gpt-4-1.json`) as the line-number field and tests the wrong prefix. Every + // separator is a candidate so a hyphen-digit path is still extras-denied, and + // a later `:digits:` in the match text cannot un-deny the real file. const grepLineDeniedFile = (line: string, rgCwd: string): boolean => { if (line.startsWith("...") || line === "--") return false; - const match = /^(.*?)[:-]\d+[:-]/.exec(line); - if (match?.[1] === undefined) return false; - const candidate = match[1].replace(/^\.\//, ""); - if (candidate.length === 0) return false; - return isExtraDenied(resolvePath(rgCwd, candidate)); + for (const match of line.matchAll(/[:-]\d+[:-]/g)) { + const candidate = line.slice(0, match.index).replace(/^\.\//, ""); + if (candidate.length === 0) continue; + if (isExtraDenied(resolvePath(rgCwd, candidate))) return true; + } + return false; }; // Drop extras-denied matches from grep output (both legs). Filtering before diff --git a/src/plugins/secret-guard-config-denylist.test.ts b/src/plugins/secret-guard-config-denylist.test.ts index d3e7dbc4d..d7140b69f 100644 --- a/src/plugins/secret-guard-config-denylist.test.ts +++ b/src/plugins/secret-guard-config-denylist.test.ts @@ -50,6 +50,23 @@ async function withFixture( } } +async function withNamedConfig( + name: string, + run: (paths: { cwd: string; customConfig: string }) => Promise, +): Promise { + const parent = await mkdtemp(join(tmpdir(), "cl9386-dated-config-")); + const cwd = join(parent, "ws"); + await mkdir(cwd, { recursive: true }); + const customConfig = join(cwd, name); + await writeFile(customConfig, `${SKIP_PAYLOAD}\n`); + await writeFile(join(cwd, "scratch.txt"), "ordinary workspace file\n"); + try { + return await run({ cwd, customConfig }); + } finally { + await rm(parent, { recursive: true, force: true }); + } +} + function runner( cwd: string, skipPermissions: boolean, @@ -158,6 +175,32 @@ describe("CL-9386 runtime-denylist the active --config path holding skip", () => }); }); + for (const datedName of [ + "2026-09-26-config.json", + "gpt-4-1.json", + ] as const) { + test(`${mode}: dir-scoped grep does not surface extras-denied ${datedName}`, async () => { + await withNamedConfig(datedName, async ({ cwd, customConfig }) => { + const { tools } = runner(cwd, skipPermissions, customConfig); + const result = await tools.run( + { + id: "1", + name: "grep", + arguments: { + pattern: "dangerouslySkipPermissions", + path: ".", + }, + }, + new AbortController().signal, + ); + expect(result.isError !== true).toBe(true); + expect(String(result.content)).not.toContain( + "dangerouslySkipPermissions", + ); + }); + }); + } + test(`${mode}: dir-scoped search_files does not surface extras-denied names`, async () => { await withFixture(async ({ cwd, customConfig }) => { const { tools } = runner(cwd, skipPermissions, customConfig); @@ -254,6 +297,29 @@ describe("CL-9386 runtime-denylist the active --config path holding skip", () => }); }); + for (const datedName of ["2026-09-26-config.json", "gpt-4-1.json"] as const) { + test(`rg missing: fallback grep does not surface extras-denied ${datedName}`, async () => { + await withNamedConfig(datedName, async ({ cwd, customConfig }) => { + const tools = createPosixTools({ + cwd, + plugins: [ripgrepPlugin(cwd, {}, rgMissingSpawn, [customConfig])], + }); + const result = await tools.run( + { + id: "1", + name: "grep", + arguments: { pattern: "dangerouslySkipPermissions", path: cwd }, + }, + new AbortController().signal, + ); + expect(result.isError !== true).toBe(true); + expect(String(result.content)).not.toContain( + "dangerouslySkipPermissions", + ); + }); + }); + } + test("rg missing: fallback search_files does not surface extras-denied names", async () => { await withFixture(async ({ cwd, customConfig }) => { const tools = createPosixTools({ diff --git a/src/session/approval-resume.test.ts b/src/session/approval-resume.test.ts index 6ec3190ee..e8afc9598 100644 --- a/src/session/approval-resume.test.ts +++ b/src/session/approval-resume.test.ts @@ -1,4 +1,7 @@ import { describe, expect, mock, test } from "bun:test"; +import { mkdtemp, mkdir, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import type { Agent, SendResult } from "@intx/agent"; import type { ApprovalSnapshot, @@ -10,6 +13,7 @@ import type { import { APPROVAL_TIMEOUT_RESULT_TEXT } from "../permission/decline-markers.js"; import type { PermissionGate } from "../permission/gate.js"; +import { createExtraDeniedPathMatcher } from "../plugins/secret-guard-plugin.js"; import { APPROVAL_DROPPED_NOTICE, createApprovalResume, @@ -137,6 +141,69 @@ describe("requestFromApprovalSnapshot aliased file tools", () => { }); }); +function shellSnapshot(command: string): ApprovalSnapshot { + return { + name: "run_shell", + description: "run a shell command", + inputSchema: {}, + arguments: { command }, + }; +} + +describe("requestFromApprovalSnapshot secret persist scopes", () => { + test("static secret shell strips persist scopes without extras", () => { + const request = requestFromApprovalSnapshot( + shellSnapshot("cat .env"), + "corr-env", + ); + expect(request?.tool).toBe("run_shell"); + expect(request?.scopes).toEqual([]); + }); + + test("non-secret shell keeps persist scopes", () => { + const request = requestFromApprovalSnapshot( + shellSnapshot("cat README.md"), + "corr-readme", + ); + expect(request?.tool).toBe("run_shell"); + expect(request?.scopes.length).toBeGreaterThan(0); + }); + + test("extras-secret shell strips persist scopes", async () => { + const parent = await mkdtemp(join(tmpdir(), "cl9386-resume-extras-")); + const cwd = join(parent, "ws"); + const customConfig = join(cwd, "operator-config.json"); + try { + await mkdir(cwd, { recursive: true }); + await writeFile( + customConfig, + `${JSON.stringify({ dangerouslySkipPermissions: true }, null, 2)}\n`, + ); + const request = requestFromApprovalSnapshot( + shellSnapshot("cat operator-config.json"), + "corr-extras", + { + cwd, + isExtraDenied: createExtraDeniedPathMatcher([customConfig]), + }, + ); + expect(request?.tool).toBe("run_shell"); + expect(request?.scopes).toEqual([]); + } finally { + await rm(parent, { recursive: true, force: true }); + } + }); + + test("pin: custom config path without extras keeps persist scopes", () => { + const request = requestFromApprovalSnapshot( + shellSnapshot("cat operator-config.json"), + "corr-no-extras", + ); + expect(request?.tool).toBe("run_shell"); + expect(request?.scopes.length).toBeGreaterThan(0); + }); +}); + describe("approval decision intent headers", () => { for (const allow of [true, false]) { test(`preserves ${allow ? "granted" : "denied"} intent and correlation through the session queue`, async () => { diff --git a/src/session/approval-resume.ts b/src/session/approval-resume.ts index 3d0b2d50b..10de4e4f7 100644 --- a/src/session/approval-resume.ts +++ b/src/session/approval-resume.ts @@ -23,7 +23,10 @@ import { getLogger } from "@intx/log"; import { LOG_NAMESPACE_ROOT } from "../branding.js"; import { canonicalToolName } from "../agent/canonical-tool-name.js"; -import { commandReferencesSensitivePath } from "../plugins/secret-guard-plugin.js"; +import { + commandReferencesSensitivePath, + createExtraDeniedPathMatcher, +} from "../plugins/secret-guard-plugin.js"; import { APPROVAL_TIMEOUT_RESULT_TEXT } from "../permission/decline-markers.js"; import { buildRequests } from "../permission/classify.js"; import type { PermissionGate } from "../permission/gate.js"; @@ -59,6 +62,10 @@ export interface ApprovalResume { export function requestFromApprovalSnapshot( snapshot: ApprovalSnapshot, correlationId: string, + extras: { + cwd?: string; + isExtraDenied?: (value: string) => boolean; + } = {}, ): PermissionRequest | null { const parsed = ApprovalSnapshotShape(snapshot); if (parsed instanceof type.errors) return null; @@ -71,7 +78,11 @@ export function requestFromApprovalSnapshot( if (request === undefined) return null; const anySecret = request.tool === "run_shell" && - commandReferencesSensitivePath(request.subject) !== undefined; + commandReferencesSensitivePath( + request.subject, + extras.cwd ?? process.cwd(), + extras.isExtraDenied ?? (() => false), + ) !== undefined; return anySecret ? { ...request, scopes: [] } : request; } @@ -184,7 +195,16 @@ export function createApprovalResume(args: { correlationId: string, ) => string | undefined | Promise; gate: PermissionGate; + // Workspace the parked shell ran in, and extras-denied config paths the live + // decide()/resolveSuspended secret check already consults. Resume rebuilds + // persistable scopes from the snapshot and must apply the same extras so + // Always/Project are not offered for extras-secret shell. + cwd?: string; + extraDeniedPaths?: readonly string[]; }): ApprovalResume { + const isExtraDenied = createExtraDeniedPathMatcher( + args.extraDeniedPaths ?? [], + ); // Retry re-await wiring: correlation ids whose decision was handed to the // reactor reuse that acceptance. A retry after an observed acceptance // returns without opening the gate or delivering again, so the parked call @@ -254,7 +274,10 @@ export function createApprovalResume(args: { const request = approvalSnapshot === undefined ? null - : requestFromApprovalSnapshot(approvalSnapshot, correlationId); + : requestFromApprovalSnapshot(approvalSnapshot, correlationId, { + ...(args.cwd !== undefined ? { cwd: args.cwd } : {}), + isExtraDenied, + }); if (request === null) { args.registerParkedCancel?.(undefined); await deliverDecision( diff --git a/src/tui/runner/session.ts b/src/tui/runner/session.ts index 739c7b3f6..c96f92066 100644 --- a/src/tui/runner/session.ts +++ b/src/tui/runner/session.ts @@ -541,6 +541,8 @@ export async function assembleTUISession( }); }, gate: permissionGate, + cwd: config.cwd, + extraDeniedPaths: [config.globalSettingsPath], }); state.enqueueAgentDeliver = ( deliverToLiveAgent: () => void,