volmark, index, sync: .squirrel-volume markers (#64)#71
Merged
Conversation
Closes #64. sync's dest.Root and restore's vol.Path are taken verbatim into rclone argv. A typo silently points sync/restore at an unrelated tree — rclone treats those files as part of the comparison set and may overwrite or shuffle them under .squirrel-history/ on the next sync. The marker file gives every squirrel-owned directory a self-identifying fingerprint so operations can refuse before damage is done. New package volmark: - MarkerName const .squirrel-volume. - Marker struct (Volume + forensic Node/Version/CreatedAt fields). - Read / Write / Validate API with ErrMissing sentinel and *ErrMismatch typed error so callers can distinguish "uninitialised tree" from "wrong volume's tree" without string-matching. - Atomic Write via tempfile + rename + fsync. index: - ensureVolumeMarker writes the marker on first non-dry-run, named Index against a volume root, and refuses on mismatch. Dry-run and legacy unnamed callers skip the dance. - visit() filters .squirrel-volume at the volume root so the marker doesn't get indexed as a regular file (would churn its mtime on every rewrite and propagate to destinations). sync: - New Options.Init flag — without it, a missing destination marker is refused with the --init hint. With it, the per-volume directory + marker are created. Mismatched markers are always refused, Init or not. - The marker gate is local-destination-only for now; remote destinations need rclone-mediated read/write and are tracked as a follow-up. - buildRcloneArgs adds a --filter to keep .squirrel-volume out of rclone's comparison/transfer on both sync and restore. restore: - validateLocalVolumeMarker refuses default (--to unset) restore when vol.Path has no marker or names a different volume. --to overrides the check since the operator is explicitly redirecting to a scratch directory. CLI: - `squirrel sync --init` plumbs through to Options.Init. Tests: - volmark: full unit coverage of Read/Write/Validate + atomicity. - index: TestIndexWritesVolumeMarker (first-write + idempotent second run + walker-filter), TestIndexRefusesMismatchedVolumeMarker. - sync: TestSyncRefusesUninitialisedDestination, TestSyncInitWritesMarker, TestSyncRefusesMismatchedDestinationMarker, TestRestoreRefusesMissingLocalMarker, TestRestoreToPathSkipsMarkerCheck. - Existing sync fixtures pre-seed the destination marker so the ~10 baseline tests don't have to pass --init; the dedicated tests above exercise the init flow. - TestCLIRestoreRoundTrip re-seeds the source marker after wiping the volume — that mirrors the documented recovery flow ("write a marker or re-run index before restoring"). Out of scope (deferred follow-ups): - Remote destinations (S3/SFTP) — needs rclone cat/copyto plumbing. - Node-sync receiver markers — receivers manage their own markers via their own index runs. - `squirrel volumes init` for explicit local bootstrap on a wiped laptop; today the workflow is "re-run index" or hand-write the marker.
2 tasks
There was a problem hiding this comment.
Pull request overview
Introduces .squirrel-volume marker files as a safety gate to prevent sync/restore operations from running against misconfigured or unintended directory trees. This fits into squirrel’s broader “safety first” posture around destructive rclone operations by requiring an explicit, volume-identity fingerprint at volume roots.
Changes:
- Adds new
volmarkpackage to read/write/validate volume marker files, including sentinel/typed errors. - Integrates marker validation/bootstrapping into
index(write-on-first-index, refuse on mismatch) andsync/restore(refuse missing/mismatched markers;sync --initbootstraps local destinations). - Extends CLI and test fixtures to seed/validate markers and filters markers out of indexing and rclone comparisons.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| volmark/volmark.go | New marker implementation (read/write/validate + typed errors). |
| volmark/volmark_test.go | Unit tests for marker behavior and error modes. |
| sync/sync.go | Adds destination marker gate + restore-time local marker validation; filters marker out of rclone args; adds Options.Init. |
| sync/sync_test.go | Updates fixtures to seed markers and adds tests for refusal/init/mismatch behavior. |
| index/index.go | Writes marker on first named non-dry-run index, refuses mismatch, and skips marker during walk. |
| index/index_test.go | Tests marker creation/idempotence, walk skip, and mismatch refusal. |
| cmd/squirrel/sync.go | Plumbs --init through to sync.Options.Init. |
| cmd/squirrel/restore_test.go | Adjusts restore smoke test to re-seed marker after wiping volume dir. |
| cmd/squirrel/main_test.go | Seeds destination markers in CLI fixtures to avoid needing --init in unrelated tests. |
Comments suppressed due to low confidence (1)
sync/sync.go:364
- The destination marker mismatch branch always reports "refuse to init over…" even when Init is false, which can mislead users who didn’t pass --init. Consider rewording this mismatch error to be init-agnostic (e.g., "refusing to sync") and optionally only mention init in the missing-marker case.
if _, ok := errors.AsType[*volmark.ErrMismatch](err); ok {
return fmt.Errorf("destination %q: %w (refuse to init over a different volume's tree)", dest.Name, err)
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+378
to
+383
| if err := volmark.Write(root, volmark.Marker{ | ||
| Volume: volumeName, | ||
| Node: self.Name, | ||
| CreatedAt: time.Now().UTC().Format(time.RFC3339), | ||
| }); err != nil { | ||
| return fmt.Errorf("destination %q: %w", dest.Name, err) |
Comment on lines
+607
to
+613
| if errors.Is(err, volmark.ErrMissing) { | ||
| return volmark.Write(absRoot, volmark.Marker{ | ||
| Volume: volumeName, | ||
| Node: nodeName, | ||
| CreatedAt: time.Now().UTC().Format(time.RFC3339), | ||
| }) | ||
| } |
| return nil | ||
| } | ||
| if errors.Is(err, volmark.ErrMissing) { | ||
| return fmt.Errorf("volume %q at %s has no .squirrel-volume marker — refusing to restore (run `squirrel index %s` first or use `--to <scratch>` to restore elsewhere)", vol.Name, vol.Path, vol.Name) |
2 tasks
- volmark.Marker drops the Version field. It was always empty in real Write call sites (sync, index) — squirrel doesn't plumb a build-time version string today, and an always-empty field is misleading documentation. Documented in the type's godoc so a future version source can re-introduce it deliberately. - sync.go's marker-missing error messages now use volmark.MarkerName instead of the hardcoded ".squirrel-volume" literal, so renaming the marker file (unlikely but) would keep diagnostics in sync.
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.
Closes #64. Stacked on #70 (
safety/62-copyfromexisting-preserve) — review/merge that first.Summary
dest.Rootandvol.Pathare taken verbatim into rclone argv. A typo silently targets an unrelated tree — rclone treats those files as part of the comparison set and shuffles them under.squirrel-history/on the next sync, or restore overwrites a directory of unrelated personal files. The new marker file gives every squirrel-owned directory a self-identifying fingerprint so operations refuse before any damage.volmark/(new package)MarkerNameconst.squirrel-volume,Markerstruct (Volume + forensic Node/Version/CreatedAt).Read/Write/ValidateAPI withErrMissingsentinel and*ErrMismatchtyped error so callers distinguish "uninitialised tree" from "wrong volume's tree" without string-matching.indexIndex. Refuses on mismatch. Dry-run and legacy unnamed callers skip..squirrel-volumeat the volume root so the marker isn't indexed (would churn mtime on every rewrite and propagate to destinations).syncOptions.Init. Without it, a missing destination marker is refused with--inithint. With it, per-volume dir + marker are created. Mismatched markers always refused,Initor not.--filter "- /.squirrel-volume"added to both sync and restore rclone argv so the marker is never compared/transferred.restorevalidateLocalVolumeMarkerrefuses default (--tounset) restore whenvol.Pathhas no marker or names a different volume.--tobypasses the check (operator explicitly redirecting to scratch).CLI
squirrel sync --initplumbs through toOptions.Init.Tests
volmark: Read/Write/Validate, sentinel propagation, atomicity.index:TestIndexWritesVolumeMarker(first-write + idempotent second run + walker filter),TestIndexRefusesMismatchedVolumeMarker.sync:TestSyncRefusesUninitialisedDestination,TestSyncInitWritesMarker,TestSyncRefusesMismatchedDestinationMarker,TestRestoreRefusesMissingLocalMarker,TestRestoreToPathSkipsMarkerCheck.--init; the dedicated tests above exercise the init flow.TestCLIRestoreRoundTripre-seeds the source marker after wiping the volume — mirrors the documented recovery flow.Out of scope (deferred follow-ups)
squirrel volumes initfor wiped-laptop recovery; today the workflow is "re-run index or write the marker by hand."Test plan
go build ./...,go vet ./...,go test ./...pass locally.