Skip to content

feat(shell): make permissive exec the default policy#302

Merged
ytallo merged 2 commits into
mainfrom
feat/shell-permissive-default
Jun 20, 2026
Merged

feat(shell): make permissive exec the default policy#302
ytallo merged 2 commits into
mainfrom
feat/shell-permissive-default

Conversation

@ytallo

@ytallo ytallo commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

What

Makes the shell worker's shipped default permissive: an empty command allowlist (which the gate treats as "open — any command runs"), inherit_env: true, a 120s foreground timeout, and a denylist trimmed to catastrophic-only patterns.

Why

The read-only default allowlist (ls/cat/grep/…) blocks the tooling a coding agent actually needs — cargo, git, bash, make, node, python3. In practice that forces operators to allowlist commands one at a time, and a half-open list is defeated the moment any shell or interpreter is added to it. For a worker that already documents itself as "not a sandbox," the honest model is: leave exec open and rely on the real boundaries.

Security posture (unchanged boundaries)

  • fs jail (fs.host_root) still confines every shell::fs::* op and the per-call cwd.
  • sandbox backend (target: { kind: "sandbox" }) is still the boundary for untrusted input.
  • per-call env gate is untouched — PATH/HOME/LD_*/DYLD_* and interpreter startup keys can never be set per call (DANGEROUS_ENV_KEYS).
  • catastrophic denylist kept as an advisory tripwire: rm -rf /, dd if=, mkfs, fork bomb, shutdown/reboot, /etc/shadow.

The sub-execution denylist entries (find -exec, sed -i, node/python -c, npm run, env <cmd>) were dropped — they only existed to plug holes in a non-empty allowlist and are pure friction once exec is open.

Changes

  • config.yamlallowlist: [], inherit_env: true, max_timeout_ms: 120000, catastrophic-only denylist.
  • src/config.rs — replaced the shipped_config_blocks_env_exec_escape test with shipped_config_is_open_exec_with_catastrophic_denylist, pinning the new contract (open exec permits cargo/git/bash; rm -rf / still trips the denylist).
  • README.md — documents the open default.

Test plan

  • cargo test — full shell crate suite green (548 passed).
  • New unit test asserts the shipped config.yaml parses, is open, and the catastrophic denylist still rejects rm -rf /.
  • Note: this only changes the first-registration seed. An already-registered worker keeps its stored config (which wins over the file) and must be updated via configuration::set shell — it hot-reloads, no restart.

https://claude.ai/code/session_01Cy5KwY8y2NcPPnyo4LVste

Summary by CodeRabbit

  • Chores
    • Extended execution timeout limits
    • Environment variables now available to executed commands by default
    • Broadened command execution permissions while preserving safety guardrails against destructive operations

Ship an open command allowlist (empty == any command runs) so an agent
can use arbitrary build/test/VCS tooling (cargo, git, bash, make, node,
python3, …) without per-deployment allowlisting. The fs jail
(fs.host_root) plus the optional sandbox backend remain the security
boundary — this worker is explicitly not a sandbox — and a catastrophic
-only denylist stays as a tripwire for host-wrecking mistakes.

- config.yaml: allowlist now empty (open); inherit_env: true so
  toolchains find their env; max_timeout_ms raised to 120000 so a real
  build/test isn't reaped at 30s; denylist trimmed to host-wrecking
  patterns only (dropped find -exec / sed -i / node -c / npm run / env
  <cmd> sub-exec escapes, which are pure friction once exec is open).
- config.rs: replace the read-only-default unit test with one pinning
  the new contract — open exec permits cargo/git/bash, the catastrophic
  denylist still trips rm -rf /.
- README: document the open default.

Claude-Session: https://claude.ai/code/session_01Cy5KwY8y2NcPPnyo4LVste
@vercel

vercel Bot commented Jun 19, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
workers Ready Ready Preview, Comment Jun 20, 2026 12:08am

Request Review

@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

The shell worker's execution policy is relaxed: max_timeout_ms increases from 30 s to 120 s, inherit_env flips to true, the allowlist becomes empty (permitting all commands by default), and denylist_patterns is trimmed to only catastrophic host-wrecking patterns. The README example and the shipped-config test are updated to match.

Changes

Shell Worker Open-Exec Policy

Layer / File(s) Summary
Core defaults and exec allowlist/denylist policy
shell/config.yaml, shell/README.md
max_timeout_ms raised to 120 000 ms, inherit_env set to true, allowlist emptied to allow all commands, and denylist_patterns reduced from a broad tool-specific set to only catastrophic patterns (rm -rf /, fork-bomb, mkfs, dd if=, shutdown, reboot, /etc/shadow). README example and inline comments updated to document the new open-exec semantics.
Shipped config exec-policy test
shell/src/config.rs
Test renamed and rewritten: asserts that common commands (cargo, git, bash, make, node, python3) and env nmap are now allowed under the empty allowlist, while rm -rf / is still rejected by the denylist.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • andersonleal

Poem

🐇 The allowlist gate swung open wide,
Old denials pushed gently aside.
Just rm -rf / still gets the glare,
And mkfs — please don't go there!
With a longer clock and env in tow,
This rabbit's shell is good to go! 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(shell): make permissive exec the default policy' is a clear, concise description of the main change—shifting from restrictive to permissive command execution policy—which aligns with the primary objective and all file changes in the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/shell-permissive-default

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

skill-check — worker

0 verified, 24 skipped (no docs/).

Layer Result
structure
vale
ai
render

Four for four. Nicely done.

Satisfies CI `cargo fmt --all -- --check`; no logic change.

Claude-Session: https://claude.ai/code/session_01Cy5KwY8y2NcPPnyo4LVste
@ytallo ytallo merged commit 4a8c855 into main Jun 20, 2026
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants