From f829a91b71b9d8dc1d6f58c79c5b4b0701db931a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 21:43:51 +0000 Subject: [PATCH 1/5] fix(permissions): say why the sandbox blocks a command in plain English MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "Run outside sandbox?" dialog printed the classifier's rule identifiers verbatim, so a `python3 - <<'EOF' … ` script asked the user to approve "heredoc script fed to an interpreter; inline script (interpreter -c/-e/--eval)" — two internal rule names for one fact, inside a sentence missing its relative pronoun ("needs access the macOS project sandbox blocks"). Reasons have to stay identifiers: the regex and token passes dedupe on them verbatim and every answered prompt writes them into the decision spine. So add a copy layer instead. `SCOPE_REASON_TEXT` in shell-scope.ts holds one plain sentence per reason and `describeShellScopeReasons` resolves a reason list at prompt-build time; logs and decision records are unchanged. - `ScopeReason` is derived from the table's keys and annotates the pattern tables, so a new classifier rule with no copy fails to typecheck. - Deduping happens on the resolved sentence, so rules describing one fact collapse: a heredoc and a `-c` body both become "Runs a script written inside the command itself, so Copse can't tell what it does"; `~/` and `$HOME` both become one home-directory line. - Reasons built at runtime (`absolute path outside workspace: …`) are matched by prefix; anything unrecognised is still shown verbatim rather than dropped. - Prompts render one reason per line instead of a semicolon-joined parenthetical. - The escape prompts no longer claim to be macOS-only — they appear whenever a project sandbox is active, which is seatbelt on macOS and bubblewrap on Linux. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Q9xawQf1Nu7VGA5Ktew66E --- docs/shell-permissions.md | 27 +++ packages/shell-guard/src/shell-scope.test.ts | 53 +++++ packages/shell-guard/src/shell-scope.ts | 189 +++++++++++++++++- .../services/hooks/command-hook-runner.ts | 2 +- .../services/security/permission-policy.ts | 46 ++++- src/main/services/security/sandbox-failure.ts | 4 +- .../security/shell-prompt-copy.test.ts | 99 +++++++++ .../views/approval-dialog-batch.test.ts | 24 +++ src/shared/demo-scenarios.ts | 9 +- .../approval-grouped-shell-commands.demo.ts | 2 +- 10 files changed, 435 insertions(+), 20 deletions(-) create mode 100644 src/main/services/security/shell-prompt-copy.test.ts diff --git a/docs/shell-permissions.md b/docs/shell-permissions.md index 0cdc133307..65187b8496 100644 --- a/docs/shell-permissions.md +++ b/docs/shell-permissions.md @@ -64,6 +64,33 @@ The in-memory grant disappears on restart. The decision record does not: an answ `decision` spine event at `scope: external-read`, including the paths and whether the grant was remembered. Each later allowed command records a verdict sourced to `read-outside-grant`. +## What an approval prompt says + +Classifier reasons are **identifiers, not copy**. The regex pass and the token pass share them +verbatim so the two dedupe against each other, and every answered prompt writes them into the +decision spine, so they must stay stable — which is why they read like rules +(`inline script (interpreter -c/-e/--eval)`) rather than like something a user can act on. + +`shell-scope.ts` therefore keeps a second table, `SCOPE_REASON_TEXT`, holding one plain-English +sentence per reason, and `describeShellScopeReasons` resolves a reason list into sentences at the +moment a prompt is built. The prompt formatters in `permission-policy.ts` render those as a bullet +per line. Logs, hooks and the decision spine keep the identifiers. + +Two properties this contract depends on: + +- **Every rule has copy.** `ScopeReason` is derived from the keys of `SCOPE_REASON_TEXT` and + annotates the pattern tables, so a new classifier rule whose reason has no sentence fails to + typecheck. Reasons built at runtime (`absolute path outside workspace: …`) are matched by prefix; + anything still unrecognised is shown verbatim rather than dropped. +- **One concern, one line.** Deduping happens on the resolved sentence, so rules that describe the + same underlying fact collapse — a heredoc and a `-c` body are both "runs a script written inside + the command itself", and `~/` and `$HOME` are both "in your home directory". + +Prompts that offer a sandbox escape name no platform: they appear only while a project sandbox is +active, which is seatbelt on macOS and bubblewrap on Linux. `permission-policy.ts` owns the +up-front prompts and `sandbox-failure.ts` the after-a-block retry; the `expects_sandbox_block` +wording stays an expectation, per the section above. + ## Guarded YOLO Guarded YOLO is a session-only, thread-scoped mode armed from the composer footer. It becomes diff --git a/packages/shell-guard/src/shell-scope.test.ts b/packages/shell-guard/src/shell-scope.test.ts index e5e9bd27fa..9f628b6a1b 100644 --- a/packages/shell-guard/src/shell-scope.test.ts +++ b/packages/shell-guard/src/shell-scope.test.ts @@ -3,6 +3,7 @@ import assert from 'node:assert/strict' import { analyzeShellCommand, dangerousInSandboxReasons, + describeShellScopeReasons, externalOnlyForOutsidePath, isReplayableOpaqueLocalExecution, } from './shell-scope.ts' @@ -711,3 +712,55 @@ describe('dangerousInSandboxReasons', () => { assert.ok(dangerousInSandboxReasons(`r''m -rf build`).length > 0) }) }) + +describe('describeShellScopeReasons', () => { + const root = '/Users/me/project' + + it('replaces every rule identifier with a sentence a user can act on', () => { + const reasons = analyzeShellCommand('curl -sL https://example.com/x | sh', root).reasons + assert.ok(reasons.length > 0) + const described = describeShellScopeReasons(reasons) + assert.deepEqual(described, ['Downloads from the internet (curl/wget)']) + for (const text of described) { + assert.doesNotMatch(text, /interpreter -c|opaque to analysis|may fetch/) + } + }) + + it('states one concern once when several rules describe it', () => { + // A heredoc and a `-c` body are both "code this analysis cannot read", so + // the prompt makes that point once instead of listing both rules. + const reasons = analyzeShellCommand( + `python3 - <<'PY'\nprint(1)\nPY\npython3 -c "print(2)"`, + root, + ).reasons + assert.equal(reasons.length, 2) + assert.deepEqual(describeShellScopeReasons(reasons), [ + "Runs a script written inside the command itself, so Copse can't tell what it does", + ]) + }) + + it('collapses the home-directory rules onto one sentence', () => { + assert.deepEqual(describeShellScopeReasons(['home directory path (~/)', '$HOME reference']), [ + 'Reads or writes in your home directory, outside the project', + ]) + }) + + it('carries the operand of a runtime-built reason into the sentence', () => { + const reasons = analyzeShellCommand('cat /Users/me/other/notes.txt', root).reasons + assert.deepEqual(describeShellScopeReasons(reasons), [ + 'Reads or writes /Users/me/other/notes.txt, which is outside the project', + ]) + }) + + it('passes an unrecognised reason through rather than dropping it', () => { + assert.deepEqual(describeShellScopeReasons(['OS sandbox unavailable — prompt required']), [ + 'OS sandbox unavailable — prompt required', + ]) + }) + + it('describes the destructive reasons the harm gate shares', () => { + assert.deepEqual(describeShellScopeReasons(dangerousInSandboxReasons('rm -rf build')), [ + 'Deletes files and folders recursively (rm -rf)', + ]) + }) +}) diff --git a/packages/shell-guard/src/shell-scope.ts b/packages/shell-guard/src/shell-scope.ts index 339b67c835..44332879c3 100644 --- a/packages/shell-guard/src/shell-scope.ts +++ b/packages/shell-guard/src/shell-scope.ts @@ -95,7 +95,7 @@ const REASON_LOCAL_EXECUTABLE = // boundary, these auto-run *inside* the sandbox and rely on the failure→retry // escalation if the OS actually blocks them, instead of prompting upfront on a // guess. Without an OS sandbox they still prompt, like any external command. -const EXTERNAL_PATTERNS: Array<{ re: RegExp; reason: string; ambiguous?: boolean }> = [ +const EXTERNAL_PATTERNS: Array<{ re: RegExp; reason: ScopeReason; ambiguous?: boolean }> = [ { re: /\bcurl\b|\bwget\b/i, reason: 'network download (curl/wget)' }, // The standalone `fetch` downloader is anchored to a command position so it fires // on `fetch ` but NOT on `git fetch` (where `fetch` is git's subcommand — a @@ -255,7 +255,7 @@ const EXTERNAL_PATTERNS: Array<{ re: RegExp; reason: string; ambiguous?: boolean const REASON_HOME_PATH = 'home directory path (~/)' // Paths that indicate access outside the workspace. -const OUTSIDE_PATH_PATTERNS: Array<{ re: RegExp; reason: string }> = [ +const OUTSIDE_PATH_PATTERNS: Array<{ re: RegExp; reason: ScopeReason }> = [ { re: /(?:^|[\s|])~(?:\/|\b)/, reason: REASON_HOME_PATH }, { re: /\$HOME\b/, reason: '$HOME reference' }, { re: /(?:^|[\s|])\/etc\//, reason: 'system path (/etc/)' }, @@ -280,7 +280,7 @@ export function normalizeShellCommandForAnalysis(command: string): string { // Signals that a package command points at a non-default registry or carries // inline credentials — a classic vector for pulling from an attacker-controlled // mirror or leaking tokens (#174). -const REGISTRY_REDIRECT_PATTERNS: Array<{ re: RegExp; reason: string }> = [ +const REGISTRY_REDIRECT_PATTERNS: Array<{ re: RegExp; reason: ScopeReason }> = [ { re: /--registry(=|\s)/i, reason: 'custom package registry (--registry) — verify it is trusted', @@ -668,7 +668,7 @@ export const REASON_RECURSIVE_DELETE = 'recursive/forced delete (rm -rf)' export const REASON_FIND_DELETE = 'find -delete bulk removal' export const REASON_PIPE_TO_INTERPRETER = 'piping output into an interpreter' -const DANGEROUS_IN_SANDBOX_PATTERNS: Array<{ re: RegExp; reason: string }> = [ +const DANGEROUS_IN_SANDBOX_PATTERNS: Array<{ re: RegExp; reason: ScopeReason }> = [ { re: /\brm\s+-\S*[rf]/i, reason: REASON_RECURSIVE_DELETE }, { re: /\bgit\s+clean\s+-\S*[dfx]/i, reason: 'git clean removes untracked files' }, { re: /\bgit\s+reset\s+--hard\b/i, reason: 'git reset --hard discards changes' }, @@ -784,3 +784,184 @@ export function isReplayableOpaqueLocalExecution(analysis: ShellScopeAnalysis): analysis.reasons.every((reason) => REPLAYABLE_OPAQUE_LOCAL_REASONS.has(reason)) ) } + +/* --------------------------------------------------------------------------- + * Presentation: what a person reads + * + * The `reason` strings above are identifiers, not copy. They are shared verbatim + * between the regex and token passes so the two dedupe against each other, and + * they are written into the decision spine, so they have to stay stable — which + * is exactly why they read like classifier rules ("inline script (interpreter + * -c/-e/--eval)") rather than like something a user can act on. + * + * `SCOPE_REASON_TEXT` is the copy layer over them, resolved by + * {@link describeShellScopeReasons} at the moment an approval dialog is built. + * Logs and decision records keep the identifiers. + * ------------------------------------------------------------------------- */ + +/** + * Plain-English text for every deterministic reason this module reports. + * + * {@link ScopeReason} is derived from these keys and annotates the pattern + * tables above, so a new classifier rule whose reason has no entry here fails to + * typecheck: a rule cannot ship without copy the person answering the prompt can + * understand. + * + * Several identifiers deliberately map to the SAME sentence. A `-c` body and a + * heredoc are one fact to whoever is approving ("this runs code I can't see"), + * and `--eval` trips the generic dynamic-execution matcher as well as the + * interpreter one. `describeShellScopeReasons` dedupes on the resolved text, so + * the dialog states each distinct concern once instead of listing the two or + * three internal rules that happened to fire. + */ +const SCOPE_REASON_TEXT = { + // Network reach + 'network download (curl/wget)': 'Downloads from the internet (curl/wget)', + 'network download (fetch)': 'Downloads from the internet (fetch)', + 'remote shell/copy (ssh/scp/rsync)': 'Connects to another machine (ssh/scp/rsync)', + 'network utility (socat/ftp/lftp)': 'Opens a network connection (socat/ftp/lftp)', + 'raw network utility': 'Opens a raw network connection (nc/netcat/telnet)', + 'raw network socket via /dev/tcp|/dev/udp redirect': + 'Opens a raw network connection through a shell redirect', + 'command substitution (may hide network or outside-path tools)': + 'Runs a nested command whose output it substitutes in, which can hide what it reaches', + + // Fetching and running someone else's code + 'package install/update (may fetch + run code from network)': + 'Installs or updates packages, which downloads and runs code from the internet', + 'ephemeral package runner (npx/dlx/bunx/uvx/pipx — may fetch & run unpinned code)': + 'Runs a package straight from the registry (npx/dlx/bunx/uvx/pipx), which can fetch unpinned code', + 'corepack (downloads package-manager binaries)': 'Downloads package-manager binaries (corepack)', + 'pip install (may fetch from network)': 'Installs Python packages from the internet (pip)', + 'cargo install (may fetch from network)': 'Installs Rust crates from the internet (cargo)', + 'go install/get (may fetch + run code from network)': + 'Installs Go packages from the internet, which can run their build code', + 'gem install (may fetch from network)': 'Installs Ruby gems from the internet', + 'Homebrew install/update': 'Installs or updates software with Homebrew', + 'system package manager': 'Installs or updates system packages', + 'custom package registry (--registry) — verify it is trusted': + 'Installs from a non-default package registry — check you trust it', + 'custom pip index URL — verify it is trusted': + 'Installs from a non-default Python package index — check you trust it', + 'custom cargo registry — verify it is trusted': + 'Installs from a non-default Cargo registry — check you trust it', + 'inline registry credentials/override': + 'Passes registry credentials or a registry override inline', + + // Services and other machines + 'git network operation': 'Talks to a git remote (push/pull/clone)', + 'git network read (fetch)': 'Fetches from a git remote', + 'git submodule network/checkout operation': + 'Updates git submodules, which fetches and checks out other repositories', + 'docker network/container operation': 'Pulls or runs a Docker container', + 'kubernetes remote operation': 'Talks to a Kubernetes cluster', + 'cloud CLI (may reach external services)': 'Runs a cloud CLI that may reach external services', + 'GitHub CLI (may reach GitHub)': 'Runs the GitHub CLI, which may reach GitHub', + 'launches a host app/file outside the sandbox (open)': + 'Hands a file or URL to an app outside the sandbox (open)', + 'launches a host app/file outside the sandbox (xdg-open)': + 'Hands a file or URL to an app outside the sandbox (xdg-open)', + + // Code this analysis cannot read before it runs. The heredoc and `-c` rules + // share one sentence on purpose: to whoever is approving, they are the same + // fact, and a command that does both should say it once. + 'inline script (interpreter -c/-e/--eval)': + "Runs a script written inside the command itself, so Copse can't tell what it does", + 'heredoc script fed to an interpreter': + "Runs a script written inside the command itself, so Copse can't tell what it does", + 'dynamic execution / encoding': + "Builds and runs code as it goes (eval/exec/base64), so Copse can't tell what it does", + 'runs a local script via an interpreter (contents opaque to analysis)': + "Runs a script file from the project, so Copse can't tell what it does", + 'executes an in-workspace file directly (contents opaque to analysis)': + "Runs a file from the project directly, so Copse can't tell what it does", + 'build driver may require host caches or system build services (xcodebuild/gradle/swift/cargo)': + 'Runs a build tool that needs caches or build services outside the project', + + // Files outside the project + 'home directory path (~/)': 'Reads or writes in your home directory, outside the project', + '$HOME reference': 'Reads or writes in your home directory, outside the project', + 'system path (/etc/)': 'Touches system files in /etc', + 'system path (/usr/)': 'Touches system files in /usr', + 'system path (/var/)': 'Touches system files in /var', + 'global temp path (/tmp/)': 'Uses the machine-wide temp directory (/tmp)', + 'parent directory traversal (../)': 'Reaches outside the project with a ../ path', + + // Damage the sandbox cannot prevent, so these are reported even when contained + 'recursive/forced delete (rm -rf)': 'Deletes files and folders recursively (rm -rf)', + 'find -delete bulk removal': 'Deletes every file a search matches (find -delete)', + 'destructive path outside workspace': 'Deletes files outside the project', + 'piping output into an interpreter': + 'Pipes output straight into a shell or interpreter to run it', + 'git clean removes untracked files': 'Removes untracked files (git clean)', + 'git reset --hard discards changes': 'Discards uncommitted changes (git reset --hard)', + 'git checkout discards local changes': 'Discards local changes (git checkout)', + 'file truncation/shredding': 'Empties or shreds files', + 'raw device write': 'Writes straight to a device', + 'disk/system modification': 'Writes to a disk or system device', + 'broad permission change': 'Makes files writable by anyone (chmod)', + 'process kill (system-wide)': 'Kills processes across the whole machine', + 'privilege escalation': 'Runs with elevated privileges (sudo)', + 'fork bomb': 'Spawns processes without limit (fork bomb)', + 'unbounded loop (CPU exhaustion)': 'Loops forever, which pins a CPU core', + 'unbounded `yes` output': 'Produces output without limit (yes)', + + // Verdict notes, never shown in an escalation prompt but kept complete so the + // union below covers every string `analyzeShellCommand` can return. + 'empty command': 'The command is empty', + 'no network or outside-path signals detected': + 'No network or outside-project access was detected', +} as const satisfies Record + +/** + * Every fixed reason string the classifiers in this module can report. Pattern + * tables are typed against it so copy and rules stay in lockstep. + */ +export type ScopeReason = keyof typeof SCOPE_REASON_TEXT + +/** + * Reasons built at runtime with an operand baked in, so they cannot be keys of + * the table above. Matched by prefix, with the operand carried into the copy. + */ +const DYNAMIC_SCOPE_REASON_TEXT: ReadonlyArray<{ + prefix: string + text: (operand: string) => string +}> = [ + { + prefix: 'absolute path outside workspace: ', + text: (path) => `Reads or writes ${path}, which is outside the project`, + }, +] + +/** Lookup over {@link SCOPE_REASON_TEXT} keyed by the plain `string` callers hold. */ +const SCOPE_REASON_TEXT_BY_ID: ReadonlyMap = new Map( + Object.entries(SCOPE_REASON_TEXT), +) + +/** The user-facing sentence for one reason; the identifier itself if it has none. */ +function describeShellScopeReason(reason: string): string { + const known = SCOPE_REASON_TEXT_BY_ID.get(reason) + if (known !== undefined) return known + for (const { prefix, text } of DYNAMIC_SCOPE_REASON_TEXT) { + if (reason.startsWith(prefix)) return text(reason.slice(prefix.length)) + } + // Unknown reason: show it verbatim rather than swallow it. A prompt that reads + // awkwardly is recoverable; one missing the reason it is interrupting for is not. + return reason +} + +/** + * Plain-English sentences for a reason list, in order, with duplicates collapsed. + * + * Deduping happens on the resolved text, not the identifier, so the several + * rules that describe one underlying fact (a heredoc and a `-c` body; `~/` and + * `$HOME`) contribute a single line to the prompt. + */ +export function describeShellScopeReasons(reasons: readonly string[]): string[] { + const described: string[] = [] + for (const reason of reasons) { + const text = describeShellScopeReason(reason) + if (!described.includes(text)) described.push(text) + } + return described +} diff --git a/src/main/services/hooks/command-hook-runner.ts b/src/main/services/hooks/command-hook-runner.ts index 4336de11ab..62c7819eb5 100644 --- a/src/main/services/hooks/command-hook-runner.ts +++ b/src/main/services/hooks/command-hook-runner.ts @@ -185,7 +185,7 @@ export function applySandboxBlock( parseOk: false, spineEvent: interpretation.spineEvent, spineDecision: { ...interpretation.spineDecision, sandboxBlocked: true }, - runtimeError: `blocked by the macOS project sandbox (${detection.reasons.join('; ')})`, + runtimeError: `blocked by the project sandbox (${detection.reasons.join('; ')})`, } } diff --git a/src/main/services/security/permission-policy.ts b/src/main/services/security/permission-policy.ts index c10b33294c..9a5d269d3d 100644 --- a/src/main/services/security/permission-policy.ts +++ b/src/main/services/security/permission-policy.ts @@ -1,6 +1,10 @@ import type { McpToolAnnotations } from '@shared/types/mcp.ts' import { isRecord } from '@shared/unknown-value.ts' -import { analyzeShellCommand, dangerousInSandboxReasons } from './shell-scope.ts' +import { + analyzeShellCommand, + dangerousInSandboxReasons, + describeShellScopeReasons, +} from './shell-scope.ts' import type { ShellHarmDecision } from './shell-harm.ts' import type { ClassificationResult } from './safety-classifier.ts' import { isWebOriginAllowed, parseFetchUrl, webOriginKey } from './web-origin-policy.ts' @@ -297,10 +301,25 @@ export function shellPromptToApprovalFields(parts: ShellPromptParts): { } } +/** + * The classifier's reasons as the person answering the prompt reads them: one + * plain sentence per distinct concern, resolved by `describeShellScopeReasons` + * from the rule identifiers the classifier records. + * + * Bulleted even for a single reason, so the copy has one shape however many + * rules fired and a long sentence wraps under its own marker instead of + * trailing off the end of a paragraph. + */ +function reasonList(reasons: readonly string[]): string { + return describeShellScopeReasons(reasons) + .map((line) => `• ${line}`) + .join('\n') +} + export function formatShellPromptParts(command: string, reasons: string[]): ShellPromptParts { return { command, - ...(reasons.length ? { bodyFooter: `Reason: ${reasons.join('; ')}` } : {}), + ...(reasons.length ? { bodyFooter: `Why this needs approval:\n${reasonList(reasons)}` } : {}), } } @@ -337,6 +356,13 @@ export function formatPortBindingPromptParts( } } +/** + * Stand-in when the caller had no reasons to pass on, so the prompt still says + * something. It travels as a reason: `describeShellScopeReasons` passes text it + * does not recognise through unchanged. + */ +const NEEDS_OUTSIDE_ACCESS: readonly string[] = ['Needs network or outside-project access'] + export function formatExternalSandboxPromptBody(command: string, reasons: string[]): string { return flattenShellPromptParts(formatExternalSandboxPromptParts(command, reasons)) } @@ -345,10 +371,14 @@ export function formatExternalSandboxPromptParts( command: string, reasons: string[], ): ShellPromptParts { - const detail = reasons.length ? reasons.join('; ') : 'network or outside-workspace access' return { command, - bodyAdvice: `This command needs access the macOS project sandbox blocks (${detail}).`, + // The platform is deliberately unnamed: this prompt only appears while a + // project sandbox is active, which is seatbelt on macOS and bubblewrap on + // Linux, and naming the wrong one is worse than naming none. + bodyAdvice: `The project sandbox would block this command:\n${reasonList( + reasons.length ? reasons : NEEDS_OUTSIDE_ACCESS, + )}`, bodyFooter: 'Allow running it once outside the sandbox?', } } @@ -367,13 +397,13 @@ export function formatExpectedSandboxBlockPromptParts( command: string, reasons: string[], ): ShellPromptParts { - const detail = reasons.length ? reasons.join('; ') : 'network or outside-workspace access' return { command, bodyAdvice: - `The agent expects this command to need access the macOS project sandbox blocks ` + - `(${detail}) and is asking to run it outside the sandbox up front, rather than ` + - 'letting it fail inside first.', + `The agent expects the project sandbox to block this command:\n${reasonList( + reasons.length ? reasons : NEEDS_OUTSIDE_ACCESS, + )}\n\n` + + 'It is asking to run outside the sandbox up front, rather than letting it fail inside first.', bodyFooter: "This is the agent's expectation, not a confirmed sandbox block. " + 'Allow running it once outside the sandbox?', diff --git a/src/main/services/security/sandbox-failure.ts b/src/main/services/security/sandbox-failure.ts index 8e0865a8c2..7907d58ab4 100644 --- a/src/main/services/security/sandbox-failure.ts +++ b/src/main/services/security/sandbox-failure.ts @@ -2,7 +2,7 @@ import type { ShellPromptParts } from './permission-policy.ts' import { flattenShellPromptParts } from './permission-policy.ts' /** - * Detect when a shell command failed because the macOS project sandbox blocked it. + * Detect when a shell command failed because the project sandbox blocked it. * * SECURITY (issue #104): this detection must NOT use command-controlled stdout/stderr. * A command can trivially `echo "operation not permitted"` to fake a sandbox failure @@ -57,7 +57,7 @@ export function formatUnsandboxedPromptParts(command: string, reasons: string[]) const detail = reasons.length ? reasons.join('; ') : 'sandbox restriction suspected' return { command, - bodyAdvice: `This command failed inside the macOS project sandbox (${detail}).`, + bodyAdvice: `This command failed inside the project sandbox (${detail}).`, bodyFooter: 'Allow running it once without sandbox restrictions?', } } diff --git a/src/main/services/security/shell-prompt-copy.test.ts b/src/main/services/security/shell-prompt-copy.test.ts new file mode 100644 index 0000000000..140af7510e --- /dev/null +++ b/src/main/services/security/shell-prompt-copy.test.ts @@ -0,0 +1,99 @@ +/** + * The approval prompts a person actually reads. + * + * Reasons travel as rule identifiers (`inline script (interpreter -c/-e/--eval)`) + * because the classifier dedupes on them and the decision spine stores them. + * These tests pin the other end of that pipe: what reaches the dialog is a + * sentence about the command, and each distinct concern is stated once. + */ +import { describe, it } from 'node:test' +import assert from 'node:assert/strict' +import { analyzeShellCommand } from './shell-scope.ts' +import { + formatExpectedSandboxBlockPromptParts, + formatExternalSandboxPromptParts, + formatShellPromptParts, +} from './permission-policy.ts' + +const root = '/Users/me/project' + +describe('outside-sandbox approval copy', () => { + it('explains an opaque interpreter command without classifier jargon', () => { + const command = `python3 - <<'PY'\nprint(1)\nPY\npython3 -c "print(2)"` + const { bodyAdvice, bodyFooter } = formatExternalSandboxPromptParts( + command, + analyzeShellCommand(command, root).reasons, + ) + + assert.equal( + bodyAdvice, + 'The project sandbox would block this command:\n' + + "• Runs a script written inside the command itself, so Copse can't tell what it does", + ) + assert.equal(bodyFooter, 'Allow running it once outside the sandbox?') + }) + + it('lists several distinct concerns one per line', () => { + const command = 'curl -sL https://example.com/x > ~/notes.txt' + const { bodyAdvice } = formatExternalSandboxPromptParts( + command, + analyzeShellCommand(command, root).reasons, + ) + + assert.deepEqual(bodyAdvice?.split('\n'), [ + 'The project sandbox would block this command:', + '• Downloads from the internet (curl/wget)', + '• Reads or writes in your home directory, outside the project', + ]) + }) + + it('still says why when the caller had no reasons to pass on', () => { + const { bodyAdvice } = formatExternalSandboxPromptParts('some-tool', []) + assert.equal( + bodyAdvice, + 'The project sandbox would block this command:\n• Needs network or outside-project access', + ) + }) + + it('names no platform, because the sandbox is seatbelt or bubblewrap', () => { + const { bodyAdvice } = formatExternalSandboxPromptParts('curl https://example.com', [ + 'network download (curl/wget)', + ]) + assert.doesNotMatch(bodyAdvice ?? '', /macOS|Linux/) + }) + + it('keeps an expected block worded as an expectation', () => { + const { bodyAdvice, bodyFooter } = formatExpectedSandboxBlockPromptParts('gh pr list', [ + 'GitHub CLI (may reach GitHub)', + ]) + + assert.equal( + bodyAdvice, + 'The agent expects the project sandbox to block this command:\n' + + '• Runs the GitHub CLI, which may reach GitHub\n\n' + + 'It is asking to run outside the sandbox up front, rather than letting it fail inside first.', + ) + assert.match(bodyFooter ?? '', /not a confirmed sandbox block/) + }) +}) + +describe('in-sandbox approval copy', () => { + it('explains why a contained command is still being asked about', () => { + const command = 'rm -rf build' + const { bodyFooter } = formatShellPromptParts(command, [ + 'recursive/forced delete (rm -rf)', + 'find -delete bulk removal', + ]) + + assert.equal( + bodyFooter, + 'Why this needs approval:\n' + + '• Deletes files and folders recursively (rm -rf)\n' + + '• Deletes every file a search matches (find -delete)', + ) + }) + + it('omits the footer entirely when there is nothing to explain', () => { + assert.deepEqual(formatShellPromptParts('ls -la', []), { command: 'ls -la' }) + }) +}) diff --git a/src/renderer/views/approval-dialog-batch.test.ts b/src/renderer/views/approval-dialog-batch.test.ts index 7470db8491..05595320fb 100644 --- a/src/renderer/views/approval-dialog-batch.test.ts +++ b/src/renderer/views/approval-dialog-batch.test.ts @@ -285,6 +285,30 @@ describe('approval dialog coalescing', () => { assert.deepEqual(responses, [{ id: 'read-access', approved: true, remember: true }]) }) + it('keeps a multi-reason advice block on one line per reason', () => { + // `formatExternalSandboxPromptParts` sends the reasons as a bulleted block; + // `.approval-advice` is `white-space: pre-wrap`, so the newlines survive to + // the user as separate lines inside a single advice element. + emit({ + id: 'outside-sandbox', + title: 'Run outside sandbox?', + body: 'curl -sL https://example.com/x > ~/notes.txt', + bodyAdvice: + 'The project sandbox would block this command:\n' + + '• Downloads from the internet (curl/wget)\n' + + '• Reads or writes in your home directory, outside the project', + bodyFooter: 'Allow running it once outside the sandbox?', + }) + fireWindow() + const advice = qsRequired(dialog, '.approval-advice') + assert.equal(dialog.querySelectorAll('.approval-advice').length, 1) + assert.deepEqual(advice.textContent.split('\n'), [ + 'The project sandbox would block this command:', + '• Downloads from the internet (curl/wget)', + '• Reads or writes in your home directory, outside the project', + ]) + }) + it('renders bodyAdvice and bodyFooter outside the monospaced command block', () => { emit({ id: 'gy', diff --git a/src/shared/demo-scenarios.ts b/src/shared/demo-scenarios.ts index 06165b9ddd..6b5d1f6edc 100644 --- a/src/shared/demo-scenarios.ts +++ b/src/shared/demo-scenarios.ts @@ -597,7 +597,8 @@ export const DEMO_SCENARIOS: readonly DemoScenario[] = [ id: 'demo-approval-light-accent-request', title: 'Run outside sandbox?', body: 'npm install', - bodyAdvice: 'This command needs access the project sandbox blocks.', + bodyAdvice: + 'The project sandbox would block this command:\n• Installs or updates packages, which downloads and runs code from the internet', bodyFooter: 'Allow running it once outside the sandbox?', type: 'shell', }, @@ -629,7 +630,7 @@ export const DEMO_SCENARIOS: readonly DemoScenario[] = [ title: 'Run outside sandbox?', body: 'COREPACK_HOME="$TMPDIR/copse-corepack" corepack pnpm run check:oracle', bodyAdvice: - 'This command needs access the macOS project sandbox blocks (corepack downloads package-manager binaries).', + 'The project sandbox would block this command:\n• Downloads package-manager binaries (corepack)', bodyFooter: 'Allow running it once outside the sandbox?', type: 'shell', }, @@ -638,7 +639,7 @@ export const DEMO_SCENARIOS: readonly DemoScenario[] = [ title: 'Run outside sandbox?', body: 'COREPACK_HOME="$TMPDIR/copse-corepack" corepack pnpm run check:e2e-syntax', bodyAdvice: - 'This command needs access the macOS project sandbox blocks (corepack downloads package-manager binaries).', + 'The project sandbox would block this command:\n• Downloads package-manager binaries (corepack)', bodyFooter: 'Allow running it once outside the sandbox?', type: 'shell', }, @@ -647,7 +648,7 @@ export const DEMO_SCENARIOS: readonly DemoScenario[] = [ title: 'Run outside sandbox?', body: 'COREPACK_HOME="$TMPDIR/copse-corepack" corepack pnpm test', bodyAdvice: - 'This command needs access the macOS project sandbox blocks (corepack downloads package-manager binaries).', + 'The project sandbox would block this command:\n• Downloads package-manager binaries (corepack)', bodyFooter: 'Allow running it once outside the sandbox?', type: 'shell', }, diff --git a/tests/demo/approval-grouped-shell-commands.demo.ts b/tests/demo/approval-grouped-shell-commands.demo.ts index 02392ecf10..27f6f84e77 100644 --- a/tests/demo/approval-grouped-shell-commands.demo.ts +++ b/tests/demo/approval-grouped-shell-commands.demo.ts @@ -17,7 +17,7 @@ describe('browser-hosted grouped shell approval', () => { await expect(dialog.$$('.approval-body-list')).toBeElementsArrayOfSize(1) await expect(dialog.$$('.approval-body')).toBeElementsArrayOfSize(3) await expect(dialog.$('.approval-advice')).toHaveText( - 'This command needs access the macOS project sandbox blocks (corepack downloads package-manager binaries).', + 'The project sandbox would block this command:\n• Downloads package-manager binaries (corepack)', ) await expect(dialog.$('.approval-footer')).toHaveText( 'Allow running it once outside the sandbox?', From 3f246ed6385f8d0888d628aadd271506c7a026e3 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 21:59:04 +0000 Subject: [PATCH 2/5] fix(permissions): apply the plain-English reasons to the harm and install prompts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rendering every shell approval variant turned up two surfaces the first pass missed. The Guarded YOLO harm prompt still printed `REASON_RECURSIVE_DELETE` and `REASON_FIND_DELETE` raw. Those constants are shared between the harm gate and the scope classifier precisely so the two agree on the wording, so an ordinary prompt saying "Deletes files and folders recursively (rm -rf)" beside a harm prompt saying "recursive/forced delete (rm -rf)" is the divergence they exist to prevent. It resolves them through the same copy layer now; harm-only reasons have no entry and pass through untouched, ahead of the existing truncation. The install and ephemeral-runner prompts still said "outside the macOS sandbox", which is wrong on Linux, where bubblewrap is the boundary — the same one-word fix already applied to the sibling prompts. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Q9xawQf1Nu7VGA5Ktew66E --- .../services/security/permission-gate.test.ts | 21 ++++++++++++++++--- .../services/security/permission-policy.ts | 11 +++++++--- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/main/services/security/permission-gate.test.ts b/src/main/services/security/permission-gate.test.ts index b83f19fc9b..a3f4a35918 100644 --- a/src/main/services/security/permission-gate.test.ts +++ b/src/main/services/security/permission-gate.test.ts @@ -21,6 +21,7 @@ import { WEB_ALLOW_USER_APPROVAL_SETTING, } from './web-origin-policy.ts' import { detectSandboxFailure } from './sandbox-failure.ts' +import { REASON_RECURSIVE_DELETE } from './shell-scope.ts' import { setPermissionGateForTests } from '../tool-registry.ts' import { ensureShellCommandPermitted, @@ -1478,19 +1479,21 @@ describe('formatInstallPromptParts', () => { assert.ok(!parts.bodyAdvice?.includes('install lifecycle scripts')) }) - it('explains the macOS sandbox exit only when running outside it', () => { + it('explains the sandbox exit only when running outside it', () => { const outside = formatInstallPromptParts('npm install', { outsideSandbox: true, safeInstall: true, jsManager: true, }) - assert.ok(outside.bodyAdvice?.includes('outside the macOS sandbox')) + assert.ok(outside.bodyAdvice?.includes('outside the project sandbox')) + // No platform in the copy: the sandbox is seatbelt on macOS, bubblewrap on Linux. + assert.doesNotMatch(outside.bodyAdvice ?? '', /macOS|Linux/) const inside = formatInstallPromptParts('npm install', { outsideSandbox: false, safeInstall: true, jsManager: true, }) - assert.ok(!inside.bodyAdvice?.includes('macOS sandbox')) + assert.ok(!inside.bodyAdvice?.includes('project sandbox')) }) it('warns when package scanning is disabled in Settings', () => { @@ -1515,6 +1518,18 @@ describe('formatGuardedYoloHarmPromptAdvice', () => { assert.ok(advice.includes('Guarded YOLO cannot skip this confirmation')) }) + it('reads a shared classifier reason the same way the ordinary prompt does', () => { + // `REASON_RECURSIVE_DELETE` is shared between the harm gate and the scope + // classifier so the two agree on the wording; both dialogs resolve it to the + // same sentence. Harm-only reasons have no entry and pass through untouched. + const advice = formatGuardedYoloHarmPromptAdvice([ + REASON_RECURSIVE_DELETE, + 'script contents could not be inspected safely: foo.sh', + ]) + assert.ok(advice.includes('Potential harm: Deletes files and folders recursively (rm -rf)')) + assert.ok(advice.includes('script contents could not be inspected safely: foo.sh')) + }) + it('caps enormous operands and total advice length', () => { const hugeOperand = `script contents could not be inspected safely: ${'a/'.repeat(500)}z` const advice = formatGuardedYoloHarmPromptAdvice([hugeOperand, hugeOperand, hugeOperand]) diff --git a/src/main/services/security/permission-policy.ts b/src/main/services/security/permission-policy.ts index 9a5d269d3d..cd5d1e4f34 100644 --- a/src/main/services/security/permission-policy.ts +++ b/src/main/services/security/permission-policy.ts @@ -424,7 +424,12 @@ function truncateGuardedYoloHarmReason(reason: string): string { export function formatGuardedYoloHarmPromptAdvice(reasons: string[]): string { const footer = '\n\nGuarded YOLO cannot skip this confirmation. Approve this bounded destructive action once?' - const listed = reasons.map(truncateGuardedYoloHarmReason) + // `shell-harm.ts` writes most of its own reasons as readable phrases, but the + // few it shares with the classifier (`REASON_RECURSIVE_DELETE`, …) are + // identifiers. Resolving them here is what keeps the wording those constants + // are shared for from diverging between this dialog and the ordinary prompt; + // harm reasons with no entry pass through untouched. + const listed = describeShellScopeReasons(reasons).map(truncateGuardedYoloHarmReason) let harm = listed.length ? `Potential harm: ${listed.join('; ')}` : 'Potential harm: unknown' const maxHarmChars = Math.max(24, MAX_GUARDED_YOLO_HARM_ADVICE_CHARS - footer.length) if (harm.length > maxHarmChars) { @@ -451,7 +456,7 @@ export function formatInstallPromptParts( opts: { outsideSandbox: boolean; safeInstall: boolean; jsManager: boolean }, ): ShellPromptParts { const access = opts.outsideSandbox - ? 'It runs once outside the macOS sandbox with network access.' + ? 'It runs once outside the project sandbox with network access.' : 'It fetches packages over the network.' const scan = opts.safeInstall ? `Socket Firewall (sfw) scans the packages for known-malicious code${ @@ -482,7 +487,7 @@ export function formatEphemeralRunnerPromptParts( opts: { outsideSandbox: boolean; safeInstall: boolean }, ): ShellPromptParts { const access = opts.outsideSandbox - ? 'It runs once outside the macOS sandbox with network access.' + ? 'It runs once outside the project sandbox with network access.' : 'It may reach the network.' const scan = opts.safeInstall ? 'Socket Firewall (sfw) scans packages for known-malicious code.' From af961db3e5587400298f64105dee862eda0ca086 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 22:43:59 +0000 Subject: [PATCH 3/5] fix(permissions): address review on the plain-English reason copy Four findings from review, plus one the review's last item turned up. - **The typecheck guarantee now matches what the docs claim.** `addReason` in the token pass took a plain `string`, and the shared reason constants were inferred, so editing one to a string with no `SCOPE_REASON_TEXT` entry compiled and fell through to "shown verbatim". Both classifier accumulators, the adder, the shared constants and the verdict-note literals are now typed `ScopeReason`. The list widens to `string[]` at exactly one point, where the runtime-built outside-path reason joins it. - **The dedupe comment claimed more than the code does.** `--eval` trips the interpreter rule and the generic dynamic-execution one, and those map to different sentences, so it reports two lines. The comment now says that, and the dynamic-execution sentence drops the trailing clause it shared with the interpreter one so the pair reads as two facts rather than one said twice. - **The Guarded YOLO cap test stopped exercising the cap.** Its three identical operands now dedupe to one line, leaving both the per-reason and the total budget untested. It uses distinct operands and enough of them to overrun the 1200-char total (advice lands at 267 and 1200 chars respectively). - **docs/shell-permissions.md** notes that the harm prompt keeps its one-paragraph shape, and describes the typing guarantee as it now is. Rendering the wrap case the review asked me to eyeball showed the second line of a long reason returning to the left margin, flush with the bullets, so it read as another bullet. Each bullet is now its own inline-block with a hanging indent, so a wrapped line sits under its own text. The newlines stay as text nodes between the spans, so the advice element's `textContent` is still exactly the string the main process sent, and every existing assertion on it holds. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Q9xawQf1Nu7VGA5Ktew66E --- docs/shell-permissions.md | 14 +++-- packages/shell-guard/src/shell-scope.ts | 56 +++++++++++-------- .../services/security/permission-gate.test.ts | 21 +++++-- src/renderer/styles/global/approval.css | 9 +++ .../views/approval-dialog-batch.test.ts | 11 ++++ src/renderer/views/approval-dialog.ts | 26 ++++++++- 6 files changed, 103 insertions(+), 34 deletions(-) diff --git a/docs/shell-permissions.md b/docs/shell-permissions.md index 65187b8496..bec69b4bac 100644 --- a/docs/shell-permissions.md +++ b/docs/shell-permissions.md @@ -73,15 +73,19 @@ decision spine, so they must stay stable — which is why they read like rules `shell-scope.ts` therefore keeps a second table, `SCOPE_REASON_TEXT`, holding one plain-English sentence per reason, and `describeShellScopeReasons` resolves a reason list into sentences at the -moment a prompt is built. The prompt formatters in `permission-policy.ts` render those as a bullet -per line. Logs, hooks and the decision spine keep the identifiers. +moment a prompt is built. The shell prompt formatters in `permission-policy.ts` render those as a +bullet per line; the Guarded YOLO harm prompt resolves the same sentences but keeps its existing +one-paragraph `Potential harm: …` shape, because it is capped by length rather than by line. Logs, +hooks and the decision spine keep the identifiers. Two properties this contract depends on: - **Every rule has copy.** `ScopeReason` is derived from the keys of `SCOPE_REASON_TEXT` and - annotates the pattern tables, so a new classifier rule whose reason has no sentence fails to - typecheck. Reasons built at runtime (`absolute path outside workspace: …`) are matched by prefix; - anything still unrecognised is shown verbatim rather than dropped. + annotates the pattern tables, the shared reason constants, and the accumulators both classifier + passes push through, so a new classifier rule whose reason has no sentence fails to typecheck. + The one deliberate exception is the runtime-built `absolute path outside workspace: …`, which + bakes in an operand and so is matched by prefix instead; anything still unrecognised is shown + verbatim rather than dropped. - **One concern, one line.** Deduping happens on the resolved sentence, so rules that describe the same underlying fact collapse — a heredoc and a `-c` body are both "runs a script written inside the command itself", and `~/` and `$HOME` are both "in your home directory". diff --git a/packages/shell-guard/src/shell-scope.ts b/packages/shell-guard/src/shell-scope.ts index 44332879c3..a511dec9c9 100644 --- a/packages/shell-guard/src/shell-scope.ts +++ b/packages/shell-guard/src/shell-scope.ts @@ -84,7 +84,7 @@ const CMD_POS = String.raw`(?:^|[\n|;&(])\s*` // this classifier (and to the file-blind safety classifier) — a hard escape even // under the sandbox, exactly like `node ./x.js`. Shared verbatim between the regex // entry and the token layer so the two dedupe against each other. (#581) -const REASON_LOCAL_EXECUTABLE = +const REASON_LOCAL_EXECUTABLE: ScopeReason = 'executes an in-workspace file directly (contents opaque to analysis)' // Commands that clearly reach outside the workspace or network. @@ -252,7 +252,7 @@ const EXTERNAL_PATTERNS: Array<{ re: RegExp; reason: ScopeReason; ambiguous?: bo ] /** Shared so the chat-store waiver below can name the rule it may waive. */ -const REASON_HOME_PATH = 'home directory path (~/)' +const REASON_HOME_PATH: ScopeReason = 'home directory path (~/)' // Paths that indicate access outside the workspace. const OUTSIDE_PATH_PATTERNS: Array<{ re: RegExp; reason: ScopeReason }> = [ @@ -302,10 +302,10 @@ const REGISTRY_REDIRECT_PATTERNS: Array<{ re: RegExp; reason: ScopeReason }> = [ // means it cannot fall behind the regex baseline it is meant to reinforce. // Exact reason strings shared with the regex entries above, so token-derived and // regex-derived hits dedupe against each other. -const REASON_INTERPRETER_FILE = +const REASON_INTERPRETER_FILE: ScopeReason = 'runs a local script via an interpreter (contents opaque to analysis)' -const REASON_INTERPRETER_INLINE = 'inline script (interpreter -c/-e/--eval)' -const REASON_BUILD_DRIVER = +const REASON_INTERPRETER_INLINE: ScopeReason = 'inline script (interpreter -c/-e/--eval)' +const REASON_BUILD_DRIVER: ScopeReason = 'build driver may require host caches or system build services (xcodebuild/gradle/swift/cargo)' const HEREDOC_INTERPRETER = /\b(?:python3?|node|deno|bun|ruby|perl)\b/ @@ -456,11 +456,14 @@ function isHostDependentBuildDriver(exe: string, args: string[]): boolean { * harm gate and trusted-command routing; anything no lexer there can turn into a * plain argv is left entirely to the regex fallback. */ -function tokenBasedExternalReasons(command: string): { reasons: string[]; hasHard: boolean } { - const reasons: string[] = [] +function tokenBasedExternalReasons(command: string): { + reasons: ScopeReason[] + hasHard: boolean +} { + const reasons: ScopeReason[] = [] let hasHard = false - const addReason = (reason: string): void => { + const addReason = (reason: ScopeReason): void => { if (!reasons.includes(reason)) reasons.push(reason) } @@ -501,8 +504,8 @@ function tokenBasedExternalReasons(command: string): { reasons: string[]; hasHar // (network download, install, git push, command substitution, …) rather than a // fuzzy `ambiguous` matcher. It decides whether the verdict is `external` // (prompt + run outside) or merely `ambiguous` (auto-run inside the sandbox). -function collectExternalReasons(command: string): { reasons: string[]; hasHard: boolean } { - const reasons: string[] = [] +function collectExternalReasons(command: string): { reasons: ScopeReason[]; hasHard: boolean } { + const reasons: ScopeReason[] = [] let hasHard = false const shellCommand = maskInterpreterHeredocBodies(command) const variants = [shellCommand, normalizeShellCommandForAnalysis(shellCommand)] @@ -691,7 +694,7 @@ const DANGEROUS_IN_SANDBOX_PATTERNS: Array<{ re: RegExp; reason: ScopeReason }> /** Destructive/resource-exhausting patterns that warrant a prompt even when sandboxed. */ export function dangerousInSandboxReasons(command: string): string[] { - const reasons: string[] = [] + const reasons: ScopeReason[] = [] const variants = [command, normalizeShellCommandForAnalysis(command)] for (const text of variants) { for (const { re, reason } of DANGEROUS_IN_SANDBOX_PATTERNS) { @@ -707,10 +710,13 @@ export function analyzeShellCommand( ): ShellScopeAnalysis { const trimmed = command.trim() if (!trimmed) { - return { verdict: 'sandbox', reasons: ['empty command'] } + return { verdict: 'sandbox', reasons: ['empty command'] satisfies ScopeReason[] } } - const { reasons, hasHard } = collectExternalReasons(trimmed) + const { reasons: scopeReasons, hasHard } = collectExternalReasons(trimmed) + // Widened here and only here: `referencesOutsideWorkspace` can return the one + // runtime-built reason, which by construction is not a `ScopeReason` key. + const reasons: string[] = scopeReasons // Outside-workspace filesystem access is always a hard escape: we want such // commands to prompt and run outside the sandbox, not attempt-then-retry. @@ -722,7 +728,10 @@ export function analyzeShellCommand( if (reasons.length === 0) { // Local-only commands with no escape signals are sandbox-contained. - return { verdict: 'sandbox', reasons: ['no network or outside-path signals detected'] } + return { + verdict: 'sandbox', + reasons: ['no network or outside-path signals detected'] satisfies ScopeReason[], + } } // Only fuzzy "may reach" matchers fired → ambiguous: safe to auto-run inside an @@ -807,12 +816,16 @@ export function isReplayableOpaqueLocalExecution(analysis: ShellScopeAnalysis): * typecheck: a rule cannot ship without copy the person answering the prompt can * understand. * - * Several identifiers deliberately map to the SAME sentence. A `-c` body and a - * heredoc are one fact to whoever is approving ("this runs code I can't see"), - * and `--eval` trips the generic dynamic-execution matcher as well as the - * interpreter one. `describeShellScopeReasons` dedupes on the resolved text, so - * the dialog states each distinct concern once instead of listing the two or - * three internal rules that happened to fire. + * Some identifiers deliberately map to the SAME sentence, and + * `describeShellScopeReasons` dedupes on the resolved text, so the rules that + * describe one fact contribute one line: a `-c` body and a heredoc are both + * "this runs code I can't see" to whoever is approving, and `~/` and `$HOME` + * are both "in your home directory". + * + * Rules describing DIFFERENT facts keep different sentences even when they fire + * together. `node --eval x` reports both the interpreter rule and the generic + * dynamic-execution one, because `--eval` matches both; the two sentences are + * worded so the second adds something rather than restating the first. */ const SCOPE_REASON_TEXT = { // Network reach @@ -869,8 +882,7 @@ const SCOPE_REASON_TEXT = { "Runs a script written inside the command itself, so Copse can't tell what it does", 'heredoc script fed to an interpreter': "Runs a script written inside the command itself, so Copse can't tell what it does", - 'dynamic execution / encoding': - "Builds and runs code as it goes (eval/exec/base64), so Copse can't tell what it does", + 'dynamic execution / encoding': 'Builds and runs code as it goes (eval/exec/base64)', 'runs a local script via an interpreter (contents opaque to analysis)': "Runs a script file from the project, so Copse can't tell what it does", 'executes an in-workspace file directly (contents opaque to analysis)': diff --git a/src/main/services/security/permission-gate.test.ts b/src/main/services/security/permission-gate.test.ts index a3f4a35918..db2399e416 100644 --- a/src/main/services/security/permission-gate.test.ts +++ b/src/main/services/security/permission-gate.test.ts @@ -1531,11 +1531,22 @@ describe('formatGuardedYoloHarmPromptAdvice', () => { }) it('caps enormous operands and total advice length', () => { - const hugeOperand = `script contents could not be inspected safely: ${'a/'.repeat(500)}z` - const advice = formatGuardedYoloHarmPromptAdvice([hugeOperand, hugeOperand, hugeOperand]) - assert.ok(advice.length <= 1300) - assert.ok(advice.includes('…')) - assert.ok(advice.endsWith('Approve this bounded destructive action once?')) + // Operands must differ: reasons are deduped on their resolved text, so + // repeating one collapses to a single line and exercises neither cap. + const huge = (n: number): string => + `script contents could not be inspected safely: ${'a/'.repeat(500)}${String(n)}` + + // Per-reason cap, well inside the total budget. + const one = formatGuardedYoloHarmPromptAdvice([huge(0)]) + assert.ok(one.includes('…')) + assert.ok(one.length < 400) + + // Enough distinct reasons to overrun the total budget as well. + const many = formatGuardedYoloHarmPromptAdvice(Array.from({ length: 12 }, (_, i) => huge(i))) + assert.ok(many.length <= 1300) + assert.ok(many.length > 400) + assert.ok(many.includes('…')) + assert.ok(many.endsWith('Approve this bounded destructive action once?')) }) }) diff --git a/src/renderer/styles/global/approval.css b/src/renderer/styles/global/approval.css index a8ffb445da..ba07817e8c 100644 --- a/src/renderer/styles/global/approval.css +++ b/src/renderer/styles/global/approval.css @@ -54,6 +54,15 @@ color: var(--text-secondary); white-space: pre-wrap; } +/* One reason bullet. Its own box, so a reason too long for the dialog wraps with + * a hanging indent under its own text rather than back to the left margin, where + * the continuation would read as another bullet. */ +.approval-advice-item { + display: inline-block; + max-width: 100%; + padding-left: 1.05em; + text-indent: -1.05em; +} .approval-footer { margin-top: var(--spacing-sm); font-size: var(--font-size-sm); diff --git a/src/renderer/views/approval-dialog-batch.test.ts b/src/renderer/views/approval-dialog-batch.test.ts index 05595320fb..e16e9fbd4a 100644 --- a/src/renderer/views/approval-dialog-batch.test.ts +++ b/src/renderer/views/approval-dialog-batch.test.ts @@ -307,6 +307,17 @@ describe('approval dialog coalescing', () => { '• Downloads from the internet (curl/wget)', '• Reads or writes in your home directory, outside the project', ]) + + // Each bullet is its own box so a reason too long for the dialog wraps with a + // hanging indent; the lead-in line is not one. The newlines live between them, + // so the advice element's text is still exactly what the main process sent. + assert.deepEqual( + [...advice.querySelectorAll('.approval-advice-item')].map((node) => node.textContent), + [ + '• Downloads from the internet (curl/wget)', + '• Reads or writes in your home directory, outside the project', + ], + ) }) it('renders bodyAdvice and bodyFooter outside the monospaced command block', () => { diff --git a/src/renderer/views/approval-dialog.ts b/src/renderer/views/approval-dialog.ts index d8d8cf3bd6..245f3d38b3 100644 --- a/src/renderer/views/approval-dialog.ts +++ b/src/renderer/views/approval-dialog.ts @@ -31,6 +31,28 @@ export const APPROVAL_COALESCE_MS = 120 */ export const APPROVAL_SETTLE_MS = 500 +/** Marker `permission-policy.ts` prefixes each reason line with. */ +const ADVICE_BULLET = '\u2022 ' + +/** + * Advice text, with each reason bullet as its own inline-block so a line too long + * for the dialog hangs under its own text instead of returning to the left margin, + * where it reads as another bullet. + * + * The newlines stay as text nodes between the spans, so the element's + * `textContent` is still exactly the string the main process sent. + */ +function adviceElement(advice: string): HTMLElement { + const children: (Node | string)[] = [] + advice.split('\n').forEach((line, index) => { + if (index > 0) children.push('\n') + children.push( + line.startsWith(ADVICE_BULLET) ? el('span', { class: 'approval-advice-item' }, line) : line, + ) + }) + return el('div', { class: 'approval-advice' }, ...children) +} + /** * Timer factory returning a cancel function. Overridable so tests drive the * coalesce/settle windows deterministically instead of waiting on real time. @@ -318,7 +340,7 @@ export function mountApprovalDialog( if (hasSharedContext) { const sharedChildren: (Node | string)[] = [] if (firstRequest.bodyAdvice) { - sharedChildren.push(el('div', { class: 'approval-advice' }, firstRequest.bodyAdvice)) + sharedChildren.push(adviceElement(firstRequest.bodyAdvice)) } const bodyLabel = firstRequest.type === 'shell' ? 'Commands requiring approval' : 'Requests' sharedChildren.push( @@ -348,7 +370,7 @@ export function mountApprovalDialog( rowChildren.push(pickers.root) } else { if (req.bodyAdvice) { - rowChildren.push(el('div', { class: 'approval-advice' }, req.bodyAdvice)) + rowChildren.push(adviceElement(req.bodyAdvice)) } if (collapseDetails) rowChildren.push(detailsToggle()) rowChildren.push(requestBody(req)) From fa1c3d29bb6b1396842e7e8b2c9f5376df02c0fb Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 09:44:19 +0000 Subject: [PATCH 4/5] fix(permissions): state the unreadable-code concern once for --eval `node --eval x` trips the interpreter rule and the generic eval/exec/base64 rule, and the two mapped to different sentences, so the prompt said the same thing twice ("Runs a script written inside the command itself" and "Builds and runs code as it goes"). Both, like the heredoc rule, mean code this analysis cannot read before it runs, so all three now share one sentence and the resolved-text dedupe collapses them; the identifiers on the decision spine are unchanged. The table comment claimed the collapse already happened, and now describes what the code does. A regression test covers `--eval`. `analyzeShellCommand` widened the typed reason list by aliasing it as `string[]` before pushing the runtime-built outside-path reason into it, which wrote a plain string back into the `ScopeReason[]`; it copies instead. docs/shell-permissions.md quotes the shared sentence and notes that the Guarded YOLO harm prompt dedupes the same way but keeps its one-paragraph join rather than one bullet per line. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_011jaQbed96h8aSZLijfiHNB --- docs/shell-permissions.md | 6 ++-- packages/shell-guard/src/shell-scope.test.ts | 13 +++++++- packages/shell-guard/src/shell-scope.ts | 31 +++++++++---------- .../security/shell-prompt-copy.test.ts | 2 +- 4 files changed, 32 insertions(+), 20 deletions(-) diff --git a/docs/shell-permissions.md b/docs/shell-permissions.md index bec69b4bac..36f2e9f647 100644 --- a/docs/shell-permissions.md +++ b/docs/shell-permissions.md @@ -87,8 +87,10 @@ Two properties this contract depends on: bakes in an operand and so is matched by prefix instead; anything still unrecognised is shown verbatim rather than dropped. - **One concern, one line.** Deduping happens on the resolved sentence, so rules that describe the - same underlying fact collapse — a heredoc and a `-c` body are both "runs a script written inside - the command itself", and `~/` and `$HOME` are both "in your home directory". + same underlying fact collapse — a heredoc, a `-c` body and an `eval` are all "runs code written + or built inside the command itself" (so `node --eval`, which trips two rules, reads as one line), + and `~/` and `$HOME` are both "in your home directory". The Guarded YOLO harm prompt dedupes the + same way but joins the result into its one paragraph rather than one bullet per line. Prompts that offer a sandbox escape name no platform: they appear only while a project sandbox is active, which is seatbelt on macOS and bubblewrap on Linux. `permission-policy.ts` owns the diff --git a/packages/shell-guard/src/shell-scope.test.ts b/packages/shell-guard/src/shell-scope.test.ts index 9f628b6a1b..35b3029740 100644 --- a/packages/shell-guard/src/shell-scope.test.ts +++ b/packages/shell-guard/src/shell-scope.test.ts @@ -735,7 +735,18 @@ describe('describeShellScopeReasons', () => { ).reasons assert.equal(reasons.length, 2) assert.deepEqual(describeShellScopeReasons(reasons), [ - "Runs a script written inside the command itself, so Copse can't tell what it does", + "Runs code written or built inside the command itself, so Copse can't tell what it does", + ]) + }) + + it('reports an --eval body once although two rules match it', () => { + // `--eval` trips the interpreter rule and the generic eval/exec/base64 one. + // Both identifiers stay on the record; the person approving reads one line. + const reasons = analyzeShellCommand('node --eval "x"', root).reasons + assert.ok(reasons.includes('inline script (interpreter -c/-e/--eval)')) + assert.ok(reasons.includes('dynamic execution / encoding')) + assert.deepEqual(describeShellScopeReasons(reasons), [ + "Runs code written or built inside the command itself, so Copse can't tell what it does", ]) }) diff --git a/packages/shell-guard/src/shell-scope.ts b/packages/shell-guard/src/shell-scope.ts index a511dec9c9..be04ccd7d6 100644 --- a/packages/shell-guard/src/shell-scope.ts +++ b/packages/shell-guard/src/shell-scope.ts @@ -715,8 +715,9 @@ export function analyzeShellCommand( const { reasons: scopeReasons, hasHard } = collectExternalReasons(trimmed) // Widened here and only here: `referencesOutsideWorkspace` can return the one - // runtime-built reason, which by construction is not a `ScopeReason` key. - const reasons: string[] = scopeReasons + // runtime-built reason, which by construction is not a `ScopeReason` key. A + // copy, so the widened list never writes a plain string back into the typed one. + const reasons: string[] = [...scopeReasons] // Outside-workspace filesystem access is always a hard escape: we want such // commands to prompt and run outside the sandbox, not attempt-then-retry. @@ -818,14 +819,10 @@ export function isReplayableOpaqueLocalExecution(analysis: ShellScopeAnalysis): * * Some identifiers deliberately map to the SAME sentence, and * `describeShellScopeReasons` dedupes on the resolved text, so the rules that - * describe one fact contribute one line: a `-c` body and a heredoc are both - * "this runs code I can't see" to whoever is approving, and `~/` and `$HOME` - * are both "in your home directory". - * - * Rules describing DIFFERENT facts keep different sentences even when they fire - * together. `node --eval x` reports both the interpreter rule and the generic - * dynamic-execution one, because `--eval` matches both; the two sentences are - * worded so the second adds something rather than restating the first. + * describe one fact contribute one line: a `-c` body, a heredoc and an `eval` + * are all "this runs code I can't see" to whoever is approving, and `~/` and + * `$HOME` are both "in your home directory". `node --eval x` trips both the + * interpreter rule and the generic dynamic-execution one, and reads as one line. */ const SCOPE_REASON_TEXT = { // Network reach @@ -875,14 +872,16 @@ const SCOPE_REASON_TEXT = { 'launches a host app/file outside the sandbox (xdg-open)': 'Hands a file or URL to an app outside the sandbox (xdg-open)', - // Code this analysis cannot read before it runs. The heredoc and `-c` rules - // share one sentence on purpose: to whoever is approving, they are the same - // fact, and a command that does both should say it once. + // Code this analysis cannot read before it runs. The `-c`, heredoc and + // eval/exec/base64 rules share one sentence on purpose: to whoever is + // approving, they are the same fact, and a command that trips several of them + // (`node --eval` matches the first and the last) should say it once. 'inline script (interpreter -c/-e/--eval)': - "Runs a script written inside the command itself, so Copse can't tell what it does", + "Runs code written or built inside the command itself, so Copse can't tell what it does", 'heredoc script fed to an interpreter': - "Runs a script written inside the command itself, so Copse can't tell what it does", - 'dynamic execution / encoding': 'Builds and runs code as it goes (eval/exec/base64)', + "Runs code written or built inside the command itself, so Copse can't tell what it does", + 'dynamic execution / encoding': + "Runs code written or built inside the command itself, so Copse can't tell what it does", 'runs a local script via an interpreter (contents opaque to analysis)': "Runs a script file from the project, so Copse can't tell what it does", 'executes an in-workspace file directly (contents opaque to analysis)': diff --git a/src/main/services/security/shell-prompt-copy.test.ts b/src/main/services/security/shell-prompt-copy.test.ts index 140af7510e..4f225bc979 100644 --- a/src/main/services/security/shell-prompt-copy.test.ts +++ b/src/main/services/security/shell-prompt-copy.test.ts @@ -28,7 +28,7 @@ describe('outside-sandbox approval copy', () => { assert.equal( bodyAdvice, 'The project sandbox would block this command:\n' + - "• Runs a script written inside the command itself, so Copse can't tell what it does", + "• Runs code written or built inside the command itself, so Copse can't tell what it does", ) assert.equal(bodyFooter, 'Allow running it once outside the sandbox?') }) From 35dc90b17492abda681ccee365d65a0b95ffa8ab Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 09:44:20 +0000 Subject: [PATCH 5/5] test(permissions): pin the Guarded YOLO advice cap exactly The total-length case asserted `<= 1300`, looser than the 1200-char `MAX_GUARDED_YOLO_HARM_ADVICE_CHARS` it exists to exercise. Twelve distinct over-long operands overrun the budget and the advice lands on exactly 1200 characters, footer included, so the test now says so. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_011jaQbed96h8aSZLijfiHNB --- src/main/services/security/permission-gate.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/services/security/permission-gate.test.ts b/src/main/services/security/permission-gate.test.ts index db2399e416..1ce0126448 100644 --- a/src/main/services/security/permission-gate.test.ts +++ b/src/main/services/security/permission-gate.test.ts @@ -1541,10 +1541,10 @@ describe('formatGuardedYoloHarmPromptAdvice', () => { assert.ok(one.includes('…')) assert.ok(one.length < 400) - // Enough distinct reasons to overrun the total budget as well. + // Enough distinct reasons to overrun the total budget as well: the advice + // lands exactly on `MAX_GUARDED_YOLO_HARM_ADVICE_CHARS`, footer included. const many = formatGuardedYoloHarmPromptAdvice(Array.from({ length: 12 }, (_, i) => huge(i))) - assert.ok(many.length <= 1300) - assert.ok(many.length > 400) + assert.equal(many.length, 1200) assert.ok(many.includes('…')) assert.ok(many.endsWith('Approve this bounded destructive action once?')) })