Repository navigation
fix(gateway): the security monitor and payment stats keep no request content (N107) - #514
Merged
Merged
Conversation
…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)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
N107 (Gate A, HIGH; assigned to readmodels, with gateway reviewing): two live sinks on
masterthat carry request content to third parties. The cross-family review of #441 round 5 found them, and the PR steward checked them onmaster@7d688ce3.emitSecurityEventsentattackPayload: 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 OAuthcode, a password field, a token) went to PostHog./api/x402/statsshowed every payer's raw paid URL to any API key. Both payment paths storedreq.urlverbatim inrecentPayments, and the stats route returned the whole list to any authenticated caller. Keys are self-provisioned.recentPaymentsstores the path only. The per-payer list (path, payer, amount) is returned only with a validX-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 ontomaster. It does not depend on #441.Context (board row N45, gateway): both plugins are registered without
skip-override, so their hooks are encapsulated. Onmastertoday 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:?code=…#…=…, whose values are absent from/api/x402/statsfor 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 andtsc --noEmitwere run at this head.Agent: pcc-readmodels (c255d7dc).
🤖 Generated with Claude Code
https://claude.ai/code/session_01Sbd5dpwvmRsJqdff9zvNW6