Schema & robustness hardening: contents immutability triggers (v21) + #114 cluster#125
Merged
Conversation
The v13->v14 reshape dropped files_blake3_immutable without installing an equivalent guard on the new contents table. v21 re-asserts the append-only contract the AGENTS.md guarantee implies: BEFORE UPDATE and BEFORE DELETE triggers on contents that RAISE(ABORT). Defense in depth -- there is no UPDATE/DELETE path on contents today. Regenerated store/schema.sql via -update-schema; golden test green. Extended the minimal v18 fixture with the contents table a real v18 DB carries, so the chain to v21 can attach the triggers. Addresses #113.
Adds a populated v18 fixture (contents, files, remote_objects, destination_run_ids) and drives it through the v19-v21 chain, asserting the offload-substrate rows survive intact (verify_method NULL-backfilled, remote_objects fingerprint preserved). Asserts the new v21 contents triggers abort an in-place UPDATE and a DELETE while the row is unchanged. Addresses #113 (#12c migration-shape coverage).
Offload defers to every run kind, but index and sync did not block on an in-flight offload, so a sync could enumerate (or an index could observe-and-flip) a tree mid-unlink. Add 'offload' to both begin-gate blocking sets to make the exclusion symmetric. Tests both directions: offload blocks a new sync and a new index/audit, and admits them again once it finishes. Addresses #114 (run-gate asymmetry).
hook_runs was the only runs-like table without FinishRun's terminal-state guard: a double finish silently overwrote the first terminal record. Read the status inside the finishing transaction and refuse with ErrAlreadyFinished when it is already terminal, leaving the row untouched. Test a double-finish is refused and the recorded outcome survives. Addresses #114 (FinishHookRun terminal guard).
The shared requireIndexedVolume gate looked the volume up by name only. A stale volumes.path would let a handler enumerate the config path's tree while the durability advance covered the DB volume's rows — a wholesale false durability claim. Refuse on mismatch, the same cross-check offload and restore already make. Covers every push handler (bucket, kopia, content-addressed, peer) since they share the gate. Test the mismatch is refused before rclone is invoked. Addresses #114 (config-path vs DB-volume-path agreement).
ensureRepository created a fresh repository on any connect failure. On a transient outage or a mistyped path that mints an empty repository while the destination's monotonic durability vector keeps claiming coverage the new repository cannot honour — and there is no CLI to rewind the vector. Require opt-in (the existing --init flag, which already gates the local marker bootstrap): without it, a connect failure is an error, not a silent create. Test connect-fail without --init refuses and never creates. Addresses #114 (kopia repo re-create guard).
rcloneVerification set verified purely from flags + exit-0. When a backend shares no hash with the source, rclone emits a NOTICE and silently falls back to a size comparison; parseJSONLog dropped NOTICE-level events, so the run was still recorded as blake3-verified. Detect the stable 'no hashes in common' phrase at any log level, flag it on RunResult, and downgrade the verification to size+mtime (unverified) so the durability vector does not advance on a copy rclone never content-checked. The guard only ever downgrades, so a false match cannot wrongly mark a run verified. Addresses #114 (--checksum blake3 silent degrade).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Final hardening PR of the adversarial-audit follow-up. Schema defense-in-depth (#113) plus the robustness cluster (#114). Every change is additive or a refusal; nothing relaxes an existing guarantee.
Addresses #113
Addresses #114
Schema change (v21)
New migration
migrateV20ToV21(additive; no existing migration edited) installs two triggers oncontents, restoring the schema-level immutability the AGENTS.md append-only contract implies and that the v13→v14 reshape dropped along withfiles_blake3_immutable:contents_no_update—BEFORE UPDATE ON contents→RAISE(ABORT, …)contents_no_delete—BEFORE DELETE ON contents→RAISE(ABORT, …)store/schema.sqlregenerated viago test ./store -update-schema;TestSchemaSnapshotgreen. Defense-in-depth: the auditor confirmed there is no UPDATE/DELETE path oncontentstoday.#113 — index/migration integrity
TestMigrateV18ChainToV21drives a populated v18 fixture (contents, files, remote_objects, destination_run_ids) through v19→v21, asserts the offload-substrate rows survive (verify_method NULL-backfilled, remote_objects fingerprint intact), and asserts the contents triggers ABORT an UPDATE and a DELETE.Skipped with note — contents/files blake3↔size consistency check (
db check): post-v14,contents.blake3is UNIQUE andfilesreferencecontent_id(not blake3/size), so a blake3→size disagreement is unrepresentable by construction; a runtime check would have nothing to find. The v21 immutability trigger plus the merged #107 stat-after-hash fix already prevent new occurrences. Did not force a degenerate check in; happy to file a follow-up if a non-degenerate framing is wanted.#114 — robustness cluster
All five items in scope landed (each a focused change with a test):
'offload'to the blocking sets ofBeginSyncRunIfClearandBeginIndexRunIfClearso index/sync refuse while an offload is in flight (offload already blocks on every kind). Tested both directions.FinishRun's first-terminal-write-wins guard (read status in the finishing tx, refuse withErrAlreadyFinishedwhen already terminal). Double-finish test asserts the recorded outcome survives.no hashes in commonfallback notice inparseJSONLogat any log level (NOTICE-level events were dropped before), flag it onRunResult, and downgradercloneVerificationto size+mtime/unverified so the durability vector does not advance on a copy rclone never content-checked. The guard only ever downgrades, so a false match can never wrongly mark a run verified (the chosen err-toward-refusing option for this vector-gating item). Detection keys on the stable substringno hashes in common(the trailing verb has varied across rclone versions).v.Path != vol.Pathcross-check (the same one offload and restore already make) to the sharedrequireIndexedVolumegate, covering every push handler (bucket, kopia, content-addressed, peer). Mismatch refused before rclone is invoked.ensureRepositorynow creates on connect-fail only when--initis set (the same flag that already gates the local-destination marker bootstrap). Without it, a connect failure is a hard error rather than minting a fresh empty repository while the monotonic, un-rewindable durability vector keeps claiming coverage. Test asserts connect-fail without--initrefuses and never creates;--inithelp text and the kopia/integration tests updated accordingly.Out of scope (note): the issue body's sixth bullet — durability-evidence staleness policy — was not in this PR's task list and is untouched here; it can stay open on #114 or move to its own follow-up.
Verification
go vet ./...cleango test ./...clean (full module)golangci-lint run— 0 issuesTestSchemaSnapshotgreen after regeneratingstore/schema.sqlJudgment calls
destination_run_ids_test.gowith thecontentstable a real v18 DB carries, since the chain now attaches triggers to it.ErrAlreadyFinishedsentinel for the hook guard (it is the runs-family terminal-write sentinel; callers alreadyerrors.Isit).