feat(alerts): stamp PROD/TEST environment on every ops alert - #509
Merged
Merged
Conversation
A Fygaro signature-rejection probe against the TEST cluster paged the ops
channel looking exactly like a prod incident: same source, same severity,
same title, no environment anywhere in the embed. Ops had to pull pod logs
on both clusters to learn it was test traffic.
Every alert sender now carries a loud environment tag derived from NETWORK
(mainnet = PROD, anything else = TEST, unset = UNKNOWN so we never claim
PROD by accident):
- Discord: author line ("Fygaro · TEST"), title prefix ("[TEST] ..."), and
a leading "Env" field ("TEST (signet)") placed first so the aggregate
budget clamp never trims it.
- Slack/Mattermost: headline prefix + an env entry on the meta line.
- PagerDuty: summary prefix + custom_details.env.
- ops-events: reuses the shared envLabel() instead of its own copy.
Tests: new env-label and Slack specs; Discord and PagerDuty specs extended
for all three tags, title capping with the prefix, and field ordering.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014etuRoG7S3DxsAmWMP9jAX
…NETWORK Review fixes for #509. NETWORK was the wrong signal for this fleet: the TEST cluster runs NETWORK=mainnet too (the flash chart defaults galoy.network to mainnet and neither cluster's values file overrides it), so the tag would have rendered every TEST alert as PROD — a false prod claim, worse than the unlabeled embed the PR set out to fix. The tag now follows IbexConfig.environment, the one config value that actually differs between the clusters: chart default `sandbox` on TEST (and the local dev stack), `production` on PROD (prod/flash-values.prod.yaml must override it or every IBEX call 401s). envLabel()/envSummary() now print that signal (`TEST (ibex:sandbox)`) instead of the bitcoin network. The UNKNOWN branch is kept because it is now reachable: the yaml schema declares ibex.environment as an enum but neither requires nor defaults it, and ibex-client silently falls back to the production hub when it is unset. An unset or unrecognised value is reported as UNKNOWN (ibex:unset) rather than guessed either way. ops-events' `env` field is switched to the same stamp so the ops feed no longer reads `mainnet` on both clusters. Tests: env-label/discord/slack/pagerduty/ops-events specs mock the IBEX environment (alongside NETWORK=mainnet), with a regression test pinning the real TEST cluster shape — NETWORK=mainnet + ibex sandbox → TEST — and a per-network case asserting NETWORK is ignored entirely. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014etuRoG7S3DxsAmWMP9jAX
islandbitcoin
approved these changes
Sep 15, 2026
islandbitcoin
added a commit
that referenced
this pull request
Sep 15, 2026
#510) ## Why On 2026-09-12 a hand-built curl probe against the **TEST** cluster (key id `x`, timestamp `1789000000`) paged ops twice: "timestamp skew — check NTP" and "HMAC mismatch — secret rotated". Neither was true; prod was crediting payments normally the whole time. The middleware treated any well-formed signature header that failed as evidence of *our* misconfiguration, whether or not the request named a credential we hold or was signed by anyone we know. Companion to #509 (which stamps PROD/TEST on the alert). That PR makes a probe *legible*; this one stops a probe from paging at all. ## What Three changes in `verify-signature.ts`, no route or config changes. - **Mismatch alert is key-id gated.** "HMAC mismatch — secret rotated or wrong" fires only when `Fygaro-Key-ID` names a secret in `FygaroConfig.webhook.secrets`. A genuine Fygaro webhook carries the credential id it was signed with, so a rotated or mispasted secret still arrives under a known key id and still pages. An unknown or absent key id can only ever mismatch and says nothing about our config: 401 + log (`keyId`, `knownKeyId`), no page. - **Skew alert is HMAC-gated, not key-id gated.** The key id is customer-visible (it is the JWT `kid` in every checkout URL), so gating skew on it alone is one base64-decode from a false page. Instead, on a stale timestamp we verify the HMAC over `${timestamp}.${rawBody}` against the candidate secrets first and page only if it validates. A correctly signed but stale request is genuine Fygaro traffic or a replay, which is exactly the clock/NTP signal. Fires under any key id and carries the raw header key id in the alert. - **Fallback-verified requests are surfaced.** When a request verifies via the try-all-secrets fallback under a key id not in config, a warn log names the configured key ids. Without this, a mis-keyed config map would silently disable the mismatch alert until the next rotation 401'd every payment with no page. Also: the no-secrets-configured alert is unchanged (our misconfiguration whatever the request says); key-id lookup is an own-property check so `constructor` / `__proto__` cannot match a secret (this also fixes a pre-existing 500 on that path); `knownKeyId` is the narrowed key id, not a boolean, so no casts. ## Tests `verify-signature.spec.ts`, 31/31 green: - unknown key id + mismatch → 401, no alert - unknown key id + stale timestamp, unsigned → 401, no alert - absent key id, both cases → 401, no alert - signed-but-stale under a known key id → skew alert - signed-but-stale under an unknown key id → skew alert, `key_id` = raw header - `constructor` as key id → 401, not treated as known - fallback-verified under an unknown key id → `next()` called, warn log lists configured key ids, message does not mention skew - positive controls: no-secrets still alerts; known key id mismatch still alerts `eslint` clean. `tsc` shows only the two pre-existing express-types errors in the GraphQL servers, also present on `main`. ## Review Three simon-review rounds. Round 1 (3 should-fix) and round 2 (2 should-fix) applied in `208f551` and `b3a089f`. Round 3's two findings (stale spec comment, stale PR body) applied in the final commit and this description. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_014etuRoG7S3DxsAmWMP9jAX
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
3 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.
Why
Yesterday's two Fygaro webhook alerts ("timestamp skew" and "HMAC signature mismatch", key id
x) came from a hand-built curl probe against the TEST cluster. Prod had zero rejections and was crediting payments normally. But the Discord embed carries no environment, so a test probe and a prod outage render identically and ops has to pull pod logs on both clusters to tell them apart.What
Every alert sender now stamps the environment. The tag is derived from
IbexConfig.environment, not fromNETWORK:ibex.environmentproductionPRODsandboxTESTUNKNOWN(never claim PROD or TEST by accident)Why not
NETWORK? The TEST cluster runsNETWORK=mainnettoo: the flash chart defaultsgaloy.network: mainnetand neither cluster's values file overrides it. A network-based tag (round 1 of this PR) would have labelled every TEST alert as PROD, which is a worse failure than the unlabelled embed this PR set out to fix.ibex.environmentis the one config value that actually differs between the clusters (chart defaultsandboxon TEST and the local dev stack; PROD must override it toproductionor every IBEX call 401s against the sandbox auth domain), so it is both cluster-specific and self-verifying. There is noNODE_ENVfallback;NODE_ENVis ignored entirely.Field value shape is
<TAG> (ibex:<environment>), e.g.TEST (ibex:sandbox),PROD (ibex:production),UNKNOWN (ibex:unset), so a reader can see which signal the tag came from.Per destination:
Fygaro · TEST, title prefix[TEST] …, and a leadingEnvfield (TEST (ibex:sandbox)) placed first soclampEmbedToBudgetnever trims it.custom_details.env.NETWORK ?? NODE_ENV ?? "unknown"label and uses the sharedenvSummary(). This is a behavior change: theenvfield on ops-event embeds (and the dropped-events summary) changes from the bare network (mainneton both clusters,regtestlocally) toTEST (ibex:sandbox)/PROD (ibex:production).New shared module:
src/services/alerts/env-label.ts(envTag,envLabel,envTagPrefix,envSummary).Tests
env-label.spec.ts: PROD onproduction, TEST onsandbox, the real TEST-cluster shape (NETWORK=mainnet+ ibexsandbox) tags as TEST not PROD, UNKNOWN when the ibex environment is unset regardless ofNODE_ENV, UNKNOWN for an unrecognised value.slack.spec.ts(there was none): TEST/PROD/UNKNOWN stamping in headline and meta line, including theNETWORK=mainnet+ sandbox case.discord.spec.tsextended: stamps per ibex environment (again withNETWORK=mainneton the TEST case), never claims PROD/TEST when unset, env field stays ahead of Source/Severity so budget trimming drops it last, prefixed title still capped at 256.pagerduty.spec.ts:[TEST]summary prefix +custom_details.env=TEST (ibex:sandbox).ops-events.spec.ts:envfield asserts the newTEST (ibex:sandbox)value.yarn test:unit test/flash/unit/services/alerts: 93/93 green.eslintclean.tscshows only the two pre-existing express-types errors in the GraphQL servers, also present onmain.Follow-up (not in this PR)
verify-signature.tsstill pages on a well-formed header with an unknown key id. Only firing the secret/skew alerts when the key id matches a secret we hold would silence probes while keeping the rotated-secret and drifted-clock alerts.🤖 Generated with Claude Code
https://claude.ai/code/session_014etuRoG7S3DxsAmWMP9jAX