Skip to content

Commit 3063cd5

Browse files
fix(security): make the job routes' API-key gate fail closed
The six API_KEY checks in src/routes/jobs.ts were written as `if (requiredApiKey) { ...verify... }`, which skipped authentication entirely whenever API_KEY was unset. A deployment that forgot to set it served these endpoints to anyone, with nothing in the logs to say so: GET /api/jobs/by-wallet/:address GET /api/jobs/:contractId GET /api/jobs/:contractId/whitelist POST /api/jobs/:contractId/whitelist/update POST /api/jobs/:contractId/milestones/:index/partial-release GET /api/jobs/:contractId/milestones/:index/time-remaining requireAdmin() in middleware/adminAuth.ts already failed closed, so the two halves of the codebase disagreed on what a missing key means. Replace all six copies with one ensureApiKey() helper that rejects when API_KEY is unset, logging at error level so a misconfigured deployment is visible. Six duplicated copies are what let this drift in the first place. Also add a production startup assertion to src/index.ts. It runs directly after dotenv.config(), before migrations, the poller and listen, so a misconfigured container exits instead of booting into a state that passes /health while serving job endpoints unauthenticated (API_KEY unset), rejecting the real frontend origin (ALLOWED_ORIGINS defaulting to localhost:3000), or indexing nothing against testnet (CONTRACT_ID unset). Tests: closing the gate invalidated 113 existing tests that reached their handlers only because the gate was open. They are not weakened — each now authenticates through a test-only autoAuth middleware that injects the key only while API_KEY still holds jest.setup's default, so the dedicated auth suites keep full control of the header and their "no key"/"wrong key" assertions are untouched. Two tests that asserted the old behaviour ("returns 200 (no gate) when API_KEY is not set") now assert 401. Adds one fail-closed test per route plus two covering the startup assertion, which imports the real entry point rather than re-implementing the check. Both verified by mutation: reverting ensureApiKey to fail-open fails exactly six tests, one per route; removing the startup assertion fails exactly two. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 2b4eccc commit 3063cd5

17 files changed

Lines changed: 234 additions & 73 deletions

__tests__/by-wallet-hardening.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { jest } from "@jest/globals";
22
import request from "supertest";
33
import express from "express";
4+
import { autoAuth, TEST_API_KEY } from "./helpers/api-key-helper.js";
45
import { resetByWalletRateLimitBuckets, walletLookupLimiter } from "../src/middleware/rateLimiter.js";
56

67
const mockGetJobsByWallet = jest.fn();
@@ -26,6 +27,7 @@ const { default: router } = await import("../src/routes/jobs.js");
2627
function buildApp() {
2728
const app = express();
2829
app.use(express.json());
30+
app.use(autoAuth);
2931
app.use("/api/jobs", router);
3032
return app;
3133
}

__tests__/by-wallet-security.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import request from "supertest";
22
import express from "express";
3+
import { autoAuth, TEST_API_KEY } from "./helpers/api-key-helper.js";
34
import { jest } from "@jest/globals";
45

56
// 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");
1516

1617
const app = express();
1718
app.use(express.json());
19+
app.use(autoAuth);
1820
app.use("/api/jobs", router);
1921

2022
describe("GET /api/jobs/by-wallet/:address — security and CORS", () => {

__tests__/by-wallet.test.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import Database from "better-sqlite3";
22
import request from "supertest";
33
import express from "express";
4+
import { autoAuth, TEST_API_KEY } from "./helpers/api-key-helper.js";
45
import type { Request, Response } from "express";
56
import { resetByWalletRateLimitBuckets } from "../src/middleware/rateLimiter.js";
67
import {
@@ -317,9 +318,8 @@ describe("GET /api/jobs/by-wallet/:address – HTTP", () => {
317318
// Dynamically import the router AFTER setDb() so it uses the in-memory DB
318319
const { default: router } = await import("../src/routes/jobs.js");
319320
app = express();
320-
// Ensure no API_KEY gate is active for the baseline HTTP suite
321-
delete process.env.API_KEY;
322321
app.use(express.json());
322+
app.use(autoAuth);
323323
app.use("/api/jobs", router);
324324
});
325325

@@ -430,6 +430,7 @@ describe("GET /api/jobs/by-wallet/:address – Zod middleware", () => {
430430
const { default: router } = await import("../src/routes/jobs.js");
431431
app = express();
432432
app.use(express.json());
433+
app.use(autoAuth);
433434
app.use("/api/jobs", router);
434435
});
435436

@@ -549,11 +550,11 @@ describe("GET /api/jobs/by-wallet/:address – status codes", () => {
549550
const { default: router } = await import("../src/routes/jobs.js");
550551
app = express();
551552
app.use(express.json());
553+
app.use(autoAuth);
552554
app.use("/api/jobs", router);
553555
});
554556

555557
afterEach(() => {
556-
delete process.env.API_KEY;
557558
resetJobsByWalletCache();
558559
});
559560

@@ -669,14 +670,14 @@ describe("GET /api/jobs/by-wallet/:address – status codes", () => {
669670
expect(res.body.success).toBe(true);
670671
});
671672

672-
it("returns 200 (no gate) when API_KEY env var is not set", async () => {
673+
it("returns 401 (fails closed) when API_KEY env var is not set", async () => {
673674
delete process.env.API_KEY;
674675

675676
const res = await request(app)
676677
.get(`/api/jobs/by-wallet/${VALID_WALLET}`)
677-
.expect(200);
678+
.expect(401);
678679

679-
expect(res.body.success).toBe(true);
680+
expect(res.body).toEqual({ success: false, error: "Unauthorized" });
680681
});
681682

682683
// -------------------------------------------------------------------------

__tests__/contract-id-response.test.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { jest } from "@jest/globals";
22
import request from "supertest";
33
import express from "express";
4+
import { autoAuth, TEST_API_KEY } from "./helpers/api-key-helper.js";
45
import logger from "../src/utils/logger.js";
56

67
const VALID_CONTRACT =
@@ -21,6 +22,7 @@ const { default: router } = await import("../src/routes/jobs.js");
2122
function buildApp() {
2223
const app = express();
2324
app.use(express.json());
25+
app.use(autoAuth);
2426
app.use("/api/jobs", router);
2527
return app;
2628
}
@@ -31,7 +33,6 @@ describe("GET /api/jobs/:contractId – response format and status codes", () =>
3133
beforeEach(() => {
3234
mockGetAccount.mockReset();
3335
mockSimulateTransaction.mockReset();
34-
delete process.env.API_KEY;
3536
mockGetAccount.mockResolvedValue({
3637
accountId: () =>
3738
"GAODBHVR63Z56MVQRBEJSYM2H5423LJ4WAPUUBOFG4JYY72S6ROKVZRX",
@@ -85,6 +86,17 @@ describe("GET /api/jobs/:contractId – response format and status codes", () =>
8586
expect(res.body).toEqual({ success: false, error: "Unauthorized" });
8687
});
8788

89+
it("returns 401 (fails closed) when API_KEY is not set", async () => {
90+
delete process.env.API_KEY;
91+
92+
const res = await request(buildApp())
93+
.get(`/api/jobs/${VALID_CONTRACT}`)
94+
.expect(401);
95+
96+
expect(res.body).toEqual({ success: false, error: "Unauthorized" });
97+
expect(mockGetAccount).not.toHaveBeenCalled();
98+
});
99+
88100
it("returns 404 when simulation reports the job was not found", async () => {
89101
mockSimulateTransaction.mockResolvedValue({
90102
error: "contract not found on network",
@@ -173,7 +185,6 @@ describe("GET /api/jobs/:contractId – logging traces", () => {
173185
const origApiKey = process.env.API_KEY;
174186

175187
beforeEach(() => {
176-
delete process.env.API_KEY;
177188
mockGetAccount.mockReset();
178189
mockSimulateTransaction.mockReset();
179190
mockGetAccount.mockResolvedValue({
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import type { Request, Response, NextFunction } from "express";
2+
3+
/**
4+
* Default API_KEY value for suites that are not themselves exercising the
5+
* API-key gate. jest.setup.ts installs this before every test.
6+
*/
7+
export const TEST_API_KEY = "jest-default-api-key";
8+
9+
/**
10+
* Test-only middleware that authenticates requests for suites whose subject
11+
* is something other than the API-key gate (validation, caching, error
12+
* mapping, rate limiting, logging).
13+
*
14+
* Background: the job routes used to skip authentication entirely when
15+
* API_KEY was unset, so these suites reached their handlers without sending
16+
* anything. That gate now fails closed, so they have to authenticate.
17+
*
18+
* Injection is deliberately conditional on API_KEY still holding
19+
* TEST_API_KEY. The dedicated auth suites set their own value (e.g.
20+
* "secret-test-key") in a beforeEach, and for those this middleware does
21+
* nothing — so assertions like "401 when a key is required but none is
22+
* provided" keep testing exactly what they did before. An explicit
23+
* x-api-key header on the request is likewise never overwritten, so
24+
* wrong-key cases still reach the gate unchanged.
25+
*/
26+
export function autoAuth(req: Request, _res: Response, next: NextFunction): void {
27+
if (
28+
process.env.API_KEY === TEST_API_KEY &&
29+
req.headers["x-api-key"] === undefined
30+
) {
31+
req.headers["x-api-key"] = TEST_API_KEY;
32+
}
33+
next();
34+
}

__tests__/jobs.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { jest } from "@jest/globals";
22
import request from "supertest";
33
import express from "express";
4+
import { autoAuth, TEST_API_KEY } from "./helpers/api-key-helper.js";
45
import type { NextFunction, Request, Response } from "express";
56

67
const VALID_CONTRACT =
@@ -21,6 +22,7 @@ const { default: router } = await import("../src/routes/jobs.js");
2122
function buildApp() {
2223
const app = express();
2324
app.use(express.json());
25+
app.use(autoAuth);
2426
app.use("/api/jobs", router);
2527
// Add error interceptor for test coverage
2628
app.use((err: unknown, _req: Request, res: Response, _next: NextFunction) => {
@@ -34,7 +36,6 @@ describe("GET /api/jobs/:contractId – error interceptor", () => {
3436
beforeEach(() => {
3537
mockGetAccount.mockReset();
3638
mockSimulateTransaction.mockReset();
37-
delete process.env.API_KEY;
3839
mockGetAccount.mockResolvedValue({
3940
accountId: () =>
4041
"GAODBHVR63Z56MVQRBEJSYM2H5423LJ4WAPUUBOFG4JYY72S6ROKVZRX",

__tests__/partial-release.test.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { jest } from "@jest/globals";
22
import request from "supertest";
33
import express from "express";
4+
import { autoAuth, TEST_API_KEY } from "./helpers/api-key-helper.js";
45

56
const VALID_CONTRACT = "CDD5WKK3WT3QVKXMXTJNDIXE4T73FK6GGXDSD6UTJAH6YYZU52SQ4MUH";
67
const VALID_ADDRESS = "GAODBHVR63Z56MVQRBEJSYM2H5423LJ4WAPUUBOFG4JYY72S6ROKVZRX";
@@ -23,6 +24,7 @@ const { resetPartialReleaseRateLimitBuckets } = await import(
2324
function buildApp() {
2425
const app = express();
2526
app.use(express.json());
27+
app.use(autoAuth);
2628
app.use("/api/jobs", router);
2729
return app;
2830
}
@@ -411,13 +413,13 @@ describe("POST /api/jobs/:contractId/milestones/:index/partial-release", () => {
411413
expect(res.body.success).toBe(true);
412414
});
413415

414-
it("returns 200 (no gate) when API_KEY is not set", async () => {
416+
it("returns 401 (fails closed) when API_KEY is not set", async () => {
415417
delete process.env.API_KEY;
416418
const res = await request(buildApp())
417419
.post(ENDPOINT)
418420
.send(VALID_BODY)
419-
.expect(200);
420-
expect(res.body.success).toBe(true);
421+
.expect(401);
422+
expect(res.body).toEqual({ success: false, error: "Unauthorized" });
421423
});
422424
});
423425

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import { jest } from "@jest/globals";
2+
3+
/**
4+
* Covers the production startup assertion in src/index.ts.
5+
*
6+
* This imports the real entry point rather than re-implementing the check,
7+
* so deleting the assertion — or moving it below a side effect — fails here.
8+
* ES module imports are evaluated before the module body, so the throw
9+
* happens after dotenv.config() but before runMigrations(), startPoller()
10+
* and app.listen(); nothing binds a port and no database is touched.
11+
*/
12+
describe("production startup assertion (src/index.ts)", () => {
13+
const ORIGINAL_ENV = { ...process.env };
14+
15+
beforeEach(() => {
16+
jest.resetModules();
17+
});
18+
19+
afterEach(() => {
20+
process.env = { ...ORIGINAL_ENV };
21+
});
22+
23+
it("throws and names every missing variable when NODE_ENV=production", async () => {
24+
process.env.NODE_ENV = "production";
25+
delete process.env.API_KEY;
26+
delete process.env.ADMIN_API_KEY;
27+
delete process.env.ALLOWED_ORIGINS;
28+
delete process.env.CONTRACT_ID;
29+
30+
// dotenv.config() may repopulate some of these from a local .env, so
31+
// assert on the ones the repo's .env.example does not provide.
32+
await expect(import("../src/index.js")).rejects.toThrow(
33+
/Missing required production environment variables:.*API_KEY/,
34+
);
35+
});
36+
37+
it("names ADMIN_API_KEY specifically when only that one is absent", async () => {
38+
process.env.NODE_ENV = "production";
39+
process.env.API_KEY = "set";
40+
process.env.ALLOWED_ORIGINS = "https://example.test";
41+
process.env.CONTRACT_ID = "CDD5WKK3WT3QVKXMXTJNDIXE4T73FK6GGXDSD6UTJAH6YYZU52SQ4MUH";
42+
delete process.env.ADMIN_API_KEY;
43+
44+
await expect(import("../src/index.js")).rejects.toThrow(
45+
"Missing required production environment variables: ADMIN_API_KEY",
46+
);
47+
});
48+
});

__tests__/route-trace-logging.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { jest } from "@jest/globals";
22
import request from "supertest";
33
import express from "express";
4+
import { autoAuth, TEST_API_KEY } from "./helpers/api-key-helper.js";
45

56
const VALID_CONTRACT = "CDD5WKK3WT3QVKXMXTJNDIXE4T73FK6GGXDSD6UTJAH6YYZU52SQ4MUH";
67
const VALID_ADDRESS = "GAODBHVR63Z56MVQRBEJSYM2H5423LJ4WAPUUBOFG4JYY72S6ROKVZRX";
@@ -44,6 +45,7 @@ const {
4445
function buildApp() {
4546
const app = express();
4647
app.use(express.json());
48+
app.use(autoAuth);
4749
app.use("/api/jobs", router);
4850
return app;
4951
}

__tests__/time-remaining-validation.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { jest } from "@jest/globals";
22
import request from "supertest";
33
import express from "express";
4+
import { autoAuth, TEST_API_KEY } from "./helpers/api-key-helper.js";
45

56
const VALID_CONTRACT = "CDD5WKK3WT3QVKXMXTJNDIXE4T73FK6GGXDSD6UTJAH6YYZU52SQ4MUH";
67

@@ -22,6 +23,7 @@ const { resetTimeRemainingRateLimitBuckets } = await import(
2223
function buildApp() {
2324
const app = express();
2425
app.use(express.json());
26+
app.use(autoAuth);
2527
app.use("/api/jobs", router);
2628
return app;
2729
}
@@ -56,6 +58,17 @@ describe("GET /api/jobs/:contractId/milestones/:index/time-remaining", () => {
5658
expect(mockSimulateTransaction).toHaveBeenCalled();
5759
});
5860

61+
it("returns 401 (fails closed) when API_KEY is not set", async () => {
62+
delete process.env.API_KEY;
63+
64+
const res = await request(buildApp())
65+
.get(`/api/jobs/${VALID_CONTRACT}/milestones/0/time-remaining`)
66+
.expect(401);
67+
68+
expect(res.body).toEqual({ success: false, error: "Unauthorized" });
69+
expect(mockSimulateTransaction).not.toHaveBeenCalled();
70+
});
71+
5972
// 2. Invalid contractId
6073
it("returns 400 for an invalid contractId", async () => {
6174
const res = await request(buildApp())

0 commit comments

Comments
 (0)