A blob upload to an rclone destination cut off by a hard kill leaves a partial object that the next backup trusts #266

Open
opened 2026-10-07 18:00:19 +02:00 by clawbot · 2 comments
Collaborator

internal/storage/rclone.go:82-99 streams a blob with operations.Rcat straight to its final key. Rclone's Rcat hands the reader to the backend under the final name. Only operations.Copy writes to a temporary name and renames it into place, and vaultik does not use Copy. The local and sftp backends open the final path and remove it only when their own write fails. A SIGKILL, an OOM kill or a power loss mid-upload therefore leaves a truncated object under the blob's real key.

uploadBlobIfNeeded (internal/snapshot/scanner.go:1554-1565) skips the upload whenever Stat succeeds and never compares the stored size with the blob's compressed size.

Trigger: storage_url: rclone://REMOTE/path with a local or sftp remote, kill the process during a blob upload, then run snapshot create again. The retry re-packs the same chunks into a blob with the same hash. It finds the truncated object with Stat, skips the upload, records the blob as uploaded and completes the snapshot. Restoring those files then fails the hash check, although the backup reported success. manifest.json.zst is written the same way, so a kill during the metadata export can leave a manifest that prune can never read.

Acceptable: the guarantee file:// got in #130, that an object appears under its key only once it is complete. In addition, an existing object is trusted only when its size matches.

Found by the second-pass audit on next at e161343, by code trace.

Definition of done

  1. uploadBlobIfNeeded uploads again when the stored object's size differs from the blob's recorded compressed size. A test plants a short object at the blob key and asserts it is replaced.
  2. The rclone backend writes each object under a temporary name and moves it into place where the remote supports a server-side move. The README's storage backends section says which backends write atomically.
  3. make check passes.

Model: fable-5-1 (audit); opus-5-5 (issue)

`internal/storage/rclone.go:82-99` streams a blob with `operations.Rcat` straight to its final key. Rclone's `Rcat` hands the reader to the backend under the final name. Only `operations.Copy` writes to a temporary name and renames it into place, and vaultik does not use `Copy`. The `local` and `sftp` backends open the final path and remove it only when their own write fails. A SIGKILL, an OOM kill or a power loss mid-upload therefore leaves a truncated object under the blob's real key. `uploadBlobIfNeeded` (`internal/snapshot/scanner.go:1554-1565`) skips the upload whenever `Stat` succeeds and never compares the stored size with the blob's compressed size. Trigger: `storage_url: rclone://REMOTE/path` with a local or sftp remote, kill the process during a blob upload, then run `snapshot create` again. The retry re-packs the same chunks into a blob with the same hash. It finds the truncated object with `Stat`, skips the upload, records the blob as uploaded and completes the snapshot. Restoring those files then fails the hash check, although the backup reported success. `manifest.json.zst` is written the same way, so a kill during the metadata export can leave a manifest that `prune` can never read. Acceptable: the guarantee `file://` got in https://git.eeqj.de/sneak/vaultik/issues/130, that an object appears under its key only once it is complete. In addition, an existing object is trusted only when its size matches. Found by the second-pass audit on `next` at `e161343`, by code trace. ## Definition of done 1. `uploadBlobIfNeeded` uploads again when the stored object's size differs from the blob's recorded compressed size. A test plants a short object at the blob key and asserts it is replaced. 2. The rclone backend writes each object under a temporary name and moves it into place where the remote supports a server-side move. The README's storage backends section says which backends write atomically. 3. `make check` passes. Model: fable-5-1 (audit); opus-5-5 (issue)
clawbot self-assigned this 2026-10-07 18:00:19 +02:00
Author
Collaborator

Fixed in #274: the rclone backend writes under a temporary .partial name and moves the object into place on remotes that can show a partial file, and a backup uploads a blob again when the stored object has a different size. The defect reproduced on next at d87202f before the fix.

Model: opus-5-5

Fixed in https://git.eeqj.de/sneak/vaultik/pulls/274: the rclone backend writes under a temporary `.partial` name and moves the object into place on remotes that can show a partial file, and a backup uploads a blob again when the stored object has a different size. The defect reproduced on `next` at `d87202f` before the fix. Model: opus-5-5
Author
Collaborator

Amending the definition of done after the review of #274. The repo's own docs decide this.

  • Item 1 is dropped (upload again when the stored size differs). docs/REPOSTRUCTURE.md says a blob is never modified once written, and that a blob with a given name stays encrypted to the recipients in force when it was first written. A blob's name depends only on its uncompressed content. A size check that overwrites a complete blob packed with a different compression_level or a different set of recipients breaks both rules, and can leave earlier snapshots unrestorable by the keys they were written for. An existing object at a blob key is trusted, as before.
  • Item 2 is widened. Write each object under a temporary name and move it into place wherever the remote supports a server-side move, whether or not rclone marks the remote as showing partial uploads (hdfs, for example). Remotes with no server-side move are written in place, and the README's storage backends section says so.

Model: opus-5-5

Amending the definition of done after the review of https://git.eeqj.de/sneak/vaultik/pulls/274. The repo's own docs decide this. - **Item 1 is dropped** (upload again when the stored size differs). `docs/REPOSTRUCTURE.md` says a blob is never modified once written, and that a blob with a given name stays encrypted to the recipients in force when it was first written. A blob's name depends only on its uncompressed content. A size check that overwrites a complete blob packed with a different `compression_level` or a different set of recipients breaks both rules, and can leave earlier snapshots unrestorable by the keys they were written for. An existing object at a blob key is trusted, as before. - **Item 2 is widened.** Write each object under a temporary name and move it into place wherever the remote supports a server-side move, whether or not rclone marks the remote as showing partial uploads (hdfs, for example). Remotes with no server-side move are written in place, and the README's storage backends section says so. Model: opus-5-5
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/vaultik#266