fix(security): resolve PR #1703 review findings (CSRF FPs, secret fixture)

.semgrep/semgrep-suppressions.yml

@@ -6,4 +6,31 @@
 # The security workflow fails when a nosemgrep comment omits a reason or
 # justification. Add accepted suppressions here during security review so future
 # reviewers can audit the exception history.
-accepted_suppressions: []
+accepted_suppressions:
+  # backend/server/createApiServer.ts — POST /api/v1/stripe/billing/webhook
+  # `app.post(path, express.raw(...))` registers a body parser before
+  # `express.json()` so the HMAC-signed bytes survive; it declares no handler.
+  # The route is mounted by `createStripeBillingRouter`, which verifies the
+  # signature (503 when unconfigured, 401 on mismatch) before decoding.
+  - rule_id: subtrackr.csrf.state-changing-route-without-csrf
+    path: backend/server/createApiServer.ts
+    reason: >-
+      Body-parser registration only; signed-webhook verification lives in
+      createStripeBillingRouter. Not duplicated at the app level because
+      verify() mutates the replay-dedup map and a second call would mark the
+      first legitimate delivery as a duplicate.
+    reviewed_by: security review of PR #1703
+    expires: 2027-04-16
+
+  # backend/server/createApiServer.ts — POST /api/v1/paddle/webhook
+  # Same shape as the Stripe registration above; verification lives in
+  # `createPaddleRouter`.
+  - rule_id: subtrackr.csrf.state-changing-route-without-csrf
+    path: backend/server/createApiServer.ts
+    reason: >-
+      Body-parser registration only; signed-webhook verification lives in
+      createPaddleRouter. Not duplicated at the app level because verify()
+      mutates the replay-dedup map and a second call would mark the first
+      legitimate delivery as a duplicate.
+    reviewed_by: security review of PR #1703
+    expires: 2027-04-16

backend/server/createApiServer.ts

@@ -161,7 +161,22 @@ export function createApiServer(options: CreateApiServerOptions = {}): Express {
   // registered for the webhook paths *before* the JSON parser, so
   // `req.body` is a Buffer holding the untouched payload. `type: '*/*'` is
   // required because providers do not agree on a Content-Type.
+  //
+  // These two registrations install a *body parser only*: they declare no
+  // handler and perform no state change. The routes themselves are mounted
+  // further down via `createStripeBillingRouter` / `createPaddleRouter`, and
+  // both verify an HMAC signature over the raw bytes before the payload is
+  // decoded — answering 503 when the secret is unconfigured and 401 when the
+  // signature does not match. The CSRF rule cannot see across that file
+  // boundary, hence the suppressions below.
+  //
+  // Note: verification must NOT be duplicated here. `verify()` records the
+  // event id for replay de-duplication, so a second call would report the
+  // first legitimate delivery as a duplicate and the router would answer
+  // `200 { duplicate: true }` without ever dispatching the event.
+  // nosemgrep: subtrackr.csrf.state-changing-route-without-csrf -- reason: body-parser-only registration, no handler and no state change; the request is HMAC-verified inside the Stripe router, which fails closed with 503 when unconfigured and 401 on a bad signature -- expires: 2027-04-16
   app.post('/api/v1/stripe/billing/webhook', express.raw({ type: '*/*', limit: MAX_JSON_BODY }));
+  // nosemgrep: subtrackr.csrf.state-changing-route-without-csrf -- reason: body-parser-only registration, no handler and no state change; the request is HMAC-verified inside the Paddle router, which fails closed with 503 when unconfigured and 401 on a bad signature -- expires: 2027-04-16
   app.post('/api/v1/paddle/webhook', express.raw({ type: '*/*', limit: MAX_JSON_BODY }));
 
   // An explicit cap keeps a single oversized POST from buffering the whole

backend/shared/security/__tests__/outboundUrlGuard.test.ts

@@ -72,7 +72,10 @@ describe('isPrivateHostname', () => {
 
 describe('validateOutboundUrl', () => {
   it('accepts a normal public https URL', () => {
-    const result = validateOutboundUrl('https://hooks.zapier.com/hooks/catch/1/2/');
+    // A real Zapier host, but deliberately *not* a `/hooks/catch/<n>/<n>/`
+    // path: that shape is indistinguishable from a live Zapier hook URL and
+    // trips secret detectors in CI (the assertion only needs "public host").
+    const result = validateOutboundUrl('https://hooks.zapier.com/example-endpoint');
     expect(result.protocol).toBe('https:');
     expect(result.hostname).toBe('hooks.zapier.com');
   });