6 months ago
9ff222d* fix: enforce read-only shell policy Co-authored-by: Max Weinbach <mweinbach@users.noreply.github.com> * test: tighten readonly shell policy coverage Co-authored-by: Max Weinbach <mweinbach@users.noreply.github.com> * fix: address readonly shell policy review feedback Co-authored-by: Max Weinbach <mweinbach@users.noreply.github.com> * Harden read-only shell policy and add bypass regression tests * Harden read-only shell command policy checks * fix: close readonly shell policy review gaps Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> * Fix shell policy parser bypasses and tee false positives * fix: close remaining shell policy review gaps The PR picked up two newer shell-policy review findings after the first follow-up push. The shared parser now skips launcher options with values before looking for -c payloads and walks nested shell-command payloads through a bounded queue so deeper shells still get inspected. Constraint: Read-only shell enforcement must stay at the shared harness policy layer Rejected: Patch only the specific bash examples from review | leaves the option-value and nesting bugs in the generic parser Confidence: high Scope-risk: narrow Directive: Keep shell-command detection launcher-aware and candidate traversal bounded; do not reintroduce shallow depth cutoffs around nested -c parsing Tested: ~/.bun/bin/bun test test/bash.readonly-policy.test.ts test/tools.test.ts; ~/.bun/bin/bun run typecheck Not-tested: Full bun test suite Co-authored-by: Codex <noreply@openai.com> * fix: address latest shell policy review gaps New review comments on the PR exposed two more read-only shell bypasses: newline-delimited commands were flattened before raw-rule parsing, and standalone brace groups were not split before executable detection. The tokenizer now preserves those shell boundaries while still treating escaped line continuations as continuations rather than separators. Constraint: Read-only enforcement must remain centralized in the shared shell policy parser Rejected: Add point fixes for just the reported examples | would leave newline and brace semantics inconsistent across the parser Confidence: high Scope-risk: narrow Directive: Preserve real shell command boundaries for raw-rule checks; do not collapse newlines or standalone grouping tokens before tokenizer-based mutation detection Tested: ~/.bun/bin/bun test test/bash.readonly-policy.test.ts test/tools.test.ts; ~/.bun/bin/bun run typecheck Not-tested: Full bun test suite Co-authored-by: Codex <noreply@openai.com> * fix: address quoted shell redirect review gap A fresh review on the latest PR commit showed that quoted redirect targets still bypassed the write detector because the old regex ran on quote-stripped input. The shell policy now detects redirections from raw input with quote-aware scanning, so quoted filenames are still blocked without regressing harmless quoted literals containing >. Constraint: Quoted shell literals that merely mention > must remain allowed for read-only inspection commands Rejected: Expand the stripped regex to cover quotes | still loses shell structure and keeps redirection parsing brittle Confidence: high Scope-risk: narrow Directive: Keep redirection detection on raw shell input with quote-aware scanning; do not move it back onto quote-stripped text Tested: ~/.bun/bin/bun test test/bash.readonly-policy.test.ts test/tools.test.ts; ~/.bun/bin/bun run typecheck Not-tested: Full bun test suite Co-authored-by: Codex <noreply@openai.com> * fix: address shell reserved-word review gap A new review on the latest PR SHA showed that shell reserved words like if/for/while and the ! negation operator could still mask later mutating commands from executable detection. The parser now treats those control-flow tokens as segment boundaries so the real executable remains visible to the shared read-only policy checks. Constraint: Compound shell forms must still be evaluated through the shared harness policy instead of ad hoc command allowlists Rejected: Only special-case if and ! from the current examples | leaves the same control-flow blind spot for other reserved-word forms Confidence: high Scope-risk: narrow Directive: Keep shell control-flow tokens out of executable resolution; if new shell forms appear, extend the shared separator/control-token set rather than layering one-off bypass checks Tested: ~/.bun/bin/bun test test/bash.readonly-policy.test.ts test/tools.test.ts; ~/.bun/bin/bun run typecheck Not-tested: Full bun test suite Co-authored-by: Codex <noreply@openai.com> * fix: harden remaining shell policy mutation paths The newest PR review found three more bypass families in the shared shell policy: mutating git subcommands outside the small write set, package-manager global flags placed before install verbs, and long nested shell chains that could exceed the fixed candidate scan cap. This pass tightens all three at the harness policy boundary. Constraint: Read-only shell enforcement must stay conservative even when command syntax varies across git and package manager CLIs Rejected: Patch only the specific review examples | would leave the same classification gaps for adjacent mutating subcommands and option forms Confidence: high Scope-risk: moderate Directive: Keep git/package mutation detection syntax-aware and avoid fixed scan caps that can drop later executed payloads Tested: ~/.bun/bin/bun test test/bash.readonly-policy.test.ts test/tools.test.ts; ~/.bun/bin/bun run typecheck Not-tested: Full bun test suite Co-authored-by: Codex <noreply@openai.com> * fix: catch remaining env and pip shell policy bypasses The latest PR review exposed two more syntax variants that bypassed the shared read-only shell guard: GNU env short options with attached values and pip global flags placed before install. This pass extends the existing parser and install classifier so those common forms stay within the same centralized write-detection boundary. Constraint: Read-only shell enforcement must cover common CLI syntax variations without moving logic into the UI or prompt layer Rejected: Special-case only env -C. and pip --quiet | would leave the same parser gap for adjacent flag variants Confidence: high Scope-risk: narrow Directive: When CLI flags can legally appear before a mutating verb, extend the shared argument scanner instead of assuming fixed positions Tested: ~/.bun/bin/bun test test/bash.readonly-policy.test.ts test/tools.test.ts; ~/.bun/bin/bun run typecheck Not-tested: Full bun test suite Co-authored-by: Codex <noreply@openai.com> * fix: cover remaining env redirection and pip policy gaps The latest review cycle surfaced three more shell-policy gaps: GNU env attached short-option values, package-manager -C case normalization, and shell syntax forms that still bypassed the shared write detector (<> redirections and nested escaped backticks). This pass closes those variants while keeping the harness-level policy as the single enforcement boundary. Constraint: The read-only shell guard must stay syntax-aware across common CLI and shell variants without regressing benign quoted inspection commands Rejected: Patch only the exact comment examples | would leave adjacent flag and substitution variants outside the shared parser contracts Confidence: high Scope-risk: narrow Directive: Normalize option matching to the parser's lowercase view and keep recursive substitution scanning faithful to executable shell syntax, not string-literal approximations Tested: ~/.bun/bin/bun test test/bash.readonly-policy.test.ts test/tools.test.ts; ~/.bun/bin/bun run typecheck Not-tested: Full bun test suite Co-authored-by: Codex <noreply@openai.com> * test: harden package manifest CI timeout budget GitHub's ubuntu runner hit the package-manifest dry-run assertion's default 5s limit even though the underlying pack manifest assertions still pass locally. The test now carries an explicit higher timeout so CI evaluates the packaging contract instead of flaking on runner speed. Constraint: CI runner timing is authoritative for this lane, even when local repro is faster Rejected: Retry the failed job without code changes | leaves the same timeout budget in place for the next runner Confidence: high Scope-risk: narrow Directive: Keep packaging-manifest checks focused on contract assertions and give runner-bound npm pack dry-runs an explicit timeout budget Tested: ~/.bun/bin/bun test test/package-manifest.test.ts; ~/.bun/bin/bun test --max-concurrency 1 -t 'packs runtime assets and excludes repo-only baggage'; ~/.bun/bin/bun run typecheck Not-tested: Full bun test suite Co-authored-by: Codex <noreply@openai.com> * fix: close fish shell policy parsing gap A new review on the latest PR SHA showed that Fish's uppercase -C init-command flag was being lowercased into -c before option-value matching, so the parser mistook it for the real command payload flag and skipped the later mutating -c command. This pass restores case-aware shell option matching for launcher-specific flags and keeps the dead thirdArg cleanup in the package-install classifier. Constraint: Shell option parsing must preserve case where the underlying shell treats -C and -c as different flags Rejected: Lowercase every launcher option key globally | breaks shells whose uppercase and lowercase short flags have different meanings Confidence: high Scope-risk: narrow Directive: For shell-specific launcher flags, preserve the shell's own case semantics before falling back to generic normalized flag handling Tested: ~/.bun/bin/bun test test/bash.readonly-policy.test.ts test/tools.test.ts; ~/.bun/bin/bun run typecheck Not-tested: Full bun test suite Co-authored-by: Codex <noreply@openai.com> * fix: handle env split-string and eval shell payloads The last practical review findings were both string-executing shell surfaces: GNU env -S and eval. This pass teaches recursive candidate extraction to inspect those payloads through the same shared harness policy path, rather than leaving them as blind spots in read-only mode. Constraint: Embedded shell command strings must be enforced at the shared parser boundary rather than by ad hoc command-specific blocks in callers Rejected: Mark env -S and eval as globally forbidden tokens | would hide the real executed payload and reduce future extensibility for adjacent executor forms Confidence: high Scope-risk: narrow Directive: When a shell primitive executes string content, feed that content back through recursive policy scanning instead of treating the primitive itself as the final executable Tested: ~/.bun/bin/bun test test/bash.readonly-policy.test.ts test/tools.test.ts; ~/.bun/bin/bun run typecheck Not-tested: Full bun test suite Co-authored-by: Codex <noreply@openai.com> * Harden shell policy against inline interpreter and python install bypass * Update commandPolicy.ts --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Max Weinbach <mweinbach@users.noreply.github.com> Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> Co-authored-by: Codex <noreply@openai.com>
Parenta8e471b
