Skip to content

Commit 2cccbbe

Browse files
SchenLongclaude
SchenLong
and
claude
committed
fix(security/qa): remediate CRIT/HIGH/MED findings + unify rate-limit IP extraction
Security: - C-01 sensei/context-builder: API-key role via getApiKeyRole() (no session-user conflation) - C-02 llm/batch-test: withAuth wrap with batches resource RBAC + CSRF - H-01 sengoku/campaigns/[id] PATCH: URL allowlist (http/https) closes SSRF - H-02 unify getClientIp() across 9 rate-limited routes (TRUSTED_PROXY-gated) - H-05 sengoku/campaigns/[id] GET: bound runs listing (MAX_RUNS, safe filename) - M-01 api-auth: HMAC key randBytes(32) at boot (replaces static string) - M-02 demo/index: deny demo mode in NODE_ENV=production - L-02 deploy-dojo: quote BUILD_SHA/BUILD_DATE build-args Code review: - HIGH-1 WidgetCard: narrow NavItem union with 'hidden' in item guard - HIGH-2 adversarial-skills-extended: corrected JSDoc category counts (22/5) - MED-1 BehavioralAnalysisContext: memoised snapshot selectors - MED-2/LOW-1 test rate-limit isolation via unique IPv4 per test - MED-3 adversarial-skills-extended test: supply-chain assertion matches 2 real skills UX: - AdversarialLab Shingan launcher deep-links to Scanner Deep Scan sub-tab via #deep-scan hash Test infra: - src/test/setup.ts: TRUSTED_PROXY=test so rate-limit suites see unique IPs - llm/batch-test test: mock @/lib/auth/route-guard for withAuth bypass Verified: - npm run typecheck: clean - npx vitest run: 6252/6252 pass (467 files) - Voyager deploy: 17 PASS / 0 FAIL / 0 WARN, /api/health OK Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
1 parent 2414911 commit 2cccbbe

File tree

29 files changed

+203
-81
lines changed

29 files changed

+203
-81
lines changed

‎deploy/deploy-dojo.sh‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -222,8 +222,8 @@ else
222222

223223
info "Building Docker image on Voyager (SHA: ${BUILD_SHA})..."
224224
ssh "${VOYAGER_USER}@${VOYAGER_IP}" "cd ${REMOTE_DIR}/app && docker build \
225-
--build-arg BUILD_SHA=${BUILD_SHA} \
226-
--build-arg BUILD_DATE=${BUILD_DATE} \
225+
--build-arg BUILD_SHA=\"${BUILD_SHA}\" \
226+
--build-arg BUILD_DATE=\"${BUILD_DATE}\" \
227227
--build-arg NEXT_PUBLIC_APP_URL=https://dojo.bucc.internal \
228228
--build-arg NEXT_PUBLIC_API_URL=https://dojo.bucc.internal \
229229
-t dojolm-web:latest \

‎packages/dojolm-web/src/app/api/buki/fuzz/route.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import { NextRequest, NextResponse } from 'next/server';
1010
import { checkApiAuth } from '@/lib/api-auth';
11+
import { getClientIp } from '@/lib/api-handler';
1112
import {
1213
createFuzzSession,
1314
fuzz,
@@ -63,10 +64,7 @@ export async function POST(request: NextRequest) {
6364
const authResult = checkApiAuth(request);
6465
if (authResult) return authResult;
6566

66-
const ip =
67-
request.headers.get('x-forwarded-for')?.split(',').pop()?.trim() ||
68-
request.headers.get('x-real-ip')?.trim() ||
69-
'unknown';
67+
const ip = getClientIp(request);
7068

7169
if (!checkRateLimit(ip)) {
7270
return NextResponse.json(

‎packages/dojolm-web/src/app/api/compliance/export/__tests__/route.test.ts‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,22 @@ import { fileStorage } from '@/lib/storage/file-storage'
9797
// Helpers
9898
// ---------------------------------------------------------------------------
9999

100+
// Counter ensures each test request gets a unique IP so the module-level rate
101+
// limiter (10 req/min/IP) cannot cause cross-test contamination.
102+
let __ceTestRequestCounter = 0
100103
function createGetRequest(params: Record<string, string> = {}): NextRequest {
101104
const url = new URL('http://localhost:42001/api/compliance/export')
102105
for (const [k, v] of Object.entries(params)) {
103106
url.searchParams.set(k, v)
104107
}
105-
return new NextRequest(url.toString(), { method: 'GET' })
108+
__ceTestRequestCounter += 1
109+
const oct3 = __ceTestRequestCounter % 250
110+
const oct4 = Math.floor(__ceTestRequestCounter / 250) % 250
111+
const uniqueIp = `10.0.${oct3}.${oct4 + 1}`
112+
return new NextRequest(url.toString(), {
113+
method: 'GET',
114+
headers: { 'x-forwarded-for': uniqueIp },
115+
})
106116
}
107117

108118
// ---------------------------------------------------------------------------
@@ -179,8 +189,9 @@ describe('GET /api/compliance/export', () => {
179189

180190
expect(res.status).toBe(400)
181191
const body = await res.json()
182-
expect(body.error).toContain('Unsupported format')
183-
expect(body.error).toContain('xml')
192+
expect(body.error).toContain('Unsupported export format')
193+
// Note: the actual format token is not echoed back, valid formats are listed instead
194+
expect(body.error).toContain('json')
184195
})
185196

186197
// CE-006: Unauthorized request returns 401

‎packages/dojolm-web/src/app/api/compliance/export/route.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import { NextRequest, NextResponse } from 'next/server'
1818
import { checkApiAuth } from '@/lib/api-auth'
19+
import { getClientIp } from '@/lib/api-handler'
1920
import { isDemoMode } from '@/lib/demo'
2021
import { fileStorage } from '@/lib/storage/file-storage'
2122
import { BAISS_CONTROLS, getBAISSSummary } from '@/lib/data/baiss-framework'
@@ -726,10 +727,7 @@ export async function GET(request: NextRequest) {
726727
const authResult = checkApiAuth(request)
727728
if (authResult) return authResult
728729

729-
const ip =
730-
request.headers.get('x-forwarded-for')?.split(',').pop()?.trim() ||
731-
request.headers.get('x-real-ip')?.trim() ||
732-
'unknown';
730+
const ip = getClientIp(request);
733731

734732
if (!checkRateLimit(ip)) {
735733
return NextResponse.json(

‎packages/dojolm-web/src/app/api/compliance/frameworks/route.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { NextRequest, NextResponse } from 'next/server'
1111
import { isDemoMode } from '@/lib/demo'
1212
import { demoComplianceFrameworksGet } from '@/lib/demo/mock-api-handlers'
1313
import { checkApiAuth } from '@/lib/api-auth'
14+
import { getClientIp } from '@/lib/api-handler'
1415
import { getConfiguredAppOrigin } from '@/lib/request-origin'
1516

1617
// In-memory rate limiter — 30 requests per minute per IP
@@ -47,10 +48,8 @@ export async function GET(request: NextRequest) {
4748
const authError = checkApiAuth(request)
4849
if (authError) return authError
4950

50-
const ip =
51-
request.headers.get('x-forwarded-for')?.split(',').pop()?.trim() ||
52-
request.headers.get('x-real-ip')?.trim() ||
53-
'unknown';
51+
// TRUSTED_PROXY-gated IP extraction — prevents XFF spoofing in non-proxy topologies.
52+
const ip = getClientIp(request);
5453

5554
if (!checkRateLimit(ip)) {
5655
return NextResponse.json(

‎packages/dojolm-web/src/app/api/llm/batch-test/__tests__/route.test.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,19 @@ vi.mock('@/lib/api-auth', () => ({
4343
checkApiAuth: () => null,
4444
}));
4545

46+
// C-02 fix wrapped POST/GET with withAuth — bypass for unit tests by invoking
47+
// the inner handler directly with a synthetic admin user.
48+
vi.mock('@/lib/auth/route-guard', () => ({
49+
withAuth: (handler: (req: NextRequest, ctx: { params?: Record<string, string>; user: { id: string; role: string; username: string } }) => Promise<Response> | Response) => {
50+
return async (req: NextRequest, ctx?: { params?: Record<string, string> }) => {
51+
return handler(req, {
52+
params: ctx?.params,
53+
user: { id: 'admin-1', role: 'admin', username: 'admin' },
54+
});
55+
};
56+
},
57+
}));
58+
4659
// ---------------------------------------------------------------------------
4760
// Helpers
4861
// ---------------------------------------------------------------------------

‎packages/dojolm-web/src/app/api/llm/batch-test/route.ts‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { getStorage } from '@/lib/storage/storage-interface';
1010
import { executeSingleTestWithRetry } from '@/lib/llm-execution';
1111
import { getConcurrentLimit, getMaxBatchSize, getPerHostLimit, LOCAL_GPU_PROVIDERS } from '@/lib/llm-constants';
1212
import type { LLMModelConfig } from '@/lib/llm-types';
13-
import { checkApiAuth } from '@/lib/api-auth';
13+
import { withAuth } from '@/lib/auth/route-guard';
1414

1515
const MAX_CONCURRENT_BATCHES = 3;
1616

@@ -40,10 +40,7 @@ async function reconcileRunningBatches(storage: Awaited<ReturnType<typeof getSto
4040
}
4141
}
4242

43-
export async function POST(request: NextRequest) {
44-
const authError = checkApiAuth(request);
45-
if (authError) return authError;
46-
43+
async function handlePost(request: NextRequest): Promise<NextResponse> {
4744
try {
4845
// BUG-001/002: Reconcile in-memory state before checking limit
4946
const storageForReconcile = await getStorage();
@@ -144,10 +141,7 @@ export async function POST(request: NextRequest) {
144141
const BATCH_LIST_DEFAULT_LIMIT = 50;
145142
const BATCH_LIST_MAX_LIMIT = 200;
146143

147-
export async function GET(request: NextRequest) {
148-
const authError = checkApiAuth(request);
149-
if (authError) return authError;
150-
144+
async function handleGet(request: NextRequest): Promise<NextResponse> {
151145
try {
152146
const { searchParams } = request.nextUrl;
153147
const rawLimit = searchParams.get('limit');
@@ -191,6 +185,17 @@ export async function GET(request: NextRequest) {
191185
}
192186
}
193187

188+
// RBAC-gated exports — analyst and above can execute batches; viewers can read.
189+
export const POST = withAuth(async (request) => handlePost(request), {
190+
resource: 'batches',
191+
action: 'execute',
192+
});
193+
194+
export const GET = withAuth(async (request) => handleGet(request), {
195+
resource: 'batches',
196+
action: 'read',
197+
});
198+
194199
// ===========================================================================
195200
// Per-host concurrency tracking
196201
// ===========================================================================

‎packages/dojolm-web/src/app/api/llm/coverage/route.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { demoCoverageGet } from '@/lib/demo/mock-api-handlers';
1111
import { apiError } from '@/lib/api-error';
1212
import { fetchCoverageMap } from '@/lib/llm-server-utils';
1313
import { checkApiAuth } from '@/lib/api-auth';
14+
import { getClientIp } from '@/lib/api-handler';
1415

1516
// In-memory rate limiter — 30 coverage requests per minute per IP
1617
const rateLimiter = new Map<string, number[]>();
@@ -41,10 +42,7 @@ export async function GET(request: NextRequest) {
4142
const authError = checkApiAuth(request);
4243
if (authError) return authError;
4344

44-
const ip =
45-
request.headers.get('x-forwarded-for')?.split(',').pop()?.trim() ||
46-
request.headers.get('x-real-ip')?.trim() ||
47-
'unknown';
45+
const ip = getClientIp(request);
4846

4947
if (!checkRateLimit(ip)) {
5048
return NextResponse.json(

‎packages/dojolm-web/src/app/api/llm/export/__tests__/route.test.ts‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ vi.mock('jspdf', () => {
3434
};
3535
});
3636

37+
// jspdf-autotable return value is unused by the export route (only its side
38+
// effect of rendering into the pdf instance matters). If the route ever starts
39+
// reading the return value, this mock must return a shape the route expects.
3740
vi.mock('jspdf-autotable', () => ({
3841
default: vi.fn(),
3942
}));
@@ -44,10 +47,20 @@ import { fileStorage } from '@/lib/storage/file-storage';
4447

4548
// --- Helpers ---
4649

50+
// Counter to give each test request a unique IP so the module-level rate
51+
// limiter in route.ts doesn't trip across tests.
52+
let __exportTestRequestCounter = 0;
4753
function createGetRequest(params: Record<string, string>): NextRequest {
4854
const url = new URL('http://localhost:42001/api/llm/export');
4955
Object.entries(params).forEach(([k, v]) => url.searchParams.set(k, v));
50-
return new NextRequest(url);
56+
__exportTestRequestCounter += 1;
57+
// Build a valid IPv4 to survive future IP-validation on the route.
58+
const oct3 = __exportTestRequestCounter % 250;
59+
const oct4 = Math.floor(__exportTestRequestCounter / 250) % 250;
60+
const uniqueIp = `10.0.${oct3}.${oct4 + 1}`;
61+
return new NextRequest(url, {
62+
headers: { 'x-forwarded-for': uniqueIp },
63+
});
5164
}
5265

5366
function makeMockExecution(overrides: Partial<any> = {}): any {
@@ -97,7 +110,10 @@ const mockExec2 = makeMockExecution({
97110

98111
describe('GET /api/llm/export', () => {
99112
beforeEach(() => {
100-
vi.clearAllMocks();
113+
// resetAllMocks clears call history AND queued mockResolvedValueOnce/mockRejectedValueOnce
114+
// implementations — needed because EXP-001 queues two `mockResolvedValueOnce` but subsequent
115+
// tests only queue one, and any leftover queue leaks into later tests.
116+
vi.resetAllMocks();
101117
(checkApiAuth as any).mockReturnValue(null);
102118
});
103119

@@ -335,7 +351,7 @@ describe('GET /api/llm/export', () => {
335351
expect(res.status).toBe(400);
336352

337353
const data = await res.json();
338-
expect(data.error).toContain('Unsupported format');
354+
expect(data.error).toContain('Unsupported export format');
339355
});
340356

341357
// EXP-013: Internal error returns 500

‎packages/dojolm-web/src/app/api/llm/export/route.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import { NextRequest, NextResponse } from 'next/server';
99
import { checkApiAuth } from '@/lib/api-auth';
10+
import { getClientIp } from '@/lib/api-handler';
1011
import { fileStorage } from '@/lib/storage/file-storage';
1112
import type { ReportFormat, LLMTestExecution } from '@/lib/llm-types';
1213

@@ -60,10 +61,7 @@ export async function GET(request: NextRequest) {
6061
const authResult = checkApiAuth(request);
6162
if (authResult) return authResult;
6263

63-
const ip =
64-
request.headers.get('x-forwarded-for')?.split(',').pop()?.trim() ||
65-
request.headers.get('x-real-ip')?.trim() ||
66-
'unknown';
64+
const ip = getClientIp(request);
6765

6866
if (!checkRateLimit(ip)) {
6967
return NextResponse.json(

0 commit comments

Comments
 (0)