-
Notifications
You must be signed in to change notification settings - Fork 7.9k
feat(linux-sandbox): add bwrap support #9938
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
3903ff5 to
5aa61a1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5aa61a15ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
5aa61a1 to
2e968d3
Compare
4d27a26 to
0198203
Compare
3a4a6ed to
077419b
Compare
a7b4ed5 to
669ccb9
Compare
## Summary Vendor Bubblewrap into the repo and add minimal build plumbing in `codex-linux-sandbox` to compile/link it. ## Why We want to move Linux sandboxing toward Bubblewrap, but in a safe two-step rollout: 1) vendoring/build setup (this PR), 2) runtime integration (follow-up PR). ## Included - Add `codex-rs/vendor/bubblewrap` sources. - Add build-time FFI path in `codex-rs/linux-sandbox`. - Update `build.rs` rerun tracking for vendored files. - Small vendored compile warning fix (`sockaddr_nl` full init). follow up in #9938
669ccb9 to
1ae8d7f
Compare
|
|
||
| /// Execute the build-time bubblewrap `main` function with the given argv. | ||
| pub(crate) fn exec_vendored_bwrap(argv: Vec<String>) -> ! { | ||
| let exit_code = run_vendored_bwrap_main(&argv); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, so this appears to be the end of main():
where:
if (execvp (exec_path, argv) == -1)
leads to:
die_with_error ("execvp %s", exec_path);
but if execvp() does not return -1, then bwrap's main returns 0, even though that suggests something has gone horribly wrong?
bolinfest
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, a handful of questions/clarifications!
Summary
This PR introduces a gated Bubblewrap (bwrap) Linux sandbox path. The curent Linux sandbox path relies on in-process restrictions (including Landlock). Bubblewrap gives us a more uniform filesystem isolation model, especially explicit writable roots with the option to make some directories read-only and granular network controls.
This is behind a feature flag so we can validate behavior safely before making it the default.
features.use_linux_sandbox_bwrap