mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-22 00:30:19 +02:00
fix(computers): release previous viewers after task reassignment
Authorize exact user-owned viewer cleanup independently of the current task environment, with route and module regressions. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
3673939bed
commit
6337156885
3 files changed
+79
-4
No files matched your search
@@ -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" };
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
|
||||
|
||||
Reference in new issue
Block a user