Read the host out of a URL for the internal_host match - #152
Merged
Conversation
Found by running the whole chain on a real app: a vulnerability detected on a site, with a
reachability flow and a vPatch template, generated a rule pinned to the app's own parameter
(`get.url` on `/api/preview`), served it — and the rule matched nothing at all.
`internal_host` classifies its value AS a hostname. That is right for the egress phase, where the
value is the destination host, and wrong for the request phase, where the same question arrives as
an application parameter whose value is a full URL. `isInternalHost('http://169.254.169.254/…')`
is false, so every request-phase SSRF rule was expressible, servable, and permanently inert — the
exact failure this engine has been hardened against repeatedly, in the one match type meant to
prevent it. Six of fifteen destinations were wrong before this, every one a false negative.
The match now extracts a host first: a URL (any scheme, including protocol-relative) is parsed and
its hostname classified; `host:port` and `[v6]:port` lose the port; a bare host passes through
untouched, so the egress path and the built-in default rule behave exactly as before.
Parsed rather than sliced, deliberately. Userinfo puts a trusted-looking name before the real host
(`http://api.stripe.example@169.254.169.254/` must match) and a fragment puts one after it
(`http://evil.example/#@127.0.0.1` must not). A substring check reads the wrong host in both
directions — a bypass in the first case, a false positive in the second — and the slice-instead-of-
parse mutation fails on exactly those two.
Classification itself is unchanged, so every canonicalisation defence still applies to what comes
out: decimal/hex IPv4, expanded and IPv4-mapped IPv6, trailing dots.
Verified end to end after the fix, against the rule the platform actually served: as served it
detects and does not block (per-rule `dry-run` overriding the site's block mode, as designed);
with enforcement promoted it returns 403; a third-party URL on the same route passes; an internal
URL on a different route passes, so the route scope holds.
1216 tests, typecheck clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
hostFromValue cleanly extracts hosts from diverse URL inputs to fix SSRF matches. 🎯 Quality: 100% Elite · 📦 Size: Medium 📈 This month: Your 96th PR — above team average · Averaging Excellent |
The matcher test that came with the fix exercises `matchValue()` directly, and the defect was not a
matcher in isolation: it was a rule bound to the right parameter, scoped to the right route,
carrying the right per-rule enforcement, arriving at the runtime intact, and never firing. Every
part was individually correct and every unit test passed.
`pulse-chain.test.ts` cannot catch that. It covers the same transport with a static lodash rule
whose conditions read `raw` — no route scope, no `get.*` source, and a match type that had only
ever been exercised on the egress path. So the pinned shape now has its own chain test, fetching
from a mock Pulse and enforcing through the HTTP guard, with the four assertions that separate
"protecting" from "present": detected in dry-run, 403 once promoted, a third-party destination on
the same route allowed, and an internal destination on another route allowed.
The fixture is captured from a real `GET pulse/rules/{uuid}` response and carries the template
beside the served copy, so the substitution is visible rather than described — and the unbound
template is asserted to be inert, which is the other half of the same failure: `<param>` is not a
parameter source, so a rule that reached an app with placeholders intact would load, report as
shipped protection, and never match.
Writing it turned up a second silent widening. The engine's scope key is `when.path`; the first
draft wrote `when.route`, which is not an error — the scope is ignored and the rule applies to
every request. Fail-open is right for a scope that cannot be EVALUATED, but a scope that cannot be
UNDERSTOOD is an authoring mistake with the opposite consequence: for a blocking rule it is a
false-positive surface across the whole app instead of one endpoint. The engine now warns once and
says which keys it understands; behaviour is unchanged, and the widening is pinned by a test so it
is documented rather than rediscovered.
1221 tests, typecheck clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
/review |
daniloradovic
approved these changes
Aug 20, 2026
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
2 participants
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.
Found by running the whole chain on a real app rather than checking that each part looked right.
What happened
A vulnerability detected on a site, with a reachability flow and a matching vPatch template, generated a rule pinned to the app's own parameter —
get.urlon/api/preview, no placeholders left unbound — and the platform served it. Then the rule matched nothing at all.internal_hostclassifies its value as a hostname. That is correct for the egress phase, where the value is the destination host, and wrong for the request phase, where the same question arrives as an application parameter whose value is a full URL.isInternalHost('http://169.254.169.254/latest/meta-data/')is false, so a request-phase SSRF rule was expressible, servable, and permanently inert — the failure mode this engine has been hardened against repeatedly, in the one match type meant to prevent it.Measured before the fix, six of fifteen destinations were wrong, every one a false negative:
169.254.169.254http://169.254.169.254/latest/meta-data/http://localhost:3000/admin//10.0.0.5/x169.254.169.254:80https://api.stripe.example/v1/chargeshow to use localhost in dockerThe change
The match extracts a host before classifying: a URL (any scheme, including protocol-relative) is parsed and its hostname classified;
host:portand[v6]:portlose the port; a bare host passes through untouched, so the egress path and the built-inegress-internal-addressdefault behave exactly as before.Classification itself is unchanged, so every canonicalisation defence still applies to what comes out — decimal/hex IPv4, expanded and IPv4-mapped IPv6, trailing dots.
Parsed rather than sliced, deliberately. Userinfo puts a trusted-looking name before the real host (
http://api.stripe.example@169.254.169.254/must match); a fragment puts one after it (http://evil.example/#@127.0.0.1must not). A substring check reads the wrong host in both directions — a bypass in the first case, a false positive in the second. Both are asserted, and the slice-instead-of-parse mutation fails on exactly those two.Verification
1216 tests, typecheck clean. Three new test groups: URL-valued destinations, the two evasions above, and a bare-host group whose job is to prove extraction is a no-op for the egress path.
Mutation-checked: removing the extraction, and replacing the URL parse with a textual slice, each fail two assertions and no others.
Then re-run end to end against the rule the platform actually served:
enforcement: dry-run, which correctly overrides the site'sblockmode; a generated pinned rule stays dry-run until promoted.Independent of the two other open PRs (#151 and the platform's presentation work); this branch is off
main.