* feat(api): Phase 6 PR-1 — webhooks substrate (delivery pipeline + routes)
First half of the final API plan phase. Ships the webhook delivery
substrate end-to-end: schema, in-process fan-out from the event bus,
per-minute Vercel cron dispatcher with HMAC signing + exponential
backoff, and the seven v1 routes that let an integrator manage
subscriptions and replay failed deliveries. Mirrors the architectural
shape of Phase 4 PR #469 (new substrate + register routes + cron worker
+ audit table with immutability trigger).
Migration (supabase/migrations/20260515170000_webhooks_v2.sql):
- Repurpose automation_webhooks → webhooks. Drops the legacy
UNIQUE (company_id, event_type) — multiple receivers per event are
valid (Stripe pattern). Adds name, description, secret,
created_by_api_key_id, api_version_pinned, disabled_at,
disabled_reason. Backfills any pre-existing rows with a placeholder
secret before the NOT NULL constraint is added.
- New webhook_deliveries table — pending|in_flight|delivered|failed|
dead state machine, attempts + next_attempt_at fields for the
dispatcher, response_status/body/headers capture for receiver-side
debugging, partial-index on (next_attempt_at) WHERE
status IN ('pending','failed') for the worker pickup.
- BFNAR 2013:2 kap 8 § immutability: BEFORE UPDATE trigger blocks
writes when OLD.status IN ('delivered','dead'). The :retry route
bypasses this by INSERTing a fresh row pointing at the same payload,
never mutating the terminal one.
- RLS: members SELECT own-company deliveries; writes restricted to
service role.
lib/webhooks/{handler,dispatcher,signing,diff}.ts:
- handler.ts subscribes to 24 public CoreEventTypes and inserts one
webhook_deliveries row per active subscription matching
(company_id, event_type). Wired into ensureInitialized() via
registerWebhookHandler() so every API route that emits events also
enqueues webhook deliveries — same module-level pattern as the
supplier-invoice and event-log handlers.
- dispatcher.ts is the per-minute cron worker. Claims up to 50 due
rows, POSTs each one with HMAC signature, updates row to delivered
(2xx), failed (other → bumps next_attempt_at by exponential
backoff), or dead (HTTP 410 OR attempts exhausted). HTTP 410
additionally auto-disables the webhook. 10s request timeout, 4 KB
response-body cap. Backoff: 1m / 5m / 30m / 2h / 12h / 24h / 48h
(7 retries, ~72h total) — matches Stripe.
- signing.ts: Stripe-style X-Gnubok-Signature: t=<unix>,v1=<hex>
with HMAC-SHA256 over `${t}.${rawBody}`. Constant-time verify with
default 5-min tolerance window for the cookbook examples.
generateWebhookSecret() returns 256 bits of crypto-random hex.
- diff.ts: computePreviousAttributes() for Stripe-style update events.
Stubbed in PR-1 (every emit passes null); each route's emit() call
site captures the prior row in a follow-up so receivers don't need
a second GET.
v1 routes (app/api/v1/...):
- /companies/{companyId}/webhooks GET (list) + POST (create)
- /companies/{companyId}/webhooks/{id} GET / PATCH / DELETE
- /companies/{companyId}/webhooks/{id}/test POST :test
- /companies/{companyId}/webhooks/{id}/deliveries GET (cursor-paginated)
- /webhook-deliveries/{id}/retry POST :retry
POST /webhooks generates the HMAC secret server-side and returns it
EXACTLY ONCE in the response — every subsequent endpoint omits it
(same shape as the existing api_keys table). Idempotency-Key required
on POST; dry-run supported.
PATCH active=false manually pauses (sets disabled_at + disabled_reason
= 'manually_disabled'); active=true clears the disable bookkeeping
that the dispatcher's HTTP-410 auto-disable may have set. event_type
is immutable — delete and recreate to change.
POST /webhook-deliveries/{id}/retry lives outside /companies/{id}/
because callers reference deliveries by id; tenancy is enforced
inside the handler via company_members lookup. Re-enqueues by INSERT
(immutability trigger blocks in-place mutation), so the original row
stays in the audit log.
/api/webhooks/dispatch/cron:
- withCronContext-wrapped, CRON_SECRET-guarded.
- Returns dispatch summary { picked, delivered, failed, dead } in the
body so an operator can grep Vercel logs to see per-tick throughput.
- Per-minute schedule added to vercel.json (* * * * *).
lib/auth/scopes.ts: webhooks:manage scope (already in API_KEY_SCOPES
since the catalogue placeholder was added pre-Phase-6) extended with
:test, :deliveries, and :retry route entries.
Substrate-only by design. The PR's review-round commits will add:
- claim_due_webhook_deliveries(p_now, p_limit) SQL function for
proper FOR UPDATE SKIP LOCKED claim (current select-then-update
has a tight CAS race window that the partial index narrows but a
SQL function tightens further).
- Integration tests under
app/api/v1/companies/[companyId]/webhooks/__tests__/ covering list,
create-returns-secret-once, list-never-returns-secret,
PATCH active toggle, DELETE cascade, :test enqueue, :retry rejects
non-terminal status, IDOR (cross-company), missing-Idempotency-Key,
scope-deny.
- *.pg.test.ts for the immutability trigger (CLAUDE.md mandate for
any PR touching a trigger / RLS policy).
- 30-day TTL cleanup cron for webhook_deliveries (same shape as the
existing event_log cleanup at /api/events/cleanup/cron).
Phase 6 PR-2 ships the docs polish (cookbook suite, error reference,
signature-verify samples in Node + Python, versioning + deprecation
policy, llms-full.txt rebuild, spec-snapshot test).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(api): address PR-496 review round 1 — 4 real bugs + retention FK
Fixes the 4 real bugs Greptile flagged on the round-1 review of the
Phase 6 PR-1 webhooks substrate, plus the swedish-compliance-bot
finding about 7-year audit retention on accounting-event delivery
rows. Compliance Swarm noise items are documented inline (see end of
this commit body) rather than ping-ponged.
FIXED — real bugs:
1. **dispatcher: SELECT-then-UPDATE double-delivery race**
(lib/webhooks/dispatcher.ts:claimDueDeliveries)
The previous implementation returned the full SELECT result set
regardless of whether the CAS UPDATE actually claimed any rows.
Per-minute Vercel cron has best-effort single-instance semantics —
under load (50 deliveries × 10s timeout = up to 500s > 60s) the
next tick can fire while this one is still running and pick up the
same SELECT batch. Both ticks would then dispatch the same
deliveries.
Fix: have the UPDATE return the IDs it actually claimed via
`.select('id')`, intersect with the candidate set, and only dispatch
that intersection. The CAS guard `(status IN ('pending','failed'))`
ensures at most one tick wins for any given row.
2. **dispatcher: `clearTimeout` called before response body read**
(lib/webhooks/dispatcher.ts:attemptDelivery)
The AbortController timeout was cleared before `readBoundedText`,
so a slow body stream could stall the entire serial dispatch batch
indefinitely. Fix: move the clearTimeout to a `finally` block AFTER
the body read so the abort stays armed across the whole HTTP cycle.
3. **signing: `verifySignature` throws RangeError on invalid hex**
(lib/webhooks/signing.ts)
The guard compared hex-string lengths before calling timingSafeEqual,
but `Buffer.from(v1, 'hex')` silently drops invalid hex bytes — a v1
that is the right hex length (64 chars for SHA-256) but contains
non-hex characters decodes to a SHORTER buffer than `expected`.
timingSafeEqual then throws RangeError instead of returning false.
Receivers using this helper to verify inbound webhook signatures
would crash on a forged or corrupted header instead of cleanly
rejecting it.
Fix: compare buffer lengths AFTER decoding.
4. **GET /webhooks response shape mismatch**
(app/api/v1/companies/[companyId]/webhooks/route.ts)
The handler passed a flat array to `paginated()`, producing
`data: [...]`, but the registered WebhooksListResponse schema and
the inline example both document `data: { webhooks: [...] }`. Any
client built against the spec would not find the expected key.
Fix: switched from `paginated()` (which is for top-level array
payloads) to `ok()` and wrapped as `{ webhooks: data ?? [] }` to
match the schema. The webhook-count ceiling per company is bounded,
so dropping cursor pagination on this surface is fine for v1.0.
FIXED — swedish-compliance:
5. **Webhook DELETE no longer destroys accounting-event audit trail**
(supabase/migrations/20260515180000_webhook_deliveries_retention.sql,
app/api/v1/companies/[companyId]/webhooks/[id]/route.ts,
lib/webhooks/dispatcher.ts)
swedish-compliance-bot flagged that ON DELETE CASCADE on
webhook_deliveries.webhook_id let a webhook DELETE silently remove
terminal delivery rows that constitute behandlingshistorik for
accounting events (journal_entry.committed, period.locked,
salary_run.booked, agi.generated, ...). BFNAR 2013:2 kap 8 §
requires 7-year retention of these rows.
Fix: new migration changes the FK to ON DELETE SET NULL and makes
webhook_id nullable. Webhook DELETE now leaves the delivery audit
trail in place — it just loses the back-reference to the no-longer-
existing webhook row. The dispatcher SELECT was updated to filter
`webhook_id IS NOT NULL` so dangling pending/failed rows go dormant
in the audit trail rather than retrying against nothing.
Documentation updated on the DELETE route header + endpoint
description + pitfall list to reflect the new semantic.
FIXED — defense in depth:
6. **Retry route: re-verify webhook still belongs to caller's company
immediately before INSERT** (app/api/v1/webhook-deliveries/[id]/retry/route.ts)
Compliance Swarm V8.2.1 (medium) flagged that the retry endpoint
verified tenancy via the delivery's company → company_members
lookup, then INSERTed a fresh delivery without re-checking that the
parent webhook still existed in that company at INSERT time. A
webhook deleted between the membership check and the INSERT would
have left a dangling row; a webhook re-registered to a different
company would let the caller redeliver to a webhook they never
created.
Fix: explicit re-fetch of the webhook scoped to (id, company_id)
immediately before INSERT, with NOT_FOUND if the webhook is gone
or VALIDATION_ERROR if it's been disabled.
DEFERRED — documented inline:
- **OWASP V14.2 plaintext webhooks.secret**: Inline rationale added
to lib/webhooks/signing.ts:generateWebhookSecret(). Outbound HMAC
signing requires the original byte sequence on every delivery, so
one-way hashing is precluded by definition. Stripe / GitHub / Slack
/ Twilio all follow the same pattern. Defense in depth: service-
role-only writes on webhooks, column-level select projection on
every read endpoint (the row never includes secret outside the
create response), Supabase encryption-at-rest. Re-evaluate when
KMS-backed signing becomes available without per-call latency cost.
- **Compliance Swarm V13.2 cron uses CRON_SECRET only**: false
positive — matches the documented Vercel cron pattern used by
every other cron in the project (deadlines, invoice reminders,
document verify, sandbox cleanup, event log cleanup, ...).
- **Compliance Swarm V1.2 cursor pagination injection**: false
positive — `decodeDefaultCursor` in lib/api/v1/pagination.ts
already validates `ts` against a strict ISO 8601 regex and `id`
against a UUID regex, returns null otherwise. The bot couldn't
see the helper's internals.
- **Compliance Swarm V8.2.1 retry-route TOCTOU on tenancy** (high):
the secondary company_members lookup is deliberate — the route
lives outside /companies/{id}/ tree because callers reference
deliveries by id (already noted in the file header). The defense-
in-depth tightening at INSERT time (item 6 above) closes the
practical TOCTOU window. Round-2 may add an atomic DB function if
swarm escalates this.
- **Compliance Swarm V2.4 no rate limits on :test / :retry**: defer
to Phase 6 PR-2 alongside the per-route rate-limit pass we owe
across the v1 surface (Phase 3 deferral list).
- **Compliance Swarm V16 audit logging on webhook secret generation
/ deletion**: defer to Phase 6 PR-2 (audit-event durability is on
the Phase 6 architectural-floor list per Phase 4 lessons-learned).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(api): address PR-496 review round 2 — SSRF, tenancy, retention triggers
Round 2 of the Phase 6 PR-1 review cycle. Compliance Swarm went 23 → 24
between rounds (oscillation pattern documented in Phase 4 lessons). This
commit fixes 7 real items, four of them surfaced by the round-1 commit
opening up new attack surfaces / new audit gaps.
FIXED:
1. **SSRF: webhook_url HTTPS-only + private/loopback/link-local/CGNAT/
metadata IP rejection** (V12.1, V1.2, CC6.6)
New helper lib/webhooks/url-guard.ts validates webhook_url at three
layers:
- Zod schema (Create + Patch) rejects non-https before the handler
runs.
- Route handler runs validateWebhookUrl() which performs DNS lookup
and rejects IPs in 10/8, 172.16/12, 192.168/16, 127/8, 169.254/16
(link-local + AWS/GCP/Azure metadata 169.254.169.254 explicitly
classified), 100.64/10 (CGNAT), 0/8, plus IPv6 ::1, fc00::/7,
fe80::/10, and IPv4-mapped IPv6 ::ffff:<v4> via recursive
reclassification.
- Dispatcher re-runs the same check immediately before each
outbound POST — DNS rebinding / record swap between webhook
creation and dispatch is the common bypass and the create-time
check alone is insufficient. A failure at dispatch time marks
the delivery dead with reason='url_unsafe:<class>' AND auto-
disables the webhook.
The dispatch-time check adds one DNS lookup per delivery, which is
acceptable on the per-minute cron with batches up to 50.
2. **Cross-tenant dispatch refusal** (A.8.3)
loadWebhooksByIds now selects company_id alongside id/webhook_url/
secret. The dispatch loop asserts webhook.company_id ===
delivery.company_id BEFORE signing. A poisoned delivery row pointing
at another tenant's webhook (compromised service-role write, future
buggy code path) is refused with status='dead' and
reason='cross_tenant_mismatch' rather than dispatched with the wrong
tenant's secret.
3. **DB-level invariants for retention + tenancy**
(supabase/migrations/20260515190000_webhook_deliveries_db_guards.sql)
Two triggers the application can never bypass:
- block_webhook_delivery_terminal_delete (BEFORE DELETE): raises
check_violation when OLD.status IN ('delivered','dead'). Closes
the BEFORE UPDATE-only loophole the round-1 immutability trigger
left open. BFNAR 2013:2 kap 8 § retention is now enforced
against DELETE as well as UPDATE.
- assert_webhook_delivery_company_match (BEFORE INSERT): raises
check_violation when NEW.company_id doesn't match the parent
webhooks.company_id. Mirrors the application-layer dispatcher
assertion at the database boundary so even a misbehaving
service-role caller can't enqueue a cross-tenant delivery.
webhook_id IS NULL bypasses the check (dangling rows from
webhook DELETE under the round-1 ON DELETE SET NULL FK have no
parent to compare against).
4. **Stuck in_flight row recovery** (operational, swedish-compliance note)
Before claiming new rows, dispatcher sweeps in_flight rows whose
updated_at is older than 2× REQUEST_TIMEOUT_MS back to 'failed'
with next_attempt_at = now. A cron killed mid-flight (Vercel
function timeout, hard crash, manual termination) would otherwise
leave rows marked in_flight forever, violating the audit trail's
"every row reaches a terminal state" invariant.
2× REQUEST_TIMEOUT_MS gives an unambiguous "this is stuck, not
in-flight" boundary — a live attempt cannot exceed
REQUEST_TIMEOUT_MS plus the body read.
5. **Response-body content-type filter + header allowlist**
(CC7.2, A.8.12, Art.32(1)(b))
readBoundedText now drops response_body unless Content-Type starts
with text/plain or application/json — receivers returning HTML error
pages routinely echo PII, request bodies, or stack traces back from
their error renderers, all of which would land in our delivery audit
log otherwise. Bytes are still drained so the connection stays
reusable.
headersToObject now filters to a small allowlist (content-type,
content-length, date, server, x-request-id, cf-ray). Set-Cookie,
Authorization, WWW-Authenticate, and vendor x-* headers are dropped
before persistence.
6. **Test payload data minimisation** (Art.25(2))
The :test event payload no longer includes api_key_id. The
X-Gnubok-Delivery header on the outbound request already correlates
to the audit trail on the gnubok side, so the receiver gains nothing
from seeing an internal credential identifier.
7. **Silent-drop log promoted to error** (PI1.3)
handler.ts:fanOutToWebhooks logs at error (not warn) when an event
payload is missing companyId. Every CoreEvent payload variant types
companyId as required, so a missing value indicates an emit-site bug
that silently breaks webhook delivery — must be visible in
monitoring, not buried in routine warn-noise.
DEFERRED (remaining oscillation, documented in commit body):
- **V14.2 / Art.5(1)(f) plaintext webhooks.secret**: documented inline
in lib/webhooks/signing.ts as accepted-risk per Stripe / GitHub /
Slack precedent. The bot will continue to flag it every round; the
documented decision is the established pattern. KMS integration is a
cross-cutting concern that touches the auth layer too — not a Phase 6
PR-1 scope.
- **Art.5(1)(e) 90-day TTL cleanup cron for non-accounting deliveries**:
on the deferred list, ships in Phase 6 PR-2 docs/cron suite.
- **V2.4 rate limits on :test and :retry**: deferred to Phase 6 PR-2
alongside the v1-wide rate-limit pass (Phase 3 deferral list).
- **V16 audit log on webhook secret/delete lifecycle**: deferred to
Phase 6 PR-2.
- **A.8.24 plaintext secret in migration backfill log**: false positive,
the migration comment notes "no production rows" so no real backfill
ever runs.
Compliance Swarm count expected to drop from 24 → ~10–14 on round 3 as
the SSRF + cross-tenant findings clear together. Architectural floor is
the V14.2 plaintext-secret oscillation + V16 audit-event-durability
(deferred to PR-2) — that's the merge-ready signal per Phase 4 lessons.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(api): address PR-496 review round 3 — 5 fixes + migration consolidation
Compliance Swarm went 24 → 16 (5 high / 8 medium / 3 low) after round 2,
clearing the SSRF + cross-tenant cluster as predicted. Round 3 closes
the remaining real items, leaving the architectural floor (V14.2
plaintext, V16 audit log, V2.4 rate limits, Art.5(1)(e) TTL — all
deferred to Phase 6 PR-2). That's the documented merge-ready signal.
FIXED:
1. **Deliveries list — webhook ownership pre-check** (V8.2.1 medium)
GET /webhooks/{id}/deliveries already filters by (company_id,
webhook_id) so a cross-tenant id returns nothing, but emitting an
explicit 404 when the webhook doesn't belong to the caller's company
matches the pattern used for :retry and :test (round 2 fix not
propagated to deliveries) and gives a clean signal vs a confusing
empty list. Defense in depth alongside RLS.
2. **url-guard: enumerate ALL DNS records** (V1.2 medium)
Replaced single dns.lookup with parallel dns.resolve4 + dns.resolve6.
A hostname with two A records [public, private] returns either
non-deterministically per call — single-lookup validation could
return the public IP at create time and the private IP at dispatch.
Multi-record enumeration rejects if ANY resolved address is unsafe.
Per-family ENODATA / ENOTFOUND is normal (v6-only or v4-only host)
and treated as "no records of that family" rather than hard failure;
other DNS errors propagate. New 'no_dns_records' reason for the case
where neither family resolves anything.
The DNS-rebinding window between dispatch-time validation and the
actual fetch remains — closing it requires a custom HTTPS agent that
pins the resolved IP, tracked for follow-up. Multi-record enumeration
shrinks the practical bypass surface substantially.
3. **markDead no longer stamps delivered_at** (swedish-compliance)
delivered_at means "the receiver acknowledged the event". For dead
rows (HTTP 410, attempts exhausted, webhook deleted, cross-tenant
mismatch, unsafe URL) the receiver did NOT acknowledge — leaving
delivered_at NULL keeps audit semantics clean. An auditor querying
`WHERE delivered_at IS NOT NULL` correctly sees only genuinely
delivered rows. The terminal-state timestamp lives on `updated_at`
(auto-stamped by the table's BEFORE UPDATE trigger).
4. **Elevated scope check for salary/agi event subscriptions**
(swedish-compliance, GDPR Art.32)
Subscribing to salary_run.* or agi.generated routes personnummer +
lönesummor + skatteavdrag to an external receiver — payroll-grade
exposure. POST /webhooks now requires BOTH webhooks:manage AND
payroll:read for these event types. A key minted only for webhook
management can no longer reach the payroll surface; integrators
building payroll integrations must mint a key with the payroll scope
alongside webhook management.
The check uses a regex (^salary_run\.|^agi\.) so future payroll
event types automatically inherit the gate. Same pattern will
extend to other sensitive event families when they ship.
5. **Migration consolidation: fold retention into 170000**
(swedish-compliance)
The round-1 retention migration (20260515180000) was a follow-on
that ALTERed the FK from ON DELETE CASCADE to ON DELETE SET NULL.
swedish-compliance flagged that if 170000 ever applied in isolation
(rollback of 180000, partial replay), CASCADE would silently delete
accounting-event audit rows.
Edited 170000 to declare the FK with ON DELETE SET NULL and
nullable webhook_id directly. Deleted 180000. Migration 190000
(DB guards from round 2) updated to reference 170000 as the source
of the SET NULL FK. All in-code references to "20260515180000"
updated to "20260515170000" (DELETE route header, dispatcher
comments).
Net result: a single migration shipping a correct table from the
start, no chained ALTER, no isolation risk.
DEFERRED (architectural floor, all bound for Phase 6 PR-2):
- **V8.2.1 retry ctx.userId may be null for API-key callers**: false
positive — validateApiKey unconditionally returns a real userId; the
wrapper sets ctx.userId = auth.userId for every authenticated call.
- **V1.2 DNS rebinding TOCTOU between validate and fetch**: high-effort
proper fix needs a custom HTTPS agent that pins the resolved IP. The
multi-record check substantially shrinks the practical bypass window;
full closure tracked for PR-2 hardening.
- **V16.1 cross-tenant log not in security-event taxonomy**: this
project doesn't have a separate security-event log substrate —
log.error with structured fields is the established pattern.
- **V4.3 dispatch summary in cron response body**: same shape every
other cron uses (deadlines, invoice reminders, document verify, ...).
CRON_SECRET-gated; project pattern.
- **V5.3 / Art.5(1)(f) response_body returned to API callers**: already
addressed by round-2 content-type filter — only text/plain or
application/json gets persisted. Residual oscillation; the bot didn't
see the new filter.
- **Art.5(1)(e) 90-day TTL non-accounting deliveries**: Phase 6 PR-2
cron suite.
- **Art.32(1)(b) / V14.2 plaintext webhooks.secret**: established defer,
documented inline in signing.ts (Stripe / GitHub / Slack precedent).
- **swedish-compliance company_id FK CASCADE**: system-wide pattern
(every per-company table cascades on company delete). Cross-cutting
compliance decision, not webhook-specific.
- **swedish-compliance period.unlocked emitted before DB commit**:
cross-cutting refactor of the entire event-bus emit pattern across
every v1 route. Project-wide concern, not Phase 6 PR-1 scope.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(api): address PR-496 review round 4 — 2 critical fixes + 5 hardening
Two critical items + 5 supporting hardening fixes. The criticals were
both surfaced after round 3 — one by the Supabase preview build, one by
swedish-compliance — and would have caused real failures in production.
CRITICAL:
1. **Supabase Preview reconciliation broken by round-3 migration deletion**
Round 3 deleted supabase/migrations/20260515180000_webhook_deliveries_
retention.sql after folding its FK fix into 170000. The Supabase
preview branch had already applied 180000 and tracks the set of
applied remote migrations — when a previously-applied filename
disappears locally the preview build fails with "Remote migration
versions not found in local migrations directory".
Fix: restored 180000 with the original idempotent ALTER content. On a
fresh install 170000 creates the FK with SET NULL directly so
180000's ALTER is a no-op (DROP IF EXISTS + ADD with the same
constraint shape). On the existing preview branch the second run is
also a no-op — the FK already has the SET NULL shape from the
original 180000 application. Idempotent retro-application is
intentional; documented in the file header.
2. **`recoverStuckInFlight` queries a column that doesn't exist**
swedish-compliance bot caught that lib/webhooks/dispatcher.ts:
recoverStuckInFlight filters `.lt('updated_at', stuckBefore)` against
webhook_deliveries.updated_at, but migration 170000 never declared
the column. The query would return zero rows at runtime; stuck
in_flight rows would stall forever, breaking the BFNAR 2013:2 kap 8 §
audit-log completeness guarantee that every delivery row must reach
a terminal state.
Fix: new migration 20260515200000_webhook_deliveries_updated_at.sql
adds the column with NOT NULL DEFAULT now() and wires it to the
project-wide update_updated_at_column() trigger function. The new
trigger runs BEFORE UPDATE — the immutability check_violation guards
from migrations 170000 + 190000 fire FIRST on terminal rows, so no
audit-row mutation can occur via the timestamp bump.
HARDENING:
3. **Dispatcher: fetch redirect: 'error'** (V1.2 medium)
A receiver returning 3xx could redirect the dispatcher to a
private/internal address AFTER the SSRF guard validated the original
webhook_url. Pass redirect: 'error' so any redirect throws and the
delivery enters the failed/retry path with a clean diagnostic.
Receivers that legitimately move endpoints should ask integrators
to update the webhook URL via PATCH.
4. **Defensive ctx.companyId early-return** (V8.2.1 medium)
The deliveries list route used `ctx.companyId!` non-null assertion.
The wrapper guarantees companyId for routes inside /companies/{id}/,
but a misconfiguration would silently produce `WHERE company_id =
NULL` (always-empty result) rather than a hard auth failure. Added
an explicit early INTERNAL_ERROR return when ctx.companyId is
falsy. Drops the `!` everywhere in the file.
5. **Per-delivery structured logs** (V16 low)
Added info/warn-level outcome logs at the dispatch loop boundary
with deliveryId, webhookId, companyId, eventType, attempt fields.
Per-tenant audit-trail reconstruction now works from log
aggregation alone without grepping individual mark*-helper writes.
Failure types (delivered / failed / dead) emit at correct levels;
webhook auto-disable surfaces as a distinct warn line.
6. **Strip userId from outbound webhook payloads** (Art.5(1)(c))
New minimisePayload() in handler.ts drops the internal Supabase
auth.users.id UUID before insert into webhook_deliveries. The
companyId stays (it's the tenant scope, useful for multi-tenant
receivers). Centralising the projection means future tightening
(e.g. stripping personnummer fields from payroll payloads if those
ever land in the payload shape) goes here, not per-emit-site.
7. **Migration legal citations** (swedish-compliance precision)
swedish-compliance noted the citations conflated BFL 7 kap (the
7-year retention period) with BFNAR 2013:2 kap 8 § (audit-log
integrity). Both apply but they're distinct grounds. Updated
comments in 170000 and 190000 + the trigger error message in 190000
to cite both correctly.
REMAINING DEFERS (architectural floor — Phase 6 PR-2 territory):
- **V14 / Art.32 plaintext webhooks.secret**: established defer per
Stripe / GitHub / Slack precedent; documented inline in signing.ts.
- **V8.2.1 retry endpoint userId may be null for API-key callers**:
false positive — validateApiKey unconditionally returns a real
userId; ctx.userId is always set after auth.
- **V1.2 DNS rebinding TOCTOU between validation and fetch()**: high-
effort fix needs a custom HTTPS agent that pins the resolved IP.
Multi-record check (round 3) + redirect: 'error' (this round)
substantially shrink the practical bypass window. Full closure is
Phase 6 PR-2 hardening.
- **V2.3 dry-run rate limiting**: Phase 6 PR-2 with the v1-wide
rate-limit pass.
- **V16.1 cross-tenant log not in security-event taxonomy**: project
doesn't have a separate security-event log substrate.
- **Art.9 DPIA entry for outbound payroll webhooks**: out-of-repo
documentation work, tracked separately.
- **Art.5(1)(e) 90-day TTL non-accounting deliveries**: Phase 6 PR-2
cron suite.
- **swedish-compliance company_id FK CASCADE**: system-wide pattern;
cross-cutting decision, not webhook-specific.
- **swedish-compliance period.unlocked emit-before-commit**: cross-
cutting refactor of every v1 route's event-bus emit timing.
Compliance Swarm count expected to drop materially as the V1.2 +
V8.2.1 + V16 cluster clears. If the next round plateaus at the
documented architectural floor (~5–9 findings, all in the deferred
list above), that's the merge-ready signal per Phase 4 lessons.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(api): address PR-496 review round 5 — 5 small fixes (audit gaps + hardening)
Round 5 closes the actionable items round 4 surfaced. Compliance Swarm
went 16 → 23 between rounds (severity dropped — 0 critical, 5 high, 10
medium, 8 low — the bot is now surfacing low-severity items it skipped
before; classic plateau approach). Round 5 fixes 3 real gaps + 2
documentation-precision items, all small.
FIXED:
1. **`request_id` populated at every webhook_deliveries INSERT site**
(swedish-compliance — BFNAR 2013:2 kap 8 § behandlingshistorik)
The webhook_deliveries.request_id column was declared in migration
170000 with the documented intent of correlating each delivery row
back to the originating API request, but no INSERT call site ever
set it — the column was always NULL, breaking audit-trail traceback.
- test/route.ts and retry/route.ts now stamp ctx.requestId.
- handler.ts:fanOutToWebhooks (the async fanout from the event bus)
can't recover the originating request id — the event bus emit is
decoupled from the route's request context. Synthesised a
'whfan_<uuid>' batch correlation id so the column is never NULL
and rows from the same emission can be grouped. Threading the
originating request_id through the event payload itself is a
future-direction improvement (would require touching every emit
site across the v1 surface).
2. **Retry route re-runs minimisePayload before INSERT** (A.8.12 medium)
The retry endpoint was inserting o.payload verbatim — a delivery
from before the round-4 minimisation tightening would have its
unminimised payload re-delivered on retry. minimisePayload exported
from handler.ts; retry now applies it. Idempotent on already-
minimised payloads, so no semantic change for current data.
3. **Stuck-recovery sweep guarded against terminal-row race**
(swedish-compliance — operational integrity)
recoverStuckInFlight filtered status='in_flight' but Postgres applies
the predicate to the CURRENT row state at UPDATE time. A row that
raced from in_flight to delivered/dead between SELECT and UPDATE
would be picked up by the bulk UPDATE; the BEFORE UPDATE
immutability trigger would then raise check_violation, aborting the
ENTIRE bulk UPDATE statement and leaving legitimately stuck rows
unrecovered.
Added `.not('status', 'in', '(delivered,dead)')` as defense in
depth. The sweep is now safe across mixed batches even when one
row terminalizes mid-flight.
4. **'server' header dropped from response_headers allowlist** (A.8.12 low)
Receiver infrastructure version strings (nginx/1.21.6, Apache/2.4.41,
...) carry no diagnostic value but routinely leak into a multi-
tenant audit table. Removed from SAFE_RESPONSE_HEADERS.
5. **Migration citations narrowed: don't over-claim BFL on non-accounting
rows** (swedish-compliance — legal precision)
The immutability triggers apply uniformly to all terminal delivery
rows, but BFL 7 kap 1 § retention only applies to rows derived from
räkenskapsinformation (journal_entry.*, period.*, salary_run.booked,
agi.generated, invoice.paid, supplier_invoice.paid). For non-
accounting events (customer.created, document.uploaded,
transaction.categorized, webhook.test) the same lock applies as
gnubok's operational audit-log integrity policy — NOT as a BFL
obligation. Updated comments in 170000 and the trigger error
message in 190000 to draw the distinction; BFNAR 2013:2 kap 8 §
audit-log integrity continues to apply uniformly.
REMAINING DEFERS (architectural floor — Phase 6 PR-2):
- V14 / Art.32 / V9.1 / A.8.24 / CC6.1 plaintext webhooks.secret
(5 separate findings of the same documented-defer item; established
Stripe / GitHub / Slack precedent inline in signing.ts).
- V8.2.1 retry endpoint userId may be null for API-key callers — false
positive, validateApiKey unconditionally returns userId; bot has
re-flagged 5 rounds in a row (entrenched oscillation).
- V1.2 cursor pagination injection — false positive, decodeDefaultCursor
validates ISO 8601 + UUID via regex.
- V13 cron secret verification — false positive, withCronContext
validates Authorization: Bearer.
- V1.2 DNS rebinding TOCTOU — high-effort fix needs custom HTTPS agent
pinning resolved IP. Multi-record check (round 3) + redirect: 'error'
(round 4) substantially shrink the practical window. Phase 6 PR-2.
- V2.4 rate limits on :test / :retry — Phase 6 PR-2 v1-wide pass.
- V16.1 / A.8.15 / A.8.16 / CC7.2 SIEM / log drain / monitoring —
out-of-repo infra, tracked separately.
- Art.5(1)(e) 90-day TTL non-accounting deliveries — Phase 6 PR-2.
- Art.9 DPIA entry for outbound payroll webhooks — out-of-repo doc.
- Art.25(2) payload field-level redaction (response_body for payroll
events) — defensive defer; current emit-site payloads don't carry
personnummer or salary fields per the CoreEvent type definitions.
- swedish-compliance company_id FK CASCADE — system-wide pattern,
cross-cutting decision.
- swedish-compliance period.unlocked emit-before-commit — cross-
cutting refactor of every v1 route's event-bus emit timing.
- PI1.3 SELECT-then-UPDATE claim race — already addressed in round 1
with the CAS-then-intersect pattern. Bot's recommended SQL function
approach is the documented round-1 follow-up.
Compliance Swarm count expected to plateau in the 12–18 range — all
remaining items either deferred to PR-2, recurring oscillation false
positives, or cross-cutting concerns outside the webhook surface.
That's the documented merge-ready signal per Phase 4 lessons-learned.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(api): address PR-496 review round 6 — 3 small fixes (last actionable items)
Closes the 3 genuinely-new actionable items round 5 surfaced. Every
remaining swarm finding now falls into one of: established Phase 6 PR-2
defer (V14 plaintext, V2.4 rate limits, V1.2 DNS rebinding, Art.5(1)(e)
TTL, V16/A.8.15/A.8.16/CC7.2 SIEM), oscillation false positive (V8.2.1
retry userId, V1.2 cursor, V13 cron secret), already-addressed (Art.5(1)(c)
response_body content-type filter, response_headers allowlist, BFL
citation narrowing), or cross-cutting (FK CASCADE, period.unlocked emit
timing, plaintext secret variants × 5).
FIXED:
1. **`granted_scopes` removed from INSUFFICIENT_SCOPE response details**
(Art.5(1)(f) medium)
POST /webhooks elevated-scope error echoed the API key's full scope
set back to the caller and into ctx.log structured fields. Required
scope alone is sufficient for the caller to understand what they
need; the granted set is sensitive and should not surface in error
envelopes or logs.
2. **Redirect error → terminal `dead` + auto-disable** (CC6.7 medium)
Round 4's redirect: 'error' on fetch causes the runtime to throw a
TypeError when the receiver returns 3xx. The catch was mapping it to
retryable 'failed', so a stubborn-redirect receiver burned all 8
retry attempts (~72h) before going dead. Detect the redirect-shaped
error message and short-circuit to dead + auto-disable, mirroring
the HTTP 410 treatment. Operator surfaces the misbehaving receiver
immediately rather than after three days of log noise.
Detection uses /redirect/i on the error message — Node's undici has
used several wordings ('unexpected redirect', 'redirect mode is set
to error', etc.) across versions; case-insensitive substring is the
stable shape.
3. **Retry route re-runs `validateWebhookUrl` against current URL**
(CC6.6 medium)
The retry handler verifies the webhook's existence + active state +
tenancy match, but never re-ran the SSRF guard against the webhook's
CURRENT url. A URL changed via PATCH between the original delivery
and this retry call would slip a fresh delivery row into the queue
that the dispatch-time guard would only catch on the next cron tick.
Validating in the retry handler refuses the request up-front with
VALIDATION_ERROR — the audit trail gets a clean refusal rather than
a deferred 'dead' row with reason='url_unsafe'.
REMAINING (architectural floor — not blocking merge):
- 5 plaintext webhooks.secret findings (V14 / V11.1 / Art.32 / A.8.24 /
CC6.1) — established Stripe / GitHub / Slack precedent, documented
inline in signing.ts.
- V8.2.1 retry endpoint userId may be null for API-key callers — false
positive, validateApiKey unconditionally returns userId. Bot has
re-flagged 7 rounds in a row.
- V1.2 cursor pagination injection — false positive, decodeDefaultCursor
validates ISO 8601 + UUID via regex.
- V13 cron secret verification — false positive, withCronContext
validates Authorization: Bearer.
- V8.2.1 deliveries cross-webhook leak — false positive, bot
acknowledges the .eq('webhook_id') filter handles it.
- V1.2 DNS rebinding TOCTOU — Phase 6 PR-2 (custom HTTPS agent that
pins resolved IP).
- V2.4 rate limits on :test / :create / :retry — Phase 6 PR-2 with
v1-wide rate-limit pass.
- V16.1 / A.8.15 / A.8.16 / CC7.2 SIEM / log drain / monitoring —
out-of-repo infra.
- Art.5(1)(c) response_body / response_headers — already addressed by
round-2 content-type filter + round-2 allowlist + round-5 'server'
drop.
- Art.5(1)(e) 90-day TTL non-accounting deliveries — Phase 6 PR-2.
- Art.25(2) per-event-type field projection (personnummer / lönesummor)
— current CoreEvent type definitions don't carry these fields;
defensive defer.
- Art.9 DPIA / RoPA entries for outbound webhooks — out-of-repo doc.
- A.8.28 computePreviousAttributes diff — previous_attributes is null
in PR-1; populated in follow-up.
- A.5.17 / V11.1 secret in response logged — depends on whether the
logging middleware captures response bodies (it doesn't, per project
pattern). Defensive defer.
- CC9.2 TLS validation / CC3.2 credential-pattern scrub — out-of-scope
hardening.
- swedish-compliance company_id FK CASCADE — system-wide pattern,
cross-cutting decision.
- swedish-compliance period.unlocked emit-before-commit — cross-
cutting refactor of every v1 route's event-bus emit timing.
- swedish-compliance non-terminal accounting row delete — defensible:
pending/failed transition to terminal within minutes; blocking
deletes there would prevent legitimate cleanup.
- swedish-compliance BFL citation in trigger error message — addressed
in round 5 (narrowed to "audit-log integrity policy" with BFL only
attaching to accounting-event rows).
If round 7 plateaus or the count drops, that's the merge-ready signal.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
