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)
+[in the format specification](docs/FORMAT.md), which refers to the protobuf
-which is included in the [project repository](https://git.eeqj.de/sneak/mfer).
+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

@@ -37,7 +37,7 @@ The outer message contains:
 | `uuid`            | 105    | bytes            | Random v4 UUID; must match the inner message UUID                        |
 | `innerMessage`    | 199    | bytes            | Zstd-compressed serialized `MFFile` message                              |
 | `signature`       | 201    | bytes (optional) | GPG signature (ASCII-armored or binary)                                  |
-| `signer`          | 202    | bytes (optional) | Full GPG key ID of the signer                                            |
+| `signer`          | 202    | bytes (optional) | Fingerprint of the signing key                                           |
 | `signingPubKey`   | 203    | bytes (optional) | Full GPG signing public key                                              |
 
 ### SHA-256 Hash
@@ -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`)
 
@@ -137,7 +143,16 @@ Where:
   compressed data)
 
 Components are separated by hyphens. The signature is produced by GPG over this
-canonical string and stored in the `signature` field of the outer message.
+canonical string and 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`.
+
+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`.
 
 ## Deterministic Serialization
 

internal/cli/check.go

@@ -108,28 +108,33 @@ 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
 }
 
 // verifyRequiredSigner enforces the --require-signature fingerprint
-// against the manifest's embedded signing key.
+// against the key that made the manifest's signature.
-func verifyRequiredSigner(
+func verifyRequiredSigner(chk *mfer.Checker, requiredSigner string) error {
-	ctx context.Context, chk *mfer.Checker, requiredSigner string,
-) error {
 	// Validate fingerprint format: must be exactly 40 hex characters
 	if len(requiredSigner) != fingerprintHexLen {
 		return fmt.Errorf("%w, got %d", errInvalidFingerprint, len(requiredSigner))
@@ -137,7 +142,7 @@ func verifyRequiredSigner(
 
 	_, 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() {
@@ -145,22 +150,17 @@ func verifyRequiredSigner(
 			errManifestNotSigned, requiredSigner)
 	}
 
-	// Extract fingerprint from the embedded public key (not from the
+	// Loading the manifest checked that the signer is the fingerprint of
-	// signer field). This validates the key is importable and gets its
+	// the key that made the signature.
-	// actual fingerprint.
+	signer := string(chk.Signer())
-	embeddedFP, err := chk.ExtractEmbeddedSigningKeyFP(ctx)
-	if err != nil {
-		return fmt.Errorf(
-			"failed to extract fingerprint from embedded signing key: %w", err)
-	}
 
 	// Compare fingerprints - must be exact match (case-insensitive)
-	if !strings.EqualFold(embeddedFP, requiredSigner) {
+	if !strings.EqualFold(signer, requiredSigner) {
 		return fmt.Errorf("embedded signing key fingerprint %s %w %s",
-			embeddedFP, errSignerMismatch, requiredSigner)
+			signer, errSignerMismatch, requiredSigner)
 	}
 
-	log.Infof("manifest signature verified (signer: %s)", embeddedFP)
+	log.Infof("manifest signature verified (signer: %s)", signer)
 
 	return nil
 }
@@ -232,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
@@ -275,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
@@ -296,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) }()
@@ -311,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)
@@ -324,13 +327,13 @@ 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
 	requiredSigner := cmd.String(flagRequireSignature)
 	if requiredSigner != "" {
-		err = verifyRequiredSigner(ctx, chk, requiredSigner)
+		err = verifyRequiredSigner(chk, requiredSigner)
 		if err != nil {
 			return err
 		}

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
@@ -60,6 +61,7 @@ func RunWithOptions(opts *RunOptions) int {
 		version:         opts.Version,
 		gitrev:          opts.Gitrev,
 		exitCode:        0,
+		maxManifestSize: mfer.MaxManifestSize,
 		Stdin:           opts.Stdin,
 		Stdout:          opts.Stdout,
 		Stderr:          opts.Stderr,

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
@@ -613,6 +652,33 @@ func runCheckAfterRewrite(t *testing.T, rewritten, msg string) {
 	assert.Equal(t, 1, exitCode, msg)
 }
 
+// TestCheckRequireSignatureRefusesOtherSigningKey runs check
+// --require-signature on a manifest signed by another key whose embedded
+// public key block also holds the required key. check must refuse it. It
+// needs gpg and is skipped without it, as the other signing tests are.
+//
+//nolint:paralleltest // signedManifest calls t.Setenv, which bars t.Parallel
+func TestCheckRequireSignatureRefusesOtherSigningKey(t *testing.T) {
+	content := []byte("signed file")
+	manifest, required := manifestSignedByAnotherKey(t,
+		map[string][]byte{testFileTxt: content})
+
+	fs := afero.NewMemMapFs()
+	require.NoError(t, fs.MkdirAll(testDir, 0o755))
+	require.NoError(t, afero.WriteFile(fs,
+		filepath.Join(testDir, testFileTxt), content, 0o644))
+	require.NoError(t, afero.WriteFile(fs, testManifest, manifest, 0o644))
+
+	opts := testOpts([]string{
+		testApp, cmdCheck, "-q", testFlagBase, testDir,
+		"--" + flagRequireSignature, required, testManifest,
+	}, fs)
+	assert.Equal(t, 1, runCLI(opts))
+	assert.Contains(t, testStderr(t, opts),
+		"load manifest: "+
+			"embedded public key block must hold exactly one key, found 2")
+}
+
 func TestCheckCommandWithCorruptedFile(t *testing.T) {
 	t.Parallel()
 
@@ -914,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

@@ -9,12 +9,14 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"slices"
 	"testing"
 
 	"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"
 )
 
@@ -86,8 +88,7 @@ func TestVerifyRequiredSignerMessages(t *testing.T) {
 	t.Run("invalid fingerprint length", func(t *testing.T) {
 		t.Parallel()
 
-		err := verifyRequiredSigner(context.Background(),
+		err := verifyRequiredSigner(unsignedChecker(t), "12345678")
-			unsignedChecker(t), "12345678")
 		require.ErrorIs(t, err, errInvalidFingerprint)
 		assert.EqualError(t, err,
 			"invalid fingerprint: must be exactly 40 hex characters, got 8")
@@ -96,8 +97,7 @@ func TestVerifyRequiredSignerMessages(t *testing.T) {
 	t.Run("manifest not signed", func(t *testing.T) {
 		t.Parallel()
 
-		err := verifyRequiredSigner(context.Background(),
+		err := verifyRequiredSigner(unsignedChecker(t), msgFpA)
-			unsignedChecker(t), msgFpA)
 		require.ErrorIs(t, err, errManifestNotSigned)
 		assert.EqualError(t, err,
 			"manifest is not signed, but signature from "+msgFpA+" is required")
@@ -105,23 +105,21 @@ func TestVerifyRequiredSignerMessages(t *testing.T) {
 }
 
 // TestSignerMismatchMessage drives verifyRequiredSigner against a real signed
-// manifest. The embedded fingerprint is whatever the generated key produced,
+// manifest. The signing key's fingerprint is whatever the generated key
-// so it is read back from the checker and substituted into the expected
+// produced, so it is read back from the checker and substituted into the
-// string; the required signer is a fixed value that cannot match it. Requires
+// expected string; the required signer is a fixed value that cannot match
-// gpg and is skipped where it is absent, as the other signing tests are.
+// it. Requires gpg and is skipped where it is absent, as the other signing
+// tests are.
 //
 //nolint:paralleltest // signedManifest calls t.Setenv, which bars t.Parallel
 func TestSignerMismatchMessage(t *testing.T) {
 	chk := signedChecker(t,
 		signedManifest(t, map[string][]byte{"f.txt": []byte("signed file")}))
 
-	embeddedFP, err := chk.ExtractEmbeddedSigningKeyFP(context.Background())
+	err := verifyRequiredSigner(chk, msgFpB)
-	require.NoError(t, err)
-
-	err = verifyRequiredSigner(context.Background(), chk, msgFpB)
 	require.ErrorIs(t, err, errSignerMismatch)
 	assert.EqualError(t, err,
-		"embedded signing key fingerprint "+embeddedFP+
+		"embedded signing key fingerprint "+string(chk.Signer())+
 			" does not match required "+msgFpB)
 }
 
@@ -191,6 +189,34 @@ func signedChecker(t *testing.T, manifest []byte) *mfer.Checker {
 	return chk
 }
 
+// manifestSignedByAnotherKey returns a manifest of files and the
+// fingerprint of a throwaway key, the required key, that did not sign it.
+// The manifest is signed by a second throwaway key; its embedded public key
+// block holds the required key followed by the second key, and its signer
+// field names the required key.
+func manifestSignedByAnotherKey(
+	t *testing.T, files map[string][]byte,
+) ([]byte, string) {
+	t.Helper()
+
+	required := new(mfer.MFFileOuter)
+	require.NoError(t, proto.Unmarshal(
+		signedManifest(t, files)[len(mfer.MAGIC):], required))
+
+	outer := new(mfer.MFFileOuter)
+	require.NoError(t, proto.Unmarshal(
+		signedManifest(t, files)[len(mfer.MAGIC):], outer))
+
+	outer.SigningPubKey = slices.Concat(
+		required.GetSigningPubKey(), outer.GetSigningPubKey())
+	outer.Signer = required.GetSigner()
+
+	data, err := proto.Marshal(outer)
+	require.NoError(t, err)
+
+	return append([]byte(mfer.MAGIC), data...), string(required.GetSigner())
+}
+
 func TestPathDoesNotExistMessage(t *testing.T) {
 	t.Parallel()
 
@@ -277,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) {
@@ -299,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) {
@@ -317,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,37 +426,45 @@ 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)
 	if requiredSigner != "" {
-		err = verifyFetchedSigner(ctx, manifestData, requiredSigner)
+		//nolint:contextcheck // mfer loads a manifest without a context
+		err = verifyFetchedSigner(manifestData, requiredSigner)
 		if err != nil {
 			return nil, nil, err
 		}
@@ -538,9 +546,7 @@ func checkNoNameClash(files []*mfer.MFFilePath) error {
 // exactly as check does. verifyRequiredSigner takes a Checker, which loads
 // its manifest from a file, so the manifest is handed to it as a file in
 // memory.
-func verifyFetchedSigner(
+func verifyFetchedSigner(manifestData []byte, requiredSigner string) error {
-	ctx context.Context, manifestData []byte, requiredSigner string,
-) error {
 	memFs := afero.NewMemMapFs()
 	manifestPath := "/" + defaultManifestName
 
@@ -549,17 +555,16 @@ func verifyFetchedSigner(
 		return err
 	}
 
-	//nolint:contextcheck // mfer loads a manifest without a context
 	chk, err := mfer.NewChecker(&mfer.CheckerOptions{
 		ManifestPath: manifestPath,
 		BasePath:     "/",
 		Fs:           memFs,
 	})
 	if err != nil {
-		return fmt.Errorf("failed to load manifest: %w", err)
+		return fmt.Errorf("load manifest: %w", err)
 	}
 
-	return verifyRequiredSigner(ctx, chk, requiredSigner)
+	return verifyRequiredSigner(chk, requiredSigner)
 }
 
 // saveManifest writes the fetched manifest into dest under the default
@@ -648,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 {
@@ -765,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() {
@@ -792,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
@@ -807,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
 		}
 	}
 
@@ -841,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
@@ -854,12 +859,7 @@ func moveIntoPlace(dest, tmpPath, localPath string) error {
 		return err
 	}
 
-	err = os.Rename(filepath.Join(dest, tmpPath), filepath.Join(dest, localPath))
+	return 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
 }
 
 // saveResponse writes resp's body to tmpPath, verifies it against entry,
@@ -894,7 +894,7 @@ func saveResponse(
 			_ = out.Close()
 			_ = os.Remove(filepath.Join(dest, tmpPath))
 
-			return fmt.Errorf("failed to set mode: %w", err)
+			return err
 		}
 	}
 
@@ -910,8 +910,10 @@ func saveResponse(
 		progress:  progress,
 	}
 
-	// Copy content while hashing and reporting progress
+	// Copy content while hashing and reporting progress. One byte past
-	written, copyErr := io.Copy(pw, resp.Body)
+	// 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())
 		})
 	}
@@ -1117,8 +1189,10 @@ func TestFetchIntoDest(t *testing.T) {
 // TestFetchRequireSignature runs fetch with --require-signature. A
 // manifest that is unsigned, or signed by another key, must stop fetch
 // with check's message before it downloads or writes anything; the
-// required key lets it through. The signed cases need gpg and are skipped
+// required key lets it through. A manifest signed by another key whose
-// without it, as the other signing tests are.
+// embedded public key block also holds the required key must stop fetch
+// too. The signed cases need gpg and are skipped without it, as the other
+// signing tests are.
 //
 //nolint:paralleltest // signedManifest calls t.Setenv, which bars t.Parallel
 func TestFetchRequireSignature(t *testing.T) {
@@ -1133,9 +1207,7 @@ func TestFetchRequireSignature(t *testing.T) {
 	t.Run("signed", func(t *testing.T) {
 		manifest := signedManifest(t, files)
 
-		signer, err := signedChecker(t, manifest).
+		signer := string(signedChecker(t, manifest).Signer())
-			ExtractEmbeddedSigningKeyFP(context.Background())
-		require.NoError(t, err)
 
 		assertFetchRefused(t, manifest, files,
 			"embedded signing key fingerprint "+signer+" does not match required "+msgFpB,
@@ -1153,6 +1225,15 @@ func TestFetchRequireSignature(t *testing.T) {
 		require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
 		assert.Equal(t, files[testFileTxt], filesUnder(t, dest)[testFileTxt])
 	})
+
+	t.Run("signed by another key embedded after the required one", func(t *testing.T) {
+		manifest, required := manifestSignedByAnotherKey(t, files)
+
+		assertFetchRefused(t, manifest, files,
+			"parse manifest: "+
+				"embedded public key block must hold exactly one key, found 2",
+			"--"+flagRequireSignature, required)
+	})
 }
 
 // TestFetchRefusesListedManifestName fetches manifests that list, at the

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(
+		return err
-			"freshen: failed to compute relative path for %s: %w", path, 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
+		// Use existing entry; the error names the entry
-		err := addExistingToBuilder(h.builder, e.existing)
+		return addExistingToBuilder(h.builder, e.existing)
-		if err != nil {
-			return fmt.Errorf("failed to add %s: %w", e.path, err)
-		}
-
-		return nil
 	}
 
 	// 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
+	// Add to builder with computed hash; a refused path is named in the error
-	err = addFileToBuilder(h.builder, e.path, e.size, e.mtime, e.mode, hash)
+	return 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
 }
 
 // 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
+		// Returned unwrapped: a read error already names the file.
-		// "failed to hash <path>: <err>" and adding a second layer here
-		// would change that message.
 		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(
+			return fmt.Errorf("enumerate current directory: %w", err)
-				"generate: failed to 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.
+// errHTTPStatus indicates an HTTP response with a non-OK status code. It is
-//
+// followed by the code, as in "unexpected HTTP status 404".
-// Its text is the literal "HTTP" prefix of the rendered "HTTP <code>"
+var errHTTPStatus = errors.New("unexpected HTTP status")
-// 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
+// errManifestTooLarge indicates a manifest download that passed
-// its message.
+// CLIApp.maxManifestSize.
-var errHTTPStatus = errors.New("HTTP")
+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()
+	return totalRead, b.addEntry(entry)
-	b.files = append(b.files, entry)
-	b.mu.Unlock()
-
-	return totalRead, nil
 }
 
 // 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
+// Returns an error if path is invalid or already added, size is negative,
-// multihash with a digest of at least 32 bytes, as long as SHA-256's.
+// 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()
+	return b.addEntry(entry)
-	b.files = append(b.files, entry)
-	b.mu.Unlock()
-
-	return nil
 }
 
 // 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/checker.go

@@ -15,7 +15,6 @@ import (
 )
 
 var (
-	errNoSigningPubKey   = errors.New("manifest has no signing public key")
 	errManifestPathEmpty = errors.New("manifest path cannot be empty")
 	errBasePathEmpty     = errors.New("base path cannot be empty")
 )
@@ -173,8 +172,14 @@ func (c *Checker) IsSigned() bool {
 	return len(c.signature) > 0
 }
 
-// Signer returns the signer fingerprint if the manifest is signed, nil otherwise.
+// Signer returns the fingerprint of the key that made the manifest's
+// signature, which loading the manifest checked, or nil if the manifest is
+// not signed.
 func (c *Checker) Signer() []byte {
+	if !c.IsSigned() {
+		return nil
+	}
+
 	return c.signer
 }
 
@@ -184,17 +189,6 @@ func (c *Checker) SigningPubKey() []byte {
 	return c.signingPubKey
 }
 
-// ExtractEmbeddedSigningKeyFP imports the manifest's embedded public key into a
-// temporary keyring and extracts its fingerprint. This validates the key and
-// returns its actual fingerprint from the key material itself.
-func (c *Checker) ExtractEmbeddedSigningKeyFP(ctx context.Context) (string, error) {
-	if len(c.signingPubKey) == 0 {
-		return "", errNoSigningPubKey
-	}
-
-	return gpgExtractPubKeyFingerprint(ctx, c.signingPubKey)
-}
-
 // Check verifies all files against the manifest.
 // Results are sent to the results channel as files are checked.
 // Progress updates are sent to the progress channel approximately once per second.

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

@@ -7,6 +7,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"strings"
 
 	"github.com/klauspost/compress/zstd"
 	"github.com/spf13/afero"
@@ -23,11 +24,13 @@ 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")
+	errInvalidManifestPath  = errors.New("invalid file entry")
-	errDecodedTooLarge      = errors.New(
+	errDecodedTooLarge      = errors.New("too much memory needed")
-		"manifest would take too much memory to decode")
+	errSignerNotSigningKey  = errors.New(
+		"signer is not the fingerprint of the key that made the signature")
 )
 
 // validateUUID checks that the byte slice is the 16 bytes of a binary UUID.
@@ -54,20 +57,22 @@ 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
 }
 
-// verifyOuterIntegrity checks the hash of the compressed payload and,
+// verifyOuterIntegrity checks the hash of the compressed payload and, if a
-// if a signature is present, verifies it against the embedded public key.
+// signature is present, verifies it against the embedded public key, which
+// must be one key, and checks that the signer field is that key's
+// fingerprint.
 func (m *manifest) verifyOuterIntegrity() error {
 	h := sha256.New()
 
 	_, 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)
@@ -85,20 +90,23 @@ func (m *manifest) verifyOuterIntegrity() error {
 
 	sigString, err := m.signatureString()
 	if err != nil {
-		return fmt.Errorf(
+		return fmt.Errorf("build signature string: %w", err)
-			"failed to generate signature string for verification: %w", err,
-		)
 	}
 
 	// Loading a manifest takes no context; gpgTimeout still bounds gpg.
-	err = gpgVerify(
+	signingKey, err := gpgVerify(
 		context.Background(),
 		[]byte(sigString),
 		m.pbOuter.GetSignature(),
 		m.pbOuter.GetSigningPubKey(),
 	)
 	if err != nil {
-		return fmt.Errorf("signature verification failed: %w", err)
+		return err
+	}
+
+	if !strings.EqualFold(string(m.pbOuter.GetSigner()), signingKey) {
+		return fmt.Errorf("%w: signer %q, signing key %s",
+			errSignerNotSigningKey, m.pbOuter.GetSigner(), signingKey)
 	}
 
 	log.Infof("signature verified successfully")
@@ -124,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()
 
@@ -139,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 {
@@ -255,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
@@ -264,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
@@ -283,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()))
@@ -308,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
 	}
@@ -346,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,8 +187,15 @@ func TestDeserializeRefusesEntriesThatDecodeTooLarge(t *testing.T) {
 		t.Run(strconv.Itoa(tt.pathLen), func(t *testing.T) {
 			t.Parallel()
 
+			id := uuid.NewV4()
+			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 i := range 1000 {
 				entry := protowire.AppendTag(nil, 1, protowire.BytesType) // MFFilePath.path
-			entry = protowire.AppendString(entry, strings.Repeat("a", tt.pathLen))
+				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
@@ -150,11 +205,6 @@ func TestDeserializeRefusesEntriesThatDecodeTooLarge(t *testing.T) {
 				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.AppendBytes(inner, id[:])
-
-			for range 1000 {
 				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
+// TestSerializeInnerNotSetMessagesVerbatim pins the messages generate and
-// "internal error" messages, which differ between generate and
+// generateOuter return when the inner message is missing.
-// generateOuter and have always done so.
+func TestSerializeInnerNotSetMessagesVerbatim(t *testing.T) {
-func TestSerializeInternalErrorMessagesVerbatim(t *testing.T) {
 	t.Parallel()
 
 	m := &manifest{}
-	require.EqualError(t, m.generate(context.Background()),
+
-		"internal error: pbInner not set")
+	err := m.generate(context.Background())
-	require.EqualError(t, m.generateOuter(context.Background()), "internal error")
+	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,16 +41,26 @@ const (
 	// fields in a gpg fingerprint record (the fingerprint is field 10).
 	gpgFingerprintMinFields = 10
 
+	// 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"
+	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")
-	errImportedFPRNotFound = errors.New("fingerprint not found in imported 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).
@@ -78,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
+// optional stdin, returning captured stdout and stderr. If gpg fails, the
-// ctx ends or gpgTimeout passes, whichever comes first.
+// 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) {
@@ -115,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) {
@@ -136,29 +162,77 @@ func parseFingerprint(colonOutput string) (string, bool) {
 	return "", false
 }
 
-// gpgSign creates a detached signature of the data using the specified key.
+// parseStatusLine returns the arguments of the status line for keyword in
-// Returns the armored detached signature.
+// gpg --status-fd output, or ok=false unless there is exactly one such line
-func gpgSign(ctx context.Context, data []byte, keyID GPGKeyID) ([]byte, error) {
+// and it has arguments.
+func parseStatusLine(statusOutput, keyword string) ([]string, bool) {
+	var found [][]string
+
+	for line := range strings.SplitSeq(statusOutput, "\n") {
+		fields := strings.Fields(line)
+		if len(fields) > 2 && fields[0] == gpgStatusPrefix && fields[1] == keyword {
+			found = append(found, fields[2:])
+		}
+	}
+
+	if len(found) != 1 {
+		return nil, false
+	}
+
+	return found[0], true
+}
+
+// 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 {
@@ -170,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(
+		return nil, fmt.Errorf("gpg fingerprint lookup: %w", err)
-			"gpg fingerprint lookup failed: %w: %s", err, stderr.String(),
-		)
 	}
 
 	fpr, ok := parseFingerprint(stdout.String())
@@ -187,69 +259,44 @@ func gpgGetKeyFingerprint(ctx context.Context, keyID GPGKeyID) ([]byte, error) {
 	return []byte(fpr), nil
 }
 
-// gpgExtractPubKeyFingerprint imports a public key into a temporary keyring
+// gpgImportOneKey imports the public key block in pubKeyFile into the
-// and extracts its fingerprint. This verifies the key is valid and returns
+// keyring in gpgHome. The block must hold exactly one primary key.
-// the actual fingerprint from the key material.
+func gpgImportOneKey(ctx context.Context, gpgHome, pubKeyFile string) error {
-func gpgExtractPubKeyFingerprint(ctx context.Context, pubKey []byte) (string, error) {
+	// --status-fd 1 sends gpg's status lines to stdout, which importing
-	// Create temporary directory for GPG operations
+	// otherwise leaves empty; its messages go to stderr.
-	tmpDir, err := os.MkdirTemp("", "mfer-gpg-fingerprint-*")
+	importStdout, _, err := runGPG(ctx, nil,
-	if err != nil {
+		gpgArgs([]string{gpgOptHomedir, gpgHome, gpgOptStatusFD, "1", "--import"},
-		return "", fmt.Errorf("failed to create temp dir: %w", err)
+			pubKeyFile)...,
-	}
-
-	defer func() { _ = os.RemoveAll(tmpDir) }()
-
-	// Set restrictive permissions
-	err = os.Chmod(tmpDir, privateDirPerms)
-	if err != nil {
-		return "", fmt.Errorf("failed to set temp dir permissions: %w", err)
-	}
-
-	// Write public key to temp file
-	pubKeyFile := filepath.Join(tmpDir, "pubkey.asc")
-
-	err = os.WriteFile(pubKeyFile, pubKey, privateFilePerms)
-	if err != nil {
-		return "", fmt.Errorf("failed to write public key: %w", err)
-	}
-
-	// Import the public key into the temporary keyring
-	_, importStderr, err := runGPG(ctx, nil,
-		gpgArgs([]string{gpgOptHomedir, tmpDir, "--import"}, pubKeyFile)...,
 	)
 	if err != nil {
-		return "", fmt.Errorf(
+		return fmt.Errorf("gpg import: %w", err)
-			"failed to import public key: %w: %s", err, importStderr.String(),
-		)
 	}
 
-	// List keys to get fingerprint
+	// The first argument of IMPORT_RES counts the primary keys gpg read
-	listStdout, listStderr, err := runGPG(ctx, nil,
+	// from the block, those it then skipped (one with no user ID, for
-		"--homedir", tmpDir,
+	// example) included.
-		"--with-colons",
+	result, ok := parseStatusLine(importStdout.String(), "IMPORT_RES")
-		"--fingerprint",
-	)
-	if err != nil {
-		return "", fmt.Errorf(
-			"failed to list keys: %w: %s", err, listStderr.String(),
-		)
-	}
-
-	fpr, ok := parseFingerprint(listStdout.String())
 	if !ok {
-		return "", errImportedFPRNotFound
+		return fmt.Errorf("%w, gpg reported no count", errSigningKeyCount)
 	}
 
-	return fpr, nil
+	if result[0] != "1" {
+		return fmt.Errorf("%w, found %s", errSigningKeyCount, result[0])
 	}
 
-// gpgVerify verifies a detached signature against data using the provided public key.
+	return nil
-// It creates a temporary keyring to import the public key for verification.
+}
-func gpgVerify(ctx context.Context, data, signature, pubKey []byte) error {
+
+// gpgVerify verifies a detached signature against data using the provided
+// public key, imported into a temporary keyring, and returns the
+// fingerprint of the primary key that made the signature. The public key
+// must hold exactly one primary key, so that a good signature can come
+// from no other key.
+func gpgVerify(ctx context.Context, data, signature, pubKey []byte) (string, error) {
 	// 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) }()
@@ -257,7 +304,7 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) error {
 	// 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
@@ -265,7 +312,7 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) error {
 
 	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
@@ -273,7 +320,7 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) error {
 
 	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
@@ -281,29 +328,31 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) error {
 
 	err = os.WriteFile(dataFile, data, privateFilePerms)
 	if err != nil {
-		return fmt.Errorf("failed to write data: %w", err)
+		return "", err
 	}
 
-	// Import the public key into the temporary keyring
+	err = gpgImportOneKey(ctx, tmpDir, pubKeyFile)
-	_, importStderr, err := runGPG(ctx, nil,
-		gpgArgs([]string{gpgOptHomedir, tmpDir, "--import"}, pubKeyFile)...,
-	)
 	if err != nil {
-		return fmt.Errorf(
+		return "", err
-			"failed to import public key: %w: %s", err, importStderr.String(),
-		)
 	}
 
-	// Verify the signature
+	// --status-fd 1 sends gpg's status lines to stdout, which verifying a
-	_, verifyStderr, err := runGPG(ctx, nil,
+	// detached signature otherwise leaves empty; its messages go to stderr.
-		gpgArgs([]string{gpgOptHomedir, tmpDir, gpgOptVerify},
+	verifyStdout, _, err := runGPG(ctx, nil,
+		gpgArgs([]string{gpgOptHomedir, tmpDir, gpgOptStatusFD, "1", gpgOptVerify},
 			sigFile, dataFile)...,
 	)
 	if err != nil {
-		return fmt.Errorf(
+		return "", fmt.Errorf("gpg verify: %w", err)
-			"signature verification failed: %w: %s", err, verifyStderr.String(),
-		)
 	}
 
-	return nil
+	// 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 valid[len(valid)-1], nil
 }

mfer/gpg_test.go

@@ -8,6 +8,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"slices"
 	"strconv"
 	"strings"
 	"syscall"
@@ -17,6 +18,7 @@ import (
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"google.golang.org/protobuf/proto"
 )
 
 // testGPGEnv sets up a temporary GPG home directory with a test key.
@@ -35,39 +37,18 @@ func testGPGEnv(t *testing.T) (GPGKeyID, string) {
 	// Create temporary GPG home directory (0700 by default)
 	gpgHome := t.TempDir()
 
-	// Generate a test key with no passphrase
+	genTestKey(t, gpgHome, testKeyParams)
-	keyParams := `%no-protection
-Key-Type: RSA
-Key-Length: 2048
-Name-Real: MFER Test Key
-Name-Email: test@mfer.test
-Expire-Date: 0
-%commit
-`
-	paramsFile := filepath.Join(gpgHome, "key-params")
-	require.NoError(t, os.WriteFile(paramsFile, []byte(keyParams), 0o600))
 
 	ctx, cancel := context.WithTimeout(context.Background(), gpgTimeout)
 	defer cancel()
 
-	//nolint:gosec // paramsFile is a test-controlled path inside t.TempDir()
-	cmd := exec.CommandContext(ctx, "gpg",
-		"--batch", "--gen-key", paramsFile)
-
-	cmd.Env = append(os.Environ(), "GNUPGHOME="+gpgHome)
-
-	output, err := cmd.CombinedOutput()
-	if err != nil {
-		t.Skipf("failed to generate test GPG key: %v: %s", err, output)
-	}
-
 	// Get the key fingerprint
-	cmd = exec.CommandContext(ctx, "gpg",
+	cmd := exec.CommandContext(ctx, "gpg",
 		"--list-keys", "--with-colons", "test@mfer.test")
 
 	cmd.Env = append(os.Environ(), "GNUPGHOME="+gpgHome)
 
-	output, err = cmd.Output()
+	output, err := cmd.Output()
 	if err != nil {
 		t.Fatalf("failed to list test key: %v", err)
 	}
@@ -92,13 +73,79 @@ Expire-Date: 0
 	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>" from the gpg key generation parameters
+// keyParams in gpgHome, which may already hold one.
+func genTestKey(t *testing.T, gpgHome, keyParams string) {
+	t.Helper()
+
+	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(params), 0o600))
+
+	ctx, cancel := context.WithTimeout(context.Background(), gpgTimeout)
+	defer cancel()
+
+	//nolint:gosec // paramsFile is a test-controlled path inside t.TempDir()
+	cmd := exec.CommandContext(ctx, "gpg",
+		"--batch", "--gen-key", paramsFile)
+
+	cmd.Env = append(os.Environ(), "GNUPGHOME="+gpgHome)
+
+	output, err := cmd.CombinedOutput()
+	if err != nil {
+		t.Skipf("failed to generate test GPG key: %v: %s", err, output)
+	}
+}
+
+// signedTestManifest returns a manifest of one file signed with keyID.
+func signedTestManifest(t *testing.T, keyID GPGKeyID) []byte {
+	t.Helper()
+
+	b := NewBuilder()
+	b.SetSigningOptions(&SigningOptions{KeyID: keyID})
+
+	content := []byte("signed file content")
+	_, err := b.AddFile("test.txt", FileSize(len(content)), ModTime{}, 0,
+		bytes.NewReader(content), nil)
+	require.NoError(t, err)
+
+	var buf bytes.Buffer
+
+	require.NoError(t, b.Build(context.Background(), &buf))
+
+	return buf.Bytes()
+}
+
+// rewriteOuter returns manifest with its outer message changed by edit.
+// A signature stays good as long as edit leaves the UUID and hash alone.
+func rewriteOuter(t *testing.T, manifest []byte, edit func(*MFFileOuter)) []byte {
+	t.Helper()
+
+	outer := new(MFFileOuter)
+	require.NoError(t, proto.Unmarshal(manifest[len(MAGIC):], outer))
+
+	edit(outer)
+
+	data, err := proto.Marshal(outer)
+	require.NoError(t, err)
+
+	return append([]byte(MAGIC), data...)
+}
+
 func TestGPGSign(t *testing.T) {
 	keyID, gpgHome := testGPGEnv(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-----")
@@ -163,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) {
@@ -259,15 +309,16 @@ 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)
 	require.NoError(t, err)
 
-	// Verify the signature
+	// Verify the signature; it names the key that made it
-	err = gpgVerify(context.Background(), data, sig, pubKey)
+	signingKey, err := gpgVerify(context.Background(), data, sig, pubKey)
 	require.NoError(t, err)
+	assert.Equal(t, string(keyID), signingKey)
 }
 
 func TestGPGVerifyInvalidSignature(t *testing.T) {
@@ -275,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)
@@ -283,7 +334,7 @@ func TestGPGVerifyInvalidSignature(t *testing.T) {
 
 	// Try to verify with different data - should fail
 	wrongData := []byte("different data")
-	err = gpgVerify(context.Background(), wrongData, sig, pubKey)
+	_, err = gpgVerify(context.Background(), wrongData, sig, pubKey)
 	assert.Error(t, err)
 }
 
@@ -292,12 +343,12 @@ 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
 	badPubKey := []byte("not a valid public key")
-	err = gpgVerify(context.Background(), data, sig, badPubKey)
+	_, err = gpgVerify(context.Background(), data, sig, badPubKey)
 	assert.Error(t, err)
 }
 
@@ -368,6 +419,144 @@ func TestManifestTamperedSignatureFails(t *testing.T) {
 	assert.Error(t, err)
 }
 
+// TestManifestRefusesSecondEmbeddedKey loads a manifest whose embedded
+// public key block holds another key before the key that signed it.
+// Loading must refuse it, although the signature is good and the signer
+// field names the key that made it.
+func TestManifestRefusesSecondEmbeddedKey(t *testing.T) {
+	otherKey, otherHome := testGPGEnv(t)
+	t.Setenv("GNUPGHOME", otherHome)
+
+	otherPubKey, err := gpgExportPublicKey(context.Background(), 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, outer.GetSigningPubKey())
+		})
+
+	_, err = NewManifestFromReader(bytes.NewReader(manifest))
+	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) {
+	keyID, gpgHome := testGPGEnv(t)
+	t.Setenv("GNUPGHOME", gpgHome)
+
+	manifest := rewriteOuter(t, signedTestManifest(t, keyID),
+		func(outer *MFFileOuter) {
+			outer.Signer = []byte(strings.Repeat("A", len(keyID)))
+		})
+
+	_, err := NewManifestFromReader(bytes.NewReader(manifest))
+	require.ErrorIs(t, err, errSignerNotSigningKey)
+}
+
+// TestBuilderSigningKeyIDMatchingTwoKeys signs with a key ID that two keys
+// in the keyring match. The manifest must embed and name only the key that
+// signed it, or loading refuses it.
+func TestBuilderSigningKeyIDMatchingTwoKeys(t *testing.T) {
+	_, gpgHome := testGPGEnv(t)
+	genTestKey(t, gpgHome, testKeyParams)
+	t.Setenv("GNUPGHOME", gpgHome)
+
+	manifest := signedTestManifest(t, GPGKeyID("test@mfer.test"))
+
+	_, err := NewManifestFromReader(bytes.NewReader(manifest))
+	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()
 
@@ -414,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
@@ -446,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,6 +80,7 @@ type FileEntry struct {
 type Scanner struct {
 	mu         sync.RWMutex
 	files      []*FileEntry
+	paths      map[RelFilePath]AbsFilePath // the file at each path in files
 	totalBytes FileSize                    // cached sum of all file sizes
 	options    *ScannerOptions
 	fs         afero.Fs
@@ -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
+	errInternal = errors.New("inner message not set")
-	// match on text, so they are kept distinct.
-	errInternal = errors.New("internal error")
 )
 
 // 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,30 +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.
+// 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)
 	}
 
-	sig, err := gpgSign(ctx, []byte(sigString), m.signingOptions.KeyID)
+	sig, signingKey, err := gpgSign(ctx, []byte(sigString), m.signingOptions.KeyID)
 	if err != nil {
-		return fmt.Errorf("failed to sign manifest: %w", err)
+		return err
 	}
 
 	m.pbOuter.Signature = sig
 
-	fingerprint, err := gpgGetKeyFingerprint(ctx, m.signingOptions.KeyID)
+	// 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 fmt.Errorf("failed to get key fingerprint: %w", err)
+		return err
 	}
 
 	m.pbOuter.Signer = fingerprint
 
-	pubKey, err := gpgExportPublicKey(ctx, m.signingOptions.KeyID)
+	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