diff --git a/src/lib/server/backups/snapshots.ts b/src/lib/server/backups/snapshots.ts index 5d90cf9..571ed2f 100644 --- a/src/lib/server/backups/snapshots.ts +++ b/src/lib/server/backups/snapshots.ts @@ -1,12 +1,12 @@ /** - * backups/snapshots.ts — the READ side: list snapshots and read their contents, + * backups/snapshots.ts - the READ side: list snapshots and read their contents, * with the ownership guards that keep one installation (or one environment) from * reading another's backups on a shared repository. * * TWO ownership checks, both enforced HERE so no read path can forget them: * - INSTANCE ownership: every content read (browse/dump/diff/metadata) first * asks restic for the snapshot filtered by THIS install's instance tag. If - * restic returns nothing, the snapshot isn't ours — we refuse. Without this, + * restic returns nothing, the snapshot isn't ours - we refuse. Without this, * anyone who knows a snapshot id could read another install's volume data * from a shared repo. * - ENVIRONMENT ownership (enterprise): the snapshot's owning environment is @@ -42,21 +42,28 @@ export async function assertInstanceOwned( destination: any, instanceId: string, snapshotId: string, + enterprise = false, ): Promise { if (!isValidSnapshotId(snapshotId)) { throw new BackupError('VALIDATION', 'invalid snapshot id'); } + // ENTERPRISE keeps the strict per-instance check: env-scoped RBAC installs enforce env + // boundaries and don't copy repositories between instances, so a snapshot must be THIS + // instance's. NON-ENTERPRISE looks up WITHOUT the instance filter, so a snapshot written + // by another Dockhand instance (a physically-copied or shared repo) is a restorable orphan + // (#1351, promised by the manual). Either way we confirm the id exists and is a Dockhand + // snapshot (has a dockhand:instance= tag), so a foreign app's snapshot can't be targeted. + const filterArgs = enterprise ? ['--tag', instanceTagFilter(instanceId)] : []; const run = await restic.runLocal(destination, [ - 'snapshots', '--json', '--no-lock', '--tag', instanceTagFilter(instanceId), snapshotId, + 'snapshots', '--json', '--no-lock', ...filterArgs, snapshotId, ]); if (run.exitCode !== 0) { throw new BackupError('RESTIC', `could not verify snapshot ownership: ${run.stderr.trim() || 'restic failed'}`); } const snaps = parseSnapshots(run.stdout); const match = snaps.find((s) => s.id === snapshotId || s.shortId === snapshotId); - if (!match) { - // Not found under our instance tag → not ours. Report as not-found so we - // don't reveal that the id exists under another instance. + if (!match || !(match.tags || []).some((t) => t.startsWith('dockhand:instance='))) { + // Not a Dockhand snapshot in this repo (or, for enterprise, not this instance's) -> refuse. throw new BackupError('VALIDATION', 'snapshot not found for this installation', { snapshotId }); } return match.tags; @@ -64,8 +71,8 @@ export async function assertInstanceOwned( /** * Assert the caller may access the snapshot's owning ENVIRONMENT (enterprise - * only). Resolves the env from the snapshot's own `dockhand:envid` tag — never a - * caller-supplied env — and fails closed if it can't be resolved. `tags` is the + * only). Resolves the env from the snapshot's own `dockhand:envid` tag - never a + * caller-supplied env - and fails closed if it can't be resolved. `tags` is the * snapshot's tag list (from assertInstanceOwned, so we don't re-query). */ export async function assertEnvAccess(access: AccessContext, tags: string[]): Promise { @@ -91,13 +98,13 @@ export async function guardSnapshotAccess( snapshotId: string, access: AccessContext, ): Promise { - const tags = await assertInstanceOwned(restic, destination, instanceId, snapshotId); + const tags = await assertInstanceOwned(restic, destination, instanceId, snapshotId, access.isEnterprise); await assertEnvAccess(access, tags); } /** * Resolve a snapshot's owning ENVIRONMENT from its own stable-id tag - * (`dockhand:envid=`), read server-side — never from a caller-supplied value. + * (`dockhand:envid=`), read server-side - never from a caller-supplied value. * Used by the route-level env gate. The lookup is instance-scoped (restic filters * by this install's instance tag), so a snapshot that isn't ours resolves to * "unresolved" and the caller fails closed. @@ -105,7 +112,7 @@ export async function guardSnapshotAccess( * Returns: * - { envId } the numeric owning environment id * - { envId: null } a 'local' / unscoped snapshot (nothing to gate on) - * - { envId: undefined, resolved: false } when the env can't be resolved — + * - { envId: undefined, resolved: false } when the env can't be resolved - * callers MUST deny on this to fail closed. */ export async function resolveSnapshotEnvId( @@ -123,7 +130,7 @@ export async function resolveSnapshotEnvId( const match = snaps.find((s) => s.id === snapshotId || s.shortId === snapshotId); if (!match) return { envId: undefined, resolved: false }; const envId = readEnvIdFromTags(match.tags); - if (envId === null) return { envId: undefined, resolved: false }; // tag absent/malformed → fail closed + if (envId === null) return { envId: undefined, resolved: false }; // tag absent/malformed -> fail closed if (envId === 'local') return { envId: null, resolved: true }; // unscoped snapshot return { envId, resolved: true }; } @@ -145,7 +152,7 @@ export async function filterSnapshotsByAccessibleEnv fail closed if (!decided.has(envId)) decided.set(envId, await canAccess(envId)); if (decided.get(envId)) kept.push(snap); } @@ -163,22 +170,34 @@ export async function listSnapshots( access: AccessContext, configId?: number, ): Promise> { - const tag = configId != null - ? `${instanceTagFilter(instanceId)},dockhand:configid=${configId}` - : instanceTagFilter(instanceId); - const run = await restic.runLocal(destination, ['snapshots', '--json', '--no-lock', '--tag', tag]); + // Per-config listing (a config row's own snapshots) stays instance-scoped - a config is + // this install's, and its configid collides across instances (a fresh install reuses + // 1,2,3...), so scoping by instance+configid is what keeps it correct. The FULL repo + // listing (no configId - feeds orphan detection) must NOT be instance-scoped, or foreign + // Dockhand snapshots (a physically-copied or shared repo) never surface as orphans (#1351). + const args = ['snapshots', '--json', '--no-lock']; + if (configId != null) args.push('--tag', `${instanceTagFilter(instanceId)},dockhand:configid=${configId}`); + const run = await restic.runLocal(destination, args); if (run.exitCode !== 0) { // An empty repo (not initialised) and a real error look different; surface // the stderr so the caller can classify. Return [] only on a clean exit. throw new BackupError('RESTIC', run.stderr.trim() || 'could not list snapshots'); } - const snaps = parseSnapshots(run.stdout); + // restic --tag can't match "has ANY dockhand:instance= tag" (exact-match only), so filter + // here: keep only snapshots this repo received FROM DOCKHAND (any instance), never those a + // different app wrote to a shared repo. Per-config results already carry the tag. + const snaps = parseSnapshots(run.stdout).filter((s) => + configId != null || (s.tags || []).some((t) => t.startsWith('dockhand:instance='))); if (!access.isEnterprise) return snaps; - // Filter to snapshots whose env the caller can access (unscoped = visible). + // Filter to snapshots whose env the caller can access. `local` is unscoped -> visible; + // a null envId (tag absent/malformed) is UNRESOLVABLE -> fail CLOSED (drop), matching + // readEnvIdFromTags's contract and the sibling filterSnapshotsByAccessibleEnv. Treating + // null as visible would leak scoped snapshots whose env tag is missing/garbled. const visible = []; for (const s of snaps) { const envId = readEnvIdFromTags(s.tags); - if (envId === null || envId === 'local') { visible.push(s); continue; } + if (envId === 'local') { visible.push(s); continue; } + if (envId === null) continue; // unresolvable env -> fail closed if (await access.canAccessEnvironment(envId)) visible.push(s); } return visible; diff --git a/src/routes/api/backup/snapshots/+server.ts b/src/routes/api/backup/snapshots/+server.ts index a4cec38..0c5c22d 100644 --- a/src/routes/api/backup/snapshots/+server.ts +++ b/src/routes/api/backup/snapshots/+server.ts @@ -1,15 +1,15 @@ import { json } from '@sveltejs/kit'; import type { RequestHandler } from './$types'; import { authorize } from '$lib/server/authorize'; +import { requireBackups } from '$lib/server/backups/route-guards'; import { getBackupConfig, getBackupDestinations } from '$lib/server/db'; import { listSnapshots } from '$lib/server/backups'; import { filterSnapshotsByEnvAccess } from '$lib/server/backups/route-guards'; export const GET: RequestHandler = async ({ url, cookies }) => { const auth = await authorize(cookies); - if (auth.authEnabled && !await auth.can('backups', 'view')) { - return json({ error: 'Permission denied' }, { status: 403 }); - } + const denied = await requireBackups(auth, 'view'); + if (denied) return denied; const configIdParam = url.searchParams.get('configId'); const destIdParam = url.searchParams.get('destinationId'); @@ -94,16 +94,22 @@ export const GET: RequestHandler = async ({ url, cookies }) => { if (isNaN(destinationId)) return json({ error: 'Invalid destinationId' }, { status: 400 }); try { - // No target filter — return all snapshots in this destination. + // No target filter — return all snapshots in this destination (incl. snapshots + // from OTHER Dockhand instances sharing/copied into the repo). The response carries + // this install's instance id so the client can tell own snapshots from foreign + // orphans without relying on configid (which collides across instances - #1351). + const { getInstanceId } = await import('$lib/server/backups/identity'); + const thisInstanceId = await getInstanceId(); const snapshots = await listSnapshots(destinationId); // (HIGH #8) On enterprise, a non-admin must not see snapshots belonging to // environments they can't access — the ids/paths alone are disclosure. Drop // any snapshot whose owning env (from its dockhand:env tag) isn't accessible. + const headers = { 'X-Dockhand-Instance': thisInstanceId }; if (auth.isEnterprise) { const filtered = await filterSnapshotsByEnvAccess(auth, snapshots); - return json(filtered); + return json(filtered, { headers }); } - return json(snapshots); + return json(snapshots, { headers }); } catch (error) { const errorMsg = error instanceof Error ? error.message : String(error); return json({ error: errorMsg }, { status: 500 }); diff --git a/src/routes/backups/+page.svelte b/src/routes/backups/+page.svelte index d040abc..6d50047 100644 --- a/src/routes/backups/+page.svelte +++ b/src/routes/backups/+page.svelte @@ -10,6 +10,7 @@ import { Button } from '$lib/components/ui/button'; import { Badge } from '$lib/components/ui/badge'; import { Input } from '$lib/components/ui/input'; + import { SnapshotLoadProgress } from '$lib/components/ui/snapshot-load-progress'; import * as Select from '$lib/components/ui/select'; import * as Tooltip from '$lib/components/ui/tooltip'; import { DataGrid } from '$lib/components/data-grid'; @@ -106,6 +107,12 @@ // Snapshot counts loaded async per destination, keyed by targetName:destId let snapshotCounts = $state>(new Map()); let snapshotCountsLoading = $state(true); + // Progress of the per-destination snapshot count sweep: how many destinations have + // responded out of the total. Drives an "X / N" indicator so a slow/unreachable + // repo (backend restic timeout is 5 min) shows visible progress instead of an + // indeterminate spinner. done === total means the sweep finished. + let snapshotCountsDone = $state(0); + let snapshotCountsTotal = $state(0); // Orphan targets: found in repos but no config exists interface OrphanTarget { key: string; targetName: string; type: string; destinationId: number; destinationName?: string; destinationRepository?: string; snapshotCount: number; latestSnapshot?: string; } let orphanTargets = $state([]); @@ -169,6 +176,8 @@ let restoreSnapshotId = $state(''); let restoreContainerName = $state(''); let restoreEnvId = $state(undefined); + let restoreDestName = $state(''); + let restoreDestRepo = $state(''); // Snapshot diff let showDiff = $state(false); @@ -313,6 +322,8 @@ async function loadSnapshotCounts() { snapshotCountsLoading = true; + snapshotCountsDone = 0; + snapshotCountsTotal = destinations.length; // Live config ids per destination — a snapshot's dockhand:configid tag matching // one of these belongs to a shown config; anything else is orphaned. const liveConfigIds = new Set(configs.map(c => c.id)); @@ -322,10 +333,19 @@ // (snapshots whose config was deleted). The right-column count must match the // expanded per-config list, so it's keyed by config, NOT by target name — two // configs backing the same container to the same repo have DIFFERENT counts. + // + // Each destination updates snapshotCounts as soon as IT responds, and the + // "Loading" indicator clears after the FIRST one finishes - so a single slow or + // unreachable repo can't hold the whole list hostage for its 30s timeout. The + // rest keep filling in counts progressively behind the scenes. await Promise.allSettled(destinations.map(async (dest) => { try { const res = await fetch(`/api/backup/snapshots?destinationId=${dest.id}`); if (!res.ok) return; + // This install id: a snapshot belongs to a LOCAL config only when THIS instance wrote it. + // configid collides across instances (a fresh install reuses 1,2,3...), so a foreign + // snapshot with configid=1 must be an orphan, not mis-bucketed into local config #1 (#1351). + const thisInstance = res.headers.get("X-Dockhand-Instance") || ""; const d = await res.json(); const snaps: any[] = d.snapshots ?? d; if (!Array.isArray(snaps)) return; @@ -338,8 +358,9 @@ const perConfig = new Map(); const orphanByName = new Map(); for (const snap of snaps) { - const cid = Number(tagVal(snap, 'dockhand:configid=')); - if (Number.isInteger(cid) && liveConfigIds.has(cid)) { + const cid = Number(tagVal(snap, "dockhand:configid=")); + const isOwn = tagVal(snap, "dockhand:instance=") === thisInstance; + if (isOwn && Number.isInteger(cid) && liveConfigIds.has(cid)) { perConfig.set(cid, (perConfig.get(cid) || 0) + 1); } else { const name = tagVal(snap, 'dockhand:name=') || 'unknown'; @@ -371,9 +392,10 @@ } snapshotCounts = newCounts; orphanTargets = newOrphans; - } catch {} + } catch {} finally { snapshotCountsDone += 1; } })); + // Backstop: if there were zero destinations, nothing cleared it above. snapshotCountsLoading = false; } @@ -622,6 +644,8 @@ restoreSnapshotId = snapshot.id; restoreContainerName = config.targetName; restoreEnvId = config.environmentId ?? undefined; + restoreDestName = snapshot._destinationName ?? config.destinationName ?? ''; + restoreDestRepo = snapshot._destinationRepository ?? config.destinationRepository ?? ''; showRestore = true; } @@ -651,11 +675,13 @@
- {#if loading || snapshotCountsLoading} + {#if loading} - {loading ? 'Loading…' : 'Loading snapshots…'} + Loading… + {:else if snapshotCountsLoading} + {/if}
@@ -1045,6 +1071,8 @@ destinationId={restoreDestId} snapshotId={restoreSnapshotId} containerName={restoreContainerName} + destinationName={restoreDestName} + destinationRepository={restoreDestRepo} environmentId={restoreEnvId} onDone={fetchData} /> diff --git a/src/routes/containers/SnapshotsPanel.svelte b/src/routes/containers/SnapshotsPanel.svelte index 39ab4c9..d85f853 100644 --- a/src/routes/containers/SnapshotsPanel.svelte +++ b/src/routes/containers/SnapshotsPanel.svelte @@ -41,6 +41,10 @@ } let loading = $state(true); + // Progress of the per-destination snapshot sweep (X of N repos) - a slow/unreachable + // repo can take up to the backend restic timeout (5 min), so show it advancing. + let loadDone = $state(0); + let loadTotal = $state(0); let snapshots = $state([]); let deletingSnapshot = $state(null); let confirmDeleteSnapshot = $state(null); @@ -55,6 +59,8 @@ let showRestore = $state(false); let restoreDestId = $state(0); let restoreSnapshotId = $state(''); + let restoreDestName = $state(''); + let restoreDestRepo = $state(''); // Diff (click one to select, another to compare) let showDiff = $state(false); let diffDestId = $state(0); @@ -97,22 +103,29 @@ const destinations: { id: number; name: string; repository: string }[] = destRes.ok ? await destRes.json() : []; const nameTag = `dockhand:name=${targetName}`; const all: Snapshot[] = []; - // Query every destination and keep only THIS target's snapshots (by tag). + loadDone = 0; + loadTotal = destinations.length; + snapshots = []; + // Query every destination and keep only THIS target's snapshots (by tag). Show each + // repo's results the moment it returns instead of blocking on the slowest repo - with + // many (or one unreachable) repos the list fills in live rather than after a long wait. await Promise.all(destinations.map(async (dest) => { try { const res = await fetch(`/api/backup/snapshots?destinationId=${dest.id}`); if (!res.ok) return; const data = await res.json(); const snaps: any[] = data.snapshots ?? data; - for (const s of snaps) { - if ((s.tags || []).some((t: string) => t === nameTag)) { - all.push({ ...s, _destinationId: dest.id, _destinationName: dest.name, _destinationRepository: dest.repository }); - } + const mine = snaps + .filter((s) => (s.tags || []).some((t: string) => t === nameTag)) + .map((s) => ({ ...s, _destinationId: dest.id, _destinationName: dest.name, _destinationRepository: dest.repository })); + if (mine.length) { + all.push(...mine); + all.sort((a, b) => new Date(b.time).getTime() - new Date(a.time).getTime()); + snapshots = [...all]; + onCount?.(all.length); } - } catch { /* skip an unreachable destination */ } + } catch { /* skip an unreachable destination */ } finally { loadDone += 1; } })); - all.sort((a, b) => new Date(b.time).getTime() - new Date(a.time).getTime()); - snapshots = all; onCount?.(all.length); // Per-snapshot stats from schedule executions (recorded at backup time). @@ -151,6 +164,8 @@ function openRestore(s: Snapshot) { restoreDestId = s._destinationId; restoreSnapshotId = s.id; + restoreDestName = s._destinationName; + restoreDestRepo = s._destinationRepository; showRestore = true; } function toggleDiff(s: Snapshot) { @@ -226,7 +241,7 @@ - + 1 ? `Loading snapshots… ${loadDone} of ${loadTotal} repos` : 'Loading snapshots…'} /> {:else if snapshots.length === 0}
@@ -333,6 +348,8 @@ destinationId={restoreDestId} snapshotId={restoreSnapshotId} containerName={targetName} + destinationName={restoreDestName} + destinationRepository={restoreDestRepo} {environmentId} onDone={loadSnapshots} />