Skip to content

Commit e08e58a

Browse files
PaulJPhilpPaulJPhilp
committed
fix(mcp-server): harden auth surfaces and endpoint metadata
1 parent 0589270 commit e08e58a

11 files changed

Lines changed: 230 additions & 156 deletions

File tree

‎packages/mcp-server/app/api/admin/circuit-breaker/route.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import { NextResponse, type NextRequest } from "next/server";
1212
import { randomUUID } from "crypto";
1313
import { Effect } from "effect";
14+
import { constantTimeEquals } from "../../../../src/auth/secureCompare";
1415
import { CircuitBreakerService } from "../../../../src/services/circuit-breaker";
1516
import { runWithRuntime } from "../../../../src/server/init";
1617

@@ -28,7 +29,11 @@ function verifyAdminAccess(request: NextRequest): boolean {
2829
return false;
2930
}
3031

31-
return adminKey === expectedKey;
32+
if (!adminKey) {
33+
return false;
34+
}
35+
36+
return constantTimeEquals(adminKey, expectedKey);
3237
}
3338

3439
/**

‎packages/mcp-server/app/api/env-check/route.ts‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,19 @@
44
* Returns non-sensitive env presence only. Do not expose lengths, prefixes, or key names.
55
*/
66

7-
import { NextResponse } from "next/server";
7+
import { Effect } from "effect";
8+
import type { NextRequest } from "next/server";
9+
import { createSimpleHandler } from "../../../src/server/routeHandler";
810

9-
export async function GET() {
10-
const envVars = {
11-
hasDatabaseUrl: !!process.env.DATABASE_URL,
12-
nodeEnv: process.env.NODE_ENV || "not set",
13-
customNodeEnv: process.env.CUSTOM_NODE_ENV || "not set",
14-
hasApiKey: !!process.env.PATTERN_API_KEY,
15-
};
11+
const handleEnvCheck = (_request: NextRequest) =>
12+
Effect.succeed({
13+
hasDatabaseUrl: !!process.env.DATABASE_URL,
14+
nodeEnv: process.env.NODE_ENV || "not set",
15+
customNodeEnv: process.env.CUSTOM_NODE_ENV || "not set",
16+
hasApiKey: !!process.env.PATTERN_API_KEY,
17+
});
1618

17-
return NextResponse.json(envVars);
18-
}
19+
export const GET = createSimpleHandler(handleEnvCheck, {
20+
requireAuth: false,
21+
requireAdmin: true,
22+
});

‎packages/mcp-server/app/api/test/route.ts‎

Lines changed: 58 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,63 +1,62 @@
11
/**
2-
* Simple test endpoint to verify database connection
2+
* Admin-only database connectivity check.
33
*/
44

5-
import { createDatabase, effectPatterns } from "@effect-patterns/toolkit"
6-
import { Effect } from "effect"
7-
import { NextResponse } from "next/server"
8-
9-
export async function GET() {
10-
try {
11-
const dbUrl = process.env.DATABASE_URL
12-
if (!dbUrl) {
13-
return NextResponse.json({
14-
success: false,
15-
error: "DATABASE_URL not set",
16-
envKeys: Object.keys(process.env).filter(k => k.includes("DATABASE") || k.includes("POSTGRES"))
17-
}, { status: 500 })
18-
}
19-
20-
const result = await Effect.runPromise(
21-
Effect.gen(function* () {
22-
const { db, close } = createDatabase(dbUrl)
23-
24-
const patterns = yield* Effect.tryPromise(async () => {
25-
return await db.select({
26-
id: effectPatterns.id,
27-
title: effectPatterns.title,
28-
skillLevel: effectPatterns.skillLevel,
29-
category: effectPatterns.category
30-
}).from(effectPatterns).limit(5)
31-
})
32-
33-
yield* Effect.promise(() => close())
34-
return patterns
35-
})
36-
)
37-
38-
return NextResponse.json({
39-
success: true,
40-
count: result.length,
41-
patterns: result.map((p: any) => ({
42-
id: p.id,
43-
title: p.title,
44-
skillLevel: p.skillLevel,
45-
category: p.category
46-
}))
47-
})
48-
} catch (error) {
49-
console.error("Database test error:", error)
50-
const errorMessage = error instanceof Error ? error.message : String(error)
51-
const errorStack = error instanceof Error ? error.stack : undefined
52-
return NextResponse.json(
53-
{
54-
success: false,
55-
error: "Database connection failed",
56-
details: errorMessage,
57-
stack: errorStack?.split('\n').slice(0, 5)
58-
},
59-
{ status: 500 }
60-
)
61-
}
62-
}
5+
import { createDatabase, effectPatterns } from "@effect-patterns/toolkit";
6+
import { Effect } from "effect";
7+
import { type NextRequest, NextResponse } from "next/server";
8+
import { validateAdminKey } from "../../../src/auth/adminAuth";
9+
import { errorHandler } from "../../../src/server/errorHandler";
10+
import { runWithRuntime } from "../../../src/server/init";
11+
12+
const handleDatabaseTest = (request: NextRequest) =>
13+
Effect.gen(function* () {
14+
yield* validateAdminKey(request);
15+
16+
const dbUrl = process.env.DATABASE_URL;
17+
if (!dbUrl) {
18+
return yield* Effect.fail(new Error("DATABASE_URL not set"));
19+
}
20+
21+
const { db, close } = createDatabase(dbUrl);
22+
const rows = yield* Effect.tryPromise({
23+
try: () =>
24+
db
25+
.select({
26+
id: effectPatterns.id,
27+
title: effectPatterns.title,
28+
skillLevel: effectPatterns.skillLevel,
29+
category: effectPatterns.category,
30+
})
31+
.from(effectPatterns)
32+
.limit(5),
33+
catch: (error) => new Error(`Database connection failed: ${String(error)}`),
34+
}).pipe(
35+
Effect.ensuring(
36+
Effect.tryPromise({
37+
try: () => close(),
38+
catch: (error) => new Error(`Failed to close database connection: ${String(error)}`),
39+
}).pipe(Effect.ignore),
40+
),
41+
);
6342

43+
return {
44+
success: true,
45+
count: rows.length,
46+
patterns: rows,
47+
};
48+
});
49+
50+
export async function GET(request: NextRequest) {
51+
const result = await runWithRuntime(
52+
handleDatabaseTest(request).pipe(
53+
Effect.catchAll((error) => errorHandler(error)),
54+
),
55+
);
56+
57+
if (result instanceof Response) {
58+
return result;
59+
}
60+
61+
return NextResponse.json(result, { status: 200 });
62+
}

‎packages/mcp-server/src/auth/__tests__/adminAuth.test.ts‎

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -72,24 +72,30 @@ describe("Admin Authentication", () => {
7272
expect(result).toBeUndefined(); // Success
7373
});
7474

75-
it("should allow valid admin key from query parameter", async () => {
76-
const request = createMockRequest({
77-
url: "http://localhost:3000/api/test?admin-key=admin-secret-key",
75+
it("should reject admin key passed via query parameter", async () => {
76+
const request = createMockRequest({
77+
url: "http://localhost:3000/api/test?admin-key=admin-secret-key",
78+
});
79+
80+
const result = await Effect.runPromise(
81+
validateAdminKey(request).pipe(
82+
Effect.provide(MCPConfigService.Default),
83+
Effect.either
84+
)
85+
);
86+
87+
expect(result._tag).toBe("Left");
88+
if (result._tag === "Left") {
89+
const errorObj = result.left as any;
90+
expect(errorObj._tag).toBe("AuthorizationError");
91+
expect(errorObj.message).toContain("required");
92+
}
7893
});
7994

80-
const result = await Effect.runPromise(
81-
validateAdminKey(request).pipe(
82-
Effect.provide(MCPConfigService.Default)
83-
)
84-
);
85-
86-
expect(result).toBeUndefined();
87-
});
88-
89-
it("should prefer header over query parameter", async () => {
90-
const request = createMockRequest({
91-
headers: { "x-admin-key": "admin-secret-key" },
92-
url: "http://localhost:3000/api/test?admin-key=wrong-key",
95+
it("should use header key and ignore query parameter", async () => {
96+
const request = createMockRequest({
97+
headers: { "x-admin-key": "admin-secret-key" },
98+
url: "http://localhost:3000/api/test?admin-key=wrong-key",
9399
});
94100

95101
const result = await Effect.runPromise(
@@ -98,8 +104,8 @@ describe("Admin Authentication", () => {
98104
)
99105
);
100106

101-
expect(result).toBeUndefined(); // Header takes precedence
102-
});
107+
expect(result).toBeUndefined();
108+
});
103109

104110
it("should reject missing admin key in production", async () => {
105111
const request = createMockRequest({});

‎packages/mcp-server/src/auth/__tests__/apiKey.test.ts‎

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/**
22
* API Key Authentication Tests
33
*
4-
* Tests for API key validation from headers and query parameters.
4+
* Tests for API key validation from headers.
55
*
66
* Architecture:
77
* - HTTP API is where ALL authentication happens
@@ -61,16 +61,6 @@ function validateApiKeySimple(
6161
};
6262
}
6363

64-
// Check query parameter
65-
const { searchParams } = new URL(request.url);
66-
const queryKey = searchParams.get("key");
67-
if (queryKey) {
68-
return {
69-
valid: queryKey === configuredApiKey,
70-
error: queryKey !== configuredApiKey ? "Invalid API key" : undefined,
71-
};
72-
}
73-
7464
return {
7565
valid: false,
7666
error: "Missing API key",
@@ -90,15 +80,16 @@ describe("validateApiKey Logic", () => {
9080
expect(result.error).toBeUndefined();
9181
});
9282

93-
it("should pass with valid API key in query parameter", () => {
83+
it("should reject query parameter API key", () => {
9484
const testApiKey = "test-api-key-query";
9585
const request = createMockRequest({
9686
url: `http://localhost:3000/api/test?key=${testApiKey}`,
9787
});
9888

9989
const result = validateApiKeySimple(request, testApiKey);
10090

101-
expect(result.valid).toBe(true);
91+
expect(result.valid).toBe(false);
92+
expect(result.error).toBe("Missing API key");
10293
});
10394

10495
it("should fail with missing API key", () => {
@@ -121,7 +112,7 @@ describe("validateApiKey Logic", () => {
121112
expect(result.error).toBe("Invalid API key");
122113
});
123114

124-
it("should prefer header over query parameter", () => {
115+
it("should use header key and ignore query parameter", () => {
125116
const testApiKey = "header-key";
126117
const request = createMockRequest({
127118
headers: { "x-api-key": testApiKey },
@@ -200,7 +191,7 @@ describe("validateApiKey Logic", () => {
200191
expect(result.valid).toBe(false);
201192
});
202193

203-
it("should decode URL-encoded query parameters", () => {
194+
it("should not accept URL-encoded query API key", () => {
204195
const testApiKey = "test key with spaces";
205196
const encodedKey = encodeURIComponent(testApiKey);
206197
const request = createMockRequest({
@@ -209,6 +200,7 @@ describe("validateApiKey Logic", () => {
209200

210201
const result = validateApiKeySimple(request, testApiKey);
211202

212-
expect(result.valid).toBe(true);
203+
expect(result.valid).toBe(false);
204+
expect(result.error).toBe("Missing API key");
213205
});
214206
});

‎packages/mcp-server/src/auth/adminAuth.ts‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,24 +11,17 @@ import { Effect } from "effect";
1111
import type { NextRequest } from "next/server";
1212
import { AuthorizationError } from "../errors";
1313
import { MCPConfigService } from "../services/config";
14+
import { constantTimeEquals } from "./secureCompare";
1415

1516
/**
1617
* Extract admin API key from request
1718
*
18-
* Checks:
19-
* 1. x-admin-key header
20-
* 2. ?admin-key query parameter (less preferred, less secure)
19+
* Checks `x-admin-key` header only.
2120
*/
2221
function extractAdminKey(request: NextRequest): string | null {
23-
// Check header first (more secure)
2422
const headerKey = request.headers.get("x-admin-key");
2523
if (headerKey) return headerKey;
2624

27-
// Check query parameter (less secure, but supported)
28-
const { searchParams } = new URL(request.url);
29-
const queryKey = searchParams.get("admin-key");
30-
if (queryKey) return queryKey;
31-
3225
return null;
3326
}
3427

@@ -80,7 +73,7 @@ export const validateAdminKey = (
8073
}
8174

8275
// Validate key (never include provided key in error for security)
83-
if (providedKey !== adminKey) {
76+
if (!constantTimeEquals(providedKey, adminKey)) {
8477
return yield* Effect.fail(
8578
new AuthorizationError({
8679
message: "Invalid admin credentials",

‎packages/mcp-server/src/auth/apiKey.ts‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,25 @@
11
/**
22
* API Key Authentication Middleware
33
*
4-
* Validates PATTERN_API_KEY from x-api-key header or ?key query param.
4+
* Validates PATTERN_API_KEY from x-api-key header.
55
* Returns 401 Unauthorized for invalid/missing keys.
66
*/
77

88
import { Effect } from "effect";
99
import type { NextRequest } from "next/server";
1010
import { AuthenticationError } from "../errors";
1111
import { MCPConfigService } from "../services/config";
12+
import { constantTimeEquals } from "./secureCompare";
1213

1314
/**
1415
* Extract API key from request
1516
*
16-
* Checks:
17-
* 1. x-api-key header
18-
* 2. ?key query parameter
17+
* Checks `x-api-key` header only.
1918
*/
2019
function extractApiKey(request: NextRequest): string | null {
21-
// Check header first
2220
const headerKey = request.headers.get("x-api-key");
2321
if (headerKey) return headerKey;
2422

25-
// Check query parameter
26-
const { searchParams } = new URL(request.url);
27-
const queryKey = searchParams.get("key");
28-
if (queryKey) return queryKey;
29-
3023
return null;
3124
}
3225

@@ -71,7 +64,7 @@ export const validateApiKey = (
7164
}
7265

7366
// Key provided - validate it matches configured key
74-
if (providedKey !== apiKey) {
67+
if (!constantTimeEquals(providedKey, apiKey)) {
7568
return yield* Effect.fail(
7669
new AuthenticationError({
7770
message: "Invalid API key"
@@ -96,7 +89,7 @@ export const validateApiKey = (
9689
}
9790

9891
// Validate key
99-
if (providedKey !== apiKey) {
92+
if (!constantTimeEquals(providedKey, apiKey)) {
10093
return yield* Effect.fail(
10194
new AuthenticationError({
10295
message: "Invalid API key"

0 commit comments

Comments
 (0)