Defect-discovery audit + fail-closed remediation across money, auth, and compliance paths

[devin-ai-integration]

Summary

Codebase-wide defect audit (methodology from the attached defect-discovery prompt) plus remediation of the confirmed findings. The audit report is committed at docs/security/defect-discovery-audit.md with file:line evidence, the Phase 0 maps, composition chains, negative results, and the residual register for findings that need policy/product decisions rather than code.

The dominant defect class was fabricated success on dependency failure: when TigerBeetle, Temporal, Mojaloop, Redis, Permify, the risk scorers, Wazuh, or the DB were unavailable, the system did not fail — it manufactured a plausible answer and told the trader, the officer, and the audit trail that money had moved or that nothing was wrong. Every such path is now fail-closed.

Representative shape of the change:

-await fetch(TEMPORAL, {...}).catch(err => log("falling back to direct confirm"));
-const updated = await updatePayment(id, { status: "confirmed" });   // money confirmed with no settlement
+const resp = await fetch(TEMPORAL, {...});          // throws -> SERVICE_UNAVAILABLE
+if (!resp.ok) throw new TRPCError({ code: "SERVICE_UNAVAILABLE", ... });
+return { paymentId, workflowId, accepted: true };   // payment stays pending until the workflow confirms

Money integrity

• payments.confirm no longer confirms a payment when Temporal is unreachable or returns non-2xx; payments.initiate no longer returns a created payment when the queue insert fails (the orphan payments row is removed and the error propagates).
• paymentWorker no longer decides transfer outcomes with Math.random() when the Mojaloop switch is down; unavailable items stay queued and are retried by the existing back-off (previously they were parked in failed, which the poller never claims again).
• ledger.* no longer writes Postgres rows with status: "posted" when the TigerBeetle bridge is unavailable (duty transfers, bond deposit/release, penalties, transit guarantees) — the bridge is the ledger of record, so those paths now throw. paymentRisk no longer defaults to LOW / APPROVE / 0.10 when the scorer is offline.
• mojaloop.getPaymentStatus was a query that settled payments: after 5s it flipped PENDING → PROCESSING, after 15s it fabricated an ILP fulfilment, wrote a posted ledger entry and an audit event. It is now read-only and ownership-scoped.
• mojaloop.initiatePayment derives the amount from decl.totalDue instead of trusting the client, checks declaration ownership, and rejects switch failures instead of falling back to simulation. The availability check moved ahead of the row insert and the failure path cleans up, so an outage no longer leaves an orphan PENDING transfer per attempt, and the initiation audit event is written only once the switch has accepted.
• The Mojaloop callback moved from a public tRPC procedure authenticated by a secret in the request body (defaulting to "dev-webhook-secret") to server/webhooks/mojaloop.ts: Express raw-body HMAC-SHA256 over the exact bytes, timingSafeEqual, and a 503 (so the switch redelivers) when TigerBeetle cannot accept the settlement transfer. Because delivery is at-least-once and that 503 invites redelivery, settlement is ordered ledger-first and guarded: status: "COMMITTED" is persisted only after the bridge transfer and ledger entry succeed (otherwise the transfer would read settled via isSettled with nothing posted), and a redelivery is short-circuited by the existing ledger entry for that mojaloopTransferId, with concurrent redeliveries serialised by an atomic claim on the existing payment_idempotency_keys table so revenue cannot be double-credited.

Authorization and identity

• onboarding.selectRole let any authenticated user assign themselves customs_officer, oga_officer, inspector or finance — and downstream gates trust the DB role. Self-selection is now limited to user.
• auditEngine.* was entirely publicProcedure (unauthenticated post-clearance audit creation, findings, closure, appeals). Now authenticated, role-gated, and trader-scoped.
• fund-flow idempotency (return false // fail open) and its local Permify helper (return true // fail open — Permify unavailable) both failed open; the helper was also dead code. Now fail-closed via assertCan, with officer/admin gates on the control-plane procedures (settlement, reconciliation, reversals, penalties, drawback approval, audit recovery, account provisioning).
• Session revocation checks failed open when Redis was down, so a revoked session kept working during a Redis outage; default session lifetime was one year. Now: revocation-check failure rejects the session, default lifetime SEVEN_DAYS_MS. A present-but-invalid Bearer token now returns UNAUTHORIZED instead of silently falling through to the session cookie — the rejection lives in sdk.authenticateRequest, where the fallback actually happened, so a cookie can no longer rescue a failed Bearer.
  • Making revocation fail closed meant the Redis singleton could no longer latch: getRedis() tripped _redisFailed = true permanently on the first connection error, which (harmless when the only consumer degraded to an in-memory rate limiter) would have turned any transient Redis blip into a permanent lockout of every user until the process restarted, and lazyConnect made the first request after each boot fail against a healthy Redis. The latch is gone, ioredis reconnects on its own, and commands await readiness on a 500ms budget — so the outage lasts exactly as long as Redis is down, and a partition can't hold request handlers open waiting for an answer that is a foregone rejection. The rate limiter still degrades open to in-memory; only revocation fails closed. The tradeoff that remains is deliberate and recorded in the audit report's residual register: a total Redis outage signs everyone out for its duration, which is the price of a revoked session not surviving one. DEMO_MODE can no longer bypass Permify or mount the demo login route in production.
• tradeFinance took applicantId/traderId from client input; ordinary users are now bound to ctx.user.id, with overrides only for officer/admin roles. batchPayments operational reads are admin-gated.

Secrets, webhooks, crons

• validateWebhookSecrets() existed but had no call site; it now runs at startup. Committed dev-default secrets (tradegateway-oga-webhook-secret-dev, tradegateway-cep-webhook-secret-dev, dev-webhook-secret) are gone, and a missing secret now rejects requests instead of disabling verification — the sanctions webhook's if (WEBHOOK_SECRET && provided !== WEBHOOK_SECRET) accepted everything when the env var was empty. Keycloak webhook verification is mandatory, timing-safe, and returns 503 if the durable audit write fails instead of {received: true}.
• JWT_SECRET is required in production, and missing production secrets throw instead of logging a warning.
• /api/scheduled/* handlers are behind cron authentication (matching the existing tenant-domain poller pattern) and return 503 {ok:false} on DB outage instead of a success shape.

Compliance and operational honesty

• Risk scoring: when both the ML scorer and the LLM fail, declarations no longer receive a fabricated (hsHash % 40) + 10 score — which could never produce a red lane — but riskScore: null and the red/manual-inspection lane. valuation throws instead of returning flagged: false when the DB is down.
• Removed synthetic operational data that was indistinguishable from real: Fluvio cargo events, ASEAN inbound messages and acknowledgements, CEN partners/alerts/statistics, Wazuh hardcoded agents, score: 78, and status: "completed" playbook runs that took no action. Clients now render unavailable rather than clean-and-empty, so an integration outage no longer looks like a healthy security posture.
• rulesOfOrigin review is restricted to submitted / under_review, so terminal certificates cannot be re-decided.

Tests

No test was weakened to make the branch pass. 80 tests asserted the removed fabrications (simulated ASEAN/CEN responses, fail-open Permify/idempotency, DB-fallback ledger postings, synthetic Wazuh events) — those were rewritten to assert the fail-closed behaviour. tsc --noEmit (72 errors) and vitest (142 failures, all pre-existing on main) are byte-for-byte identical to the main baseline: no new type errors, no new failing tests.

Link to Devin session: https://app.devin.ai/sessions/e68f0a7bf0e04fb8a3ba32ddb8e1fa23
Requested by: @munisp

[devin-ai-integration]

Original prompt from Patrick

Clone, review and analyze this https://github.com/munisp/singlewindow
Afterwhich, using Attach file analyze the codebase and implement the findings
ATTACHMENT:"https://app.devin.ai/attachments/80878dc3-af42-4287-9b79-2ab7bdfe3f03/codebase-defect-discovery-prompt.pdf"

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

[devin-ai-integration]

Devin Review found 2 potential issues.

View 2 additional findings in Devin Review. (Configure)

[devin-ai-integration]

🔴 Mojaloop transfer marked settled before its ledger entry exists

The COMMITTED branch persists status: "COMMITTED" and logs a committed audit event before the TigerBeetle bridge is checked and the ledger entry is created. When the bridge is down the handler returns 503, but the transaction row already reads COMMITTED, so getPaymentStatus reports isSettled true with no ledger entry (mojaloop.ts). If redelivery stops, the settlement is permanent and unbacked.

Prompt for agents

In registerMojaloopWebhookRoute (server/webhooks/mojaloop.ts), the COMMITTED transfer state is persisted to mojaloop_transactions (updateMojaloopTransaction) and a 'mojaloop_webhook_committed' audit event is logged before the TigerBeetle ledger settlement is attempted. Because getPaymentStatus derives isSettled from the persisted status, a transfer reports as settled even when the ledger posting later fails and the handler returns 503. Reorder so that for COMMITTED events the TigerBeetle bridge settlement (and createLedgerEntry) succeeds FIRST, and only then persist status=COMMITTED / committedAt and log the committed audit event. On ledger failure, leave the status unchanged (or a non-terminal state) and return 503 so redelivery can retry without exposing a phantom-settled transfer.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

Correct, and this was the same class of lie the PR is meant to remove — getPaymentStatus derives isSettled from the persisted status, so a 503 for redelivery still left a transfer reading settled with nothing posted. Fixed in 403cfc8: for COMMITTED, the bridge transfer and the ledger entry must succeed first; only then are status/committedAt/fulfilment persisted and the committed audit event logged. On settlement failure the transaction keeps its prior state and the handler returns 503. RECEIVED/RESERVED/ABORTED are unchanged.

[devin-ai-integration]

🔴 Duplicate revenue postings on webhook redelivery

The COMMITTED block posts a bridge transfer and inserts a ledger entry with no check on the transfer's current state. Webhook delivery is at-least-once, so a redelivered COMMITTED callback for the same transferId posts the ledger transfer again, double-crediting customs revenue.

Prompt for agents

In registerMojaloopWebhookRoute (server/webhooks/mojaloop.ts), the COMMITTED handling unconditionally posts a TigerBeetle transfer and creates a ledger entry. Mojaloop switches redeliver webhooks (at-least-once), and there is no guard against reprocessing a transfer that is already COMMITTED with an existing ledger entry. Add an idempotency guard: before posting to the bridge, check whether the transaction is already in a terminal COMMITTED state with a recorded settlement (e.g. existing ledger entry / fulfilment) and short-circuit to a success response if so, or use a durable idempotency key on the settlement so redelivery cannot double-post revenue.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

Agreed — and the 503 the other fix introduces actively invites redelivery, so this had to land with it. Fixed in 403cfc8 with two layers: a lookup of the existing ledger entry by mojaloopTransferId short-circuits a redelivery to success without re-posting, and an atomic claim on the existing payment_idempotency_keys table (INSERT ... ON CONFLICT DO NOTHING) keyed on the transfer serialises concurrent redeliveries so only one can post. The claim is released when the bridge did not accept the transfer, and retained once it did, so a failure after acceptance can't produce a second bridge transfer. server/mojaloop.webhook.test.ts covers the bridge-unavailable, sequential-duplicate, and concurrent-duplicate cases.

[devin-ai-integration]

Runtime test report — fail-closed remediation (d33cd09)

Tested against a local dev instance (Postgres + Redis up; TigerBeetle bridge, Temporal, Mojaloop switch, Fluvio, Kafka, Wazuh, ASEAN/CEN deliberately absent). ⚠️ No UI evidence in this report: the browser automation subsystem on the test box was unavailable for the entire run, so all visual assertions (amber "unavailable" screens, RED lane badge, onboarding role card) are untested. Everything below is HTTP + database evidence.

Mojaloop settlement fails closed and cannot double-post (the money path)

Real mojaloop_transactions row, valid raw-body HMAC-SHA256 COMMITTED callback, TigerBeetle bridge down:

POST /api/webhooks/mojaloop  x2
→ 503 {"error":"Ledger settlement unavailable; webhook will be retried"}

transfer_id    | status  | committed_at
TRF-TEST-503-A | PENDING | (null)

tigerbeetle_ledger_entries for transfer .......... 0
audit_events 'mojaloop_webhook_committed' ....... 0
leftover settlement claim (sha256 mojaloop-webhook-settlement:…) . absent

mojaloop.getPaymentStatus → {status: PENDING, isSettled: false, committedAt: null, fulfilment: null}

Three consecutive deliveries each retried cleanly (no already being processed wedge) and produced zero ledger rows. Signature auth: missing → 401, wrong → 401, valid → past auth; no configured secret → 503.

Session/auth regressions found during testing and fixed in d33cd09

Two regressions introduced by earlier revisions of this PR, both now verified fixed at runtime:

• Redis-backed session revocation became permanently latched, locking every user out until a process restart. Now: Redis stopped → authenticated requests 401; Redis restarted → 200 within 2s with the same Node pid, no restart.
• First authenticated request after each boot spuriously 401'd. Now 200 on the first request across 3 consecutive boots.
• Invalid Authorization: Bearer <garbage> alongside a valid session cookie previously returned 200 (silent fallback to the cookie). Now 401 "Bearer token verification failed"; no-Bearer + valid cookie still 200; public routes unaffected.

Other fail-closed assertions (HTTP)

• declarations.submit with the ML/LLM scorers down → riskLane: "red", riskScore: null, aiExplanation.source: "unavailable", status under_assessment. The RED badge / absent gauge in the UI is untested.
• mojaloop.initiatePayment with the switch down, amount exactly equal to totalDue → SERVICE_UNAVAILABLE "Mojaloop switch is unavailable"; no payments row, no ledger row. Amount-mismatch and currency guards also fire. (Minor, filed as follow-up: a PENDING mojaloop_transactions row is still inserted before the availability check — pending, not settled, but it leaves an orphan row per attempt.)
• stream.getRecentEvents → {events: [], count: 0, source: "unavailable", unavailable: true, reason: "Fluvio consumer unavailable"} — no synthetic cargo events. Amber banner untested.
• auditEngine.getAuditStats: unauthenticated → 401; authenticated trader → 403 "Audit officer access required".
• Webhooks: /api/webhooks/oga → 401, /api/webhooks/cep-event → 401, /api/webhooks/sanctions-hit → 401, /api/webhooks/keycloak-event → 503 (no secret configured). Note for future testers: /api/webhooks/cep and /api/webhooks/sanctions (without the suffixes) fall through to the Vite HTML and return 200 — the real routes are the suffixed ones.
• POST /api/scheduled/sla-breach-escalation unauthenticated → 503 {"ok":false,"error":"Cron authentication unavailable"}, not {ok:true,processed:0}.

Pre-existing issues hit during setup (present on main, out of scope for this PR)

• drizzle-kit migrate / pnpm db:push fails with Postgres 55P04 (enum created and used in one transaction); drizzle-kit push --force works.
• scripts/seed-demo-users.sql step 2 fails: onboarding_progress.current_step is enum onboarding_step but the script inserts integer 6.
• Dev requires VITE_APP_ID (else every session JWT is rejected: [Auth] Session payload missing required fields) and VITE_OAUTH_PORTAL_URL (else the React app crashes on load: TypeError: Invalid URL at getLoginUrl); neither is documented outside .env.example.
• pnpm@11.12.0 (declared in package.json) is incompatible with Node 22.12 — pnpm 10 works.

Test baseline for reviewers: tsc --noEmit (72 errors) and vitest (142 failing tests) are identical on this branch and on main; the four failing files in the sweep were diffed test-name-by-test-name against a clean main worktree with an empty diff. This repo has no CI checks configured, so there is no pipeline result to point at.