Skip to content

fix(gateway): the security monitor and payment stats keep no request content (N107) - #514

Merged
LamaSu merged 2 commits into
masterfrom
fix/n107-telemetry-sinks
Oct 3, 2026
Merged

LamaSu merged 2 commits into
masterfrom
fix/n107-telemetry-sinks

Conversation

@LamaSu

@LamaSu LamaSu commented Oct 3, 2026

Copy link
Copy Markdown
Owner

Summary

N107 (Gate A, HIGH; assigned to readmodels, with gateway reviewing): two live sinks on master that carry request content to third parties. The cross-family review of #441 round 5 found them, and the PR steward checked them on master @7d688ce3.

  1. The security monitor sent raw request content to PostHog. On any pattern match, emitSecurityEvent sent attackPayload: up to 500 characters of the raw URL (query included), a header value, or the body, only HTML-escaped. The fingerprint also carried the full Referer. A credential in that content (an OAuth code, a password field, a token) went to PostHog.
    • Now: an attack event carries the attack type, its source, the content's length, and the request path without query or fragment. It never carries the content. The Referer is cut to its path, and the monitor's own log lines carry the path, not the URL.
  2. /api/x402/stats showed every payer's raw paid URL to any API key. Both payment paths stored req.url verbatim in recentPayments, and the stats route returned the whole list to any authenticated caller. Keys are self-provisioned.
    • Now: recentPayments stores the path only. The per-payer list (path, payer, amount) is returned only with a valid X-Admin-Key; other callers get the counts. No client in apps, scripts, packages or docs reads this route.

This is the same commit as #441's 0a92f6cf, cherry-picked onto master. It does not depend on #441.

Context (board row N45, gateway): both plugins are registered without skip-override, so their hooks are encapsulated. On master today the monitor scans only its honeypot routes, and the payment gate runs only on its own routes. These sinks widen when N45 arms the plugins, and this PR closes them first.

Tests

packages/gateway/src/__tests__/n107-telemetry-sinks.test.ts (5 tests) covers:

  • a JSON-body credential, a URL query value, a cookie, a Referer query and a path fragment, none of which reaches the captured PostHog events;
  • a verified legacy-x402 paid request with ?code=…#…=…, whose values are absent from /api/x402/stats for another key, while the list stays visible to an admin.

On master's own sources, all 5 tests fail; with this change, all 5 pass. The full gateway suite and tsc --noEmit were run at this head.

Agent: pcc-readmodels (c255d7dc).

🤖 Generated with Claude Code

https://claude.ai/code/session_01Sbd5dpwvmRsJqdff9zvNW6

LamaSu added 2 commits October 3, 2026 01:46
…content (N107)

Cross-family review r5 of #441 (rm-px7-441-r5-179c4777), CRITICAL 2 and 3,
split out as N107 (PR steward #5254). Both sinks are live on master too, and
these files are byte-identical on master 7d688ce.

- The security monitor sent PostHog the raw request URL, query parameter,
  cookie or body as an attack event's attackPayload. Every event's
  fingerprint also carried the raw Referer, and a path that kept its
  fragment. Now:
  - an attack event carries a summary: its type, its source, the path with
    no query or fragment, and the content's length (attackLength);
  - the Referer is reduced to its URL with no query or fragment;
  - the monitor's own log lines carry the path instead of the URL.
- The payment gate kept each paid request's raw URL in recentPayments, and
  GET /api/x402/stats showed the list to any caller. It now keeps the path
  with no query or fragment, and only an admin (X-Admin-Key) sees the list.
  Everyone else gets the counts. No client reads this route.

Reproduced first: n107-telemetry-sinks.test.ts failed 5 of 5 at 179c477,
with every marker sent under an arbitrary name. Each fix was
mutation-checked with file copies: undoing any one of the five fails its
test.

agent: pcc-readmodels (c255d7dc)
…or log or payment row keeps a caller's value (N107 r2)

Cross-family review r1 of #514 (rm-n107-514-r1-081b0c49, DO-NOT-SHIP). Each finding was reproduced at
081b0c4 by n107-r1.test.ts before any fix: 3 of its tests failed.

- CRITICAL 1: every fingerprint sent PostHog raw header values (User-Agent, Accept-Language,
  Content-Type, cf-*, X-Forwarded-For, x-railway-edge), and the Referer kept its userinfo.
  buildFingerprint is now a closed schema of derived values:
    - the client is a keyed hash (per-process key), which PostHog's distinct id uses too;
    - the User-Agent is a class (browser, http_library, scanner and so on);
    - the language is its primary subtag;
    - the Referer is a kind (direct, same_origin, cross_origin, invalid);
    - the content type is an allowlisted media type, and the country an ISO code;
    - the edge is an enum, and the forwarding chain a hop count;
    - the path is the matched route's pattern.
- CRITICAL 2: the monitor's logs carried User-Agent text (HONEYPOT's ua, and bot reasons). Bot
  reasons now name the signal only. The log lines carry the validated address, the route and the
  UA class.
- MEDIUM 3: MPP storage, both protocols' 402 paths, and exact counter deltas had no test.
  n107-mpp.test.ts (a scripted MPP charge handler: a 402, a success, a failed charge) and the
  legacy case in n107-r1.test.ts now pin them. Both passed at 081b0c4 too: the behaviour was
  right, and only the tests were missing.
- recentPayments now keeps the route's pattern, not pathOnly(req.url). An encoded separator
  (%3F, %23) or a value in a path segment can no longer reach the stats. This is the same class
  as cross-family review r6 of #441, MEDIUM 3.

Mutation checks on file copies, each restored and checked with cmp, fail the tests:
- a raw User-Agent in the fingerprint: 2 tests fail;
- the User-Agent back in the HONEYPOT line: 1 fails;
- the raw URL in a legacy payment row: 1 fails.

agent: pcc-readmodels (c255d7dc)
LamaSu added a commit that referenced this pull request Oct 3, 2026
…ity redaction (#441 r7)

This fixes cross-family review r6 of #441 (rm-px7-441-r6-57233724), MEDIUM 3: the value-free rule
found a query or fragment only by a literal ? or #. So "/cb%3Fzq1%3D<value>" kept its value in
strings sent to Sentry, in the request log and in the audit row's URL.

Reproduced at 5723372 by observability-r7.test.ts before the fix: 2 of 2 failed, the verdict's
redactCredentials case and the request log plus audit row through the real createGateway.

observability-redact.ts now reads %3F, %23, %26 and %3D (any case) as ?, #, & and = wherever a rule
looks for separators: in withoutQueryValues (the request log's serializer, extra.url and the audit
URL), in each URL-like token of any string, and in the whole-string form rule. The output shows them
decoded. A mutation check confirms the tests catch it: with 5723372's redactor, both new tests fail.

Under the PR steward's strict ruling (#5315, #5319), the redactors stay as defence in depth. The
closed producer schema (the monitor's fingerprint, PostHog's distinct id, allowlisted header values,
generic property values) is built in #514, and #441 rebases onto it. Those r6 findings were reproduced
at 5723372 and are not changed here.

agent: pcc-readmodels (c255d7dc)
@LamaSu
LamaSu merged commit b75ff12 into master Oct 3, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant