Fix/sr 131 134 security hardening

backend/.env.example

@@ -14,6 +14,8 @@ AWS_REGION=us-east-1
 #   - swiftremit/FX_API_KEY (optional)
 #   - swiftremit/JWT_SECRET (for API service)
 #   - swiftremit/WEBHOOK_SECRET_{ANCHOR_ID} (per anchor)
+#   - swiftremit/ENCRYPTION_KEY (optional; required outside NODE_ENV=test —
+#     see backend/src/privacy/encryption.ts, SR-131)
 
 # PostgreSQL Connection Pool Tuning
 DB_POOL_MAX=20
@@ -73,9 +75,16 @@ WEBHOOK_RETRY_BASE_MS=1000
 WEBHOOK_RETRY_MAX_MS=300000
 WEBHOOK_RETRY_JITTER_PERCENT=20
 
-# SEP-12 KYC Webhook Configuration
-# Comma-separated list of anchor IDs that are trusted to send KYC webhooks
-TRUSTED_ANCHOR_IDS=moneygram,circle,nexo
+# SEP-12 KYC Webhook Configuration (SR-131)
+# Each anchor's webhook requests must carry an HMAC-SHA256 signature (header
+# x-webhook-signature, computed over "${x-webhook-timestamp}.${rawBody}")
+# verified against a per-anchor secret named WEBHOOK_SECRET_<ANCHOR_ID>
+# (anchor id uppercased, non-alphanumeric characters replaced with `_`).
+# TRUSTED_ANCHOR_IDS is no longer read — anchor_id alone was never a real
+# authentication check, since it is a caller-controlled URL parameter.
+# WEBHOOK_SECRET_MONEYGRAM=changeme
+# WEBHOOK_SECRET_CIRCLE=changeme
+# WEBHOOK_SECRET_NEXO=changeme
 
 # KYC Re-verification (#862)
 KYC_RENEWAL_BASE_URL=https://app.swiftremit.io/kyc/renew

backend/AUTH_MATRIX.md

@@ -0,0 +1,50 @@
+# Backend authorisation matrix
+
+Companion to `api/AUTH_MATRIX.md`, covering the `backend` service. Historically
+this service had no equivalent contract, which is how `/api/admin`, `/api/aml`
+and `/api/compliance` shipped reachable with no authentication at all (see the
+SR-131 finding this file was added to close). Every route group below and the
+guard it requires must match `src/__tests__/auth-matrix.test.ts`; drift fails
+the build.
+
+## Guard types
+
+| Guard | Enforcement |
+|---|---|
+| `public` | No authentication |
+| `apiKeyRateLimiter` | Flat per-IP/key bucket only — no identity check |
+| `scopedApiKey(scope)` | `scopedApiKeyMiddleware` (`src/middleware/api-key-rate-limit.ts`) resolves the caller's API key, rejects missing/revoked/expired keys with 401, and rejects keys lacking `scope` with 403. The mapping from route prefix to required scope lives in `ROUTE_SCOPES` (`src/middleware/api-key-store.ts`) — that table is the enforcement source of truth, this document is the human-readable mirror of it. |
+| `officer` | `requireOfficer` (`src/routes/aml.ts`) — resolves from the verified API-key owner first, x-officer-id header only as legacy fallback |
+
+`admin:*` is a wildcard scope that satisfies every other scope check
+(`ApiKeyStore.hasScope`).
+
+## Matrix
+
+| Method | Route prefix | Guard | Notes |
+|---|---|---|---|
+| GET/POST/DELETE/PATCH | `/api/admin/*` | `scopedApiKey(admin:*)` | Audit log, jobs, webhook rotation. Was previously reachable with only a 20 req/min IP limiter. |
+| GET | `/api/aml/*` | `scopedApiKey(read:compliance)` | Screening results, alert queue, SAR/travel-rule reads. |
+| POST/PATCH | `/api/aml/*` | `scopedApiKey(admin:*)` + `officer` | Mutating AML actions also require `requireOfficer`, which now derives the actor from the API-key owner rather than the caller-supplied `x-officer-id` header. |
+| GET | `/api/compliance/*` | `scopedApiKey(read:compliance)` | Includes `/api/compliance/report`, which returns joined remittance + transaction PII and previously had no auth at all. |
+| POST/PATCH | `/api/compliance/*` | `scopedApiKey(admin:*)` | Threshold and flag-status mutations. |
+| GET | `/api/devices/*` | `scopedApiKey(read:devices)` | |
+| POST/PATCH/DELETE | `/api/devices/*` | `scopedApiKey(admin:*)` | |
+| POST/GET/DELETE | `/api/developers/keys` | `scopedApiKey(admin:*)` + Bearer owner token | Unchanged from SR-043. |
+| GET | `/api/verification/*` | `scopedApiKey(read:verification)` | Most permissive; default free-tier scope. |
+| GET | `/api/kyc/*` | `scopedApiKey(read:kyc)` | |
+| POST | `/api/kyc/register` | `scopedApiKey(write:kyc)` | |
+| POST | `/api/remittance`, `/api/fx-rate` | `scopedApiKey(write:remittance)` | |
+| GET | `/api/remittance/*`, `/api/fx-rate/*` | `scopedApiKey(read:remittance)` | |
+| ALL | `/health`, `/metrics`, `/api/docs` | `public` | No PII, no mutation. |
+| POST | `/webhooks/kyc/:anchor_id` | HMAC signature (`verifyAnchorSignature`, SR-131) | Previously `public` — see the KYC webhook finding. |
+
+## Known limitations
+
+- `ROUTE_SCOPES` matches on a literal method+path-prefix string, not the
+  Express route pattern — a route registered under a prefix not listed here
+  silently requires no scope (`requiredScopeForRoute` returns `null`). New
+  routers under `/api/*` must add an entry here and in `ROUTE_SCOPES`.
+- Officer/admin identity is still just "the API key's `owner_id`" — there is
+  no role table distinguishing an operations officer from any other key
+  owner. A key with `admin:*` can act as an AML officer.

backend/src/__tests__/auth-matrix.test.ts

@@ -0,0 +1,63 @@
+import { describe, it, expect } from 'vitest';
+import request from 'supertest';
+import app from '../api';
+import { ROUTE_SCOPES, requiredScopeForRoute } from '../middleware/api-key-store';
+
+/**
+ * SR-131 — backend authorisation-matrix drift test.
+ *
+ * Mirrors api/src/__tests__/auth-matrix.test.ts (SR-048) for the backend
+ * service. Before this fix, /api/admin, /api/aml and /api/compliance were
+ * mounted with only a flat rate limiter and no identity check at all —
+ * these assertions fail loudly if that regresses.
+ */
+describe('backend/AUTH_MATRIX.md — route scope drift', () => {
+  it('declares a scope for every previously-unauthenticated route group', () => {
+    for (const prefix of ['GET /api/admin', 'GET /api/aml', 'GET /api/compliance', 'GET /api/devices']) {
+      const [method, path] = prefix.split(' ');
+      expect(requiredScopeForRoute(method, path)).not.toBeNull();
+    }
+  });
+
+  it('has no route-scope entry with an empty scope', () => {
+    for (const entry of ROUTE_SCOPES) {
+      expect(entry.scope).toBeTruthy();
+    }
+  });
+});
+
+describe('SR-131 — admin/aml/compliance reject unauthenticated requests', () => {
+  it('rejects GET /api/admin/audit-log with no API key', async () => {
+    const res = await request(app).get('/api/admin/audit-log');
+    expect(res.status).toBe(401);
+  });
+
+  it('rejects GET /api/admin/jobs with no API key', async () => {
+    const res = await request(app).get('/api/admin/jobs');
+    expect(res.status).toBe(401);
+  });
+
+  it('rejects GET /api/aml/alerts/summary with no API key', async () => {
+    const res = await request(app).get('/api/aml/alerts/summary');
+    expect(res.status).toBe(401);
+  });
+
+  it('rejects GET /api/compliance/report with no API key', async () => {
+    const res = await request(app).get('/api/compliance/report');
+    expect(res.status).toBe(401);
+  });
+
+  it('rejects GET /api/devices with no API key', async () => {
+    const res = await request(app).get('/api/devices');
+    expect(res.status).toBe(401);
+  });
+
+  it('rejects an API key lacking the required scope with 403', async () => {
+    // A key that only carries read:verification must not reach admin routes.
+    const res = await request(app)
+      .get('/api/admin/jobs')
+      .set('x-api-key', 'sr_live_not-a-real-key-but-store-lookup-will-401-first');
+    // Unknown key → 401 (invalid), not a silent pass-through.
+    expect([401, 403]).toContain(res.status);
+  });
+});

backend/src/__tests__/encryption-fail-closed.test.ts

@@ -0,0 +1,56 @@
+import { describe, it, expect, afterEach } from 'vitest';
+
+/**
+ * SR-131 — encryption.ts must fail closed when ENCRYPTION_KEY is unset.
+ *
+ * Before this fix, getMasterKey() silently returned a literal, publicly
+ * visible constant (DEFAULT_KEY_HEX) committed to the repository whenever
+ * ENCRYPTION_KEY was missing, in every environment including production —
+ * "encrypted" PII would be ciphertext anyone reading the source already had
+ * the key for. Import is deferred to a helper inside each test so the module
+ * (and its NODE_ENV check) is re-evaluated per env var combination — Node's
+ * module cache would otherwise serve the first import's behaviour.
+ */
+async function freshEncryptionModule() {
+  const modPath = '../privacy/encryption';
+  const resolved = require.resolve(modPath);
+  delete require.cache[resolved];
+  return require(modPath) as typeof import('../privacy/encryption');
+}
+
+describe('privacy/encryption — fail closed without ENCRYPTION_KEY', () => {
+  const originalKey = process.env.ENCRYPTION_KEY;
+  const originalEnv = process.env.NODE_ENV;
+
+  afterEach(() => {
+    if (originalKey === undefined) delete process.env.ENCRYPTION_KEY;
+    else process.env.ENCRYPTION_KEY = originalKey;
+    process.env.NODE_ENV = originalEnv;
+  });
+
+  it('throws instead of silently using a known default key outside test env', async () => {
+    delete process.env.ENCRYPTION_KEY;
+    process.env.NODE_ENV = 'production';
+    const { encryptColumn } = await freshEncryptionModule();
+
+    expect(() => encryptColumn('sensitive-value')).toThrow(/ENCRYPTION_KEY is not set/);
+  });
+
+  it('throws on decryptColumn too, not just encryptColumn', async () => {
+    delete process.env.ENCRYPTION_KEY;
+    process.env.NODE_ENV = 'development';
+    const { decryptColumn } = await freshEncryptionModule();
+
+    expect(() => decryptColumn('enc:v1:aa:bb:cc')).toThrow(/ENCRYPTION_KEY is not set/);
+  });
+
+  it('does not throw when ENCRYPTION_KEY is set, regardless of environment', async () => {
+    process.env.ENCRYPTION_KEY = 'a'.repeat(64);
+    process.env.NODE_ENV = 'production';
+    const { encryptColumn, decryptColumn, isEncrypted } = await freshEncryptionModule();
+
+    const encrypted = encryptColumn('sensitive-value')!;
+    expect(isEncrypted(encrypted)).toBe(true);
+    expect(decryptColumn(encrypted)).toBe('sensitive-value');
+  });
+});