Commit Graph

9 Commits

  • fix: Fix tilde expansion to avoid absolute-path escape (#9621)
    ### Motivation
    - Prevent inputs like `~//` or `~///etc` from expanding to arbitrary
    absolute paths (e.g. `/`) because `Path::join` discards the left side
    when the right side is absolute, which could allow config values to
    escape `HOME` and broaden writable roots.
    
    ### Description
    - In `codex-rs/utils/absolute-path/src/lib.rs` update
    `maybe_expand_home_directory` to trim leading separators from the suffix
    and return `home` when the remainder is empty so tilde expansion stays
    rooted under `HOME`.
    - Add a non-Windows unit test
    `home_directory_double_slash_on_non_windows_is_expanded_in_deserialization`
    that validates `"~//code"` expands to `home.join("code")`.
    
    ### Testing
    - Ran `just fmt` successfully.
    - Ran `just fix -p codex-utils-absolute-path` (Clippy autofix)
    successfully.
    - Ran `cargo test -p codex-utils-absolute-path` and all tests passed.
    
    ------
    [Codex
    Task](https://chatgpt.com/codex/tasks/task_i_697007481cac832dbeb1ee144d1e4cbe)
  • 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)
  • feat: add support for building with Bazel (#8875)
    This PR configures Codex CLI so it can be built with
    [Bazel](https://bazel.build) in addition to Cargo. The `.bazelrc`
    includes configuration so that remote builds can be done using
    [BuildBuddy](https://www.buildbuddy.io).
    
    If you are familiar with Bazel, things should work as you expect, e.g.,
    run `bazel test //... --keep-going` to run all the tests in the repo,
    but we have also added some new aliases in the `justfile` for
    convenience:
    
    - `just bazel-test` to run tests locally
    - `just bazel-remote-test` to run tests remotely (currently, the remote
    build is for x86_64 Linux regardless of your host platform). Note we are
    currently seeing the following test failures in the remote build, so we
    still need to figure out what is happening here:
    
    ```
    failures:
        suite::compact::manual_compact_twice_preserves_latest_user_messages
        suite::compact_resume_fork::compact_resume_after_second_compaction_preserves_history
        suite::compact_resume_fork::compact_resume_and_fork_preserve_model_history_view
    ```
    
    - `just build-for-release` to build release binaries for all
    platforms/architectures remotely
    
    To setup remote execution:
    - [Create a buildbuddy account](https://app.buildbuddy.io/) (OpenAI
    employees should also request org access at
    https://openai.buildbuddy.io/join/ with their `@openai.com` email
    address.)
    - [Copy your API key](https://app.buildbuddy.io/docs/setup/) to
    `~/.bazelrc` (add the line `build
    --remote_header=x-buildbuddy-api-key=YOUR_KEY`)
    - Use `--config=remote` in your `bazel` invocations (or add `common
    --config=remote` to your `~/.bazelrc`, or use the `just` commands)
    
    ## CI
    
    In terms of CI, this PR introduces `.github/workflows/bazel.yml`, which
    uses Bazel to run the tests _locally_ on Mac and Linux GitHub runners
    (we are working on supporting Windows, but that is not ready yet). Note
    that the failures we are seeing in `just bazel-remote-test` do not occur
    on these GitHub CI jobs, so everything in `.github/workflows/bazel.yml`
    is green right now.
    
    The `bazel.yml` uses extra config in `.github/workflows/ci.bazelrc` so
    that macOS CI jobs build _remotely_ on Linux hosts (using the
    `docker://docker.io/mbolin491/codex-bazel` Docker image declared in the
    root `BUILD.bazel`) using cross-compilation to build the macOS
    artifacts. Then these artifacts are downloaded locally to GitHub's macOS
    runner so the tests can be executed natively. This is the relevant
    config that enables this:
    
    ```
    common:macos --config=remote
    common:macos --strategy=remote
    common:macos --strategy=TestRunner=darwin-sandbox,local
    ```
    
    Because of the remote caching benefits we get from BuildBuddy, these new
    CI jobs can be extremely fast! For example, consider these two jobs that
    ran all the tests on Linux x86_64:
    
    - Bazel 1m37s
    https://github.com/openai/codex/actions/runs/20861063212/job/59940545209?pr=8875
    - Cargo 9m20s
    https://github.com/openai/codex/actions/runs/20861063192/job/59940559592?pr=8875
    
    For now, we will continue to run both the Bazel and Cargo jobs for PRs,
    but once we add support for Windows and running Clippy, we should be
    able to cutover to using Bazel exclusively for PRs, which should still
    speed things up considerably. We will probably continue to run the Cargo
    jobs post-merge for commits that land on `main` as a sanity check.
    
    Release builds will also continue to be done by Cargo for now.
    
    Earlier attempt at this PR: https://github.com/openai/codex/pull/8832
    Earlier attempt to add support for Buck2, now abandoned:
    https://github.com/openai/codex/pull/8504
    
    ---------
    
    Co-authored-by: David Zbarsky <dzbarsky@gmail.com>
    Co-authored-by: Michael Bolin <mbolin@openai.com>
  • fix: implement 'Allow this session' for apply_patch approvals (#8451)
    **Summary**
    This PR makes “ApprovalDecision::AcceptForSession / don’t ask again this
    session” actually work for `apply_patch` approvals by caching approvals
    based on absolute file paths in codex-core, properly wiring it through
    app-server v2, and exposing the choice in both TUI and TUI2.
    - This brings `apply_patch` calls to be at feature-parity with general
    shell commands, which also have a "Yes, and don't ask again" option.
    - This also fixes VSCE's "Allow this session" button to actually work.
    
    While we're at it, also split the app-server v2 protocol's
    `ApprovalDecision` enum so execpolicy amendments are only available for
    command execution approvals.
    
    **Key changes**
    - Core: per-session patch approval allowlist keyed by absolute file
    paths
    - Handles multi-file patches and renames/moves by recording both source
    and destination paths for `Update { move_path: Some(...) }`.
    - Extend the `Approvable` trait and `ApplyPatchRuntime` to work with
    multiple keys, because an `apply_patch` tool call can modify multiple
    files. For a request to be auto-approved, we will need to check that all
    file paths have been approved previously.
    - App-server v2: honor AcceptForSession for file changes
    - File-change approval responses now map AcceptForSession to
    ReviewDecision::ApprovedForSession (no longer downgraded to plain
    Approved).
    - Replace `ApprovalDecision` with two enums:
    `CommandExecutionApprovalDecision` and `FileChangeApprovalDecision`
    - TUI / TUI2: expose “don’t ask again for these files this session”
    - Patch approval overlays now include a third option (“Yes, and don’t
    ask again for these files this session (s)”).
        - Snapshot updates for the approval modal.
    
    **Tests added/updated**
    - Core:
    - Integration test that proves ApprovedForSession on a patch skips the
    next patch prompt for the same file
    - App-server:
    - v2 integration test verifying
    FileChangeApprovalDecision::AcceptForSession works properly
    
    **User-visible behavior**
    - When the user approves a patch “for session”, future patches touching
    only those previously approved file(s) will no longer prompt gain during
    that session (both via app-server v2 and TUI/TUI2).
    
    **Manual testing**
    Tested both TUI and TUI2 - see screenshots below.
    
    TUI:
    <img width="1082" height="355" alt="image"
    src="https://github.com/user-attachments/assets/adcf45ad-d428-498d-92fc-1a0a420878d9"
    />
    
    
    TUI2:
    <img width="1089" height="438" alt="image"
    src="https://github.com/user-attachments/assets/dd768b1a-2f5f-4bd6-98fd-e52c1d3abd9e"
    />
  • feat: load ExecPolicyManager from ConfigLayerStack (#8453)
    https://github.com/openai/codex/pull/8354 added support for in-repo
    `.config/` files, so this PR updates the logic for loading `*.rules`
    files to load `*.rules` files from all relevant layers. The main change
    to the business logic is `load_exec_policy()` in
    `codex-rs/core/src/exec_policy.rs`.
    
    Note this adds a `config_folder()` method to `ConfigLayerSource` that
    returns `Option<AbsolutePathBuf>` so that it is straightforward to
    iterate over the sources and get the associated config folder, if any.
  • feat: support in-repo .codex/config.toml entries as sources of config info (#8354)
    - We now support `.codex/config.toml` in repo (from `cwd` up to the
    first `.git` found, if any) as layers in `ConfigLayerStack`. A new
    `ConfigLayerSource::Project` variant was added to support this.
    - In doing this work, I realized that we were resolving relative paths
    in `config.toml` after merging everything into one `toml::Value`, which
    is wrong: paths should be relativized with respect to the folder
    containing the `config.toml` that was deserialized. This PR introduces a
    deserialize/re-serialize strategy to account for this in
    `resolve_config_paths()`. (This is why `Serialize` is added to so many
    types as part of this PR.)
    - Added tests to verify this new behavior.
    
    
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/8354).
    * #8359
    * __->__ #8354
  • chore: enusre the logic that creates ConfigLayerStack has access to cwd (#8353)
    `load_config_layers_state()` should load config from a
    `.codex/config.toml` in any folder between the `cwd` for a thread and
    the project root. Though in order to do that,
    `load_config_layers_state()` needs to know what the `cwd` is, so this PR
    does the work to thread the `cwd` through for existing callsites.
    
    A notable exception is the `/config` endpoint in app server for which a
    `cwd` is not guaranteed to be associated with the query, so the `cwd`
    param is `Option<AbsolutePathBuf>` to account for this case.
    
    The logic to make use of the `cwd` will be done in a follow-up PR.
  • 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.