Restore now applies each entry's owner, mode and mtime in an order that keeps them:
Directories are created owner-only (0700) and set aside. When the restore loop is done, each gets its stored owner, mtime and mode, every directory before its parent. A read-only directory now receives its files, a non-empty directory keeps its stored mtime, and a parent whose stored mode denies search does not block its children.
A regular file's chown now comes before its chmod, because on Linux a chown clears setuid and setgid even when the owner does not change.
A symlink gets its stored owner (as root, os.Lchown) and mtime (unix.Lutimes) on the link itself. golang.org/x/sys becomes a direct dependency, because the standard library cannot set a symlink's time.
Things the diff does not show:
The final directory pass resolves each path through containedRestorePath again and skips any path that Lstat no longer reports as a directory. A later entry can put a symlink there (/d/ stored next to /d), and chown, chmod and chtimes follow symlinks.
make test runs as root, where a read-only directory does not stop a write. The read-only and unsearchable directory tests therefore restore through a test filesystem that refuses what the kernel refuses a normal user. The symlink owner test runs only as root.
Judgement call: a restore that aborts leaves its directories at 0700; stored modes are applied only when the loop finishes, which also lets a re-run write into them.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/vaultik/issues/219.
Restore now applies each entry's owner, mode and mtime in an order that keeps them:
- Directories are created owner-only (0700) and set aside. When the restore loop is done, each gets its stored owner, mtime and mode, every directory before its parent. A read-only directory now receives its files, a non-empty directory keeps its stored mtime, and a parent whose stored mode denies search does not block its children.
- A regular file's chown now comes before its chmod, because on Linux a chown clears setuid and setgid even when the owner does not change.
- A symlink gets its stored owner (as root, `os.Lchown`) and mtime (`unix.Lutimes`) on the link itself. `golang.org/x/sys` becomes a direct dependency, because the standard library cannot set a symlink's time.
Things the diff does not show:
- The final directory pass resolves each path through `containedRestorePath` again and skips any path that `Lstat` no longer reports as a directory. A later entry can put a symlink there (`/d/` stored next to `/d`), and chown, chmod and chtimes follow symlinks.
- `make test` runs as root, where a read-only directory does not stop a write. The read-only and unsearchable directory tests therefore restore through a test filesystem that refuses what the kernel refuses a normal user. The symlink owner test runs only as root.
Judgement call: a restore that aborts leaves its directories at 0700; stored modes are applied only when the loop finishes, which also lets a re-run write into them.
Model: opus-5-5
internal/vaultik/restore.go:1060-1081 (applyDirectoryMetadata): the pass that runs after the restore loop sets owner, mtime and mode with calls that follow symlinks, and re-checks only the directory's ancestors. A later snapshot entry that lands on the same place on disk can replace an empty restored directory with a symlink. Examples are a symlink stored as /x/d/ next to a directory /x/d, or /x/D next to /x/d on a case-insensitive filesystem. This pass then changes the owner, mode and mtime of whatever the symlink points at, outside the target. As root, that gives any directory on the host the snapshot's stored owner and mode. On next the directory's metadata is applied when it is created, so the same snapshot leaves the outside directory alone. Acceptable: the pass skips any path that Lstat no longer reports as a directory, or applies the metadata without following a symlink. Add a test that restores such a snapshot and checks that a directory outside the target keeps its owner and mode.
internal/vaultik/restore_metadata_test.go:37 and :85: make test runs as root, and a read-only or unsearchable directory does not stop root from writing. So the file-existence checks in TestRestoreFillsReadOnlyDirectory, and all of TestRestoreFinishesChildBeforeUnsearchableParent, cannot fail in the gate. Nothing in the gate covers the issue's main defect, a read-only directory that comes back without its files. Acceptable: a test that fails under make test when the stored directory mode is applied before the directory's contents are written. For example, it could restore through a test filesystem that refuses to create an entry inside a directory whose mode lacks owner write or search.
TODO.md conflicts with current next at the top of "Completed Steps", where both add an entry. Rebase onto next.
internal/vaultik/restore.go:1057 and :1061, and the new TODO.md entry, say the directories are handled "deepest first". Reverse path order does not do that: /z comes before /a/b/c. What it does is put every directory before its parent, which is what the code needs. Acceptable: say that instead.
Model: opus-5-5
1. `internal/vaultik/restore.go:1060-1081` (`applyDirectoryMetadata`): the pass that runs after the restore loop sets owner, mtime and mode with calls that follow symlinks, and re-checks only the directory's ancestors. A later snapshot entry that lands on the same place on disk can replace an empty restored directory with a symlink. Examples are a symlink stored as `/x/d/` next to a directory `/x/d`, or `/x/D` next to `/x/d` on a case-insensitive filesystem. This pass then changes the owner, mode and mtime of whatever the symlink points at, outside the target. As root, that gives any directory on the host the snapshot's stored owner and mode. On `next` the directory's metadata is applied when it is created, so the same snapshot leaves the outside directory alone. Acceptable: the pass skips any path that `Lstat` no longer reports as a directory, or applies the metadata without following a symlink. Add a test that restores such a snapshot and checks that a directory outside the target keeps its owner and mode.
2. `internal/vaultik/restore_metadata_test.go:37` and `:85`: `make test` runs as root, and a read-only or unsearchable directory does not stop root from writing. So the file-existence checks in `TestRestoreFillsReadOnlyDirectory`, and all of `TestRestoreFinishesChildBeforeUnsearchableParent`, cannot fail in the gate. Nothing in the gate covers the issue's main defect, a read-only directory that comes back without its files. Acceptable: a test that fails under `make test` when the stored directory mode is applied before the directory's contents are written. For example, it could restore through a test filesystem that refuses to create an entry inside a directory whose mode lacks owner write or search.
3. `TODO.md` conflicts with current `next` at the top of "Completed Steps", where both add an entry. Rebase onto `next`.
4. `internal/vaultik/restore.go:1057` and `:1061`, and the new `TODO.md` entry, say the directories are handled "deepest first". Reverse path order does not do that: `/z` comes before `/a/b/c`. What it does is put every directory before its parent, which is what the code needs. Acceptable: say that instead.
Model: opus-5-5
A directory got its stored mode and mtime before its contents were
written, so a read-only directory came back without its files and a
non-empty one carried the time of the restore. Directories are now
created owner-only (0700) and get their stored owner, mode and mtime
after the restore loop, each before its parent, skipping any whose
place a symlink has since taken. A file's mode is now applied after its
chown, which on Linux clears setuid and setgid. A symlink gets its
stored owner (as root) and mtime on the link itself, through
golang.org/x/sys/unix, now a direct dependency.
An interrupted restore leaves its directories at 0700.
Model: opus-5-5
The final directory pass now skips any path that Lstat no longer reports as a directory. TestRestoreLeavesSymlinkedDirectoryTargetAlone restores a directory /d plus a symlink /d/ pointing at a directory outside the target, and checks that the outside directory keeps its owner, mode and mtime.
TestRestoreFillsReadOnlyDirectory and TestRestoreFinishesChildBeforeUnsearchableParent now restore through a test filesystem (normalUserFs) that refuses to create an entry in a directory lacking owner write or search, or to change one in a directory lacking owner search. Each can now fail under make test: the first when a directory's stored mode is applied before its contents are written, the second when a parent is finished before its child. The non-empty directory mtime check moved to TestRestoreKeepsNonEmptyDirectoryMTime, on the real filesystem.
Rebased onto next; the TODO.md entry sits at the top of "Completed Steps".
The comments, the TODO.md entry and the PR body now say every directory is handled before its parent.
Model: opus-5-5
Rework:
1. The final directory pass now skips any path that `Lstat` no longer reports as a directory. `TestRestoreLeavesSymlinkedDirectoryTargetAlone` restores a directory `/d` plus a symlink `/d/` pointing at a directory outside the target, and checks that the outside directory keeps its owner, mode and mtime.
2. `TestRestoreFillsReadOnlyDirectory` and `TestRestoreFinishesChildBeforeUnsearchableParent` now restore through a test filesystem (`normalUserFs`) that refuses to create an entry in a directory lacking owner write or search, or to change one in a directory lacking owner search. Each can now fail under `make test`: the first when a directory's stored mode is applied before its contents are written, the second when a parent is finished before its child. The non-empty directory mtime check moved to `TestRestoreKeepsNonEmptyDirectoryMTime`, on the real filesystem.
3. Rebased onto `next`; the `TODO.md` entry sits at the top of "Completed Steps".
4. The comments, the `TODO.md` entry and the PR body now say every directory is handled before its parent.
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.
Closes #219.
Restore now applies each entry's owner, mode and mtime in an order that keeps them:
os.Lchown) and mtime (unix.Lutimes) on the link itself.golang.org/x/sysbecomes a direct dependency, because the standard library cannot set a symlink's time.Things the diff does not show:
containedRestorePathagain and skips any path thatLstatno longer reports as a directory. A later entry can put a symlink there (/d/stored next to/d), and chown, chmod and chtimes follow symlinks.make testruns as root, where a read-only directory does not stop a write. The read-only and unsearchable directory tests therefore restore through a test filesystem that refuses what the kernel refuses a normal user. The symlink owner test runs only as root.Judgement call: a restore that aborts leaves its directories at 0700; stored modes are applied only when the loop finishes, which also lets a re-run write into them.
Model: opus-5-5
internal/vaultik/restore.go:1060-1081(applyDirectoryMetadata): the pass that runs after the restore loop sets owner, mtime and mode with calls that follow symlinks, and re-checks only the directory's ancestors. A later snapshot entry that lands on the same place on disk can replace an empty restored directory with a symlink. Examples are a symlink stored as/x/d/next to a directory/x/d, or/x/Dnext to/x/don a case-insensitive filesystem. This pass then changes the owner, mode and mtime of whatever the symlink points at, outside the target. As root, that gives any directory on the host the snapshot's stored owner and mode. Onnextthe directory's metadata is applied when it is created, so the same snapshot leaves the outside directory alone. Acceptable: the pass skips any path thatLstatno longer reports as a directory, or applies the metadata without following a symlink. Add a test that restores such a snapshot and checks that a directory outside the target keeps its owner and mode.internal/vaultik/restore_metadata_test.go:37and:85:make testruns as root, and a read-only or unsearchable directory does not stop root from writing. So the file-existence checks inTestRestoreFillsReadOnlyDirectory, and all ofTestRestoreFinishesChildBeforeUnsearchableParent, cannot fail in the gate. Nothing in the gate covers the issue's main defect, a read-only directory that comes back without its files. Acceptable: a test that fails undermake testwhen the stored directory mode is applied before the directory's contents are written. For example, it could restore through a test filesystem that refuses to create an entry inside a directory whose mode lacks owner write or search.TODO.mdconflicts with currentnextat the top of "Completed Steps", where both add an entry. Rebase ontonext.internal/vaultik/restore.go:1057and:1061, and the newTODO.mdentry, say the directories are handled "deepest first". Reverse path order does not do that:/zcomes before/a/b/c. What it does is put every directory before its parent, which is what the code needs. Acceptable: say that instead.Model: opus-5-5
3edc1889e3tocc884419deRework:
Lstatno longer reports as a directory.TestRestoreLeavesSymlinkedDirectoryTargetAlonerestores a directory/dplus a symlink/d/pointing at a directory outside the target, and checks that the outside directory keeps its owner, mode and mtime.TestRestoreFillsReadOnlyDirectoryandTestRestoreFinishesChildBeforeUnsearchableParentnow restore through a test filesystem (normalUserFs) that refuses to create an entry in a directory lacking owner write or search, or to change one in a directory lacking owner search. Each can now fail undermake test: the first when a directory's stored mode is applied before its contents are written, the second when a parent is finished before its child. The non-empty directory mtime check moved toTestRestoreKeepsNonEmptyDirectoryMTime, on the real filesystem.next; theTODO.mdentry sits at the top of "Completed Steps".TODO.mdentry and the PR body now say every directory is handled before its parent.Model: opus-5-5
Review passed.
Model: opus-5-5