feat(sandbox): tool sandbox#1105
Merged
Merged
Conversation
Contributor
PR Review SummarySize
Affected crates
Blast radius — BroadThis PR touches: source code,documentation,configuration / policy files Updated automatically on each push to this PR. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request implements Broker-Mediated Ephemeral Tool Execution (ETI) for nono, enabling fine-grained, isolated micro sandboxes for specific command invocations with brokered chaining, credential isolation via a token broker, and Landlock execute-only gates on Linux. Security reviews identified a critical fail-open vulnerability in pack verification when the trust bundle is missing, as well as several memory-safety issues in the token broker where sensitive credential data is cloned into un-zeroized heap memory instead of using Zeroizing wrappers.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
- Adopt the `if let ... && let ...` pattern for more concise conditional logic. - Introduce `ToolSandboxPrepare` to bundle arguments for `PreparedToolSandboxRuntime::prepare`, improving API readability. - Add `clippy::disallowed_methods` and `clippy::too_many_arguments` lint suppressions where specific design choices are intentional. - Refactor test setup for `trust_intercept` to use a common `test_tempdir` helper. - Remove the unused `tool_sandbox_invoke` benchmark. Signed-off-by: Luke Hinds <lukehinds@gmail.com>
Avoids unnecessary conversions from `Vec<u8>` to `OsString` and `PathBuf` when comparing paths in sandbox test assertions. The `grant.path` is already a byte vector, so direct byte slice comparison or creating an `OsStr` from bytes is more efficient. Signed-off-by: Luke Hinds <lukehinds@gmail.com>
feat(profiles): add granular policy and approval configuration Introduce powerful new configuration options to define granular policies for commands, credentials, resource limits, and approval workflows. This release significantly expands the profile schema to include: - **Invocation Policies**: Define rules based on command arguments (argv) and environment variables for commands invoked via a sandbox edge. - **Endpoint Policies**: Implement fine-grained HTTP method and path policies for proxy credentials, enabling detailed control over credential usage. - **Approval Backends**: Configure custom approval workflows with `terminal`, `webhook`, and `chain` types, including default settings and timeouts. - **Resource Limits**: Apply memory, CPU, wall time, process, and file size limits to sandboxed commands. - **Stdio Limits**: Control stdout/stderr output size with truncation or termination policies. - **Mutual TLS**: Configure TLS CA bundles, client certificates, and keys for proxy credentials. - **Network Controls**: New `allow_all` option for unrestricted network access. Alongside these new features, comprehensive validation is added for all new timeout fields, ensuring positive values. Extensive schema tests verify that the JSON schema accurately reflects the Rust type definitions. Signed-off-by: Luke Hinds <lukehinds@gmail.com>
- Introduce `credential_capture` profile entry to define supervisor-side commands for producing credentials. - Enable `cmd://<name>` URIs in `custom_credentials` to reference these commands. - The proxy executes capture commands lazily on first access: - Captures stdout for credential value. - Caches value for a configurable TTL. - Injects value into upstream request. - Captured credentials are never exposed to the sandboxed child. - Audit records include command path, redacted argv, exit status, duration, stdout byte count, and redacted stderr for failures; the credential value is never logged. - Proxy-related environment variables are removed from capture command environments to prevent recursive proxying. Signed-off-by: Luke Hinds <lukehinds@gmail.com>
The "Ephemeral Tool Invocation" (ETI) feature has been renamed to "Tool Sandbox" across documentation, internal code comments, error messages, and associated tests. This change provides clearer, more descriptive terminology for the core command sandboxing functionality. Additionally, this commit includes: - Enhancements to network audit records by adding new credential capture fields for cache scope, output format, header names, stdin mode, and interactivity. - Improvements to integration tests for audit session lookup, profile output verification, network test robustness, and silent output verbosity. - A stricter default sandbox policy on Linux, now explicitly blocking bare `/sys` directory access. - Introduction of a new integration test suite focused on child tool boundaries. Signed-off-by: Luke Hinds <lukehinds@gmail.com>
Add per-command `open_urls` policy so a brokered tool-sandbox child (e.g. an OAuth2 login flow) can open a browser via the unsandboxed runtime, gated by an origin allow-list. Also fixes keychain access and debug-log propagation for the brokered-child launcher. open_urls feature: - Add `open_urls` (OpenUrlConfig) and macOS-only `allow_launch_services` to CommandSandboxConfig. `open_urls` is replace-not-merge across `from` edges; `allow_launch_services` is OR-merged. Validation rejects malformed origins, warns when both flags are set, and flags allow_launch_services as macOS-only. - Bind a dedicated `url.sock` listener only when a command declares open_urls (zero attack surface otherwise). The brokered child cannot exec /usr/bin/open, so the runtime materializes a copy of the nono binary named `open` in the sealed shim dir; the child execs it (also wired as $BROWSER) and it forwards the URL over the socket. macOS uses a dedicated accept thread; Linux drains it on the supervisor poll loop. - Resolve the requesting command from the connecting PID (ancestry walk); the `command` field in the request is advisory/audit-only and never trusted for the allow-list decision. Outcomes log to the unsandboxed runtime stderr and are recorded via record_open_url. - Extract URL validation + browser launch into url_open.rs as the single source of truth shared by the supervisor and tool-sandbox paths so they cannot drift. - Thread the launch Caller through launch_child*/track_child and store it on ActiveChild so the URL-open path selects the command's own running policy rather than a nonexistent <cmd>.can_use[<cmd>] self-edge. fix(keychain): forward HOME to the brokered-child launcher process so the library's has_explicit_keychain_db_access recognizes user keychain DBs and drops the securityd mach denies. The child is still execve'd with filtered env, so HOME does not leak to it. Also add the keychain DB WAL/SHM sibling-file grants. fix(logging): forward RUST_LOG to the launcher re-exec and init a stderr subscriber in run_child_launcher so the library's generated-profile debug log surfaces under -vv for the brokered child. Verified on macOS: fmt, build, nono-cli clippy, and nono-cli tests pass (pre-existing failure test_load_nonexistent_profile excluded; unrelated). Linux open_urls paths are not compile-checked on this Darwin host. Signed-off-by: Leonardo Zanivan <leonardo.zanivan@gmail.com>
Signed-off-by: Luke Hinds <lukehinds@gmail.com>
This profile and its associated test were removed from the built-in profiles. Tool sandbox example profiles are intended to be distributed via registry packs rather than being embedded directly within the CLI. This change aligns with the strategy to decentralize example and specialized profiles. This ensures the `nono-cli` distribution is leaner and encourages discovery and maintenance of such profiles through external registries. Signed-off-by: Luke Hinds <lukehinds@gmail.com>
…apabilities The previous checks for executables and their parent directories being writable by the supervisor user, group, or world have been removed. Writable executable trust downgrades now explicitly refer to targets writable through the outer sandbox capability set. - This aligns the trust model with the sandbox's own capability to modify the binary or its parent, rather than relying on general host filesystem permissions. - Documentation, JSON schema, and tests have been updated to reflect this change in definition and behavior. - The `reject_user_writable_path` helper was removed and `outer_caps_grant_file_write` was introduced for more precise file write capability checks. Signed-off-by: Luke Hinds <lukehinds@gmail.com>
Signed-off-by: Luke Hinds <lukehinds@gmail.com>
Allow proxy configuration environment variables to pass into the tool sandbox. This change includes `HTTPS_PROXY`, `HTTP_PROXY`, `NO_PROXY`, and their lowercase versions. It ensures that tools executed within the sandbox can respect system-wide or user-defined proxy settings for network requests. Signed-off-by: Luke Hinds <lukehinds@gmail.com>
Some Python interpreters on macOS, particularly those installed via Homebrew or similar package managers, are part of a `Python.framework` structure. These interpreters might implicitly or explicitly rely on a sibling `Python.app` bundle executable for certain operations. This change adds logic to identify the associated `Python.app` bundle path when a `Python.framework` interpreter is encountered. It then ensures that the sandbox grants the necessary capabilities: - Read access to the bundle executable when establishing the executable shape baseline. - Execution permission for the bundle executable when allowing child processes. This prevents sandbox violations and ensures correct operation of Python environments that utilize this common macOS framework structure. Signed-off-by: Luke Hinds <lukehinds@gmail.com>
Rebasing the tool-sandbox commits onto main combined both branches' additions to several shared structs, which left test fixtures (and one production CustomCredentialDef construction) missing the now-required fields: - RouteConfig / CustomCredentialDef: gained endpoint_policy (tool-sandbox) and aws_auth (main) - NetworkAuditEvent: gained endpoint-policy / approval / credential-capture fields and `upstream` - ProxyHandle: gained `diagnostics` - AuditEntry.request: CapabilityRequest -> ApprovalRequest::Capability enum - CredentialStore::load deprecated in favour of load_with_diagnostics - PreparedSandbox: gained profile_display_name / command_policies / credential_capture / tls_intercept Also drops a pre-existing unused import (TOOL_SANDBOX_URL_SOCKET_ENV) in the Linux tool-sandbox platform module that only surfaces when building on Linux. Completes those initializers so the workspace compiles and the full unit / integration-unit test suite passes. Behaviour unchanged. Signed-off-by: Luke Hinds <luke@alwaysfurther.ai> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Linux tool-sandbox computes an ELF dependency closure for every command binary, interpreter, and shim (build_baseline_cache / build_outer_exec_files), each with a fresh per-closure dedup set. Those closures overlap heavily — libc, ld-linux, glib, and the NSS/glib/samba graph that name/DNS lookups pull in — so the same shared libraries were re-searched and re-canonicalized once per referencing edge, across every closure. For `gh auth token` under a tool-sandbox profile this produced ~955k readlink + ~451k statx syscalls (e.g. libc.so.6 canonicalized 69,343 times) — ~4s of kernel time on Linux, vs ~0 on macOS (Seatbelt builds no ELF closure). Memoize resolution per prep pass: - cached_canonicalize: canonicalize each path once (canonicalization is a pure function of the read-only-during-prep filesystem). - resolve_shared_library: cache by (soname, search_dirs) — the only inputs that determine the result — so a library referenced by many objects is searched once, not once per edge. Caches are thread-local and reset at the start of each closure-building batch, so the resolved closure is identical; only redundant syscalls are removed. Pre-existing issue, independent of the main rebase. Signed-off-by: Luke Hinds <luke@alwaysfurther.ai> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Custom credential definitions (`custom_credentials`) now require explicit activation by being listed in the `credentials` section of a policy. Previously, merely defining `custom_credentials` implicitly activated them and caused the proxy to launch, even if not explicitly requested. This change aligns with the principle that credential definitions are templates, and activation requires a deliberate choice. - `resolve_credentials` no longer implicitly includes all `custom_credentials` for resolution; only those specified in `service_names` are processed. - `prepare_proxy_launch_options` no longer considers the mere presence of `custom_credentials` as a trigger for proxy activation. Additionally, shared library dependency resolution on Linux has been optimized: `parse_elf` results are now memoized using a new `ELF_PARSE_CACHE`. This ensures each ELF file is read and parsed only once per pass, reducing redundant I/O and CPU usage during sandbox setup. Signed-off-by: Luke Hinds <lukehinds@gmail.com>
Previously, ELF resolution memo caches were reset multiple times during sandbox setup within the `build_outer_exec_files` and `build_baseline_cache` functions. This caused redundant re-evaluation of shared object dependencies across different phases of a single sandbox launch. This commit centralizes the cache reset to occur only once at the beginning of `PreparedToolSandboxRuntime::start`. This ensures that a single set of memoized ELF resolutions (canonical paths, parsed ELFs) spans both the outer-exec gate and baseline cache building phases, reducing overall work. Additionally, a new `ELF_FILEID_CACHE` is introduced to memoize `dev_t` and `ino_t` (file identity) for canonical paths. This prevents redundant `statx` syscalls when deduplicating files during dependency closure resolution. Signed-off-by: Luke Hinds <lukehinds@gmail.com>
- Introduce dynamic path expansion in sandbox policies, enabling profile authors to include paths like `@git:config-files` and `@git:hooks-path`. These tokens are replaced with concrete filesystem paths derived from the local environment at runtime, such as git configuration files or global/system hook directories. - Enhance `capture_credential` intercept rules with a `grant_to` field. This allows profiles to specify a list of consumer IDs (e.g., `cmd.<name>` for other commands, `proxy.<route_id>` for proxy routes) that are authorized to redeem the issued nonce. An empty list implies `GrantSet::All`. - Enable the proxy runtime to resolve tool-sandbox broker nonces found in L7 request headers (e.g., `Authorization`). For admitted consumers, `nono_<hex>` tokens are replaced with their real credential values before forwarding upstream. This facilitates the secure flow of credentials captured by commands to services proxied by Nono. - Refine command binary resolution: if an `executable` is specified in a command policy but not found/executable, or if a command name cannot be resolved on PATH, a warning is emitted and the command is skipped rather than causing the entire command resolution to fail. This improves robustness. Signed-off-by: Luke Hinds <lukehinds@gmail.com>
Document the new `@git:config-files` and `@git:hooks-path` dynamic provider tokens for tool-sandbox policies. These tokens allow `fs_read`, `fs_write`, `fs_read_file`, and `fs_write_file` paths to expand to trusted global/system Git configuration and hook paths. They are opt-in and ignore repo-local/worktree Git config to prevent a repository from self-granting additional filesystem access. Signed-off-by: Luke Hinds <lukehinds@gmail.com>
- Introduce `CaptureErrorDetails` struct to consolidate error data, making `capture_error` calls cleaner and more explicit. - Box `CredentialCaptureMetadata` and `NetworkAuditEvent` in their respective enum variants to reduce stack size and improve memory locality for potentially large structs. - Simplify `capture_cache_scope` logic in `proxy_runtime.rs`. - Enhance test clarity in `token_broker.rs` and `test_pack_resolution.sh` with explicit messages and direct profile paths for pack enforcement. Signed-off-by: Luke Hinds <lukehinds@gmail.com>
SequeI
approved these changes
Jun 22, 2026
oscarmackjr-twg
added a commit
to OscarMackJr/nono
that referenced
this pull request
Jun 27, 2026
…1fd10e Extracts additive shared-surface hunks from upstream 11fd10e feat(sandbox): tool sandbox (nolabs-ai#1105). Skips the tool-sandbox subsystem directory (absent in fork), tls_intercept/ hunks (Cluster F carve-out), exec_strategy.rs timeout refactor (entangled with tool-sandbox), and ApprovalRequest type plumbing. Applied: - crates/nono/src/audit.rs: SandboxRuntimeAuditEvent + CommandPolicyAuditEvent structs; record_sandbox_runtime_event() + record_command_policy_event() methods; record_network_event() signature: Box<NetworkAuditEvent> (updated all call sites) - crates/nono/src/sandbox/linux.rs: has_execute() + restrict_execute() pub fns - crates/nono/src/sandbox/mod.rs: pub use linux::restrict_execute re-export - crates/nono/src/keystore.rs: CMD_URI_PREFIX const + cmd:// early-return guard - crates/nono/src/scrub.rs: SENSITIVE_ENV_VARS expansion - crates/nono-proxy/src/config.rs: CompiledEndpointPolicy + EndpointPolicyConfig types - crates/nono-proxy/src/route.rs: endpoint_policy field (no tls_intercept refs) - crates/nono-proxy/src/server.rs: shutdown_tx.send(true) call - crates/nono-proxy/src/credential.rs: endpoint_policy: None in test struct literals - crates/nono-proxy/src/reverse.rs: endpoint_policy: None in test struct literal - crates/nono-cli/src/network_policy.rs: endpoint_policy: None in RouteConfig construction Skipped: tool-sandbox/ dir, tls_intercept/, ApprovalRequest enum, exec_strategy.rs timeout refactor. CR-02 records_verified: event_count > 0 preserved intact. Rule 3 (blocking fix): endpoint_policy field addition to RouteConfig required updating all struct literals in credential.rs, reverse.rs, and network_policy.rs to compile. All changes are endpoint_policy: None — no behavioral change. Cherry-picked from upstream nolabs-ai/nono 11fd10e Signed-off-by: oscarmackjr-twg <oscar.mack.jr@gmail.com>
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
3 participants
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.
Description coming
Closes #887 #886
Summary