Commit Graph

22 Commits

  • fix: refresh network proxy settings when sandbox mode changes (#17040)
    ## Summary
    
    Fix network proxy sessions so changing sandbox mode recomputes the
    effective managed network policy and applies it to the already-running
    per-session proxy.
    
    ## Root Cause
    
    `danger_full_access_denylist_only` injects `"*"` only while building the
    proxy spec for Full Access. Sessions built that spec once at startup, so
    a later permission switch to Full Access left the live proxy in its
    original restricted policy. Switching back needed the same recompute
    path to remove the synthetic wildcard again.
    
    ## What Changed
    
    - Preserve the original managed network proxy config/requirements so the
    effective spec can be recomputed for a new sandbox policy.
    - Refresh the current session proxy when sandbox settings change, then
    reapply exec-policy network overlays.
    - Add an in-place proxy state update path while rejecting
    listener/port/SOCKS changes that cannot be hot-reloaded.
    - Keep runtime proxy settings cheap to snapshot and update.
    - Add regression coverage for workspace-write -> Full Access ->
    workspace-write.
  • 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.
  • chore: refactor network permissions to use explicit domain and unix socket rule maps (#15120)
    ## Summary
    
    This PR replaces the legacy network allow/deny list model with explicit
    rule maps for domains and unix sockets across managed requirements,
    permissions profiles, the network proxy config, and the app server
    protocol.
    
    Concretely, it:
    
    - introduces typed domain (`allow` / `deny`) and unix socket permission
    (`allow` / `none`) entries instead of separate `allowed_domains`,
    `denied_domains`, and `allow_unix_sockets` lists
    - updates config loading, managed requirements merging, and exec-policy
    overlays to read and upsert rule entries consistently
    - exposes the new shape through protocol/schema outputs, debug surfaces,
    and app-server config APIs
    - rejects the legacy list-based keys and updates docs/tests to reflect
    the new config format
    
    ## Why
    
    The previous representation split related network policy across multiple
    parallel lists, which made merging and overriding rules harder to reason
    about. Moving to explicit keyed permission maps gives us a single source
    of truth per host/socket entry, makes allow/deny precedence clearer, and
    gives protocol consumers access to the full rule state instead of
    derived projections only.
    
    ## Backward Compatibility
    
    ### Backward compatible
    
    - Managed requirements still accept the legacy
    `experimental_network.allowed_domains`,
    `experimental_network.denied_domains`, and
    `experimental_network.allow_unix_sockets` fields. They are normalized
    into the new canonical `domains` and `unix_sockets` maps internally.
    - App-server v2 still deserializes legacy `allowedDomains`,
    `deniedDomains`, and `allowUnixSockets` payloads, so older clients can
    continue reading managed network requirements.
    - App-server v2 responses still populate `allowedDomains`,
    `deniedDomains`, and `allowUnixSockets` as legacy compatibility views
    derived from the canonical maps.
    - `managed_allowed_domains_only` keeps the same behavior after
    normalization. Legacy managed allowlists still participate in the same
    enforcement path as canonical `domains` entries.
    
    ### Not backward compatible
    
    - Permissions profiles under `[permissions.<profile>.network]` no longer
    accept the legacy list-based keys. Those configs must use the canonical
    `[domains]` and `[unix_sockets]` tables instead of `allowed_domains`,
    `denied_domains`, or `allow_unix_sockets`.
    - Managed `experimental_network` config cannot mix canonical and legacy
    forms in the same block. For example, `domains` cannot be combined with
    `allowed_domains` or `denied_domains`, and `unix_sockets` cannot be
    combined with `allow_unix_sockets`.
    - The canonical format can express explicit `"none"` entries for unix
    sockets, but those entries do not round-trip through the legacy
    compatibility fields because the legacy fields only represent allow/deny
    lists.
    ## Testing
    `/target/debug/codex sandbox macos --log-denials /bin/zsh -c 'curl
    https://www.example.com' ` gives 200 with config
    ```
    [permissions.workspace.network.domains]
    "www.example.com" = "allow"
    ```
    and fails when set to deny: `curl: (56) CONNECT tunnel failed, response
    403`.
    
    Also tested backward compatibility path by verifying that adding the
    following to `/etc/codex/requirements.toml` works:
    ```
    [experimental_network]
    allowed_domains = ["www.example.com"]
    ```
  • fix(network-proxy): fail closed on network-proxy DNS lookup errors (#15909)
    ## Summary
    
    Fail closed when the network proxy's local/private IP pre-check hits a
    DNS lookup error or timeout, instead of treating the hostname as public
    and allowing the request.
    
    ## Root cause
    
    `host_resolves_to_non_public_ip()` returned `false` on resolver failure,
    which created a fail-open path in the `allow_local_binding = false`
    boundary. The eventual connect path performs its own DNS resolution
    later, so a transient pre-check failure is not evidence that the
    destination is public.
    
    ## Changes
    
    - Treat DNS lookup errors/timeouts as local/private for blocking
    purposes
    - Add a regression test for an allowlisted hostname that fails DNS
    resolution
    
    ## Validation
    
    - `cargo test -p codex-network-proxy`
    - `cargo clippy -p codex-network-proxy --all-targets -- -D warnings`
    - `just fmt`
    - `just argument-comment-lint`
  • Allow global network allowlist wildcard (#15549)
    ## Problem
    
    Today `codex-network-proxy` rejects a global `*` in
    `network.allowed_domains`, so there is no static way to configure a
    denylist-only posture for public hosts. Users have to enumerate broad
    allowlist patterns instead.
    
    ## Approach
    
    - Make global wildcard acceptance field-specific: `allowed_domains` can
    use `*`, while `denied_domains` still rejects a global wildcard.
    - Keep the existing evaluation order, so explicit denies still win first
    and local/private protections still apply unless separately enabled.
    - Add coverage for the denylist-only behavior and update the README to
    document it.
    
    ## Validation
    
    - `just fmt`
    - `cargo test -p codex-network-proxy` (full run had one unrelated flaky
    telemetry test:
    `network_policy::tests::emit_block_decision_audit_event_emits_non_domain_event`;
    reran in isolation and it passed)
    - `cargo test -p codex-network-proxy
    network_policy::tests::emit_block_decision_audit_event_emits_non_domain_event
    -- --exact --nocapture`
    - `just fix -p codex-network-proxy`
    - `just argument-comment-lint`
  • 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
  • fix: support managed network allowlist controls (#12752)
    ## Summary
    - treat `requirements.toml` `allowed_domains` and `denied_domains` as
    managed network baselines for the proxy
    - in restricted modes by default, build the effective runtime policy
    from the managed baseline plus user-configured allowlist and denylist
    entries, so common hosts can be pre-approved without blocking later user
    expansion
    - add `experimental_network.managed_allowed_domains_only = true` to pin
    the effective allowlist to managed entries, ignore user allowlist
    additions, and hard-deny non-managed domains without prompting
    - apply `managed_allowed_domains_only` anywhere managed network
    enforcement is active, including full access, while continuing to
    respect denied domains from all sources
    - add regression coverage for merged-baseline behavior, managed-only
    behavior, and full-access managed-only enforcement
    
    ## Behavior
    Assuming `requirements.toml` defines both
    `experimental_network.allowed_domains` and
    `experimental_network.denied_domains`.
    
    ### Default mode
    - By default, the effective allowlist is
    `experimental_network.allowed_domains` plus user or persisted allowlist
    additions.
    - By default, the effective denylist is
    `experimental_network.denied_domains` plus user or persisted denylist
    additions.
    - Allowlist misses can go through the network approval flow.
    - Explicit denylist hits and local or private-network blocks are still
    hard-denied.
    - When `experimental_network.managed_allowed_domains_only = true`, only
    managed `allowed_domains` are respected, user allowlist additions are
    ignored, and non-managed domains are hard-denied without prompting.
    - Denied domains continue to be respected from all sources.
    
    ### Full access
    - With managed requirements present, the effective allowlist is pinned
    to `experimental_network.allowed_domains`.
    - With managed requirements present, the effective denylist is pinned to
    `experimental_network.denied_domains`.
    - There is no allowlist-miss approval path in full access.
    - Explicit denylist hits are hard-denied.
    - `experimental_network.managed_allowed_domains_only = true` now also
    applies in full access, so managed-only behavior remains in effect
    anywhere managed network enforcement is active.
  • fix: reject global wildcard network proxy domains (#13789)
    ## Summary
    - reject the global `*` domain pattern in proxy allow/deny lists and
    managed constraints introduced for testing earlier
    - keep exact hosts plus scoped wildcards like `*.example.com` and
    `**.example.com`
    - update docs and regression tests for the new invalid-config behavior
  • refactor: remove proxy admin endpoint (#13687)
    ## Summary
    - delete the network proxy admin server and its runtime listener/task
    plumbing
    - remove the admin endpoint config, runtime, requirement, protocol,
    schema, and debug-surface fields
    - update proxy docs to reflect the remaining HTTP and SOCKS listeners
    only
  • feat(network-proxy): add embedded OTEL policy audit logging (#12046)
    **PR Summary**
    
    This PR adds embedded-only OTEL policy audit logging for
    `codex-network-proxy` and threads audit metadata from `codex-core` into
    managed proxy startup.
    
    ### What changed
    - Added structured audit event emission in `network_policy.rs` with
    target `codex_otel.network_proxy`.
    - Emitted:
    - `codex.network_proxy.domain_policy_decision` once per domain-policy
    evaluation.
      - `codex.network_proxy.block_decision` for non-domain denies.
    - Added required policy/network fields, RFC3339 UTC millisecond
    `event.timestamp`, and fallback defaults (`http.request.method="none"`,
    `client.address="unknown"`).
    - Added non-domain deny audit emission in HTTP/SOCKS handlers for
    mode-guard and proxy-state denies, including unix-socket deny paths.
    - Added `REASON_UNIX_SOCKET_UNSUPPORTED` and used it for unsupported
    unix-socket auditing.
    - Added `NetworkProxyAuditMetadata` to runtime/state, re-exported from
    `lib.rs` and `state.rs`.
    - Added `start_proxy_with_audit_metadata(...)` in core config, with
    `start_proxy()` delegating to default metadata.
    - Wired metadata construction in `codex.rs` from session/auth context,
    including originator sanitization for OTEL-safe tagging.
    - Updated `network-proxy/README.md` with embedded-mode audit schema and
    behavior notes.
    - Refactored HTTP block-audit emission to a small local helper to reduce
    duplication.
    - Preserved existing unix-socket proxy-disabled host/path behavior for
    responses and blocked history while using an audit-only endpoint
    override (`server.address="unix-socket"`, `server.port=0`).
    
    ### Explicit exclusions
    - No standalone proxy OTEL startup work.
    - No `main.rs` binary wiring.
    - No `standalone_otel.rs`.
    - No standalone docs/tests.
    
    ### Tests
    - Extended `network_policy.rs` tests for event mapping, metadata
    propagation, fallbacks, timestamp format, and target prefix.
    - Extended HTTP tests to assert unix-socket deny block audit events.
    - Extended SOCKS tests to cover deny emission from handler deny
    branches.
    - Added/updated core tests to verify audit metadata threading into
    managed proxy state.
    
    ### Validation run
    - `just fmt`
    - `cargo test -p codex-network-proxy` 
    - `cargo test -p codex-core` ran with one unrelated flaky timeout
    (`shell_snapshot::tests::snapshot_shell_does_not_inherit_stdin`), and
    the test passed when rerun directly 
    
    ---------
    
    Co-authored-by: viyatb-oai <viyatb@openai.com>
  • feat(network-proxy): add MITM support and gate limited-mode CONNECT (#9859)
    ## Description
    - Adds MITM support (CA load/issue, TLS termination, optional body
    inspection).
    - Adds `codex-network-proxy init` to create
    `CODEX_HOME/network_proxy/mitm`.
    - Enforces limited-mode HTTPS correctly: `CONNECT` requires MITM,
    otherwise blocked with `mitm_required`.
    - Keeps `origin/main` layering/reload semantics (managed layers included
    in reload checks).
    - Centralizes block reasons (`REASON_MITM_REQUIRED`) and removes
    `println!`.
    - Scope is MITM-only (no SOCKS changes).
    
    gated by `mitm=false` (default)
  • feat(core): persist network approvals in execpolicy (#12357)
    ## Summary
    Persist network approval allow/deny decisions as `network_rule(...)`
    entries in execpolicy (not proxy config)
    
    It adds `network_rule` parsing + append support in `codex-execpolicy`,
    including `decision="prompt"` (parse-only; not compiled into proxy
    allow/deny lists)
    - compile execpolicy network rules into proxy allow/deny lists and
    update the live proxy state on approval
    - preserve requirements execpolicy `network_rule(...)` entries when
    merging with file-based execpolicy
    - reject broad wildcard hosts (for example `*`) for persisted
    `network_rule(...)`
  • fix(network-proxy): add unix socket allow-all and update seatbelt rules (#11368)
    ## Summary
    Adds support for a Unix socket escape hatch so we can bypass socket
    allowlisting when explicitly enabled.
    
    ## Description
    * added a new flag, `network.dangerously_allow_all_unix_sockets` as an
    explicit escape hatch
    * In codex-network-proxy, enabling that flag now allows any absolute
    Unix socket path from x-unix-socket instead of requiring each path to be
    explicitly allowlisted. Relative paths are still rejected.
    * updated the macOS seatbelt path in core so it enforces the same Unix
    socket behavior:
      * allowlisted sockets generate explicit network* subpath rules
      * allow-all generates a broad network* (subpath "/") rule
    
    ---------
    
    Co-authored-by: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
  • Refactor network approvals to host/protocol/port scope (#12140)
    ## Summary
    Simplify network approvals by removing per-attempt proxy correlation and
    moving to session-level approval dedupe keyed by (host, protocol, port).
    Instead of encoding attempt IDs into proxy credentials/URLs, we now
    treat approvals as a destination policy decision.
    
    - Concurrent calls to the same destination share one approval prompt.
    - Different destinations (or same host on different ports) get separate
    prompts.
    - Allow once approves the current queued request group only.
    - Allow for session caches that (host, protocol, port) and auto-allows
    future matching requests.
    - Never policy continues to deny without prompting.
    
    Example:
    - 3 calls: 
      - a.com (line 443)
      - b.com (line 443)
      - a.com (line 443)
    => 2 prompts total (a, b), second a waits on the first decision.
    - a.com:80 is treated separately from a.com line 443
    
    ## Testing
    - `just fmt` (in `codex-rs`)
    - `cargo test -p codex-core tools::network_approval::tests`
    - `cargo test -p codex-core` (unit tests pass; existing
    integration-suite failures remain in this environment)
  • feat(core): add structured network approval plumbing and policy decision model (#11672)
    ### Description
    #### Summary
    Introduces the core plumbing required for structured network approvals
    
    #### What changed
    - Added structured network policy decision modeling in core.
    - Added approval payload/context types needed for network approval
    semantics.
    - Wired shell/unified-exec runtime plumbing to consume structured
    decisions.
    - Updated related core error/event surfaces for structured handling.
    - Updated protocol plumbing used by core approval flow.
    - Included small CLI debug sandbox compatibility updates needed by this
    layer.
    
    #### Why
    establishes the minimal backend foundation for network approvals without
    yet changing high-level orchestration or TUI behavior.
    
    #### Notes
    - Behavior remains constrained by existing requirements/config gating.
    - Follow-up PRs in the stack handle orchestration, UX, and app-server
    integration.
    
    ---------
    
    Co-authored-by: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
  • feat(network-proxy): structured policy signaling and attempt correlation to core (#11662)
    ## Summary
    When network requests were blocked, downstream code often had to infer
    ask vs deny from free-form response text. That was brittle and led to
    incorrect approval behavior.
    This PR fixes the proxy side so blocked decisions are structured and
    request metadata survives reliably.
    
    ## Description
    - Blocked proxy responses now carry consistent structured policy
    decision data.
    - Request attempt metadata is preserved across proxy env paths
    (including ALL_PROXY flows).
    - Header stripping was tightened so we still remove unsafe forwarding
    headers, but keep metadata needed for policy handling.
    - Block messages were clarified (for example, allowlist miss vs explicit
    deny).
    - Added unified violation log entries so policy failures can be
    inspected in one place.
    - Added/updated tests for these behaviors.
    
    ---------
    
    Co-authored-by: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
  • chore: change ConfigState so it no longer depends on a single config.toml file for reloading (#11262)
    If anything, it should depend on `ConfigLayerStack`.
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/11262).
    * #11207
    * __->__ #11262
  • chore: refactor network-proxy so that ConfigReloader is injectable behavior (#11114)
    Currently, `codex-network-proxy` depends on `codex-core`, but this
    should be the other way around. As a first step, refactor out
    `ConfigReloader`, which should make it easier to move
    `codex-rs/network-proxy/src/state.rs` to `codex-core` in a subsequent
    commit.
  • refactor(network-proxy): flatten network config under [network] (#10965)
    Summary:
    - Rename config table from network_proxy to network.
    - Flatten allowed_domains, denied_domains, allow_unix_sockets, and
    allow_local_binding onto NetworkProxySettings.
    - Update runtime, state constraints, tests, and README to the new config
    shape.
  • chore: introduce *Args types for new() methods (#10009)
    Constructors with long param lists can be hard to reason about when a
    number of the args are `None`, in practice. Introducing a struct to use
    as the args type helps make things more self-documenting.
  • feat: introducing a network sandbox proxy (#8442)
    This add a new crate, `codex-network-proxy`, a local network proxy
    service used by Codex to enforce fine-grained network policy (domain
    allow/deny) and to surface blocked network events for interactive
    approvals.
    
    - New crate: `codex-rs/network-proxy/` (`codex-network-proxy` binary +
    library)
    - Core capabilities:
      - HTTP proxy support (including CONNECT tunneling)
      - SOCKS5 proxy support (in the later PR)
    - policy evaluation (allowed/denied domain lists; denylist wins;
    wildcard support)
      - small admin API for polling/reload/mode changes
    - optional MITM support for HTTPS CONNECT to enforce “limited mode”
    method restrictions (later PR)
    
    Will follow up integration with codex in subsequent PRs.
    
    ## Testing
    
    - `cd codex-rs && cargo build -p codex-network-proxy`
    - `cd codex-rs && cargo run -p codex-network-proxy -- proxy`