warn before saving a stack whose compose path won't survive a recreate

src/lib/server/host-path.ts

@@ -216,6 +216,14 @@ export function resetHostDetection(): void {
 	cachedOwnExtraHosts = null;
 }
 
+/** Test-only: seed the container-mount table so mount-dependent path translation
+ *  (translateContainerPathViaMount / rewriteComposeVolumePaths) can be unit-tested. */
+export function __setCachedMountsForTest(
+	mounts: Array<{ source: string; destination: string }> | null
+): void {
+	cachedMounts = mounts;
+}
+
 /**
  * Get the host path for our DATA_DIR mount by inspecting our own container
  */
@@ -426,6 +434,58 @@ export function getCachedContainerMounts(): Array<{ source: string; destination:
 	return cachedMounts ? [...cachedMounts] : [];
 }
 
+/**
+ * True when a container mount destination STRICTLY DEEPER than DATA_DIR covers
+ * `containerPath` (e.g. DATA_DIR=/app/data with a separate bind at /app/data/stacks).
+ * In that topology translateToHostPath() is WRONG - it resolves the path against
+ * DATA_DIR's host root (the outer volume), but the files physically live under the
+ * more-specific bind. translateContainerPathViaMount() has the right answer, so the
+ * caller must prefer it. Pure over the passed-in mount list for unit testing (#1533).
+ */
+export function pathOverriddenBySubMount(
+	containerPath: string,
+	dataDir: string,
+	mounts: Array<{ source: string; destination: string }>
+): boolean {
+	const norm = (p: string) => p.replace(/\/+$/, '');
+	const dd = norm(dataDir);
+	const cp = norm(containerPath);
+	return mounts.some((m) => {
+		const dest = norm(m.destination);
+		// Deeper than DATA_DIR...
+		if (!(dest.startsWith(dd + '/') && dest.length > dd.length)) return false;
+		// ...and it covers the path.
+		return cp === dest || cp.startsWith(dest + '/');
+	});
+}
+
+/**
+ * Warn when a custom stack compose path won't survive a Dockhand recreate (#1524).
+ * A path Dockhand writes to that is NOT under any of its container mounts lands on the
+ * container's throwaway layer - it looks saved, then vanishes when the container is
+ * recreated (an update/restart), leaving the stack's compose "blank". Returns a
+ * human-readable warning, or null when the path is fine.
+ *
+ * Pure over the passed-in mount list so it unit-tests without a live container. Only
+ * fires when Dockhand is containerized (mounts non-empty) and the path is absolute and
+ * not under any mount destination; bare-metal (no mounts) and default in-DATA_DIR paths
+ * return null.
+ */
+export function unpersistedComposePathWarning(
+	composePath: string | null | undefined,
+	mounts: Array<{ source: string; destination: string }>
+): string | null {
+	if (!composePath || mounts.length === 0) return null;
+	// Only a path Dockhand itself resolves inside its container can be orphaned; a
+	// relative path is resolved against DATA_DIR elsewhere, so only guard absolute paths.
+	if (!composePath.startsWith('/')) return null;
+	const underMount = [...mounts]
+		.sort((a, b) => b.destination.length - a.destination.length)
+		.some((m) => composePath === m.destination || composePath.startsWith(m.destination + '/'));
+	if (underMount) return null;
+	return `The compose file path "${composePath}" is not inside any of Dockhand's mounted volumes, so it is written inside the Dockhand container and will be LOST when the container is recreated (an update or restart). Use a path under a mounted volume (e.g. the default location, or add a bind mount for this path to Dockhand's own compose), then re-save.`;
+}
+
 /**
  * Get the host path for the Docker socket mount.
  * This is needed for sibling containers (e.g., scanners) that need socket access.
@@ -529,14 +589,19 @@ export function rewriteComposeVolumePaths(composeContent: string, workingDir: st
 			const absoluteContainerPath = resolve(workingDir, relativeSrc);
 			const absoluteHostPath = translateContainerPathViaMount(absoluteContainerPath);
 
-			if (absoluteHostPath) {
+			if (absoluteHostPath && absoluteHostPath !== absoluteContainerPath) {
 				const newLine = `${prefix}${absoluteHostPath}${destPart}`;
 				modifiedLines.push(newLine);
 				changes.push(`  ${relativeSrc} -> ${absoluteHostPath}`);
 			} else {
-				// Can't translate — leave line unchanged. Compose will resolve
-				// it relative to its cwd; if that's wrong the deploy fails
-				// loudly, which is better than producing a misleading host path.
+				// Either untranslatable, OR a 1:1 mount where the host path equals the
+				// container path (#1514): rewriting `./x` to the identical absolute path
+				// is pointless and forces stdin deploy (`-f -`), which loses the real
+				// com.docker.compose.project.config_files label. Leave the relative bind
+				// so the deploy can pass the compose file by path; compose resolves `./x`
+				// against the file's dir to the same host location. If it was simply
+				// untranslatable, compose still resolves it against its cwd - a wrong path
+				// there fails the deploy loudly, better than a misleading host path.
 				modifiedLines.push(line);
 			}
 		} else {

src/routes/api/stacks/[name]/check-path-change/+server.ts

@@ -3,6 +3,7 @@ import type { RequestHandler } from './$types';
 import { authorize } from '$lib/server/authorize';
 import { getStackSource } from '$lib/server/db';
 import { findStackDir } from '$lib/server/stacks';
+import { getCachedContainerMounts, unpersistedComposePathWarning } from '$lib/server/host-path';
 import { existsSync, readdirSync } from 'node:fs';
 import { join, dirname } from 'node:path';
 
@@ -15,7 +16,7 @@ import { join, dirname } from 'node:path';
  * query: env:integer Environment ID the stack belongs to (from GET /api/environments)
  * body: {newComposePath:string!}
  * body-example: {"newComposePath":"/opt/stacks/web/compose.yaml"}
- * resp-200: {hasChanges:boolean!, oldDir:string, newDir:string, fileCount:integer!, currentComposePath:string}
+ * resp-200: {hasChanges:boolean!, oldDir:string, newDir:string, fileCount:integer!, currentComposePath:string, persistenceWarning:string}
  * resp-200-example: {"hasChanges":true,"oldDir":"/opt/stacks/old","newDir":"/opt/stacks/web","fileCount":3,"currentComposePath":"/opt/stacks/old/compose.yaml"}
  * resp-403: Permission denied (requires stacks:edit)
  * resp-500: Failed to check path changes
@@ -25,7 +26,10 @@ export const POST: RequestHandler = async ({ params, request, url, cookies }) =>
 	const { name } = params;
 	const envId = url.searchParams.get('env');
 	const envIdNum = envId ? parseInt(envId) : undefined;
-	if (auth.authEnabled && !(await auth.can('stacks', 'edit', envIdNum))) {
+	// edit OR create: this pre-flight (files-to-move + persistence warning) is used by both
+	// the relocate/save flows (edit) and the create flow (a not-yet-existing stack), and it
+	// reveals nothing beyond the caller's own proposed path + Dockhand's mount layout.
+	if (auth.authEnabled && !(await auth.can('stacks', 'edit', envIdNum)) && !(await auth.can('stacks', 'create', envIdNum))) {
 		return json({ error: 'Permission denied' }, { status: 403 });
 	}
 	const envAccessDenied = await auth.requireEnvAccess(envIdNum ?? null);
@@ -78,12 +82,17 @@ export const POST: RequestHandler = async ({ params, request, url, cookies }) =>
 			}
 		}
 
+		// Warn if the chosen path won't survive a Dockhand recreate because it is not
+		// under any of Dockhand's mounted volumes (#1524).
+		const persistenceWarning = unpersistedComposePathWarning(newComposePath, getCachedContainerMounts());
+
 		return json({
 			hasChanges,
 			oldDir: currentDir,
 			newDir,
 			fileCount,
-			currentComposePath
+			currentComposePath,
+			persistenceWarning
 		});
 	} catch (error: any) {
 		console.error(`Error checking path change for stack ${name}:`, error);

src/routes/stacks/StackModal.svelte

@@ -136,6 +136,11 @@
 	const selectedProviderName = $derived(
 		secretProviders.find((p) => p.id === formSecretProviderId)?.name ?? null
 	);
+	// Whether a provider is currently bound AND still exists (a deleted provider leaves
+	// formSecretProviderId pointing at a gone id) - drives the historical banner (#1522).
+	const selectedProviderBound = $derived(
+		formSecretProviderId != null && secretProviders.some((p) => p.id === formSecretProviderId)
+	);
 	// Live probe of the bound provider: key NAMES currently present (bulk + resolved
 	// inline refs). Drives the editor's green IN VAULT marker. Empty when no provider
 	// is bound or the probe failed; probeError holds the reason on failure.
@@ -454,6 +459,43 @@
 	let changeLocationFileCount = $state(0);
 	let changeLocationOldDir = $state<string | null>(null);
 	let movingLocation = $state(false);
+	// Persistence warning: the chosen compose path is not under a Dockhand mount, so it
+	// would vanish on a container recreate (#1524). Shown as a confirm before committing.
+	let showPersistenceWarn = $state(false);
+	let persistenceWarnText = $state('');
+	let pendingHasFilesToMove = $state(false);
+	// Continuation run when the user accepts the persistence warning ("Use it anyway").
+	// Lets create / save / relocate share one dialog: each stashes what to do next.
+	let persistenceWarnProceed: (() => void) | null = null;
+
+	/**
+	 * Pre-flight a compose path: if it is not under a Dockhand mount (would be lost on
+	 * recreate, #1524), show the warning dialog and defer `proceed` until the user accepts;
+	 * otherwise run `proceed` immediately. On any check failure, proceed (never block on the
+	 * guard itself). Only meaningful for an absolute custom path.
+	 */
+	async function guardComposePersistence(composePath: string, proceed: () => void) {
+		const envId = $currentEnvironment?.id ?? null;
+		const probeName = mode === 'edit' ? stackName : (newStackName.trim() || 'new-stack');
+		try {
+			const res = await fetch(
+				appendEnvParam(`/api/stacks/${encodeURIComponent(probeName)}/check-path-change`, envId),
+				{ method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ newComposePath: composePath }) }
+			);
+			if (res.ok) {
+				const data = await res.json();
+				if (data.persistenceWarning) {
+					persistenceWarnText = data.persistenceWarning;
+					persistenceWarnProceed = proceed;
+					showPersistenceWarn = true;
+					return;
+				}
+			}
+		} catch (e) {
+			console.warn('Persistence pre-flight failed:', e);
+		}
+		proceed();
+	}
 
 	async function handleChangeLocation(selectedDir: string, _name: string) {
 		showFileBrowser = false;
@@ -482,13 +524,22 @@
 
 			if (response.ok) {
 				const data = await response.json();
-				if (data.hasChanges && data.oldDir && data.fileCount > 0) {
-					// Show confirmation dialog
-					pendingNewLocation = newDir;
-					pendingNewComposePath = newComposePath;
-					pendingNewEnvPath = newEnvPath;
-					changeLocationOldDir = data.oldDir;
-					changeLocationFileCount = data.fileCount;
+				// Stash the target so a follow-up confirm (persistence and/or move) can act on it.
+				pendingNewLocation = newDir;
+				pendingNewComposePath = newComposePath;
+				pendingNewEnvPath = newEnvPath;
+				changeLocationOldDir = data.oldDir ?? null;
+				changeLocationFileCount = data.fileCount ?? 0;
+				pendingHasFilesToMove = !!(data.hasChanges && data.oldDir && data.fileCount > 0);
+
+				// A non-persisted path is the more serious warning - confirm it FIRST; on
+				// confirm we fall through to the move dialog (if any) or commit (#1524).
+				if (data.persistenceWarning) {
+					persistenceWarnText = data.persistenceWarning;
+					showPersistenceWarn = true;
+					return;
+				}
+				if (pendingHasFilesToMove) {
 					showChangeLocationConfirm = true;
 					return;
 				}
@@ -503,6 +554,41 @@
 		isDirty = true;
 	}
 
+	// "Use it anyway". A continuation (create/save) runs first; otherwise this is the
+	// change-location flow, so continue to the move dialog or commit the new path.
+	function confirmPersistenceWarn() {
+		showPersistenceWarn = false;
+		if (persistenceWarnProceed) {
+			const go = persistenceWarnProceed;
+			persistenceWarnProceed = null;
+			go();
+			return;
+		}
+		if (pendingHasFilesToMove) {
+			showChangeLocationConfirm = true;
+			return;
+		}
+		if (pendingNewComposePath) workingComposePath = pendingNewComposePath;
+		if (pendingNewEnvPath !== null) workingEnvPath = pendingNewEnvPath;
+		isDirty = true;
+		clearPendingLocation();
+	}
+
+	function cancelPersistenceWarn() {
+		showPersistenceWarn = false;
+		persistenceWarnProceed = null;
+		clearPendingLocation();
+	}
+
+	function clearPendingLocation() {
+		pendingNewLocation = null;
+		pendingNewComposePath = null;
+		pendingNewEnvPath = null;
+		changeLocationOldDir = null;
+		changeLocationFileCount = 0;
+		pendingHasFilesToMove = false;
+	}
+
 	function cancelChangeLocation() {
 		showChangeLocationConfirm = false;
 		pendingNewLocation = null;
@@ -1247,7 +1333,7 @@
 		composeContent = newContent;
 	}
 
-	async function handleCreate(start: boolean = false) {
+	async function handleCreate(start: boolean = false, persistenceAcked = false) {
 		errors = {};
 		let hasErrors = false;
 
@@ -1267,6 +1353,11 @@
 
 		if (hasErrors) return;
 
+		// Warn if the chosen compose path won't survive a recreate (#1524) - before creating.
+		if (!persistenceAcked && workingComposePath.trim()) {
+			return guardComposePersistence(workingComposePath.trim(), () => handleCreate(start, true));
+		}
+
 		const envId = $currentEnvironment?.id ?? null;
 
 		// Check if stack already exists
@@ -1364,7 +1455,7 @@
 		}
 	}
 
-	async function handleSave(restart = false, moveFromDir: string | null | undefined = undefined) {
+	async function handleSave(restart = false, moveFromDir: string | null | undefined = undefined, persistenceAcked = false) {
 		errors = {};
 
 		// Validate compose content (unless file location is needed and we have a path)
@@ -1399,6 +1490,13 @@
 					);
 					if (checkResponse.ok) {
 						const checkData = await checkResponse.json();
+						// Non-persisted path is the more serious warning - surface it first (#1524).
+						if (checkData.persistenceWarning && !persistenceAcked) {
+							persistenceWarnText = checkData.persistenceWarning;
+							persistenceWarnProceed = () => handleSave(restart, moveFromDir, true);
+							showPersistenceWarn = true;
+							return;
+						}
 						if (checkData.hasChanges && checkData.oldDir && checkData.fileCount > 0) {
 							// Show confirmation dialog
 							pathChangeOldDir = checkData.oldDir;
@@ -2160,6 +2258,7 @@
 									injectedSecretKeys={mode === 'edit' ? injectedSecretKeys : []}
 									providerType={selectedProviderType}
 									providerName={selectedProviderName}
+									providerBound={selectedProviderBound}
 									{probeError}
 									{providerKeySet}
 									{readonly}
@@ -2353,6 +2452,29 @@
 	</Dialog.Content>
 </Dialog.Root>
 
+<!-- Persistence warning: chosen compose path is not under a Dockhand mount (#1524) -->
+<Dialog.Root bind:open={showPersistenceWarn}>
+	<Dialog.Content class="max-w-lg">
+		<Dialog.Header>
+			<Dialog.Title class="flex items-center gap-2">
+				<TriangleAlert class="w-5 h-5 text-amber-500 shrink-0" />
+				This location isn't persisted
+			</Dialog.Title>
+		</Dialog.Header>
+		<p class="text-sm text-muted-foreground mt-1">
+			{persistenceWarnText}
+		</p>
+		<div class="flex justify-end gap-1.5 mt-4">
+			<Button variant="default" size="sm" onclick={cancelPersistenceWarn}>
+				Pick another location
+			</Button>
+			<Button variant="outline" size="sm" onclick={confirmPersistenceWarn}>
+				Use it anyway
+			</Button>
+		</div>
+	</Dialog.Content>
+</Dialog.Root>
+
 <!-- Change location confirmation dialog -->
 <Dialog.Root bind:open={showChangeLocationConfirm}>
 	<Dialog.Content class="max-w-lg">

tests/unpersisted-compose-path.test.ts

@@ -0,0 +1,113 @@
+/**
+ * #1524: warn when a custom stack compose path is not under any Dockhand container
+ * mount, so it would be written to the container's throwaway layer and lost on recreate.
+ */
+
+import { describe, test, expect } from 'bun:test';
+import { unpersistedComposePathWarning, pathOverriddenBySubMount, translateContainerPathViaMount, __setCachedMountsForTest } from '../src/lib/server/host-path';
+
+const MOUNTS = [
+	{ source: 'dockhand_data', destination: '/app/data' },
+	{ source: '/opt/stacks', destination: '/opt/stacks' },
+];
+
+describe('unpersistedComposePathWarning', () => {
+	test('warns for a path outside every mount', () => {
+		const w = unpersistedComposePathWarning('/home/docker/minecraft/compose.yaml', MOUNTS);
+		expect(w).toContain('/home/docker/minecraft/compose.yaml');
+		expect(w).toContain('recreated');
+	});
+
+	test('no warning for a path under the data volume (default location)', () => {
+		expect(unpersistedComposePathWarning('/app/data/stacks/Production/web/compose.yaml', MOUNTS)).toBeNull();
+	});
+
+	test('no warning for a path under a user-added bind mount', () => {
+		expect(unpersistedComposePathWarning('/opt/stacks/web/compose.yaml', MOUNTS)).toBeNull();
+	});
+
+	test('a path exactly equal to a mount destination is under it', () => {
+		expect(unpersistedComposePathWarning('/opt/stacks', MOUNTS)).toBeNull();
+	});
+
+	test('a prefix that is not a path boundary does NOT count as under the mount', () => {
+		// /app/database is not under /app/data even though the string starts with it.
+		expect(unpersistedComposePathWarning('/app/database/compose.yaml', MOUNTS)).not.toBeNull();
+	});
+
+	test('bare metal (no mounts) never warns', () => {
+		expect(unpersistedComposePathWarning('/home/docker/x/compose.yaml', [])).toBeNull();
+	});
+
+	test('a relative path is not guarded (resolved against DATA_DIR elsewhere)', () => {
+		expect(unpersistedComposePathWarning('compose.yaml', MOUNTS)).toBeNull();
+		expect(unpersistedComposePathWarning('./sub/compose.yaml', MOUNTS)).toBeNull();
+	});
+
+	test('null / empty path never warns', () => {
+		expect(unpersistedComposePathWarning(null, MOUNTS)).toBeNull();
+		expect(unpersistedComposePathWarning('', MOUNTS)).toBeNull();
+		expect(unpersistedComposePathWarning(undefined, MOUNTS)).toBeNull();
+	});
+});
+
+describe('pathOverriddenBySubMount (#1533)', () => {
+	// The exact #1533 topology: /app/data on a named volume, /app/data/stacks on a
+	// separate bind. A stack under /app/data/stacks lives on the bind, not the volume,
+	// so the DATA_DIR translation is wrong and must yield to the mount translation.
+	const SUBMOUNT = [
+		{ source: '/home/docker/docker/volumes/dockhand_data/_data', destination: '/app/data' },
+		{ source: '/home/docker/compose', destination: '/app/data/stacks' },
+	];
+
+	test('true when a deeper bind overrides the stack subpath', () => {
+		expect(pathOverriddenBySubMount('/app/data/stacks/adguard', '/app/data', SUBMOUNT)).toBe(true);
+	});
+
+	test('true when the path equals the sub-mount destination', () => {
+		expect(pathOverriddenBySubMount('/app/data/stacks', '/app/data', SUBMOUNT)).toBe(true);
+	});
+
+	test('false for a DATA_DIR path NOT covered by the sub-mount', () => {
+		// /app/data/db is under DATA_DIR but not under the /app/data/stacks bind.
+		expect(pathOverriddenBySubMount('/app/data/db', '/app/data', SUBMOUNT)).toBe(false);
+	});
+
+	test('false when the only mount IS DATA_DIR itself (normal single-volume setup)', () => {
+		const single = [{ source: 'dockhand_data', destination: '/app/data' }];
+		expect(pathOverriddenBySubMount('/app/data/stacks/adguard', '/app/data', single)).toBe(false);
+	});
+
+	test('false with no mounts (bare metal)', () => {
+		expect(pathOverriddenBySubMount('/app/data/stacks/adguard', '/app/data', [])).toBe(false);
+	});
+
+	test('path-boundary safe: /app/database is not under /app/data', () => {
+		const m = [{ source: '/x', destination: '/app/database' }];
+		expect(pathOverriddenBySubMount('/app/data/stacks/adguard', '/app/data', m)).toBe(false);
+	});
+
+	test('trailing slashes on DATA_DIR / destinations are normalized', () => {
+		const m = [{ source: '/home/docker/compose', destination: '/app/data/stacks/' }];
+		expect(pathOverriddenBySubMount('/app/data/stacks/adguard', '/app/data/', m)).toBe(true);
+	});
+
+	// The load-bearing invariant of the #1533 fix: when we NULL dataDirHostPath because a
+	// sub-mount overrides, mountHostPath (translateContainerPathViaMount) MUST resolve non-null
+	// - otherwise we'd regress a wrong-but-found path into a hard-fail (unknown).
+	test('overridden => translateContainerPathViaMount resolves non-null (no hard-fail regression)', () => {
+		const mounts = [
+			{ source: '/home/docker/docker/volumes/dockhand_data/_data', destination: '/app/data' },
+			{ source: '/home/docker/compose', destination: '/app/data/stacks' },
+		];
+		const path = '/app/data/stacks/adguard';
+		try {
+			__setCachedMountsForTest(mounts);
+			expect(pathOverriddenBySubMount(path, '/app/data', mounts)).toBe(true);
+			// the very folder the backup helper will bind-mount:
+			expect(translateContainerPathViaMount(path)).toBe('/home/docker/compose/adguard');
+		} finally {
+			__setCachedMountsForTest(null);
+		}
+	});
+});