feat(sandbox): implement macOS SandboxExecutor using Seatbelt/SBPL#67
Merged
feat(sandbox): implement macOS SandboxExecutor using Seatbelt/SBPL#67
SandboxExecutor using Seatbelt/SBPL#67Conversation
The sandbox execution layer had only a stub implementation that returned "not yet implemented" errors. This prevented sandbox-enforced command execution on macOS. Add `MacOsSandboxExecutor` that generates SBPL profiles dynamically from `SandboxPolicy` and executes commands under `/usr/bin/sandbox-exec`. - SBPL generation: deny all file-write by default, selectively allow writable_roots, deny read_only_subpaths (last-match-wins ordering), control network access, always allow /dev/null writes - `is_supported()` method added to `SandboxExecutor` trait to check for `/usr/bin/sandbox-exec` availability at runtime - `ExecAdapter` now resolves sandbox preset names to `SandboxPolicy` and passes them to the executor instead of ignoring them - `main.rs` uses `MacOsSandboxExecutor` when available, falls back to `StubSandboxExecutor` on other platforms Co-Authored-By: Claude Opus 4.6 <[email protected]>
Summary of ChangesHello @fohte, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces robust OS-level sandboxing for macOS by integrating with the native Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Replace unreachable `None` branch in `resolve_sandbox_policy` with `let-else` + `unreachable!` since `merge_strictest` with a non-empty slice always returns `Some` - Deduplicate `exit_code_from_status` by making the version in `command_executor.rs` `pub(crate)` and importing it in `macos_sandbox.rs` - Consolidate five separate SBPL generation tests into a single parameterized `#[rstest]` test with named `#[case]` variants - Extract `macos_executor` and `default_policy` fixtures for macOS integration tests to reduce setup duplication
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #67 +/- ##
==========================================
- Coverage 87.59% 87.39% -0.20%
==========================================
Files 27 29 +2
Lines 5344 5571 +227
==========================================
+ Hits 4681 4869 +188
- Misses 663 702 +39 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
SBPL's `(subpath ...)` filter requires absolute paths and does not support glob patterns. When `read_only_subpaths` contained relative paths (e.g., `.git`) or globs (e.g., `.env*`, `/etc/**`), the deny rules were silently ineffective. Classify each deny path and use the appropriate SBPL filter: - Absolute literal paths use `(subpath ...)` as before - Relative literal paths are resolved against each `writable_root` - Glob patterns are converted to `(regex ...)` filters via `glob_to_sbpl_regex`
Hand-written glob-to-regex conversion is fragile and prone to edge-case bugs. The globset crate (by BurntSushi, 139M+ downloads) provides battle-tested glob-to-regex conversion with correct handling of `*`, `**`, and metacharacter escaping. Use `GlobBuilder::literal_separator(true)` so `*` does not match `/`, matching the expected filesystem glob semantics. Remove the custom `glob_to_sbpl_regex` and `is_regex_metachar` functions entirely.
On Linux CI, the `fixture` import was unused because all fixtures are gated behind `#[cfg(target_os = "macos")]`. This caused a build failure due to `-D unused-imports`.
When the CLI `--sandbox` argument specified a preset name not found in `definitions.sandbox`, `resolve_sandbox_policy` returned `Ok(None)`, causing the command to execute without any sandbox silently. This is a security issue since users expect sandboxing to be applied. Return an error instead so the user gets clear feedback that the preset name is invalid.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…[ globs globset emits Rust-specific (?-u) (disable Unicode) flag prefix in its regex output, but SBPL's regex engine uses POSIX ERE and rejects this syntax with "unexpected ^ operator" (exit code 65), preventing the sandboxed process from starting at all. Also extend classify_deny_path to detect ? and [ as glob metacharacters, not just *, so patterns like file?.txt and log[0-9].txt are correctly routed to regex-based SBPL filters.
…t ? and [ globs" This reverts commit 51d66fc.
…nly tests" This reverts commit 94e33a2.
…obset crate" This reverts commit 0bb40d3.
…in regex conversion `classify_deny_path` only checked for `*`, causing `?` and `[...]` patterns to be misclassified as literal paths. `glob_to_sbpl_regex` also lacked support for these glob characters. Add `?` → `[^/]` and `[...]` passthrough to the POSIX ERE conversion, and update the classifier to detect all three glob metacharacters.
…-macos-sandbox # Conflicts: # src/exec/command_executor.rs # src/main.rs
…r srt
The glob-to-regex converter did not handle `{a,b}` brace expansion,
causing patterns like `*.{js,ts}` to be treated as literal text.
Also, `classify_deny_path` did not recognize `{` as a glob character.
The macOS integration tests call `sandbox-exec`, which cannot nest
inside another sandbox (srt sets `SANDBOX_RUNTIME=1`). Skip these
tests when running under srt to avoid spurious failures.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
…e import The merge introduced the `is_supported` method on `SandboxExecutor` trait but the `LinuxSandboxExecutor` impl was missing it, causing compilation failure on Linux CI. Also conditionally import `fixture` for macOS-only tests to avoid unused import warning on Linux.
`SandboxExecutor` and `MacOsSandboxExecutor` are only used in the `#[cfg(target_os = "macos")]` block of `create_executor`, so they must be conditionally imported to avoid unused import errors on Linux.
`macos_sandbox.rs` was growing large with mixed concerns: SBPL generation, sandbox execution, and glob-to-regex conversion. Move `DenyPathKind`, `classify_deny_path`, `glob_to_sbpl_regex`, and `is_regex_metachar` along with their tests into `glob_pattern.rs` to keep each module focused on a single responsibility.
`glob_pattern` is specific to macOS SBPL regex generation, not a general-purpose utility. Move it from `exec/glob_pattern.rs` to `exec/macos_sandbox/glob_pattern.rs` by converting `macos_sandbox` from a single file to a directory module. Narrow visibility from `pub(crate)` to `pub(super)`.
The existing tests only covered basic patterns. Add cases for:
- brace expansion edge cases: empty alternative (`ts{x,}`), nested
braces (`{a,{b,c}}`), dots inside alternatives (`*.{tar.gz,zip}`),
globs mixed with braces (`{src,lib}/**/*.rs`)
- double star edge cases: bare `**`, prefix `**/*.log`, trailing
`a/**`, multiple `a/**/b/**/c`
- character class: negation `[!a-z]`, consecutive `[abc][0-9]`
- metacharacter escaping: `.`, `+`, `()`, `|`
- misc: empty pattern, multiple `?`, combined `*` and `?`
String comparison alone cannot catch semantic bugs like `*` incorrectly matching across directory separators. Add `should_match` / `should_not_match` path lists to each existing test case so the generated regex is validated against actual paths using the `regex` crate. Add `regex` as a dev-dependency for match verification. Co-Authored-By: Claude Opus 4.6 <[email protected]>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Glob syntax uses `!` for character class negation (e.g., `[!a-z]`) while POSIX ERE uses `^` (e.g., `[^a-z]`). Without this conversion, a deny pattern like `[!a-z]file` would generate `[!a-z]file` which matches `!` or `a-z` — the opposite of the intended semantics. Co-Authored-By: Claude Opus 4.6 <[email protected]>
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.
Why
sandbox-exec)What
SandboxPolicyand execute commands undersandbox-execwith policy-based sandboxingis_supported()method to theSandboxExecutortrait for runtime platform detectionExecAdapterand pass them to the executor