From 781f48de3314358061582b87f8614f07818f59e8 Mon Sep 17 00:00:00 2001 From: Brad Harris Date: Sun, 9 Aug 2026 09:27:06 -0600 Subject: [PATCH 1/7] fix: migrate legacy shared media paths --- apps/server/src/agents/manager.ts | 7 +- apps/server/src/agents/telemetry.ts | 3 +- apps/server/src/config.ts | 18 +-- apps/server/src/shared/media.ts | 14 +- apps/server/test/db/agent-manager.test.ts | 44 +++++ apps/server/test/migrate-legacy-media.test.ts | 148 +++++++++++++++++ apps/server/test/pack-release.test.ts | 1 + apps/server/test/shared-media.test.ts | 19 +++ bin/migrate-legacy-media | 151 ++++++++++++++++++ release-notes/next-assisted-update.json | 12 ++ .../0012-legacy-media-paths.yaml | 59 +++++++ 11 files changed, 462 insertions(+), 14 deletions(-) create mode 100644 apps/server/test/migrate-legacy-media.test.ts create mode 100755 bin/migrate-legacy-media create mode 100644 release-notes/next-assisted-update.json create mode 100644 update-migrations/0012-legacy-media-paths.yaml diff --git a/apps/server/src/agents/manager.ts b/apps/server/src/agents/manager.ts index 77274d12f..c41cdb43d 100644 --- a/apps/server/src/agents/manager.ts +++ b/apps/server/src/agents/manager.ts @@ -17,6 +17,7 @@ import { worktreePathSlug, } from "../shared/git/worktree.js"; import { readWorktreeStatus } from "../shared/git/worktree-status.js"; +import { resolveMediaDir } from "../shared/media.js"; import { buildGitContextForWorktree, probeGitContext, @@ -905,7 +906,11 @@ export class AgentManager { } try { - const mediaDir = agent.mediaDir ?? this.defaultMediaDir(id); + const mediaDir = resolveMediaDir( + id, + agent.mediaDir, + this.config.mediaRoot + ); // mediaDir must exist before launch — both runtimes assume the // directory is present. (The original inert path created it // explicitly; the tmux setup-script path created it via the diff --git a/apps/server/src/agents/telemetry.ts b/apps/server/src/agents/telemetry.ts index 73f03a89b..111807201 100644 --- a/apps/server/src/agents/telemetry.ts +++ b/apps/server/src/agents/telemetry.ts @@ -2,6 +2,7 @@ import path from "node:path"; import type { Pool } from "pg"; +import { resolveStoragePath } from "../shared/media.js"; import type { AgentGitContext, AgentPin } from "./types.js"; export type ActivitySummaryResult = { @@ -515,7 +516,7 @@ export async function listMedia( return result.rows.map((row) => ({ fileName: row.fileName, filePath: path.join( - row.mediaDir ?? fallbackMediaDir(agentId), + resolveStoragePath(row.mediaDir ?? fallbackMediaDir(agentId)), row.fileName ), description: row.description, diff --git a/apps/server/src/config.ts b/apps/server/src/config.ts index 667e3d2a6..f9f951fb3 100644 --- a/apps/server/src/config.ts +++ b/apps/server/src/config.ts @@ -1,8 +1,10 @@ import "dotenv/config"; import { execSync } from "node:child_process"; import { readFileSync } from "node:fs"; +import os from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; +import { resolveStoragePath } from "./shared/media.js"; const __dirname = path.dirname(fileURLToPath(import.meta.url)); @@ -37,12 +39,6 @@ function requireEnv(name: string): string { return value; } -function expandHome(p: string): string { - return p.startsWith("~/") - ? path.join(process.env.HOME ?? "/tmp", p.slice(2)) - : p; -} - function loadTls(): TlsConfig | null { const certPath = process.env.TLS_CERT; const keyPath = process.env.TLS_KEY; @@ -51,8 +47,8 @@ function loadTls(): TlsConfig | null { throw new Error("Both TLS_CERT and TLS_KEY must be set to enable TLS"); } return { - cert: readFileSync(expandHome(certPath)), - key: readFileSync(expandHome(keyPath)), + cert: readFileSync(resolveStoragePath(certPath)), + key: readFileSync(resolveStoragePath(keyPath)), }; } @@ -84,9 +80,9 @@ export function loadConfig(): AppConfig { port: Number(process.env.DISPATCH_PORT ?? process.env.PORT ?? 6767), databaseUrl: requireEnv("DATABASE_URL"), authToken: "", // resolved from DB in start() via getOrCreateAuthToken() - mediaRoot: - process.env.MEDIA_ROOT ?? - path.join(process.env.HOME ?? "/tmp", ".dispatch", "media"), + mediaRoot: resolveStoragePath( + process.env.MEDIA_ROOT ?? path.join(os.homedir(), ".dispatch", "media") + ), dispatchBinDir: path.resolve(__dirname, "..", "..", "..", "bin"), codexBin: process.env.DISPATCH_CODEX_BIN ?? process.env.CODEX_BIN ?? "codex", diff --git a/apps/server/src/shared/media.ts b/apps/server/src/shared/media.ts index e59cef81e..75013fac4 100644 --- a/apps/server/src/shared/media.ts +++ b/apps/server/src/shared/media.ts @@ -1,4 +1,16 @@ import path from "node:path"; +import os from "node:os"; + +/** Resolve a storage path without treating a leading `~` as a literal name. */ +export function resolveStoragePath(storagePath: string): string { + const expanded = + storagePath === "~" + ? os.homedir() + : storagePath.startsWith("~/") + ? path.join(os.homedir(), storagePath.slice(2)) + : storagePath; + return path.resolve(expanded); +} import { extensionForMime, @@ -46,7 +58,7 @@ export function resolveMediaDir( mediaDir: string | null, mediaRoot: string ): string { - return mediaDir ?? path.join(mediaRoot, agentId); + return resolveStoragePath(mediaDir ?? path.join(mediaRoot, agentId)); } export function toMediaKey(file: { name: string; updatedAt: string }): string { diff --git a/apps/server/test/db/agent-manager.test.ts b/apps/server/test/db/agent-manager.test.ts index 969247441..7ee22c585 100644 --- a/apps/server/test/db/agent-manager.test.ts +++ b/apps/server/test/db/agent-manager.test.ts @@ -1509,6 +1509,45 @@ describe("AgentManager", () => { expect(launchCommand).toContain(sessionId); }); + it("should resolve a legacy home-relative media_dir before restarting", async () => { + const { runCommand } = + await import("../../src/shared/lib/run-command.js"); + const agent = await createStoppedAgent({ type: "claude" }); + const fakeHome = await mkdtemp(path.join(os.tmpdir(), "dispatch-home-")); + const legacyMediaDir = `~/.dispatch/legacy-media-${agent.id}`; + const expectedMediaDir = path.join( + fakeHome, + ".dispatch", + `legacy-media-${agent.id}` + ); + const newSessionArgs: string[][] = []; + const homedirSpy = vi.spyOn(os, "homedir").mockReturnValue(fakeHome); + + try { + await pool.query(`UPDATE agents SET media_dir = $2 WHERE id = $1`, [ + agent.id, + legacyMediaDir, + ]); + vi.mocked(runCommand).mockImplementation(async (_cmd, args) => { + if (args[0] === "has-session") { + if (newSessionArgs.length === 0) + return { exitCode: 1, stdout: "", stderr: "" }; + return { exitCode: 0, stdout: "", stderr: "" }; + } + if (args.includes("new-session")) newSessionArgs.push(args); + return { exitCode: 0, stdout: "", stderr: "" }; + }); + + await manager.startAgent(agent.id); + + expect(newSessionArgs).toHaveLength(1); + expect(newSessionArgs[0]!.join(" ")).toContain(expectedMediaDir); + } finally { + homedirSpy.mockRestore(); + await rm(fakeHome, { recursive: true, force: true }); + } + }); + it("should not include --resume flag for fresh sessions", async () => { const { runCommand } = await import("../../src/shared/lib/run-command.js"); @@ -2219,6 +2258,11 @@ describe("AgentManager", () => { expect(typeof item.createdAt).toBe("string"); expect(item.filePath.endsWith(`${agent.id}/doc.pdf`)).toBe(true); expect(path.isAbsolute(item.filePath)).toBe(true); + + await writeFile(item.filePath, "shared media"); + await expect(readFile(item.filePath, "utf-8")).resolves.toBe( + "shared media" + ); }); it("should resolve filePath using the agent's media_dir override", async () => { diff --git a/apps/server/test/migrate-legacy-media.test.ts b/apps/server/test/migrate-legacy-media.test.ts new file mode 100644 index 000000000..57a839b58 --- /dev/null +++ b/apps/server/test/migrate-legacy-media.test.ts @@ -0,0 +1,148 @@ +import { execFile } from "node:child_process"; +import { + chmod, + copyFile, + lstat, + mkdir, + mkdtemp, + readFile, + rm, + symlink, + writeFile, +} from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { promisify } from "node:util"; + +import { afterEach, describe, expect, it } from "vitest"; + +const execFileAsync = promisify(execFile); +const scriptSource = path.resolve( + import.meta.dirname, + "../../..", + "bin/migrate-legacy-media" +); +const tempDirs: string[] = []; + +afterEach(async () => { + await Promise.all( + tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true })) + ); +}); + +async function createFixture() { + const root = await mkdtemp(path.join(os.tmpdir(), "dispatch-media-root-")); + const home = await mkdtemp(path.join(os.tmpdir(), "dispatch-media-home-")); + tempDirs.push(root, home); + const script = path.join(root, "bin", "migrate-legacy-media"); + await mkdir(path.dirname(script), { recursive: true }); + await copyFile(scriptSource, script); + await chmod(script, 0o755); + return { + root, + home, + script, + sourceDir: path.join(root, "~", ".dispatch", "media"), + destinationDir: path.join(home, ".dispatch", "media"), + }; +} + +async function runMigration( + script: string, + home: string, + mode: "--dry-run" | "--apply" +) { + return execFileAsync(script, [mode], { + env: { ...process.env, HOME: home }, + }); +} + +describe("migrate-legacy-media", () => { + it("moves literal-tilde media only after an explicit apply", async () => { + const fixture = await createFixture(); + const source = path.join(fixture.sourceDir, "agt_1", "report.pdf"); + const destination = path.join( + fixture.destinationDir, + "agt_1", + "report.pdf" + ); + await mkdir(path.dirname(source), { recursive: true }); + await writeFile(source, "legacy report"); + + const dryRun = await runMigration( + fixture.script, + fixture.home, + "--dry-run" + ); + expect(dryRun.stdout).toContain("would move: agt_1/report.pdf"); + await expect(readFile(source, "utf8")).resolves.toBe("legacy report"); + + const applied = await runMigration(fixture.script, fixture.home, "--apply"); + expect(applied.stdout).toContain("moved: agt_1/report.pdf"); + await expect(readFile(destination, "utf8")).resolves.toBe("legacy report"); + await expect(readFile(source, "utf8")).rejects.toMatchObject({ + code: "ENOENT", + }); + }); + + it("does not overwrite a conflicting destination file", async () => { + const fixture = await createFixture(); + const source = path.join(fixture.sourceDir, "agt_1", "report.pdf"); + const destination = path.join( + fixture.destinationDir, + "agt_1", + "report.pdf" + ); + await mkdir(path.dirname(source), { recursive: true }); + await mkdir(path.dirname(destination), { recursive: true }); + await writeFile(source, "legacy report"); + await writeFile(destination, "new report"); + + await expect( + runMigration(fixture.script, fixture.home, "--apply") + ).rejects.toMatchObject({ code: 2 }); + await expect(readFile(source, "utf8")).resolves.toBe("legacy report"); + await expect(readFile(destination, "utf8")).resolves.toBe("new report"); + }); + + it("does not replace a dangling destination symlink", async () => { + const fixture = await createFixture(); + const source = path.join(fixture.sourceDir, "agt_1", "report.pdf"); + const destination = path.join( + fixture.destinationDir, + "agt_1", + "report.pdf" + ); + await mkdir(path.dirname(source), { recursive: true }); + await mkdir(path.dirname(destination), { recursive: true }); + await writeFile(source, "legacy report"); + await symlink("missing-report.pdf", destination); + + await expect( + runMigration(fixture.script, fixture.home, "--apply") + ).rejects.toMatchObject({ code: 2 }); + await expect(readFile(source, "utf8")).resolves.toBe("legacy report"); + expect((await lstat(destination)).isSymbolicLink()).toBe(true); + }); + + it("does not follow a destination ancestor symlink", async () => { + const fixture = await createFixture(); + const outside = await mkdtemp(path.join(os.tmpdir(), "dispatch-outside-")); + tempDirs.push(outside); + const source = path.join(fixture.sourceDir, "agt_1", "report.pdf"); + const linkedAgentDir = path.join(fixture.destinationDir, "agt_1"); + await mkdir(path.dirname(source), { recursive: true }); + await mkdir(fixture.destinationDir, { recursive: true }); + await writeFile(source, "legacy report"); + await symlink(outside, linkedAgentDir); + + await expect( + runMigration(fixture.script, fixture.home, "--apply") + ).rejects.toMatchObject({ code: 2 }); + await expect(readFile(source, "utf8")).resolves.toBe("legacy report"); + expect((await lstat(linkedAgentDir)).isSymbolicLink()).toBe(true); + await expect( + readFile(path.join(outside, "report.pdf"), "utf8") + ).rejects.toMatchObject({ code: "ENOENT" }); + }); +}); diff --git a/apps/server/test/pack-release.test.ts b/apps/server/test/pack-release.test.ts index c6f84decf..36a17f93e 100644 --- a/apps/server/test/pack-release.test.ts +++ b/apps/server/test/pack-release.test.ts @@ -60,6 +60,7 @@ describe.skipIf(!BUILDS_EXIST)("pack-release", () => { // reference it, but old checkout-based services need it through the // fixed-runtime migration. expect(files).toContain("bin/dispatch-launchd-wrapper"); + expect(files).toContain("bin/migrate-legacy-media"); }); it("does NOT embed macOS xattr/pax metadata (e.g. com.apple.provenance)", () => { diff --git a/apps/server/test/shared-media.test.ts b/apps/server/test/shared-media.test.ts index 8c55c934a..059bcd817 100644 --- a/apps/server/test/shared-media.test.ts +++ b/apps/server/test/shared-media.test.ts @@ -1,3 +1,6 @@ +import os from "node:os"; +import path from "node:path"; + import { describe, expect, it } from "vitest"; import { @@ -7,6 +10,8 @@ import { isTextFile, isValidMediaKey, mimeType, + resolveMediaDir, + resolveStoragePath, sanitizeUploadedFileName, toMediaKey, } from "../src/shared/media.js"; @@ -15,6 +20,20 @@ import { TEXT_EXTENSIONS, } from "../src/shared/media-file-types.js"; +describe("media storage paths", () => { + it("expands a home-relative storage path to an absolute path", () => { + expect(resolveStoragePath("~/.dispatch/media")).toBe( + path.join(os.homedir(), ".dispatch", "media") + ); + }); + + it("returns an absolute media directory when the configured root uses ~", () => { + expect(resolveMediaDir("agt_test", null, "~/.dispatch/media")).toBe( + path.join(os.homedir(), ".dispatch", "media", "agt_test") + ); + }); +}); + describe("sanitizeUploadedFileName", () => { it("passes through a clean filename unchanged", () => { expect(sanitizeUploadedFileName("screenshot.png")).toBe("screenshot.png"); diff --git a/bin/migrate-legacy-media b/bin/migrate-legacy-media new file mode 100755 index 000000000..e05162cf4 --- /dev/null +++ b/bin/migrate-legacy-media @@ -0,0 +1,151 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Move media written by Dispatch versions that treated a leading `~` as a +# literal directory name. This is intentionally a standalone shell script: it +# ships in release tarballs and runs on installs that only have the compiled +# Dispatch binary (not Bun, pnpm, or source files). + +ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +MODE="dry-run" + +usage() { + cat <<'USAGE' +Usage: bin/migrate-legacy-media [--dry-run|--apply] + +Moves the legacy literal-tilde media tree at: + /~/.dispatch/media + +to the correct home-relative media tree: + $HOME/.dispatch/media + +--dry-run is the default. --apply moves files that do not already exist at +the destination, removes source duplicates with identical contents, and +leaves conflicting files untouched. Exit status 2 means conflicts or +unsupported entries need manual handling. +USAGE +} + +case "${1:-}" in + ""|--dry-run) ;; + --apply) MODE="apply" ;; + --help|-h) usage; exit 0 ;; + *) echo "error: unknown argument: $1" >&2; usage >&2; exit 1 ;; +esac + +if [[ -z "${HOME:-}" ]]; then + echo "error: HOME must be set to migrate legacy media" >&2 + exit 1 +fi + +SOURCE_DIR="$ROOT_DIR/~/.dispatch/media" +DESTINATION_DIR="$HOME/.dispatch/media" + +destination_ancestors_are_safe() { + local relative_path="$1" + local parent_path="" + local current="$HOME" + local component + + if [[ "$relative_path" == */* ]]; then + parent_path="${relative_path%/*}" + fi + + # Check every existing path component below HOME. A symlink at any level + # would make mkdir -p follow it and let a migration write outside the media + # tree. Missing components are safe; mkdir -p creates them during --apply. + for component in ".dispatch" "media"; do + current="$current/$component" + if [[ -L "$current" || ( -e "$current" && ! -d "$current" ) ]]; then + echo "$current" + return 1 + fi + done + + while [[ -n "$parent_path" ]]; do + component="${parent_path%%/*}" + current="$current/$component" + if [[ -L "$current" || ( -e "$current" && ! -d "$current" ) ]]; then + echo "$current" + return 1 + fi + if [[ "$parent_path" == */* ]]; then + parent_path="${parent_path#*/}" + else + parent_path="" + fi + done +} + +if [[ ! -e "$SOURCE_DIR" ]]; then + echo "No legacy media directory found at $SOURCE_DIR; nothing to migrate." + exit 0 +fi +if [[ ! -d "$SOURCE_DIR" ]]; then + echo "error: legacy media path is not a directory: $SOURCE_DIR" >&2 + exit 1 +fi + +moved=0 +duplicates=0 +conflicts=0 +unsupported=0 + +while IFS= read -r -d '' source_file; do + relative_path="${source_file#"$SOURCE_DIR"/}" + destination_file="$DESTINATION_DIR/$relative_path" + + if ! unsafe_ancestor="$(destination_ancestors_are_safe "$relative_path")"; then + conflicts=$((conflicts + 1)) + echo "conflict (unsafe destination ancestor left untouched): $relative_path ($unsafe_ancestor)" >&2 + continue + fi + + # -e is false for dangling symlinks, so test -L first. Never replace a + # symlink, directory, device, or other non-regular destination entry. + if [[ -L "$destination_file" || ( -e "$destination_file" && ! -f "$destination_file" ) ]]; then + conflicts=$((conflicts + 1)) + echo "conflict (non-regular destination left untouched): $relative_path" >&2 + continue + fi + + if [[ -f "$destination_file" ]]; then + if cmp -s "$source_file" "$destination_file"; then + duplicates=$((duplicates + 1)) + if [[ "$MODE" == "apply" ]]; then + rm "$source_file" + echo "removed duplicate: $relative_path" + else + echo "would remove duplicate: $relative_path" + fi + else + conflicts=$((conflicts + 1)) + echo "conflict (left untouched): $relative_path" >&2 + fi + continue + fi + + moved=$((moved + 1)) + if [[ "$MODE" == "apply" ]]; then + mkdir -p "$(dirname "$destination_file")" + mv "$source_file" "$destination_file" + echo "moved: $relative_path" + else + echo "would move: $relative_path" + fi +done < <(find "$SOURCE_DIR" -type f -print0) + +while IFS= read -r -d '' unsupported_path; do + unsupported=$((unsupported + 1)) + echo "unsupported entry (left untouched): ${unsupported_path#"$SOURCE_DIR"/}" >&2 +done < <(find "$SOURCE_DIR" \( -type l -o -type p -o -type s -o -type b -o -type c \) -print0) + +if [[ "$MODE" == "apply" && "$conflicts" -eq 0 && "$unsupported" -eq 0 ]]; then + find "$SOURCE_DIR" -depth -type d -empty -delete +fi + +echo "Legacy media migration ($MODE): $moved file(s) moved, $duplicates duplicate(s) removed, $conflicts conflict(s), $unsupported unsupported entry(s)." + +if [[ "$conflicts" -gt 0 || "$unsupported" -gt 0 ]]; then + exit 2 +fi diff --git a/release-notes/next-assisted-update.json b/release-notes/next-assisted-update.json new file mode 100644 index 000000000..ded5e0854 --- /dev/null +++ b/release-notes/next-assisted-update.json @@ -0,0 +1,12 @@ +{ + "mode": "required", + "title": "Migrate shared media from legacy tilde paths", + "summary": "This release corrects shared-media storage paths configured with ~/.dispatch/media. The required assisted update safely moves recoverable historical files from the old literal-tilde directory into the correct home media directory, preserving conflicts for manual recovery.", + "instructions": "1. Use the managed update flow to install the target release and wait for health.\n2. Identify and stop every active agent that still has a legacy literal-tilde media path or was launched before this release, even if the old directory is absent; a retained agent environment can recreate it. If any cannot be safely stopped, block the update before moving files.\n3. From the Dispatch install directory, run bin/migrate-legacy-media --dry-run.\n4. If the dry run reports no conflicts or unsupported entries, run bin/migrate-legacy-media --apply.\n5. If any conflict is reported, do not overwrite either file; report the affected path for manual recovery.\n6. Restart only the agents stopped for migration after it succeeds, including after a no-op migration, then confirm the legacy directory is empty or absent, the health endpoint is healthy, and release.json reports the target tag.", + "requiredChecks": [ + "service_restarted", + "health_endpoint", + "version_converged" + ], + "rollbackGuidance": "If the target service is unhealthy, roll back using the normal Dispatch rollback flow. Do not delete media from either location: the migrator never overwrites destination files, and it is safe to rerun after recovery." +} diff --git a/update-migrations/0012-legacy-media-paths.yaml b/update-migrations/0012-legacy-media-paths.yaml new file mode 100644 index 000000000..b7b7290a0 --- /dev/null +++ b/update-migrations/0012-legacy-media-paths.yaml @@ -0,0 +1,59 @@ +id: legacy-media-paths +title: Migrate shared media from the legacy literal-tilde directory +summary: > + Earlier Dispatch versions accepted MEDIA_ROOT values such as + ~/.dispatch/media without expanding the tilde. Those installs stored shared + media beneath a literal ~ directory inside the Dispatch install. This + migration moves that legacy tree into the correct home-relative media + directory so historical dispatch_list_media entries remain readable after + the path-resolution fix. + +alreadySatisfied: + description: > + The legacy directory /~/.dispatch/media does not exist, + or it is empty after migration; no active agent has a stored media_dir + beginning with ~/ or a session launched before this release; and the + current service is healthy on the target tag. An empty or absent legacy + directory alone is insufficient because an already-running agent can + recreate it using its retained literal-tilde environment. + +instructions: + - During inspect, independently check whether + /~/.dispatch/media exists and identify every active agent + whose stored media_dir begins with ~/ or whose session was launched before + this release. Record affected agents even when the legacy directory is + absent, because their retained environment can create it later. + - If alreadySatisfied is false, use the managed update endpoint to install + the target release and wait for the service to become healthy. + - After the target service is healthy and regardless of whether the legacy + directory exists, stop every affected active agent cleanly so no process + continues writing to the literal-tilde tree. If an agent cannot be safely + stopped, report the update as blocked and do not run the file migration. + - From the Dispatch install directory, run + bin/migrate-legacy-media --dry-run and inspect its summary before moving + anything. Do not manually overwrite destination files. + - Run bin/migrate-legacy-media --apply only when the dry run reports zero + conflicts and zero unsupported entries. The command is idempotent and + retains conflicting source files for manual recovery. + - If the command reports conflicts or unsupported entries, report the + update as blocked with the affected relative paths; do not delete either + copy. Otherwise confirm the legacy directory is absent or empty. + - Restart each agent stopped for this migration only after the file migration + succeeds (including when the migration is a no-op), so its + DISPATCH_MEDIA_DIR is recreated from the corrected absolute path. + - Confirm $DISPATCH_API_URL/api/v1/health returns status=ok and release.json + under the install directory reports the target tag. + +validation: + requiredChecks: + - service_restarted + - health_endpoint + - version_converged + +rollback: + - If the target service does not return healthy, roll back to the previous + healthy release using the normal Dispatch rollback flow. + - If media migration has started, leave both the corrected destination files + and any unresolved source files in place; the migration never overwrites + destination data, so rerunning it is safe after recovery. + - Restart the prior service and confirm its health endpoint returns status=ok. From f7283e41389568ae5f8b593276a0bc0f3f4df59a Mon Sep 17 00:00:00 2001 From: Brad Harris Date: Thu, 20 Aug 2026 08:24:07 -0600 Subject: [PATCH 2/7] fix: harden legacy media migrator against review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Derive the legacy/expanded media paths from the install's effective MEDIA_ROOT (--media-root, env, or /.env) instead of hardcoding ~/.dispatch/media. A non-default tilde root such as ~/dispatch-media was previously reported as migrated while its media stayed unreadable. - Exit 0 as an explicit no-op when MEDIA_ROOT is absolute: those installs were never affected and must not incur a stop-all-agents outage. - Replace `mv` with an atomic no-clobber link(2) placement (plus a staged cross-device fallback). The preflight checks alone could not hold: the Dispatch server writes media while the migration runs, and `mv` silently overwrote a destination file that appeared in that window. - Materialize both `find` traversals and check their exit status. Streaming through a process substitution hid a failed scan, so an unreadable directory produced a zero-conflict summary and exit 0 — which the manifest treats as permission to run --apply. - Walk the ancestor-safety check over the derived destination components, and return an explicit success status from that function. - Manifest: branch on MEDIA_ROOT before stopping anything, replace the unevaluable "launched before this release" guard with stop-all-agents, and correct the rollback guidance — after --apply a plain rollback passes its health check with every migrated file unreadable. Co-Authored-By: Claude Opus 5 --- apps/server/test/migrate-legacy-media.test.ts | 150 +++++++++++- bin/migrate-legacy-media | 215 ++++++++++++++++-- release-notes/next-assisted-update.json | 6 +- .../0012-legacy-media-paths.yaml | 81 +++++-- 4 files changed, 399 insertions(+), 53 deletions(-) diff --git a/apps/server/test/migrate-legacy-media.test.ts b/apps/server/test/migrate-legacy-media.test.ts index 57a839b58..64188cd4d 100644 --- a/apps/server/test/migrate-legacy-media.test.ts +++ b/apps/server/test/migrate-legacy-media.test.ts @@ -50,13 +50,26 @@ async function createFixture() { async function runMigration( script: string, home: string, - mode: "--dry-run" | "--apply" + mode: "--dry-run" | "--apply", + options: { args?: string[]; env?: NodeJS.ProcessEnv } = {} ) { - return execFileAsync(script, [mode], { - env: { ...process.env, HOME: home }, + return execFileAsync(script, [mode, ...(options.args ?? [])], { + env: { + ...process.env, + // The script falls back to MEDIA_ROOT from the environment; an inherited + // value from the outer test runner would silently retarget the run. + MEDIA_ROOT: undefined, + ...options.env, + HOME: home, + }, }); } +/** Write MEDIA_ROOT into the install .env the way the service reads it. */ +async function writeEnvFile(root: string, contents: string) { + await writeFile(path.join(root, ".env"), contents); +} + describe("migrate-legacy-media", () => { it("moves literal-tilde media only after an explicit apply", async () => { const fixture = await createFixture(); @@ -145,4 +158,135 @@ describe("migrate-legacy-media", () => { readFile(path.join(outside, "report.pdf"), "utf8") ).rejects.toMatchObject({ code: "ENOENT" }); }); + + it("resolves a non-default tilde MEDIA_ROOT from the install .env", async () => { + const fixture = await createFixture(); + await writeEnvFile(fixture.root, "MEDIA_ROOT=~/dispatch-media\n"); + // The legacy tree lives under the configured value, not the default one. + const source = path.join( + fixture.root, + "~", + "dispatch-media", + "agt_1", + "shot.png" + ); + const destination = path.join( + fixture.home, + "dispatch-media", + "agt_1", + "shot.png" + ); + await mkdir(path.dirname(source), { recursive: true }); + await writeFile(source, "legacy shot"); + + const applied = await runMigration(fixture.script, fixture.home, "--apply"); + expect(applied.stdout).toContain("moved: agt_1/shot.png"); + await expect(readFile(destination, "utf8")).resolves.toBe("legacy shot"); + }); + + it("accepts an explicit --media-root over the .env value", async () => { + const fixture = await createFixture(); + await writeEnvFile(fixture.root, "MEDIA_ROOT=~/dispatch-media\n"); + const source = path.join( + fixture.root, + "~", + "override", + "agt_1", + "shot.png" + ); + const destination = path.join( + fixture.home, + "override", + "agt_1", + "shot.png" + ); + await mkdir(path.dirname(source), { recursive: true }); + await writeFile(source, "legacy shot"); + + const applied = await runMigration( + fixture.script, + fixture.home, + "--apply", + { + args: ["--media-root", "~/override"], + } + ); + expect(applied.stdout).toContain("moved: agt_1/shot.png"); + await expect(readFile(destination, "utf8")).resolves.toBe("legacy shot"); + }); + + it("is a no-op on an install whose MEDIA_ROOT is absolute", async () => { + const fixture = await createFixture(); + await writeEnvFile(fixture.root, "MEDIA_ROOT=/var/lib/dispatch/media\n"); + // A stray legacy tree must still be left alone: an absolute MEDIA_ROOT was + // never resolved through the broken tilde path, so nothing here is ours. + const source = path.join(fixture.sourceDir, "agt_1", "report.pdf"); + await mkdir(path.dirname(source), { recursive: true }); + await writeFile(source, "legacy report"); + + const applied = await runMigration(fixture.script, fixture.home, "--apply"); + expect(applied.stdout).toContain("nothing to migrate"); + await expect(readFile(source, "utf8")).resolves.toBe("legacy report"); + }); + + it("fails instead of reporting a clean result when the scan is incomplete", async () => { + const fixture = await createFixture(); + const readable = path.join(fixture.sourceDir, "agt_1", "report.pdf"); + const lockedDir = path.join(fixture.sourceDir, "agt_locked"); + await mkdir(path.dirname(readable), { recursive: true }); + await mkdir(lockedDir, { recursive: true }); + await writeFile(readable, "legacy report"); + await writeFile(path.join(lockedDir, "hidden.pdf"), "hidden"); + await chmod(lockedDir, 0o000); + + try { + // find cannot descend into the locked directory. Exiting 0 with a + // zero-conflict summary here would let the manifest greenlight --apply + // from a scan that never saw `hidden.pdf`. + const result = await runMigration( + fixture.script, + fixture.home, + "--dry-run" + ).then( + () => null, + (err: { code?: number; stderr?: string }) => err + ); + expect(result?.code).toBe(1); + expect(result?.stderr).toContain("incomplete scan"); + } finally { + await chmod(lockedDir, 0o755); + } + }); + + it("treats a destination that appears mid-migration as a conflict", async () => { + const fixture = await createFixture(); + const source = path.join(fixture.sourceDir, "agt_1", "report.pdf"); + const destination = path.join( + fixture.destinationDir, + "agt_1", + "report.pdf" + ); + await mkdir(path.dirname(source), { recursive: true }); + await writeFile(source, "legacy report"); + + // Shim `mkdir` so a file lands at the destination in the exact window + // between the script's preflight checks and its placement — the race a + // live Dispatch server can win by writing media while the migration runs. + const shimDir = await mkdtemp(path.join(os.tmpdir(), "dispatch-shim-")); + tempDirs.push(shimDir); + const shim = path.join(shimDir, "mkdir"); + await writeFile( + shim, + `#!/bin/bash\n/bin/mkdir "$@"\nprintf 'raced in' > ${JSON.stringify(destination)} 2>/dev/null || true\n` + ); + await chmod(shim, 0o755); + + await expect( + runMigration(fixture.script, fixture.home, "--apply", { + env: { PATH: `${shimDir}:${process.env.PATH ?? ""}` }, + }) + ).rejects.toMatchObject({ code: 2 }); + await expect(readFile(destination, "utf8")).resolves.toBe("raced in"); + await expect(readFile(source, "utf8")).resolves.toBe("legacy report"); + }); }); diff --git a/bin/migrate-legacy-media b/bin/migrate-legacy-media index e05162cf4..0b65dc44e 100755 --- a/bin/migrate-legacy-media +++ b/bin/migrate-legacy-media @@ -8,16 +8,26 @@ set -euo pipefail ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" MODE="dry-run" +MEDIA_ROOT_OVERRIDE="" usage() { cat <<'USAGE' -Usage: bin/migrate-legacy-media [--dry-run|--apply] +Usage: bin/migrate-legacy-media [--dry-run|--apply] [--media-root VALUE] -Moves the legacy literal-tilde media tree at: - /~/.dispatch/media +Dispatch versions before the tilde-expansion fix stored media under a +directory literally named `~` inside the install tree whenever MEDIA_ROOT was +configured with a leading tilde. This moves that legacy tree to the location +the fixed runtime actually reads. -to the correct home-relative media tree: - $HOME/.dispatch/media +For MEDIA_ROOT=~/.dispatch/media (the documented default) that means: + from /~/.dispatch/media + to $HOME/.dispatch/media + +The media root is not assumed. It is taken from --media-root, else the +MEDIA_ROOT environment variable, else MEDIA_ROOT in /.env +(the file the service loads), else the documented default. An install whose +MEDIA_ROOT is an absolute path was never affected and exits 0 immediately +without touching anything. --dry-run is the default. --apply moves files that do not already exist at the destination, removes source duplicates with identical contents, and @@ -26,25 +36,94 @@ unsupported entries need manual handling. USAGE } -case "${1:-}" in - ""|--dry-run) ;; - --apply) MODE="apply" ;; - --help|-h) usage; exit 0 ;; - *) echo "error: unknown argument: $1" >&2; usage >&2; exit 1 ;; -esac +while [[ $# -gt 0 ]]; do + case "$1" in + --dry-run) MODE="dry-run" ;; + --apply) MODE="apply" ;; + --media-root) + if [[ $# -lt 2 ]]; then + echo "error: --media-root requires a value" >&2 + exit 1 + fi + MEDIA_ROOT_OVERRIDE="$2" + shift + ;; + --media-root=*) MEDIA_ROOT_OVERRIDE="${1#--media-root=}" ;; + --help|-h) usage; exit 0 ;; + *) echo "error: unknown argument: $1" >&2; usage >&2; exit 1 ;; + esac + shift +done if [[ -z "${HOME:-}" ]]; then echo "error: HOME must be set to migrate legacy media" >&2 exit 1 fi -SOURCE_DIR="$ROOT_DIR/~/.dispatch/media" -DESTINATION_DIR="$HOME/.dispatch/media" +# Read MEDIA_ROOT out of the env file the service itself loads (systemd +# EnvironmentFile / the dotenv import in the runtime). Only the last +# assignment wins, matching dotenv, and surrounding quotes are stripped. +media_root_from_env_file() { + local env_file="$ROOT_DIR/.env" + local line value + [[ -r "$env_file" ]] || return 0 + line="$(grep -E '^[[:space:]]*(export[[:space:]]+)?MEDIA_ROOT=' "$env_file" | tail -1)" || return 0 + [[ -n "$line" ]] || return 0 + value="${line#*=}" + # Trim a trailing carriage return (CRLF env files) and surrounding quotes. + value="${value%$'\r'}" + if [[ "$value" == \"*\" || "$value" == \'*\' ]]; then + value="${value:1:${#value}-2}" + fi + printf '%s' "$value" +} + +CONFIGURED_MEDIA_ROOT="" +CONFIGURED_SOURCE="default" +if [[ -n "$MEDIA_ROOT_OVERRIDE" ]]; then + CONFIGURED_MEDIA_ROOT="$MEDIA_ROOT_OVERRIDE" + CONFIGURED_SOURCE="--media-root" +elif [[ -n "${MEDIA_ROOT:-}" ]]; then + CONFIGURED_MEDIA_ROOT="$MEDIA_ROOT" + CONFIGURED_SOURCE="MEDIA_ROOT environment variable" +else + CONFIGURED_MEDIA_ROOT="$(media_root_from_env_file)" + [[ -n "$CONFIGURED_MEDIA_ROOT" ]] && CONFIGURED_SOURCE="$ROOT_DIR/.env" +fi + +if [[ -n "$CONFIGURED_MEDIA_ROOT" && "$CONFIGURED_MEDIA_ROOT" != "~" && "$CONFIGURED_MEDIA_ROOT" != "~/"* ]]; then + echo "MEDIA_ROOT is $CONFIGURED_MEDIA_ROOT (from $CONFIGURED_SOURCE), which has no leading tilde." + echo "This install never wrote media to a literal-tilde path; nothing to migrate." + exit 0 +fi + +# No explicit setting anywhere: fall back to the documented default so an +# install whose .env is unreadable is still checked. Costs nothing — if the +# legacy tree is absent the run is a no-op. +if [[ -z "$CONFIGURED_MEDIA_ROOT" ]]; then + CONFIGURED_MEDIA_ROOT="~/.dispatch/media" +fi + +# The legacy tree sits under the service's working directory, which both the +# launchd plist and the systemd unit set to the install directory. +SOURCE_DIR="$ROOT_DIR/$CONFIGURED_MEDIA_ROOT" +if [[ "$CONFIGURED_MEDIA_ROOT" == "~" ]]; then + DESTINATION_DIR="$HOME" +else + DESTINATION_DIR="$HOME/${CONFIGURED_MEDIA_ROOT#\~/}" +fi + +# Components of the destination tree below $HOME, walked by the ancestor +# check. Derived from the configured media root rather than hardcoded, so a +# non-default MEDIA_ROOT is checked as thoroughly as the default one. +DESTINATION_RELATIVE="${DESTINATION_DIR#"$HOME"}" +DESTINATION_RELATIVE="${DESTINATION_RELATIVE#/}" destination_ancestors_are_safe() { local relative_path="$1" local parent_path="" local current="$HOME" + local remaining local component if [[ "$relative_path" == */* ]]; then @@ -54,12 +133,19 @@ destination_ancestors_are_safe() { # Check every existing path component below HOME. A symlink at any level # would make mkdir -p follow it and let a migration write outside the media # tree. Missing components are safe; mkdir -p creates them during --apply. - for component in ".dispatch" "media"; do + remaining="$DESTINATION_RELATIVE" + while [[ -n "$remaining" ]]; do + component="${remaining%%/*}" current="$current/$component" if [[ -L "$current" || ( -e "$current" && ! -d "$current" ) ]]; then echo "$current" return 1 fi + if [[ "$remaining" == */* ]]; then + remaining="${remaining#*/}" + else + remaining="" + fi done while [[ -n "$parent_path" ]]; do @@ -75,6 +161,11 @@ destination_ancestors_are_safe() { parent_path="" fi done + + # Explicit success: the loops above may not run at all, and without this the + # function would return whatever status the last `[[ ]]` test happened to + # leave behind. + return 0 } if [[ ! -e "$SOURCE_DIR" ]]; then @@ -86,11 +177,76 @@ if [[ ! -d "$SOURCE_DIR" ]]; then exit 1 fi +# Place $1 at $2 without ever replacing an existing destination entry. +# +# `mv` is unsafe here: the earlier `[[ -f ]]`/`[[ -L ]]` tests are only a +# preflight, and the Dispatch server keeps writing media (browser-extension +# screenshots, whiteboard snapshots) while this runs, so a destination file +# can appear in the window between the test and the move — and `mv` would +# silently overwrite it. link(2) is atomic and fails with EEXIST instead, +# including when the destination is a symlink, which it never follows. +# +# Exit status: 0 placed, 1 destination appeared (treat as a conflict), +# 2 the copy itself failed. +place_file() { + local src="$1" + local dst="$2" + local tmp + + if ln "$src" "$dst" 2>/dev/null; then + rm -f "$src" + return 0 + fi + + # Either the destination raced into existence, or source and destination + # live on different filesystems (hard links cannot span them, which happens + # when the install tree and $HOME are separate mounts). + if [[ -e "$dst" || -L "$dst" ]]; then + return 1 + fi + + # Cross-device: stage a private copy inside the destination directory, then + # claim the final name with the same atomic no-clobber link. + tmp="$(mktemp "$(dirname "$dst")/.migrate-legacy-media.XXXXXX")" || return 2 + if ! cp -p "$src" "$tmp"; then + rm -f "$tmp" + return 2 + fi + if ln "$tmp" "$dst" 2>/dev/null; then + rm -f "$tmp" "$src" + return 0 + fi + rm -f "$tmp" + return 1 +} + moved=0 duplicates=0 conflicts=0 unsupported=0 +# Enumerate up front rather than streaming `find` through a process +# substitution: there the parent shell never sees find's exit status, so an +# unreadable subdirectory prints a "Permission denied" line to stderr and the +# run still ends with a zero-conflict summary and exit 0. The manifest gates +# `--apply` on exactly that summary, so a partial scan must be a hard failure +# rather than a clean-looking result. +SCAN_DIR="$(mktemp -d "${TMPDIR:-/tmp}/migrate-legacy-media.XXXXXX")" +trap 'rm -rf "$SCAN_DIR"' EXIT + +REGULAR_LIST="$SCAN_DIR/regular" +SPECIAL_LIST="$SCAN_DIR/special" + +scan_failed() { + echo "error: could not fully scan $SOURCE_DIR (see the find errors above)." >&2 + echo "Refusing to report a result from an incomplete scan; fix the unreadable entries and rerun." >&2 + exit 1 +} + +find "$SOURCE_DIR" -type f -print0 >"$REGULAR_LIST" || scan_failed +find "$SOURCE_DIR" \( -type l -o -type p -o -type s -o -type b -o -type c \) \ + -print0 >"$SPECIAL_LIST" || scan_failed + while IFS= read -r -d '' source_file; do relative_path="${source_file#"$SOURCE_DIR"/}" destination_file="$DESTINATION_DIR/$relative_path" @@ -125,20 +281,35 @@ while IFS= read -r -d '' source_file; do continue fi - moved=$((moved + 1)) - if [[ "$MODE" == "apply" ]]; then - mkdir -p "$(dirname "$destination_file")" - mv "$source_file" "$destination_file" - echo "moved: $relative_path" - else + if [[ "$MODE" != "apply" ]]; then + moved=$((moved + 1)) echo "would move: $relative_path" + continue fi -done < <(find "$SOURCE_DIR" -type f -print0) + + mkdir -p "$(dirname "$destination_file")" + place_status=0 + place_file "$source_file" "$destination_file" || place_status=$? + case "$place_status" in + 0) + moved=$((moved + 1)) + echo "moved: $relative_path" + ;; + 1) + conflicts=$((conflicts + 1)) + echo "conflict (destination appeared during migration, left untouched): $relative_path" >&2 + ;; + *) + conflicts=$((conflicts + 1)) + echo "conflict (could not copy across filesystems, left untouched): $relative_path" >&2 + ;; + esac +done <"$REGULAR_LIST" while IFS= read -r -d '' unsupported_path; do unsupported=$((unsupported + 1)) echo "unsupported entry (left untouched): ${unsupported_path#"$SOURCE_DIR"/}" >&2 -done < <(find "$SOURCE_DIR" \( -type l -o -type p -o -type s -o -type b -o -type c \) -print0) +done <"$SPECIAL_LIST" if [[ "$MODE" == "apply" && "$conflicts" -eq 0 && "$unsupported" -eq 0 ]]; then find "$SOURCE_DIR" -depth -type d -empty -delete diff --git a/release-notes/next-assisted-update.json b/release-notes/next-assisted-update.json index ded5e0854..41aba571a 100644 --- a/release-notes/next-assisted-update.json +++ b/release-notes/next-assisted-update.json @@ -1,12 +1,12 @@ { "mode": "required", "title": "Migrate shared media from legacy tilde paths", - "summary": "This release corrects shared-media storage paths configured with ~/.dispatch/media. The required assisted update safely moves recoverable historical files from the old literal-tilde directory into the correct home media directory, preserving conflicts for manual recovery.", - "instructions": "1. Use the managed update flow to install the target release and wait for health.\n2. Identify and stop every active agent that still has a legacy literal-tilde media path or was launched before this release, even if the old directory is absent; a retained agent environment can recreate it. If any cannot be safely stopped, block the update before moving files.\n3. From the Dispatch install directory, run bin/migrate-legacy-media --dry-run.\n4. If the dry run reports no conflicts or unsupported entries, run bin/migrate-legacy-media --apply.\n5. If any conflict is reported, do not overwrite either file; report the affected path for manual recovery.\n6. Restart only the agents stopped for migration after it succeeds, including after a no-op migration, then confirm the legacy directory is empty or absent, the health endpoint is healthy, and release.json reports the target tag.", + "summary": "This release corrects shared-media storage paths on installs whose MEDIA_ROOT is configured with a leading tilde (for example ~/.dispatch/media). The assisted update safely moves recoverable historical files from the old literal-tilde directory into the location the fixed runtime reads, preserving conflicts for manual recovery. Installs with an absolute MEDIA_ROOT were never affected and the migration is a no-op for them.", + "instructions": "1. Read MEDIA_ROOT from /.env. If it is absent or does not begin with `~`, this install was never affected: report the no-op, do not stop any agents, and skip to the health/version checks.\n2. Use the managed update flow to install the target release and wait for health.\n3. On a tilde-configured install, stop every active agent cleanly after the target service is healthy — all of them, not a subset, because a running agent retains a literal-tilde DISPATCH_MEDIA_DIR that can recreate the legacy tree after the move. If any cannot be safely stopped, block the update before moving files.\n4. From the Dispatch install directory, run bin/migrate-legacy-media --dry-run. It reports which media root it resolved and from where. Exit status 1 means the scan was incomplete (for example an unreadable directory) — block and fix that first rather than treating it as a clean result.\n5. If the dry run reports no conflicts or unsupported entries, run bin/migrate-legacy-media --apply.\n6. If any conflict is reported, do not overwrite either file; report the affected path for manual recovery.\n7. Restart only the agents stopped for migration after it succeeds, including after a no-op migration, then confirm the legacy directory is empty or absent, the health endpoint is healthy, and release.json reports the target tag.", "requiredChecks": [ "service_restarted", "health_endpoint", "version_converged" ], - "rollbackGuidance": "If the target service is unhealthy, roll back using the normal Dispatch rollback flow. Do not delete media from either location: the migrator never overwrites destination files, and it is safe to rerun after recovery." + "rollbackGuidance": "Rollback is unrestricted before bin/migrate-legacy-media --apply runs: nothing has moved. After --apply, a plain rollback is NOT safe — the pre-fix runtime resolves `~` literally again while the migrated files exist only under the expanded home path, so the service passes its health check with every migrated file unreadable. Prefer resolving forward on the fixed release. If rolling back after a move is unavoidable, move the migrated files back into the literal-tilde tree under the install directory before starting the old binary. Do not delete media from either location: the migrator never overwrites destination files, and it is safe to rerun after recovery." } diff --git a/update-migrations/0012-legacy-media-paths.yaml b/update-migrations/0012-legacy-media-paths.yaml index b7b7290a0..28b4ff239 100644 --- a/update-migrations/0012-legacy-media-paths.yaml +++ b/update-migrations/0012-legacy-media-paths.yaml @@ -3,35 +3,51 @@ title: Migrate shared media from the legacy literal-tilde directory summary: > Earlier Dispatch versions accepted MEDIA_ROOT values such as ~/.dispatch/media without expanding the tilde. Those installs stored shared - media beneath a literal ~ directory inside the Dispatch install. This - migration moves that legacy tree into the correct home-relative media - directory so historical dispatch_list_media entries remain readable after - the path-resolution fix. + media beneath a literal `~` directory inside the Dispatch install. This + migration moves that legacy tree to the location the fixed runtime reads, so + historical dispatch_list_media entries remain readable after the + path-resolution fix. Installs whose MEDIA_ROOT is an absolute path were + never affected and this migration is a no-op for them. alreadySatisfied: description: > - The legacy directory /~/.dispatch/media does not exist, - or it is empty after migration; no active agent has a stored media_dir - beginning with ~/ or a session launched before this release; and the - current service is healthy on the target tag. An empty or absent legacy - directory alone is insufficient because an already-running agent can - recreate it using its retained literal-tilde environment. + Either the install's effective MEDIA_ROOT does not begin with `~` — in + which case it never wrote to a literal-tilde path and nothing needs to + happen — or the legacy directory derived from that MEDIA_ROOT is absent or + empty, no active agent has a stored media_dir beginning with `~`, and the + current service is healthy on the target tag. Determine MEDIA_ROOT from + /.env (the file the service loads); `bin/migrate-legacy-media + --dry-run` reports which root it resolved and from where. instructions: - - During inspect, independently check whether - /~/.dispatch/media exists and identify every active agent - whose stored media_dir begins with ~/ or whose session was launched before - this release. Record affected agents even when the legacy directory is - absent, because their retained environment can create it later. + - > + First determine the install's effective MEDIA_ROOT by reading MEDIA_ROOT + from /.env. If it is absent or does not begin with `~`, + this migration is an explicit no-op: report that the install was never + affected, do NOT stop any agents, and continue with the rest of the update + plan. Only tilde-configured installs need the steps below. + - > + During inspect on a tilde-configured install, check whether the legacy + directory / exists and list + every active agent. Running `bin/migrate-legacy-media --dry-run` is the + supported way to resolve the media root and see the legacy tree's contents + without moving anything. - If alreadySatisfied is false, use the managed update endpoint to install the target release and wait for the service to become healthy. - - After the target service is healthy and regardless of whether the legacy - directory exists, stop every affected active agent cleanly so no process - continues writing to the literal-tilde tree. If an agent cannot be safely - stopped, report the update as blocked and do not run the file migration. + - > + After the target service is healthy, stop every active agent on this + install cleanly. Stop them all rather than trying to identify which ones + predate the fix: a still-running agent holds a literal-tilde + DISPATCH_MEDIA_DIR in its retained environment and can recreate the legacy + tree after the move, and there is no reliable way to inspect a live + agent's launch-time environment. If an agent cannot be safely stopped, + report the update as blocked and do not run the file migration. - From the Dispatch install directory, run bin/migrate-legacy-media --dry-run and inspect its summary before moving - anything. Do not manually overwrite destination files. + anything. Do not manually overwrite destination files. Exit status 1 means + the scan itself was incomplete (for example an unreadable directory) — + treat that as blocked and fix it before proceeding, because a partial scan + can otherwise look like a clean result. - Run bin/migrate-legacy-media --apply only when the dry run reports zero conflicts and zero unsupported entries. The command is idempotent and retains conflicting source files for manual recovery. @@ -51,9 +67,24 @@ validation: - version_converged rollback: - - If the target service does not return healthy, roll back to the previous - healthy release using the normal Dispatch rollback flow. - - If media migration has started, leave both the corrected destination files - and any unresolved source files in place; the migration never overwrites - destination data, so rerunning it is safe after recovery. + - > + Before bin/migrate-legacy-media --apply has run, rollback is unrestricted: + no files have moved, so the previous release resolves media exactly as it + did before. Roll back using the normal Dispatch rollback flow if the target + service does not return healthy. + - > + After --apply has moved files, a plain rollback is NOT safe. The + pre-fix runtime resolves `~` literally again, while the migrated files now + exist only under the expanded home path — the service will pass its health + check while every migrated media file reads as missing. Prefer staying on + the fixed release and resolving the failure forward. + - > + If rolling back after a move is unavoidable, first move the migrated files + back into the literal-tilde tree under the install directory before + starting the old binary, so the pre-fix runtime finds them where it + expects. Never delete either copy while doing so. + - If media migration has started and files remain in both places, leave both + the corrected destination files and any unresolved source files in place; + the migration never overwrites destination data, so rerunning it is safe + after recovery. - Restart the prior service and confirm its health endpoint returns status=ok. From e6cc5851a0b16268908edf46838f3a85e300eaf4 Mon Sep 17 00:00:00 2001 From: Brad Harris Date: Thu, 20 Aug 2026 08:33:23 -0600 Subject: [PATCH 3/7] fix: fail closed on ancestor swaps and service-level MEDIA_ROOT overrides MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 review findings on the legacy media migrator. Ancestor traversal now fails closed. The previous round made the final component atomic via link(2), but a path re-resolves on every syscall, so an ancestor swapped for a symlink after the preflight still redirected the write outside the media tree (reproduced: the file landed outside and the source was deleted). Placement now `cd -P`s into the destination directory once and links under a single-component name — the shell's cwd is a kernel-held directory reference, so a later swap of the parent path cannot redirect it — and the containment check runs after the cd on the physical path actually reached, so a swap that already happened is refused rather than followed. MEDIA_ROOT is no longer classified from .env alone. dotenv does not override a value already present in the process environment, so a systemd Environment= or launchd EnvironmentVariables entry is what the service actually ran with; the script now reads those first. And a literal-tilde tree that the readable config cannot explain is treated as evidence of an override this script cannot see: it exits 1 asking for --media-root instead of silently declaring the install unaffected and stranding the files. Co-Authored-By: Claude Opus 5 --- apps/server/test/migrate-legacy-media.test.ts | 85 +++++++++ bin/migrate-legacy-media | 175 ++++++++++++++---- release-notes/next-assisted-update.json | 2 +- .../0012-legacy-media-paths.yaml | 26 ++- 4 files changed, 247 insertions(+), 41 deletions(-) diff --git a/apps/server/test/migrate-legacy-media.test.ts b/apps/server/test/migrate-legacy-media.test.ts index 64188cd4d..ea5b6cb69 100644 --- a/apps/server/test/migrate-legacy-media.test.ts +++ b/apps/server/test/migrate-legacy-media.test.ts @@ -289,4 +289,89 @@ describe("migrate-legacy-media", () => { await expect(readFile(destination, "utf8")).resolves.toBe("raced in"); await expect(readFile(source, "utf8")).resolves.toBe("legacy report"); }); + + it("prefers a service-level MEDIA_ROOT over the .env value", async () => { + const fixture = await createFixture(); + // dotenv does not override a variable already in the process environment, + // so a systemd Environment= line is what the service actually ran with. + await writeEnvFile(fixture.root, "MEDIA_ROOT=/var/lib/dispatch/media\n"); + const unitDir = path.join(fixture.home, ".config", "systemd", "user"); + await mkdir(unitDir, { recursive: true }); + await writeFile( + path.join(unitDir, "dispatch.service"), + "[Service]\nEnvironment=MEDIA_ROOT=~/svc-media\nExecStart=/x\n" + ); + const source = path.join(fixture.root, "~", "svc-media", "agt_1", "s.png"); + const destination = path.join(fixture.home, "svc-media", "agt_1", "s.png"); + await mkdir(path.dirname(source), { recursive: true }); + await writeFile(source, "service shot"); + + const applied = await runMigration(fixture.script, fixture.home, "--apply"); + expect(applied.stdout).toContain("moved: agt_1/s.png"); + await expect(readFile(destination, "utf8")).resolves.toBe("service shot"); + }); + + it("refuses to classify an install as unaffected while a legacy tree exists", async () => { + const fixture = await createFixture(); + await writeEnvFile(fixture.root, "MEDIA_ROOT=/var/lib/dispatch/media\n"); + // The readable config says absolute, but files under a literal `~` prove + // the running service used something else. Guessing "no-op" here would + // silently strand them. + const source = path.join(fixture.root, "~", "mystery", "agt_1", "o.png"); + await mkdir(path.dirname(source), { recursive: true }); + await writeFile(source, "orphan"); + + const result = await runMigration( + fixture.script, + fixture.home, + "--apply" + ).then( + () => null, + (err: { code?: number; stderr?: string }) => err + ); + expect(result?.code).toBe(1); + expect(result?.stderr).toContain("--media-root"); + await expect(readFile(source, "utf8")).resolves.toBe("orphan"); + }); + + it("fails closed when a destination ancestor is swapped mid-migration", async () => { + const fixture = await createFixture(); + const outside = await mkdtemp(path.join(os.tmpdir(), "dispatch-outside-")); + tempDirs.push(outside); + const source = path.join(fixture.sourceDir, "agt_1", "report.pdf"); + const agentDir = path.join(fixture.destinationDir, "agt_1"); + await mkdir(path.dirname(source), { recursive: true }); + await mkdir(fixture.destinationDir, { recursive: true }); + await writeFile(source, "legacy report"); + + // Swap the agent directory for a symlink *after* the ancestor preflight, + // in the window a path-based check cannot cover. Placement holds a kernel + // directory reference and verifies where it landed, so this must not write + // through the symlink. + const shimDir = await mkdtemp(path.join(os.tmpdir(), "dispatch-shim-")); + tempDirs.push(shimDir); + const shim = path.join(shimDir, "mkdir"); + await writeFile( + shim, + [ + "#!/bin/bash", + '/bin/mkdir "$@"', + `if [[ -d ${JSON.stringify(agentDir)} && ! -L ${JSON.stringify(agentDir)} ]]; then`, + ` /bin/rmdir ${JSON.stringify(agentDir)} 2>/dev/null && /bin/ln -s ${JSON.stringify(outside)} ${JSON.stringify(agentDir)}`, + "fi", + "", + ].join("\n") + ); + await chmod(shim, 0o755); + + await expect( + runMigration(fixture.script, fixture.home, "--apply", { + env: { PATH: `${shimDir}:${process.env.PATH ?? ""}` }, + }) + ).rejects.toMatchObject({ code: 2 }); + await expect(readFile(source, "utf8")).resolves.toBe("legacy report"); + await expect( + readFile(path.join(outside, "report.pdf"), "utf8") + ).rejects.toMatchObject({ code: "ENOENT" }); + }); }); diff --git a/bin/migrate-legacy-media b/bin/migrate-legacy-media index 0b65dc44e..eab88bcd3 100755 --- a/bin/migrate-legacy-media +++ b/bin/migrate-legacy-media @@ -78,6 +78,44 @@ media_root_from_env_file() { printf '%s' "$value" } +# A service-level MEDIA_ROOT (systemd Environment=, launchd +# EnvironmentVariables, docker -e) beats the .env file: the runtime loads .env +# through dotenv, which does not override a variable already present in the +# process environment. The operator usually runs this script from a plain +# shell that never saw that value, so .env alone cannot be trusted to describe +# the effective configuration. +media_root_from_service_definition() { + local unit="$HOME/.config/systemd/user/dispatch.service" + local plist="$HOME/Library/LaunchAgents/com.dispatch.server.plist" + local line value + + if [[ -r "$unit" ]]; then + line="$(grep -E '^[[:space:]]*Environment=.*MEDIA_ROOT=' "$unit" | tail -1)" || line="" + if [[ -n "$line" ]]; then + value="${line#*MEDIA_ROOT=}" + value="${value%%[[:space:]]*}" + value="${value%\"}" + value="${value#\"}" + printf '%s' "$value" + return 0 + fi + fi + + if [[ -r "$plist" ]]; then + # MEDIA_ROOTVALUE, with or without newlines. + value="$(tr '\n' ' ' <"$plist" \ + | grep -o 'MEDIA_ROOT[[:space:]]*[^<]*' \ + | tail -1 \ + | sed -e 's|.*||' -e 's|||')" || value="" + if [[ -n "$value" ]]; then + printf '%s' "$value" + return 0 + fi + fi + + return 0 +} + CONFIGURED_MEDIA_ROOT="" CONFIGURED_SOURCE="default" if [[ -n "$MEDIA_ROOT_OVERRIDE" ]]; then @@ -87,11 +125,35 @@ elif [[ -n "${MEDIA_ROOT:-}" ]]; then CONFIGURED_MEDIA_ROOT="$MEDIA_ROOT" CONFIGURED_SOURCE="MEDIA_ROOT environment variable" else - CONFIGURED_MEDIA_ROOT="$(media_root_from_env_file)" - [[ -n "$CONFIGURED_MEDIA_ROOT" ]] && CONFIGURED_SOURCE="$ROOT_DIR/.env" + CONFIGURED_MEDIA_ROOT="$(media_root_from_service_definition)" + if [[ -n "$CONFIGURED_MEDIA_ROOT" ]]; then + CONFIGURED_SOURCE="the service definition" + else + CONFIGURED_MEDIA_ROOT="$(media_root_from_env_file)" + [[ -n "$CONFIGURED_MEDIA_ROOT" ]] && CONFIGURED_SOURCE="$ROOT_DIR/.env" + fi fi +# Physical evidence overrides configuration guesses. If a literal-tilde tree +# exists under the install directory, this install *did* write through the +# broken path at some point, whatever the config we managed to read now says. +# Refuse to classify it as unaffected rather than silently skipping real files. +legacy_tree_present() { + local candidate + for candidate in "$ROOT_DIR"/~/*; do + [[ -e "$candidate" ]] && return 0 + done + return 1 +} + if [[ -n "$CONFIGURED_MEDIA_ROOT" && "$CONFIGURED_MEDIA_ROOT" != "~" && "$CONFIGURED_MEDIA_ROOT" != "~/"* ]]; then + if legacy_tree_present; then + echo "error: MEDIA_ROOT reads as $CONFIGURED_MEDIA_ROOT (from $CONFIGURED_SOURCE)," >&2 + echo "but a literal-tilde tree exists at $ROOT_DIR/~ — the effective media root" >&2 + echo "used by the running service must have differed. Re-run with --media-root" >&2 + echo "set to the value that service actually used." >&2 + exit 1 + fi echo "MEDIA_ROOT is $CONFIGURED_MEDIA_ROOT (from $CONFIGURED_SOURCE), which has no leading tilde." echo "This install never wrote media to a literal-tilde path; nothing to migrate." exit 0 @@ -119,6 +181,15 @@ fi DESTINATION_RELATIVE="${DESTINATION_DIR#"$HOME"}" DESTINATION_RELATIVE="${DESTINATION_RELATIVE#/}" +# Physical root of the media tree, resolved once. Only $HOME is resolved +# through symlinks — it can legitimately be one (/home -> /var/home). The +# components below it are deliberately NOT resolved: destination_ancestors_are_safe +# rejects a symlink among them, so resolving here would launder exactly the +# case that check exists to catch. +HOME_PHYSICAL="$(cd -P "$HOME" && pwd -P)" +DESTINATION_PHYSICAL="$HOME_PHYSICAL" +[[ -n "$DESTINATION_RELATIVE" ]] && DESTINATION_PHYSICAL="$HOME_PHYSICAL/$DESTINATION_RELATIVE" + destination_ancestors_are_safe() { local relative_path="$1" local parent_path="" @@ -177,49 +248,80 @@ if [[ ! -d "$SOURCE_DIR" ]]; then exit 1 fi -# Place $1 at $2 without ever replacing an existing destination entry. +# Place $1 (absolute source path) inside destination directory $2 under the +# single-component name $3, without ever replacing an existing entry and +# without ever writing outside the media tree. # -# `mv` is unsafe here: the earlier `[[ -f ]]`/`[[ -L ]]` tests are only a -# preflight, and the Dispatch server keeps writing media (browser-extension -# screenshots, whiteboard snapshots) while this runs, so a destination file -# can appear in the window between the test and the move — and `mv` would -# silently overwrite it. link(2) is atomic and fails with EEXIST instead, -# including when the destination is a symlink, which it never follows. +# Two hazards, both handled here rather than by a preflight test: +# +# * `mv` silently overwrites. The Dispatch server keeps writing media +# (browser-extension screenshots, whiteboard snapshots) while this runs, so +# a destination file can appear between any check and the move. link(2) is +# atomic and fails with EEXIST instead, and never follows a symlink at the +# destination. +# +# * A path re-resolves on every syscall, so checking the ancestors and then +# writing through the same path string is not sound — a component can be +# swapped for a symlink in between. Instead `cd -P` into the destination +# directory once and link under a single-component name: the shell's cwd is +# a kernel-held directory reference, so a later swap of the parent path +# cannot redirect the write. The containment check runs after the cd, on +# the physical path we actually landed in, so a swap that already happened +# fails closed here. # # Exit status: 0 placed, 1 destination appeared (treat as a conflict), -# 2 the copy itself failed. +# 2 the copy itself failed, 3 the destination escaped the media tree. place_file() { local src="$1" - local dst="$2" - local tmp + local dst_dir="$2" + local base="$3" + local status - if ln "$src" "$dst" 2>/dev/null; then - rm -f "$src" - return 0 - fi + ( + cd -P "$dst_dir" 2>/dev/null || exit 3 - # Either the destination raced into existence, or source and destination - # live on different filesystems (hard links cannot span them, which happens - # when the install tree and $HOME are separate mounts). - if [[ -e "$dst" || -L "$dst" ]]; then - return 1 - fi + landed="$(pwd -P)" + case "$landed" in + "$DESTINATION_PHYSICAL"|"$DESTINATION_PHYSICAL"/*) ;; + *) exit 3 ;; + esac + + if ln "$src" "$base" 2>/dev/null; then + exit 0 + fi + + # Either the destination raced into existence, or source and destination + # live on different filesystems (hard links cannot span them, which + # happens when the install tree and $HOME are separate mounts). + if [[ -e "$base" || -L "$base" ]]; then + exit 1 + fi - # Cross-device: stage a private copy inside the destination directory, then - # claim the final name with the same atomic no-clobber link. - tmp="$(mktemp "$(dirname "$dst")/.migrate-legacy-media.XXXXXX")" || return 2 - if ! cp -p "$src" "$tmp"; then + # Cross-device: stage a private copy inside the destination directory, + # then claim the final name with the same atomic no-clobber link. The + # staging file is created relative to the held cwd too. + tmp="$(mktemp ./.migrate-legacy-media.XXXXXX)" || exit 2 + if ! cp -p "$src" "$tmp"; then + rm -f "$tmp" + exit 2 + fi + if ln "$tmp" "$base" 2>/dev/null; then + rm -f "$tmp" + exit 0 + fi rm -f "$tmp" - return 2 - fi - if ln "$tmp" "$dst" 2>/dev/null; then - rm -f "$tmp" "$src" - return 0 + exit 1 + ) + status=$? + + # The source is only dropped once the destination is definitely in place. + if [[ "$status" -eq 0 ]]; then + rm -f "$src" fi - rm -f "$tmp" - return 1 + return "$status" } + moved=0 duplicates=0 conflicts=0 @@ -289,7 +391,8 @@ while IFS= read -r -d '' source_file; do mkdir -p "$(dirname "$destination_file")" place_status=0 - place_file "$source_file" "$destination_file" || place_status=$? + place_file "$source_file" "$(dirname "$destination_file")" \ + "$(basename "$destination_file")" || place_status=$? case "$place_status" in 0) moved=$((moved + 1)) @@ -299,6 +402,10 @@ while IFS= read -r -d '' source_file; do conflicts=$((conflicts + 1)) echo "conflict (destination appeared during migration, left untouched): $relative_path" >&2 ;; + 3) + conflicts=$((conflicts + 1)) + echo "conflict (destination escaped the media tree, left untouched): $relative_path" >&2 + ;; *) conflicts=$((conflicts + 1)) echo "conflict (could not copy across filesystems, left untouched): $relative_path" >&2 diff --git a/release-notes/next-assisted-update.json b/release-notes/next-assisted-update.json index 41aba571a..a4c1f969b 100644 --- a/release-notes/next-assisted-update.json +++ b/release-notes/next-assisted-update.json @@ -2,7 +2,7 @@ "mode": "required", "title": "Migrate shared media from legacy tilde paths", "summary": "This release corrects shared-media storage paths on installs whose MEDIA_ROOT is configured with a leading tilde (for example ~/.dispatch/media). The assisted update safely moves recoverable historical files from the old literal-tilde directory into the location the fixed runtime reads, preserving conflicts for manual recovery. Installs with an absolute MEDIA_ROOT were never affected and the migration is a no-op for them.", - "instructions": "1. Read MEDIA_ROOT from /.env. If it is absent or does not begin with `~`, this install was never affected: report the no-op, do not stop any agents, and skip to the health/version checks.\n2. Use the managed update flow to install the target release and wait for health.\n3. On a tilde-configured install, stop every active agent cleanly after the target service is healthy — all of them, not a subset, because a running agent retains a literal-tilde DISPATCH_MEDIA_DIR that can recreate the legacy tree after the move. If any cannot be safely stopped, block the update before moving files.\n4. From the Dispatch install directory, run bin/migrate-legacy-media --dry-run. It reports which media root it resolved and from where. Exit status 1 means the scan was incomplete (for example an unreadable directory) — block and fix that first rather than treating it as a clean result.\n5. If the dry run reports no conflicts or unsupported entries, run bin/migrate-legacy-media --apply.\n6. If any conflict is reported, do not overwrite either file; report the affected path for manual recovery.\n7. Restart only the agents stopped for migration after it succeeds, including after a no-op migration, then confirm the legacy directory is empty or absent, the health endpoint is healthy, and release.json reports the target tag.", + "instructions": "1. Determine the effective MEDIA_ROOT by running bin/migrate-legacy-media --dry-run, which checks the service definition (systemd Environment= / launchd EnvironmentVariables) before /.env and reports which source it used. Do not read .env alone — dotenv does not override a value already set in the service process environment. If the resolved root does not begin with `~`, this install was never affected: report the no-op, do not stop any agents, and skip to the health/version checks. If the command exits 1 asking for --media-root, a legacy tree exists that the readable config does not explain; find the value the service actually ran with and rerun with --media-root rather than treating it as a no-op.\n2. Use the managed update flow to install the target release and wait for health.\n3. On a tilde-configured install, stop every active agent cleanly after the target service is healthy — all of them, not a subset, because a running agent retains a literal-tilde DISPATCH_MEDIA_DIR that can recreate the legacy tree after the move. If any cannot be safely stopped, block the update before moving files.\n4. From the Dispatch install directory, run bin/migrate-legacy-media --dry-run. It reports which media root it resolved and from where. Exit status 1 means the scan was incomplete (for example an unreadable directory) — block and fix that first rather than treating it as a clean result.\n5. If the dry run reports no conflicts or unsupported entries, run bin/migrate-legacy-media --apply.\n6. If any conflict is reported, do not overwrite either file; report the affected path for manual recovery.\n7. Restart only the agents stopped for migration after it succeeds, including after a no-op migration, then confirm the legacy directory is empty or absent, the health endpoint is healthy, and release.json reports the target tag.", "requiredChecks": [ "service_restarted", "health_endpoint", diff --git a/update-migrations/0012-legacy-media-paths.yaml b/update-migrations/0012-legacy-media-paths.yaml index 28b4ff239..c97c01448 100644 --- a/update-migrations/0012-legacy-media-paths.yaml +++ b/update-migrations/0012-legacy-media-paths.yaml @@ -15,17 +15,31 @@ alreadySatisfied: which case it never wrote to a literal-tilde path and nothing needs to happen — or the legacy directory derived from that MEDIA_ROOT is absent or empty, no active agent has a stored media_dir beginning with `~`, and the - current service is healthy on the target tag. Determine MEDIA_ROOT from - /.env (the file the service loads); `bin/migrate-legacy-media - --dry-run` reports which root it resolved and from where. + current service is healthy on the target tag. Do not read MEDIA_ROOT from + /.env alone: the runtime loads that file through dotenv, + which does not override a value already set in the service process + environment (systemd Environment=, launchd EnvironmentVariables, docker + -e). Run `bin/migrate-legacy-media --dry-run`, which checks the service + definition before .env and reports which root it resolved and from where. instructions: - > - First determine the install's effective MEDIA_ROOT by reading MEDIA_ROOT - from /.env. If it is absent or does not begin with `~`, - this migration is an explicit no-op: report that the install was never + First determine the install's effective MEDIA_ROOT. Run + `bin/migrate-legacy-media --dry-run`: it resolves the root from the service + definition (systemd Environment= / launchd EnvironmentVariables) before + falling back to /.env, and reports which source it used. + Do not classify from .env alone — dotenv does not override a value already + present in the service process environment, so .env can disagree with what + the service actually ran with. If the resolved root does not begin with + `~`, this migration is an explicit no-op: report that the install was never affected, do NOT stop any agents, and continue with the rest of the update plan. Only tilde-configured installs need the steps below. + - > + If the command exits 1 asking for --media-root, it found a literal-tilde + tree that the configuration it could read does not explain — the service + ran with an override this script cannot see. Determine the value that + service actually used (its unit, plist, or container definition) and rerun + with --media-root set to it. Do not treat this as a no-op. - > During inspect on a tilde-configured install, check whether the legacy directory / exists and list From 86cf16f2a9ca12818e7013129291295a78c40847 Mon Sep 17 00:00:00 2001 From: Brad Harris Date: Thu, 20 Aug 2026 08:40:20 -0600 Subject: [PATCH 4/7] fix: resolve MEDIA_ROOT from systemd's effective environment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hand-parsing the unit file missed two sources systemd merges itself: dispatch.service.d/*.conf drop-ins and EnvironmentFile=. A tilde root in either one, with an absolute .env and no legacy tree on disk yet, produced a false no-op — and a still-running agent would then recreate the tree. Ask systemd instead of reimplementing its merge: read the live process environment via MainPID when the service is running, else `systemctl show` for the merged Environment= (drop-ins included) and the EnvironmentFiles it reports. launchd needs no equivalent — its plist has no drop-in mechanism. When a systemd service exists but cannot be interrogated, exit 1 asking for --media-root rather than falling back to .env: physical evidence cannot catch a wrong verdict here, because the legacy tree does not exist yet. Two bugs in the previous parser fixed along the way: a no-match `grep` tripped pipefail and aborted the lookup, and the status global was assigned inside a command substitution so it never reached the caller. Drops the unit-file-parsing test, whose fixture no longer describes reachable behavior: a unit present without a usable systemctl is genuinely unresolvable and now fails closed. Its intent is covered by the drop-in test. Co-Authored-By: Claude Opus 5 --- apps/server/test/migrate-legacy-media.test.ts | 151 +++++++++++++++--- bin/migrate-legacy-media | 147 ++++++++++++++--- release-notes/next-assisted-update.json | 2 +- .../0012-legacy-media-paths.yaml | 23 +-- 4 files changed, 266 insertions(+), 57 deletions(-) diff --git a/apps/server/test/migrate-legacy-media.test.ts b/apps/server/test/migrate-legacy-media.test.ts index ea5b6cb69..1dfde8e8f 100644 --- a/apps/server/test/migrate-legacy-media.test.ts +++ b/apps/server/test/migrate-legacy-media.test.ts @@ -65,6 +65,38 @@ async function runMigration( }); } +/** + * Stub `systemctl` on PATH so the effective-environment lookup can be driven + * without a real systemd. Mirrors the properties the script queries. + */ +async function stubSystemd( + home: string, + properties: { environment?: string; environmentFiles?: string } +) { + const unitDir = path.join(home, ".config", "systemd", "user"); + await mkdir(unitDir, { recursive: true }); + await writeFile( + path.join(unitDir, "dispatch.service"), + "[Service]\nExecStart=/x\n" + ); + const shimDir = await mkdtemp(path.join(os.tmpdir(), "dispatch-systemctl-")); + tempDirs.push(shimDir); + await writeFile( + path.join(shimDir, "systemctl"), + [ + "#!/bin/bash", + 'case "$*" in', + " *MainPID*) echo 0 ;;", + ` *EnvironmentFiles*) echo ${JSON.stringify(properties.environmentFiles ?? "")} ;;`, + ` *Environment*) echo ${JSON.stringify(properties.environment ?? "")} ;;`, + "esac", + "", + ].join("\n") + ); + await chmod(path.join(shimDir, "systemctl"), 0o755); + return shimDir; +} + /** Write MEDIA_ROOT into the install .env the way the service reads it. */ async function writeEnvFile(root: string, contents: string) { await writeFile(path.join(root, ".env"), contents); @@ -290,27 +322,6 @@ describe("migrate-legacy-media", () => { await expect(readFile(source, "utf8")).resolves.toBe("legacy report"); }); - it("prefers a service-level MEDIA_ROOT over the .env value", async () => { - const fixture = await createFixture(); - // dotenv does not override a variable already in the process environment, - // so a systemd Environment= line is what the service actually ran with. - await writeEnvFile(fixture.root, "MEDIA_ROOT=/var/lib/dispatch/media\n"); - const unitDir = path.join(fixture.home, ".config", "systemd", "user"); - await mkdir(unitDir, { recursive: true }); - await writeFile( - path.join(unitDir, "dispatch.service"), - "[Service]\nEnvironment=MEDIA_ROOT=~/svc-media\nExecStart=/x\n" - ); - const source = path.join(fixture.root, "~", "svc-media", "agt_1", "s.png"); - const destination = path.join(fixture.home, "svc-media", "agt_1", "s.png"); - await mkdir(path.dirname(source), { recursive: true }); - await writeFile(source, "service shot"); - - const applied = await runMigration(fixture.script, fixture.home, "--apply"); - expect(applied.stdout).toContain("moved: agt_1/s.png"); - await expect(readFile(destination, "utf8")).resolves.toBe("service shot"); - }); - it("refuses to classify an install as unaffected while a legacy tree exists", async () => { const fixture = await createFixture(); await writeEnvFile(fixture.root, "MEDIA_ROOT=/var/lib/dispatch/media\n"); @@ -374,4 +385,102 @@ describe("migrate-legacy-media", () => { readFile(path.join(outside, "report.pdf"), "utf8") ).rejects.toMatchObject({ code: "ENOENT" }); }); + + it("reads MEDIA_ROOT from a systemd drop-in via the merged environment", async () => { + const fixture = await createFixture(); + await writeEnvFile(fixture.root, "MEDIA_ROOT=/var/lib/dispatch/media\n"); + // systemctl already folds dispatch.service.d/*.conf into Environment=, + // which is exactly why the script asks systemd instead of parsing the unit. + const shimDir = await stubSystemd(fixture.home, { + environment: "FOO=1 MEDIA_ROOT=~/dropin-media", + }); + const source = path.join( + fixture.root, + "~", + "dropin-media", + "agt_1", + "d.png" + ); + const destination = path.join( + fixture.home, + "dropin-media", + "agt_1", + "d.png" + ); + await mkdir(path.dirname(source), { recursive: true }); + await writeFile(source, "dropin shot"); + + const applied = await runMigration( + fixture.script, + fixture.home, + "--apply", + { + env: { PATH: `${shimDir}:${process.env.PATH ?? ""}` }, + } + ); + expect(applied.stdout).toContain("moved: agt_1/d.png"); + await expect(readFile(destination, "utf8")).resolves.toBe("dropin shot"); + }); + + it("reads MEDIA_ROOT from a systemd EnvironmentFile", async () => { + const fixture = await createFixture(); + await writeEnvFile(fixture.root, "MEDIA_ROOT=/var/lib/dispatch/media\n"); + const serviceEnv = path.join(fixture.home, "svc.env"); + await writeFile(serviceEnv, "MEDIA_ROOT=~/envfile-media\n"); + const shimDir = await stubSystemd(fixture.home, { + environmentFiles: `${serviceEnv} (ignore_errors=no)`, + }); + const source = path.join( + fixture.root, + "~", + "envfile-media", + "agt_1", + "e.png" + ); + const destination = path.join( + fixture.home, + "envfile-media", + "agt_1", + "e.png" + ); + await mkdir(path.dirname(source), { recursive: true }); + await writeFile(source, "envfile shot"); + + const applied = await runMigration( + fixture.script, + fixture.home, + "--apply", + { + env: { PATH: `${shimDir}:${process.env.PATH ?? ""}` }, + } + ); + expect(applied.stdout).toContain("moved: agt_1/e.png"); + await expect(readFile(destination, "utf8")).resolves.toBe("envfile shot"); + }); + + it("fails closed when a systemd service cannot be interrogated", async () => { + const fixture = await createFixture(); + await writeEnvFile(fixture.root, "MEDIA_ROOT=/var/lib/dispatch/media\n"); + const unitDir = path.join(fixture.home, ".config", "systemd", "user"); + await mkdir(unitDir, { recursive: true }); + await writeFile(path.join(unitDir, "dispatch.service"), "[Service]\n"); + const shimDir = await mkdtemp( + path.join(os.tmpdir(), "dispatch-systemctl-") + ); + tempDirs.push(shimDir); + await writeFile(path.join(shimDir, "systemctl"), "#!/bin/bash\nexit 1\n"); + await chmod(path.join(shimDir, "systemctl"), 0o755); + + // No legacy tree exists yet, so physical evidence cannot catch a wrong + // verdict here — a drop-in or EnvironmentFile tilde root would be missed + // and a running agent would recreate the tree after the "no-op". + const result = await runMigration(fixture.script, fixture.home, "--apply", { + env: { PATH: `${shimDir}:${process.env.PATH ?? ""}` }, + }).then( + () => null, + (err: { code?: number; stderr?: string }) => err + ); + expect(result?.code).toBe(1); + expect(result?.stderr).toContain("--media-root"); + }); }); diff --git a/bin/migrate-legacy-media b/bin/migrate-legacy-media index eab88bcd3..dd499e925 100755 --- a/bin/migrate-legacy-media +++ b/bin/migrate-legacy-media @@ -84,38 +84,118 @@ media_root_from_env_file() { # process environment. The operator usually runs this script from a plain # shell that never saw that value, so .env alone cannot be trusted to describe # the effective configuration. -media_root_from_service_definition() { +# Extract MEDIA_ROOT from a NUL- or newline-separated environment dump on +# stdin. Prints nothing when absent; `grep` finding no match is a normal +# outcome here, not a failure, so it must not trip errexit/pipefail. +media_root_from_environ_dump() { + local line + line="$(tr '\0' '\n' | grep -E '^MEDIA_ROOT=' | tail -1)" || line="" + [[ -n "$line" ]] || return 0 + line="${line#MEDIA_ROOT=}" + line="${line%$'\r'}" + if [[ "$line" == \"*\" || "$line" == \'*\' ]]; then + line="${line:1:${#line}-2}" + fi + printf '%s' "$line" +} + +# Resolve MEDIA_ROOT from systemd's *effective* service environment into the +# globals SERVICE_MEDIA_ROOT / SERVICE_LOOKUP_STATUS. Assigned rather than +# printed: a command substitution would run this in a subshell and the status +# would never reach the caller. +# +# Parsing the unit file by hand is not enough — systemd merges +# dispatch.service.d/*.conf drop-ins and reads EnvironmentFile= at exec time, +# so a tilde root can live in a source a naive parser never opens. Ask systemd +# instead of reimplementing its merge. +# +# SERVICE_LOOKUP_STATUS is one of: +# found — positively established (value in SERVICE_MEDIA_ROOT) +# absent — systemd answered and there is definitively no MEDIA_ROOT +# unknown — systemd is present but could not be interrogated +# no-systemd — no systemd Dispatch service on this host +SERVICE_MEDIA_ROOT="" +SERVICE_LOOKUP_STATUS="no-systemd" + +media_root_from_systemd() { local unit="$HOME/.config/systemd/user/dispatch.service" - local plist="$HOME/Library/LaunchAgents/com.dispatch.server.plist" - local line value + local main_pid environment env_files env_file value - if [[ -r "$unit" ]]; then - line="$(grep -E '^[[:space:]]*Environment=.*MEDIA_ROOT=' "$unit" | tail -1)" || line="" - if [[ -n "$line" ]]; then - value="${line#*MEDIA_ROOT=}" - value="${value%%[[:space:]]*}" - value="${value%\"}" - value="${value#\"}" - printf '%s' "$value" - return 0 - fi + SERVICE_MEDIA_ROOT="" + if [[ ! -e "$unit" ]]; then + SERVICE_LOOKUP_STATUS="no-systemd" + return 0 + fi + if ! command -v systemctl >/dev/null 2>&1; then + SERVICE_LOOKUP_STATUS="unknown" + return 0 fi - if [[ -r "$plist" ]]; then - # MEDIA_ROOTVALUE, with or without newlines. - value="$(tr '\n' ' ' <"$plist" \ - | grep -o 'MEDIA_ROOT[[:space:]]*[^<]*' \ - | tail -1 \ - | sed -e 's|.*||' -e 's|||')" || value="" + # Best source: the live process environment. It is ground truth, whatever + # combination of unit, drop-in, EnvironmentFile or manager environment + # produced it. + main_pid="$(systemctl --user show dispatch.service --property=MainPID --value 2>/dev/null)" || main_pid="" + if [[ -n "$main_pid" && "$main_pid" != "0" && -r "/proc/$main_pid/environ" ]]; then + value="$(media_root_from_environ_dump <"/proc/$main_pid/environ")" || value="" if [[ -n "$value" ]]; then - printf '%s' "$value" - return 0 + SERVICE_MEDIA_ROOT="$value" + SERVICE_LOOKUP_STATUS="found" + else + SERVICE_LOOKUP_STATUS="absent" fi + return 0 + fi + + # Service not running (or no procfs): ask systemd for the merged directives. + # `show` already folds in drop-ins; EnvironmentFile= is reported separately. + if ! environment="$(systemctl --user show dispatch.service --property=Environment --value 2>/dev/null)"; then + SERVICE_LOOKUP_STATUS="unknown" + return 0 + fi + value="$(printf '%s\n' "$environment" | tr ' ' '\n' | media_root_from_environ_dump)" || value="" + if [[ -n "$value" ]]; then + SERVICE_MEDIA_ROOT="$value" + SERVICE_LOOKUP_STATUS="found" + return 0 + fi + + env_files="$(systemctl --user show dispatch.service --property=EnvironmentFiles --value 2>/dev/null)" || env_files="" + if [[ -n "$env_files" ]]; then + # Reported as "/path/to/file (ignore_errors=no)", one per line. + while IFS= read -r env_file; do + env_file="${env_file%% (*}" + if [[ -n "$env_file" && -r "$env_file" ]]; then + value="$(media_root_from_environ_dump <"$env_file")" || value="" + if [[ -n "$value" ]]; then + SERVICE_MEDIA_ROOT="$value" + SERVICE_LOOKUP_STATUS="found" + return 0 + fi + fi + done <MEDIA_ROOT[[:space:]]*[^<]*' \ + | tail -1 \ + | sed -e 's|.*||' -e 's|||')" || value="" + printf '%s' "$value" +} + + CONFIGURED_MEDIA_ROOT="" CONFIGURED_SOURCE="default" if [[ -n "$MEDIA_ROOT_OVERRIDE" ]]; then @@ -125,12 +205,27 @@ elif [[ -n "${MEDIA_ROOT:-}" ]]; then CONFIGURED_MEDIA_ROOT="$MEDIA_ROOT" CONFIGURED_SOURCE="MEDIA_ROOT environment variable" else - CONFIGURED_MEDIA_ROOT="$(media_root_from_service_definition)" - if [[ -n "$CONFIGURED_MEDIA_ROOT" ]]; then - CONFIGURED_SOURCE="the service definition" + media_root_from_systemd + if [[ "$SERVICE_LOOKUP_STATUS" == "found" ]]; then + CONFIGURED_MEDIA_ROOT="$SERVICE_MEDIA_ROOT" + CONFIGURED_SOURCE="the systemd service environment" else - CONFIGURED_MEDIA_ROOT="$(media_root_from_env_file)" - [[ -n "$CONFIGURED_MEDIA_ROOT" ]] && CONFIGURED_SOURCE="$ROOT_DIR/.env" + CONFIGURED_MEDIA_ROOT="$(media_root_from_launchd)" + if [[ -n "$CONFIGURED_MEDIA_ROOT" ]]; then + CONFIGURED_SOURCE="the launchd service definition" + elif [[ "$SERVICE_LOOKUP_STATUS" == "unknown" ]]; then + # systemd is here but would not answer. Its environment can carry a + # tilde root from a drop-in or EnvironmentFile that .env does not show, + # and a "not affected" verdict from .env alone would skip a real + # migration — with no legacy tree on disk yet to catch the mistake. + echo "error: a systemd Dispatch service exists but its effective environment" >&2 + echo "could not be read, so MEDIA_ROOT cannot be established. Determine the" >&2 + echo "value that service runs with and rerun with --media-root set to it." >&2 + exit 1 + else + CONFIGURED_MEDIA_ROOT="$(media_root_from_env_file)" + [[ -n "$CONFIGURED_MEDIA_ROOT" ]] && CONFIGURED_SOURCE="$ROOT_DIR/.env" + fi fi fi diff --git a/release-notes/next-assisted-update.json b/release-notes/next-assisted-update.json index a4c1f969b..c2a0ad2f1 100644 --- a/release-notes/next-assisted-update.json +++ b/release-notes/next-assisted-update.json @@ -2,7 +2,7 @@ "mode": "required", "title": "Migrate shared media from legacy tilde paths", "summary": "This release corrects shared-media storage paths on installs whose MEDIA_ROOT is configured with a leading tilde (for example ~/.dispatch/media). The assisted update safely moves recoverable historical files from the old literal-tilde directory into the location the fixed runtime reads, preserving conflicts for manual recovery. Installs with an absolute MEDIA_ROOT were never affected and the migration is a no-op for them.", - "instructions": "1. Determine the effective MEDIA_ROOT by running bin/migrate-legacy-media --dry-run, which checks the service definition (systemd Environment= / launchd EnvironmentVariables) before /.env and reports which source it used. Do not read .env alone — dotenv does not override a value already set in the service process environment. If the resolved root does not begin with `~`, this install was never affected: report the no-op, do not stop any agents, and skip to the health/version checks. If the command exits 1 asking for --media-root, a legacy tree exists that the readable config does not explain; find the value the service actually ran with and rerun with --media-root rather than treating it as a no-op.\n2. Use the managed update flow to install the target release and wait for health.\n3. On a tilde-configured install, stop every active agent cleanly after the target service is healthy — all of them, not a subset, because a running agent retains a literal-tilde DISPATCH_MEDIA_DIR that can recreate the legacy tree after the move. If any cannot be safely stopped, block the update before moving files.\n4. From the Dispatch install directory, run bin/migrate-legacy-media --dry-run. It reports which media root it resolved and from where. Exit status 1 means the scan was incomplete (for example an unreadable directory) — block and fix that first rather than treating it as a clean result.\n5. If the dry run reports no conflicts or unsupported entries, run bin/migrate-legacy-media --apply.\n6. If any conflict is reported, do not overwrite either file; report the affected path for manual recovery.\n7. Restart only the agents stopped for migration after it succeeds, including after a no-op migration, then confirm the legacy directory is empty or absent, the health endpoint is healthy, and release.json reports the target tag.", + "instructions": "1. Determine the effective MEDIA_ROOT by running bin/migrate-legacy-media --dry-run, which asks systemd for the service's effective environment (covering drop-ins and EnvironmentFile=) or reads the launchd plist, before falling back to /.env, and reports which source it used. Do not read .env alone — dotenv does not override a value already set in the service process environment. If the resolved root does not begin with `~`, this install was never affected: report the no-op, do not stop any agents, and skip to the health/version checks. If the command exits 1 asking for --media-root, the effective root could not be established — a systemd service it cannot interrogate, or a legacy tree the readable config does not explain; find the value the service actually ran with and rerun with --media-root rather than treating it as a no-op.\n2. Use the managed update flow to install the target release and wait for health.\n3. On a tilde-configured install, stop every active agent cleanly after the target service is healthy — all of them, not a subset, because a running agent retains a literal-tilde DISPATCH_MEDIA_DIR that can recreate the legacy tree after the move. If any cannot be safely stopped, block the update before moving files.\n4. From the Dispatch install directory, run bin/migrate-legacy-media --dry-run. It reports which media root it resolved and from where. Exit status 1 means the scan was incomplete (for example an unreadable directory) — block and fix that first rather than treating it as a clean result.\n5. If the dry run reports no conflicts or unsupported entries, run bin/migrate-legacy-media --apply.\n6. If any conflict is reported, do not overwrite either file; report the affected path for manual recovery.\n7. Restart only the agents stopped for migration after it succeeds, including after a no-op migration, then confirm the legacy directory is empty or absent, the health endpoint is healthy, and release.json reports the target tag.", "requiredChecks": [ "service_restarted", "health_endpoint", diff --git a/update-migrations/0012-legacy-media-paths.yaml b/update-migrations/0012-legacy-media-paths.yaml index c97c01448..7036a7f0a 100644 --- a/update-migrations/0012-legacy-media-paths.yaml +++ b/update-migrations/0012-legacy-media-paths.yaml @@ -18,16 +18,20 @@ alreadySatisfied: current service is healthy on the target tag. Do not read MEDIA_ROOT from /.env alone: the runtime loads that file through dotenv, which does not override a value already set in the service process - environment (systemd Environment=, launchd EnvironmentVariables, docker - -e). Run `bin/migrate-legacy-media --dry-run`, which checks the service - definition before .env and reports which root it resolved and from where. + environment (systemd Environment= including drop-ins and EnvironmentFile=, + launchd EnvironmentVariables, docker -e). Run `bin/migrate-legacy-media + --dry-run`, which asks systemd for the effective environment before falling + back to .env and reports which root it resolved and from where. instructions: - > First determine the install's effective MEDIA_ROOT. Run - `bin/migrate-legacy-media --dry-run`: it resolves the root from the service - definition (systemd Environment= / launchd EnvironmentVariables) before - falling back to /.env, and reports which source it used. + `bin/migrate-legacy-media --dry-run`: it resolves the root from the + service's effective environment — asking systemd, so drop-ins and + EnvironmentFile= are covered, or reading the launchd plist — before falling + back to /.env, and reports which source it used. If a + systemd service exists but cannot be interrogated it exits 1 rather than + guessing. Do not classify from .env alone — dotenv does not override a value already present in the service process environment, so .env can disagree with what the service actually ran with. If the resolved root does not begin with @@ -35,9 +39,10 @@ instructions: affected, do NOT stop any agents, and continue with the rest of the update plan. Only tilde-configured installs need the steps below. - > - If the command exits 1 asking for --media-root, it found a literal-tilde - tree that the configuration it could read does not explain — the service - ran with an override this script cannot see. Determine the value that + If the command exits 1 asking for --media-root, it could not establish the + effective media root: either a systemd service it cannot interrogate, or a + literal-tilde tree the readable configuration does not explain — the + service ran with an override this script cannot see. Determine the value that service actually used (its unit, plist, or container definition) and rerun with --media-root set to it. Do not treat this as a no-op. - > From 60e981b056f517fe1b27840c758981c6a2c3db5f Mon Sep 17 00:00:00 2001 From: Brad Harris Date: Thu, 20 Aug 2026 21:50:41 -0600 Subject: [PATCH 5/7] fix: drop the migration apparatus; correct the documented MEDIA_ROOT default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Investigation showed this was never a bad default Dispatch shipped. The code default has been absolute since the initial commit, and install-dispatch.sh never writes MEDIA_ROOT into .env, so a stock install cannot hit the bug. It requires an operator to set MEDIA_ROOT to a tilde path explicitly. The actual vector was the operations runbook, which documented MEDIA_ROOT's default as `~/.dispatch/media` — not what the code does, and precisely the value that breaks things when copied into .env. Corrected to $HOME/... with a note that a leading tilde is expanded but an absolute path is preferred. With no known affected installs, a required assisted update that stops all agents was disproportionate, so the manifest, the assisted-update metadata, the bash migrator and its tests are removed. What remains is the actual bug fix: a leading `~` in a storage path is expanded rather than treated as a directory named "~". Co-Authored-By: Claude Opus 5 --- apps/server/test/migrate-legacy-media.test.ts | 486 ---------------- apps/server/test/pack-release.test.ts | 1 - bin/migrate-legacy-media | 524 ------------------ docs/10-operations-runbook.md | 2 +- release-notes/next-assisted-update.json | 12 - .../0012-legacy-media-paths.yaml | 109 ---- 6 files changed, 1 insertion(+), 1133 deletions(-) delete mode 100644 apps/server/test/migrate-legacy-media.test.ts delete mode 100755 bin/migrate-legacy-media delete mode 100644 release-notes/next-assisted-update.json delete mode 100644 update-migrations/0012-legacy-media-paths.yaml diff --git a/apps/server/test/migrate-legacy-media.test.ts b/apps/server/test/migrate-legacy-media.test.ts deleted file mode 100644 index 1dfde8e8f..000000000 --- a/apps/server/test/migrate-legacy-media.test.ts +++ /dev/null @@ -1,486 +0,0 @@ -import { execFile } from "node:child_process"; -import { - chmod, - copyFile, - lstat, - mkdir, - mkdtemp, - readFile, - rm, - symlink, - writeFile, -} from "node:fs/promises"; -import os from "node:os"; -import path from "node:path"; -import { promisify } from "node:util"; - -import { afterEach, describe, expect, it } from "vitest"; - -const execFileAsync = promisify(execFile); -const scriptSource = path.resolve( - import.meta.dirname, - "../../..", - "bin/migrate-legacy-media" -); -const tempDirs: string[] = []; - -afterEach(async () => { - await Promise.all( - tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true })) - ); -}); - -async function createFixture() { - const root = await mkdtemp(path.join(os.tmpdir(), "dispatch-media-root-")); - const home = await mkdtemp(path.join(os.tmpdir(), "dispatch-media-home-")); - tempDirs.push(root, home); - const script = path.join(root, "bin", "migrate-legacy-media"); - await mkdir(path.dirname(script), { recursive: true }); - await copyFile(scriptSource, script); - await chmod(script, 0o755); - return { - root, - home, - script, - sourceDir: path.join(root, "~", ".dispatch", "media"), - destinationDir: path.join(home, ".dispatch", "media"), - }; -} - -async function runMigration( - script: string, - home: string, - mode: "--dry-run" | "--apply", - options: { args?: string[]; env?: NodeJS.ProcessEnv } = {} -) { - return execFileAsync(script, [mode, ...(options.args ?? [])], { - env: { - ...process.env, - // The script falls back to MEDIA_ROOT from the environment; an inherited - // value from the outer test runner would silently retarget the run. - MEDIA_ROOT: undefined, - ...options.env, - HOME: home, - }, - }); -} - -/** - * Stub `systemctl` on PATH so the effective-environment lookup can be driven - * without a real systemd. Mirrors the properties the script queries. - */ -async function stubSystemd( - home: string, - properties: { environment?: string; environmentFiles?: string } -) { - const unitDir = path.join(home, ".config", "systemd", "user"); - await mkdir(unitDir, { recursive: true }); - await writeFile( - path.join(unitDir, "dispatch.service"), - "[Service]\nExecStart=/x\n" - ); - const shimDir = await mkdtemp(path.join(os.tmpdir(), "dispatch-systemctl-")); - tempDirs.push(shimDir); - await writeFile( - path.join(shimDir, "systemctl"), - [ - "#!/bin/bash", - 'case "$*" in', - " *MainPID*) echo 0 ;;", - ` *EnvironmentFiles*) echo ${JSON.stringify(properties.environmentFiles ?? "")} ;;`, - ` *Environment*) echo ${JSON.stringify(properties.environment ?? "")} ;;`, - "esac", - "", - ].join("\n") - ); - await chmod(path.join(shimDir, "systemctl"), 0o755); - return shimDir; -} - -/** Write MEDIA_ROOT into the install .env the way the service reads it. */ -async function writeEnvFile(root: string, contents: string) { - await writeFile(path.join(root, ".env"), contents); -} - -describe("migrate-legacy-media", () => { - it("moves literal-tilde media only after an explicit apply", async () => { - const fixture = await createFixture(); - const source = path.join(fixture.sourceDir, "agt_1", "report.pdf"); - const destination = path.join( - fixture.destinationDir, - "agt_1", - "report.pdf" - ); - await mkdir(path.dirname(source), { recursive: true }); - await writeFile(source, "legacy report"); - - const dryRun = await runMigration( - fixture.script, - fixture.home, - "--dry-run" - ); - expect(dryRun.stdout).toContain("would move: agt_1/report.pdf"); - await expect(readFile(source, "utf8")).resolves.toBe("legacy report"); - - const applied = await runMigration(fixture.script, fixture.home, "--apply"); - expect(applied.stdout).toContain("moved: agt_1/report.pdf"); - await expect(readFile(destination, "utf8")).resolves.toBe("legacy report"); - await expect(readFile(source, "utf8")).rejects.toMatchObject({ - code: "ENOENT", - }); - }); - - it("does not overwrite a conflicting destination file", async () => { - const fixture = await createFixture(); - const source = path.join(fixture.sourceDir, "agt_1", "report.pdf"); - const destination = path.join( - fixture.destinationDir, - "agt_1", - "report.pdf" - ); - await mkdir(path.dirname(source), { recursive: true }); - await mkdir(path.dirname(destination), { recursive: true }); - await writeFile(source, "legacy report"); - await writeFile(destination, "new report"); - - await expect( - runMigration(fixture.script, fixture.home, "--apply") - ).rejects.toMatchObject({ code: 2 }); - await expect(readFile(source, "utf8")).resolves.toBe("legacy report"); - await expect(readFile(destination, "utf8")).resolves.toBe("new report"); - }); - - it("does not replace a dangling destination symlink", async () => { - const fixture = await createFixture(); - const source = path.join(fixture.sourceDir, "agt_1", "report.pdf"); - const destination = path.join( - fixture.destinationDir, - "agt_1", - "report.pdf" - ); - await mkdir(path.dirname(source), { recursive: true }); - await mkdir(path.dirname(destination), { recursive: true }); - await writeFile(source, "legacy report"); - await symlink("missing-report.pdf", destination); - - await expect( - runMigration(fixture.script, fixture.home, "--apply") - ).rejects.toMatchObject({ code: 2 }); - await expect(readFile(source, "utf8")).resolves.toBe("legacy report"); - expect((await lstat(destination)).isSymbolicLink()).toBe(true); - }); - - it("does not follow a destination ancestor symlink", async () => { - const fixture = await createFixture(); - const outside = await mkdtemp(path.join(os.tmpdir(), "dispatch-outside-")); - tempDirs.push(outside); - const source = path.join(fixture.sourceDir, "agt_1", "report.pdf"); - const linkedAgentDir = path.join(fixture.destinationDir, "agt_1"); - await mkdir(path.dirname(source), { recursive: true }); - await mkdir(fixture.destinationDir, { recursive: true }); - await writeFile(source, "legacy report"); - await symlink(outside, linkedAgentDir); - - await expect( - runMigration(fixture.script, fixture.home, "--apply") - ).rejects.toMatchObject({ code: 2 }); - await expect(readFile(source, "utf8")).resolves.toBe("legacy report"); - expect((await lstat(linkedAgentDir)).isSymbolicLink()).toBe(true); - await expect( - readFile(path.join(outside, "report.pdf"), "utf8") - ).rejects.toMatchObject({ code: "ENOENT" }); - }); - - it("resolves a non-default tilde MEDIA_ROOT from the install .env", async () => { - const fixture = await createFixture(); - await writeEnvFile(fixture.root, "MEDIA_ROOT=~/dispatch-media\n"); - // The legacy tree lives under the configured value, not the default one. - const source = path.join( - fixture.root, - "~", - "dispatch-media", - "agt_1", - "shot.png" - ); - const destination = path.join( - fixture.home, - "dispatch-media", - "agt_1", - "shot.png" - ); - await mkdir(path.dirname(source), { recursive: true }); - await writeFile(source, "legacy shot"); - - const applied = await runMigration(fixture.script, fixture.home, "--apply"); - expect(applied.stdout).toContain("moved: agt_1/shot.png"); - await expect(readFile(destination, "utf8")).resolves.toBe("legacy shot"); - }); - - it("accepts an explicit --media-root over the .env value", async () => { - const fixture = await createFixture(); - await writeEnvFile(fixture.root, "MEDIA_ROOT=~/dispatch-media\n"); - const source = path.join( - fixture.root, - "~", - "override", - "agt_1", - "shot.png" - ); - const destination = path.join( - fixture.home, - "override", - "agt_1", - "shot.png" - ); - await mkdir(path.dirname(source), { recursive: true }); - await writeFile(source, "legacy shot"); - - const applied = await runMigration( - fixture.script, - fixture.home, - "--apply", - { - args: ["--media-root", "~/override"], - } - ); - expect(applied.stdout).toContain("moved: agt_1/shot.png"); - await expect(readFile(destination, "utf8")).resolves.toBe("legacy shot"); - }); - - it("is a no-op on an install whose MEDIA_ROOT is absolute", async () => { - const fixture = await createFixture(); - await writeEnvFile(fixture.root, "MEDIA_ROOT=/var/lib/dispatch/media\n"); - // A stray legacy tree must still be left alone: an absolute MEDIA_ROOT was - // never resolved through the broken tilde path, so nothing here is ours. - const source = path.join(fixture.sourceDir, "agt_1", "report.pdf"); - await mkdir(path.dirname(source), { recursive: true }); - await writeFile(source, "legacy report"); - - const applied = await runMigration(fixture.script, fixture.home, "--apply"); - expect(applied.stdout).toContain("nothing to migrate"); - await expect(readFile(source, "utf8")).resolves.toBe("legacy report"); - }); - - it("fails instead of reporting a clean result when the scan is incomplete", async () => { - const fixture = await createFixture(); - const readable = path.join(fixture.sourceDir, "agt_1", "report.pdf"); - const lockedDir = path.join(fixture.sourceDir, "agt_locked"); - await mkdir(path.dirname(readable), { recursive: true }); - await mkdir(lockedDir, { recursive: true }); - await writeFile(readable, "legacy report"); - await writeFile(path.join(lockedDir, "hidden.pdf"), "hidden"); - await chmod(lockedDir, 0o000); - - try { - // find cannot descend into the locked directory. Exiting 0 with a - // zero-conflict summary here would let the manifest greenlight --apply - // from a scan that never saw `hidden.pdf`. - const result = await runMigration( - fixture.script, - fixture.home, - "--dry-run" - ).then( - () => null, - (err: { code?: number; stderr?: string }) => err - ); - expect(result?.code).toBe(1); - expect(result?.stderr).toContain("incomplete scan"); - } finally { - await chmod(lockedDir, 0o755); - } - }); - - it("treats a destination that appears mid-migration as a conflict", async () => { - const fixture = await createFixture(); - const source = path.join(fixture.sourceDir, "agt_1", "report.pdf"); - const destination = path.join( - fixture.destinationDir, - "agt_1", - "report.pdf" - ); - await mkdir(path.dirname(source), { recursive: true }); - await writeFile(source, "legacy report"); - - // Shim `mkdir` so a file lands at the destination in the exact window - // between the script's preflight checks and its placement — the race a - // live Dispatch server can win by writing media while the migration runs. - const shimDir = await mkdtemp(path.join(os.tmpdir(), "dispatch-shim-")); - tempDirs.push(shimDir); - const shim = path.join(shimDir, "mkdir"); - await writeFile( - shim, - `#!/bin/bash\n/bin/mkdir "$@"\nprintf 'raced in' > ${JSON.stringify(destination)} 2>/dev/null || true\n` - ); - await chmod(shim, 0o755); - - await expect( - runMigration(fixture.script, fixture.home, "--apply", { - env: { PATH: `${shimDir}:${process.env.PATH ?? ""}` }, - }) - ).rejects.toMatchObject({ code: 2 }); - await expect(readFile(destination, "utf8")).resolves.toBe("raced in"); - await expect(readFile(source, "utf8")).resolves.toBe("legacy report"); - }); - - it("refuses to classify an install as unaffected while a legacy tree exists", async () => { - const fixture = await createFixture(); - await writeEnvFile(fixture.root, "MEDIA_ROOT=/var/lib/dispatch/media\n"); - // The readable config says absolute, but files under a literal `~` prove - // the running service used something else. Guessing "no-op" here would - // silently strand them. - const source = path.join(fixture.root, "~", "mystery", "agt_1", "o.png"); - await mkdir(path.dirname(source), { recursive: true }); - await writeFile(source, "orphan"); - - const result = await runMigration( - fixture.script, - fixture.home, - "--apply" - ).then( - () => null, - (err: { code?: number; stderr?: string }) => err - ); - expect(result?.code).toBe(1); - expect(result?.stderr).toContain("--media-root"); - await expect(readFile(source, "utf8")).resolves.toBe("orphan"); - }); - - it("fails closed when a destination ancestor is swapped mid-migration", async () => { - const fixture = await createFixture(); - const outside = await mkdtemp(path.join(os.tmpdir(), "dispatch-outside-")); - tempDirs.push(outside); - const source = path.join(fixture.sourceDir, "agt_1", "report.pdf"); - const agentDir = path.join(fixture.destinationDir, "agt_1"); - await mkdir(path.dirname(source), { recursive: true }); - await mkdir(fixture.destinationDir, { recursive: true }); - await writeFile(source, "legacy report"); - - // Swap the agent directory for a symlink *after* the ancestor preflight, - // in the window a path-based check cannot cover. Placement holds a kernel - // directory reference and verifies where it landed, so this must not write - // through the symlink. - const shimDir = await mkdtemp(path.join(os.tmpdir(), "dispatch-shim-")); - tempDirs.push(shimDir); - const shim = path.join(shimDir, "mkdir"); - await writeFile( - shim, - [ - "#!/bin/bash", - '/bin/mkdir "$@"', - `if [[ -d ${JSON.stringify(agentDir)} && ! -L ${JSON.stringify(agentDir)} ]]; then`, - ` /bin/rmdir ${JSON.stringify(agentDir)} 2>/dev/null && /bin/ln -s ${JSON.stringify(outside)} ${JSON.stringify(agentDir)}`, - "fi", - "", - ].join("\n") - ); - await chmod(shim, 0o755); - - await expect( - runMigration(fixture.script, fixture.home, "--apply", { - env: { PATH: `${shimDir}:${process.env.PATH ?? ""}` }, - }) - ).rejects.toMatchObject({ code: 2 }); - await expect(readFile(source, "utf8")).resolves.toBe("legacy report"); - await expect( - readFile(path.join(outside, "report.pdf"), "utf8") - ).rejects.toMatchObject({ code: "ENOENT" }); - }); - - it("reads MEDIA_ROOT from a systemd drop-in via the merged environment", async () => { - const fixture = await createFixture(); - await writeEnvFile(fixture.root, "MEDIA_ROOT=/var/lib/dispatch/media\n"); - // systemctl already folds dispatch.service.d/*.conf into Environment=, - // which is exactly why the script asks systemd instead of parsing the unit. - const shimDir = await stubSystemd(fixture.home, { - environment: "FOO=1 MEDIA_ROOT=~/dropin-media", - }); - const source = path.join( - fixture.root, - "~", - "dropin-media", - "agt_1", - "d.png" - ); - const destination = path.join( - fixture.home, - "dropin-media", - "agt_1", - "d.png" - ); - await mkdir(path.dirname(source), { recursive: true }); - await writeFile(source, "dropin shot"); - - const applied = await runMigration( - fixture.script, - fixture.home, - "--apply", - { - env: { PATH: `${shimDir}:${process.env.PATH ?? ""}` }, - } - ); - expect(applied.stdout).toContain("moved: agt_1/d.png"); - await expect(readFile(destination, "utf8")).resolves.toBe("dropin shot"); - }); - - it("reads MEDIA_ROOT from a systemd EnvironmentFile", async () => { - const fixture = await createFixture(); - await writeEnvFile(fixture.root, "MEDIA_ROOT=/var/lib/dispatch/media\n"); - const serviceEnv = path.join(fixture.home, "svc.env"); - await writeFile(serviceEnv, "MEDIA_ROOT=~/envfile-media\n"); - const shimDir = await stubSystemd(fixture.home, { - environmentFiles: `${serviceEnv} (ignore_errors=no)`, - }); - const source = path.join( - fixture.root, - "~", - "envfile-media", - "agt_1", - "e.png" - ); - const destination = path.join( - fixture.home, - "envfile-media", - "agt_1", - "e.png" - ); - await mkdir(path.dirname(source), { recursive: true }); - await writeFile(source, "envfile shot"); - - const applied = await runMigration( - fixture.script, - fixture.home, - "--apply", - { - env: { PATH: `${shimDir}:${process.env.PATH ?? ""}` }, - } - ); - expect(applied.stdout).toContain("moved: agt_1/e.png"); - await expect(readFile(destination, "utf8")).resolves.toBe("envfile shot"); - }); - - it("fails closed when a systemd service cannot be interrogated", async () => { - const fixture = await createFixture(); - await writeEnvFile(fixture.root, "MEDIA_ROOT=/var/lib/dispatch/media\n"); - const unitDir = path.join(fixture.home, ".config", "systemd", "user"); - await mkdir(unitDir, { recursive: true }); - await writeFile(path.join(unitDir, "dispatch.service"), "[Service]\n"); - const shimDir = await mkdtemp( - path.join(os.tmpdir(), "dispatch-systemctl-") - ); - tempDirs.push(shimDir); - await writeFile(path.join(shimDir, "systemctl"), "#!/bin/bash\nexit 1\n"); - await chmod(path.join(shimDir, "systemctl"), 0o755); - - // No legacy tree exists yet, so physical evidence cannot catch a wrong - // verdict here — a drop-in or EnvironmentFile tilde root would be missed - // and a running agent would recreate the tree after the "no-op". - const result = await runMigration(fixture.script, fixture.home, "--apply", { - env: { PATH: `${shimDir}:${process.env.PATH ?? ""}` }, - }).then( - () => null, - (err: { code?: number; stderr?: string }) => err - ); - expect(result?.code).toBe(1); - expect(result?.stderr).toContain("--media-root"); - }); -}); diff --git a/apps/server/test/pack-release.test.ts b/apps/server/test/pack-release.test.ts index 36a17f93e..c6f84decf 100644 --- a/apps/server/test/pack-release.test.ts +++ b/apps/server/test/pack-release.test.ts @@ -60,7 +60,6 @@ describe.skipIf(!BUILDS_EXIST)("pack-release", () => { // reference it, but old checkout-based services need it through the // fixed-runtime migration. expect(files).toContain("bin/dispatch-launchd-wrapper"); - expect(files).toContain("bin/migrate-legacy-media"); }); it("does NOT embed macOS xattr/pax metadata (e.g. com.apple.provenance)", () => { diff --git a/bin/migrate-legacy-media b/bin/migrate-legacy-media deleted file mode 100755 index dd499e925..000000000 --- a/bin/migrate-legacy-media +++ /dev/null @@ -1,524 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail - -# Move media written by Dispatch versions that treated a leading `~` as a -# literal directory name. This is intentionally a standalone shell script: it -# ships in release tarballs and runs on installs that only have the compiled -# Dispatch binary (not Bun, pnpm, or source files). - -ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" -MODE="dry-run" -MEDIA_ROOT_OVERRIDE="" - -usage() { - cat <<'USAGE' -Usage: bin/migrate-legacy-media [--dry-run|--apply] [--media-root VALUE] - -Dispatch versions before the tilde-expansion fix stored media under a -directory literally named `~` inside the install tree whenever MEDIA_ROOT was -configured with a leading tilde. This moves that legacy tree to the location -the fixed runtime actually reads. - -For MEDIA_ROOT=~/.dispatch/media (the documented default) that means: - from /~/.dispatch/media - to $HOME/.dispatch/media - -The media root is not assumed. It is taken from --media-root, else the -MEDIA_ROOT environment variable, else MEDIA_ROOT in /.env -(the file the service loads), else the documented default. An install whose -MEDIA_ROOT is an absolute path was never affected and exits 0 immediately -without touching anything. - ---dry-run is the default. --apply moves files that do not already exist at -the destination, removes source duplicates with identical contents, and -leaves conflicting files untouched. Exit status 2 means conflicts or -unsupported entries need manual handling. -USAGE -} - -while [[ $# -gt 0 ]]; do - case "$1" in - --dry-run) MODE="dry-run" ;; - --apply) MODE="apply" ;; - --media-root) - if [[ $# -lt 2 ]]; then - echo "error: --media-root requires a value" >&2 - exit 1 - fi - MEDIA_ROOT_OVERRIDE="$2" - shift - ;; - --media-root=*) MEDIA_ROOT_OVERRIDE="${1#--media-root=}" ;; - --help|-h) usage; exit 0 ;; - *) echo "error: unknown argument: $1" >&2; usage >&2; exit 1 ;; - esac - shift -done - -if [[ -z "${HOME:-}" ]]; then - echo "error: HOME must be set to migrate legacy media" >&2 - exit 1 -fi - -# Read MEDIA_ROOT out of the env file the service itself loads (systemd -# EnvironmentFile / the dotenv import in the runtime). Only the last -# assignment wins, matching dotenv, and surrounding quotes are stripped. -media_root_from_env_file() { - local env_file="$ROOT_DIR/.env" - local line value - [[ -r "$env_file" ]] || return 0 - line="$(grep -E '^[[:space:]]*(export[[:space:]]+)?MEDIA_ROOT=' "$env_file" | tail -1)" || return 0 - [[ -n "$line" ]] || return 0 - value="${line#*=}" - # Trim a trailing carriage return (CRLF env files) and surrounding quotes. - value="${value%$'\r'}" - if [[ "$value" == \"*\" || "$value" == \'*\' ]]; then - value="${value:1:${#value}-2}" - fi - printf '%s' "$value" -} - -# A service-level MEDIA_ROOT (systemd Environment=, launchd -# EnvironmentVariables, docker -e) beats the .env file: the runtime loads .env -# through dotenv, which does not override a variable already present in the -# process environment. The operator usually runs this script from a plain -# shell that never saw that value, so .env alone cannot be trusted to describe -# the effective configuration. -# Extract MEDIA_ROOT from a NUL- or newline-separated environment dump on -# stdin. Prints nothing when absent; `grep` finding no match is a normal -# outcome here, not a failure, so it must not trip errexit/pipefail. -media_root_from_environ_dump() { - local line - line="$(tr '\0' '\n' | grep -E '^MEDIA_ROOT=' | tail -1)" || line="" - [[ -n "$line" ]] || return 0 - line="${line#MEDIA_ROOT=}" - line="${line%$'\r'}" - if [[ "$line" == \"*\" || "$line" == \'*\' ]]; then - line="${line:1:${#line}-2}" - fi - printf '%s' "$line" -} - -# Resolve MEDIA_ROOT from systemd's *effective* service environment into the -# globals SERVICE_MEDIA_ROOT / SERVICE_LOOKUP_STATUS. Assigned rather than -# printed: a command substitution would run this in a subshell and the status -# would never reach the caller. -# -# Parsing the unit file by hand is not enough — systemd merges -# dispatch.service.d/*.conf drop-ins and reads EnvironmentFile= at exec time, -# so a tilde root can live in a source a naive parser never opens. Ask systemd -# instead of reimplementing its merge. -# -# SERVICE_LOOKUP_STATUS is one of: -# found — positively established (value in SERVICE_MEDIA_ROOT) -# absent — systemd answered and there is definitively no MEDIA_ROOT -# unknown — systemd is present but could not be interrogated -# no-systemd — no systemd Dispatch service on this host -SERVICE_MEDIA_ROOT="" -SERVICE_LOOKUP_STATUS="no-systemd" - -media_root_from_systemd() { - local unit="$HOME/.config/systemd/user/dispatch.service" - local main_pid environment env_files env_file value - - SERVICE_MEDIA_ROOT="" - if [[ ! -e "$unit" ]]; then - SERVICE_LOOKUP_STATUS="no-systemd" - return 0 - fi - if ! command -v systemctl >/dev/null 2>&1; then - SERVICE_LOOKUP_STATUS="unknown" - return 0 - fi - - # Best source: the live process environment. It is ground truth, whatever - # combination of unit, drop-in, EnvironmentFile or manager environment - # produced it. - main_pid="$(systemctl --user show dispatch.service --property=MainPID --value 2>/dev/null)" || main_pid="" - if [[ -n "$main_pid" && "$main_pid" != "0" && -r "/proc/$main_pid/environ" ]]; then - value="$(media_root_from_environ_dump <"/proc/$main_pid/environ")" || value="" - if [[ -n "$value" ]]; then - SERVICE_MEDIA_ROOT="$value" - SERVICE_LOOKUP_STATUS="found" - else - SERVICE_LOOKUP_STATUS="absent" - fi - return 0 - fi - - # Service not running (or no procfs): ask systemd for the merged directives. - # `show` already folds in drop-ins; EnvironmentFile= is reported separately. - if ! environment="$(systemctl --user show dispatch.service --property=Environment --value 2>/dev/null)"; then - SERVICE_LOOKUP_STATUS="unknown" - return 0 - fi - value="$(printf '%s\n' "$environment" | tr ' ' '\n' | media_root_from_environ_dump)" || value="" - if [[ -n "$value" ]]; then - SERVICE_MEDIA_ROOT="$value" - SERVICE_LOOKUP_STATUS="found" - return 0 - fi - - env_files="$(systemctl --user show dispatch.service --property=EnvironmentFiles --value 2>/dev/null)" || env_files="" - if [[ -n "$env_files" ]]; then - # Reported as "/path/to/file (ignore_errors=no)", one per line. - while IFS= read -r env_file; do - env_file="${env_file%% (*}" - if [[ -n "$env_file" && -r "$env_file" ]]; then - value="$(media_root_from_environ_dump <"$env_file")" || value="" - if [[ -n "$value" ]]; then - SERVICE_MEDIA_ROOT="$value" - SERVICE_LOOKUP_STATUS="found" - return 0 - fi - fi - done <MEDIA_ROOT[[:space:]]*[^<]*' \ - | tail -1 \ - | sed -e 's|.*||' -e 's|||')" || value="" - printf '%s' "$value" -} - - -CONFIGURED_MEDIA_ROOT="" -CONFIGURED_SOURCE="default" -if [[ -n "$MEDIA_ROOT_OVERRIDE" ]]; then - CONFIGURED_MEDIA_ROOT="$MEDIA_ROOT_OVERRIDE" - CONFIGURED_SOURCE="--media-root" -elif [[ -n "${MEDIA_ROOT:-}" ]]; then - CONFIGURED_MEDIA_ROOT="$MEDIA_ROOT" - CONFIGURED_SOURCE="MEDIA_ROOT environment variable" -else - media_root_from_systemd - if [[ "$SERVICE_LOOKUP_STATUS" == "found" ]]; then - CONFIGURED_MEDIA_ROOT="$SERVICE_MEDIA_ROOT" - CONFIGURED_SOURCE="the systemd service environment" - else - CONFIGURED_MEDIA_ROOT="$(media_root_from_launchd)" - if [[ -n "$CONFIGURED_MEDIA_ROOT" ]]; then - CONFIGURED_SOURCE="the launchd service definition" - elif [[ "$SERVICE_LOOKUP_STATUS" == "unknown" ]]; then - # systemd is here but would not answer. Its environment can carry a - # tilde root from a drop-in or EnvironmentFile that .env does not show, - # and a "not affected" verdict from .env alone would skip a real - # migration — with no legacy tree on disk yet to catch the mistake. - echo "error: a systemd Dispatch service exists but its effective environment" >&2 - echo "could not be read, so MEDIA_ROOT cannot be established. Determine the" >&2 - echo "value that service runs with and rerun with --media-root set to it." >&2 - exit 1 - else - CONFIGURED_MEDIA_ROOT="$(media_root_from_env_file)" - [[ -n "$CONFIGURED_MEDIA_ROOT" ]] && CONFIGURED_SOURCE="$ROOT_DIR/.env" - fi - fi -fi - -# Physical evidence overrides configuration guesses. If a literal-tilde tree -# exists under the install directory, this install *did* write through the -# broken path at some point, whatever the config we managed to read now says. -# Refuse to classify it as unaffected rather than silently skipping real files. -legacy_tree_present() { - local candidate - for candidate in "$ROOT_DIR"/~/*; do - [[ -e "$candidate" ]] && return 0 - done - return 1 -} - -if [[ -n "$CONFIGURED_MEDIA_ROOT" && "$CONFIGURED_MEDIA_ROOT" != "~" && "$CONFIGURED_MEDIA_ROOT" != "~/"* ]]; then - if legacy_tree_present; then - echo "error: MEDIA_ROOT reads as $CONFIGURED_MEDIA_ROOT (from $CONFIGURED_SOURCE)," >&2 - echo "but a literal-tilde tree exists at $ROOT_DIR/~ — the effective media root" >&2 - echo "used by the running service must have differed. Re-run with --media-root" >&2 - echo "set to the value that service actually used." >&2 - exit 1 - fi - echo "MEDIA_ROOT is $CONFIGURED_MEDIA_ROOT (from $CONFIGURED_SOURCE), which has no leading tilde." - echo "This install never wrote media to a literal-tilde path; nothing to migrate." - exit 0 -fi - -# No explicit setting anywhere: fall back to the documented default so an -# install whose .env is unreadable is still checked. Costs nothing — if the -# legacy tree is absent the run is a no-op. -if [[ -z "$CONFIGURED_MEDIA_ROOT" ]]; then - CONFIGURED_MEDIA_ROOT="~/.dispatch/media" -fi - -# The legacy tree sits under the service's working directory, which both the -# launchd plist and the systemd unit set to the install directory. -SOURCE_DIR="$ROOT_DIR/$CONFIGURED_MEDIA_ROOT" -if [[ "$CONFIGURED_MEDIA_ROOT" == "~" ]]; then - DESTINATION_DIR="$HOME" -else - DESTINATION_DIR="$HOME/${CONFIGURED_MEDIA_ROOT#\~/}" -fi - -# Components of the destination tree below $HOME, walked by the ancestor -# check. Derived from the configured media root rather than hardcoded, so a -# non-default MEDIA_ROOT is checked as thoroughly as the default one. -DESTINATION_RELATIVE="${DESTINATION_DIR#"$HOME"}" -DESTINATION_RELATIVE="${DESTINATION_RELATIVE#/}" - -# Physical root of the media tree, resolved once. Only $HOME is resolved -# through symlinks — it can legitimately be one (/home -> /var/home). The -# components below it are deliberately NOT resolved: destination_ancestors_are_safe -# rejects a symlink among them, so resolving here would launder exactly the -# case that check exists to catch. -HOME_PHYSICAL="$(cd -P "$HOME" && pwd -P)" -DESTINATION_PHYSICAL="$HOME_PHYSICAL" -[[ -n "$DESTINATION_RELATIVE" ]] && DESTINATION_PHYSICAL="$HOME_PHYSICAL/$DESTINATION_RELATIVE" - -destination_ancestors_are_safe() { - local relative_path="$1" - local parent_path="" - local current="$HOME" - local remaining - local component - - if [[ "$relative_path" == */* ]]; then - parent_path="${relative_path%/*}" - fi - - # Check every existing path component below HOME. A symlink at any level - # would make mkdir -p follow it and let a migration write outside the media - # tree. Missing components are safe; mkdir -p creates them during --apply. - remaining="$DESTINATION_RELATIVE" - while [[ -n "$remaining" ]]; do - component="${remaining%%/*}" - current="$current/$component" - if [[ -L "$current" || ( -e "$current" && ! -d "$current" ) ]]; then - echo "$current" - return 1 - fi - if [[ "$remaining" == */* ]]; then - remaining="${remaining#*/}" - else - remaining="" - fi - done - - while [[ -n "$parent_path" ]]; do - component="${parent_path%%/*}" - current="$current/$component" - if [[ -L "$current" || ( -e "$current" && ! -d "$current" ) ]]; then - echo "$current" - return 1 - fi - if [[ "$parent_path" == */* ]]; then - parent_path="${parent_path#*/}" - else - parent_path="" - fi - done - - # Explicit success: the loops above may not run at all, and without this the - # function would return whatever status the last `[[ ]]` test happened to - # leave behind. - return 0 -} - -if [[ ! -e "$SOURCE_DIR" ]]; then - echo "No legacy media directory found at $SOURCE_DIR; nothing to migrate." - exit 0 -fi -if [[ ! -d "$SOURCE_DIR" ]]; then - echo "error: legacy media path is not a directory: $SOURCE_DIR" >&2 - exit 1 -fi - -# Place $1 (absolute source path) inside destination directory $2 under the -# single-component name $3, without ever replacing an existing entry and -# without ever writing outside the media tree. -# -# Two hazards, both handled here rather than by a preflight test: -# -# * `mv` silently overwrites. The Dispatch server keeps writing media -# (browser-extension screenshots, whiteboard snapshots) while this runs, so -# a destination file can appear between any check and the move. link(2) is -# atomic and fails with EEXIST instead, and never follows a symlink at the -# destination. -# -# * A path re-resolves on every syscall, so checking the ancestors and then -# writing through the same path string is not sound — a component can be -# swapped for a symlink in between. Instead `cd -P` into the destination -# directory once and link under a single-component name: the shell's cwd is -# a kernel-held directory reference, so a later swap of the parent path -# cannot redirect the write. The containment check runs after the cd, on -# the physical path we actually landed in, so a swap that already happened -# fails closed here. -# -# Exit status: 0 placed, 1 destination appeared (treat as a conflict), -# 2 the copy itself failed, 3 the destination escaped the media tree. -place_file() { - local src="$1" - local dst_dir="$2" - local base="$3" - local status - - ( - cd -P "$dst_dir" 2>/dev/null || exit 3 - - landed="$(pwd -P)" - case "$landed" in - "$DESTINATION_PHYSICAL"|"$DESTINATION_PHYSICAL"/*) ;; - *) exit 3 ;; - esac - - if ln "$src" "$base" 2>/dev/null; then - exit 0 - fi - - # Either the destination raced into existence, or source and destination - # live on different filesystems (hard links cannot span them, which - # happens when the install tree and $HOME are separate mounts). - if [[ -e "$base" || -L "$base" ]]; then - exit 1 - fi - - # Cross-device: stage a private copy inside the destination directory, - # then claim the final name with the same atomic no-clobber link. The - # staging file is created relative to the held cwd too. - tmp="$(mktemp ./.migrate-legacy-media.XXXXXX)" || exit 2 - if ! cp -p "$src" "$tmp"; then - rm -f "$tmp" - exit 2 - fi - if ln "$tmp" "$base" 2>/dev/null; then - rm -f "$tmp" - exit 0 - fi - rm -f "$tmp" - exit 1 - ) - status=$? - - # The source is only dropped once the destination is definitely in place. - if [[ "$status" -eq 0 ]]; then - rm -f "$src" - fi - return "$status" -} - - -moved=0 -duplicates=0 -conflicts=0 -unsupported=0 - -# Enumerate up front rather than streaming `find` through a process -# substitution: there the parent shell never sees find's exit status, so an -# unreadable subdirectory prints a "Permission denied" line to stderr and the -# run still ends with a zero-conflict summary and exit 0. The manifest gates -# `--apply` on exactly that summary, so a partial scan must be a hard failure -# rather than a clean-looking result. -SCAN_DIR="$(mktemp -d "${TMPDIR:-/tmp}/migrate-legacy-media.XXXXXX")" -trap 'rm -rf "$SCAN_DIR"' EXIT - -REGULAR_LIST="$SCAN_DIR/regular" -SPECIAL_LIST="$SCAN_DIR/special" - -scan_failed() { - echo "error: could not fully scan $SOURCE_DIR (see the find errors above)." >&2 - echo "Refusing to report a result from an incomplete scan; fix the unreadable entries and rerun." >&2 - exit 1 -} - -find "$SOURCE_DIR" -type f -print0 >"$REGULAR_LIST" || scan_failed -find "$SOURCE_DIR" \( -type l -o -type p -o -type s -o -type b -o -type c \) \ - -print0 >"$SPECIAL_LIST" || scan_failed - -while IFS= read -r -d '' source_file; do - relative_path="${source_file#"$SOURCE_DIR"/}" - destination_file="$DESTINATION_DIR/$relative_path" - - if ! unsafe_ancestor="$(destination_ancestors_are_safe "$relative_path")"; then - conflicts=$((conflicts + 1)) - echo "conflict (unsafe destination ancestor left untouched): $relative_path ($unsafe_ancestor)" >&2 - continue - fi - - # -e is false for dangling symlinks, so test -L first. Never replace a - # symlink, directory, device, or other non-regular destination entry. - if [[ -L "$destination_file" || ( -e "$destination_file" && ! -f "$destination_file" ) ]]; then - conflicts=$((conflicts + 1)) - echo "conflict (non-regular destination left untouched): $relative_path" >&2 - continue - fi - - if [[ -f "$destination_file" ]]; then - if cmp -s "$source_file" "$destination_file"; then - duplicates=$((duplicates + 1)) - if [[ "$MODE" == "apply" ]]; then - rm "$source_file" - echo "removed duplicate: $relative_path" - else - echo "would remove duplicate: $relative_path" - fi - else - conflicts=$((conflicts + 1)) - echo "conflict (left untouched): $relative_path" >&2 - fi - continue - fi - - if [[ "$MODE" != "apply" ]]; then - moved=$((moved + 1)) - echo "would move: $relative_path" - continue - fi - - mkdir -p "$(dirname "$destination_file")" - place_status=0 - place_file "$source_file" "$(dirname "$destination_file")" \ - "$(basename "$destination_file")" || place_status=$? - case "$place_status" in - 0) - moved=$((moved + 1)) - echo "moved: $relative_path" - ;; - 1) - conflicts=$((conflicts + 1)) - echo "conflict (destination appeared during migration, left untouched): $relative_path" >&2 - ;; - 3) - conflicts=$((conflicts + 1)) - echo "conflict (destination escaped the media tree, left untouched): $relative_path" >&2 - ;; - *) - conflicts=$((conflicts + 1)) - echo "conflict (could not copy across filesystems, left untouched): $relative_path" >&2 - ;; - esac -done <"$REGULAR_LIST" - -while IFS= read -r -d '' unsupported_path; do - unsupported=$((unsupported + 1)) - echo "unsupported entry (left untouched): ${unsupported_path#"$SOURCE_DIR"/}" >&2 -done <"$SPECIAL_LIST" - -if [[ "$MODE" == "apply" && "$conflicts" -eq 0 && "$unsupported" -eq 0 ]]; then - find "$SOURCE_DIR" -depth -type d -empty -delete -fi - -echo "Legacy media migration ($MODE): $moved file(s) moved, $duplicates duplicate(s) removed, $conflicts conflict(s), $unsupported unsupported entry(s)." - -if [[ "$conflicts" -gt 0 || "$unsupported" -gt 0 ]]; then - exit 2 -fi diff --git a/docs/10-operations-runbook.md b/docs/10-operations-runbook.md index 3fd25f92d..da7070ed3 100644 --- a/docs/10-operations-runbook.md +++ b/docs/10-operations-runbook.md @@ -203,7 +203,7 @@ Server configuration lives in `~/.dispatch/server/.env`. Key variables: | `DISPATCH_HOST` | `127.0.0.1` | Interface to bind the API server to. Set `0.0.0.0` only when the machine must accept remote connections. | | `DISPATCH_PORT` | `6767` | HTTP port the server listens on | | `DATABASE_URL` | `postgres://dispatch:dispatch@127.0.0.1:5432/dispatch` | Postgres connection string | -| `MEDIA_ROOT` | `~/.dispatch/media` | File upload storage path | +| `MEDIA_ROOT` | `$HOME/.dispatch/media` | File upload storage path. A leading `~` is expanded, but prefer an absolute path. | | `DISPATCH_AGENT_RUNTIME` | `tmux` | Agent runtime mode (`tmux` or `inert` for dev/test) | | `DISPATCH_COPY_DISPLAY` | — | Virtual X display for clipboard image paste on Linux (e.g. `:99`) | | `TLS_CERT` | — | Path to TLS certificate file (enables HTTPS when both cert and key are set) | diff --git a/release-notes/next-assisted-update.json b/release-notes/next-assisted-update.json deleted file mode 100644 index c2a0ad2f1..000000000 --- a/release-notes/next-assisted-update.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "mode": "required", - "title": "Migrate shared media from legacy tilde paths", - "summary": "This release corrects shared-media storage paths on installs whose MEDIA_ROOT is configured with a leading tilde (for example ~/.dispatch/media). The assisted update safely moves recoverable historical files from the old literal-tilde directory into the location the fixed runtime reads, preserving conflicts for manual recovery. Installs with an absolute MEDIA_ROOT were never affected and the migration is a no-op for them.", - "instructions": "1. Determine the effective MEDIA_ROOT by running bin/migrate-legacy-media --dry-run, which asks systemd for the service's effective environment (covering drop-ins and EnvironmentFile=) or reads the launchd plist, before falling back to /.env, and reports which source it used. Do not read .env alone — dotenv does not override a value already set in the service process environment. If the resolved root does not begin with `~`, this install was never affected: report the no-op, do not stop any agents, and skip to the health/version checks. If the command exits 1 asking for --media-root, the effective root could not be established — a systemd service it cannot interrogate, or a legacy tree the readable config does not explain; find the value the service actually ran with and rerun with --media-root rather than treating it as a no-op.\n2. Use the managed update flow to install the target release and wait for health.\n3. On a tilde-configured install, stop every active agent cleanly after the target service is healthy — all of them, not a subset, because a running agent retains a literal-tilde DISPATCH_MEDIA_DIR that can recreate the legacy tree after the move. If any cannot be safely stopped, block the update before moving files.\n4. From the Dispatch install directory, run bin/migrate-legacy-media --dry-run. It reports which media root it resolved and from where. Exit status 1 means the scan was incomplete (for example an unreadable directory) — block and fix that first rather than treating it as a clean result.\n5. If the dry run reports no conflicts or unsupported entries, run bin/migrate-legacy-media --apply.\n6. If any conflict is reported, do not overwrite either file; report the affected path for manual recovery.\n7. Restart only the agents stopped for migration after it succeeds, including after a no-op migration, then confirm the legacy directory is empty or absent, the health endpoint is healthy, and release.json reports the target tag.", - "requiredChecks": [ - "service_restarted", - "health_endpoint", - "version_converged" - ], - "rollbackGuidance": "Rollback is unrestricted before bin/migrate-legacy-media --apply runs: nothing has moved. After --apply, a plain rollback is NOT safe — the pre-fix runtime resolves `~` literally again while the migrated files exist only under the expanded home path, so the service passes its health check with every migrated file unreadable. Prefer resolving forward on the fixed release. If rolling back after a move is unavoidable, move the migrated files back into the literal-tilde tree under the install directory before starting the old binary. Do not delete media from either location: the migrator never overwrites destination files, and it is safe to rerun after recovery." -} diff --git a/update-migrations/0012-legacy-media-paths.yaml b/update-migrations/0012-legacy-media-paths.yaml deleted file mode 100644 index 7036a7f0a..000000000 --- a/update-migrations/0012-legacy-media-paths.yaml +++ /dev/null @@ -1,109 +0,0 @@ -id: legacy-media-paths -title: Migrate shared media from the legacy literal-tilde directory -summary: > - Earlier Dispatch versions accepted MEDIA_ROOT values such as - ~/.dispatch/media without expanding the tilde. Those installs stored shared - media beneath a literal `~` directory inside the Dispatch install. This - migration moves that legacy tree to the location the fixed runtime reads, so - historical dispatch_list_media entries remain readable after the - path-resolution fix. Installs whose MEDIA_ROOT is an absolute path were - never affected and this migration is a no-op for them. - -alreadySatisfied: - description: > - Either the install's effective MEDIA_ROOT does not begin with `~` — in - which case it never wrote to a literal-tilde path and nothing needs to - happen — or the legacy directory derived from that MEDIA_ROOT is absent or - empty, no active agent has a stored media_dir beginning with `~`, and the - current service is healthy on the target tag. Do not read MEDIA_ROOT from - /.env alone: the runtime loads that file through dotenv, - which does not override a value already set in the service process - environment (systemd Environment= including drop-ins and EnvironmentFile=, - launchd EnvironmentVariables, docker -e). Run `bin/migrate-legacy-media - --dry-run`, which asks systemd for the effective environment before falling - back to .env and reports which root it resolved and from where. - -instructions: - - > - First determine the install's effective MEDIA_ROOT. Run - `bin/migrate-legacy-media --dry-run`: it resolves the root from the - service's effective environment — asking systemd, so drop-ins and - EnvironmentFile= are covered, or reading the launchd plist — before falling - back to /.env, and reports which source it used. If a - systemd service exists but cannot be interrogated it exits 1 rather than - guessing. - Do not classify from .env alone — dotenv does not override a value already - present in the service process environment, so .env can disagree with what - the service actually ran with. If the resolved root does not begin with - `~`, this migration is an explicit no-op: report that the install was never - affected, do NOT stop any agents, and continue with the rest of the update - plan. Only tilde-configured installs need the steps below. - - > - If the command exits 1 asking for --media-root, it could not establish the - effective media root: either a systemd service it cannot interrogate, or a - literal-tilde tree the readable configuration does not explain — the - service ran with an override this script cannot see. Determine the value that - service actually used (its unit, plist, or container definition) and rerun - with --media-root set to it. Do not treat this as a no-op. - - > - During inspect on a tilde-configured install, check whether the legacy - directory / exists and list - every active agent. Running `bin/migrate-legacy-media --dry-run` is the - supported way to resolve the media root and see the legacy tree's contents - without moving anything. - - If alreadySatisfied is false, use the managed update endpoint to install - the target release and wait for the service to become healthy. - - > - After the target service is healthy, stop every active agent on this - install cleanly. Stop them all rather than trying to identify which ones - predate the fix: a still-running agent holds a literal-tilde - DISPATCH_MEDIA_DIR in its retained environment and can recreate the legacy - tree after the move, and there is no reliable way to inspect a live - agent's launch-time environment. If an agent cannot be safely stopped, - report the update as blocked and do not run the file migration. - - From the Dispatch install directory, run - bin/migrate-legacy-media --dry-run and inspect its summary before moving - anything. Do not manually overwrite destination files. Exit status 1 means - the scan itself was incomplete (for example an unreadable directory) — - treat that as blocked and fix it before proceeding, because a partial scan - can otherwise look like a clean result. - - Run bin/migrate-legacy-media --apply only when the dry run reports zero - conflicts and zero unsupported entries. The command is idempotent and - retains conflicting source files for manual recovery. - - If the command reports conflicts or unsupported entries, report the - update as blocked with the affected relative paths; do not delete either - copy. Otherwise confirm the legacy directory is absent or empty. - - Restart each agent stopped for this migration only after the file migration - succeeds (including when the migration is a no-op), so its - DISPATCH_MEDIA_DIR is recreated from the corrected absolute path. - - Confirm $DISPATCH_API_URL/api/v1/health returns status=ok and release.json - under the install directory reports the target tag. - -validation: - requiredChecks: - - service_restarted - - health_endpoint - - version_converged - -rollback: - - > - Before bin/migrate-legacy-media --apply has run, rollback is unrestricted: - no files have moved, so the previous release resolves media exactly as it - did before. Roll back using the normal Dispatch rollback flow if the target - service does not return healthy. - - > - After --apply has moved files, a plain rollback is NOT safe. The - pre-fix runtime resolves `~` literally again, while the migrated files now - exist only under the expanded home path — the service will pass its health - check while every migrated media file reads as missing. Prefer staying on - the fixed release and resolving the failure forward. - - > - If rolling back after a move is unavoidable, first move the migrated files - back into the literal-tilde tree under the install directory before - starting the old binary, so the pre-fix runtime finds them where it - expects. Never delete either copy while doing so. - - If media migration has started and files remain in both places, leave both - the corrected destination files and any unresolved source files in place; - the migration never overwrites destination data, so rerunning it is safe - after recovery. - - Restart the prior service and confirm its health endpoint returns status=ok. From 270d3a7fe7f0e37da6725f6091da2faebc31c35c Mon Sep 17 00:00:00 2001 From: Brad Harris Date: Thu, 20 Aug 2026 22:07:49 -0600 Subject: [PATCH 6/7] fix: resolve every configured path through one shared helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Expanding `~` for MEDIA_ROOT alone was a half-measure. Dispatch reads ten path-ish env vars and only TLS_CERT/TLS_KEY expanded a leading tilde, so the same `~/foo` string worked for a TLS cert and silently produced a directory *named* `~` for media — writes succeeding where nothing could find them again. Fixing one var moved that inconsistency rather than removing it. Rather than add a third tilde helper, this builds on the one already in the repo. `shared/lib/resolve-tilde.ts` gains `resolveConfiguredPath` — resolveTilde plus path.resolve — and the duplicate `resolveStoragePath` added to shared/media.ts is deleted. Every configured path now goes through it: MEDIA_ROOT, TLS_CERT/KEY, the four store paths, the release cache dir, the runtime path, the service definition path, the authoring repo dir, and DISPATCH_SERVER_DIR. Defaults were already absolute via os.homedir(), so only the configured branch changes behaviour; path.resolve on an absolute path is a no-op. Tests cover the helper directly and assert two representative stores — one resolving per call, one at module load — write to the expanded location and never create a literal `~` directory. All three fail against the unfixed stores. Co-Authored-By: Claude Opus 5 --- apps/server/src/agents/telemetry.ts | 4 +- apps/server/src/applied-migrations-store.ts | 5 +- apps/server/src/assisted-update-store.ts | 5 +- apps/server/src/config.ts | 8 +- apps/server/src/release-candidate-store.ts | 5 +- apps/server/src/release-checks.ts | 3 +- apps/server/src/release-store.ts | 6 +- apps/server/src/release-tarball-cache.ts | 5 +- apps/server/src/server.ts | 6 +- apps/server/src/server/release-helpers.ts | 7 +- apps/server/src/shared/lib/resolve-tilde.ts | 14 +++ apps/server/src/shared/media.ts | 14 +-- apps/server/test/configured-paths.test.ts | 101 ++++++++++++++++++++ apps/server/test/resolve-tilde.test.ts | 42 +++++++- apps/server/test/shared-media.test.ts | 4 +- 15 files changed, 193 insertions(+), 36 deletions(-) create mode 100644 apps/server/test/configured-paths.test.ts diff --git a/apps/server/src/agents/telemetry.ts b/apps/server/src/agents/telemetry.ts index 111807201..bbcd19e4f 100644 --- a/apps/server/src/agents/telemetry.ts +++ b/apps/server/src/agents/telemetry.ts @@ -2,7 +2,7 @@ import path from "node:path"; import type { Pool } from "pg"; -import { resolveStoragePath } from "../shared/media.js"; +import { resolveConfiguredPath } from "../shared/lib/resolve-tilde.js"; import type { AgentGitContext, AgentPin } from "./types.js"; export type ActivitySummaryResult = { @@ -516,7 +516,7 @@ export async function listMedia( return result.rows.map((row) => ({ fileName: row.fileName, filePath: path.join( - resolveStoragePath(row.mediaDir ?? fallbackMediaDir(agentId)), + resolveConfiguredPath(row.mediaDir ?? fallbackMediaDir(agentId)), row.fileName ), description: row.description, diff --git a/apps/server/src/applied-migrations-store.ts b/apps/server/src/applied-migrations-store.ts index dc7945883..6a94fe712 100644 --- a/apps/server/src/applied-migrations-store.ts +++ b/apps/server/src/applied-migrations-store.ts @@ -1,5 +1,6 @@ import { randomBytes } from "node:crypto"; import os from "node:os"; +import { resolveConfiguredPath } from "./shared/lib/resolve-tilde.js"; import path from "node:path"; import { mkdir, readFile, rename, writeFile } from "node:fs/promises"; @@ -14,9 +15,9 @@ import { mkdir, readFile, rename, writeFile } from "node:fs/promises"; // reloading the module. Production hosts only set the env var at boot, so // the lookup cost is negligible. function appliedStorePath(): string { - return ( + return resolveConfiguredPath( process.env.DISPATCH_APPLIED_MIGRATIONS_STORE_PATH ?? - path.join(os.homedir(), ".dispatch", "applied-migrations.json") + path.join(os.homedir(), ".dispatch", "applied-migrations.json") ); } diff --git a/apps/server/src/assisted-update-store.ts b/apps/server/src/assisted-update-store.ts index 2fd6a5078..aef46a226 100644 --- a/apps/server/src/assisted-update-store.ts +++ b/apps/server/src/assisted-update-store.ts @@ -1,5 +1,6 @@ import { randomBytes } from "node:crypto"; import os from "node:os"; +import { resolveConfiguredPath } from "./shared/lib/resolve-tilde.js"; import path from "node:path"; import { mkdir, readFile, rename, writeFile } from "node:fs/promises"; import type { AssistedUpdateMetadata } from "./release-metadata.js"; @@ -71,9 +72,9 @@ export type AssistedUpdateState = { // reloading the module. Production hosts only set the env var at boot, so // the lookup cost is negligible. function assistedStorePath(): string { - return ( + return resolveConfiguredPath( process.env.DISPATCH_ASSISTED_UPDATE_STORE_PATH ?? - path.join(os.homedir(), ".dispatch", "assisted-update.json") + path.join(os.homedir(), ".dispatch", "assisted-update.json") ); } diff --git a/apps/server/src/config.ts b/apps/server/src/config.ts index f9f951fb3..5a81ea830 100644 --- a/apps/server/src/config.ts +++ b/apps/server/src/config.ts @@ -4,7 +4,7 @@ import { readFileSync } from "node:fs"; import os from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; -import { resolveStoragePath } from "./shared/media.js"; +import { resolveConfiguredPath } from "./shared/lib/resolve-tilde.js"; const __dirname = path.dirname(fileURLToPath(import.meta.url)); @@ -47,8 +47,8 @@ function loadTls(): TlsConfig | null { throw new Error("Both TLS_CERT and TLS_KEY must be set to enable TLS"); } return { - cert: readFileSync(resolveStoragePath(certPath)), - key: readFileSync(resolveStoragePath(keyPath)), + cert: readFileSync(resolveConfiguredPath(certPath)), + key: readFileSync(resolveConfiguredPath(keyPath)), }; } @@ -80,7 +80,7 @@ export function loadConfig(): AppConfig { port: Number(process.env.DISPATCH_PORT ?? process.env.PORT ?? 6767), databaseUrl: requireEnv("DATABASE_URL"), authToken: "", // resolved from DB in start() via getOrCreateAuthToken() - mediaRoot: resolveStoragePath( + mediaRoot: resolveConfiguredPath( process.env.MEDIA_ROOT ?? path.join(os.homedir(), ".dispatch", "media") ), dispatchBinDir: path.resolve(__dirname, "..", "..", "..", "bin"), diff --git a/apps/server/src/release-candidate-store.ts b/apps/server/src/release-candidate-store.ts index 64dad5494..1541bc8f2 100644 --- a/apps/server/src/release-candidate-store.ts +++ b/apps/server/src/release-candidate-store.ts @@ -1,11 +1,12 @@ import os from "node:os"; +import { resolveConfiguredPath } from "./shared/lib/resolve-tilde.js"; import path from "node:path"; import { mkdir, readFile, rename, rm, writeFile } from "node:fs/promises"; function candidateStorePath(): string { - return ( + return resolveConfiguredPath( process.env.DISPATCH_RELEASE_CANDIDATE_STORE_PATH ?? - path.join(os.homedir(), ".dispatch", "release-candidate.json") + path.join(os.homedir(), ".dispatch", "release-candidate.json") ); } diff --git a/apps/server/src/release-checks.ts b/apps/server/src/release-checks.ts index aea745014..842b92eb9 100644 --- a/apps/server/src/release-checks.ts +++ b/apps/server/src/release-checks.ts @@ -1,6 +1,7 @@ import { lstat, readFile } from "node:fs/promises"; import https from "node:https"; import os from "node:os"; +import { resolveConfiguredPath } from "./shared/lib/resolve-tilde.js"; import path from "node:path"; import type { RequiredCheckName } from "./release-metadata.js"; import { readReleaseStore } from "./release-store.js"; @@ -153,7 +154,7 @@ function escapeRegex(value: string): string { function serviceDefinitionPath(): string { const configured = process.env.DISPATCH_SERVICE_DEFINITION_PATH?.trim(); - if (configured) return configured; + if (configured) return resolveConfiguredPath(configured); return process.platform === "darwin" ? path.join( os.homedir(), diff --git a/apps/server/src/release-store.ts b/apps/server/src/release-store.ts index 9b931fe5f..027e7a8ce 100644 --- a/apps/server/src/release-store.ts +++ b/apps/server/src/release-store.ts @@ -1,13 +1,15 @@ import os from "node:os"; +import { resolveConfiguredPath } from "./shared/lib/resolve-tilde.js"; import path from "node:path"; import { mkdir, readFile, writeFile } from "node:fs/promises"; // Isolated dev stacks and E2E runs set DISPATCH_RELEASE_STORE_PATH to keep // from reading the host's production release state. Default is the // machine-scoped production path. -const RELEASE_STORE_PATH = +const RELEASE_STORE_PATH = resolveConfiguredPath( process.env.DISPATCH_RELEASE_STORE_PATH ?? - path.join(os.homedir(), ".dispatch", "release.json"); + path.join(os.homedir(), ".dispatch", "release.json") +); export type ReleaseRecord = { tag: string; diff --git a/apps/server/src/release-tarball-cache.ts b/apps/server/src/release-tarball-cache.ts index 71fe5ff28..0db90f80e 100644 --- a/apps/server/src/release-tarball-cache.ts +++ b/apps/server/src/release-tarball-cache.ts @@ -4,6 +4,7 @@ import { mkdir, mkdtemp, readFile, rename, rm, stat } from "node:fs/promises"; import { readdir, unlink } from "node:fs/promises"; import https from "node:https"; import os from "node:os"; +import { resolveConfiguredPath } from "./shared/lib/resolve-tilde.js"; import path from "node:path"; import { Readable } from "node:stream"; import { pipeline } from "node:stream/promises"; @@ -32,9 +33,9 @@ export const RELEASE_ARTIFACT_NAME = "dispatch-release.tar.gz"; // reloading the module. Production hosts only set the env var at boot, so // the lookup cost is negligible. function cacheDir(): string { - return ( + return resolveConfiguredPath( process.env.DISPATCH_RELEASE_CACHE_DIR ?? - path.join(os.homedir(), ".dispatch", "cache") + path.join(os.homedir(), ".dispatch", "cache") ); } diff --git a/apps/server/src/server.ts b/apps/server/src/server.ts index 91edbd1c1..186d6307c 100644 --- a/apps/server/src/server.ts +++ b/apps/server/src/server.ts @@ -159,6 +159,7 @@ import { type HttpRequestToken, } from "./observability/service-resources.js"; import { readServiceResourcesCollectionEnabled } from "./observability/service-resources-settings.js"; +import { resolveConfiguredPath } from "./shared/lib/resolve-tilde.js"; const config = loadConfig(); const app = Fastify({ @@ -260,9 +261,10 @@ function withStreamFlag( return { ...agent, hasStream: streamManager.hasStream(agent.id) }; } -const serverDir = +const serverDir = resolveConfiguredPath( process.env.DISPATCH_SERVER_DIR ?? - path.join(os.homedir(), ".dispatch", "server"); + path.join(os.homedir(), ".dispatch", "server") +); const releaseRuntime = createReleaseRuntime({ pool, config, diff --git a/apps/server/src/server/release-helpers.ts b/apps/server/src/server/release-helpers.ts index 1afc73e9f..e4d6c9e3a 100644 --- a/apps/server/src/server/release-helpers.ts +++ b/apps/server/src/server/release-helpers.ts @@ -1,5 +1,7 @@ import path from "node:path"; +import { resolveConfiguredPath } from "../shared/lib/resolve-tilde.js"; + export type RunCommand = ( command: string, args: string[], @@ -45,7 +47,8 @@ export function isReleaseAuthoringEnabled(): boolean { * must agree on which checkout that is. */ export function resolveAuthoringRepoDir(serverDir: string): string { - return process.env.DISPATCH_RELEASE_AUTHORING_REPO_DIR?.trim() || serverDir; + const configured = process.env.DISPATCH_RELEASE_AUTHORING_REPO_DIR?.trim(); + return configured ? resolveConfiguredPath(configured) : serverDir; } export type AuthoringRemoteRefreshResult = @@ -150,7 +153,7 @@ export function compareSemver(a: string, b: string): number { export function fixedRuntimePath(serverDir: string): string { const configured = process.env.DISPATCH_RUNTIME_PATH?.trim(); return configured - ? path.resolve(configured) + ? resolveConfiguredPath(configured) : path.join(serverDir, "dispatch"); } diff --git a/apps/server/src/shared/lib/resolve-tilde.ts b/apps/server/src/shared/lib/resolve-tilde.ts index 9543d4a9c..4282e25e8 100644 --- a/apps/server/src/shared/lib/resolve-tilde.ts +++ b/apps/server/src/shared/lib/resolve-tilde.ts @@ -6,3 +6,17 @@ export function resolveTilde(value: string): string { if (value === "~") return os.homedir(); return value; } + +/** + * Resolve a path that came from configuration — an env var or a stored + * column — into an absolute path. + * + * Config values are not read by a shell, so a leading `~` arrives as a + * literal character. Left alone it becomes a directory *named* `~` next to + * the process's working directory, which fails silently: writes succeed, + * and nothing can find them again. Every configured path goes through here + * so `~` means the same thing everywhere it can be written. + */ +export function resolveConfiguredPath(value: string): string { + return path.resolve(resolveTilde(value)); +} diff --git a/apps/server/src/shared/media.ts b/apps/server/src/shared/media.ts index 75013fac4..98d8cb33e 100644 --- a/apps/server/src/shared/media.ts +++ b/apps/server/src/shared/media.ts @@ -1,16 +1,6 @@ import path from "node:path"; -import os from "node:os"; -/** Resolve a storage path without treating a leading `~` as a literal name. */ -export function resolveStoragePath(storagePath: string): string { - const expanded = - storagePath === "~" - ? os.homedir() - : storagePath.startsWith("~/") - ? path.join(os.homedir(), storagePath.slice(2)) - : storagePath; - return path.resolve(expanded); -} +import { resolveConfiguredPath } from "./lib/resolve-tilde.js"; import { extensionForMime, @@ -58,7 +48,7 @@ export function resolveMediaDir( mediaDir: string | null, mediaRoot: string ): string { - return resolveStoragePath(mediaDir ?? path.join(mediaRoot, agentId)); + return resolveConfiguredPath(mediaDir ?? path.join(mediaRoot, agentId)); } export function toMediaKey(file: { name: string; updatedAt: string }): string { diff --git a/apps/server/test/configured-paths.test.ts b/apps/server/test/configured-paths.test.ts new file mode 100644 index 000000000..9e8c1be9a --- /dev/null +++ b/apps/server/test/configured-paths.test.ts @@ -0,0 +1,101 @@ +import { mkdtemp, rm, stat } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; + +import { afterEach, describe, expect, it, vi } from "vitest"; + +/** + * Every path Dispatch reads from configuration must expand a leading `~`. + * + * Each of these modules resolves its own env var, so the expansion is easy to + * add in one place and forget in another — which is what happened with + * MEDIA_ROOT: a literal `~` produced a directory *named* `~` beside the + * process working directory, writes succeeded, and nothing could find them + * again. These assert the file lands at the expanded location and that no + * literal-tilde directory is created anywhere. + * + * They cover both shapes present in the codebase: a path resolved inside a + * function (applied-migrations-store) and one resolved once at module load + * (release-store), which only reads the env var on first import. + */ + +let tempHome: string; +const cleanup: string[] = []; + +async function withTildeConfig( + envName: string, + relative: string, + body: (expected: string) => Promise +): Promise { + tempHome = await mkdtemp(path.join(os.tmpdir(), "dispatch-cfg-home-")); + cleanup.push(tempHome); + const prevHome = process.env.HOME; + const prevValue = process.env[envName]; + // os.homedir() reads $HOME on POSIX, so this keeps the test off the real one. + process.env.HOME = tempHome; + process.env[envName] = `~/${relative}`; + vi.resetModules(); + try { + return await body(path.join(tempHome, relative)); + } finally { + if (prevHome === undefined) delete process.env.HOME; + else process.env.HOME = prevHome; + if (prevValue === undefined) delete process.env[envName]; + else process.env[envName] = prevValue; + } +} + +afterEach(async () => { + vi.resetModules(); + await Promise.all( + cleanup.splice(0).map((dir) => rm(dir, { recursive: true, force: true })) + ); + await rm(path.join(process.cwd(), "~"), { recursive: true, force: true }); +}); + +describe("configured paths expand a leading tilde", () => { + it("DISPATCH_APPLIED_MIGRATIONS_STORE_PATH (resolved per call)", async () => { + await withTildeConfig( + "DISPATCH_APPLIED_MIGRATIONS_STORE_PATH", + "state/applied-migrations.json", + async (expected) => { + const store = await import("../src/applied-migrations-store.js"); + await store.writeAppliedMigrationsState({ + appliedMigrations: { + "some-id": { appliedAt: "now", targetTag: "v1" }, + }, + }); + expect((await stat(expected)).isFile()).toBe(true); + } + ); + }); + + it("DISPATCH_RELEASE_STORE_PATH (resolved at module load)", async () => { + await withTildeConfig( + "DISPATCH_RELEASE_STORE_PATH", + "state/release.json", + async (expected) => { + const store = await import("../src/release-store.js"); + await store.writeReleaseStore({ + tag: "v1.2.3", + deployedAt: new Date(0).toISOString(), + }); + expect((await stat(expected)).isFile()).toBe(true); + } + ); + }); + + it("never creates a directory literally named ~", async () => { + await withTildeConfig( + "DISPATCH_APPLIED_MIGRATIONS_STORE_PATH", + "state/applied-migrations.json", + async () => { + const store = await import("../src/applied-migrations-store.js"); + await store.writeAppliedMigrationsState({ appliedMigrations: {} }); + await expect(stat(path.join(process.cwd(), "~"))).rejects.toMatchObject( + { code: "ENOENT" } + ); + } + ); + }); +}); diff --git a/apps/server/test/resolve-tilde.test.ts b/apps/server/test/resolve-tilde.test.ts index e0ad895bd..42aa7cade 100644 --- a/apps/server/test/resolve-tilde.test.ts +++ b/apps/server/test/resolve-tilde.test.ts @@ -3,7 +3,10 @@ import path from "node:path"; import { describe, expect, it } from "vitest"; -import { resolveTilde } from "../src/shared/lib/resolve-tilde.js"; +import { + resolveConfiguredPath, + resolveTilde, +} from "../src/shared/lib/resolve-tilde.js"; describe("resolveTilde", () => { const home = os.homedir(); @@ -40,3 +43,40 @@ describe("resolveTilde", () => { expect(resolveTilde("~/.config")).toBe(path.join(home, ".config")); }); }); + +describe("resolveConfiguredPath", () => { + const home = os.homedir(); + + it("expands a leading tilde instead of creating a directory named ~", () => { + // The bug this exists to prevent: a config value is never read by a + // shell, so an unexpanded "~/..." becomes a literal "~" directory next + // to the process cwd, and writes succeed where nothing can find them. + const resolved = resolveConfiguredPath("~/.dispatch/media"); + expect(resolved).toBe(path.join(home, ".dispatch", "media")); + expect(resolved).not.toContain("~"); + }); + + it("expands a bare tilde", () => { + expect(resolveConfiguredPath("~")).toBe(path.resolve(home)); + }); + + it("leaves an absolute path unchanged", () => { + expect(resolveConfiguredPath("/var/lib/dispatch/media")).toBe( + "/var/lib/dispatch/media" + ); + }); + + it("makes a relative path absolute against the working directory", () => { + expect(resolveConfiguredPath("relative/dir")).toBe( + path.resolve("relative/dir") + ); + }); + + it("does not expand ~user-style paths, but still absolutizes them", () => { + // No home lookup for another user, so `~otheruser` stays a literal name. + // Matching resolveTilde here is deliberate: expanding it would guess. + expect(resolveConfiguredPath("~otheruser/dir")).toBe( + path.resolve("~otheruser/dir") + ); + }); +}); diff --git a/apps/server/test/shared-media.test.ts b/apps/server/test/shared-media.test.ts index 059bcd817..3967578e8 100644 --- a/apps/server/test/shared-media.test.ts +++ b/apps/server/test/shared-media.test.ts @@ -1,6 +1,7 @@ import os from "node:os"; import path from "node:path"; +import { resolveConfiguredPath } from "../src/shared/lib/resolve-tilde.js"; import { describe, expect, it } from "vitest"; import { @@ -11,7 +12,6 @@ import { isValidMediaKey, mimeType, resolveMediaDir, - resolveStoragePath, sanitizeUploadedFileName, toMediaKey, } from "../src/shared/media.js"; @@ -22,7 +22,7 @@ import { describe("media storage paths", () => { it("expands a home-relative storage path to an absolute path", () => { - expect(resolveStoragePath("~/.dispatch/media")).toBe( + expect(resolveConfiguredPath("~/.dispatch/media")).toBe( path.join(os.homedir(), ".dispatch", "media") ); }); From db4a0eae33756a9aafec272f966a3ba574891791 Mon Sep 17 00:00:00 2001 From: Brad Harris Date: Fri, 21 Aug 2026 08:36:37 -0600 Subject: [PATCH 7/7] style: keep the new import out of the node builtin blocks The resolveConfiguredPath imports landed between "node:os" and "node:path" in six files. Prettier does not reorder imports, so nothing complained. Moved them below the builtins to match the surrounding convention. Co-Authored-By: Claude Opus 5 --- apps/server/src/applied-migrations-store.ts | 3 ++- apps/server/src/assisted-update-store.ts | 3 ++- apps/server/src/release-candidate-store.ts | 3 ++- apps/server/src/release-checks.ts | 3 ++- apps/server/src/release-store.ts | 3 ++- apps/server/src/release-tarball-cache.ts | 3 ++- 6 files changed, 12 insertions(+), 6 deletions(-) diff --git a/apps/server/src/applied-migrations-store.ts b/apps/server/src/applied-migrations-store.ts index 6a94fe712..b957469f0 100644 --- a/apps/server/src/applied-migrations-store.ts +++ b/apps/server/src/applied-migrations-store.ts @@ -1,9 +1,10 @@ import { randomBytes } from "node:crypto"; import os from "node:os"; -import { resolveConfiguredPath } from "./shared/lib/resolve-tilde.js"; import path from "node:path"; import { mkdir, readFile, rename, writeFile } from "node:fs/promises"; +import { resolveConfiguredPath } from "./shared/lib/resolve-tilde.js"; + /** * Local source of truth for which install-update migrations (CRU-146) have * been applied on this install. Lives outside the repo checkout so reinstalls diff --git a/apps/server/src/assisted-update-store.ts b/apps/server/src/assisted-update-store.ts index aef46a226..74a86bba3 100644 --- a/apps/server/src/assisted-update-store.ts +++ b/apps/server/src/assisted-update-store.ts @@ -1,8 +1,9 @@ import { randomBytes } from "node:crypto"; import os from "node:os"; -import { resolveConfiguredPath } from "./shared/lib/resolve-tilde.js"; import path from "node:path"; import { mkdir, readFile, rename, writeFile } from "node:fs/promises"; + +import { resolveConfiguredPath } from "./shared/lib/resolve-tilde.js"; import type { AssistedUpdateMetadata } from "./release-metadata.js"; import type { CheckResult } from "./release-checks.js"; import type { UpdateMigrationManifest } from "./update-migrations.js"; diff --git a/apps/server/src/release-candidate-store.ts b/apps/server/src/release-candidate-store.ts index 1541bc8f2..1de74b270 100644 --- a/apps/server/src/release-candidate-store.ts +++ b/apps/server/src/release-candidate-store.ts @@ -1,8 +1,9 @@ import os from "node:os"; -import { resolveConfiguredPath } from "./shared/lib/resolve-tilde.js"; import path from "node:path"; import { mkdir, readFile, rename, rm, writeFile } from "node:fs/promises"; +import { resolveConfiguredPath } from "./shared/lib/resolve-tilde.js"; + function candidateStorePath(): string { return resolveConfiguredPath( process.env.DISPATCH_RELEASE_CANDIDATE_STORE_PATH ?? diff --git a/apps/server/src/release-checks.ts b/apps/server/src/release-checks.ts index 842b92eb9..449bd5d2d 100644 --- a/apps/server/src/release-checks.ts +++ b/apps/server/src/release-checks.ts @@ -1,8 +1,9 @@ import { lstat, readFile } from "node:fs/promises"; import https from "node:https"; import os from "node:os"; -import { resolveConfiguredPath } from "./shared/lib/resolve-tilde.js"; import path from "node:path"; + +import { resolveConfiguredPath } from "./shared/lib/resolve-tilde.js"; import type { RequiredCheckName } from "./release-metadata.js"; import { readReleaseStore } from "./release-store.js"; import { errorMessage } from "./shared/lib/error-message.js"; diff --git a/apps/server/src/release-store.ts b/apps/server/src/release-store.ts index 027e7a8ce..16ab964e5 100644 --- a/apps/server/src/release-store.ts +++ b/apps/server/src/release-store.ts @@ -1,8 +1,9 @@ import os from "node:os"; -import { resolveConfiguredPath } from "./shared/lib/resolve-tilde.js"; import path from "node:path"; import { mkdir, readFile, writeFile } from "node:fs/promises"; +import { resolveConfiguredPath } from "./shared/lib/resolve-tilde.js"; + // Isolated dev stacks and E2E runs set DISPATCH_RELEASE_STORE_PATH to keep // from reading the host's production release state. Default is the // machine-scoped production path. diff --git a/apps/server/src/release-tarball-cache.ts b/apps/server/src/release-tarball-cache.ts index 0db90f80e..437e16c75 100644 --- a/apps/server/src/release-tarball-cache.ts +++ b/apps/server/src/release-tarball-cache.ts @@ -4,10 +4,11 @@ import { mkdir, mkdtemp, readFile, rename, rm, stat } from "node:fs/promises"; import { readdir, unlink } from "node:fs/promises"; import https from "node:https"; import os from "node:os"; -import { resolveConfiguredPath } from "./shared/lib/resolve-tilde.js"; import path from "node:path"; import { Readable } from "node:stream"; import { pipeline } from "node:stream/promises"; + +import { resolveConfiguredPath } from "./shared/lib/resolve-tilde.js"; import { formatBytes } from "./shared/lib/format-bytes.js"; import { runCommand } from "./shared/lib/run-command.js";