From 2221d8f93d031a905632f9bd19adf8841fa37a3c Mon Sep 17 00:00:00 2001 From: "Aaron K. Clark" Date: Tue, 19 May 2026 01:19:46 -0500 Subject: [PATCH] feat(pagination): PUBLIC_BASE_URL pin to defend Link header against host injection `buildLinkHeader` used `req.protocol + req.get('host')` to build the absolute URLs that go into the RFC 5988 `Link` header. The Host header is client-controlled, so any deployment that doesn't already filter unknown Host values upstream (raw `docker-compose up`, behind a tolerant NAT, sidecar-style local exposure) can be fed a malicious Host and reflect it back into the next/prev/first/last URLs. Paginating clients following those URLs would then hit the attacker. Add an opt-in `PUBLIC_BASE_URL` env var. When set, the helper uses it as the canonical `scheme://host` base; when unset, fall back to the existing req-derived behavior. Trailing slashes are stripped so operators who include them don't end up with `//path` in the emitted links. Whitespace-only values are treated as unset. Existing callers see no behavior change unless they opt in; the TLS-fronted compose deployment (Caddy with `TLS_DOMAIN` pinned) remains the recommended config and is already safe without setting the env var, since Caddy rejects mismatched Host values upstream. Three new tests cover: set value pins the host (malicious Host ignored); trailing-slash normalization; whitespace-only falls back to default. README env table + .env.example document the new var. Co-Authored-By: Claude Opus 4.7 (1M context) --- .env.example | 9 ++++++++ README.md | 1 + app/middleware/pagination.js | 26 +++++++++++++++++++--- tests/unit/pagination.test.js | 41 ++++++++++++++++++++++++++++++++++- 4 files changed, 73 insertions(+), 4 deletions(-) diff --git a/.env.example b/.env.example index e7069c5..5b46707 100644 --- a/.env.example +++ b/.env.example @@ -98,6 +98,15 @@ DB_PASSWORD=changeme # non-proxied client. # TRUST_PROXY=true +# Canonical base URL the API is reachable at, used to build absolute +# URLs in the RFC 5988 Link header (next/prev/first/last pagination +# refs). Pin this in production so a client sending `Host: evil.com` +# can't get evil.com echoed back into the Link header and influence +# how its own paginating client walks the result set. Unset = derive +# from `req.protocol` + `req.get('host')` (the express default, +# which is safe when an upstream proxy filters Host). +# PUBLIC_BASE_URL=https://api.example.com + # Optional bearer token gating /metrics. Unset = open scrape (the # usual private-network deployment pattern). When set, the # Prometheus scrape must include `Authorization: Bearer `. diff --git a/README.md b/README.md index 06aa4fb..565d056 100644 --- a/README.md +++ b/README.md @@ -208,6 +208,7 @@ production). See `.env.example` for the canonical reference. | `DB_NAME` | `timetracker` | Database name. | | `DB_USER` | `timetracker` | Database user (must have access to the `dbo` schema). | | `DB_PASSWORD` | (empty) | Database password. **Required.** Setting it empty will cause connection failures and a startup warning. | +| `PUBLIC_BASE_URL` | (unset) | Canonical `scheme://host` the API is publicly reachable at. Used as the base for absolute URLs in the RFC 5988 `Link` header (pagination next/prev/first/last). Pin in production so a client sending a malicious `Host` header can't get it echoed back. Unset = derive from `req.protocol` + `req.get('host')`. | `.env` is gitignored. Never commit a populated `.env`. diff --git a/app/middleware/pagination.js b/app/middleware/pagination.js index 3f37811..cdff865 100644 --- a/app/middleware/pagination.js +++ b/app/middleware/pagination.js @@ -33,18 +33,38 @@ function buildLinkHeader({ req, limit, offset, count }) { // Resolve the URL minus the query string. req.originalUrl is "/path?qs"; // strip the qs portion deterministically. - const proto = (req.protocol || 'http'); - const host = (req.get && req.get('host')) || 'localhost'; const url = req.originalUrl || '/'; const qIdx = url.indexOf('?'); const basePath = qIdx === -1 ? url : url.slice(0, qIdx); const existingQs = qIdx === -1 ? '' : url.slice(qIdx + 1); + // Base URL resolution, in priority order: + // 1. PUBLIC_BASE_URL env var — operator-pinned canonical hostname. + // Use this in production behind a reverse proxy that may + // receive arbitrary Host headers; pinning here prevents a + // client that sends `Host: evil.com` from getting evil.com + // echoed back into the Link header (next/prev/first/last) + // and influencing how its own paginating client walks the + // result set. + // 2. req.protocol + req.get('host') — the express defaults. + // Useful for local development and for deployments where the + // Host header is already filtered upstream (Caddy with a + // pinned TLS_DOMAIN, nginx with a server_name match, etc.). + let baseUrl; + const envBase = (process.env.PUBLIC_BASE_URL || '').trim().replace(/\/+$/, ''); + if (envBase) { + baseUrl = envBase; + } else { + const proto = (req.protocol || 'http'); + const host = (req.get && req.get('host')) || 'localhost'; + baseUrl = `${proto}://${host}`; + } + const buildLink = (newOffset) => { const params = new URLSearchParams(existingQs); params.set('limit', String(lim)); params.set('offset', String(newOffset)); - return `${proto}://${host}${basePath}?${params.toString()}`; + return `${baseUrl}${basePath}?${params.toString()}`; }; const links = []; diff --git a/tests/unit/pagination.test.js b/tests/unit/pagination.test.js index 697706f..cc2962a 100644 --- a/tests/unit/pagination.test.js +++ b/tests/unit/pagination.test.js @@ -3,7 +3,7 @@ // // Unit tests for the RFC 5988 Link header builder. -import { describe, test, expect } from 'vitest'; +import { describe, test, expect, afterEach } from 'vitest'; import { buildLinkHeader } from '../../app/middleware/pagination.js'; function fakeReq({ originalUrl = '/v1/customer/bycompany/1', host = 'api.example.com', protocol = 'https' } = {}) { @@ -73,4 +73,43 @@ describe('buildLinkHeader', () => { const link = buildLinkHeader({ req: fakeReq(), limit: 30, offset: 0, count: 100 }); expect(link).toContain('offset=90'); // last page anchor }); + + describe('PUBLIC_BASE_URL pins the Link header base', () => { + // Save + restore env so we don't leak into sibling tests. + const ORIG = process.env.PUBLIC_BASE_URL; + afterEach(() => { + if (ORIG === undefined) delete process.env.PUBLIC_BASE_URL; + else process.env.PUBLIC_BASE_URL = ORIG; + }); + + test('when set, builds Links against PUBLIC_BASE_URL (ignoring Host header)', () => { + process.env.PUBLIC_BASE_URL = 'https://node.timetrackerapi.com'; + // Malicious Host header should be ignored entirely. + const link = buildLinkHeader({ + req: fakeReq({ host: 'evil.example', protocol: 'http' }), + limit: 10, offset: 0, count: 50, + }); + expect(link).toContain('https://node.timetrackerapi.com/v1/customer/bycompany/1'); + expect(link).not.toContain('evil.example'); + }); + + test('trailing slash on PUBLIC_BASE_URL is stripped to prevent `//path`', () => { + process.env.PUBLIC_BASE_URL = 'https://api.example.com//'; + const link = buildLinkHeader({ + req: fakeReq(), + limit: 10, offset: 0, count: 50, + }); + expect(link).toContain('https://api.example.com/v1/customer/bycompany/1'); + expect(link).not.toContain('//v1/customer'); + }); + + test('empty / whitespace PUBLIC_BASE_URL falls back to req-based default', () => { + process.env.PUBLIC_BASE_URL = ' '; + const link = buildLinkHeader({ + req: fakeReq({ host: 'api.example.com', protocol: 'https' }), + limit: 10, offset: 0, count: 50, + }); + expect(link).toContain('https://api.example.com/v1/customer/bycompany/1'); + }); + }); });