Commit Graph

7 Commits

  • Fix rust-ci-full failures due to missing bwrap (#21604)
    Since https://github.com/openai/codex/pull/21255, `rust-ci-full` has
    been failing due to a missing `bwrap`.
    
    ```
    thread 'main' panicked at linux-sandbox/src/launcher.rs:43:13:
    bubblewrap is unavailable: no system bwrap was found on PATH and no bundled codex-resources/bwrap binary was found next to the Codex executable
    ```
    
    Since the happy path is now to use the system binary, let's ensure
    that's installed.
    
    
    https://github.com/openai/codex/pull/21604/commits/8d5182663158ee2d15965f39eed26ffa339ecb7d
    was necessary for the `bwrap` executable to be discoverable when the
    working directory is `/`.
    
    I ran `rust-ci-full` at
    https://github.com/openai/codex/actions/runs/25528074506
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • fix(sandboxing): Bound advisory system bwrap startup probe (#20111)
    ## Why
    
    Linux startup runs an advisory system `bwrap` warning probe on each
    launch. On hosts with NFS or autofs mounts, its `--ro-bind / /` probe
    can take tens of seconds before Codex prints anything, matching #19828.
    Because this probe only decides whether to surface a warning, it should
    not be allowed to stall startup.
    
    Relevant pre-change path:
    [`codex-rs/sandboxing/src/bwrap.rs`](https://github.com/openai/codex/blob/de2ccf94735a3d8a2a7077e6a5292026413867cf/codex-rs/sandboxing/src/bwrap.rs#L64-L80)
    
    ## What changed
    
    - Bound the advisory system `bwrap` probe to 500 ms.
    - Preserve the existing warning behavior when `bwrap` promptly reports a
    known user-namespace failure.
    - Kill and reap the probe child on timeout, then suppress the advisory
    warning instead of blocking startup.
    - Read probe stderr with a bounded nonblocking drain so descendants that
    inherit the pipe cannot extend startup after the probe child exits.
    - Add regression coverage for both a deliberately slow fake `bwrap`
    process and a fake probe whose descendant keeps stderr open.
    
    ## Security
    
    This only bounds the advisory startup probe. It does not change the
    command execution path or add a fail-open sandbox fallback. The related
    command-side hang in #20017 remains separate from this PR.
    
    ## Verification
    
    - Added `system_bwrap_probe_times_out_without_reporting_a_warning`.
    - Added
    `system_bwrap_probe_does_not_wait_for_descendants_holding_stderr_open`.
    - `cargo test -p codex-sandboxing`
    - `cargo clippy -p codex-sandboxing --all-targets -- -D warnings`
    
    Fixes #19828
    Related: #20017
  • fix(sandboxing): reject WSL1 bubblewrap sandboxing (#17559)
    ## Summary
    
    - detect WSL1 before Codex probes or invokes the Linux bubblewrap
    sandbox
    - fail early with a clear unsupported-operation message when a command
    would require bubblewrap on WSL1
    - document that WSL2 follows the normal Linux bubblewrap path while WSL1
    is unsupported
    
    ## Why
    
    Codex 0.115.0 made bubblewrap the default Linux sandbox. WSL1 cannot
    create the user namespaces that bubblewrap needs, so shell commands
    currently fail later with a raw bwrap namespace error. This makes the
    unsupported environment explicit and keeps non-bubblewrap paths
    unchanged.
    
    The WSL detection reads /proc/version, lets an explicit WSL<version>
    marker decide WSL1 vs WSL2+, and only treats a bare Microsoft marker as
    WSL1 when no explicit WSL version is present.
    
    addresses https://github.com/openai/codex/issues/16076
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • fix: warn when bwrap cannot create user namespaces (#15893)
    ## Summary
    - add a Linux startup warning when system `bwrap` is present but cannot
    create user namespaces
    - keep the Linux-specific probe, sandbox-policy gate, and stderr
    matching in `codex-sandboxing`
    - polish the missing-`bwrap` warning to point users at the sandbox
    prerequisites and OS package-manager install path
    
    ## Details
    - probes system `bwrap` with `--unshare-user`, `--unshare-net`, and a
    minimal bind before command execution
    - detects known bubblewrap setup failures for `RTM_NEWADDR`,
    `RTM_NEWLINK`, uid-map permission denial, and `No permissions to create
    a new namespace`
    - preserves the existing suppression for sandbox-bypassed policies such
    as `danger-full-access` and `external-sandbox`
    - updates the Linux sandbox docs to call out the user-namespace
    requirement
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • chore: clean up argument-comment lint and roll out all-target CI on macOS (#16054)
    ## Why
    
    `argument-comment-lint` was green in CI even though the repo still had
    many uncommented literal arguments. The main gap was target coverage:
    the repo wrapper did not force Cargo to inspect test-only call sites, so
    examples like the `latest_session_lookup_params(true, ...)` tests in
    `codex-rs/tui_app_server/src/lib.rs` never entered the blocking CI path.
    
    This change cleans up the existing backlog, makes the default repo lint
    path cover all Cargo targets, and starts rolling that stricter CI
    enforcement out on the platform where it is currently validated.
    
    ## What changed
    
    - mechanically fixed existing `argument-comment-lint` violations across
    the `codex-rs` workspace, including tests, examples, and benches
    - updated `tools/argument-comment-lint/run-prebuilt-linter.sh` and
    `tools/argument-comment-lint/run.sh` so non-`--fix` runs default to
    `--all-targets` unless the caller explicitly narrows the target set
    - fixed both wrappers so forwarded cargo arguments after `--` are
    preserved with a single separator
    - documented the new default behavior in
    `tools/argument-comment-lint/README.md`
    - updated `rust-ci` so the macOS lint lane keeps the plain wrapper
    invocation and therefore enforces `--all-targets`, while Linux and
    Windows temporarily pass `-- --lib --bins`
    
    That temporary CI split keeps the stricter all-targets check where it is
    already cleaned up, while leaving room to finish the remaining Linux-
    and Windows-specific target-gated cleanup before enabling
    `--all-targets` on those runners. The Linux and Windows failures on the
    intermediate revision were caused by the wrapper forwarding bug, not by
    additional lint findings in those lanes.
    
    ## Validation
    
    - `bash -n tools/argument-comment-lint/run.sh`
    - `bash -n tools/argument-comment-lint/run-prebuilt-linter.sh`
    - shell-level wrapper forwarding check for `-- --lib --bins`
    - shell-level wrapper forwarding check for `-- --tests`
    - `just argument-comment-lint`
    - `cargo test` in `tools/argument-comment-lint`
    - `cargo test -p codex-terminal-detection`
    
    ## Follow-up
    
    - Clean up remaining Linux-only target-gated callsites, then switch the
    Linux lint lane back to the plain wrapper invocation.
    - Clean up remaining Windows-only target-gated callsites, then switch
    the Windows lint lane back to the plain wrapper invocation.
  • fix(sandbox): fix bwrap lookup for multi-entry PATH (#15973)
    ## Summary
    - split the joined `PATH` before running system `bwrap` lookup
    - keep the existing workspace-local `bwrap` skip behavior intact
    - add regression tests that exercise real multi-entry search paths
    
    ## Why
    The PATH-based lookup added in #15791 still wrapped the raw `PATH`
    environment value as a single `PathBuf` before passing it through
    `join_paths()`. On Unix, a normal multi-entry `PATH` contains `:`, so
    that wrapper path is invalid as one path element and the lookup returns
    `None`.
    
    That made Codex behave as if no system `bwrap` was installed even when
    `bwrap` was available on `PATH`, which is what users in #15340 were
    still hitting on `0.117.0-alpha.25`.
    
    ## Impact
    System `bwrap` discovery now works with normal multi-entry `PATH` values
    instead of silently falling back to the vendored binary.
    
    Fixes #15340.
    
    ## Validation
    - `just fmt`
    - `cargo test -p codex-sandboxing`
    - `cargo test -p codex-linux-sandbox`
    - `just fix -p codex-sandboxing`
    - `just argument-comment-lint`
  • chore: move bwrap config helpers into dedicated module (#15898)
    ## Summary
    - move the bwrap PATH lookup and warning helpers out of config/mod.rs
    - move the related tests into a dedicated bwrap_tests.rs file
    
    ## Validation
    - git diff --check
    - skipped heavier local tests per request
    
    Follow-up to #15791.