From 4d09c9c76f49b53a682506e0a8aa4efc0d5c046f Mon Sep 17 00:00:00 2001 From: Jarek Krochmalski Date: Wed, 2 Sep 2026 15:52:44 +0200 Subject: [PATCH] local-path backup destinations validate the configured path --- src/lib/server/backups/index.ts | 23 ++++ src/lib/server/backups/local-repo-path.ts | 94 ++++++++++++++++ src/lib/server/backups/restic-script.ts | 2 +- src/lib/server/host-path.ts | 9 ++ .../settings/backups/DestinationModal.svelte | 4 +- tests/backups/local-repo-path.test.ts | 100 ++++++++++++++++++ 6 files changed, 229 insertions(+), 3 deletions(-) create mode 100644 src/lib/server/backups/local-repo-path.ts create mode 100644 tests/backups/local-repo-path.test.ts diff --git a/src/lib/server/backups/index.ts b/src/lib/server/backups/index.ts index a30a25f..082f3bd 100644 --- a/src/lib/server/backups/index.ts +++ b/src/lib/server/backups/index.ts @@ -20,6 +20,7 @@ import { initRepository as initRepoCore, testRepository as testRepoCore, checkRe import { parseRetention, buildForgetArgs, checkWouldWipe } from './retention'; import { buildSnapshotLayout, serializeLayout, parseSnapshotLayout, type SnapshotLayout, type SnapshotStack, type SnapshotSecret } from './snapshot-layout'; import { instanceTagFilter, parseSnapshots, retentionTagFilter, BackupError } from './models'; +import { localRepoIssueFor } from './local-repo-path'; import { parseOptionsJson, buildJobOptions, parseSelectedVolumes, parseBackupFlags, sanitizeRestoreFlags, fireWebhook, parseResticDiff, type SnapshotDiff } from './helpers'; import { getHostname } from '../license'; import { getBackupConfig, getBackupConfigs, getBackupDestination, updateBackupConfig, updateBackupDestination, decryptBackupDestination } from '../db'; @@ -1321,8 +1322,25 @@ export async function forgetSnapshot(destinationId: number, snapshotId: string): // --- repository maintenance --- +/** + * Reject a local-path repo whose path isn't under any Dockhand bind mount (#1506): + * restic would write it into the container's ephemeral filesystem while the helper + * looks for it on the host, so both "succeed" yet never share one repo. Returns an + * error string to surface, or null when the path is fine (bare-metal, or under a bind). + */ +async function localRepoPathIssue(destination: { repository: string }): Promise { + const { isLocalRepo } = await import('./models'); + const { getCachedContainerMounts } = await import('../host-path'); + return localRepoIssueFor(destination.repository, isLocalRepo(destination.repository), getCachedContainerMounts()); +} + export async function initRepository(destinationId: number): Promise { const destination = await loadDest(destinationId); + const pathIssue = await localRepoPathIssue(destination); + if (pathIssue) { + logRepoOp(destination.name, 'init', false, { error: pathIssue }); + throw new Error(pathIssue); + } const r = await initRepoCore(reader(), destination); logRepoOp(destination.name, 'init', r.ok, r.ok ? { output: r.output } : { error: r.error }); if (!r.ok) throw new Error(r.error); @@ -1330,6 +1348,11 @@ export async function initRepository(destinationId: number): Promise { export async function testRepository(destinationId: number): Promise<{ ok: boolean; needsInit?: boolean; error?: string }> { const destination = await loadDest(destinationId); + const pathIssue = await localRepoPathIssue(destination); + if (pathIssue) { + logRepoOp(destination.name, 'test', false, { error: pathIssue }); + return { ok: false, error: pathIssue }; + } const r = await testRepoCore(reader(), destination); logRepoOp(destination.name, 'test', r.ok, r.ok ? undefined : { error: r.error }); if (r.ok) return { ok: true }; diff --git a/src/lib/server/backups/local-repo-path.ts b/src/lib/server/backups/local-repo-path.ts new file mode 100644 index 0000000..e57700e --- /dev/null +++ b/src/lib/server/backups/local-repo-path.ts @@ -0,0 +1,94 @@ +/** + * Pure validation for a LOCAL-path backup repository, shared by init/test (which run + * restic in the Dockhand container) and the backup/restore helper (a sibling container + * that bind-mounts the repo by its HOST path). + * + * The trap this guards (#1506): when Dockhand runs in a container, the "Path" field is + * the path AS THE DOCKHAND CONTAINER SEES IT. If the user types the HOST path instead + * (e.g. /opt/backups) while Dockhand only has it bind-mounted at /app/local-backups, + * `restic init` writes the repo into the container's own ephemeral filesystem (it + * vanishes on restart) and the helper - which mounts the host path - later finds an + * empty dir. Both sides "work" in isolation but never agree on one location. + * + * A local repo is only usable when its container path is UNDER one of Dockhand's bind + * mounts, so the same bytes are reachable from the host (where the helper mounts them). + * + * No I/O here: callers pass the cached mount table so this stays unit-testable. + */ + +export interface ContainerMount { + source: string; // host path + destination: string; // container path +} + +export type LocalRepoPathVerdict = + | { ok: true; hostPath: string } // path is under a bind mount; helper mounts hostPath + | { ok: true; hostPath: null; reason: 'no-mounts' } // bare-metal: container path IS the host path + | { ok: false; reason: 'not-under-bind'; mountHints: string[] }; + +function isUnder(child: string, parent: string): boolean { + return child === parent || child.startsWith(parent.replace(/\/+$/, '') + '/'); +} + +/** + * Decide whether a local repo container path is a usable bind-mounted location. + * - No mounts known (bare metal, or detection not run): allow; the path is already a + * host path (there is no container/host split to get wrong). + * - Mounts known and the path is under one: allow, returning the translated host path. + * - Mounts known and the path is NOT under any: reject - this is the #1506 host-path + * mistake that would write to the container's ephemeral filesystem. + */ +export function classifyLocalRepoPath( + repoPath: string, + mounts: ContainerMount[] +): LocalRepoPathVerdict { + if (!mounts || mounts.length === 0) { + return { ok: true, hostPath: null, reason: 'no-mounts' }; + } + // Longest destination first so the most specific mount wins. + const sorted = [...mounts].sort((a, b) => b.destination.length - a.destination.length); + for (const m of sorted) { + if (isUnder(repoPath, m.destination)) { + const rel = repoPath.slice(m.destination.replace(/\/+$/, '').length); + return { ok: true, hostPath: m.source.replace(/\/+$/, '') + rel }; + } + } + return { + ok: false, + reason: 'not-under-bind', + mountHints: sorted.map((m) => m.destination) + }; +} + +/** + * The init/test decision as a pure function: returns an error string to surface, or + * null when the repo is fine. `isLocal` is injected (the caller passes isLocalRepo) so + * both the remote-exemption gate and the local-path verdict are unit-testable without + * touching the mount cache or the models module. + * - Remote repo (isLocal false): always null - never reject a working s3/rest/b2/sftp. + * - Local repo under a bind (or bare metal): null. + * - Local repo not under any bind: the #1506 host-path error. + */ +export function localRepoIssueFor( + repository: string, + isLocal: boolean, + mounts: ContainerMount[] +): string | null { + if (!isLocal) return null; + const verdict = classifyLocalRepoPath(repository, mounts); + if (verdict.ok) return null; + return localRepoPathError(repository, verdict.mountHints); +} + +/** Human-readable error for a rejected local repo path (surfaced at init/test). */ +export function localRepoPathError(repoPath: string, mountHints: string[]): string { + const hints = mountHints.length + ? ` Use a path under one of Dockhand's bind mounts instead: ${mountHints.join(', ')}.` + : ''; + return ( + `Local path "${repoPath}" is not inside any of Dockhand's bind mounts, so the ` + + `repository would be written to the container's own filesystem and lost on restart, ` + + `and the backup helper on the host would not find it.${hints} ` + + `Enter the path as Dockhand sees it (the container side of your volume mount), not the host path.` + ); +} diff --git a/src/lib/server/backups/restic-script.ts b/src/lib/server/backups/restic-script.ts index 27ecfad..9bd58f1 100644 --- a/src/lib/server/backups/restic-script.ts +++ b/src/lib/server/backups/restic-script.ts @@ -76,7 +76,7 @@ export function tlsCertPreamble(): string { */ export function localRepoGuard(repository: string): string { if (!isLocalRepo(repository)) return ''; - const msg = `restic repository not found at $RESTIC_REPOSITORY on this environment's Docker host. A local-path repository only works when the environment's Docker daemon runs on the same host as Dockhand (e.g. a co-located socket-proxy). For a remote host, use an S3 or REST destination.`; + const msg = `restic repository not found at $RESTIC_REPOSITORY on this environment's Docker host. The repo directory is empty or missing there. Two common causes: (1) the destination Path is the host path instead of the path Dockhand sees inside its container - enter the container-side path of your backup volume mount, then re-init; (2) the environment's Docker daemon is on a different host than Dockhand - a local path can't reach it, so use an S3 or REST destination instead.`; return `test -f "$RESTIC_REPOSITORY/config" || { echo ${shellQuote(msg)} >&2; exit 1; }; `; } diff --git a/src/lib/server/host-path.ts b/src/lib/server/host-path.ts index 4607459..5bfe70d 100644 --- a/src/lib/server/host-path.ts +++ b/src/lib/server/host-path.ts @@ -417,6 +417,15 @@ export function translateContainerPathViaMount(containerPath: string): string | return null; } +/** + * The container mounts cached at startup (empty when Dockhand runs on bare metal, + * or before detectHostDataDir has run). Callers use this to tell "Dockhand is + * containerized and this path is not under any bind" from "no mount info at all". + */ +export function getCachedContainerMounts(): Array<{ source: string; destination: string }> { + return cachedMounts ? [...cachedMounts] : []; +} + /** * Get the host path for the Docker socket mount. * This is needed for sibling containers (e.g., scanners) that need socket access. diff --git a/src/routes/settings/backups/DestinationModal.svelte b/src/routes/settings/backups/DestinationModal.svelte index 7d77ff2..99dd743 100644 --- a/src/routes/settings/backups/DestinationModal.svelte +++ b/src/routes/settings/backups/DestinationModal.svelte @@ -96,7 +96,7 @@ { value: 'local', label: 'Local path', icon: HardDrive, fields: [ - { key: 'path', label: 'Path', placeholder: '/mnt/backups/dockhand', optional: true } + { key: 'path', label: 'Path', placeholder: '/app/local-backups/myrepo', optional: true } ], buildRepo: (f) => f.path || '', parseRepo: (repo) => ({ path: repo }) @@ -564,7 +564,7 @@ {#if selectedBackend.value === 'local'}
- Local path repositories work on the local Docker host, or a co-located socket-proxy on that same host. For remote hosts, use S3, REST, or another remote backend. + Enter the path as Dockhand sees it inside its own container (the container side of your backup volume mount, e.g. /app/local-backups/...), not the host path. Local path repositories work on the local Docker host, or a co-located socket-proxy on that same host. For remote hosts, use S3, REST, or another remote backend.
{/if}
diff --git a/tests/backups/local-repo-path.test.ts b/tests/backups/local-repo-path.test.ts new file mode 100644 index 0000000..977e845 --- /dev/null +++ b/tests/backups/local-repo-path.test.ts @@ -0,0 +1,100 @@ +import { describe, test, expect } from 'bun:test'; +import { classifyLocalRepoPath, localRepoPathError, localRepoIssueFor, type ContainerMount } from '../../src/lib/server/backups/local-repo-path'; + +// The #1506 mount: host /opt/dockhand-backups -> container /app/local-backups. +const MOUNTS: ContainerMount[] = [ + { source: '/opt/dockhand-backups', destination: '/app/local-backups' }, + { source: '/host/data', destination: '/app/data' } +]; + +describe('classifyLocalRepoPath', () => { + test('container-side path under a bind resolves to the host path (the correct config)', () => { + const v = classifyLocalRepoPath('/app/local-backups/Nextcloud', MOUNTS); + expect(v.ok).toBe(true); + expect(v.ok && v.hostPath).toBe('/opt/dockhand-backups/Nextcloud'); + }); + + test('exact mount destination maps to the mount source', () => { + const v = classifyLocalRepoPath('/app/local-backups', MOUNTS); + expect(v.ok && v.hostPath).toBe('/opt/dockhand-backups'); + }); + + test('the #1506 mistake (host path typed in) is rejected', () => { + const v = classifyLocalRepoPath('/opt/dockhand-backups/Nextcloud', MOUNTS); + expect(v.ok).toBe(false); + expect(v.ok === false && v.reason).toBe('not-under-bind'); + expect(v.ok === false && v.mountHints).toContain('/app/local-backups'); + }); + + test('a path under no mount at all is rejected', () => { + const v = classifyLocalRepoPath('/some/random/dir', MOUNTS); + expect(v.ok).toBe(false); + }); + + test('no mounts known (bare metal) -> allowed, container path IS the host path', () => { + const v = classifyLocalRepoPath('/mnt/backups/repo', []); + expect(v.ok).toBe(true); + expect(v.ok && v.hostPath).toBeNull(); + expect(v.ok && 'reason' in v && v.reason).toBe('no-mounts'); + }); + + test('most specific mount wins (longest destination)', () => { + const nested: ContainerMount[] = [ + { source: '/host/a', destination: '/app' }, + { source: '/host/b', destination: '/app/local-backups' } + ]; + const v = classifyLocalRepoPath('/app/local-backups/repo', nested); + expect(v.ok && v.hostPath).toBe('/host/b/repo'); + }); + + test('a sibling that only shares a prefix is not treated as inside the mount', () => { + // /app/local-backups-old must NOT match the /app/local-backups bind. + const v = classifyLocalRepoPath('/app/local-backups-old/repo', MOUNTS); + expect(v.ok).toBe(false); + }); + + test('trailing slashes on the mount destination/source are handled', () => { + const trailing: ContainerMount[] = [{ source: '/host/x/', destination: '/app/y/' }]; + const v = classifyLocalRepoPath('/app/y/repo', trailing); + expect(v.ok && v.hostPath).toBe('/host/x/repo'); + }); +}); + +describe('localRepoIssueFor (the init/test guard decision)', () => { + test('remote repo is NEVER rejected, whatever the mounts (the key safety invariant)', () => { + // isLocal=false: s3/rest/b2/sftp must always pass, even with mounts present. + expect(localRepoIssueFor('s3:https://s3.amazonaws.com/bucket', false, MOUNTS)).toBeNull(); + expect(localRepoIssueFor('rest:http://rest:8000/repo', false, [])).toBeNull(); + expect(localRepoIssueFor('sftp:user@host:/repo', false, MOUNTS)).toBeNull(); + }); + + test('local repo under a bind is allowed', () => { + expect(localRepoIssueFor('/app/local-backups/Nextcloud', true, MOUNTS)).toBeNull(); + }); + + test('local repo on bare metal (no mounts) is allowed', () => { + expect(localRepoIssueFor('/mnt/backups/repo', true, [])).toBeNull(); + }); + + test('local repo NOT under any bind returns the #1506 error', () => { + const issue = localRepoIssueFor('/opt/dockhand-backups/Nextcloud', true, MOUNTS); + expect(issue).not.toBeNull(); + expect(issue).toContain('/opt/dockhand-backups/Nextcloud'); + expect(issue).toContain('/app/local-backups'); + }); +}); + +describe('localRepoPathError', () => { + test('names the bad path and lists the mount hints', () => { + const msg = localRepoPathError('/opt/dockhand-backups/Nextcloud', ['/app/local-backups', '/app/data']); + expect(msg).toContain('/opt/dockhand-backups/Nextcloud'); + expect(msg).toContain('/app/local-backups'); + expect(msg.toLowerCase()).toContain('container'); + }); + + test('reads cleanly with no mount hints', () => { + const msg = localRepoPathError('/x', []); + expect(msg).toContain('/x'); + expect(msg).not.toContain('undefined'); + }); +});