report and trees open the database read-write and set journal_mode #8

Closed
opened 2026-08-09 03:44:07 +02:00 by clawbot · 2 comments
Collaborator

db.go:78-95 builds one DSN shared by openScanDatabase (db.go:99) and openReportDatabase (db.go:133), and that DSN includes _pragma=journal_mode(WAL). Setting journal_mode is a write.

README §Database describes the intended deployment as a daily scan cron job maintaining /var/lib/sfdupes/db.sqlite, with report and trees "run interactively whenever needed". In that deployment the database is owned by the cron user (typically root) and the interactive report is not. An unprivileged report therefore fails to open a database it only needs to read. The same failure hits a read-only mount, a ZFS snapshot copy, or a database on read-only media — all natural things to want to run a report against, given the whole design promises analysis never touches the scanned filesystem.

Definition of done

  1. openReportDatabase uses a read-intent DSN: mode=ro plus _pragma=query_only(1), retaining the busy timeout so a report during a concurrent scan still behaves. It must not set journal_mode.
  2. openScanDatabase keeps the read-write DSN unchanged, including WAL.
  3. A test chmods a populated database to 0444 and asserts both report and trees succeed and produce correct output.
  4. README §Database states that report and trees require only read access to the database file.
  5. make check green.
`db.go:78-95` builds one DSN shared by `openScanDatabase` (`db.go:99`) and `openReportDatabase` (`db.go:133`), and that DSN includes `_pragma=journal_mode(WAL)`. Setting `journal_mode` is a write. README §Database describes the intended deployment as a daily `scan` cron job maintaining `/var/lib/sfdupes/db.sqlite`, with `report` and `trees` "run interactively whenever needed". In that deployment the database is owned by the cron user (typically root) and the interactive report is not. An unprivileged `report` therefore fails to open a database it only needs to read. The same failure hits a read-only mount, a ZFS snapshot copy, or a database on read-only media — all natural things to want to run a report against, given the whole design promises analysis never touches the scanned filesystem. ## Definition of done 1. `openReportDatabase` uses a read-intent DSN: `mode=ro` plus `_pragma=query_only(1)`, retaining the busy timeout so a report during a concurrent scan still behaves. It must not set `journal_mode`. 2. `openScanDatabase` keeps the read-write DSN unchanged, including WAL. 3. A test chmods a populated database to `0444` and asserts both `report` and `trees` succeed and produce correct output. 4. README §Database states that `report` and `trees` require only read access to the database file. 5. `make check` green.
clawbot added this to the 1.0.0 milestone 2026-08-09 03:44:07 +02:00
Author
Collaborator

Plan (implementer's brief).

  • openReportDatabase gets its own read-only connection string as the definition of done says: mode=ro, query_only(1), the busy timeout, and no journal_mode. openScanDatabase is unchanged.
  • Trap, and the real test: the database is in WAL mode. A read-only connection to a WAL database needs its -shm file to exist or be creatable. In the documented deployment the cron scan runs as root and owns /var/lib/sfdupes, so after it exits there is no -shm and an unprivileged report cannot create one there. A test that only makes the database file 0444 leaves the directory writable and passes for the wrong reason. The test must make both the file and its directory read-only, with no -wal or -shm present, and run both report and trees (the test suite runs as a non-root user in the Docker build, so permission bits apply; skip the test when running as root, and say so).
  • If that fails, the fix is yours to choose and to disclose in the PR, for example scan switching the database back to rollback-journal mode as its last act before closing, so a reader needs nothing but the file. A report during a running scan must keep working (README.md §Database).
  • README.md §Database: report and trees need only read access to the database file (state exactly what the tree needs, nothing more).

Model: opus-5-5

Plan (implementer's brief). - `openReportDatabase` gets its own read-only connection string as the definition of done says: `mode=ro`, `query_only(1)`, the busy timeout, and no `journal_mode`. `openScanDatabase` is unchanged. - Trap, and the real test: the database is in WAL mode. A read-only connection to a WAL database needs its `-shm` file to exist or be creatable. In the documented deployment the cron `scan` runs as root and owns `/var/lib/sfdupes`, so after it exits there is no `-shm` and an unprivileged `report` cannot create one there. A test that only makes the database file `0444` leaves the directory writable and passes for the wrong reason. The test must make both the file and its directory read-only, with no `-wal` or `-shm` present, and run both `report` and `trees` (the test suite runs as a non-root user in the Docker build, so permission bits apply; skip the test when running as root, and say so). - If that fails, the fix is yours to choose and to disclose in the PR, for example `scan` switching the database back to rollback-journal mode as its last act before closing, so a reader needs nothing but the file. A `report` during a running `scan` must keep working (`README.md` §Database). - `README.md` §Database: `report` and `trees` need only read access to the database file (state exactly what the tree needs, nothing more). Model: opus-5-5
clawbot self-assigned this 2026-10-03 14:09:46 +02:00
Author
Collaborator

Built in #72: report and trees open the database read-only, and scan takes it out of WAL mode when it closes it, so reading needs only read access to the database file. The read-only connection alone was not enough (the WAL trap from the plan); the PR body covers the cases where a report and a scan overlap.

Model: opus-5-5

Built in https://git.eeqj.de/sneak/sfdupes/pulls/72: `report` and `trees` open the database read-only, and `scan` takes it out of WAL mode when it closes it, so reading needs only read access to the database file. The read-only connection alone was not enough (the WAL trap from the plan); the PR body covers the cases where a report and a scan overlap. Model: opus-5-5
Sign in to join this conversation.