From 32846e2c078ed99fb3e488c7765521d1380672b5 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 13 Oct 2026 12:59:13 +0000 Subject: [PATCH] Read only regular files in gen, freshen and check (closes #189) Opening a named pipe that has no writer blocks, so gen and freshen hung on a tree holding one, and check hung on a listed path that is a pipe on disk. gen and freshen now list regular files only (with --follow-symlinks, what a symlink points to must be one) and name what they skip with -v. check reports a listed path that is not a regular file as an error without opening it, and its search for extra files leaves out pipes, sockets and devices. Judgement call: the pipe tests build on unix only, where syscall.Mkfifo exists. Unverified: sockets and devices are not tested; the tests use pipes. Model: opus-5-5 --- README.md | 7 ++- internal/cli/freshen.go | 7 +++ internal/cli/pipe_test.go | 94 +++++++++++++++++++++++++++++ mfer/checker.go | 14 +++++ mfer/pipe_test.go | 123 ++++++++++++++++++++++++++++++++++++++ mfer/scanner.go | 7 +++ 6 files changed, 250 insertions(+), 2 deletions(-) create mode 100644 internal/cli/pipe_test.go create mode 100644 mfer/pipe_test.go diff --git a/README.md b/README.md index 9ba95de..fdc96a7 100644 --- a/README.md +++ b/README.md @@ -268,6 +268,8 @@ are now tracked only in the [issues](https://git.eeqj.de/sneak/mfer/issues). - `mfer gen` / `mfer gen .` - recurses under current directory and writes out an `index.mf` + - lists regular files only, and skips a named pipe, socket or device, which + it names with `-v` - records every file's mode as `0000` unless given `--include-permissions`, which records each file's permission bits (`0777` at most) - `mfer gen /media/drive` @@ -294,9 +296,10 @@ are now tracked only in the [issues](https://git.eeqj.de/sneak/mfer/issues). - rewrites `index.mf` to list the files now under the directory holding it, or under the one `--base` names, hashing only the files that are new or changed + - lists regular files only, as gen does - leaves out hidden files unless given `--include-dotfiles`, and symlinks - unless given `--follow-symlinks`, which lists each symlink to a file under - its own name with the contents of the file it points to + unless given `--follow-symlinks`, which lists each symlink to a regular + file under its own name with the contents of the file it points to - `mfer gen --sign-key key.asc` / `mfer freshen --sign-key key.asc` - signs the manifest with the OpenPGP secret key in `key.asc`, armored or binary, as `gpg --export-secret-keys` writes it; `MFER_SIGN_KEY` names the diff --git a/internal/cli/freshen.go b/internal/cli/freshen.go index d3f25ae..4a9b246 100644 --- a/internal/cli/freshen.go +++ b/internal/cli/freshen.go @@ -192,6 +192,13 @@ func (s *freshenScanner) walk(path string, info fs.FileInfo, walkErr error) erro info = realInfo } + // Opening a named pipe can block, and reading a device may never end. + if !info.Mode().IsRegular() { + log.Verbosef("skipping %s: not a regular file", relPath) + + return nil + } + for _, excluded := range s.excluded { if os.SameFile(info, excluded) { return nil diff --git a/internal/cli/pipe_test.go b/internal/cli/pipe_test.go new file mode 100644 index 0000000..47953ab --- /dev/null +++ b/internal/cli/pipe_test.go @@ -0,0 +1,94 @@ +//go:build unix + +//nolint:testpackage // white-box tests exercise unexported internals +package cli + +import ( + "os" + "path/filepath" + "syscall" + "testing" + "time" + + "github.com/spf13/afero" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + // testPipe is the name of the named pipe the pipe tests make in a tree. + testPipe = "pipe" + + // pipeDeadline is how long a test lets a run go before it opens the + // test's named pipes for writing. + pipeDeadline = 5 * time.Second +) + +// makePipe makes a named pipe at path. Opening a named pipe for reading +// blocks until it is opened for writing, so once pipeDeadline has passed, +// unless the test has ended, makePipe opens it for writing and closes it +// again. A run that wrongly reads the pipe then ends, and the test fails on +// what it reports instead of hanging. That lets one read through, so each +// path a wrong run would read gets a pipe of its own. +func makePipe(t *testing.T, path string) { + t.Helper() + + require.NoError(t, syscall.Mkfifo(path, 0o600)) + + timer := time.AfterFunc(pipeDeadline, func() { + f, err := os.OpenFile(path, os.O_WRONLY, 0) //nolint:gosec // test-controlled path + if err == nil { + _ = f.Close() + } + }) + + t.Cleanup(func() { timer.Stop() }) +} + +// TestGenerateSkipsPipe runs gen -v on a tree holding a named pipe: the +// manifest lists the regular file only, and gen names the pipe it skipped. +func TestGenerateSkipsPipe(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + fs := afero.NewOsFs() + writeTestFile(t, fs, filepath.Join(dir, testFileTxt), "regular") + makePipe(t, filepath.Join(dir, testPipe)) + + opts := testOpts([]string{testApp, cmdGenerate, "-v", dir}, fs) + require.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts)) + + assert.Equal(t, []string{testFileTxt}, + manifestPaths(t, fs, filepath.Join(dir, defaultManifestName))) + assert.Contains(t, testStderr(t, opts), "skipping pipe: not a regular file") +} + +// TestFreshenSkipsPipes adds a named pipe to a tree after gen made its +// manifest, and a symlink to another pipe. freshen -v --follow-symlinks +// leaves the manifest listing the regular file only, and names both entries +// it skipped. +func TestFreshenSkipsPipes(t *testing.T) { + t.Parallel() + + fs := afero.NewOsFs() + root, manifestPath := setupFreshenDir(t, fs, + map[string]string{testFileTxt: "regular"}) + makePipe(t, filepath.Join(root, testPipe)) + + // The symlink points outside the tree, so no pipe is read twice. + target := filepath.Join(t.TempDir(), testPipe) + makePipe(t, target) + require.NoError(t, os.Symlink(target, filepath.Join(root, testLink))) + + opts := testOpts([]string{ + testApp, cmdFreshen, "-v", "--follow-symlinks", + testFlagBase, root, manifestPath, + }, fs) + require.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts)) + + assert.Equal(t, []string{testFileTxt}, manifestPaths(t, fs, manifestPath)) + + stderr := testStderr(t, opts) + assert.Contains(t, stderr, "skipping pipe: not a regular file") + assert.Contains(t, stderr, "skipping link: not a regular file") +} diff --git a/mfer/checker.go b/mfer/checker.go index 3077303..8cf79f9 100644 --- a/mfer/checker.go +++ b/mfer/checker.go @@ -318,6 +318,11 @@ func (c *Checker) FindExtraFiles(ctx context.Context, results chan<- Result) err return nil } + // gen never lists a named pipe, a socket or a device. + if info.Mode()&(os.ModeNamedPipe|os.ModeSocket|os.ModeDevice) != 0 { + return nil + } + // 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 { @@ -371,6 +376,15 @@ func (c *Checker) checkFile(entry *MFFilePath, checkedBytes *FileSize) Result { return Result{Path: relPath, Status: StatusError, Message: err.Error()} } + // Opening a named pipe can block, and reading a device may never end. + if !info.Mode().IsRegular() { + return Result{ + Path: relPath, + Status: StatusError, + Message: "not a regular file", + } + } + // Check size if info.Size() != entry.GetSize() { *checkedBytes += FileSize(info.Size()) diff --git a/mfer/pipe_test.go b/mfer/pipe_test.go new file mode 100644 index 0000000..69b8abc --- /dev/null +++ b/mfer/pipe_test.go @@ -0,0 +1,123 @@ +//go:build unix + +//nolint:testpackage // white-box tests exercise unexported internals +package mfer + +import ( + "bytes" + "context" + "os" + "path/filepath" + "syscall" + "testing" + "time" + + "github.com/spf13/afero" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// pipeDeadline is how long a test lets a run go before it opens the test's +// named pipes for writing. +const pipeDeadline = 5 * time.Second + +// makePipe makes a named pipe at path. Opening a named pipe for reading +// blocks until it is opened for writing, so once pipeDeadline has passed, +// unless the test has ended, makePipe opens it for writing and closes it +// again. A run that wrongly reads the pipe then ends, and the test fails on +// what it reports instead of hanging. That lets one read through, so each +// path a wrong run would read gets a pipe of its own. +func makePipe(t *testing.T, path string) { + t.Helper() + + require.NoError(t, syscall.Mkfifo(path, 0o600)) + + timer := time.AfterFunc(pipeDeadline, func() { + f, err := os.OpenFile(path, os.O_WRONLY, 0) //nolint:gosec // test-controlled path + if err == nil { + _ = f.Close() + } + }) + + t.Cleanup(func() { timer.Stop() }) +} + +// TestScannerSkipsPipes scans a tree holding a named pipe and a symlink to +// another one, following symlinks: the manifest lists the regular file only. +func TestScannerSkipsPipes(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, testFile1), []byte("a"), 0o600)) + makePipe(t, filepath.Join(dir, "pipe")) + + // The symlink points outside the tree, so no pipe is read twice. + target := filepath.Join(t.TempDir(), "pipe") + makePipe(t, target) + require.NoError(t, os.Symlink(target, filepath.Join(dir, "link"))) + + s := NewScannerWithOptions(&ScannerOptions{FollowSymLinks: true}) + require.NoError(t, s.EnumeratePath(dir, nil)) + + var buf bytes.Buffer + require.NoError(t, s.ToManifest(context.Background(), &buf, nil)) + + m, err := NewManifestFromReader(&buf) + require.NoError(t, err) + require.Len(t, m.Files(), 1) + assert.Equal(t, testFile1, m.Files()[0].GetPath()) +} + +// TestCheckPipes checks a tree against a manifest that lists, as an empty +// file, a path that is a named pipe on disk. The tree holds another pipe the +// manifest does not list. check reports the listed pipe as an error without +// reading it, and the search for extra files does not report the other. +func TestCheckPipes(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + manifestPath := filepath.Join(dir, "index.mf") + content := []byte("in manifest") + listedPipe := RelFilePath("listed-pipe") + + fs := afero.NewOsFs() + createTestManifest(t, fs, manifestPath, map[string][]byte{ + testFile1: content, + string(listedPipe): {}, + }) + require.NoError(t, afero.WriteFile(fs, filepath.Join(dir, testFile1), content, 0o600)) + makePipe(t, filepath.Join(dir, string(listedPipe))) + makePipe(t, filepath.Join(dir, "unlisted-pipe")) + + chk, err := NewChecker(&CheckerOptions{ + ManifestPath: manifestPath, + BasePath: dir, + Fs: fs, + }) + require.NoError(t, err) + + results := make(chan Result, 10) + require.NoError(t, chk.Check(context.Background(), results, nil)) + + byPath := map[RelFilePath]Result{} + for r := range results { + byPath[r.Path] = r + } + + assert.Equal(t, StatusOK, byPath[testFile1].Status) + assert.Equal(t, Result{ + Path: listedPipe, + Status: StatusError, + Message: "not a regular file", + }, byPath[listedPipe]) + + extras := make(chan Result, 10) + require.NoError(t, chk.FindExtraFiles(context.Background(), extras)) + + var extraPaths []RelFilePath + for r := range extras { + extraPaths = append(extraPaths, r.Path) + } + + assert.Empty(t, extraPaths) +} diff --git a/mfer/scanner.go b/mfer/scanner.go index 2dcfbc2..358ffdb 100644 --- a/mfer/scanner.go +++ b/mfer/scanner.go @@ -474,6 +474,13 @@ func (s *Scanner) enumerateFileWithInfo( info = realInfo } + // Opening a named pipe can block, and reading a device may never end. + if !info.Mode().IsRegular() { + log.Verbosef("skipping %s: not a regular file", cleanPath) + + return nil + } + for _, excluded := range s.excluded { if os.SameFile(info, excluded) { return nil