Make error message wording consistent (closes #165)

Error messages in mfer/ and internal/cli/ are lowercase except names and
acronyms, carry no "failed to" or command-name prefix, and each wrap names
only the operation and thing the wrapped error does not already name, so a
stacked message names what failed once. Wraps around errors that already
name their operation and path (os and afero path errors, url.Error, the gpg
helpers' own errors) are dropped. runGPG appends gpg's stderr only when gpg
wrote some, so no message ends in a colon. errHTTPStatus reads "unexpected
HTTP status"; both inner-not-set sentinels read "inner message not set".
No sentinel, errors.Is result or exit status changes. Message tests pin the
new text through the real call sites.

Model: opus-5-5

mfer/builder.go

@@ -246,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 {
@@ -329,22 +329,19 @@ 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 nil
+	return err
 }
 
 // addEntry adds entry to the manifest unless an entry with its path is

mfer/deserialize.go

@@ -24,13 +24,12 @@ var (
 	errCompressedHashWrong  = errors.New("compressed data hash mismatch")
 	errSignatureNoPubKey    = errors.New("signature present but no public key")
 	errDecompressedTooLarge = errors.New("decompressed data exceeds maximum allowed size")
-	errManifestTooLarge     = errors.New("manifest exceeds maximum allowed size")
+	errManifestTooLarge     = errors.New("file exceeds maximum allowed size")
 	errUUIDMismatch         = errors.New("outer and inner UUID mismatch")
 	errInvalidFileFormat    = errors.New("invalid file format")
-	errInvalidManifestPath  = errors.New("manifest contains invalid path")
-	errDecodedTooLarge      = errors.New(
-		"manifest would take too much memory to decode")
-	errSignerNotSigningKey = errors.New(
+	errInvalidManifestPath  = errors.New("invalid file entry")
+	errDecodedTooLarge      = errors.New("too much memory needed")
+	errSignerNotSigningKey  = errors.New(
 		"signer is not the fingerprint of the key that made the signature")
 )
 
@@ -58,7 +57,7 @@ func (m *manifest) validateOuterHeader() error {
 	// Validate outer UUID before any decompression
 	err := validateUUID(m.pbOuter.GetUuid())
 	if err != nil {
-		return fmt.Errorf("outer UUID invalid: %w", err)
+		return fmt.Errorf("outer message: %w", err)
 	}
 
 	return nil
@@ -73,7 +72,7 @@ func (m *manifest) verifyOuterIntegrity() error {
 
 	_, err := h.Write(m.pbOuter.GetInnerMessage())
 	if err != nil {
-		return fmt.Errorf("deserialize: hash write: %w", err)
+		return fmt.Errorf("hash inner message: %w", err)
 	}
 
 	sha256Hash := h.Sum(nil)
@@ -91,9 +90,7 @@ func (m *manifest) verifyOuterIntegrity() error {
 
 	sigString, err := m.signatureString()
 	if err != nil {
-		return fmt.Errorf(
-			"failed to generate signature string for verification: %w", err,
-		)
+		return fmt.Errorf("build signature string: %w", err)
 	}
 
 	// Loading a manifest takes no context; gpgTimeout still bounds gpg.
@@ -104,7 +101,7 @@ func (m *manifest) verifyOuterIntegrity() error {
 		m.pbOuter.GetSigningPubKey(),
 	)
 	if err != nil {
-		return fmt.Errorf("signature verification failed: %w", err)
+		return err
 	}
 
 	if !strings.EqualFold(string(m.pbOuter.GetSigner()), signingKey) {
@@ -135,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()
 
@@ -150,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 {
@@ -266,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
@@ -275,7 +272,7 @@ 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 {
@@ -285,7 +282,7 @@ func (m *manifest) deserializeInner() error {
 	// 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

mfer/deserialize_test.go

@@ -49,6 +49,6 @@ func TestReadAtMost(t *testing.T) {
 	_, err = readAtMost(input, maxSize)
 	require.ErrorIs(t, err, errManifestTooLarge)
 	require.EqualError(t, err,
-		"manifest exceeds maximum allowed size of 65536 bytes")
+		"file exceeds maximum allowed size of 65536 bytes")
 	assert.Equal(t, maxSize-1, input.Len(), "bytes left unread")
 }

mfer/errmsg_test.go

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

mfer/gpg.go

@@ -53,7 +53,7 @@ const (
 )
 
 var (
-	errGPGKeyNotFound      = errors.New("gpg key not found")
+	errGPGKeyNotFound      = errors.New("GPG key not found")
 	errFingerprintNotFound = errors.New("fingerprint not found for key")
 	errSigningKeyCount     = errors.New(
 		"embedded public key block must hold exactly one key")
@@ -88,11 +88,12 @@ func gpgArgs(opts []string, positional ...string) []string {
 }
 
 // runGPG runs the gpg binary in batch mode with the given arguments and
-// optional stdin, returning captured stdout and stderr. gpg is killed when
-// ctx ends or gpgTimeout passes, whichever comes first.
+// optional stdin, returning captured stdout. If gpg fails, the error ends
+// with what gpg wrote to stderr. gpg is killed when ctx ends or gpgTimeout
+// passes, whichever comes first.
 func runGPG(
 	ctx context.Context, stdin io.Reader, args ...string,
-) (*bytes.Buffer, *bytes.Buffer, error) {
+) (*bytes.Buffer, error) {
 	// exec.CommandContext kills only gpg itself. A gpg-agent that gpg
 	// starts runs detached and holds none of gpg's output, but another
 	// process gpg leaves behind (a wrapper script that runs the real gpg
@@ -125,11 +126,16 @@ 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)
 		}
 	}
 
-	return &stdout, &stderr, err
+	messages := strings.TrimSpace(stderr.String())
+	if err != nil && messages != "" {
+		err = fmt.Errorf("%w: %s", err, messages)
+	}
+
+	return &stdout, err
 }
 
 // parseFingerprint extracts the first fingerprint from gpg --with-colons
@@ -174,7 +180,7 @@ func gpgSign(
 ) ([]byte, string, error) {
 	tmpDir, err := os.MkdirTemp("", "mfer-gpg-sign-*")
 	if err != nil {
-		return nil, "", fmt.Errorf("failed to create temp dir: %w", err)
+		return nil, "", err
 	}
 
 	defer func() { _ = os.RemoveAll(tmpDir) }()
@@ -183,7 +189,7 @@ func gpgSign(
 
 	// 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),
+	stdout, err := runGPG(ctx, bytes.NewReader(data),
 		"--detach-sign",
 		gpgOptArmor,
 		"--output", sigFile,
@@ -191,19 +197,19 @@ func gpgSign(
 		"--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)
 	}
 
 	// 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, "", fmt.Errorf("%w: %s", errSigningKeyNotReported, stderr.String())
+		return nil, "", errSigningKeyNotReported
 	}
 
 	sig, err := os.ReadFile(sigFile) //nolint:gosec // G304: inside tmpDir, made above
 	if err != nil {
-		return nil, "", fmt.Errorf("failed to read signature: %w", err)
+		return nil, "", err
 	}
 
 	return sig, created[len(created)-1], nil
@@ -212,11 +218,11 @@ func gpgSign(
 // 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 {
@@ -228,13 +234,11 @@ func gpgExportPublicKey(ctx context.Context, keyID GPGKeyID) ([]byte, error) {
 
 // gpgGetKeyFingerprint gets the full fingerprint for a key ID.
 func gpgGetKeyFingerprint(ctx context.Context, keyID GPGKeyID) ([]byte, error) {
-	stdout, stderr, err := runGPG(ctx, nil,
+	stdout, err := runGPG(ctx, nil,
 		gpgArgs([]string{"--with-colons", "--fingerprint"}, string(keyID))...,
 	)
 	if err != nil {
-		return nil, fmt.Errorf(
-			"gpg fingerprint lookup failed: %w: %s", err, stderr.String(),
-		)
+		return nil, fmt.Errorf("gpg fingerprint lookup: %w", err)
 	}
 
 	fpr, ok := parseFingerprint(stdout.String())
@@ -250,14 +254,12 @@ func gpgGetKeyFingerprint(ctx context.Context, keyID GPGKeyID) ([]byte, error) {
 func gpgImportOneKey(ctx context.Context, gpgHome, pubKeyFile string) error {
 	// --status-fd 1 sends gpg's status lines to stdout, which importing
 	// otherwise leaves empty; its messages go to stderr.
-	importStdout, importStderr, err := runGPG(ctx, nil,
+	importStdout, err := runGPG(ctx, nil,
 		gpgArgs([]string{gpgOptHomedir, gpgHome, gpgOptStatusFD, "1", "--import"},
 			pubKeyFile)...,
 	)
 	if err != nil {
-		return fmt.Errorf(
-			"failed to import public key: %w: %s", err, importStderr.String(),
-		)
+		return fmt.Errorf("gpg import: %w", err)
 	}
 
 	// The first argument of IMPORT_RES counts the primary keys gpg read
@@ -284,7 +286,7 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) (string, err
 	// Create temporary directory for GPG operations
 	tmpDir, err := os.MkdirTemp("", "mfer-gpg-verify-*")
 	if err != nil {
-		return "", fmt.Errorf("failed to create temp dir: %w", err)
+		return "", err
 	}
 
 	defer func() { _ = os.RemoveAll(tmpDir) }()
@@ -292,7 +294,7 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) (string, err
 	// Set restrictive permissions
 	err = os.Chmod(tmpDir, privateDirPerms)
 	if err != nil {
-		return "", fmt.Errorf("failed to set temp dir permissions: %w", err)
+		return "", err
 	}
 
 	// Write public key to temp file
@@ -300,7 +302,7 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) (string, err
 
 	err = os.WriteFile(pubKeyFile, pubKey, privateFilePerms)
 	if err != nil {
-		return "", fmt.Errorf("failed to write public key: %w", err)
+		return "", err
 	}
 
 	// Write signature to temp file
@@ -308,7 +310,7 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) (string, err
 
 	err = os.WriteFile(sigFile, signature, privateFilePerms)
 	if err != nil {
-		return "", fmt.Errorf("failed to write signature: %w", err)
+		return "", err
 	}
 
 	// Write data to temp file
@@ -316,7 +318,7 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) (string, err
 
 	err = os.WriteFile(dataFile, data, privateFilePerms)
 	if err != nil {
-		return "", fmt.Errorf("failed to write data: %w", err)
+		return "", err
 	}
 
 	err = gpgImportOneKey(ctx, tmpDir, pubKeyFile)
@@ -326,14 +328,12 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) (string, err
 
 	// --status-fd 1 sends gpg's status lines to stdout, which verifying a
 	// detached signature otherwise leaves empty; its messages go to stderr.
-	verifyStdout, verifyStderr, err := runGPG(ctx, nil,
+	verifyStdout, err := runGPG(ctx, nil,
 		gpgArgs([]string{gpgOptHomedir, tmpDir, gpgOptStatusFD, "1", gpgOptVerify},
 			sigFile, dataFile)...,
 	)
 	if err != nil {
-		return "", fmt.Errorf(
-			"signature verification failed: %w: %s", err, verifyStderr.String(),
-		)
+		return "", fmt.Errorf("gpg verify: %w", err)
 	}
 
 	// gpg writes a VALIDSIG line for each good signature. Its first

mfer/gpg_test.go

@@ -451,7 +451,7 @@ func TestManifestRefusesSecondEmbeddedKeyWithoutUserID(t *testing.T) {
 	t.Setenv("GNUPGHOME", otherHome)
 
 	// Keeping only the user IDs that match "nobody" exports none.
-	otherPubKey, _, err := runGPG(context.Background(), nil,
+	otherPubKey, err := runGPG(context.Background(), nil,
 		gpgArgs([]string{
 			"--export", gpgOptArmor, "--export-filter", "keep-uid=uid = nobody",
 		}, string(otherKey))...)
@@ -603,7 +603,8 @@ 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"))
 
@@ -612,7 +613,7 @@ func TestGPGTimeoutKillsGPG(t *testing.T) {
 
 	_, _, 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")
 }
 
 // TestGPGCancelWhenChildHoldsOutput uses a fake gpg that runs sleep as a

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/serialize.go

@@ -20,11 +20,9 @@ const MAGIC string = "ZNAVSRFG"
 var (
 	// errInnerNotSet is returned by generate when the inner manifest is
 	// missing.
-	errInnerNotSet = errors.New("internal error: pbInner not set")
+	errInnerNotSet = errors.New("inner message not set")
 	// errInternal is returned by generateOuter for the same condition.
-	// The two messages differ, and both are load-bearing for callers that
-	// match on text, so they are kept distinct.
-	errInternal = errors.New("internal error")
+	errInternal = errors.New("inner message not set")
 )
 
 // nanosecondsInt32 converts t's nanosecond component to int32.
@@ -65,14 +63,14 @@ func (m *manifest) generate(ctx context.Context) error {
 
 	dat, err := proto.MarshalOptions{Deterministic: true}.Marshal(m.pbOuter)
 	if err != nil {
-		return fmt.Errorf("serialize: marshal outer: %w", err)
+		return fmt.Errorf("marshal outer message: %w", err)
 	}
 
 	m.output = bytes.NewBufferString(MAGIC)
 
 	_, err = m.output.Write(dat)
 	if err != nil {
-		return fmt.Errorf("serialize: write output: %w", err)
+		return fmt.Errorf("write outer message: %w", err)
 	}
 
 	return nil
@@ -95,7 +93,7 @@ func (m *manifest) generateOuter(ctx context.Context) error {
 
 	innerData, err := proto.MarshalOptions{Deterministic: true}.Marshal(m.pbInner)
 	if err != nil {
-		return fmt.Errorf("serialize: marshal inner: %w", err)
+		return fmt.Errorf("marshal inner message: %w", err)
 	}
 
 	// Compress the inner data
@@ -103,12 +101,12 @@ func (m *manifest) generateOuter(ctx context.Context) error {
 
 	zw, err := zstd.NewWriter(idc, zstd.WithEncoderLevel(zstd.SpeedBestCompression))
 	if err != nil {
-		return fmt.Errorf("serialize: create compressor: %w", err)
+		return fmt.Errorf("create compressor: %w", err)
 	}
 
 	_, err = zw.Write(innerData)
 	if err != nil {
-		return fmt.Errorf("serialize: compress: %w", err)
+		return fmt.Errorf("compress inner message: %w", err)
 	}
 
 	_ = zw.Close()
@@ -120,7 +118,7 @@ func (m *manifest) generateOuter(ctx context.Context) error {
 
 	_, err = h.Write(compressedData)
 	if err != nil {
-		return fmt.Errorf("serialize: hash write: %w", err)
+		return fmt.Errorf("hash inner message: %w", err)
 	}
 
 	sha256Hash := h.Sum(nil)
@@ -149,12 +147,12 @@ func (m *manifest) generateOuter(ctx context.Context) error {
 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, 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
@@ -163,14 +161,14 @@ func (m *manifest) signOuter(ctx context.Context) error {
 	// 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, GPGKeyID(fingerprint))
 	if err != nil {
-		return fmt.Errorf("failed to export public key: %w", err)
+		return err
 	}
 
 	m.pbOuter.SigningPubKey = pubKey