diff --git a/apps/server/src/agents/manager.ts b/apps/server/src/agents/manager.ts index 77274d12..c41cdb43 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 73f03a89..bbcd19e4 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 { resolveConfiguredPath } from "../shared/lib/resolve-tilde.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), + 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 dc794588..b957469f 100644 --- a/apps/server/src/applied-migrations-store.ts +++ b/apps/server/src/applied-migrations-store.ts @@ -3,6 +3,8 @@ import os from "node:os"; 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 @@ -14,9 +16,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 2fd6a507..74a86bba 100644 --- a/apps/server/src/assisted-update-store.ts +++ b/apps/server/src/assisted-update-store.ts @@ -2,6 +2,8 @@ import { randomBytes } from "node:crypto"; import os from "node:os"; 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"; @@ -71,9 +73,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 667e3d2a..5a81ea83 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 { resolveConfiguredPath } from "./shared/lib/resolve-tilde.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(resolveConfiguredPath(certPath)), + key: readFileSync(resolveConfiguredPath(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: resolveConfiguredPath( + 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/release-candidate-store.ts b/apps/server/src/release-candidate-store.ts index 64dad549..1de74b27 100644 --- a/apps/server/src/release-candidate-store.ts +++ b/apps/server/src/release-candidate-store.ts @@ -2,10 +2,12 @@ import os from "node:os"; 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 ( + 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 aea74501..449bd5d2 100644 --- a/apps/server/src/release-checks.ts +++ b/apps/server/src/release-checks.ts @@ -2,6 +2,8 @@ import { lstat, readFile } from "node:fs/promises"; import https from "node:https"; import os from "node:os"; 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"; @@ -153,7 +155,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 9b931fe5..16ab964e 100644 --- a/apps/server/src/release-store.ts +++ b/apps/server/src/release-store.ts @@ -2,12 +2,15 @@ import os from "node:os"; 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. -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 71fe5ff2..437e16c7 100644 --- a/apps/server/src/release-tarball-cache.ts +++ b/apps/server/src/release-tarball-cache.ts @@ -7,6 +7,8 @@ import os from "node:os"; 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"; @@ -32,9 +34,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 91edbd1c..186d6307 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 1afc73e9..e4d6c9e3 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 9543d4a9..4282e25e 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 e59cef81..98d8cb33 100644 --- a/apps/server/src/shared/media.ts +++ b/apps/server/src/shared/media.ts @@ -1,5 +1,7 @@ import path from "node:path"; +import { resolveConfiguredPath } from "./lib/resolve-tilde.js"; + import { extensionForMime, isDocumentFile, @@ -46,7 +48,7 @@ export function resolveMediaDir( mediaDir: string | null, mediaRoot: string ): string { - return 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 00000000..9e8c1be9 --- /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/db/agent-manager.test.ts b/apps/server/test/db/agent-manager.test.ts index 96924744..7ee22c58 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/resolve-tilde.test.ts b/apps/server/test/resolve-tilde.test.ts index e0ad895b..42aa7cad 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 8c55c934..3967578e 100644 --- a/apps/server/test/shared-media.test.ts +++ b/apps/server/test/shared-media.test.ts @@ -1,3 +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 { @@ -7,6 +11,7 @@ import { isTextFile, isValidMediaKey, mimeType, + resolveMediaDir, 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(resolveConfiguredPath("~/.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/docs/10-operations-runbook.md b/docs/10-operations-runbook.md index 3fd25f92..da7070ed 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) |