Make error message wording consistent (closes #165) #176

Merged
clawbot merged 1 commits from issue-165-error-wording into next 2026-10-07 17:25:42 +02:00
18 changed files with 289 additions and 175 deletions
Showing only changes of commit 437c158fec - Show all commits
+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")
} }
+104 -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,108 @@ 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, "/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) { 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)
}) })
+15 -28
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,26 +288,21 @@ 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 err
} }
h.hashedBytes += bytesRead h.hashedBytes += bytesRead
h.hashedFiles++ 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 // writeFreshenedManifest writes the manifest atomically (write to a
@@ -325,7 +314,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 +323,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 +331,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 +397,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 +466,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 +492,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 +503,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 +595,7 @@ func hashFile(r io.Reader, progress func(int64)) ([]byte, int64, error) {
break 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 { 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")
} }
+37 -27
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,8 +88,9 @@ 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 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( 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, *bytes.Buffer, error) {
@@ -125,13 +126,28 @@ 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)
} }
} }
if err != nil {
err = withStderr(err, &stderr)
}
return &stdout, &stderr, err 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 // parseFingerprint extracts the first fingerprint from gpg --with-colons
// output, or returns ok=false if none is present. // output, or returns ok=false if none is present.
func parseFingerprint(colonOutput string) (string, bool) { func parseFingerprint(colonOutput string) (string, bool) {
@@ -174,7 +190,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) }()
@@ -191,19 +207,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, "", withStderr(errSigningKeyNotReported, stderr)
} }
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 +228,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 +244,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 +264,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 +296,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 +304,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 +312,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 +320,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 +328,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 +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 // --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
+28 -2
View File
@@ -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,32 @@ 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")
}
// 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 // 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