Keep a detection's rule identity, and let a guard be shut down - #164
Keep a detection's rule identity, and let a guard be shut down#164patchstackdave wants to merge 2 commits into
Conversation
Four fixes to the reporting and refresh paths, all cases of something reporting success while doing nothing useful. **A detection named the wrong rule bundle.** The reporter was handed the ETag read at boot and never heard about a refresh, so once a guard picked up new rules every later detection still claimed the old bundle. Anyone following that up would open a rule document that may not even contain the rule that fired. The identity is now mutable and re-stamped after an accepted swap — and only after one, so a refresh that fell back to cached rules keeps the previous identity, which is the one still true. **A reporting-only guard could not be stopped.** The stop method was installed only when a refresh loop or a block log existed, so a guard configured for detection reporting alone had a live reporter nothing could reach: its last batch depended on a timer, and a clean shutdown was impossible. There is now one `stop()`, always present, that stops the loop, the block log and the reporter and flushes what is buffered. `stopRefresh()` is kept as an alias. **Reporting with no credential posted into a refusal.** The detections endpoint needs a verified, site-bound token. With `reportDetections: true` and no credential resolvable, the guard now starts no reporter, warns once, and says `detectionReporting = 'unavailable-no-credential'` rather than spending a request per batch on nothing. `detectionHealth()` reports local counts — attempted, acknowledged, refused, dropped — and the last acknowledgement, so a delivery path that refuses everything is distinguishable from an app where no rule fired. **The refresh poller did not back off.** Resolving rules deliberately absorbs an API or network failure into cached or bundled rules, so the tick never threw and the scheduler read a fleet-wide outage as a healthy poll — every installed guard kept knocking at its normal interval for as long as it lasted. Rule resolution now reports whether the rules came from the source, the loop backs off on that, and `refresh()` resolves with the same answer instead of rejecting. Also corrects source comments that still described a rules or detections fetch as something the server accepts without a credential. Tests: the new guards are each mutation-checked — remove the re-stamp, the credential gate, the always-installed stop, the delivered/refused split, or the backoff, and exactly the test naming that guarantee fails. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Adds rule revision tracking and unified shutdown for detection reporting. 🎯 Quality: 88% Excellent · 📦 Size: Extra Large — strongly consider breaking this down 🛡️ Standards: no pre-flight fit check ran for this change — wire 📈 This month: Your 116th PR — above team average · Averaging Good |
The bundle ETag says which bundle a detection came from, and it changes whenever anything in that bundle changes — a rule added, one withdrawn, the site's enforcement flipped. So it cannot answer the question a reviewer actually has about one rule: are these counts evidence about the document this rule has now? Each served rule can carry its own `source_revision`, and a detection now forwards it. Forwarded, not derived: the side that served the rule knows which document it is, and a value computed here would be this client's opinion of it. Null when the bundle carried none — a customer's own rule has no revision — because "cannot say" has to stay distinguishable from a revision that no longer matches. The field is an identifier, like the rest of the payload, and the shipped disclosure names it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
One addition here, from the round-two finding on the platform side: detection counts were being judged against the whole bundle ETag, which changes whenever anything in the bundle changes — an unrelated rule added or withdrawn, site enforcement flipped — so a rule's own history was marked stale for reasons that said nothing about that rule. A served rule can now carry its own It is an identifier like the rest of the payload, and the shipped disclosure names it — the payload-contract test fails if a field is emitted without being described.
|
1 participant
Four fixes on the reporting and refresh paths. Each is a case of something reporting success while doing nothing useful.
A detection named the wrong rule bundle
The detection reporter was built once with the rule-bundle ETag read at boot and never heard about a refresh. So after a guard picked up new rules, every later detection still claimed the old bundle — and anyone following one up would open a rule document that may not even contain the rule that fired.
The identity is now mutable, and re-stamped after an accepted swap. Only after one: a refresh that fell back to cached or bundled rules kept the previous rules, so it keeps the previous identity, which is the answer that is still true.
A reporting-only guard could not be stopped
The stop method was installed only when a refresh loop or a block log existed. A guard configured for detection reporting alone — reporting on, no refresh interval, no API key — had a live reporter that nothing could reach: its final batch depended on its own timer, and a graceful shutdown was impossible.
There is now one
protection.stop(), always present, that stops the refresh loop, the block-log reporter and the detection reporter, and flushes what is buffered.stopRefresh()stays as an alias.Reporting with no credential posted into a refusal
The detections endpoint is site-addressed and requires a verified, site-bound token. With
reportDetections: trueand no credential resolvable — common on runtimes with no filesystem, where.patchstackrc.jsondoes not exist — the guard queued detections, posted them, and was refused, spending an outbound request per batch while the config said reporting was on.Now it starts no reporter, warns once, and reports
detectionReporting: 'unavailable-no-credential', which is a different statement from'off'. When reporting is on,detectionHealth()returns local counts — attempted, acknowledged, refused or unreachable, dropped for queue pressure — plus the last acknowledgement, so a delivery path that refuses everything can be told apart from an app where no rule fired. Nothing extra is sent to produce those counts.The refresh poller did not back off
Rule resolution deliberately absorbs an API or network failure into cached or bundled rules, because protection has to keep running. The consequence was that the poll tick never threw, the scheduler counted every poll as healthy, and every installed guard kept knocking at its normal interval for the whole length of an outage — then all came back at once when it ended.
Rule resolution now reports whether the rules came from the source. The loop backs off on that as well as on a thrown error, the push endpoint answers
refreshed: falsewhen nothing arrived, andrefresh()resolves with{ ok, reason? }rather than rejecting — a caller's manual refresh must not start failing because the platform is down and the cached rules held.Also
Source comments that still described a rules or detections fetch as something the server accepts without a credential now describe the contract that is in force: site-addressed endpoints require a verified, site-bound token, and only first-time provisioning is anonymous.
Tests
npm test(1308 passed) andnpm run typecheck. Each new guard is mutation-checked: remove the post-swap re-stamp, the credential gate, the always-installed stop, the delivered/refused split, or the backoff, and exactly the test naming that guarantee fails.The new documentation covers the added surface:
detectionReporting,detectionHealth(),stop(), and the credential requirement for reporting.🤖 Generated with Claude Code