Commit Graph

12 Commits

  • 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"]
    ```
  • 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.
  • 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 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)
  • 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>
  • Enable SOCKS defaults for common local network proxy use cases (#11362)
    ## Summary
    - enable local-use defaults in network proxy settings: SOCKS5 on, SOCKS5
    UDP on, upstream proxying on, and local binding on
    - add a regression test that asserts the full
    `NetworkProxySettings::default()` baseline
    - Fixed managed listener reservation behavior.
    Before: we always reserved a loopback SOCKS listener, even when
    enable_socks5 = false.
    Now: SOCKS listener is only reserved when SOCKS is enabled.
    - Fixed /debug-config env output for SOCKS-disabled sessions.
    ALL_PROXY now shows the HTTP proxy URL when SOCKS is disabled (instead
    of incorrectly showing socks5h://...).
    
    
    ## Validation
    - just fmt
    - cargo test -p codex-network-proxy
    - cargo clippy -p codex-network-proxy --all-targets
  • feat: retain NetworkProxy, when appropriate (#11207)
    As of this PR, `SessionServices` retains a
    `Option<StartedNetworkProxy>`, if appropriate.
    
    Now the `network` field on `Config` is `Option<NetworkProxySpec>`
    instead of `Option<NetworkProxy>`.
    
    Over in `Session::new()`, we invoke `NetworkProxySpec::start_proxy()` to
    create the `StartedNetworkProxy`, which is a new struct that retains the
    `NetworkProxy` as well as the `NetworkProxyHandle`. (Note that `Drop` is
    implemented for `NetworkProxyHandle` to ensure the proxies are shutdown
    when it is dropped.)
    
    The `NetworkProxy` from the `StartedNetworkProxy` is threaded through to
    the appropriate places.
    
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/11207).
    * #11285
    * __->__ #11207
  • 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.
  • 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
  • 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`