15 Commits

• Use AbsolutePathBuf in skill loading and codex_home (#17407)

  Helps with FS migration later

  pakrym-oai · 2026-04-13 10:26:51 -07:00
  ac82443d07
• 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>

  viyatb-oai · 2026-04-10 17:00:58 -07:00
  b114781495
• [codex] Migrate apply_patch to executor filesystem (#17027)

  - Migrate apply-patch verification and application internals to use the
  async `ExecutorFileSystem` abstraction from `exec-server`.
  - Convert apply-patch `cwd` handling to `AbsolutePathBuf` through the
  verifier/parser/handler boundary.
  
  Doesn't change how the tool itself works.

  pakrym-oai · 2026-04-07 21:20:22 +00:00
  e9702411ab
• [codex] Make AbsolutePathBuf joins infallible (#16981)

  Having to check for errors every time join is called is painful and
  unnecessary.

  pakrym-oai · 2026-04-07 10:52:08 -07:00
  f1a2b920f9
• Expand home-relative paths on Windows (#15817)

  Follow up to: https://github.com/openai/codex/pull/9193, also support
  this for Windows.
  
  ---------
  
  Co-authored-by: Michael Bolin <mbolin@openai.com>

  Tiffany Citra · 2026-03-25 21:19:57 -07:00
  6d0525ae70
• Use AbsolutePathBuf for cwd state (#15710)

  Migrate `cwd` and related session/config state to `AbsolutePathBuf` so
  downstream consumers consistently see absolute working directories.
  
  Add test-only `.abs()` helpers for `Path`, `PathBuf`, and `TempDir`, and
  update branch-local tests to use them instead of
  `AbsolutePathBuf::try_from(...)`.
  
  For the remaining TUI/app-server snapshot coverage that renders absolute
  cwd values, keep the snapshots unchanged and skip the Windows-only cases
  where the platform-specific absolute path layout differs.

  pakrym-oai · 2026-03-25 16:02:22 +00:00
  504aeb0e09
• Avoid AbsolutePathBuf::parent() panic under EMFILE by skipping re-absolutization (#12647)

  Fixes #12216
  
  Fixes a panic in `AbsolutePathBuf::parent()` when the process hits file
  descriptor exhaustion (`EMFILE` / "Too many open files").
  
  ### Root cause
  
  `AbsolutePathBuf::parent()` was re-validating the parent path via
  `from_absolute_path(...).expect(...)`.
  
  `from_absolute_path()` calls `path_absolutize::absolutize()`, which can
  depend on `std::env::current_dir()`. Under `EMFILE`, that can fail,
  causing `parent()` to panic even though the parent of an absolute path
  is already known.
  
  ### Change
  
  - Stop re-absolutizing the result of `self.0.parent()`
  - Construct `AbsolutePathBuf` directly from the known parent path
  - Keep an invariant check with `debug_assert!(p.is_absolute())`
  
  ### Why this is safe
  
  `self` is already an `AbsolutePathBuf`, so `self.0` is
  absolute/normalized. The parent of an absolute path is expected to be
  absolute, so re-running fallible normalization here is unnecessary and
  can introduce unrelated panics.

  Eric Traut · 2026-02-23 21:59:33 -08:00
  a4076ab4b1
• 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)

  Tiffany Citra · 2026-01-21 10:43:10 -08:00
  8179312ff5
• 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)

  Tiffany Citra · 2026-01-20 10:55:01 -08:00
  2c3843728c
• 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"
  />

  Owen Lin · 2026-01-07 20:11:12 +00:00
  66450f0445
• 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.

  Michael Bolin · 2025-12-22 17:24:17 -08:00
  277babba79
• 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

  Michael Bolin · 2025-12-22 11:07:36 -08:00
  8ff16a7714
• 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.

  Michael Bolin · 2025-12-19 20:11:27 -08:00
  a6974087e5
• 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.

  Michael Bolin · 2025-12-12 15:25:22 -08:00
  642b7566df
• 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.

  Michael Bolin · 2025-12-09 17:37:52 -08:00
  fa4cac1e6b