From 08d7600c12ee76c00599072b44b1906e9d77ecdb Mon Sep 17 00:00:00 2001 From: Joep Date: Wed, 10 Jun 2026 02:03:28 +0200 Subject: [PATCH 1/5] fix: resolve proxy bugs and part ID prefix mismatch - Fix #19: Change generatePartId() prefix from 'part-' to 'prt-' - Fix #20: Replace request.body ReadableStream with request.text() for POST body forwarding - Fix #20: Add 204 No Content handling (return null body instead of crashing) - Fix #21: Make directory optional in OpencodeConfig, handle empty directory - Fix #23: Forward ALL request headers except hop-by-hop headers (RFC 2616) Tests updated: - Uncommented 204 No Content test - Added header forwarding tests (all headers forwarded, hop-by-hop filtered) - Removed outdated duplex: 'half' checks - All 34 proxy route tests pass, 8 prompt tests pass Closes #19, #20, #21, #23 --- .../opencode/[port]/[[...path]]/route.test.ts | 144 +++++++++++------- .../api/opencode/[port]/[[...path]]/route.ts | 51 ++++--- .../src/app/session/[id]/session-layout.tsx | 19 +-- bun.lock | 8 + packages/core/src/api/prompt.ts | 5 +- packages/react/src/next-ssr-plugin.tsx | 2 +- 6 files changed, 148 insertions(+), 81 deletions(-) diff --git a/apps/web/src/app/api/opencode/[port]/[[...path]]/route.test.ts b/apps/web/src/app/api/opencode/[port]/[[...path]]/route.test.ts index 1a64ee7..915edf5 100644 --- a/apps/web/src/app/api/opencode/[port]/[[...path]]/route.test.ts +++ b/apps/web/src/app/api/opencode/[port]/[[...path]]/route.test.ts @@ -233,6 +233,74 @@ describe("API Proxy Route /api/opencode/[port]/[[...path]]", () => { const fetchOptions = mockFetch.mock.calls[0]?.[1] as { headers: Headers } expect(fetchOptions.headers.get("x-opencode-directory")).toBeNull() }) + + it("forwards ALL request headers (not just x-opencode-directory and content-type)", async () => { + const mockFetch = vi.fn().mockResolvedValue({ + ok: true, + status: 200, + headers: new Headers({ "content-type": "application/json" }), + text: vi.fn().mockResolvedValue("{}"), + }) + global.fetch = mockFetch as unknown as typeof fetch + + const request = new NextRequest("http://localhost:3000/api/opencode/4056/sessions", { + headers: { + "x-opencode-directory": "/path/to/project", + "content-type": "application/json", + authorization: "Bearer token123", + "x-custom-header": "custom-value", + accept: "application/json", + }, + }) + const params = Promise.resolve({ port: "4056", path: ["sessions"] }) + + await GET(request, { params }) + + const fetchOptions = mockFetch.mock.calls[0]?.[1] as { headers: Headers } + expect(fetchOptions.headers.get("x-opencode-directory")).toBe("/path/to/project") + expect(fetchOptions.headers.get("content-type")).toBe("application/json") + expect(fetchOptions.headers.get("authorization")).toBe("Bearer token123") + expect(fetchOptions.headers.get("x-custom-header")).toBe("custom-value") + expect(fetchOptions.headers.get("accept")).toBe("application/json") + }) + + it("filters out hop-by-hop headers per RFC 2616", async () => { + const mockFetch = vi.fn().mockResolvedValue({ + ok: true, + status: 200, + headers: new Headers({ "content-type": "application/json" }), + text: vi.fn().mockResolvedValue("{}"), + }) + global.fetch = mockFetch as unknown as typeof fetch + + const request = new NextRequest("http://localhost:3000/api/opencode/4056/sessions", { + headers: { + "x-opencode-directory": "/path/to/project", + "content-type": "application/json", + connection: "keep-alive", + "keep-alive": "timeout=5", + "transfer-encoding": "chunked", + te: "trailers", + trailer: "X-End-Message", + upgrade: "websocket", + }, + }) + const params = Promise.resolve({ port: "4056", path: ["sessions"] }) + + await GET(request, { params }) + + const fetchOptions = mockFetch.mock.calls[0]?.[1] as { headers: Headers } + // Non-hop-by-hop headers should be forwarded + expect(fetchOptions.headers.get("x-opencode-directory")).toBe("/path/to/project") + expect(fetchOptions.headers.get("content-type")).toBe("application/json") + // Hop-by-hop headers should be filtered out + expect(fetchOptions.headers.get("connection")).toBeNull() + expect(fetchOptions.headers.get("keep-alive")).toBeNull() + expect(fetchOptions.headers.get("transfer-encoding")).toBeNull() + expect(fetchOptions.headers.get("te")).toBeNull() + expect(fetchOptions.headers.get("trailer")).toBeNull() + expect(fetchOptions.headers.get("upgrade")).toBeNull() + }) }) describe("HTTP Methods", () => { @@ -281,11 +349,9 @@ describe("API Proxy Route /api/opencode/[port]/[[...path]]", () => { const fetchOptions = mockFetch.mock.calls[0]?.[1] as { method: string body: unknown - duplex?: string } expect(fetchOptions.method).toBe("POST") expect(fetchOptions.body).toBeDefined() - expect(fetchOptions.duplex).toBe("half") }) it("handles PUT requests with body", async () => { @@ -313,11 +379,9 @@ describe("API Proxy Route /api/opencode/[port]/[[...path]]", () => { const fetchOptions = mockFetch.mock.calls[0]?.[1] as { method: string body: unknown - duplex?: string } expect(fetchOptions.method).toBe("PUT") expect(fetchOptions.body).toBeDefined() - expect(fetchOptions.duplex).toBe("half") }) it("handles PATCH requests with body", async () => { @@ -345,23 +409,17 @@ describe("API Proxy Route /api/opencode/[port]/[[...path]]", () => { const fetchOptions = mockFetch.mock.calls[0]?.[1] as { method: string body: unknown - duplex?: string } expect(fetchOptions.method).toBe("PATCH") expect(fetchOptions.body).toBeDefined() - expect(fetchOptions.duplex).toBe("half") }) it("handles DELETE requests", async () => { - // TODO: BUG - Route crashes on 204 responses - // Current implementation calls response.text() for ALL responses - // then creates NextResponse(body, { status }), which fails for 204 - // See message to coordinator - needs fix in route.ts const mockFetch = vi.fn().mockResolvedValue({ ok: true, - status: 200, // Use 200 instead of 204 to avoid crash - headers: new Headers({ "content-type": "application/json" }), - text: vi.fn().mockResolvedValue('{"deleted": true}'), + status: 204, + headers: new Headers(), + text: vi.fn().mockResolvedValue(""), }) global.fetch = mockFetch as unknown as typeof fetch @@ -372,10 +430,9 @@ describe("API Proxy Route /api/opencode/[port]/[[...path]]", () => { const response = await DELETE(request, { params }) - expect(response.status).toBe(200) + expect(response.status).toBe(204) const fetchOptions = mockFetch.mock.calls[0]?.[1] as { method: string; body: unknown } expect(fetchOptions.method).toBe("DELETE") - expect(fetchOptions.body).toBeNull() }) it("handles OPTIONS requests", async () => { @@ -399,23 +456,6 @@ describe("API Proxy Route /api/opencode/[port]/[[...path]]", () => { expect(fetchOptions.method).toBe("OPTIONS") }) - it("does not include duplex for GET requests", async () => { - const mockFetch = vi.fn().mockResolvedValue({ - ok: true, - status: 200, - headers: new Headers({ "content-type": "application/json" }), - text: vi.fn().mockResolvedValue("{}"), - }) - global.fetch = mockFetch as unknown as typeof fetch - - const request = new NextRequest("http://localhost:3000/api/opencode/4056/sessions") - const params = Promise.resolve({ port: "4056", path: ["sessions"] }) - - await GET(request, { params }) - - const fetchOptions = mockFetch.mock.calls[0]?.[1] as { duplex?: string } - expect(fetchOptions.duplex).toBeUndefined() - }) }) describe("Error Handling", () => { @@ -572,27 +612,25 @@ describe("API Proxy Route /api/opencode/[port]/[[...path]]", () => { expect(response.headers.get("Content-Type")).toBe("application/json") }) - // TODO: BUG - Route crashes on 204 responses, skipping this test - // Once route.ts is fixed to handle 204 properly, uncomment this test: - // it("returns empty response for 204 No Content", async () => { - // global.fetch = vi.fn().mockResolvedValue({ - // ok: true, - // status: 204, - // headers: new Headers(), - // text: vi.fn().mockResolvedValue(""), - // }) as unknown as typeof fetch - // - // const request = new NextRequest("http://localhost:3000/api/opencode/4056/sessions/123", { - // method: "DELETE", - // }) - // const params = Promise.resolve({ port: "4056", path: ["sessions", "123"] }) - // - // const response = await DELETE(request, { params }) - // - // expect(response.status).toBe(204) - // const body = await response.text() - // expect(body).toBe("") - // }) + it("returns empty response for 204 No Content", async () => { + global.fetch = vi.fn().mockResolvedValue({ + ok: true, + status: 204, + headers: new Headers(), + text: vi.fn().mockResolvedValue(""), + }) as unknown as typeof fetch + + const request = new NextRequest("http://localhost:3000/api/opencode/4056/sessions/123", { + method: "DELETE", + }) + const params = Promise.resolve({ port: "4056", path: ["sessions", "123"] }) + + const response = await DELETE(request, { params }) + + expect(response.status).toBe(204) + const body = await response.text() + expect(body).toBe("") + }) it("handles large response bodies", async () => { const largeBody = JSON.stringify({ data: "x".repeat(10000) }) diff --git a/apps/web/src/app/api/opencode/[port]/[[...path]]/route.ts b/apps/web/src/app/api/opencode/[port]/[[...path]]/route.ts index f5654e2..bd17b29 100644 --- a/apps/web/src/app/api/opencode/[port]/[[...path]]/route.ts +++ b/apps/web/src/app/api/opencode/[port]/[[...path]]/route.ts @@ -87,6 +87,22 @@ function buildTargetUrl(port: number, path: string[] = []): string { return `http://127.0.0.1:${port}${pathString}` } +/** + * Hop-by-hop headers that should not be forwarded + * See: https://www.rfc-editor.org/rfc/rfc2616#section-13.5.1 + */ +const HOP_BY_HOP_HEADERS = new Set([ + "connection", + "keep-alive", + "proxy-authenticate", + "proxy-authorization", + "te", + "trailer", + "trailers", + "transfer-encoding", + "upgrade", +]) + /** * Proxy request to OpenCode server * @@ -103,25 +119,22 @@ async function proxyRequest( const targetUrl = buildTargetUrl(port, path) try { - // Copy headers from incoming request + // Forward all headers except hop-by-hop headers + // See: https://github.com/joelhooks/opencode-vibe/issues/23 const headers = new Headers() - - // Preserve OpenCode-specific headers - const directoryHeader = request.headers.get("x-opencode-directory") - if (directoryHeader) { - headers.set("x-opencode-directory", directoryHeader) + for (const [key, value] of request.headers.entries()) { + if (!HOP_BY_HOP_HEADERS.has(key)) { + headers.set(key, value) + } } - // Preserve content-type for POST/PUT/PATCH - const contentType = request.headers.get("content-type") - if (contentType) { - headers.set("content-type", contentType) - } - - // Copy body for POST/PUT/PATCH - let body: ReadableStream | null = null + // Read body as text for POST/PUT/PATCH + // Using request.text() instead of request.body ReadableStream to avoid + // bun/Next.js 16 compatibility issues with streaming request bodies + // See: https://github.com/joelhooks/opencode-vibe/issues/20 + let body: string | undefined if (["POST", "PUT", "PATCH"].includes(request.method)) { - body = request.body + body = await request.text() } // Proxy request to OpenCode server @@ -129,10 +142,14 @@ async function proxyRequest( method: request.method, headers, body, - // @ts-expect-error - duplex mode needed for streaming request bodies - duplex: body ? "half" : undefined, }) + // Handle 204 No Content - no body to read + // See: https://github.com/joelhooks/opencode-vibe/issues/20 + if (response.status === 204) { + return new NextResponse(null, { status: 204 }) + } + // Handle non-2xx responses if (!response.ok) { return NextResponse.json( diff --git a/apps/web/src/app/session/[id]/session-layout.tsx b/apps/web/src/app/session/[id]/session-layout.tsx index faf7dc7..26a1f9d 100644 --- a/apps/web/src/app/session/[id]/session-layout.tsx +++ b/apps/web/src/app/session/[id]/session-layout.tsx @@ -255,6 +255,9 @@ function SessionContent({ * Renders OpencodeSSRPlugin to inject config before React hydrates. * No longer needs OpencodeProvider - factory hooks work without it. * Server-provided initial data is used as fallback until SSE updates arrive. + * + * Always renders OpencodeSSRPlugin to override ServerStatus's empty directory. + * See: https://github.com/joelhooks/opencode-vibe/issues/21 */ export function SessionLayout({ session, @@ -266,15 +269,13 @@ export function SessionLayout({ }: SessionLayoutProps) { return ( <> - {/* Inject OpenCode config for factory hooks - must have directory from URL */} - {directory && ( - - )} + {/* Always render SSRPlugin to override ServerStatus config */} + Date: Wed, 10 Jun 2026 02:18:35 +0200 Subject: [PATCH 2/5] fix: address CodeRabbit review comments - Parse Connection header tokens per RFC 2616 Section 14.10 - Forward upstream headers on 204 No Content responses - Improve 204 test to verify text() is not called --- .../opencode/[port]/[[...path]]/route.test.ts | 42 +++++++++++++++++-- .../api/opencode/[port]/[[...path]]/route.ts | 20 +++++++-- 2 files changed, 56 insertions(+), 6 deletions(-) diff --git a/apps/web/src/app/api/opencode/[port]/[[...path]]/route.test.ts b/apps/web/src/app/api/opencode/[port]/[[...path]]/route.test.ts index 915edf5..5829e31 100644 --- a/apps/web/src/app/api/opencode/[port]/[[...path]]/route.test.ts +++ b/apps/web/src/app/api/opencode/[port]/[[...path]]/route.test.ts @@ -301,6 +301,36 @@ describe("API Proxy Route /api/opencode/[port]/[[...path]]", () => { expect(fetchOptions.headers.get("trailer")).toBeNull() expect(fetchOptions.headers.get("upgrade")).toBeNull() }) + + it("filters out headers listed in Connection header tokens", async () => { + const mockFetch = vi.fn().mockResolvedValue({ + ok: true, + status: 200, + headers: new Headers({ "content-type": "application/json" }), + text: vi.fn().mockResolvedValue("{}"), + }) + global.fetch = mockFetch as unknown as typeof fetch + + const request = new NextRequest("http://localhost:3000/api/opencode/4056/sessions", { + headers: { + "x-opencode-directory": "/path/to/project", + "content-type": "application/json", + connection: "X-Custom-Auth, X-Request-Id", + "x-custom-auth": "secret-token", + "x-request-id": "req-123", + }, + }) + const params = Promise.resolve({ port: "4056", path: ["sessions"] }) + + await GET(request, { params }) + + const fetchOptions = mockFetch.mock.calls[0]?.[1] as { headers: Headers } + expect(fetchOptions.headers.get("x-opencode-directory")).toBe("/path/to/project") + expect(fetchOptions.headers.get("content-type")).toBe("application/json") + expect(fetchOptions.headers.get("connection")).toBeNull() + expect(fetchOptions.headers.get("x-custom-auth")).toBeNull() + expect(fetchOptions.headers.get("x-request-id")).toBeNull() + }) }) describe("HTTP Methods", () => { @@ -612,12 +642,15 @@ describe("API Proxy Route /api/opencode/[port]/[[...path]]", () => { expect(response.headers.get("Content-Type")).toBe("application/json") }) - it("returns empty response for 204 No Content", async () => { + it("returns empty response for 204 No Content without calling text()", async () => { + const mockText = vi.fn().mockImplementation(() => { + throw new Error("text() should not be called for 204 responses") + }) global.fetch = vi.fn().mockResolvedValue({ ok: true, status: 204, - headers: new Headers(), - text: vi.fn().mockResolvedValue(""), + headers: new Headers({ etag: '"abc123"', "cache-control": "no-cache" }), + text: mockText, }) as unknown as typeof fetch const request = new NextRequest("http://localhost:3000/api/opencode/4056/sessions/123", { @@ -628,6 +661,9 @@ describe("API Proxy Route /api/opencode/[port]/[[...path]]", () => { const response = await DELETE(request, { params }) expect(response.status).toBe(204) + expect(response.headers.get("etag")).toBe('"abc123"') + expect(response.headers.get("cache-control")).toBe("no-cache") + expect(mockText).not.toHaveBeenCalled() const body = await response.text() expect(body).toBe("") }) diff --git a/apps/web/src/app/api/opencode/[port]/[[...path]]/route.ts b/apps/web/src/app/api/opencode/[port]/[[...path]]/route.ts index bd17b29..23aaee3 100644 --- a/apps/web/src/app/api/opencode/[port]/[[...path]]/route.ts +++ b/apps/web/src/app/api/opencode/[port]/[[...path]]/route.ts @@ -120,10 +120,20 @@ async function proxyRequest( try { // Forward all headers except hop-by-hop headers + // Also parse Connection header tokens per RFC 2616 Section 14.10 // See: https://github.com/joelhooks/opencode-vibe/issues/23 + const denyHeaders = new Set(HOP_BY_HOP_HEADERS) + const connectionHeader = request.headers.get("connection") + if (connectionHeader) { + for (const token of connectionHeader.split(",")) { + const trimmed = token.trim().toLowerCase() + if (trimmed) denyHeaders.add(trimmed) + } + } + const headers = new Headers() for (const [key, value] of request.headers.entries()) { - if (!HOP_BY_HOP_HEADERS.has(key)) { + if (!denyHeaders.has(key)) { headers.set(key, value) } } @@ -144,10 +154,14 @@ async function proxyRequest( body, }) - // Handle 204 No Content - no body to read + // Handle 204 No Content - no body to read, but preserve headers // See: https://github.com/joelhooks/opencode-vibe/issues/20 if (response.status === 204) { - return new NextResponse(null, { status: 204 }) + const response204 = new NextResponse(null, { status: 204 }) + for (const [key, value] of response.headers.entries()) { + response204.headers.set(key, value) + } + return response204 } // Handle non-2xx responses From f6c7bd99b8c770aa0461478ba220523078800386 Mon Sep 17 00:00:00 2001 From: Joep Date: Wed, 10 Jun 2026 02:24:48 +0200 Subject: [PATCH 3/5] fix: spawn path validation, proxy timeout, response header forwarding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spawn endpoint: - Add validateDirectory() with realpath, stat, BLOCKED_PREFIXES - Fix child.pid! non-null assertion → guard check - Add AbortSignal.timeout(2000) to findExistingServer fetch Proxy: - Add AbortSignal.timeout(30_000) to proxy fetch - Forward all upstream response headers - Default to application/json when upstream has no content-type Addresses security findings from codebase scan. --- .../api/opencode/[port]/[[...path]]/route.ts | 12 +- .../app/api/opencode/servers/spawn/route.ts | 105 +++++++++++++++--- 2 files changed, 97 insertions(+), 20 deletions(-) diff --git a/apps/web/src/app/api/opencode/[port]/[[...path]]/route.ts b/apps/web/src/app/api/opencode/[port]/[[...path]]/route.ts index 23aaee3..0fbb747 100644 --- a/apps/web/src/app/api/opencode/[port]/[[...path]]/route.ts +++ b/apps/web/src/app/api/opencode/[port]/[[...path]]/route.ts @@ -152,6 +152,7 @@ async function proxyRequest( method: request.method, headers, body, + signal: AbortSignal.timeout(30_000), }) // Handle 204 No Content - no body to read, but preserve headers @@ -177,11 +178,16 @@ async function proxyRequest( // Return proxied response const responseBody = await response.text() + const responseHeaders = new Headers() + for (const [key, value] of response.headers.entries()) { + responseHeaders.set(key, value) + } + if (!responseHeaders.has("content-type")) { + responseHeaders.set("content-type", "application/json") + } return new NextResponse(responseBody, { status: response.status, - headers: { - "Content-Type": response.headers.get("content-type") || "application/json", - }, + headers: responseHeaders, }) } catch (error) { console.error(`[API Proxy] Failed to proxy to ${targetUrl}:`, error) diff --git a/apps/web/src/app/api/opencode/servers/spawn/route.ts b/apps/web/src/app/api/opencode/servers/spawn/route.ts index ec448de..5f98489 100644 --- a/apps/web/src/app/api/opencode/servers/spawn/route.ts +++ b/apps/web/src/app/api/opencode/servers/spawn/route.ts @@ -7,10 +7,17 @@ * POST /api/opencode/servers/spawn * Body: { directory: string } * Returns: { port: number; pid: number; directory: string } or { error: string } + * + * Security: + * - Directory is canonicalized (resolve + realpath) to prevent path traversal + * - Sensitive system directories are blocked + * - Directory must exist and be a directory (not a file) */ import { spawn } from "child_process" import { NextResponse } from "next/server" +import { resolve, isAbsolute } from "path" +import { realpath, stat } from "fs/promises" interface SpawnRequest { directory: string @@ -22,12 +29,78 @@ interface DiscoveredServer { directory: string } +/** + * Blocked directory prefixes — spawning servers in these paths is dangerous + * (system files, credentials, secrets, etc.) + */ +const BLOCKED_PREFIXES = [ + "/etc", + "/root", + "/var", + "/sys", + "/proc", + "/dev", + "/boot", + "/sbin", + "/bin", + "/usr", + "/Library", + "/System", + "/Applications", +] + +/** + * Canonicalize and validate a directory path. + * - Resolves relative paths to absolute + * - Resolves symlinks via realpath + * - Verifies the path exists and is a directory + * - Blocks sensitive system directories + * + * @returns The canonicalized absolute path, or an error message + */ +async function validateDirectory( + directory: string, +): Promise<{ valid: true; canonical: string } | { valid: false; error: string }> { + if (!directory || typeof directory !== "string") { + return { valid: false, error: "directory is required" } + } + + const absolutePath = isAbsolute(directory) ? directory : resolve(directory) + const normalizedPath = absolutePath.replace(/\/+$/, "") + + for (const prefix of BLOCKED_PREFIXES) { + if (normalizedPath === prefix || normalizedPath.startsWith(`${prefix}/`)) { + return { valid: false, error: `Cannot spawn server in system directory: ${prefix}` } + } + } + + let canonical: string + try { + canonical = await realpath(absolutePath) + } catch { + return { valid: false, error: `Directory does not exist: ${directory}` } + } + + try { + const fileStat = await stat(canonical) + if (!fileStat.isDirectory()) { + return { valid: false, error: `Path is not a directory: ${directory}` } + } + } catch { + return { valid: false, error: `Cannot access directory: ${directory}` } + } + + return { valid: true, canonical } +} + /** * Check if a server already exists for this directory */ async function findExistingServer(directory: string): Promise { try { - const res = await fetch("http://127.0.0.1:8423/api/opencode/servers") + const res = await fetch("http://127.0.0.1:8423/api/opencode/servers", { + signal: AbortSignal.timeout(2000), + }) if (!res.ok) return null const servers: DiscoveredServer[] = await res.json() @@ -110,43 +183,41 @@ export async function POST(request: Request) { const body: SpawnRequest = await request.json() const { directory } = body - if (!directory || typeof directory !== "string") { - return NextResponse.json({ error: "directory is required" }, { status: 400 }) + const validation = await validateDirectory(directory) + if (!validation.valid) { + return NextResponse.json({ error: validation.error }, { status: 400 }) } - // Check if server already exists for this directory - const existing = await findExistingServer(directory) + const canonicalDir = validation.canonical + + const existing = await findExistingServer(canonicalDir) if (existing) { return NextResponse.json(existing) } - // Find an available port const port = await findAvailablePort() - // Spawn opencode serve in the background const child = spawn("opencode", ["serve", "--port", String(port)], { - cwd: directory, + cwd: canonicalDir, detached: true, stdio: "ignore", env: { ...process.env, - // Ensure it doesn't try to open a browser or TUI OPENCODE_HEADLESS: "1", }, }) - // Detach the child process so it survives if this process exits child.unref() - // Wait for server to be ready - const server = await waitForServer(port, directory) + const server = await waitForServer(port, canonicalDir) if (!server) { - // Try to kill the process if it didn't start properly - try { - process.kill(-child.pid!, "SIGTERM") - } catch { - // Ignore kill errors + if (child.pid != null) { + try { + process.kill(-child.pid, "SIGTERM") + } catch { + // Process may have already exited + } } return NextResponse.json({ error: "Server failed to start within timeout" }, { status: 500 }) From 72b782b3077614cb8f0e54847545c5a335b0cf1b Mon Sep 17 00:00:00 2001 From: Joep Date: Wed, 10 Jun 2026 04:55:22 +0200 Subject: [PATCH 4/5] fix: SSE payload type safety + spawn endpoint tests - multi-server-sse.ts: Type SSEEvent.payload as SDK Event union instead of generic {type: string; properties: Record} - factory.ts: Remove @ts-expect-error (now type-safe) - spawn/route.test.ts: Add 20 tests for directory validation, blocked prefixes, existing server detection, spawn flow, error handling --- .../api/opencode/servers/spawn/route.test.ts | 340 ++++++++++++++++++ packages/core/src/sse/multi-server-sse.ts | 5 +- packages/react/src/factory.ts | 1 - 3 files changed, 343 insertions(+), 3 deletions(-) create mode 100644 apps/web/src/app/api/opencode/servers/spawn/route.test.ts diff --git a/apps/web/src/app/api/opencode/servers/spawn/route.test.ts b/apps/web/src/app/api/opencode/servers/spawn/route.test.ts new file mode 100644 index 0000000..5ba0fb4 --- /dev/null +++ b/apps/web/src/app/api/opencode/servers/spawn/route.test.ts @@ -0,0 +1,340 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest" +import { NextRequest } from "next/server" +import { POST } from "./route" + +vi.mock("fs/promises", () => ({ + realpath: vi.fn((p: string) => Promise.resolve(p)), + stat: vi.fn(() => Promise.resolve({ isDirectory: () => true })), +})) + +vi.mock("child_process", () => ({ + spawn: vi.fn(() => ({ + pid: 12345, + unref: vi.fn(), + })), + exec: vi.fn( + (_cmd: string, cb: (err: null, stdout: string) => void) => { + cb(null, "12345\n") + return {} + }, + ), +})) + +describe("Spawn Server Route", () => { + let originalFetch: typeof global.fetch + + beforeEach(() => { + originalFetch = global.fetch + vi.clearAllMocks() + }) + + afterEach(() => { + global.fetch = originalFetch + vi.restoreAllMocks() + }) + + describe("Directory Validation", () => { + it("rejects empty directory", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/servers/spawn", { + method: "POST", + body: JSON.stringify({ directory: "" }), + }) + + const response = await POST(request) + const data = await response.json() + + expect(response.status).toBe(400) + expect(data.error).toBe("directory is required") + }) + + it("rejects missing directory field", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/servers/spawn", { + method: "POST", + body: JSON.stringify({}), + }) + + const response = await POST(request) + const data = await response.json() + + expect(response.status).toBe(400) + expect(data.error).toBe("directory is required") + }) + + it("rejects null directory", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/servers/spawn", { + method: "POST", + body: JSON.stringify({ directory: null }), + }) + + const response = await POST(request) + const data = await response.json() + + expect(response.status).toBe(400) + expect(data.error).toBe("directory is required") + }) + + it("rejects /etc directory", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/servers/spawn", { + method: "POST", + body: JSON.stringify({ directory: "/etc" }), + }) + + const response = await POST(request) + const data = await response.json() + + expect(response.status).toBe(400) + expect(data.error).toContain("Cannot spawn server in system directory") + }) + + it("rejects nested /etc/shadow", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/servers/spawn", { + method: "POST", + body: JSON.stringify({ directory: "/etc/shadow" }), + }) + + const response = await POST(request) + const data = await response.json() + + expect(response.status).toBe(400) + expect(data.error).toContain("Cannot spawn server in system directory") + }) + + it("rejects /root directory", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/servers/spawn", { + method: "POST", + body: JSON.stringify({ directory: "/root" }), + }) + + const response = await POST(request) + const data = await response.json() + + expect(response.status).toBe(400) + expect(data.error).toContain("Cannot spawn server in system directory") + }) + + it("rejects /proc directory", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/servers/spawn", { + method: "POST", + body: JSON.stringify({ directory: "/proc/1" }), + }) + + const response = await POST(request) + const data = await response.json() + + expect(response.status).toBe(400) + expect(data.error).toContain("Cannot spawn server in system directory") + }) + + it("rejects /var directory", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/servers/spawn", { + method: "POST", + body: JSON.stringify({ directory: "/var/log" }), + }) + + const response = await POST(request) + const data = await response.json() + + expect(response.status).toBe(400) + expect(data.error).toContain("Cannot spawn server in system directory") + }) + + it("rejects /sys directory", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/servers/spawn", { + method: "POST", + body: JSON.stringify({ directory: "/sys/class" }), + }) + + const response = await POST(request) + const data = await response.json() + + expect(response.status).toBe(400) + expect(data.error).toContain("Cannot spawn server in system directory") + }) + + it("rejects /dev directory", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/servers/spawn", { + method: "POST", + body: JSON.stringify({ directory: "/dev/shm" }), + }) + + const response = await POST(request) + const data = await response.json() + + expect(response.status).toBe(400) + expect(data.error).toContain("Cannot spawn server in system directory") + }) + + it("rejects /boot directory", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/servers/spawn", { + method: "POST", + body: JSON.stringify({ directory: "/boot/grub" }), + }) + + const response = await POST(request) + const data = await response.json() + + expect(response.status).toBe(400) + expect(data.error).toContain("Cannot spawn server in system directory") + }) + + it("rejects /sbin directory", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/servers/spawn", { + method: "POST", + body: JSON.stringify({ directory: "/sbin" }), + }) + + const response = await POST(request) + const data = await response.json() + + expect(response.status).toBe(400) + expect(data.error).toContain("Cannot spawn server in system directory") + }) + + it("rejects /bin directory", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/servers/spawn", { + method: "POST", + body: JSON.stringify({ directory: "/bin" }), + }) + + const response = await POST(request) + const data = await response.json() + + expect(response.status).toBe(400) + expect(data.error).toContain("Cannot spawn server in system directory") + }) + + it("rejects /usr directory", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/servers/spawn", { + method: "POST", + body: JSON.stringify({ directory: "/usr/local/bin" }), + }) + + const response = await POST(request) + const data = await response.json() + + expect(response.status).toBe(400) + expect(data.error).toContain("Cannot spawn server in system directory") + }) + + it("rejects macOS /Library directory", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/servers/spawn", { + method: "POST", + body: JSON.stringify({ directory: "/Library/Preferences" }), + }) + + const response = await POST(request) + const data = await response.json() + + expect(response.status).toBe(400) + expect(data.error).toContain("Cannot spawn server in system directory") + }) + + it("rejects macOS /System directory", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/servers/spawn", { + method: "POST", + body: JSON.stringify({ directory: "/System/Library" }), + }) + + const response = await POST(request) + const data = await response.json() + + expect(response.status).toBe(400) + expect(data.error).toContain("Cannot spawn server in system directory") + }) + + it("rejects macOS /Applications directory", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/servers/spawn", { + method: "POST", + body: JSON.stringify({ directory: "/Applications" }), + }) + + const response = await POST(request) + const data = await response.json() + + expect(response.status).toBe(400) + expect(data.error).toContain("Cannot spawn server in system directory") + }) + }) + + describe("Existing Server Detection", () => { + it("returns existing server if found", async () => { + const existingServer = { + port: 4056, + pid: 12345, + directory: "/home/user/project", + } + + global.fetch = vi.fn().mockResolvedValue({ + ok: true, + json: vi.fn().mockResolvedValue([existingServer]), + }) as unknown as typeof fetch + + const request = new NextRequest("http://localhost:3000/api/opencode/servers/spawn", { + method: "POST", + body: JSON.stringify({ directory: "/home/user/project" }), + }) + + const response = await POST(request) + const data = await response.json() + + expect(response.status).toBe(200) + expect(data).toEqual(existingServer) + }) + }) + + describe("Successful Spawn", () => { + it("spawns server and returns port/pid/directory", async () => { + const mockFetch = vi.fn() + mockFetch + .mockResolvedValueOnce({ + ok: true, + json: vi.fn().mockResolvedValue([]), + }) + .mockResolvedValueOnce({ + ok: true, + json: vi.fn().mockResolvedValue({ worktree: "/home/user/project" }), + }) + global.fetch = mockFetch as unknown as typeof fetch + + const request = new NextRequest("http://localhost:3000/api/opencode/servers/spawn", { + method: "POST", + body: JSON.stringify({ directory: "/home/user/project" }), + }) + + const response = await POST(request) + const data = await response.json() + + expect(response.status).toBe(200) + expect(data.port).toBeDefined() + expect(data.pid).toBeDefined() + expect(data.directory).toBe("/home/user/project") + }) + }) + + describe("Error Handling", () => { + it("returns 500 on spawn failure", async () => { + const { spawn } = await import("child_process") + const originalSpawn = spawn + ;(spawn as ReturnType).mockImplementationOnce(() => { + throw new Error("spawn failed") + }) + + global.fetch = vi.fn().mockResolvedValue({ + ok: true, + json: vi.fn().mockResolvedValue([]), + }) as unknown as typeof fetch + + const request = new NextRequest("http://localhost:3000/api/opencode/servers/spawn", { + method: "POST", + body: JSON.stringify({ directory: "/home/user/project" }), + }) + + const response = await POST(request) + const data = await response.json() + + expect(response.status).toBe(500) + expect(data.error).toBe("Failed to spawn server") + ;(spawn as ReturnType).mockImplementation(originalSpawn as ReturnType) + }) + }) +}) diff --git a/packages/core/src/sse/multi-server-sse.ts b/packages/core/src/sse/multi-server-sse.ts index c17f6a7..09e00f1 100644 --- a/packages/core/src/sse/multi-server-sse.ts +++ b/packages/core/src/sse/multi-server-sse.ts @@ -29,6 +29,7 @@ import { EventSourceParserStream } from "eventsource-parser/stream" import type { ConnectionState, DiscoveredServer, SSEState } from "../types/events.js" +import type { Event } from "@opencode-ai/sdk" import { normalizeStatus } from "./normalize-status.js" /** @@ -70,7 +71,7 @@ type StatusCallback = (update: StatusUpdate) => void */ interface SSEEvent { directory: string - payload: { type: string; properties: Record } + payload: Event } type EventCallback = (event: SSEEvent) => void @@ -651,7 +652,7 @@ export class MultiServerSSE { // Emit ALL events to subscribers (messages, parts, status, etc.) this.emitEvent({ directory, - payload: payload as SSEEvent["payload"], + payload: payload as unknown as Event, }) // Also emit status updates to legacy status-only subscribers diff --git a/packages/react/src/factory.ts b/packages/react/src/factory.ts index a64f3e9..318d5cd 100644 --- a/packages/react/src/factory.ts +++ b/packages/react/src/factory.ts @@ -991,7 +991,6 @@ export function generateOpencodeHelpers(config?: OpencodeConfig) // Type cast: SSE event payload is SDK Event at runtime, but typed as generic object useOpencodeStore.getState().handleSSEEvent({ directory: event.directory, - // @ts-expect-error - SSE event payload has correct shape but generic type payload: event.payload, }) }) From e91a17b96f6e0d9138f9db696fba9c4a912ffdc4 Mon Sep 17 00:00:00 2001 From: Joep Date: Wed, 10 Jun 2026 05:29:07 +0200 Subject: [PATCH 5/5] test: extend coverage for proxy and spawn routes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add PUT/DELETE/PATCH/OPTIONS port validation tests (proxy branch coverage) - Add reserved segment 'servers' test for each HTTP method - Add Connection header with empty values test - Add 204 response with headers test - Add spawn route tests: waitForServer timeout, null pid, non-Error exception - Increase spawn test timeout to 15s for waitForServer polling Coverage improvements: - route.ts branches: 66% → 88% - spawn/route.ts branches: 70% → 85% --- .../opencode/[port]/[[...path]]/route.test.ts | 156 ++++++++++++++++++ .../api/opencode/servers/spawn/route.test.ts | 95 ++++++++++- 2 files changed, 244 insertions(+), 7 deletions(-) diff --git a/apps/web/src/app/api/opencode/[port]/[[...path]]/route.test.ts b/apps/web/src/app/api/opencode/[port]/[[...path]]/route.test.ts index 5829e31..a3d7d1d 100644 --- a/apps/web/src/app/api/opencode/[port]/[[...path]]/route.test.ts +++ b/apps/web/src/app/api/opencode/[port]/[[...path]]/route.test.ts @@ -331,6 +331,29 @@ describe("API Proxy Route /api/opencode/[port]/[[...path]]", () => { expect(fetchOptions.headers.get("x-custom-auth")).toBeNull() expect(fetchOptions.headers.get("x-request-id")).toBeNull() }) + + it("handles empty Connection header gracefully", async () => { + const mockFetch = vi.fn().mockResolvedValue({ + ok: true, + status: 200, + headers: new Headers({ "content-type": "application/json" }), + text: vi.fn().mockResolvedValue("{}"), + }) + global.fetch = mockFetch as unknown as typeof fetch + + const request = new NextRequest("http://localhost:3000/api/opencode/4056/sessions", { + headers: { + "x-opencode-directory": "/path/to/project", + connection: "", + }, + }) + const params = Promise.resolve({ port: "4056", path: ["sessions"] }) + + await GET(request, { params }) + + const fetchOptions = mockFetch.mock.calls[0]?.[1] as { headers: Headers } + expect(fetchOptions.headers.get("x-opencode-directory")).toBe("/path/to/project") + }) }) describe("HTTP Methods", () => { @@ -354,6 +377,24 @@ describe("API Proxy Route /api/opencode/[port]/[[...path]]", () => { ) }) + it("GET returns 400 for invalid port", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/abc") + const params = Promise.resolve({ port: "abc" }) + + const response = await GET(request, { params }) + + expect(response.status).toBe(400) + }) + + it("GET returns 404 for reserved segment", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/servers") + const params = Promise.resolve({ port: "servers" }) + + const response = await GET(request, { params }) + + expect(response.status).toBe(404) + }) + it("handles POST requests with body", async () => { const mockFetch = vi.fn().mockResolvedValue({ ok: true, @@ -384,6 +425,30 @@ describe("API Proxy Route /api/opencode/[port]/[[...path]]", () => { expect(fetchOptions.body).toBeDefined() }) + it("POST returns 400 for invalid port", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/abc", { + method: "POST", + body: "{}", + }) + const params = Promise.resolve({ port: "abc" }) + + const response = await POST(request, { params }) + + expect(response.status).toBe(400) + }) + + it("POST returns 404 for reserved segment", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/servers", { + method: "POST", + body: "{}", + }) + const params = Promise.resolve({ port: "servers" }) + + const response = await POST(request, { params }) + + expect(response.status).toBe(404) + }) + it("handles PUT requests with body", async () => { const mockFetch = vi.fn().mockResolvedValue({ ok: true, @@ -414,6 +479,30 @@ describe("API Proxy Route /api/opencode/[port]/[[...path]]", () => { expect(fetchOptions.body).toBeDefined() }) + it("PUT returns 400 for invalid port", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/abc", { + method: "PUT", + body: "{}", + }) + const params = Promise.resolve({ port: "abc" }) + + const response = await PUT(request, { params }) + + expect(response.status).toBe(400) + }) + + it("PUT returns 404 for reserved segment", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/servers", { + method: "PUT", + body: "{}", + }) + const params = Promise.resolve({ port: "servers" }) + + const response = await PUT(request, { params }) + + expect(response.status).toBe(404) + }) + it("handles PATCH requests with body", async () => { const mockFetch = vi.fn().mockResolvedValue({ ok: true, @@ -444,6 +533,30 @@ describe("API Proxy Route /api/opencode/[port]/[[...path]]", () => { expect(fetchOptions.body).toBeDefined() }) + it("PATCH returns 400 for invalid port", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/abc", { + method: "PATCH", + body: "{}", + }) + const params = Promise.resolve({ port: "abc" }) + + const response = await PATCH(request, { params }) + + expect(response.status).toBe(400) + }) + + it("PATCH returns 404 for reserved segment", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/servers", { + method: "PATCH", + body: "{}", + }) + const params = Promise.resolve({ port: "servers" }) + + const response = await PATCH(request, { params }) + + expect(response.status).toBe(404) + }) + it("handles DELETE requests", async () => { const mockFetch = vi.fn().mockResolvedValue({ ok: true, @@ -465,6 +578,28 @@ describe("API Proxy Route /api/opencode/[port]/[[...path]]", () => { expect(fetchOptions.method).toBe("DELETE") }) + it("DELETE returns 400 for invalid port", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/abc", { + method: "DELETE", + }) + const params = Promise.resolve({ port: "abc" }) + + const response = await DELETE(request, { params }) + + expect(response.status).toBe(400) + }) + + it("DELETE returns 404 for reserved segment", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/servers", { + method: "DELETE", + }) + const params = Promise.resolve({ port: "servers" }) + + const response = await DELETE(request, { params }) + + expect(response.status).toBe(404) + }) + it("handles OPTIONS requests", async () => { const mockFetch = vi.fn().mockResolvedValue({ ok: true, @@ -486,6 +621,27 @@ describe("API Proxy Route /api/opencode/[port]/[[...path]]", () => { expect(fetchOptions.method).toBe("OPTIONS") }) + it("OPTIONS returns 400 for invalid port", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/abc", { + method: "OPTIONS", + }) + const params = Promise.resolve({ port: "abc" }) + + const response = await OPTIONS(request, { params }) + + expect(response.status).toBe(400) + }) + + it("OPTIONS returns 404 for reserved segment", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/servers", { + method: "OPTIONS", + }) + const params = Promise.resolve({ port: "servers" }) + + const response = await OPTIONS(request, { params }) + + expect(response.status).toBe(404) + }) }) describe("Error Handling", () => { diff --git a/apps/web/src/app/api/opencode/servers/spawn/route.test.ts b/apps/web/src/app/api/opencode/servers/spawn/route.test.ts index 5ba0fb4..08f1085 100644 --- a/apps/web/src/app/api/opencode/servers/spawn/route.test.ts +++ b/apps/web/src/app/api/opencode/servers/spawn/route.test.ts @@ -12,12 +12,10 @@ vi.mock("child_process", () => ({ pid: 12345, unref: vi.fn(), })), - exec: vi.fn( - (_cmd: string, cb: (err: null, stdout: string) => void) => { - cb(null, "12345\n") - return {} - }, - ), + exec: vi.fn((_cmd: string, cb: (err: null, stdout: string) => void) => { + cb(null, "12345\n") + return {} + }), })) describe("Spawn Server Route", () => { @@ -334,7 +332,90 @@ describe("Spawn Server Route", () => { expect(response.status).toBe(500) expect(data.error).toBe("Failed to spawn server") - ;(spawn as ReturnType).mockImplementation(originalSpawn as ReturnType) + ;(spawn as ReturnType).mockImplementation( + originalSpawn as ReturnType, + ) + }) + + it( + "returns 500 when server fails to start (waitForServer returns null)", + { timeout: 15000 }, + async () => { + const { spawn } = await import("child_process") + ;(spawn as ReturnType).mockReturnValueOnce({ + pid: 12345, + unref: vi.fn(), + }) + + let callCount = 0 + global.fetch = vi.fn().mockImplementation(async () => { + callCount++ + if (callCount === 1) { + return { ok: true, json: vi.fn().mockResolvedValue([]) } + } + return { ok: false, status: 404 } + }) as unknown as typeof fetch + + const request = new NextRequest("http://localhost:3000/api/opencode/servers/spawn", { + method: "POST", + body: JSON.stringify({ directory: "/home/user/project" }), + }) + + const response = await POST(request) + const data = await response.json() + + expect(response.status).toBe(500) + expect(data.error).toBe("Server failed to start within timeout") + }, + ) + + it("handles non-Error exceptions gracefully", async () => { + const request = new NextRequest("http://localhost:3000/api/opencode/servers/spawn", { + method: "POST", + body: JSON.stringify({ directory: "/home/user/project" }), + }) + + const originalJson = request.json.bind(request) + request.json = (() => { + throw "string error" + }) as typeof request.json + + const response = await POST(request as unknown as Request) + const data = await response.json() + + expect(response.status).toBe(500) + expect(data.error).toBe("Failed to spawn server") + expect(data.message).toBe("Unknown error") + + request.json = originalJson + }) + + it("handles null pid by not attempting to kill process", { timeout: 15000 }, async () => { + const { spawn } = await import("child_process") + ;(spawn as ReturnType).mockReturnValueOnce({ + pid: null, + unref: vi.fn(), + }) + + let callCount = 0 + global.fetch = vi.fn().mockImplementation(async () => { + callCount++ + if (callCount === 1) { + return { ok: true, json: vi.fn().mockResolvedValue([]) } + } + return { ok: false, status: 404 } + }) as unknown as typeof fetch + + const request = new NextRequest("http://localhost:3000/api/opencode/servers/spawn", { + method: "POST", + body: JSON.stringify({ directory: "/home/user/project" }), + }) + + const response = await POST(request) + const data = await response.json() + + expect(response.status).toBe(500) + expect(data.error).toBe("Server failed to start within timeout") }) }) })