diff --git a/src/app/api/admin/backups/__tests__/route.test.ts b/src/app/api/admin/backups/__tests__/route.test.ts index e182e8e17..70b583451 100644 --- a/src/app/api/admin/backups/__tests__/route.test.ts +++ b/src/app/api/admin/backups/__tests__/route.test.ts @@ -8,7 +8,7 @@ * * The bucket is never touched here, and that is the assertion that matters — * the verdicts come from the ledger the worker writes, so an operator whose - * worker holds PutObject and nothing else still gets a true answer. + * worker holds no listing grant on the bucket still gets a true answer. */ import { beforeEach, describe, expect, it, vi } from "vitest"; @@ -107,28 +107,35 @@ describe("GET /api/admin/backups — off-host freshness", () => { expect(body.offhost.configured).toBe(true); expect(body.offhost.periodHours).toBe(24); + // Worst first, alphabetical within a verdict. The database hands them + // back by username; the one account that needs attention must not be + // wherever the alphabet leaves it on a hundred-account cohort. expect( body.offhost.rows.map((row) => [row.username, row.freshness]), ).toEqual([ - ["account-one", "fresh"], - ["account-two", "due"], ["account-three", "stale"], + ["account-two", "due"], ["account-four", "never"], ["account-five", "unknown"], + ["account-one", "fresh"], ]); - expect(body.offhost.rows[0]?.sizeBytes).toBe(4096); - expect(typeof body.offhost.rows[0]?.sizeBytes).toBe("number"); - expect(body.offhost.rows[0]?.ageHours).toBe(3); - expect(body.offhost.rows[3]?.lastSuccessAt).toBeNull(); - expect(body.offhost.rows[3]?.sizeBytes).toBeNull(); - expect(body.offhost.rows[3]?.lastAttemptAt).not.toBeNull(); + const byName = new Map(body.offhost.rows.map((row) => [row.username, row])); + expect(byName.get("account-one")?.sizeBytes).toBe(4096); + expect(typeof byName.get("account-one")?.sizeBytes).toBe("number"); + expect(byName.get("account-one")?.ageHours).toBe(3); + expect(byName.get("account-four")?.lastSuccessAt).toBeNull(); + expect(byName.get("account-four")?.sizeBytes).toBeNull(); + expect(byName.get("account-four")?.lastAttemptAt).not.toBeNull(); // `unknown` is the honest one: the ledger has no history for this // account, which is not the same claim as "the bucket is empty". - expect(body.offhost.rows[4]?.lastAttemptAt).toBeNull(); + expect(byName.get("account-five")?.lastAttemptAt).toBeNull(); }); - it("says so and reads no account when off-host backup is not configured", async () => { + it("says so when neither the environment nor the ledger knows of any upload", async () => { configuredMock.mockReturnValue(false); + findManyMock.mockResolvedValue([ + { id: "u1", username: "account-one", offhostBackupState: null }, + ]); const body = await read(); @@ -137,9 +144,30 @@ describe("GET /api/admin/backups — off-host freshness", () => { periodHours: 24, rows: [], }); - // Not merely empty output — the query never runs, so an unconfigured host - // does not pay for a table scan to be told nothing is set up. - expect(findManyMock).not.toHaveBeenCalled(); + }); + + it("lets a ledger row outrank this process's own environment", async () => { + // A hand-rolled split deployment can give BACKUP_S3_* to the worker and + // not to the web process. The rows are proof uploads are happening; + // saying "nothing leaves this host" over them would be this process + // reporting on a job it cannot see. + configuredMock.mockReturnValue(false); + findManyMock.mockResolvedValue([ + { + id: "u1", + username: "account-one", + offhostBackupState: { + lastAttemptAt: new Date(), + lastSuccessAt: new Date(), + sizeBytes: BigInt(4096), + }, + }, + ]); + + const body = await read(); + + expect(body.offhost.configured).toBe(true); + expect(body.offhost.rows.map((row) => row.freshness)).toEqual(["fresh"]); }); it("never ships a credential or an object key with the verdicts", async () => { diff --git a/src/app/api/admin/backups/route.ts b/src/app/api/admin/backups/route.ts index b8f38b14e..3aab2ff45 100644 --- a/src/app/api/admin/backups/route.ts +++ b/src/app/api/admin/backups/route.ts @@ -18,7 +18,7 @@ * host last put that account's encrypted copy in the operator's bucket and how * stale that is against the nightly schedule. It comes from the ledger the * worker writes, never from a bucket listing — the answer has to hold on a host - * whose worker grant is PutObject and nothing else. + * whose worker holds no listing grant on the bucket. */ import { prisma } from "@/lib/db"; import { apiHandler, requireAdmin } from "@/lib/api-handler"; @@ -31,6 +31,7 @@ import { offhostBackupConfigured } from "@/lib/jobs/offhost-backup"; import { classifyOffhostBackup, OFFHOST_BACKUP_PERIOD_HOURS, + type OffhostBackupFreshness, } from "@/lib/jobs/offhost-backup-freshness"; // v1.4.41 W-ORG — `BackupRow` / `BackupsList` moved to `src/types/backups.ts` // so callers (in particular `components/admin/backups-section.tsx`) don't @@ -84,27 +85,35 @@ export const GET = apiHandler(async () => { // The off-host leg, per account, read from the ledger the nightly worker // writes rather than from the bucket. The page must be able to answer "which - // account has no recent copy off-host" on a host whose worker grant is - // PutObject and nothing else, and a listing call from a page render would - // put the operator's credentials on the request path of every page view. + // account has no recent copy off-host" on a host whose worker holds the + // three grants the runbook documents and no listing grant at all, and a + // listing call from a page render would put the operator's credentials on + // the request path of every page view. const now = new Date(); - const configured = offhostBackupConfigured(); - const offhostAccounts = configured - ? await prisma.user.findMany({ + const offhostAccounts = await prisma.user.findMany({ + select: { + id: true, + username: true, + offhostBackupState: { select: { - id: true, - username: true, - offhostBackupState: { - select: { - lastAttemptAt: true, - lastSuccessAt: true, - sizeBytes: true, - }, - }, + lastAttemptAt: true, + lastSuccessAt: true, + sizeBytes: true, }, - orderBy: { username: "asc" }, - }) - : []; + }, + }, + orderBy: { username: "asc" }, + }); + + // Evidence beats this process's environment. `offhostBackupConfigured()` + // reads the variables of whichever process serves this request, and a + // hand-rolled split that gives `BACKUP_S3_*` only to the worker would make + // the web process say "nothing leaves this host" over a ledger full of + // fresh rows. A row in that table is proof an upload happened; no + // environment read can outrank it. + const configured = + offhostBackupConfigured() || + offhostAccounts.some((account) => account.offhostBackupState !== null); const offhostRows: OffhostAccountRow[] = offhostAccounts.map((account) => { const state = account.offhostBackupState; @@ -130,6 +139,23 @@ export const GET = apiHandler(async () => { }; }); + // Worst first. Sorting by username puts the one account that needs + // attention wherever the alphabet happens to leave it, which on a + // hundred-account cohort is the same "one bad row hides in the crowd" + // problem this card was built to end, moved out of a count and into a list. + const FRESHNESS_ORDER: Record = { + stale: 0, + due: 1, + never: 2, + unknown: 3, + fresh: 4, + }; + offhostRows.sort( + (a, b) => + FRESHNESS_ORDER[a.freshness] - FRESHNESS_ORDER[b.freshness] || + a.username.localeCompare(b.username), + ); + const payload: BackupsList = { rows: list, // Matches the retention window the backup-prune job enforces so the @@ -145,7 +171,9 @@ export const GET = apiHandler(async () => { offhost: { configured, periodHours: OFFHOST_BACKUP_PERIOD_HOURS, - rows: offhostRows, + // An unconfigured host has nothing to list, and the card says so rather + // than painting an empty table that reads as "no problems". + rows: configured ? offhostRows : [], }, }; diff --git a/src/components/admin/backups-section.tsx b/src/components/admin/backups-section.tsx index d776da699..fb065f101 100644 --- a/src/components/admin/backups-section.tsx +++ b/src/components/admin/backups-section.tsx @@ -47,6 +47,7 @@ import { SettingsCard } from "@/components/settings/settings-card"; import { SettingsCardHeader } from "@/components/settings/_card-header"; import { ListRow } from "@/components/ui/list-row"; import { useFormatters, useTranslations } from "@/lib/i18n/context"; +import { formatBytes } from "@/lib/format/bytes"; import { queryKeys } from "@/lib/query-keys"; import { randomId } from "@/lib/random-id"; import type { BackupRow, BackupsList } from "@/types/backups"; @@ -322,21 +323,6 @@ function RestoreRowDialog({ ); } -/** - * Exported for the off-host card, which reads the same sizes out of the same - * response and must not spell "1.25 MB" a second way two cards apart. - */ -export function formatBytes( - bytes: number, - fmt: ReturnType, -) { - if (bytes < 1024) return `${bytes} B`; - if (bytes < 1024 * 1024) { - return `${fmt.integer(Math.round(bytes / 1024))} KB`; - } - return `${fmt.number(bytes / 1024 / 1024, 2)} MB`; -} - /** * Render the `DataBackup.type` enum (`WEEKLY_AUTO` / `MANUAL`) as a * human label. Unknown values fall through to the raw enum so a future diff --git a/src/components/admin/offhost-backups-section.tsx b/src/components/admin/offhost-backups-section.tsx index cf3de6837..88e8cda76 100644 --- a/src/components/admin/offhost-backups-section.tsx +++ b/src/components/admin/offhost-backups-section.tsx @@ -11,9 +11,11 @@ * been refusing the worker's signature since March. * * Every verdict is computed server-side from the ledger the worker writes. The - * page never lists the bucket: the worker's grant is deliberately PutObject + - * GetObject, and putting a credentialed listing call on the render path of an - * admin page would be a poor trade for a number the database already holds. + * page never lists the bucket: the worker's grant is deliberately PutObject, + * GetObject and AbortMultipartUpload — the three the runbook documents, the + * abort being what the multipart path needs — and putting a credentialed + * listing call on the render path of an admin page would be a poor trade for + * a number the database already holds. */ import { useQuery } from "@tanstack/react-query"; @@ -31,7 +33,7 @@ import { useFormatters, useTranslations } from "@/lib/i18n/context"; import { queryKeys } from "@/lib/query-keys"; import type { OffhostBackupFreshness } from "@/lib/jobs/offhost-backup-freshness"; import type { BackupsList, OffhostAccountRow } from "@/types/backups"; -import { formatBytes } from "./backups-section"; +import { formatBytes } from "@/lib/format/bytes"; /** * Status wash per verdict. `never` and `unknown` are deliberately neutral diff --git a/src/lib/format/bytes.ts b/src/lib/format/bytes.ts new file mode 100644 index 000000000..c59732ce2 --- /dev/null +++ b/src/lib/format/bytes.ts @@ -0,0 +1,22 @@ +import type { useFormatters } from "@/lib/i18n/context"; + +/** + * One spelling of "1.25 MB" for every surface that shows a byte count from + * the admin backups response. + * + * It lives here rather than beside the card that happened to need it first: + * the second reader was importing a formatter out of a thousand-line client + * component, which is a dependency on a page's worth of code for four lines of + * arithmetic. Both cards render on the same route today, so it costs nothing + * today — and it becomes a cost the moment either one moves. + */ +export function formatBytes( + bytes: number, + fmt: ReturnType, +): string { + if (bytes < 1024) return `${bytes} B`; + if (bytes < 1024 * 1024) { + return `${fmt.integer(Math.round(bytes / 1024))} KB`; + } + return `${fmt.number(bytes / 1024 / 1024, 2)} MB`; +} diff --git a/src/types/backups.ts b/src/types/backups.ts index 0cd2a20ad..635b65c27 100644 --- a/src/types/backups.ts +++ b/src/types/backups.ts @@ -49,7 +49,7 @@ export interface OffhostAccountRow { * * Computed from the ledger the nightly worker writes, never from the bucket: * the page must be able to answer "does this account have a recent copy - * off-host" on a host whose worker holds PutObject and nothing else. + * off-host" on a host whose worker holds no listing grant on the bucket. */ export interface OffhostBackupOverview { /** Whether this host has the variables the nightly job needs. */