What was wrong: the file:// backend's List and ListStream walked the destination with afero.Walk, which uses Lstat and so reports a symlinked directory as one file. Put, Get and Stat follow the link, so with metadata/ or blobs/ replaced by a symlink, backups and restores kept working while the store listed as empty; prune then removed the local snapshot records and deleted every blob. remote nuke also left .partial files behind such a link.
What changed: both listings and DeletePartialUploads, which remote nuke uses, go through a small walk helper that hands afero.Walk the filesystem wrapped in a struct carrying only the afero.Fs methods, so afero falls back to Stat and follows the link. Each directory from the destination directory down to the prefix is checked first. A prefix lists as empty only when it is truly missing; when it or a directory above it is a symlink whose target is missing, such as a link to an unmounted volume, the listing fails, as a missing destination directory does, and snapshot remove warns. Tests cover a symlinked metadata/ and blobs/, with the target present and missing, in the storage listings, DeletePartialUploads, prune and snapshot remove, plus a symlink loop.
Judgement call: no loop detection. A link back up the tree is followed until the system refuses the path, and the listing fails.
The blobs/ case of the prune test with the target present also passes without the fix; the storage test is the one that fails without it.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/vaultik/issues/296.
**What was wrong:** the `file://` backend's `List` and `ListStream` walked the destination with `afero.Walk`, which uses Lstat and so reports a symlinked directory as one file. `Put`, `Get` and `Stat` follow the link, so with `metadata/` or `blobs/` replaced by a symlink, backups and restores kept working while the store listed as empty; `prune` then removed the local snapshot records and deleted every blob. `remote nuke` also left `.partial` files behind such a link.
**What changed:** both listings and `DeletePartialUploads`, which `remote nuke` uses, go through a small `walk` helper that hands `afero.Walk` the filesystem wrapped in a struct carrying only the `afero.Fs` methods, so afero falls back to Stat and follows the link. Each directory from the destination directory down to the prefix is checked first. A prefix lists as empty only when it is truly missing; when it or a directory above it is a symlink whose target is missing, such as a link to an unmounted volume, the listing fails, as a missing destination directory does, and `snapshot remove` warns. Tests cover a symlinked `metadata/` and `blobs/`, with the target present and missing, in the storage listings, `DeletePartialUploads`, `prune` and `snapshot remove`, plus a symlink loop.
- Judgement call: no loop detection. A link back up the tree is followed until the system refuses the path, and the listing fails.
- The `blobs/` case of the `prune` test with the target present also passes without the fix; the storage test is the one that fails without it.
Model: opus-5-5
internal/storage/file.go, List and ListStream: a metadata/ or blobs/ that is a symlink whose target is missing, such as a link to a volume that is not mounted, still lists as empty with no error. With metadata/ linked that way, prune --force removes the local snapshot record, deletes every blob and exits 0. That is the loss #296 is about, and its definition of done rules it out ("it never reports a store it could not read as empty"). Listing the whole destination already fails on the same link, so the two listings also disagree. Acceptable: when the listed prefix is a symlink whose target is missing, List and ListStream fail, as a missing destination directory does since #220. A test shows that prune then deletes no blob and keeps the local snapshot record, and the PR body drops the judgement call that says such a link lists as empty.
The branch no longer rebases onto current next: TODO.md conflicts with the Completed Steps entry for #298. Acceptable: rebased onto next with both entries kept.
Not verified: the full test and lint run, since the branch does not rebase onto current next.
Model: opus-5-5
- `internal/storage/file.go`, `List` and `ListStream`: a `metadata/` or `blobs/` that is a symlink whose target is missing, such as a link to a volume that is not mounted, still lists as empty with no error. With `metadata/` linked that way, `prune --force` removes the local snapshot record, deletes every blob and exits 0. That is the loss https://git.eeqj.de/sneak/vaultik/issues/296 is about, and its definition of done rules it out ("it never reports a store it could not read as empty"). Listing the whole destination already fails on the same link, so the two listings also disagree. Acceptable: when the listed prefix is a symlink whose target is missing, `List` and `ListStream` fail, as a missing destination directory does since https://git.eeqj.de/sneak/vaultik/issues/220. A test shows that `prune` then deletes no blob and keeps the local snapshot record, and the PR body drops the judgement call that says such a link lists as empty.
- The branch no longer rebases onto current `next`: `TODO.md` conflicts with the Completed Steps entry for https://git.eeqj.de/sneak/vaultik/issues/298. Acceptable: rebased onto `next` with both entries kept.
Not verified: the full test and lint run, since the branch does not rebase onto current `next`.
Model: opus-5-5
Symlink with a missing target: the prefix check in List and ListStream now uses Lstat, so such a link fails the listing and prune deletes nothing; new storage and prune tests; the judgement call is gone from the PR body.
TODO.md: rebased onto next with both Completed Steps entries kept, this one on top.
The authoritative make check is left to the reviewer's gate.
Model: opus-5-5
Rework:
- Symlink with a missing target: the prefix check in `List` and `ListStream` now uses Lstat, so such a link fails the listing and `prune` deletes nothing; new storage and `prune` tests; the judgement call is gone from the PR body.
- `TODO.md`: rebased onto `next` with both Completed Steps entries kept, this one on top.
The authoritative `make check` is left to the reviewer's gate.
Model: opus-5-5
internal/storage/file.go, List and ListStream: a prefix that lies under a symlink whose target is missing still lists as empty, with no error. With metadata/ linked to a volume that is not mounted, listing one snapshot's directory under metadata/ returns nothing. snapshot remove for a snapshot in the local index then removes the local record, gives none of the warning the README promises when the destination store is unreachable, and leaves the snapshot's metadata on the volume. The TODO.md entry says such a link makes the listings fail, which is false for this case. Acceptable: a prefix lists as empty only when it is truly absent. When metadata/, blobs/ or any other directory between the destination directory and the prefix is a symlink whose target is missing, the listing fails. A test shows that snapshot remove then warns, as it does for a missing destination directory (#220).
internal/storage/file.go, DeletePartialUploads: it still walks with afero.Walk, so it does not enter a symlinked metadata/ or blobs/. remote nuke then deletes the blobs through the link but leaves .partial files behind it, and still prints "Backup destination store is now empty." That breaks the README's promise that remote nuke leaves the bucket prefix empty, and it is the same message #296 reports with content left behind. Acceptable: it walks through the link as the listings now do, a test covers it, and the PR body drops its "Left alone" line.
Model: opus-5-5
- `internal/storage/file.go`, `List` and `ListStream`: a prefix that lies under a symlink whose target is missing still lists as empty, with no error. With `metadata/` linked to a volume that is not mounted, listing one snapshot's directory under `metadata/` returns nothing. `snapshot remove` for a snapshot in the local index then removes the local record, gives none of the warning the README promises when the destination store is unreachable, and leaves the snapshot's metadata on the volume. The `TODO.md` entry says such a link makes the listings fail, which is false for this case. Acceptable: a prefix lists as empty only when it is truly absent. When `metadata/`, `blobs/` or any other directory between the destination directory and the prefix is a symlink whose target is missing, the listing fails. A test shows that `snapshot remove` then warns, as it does for a missing destination directory (https://git.eeqj.de/sneak/vaultik/issues/220).
- `internal/storage/file.go`, `DeletePartialUploads`: it still walks with `afero.Walk`, so it does not enter a symlinked `metadata/` or `blobs/`. `remote nuke` then deletes the blobs through the link but leaves `.partial` files behind it, and still prints "Backup destination store is now empty." That breaks the README's promise that `remote nuke` leaves the bucket prefix empty, and it is the same message https://git.eeqj.de/sneak/vaultik/issues/296 reports with content left behind. Acceptable: it walks through the link as the listings now do, a test covers it, and the PR body drops its "Left alone" line.
Model: opus-5-5
List and ListStream walked the destination with afero.Walk, which uses
Lstat and so reported a symlinked metadata/ or blobs/ directory as one
file, while Put, Get and Stat follow the link. The store then listed as
empty, and prune removed the local snapshot records and deleted every
blob. The listings, and DeletePartialUploads, which remote nuke uses,
now walk a view of the filesystem without its Lstat method, so afero
falls back to Stat and enters the link. A prefix lists as empty only
when it is truly missing: when it or a directory above it is a symlink
whose target is missing, such as a link to an unmounted volume, the
listing fails.
Judgement call: a symlink loop is not detected; it is followed until the
system refuses the path, and the listing fails.
Model: opus-5-5
Symlink with a missing target above the prefix: each directory from the destination directory down to the prefix is now checked, so such a link anywhere on the way fails the listing and snapshot remove warns; new storage and snapshot remove tests.
DeletePartialUploads now walks through a symlinked directory as the listings do; new storage test; the "Left alone" line is gone from the PR body.
TODO.md change dropped, per #334; rebased onto next.
Model: opus-5-5
Rework:
- Symlink with a missing target above the prefix: each directory from the destination directory down to the prefix is now checked, so such a link anywhere on the way fails the listing and `snapshot remove` warns; new storage and `snapshot remove` tests.
- `DeletePartialUploads` now walks through a symlinked directory as the listings do; new storage test; the "Left alone" line is gone from the PR body.
- `TODO.md` change dropped, per https://git.eeqj.de/sneak/vaultik/issues/334; rebased onto `next`.
Model: opus-5-5
The commit message: its body is 134 words, over the limit of about 120. Acceptable: the same single commit with the body trimmed to about 120 words, for example by leaving out detail the PR body already gives. No code change is needed.
Model: opus-5-5
- The commit message: its body is 134 words, over the limit of about 120. Acceptable: the same single commit with the body trimmed to about 120 words, for example by leaving out detail the PR body already gives. No code change is needed.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes #296.
What was wrong: the
file://backend'sListandListStreamwalked the destination withafero.Walk, which uses Lstat and so reports a symlinked directory as one file.Put,GetandStatfollow the link, so withmetadata/orblobs/replaced by a symlink, backups and restores kept working while the store listed as empty;prunethen removed the local snapshot records and deleted every blob.remote nukealso left.partialfiles behind such a link.What changed: both listings and
DeletePartialUploads, whichremote nukeuses, go through a smallwalkhelper that handsafero.Walkthe filesystem wrapped in a struct carrying only theafero.Fsmethods, so afero falls back to Stat and follows the link. Each directory from the destination directory down to the prefix is checked first. A prefix lists as empty only when it is truly missing; when it or a directory above it is a symlink whose target is missing, such as a link to an unmounted volume, the listing fails, as a missing destination directory does, andsnapshot removewarns. Tests cover a symlinkedmetadata/andblobs/, with the target present and missing, in the storage listings,DeletePartialUploads,pruneandsnapshot remove, plus a symlink loop.blobs/case of theprunetest with the target present also passes without the fix; the storage test is the one that fails without it.Model: opus-5-5
internal/storage/file.go,ListandListStream: ametadata/orblobs/that is a symlink whose target is missing, such as a link to a volume that is not mounted, still lists as empty with no error. Withmetadata/linked that way,prune --forceremoves the local snapshot record, deletes every blob and exits 0. That is the loss #296 is about, and its definition of done rules it out ("it never reports a store it could not read as empty"). Listing the whole destination already fails on the same link, so the two listings also disagree. Acceptable: when the listed prefix is a symlink whose target is missing,ListandListStreamfail, as a missing destination directory does since #220. A test shows thatprunethen deletes no blob and keeps the local snapshot record, and the PR body drops the judgement call that says such a link lists as empty.next:TODO.mdconflicts with the Completed Steps entry for #298. Acceptable: rebased ontonextwith both entries kept.Not verified: the full test and lint run, since the branch does not rebase onto current
next.Model: opus-5-5
f172b42d85to716fb0453d716fb0453dtoad7941c215ad7941c215tob894f6e5e9Rework:
ListandListStreamnow uses Lstat, so such a link fails the listing andprunedeletes nothing; new storage andprunetests; the judgement call is gone from the PR body.TODO.md: rebased ontonextwith both Completed Steps entries kept, this one on top.The authoritative
make checkis left to the reviewer's gate.Model: opus-5-5
internal/storage/file.go,ListandListStream: a prefix that lies under a symlink whose target is missing still lists as empty, with no error. Withmetadata/linked to a volume that is not mounted, listing one snapshot's directory undermetadata/returns nothing.snapshot removefor a snapshot in the local index then removes the local record, gives none of the warning the README promises when the destination store is unreachable, and leaves the snapshot's metadata on the volume. TheTODO.mdentry says such a link makes the listings fail, which is false for this case. Acceptable: a prefix lists as empty only when it is truly absent. Whenmetadata/,blobs/or any other directory between the destination directory and the prefix is a symlink whose target is missing, the listing fails. A test shows thatsnapshot removethen warns, as it does for a missing destination directory (#220).internal/storage/file.go,DeletePartialUploads: it still walks withafero.Walk, so it does not enter a symlinkedmetadata/orblobs/.remote nukethen deletes the blobs through the link but leaves.partialfiles behind it, and still prints "Backup destination store is now empty." That breaks the README's promise thatremote nukeleaves the bucket prefix empty, and it is the same message #296 reports with content left behind. Acceptable: it walks through the link as the listings now do, a test covers it, and the PR body drops its "Left alone" line.Model: opus-5-5
b894f6e5e9to188b48ef39188b48ef39tof32f1cd33af32f1cd33ato3fc273e47fRework:
snapshot removewarns; new storage andsnapshot removetests.DeletePartialUploadsnow walks through a symlinked directory as the listings do; new storage test; the "Left alone" line is gone from the PR body.TODO.mdchange dropped, per #334; rebased ontonext.Model: opus-5-5
Model: opus-5-5