fix(backup): make the console restore leave instance settings alone

A disaster-recovery snapshot carries the singleton `app_settings` row —
registration, the MFA requirement, the default locale and timezone, module
availability, the notification and AI configuration, the document size cap and
quota — and the restore wrote it back as a side effect of restoring one
account. An operator putting a single record back on Wednesday reconfigured the
whole host from Monday's file, silently, with nothing on screen saying so.

Rebuilding a host from a snapshot is a real case, so the write stays: it is now
`restoreInstanceSettings: true` on the request and a checkbox beside the typed
gate, off unless the operator ticks it. Restoring an account and reconfiguring
the installation are two decisions, so they are two answers.

The 422 confirmation sentence and the dialog copy drop the instance-wide claim;
the opt-in carries it instead, in all seven catalogues. Integration coverage
runs both arms: without the flag the settings row keeps its own values, with it
the file's values land.
This commit is contained in:
Marc-André Bombeck committed 2026-09-10 05:16:59 +02:00
1 parent 5cb5c7df44
commit 6b366b4f27
12 files changed
+189 -22

No files matched your search

@@ -11,6 +11,10 @@
* Metadata-only portable document exports are rejected before mutation; the
* importer never fabricates content. Audit rows remain outside the wipe, and
* cache invalidation runs only after the complete restore transaction.
*
* Everything above is scoped to ONE account. The instance-wide settings a
* disaster-recovery payload also carries are not, and they are opt-in for that
* reason: see the `restoreInstanceSettings` block below.
*/
import { Buffer } from "node:buffer";
@@ -142,17 +146,26 @@ const handler = apiHandler(
const { id } = await params;
annotate({ action: { name: "admin.backups.restore" }, meta: { id } });
let body: { confirm?: string } = {};
let body: { confirm?: string; restoreInstanceSettings?: boolean } = {};
try {
const raw = await request.text();
if (raw.length > 64 * 1024) {
return apiError(`Request body exceeds ${64 * 1024} bytes`, 413);
}
body = JSON.parse(raw) as { confirm?: string };
body = JSON.parse(raw) as {
confirm?: string;
restoreInstanceSettings?: boolean;
};
} catch {
return apiError("Invalid JSON body", 400);
}
// Restoring one account's data and reconfiguring the whole host are two
// different decisions, so they are two different answers. Anything but a
// literal `true` restores the account alone.
const restoreInstanceSettings = body.restoreInstanceSettings === true;
annotate({ meta: { restore_instance_settings: restoreInstanceSettings } });
if (body.confirm !== "RESTORE") {
await auditLog("admin.backups.restore.denied", {
userId: admin.id,
@@ -160,7 +173,7 @@ const handler = apiHandler(
details: { reason: "missing_confirmation", backupId: id },
});
return apiError(
"Confirmation token missing — restoring user data and included instance-wide settings requires confirm: 'RESTORE'",
"Confirmation token missing — replacing an account's data requires confirm: 'RESTORE'",
422,
);
}
@@ -292,6 +305,7 @@ const handler = apiHandler(
ownerId,
ownerUsername: owner.username,
snapshotExportedAt: payload.exportedAt,
restoreInstanceSettings,
},
});
@@ -441,7 +455,20 @@ const handler = apiHandler(
where: { userId: ownerId },
});
if (payload.appSettings) {
// The one section of the file that is not this account's.
//
// A disaster-recovery payload carries the singleton `app_settings`
// row — registration, the MFA requirement, the default locale and
// timezone, module availability, the notification and AI
// configuration, the document size cap and quota. Writing it back as
// a side effect of restoring ONE account reconfigures the host for
// everybody on it, silently and hours or weeks out of date: an
// operator putting a single record back on Wednesday has no reason to
// expect Monday's registration switch and Monday's upload cap to come
// with it. Rebuilding a host from a snapshot is a real case, so the
// write stays available — as an answer the operator gives, not one
// the account restore assumes.
if (payload.appSettings && restoreInstanceSettings) {
const settings = payload.appSettings;
const settingsData = {
registrationEnabled: settings.registrationEnabled,
@@ -2084,6 +2111,10 @@ const handler = apiHandler(
backupId: id,
ownerId,
ownerUsername: owner.username,
// Whether this restore also rewrote the host's own settings. The one
// effect of a restore that reaches accounts other than `ownerId`, so
// the trail has to be able to answer it later.
restoreInstanceSettings,
cleared,
// The durable half of the report. The response reaches whoever was
// looking at the screen; the audit row is still here next week when
@@ -140,12 +140,21 @@ describe("BackupsSection — empty state", () => {
expect(className).not.toMatch(/\b(?:opacity-\d+|text-\S+\/\d+)\b/);
});
it("warns in both catalogs that restore overwrites instance-wide settings", () => {
expect(de.admin.section.backups.restoreDescription).toMatch(
it("names the instance-wide effect on the opt-in that carries it, in both catalogs", () => {
// The restore rewrites the host's own settings only when the operator
// ticks the box, so the warning belongs to the box rather than to the
// sentence above it — and the box has to say what it turns on.
expect(de.admin.section.backups.restoreInstanceSettingsLabel).toMatch(
/instanzweite Einstellungen/i,
);
expect(en.admin.section.backups.restoreDescription).toMatch(
/instance-wide settings/i,
expect(de.admin.section.backups.restoreInstanceSettingsHint).toMatch(
/Installation/,
);
expect(en.admin.section.backups.restoreInstanceSettingsLabel).toMatch(
/instance settings/i,
);
expect(en.admin.section.backups.restoreInstanceSettingsHint).toMatch(
/installation/i,
);
});
});
+50 -9
View File
@@ -39,6 +39,7 @@ import { Badge } from "@/components/ui/badge";
import { QueryErrorRow } from "@/components/ui/query-error-row";
import { SettingsCardActions } from "@/components/settings/_card-actions";
import { Button } from "@/components/ui/button";
import { Checkbox } from "@/components/ui/checkbox";
import { EmptyState } from "@/components/ui/empty-state";
import { Input } from "@/components/ui/input";
import { Label } from "@/components/ui/label";
@@ -124,13 +125,18 @@ function RestoreRowDialog({
}: {
row: BackupRow;
pending: boolean;
onConfirm: () => void;
onConfirm: (options: { restoreInstanceSettings: boolean }) => void;
}) {
const { t } = useTranslations();
const fmt = useFormatters();
const [open, setOpen] = useState(false);
const [typed, setTyped] = useState("");
const matched = typed.trim() === "RESTORE";
// Off unless the operator says otherwise. A disaster-recovery snapshot
// carries the host's own settings alongside the account's data, and putting
// one account back is not a reason to reconfigure the installation for
// everybody on it — so the second effect is a second decision.
const [withInstanceSettings, setWithInstanceSettings] = useState(false);
// v1.37.20 — restore preview: fetch what the file contains the moment the
// dialog opens, so the typed confirmation is an informed one. Derived from
@@ -170,7 +176,10 @@ function RestoreRowDialog({
open={open}
onOpenChange={(next) => {
setOpen(next);
if (!next) setTyped("");
if (!next) {
setTyped("");
setWithInstanceSettings(false);
}
}}
>
<AlertDialogTrigger asChild>
@@ -255,6 +264,26 @@ function RestoreRowDialog({
</ul>
)}
</div>
{/* The second decision, before the typed gate rather than after it:
the account's data comes back either way, the host's settings only
if this is ticked. */}
<label
className="flex items-start gap-2 text-sm"
data-slot="restore-instance-settings"
>
<Checkbox
data-testid="backup-restore-instance-settings"
checked={withInstanceSettings}
onCheckedChange={(next) => setWithInstanceSettings(next === true)}
className="mt-0.5"
/>
<span>
{t("admin.section.backups.restoreInstanceSettingsLabel")}
<span className="text-muted-foreground block text-xs">
{t("admin.section.backups.restoreInstanceSettingsHint")}
</span>
</span>
</label>
<div className="space-y-2">
<Label htmlFor={`restore-prompt-${row.id}`}>
{t("admin.section.backups.restorePromptLabel")}
@@ -279,7 +308,8 @@ function RestoreRowDialog({
if (!matched) return;
setOpen(false);
setTyped("");
onConfirm();
onConfirm({ restoreInstanceSettings: withInstanceSettings });
setWithInstanceSettings(false);
}}
>
{pending
@@ -781,14 +811,20 @@ export function BackupsSection() {
// and used inline by `<RestoreRowDialog>` below — keeping the
// mutation here lets the parent invalidate the list query on success.
const restore = useMutation({
mutationFn: async (row: BackupRow) => {
mutationFn: async ({
row,
restoreInstanceSettings,
}: {
row: BackupRow;
restoreInstanceSettings: boolean;
}) => {
// Idempotency-Key prevents a double-click from re-running the
// destructive transaction. Include the row id so two different
// backups can both be restored independently in the same minute.
const idempotencyKey = `restore-${row.id}-${randomId()}`;
return apiPost<{ restored: true; skipped?: RestoreSkipSummary }>(
`/api/admin/backups/${row.id}/restore`,
{ confirm: "RESTORE" },
{ confirm: "RESTORE", restoreInstanceSettings },
{ headers: { "Idempotency-Key": idempotencyKey } },
);
},
@@ -1079,9 +1115,11 @@ export function BackupsSection() {
row={row}
pending={
restore.isPending &&
restore.variables?.id === row.id
restore.variables?.row.id === row.id
}
onConfirm={(options) =>
restore.mutate({ row, ...options })
}
onConfirm={() => restore.mutate(row)}
/>
</div>
</td>
@@ -1139,9 +1177,12 @@ export function BackupsSection() {
<RestoreRowDialog
row={row}
pending={
restore.isPending && restore.variables?.id === row.id
restore.isPending &&
restore.variables?.row.id === row.id
}
onConfirm={(options) =>
restore.mutate({ row, ...options })
}
onConfirm={() => restore.mutate(row)}
/>
</div>
</li>