Skip to content

Add MITM hook config model#18868

Open
evawong-oai wants to merge 3 commits into
mainfrom
codex/mitm-proxy-landing
Open

Add MITM hook config model#18868
evawong-oai wants to merge 3 commits into
mainfrom
codex/mitm-proxy-landing

Conversation

@evawong-oai
Copy link
Copy Markdown
Contributor

@evawong-oai evawong-oai commented Apr 21, 2026

Stack

  1. This PR adds MITM hook config and model only.
  2. Runtime follow up: Wire MITM hooks into runtime enforcement #20659 wires hook enforcement into the proxy request path.
  3. User facing config follow up: Use named MITM permissions config #18240 moves MITM policy into the PermissionProfile network tree.

Why

  1. Viyat asked for the original parent PR to be split so reviewers can inspect the policy model before request behavior changes.
  2. This PR gives the proxy a typed MITM hook model, validation, matcher compilation, permissions TOML plumbing, schema support, and config tests.
  3. This PR deliberately does not change CONNECT or MITM request handling.
  4. Keeping runtime behavior out of this PR makes the review boundary simple: does the policy model parse, validate, compile, and lower correctly.

Summary

  1. Add the MITM hook config model and matcher compilation.
  2. Validate hosts, methods, paths, query matchers, header matchers, secret sources, and reserved body matching.
  3. Add wildcard matcher support for path, query value, and header value matching.
  4. Add permissions TOML and schema support for flat runtime hook config.
  5. Add config loader tests for MITM hook overlay behavior.

Validation

  1. Regenerated the config schema.
  2. Ran the network proxy MITM hook unit tests.
  3. Ran the core permission profile MITM hook parsing tests.
  4. Ran the core config schema fixture test.
  5. Ran the scoped Clippy fixer for the network proxy crate.
  6. Ran the scoped Clippy fixer for the core crate.

Notes

  1. Runtime enforcement moved to Wire MITM hooks into runtime enforcement #20659.
  2. User facing PermissionProfile TOML shape remains in Use named MITM permissions config #18240.

@evawong-oai evawong-oai requested a review from viyatb-oai April 21, 2026 17:49
@evawong-oai evawong-oai marked this pull request as ready for review April 21, 2026 17:49
@evawong-oai evawong-oai requested a review from a team as a code owner April 21, 2026 17:49
Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

fn apply_network_constraints(network: NetworkToml, constraints: &mut NetworkProxyConstraints) {
if let Some(enabled) = network.enabled {
constraints.enabled = Some(enabled);
}
if let Some(mode) = network.mode {
constraints.mode = Some(mode);
}
if let Some(allow_upstream_proxy) = network.allow_upstream_proxy {
constraints.allow_upstream_proxy = Some(allow_upstream_proxy);
}
if let Some(dangerously_allow_non_loopback_proxy) = network.dangerously_allow_non_loopback_proxy
{
constraints.dangerously_allow_non_loopback_proxy =
Some(dangerously_allow_non_loopback_proxy);
}
if let Some(dangerously_allow_all_unix_sockets) = network.dangerously_allow_all_unix_sockets {
constraints.dangerously_allow_all_unix_sockets = Some(dangerously_allow_all_unix_sockets);
}
if let Some(domains) = network.domains.as_ref() {
let mut config = NetworkProxyConfig::default();
if let Some(allowed_domains) = constraints.allowed_domains.take() {
config.network.set_allowed_domains(allowed_domains);
}
if let Some(denied_domains) = constraints.denied_domains.take() {
config.network.set_denied_domains(denied_domains);
}
overlay_network_domain_permissions(&mut config, domains);
constraints.allowed_domains = config.network.allowed_domains();
constraints.denied_domains = config.network.denied_domains();
}
if let Some(unix_sockets) = network.unix_sockets.as_ref() {
let allow_unix_sockets = unix_sockets.allow_unix_sockets();
constraints.allow_unix_sockets = Some(allow_unix_sockets);
}
if let Some(allow_local_binding) = network.allow_local_binding {
constraints.allow_local_binding = Some(allow_local_binding);
}

P1 Badge Enforce managed constraints for new MITM hook fields

network.mitm and network.mitm_hooks were added, but trusted-layer constraint derivation does not track them. As a result, higher-precedence user/project config can disable or replace managed MITM hooks without constraint rejection, weakening centrally enforced HTTPS policy.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codex-rs/network-proxy/src/mitm.rs Outdated
@evawong-oai
Copy link
Copy Markdown
Contributor Author

evawong-oai commented Apr 21, 2026

Codex review item about managed constraints for MITM hook fields: this is valid, and it is tracked in Linear as AGE 248.

Why it is not in this PR: PR 18868 is now only the typed MITM hook config model. Enforcing trusted constraints for network.mitm and network.mitm_hooks depends on the final protected destination policy shape and the PermissionProfile trusted layer rules. Putting that here would mix model parsing, precedence, and central policy enforcement in one review.

This remains blocked before dogfood or feature gate rollout. Trusted config must prevent user or project config from disabling or replacing managed MITM hooks before protected destinations are enabled.

The separate method policy ordering concern is tracked in Linear as AGE 247.

@evawong-oai evawong-oai force-pushed the codex/mitm-proxy-landing branch from 2f3ea9b to fe69321 Compare May 1, 2026 18:25
@evawong-oai evawong-oai changed the title Add MITM hooks for host specific HTTPS request clamping Add MITM hook config model May 1, 2026
@evawong-oai evawong-oai force-pushed the codex/mitm-proxy-landing branch 9 times, most recently from b11141b to 5c1bf13 Compare May 6, 2026 21:18
@evawong-oai
Copy link
Copy Markdown
Contributor Author

Thanks. This is intentional for this PR.

The MITM feature is still being built. PR 18868 only lands the typed config model. It does not turn on enforcement.

Enforcement will come in later PRs after the user flow, logging, feature gate, and protected destination policy are ready. That avoids surprising users while the rollout pieces are still being developed.

The follow up work is tracked in Linear:

  1. AGE 247 for method policy ordering.
  2. AGE 248 for managed config constraints.

Those tasks are required before dogfood or feature gate rollout.

Comment thread codex-rs/network-proxy/src/mitm_hook.rs
Comment thread codex-rs/config/src/permissions_toml.rs Outdated
Comment thread codex-rs/network-proxy/src/config.rs Outdated
@evawong-oai evawong-oai force-pushed the codex/mitm-proxy-landing branch from a425e22 to 6f6a861 Compare May 7, 2026 19:32
Copy link
Copy Markdown

@winston-openai winston-openai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, though you may still want to consider splitting this PR up into smaller, more reviewable chunks

@winston-openai
Copy link
Copy Markdown

it may also be worth keeping the config shape internal rather than exposing it in the PermissionProfile given it's all getting redone in #18240

@evawong-oai evawong-oai force-pushed the codex/mitm-proxy-landing branch from 6f6a861 to f7779d8 Compare May 14, 2026 13:00
@evawong-oai
Copy link
Copy Markdown
Contributor Author

Done.

#18868 now keeps the config shape internal. The public PermissionProfile config is in #18240, where the new named hook and action shape lives.

@winston-openai winston-openai force-pushed the codex/mitm-proxy-landing branch from f7779d8 to af01135 Compare May 15, 2026 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants