fix: fix old system bubblewrap compatibility without falling back to vendored bwrap#15693
Merged
viyatb-oai merged 25 commits intomainfrom Mar 26, 2026
Merged
Conversation
viyatb-oai
changed the title
viyatb-oai
force-pushed
the
codex/viyatb/fix-argv0-bubblewrap-fallback
branch
from
902dee3 to
3855146
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3855146038
ℹ️ 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".
bolinfest
reviewed
Mar 24, 2026
bolinfest
reviewed
Mar 25, 2026
bolinfest
approved these changes
Mar 25, 2026
Collaborator
bolinfest
left a comment
bolinfest
left a comment
There was a problem hiding this comment.
If you did get an Ubuntu 20 to test on, can you please update the PR body to explain what you tested?
| }; | ||
| spawn_child_async(SpawnChildRequest { | ||
| program: codex_linux_sandbox_exe.as_ref().to_path_buf(), | ||
| program: codex_linux_sandbox_exe.to_path_buf(), |
Collaborator
There was a problem hiding this comment.
Outside the scope of this PR, but I would like to change program to be AsRef<OsStr> instead of String so we don't have to use to_string_lossy() quite so much.
bolinfest
reviewed
Mar 25, 2026
bolinfest
reviewed
Mar 25, 2026
bolinfest
reviewed
Mar 25, 2026
bolinfest
reviewed
Mar 26, 2026
bolinfest
reviewed
Mar 26, 2026
bolinfest
reviewed
Mar 26, 2026
bolinfest
reviewed
Mar 26, 2026
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Fixes #15283.
Summary
Older system bubblewrap builds reject
--argv0, which makes our Linux sandbox fail before the helper can re-exec. This PR keeps using system/usr/bin/bwrapwhenever it exists and only falls back to vendored bwrap when the system binary is missing. That matters on stricter AppArmor hosts, where the distro bwrap package also provides the policy setup needed for user namespaces.For old system bwrap, we avoid
--argv0instead of switching binaries:argv0,current_exe() + --argv0path when the selected launcher supports it,--argv0and re-exec through the helper's ownargv[0]path, whose basename still dispatches ascodex-linux-sandbox.Also updates the launcher/warning tests and docs so they match the new behavior: present-but-old system bwrap uses the compatibility path, and only absent system bwrap falls back to vendored.
Validation
argv0support