From 717253cf8e6dfd1dac3483cb33ed16bfddddf42b Mon Sep 17 00:00:00 2001 From: suraj-markup Date: Sun, 29 Mar 2026 03:17:42 +0530 Subject: [PATCH] fix(web): harden project validation, fetch timeout, and test coverage - centralize project existence check via validateConfiguredProject (Object.hasOwn) and apply to spawn, issues, verify, orchestrators routes - add AbortController (1.5s) to runtime terminal config fetch in DirectTerminal - add runtimeFetchDone flag to prevent repeated fetches on reconnect - restore resolveDashboardProjectFilter fallback to getPrimaryProjectId() - expand runtime terminal endpoint tests (defaults, invalid ports, proxy path) - add validation.test.ts covering prototype-chain bypass cases - add undefined case test for resolveDashboardProjectFilter - update changeset with full list of changes for npm publish Co-Authored-By: Claude Sonnet 4.6 --- .changeset/five-lamps-heal.md | 8 +- packages/web/src/__tests__/api-routes.test.ts | 98 +++++++++++++++---- packages/web/src/app/api/issues/route.ts | 9 +- .../web/src/app/api/orchestrators/route.ts | 10 +- packages/web/src/app/api/spawn/route.ts | 13 +-- packages/web/src/app/api/verify/route.ts | 9 +- .../web/src/components/DirectTerminal.tsx | 15 ++- .../lib/__tests__/dashboard-page-data.test.ts | 4 + .../web/src/lib/__tests__/validation.test.ts | 28 ++++++ packages/web/src/lib/dashboard-page-data.ts | 2 +- packages/web/src/lib/validation.ts | 14 +++ 11 files changed, 166 insertions(+), 44 deletions(-) create mode 100644 packages/web/src/lib/__tests__/validation.test.ts diff --git a/.changeset/five-lamps-heal.md b/.changeset/five-lamps-heal.md index 94f2c5a43..3a20b6a86 100644 --- a/.changeset/five-lamps-heal.md +++ b/.changeset/five-lamps-heal.md @@ -2,9 +2,13 @@ "@composio/ao-web": patch --- -Fix runtime terminal websocket connectivity for npm-installed/prebuilt runs and harden spawn project validation. +Fix runtime terminal websocket connectivity for npm-installed/prebuilt runs and harden project validation across API routes. - add runtime terminal config endpoint (`/api/runtime/terminal`) so the browser can read runtime-selected ports - make direct terminal client resolve websocket target from runtime config before connect/reconnect -- return deterministic `404 Unknown project` from `/api/spawn` for non-configured project IDs +- add AbortController (1.5s) to runtime config fetch so a slow endpoint cannot block WebSocket connection +- prevent repeated runtime config fetches on reconnect when the endpoint is unavailable +- centralize project existence check via `validateConfiguredProject` (uses `Object.hasOwn` to avoid prototype-chain bypass) +- apply semantic project validation to `/api/spawn`, `/api/issues`, `/api/verify`, and `/api/orchestrators` +- return deterministic `404 Unknown project` from all routes for non-configured project IDs - normalize dashboard project filter to configured project IDs to prevent invalid query state propagation diff --git a/packages/web/src/__tests__/api-routes.test.ts b/packages/web/src/__tests__/api-routes.test.ts index 7afa272fd..c67a31032 100644 --- a/packages/web/src/__tests__/api-routes.test.ts +++ b/packages/web/src/__tests__/api-routes.test.ts @@ -393,31 +393,91 @@ describe("API Routes", () => { }); describe("GET /api/runtime/terminal", () => { + function withEnv(overrides: Record, fn: () => Promise) { + const saved: Record = {}; + for (const key of Object.keys(overrides)) { + saved[key] = process.env[key]; + if (overrides[key] === undefined) { + delete process.env[key]; + } else { + process.env[key] = overrides[key]; + } + } + return fn().finally(() => { + for (const key of Object.keys(saved)) { + if (saved[key] === undefined) { + delete process.env[key]; + } else { + process.env[key] = saved[key]; + } + } + }); + } + it("returns runtime direct terminal port from server env", async () => { - const previousDirect = process.env.DIRECT_TERMINAL_PORT; - const previousTerminal = process.env.TERMINAL_PORT; - - process.env.DIRECT_TERMINAL_PORT = "14803"; - process.env.TERMINAL_PORT = "14802"; - - try { + await withEnv({ DIRECT_TERMINAL_PORT: "14803", TERMINAL_PORT: "14802" }, async () => { const res = await runtimeTerminalGET(); expect(res.status).toBe(200); const data = await res.json(); expect(data.directTerminalPort).toBe("14803"); expect(data.terminalPort).toBe("14802"); - } finally { - if (previousDirect === undefined) { - delete process.env.DIRECT_TERMINAL_PORT; - } else { - process.env.DIRECT_TERMINAL_PORT = previousDirect; - } - if (previousTerminal === undefined) { - delete process.env.TERMINAL_PORT; - } else { - process.env.TERMINAL_PORT = previousTerminal; - } - } + }); + }); + + it("falls back to default ports when env vars are absent", async () => { + await withEnv({ DIRECT_TERMINAL_PORT: undefined, TERMINAL_PORT: undefined }, async () => { + const res = await runtimeTerminalGET(); + expect(res.status).toBe(200); + const data = await res.json(); + expect(data.directTerminalPort).toBe("14801"); + expect(data.terminalPort).toBe("14800"); + }); + }); + + it("falls back to default ports for non-numeric env values", async () => { + await withEnv({ DIRECT_TERMINAL_PORT: "abc", TERMINAL_PORT: "not-a-port" }, async () => { + const res = await runtimeTerminalGET(); + expect(res.status).toBe(200); + const data = await res.json(); + expect(data.directTerminalPort).toBe("14801"); + expect(data.terminalPort).toBe("14800"); + }); + }); + + it("falls back to default ports for out-of-range port values", async () => { + await withEnv({ DIRECT_TERMINAL_PORT: "99999", TERMINAL_PORT: "0" }, async () => { + const res = await runtimeTerminalGET(); + expect(res.status).toBe(200); + const data = await res.json(); + expect(data.directTerminalPort).toBe("14801"); + expect(data.terminalPort).toBe("14800"); + }); + }); + + it("returns null proxyWsPath when TERMINAL_WS_PATH is absent", async () => { + await withEnv( + { TERMINAL_WS_PATH: undefined, NEXT_PUBLIC_TERMINAL_WS_PATH: undefined }, + async () => { + const res = await runtimeTerminalGET(); + expect(res.status).toBe(200); + const data = await res.json(); + expect(data.proxyWsPath).toBeNull(); + }, + ); + }); + + it("rejects proxyWsPath that does not start with /", async () => { + await withEnv({ TERMINAL_WS_PATH: "no-leading-slash" }, async () => { + const res = await runtimeTerminalGET(); + expect(res.status).toBe(200); + const data = await res.json(); + expect(data.proxyWsPath).toBeNull(); + }); + }); + + it("sets Cache-Control: no-store header", async () => { + const res = await runtimeTerminalGET(); + expect(res.headers.get("Cache-Control")).toBe("no-store"); }); }); diff --git a/packages/web/src/app/api/issues/route.ts b/packages/web/src/app/api/issues/route.ts index 607360b9d..892d3f6c8 100644 --- a/packages/web/src/app/api/issues/route.ts +++ b/packages/web/src/app/api/issues/route.ts @@ -1,6 +1,6 @@ import { type NextRequest, NextResponse } from "next/server"; import { getServices } from "@/lib/services"; -import { validateString } from "@/lib/validation"; +import { validateString, validateConfiguredProject } from "@/lib/validation"; import type { Tracker } from "@composio/ao-core"; export const dynamic = "force-dynamic"; @@ -70,10 +70,11 @@ export async function POST(request: NextRequest) { try { const { config, registry } = await getServices(); - const project = config.projects[projectId]; - if (!project) { - return NextResponse.json({ error: `Unknown project: ${projectId}` }, { status: 404 }); + const projectErr = validateConfiguredProject(config.projects, projectId); + if (projectErr) { + return NextResponse.json({ error: projectErr }, { status: 404 }); } + const project = config.projects[projectId]; if (!project.tracker) { return NextResponse.json({ error: "No tracker configured for this project" }, { status: 422 }); diff --git a/packages/web/src/app/api/orchestrators/route.ts b/packages/web/src/app/api/orchestrators/route.ts index 03dce69ec..64f5d6901 100644 --- a/packages/web/src/app/api/orchestrators/route.ts +++ b/packages/web/src/app/api/orchestrators/route.ts @@ -1,7 +1,7 @@ import { type NextRequest, NextResponse } from "next/server"; import { generateOrchestratorPrompt } from "@composio/ao-core"; import { getServices } from "@/lib/services"; -import { validateIdentifier } from "@/lib/validation"; +import { validateIdentifier, validateConfiguredProject } from "@/lib/validation"; export async function POST(request: NextRequest) { const body = (await request.json().catch(() => null)) as Record | null; @@ -17,11 +17,11 @@ export async function POST(request: NextRequest) { try { const { config, sessionManager } = await getServices(); const projectId = body.projectId as string; - const project = config.projects[projectId]; - - if (!project) { - return NextResponse.json({ error: `Unknown project: ${projectId}` }, { status: 404 }); + const projectErr = validateConfiguredProject(config.projects, projectId); + if (projectErr) { + return NextResponse.json({ error: projectErr }, { status: 404 }); } + const project = config.projects[projectId]; const systemPrompt = generateOrchestratorPrompt({ config, projectId, project }); const session = await sessionManager.spawnOrchestrator({ projectId, systemPrompt }); diff --git a/packages/web/src/app/api/spawn/route.ts b/packages/web/src/app/api/spawn/route.ts index f8692e115..0b97d1fca 100644 --- a/packages/web/src/app/api/spawn/route.ts +++ b/packages/web/src/app/api/spawn/route.ts @@ -1,5 +1,5 @@ import { type NextRequest } from "next/server"; -import { validateIdentifier } from "@/lib/validation"; +import { validateIdentifier, validateConfiguredProject } from "@/lib/validation"; import { getServices } from "@/lib/services"; import { sessionToDashboard } from "@/lib/serialize"; import { getCorrelationId, jsonWithCorrelation, recordApiObservation } from "@/lib/observability"; @@ -28,7 +28,8 @@ export async function POST(request: NextRequest) { try { const { config, sessionManager } = await getServices(); const projectId = body.projectId as string; - if (!config.projects[projectId]) { + const projectErr = validateConfiguredProject(config.projects, projectId); + if (projectErr) { recordApiObservation({ config, method: "POST", @@ -38,14 +39,10 @@ export async function POST(request: NextRequest) { outcome: "failure", statusCode: 404, projectId, - reason: `Unknown project: ${projectId}`, + reason: projectErr, data: { issueId: body.issueId }, }); - return jsonWithCorrelation( - { error: `Unknown project: ${projectId}` }, - { status: 404 }, - correlationId, - ); + return jsonWithCorrelation({ error: projectErr }, { status: 404 }, correlationId); } const session = await sessionManager.spawn({ diff --git a/packages/web/src/app/api/verify/route.ts b/packages/web/src/app/api/verify/route.ts index 5456b654f..12f059ab5 100644 --- a/packages/web/src/app/api/verify/route.ts +++ b/packages/web/src/app/api/verify/route.ts @@ -1,5 +1,6 @@ import { type NextRequest, NextResponse } from "next/server"; import { getVerifyIssues, getServices } from "@/lib/services"; +import { validateConfiguredProject } from "@/lib/validation"; import type { Tracker } from "@composio/ao-core"; export const dynamic = "force-dynamic"; @@ -48,9 +49,13 @@ export async function POST(req: NextRequest) { } const { config, registry } = await getServices(); + const projectErr = validateConfiguredProject(config.projects, projectId); + if (projectErr) { + return NextResponse.json({ error: projectErr }, { status: 404 }); + } const project = config.projects[projectId]; - if (!project?.tracker) { - return NextResponse.json({ error: `Project ${projectId} has no tracker` }, { status: 404 }); + if (!project.tracker) { + return NextResponse.json({ error: `Project ${projectId} has no tracker` }, { status: 422 }); } const tracker = registry.get("tracker", project.tracker.plugin); diff --git a/packages/web/src/components/DirectTerminal.tsx b/packages/web/src/components/DirectTerminal.tsx index a12bc6b14..90c15e65c 100644 --- a/packages/web/src/components/DirectTerminal.tsx +++ b/packages/web/src/components/DirectTerminal.tsx @@ -347,6 +347,7 @@ export function DirectTerminal({ // Runtime WS config cache. We do not rely on build-time NEXT_PUBLIC_* here // because `ao start` can choose terminal ports dynamically at runtime. const runtimeConnectionConfig: TerminalConnectionConfig = {}; + let runtimeFetchDone = false; // ── Preserve selection while terminal receives output ──────── // xterm.js clears the selection on every terminal.write(). We @@ -437,16 +438,24 @@ export function DirectTerminal({ directTerminalPort: normalizePortValue(process.env.NEXT_PUBLIC_DIRECT_TERMINAL_PORT), }; - if (!fromBuild.proxyWsPath && !runtimeConnectionConfig.directTerminalPort) { + if (!fromBuild.proxyWsPath && !runtimeFetchDone) { + runtimeFetchDone = true; + const controller = new AbortController(); + const fetchTimeout = setTimeout(() => controller.abort(), 1500); try { - const response = await fetch("/api/runtime/terminal", { cache: "no-store" }); + const response = await fetch("/api/runtime/terminal", { + cache: "no-store", + signal: controller.signal, + }); if (response.ok) { const runtimeConfig = parseRuntimeTerminalConfig(await response.json()); runtimeConnectionConfig.proxyWsPath = runtimeConfig.proxyWsPath; runtimeConnectionConfig.directTerminalPort = runtimeConfig.directTerminalPort; } } catch { - // Runtime config endpoint is optional; fallback to build-time values. + // Runtime config endpoint is optional (timeout or network failure); fallback to build-time values. + } finally { + clearTimeout(fetchTimeout); } } diff --git a/packages/web/src/lib/__tests__/dashboard-page-data.test.ts b/packages/web/src/lib/__tests__/dashboard-page-data.test.ts index 03963c46d..a817fcd47 100644 --- a/packages/web/src/lib/__tests__/dashboard-page-data.test.ts +++ b/packages/web/src/lib/__tests__/dashboard-page-data.test.ts @@ -41,4 +41,8 @@ describe("resolveDashboardProjectFilter", () => { it("falls back to primary project for unknown ids", () => { expect(resolveDashboardProjectFilter("mono-orchestrator")).toBe("mono"); }); + + it("falls back to primary project when no project is given", () => { + expect(resolveDashboardProjectFilter(undefined)).toBe("mono"); + }); }); diff --git a/packages/web/src/lib/__tests__/validation.test.ts b/packages/web/src/lib/__tests__/validation.test.ts new file mode 100644 index 000000000..14ebf4304 --- /dev/null +++ b/packages/web/src/lib/__tests__/validation.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, it } from "vitest"; +import { validateConfiguredProject } from "@/lib/validation"; + +describe("validateConfiguredProject", () => { + const projects = { "my-app": { name: "My App" }, docs: { name: "Docs" } }; + + it("returns null for a configured project", () => { + expect(validateConfiguredProject(projects, "my-app")).toBeNull(); + }); + + it("returns error for an unknown project id", () => { + expect(validateConfiguredProject(projects, "ghost")).toBe("Unknown project: ghost"); + }); + + it("rejects prototype chain keys like 'constructor'", () => { + expect(validateConfiguredProject(projects, "constructor")).toBe( + "Unknown project: constructor", + ); + }); + + it("rejects prototype chain keys like 'toString'", () => { + expect(validateConfiguredProject(projects, "toString")).toBe("Unknown project: toString"); + }); + + it("rejects '__proto__'", () => { + expect(validateConfiguredProject(projects, "__proto__")).toBe("Unknown project: __proto__"); + }); +}); diff --git a/packages/web/src/lib/dashboard-page-data.ts b/packages/web/src/lib/dashboard-page-data.ts index 04c22b76f..830b6e168 100644 --- a/packages/web/src/lib/dashboard-page-data.ts +++ b/packages/web/src/lib/dashboard-page-data.ts @@ -40,7 +40,7 @@ export function resolveDashboardProjectFilter(project?: string): string { if (project && projects.some((entry) => entry.id === project)) { return project; } - return projects[0]?.id ?? getPrimaryProjectId(); + return getPrimaryProjectId(); } export const getDashboardPageData = cache(async function getDashboardPageData(project?: string): Promise { diff --git a/packages/web/src/lib/validation.ts b/packages/web/src/lib/validation.ts index 761413859..c354eb00a 100644 --- a/packages/web/src/lib/validation.ts +++ b/packages/web/src/lib/validation.ts @@ -33,6 +33,20 @@ export function validateIdentifier( return null; } +/** + * Validate that a projectId is a configured project (own property, not prototype chain). + * Returns an error message or null. + */ +export function validateConfiguredProject( + projects: Record, + projectId: string, +): string | null { + if (!Object.hasOwn(projects, projectId)) { + return `Unknown project: ${projectId}`; + } + return null; +} + /** * Strip control characters (U+0000–U+001F, U+007F–U+009F) from a string. * Critical for messages that may be passed to shell-based runtimes (tmux send-keys, etc.)