sneak/vaultik
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

The documented PUT-only source host fails every backup after its first: the destination marker check needs HEAD permission on S3 #389

New Issue
Closed
opened 2026-10-14 09:51:59 +02:00 by clawbot · 2 comments
clawbot commented 2026-10-14 09:51:59 +02:00
Collaborator
Copy Link
Copy Source

Severity: medium

What is wrong: docs/REPOSTRUCTURE.md (Accepted Risks, item 5: "The backed-up host needs only PUT to run snapshot create: it writes blobs and metadata and neither reads nor deletes them") and the README's opening ("no private keys, secrets, or credentials (other than those required to PUT ...)") promise that PUT permission on the destination is enough to back up. Since #239, every backup first looks for vaultik-destination.txt with Stat, an S3 HeadObject, and treats any error other than not-found as fatal. With credentials that allow only s3:PutObject, S3 answers HeadObject with 403, because HEAD on an object needs s3:GetObject (without s3:ListBucket the answer is 403 even for a missing object). The first backup still works, because the marker check is skipped while the local index holds no snapshot; every backup after it fails with checking for vaultik-destination.txt: ... 403. The per-blob Stat in uploadBlobIfNeeded is not affected: it treats an error as "not stored" and uploads.

Where: internal/vaultik/destination_marker.go checkDestinationMarker (the err != nil branch after the ErrNotFound case); internal/storage/s3.go Stat (only NotFound and NoSuchKey map to ErrNotFound); docs/REPOSTRUCTURE.md Accepted Risks item 5; README.md first paragraph.

How it was shown: By reading the code path above together with S3's documented permission rule for HeadObject. Not run against an S3 server with a PUT-only policy; confidence high.

Definition of done

  • Either the marker check treats a permission error as "could not check" and goes on with a warning (the check exists to catch an unplugged volume's empty mount point, which a file:// destination can always Stat), so a PUT-only source host keeps working; or the two documents stop promising PUT-only and name the permissions snapshot create needs on S3 (PutObject plus GetObject, or ListBucket, for the HEAD).
  • A test covers a Stat of the marker that fails with an error other than not-found.

Model: fable-5-1

**Severity:** medium **What is wrong:** `docs/REPOSTRUCTURE.md` (Accepted Risks, item 5: "The backed-up host needs only PUT to run `snapshot create`: it writes blobs and metadata and neither reads nor deletes them") and the README's opening ("no private keys, secrets, or credentials (other than those required to PUT ...)") promise that PUT permission on the destination is enough to back up. Since https://git.eeqj.de/sneak/vaultik/issues/239, every backup first looks for `vaultik-destination.txt` with `Stat`, an S3 `HeadObject`, and treats any error other than not-found as fatal. With credentials that allow only `s3:PutObject`, S3 answers `HeadObject` with 403, because HEAD on an object needs `s3:GetObject` (without `s3:ListBucket` the answer is 403 even for a missing object). The first backup still works, because the marker check is skipped while the local index holds no snapshot; every backup after it fails with `checking for vaultik-destination.txt: ... 403`. The per-blob `Stat` in `uploadBlobIfNeeded` is not affected: it treats an error as "not stored" and uploads. **Where:** `internal/vaultik/destination_marker.go` `checkDestinationMarker` (the `err != nil` branch after the `ErrNotFound` case); `internal/storage/s3.go` `Stat` (only `NotFound` and `NoSuchKey` map to `ErrNotFound`); `docs/REPOSTRUCTURE.md` Accepted Risks item 5; `README.md` first paragraph. **How it was shown:** By reading the code path above together with S3's documented permission rule for HeadObject. Not run against an S3 server with a PUT-only policy; confidence high. **Definition of done** - Either the marker check treats a permission error as "could not check" and goes on with a warning (the check exists to catch an unplugged volume's empty mount point, which a `file://` destination can always `Stat`), so a PUT-only source host keeps working; or the two documents stop promising PUT-only and name the permissions `snapshot create` needs on S3 (`PutObject` plus `GetObject`, or `ListBucket`, for the HEAD). - A test covers a `Stat` of the marker that fails with an error other than not-found. Model: fable-5-1
clawbot commented 2026-10-14 22:56:57 +02:00
Author
Collaborator
Copy Link
Copy Source

Plan (first option of the definition of done; the documents keep their PUT-only promise):

  • checkDestinationMarker treats a Stat error other than not-found as "could not check": it logs the error at info level and the backup goes on and writes the marker as before. The check exists for a file:// volume's empty mount point, which always answers not-found, so nothing it guards is lost; a destination that cannot be written fails on the next PUT anyway.
  • Info level rather than a warning (judgement call, departing from "with a warning" in the definition of done): a PUT-only host is a documented setup, and --cron must stay silent on its successful runs instead of mailing a warning every night.
  • Test: with the local index holding a snapshot, a marker Stat that fails with an error other than not-found lets the backup finish.

Model: opus-5-5

Plan (first option of the definition of done; the documents keep their PUT-only promise): - `checkDestinationMarker` treats a `Stat` error other than not-found as "could not check": it logs the error at info level and the backup goes on and writes the marker as before. The check exists for a `file://` volume's empty mount point, which always answers not-found, so nothing it guards is lost; a destination that cannot be written fails on the next PUT anyway. - Info level rather than a warning (judgement call, departing from "with a warning" in the definition of done): a PUT-only host is a documented setup, and `--cron` must stay silent on its successful runs instead of mailing a warning every night. - Test: with the local index holding a snapshot, a marker `Stat` that fails with an error other than not-found lets the backup finish. Model: opus-5-5
clawbot commented 2026-10-14 23:14:52 +02:00
Author
Collaborator
Copy Link
Copy Source

Built as planned in #392: a marker look-up that fails with anything other than not found is logged at info level and the backup goes on and writes the marker; not found still stops it. The fault-injecting test store gains a Stat hook for the test. The README and docs/REPOSTRUCTURE.md keep their PUT-only promise unchanged.

Model: opus-5-5

Built as planned in https://git.eeqj.de/sneak/vaultik/pulls/392: a marker look-up that fails with anything other than not found is logged at info level and the backup goes on and writes the marker; not found still stops it. The fault-injecting test store gains a `Stat` hook for the test. The README and `docs/REPOSTRUCTURE.md` keep their PUT-only promise unchanged. Model: opus-5-5
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
critical
Extremely important: data loss, a security hole, production down, or basic use impossible
 merge-ready
All checks pass, reviewed, rebased, ready for merge
 needs-checks
make check does not pass cleanly
 needs-rebase
PR has merge conflicts or is behind main
 needs-review
Code review not yet done
 needs-rework
Code review found issues to fix
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
clawbot sneak (Jeffrey Paul)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/vaultik#389