Report blocked attacks, with rule-scoped evidence - #200
Merged
Merged
Conversation
Detection reporting is retained security-event evidence for sites the platform manages. It now turns on for an enrolled site running managed rules with a credential, and stays off everywhere else — a local install, a guard running its caller's own bundle, a site with no credential. `reportingState` holds the whole decision as a pure function, so every input combination is enumerable in a test rather than reachable only by constructing a guard. Six states, each distinguishable at the platform, because "no events arrived" otherwise covers nothing matched, reporting switched off, never enrolled, and delivery broken. Two precedence rules carry meaning: An explicit opt-out outranks everything, so a deployment that switched reporting off is told that, not that it lacks a credential it never needed. `PATCHSTACK_REPORT_DETECTIONS` and `PATCHSTACK_TELEMETRY` stay distinguishable so an operator who set one is not sent to check the other. The credential is checked before the rule origin, because a missing credential is what causes managed rules to be missing: the fetch is refused and resolution falls back to the caller's bundle or to nothing. Asking about the origin first would report `no-managed-rules` for a site whose real problem is fixable. `reportDetections` becomes an opt-out only. It can switch reporting off; it cannot switch it on, because whether a site is managed is the platform's answer and a guard that could self-declare it would report against rule ids the platform never issued. Rule resolution now reports which leg supplied the rules in force — `api`, `cache`, `bundled` or `empty` — separately from whether resolution was clean. `cache` counts as managed: the rules came from the platform, just not on this call, and excluding it would silence reporting for the sites whose delivery is degraded and whose evidence is most worth having. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The state the platform receives is now the state the guard is in. The rules fetch carries the reporting state itself rather than a single bit, so "no events arrived" can be told apart from an explicit opt-out, a site that never enrolled, and a credential that did not resolve. A bit could express none of those, and it asserted the reassuring reading — reporting is on — in cases where it was not. Reporting also follows refreshes. It is recomputed from the origin each refresh resolves, and the reporter is started or stopped accordingly, because enrolment is not a boot-time fact: a guard that started on cached or bundled rules can receive managed rules later, and one that was reporting can have reporting switched off under it. Stopping flushes what is held — those events were collected while reporting was on. `detectionReporting` and `detectionHealth` are getters for the same reason. A property assigned once reported its boot value for the life of the process, including after reporting had started or stopped. The fetch before the first resolution carries the state derived from what the store already knows, which is the honest answer at the moment of asking: a site the platform has delivered rules to before reports managed, and a site with nothing cached reports that it holds no managed rules yet. `protect.d.ts` declares all six states and the `origin` that `refresh()` returns. Runtime tests cover the wiring rather than the calculator: default-on for a managed site with no config flag, each opt-out reaching the runtime, the state travelling on the rules request, and both directions of the transition. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The state carried on the rules fetch is decided before that fetch determines where the rules came from, so a site booting with an empty cache declares that it holds no managed rules and then receives them on the same request. The corrected state is now sent on an authenticated request of its own, because the alternative was waiting for the next refresh — and a guard with refreshing switched off has none. It carries no events; its only content is the state, and it is only made when resolution settled somewhere other than the fetch declared. A refresh derives the state it sends from the origin in force, re-reading the environment, rather than resending the last reported value. An opt-out appearing under a running guard now travels on the next request rather than the one after it. Reporting on by default for enrolled sites is a change in what an installed app does on the network, so the shipped documentation says so: `AGENT-INSTALL.md` and the option documentation now describe the default, the three ways to switch it off, and that `reportDetections` is an opt-out which cannot enable reporting for a site that is not enrolled. Two states cannot travel over this header and are no longer claimed to. `not-enrolled` makes no site-addressed request, and `unavailable-no-credential` cannot produce an authenticated one — the declaration is withheld from unauthenticated requests because a claim about a site carries no weight without a verified token. Both stay useful locally; server-side absence needs modelling on its own terms. The comments no longer say enrolment or the credential can be lost mid-process. The credential is resolved once at boot, and managed rules stay in the memory tier, so within one process the reachable changes are a rules source that starts being the platform's and an opt-out appearing in the environment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The event counters are measured in events. A state announcement carries none, so letting it advance `lastDeliveredAt` or `failed` produced readings that describe no real delivery — `sent: 0` with `failed: 1` — and made a capability acknowledgement indistinguishable from a delivered detection. `detectionHealth()` now reports a separate `capability` block: announced, acknowledged, failed, and when one was last acknowledged. Zero there alongside delivered events is a normal state, and so is the reverse. The declared type carries the shape. The shipped documentation describes the state-only POST, because it is outbound behaviour: what it contains, that it carries no detections, that it is sent once per process and only when resolution settled somewhere other than the rules request declared, and that a guard with cached rules never sends it. It also says which two states never travel and why they are local diagnostics. The refresh comment is narrowed to the reachable direction: a rules source can start being the platform's within one process but cannot stop, because an accepted bundle stays in the memory tier and a later failed fetch still resolves to cache. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every request that carries a reporting state declares it before resolution decides where the rules came from, so any of them can settle somewhere else. The boot path corrected that; the refresh path did not — so a guard that started on bundled or empty rules, recovered on a one-shot manual refresh, and never refreshed again left the platform holding the pre-resolution answer for the life of the process. Both paths now go through one helper that applies the settled state and acknowledges a mismatch, so they cannot diverge again. The refresh holds the state it declared in a binding and compares against exactly that, rather than recomputing it from the resolved origin — which would always agree with itself and never acknowledge anything. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…venance A client address is only as trustworthy as whatever supplied it. A transport peer address is observed and cannot be set by the caller; a forwarded header is ordinary request input, meaningful only when the request is known to have arrived through a proxy that sets it. `resolveClientIp` reports the address together with where it came from — `runtime`, `trusted-proxy`, or `unavailable` — and `unavailable` is both the default and a real answer. A forwarded header is read only when two things hold together. The peer must be one the deployment declared, because every direct connection has a peer and its existence proves nothing. And the chain is walked from the application side inward, stepping over declared hops to the first untrusted address, because a proxy appends rather than replaces — taking the client-most entry trusts whatever the caller prepended. A policy has to say who is trusted: `peers` as CIDRs, `hops` as a count, or an `isTrusted` predicate. A configuration naming only a header declares nothing. Where `peers` or `isTrusted` is present that verdict gates the peer; a policy declaring only `hops` states the trust numerically, so the count is evaluated on its own. `hops` counts from the peer, matching the numeric form of Express's trust policy, and is pinned across the range because a number copied from an Express configuration has to land on the same address. Trust configuration fails closed. One unparseable member invalidates the whole policy rather than being dropped, and a prefix accepts exactly one optional slash. Addresses are parsed strictly and reported canonically, so a value the validator accepted is the same value wherever it is matched or stored: IPv4 in dotted decimal with leading zeros refused, IPv6 lowercased and compressed, IPv4-mapped IPv6 reduced to the IPv4 it carries, a zone identifier only on IPv6 and only when it names something. A chain entry that is not an address stops the walk. There are no provider shortcuts. A provider's name does not establish that the provider overwrote the header — several document that a client-supplied value survives unless the service is configured to replace it, and one that does overwrite it does so only for requests that actually traversed it. A shortcut worth having encodes a policy verifiable at run time, which needs an adapter that positively establishes the runtime. Not yet wired into the guards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…se it A malformed field no longer changes the policy quietly. Previously only a broken `peers` list failed closed: a non-string `header` fell back to the default, and an invalid `hops` or `isTrusted` was dropped, so a configuration with one typo installed a policy the operator did not write and its behaviour did not reveal which part had taken effect. Every field that is present is now validated, and anything invalid rejects the whole policy. An explicitly empty `peers` list is a declaration that no peer is trusted, so it is refused rather than treated as an absent field. Read as absent, a hop count alongside it supplied the trust instead and the forwarded address was believed — the inverse of what was written. An IPv6 zone identifies an interface, so it is either honoured or refused, never silently discarded. The canonical spelling keeps it, because dropping it made two addresses on different interfaces compare equal. A policy entry may not carry one, since the zone plays no part in the numeric comparison and such an entry would trust those bits on every interface — including the one it was written to exclude. A zoneless prefix still covers a scoped address, which is what a prefix means. At most one zone delimiter is accepted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A trust policy is now read strictly from its own properties, and an unrecognised key rejects it. An inherited field was not written by whoever configured the object, and an unknown key is a typo rather than an extension — `heder` was ignored while the default header was used, which is the quiet substitution this function exists to prevent. A forwarded header must be an own property of the header bag. Read through the prototype chain, a polluted prototype elsewhere in an application could supply an attributed client address; the observed peer stands instead. A zone identifier follows a conservative grammar: interface names and numeric scope ids, up to 64 characters of letters, digits, dot, underscore and hyphen. Whitespace, newlines, path separators, brackets and non-ASCII are malformed addresses rather than addresses with decoration, because a zone reaches logs and retained event payloads. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An IPv4-mapped IPv6 address canonicalises to its IPv4 form, and an IPv4 identity has no zone to carry — so keeping such an address would mean discarding the scope silently, which is the one thing a zone must not do. It is refused instead, in both the dotted and hexadecimal spellings of the mapped form. The zoneless form is unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rule matching, the detection record and the block log each need a client address. Deriving it separately let them disagree, so one request could appear as several clients — worse than having no address, because each value looks authoritative. The address is now resolved once, where the request is shaped, and carried on the shaped object with its provenance. Every consumer reads that: The engine reads an explicit own value and nothing else. Reaching for the socket, or for a forwarded header, would answer differently from the consumer that resolved properly. The Express path presents the engine a prototype-linked view of the request with the resolved address as an own property. The application's own request is left untouched, because `req.ip` there is defined by the framework from its own `trust proxy` setting and overwriting it would change what the application sees. That value is never consulted: it is header-derived by a policy this guard has not verified. Response-phase detections carry the originating request's resolution, so a response detection names the same client as the request detection for that request. `.fetch()` shares one request screening with `fetchGuard()` to get it; `.express()` and `.node()` pass theirs into the response context. The standalone `screenResponse()` has no request phase, so it resolves once for itself. A detection carries the provenance alongside the address. Without it a consumer cannot tell an observed peer from a value read out of a header, nor a null address from one that could not be established. `trustedProxy` is the only configuration path, documented on the public option type. Behaviour change: an application behind a proxy sees the proxy's address until it declares a policy, and a runtime with no transport peer reports no address at all. Also restores a guard lost in an earlier revision of the resolver: the unspecified addresses parse correctly and identify nobody, so they are not reported as an address for a peer or out of a chain. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…through The Express view now materialises every field a rule can address as an own property. The engine normalises with a spread, which copies own properties only, so a prototype-linked view arrived carrying just the two properties added to it — and a rule scoped to a method, or reading an uploaded file or a parsed cookie, found nothing. Controls for all three. The transported detection carries the address and its provenance, through the same projection the rest of the code uses: `client_ip_source` always, `client_ip` omitted entirely when there is none. Asserted on the serialized request body for a trusted proxy, an observed peer, and an address that could not be established. That projection now also refuses an address whose provenance does not support it. `unavailable` means nothing was established, so an address arriving with that provenance is incoherent, and reporting it would attach a value to a claim that denies it. Both new fields are disclosed in the shipped documentation, including that the address is omitted rather than sent empty and that a forwarded header is never trusted implicitly. The payload contract test maps them, and states which field is conditional so its completeness check does not demand one the payload correctly omits. The standalone Node adapter forwards its caller's trusted-proxy policy, which it was dropping — so it always reported the socket peer even where a front end was declared. Pinned at the adapter level, with the no-policy answer as the control. `onDetect` declares the provenance alongside the address, so a consumer can use it without leaving the public contract. The response phase is covered: a response detection names the same address the request phase resolved, and the resolution is not repeated for it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Express view carries the verbatim request body. A raw view reconstructed by re-serialising the parsed body cannot carry what parsing does not keep: a body that is not valid for the type it declares, a duplicate key where only the last value survives, or the exact bytes a signature was written against. Raw rules need the bytes as sent. What the platform is told about a detection is the engine's account of it. The host callback receives its own object, and the rule on it is a deep clone: the rule the engine matches with is the policy in force, and enforcement lives in its nested parts — `rule_v2`, `when`, and the match and action objects inside them — which a shallow copy would still share. Where a rule cannot be cloned, the identifying fields are projected instead, because passing the live object would be worse than passing less of it. Each internal reporter builds its own record from the detection; reading before the callback runs is defence in depth for the case where the copy is weakened, not the control itself. The address projection states the payload invariant, so it validates rather than assumes. An address reaches an event only with a provenance that supports it, only if it parses, and only if it identifies somebody: a hostname, a malformed literal or an unspecified address is reported as `unavailable`, and an address that is emitted is emitted canonically, so one client cannot appear as two records because two hops spelled it differently. Resolution happens once per request. The response-phase test observes the resolved view itself, so a repeated resolution is visible on every path, including one with no transport peer. Rule fixtures are built per call, since nested rule objects are what a callback can reach and a shared fixture would let one test change the policy another enforces. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… the request
`onDetect` documents `rule` as `{ id, category }`, and that is now exactly what it
receives. The rule the engine matches with IS the policy in force: enforcement
lives in `rule_v2`, in `when`, and in the match and action objects inside them, so
any view that still shares them lets a callback change what later requests through
that guard are screened for.
Projected rather than deep-cloned because this path runs on every detection, which
an attacker can drive. On a 1 MiB rule, 1,000 detections cost about 1.2 s to clone
and about 0.1 ms to project, and the clone existed only to hand back fields the
contract does not promise. The internal reporters go on reading the real rule; only
the callback's view is narrowed.
Raw-body evidence is taken only from an own property, at the Express view and at
the engine's normalising funnel. Evidence is what a request actually carried; a
`_rawBody` reachable through a polluted prototype was carried by nothing, and
accepting it would let a single pollution stand as verbatim bytes and fire every
raw rule that matches it. Both layers are load-bearing: a shaped request still
inherits from `Object.prototype`, so stripping the value at one layer does not keep
it out of the other. The adapters that capture real bytes define `_rawBody` on the
request object itself.
Rule scope is on request in the wiring fixtures, so a view isolating `rule_v2`
while sharing `when` is caught by a second request rather than passing.
The shipped docs record the address behaviour for anyone upgrading: Express no
longer consults `req.ip`, an app behind an undeclared proxy sees the proxy's
address, and a Fetch runtime with no transport peer reports none.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…hange for Node A field that only `Object.prototype` supplies was carried by nothing, so it is not evidence. Gating `_rawBody` alone did not achieve that: the raw view falls back to serialising the PARSED body, so a polluted `body` still became verbatim bytes by that route and fired `raw` and `post.` rules alike. Every field both boundaries read — body, query, headers, url, originalUrl — now comes through one gate, at the Express projection and at the engine's normalising funnel. Both are load-bearing: the projection turns whatever it reads into an own property, so a value laundered there is indistinguishable from real data afterwards. The gate is the SUPPLIER of a field, not ownership of it. Frameworks expose real request data through getters on their own prototypes: `headers` is a getter on `IncomingMessage.prototype`, and Express defines `query` on its request prototype. Requiring an own property would discard those and silently stop screening headers and query strings — a worse failure than the pollution it prevents. So the chain is walked to whichever object defines the key, and only `Object.prototype` is refused. Fields the adapters create themselves keep the stricter own-property rule, because every writer of those is ours. Each field is covered separately at the funnel, since the Express projection strips these before the funnel sees them and a shared assertion would leave the non-Express paths unproven. The upgrade note now covers the Node guard as well as Express. Both previously took the address from a forwarded header — `X-Forwarded-For`, `CF-Connecting-IP` or `X-Real-IP` on the Node path, `req.ip` on the Express path — so both now show the proxy's address until `trustedProxy` is declared, which moves attribution and any rule reading `server.ip`. On a runtime with no transport peer no policy shape yields an address, and earlier versions reported a client-supplied one there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`Object.prototype` is not the only writable prototype, so refusing that one alone accepted anything installed closer to the request: a value parked on an intermediate prototype arrived as body, query and raw-body evidence the request never carried. An own property is evidence. Everything inherited is refused, with one narrow exception — a getter on a prototype, for `headers` and `query` alone. Those are the only fields a supported framework supplies that way: `headers` is a getter on `IncomingMessage.prototype` and Express defines `query` on its request prototype, while body parsers, cookie parsers and upload middleware all assign own properties and the Node and Fetch adapters build their shape as a literal. Requiring an own property everywhere would stop screening headers and query strings on real requests, which is a worse failure than the pollution it prevents. The exception is narrow in both directions. An inherited DATA property is refused however it arrives, because that is not how a framework delivers request data and is exactly how a write to a prototype presents. An accessor is refused for any other field, since no supported framework supplies one. And `Object.prototype` is refused even for an accessor: pollution can define a getter there, which would otherwise come through the one door left open. Each direction has its own test — the two framework getters separately, so a failure on one cannot hide the other, and refusals for an intermediate data property, an intermediate accessor on a field outside the exception, and an accessor on `Object.prototype` for a field inside it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A report is evidence, so losing a batch to a restart or a rate limit is worth another attempt. Delivery now retries up to four attempts per batch with exponential backoff and jitter, honouring `Retry-After` when the endpoint sets one. Three bounds make that safe rather than merely hopeful: Only a transient failure is retried. Unreachable, rate-limited or a server error gets another attempt; a batch refused on its merits does not, because retrying it spends the app's time to be told the same thing. Every attempt of one batch carries the same `Idempotency-Key`. An acknowledgement can be lost after the server has committed a batch, so without a stable key a redelivery would be counted twice and inflate exactly the numbers these reports are read for. A different batch gets a different key, or the server would discard it. One send is in flight at a time, and attempts are bounded. Memory stays at the queue plus one batch, a slow endpoint applies back pressure to the queue rather than to open sockets, and a batch that exhausts its attempts is dropped and counted where an unbounded retry would be an app quietly spending itself on an endpoint that will not take it. Nothing is scheduled after `stop()`, and retry timers do not hold a process open. A send now happens only when one is due — the interval elapsed, the batch bound reached, or a state waiting. Sending whenever the previous request finished would have applied the flush interval only to the first batch of a guard's life. Reporting states supersede rather than accumulate. They travel on the same transport, so they inherit the retry and the key, and declarations made in the same turn coalesce into one: what the platform needs is the state now, not how it got there. A state rides with queued events in a single request where there are any. Every field of an event is bounded, and an event that had to be shortened says which fields were shortened and how many parameters the rule really reads. The per-batch byte bound is set where a full batch of worst-case events can actually reach it — a bound above anything the field caps allow is a comment, not a bound — and what does not fit stays queued in order rather than being dropped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…n bytes `stop()` now answers everything that can be outstanding. A batch waiting on a retry timer holds the only send slot, so clearing that timer alone left it neither delivered nor counted: it gets one final attempt, with no retry behind it. A request in flight is aborted rather than waited on. Events still queued are drained a batch at a time, each attempted once, and whatever cannot be sent — including everything queued in a runtime with no usable `fetch` — is counted as dropped. Every recorded event now ends up delivered, refused or dropped rather than sitting in a queue that appears in no number. The body bound is measured on the request that is actually sent. It was measured on the events alone, using string length: the first leaves out the envelope and the drop count, and the second counts a multi-byte character as one byte, so a body over the bound on the wire passed the check. It is now the serialised request, in bytes. A retry of a state-only declaration no longer moves the event counter. Retries are counted against whatever the request carried — events, a declaration, or both — which is the same separation the delivered and acknowledged counts already keep. Any 5xx is retried, not four chosen statuses. A server error is the endpoint saying the fault is its own, and the documented contract says such a failure is retried, so abandoning most of the range on the first attempt was a difference nothing in the output would reveal. Each attempt is abandoned after ten seconds. With one send in flight, a request that never settles would hold that slot for the life of the process: the queue would fill, later events would be dropped for pressure, and the health counters would show one attempt that never failed. A parameter total is reported only when parameters were left out, rather than whenever any field was shortened, and a detection with no route keeps `null` instead of an empty string — an empty route reads as a known path that happens to be blank. The shipped documentation no longer says reports are dropped rather than retried, and no longer promises that a redelivery is not counted twice: Connect sends a stable key per batch, and what that key guarantees depends on the endpoint honouring it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…drain The reporter asked for a credential header without passing a timeout, and the exchange bounds itself with a timeout built from that value: absent, the construction throws, the exchange reports "no token", and the batch goes out with no `Authorization` for a site-addressed endpoint to refuse. Boot hid it, because the rules fetch happens first and primes the shared token cache — so the failure only appeared once that cache expired, or wherever the reporter ran without a rules fetch before it. It now passes a timeout, clamped to an attempt's own bound: the exchange is a separate request that an attempt's abort does not reach, so a longer bound would hold the single send slot past the point the attempt was meant to end. Detections also go through the shared Pulse path now, so a 401 discards the token and retries once with a fresh one. A credential can be rotated or revoked before the token's own expiry, which makes the server's refusal authoritative over our clock; the batch's headers travel through both sends, so the redelivery is still recognisable as the same batch. A batch still refused after that is a refusal on the merits and is counted, not retried. `stop()` returns a promise that settles when nothing is outstanding. The drain was already asynchronous, so a host shutting down had no way to wait for it and could interrupt the final attempt it had been promised. The wait is bounded by its own budget and remains best-effort — a runtime that terminates regardless still wins — and ignoring the promise behaves exactly as before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…uffer The budget only resolved the promise. The request stayed open, the queue stayed unaccounted, and later batches were free to follow — so the promise was reporting a completion that had not happened, and the test covering it proved only that the wait ended. The budget now ENDS the drain: the attempt is abandoned and counted, the queue is accounted for, and a response landing afterwards moves nothing, because the numbers have already been reported as final. All three outcomes are covered, since an acknowledgement, a retryable refusal and a refusal on the merits each take a different path out of an attempt. Writing that test found a related defect. An attempt cleared `inFlight.controller` in its `finally`, but on the acknowledged path the next batch is already in flight by the time that runs — so a batch the drain started could have its own controller cleared by the batch before it, leaving nothing able to abort it. The controller is now cleared only by the attempt that created it. `stop()` waits for the block log as well as the detection reporter. Its flush started a token exchange and a post and returned nothing, so the promise could resolve with records still outstanding — and resolve immediately in a configuration where the reporter it waited for was never built. The block log now returns its send, drains a batch at a time, and never rejects, so a caller that ignores it is unaffected and one that awaits it is waiting for the attempt rather than asking whether it succeeded. The credential exchange's bound is the reporter's own, fixed at one attempt's length. It read an app-wide option that no public option feeds, so the value was always absent and always fell through — a setting presented to callers who could not set it. A redelivery after a refused token is counted and reported as itself. The credential path can send a batch twice, which left the attempt count at one and the retry count at zero for a batch that went out twice. It is not a backoff retry, so it has its own number rather than being folded into one whose meaning would then need a caveat. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… knows about A flush that has already taken its batch leaves an empty queue behind it, so a shutdown looking only at the queue saw nothing to wait for while a token exchange or a post was still open. What is outstanding is the set of sends, not the contents of the queue, so the sends are now tracked and waited for. Stopping twice hands back the same wait rather than a second drain, or a resolved promise while the first one is still running. That drain is bounded, like the detection reporter's and for the same reason: a hung transport would otherwise keep a shutdown pending for as long as the process lived, which is not the bounded shutdown the contract describes. The budget both aborts and detaches — both request phases carry the signal, and aborting alone would still leave the wait depending on a transport to honour it. What the two reporters promise is now stated separately, because it differs. Detection events are delivered or else counted, and their health accounts for every one. Block-log records only have their outstanding attempts completed: that path keeps no counters, so a record lost to a failed exchange or post is reported nowhere, and saying otherwise claimed an accounting that does not exist. A redelivery after a refused token is counted as the second send is made, under the attempt's own epoch, rather than after the credential path returns — otherwise a refresh completing after a shutdown had reported its numbers as final could still move one. Termination also clears the attempt's own timer, which outlives the shutdown budget when a transport ignores an abort, and accounts for a reporting state still waiting for a request it will now never get. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The shutdown controller was created inside `stop()`, by which time the sends worth ending had already started with no signal at all — so the budget could not abort the very requests the tracking set had been added to find. One controller now covers every request the reporter makes, for its whole life. Not one per send either: the token exchange is shared, so a second send awaits the first send's request, and a signal belonging to the second would not reach it. Racing the wait against the budget ended the wait but left the work alive: still blocked, still holding its entry, and free to run another flush if the transport answered later — after the shutdown had reported itself finished. The budget now ends the reporter. It is marked ended, which closes the send, the flush and the drain loop; the open requests are aborted; and what was never sent is discarded rather than retained by a reporter nobody will read again. The public wording no longer claims the underlying requests completed. An abort is a request to stop, not a guarantee, so a transport that ignores it is detached: the promise resolving means the reporter is finished with it. What is accounted for is also stated per reporter, because it differs — every detection event ends up delivered, refused or dropped and appears in the health counts, while block-log records have no counters and a lost one is reported nowhere. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An armed flush interval could fire mid-drain, take the queued records for itself, and start a send the drain never learned about — so the drain looked at an empty queue, concluded it was finished, and resolved with that send still open. The interval is now cleared as the shutdown starts, before anything else: from that point the drain is the only thing that takes from the queue. The drain also asks again after every wait instead of waiting on one snapshot of what was in flight. A send holds records that have left the queue and the queue holds records that will become a send, so either can produce the other; a snapshot is satisfied as soon as the sends it happened to capture are done, whatever appeared meanwhile. It ends when both are empty, when a pass cannot shrink the queue, or when the budget ends it. A finished send now removes itself in its own `finally`, before its promise settles, so a waiter looking at the set the moment its wait resolves cannot see a send that has already completed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A detection says a rule matched. That is enough to count hits and not enough to act on: whoever triages it still has to decide whether the request was really an attack, and for that they need to see what the rule saw. It is also the point where a security channel could quietly become a copy of an application's traffic. So capture is not a switch. What may be captured is derived from the rule, and a rule earns each permission by naming what it reads. A named parameter permits its own value, because the rule was written to inspect it. A prefix permits the keys that match it and no others, bounded by a count, so that a rule reading a prefix does not become a rule reading the whole body. `raw` and `all` permit nothing: they read the entire request, so deriving permission from them would have the broadest rules granting the broadest capture. Response sources permit nothing either — the phase that reads them exists to redact secrets, and capturing them would collect the values that redaction is there to stop leaving. Raw request bytes need an explicit opt-in on the rule, bounded regardless of what the rule asks for. A plan covers the union of everything its rule reads, because the engine reports which rule matched and not which of its conditions did; a plan claiming clause-level precision would be claiming a precision the detection does not have. It is derived from the immutable revision and cached against it, and carries a content-derived reference, so a captured value can be traced to the policy that permitted it rather than to whatever the rule says by the time someone looks. An unreadable rule permits nothing, in every direction tested: failing to understand a rule must never be the reason something gets captured. Nothing is wired. This module is imported nowhere, no value is captured, and the payload is unchanged — the policy is reviewable before anything can act on it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contract version 2.8. The contract now describes `capture`, so a consumer holding an earlier copy holds a different document and the version says so. `capture` is a rule property the contract defines, validates and publishes as structure rather than prose: the keys it takes, that `version` is required and must be one this guard implements, that `raw_chars` is a required positive integer, and that no other key is accepted. A consumer implements against that instead of reproducing the check in its own language. The ceiling on `raw_chars` is published beside the schema, not inside it. Asking for more than the runtime delivers is a request rather than an authoring error, so the schema accepts any positive integer and the runtime answers with the ceiling; a schema maximum would have a consumer refuse a rule this guard accepts and runs. Capture validity governs collection and never protection. A rule is a mitigation, so a `capture` this guard cannot read grants no capture and leaves the rule running, including when it is authored as null. The properties exempt from the null rule are published, because a consumer has only the artifact and an exception it cannot see is one it cannot honour. Deciding a rule's fate on evidence metadata would let a newer server, by adding a capture version, switch shielding off on every older guard. What a rule permits is derived from the contract's own grammar: `parameterProblem` and `SOURCES` decide what a parameter is, so a permission cannot exist for something the engine cannot read, and there is no second grammar to keep in step. A parameter list is one level deep, which is what the engine expands — a member that is itself a list resolves to nothing, so a rule holding one names parameters and matches on none, and the contract refuses it rather than accepting protection that reads as present. A permission exists to explain a detection, so the question a plan asks first is whether this is a rule the guard would run — the validator's own judgement, minus capture validity. A rule naming a parameter with no match is refused there and authorises nothing here, while a rule matching on the whole request carries no parameter at all and is perfectly able to fire. The plan reference covers the whole plan, limits included, since two plans naming the same parameters under different bounds are different permissions. It is 128 output bits from four FNV-1a lanes: not cryptographic, and not built to resist anyone trying to collide it, but comfortably wide for the accidental case. The algorithm and canonical form are what `cp1-` means, so a vector over a literal plan pins them — literal, so that a future change to the limits gives a different reference under the same algorithm rather than reading as a reason to change the prefix. Plans are keyed on the rule object, because a revision identifies a version of one rule rather than a rule, and two rules sharing one must not share permissions. Plans and the limits are frozen: the reference names a set of permissions, and permissions that can change afterwards leave it naming something else. Still wired nowhere, and still capturing nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ded by A plan says what may be captured. This reads it: only the parameters the plan names, in the plan's order, and through the resolver handed in from the match — never one built here. A request can answer differently every time it is asked, so a second reading records a value the rule never saw, and evidence disagreeing with the match it belongs to is worse than no evidence. There is no request and no factory to read one with, so there is nothing here that could diverge; the engine hands out the resolver its decision was made on. Which form of a parameter that is: three exist. The engine normalises a request — URL-decoding, entity-decoding, stripping comments and control characters, collapsing whitespace — and then a condition applies whatever mutations it asks for. This records the middle one, the resolver's answer. So a percent-encoded payload arrives decoded, while a rule's own `base64_decode` is not applied and its subject arrives still encoded. Reproducing what the matcher finally compared would need condition-level evidence: the engine reports which rule fired, not which condition, and this plan deliberately does not model one. Absence and failure are different answers, and the result says which. A field that was not there, a value refused for its type, a parameter whose read threw, and a request nothing could be read from would otherwise arrive alike as "no evidence", letting a reviewer treat incomplete evidence as complete. `unavailable` means no read completed at all — no resolver, or every permitted read threw — which is the opposite conclusion from a request that simply carried none of what the rule reads. One parameter failing among several is partial, not unreadable. The counts carry no content. An empty string is a value. A rule can be written so that its finding IS that a parameter is empty, and an absent field already resolves to no value at all. What a value is, is settled before there is any question of room for it — for named values and for prefix values alike. Otherwise "refused for its type" and "left out by a bound" swap places depending on where a value falls in a request, which misreports why the evidence is short. Every value a bound excludes is counted, not merely the first: one named parameter can resolve to many values, as several files uploaded under one field name do. The bounds are named for what they count. `capturedValues` covers named and prefix values together; raw is bounded by its own opt-in and does not draw on that budget, since making it consume a value slot would have an opt-in silently reduce the named evidence a reviewer needs. `prefixValues` counts resolved values rather than matched keys, because the resolver answers a wildcard with values and a bound on keys is one this cannot enforce. Renaming them changes the canonical form, so the plan reference reads `cp2-`. Still wired nowhere, and still capturing nothing: the payload is unchanged and the disclosure still says no values of any kind, which remains true until this is connected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Counting that a rule fired is not enough to act on it: whoever triages a detection still has to decide whether the request was really an attack, and for that they need to see what the rule saw. A detection now carries the values the matched rule's own plan permits. The evidence is derived at the moment of the match, from the reading the match was decided by, and the resolver stays inside that function. What leaves is the bounded result — a fixed number of values, each of fixed length, and counts of what did not fit. Handing the resolver onward would put the whole request within reach of every consumer of a detection, which is the opposite of a plan that names what may be read. Every report names the policy that allowed it, and does so even when the policy allowed nothing: "this rule was permitted to show you nothing" and "this rule showed you nothing" are different facts, and only the first is policy. The host callback does not receive it. The documented callback carries the rule's identity and the request's own metadata, and a host already holds the request these values came from, so widening that contract is a decision of its own rather than a side effect of collecting anything. The shipped disclosure now describes what a report can contain rather than promising it contains no values. It states which parameter a value may come from and on whose authority, that a whole-request read grants nothing, that response values are never captured, that raw bytes need a reviewed per-rule opt-in, what every bound is, that the bounds report themselves, and that each report names its capture plan. The exclusion is scoped to the parameters a rule does NOT name, because the ones it names are the point. The contract tests now drive real evidence through the real plan and extractor, with a rule naming two planted values and not the rest — so the payload is the boundary itself. The values a plan permits are asserted present, because a list of absences taken from a payload where capture never ran would prove nothing. `capture` is inside the field-disclosure gate, which refuses a payload carrying a field the shipped docs do not describe. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…verywhere Every phase names its capture policy. A response detection carries one like a request or an egress detection: response sources are never capturable, so a response-only rule reports a plan that permitted nothing — the policy, which is a different fact from a rule that found nothing. A response rule scoped on the request captures what that request parameter holds, because those sources are capturable wherever they are read from. Evidence is bounded again at the wire. A parameter NAME comes from the rule and rules carry no length limit, so a label alone can carry an event past the body bound — and a batch always sends at least one event, so an event that cannot fit could never be delivered at all. The reporter re-applies every bound rather than trusting whatever produced the capture, and marks what it shortened. The baseline every detection carries is the settled one: the request's method, its path, the query string's parameter names, the user agent, the client address with its provenance, and a timestamp. The user agent is the one header value that travels, because attribution is what the channel is for and a detection without it cannot be told from another client's. The query travels as NAMES, not as a URL. A verbatim query string carries the values of parameters the matched rule may never have named, which is what the capture plan exists to prevent; the path plus the query's parameter names says what was requested without saying what was in it. The shipped type declaration and the reporter's own module documentation now say what the payload carries, in the same terms as `AGENT-INSTALL.md`. Both are read as authoritative — one by an editor, one by a reader of the source — and the disclosure guard covers all three, so a privacy statement cannot be current in one place and stale in another. Two statements corrected. A `capture.plan` identifies the PERMISSIONS rather than the rule: two rules reading the same parameters share one reference, and the rule document is identified by `rule_id` with `rule_revision`. And capture is the union of everything a rule reads, not the condition that fired — the engine reports which rule matched, not which of its conditions did, so a reader needs to know that a broad rule captures what it is broad about. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…hes the wire The query survives to the reporter on every path. The reporter is what removes the query's values while keeping its parameter names, so a path that trimmed the query before then would leave a Fetch or a response detection unable to say what was requested — working on Node and Express and empty on the runtimes this exists for. An egress detection carries the outbound request's own method and path. It carries no client address and no user agent by nature: the call is the application's, not a visitor's, so there is nobody to attribute it to. Its destination host reaches the report through capture, for a rule that names `egress.url` or `egress.host`, which the internal-host rules do. The User-Agent is disclosed as what it is: the one header value that travels whether or not a rule names it, because attribution is what this channel is for and a detection nobody can attribute is of little use. It is the single exception to the rule-scoped policy, stated in the shipped docs, the shipped type declaration and the reporter's own documentation — a carve-out stated in one place is not stated at all — and the disclosure guard pins it in all three. Every shortened field says so. The method, the user agent and the query-key names each report their own cap, and a query with more keys than the event carries reports the total, so a short list is never read as a complete one. The wire gate validates rather than coerces. `String(x)` runs whatever `toString` an object carries, which would turn a value this channel refuses into reportable content — so a plan that is not a string, a parameter that is not a string, and a value outside the documented scalar types are refused, and each refusal is counted against what a bound left out. The gate reads the capture once: reading it twice would let a getter answer differently between the check and the send. What the gate drops is added to the count the producer already made. The documented promise is that values excluded by a bound are counted, and a reader cannot otherwise tell eleven values from two hundred. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s own
A capture is an ordinary object, so a write to `Object.prototype` supplies any field
it does not carry itself: `Object.prototype.raw = { value: … }` would have a plan
that permitted nothing transmit that value. Every field of a capture and of each of
its entries is now read as an own property, once. This guard shields applications
against prototype pollution; its own reporting must not be the way one lands.
A capture with no plan of its own is refused entirely rather than borrowing one — a
report naming someone else's policy would be worse than a report naming none.
Exclusions are counted where they belong. A value left out because the event was full
is omitted; a value refused because its type is not one this channel reports is
unsupported, and invalid raw evidence is accounted for the same way. Merging them
would leave a reader unable to tell a full event from a refused value, which the two
counters exist to separate.
An egress detection has its own baseline, and the documentation states it as one. It
carries the outbound method and path, the query's parameter names, and a timestamp,
and it carries no user agent and no client address, because the call was the
application's own and there is no visitor to attribute it to. A single "every report
carries" claim was false for it in both directions.
The query-value exclusion is qualified as being about baseline metadata. `route` and
`query_keys` describe a URL without disclosing what was in it — but a rule naming
`egress.url` reads the outbound URL, so its capture carries that URL as the rule read
it, query values included. That is the rule-scoped policy working rather than an
exception to it, and stating it unqualified would have been an overclaim in the
direction that flatters us.
`query_keys_total` says how many query parameters a request carried when the event
lists fewer, documented beside `parameters_total` and inside the field-disclosure
inventory — with the fixture that exercises it actually carrying more keys than one
event holds.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…name a query parameter as the guard does Every surface that lists what a report carries now separates the two. The shared baseline is the rule and bundle identity, the phase, whether it was enforced, the path, the query's parameter names, the method and a timestamp. Two fields depend on the phase: a request or response detection also carries the user agent and the client address with its provenance, and an egress detection carries neither, because the call was the application's own and there is no visitor to attribute it to. Said in the shipped docs, the shipped type declaration and the reporter's own documentation, since a qualification stated in one place is not stated at all. Reported query names are the names the guard addresses a parameter by. They come from `URLSearchParams`, which is what resolves a query parameter for a rule: `+` is a space, percent sequences are decoded, and an invalid sequence is left as written. A name decoded by hand reported `first+name` for a parameter every rule addresses as `first name` — a name nobody could look up. `query_keys_total` counts DISTINCT names, which is what the list beside it holds: a parameter repeated three times is one name to look up, and a total that counted repeats would not describe the list it accompanies. Documented as that. The disclosure guard checks each section that lists these fields, rather than checking that the qualification appears somewhere in the file. One section being right while another promises a user agent on every detection is exactly the failure a file-wide match cannot see. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
It answers with the names and a count of distinct names, so the annotation says that rather than a list — an editor infers from it, and one that disagrees with the function is worse than none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Publication is a button press: `Release` cuts a tag and dispatches `Publish`, and a published version cannot be withdrawn from anyone who already installed it. A stop line in a pull request is read once, by one person, so this is a file both release workflows refuse to run past — checked immediately after checkout, before anything can tag or publish. `.publish-hold` names the two server-side changes this package's behaviour depends on: the detections endpoint deduplicating on `Idempotency-Key`, and ingest accepting the current payload. Lifting the hold is a commit someone reviews, naming the change that made it safe. The gate is tested where it can be: that both workflows check the file, that the check is the first thing after checkout, and that the step fails the run rather than warning — asserted against that step's own body, since `exit 1` appears elsewhere in these files and a whole-file match would pass for a hold that printed a warning and carried on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Detection reporting depends on two server-side behaviours: the detections endpoint deduplicating on `Idempotency-Key`, and ingest accepting the current payload. They belong in `RELEASING.md`, which is what someone reads at the moment they cut a release, with what goes wrong if the check is skipped. Publishing already requires a maintainer to run the release workflow deliberately, so the decision has a person in it either way. A workflow that refused to run would block every release rather than this one — including an unrelated fix — and would be deleted by whoever hit it under time pressure, which discards the reasoning exactly when it matters. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
This PR is too large for detailed analysis—consider breaking it into smaller, focused changes. 🎯 oversized — strongly consider breaking this down 🛡️ Standards: no pre-flight fit check ran for this change — wire |
Contributor
Author
|
/review |
daniloradovic
approved these changes
Sep 1, 2026
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
2 participants
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.
Do not publish before two server-side changes land
This changes what leaves an application, and a published version cannot be withdrawn from anyone who has already installed it.
RELEASING.mdcarries the same two items where a release is actually cut.Idempotency-Key. Connect sends a stable key for every attempt at a batch and a fresh one per batch, so a redelivery is identifiable — but whether it is counted once is the endpoint's to decide. Published ahead of that, a retry after a lost acknowledgement inflates the very counts these reports are read for.captureobject, the baseline fieldsmethod,user_agent,query_keysandquery_keys_total, andreporting_stateon the detections body. An endpoint that rejects or silently drops them turns every report into a delivery failure, or into a record missing the evidence it was sent to carry.Merging is safe: reporting is gated on a site being enrolled and running platform-delivered rules, so nothing changes for a local install or for a guard running its own
rules.What this adds
Blocked-attack event reporting, built as four pieces.
Enrolment gate and capability states. Reporting is on by default once a site is enrolled and running platform-delivered rules with a resolvable credential; off for a local install. Six states —
on,disabled-by-config,disabled-by-telemetry-opt-out,not-enrolled,no-managed-rules,unavailable-no-credential— with the four that can travel declared to the platform and re-declared after a refresh through one shared apply-and-acknowledge path.Client address with provenance. One resolution per request, shared by rule matching, block logging and detection reports so they cannot disagree about who a request came from. A forwarded header is never believed implicitly:
trustedProxydeclares the proxies you run, the chain is read from the application inward to the first undeclared hop, and there are no provider presets. Addresses are canonicalised, and one that identifies nobody is refused.Bounded delivery. Batched, queue-bounded, retried up to four attempts with jittered backoff honouring
Retry-After; only transient failures retry. One request in flight at a time, each attempt abandoned after 10s.stop()returns a promise that settles when both reporters are finished or their budget elapses — and the budget ends the drain rather than only ending the wait. Health counts what was attempted, acknowledged, refused, dropped, retried and reauthorised.Rule-scoped evidence capture. A detection can carry the values of the parameters the matched rule names — because counting that a rule fired is not enough to triage it. What may be captured is derived from the rule, never configured per site:
post.f*)raw/allresponse.*captureopt-inCapture is the union of everything the rule reads, not the condition that fired, since the engine reports a rule and not a clause. Evidence is read through the resolver the match was decided by — never a second read of the request. Bounds report themselves: values excluded by a bound, refused for their type, or unreadable are counted separately, so a short list is never mistaken for a complete one. Every report names the permission plan that allowed it.
captureis a versioned rule-contract property (contract 2.8); a capture this guard cannot read grants no capture and leaves the rule enforcing, so a future capture version can never switch shielding off on an older guard.Disclosure
AGENT-INSTALL.mdno longer says a report contains "no values of any kind" — it says which values, on whose authority, every bound, and the exceptions: the User-Agent travels on a request or response detection whether or not a rule names it, and a rule namingegress.urlcaptures that URL as it read it, query values included. The shipped type declaration and the reporter's module documentation say the same thing, and the disclosure guard checks all three, per section.The payload-contract test drives real evidence through the real plan and extractor with a rule naming two planted values and not the rest, asserting both that the named ones are sent and the others are not — an exclusion list taken from a payload where capture never ran would prove nothing.
Verification
2206 tests passing, typecheck, build and rule-contract check clean. Every fix in this branch was mutation-tested; where a control turned out not to be individually observable, that is stated in the commit rather than implied.
Known, not blocking
The map/canary tests time out intermittently at their 5s limit under full-suite load. Pre-existing, worth tracking separately.