Skip to content

Refuse to open a registry this machine does not own (#87) - #88

Merged
spaceshipmike merged 3 commits into
mainfrom
app-db-owner-guard
Jul 30, 2026
Merged

Refuse to open a registry this machine does not own (#87)#88
spaceshipmike merged 3 commits into
mainfrom
app-db-owner-guard

Conversation

@spaceshipmike

Copy link
Copy Markdown
Owner

Ships option (b) of #87 — the cheap refusal, not the remote mode.

The failure this removes

packages/app links better-sqlite3 and opens registry.db directly. Served mode (#85, D-017) moved the database boundary for agent sessions but gave the app no client, so on a machine that is not the registry owner the app opens whatever file sits at the canonical path — a different database. It does not error: it silently shows and mutates stale data, and when the file is absent initDb creates a fresh empty registry, which presents as total data loss (served-mode §3.8).

What it does

packages/app/src/main/db-owner.ts decides from evidence on disk whether this machine may open the local registry. It runs in app.whenReady() before registerIpcHandlers (the site that constructs the Registry), before the tray, and before the shortcuts — so a refusal has no IPC surface at all and nothing reads or writes the database file. The refusal is a window, not a log line: scriptless static HTML, no preload, sandboxed, naming the owner, the served URL, the file it did not open, and what to do. Closing it quits, whatever menu-bar persistence says.

Two signals, in precedence order:

Signal Catches Written by
<data-dir>/registry-remote.json (or SETLIST_REGISTRY_REMOTE_URL) a machine whose registry is served elsewhere — the only signal that catches an absent local file, which is the laptop's state after runbook Phase 7 the operator
<data-dir>/db-owner.json naming a different machine a copied/cloned data dir, with no operator action the app, on first launch on the owner machine

The asymmetries (they are the design, not decoration)

  • Refusal requires positive evidence. A missing marker, an unparseable marker, and an undeterminable machine identity all allow. A guard whose false positive is "the app will not start" must not fire on ambiguity — the failure prevented is silent divergence, not startup.
  • A blank SETLIST_REGISTRY_REMOTE_URL means unset (env vars get exported empty by accident), but a registry-remote.json that exists without a usable URL is an error, not a fallback to the local file — that file exists only because somebody wrote it. Same rule as served mode's empty token file.
  • Identity is hardware-derived (macOS IOPlatformUUID, Linux machine-id), never the hostname: hostnames churn, and a hostname-only mismatch would refuse to start on the owner machine.
  • SETLIST_APP_DB_OWNER_OVERRIDE=1 is the documented last resort, offered as the last step of every refusal.

Evidence

  • packages/app/src/main/db-owner.test.ts — 28 cases over the predicate: both refusal signals, every allow-on-ambiguity path, the pointer shapes an operator would actually write, blank-vs-invalid, the override, HTML escaping of values read off disk, and a real temp-dir claim → re-read-as-owner → refuse-a-foreign-dir walk asserting no database file ever appears.
  • packages/app/e2e/app.spec.ts — a real Electron launch against a data dir claimed by another machine: the refusal window renders, registry.db/-wal/-shm do not exist, and window.setlist is absent. Verified locally (npm run test:e2e -- --grep "refuses to open a registry" → passed); also verified by hand that the allow path still launches normally, claims the data dir, and loads the renderer.
  • npm test 1197 passed · npm run typecheck clean · npm run build -w packages/app + npm run verify:mcp-abi OK.

Docs / agent model

  • served-mode §3.8.1 — the guard, the two files, the env-var table, and the honest limits.
  • runbook Phase 7 — writing the pointer is now step 1 (the step that actually closes the silent-divergence class); the chmod 444 neutralization stays, because the CLI is not covered.
  • New State ownership invariant (ENFORCED), plus a CLAUDE.md line.

Known limits, stated rather than papered over

  • The CLI and MCP server on a non-owner machine are still unguarded (the runbook takes the CLI off PATH).
  • A machine with neither file gets no protection — the guard never refuses on absence of evidence, so the laptop needs Phase 7's pointer file.
  • A refusing machine does not auto-update (the updater is not initialized in refusal mode).
  • The second commit is a mechanical out/ bundle rebuild (app: committed packages/app/out/ bundle drifts from source per-PR (silent) #63 — nothing asserts the tracked bundle matches source; it also picks up drift from earlier merges).
  • Pre-existing and unrelated: 9 of the 12 app e2e tests time out on this machine, on the HEAD bundle too (verified by re-running one against the pre-change bundle). Not investigated here; e2e is not in CI.

Addresses #87; the remote-capable app (option (a)) stays open there.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Twbdbg1U4BJo5tz7vFegsx

spaceshipmike and others added 2 commits July 29, 2026 22:51
The desktop app is the one surface with no protocol client: served mode
(#85, D-017) moved the database boundary for agent sessions, but the
Electron main process still builds `new Registry(...)` over whatever file
sits at the canonical path. On a machine that is not the registry owner
that is a *different* database, and it does not error — it silently shows
and mutates stale data, and when the file is absent `initDb` creates an
empty registry that presents as total data loss.

This is option (b) of #87 — the refusal, not the remote mode.

`packages/app/src/main/db-owner.ts` decides, from evidence on disk,
whether this machine may open the local registry. It runs in
`app.whenReady()` before `registerIpcHandlers` (which constructs the
`Registry`), before the tray and before the shortcuts, so a refusal has
no IPC surface at all and nothing reads or writes the database file. The
refusal is a window, not a log line: scriptless static HTML, no preload,
sandboxed, naming the owner, the served URL, the file it did not open,
and what to do about it. Closing it quits, whatever menu-bar persistence
says.

Two signals, in precedence order:

  - `<data-dir>/registry-remote.json` (or `SETLIST_REGISTRY_REMOTE_URL`),
    an operator's declaration that this machine's registry is served
    elsewhere. This is the only signal that can catch an *absent* local
    file, which is the laptop's state after the cutover runbook.
  - `<data-dir>/db-owner.json` naming a different machine — a copied or
    cloned data directory, caught with no operator action. The app writes
    this marker for itself on first launch.

The asymmetries are the design:

  - Refusal requires positive evidence. A missing marker, an unparseable
    marker, and an undeterminable machine identity all allow: a guard
    whose false positive is "the app will not start" must not fire on
    ambiguity, because the failure it prevents is silent divergence, not
    startup.
  - A blank `SETLIST_REGISTRY_REMOTE_URL` means unset (env vars get
    exported empty by accident), but a `registry-remote.json` that exists
    without a usable URL is an error rather than a fallback to the local
    file — that file exists only because somebody wrote it. Same rule as
    served mode's empty token file.
  - Identity is hardware-derived (macOS IOPlatformUUID, Linux
    machine-id), never the hostname: hostnames churn, and a hostname-only
    mismatch would refuse to start on the owner machine.
  - `SETLIST_APP_DB_OWNER_OVERRIDE=1` is the documented last resort.

Tests: 28 unit cases over the predicate (both refusal signals, every
allow-on-ambiguity path, the pointer shapes an operator would write, the
blank-vs-invalid asymmetry, the override, HTML escaping, and a temp-dir
claim/refuse walk asserting no database file appears), plus an Electron
e2e launch against a data dir claimed by another machine asserting the
refusal window, the absence of registry.db/-wal/-shm, and that no
`window.setlist` bridge exists.

Docs: served-mode §3.8.1 (the guard, the files, the env vars, the honest
limits), runbook Phase 7 now writes the pointer as its first step, a new
State-ownership invariant (ENFORCED), and CLAUDE.md.

Still open in #87: the app resolving its ~60 IPC channels through an MCP
client against the served registry. The CLI and MCP server on a
non-owner machine remain unguarded, and a machine with neither file gets
no protection.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Twbdbg1U4BJo5tz7vFegsx
Mechanical: `npm run build -w packages/app` output for the owner guard.
The tracked bundle also picks up drift from earlier merges that did not
rebuild it (#63 — nothing asserts `out/` matches source).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Twbdbg1U4BJo5tz7vFegsx
@spaceshipmike

Copy link
Copy Markdown
Owner Author

🤖 Codex review — REQUEST_CHANGES

Local Codex review of this PR's changes against origin/main.

The ownership guard has a startup-order regression: refusal mode still imports the full IPC/core stack before it can evaluate ownership, so it can fail before showing the intended refusal window.

Findings (1)

  • [P2] packages/app/src/main/index.ts:6index.ts imports resolveAppDbPath from ./ipc.js, and ipc.ts imports @setlist/core at module load time, including better-sqlite3. That happens before evaluateRegistryOwnership() runs, so a non-owner machine with a stranded/wrong native SQLite ABI can crash during import and never reach the refusal window. This also violates the intended isolation of refusal mode from the registry/IPC stack. Move the DB-path resolution used by the guard into a tiny module with no @setlist/core/better-sqlite3 dependency, or otherwise defer importing ipc.js until after ownership has been allowed.

Codex flagged that `index.ts` reached `resolveAppDbPath` through `./ipc.js`,
which imports `@setlist/core` (and so `better-sqlite3`) at module load —
before the guard could run. The stated crash does not reproduce
(better-sqlite3 loads its addon lazily inside the `Database` constructor,
`lib/database.js:48`, so refusal mode survived even with the native binary
removed entirely — verified), but relying on a dependency's internal
laziness to keep the "do not open this database" window alive is the wrong
guarantee. Make it structural:

  - `db-path.ts` resolves the path with no `@setlist/core` /
    `better-sqlite3` import. It re-derives core's canonical location as a
    *checked* duplicate: `db-path.test.ts` asserts it equals `getDbPath()`,
    so the mirror cannot drift silently.
  - `./ipc.js` is now loaded with `await import()` only after ownership is
    allowed, and lands in its own bundle chunk (`out/main/ipc-*.js`).

Verified: the refusal e2e passes with the native binding absent; the allow
path still claims the data dir and loads the renderer normally.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Twbdbg1U4BJo5tz7vFegsx
@spaceshipmike

Copy link
Copy Markdown
Owner Author

Response to the Codex finding (P2, index.ts:6)

Fixed in 6677882 — but the finding's stated failure mode does not reproduce, and the distinction matters for anyone reading this later.

What I could not reproduce: "a stranded/wrong native SQLite ABI can crash during import and never reach the refusal window." better-sqlite3 loads its addon lazily inside the Database constructor (node_modules/better-sqlite3/lib/database.js:48addon = DEFAULT_ADDON || (DEFAULT_ADDON = require('bindings')('better_sqlite3.node'))), so importing @setlist/core never dlopens anything. Empirically: with node_modules/better-sqlite3/build/Release/better_sqlite3.node removed entirely and the pre-fix bundle in place, the refusal e2e still passed — the window rendered.

Why I still made the change: keeping the "do not open this database" window alive must not depend on a third-party module's internal laziness. That is an implementation detail of a dependency, one release away from becoming a top-level require, and the thing it would break is the one surface that must work on a machine whose registry stack is suspect. So the isolation is now structural:

  • packages/app/src/main/db-path.ts resolves the database path with no @setlist/core / better-sqlite3 import. It re-derives core's canonical location as a checked duplicate — db-path.test.ts asserts it equals getDbPath(), so the mirror cannot drift silently (the alternative, an unchecked hardcoded path, would violate the "core resolves the canonical path" invariant).
  • ./ipc.js is loaded with await import() only after ownership is allowed. electron-vite emits it as its own chunk (out/main/ipc-FNHv0VE1.js), which is the structural proof: the entry file no longer contains the database stack.

Verification after the fix: refusal e2e passes both normally and with the native binding absent; the allow path still claims the data dir and loads the renderer (checked with a real electron . launch); npm test 1200 passed; typecheck clean; verify:mcp-abi OK.

The invariant and served-mode §3.8.1 now state the deferred-import property, so it is a documented guarantee rather than an accident of bundling.

@spaceshipmike

Copy link
Copy Markdown
Owner Author

🤖 Codex review — APPROVE

Local Codex review of this PR's changes against origin/main.

No blocking correctness issues found. I reviewed the HEAD diff against origin/main, including the owner-guard decision logic, Electron startup integration, IPC loading boundary, tests, docs, and generated app output. Typecheck could not be run because tsc is not installed in this worktree.

No issues found.

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