Merge pull request #522 from jason5ng32/fix/guard-query-rewrites

Fix(api): make guard query rewrites survive Express 5

api/AGENTS.md

@@ -126,6 +126,8 @@ behind `dns-resolver` (gated by `tests/dns-resolvers-data.test.js`).
 - `requireValidReportId()` — `/api/report/:id` route param (22-char base64url).
 
 New param shape → new guard there, attached in `backend-server.js`.
+Rewrites go through `setQueryParam()` there: Express 5's `req.query` re-parses
+on every read, so a direct write is lost.
 
 ### Response enrichment lives in middleware too
 

server/guards.js

@@ -11,6 +11,18 @@ import { STATUS_PROVIDER_IDS } from './service-status-providers.js';
 import { DNS_RECORD_TYPE_SET } from '../common/dns-record-types.js';
 import { parseAsnInput } from '../common/asn-input.js';
 
+// Express 5's req.query is a prototype getter that re-parses the URL on every
+// read, so a direct write is lost. Pin the parsed object as an own property
+// first; later guards, cacheable resolvers and the handler then share it.
+const setQueryParam = (req, name, value) => {
+    if (!Object.hasOwn(req, 'query')) {
+        Object.defineProperty(req, 'query', {
+            value: req.query, writable: true, configurable: true, enumerable: true,
+        });
+    }
+    req.query[name] = value;
+};
+
 // Reject requests without an allowed referer. The error message variant
 // preserves the existing user-facing wording.
 export const requireReferer = (req, res, next) => {
@@ -57,7 +69,7 @@ export const requireValidDomain = (paramName = 'domain') => (req, res, next) =>
     if (!isValidDomain(domain)) {
         return res.status(400).json({ error: 'Invalid domain' });
     }
-    req.query[paramName] = domain;
+    setQueryParam(req, paramName, domain);
     next();
 };
 
@@ -89,7 +101,7 @@ export const requireValidASN = (paramName = 'asn') => (req, res, next) => {
     if (asn === null) {
         return res.status(400).json({ error: 'Invalid ASN' });
     }
-    req.query[paramName] = String(asn);
+    setQueryParam(req, paramName, String(asn));
     next();
 };
 
@@ -104,7 +116,7 @@ export const normalizeAsnQuery = (paramName = 'q') => (req, res, next) => {
     if (asn === null) {
         return res.status(400).json({ error: 'Invalid ASN' });
     }
-    req.query[paramName] = `AS${asn}`;
+    setQueryParam(req, paramName, `AS${asn}`);
     next();
 };
 
@@ -119,7 +131,7 @@ export const requireValidCountry = (paramName = 'country') => (req, res, next) =
     if (!/^[A-Za-z]{2}$/.test(raw)) {
         return res.status(400).json({ error: 'Invalid country' });
     }
-    req.query[paramName] = String(raw).toUpperCase();
+    setQueryParam(req, paramName, String(raw).toUpperCase());
     next();
 };
 
@@ -145,7 +157,7 @@ export const requireValidRecordType = (paramName = 'type') => (req, res, next) =
     if (!DNS_RECORD_TYPE_SET.has(String(type).toUpperCase())) {
         return res.status(400).json({ error: 'Invalid record type' });
     }
-    req.query[paramName] = String(type).toUpperCase();
+    setQueryParam(req, paramName, String(type).toUpperCase());
     next();
 };
 

tests/guards.test.js

@@ -1,5 +1,6 @@
 import assert from 'node:assert/strict';
 import { describe, it } from 'node:test';
+import express from 'express';
 
 import { requireReferer, requirePublicIP, requireValidPrefix, requireValidDomain, requireValidProviderId, requireValidRecordType, requireValidReportId, requireValidCountry, requireValidASN, normalizeAsnQuery } from '../server/guards.js';
 
@@ -407,3 +408,47 @@ describe('normalizeAsnQuery', () => {
         }
     });
 });
+
+// Express 5 defines req.query as a prototype getter that re-parses the URL
+// on every read; a real request (never listening) catches rewrites that a
+// plain-object stub would keep.
+describe('guard rewrites on a real Express request', () => {
+    const app = express();
+    const makeExpressReq = (url) => {
+        const req = Object.create(app.request);
+        req.url = url;
+        req.headers = {};
+        return req;
+    };
+
+    it('reproduces the re-parsing getter', () => {
+        const req = makeExpressReq('/x?type=caa');
+        assert.notEqual(req.query, req.query);
+    });
+
+    for (const [name, guard, url, param, expected] of [
+        ['requireValidRecordType', requireValidRecordType(), '/x?type=caa', 'type', 'CAA'],
+        ['requireValidDomain', requireValidDomain(), '/x?domain=Example.COM', 'domain', 'example.com'],
+        ['requireValidASN', requireValidASN(), '/x?asn=AS013335', 'asn', '13335'],
+        ['normalizeAsnQuery', normalizeAsnQuery(), '/x?q=as13335', 'q', 'AS13335'],
+        ['requireValidCountry', requireValidCountry(), '/x?country=us', 'country', 'US'],
+    ]) {
+        it(`${name} rewrite survives a later req.query read`, () => {
+            const req = makeExpressReq(url);
+            let nextCalled = false;
+            guard(req, makeRes(), () => { nextCalled = true; });
+            assert.equal(nextCalled, true);
+            assert.equal(req.query[param], expected);
+        });
+    }
+
+    it('keeps both rewrites when chained like /api/dnsresolver', () => {
+        const req = makeExpressReq('/x?type=caa&hostname=Example.COM');
+        let calls = 0;
+        requireValidDomain('hostname')(req, makeRes(), () => { calls++; });
+        requireValidRecordType()(req, makeRes(), () => { calls++; });
+        assert.equal(calls, 2);
+        assert.deepEqual({ ...req.query }, { type: 'CAA', hostname: 'example.com' });
+        assert.equal(req.query, req.query);
+    });
+});