Sandbox hardening: dynamic mode, env sanitization, leak detection#259
Merged
Sandbox hardening: dynamic mode, env sanitization, leak detection#259
Conversation
…ion, durable binaries Implement all 5 phases from docs/design-docs/sandbox-hardening.md: Phase 1 — Dynamic sandbox mode: Sandbox reads mode from shared ArcSwap on every wrap() call. RuntimeConfig.sandbox is now Arc<ArcSwap<SandboxConfig>> and reload_config() stores new values. Toggling sandbox via API takes effect immediately without restart. Phase 2 — Environment sanitization: All sandbox modes (bubblewrap, sandbox-exec, passthrough) now clear inherited env vars and re-inject only PATH + safe vars (HOME, USER, LANG, TERM, TMPDIR) + user- configured passthrough_env. Prevents system secrets from leaking to worker subprocesses. Phase 3 — Durable binary instruction: Worker prompt now includes the persistent tools/bin directory location. New GET /agents/tools endpoint lists installed binaries for dashboard observability. Phase 4 — OpenCode output protection: Extracted leak detection from SpacebotHook into shared secrets::scrub module. OpenCode worker now scans text and tool output for known secret patterns, terminating on detection. Added StreamScrubber for exact-match redaction with cross- chunk boundary handling. Phase 5 — send_file hardening: Added symlink traversal rejection to match the file tool's TOCTOU protection.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a durable Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/sandbox.rs (1)
182-199: Minor inconsistency in config passing to wrap methods.
wrap_bubblewrapandwrap_sandbox_execreceive&configas a parameter (lines 193, 196), butwrap_passthroughloads the config internally (line 337). This results in double config loading when sandbox is disabled (line 167 + line 337).While
ArcSwap::load()is cheap (atomic load), consider passing the already-loaded config towrap_passthroughfor consistency:♻️ Optional: Pass config to wrap_passthrough for consistency
- fn wrap_passthrough( - &self, - program: &str, - args: &[&str], - working_dir: &Path, - path_env: &str, - ) -> Command { - let config = self.config.load(); + fn wrap_passthrough( + &self, + program: &str, + args: &[&str], + working_dir: &Path, + path_env: &str, + config: &SandboxConfig, + ) -> Command {And update call sites:
- return self.wrap_passthrough(program, args, working_dir, &path_env); + return self.wrap_passthrough(program, args, working_dir, &path_env, &config);- SandboxBackend::None => self.wrap_passthrough(program, args, working_dir, &path_env), + SandboxBackend::None => self.wrap_passthrough(program, args, working_dir, &path_env, &config),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/sandbox.rs` around lines 182 - 199, The current call sites pass &config into wrap_bubblewrap and wrap_sandbox_exec but not wrap_passthrough, causing an extra internal config load; update wrap_passthrough to accept the already-loaded config reference (same type as the local config variable), change its signature (wrap_passthrough(..., config: &ConfigType)) and remove its internal ArcSwap::load() usage, and then update all call sites (including the early-return branch and any other callers) to pass &config consistently; keep wrap_bubblewrap and wrap_sandbox_exec calls unchanged except for using the same config type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/api/tools.rs`:
- Around line 33-51: Replace blocking std::fs calls in the async handler
list_tools with tokio async equivalents: call
tokio::fs::read_dir(&tools_bin).await (instead of std::fs::read_dir) and iterate
with the async directory stream, and call entry.metadata().await (instead of
entry.metadata()). On errors, map_err to StatusCode::INTERNAL_SERVER_ERROR when
read_dir fails and log metadata errors instead of silently continuing (use
tracing::warn or tracing::debug with the entry path and the error) so failures
are observable; update references to tools_bin, list_tools, and entry.metadata()
accordingly.
In `@src/secrets/scrub.rs`:
- Around line 158-161: The current split uses byte index `split_at` on
`scrubbed`, which can panic on multi-byte UTF-8; change the logic in the `if
self.max_secret_len > 0 && scrubbed.len() > self.max_secret_len` block to find a
safe character boundary at or before `split_at` (e.g., check
`scrubbed.is_char_boundary(split_at)` and if false walk back using
`char_indices()` to the last valid boundary), then set `self.tail` to the
substring after that boundary and return the substring before that boundary;
reference `self.max_secret_len`, `scrubbed`, `split_at`, and `self.tail` when
making the change.
In `@src/tools/send_file.rs`:
- Line 69: The match arm that currently silently discards errors from the
symlink_metadata call (the `_ => {}` arm) must not ignore I/O errors; modify the
code around the symlink_metadata call so that errors are handled or propagated
instead of dropped — for example, replace the match with use of the ? operator
or change the `_ => {}` arm to capture the error (e) and return Err(e) or log
the error (e.g., tracing::error!) and then return a suitable Err; locate the
symlink_metadata invocation and the `_ => {}` match arm in send_file.rs and
implement one of these error-handling behaviors (propagate with ?, return
Err(e), or log+return) to satisfy the guideline.
- Around line 54-71: The symlink check currently iterates components of the
already-resolved canonical path (`canonical` / `relative`) so it never sees
user-supplied symlinks; change the loop to iterate the original `path`'s
components instead (e.g. compute a `relative_original` from
`path.strip_prefix(&workspace_canonical)` or fallback to `&path`) and push those
components onto `walk` (still starting from `workspace_canonical`) and call
`walk.symlink_metadata()` to detect symlinks and return the same SendFileError;
keep the existing error behavior but replace `relative.components()` with the
original path components to reject symlink traversal prior to reading the file.
In `@src/tools/shell.rs`:
- Around line 128-136: After env_clear() in the Windows branch where you build
Command (variable c in the block using Command::new("cmd") /C &args.command and
c.current_dir(&working_dir)), re-inject the same "safe" environment variables as
Unix: check std::env::var for PATH, HOME, USER, LANG, TERM, TMPDIR and call
c.env(key, value) for each that exists; also iterate the module's
passthrough_env entries and re-inject any of those vars the caller requested
(using c.env(name, value) when std::env::var(name) is Ok). This mirrors
sandbox.wrap() behavior and makes Windows sanitization consistent with Unix.
---
Nitpick comments:
In `@src/sandbox.rs`:
- Around line 182-199: The current call sites pass &config into wrap_bubblewrap
and wrap_sandbox_exec but not wrap_passthrough, causing an extra internal config
load; update wrap_passthrough to accept the already-loaded config reference
(same type as the local config variable), change its signature
(wrap_passthrough(..., config: &ConfigType)) and remove its internal
ArcSwap::load() usage, and then update all call sites (including the
early-return branch and any other callers) to pass &config consistently; keep
wrap_bubblewrap and wrap_sandbox_exec calls unchanged except for using the same
config type.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
prompts/en/worker.md.j2src/api.rssrc/api/agents.rssrc/api/config.rssrc/api/server.rssrc/api/tools.rssrc/config.rssrc/hooks/spacebot.rssrc/main.rssrc/opencode/worker.rssrc/sandbox.rssrc/secrets.rssrc/secrets/scrub.rssrc/tools/send_file.rssrc/tools/shell.rs
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
Add standalone sandbox.mdx covering containment, env sanitization, leak detection, durable binaries, and dynamic mode switching. Update config, workers, tools, and permissions docs to reference the new page, add passthrough_env to the config table, and reflect env sanitization.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
docs/content/docs/(features)/tools.mdx (1)
188-192: Minor redundancy between "Status reporting" and "Fire-and-forget sends" sections.Line 188 already states that status updates use
try_send(non-blocking). The "Fire-and-forget sends" section at lines 190-192 repeats this information. Consider consolidating into a single description under "Status reporting" or removing the separate section.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/docs/`(features)/tools.mdx around lines 188 - 192, Remove the redundant "Fire-and-forget sends" subsection and consolidate its content into the existing "Status reporting" paragraph: mention that workers call set_status and that status updates use try_send (non-blocking) so updates are dropped when the channel is full rather than blocking the worker, and delete the duplicate lines under "Fire-and-forget sends" to avoid repetition.docs/content/docs/(configuration)/config.mdx (1)
558-559: Add a least-privilege warning forpassthrough_env.The current text may encourage forwarding broad credentials. Add a short caution to forward only strictly required vars.
✏️ Suggested wording tweak
-Regardless of mode, all worker subprocesses start with a clean environment. Only `PATH` (with `tools/bin` prepended), safe variables (`HOME`, `USER`, `LANG`, `TERM`, `TMPDIR`), and any `passthrough_env` entries are injected. Use `passthrough_env` to forward credentials set in Docker Compose or systemd to workers. +Regardless of mode, all worker subprocesses start with a clean environment. Only `PATH` (with `tools/bin` prepended), safe variables (`HOME`, `USER`, `LANG`, `TERM`, `TMPDIR`), and any `passthrough_env` entries are injected. Use `passthrough_env` with least privilege: only forward variables required by worker tools (for example, credentials set in Docker Compose or systemd).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/docs/`(configuration)/config.mdx around lines 558 - 559, Add a short least-privilege caution to the paragraph describing passthrough_env: mention that forwarding environment variables via passthrough_env should be limited to only the strictly required variables (e.g., specific credential or token names) to minimize exposure, and recommend avoiding broad or sensitive creds; reference the existing terms PATH, safe variables (HOME, USER, LANG, TERM, TMPDIR) and passthrough_env so reviewers can place the warning inline with that text.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/content/docs/`(configuration)/permissions.mdx:
- Line 12: Update the unconditional sandbox guarantees in the Permissions doc by
qualifying them to reflect mode/platform dependence: find the sentences
containing "OS-level containment is handled by the [Sandbox]..." and the other
instance that claims unconditional sandbox guarantees, and change wording to
something like "When sandboxing is enabled, the sandbox enforces..." and add a
note such as "capabilities vary by backend/platform" (or similar phrasing) so
both occurrences clearly state sandbox behavior is conditional and
backend-dependent.
In `@docs/content/docs/`(features)/workers.mdx:
- Around line 156-160: Update the docs paragraph to avoid absolutes about secret
visibility and to document memory-path write guards: change the sentence that
claims workers "never" see parent environment to state that workers only inherit
PATH, safe vars (HOME, USER, LANG, TERM, TMPDIR) and any explicitly configured
passthrough_env entries (so secrets forwarded via passthrough_env are visible if
configured), and add a sentence that the file tool rejects writes to identity
files (SOUL.md, IDENTITY.md, USER.md) and memory-paths with an error instructing
the LLM to use the proper tool; also mention that the exec tool blocks dangerous
env vars (LD_PRELOAD, DYLD_INSERT_LIBRARIES, etc.) as before and link to the
Sandbox page for full containment and leak-detection details (update the
paragraph containing "Worker subprocesses also start with a clean environment"
and the following sentences accordingly).
---
Nitpick comments:
In `@docs/content/docs/`(configuration)/config.mdx:
- Around line 558-559: Add a short least-privilege caution to the paragraph
describing passthrough_env: mention that forwarding environment variables via
passthrough_env should be limited to only the strictly required variables (e.g.,
specific credential or token names) to minimize exposure, and recommend avoiding
broad or sensitive creds; reference the existing terms PATH, safe variables
(HOME, USER, LANG, TERM, TMPDIR) and passthrough_env so reviewers can place the
warning inline with that text.
In `@docs/content/docs/`(features)/tools.mdx:
- Around line 188-192: Remove the redundant "Fire-and-forget sends" subsection
and consolidate its content into the existing "Status reporting" paragraph:
mention that workers call set_status and that status updates use try_send
(non-blocking) so updates are dropped when the channel is full rather than
blocking the worker, and delete the duplicate lines under "Fire-and-forget
sends" to avoid repetition.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
docs/content/docs/(configuration)/meta.jsonis excluded by!**/*.json
📒 Files selected for processing (5)
docs/content/docs/(configuration)/config.mdxdocs/content/docs/(configuration)/permissions.mdxdocs/content/docs/(configuration)/sandbox.mdxdocs/content/docs/(features)/tools.mdxdocs/content/docs/(features)/workers.mdx
✅ Files skipped from review due to trivial changes (1)
- docs/content/docs/(configuration)/sandbox.mdx
Tighten async filesystem handling, UTF-8-safe scrubbing, symlink validation, and Windows env sanitization parity while updating docs to reflect mode-dependent sandbox guarantees.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/content/docs/(features)/workers.mdx (1)
148-155: Qualify the kernel-enforcement claim to avoid over-promising.Line 154 reads unconditional, but sandboxing is mode/backend dependent. A small qualifier keeps this precise.
✏️ Suggested wording
-Worker shell and exec commands run inside an OS-level sandbox (bubblewrap on Linux, sandbox-exec on macOS). The entire host filesystem is mounted read-only except: +When sandbox mode is enabled and a backend is available, worker shell and exec commands run inside an OS-level sandbox (bubblewrap on Linux, sandbox-exec on macOS). The entire host filesystem is mounted read-only except: @@ -The agent's data directory (databases, config files) is explicitly re-mounted read-only. This is enforced at the kernel level -- no amount of command creativity can bypass it. +In that mode, the agent's data directory (databases, config files) is explicitly re-mounted read-only and enforced at the kernel level.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/docs/`(features)/workers.mdx around lines 148 - 155, The statement that the agent's data directory is "enforced at the kernel level" overstates guarantees; update the paragraph containing "Worker shell and exec commands..." and the sentence "The agent's data directory (databases, config files) is explicitly re-mounted read-only. This is enforced at the kernel level -- no amount of command creativity can bypass it." to add a qualification that enforcement depends on the sandbox backend and kernel support (e.g., bubblewrap on Linux, sandbox-exec on macOS) and may vary across platforms and configurations, e.g., replace the absolute claim with language like "where supported by the sandbox backend and kernel" or "depending on the sandbox backend and underlying OS/kernel guarantees."
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/sandbox.rs`:
- Around line 275-280: The passthrough loop re-applies user env entries from
config.passthrough_env after the hardened defaults and can overwrite
sanitized/reserved vars (e.g., PATH), weakening isolation; update each
passthrough application (the loops that iterate config.passthrough_env which
call cmd.arg("--setenv").arg(var_name).arg(value)) to skip any variable in the
reserved/sanitized set (e.g., PATH, LD_LIBRARY_PATH, PYTHONPATH, any keys you
populate earlier) or if that variable was already set by the hardened/default
logic — i.e., check var_name against a small ReservedSanitizedVars set (or
against the map of already-set env keys) and only apply the passthrough entry
when it is not in that set; make the same change for the other two identical
blocks that handle config.passthrough_env so user-provided entries cannot
override the hardened defaults.
In `@src/secrets/scrub.rs`:
- Around line 121-126: The current replacement logic can leak suffixes when a
secret is a prefix of another because replacements run in input order; in pub fn
new(secrets: Vec<(String, String)>) and the other replacement sites (around the
code using the secrets vector at the regions corresponding to lines 150-153 and
184-187) sort the secrets by descending value length before storing/iterating
them so longer secrets are replaced first; update SecretScrub::new (or the
constructor that receives secrets) to sort the incoming Vec<(String,String)> by
value.len() in descending order (or build an internal ordered list) and ensure
all places that iterate over the secrets use that sorted list to perform
replacements.
---
Nitpick comments:
In `@docs/content/docs/`(features)/workers.mdx:
- Around line 148-155: The statement that the agent's data directory is
"enforced at the kernel level" overstates guarantees; update the paragraph
containing "Worker shell and exec commands..." and the sentence "The agent's
data directory (databases, config files) is explicitly re-mounted read-only.
This is enforced at the kernel level -- no amount of command creativity can
bypass it." to add a qualification that enforcement depends on the sandbox
backend and kernel support (e.g., bubblewrap on Linux, sandbox-exec on macOS)
and may vary across platforms and configurations, e.g., replace the absolute
claim with language like "where supported by the sandbox backend and kernel" or
"depending on the sandbox backend and underlying OS/kernel guarantees."
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
docs/content/docs/(configuration)/config.mdxdocs/content/docs/(configuration)/permissions.mdxdocs/content/docs/(features)/tools.mdxdocs/content/docs/(features)/workers.mdxsrc/api/tools.rssrc/opencode/worker.rssrc/sandbox.rssrc/secrets/scrub.rssrc/tools/send_file.rssrc/tools/shell.rs
🚧 Files skipped from review as they are similar to previous changes (5)
- src/tools/shell.rs
- src/tools/send_file.rs
- src/api/tools.rs
- docs/content/docs/(features)/tools.mdx
- src/opencode/worker.rs
…ntainment visibility Replace the full-filesystem-read-only sandbox model with a minimal read-only allowlist (system runtime roots only). The agent data directory is now masked with an empty tmpfs (bubblewrap) or denied for both reads and writes (macOS) instead of being re-mounted read-only. HOME and TMPDIR are set deterministically to workspace and /tmp respectively. Worker and channel prompts now receive live sandbox state so the LLM knows exactly which paths are readable and writable.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/sandbox.rs (1)
420-423:⚠️ Potential issue | 🟠 MajorBlock reserved env keys in
passthrough_envto preserve sanitization guarantees.
passthrough_envis applied after hardened vars, so reserved keys (PATH,HOME,TMPDIR, and safe vars) can be overridden and weaken containment behavior.Suggested fix
const SAFE_ENV_VARS: &[&str] = &["USER", "LANG", "TERM"]; + +fn is_reserved_env_var(variable_name: &str) -> bool { + matches!(variable_name, "PATH" | "HOME" | "TMPDIR") + || SAFE_ENV_VARS.contains(&variable_name) +} @@ - for var_name in &config.passthrough_env { - if let Ok(value) = std::env::var(var_name) { - cmd.arg("--setenv").arg(var_name).arg(value); + for variable_name in &config.passthrough_env { + let variable_name = variable_name.as_str(); + if is_reserved_env_var(variable_name) { + tracing::debug!(%variable_name, "skipping reserved passthrough_env variable"); + continue; + } + if let Ok(value) = std::env::var(variable_name) { + cmd.arg("--setenv").arg(variable_name).arg(value); } } @@ - for var_name in &config.passthrough_env { - if let Ok(value) = std::env::var(var_name) { - cmd.env(var_name, value); + for variable_name in &config.passthrough_env { + let variable_name = variable_name.as_str(); + if is_reserved_env_var(variable_name) { + tracing::debug!(%variable_name, "skipping reserved passthrough_env variable"); + continue; + } + if let Ok(value) = std::env::var(variable_name) { + cmd.env(variable_name, value); } } @@ - for var_name in &config.passthrough_env { - if let Ok(value) = std::env::var(var_name) { - cmd.env(var_name, value); + for variable_name in &config.passthrough_env { + let variable_name = variable_name.as_str(); + if is_reserved_env_var(variable_name) { + tracing::debug!(%variable_name, "skipping reserved passthrough_env variable"); + continue; + } + if let Ok(value) = std::env::var(variable_name) { + cmd.env(variable_name, value); } }Also applies to: 465-468, 503-506
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/sandbox.rs` around lines 420 - 423, The loop that applies config.passthrough_env directly to the command (the code iterating over config.passthrough_env and calling cmd.arg("--setenv").arg(var_name).arg(value)) must skip reserved/sanitized environment keys so hardened vars like PATH, HOME, TMPDIR and other “safe” keys cannot be overridden; change the logic to filter out these reserved keys (e.g., maintain a const RESERVED_ENV = { "PATH","HOME","TMPDIR", ...safe_keys } and only call cmd.arg("--setenv") for var_name not in that set), and apply the same filtering in the other identical passthrough_env sites noted (the other loops at the indicated locations).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/content/docs/`(configuration)/sandbox.mdx:
- Line 8: Fix the minor typo in the sentence containing "OS-level containment
for builtin worker subprocesses (`shell` and `exec`)." — change "builtin" to the
hyphenated form "built-in" so the text reads "OS-level containment for built-in
worker subprocesses (`shell` and `exec`)."
- Around line 166-167: Replace the sentence "If a leak is detected, the process
is terminated immediately with an error. The leaked value is logged for
debugging but not returned to the LLM." with wording that explicitly forbids
logging raw secret values and describes safe handling instead (e.g., terminate
the process on leak, record only a redacted value or a non-reversible
fingerprint/hash and minimal contextual metadata for debugging, and never store
or display the raw leaked secret), ensuring the doc now states use of
redaction/fingerprinting rather than logging raw leaked values.
In `@src/prompts/engine.rs`:
- Around line 245-253: Several public prompt API signatures changed and callers
must be updated: for each call to render_worker_prompt add the four new
arguments (sandbox_enabled: bool, sandbox_containment_active: bool,
sandbox_read_allowlist: Vec<String>, sandbox_write_allowlist: Vec<String>) in
the same order as the signature; for each call to
render_channel_prompt_with_links add the new sandbox_enabled: bool parameter at
the correct position. Locate usages of render_worker_prompt and
render_channel_prompt_with_links in the codebase and tests, update their
call-sites to pass the appropriate sandbox flags and allowlist vectors (or empty
Vec::new() / false defaults where appropriate), and ensure the types match the
signature so the project compiles.
---
Duplicate comments:
In `@src/sandbox.rs`:
- Around line 420-423: The loop that applies config.passthrough_env directly to
the command (the code iterating over config.passthrough_env and calling
cmd.arg("--setenv").arg(var_name).arg(value)) must skip reserved/sanitized
environment keys so hardened vars like PATH, HOME, TMPDIR and other “safe” keys
cannot be overridden; change the logic to filter out these reserved keys (e.g.,
maintain a const RESERVED_ENV = { "PATH","HOME","TMPDIR", ...safe_keys } and
only call cmd.arg("--setenv") for var_name not in that set), and apply the same
filtering in the other identical passthrough_env sites noted (the other loops at
the indicated locations).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/content/docs/(configuration)/sandbox.mdxprompts/en/channel.md.j2prompts/en/worker.md.j2src/agent/cortex.rssrc/prompts/engine.rssrc/sandbox.rs
✅ Files skipped from review due to trivial changes (1)
- prompts/en/channel.md.j2
🚧 Files skipped from review as they are similar to previous changes (1)
- prompts/en/worker.md.j2
passthrough_env entries were applied after the hardened defaults (PATH, HOME, TMPDIR, SAFE_ENV_VARS), allowing user config to silently override them — weakening env sanitization and potentially dropping tools/bin from PATH precedence. Add is_reserved_env_var() guard to all three passthrough loops (bubblewrap, sandbox-exec, passthrough) so reserved variables are skipped with a debug log.
- tests/context_dump.rs: pass new sandbox args to render_worker_prompt (2 sites) and sandbox_enabled bool to render_channel_prompt so the test compiles again after the prompt API signature changes. - interface: rename 'Writable Paths' to 'Extra Allowed Paths' and update description since the sandbox now controls read access too, not just writes. - docs/sandbox.mdx: replace claim that leaked values are logged with language specifying only redacted fingerprints and metadata are recorded — raw secrets are never logged or returned.
Add sandbox_enabled to both render_channel_prompt_with_links calls and the four sandbox args (enabled, containment_active, read/write allowlists) to the render_worker_prompt call in spawn_worker_from_state. These were missed when the prompt API signatures were updated.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements all 5 phases from
docs/design-docs/sandbox-hardening.md. Fixes the hot-reload bug where toggling sandbox mode via the UI silently reverts, adds environment sanitization to prevent secret leakage through env vars, and extends leak detection to cover OpenCode workers.Changes
Phase 1 — Dynamic Sandbox Mode (Hot-Reload Fix)
RuntimeConfig.sandboxchanged fromArcSwap<SandboxConfig>toArc<ArcSwap<SandboxConfig>>so it can be shared with theSandboxstructreload_config()now stores sandbox config updates (removed the skip comment)Sandboxstruct reads mode dynamically fromArcSwapon everywrap()call — toggling via API takes effect immediatelySandbox::new()call sites (main.rs,api/agents.rs) pass the sharedArc<ArcSwap<SandboxConfig>>Phase 2 — Environment Sanitization
passthrough_env: Vec<String>toSandboxConfigfor self-hosted users who set env vars in Docker/systemd--clearenv(bubblewrap) orenv_clear()(sandbox-exec, passthrough)PATH(with tools/bin prepended), safe vars (HOME,USER,LANG,TERM,TMPDIR), andpassthrough_enventriesprintenv ANTHROPIC_API_KEYto extract system secretspassthrough_envPhase 3 — Durable Binary Instruction
tools/bindirectory location with explanation of ephemeral vs durable storageGET /agents/toolsendpoint lists installed binaries (name, size, modified timestamp) for dashboard observabilityPhase 4 — OpenCode Output Protection
src/secrets/scrub.rswithStreamScrubber(exact-match redaction with cross-chunk boundary handling) and sharedscan_for_leaks()/match_leak_patterns()functionsSpacebotHooknow delegates to shared functions — eliminated code duplicationPart::Textcontent andPart::Toolcompleted output for known secret patterns, terminating with error on detectionPhase 5 — send_file Hardening
send_file's workspace validation, matching the file tool's TOCTOU protection patternFiles Changed
src/sandbox.rsArc<ArcSwap<SandboxConfig>>, env sanitization in all wrapping modes,passthrough_envsupportsrc/config.rsRuntimeConfig.sandboxtype change,reload_config()stores sandbox,Arc::new()in constructorsrc/main.rsruntime_config.sandbox.clone()toSandbox::new()src/api/agents.rsSandbox::new()signature updatesrc/api/config.rspassthrough_envinSandboxSection,SandboxUpdate, and TOML serializersrc/api/server.rs/agents/toolsroutesrc/api/tools.rsGET /agents/toolsdirectory listing endpointsrc/api.rstoolsmodulesrc/secrets.rsscrubmodulesrc/secrets/scrub.rsStreamScrubber,scan_for_leaks(),match_leak_patterns(), 6 testssrc/hooks/spacebot.rssecrets::scrub::scan_for_leaks()src/opencode/worker.rssrc/tools/shell.rsenv_clear()for Windows pathsrc/tools/send_file.rsprompts/en/worker.md.j2Verification
cargo check— cleancargo clippy --lib— no warningscargo test --lib— 280 tests passcargo fmt --all -- --check— clean