Skip to content

Header allowlist for service logs โ€‹

What changed? โ€‹

PINO_REDACT_CONFIG stopped being a denylist of headers and became an allowlist. โ€‹

  • Before: five named headers were scrubbed and every other header a browser sent was logged.
  • After: three named headers are kept and every other header is dropped, request and response alike.
  • The operator's words on the choice: "Only keep what we use." Her standing rule the same day: "Best rule of thumb is to always follow signal's builds. Meaning, we shouldn't be collecting user agent either." A field is presumed out unless it justifies itself.

A denylist loses by construction, which is why the direction had to flip. โ€‹

  • pino-http serializes the entire headers object, so the default is collect. A denylist only ever catches headers somebody already thought of.
  • That is not theoretical here. user-agent was fixed on #1004, and the survey that followed found sec-ch-ua-platform (live value "Windows"), sec-ch-ua, sec-ch-ua-mobile, accept-language, referer, x-forwarded-for and forwarded all still landing. Client Hints are the successor to the user-agent string, so a user-agent-only fix ages out by design.
  • Both findings came from a human reading logs. The allowlist removes the need for that human.

What survives, and why each one? โ€‹

Every survivor is justified by a read in service code, not by taste.

HeaderRead whereWhy a log line needs it
hostreq.get('host') in all seven services, to build the OpenAPI server originNames which deployment served the request
originreq.headers.origin in every service's CORS blockA rejected origin is otherwise invisible in the log
content-typeservices/api/auth/src/routes/appeals.js:412, gating multipart parsingExplains a body-parse 400 or 415
  • The same three-name list is applied to res.headers as well, so set-cookie is now dropped by construction rather than by name.
  • LOGGED_HEADERS_ALLOWLIST is exported and frozen, so the list is one grep away and a test asserts its casing.

What got dropped? โ€‹

Everything else, including four headers that code does read. โ€‹

DroppedRead byWhy the log line does not need it
authorizationverifyFirebaseToken, schedulerAuthA credential. The middleware reads req.headers directly; log serialization never touches that
x-firebase-appcheckthe App Check middlewareSame class. An attestation token, useless in a log
x-forwarded-forExpress, to derive req.ip under trust proxySee the section below. Middleware unaffected
forwardednothingThe visitor IP again, in the other format

Two non-header additions in the same change. โ€‹

  • req.remoteAddress and req.remotePort are now censored. On Cloud Run the peer is Google's front end, but a locally run or directly invoked service sees the real client, and dropping x-forwarded-for while keeping the socket address would be half a fix.
  • This is the one item beyond a strict reading of "headers" and it is called out on purpose. It is trivially reversible: delete two lines from paths.

Eleven req.body.* paths were removed because they never matched anything. โ€‹

  • pino-http's request serializer emits id, method, url, query, params, headers, remoteAddress, remotePort. There is no body in it, at any point.
  • Left in place they advertise a protection that does not exist, and the next person adding a twelfth PII field thinks they are covered. A test now asserts no req.body path is present, so the decision does not quietly reverse.
  • req.query.phone and req.query.email were kept, because query IS serialized. Those two were live all along and are the only ones in this config that ever fired.

What happened to x-forwarded-for? โ€‹

The log line lost it. The middleware did not. Those are separable and only one of them changed. โ€‹

  • Express reads req.headers['x-forwarded-for'] itself to resolve req.ip under app.set('trust proxy', 1). Pino's serializer runs later, on a copy of the request shape, and fast-redact restores the original object after writing the line.
  • Rate limiting is proven intact by request, not by reasoning. See the live proof below.

The one thing an operator should know it costs. โ€‹

  • The earlier survey in user-agent-redaction.md flagged that an IP is the primary abuse-investigation signal and that dropping it from logs is a security tradeoff to make deliberately. This change makes it.
  • It is not the last copy. Cloud Run writes its own httpRequest.remoteIp on every request, which no application-side config can reach; whether that log survives at all is the separate decision in cloud-run-request-logs.md.
  • If she wants the IP back in our own line, it is one string in LOGGED_HEADERS_ALLOWLIST.

How was it proven? โ€‹

Sixteen tests, and the one that matters is the unknown-header test. โ€‹

  • packages/shared/__tests__/services/pinoRedact.test.js drives a real pino logger with the real config and asserts on the emitted line, not on the config array.
  • x-lantern-unknown-probe is the allowlist test. It appears nowhere in the config, in pino, or in any browser, and it must not reach the log. A longer denylist would fail it. That single test is the difference between the two designs.
  • Also pinned: Client Hints, user-agent, the IP headers, referer, accept-language, credentials, set-cookie on the response, the query censors, method / url / statusCode / responseTime surviving, and a log line with no req at all not throwing.

The casing trap has its own two tests. โ€‹

  • Node lowercases every incoming header name before it reaches req.headers, and res.getHeaders() is lowercased too, so a mixed-case allowlist entry would match nothing and fail silently. That is the shape the previous fix nearly shipped.
  • One test asserts every allowlist entry equals its own lowercase form. A second asserts the censor still matches a Content-Type key presented in mixed case, so a non-Node caller does not get silently stripped.

A live run against a real service. โ€‹

  • auth-api run locally from services/api/auth/src/index.js on port 8194 (non-default, to stay clear of the other lane), APP_CHECK_DISABLED=true, log captured to a file. This was a stack this session stood up, not the operator's.
  • One request carrying fourteen junk or sensitive headers: User-Agent, sec-ch-ua, sec-ch-ua-mobile, sec-ch-ua-platform, Accept-Language, Referer, X-Forwarded-For, Forwarded, Cookie, Authorization, X-Firebase-AppCheck, X-Lantern-Unknown-Probe, DNT, Origin.
{"level":30,...,"req":{"id":2,"method":"GET","url":"/health","query":{},"params":{},
 "headers":{"host":"127.0.0.1:8194","origin":"https://dev.ourlantern.app"},
 "remoteAddress":"[REDACTED]","remotePort":"[REDACTED]"},
 "res":{"statusCode":200,"headers":{"content-type":"application/json; charset=utf-8"}},
 "responseTime":1,"msg":"request completed"}
  • Grepping the whole log for all twenty probe tokens returned 0 for every one: LANTERNUAPROBE1004, Pixel 8, Chromium, Windows, sec-ch-ua, en-GB, PROBEREFERER, 203.0.113.7, x-forwarded-for, forwarded, NEVERLOGTHISCOOKIE, NEVERLOGTHISTOKEN, APPCHECKSENTINEL1007, UNKNOWNHEADERPROBE1004, x-lantern-unknown-probe, user-agent, authorization, cookie, dnt, RATEPROBE.

Rate limiting proven by request, on the same running service. โ€‹

POST /auth/phone/createUser carries ipRateLimit(10, 15 minutes).

RequestsX-Forwarded-ForResult
1 to 10198.51.100.10401 (limiter passed, auth rejected)
11198.51.100.10429 RATE_LIMITED, retryAfter: 900
12 and 13198.51.100.99401 again
  • The second row is the half that proves it. A different X-Forwarded-For gets a fresh bucket, so req.ip is still being derived from the header even though the header no longer appears in the log. If trust proxy had broken, every request would have shared one socket-address bucket and the .99 requests would have come back 429.
  • Neither 198.51.100.10 nor 198.51.100.99 appears anywhere in the log file.

What is still open? โ€‹

This is not deployed, and the predecessor commits are not either. โ€‹

  • Same standing finding as cloud-run-request-logs.md: the user-agent and App Check redactions sit unmerged on this branch, so deployed dev is still logging raw values today. This change joins them rather than fixing that.

req.url still carries the raw query string, and that is deliberate. โ€‹

  • pino-http sets req.url from Express's originalUrl, which includes ?phone=... verbatim. The req.query.phone censor does not reach it.
  • Blanking url would gut the logs, and url is the primary request diagnostic, so it was not touched. A route that accepts PII in a query param leaks it there regardless of this config, which makes it a routing question rather than a logging one. Recorded here so nobody assumes the query censors are complete.

req.params is serialized and untouched. โ€‹

  • Route params such as a userId reach the log. They are pseudonymous ids that already appear in url, so nothing is added by dropping one and not the other. Named so it is a decision rather than an oversight.

Built with VitePress