chris/budget-tracker
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Activity Actions

[BTAPI-80] Add alert-webhook receiver for wealthsimple-api dead-session alerts #94

Merged
chris merged 13 commits from feature/BTAPI-80 into main 2026-09-15 13:25:21 -06:00
Conversation 0 Commits 13 Files changed 16 +519 -12
chris commented 2026-09-14 15:09:00 -06:00
Owner
Copy link

Ticket

BTAPI-80 Add alert-webhook receiver so wealthsimple-api can push dead-session alerts

Summary

  • Adds POST /alert?token=... — a query-string-token-authenticated receiver (exempted from the standard bearer gate via PUBLIC_PATHS, since wealthsimple-api's outbound alert-webhook sender, WSAPI-7, never sends an Authorization header) that validates { kind, body } against budget-tracker-api's full AlertKind set and forwards into Notifications.sendAlert, answering 200 { delivered: true } on success or a genuine 500 if it reached zero devices.
  • New Config.alertWebhookToken (env ALERT_WEBHOOK_TOKEN), compared with the existing timing-safe matches() helper (now exported from api/bearer-auth), and a 30/min rate limiter mirroring /sync/wealthsimple's pattern.
  • Redacts the token query param from request logs (api/create-app's pinoHttp serializer) — pino-http's default serializer logs the full URL and parsed query at info level on every request, including rejected ones, and no prior route carried a credential in the query string.
  • Updates root CLAUDE.md, server/CLAUDE.md, and server/lib/wealthsimple/CLAUDE.md for the new PUBLIC_PATHS entry (two → three) and the new endpoint, plus docs/DEV-ENVIRONMENT.md and a stale comment in api/routes/log.
  • Pre-PR review (two independent passes) found and this branch fixed: a token-leak-into-logs gap (including three follow-up bypasses — repeated query key, percent-encoded key, case mismatch — found on a second pass), a missing end-to-end test of the query-string auth seam through the fully assembled app, an unbounded alertSchema.body, a missing compile-time link between alertSchema's hand-copied kind literals and lib/models.ts's AlertKind, a cross-file Bun mock.module collision in the route's own test, and a shared rate-limit counter across that test file's cases.

🤖 Generated with Claude Code

## Ticket [BTAPI-80](http://192.168.2.100:7123/home/browse/BTAPI-80/) Add alert-webhook receiver so wealthsimple-api can push dead-session alerts ## Summary - Adds `POST /alert?token=...` — a query-string-token-authenticated receiver (exempted from the standard bearer gate via `PUBLIC_PATHS`, since wealthsimple-api's outbound alert-webhook sender, WSAPI-7, never sends an `Authorization` header) that validates `{ kind, body }` against budget-tracker-api's full `AlertKind` set and forwards into `Notifications.sendAlert`, answering `200 { delivered: true }` on success or a genuine `500` if it reached zero devices. - New `Config.alertWebhookToken` (env `ALERT_WEBHOOK_TOKEN`), compared with the existing timing-safe `matches()` helper (now exported from `api/bearer-auth`), and a 30/min rate limiter mirroring `/sync/wealthsimple`'s pattern. - Redacts the `token` query param from request logs (`api/create-app`'s `pinoHttp` serializer) — pino-http's default serializer logs the full URL and parsed query at info level on every request, including rejected ones, and no prior route carried a credential in the query string. - Updates root `CLAUDE.md`, `server/CLAUDE.md`, and `server/lib/wealthsimple/CLAUDE.md` for the new `PUBLIC_PATHS` entry (two → three) and the new endpoint, plus `docs/DEV-ENVIRONMENT.md` and a stale comment in `api/routes/log`. - Pre-PR review (two independent passes) found and this branch fixed: a token-leak-into-logs gap (including three follow-up bypasses — repeated query key, percent-encoded key, case mismatch — found on a second pass), a missing end-to-end test of the query-string auth seam through the fully assembled app, an unbounded `alertSchema.body`, a missing compile-time link between `alertSchema`'s hand-copied kind literals and `lib/models.ts`'s `AlertKind`, a cross-file Bun `mock.module` collision in the route's own test, and a shared rate-limit counter across that test file's cases. 🤖 Generated with [Claude Code](https://claude.com/claude-code) 
Query-string credential POST /alert will check, since wealthsimple-api's
outbound webhook sender (WSAPI-7) sends no Authorization header at all.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Validated against budget-tracker-api's own AlertKind set, not narrowed to
what wealthsimple-api sends today, so any future sender using an
already-recognized kind forwards without a code change here.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
No behavior change. Lets the new /alert route reuse the existing
timing-safe comparison instead of re-implementing it for its
query-string token check.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Query-string token auth (`?token=...`), 30/min rate limit, and a body
validated against alertSchema. Awaits sendAlert so a zero-device delivery
failure propagates to a genuine 500 rather than a blind 200.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Its caller (wealthsimple-api's alert-webhook sender, WSAPI-7) never sends
an Authorization header, so /alert joins PUBLIC_PATHS alongside /health
and /log — guarded instead by its own query-string token check.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Updates PUBLIC_PATHS's entry count (two -> three) in both the root and
server CLAUDE.md (the latter wasn't in the original plan's file list but
states the same now-stale count), adds the /alert row to the root
endpoint table, and notes in server/lib/wealthsimple/CLAUDE.md that
wealthsimple-api's ALERT_WEBHOOK_URL should target this endpoint in
production.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
mock.module('expo-server-sdk', ...) is process-wide in Bun, and
lib/notifications/.test.ts already claims it for its own fixture — a
second mock of that module silently lost the race depending on file
order, which whole-suite verification (Step 8a) surfaced as a flaky
failure. Swap Notifications.sendAlert directly instead, the same
approach lib/wealthsimple/auth/.test.ts already documents and uses for
this exact reason.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
pino-http's default req serializer logs the full url and parsed query
object at info level on every request, including a rejected one — so
/alert?token=<secret> leaked the token into logs on every hit, accepted
or not. No prior route carried a credential in the query string.

Adds a serializers.req override in create-app that redacts the `token`
query param from both the logged url and query object, scoped to that
one param (the separately-known Authorization header logging is
pre-existing and out of scope here).

Also adds end-to-end coverage in create-app/.test.ts that was missing:
PUBLIC_PATHS matches request.path only, excluding the query string, so
/alert?token=xyz is treated as the exempt /alert path — a property that
only exists at the level of the whole assembled app, which
api/routes/alert/.test.ts (mounting the router directly) can't prove.
New tests assert 200/401 through the real app and that a captured log
stream never contains the raw token for either outcome.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
body was z.string().min(1) with no upper bound — the only free-form
string this API accepts from outside the bearer gate, flowing straight
into a push notification via Notifications.sendAlert. Capped at 500,
mirroring wealthsimpleSyncTriggerSchema's own capped strings.

kind's enum literals are hand-copied from lib/models.ts's AlertKind with
no compile-time link, so a fifth AlertKind value would silently keep
400ing here with no signal. Adds a local ALERT_KINDS `as const` tuple
that both z.enum(...) and a new compile-time-only exhaustiveness guard
consume — no shared runtime derivation between the two files, just a
safety net on top of the existing hand-spelled convention.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
server/api/routes/log/index.ts's comment and docs/DEV-ENVIRONMENT.md's
API_KEY row still said two/only GET /health and POST /log; both missed
in this branch's original doc pass (root CLAUDE.md and server/CLAUDE.md
were already corrected).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
startApi's default router was the shared default-export alertRouter,
whose 30/min limiter is created once at import time and never reset —
every test in the file was quietly sharing one counter. Worked today
only because none individually approached 30 requests; a footgun for
whatever gets added next. Defaults to a fresh createAlertRouter()
instead; the one test that must exercise the real default export still
passes alertRouter explicitly.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
[BTAPI-80] Close three bypasses in the alert-token log redaction.
Some checks failed
server / check (pull_request) Failing after 30s
Details
301c8b1e96 
The regex-based redaction had three gaps a real caller could hit: no
`g` flag missed a repeated `token=` param; `qs` percent-decodes query
keys, so `?%74oken=secret` authenticates but never contains a literal
`token=` for the regex to find (the worst case — it both works and
logs the secret); and a case-sensitive `query` check disagreed with a
case-insensitive `url` regex, leaking `?TOKEN=secret` on the query
side. Replaced the text substitution with URLSearchParams-based
matching on parsed keys, which decodes percent-encoding, exposes every
key for a case-insensitive comparison, and collapses duplicates when
re-serializing — closing all three at once.

Extends the log-capture test with the two query-shape bypasses
(repeated token, token not first in the query string).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Pin bun-version to 1.3.11 in the server CI workflow
All checks were successful
server / check (pull_request) Successful in 35s
Details
plane-sync / sync (pull_request) Successful in 2s
Details
ba6b69141a 
setup-bun@v1 grabs a floating "latest" build, which just moved to 1.4.2.
That version changed the timing of bun:test's process-wide mock.module,
breaking lib/notifications/.test.ts's mocked expo-server-sdk when the
full suite runs — the real SDK's token validation then rejects the
fixture's fake tokens, cascading into 20 failures unrelated to this PR's
diff. Pin to the last version confirmed to run the suite clean, matching
the "pinned, not latest" principle docs/CI.md already applies elsewhere.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
chris merged commit 304744aa68 into main 2026-09-15 13:25:21 -06:00
chris deleted branch feature/BTAPI-80 2026-09-15 13:25:21 -06:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
chris/budget-tracker!94
No description provided.