README.md

@@ -40,7 +40,7 @@ tree by URL:
 bin/mfer gen .
 
 # Verify the files on disk against the manifest. Exits nonzero if any file
-# is missing or corrupted.
+# it lists is missing or corrupted; warns about files it does not list.
 bin/mfer check index.mf
 
 # Download and cryptographically verify a tree published over HTTP: mfer
@@ -251,6 +251,9 @@ are now tracked only in the [issues](https://git.eeqj.de/sneak/mfer/issues).
 - `mfer check` / `mfer check .`
     - verifies checksums of all files in manifest, displaying error and exiting
       nonzero if any files are missing or corrupted
+    - warns about each file under the base directory that the manifest does not
+      list, hidden files included; with `--no-extra-files` each one is a failure
+      instead
 - `mfer fetch https://example.com/stuff/`
     - fetches `/stuff/index.mf` and downloads all files listed in manifest,
       optionally resuming any that already exist locally, and assures

internal/cli/check.go

@@ -206,16 +206,21 @@ func countCheckFailures(
 }
 
 // findExtraFiles reports files present on disk but absent from the
-// manifest, counting each as a failure.
+// manifest, and anything the search cannot read: each is a failure under
+// --no-extra-files, otherwise a warning.
 func findExtraFiles(ctx *cli.Context, chk *mfer.Checker, failures *int64) error {
 	extraResults := make(chan mfer.Result, 1)
 	extraDone := make(chan struct{})
 
 	go func() {
 		for result := range extraResults {
-			*failures++
+			if ctx.Bool("no-extra-files") {
+				*failures++
 
-			log.Infof("%s: %s (%s)", result.Status, result.Path, result.Message)
+				log.Infof("%s: %s (%s)", result.Status, result.Path, result.Message)
+			} else {
+				log.Warnf("%s: %s (%s)", result.Status, result.Path, result.Message)
+			}
 		}
 
 		close(extraDone)
@@ -270,12 +275,9 @@ func runCheck(ctx *cli.Context, chk *mfer.Checker, showProgress bool) (int64, er
 	// Wait for results processing to complete
 	<-done
 
-	// Check for extra files if requested
-	if ctx.Bool("no-extra-files") {
-		err = findExtraFiles(ctx, chk, &failures)
-		if err != nil {
-			return 0, err
-		}
+	err = findExtraFiles(ctx, chk, &failures)
+	if err != nil {
+		return 0, err
 	}
 
 	return failures, nil

internal/cli/entry_test.go

@@ -705,30 +705,155 @@ func TestNoExtraFilesWithSubdirectory(t *testing.T) {
 		"check should fail when extra files exist in subdirectory")
 }
 
-func TestCheckWithoutNoExtraFilesIgnoresExtra(t *testing.T) {
+// TestCheckWarnsAboutUnlistedFiles adds one file the manifest does not list
+// to a tree that had none: it gets one warning and the check passes, unless
+// --no-extra-files makes it a failure. --quiet hides the warning, not the
+// failure.
+func TestCheckWarnsAboutUnlistedFiles(t *testing.T) {
 	t.Parallel()
 
-	fs := afero.NewMemMapFs()
+	for name, unlisted := range map[string]string{
+		"regular file":             "extra.txt",
+		"dotfile":                  ".hidden",
+		"file in hidden directory": ".git/config",
+	} {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
 
-	// Create test file
-	require.NoError(t, fs.MkdirAll(testDir, 0o755))
-	writeTestFile(t, fs, testFile1, "hello")
+			fs := afero.NewMemMapFs()
+			require.NoError(t, fs.MkdirAll(testDir, 0o755))
+			writeTestFile(t, fs, testFile1, "hello")
 
-	// Generate manifest
-	opts := testOpts([]string{testApp, cmdGenerate, "-q", "-o", testManifest, testDir}, fs)
-	exitCode := runCLI(opts)
-	require.Equal(t, 0, exitCode)
+			opts := testOpts([]string{
+				testApp, cmdGenerate, "-q", "-o", testManifest, testDir,
+			}, fs)
+			require.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts))
 
-	// Add extra file
-	writeTestFile(t, fs, "/testdir/extra.txt", "extra")
+			check := func(flags ...string) (int, string) {
+				args := append([]string{testApp, cmdCheck, testFlagBase, testDir}, flags...)
+				opts := testOpts(append(args, testManifest), fs)
+
+				return runCLI(opts), testStderr(t, opts)
+			}
+
+			exitCode, stderr := check()
+			assert.Equal(t, 0, exitCode, "stderr: %s", stderr)
+			assert.NotContains(t, stderr, "not in manifest")
+
+			writeTestFile(t, fs, filepath.Join(testDir, unlisted), "unlisted")
+
+			exitCode, stderr = check()
+			assert.Equal(t, 0, exitCode, "stderr: %s", stderr)
+			assert.Equal(t, 1, strings.Count(stderr, "not in manifest"), stderr)
+			assert.Contains(t, stderr, unlisted)
+
+			exitCode, stderr = check("-q")
+			assert.Equal(t, 0, exitCode, "stderr: %s", stderr)
+			assert.NotContains(t, stderr, "not in manifest")
+
+			exitCode, stderr = check(testFlagNoExtra)
+			assert.Equal(t, 1, exitCode, "stderr: %s", stderr)
+			assert.Contains(t, stderr, unlisted)
+
+			exitCode, _ = check("-q", testFlagNoExtra)
+			assert.Equal(t, 1, exitCode)
+		})
+	}
+}
+
+// TestCheckNeverReportsManifest keeps the manifest inside the checked tree,
+// under several names and path spellings, and names the tree both directly
+// and through a symlink: the manifest is never reported, even under
+// --no-extra-files. The manifest is recognized by file identity, which needs
+// the real filesystem.
+func TestCheckNeverReportsManifest(t *testing.T) {
+	t.Parallel()
+
+	// Manifest paths are relative to the checked tree.
+	for name, manifest := range map[string]string{
+		"default name":                 defaultManifestName,
+		"hidden name":                  ".index.mf",
+		"other name in a subdirectory": "sub/listing.mf",
+		"path spelled through ..":      "sub/../index.mf",
+	} {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			// A temp dir holding data/tree and link, a symlink to data.
+			root := t.TempDir()
+			tree := filepath.Join(root, "data", "tree")
+			// Not filepath.Join, which would clean away a "..".
+			manifestPath := tree + "/" + manifest
+
+			fs := afero.NewOsFs()
+			require.NoError(t, fs.MkdirAll(filepath.Join(tree, "sub"), 0o750))
+			require.NoError(t,
+				os.Symlink(filepath.Join(root, "data"), filepath.Join(root, "link")))
+			writeTestFile(t, fs, filepath.Join(tree, testFileTxt), "hello")
+
+			opts := testOpts([]string{
+				testApp, cmdGenerate, "-q", "-o", manifestPath, tree,
+			}, fs)
+			require.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts))
+
+			for _, base := range []string{tree, filepath.Join(root, "link", "tree")} {
+				opts = testOpts([]string{
+					testApp, cmdCheck, testFlagNoExtra, testFlagBase, base, manifestPath,
+				}, fs)
+				assert.Equal(t, 0, runCLI(opts),
+					"base %s, stderr: %s", base, testStderr(t, opts))
+				assert.NotContains(t, testStderr(t, opts), "not in manifest")
+			}
+		})
+	}
+}
+
+// unlistableDirFs is a filesystem on which one directory cannot be listed.
+type unlistableDirFs struct {
+	afero.Fs
+
+	dir string
+}
+
+//nolint:ireturn // Open must return afero.File to satisfy afero.Fs.
+func (f unlistableDirFs) Open(name string) (afero.File, error) {
+	if name == f.dir {
+		return nil, os.ErrPermission
+	}
+
+	return f.Fs.Open(name)
+}
+
+// TestCheckWithUnlistableDirectory has a directory under the base that
+// cannot be listed, followed by an unlisted file: both are warned about and
+// the check still passes, unless --no-extra-files is given.
+func TestCheckWithUnlistableDirectory(t *testing.T) {
+	t.Parallel()
+
+	mem := afero.NewMemMapFs()
+	require.NoError(t, mem.MkdirAll("/testdir/locked", 0o755))
+	writeTestFile(t, mem, testFile1, "hello")
+
+	opts := testOpts([]string{
+		testApp, cmdGenerate, "-q", "-o", testManifest, testDir,
+	}, mem)
+	require.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts))
+
+	// Directories are searched in name order, so this comes after "locked".
+	writeTestFile(t, mem, "/testdir/unlisted.txt", "unlisted")
+
+	fs := unlistableDirFs{Fs: mem, dir: "/testdir/locked"}
+
+	opts = testOpts([]string{testApp, cmdCheck, testFlagBase, testDir, testManifest}, fs)
+	assert.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts))
+	assert.Contains(t, testStderr(t, opts), os.ErrPermission.Error())
+	assert.Contains(t, testStderr(t, opts), "unlisted.txt")
 
-	// Check WITHOUT --no-extra-files (should pass - extra files ignored)
 	opts = testOpts([]string{
-		testApp, cmdCheck, "-q", testFlagBase, testDir, testManifest,
+		testApp, cmdCheck, testFlagNoExtra, testFlagBase, testDir, testManifest,
 	}, fs)
-	exitCode = runCLI(opts)
-	assert.Equal(t, 0, exitCode,
-		"check without --no-extra-files should ignore extra files")
+	assert.Equal(t, 1, runCLI(opts), "stderr: %s", testStderr(t, opts))
+	assert.Contains(t, testStderr(t, opts), "unlisted.txt")
 }
 
 func TestGenerateAtomicWriteNoTempFileOnSuccess(t *testing.T) {

internal/cli/mfer.go

@@ -227,7 +227,7 @@ func (mfa *CLIApp) checkCommand() *cli.Command {
 			},
 			&cli.BoolFlag{
 				Name:  "no-extra-files",
-				Usage: "Fail if files exist in base directory that are not in manifest",
+				Usage: "Fail, instead of warning, if files in base directory are not in manifest",
 			},
 			&cli.StringFlag{
 				Name:    "require-signature",

mfer/checker.go

@@ -77,9 +77,9 @@ type Checker struct {
 	fs       afero.Fs
 	// manifestPaths is a set of paths in the manifest for quick lookup
 	manifestPaths map[RelFilePath]struct{}
-	// manifestRelPath is the relative path of the manifest file from
-	// basePath (for exclusion)
-	manifestRelPath RelFilePath
+	// manifestInfo is the manifest file, which FindExtraFiles leaves out,
+	// matched with os.SameFile.
+	manifestInfo os.FileInfo
 	// signature info from the manifest
 	signature     []byte
 	signer        []byte
@@ -133,26 +133,20 @@ func NewChecker(opts *CheckerOptions) (*Checker, error) {
 		manifestPaths[RelFilePath(f.GetPath())] = struct{}{}
 	}
 
-	// Compute manifest's relative path from basePath for exclusion in FindExtraFiles
-	absManifest, err := filepath.Abs(opts.ManifestPath)
+	manifestInfo, err := fs.Stat(opts.ManifestPath)
 	if err != nil {
 		return nil, err
 	}
 
-	manifestRel, err := filepath.Rel(abs, absManifest)
-	if err != nil {
-		manifestRel = ""
-	}
-
 	return &Checker{
-		basePath:        AbsFilePath(abs),
-		files:           files,
-		fs:              fs,
-		manifestPaths:   manifestPaths,
-		manifestRelPath: RelFilePath(manifestRel),
-		signature:       m.pbOuter.GetSignature(),
-		signer:          m.pbOuter.GetSigner(),
-		signingPubKey:   m.pbOuter.GetSigningPubKey(),
+		basePath:      AbsFilePath(abs),
+		files:         files,
+		fs:            fs,
+		manifestPaths: manifestPaths,
+		manifestInfo:  manifestInfo,
+		signature:     m.pbOuter.GetSignature(),
+		signer:        m.pbOuter.GetSigner(),
+		signingPubKey: m.pbOuter.GetSigningPubKey(),
 	}, nil
 }
 
@@ -273,20 +267,27 @@ func (c *Checker) Check(
 	return nil
 }
 
-// FindExtraFiles walks the filesystem and reports files not in the manifest.
-// Results are sent to the results channel. The channel is closed when done.
-// Hidden files/directories (starting with .) are skipped, as they are excluded
-// from manifests by default. The manifest file itself is also skipped.
+// FindExtraFiles walks the filesystem and reports files not in the manifest,
+// hidden files and directories included. The manifest file itself is not
+// reported. Anything the search cannot read, such as a directory that cannot
+// be listed, is reported with StatusError and the search goes on. Results are
+// sent to the results channel. The channel is closed when done.
 func (c *Checker) FindExtraFiles(ctx context.Context, results chan<- Result) error {
 	if results != nil {
 		defer close(results)
 	}
 
-	walkFn := func(walkPath string, info os.FileInfo, err error) error {
-		if err != nil {
-			return err
-		}
+	// The search does not follow symlinks, so a base directory named
+	// through one is resolved first. If that fails, the base is searched as
+	// named and the search reports the problem.
+	root := string(c.basePath)
 
+	resolved, err := filepath.EvalSymlinks(root)
+	if err == nil {
+		root = resolved
+	}
+
+	walkFn := func(walkPath string, info os.FileInfo, walkErr error) error {
 		select {
 		case <-ctx.Done():
 			return ctx.Err()
@@ -294,15 +295,22 @@ func (c *Checker) FindExtraFiles(ctx context.Context, results chan<- Result) err
 		}
 
 		// Get relative path
-		rel, err := filepath.Rel(string(c.basePath), walkPath)
+		rel, err := filepath.Rel(root, walkPath)
 		if err != nil {
 			return err
 		}
 
-		// Skip hidden files and directories (dotfiles)
-		if IsHiddenPath(filepath.ToSlash(rel)) {
-			if info.IsDir() {
-				return filepath.SkipDir
+		relPath := RelFilePath(rel)
+
+		// Report what cannot be read, such as a directory that cannot be
+		// listed, and go on with the rest.
+		if walkErr != nil {
+			if results != nil {
+				results <- Result{
+					Path:    relPath,
+					Status:  StatusError,
+					Message: walkErr.Error(),
+				}
 			}
 
 			return nil
@@ -313,10 +321,17 @@ func (c *Checker) FindExtraFiles(ctx context.Context, results chan<- Result) err
 			return nil
 		}
 
-		relPath := RelFilePath(rel)
+		// A symlink is compared by what it points to, so a manifest reached
+		// through one is not reported either.
+		if info.Mode()&os.ModeSymlink != 0 {
+			target, statErr := c.fs.Stat(walkPath)
+			if statErr == nil {
+				info = target
+			}
+		}
 
-		// Skip the manifest file itself
-		if relPath == c.manifestRelPath {
+		// Skip the manifest file itself, however its path is spelled
+		if os.SameFile(info, c.manifestInfo) {
 			return nil
 		}
 
@@ -334,7 +349,7 @@ func (c *Checker) FindExtraFiles(ctx context.Context, results chan<- Result) err
 		return nil
 	}
 
-	return afero.Walk(c.fs, string(c.basePath), walkFn)
+	return afero.Walk(c.fs, root, walkFn)
 }
 
 func (c *Checker) checkFile(entry *MFFilePath, checkedBytes *FileSize) Result {

mfer/checker_test.go

@@ -21,8 +21,6 @@ const (
 	testExistsFile   = "exists.txt"
 	testManifestPath = "/manifest.mf"
 	testDataDir      = "/data"
-	// testDataManifestPath is a manifest kept inside the checked tree.
-	testDataManifestPath = testDataDir + "/index.mf"
 )
 
 func TestStatusString(t *testing.T) {
@@ -459,50 +457,120 @@ func TestFindExtraFiles(t *testing.T) {
 	assert.Equal(t, "not in manifest", extras[0].Message)
 }
 
-func TestFindExtraFilesSkipsManifestAndDotfiles(t *testing.T) {
+// TestFindExtraFilesReportsHiddenFilesButNotManifest keeps the manifest
+// inside the checked tree: hidden files and directories are reported, the
+// manifest is not. The manifest is recognized by file identity, which needs
+// the real filesystem.
+func TestFindExtraFilesReportsHiddenFilesButNotManifest(t *testing.T) {
 	t.Parallel()
 
-	fs := afero.NewMemMapFs()
-	manifestFiles := map[string][]byte{
-		testFile1: []byte("in manifest"),
-	}
-	createTestManifest(t, fs, testDataManifestPath, manifestFiles)
-	createFilesOnDisk(t, fs, map[string][]byte{
+	dir := t.TempDir()
+	manifestPath := filepath.Join(dir, "index.mf")
+
+	fs := afero.NewOsFs()
+	createTestManifest(t, fs, manifestPath, map[string][]byte{
 		testFile1: []byte("in manifest"),
 	})
-	// Create dotfile and manifest that should be skipped
-	require.NoError(t, afero.WriteFile(fs, "/data/.hidden", []byte("hidden"), 0o644))
-	require.NoError(t, afero.WriteFile(fs, "/data/.config/settings", []byte("cfg"), 0o644))
-	// Create a real extra file
-	require.NoError(t, fs.MkdirAll(testDataDir, 0o755))
-	require.NoError(t, afero.WriteFile(fs, "/data/extra.txt", []byte("extra"), 0o644))
+
+	unlisted := []RelFilePath{"extra.txt", ".hidden", ".git/config"}
+	for _, p := range append([]RelFilePath{testFile1}, unlisted...) {
+		path := filepath.Join(dir, string(p))
+		require.NoError(t, fs.MkdirAll(filepath.Dir(path), 0o750))
+		require.NoError(t, afero.WriteFile(fs, path, []byte("x"), 0o600))
+	}
 
 	chk, err := NewChecker(&CheckerOptions{
-		ManifestPath: testDataManifestPath,
-		BasePath:     testDataDir,
+		ManifestPath: manifestPath,
+		BasePath:     dir,
 		Fs:           fs,
 	})
 	require.NoError(t, err)
 
 	results := make(chan Result, 10)
-	err = chk.FindExtraFiles(context.Background(), results)
+	require.NoError(t, chk.FindExtraFiles(context.Background(), results))
+
+	var extras []RelFilePath
+	for r := range results {
+		extras = append(extras, r.Path)
+	}
+
+	assert.ElementsMatch(t, unlisted, extras)
+}
+
+// TestFindExtraFilesSkipsManifestReachedThroughSymlink checks a tree whose
+// index.mf is a symlink to the manifest kept outside the tree: the symlink is
+// not reported.
+func TestFindExtraFilesSkipsManifestReachedThroughSymlink(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+	tree := filepath.Join(dir, "tree")
+	manifestPath := filepath.Join(dir, "real.mf")
+	linkPath := filepath.Join(tree, "index.mf")
+
+	fs := afero.NewOsFs()
+	createTestManifest(t, fs, manifestPath, map[string][]byte{testFile1: []byte("x")})
+	require.NoError(t, fs.MkdirAll(tree, 0o750))
+	require.NoError(t,
+		afero.WriteFile(fs, filepath.Join(tree, testFile1), []byte("x"), 0o600))
+	require.NoError(t, os.Symlink(manifestPath, linkPath))
+
+	chk, err := NewChecker(&CheckerOptions{
+		ManifestPath: linkPath,
+		BasePath:     tree,
+		Fs:           fs,
+	})
 	require.NoError(t, err)
 
-	var extras []Result
+	results := make(chan Result, 10)
+	require.NoError(t, chk.FindExtraFiles(context.Background(), results))
+
+	var extras []RelFilePath
 	for r := range results {
-		extras = append(extras, r)
+		extras = append(extras, r.Path)
 	}
 
-	// Should only report extra.txt, not .hidden, .config/settings, or index.mf
-	for _, e := range extras {
-		t.Logf("extra: %s", e.Path)
+	assert.Empty(t, extras)
+}
+
+// TestFindExtraFilesSearchesBaseNamedThroughSymlink names the checked tree
+// through a symlink to it: the files in the tree are searched, and the
+// symlink itself is not reported.
+func TestFindExtraFilesSearchesBaseNamedThroughSymlink(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+	tree := filepath.Join(dir, "tree")
+	link := filepath.Join(dir, "link")
+	manifestPath := filepath.Join(dir, "index.mf")
+
+	fs := afero.NewOsFs()
+	createTestManifest(t, fs, manifestPath, map[string][]byte{testFile1: []byte("x")})
+	require.NoError(t, fs.MkdirAll(tree, 0o750))
+
+	for _, name := range []string{testFile1, testFile2} {
+		require.NoError(t,
+			afero.WriteFile(fs, filepath.Join(tree, name), []byte("x"), 0o600))
 	}
 
-	assert.Len(t, extras, 1)
+	require.NoError(t, os.Symlink(tree, link))
 
-	if len(extras) > 0 {
-		assert.Equal(t, RelFilePath("extra.txt"), extras[0].Path)
+	chk, err := NewChecker(&CheckerOptions{
+		ManifestPath: manifestPath,
+		BasePath:     link,
+		Fs:           fs,
+	})
+	require.NoError(t, err)
+
+	results := make(chan Result, 10)
+	require.NoError(t, chk.FindExtraFiles(context.Background(), results))
+
+	var extras []RelFilePath
+	for r := range results {
+		extras = append(extras, r.Path)
 	}
+
+	assert.Equal(t, []RelFilePath{testFile2}, extras)
 }
 
 func TestFindExtraFilesContextCancellation(t *testing.T) {
@@ -649,77 +717,6 @@ func TestCheckMissingFileDetectedWithoutFallback(t *testing.T) {
 	assert.Equal(t, 0, statusCounts[StatusError], "no files should be ERROR")
 }
 
-func TestFindExtraFilesSkipsDotfiles(t *testing.T) {
-	t.Parallel()
-
-	// Regression test for #16: FindExtraFiles should not report dotfiles
-	// or the manifest file itself as extra files.
-	fs := afero.NewMemMapFs()
-	files := map[string][]byte{
-		testFile1: []byte("in manifest"),
-	}
-	createTestManifest(t, fs, testDataManifestPath, files)
-	createFilesOnDisk(t, fs, files)
-
-	// Add dotfiles and manifest file on disk
-	require.NoError(t, afero.WriteFile(fs, "/data/.hidden", []byte("dotfile"), 0o644))
-	require.NoError(t, fs.MkdirAll("/data/.git", 0o755))
-	require.NoError(t,
-		afero.WriteFile(fs, "/data/.git/config", []byte("git config"), 0o644))
-
-	chk, err := NewChecker(&CheckerOptions{
-		ManifestPath: testDataManifestPath,
-		BasePath:     testDataDir,
-		Fs:           fs,
-	})
-	require.NoError(t, err)
-
-	results := make(chan Result, 10)
-	err = chk.FindExtraFiles(context.Background(), results)
-	require.NoError(t, err)
-
-	var extras []Result
-	for r := range results {
-		extras = append(extras, r)
-	}
-
-	// Should report NO extra files — dotfiles and manifest should be skipped
-	assert.Empty(t, extras,
-		"FindExtraFiles should not report dotfiles or manifest file as extra; got: %v",
-		extras)
-}
-
-func TestFindExtraFilesSkipsManifestFile(t *testing.T) {
-	t.Parallel()
-
-	// The manifest file itself should never be reported as extra
-	fs := afero.NewMemMapFs()
-	files := map[string][]byte{
-		testFile1: []byte("content"),
-	}
-	createTestManifest(t, fs, testDataManifestPath, files)
-	createFilesOnDisk(t, fs, files)
-
-	chk, err := NewChecker(&CheckerOptions{
-		ManifestPath: testDataManifestPath,
-		BasePath:     testDataDir,
-		Fs:           fs,
-	})
-	require.NoError(t, err)
-
-	results := make(chan Result, 10)
-	err = chk.FindExtraFiles(context.Background(), results)
-	require.NoError(t, err)
-
-	var extras []Result
-	for r := range results {
-		extras = append(extras, r)
-	}
-
-	assert.Empty(t, extras,
-		"manifest file should not be reported as extra; got: %v", extras)
-}
-
 func TestCheckEmptyManifest(t *testing.T) {
 	t.Parallel()