Skip to content

Commit 8a1e660

Browse files
sweetmantechclaude
andcommitted
refactor(sandbox): extract validateSandboxReconnectRequest + fix preview build
Two changes bundled: 1) SRP per review feedback (sweetmantech) — extract the auth + sessionId-from-query + session lookup + ownership check pre-flight from `getSandboxReconnectHandler` into its own `validateSandboxReconnectRequest.ts`. Mirrors the `validateCreateSandboxBody` pattern: returns either a 4xx NextResponse describing the first failure, or `{ row, auth }` for the handler to consume. 2) Type fix for `next build` — `connectSandbox(row.sandbox_state as SandboxState)` failed to compile against `Json` (union includes primitives + arrays); cast through `unknown` first. The `hasRuntimeSandboxState` gate above ensures the runtime shape is safe at the call site, so the double cast is justified — comment added explaining why. The vitest pass alone wasn't enough to catch the type error — `next build` runs a separate `tsc` step that the test runner skips. Caught by the Vercel preview build failing on the previous commit. TDD red -> green: - 5 unit tests for the new validator covering auth fail (passes through), missing sessionId (400), session not found (404), ownership mismatch (403), and happy-path return shape ({row, auth}) - Existing handler tests pass unchanged — the module-level mocks for validateAuthContext / selectSessions still intercept the calls now that they're behind the new validator - Suite: 2529 -> 2534, pnpm lint:check clean Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 23043f1 commit 8a1e660

3 files changed

Lines changed: 152 additions & 33 deletions

File tree

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
import { describe, it, expect, vi, beforeEach } from "vitest";
2+
import { NextRequest, NextResponse } from "next/server";
3+
4+
import { validateSandboxReconnectRequest } from "@/lib/sandbox/validateSandboxReconnectRequest";
5+
import { validateAuthContext } from "@/lib/auth/validateAuthContext";
6+
import { selectSessions } from "@/lib/supabase/sessions/selectSessions";
7+
8+
vi.mock("@/lib/networking/getCorsHeaders", () => ({
9+
getCorsHeaders: () => ({ "Access-Control-Allow-Origin": "*" }),
10+
}));
11+
vi.mock("@/lib/auth/validateAuthContext", () => ({
12+
validateAuthContext: vi.fn(),
13+
}));
14+
vi.mock("@/lib/supabase/sessions/selectSessions", () => ({
15+
selectSessions: vi.fn(),
16+
}));
17+
18+
const ACCOUNT_ID = "acc-1";
19+
const baseRow = { id: "sess-1", account_id: ACCOUNT_ID } as never;
20+
21+
function makeReq(query = "?sessionId=sess-1"): NextRequest {
22+
return new NextRequest(`http://localhost/api/sandbox/reconnect${query}`, { method: "GET" });
23+
}
24+
25+
describe("validateSandboxReconnectRequest", () => {
26+
beforeEach(() => {
27+
vi.clearAllMocks();
28+
vi.mocked(validateAuthContext).mockResolvedValue({
29+
accountId: ACCOUNT_ID,
30+
orgId: null,
31+
authToken: "k",
32+
});
33+
});
34+
35+
it("returns the auth response unchanged when auth fails", async () => {
36+
const fail = NextResponse.json({ status: "error", error: "Unauthorized" }, { status: 401 });
37+
vi.mocked(validateAuthContext).mockResolvedValueOnce(fail);
38+
39+
const result = await validateSandboxReconnectRequest(makeReq());
40+
41+
expect(result).toBe(fail);
42+
});
43+
44+
it("returns 400 when sessionId is missing from the query", async () => {
45+
const result = await validateSandboxReconnectRequest(makeReq(""));
46+
47+
expect(result).toBeInstanceOf(NextResponse);
48+
expect((result as NextResponse).status).toBe(400);
49+
});
50+
51+
it("returns 404 when no session exists with the given id", async () => {
52+
vi.mocked(selectSessions).mockResolvedValue([]);
53+
54+
const result = await validateSandboxReconnectRequest(makeReq());
55+
56+
expect(result).toBeInstanceOf(NextResponse);
57+
expect((result as NextResponse).status).toBe(404);
58+
});
59+
60+
it("returns 403 when the session is not owned by the authenticated account", async () => {
61+
vi.mocked(selectSessions).mockResolvedValue([
62+
{ ...baseRow, account_id: "someone-else" } as never,
63+
]);
64+
65+
const result = await validateSandboxReconnectRequest(makeReq());
66+
67+
expect(result).toBeInstanceOf(NextResponse);
68+
expect((result as NextResponse).status).toBe(403);
69+
});
70+
71+
it("returns the validated row + auth on the happy path", async () => {
72+
vi.mocked(selectSessions).mockResolvedValue([baseRow]);
73+
74+
const result = await validateSandboxReconnectRequest(makeReq());
75+
76+
expect(result).not.toBeInstanceOf(NextResponse);
77+
if (result instanceof NextResponse) return;
78+
expect(result.row.id).toBe("sess-1");
79+
expect(result.auth.accountId).toBe(ACCOUNT_ID);
80+
});
81+
});

lib/sandbox/getSandboxReconnectHandler.ts

Lines changed: 11 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
import { NextRequest, NextResponse } from "next/server";
22
import { getCorsHeaders } from "@/lib/networking/getCorsHeaders";
3-
import { validateAuthContext } from "@/lib/auth/validateAuthContext";
43
import { buildLifecycle } from "@/lib/sandbox/buildLifecycle";
54
import { connectSandbox } from "@/lib/sandbox/factory";
65
import { hasRuntimeSandboxState } from "@/lib/sandbox/hasRuntimeSandboxState";
76
import { noSandboxResponse } from "@/lib/sandbox/noSandboxResponse";
8-
import { selectSessions } from "@/lib/supabase/sessions/selectSessions";
7+
import { validateSandboxReconnectRequest } from "@/lib/sandbox/validateSandboxReconnectRequest";
98
import { updateSession } from "@/lib/supabase/sessions/updateSession";
109
import type { SandboxState } from "@/lib/sandbox/factory";
1110

@@ -31,42 +30,21 @@ interface ReconnectBody {
3130
* `GET /api/sandbox/status` agree with the probe.
3231
*/
3332
export async function getSandboxReconnectHandler(request: NextRequest): Promise<NextResponse> {
34-
const auth = await validateAuthContext(request);
35-
if (auth instanceof NextResponse) {
36-
return auth;
37-
}
38-
39-
const sessionId = request.nextUrl.searchParams.get("sessionId");
40-
if (!sessionId) {
41-
return NextResponse.json(
42-
{ status: "error", error: "Missing sessionId" },
43-
{ status: 400, headers: getCorsHeaders() },
44-
);
45-
}
46-
47-
const rows = await selectSessions({ id: sessionId });
48-
const row = rows[0];
49-
50-
if (!row) {
51-
return NextResponse.json(
52-
{ status: "error", error: "Session not found" },
53-
{ status: 404, headers: getCorsHeaders() },
54-
);
55-
}
56-
57-
if (row.account_id !== auth.accountId) {
58-
return NextResponse.json(
59-
{ status: "error", error: "Forbidden" },
60-
{ status: 403, headers: getCorsHeaders() },
61-
);
33+
const validated = await validateSandboxReconnectRequest(request);
34+
if (validated instanceof NextResponse) {
35+
return validated;
6236
}
37+
const { row } = validated;
6338

6439
if (!hasRuntimeSandboxState(row.sandbox_state)) {
6540
return noSandboxResponse(row);
6641
}
6742

6843
try {
69-
const sandbox = await connectSandbox(row.sandbox_state as SandboxState);
44+
// Safe cast: hasRuntimeSandboxState above narrowed sandbox_state to an
45+
// object with a non-empty `sandboxName` — but the Json type is wider, so
46+
// TS needs the unknown bridge to accept the conversion.
47+
const sandbox = await connectSandbox(row.sandbox_state as unknown as SandboxState);
7048
await sandbox.exec("pwd", sandbox.workingDirectory, PROBE_TIMEOUT_MS);
7149

7250
const body: ReconnectBody = {
@@ -78,9 +56,9 @@ export async function getSandboxReconnectHandler(request: NextRequest): Promise<
7856
return NextResponse.json(body, { status: 200, headers: getCorsHeaders() });
7957
} catch (error) {
8058
const message = error instanceof Error ? error.message : String(error);
81-
console.warn(`[getSandboxReconnectHandler] probe failed for ${sessionId}: ${message}`);
59+
console.warn(`[getSandboxReconnectHandler] probe failed for ${row.id}: ${message}`);
8260

83-
await updateSession(sessionId, {
61+
await updateSession(row.id, {
8462
sandbox_state: null,
8563
lifecycle_state: "hibernated",
8664
sandbox_expires_at: null,
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import { NextRequest, NextResponse } from "next/server";
2+
import { getCorsHeaders } from "@/lib/networking/getCorsHeaders";
3+
import { validateAuthContext } from "@/lib/auth/validateAuthContext";
4+
import type { AuthContext } from "@/lib/auth/validateAuthContext";
5+
import { selectSessions } from "@/lib/supabase/sessions/selectSessions";
6+
import type { Tables } from "@/types/database.types";
7+
8+
export interface ValidatedSandboxReconnectRequest {
9+
row: Tables<"sessions">;
10+
auth: AuthContext;
11+
}
12+
13+
/**
14+
* Validates a `GET /api/sandbox/reconnect` request end-to-end:
15+
* 1. Authenticates the caller via Privy Bearer / x-api-key
16+
* 2. Requires a `sessionId` query parameter
17+
* 3. Looks up the session row
18+
* 4. Enforces ownership (the authed account must match `account_id`)
19+
*
20+
* Returns either a 4xx NextResponse describing the first failure, or
21+
* the validated `{ row, auth }` ready for the handler to consume.
22+
*
23+
* @param request - The incoming GET request.
24+
* @returns A NextResponse on validation failure, or the validated row + auth.
25+
*/
26+
export async function validateSandboxReconnectRequest(
27+
request: NextRequest,
28+
): Promise<NextResponse | ValidatedSandboxReconnectRequest> {
29+
const auth = await validateAuthContext(request);
30+
if (auth instanceof NextResponse) {
31+
return auth;
32+
}
33+
34+
const sessionId = request.nextUrl.searchParams.get("sessionId");
35+
if (!sessionId) {
36+
return NextResponse.json(
37+
{ status: "error", error: "Missing sessionId" },
38+
{ status: 400, headers: getCorsHeaders() },
39+
);
40+
}
41+
42+
const rows = await selectSessions({ id: sessionId });
43+
const row = rows[0];
44+
45+
if (!row) {
46+
return NextResponse.json(
47+
{ status: "error", error: "Session not found" },
48+
{ status: 404, headers: getCorsHeaders() },
49+
);
50+
}
51+
52+
if (row.account_id !== auth.accountId) {
53+
return NextResponse.json(
54+
{ status: "error", error: "Forbidden" },
55+
{ status: 403, headers: getCorsHeaders() },
56+
);
57+
}
58+
59+
return { row, auth };
60+
}

0 commit comments

Comments
 (0)