Read the host out of a URL for the internal_host match

src/protect/engine/engine.js

@@ -171,11 +171,58 @@ function warnUnsupportedMatchType(type) {
   );
 }
 
-// Internal / private / loopback / link-local / cloud-metadata host check, used by the
-// `internal_host` match type for SSRF egress rules. It CANONICALIZES the host before classifying —
+// Internal / private / loopback / link-local / cloud-metadata host check behind the `internal_host`
+// match type. It CANONICALIZES the host before classifying —
 // a textual/prefix check is bypassable by alternate encodings (decimal/hex/octal IPv4, expanded or
 // IPv4-mapped IPv6), which is a classic SSRF evasion. Handles localhost / *.local / GCP metadata
 // names, every IPv4 spelling inet_aton accepts, and IPv6 loopback/link-local/unique-local/mapped.
+/**
+ * The host to classify out of a rule parameter's value.
+ *
+ * `internal_host` was written for the egress phase, where the value IS the destination host. On the
+ * request phase the same question arrives as an application parameter, and there the value is almost
+ * always a full URL (`?url=http://169.254.169.254/latest/meta-data/`) or a `host:port` pair — neither of
+ * which is a hostname, so classifying the raw string answered "not internal" for every one of them. A
+ * request-phase SSRF rule was therefore expressible, servable and permanently inert: the exact failure
+ * this engine has been repeatedly hardened against, in the one match type meant to prevent it.
+ *
+ * Only the host is extracted; the classification itself is unchanged, so every canonicalization defence
+ * (decimal/hex IPv4, expanded and IPv4-mapped IPv6, trailing dots) still applies to what comes out. A
+ * value that is already a bare host passes through untouched, which is what keeps the egress path and
+ * the built-in default rule behaving exactly as before.
+ */
+function hostFromValue(value) {
+  const raw = String(value ?? '').trim();
+  if (raw === '') return '';
+
+  // A scheme (`http://`, and deliberately any other) or a protocol-relative URL. Parsing rather than
+  // string-slicing is what makes the userinfo evasion (`http://trusted@169.254.169.254/`) resolve to the
+  // host actually contacted, and keeps `http://evil.com#@127.0.0.1` resolving to evil.com.
+  if (/^[a-z][a-z0-9+.-]*:\/\//i.test(raw) || raw.startsWith('//')) {
+    try {
+      return new URL(raw.startsWith('//') ? `http:${raw}` : raw).hostname;
+    } catch {
+      // Unparseable: hand the raw value on, where the host check rejects it rather than guessing.
+      return raw;
+    }
+  }
+
+  // `[::1]:8080` — bracketed IPv6 with or without a port.
+  if (raw.startsWith('[')) {
+    const end = raw.indexOf(']');
+    if (end > 0) return raw.slice(1, end);
+  }
+
+  // `169.254.169.254:80`. Only a single colon followed by digits: a bare IPv6 address has several, and
+  // must not have its last group mistaken for a port.
+  const colon = raw.indexOf(':');
+  if (colon > 0 && raw.indexOf(':', colon + 1) === -1 && /^\d+$/.test(raw.slice(colon + 1))) {
+    return raw.slice(0, colon);
+  }
+
+  return raw;
+}
+
 function isInternalHost(hostname) {
   if (!hostname) return false;
   let host = String(hostname).toLowerCase().replace(/^\[|\]$/g, '');
@@ -282,10 +329,28 @@ function expandIPv6(host) {
   return out;
 }
 
+// Report a `when` block that names nothing this engine understands. Fail-open is correct for a scope that
+// cannot be EVALUATED, but a scope that cannot be UNDERSTOOD is an authoring mistake with the opposite
+// consequence: the rule silently applies to every request instead of one route, which for a blocking rule
+// is a false-positive surface across the whole app. Warned once so it is discoverable in a log.
+const warnedScopes = new Set();
+function warnUnrecognisedScope(when) {
+  const key = Object.keys(when).sort().join(',');
+  if (warnedScopes.has(key)) return;
+  warnedScopes.add(key);
+  console.warn(
+    `[patchstack] Rule scope \`when: { ${key} }\` names no supported key — the engine understands ` +
+      `\`method\` and \`path\`. The scope is IGNORED and the rule applies to every request.`
+  );
+}
+
 // Route/method scope for a rule's optional `when: { method, path }`. Fail-open: if the scope can't
 // be evaluated, the rule still applies (never silently suppress a rule).
 function ruleAppliesTo(when, resolver) {
   try {
+    if (when.method === undefined && when.path === undefined && Object.keys(when).length > 0) {
+      warnUnrecognisedScope(when);
+    }
     if (when.method) {
       const methods = (Array.isArray(when.method) ? when.method : [when.method]).map((m) => String(m).toUpperCase());
       const actual = String(resolver.resolve('server.REQUEST_METHOD')[0] ?? 'GET').toUpperCase();
@@ -502,8 +567,9 @@ export function matchValue(type, value, matchVal, matchObj) {
     }
 
     case 'internal_host':
-      // SSRF egress: private / loopback / link-local / cloud-metadata destinations.
-      return isInternalHost(strValue);
+      // SSRF: private / loopback / link-local / cloud-metadata destinations. The value may be a bare
+      // host (egress) or a URL / host:port in an application parameter (request) — see `hostFromValue`.
+      return isInternalHost(hostFromValue(strValue));
 
     case 'quotes':
     // engine-php exposes `inline_js_xss` as an alias of `quotes`.

tests/protect/engine.test.ts

@@ -123,6 +123,52 @@ describe('RuleEngine', () => {
       }
     });
 
+    it('should match internal_host when the value is a URL, not a bare host', () => {
+      // The gap this closes: `internal_host` was written for the egress phase, where the value IS the
+      // destination host. On the request phase the same question arrives as an application parameter and
+      // the value is a full URL — so a served, correctly-pinned SSRF rule matched nothing at all.
+      for (const value of [
+        'http://169.254.169.254/latest/meta-data/',
+        'http://localhost:3000/admin',
+        'https://127.0.0.1/x',
+        'http://[::1]/x',
+        'http://metadata.google.internal/computeMetadata/v1/',
+        '//10.0.0.5/x',
+        '169.254.169.254:80',
+        '[::1]:8080',
+      ]) {
+        assert.strictEqual(matchValue('internal_host', value, null), true, `${value} should be internal`);
+      }
+
+      for (const value of [
+        'https://api.stripe.example/v1/charges',
+        'http://8.8.8.8/resolve',
+        'how to use localhost in docker',
+        'https://example.com/?next=/admin',
+      ]) {
+        assert.strictEqual(matchValue('internal_host', value, null), false, `${value} should be external`);
+      }
+    });
+
+    it('should read the host a URL actually contacts, not the one it advertises', () => {
+      // Why the host is parsed rather than sliced out of the string. Userinfo puts a trusted-looking name
+      // before the real host, and a fragment puts one after it; a substring check reads the wrong one in
+      // both directions, which is a bypass in the first case and a false positive in the second.
+      assert.strictEqual(matchValue('internal_host', 'http://api.stripe.example@169.254.169.254/', null), true);
+      assert.strictEqual(matchValue('internal_host', 'http://evil.example/#@127.0.0.1', null), false);
+      assert.strictEqual(matchValue('internal_host', 'http://evil.example/?next=http://127.0.0.1/', null), false);
+    });
+
+    it('should leave a bare host classified exactly as before', () => {
+      // The egress path and the built-in default rule pass `egress.host`, which is already a hostname.
+      // Extraction must be a no-op for those, or this change would alter what a live guard blocks today.
+      assert.strictEqual(matchValue('internal_host', '169.254.169.254', null), true);
+      assert.strictEqual(matchValue('internal_host', '::1', null), true);
+      assert.strictEqual(matchValue('internal_host', '2130706433', null), true); // decimal 127.0.0.1
+      assert.strictEqual(matchValue('internal_host', 'example.com', null), false);
+      assert.strictEqual(matchValue('internal_host', '', null), false);
+    });
+
     it('should match quotes (and the inline_js_xss alias)', () => {
       assert.strictEqual(matchValue('quotes', `x' OR 1=1`, null), true);
       assert.strictEqual(matchValue('quotes', 'no quotes here', null), false);

tests/protect/fixtures/generated-pinned-rule.json

@@ -0,0 +1,44 @@
+{
+  "note": "Captured from a platform `GET pulse/rules/{uuid}` response for an app whose attack-surface map proved request input reaching an HTTP client. This is the SHAPE a coordinate-pinned rule arrives in — the parameter and route are the app's own, bound from a template at generation time. No advisory identifier: which advisories are shielded is not a public list.",
+  "shape": {
+    "routeScopedInRuleV2": "The platform scopes the route with an ANDed `server.REQUEST_URI contains` condition rather than a `when` block. Both forms are exercised by the suite, because the scope moving into `when` must not silently drop the scope.",
+    "enforcement": "A generated rule arrives dry-run regardless of the site's mode, and only promotion flips it. The per-rule value overriding a site-wide `block` is the safety property that keeps an unproven pinned rule from blocking traffic.",
+    "boundPlaceholders": "`<param>` and `<route>` are substituted before serving. A rule that reached an app with them intact would load and never match — `<param>` is not a parameter source — which is indistinguishable from protection until someone fires an exploit at it."
+  },
+  "served": {
+    "id": "pulse-1",
+    "title": "Block internal-host URLs reaching the vulnerable HTTP client",
+    "rule_v2": [
+      {
+        "match": { "type": "contains", "value": "/api/preview" },
+        "inclusive": true,
+        "mutations": ["urldecode"],
+        "parameter": "server.REQUEST_URI"
+      },
+      {
+        "match": { "type": "internal_host" },
+        "inclusive": true,
+        "mutations": ["urldecode"],
+        "parameter": "get.url"
+      }
+    ],
+    "enforcement": "dry-run"
+  },
+  "template": {
+    "note": "The same rule before binding, as authored. Kept beside the served copy so the substitution is visible rather than described, and so a test can prove the unbound form is inert.",
+    "rule_v2": [
+      {
+        "match": { "type": "contains", "value": "<route>" },
+        "inclusive": true,
+        "mutations": ["urldecode"],
+        "parameter": "server.REQUEST_URI"
+      },
+      {
+        "match": { "type": "internal_host" },
+        "inclusive": true,
+        "mutations": ["urldecode"],
+        "parameter": "<param>"
+      }
+    ]
+  }
+}