Skip to content

fix: resolve a leading ~ in every configured path, not just some - #986

Open
selfcontained wants to merge 6 commits into
mainfrom
fix/legacy-shared-media-paths
Open

fix: resolve a leading ~ in every configured path, not just some#986
selfcontained wants to merge 6 commits into
mainfrom
fix/legacy-shared-media-paths

Conversation

@selfcontained

@selfcontained selfcontained commented Aug 20, 2026

Copy link
Copy Markdown
Owner

Problem

Dispatch reads ten path-ish values from configuration. Only TLS_CERT and TLS_KEY expanded a leading ~, via a local expandHome() helper in config.ts. MEDIA_ROOT did not.

So the same ~/foo string meant two different things depending on which variable you wrote it in. For media it became a directory literally named ~ beside the process working directory — writes succeeded, and nothing could ever find them again. Silent, not loud.

Configuration is not read by a shell, so a ~ arrives as a literal character; the only question is whether the program expands it. Dispatch had already answered yes — resolveTilde exists in shared/lib/ and is used for user-supplied paths in routes/jobs.ts, routes/templates.ts and routes/system.ts — it just wasn't applied to configured paths consistently.

Before / after, from the same live endpoint

An agent row with media_dir = '~/dispatch-devtest-media/agt_legacytilde' and the real file present at the expanded location:

GET /api/v1/agents/{id}/media/legacy-shot.png dispatch_list_mediafilePath
before 404 {"error":"Media file not found."} ~/dispatch-devtest-media/agt_legacytilde/legacy-shot.png
after 200, correct body /Users/brad/dispatch-devtest-media/agt_legacytilde/legacy-shot.png

The fix

One resolver, applied everywhere a path comes from configuration.

  • apps/server/src/shared/lib/resolve-tilde.ts gains resolveConfiguredPath(value)resolveTilde plus path.resolve. It builds on the helper already in the repo rather than adding a third one.
  • Applied at every configured-path site: MEDIA_ROOT, TLS_CERT/TLS_KEY, the four store paths (DISPATCH_APPLIED_MIGRATIONS_STORE_PATH, DISPATCH_ASSISTED_UPDATE_STORE_PATH, DISPATCH_RELEASE_CANDIDATE_STORE_PATH, DISPATCH_RELEASE_STORE_PATH), DISPATCH_RELEASE_CACHE_DIR, DISPATCH_RUNTIME_PATH, DISPATCH_SERVICE_DEFINITION_PATH, DISPATCH_RELEASE_AUTHORING_REPO_DIR, and DISPATCH_SERVER_DIR.
  • resolveMediaDir() and listMedia() also resolve the stored media_dir column, which inherits its value from mediaRoot.
  • config.ts drops its local expandHome(); process.env.HOME ?? "/tmp" becomes os.homedir().
  • docs/10-operations-runbook.md — the runbook documented MEDIA_ROOT's default as ~/.dispatch/media, which is both wrong (the code default is absolute) and precisely the value that triggers the bug when copied into .env. Corrected.

Behaviour is unchanged for existing installs: every default was already absolute via os.homedir(), so only the configured branch is affected, and path.resolve on an absolute path is a no-op.

Why there is no migration

Earlier revisions of this branch carried a required assisted update, a bin/migrate-legacy-media bash migrator and a manifest. All removed:

  • Dispatch never shipped a tilde default. config.ts has used path.join(HOME, ".dispatch", "media") since the initial commit (v0.11.12), and bin/install-dispatch.sh never writes MEDIA_ROOT into .env. A stock install cannot reach the bug.
  • The vector was the docs, corrected above.
  • No stranded data exists. On the one machine known to have hit this, MEDIA_ROOT is unset and the only literal-tilde directory present is empty — zero files.

A required update that stops every agent to migrate nothing was the wrong trade.

Verification

  • pnpm run check clean; pnpm run test — 2808 passing; pnpm run test:e2e — 181 passed, 12 skipped.
  • Live against an isolated dev stack: server boots healthy with the resolver on every config path; the media before/after above, including the dispatch_share write path creating no literal ~ directory.
  • apps/server/test/configured-paths.test.ts covers both resolution shapes present in the codebase — one store resolving per call, one at module load — asserting the file lands at the expanded location and that no ~ directory is created. All three were confirmed to fail against the unfixed stores.
  • Helper-level coverage in apps/server/test/resolve-tilde.test.ts.

🤖 Generated with Claude Code

selfcontained and others added 5 commits August 20, 2026 08:13
- Derive the legacy/expanded media paths from the install's effective
  MEDIA_ROOT (--media-root, env, or <install>/.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 <noreply@anthropic.com>
…ides

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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
…default

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 <noreply@anthropic.com>
@selfcontained selfcontained changed the title fix: expand ~ in shared-media paths and migrate legacy literal-tilde trees fix: expand ~ in shared-media paths Aug 21, 2026
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 <noreply@anthropic.com>
@selfcontained selfcontained changed the title fix: expand ~ in shared-media paths fix: resolve a leading ~ in every configured path, not just some Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant