Commit Graph

15 Commits

  • permissions: remove core legacy policy round trips (#19394)
    ## Why
    
    Several execution paths still converted profile-backed permissions into
    `SandboxPolicy` and then rebuilt runtime permissions from that legacy
    shape. Those round trips are unnecessary after the preceding PRs and can
    lose split filesystem semantics. Core approval and escalation should
    carry the resolved profile directly.
    
    ## What Changed
    
    - Removes `sandbox_policy` from `ResolvedPermissionProfile`; the
    resolved permission object now carries the canonical `PermissionProfile`
    directly.
    - Updates exec-policy fallback, shell/unified-exec interception,
    escalation reruns, and related tests to pass profiles instead of legacy
    policies.
    - Removes legacy additional-permission merge helpers that built an
    effective `SandboxPolicy` before rebuilding runtime permissions.
    - Keeps legacy projections only at compatibility boundaries that still
    require `SandboxPolicy`, not in core permission computation.
    
    ## Verification
    
    - `cargo test -p codex-core direct_write_roots`
    - `cargo test -p codex-core runtime_roots_to_legacy_projection`
    - `cargo test -p codex-app-server
    requested_permissions_trust_project_uses_permission_profile_intent`
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/19394).
    * #19737
    * #19736
    * #19735
    * #19734
    * #19395
    * __->__ #19394
  • [codex] Move config loading into codex-config (#19487)
    ## Why
    
    Config loading had become split across crates: `codex-config` owned the
    config types and merge logic, while `codex-core` still owned the loader
    that assembled the layer stack. This change consolidates that
    responsibility in `codex-config`, so the crate that defines config
    behavior also owns how configs are discovered and loaded.
    
    To make that move possible without reintroducing the old dependency
    cycle, the shell-environment policy types and helpers that
    `codex-exec-server` needs now live in `codex-protocol` instead of
    flowing through `codex-config`.
    
    This also makes the migrated loader tests more deterministic on machines
    that already have managed or system Codex config installed by letting
    tests override the system config and requirements paths instead of
    reading the host's `/etc/codex`.
    
    ## What Changed
    
    - moved the config loader implementation from `codex-core` into
    `codex-config::loader` and deleted the old `core::config_loader` module
    instead of leaving a compatibility shim
    - moved shell-environment policy types and helpers into
    `codex-protocol`, then updated `codex-exec-server` and other downstream
    crates to import them from their new home
    - updated downstream callers to use loader/config APIs from
    `codex-config`
    - added test-only loader overrides for system config and requirements
    paths so loader-focused tests do not depend on host-managed config state
    - cleaned up now-unused dependency entries and platform-specific cfgs
    that were surfaced by post-push CI
    
    ## Testing
    
    - `cargo test -p codex-config`
    - `cargo test -p codex-core config_loader_tests::`
    - `cargo test -p codex-protocol -p codex-exec-server -p
    codex-cloud-requirements -p codex-rmcp-client --lib`
    - `cargo test --lib -p codex-app-server-client -p codex-exec`
    - `cargo test --no-run --lib -p codex-app-server`
    - `cargo test -p codex-linux-sandbox --lib`
    - `cargo shear`
    - `just bazel-lock-check`
    
    ## Notes
    
    - I did not chase unrelated full-suite failures outside the migrated
    loader surface.
    - `cargo test -p codex-core --lib` still hits unrelated proxy-sensitive
    failures on this machine, and Windows CI still shows unrelated
    long-running/timeouting test noise outside the loader migration itself.
  • feat: add --ignore-user-config and --ignore-rules (#18646)
    Add those 2 flags to be able to fully isolate a run of `codex exec` from
    any rules or tools.
    This will be used by Chronicle
  • fix: trust-gate project hooks and exec policies (#14718)
    ## Summary
    - trust-gate project `.codex` layers consistently, including repos that
    have `.codex/hooks.json` or `.codex/execpolicy/*.rules` but no
    `.codex/config.toml`
    - keep disabled project layers in the config stack so nested trusted
    project layers still resolve correctly, while preventing hooks and exec
    policies from loading until the project is trusted
    - update app-server/TUI onboarding copy to make the trust boundary
    explicit and add regressions for loader, hooks, exec-policy, and
    onboarding coverage
    
    ## Security
    Before this change, an untrusted repo could auto-load project hooks or
    exec policies from `.codex/` as long as `config.toml` was absent. This
    makes trust the single gate for project-local config, hooks, and exec
    policies.
    
    ## Stack
    - Parent of #15936
    
    ## Test
    - cargo test -p codex-core without_config_toml
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • Fix Windows exec policy test flake (#18304)
    ## Summary
    
    This fixes a Windows-only failure in the exec policy multi-segment shell
    test. The test was meant to verify that a compound shell command only
    bypasses sandboxing when every parsed segment has an explicit exec
    policy allow rule.
    
    On Windows, the read-only sandbox setup is intentionally treated as
    lacking sandbox protection, so the old fixture could take the approval
    path before reaching the intended bypass assertion. The test now uses
    the workspace-write sandbox policy, keeping the focus on the per-segment
    bypass rule while preserving the expected bypass_sandbox false result
    when only cat is explicitly allowed.
  • fix(exec-policy) rules parsing (#18126)
    ## Summary
    See scenarios - rules must always be enforced on all commands in the
    string
    
    ## Testing
    - [x] Added ExecApprovalRequirementScenario tests
  • feat(requirements): support allowed_approval_reviewers (#16701)
    ## Description
    
    Add requirements.toml support for `allowed_approvals_reviewers =
    ["user", "guardian_subagent"]`, so admins can now restrict the use of
    guardian mode.
    
    Note: If a user sets a reviewer that isn’t allowed by requirements.toml,
    config loading falls back to the first allowed reviewer and emits a
    startup warning.
    
    The table below describes the possible admin controls.
    | Admin intent | `requirements.toml` | User `config.toml` | End result |
    |---|---|---|---|
    | Leave Guardian optional | omit `allowed_approvals_reviewers` or set
    `["user", "guardian_subagent"]` | user chooses `approvals_reviewer =
    "user"` or `"guardian_subagent"` | Guardian off for `user`, on for
    `guardian_subagent` + `approval_policy = "on-request"` |
    | Force Guardian off | `allowed_approvals_reviewers = ["user"]` | any
    user value | Effective reviewer is `user`; Guardian off |
    | Force Guardian on | `allowed_approvals_reviewers =
    ["guardian_subagent"]` and usually `allowed_approval_policies =
    ["on-request"]` | any user reviewer value; user should also have
    `approval_policy = "on-request"` unless policy is forced | Effective
    reviewer is `guardian_subagent`; Guardian on when effective approval
    policy is `on-request` |
    | Allow both, but default to manual if user does nothing |
    `allowed_approvals_reviewers = ["user", "guardian_subagent"]` | omit
    `approvals_reviewer` | Effective reviewer is `user`; Guardian off |
    | Allow both, and user explicitly opts into Guardian |
    `allowed_approvals_reviewers = ["user", "guardian_subagent"]` |
    `approvals_reviewer = "guardian_subagent"` and `approval_policy =
    "on-request"` | Guardian on |
    | Invalid admin config | `allowed_approvals_reviewers = []` | anything |
    Config load error |
  • 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.
  • 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(exec_policy) ExecPolicyRequirementScenario tests (#15415)
    ## Summary
    Consolidate exec_policy_tests on `ExecApprovalRequirementScenario` for
    consistency.
    
    ## Testing
    - [x] These are tests
  • fix(core) disable command_might_be_dangerous when unsandboxed (#15036)
    ## Summary
    If we are in a mode that is already explicitly un-sandboxed, then
    `ApprovalPolicy::Never` should not block dangerous commands.
    
    ## Testing
    - [x] Existing unit test covers old behavior
    - [x] Added a unit test for this new case
  • fix(subagents) share execpolicy by default (#13702)
    ## Summary
    If a subagent requests approval, and the user persists that approval to
    the execpolicy, it should (by default) propagate. We'll need to rethink
    this a bit in light of coming Permissions changes, though I think this
    is closer to the end state that we'd want, which is that execpolicy
    changes to one permissions profile should be synced across threads.
    
    ## Testing
    - [x] Added integration test
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • 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`