diff --git a/server/src/__tests__/computer-routes.test.ts b/server/src/__tests__/computer-routes.test.ts index 9fcc6c256c0..b46d3583d03 100644 --- a/server/src/__tests__/computer-routes.test.ts +++ b/server/src/__tests__/computer-routes.test.ts @@ -195,6 +195,51 @@ beforeEach(() => { }); describe("Computer routes", () => { + it.each(["changed", "removed"])("disconnects the previous viewer when the task computer has %s", async state => { + const f = fixture(); + const nextEnvironmentId = "77777777-7777-4777-8777-777777777777"; + f.rows.set(environmentLeases, state === "changed" ? [{ ...f.lease, environmentId: nextEnvironmentId }] : []); + f.rows.set(environments, state === "changed" ? [{ ...f.environment, id: nextEnvironmentId }] : []); + const response = await request(f.app).post(`${endpoint}/disconnect`).send({ environmentId, owner }); + expect(response.status).toBe(204); + expect(f.select).toHaveBeenCalledTimes(1); // Task authorization precedes cleanup; no current-computer lookup. + expect(f.computers.disconnectViewer).toHaveBeenCalledExactlyOnceWith({ companyId, environmentId, owner, userId: "alice" }); + expect(mocks.audit).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ + action: "computer.disconnected", companyId, entityId: environmentId, + })); + for (const action of ["connect", "presence", "preview"]) { + const input = action === "connect" ? { environmentId } + : action === "preview" ? { environmentId, port: 5173 } : { environmentId, owner }; + expect((await request(f.app).post(`${endpoint}/${action}`).send(input)).status).toBe(404); + } + expect(f.computers.connect).not.toHaveBeenCalled(); + expect(f.computers.renewViewer).not.toHaveBeenCalled(); + expect(f.computers.preview).not.toHaveBeenCalled(); + }); + + it.each(["not_found", "forbidden", "conflict"] as const)("preserves scoped disconnect ownership failures (%s)", async code => { + const f = fixture(); + f.rows.set(environments, []); + f.computers.disconnectViewer.mockRejectedValue(new ComputerError(code, "Viewer unavailable")); + const response = await request(f.app).post(`${endpoint}/disconnect`).send({ environmentId, owner }); + expect(response.status).toBe(code === "not_found" ? 404 : code === "forbidden" ? 403 : 409); + expect(f.computers.disconnectViewer).toHaveBeenCalledExactlyOnceWith({ companyId, environmentId, owner, userId: "alice" }); + expect(mocks.audit).not.toHaveBeenCalled(); + }); + + it("authorizes the task and validates the exact owner before requesting stale-viewer cleanup", async () => { + const foreign = fixture({ issueCompanyId: otherCompanyId }); + expect((await request(foreign.app).post(`${endpoint}/disconnect`).send({ environmentId, owner })).status).toBe(404); + expect(foreign.computers.disconnectViewer).not.toHaveBeenCalled(); + const agent = fixture({ actor: "agent" }); + expect((await request(agent.app).post(`${endpoint}/disconnect`).send({ environmentId, owner })).status).toBe(403); + expect(agent.computers.disconnectViewer).not.toHaveBeenCalled(); + const board = fixture(); + expect((await request(board.app).post(`${endpoint}/disconnect`).send({ environmentId, owner: { ...owner, generation: 0 } })).status).toBe(400); + expect(board.computers.disconnectViewer).not.toHaveBeenCalled(); + expect(mocks.audit).not.toHaveBeenCalled(); + }); + it.each([true, false])("uses the latest admission despite an older lease's later finalization (latest live=%s)", async latestLive => { const f = fixture(); const older = { ...f.lease, id: "older", createdAt: "2026-10-10T20:00:00Z", updatedAt: "2026-10-10T20:12:00Z" }; diff --git a/server/src/modules/computers/application/service.test.ts b/server/src/modules/computers/application/service.test.ts index d7211d9a7ba..a576befd684 100644 --- a/server/src/modules/computers/application/service.test.ts +++ b/server/src/modules/computers/application/service.test.ts @@ -298,6 +298,30 @@ describe("computer ownership", () => { await f.service.reconcile(); expect(f.backend.stop).toHaveBeenCalledOnce(); }); + it("disconnects only the exact company-scoped viewer owned by the current user without provider access", async () => { + const f = fixture(); + await f.attach(); + const viewer = await f.service.connect({ ...f.scope, userId: "alice", idleTimeoutMs: 60_000 }); + const other = await f.service.connect({ ...f.scope, userId: "bob", idleTimeoutMs: 60_000 }); + const runner = await f.admit(); + vi.clearAllMocks(); + const input = { ...f.scope, owner: viewer.owner, userId: "alice" }; + await expect(f.service.disconnectViewer({ ...input, companyId: "foreign-company" })).rejects.toMatchObject({ code: "not_found" }); + await expect(f.service.disconnectViewer({ ...input, environmentId: "foreign-environment" })).rejects.toMatchObject({ code: "not_found" }); + await expect(f.service.disconnectViewer({ ...input, userId: "bob" })).rejects.toMatchObject({ code: "forbidden" }); + await expect(f.service.disconnectViewer({ ...input, owner: { ...viewer.owner, generation: viewer.owner.generation + 1 } })).rejects.toMatchObject({ code: "conflict" }); + await expect(f.service.disconnectViewer({ ...input, owner: { ...viewer.owner, computerId: "another-computer" } })).rejects.toMatchObject({ code: "conflict" }); + await expect(f.service.disconnectViewer({ ...input, owner: runner.owner })).rejects.toMatchObject({ code: "forbidden" }); + expect((await f.repository.get(f.scope)).ledger.owners.every(owner => owner.phase === "active")).toBe(true); + await f.service.disconnectViewer(input); + const owners = (await f.repository.get(f.scope)).ledger.owners; + expect(owners.find(owner => owner.id === viewer.owner.ownerId)?.phase).toBe("retired"); + expect(owners.find(owner => owner.id === other.owner.ownerId)?.phase).toBe("active"); + expect(owners.find(owner => owner.id === runner.owner.ownerId)?.phase).toBe("active"); + for (const method of [f.backend.inspect, f.backend.ready, f.backend.retire, f.backend.stop, f.backend.renew, f.backend.desktop]) { + expect(method).not.toHaveBeenCalled(); + } + }); it("prunes retired viewers while preserving active viewers and runner stop proof", async () => { const f = fixture(); await f.attach(); diff --git a/server/src/routes/computers.ts b/server/src/routes/computers.ts index 591db369f0e..e00115136e6 100644 --- a/server/src/routes/computers.ts +++ b/server/src/routes/computers.ts @@ -28,6 +28,13 @@ export function computerRoutes(db: Db, computers = computerService(db)) { if (!issue) return; res.setHeader("Cache-Control", "no-store"); res.setHeader("Referrer-Policy", "no-referrer"); + if (req.method === "POST" && req.path === "/disconnect") { + // A task may have moved since this viewer connected. Cleanup is scoped + // by the module to the posted environment, exact owner, and current user. + res.locals.computer = { issue }; + next(); + return; + } if (!(await settings.getExperimental()).enableBoatEnvironments && !req.path.endsWith("/disconnect")) { if (req.method === "GET") { res.json(null); return; } throw unprocessable("Boat environments are disabled."); @@ -76,13 +83,12 @@ export function computerRoutes(db: Db, computers = computerService(db)) { router.post("/issues/:issueId/computer/disconnect", async (req, res) => { const input = presenceSchema.parse(req.body); - const { environment, issue } = res.locals.computer; - if (input.environmentId !== environment.id) throw notFound("Computer not available for this task"); + const { issue } = res.locals.computer; const actor = getActorInfo(req); - await computers.disconnectViewer({ companyId: issue.companyId, environmentId: environment.id, + await computers.disconnectViewer({ companyId: issue.companyId, environmentId: input.environmentId, owner: input.owner, userId: actor.actorId }); await logActivity(db, { companyId: issue.companyId, actorType: actor.actorType, actorId: actor.actorId, - action: "computer.disconnected", entityType: "environment", entityId: environment.id, details: { issueId: issue.id } }); + action: "computer.disconnected", entityType: "environment", entityId: input.environmentId, details: { issueId: issue.id } }); res.status(204).end(); });