Warn about restored ownership only when an owner differs (closes #316)

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

README.md

@@ -712,10 +712,8 @@ the `NO_COLOR` environment variable is unset (https://no-color.org/).
   reports and tables (`version`, `info`, `remote info`, `snapshot
   verify`, `snapshot list`) but never the machine-consumed `config get`
   value or the `--json` documents, which a script depends on. The
-  confirmation prompts of `database delete`, `prune`, `snapshot purge`
-  and `snapshot remove`, each with what it asks to delete, are also
-  written this way and always shown: they are an interactive exchange
-  the operator must see.
+  `database delete` confirmation prompt is also written this way and
+  always shown: it is an interactive exchange the operator must see.
 
 `internal/ui` writes to stdout; it is the output the user asked for. The
 exceptions are the startup banner and the error a failed command ends

internal/cli/entry_prune_quiet_test.go

@@ -1,123 +0,0 @@
-package cli //nolint:testpackage // shares runEntry and the hermetic config helpers
-
-import (
-	"os"
-	"path/filepath"
-	"strings"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-// flagCron is used to build the argument vectors below.
-const flagCron = "--cron"
-
-// plantUnreferencedBlob writes a blob that no snapshot references to the
-// destination store at storeDir, so that a prune deletes it. Returns its
-// path.
-func plantUnreferencedBlob(t *testing.T, storeDir string) string {
-	t.Helper()
-
-	hash := strings.Repeat("b", 64)
-	path := filepath.Join(storeDir, "blobs", hash[:2], hash[2:4], hash)
-
-	require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o750))
-	require.NoError(t, os.WriteFile(path, []byte("unreferenced"), 0o600))
-
-	return path
-}
-
-// TestEntrySnapshotCreatePruneIsSilent checks that a successful
-// `snapshot create --prune` under --cron or --quiet prints nothing on
-// stdout or stderr, whether or not the prune finds a blob to delete.
-//
-//nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals
-func TestEntrySnapshotCreatePruneIsSilent(t *testing.T) {
-	for _, testCase := range []struct {
-		name      string
-		args      []string
-		plantBlob bool
-	}{
-		{
-			name: "cron, no blob to delete",
-			args: []string{cmdSnapshot, cmdCreate, flagCron, flagPrune},
-		},
-		{
-			name:      "cron, blob to delete",
-			args:      []string{cmdSnapshot, cmdCreate, flagCron, flagPrune},
-			plantBlob: true,
-		},
-		{
-			name: "cron with keep-newer-than",
-			args: []string{
-				cmdSnapshot, cmdCreate, flagCron, flagPrune,
-				flagKeepNewerThan, "4w",
-			},
-			plantBlob: true,
-		},
-		{
-			name:      "quiet",
-			args:      []string{flagQuiet, cmdSnapshot, cmdCreate, flagPrune},
-			plantBlob: true,
-		},
-	} {
-		t.Run(testCase.name, func(t *testing.T) {
-			configPath, storeDir := writeBackupConfig(t)
-
-			var blob string
-			if testCase.plantBlob {
-				blob = plantUnreferencedBlob(t, storeDir)
-			}
-
-			code, stdout, stderr := runEntry(t,
-				append([]string{flagConfig, configPath}, testCase.args...)...)
-
-			assert.Equal(t, 0, code)
-			assert.Empty(t, stdout)
-			assert.Empty(t, stderr)
-
-			if testCase.plantBlob {
-				assert.NoFileExists(t, blob, "the prune must delete the blob")
-			}
-		})
-	}
-}
-
-// TestEntryPruneQuietIsSilent checks that a successful `prune --force`
-// under --quiet prints nothing on stdout or stderr, both when there is
-// nothing to remove and when it removes a stale local snapshot record
-// and an unreferenced blob.
-//
-//nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals
-func TestEntryPruneQuietIsSilent(t *testing.T) {
-	for _, testCase := range []struct {
-		name               string
-		staleRecordAndBlob bool
-	}{
-		{name: "nothing to remove"},
-		{name: "stale record and blob to remove", staleRecordAndBlob: true},
-	} {
-		t.Run(testCase.name, func(t *testing.T) {
-			configPath := writeHermeticPruneConfig(t, testCase.staleRecordAndBlob)
-
-			var blob string
-			if testCase.staleRecordAndBlob {
-				// writeHermeticPruneConfig puts the store beside the config.
-				blob = plantUnreferencedBlob(t,
-					filepath.Join(filepath.Dir(configPath), "store"))
-			}
-
-			code, stdout, stderr := runEntry(t,
-				flagConfig, configPath, flagQuiet, cmdPrune, flagForce)
-
-			assert.Equal(t, 0, code)
-			assert.Empty(t, stdout)
-			assert.Empty(t, stderr)
-
-			if testCase.staleRecordAndBlob {
-				assert.NoFileExists(t, blob, "the prune must delete the blob")
-			}
-		})
-	}
-}

internal/snapshot/exclude_test.go

@@ -578,54 +578,3 @@ func TestExcludePatterns_DoubleStarFileAtEveryDepth(t *testing.T) {
 	require.Equal(t, 1, result.FilesScanned,
 		"**/ file pattern should match at the top level and nested")
 }
-
-func TestExcludePatterns_AnchoredDoubleStarAtEveryDepth(t *testing.T) {
-	t.Parallel()
-
-	// As in gitignore, /**/ also matches directly under the source dir
-	fs := setupAnchoredTestFS(t)
-
-	scanner, repos, cleanup := createTestScanner(t, fs, []string{"/**/projectname"})
-	defer cleanup()
-
-	ctx := context.Background()
-	createSnapshotRecord(ctx, t, repos, "test-snapshot")
-
-	result, err := scanner.Scan(ctx, "/backup", "test-snapshot")
-	require.NoError(t, err)
-
-	// /**/projectname should exclude BOTH:
-	// - /backup/projectname/file.txt
-	// - /backup/otherproject/projectname/file.txt
-	// Total files: 4, excluded: 2
-	require.Equal(t, 2, result.FilesScanned,
-		"/**/ pattern should match at the top level and nested")
-}
-
-func TestExcludePatterns_DoubleStarInMiddleMatchesZeroDirectories(t *testing.T) {
-	t.Parallel()
-
-	// As in gitignore, a/**/b matches a/b as well as a/c/b. The last part
-	// is a glob: with a plain name the glob library already matched a/b.
-	fs := &afero.MemMapFs{}
-	files := []string{"/backup/a/x.log", "/backup/a/c/y.log", "/backup/a/c/kept.txt"}
-
-	for _, path := range files {
-		require.NoError(t, fs.MkdirAll(filepath.Dir(path), 0755))
-		require.NoError(t, afero.WriteFile(fs, path, []byte(path), 0644))
-	}
-
-	scanner, repos, cleanup := createTestScanner(t, fs, []string{"a/**/*.log"})
-	defer cleanup()
-
-	ctx := context.Background()
-	createSnapshotRecord(ctx, t, repos, "test-snapshot")
-
-	result, err := scanner.Scan(ctx, "/backup", "test-snapshot")
-	require.NoError(t, err)
-
-	// a/**/*.log should exclude /backup/a/x.log and /backup/a/c/y.log
-	// Total files: 3, excluded: 2
-	require.Equal(t, 1, result.FilesScanned,
-		"a/**/*.log should match a/x.log as well as a/c/y.log")
-}

internal/snapshot/scanner.go

@@ -2035,60 +2035,33 @@ func compileExcludePatterns(patterns []string) []compiledPattern {
 		pattern = strings.TrimSuffix(pattern, "/")
 
 		// A leading "**/" demands a "/" before the name and so misses an
-		// entry directly under the source path. It means "at every depth",
-		// which is what an unanchored pattern already does, so drop the
-		// prefix and match unanchored: as in gitignore, "**/x" and
-		// "/**/x" then also match a top-level "x".
-		if rest, found := strings.CutPrefix(pattern, "**/"); found {
-			pattern = rest
-			anchored = false
+		// entry directly under the source path. An unanchored pattern
+		// already matches at every depth, so drop the prefix: as in
+		// gitignore, "**/x" then also matches a top-level "x".
+		if !anchored {
+			pattern = strings.TrimPrefix(pattern, "**/")
 		}
 
-		// Likewise "/**/" inside a pattern demands two slashes, so
-		// "a/**/*.log" misses "a/x.log". As in gitignore it stands for
-		// zero or more directories, so also compile the pattern with it
-		// made a single "/". The glob library's {a,b} is no help: with a
-		// "**" inside it in mid-pattern, it misses paths it should match.
-		for _, variant := range doubleStarVariants(pattern) {
-			// For patterns without path separators, we need to match them as components
-			// e.g., ".git" should match ".git" anywhere in the path
-			g, err := glob.Compile(variant, '/')
-			if err != nil {
-				log.Warn("Invalid exclude pattern, skipping", "pattern", p, "error", err)
+		// Compile the glob pattern
+		// For patterns without path separators, we need to match them as components
+		// e.g., ".git" should match ".git" anywhere in the path
+		g, err := glob.Compile(pattern, '/')
+		if err != nil {
+			log.Warn("Invalid exclude pattern, skipping", "pattern", p, "error", err)
 
-				break
-			}
-
-			compiled = append(compiled, compiledPattern{
-				pattern:  g,
-				anchored: anchored,
-				original: p,
-			})
+			continue
 		}
+
+		compiled = append(compiled, compiledPattern{
+			pattern:  g,
+			anchored: anchored,
+			original: p,
+		})
 	}
 
 	return compiled
 }
 
-// doubleStarVariants returns pattern once for every way of keeping each
-// "/**/" in it or making it a single "/".
-func doubleStarVariants(pattern string) []string {
-	const doubleStar = "/**/"
-
-	before, after, found := strings.Cut(pattern, doubleStar)
-	if !found {
-		return []string{pattern}
-	}
-
-	var variants []string
-
-	for _, rest := range doubleStarVariants(after) {
-		variants = append(variants, before+doubleStar+rest, before+"/"+rest)
-	}
-
-	return variants
-}
-
 // shouldExclude checks if a path should be excluded based on exclude patterns
 // filePath is the full path to the file
 // rootPath is the root of the backup source directory

internal/storage/rclone.go

@@ -7,12 +7,10 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"os"
 	"strings"
 	"time"
 
 	"github.com/rclone/rclone/fs"
-	"github.com/rclone/rclone/fs/config"
 	"github.com/rclone/rclone/fs/config/configfile"
 	"github.com/rclone/rclone/fs/operations"
 
@@ -35,15 +33,6 @@ type RcloneStorer struct {
 // The remote parameter is the rclone remote name (as configured via `rclone config`).
 // The path parameter is the path within the remote.
 func NewRcloneStorer(ctx context.Context, remote, path string) (*RcloneStorer, error) {
-	// rclone reads RCLONE_CONFIG only as the value of its --config flag,
-	// which vaultik does not have, so it is applied here as that flag would.
-	if configPath, ok := os.LookupEnv("RCLONE_CONFIG"); ok {
-		err := config.SetConfigPath(configPath)
-		if err != nil {
-			return nil, fmt.Errorf("using RCLONE_CONFIG %q: %w", configPath, err)
-		}
-	}
-
 	// Install the default config file handler
 	configfile.Install()
 

internal/storage/rclone_test.go

@@ -11,7 +11,6 @@ import (
 	"testing"
 
 	"github.com/rclone/rclone/fs"
-	"github.com/rclone/rclone/fs/config"
 	"github.com/rclone/rclone/fs/config/configmap"
 	"sneak.berlin/go/vaultik/internal/storage"
 )
@@ -391,41 +390,3 @@ func TestNewRcloneStorerUnknownRemote(t *testing.T) {
 		t.Errorf("NewRcloneStorer error = %v, want ErrRemoteNotFound", err)
 	}
 }
-
-// TestNewRcloneStorerReadsRcloneConfigEnv checks that a remote defined only
-// in the file RCLONE_CONFIG names is found and written to, as rclone itself
-// would.
-func TestNewRcloneStorerReadsRcloneConfigEnv(t *testing.T) {
-	// rclone keeps the config file path in a process-global variable, which
-	// NewRcloneStorer leaves pointing at this test's file.
-	previous := config.GetConfigPath()
-
-	t.Cleanup(func() { _ = config.SetConfigPath(previous) })
-
-	confFile := filepath.Join(t.TempDir(), "rclone.conf")
-
-	err := os.WriteFile(confFile, []byte("[loc]\ntype = local\n"), 0o600)
-	if err != nil {
-		t.Fatalf("writing rclone config: %v", err)
-	}
-
-	t.Setenv("RCLONE_CONFIG", confFile)
-
-	dir := t.TempDir()
-	ctx := context.Background()
-
-	s, err := storage.NewRcloneStorer(ctx, "loc", dir)
-	if err != nil {
-		t.Fatalf("NewRcloneStorer: %v", err)
-	}
-
-	err = s.Put(ctx, testBlobKey, strings.NewReader("blob-bytes"))
-	if err != nil {
-		t.Fatalf("Put: %v", err)
-	}
-
-	_, err = os.Stat(filepath.Join(dir, testBlobKey))
-	if err != nil {
-		t.Errorf("object is not in the remote's directory: %v", err)
-	}
-}

internal/vaultik/prune.go

@@ -204,7 +204,7 @@ func (v *Vaultik) PruneBlobs(opts *PruneOptions) error {
 			return v.outputPruneBlobsJSON(result)
 		}
 
-		v.UI.Infof("No unreferenced blobs to remove.")
+		v.printlnStdout("No unreferenced blobs to remove.")
 
 		return nil
 	}
@@ -212,17 +212,12 @@ func (v *Vaultik) PruneBlobs(opts *PruneOptions) error {
 	log.Info("Found unreferenced blobs",
 		"count", len(unreferencedBlobs), "total_size", ubytes(totalSize))
 
-	if opts.Force && !opts.JSON {
-		v.UI.Infof("Found %d unreferenced blob(s) totaling %s.",
+	if !opts.JSON {
+		v.stdoutf("Found %d unreferenced blob(s) totaling %s\n",
 			len(unreferencedBlobs), ubytes(totalSize))
 	}
 
 	if !opts.Force && !opts.JSON {
-		// The count, the prompt and its answer bypass the UI: they are an
-		// interactive exchange the operator must see, so --quiet does not
-		// silence them.
-		v.stdoutf("Found %d unreferenced blob(s) totaling %s.\n",
-			len(unreferencedBlobs), ubytes(totalSize))
 		v.stdoutf("\nDelete %d unreferenced blob(s)? [y/N] ", len(unreferencedBlobs))
 
 		var confirm string
@@ -249,7 +244,7 @@ func (v *Vaultik) PruneBlobs(opts *PruneOptions) error {
 			return err
 		}
 	} else {
-		v.UI.Infof("Deleted %d blob(s) totaling %s.",
+		v.stdoutf("\nDeleted %d blob(s) totaling %s\n",
 			result.BlobsDeleted, ubytes(result.BytesFreed))
 	}
 

internal/vaultik/purge_local_remote_consistency_test.go

@@ -14,7 +14,6 @@ import (
 	"sneak.berlin/go/vaultik/internal/log"
 	"sneak.berlin/go/vaultik/internal/snapshot"
 	"sneak.berlin/go/vaultik/internal/types"
-	"sneak.berlin/go/vaultik/internal/ui"
 	"sneak.berlin/go/vaultik/internal/vaultik"
 )
 
@@ -59,16 +58,13 @@ func setupConsistencyTest(
 		require.NoError(t, err)
 	}
 
-	stdout := &bytes.Buffer{}
-
 	v := &vaultik.Vaultik{
 		Storage:      mockStorage,
 		Repositories: repos,
 		DB:           db,
-		Stdout:       stdout,
+		Stdout:       &bytes.Buffer{},
 		Stderr:       &bytes.Buffer{},
 		Stdin:        &bytes.Buffer{},
-		UI:           ui.NewWithColor(stdout, false),
 	}
 	v.SetContext(ctx)
 

internal/vaultik/purge_per_name_test.go

@@ -14,7 +14,6 @@ import (
 	"sneak.berlin/go/vaultik/internal/log"
 	"sneak.berlin/go/vaultik/internal/snapshot"
 	"sneak.berlin/go/vaultik/internal/types"
-	"sneak.berlin/go/vaultik/internal/ui"
 	"sneak.berlin/go/vaultik/internal/vaultik"
 )
 
@@ -87,7 +86,6 @@ func setupPurgeTest(
 		Stdout:       stdout,
 		Stderr:       stderr,
 		Stdin:        stdin,
-		UI:           ui.NewWithColor(stdout, false),
 	}
 	v.SetContext(ctx)
 

internal/vaultik/remove_quiet_test.go

@@ -1,148 +0,0 @@
-package vaultik_test
-
-import (
-	"bytes"
-	"context"
-	"encoding/json"
-	"path/filepath"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/vaultik/internal/log"
-	"sneak.berlin/go/vaultik/internal/ui"
-	"sneak.berlin/go/vaultik/internal/vaultik"
-)
-
-// TestPurgeSnapshots_QuietSuppressesReport checks that `snapshot purge
-// --keep-latest --force` under --quiet deletes the older snapshot and
-// prints nothing.
-func TestPurgeSnapshots_QuietSuppressesReport(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	v := setupPurgeTest(t, snapHostname, []string{snapHomeT0, snapHomeT1})
-
-	var out bytes.Buffer
-
-	v.Stdout = &out
-	v.UI = ui.NewWithColor(&out, false)
-	v.UI.SetQuiet(true)
-
-	require.NoError(t, v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{
-		KeepLatest: true,
-		Force:      true,
-	}))
-
-	assert.Equal(t, []string{snapHomeT1}, listRemainingSnapshots(t, v))
-	assert.Empty(t, out.String())
-}
-
-// TestPurgeSnapshots_QuietPromptListsSnapshots checks that `snapshot
-// purge --keep-latest` without --force still lists the snapshots its
-// prompt asks to delete under --quiet.
-func TestPurgeSnapshots_QuietPromptListsSnapshots(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	v := setupPurgeTest(t, snapHostname, []string{snapHomeT0, snapHomeT1})
-
-	var out bytes.Buffer
-
-	v.Stdout = &out
-	v.UI = ui.NewWithColor(&out, false)
-	v.UI.SetQuiet(true)
-
-	// Stdin is empty, so the prompt reads no answer and cancels.
-	require.NoError(t, v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{
-		KeepLatest: true,
-	}))
-
-	assert.Contains(t, out.String(), snapHomeT0)
-	assert.Contains(t, out.String(), "Delete 1 snapshot(s)? [y/N]")
-	assert.Len(t, listRemainingSnapshots(t, v), 2)
-}
-
-// TestPruneBlobs_QuietPromptShowsBlobTotal checks that `prune` without
-// --force still states the count and total size of the blobs its prompt
-// asks to delete under --quiet.
-func TestPruneBlobs_QuietPromptShowsBlobTotal(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	env := newListEnv(t)
-	env.v.UI.SetQuiet(true)
-
-	// The store holds no manifest, so nothing references this blob.
-	addBlob(t, env.store.testStorer, testBlobHashA)
-
-	// Stdin is empty, so the prompt reads no answer and cancels.
-	require.NoError(t, env.v.PruneBlobs(&vaultik.PruneOptions{}))
-
-	assert.Contains(t, env.stdout.String(),
-		"Found 1 unreferenced blob(s) totaling")
-	assert.Contains(t, env.stdout.String(),
-		"Delete 1 unreferenced blob(s)? [y/N]")
-}
-
-// TestRemoveSnapshot_QuietSuppressesReport checks that `snapshot remove`
-// under --quiet prints nothing with --dry-run or with --force, and that
-// the --json document still prints.
-func TestRemoveSnapshot_QuietSuppressesReport(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	newQuietRemover := func() (*vaultik.TestVaultik, *testStorer) {
-		store := newTestStorer()
-		addManifest(t, store, "snapshot-001", nil)
-
-		tv := vaultik.NewForTesting(store)
-		tv.Repositories = newLocalIndex(t, "snapshot-001")
-		tv.UI.SetQuiet(true)
-
-		return tv, store
-	}
-
-	tv, store := newQuietRemover()
-
-	_, err := tv.RemoveSnapshot("snapshot-001",
-		&vaultik.RemoveOptions{DryRun: true})
-	require.NoError(t, err)
-	assert.Empty(t, tv.Stdout.String(),
-		"--dry-run must print nothing under --quiet")
-
-	_, err = tv.RemoveSnapshot("snapshot-001",
-		&vaultik.RemoveOptions{Force: true})
-	require.NoError(t, err)
-	assert.False(t, store.hasKey(remoteKeyPath("snapshot-001")))
-	assert.Empty(t, tv.Stdout.String(),
-		"--force must print nothing under --quiet")
-
-	tv, _ = newQuietRemover()
-
-	_, err = tv.RemoveSnapshot("snapshot-001",
-		&vaultik.RemoveOptions{JSON: true})
-	require.NoError(t, err)
-
-	var result vaultik.RemoveResult
-
-	require.NoError(t, json.Unmarshal(tv.Stdout.Bytes(), &result),
-		"the --json document must still print under --quiet")
-	assert.True(t, result.RemoteRemoved)
-}
-
-// TestNukeRemote_QuietSuppressesReport checks that `remote nuke --force`
-// under --quiet prints nothing while it removes a snapshot's metadata
-// and its blobs.
-func TestNukeRemote_QuietSuppressesReport(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	ctx := context.Background()
-	storeDir := filepath.Join(t.TempDir(), "store")
-	v, _, out := backUpToFileDestination(ctx, t, storeDir)
-	v.UI.SetQuiet(true)
-
-	require.NoError(t, v.NukeRemote(true))
-	assert.Empty(t, out.String())
-}

internal/vaultik/remove_snapshot_test.go

@@ -350,7 +350,7 @@ func TestRemoveSnapshot_DryRun(t *testing.T) {
 	assert.True(t, store.hasKey("blobs/aa/aa/"+blobA))
 	assert.True(t, store.hasKey(remoteKeyPath("snapshot-001")))
 
-	assert.Contains(t, tv.Stdout.String(), "Dry run: no changes made.")
+	assert.Contains(t, tv.Stdout.String(), "[Dry run - no changes made]")
 }
 
 // TestRemoveSnapshot_UnknownIDIsError checks that an ID naming no
@@ -504,7 +504,7 @@ func TestRemoveAllSnapshots_DryRun(t *testing.T) {
 
 	assert.Equal(t, initialCount, store.keyCount())
 
-	assert.Contains(t, tv.Stdout.String(), "Dry run: no changes made.")
+	assert.Contains(t, tv.Stdout.String(), "[Dry run - no changes made]")
 }
 
 func TestRemoveAllSnapshots_NoSnapshots(t *testing.T) {

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.

internal/vaultik/restore_ownership_warning_test.go

@@ -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())
+			}
+		})
+	}
+}

internal/vaultik/snapshot.go

@@ -574,7 +574,7 @@ func (v *Vaultik) PurgeSnapshotsWithOptions(opts *SnapshotPurgeOptions) error {
 
 	if len(toDelete) == 0 {
 		if !opts.Quiet {
-			v.UI.Infof("No snapshots to delete.")
+			v.printlnStdout("No snapshots to delete")
 		}
 
 		return nil
@@ -631,19 +631,19 @@ func selectSnapshotsToPurge(
 func (v *Vaultik) confirmAndExecutePurge(
 	toDelete []SnapshotInfo, force, quiet bool,
 ) error {
-	// Confirm unless --force is used. The list, the prompt and its answer
-	// bypass the UI: they are an interactive exchange the operator must
-	// see, so --quiet does not silence them.
-	if !force {
+	if !quiet {
 		v.stdoutf("The following snapshots will be deleted:\n\n")
 
 		for _, snap := range toDelete {
 			v.stdoutf("  %s (%s, %s)\n",
 				snap.ID,
-				snap.Timestamp.Format(time.DateTime),
+				snap.Timestamp.Format("2006-01-02 15:04:05"),
 				ubytes(snap.CompressedSize))
 		}
+	}
 
+	// Confirm unless --force is used
+	if !force {
 		v.stdoutf("\nDelete %d snapshot(s)? [y/N] ", len(toDelete))
 
 		var confirm string
@@ -661,16 +661,7 @@ func (v *Vaultik) confirmAndExecutePurge(
 			return nil
 		}
 	} else if !quiet {
-		v.UI.Infof("The following snapshots will be deleted:")
-
-		for _, snap := range toDelete {
-			v.UI.Detailf("%s (%s, %s)",
-				v.UI.Snapshot(snap.ID.String()),
-				snap.Timestamp.Format(time.DateTime),
-				ubytes(snap.CompressedSize))
-		}
-
-		v.UI.Infof("Deleting %d snapshot(s) (--force specified).", len(toDelete))
+		v.stdoutf("\nDeleting %d snapshot(s) (--force specified)\n", len(toDelete))
 	}
 
 	// Delete snapshots (both local and remote)
@@ -703,9 +694,9 @@ func (v *Vaultik) confirmAndExecutePurge(
 	}
 
 	if !quiet {
-		v.UI.Infof("Deleted %d snapshot(s).", len(toDelete))
-		v.UI.Noticef("Run '%s' to clean up unreferenced remote blobs.",
-			pruneCommandHint)
+		v.stdoutf("Deleted %d snapshot(s)\n", len(toDelete))
+		v.printlnStdout(
+			"\nNote: Run 'vaultik prune' to clean up unreferenced remote blobs.")
 	}
 
 	return nil
@@ -1078,7 +1069,7 @@ func (v *Vaultik) CleanupLocalSnapshots(opts *PruneOptions) error {
 			log.Info("Removing stale local snapshot record", "snapshot_id", id)
 
 			if !opts.JSON {
-				v.UI.Infof("Removing stale local record: %s.", v.UI.Snapshot(id))
+				v.stdoutf("Removing stale local record: %s\n", id)
 			}
 
 			err = v.deleteSnapshotFromLocalDB(id)
@@ -1101,9 +1092,9 @@ func (v *Vaultik) CleanupLocalSnapshots(opts *PruneOptions) error {
 	}
 
 	if removed == 0 {
-		v.UI.Infof("No stale local snapshots found.")
+		v.printlnStdout("No stale local snapshots found.")
 	} else {
-		v.UI.Infof("Removed %d stale local snapshot record(s).", removed)
+		v.stdoutf("Removed %d stale local snapshot record(s).\n", removed)
 	}
 
 	return nil
@@ -1282,15 +1273,14 @@ func (v *Vaultik) RemoveSnapshot(
 	}
 
 	if inLocalIndex {
-		v.UI.Infof("Removed snapshot '%s' from local database.",
-			v.UI.Snapshot(snapshotID))
+		v.stdoutf("Removed snapshot '%s' from local database\n", snapshotID)
 	}
 
 	if result.RemoteRemoved {
-		v.UI.Infof("Removed snapshot metadata from remote storage.")
-		v.UI.Noticef("The removed snapshot's blobs remain on the remote. "+
+		v.printlnStdout("Removed snapshot metadata from remote storage")
+		v.stdoutf("\nNote: The removed snapshot's blobs remain on the remote. "+
 			"Run '%s' to delete any blobs no longer referenced by any "+
-			"remaining remote snapshot.", pruneCommandHint)
+			"remaining remote snapshot.\n", pruneCommandHint)
 	}
 
 	return result, nil
@@ -1346,23 +1336,21 @@ func (v *Vaultik) removeSnapshotDryRun(
 		return result, v.outputRemoveJSON(result)
 	}
 
-	v.UI.Infof("Would remove snapshot: %s.", v.UI.Snapshot(snapshotID))
+	v.stdoutf("Would remove snapshot: %s\n", snapshotID)
 
 	if !opts.LocalOnly {
-		v.UI.Infof("Would also remove snapshot metadata from " +
-			"remote storage (blobs untouched).")
+		v.printlnStdout("Would also remove snapshot metadata from " +
+			"remote storage (blobs untouched)")
 	}
 
-	v.UI.Noticef("Dry run: no changes made.")
+	v.printlnStdout("[Dry run - no changes made]")
 
 	return result, nil
 }
 
 // confirmRemoveSnapshot prompts for confirmation before removing a
 // snapshot. Returns false (treating EOF/read errors as "no") unless the
-// user answers "y". The prompt and its answer bypass the UI: they are an
-// interactive exchange the operator must see, so --quiet does not
-// silence them.
+// user answers "y".
 func (v *Vaultik) confirmRemoveSnapshot(snapshotID string, opts *RemoveOptions) bool {
 	if opts.LocalOnly {
 		v.stdoutf("Remove snapshot '%s' from local database "+
@@ -1468,7 +1456,7 @@ func (v *Vaultik) RemoveAllSnapshots(opts *RemoveOptions) (*RemoveResult, error)
 
 	if len(localSnaps) == 0 && len(orphanRemoteKeys) == 0 {
 		if !opts.JSON {
-			v.UI.Infof("No snapshots found.")
+			v.printlnStdout("No snapshots found")
 		}
 
 		return &RemoveResult{}, nil
@@ -1573,27 +1561,27 @@ func (v *Vaultik) handleRemoveAllDryRun(
 	}
 
 	if !opts.JSON {
-		v.UI.Infof("Would remove %d local snapshot(s):", len(localSnaps))
+		v.stdoutf("Would remove %d local snapshot(s):\n", len(localSnaps))
 
 		for _, id := range localSnaps {
-			v.UI.Detailf("%s", v.UI.Snapshot(id))
+			v.stdoutf("  %s\n", id)
 		}
 
 		if !opts.LocalOnly {
 			if len(orphanRemoteKeys) > 0 {
-				v.UI.Infof("Would also remove %d orphan remote snapshot key(s):",
+				v.stdoutf("Would also remove %d orphan remote snapshot key(s):\n",
 					len(orphanRemoteKeys))
 
 				for _, key := range orphanRemoteKeys {
-					v.UI.Detailf("%s", key)
+					v.stdoutf("  %s\n", key)
 				}
 			} else {
-				v.UI.Infof("Would also remove snapshot metadata from " +
-					"remote storage (blobs untouched).")
+				v.printlnStdout("Would also remove snapshot metadata from " +
+					"remote storage (blobs untouched)")
 			}
 		}
 
-		v.UI.Noticef("Dry run: no changes made.")
+		v.printlnStdout("[Dry run - no changes made]")
 	}
 
 	if opts.JSON {
@@ -1642,10 +1630,10 @@ func (v *Vaultik) executeRemoveAll(
 		return result, v.outputRemoveJSON(result)
 	}
 
-	v.UI.Infof("Removed %d snapshot(s).", len(result.SnapshotsRemoved))
+	v.stdoutf("Removed %d snapshot(s)\n", len(result.SnapshotsRemoved))
 
 	if !opts.LocalOnly && result.RemoteRemoved {
-		v.UI.Infof("Removed snapshot metadata from remote storage.")
+		v.printlnStdout("Removed snapshot metadata from remote storage")
 	}
 
 	return result, nil