Skip to content

fix: resolve bwrap from trusted PATH entry#15791

Merged
viyatb-oai merged 2 commits intomainfrom
codex/viyatb/fix-bwrap-system-path
Mar 26, 2026
Merged

fix: resolve bwrap from trusted PATH entry#15791
viyatb-oai merged 2 commits intomainfrom
codex/viyatb/fix-bwrap-system-path

Conversation

@viyatb-oai
Copy link
Copy Markdown
Collaborator

@viyatb-oai viyatb-oai commented Mar 25, 2026

Summary

  • resolve system bwrap from PATH instead of hardcoding /usr/bin/bwrap
  • skip PATH entries that resolve inside the current workspace before launching the sandbox helper
  • keep the vendored bubblewrap fallback when no trusted system bwrap is found

Validation

  • cargo test -p codex-core bwrap --lib
  • cargo test -p codex-linux-sandbox
  • just fix -p codex-core
  • just fix -p codex-linux-sandbox
  • just fmt
  • just argument-comment-lint
  • cargo clean

@viyatb-oai viyatb-oai requested a review from bolinfest March 26, 2026 16:20
@viyatb-oai viyatb-oai marked this pull request as ready for review March 26, 2026 16:20
Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a 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: 7e0832580e

ℹ️ 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".

@viyatb-oai viyatb-oai force-pushed the codex/viyatb/fix-bwrap-system-path branch from 8f9bf03 to 59aa4bf Compare March 26, 2026 17:59
@viyatb-oai viyatb-oai changed the title Fix bwrap PATH lookup and suppress bypass-mode warnings Resolve bwrap from trusted PATH entry Mar 26, 2026
const OPENAI_BASE_URL_ENV_VAR: &str = "OPENAI_BASE_URL";
#[cfg(target_os = "linux")]
const SYSTEM_BWRAP_PATH: &str = "/usr/bin/bwrap";
const SYSTEM_BWRAP_PROGRAM: &str = "bwrap";
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a follow-up, we should move all the bwrap stuff out of this file (which is already quite large) so we end up with less #[cfg(target_os = "linux")] in here.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved this cleanup to follow-up PR #15898 to keep this PR focused on the behavior change.


#[cfg(target_os = "linux")]
#[test]
fn skips_workspace_local_bwrap_in_search_paths() {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All these tests should go in their own file, as well.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved this cleanup to follow-up PR #15898 to keep this PR focused on the behavior change.

@viyatb-oai viyatb-oai changed the title Resolve bwrap from trusted PATH entry fix: resolve bwrap from trusted PATH entry Mar 26, 2026
@viyatb-oai viyatb-oai merged commit b6050b4 into main Mar 26, 2026
36 checks passed
@viyatb-oai viyatb-oai deleted the codex/viyatb/fix-bwrap-system-path branch March 26, 2026 19:13
@github-actions github-actions bot locked and limited conversation to collaborators Mar 26, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants