Commit Graph

14 Commits

  • Lift app-server JSON-RPC error handling to request boundary (#19484)
    ## Why
    
    App-server request handling had a lot of repeated JSON-RPC error
    construction and one-off `send_error`/`return` branches. This made small
    handlers noisy and pushed error response details into leaf code that
    otherwise only needed to validate input or call the underlying API.
    
    ## What Changed
    
    - Added shared JSON-RPC error constructors in
    `codex-rs/app-server/src/error_code.rs`.
    - Lifted straightforward request result emission into
    `codex-rs/app-server/src/message_processor.rs` so response/error
    dispatch happens at the request boundary.
    - Reused the result helpers across command exec, config, filesystem,
    device-key, external-agent config, fs-watch, and outgoing-message paths.
    - Removed leaf wrapper handlers where the method body was only
    forwarding to a response helper.
    - Returned request validation errors upward in the simple cases instead
    of sending an error locally and immediately returning.
    
    ## Verification
    
    - `cargo test -p codex-app-server --lib command_exec::tests`
    - `cargo test -p codex-app-server --lib outgoing_message::tests`
    - `cargo test -p codex-app-server --lib in_process::tests`
    - `cargo test -p codex-app-server --test all v2::fs`
    - `cargo test -p codex-app-server --test all v2::config_rpc`
    - `cargo test -p codex-app-server --test all v2::external_agent_config`
    - `cargo test -p codex-app-server --test all v2::initialize`
    - `just fix -p codex-app-server`
    - `git diff --check`
    
    Note: full `cargo test -p codex-app-server` was attempted and stopped in
    `message_processor::tracing_tests::turn_start_jsonrpc_span_parents_core_turn_spans`
    with a stack overflow after unrelated tests had already passed.
  • permissions: make runtime config profile-backed (#19606)
    ## Why
    
    This supersedes #19391. During stack repair, GitHub marked #19391 as
    merged into a temporary stack branch rather than into `main`, so the
    runtime-config change needed a fresh PR.
    
    `PermissionProfile` is now the canonical permissions shape after #19231
    because it can distinguish `Managed`, `Disabled`, and `External`
    enforcement while also carrying filesystem rules that legacy
    `SandboxPolicy` cannot represent cleanly. Core config and session state
    still needed to accept profile-backed permissions without forcing every
    profile through the strict legacy bridge, which rejected valid runtime
    profiles such as direct write roots.
    
    The unrelated CI/test hardening that previously rode along with this PR
    has been split into #19683 so this PR stays focused on the permissions
    model migration.
    
    ## What Changed
    
    - Adds `Permissions.permission_profile` and
    `SessionConfiguration.permission_profile` as constrained runtime state,
    while keeping `sandbox_policy` as a legacy compatibility projection.
    - Introduces profile setters that keep `PermissionProfile`, split
    filesystem/network policies, and legacy `SandboxPolicy` projections
    synchronized.
    - Uses a compatibility projection for requirement checks and legacy
    consumers instead of rejecting profiles that cannot round-trip through
    `SandboxPolicy` exactly.
    - Updates config loading, config overrides, session updates, turn
    context plumbing, prompt permission text, sandbox tags, and exec request
    construction to carry profile-backed runtime permissions.
    - Preserves configured deny-read entries and `glob_scan_max_depth` when
    command/session profiles are narrowed.
    - Adds `PermissionProfile::read_only()` and
    `PermissionProfile::workspace_write()` presets that match legacy
    defaults.
    
    ## 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/19606).
    * #19395
    * #19394
    * #19393
    * #19392
    * __->__ #19606
  • permissions: remove legacy read-only access modes (#19449)
    ## Why
    
    `ReadOnlyAccess` was a transitional legacy shape on `SandboxPolicy`:
    `FullAccess` meant the historical read-only/workspace-write modes could
    read the full filesystem, while `Restricted` tried to carry partial
    readable roots. The partial-read model now belongs in
    `FileSystemSandboxPolicy` and `PermissionProfile`, so keeping it on
    `SandboxPolicy` makes every legacy projection reintroduce lossy
    read-root bookkeeping and creates unnecessary noise in the rest of the
    permissions migration.
    
    This PR makes the legacy policy model narrower and explicit:
    `SandboxPolicy::ReadOnly` and `SandboxPolicy::WorkspaceWrite` represent
    the old full-read sandbox modes only. Split readable roots, deny-read
    globs, and platform-default/minimal read behavior stay in the runtime
    permissions model.
    
    ## What changed
    
    - Removes `ReadOnlyAccess` from
    `codex_protocol::protocol::SandboxPolicy`, including the generated
    `access` and `readOnlyAccess` API fields.
    - Updates legacy policy/profile conversions so restricted filesystem
    reads are represented only by `FileSystemSandboxPolicy` /
    `PermissionProfile` entries.
    - Keeps app-server v2 compatible with legacy `fullAccess` read-access
    payloads by accepting and ignoring that no-op shape, while rejecting
    legacy `restricted` read-access payloads instead of silently widening
    them to full-read legacy policies.
    - Carries Windows sandbox platform-default read behavior with an
    explicit override flag instead of depending on
    `ReadOnlyAccess::Restricted`.
    - Refreshes generated app-server schema/types and updates tests/docs for
    the simplified legacy policy shape.
    
    ## Verification
    
    - `cargo check -p codex-app-server-protocol --tests`
    - `cargo check -p codex-windows-sandbox --tests`
    - `cargo test -p codex-app-server-protocol sandbox_policy_`
    
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/19449).
    * #19395
    * #19394
    * #19393
    * #19392
    * #19391
    * __->__ #19449
  • Use AbsolutePathBuf for exec cwd plumbing (#17063)
    ## Summary
    - Carry `AbsolutePathBuf` through tool cwd parsing/resolution instead of
    resolving workdirs to raw `PathBuf`s.
    - Type exec/sandbox request cwd fields as `AbsolutePathBuf` through
    `ExecParams`, `ExecRequest`, `SandboxCommand`, and unified exec runtime
    requests.
    - Keep `PathBuf` conversions at external/event boundaries and update
    existing tests/fixtures for the typed cwd.
    
    ## Validation
    - `cargo check -p codex-core --tests`
    - `cargo check -p codex-sandboxing --tests`
    - `cargo test -p codex-sandboxing`
    - `cargo test -p codex-core --lib tools::handlers::`
    - `just fix -p codex-sandboxing`
    - `just fix -p codex-core`
    - `just fmt`
    
    Full `codex-core` test suite was not run locally; per repo guidance I
    kept local validation targeted.
  • remove temporary ownership re-exports (#16626)
    Stacked on #16508.
    
    This removes the temporary `codex-core` / `codex-login` re-export shims
    from the ownership split and rewrites callsites to import directly from
    `codex-model-provider-info`, `codex-models-manager`, `codex-api`,
    `codex-protocol`, `codex-feedback`, and `codex-response-debug-context`.
    
    No behavior change intended; this is the mechanical import cleanup layer
    split out from the ownership move.
    
    ---------
    
    Co-authored-by: Codex <noreply@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
  • Drop sandbox_permissions from sandbox exec requests (#15665)
    ## Summary
    - drop `sandbox_permissions` from the sandboxing `ExecOptions` and
    `ExecRequest` adapter types
    - remove the now-unused plumbing from shell, unified exec, JS REPL, and
    apply-patch runtime call sites
    - default reconstructed `ExecParams` to `SandboxPermissions::UseDefault`
    where the lower-level API still requires the field
    
    ## Testing
    - `just fmt`
    - `just argument-comment-lint`
    - `cargo test -p codex-core` (still running locally; first failures
    observed in `suite::cli_stream::responses_mode_stream_cli`,
    `suite::cli_stream::responses_mode_stream_cli_supports_openai_base_url_config_override`,
    and
    `suite::cli_stream::responses_mode_stream_cli_supports_openai_base_url_env_fallback`)
  • app-server: Add back pressure and batching to command/exec (#15547)
    * Add
    `OutgoingMessageSender::send_server_notification_to_connection_and_wait`
    which returns only once message is written to websocket (or failed to do
    so)
    * Use this mechanism to apply back pressure to stdout/stderr streams of
    processes spawned by `command/exec`, to limit them to at most one
    message in-memory at a time
    * Use back pressure signal to also batch smaller chunks into ≈64KiB ones
    
    This should make commands execution more robust over
    high-latency/low-throughput networks
  • Apply argument comment lint across codex-rs (#14652)
    ## Why
    
    Once the repo-local lint exists, `codex-rs` needs to follow the
    checked-in convention and CI needs to keep it from drifting. This commit
    applies the fallback `/*param*/` style consistently across existing
    positional literal call sites without changing those APIs.
    
    The longer-term preference is still to avoid APIs that require comments
    by choosing clearer parameter types and call shapes. This PR is
    intentionally the mechanical follow-through for the places where the
    existing signatures stay in place.
    
    After rebasing onto newer `main`, the rollout also had to cover newly
    introduced `tui_app_server` call sites. That made it clear the first cut
    of the CI job was too expensive for the common path: it was spending
    almost as much time installing `cargo-dylint` and re-testing the lint
    crate as a representative test job spends running product tests. The CI
    update keeps the full workspace enforcement but trims that extra
    overhead from ordinary `codex-rs` PRs.
    
    ## What changed
    
    - keep a dedicated `argument_comment_lint` job in `rust-ci`
    - mechanically annotate remaining opaque positional literals across
    `codex-rs` with exact `/*param*/` comments, including the rebased
    `tui_app_server` call sites that now fall under the lint
    - keep the checked-in style aligned with the lint policy by using
    `/*param*/` and leaving string and char literals uncommented
    - cache `cargo-dylint`, `dylint-link`, and the relevant Cargo
    registry/git metadata in the lint job
    - split changed-path detection so the lint crate's own `cargo test` step
    runs only when `tools/argument-comment-lint/*` or `rust-ci.yml` changes
    - continue to run the repo wrapper over the `codex-rs` workspace, so
    product-code enforcement is unchanged
    
    Most of the code changes in this commit are intentionally mechanical
    comment rewrites or insertions driven by the lint itself.
    
    ## Verification
    
    - `./tools/argument-comment-lint/run.sh --workspace`
    - `cargo test -p codex-tui-app-server -p codex-tui`
    - parsed `.github/workflows/rust-ci.yml` locally with PyYAML
    
    ---
    
    * -> #14652
    * #14651
  • 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`
  • sandboxing: plumb split sandbox policies through runtime (#13439)
    ## Why
    
    `#13434` introduces split `FileSystemSandboxPolicy` and
    `NetworkSandboxPolicy`, but the runtime still made most execution-time
    sandbox decisions from the legacy `SandboxPolicy` projection.
    
    That projection loses information about combinations like unrestricted
    filesystem access with restricted network access. In practice, that
    means the runtime can choose the wrong platform sandbox behavior or set
    the wrong network-restriction environment for a command even when config
    has already separated those concerns.
    
    This PR carries the split policies through the runtime so sandbox
    selection, process spawning, and exec handling can consult the policy
    that actually matters.
    
    ## What changed
    
    - threaded `FileSystemSandboxPolicy` and `NetworkSandboxPolicy` through
    `TurnContext`, `ExecRequest`, sandbox attempts, shell escalation state,
    unified exec, and app-server exec overrides
    - updated sandbox selection in `core/src/sandboxing/mod.rs` and
    `core/src/exec.rs` to key off `FileSystemSandboxPolicy.kind` plus
    `NetworkSandboxPolicy`, rather than inferring behavior only from the
    legacy `SandboxPolicy`
    - updated process spawning in `core/src/spawn.rs` and the platform
    wrappers to use `NetworkSandboxPolicy` when deciding whether to set
    `CODEX_SANDBOX_NETWORK_DISABLED`
    - kept additional-permissions handling and legacy `ExternalSandbox`
    compatibility projections aligned with the split policies, including
    explicit user-shell execution and Windows restricted-token routing
    - updated callers across `core`, `app-server`, and `linux-sandbox` to
    pass the split policies explicitly
    
    ## Verification
    
    - added regression coverage in `core/tests/suite/user_shell_cmd.rs` to
    verify `RunUserShellCommand` does not inherit
    `CODEX_SANDBOX_NETWORK_DISABLED` from the active turn
    - added coverage in `core/src/exec.rs` for Windows restricted-token
    sandbox selection when the legacy projection is `ExternalSandbox`
    - updated Linux sandbox coverage in
    `linux-sandbox/tests/suite/landlock.rs` to exercise the split-policy
    exec path
    - verified the current PR state with `just clippy`
    
    
    
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/13439).
    * #13453
    * #13452
    * #13451
    * #13449
    * #13448
    * #13445
    * #13440
    * __->__ #13439
    
    ---------
    
    Co-authored-by: viyatb-oai <viyatb@openai.com>
  • app-server: Add streaming and tty/pty capabilities to command/exec (#13640)
    * Add an ability to stream stdin, stdout, and stderr
    * Streaming of stdout and stderr has a configurable cap for total amount
    of transmitted bytes (with an ability to disable it)
    * Add support for overriding environment variables
    * Add an ability to terminate running applications (using
    `command/exec/terminate`)
    * Add TTY/PTY support, with an ability to resize the terminal (using
    `command/exec/resize`)