Commit Graph
11 Commits
Author SHA1 Message Date
Winston Howes 634ebc1865 Support credential brokering in plaintext HTTP tunnels (#44089)
## Why

Configured HTTP credential destinations need dummy credentials translated inside `CONNECT` and SOCKS5 tunnels, where credential interception previously detected only TLS.

## What changed

- Detect plaintext HTTP for configured credential destinations and proxy requests with URL-scoped credential substitution.
- Keep requests bound to the authorized tunnel destination, rejecting mismatched authorities and nested `CONNECT` requests.
- Support HTTP/2 clients and preserve HTTP upgrades, including `h2c`, and opaque traffic in full mode.
- Enforce limited-mode method restrictions on plaintext HTTP and reject upgrades and opaque traffic.

## Testing

Add end-to-end tests for both tunnel transports covering credential URL scope, request bodies, HTTP/2, destination mismatches, upgrades, lossless opaque forwarding, and limited-mode enforcement.

GitOrigin-RevId: 89262c996ea4ee156c628f9ed8e72b52504b2f6b
2026-09-09 07:06:12 +00:00
Winston Howes 56d3e8192f Refactor credential-broker tunnel protocol detection (#44077)
## What changed

Move TLS prefix detection into `brokered_tunnel` and carry explicit `BrokeredProtocols` requirements from the credential broker through HTTP `CONNECT` and SOCKS5 tunnel handling. Dispatch on `TunnelProtocol::Tls` or `TunnelProtocol::Opaque`, preserving the initial-read timeout, fragmented TLS header detection, and replay of all inspected bytes.

## Testing

Move the fragmented TLS regression test alongside the detector, retaining coverage for delayed header fragments and lossless prefix replay. Update broker and proxy assertions for the protocol-aware types.

GitOrigin-RevId: 6e5a92d8bdf6db5b59cff7f210c17b01d93ee23b
2026-09-09 06:26:36 +00:00
kevinlin-openai 7a0e974e08 Harden network proxy MITM authorization (#37211)
## Why

MITM hooks authorize requests before the upstream server parses them. Paths that
can be decoded or normalized to a different resource must not match an allowed
path, and hosts that require MITM inspection must not bypass it through the
plain HTTP proxy path.

## What changed

- Reject ambiguous hook paths, including traversal segments, backslashes,
  malformed percent encodings, and encoded separators or percent signs.
- Block plain HTTP proxy requests for hosts whose policy always requires MITM,
  recording the decision as `mitm_required`.

## Testing

- Cover safe and ambiguous path forms, encoded traversal through repository
  allowlists, and absolute-form HTTPS requests sent to the HTTP proxy.

GitOrigin-RevId: 8812a980ac64a97cbac3a237376d29be5ded9220
2026-08-06 03:57:10 +00:00
jif cf6d8ad921 Remove the network proxy config wrapper (#31767)
## Why

`NetworkProxyConfig` only wrapped `NetworkProxySettings` in a single
`network` field. That extra level made runtime callers repeat `.network`
everywhere without representing a real boundary.

## What changed

- move the managed-network fields directly onto `NetworkProxyConfig`
- collapse the matching partial-config wrapper
- update runtime callers and tests to use the direct fields
- keep the user-facing permissions/profile TOML layout unchanged

The internal serialized shape now matches the runtime type itself. This
does not change managed-network behavior or the `config.toml` shape.
2026-07-09 17:33:37 +01:00
989f55defa feat(network-proxy): experimental local credential broker (#28034)
## Why

Codex child processes can inherit injectable local credentials directly,
which lets commands read and exfiltrate the real values. This
experimental slice keeps supported workflows working while moving those
credentials behind the managed network proxy.

This PR contains only the proxy-owned broker implementation. The Codex
config and runtime integration is stacked separately in #29752.

## What changed

- discover supported credentials during child setup, retain real values
only in the in-memory proxy broker, and replace them with shaped dummy
values
- require a presented dummy to select a stored credential and preserve
unrelated explicit authorization headers
- bind GitHub cloud, GitHub Enterprise, and OpenAI credentials to their
intended hosts
- inject credentials only into TLS traffic by default; plaintext
injection requires the explicit dangerous opt-in
- use TLS ClientHello routing for CONNECT so non-TLS protocols remain
opaque tunnels
- expose a pure API that identifies environment keys still holding
broker-generated dummies without mutating the caller's environment

## Scope

- supported credentials: `GH_TOKEN`, `GITHUB_TOKEN`,
`GH_ENTERPRISE_TOKEN`, `GITHUB_ENTERPRISE_TOKEN`, and `OPENAI_API_KEY`
- GitHub cloud credentials match `github.com`, `api.github.com`, and
`*.ghe.com`
- GitHub Enterprise credentials match only the normalized non-cloud
`GH_HOST`
- OpenAI API keys match only `api.openai.com`
- this does not cover SSH agents, kube client certificates, filesystem
secret discovery, or context-injected secret scrubbing

## Validation

- `just test -p codex-network-proxy` (191 passed)
- focused opaque CONNECT, plaintext opt-in, dummy-selection, and
child-isolation regressions passed
- scoped Clippy check for `codex-network-proxy` passed

---------

Co-authored-by: viyatb-oai <viyatb@openai.com>
Co-authored-by: Codex <noreply@openai.com>
2026-06-24 13:21:16 -07:00
evawong-oaiandWinston Howes 3cae84009a Use named MITM permissions config (#18240)
## Stack
1. Parent PR: #18868 adds MITM hook config and model only.
2. Parent PR: #20659 wires hook enforcement into the proxy request path.
3. This PR changes the user facing PermissionProfile TOML shape.

## Why
1. The broader goal is to make MITM clamping usable from the same
permission profile that already controls network behavior.
2. This PR is the config UX layer for the stack. It moves MITM policy
into `[permissions.<profile>.network.mitm]` instead of exposing the flat
runtime shape to users.
3. The named hook and action tables belong here because users need
reusable policy blocks that are easy to review, while the proxy runtime
only needs a flat hook list.
4. This PR validates action refs during config parsing so mistakes in
the user facing policy fail before a proxy session starts.
5. Keeping the lowering here lets the proxy keep its simpler runtime
model and lets PermissionProfile remain the single source of network
permission policy.

## Summary
1. Keep MITM policy inside `[permissions.<profile>.network.mitm]` so the
selected PermissionProfile owns network proxy policy.
2. Use named MITM hooks under
`[permissions.<profile>.network.mitm.hooks.<name>]`.
3. Put host, methods, path prefixes, query, headers, body, and action
refs on the hook table.
4. Define reusable action blocks under
`[permissions.<profile>.network.mitm.actions.<name>]`.
5. Represent action blocks with `NetworkMitmActionToml`, then lower them
into the proxy runtime action config.
6. Reject unknown refs, empty refs, and empty action blocks during
config parsing.
7. Keep the runtime hook model unchanged by lowering config into the
existing proxy hook list.
8. Preserve the #20659 activation fix for nested MITM policy.

## Example
```toml
[permissions.workspace.network.mitm]
enabled = true

[permissions.workspace.network.mitm.hooks.github_write]
host = "api.github.com"
methods = ["POST", "PUT"]
path_prefixes = ["/repos/openai/"]
action = ["strip_auth"]

[permissions.workspace.network.mitm.actions.strip_auth]
strip_request_headers = ["authorization"]
```

## Validation
1. Regenerated the config schema.
2. Ran the core MITM config parsing and validation tests.
3. Ran the core PermissionProfile MITM proxy activation tests.
4. Ran the core config schema fixture test.
5. Ran the network proxy MITM policy tests.
6. Ran the scoped Clippy fixer for the network proxy crate.
7. Ran the scoped Clippy fixer for the core crate.

---------

Co-authored-by: Winston Howes <winston@openai.com>
2026-05-20 17:10:37 -07:00
evawong-oai f6970214d2 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.
2026-05-20 14:08:14 -07:00
Michael Bolin 61dfe0b86c 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.
2026-03-27 19:00:44 -07:00
Celia Chen dd30c8eedd 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"]
```
2026-03-27 06:17:59 +00:00
viyatb-oai 9a4787c240 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
2026-03-06 21:06:24 +00:00
viyatb-oai 8d3d58f992 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)
2026-02-24 18:15:15 +00:00