Commit Graph

12 Commits

  • 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`
  • permissions: remove macOS seatbelt extension profiles (#15918)
    ## Why
    
    `PermissionProfile` should only describe the per-command permissions we
    still want to grant dynamically. Keeping
    `MacOsSeatbeltProfileExtensions` in that surface forced extra macOS-only
    approval, protocol, schema, and TUI branches for a capability we no
    longer want to expose.
    
    ## What changed
    
    - Removed the macOS-specific permission-profile types from
    `codex-protocol`, the app-server v2 API, and the generated
    schema/TypeScript artifacts.
    - Deleted the core and sandboxing plumbing that threaded
    `MacOsSeatbeltProfileExtensions` through execution requests and seatbelt
    construction.
    - Simplified macOS seatbelt generation so it always includes the fixed
    read-only preferences allowlist instead of carrying a configurable
    profile extension.
    - Removed the macOS additional-permissions UI/docs/test coverage and
    deleted the obsolete macOS permission modules.
    - Tightened `request_permissions` intersection handling so explicitly
    empty requested read lists are preserved only when that field was
    actually granted, avoiding zero-grant responses being stored as active
    permissions.
  • 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.
  • sandboxing: use OsString for SandboxCommand.program (#15897)
    ## Why
    
    `SandboxCommand.program` represents an executable path, but keeping it
    as `String` forced path-backed callers to run `to_string_lossy()` before
    the sandbox layer ever touched the command. That loses fidelity earlier
    than necessary and adds avoidable conversions in runtimes that already
    have a `PathBuf`.
    
    ## What changed
    
    - Changed `SandboxCommand.program` to `OsString`.
    - Updated `SandboxManager::transform` to keep the program and argv in
    `OsString` form until the `SandboxExecRequest` conversion boundary.
    - Switched the path-backed `apply_patch` and `js_repl` runtimes to pass
    `into_os_string()` instead of `to_string_lossy()`.
    - Updated the remaining string-backed builders and tests to match the
    new type while preserving the existing Linux helper `arg0` behavior.
    
    ## Verification
    
    - `cargo test -p codex-sandboxing`
    - `just argument-comment-lint -p codex-core -p codex-sandboxing`
    - `cargo test -p codex-core` currently fails in unrelated existing
    config tests: `config::tests::approvals_reviewer_*` and
    `config::tests::smart_approvals_alias_*`
  • Protect first-time project .codex creation across Linux and macOS sandboxes (#15067)
    ## Problem
    
    Codex already treated an existing top-level project `./.codex` directory
    as protected, but there was a gap on first creation.
    
    If `./.codex` did not exist yet, a turn could create files under it,
    such as `./.codex/config.toml`, without going through the same approval
    path as later modifications. That meant the initial write could bypass
    the intended protection for project-local Codex state.
    
    ## What this changes
    
    This PR closes that first-creation gap in the Unix enforcement layers:
    
    - `codex-protocol`
    - treat the top-level project `./.codex` path as a protected carveout
    even when it does not exist yet
    - avoid injecting the default carveout when the user already has an
    explicit rule for that exact path
    - macOS Seatbelt
    - deny writes to both the exact protected path and anything beneath it,
    so creating `./.codex` itself is blocked in addition to writes inside it
    - Linux bubblewrap
    - preserve the same protected-path behavior for first-time creation
    under `./.codex`
    - tests
    - add protocol regressions for missing `./.codex` and explicit-rule
    collisions
    - add Unix sandbox coverage for blocking first-time `./.codex` creation
      - tighten Seatbelt policy assertions around excluded subpaths
    
    ## Scope
    
    This change is intentionally scoped to protecting the top-level project
    `.codex` subtree from agent writes.
    
    It does not make `.codex` unreadable, and it does not change the product
    behavior around loading project skills from `.codex` when project config
    is untrusted.
    
    ## Why this shape
    
    The fix is pointed rather than broad:
    - it preserves the current model of “project `.codex` is protected from
    writes”
    - it closes the security-relevant first-write hole
    - it avoids folding a larger permissions-model redesign into this PR
    
    ## Validation
    
    - `cargo test -p codex-protocol`
    - `cargo test -p codex-sandboxing seatbelt`
    - `cargo test -p codex-exec --test all
    sandbox_blocks_first_time_dot_codex_creation -- --nocapture`
    
    ---------
    
    Co-authored-by: Michael Bolin <mbolin@openai.com>
  • fix: fix old system bubblewrap compatibility without falling back to vendored bwrap (#15693)
    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/bwrap` whenever 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 `--argv0` instead of switching binaries:
    - pass the sandbox helper a full-path `argv0`,
    - keep the existing `current_exe() + --argv0` path when the selected
    launcher supports it,
    - otherwise omit `--argv0` and re-exec through the helper's own
    `argv[0]` path, whose basename still dispatches as
    `codex-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
    
    1. Install Ubuntu 20.04 in a VM
    2. Compile codex and run without bubblewrap installed - see a warning
    about falling back to the vendored bwrap
    3. Install bwrap and verify version is 0.4.0 without `argv0` support
    4. run codex and use apply_patch tool without errors
    
    <img width="802" height="631" alt="Screenshot 2026-03-25 at 11 48 36 PM"
    src="https://github.com/user-attachments/assets/77248a29-aa38-4d7c-9833-496ec6a458b8"
    />
    <img width="807" height="634" alt="Screenshot 2026-03-25 at 11 47 32 PM"
    src="https://github.com/user-attachments/assets/5af8b850-a466-489b-95a6-455b76b5050f"
    />
    <img width="812" height="635" alt="Screenshot 2026-03-25 at 11 45 45 PM"
    src="https://github.com/user-attachments/assets/438074f0-8435-4274-a667-332efdd5cb57"
    />
    <img width="801" height="623" alt="Screenshot 2026-03-25 at 11 43 56 PM"
    src="https://github.com/user-attachments/assets/0dc8d3f5-e8cf-4218-b4b4-a4f7d9bf02e3"
    />
    
    ---------
    
    Co-authored-by: Michael Bolin <mbolin@openai.com>
  • Move sandbox policy transforms into codex-sandboxing (#15599)
    ## Summary
    - move the pure sandbox policy transform helpers from `codex-core` into
    `codex-sandboxing`
    - move the corresponding unit tests with the extracted implementation
    - update `core` and `app-server` callers to import the moved APIs
    directly, without re-exports or proxy methods
    
    ## Testing
    - cargo test -p codex-sandboxing
    - cargo test -p codex-core sandboxing
    - cargo test -p codex-app-server --lib
    - just fix -p codex-sandboxing
    - just fix -p codex-core
    - just fix -p codex-app-server
    - just fmt
    - just argument-comment-lint
  • Move macOS sandbox builders into codex-sandboxing (#15593)
    ## Summary
    - move macOS permission merging/intersection logic and tests from
    `codex-core` into `codex-sandboxing`
    - move seatbelt policy builders, permissions logic, SBPL assets, and
    their tests into `codex-sandboxing`
    - keep `codex-core` owning only the seatbelt spawn wrapper and switch
    call sites to import the moved APIs directly
    
    ## Notes
    - no re-exports added
    - moved the seatbelt tests with the implementation so internal helpers
    could stay private
    - local verification is still finishing while this PR is open
  • Extract landlock helpers into codex-sandboxing (#15592)
    ## Summary
    - add a new `codex-sandboxing` crate for sandboxing extraction work
    - move the pure Linux sandbox argv builders and their unit tests out of
    `codex-core`
    - keep `core::landlock` as the spawn wrapper and update direct callers
    to use `codex_sandboxing::landlock`
    
    ## Testing
    - `cargo test -p codex-sandboxing`
    - `cargo test -p codex-core landlock`
    - `cargo test -p codex-cli debug_sandbox`
    - `just argument-comment-lint`
    
    ## Notes
    - this is step 1 of the move plan aimed at minimizing per-PR diffs
    - no re-exports or no-op proxy methods were added