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 <noreply@anthropic.com>
This commit is contained in:
parent
5315e4e0ab
commit
717253cf8e
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -393,31 +393,91 @@ describe("API Routes", () => {
|
|||
});
|
||||
|
||||
describe("GET /api/runtime/terminal", () => {
|
||||
function withEnv(overrides: Record<string, string | undefined>, fn: () => Promise<void>) {
|
||||
const saved: Record<string, string | undefined> = {};
|
||||
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");
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -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 });
|
||||
|
|
|
|||
|
|
@ -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<string, unknown> | 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 });
|
||||
|
|
|
|||
|
|
@ -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({
|
||||
|
|
|
|||
|
|
@ -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>("tracker", project.tracker.plugin);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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__");
|
||||
});
|
||||
});
|
||||
|
|
@ -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<DashboardPageData> {
|
||||
|
|
|
|||
|
|
@ -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<string, unknown>,
|
||||
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.)
|
||||
|
|
|
|||
Loading…
Reference in New Issue