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 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
This commit is contained in:
2026-10-07 13:49:31 +00:00
parent 4fe1ff2fe1
commit 98c916ef20
18 changed files with 214 additions and 173 deletions
+8 -8
View File
@@ -108,7 +108,7 @@ func (mfa *CLIApp) fetchManifestToTemp(
if tmpErr != nil { if tmpErr != nil {
_ = rc.Close() _ = rc.Close()
return "", fmt.Errorf("failed to create temp file: %w", tmpErr) return "", tmpErr
} }
tmpPath := tmpFile.Name() tmpPath := tmpFile.Name()
@@ -126,7 +126,7 @@ func (mfa *CLIApp) fetchManifestToTemp(
if cpErr != nil { if cpErr != nil {
_ = mfa.Fs.Remove(tmpPath) _ = mfa.Fs.Remove(tmpPath)
return "", fmt.Errorf("failed to download manifest: %w", cpErr) return "", fmt.Errorf("download manifest: %w", cpErr)
} }
return tmpPath, nil return tmpPath, nil
@@ -142,7 +142,7 @@ func verifyRequiredSigner(chk *mfer.Checker, requiredSigner string) error {
_, err := hex.DecodeString(requiredSigner) _, err := hex.DecodeString(requiredSigner)
if err != nil { if err != nil {
return fmt.Errorf("invalid fingerprint: must be valid hex: %w", err) return fmt.Errorf("invalid fingerprint: %w", err)
} }
if !chk.IsSigned() { if !chk.IsSigned() {
@@ -232,7 +232,7 @@ func findExtraFiles(
err := chk.FindExtraFiles(ctx, extraResults) err := chk.FindExtraFiles(ctx, extraResults)
if err != nil { if err != nil {
return fmt.Errorf("failed to check for extra files: %w", err) return fmt.Errorf("find extra files: %w", err)
} }
<-extraDone <-extraDone
@@ -275,7 +275,7 @@ func runCheck(
progressWg.Wait() progressWg.Wait()
if err != nil { 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 // Wait for results processing to complete
@@ -296,14 +296,14 @@ func (mfa *CLIApp) checkManifestOperation(
manifestPath, err := mfa.resolveManifestArg(cmd) manifestPath, err := mfa.resolveManifestArg(cmd)
if err != nil { if err != nil {
return fmt.Errorf("check: %w", err) return err
} }
// URL manifests need to be downloaded to a temp file for the checker // URL manifests need to be downloaded to a temp file for the checker
if isHTTPURL(manifestPath) { if isHTTPURL(manifestPath) {
tmpPath, tmpErr := mfa.fetchManifestToTemp(ctx, manifestPath) tmpPath, tmpErr := mfa.fetchManifestToTemp(ctx, manifestPath)
if tmpErr != nil { if tmpErr != nil {
return fmt.Errorf("check: %w", tmpErr) return tmpErr
} }
defer func() { _ = mfa.Fs.Remove(tmpPath) }() defer func() { _ = mfa.Fs.Remove(tmpPath) }()
@@ -324,7 +324,7 @@ func (mfa *CLIApp) checkManifestOperation(
Fs: mfa.Fs, Fs: mfa.Fs,
}) })
if err != nil { if err != nil {
return fmt.Errorf("failed to load manifest: %w", err) return fmt.Errorf("load manifest: %w", err)
} }
// Check signature requirement // Check signature requirement
+2 -2
View File
@@ -382,7 +382,7 @@ func TestGenerateRefusesTwoFilesAtOnePath(t *testing.T) {
}, fs) }, fs)
assert.Equal(t, 1, runCLI(opts)) assert.Equal(t, 1, runCLI(opts))
assert.Contains(t, testStderr(t, opts), assert.Contains(t, testStderr(t, opts),
`generate: failed to enumerate paths: duplicate path "a.txt": `+ `enumerate files: duplicate path "a.txt": `+
tc.first+"/a.txt and "+tc.second+"/a.txt") tc.first+"/a.txt and "+tc.second+"/a.txt")
exists, err := afero.Exists(fs, testOutput) exists, err := afero.Exists(fs, testOutput)
@@ -674,7 +674,7 @@ func TestCheckRequireSignatureRefusesOtherSigningKey(t *testing.T) {
}, fs) }, fs)
assert.Equal(t, 1, runCLI(opts)) assert.Equal(t, 1, runCLI(opts))
assert.Contains(t, testStderr(t, opts), assert.Contains(t, testStderr(t, opts),
"failed to load manifest: signature verification failed: "+ "load manifest: "+
"embedded public key block must hold exactly one key, found 2") "embedded public key block must hold exactly one key, found 2")
} }
+61 -3
View File
@@ -303,7 +303,7 @@ func TestManifestLoaderHTTPStatusMessage(t *testing.T) {
_, err := mfa.openManifestReader(context.Background(), server.URL+"/foo.mf") _, err := mfa.openManifestReader(context.Background(), server.URL+"/foo.mf")
require.ErrorIs(t, err, errHTTPStatus) require.ErrorIs(t, err, errHTTPStatus)
assert.EqualError(t, err, 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) { func TestFetchManifestHTTPStatusMessage(t *testing.T) {
@@ -325,7 +325,7 @@ func TestFetchManifestHTTPStatusMessage(t *testing.T) {
return cmd.Run(context.Background(), []string{cmdFetch, server.URL}) return cmd.Run(context.Background(), []string{cmdFetch, server.URL})
}) })
require.ErrorIs(t, err, errHTTPStatus) 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) { func TestFetchFileHTTPStatusMessage(t *testing.T) {
@@ -343,7 +343,65 @@ func TestFetchFileHTTPStatusMessage(t *testing.T) {
&mfer.MFFilePath{}, nil) &mfer.MFFilePath{}, nil)
}) })
require.ErrorIs(t, err, errHTTPStatus) 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, "/index.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, "/index.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")
} }
func TestURLRequiredMessage(t *testing.T) { func TestURLRequiredMessage(t *testing.T) {
+4 -4
View File
@@ -26,12 +26,12 @@ func (mfa *CLIApp) exportManifestOperation(
) error { ) error {
pathOrURL, err := mfa.resolveManifestArg(cmd) pathOrURL, err := mfa.resolveManifestArg(cmd)
if err != nil { if err != nil {
return fmt.Errorf("export: %w", err) return err
} }
rc, err := mfa.openManifestReader(ctx, pathOrURL) rc, err := mfa.openManifestReader(ctx, pathOrURL)
if err != nil { if err != nil {
return fmt.Errorf("export: %w", err) return err
} }
defer func() { _ = rc.Close() }() defer func() { _ = rc.Close() }()
@@ -39,7 +39,7 @@ func (mfa *CLIApp) exportManifestOperation(
//nolint:contextcheck // mfer loads a manifest without a context //nolint:contextcheck // mfer loads a manifest without a context
manifest, err := mfer.NewManifestFromReader(rc) manifest, err := mfer.NewManifestFromReader(rc)
if err != nil { if err != nil {
return fmt.Errorf("export: failed to parse manifest: %w", err) return fmt.Errorf("parse manifest: %w", err)
} }
files := manifest.Files() files := manifest.Files()
@@ -76,7 +76,7 @@ func (mfa *CLIApp) exportManifestOperation(
err = enc.Encode(entries) err = enc.Encode(entries)
if err != nil { if err != nil {
return fmt.Errorf("export: failed to encode JSON: %w", err) return fmt.Errorf("encode JSON: %w", err)
} }
return nil return nil
+16 -21
View File
@@ -239,7 +239,7 @@ func reportDownloadProgress(progress <-chan DownloadProgress, done chan<- struct
func manifestBaseURL(manifestURL string) (*url.URL, error) { func manifestBaseURL(manifestURL string) (*url.URL, error) {
parsed, err := url.Parse(manifestURL) parsed, err := url.Parse(manifestURL)
if err != nil { 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 // 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 // Sanitize the path to prevent path traversal attacks
localPath, err := sanitizePath(f.GetPath()) localPath, err := sanitizePath(f.GetPath())
if err != nil { 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) { if alreadyPresent(dest, localPath, f) {
@@ -284,7 +284,7 @@ func downloadManifestFiles(
err = downloadFile(ctx, client, fileURL, dest, localPath, f, progress) err = downloadFile(ctx, client, fileURL, dest, localPath, f, progress)
if err != nil { 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++ downloaded++
@@ -376,7 +376,7 @@ func (mfa *CLIApp) fetchManifestOperation(
err = os.MkdirAll(dest, dirPerms) err = os.MkdirAll(dest, dirPerms)
if err != nil { 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 // Create progress channel and start progress reporter goroutine
@@ -403,7 +403,7 @@ func (mfa *CLIApp) fetchManifestOperation(
// "mfer check" can verify the tree later. // "mfer check" can verify the tree later.
err = saveManifest(dest, manifestData) err = saveManifest(dest, manifestData)
if err != nil { if err != nil {
return fmt.Errorf("failed to save manifest: %w", err) return fmt.Errorf("save manifest: %w", err)
} }
// Print summary // Print summary
@@ -446,11 +446,11 @@ func (mfa *CLIApp) fetchManifest(
return readErr return readErr
}) })
if err != nil { 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 { if int64(len(manifestData)) > mfa.maxManifestSize {
return nil, nil, fmt.Errorf("failed to fetch manifest: %w of %d bytes", return nil, nil, fmt.Errorf("download manifest: %w of %d bytes",
errManifestTooLarge, mfa.maxManifestSize) errManifestTooLarge, mfa.maxManifestSize)
} }
@@ -458,7 +458,7 @@ func (mfa *CLIApp) fetchManifest(
//nolint:contextcheck // mfer loads a manifest without a context //nolint:contextcheck // mfer loads a manifest without a context
manifest, err := mfer.NewManifestFromReader(bytes.NewReader(manifestData)) manifest, err := mfer.NewManifestFromReader(bytes.NewReader(manifestData))
if err != nil { 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) requiredSigner := cmd.String(flagRequireSignature)
@@ -561,7 +561,7 @@ func verifyFetchedSigner(manifestData []byte, requiredSigner string) error {
Fs: memFs, Fs: memFs,
}) })
if err != nil { if err != nil {
return fmt.Errorf("failed to load manifest: %w", err) return fmt.Errorf("load manifest: %w", err)
} }
return verifyRequiredSigner(chk, requiredSigner) return verifyRequiredSigner(chk, requiredSigner)
@@ -653,7 +653,7 @@ func checkNoSymlinks(dest, p string) error {
} }
if err != nil { if err != nil {
return fmt.Errorf("failed to check %s for a symlink: %w", current, err) return err
} }
if info.Mode()&os.ModeSymlink != 0 { if info.Mode()&os.ModeSymlink != 0 {
@@ -770,7 +770,7 @@ func tempPathFor(localPath string) string {
func verifyDownloadedHash(digest []byte, entry *mfer.MFFilePath) error { func verifyDownloadedHash(digest []byte, entry *mfer.MFFilePath) error {
computed, err := multihash.Encode(digest, multihash.SHA2_256) computed, err := multihash.Encode(digest, multihash.SHA2_256)
if err != nil { if err != nil {
return fmt.Errorf("failed to encode hash: %w", err) return fmt.Errorf("encode hash: %w", err)
} }
for _, hash := range entry.GetHashes() { for _, hash := range entry.GetHashes() {
@@ -797,7 +797,7 @@ func downloadFile(
// so every entry point to downloadFile gets the same treatment. // so every entry point to downloadFile gets the same treatment.
localPath, err := sanitizePath(localPath) localPath, err := sanitizePath(localPath)
if err != nil { if err != nil {
return fmt.Errorf("invalid path: %w", err) return fmt.Errorf("invalid file entry: %w", err)
} }
// Create parent directories if needed // Create parent directories if needed
@@ -812,7 +812,7 @@ func downloadFile(
err = os.MkdirAll(dir, dirPerms) err = os.MkdirAll(dir, dirPerms)
if err != nil { if err != nil {
return fmt.Errorf("failed to create directory %s: %w", dir, err) return err
} }
} }
@@ -846,7 +846,7 @@ func createTempFile(dest, tmpPath string) (*os.File, error) {
out, err := os.OpenFile( //nolint:gosec // G304: see comment above out, err := os.OpenFile( //nolint:gosec // G304: see comment above
path, os.O_RDWR|os.O_CREATE|os.O_EXCL, filePerms) path, os.O_RDWR|os.O_CREATE|os.O_EXCL, filePerms)
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to create temp file: %w", err) return nil, err
} }
return out, nil return out, nil
@@ -859,12 +859,7 @@ func moveIntoPlace(dest, tmpPath, localPath string) error {
return err 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, // saveResponse writes resp's body to tmpPath, verifies it against entry,
@@ -899,7 +894,7 @@ func saveResponse(
_ = out.Close() _ = out.Close()
_ = os.Remove(filepath.Join(dest, tmpPath)) _ = os.Remove(filepath.Join(dest, tmpPath))
return fmt.Errorf("failed to set mode: %w", err) return err
} }
} }
+13 -11
View File
@@ -502,8 +502,9 @@ func TestManifestDownloadStopsAtLimit(t *testing.T) {
[]string{cmd.Name, server.URL + "/index.mf"}) []string{cmd.Name, server.URL + "/index.mf"})
}) })
require.ErrorIs(t, err, errManifestTooLarge, cmd.Name) require.ErrorIs(t, err, errManifestTooLarge, cmd.Name)
require.ErrorContains(t, err, require.EqualError(t, err,
"maximum allowed size of 65536 bytes", cmd.Name) "download manifest: file exceeds maximum allowed size of 65536 bytes",
cmd.Name)
} }
leftover, err := os.ReadDir(tmpDir) leftover, err := os.ReadDir(tmpDir)
@@ -605,27 +606,27 @@ func TestFetchRefusesSymlinks(t *testing.T) {
}{ }{
{ {
"parent directory", "sub/deeper/file.txt", "sub", ".", "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", ".", "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, "temp file", testFileTxt, ".file.txt.tmp", newFile,
"failed to download " + testFileTxt, "download " + testFileTxt,
}, },
{ {
"file", testFileTxt, testFileTxt, newFile, "file", testFileTxt, testFileTxt, newFile,
"failed to download " + testFileTxt, "download " + testFileTxt,
}, },
{ {
"manifest temp file", testFileTxt, tempPathFor(defaultManifestName), newFile, "manifest temp file", testFileTxt, tempPathFor(defaultManifestName), newFile,
"failed to save manifest", "save manifest",
}, },
{ {
"manifest", testFileTxt, defaultManifestName, newFile, "manifest", testFileTxt, defaultManifestName, newFile,
"failed to save manifest", "save manifest",
}, },
} }
@@ -692,7 +693,7 @@ func TestFetchDoesNotSkipThroughSymlink(t *testing.T) {
}, afero.NewOsFs()) }, afero.NewOsFs())
assert.Equal(t, 1, runCLI(opts)) assert.Equal(t, 1, runCLI(opts))
assert.Contains(t, testStderr(t, 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)) assert.Equal(t, map[string][]byte{testFileTxt: content}, filesUnder(t, outside))
} }
@@ -762,7 +763,8 @@ func TestGetRetriesTransientStatusesOnly(t *testing.T) {
err := getNothing(testClient(), server.URL, 10*time.Second) err := getNothing(testClient(), server.URL, 10*time.Second)
require.ErrorIs(t, err, errHTTPStatus) 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()) assert.Equal(t, tt.requests, requests.Load())
}) })
} }
@@ -1228,7 +1230,7 @@ func TestFetchRequireSignature(t *testing.T) {
manifest, required := manifestSignedByAnotherKey(t, files) manifest, required := manifestSignedByAnotherKey(t, files)
assertFetchRefused(t, manifest, files, assertFetchRefused(t, manifest, files,
"failed to parse manifest: signature verification failed: "+ "parse manifest: "+
"embedded public key block must hold exactly one key, found 2", "embedded public key block must hold exactly one key, found 2",
"--"+flagRequireSignature, required) "--"+flagRequireSignature, required)
}) })
+14 -22
View File
@@ -165,8 +165,7 @@ func (s *freshenScanner) walk(path string, info fs.FileInfo, walkErr error) erro
// Get relative path // Get relative path
relPath, err := filepath.Rel(s.absBase, path) relPath, err := filepath.Rel(s.absBase, path)
if err != nil { if err != nil {
return fmt.Errorf( return err
"freshen: failed to compute relative path for %s: %w", path, err)
} }
// Handle dotfiles // Handle dotfiles
@@ -280,13 +279,8 @@ func (h *freshenHasher) reportProgress(n int64) {
// processEntry hashes the entry if needed and adds it to the builder. // processEntry hashes the entry if needed and adds it to the builder.
func (h *freshenHasher) processEntry(e *freshenEntry) error { func (h *freshenHasher) processEntry(e *freshenEntry) error {
if !e.needsHash { 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 // Need to read and hash the file
@@ -294,14 +288,14 @@ func (h *freshenHasher) processEntry(e *freshenEntry) error {
f, err := h.fs.Open(absPath) f, err := h.fs.Open(absPath)
if err != nil { if err != nil {
return fmt.Errorf("failed to open %s: %w", e.path, err) return err
} }
hash, bytesRead, err := hashFile(f, h.reportProgress) hash, bytesRead, err := hashFile(f, h.reportProgress)
_ = f.Close() _ = f.Close()
if err != nil { if err != nil {
return fmt.Errorf("failed to hash %s: %w", e.path, err) return fmt.Errorf("hash %s: %w", e.path, err)
} }
h.hashedBytes += bytesRead h.hashedBytes += bytesRead
@@ -310,7 +304,7 @@ func (h *freshenHasher) processEntry(e *freshenEntry) error {
// Add to builder with computed hash // Add to builder with computed hash
err = addFileToBuilder(h.builder, e.path, e.size, e.mtime, e.mode, hash) err = addFileToBuilder(h.builder, e.path, e.size, e.mtime, e.mode, hash)
if err != nil { if err != nil {
return fmt.Errorf("failed to add %s: %w", e.path, err) return fmt.Errorf("add %s: %w", e.path, err)
} }
return nil return nil
@@ -325,7 +319,7 @@ func writeFreshenedManifest(
outFile, err := afs.Create(tmpPath) outFile, err := afs.Create(tmpPath)
if err != nil { if err != nil {
return fmt.Errorf("failed to create temp file: %w", err) return err
} }
err = builder.Build(ctx, outFile) err = builder.Build(ctx, outFile)
@@ -334,7 +328,7 @@ func writeFreshenedManifest(
if err != nil { if err != nil {
_ = afs.Remove(tmpPath) _ = afs.Remove(tmpPath)
return fmt.Errorf("failed to write manifest: %w", err) return fmt.Errorf("build manifest: %w", err)
} }
// Rename temp to final // Rename temp to final
@@ -342,7 +336,7 @@ func writeFreshenedManifest(
if err != nil { if err != nil {
_ = afs.Remove(tmpPath) _ = afs.Remove(tmpPath)
return fmt.Errorf("failed to rename manifest: %w", err) return err
} }
return nil return nil
@@ -408,7 +402,7 @@ func (mfa *CLIApp) freshenScan(
} }
if err != nil { 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 // Remaining entries in existingByPath are removed files
@@ -477,7 +471,7 @@ func (mfa *CLIApp) loadExistingEntries(
Fs: mfa.Fs, Fs: mfa.Fs,
}) })
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to load manifest: %w", err) return nil, fmt.Errorf("load manifest: %w", err)
} }
existingFiles := manifest.Files() existingFiles := manifest.Files()
@@ -503,7 +497,7 @@ func (mfa *CLIApp) freshenManifestOperation(
// Find manifest file // Find manifest file
manifestPath, err := mfa.resolveFreshenManifestPath(cmd) manifestPath, err := mfa.resolveFreshenManifestPath(cmd)
if err != nil { if err != nil {
return fmt.Errorf("freshen: %w", err) return err
} }
//nolint:contextcheck // mfer loads a manifest without a context //nolint:contextcheck // mfer loads a manifest without a context
@@ -514,7 +508,7 @@ func (mfa *CLIApp) freshenManifestOperation(
absBase, err := filepath.Abs(basePath) absBase, err := filepath.Abs(basePath)
if err != nil { if err != nil {
return fmt.Errorf("freshen: invalid base path: %w", err) return fmt.Errorf("invalid base path: %w", err)
} }
// Phase 1: Scan filesystem // Phase 1: Scan filesystem
@@ -606,9 +600,7 @@ func hashFile(r io.Reader, progress func(int64)) ([]byte, int64, error) {
break break
} }
// Returned unwrapped: the caller renders this as // Returned unwrapped: the caller adds the path.
// "failed to hash <path>: <err>" and adding a second layer here
// would change that message.
if err != nil { if err != nil {
return nil, total, err return nil, total, err
} }
+7 -8
View File
@@ -74,7 +74,7 @@ func (mfa *CLIApp) collectInputPaths(args cli.Args) ([]string, error) {
ap, err := filepath.Abs(inputPath) ap, err := filepath.Abs(inputPath)
if err != nil { 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 // Validate path exists before adding to list
if exists, _ := afero.Exists(mfa.Fs, ap); !exists { if exists, _ := afero.Exists(mfa.Fs, ap); !exists {
@@ -129,8 +129,7 @@ func (mfa *CLIApp) enumerateInputs(
// Default to current directory // Default to current directory
err := s.EnumeratePath(".", enumProgress) err := s.EnumeratePath(".", enumProgress)
if err != nil { if err != nil {
return fmt.Errorf( return fmt.Errorf("enumerate current directory: %w", err)
"generate: failed to enumerate current directory: %w", err)
} }
return nil return nil
@@ -144,7 +143,7 @@ func (mfa *CLIApp) enumerateInputs(
err = s.EnumeratePaths(enumProgress, paths...) err = s.EnumeratePaths(enumProgress, paths...)
if err != nil { if err != nil {
return fmt.Errorf("generate: failed to enumerate paths: %w", err) return fmt.Errorf("enumerate files: %w", err)
} }
return nil return nil
@@ -229,7 +228,7 @@ func (mfa *CLIApp) generateManifestOperation(
outFile, err := mfa.Fs.Create(tmpPath) outFile, err := mfa.Fs.Create(tmpPath)
if err != nil { 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 // Set up signal handler to clean up temp file on Ctrl-C
@@ -268,19 +267,19 @@ func (mfa *CLIApp) generateManifestOperation(
scanWg.Wait() scanWg.Wait()
if err != nil { 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 // Close file before rename to ensure all data is flushed
err = outFile.Close() err = outFile.Close()
if err != nil { if err != nil {
return fmt.Errorf("failed to close temp file: %w", err) return err
} }
// Atomic rename // Atomic rename
err = mfa.Fs.Rename(tmpPath, outputPath) err = mfa.Fs.Rename(tmpPath, outputPath)
if err != nil { if err != nil {
return fmt.Errorf("failed to rename temp file: %w", err) return err
} }
success = true success = true
+3 -3
View File
@@ -19,12 +19,12 @@ func (mfa *CLIApp) listManifestOperation(ctx context.Context, cmd *cli.Command)
pathOrURL, err := mfa.resolveManifestArg(cmd) pathOrURL, err := mfa.resolveManifestArg(cmd)
if err != nil { if err != nil {
return fmt.Errorf("list: %w", err) return err
} }
rc, err := mfa.openManifestReader(ctx, pathOrURL) rc, err := mfa.openManifestReader(ctx, pathOrURL)
if err != nil { if err != nil {
return fmt.Errorf("list: %w", err) return err
} }
defer func() { _ = rc.Close() }() 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 //nolint:contextcheck // mfer loads a manifest without a context
manifest, err := mfer.NewManifestFromReader(rc) manifest, err := mfer.NewManifestFromReader(rc)
if err != nil { if err != nil {
return fmt.Errorf("list: failed to parse manifest: %w", err) return fmt.Errorf("parse manifest: %w", err)
} }
files := manifest.Files() files := manifest.Files()
+9 -11
View File
@@ -15,17 +15,13 @@ import (
// manifestFetchTimeout bounds HTTP requests made to fetch a manifest. // manifestFetchTimeout bounds HTTP requests made to fetch a manifest.
const manifestFetchTimeout = 30 * time.Second 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
// its message.
var errHTTPStatus = errors.New("HTTP")
// errManifestTooLarge indicates a manifest download that passed // errManifestTooLarge indicates a manifest download that passed
// CLIApp.maxManifestSize. // CLIApp.maxManifestSize.
var errManifestTooLarge = errors.New("manifest exceeds maximum allowed size") var errManifestTooLarge = errors.New("file exceeds maximum allowed size")
// isHTTPURL returns true if the string starts with http:// or https://. // isHTTPURL returns true if the string starts with http:// or https://.
func isHTTPURL(s string) bool { func isHTTPURL(s string) bool {
@@ -40,20 +36,22 @@ func (mfa *CLIApp) openManifestReader(
if isHTTPURL(pathOrURL) { if isHTTPURL(pathOrURL) {
client := &http.Client{Timeout: manifestFetchTimeout} 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) req, err := http.NewRequestWithContext(ctx, http.MethodGet, pathOrURL, nil)
if err != 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) resp, err := client.Do(req)
if err != nil { 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 { if resp.StatusCode != http.StatusOK {
_ = resp.Body.Close() _ = 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) pathOrURL, errHTTPStatus, resp.StatusCode)
} }
+4 -7
View File
@@ -246,7 +246,7 @@ func (b *Builder) AddFileWithHash(
) error { ) error {
err := ValidatePath(string(path)) err := ValidatePath(string(path))
if err != nil { if err != nil {
return fmt.Errorf("add file: %w", err) return err
} }
if size < 0 { if size < 0 {
@@ -329,22 +329,19 @@ func (b *Builder) Build(ctx context.Context, w io.Writer) error {
// Generate outer wrapper // Generate outer wrapper
err := m.generateOuter(ctx) err := m.generateOuter(ctx)
if err != nil { if err != nil {
return fmt.Errorf("build: generate outer: %w", err) return err
} }
// Generate final output // Generate final output
err = m.generate(ctx) err = m.generate(ctx)
if err != nil { if err != nil {
return fmt.Errorf("build: generate: %w", err) return err
} }
// Write to output // Write to output
_, err = w.Write(m.output.Bytes()) _, 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 // addEntry adds entry to the manifest unless an entry with its path is
+13 -16
View File
@@ -24,13 +24,12 @@ var (
errCompressedHashWrong = errors.New("compressed data hash mismatch") errCompressedHashWrong = errors.New("compressed data hash mismatch")
errSignatureNoPubKey = errors.New("signature present but no public key") errSignatureNoPubKey = errors.New("signature present but no public key")
errDecompressedTooLarge = errors.New("decompressed data exceeds maximum allowed size") 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") errUUIDMismatch = errors.New("outer and inner UUID mismatch")
errInvalidFileFormat = errors.New("invalid file format") 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(
errSignerNotSigningKey = errors.New(
"signer is not the fingerprint of the key that made the signature") "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 // Validate outer UUID before any decompression
err := validateUUID(m.pbOuter.GetUuid()) err := validateUUID(m.pbOuter.GetUuid())
if err != nil { if err != nil {
return fmt.Errorf("outer UUID invalid: %w", err) return fmt.Errorf("outer message: %w", err)
} }
return nil return nil
@@ -73,7 +72,7 @@ func (m *manifest) verifyOuterIntegrity() error {
_, err := h.Write(m.pbOuter.GetInnerMessage()) _, err := h.Write(m.pbOuter.GetInnerMessage())
if err != nil { if err != nil {
return fmt.Errorf("deserialize: hash write: %w", err) return fmt.Errorf("hash inner message: %w", err)
} }
sha256Hash := h.Sum(nil) sha256Hash := h.Sum(nil)
@@ -91,9 +90,7 @@ func (m *manifest) verifyOuterIntegrity() error {
sigString, err := m.signatureString() sigString, err := m.signatureString()
if err != nil { 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. // Loading a manifest takes no context; gpgTimeout still bounds gpg.
@@ -104,7 +101,7 @@ func (m *manifest) verifyOuterIntegrity() error {
m.pbOuter.GetSigningPubKey(), m.pbOuter.GetSigningPubKey(),
) )
if err != nil { if err != nil {
return fmt.Errorf("signature verification failed: %w", err) return err
} }
if !strings.EqualFold(string(m.pbOuter.GetSigner()), signingKey) { if !strings.EqualFold(string(m.pbOuter.GetSigner()), signingKey) {
@@ -135,7 +132,7 @@ func (m *manifest) decompressInner() ([]byte, error) {
zstd.WithDecodeBuffersBelow(0), zstd.WithDecodeBuffersBelow(0),
zstd.WithDecoderMaxWindow(zstdWindowSize)) zstd.WithDecoderMaxWindow(zstdWindowSize))
if err != nil { if err != nil {
return nil, fmt.Errorf("deserialize: zstd reader: %w", err) return nil, fmt.Errorf("create decompressor: %w", err)
} }
defer zr.Close() defer zr.Close()
@@ -150,7 +147,7 @@ func (m *manifest) decompressInner() ([]byte, error) {
dat, err := io.ReadAll(limitedReader) dat, err := io.ReadAll(limitedReader)
if err != nil { 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 { if int64(len(dat)) >= MaxDecompressedSize {
@@ -266,7 +263,7 @@ func (m *manifest) deserializeInner() error {
err = checkDecodedSize(dat) err = checkDecodedSize(dat)
if err != nil { if err != nil {
return fmt.Errorf("deserialize: unmarshal inner: %w", err) return fmt.Errorf("unmarshal inner message: %w", err)
} }
// Deserialize inner message // Deserialize inner message
@@ -275,7 +272,7 @@ func (m *manifest) deserializeInner() error {
// Unknown fields would cost memory; mfer never writes a loaded manifest out. // Unknown fields would cost memory; mfer never writes a loaded manifest out.
err = proto.UnmarshalOptions{DiscardUnknown: true}.Unmarshal(dat, m.pbInner) err = proto.UnmarshalOptions{DiscardUnknown: true}.Unmarshal(dat, m.pbInner)
if err != nil { 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 { if m.pbInner.GetVersion() != MFFile_VERSION_ONE {
@@ -285,7 +282,7 @@ func (m *manifest) deserializeInner() error {
// Validate inner UUID // Validate inner UUID
err = validateUUID(m.pbInner.GetUuid()) err = validateUUID(m.pbInner.GetUuid())
if err != nil { if err != nil {
return fmt.Errorf("inner UUID invalid: %w", err) return fmt.Errorf("inner message: %w", err)
} }
// Verify UUIDs match // Verify UUIDs match
+1 -1
View File
@@ -49,6 +49,6 @@ func TestReadAtMost(t *testing.T) {
_, err = readAtMost(input, maxSize) _, err = readAtMost(input, maxSize)
require.ErrorIs(t, err, errManifestTooLarge) require.ErrorIs(t, err, errManifestTooLarge)
require.EqualError(t, err, 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") assert.Equal(t, maxSize-1, input.Len(), "bytes left unread")
} }
+11 -7
View File
@@ -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() t.Parallel()
m := &manifest{} 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")
} }
+30 -30
View File
@@ -53,7 +53,7 @@ const (
) )
var ( var (
errGPGKeyNotFound = errors.New("gpg key not found") errGPGKeyNotFound = errors.New("GPG key not found")
errFingerprintNotFound = errors.New("fingerprint not found for key") errFingerprintNotFound = errors.New("fingerprint not found for key")
errSigningKeyCount = errors.New( errSigningKeyCount = errors.New(
"embedded public key block must hold exactly one key") "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 // 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. If gpg fails, the error ends
// ctx ends or gpgTimeout passes, whichever comes first. // with what gpg wrote to stderr. gpg is killed when ctx ends or gpgTimeout
// passes, whichever comes first.
func runGPG( func runGPG(
ctx context.Context, stdin io.Reader, args ...string, 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 // exec.CommandContext kills only gpg itself. A gpg-agent that gpg
// starts runs detached and holds none of gpg's output, but another // starts runs detached and holds none of gpg's output, but another
// process gpg leaves behind (a wrapper script that runs the real gpg // process gpg leaves behind (a wrapper script that runs the real gpg
@@ -125,11 +126,16 @@ func runGPG(
// "signal: killed"; return the reason instead. // "signal: killed"; return the reason instead.
err = ctx.Err() err = ctx.Err()
if errors.Is(err, context.DeadlineExceeded) { 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 // parseFingerprint extracts the first fingerprint from gpg --with-colons
@@ -174,7 +180,7 @@ func gpgSign(
) ([]byte, string, error) { ) ([]byte, string, error) {
tmpDir, err := os.MkdirTemp("", "mfer-gpg-sign-*") tmpDir, err := os.MkdirTemp("", "mfer-gpg-sign-*")
if err != nil { if err != nil {
return nil, "", fmt.Errorf("failed to create temp dir: %w", err) return nil, "", err
} }
defer func() { _ = os.RemoveAll(tmpDir) }() 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 // The signature goes to sigFile, so --status-fd 1 can send gpg's status
// lines to stdout; its messages go to stderr. // 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", "--detach-sign",
gpgOptArmor, gpgOptArmor,
"--output", sigFile, "--output", sigFile,
@@ -191,19 +197,19 @@ func gpgSign(
"--local-user", string(keyID), "--local-user", string(keyID),
) )
if err != nil { 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 // The last argument of SIG_CREATED is the fingerprint of the key that
// made the signature. // made the signature.
created, ok := parseStatusLine(stdout.String(), "SIG_CREATED") created, ok := parseStatusLine(stdout.String(), "SIG_CREATED")
if !ok { 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 sig, err := os.ReadFile(sigFile) //nolint:gosec // G304: inside tmpDir, made above
if err != nil { if err != nil {
return nil, "", fmt.Errorf("failed to read signature: %w", err) return nil, "", err
} }
return sig, created[len(created)-1], nil return sig, created[len(created)-1], nil
@@ -212,11 +218,11 @@ func gpgSign(
// gpgExportPublicKey exports the public key for the specified key ID. // gpgExportPublicKey exports the public key for the specified key ID.
// Returns the armored public key. // Returns the armored public key.
func gpgExportPublicKey(ctx context.Context, keyID GPGKeyID) ([]byte, error) { 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))..., gpgArgs([]string{"--export", gpgOptArmor}, string(keyID))...,
) )
if err != nil { 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 { 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. // gpgGetKeyFingerprint gets the full fingerprint for a key ID.
func gpgGetKeyFingerprint(ctx context.Context, keyID GPGKeyID) ([]byte, error) { 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))..., gpgArgs([]string{"--with-colons", "--fingerprint"}, string(keyID))...,
) )
if err != nil { 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()) 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 { func gpgImportOneKey(ctx context.Context, gpgHome, pubKeyFile string) error {
// --status-fd 1 sends gpg's status lines to stdout, which importing // --status-fd 1 sends gpg's status lines to stdout, which importing
// otherwise leaves empty; its messages go to stderr. // 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"}, gpgArgs([]string{gpgOptHomedir, gpgHome, gpgOptStatusFD, "1", "--import"},
pubKeyFile)..., pubKeyFile)...,
) )
if err != nil { if err != nil {
return fmt.Errorf( return fmt.Errorf("gpg import: %w", err)
"failed to import public key: %w: %s", err, importStderr.String(),
)
} }
// The first argument of IMPORT_RES counts the primary keys gpg read // 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 // Create temporary directory for GPG operations
tmpDir, err := os.MkdirTemp("", "mfer-gpg-verify-*") tmpDir, err := os.MkdirTemp("", "mfer-gpg-verify-*")
if err != nil { if err != nil {
return "", fmt.Errorf("failed to create temp dir: %w", err) return "", err
} }
defer func() { _ = os.RemoveAll(tmpDir) }() defer func() { _ = os.RemoveAll(tmpDir) }()
@@ -292,7 +294,7 @@ func gpgVerify(ctx context.Context, data, signature, pubKey []byte) (string, err
// Set restrictive permissions // Set restrictive permissions
err = os.Chmod(tmpDir, privateDirPerms) err = os.Chmod(tmpDir, privateDirPerms)
if err != nil { if err != nil {
return "", fmt.Errorf("failed to set temp dir permissions: %w", err) return "", err
} }
// Write public key to temp file // 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) err = os.WriteFile(pubKeyFile, pubKey, privateFilePerms)
if err != nil { if err != nil {
return "", fmt.Errorf("failed to write public key: %w", err) return "", err
} }
// Write signature to temp file // 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) err = os.WriteFile(sigFile, signature, privateFilePerms)
if err != nil { if err != nil {
return "", fmt.Errorf("failed to write signature: %w", err) return "", err
} }
// Write data to temp file // 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) err = os.WriteFile(dataFile, data, privateFilePerms)
if err != nil { if err != nil {
return "", fmt.Errorf("failed to write data: %w", err) return "", err
} }
err = gpgImportOneKey(ctx, tmpDir, pubKeyFile) 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 // --status-fd 1 sends gpg's status lines to stdout, which verifying a
// detached signature otherwise leaves empty; its messages go to stderr. // 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}, gpgArgs([]string{gpgOptHomedir, tmpDir, gpgOptStatusFD, "1", gpgOptVerify},
sigFile, dataFile)..., sigFile, dataFile)...,
) )
if err != nil { if err != nil {
return "", fmt.Errorf( return "", fmt.Errorf("gpg verify: %w", err)
"signature verification failed: %w: %s", err, verifyStderr.String(),
)
} }
// gpg writes a VALIDSIG line for each good signature. Its first // gpg writes a VALIDSIG line for each good signature. Its first
+4 -3
View File
@@ -451,7 +451,7 @@ func TestManifestRefusesSecondEmbeddedKeyWithoutUserID(t *testing.T) {
t.Setenv("GNUPGHOME", otherHome) t.Setenv("GNUPGHOME", otherHome)
// Keeping only the user IDs that match "nobody" exports none. // Keeping only the user IDs that match "nobody" exports none.
otherPubKey, _, err := runGPG(context.Background(), nil, otherPubKey, err := runGPG(context.Background(), nil,
gpgArgs([]string{ gpgArgs([]string{
"--export", gpgOptArmor, "--export-filter", "keep-uid=uid = nobody", "--export", gpgOptArmor, "--export-filter", "keep-uid=uid = nobody",
}, string(otherKey))...) }, string(otherKey))...)
@@ -603,7 +603,8 @@ func fakeGPGPath(t *testing.T, script string) string {
// TestGPGTimeoutKillsGPG puts a fake gpg that never finishes first on // TestGPGTimeoutKillsGPG puts a fake gpg that never finishes first on
// PATH and checks that a run past its deadline is killed and reported as // 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) { func TestGPGTimeoutKillsGPG(t *testing.T) {
t.Setenv("PATH", fakeGPGPath(t, "#!/bin/sh\nexec sleep 10\n")) 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")) _, _, err := gpgSign(ctx, []byte("data"), GPGKeyID("any"))
require.ErrorIs(t, err, context.DeadlineExceeded) 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 // TestGPGCancelWhenChildHoldsOutput uses a fake gpg that runs sleep as a
+2 -2
View File
@@ -10,7 +10,7 @@ import (
) )
var ( var (
errOuterNotSet = errors.New("pbOuter not set") errOuterNotSet = errors.New("outer message not set")
errUUIDNotSet = errors.New("UUID not set") errUUIDNotSet = errors.New("UUID not set")
errSHA256NotSet = errors.New("SHA256 hash 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) mh, err := multihash.Encode(m.pbOuter.GetSha256(), multihash.SHA2_256)
if err != nil { 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()) uuidStr := hex.EncodeToString(m.pbOuter.GetUuid())
+12 -14
View File
@@ -20,11 +20,9 @@ const MAGIC string = "ZNAVSRFG"
var ( var (
// errInnerNotSet is returned by generate when the inner manifest is // errInnerNotSet is returned by generate when the inner manifest is
// missing. // 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. // 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. // 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) dat, err := proto.MarshalOptions{Deterministic: true}.Marshal(m.pbOuter)
if err != nil { if err != nil {
return fmt.Errorf("serialize: marshal outer: %w", err) return fmt.Errorf("marshal outer message: %w", err)
} }
m.output = bytes.NewBufferString(MAGIC) m.output = bytes.NewBufferString(MAGIC)
_, err = m.output.Write(dat) _, err = m.output.Write(dat)
if err != nil { if err != nil {
return fmt.Errorf("serialize: write output: %w", err) return fmt.Errorf("write outer message: %w", err)
} }
return nil return nil
@@ -95,7 +93,7 @@ func (m *manifest) generateOuter(ctx context.Context) error {
innerData, err := proto.MarshalOptions{Deterministic: true}.Marshal(m.pbInner) innerData, err := proto.MarshalOptions{Deterministic: true}.Marshal(m.pbInner)
if err != nil { if err != nil {
return fmt.Errorf("serialize: marshal inner: %w", err) return fmt.Errorf("marshal inner message: %w", err)
} }
// Compress the inner data // 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)) zw, err := zstd.NewWriter(idc, zstd.WithEncoderLevel(zstd.SpeedBestCompression))
if err != nil { if err != nil {
return fmt.Errorf("serialize: create compressor: %w", err) return fmt.Errorf("create compressor: %w", err)
} }
_, err = zw.Write(innerData) _, err = zw.Write(innerData)
if err != nil { if err != nil {
return fmt.Errorf("serialize: compress: %w", err) return fmt.Errorf("compress inner message: %w", err)
} }
_ = zw.Close() _ = zw.Close()
@@ -120,7 +118,7 @@ func (m *manifest) generateOuter(ctx context.Context) error {
_, err = h.Write(compressedData) _, err = h.Write(compressedData)
if err != nil { if err != nil {
return fmt.Errorf("serialize: hash write: %w", err) return fmt.Errorf("hash inner message: %w", err)
} }
sha256Hash := h.Sum(nil) sha256Hash := h.Sum(nil)
@@ -149,12 +147,12 @@ func (m *manifest) generateOuter(ctx context.Context) error {
func (m *manifest) signOuter(ctx context.Context) error { func (m *manifest) signOuter(ctx context.Context) error {
sigString, err := m.signatureString() sigString, err := m.signatureString()
if err != nil { 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) sig, signingKey, err := gpgSign(ctx, []byte(sigString), m.signingOptions.KeyID)
if err != nil { if err != nil {
return fmt.Errorf("failed to sign manifest: %w", err) return err
} }
m.pbOuter.Signature = sig m.pbOuter.Signature = sig
@@ -163,14 +161,14 @@ func (m *manifest) signOuter(ctx context.Context) error {
// fingerprint first. // fingerprint first.
fingerprint, err := gpgGetKeyFingerprint(ctx, GPGKeyID(signingKey)) fingerprint, err := gpgGetKeyFingerprint(ctx, GPGKeyID(signingKey))
if err != nil { if err != nil {
return fmt.Errorf("failed to get key fingerprint: %w", err) return err
} }
m.pbOuter.Signer = fingerprint m.pbOuter.Signer = fingerprint
pubKey, err := gpgExportPublicKey(ctx, GPGKeyID(fingerprint)) pubKey, err := gpgExportPublicKey(ctx, GPGKeyID(fingerprint))
if err != nil { if err != nil {
return fmt.Errorf("failed to export public key: %w", err) return err
} }
m.pbOuter.SigningPubKey = pubKey m.pbOuter.SigningPubKey = pubKey