Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d9f2f68f7
ℹ️ 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".
| if [[ "${{ matrix.target }}" == *linux* && "${{ matrix.bundle }}" == "primary" ]]; then | ||
| cp "target/${{ matrix.target }}/release/bwrap" "$dest/bwrap-${{ matrix.target }}" | ||
| cp "target/${{ matrix.target }}/release/bwrap.sigstore" \ | ||
| "$dest/bwrap-${{ matrix.target }}.sigstore" |
There was a problem hiding this comment.
For Linux releases installed through the documented standalone/npm path, staging bwrap only as a top-level release asset leaves it unused: scripts/install/install.sh installs from the platform npm tarball and only copies codex plus codex-resources/rg, while the launcher searches for codex-resources/bwrap next to the executable. In environments without a system bwrap, those installs will still hit the “no bundled codex-resources/bwrap” path despite this workflow publishing a separate asset; the platform package/installer needs to include or fetch this binary into codex-resources/bwrap.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good callout. In this stack, #21256 is intentionally just the producer step: build/sign/stage the Linux bwrap release artifact and burn its digest into the Codex binary. The current consumer/package layout fix is in the next PR, #21257, which places Linux bwrap at vendor/<target>/codex-resources/bwrap for npm and has the current standalone installer copy it to codex-resources/bwrap.
One related wrinkle: #18910/#18901 are introducing a newer standalone archive path that bypasses npm internals. If that stack lands, it should pick up the same layout rule by adding bwrap to the Linux standalone archive staging path and copying it into archive codex-resources/bwrap. So I do not think this should move into #21256, but #18910 should rebase on this stack or take the equivalent archive-staging update.
bolinfest
force-pushed
the
pr21255
branch
2 times, most recently
from
3477832 to
6e7259e
Compare
bolinfest
force-pushed
the
pr21256
branch
2 times, most recently
from
3ef40b7 to
e8ae081
Compare
bolinfest
force-pushed
the
pr21255
branch
2 times, most recently
from
1a8cae2 to
74039c1
Compare
**Summary** - Add `codex-bwrap`, a standalone `bwrap` binary built from the existing vendored bubblewrap sources. - Remove the linked vendored bwrap path from `codex-linux-sandbox`; runtime now prefers system `bwrap` and falls back to bundled `codex-resources/bwrap`. - Add bundled SHA-256 verification with missing/all-zero digest as the dev-mode skip value, then exec the verified file through `/proc/self/fd`. - Keep `launcher.rs` focused on choosing and dispatching the preferred launcher. Bundled lookup, digest verification, and bundled exec now live in `linux-sandbox/src/bundled_bwrap.rs`; Bazel runfiles lookup lives in `linux-sandbox/src/bazel_bwrap.rs`; shared argv/fd exec helpers live in `linux-sandbox/src/exec_util.rs`. - Teach Bazel tests to surface the Bazel-built `//codex-rs/bwrap:bwrap` through `CARGO_BIN_EXE_bwrap`; `codex-linux-sandbox` only honors that fallback in debug Bazel runfiles environments so release/user runtime lookup stays tied to `codex-resources/bwrap`. - Allow `codex-exec-server` filesystem helpers to preserve just the Bazel bwrap/runfiles variables they need in debug Bazel builds, since those helpers intentionally rebuild a small environment before spawning `codex-linux-sandbox`. - Verify the Bazel bwrap target in Linux release CI with a build-only check. Running `bwrap --version` is too strong for GitHub runners because bubblewrap still attempts namespace setup there. **Verification** - Latest update: `cargo test -p codex-linux-sandbox` - Latest update: `just fix -p codex-linux-sandbox` - `cargo check --target x86_64-unknown-linux-gnu -p codex-linux-sandbox` could not run locally because this macOS machine does not have `x86_64-linux-gnu-gcc`; GitHub Linux Bazel CI is expected to cover the Linux-only modules. - Earlier in this PR: `cargo test -p codex-bwrap` - Earlier in this PR: `cargo test -p codex-exec-server` - Earlier in this PR: `cargo check --release -p codex-exec-server` - Earlier in this PR: `just fix -p codex-linux-sandbox -p codex-exec-server` - Earlier in this PR: `bazel test --nobuild //codex-rs/linux-sandbox:linux-sandbox-all-test //codex-rs/core:core-all-test //codex-rs/exec-server:exec-server-file_system-test //codex-rs/app-server:app-server-all-test` (analysis completed; Bazel then refuses to run tests under `--nobuild`) - Earlier in this PR: `bazel build --nobuild //codex-rs/bwrap:bwrap` - Prior to this update: `just bazel-lock-update`, `just bazel-lock-check`, and YAML parse check for `.github/workflows/bazel.yml` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/21255). * #21257 * #21256 * __->__ #21255
2 participants
Summary
bwrapbefore the main release binaries.bwrapSHA-256 asCODEX_BWRAP_SHA256so the Codex binary can verify the bundled fallback.bwrapalongside the primary Linux release artifacts.Verification
.github/workflows/rust-release.ymlStack created with Sapling. Best reviewed with ReviewStack.