alerting: satd-alert crate + webhook dispatcher, absorbing reorgwebhook

[bkeroack]

PR 3 of the A3 alerting stack (SATD_ALERTING_DESIGN.md §4). Stacked on #481 (which is stacked on #480) — merge bottom-up.

PR 2 made the node detect its own problems. This delivers them somewhere.

What

alertfile=<path> configures any number of signed HTTP hooks, each filtered by category, status kind, and severity floor:

version = 1

[[webhook]]
id = "pager"
url = "https://alerts.example/satd"
secret = "a-long-random-string"
categories = ["status"]
kinds = ["tip_stall", "disk_low"]
min_severity = "warning"

New satd-alert crate holds the pure half — alertfile parsing/validation/permissions, the delivery contract (headers + HMAC), and retry classification — so every rule is testable without a socket and the signature scheme is pinned by golden vectors a third-party receiver can check itself against. reqwest and the dispatcher task stay in the binary. Same split, same reasoning, as satd-auth.

Decisions worth reviewing

• One wire schema, not two. The POST body is byte-identical to the JSON a WebSocket subscriber receives for the same event. Delivery metadata rides in headers — putting the attempt counter or hook id in the body would change the bytes between retries of one event, breaking both signing and receiver-side dedup.
• A permanent 4xx is skipped, not retried forever. A receiver answering 404 must not pin the head of the queue and convert every later event into an overflow drop. Transient failures (5xx/408/429/timeout/connect) retry indefinitely with backoff to a 5-minute ceiling.
• A gap is never silent. Both drop paths — full hook queue, lagged broadcast receiver — emit a synthesized lagged body carrying the count and a resume cursor ahead of the next delivery.
• At-least-once for confirmed chain events across a restart, via a per-hook cursor in the chainstate metadata CF plus a startup replay through the same machinery the streaming carriers use (same 10k clamp, same clamp signal). Health events come back by re-evaluation; mempool stays best-effort — the contract the bus itself offers.
• Config follows the authfile model: path restart-only, contents re-read on every SIGHUP, 0600 enforced from the open handle (not the path — no TOCTOU window). A parse error keeps the last-good hook set, because alerting that silently stopped after a typo is the worse failure. Validation is recognize-and-reject throughout: unknown category, typo'd kind, duplicate id, missing secret are all errors rather than a rule that quietly matches nothing.
• reorgwebhook= is absorbed, not replaced. Its payload stays the shipped ReorgRecord JSON byte for byte — ChainEvent::Reorg carries neither depth, fork_height, nor the disconnected/reconnected lists, so re-shaping it would have silently broken deployed receivers. Routing it through this module also moves that outbound HTTP off the consensus runtime, where it should never have been (design D7).
• Plaintext http:// is gated to loopback/RFC1918 unless allow_insecure_http = true. Bodies carry chain data rather than secrets, but signed-then-cleartext is still a footgun.
• satd-alert depends on node (unlike satd-auth, which depends on nothing). Deliberate: the health taxonomy gets exactly one definition, so an alertfile's kinds/categories/min_severity are validated against the live enum rather than a copy that could drift. The no-async/no-HTTP constraint that actually matters is kept.

Acceptance (design §10 PR 3)

• §6.1/§6.2 delivery semantics — E2E: signed status delivery verified exactly as a third party would (HMAC over the raw body), retry-until-recovery proven via X-Satd-Attempt, permanent 4xx not wedging the queue behind it.
• §6.4 signature golden vectors in satd-alert (reproduced in the docs; the relay will re-assert them).
• §6.5 config: perms/parse rejection matrix, SIGHUP add/edit/remove, HANDLER_RELOAD_KEYS coverage test.
• No dispatcher configured ⇒ zero new tasks and no satd_alertwebhook_* on /metrics (unit-asserted, same invisibility property the policy metrics have).
• clippy --workspace --all-targets --all-features clean; full suite green except the 10 pre-existing test_address_index_backfill_* disk-guard failures on this machine.

Docs: Operator Manual gains an Alert webhooks section (format, delivery contract, failure behavior) and the config reference entry. The normative docs/api/webhooks.md spec + the stalled-endpoint latency bench are PR 5, per the design's split.

🤖 Generated with Claude Code

https://claude.ai/code/session_018AfAm3VVN8gBeYcTL5BJqH

[bkeroack]

Round-1 review fixes (both from the same pass).

Medium — SSRF blast radius. Both delivery clients built with reqwest's default redirect policy, which follows them. A followed 30x would move the POST — body, X-Satd-Signature, hook identity — to a host that never passed validate_url, and the useful targets are the ones an operator cannot see from outside: a metadata endpoint, an RFC1918 admin port, the node's own RPC. Both clients now use Policy::none(), and a 3xx classifies as a permanent drop so the misconfiguration shows up in the log rather than as silent non-delivery. E2E: the redirect target is a second mock receiver that must never be touched.

Low–Medium — cursor parked on permanent drop. A permanent 4xx skipped the event without advancing the resume position, so a receiver that hard-rejects one body shape made every reload and restart rebuild and re-queue the same refused span forever. The event is lost either way; the only question is whether the hook makes progress past it. The skip now persists the cursor, still counted and logged. E2E counts request attempts across a reload — counting accepted deliveries would have passed either way, since a replayed block gets refused again.

Both tests verified to fail without their fix.

[bkeroack]

Deep review — findings (#482 + #484)

Independent review. Marked [verified] = I re-checked against the code myself; the rest are reviewer-reported.

Clean and genuinely well done: outbound-request safety (Policy::none() so no redirect following, url::Url scheme allowlist that handles @-last and \-authority tricks, 10 s per-attempt timeout, response body never read, bounded 1024 queue, serial per-hook delivery); consensus isolation (ReorgLog::record try_sends, both dispatchers on the API runtime, no lock held across an await); HMAC construction (LF-delimited signing string with every pre-body field constrained to an LF-free charset and the body last — unambiguous; independently computed golden vectors; secrets kept out of Debug, TOML error snippets, and delivery logs); default-off.

1. The legacy reorg path logs the webhook URL verbatim — MEDIUM [verified]

satd/src/alert.rs — the alertfile path applies e.without_url() with a comment explaining exactly why ("a webhook URL is frequently the credential itself (Slack, Discord, PagerDuty)"). The reorgwebhook path ~300 lines later logs error = %e without it. With reorgwebhook=https://hooks.slack.com/services/T00/B00/<secret>, any endpoint blip puts the full credential-equivalent URL into stdout → journald → the operator's log shipper, three times per record.

2. signed_at is stamped at dequeue, so the freshness window bounds only retry span — MEDIUM

The spec mandates receivers reject anything outside 600 s, and the signed timestamp is the only staleness signal the documented receiver algorithm checks. But signed_at is taken after rx.recv(). With a hard-down endpoint, delivery is serial and each event burns ~10 min of retries, so a tip_stall raised at T+0 can be popped at T+2h, signed with now, pass the receiver's freshness check, and page on-call for a condition that raised and cleared two hours earlier.

3. "Retries continue indefinitely" is false — MEDIUM [verified]

Delivery is abandoned once unix_secs() - signed_at > MAX_TIMESTAMP_SKEW_SECS (600), reached around attempt 10–11. Contradicted in three places: docs/api/webhooks.md "jittered, and continue indefinitely", the private-CA note "will therefore fail verification and be retried forever", and docs/manual/src/observability.md "1 s doubling to a 5-minute ceiling, indefinitely". An operator reading that concludes a 15-minute relay redeploy is covered and doesn't alert on satd_alertwebhook_dropped_total.

4. CHANGELOG and release notes promise a lagged gap notice that does not exist — MEDIUM [verified]

CHANGELOG.md and 0.5.0-pre.md describe "a bounded queue whose overflow is reported in-band as a lagged event rather than silently" and a skip that "advances the resume position". Nothing is persisted and nothing is synthesized — enqueue drops silently on Full, docs/api/webhooks.md §5.3 says "There is no in-band notice", and the branch's own E2E asserts the absence ("a lagged body was delivered; gap accounting was removed with the cursor"). Stale claims from the pre-revision design. An integrator who treats absence of lagged as proof of no gap silently misses every drop.

5. A dead delivery task is carried across every SIGHUP — LOW

The reload keeps an existing RunningHook whenever r.config == *hook, with no liveness check on r.tx. If webhook_client() failed at startup for one hook, its channel is closed forever; every SIGHUP re-adopts the closed sender and the hook is permanently dark, recoverable only by restart or editing the stanza to force a mismatch. A !r.tx.is_closed() in the match guard fixes it.

6. is_local_host exempts IPv4 link-local, including 169.254.169.254 — LOW

satd-alert/src/config.rs — v4.is_link_local() waives the plaintext gate, so http://169.254.169.254/… is accepted with no allow_insecure_http. Both the spec and the manual say the waiver is for "loopback and RFC1918" only, and the IPv6 arm is stricter (is_unique_local() only, so fe80:: does require the opt-in). Low impact — the alertfile is operator-controlled — but it's the one address the module's own comments name as the interesting target.

7. alertfile= path changes are silently ignored on SIGHUP — LOW

alertfile is in HANDLER_RELOAD_KEYS only. authfile, whose model this copies, has both a handler and a restart! spec. AlertReloader binds self.path at construction, so changing alertfile= and SIGHUPing reloads the old file and logs success against it. That's the accepted-but-ignored-config failure the parser's whole posture exists to prevent.

8. The Operator Manual still says reorgwebhook= users need no edits — MEDIUM [verified]

docs/manual/src/observability.md: "existing receivers need no edits." #484 correctly documents the redirect-following break in the CHANGELOG and release notes, but not in the published manual. An operator whose reorg endpoint answers 301/302 (an http→https proxy hop, a trailing-slash redirect) now silently loses every reorg record, and the canonical doc says nothing changed.

[bkeroack]

Tier 1 + Tier 2 fixes (beceb292 and its lineage)

Finding 2 — signed_at stamped at dequeue (MEDIUM). Fixed. The timestamp is now taken once in the fan-in, when the event leaves the bus, and carried on Delivery::Event::queued_at.

Delivery is serial per hook and a hard-down endpoint burns its full retry budget on each event in turn, so a tip_stall raised at T+0 could be popped at T+2h, signed with now, pass the receiver's 600 s check, and page on-call for a condition that raised and cleared two hours earlier. The signed timestamp is the only staleness signal the documented receiver algorithm has, so this defeated it entirely. One clock read fans out to every hook, so an event is never stale for one receiver and fresh for another.

Finding 5 — dead delivery task re-adopted across every SIGHUP (LOW, but it had a second half). Fixed, and the obvious one-line version would not have worked.

Adding !r.tx.is_closed() to the match guard is correct but unreachable on the path that matters: apply() short-circuits when the alertfile is unchanged, which is the common case for a SIGHUP — an operator reloads precisely because something is wrong. So the short-circuit now makes an exception for a closed sender. Without that, "edit the stanza to force a mismatch" would have remained the only recovery, which is exactly what the finding described.

Lock ordering checked: last_applied → running is the only nesting and nothing takes them the other way.

Findings 3, 4, 8 — docs asserting things the code does not do. Fixed across this PR and #484:

• "Retries continue indefinitely" (3 places) — they stop once a delivery ages past the 600 s freshness window, around attempt 10–11. The consequence that was actually missing is now stated: a receiver down more than ~10 minutes loses the events raised during the outage. An operator reading "indefinitely" would conclude the opposite and not alert on satd_alertwebhook_dropped_total.
• The phantom lagged gap notice — nothing synthesizes one and this branch's own E2E asserts its absence. Removed from CHANGELOG and release notes, replaced with the truth plus "do not read the absence of a notice as the absence of a gap".
• "advances the resume position" — also stale; the per-hook cursor was deleted with the durability machinery. Caught during the rebase and removed.
• The manual's "existing receivers need no edits" — now carries the redirect-following break explicitly, since a reorgwebhook endpoint answering 301/302 silently loses every record.

Deferred (Tier 3)

• Finding 6 — is_local_host exempts 169.254.169.254. The alertfile is operator-controlled and mode-0600, so this is a code-vs-its-own-docs divergence rather than something an attacker reaches. One-line fix (drop is_link_local() from the v4 arm, matching the stricter v6 arm); worth doing, not urgent.
• Finding 7 — alertfile= path changes silently ignored on SIGHUP. The fix is a restart! spec alongside the handler, the model authfile follows. alertfile is genuinely both hot (contents) and restart-required (path), and expressing that needs a small addition to FieldSpec, which every other key shares. Out of scope for a review round, but this is the accept-and-ignore failure the config parser exists to prevent, so it should not sit long.

The dead-task test was run against the previous behavior and fails there.