Use named MITM permissions config#18240
Merged
Merged
Conversation
90964e9 to
fe1b53e
Compare
ddf1703 to
45afcb8
Compare
Contributor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7da762df6
ℹ️ 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".
2f3ea9b to
fe69321
Compare
2cf89ee to
ad3615b
Compare
ec08b07 to
6edf86c
Compare
ac139fc to
d37498d
Compare
6edf86c to
03765b9
Compare
d37498d to
ac760a4
Compare
03765b9 to
1c1ed25
Compare
ac760a4 to
e0bf393
Compare
1c1ed25 to
a5db4d3
Compare
e0bf393 to
494cf09
Compare
a5db4d3 to
c4188b4
Compare
494cf09 to
f52732a
Compare
c4188b4 to
912d8e7
Compare
f52732a to
0ea4b1e
Compare
912d8e7 to
16c939f
Compare
e342e00 to
599ba13
Compare
ec95472 to
06bab42
Compare
Contributor
winston-openai
left a comment
There was a problem hiding this comment.
3 things:
- Codex configs are layered (e.g. system config vs user config). It'd be helpful to allow higher level configs to override the named hook actions. Right now validation is just scoped to the config itself, so an override in a higher level config of just the named action would be thrown away.
mitm.enabledseems like it's an implementation detail rather than something we should expose at the config level. I only see 2 cases when we'd want it enabled: hooks exist ormode=limited. The fact someone can disable it during those times feels broken and I can't think of when you'd want to enable it outside of those two settings, so let's just make it conditional and hide it from the user-facing config.- More of a nit, but you'd made a previous change to the README based on your older config; we should update that to match this new config
599ba13 to
5c28c96
Compare
06bab42 to
7a319d2
Compare
Contributor
Author
|
Done.
I also kept the external env ordering change out of this stack. |
7a319d2 to
60c5973
Compare
5c28c96 to
2c9ccbe
Compare
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
Contributor
|
I have read the CLA Document and I hereby sign the CLA |
Contributor
|
recheck |
winston-openai
approved these changes
May 16, 2026
f749514 to
60c5973
Compare
pakrym-oai
approved these changes
May 20, 2026
evawong-oai
added a commit
that referenced
this pull request
May 20, 2026
## Stack 1. This PR adds MITM hook config and model only. 2. Runtime follow up: #20659 wires hook enforcement into the proxy request path. 3. User facing config follow up: #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 #20659. 2. User facing PermissionProfile TOML shape remains in #18240.
74ac468 to
e226b36
Compare
evawong-oai
added a commit
that referenced
this pull request
May 20, 2026
## 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.
fb26f1f to
09024fe
Compare
09024fe to
b4ceb64
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack
Why
[permissions.<profile>.network.mitm]instead of exposing the flat runtime shape to users.Summary
[permissions.<profile>.network.mitm]so the selected PermissionProfile owns network proxy policy.[permissions.<profile>.network.mitm.hooks.<name>].[permissions.<profile>.network.mitm.actions.<name>].NetworkMitmActionToml, then lower them into the proxy runtime action config.Example
Validation