fix: resolve security and correctness issues (#1099 #1100 #1101 #1102)

.github/workflows/ci.yml

@@ -69,6 +69,40 @@ jobs:
           INTERNAL_API_KEY: test_internal_api_key
           FRONTEND_URL: https://frontend.example.com
 
+  migration-check:
+    needs: supply-chain-audit
+    runs-on: ubuntu-latest
+    services:
+      postgres:
+        image: postgres:16
+        env:
+          POSTGRES_USER: pguser
+          POSTGRES_PASSWORD: pgpass
+          POSTGRES_DB: remitlend_test
+        ports:
+          - 5432:5432
+        options: >-
+          --health-cmd "pg_isready -U pguser"
+          --health-interval 5s
+          --health-timeout 5s
+          --health-retries 10
+    steps:
+      - uses: actions/checkout@v4
+      - name: Use Node.js 20
+        uses: actions/setup-node@v4
+        with:
+          node-version: 20
+          cache: "npm"
+          cache-dependency-path: backend/package-lock.json
+      - name: Install dependencies
+        run: npm ci
+        working-directory: backend
+      - name: Run full migration set against empty database
+        run: npm run migrate:up
+        working-directory: backend
+        env:
+          DATABASE_URL: postgres://pguser:pgpass@localhost:5432/remitlend_test
+
   frontend:
     needs: supply-chain-audit
     runs-on: ubuntu-latest

backend/migrations/1789000000000_ensure-core-tables.js

@@ -25,11 +25,16 @@ export const up = (pgm) => {
     END $$;
   `);
 
-  // Ensure loan_events table matches requested schema
+  // Ensure loan_events table matches requested schema.
+  // Use to_regclass instead of pg_tables: pg_tables only lists ordinary tables
+  // (relkind r/p) and excludes views, so the guard would evaluate TRUE when
+  // loan_events exists as a compat VIEW (created by migration
+  // 1788000000018_unified-contract-events), causing CREATE TABLE to fail with
+  // "relation loan_events already exists".
   pgm.sql(`
     DO $$
     BEGIN
-      IF NOT EXISTS (SELECT FROM pg_tables WHERE schemaname = 'public' AND tablename = 'loan_events') THEN
+      IF to_regclass('public.loan_events') IS NULL THEN
         CREATE TABLE loan_events (
           id SERIAL PRIMARY KEY,
           loan_id INTEGER,

backend/src/__tests__/apiKeyScopes.test.ts

@@ -183,6 +183,26 @@ describe("requireApiKey – scope support", () => {
     });
   });
 
+  describe("constant-time comparison", () => {
+    beforeEach(() => {
+      process.env.INTERNAL_API_KEY = "correctsecret";
+    });
+
+    it("rejects a wrong key that has the same length as the correct key", async () => {
+      const requireApiKey = await loadMiddleware();
+      const next = makeNext();
+      // "wrongsecreXX" has same byte-length as "correctsecret" (13 chars)
+      expect(() =>
+        requireApiKey()(
+          makeReq("wrongsecretXX") as Request,
+          makeRes() as Response,
+          next,
+        ),
+      ).toThrow();
+      expect(next.calls.length).toBe(0);
+    });
+  });
+
   describe("INTERNAL_API_KEY not set", () => {
     beforeEach(() => {
       delete process.env.INTERNAL_API_KEY;

backend/src/__tests__/scoreBreakdown.test.ts

@@ -1,5 +1,6 @@
 import { jest } from "@jest/globals";
 import request from "supertest";
+import jwt from "jsonwebtoken";
 
 // Mock the database connection module before any other imports
 jest.unstable_mockModule("../db/connection.js", () => ({
@@ -47,6 +48,24 @@ describe("GET /api/score/:userId/breakdown", () => {
     expect(response.status).toBe(401);
   });
 
+  it("should return 403 for a token lacking read:score scope", async () => {
+    const tokenWithoutReadScore = jwt.sign(
+      {
+        publicKey: "user123",
+        role: "lender",
+        scopes: ["read:loans", "read:pool"],
+      },
+      process.env.JWT_SECRET!,
+      { algorithm: "HS256", expiresIn: "1h" },
+    );
+
+    const response = await request(app)
+      .get("/api/score/user123/breakdown")
+      .set("Authorization", `Bearer ${tokenWithoutReadScore}`);
+
+    expect(response.status).toBe(403);
+  });
+
   it("should return a breakdown for a valid userId", async () => {
     // Mock the optimized single CTE query (returns all breakdown metrics)
     mockedQuery.mockResolvedValueOnce({

backend/src/middleware/auth.ts

@@ -1,3 +1,4 @@
+import crypto from "node:crypto";
 import type { Request, Response, NextFunction } from "express";
 import { AppError } from "../errors/AppError.js";
 
@@ -89,7 +90,10 @@ export const requireApiKey = (requiredScope?: ApiKeyScope) => {
     const keyStr = Array.isArray(providedKey) ? providedKey[0] : providedKey;
 
     const match = configuredKeys.find((k) => {
-      if (k.secret !== keyStr) return false;
+      const expectedBuf = Buffer.from(k.secret);
+      const providedBuf = Buffer.from(keyStr);
+      if (expectedBuf.length !== providedBuf.length) return false;
+      if (!crypto.timingSafeEqual(expectedBuf, providedBuf)) return false;
       if (requiredScope === undefined) return true; // any valid key is fine
       if (k.scope === null) return true; // legacy key grants all scopes
       return k.scope === requiredScope;

backend/src/routes/loanRoutes.ts

@@ -68,16 +68,6 @@ if (process.env.NODE_ENV === "test" || process.env.NODE_ENV === "development") {
   );
 }
 
-// TEST/DEV ONLY: Mark a loan as defaulted for test setup
-if (process.env.NODE_ENV === "test" || process.env.NODE_ENV === "development") {
-  router.post(
-    "/:loanId/mark-defaulted",
-    requireJwtAuth,
-    requireLoanOwner,
-    markLoanDefaulted,
-  );
-}
-
 router.get("/config", getLoanConfigEndpoint);
 
 /**

backend/src/routes/scoreRoutes.ts

@@ -148,6 +148,7 @@ router.get(
 router.get(
   "/:userId/breakdown",
   requireJwtAuth,
+  requireScopes("read:score"),
   requireWalletParamMatchesJwt("userId"),
   validate(getScoreSchema),
   getScoreBreakdown,