Make error message wording consistent (closes #165)
check / check (push) Waiting to run

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
builder's path errors, the gpg helpers' own errors) are dropped. gpg's
stderr is appended to a gpg failure, and to the error for a signing key gpg
did not report, only when gpg wrote some. errHTTPStatus reads "unexpected
HTTP status"; both inner-not-set sentinels read "inner message not set".
No sentinel, errors.Is result or exit status changes.

Model: opus-5-5
This commit was merged in pull request #176.
This commit is contained in:
2026-10-07 17:25:41 +02:00
parent dce5e050c3
commit c0b099cc48
18 changed files with 289 additions and 175 deletions
+37 -27
View File
@@ -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,8 +88,9 @@ func gpgArgs(opts []string, positional ...string) []string {
}
// runGPG runs the gpg binary in batch mode with the given arguments and
// optional stdin, returning captured stdout and stderr. gpg is killed when
// ctx ends or gpgTimeout passes, whichever comes first.
// optional stdin, returning captured stdout and stderr. If gpg fails, the
// error ends with what gpg wrote to stderr. gpg is killed when ctx ends or
// gpgTimeout passes, whichever comes first.
func runGPG(
ctx context.Context, stdin io.Reader, args ...string,
) (*bytes.Buffer, *bytes.Buffer, error) {
@@ -125,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) {
@@ -174,7 +190,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) }()
@@ -191,19 +207,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, "", withStderr(errSigningKeyNotReported, stderr)
}
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 +228,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 +244,11 @@ func gpgExportPublicKey(ctx context.Context, keyID GPGKeyID) ([]byte, error) {
// gpgGetKeyFingerprint gets the full fingerprint for a key ID.
func gpgGetKeyFingerprint(ctx context.Context, keyID GPGKeyID) ([]byte, error) {
stdout, stderr, err := runGPG(ctx, nil,
stdout, _, err := runGPG(ctx, nil,
gpgArgs([]string{"--with-colons", "--fingerprint"}, string(keyID))...,
)
if err != nil {
return nil, fmt.Errorf(
"gpg fingerprint lookup failed: %w: %s", err, stderr.String(),
)
return nil, fmt.Errorf("gpg fingerprint lookup: %w", err)
}
fpr, ok := parseFingerprint(stdout.String())
@@ -250,14 +264,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 +296,7 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) (string, err
// Create temporary directory for GPG operations
tmpDir, err := os.MkdirTemp("", "mfer-gpg-verify-*")
if err != nil {
return "", fmt.Errorf("failed to create temp dir: %w", err)
return "", err
}
defer func() { _ = os.RemoveAll(tmpDir) }()
@@ -292,7 +304,7 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) (string, err
// Set restrictive permissions
err = os.Chmod(tmpDir, privateDirPerms)
if err != nil {
return "", fmt.Errorf("failed to set temp dir permissions: %w", err)
return "", err
}
// Write public key to temp file
@@ -300,7 +312,7 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) (string, err
err = os.WriteFile(pubKeyFile, pubKey, privateFilePerms)
if err != nil {
return "", fmt.Errorf("failed to write public key: %w", err)
return "", err
}
// Write signature to temp file
@@ -308,7 +320,7 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) (string, err
err = os.WriteFile(sigFile, signature, privateFilePerms)
if err != nil {
return "", fmt.Errorf("failed to write signature: %w", err)
return "", err
}
// Write data to temp file
@@ -316,7 +328,7 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) (string, err
err = os.WriteFile(dataFile, data, privateFilePerms)
if err != nil {
return "", fmt.Errorf("failed to write data: %w", err)
return "", err
}
err = gpgImportOneKey(ctx, tmpDir, pubKeyFile)
@@ -326,14 +338,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