fix: a damaged entry truncates the archive instead of failing it

a read error while copying one entry (a zip CRC mismatch, a corrupt deflate stream) was returned from `add`. That aborted the walk, and `Extract` threw away every entry already stored. So a large war with one damaged, irrelevant entry produced zero packages with the feature on, while java's legacy path (which reads only the entries it needs) still cataloged it.

- a read error on one entry now drops that entry and marks the resolver truncated (`entry "x" could not be read: ...`), and the walk continues
- if the stream itself breaks partway (a truncated `.tar.gz`), whatever was read before the break is kept and the archive is truncated. With nothing read it is still an error, as before
- disk limit and decompression budget errors still stop the walk

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

internal/archive/extract.go

@@ -156,6 +156,11 @@ func (r *Resolver) extract(ctx context.Context, format archives.Extractor, conte
 		r.truncate("extraction stopped: the archive and those nested in it decompressed to far more than its size (possible decompression bomb)")
 		return nil
 	}
+	if err != nil && ctx.Err() == nil && len(r.files) > 0 {
+		// the stream broke partway (a truncated .tar.gz); what was read before that is still good
+		r.truncate(fmt.Sprintf("extraction stopped early: %v", err))
+		return nil
+	}
 	return err
 }
 

internal/archive/extract_test.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"io"
 	"io/fs"
+	"math/rand"
 	"os"
 	"path/filepath"
 	"strings"
@@ -344,6 +345,25 @@ func TestExtract_heldContentWrittenToDiskStaysUntilCleanup(t *testing.T) {
 	assert.Zero(t, disk)
 }
 
+func TestExtract_aTruncatedStreamKeepsWhatWasRead(t *testing.T) {
+	// a .tar.gz cut off partway breaks the stream for every entry after the cut; the ones before it are
+	// still good and must not be thrown away
+	noise := make([]byte, 64*1024)
+	rand.New(rand.NewSource(1)).Read(noise)
+	data := tarGzBytes(t, map[string]string{"a.txt": "first", "b.bin": string(noise)})
+	cut := data[:len(data)/2]
+
+	extracted, err := Extract(context.Background(), bytes.NewReader(cut), "", "app.tar.gz", NewLimiter(unboundedLimits), nil)
+	require.NoError(t, err)
+	require.NotNil(t, extracted)
+	t.Cleanup(extracted.Cleanup)
+
+	assert.True(t, extracted.Truncated)
+	locations, err := extracted.FilesByGlob("**")
+	require.NoError(t, err)
+	assert.Equal(t, []string{"a.txt"}, realPaths(locations))
+}
+
 func TestExtract_aPanicStillReleasesEverything(t *testing.T) {
 	// a decoder panicking on hostile bytes must not leak the charge or the spill file
 	ctx, root := scanContext(t)

internal/archive/resolver.go

@@ -167,10 +167,12 @@ func (r *Resolver) add(hdr tar.Header, content io.Reader) error {
 				r.truncate(fmt.Sprintf("an entry larger than %s was skipped", humanize.IBytes(uint64(maxEntryBytes))))
 				return nil
 			}
-			if !errors.Is(err, ErrDiskLimitReached) {
-				return fmt.Errorf("unable to read archive entry %q: %w", hdr.Name, err)
+			if errors.Is(err, ErrDiskLimitReached) || errors.Is(err, errBudgetSpent) {
+				return err
 			}
-			return err
+			// a damaged entry (a CRC mismatch, a corrupt deflate stream) costs only that entry
+			r.truncate(fmt.Sprintf("entry %q could not be read: %v", hdr.Name, err))
+			return nil
 		}
 	}
 

internal/task/archive_tasks_test.go

@@ -221,34 +221,29 @@ func Test_archiveCataloger_truncationStillCatalogs(t *testing.T) {
 	assert.Contains(t, reasons[0], ArchiveCatalogerTaskName, "the unknown names the task that recorded it")
 }
 
-func Test_archiveCataloger_failureIsRecordedAndSkipsSubPipeline(t *testing.T) {
+func Test_archiveCataloger_aDamagedEntryCostsOnlyThatEntry(t *testing.T) {
 	rootDir := t.TempDir()
-	// entry data overwritten but header and end-of-central-directory intact: detected as an archive,
-	// fails when the entry is read
-	corrupt := makeZip(t, map[string][]byte{"data.bin": incompressible(4096)})
+	// entry data overwritten but headers and end-of-central-directory intact: detected as an archive,
+	// and data.bin fails when it is read. ok.txt comes after it and is intact.
+	corrupt := makeZip(t, map[string][]byte{"data.bin": incompressible(4096), "ok.txt": []byte("ok")})
 	require.Greater(t, len(corrupt), 1024)
 	for i := 100; i < 600; i++ {
 		corrupt[i] ^= 0xff
 	}
 	require.NoError(t, os.WriteFile(filepath.Join(rootDir, "corrupt.zip"), corrupt, 0o600))
 
-	goodZip := makeZip(t, map[string][]byte{"ok.txt": []byte("ok")})
-	require.NoError(t, os.WriteFile(filepath.Join(rootDir, "good.zip"), goodZip, 0o600))
+	var seen []string
+	fileCounts := map[string]int{}
+	tsk := newTestTask(t, cataloging.DefaultArchiveSearchConfig().WithMaxDepth(1), capturingTask(t, &seen, &fileCounts))
 
-	var ran int
-	tsk := newTestTask(t, cataloging.DefaultArchiveSearchConfig().WithMaxDepth(1), countingTask(&ran))
+	s := newTestSBOM()
+	require.NoError(t, tsk.Execute(context.Background(), dirTestResolver{dir: rootDir}, sbomsync.NewBuilder(s)))
 
-	err := tsk.Execute(context.Background(), dirTestResolver{dir: rootDir}, sbomsync.NewBuilder(newTestSBOM()))
-
-	// a coordinate error is what executor.go pulls out into sbom.Artifacts.Unknowns; a plain error would
-	// fail the scan instead
-	require.Error(t, err)
-	coordErrs, remaining := unknown.ExtractCoordinateErrors(err)
-	assert.NoError(t, remaining, "nothing may escape as a non-coordinate error")
-	require.Len(t, coordErrs, 1)
-	assert.Equal(t, "/corrupt.zip", coordErrs[0].Coordinates.RealPath)
-
-	assert.Equal(t, 1, ran, "the sub-pipeline must still run for the archive that extracted cleanly")
+	assert.Equal(t, []string{"/corrupt.zip"}, seen, "the archive is still cataloged")
+	assert.Equal(t, 1, fileCounts["/corrupt.zip"], "from the entries that could be read")
+	reasons := s.Artifacts.Unknowns[file.Coordinates{RealPath: "/corrupt.zip"}]
+	require.Len(t, reasons, 1)
+	assert.Contains(t, reasons[0], `entry "data.bin" could not be read`)
 }
 
 func Test_NewArchiveCatalogerTask_gating(t *testing.T) {