Commit Graph

15 Commits

  • Scope network approvals by environment (#28899)
    Stacked on #28766.
    
    ## Why
    
    Network approvals are environment-scoped: allowing a host in one
    execution environment should not allow the same host in another
    environment.
    
    #28766 adds the inert IDs and constructor plumbing. This PR applies the
    behavior on top.
    
    ## What changed
    
    - Route managed network traffic through per-environment HTTP and SOCKS
    proxy listeners.
    - Stamp HTTP, HTTPS CONNECT, SOCKS TCP, and SOCKS UDP policy requests
    with the source environment at the proxy boundary.
    - Carry the selected execution environment through shell, unified exec,
    zsh-fork, and sandbox transform paths.
    - Include the environment in pending, approved-for-session, and
    denied-for-session network approval cache keys.
    - Include the environment in approval IDs and approval prompts.
    - Preserve legacy fallback for unattributed requests, but deny when
    active-call attribution is ambiguous.
    - Fail closed if an environment-specific proxy endpoint cannot be
    prepared.
    
    ## Validation
    
    - just fmt
    - CI will run tests and clippy
  • Add network environment ID plumbing (#28766)
    ## Why
    
    Prepare network approval scoping to distinguish execution environments
    without changing behavior yet.
    
    ## What changed
    
    - Add optional environment IDs to network policy requests.
    - Add optional network environment IDs to exec and sandbox request
    structs.
    - Thread default None values through existing construction points.
    - Fix stale constructor call sites that caused the CI compile failures.
    
    ## Not included
    
    - Per-environment proxy listeners.
    - Network approval cache or prompt behavior changes.
    - Ambiguous request attribution handling.
    
    Those behavior changes moved to stacked follow-up #28899.
    
    ## Validation
    
    - just fmt
    - CI will run tests and clippy
  • [codex] Remove async_trait from first-party code (#27475)
    ## Why
    
    First-party async traits should expose their `Send` contracts explicitly
    without requiring `async_trait`. This completes the migration pattern
    established in #27303 and #27304.
    
    ## What changed
    
    - Replaced the remaining first-party `async_trait` traits with native
    return-position `impl Future + Send` where statically dispatched and
    explicit boxed `Send` futures where object safety is required.
    - Kept implementations behavior-preserving, outlining existing async
    bodies into inherent methods where that keeps the diff reviewable.
    - Removed all direct first-party `async-trait` dependencies and the
    workspace dependency declaration.
    - Added a cargo-deny policy that permits `async-trait` only through the
    remaining transitive wrapper crates.
    - Updated `rand` from 0.8.5 to 0.8.6 to resolve RUSTSEC-2026-0097 and
    keep the full cargo-deny check passing.
    
    ## Validation
    
    - `just test -p codex-exec-server`: 216 passed, 2 skipped.
    - `just test -p codex-model-provider`: 39 passed.
    - `just test -p codex-core` and `just test`: changed tests passed;
    remaining failures are environment-sensitive suites unrelated to this
    migration.
    - `cargo deny check`
    - `just fix`
    - `just fmt`
    - `cargo shear`
    - `just bazel-lock-check`
  • Add SOCKS5 TCP MITM coverage (#22685)
    ## Summary
    - reuse the MITM HTTPS serving path for raw SOCKS5 TCP streams
    - route limited-mode and hooked SOCKS5 TCP requests through MITM before
    dialing upstream
    - keep SOCKS5 UDP limited-mode behavior unchanged
    
    ## Validation
    - `just fmt`
    - `just test -p codex-network-proxy`
    - `just fix -p codex-network-proxy`
    - `git diff --check`
  • Wire MITM hooks into runtime enforcement (#20659)
    ## Stack
    1. Parent PR: #18868 adds MITM hook config and model only.
    2. This PR wires runtime enforcement.
    3. User facing config follow up: #18240 moves MITM policy into the
    PermissionProfile network tree.
    
    ## Why
    1. After the hook model exists, the proxy needs a separate behavior
    change that can be tested at the request path.
    2. This PR makes hooked HTTPS hosts require MITM, evaluates inner
    requests after CONNECT, mutates headers for matching hooks, and blocks
    hooked hosts when no hook matches.
    3. It also fixes the activation path so a permission profile with MITM
    hook policy starts the managed proxy.
    4. Keeping this separate from #18868 lets reviewers focus on runtime
    effects, telemetry, and request mutation.
    
    ## Summary
    1. Store compiled MITM hooks in network proxy state.
    2. Require MITM for hooked hosts even when network mode is full.
    3. Evaluate inner HTTPS requests against host specific hooks.
    4. Apply hook actions by replacing request headers before forwarding.
    5. Block hooked hosts when no hook matches and record block telemetry.
    6. Treat profile MITM hook policy as managed proxy policy so the proxy
    starts when needed.
    7. Keep the duplicate authorization header replacement and query
    preserving request rebuild in this runtime PR.
    8. Add runtime tests and README guidance for hook enforcement.
    
    ## Validation
    1. Ran the network proxy MITM policy tests.
    2. Ran the hooked host CONNECT test.
    3. Ran the authorization header replacement test.
    4. Ran the core permission profile proxy activation test for MITM hooks.
    5. Ran the scoped Clippy fixer for the network proxy crate.
    6. Ran the scoped Clippy fixer for the core crate.
  • chore: add minimal proxy egress diagnostics (#21220)
    ## Why
    Recent Auto Review reports show Git traffic hanging through the local
    proxy on both SSH and HTTPS paths. Today the support bundle does not
    make it obvious whether a request is stuck before upstream dialing,
    during the proxy hop, or after the upstream response begins, which slows
    down root-cause triage.
    
    This adds a small amount of runtime visibility at the existing proxy
    boundaries without changing routing or policy behavior.
    
    ## What changed
    - log whether HTTP and CONNECT traffic take the direct or upstream-proxy
    route
    - log start / success / failure timings for CONNECT, HTTP, and SOCKS5
    upstream dials
    - log CONNECT forwarding lifecycle events
    - describe HTTP success at the response-header boundary that is actually
    observed, rather than implying the full body finished
    
    ## Verification
    - `cargo test -p codex-network-proxy`
    - `cargo clippy -p codex-network-proxy --all-targets -- -D warnings`
  • fix(network-proxy): recheck network proxy connect targets (#19999)
    ## Why
    The proxy checks the requested host before opening the upstream
    connection, but DNS can resolve an allowed hostname to a loopback,
    private, or other non-public address after that first decision. Without
    a final check on the actual socket target, a request that looks
    acceptable at the hostname layer can still connect to a local service
    once resolution completes.
    
    ## What changed
    - add a shared TCP connector check for direct proxy egress
    - use that path for HTTP, `CONNECT`, SOCKS5, and MITM upstream
    connections
    - keep configured upstream proxy hops on the existing proxy path
    - add direct-connector coverage for allowed and rejected local targets
    
    ## Security impact
    Direct proxy egress now rechecks the resolved socket address before
    connecting, closing the gap between hostname policy evaluation and the
    final network target.
    
    ## Verification
    - `cargo test -p codex-network-proxy`
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • 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.
  • 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>
  • 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(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>
  • feat: reserve loopback ephemeral listeners for managed proxy (#11269)
    Codex may run many per-thread proxy instances, so hardcoded proxy ports
    are brittle and conflict-prone. The previous "ephemeral" approach still
    had a race: `build()` read `local_addr()` from temporary listeners and
    dropped them before `run()` rebound the ports. That left a
    [TOCTOU](https://en.wikipedia.org/wiki/Time-of-check_to_time-of-use)
    window where the OS (or another process) could reuse the same port,
    causing intermittent `EADDRINUSE` and partial proxy startup.
    
    Change the managed proxy path to reserve real listener sockets up front
    and keep them alive until startup:
    
    - add `ReservedListeners` on `NetworkProxy` to hold HTTP/SOCKS/admin std
    listeners allocated during `build()`
    - in managed mode, bind `127.0.0.1:0` for each listener and carry those
    bound sockets into `run()` instead of rebinding by address later
    - add `run_*_with_std_listener` entry points for HTTP, SOCKS5, and admin
    servers so `run()` can start services from already-reserved sockets
    - keep static/configured ports only when `managed_by_codex(false)`,
    including explicit `socks_addr` override support
    - remove fallback synthetic port allocation and add tests for managed
    ephemeral loopback binding and unmanaged configured-port behavior
    
    This makes managed startup deterministic, avoids port collisions, and
    preserves the intended distinction between Codex-managed ephemeral ports
    and externally managed fixed ports.
  • feat(network-proxy): add structured policy decision to blocked errors (#10420)
    ## Summary
    Add explicit, model-visible network policy decision metadata to blocked
    proxy responses/errors.
    
    Introduces a standardized prefix line: `CODEX_NETWORK_POLICY_DECISION
    {json}`
    
    and wires it through blocked paths for:
    - HTTP requests
    - HTTPS CONNECT
    - SOCKS5 TCP/UDP denials
    
    ## Why
    The model should see *why* a request was blocked
    (reason/source/protocol/host/port) so it can choose the correct next
    action.
    
    ## Notes
    - This PR is intentionally independent of config-layering/network-rule
    runtime integration.
    - Focus is blocked decision surface only.
  • 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(network-proxy): add a SOCKS5 proxy with policy enforcement (#9803)
    ### Summary
    - Adds an optional SOCKS5 listener via `rama-socks5`
    - SOCKS5 is disabled by default and gated by config
    - Reuses existing policy enforcement and blocked-request recording
    - Blocks SOCKS5 in limited mode to prevent method-policy bypass
    - Applies bind clamping to the SOCKS5 listener
    
    ### Config
    New/used fields under `network_proxy`:
    - `enable_socks5`
    - `socks_url`
    - `enable_socks5_udp`
    
    ### Scope
    - Changes limited to `codex-rs/network-proxy` (+ `codex-rs/Cargo.lock`)
    
    ### Testing
    ```bash
    cd codex-rs
    just fmt
    cargo test -p codex-network-proxy --offline