Database permission hardening races with SQLite WAL removal

[munezaclovis]

Summary

A reconciliation failed transiently because PV observed pv.db-wal, then SQLite removed the ephemeral WAL before PV could chmod/validate it:

state error: filesystem error at ~/.pv/pv.db-wal:
No such file or directory (os error 2)

The next reconciliation started normally 12 seconds later. The evidence points to a TOCTOU race in database-file permission hardening rather than database corruption.

Current code path

Database::open_with_migrations opens SQLite and calls secure_database_files before and after connection configuration/migrations: database.rs.

secure_database_files checks each database/WAL/SHM path for existence and then performs separate chmod/metadata operations: fs.rs.

The database-side files include ephemeral pv.db-wal and pv.db-shm: fs.rs.

The Project config watcher opens a writable/migrating Database every 100 ms poll: watcher.rs, increasing opportunities for the race.

Likely sequence:

1. path_exists(pv.db-wal) returns true.
2. SQLite checkpoints/closes and removes the WAL.
3. secure_sensitive_file(pv.db-wal) receives ENOENT.
4. The whole database open/reconciliation fails.

Expected behavior

Disappearance of SQLite's optional WAL/SHM files during secure inspection should be handled as a benign race. Real permission, ownership, or I/O failures must remain errors, and the persistent main database must still be hardened.

Acceptance criteria

• A WAL or SHM file disappearing between discovery and chmod/metadata validation does not fail Database::open.
• A missing persistent pv.db after creation/open is not silently ignored.
• Permission and owner mismatches on existing database-side files still fail clearly.
• Add deterministic coverage that removes an auxiliary file between existence observation and the security operation.
• Preserve user-only permissions for database, WAL, and SHM whenever those files exist.
• Verify Project watcher polling cannot create failed reconciliation jobs from this race.
• Avoid broad retry loops or weakening filesystem ownership checks.

Related

• pv update incurs a 10-second launchd throttle after switching app binary #300
• Full incident evidence

[pullfrog]

Assessment

This is actionable. secure_database_files currently performs a separate path_exists check before secure_sensitive_file, while SQLite is free to unlink pv.db-wal and pv.db-shm at any point. Any NotFound from chmod, mode validation, or owner validation then escapes Database::open_with_migrations and can fail whichever daemon path opened the database. The Project watcher increases the frequency of opens, but it does not need a separate behavioral fix because the race is in the shared state-layer open path.

The persistent database needs stricter treatment than it receives today: the same existence-check skip can silently omit hardening if pv.db is absent. The fix should therefore distinguish the required database from optional SQLite side files rather than broadly ignoring missing paths. This is consistent with DESIGN.md; no design change or migration is needed.

Plan

1. Update crates/state/src/fs.rs to classify pv.db as required and pv.db-wal / pv.db-shm as ephemeral. Remove the preflight existence check from database hardening, always run the full 0600 mode and ownership operation, and treat only StateError::Filesystem with ErrorKind::NotFound as benign for the two ephemeral files. Preserve every other filesystem, permission, ownership, and unsupported-platform error unchanged.
2. Add a narrow internal test seam around the per-file hardening operation so a deterministic state test can observe an existing auxiliary file, remove it immediately before hardening, and verify the resulting NotFound is ignored. Add companion coverage proving a missing pv.db remains an error and non-NotFound failures for auxiliary files are propagated.
3. Extend the existing state foundation coverage to confirm Database::open still leaves every present database, WAL, and SHM file user-only. Keep Database::open_with_migrations and the Project watcher unchanged: both existing hardening passes and every daemon caller automatically receive the centralized behavior, so polling can no longer surface this auxiliary-file race as an open or reconciliation failure.
4. Verify with cargo nextest run -p state --test state_foundation --locked, the relevant daemon watcher tests via cargo nextest run -p daemon -E 'test(watcher)' --locked, cargo fmt --all --check, and cargo clippy --workspace --all-targets --all-features --locked -- -D warnings.

Task list (4/4 completed)

• Read issue details, repository guidance, and relevant learnings
• Trace SQLite startup, WAL cleanup, and permission hardening behavior
• Assess actionability and produce a verified implementation plan
• Publish the plan on issue Database permission hardening races with SQLite WAL removal #309

  ^{｜ Implement plan ➔ ｜ View workflow run ｜ via Pullfrog ｜ Using GPT Sol ｜ 𝕏}