Repository navigation
aggregator(alerts): GET /api/push/subscriptions returns every subscription's p256dh + auth secret — unauthenticated, unused by any client, and the /alerts page for the same data redacts them #3693
Description
Problem
GET /api/push/subscriptions returns every push subscription's full database row — including the Web Push auth secret and p256dh key — to any caller who can reach the aggregator, with no authentication. No in-repo client uses this endpoint.
Evidence (verified this run, main 0bb00794)
pm-stack/aggregator/src/server.ts:976— the GET handler isreturn json({ subscriptions: await store.listSubscriptions() }). Unlike the incident intake (POST /api/alerts, gated byalertsIntakeAuthorized, server.ts:1000-1001), this route has no auth check.pm-stack/aggregator/src/alerts.ts:265-267—listSubscriptions()isselect * from alerts.push_subscriptions, so the wire payload carriesendpoint,p256dh, andauthverbatim for every subscriber.- The
authvalue is the RFC 8291 push-encryption secret for that subscription;p256dhis its encryption public key. Together with the endpoint they are the complete per-subscription key material the server itself uses indeliverToAll(alerts.ts:447-448). - Contrast — the HTML surface for the same data deliberately does not expose the keys:
subscriptionRow(render.ts:5550-5560) renders only the endpoint host, classes, quiet window, and user agent. - No consumer needs the GET: repo-wide grep shows
push-client.jsonly POSTs/DELETEs/api/push/subscriptions, and the inline/alertsscript only DELETEs. The engine reads subscriptions internally via the store, never over HTTP.
Probe (laptop, ~/git/fleet/probe-2026-11-01-laneA-subs-leak/probe.ts, blobs sha-matched to origin/main)
AlertsStore backed by a fake DB returning one sentinel row, serialized exactly as the GET handler serializes it:
GET wire contains p256dh sentinel: true
GET wire contains auth sentinel: true
GET wire contains endpoint sentinel: true
HTML /alerts contains p256dh sentinel: false
HTML /alerts contains auth sentinel: false
HTML /alerts contains endpoint host: true
Why it matters
The alert throat (#3676) exists so critical incidents reliably reach Drew's devices. This endpoint hands every subscriber's push key material to any tailnet-reachable caller (the aggregator binds loopback + tailnet and is fronted by Tailscale Serve at cfg.publicOrigin), and enumerates the exact endpoints that the likewise-unauthenticated DELETE /api/push/subscriptions (server.ts:986-994) accepts — so a caller can both harvest the secrets and silently unsubscribe every device from critical alerts. The keys serve no client need: the one legitimate read surface (the /alerts page) already projects them away.
Acceptance criteria
-
GET /api/push/subscriptionsno longer returnsp256dhorauthvalues for any subscription (project each row to the non-secret fields the page uses — id, endpoint host or endpoint, classes, quiet, user_agent, created_at/last_seen_at, gone_count — or remove the GET route entirely if nothing needs it). - A test pins the redaction: a subscription row with sentinel
p256dh/authvalues serialized through the GET path contains neither sentinel (mirroring the probe above). - Internal delivery is unaffected:
runAlertTick/deliverToAllkeep reading full rows through the store directly, and the existing alerts test suite passes. - The
/alertspage subscription list renders unchanged.
Activity
developing-today commentedon Nov 1, 2026
CODE-1504
dezren39 commentedon Nov 1, 2026
🤖 Agent note · filed
dezren39 commentedon Nov 1, 2026
🤖 Agent note · adopted as Phase 1 of design #3702
/api/push/subscriptions— do not substitute a projected GET (no in-repo client uses the GET, and a projected response still enumerates every endpoint); a future consumer re-adds a projected read under the design's standing rule. (2) Fix at the HTTP seam only — never narrowAlertsStore.listSubscriptions(), which the delivery engine reads in-process with full rows.source/design-architect. Existing queued triage packet stands.10 remaining items
dezren39 commentedon Nov 2, 2026
Progress update (queue task 6166): implemented a shared
toPublicSubscriptionprojection inpm-stack/aggregator/src/alerts.tsand applied it to both GET and POST/api/push/subscriptions; the stored/internal rows used by delivery and/alertsremain unchanged. Added sentinel coverage for GET/POST serialization.bun test test/alerts.test.tsfrompm-stack/aggregatorpasses (32 tests). Fulltsc --noEmitreports existing unrelated errors inprojects.test.tsandquota-current.test.ts; no diagnostics in the changed files. Committing/pushing now; PR to follow.dezren39 commentedon Nov 2, 2026
Final update (queue task 6166): change is committed and pushed as
1cdf09588onqueue/6166-3693-push-subscriptions-projection. PR is open and intentionally not merged: #4413. Exact acceptance commandbun test pm-stack/aggregator/test/alerts.test.tspasses (32 pass, 0 fail). GET/POST response projections strip both secret values and keys; delivery still consumes full rows fromlistSubscriptions, and the unchanged/alertsrender tests pass as part of the suite. Full typecheck has unrelated errors inprojects.test.tsandquota-current.test.ts; no errors reported in changed files.dezren39 commentedon Nov 2, 2026
Follow-up on PR #4413 (commit
9c3c0d1):toPublicSubscriptionnow constructs the public object explicitly field-by-field (no rest-spread, so a future secret column can't leak through), and the sentinel test asserts the exact serialized key set for both GET and POST shapes.bun test pm-stack/aggregator/test/alerts.test.ts: 32 pass, 0 fail.dezren39 commentedon Nov 2, 2026
🤖 Conductor binding · manual dispatch wave (Drew-ordered queue unblock) 2026-11-02
small_fix— "Fix aggregator(alerts): GET /api/push/subscriptions returns every subscription's p256dh + auth secret — unauthenticated, unused by any client, and the /alerts page for the same data redacts them #3693: project GET /api/push/subscriptions to non-secret fields (drop p256dh + auth)") ↔ issue aggregator(alerts): GET /api/push/subscriptions returns every subscription's p256dh + auth secret — unauthenticated, unused by any client, and the /alerts page for the same data redacts them #3693ses_e5e6328363effecK5DlHyFKq8I(laptop OpenChamber, agent build)opencode-route/workhorse(route id only; pooled route, no single-plan direct seat). Gate: fresh laptop quota read this run ~09:09 CST — meta 5h 8% / weekly 2%, codex weekly 80%, openai.alternate 5h 0% / weekly 0% — Workhorse OPEN; plan quotas only, never PAYG.~/git/fleet/6166-3693-push-subscriptions-projectionon the laptop, branchqueue/6166-3693-push-subscriptions-projection(from origin/main d1de7ee)queuedin the fleet queue (there is no targeted claim/binding verb yet — code#1337 / openchamber#1272); the dispatch is recorded in the task's progress. The owning lane reconciles/harvests from here, and the conductor shepherds the session.dezren39 commentedon Nov 2, 2026
Agent note — task 6166 picked up by lane code-d (direct execution lane). The conductor-dispatched session's work is already on PR #4413 (branch queue/6166-3693-push-subscriptions-projection, head 9c3c0d1); this lane is reconciling and harvesting it: verifying the diff against the task 6166 scope, dispositioning any Codex findings, and merging under the standing criteria so #3693 closes.
dezren39 commentedon Nov 2, 2026
Agent note — task 6166 done. PR #4413 MERGED (merge commit 10b3415) and #3693 is CLOSED. The diff was verified against the task 6166 scope:
toPublicSubscriptionprojection in alerts.ts (id, endpoint, classes, quiet, user_agent, created_at, last_seen_at, gone_count — p256dh/auth omitted), GET and POST responses in server.ts both projected, and a sentinel regression test in alerts.test.ts covering both response shapes. Codex review completed clean at head 9c3c0d1 (no findings); no GitHub Actions runs registered for the PR (runner backlog), with the session's laptopbun test test/alerts.test.tsat 32 passed recorded in the PR body.