Refuse a file whose size changed between freshen's scan and hash (closes #207)

freshen took a changed or new file's size from the scan and its hash
from a later read, without comparing the bytes read with that size. A
file that grew or shrank in between got an entry no content can match,
so check failed it with nothing saying why. processEntry now fails with
a size mismatch naming the file, as gen does, so freshen exits 1 and
leaves the manifest as it was.

Judgement call: the error wraps the package's existing errSizeMismatch
from fetch; its comment now covers both uses.

Model: opus-5-5

README.md

@@ -17,6 +17,15 @@ The current version is pre-1.0 and while the repo was published in 2022, there
 has not yet been any versioned release. [SemVer](https://semver.org) will be
 used for releases.
 
+A version is a git tag, and no file in the tree records a version number.
+`mfer version` prints the revision the build stamped into the binary, which for
+`make build` and the Docker image is the output of `git describe` on the
+checkout they build. A binary built without that stamp, as
+`go install sneak.berlin/go/mfer/cmd/mfer@<tag>` builds it, prints the module
+version Go records in it instead. A program that imports the `mfer` package
+finds the version it was built against in its own build information
+(`runtime/debug.ReadBuildInfo`).
+
 This project was started by [@sneak](https://sneak.berlin) to scratch an itch in
 2022 and is currently a one-person effort, though the goal is for this to emerge
 as a de-facto standard and be incorporated into other software. A compatible
@@ -268,6 +277,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`
@@ -287,22 +298,32 @@ are now tracked only in the [issues](https://git.eeqj.de/sneak/mfer/issues).
       else the directory holding the manifest, or the current directory for a
       manifest given by URL. So `mfer check /media/drive` checks a drive against
       the `index.mf` at its root, from any directory
-    - 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
+    - warns about each regular file under the base directory that the manifest
+      does not list, hidden files included, and each symlink there to a regular
+      file, which gen lists only with `--follow-symlinks`; with
+      `--no-extra-files` each one is a failure instead. A named pipe, socket or
+      device, a symlink to a directory and a broken symlink are never reported,
+      since gen cannot list them
 - `mfer freshen` / `mfer freshen .`
     - 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
+    - refuses a signed manifest, leaving it as it was and naming the fingerprint
+      of the key that signed it, unless `--sign-key` or `MFER_SIGN_KEY` names a
+      key to sign the new one with
 - `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/`

cmd/mfer/main.go

@@ -10,14 +10,12 @@ import (
 // Appname is the name of this program.
 const Appname = "mfer"
 
-// Version and Gitrev are injected at build time via -ldflags.
+// Gitrev is the revision `mfer version` prints, injected at build time via
+// -ldflags.
 //
 //nolint:gochecknoglobals // set via ldflags at build time
-var (
-	Version string
-	Gitrev  string
-)
+var Gitrev string
 
 func main() {
-	os.Exit(cli.Run(Appname, Version, Gitrev))
+	os.Exit(cli.Run(Appname, Gitrev))
 }

cmd/mfer/main_test.go

@@ -15,11 +15,10 @@ func TestAppname(t *testing.T) {
 	assert.Equal(t, "mfer", Appname)
 }
 
-// TestVersionDefaults documents that Version and Gitrev are empty unless
-// injected at build time via -ldflags.
-func TestVersionDefaults(t *testing.T) {
+// TestGitrevDefault documents that Gitrev is empty unless injected at build
+// time via -ldflags.
+func TestGitrevDefault(t *testing.T) {
 	t.Parallel()
 
-	assert.Empty(t, Version)
 	assert.Empty(t, Gitrev)
 }

docs/FORMAT.md

@@ -36,8 +36,8 @@ The outer message contains:
 | `sha256`          | 104    | bytes            | SHA-256 hash of the **compressed** `innerMessage` (corruption detection) |
 | `uuid`            | 105    | bytes            | Random v4 UUID; must match the inner message UUID                        |
 | `innerMessage`    | 199    | bytes            | Zstd-compressed serialized `MFFile` message                              |
-| `signature`       | 201    | bytes (optional) | OpenPGP detached signature (ASCII-armored or binary)                     |
-| `signer`          | 202    | bytes (optional) | Fingerprint of the signing key                                           |
+| `signature`       | 201    | bytes (optional) | One OpenPGP signature over the canonical string, armored or binary       |
+| `signer`          | 202    | bytes (optional) | Fingerprint of the signing primary key, in upper-case hex                |
 | `signingPubKey`   | 203    | bytes (optional) | Full OpenPGP public key of the signing key (ASCII-armored or binary)     |
 
 ### SHA-256 Hash
@@ -75,15 +75,15 @@ After decompressing `innerMessage`, the result is a serialized `MFFile`
 
 Each file entry contains:
 
-| Field      | Number | Type                      | Description                         |
-| ---------- | ------ | ------------------------- | ----------------------------------- |
-| `path`     | 1      | string                    | Relative file path (see Path Rules) |
-| `size`     | 2      | int64                     | File size in bytes                  |
-| `hashes`   | 3      | repeated `MFFileChecksum` | At least one hash required          |
-| `mimeType` | 301    | string (optional)         | MIME type                           |
-| `mtime`    | 302    | Timestamp (optional)      | Modification time                   |
-| `ctime`    | 303    | Timestamp (optional)      | Change time (inode metadata change) |
-| `mode`     | 304    | uint32                    | Permission bits (see File Mode)     |
+| Field      | Number | Type                      | Description                                                   |
+| ---------- | ------ | ------------------------- | ------------------------------------------------------------- |
+| `path`     | 1      | string                    | Relative file path (see Path Rules)                           |
+| `size`     | 2      | int64                     | File size in bytes                                            |
+| `hashes`   | 3      | repeated `MFFileChecksum` | At least one hash required; readers reject an entry with none |
+| `mimeType` | 301    | string (optional)         | MIME type                                                     |
+| `mtime`    | 302    | Timestamp (optional)      | Modification time                                             |
+| `ctime`    | 303    | Timestamp (optional)      | Change time (inode metadata change)                           |
+| `mode`     | 304    | uint32                    | Permission bits (see File Mode)                               |
 
 ## File Mode
 
@@ -107,6 +107,7 @@ All `path` values must satisfy these invariants:
 - **Forward slashes**: use `/` as the path separator (never `\`)
 - **Relative only**: no leading `/`
 - **No parent traversal**: no `..` path segments
+- **No current-directory segments**: no `.` path segments
 - **No empty segments**: no `//` sequences
 - **No trailing slash**: paths refer to files, not directories
 - **Listed once**: each path appears at most once in a manifest, compared byte
@@ -128,31 +129,64 @@ Implementations must be able to verify SHA-256 multihashes at minimum.
 
 ## Signature Scheme
 
-Signing is optional. When present, the signature covers a canonical string
-constructed as:
+Signing is optional. A signature is stored only in the outer message's
+`signature` field, never in a separate file. It is one OpenPGP signature over
+the canonical string below, armored or binary, and does not contain the string,
+which a verifier builds again from the outer message.
+
+The canonical string is three parts separated by hyphens (`-`):
 
 ```
 ZNAVSRFG-<UUID>-<SHA256>
 ```
 
-Where:
+- `ZNAVSRFG` is the magic, in ASCII.
+- `<UUID>` is the 16 bytes of the outer message's `uuid` as 32 lower-case hex
+  digits, with no hyphens.
+- `<SHA256>` is the lower-case hex of the outer message's `sha256` encoded as a
+  SHA-256 multihash (see Hash Format). That multihash is the algorithm code
+  `0x12`, the digest length `0x20` and the 32 bytes of the hash, so this part is
+  68 hex digits starting `1220`.
 
-- `ZNAVSRFG` is the magic bytes string (literal ASCII)
-- `<UUID>` is the hex-encoded UUID from the outer message
-- `<SHA256>` is the hex-encoded SHA-256 hash from the outer message (covering
-  compressed data)
+The signature covers exactly the 110 ASCII characters of the string, with no
+line ending. For example, the UUID `b0fb6918-6298-443b-ada2-1b162f4dc0b3` and a
+`sha256` of `e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855`
+in hex give this canonical string:
 
-Components are separated by hyphens. The signature is an OpenPGP detached
-signature over this canonical string, stored in the `signature` field of the
-outer message. The signing key's public key goes in `signingPubKey` and its
-fingerprint, in hex, in `signer`.
+```
+ZNAVSRFG-b0fb69186298443bada21b162f4dc0b3-1220e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
+```
+
+The signature covers `innerMessage` only through `sha256`, so a verifier checks
+`sha256` against `innerMessage` (see SHA-256 Hash) as well as the signature.
+
+The signing key's public key goes in `signingPubKey` and the fingerprint of its
+primary key, in upper-case hex, in `signer`. The reference implementation signs
+only with version 4 keys, whose fingerprints are 40 hex digits, and writes both
+the signature and the public key armored.
 
 A verifier accepts a signed manifest only if `signingPubKey` holds exactly one
 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`.
+that key (or one of its subkeys), and `signer` is that primary key's
+fingerprint, compared without regard to case. 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`. 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.
+
+A signature is good only if the primary key, and the subkey that made the
+signature, were valid at the signature's creation time under the key validity
+rules of RFC 9580. The key is judged by its current self-signatures and binding
+signatures: for a version 4 key the newest self-signature on its primary user
+ID, for a version 6 key its newest direct-key signature, and for a subkey its
+newest binding signature with the signature embedded in it. So a signature made
+before the newest of those is not good, even if an older one made the key valid
+at that time. The reference implementation checks this by verifying the
+signature a second time with its clock set to the signature's creation time. A
+key that has expired since does not make the signature bad, because a manifest
+outlives its key. A signature past its own expiry time is not good, whether or
+not the key has expired too.
 
 ## Deterministic Serialization
 

go.mod

@@ -4,6 +4,7 @@ go 1.27.1
 
 require (
 	github.com/ProtonMail/go-crypto v1.5.2
+	github.com/creack/pty v1.1.25-0.20260601142114-9246436fffe8
 	github.com/davecgh/go-spew v1.1.1
 	github.com/dustin/go-humanize v1.1.0
 	github.com/klauspost/compress v1.20.1

go.sum

@@ -2,6 +2,8 @@ github.com/ProtonMail/go-crypto v1.5.2 h1:cucYnvqcY7UOXVD//mSyjeaPY0SSN3v5cDkYPx
 github.com/ProtonMail/go-crypto v1.5.2/go.mod h1:/RaSu30DaKO4RY+XdV/ACcCcZkGr7AhUIduq5sjzzCo=
 github.com/cloudflare/circl v1.6.3 h1:9GPOhQGF9MCYUeXyMYlqTR6a5gTrgR/fBLXvUgtVcg8=
 github.com/cloudflare/circl v1.6.3/go.mod h1:2eXP6Qfat4O/Yhh8BznvKnJ+uzEoTQ6jVKJRn81BiS4=
+github.com/creack/pty v1.1.25-0.20260601142114-9246436fffe8 h1:CY3gjC7naqYGLMiywvj3suPfa1i0p/QEr7o8ujxL/2M=
+github.com/creack/pty v1.1.25-0.20260601142114-9246436fffe8/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/dustin/go-humanize v1.1.0 h1:dbKTrvD0klcbBV/h4AWJdMuZogJACoMlvWIWZ5b2xWg=

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 (
@@ -189,8 +189,8 @@ func reportCheckProgress(progress <-chan mfer.CheckStatus, wg *sync.WaitGroup) {
 	log.ProgressDone()
 }
 
-// countCheckFailures consumes check results, counting and logging
-// failures, then closes done.
+// countCheckFailures consumes check results, counting failures and logging
+// each at error level so that --quiet still shows them, then closes done.
 func countCheckFailures(
 	results <-chan mfer.Result, failures *int64, done chan<- struct{},
 ) {
@@ -198,7 +198,7 @@ func countCheckFailures(
 		if result.Status != mfer.StatusOK {
 			*failures++
 
-			log.Infof("%s: %s (%s)", result.Status, result.Path, result.Message)
+			log.Errorf("%s: %s (%s)", result.Status, result.Path, result.Message)
 		} else {
 			log.Verbosef("%s: %s", result.Status, result.Path)
 		}
@@ -208,8 +208,8 @@ func countCheckFailures(
 }
 
 // findExtraFiles reports files present on disk but absent from the
-// manifest, and anything the search cannot read: each is a failure under
-// --no-extra-files, otherwise a warning.
+// manifest, and anything the search cannot read: each is a failure, logged
+// at error level, under --no-extra-files, otherwise a warning.
 func findExtraFiles(
 	ctx context.Context, cmd *cli.Command, chk *mfer.Checker, failures *int64,
 ) error {
@@ -221,7 +221,7 @@ func findExtraFiles(
 			if cmd.Bool("no-extra-files") {
 				*failures++
 
-				log.Infof("%s: %s (%s)", result.Status, result.Path, result.Message)
+				log.Errorf("%s: %s (%s)", result.Status, result.Path, result.Message)
 			} else {
 				log.Warnf("%s: %s (%s)", result.Status, result.Path, result.Message)
 			}
@@ -329,10 +329,10 @@ func (mfa *CLIApp) checkManifestOperation(
 		return fmt.Errorf("load manifest: %w", err)
 	}
 
-	// Check signature requirement
-	requiredSigner := cmd.String(flagRequireSignature)
-	if requiredSigner != "" {
-		err = verifyRequiredSigner(chk, requiredSigner)
+	// Check signature requirement. An empty fingerprint is checked too,
+	// and refused as one of the wrong length.
+	if cmd.IsSet(flagRequireSignature) {
+		err = verifyRequiredSigner(chk, cmd.String(flagRequireSignature))
 		if err != nil {
 			return err
 		}

internal/cli/entry.go

@@ -26,8 +26,7 @@ func noColorEnvSet() bool {
 // Use DefaultRunOptions for standard CLI execution, or construct manually for testing.
 type RunOptions struct {
 	Appname string    // Application name displayed in help and version output
-	Version string    // Version string (typically set at build time)
-	Gitrev  string    // Git revision hash (typically set at build time)
+	Gitrev  string    // Revision stamped in at build time, or empty
 	Args    []string  // Command-line arguments (typically os.Args)
 	Stdin   io.Reader // Standard input stream
 	Stdout  io.Writer // Standard output stream
@@ -36,10 +35,9 @@ type RunOptions struct {
 }
 
 // DefaultRunOptions returns RunOptions configured for normal CLI execution.
-func DefaultRunOptions(appname, version, gitrev string) *RunOptions {
+func DefaultRunOptions(appname, gitrev string) *RunOptions {
 	return &RunOptions{
 		Appname: appname,
-		Version: version,
 		Gitrev:  gitrev,
 		Args:    os.Args,
 		Stdin:   os.Stdin,
@@ -50,15 +48,14 @@ func DefaultRunOptions(appname, version, gitrev string) *RunOptions {
 }
 
 // Run creates and runs the CLI application with default options.
-func Run(appname, version, gitrev string) int {
-	return RunWithOptions(DefaultRunOptions(appname, version, gitrev))
+func Run(appname, gitrev string) int {
+	return RunWithOptions(DefaultRunOptions(appname, gitrev))
 }
 
 // RunWithOptions creates and runs the CLI application with the given options.
 func RunWithOptions(opts *RunOptions) int {
 	m := &CLIApp{
 		appname:         opts.Appname,
-		version:         opts.Version,
 		gitrev:          opts.Gitrev,
 		exitCode:        0,
 		maxManifestSize: mfer.MaxManifestSize,

internal/cli/entry_test.go

@@ -11,6 +11,7 @@ import (
 	"net/http/httptest"
 	"os"
 	"path/filepath"
+	"runtime/debug"
 	"slices"
 	"strings"
 	"sync"
@@ -27,6 +28,7 @@ import (
 
 const (
 	testApp         = "mfer"
+	testGitrev      = "abc123"
 	testDir         = "/testdir"
 	testFile1       = "/testdir/file1.txt"
 	testMF          = "/testdir/test.mf"
@@ -36,6 +38,8 @@ const (
 	testFlagNoExtra = "--no-extra-files"
 	testFlagVersion = "--version"
 	testFlagVerbose = "--verbose"
+	testFlagHelp    = "--help"
+	testCmdHelp     = "help"
 )
 
 var errSimulatedWrite = errors.New("simulated write failure")
@@ -58,9 +62,10 @@ func runCLI(opts *RunOptions) int {
 // runCLIWithLevel is runCLI that also returns the log level the run left
 // set, read while runMu still keeps other runs from changing it. Each run
 // starts at the default level, as a new process does. Before releasing the
-// lock it points the process-global logger at io.Discard: other tests log
-// outside the lock (manifest loads, scans), and those lines must not land in
-// this run's buffers once it has returned and its test is reading them.
+// lock it points the process-global logger at io.Discard: test helpers also
+// log under runMu outside any run (manifest loads, scans), and those lines
+// must not land in this run's buffers once it has returned and its test is
+// reading them.
 func runCLIWithLevel(opts *RunOptions) (int, log.Level) {
 	runMu.Lock()
 	defer runMu.Unlock()
@@ -93,8 +98,7 @@ func TestBuild(t *testing.T) {
 func testOpts(args []string, fs afero.Fs) *RunOptions {
 	return &RunOptions{
 		Appname: testApp,
-		Version: "1.0.0",
-		Gitrev:  "abc123",
+		Gitrev:  testGitrev,
 		Args:    args,
 		Stdin:   &bytes.Buffer{},
 		Stdout:  &bytes.Buffer{},
@@ -136,10 +140,35 @@ func TestVersionCommand(t *testing.T) {
 	exitCode := runCLI(opts)
 
 	assert.Equal(t, 0, exitCode)
+	assert.Equal(t, "mfer version "+testGitrev+"\n", testStdout(t, opts))
+}
 
-	stdout := testStdout(t, opts)
-	assert.Contains(t, stdout, mfer.Version)
-	assert.Contains(t, stdout, "abc123")
+// TestVersionFromBuildInfo covers a binary whose build stamped no Gitrev,
+// as `go install` builds it: mfer version prints the main module's version
+// from the build information instead.
+func TestVersionFromBuildInfo(t *testing.T) {
+	t.Parallel()
+
+	info, ok := debug.ReadBuildInfo()
+	require.True(t, ok)
+	require.NotEmpty(t, info.Main.Version)
+
+	opts := testOpts([]string{testApp, cmdVersion}, afero.NewMemMapFs())
+	opts.Gitrev = ""
+
+	require.Equal(t, 0, runCLI(opts))
+	assert.Equal(t, "mfer version "+info.Main.Version+"\n", testStdout(t, opts))
+}
+
+// TestModuleVersionEmpty covers build information that holds no module
+// version, as in a build with GO111MODULE=off, and a binary with no build
+// information at all. Both report "unknown": an empty version would print
+// nothing after "mfer version" and drop the --version flag.
+func TestModuleVersionEmpty(t *testing.T) {
+	t.Parallel()
+
+	assert.Equal(t, "unknown", moduleVersion(&debug.BuildInfo{}, true))
+	assert.Equal(t, "unknown", moduleVersion(nil, false))
 }
 
 // TestVFlagCollision covers the -v/--verbose vs --version flag interaction
@@ -165,7 +194,7 @@ func TestVFlagCollision(t *testing.T) {
 			exitCode := runCLI(opts)
 
 			assert.Equal(t, 0, exitCode, "stderr: %s", testStderr(t, opts))
-			assert.Contains(t, testStdout(t, opts), mfer.Version)
+			assert.Contains(t, testStdout(t, opts), testGitrev)
 			assert.NotContains(t, testStderr(t, opts), "two forms of the same flag")
 		})
 	}
@@ -206,13 +235,13 @@ func TestVersionFlagAndCommandMatch(t *testing.T) {
 
 // TestRootVerbosityFlags asserts that -q and -v given before any subcommand
 // name take effect: in the root itself, and in the fetch and export
-// subcommands (issue #64). It does not run in parallel: other tests log
-// outside runMu (manifest loads, scans), and a line logged while one of these
-// runs is in progress lands in its output.
-//
-//nolint:paralleltest // see above
+// subcommands (issue #64).
 func TestRootVerbosityFlags(t *testing.T) {
+	t.Parallel()
+
 	t.Run("quiet with no subcommand hides the banner", func(t *testing.T) {
+		t.Parallel()
+
 		loud := testOpts([]string{testApp}, afero.NewMemMapFs())
 		require.Equal(t, 0, runCLI(loud))
 		assert.Contains(t, testStdout(t, loud), banner)
@@ -224,6 +253,8 @@ func TestRootVerbosityFlags(t *testing.T) {
 	})
 
 	t.Run("quiet before fetch hides the banner", func(t *testing.T) {
+		t.Parallel()
+
 		opts := testOpts([]string{testApp, "-q", cmdFetch}, afero.NewMemMapFs())
 
 		assert.Equal(t, 1, runCLI(opts))
@@ -232,6 +263,8 @@ func TestRootVerbosityFlags(t *testing.T) {
 	})
 
 	t.Run("verbose twice before fetch enables debug logging", func(t *testing.T) {
+		t.Parallel()
+
 		opts := testOpts([]string{testApp, "-v", "-v", cmdFetch}, afero.NewMemMapFs())
 
 		assert.Equal(t, 1, runCLI(opts))
@@ -239,6 +272,8 @@ func TestRootVerbosityFlags(t *testing.T) {
 	})
 
 	t.Run("quiet before export hides the manifest summary", func(t *testing.T) {
+		t.Parallel()
+
 		fs := afero.NewMemMapFs()
 		manifest := buildTestManifest(t, map[string][]byte{"a.txt": []byte("a")})
 		require.NoError(t, afero.WriteFile(fs, testManifest, manifest, 0o644))
@@ -321,7 +356,7 @@ func TestHelpCommand(t *testing.T) {
 	t.Parallel()
 
 	fs := afero.NewMemMapFs()
-	opts := testOpts([]string{testApp, "--help"}, fs)
+	opts := testOpts([]string{testApp, testFlagHelp}, fs)
 
 	exitCode := runCLI(opts)
 
@@ -694,6 +729,33 @@ func TestCheckCommandWithSizeMismatch(t *testing.T) {
 		"check should have failed for size mismatch")
 }
 
+// TestCheckQuietShowsFailedFiles asserts that check -q prints nothing for a
+// tree that passes and still names each file that fails: --quiet hides
+// everything except errors, and a failed file is an error.
+func TestCheckQuietShowsFailedFiles(t *testing.T) {
+	t.Parallel()
+
+	fs := afero.NewMemMapFs()
+	require.NoError(t, fs.MkdirAll(testDir, 0o755))
+	writeTestFile(t, fs, testFile1, "hello world")
+
+	opts := testOpts([]string{testApp, cmdGenerate, "-q", "-o", testManifest, testDir}, fs)
+	require.Equal(t, 0, runCLI(opts), "generate failed: %s", testStderr(t, opts))
+
+	args := []string{testApp, cmdCheck, "-q", testFlagBase, testDir, testManifest}
+
+	opts = testOpts(args, fs)
+	require.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts))
+	assert.Empty(t, testStdout(t, opts))
+	assert.Empty(t, testStderr(t, opts))
+
+	writeTestFile(t, fs, testFile1, "HELLO WORLD")
+
+	opts = testOpts(args, fs)
+	assert.Equal(t, 1, runCLI(opts))
+	assert.Contains(t, testStderr(t, opts), "HASH_MISMATCH: file1.txt")
+}
+
 func TestBannerOutput(t *testing.T) {
 	t.Parallel()
 
@@ -712,6 +774,9 @@ func TestBannerOutput(t *testing.T) {
 	stdout := testStdout(t, opts)
 	assert.Contains(t, stdout, "___")
 	assert.Contains(t, stdout, "\\")
+
+	// The banner shows the version mfer version prints, and nothing after it.
+	assert.Contains(t, stdout, "mfer by @sneak: "+testGitrev+"\n")
 }
 
 func TestUnknownCommand(t *testing.T) {
@@ -744,13 +809,9 @@ func TestGenerateExcludesDotfilesByDefault(t *testing.T) {
 	assert.True(t, exists)
 
 	// Verify manifest only has 1 file (the non-dotfile)
-	manifest, err := mfer.NewManifestFromFile(&mfer.ManifestFromFileOptions{
-		Path: testMF,
-		Fs:   fs,
-	})
-	require.NoError(t, err)
-	assert.Len(t, manifest.Files(), 1)
-	assert.Equal(t, "file1.txt", manifest.Files()[0].GetPath())
+	paths := manifestPaths(t, fs, testMF)
+	require.Len(t, paths, 1)
+	assert.Equal(t, "file1.txt", paths[0])
 }
 
 func TestGenerateWithIncludeDotfiles(t *testing.T) {
@@ -771,12 +832,7 @@ func TestGenerateWithIncludeDotfiles(t *testing.T) {
 	require.Equal(t, 0, exitCode)
 
 	// Verify manifest has 2 files (including dotfile)
-	manifest, err := mfer.NewManifestFromFile(&mfer.ManifestFromFileOptions{
-		Path: testMF,
-		Fs:   fs,
-	})
-	require.NoError(t, err)
-	assert.Len(t, manifest.Files(), 2)
+	assert.Len(t, manifestPaths(t, fs, testMF), 2)
 }
 
 func TestMultipleInputPaths(t *testing.T) {
@@ -926,8 +982,56 @@ func TestCheckWarnsAboutUnlistedFiles(t *testing.T) {
 			assert.Equal(t, 1, exitCode, "stderr: %s", stderr)
 			assert.Contains(t, stderr, unlisted)
 
-			exitCode, _ = check("-q", testFlagNoExtra)
+			exitCode, stderr = check("-q", testFlagNoExtra)
 			assert.Equal(t, 1, exitCode)
+			assert.Contains(t, stderr, unlisted)
+		})
+	}
+}
+
+// TestCheckReportsUnlistedSymlinkToFileOnly makes a tree holding a symlink
+// to a directory, one to a file and a broken one. gen lists the symlink to
+// the file only with --follow-symlinks, and never lists the other two, so
+// check --no-extra-files passes on the manifest gen --follow-symlinks writes
+// and fails only on the symlink to the file on the one plain gen writes.
+func TestCheckReportsUnlistedSymlinkToFileOnly(t *testing.T) {
+	t.Parallel()
+
+	for name, tc := range map[string]struct {
+		genFlags []string
+		exitCode int
+		extras   []string // what check reports as not in the manifest
+	}{
+		"gen --follow-symlinks": {[]string{"--follow-symlinks"}, 0, nil},
+		"plain gen":             {nil, 1, []string{"filelink"}},
+	} {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			dir := t.TempDir()
+			fs := afero.NewOsFs()
+			writeTestTree(t, fs, dir)
+			require.NoError(t, os.Symlink(testSubdir, filepath.Join(dir, "dirlink")))
+			require.NoError(t, os.Symlink(testFileTxt, filepath.Join(dir, "filelink")))
+			require.NoError(t,
+				os.Symlink("nowhere", filepath.Join(dir, "brokenlink")))
+
+			opts := testOpts(slices.Concat(
+				[]string{testApp, cmdGenerate, "-q"}, tc.genFlags, []string{dir},
+			), fs)
+			require.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts))
+
+			opts = testOpts([]string{testApp, cmdCheck, testFlagNoExtra, dir}, fs)
+			exitCode := runCLI(opts)
+			stderr := testStderr(t, opts)
+
+			assert.Equal(t, tc.exitCode, exitCode, "stderr: %s", stderr)
+			assert.Equal(t, len(tc.extras),
+				strings.Count(stderr, "not in manifest"), stderr)
+
+			for _, extra := range tc.extras {
+				assert.Contains(t, stderr, "EXTRA: "+extra)
+			}
 		})
 	}
 }
@@ -1196,6 +1300,11 @@ func TestGenerateFailsWithoutForceWhenOutputExists(t *testing.T) {
 func manifestPaths(t *testing.T, fs afero.Fs, path string) []string {
 	t.Helper()
 
+	// Loading a manifest logs to the process-global logger, so serialize
+	// with the CLI runs.
+	runMu.Lock()
+	defer runMu.Unlock()
+
 	manifest, err := mfer.NewManifestFromFile(&mfer.ManifestFromFileOptions{
 		Path: path,
 		Fs:   fs,
@@ -1368,6 +1477,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 +1773,200 @@ 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())
+}
+
+// TestPathNamedHelp runs gen, then check with --require-signature, on a
+// directory named help and on one named h, the two names of urfave/cli's
+// help command. gen must write the manifest into the directory, and check
+// must refuse it as unsigned.
+//
+//nolint:paralleltest // changes the process-global working directory
+func TestPathNamedHelp(t *testing.T) {
+	for _, name := range []string{testCmdHelp, "h"} {
+		t.Run(name, func(t *testing.T) {
+			chdirTemp(t)
+
+			fs := afero.NewOsFs()
+			require.NoError(t, fs.Mkdir(name, 0o755))
+			writeTestFile(t, fs, filepath.Join(name, testFileTxt), "content")
+
+			opts := testOpts([]string{testApp, cmdGenerate, "-q", name}, fs)
+			require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
+
+			files := manifestFiles(t, fs, filepath.Join(name, defaultManifestName))
+			require.Len(t, files, 1)
+
+			opts = testOpts([]string{
+				testApp, cmdCheck, "-q", name, "--" + flagRequireSignature, msgFpA,
+			}, fs)
+			assert.Equal(t, 1, runCLI(opts))
+			assert.Contains(t, testStderr(t, opts), "manifest is not signed")
+		})
+	}
+}
+
+// TestCommandsReadHelpAsArgument runs each command that takes a path or URL
+// on help and on h where nothing by that name exists. Each must fail on the
+// missing path or bad URL instead of printing its help.
+func TestCommandsReadHelpAsArgument(t *testing.T) {
+	t.Parallel()
+
+	for _, command := range []string{
+		cmdGenerate, cmdCheck, cmdFreshen, cmdList, cmdExport, cmdFetch,
+	} {
+		for _, name := range []string{testCmdHelp, "h"} {
+			t.Run(command+" "+name, func(t *testing.T) {
+				t.Parallel()
+
+				opts := testOpts([]string{testApp, command, name}, afero.NewMemMapFs())
+				assert.Equal(t, 1, runCLI(opts))
+				assert.NotContains(t, testStdout(t, opts), "USAGE:")
+			})
+		}
+	}
+}
+
+// TestHelpBesideArgument runs each command that takes a path or URL with
+// --help or -h before and after a directory, and runs help on that command.
+// Each must print what the command's own --help prints.
+func TestHelpBesideArgument(t *testing.T) {
+	t.Parallel()
+
+	for _, command := range []string{
+		cmdGenerate, cmdCheck, cmdFreshen, cmdList, cmdExport, cmdFetch,
+	} {
+		want := testOpts([]string{testApp, command, testFlagHelp}, afero.NewMemMapFs())
+		require.Equal(t, 0, runCLI(want))
+		require.Contains(t, testStdout(t, want), testApp+" "+command)
+
+		for _, args := range [][]string{
+			{command, testDir, testFlagHelp},
+			{command, testFlagHelp, testDir},
+			{command, testDir, "-h"},
+			{testCmdHelp, command},
+		} {
+			t.Run(strings.Join(args, " "), func(t *testing.T) {
+				t.Parallel()
+
+				opts := testOpts(slices.Concat([]string{testApp}, args), afero.NewMemMapFs())
+				assert.Equal(t, 0, runCLI(opts), testStderr(t, opts))
+				assert.Equal(t, testStdout(t, want), testStdout(t, opts))
+			})
+		}
+	}
 }
 
 func TestCheckDetectsManifestCorruption(t *testing.T) {

internal/cli/errmsg_test.go

@@ -5,11 +5,11 @@ import (
 	"bytes"
 	"context"
 	"encoding/hex"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"os"
 	"path/filepath"
-	"slices"
 	"strings"
 	"testing"
 
@@ -54,6 +54,11 @@ func runLocked(fn func() error) error {
 func unsignedChecker(t *testing.T) *mfer.Checker {
 	t.Helper()
 
+	// The scan and the manifest load log to the process-global logger, so
+	// serialize with the CLI runs.
+	runMu.Lock()
+	defer runMu.Unlock()
+
 	fs := afero.NewMemMapFs()
 	require.NoError(t, fs.MkdirAll("/d", 0o755))
 	require.NoError(t, afero.WriteFile(fs, "/d/f.txt", []byte("hi"), 0o644))
@@ -128,13 +133,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 +166,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})
@@ -177,6 +188,11 @@ func signedManifest(t *testing.T, files map[string][]byte) []byte {
 func signedChecker(t *testing.T, manifest []byte) *mfer.Checker {
 	t.Helper()
 
+	// Loading a manifest logs to the process-global logger, so serialize
+	// with the CLI runs.
+	runMu.Lock()
+	defer runMu.Unlock()
+
 	fs := afero.NewMemMapFs()
 	require.NoError(t, afero.WriteFile(fs, "/index.mf", manifest, 0o644))
 
@@ -209,10 +225,26 @@ func manifestSignedByAnotherKey(
 	require.NoError(t, proto.Unmarshal(
 		signedManifest(t, files)[len(mfer.MAGIC):], outer))
 
-	// Armored blocks start on a line of their own; mfer, unlike gpg, ends
-	// one without a newline.
-	outer.SigningPubKey = slices.Concat(
-		required.GetSigningPubKey(), []byte("\n"), outer.GetSigningPubKey())
+	// One armored block holding both keys, as gpg --export --armor writes
+	// two keys.
+	var block bytes.Buffer
+
+	w, err := armor.Encode(&block, openpgp.PublicKeyType, nil)
+	require.NoError(t, err)
+
+	for _, key := range [][]byte{
+		required.GetSigningPubKey(), outer.GetSigningPubKey(),
+	} {
+		decoded, err := armor.Decode(bytes.NewReader(key))
+		require.NoError(t, err)
+
+		_, err = io.Copy(w, decoded.Body)
+		require.NoError(t, err)
+	}
+
+	require.NoError(t, w.Close())
+
+	outer.SigningPubKey = block.Bytes()
 	outer.Signer = required.GetSigner()
 
 	data, err := proto.Marshal(outer)
@@ -451,6 +483,49 @@ func TestFreshenReadErrorMessage(t *testing.T) {
 		"read "+filepath.Join(root, "sub")+": is a directory")
 }
 
+// TestFreshenSizeChangedMessage has freshen hash a file that has grown or
+// shrunk since the scan recorded its size.
+func TestFreshenSizeChangedMessage(t *testing.T) {
+	t.Parallel()
+
+	fs := afero.NewMemMapFs()
+	writeTestFile(t, fs, "/d/log.txt", "hello")
+
+	t.Run("grown", func(t *testing.T) {
+		t.Parallel()
+
+		hasher := &freshenHasher{
+			fs:      fs,
+			absBase: "/d",
+			builder: mfer.NewBuilder(),
+		}
+
+		err := hasher.processEntry(
+			&freshenEntry{path: "log.txt", size: 3, needsHash: true})
+		require.ErrorIs(t, err, errSizeMismatch)
+		assert.EqualError(t, err, `size mismatch for "log.txt": `+
+			`3 bytes when scanned but 5 bytes when read; `+
+			`the file changed while freshen ran`)
+	})
+
+	t.Run("shrunk", func(t *testing.T) {
+		t.Parallel()
+
+		hasher := &freshenHasher{
+			fs:      fs,
+			absBase: "/d",
+			builder: mfer.NewBuilder(),
+		}
+
+		err := hasher.processEntry(
+			&freshenEntry{path: "log.txt", size: 8, needsHash: true})
+		require.ErrorIs(t, err, errSizeMismatch)
+		assert.EqualError(t, err, `size mismatch for "log.txt": `+
+			`8 bytes when scanned but 5 bytes when read; `+
+			`the file changed while freshen ran`)
+	})
+}
+
 func TestURLRequiredMessage(t *testing.T) {
 	t.Parallel()
 

internal/cli/export_test.go

@@ -20,6 +20,11 @@ const testCmdExport = "export"
 func buildTestManifest(t *testing.T, files map[string][]byte) []byte {
 	t.Helper()
 
+	// The scan logs to the process-global logger, so serialize with the
+	// CLI runs.
+	runMu.Lock()
+	defer runMu.Unlock()
+
 	sourceFs := afero.NewMemMapFs()
 	for path, content := range files {
 		require.NoError(t, sourceFs.MkdirAll("/", 0o755))

internal/cli/fetch.go

@@ -86,7 +86,8 @@ var (
 	// symlink that already exists in the target directory.
 	errSymlinkInPath = errors.New("symlink in path not allowed")
 	// errSizeMismatch indicates a downloaded file with an unexpected
-	// size.
+	// size, or a file freshen read at a size other than the one its scan
+	// recorded.
 	errSizeMismatch = errors.New("size mismatch")
 	// errHashMismatch indicates a downloaded file whose hash matches no
 	// manifest hash.
@@ -429,6 +430,14 @@ func (mfa *CLIApp) fetchManifestOperation(
 func (mfa *CLIApp) fetchManifest(
 	ctx context.Context, cmd *cli.Command, client retryingClient, manifestURL string,
 ) ([]byte, []*mfer.MFFilePath, error) {
+	// An empty fingerprint is refused before anything is requested, with
+	// the message check gives for it; any other is checked against the
+	// manifest once it is downloaded.
+	requiredSigner := cmd.String(flagRequireSignature)
+	if cmd.IsSet(flagRequireSignature) && requiredSigner == "" {
+		return nil, nil, fmt.Errorf("%w, got 0", errInvalidFingerprint)
+	}
+
 	log.Infof("fetching manifest from %s", manifestURL)
 
 	// Read the whole manifest before parsing it, so that a connection
@@ -460,7 +469,6 @@ func (mfa *CLIApp) fetchManifest(
 		return nil, nil, fmt.Errorf("parse manifest: %w", err)
 	}
 
-	requiredSigner := cmd.String(flagRequireSignature)
 	if requiredSigner != "" {
 		err = verifyFetchedSigner(manifestData, requiredSigner)
 		if err != nil {
@@ -679,21 +687,26 @@ func resolveManifestURL(inputURL string) (string, error) {
 	return parsed.JoinPath(defaultManifestName).String(), nil
 }
 
-// progressWriter wraps an io.Writer and reports progress to a channel.
+// progressWriter wraps an io.Writer and reports progress to a channel,
+// at most once per second and once more when the last byte is written.
 type progressWriter struct {
-	w         io.Writer
-	path      string
-	total     int64
-	written   int64
-	startTime time.Time
-	progress  chan<- DownloadProgress
+	w            io.Writer
+	path         string
+	total        int64
+	written      int64
+	startTime    time.Time
+	lastProgress time.Time
+	progress     chan<- DownloadProgress
 }
 
 func (pw *progressWriter) Write(p []byte) (int, error) {
 	n, err := pw.w.Write(p)
 
 	pw.written += int64(n)
-	if pw.progress != nil {
+
+	isLast := pw.written == pw.total
+	if pw.progress != nil &&
+		(isLast || time.Since(pw.lastProgress) >= time.Second) {
 		var (
 			bytesPerSec float64
 			eta         time.Duration
@@ -715,6 +728,8 @@ func (pw *progressWriter) Write(p []byte) (int, error) {
 			BytesPerSec: bytesPerSec,
 			ETA:         eta,
 		})
+
+		pw.lastProgress = time.Now()
 	}
 
 	return n, err
@@ -899,13 +914,16 @@ func saveResponse(
 	// Set up hash computation
 	h := sha256.New()
 
-	// Create progress-reporting writer that also computes hash
+	// Create progress-reporting writer that also computes hash. The first
+	// update waits a second, as gen's and check's do.
+	start := time.Now()
 	pw := &progressWriter{
-		w:         io.MultiWriter(out, h),
-		path:      localPath,
-		total:     totalBytes,
-		startTime: time.Now(),
-		progress:  progress,
+		w:            io.MultiWriter(out, h),
+		path:         localPath,
+		total:        totalBytes,
+		startTime:    start,
+		lastProgress: start,
+		progress:     progress,
 	}
 
 	// Copy content while hashing and reporting progress. One byte past

internal/cli/fetch_test.go

@@ -171,6 +171,11 @@ func TestResolveManifestURL(t *testing.T) {
 func scanToManifest(t *testing.T, sourceFs afero.Fs) []byte {
 	t.Helper()
 
+	// The scan logs to the process-global logger, so serialize with the
+	// CLI runs.
+	runMu.Lock()
+	defer runMu.Unlock()
+
 	s := mfer.NewScannerWithOptions(&mfer.ScannerOptions{Fs: sourceFs})
 	require.NoError(t, s.EnumerateFS(sourceFs, "/", nil))
 
@@ -314,8 +319,11 @@ func TestFetchFromHTTP(t *testing.T) {
 	// Change to a fresh destination directory for the test
 	destDir := chdirTemp(t)
 
-	// Parse the manifest to get file entries
+	// Parse the manifest to get file entries. Loading a manifest logs to
+	// the process-global logger.
+	runMu.Lock()
 	manifest, err := mfer.NewManifestFromReader(bytes.NewReader(manifestData))
+	runMu.Unlock()
 	require.NoError(t, err)
 
 	files := manifest.Files()
@@ -365,7 +373,10 @@ func TestFetchHashMismatch(t *testing.T) {
 	// Generate and parse manifest
 	manifestData := scanToManifest(t, sourceFs)
 
+	// Loading a manifest logs to the process-global logger.
+	runMu.Lock()
 	manifest, err := mfer.NewManifestFromReader(bytes.NewReader(manifestData))
+	runMu.Unlock()
 	require.NoError(t, err)
 
 	files := manifest.Files()
@@ -411,7 +422,10 @@ func TestFetchSizeMismatch(t *testing.T) {
 	// Generate and parse manifest
 	manifestData := scanToManifest(t, sourceFs)
 
+	// Loading a manifest logs to the process-global logger.
+	runMu.Lock()
 	manifest, err := mfer.NewManifestFromReader(bytes.NewReader(manifestData))
+	runMu.Unlock()
 	require.NoError(t, err)
 
 	files := manifest.Files()
@@ -516,14 +530,17 @@ func TestManifestDownloadStopsAtLimit(t *testing.T) {
 func TestFetchProgress(t *testing.T) {
 	// Create source filesystem with a larger test file
 	sourceFs := afero.NewMemMapFs()
-	// Create content large enough to trigger multiple progress updates
+	// Content large enough that io.Copy writes it in at least four chunks
 	content := bytes.Repeat([]byte("x"), 100*1024) // 100KB
 	require.NoError(t, afero.WriteFile(sourceFs, "/large.txt", content, 0o644))
 
 	// Generate and parse manifest
 	manifestData := scanToManifest(t, sourceFs)
 
+	// Loading a manifest logs to the process-global logger.
+	runMu.Lock()
 	manifest, err := mfer.NewManifestFromReader(bytes.NewReader(manifestData))
+	runMu.Unlock()
 	require.NoError(t, err)
 
 	files := manifest.Files()
@@ -559,8 +576,11 @@ func TestFetchProgress(t *testing.T) {
 	}()
 
 	// Download
+	start := time.Now()
 	err = downloadFile(context.Background(), testClient(),
 		server.URL+"/large.txt", ".", "large.txt", files[0], progress)
+	elapsed := time.Since(start)
+
 	close(progress)
 	<-done
 
@@ -569,6 +589,12 @@ func TestFetchProgress(t *testing.T) {
 	// Verify we got progress updates
 	assert.NotEmpty(t, progressUpdates, "should have received progress updates")
 
+	// At most one update per second, plus the one reporting the file
+	// complete: a download that takes under a second sends two at most,
+	// however many chunks it writes.
+	assert.LessOrEqual(t, len(progressUpdates), 2+int(elapsed/time.Second),
+		"one progress update per second at most, plus the last")
+
 	// Verify final progress shows complete
 	if len(progressUpdates) > 0 {
 		last := progressUpdates[len(progressUpdates)-1]
@@ -897,9 +923,12 @@ func TestGetStopsWaitingWhenCanceled(t *testing.T) {
 //nolint:paralleltest // changes the process-global working directory
 func TestDownloadFileRetriesToSuccess(t *testing.T) {
 	content := []byte("the whole file, every byte of it")
+	manifestData := manifestOf(t, map[string][]byte{testFileTxt: content})
 
-	manifest, err := mfer.NewManifestFromReader(bytes.NewReader(
-		manifestOf(t, map[string][]byte{testFileTxt: content})))
+	// Loading a manifest logs to the process-global logger.
+	runMu.Lock()
+	manifest, err := mfer.NewManifestFromReader(bytes.NewReader(manifestData))
+	runMu.Unlock()
 	require.NoError(t, err)
 
 	var requests atomic.Int32
@@ -922,8 +951,11 @@ func TestDownloadFileRetriesToSuccess(t *testing.T) {
 
 	chdirTemp(t)
 
-	err = downloadFile(context.Background(), testClient(),
-		server.URL+"/"+testFileTxt, ".", testFileTxt, manifest.Files()[0], nil)
+	// downloadFile logs each retry to the process-global logger.
+	err = runLocked(func() error {
+		return downloadFile(context.Background(), testClient(),
+			server.URL+"/"+testFileTxt, ".", testFileTxt, manifest.Files()[0], nil)
+	})
 	require.NoError(t, err)
 	assert.Equal(t, int32(3), requests.Load())
 
@@ -1243,10 +1275,10 @@ func TestFetchRequireSignature(t *testing.T) {
 
 // TestFetchRefusesListedManifestName fetches manifests that list, at the
 // top of the tree, the name fetch saves the manifest under or that name's
-// temp file: as a file, as a directory, in capitals, and with a leading
-// "./". Saving the manifest would replace or remove what is listed there,
-// so fetch must refuse the manifest before it creates the destination or
-// requests any file.
+// temp file: as a file, as a directory and in capitals. Saving the
+// manifest would replace or remove what is listed there, so fetch must
+// refuse the manifest before it creates the destination or requests any
+// file.
 func TestFetchRefusesListedManifestName(t *testing.T) {
 	t.Parallel()
 
@@ -1255,7 +1287,6 @@ func TestFetchRefusesListedManifestName(t *testing.T) {
 		tempPathFor(defaultManifestName),
 		defaultManifestName + "/" + testFileTxt,
 		"INDEX.MF",
-		"./" + defaultManifestName,
 	} {
 		t.Run(listed, func(t *testing.T) {
 			t.Parallel()
@@ -1331,9 +1362,9 @@ func TestFetchRefusesListedTempName(t *testing.T) {
 // directory, one of them under a spelling the manifest does not list,
 // and check reports that file as not in the manifest. So fetch must
 // refuse each on every filesystem before it creates the destination or
-// requests any file. A file and a directory with the same name, and a
-// file listed twice, are refused the same way. A manifest whose names
-// differ in more than letter case is fetched in full.
+// requests any file. A file and a directory with the same name are
+// refused the same way. A manifest whose names differ in more than
+// letter case is fetched in full.
 func TestFetchRefusesNamesEqualIgnoringCase(t *testing.T) {
 	t.Parallel()
 
@@ -1343,7 +1374,6 @@ func TestFetchRefusesNamesEqualIgnoringCase(t *testing.T) {
 		{"Dir", "dir/x", "dir/x (the file Dir)"},
 		{"Dir/a.txt", "dir/b.txt", "dir/b.txt (the directory Dir)"},
 		{"dir", "dir/x", "dir/x (the file dir)"},
-		{"./x.txt", "x.txt", "x.txt (the file ./x.txt)"},
 	} {
 		t.Run(tc.first+" and "+tc.second, func(t *testing.T) {
 			t.Parallel()
@@ -1404,14 +1434,18 @@ func TestFetchSetsRecordedMode(t *testing.T) {
 		require.NoError(t, afero.WriteFile(sourceFs, "/"+p, content, modes[p]))
 	}
 
-	scanner := mfer.NewScannerWithOptions(&mfer.ScannerOptions{
-		Fs:                 sourceFs,
-		IncludePermissions: true,
-	})
-	require.NoError(t, scanner.EnumerateFS(sourceFs, "/", nil))
-
 	var manifest bytes.Buffer
-	require.NoError(t, scanner.ToManifest(context.Background(), &manifest, nil))
+
+	// The scan logs to the process-global logger.
+	require.NoError(t, runLocked(func() error {
+		scanner := mfer.NewScannerWithOptions(&mfer.ScannerOptions{
+			Fs:                 sourceFs,
+			IncludePermissions: true,
+		})
+		require.NoError(t, scanner.EnumerateFS(sourceFs, "/", nil))
+
+		return scanner.ToManifest(context.Background(), &manifest, nil)
+	}))
 
 	tree := fetchTestHandler(manifest.Bytes(), files)
 
@@ -1573,8 +1607,7 @@ func manifestWithMode(t *testing.T, path string, content []byte, mode uint32) []
 }
 
 // builtManifest returns a manifest of files, built directly rather than
-// scanned, since a scan lists no hidden files and never a path starting
-// with "./".
+// scanned, since a scan lists no hidden files.
 func builtManifest(t *testing.T, files map[string][]byte) []byte {
 	t.Helper()
 

internal/cli/freshen.go

@@ -28,9 +28,19 @@ const (
 	scanProgressInterval = 100
 )
 
-// errEntryMissingMtime indicates a manifest entry that carries no
-// modification time where one is required to carry it forward unchanged.
-var errEntryMissingMtime = errors.New("manifest entry has no mtime")
+var (
+	// errEntryMissingMtime indicates a manifest entry that carries no
+	// modification time where one is required to carry it forward unchanged.
+	errEntryMissingMtime = errors.New("manifest entry has no mtime")
+
+	// errEntryMissingHash indicates a manifest entry that carries no hash
+	// to carry forward.
+	errEntryMissingHash = errors.New("manifest entry has no hash")
+)
+
+// errSignKeyNeeded indicates a signed manifest given to freshen with no
+// key to sign the rewritten one.
+var errSignKeyNeeded = errors.New("give --sign-key to keep it signed")
 
 // FreshenStatus contains progress information for the freshen operation.
 type FreshenStatus struct {
@@ -192,6 +202,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
@@ -298,6 +315,16 @@ func (h *freshenHasher) processEntry(e *freshenEntry) error {
 		return err
 	}
 
+	// The scan recorded the size; a file that grew or shrank since would
+	// get an entry whose size and hash describe different contents.
+	if bytesRead != e.size {
+		return fmt.Errorf(
+			"%w for %q: %d bytes when scanned but %d bytes when read; "+
+				"the file changed while freshen ran",
+			errSizeMismatch, e.path, e.size, bytesRead,
+		)
+	}
+
 	h.hashedBytes += bytesRead
 	h.hashedFiles++
 
@@ -338,7 +365,7 @@ func writeFreshenedManifest(
 }
 
 // newFreshenBuilder constructs the manifest builder configured from CLI
-// flags.
+// flags. A --sign-key given with an empty value is refused.
 func (mfa *CLIApp) newFreshenBuilder(cmd *cli.Command) (*mfer.Builder, error) {
 	builder := mfer.NewBuilder()
 	if cmd.Bool("include-timestamps") {
@@ -346,7 +373,12 @@ func (mfa *CLIApp) newFreshenBuilder(cmd *cli.Command) (*mfer.Builder, error) {
 	}
 
 	// Set up signing options if sign-key is provided
-	if signKey := cmd.String("sign-key"); signKey != "" {
+	if cmd.IsSet("sign-key") {
+		signKey := cmd.String("sign-key")
+		if signKey == "" {
+			return nil, errEmptySignKey
+		}
+
 		signing, err := mfa.signingOptions(signKey)
 		if err != nil {
 			return nil, err
@@ -381,6 +413,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 +432,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()
@@ -460,9 +500,10 @@ func runFreshenHash(
 }
 
 // loadExistingEntries loads the manifest and indexes its file entries
-// by path.
+// by path. It refuses a signed manifest when no --sign-key is given, as
+// the rewritten manifest would be unsigned.
 func (mfa *CLIApp) loadExistingEntries(
-	manifestPath string,
+	cmd *cli.Command, manifestPath string,
 ) (map[string]*mfer.MFFilePath, error) {
 	log.Infof("loading manifest from %s", manifestPath)
 
@@ -475,6 +516,10 @@ func (mfa *CLIApp) loadExistingEntries(
 		return nil, fmt.Errorf("load manifest: %w", err)
 	}
 
+	if signer := manifest.Signer(); signer != nil && cmd.String("sign-key") == "" {
+		return nil, fmt.Errorf("manifest is signed by %s: %w", signer, errSignKeyNeeded)
+	}
+
 	existingFiles := manifest.Files()
 	log.Infof("manifest contains %d files", len(existingFiles))
 
@@ -505,7 +550,7 @@ func (mfa *CLIApp) freshenManifestOperation(
 		return err
 	}
 
-	existingByPath, err := mfa.loadExistingEntries(manifestPath)
+	existingByPath, err := mfa.loadExistingEntries(cmd, manifestPath)
 	if err != nil {
 		return err
 	}
@@ -629,6 +674,8 @@ func addFileToBuilder(
 // Entries reach this path only when recordEntry classified them as
 // unchanged, which requires a recorded mtime, so an absent mtime here is
 // an error rather than something to paper over with the Unix epoch.
+// Loading refuses an entry with no hash; one that got here anyway is an
+// error too, not a file to leave out of the manifest.
 func addExistingToBuilder(b *mfer.Builder, entry *mfer.MFFilePath) error {
 	mtime, ok := entryMtime(entry)
 	if !ok {
@@ -636,7 +683,7 @@ func addExistingToBuilder(b *mfer.Builder, entry *mfer.MFFilePath) error {
 	}
 
 	if len(entry.GetHashes()) == 0 {
-		return nil
+		return fmt.Errorf("%w: %s", errEntryMissingHash, entry.GetPath())
 	}
 
 	err := b.AddFileWithHash(mfer.RelFilePath(entry.GetPath()),

internal/cli/freshen_test.go

@@ -70,6 +70,11 @@ func runFreshen(t *testing.T, fs afero.Fs, root, manifestPath string) {
 func manifestFiles(t *testing.T, fs afero.Fs, path string) []*mfer.MFFilePath {
 	t.Helper()
 
+	// Loading a manifest logs to the process-global logger, so serialize
+	// with the CLI runs.
+	runMu.Lock()
+	defer runMu.Unlock()
+
 	manifest, err := mfer.NewManifestFromFile(&mfer.ManifestFromFileOptions{
 		Path: path,
 		Fs:   fs,
@@ -243,6 +248,58 @@ func TestFreshenWithChanges(t *testing.T) {
 	}
 }
 
+// growingFileFs appends to one file each time it is opened, as a program
+// still writing to the file would between freshen's scan and its hash.
+type growingFileFs struct {
+	afero.Fs
+
+	path string
+}
+
+//nolint:ireturn // Open must return afero.File to satisfy afero.Fs.
+func (f growingFileFs) Open(name string) (afero.File, error) {
+	if name == f.path {
+		file, err := f.OpenFile(name, os.O_APPEND|os.O_WRONLY, 0)
+		if err != nil {
+			return nil, err
+		}
+
+		_, err = file.WriteString(" world")
+		_ = file.Close()
+
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	return f.Fs.Open(name)
+}
+
+// TestFreshenFileChangedSinceScan has a new file grow after freshen's scan
+// recorded its size and before freshen hashes it: freshen must fail and
+// leave the manifest as it was.
+func TestFreshenFileChangedSinceScan(t *testing.T) {
+	t.Parallel()
+
+	osFs := afero.NewOsFs()
+	root, manifestPath := setupFreshenDir(t, osFs,
+		map[string]string{testFileTxt: "content"})
+	grown := filepath.Join(root, "log.txt")
+	writeTestFile(t, osFs, grown, "hello")
+
+	before, err := afero.ReadFile(osFs, manifestPath)
+	require.NoError(t, err)
+
+	opts := testOpts([]string{
+		testApp, cmdFreshen, "-q", testFlagBase, root, manifestPath,
+	}, growingFileFs{Fs: osFs, path: grown})
+	assert.Equal(t, 1, runCLI(opts), "stderr: %s", testStderr(t, opts))
+
+	after, err := afero.ReadFile(osFs, manifestPath)
+	require.NoError(t, err)
+	assert.Equal(t, before, after, "manifest changed")
+}
+
 // TestFreshenLeavesManifestOutOfListing freshens a manifest kept in a
 // subdirectory of the tree it lists: the manifest is not listed, while an
 // ordinary file of the same name at the top of the tree is. The manifest
@@ -336,6 +393,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.
@@ -393,7 +493,11 @@ func TestFreshenRecordEntryMtimePresence(t *testing.T) {
 			s := &freshenScanner{
 				existingByPath: map[string]*mfer.MFFilePath{relPath: tc.entry},
 			}
+
+			// recordEntry logs to the process-global logger.
+			runMu.Lock()
 			s.recordEntry(relPath, info)
+			runMu.Unlock()
 
 			require.Len(t, s.entries, 1)
 			assert.Equal(t, tc.needsHash, s.entries[0].needsHash)
@@ -428,6 +532,24 @@ func TestFreshenAddExistingRejectsMissingMtime(t *testing.T) {
 	assert.Contains(t, err.Error(), "file1.txt")
 }
 
+// TestFreshenAddExistingRejectsMissingHash pins that an entry with no hash
+// is an error rather than a file left out of the rebuilt manifest.
+func TestFreshenAddExistingRejectsMissingHash(t *testing.T) {
+	t.Parallel()
+
+	b := mfer.NewBuilder()
+	entry := &mfer.MFFilePath{
+		Path:  "data.bin",
+		Size:  8,
+		Mtime: &mfer.Timestamp{Seconds: 1_700_000_000},
+	}
+
+	err := addExistingToBuilder(b, entry)
+	require.ErrorIs(t, err, errEntryMissingHash)
+	assert.Contains(t, err.Error(), "data.bin")
+	assert.Zero(t, b.FileCount())
+}
+
 // TestFreshenAddExistingRejectsShortHash pins that an existing manifest
 // entry whose hash the builder refuses is reported as a problem with the
 // manifest, with the command that fixes it.

internal/cli/gen.go

@@ -29,6 +29,8 @@ var (
 		"already exists (use --force to overwrite)")
 	// errEmptyOutput indicates --output given with an empty value.
 	errEmptyOutput = errors.New("--output must not be empty")
+	// errEmptySignKey indicates --sign-key given with an empty value.
+	errEmptySignKey = errors.New("--sign-key must not be empty")
 )
 
 // reportEnumProgress renders enumeration progress until the channel
@@ -121,7 +123,8 @@ func (mfa *CLIApp) outputPath(cmd *cli.Command) (string, error) {
 }
 
 // buildScannerOptions constructs scanner options from the CLI flags and
-// the path the manifest is written to.
+// the path the manifest is written to. A --sign-key given with an empty
+// value is refused.
 func (mfa *CLIApp) buildScannerOptions(
 	cmd *cli.Command, output string,
 ) (*mfer.ScannerOptions, error) {
@@ -144,7 +147,12 @@ func (mfa *CLIApp) buildScannerOptions(
 	}
 
 	// Set up signing options if sign-key is provided
-	if signKey := cmd.String("sign-key"); signKey != "" {
+	if cmd.IsSet("sign-key") {
+		signKey := cmd.String("sign-key")
+		if signKey == "" {
+			return nil, errEmptySignKey
+		}
+
 		signing, err := mfa.signingOptions(signKey)
 		if err != nil {
 			return nil, err

internal/cli/mfer.go

@@ -6,12 +6,12 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"runtime/debug"
 	"time"
 
 	"github.com/spf13/afero"
 	"github.com/urfave/cli/v3"
 	"sneak.berlin/go/mfer/internal/log"
-	"sneak.berlin/go/mfer/mfer"
 )
 
 // Command and flag names shared across command definitions and tests.
@@ -48,13 +48,16 @@ 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.
 //
 //nolint:revive // established name used throughout the codebase and tests
 type CLIApp struct {
 	appname     string
-	version     string
 	gitrev      string
 	startupTime time.Time
 	exitCode    int
@@ -83,13 +86,26 @@ const banner = `
    \  \:\        \  \:\      \  \::/       \  \:\
     \__\/         \__\/       \__\/         \__\/`
 
-// VersionString returns the version and git revision formatted for display.
+// VersionString returns the version mfer reports: the revision the build
+// stamped into main.Gitrev or, when it stamped none, as with `go install`,
+// the main module's version from the binary's build information.
 func (mfa *CLIApp) VersionString() string {
 	if mfa.gitrev != "" {
-		return fmt.Sprintf("%s (%s)", mfer.Version, mfa.gitrev)
+		return mfa.gitrev
 	}
 
-	return mfer.Version
+	return moduleVersion(debug.ReadBuildInfo())
+}
+
+// moduleVersion returns the main module's version from the binary's build
+// information, or "unknown" when the binary has none or it holds no version,
+// as in a build with GO111MODULE=off.
+func moduleVersion(info *debug.BuildInfo, ok bool) string {
+	if !ok || info.Main.Version == "" {
+		return "unknown"
+	}
+
+	return info.Main.Version
 }
 
 // printVersion writes the version line shared by the --version flag and the
@@ -102,8 +118,7 @@ func (mfa *CLIApp) printBanner() {
 	if log.GetLevel() <= log.InfoLevel {
 		_, _ = fmt.Fprintln(mfa.Stdout, banner)
 		_, _ = fmt.Fprintf(mfa.Stdout,
-			"        mfer by @sneak: v%s released %s\n",
-			mfer.Version, mfer.ReleaseDate)
+			"        mfer by @sneak: %s\n", mfa.VersionString())
 		_, _ = fmt.Fprintln(mfa.Stdout, "            https://sneak.berlin/go/mfer")
 	}
 }
@@ -154,13 +169,25 @@ 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
+}
+
+// showCommandHelp is the CommandNotFound of the commands that take a path or
+// URL. Those commands set HideHelpCommand, so that a path or URL named help or
+// h is an argument rather than urfave/cli's help command; each sets it itself,
+// because on the root it would also remove `mfer help`. With --help,
+// urfave/cli looks the command's first argument up as a help topic, so
+// `check DIR --help` asks for a topic DIR and finds none; this prints the
+// command's own help instead, as `check --help` does.
+func showCommandHelp(ctx context.Context, cmd *cli.Command, _ string) {
+	_ = cli.ShowCommandHelp(ctx, cmd.Root(), cmd.Name)
 }
 
 // requireSignatureFlag returns the --require-signature flag taken by the
@@ -186,11 +213,12 @@ 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 ...]",
+		HideHelpCommand: true,
+		CommandNotFound: showCommandHelp,
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			mfa.setVerbosity(cmd)
 			mfa.printBanner()
@@ -249,10 +277,12 @@ func (mfa *CLIApp) generateCommand() *cli.Command {
 
 func (mfa *CLIApp) checkCommand() *cli.Command {
 	return &cli.Command{
-		Name:         cmdCheck,
-		Usage:        "Validate files using manifest file",
-		ArgsUsage:    manifestArgsUsage,
-		StopOnNthArg: stopOnFirstArg(),
+		Name:            cmdCheck,
+		Usage:           "Validate files using manifest file",
+		ArgsUsage:       manifestArgsUsage,
+		ArgValidator:    refuseExtraArgs,
+		HideHelpCommand: true,
+		CommandNotFound: showCommandHelp,
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			mfa.setVerbosity(cmd)
 			mfa.printBanner()
@@ -283,10 +313,12 @@ func (mfa *CLIApp) checkCommand() *cli.Command {
 
 func (mfa *CLIApp) freshenCommand() *cli.Command {
 	return &cli.Command{
-		Name:         cmdFreshen,
-		Usage:        "Update manifest with changed, new, and removed files",
-		ArgsUsage:    manifestArgsUsage,
-		StopOnNthArg: stopOnFirstArg(),
+		Name:            cmdFreshen,
+		Usage:           "Update manifest with changed, new, and removed files",
+		ArgsUsage:       manifestArgsUsage,
+		ArgValidator:    refuseExtraArgs,
+		HideHelpCommand: true,
+		CommandNotFound: showCommandHelp,
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			mfa.setVerbosity(cmd)
 			mfa.printBanner()
@@ -334,10 +366,12 @@ func (mfa *CLIApp) freshenCommand() *cli.Command {
 
 func (mfa *CLIApp) exportCommand() *cli.Command {
 	return &cli.Command{
-		Name:         cmdExport,
-		Usage:        "Export manifest contents as JSON",
-		ArgsUsage:    "[manifest file or URL]",
-		StopOnNthArg: stopOnFirstArg(),
+		Name:            cmdExport,
+		Usage:           "Export manifest contents as JSON",
+		ArgsUsage:       "[manifest file or URL]",
+		ArgValidator:    refuseExtraArgs,
+		HideHelpCommand: true,
+		CommandNotFound: showCommandHelp,
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			mfa.setVerbosity(cmd)
 
@@ -348,9 +382,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()
 
@@ -361,12 +394,14 @@ func (mfa *CLIApp) versionCommand() *cli.Command {
 
 func (mfa *CLIApp) listCommand() *cli.Command {
 	return &cli.Command{
-		Name:         cmdList,
-		Aliases:      []string{"ls"},
-		Usage:        "List files in manifest",
-		ArgsUsage:    manifestArgsUsage,
-		StopOnNthArg: stopOnFirstArg(),
-		Action:       mfa.listManifestOperation,
+		Name:            cmdList,
+		Aliases:         []string{"ls"},
+		Usage:           "List files in manifest",
+		ArgsUsage:       manifestArgsUsage,
+		ArgValidator:    refuseExtraArgs,
+		HideHelpCommand: true,
+		CommandNotFound: showCommandHelp,
+		Action:          mfa.listManifestOperation,
 		Flags: []cli.Flag{
 			&cli.BoolFlag{
 				Name:    "long",
@@ -383,10 +418,12 @@ func (mfa *CLIApp) listCommand() *cli.Command {
 
 func (mfa *CLIApp) fetchCommand() *cli.Command {
 	return &cli.Command{
-		Name:         cmdFetch,
-		Usage:        "fetch manifest and referenced files",
-		ArgsUsage:    "URL",
-		StopOnNthArg: stopOnFirstArg(),
+		Name:            cmdFetch,
+		Usage:           "fetch manifest and referenced files",
+		ArgsUsage:       "URL",
+		ArgValidator:    refuseExtraArgs,
+		HideHelpCommand: true,
+		CommandNotFound: showCommandHelp,
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			mfa.setVerbosity(cmd)
 			mfa.printBanner()
@@ -445,9 +482,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,8 +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.
+// 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 {
@@ -39,13 +40,19 @@ 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)
+	// 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, err
+		return nil, fmt.Errorf("%s: %w", path, err)
 	}
 
 	return opts, nil

internal/cli/signing_test.go

@@ -2,15 +2,28 @@
 package cli
 
 import (
+	"bufio"
+	"io"
+	"net/http"
+	"net/http/httptest"
 	"path/filepath"
+	"slices"
+	"strings"
+	"sync/atomic"
 	"testing"
+	"time"
 
+	"github.com/ProtonMail/go-crypto/openpgp/packet"
+	"github.com/creack/pty"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
-const testFlagSignKey = "--sign-key"
+const (
+	testFlagSignKey = "--sign-key"
+	testKeyFile     = "/key.asc"
+)
 
 // TestGenAndFreshenSignWithKeyFile runs gen, then freshen after a file is
 // added, with --sign-key naming a key file: one key with no passphrase and
@@ -26,7 +39,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")
@@ -64,21 +77,71 @@ func TestGenAndFreshenSignWithKeyFile(t *testing.T) {
 	}
 }
 
+// TestFreshenSignedManifestNeedsSignKey runs freshen on a signed manifest
+// after a file is added. Without --sign-key it must fail, naming the
+// signer and --sign-key, and leave the manifest as it was; with --sign-key
+// it must rewrite the manifest, signed.
+func TestFreshenSignedManifestNeedsSignKey(t *testing.T) {
+	t.Setenv(envSignKeyPassphrase, "")
+
+	secretKey, fingerprint := testSecretKey(t, nil, nil)
+
+	fs := afero.NewOsFs()
+	keyFile := filepath.Join(t.TempDir(), "key.asc")
+	root := t.TempDir()
+	manifestPath := filepath.Join(root, defaultManifestName)
+
+	require.NoError(t, afero.WriteFile(fs, keyFile, secretKey, 0o600))
+	writeTestFile(t, fs, filepath.Join(root, testFileTxt), "hello")
+
+	opts := testOpts([]string{
+		testApp, cmdGenerate, "-q", testFlagSignKey, keyFile,
+		"-o", manifestPath, root,
+	}, fs)
+	require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
+
+	before, err := afero.ReadFile(fs, manifestPath)
+	require.NoError(t, err)
+
+	writeTestFile(t, fs, filepath.Join(root, "added.txt"), "added")
+
+	opts = testOpts([]string{testApp, cmdFreshen, "-q", manifestPath}, fs)
+	assert.Equal(t, 1, runCLI(opts))
+	assert.Contains(t, testStderr(t, opts), fingerprint)
+	assert.Contains(t, testStderr(t, opts), testFlagSignKey)
+
+	after, err := afero.ReadFile(fs, manifestPath)
+	require.NoError(t, err)
+	assert.Equal(t, before, after)
+
+	opts = testOpts([]string{
+		testApp, cmdFreshen, "-q", testFlagSignKey, keyFile, manifestPath,
+	}, fs)
+	require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
+
+	opts = testOpts([]string{
+		testApp, cmdCheck, "-q",
+		"--" + flagRequireSignature, fingerprint, manifestPath,
+	}, fs)
+	require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
+	assert.Len(t, manifestFiles(t, fs, manifestPath), 2)
+}
+
 // TestSignWithProtectedKeyNeedsPassphrase runs gen with a protected key,
 // with MFER_SIGN_KEY_PASSPHRASE empty and no terminal to ask on. gen must
 // fail, naming the variable, and write no manifest.
 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, "/key.asc", secretKey, 0o600))
+	require.NoError(t, afero.WriteFile(fs, testKeyFile, secretKey, 0o600))
 	require.NoError(t, fs.MkdirAll(testDir, 0o755))
 	writeTestFile(t, fs, testFile1, "hello")
 
 	opts := testOpts([]string{
-		testApp, cmdGenerate, "-q", testFlagSignKey, "/key.asc",
+		testApp, cmdGenerate, "-q", testFlagSignKey, testKeyFile,
 		"-o", testMF, testDir,
 	}, fs)
 	assert.Equal(t, 1, runCLI(opts))
@@ -89,3 +152,214 @@ func TestSignWithProtectedKeyNeedsPassphrase(t *testing.T) {
 	require.NoError(t, err)
 	assert.False(t, exists)
 }
+
+// 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")
+
+	wrongPassphrase, _ := testSecretKey(t, []byte("right"), nil)
+
+	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()),
+	})
+
+	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,
+	} {
+		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])
+		}
+	}
+}
+
+// TestGenAsksForPassphraseOnTerminal runs gen with a protected key, no
+// MFER_SIGN_KEY_PASSPHRASE, and a terminal as stdin and stderr. gen must
+// ask for the passphrase on stderr, and sign with what is typed after the
+// prompt.
+func TestGenAsksForPassphraseOnTerminal(t *testing.T) {
+	t.Setenv(envSignKeyPassphrase, "")
+
+	secretKey, fingerprint := testSecretKey(t, []byte("passphrase"), nil)
+
+	fs := afero.NewOsFs()
+	keyFile := filepath.Join(t.TempDir(), "key.asc")
+	root := t.TempDir()
+	manifestPath := filepath.Join(root, defaultManifestName)
+
+	require.NoError(t, afero.WriteFile(fs, keyFile, secretKey, 0o600))
+	writeTestFile(t, fs, filepath.Join(root, testFileTxt), "hello")
+
+	terminal, tty, err := pty.Open()
+	require.NoError(t, err)
+
+	t.Cleanup(func() { _ = terminal.Close() })
+
+	opts := testOpts([]string{
+		testApp, cmdGenerate, "-q", testFlagSignKey, keyFile,
+		"-o", manifestPath, root,
+	}, fs)
+	opts.Stdin = tty
+	opts.Stderr = tty
+
+	exitCode := make(chan int, 1)
+
+	go func() {
+		exitCode <- runCLI(opts)
+
+		// Once gen has ended, reading the terminal fails instead of
+		// waiting for a prompt that will not come.
+		_ = tty.Close()
+	}()
+
+	prompt := "Passphrase for " + keyFile + ": "
+	output := bufio.NewReader(terminal)
+	written := ""
+
+	for !strings.HasSuffix(written, prompt) {
+		b, err := output.ReadByte()
+		require.NoError(t, err, "gen wrote %q and no prompt", written)
+
+		written += string(b)
+	}
+
+	_, err = terminal.WriteString("passphrase\n")
+	require.NoError(t, err)
+
+	code := <-exitCode
+	rest, _ := io.ReadAll(output)
+	require.Equal(t, 0, code, "gen wrote %q", rest)
+
+	check := testOpts([]string{
+		testApp, cmdCheck, "-q",
+		"--" + flagRequireSignature, fingerprint, manifestPath,
+	}, fs)
+	require.Equal(t, 0, runCLI(check), testStderr(t, check))
+}
+
+// assertEmptySignKeyRefused runs gen and freshen, each with flags, on a
+// directory holding a file and an unsigned manifest. Each must fail,
+// naming --sign-key, and leave the manifest as it was.
+func assertEmptySignKeyRefused(t *testing.T, flags ...string) {
+	t.Helper()
+
+	manifest := manifestOf(t, map[string][]byte{testFileTxt: []byte("listed")})
+
+	for command, rest := range map[string][]string{
+		cmdGenerate: {"-q", "-f", "-o", testMF, testDir},
+		cmdFreshen:  {"-q", testMF},
+	} {
+		fs := afero.NewMemMapFs()
+		require.NoError(t, fs.MkdirAll(testDir, 0o755))
+		writeTestFile(t, fs, testFile1, "hello")
+		require.NoError(t, afero.WriteFile(fs, testMF, manifest, 0o644))
+
+		opts := testOpts(slices.Concat([]string{testApp, command}, flags, rest), fs)
+		assert.Equal(t, 1, runCLI(opts), command)
+		assert.Contains(t, testStderr(t, opts), errEmptySignKey.Error(), command)
+
+		content, err := afero.ReadFile(fs, testMF)
+		require.NoError(t, err)
+		assert.Equal(t, manifest, content, command)
+	}
+}
+
+// TestGenAndFreshenRefuseEmptySignKey runs gen and freshen with --sign-key
+// given an empty value, as an unset shell variable gives it.
+func TestGenAndFreshenRefuseEmptySignKey(t *testing.T) {
+	t.Parallel()
+
+	assertEmptySignKeyRefused(t, testFlagSignKey, "")
+}
+
+// TestGenAndFreshenRefuseEmptySignKeyFromEnvironment runs gen and freshen
+// with MFER_SIGN_KEY set to an empty string.
+func TestGenAndFreshenRefuseEmptySignKeyFromEnvironment(t *testing.T) {
+	t.Setenv("MFER_SIGN_KEY", "")
+
+	assertEmptySignKeyRefused(t)
+}
+
+// assertEmptyFingerprintRefused runs check on an unsigned manifest, and
+// fetch of that manifest from a server, each with flags. Each must refuse
+// the empty fingerprint as malformed, and fetch must request nothing.
+func assertEmptyFingerprintRefused(t *testing.T, flags ...string) {
+	t.Helper()
+
+	const want = "invalid fingerprint: must be exactly 40 hex characters, got 0"
+
+	files := map[string][]byte{testFileTxt: []byte("listed")}
+	manifest := manifestOf(t, files)
+
+	fs := afero.NewMemMapFs()
+	require.NoError(t, fs.MkdirAll(testDir, 0o755))
+	writeTestFile(t, fs, filepath.Join(testDir, testFileTxt), "listed")
+	require.NoError(t, afero.WriteFile(fs, testMF, manifest, 0o644))
+
+	opts := testOpts(slices.Concat(
+		[]string{testApp, cmdCheck, "-q"}, flags, []string{testMF},
+	), fs)
+	assert.Equal(t, 1, runCLI(opts), cmdCheck)
+	assert.Contains(t, testStderr(t, opts), want, cmdCheck)
+
+	var requests atomic.Int32
+
+	tree := fetchTestHandler(manifest, files)
+
+	server := httptest.NewServer(
+		http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			requests.Add(1)
+			tree.ServeHTTP(w, r)
+		}))
+	defer server.Close()
+
+	opts = testOpts(slices.Concat(
+		[]string{testApp, cmdFetch, "-q", "--" + flagDest, t.TempDir()},
+		flags, []string{server.URL},
+	), afero.NewOsFs())
+	assert.Equal(t, 1, runCLI(opts), cmdFetch)
+	assert.Contains(t, testStderr(t, opts), want, cmdFetch)
+	assert.Zero(t, requests.Load(), "fetch made a request")
+}
+
+// TestCheckAndFetchRefuseEmptyRequireSignature runs check and fetch with
+// --require-signature given an empty value, as an unset shell variable
+// gives it.
+func TestCheckAndFetchRefuseEmptyRequireSignature(t *testing.T) {
+	t.Parallel()
+
+	assertEmptyFingerprintRefused(t, "--"+flagRequireSignature, "")
+}
+
+// TestCheckAndFetchRefuseEmptyRequireSignatureFromEnvironment runs check
+// and fetch with MFER_REQUIRE_SIGNATURE set to an empty string.
+func TestCheckAndFetchRefuseEmptyRequireSignatureFromEnvironment(t *testing.T) {
+	t.Setenv("MFER_REQUIRE_SIGNATURE", "")
+
+	assertEmptyFingerprintRefused(t)
+}

mfer/builder.go

@@ -34,6 +34,7 @@ var (
 	errPathAbsolute     = errors.New("is absolute; must be relative")
 	errPathEmptySegment = errors.New("contains empty segment")
 	errPathDotDot       = errors.New("contains '..' segment")
+	errPathDot          = errors.New("contains '.' segment")
 	errSizeMismatch     = errors.New("size mismatch")
 	errNegativeSize     = errors.New("size cannot be negative")
 	errHashNotMultihash = errors.New("hash is not a valid multihash")
@@ -46,6 +47,7 @@ var (
 //   - Must use forward slashes only (no backslashes)
 //   - Must be relative (no leading /)
 //   - Must not contain ".." segments
+//   - Must not contain "." segments
 //   - Must not contain empty segments (no "//")
 //   - Must not be empty
 func ValidatePath(p string) error {
@@ -73,6 +75,10 @@ func ValidatePath(p string) error {
 		if seg == ".." {
 			return fmt.Errorf("path %q %w", p, errPathDotDot)
 		}
+
+		if seg == "." {
+			return fmt.Errorf("path %q %w", p, errPathDot)
+		}
 	}
 
 	return nil

mfer/builder_test.go

@@ -286,6 +286,9 @@ func TestValidatePath(t *testing.T) {
 		{"has/../traversal", "dot-dot segment"},
 		{"has//double", "empty segment"},
 		{"..", "just dot-dot"},
+		{"./a.txt", "leading dot segment"},
+		{"has/./dot", "interior dot segment"},
+		{".", "just dot"},
 		{string([]byte{0xff, 0xfe}), "invalid UTF-8"},
 	}
 	for _, tt := range invalid {

mfer/checker.go

@@ -264,11 +264,12 @@ func (c *Checker) Check(
 	return nil
 }
 
-// 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.
+// FindExtraFiles walks the filesystem and reports the regular files, and
+// symlinks to them, that are 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)
@@ -327,6 +328,13 @@ func (c *Checker) FindExtraFiles(ctx context.Context, results chan<- Result) err
 			}
 		}
 
+		// gen lists regular files and symlinks to them only, so a named
+		// pipe, a socket, a device, a symlink to a directory and a broken
+		// symlink are never reported.
+		if !info.Mode().IsRegular() {
+			return nil
+		}
+
 		// Skip the manifest file itself, however its path is spelled
 		if os.SameFile(info, c.manifestInfo) {
 			return nil
@@ -371,6 +379,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/constants.go

@@ -1,12 +1,6 @@
 package mfer
 
 const (
-	// Version is the current mfer release version.
-	Version = "0.1.0"
-
-	// ReleaseDate is the date on which Version was released.
-	ReleaseDate = "2025-12-17"
-
 	// MaxDecompressedSize is the maximum allowed size of decompressed manifest
 	// data (256 MiB). This prevents decompression bombs from consuming excessive
 	// memory.

mfer/deserialize.go

@@ -27,6 +27,7 @@ var (
 	errUUIDMismatch         = errors.New("outer and inner UUID mismatch")
 	errInvalidFileFormat    = errors.New("invalid file format")
 	errInvalidManifestPath  = errors.New("invalid file entry")
+	errEntryMissingHash     = errors.New("file entry has no hash")
 	errDecodedTooLarge      = errors.New("too much memory needed")
 	errSignerNotSigningKey  = errors.New(
 		"signer is not the fingerprint of the key that made the signature")
@@ -293,7 +294,8 @@ func (m *manifest) deserializeInner() error {
 	// untrusted .mf. Reject loudly on the first offender rather than
 	// dropping entries, which would let a hostile manifest hide files from a
 	// check. A path listed twice is refused too: check would check the one
-	// file against both entries.
+	// file against both entries. So is an entry with no hash: there is
+	// nothing to check the file against.
 	seen := make(map[string]bool, len(m.pbInner.GetFiles()))
 
 	for _, f := range m.pbInner.GetFiles() {
@@ -302,6 +304,10 @@ func (m *manifest) deserializeInner() error {
 			return fmt.Errorf("%w: %w", errInvalidManifestPath, err)
 		}
 
+		if len(f.GetHashes()) == 0 {
+			return fmt.Errorf("%w: %q", errEntryMissingHash, f.GetPath())
+		}
+
 		if seen[f.GetPath()] {
 			return fmt.Errorf("%w %q", errDuplicatePath, f.GetPath())
 		}

mfer/deserialize_path_test.go

@@ -80,6 +80,8 @@ func TestDeserializeRejectsInvalidEntryPaths(t *testing.T) {
 	}{
 		{"parent traversal", "../escape"},
 		{"interior traversal", "a/../../escape"},
+		{"leading dot segment", "./a.txt"},
+		{"interior dot segment", "dir/./a.txt"},
 		{"absolute path", "/etc/passwd"},
 		{"backslash path", `a\b`},
 		{"double slash", "a//b"},
@@ -165,6 +167,24 @@ func TestDeserializeRefusesPathListedTwice(t *testing.T) {
 	}
 }
 
+// A manifest holding a file entry with no hash is refused as it is loaded,
+// naming the path: there is nothing to check the file against.
+func TestDeserializeRefusesEntryWithNoHash(t *testing.T) {
+	t.Parallel()
+
+	id := uuid.NewV4()
+	inner, err := proto.Marshal(&MFFile{
+		Version: MFFile_VERSION_ONE,
+		Files:   []*MFFilePath{{Path: "x", Size: 1}},
+		Uuid:    id[:],
+	})
+	require.NoError(t, err)
+
+	_, err = NewManifestFromReader(bytes.NewReader(wrapInner(t, id, inner)))
+	require.ErrorIs(t, err, errEntryMissingHash)
+	require.EqualError(t, err, `file entry has no hash: "x"`)
+}
+
 // Entries of a path, an empty hash, an empty MIME type and empty modification
 // and change times are counted at 432 bytes each (176 + 112 + 16 + 64 + 64)
 // and take 16 bytes plus the path to encode. A 37-character path makes that
@@ -229,6 +249,8 @@ func TestDeserializeDropsUnknownFields(t *testing.T) {
 
 	entry := protowire.AppendTag(nil, 1, protowire.BytesType) // MFFilePath.path
 	entry = protowire.AppendString(entry, "a")
+	entry = protowire.AppendTag(entry, 3, protowire.BytesType) // MFFilePath.hashes
+	entry = protowire.AppendBytes(entry, nil)
 	entry = append(entry, unknown...)
 
 	id := uuid.NewV4()

mfer/errmsg_test.go

@@ -62,6 +62,12 @@ func TestValidatePathMessagesVerbatim(t *testing.T) {
 			want: `path "a/../b" contains '..' segment`,
 			is:   errPathDotDot,
 		},
+		{
+			name: "dot segment",
+			path: "a/./b",
+			want: `path "a/./b" contains '.' segment`,
+			is:   errPathDot,
+		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()

mfer/manifest.go

@@ -47,6 +47,17 @@ func (m *manifest) Files() []*MFFilePath {
 	return m.pbInner.GetFiles()
 }
 
+// Signer returns the fingerprint of the key that made a loaded manifest's
+// signature, which loading the manifest checked, or nil if the manifest is
+// not signed.
+func (m *manifest) Signer() []byte {
+	if len(m.pbOuter.GetSignature()) == 0 {
+		return nil
+	}
+
+	return m.pbOuter.GetSigner()
+}
+
 // signatureString generates the canonical string used for signing/verification.
 // Format: MAGIC-UUID-MULTIHASH where UUID and multihash are hex-encoded.
 // Requires pbOuter to be set with Uuid and Sha256 fields.

mfer/manifest_test.go

@@ -0,0 +1,30 @@
+//nolint:testpackage // white-box tests exercise unexported internals
+package mfer
+
+import (
+	"encoding/hex"
+	"testing"
+	"uuid"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestSignatureString checks the canonical string for the UUID and sha256
+// of the example in the Signature Scheme section of docs/FORMAT.md, which
+// shows the same string.
+func TestSignatureString(t *testing.T) {
+	t.Parallel()
+
+	manifestUUID := uuid.MustParse("b0fb6918-6298-443b-ada2-1b162f4dc0b3")
+	sha256Hash, err := hex.DecodeString(
+		"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855")
+	require.NoError(t, err)
+
+	m := &manifest{pbOuter: &MFFileOuter{Uuid: manifestUUID[:], Sha256: sha256Hash}}
+
+	got, err := m.signatureString()
+	require.NoError(t, err)
+	assert.Equal(t, "ZNAVSRFG-b0fb69186298443bada21b162f4dc0b3-"+
+		"1220e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", got)
+}

mfer/mf.pb.go

@@ -223,11 +223,14 @@ type MFFileOuter struct {
 	// uuid must match the uuid in the inner message
 	Uuid         []byte `protobuf:"bytes,105,opt,name=uuid,proto3" json:"uuid,omitempty"`
 	InnerMessage []byte `protobuf:"bytes,199,opt,name=innerMessage,proto3" json:"innerMessage,omitempty"`
-	//detached signature, ascii or binary
+	// one OpenPGP signature, armored or binary, over the canonical string
+	// ZNAVSRFG-<uuid>-<sha256 as a multihash>, with the uuid as 32
+	// lower-case hex digits and no hyphens, and the multihash in lower-case
+	// hex (see docs/FORMAT.md). A signature is never stored anywhere else.
 	Signature []byte `protobuf:"bytes,201,opt,name=signature,proto3,oneof" json:"signature,omitempty"`
-	//full GPG key id
+	// fingerprint of the signing primary key, in upper-case hex
 	Signer []byte `protobuf:"bytes,202,opt,name=signer,proto3,oneof" json:"signer,omitempty"`
-	//full GPG signing public key, ascii or binary
+	// OpenPGP public key of the signer, one primary key, armored or binary
 	SigningPubKey []byte `protobuf:"bytes,203,opt,name=signingPubKey,proto3,oneof" json:"signingPubKey,omitempty"`
 	unknownFields protoimpl.UnknownFields
 	sizeCache     protoimpl.SizeCache
@@ -330,8 +333,8 @@ type MFFilePath struct {
 	state protoimpl.MessageState `protogen:"open.v1"`
 	// required attributes:
 	// Path invariants: must be valid UTF-8, use forward slashes only,
-	// be relative (no leading /), contain no ".." segments, and no
-	// empty segments (no "//").
+	// be relative (no leading /), contain no ".." or "." segments, and
+	// no empty segments (no "//").
 	Path string `protobuf:"bytes,1,opt,name=path,proto3" json:"path,omitempty"`
 	Size int64  `protobuf:"varint,2,opt,name=size,proto3" json:"size,omitempty"`
 	// gotta have at least one:

mfer/mf.proto

@@ -32,23 +32,25 @@ message MFFileOuter {
     bytes uuid = 105;
 
     bytes innerMessage = 199;
-    // 2xx for optional manifest root attributes
-    // think we might use gosignify instead of gpg:
-    // github.com/frankbraun/gosignify
 
-    //detached signature, ascii or binary
+    // 2xx for optional manifest root attributes
+
+    // one OpenPGP signature, armored or binary, over the canonical string
+    // ZNAVSRFG-<uuid>-<sha256 as a multihash>, with the uuid as 32
+    // lower-case hex digits and no hyphens, and the multihash in lower-case
+    // hex (see docs/FORMAT.md). A signature is never stored anywhere else.
     optional bytes signature = 201;
-    //full GPG key id
+    // fingerprint of the signing primary key, in upper-case hex
     optional bytes signer = 202;
-    //full GPG signing public key, ascii or binary
+    // OpenPGP public key of the signer, one primary key, armored or binary
     optional bytes signingPubKey = 203;
 }
 
 message MFFilePath {
     // required attributes:
     // Path invariants: must be valid UTF-8, use forward slashes only,
-    // be relative (no leading /), contain no ".." segments, and no
-    // empty segments (no "//").
+    // be relative (no leading /), contain no ".." or "." segments, and
+    // no empty segments (no "//").
     string path = 1;
     int64 size = 2;
 

mfer/mf.proto.sha256

@@ -1 +1 @@
-3d4dcb0b2f4640dd3ae6bb48b833e9f26ae9771e2d3e88bd646e7f1724dda654  mf.proto
+1668b2f6b8639a9213873474e58dbacb3a4ef77cc27c42253cad8dd076e5f206  mf.proto

mfer/openpgp.go

@@ -8,6 +8,7 @@ import (
 	"io"
 	"slices"
 	"strings"
+	"time"
 
 	"github.com/ProtonMail/go-crypto/openpgp"
 	"github.com/ProtonMail/go-crypto/openpgp/armor"
@@ -16,24 +17,49 @@ 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
 
-	// armorEnd starts the line that ends an armored block.
-	armorEnd = "-----END "
+	// 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.
+	armorBegin = "-----BEGIN "
+	armorEnd   = "-----END "
 )
 
 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")
+	errKeyNotValidWhenSigned = errors.New(
+		"signature was made when its key was not valid")
 )
 
 // SigningOptions holds the key a manifest is signed with.
@@ -47,7 +73,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 {
@@ -57,6 +83,26 @@ func SecretKeyIsProtected(secretKey []byte) (bool, error) {
 	return isProtected(key), nil
 }
 
+// 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 {
+	key, err := readSigningKey(opts)
+	if err != nil {
+		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
 // opts.Passphrase if it is protected.
 func readSigningKey(opts *SigningOptions) (*openpgp.Entity, error) {
@@ -82,13 +128,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
 	}
@@ -97,8 +148,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)
@@ -113,6 +165,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)
@@ -170,12 +245,13 @@ func fingerprint(key *openpgp.Entity) string {
 	return strings.ToUpper(hex.EncodeToString(key.PrimaryKey.Fingerprint))
 }
 
-// verifySignature checks that signature is one good OpenPGP signature
-// over data, made by the one primary key in pubKey or one of its subkeys,
-// and returns that primary key's fingerprint. signature and pubKey may each
-// be armored or binary.
+// verifySignature checks that signature is one good, unexpired OpenPGP
+// signature over data, made by the one primary key in pubKey or one of its
+// subkeys while that key was valid, and returns that primary key's
+// fingerprint. The key may have expired since. 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
 	}
@@ -194,51 +270,78 @@ func verifySignature(data, signature, pubKey []byte) (string, error) {
 		return "", fmt.Errorf("%w, found %d", errNotOneSignature, sigs)
 	}
 
-	_, err = openpgp.CheckDetachedSignature(openpgp.EntityList{key},
+	sig, _, err := openpgp.VerifyDetachedSignature(openpgp.EntityList{key},
 		bytes.NewReader(data), bytes.NewReader(sigData), nil)
 	// A manifest outlives its signing key, so a signature by a key that
-	// has expired since is still good.
+	// has expired since is still good, unless the signature has expired
+	// itself. The library does not check that once it finds the key
+	// expired.
+	if errors.Is(err, pgperrors.ErrKeyExpired) && sig.SigExpired(time.Now()) {
+		err = pgperrors.ErrSignatureExpired
+	}
+
 	if err != nil && !errors.Is(err, pgperrors.ErrKeyExpired) {
 		return "", fmt.Errorf("verify signature: %w", err)
 	}
 
+	// The check above judges the key as of now. Checking the signature a
+	// second time with the library's clock set to its creation time
+	// requires the primary key, and the subkey that made the signature, to
+	// have been valid then under the key validity rules of RFC 9580. The
+	// key is judged by its current self-signatures and binding signatures:
+	// for a version 4 key the newest self-signature on its primary user
+	// ID, for a version 6 key its newest direct-key signature, and for a
+	// subkey its newest binding signature with the signature embedded in
+	// it. So a signature made before the newest of those is refused.
+	signedAt := &packet.Config{
+		Time: func() time.Time { return sig.CreationTime },
+	}
+
+	_, _, err = openpgp.VerifyDetachedSignature(openpgp.EntityList{key},
+		bytes.NewReader(data), bytes.NewReader(sigData), signedAt)
+	if err != nil {
+		return "", fmt.Errorf("verify signature: %w: %w",
+			errKeyNotValidWhenSigned, err)
+	}
+
 	return fingerprint(key), nil
 }
 
 // dearmor returns the binary OpenPGP data in data: data itself when it is
-// not armored, or else the bodies of all its armored blocks, one after
-// another. armor.Decode reads only the first block it finds, so dearmor
-// decodes the text after each block's END line in turn, and a second key
-// or signature after the first is read too.
+// not armored, or else the body of its armored block. Armored data must be
+// one block and nothing else: its first line is the only BEGIN line and
+// its last line the only END line, white space around them aside.
+// armor.Decode skips any text before a BEGIN line and reads only the first
+// block, so without this a second key or signature would go unseen.
 func dearmor(data []byte) ([]byte, error) {
-	var binary []byte
-
-	rest := data
-
-	for {
-		block, err := armor.Decode(bytes.NewReader(rest))
-		if errors.Is(err, io.EOF) {
-			break
-		}
-
-		if err != nil {
-			return nil, err
-		}
-
-		body, err := io.ReadAll(block.Body)
-		if err != nil {
-			return nil, err
-		}
-
-		binary = append(binary, body...)
-		_, rest, _ = bytes.Cut(rest, []byte(armorEnd))
-	}
-
-	if binary == nil {
+	if !bytes.Contains(data, []byte(armorBegin)) {
 		return data, nil
 	}
 
-	return binary, nil
+	text := bytes.TrimSpace(data)
+	lastLine := text[bytes.LastIndexByte(text, '\n')+1:]
+
+	if !bytes.HasPrefix(text, []byte(armorBegin)) ||
+		bytes.Count(text, []byte(armorBegin)) != 1 ||
+		!bytes.HasPrefix(lastLine, []byte(armorEnd)) ||
+		bytes.Count(text, []byte(armorEnd)) != 1 {
+		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, errMalformedArmor
+	}
+
+	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
@@ -264,3 +367,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,10 @@ package mfer
 import (
 	"bytes"
 	"context"
+	"crypto"
+	"crypto/dsa" //nolint:staticcheck // SA1019: tests need a DSA key to refuse
 	"io"
+	"math/big"
 	"os"
 	"path/filepath"
 	"slices"
@@ -15,6 +18,8 @@ import (
 
 	"github.com/ProtonMail/go-crypto/openpgp"
 	"github.com/ProtonMail/go-crypto/openpgp/armor"
+	"github.com/ProtonMail/go-crypto/openpgp/elgamal"
+	pgperrors "github.com/ProtonMail/go-crypto/openpgp/errors"
 	"github.com/ProtonMail/go-crypto/openpgp/packet"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
@@ -172,6 +177,25 @@ func TestBuilderWithSigning(t *testing.T) {
 		"-----BEGIN PGP PUBLIC KEY BLOCK-----")
 }
 
+// TestManifestSigner checks that Signer gives the signing key's
+// fingerprint for a signed manifest, and nil once the signature is taken
+// out, although the signer field is still there.
+func TestManifestSigner(t *testing.T) {
+	t.Parallel()
+
+	key := newTestKey(t, nil)
+	data := signedTestManifest(t, &SigningOptions{SecretKey: armoredSecretKeys(t, key)})
+
+	signed, err := NewManifestFromReader(bytes.NewReader(data))
+	require.NoError(t, err)
+	assert.Equal(t, fingerprint(key), string(signed.Signer()))
+
+	unsigned, err := NewManifestFromReader(bytes.NewReader(
+		rewriteOuter(t, data, func(outer *MFFileOuter) { outer.Signature = nil })))
+	require.NoError(t, err)
+	assert.Nil(t, unsigned.Signer())
+}
+
 func TestScannerWithSigning(t *testing.T) {
 	t.Parallel()
 
@@ -284,6 +308,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()
 
@@ -322,8 +437,11 @@ func TestVerifySignature(t *testing.T) {
 	assert.Error(t, err)
 }
 
-// TestVerifySignatureKeyExpiredSince verifies a signature made in 2020 by
-// a key that expired a day after it was made. The signature is still good.
+// TestVerifySignatureKeyExpiredSince verifies two signatures made in 2020
+// by a key that expired a day after it was made. A signature with no
+// expiry time is still good, as is one made then by a signing subkey that
+// expired a day after it was made. One that expired an hour after it was
+// made is refused, although the key has expired too.
 func TestVerifySignatureKeyExpiredSince(t *testing.T) {
 	t.Parallel()
 
@@ -335,9 +453,14 @@ func TestVerifySignatureKeyExpiredSince(t *testing.T) {
 	key := newTestKey(t, config)
 	data := []byte("signed in 2020")
 
-	var sig bytes.Buffer
+	var sig, expiringSig bytes.Buffer
 
 	require.NoError(t, openpgp.ArmoredDetachSign(&sig, key, bytes.NewReader(data), config))
+	require.NoError(t, openpgp.ArmoredDetachSign(&expiringSig, key,
+		bytes.NewReader(data), &packet.Config{
+			Time:            config.Time,
+			SigLifetimeSecs: uint32(time.Hour.Seconds()),
+		}))
 
 	pubKey, err := armoredPublicKey(key)
 	require.NoError(t, err)
@@ -345,6 +468,131 @@ func TestVerifySignatureKeyExpiredSince(t *testing.T) {
 	signer, err := verifySignature(data, sig.Bytes(), pubKey)
 	require.NoError(t, err)
 	assert.Equal(t, fingerprint(key), signer)
+
+	withSubkey := newTestKey(t, &packet.Config{Time: config.Time})
+	require.NoError(t, withSubkey.AddSigningSubkey(&packet.Config{
+		Algorithm: packet.PubKeyAlgoEdDSA, Time: config.Time,
+		KeyLifetimeSecs: config.KeyLifetimeSecs,
+	}))
+
+	var subkeySig bytes.Buffer
+
+	require.NoError(t, openpgp.ArmoredDetachSign(&subkeySig, withSubkey,
+		bytes.NewReader(data), config))
+
+	subkeyPubKey, err := armoredPublicKey(withSubkey)
+	require.NoError(t, err)
+
+	_, err = verifySignature(data, subkeySig.Bytes(), subkeyPubKey)
+	require.NoError(t, err)
+
+	_, err = verifySignature(data, expiringSig.Bytes(), pubKey)
+	require.ErrorIs(t, err, pgperrors.ErrSignatureExpired)
+}
+
+// signDated returns a detached signature over data by signer, dated when.
+// openpgp.DetachSign signs only with a key that is valid at the time its
+// config gives, so this makes the signature packet without that check.
+func signDated(t *testing.T, signer *packet.PrivateKey, data []byte,
+	when time.Time,
+) []byte {
+	t.Helper()
+
+	sig := &packet.Signature{
+		SigType:      packet.SigTypeBinary,
+		PubKeyAlgo:   signer.PubKeyAlgo,
+		Hash:         crypto.SHA256,
+		CreationTime: when,
+		IssuerKeyId:  &signer.KeyId,
+	}
+
+	h, err := sig.PrepareSign(nil)
+	require.NoError(t, err)
+
+	_, err = h.Write(data)
+	require.NoError(t, err)
+
+	require.NoError(t, sig.Sign(h, signer, nil))
+
+	var buf bytes.Buffer
+
+	require.NoError(t, sig.Serialize(&buf))
+
+	return buf.Bytes()
+}
+
+// TestVerifySignatureRefusesKeyNotValidWhenSigned verifies signatures
+// dated when the key or subkey that made them was not valid: after the key
+// expired; after the key's self-signature, or the signing subkey's binding
+// signature, expired, the key or subkey having expired since as well; and
+// before the key was made, also by a key made in 2100. Each must be
+// refused.
+func TestVerifySignatureRefusesKeyNotValidWhenSigned(t *testing.T) {
+	t.Parallel()
+
+	made := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)
+	twoDaysLater := made.Add(48 * time.Hour)
+	day := uint32((24 * time.Hour).Seconds())
+	clock := func(when time.Time) func() time.Time {
+		return func() time.Time { return when }
+	}
+
+	expiredAfterADay := newTestKey(t, &packet.Config{
+		Time: clock(made), KeyLifetimeSecs: day,
+	})
+	selfSignatureExpiredAfterADay := newTestKey(t, &packet.Config{
+		Time: clock(made), KeyLifetimeSecs: 30 * day, SigLifetimeSecs: day,
+	})
+
+	withSubkey := newTestKey(t, &packet.Config{Time: clock(made)})
+	require.NoError(t, withSubkey.AddSigningSubkey(&packet.Config{
+		Algorithm: packet.PubKeyAlgoEdDSA, Time: clock(made),
+		KeyLifetimeSecs: 30 * day, SigLifetimeSecs: day,
+	}))
+	subkey := withSubkey.Subkeys[len(withSubkey.Subkeys)-1].PrivateKey
+
+	madeADayLater := newTestKey(t, &packet.Config{
+		Time: clock(made.Add(24 * time.Hour)),
+	})
+	madeIn2100 := newTestKey(t, &packet.Config{
+		Time: clock(time.Date(2100, 1, 1, 0, 0, 0, 0, time.UTC)),
+	})
+
+	for name, test := range map[string]struct {
+		key    *openpgp.Entity
+		signer *packet.PrivateKey
+		when   time.Time
+	}{
+		"after the key expired": {
+			expiredAfterADay, expiredAfterADay.PrivateKey, twoDaysLater,
+		},
+		"after the self-signature expired": {
+			selfSignatureExpiredAfterADay,
+			selfSignatureExpiredAfterADay.PrivateKey, twoDaysLater,
+		},
+		"after the binding signature expired": {
+			withSubkey, subkey, twoDaysLater,
+		},
+		"before the key was made": {
+			madeADayLater, madeADayLater.PrivateKey, made,
+		},
+		"before a key made in 2100": {
+			madeIn2100, madeIn2100.PrivateKey, made,
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			data := []byte("signed in 2020")
+
+			pubKey, err := armoredPublicKey(test.key)
+			require.NoError(t, err)
+
+			_, err = verifySignature(data,
+				signDated(t, test.signer, data, test.when), pubKey)
+			require.ErrorIs(t, err, errKeyNotValidWhenSigned)
+		})
+	}
 }
 
 func TestManifestSignatureVerification(t *testing.T) {
@@ -403,12 +651,12 @@ func TestManifestSignedByGPGLoads(t *testing.T) {
 }
 
 // TestManifestRefusesSecondEmbeddedKey loads manifests whose embedded
-// public key block holds another key besides the key that signed it: in an
-// armored block of its own, in the same armored block, as a secret key
-// with no user ID, which openpgp.ReadKeyRing skips, and written as a
-// subkey packet at the start of the block, which openpgp.ReadKeyRing reads
-// as a primary key. Loading must refuse each, although the signature is
-// good and the signer field names the key that made it.
+// public key block holds another key besides the key that signed it: as a
+// public key, as a secret key with no user ID, which openpgp.ReadKeyRing
+// skips, and written as a subkey packet at the start of the block, which
+// openpgp.ReadKeyRing reads as a primary key. Loading must refuse each,
+// although the signature is good and the signer field names the key that
+// made it.
 func TestManifestRefusesSecondEmbeddedKey(t *testing.T) {
 	t.Parallel()
 
@@ -424,12 +672,9 @@ func TestManifestRefusesSecondEmbeddedKey(t *testing.T) {
 	otherAsSubkey.PrimaryKey.IsSubkey = true
 
 	for name, block := range map[string][]byte{
-		"armored block of its own": joinArmored(
-			armoredPublicKeys(t, other), armoredPublicKeys(t, signer)),
-		"same armored block": armoredPublicKeys(t, other, signer),
-		"secret key without user ID": joinArmored(
-			armoredPublicKeys(t, signer), armoredSecretKeys(t, otherWithoutUserID)),
-		"subkey packet first": armoredPublicKeys(t, otherAsSubkey, signer),
+		"public key":                 armoredPublicKeys(t, other, signer),
+		"secret key without user ID": armoredSecretKeys(t, signer, otherWithoutUserID),
+		"subkey packet first":        armoredPublicKeys(t, otherAsSubkey, signer),
 	} {
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
@@ -454,31 +699,175 @@ func TestManifestRefusesSecondEmbeddedKeyWithoutUserID(t *testing.T) {
 	other := newTestKey(t, nil)
 	other.Identities = map[string]*openpgp.Identity{}
 
-	manifest := rewriteOuter(t, signedTestManifest(t, testSigningOptions(t)),
+	signer := newTestKey(t, nil)
+
+	manifest := rewriteOuter(t, signedTestManifest(t,
+		&SigningOptions{SecretKey: armoredSecretKeys(t, signer)}),
 		func(outer *MFFileOuter) {
-			outer.SigningPubKey = joinArmored(
-				armoredPublicKeys(t, other), outer.GetSigningPubKey())
+			outer.SigningPubKey = armoredPublicKeys(t, other, signer)
 		})
 
 	_, err := NewManifestFromReader(bytes.NewReader(manifest))
 	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. Loading must refuse it.
+// holds its good signature twice, not armored. Loading must refuse it.
 func TestManifestRefusesTwoSignatures(t *testing.T) {
 	t.Parallel()
 
 	manifest := rewriteOuter(t, signedTestManifest(t, testSigningOptions(t)),
 		func(outer *MFFileOuter) {
-			outer.Signature = joinArmored(
-				outer.GetSignature(), outer.GetSignature())
+			sig, err := dearmor(outer.GetSignature())
+			require.NoError(t, err)
+
+			outer.Signature = slices.Concat(sig, sig)
 		})
 
 	_, err := NewManifestFromReader(bytes.NewReader(manifest))
 	require.ErrorIs(t, err, errNotOneSignature)
 }
 
+// TestManifestRefusesFieldNotOneArmoredBlock loads manifests whose
+// signature or embedded public key block holds its good armored block with
+// something else: a second armored block, text after the END line, or many
+// END lines before the block. Decoding the block once for each END line
+// before it would take time and memory that grow with the square of the
+// field's size. Loading must refuse each.
+func TestManifestRefusesFieldNotOneArmoredBlock(t *testing.T) {
+	t.Parallel()
+
+	manifest := signedTestManifest(t, testSigningOptions(t))
+
+	for name, change := range map[string]func([]byte) []byte{
+		"second armored block": func(block []byte) []byte {
+			return joinArmored(block, block)
+		},
+		"text after the END line": func(block []byte) []byte {
+			return slices.Concat(block, []byte("\nmore text\n"))
+		},
+		"END lines before the block": func(block []byte) []byte {
+			return slices.Concat(
+				[]byte(strings.Repeat(armorEnd+"\n", 1000)), block)
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			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, errNotOneArmoredBlock)
+			}
+		})
+	}
+}
+
 // TestManifestSignedWithSubkey signs with a key that has a signing subkey,
 // which signs in place of the primary key. The manifest must load, with
 // the primary key's fingerprint as signer.

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