diff --git a/docs/shell-permissions.md b/docs/shell-permissions.md index 0cdc133307..36f2e9f647 100644 --- a/docs/shell-permissions.md +++ b/docs/shell-permissions.md @@ -64,6 +64,39 @@ 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 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, 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, 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 +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/hooks-dialects/src/command-hook-runner.ts b/packages/hooks-dialects/src/command-hook-runner.ts index 35a4144371..192e88960a 100644 --- a/packages/hooks-dialects/src/command-hook-runner.ts +++ b/packages/hooks-dialects/src/command-hook-runner.ts @@ -210,7 +210,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/packages/hooks-dialects/src/sandbox-failure-detection.ts b/packages/hooks-dialects/src/sandbox-failure-detection.ts index 7fe89c5518..f4fd7d0f96 100644 --- a/packages/hooks-dialects/src/sandbox-failure-detection.ts +++ b/packages/hooks-dialects/src/sandbox-failure-detection.ts @@ -1,5 +1,5 @@ /** - * 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 diff --git a/packages/shell-guard/src/shell-scope.test.ts b/packages/shell-guard/src/shell-scope.test.ts index e5e9bd27fa..35b3029740 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,66 @@ 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 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", + ]) + }) + + 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..be04ccd7d6 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. @@ -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 @@ -252,10 +252,10 @@ const EXTERNAL_PATTERNS: Array<{ re: RegExp; reason: string; ambiguous?: boolean ] /** 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: 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', @@ -302,10 +302,10 @@ const REGISTRY_REDIRECT_PATTERNS: Array<{ re: RegExp; reason: string }> = [ // 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)] @@ -668,7 +671,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' }, @@ -691,7 +694,7 @@ const DANGEROUS_IN_SANDBOX_PATTERNS: Array<{ re: RegExp; reason: string }> = [ /** 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,14 @@ 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. 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. @@ -722,7 +729,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 @@ -784,3 +794,185 @@ 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. + * + * 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, 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 + '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 `-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 code written or built inside the command itself, so Copse can't tell what it does", + 'heredoc script fed to an interpreter': + "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)': + "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/security/permission-gate.test.ts b/src/main/services/security/permission-gate.test.ts index b83f19fc9b..1ce0126448 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,12 +1518,35 @@ 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]) - 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: the advice + // lands exactly on `MAX_GUARDED_YOLO_HARM_ADVICE_CHARS`, footer included. + const many = formatGuardedYoloHarmPromptAdvice(Array.from({ length: 12 }, (_, i) => huge(i))) + assert.equal(many.length, 1200) + assert.ok(many.includes('…')) + assert.ok(many.endsWith('Approve this bounded destructive action once?')) }) }) diff --git a/src/main/services/security/permission-policy.ts b/src/main/services/security/permission-policy.ts index c10b33294c..cd5d1e4f34 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?', @@ -394,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) { @@ -421,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${ @@ -452,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.' diff --git a/src/main/services/security/sandbox-failure.ts b/src/main/services/security/sandbox-failure.ts index b99076f6cd..44aec6d339 100644 --- a/src/main/services/security/sandbox-failure.ts +++ b/src/main/services/security/sandbox-failure.ts @@ -17,7 +17,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..4f225bc979 --- /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 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?') + }) + + 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/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 7470db8491..e16e9fbd4a 100644 --- a/src/renderer/views/approval-dialog-batch.test.ts +++ b/src/renderer/views/approval-dialog-batch.test.ts @@ -285,6 +285,41 @@ 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', + ]) + + // 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', () => { emit({ id: 'gy', 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)) 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?',