1 Commits
Author SHA1 Message Date
sneak 62f73397b3 Warn about restored ownership only when an owner differs (closes #316)
check / check (push) Waiting to run
A restore by a normal user always warned that file ownership was not
preserved, even when every recorded owner and group were the restoring
user's own, so ownership came out exactly as backed up. The warning is
now printed only when some entry to restore is recorded with another
owner or group than the restoring user's effective ids. A restore as
root never warns, as before.

The tests pass the restoring user's ids in, since make test runs as root.
Judgement call: an entry that failed to restore still counts.
Judgement call: a recorded group that is another of the user's groups
still warns, since restore changes no group as a normal user.

Model: opus-5-5
2026-10-13 22:11:20 +00:00
2 changed files with 123 additions and 7 deletions
+26 -7
View File
@@ -218,11 +218,7 @@ func (v *Vaultik) finishRestore(
v.UI.Duration(result.Duration),
)
if os.Geteuid() != 0 {
v.UI.Warningf("Restore did not preserve file ownership: chown(2) " +
"requires root. Re-run as root (e.g. with sudo) if you need " +
"original UID/GID preserved.")
}
v.warnIfOwnershipNotPreserved(files, os.Geteuid(), os.Getegid())
if result.FilesFailed > 0 {
v.UI.Warningf("%d file(s) failed to restore:", result.FilesFailed)
@@ -247,6 +243,28 @@ func (v *Vaultik) finishRestore(
return unmatchedErr
}
// warnIfOwnershipNotPreserved warns when restoring files as user uid
// and group gid leaves some entry with another owner or group than the
// snapshot records. Restore changes ownership only as root (uid 0); any
// other user's restore leaves every entry with that user's ids.
func (v *Vaultik) warnIfOwnershipNotPreserved(
files []*database.File, uid, gid int,
) {
if uid == 0 {
return
}
for _, file := range files {
if int(file.UID) != uid || int(file.GID) != gid {
v.UI.Warningf("Restore did not preserve file ownership: chown(2) " +
"requires root. Re-run as root (e.g. with sudo) if you need " +
"original UID/GID preserved.")
return
}
}
}
// reportUnmatchedPaths warns, by name, about each requested path that
// matched nothing in the snapshot, and returns an error counting them,
// or nil when there are none.
@@ -967,8 +985,9 @@ type restoreSession struct {
// root can chown a file to an arbitrary UID/GID — non-root chown
// always fails with EPERM. Attempting it anyway produces N
// guaranteed-failed syscalls + N noisy debug lines, so we skip
// the call entirely as non-root and emit one warning at the end
// of the restore explaining that ownership was not preserved.
// the call entirely as non-root; warnIfOwnershipNotPreserved warns
// once at the end of the restore if that left any owner or group
// other than recorded.
runningAsRoot bool
// directories holds every restored directory, for
// applyDirectoryMetadata to finish after the restore loop.
@@ -0,0 +1,97 @@
package vaultik //nolint:testpackage // calls the unexported ownership check
import (
"bytes"
"context"
"testing"
"github.com/spf13/afero"
"github.com/stretchr/testify/assert"
"sneak.berlin/go/vaultik/internal/database"
"sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/ui"
)
// These tests cover https://git.eeqj.de/sneak/vaultik/issues/316: a
// restore by a normal user warns that ownership was not preserved only
// when some entry's recorded owner or group differs from the restoring
// user's. The restoring user's ids are passed in, because make test runs
// as root.
const (
restoringUserID = 1000
restoringGroupID = 1000
rootUserID = 0
otherOwnedPath = "/other"
)
func TestRestoreOwnershipWarning(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ownFile := &database.File{
Path: "/own", UID: restoringUserID, GID: restoringGroupID,
}
tests := []struct {
name string
files []*database.File
uid int
wantWarn bool
}{
{
name: "every owner matches",
files: []*database.File{
ownFile,
{Path: "/own/dir", UID: restoringUserID, GID: restoringGroupID},
},
uid: restoringUserID,
wantWarn: false,
},
{
name: "another owner",
files: []*database.File{
ownFile,
{Path: otherOwnedPath, UID: otherOwnerID, GID: restoringGroupID},
},
uid: restoringUserID,
wantWarn: true,
},
{
name: "another group",
files: []*database.File{
ownFile,
{Path: otherOwnedPath, UID: restoringUserID, GID: otherOwnerID},
},
uid: restoringUserID,
wantWarn: true,
},
{
name: "root restores another owner",
files: []*database.File{
{Path: otherOwnedPath, UID: otherOwnerID, GID: otherOwnerID},
},
uid: rootUserID,
wantWarn: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
var out bytes.Buffer
v := newContainmentVaultik(context.Background(), afero.NewMemMapFs())
v.UI = ui.NewWithColor(&out, false)
v.warnIfOwnershipNotPreserved(tt.files, tt.uid, restoringGroupID)
if tt.wantWarn {
assert.Contains(t, out.String(), "did not preserve file ownership")
} else {
assert.Empty(t, out.String())
}
})
}
}