Commit Graph

8 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.
  • 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: support split carveouts in windows restricted-token sandbox (#14172)
    ## Summary
    - keep legacy Windows restricted-token sandboxing as the supported
    baseline
    - support the split-policy subset that restricted-token can enforce
    directly today
    - support full-disk read, the same writable root set as legacy
    `WorkspaceWrite`, and extra read-only carveouts under those writable
    roots via additional deny-write ACLs
    - continue to fail closed for unsupported split-only shapes, including
    explicit unreadable (`none`) carveouts, reopened writable descendants
    under read-only carveouts, and writable root sets that do not match the
    legacy workspace roots
    
    ## Example
    Given a filesystem policy like:
    
    ```toml
    ":root" = "read"
    ":cwd" = "write"
    "./docs" = "read"
    ```
    
    the restricted-token backend can keep the workspace writable while
    denying writes under `docs` by layering an extra deny-write carveout on
    top of the legacy workspace-write roots.
    
    A policy like:
    
    ```toml
    "/workspace" = "write"
    "/workspace/docs" = "read"
    "/workspace/docs/tmp" = "write"
    ```
    
    still fails closed, because the unelevated backend cannot reopen the
    nested writable descendant safely.
    
    ## Stack
    -> fix: support split carveouts in windows restricted-token sandbox
    #14172
    fix: support split carveouts in windows elevated sandbox #14568
  • feat: support restricted ReadOnlyAccess in elevated Windows sandbox (#14610)
    ## Summary
    - support legacy `ReadOnlyAccess::Restricted` on Windows in the elevated
    setup/runner backend
    - keep the unelevated restricted-token backend on the legacy full-read
    model only, and fail closed for restricted read-only policies there
    - keep the legacy full-read Windows path unchanged while deriving
    narrower read roots only for elevated restricted-read policies
    - honor `include_platform_defaults` by adding backend-managed Windows
    system roots only when requested, while always keeping helper roots and
    the command `cwd` readable
    - preserve `workspace-write` semantics by keeping writable roots
    readable when restricted read access is in use in the elevated backend
    - document the current Windows boundary: legacy `SandboxPolicy` is
    supported on both backends, while richer split-only carveouts still fail
    closed instead of running with weaker enforcement
    
    ## Testing
    - `cargo test -p codex-windows-sandbox`
    - `cargo check -p codex-windows-sandbox --tests --target
    x86_64-pc-windows-msvc`
    - `cargo clippy -p codex-windows-sandbox --tests --target
    x86_64-pc-windows-msvc -- -D warnings`
    - `cargo test -p codex-core windows_restricted_token_`
    
    ## Notes
    - local `cargo test -p codex-windows-sandbox` on macOS only exercises
    the non-Windows stubs; the Windows-targeted compile and clippy runs
    provide the local signal, and GitHub Windows CI exercises the runtime
    path
  • Use a private desktop for Windows sandbox instead of Winsta0\Default (#14400)
    ## Summary
    - launch Windows sandboxed children on a private desktop instead of
    `Winsta0\Default`
    - make private desktop the default while keeping
    `windows.sandbox_private_desktop=false` as the escape hatch
    - centralize process launch through the shared
    `create_process_as_user(...)` path
    - scope the private desktop ACL to the launching logon SID
    
    ## Why
    Today sandboxed Windows commands run on the visible shared desktop. That
    leaves an avoidable same-desktop attack surface for window interaction,
    spoofing, and related UI/input issues. This change moves sandboxed
    commands onto a dedicated per-launch desktop by default so the sandbox
    no longer shares `Winsta0\Default` with the user session.
    
    The implementation stays conservative on security with no silent
    fallback back to `Winsta0\Default`
    
    If private-desktop setup fails on a machine, users can still opt out
    explicitly with `windows.sandbox_private_desktop=false`.
    
    ## Validation
    - `cargo build -p codex-cli`
    - elevated-path `codex exec` desktop-name probe returned
    `CodexSandboxDesktop-*`
    - elevated-path `codex exec` smoke sweep for shell commands, nested
    `pwsh`, jobs, and hidden `notepad` launch
    - unelevated-path full private-desktop compatibility sweep via `codex
    exec` with `-c windows.sandbox=unelevated`
  • fix: move inline codex-rs/core unit tests into sibling files (#14444)
    ## Why
    PR #13783 moved the `codex.rs` unit tests into `codex_tests.rs`. This
    applies the same extraction pattern across the rest of `codex-rs/core`
    so the production modules stay focused on runtime code instead of large
    inline test blocks.
    
    Keeping the tests in sibling files also makes follow-up edits easier to
    review because product changes no longer have to share a file with
    hundreds or thousands of lines of test scaffolding.
    
    ## What changed
    - replaced each inline `mod tests { ... }` in `codex-rs/core/src/**`
    with a path-based module declaration
    - moved each extracted unit test module into a sibling `*_tests.rs`
    file, using `mod_tests.rs` for `mod.rs` modules
    - preserved the existing `cfg(...)` guards and module-local structure so
    the refactor remains structural rather than behavioral
    
    ## Testing
    - `cargo test -p codex-core --lib` (`1653 passed; 0 failed; 5 ignored`)
    - `just fix -p codex-core`
    - `cargo fmt --check`
    - `cargo shear`