diff --git a/src/lib/utils/backup.ts b/src/lib/utils/backup.ts index e3cb951..58d0d49 100644 --- a/src/lib/utils/backup.ts +++ b/src/lib/utils/backup.ts @@ -388,3 +388,49 @@ export async function fetchBackupExecutions( })); return computeExecutionTally(all); } + +/** + * Does a snapshot (by its restic tags) belong to the stack/container Backups view for a + * given set of ENV-SCOPED config ids? A same-name stack on another environment writes to + * the same shared repo and carries the same `dockhand:name=` tag, so filtering by + * name alone mixed snapshots (and "latest") across environments (#1546). `configIds` are + * already env-scoped by the caller and `dockhand:configid` uniquely identifies a + * (target, env) config, so we match on that. When no config ids are known (should not + * happen once a target has any backup config) fall back to the name tag rather than hide + * everything. Pure + unit-testable. + */ +export function snapshotMatchesConfigScope( + tags: string[] | undefined, + configIds: number[], + targetName: string +): boolean { + const t = tags ?? []; + if (configIds.length > 0) { + for (const tag of t) { + if (tag.startsWith('dockhand:configid=')) { + const n = Number(tag.slice('dockhand:configid='.length)); + if (Number.isInteger(n) && configIds.includes(n)) return true; + } + } + return false; + } + return t.includes(`dockhand:name=${targetName}`); +} + +/** + * Pick THIS target's snapshots from one destination's raw restic list and stamp each with + * the destination it came from - the exact filter+map the Backups panel applies per + * destination. Extracted so the wired call site (not just snapshotMatchesConfigScope in + * isolation) is covered by a test: a regression back to a name-only filter changes this + * helper's output and fails the suite (#1546). + */ +export function selectOwnSnapshotsFromDestination( + snaps: S[], + dest: { id: number; name: string; repository: string }, + configIds: number[], + targetName: string +): (S & { _destinationId: number; _destinationName: string; _destinationRepository: string })[] { + return snaps + .filter((s) => snapshotMatchesConfigScope(s.tags, configIds, targetName)) + .map((s) => ({ ...s, _destinationId: dest.id, _destinationName: dest.name, _destinationRepository: dest.repository })); +} diff --git a/src/routes/containers/SnapshotsPanel.svelte b/src/routes/containers/SnapshotsPanel.svelte index 70467d0..07ae660 100644 --- a/src/routes/containers/SnapshotsPanel.svelte +++ b/src/routes/containers/SnapshotsPanel.svelte @@ -7,7 +7,7 @@ import { formatDateTime, formatRelativeTime } from '$lib/stores/settings'; import { formatBytes } from '$lib/utils/format'; import { readJobResponse } from '$lib/utils/sse-fetch'; - import { getRepoTypeIcon } from '$lib/utils/backup'; + import { getRepoTypeIcon, selectOwnSnapshotsFromDestination } from '$lib/utils/backup'; import SnapshotBrowser from './SnapshotBrowser.svelte'; import RestoreModal from './RestoreModal.svelte'; import SnapshotDiffModal from '../backups/SnapshotDiffModal.svelte'; @@ -102,7 +102,11 @@ try { const destRes = await fetch('/api/backup/destinations'); const destinations: { id: number; name: string; repository: string }[] = destRes.ok ? await destRes.json() : []; - const nameTag = `dockhand:name=${targetName}`; + // Scope to THIS env's configs, not just the name: same-name stacks on different + // environments write to a shared repo and all carry dockhand:name=, so a + // name-only filter mixed their snapshots (and "latest") across envs (#1546). + // configIds is env-scoped (BackupPanel fetches configs with ?env=), so a snapshot's + // own dockhand:configid tag uniquely identifies (stack, env). See snapshotMatchesConfigScope. const all: Snapshot[] = []; loadDone = 0; loadTotal = destinations.length; @@ -119,9 +123,7 @@ const data = await readJobResponse(res); if (data?.error) return; const snaps: any[] = data.snapshots ?? data; - const mine = snaps - .filter((s) => (s.tags || []).some((t: string) => t === nameTag)) - .map((s) => ({ ...s, _destinationId: dest.id, _destinationName: dest.name, _destinationRepository: dest.repository })); + const mine = selectOwnSnapshotsFromDestination(snaps, dest, configIds, targetName); if (mine.length) { all.push(...mine); all.sort((a, b) => new Date(b.time).getTime() - new Date(a.time).getTime()); diff --git a/tests/snapshot-config-scope.test.ts b/tests/snapshot-config-scope.test.ts new file mode 100644 index 0000000..fb32b6c --- /dev/null +++ b/tests/snapshot-config-scope.test.ts @@ -0,0 +1,84 @@ +/** + * snapshotMatchesConfigScope: the stack/container Backups view must show only THIS + * env's snapshots. Same-name stacks on different envs share one repo and all carry + * `dockhand:name=`, so a name-only filter leaked snapshots (and "latest") + * across environments (#1546). Scoping by the env-scoped configid tag fixes it. + */ +import { describe, it, expect } from 'bun:test'; +import { snapshotMatchesConfigScope, selectOwnSnapshotsFromDestination } from '../src/lib/utils/backup'; + +const tags = (name: string, configid: number, envid: number | 'local') => [ + 'dockhand:instance=inst-1', + `dockhand:configid=${configid}`, + `dockhand:envid=${envid}`, + 'dockhand:type=stack', + `dockhand:name=${name}`, +]; + +describe('snapshotMatchesConfigScope', () => { + it('keeps a snapshot whose configid is in this env’s config set', () => { + expect(snapshotMatchesConfigScope(tags('caddy', 1, 1), [1], 'caddy')).toBe(true); + }); + + it('DROPS a same-name snapshot from another env (different configid) - the #1546 leak', () => { + // env-1 view has config 1; a caddy snapshot from env-2 (config 5) must not show. + expect(snapshotMatchesConfigScope(tags('caddy', 5, 2), [1], 'caddy')).toBe(false); + }); + + it('matches any of several env-scoped configs (a stack with configs on >1 destination)', () => { + expect(snapshotMatchesConfigScope(tags('caddy', 8, 3), [7, 8, 9], 'caddy')).toBe(true); + expect(snapshotMatchesConfigScope(tags('caddy', 5, 2), [7, 8, 9], 'caddy')).toBe(false); + }); + + it('name tag alone is NOT enough once configIds are known', () => { + // Same name, but its configid (99) is not one of ours -> excluded despite the name match. + expect(snapshotMatchesConfigScope(tags('caddy', 99, 4), [1], 'caddy')).toBe(false); + }); + + it('falls back to the name tag when no configIds are known (defensive, pre-config)', () => { + expect(snapshotMatchesConfigScope(tags('caddy', 1, 1), [], 'caddy')).toBe(true); + expect(snapshotMatchesConfigScope(tags('other', 1, 1), [], 'caddy')).toBe(false); + }); + + it('handles missing/empty tags without throwing', () => { + expect(snapshotMatchesConfigScope(undefined, [1], 'caddy')).toBe(false); + expect(snapshotMatchesConfigScope([], [1], 'caddy')).toBe(false); + expect(snapshotMatchesConfigScope([], [], 'caddy')).toBe(false); + }); + + it('ignores a configid substring/prefix false match', () => { + // configIds=[1] must NOT match dockhand:configid=15 or =10 + expect(snapshotMatchesConfigScope(tags('caddy', 15, 1), [1], 'caddy')).toBe(false); + expect(snapshotMatchesConfigScope(tags('caddy', 10, 1), [1], 'caddy')).toBe(false); + }); +}); + +// The wired filter the Backups panel actually applies per destination. Testing THIS +// (not just snapshotMatchesConfigScope in isolation) makes the panel's call site +// load-bearing: a regression back to a name-only filter fails here (#1546 review). +describe('selectOwnSnapshotsFromDestination', () => { + const dest = { id: 7, name: 'Restic Server', repository: 'rest:http://repo' }; + + it('keeps only this env-scoped config’s snapshots and stamps the destination', () => { + const snaps = [ + { id: 'a', tags: tags('caddy', 1, 1) }, // env A (config 1) + { id: 'b', tags: tags('caddy', 5, 2) }, // env B (config 5) - same name, must be dropped + ]; + const mine = selectOwnSnapshotsFromDestination(snaps, dest, [1], 'caddy'); + expect(mine.map((s) => s.id)).toEqual(['a']); // NOT 'b' - the #1546 leak is closed + expect(mine[0]._destinationId).toBe(7); + expect(mine[0]._destinationName).toBe('Restic Server'); + expect(mine[0]._destinationRepository).toBe('rest:http://repo'); + }); + + it('a name-only filter would have kept both - this asserts we do NOT', () => { + const snaps = [ + { id: 'a', tags: tags('caddy', 1, 1) }, + { id: 'b', tags: tags('caddy', 5, 2) }, + ]; + // name-only (the old, buggy behaviour) keeps both; the wired helper keeps one. + const nameOnly = snaps.filter((s) => s.tags.includes('dockhand:name=caddy')); + expect(nameOnly.length).toBe(2); + expect(selectOwnSnapshotsFromDestination(snaps, dest, [1], 'caddy').length).toBe(1); + }); +});