mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-11 22:02:16 +00:00
fix(backup): keep an erased account's copy out of admin accounts
A restore read the account settings under the operator's AI endpoint policy whenever the receiving account was an admin. For a copy put into a chosen account, the settings were written by the erased account's owner, not by the operator, yet an admin account could be chosen as the target, and an ordinary target promoted later got the same trust. Admin accounts are now refused as a target, in the check and the picker, with backup.target.admin_account. A copy uploaded into a chosen account is stored as RETARGETED_UPLOAD_<ms>, and its restore never reads its settings as the operator's, whatever the account's role is by then.
This commit is contained in:
1 parent
dfe34386be
commit
16d1b542a5
24 files changed
+276
-46
No files matched your search
@@ -10,8 +10,10 @@
|
||||
* 3. Parses and validates the JSON against the canonical schema.
|
||||
* 4. Rejects files written by a future schema version (incompatible).
|
||||
* 5. Encrypts the file as it arrives and inserts a new `DataBackup` row
|
||||
* of type `MANUAL_UPLOAD_<unix-ms>` so multiple uploads coexist with
|
||||
* the rolling `WEEKLY_AUTO` snapshot AND with each other.
|
||||
* of type `MANUAL_UPLOAD_<unix-ms>` (`RETARGETED_UPLOAD_<unix-ms>` for
|
||||
* a copy put into a chosen account, `backup-upload-type.ts`) so
|
||||
* multiple uploads coexist with the rolling `WEEKLY_AUTO` snapshot AND
|
||||
* with each other.
|
||||
*
|
||||
* The file is read as a stream and never held whole (#1031): an export of an
|
||||
* account with 1.25 million measurements is a few hundred megabytes of JSON,
|
||||
@@ -85,6 +87,7 @@ import {
|
||||
type RestoreTargetVerdict,
|
||||
} from "@/lib/export/restore-target";
|
||||
import { storeBackupBlob } from "@/lib/export/store-backup-blob";
|
||||
import { uploadedBackupType } from "@/lib/export/backup-upload-type";
|
||||
import {
|
||||
readStreamedBackup,
|
||||
StreamedBackupInvalidError,
|
||||
@@ -476,10 +479,10 @@ export const POST = apiHandler(async (request: NextRequest) => {
|
||||
);
|
||||
}
|
||||
|
||||
// `MANUAL_UPLOAD_<unix-ms>` keeps the (userId, type) unique-constraint
|
||||
// intact so multiple uploads can coexist for the same user without
|
||||
// overwriting each other or the rolling WEEKLY_AUTO snapshot.
|
||||
const uploadType = `MANUAL_UPLOAD_${Date.now()}`;
|
||||
// The millisecond suffix keeps uploads and WEEKLY_AUTO apart under the
|
||||
// (userId, type) unique constraint; a copy put into a chosen account is
|
||||
// marked as such, for its restore (`backup-upload-type.ts`).
|
||||
const uploadType = uploadedBackupType(Boolean(choice), Date.now());
|
||||
let payload: BackupPayload | undefined;
|
||||
let measurementCount = 0;
|
||||
let owner: { id: string; username: string } | null = null;
|
||||
|
||||
@@ -80,10 +80,9 @@ function render(
|
||||
}
|
||||
|
||||
describe("pickableAccounts", () => {
|
||||
it("offers neither managed profiles nor accounts pending deletion", () => {
|
||||
it("offers neither managed profiles, accounts pending deletion nor admin accounts", () => {
|
||||
expect(pickableAccounts(USERS).map((u) => u.username)).toEqual([
|
||||
"returning-person",
|
||||
"operator",
|
||||
]);
|
||||
});
|
||||
});
|
||||
@@ -152,6 +151,9 @@ describe("targetUploadFailureKey", () => {
|
||||
expect(key("backup.target.pending_deletion")).toBe(
|
||||
"admin.section.backups.target.failurePendingDeletion",
|
||||
);
|
||||
expect(key("backup.target.admin_account")).toBe(
|
||||
"admin.section.backups.target.failureAdminAccount",
|
||||
);
|
||||
expect(key("backup.target.not_found")).toBe(
|
||||
"admin.section.backups.target.failureNotFound",
|
||||
);
|
||||
@@ -190,7 +192,7 @@ describe("<BackupTargetForm>", () => {
|
||||
{ data: ELIGIBLE, loading: false, error: null },
|
||||
);
|
||||
expect(html).toContain("returning-person (r@example.test)");
|
||||
expect(html).toContain(">operator<");
|
||||
expect(html).not.toContain(">operator<");
|
||||
expect(html).not.toContain("managed-cafe01");
|
||||
expect(html).not.toContain(">leaving<");
|
||||
expect(html).toContain('data-slot="backup-target-eligible"');
|
||||
|
||||
@@ -49,11 +49,15 @@ export interface BackupTargetCheck {
|
||||
|
||||
/**
|
||||
* The accounts the picker offers: not a managed profile (its Guardians run
|
||||
* it) and not pending deletion. Whether one holds records is the server's to
|
||||
* it), not pending deletion and not an admin account (the server refuses all
|
||||
* three, `restore-target.ts`). Whether one holds records is the server's to
|
||||
* say, per account, once it is picked.
|
||||
*/
|
||||
export function pickableAccounts(users: readonly AdminUser[]): AdminUser[] {
|
||||
return users.filter((user) => !user.managedProfile && user.deletion === null);
|
||||
return users.filter(
|
||||
(user) =>
|
||||
!user.managedProfile && user.deletion === null && user.role !== "ADMIN",
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -87,6 +91,8 @@ export function targetFailureKey(code: string): string {
|
||||
return "admin.section.backups.target.failureManagedProfile";
|
||||
case "backup.target.pending_deletion":
|
||||
return "admin.section.backups.target.failurePendingDeletion";
|
||||
case "backup.target.admin_account":
|
||||
return "admin.section.backups.target.failureAdminAccount";
|
||||
case "backup.target.not_found":
|
||||
return "admin.section.backups.target.failureNotFound";
|
||||
case "backup.target.unconfirmed":
|
||||
|
||||
@@ -0,0 +1,66 @@
|
||||
/**
|
||||
* Whose word a restored AI endpoint is taken on. An admin's own copy is the
|
||||
* operator's configuration, so the operator's wider endpoint policy applies;
|
||||
* a copy uploaded into an account other than the one that wrote it is the
|
||||
* word of whoever wrote it, whatever the receiving account's role.
|
||||
*/
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
import type { Prisma } from "@/generated/prisma/client";
|
||||
import { restoreAccountSettings } from "@/lib/export/account-settings-restore";
|
||||
|
||||
const PRIVATE_ENDPOINT = "http://10.20.30.40:11434";
|
||||
|
||||
function adminTx() {
|
||||
const update = vi.fn(async () => ({}));
|
||||
const tx = {
|
||||
user: {
|
||||
findUniqueOrThrow: vi.fn(async () => ({
|
||||
role: "ADMIN",
|
||||
aiProvider: null,
|
||||
unitPreference: "metric",
|
||||
glucoseUnit: "mg/dL",
|
||||
})),
|
||||
update,
|
||||
},
|
||||
} as unknown as Prisma.TransactionClient;
|
||||
return { tx, update };
|
||||
}
|
||||
|
||||
const payload = {
|
||||
accountSettings: { aiProvider: "LOCAL", aiBaseUrl: PRIVATE_ENDPOINT },
|
||||
};
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllEnvs();
|
||||
});
|
||||
|
||||
describe("restoring an admin account's settings", () => {
|
||||
it("takes a private AI endpoint from the admin's own copy under the operator's policy", async () => {
|
||||
vi.stubEnv("AI_PRIVATE_ORIGINS", "");
|
||||
vi.stubEnv("ALLOW_LOCAL_AI_PRIVATE_HOSTS", "true");
|
||||
const { tx, update } = adminTx();
|
||||
const result = await restoreAccountSettings(tx, "admin-1", payload, {
|
||||
retargeted: false,
|
||||
});
|
||||
expect(result.refused).toEqual([]);
|
||||
expect(update).toHaveBeenCalledWith({
|
||||
where: { id: "admin-1" },
|
||||
data: { aiProvider: "LOCAL", aiBaseUrl: PRIVATE_ENDPOINT },
|
||||
});
|
||||
});
|
||||
|
||||
it("refuses it from a copy that was uploaded into the account from someone else's file", async () => {
|
||||
vi.stubEnv("AI_PRIVATE_ORIGINS", "");
|
||||
vi.stubEnv("ALLOW_LOCAL_AI_PRIVATE_HOSTS", "true");
|
||||
const { tx, update } = adminTx();
|
||||
const result = await restoreAccountSettings(tx, "admin-1", payload, {
|
||||
retargeted: true,
|
||||
});
|
||||
expect(result.refused).toEqual(["aiBaseUrl"]);
|
||||
expect(update).toHaveBeenCalledWith({
|
||||
where: { id: "admin-1" },
|
||||
data: { aiProvider: "LOCAL" },
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -15,6 +15,15 @@ export interface AccountSettingsRestoreInput {
|
||||
accountSettings?: AccountSettingsBackupEntry | null;
|
||||
}
|
||||
|
||||
/** Where the copy being restored came from. */
|
||||
export interface AccountSettingsRestoreSource {
|
||||
/**
|
||||
* The copy was uploaded into this account from a file another account
|
||||
* wrote (`backup-upload-type.ts`), so its settings are that author's word.
|
||||
*/
|
||||
retargeted: boolean;
|
||||
}
|
||||
|
||||
export interface AccountSettingsRestoreResult {
|
||||
/** How many setting columns were written. */
|
||||
applied: number;
|
||||
@@ -32,11 +41,16 @@ export interface AccountSettingsRestoreResult {
|
||||
* the row are never in the write, so they stay exactly as the receiving
|
||||
* account has them. A file without the section leaves the row untouched,
|
||||
* which is how every backup written before the section existed restores.
|
||||
*
|
||||
* An admin's own copy is the operator's configuration and is read under the
|
||||
* operator's AI endpoint policy. A retargeted copy never is: the admin chose
|
||||
* where it went, but the file's author wrote its settings.
|
||||
*/
|
||||
export async function restoreAccountSettings(
|
||||
tx: Prisma.TransactionClient,
|
||||
ownerId: string,
|
||||
payload: AccountSettingsRestoreInput,
|
||||
source: AccountSettingsRestoreSource,
|
||||
): Promise<AccountSettingsRestoreResult> {
|
||||
const entry = payload.accountSettings;
|
||||
if (!entry) return { applied: 0, refused: [], unitsChanged: false };
|
||||
@@ -52,7 +66,7 @@ export async function restoreAccountSettings(
|
||||
});
|
||||
const { data, refused } = admitAccountSettings(
|
||||
entry,
|
||||
{ operatorTrusted: current.role === "ADMIN" },
|
||||
{ operatorTrusted: !source.retargeted && current.role === "ADMIN" },
|
||||
current.aiProvider,
|
||||
);
|
||||
|
||||
|
||||
@@ -190,6 +190,7 @@ export const RESTORE_STEPS: readonly RestoreStep[] = [
|
||||
ctx.tx,
|
||||
ctx.ownerId,
|
||||
ctx.payload,
|
||||
{ retargeted: ctx.retargeted },
|
||||
);
|
||||
for (const column of result.refused) {
|
||||
ctx.skips.push({ catalogue: "accountSetting", key: column, links: 1 });
|
||||
|
||||
@@ -0,0 +1,23 @@
|
||||
/**
|
||||
* The `DataBackup.type` of a copy an admin uploaded.
|
||||
*
|
||||
* A file uploaded for the account it names is `MANUAL_UPLOAD_<ms>`. A file
|
||||
* uploaded into another account (the copy of an erased account, put into the
|
||||
* account chosen for it) is `RETARGETED_UPLOAD_<ms>`, and the mark stays with
|
||||
* the stored copy: its settings were written by the file's author, not by the
|
||||
* account it now belongs to, so its restore never takes them on the
|
||||
* operator's word, whatever that account's role is by then. The millisecond
|
||||
* suffix keeps the `(userId, type)` unique constraint free for several
|
||||
* uploads beside the rolling `WEEKLY_AUTO` copy.
|
||||
*/
|
||||
const MANUAL_UPLOAD_PREFIX = "MANUAL_UPLOAD_";
|
||||
const RETARGETED_UPLOAD_PREFIX = "RETARGETED_UPLOAD_";
|
||||
|
||||
export function uploadedBackupType(retargeted: boolean, now: number): string {
|
||||
return `${retargeted ? RETARGETED_UPLOAD_PREFIX : MANUAL_UPLOAD_PREFIX}${now}`;
|
||||
}
|
||||
|
||||
/** Whether a stored copy was uploaded into an account other than its own. */
|
||||
export function isRetargetedUploadType(type: string): boolean {
|
||||
return type.startsWith(RETARGETED_UPLOAD_PREFIX);
|
||||
}
|
||||
@@ -31,6 +31,7 @@ import { auditLog } from "@/lib/auth/audit";
|
||||
import { invalidateUserData } from "@/lib/cache/invalidate";
|
||||
import { prisma } from "@/lib/db";
|
||||
import type { AccountSettingsRestoreResult } from "@/lib/export/account-settings-restore";
|
||||
import { isRetargetedUploadType } from "@/lib/export/backup-upload-type";
|
||||
import {
|
||||
RESTORE_CLEAR_STEPS,
|
||||
RESTORE_STEPS,
|
||||
@@ -259,6 +260,7 @@ async function replaceAccount(
|
||||
read: ReadBackup,
|
||||
ownerId: string,
|
||||
restoreInstanceSettings: boolean,
|
||||
retargeted: boolean,
|
||||
transactionTimeoutMs: number,
|
||||
tracker: RestoreProgressTracker,
|
||||
prepass: MeasurementRestorePrepass,
|
||||
@@ -285,6 +287,7 @@ async function replaceAccount(
|
||||
streamed: read.streamed,
|
||||
skips,
|
||||
restoreInstanceSettings,
|
||||
retargeted,
|
||||
measurementPrepass: prepass,
|
||||
links: emptyRestoreLinks(),
|
||||
outputs: {
|
||||
@@ -592,6 +595,12 @@ export async function restoreBackup(
|
||||
const checked = await checkBackupBeforeRestore(input, read);
|
||||
if (!checked.ok) return checked.outcome;
|
||||
const { owner, transactionTimeoutMs } = checked;
|
||||
// Read from the row, not passed in, so no caller can leave it out.
|
||||
const stored = await prisma.dataBackup.findUnique({
|
||||
where: { id: backup.id },
|
||||
select: { type: true },
|
||||
});
|
||||
const retargeted = isRetargetedUploadType(stored?.type ?? "");
|
||||
|
||||
let result: TransactionResult;
|
||||
try {
|
||||
@@ -599,6 +608,7 @@ export async function restoreBackup(
|
||||
read,
|
||||
owner.id,
|
||||
restoreInstanceSettings,
|
||||
retargeted,
|
||||
transactionTimeoutMs,
|
||||
tracker,
|
||||
prepass,
|
||||
|
||||
@@ -179,6 +179,11 @@ export interface RestoreContext {
|
||||
/** Declared inside the transaction, so a rollback takes the report with it. */
|
||||
readonly skips: RestoreSkipLog;
|
||||
readonly restoreInstanceSettings: boolean;
|
||||
/**
|
||||
* The copy was uploaded into this account from another account's file
|
||||
* (`backup-upload-type.ts`); its settings are not the operator's word.
|
||||
*/
|
||||
readonly retargeted: boolean;
|
||||
/** What the file's first read noted about its readings. */
|
||||
readonly measurementPrepass: MeasurementRestorePrepass;
|
||||
readonly links: RestoreLinks;
|
||||
|
||||
@@ -14,12 +14,16 @@
|
||||
* A reading or a medication entered during setup is a record like any other
|
||||
* and still refuses, because the restore would delete it.
|
||||
*
|
||||
* The other two refusals keep the account rules intact. A managed profile is
|
||||
* The other refusals keep the account rules intact. A managed profile is
|
||||
* its Guardians' record, which an admin does not write into (the users list
|
||||
* offers no admin deletion on one for the same reason). An account pending
|
||||
* deletion is locked and will be erased, with whatever was restored into it.
|
||||
* Neither the restore nor this check touches the role, so the last-admin rule
|
||||
* is not in play.
|
||||
* An admin account is the operator's: its settings are taken on the
|
||||
* operator's word (a wider AI endpoint policy) and its role reaches every
|
||||
* account, so another person's record does not go into it. Someone who has
|
||||
* signed up again holds an ordinary account, and a new one can always be
|
||||
* made. Neither the restore nor this check touches the role, so the
|
||||
* last-admin rule is not in play.
|
||||
*/
|
||||
import type { Prisma } from "@/generated/prisma/client";
|
||||
import { BACKED_UP_MODELS } from "@/lib/export/backup-plan";
|
||||
@@ -112,7 +116,11 @@ export async function modelsHoldingRecords(
|
||||
}
|
||||
|
||||
export type RestoreTargetRefusal =
|
||||
"not_found" | "managed_profile" | "pending_deletion" | "holds_records";
|
||||
| "not_found"
|
||||
| "managed_profile"
|
||||
| "pending_deletion"
|
||||
| "admin_account"
|
||||
| "holds_records";
|
||||
|
||||
export type RestoreTargetVerdict =
|
||||
| { ok: true; target: { id: string; username: string } }
|
||||
@@ -136,6 +144,7 @@ export async function checkRestoreTarget(
|
||||
username: true,
|
||||
managedProfileAt: true,
|
||||
deletionRequestedAt: true,
|
||||
role: true,
|
||||
},
|
||||
});
|
||||
if (!row) {
|
||||
@@ -148,6 +157,9 @@ export async function checkRestoreTarget(
|
||||
if (row.deletionRequestedAt) {
|
||||
return { ok: false, reason: "pending_deletion", target, models: [] };
|
||||
}
|
||||
if (row.role === "ADMIN") {
|
||||
return { ok: false, reason: "admin_account", target, models: [] };
|
||||
}
|
||||
const models = await modelsHoldingRecords(db, row.id);
|
||||
if (models.length > 0) {
|
||||
return { ok: false, reason: "holds_records", target, models };
|
||||
@@ -160,6 +172,7 @@ export const BACKUP_TARGET_REFUSAL_CODES = {
|
||||
not_found: "backup.target.not_found",
|
||||
managed_profile: "backup.target.managed_profile",
|
||||
pending_deletion: "backup.target.pending_deletion",
|
||||
admin_account: "backup.target.admin_account",
|
||||
holds_records: "backup.target.holds_records",
|
||||
} as const satisfies Record<RestoreTargetRefusal, string>;
|
||||
|
||||
@@ -187,6 +200,12 @@ export const RESTORE_TARGET_REFUSALS: Readonly<
|
||||
message:
|
||||
"The chosen account is pending deletion. Undo the deletion first, or choose another account.",
|
||||
},
|
||||
admin_account: {
|
||||
status: 409,
|
||||
code: BACKUP_TARGET_REFUSAL_CODES.admin_account,
|
||||
message:
|
||||
"The chosen account is an admin account. A copy of another person's record cannot be restored into it; choose an ordinary account or create a new one.",
|
||||
},
|
||||
holds_records: {
|
||||
status: 409,
|
||||
code: BACKUP_TARGET_REFUSAL_CODES.holds_records,
|
||||
|
||||
@@ -105,6 +105,7 @@ export const ERROR_CODE_CATALOGUE: Readonly<Record<string, readonly string[]>> =
|
||||
"backup.payload.undecryptable",
|
||||
"backup.restore.active",
|
||||
"backup.section.missing",
|
||||
"backup.target.admin_account",
|
||||
"backup.target.holds_records",
|
||||
"backup.target.managed_profile",
|
||||
"backup.target.not_found",
|
||||
|
||||
@@ -287,7 +287,7 @@ export const adminBackupPaths: NonNullable<ZodOpenApiObject["paths"]> = {
|
||||
},
|
||||
"409": {
|
||||
description:
|
||||
"The chosen target cannot take the copy, and nothing was stored: a managed profile (`backup.target.managed_profile`), an account pending deletion (`backup.target.pending_deletion`), an account that already holds records (`backup.target.holds_records`, `meta.models` naming them), a username already taken (`backup.target.username_taken`), or a file whose own account is still here (`backup.owner.exists`).",
|
||||
"The chosen target cannot take the copy, and nothing was stored: a managed profile (`backup.target.managed_profile`), an account pending deletion (`backup.target.pending_deletion`), an admin account (`backup.target.admin_account`), an account that already holds records (`backup.target.holds_records`, `meta.models` naming them), a username already taken (`backup.target.username_taken`), or a file whose own account is still here (`backup.owner.exists`).",
|
||||
content: { "application/json": { schema: errorEnvelope } },
|
||||
},
|
||||
"413": {
|
||||
|
||||
Reference in new issue
Block a user