Merge pull request #405 from Goldii-locks/feat/api-key-fail-closed

fix(security): make the job routes' API-key gate fail closed

Author: godamongstmen897

__tests__/by-wallet-hardening.test.ts

@@ -1,6 +1,7 @@
 import { jest } from "@jest/globals";
 import request from "supertest";
 import express from "express";
+import { autoAuth, TEST_API_KEY } from "./helpers/api-key-helper.js";
 import { resetByWalletRateLimitBuckets, walletLookupLimiter } from "../src/middleware/rateLimiter.js";
 
 const mockGetJobsByWallet = jest.fn();
@@ -26,6 +27,7 @@ const { default: router } = await import("../src/routes/jobs.js");
 function buildApp() {
   const app = express();
   app.use(express.json());
+  app.use(autoAuth);
   app.use("/api/jobs", router);
   return app;
 }

__tests__/by-wallet-security.test.ts

@@ -1,5 +1,6 @@
 import request from "supertest";
 import express from "express";
+import { autoAuth, TEST_API_KEY } from "./helpers/api-key-helper.js";
 import { jest } from "@jest/globals";
 
 // Mock the indexer DB so we don't need a real SQLite connection for security tests
@@ -15,6 +16,7 @@ const { default: router } = await import("../src/routes/jobs.js");
 
 const app = express();
 app.use(express.json());
+app.use(autoAuth);
 app.use("/api/jobs", router);
 
 describe("GET /api/jobs/by-wallet/:address — security and CORS", () => {

__tests__/by-wallet.test.ts

@@ -1,6 +1,7 @@
 import Database from "better-sqlite3";
 import request from "supertest";
 import express from "express";
+import { autoAuth, TEST_API_KEY } from "./helpers/api-key-helper.js";
 import type { Request, Response } from "express";
 import { resetByWalletRateLimitBuckets } from "../src/middleware/rateLimiter.js";
 import {
@@ -317,9 +318,8 @@ describe("GET /api/jobs/by-wallet/:address – HTTP", () => {
     // Dynamically import the router AFTER setDb() so it uses the in-memory DB
     const { default: router } = await import("../src/routes/jobs.js");
     app = express();
-    // Ensure no API_KEY gate is active for the baseline HTTP suite
-    delete process.env.API_KEY;
     app.use(express.json());
+    app.use(autoAuth);
     app.use("/api/jobs", router);
   });
 
@@ -430,6 +430,7 @@ describe("GET /api/jobs/by-wallet/:address – Zod middleware", () => {
     const { default: router } = await import("../src/routes/jobs.js");
     app = express();
     app.use(express.json());
+    app.use(autoAuth);
     app.use("/api/jobs", router);
   });
 
@@ -549,11 +550,11 @@ describe("GET /api/jobs/by-wallet/:address – status codes", () => {
     const { default: router } = await import("../src/routes/jobs.js");
     app = express();
     app.use(express.json());
+    app.use(autoAuth);
     app.use("/api/jobs", router);
   });
 
   afterEach(() => {
-    delete process.env.API_KEY;
     resetJobsByWalletCache();
   });
 
@@ -669,14 +670,14 @@ describe("GET /api/jobs/by-wallet/:address – status codes", () => {
     expect(res.body.success).toBe(true);
   });
 
-  it("returns 200 (no gate) when API_KEY env var is not set", async () => {
+  it("returns 401 (fails closed) when API_KEY env var is not set", async () => {
     delete process.env.API_KEY;
 
     const res = await request(app)
       .get(`/api/jobs/by-wallet/${VALID_WALLET}`)
-      .expect(200);
+      .expect(401);
 
-    expect(res.body.success).toBe(true);
+    expect(res.body).toEqual({ success: false, error: "Unauthorized" });
   });
 
   // -------------------------------------------------------------------------

__tests__/contract-id-response.test.ts

@@ -1,6 +1,7 @@
 import { jest } from "@jest/globals";
 import request from "supertest";
 import express from "express";
+import { autoAuth, TEST_API_KEY } from "./helpers/api-key-helper.js";
 import logger from "../src/utils/logger.js";
 
 const VALID_CONTRACT =
@@ -21,6 +22,7 @@ const { default: router } = await import("../src/routes/jobs.js");
 function buildApp() {
   const app = express();
   app.use(express.json());
+  app.use(autoAuth);
   app.use("/api/jobs", router);
   return app;
 }
@@ -31,7 +33,6 @@ describe("GET /api/jobs/:contractId – response format and status codes", () =>
   beforeEach(() => {
     mockGetAccount.mockReset();
     mockSimulateTransaction.mockReset();
-    delete process.env.API_KEY;
     mockGetAccount.mockResolvedValue({
       accountId: () =>
         "GAODBHVR63Z56MVQRBEJSYM2H5423LJ4WAPUUBOFG4JYY72S6ROKVZRX",
@@ -85,6 +86,17 @@ describe("GET /api/jobs/:contractId – response format and status codes", () =>
     expect(res.body).toEqual({ success: false, error: "Unauthorized" });
   });
 
+  it("returns 401 (fails closed) when API_KEY is not set", async () => {
+    delete process.env.API_KEY;
+
+    const res = await request(buildApp())
+      .get(`/api/jobs/${VALID_CONTRACT}`)
+      .expect(401);
+
+    expect(res.body).toEqual({ success: false, error: "Unauthorized" });
+    expect(mockGetAccount).not.toHaveBeenCalled();
+  });
+
   it("returns 404 when simulation reports the job was not found", async () => {
     mockSimulateTransaction.mockResolvedValue({
       error: "contract not found on network",
@@ -173,7 +185,6 @@ describe("GET /api/jobs/:contractId – logging traces", () => {
   const origApiKey = process.env.API_KEY;
 
   beforeEach(() => {
-    delete process.env.API_KEY;
     mockGetAccount.mockReset();
     mockSimulateTransaction.mockReset();
     mockGetAccount.mockResolvedValue({

__tests__/helpers/api-key-helper.ts

@@ -0,0 +1,34 @@
+import type { Request, Response, NextFunction } from "express";
+
+/**
+ * Default API_KEY value for suites that are not themselves exercising the
+ * API-key gate. jest.setup.ts installs this before every test.
+ */
+export const TEST_API_KEY = "jest-default-api-key";
+
+/**
+ * Test-only middleware that authenticates requests for suites whose subject
+ * is something other than the API-key gate (validation, caching, error
+ * mapping, rate limiting, logging).
+ *
+ * Background: the job routes used to skip authentication entirely when
+ * API_KEY was unset, so these suites reached their handlers without sending
+ * anything. That gate now fails closed, so they have to authenticate.
+ *
+ * Injection is deliberately conditional on API_KEY still holding
+ * TEST_API_KEY. The dedicated auth suites set their own value (e.g.
+ * "secret-test-key") in a beforeEach, and for those this middleware does
+ * nothing — so assertions like "401 when a key is required but none is
+ * provided" keep testing exactly what they did before. An explicit
+ * x-api-key header on the request is likewise never overwritten, so
+ * wrong-key cases still reach the gate unchanged.
+ */
+export function autoAuth(req: Request, _res: Response, next: NextFunction): void {
+  if (
+    process.env.API_KEY === TEST_API_KEY &&
+    req.headers["x-api-key"] === undefined
+  ) {
+    req.headers["x-api-key"] = TEST_API_KEY;
+  }
+  next();
+}

__tests__/jobs.test.ts

@@ -1,6 +1,7 @@
 import { jest } from "@jest/globals";
 import request from "supertest";
 import express from "express";
+import { autoAuth, TEST_API_KEY } from "./helpers/api-key-helper.js";
 import type { NextFunction, Request, Response } from "express";
 
 const VALID_CONTRACT =
@@ -21,6 +22,7 @@ const { default: router } = await import("../src/routes/jobs.js");
 function buildApp() {
   const app = express();
   app.use(express.json());
+  app.use(autoAuth);
   app.use("/api/jobs", router);
   // Add error interceptor for test coverage
   app.use((err: unknown, _req: Request, res: Response, _next: NextFunction) => {
@@ -34,7 +36,6 @@ describe("GET /api/jobs/:contractId – error interceptor", () => {
   beforeEach(() => {
     mockGetAccount.mockReset();
     mockSimulateTransaction.mockReset();
-    delete process.env.API_KEY;
     mockGetAccount.mockResolvedValue({
       accountId: () =>
         "GAODBHVR63Z56MVQRBEJSYM2H5423LJ4WAPUUBOFG4JYY72S6ROKVZRX",

__tests__/partial-release.test.ts

@@ -1,6 +1,7 @@
 import { jest } from "@jest/globals";
 import request from "supertest";
 import express from "express";
+import { autoAuth, TEST_API_KEY } from "./helpers/api-key-helper.js";
 
 const VALID_CONTRACT = "CDD5WKK3WT3QVKXMXTJNDIXE4T73FK6GGXDSD6UTJAH6YYZU52SQ4MUH";
 const VALID_ADDRESS = "GAODBHVR63Z56MVQRBEJSYM2H5423LJ4WAPUUBOFG4JYY72S6ROKVZRX";
@@ -23,6 +24,7 @@ const { resetPartialReleaseRateLimitBuckets } = await import(
 function buildApp() {
   const app = express();
   app.use(express.json());
+  app.use(autoAuth);
   app.use("/api/jobs", router);
   return app;
 }
@@ -411,13 +413,13 @@ describe("POST /api/jobs/:contractId/milestones/:index/partial-release", () => {
       expect(res.body.success).toBe(true);
     });
 
-    it("returns 200 (no gate) when API_KEY is not set", async () => {
+    it("returns 401 (fails closed) when API_KEY is not set", async () => {
       delete process.env.API_KEY;
       const res = await request(buildApp())
         .post(ENDPOINT)
         .send(VALID_BODY)
-        .expect(200);
-      expect(res.body.success).toBe(true);
+        .expect(401);
+      expect(res.body).toEqual({ success: false, error: "Unauthorized" });
     });
   });
 

__tests__/production-env-assertion.test.ts

@@ -0,0 +1,48 @@
+import { jest } from "@jest/globals";
+
+/**
+ * Covers the production startup assertion in src/index.ts.
+ *
+ * This imports the real entry point rather than re-implementing the check,
+ * so deleting the assertion — or moving it below a side effect — fails here.
+ * ES module imports are evaluated before the module body, so the throw
+ * happens after dotenv.config() but before runMigrations(), startPoller()
+ * and app.listen(); nothing binds a port and no database is touched.
+ */
+describe("production startup assertion (src/index.ts)", () => {
+  const ORIGINAL_ENV = { ...process.env };
+
+  beforeEach(() => {
+    jest.resetModules();
+  });
+
+  afterEach(() => {
+    process.env = { ...ORIGINAL_ENV };
+  });
+
+  it("throws and names every missing variable when NODE_ENV=production", async () => {
+    process.env.NODE_ENV = "production";
+    delete process.env.API_KEY;
+    delete process.env.ADMIN_API_KEY;
+    delete process.env.ALLOWED_ORIGINS;
+    delete process.env.CONTRACT_ID;
+
+    // dotenv.config() may repopulate some of these from a local .env, so
+    // assert on the ones the repo's .env.example does not provide.
+    await expect(import("../src/index.js")).rejects.toThrow(
+      /Missing required production environment variables:.*API_KEY/,
+    );
+  });
+
+  it("names ADMIN_API_KEY specifically when only that one is absent", async () => {
+    process.env.NODE_ENV = "production";
+    process.env.API_KEY = "set";
+    process.env.ALLOWED_ORIGINS = "https://example.test";
+    process.env.CONTRACT_ID = "CDD5WKK3WT3QVKXMXTJNDIXE4T73FK6GGXDSD6UTJAH6YYZU52SQ4MUH";
+    delete process.env.ADMIN_API_KEY;
+
+    await expect(import("../src/index.js")).rejects.toThrow(
+      "Missing required production environment variables: ADMIN_API_KEY",
+    );
+  });
+});

__tests__/route-trace-logging.test.ts

@@ -1,6 +1,7 @@
 import { jest } from "@jest/globals";
 import request from "supertest";
 import express from "express";
+import { autoAuth, TEST_API_KEY } from "./helpers/api-key-helper.js";
 
 const VALID_CONTRACT = "CDD5WKK3WT3QVKXMXTJNDIXE4T73FK6GGXDSD6UTJAH6YYZU52SQ4MUH";
 const VALID_ADDRESS = "GAODBHVR63Z56MVQRBEJSYM2H5423LJ4WAPUUBOFG4JYY72S6ROKVZRX";
@@ -44,6 +45,7 @@ const {
 function buildApp() {
   const app = express();
   app.use(express.json());
+  app.use(autoAuth);
   app.use("/api/jobs", router);
   return app;
 }

__tests__/time-remaining-validation.test.ts

@@ -1,6 +1,7 @@
 import { jest } from "@jest/globals";
 import request from "supertest";
 import express from "express";
+import { autoAuth, TEST_API_KEY } from "./helpers/api-key-helper.js";
 
 const VALID_CONTRACT = "CDD5WKK3WT3QVKXMXTJNDIXE4T73FK6GGXDSD6UTJAH6YYZU52SQ4MUH";
 
@@ -22,6 +23,7 @@ const { resetTimeRemainingRateLimitBuckets } = await import(
 function buildApp() {
   const app = express();
   app.use(express.json());
+  app.use(autoAuth);
   app.use("/api/jobs", router);
   return app;
 }
@@ -56,6 +58,17 @@ describe("GET /api/jobs/:contractId/milestones/:index/time-remaining", () => {
     expect(mockSimulateTransaction).toHaveBeenCalled();
   });
 
+  it("returns 401 (fails closed) when API_KEY is not set", async () => {
+    delete process.env.API_KEY;
+
+    const res = await request(buildApp())
+      .get(`/api/jobs/${VALID_CONTRACT}/milestones/0/time-remaining`)
+      .expect(401);
+
+    expect(res.body).toEqual({ success: false, error: "Unauthorized" });
+    expect(mockSimulateTransaction).not.toHaveBeenCalled();
+  });
+
   // 2. Invalid contractId
   it("returns 400 for an invalid contractId", async () => {
     const res = await request(buildApp())