From 36fd6a625efe954bc53f2f45314ff171dd612a30 Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Tue, 18 Aug 2026 08:10:43 +0200 Subject: [PATCH] Resolve request headers by presence rather than truthiness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `server.HTTP_*` parameter source read an arbitrary header with a truthiness check, so a header present with an empty value resolved to nothing at all — indistinguishable from a header that was never sent. A rule authored as a presence check against a header therefore matched every non-empty value and silently missed the empty one, which is the worst kind of gap: the rule reports as active protection while one spelling of the input walks past it. Resolve the generic `HTTP_*` branch by presence instead, reading only own properties so an inherited name like `constructor` cannot masquerade as a header that every request carries. The named cases (host, origin, referer, user-agent, content-type, content-length) keep value semantics deliberately: they feed matchers for which an empty string and an absent header already mean the same thing, and handing those an empty string instead of nothing would change comparisons that are working correctly. Co-Authored-By: Claude Opus 5 (1M context) --- src/protect/engine/request.js | 9 ++++++++- tests/protect/request.test.ts | 20 ++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/protect/engine/request.js b/src/protect/engine/request.js index 604578b..c0d8055 100644 --- a/src/protect/engine/request.js +++ b/src/protect/engine/request.js @@ -253,7 +253,14 @@ export class RequestResolver { default: { if (key.startsWith('HTTP_')) { const headerName = key.substring(5).toLowerCase().replace(/_/g, '-'); - return req.headers?.[headerName] ? [req.headers[headerName]] : []; + const headers = req.headers; + // Presence, not truthiness. A header sent with an empty value IS present, and an `isset` + // rule authored against it must see it — some bypasses are carried by the header existing + // at all, so treating `Header:` as absent would make the rule quietly miss the shape it + // was written for. (The named cases above keep value semantics: for host/origin/referer an + // empty string and an absent header mean the same thing to the matchers that read them.) + if (headers === null || typeof headers !== 'object') return []; + return Object.prototype.hasOwnProperty.call(headers, headerName) ? [headers[headerName]] : []; } return []; } diff --git a/tests/protect/request.test.ts b/tests/protect/request.test.ts index 03c343a..9d27a07 100644 --- a/tests/protect/request.test.ts +++ b/tests/protect/request.test.ts @@ -137,6 +137,26 @@ describe('RequestResolver', () => { assert.deepStrictEqual(resolver.resolve('server.HTTP_X_FORWARDED_FOR'), ['1.2.3.4']); }); + it('should treat an empty header value as present, not absent', () => { + // A header sent with no value IS present, and some bypasses are carried by a header existing at + // all rather than by what it contains — so an `isset` rule authored against one must see it. + // Resolving by truthiness returned [] here, making the rule silently miss that shape. + const resolver = new RequestResolver(createReq({ headers: { 'x-internal-marker': '' } })); + assert.deepStrictEqual(resolver.resolve('server.HTTP_X_INTERNAL_MARKER'), ['']); + }); + + it('should still resolve an absent header to nothing', () => { + const resolver = new RequestResolver(createReq({ headers: {} })); + assert.deepStrictEqual(resolver.resolve('server.HTTP_X_INTERNAL_MARKER'), []); + }); + + it('should not treat an inherited property name as a header', () => { + // `headers.constructor` exists on every object; reading presence off the prototype chain would + // make every request appear to carry a `constructor` header. + const resolver = new RequestResolver(createReq({ headers: {} })); + assert.deepStrictEqual(resolver.resolve('server.HTTP_CONSTRUCTOR'), []); + }); + it('should resolve REMOTE_ADDR from ip', () => { const resolver = new RequestResolver(createReq({ ip: '10.0.0.1' })); assert.deepStrictEqual(resolver.resolve('server.REMOTE_ADDR'), ['10.0.0.1']);