diff --git a/CHANGELOG.md b/CHANGELOG.md index a28d4873..165d710f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,14 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### Wiping a computer is recorded even if clearing its stored state fails + +Resetting a computer destroys the profile first and wrote the audit row last, after two Postgres +deletes. A connection reset, a failover or a statement timeout in either delete threw before the row +was written, so the most destructive button in the product could leave a wiped computer -- every +login gone, no undo -- with nothing on the trail to say who wiped it or when. The row is now written +as soon as the profile is gone, which is the point after which nothing can be put back. A failure in +either delete is still reported to the caller. ### Pressing Stop is recorded as a stop, not as a computer that is not running The computer transport answered a Stop correctly only when it arrived before the request left. The diff --git a/server/src/computer/gateway.ts b/server/src/computer/gateway.ts index 21c7d985..99f0cf09 100644 --- a/server/src/computer/gateway.ts +++ b/server/src/computer/gateway.ts @@ -713,6 +713,22 @@ export function createComputerGateway( */ async resetComputer(botId: string, actor: ActionActor) { const result = await provider.reset(botId); + /* + * The row goes in HERE, before the two deletes below, because this line is the point of no + * return: the profile is already gone and nothing after it can put the logins back. + * + * Both clears are Postgres deletes, and a connection reset, a failover or a statement timeout + * in either used to throw before the row was written -- leaving a computer wiped with nothing + * on the trail to say who wiped it, which is the one outcome the note above rules out. The + * failure still propagates, so the caller is told the clears did not finish. + */ + await writeControlEvent(auditStore, "computer.reset", { + botId, + actor, + reason: result.cleared + ? "the computer and its saved state were deleted" + : "no saved state was present to delete", + }); // The refs the last snapshot handed out describe a page that no longer exists, and a fresh // computer counts generations from one again, so the row has to go with the profile. await snapshots.clear(botId); @@ -725,13 +741,6 @@ export function createComputerGateway( * anything a person would recognise as private. */ await pageFrames?.clear(botId); - await writeControlEvent(auditStore, "computer.reset", { - botId, - actor, - reason: result.cleared - ? "the computer and its saved state were deleted" - : "no saved state was present to delete", - }); return result; }, diff --git a/server/tests/computer-gateway.test.ts b/server/tests/computer-gateway.test.ts index e2b4c255..75518a69 100644 --- a/server/tests/computer-gateway.test.ts +++ b/server/tests/computer-gateway.test.ts @@ -687,6 +687,67 @@ describe("the computer gateway", () => { expect(rows[0]?.targetId).toBe("bot-2"); }); + /* + * "The most destructive button we have. Every login the Bot had is gone and no undo exists, so the + * row is written whatever happens next." + * + * The profile is destroyed by `provider.reset` before either of the two Postgres deletes runs, so + * a delete that fails cannot put it back -- it can only take the row with it, which is the one + * thing that note rules out. + */ + test("resetComputer records the reset even when clearing the stored page fails", async () => { + const { provider, fetchImpl } = fakeComputer({ + resetResult: { cleared: true }, + }); + const { store, rows } = fakeAudit(); + const snapshots: SnapshotStore = { + ...createInMemorySnapshotStore(), + clear: async () => { + throw new Error("connection reset by peer"); + }, + }; + const gateway = createComputerGateway({ + provider, + fetchImpl, + auditStore: store, + policy: () => PERMISSIVE, + snapshots, + }); + + await expect(gateway.resetComputer("bot-1", ACTOR)).rejects.toThrow( + "connection reset by peer", + ); + + expect(rows.map((row) => row.eventType)).toContain("computer.reset"); + expect(rows[0]?.targetId).toBe("bot-1"); + }); + + test("resetComputer records the reset even when clearing the screenshots fails", async () => { + const { provider, fetchImpl } = fakeComputer({ + resetResult: { cleared: true }, + }); + const { store, rows } = fakeAudit(); + const gateway = createComputerGateway({ + provider, + fetchImpl, + auditStore: store, + policy: () => PERMISSIVE, + pageFrames: { + clear: async () => { + throw new Error("statement timeout"); + }, + } as unknown as NonNullable< + Parameters[0]["pageFrames"] + >, + }); + + await expect(gateway.resetComputer("bot-1", ACTOR)).rejects.toThrow( + "statement timeout", + ); + + expect(rows.map((row) => row.eventType)).toContain("computer.reset"); + }); + test("computers maps provider status 'running' and 'stopped' directly and preserves egress distinctions", async () => { const locations: ComputerLocation[] = [ {