6 Commits

  • Disable empty Cargo test targets (#21584)
    ## Summary
    
    `cargo test` has entails both running standard Rust tests and doctests.
    It turns out that the doctest discovery is fairly slow, and it's a cost
    you pay even for crates that don't include any doctests.
    
    This PR disables doctests with `doctest = false` for crates that lack
    any doctests.
    
    For the collection of crates below, this speeds up test execution by
    >4x.
    
    E.g., before this PR:
    
    ```
    Benchmark 1: cargo test     -p codex-utils-absolute-path     -p codex-utils-cache     -p codex-utils-cli     -p codex-utils-home-dir     -p codex-utils-output-truncation     -p codex-utils-path     -p codex-utils-string     -p codex-utils-template     -p codex-utils-elapsed     -p codex-utils-json-to-toml
      Time (mean ± σ):      1.849 s ±  4.455 s    [User: 0.752 s, System: 1.367 s]
      Range (min … max):    0.418 s … 14.529 s    10 runs
    ```
    
    And after:
    
    ```
    Benchmark 1: cargo test     -p codex-utils-absolute-path     -p codex-utils-cache     -p codex-utils-cli     -p codex-utils-home-dir     -p codex-utils-output-truncation     -p codex-utils-path     -p codex-utils-string     -p codex-utils-template     -p codex-utils-elapsed     -p codex-utils-json-to-toml
      Time (mean ± σ):     428.6 ms ±   6.9 ms    [User: 187.7 ms, System: 219.7 ms]
      Range (min … max):   418.0 ms … 436.8 ms    10 runs
    ```
    
    For a single crate, with >2x speedup, before:
    
    ```
    Benchmark 1: cargo test -p codex-utils-string
      Time (mean ± σ):     491.1 ms ±   9.0 ms    [User: 229.8 ms, System: 234.9 ms]
      Range (min … max):   480.9 ms … 512.0 ms    10 runs
    ```
    
    And after:
    
    ```
    Benchmark 1: cargo test -p codex-utils-string
      Time (mean ± σ):     213.9 ms ±   4.3 ms    [User: 112.8 ms, System: 84.0 ms]
      Range (min … max):   206.8 ms … 221.0 ms    13 runs
    ```
    
    Co-authored-by: Codex <noreply@openai.com>
  • fix(permissions): fix symlinked writable roots in sandbox permissions (#15981)
    ## Summary
    - preserve logical symlink paths during permission normalization and
    config cwd handling
    - bind real targets for symlinked readable/writable roots in bwrap and
    remap carveouts and unreadable roots there
    - add regressions for symlinked carveouts and nested symlink escape
    masking
    
    ## Root cause
    Permission normalization canonicalized symlinked writable roots and cwd
    to their real targets too early. That drifted policy checks away from
    the logical paths the sandboxed process can actually address, while
    bwrap still needed the real targets for mounts. The mismatch caused
    shell and apply_patch failures on symlinked writable roots.
    
    ## Impact
    Fixes #15781.
    
    Also fixes #17079:
    - #17079 is the protected symlinked carveout side: bwrap now binds the
    real symlinked writable-root target and remaps carveouts before masking.
    
    Related to #15157:
    - #15157 is the broader permission-check side of this path-identity
    problem. This PR addresses the shared logical-vs-canonical normalization
    issue, but the reported Darwin prompt behavior should be validated
    separately before auto-closing it.
    
    This should also fix #14672, #14694, #14715, and #15725:
    - #14672, #14694, and #14715 are the same Linux
    symlinked-writable-root/bwrap family as #15781.
    - #15725 is the protected symlinked workspace path variant; the PR
    preserves the protected logical path in policy space while bwrap applies
    read-only or unreadable treatment to the resolved target so
    file-vs-directory bind mismatches do not abort sandbox setup.
    
    ## Notes
    - Added Linux-only regressions for symlinked writable ancestors and
    protected symlinked directory targets, including nested symlink escape
    masking without rebinding the escape target writable.
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • [codex] Make AbsolutePathBuf joins infallible (#16981)
    Having to check for errors every time join is called is painful and
    unnecessary.
  • fix: writable_roots doesn't recognize home directory symbol in non-windows OS (#9193)
    Fixes:
    ```
    [sandbox_workspace_write]
    writable_roots = ["~/code/"]
    ```
    
    translates to
    ```
    /Users/ccunningham/.codex/~/code
    ```
    (i.e. the home dir symbol isn't recognized)
  • fix: introduce AbsolutePathBuf as part of sandbox config (#7856)
    Changes the `writable_roots` field of the `WorkspaceWrite` variant of
    the `SandboxPolicy` enum from `Vec<PathBuf>` to `Vec<AbsolutePathBuf>`.
    This is helpful because now callers can be sure the value is an absolute
    path rather than a relative one. (Though when using an absolute path in
    a Seatbelt config policy, we still have to _canonicalize_ it first.)
    
    Because `writable_roots` can be read from a config file, it is important
    that we are able to resolve relative paths properly using the parent
    folder of the config file as the base path.
  • fix: introduce AbsolutePathBuf and resolve relative paths in config.toml (#7796)
    This PR attempts to solve two problems by introducing a
    `AbsolutePathBuf` type with a special deserializer:
    
    - `AbsolutePathBuf` attempts to be a generally useful abstraction, as it
    ensures, by constructing, that it represents a value that is an
    absolute, normalized path, which is a stronger guarantee than an
    arbitrary `PathBuf`.
    - Values in `config.toml` that can be either an absolute or relative
    path should be resolved against the folder containing the `config.toml`
    in the relative path case. This PR makes this easy to support: the main
    cost is ensuring `AbsolutePathBufGuard` is used inside
    `deserialize_config_toml_with_base()`.
    
    While `AbsolutePathBufGuard` may seem slightly distasteful because it
    relies on thread-local storage, this seems much cleaner to me than using
    than my various experiments with
    https://docs.rs/serde/latest/serde/de/trait.DeserializeSeed.html.
    Further, since the `deserialize()` method from the `Deserialize` trait
    is not async, we do not really have to worry about the deserialization
    work being spread across multiple threads in a way that would interfere
    with `AbsolutePathBufGuard`.
    
    To start, this PR introduces the use of `AbsolutePathBuf` in
    `OtelTlsConfig`. Note how this simplifies `otel_provider.rs` because it
    no longer requires `settings.codex_home` to be threaded through.
    Furthermore, this sets us up better for a world where multiple
    `config.toml` files from different folders could be loaded and then
    merged together, as the absolutifying of the paths must be done against
    the correct parent folder.