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

@@ -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,15 +296,19 @@ 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
       file too. mfer signs it itself and does not need `gpg`. A file holding
       more than one key is refused, and a key held only on a smartcard cannot
-      sign
+      sign. The key must be a version 4 key, whose 40-character fingerprint is
+      what `--require-signature` takes, and not a DSA key. A key that cannot
+      sign, because it has expired or been revoked or its passphrase is wrong,
+      stops `gen` and `freshen` before they read any file
     - takes a protected key's passphrase from `MFER_SIGN_KEY_PASSPHRASE`, or
       else asks for it at the terminal
 - `mfer fetch https://example.com/stuff/`

docs/FORMAT.md

@@ -152,7 +152,10 @@ primary key, `signature` is one good signature over the canonical string made by
 that key (or one of its subkeys), and `signer` is that key's fingerprint. The
 reference implementation refuses to load a manifest that fails these checks;
 `check` and `fetch` given `--require-signature` then compare the required
-fingerprint with `signer`.
+fingerprint with `signer`. It also refuses a manifest whose `signingPubKey`
+holds a DSA key or subkey, or any secret key or subkey, since checking the
+self-signatures of a DSA key or the numbers of a secret key can take hours when
+those numbers are very large.
 
 ## Deterministic Serialization
 

internal/cli/check.go

@@ -21,8 +21,8 @@ import (
 	"sneak.berlin/go/mfer/mfer"
 )
 
-// fingerprintHexLen is the length of a full OpenPGP key fingerprint in hex
-// characters.
+// fingerprintHexLen is the length in hex characters of the fingerprint of
+// an OpenPGP version 4 key, the only version mfer signs with.
 const fingerprintHexLen = 40
 
 var (

internal/cli/entry_test.go

@@ -1368,6 +1368,51 @@ func TestGenerateSeveralPathsDefaultOutput(t *testing.T) {
 	}
 }
 
+// testLink is the name of the symlink to a tree that the
+// DirectoryNamedThroughSymlink tests make in a temp dir.
+const testLink = "link"
+
+// TestGenerateDirectoryNamedThroughSymlink runs gen on a directory named
+// through a symlink, given as the argument or as the working directory: the
+// manifest lists the files in the directory the symlink points to, and
+// leaves out a symlink inside it, as gen does without --follow-symlinks.
+//
+//nolint:paralleltest // changes the process-global working directory
+func TestGenerateDirectoryNamedThroughSymlink(t *testing.T) {
+	// Paths are relative to a temp dir holding data and link, a symlink to
+	// data.
+	for name, tc := range map[string]struct {
+		workDir string
+		args    []string
+	}{
+		"argument":          {".", []string{testLink}},
+		"working directory": {testLink, nil},
+	} {
+		t.Run(name, func(t *testing.T) {
+			root := t.TempDir()
+			data := filepath.Join(root, "data")
+
+			fs := afero.NewOsFs()
+			writeTestTree(t, fs, data)
+			require.NoError(t,
+				os.Symlink(testFileTxt, filepath.Join(data, "alias.txt")))
+			require.NoError(t, os.Symlink(data, filepath.Join(root, testLink)))
+
+			// t.Chdir sets PWD to the path it is given, as a shell does, and
+			// os.Getwd returns PWD when it names the working directory.
+			t.Chdir(filepath.Join(root, tc.workDir))
+
+			opts := testOpts(slices.Concat(
+				[]string{testApp, cmdGenerate, "-q"}, tc.args,
+			), fs)
+			require.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts))
+
+			assert.ElementsMatch(t, []string{testFileTxt, "sub/nested.txt"},
+				manifestPaths(t, fs, filepath.Join(data, defaultManifestName)))
+		})
+	}
+}
+
 // TestGenerateBytesDoNotDependOnOutput runs gen --seed on one tree, each
 // time writing to another file, over a file already there and beside an
 // earlier run's temp file: neither is listed, and what is listed depends
@@ -1619,20 +1664,119 @@ func TestGenerateValidatesInputPaths(t *testing.T) {
 	})
 }
 
-// TestFlagAfterArgumentIsArgument asserts that flags are read only before a
-// command's first argument: after it, -v is a path, not the verbose flag.
-func TestFlagAfterArgumentIsArgument(t *testing.T) {
-	t.Parallel()
+// generateTestDir writes testFile1 and runs gen on testDir, which writes
+// the manifest to testDir/index.mf.
+func generateTestDir(t *testing.T, fs afero.Fs) {
+	t.Helper()
 
-	fs := afero.NewMemMapFs()
 	require.NoError(t, fs.MkdirAll(testDir, 0o755))
 	writeTestFile(t, fs, testFile1, "content")
 
-	opts := testOpts([]string{testApp, cmdGenerate, testDir, "-v"}, fs)
-	exitCode := runCLI(opts)
+	opts := testOpts([]string{testApp, cmdGenerate, "-q", testDir}, fs)
+	require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
+}
 
-	assert.Equal(t, 1, exitCode)
-	assert.Contains(t, testStderr(t, opts), "path does not exist: -v")
+// TestCheckReadsOptionAfterPath runs check on an unsigned manifest with
+// --require-signature after the directory. check must refuse the manifest.
+func TestCheckReadsOptionAfterPath(t *testing.T) {
+	t.Parallel()
+
+	fs := afero.NewMemMapFs()
+	generateTestDir(t, fs)
+
+	opts := testOpts([]string{
+		testApp, cmdCheck, "-q", testDir, "--" + flagRequireSignature, msgFpA,
+	}, fs)
+	assert.Equal(t, 1, runCLI(opts))
+	assert.Contains(t, testStderr(t, opts), "manifest is not signed")
+}
+
+// TestFreshenReadsOptionAfterPath runs freshen with --include-permissions
+// after the directory. freshen must record the file's mode.
+func TestFreshenReadsOptionAfterPath(t *testing.T) {
+	t.Parallel()
+
+	fs := afero.NewOsFs()
+	root, manifestPath := setupFreshenDir(t, fs,
+		map[string]string{testFileTxt: "a file"})
+	require.NoError(t, fs.Chmod(filepath.Join(root, testFileTxt), 0o640))
+
+	opts := testOpts([]string{
+		testApp, cmdFreshen, "-q", root, "--" + flagIncludePermissions,
+	}, fs)
+	require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
+
+	files := manifestFiles(t, fs, manifestPath)
+	require.Len(t, files, 1)
+	assert.Equal(t, uint32(0o640), files[0].GetMode())
+}
+
+// TestFetchReadsOptionAfterURL runs fetch with --dest after the URL. The
+// file and the manifest must land in that directory and nothing in the
+// current one.
+//
+//nolint:paralleltest // changes the process-global working directory
+func TestFetchReadsOptionAfterURL(t *testing.T) {
+	content := []byte("fetched")
+	manifest := manifestOf(t, map[string][]byte{testFileTxt: content})
+
+	server := httptest.NewServer(fetchTestHandler(manifest,
+		map[string][]byte{testFileTxt: content}))
+	defer server.Close()
+
+	cwd := chdirTemp(t)
+	dest := t.TempDir()
+
+	opts := testOpts([]string{
+		testApp, cmdFetch, "-q", server.URL, "--" + flagDest, dest,
+	}, afero.NewOsFs())
+	require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
+
+	assert.Equal(t,
+		map[string][]byte{testFileTxt: content, defaultManifestName: manifest},
+		filesUnder(t, dest))
+	assert.Empty(t, filesUnder(t, cwd), "fetch wrote outside --dest")
+}
+
+// TestOneArgumentCommandsRefuseSecond runs each command that takes one
+// argument with two. It must fail naming the second.
+func TestOneArgumentCommandsRefuseSecond(t *testing.T) {
+	t.Parallel()
+
+	for _, command := range []string{
+		cmdCheck, cmdFreshen, cmdList, cmdExport, cmdFetch,
+	} {
+		t.Run(command, func(t *testing.T) {
+			t.Parallel()
+
+			fs := afero.NewMemMapFs()
+			generateTestDir(t, fs)
+
+			opts := testOpts([]string{testApp, command, testDir, "/second"}, fs)
+			assert.Equal(t, 1, runCLI(opts))
+			assert.Contains(t, testStderr(t, opts),
+				`unexpected argument "/second"`)
+		})
+	}
+}
+
+// TestGenerateReadsPathAfterDoubleDash runs gen -- -x where -x is a
+// directory. -x must be read as the path to list, not as an option.
+//
+//nolint:paralleltest // changes the process-global working directory
+func TestGenerateReadsPathAfterDoubleDash(t *testing.T) {
+	chdirTemp(t)
+
+	fs := afero.NewOsFs()
+	require.NoError(t, fs.Mkdir("-x", 0o755))
+	writeTestFile(t, fs, filepath.Join("-x", testFileTxt), "content")
+
+	opts := testOpts([]string{testApp, cmdGenerate, "-q", "--", "-x"}, fs)
+	require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
+
+	files := manifestFiles(t, fs, filepath.Join("-x", defaultManifestName))
+	require.Len(t, files, 1)
+	assert.Equal(t, testFileTxt, files[0].GetPath())
 }
 
 func TestCheckDetectsManifestCorruption(t *testing.T) {

internal/cli/errmsg_test.go

@@ -128,13 +128,19 @@ func TestSignerMismatchMessage(t *testing.T) {
 
 // testSecretKey returns a new OpenPGP key with its secret key, armored, as
 // gpg --export-secret-keys --armor writes it, and the key's fingerprint.
-// The key is protected by passphrase unless that is nil. It is an Ed25519
-// key, which is quick to make.
-func testSecretKey(t *testing.T, passphrase []byte) ([]byte, string) {
+// The key is protected by passphrase unless that is nil. config sets how
+// the key is made; without one it is an Ed25519 key, which is quick to
+// make.
+func testSecretKey(
+	t *testing.T, passphrase []byte, config *packet.Config,
+) ([]byte, string) {
 	t.Helper()
 
-	key, err := openpgp.NewEntity("MFER Test Key", "", "test@mfer.test",
-		&packet.Config{Algorithm: packet.PubKeyAlgoEdDSA})
+	if config == nil {
+		config = &packet.Config{Algorithm: packet.PubKeyAlgoEdDSA}
+	}
+
+	key, err := openpgp.NewEntity("MFER Test Key", "", "test@mfer.test", config)
 	require.NoError(t, err)
 
 	if passphrase != nil {
@@ -155,7 +161,7 @@ func testSecretKey(t *testing.T, passphrase []byte) ([]byte, string) {
 func signedManifest(t *testing.T, files map[string][]byte) []byte {
 	t.Helper()
 
-	secretKey, _ := testSecretKey(t, nil)
+	secretKey, _ := testSecretKey(t, nil, nil)
 
 	b := mfer.NewBuilder()
 	b.SetSigningOptions(&mfer.SigningOptions{SecretKey: secretKey})

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
@@ -381,6 +388,14 @@ func (mfa *CLIApp) freshenScan(
 		}
 	}
 
+	// The walk does not follow a symlink at its top, so a base directory
+	// named through one is resolved first. If that fails, the base is
+	// walked as named and the walk reports the problem.
+	resolved, err := filepath.EvalSymlinks(absBase)
+	if err == nil {
+		absBase = resolved
+	}
+
 	scanner := &freshenScanner{
 		fs:                 mfa.Fs,
 		absBase:            absBase,
@@ -392,7 +407,7 @@ func (mfa *CLIApp) freshenScan(
 		existingByPath:     existingByPath,
 	}
 
-	err := afero.Walk(mfa.Fs, absBase, scanner.walk)
+	err = afero.Walk(mfa.Fs, absBase, scanner.walk)
 
 	if showProgress {
 		log.ProgressDone()

internal/cli/freshen_test.go

@@ -336,6 +336,49 @@ func TestFreshenResolvesEntriesAgainstManifestDirectory(t *testing.T) {
 	}
 }
 
+// TestFreshenDirectoryNamedThroughSymlink adds a file to a tree after gen
+// made its manifest, then freshens it with the tree named through a symlink,
+// given as the argument or as the working directory: the manifest lists the
+// files in the tree, and leaves out a symlink inside it, as freshen does
+// without --follow-symlinks.
+//
+//nolint:paralleltest // changes the process-global working directory
+func TestFreshenDirectoryNamedThroughSymlink(t *testing.T) {
+	// Paths are relative to a temp dir holding link, a symlink to the tree.
+	for name, tc := range map[string]struct {
+		workDir string
+		args    []string
+	}{
+		"argument":          {".", []string{testLink}},
+		"working directory": {testLink, nil},
+	} {
+		t.Run(name, func(t *testing.T) {
+			fs := afero.NewOsFs()
+			tree, manifestPath := setupFreshenDir(t, fs,
+				map[string]string{testFileTxt: "in the tree"})
+			writeTestFile(t, fs, filepath.Join(tree, "later.txt"), "added later")
+			require.NoError(t,
+				os.Symlink(testFileTxt, filepath.Join(tree, "alias.txt")))
+
+			root := t.TempDir()
+			require.NoError(t, os.Symlink(tree, filepath.Join(root, testLink)))
+
+			// t.Chdir sets PWD to the path it is given, as a shell does, and
+			// os.Getwd returns PWD when it names the working directory.
+			t.Chdir(filepath.Join(root, tc.workDir))
+
+			opts := testOpts(slices.Concat(
+				[]string{testApp, cmdFreshen, "-q"}, tc.args,
+			), fs)
+			require.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts))
+
+			assertManifestLists(t, fs, tree, manifestPath, map[string]string{
+				testFileTxt: "in the tree", "later.txt": "added later",
+			})
+		})
+	}
+}
+
 // TestFreshenRecordEntryMtimePresence pins the behavior of recordEntry
 // with respect to MFFilePath.Mtime, which is a message pointer with
 // proto3 field presence and may legitimately be absent.

internal/cli/mfer.go

@@ -48,6 +48,10 @@ func manifestTempPath(out string) string {
 // errUnknownCommand indicates an unrecognized command argument.
 var errUnknownCommand = errors.New("unknown command")
 
+// errUnexpectedArgument indicates a second argument to a command that takes
+// one.
+var errUnexpectedArgument = errors.New("unexpected argument")
+
 // CLIApp is the main CLI application container. It holds configuration,
 // I/O streams, and filesystem abstraction to enable testing and flexibility.
 //
@@ -154,13 +158,14 @@ func commonFlags() []cli.Flag {
 	}
 }
 
-// stopOnFirstArg returns the StopOnNthArg setting every command uses: flags
-// are read only before the command's first argument, and everything after it
-// is an argument, as with urfave/cli v2. So `mfer gen d -v` names a path "-v".
-func stopOnFirstArg() *int {
-	n := 1
+// refuseExtraArgs is the ArgValidator of the commands that take one
+// argument: it refuses a second one, naming it.
+func refuseExtraArgs(_ context.Context, cmd *cli.Command) error {
+	if cmd.Args().Len() > 1 {
+		return fmt.Errorf("%w %q", errUnexpectedArgument, cmd.Args().Get(1))
+	}
 
-	return &n
+	return nil
 }
 
 // requireSignatureFlag returns the --require-signature flag taken by the
@@ -186,11 +191,10 @@ func includePermissionsFlag() *cli.BoolFlag {
 
 func (mfa *CLIApp) generateCommand() *cli.Command {
 	return &cli.Command{
-		Name:         cmdGenerate,
-		Aliases:      []string{"gen"},
-		Usage:        "Generate manifest file",
-		ArgsUsage:    "[path ...]",
-		StopOnNthArg: stopOnFirstArg(),
+		Name:      cmdGenerate,
+		Aliases:   []string{"gen"},
+		Usage:     "Generate manifest file",
+		ArgsUsage: "[path ...]",
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			mfa.setVerbosity(cmd)
 			mfa.printBanner()
@@ -252,7 +256,7 @@ func (mfa *CLIApp) checkCommand() *cli.Command {
 		Name:         cmdCheck,
 		Usage:        "Validate files using manifest file",
 		ArgsUsage:    manifestArgsUsage,
-		StopOnNthArg: stopOnFirstArg(),
+		ArgValidator: refuseExtraArgs,
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			mfa.setVerbosity(cmd)
 			mfa.printBanner()
@@ -286,7 +290,7 @@ func (mfa *CLIApp) freshenCommand() *cli.Command {
 		Name:         cmdFreshen,
 		Usage:        "Update manifest with changed, new, and removed files",
 		ArgsUsage:    manifestArgsUsage,
-		StopOnNthArg: stopOnFirstArg(),
+		ArgValidator: refuseExtraArgs,
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			mfa.setVerbosity(cmd)
 			mfa.printBanner()
@@ -337,7 +341,7 @@ func (mfa *CLIApp) exportCommand() *cli.Command {
 		Name:         cmdExport,
 		Usage:        "Export manifest contents as JSON",
 		ArgsUsage:    "[manifest file or URL]",
-		StopOnNthArg: stopOnFirstArg(),
+		ArgValidator: refuseExtraArgs,
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			mfa.setVerbosity(cmd)
 
@@ -348,9 +352,8 @@ func (mfa *CLIApp) exportCommand() *cli.Command {
 
 func (mfa *CLIApp) versionCommand() *cli.Command {
 	return &cli.Command{
-		Name:         cmdVersion,
-		Usage:        "Show version",
-		StopOnNthArg: stopOnFirstArg(),
+		Name:  cmdVersion,
+		Usage: "Show version",
 		Action: func(context.Context, *cli.Command) error {
 			mfa.printVersion()
 
@@ -365,7 +368,7 @@ func (mfa *CLIApp) listCommand() *cli.Command {
 		Aliases:      []string{"ls"},
 		Usage:        "List files in manifest",
 		ArgsUsage:    manifestArgsUsage,
-		StopOnNthArg: stopOnFirstArg(),
+		ArgValidator: refuseExtraArgs,
 		Action:       mfa.listManifestOperation,
 		Flags: []cli.Flag{
 			&cli.BoolFlag{
@@ -386,7 +389,7 @@ func (mfa *CLIApp) fetchCommand() *cli.Command {
 		Name:         cmdFetch,
 		Usage:        "fetch manifest and referenced files",
 		ArgsUsage:    "URL",
-		StopOnNthArg: stopOnFirstArg(),
+		ArgValidator: refuseExtraArgs,
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			mfa.setVerbosity(cmd)
 			mfa.printBanner()
@@ -445,9 +448,8 @@ func (mfa *CLIApp) run(args []string) {
 		Writer:                mfa.Stdout,
 		// v3 writes its "Incorrect Usage" line to ErrWriter; v2 wrote it to
 		// stdout, before the help, so it stays on stdout.
-		ErrWriter:    mfa.Stdout,
-		Flags:        commonFlags(),
-		StopOnNthArg: stopOnFirstArg(),
+		ErrWriter: mfa.Stdout,
+		Flags:     commonFlags(),
 		Action: func(_ context.Context, cmd *cli.Command) error {
 			if cmd.Args().Len() > 0 {
 				return fmt.Errorf("%w %q", errUnknownCommand, cmd.Args().First())

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")
+}

internal/cli/signing.go

@@ -23,9 +23,9 @@ var errNoPassphrase = errors.New(
 	"signing key is protected: set " + envSignKeyPassphrase + " to its passphrase")
 
 // signingOptions returns the signing options for the OpenPGP secret key in
-// the file path. The passphrase of a protected key comes from
-// MFER_SIGN_KEY_PASSPHRASE, or else from the terminal on stdin, and must
-// unlock the key.
+// the file path, which must be able to sign. The passphrase of a protected
+// key comes from MFER_SIGN_KEY_PASSPHRASE, or else from the terminal on
+// stdin, and must unlock the key.
 func (mfa *CLIApp) signingOptions(path string) (*mfer.SigningOptions, error) {
 	secretKey, err := afero.ReadFile(mfa.Fs, path)
 	if err != nil {
@@ -40,17 +40,16 @@ func (mfa *CLIApp) signingOptions(path string) (*mfer.SigningOptions, error) {
 	log.Infof("signing manifest with the OpenPGP key in %s", path)
 
 	opts := &mfer.SigningOptions{SecretKey: secretKey}
-	if !protected {
-		return opts, nil
+	if protected {
+		opts.Passphrase, err = mfa.readPassphrase(path)
+		if err != nil {
+			return nil, err
+		}
 	}
 
-	opts.Passphrase, err = mfa.readPassphrase(path)
-	if err != nil {
-		return nil, err
-	}
-
-	// gen and freshen read the signing options before any file, so a
-	// wrong passphrase stops them before they hash anything.
+	// gen and freshen read the signing options before any file, so a key
+	// that cannot sign, or a wrong passphrase, stops them before they hash
+	// anything.
 	err = mfer.CheckSigningKey(opts)
 	if err != nil {
 		return nil, fmt.Errorf("%s: %w", path, err)

internal/cli/signing_test.go

@@ -7,7 +7,9 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 
+	"github.com/ProtonMail/go-crypto/openpgp/packet"
 	"github.com/creack/pty"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
@@ -33,7 +35,7 @@ func TestGenAndFreshenSignWithKeyFile(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			t.Setenv(envSignKeyPassphrase, string(passphrase))
 
-			secretKey, fingerprint := testSecretKey(t, passphrase)
+			secretKey, fingerprint := testSecretKey(t, passphrase, nil)
 
 			fs := afero.NewOsFs()
 			keyFile := filepath.Join(t.TempDir(), "key.asc")
@@ -77,7 +79,7 @@ func TestGenAndFreshenSignWithKeyFile(t *testing.T) {
 func TestSignWithProtectedKeyNeedsPassphrase(t *testing.T) {
 	t.Setenv(envSignKeyPassphrase, "")
 
-	secretKey, _ := testSecretKey(t, []byte("secret"))
+	secretKey, _ := testSecretKey(t, []byte("secret"), nil)
 
 	fs := afero.NewMemMapFs()
 	require.NoError(t, afero.WriteFile(fs, testKeyFile, secretKey, 0o600))
@@ -97,29 +99,48 @@ func TestSignWithProtectedKeyNeedsPassphrase(t *testing.T) {
 	assert.False(t, exists)
 }
 
-// TestSignWithWrongPassphraseFailsFirst runs gen on a directory and
-// freshen on a manifest, neither of which exists, with a protected key and
-// a wrong MFER_SIGN_KEY_PASSPHRASE. Each must fail to unlock the key: it
-// does that before it reads any file, so a missing file goes unnoticed.
-func TestSignWithWrongPassphraseFailsFirst(t *testing.T) {
+// TestSignWithKeyThatCannotSignFailsFirst runs gen on a directory and
+// freshen on a manifest, neither of which exists, with keys that cannot
+// sign: a protected key with a wrong MFER_SIGN_KEY_PASSPHRASE, a key that
+// expired in 2020, and a version 6 key. Each run must fail on the key: it
+// checks the key before it reads any file, so a missing file goes
+// unnoticed.
+func TestSignWithKeyThatCannotSignFailsFirst(t *testing.T) {
 	t.Setenv(envSignKeyPassphrase, "wrong")
 
-	secretKey, _ := testSecretKey(t, []byte("right"))
+	wrongPassphrase, _ := testSecretKey(t, []byte("right"), nil)
 
-	fs := afero.NewMemMapFs()
-	require.NoError(t, afero.WriteFile(fs, testKeyFile, secretKey, 0o600))
+	made := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)
+	expired, _ := testSecretKey(t, nil, &packet.Config{
+		Algorithm:       packet.PubKeyAlgoEdDSA,
+		Time:            func() time.Time { return made },
+		KeyLifetimeSecs: uint32((24 * time.Hour).Seconds()),
+	})
 
-	for _, args := range [][]string{
-		{
-			testApp, cmdGenerate, "-q", testFlagSignKey, testKeyFile,
-			"-o", testMF, "/missing",
-		},
-		{testApp, cmdFreshen, "-q", testFlagSignKey, testKeyFile, "/missing.mf"},
+	version6, _ := testSecretKey(t, nil, &packet.Config{
+		Algorithm: packet.PubKeyAlgoEd25519,
+		V6Keys:    true,
+	})
+
+	for want, secretKey := range map[string][]byte{
+		"unlock signing key":                           wrongPassphrase,
+		"signing key cannot sign":                      expired,
+		"signing key must be an OpenPGP version 4 key": version6,
 	} {
-		opts := testOpts(args, fs)
-		assert.Equal(t, 1, runCLI(opts), args[1])
-		assert.Contains(t, testStderr(t, opts),
-			testKeyFile+": unlock signing key", args[1])
+		fs := afero.NewMemMapFs()
+		require.NoError(t, afero.WriteFile(fs, testKeyFile, secretKey, 0o600))
+
+		for _, args := range [][]string{
+			{
+				testApp, cmdGenerate, "-q", testFlagSignKey, testKeyFile,
+				"-o", testMF, "/missing",
+			},
+			{testApp, cmdFreshen, "-q", testFlagSignKey, testKeyFile, "/missing.mf"},
+		} {
+			opts := testOpts(args, fs)
+			assert.Equal(t, 1, runCLI(opts), args[1], want)
+			assert.Contains(t, testStderr(t, opts), testKeyFile+": "+want, args[1])
+		}
 	}
 }
 
@@ -130,7 +151,7 @@ func TestSignWithWrongPassphraseFailsFirst(t *testing.T) {
 func TestGenAsksForPassphraseOnTerminal(t *testing.T) {
 	t.Setenv(envSignKeyPassphrase, "")
 
-	secretKey, fingerprint := testSecretKey(t, []byte("passphrase"))
+	secretKey, fingerprint := testSecretKey(t, []byte("passphrase"), nil)
 
 	fs := afero.NewOsFs()
 	keyFile := filepath.Join(t.TempDir(), "key.asc")

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())

mfer/openpgp.go

@@ -16,12 +16,26 @@ import (
 )
 
 const (
-	// The tags of OpenPGP signature, secret key and public key packets
-	// (RFC 9580, section 5). Subkeys have tags of their own, so each
-	// secret or public key packet is one primary key.
-	signaturePacketTag = 2
-	secretKeyPacketTag = 5
-	publicKeyPacketTag = 6
+	// The tags of OpenPGP signature, key and subkey packets (RFC 9580,
+	// section 5). Subkeys have tags of their own, so each secret or public
+	// key packet is one primary key.
+	signaturePacketTag    = 2
+	secretKeyPacketTag    = 5
+	publicKeyPacketTag    = 6
+	secretSubkeyPacketTag = 7
+	publicSubkeyPacketTag = 14
+
+	// In the body of a key or subkey packet the algorithm octet follows
+	// the version octet and the four-octet creation time, and from version
+	// 5 on a four-octet length as well (RFC 9580, section 5.5.2).
+	keyAlgorithmOffset            = 5
+	firstKeyVersionWithLength     = 5
+	keyAlgorithmOffsetAfterLength = 9
+
+	// signingKeyVersion is the only OpenPGP key version mfer signs with.
+	// Its fingerprints are 40 hex characters, the length
+	// --require-signature takes.
+	signingKeyVersion = 4
 
 	// armorBegin and armorEnd start the lines that begin and end an
 	// armored block.
@@ -30,14 +44,19 @@ const (
 )
 
 var (
-	errKeyCount     = errors.New("must hold exactly one key")
-	errNoSecretKey  = errors.New("signing key file holds no secret key")
+	errKeyCount    = errors.New("must hold exactly one key")
+	errDSAKey      = errors.New("must not hold a DSA key")
+	errSecretKey   = errors.New("must not hold a secret key")
+	errNoSecretKey = errors.New("signing key file holds no secret key")
+	errNotV4Key    = errors.New("signing key must be an OpenPGP version 4 key, " +
+		"the only kind whose fingerprint --require-signature takes")
 	errNoPassphrase = errors.New(
 		"signing key is protected and no passphrase was given")
 	errNotOneSignature = errors.New(
 		"signature must hold exactly one signature")
 	errNotOneArmoredBlock = errors.New(
 		"must be exactly one armored block and nothing else")
+	errMalformedArmor = errors.New("armor is malformed")
 )
 
 // SigningOptions holds the key a manifest is signed with.
@@ -51,7 +70,7 @@ type SigningOptions struct {
 
 // SecretKeyIsProtected reports whether the OpenPGP secret key secretKey,
 // armored or binary, needs a passphrase to sign. It fails unless
-// secretKey holds one primary key with its secret key.
+// secretKey holds one version 4 primary key with its secret key.
 func SecretKeyIsProtected(secretKey []byte) (bool, error) {
 	key, err := readSecretKey(secretKey)
 	if err != nil {
@@ -61,14 +80,24 @@ func SecretKeyIsProtected(secretKey []byte) (bool, error) {
 	return isProtected(key), nil
 }
 
-// CheckSigningKey fails unless opts can sign: opts.SecretKey must hold one
-// primary key with its secret key, and opts.Passphrase must unlock it when
-// it is protected. It lets a caller find a wrong passphrase before it
-// builds a manifest.
+// CheckSigningKey fails unless opts can sign now: opts.SecretKey must hold
+// one version 4 primary key with a secret key that may sign and has not
+// expired or been revoked, and opts.Passphrase must unlock it when it is
+// protected. It lets a caller find a key that cannot sign before it builds
+// a manifest.
 func CheckSigningKey(opts *SigningOptions) error {
-	_, err := readSigningKey(opts)
+	key, err := readSigningKey(opts)
+	if err != nil {
+		return err
+	}
 
-	return err
+	// Signing nothing fails wherever signing the manifest would.
+	err = openpgp.DetachSign(io.Discard, key, bytes.NewReader(nil), nil)
+	if err != nil {
+		return fmt.Errorf("signing key cannot sign: %w", err)
+	}
+
+	return nil
 }
 
 // readSigningKey returns the key in opts.SecretKey, unlocked with
@@ -96,13 +125,18 @@ func readSigningKey(opts *SigningOptions) (*openpgp.Entity, error) {
 }
 
 // readSecretKey returns the one key in secretKey, armored or binary,
-// which must include its secret key.
+// which must be a version 4 key and include its secret key.
 func readSecretKey(secretKey []byte) (*openpgp.Entity, error) {
-	key, err := readOneKey(secretKey, "signing key file")
+	key, err := readOneKey(secretKey, "signing key file", false)
 	if err != nil {
 		return nil, err
 	}
 
+	if key.PrimaryKey.Version != signingKeyVersion {
+		return nil, fmt.Errorf("%w; this key is version %d",
+			errNotV4Key, key.PrimaryKey.Version)
+	}
+
 	if key.PrivateKey == nil {
 		return nil, errNoSecretKey
 	}
@@ -111,8 +145,9 @@ func readSecretKey(secretKey []byte) (*openpgp.Entity, error) {
 }
 
 // readOneKey returns the key in data, armored or binary, which must hold
-// exactly one primary key. what names data in errors.
-func readOneKey(data []byte, what string) (*openpgp.Entity, error) {
+// exactly one primary key and no DSA key or subkey, and when publicOnly no
+// secret key or subkey either. what names data in errors.
+func readOneKey(data []byte, what string, publicOnly bool) (*openpgp.Entity, error) {
 	packets, err := dearmor(data)
 	if err != nil {
 		return nil, fmt.Errorf("read %s: %w", what, err)
@@ -127,6 +162,29 @@ func readOneKey(data []byte, what string) (*openpgp.Entity, error) {
 		return nil, fmt.Errorf("%s %w, found %d", what, errKeyCount, keys)
 	}
 
+	// openpgp.ReadKeyRing checks the numbers of every secret key it reads,
+	// and an ElGamal secret subkey with a very large prime makes that take
+	// minutes.
+	secretKeys, err := countPackets(packets, secretKeyPacketTag, secretSubkeyPacketTag)
+	if err != nil {
+		return nil, fmt.Errorf("read %s: %w", what, err)
+	}
+
+	if publicOnly && secretKeys != 0 {
+		return nil, fmt.Errorf("%s %w", what, errSecretKey)
+	}
+
+	// openpgp.ReadKeyRing also checks every self-signature, and a DSA key
+	// with very large numbers makes each check take seconds to minutes.
+	dsa, err := holdsDSAKey(packets)
+	if err != nil {
+		return nil, fmt.Errorf("read %s: %w", what, err)
+	}
+
+	if dsa {
+		return nil, fmt.Errorf("%s %w", what, errDSAKey)
+	}
+
 	keyring, err := openpgp.ReadKeyRing(bytes.NewReader(packets))
 	if err != nil {
 		return nil, fmt.Errorf("read %s: %w", what, err)
@@ -189,7 +247,7 @@ func fingerprint(key *openpgp.Entity) string {
 // and returns that primary key's fingerprint. signature and pubKey may each
 // be armored or binary.
 func verifySignature(data, signature, pubKey []byte) (string, error) {
-	key, err := readOneKey(pubKey, "embedded public key block")
+	key, err := readOneKey(pubKey, "embedded public key block", true)
 	if err != nil {
 		return "", err
 	}
@@ -240,12 +298,20 @@ func dearmor(data []byte) ([]byte, error) {
 		return nil, errNotOneArmoredBlock
 	}
 
+	// armor.Decode passes over a block it cannot read, such as one with a
+	// header line that has no colon, and returns io.EOF on finding no
+	// other.
 	block, err := armor.Decode(bytes.NewReader(text))
 	if err != nil {
-		return nil, err
+		return nil, errMalformedArmor
 	}
 
-	return io.ReadAll(block.Body)
+	body, err := io.ReadAll(block.Body)
+	if err != nil {
+		return nil, fmt.Errorf("%w: %w", errMalformedArmor, err)
+	}
+
+	return body, nil
 }
 
 // countPackets returns how many packets in the binary OpenPGP data have
@@ -271,3 +337,40 @@ func countPackets(data []byte, tags ...uint8) (int, error) {
 		}
 	}
 }
+
+// holdsDSAKey reports whether any key or subkey packet in the binary
+// OpenPGP data holds a DSA key. It reads each packet's algorithm octet
+// rather than parsing the packet, since parsing a secret key packet checks
+// its numbers, which for a DSA key with very large numbers is as slow as
+// checking a self-signature.
+func holdsDSAKey(data []byte) (bool, error) {
+	packets := packet.NewOpaqueReader(bytes.NewReader(data))
+
+	for {
+		p, err := packets.Next()
+		if errors.Is(err, io.EOF) {
+			return false, nil
+		}
+
+		if err != nil {
+			return false, err
+		}
+
+		if !slices.Contains([]uint8{
+			secretKeyPacketTag, publicKeyPacketTag,
+			secretSubkeyPacketTag, publicSubkeyPacketTag,
+		}, p.Tag) {
+			continue
+		}
+
+		offset := keyAlgorithmOffset
+		if len(p.Contents) > 0 && p.Contents[0] >= firstKeyVersionWithLength {
+			offset = keyAlgorithmOffsetAfterLength
+		}
+
+		if len(p.Contents) > offset &&
+			packet.PublicKeyAlgorithm(p.Contents[offset]) == packet.PubKeyAlgoDSA {
+			return true, nil
+		}
+	}
+}

mfer/openpgp_test.go

@@ -4,7 +4,9 @@ package mfer
 import (
 	"bytes"
 	"context"
+	"crypto/dsa" //nolint:staticcheck // SA1019: tests need a DSA key to refuse
 	"io"
+	"math/big"
 	"os"
 	"path/filepath"
 	"slices"
@@ -15,6 +17,7 @@ import (
 
 	"github.com/ProtonMail/go-crypto/openpgp"
 	"github.com/ProtonMail/go-crypto/openpgp/armor"
+	"github.com/ProtonMail/go-crypto/openpgp/elgamal"
 	"github.com/ProtonMail/go-crypto/openpgp/packet"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
@@ -284,6 +287,97 @@ func TestSigningKeyFileWithoutSecretKey(t *testing.T) {
 	require.ErrorIs(t, b.Build(context.Background(), io.Discard), errNoSecretKey)
 }
 
+// TestCheckSigningKeyRefusesKeyThatCannotSign checks a key that can sign,
+// and keys that read and need no passphrase but cannot sign now: one that
+// expired in 2020 and one that has been revoked. CheckSigningKey must
+// refuse each of the two, as signing a manifest with it would fail.
+func TestCheckSigningKeyRefusesKeyThatCannotSign(t *testing.T) {
+	t.Parallel()
+
+	require.NoError(t, CheckSigningKey(testSigningOptions(t)))
+
+	made := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)
+	expired := newTestKey(t, &packet.Config{
+		Time:            func() time.Time { return made },
+		KeyLifetimeSecs: uint32((24 * time.Hour).Seconds()),
+	})
+
+	revoked := newTestKey(t, nil)
+	require.NoError(t, revoked.RevokeKey(packet.KeyRetired, "", nil))
+
+	for name, key := range map[string]*openpgp.Entity{
+		"expired": expired,
+		"revoked": revoked,
+	} {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			assert.ErrorContains(t, CheckSigningKey(
+				&SigningOptions{SecretKey: armoredSecretKeys(t, key)}),
+				"signing key cannot sign")
+		})
+	}
+}
+
+// TestSigningRefusesVersion6Key signs with an OpenPGP version 6 key, whose
+// fingerprint is 64 hex characters. mfer signs only with version 4 keys.
+func TestSigningRefusesVersion6Key(t *testing.T) {
+	t.Parallel()
+
+	key, err := openpgp.NewEntity("MFER Test Key", "", "test@mfer.test",
+		&packet.Config{V6Keys: true, Algorithm: packet.PubKeyAlgoEd25519})
+	require.NoError(t, err)
+
+	secretKey := armoredSecretKeys(t, key)
+
+	_, err = SecretKeyIsProtected(secretKey)
+	require.ErrorIs(t, err, errNotV4Key)
+
+	b := NewBuilder()
+	b.SetSigningOptions(&SigningOptions{SecretKey: secretKey})
+	require.ErrorIs(t, b.Build(context.Background(), io.Discard), errNotV4Key)
+}
+
+// TestMalformedArmorIsNamed reads a signing key file, a signature and an
+// embedded public key block whose armor is malformed: a header line with
+// no colon, or no blank line after the BEGIN line. Each must fail saying
+// the armor is malformed.
+func TestMalformedArmorIsNamed(t *testing.T) {
+	t.Parallel()
+
+	opts := testSigningOptions(t)
+	manifest := signedTestManifest(t, opts)
+
+	for name, change := range map[string]func([]byte) []byte{
+		"header line with no colon": func(block []byte) []byte {
+			return bytes.Replace(block,
+				[]byte("-----\n"), []byte("-----\nno colon\n"), 1)
+		},
+		"no blank line after the BEGIN line": func(block []byte) []byte {
+			return bytes.Replace(block, []byte("-----\n\n"), []byte("-----\n"), 1)
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			_, err := SecretKeyIsProtected(change(opts.SecretKey))
+			require.ErrorIs(t, err, errMalformedArmor)
+
+			for _, changed := range [][]byte{
+				rewriteOuter(t, manifest, func(outer *MFFileOuter) {
+					outer.Signature = change(outer.GetSignature())
+				}),
+				rewriteOuter(t, manifest, func(outer *MFFileOuter) {
+					outer.SigningPubKey = change(outer.GetSigningPubKey())
+				}),
+			} {
+				_, err = NewManifestFromReader(bytes.NewReader(changed))
+				require.ErrorIs(t, err, errMalformedArmor)
+			}
+		})
+	}
+}
+
 func TestVerifySignature(t *testing.T) {
 	t.Parallel()
 
@@ -463,6 +557,105 @@ func TestManifestRefusesSecondEmbeddedKeyWithoutUserID(t *testing.T) {
 	require.ErrorIs(t, err, errKeyCount)
 }
 
+// dsaKeyPacket returns a public key packet holding a DSA key, or a public
+// subkey packet when isSubkey. Its numbers are not a working key: loading
+// must refuse the packet before it uses them.
+func dsaKeyPacket(t *testing.T, isSubkey bool) []byte {
+	t.Helper()
+
+	key := packet.NewDSAPublicKey(time.Now(), &dsa.PublicKey{
+		P: big.NewInt(23), Q: big.NewInt(11), G: big.NewInt(4), Y: big.NewInt(8),
+	})
+	key.IsSubkey = isSubkey
+
+	var buf bytes.Buffer
+
+	require.NoError(t, key.Serialize(&buf))
+
+	return buf.Bytes()
+}
+
+// TestManifestRefusesDSAKey loads manifests whose embedded public key
+// block holds a DSA key: alone, or as a subkey after the key that signed
+// the manifest. openpgp.ReadKeyRing checks a key's self-signatures, which
+// for a DSA key with very large numbers takes minutes each. Loading must
+// refuse each block before that.
+func TestManifestRefusesDSAKey(t *testing.T) {
+	t.Parallel()
+
+	signer := newTestKey(t, nil)
+	manifest := signedTestManifest(t,
+		&SigningOptions{SecretKey: armoredSecretKeys(t, signer)})
+
+	var signerKey bytes.Buffer
+
+	require.NoError(t, signer.Serialize(&signerKey))
+
+	for name, block := range map[string][]byte{
+		"DSA key":    dsaKeyPacket(t, false),
+		"DSA subkey": slices.Concat(signerKey.Bytes(), dsaKeyPacket(t, true)),
+	} {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			embedded := rewriteOuter(t, manifest, func(outer *MFFileOuter) {
+				outer.SigningPubKey = block
+			})
+
+			_, err := NewManifestFromReader(bytes.NewReader(embedded))
+			require.ErrorIs(t, err, errDSAKey)
+		})
+	}
+}
+
+// elGamalSecretSubkeyPacket returns a secret subkey packet holding an
+// ElGamal key. Its numbers are not a working key: loading must refuse the
+// packet before it uses them.
+func elGamalSecretSubkeyPacket(t *testing.T) []byte {
+	t.Helper()
+
+	key := packet.NewElGamalPrivateKey(time.Now(), &elgamal.PrivateKey{
+		P: big.NewInt(23), G: big.NewInt(4), Y: big.NewInt(8), X: big.NewInt(3),
+	})
+	key.IsSubkey = true
+
+	var buf bytes.Buffer
+
+	require.NoError(t, key.Serialize(&buf))
+
+	return buf.Bytes()
+}
+
+// TestManifestRefusesSecretKey loads manifests whose embedded public key
+// block holds a secret key: the key that signed the manifest with its
+// secret key, or its public key followed by an ElGamal secret subkey.
+// openpgp.ReadKeyRing checks the numbers of every secret key it reads,
+// which for an ElGamal key with a very large prime takes minutes. Loading
+// must refuse each block before that.
+func TestManifestRefusesSecretKey(t *testing.T) {
+	t.Parallel()
+
+	signer := newTestKey(t, nil)
+	manifest := signedTestManifest(t,
+		&SigningOptions{SecretKey: armoredSecretKeys(t, signer)})
+
+	var signerKey bytes.Buffer
+
+	require.NoError(t, signer.Serialize(&signerKey))
+
+	for _, block := range [][]byte{
+		armoredSecretKeys(t, signer),
+		slices.Concat(signerKey.Bytes(), elGamalSecretSubkeyPacket(t)),
+	} {
+		embedded := rewriteOuter(t, manifest, func(outer *MFFileOuter) {
+			outer.SigningPubKey = block
+		})
+
+		_, err := NewManifestFromReader(bytes.NewReader(embedded))
+		require.ErrorIs(t, err, errSecretKey)
+	}
+}
+
 // TestManifestRefusesTwoSignatures loads a manifest whose signature field
 // holds its good signature twice, not armored. Loading must refuse it.
 func TestManifestRefusesTwoSignatures(t *testing.T) {

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

mfer/scanner.go

@@ -139,25 +139,15 @@ func (s *Scanner) EnumerateFile(filePath string) error {
 	return s.enumerateFileWithInfo(filepath.Base(abs), basePath, info, nil)
 }
 
-// EnumeratePath walks a directory path and adds all files to the scanner.
+// EnumeratePath adds inputPath, a directory or a file, to the scanner as
+// EnumeratePaths adds each of its paths.
 // If progress is non-nil, status updates are sent as files are discovered.
 // The progress channel is closed when the method returns.
 func (s *Scanner) EnumeratePath(
 	inputPath string,
 	progress chan<- EnumerateStatus,
 ) error {
-	if progress != nil {
-		defer close(progress)
-	}
-
-	abs, err := filepath.Abs(inputPath)
-	if err != nil {
-		return err
-	}
-
-	afs := afero.NewReadOnlyFs(afero.NewBasePathFs(s.fs, abs))
-
-	return s.enumerateFS(afs, abs, progress)
+	return s.EnumeratePaths(progress, inputPath)
 }
 
 // EnumeratePaths adds to the scanner the files under each directory path,
@@ -185,6 +175,14 @@ func (s *Scanner) EnumeratePaths(
 		}
 
 		if info.IsDir() {
+			// The walk does not follow a symlink at its top, so a directory
+			// named through one is resolved first. If that fails, the
+			// directory is walked as named and the walk reports the problem.
+			resolved, evalErr := filepath.EvalSymlinks(abs)
+			if evalErr == nil {
+				abs = resolved
+			}
+
 			afs := afero.NewReadOnlyFs(afero.NewBasePathFs(s.fs, abs))
 			err = s.enumerateFS(afs, abs, progress)
 		} else {
@@ -476,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

mfer/scanner_test.go

@@ -118,6 +118,27 @@ func TestScannerEnumeratePathWithProgress(t *testing.T) {
 	assert.Equal(t, FileSize(6), final.BytesFound)
 }
 
+// TestScannerEnumeratePathFile gives EnumeratePath a file: it is listed
+// by its name, as EnumerateFile lists it, and the manifest can be built.
+func TestScannerEnumeratePathFile(t *testing.T) {
+	t.Parallel()
+
+	fs := afero.NewMemMapFs()
+	require.NoError(t, fs.MkdirAll("/dir", 0o755))
+	require.NoError(t, afero.WriteFile(fs, "/dir/one.txt", []byte("1"), 0o644))
+
+	s := NewScannerWithOptions(&ScannerOptions{Fs: fs})
+	require.NoError(t, s.EnumeratePath("/dir/one.txt", 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, "one.txt", m.Files()[0].GetPath())
+}
+
 func TestScannerEnumeratePaths(t *testing.T) {
 	t.Parallel()