diff --git a/internal/vaultik/restore.go b/internal/vaultik/restore.go index 9b114ee..c9a1c89 100644 --- a/internal/vaultik/restore.go +++ b/internal/vaultik/restore.go @@ -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. diff --git a/internal/vaultik/restore_ownership_warning_test.go b/internal/vaultik/restore_ownership_warning_test.go new file mode 100644 index 0000000..978eec1 --- /dev/null +++ b/internal/vaultik/restore_ownership_warning_test.go @@ -0,0 +1,96 @@ +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 +) + +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: "/other", UID: otherOwnerID, GID: restoringGroupID}, + }, + uid: restoringUserID, + wantWarn: true, + }, + { + name: "another group", + files: []*database.File{ + ownFile, + {Path: "/other", UID: restoringUserID, GID: otherOwnerID}, + }, + uid: restoringUserID, + wantWarn: true, + }, + { + name: "root restores another owner", + files: []*database.File{ + {Path: "/other", 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()) + } + }) + } +}