fix(protect): egress redirect re-screening + redaction case-sensitivity + IPv6-ULA over-match - #105
Merged
patchstackdave merged 2 commits intoAug 12, 2026
Conversation
…y, IPv6-ULA over-match Three output-filtering / egress correctness fixes surfaced by a review of the response + egress phases: - egress: the guarded fetch only screened the initial URL and let native `redirect: follow` follow a 3xx to an internal address — an SSRF-via-open-redirect bypass (302 -> 169.254.169.254 reached the metadata endpoint). Follow redirects ourselves with `redirect: manual`, screening every hop; buffer the body once so 307/308 can replay it, rewrite 303/POST -> GET, and strip Authorization/Cookie on cross-origin hops. Fail open on un-normalizable input. - response redaction: `contains`/`stripos` detection is case-insensitive but the mask was case-sensitive, so a `contains: "SECRET"` rule detected `secret` yet masked nothing — the leak was served while telemetry reported a redaction. Mask case-insensitively. - isInternalHost: the `fc`/`fd` IPv6-ULA prefix check fired on any hostname starting with those letters (e.g. fcm.googleapis.com), blocking legitimate egress in block mode. Require a colon (actual IPv6) before applying it. 600 tests pass (+5 new in tests/protect/correctness-fixes.test.ts); typecheck clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Comprehensive bug-fix with robust egress screening and thorough edge-case tests. 🎯 Quality: 100% Elite · 📦 Size: Medium 📈 This month: Your 49th PR — above team average · Averaging Excellent |
Contributor
Author
|
/review |
daniloradovic
approved these changes
Aug 12, 2026
devlob
approved these changes
Aug 12, 2026
Add edge cases for the correctness fixes: redirect method/body semantics (307 preserves method+body, 303 rewrites to a bodyless GET), cross-origin Authorization stripping, the max-redirects cap, caller redirect:'manual' passthrough (not followed), uppercase/bracketed IPv6-ULA classification, and case-insensitive redaction of a secret in a response header as well as the body. 13 tests in the file; full suite green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
patchstackdave
deleted the
fix/egress-redirect-rescreen-and-redaction-correctness
branch
August 12, 2026 11:03
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Three correctness fixes in the
protectresponse/egress phases, from a review of the output-filtering surface. All contained; no API changes.1. Egress: redirects weren't re-screened (SSRF via open redirect)
The guarded
fetchscreened only the initial URL, then delegated to nativeredirect: 'follow'— so a response that 302-redirects to an internal address (e.g.http://169.254.169.254/) reached it unchecked. Now we follow redirects ourselves withredirect: 'manual', screening every hop. Body is buffered once so 307/308 can replay it; 303 (and 301/302 on POST) rewrite to a bodyless GET; Authorization/Cookie are stripped on cross-origin hops; capped at 20 redirects; fail-open on un-normalizable input.(The true fetch/undici connection pin — vs. resolve-and-screen — remains the separate roadmap item.)
2. Response redaction was case-sensitive while detection is case-insensitive
contains/striposdetection lowercases both sides, but the redactor masked with a case-sensitivesplit(literal). So acontains: "SECRET"rule detectedsecretyet masked nothing — the leak was served in the clear while telemetry reported a redaction. Now masks case-insensitively (escaped literal →giregex), andencodeescapes the actual matched span.3.
isInternalHostfc/fdprefix over-matched hostnamesThe IPv6 ULA (
fc00::/7) check fired on any hostname starting withfc/fd— e.g.fcm.googleapis.com— blocking legitimate egress in block mode. Now requires an actual IPv6 (:present).Tests
tests/protect/correctness-fixes.test.ts(5 new): IPv6-ULA vs. hostname classification, case-insensitive redaction, redirect-to-internal blocked, redirect-to-allowed followed. Full suite 600 passed; typecheck clean.🤖 Generated with Claude Code