Skip to content

fix(network-proxy): normalize network proxy host matching#19995

Merged
evawong-oai merged 5 commits intomainfrom
codex/viyatb/network-host-canonicalization
Apr 28, 2026
Merged

fix(network-proxy): normalize network proxy host matching#19995
evawong-oai merged 5 commits intomainfrom
codex/viyatb/network-host-canonicalization

Conversation

@viyatb-oai
Copy link
Copy Markdown
Collaborator

@viyatb-oai viyatb-oai commented Apr 28, 2026

Why

The proxy matches allow and deny rules against normalized host strings. Scoped IPv6 literals can arrive in equivalent forms, such as fd00::1%eth0, [fd00::1%eth0], or [fd00::1%25eth0]. Policy should canonicalize those spellings without erasing scope granularity: an unscoped rule like fd00::1 should still cover scoped requests for that address, while a scoped rule like fd00::1%eth0 should remain exact to that scope.

What changed

  • preserve IPv6 scope IDs during host normalization and canonicalize %25scope to %scope
  • match policy against the exact normalized host plus the unscoped IP base for scoped literals
  • keep local-address explicit allow checks aligned with the same scoped/unscoped semantics
  • add focused coverage for scoped IPv6 normalization, scoped allow rules, and scoped deny rules in network-proxy

Security impact

A request cannot bypass a broad deny rule by adding an IPv6 scope suffix. At the same time, scoped policy remains precise: deny=fd00::1%eth0 affects that scoped spelling without collapsing fd00::1%eth1 onto the same key, and allow=fe80::1%eth0 does not implicitly allow other scopes.

Verification

  • just fmt
  • cargo test -p codex-network-proxy
  • just fix -p codex-network-proxy
  • git diff --check

Co-authored-by: Codex <noreply@openai.com>
@viyatb-oai viyatb-oai changed the title [codex] Normalize network proxy host matching fix(network-proxy): normalize network proxy host matching Apr 28, 2026
Co-authored-by: Codex noreply@openai.com
@viyatb-oai viyatb-oai marked this pull request as ready for review April 28, 2026 16:47
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

Here are some automated review suggestions for this pull request.

Reviewed commit: eebaeec607

ℹ️ 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/policy.rs Outdated
@evawong-oai
Copy link
Copy Markdown
Contributor

evawong-oai commented Apr 28, 2026

Validation update

  1. I reviewed the scoped IPv6 host normalization changes for the linked tracker items. The code now keeps scoped IPv6 literals canonical for exact policy entries, and also checks the unscoped literal for allow and deny matching where that is the intended policy boundary.
  2. Local PR head validation passed with cargo test for codex network proxy, 138 passed and 0 failed.
  3. Local merge validation against current main passed with cargo test for codex network proxy, 140 passed and 0 failed.
  4. I also ran git diff check on the merge worktree and it passed.
  5. I updated the PR branch to current main. Current head is 2540eaf.
  6. Fresh GitHub checks are green. The first Windows full Bazel lane hit the 30 minute job limit after the main test step completed. I reran the failed job, and the rerun passed.

Ship decision: I think this is ready after review approval.

@evawong-oai
Copy link
Copy Markdown
Contributor

evawong-oai commented Apr 28, 2026

Validation done against the readiness row for the scoped IPv6 denial bypass.

  1. The row says current main allowed fd00:dead:beef::1%eth0 when fd00:dead:beef::1 was denied and local binding was enabled. It calls for IPv6 scope canonicalization before deny, allow, and local checks, with bracketed, raw, and %25 coverage.
  2. I added a temporary exact repro test locally with allowlist *, denylist fd00:dead:beef::1, and local binding enabled. Base 10e2a73b3c9be61cd09c990b60551b90bbab5f74 failed as expected: fd00:dead:beef::1%eth0 returned Allowed instead of Blocked(Denied).
  3. The same exact repro passes on this PR head 2540eaf16e9356876993efb9b1b2ce87e7ceda0a for fd00:dead:beef::1%eth0, [fd00:dead:beef::1%eth0], and [fd00:dead:beef::1%25eth0].
  4. CI is green, including Linux, macOS, Windows Bazel build and release checks.

Verdict: this fixes the tracked policy bypass described in the readiness row. Remaining readiness work is merge plus validation on the final event build if the row is held to published build readiness.

Copy link
Copy Markdown
Contributor

@evawong-oai evawong-oai left a comment

Choose a reason for hiding this comment

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

Approved based on validation against the readiness row. The exact repro fails on base and passes on this head, and CI is green.

@evawong-oai evawong-oai merged commit 2dbde94 into main Apr 28, 2026
35 of 36 checks passed
@evawong-oai evawong-oai deleted the codex/viyatb/network-host-canonicalization branch April 28, 2026 22:50
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 28, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants