Skip to content

open_read_only writes: audit verify and audit export migrate the database and fail on a read-only copy #267

Description

@vladimirrott

TransactionStore::open_read_only (crates/sysknife-daemon/src/transactions.rs:325) calls initialize(), and initialize() opens a write transaction, creates schema_migrations if absent, applies pending migrations, and sets journal_mode=WAL on the way in:

pub fn open_read_only(path: impl AsRef<Path>) -> Result<Self, TransactionStoreError> {
    ...
    store.initialize()?;
let tx = conn.transaction_with_behavior(TransactionBehavior::Immediate)?;
... "CREATE TABLE IF NOT EXISTS schema_migrations ("

So all three commands that claim to read the audit chain without touching it, sysknife audit verify, sysknife audit export and sysknife audit checkpoint, write to it.

Why it matters

Four consequences, in rising order of how much they hurt:

  1. -wal and -shm sidecars appear next to a database the operator only read.
  2. A newer CLI silently upgrades an older audit database's schema and inserts a schema_migrations row.
  3. An older CLI refuses outright: initialize() returns sqlite schema version N is newer than this binary supports, so a binary that could read all 17 columns fine will not read any.
  4. A read-only copy fails. PRAGMA journal_mode=WAL on a 0400 file or a read-only mount returns attempt to write a readonly database, so the offline-auditor workflow docs/the-audit-chain.md advertises for audit export cannot run on the artifact an auditor would actually be handed.

There is also a contention path: BEGIN IMMEDIATE takes a write lock with a 5s busy timeout, so sysknife audit export > rows.json in cron against a busy daemon intermittently exits 4 after the shell has already truncated the output file.

ensure_private_dir is not part of this problem: it creates a missing directory at 0700 and leaves an existing one alone.

Scope

Give the read paths a genuinely read-only open: OpenFlags::SQLITE_OPEN_READ_ONLY, no pragma writes, no initialize(), and a schema-version check by SELECT rather than by running migrations. Keep open_with_key as it is, since the daemon does need to migrate.

Tests first

The failing test is short: write a database with open_with_key, chmod 0400 it, then open it read-only and read the chain rows. That fails today with a write error, and no existing test covers a non-writable database.

Difficulty

medium. The open path is small, but the schema-version check has to move from migration to inspection.

Getting started

CONTRIBUTING.md has the build and test commands, and docs/developer-guide.md covers the setup steps and how to reproduce each required check locally. No CLA and no copyright waiver. The project is MIT.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinghelp wantedExtra attention is neededmediumDifficulty: needs familiarity with one subsystem

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions