Skip to content

Prove journal and evidence recovery with a consistent backup format and restore drill #431

Open

Description

@valentinyanakiev

Part of #330. Architecture review against main at 8d7da844 (2026-09-26). Related: #361 (contract lifecycle), #366 (security/support claims), #271 (outcome evidence).

Problem

Verified export/import and Git keep refs provide useful building blocks, but no complete backup/restore procedure or repeatable drill was found. A database copy alone does not establish that the pinned manifests, cited artifacts and Git objects needed to inspect the run can be recovered. Append-only hashes also do not authenticate a backup's producer.

The store uses SQLite WAL and verifies exported runs. Define an operational recovery contract around those existing mechanisms, preserving their import collision and integrity rules.

Work

  1. Define the recovery unit and a versioned inventory: consistent journal snapshot, pinned manifests, retained referenced artifacts/Git objects, versions, sizes and checksums. Declare exclusions and unavailable references explicitly. Credentials, raw transcripts and unrelated working trees must not be copied implicitly; document how operators separately recover required credentials or opt into sensitive evidence.
  2. Use a supported consistent SQLite snapshot mechanism while writers run. Record the snapshot boundary; an interrupted backup must not be mistaken for a complete one. Verify integrity before accepting a restore.
  3. Restore into a new location without overwriting a live journal. Reconstruct inspect/export/verify behavior and required evidence links. Runs that were in flight at snapshot remain subject to the existing indeterminate recovery rule; restore must not silently start agents.
  4. Define configurable retention and a dry-run inventory of what would be removed. Document supported upgrade/downgrade behavior and compatibility across the declared contract window, coordinating Contract versions have no lifecycle: an accepted decision to mint one, a ruled window to retire one #361.
  5. Add an offline restore drill that removes access to the original source and proves the restored copy is independently usable. Measure duration, snapshot size, recovery point and missing evidence rather than claiming an unmeasured RTO/RPO.

Acceptance

  • A backup taken during appends restores a coherent journal with a declared boundary and verifying chains.
  • Interrupted, truncated, altered and incomplete backups fail with specific diagnostics and leave the live destination untouched.
  • The drill restores completed, parked and in-flight examples; state/manifest identity matches the snapshot and no external effect runs automatically.
  • Referenced artifacts and Git objects in the declared retention scope remain accessible after the source directory is unavailable.
  • Secrets and transcript exclusions are tested; reports distinguish integrity verification from authenticated provenance.
  • Restore and supported migration procedures are documented and tested on supported hosts.

Choose the smallest command/script surface that makes the drill repeatable; a new storage service is not required. Any new contract semantics follow the repository's decision process.

Activity

  1. valentinyanakiev commented on Oct 30, 2026

    @valentinyanakiev
    ContributorAuthor

    Operator ruling, 2026-10-30, in chat, verbatim: "go with recommended on all the rulings".

    It takes the one pending choice on this issue as recommended.

    431s2, the fix round (recommended) for 431s1's security hold: H1 restore refuses an occupied destination, its SQLite sidecars included; H2 an event no run owns is counted, so backup and restore cannot carry it through unseen; C1 the refusal tests match variants, not text; G1 one schema vocabulary and reader. Decision 0100 also says the backup verifies but does not authenticate. Queued on story-431-journal-backup at 2df1942b9.

  2. valentinyanakiev commented on Oct 30, 2026

    @valentinyanakiev
    ContributorAuthor

    Fix round 431s2 stopped at REVIEW-SECURITY-HOLD, high, 2026-10-30. Run fix-round-431s2-for-issue-431-sl-7128218e, branch story-431-journal-backup at 406f518a1 (5e2db36f2, 99dffc1f3, 2df1942b9, 406f518a1 on bc36e93). Its H1, H2, C1 and G1 fixes are present and verified:

    • early symlink_metadata checks cover all four destination names;
    • ownership and whole-table counts are shared by backup and restore;
    • the projection refusal reuses summary_disagreements;
    • the refusal tests destructure variants;
    • one schema reader.

    backup.rs is at its 800-line ceiling. The chief found a new defect in the same verification class, written as a test case:

    1. H3, HIGH, security: an event row's SQL identity and stored hash are never checked against its envelope. backup.rs:483-493 derives the verified boundary only from Store::load's envelopes, and read.rs:95-108 never compares a row's seq/event_hash with the parsed envelope. So verify_snapshot (backup.rs:422-430) accepts rows that head_hash, load_after and append_next later use.
      • Case A: import conclude-already-done-hand-built.ndjson (one run, eight events). Drop events_append_only_update, change only the SQL head's event_hash to bad, and restore the trigger. Then back up, and restore into an absent destination.
        • Observed: integrity is ok, and both backup and restore succeed. head_hash then returns (8, 'bad'), load_after refuses UnknownHead { seq: 8 }, and append_next writes 9 with previous_hash = 'bad'. A later load refuses BrokenChain.
      • Case B: the same, but the SQL seq is moved to seq+100 with every envelope untouched. Both operations succeed, head_hash reports 108, and append_next writes 109 before a load refuses SeqGap.
      • Expected: the inconsistent rows are refused before publication, and the destination stays absent.
      • Fix: check every event row's identity and stored hash against its envelope. Checking only the last row would not close the class.
    2. L1, LOW: backup.rs:529's try_into().unwrap_or(0) reports a malformed position of -1 as seq: 0. Expected: the actual key, or a typed invalid-position refusal.
    3. Run integrity: panel prose argued "no security finding"; the chief rejected it.

    The PR's asks so far: (a) Git objects and refs/forge anchors, (b) artifacts outside the journal, (c) retention with a dry-run inventory, (d) the measured offline restore drill, (e) upgrade and downgrade across the contract window (#361), then signing and anchoring. New: (f) backup refuses a source with unowned events, (g) 0100's per-table account ("verifies, does not authenticate"), and (h) the public API: BackupError::UnownedEvents and ProjectionDisagreement.

    Fix round 431s3 is drafted to fix findings 1 and 2. A security hold waits for the operator's word.

  3. valentinyanakiev commented on Oct 30, 2026

    @valentinyanakiev
    ContributorAuthor

    Operator ruling, 2026-10-30, in chat, verbatim: "go with recommended".

    It takes the pending choice on this issue as recommended.

    431s3, the fix round (recommended) for 431s2's security hold (#431 (comment)): backup and restore check every event row's SQL identity and stored hash against its own envelope (the append's hash function, reused) before publication, refusing with a typed error; a malformed position is reported as itself, never 0. Queued on story-431-journal-backup at 406f518a1.

  4. valentinyanakiev commented on Oct 31, 2026

    @valentinyanakiev
    ContributorAuthor

    Landing 431L parked, 2026-10-31: OpenAI's content filter refused its review chief twice. Run landing-pr-for-issue-431-slice-1-45f8d5da, branch story-431-journal-backup at 51da2cb99. Verify passed (VERIFY-PASS) and the review panel finished. The chief, gpt-6-astra through codex, then exited 1 twice. Its codex rollout records cyber_policy: "This content was flagged for possible cybersecurity risk". The same filter refused 908d's adversarial seat.

  5. valentinyanakiev commented on Oct 31, 2026

    @valentinyanakiev
    ContributorAuthor

    Landing 431L2 stopped at REVIEW-SECURITY-HOLD, high, 2026-10-31. The cause is the controller's own coverage commit. Run landing-pr-for-issue-431-slice-1-2bb17cdf, branch story-431-journal-backup at 02c2857ac on c3e52bd, with a Fable 5.1 chief. The chief reproduced the finding through the built binary.

    H1, HIGH (decision 0100 ruling 4): a restore no longer checks the size of the bytes it copied.

    • The controller's coverage commit (f8c3f5cf3, 0808e8360 before the restack) removed verify_snapshot's post-copy size check as unreachable. The argument was that the pre-copy check of the source's metadata length refuses the same mismatch.
    • That is false: a source's metadata length is not the length of the stream copied from it. restore_through (backup.rs:361-369) now checks only the source's metadata.
    • Case: import fixtures/journals/conclude-already-done-hand-built.ndjson, run brokkr backup, then set inventory.snapshot.bytes to 0.
      • Control: with the real journal.db (176,128 bytes), restore refuses with exit 1, "the backup's snapshot is 176128 bytes; its inventory says 0", and writes no destination.
      • The finding: replace journal.db with a FIFO (metadata length 0) streaming the same bytes. brokkr restore exits 0 and publishes a 176,128-byte journal while the inventory says 0.
    • The digest still binds the published bytes to the inventory's SHA-256, so the journal published is the one the digest names. But an inventory that contradicts itself is accepted, and a refusal decision 0100 promises is weakened.

    INFO, prose only: the 0100 index cell omits the row/envelope check, the unowned-events refusal and the projection comparison, and says "renaming last" where the code hard-links and refuses an existing name. backup.rs:373, :431-432 and :682 also say "rename" for that hard link.

    Recommended fix:

    • Check the copied length (std::io::copy's count) against inventory.snapshot.bytes before the digest, and refuse with the existing SizeMismatch.
    • Hold it with a FIFO test on Linux and macOS that fails when the check is removed.
    • Correct verify_snapshot's doc and the coverage commit's removal list in the fixing commit's message, and fold in the two prose notes.
    • Then a fresh landing.

    A security hold waits for the operator's word.

  6. valentinyanakiev commented on Oct 31, 2026

    @valentinyanakiev
    ContributorAuthor

    Operator ruling, 2026-10-31, in chat, verbatim: "1. fix it".

    For landing 431L2's security hold (#431 (comment)), option 1: the controller restores the check on the copied length against inventory.snapshot.bytes before the digest, holds it with a FIFO test on Linux and macOS, corrects verify_snapshot's doc and the coverage commit's removal list, folds in the two prose notes, and then runs a fresh landing.

  7. valentinyanakiev commented on Nov 1, 2026

    @valentinyanakiev
    ContributorAuthor

    Operator ruling, 2026-11-01, in chat: "go with recommended on #943"

    PR #943's asks, as recommended:

    431L3's INFO F1–F3 go to one test-and-doc follow-up after the merge:

    • F1: BackupOptions::to's doc;
    • F2: a test that binds refuse_existing's symlink clause;
    • F3: the FIFO test's writer cannot hang the suite.
  8. valentinyanakiev commented on Nov 1, 2026

    @valentinyanakiev
    ContributorAuthor

    431L3's INFO F1–F3 are filed as #945, to land after #943 merges.

  9. valentinyanakiev commented on Nov 1, 2026

    @valentinyanakiev
    ContributorAuthor

    Operator ruling, 2026-11-01, in chat: "go with recommended on all the rulings"

    431L3's host lint failed with run_with: CC 37.0 over its baseline 35.0 (crates/brokkr-cli/src/lib.rs:1461). The ruling raises run_with's complexity baseline from 35 to 37, under the by-hand label. The controller commits the baseline, reruns the lints and the anchor, and opens #431's PR as a DRAFT with asks (a)–(h).

  10. valentinyanakiev commented on Nov 1, 2026

    @valentinyanakiev
    ContributorAuthor

    PR #943's macOS leg fails one of this branch's own tests (run 41289946389, test (macos-latest)):

    • backup::tests::a_path_sqlite_cannot_name_is_refused_by_name (crates/brokkr-store/src/backup/tests.rs:712) expects BackupError::UnnameablePath, but gets Io(Os { code: 92, message: "Illegal byte sequence" }).

    Cause.

    • backup_through creates the destination directory (prepare_directory) before anything asks whether SQLite can name the path. That question is asked only later, in vacuum_into.
    • On Linux the directory …/\xff is created, and vacuum_into then refuses UnnameablePath.
    • On macOS, APFS itself refuses to create a non-UTF-8 name with EILSEQ, so the refusal surfaces as a bare I/O error.

    Both hosts fail closed, but only Linux refuses by name, and Linux also leaves the empty directory behind.

    The fix, drafted in the worktree and uncommitted (+7/−1, crates/brokkr-store/src/backup.rs):

    • backup_through refuses UnnameablePath(to) before prepare_directory, so nothing is created and both hosts refuse by name.
    • vacuum_into's own check becomes an expect: a nameable directory joined with an ASCII file name is always nameable, so the old arm would be unreachable under exact coverage.
    • backup.rs grows 793 → 799 lines (ceiling 800).
    • Linux: cargo test -p brokkr-store --lib -- backup passes, 50 tests.
    • Removal control: dropping the early check fails the test by name on Linux too (it panics at the expect, backup.rs:606). Restored.

    This is a production change after landing 431L3. As with 0a6c426 and #908's c3cec85, it needs the operator's word: a fresh landing (recommended), or the existing by-hand label. Auto-merge on #943 is off until then.

  11. valentinyanakiev commented on Nov 1, 2026

    @valentinyanakiev
    ContributorAuthor

    Operator ruling, 2026-11-01, in chat: "and follow your recommendation"

    The macOS defect from PR #943 is fixed by the controller's commit d2aa5e8: backup_through refuses UnnameablePath before it creates the destination, so both hosts refuse by name. A fresh landing, 431L4, re-judges the whole branch, as with 0a6c426. #943's auto-merge stays off until that landing and the macOS leg pass.

  12. valentinyanakiev commented on Nov 1, 2026

    @valentinyanakiev
    ContributorAuthor

    Landing 431L4 stopped at REVIEW-SECURITY-HOLD, high (landing-pr-for-issue-431-slice-1-f49b8b06, Fable 5.1 chief, HEAD d2aa5e8).

    H1, HIGH, security (decision 0100 ruling 3; 0071 rulings 3 and 9): a file: destination escapes the checked directory and can write over a file.

    • The cause: the bundled SQLite is built with -DSQLITE_USE_URI (libsqlite3-sys 0.30.1, build.rs:137), so every open in this binary reads a filename beginning file: as a URI, whatever the connection flags. vacuum_into (backup.rs about :605-612) then writes to the URI's path, not to the directory backup_through checked and created.
    • Reproduced by the chief through the binary built from HEAD:
      • (a) An empty file is overwritten. : > victim; backup --to 'file:victim?unused=1' exits 1 with io: No such file or directory. victim is then a 176,128-byte SQLite database, and the literal directory file:victim?unused=1 is left behind, empty.
      • (b) A one-byte file is written over the same way.
      • (c) A full journal survives, refused with SQLite's own "output file already exists", its sha256 unchanged. An occupied name is protected only by SQLite's check, never by the backup's.
      • (d) A stray escapes the directory. mkdir escaped; backup --to file:escaped exits 1 and leaves a 176,128-byte .brokkr-backup-<uuid>-snapshot.db inside escaped/, outside the directory that was checked, created and cleaned.
      • (e) Restore fails closed: restore --to 'file:rest/journal.db' is refused, with the destination absent and the literal parent left empty.
    • The chief's shape for the fix: bind each path to one literal identity at the edge, before any SQLite or filesystem call, in both verbs. std::path::absolute does it (MSRV 1.88 has it): an absolute path never begins with file:, and an empty destination becomes the cwd, which prepare_directory then refuses. Or refuse the file: prefix by name.
    • Binding tests:
      • a relative file:<name>?x=1 destination beside an empty file <name> leaves <name> at 0 bytes and publishes nothing outside to;
      • after --to file:escaped, an existing escaped/ holds no stray.

    Below H1:

    • L1: vacuum_into's expect comment names no test; it should name a_path_sqlite_cannot_name_is_refused_by_name.
    • L2: that test asserts only the variant and the path. Moving the check back after prepare_directory stays green on Linux. It should also assert that the destination is absent.
    • L3: BackupError::UnrecordedSchema { key: &'static str } is a two-value string vocabulary; an enum binds it.
    • I1–I4: informational.

    New ask, for a follow-up issue, not this PR: --db and every other path the CLI hands SQLite on main share the same file: URI exposure, through the bundled SQLITE_USE_URI. One literal-path rule for all of them is the operator's call.

    PR #943 stays held: no auto-merge until H1 is fixed and a landing passes. The recommended fix is in the controller's next message to the operator, in chat.

  13. valentinyanakiev commented on Nov 1, 2026

    @valentinyanakiev
    ContributorAuthor

    Operator ruling, 2026-11-01, in chat: "go with recommended on all the rulings"

    On landing 431L4's security hold, option 1, as recommended:

  14. 3 remaining items

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions