2 Commits
Author SHA1 Message Date
sneak 4dc14895d4 Join the S3 prefix to every key with one slash (closes #222)
check / check (push) Successful in 10m54s
The S3 client built each key as prefix + key, and the URL parser keeps
the prefix as written, so s3://bucket/p stored p + "blobs/..." with no
slash while s3://bucket/p/ stored p/blobs/.... A recovery host that wrote
the URL the other way found no snapshots.

NewClient now strips trailing slashes from the prefix and adds one back
when anything is left, giving the README layout for both URL forms; an
empty prefix stays at the bucket root. The s3.prefix config setting
goes through the same client and gets the same join.

A new test writes through each URL shape against an in-process S3
server, checks the key in the bucket, and lists through both List and
ListStream, which every snapshot listing uses.

Model: opus-5-5
2026-10-06 16:44:01 +00:00
clawbot 5aa5ba5544 Apply restored owners, modes and times in an order that keeps them (closes #219)
check / check (push) Successful in 11m53s
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
2026-10-06 18:29:09 +02:00
5 changed files with 476 additions and 21 deletions
+11
View File
@@ -30,6 +30,17 @@ the tag exists and is exercised; what is left is merging `next` to
`<bucket>/<prefix>/blobs/...` layout. The `s3.prefix` config setting goes
through the same client and gets the same join.
- 2026-10-06: Made restore apply owners, modes and times in an order
that keeps them
([issue #219](https://git.eeqj.de/sneak/vaultik/issues/219)). 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 and get their stored owner, mode and mtime once the
restore loop is done, each before its parent. A file's mode is applied
after its chown, which on Linux clears setuid and setgid, and a
symlink gets its stored owner (as root) and mtime on the link itself.
- 2026-10-06: Made `go.mod` what `go mod tidy` writes, so the
pre-commit hook no longer stops every commit
([issue #246](https://git.eeqj.de/sneak/vaultik/issues/246)). A test
+1 -1
View File
@@ -24,6 +24,7 @@ require (
github.com/stretchr/testify v1.11.1
go.uber.org/fx v1.24.0
golang.org/x/sync v0.18.0
golang.org/x/sys v0.38.0
golang.org/x/term v0.37.0
gopkg.in/yaml.v3 v3.0.1
modernc.org/sqlite v1.38.0
@@ -263,7 +264,6 @@ require (
golang.org/x/exp v0.0.0-20251023183803-a4bb9ffd2546 // indirect
golang.org/x/net v0.47.0 // indirect
golang.org/x/oauth2 v0.33.0 // indirect
golang.org/x/sys v0.38.0 // indirect
golang.org/x/text v0.31.0 // indirect
golang.org/x/time v0.14.0 // indirect
golang.org/x/tools v0.38.0 // indirect
+29 -4
View File
@@ -88,13 +88,15 @@ func TestS3StorerMissingKeyMapsToErrNotFound(t *testing.T) {
// each key by one "/". s3://b/p and s3://b/p/ must be the same
// destination, or a host that writes the URL the other way finds no
// snapshots. The listed object is put straight into the bucket, as
// another host would have written it.
// another host would have written it. Both List and ListStream are
// checked: ListStream is what every snapshot listing goes through.
func TestS3URLPrefixKeyLayout(t *testing.T) {
t.Parallel()
const (
blobKey = "blobs/aa/bb/aabbccdd"
manifestKey = "metadata/snap/manifest.json.zst"
listPrefix = "metadata/"
manifestKey = listPrefix + "snap/manifest.json.zst"
manifestBody = "manifest"
)
@@ -152,14 +154,37 @@ func TestS3URLPrefixKeyLayout(t *testing.T) {
t.Fatalf("seed manifest: %v", err)
}
keys, err := storer.List(ctx, "metadata/")
keys, err := storer.List(ctx, listPrefix)
if err != nil {
t.Fatalf("List: %v", err)
}
if !slices.Equal(keys, []string{manifestKey}) {
t.Errorf("List(metadata/) = %q, want [%q]", keys, manifestKey)
t.Errorf("List(%q) = %q, want [%q]", listPrefix, keys, manifestKey)
}
streamed := listStreamKeys(t, storer, listPrefix)
if !slices.Equal(streamed, []string{manifestKey}) {
t.Errorf("ListStream(%q) = %q, want [%q]", listPrefix, streamed, manifestKey)
}
})
}
}
// listStreamKeys returns the keys ListStream yields under a prefix, and
// fails the test on a listing error.
func listStreamKeys(t *testing.T, s storage.Storer, prefix string) []string {
t.Helper()
var keys []string
for obj := range s.ListStream(context.Background(), prefix) {
if obj.Err != nil {
t.Fatalf("ListStream %q: %v", prefix, obj.Err)
}
keys = append(keys, obj.Key)
}
return keys
}
+86 -16
View File
@@ -10,11 +10,13 @@ import (
"math"
"os"
"path/filepath"
"slices"
"strings"
"time"
"filippo.io/age"
"github.com/spf13/afero"
"golang.org/x/sys/unix"
"sneak.berlin/go/vaultik/internal/blobgen"
"sneak.berlin/go/vaultik/internal/database"
"sneak.berlin/go/vaultik/internal/log"
@@ -63,6 +65,12 @@ const snapshotDBFilename = "snapshot.db"
// directories themselves get their stored mode).
const restoreDirMode = 0o755
// restoreDirCreateMode is the owner-only mode a directory from the
// snapshot is created with during restore, so its contents can be written
// whatever its stored mode. The stored mode is applied after the restore
// loop, by applyDirectoryMetadata.
const restoreDirCreateMode = 0o700
// restoreFileMode is the restrictive mode a regular file is created with
// during restore. Content is written while the file holds this mode; the
// stored mode is applied only after the file is fully written and closed,
@@ -332,6 +340,8 @@ func (v *Vaultik) restoreAllFiles(
return nil, err
}
session.applyDirectoryMetadata()
return result, nil
}
@@ -901,6 +911,9 @@ type restoreSession struct {
// the call entirely as non-root and emit one warning at the end
// of the restore explaining that ownership was not preserved.
runningAsRoot bool
// directories holds every restored directory, for
// applyDirectoryMetadata to finish after the restore loop.
directories []*database.File
}
// containedRestorePath resolves rel — a path read from the snapshot
@@ -1007,6 +1020,8 @@ func (s *restoreSession) restoreSymlink(file *database.File, targetPath string)
if err != nil {
return fmt.Errorf("creating symlink: %w", err)
}
s.applySymlinkMetadata(file, targetPath)
} else {
log.Debug("Symlink creation not supported on this filesystem",
"path", file.Path, "target", file.LinkTarget)
@@ -1019,35 +1034,90 @@ func (s *restoreSession) restoreSymlink(file *database.File, targetPath string)
return nil
}
// restoreDirectory restores a directory with its permissions, mtime,
// and (on real filesystems, with sufficient privileges) ownership.
// restoreDirectory creates a directory with restoreDirCreateMode. Its
// stored mode, owner and mtime are applied after the restore loop by
// applyDirectoryMetadata: a read-only stored mode would block writing
// its contents, and writing them changes its mtime.
func (s *restoreSession) restoreDirectory(
file *database.File, targetPath string,
) error {
err := s.v.Fs.MkdirAll(targetPath, os.FileMode(file.Mode))
err := s.v.Fs.MkdirAll(targetPath, restoreDirCreateMode)
if err != nil {
return fmt.Errorf("creating directory: %w", err)
}
// MkdirAll applies the process umask, so chmod to the exact stored
// mode. A failure here is non-fatal.
err = s.v.Fs.Chmod(targetPath, os.FileMode(file.Mode))
if err != nil {
log.Debug("Failed to set permissions", "path", targetPath, "error", err)
}
s.applyFileMetadata(file, targetPath)
s.directories = append(s.directories, file)
s.result.FilesRestored++
return nil
}
// applyDirectoryMetadata applies the stored owner, mtime and mode to every
// restored directory, each before its parent, so a parent whose stored
// mode denies search does not block its children. Failures are logged at
// debug level and do not abort the restore.
func (s *restoreSession) applyDirectoryMetadata() {
// A path sorts after its parent's, so reverse order puts every
// directory before its parent.
slices.SortFunc(s.directories, func(a, b *database.File) int {
return strings.Compare(b.Path.String(), a.Path.String())
})
for _, dir := range s.directories {
targetPath, err := containedRestorePath(
s.v.Fs, s.opts.TargetDir, dir.Path.String())
if err != nil {
log.Debug("Failed to set directory metadata",
"path", dir.Path, "error", err)
continue
}
// A later entry can have put a symlink in the directory's place,
// for example one stored as "/d/" next to the directory "/d". The
// calls below follow symlinks, so they would change its target.
info, err := lstatIfPossible(s.v.Fs, targetPath)
if err != nil || !info.IsDir() {
log.Debug("Not setting directory metadata: no longer a directory",
"path", targetPath, "error", err)
continue
}
s.applyFileMetadata(dir, targetPath)
err = s.v.Fs.Chmod(targetPath, os.FileMode(dir.Mode))
if err != nil {
log.Debug("Failed to set permissions", "path", targetPath, "error", err)
}
}
}
// applySymlinkMetadata applies ownership (when running as root) and mtime
// to a restored symlink itself; os.Chown and Chtimes would follow it.
// Failures are logged at debug level and do not abort the restore.
func (s *restoreSession) applySymlinkMetadata(file *database.File, targetPath string) {
if s.runningAsRoot {
err := os.Lchown(targetPath, int(file.UID), int(file.GID))
if err != nil {
log.Debug("Failed to set ownership", "path", targetPath, "error", err)
}
}
mtime := unix.NsecToTimeval(file.MTime.UnixNano())
err := unix.Lutimes(targetPath, []unix.Timeval{mtime, mtime})
if err != nil {
log.Debug("Failed to set mtime", "path", targetPath, "error", err)
}
}
// applyFileMetadata applies ownership (when running as root on a real
// filesystem) and mtime to a restored path. Permission mode is applied
// separately by each caller, with different failure handling, so it is
// not touched here. Failures are logged at debug level and do not abort
// the restore.
// filesystem) and mtime to a restored path. The caller applies the mode
// afterwards: on Linux a chown clears the setuid and setgid bits of a
// regular file. Failures are logged at debug level and do not abort the
// restore.
func (s *restoreSession) applyFileMetadata(file *database.File, targetPath string) {
if s.runningAsRoot {
if _, ok := s.v.Fs.(*afero.OsFs); ok {
@@ -1138,8 +1208,8 @@ func (s *restoreSession) restoreRegularFile(
return fmt.Errorf("closing output file: %w", err)
}
s.applyRestoredFileMode(file, targetPath)
s.applyFileMetadata(file, targetPath)
s.applyRestoredFileMode(file, targetPath)
s.result.FilesRestored++
s.result.BytesRestored += bytesWritten
+349
View File
@@ -0,0 +1,349 @@
package vaultik //nolint:testpackage // drives unexported restore internals
import (
"context"
"os"
"path/filepath"
"syscall"
"testing"
"time"
"github.com/spf13/afero"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/database"
"sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/types"
)
// These tests check that restore applies each entry's owner, mode and
// mtime in an order that keeps them.
const (
readOnlyDirMode = uint32(os.ModeDir | 0o555)
unsearchableMode = uint32(os.ModeDir | 0o600)
plainDirMode = uint32(os.ModeDir | 0o755)
plainFileMode = uint32(0o644)
setuidFileMode = uint32(os.ModeSetuid | 0o755)
otherOwnerID = uint32(4321)
writableTestMode = 0o755
symlinkTargetPath = "/nonexistent/target"
memTargetDir = "/restore"
// Owner bits a normal user needs on a directory to create an entry
// in it, and to change an entry in it.
ownerWriteAndSearch = os.FileMode(0o300)
ownerSearch = os.FileMode(0o100)
)
// normalUserFs refuses what the kernel refuses a normal user. make test
// runs as root, which a read-only or unsearchable directory does not
// stop, so without it the tests below could not fail there. Creating an
// entry needs owner write and search on the directory holding it;
// changing an entry's mode or times needs owner search. Only that one
// directory is checked, not every ancestor.
type normalUserFs struct {
afero.Fs
}
//nolint:ireturn // afero.Fs.OpenFile is defined to return the interface
func (fs normalUserFs) OpenFile(
name string, flag int, perm os.FileMode,
) (afero.File, error) {
if flag&os.O_CREATE != 0 {
err := fs.checkParent(name, ownerWriteAndSearch)
if err != nil {
return nil, err
}
}
return fs.Fs.OpenFile(name, flag, perm)
}
func (fs normalUserFs) MkdirAll(path string, perm os.FileMode) error {
_, err := fs.Stat(path)
if err != nil {
err = fs.checkParent(path, ownerWriteAndSearch)
if err != nil {
return err
}
}
return fs.Fs.MkdirAll(path, perm)
}
func (fs normalUserFs) Chmod(name string, mode os.FileMode) error {
err := fs.checkParent(name, ownerSearch)
if err != nil {
return err
}
return fs.Fs.Chmod(name, mode)
}
func (fs normalUserFs) Chtimes(name string, atime, mtime time.Time) error {
err := fs.checkParent(name, ownerSearch)
if err != nil {
return err
}
return fs.Fs.Chtimes(name, atime, mtime)
}
// checkParent returns a permission error when the directory holding name
// exists and its owner bits lack any of need.
func (fs normalUserFs) checkParent(name string, need os.FileMode) error {
info, err := fs.Stat(filepath.Dir(name))
if err == nil && info.Mode().Perm()&need != need {
return &os.PathError{Op: "access", Path: name, Err: os.ErrPermission}
}
return nil
}
// TestRestoreFillsReadOnlyDirectory checks that a read-only directory
// still receives the entries inside it, and ends with its stored mode.
func TestRestoreFillsReadOnlyDirectory(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ctx := context.Background()
rows, repos := makeFiles(ctx, t, []*database.File{
{Path: "/ro", Mode: readOnlyDirMode},
{Path: "/ro/file", Mode: plainFileMode},
{Path: "/ro/sub", Mode: readOnlyDirMode},
{Path: "/ro/sub/file", Mode: plainFileMode},
})
fs := afero.NewMemMapFs()
v := newContainmentVaultik(ctx, normalUserFs{Fs: fs})
_, err := v.restoreAllFiles(rows, repos,
&RestoreOptions{TargetDir: memTargetDir}, nil, nil)
require.NoError(t, err)
for _, path := range []string{"/ro/file", "/ro/sub/file"} {
_, err := fs.Stat(filepath.Join(memTargetDir, path))
require.NoErrorf(t, err, "file inside a read-only directory: %s", path)
}
for _, dir := range []string{"/ro", "/ro/sub"} {
info, err := fs.Stat(filepath.Join(memTargetDir, dir))
require.NoError(t, err)
assert.Equalf(t, os.FileMode(0o555), info.Mode().Perm(), "mode of %s", dir)
}
}
// TestRestoreKeepsNonEmptyDirectoryMTime checks that a directory keeps
// its stored mtime although entries were written into it. It runs on the
// real filesystem, where writing an entry changes its directory's mtime.
func TestRestoreKeepsNonEmptyDirectoryMTime(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ctx := context.Background()
targetDir := t.TempDir()
mtime := time.Date(2001, time.February, 3, 4, 5, 6, 0, time.UTC)
rows, repos := makeFiles(ctx, t, []*database.File{
{Path: "/dir", Mode: plainDirMode, MTime: mtime},
{Path: "/dir/file", Mode: plainFileMode, MTime: mtime},
})
v := newContainmentVaultik(ctx, afero.NewOsFs())
_, err := v.restoreAllFiles(rows, repos,
&RestoreOptions{TargetDir: targetDir}, nil, nil)
require.NoError(t, err)
info, err := os.Stat(filepath.Join(targetDir, "dir"))
require.NoError(t, err)
assert.Truef(t, info.ModTime().Equal(mtime),
"directory mtime is %s, stored %s", info.ModTime(), mtime)
}
// TestRestoreFinishesChildBeforeUnsearchableParent checks that a
// directory inside one whose stored mode denies search still gets its
// own stored mode and mtime.
func TestRestoreFinishesChildBeforeUnsearchableParent(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ctx := context.Background()
mtime := time.Date(2001, time.February, 3, 4, 5, 6, 0, time.UTC)
rows, repos := makeFiles(ctx, t, []*database.File{
{Path: "/locked", Mode: unsearchableMode, MTime: mtime},
{Path: "/locked/sub", Mode: plainDirMode, MTime: mtime},
})
fs := afero.NewMemMapFs()
v := newContainmentVaultik(ctx, normalUserFs{Fs: fs})
_, err := v.restoreAllFiles(rows, repos,
&RestoreOptions{TargetDir: memTargetDir}, nil, nil)
require.NoError(t, err)
info, err := fs.Stat(filepath.Join(memTargetDir, "locked"))
require.NoError(t, err)
assert.Equal(t, os.FileMode(0o600), info.Mode().Perm())
info, err = fs.Stat(filepath.Join(memTargetDir, "locked", "sub"))
require.NoError(t, err)
assert.Equal(t, os.FileMode(0o755), info.Mode().Perm())
assert.Truef(t, info.ModTime().Equal(mtime),
"mtime of sub is %s, stored %s", info.ModTime(), mtime)
}
// TestRestoreLeavesSymlinkedDirectoryTargetAlone checks that a directory
// whose place a later entry takes with a symlink does not hand its stored
// owner, mode and mtime to whatever the symlink points at. "/d" and "/d/"
// are different stored paths for the same place on disk.
func TestRestoreLeavesSymlinkedDirectoryTargetAlone(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ctx := context.Background()
tempDir := t.TempDir()
targetDir := filepath.Join(tempDir, "target")
outsideDir := filepath.Join(tempDir, "outside")
require.NoError(t, os.Mkdir(outsideDir, writableTestMode))
before, err := os.Stat(outsideDir)
require.NoError(t, err)
mtime := time.Date(2001, time.February, 3, 4, 5, 6, 0, time.UTC)
rows, repos := makeFiles(ctx, t, []*database.File{
{
Path: "/d",
Mode: readOnlyDirMode,
UID: otherOwnerID,
GID: otherOwnerID,
MTime: mtime,
},
{Path: "/d/", LinkTarget: types.FilePath(outsideDir), MTime: mtime},
})
v := newContainmentVaultik(ctx, afero.NewOsFs())
_, err = v.restoreAllFiles(rows, repos,
&RestoreOptions{TargetDir: targetDir}, nil, nil)
require.NoError(t, err)
after, err := os.Stat(outsideDir)
require.NoError(t, err)
assert.Equal(t, before.Mode(), after.Mode())
assert.Truef(t, after.ModTime().Equal(before.ModTime()),
"mtime changed from %s to %s", before.ModTime(), after.ModTime())
beforeOwner, ok := before.Sys().(*syscall.Stat_t)
require.True(t, ok)
afterOwner, ok := after.Sys().(*syscall.Stat_t)
require.True(t, ok)
assert.Equal(t, beforeOwner.Uid, afterOwner.Uid)
assert.Equal(t, beforeOwner.Gid, afterOwner.Gid)
}
// TestRestoreKeepsSetuidThroughChown checks that a setuid file keeps the
// bit when restore changes its owner. Linux clears setuid on any chown of
// a regular file, even one to its current owner, so the file is recorded
// with the current user as owner and the session is told it runs as
// root: the chown then needs no privilege.
func TestRestoreKeepsSetuidThroughChown(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ctx := context.Background()
targetDir := t.TempDir()
info, err := os.Stat(targetDir)
require.NoError(t, err)
owner, ok := info.Sys().(*syscall.Stat_t)
require.True(t, ok)
rows, repos := makeFiles(ctx, t, []*database.File{{
Path: "/suid",
Mode: setuidFileMode,
UID: owner.Uid,
GID: owner.Gid,
MTime: time.Date(2001, time.February, 3, 4, 5, 6, 0, time.UTC),
}})
session := &restoreSession{
v: newContainmentVaultik(ctx, afero.NewOsFs()),
ctx: ctx,
repos: repos,
opts: &RestoreOptions{TargetDir: targetDir},
result: &RestoreResult{},
runningAsRoot: true,
}
require.NoError(t, session.restoreFile(rows[0]))
info, err = os.Stat(filepath.Join(targetDir, "suid"))
require.NoError(t, err)
assert.Equal(t, os.FileMode(setuidFileMode),
info.Mode()&(os.ModeSetuid|os.ModePerm))
}
// TestRestoreSetsSymlinkMTime checks that a restored symlink gets its
// stored mtime on the link itself. The link dangles, so a call that
// follows it would fail.
func TestRestoreSetsSymlinkMTime(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ctx := context.Background()
targetDir := t.TempDir()
mtime := time.Date(2001, time.February, 3, 4, 5, 6, 0, time.UTC)
rows, repos := makeFiles(ctx, t, []*database.File{{
Path: "/link",
LinkTarget: types.FilePath(symlinkTargetPath),
MTime: mtime,
}})
v := newContainmentVaultik(ctx, afero.NewOsFs())
_, err := v.restoreAllFiles(rows, repos,
&RestoreOptions{TargetDir: targetDir}, nil, nil)
require.NoError(t, err)
info, err := os.Lstat(filepath.Join(targetDir, "link"))
require.NoError(t, err)
assert.Truef(t, info.ModTime().Equal(mtime),
"symlink mtime is %s, stored %s", info.ModTime(), mtime)
}
// TestRestoreSetsSymlinkOwnerAsRoot checks that a symlink restored as
// root gets its stored owner on the link itself.
func TestRestoreSetsSymlinkOwnerAsRoot(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
if os.Geteuid() != 0 {
t.Skip("giving a file to another user needs root")
}
ctx := context.Background()
targetDir := t.TempDir()
rows, repos := makeFiles(ctx, t, []*database.File{{
Path: "/link",
LinkTarget: types.FilePath(symlinkTargetPath),
UID: otherOwnerID,
GID: otherOwnerID,
MTime: time.Date(2001, time.February, 3, 4, 5, 6, 0, time.UTC),
}})
v := newContainmentVaultik(ctx, afero.NewOsFs())
_, err := v.restoreAllFiles(rows, repos,
&RestoreOptions{TargetDir: targetDir}, nil, nil)
require.NoError(t, err)
info, err := os.Lstat(filepath.Join(targetDir, "link"))
require.NoError(t, err)
owner, ok := info.Sys().(*syscall.Stat_t)
require.True(t, ok)
assert.Equal(t, otherOwnerID, owner.Uid)
assert.Equal(t, otherOwnerID, owner.Gid)
}