fix(sync): content-hash change detection via a name-encoded marker (#102)#107
Merged
Conversation
…arker (#102) SmartPull decided "did the remote change?" purely from (ModTime, Size). Because the vault is AES-GCM (ciphertext length = plaintext length + fixed overhead), a same-length remote edit (rotate a password to an equal-length one, flip a fixed-width field) keeps Size byte-identical, leaving ModTime as the only signal — and some rclone backends preserve ModTime poorly. A false "unchanged" on a write path skips conflict detection and lets the blind SmartPush overwrite a real remote change: silent cross-device data loss. Fix: make pass-cli's own sha256 the authoritative remote-change signal, read for zero extra round-trips. On push, write a zero-byte marker named `<vault>.<sha256>.synchash` into the vault dir; rclone sync carries it to the remote. SmartPull reads the remote content hash straight from the marker name in the single `rclone lsjson` listing it already fetches: - marker present → remote changed iff markerHash != LastPushHash (content- authoritative; ignores modtime noise, so no false conflicts). - marker absent → fall back to the legacy (ModTime, Size) heuristic (old vaults / devices that pushed before markers existed). SmartPush writes exactly one marker and removes any stale one, so the dir (and, after sync, the remote) holds a single current marker. Known limitation (documented in code): an interrupted remote push that uploaded vault.enc but not the marker leaves a stale marker; the next successful push self-heals it. Accepted in exchange for zero-round-trip, false-conflict-free detection. Tests: marker parse (present / legacy / backup-lookalike / ambiguous); the #102 regression (identical Size+ModTime but differing marker → detected as changed); marker authoritative skip on modtime noise; marker conflict; push writes/cleans the marker. Closes #102. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019sxsM218vNzDbuMZ2nhMzx
) Note that a zero-byte vault.enc.<sha256>.synchash object now appears in the remote bucket, how it powers zero-round-trip content-based change detection, and that it is safe to leave alone. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019sxsM218vNzDbuMZ2nhMzx
CI golangci-lint v2.5 flagged the negated conjunction in the marker-absent fallback. Rewrite !(A && B) as !A || !B; behavior identical. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019sxsM218vNzDbuMZ2nhMzx
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.
Problem
SmartPulldecided "did the remote change?" purely from (ModTime, Size) (internal/sync/sync.go). Because the vault is AES-GCM (ciphertext length = plaintext length + fixed overhead), a same-length remote edit — rotating a password to an equal-length one, flipping a fixed-width field, editing a same-width timestamp — keepsSizebyte-identical, leavingModTimeas the only signal. Some rclone backends preserve ModTime poorly (coarse granularity, synthesized at upload).That same comparison gates conflict detection, so a false "unchanged" on a write path (
add/update/delete) skips conflict detection entirely → the blindSmartPushoverwrites a real remote change → silent cross-device data loss. Low probability, high severity — exactly what you don't want in a password manager.Fix — pass-cli's own sha256, for zero extra round-trips
rclone lsjson --hashreturns the backend's hash, not ours, so it can't be compared to theLastPushHashsha256 we track. Instead:vault.enc.<sha256>.synchashinto the vault dir;rclone synccarries it to the remote alongsidevault.enc.rclone lsjsonlistingSmartPullalready fetches — no extra network call (doesn't regress Reduce (and surface) pre-unlock sync latency: SmartPull does a silent network round trip on every command #96, doesn't fight perf(sync): overlap pre-unlock remote probe with unlock to hide latency (split from #96) #103).Detection becomes:
markerHash != LastPushHash(content-authoritative; ignores modtime noise, so no false conflicts).(ModTime, Size)heuristic (existing vaults / devices that pushed before markers existed).SmartPushwrites exactly one marker and removes any stale one, so the dir — and, after sync, the remote — holds a single current marker.Known limitation (documented in code)
An interrupted remote push that uploaded
vault.encbut not the marker leaves a stale marker that reads as "unchanged"; the next successful push self-heals it. Accepted in exchange for zero-round-trip, false-conflict-free detection. A user will see one*.synchashobject in their bucket — documented in the sync guide as expected and safe.Tests
parseRemoteMarkerHash: present / legacy-absent / backup-lookalike ignored / ambiguous-falls-back.Size+ModTimebut a differing marker → detected as changed (the false-negative the issue is about).SmartPushwrites & cleans the marker.All unit + sync integration tests green.
Closes #102.
🤖 Generated with Claude Code