Resolve check and freshen paths against the manifest's directory (closes #177)

Without --base, check and freshen now look for a manifest's files in the
directory that holds it, the file named or the one found in a directory
argument, instead of the current directory. So `mfer check /media/drive`
checks a drive against its own index.mf from anywhere. check of a manifest
given by URL still uses the current directory, and --base still overrides,
even when it names the current directory. The --base help text of both
commands and the README's Tool Examples state the default; the README gains
a freshen entry for this, which also names the hidden files and symlinks
freshen leaves out by default and the flags that include them.

Model: opus-5-5

README.md

@@ -9,8 +9,9 @@ downloading, streaming, and mirroring. It was first published in 2022. The
 manifest files' data is serialized with Google's
 [protobuf serialization format](https://developers.google.com/protocol-buffers).
 The structure of these files can be found
-[in the format specification](https://git.eeqj.de/sneak/mfer/src/branch/main/mfer/mf.proto)
-which is included in the [project repository](https://git.eeqj.de/sneak/mfer).
+[in the format specification](docs/FORMAT.md), which refers to the protobuf
+schema `mfer/mf.proto` for exact field numbers and types. Both are included in
+the [project repository](https://git.eeqj.de/sneak/mfer).
 
 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
@@ -272,9 +273,20 @@ are now tracked only in the [issues](https://git.eeqj.de/sneak/mfer/issues).
     - verifies checksums of all files in manifest, displaying error and exiting
       nonzero if any files are missing or corrupted, or have permission bits
       other than the mode the manifest records, unless that is `0000`
+    - looks for those files under the base directory: the one `--base` names, or
+      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
+- `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
+    - 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
 - `mfer fetch https://example.com/stuff/`
     - fetches `/stuff/index.mf` and downloads all files listed in manifest into
       the current directory, or the one given with `--dest`, and assures

docs/FORMAT.md

@@ -50,11 +50,14 @@ allows verifying data integrity before decompression.
 The `innerMessage` field is compressed with
 [Zstandard (zstd)](https://facebook.github.io/zstd/). Implementations must
 enforce a decompression size limit to prevent decompression bombs. The reference
-implementation limits decompressed size to 256 MB. It writes zstd frames with a
+implementation limits decompressed size to 256 MiB. It writes zstd frames with a
 window of at most 8 MiB, the largest window the zstd format recommends decoders
 support, and refuses frames that ask for a larger one. It also refuses an inner
 message whose file entries, hashes, timestamps and MIME types, counted at 176,
-112, 64 and 16 bytes each, add up to more than 8 times its size.
+112, 64 and 16 bytes each, add up to more than 8 times its size. It refuses a
+manifest file larger than 258 MiB without reading the rest of it: zstd's worst
+case grows a 256 MiB inner message by 1/256 to 257 MiB, and the last MiB is room
+for the signature, the signing key and the other outer fields.
 
 ## Inner Message (`MFFile`)
 
@@ -106,9 +109,12 @@ All `path` values must satisfy these invariants:
 - **No parent traversal**: 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
+  for byte, so `A.txt` and `a.txt` are two paths
 
 Implementations must validate these invariants when reading and writing
-manifests. Paths that violate these rules must be rejected.
+manifests. Paths that violate these rules must be rejected, and a reader must
+reject a manifest that lists a path more than once.
 
 ## Hash Format (`MFFileChecksum`)
 

internal/cli/check.go

@@ -108,18 +108,25 @@ func (mfa *CLIApp) fetchManifestToTemp(
 	if tmpErr != nil {
 		_ = rc.Close()
 
-		return "", fmt.Errorf("failed to create temp file: %w", tmpErr)
+		return "", tmpErr
 	}
 
 	tmpPath := tmpFile.Name()
-	_, cpErr := io.Copy(tmpFile, rc)
+
+	// Copying stops one byte past mfa.maxManifestSize, which is enough to
+	// tell that the manifest is too large.
+	written, cpErr := io.Copy(tmpFile, io.LimitReader(rc, mfa.maxManifestSize+1))
 	_ = rc.Close()
 	_ = tmpFile.Close()
 
+	if cpErr == nil && written > mfa.maxManifestSize {
+		cpErr = fmt.Errorf("%w of %d bytes", errManifestTooLarge, mfa.maxManifestSize)
+	}
+
 	if cpErr != nil {
 		_ = mfa.Fs.Remove(tmpPath)
 
-		return "", fmt.Errorf("failed to download manifest: %w", cpErr)
+		return "", fmt.Errorf("download manifest: %w", cpErr)
 	}
 
 	return tmpPath, nil
@@ -135,7 +142,7 @@ func verifyRequiredSigner(chk *mfer.Checker, requiredSigner string) error {
 
 	_, err := hex.DecodeString(requiredSigner)
 	if err != nil {
-		return fmt.Errorf("invalid fingerprint: must be valid hex: %w", err)
+		return fmt.Errorf("invalid fingerprint: %w", err)
 	}
 
 	if !chk.IsSigned() {
@@ -225,7 +232,7 @@ func findExtraFiles(
 
 	err := chk.FindExtraFiles(ctx, extraResults)
 	if err != nil {
-		return fmt.Errorf("failed to check for extra files: %w", err)
+		return fmt.Errorf("find extra files: %w", err)
 	}
 
 	<-extraDone
@@ -268,7 +275,7 @@ func runCheck(
 	progressWg.Wait()
 
 	if err != nil {
-		return 0, fmt.Errorf("check failed: %w", err)
+		return 0, fmt.Errorf("check files: %w", err)
 	}
 
 	// Wait for results processing to complete
@@ -289,14 +296,18 @@ func (mfa *CLIApp) checkManifestOperation(
 
 	manifestPath, err := mfa.resolveManifestArg(cmd)
 	if err != nil {
-		return fmt.Errorf("check: %w", err)
+		return err
 	}
 
+	// Done before a URL is swapped for the temp file it is downloaded to,
+	// whose directory is not the base.
+	basePath := resolveBasePath(cmd, manifestPath)
+
 	// URL manifests need to be downloaded to a temp file for the checker
 	if isHTTPURL(manifestPath) {
 		tmpPath, tmpErr := mfa.fetchManifestToTemp(ctx, manifestPath)
 		if tmpErr != nil {
-			return fmt.Errorf("check: %w", tmpErr)
+			return tmpErr
 		}
 
 		defer func() { _ = mfa.Fs.Remove(tmpPath) }()
@@ -304,7 +315,6 @@ func (mfa *CLIApp) checkManifestOperation(
 		manifestPath = tmpPath
 	}
 
-	basePath := cmd.String("base")
 	showProgress := cmd.Bool("progress")
 
 	log.Infof("checking manifest %s with base %s", manifestPath, basePath)
@@ -317,7 +327,7 @@ func (mfa *CLIApp) checkManifestOperation(
 		Fs:           mfa.Fs,
 	})
 	if err != nil {
-		return fmt.Errorf("failed to load manifest: %w", err)
+		return fmt.Errorf("load manifest: %w", err)
 	}
 
 	// Check signature requirement

internal/cli/entry.go

@@ -5,6 +5,7 @@ import (
 	"os"
 
 	"github.com/spf13/afero"
+	"sneak.berlin/go/mfer/mfer"
 )
 
 // NoColor disables colored output when set. Automatically true if the
@@ -56,14 +57,15 @@ func Run(appname, version, gitrev string) int {
 // 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,
-		Stdin:    opts.Stdin,
-		Stdout:   opts.Stdout,
-		Stderr:   opts.Stderr,
-		Fs:       opts.Fs,
+		appname:         opts.Appname,
+		version:         opts.Version,
+		gitrev:          opts.Gitrev,
+		exitCode:        0,
+		maxManifestSize: mfer.MaxManifestSize,
+		Stdin:           opts.Stdin,
+		Stdout:          opts.Stdout,
+		Stderr:          opts.Stderr,
+		Fs:              opts.Fs,
 	}
 
 	m.run(opts.Args)

internal/cli/entry_test.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"io"
 	"math/rand"
+	"net/http/httptest"
 	"os"
 	"path/filepath"
 	"slices"
@@ -354,6 +355,44 @@ func TestGenerateCommand(t *testing.T) {
 	assert.True(t, exists)
 }
 
+// TestGenerateRefusesTwoFilesAtOnePath runs gen on arguments whose files
+// would share a path in the manifest: two directories that each hold a.txt,
+// and one directory given twice. gen must fail while it lists the files,
+// before it hashes any, naming the path, and write no manifest.
+func TestGenerateRefusesTwoFilesAtOnePath(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range []struct {
+		name          string
+		first, second string
+	}{
+		{"two directories", testDir, "/other"},
+		{"one directory twice", testDir, testDir},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			fs := afero.NewMemMapFs()
+			require.NoError(t, fs.MkdirAll(testDir, 0o755))
+			require.NoError(t, fs.MkdirAll("/other", 0o755))
+			writeTestFile(t, fs, "/testdir/a.txt", "first")
+			writeTestFile(t, fs, "/other/a.txt", "second")
+
+			opts := testOpts([]string{
+				testApp, cmdGenerate, "-q", "-o", testOutput, tc.first, tc.second,
+			}, fs)
+			assert.Equal(t, 1, runCLI(opts))
+			assert.Contains(t, testStderr(t, opts),
+				`enumerate files: duplicate path "a.txt": `+
+					tc.first+"/a.txt and "+tc.second+"/a.txt")
+
+			exists, err := afero.Exists(fs, testOutput)
+			require.NoError(t, err)
+			assert.False(t, exists)
+		})
+	}
+}
+
 // TestGenerateSeededManifestBytes pins the exact bytes `gen --seed` writes
 // for a fixed tree, so that a Go or dependency update that changes what
 // mfer writes fails here. testdata/seeded.mf was written by an mfer built
@@ -636,7 +675,7 @@ func TestCheckRequireSignatureRefusesOtherSigningKey(t *testing.T) {
 	}, fs)
 	assert.Equal(t, 1, runCLI(opts))
 	assert.Contains(t, testStderr(t, opts),
-		"failed to load manifest: signature verification failed: "+
+		"load manifest: "+
 			"embedded public key block must hold exactly one key, found 2")
 }
 
@@ -941,6 +980,90 @@ func TestCheckNeverReportsManifest(t *testing.T) {
 	}
 }
 
+// The directory setupManifestInSubdir makes and the manifest it writes there,
+// relative to the working directory it sets.
+const (
+	testSubdir         = "sub"
+	testSubdirManifest = testSubdir + "/" + defaultManifestName
+)
+
+// setupManifestInSubdir makes a temp dir holding file.txt and sub/b.txt,
+// where sub/index.mf is the manifest gen writes for sub, and makes it the
+// working directory, so a test calling it cannot run in parallel. It returns
+// the temp dir.
+func setupManifestInSubdir(t *testing.T) string {
+	t.Helper()
+
+	root := t.TempDir()
+	sub := filepath.Join(root, testSubdir)
+
+	fs := afero.NewOsFs()
+	require.NoError(t, fs.MkdirAll(sub, 0o750))
+	writeTestFile(t, fs, filepath.Join(root, testFileTxt), "not in the manifest")
+	writeTestFile(t, fs, filepath.Join(sub, "b.txt"), "in the manifest")
+
+	opts := testOpts([]string{
+		testApp, cmdGenerate, "-q", "-o", filepath.Join(sub, defaultManifestName), sub,
+	}, fs)
+	require.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts))
+
+	t.Chdir(root)
+
+	return root
+}
+
+// TestCheckResolvesEntriesAgainstManifestDirectory runs check from the
+// directory above sub, on the manifest in sub. Without --base, the
+// manifest's entries are looked for in sub, whether check is given the
+// manifest or sub, and the files above sub are not reported. --base names
+// the directory to look in instead, the current one included.
+//
+//nolint:paralleltest // changes the process-global working directory
+func TestCheckResolvesEntriesAgainstManifestDirectory(t *testing.T) {
+	root := setupManifestInSubdir(t)
+
+	for _, tc := range []struct {
+		args     []string
+		exitCode int
+		failure  string // a line check must print, if any
+	}{
+		{[]string{testSubdir}, 0, ""},
+		{[]string{testSubdirManifest}, 0, ""},
+		{[]string{filepath.Join(root, testSubdir)}, 0, ""},
+		{[]string{testFlagBase, testSubdir, testSubdirManifest}, 0, ""},
+		{[]string{testFlagBase, ".", testSubdirManifest}, 1, "MISSING: b.txt"},
+	} {
+		t.Run(strings.Join(tc.args, " "), func(t *testing.T) {
+			opts := testOpts(slices.Concat(
+				[]string{testApp, cmdCheck, testFlagNoExtra}, tc.args,
+			), afero.NewOsFs())
+			assert.Equal(t, tc.exitCode, runCLI(opts), "stderr: %s", testStderr(t, opts))
+			assert.Contains(t, testStderr(t, opts), tc.failure)
+		})
+	}
+}
+
+// TestCheckURLManifestResolvesEntriesAgainstCurrentDirectory runs check on
+// a manifest given by URL, from a directory holding the file it lists: the
+// file is looked for there.
+//
+//nolint:paralleltest // changes the process-global working directory
+func TestCheckURLManifestResolvesEntriesAgainstCurrentDirectory(t *testing.T) {
+	files := map[string][]byte{testFileTxt: []byte("hello")}
+
+	server := httptest.NewServer(fetchTestHandler(manifestOf(t, files), files))
+	defer server.Close()
+
+	cwd := chdirTemp(t)
+	require.NoError(t,
+		os.WriteFile(filepath.Join(cwd, testFileTxt), files[testFileTxt], 0o600))
+
+	opts := testOpts([]string{
+		testApp, cmdCheck, testFlagNoExtra, server.URL + "/" + defaultManifestName,
+	}, afero.NewOsFs())
+	assert.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts))
+}
+
 // unlistableDirFs is a filesystem on which one directory cannot be listed.
 type unlistableDirFs struct {
 	afero.Fs

internal/cli/errmsg_test.go

@@ -303,7 +303,7 @@ func TestManifestLoaderHTTPStatusMessage(t *testing.T) {
 	_, err := mfa.openManifestReader(context.Background(), server.URL+"/foo.mf")
 	require.ErrorIs(t, err, errHTTPStatus)
 	assert.EqualError(t, err,
-		"failed to fetch "+server.URL+"/foo.mf: HTTP 404")
+		"download manifest "+server.URL+"/foo.mf: unexpected HTTP status 404")
 }
 
 func TestFetchManifestHTTPStatusMessage(t *testing.T) {
@@ -325,7 +325,7 @@ func TestFetchManifestHTTPStatusMessage(t *testing.T) {
 		return cmd.Run(context.Background(), []string{cmdFetch, server.URL})
 	})
 	require.ErrorIs(t, err, errHTTPStatus)
-	assert.EqualError(t, err, "failed to fetch manifest: HTTP 404")
+	assert.EqualError(t, err, "download manifest: unexpected HTTP status 404")
 }
 
 func TestFetchFileHTTPStatusMessage(t *testing.T) {
@@ -343,7 +343,108 @@ func TestFetchFileHTTPStatusMessage(t *testing.T) {
 			&mfer.MFFilePath{}, nil)
 	})
 	require.ErrorIs(t, err, errHTTPStatus)
-	assert.EqualError(t, err, "HTTP 500")
+	assert.EqualError(t, err, "unexpected HTTP status 500")
+}
+
+// TestCheckCorruptManifestMessage runs check on a file that is not a
+// manifest.
+func TestCheckCorruptManifestMessage(t *testing.T) {
+	t.Parallel()
+
+	fs := afero.NewMemMapFs()
+	require.NoError(t, afero.WriteFile(fs, "/bad.mf", []byte("not a manifest"), 0o644))
+
+	mfa := &CLIApp{Fs: fs}
+	cmd := mfa.checkCommand()
+	cmd.Action = mfa.checkManifestOperation
+
+	// checkManifestOperation logs to the process-global logger.
+	err := runLocked(func() error {
+		return cmd.Run(context.Background(), []string{cmdCheck, "/bad.mf"})
+	})
+	assert.EqualError(t, err, "load manifest: invalid file format")
+}
+
+// TestListMissingManifestMessage runs list on a manifest file that does not
+// exist.
+func TestListMissingManifestMessage(t *testing.T) {
+	t.Parallel()
+
+	mfa := &CLIApp{Fs: afero.NewMemMapFs()}
+	cmd := mfa.listCommand()
+
+	// listManifestOperation sets the process-global log level.
+	err := runLocked(func() error {
+		return cmd.Run(context.Background(), []string{cmdList, "/nope.mf"})
+	})
+	require.ErrorIs(t, err, os.ErrNotExist)
+	assert.EqualError(t, err, "open /nope.mf: file does not exist")
+}
+
+// TestFetchHashMismatchMessage runs fetch against a server that sends a
+// listed file with other content of the same size.
+func TestFetchHashMismatchMessage(t *testing.T) {
+	t.Parallel()
+
+	manifest := builtManifest(t, map[string][]byte{testFileTxt: []byte("listed")})
+
+	server := httptest.NewServer(fetchTestHandler(manifest,
+		map[string][]byte{testFileTxt: []byte("served")}))
+	defer server.Close()
+
+	mfa := &CLIApp{Fs: afero.NewMemMapFs(), maxManifestSize: mfer.MaxManifestSize}
+	cmd := mfa.fetchCommand()
+	cmd.Action = mfa.fetchManifestOperation
+
+	// fetchManifestOperation logs to the process-global logger.
+	err := runLocked(func() error {
+		return cmd.Run(context.Background(),
+			[]string{cmdFetch, "--" + flagDest, t.TempDir(), server.URL})
+	})
+	require.ErrorIs(t, err, errHashMismatch)
+	assert.EqualError(t, err, "download "+testFileTxt+": hash mismatch")
+}
+
+// TestFreshenBackslashPathMessage runs freshen on a tree that has gained a
+// file whose name holds a backslash, which a manifest path may not contain.
+func TestFreshenBackslashPathMessage(t *testing.T) {
+	t.Parallel()
+
+	fs := afero.NewOsFs()
+	root, manifestPath := setupFreshenDir(t, fs,
+		map[string]string{testFileTxt: "content"})
+	writeTestFile(t, fs, filepath.Join(root, `a\b.txt`), "new")
+
+	mfa := &CLIApp{Fs: fs}
+	cmd := mfa.freshenCommand()
+	cmd.Action = mfa.freshenManifestOperation
+
+	// freshenManifestOperation logs to the process-global logger.
+	err := runLocked(func() error {
+		return cmd.Run(context.Background(),
+			[]string{cmdFreshen, testFlagBase, root, manifestPath})
+	})
+	assert.EqualError(t, err,
+		`path "a\\b.txt" contains backslash; use forward slashes only`)
+}
+
+// TestFreshenReadErrorMessage has freshen hash a directory as though it
+// were a file, so reading it fails.
+func TestFreshenReadErrorMessage(t *testing.T) {
+	t.Parallel()
+
+	root := t.TempDir()
+	require.NoError(t, os.Mkdir(filepath.Join(root, "sub"), 0o750))
+
+	hasher := &freshenHasher{
+		fs:      afero.NewOsFs(),
+		absBase: root,
+		builder: mfer.NewBuilder(),
+	}
+
+	err := hasher.processEntry(&freshenEntry{path: "sub", needsHash: true})
+	assert.EqualError(t, err,
+		"read "+filepath.Join(root, "sub")+": is a directory")
 }
 
 func TestURLRequiredMessage(t *testing.T) {

internal/cli/export.go

@@ -26,12 +26,12 @@ func (mfa *CLIApp) exportManifestOperation(
 ) error {
 	pathOrURL, err := mfa.resolveManifestArg(cmd)
 	if err != nil {
-		return fmt.Errorf("export: %w", err)
+		return err
 	}
 
 	rc, err := mfa.openManifestReader(ctx, pathOrURL)
 	if err != nil {
-		return fmt.Errorf("export: %w", err)
+		return err
 	}
 
 	defer func() { _ = rc.Close() }()
@@ -39,7 +39,7 @@ func (mfa *CLIApp) exportManifestOperation(
 	//nolint:contextcheck // mfer loads a manifest without a context
 	manifest, err := mfer.NewManifestFromReader(rc)
 	if err != nil {
-		return fmt.Errorf("export: failed to parse manifest: %w", err)
+		return fmt.Errorf("parse manifest: %w", err)
 	}
 
 	files := manifest.Files()
@@ -76,7 +76,7 @@ func (mfa *CLIApp) exportManifestOperation(
 
 	err = enc.Encode(entries)
 	if err != nil {
-		return fmt.Errorf("export: failed to encode JSON: %w", err)
+		return fmt.Errorf("encode JSON: %w", err)
 	}
 
 	return nil

internal/cli/fetch.go

@@ -239,7 +239,7 @@ func reportDownloadProgress(progress <-chan DownloadProgress, done chan<- struct
 func manifestBaseURL(manifestURL string) (*url.URL, error) {
 	parsed, err := url.Parse(manifestURL)
 	if err != nil {
-		return nil, fmt.Errorf("fetch: invalid manifest URL: %w", err)
+		return nil, fmt.Errorf("invalid manifest URL: %w", err)
 	}
 
 	// JoinPath cleans the path it builds, so ".." drops the manifest's
@@ -268,7 +268,7 @@ func downloadManifestFiles(
 		// Sanitize the path to prevent path traversal attacks
 		localPath, err := sanitizePath(f.GetPath())
 		if err != nil {
-			return 0, 0, fmt.Errorf("invalid path in manifest: %w", err)
+			return 0, 0, fmt.Errorf("invalid file entry: %w", err)
 		}
 
 		if alreadyPresent(dest, localPath, f) {
@@ -284,7 +284,7 @@ func downloadManifestFiles(
 
 		err = downloadFile(ctx, client, fileURL, dest, localPath, f, progress)
 		if err != nil {
-			return 0, 0, fmt.Errorf("failed to download %s: %w", f.GetPath(), err)
+			return 0, 0, fmt.Errorf("download %s: %w", f.GetPath(), err)
 		}
 
 		downloaded++
@@ -361,7 +361,7 @@ func (mfa *CLIApp) fetchManifestOperation(
 		firstDelay: firstRetryDelay,
 	}
 
-	manifestData, files, err := fetchManifest(ctx, cmd, client, manifestURL)
+	manifestData, files, err := mfa.fetchManifest(ctx, cmd, client, manifestURL)
 	if err != nil {
 		return err
 	}
@@ -376,7 +376,7 @@ func (mfa *CLIApp) fetchManifestOperation(
 
 	err = os.MkdirAll(dest, dirPerms)
 	if err != nil {
-		return fmt.Errorf("failed to create destination directory %s: %w", dest, err)
+		return err
 	}
 
 	// Create progress channel and start progress reporter goroutine
@@ -403,7 +403,7 @@ func (mfa *CLIApp) fetchManifestOperation(
 	// "mfer check" can verify the tree later.
 	err = saveManifest(dest, manifestData)
 	if err != nil {
-		return fmt.Errorf("failed to save manifest: %w", err)
+		return fmt.Errorf("save manifest: %w", err)
 	}
 
 	// Print summary
@@ -426,32 +426,39 @@ func (mfa *CLIApp) fetchManifestOperation(
 // that lists a file where fetch writes another or a mode outside 0777. It
 // returns the manifest as downloaded, to be saved once the files are in
 // place, and the files it lists.
-func fetchManifest(
+func (mfa *CLIApp) fetchManifest(
 	ctx context.Context, cmd *cli.Command, client retryingClient, manifestURL string,
 ) ([]byte, []*mfer.MFFilePath, error) {
 	log.Infof("fetching manifest from %s", manifestURL)
 
 	// Read the whole manifest before parsing it, so that a connection
 	// lost partway through is retried rather than reported as a bad
-	// manifest.
+	// manifest. Reading stops one byte past mfa.maxManifestSize, which is
+	// enough to tell that the manifest is too large.
 	var manifestData []byte
 
 	err := client.get(ctx, manifestURL, func(resp *http.Response) error {
 		var readErr error
 
-		manifestData, readErr = io.ReadAll(resp.Body)
+		manifestData, readErr = io.ReadAll(
+			io.LimitReader(resp.Body, mfa.maxManifestSize+1))
 
 		return readErr
 	})
 	if err != nil {
-		return nil, nil, fmt.Errorf("failed to fetch manifest: %w", err)
+		return nil, nil, fmt.Errorf("download manifest: %w", err)
+	}
+
+	if int64(len(manifestData)) > mfa.maxManifestSize {
+		return nil, nil, fmt.Errorf("download manifest: %w of %d bytes",
+			errManifestTooLarge, mfa.maxManifestSize)
 	}
 
 	// Parse manifest
 	//nolint:contextcheck // mfer loads a manifest without a context
 	manifest, err := mfer.NewManifestFromReader(bytes.NewReader(manifestData))
 	if err != nil {
-		return nil, nil, fmt.Errorf("failed to parse manifest: %w", err)
+		return nil, nil, fmt.Errorf("parse manifest: %w", err)
 	}
 
 	requiredSigner := cmd.String(flagRequireSignature)
@@ -554,7 +561,7 @@ func verifyFetchedSigner(manifestData []byte, requiredSigner string) error {
 		Fs:           memFs,
 	})
 	if err != nil {
-		return fmt.Errorf("failed to load manifest: %w", err)
+		return fmt.Errorf("load manifest: %w", err)
 	}
 
 	return verifyRequiredSigner(chk, requiredSigner)
@@ -646,7 +653,7 @@ func checkNoSymlinks(dest, p string) error {
 		}
 
 		if err != nil {
-			return fmt.Errorf("failed to check %s for a symlink: %w", current, err)
+			return err
 		}
 
 		if info.Mode()&os.ModeSymlink != 0 {
@@ -763,7 +770,7 @@ func tempPathFor(localPath string) string {
 func verifyDownloadedHash(digest []byte, entry *mfer.MFFilePath) error {
 	computed, err := multihash.Encode(digest, multihash.SHA2_256)
 	if err != nil {
-		return fmt.Errorf("failed to encode hash: %w", err)
+		return fmt.Errorf("encode hash: %w", err)
 	}
 
 	for _, hash := range entry.GetHashes() {
@@ -790,7 +797,7 @@ func downloadFile(
 	// so every entry point to downloadFile gets the same treatment.
 	localPath, err := sanitizePath(localPath)
 	if err != nil {
-		return fmt.Errorf("invalid path: %w", err)
+		return fmt.Errorf("invalid file entry: %w", err)
 	}
 
 	// Create parent directories if needed
@@ -805,7 +812,7 @@ func downloadFile(
 
 		err = os.MkdirAll(dir, dirPerms)
 		if err != nil {
-			return fmt.Errorf("failed to create directory %s: %w", dir, err)
+			return err
 		}
 	}
 
@@ -839,7 +846,7 @@ func createTempFile(dest, tmpPath string) (*os.File, error) {
 	out, err := os.OpenFile( //nolint:gosec // G304: see comment above
 		path, os.O_RDWR|os.O_CREATE|os.O_EXCL, filePerms)
 	if err != nil {
-		return nil, fmt.Errorf("failed to create temp file: %w", err)
+		return nil, err
 	}
 
 	return out, nil
@@ -852,12 +859,7 @@ func moveIntoPlace(dest, tmpPath, localPath string) error {
 		return err
 	}
 
-	err = os.Rename(filepath.Join(dest, tmpPath), filepath.Join(dest, localPath))
-	if err != nil {
-		return fmt.Errorf("failed to rename temp file: %w", err)
-	}
-
-	return nil
+	return os.Rename(filepath.Join(dest, tmpPath), filepath.Join(dest, localPath))
 }
 
 // saveResponse writes resp's body to tmpPath, verifies it against entry,
@@ -892,7 +894,7 @@ func saveResponse(
 			_ = out.Close()
 			_ = os.Remove(filepath.Join(dest, tmpPath))
 
-			return fmt.Errorf("failed to set mode: %w", err)
+			return err
 		}
 	}
 
@@ -908,8 +910,10 @@ func saveResponse(
 		progress:  progress,
 	}
 
-	// Copy content while hashing and reporting progress
-	written, copyErr := io.Copy(pw, resp.Body)
+	// Copy content while hashing and reporting progress. One byte past
+	// the listed size is enough for finishDownload to report a size
+	// mismatch.
+	written, copyErr := io.Copy(pw, io.LimitReader(resp.Body, expectedSize+1))
 
 	// Close file before checking errors (to flush writes)
 	closeErr := out.Close()

internal/cli/fetch_test.go

@@ -26,6 +26,7 @@ import (
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	urfcli "github.com/urfave/cli/v3"
 	"google.golang.org/protobuf/proto"
 	"sneak.berlin/go/mfer/mfer"
 )
@@ -441,6 +442,76 @@ func TestFetchSizeMismatch(t *testing.T) {
 		"temp file should be cleaned up on size mismatch")
 }
 
+// zeros is an io.Reader of zero bytes without end.
+type zeros struct{}
+
+func (zeros) Read(p []byte) (int, error) {
+	clear(p)
+
+	return len(p), nil
+}
+
+// TestFetchStopsReadingFilePastListedSize serves a body that never ends
+// for a file listed at 16 bytes. fetch must stop reading one byte past
+// the listed size, report the size mismatch and remove its temp file.
+//
+//nolint:paralleltest // changes the process-global working directory
+func TestFetchStopsReadingFilePastListedSize(t *testing.T) {
+	server := httptest.NewServer(
+		http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+			_, _ = io.Copy(w, zeros{})
+		}))
+	defer server.Close()
+
+	chdirTemp(t)
+
+	err := downloadFile(context.Background(), testClient(),
+		server.URL+"/"+testFileTxt, ".", testFileTxt,
+		&mfer.MFFilePath{Path: testFileTxt, Size: 16}, nil)
+	require.ErrorIs(t, err, errSizeMismatch)
+	require.EqualError(t, err, "size mismatch: expected 16 bytes, got 17")
+	assert.NoFileExists(t, tempPathFor(testFileTxt))
+}
+
+// TestManifestDownloadStopsAtLimit serves a manifest that never ends to
+// fetch and to check, with the most they download of a manifest lowered
+// to 64 KiB. Each must stop reading at that limit, fail with an error
+// naming it and leave no temp file.
+func TestManifestDownloadStopsAtLimit(t *testing.T) {
+	server := httptest.NewServer(
+		http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+			_, _ = io.Copy(w, zeros{})
+		}))
+	defer server.Close()
+
+	tmpDir := t.TempDir()
+	t.Setenv("TMPDIR", tmpDir)
+
+	mfa := &CLIApp{Fs: afero.NewOsFs(), maxManifestSize: 64 << 10}
+
+	fetch := mfa.fetchCommand()
+	fetch.Action = mfa.fetchManifestOperation
+
+	check := mfa.checkCommand()
+	check.Action = mfa.checkManifestOperation
+
+	for _, cmd := range []*urfcli.Command{fetch, check} {
+		// Both operations log to the process-global logger.
+		err := runLocked(func() error {
+			return cmd.Run(context.Background(),
+				[]string{cmd.Name, server.URL + "/index.mf"})
+		})
+		require.ErrorIs(t, err, errManifestTooLarge, cmd.Name)
+		require.EqualError(t, err,
+			"download manifest: file exceeds maximum allowed size of 65536 bytes",
+			cmd.Name)
+	}
+
+	leftover, err := os.ReadDir(tmpDir)
+	require.NoError(t, err)
+	assert.Empty(t, leftover)
+}
+
 //nolint:paralleltest // changes the process-global working directory
 func TestFetchProgress(t *testing.T) {
 	// Create source filesystem with a larger test file
@@ -535,27 +606,27 @@ func TestFetchRefusesSymlinks(t *testing.T) {
 	}{
 		{
 			"parent directory", "sub/deeper/file.txt", "sub", ".",
-			"failed to download sub/deeper/file.txt",
+			"download sub/deeper/file.txt",
 		},
 		{
 			"directory inside a plain directory", "docs/data/passwd", "docs/data", ".",
-			"failed to download docs/data/passwd",
+			"download docs/data/passwd",
 		},
 		{
 			"temp file", testFileTxt, ".file.txt.tmp", newFile,
-			"failed to download " + testFileTxt,
+			"download " + testFileTxt,
 		},
 		{
 			"file", testFileTxt, testFileTxt, newFile,
-			"failed to download " + testFileTxt,
+			"download " + testFileTxt,
 		},
 		{
 			"manifest temp file", testFileTxt, tempPathFor(defaultManifestName), newFile,
-			"failed to save manifest",
+			"save manifest",
 		},
 		{
 			"manifest", testFileTxt, defaultManifestName, newFile,
-			"failed to save manifest",
+			"save manifest",
 		},
 	}
 
@@ -622,7 +693,7 @@ func TestFetchDoesNotSkipThroughSymlink(t *testing.T) {
 	}, afero.NewOsFs())
 	assert.Equal(t, 1, runCLI(opts))
 	assert.Contains(t, testStderr(t, opts),
-		"failed to download sub/"+testFileTxt+": symlink in path not allowed: "+link)
+		"download sub/"+testFileTxt+": symlink in path not allowed: "+link)
 	assert.Equal(t, map[string][]byte{testFileTxt: content}, filesUnder(t, outside))
 }
 
@@ -692,7 +763,8 @@ func TestGetRetriesTransientStatusesOnly(t *testing.T) {
 
 			err := getNothing(testClient(), server.URL, 10*time.Second)
 			require.ErrorIs(t, err, errHTTPStatus)
-			require.EqualError(t, err, fmt.Sprintf("HTTP %d", tt.status))
+			require.EqualError(t, err,
+				fmt.Sprintf("unexpected HTTP status %d", tt.status))
 			assert.Equal(t, tt.requests, requests.Load())
 		})
 	}
@@ -1158,7 +1230,7 @@ func TestFetchRequireSignature(t *testing.T) {
 		manifest, required := manifestSignedByAnotherKey(t, files)
 
 		assertFetchRefused(t, manifest, files,
-			"failed to parse manifest: signature verification failed: "+
+			"parse manifest: "+
 				"embedded public key block must hold exactly one key, found 2",
 			"--"+flagRequireSignature, required)
 	})

internal/cli/freshen.go

@@ -165,8 +165,7 @@ func (s *freshenScanner) walk(path string, info fs.FileInfo, walkErr error) erro
 	// Get relative path
 	relPath, err := filepath.Rel(s.absBase, path)
 	if err != nil {
-		return fmt.Errorf(
-			"freshen: failed to compute relative path for %s: %w", path, err)
+		return err
 	}
 
 	// Handle dotfiles
@@ -280,13 +279,8 @@ func (h *freshenHasher) reportProgress(n int64) {
 // processEntry hashes the entry if needed and adds it to the builder.
 func (h *freshenHasher) processEntry(e *freshenEntry) error {
 	if !e.needsHash {
-		// Use existing entry
-		err := addExistingToBuilder(h.builder, e.existing)
-		if err != nil {
-			return fmt.Errorf("failed to add %s: %w", e.path, err)
-		}
-
-		return nil
+		// Use existing entry; the error names the entry
+		return addExistingToBuilder(h.builder, e.existing)
 	}
 
 	// Need to read and hash the file
@@ -294,26 +288,21 @@ func (h *freshenHasher) processEntry(e *freshenEntry) error {
 
 	f, err := h.fs.Open(absPath)
 	if err != nil {
-		return fmt.Errorf("failed to open %s: %w", e.path, err)
+		return err
 	}
 
 	hash, bytesRead, err := hashFile(f, h.reportProgress)
 	_ = f.Close()
 
 	if err != nil {
-		return fmt.Errorf("failed to hash %s: %w", e.path, err)
+		return err
 	}
 
 	h.hashedBytes += bytesRead
 	h.hashedFiles++
 
-	// Add to builder with computed hash
-	err = addFileToBuilder(h.builder, e.path, e.size, e.mtime, e.mode, hash)
-	if err != nil {
-		return fmt.Errorf("failed to add %s: %w", e.path, err)
-	}
-
-	return nil
+	// Add to builder with computed hash; a refused path is named in the error
+	return addFileToBuilder(h.builder, e.path, e.size, e.mtime, e.mode, hash)
 }
 
 // writeFreshenedManifest writes the manifest atomically (write to a
@@ -325,7 +314,7 @@ func writeFreshenedManifest(
 
 	outFile, err := afs.Create(tmpPath)
 	if err != nil {
-		return fmt.Errorf("failed to create temp file: %w", err)
+		return err
 	}
 
 	err = builder.Build(ctx, outFile)
@@ -334,7 +323,7 @@ func writeFreshenedManifest(
 	if err != nil {
 		_ = afs.Remove(tmpPath)
 
-		return fmt.Errorf("failed to write manifest: %w", err)
+		return fmt.Errorf("build manifest: %w", err)
 	}
 
 	// Rename temp to final
@@ -342,7 +331,7 @@ func writeFreshenedManifest(
 	if err != nil {
 		_ = afs.Remove(tmpPath)
 
-		return fmt.Errorf("failed to rename manifest: %w", err)
+		return err
 	}
 
 	return nil
@@ -408,7 +397,7 @@ func (mfa *CLIApp) freshenScan(
 	}
 
 	if err != nil {
-		return nil, 0, fmt.Errorf("failed to scan filesystem: %w", err)
+		return nil, 0, fmt.Errorf("scan filesystem: %w", err)
 	}
 
 	// Remaining entries in existingByPath are removed files
@@ -477,7 +466,7 @@ func (mfa *CLIApp) loadExistingEntries(
 		Fs:   mfa.Fs,
 	})
 	if err != nil {
-		return nil, fmt.Errorf("failed to load manifest: %w", err)
+		return nil, fmt.Errorf("load manifest: %w", err)
 	}
 
 	existingFiles := manifest.Files()
@@ -497,13 +486,12 @@ func (mfa *CLIApp) freshenManifestOperation(
 ) error {
 	log.Debug("freshenManifestOperation()")
 
-	basePath := cmd.String("base")
 	showProgress := cmd.Bool("progress")
 
 	// Find manifest file
 	manifestPath, err := mfa.resolveFreshenManifestPath(cmd)
 	if err != nil {
-		return fmt.Errorf("freshen: %w", err)
+		return err
 	}
 
 	//nolint:contextcheck // mfer loads a manifest without a context
@@ -512,9 +500,9 @@ func (mfa *CLIApp) freshenManifestOperation(
 		return err
 	}
 
-	absBase, err := filepath.Abs(basePath)
+	absBase, err := filepath.Abs(resolveBasePath(cmd, manifestPath))
 	if err != nil {
-		return fmt.Errorf("freshen: invalid base path: %w", err)
+		return fmt.Errorf("invalid base path: %w", err)
 	}
 
 	// Phase 1: Scan filesystem
@@ -606,9 +594,7 @@ func hashFile(r io.Reader, progress func(int64)) ([]byte, int64, error) {
 			break
 		}
 
-		// Returned unwrapped: the caller renders this as
-		// "failed to hash <path>: <err>" and adding a second layer here
-		// would change that message.
+		// Returned unwrapped: a read error already names the file.
 		if err != nil {
 			return nil, total, err
 		}

internal/cli/freshen_test.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"path/filepath"
 	"slices"
+	"strings"
 	"testing"
 	"time"
 
@@ -300,6 +301,41 @@ func TestFreshenLeavesLeftoverTempFileOutOfListing(t *testing.T) {
 		manifestPaths(t, fs, manifestPath))
 }
 
+// TestFreshenResolvesEntriesAgainstManifestDirectory adds sub/c.txt, then
+// runs freshen from the directory above sub, on the manifest in sub.
+// Without --base, the manifest then lists the files in sub, whether freshen
+// is given the manifest or sub. --base names the directory to list instead,
+// the current one included.
+//
+//nolint:paralleltest // changes the process-global working directory
+func TestFreshenResolvesEntriesAgainstManifestDirectory(t *testing.T) {
+	for _, tc := range []struct {
+		args []string
+		want []string // the paths the manifest lists afterwards
+	}{
+		{[]string{testSubdir}, []string{"b.txt", "c.txt"}},
+		{[]string{testSubdirManifest}, []string{"b.txt", "c.txt"}},
+		{
+			[]string{testFlagBase, ".", testSubdirManifest},
+			[]string{testFileTxt, "sub/b.txt", "sub/c.txt"},
+		},
+	} {
+		t.Run(strings.Join(tc.args, " "), func(t *testing.T) {
+			fs := afero.NewOsFs()
+			root := setupManifestInSubdir(t)
+			writeTestFile(t, fs, filepath.Join(root, testSubdir, "c.txt"), "added")
+
+			opts := testOpts(slices.Concat(
+				[]string{testApp, cmdFreshen, "-q"}, tc.args,
+			), fs)
+			require.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts))
+
+			assert.ElementsMatch(t, tc.want, manifestPaths(t, fs,
+				filepath.Join(root, testSubdirManifest)))
+		})
+	}
+}
+
 // TestFreshenRecordEntryMtimePresence pins the behavior of recordEntry
 // with respect to MFFilePath.Mtime, which is a message pointer with
 // proto3 field presence and may legitimately be absent.

internal/cli/gen.go

@@ -74,7 +74,7 @@ func (mfa *CLIApp) collectInputPaths(args cli.Args) ([]string, error) {
 
 		ap, err := filepath.Abs(inputPath)
 		if err != nil {
-			return nil, fmt.Errorf("generate: invalid path %q: %w", inputPath, err)
+			return nil, fmt.Errorf("invalid path %q: %w", inputPath, err)
 		}
 		// Validate path exists before adding to list
 		if exists, _ := afero.Exists(mfa.Fs, ap); !exists {
@@ -129,8 +129,7 @@ func (mfa *CLIApp) enumerateInputs(
 		// Default to current directory
 		err := s.EnumeratePath(".", enumProgress)
 		if err != nil {
-			return fmt.Errorf(
-				"generate: failed to enumerate current directory: %w", err)
+			return fmt.Errorf("enumerate current directory: %w", err)
 		}
 
 		return nil
@@ -144,7 +143,7 @@ func (mfa *CLIApp) enumerateInputs(
 
 	err = s.EnumeratePaths(enumProgress, paths...)
 	if err != nil {
-		return fmt.Errorf("generate: failed to enumerate paths: %w", err)
+		return fmt.Errorf("enumerate files: %w", err)
 	}
 
 	return nil
@@ -229,7 +228,7 @@ func (mfa *CLIApp) generateManifestOperation(
 
 	outFile, err := mfa.Fs.Create(tmpPath)
 	if err != nil {
-		return fmt.Errorf("failed to create temp file: %w", err)
+		return err
 	}
 
 	// Set up signal handler to clean up temp file on Ctrl-C
@@ -268,19 +267,19 @@ func (mfa *CLIApp) generateManifestOperation(
 	scanWg.Wait()
 
 	if err != nil {
-		return fmt.Errorf("failed to generate manifest: %w", err)
+		return fmt.Errorf("generate manifest: %w", err)
 	}
 
 	// Close file before rename to ensure all data is flushed
 	err = outFile.Close()
 	if err != nil {
-		return fmt.Errorf("failed to close temp file: %w", err)
+		return err
 	}
 
 	// Atomic rename
 	err = mfa.Fs.Rename(tmpPath, outputPath)
 	if err != nil {
-		return fmt.Errorf("failed to rename temp file: %w", err)
+		return err
 	}
 
 	success = true

internal/cli/list.go

@@ -19,12 +19,12 @@ func (mfa *CLIApp) listManifestOperation(ctx context.Context, cmd *cli.Command)
 
 	pathOrURL, err := mfa.resolveManifestArg(cmd)
 	if err != nil {
-		return fmt.Errorf("list: %w", err)
+		return err
 	}
 
 	rc, err := mfa.openManifestReader(ctx, pathOrURL)
 	if err != nil {
-		return fmt.Errorf("list: %w", err)
+		return err
 	}
 
 	defer func() { _ = rc.Close() }()
@@ -32,7 +32,7 @@ func (mfa *CLIApp) listManifestOperation(ctx context.Context, cmd *cli.Command)
 	//nolint:contextcheck // mfer loads a manifest without a context
 	manifest, err := mfer.NewManifestFromReader(rc)
 	if err != nil {
-		return fmt.Errorf("list: failed to parse manifest: %w", err)
+		return fmt.Errorf("parse manifest: %w", err)
 	}
 
 	files := manifest.Files()

internal/cli/manifest_loader.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"path/filepath"
 	"strings"
 	"time"
 
@@ -15,13 +16,13 @@ import (
 // manifestFetchTimeout bounds HTTP requests made to fetch a manifest.
 const manifestFetchTimeout = 30 * time.Second
 
-// errHTTPStatus indicates an HTTP response with a non-OK status code.
-//
-// Its text is the literal "HTTP" prefix of the rendered "HTTP <code>"
-// message that mfer has always printed, so that wrapping it does not
-// change any user-visible output. Match it with errors.Is; do not read
-// its message.
-var errHTTPStatus = errors.New("HTTP")
+// errHTTPStatus indicates an HTTP response with a non-OK status code. It is
+// followed by the code, as in "unexpected HTTP status 404".
+var errHTTPStatus = errors.New("unexpected HTTP status")
+
+// errManifestTooLarge indicates a manifest download that passed
+// CLIApp.maxManifestSize.
+var errManifestTooLarge = errors.New("file exceeds maximum allowed size")
 
 // isHTTPURL returns true if the string starts with http:// or https://.
 func isHTTPURL(s string) bool {
@@ -36,20 +37,22 @@ func (mfa *CLIApp) openManifestReader(
 	if isHTTPURL(pathOrURL) {
 		client := &http.Client{Timeout: manifestFetchTimeout}
 
+		// The *url.Error that NewRequestWithContext and Do return names
+		// the URL.
 		req, err := http.NewRequestWithContext(ctx, http.MethodGet, pathOrURL, nil)
 		if err != nil {
-			return nil, fmt.Errorf("failed to fetch %s: %w", pathOrURL, err)
+			return nil, fmt.Errorf("download manifest: %w", err)
 		}
 
 		resp, err := client.Do(req)
 		if err != nil {
-			return nil, fmt.Errorf("failed to fetch %s: %w", pathOrURL, err)
+			return nil, fmt.Errorf("download manifest: %w", err)
 		}
 
 		if resp.StatusCode != http.StatusOK {
 			_ = resp.Body.Close()
 
-			return nil, fmt.Errorf("failed to fetch %s: %w %d",
+			return nil, fmt.Errorf("download manifest %s: %w %d",
 				pathOrURL, errHTTPStatus, resp.StatusCode)
 		}
 
@@ -84,3 +87,17 @@ func (mfa *CLIApp) resolveManifestArg(cmd *cli.Command) (string, error) {
 
 	return findManifest(mfa.Fs, ".")
 }
+
+// resolveBasePath returns the directory a manifest's paths are resolved
+// against: the one --base names, or else the directory holding the manifest,
+// or the current directory for a manifest URL.
+func resolveBasePath(cmd *cli.Command, manifestPath string) string {
+	switch {
+	case cmd.IsSet(flagBase):
+		return cmd.String(flagBase)
+	case isHTTPURL(manifestPath):
+		return "."
+	default:
+		return filepath.Dir(manifestPath)
+	}
+}

internal/cli/mfer.go

@@ -24,6 +24,7 @@ const (
 	cmdList     = "list"
 	cmdVersion  = "version"
 
+	flagBase               = "base"
 	flagProgress           = "progress"
 	flagTimeout            = "timeout"
 	flagDest               = "dest"
@@ -59,6 +60,10 @@ type CLIApp struct {
 	exitCode    int
 	app         *cli.Command
 
+	// maxManifestSize is the most of a manifest that fetch, and check
+	// given a URL, download: mfer.MaxManifestSize, which tests lower.
+	maxManifestSize int64
+
 	Stdin  io.Reader // Standard input stream
 	Stdout io.Writer // Standard output stream for normal output
 	Stderr io.Writer // Standard error stream for diagnostics
@@ -255,10 +260,11 @@ func (mfa *CLIApp) checkCommand() *cli.Command {
 		},
 		Flags: append(commonFlags(),
 			&cli.StringFlag{
-				Name:    "base",
+				Name:    flagBase,
 				Aliases: []string{"b"},
-				Value:   ".",
-				Usage:   "Base directory for resolving relative paths from manifest",
+				Usage: "Base directory for resolving relative paths from manifest " +
+					"(by default the directory holding the manifest, or the " +
+					"current directory for a manifest URL)",
 			},
 			&cli.BoolFlag{
 				Name:    flagProgress,
@@ -288,10 +294,10 @@ func (mfa *CLIApp) freshenCommand() *cli.Command {
 		},
 		Flags: append(commonFlags(),
 			&cli.StringFlag{
-				Name:    "base",
+				Name:    flagBase,
 				Aliases: []string{"b"},
-				Value:   ".",
-				Usage:   "Base directory for resolving relative paths",
+				Usage: "Base directory for resolving relative paths " +
+					"(by default the directory holding the manifest)",
 			},
 			&cli.BoolFlag{
 				Name:    "follow-symlinks",

mfer/builder.go

@@ -38,6 +38,7 @@ var (
 	errNegativeSize     = errors.New("size cannot be negative")
 	errHashNotMultihash = errors.New("hash is not a valid multihash")
 	errHashTooShort     = errors.New("hash digest is too short")
+	errDuplicatePath    = errors.New("duplicate path")
 )
 
 // ValidatePath checks that a file path conforms to manifest path invariants:
@@ -115,6 +116,7 @@ type FileHashProgress struct {
 type Builder struct {
 	mu                sync.Mutex
 	files             []*MFFilePath
+	paths             map[string]bool // the path of each entry in files
 	createdAt         time.Time
 	includeTimestamps bool
 	signingOptions    *SigningOptions
@@ -125,6 +127,7 @@ type Builder struct {
 func NewBuilder() *Builder {
 	return &Builder{
 		files:     make([]*MFFilePath, 0),
+		paths:     make(map[string]bool),
 		createdAt: time.Now(),
 	}
 }
@@ -138,6 +141,7 @@ func (b *Builder) SetSeed(seed string) {
 }
 
 // AddFile reads file content from reader, computes hashes, and adds to manifest.
+// A path already added is refused once the file is read.
 // Only mode's permission bits (mode.Perm()) are recorded; 0 records none.
 // Progress updates are sent to the progress channel (if non-nil) without blocking.
 // Returns the number of bytes read.
@@ -204,11 +208,7 @@ func (b *Builder) AddFile(
 		Mode:  uint32(mode.Perm()),
 	}
 
-	b.mu.Lock()
-	b.files = append(b.files, entry)
-	b.mu.Unlock()
-
-	return totalRead, nil
+	return totalRead, b.addEntry(entry)
 }
 
 // sendFileHashProgress sends a progress update without blocking.
@@ -234,8 +234,9 @@ func (b *Builder) FileCount() int {
 // AddFileWithHash adds a file entry with a pre-computed hash.
 // This is useful when the hash is already known (e.g., from an existing manifest).
 // Only mode's permission bits (mode.Perm()) are recorded; 0 records none.
-// Returns an error if path is invalid, size is negative, or hash is not a
-// multihash with a digest of at least 32 bytes, as long as SHA-256's.
+// Returns an error if path is invalid or already added, size is negative,
+// or hash is not a multihash with a digest of at least 32 bytes, as long
+// as SHA-256's.
 func (b *Builder) AddFileWithHash(
 	path RelFilePath,
 	size FileSize,
@@ -245,7 +246,7 @@ func (b *Builder) AddFileWithHash(
 ) error {
 	err := ValidatePath(string(path))
 	if err != nil {
-		return fmt.Errorf("add file: %w", err)
+		return err
 	}
 
 	if size < 0 {
@@ -277,11 +278,7 @@ func (b *Builder) AddFileWithHash(
 		Mode:  uint32(mode.Perm()),
 	}
 
-	b.mu.Lock()
-	b.files = append(b.files, entry)
-	b.mu.Unlock()
-
-	return nil
+	return b.addEntry(entry)
 }
 
 // SetIncludeTimestamps controls whether the manifest includes a createdAt timestamp.
@@ -332,20 +329,33 @@ func (b *Builder) Build(ctx context.Context, w io.Writer) error {
 	// Generate outer wrapper
 	err := m.generateOuter(ctx)
 	if err != nil {
-		return fmt.Errorf("build: generate outer: %w", err)
+		return err
 	}
 
 	// Generate final output
 	err = m.generate(ctx)
 	if err != nil {
-		return fmt.Errorf("build: generate: %w", err)
+		return err
 	}
 
 	// Write to output
 	_, err = w.Write(m.output.Bytes())
-	if err != nil {
-		return fmt.Errorf("build: write output: %w", err)
+
+	return err
+}
+
+// addEntry adds entry to the manifest unless an entry with its path is
+// already there.
+func (b *Builder) addEntry(entry *MFFilePath) error {
+	b.mu.Lock()
+	defer b.mu.Unlock()
+
+	if b.paths[entry.GetPath()] {
+		return fmt.Errorf("%w %q", errDuplicatePath, entry.GetPath())
 	}
 
+	b.paths[entry.GetPath()] = true
+	b.files = append(b.files, entry)
+
 	return nil
 }

mfer/builder_test.go

@@ -125,6 +125,32 @@ func TestBuilderAddFileWithHashRejectsBadHashes(t *testing.T) {
 	}
 }
 
+// TestBuilderRefusesPathAlreadyAdded adds a path, then adds it again with
+// AddFile and with AddFileWithHash. Each must refuse it, naming it, and
+// keep the one entry already added.
+func TestBuilderRefusesPathAlreadyAdded(t *testing.T) {
+	t.Parallel()
+
+	hash, err := multihash.Encode(make([]byte, sha256.Size), multihash.SHA2_256)
+	require.NoError(t, err)
+
+	b := NewBuilder()
+	require.NoError(t, b.AddFileWithHash("dir/a.txt", 4, ModTime{}, 0, hash))
+
+	content := []byte("data")
+	_, err = b.AddFile(
+		"dir/a.txt", FileSize(len(content)), ModTime{}, 0, bytes.NewReader(content), nil,
+	)
+	require.ErrorIs(t, err, errDuplicatePath)
+	require.EqualError(t, err, `duplicate path "dir/a.txt"`)
+
+	err = b.AddFileWithHash("dir/a.txt", 4, ModTime{}, 0, hash)
+	require.ErrorIs(t, err, errDuplicatePath)
+	require.EqualError(t, err, `duplicate path "dir/a.txt"`)
+
+	assert.Equal(t, 1, b.FileCount())
+}
+
 func TestBuilderBuild(t *testing.T) {
 	t.Parallel()
 

mfer/constants.go

@@ -8,10 +8,17 @@ const (
 	ReleaseDate = "2025-12-17"
 
 	// MaxDecompressedSize is the maximum allowed size of decompressed manifest
-	// data (256 MB). This prevents decompression bombs from consuming excessive
+	// data (256 MiB). This prevents decompression bombs from consuming excessive
 	// memory.
 	MaxDecompressedSize int64 = 256 * 1024 * 1024
 
+	// MaxManifestSize is the largest manifest file mfer reads (258 MiB).
+	// zstd's worst case grows data it cannot compress by 1/256, so an inner
+	// message of MaxDecompressedSize compresses to at most 257 MiB; the
+	// last MiB is room for the signature, the signing key and the other
+	// outer fields.
+	MaxManifestSize = MaxDecompressedSize + MaxDecompressedSize/256 + 1<<20
+
 	// zstdWindowSize is the zstd window zstd.SpeedBestCompression gives mfer's writer.
 	zstdWindowSize = 8 << 20
 

mfer/deserialize.go

@@ -24,12 +24,12 @@ var (
 	errCompressedHashWrong  = errors.New("compressed data hash mismatch")
 	errSignatureNoPubKey    = errors.New("signature present but no public key")
 	errDecompressedTooLarge = errors.New("decompressed data exceeds maximum allowed size")
+	errManifestTooLarge     = errors.New("file exceeds maximum allowed size")
 	errUUIDMismatch         = errors.New("outer and inner UUID mismatch")
 	errInvalidFileFormat    = errors.New("invalid file format")
-	errInvalidManifestPath  = errors.New("manifest contains invalid path")
-	errDecodedTooLarge      = errors.New(
-		"manifest would take too much memory to decode")
-	errSignerNotSigningKey = errors.New(
+	errInvalidManifestPath  = errors.New("invalid file entry")
+	errDecodedTooLarge      = errors.New("too much memory needed")
+	errSignerNotSigningKey  = errors.New(
 		"signer is not the fingerprint of the key that made the signature")
 )
 
@@ -57,7 +57,7 @@ func (m *manifest) validateOuterHeader() error {
 	// Validate outer UUID before any decompression
 	err := validateUUID(m.pbOuter.GetUuid())
 	if err != nil {
-		return fmt.Errorf("outer UUID invalid: %w", err)
+		return fmt.Errorf("outer message: %w", err)
 	}
 
 	return nil
@@ -72,7 +72,7 @@ func (m *manifest) verifyOuterIntegrity() error {
 
 	_, err := h.Write(m.pbOuter.GetInnerMessage())
 	if err != nil {
-		return fmt.Errorf("deserialize: hash write: %w", err)
+		return fmt.Errorf("hash inner message: %w", err)
 	}
 
 	sha256Hash := h.Sum(nil)
@@ -90,9 +90,7 @@ func (m *manifest) verifyOuterIntegrity() error {
 
 	sigString, err := m.signatureString()
 	if err != nil {
-		return fmt.Errorf(
-			"failed to generate signature string for verification: %w", err,
-		)
+		return fmt.Errorf("build signature string: %w", err)
 	}
 
 	// Loading a manifest takes no context; gpgTimeout still bounds gpg.
@@ -103,7 +101,7 @@ func (m *manifest) verifyOuterIntegrity() error {
 		m.pbOuter.GetSigningPubKey(),
 	)
 	if err != nil {
-		return fmt.Errorf("signature verification failed: %w", err)
+		return err
 	}
 
 	if !strings.EqualFold(string(m.pbOuter.GetSigner()), signingKey) {
@@ -134,7 +132,7 @@ func (m *manifest) decompressInner() ([]byte, error) {
 		zstd.WithDecodeBuffersBelow(0),
 		zstd.WithDecoderMaxWindow(zstdWindowSize))
 	if err != nil {
-		return nil, fmt.Errorf("deserialize: zstd reader: %w", err)
+		return nil, fmt.Errorf("create decompressor: %w", err)
 	}
 	defer zr.Close()
 
@@ -149,7 +147,7 @@ func (m *manifest) decompressInner() ([]byte, error) {
 
 	dat, err := io.ReadAll(limitedReader)
 	if err != nil {
-		return nil, fmt.Errorf("deserialize: decompress: %w", err)
+		return nil, fmt.Errorf("decompress inner message: %w", err)
 	}
 
 	if int64(len(dat)) >= MaxDecompressedSize {
@@ -265,7 +263,7 @@ func (m *manifest) deserializeInner() error {
 
 	err = checkDecodedSize(dat)
 	if err != nil {
-		return fmt.Errorf("deserialize: unmarshal inner: %w", err)
+		return fmt.Errorf("unmarshal inner message: %w", err)
 	}
 
 	// Deserialize inner message
@@ -274,13 +272,17 @@ func (m *manifest) deserializeInner() error {
 	// Unknown fields would cost memory; mfer never writes a loaded manifest out.
 	err = proto.UnmarshalOptions{DiscardUnknown: true}.Unmarshal(dat, m.pbInner)
 	if err != nil {
-		return fmt.Errorf("deserialize: unmarshal inner: %w", err)
+		return fmt.Errorf("unmarshal inner message: %w", err)
+	}
+
+	if m.pbInner.GetVersion() != MFFile_VERSION_ONE {
+		return errUnknownVersion
 	}
 
 	// Validate inner UUID
 	err = validateUUID(m.pbInner.GetUuid())
 	if err != nil {
-		return fmt.Errorf("inner UUID invalid: %w", err)
+		return fmt.Errorf("inner message: %w", err)
 	}
 
 	// Verify UUIDs match
@@ -293,12 +295,21 @@ func (m *manifest) deserializeInner() error {
 	// extract path tomorrow — acts on a traversal or absolute path from an
 	// untrusted .mf. Reject loudly on the first offender rather than
 	// dropping entries, which would let a hostile manifest hide files from a
-	// check.
+	// check. A path listed twice is refused too: check would check the one
+	// file against both entries.
+	seen := make(map[string]bool, len(m.pbInner.GetFiles()))
+
 	for _, f := range m.pbInner.GetFiles() {
 		err = ValidatePath(f.GetPath())
 		if err != nil {
 			return fmt.Errorf("%w: %w", errInvalidManifestPath, err)
 		}
+
+		if seen[f.GetPath()] {
+			return fmt.Errorf("%w %q", errDuplicatePath, f.GetPath())
+		}
+
+		seen[f.GetPath()] = true
 	}
 
 	log.Infof("loaded manifest with %d files", len(m.pbInner.GetFiles()))
@@ -318,13 +329,14 @@ func validateMagic(dat []byte) bool {
 	return bytes.Equal(got, expected)
 }
 
-// NewManifestFromReader reads a manifest from an io.Reader.
+// NewManifestFromReader reads a manifest from an io.Reader. It refuses a
+// manifest larger than MaxManifestSize, reading at most one byte past it.
 //
 //nolint:revive // unexported-return: exporting manifest is owner question 13
 func NewManifestFromReader(input io.Reader) (*manifest, error) {
 	m := &manifest{}
 
-	dat, err := io.ReadAll(input)
+	dat, err := readAtMost(input, MaxManifestSize)
 	if err != nil {
 		return nil, err
 	}
@@ -356,6 +368,21 @@ func NewManifestFromReader(input io.Reader) (*manifest, error) {
 	return m, nil
 }
 
+// readAtMost reads all of input, or refuses it with errManifestTooLarge
+// once it passes maxSize bytes, after reading one byte past maxSize.
+func readAtMost(input io.Reader, maxSize int64) ([]byte, error) {
+	dat, err := io.ReadAll(io.LimitReader(input, maxSize+1))
+	if err != nil {
+		return nil, err
+	}
+
+	if int64(len(dat)) > maxSize {
+		return nil, fmt.Errorf("%w of %d bytes", errManifestTooLarge, maxSize)
+	}
+
+	return dat, nil
+}
+
 // ManifestFromFileOptions configures NewManifestFromFile.
 type ManifestFromFileOptions struct {
 	// Path is the manifest file to read (required).

mfer/deserialize_path_test.go

@@ -7,7 +7,6 @@ import (
 	"crypto/sha256"
 	"fmt"
 	"strconv"
-	"strings"
 	"testing"
 	"time"
 	"uuid"
@@ -118,12 +117,61 @@ func TestDeserializeRejectsInvalidEntryPaths(t *testing.T) {
 	}
 }
 
+// A manifest that lists a path twice is refused as it is loaded, naming the
+// path. Paths are compared byte for byte: two that differ only in letter case
+// load, and fetch refuses those itself. Each entry has a hash, as entries mfer
+// writes do; entries of a path alone would take too much memory to decode.
+func TestDeserializeRefusesPathListedTwice(t *testing.T) {
+	t.Parallel()
+
+	hash, err := multihash.Encode(make([]byte, sha256.Size), multihash.SHA2_256)
+	require.NoError(t, err)
+
+	tests := []struct {
+		name    string
+		paths   []string
+		refused bool
+	}{
+		{"same path twice", []string{"dir/a.txt", "other.txt", "dir/a.txt"}, true},
+		{"paths differing in letter case", []string{"dir/b.txt", "dir/B.txt"}, false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			id := uuid.NewV4()
+			inner := &MFFile{Version: MFFile_VERSION_ONE, Uuid: id[:]}
+
+			for _, p := range tt.paths {
+				inner.Files = append(inner.Files, &MFFilePath{
+					Path:   p,
+					Hashes: []*MFFileChecksum{{MultiHash: hash}},
+				})
+			}
+
+			innerData, err := proto.Marshal(inner)
+			require.NoError(t, err)
+
+			m, err := NewManifestFromReader(bytes.NewReader(wrapInner(t, id, innerData)))
+			if tt.refused {
+				require.ErrorIs(t, err, errDuplicatePath)
+				require.EqualError(t, err, `duplicate path "dir/a.txt"`)
+			} else {
+				require.NoError(t, err)
+				assert.Len(t, m.Files(), len(tt.paths))
+			}
+		})
+	}
+}
+
 // 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
 // 53 bytes, about 8.2 times: refused, and leaving any one of the five
 // uncounted, even the MIME type, brings it under 8. A 39-character path makes
-// it 55 bytes, about 7.9 times: loaded.
+// it 55 bytes, about 7.9 times: loaded. Each entry's path is its number,
+// padded with zeros to that length, since a manifest lists a path only once.
 func TestDeserializeRefusesEntriesThatDecodeTooLarge(t *testing.T) {
 	t.Parallel()
 
@@ -139,22 +187,24 @@ func TestDeserializeRefusesEntriesThatDecodeTooLarge(t *testing.T) {
 		t.Run(strconv.Itoa(tt.pathLen), func(t *testing.T) {
 			t.Parallel()
 
-			entry := protowire.AppendTag(nil, 1, protowire.BytesType) // MFFilePath.path
-			entry = protowire.AppendString(entry, strings.Repeat("a", tt.pathLen))
-			entry = protowire.AppendTag(entry, 3, protowire.BytesType) // MFFilePath.hashes
-			entry = protowire.AppendBytes(entry, nil)
-			entry = protowire.AppendTag(entry, 301, protowire.BytesType) // MFFilePath.mimeType
-			entry = protowire.AppendBytes(entry, nil)
-			entry = protowire.AppendTag(entry, 302, protowire.BytesType) // MFFilePath.mtime
-			entry = protowire.AppendBytes(entry, nil)
-			entry = protowire.AppendTag(entry, 303, protowire.BytesType) // MFFilePath.ctime
-			entry = protowire.AppendBytes(entry, nil)
-
 			id := uuid.NewV4()
-			inner := protowire.AppendTag(nil, 102, protowire.BytesType) // MFFile.uuid
+			inner := protowire.AppendTag(nil, 100, protowire.VarintType) // MFFile.version
+			inner = protowire.AppendVarint(inner, uint64(MFFile_VERSION_ONE))
+			inner = protowire.AppendTag(inner, 102, protowire.BytesType) // MFFile.uuid
 			inner = protowire.AppendBytes(inner, id[:])
 
-			for range 1000 {
+			for i := range 1000 {
+				entry := protowire.AppendTag(nil, 1, protowire.BytesType) // MFFilePath.path
+				entry = protowire.AppendString(entry, fmt.Sprintf("%0*d", tt.pathLen, i))
+				entry = protowire.AppendTag(entry, 3, protowire.BytesType) // MFFilePath.hashes
+				entry = protowire.AppendBytes(entry, nil)
+				entry = protowire.AppendTag(entry, 301, protowire.BytesType) // MFFilePath.mimeType
+				entry = protowire.AppendBytes(entry, nil)
+				entry = protowire.AppendTag(entry, 302, protowire.BytesType) // MFFilePath.mtime
+				entry = protowire.AppendBytes(entry, nil)
+				entry = protowire.AppendTag(entry, 303, protowire.BytesType) // MFFilePath.ctime
+				entry = protowire.AppendBytes(entry, nil)
+
 				inner = protowire.AppendTag(inner, 101, protowire.BytesType) // MFFile.files
 				inner = protowire.AppendBytes(inner, entry)
 			}
@@ -182,7 +232,9 @@ func TestDeserializeDropsUnknownFields(t *testing.T) {
 	entry = append(entry, unknown...)
 
 	id := uuid.NewV4()
-	inner := protowire.AppendTag(nil, 101, protowire.BytesType) // MFFile.files
+	inner := protowire.AppendTag(nil, 100, protowire.VarintType) // MFFile.version
+	inner = protowire.AppendVarint(inner, uint64(MFFile_VERSION_ONE))
+	inner = protowire.AppendTag(inner, 101, protowire.BytesType) // MFFile.files
 	inner = protowire.AppendBytes(inner, entry)
 	inner = protowire.AppendTag(inner, 102, protowire.BytesType) // MFFile.uuid
 	inner = protowire.AppendBytes(inner, id[:])

mfer/deserialize_test.go

@@ -0,0 +1,54 @@
+//nolint:testpackage // white-box tests exercise unexported internals
+package mfer
+
+import (
+	"bytes"
+	"testing"
+	"uuid"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"google.golang.org/protobuf/proto"
+)
+
+// An inner message whose version is not VERSION_ONE, whether version 0 or a
+// later one, is refused with the same error as an outer message's.
+func TestDeserializeRefusesUnknownInnerVersion(t *testing.T) {
+	t.Parallel()
+
+	for _, version := range []MFFile_Version{MFFile_VERSION_NONE, MFFile_VERSION_ONE + 1} {
+		t.Run(version.String(), func(t *testing.T) {
+			t.Parallel()
+
+			id := uuid.NewV4()
+			inner, err := proto.Marshal(&MFFile{Version: version, Uuid: id[:]})
+			require.NoError(t, err)
+
+			_, err = NewManifestFromReader(bytes.NewReader(wrapInner(t, id, inner)))
+			require.ErrorIs(t, err, errUnknownVersion)
+		})
+	}
+}
+
+// TestReadAtMost gives readAtMost exactly its maximum, which it must
+// return whole, and twice its maximum, which it must refuse after reading
+// one byte past the maximum, and no more. NewManifestFromReader reads
+// through it with MaxManifestSize; the test uses 64 KiB, since reading
+// MaxManifestSize under the race detector takes gigabytes of memory.
+func TestReadAtMost(t *testing.T) {
+	t.Parallel()
+
+	const maxSize = 64 << 10
+
+	dat, err := readAtMost(bytes.NewReader(make([]byte, maxSize)), maxSize)
+	require.NoError(t, err)
+	assert.Len(t, dat, maxSize)
+
+	input := bytes.NewReader(make([]byte, 2*maxSize))
+
+	_, err = readAtMost(input, maxSize)
+	require.ErrorIs(t, err, errManifestTooLarge)
+	require.EqualError(t, err,
+		"file exceeds maximum allowed size of 65536 bytes")
+	assert.Equal(t, maxSize-1, input.Len(), "bytes left unread")
+}

mfer/errmsg_test.go

@@ -74,14 +74,18 @@ func TestValidatePathMessagesVerbatim(t *testing.T) {
 	}
 }
 
-// TestSerializeInternalErrorMessagesVerbatim pins the two distinct
-// "internal error" messages, which differ between generate and
-// generateOuter and have always done so.
-func TestSerializeInternalErrorMessagesVerbatim(t *testing.T) {
+// TestSerializeInnerNotSetMessagesVerbatim pins the messages generate and
+// generateOuter return when the inner message is missing.
+func TestSerializeInnerNotSetMessagesVerbatim(t *testing.T) {
 	t.Parallel()
 
 	m := &manifest{}
-	require.EqualError(t, m.generate(context.Background()),
-		"internal error: pbInner not set")
-	require.EqualError(t, m.generateOuter(context.Background()), "internal error")
+
+	err := m.generate(context.Background())
+	require.ErrorIs(t, err, errInnerNotSet)
+	require.EqualError(t, err, "inner message not set")
+
+	err = m.generateOuter(context.Background())
+	require.ErrorIs(t, err, errInternal)
+	require.EqualError(t, err, "inner message not set")
 }

mfer/gpg.go

@@ -41,28 +41,26 @@ const (
 	// fields in a gpg fingerprint record (the fingerprint is field 10).
 	gpgFingerprintMinFields = 10
 
-	// gpgPrimaryKeyRecord starts the record of each primary public key in
-	// gpg --with-colons output.
-	gpgPrimaryKeyRecord = "pub:"
-
-	// gpgValidSigStatus starts the status line gpg --verify writes for a
-	// good signature. Its last field is the fingerprint of the primary key
-	// that made the signature.
-	gpgValidSigStatus = "[GNUPG:] VALIDSIG "
+	// gpgStatusPrefix starts each status line gpg writes to the file
+	// descriptor named by --status-fd.
+	gpgStatusPrefix = "[GNUPG:]"
 
 	// gpg option names used from more than one call site.
-	gpgOptArmor   = "--armor"
-	gpgOptHomedir = "--homedir"
-	gpgOptVerify  = "--verify"
+	gpgOptArmor    = "--armor"
+	gpgOptHomedir  = "--homedir"
+	gpgOptStatusFD = "--status-fd"
+	gpgOptVerify   = "--verify"
 )
 
 var (
-	errGPGKeyNotFound      = errors.New("gpg key not found")
+	errGPGKeyNotFound      = errors.New("GPG key not found")
 	errFingerprintNotFound = errors.New("fingerprint not found for key")
 	errSigningKeyCount     = errors.New(
 		"embedded public key block must hold exactly one key")
 	errNotOneGoodSignature = errors.New(
 		"gpg did not report exactly one good signature")
+	errSigningKeyNotReported = errors.New(
+		"gpg did not report the key that made the signature")
 )
 
 // GPGKeyID represents a GPG key identifier (fingerprint or key ID).
@@ -90,8 +88,9 @@ func gpgArgs(opts []string, positional ...string) []string {
 }
 
 // runGPG runs the gpg binary in batch mode with the given arguments and
-// optional stdin, returning captured stdout and stderr. gpg is killed when
-// ctx ends or gpgTimeout passes, whichever comes first.
+// optional stdin, returning captured stdout and stderr. If gpg fails, the
+// error ends with what gpg wrote to stderr. gpg is killed when ctx ends or
+// gpgTimeout passes, whichever comes first.
 func runGPG(
 	ctx context.Context, stdin io.Reader, args ...string,
 ) (*bytes.Buffer, *bytes.Buffer, error) {
@@ -127,13 +126,28 @@ func runGPG(
 		// "signal: killed"; return the reason instead.
 		err = ctx.Err()
 		if errors.Is(err, context.DeadlineExceeded) {
-			err = fmt.Errorf("gpg timed out: %w", err)
+			err = fmt.Errorf("timed out: %w", err)
 		}
 	}
 
+	if err != nil {
+		err = withStderr(err, &stderr)
+	}
+
 	return &stdout, &stderr, err
 }
 
+// withStderr returns err followed by what gpg wrote to stderr, or err alone
+// when gpg wrote nothing.
+func withStderr(err error, stderr *bytes.Buffer) error {
+	messages := strings.TrimSpace(stderr.String())
+	if messages == "" {
+		return err
+	}
+
+	return fmt.Errorf("%w: %s", err, messages)
+}
+
 // parseFingerprint extracts the first fingerprint from gpg --with-colons
 // output, or returns ok=false if none is present.
 func parseFingerprint(colonOutput string) (string, bool) {
@@ -148,63 +162,77 @@ func parseFingerprint(colonOutput string) (string, bool) {
 	return "", false
 }
 
-// countPrimaryKeys returns the number of primary keys in gpg --with-colons
-// key listing output.
-func countPrimaryKeys(colonOutput string) int {
-	count := 0
-
-	for line := range strings.SplitSeq(colonOutput, "\n") {
-		if strings.HasPrefix(line, gpgPrimaryKeyRecord) {
-			count++
-		}
-	}
-
-	return count
-}
-
-// parseSigningKey returns the fingerprint of the primary key that made a
-// signature, the last field of the VALIDSIG line in gpg --verify status
-// output, or ok=false unless there is exactly one such line.
-func parseSigningKey(statusOutput string) (string, bool) {
-	var fingerprints []string
+// parseStatusLine returns the arguments of the status line for keyword in
+// gpg --status-fd output, or ok=false unless there is exactly one such line
+// and it has arguments.
+func parseStatusLine(statusOutput, keyword string) ([]string, bool) {
+	var found [][]string
 
 	for line := range strings.SplitSeq(statusOutput, "\n") {
-		if strings.HasPrefix(line, gpgValidSigStatus) {
-			fields := strings.Fields(line)
-			fingerprints = append(fingerprints, fields[len(fields)-1])
+		fields := strings.Fields(line)
+		if len(fields) > 2 && fields[0] == gpgStatusPrefix && fields[1] == keyword {
+			found = append(found, fields[2:])
 		}
 	}
 
-	if len(fingerprints) != 1 {
-		return "", false
+	if len(found) != 1 {
+		return nil, false
 	}
 
-	return fingerprints[0], true
+	return found[0], true
 }
 
-// gpgSign creates a detached signature of the data using the specified key.
-// Returns the armored detached signature.
-func gpgSign(ctx context.Context, data []byte, keyID GPGKeyID) ([]byte, error) {
+// gpgSign creates an armored detached signature of data with the key gpg
+// picks for keyID, and returns it with the fingerprint of the key that made
+// it, which is a subkey's when gpg signed with a subkey.
+func gpgSign(
+	ctx context.Context, data []byte, keyID GPGKeyID,
+) ([]byte, string, error) {
+	tmpDir, err := os.MkdirTemp("", "mfer-gpg-sign-*")
+	if err != nil {
+		return nil, "", err
+	}
+
+	defer func() { _ = os.RemoveAll(tmpDir) }()
+
+	sigFile := filepath.Join(tmpDir, "signature.asc")
+
+	// The signature goes to sigFile, so --status-fd 1 can send gpg's status
+	// lines to stdout; its messages go to stderr.
 	stdout, stderr, err := runGPG(ctx, bytes.NewReader(data),
 		"--detach-sign",
 		gpgOptArmor,
+		"--output", sigFile,
+		gpgOptStatusFD, "1",
 		"--local-user", string(keyID),
 	)
 	if err != nil {
-		return nil, fmt.Errorf("gpg sign failed: %w: %s", err, stderr.String())
+		return nil, "", fmt.Errorf("gpg sign: %w", err)
 	}
 
-	return stdout.Bytes(), nil
+	// The last argument of SIG_CREATED is the fingerprint of the key that
+	// made the signature.
+	created, ok := parseStatusLine(stdout.String(), "SIG_CREATED")
+	if !ok {
+		return nil, "", withStderr(errSigningKeyNotReported, stderr)
+	}
+
+	sig, err := os.ReadFile(sigFile) //nolint:gosec // G304: inside tmpDir, made above
+	if err != nil {
+		return nil, "", err
+	}
+
+	return sig, created[len(created)-1], nil
 }
 
 // gpgExportPublicKey exports the public key for the specified key ID.
 // Returns the armored public key.
 func gpgExportPublicKey(ctx context.Context, keyID GPGKeyID) ([]byte, error) {
-	stdout, stderr, err := runGPG(ctx, nil,
+	stdout, _, err := runGPG(ctx, nil,
 		gpgArgs([]string{"--export", gpgOptArmor}, string(keyID))...,
 	)
 	if err != nil {
-		return nil, fmt.Errorf("gpg export failed: %w: %s", err, stderr.String())
+		return nil, fmt.Errorf("gpg export: %w", err)
 	}
 
 	if stdout.Len() == 0 {
@@ -216,13 +244,11 @@ func gpgExportPublicKey(ctx context.Context, keyID GPGKeyID) ([]byte, error) {
 
 // gpgGetKeyFingerprint gets the full fingerprint for a key ID.
 func gpgGetKeyFingerprint(ctx context.Context, keyID GPGKeyID) ([]byte, error) {
-	stdout, stderr, err := runGPG(ctx, nil,
+	stdout, _, err := runGPG(ctx, nil,
 		gpgArgs([]string{"--with-colons", "--fingerprint"}, string(keyID))...,
 	)
 	if err != nil {
-		return nil, fmt.Errorf(
-			"gpg fingerprint lookup failed: %w: %s", err, stderr.String(),
-		)
+		return nil, fmt.Errorf("gpg fingerprint lookup: %w", err)
 	}
 
 	fpr, ok := parseFingerprint(stdout.String())
@@ -233,34 +259,29 @@ func gpgGetKeyFingerprint(ctx context.Context, keyID GPGKeyID) ([]byte, error) {
 	return []byte(fpr), nil
 }
 
-// gpgImportOneKey imports the public key in pubKeyFile into the keyring in
-// gpgHome, which must then hold exactly one primary key.
+// gpgImportOneKey imports the public key block in pubKeyFile into the
+// keyring in gpgHome. The block must hold exactly one primary key.
 func gpgImportOneKey(ctx context.Context, gpgHome, pubKeyFile string) error {
-	// Import the public key into the keyring
-	_, importStderr, err := runGPG(ctx, nil,
-		gpgArgs([]string{gpgOptHomedir, gpgHome, "--import"}, pubKeyFile)...,
+	// --status-fd 1 sends gpg's status lines to stdout, which importing
+	// otherwise leaves empty; its messages go to stderr.
+	importStdout, _, err := runGPG(ctx, nil,
+		gpgArgs([]string{gpgOptHomedir, gpgHome, gpgOptStatusFD, "1", "--import"},
+			pubKeyFile)...,
 	)
 	if err != nil {
-		return fmt.Errorf(
-			"failed to import public key: %w: %s", err, importStderr.String(),
-		)
+		return fmt.Errorf("gpg import: %w", err)
 	}
 
-	// List keys to count them
-	listStdout, listStderr, err := runGPG(ctx, nil,
-		gpgOptHomedir, gpgHome,
-		"--with-colons",
-		"--list-keys",
-	)
-	if err != nil {
-		return fmt.Errorf(
-			"failed to list keys: %w: %s", err, listStderr.String(),
-		)
+	// The first argument of IMPORT_RES counts the primary keys gpg read
+	// from the block, those it then skipped (one with no user ID, for
+	// example) included.
+	result, ok := parseStatusLine(importStdout.String(), "IMPORT_RES")
+	if !ok {
+		return fmt.Errorf("%w, gpg reported no count", errSigningKeyCount)
 	}
 
-	keys := countPrimaryKeys(listStdout.String())
-	if keys != 1 {
-		return fmt.Errorf("%w, found %d", errSigningKeyCount, keys)
+	if result[0] != "1" {
+		return fmt.Errorf("%w, found %s", errSigningKeyCount, result[0])
 	}
 
 	return nil
@@ -275,7 +296,7 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) (string, err
 	// Create temporary directory for GPG operations
 	tmpDir, err := os.MkdirTemp("", "mfer-gpg-verify-*")
 	if err != nil {
-		return "", fmt.Errorf("failed to create temp dir: %w", err)
+		return "", err
 	}
 
 	defer func() { _ = os.RemoveAll(tmpDir) }()
@@ -283,7 +304,7 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) (string, err
 	// Set restrictive permissions
 	err = os.Chmod(tmpDir, privateDirPerms)
 	if err != nil {
-		return "", fmt.Errorf("failed to set temp dir permissions: %w", err)
+		return "", err
 	}
 
 	// Write public key to temp file
@@ -291,7 +312,7 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) (string, err
 
 	err = os.WriteFile(pubKeyFile, pubKey, privateFilePerms)
 	if err != nil {
-		return "", fmt.Errorf("failed to write public key: %w", err)
+		return "", err
 	}
 
 	// Write signature to temp file
@@ -299,7 +320,7 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) (string, err
 
 	err = os.WriteFile(sigFile, signature, privateFilePerms)
 	if err != nil {
-		return "", fmt.Errorf("failed to write signature: %w", err)
+		return "", err
 	}
 
 	// Write data to temp file
@@ -307,7 +328,7 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) (string, err
 
 	err = os.WriteFile(dataFile, data, privateFilePerms)
 	if err != nil {
-		return "", fmt.Errorf("failed to write data: %w", err)
+		return "", err
 	}
 
 	err = gpgImportOneKey(ctx, tmpDir, pubKeyFile)
@@ -317,20 +338,21 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) (string, err
 
 	// --status-fd 1 sends gpg's status lines to stdout, which verifying a
 	// detached signature otherwise leaves empty; its messages go to stderr.
-	verifyStdout, verifyStderr, err := runGPG(ctx, nil,
-		gpgArgs([]string{gpgOptHomedir, tmpDir, "--status-fd", "1", gpgOptVerify},
+	verifyStdout, _, err := runGPG(ctx, nil,
+		gpgArgs([]string{gpgOptHomedir, tmpDir, gpgOptStatusFD, "1", gpgOptVerify},
 			sigFile, dataFile)...,
 	)
 	if err != nil {
-		return "", fmt.Errorf(
-			"signature verification failed: %w: %s", err, verifyStderr.String(),
-		)
+		return "", fmt.Errorf("gpg verify: %w", err)
 	}
 
-	fingerprint, ok := parseSigningKey(verifyStdout.String())
+	// gpg writes a VALIDSIG line for each good signature. Its first
+	// argument is the fingerprint of the key that made the signature,
+	// which may be a subkey; its last is that of the primary key.
+	valid, ok := parseStatusLine(verifyStdout.String(), "VALIDSIG")
 	if !ok {
 		return "", errNotOneGoodSignature
 	}
 
-	return fingerprint, nil
+	return valid[len(valid)-1], nil
 }

mfer/gpg_test.go

@@ -37,7 +37,7 @@ func testGPGEnv(t *testing.T) (GPGKeyID, string) {
 	// Create temporary GPG home directory (0700 by default)
 	gpgHome := t.TempDir()
 
-	genTestKey(t, gpgHome)
+	genTestKey(t, gpgHome, testKeyParams)
 
 	ctx, cancel := context.WithTimeout(context.Background(), gpgTimeout)
 	defer cancel()
@@ -73,21 +73,20 @@ func testGPGEnv(t *testing.T) (GPGKeyID, string) {
 	return GPGKeyID(keyID), gpgHome
 }
 
+// testKeyParams are the gpg key generation parameters of an RSA key that
+// signs and does not expire.
+const testKeyParams = "Key-Type: RSA\nKey-Length: 2048\nExpire-Date: 0\n"
+
 // genTestKey generates a key with no passphrase for
-// "MFER Test Key <test@mfer.test>" in gpgHome, which may already hold one.
-func genTestKey(t *testing.T, gpgHome string) {
+// "MFER Test Key <test@mfer.test>" from the gpg key generation parameters
+// keyParams in gpgHome, which may already hold one.
+func genTestKey(t *testing.T, gpgHome, keyParams string) {
 	t.Helper()
 
-	keyParams := `%no-protection
-Key-Type: RSA
-Key-Length: 2048
-Name-Real: MFER Test Key
-Name-Email: test@mfer.test
-Expire-Date: 0
-%commit
-`
+	params := "%no-protection\n" + keyParams +
+		"Name-Real: MFER Test Key\nName-Email: test@mfer.test\n%commit\n"
 	paramsFile := filepath.Join(gpgHome, "key-params")
-	require.NoError(t, os.WriteFile(paramsFile, []byte(keyParams), 0o600))
+	require.NoError(t, os.WriteFile(paramsFile, []byte(params), 0o600))
 
 	ctx, cancel := context.WithTimeout(context.Background(), gpgTimeout)
 	defer cancel()
@@ -144,8 +143,9 @@ func TestGPGSign(t *testing.T) {
 	t.Setenv("GNUPGHOME", gpgHome)
 
 	data := []byte("test data to sign")
-	sig, err := gpgSign(context.Background(), data, keyID)
+	sig, signingKey, err := gpgSign(context.Background(), data, keyID)
 	require.NoError(t, err)
+	assert.Equal(t, string(keyID), signingKey)
 	assert.NotEmpty(t, sig)
 	assert.Contains(t, string(sig), "-----BEGIN PGP SIGNATURE-----")
 	assert.Contains(t, string(sig), "-----END PGP SIGNATURE-----")
@@ -210,15 +210,18 @@ func TestGPGOptionLikeKeyIDIsNotAnOption(t *testing.T) {
 	assert.NotContains(t, string(fpr), "gpg (GnuPG)")
 }
 
+// TestGPGSignInvalidKey signs with a key that has no secret key in the
+// keyring. The error must hold gpg's messages and none of its status lines.
 func TestGPGSignInvalidKey(t *testing.T) {
 	// Set up test environment (we need GNUPGHOME set)
 	_, gpgHome := testGPGEnv(t)
 	t.Setenv("GNUPGHOME", gpgHome)
 
 	data := []byte("test data")
-	_, err := gpgSign(context.Background(), data,
+	_, _, err := gpgSign(context.Background(), data,
 		GPGKeyID("NONEXISTENT_KEY_ID_12345"))
-	assert.Error(t, err)
+	require.Error(t, err)
+	assert.NotContains(t, err.Error(), gpgStatusPrefix)
 }
 
 func TestBuilderWithSigning(t *testing.T) {
@@ -306,7 +309,7 @@ func TestGPGVerify(t *testing.T) {
 	t.Setenv("GNUPGHOME", gpgHome)
 
 	data := []byte("test data to sign and verify")
-	sig, err := gpgSign(context.Background(), data, keyID)
+	sig, _, err := gpgSign(context.Background(), data, keyID)
 	require.NoError(t, err)
 
 	pubKey, err := gpgExportPublicKey(context.Background(), keyID)
@@ -323,7 +326,7 @@ func TestGPGVerifyInvalidSignature(t *testing.T) {
 	t.Setenv("GNUPGHOME", gpgHome)
 
 	data := []byte("test data to sign")
-	sig, err := gpgSign(context.Background(), data, keyID)
+	sig, _, err := gpgSign(context.Background(), data, keyID)
 	require.NoError(t, err)
 
 	pubKey, err := gpgExportPublicKey(context.Background(), keyID)
@@ -340,7 +343,7 @@ func TestGPGVerifyBadPublicKey(t *testing.T) {
 	t.Setenv("GNUPGHOME", gpgHome)
 
 	data := []byte("test data")
-	sig, err := gpgSign(context.Background(), data, keyID)
+	sig, _, err := gpgSign(context.Background(), data, keyID)
 	require.NoError(t, err)
 
 	// Try to verify with invalid public key - should fail
@@ -439,6 +442,70 @@ func TestManifestRefusesSecondEmbeddedKey(t *testing.T) {
 	require.ErrorIs(t, err, errSigningKeyCount)
 }
 
+// TestManifestRefusesSecondEmbeddedKeyWithoutUserID loads a manifest whose
+// embedded public key block holds, before the key that signed it, another
+// key with its user ID removed, which gpg skips on import. Loading must
+// refuse it: the block holds two keys.
+func TestManifestRefusesSecondEmbeddedKeyWithoutUserID(t *testing.T) {
+	otherKey, otherHome := testGPGEnv(t)
+	t.Setenv("GNUPGHOME", otherHome)
+
+	// Keeping only the user IDs that match "nobody" exports none.
+	otherPubKey, _, err := runGPG(context.Background(), nil,
+		gpgArgs([]string{
+			"--export", gpgOptArmor, "--export-filter", "keep-uid=uid = nobody",
+		}, string(otherKey))...)
+	require.NoError(t, err)
+
+	keyID, gpgHome := testGPGEnv(t)
+	t.Setenv("GNUPGHOME", gpgHome)
+
+	manifest := rewriteOuter(t, signedTestManifest(t, keyID),
+		func(outer *MFFileOuter) {
+			outer.SigningPubKey = slices.Concat(
+				otherPubKey.Bytes(), outer.GetSigningPubKey())
+		})
+
+	_, err = NewManifestFromReader(bytes.NewReader(manifest))
+	require.ErrorIs(t, err, errSigningKeyCount)
+}
+
+// TestManifestRefusesTwoSignatures loads a manifest whose signature field
+// holds its good signature twice. Loading must refuse it.
+func TestManifestRefusesTwoSignatures(t *testing.T) {
+	keyID, gpgHome := testGPGEnv(t)
+	t.Setenv("GNUPGHOME", gpgHome)
+
+	manifest := rewriteOuter(t, signedTestManifest(t, keyID),
+		func(outer *MFFileOuter) {
+			outer.Signature = slices.Concat(
+				outer.GetSignature(), outer.GetSignature())
+		})
+
+	_, err := NewManifestFromReader(bytes.NewReader(manifest))
+	require.ErrorIs(t, err, errNotOneGoodSignature)
+}
+
+// TestManifestSignedWithSubkey signs with a key whose primary key can only
+// certify, so gpg signs with its signing subkey. The manifest must load,
+// with the primary key's fingerprint as signer.
+func TestManifestSignedWithSubkey(t *testing.T) {
+	gpgHome := t.TempDir()
+	t.Setenv("GNUPGHOME", gpgHome)
+
+	genTestKey(t, gpgHome, "Key-Type: RSA\nKey-Length: 2048\nKey-Usage: cert\n"+
+		"Subkey-Type: RSA\nSubkey-Length: 2048\nSubkey-Usage: sign\n"+
+		"Expire-Date: 0\n")
+
+	primary, err := gpgGetKeyFingerprint(context.Background(), "test@mfer.test")
+	require.NoError(t, err)
+
+	m, err := NewManifestFromReader(bytes.NewReader(
+		signedTestManifest(t, GPGKeyID("test@mfer.test"))))
+	require.NoError(t, err)
+	assert.Equal(t, primary, m.pbOuter.GetSigner())
+}
+
 // TestManifestRefusesSignerOtherThanSigningKey loads a manifest whose
 // signer field names a key other than the one that made the signature.
 func TestManifestRefusesSignerOtherThanSigningKey(t *testing.T) {
@@ -459,7 +526,7 @@ func TestManifestRefusesSignerOtherThanSigningKey(t *testing.T) {
 // signed it, or loading refuses it.
 func TestBuilderSigningKeyIDMatchingTwoKeys(t *testing.T) {
 	_, gpgHome := testGPGEnv(t)
-	genTestKey(t, gpgHome)
+	genTestKey(t, gpgHome, testKeyParams)
 	t.Setenv("GNUPGHOME", gpgHome)
 
 	manifest := signedTestManifest(t, GPGKeyID("test@mfer.test"))
@@ -468,6 +535,28 @@ func TestBuilderSigningKeyIDMatchingTwoKeys(t *testing.T) {
 	require.NoError(t, err)
 }
 
+// TestBuilderSigningUserIDWithExpiredFirstKey signs with a user ID whose
+// first key in the keyring has expired. gpg signs with the other key for
+// that user ID, and the manifest must name and embed that key.
+func TestBuilderSigningUserIDWithExpiredFirstKey(t *testing.T) {
+	gpgHome := t.TempDir()
+	t.Setenv("GNUPGHOME", gpgHome)
+
+	// Made in 2020 and valid for one day.
+	genTestKey(t, gpgHome, "Key-Type: RSA\nKey-Length: 2048\n"+
+		"Creation-Date: 20200101T000000\nExpire-Date: 1d\n")
+
+	expired, err := gpgGetKeyFingerprint(context.Background(), "test@mfer.test")
+	require.NoError(t, err)
+
+	genTestKey(t, gpgHome, testKeyParams)
+
+	m, err := NewManifestFromReader(bytes.NewReader(
+		signedTestManifest(t, GPGKeyID("test@mfer.test"))))
+	require.NoError(t, err)
+	assert.NotEqual(t, expired, m.pbOuter.GetSigner())
+}
+
 func TestBuilderWithoutSigning(t *testing.T) {
 	t.Parallel()
 
@@ -514,16 +603,42 @@ func fakeGPGPath(t *testing.T, script string) string {
 
 // TestGPGTimeoutKillsGPG puts a fake gpg that never finishes first on
 // PATH and checks that a run past its deadline is killed and reported as
-// a timeout of the named operation, instead of hanging.
+// a timeout of the named operation, instead of hanging. The fake gpg writes
+// nothing to stderr, so the message ends with the timeout.
 func TestGPGTimeoutKillsGPG(t *testing.T) {
 	t.Setenv("PATH", fakeGPGPath(t, "#!/bin/sh\nexec sleep 10\n"))
 
 	ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
 	defer cancel()
 
-	_, err := gpgSign(ctx, []byte("data"), GPGKeyID("any"))
+	_, _, err := gpgSign(ctx, []byte("data"), GPGKeyID("any"))
 	require.ErrorIs(t, err, context.DeadlineExceeded)
-	assert.Contains(t, err.Error(), "gpg sign failed: gpg timed out")
+	assert.EqualError(t, err, "gpg sign: timed out: context deadline exceeded")
+}
+
+// TestGPGSignKeyNotReportedKeepsStderr puts a fake gpg first on PATH that
+// exits cleanly without reporting the key that signed, and checks that what
+// it wrote to stderr is in the message.
+func TestGPGSignKeyNotReportedKeepsStderr(t *testing.T) {
+	t.Setenv("PATH", fakeGPGPath(t,
+		"#!/bin/sh\necho 'gpg: note from the fake gpg' >&2\n"))
+
+	_, _, err := gpgSign(context.Background(), []byte("data"), GPGKeyID("any"))
+	require.ErrorIs(t, err, errSigningKeyNotReported)
+	assert.EqualError(t, err,
+		"gpg did not report the key that made the signature: "+
+			"gpg: note from the fake gpg")
+}
+
+// TestGPGFailureKeepsStderr puts a fake gpg first on PATH that writes to
+// stderr and exits non-zero, and checks that what it wrote ends the message.
+func TestGPGFailureKeepsStderr(t *testing.T) {
+	t.Setenv("PATH", fakeGPGPath(t,
+		"#!/bin/sh\necho 'gpg: signing failed: No secret key' >&2\nexit 2\n"))
+
+	_, _, err := gpgSign(context.Background(), []byte("data"), GPGKeyID("any"))
+	assert.EqualError(t, err,
+		"gpg sign: exit status 2: gpg: signing failed: No secret key")
 }
 
 // TestGPGCancelWhenChildHoldsOutput uses a fake gpg that runs sleep as a
@@ -546,7 +661,7 @@ func TestGPGCancelWhenChildHoldsOutput(t *testing.T) {
 	signErr := make(chan error, 1)
 
 	go func() {
-		_, err := gpgSign(ctx, []byte("data"), GPGKeyID("any"))
+		_, _, err := gpgSign(ctx, []byte("data"), GPGKeyID("any"))
 		signErr <- err
 	}()
 

mfer/manifest.go

@@ -10,7 +10,7 @@ import (
 )
 
 var (
-	errOuterNotSet  = errors.New("pbOuter not set")
+	errOuterNotSet  = errors.New("outer message not set")
 	errUUIDNotSet   = errors.New("UUID not set")
 	errSHA256NotSet = errors.New("SHA256 hash not set")
 )
@@ -65,7 +65,7 @@ func (m *manifest) signatureString() (string, error) {
 
 	mh, err := multihash.Encode(m.pbOuter.GetSha256(), multihash.SHA2_256)
 	if err != nil {
-		return "", fmt.Errorf("failed to encode multihash: %w", err)
+		return "", fmt.Errorf("encode multihash: %w", err)
 	}
 
 	uuidStr := hex.EncodeToString(m.pbOuter.GetUuid())

mfer/scanner.go

@@ -2,6 +2,7 @@ package mfer
 
 import (
 	"context"
+	"fmt"
 	"io"
 	"io/fs"
 	"os"
@@ -79,7 +80,8 @@ type FileEntry struct {
 type Scanner struct {
 	mu         sync.RWMutex
 	files      []*FileEntry
-	totalBytes FileSize // cached sum of all file sizes
+	paths      map[RelFilePath]AbsFilePath // the file at each path in files
+	totalBytes FileSize                    // cached sum of all file sizes
 	options    *ScannerOptions
 	fs         afero.Fs
 	excluded   []fs.FileInfo // the files named in ExcludePaths that exist
@@ -103,6 +105,7 @@ func NewScannerWithOptions(opts *ScannerOptions) *Scanner {
 
 	s := &Scanner{
 		files:   make([]*FileEntry, 0),
+		paths:   make(map[RelFilePath]AbsFilePath),
 		options: opts,
 		fs:      fs,
 	}
@@ -478,6 +481,18 @@ func (s *Scanner) enumerateFileWithInfo(
 	}
 
 	s.mu.Lock()
+
+	// Each path is relative to the input path it was found under, so files
+	// under two input paths can share one.
+	first, ok := s.paths[entry.Path]
+	if ok {
+		s.mu.Unlock()
+
+		return fmt.Errorf("%w %q: %s and %s",
+			errDuplicatePath, entry.Path, first, entry.AbsPath)
+	}
+
+	s.paths[entry.Path] = entry.AbsPath
 	s.files = append(s.files, entry)
 	s.totalBytes += entry.Size
 	filesFound := FileCount(len(s.files))

mfer/serialize.go

@@ -20,11 +20,9 @@ const MAGIC string = "ZNAVSRFG"
 var (
 	// errInnerNotSet is returned by generate when the inner manifest is
 	// missing.
-	errInnerNotSet = errors.New("internal error: pbInner not set")
+	errInnerNotSet = errors.New("inner message not set")
 	// errInternal is returned by generateOuter for the same condition.
-	// The two messages differ, and both are load-bearing for callers that
-	// match on text, so they are kept distinct.
-	errInternal = errors.New("internal error")
+	errInternal = errors.New("inner message not set")
 )
 
 // nanosecondsInt32 converts t's nanosecond component to int32.
@@ -65,14 +63,14 @@ func (m *manifest) generate(ctx context.Context) error {
 
 	dat, err := proto.MarshalOptions{Deterministic: true}.Marshal(m.pbOuter)
 	if err != nil {
-		return fmt.Errorf("serialize: marshal outer: %w", err)
+		return fmt.Errorf("marshal outer message: %w", err)
 	}
 
 	m.output = bytes.NewBufferString(MAGIC)
 
 	_, err = m.output.Write(dat)
 	if err != nil {
-		return fmt.Errorf("serialize: write output: %w", err)
+		return fmt.Errorf("write outer message: %w", err)
 	}
 
 	return nil
@@ -95,7 +93,7 @@ func (m *manifest) generateOuter(ctx context.Context) error {
 
 	innerData, err := proto.MarshalOptions{Deterministic: true}.Marshal(m.pbInner)
 	if err != nil {
-		return fmt.Errorf("serialize: marshal inner: %w", err)
+		return fmt.Errorf("marshal inner message: %w", err)
 	}
 
 	// Compress the inner data
@@ -103,12 +101,12 @@ func (m *manifest) generateOuter(ctx context.Context) error {
 
 	zw, err := zstd.NewWriter(idc, zstd.WithEncoderLevel(zstd.SpeedBestCompression))
 	if err != nil {
-		return fmt.Errorf("serialize: create compressor: %w", err)
+		return fmt.Errorf("create compressor: %w", err)
 	}
 
 	_, err = zw.Write(innerData)
 	if err != nil {
-		return fmt.Errorf("serialize: compress: %w", err)
+		return fmt.Errorf("compress inner message: %w", err)
 	}
 
 	_ = zw.Close()
@@ -120,7 +118,7 @@ func (m *manifest) generateOuter(ctx context.Context) error {
 
 	_, err = h.Write(compressedData)
 	if err != nil {
-		return fmt.Errorf("serialize: hash write: %w", err)
+		return fmt.Errorf("hash inner message: %w", err)
 	}
 
 	sha256Hash := h.Sum(nil)
@@ -143,32 +141,34 @@ func (m *manifest) generateOuter(ctx context.Context) error {
 }
 
 // signOuter signs the outer message with the configured GPG key and
-// embeds the signature, signer fingerprint, and public key. It signs with
-// and exports the key by its fingerprint, so that a key ID matching more
-// than one key cannot embed a key other than the one that signed.
+// embeds the signature, signer fingerprint, and public key. The signer
+// and public key are those of the key gpg reports it signed with, so that
+// a key ID matching more than one key cannot name or embed another key.
 func (m *manifest) signOuter(ctx context.Context) error {
 	sigString, err := m.signatureString()
 	if err != nil {
-		return fmt.Errorf("failed to generate signature string: %w", err)
+		return fmt.Errorf("build signature string: %w", err)
 	}
 
-	fingerprint, err := gpgGetKeyFingerprint(ctx, m.signingOptions.KeyID)
+	sig, signingKey, err := gpgSign(ctx, []byte(sigString), m.signingOptions.KeyID)
 	if err != nil {
-		return fmt.Errorf("failed to get key fingerprint: %w", err)
-	}
-
-	m.pbOuter.Signer = fingerprint
-
-	sig, err := gpgSign(ctx, []byte(sigString), GPGKeyID(fingerprint))
-	if err != nil {
-		return fmt.Errorf("failed to sign manifest: %w", err)
+		return err
 	}
 
 	m.pbOuter.Signature = sig
 
+	// Listing the signing key, a subkey's included, puts its primary key's
+	// fingerprint first.
+	fingerprint, err := gpgGetKeyFingerprint(ctx, GPGKeyID(signingKey))
+	if err != nil {
+		return err
+	}
+
+	m.pbOuter.Signer = fingerprint
+
 	pubKey, err := gpgExportPublicKey(ctx, GPGKeyID(fingerprint))
 	if err != nil {
-		return fmt.Errorf("failed to export public key: %w", err)
+		return err
 	}
 
 	m.pbOuter.SigningPubKey = pubKey