Add reproducible content-addressed scenario catalog - #4
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 807d5b91c1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| this.#migrate(); | ||
| this.assertHealthy(); | ||
| if (path !== ":memory:") { | ||
| chmodSync(path, 0o600); |
There was a problem hiding this comment.
Protect SQLite WAL sidecar files
When CAUSAL_LAB_DB points inside an existing traversable directory such as /tmp or a shared project directory, enabling WAL creates catalog.sqlite-wal and catalog.sqlite-shm with the process umask before this chmod runs; commonly those sidecars are 0644 and the WAL contains the canonical scenario/report JSON even though the main DB is 0600. Since mkdirSync(..., { mode: 0o700 }) does not change permissions on existing parents, either reject non-private parent directories or chmod/check the -wal/-shm files as well so catalog contents are not exposed to other local users.
Useful? React with 👍 / 👎.
| throw new Error("catalog path cannot be empty"); | ||
| } | ||
| if (path !== ":memory:") { | ||
| if (existsSync(path) && lstatSync(path).isSymbolicLink()) { |
There was a problem hiding this comment.
Reject dangling symlink catalog paths
When CAUSAL_LAB_DB names a dangling symlink, existsSync(path) is false, so this guard skips lstatSync and DatabaseSync follows the link and creates/opens the target. In a directory where the configured path can be pre-created or left stale, this bypasses the advertised symlink rejection and can make the catalog write/chmod an unintended file; call lstatSync even when existsSync is false and reject symlinks before opening.
Useful? React with 👍 / 👎.
| CREATE TRIGGER IF NOT EXISTS scenarios_immutable_update | ||
| BEFORE UPDATE ON scenarios BEGIN | ||
| SELECT RAISE(ABORT, 'scenarios are immutable'); | ||
| END; |
There was a problem hiding this comment.
Block replace inserts for immutable rows
These immutable triggers only cover explicit UPDATE and DELETE statements. With SQLite's default recursive_triggers setting, a maintenance/import path that uses INSERT OR REPLACE on an existing primary key can delete and reinsert the row without firing this trigger, so catalog rows are still mutable at the database layer despite the schema contract; add a duplicate-key BEFORE INSERT guard or enable recursive triggers before relying on these triggers.
Useful? React with 👍 / 👎.
| const migration = this.#database | ||
| .prepare("SELECT COALESCE(MAX(version), 0) AS version FROM schema_migrations") | ||
| .get() as { version: number }; | ||
| if (migration.version !== 1) { | ||
| throw new Error("catalog schema version is not supported"); |
There was a problem hiding this comment.
Verify existing catalog schema, not just version
When CAUSAL_LAB_DB points at an existing SQLite file that already has schema_migrations(version=1) but tables created without the STRICT/check/trigger definitions, CREATE TABLE IF NOT EXISTS leaves those objects untouched and this health check still passes because integrity and foreign-key checks do not validate schema shape. That lets a stale or drifted catalog run without the constraints the adapter relies on, so verify the table SQL/triggers/PRAGMAs or store a schema digest before accepting version 1.
Useful? React with 👍 / 👎.
Summary
Architecture
CRDT and simulation modules remain storage-free. The node:sqlite adapter is isolated at the composition boundary, rejects semantically invalid scenarios, uses immutable rows, and verifies schema, integrity, foreign keys, byte counts, and receipts.
Verification