Warn about restored ownership only when an entry's owner or group differs (closes #316)

A restore by a normal user always warned that file ownership was not
preserved, even when every entry came out with its recorded owner and
group. After such a restore, the owner and group each restored entry has
on disk are now compared with the recorded ones, and the warning is
printed once if any differ. The comparison is made on disk because a new
entry does not always get the user's group: on macOS, and in a setgid
directory on Linux, it takes the group of its directory. A restore as
root never warns, as before.

Judgement call: an entry that is not on disk after the restore, such as one that failed, is not compared.
Unverified: not run on a Mac; the directory-group test uses a setgid directory on Linux and runs only as root.

Model: opus-5-5

internal/vaultik/blob_fetch.go

@@ -104,7 +104,8 @@ func (v *Vaultik) FetchAndDecryptBlob(
 
 // FetchBlob downloads a blob and returns a reader for the encrypted data.
 // The reader is closed as soon as ctx is cancelled, which ends a read
-// blocked inside it.
+// blocked inside it. A storage error is returned as is: restore and deep
+// verify name the blob in their own errors.
 func (v *Vaultik) FetchBlob(
 	ctx context.Context, blobHash string,
 ) (io.ReadCloser, error) {
@@ -119,7 +120,7 @@ func (v *Vaultik) FetchBlob(
 
 	rc, err := v.Storage.Get(ctx, blobPath)
 	if err != nil {
-		return nil, fmt.Errorf("downloading blob %s: %w", shortHash(blobHash), err)
+		return nil, err
 	}
 
 	return newCloseOnCancelReader(ctx, rc), nil

internal/vaultik/deep_verify_test.go

@@ -6,6 +6,7 @@ import (
 	"crypto/rand"
 	"io"
 	"os"
+	"path"
 	"path/filepath"
 	"testing"
 
@@ -142,7 +143,8 @@ func TestDeepVerifyJSONCountsBlobs(t *testing.T) {
 
 // TestDeepVerifyJSONCountsWrongSizeBlob checks that under --deep --json a
 // blob of the wrong size is counted in mismatched and fails the run, and
-// that the check goes on past it to count a missing blob after it.
+// that the check goes on past it to count a missing blob after it. The
+// store is on disk, so the message must not call it S3.
 func TestDeepVerifyJSONCountsWrongSizeBlob(t *testing.T) {
 	log.Initialize(log.Config{})
 	t.Parallel()
@@ -168,11 +170,14 @@ func TestDeepVerifyJSONCountsWrongSizeBlob(t *testing.T) {
 	require.Equal(t, 1, got.Mismatched)
 	require.Equal(t, 1, got.Missing)
 	require.Zero(t, got.Verified)
+	require.Contains(t, got.ErrorMessage, "with the wrong size")
+	require.NotContains(t, got.ErrorMessage, "S3")
 }
 
 // TestDeepVerifyStopsOnBlobThatCannotBeChecked checks that a blob that is
 // present in storage but cannot be checked is not counted as missing:
-// deep verify stops and fails with the storage error.
+// deep verify stops and fails, saying it could not check the blob, with
+// the storage error.
 func TestDeepVerifyStopsOnBlobThatCannotBeChecked(t *testing.T) {
 	log.Initialize(log.Config{})
 	t.Parallel()
@@ -187,6 +192,8 @@ func TestDeepVerifyStopsOnBlobThatCannotBeChecked(t *testing.T) {
 	require.Error(t, err)
 	require.Equal(t, "failed", got.Status)
 	require.Zero(t, got.Missing)
+	require.Contains(t, got.ErrorMessage,
+		"could not check blob "+path.Base(keys[0]))
 	require.Contains(t, got.ErrorMessage, errStatDenied.Error())
 }
 

internal/vaultik/purge_per_name_test.go

@@ -281,7 +281,8 @@ func TestPurgeOlderThan_TooLargeDeletesNothing(t *testing.T) {
 		OlderThan: "1000y",
 		Force:     true,
 	})
-	require.Error(t, err)
+	require.EqualError(t, err,
+		`invalid duration: "1000y" is longer than about 292 years`)
 
 	err = v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{
 		OlderThan: "292y",

internal/vaultik/restore_skip_errors_test.go

@@ -231,7 +231,8 @@ func (f trailingByteFile) Close() error {
 }
 
 // TestRestoreMissingBlobAbortsWithoutSkipErrors checks that a deleted blob
-// still ends the restore with an error when SkipErrors is not set.
+// still ends the restore with an error when SkipErrors is not set, and
+// that the error names the blob once.
 func TestRestoreMissingBlobAbortsWithoutSkipErrors(t *testing.T) {
 	log.Initialize(log.Config{})
 	t.Parallel()
@@ -247,6 +248,8 @@ func TestRestoreMissingBlobAbortsWithoutSkipErrors(t *testing.T) {
 	})
 
 	require.ErrorIs(t, err, storage.ErrNotFound)
+	assert.Equal(t, 1, strings.Count(err.Error(), "downloading blob"),
+		err.Error())
 }
 
 // TestRestoreChunkInNoBlobAbortsWithoutSkipErrors checks that a chunk held

internal/vaultik/shallow_verify_test.go

@@ -27,7 +27,7 @@ import (
 // then grows one stored blob so its size no longer matches the manifest.
 // Shallow verify must then fail, count the grown blob as a size mismatch,
 // and drop it from the verified count rather than continuing to report it
-// as checked.
+// as checked, and its error must not repeat the cli layer's prefix.
 func TestShallowVerifyDetectsWrongBlobSize(t *testing.T) {
 	log.Initialize(log.Config{})
 	t.Parallel()
@@ -96,6 +96,12 @@ func TestShallowVerifyDetectsWrongBlobSize(t *testing.T) {
 	require.Equal(t, 1, bad.Mismatched)
 	require.Equal(t, healthy.BlobCount-1, bad.Verified,
 		"the wrong-sized blob must not be counted as verified")
+
+	// The cli layer prints this error after its own "Verification failed:".
+	err = newVerifier(io.Discard).VerifySnapshotWithOptions(
+		snapshotID, &vaultik.VerifyOptions{})
+	require.EqualError(t, err,
+		"problems found in storage: 1 blobs have the wrong size")
 }
 
 // TestShallowVerifyDetectsMissingDatabase backs up a real snapshot,

internal/vaultik/snapshot.go

@@ -23,6 +23,7 @@ var (
 	errSnapshotNotInConfig  = errors.New("snapshot not found in config")
 	errNoSnapshotsInConfig  = errors.New("no snapshots configured")
 	errSnapshotVerifyFailed = errors.New("verification failed")
+	errProblemsInStorage    = errors.New("problems found in storage")
 	errRemoveAllNeedsForce  = errors.New("--all requires --force")
 	errSnapshotNotFound     = errors.New("snapshot not found")
 	errSnapshotRemoveFailed = errors.New("snapshot(s) could not be removed")
@@ -612,7 +613,7 @@ func selectSnapshotsToPurge(
 	case opts.OlderThan != "":
 		duration, err := parseDuration(opts.OlderThan)
 		if err != nil {
-			return nil, fmt.Errorf("invalid duration: %w", err)
+			return nil, err
 		}
 
 		cutoff := time.Now().UTC().Add(-duration)
@@ -941,8 +942,10 @@ func (v *Vaultik) formatVerifyResult(
 		v.printVerifySummary(result, failure)
 	}
 
+	// Not errSnapshotVerifyFailed: the cli layer prints this after its own
+	// "Verification failed:".
 	if failure != "" {
-		return fmt.Errorf("%w: %s", errSnapshotVerifyFailed, failure)
+		return fmt.Errorf("%w: %s", errProblemsInStorage, failure)
 	}
 
 	return nil

internal/vaultik/verify.go

@@ -680,7 +680,7 @@ func (v *Vaultik) verifyManifestAgainstDatabase(
 func (v *Vaultik) verifyBlobExistenceFromDB(
 	blobs []snapshot.BlobInfo, result *VerifyResult,
 ) error {
-	log.Info("Verifying blob existence in S3", "blob_count", len(blobs))
+	log.Info("Verifying blob existence in storage", "blob_count", len(blobs))
 
 	for i, blob := range blobs {
 		// The hash is read from the snapshot database, which is not
@@ -700,7 +700,7 @@ func (v *Vaultik) verifyBlobExistenceFromDB(
 			result.Missing++
 			result.MissingSize += blob.CompressedSize
 		case err != nil:
-			return fmt.Errorf("checking blob %s in storage: %w",
+			return fmt.Errorf("could not check blob %s in storage: %w",
 				blob.Hash, err)
 		case stat.Size != blob.CompressedSize:
 			log.Warn("Blob has the wrong size in storage",