diff --git a/src/server/computer-routes.ts b/src/server/computer-routes.ts index a7d68249..aa94bd4c 100644 --- a/src/server/computer-routes.ts +++ b/src/server/computer-routes.ts @@ -4,17 +4,27 @@ import type { ComputerService } from './computer-service.js'; import type { ComputerAction } from '../shared/computer-types.js'; export function computerRoutes(computers: ComputerService) { const app = new Hono(); - app.onError((error, c) => - c.json( - { - error: - error instanceof z.ZodError - ? 'Invalid computer request.' - : error.message, - }, - 400, - ), - ); + app.onError((error, c) => { + if (error instanceof SyntaxError) + return c.json({ error: 'Invalid JSON request.' }, 400); + if (error instanceof z.ZodError) + return c.json({ error: 'Invalid computer request.' }, 400); + const text = error.message; + if (text === 'Dot not found.') return c.json({ error: text }, 404); + if ( + text === 'Computer permission is disabled.' || + text === 'Human controls are owner-only.' + ) + return c.json({ error: text }, 403); + if ( + text === 'Start this Dot’s computer first.' || + text === 'There is no active control request.' + ) + return c.json({ error: text }, 409); + if (text === 'Unknown computer action.') + return c.json({ error: text }, 400); + return c.json({ error: text }, 503); + }); app.get('/dots/:id/computer', async (c) => c.json(await computers.status(c.req.param('id'))), ); diff --git a/src/server/computer-service.ts b/src/server/computer-service.ts index 2e66de2f..84b72b4c 100644 --- a/src/server/computer-service.ts +++ b/src/server/computer-service.ts @@ -141,8 +141,16 @@ export class ComputerService { signal, ); } + private parseUpstream(schema: z.ZodType, raw: unknown): T { + const result = schema.safeParse(raw); + if (!result.success) + throw new Error('Computer service returned an invalid response.', { + cause: result.error, + }); + return result.data; + } private endpoint(id: string, raw: unknown) { - const state = stateSchema.parse(raw); + const state = this.parseUpstream(stateSchema, raw); const ns = this.config.computerNamespace ?? 'opendots'; if (!/^[A-Za-z0-9][A-Za-z0-9_-]{0,63}$/.test(ns)) throw new Error('Invalid computer namespace.'); @@ -175,9 +183,10 @@ export class ComputerService { return url.origin; } private async existing(id: string, signal?: AbortSignal) { - const listing = z - .object({ computers: z.array(stateSchema) }) - .parse(await this.supervisor('/computers', undefined, signal)); + const listing = this.parseUpstream( + z.object({ computers: z.array(stateSchema) }), + await this.supervisor('/computers', undefined, signal), + ); return listing.computers.find((c) => c.botId === id); } private async running(id: string, signal?: AbortSignal) { @@ -264,7 +273,8 @@ export class ComputerService { if (verb === 'take') this.allowed(id, 'browser', 'owner'); const url = await this.running(id); if (verb === 'take') this.allowed(id, 'browser', 'owner'); - let control: ComputerControl = controlSchema.parse( + let control: ComputerControl = this.parseUpstream( + controlSchema, await this.json( `${url}/control`, this.token(id), @@ -274,7 +284,8 @@ export class ComputerService { ), ); if (verb === 'take' && !control.request) - control = controlSchema.parse( + control = this.parseUpstream( + controlSchema, await this.json( `${url}/control/request`, this.token(id), @@ -288,7 +299,8 @@ export class ComputerService { control.request && !['waiting', 'taken'].includes(control.request.status) ) - control = controlSchema.parse( + control = this.parseUpstream( + controlSchema, await this.json( `${url}/control/request`, this.token(id), diff --git a/tests/computer-routes.test.ts b/tests/computer-routes.test.ts new file mode 100644 index 00000000..44216147 --- /dev/null +++ b/tests/computer-routes.test.ts @@ -0,0 +1,180 @@ +import { afterEach, expect, it } from 'vitest'; +import { WorkspaceStore } from '../src/server/workspace.js'; +import { ComputerService } from '../src/server/computer-service.js'; +import { computerRoutes } from '../src/server/computer-routes.js'; + +const stores: WorkspaceStore[] = []; +afterEach(() => { + for (const store of stores.splice(0)) store.close(); +}); + +function fixture() { + const workspace = new WorkspaceStore(':memory:', 'owner'); + stores.push(workspace); + const id = workspace.dots()[0].id; + const config = { + baseUrl: 'https://example.com', + voiceName: 'voice', + slackUsers: [], + runtimeUrl: 'http://localhost', + }; + // No computer supervisor configured: `configured` is false, so any + // action that reaches `allowed()` fails with a plain domain error + // rather than attempting a network call. + const service = new ComputerService(workspace, config, () => false); + const app = computerRoutes(service); + return { workspace, id, app }; +} + +// `configured: true`, so `allowed()` reaches its permission check instead of +// failing earlier with "Computer service is not configured."; `transport` is +// never called because `allowed()` throws before any network request. +function configuredFixture() { + const workspace = new WorkspaceStore(':memory:', 'owner'); + stores.push(workspace); + const id = workspace.dots()[0].id; + const config = { + baseUrl: 'https://example.com', + voiceName: 'voice', + slackUsers: [], + runtimeUrl: 'http://localhost', + computerSupervisorUrl: 'https://supervisor.example.com', + computerSupervisorToken: 'supervisor-token', + computerToken: 'computer-token', + }; + const transport = () => { + throw new Error('transport should not be called in this test'); + }; + const service = new ComputerService( + workspace, + config, + () => false, + transport, + ); + const app = computerRoutes(service); + return { workspace, id, app }; +} + +it('answers malformed JSON on the actions route with 400, not a service error', async () => { + const { app, id } = fixture(); + const response = await app.request(`/dots/${id}/computer/actions`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: '{', + }); + expect(response.status).toBe(400); + expect(await response.json()).toEqual({ error: 'Invalid JSON request.' }); +}); + +it('answers an invalid action shape with 400 from the Zod schema', async () => { + const { app, id } = fixture(); + const response = await app.request(`/dots/${id}/computer/actions`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ input: {} }), + }); + expect(response.status).toBe(400); + expect(await response.json()).toEqual({ error: 'Invalid computer request.' }); +}); + +it('answers an unconfigured computer service with 503, not 400', async () => { + const { app, id } = fixture(); + const response = await app.request(`/dots/${id}/computer/start`, { + method: 'POST', + }); + expect(response.status).toBe(503); + expect(await response.json()).toEqual({ + error: 'Computer service is not configured.', + }); +}); + +it('answers an unknown Dot id with 404, not a service error', async () => { + const { app } = fixture(); + const response = await app.request('/dots/missing/computer/start', { + method: 'POST', + }); + expect(response.status).toBe(404); + expect(await response.json()).toEqual({ error: 'Dot not found.' }); +}); + +it('answers an unknown computer action with 400, not a service error', async () => { + const { app, id } = fixture(); + const response = await app.request(`/dots/${id}/computer/actions`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ action: 'not_a_real_action', input: {} }), + }); + expect(response.status).toBe(400); + expect(await response.json()).toEqual({ error: 'Unknown computer action.' }); +}); + +it('answers a disabled computer permission with 403, not a service error', async () => { + const { app, id, workspace } = configuredFixture(); + workspace.computers.patch(id, { enabled: false }); + const response = await app.request(`/dots/${id}/computer/take`, { + method: 'POST', + }); + expect(response.status).toBe(403); + expect(await response.json()).toEqual({ + error: 'Computer permission is disabled.', + }); +}); + +// `stateSchema`/`controlSchema` also validate the supervisor's own response, +// so a malformed upstream body must not be mistaken for the caller's fault: +// it is a service failure (503), not an invalid request (400). +function upstreamFixture(responses: Record) { + const workspace = new WorkspaceStore(':memory:', 'owner'); + stores.push(workspace); + const id = workspace.dots()[0].id; + workspace.computers.patch(id, { enabled: true, browser: true }); + const config = { + baseUrl: 'https://example.com', + voiceName: 'voice', + slackUsers: [], + runtimeUrl: 'http://localhost', + computerSupervisorUrl: 'https://supervisor.example.com', + computerSupervisorToken: 'supervisor-token', + computerToken: 'computer-token', + }; + const transport = async (url: RequestInfo | URL) => { + for (const [suffix, body] of Object.entries(responses)) + if (String(url).endsWith(suffix)) + return new Response(JSON.stringify(body), { status: 200 }); + throw new Error(`transport should not be called for ${url}`); + }; + const service = new ComputerService( + workspace, + config, + () => false, + transport, + ); + const app = computerRoutes(service); + return { id, app }; +} + +it('answers a malformed supervisor ensure response with 503, not a caller validation error', async () => { + const { app, id } = upstreamFixture({ '/ensure': {} }); + const response = await app.request(`/dots/${id}/computer/start`, { + method: 'POST', + }); + expect(response.status).toBe(503); + expect(await response.json()).toEqual({ + error: 'Computer service returned an invalid response.', + }); +}); + +it('answers a malformed supervisor listing response with 503, not a caller validation error', async () => { + const { app, id } = upstreamFixture({ + '/computers': { computers: [{ botId: 'placeholder' }] }, + }); + const response = await app.request(`/dots/${id}/computer/actions`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ action: 'read', input: {} }), + }); + expect(response.status).toBe(503); + expect(await response.json()).toEqual({ + error: 'Computer service returned an invalid response.', + }); +});