Skip to content

Commit 5e5de36

Browse files
committed
fix(api-server): address code audit findings
- fix: make ApiKeyAuth constructor private; add static resetInstance() for tests - fix: enforce MAX_REQUEST_SIZE via content-length header check (413 response) - fix: add PAYLOAD_TOO_LARGE to ErrorCode enum and getErrorCodeForStatus map - fix: add --watch flag to api:server:dev script for hot-reload - fix: add ALLOWED_ORIGINS to docker-compose.yml environment section - fix: expand api-validate.yml trigger paths to include api-server/** and package.json - chore: remove implementation-plan-fetch-ready.md from repo root
1 parent 61ad02e commit 5e5de36

File tree

10 files changed

+62
-444
lines changed

10 files changed

+62
-444
lines changed

.github/workflows/api-validate.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,18 @@ name: API Validate
33
on:
44
workflow_dispatch:
55
push:
6+
branches:
7+
- main
8+
- "feat/**"
69
paths:
710
- ".github/workflows/api-validate.yml"
11+
- "api-server/**"
12+
- "package.json"
813
pull_request:
914
paths:
1015
- ".github/workflows/api-validate.yml"
16+
- "api-server/**"
17+
- "package.json"
1118

1219
jobs:
1320
api-validate:

api-server/auth.test.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@ describe("ApiKeyAuth", () => {
1616
let auth: ApiKeyAuth;
1717

1818
beforeEach(() => {
19-
// Clear any existing instance and create fresh one for each test
20-
ApiKeyAuth["instance"] = undefined;
21-
auth = new ApiKeyAuth();
19+
// Reset the singleton and obtain a fresh instance for each test
20+
ApiKeyAuth.resetInstance();
21+
auth = ApiKeyAuth.getInstance();
2222
// Ensure no keys are loaded from environment for consistent testing
2323
auth.clearKeys();
2424
});
@@ -243,7 +243,8 @@ describe("ApiKeyAuth", () => {
243243

244244
describe("Hash collision resistance", () => {
245245
it("should produce different hashes for different keys", () => {
246-
const auth = new ApiKeyAuth();
246+
ApiKeyAuth.resetInstance();
247+
const auth = ApiKeyAuth.getInstance();
247248
const keys = [
248249
"test-key-aaaa-1234567890",
249250
"test-key-bbbb-1234567890",
@@ -272,7 +273,8 @@ describe("ApiKeyAuth", () => {
272273
});
273274

274275
it("should not authenticate with a key that has the same hash length but different content", () => {
275-
const auth = new ApiKeyAuth();
276+
ApiKeyAuth.resetInstance();
277+
const auth = ApiKeyAuth.getInstance();
276278
auth.addKey("real", "real-api-key-1234567890ab", {
277279
name: "real",
278280
active: true,

api-server/auth.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,10 @@ export interface AuthResult {
5151
* Keys are loaded from environment variables in format: API_KEY_<name>
5252
*/
5353
export class ApiKeyAuth {
54-
private static instance: ApiKeyAuth;
54+
private static instance: ApiKeyAuth | undefined;
5555
private apiKeys: Map<string, ApiKeyRecord> = new Map();
5656

57-
public constructor() {
57+
private constructor() {
5858
this.loadKeysFromEnv();
5959
}
6060

@@ -68,6 +68,14 @@ export class ApiKeyAuth {
6868
return ApiKeyAuth.instance;
6969
}
7070

71+
/**
72+
* Reset the singleton instance and clear all keys.
73+
* Intended for use in tests only.
74+
*/
75+
static resetInstance(): void {
76+
ApiKeyAuth.instance = undefined;
77+
}
78+
7179
/**
7280
* Load API keys from environment variables
7381
* Format: API_KEY_<name> = <key value>

api-server/github-actions-secret-handling.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,8 @@ describe("GitHub Actions Secret Handling", () => {
6262
describe("API key auth compatibility", () => {
6363
it("accepts GitHub Actions bearer token style keys", () => {
6464
// Reset singleton for isolated test behavior.
65-
ApiKeyAuth["instance"] = undefined;
66-
const auth = new ApiKeyAuth();
65+
ApiKeyAuth.resetInstance();
66+
const auth = ApiKeyAuth.getInstance();
6767

6868
const token = "gha_" + "a".repeat(32);
6969
auth.addKey("GITHUB_ACTIONS", token, {

api-server/module-extraction.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,8 @@ describe("Module Extraction - extractKeyFromHeader (auth module)", () => {
129129
let auth: ApiKeyAuth;
130130

131131
beforeEach(() => {
132-
ApiKeyAuth["instance"] = undefined;
133-
auth = new ApiKeyAuth();
132+
ApiKeyAuth.resetInstance();
133+
auth = ApiKeyAuth.getInstance();
134134
});
135135

136136
const extractKeyFromHeader = (header: string): string | null => {

api-server/request-handler.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
createErrorResponse,
1212
type ErrorResponse,
1313
} from "./response-schemas";
14-
import { isPublicEndpoint } from "./validation";
14+
import { isPublicEndpoint, MAX_REQUEST_SIZE } from "./validation";
1515
import { routeRequest } from "./router";
1616

1717
/**
@@ -29,6 +29,29 @@ export async function handleRequest(req: Request): Promise<Response> {
2929
const url = new URL(req.url);
3030
const path = url.pathname;
3131

32+
// Reject oversized requests early using the Content-Length header.
33+
// Only reject when the header is present; chunked requests (no header) pass through.
34+
const contentLength = req.headers.get("content-length");
35+
if (
36+
contentLength !== null &&
37+
parseInt(contentLength, 10) > MAX_REQUEST_SIZE
38+
) {
39+
const error: ErrorResponse = createErrorResponse(
40+
ErrorCode.PAYLOAD_TOO_LARGE,
41+
"Request body exceeds 1MB limit",
42+
413,
43+
requestId
44+
);
45+
return new Response(JSON.stringify(error, null, 2), {
46+
status: 413,
47+
headers: {
48+
"Content-Type": "application/json",
49+
...getCorsHeaders(requestOrigin),
50+
"X-Request-ID": requestId,
51+
},
52+
});
53+
}
54+
3255
// Check if endpoint is public or CORS preflight (OPTIONS)
3356
// CORS preflight requests must skip auth since browsers don't send credentials
3457
const isPublic = isPublicEndpoint(path) || req.method === "OPTIONS";

api-server/response-schemas.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ export enum ErrorCode {
3838
// Rate limiting (4xx)
3939
RATE_LIMIT_EXCEEDED = "RATE_LIMIT_EXCEEDED",
4040

41+
// Payload errors (4xx)
42+
PAYLOAD_TOO_LARGE = "PAYLOAD_TOO_LARGE",
43+
4144
// Server errors (5xx)
4245
INTERNAL_ERROR = "INTERNAL_ERROR",
4346
SERVICE_UNAVAILABLE = "SERVICE_UNAVAILABLE",
@@ -273,6 +276,7 @@ export function getErrorCodeForStatus(status: number): ErrorCode {
273276
403: ErrorCode.FORBIDDEN,
274277
404: ErrorCode.NOT_FOUND,
275278
409: ErrorCode.CONFLICT,
279+
413: ErrorCode.PAYLOAD_TOO_LARGE,
276280
429: ErrorCode.RATE_LIMIT_EXCEEDED,
277281
500: ErrorCode.INTERNAL_ERROR,
278282
503: ErrorCode.SERVICE_UNAVAILABLE,

docker-compose.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,11 @@ services:
7575
# Example: API_KEY_DEPLOYMENT=your-secret-key-min-16-chars
7676
API_KEY_GITHUB_ACTIONS: ${API_KEY_GITHUB_ACTIONS}
7777

78+
# CORS Origin Allowlist (optional, comma-separated)
79+
# Leave unset to allow all origins; set to restrict cross-origin access
80+
# Example: ALLOWED_ORIGINS=https://your-app.com,https://admin.your-app.com
81+
ALLOWED_ORIGINS: ${ALLOWED_ORIGINS:-}
82+
7883
# Volume mounts for persistent data
7984
volumes:
8085
# Mount job persistence directory

0 commit comments

Comments
 (0)