From e7d7f6f626fadbbbd01afe3d66ae9f7c68fca197 Mon Sep 17 00:00:00 2001 From: rust Date: Wed, 29 Apr 2026 12:08:14 -0400 Subject: [PATCH 1/3] fix nested archive handler error wraps to preserve errors.Is identity The non-archive content paths in pkg/handlers/ar.go:109 and pkg/handlers/rpm.go:120 wrapped the inner error returned from handleNonArchiveContent with %v, dropping its identity from the errors.Is chain. Because handleNonArchiveContent can return ctx.Err() (context.Canceled or context.DeadlineExceeded) when its CancellableWrite fails, isFatal at handlers.go:425 then misclassified those cancelled or deadline-exceeded conditions as non-fatal warnings and continued processing. Switch both wraps to %w so the inner cause is preserved alongside ErrProcessingWarning, matching the same-shape fix already applied to the canonical wrap at default.go:137. Add regression tests for both handlers that exercise the production wrap path via an injected chunk reader that cancels the parent context before its chunk is written, which forces handleNonArchiveContent to return context.Canceled and processARFiles / processRPMFiles to wrap it. The tests assert the outer ErrProcessingWarning wrap is preserved, the inner cancellation cause is inspectable via errors.Is, and isFatal classifies the result as fatal. --- pkg/handlers/ar.go | 2 +- pkg/handlers/ar_test.go | 65 ++++++++++++++++++++++++++++++++++++++++ pkg/handlers/rpm.go | 2 +- pkg/handlers/rpm_test.go | 57 +++++++++++++++++++++++++++++++++++ 4 files changed, 124 insertions(+), 2 deletions(-) diff --git a/pkg/handlers/ar.go b/pkg/handlers/ar.go index 3f037887e52f..c25d16be6eeb 100644 --- a/pkg/handlers/ar.go +++ b/pkg/handlers/ar.go @@ -106,7 +106,7 @@ func (h *arHandler) processARFiles(ctx logContext.Context, reader *deb.Ar, dataO if err := h.handleNonArchiveContent(fileCtx, rdr, dataOrErrChan); err != nil { dataOrErrChan <- DataOrErr{ - Err: fmt.Errorf("%w: error handling archive content in AR: %v", ErrProcessingWarning, err), + Err: fmt.Errorf("%w: error handling archive content in AR: %w", ErrProcessingWarning, err), } h.metrics.incErrors() continue diff --git a/pkg/handlers/ar_test.go b/pkg/handlers/ar_test.go index 2f908e4d7c64..5649460ebc2d 100644 --- a/pkg/handlers/ar_test.go +++ b/pkg/handlers/ar_test.go @@ -1,13 +1,18 @@ package handlers import ( + stdctx "context" + "errors" + "io" "os" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/trufflesecurity/trufflehog/v3/pkg/context" + "github.com/trufflesecurity/trufflehog/v3/pkg/sources" ) func TestHandleARFile(t *testing.T) { @@ -34,3 +39,63 @@ func TestHandleARFile(t *testing.T) { assert.Equal(t, wantChunkCount, count) } + +// TestARHandler_NonArchiveErrPreservesIdentity is a regression test for the +// %v wrap at ar.go:109. Before the fix, wrapping handleNonArchiveContent's +// return value with %v dropped the inner error's identity from the errors.Is +// chain, causing isFatal in handleChunksWithError to misclassify cancellation +// (and other fatal causes) as a non-fatal warning. The test injects a +// cancelling chunk reader so handleNonArchiveContent's CancellableWrite +// returns context.Canceled, which is then wrapped by processARFiles. It +// asserts both that the outer ErrProcessingWarning wrap is preserved and +// that the inner cancellation cause remains inspectable. +func TestARHandler_NonArchiveErrPreservesIdentity(t *testing.T) { + file, err := os.Open("testdata/test.deb") + require.NoError(t, err) + defer file.Close() + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + rdr, err := newFileReader(ctx, file) + require.NoError(t, err) + defer rdr.Close() + + // Cancel the parent context the moment handleNonArchiveContent asks for + // chunks, then deliver a single non-error chunk. CancellableWrite of that + // chunk's data sees the cancelled context and returns context.Canceled, + // which handleNonArchiveContent returns and processARFiles wraps. This + // is the only path that exercises the ar.go:107-110 wrap. + cancellingChunkReader := sources.ChunkReader(func(_ context.Context, _ io.Reader) <-chan sources.ChunkResult { + ch := make(chan sources.ChunkResult, 1) + cancel() + ch <- sources.NewChunkResult([]byte("data"), 4) + close(ch) + return ch + }) + + handler := &arHandler{ + defaultHandler: newDefaultHandler(arHandlerType, withChunkReader(cancellingChunkReader)), + } + + var got []DataOrErr + for d := range handler.HandleFile(context.AddLogger(ctx), rdr) { + got = append(got, d) + } + + var warnErr error + for _, d := range got { + if d.Err != nil && errors.Is(d.Err, ErrProcessingWarning) { + warnErr = d.Err + break + } + } + require.Error(t, warnErr, "expected wrapped warning from ar.go non-archive error path") + + assert.True(t, errors.Is(warnErr, ErrProcessingWarning), + "outer ErrProcessingWarning wrap should be preserved") + assert.True(t, errors.Is(warnErr, stdctx.Canceled), + "inner cancellation cause should be inspectable via errors.Is") + assert.True(t, isFatal(warnErr), + "isFatal should classify the wrapped error based on the inner cause") +} diff --git a/pkg/handlers/rpm.go b/pkg/handlers/rpm.go index 3450b8749f98..674fb55bcef9 100644 --- a/pkg/handlers/rpm.go +++ b/pkg/handlers/rpm.go @@ -117,7 +117,7 @@ func (h *rpmHandler) processRPMFiles( if err := h.handleNonArchiveContent(fileCtx, rdr, dataOrErrChan); err != nil { dataOrErrChan <- DataOrErr{ - Err: fmt.Errorf("%w: error processing RPM archive: %v", ErrProcessingWarning, err), + Err: fmt.Errorf("%w: error processing RPM archive: %w", ErrProcessingWarning, err), } h.metrics.incErrors() } diff --git a/pkg/handlers/rpm_test.go b/pkg/handlers/rpm_test.go index ba4d5d622369..9faa5053b88b 100644 --- a/pkg/handlers/rpm_test.go +++ b/pkg/handlers/rpm_test.go @@ -1,13 +1,18 @@ package handlers import ( + stdctx "context" + "errors" + "io" "os" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/trufflesecurity/trufflehog/v3/pkg/context" + "github.com/trufflesecurity/trufflehog/v3/pkg/sources" ) func TestHandleRPMFile(t *testing.T) { @@ -34,3 +39,55 @@ func TestHandleRPMFile(t *testing.T) { assert.Equal(t, wantChunkCount, count) } + +// TestRPMHandler_NonArchiveErrPreservesIdentity is a regression test for the +// %v wrap at rpm.go:120. Same shape as TestARHandler_NonArchiveErrPreservesIdentity: +// inject a cancelling chunk reader so handleNonArchiveContent's CancellableWrite +// returns context.Canceled, which processRPMFiles wraps with ErrProcessingWarning. +// Asserts the outer warning wrap and inner cancellation cause are both +// observable via errors.Is so isFatal can classify correctly. +func TestRPMHandler_NonArchiveErrPreservesIdentity(t *testing.T) { + file, err := os.Open("testdata/test.rpm") + require.NoError(t, err) + defer file.Close() + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + rdr, err := newFileReader(ctx, file) + require.NoError(t, err) + defer rdr.Close() + + cancellingChunkReader := sources.ChunkReader(func(_ context.Context, _ io.Reader) <-chan sources.ChunkResult { + ch := make(chan sources.ChunkResult, 1) + cancel() + ch <- sources.NewChunkResult([]byte("data"), 4) + close(ch) + return ch + }) + + handler := &rpmHandler{ + defaultHandler: newDefaultHandler(rpmHandlerType, withChunkReader(cancellingChunkReader)), + } + + var got []DataOrErr + for d := range handler.HandleFile(context.AddLogger(ctx), rdr) { + got = append(got, d) + } + + var warnErr error + for _, d := range got { + if d.Err != nil && errors.Is(d.Err, ErrProcessingWarning) { + warnErr = d.Err + break + } + } + require.Error(t, warnErr, "expected wrapped warning from rpm.go non-archive error path") + + assert.True(t, errors.Is(warnErr, ErrProcessingWarning), + "outer ErrProcessingWarning wrap should be preserved") + assert.True(t, errors.Is(warnErr, stdctx.Canceled), + "inner cancellation cause should be inspectable via errors.Is") + assert.True(t, isFatal(warnErr), + "isFatal should classify the wrapped error based on the inner cause") +} From 4a0369e974aef4547253ffd408f7bb2c54922c18 Mon Sep 17 00:00:00 2001 From: rust Date: Wed, 29 Apr 2026 13:56:30 -0400 Subject: [PATCH 2/3] test: address review on PR #4931 - Drop stdctx alias on the stdlib context import; alias trufflehog's pkg/context as trContext to match the package idiom. - Replace assert.True(t, errors.Is(...)) with assert.ErrorIs to get better diagnostics on failure. --- pkg/handlers/ar_test.go | 21 ++++++++++----------- pkg/handlers/rpm_test.go | 21 ++++++++++----------- 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/pkg/handlers/ar_test.go b/pkg/handlers/ar_test.go index 5649460ebc2d..bf18d0891863 100644 --- a/pkg/handlers/ar_test.go +++ b/pkg/handlers/ar_test.go @@ -1,8 +1,7 @@ package handlers import ( - stdctx "context" - "errors" + "context" "io" "os" "testing" @@ -11,7 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/trufflesecurity/trufflehog/v3/pkg/context" + trContext "github.com/trufflesecurity/trufflehog/v3/pkg/context" "github.com/trufflesecurity/trufflehog/v3/pkg/sources" ) @@ -20,7 +19,7 @@ func TestHandleARFile(t *testing.T) { assert.Nil(t, err) defer file.Close() - ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + ctx, cancel := trContext.WithTimeout(trContext.Background(), 3*time.Second) defer cancel() rdr, err := newFileReader(ctx, file) @@ -28,7 +27,7 @@ func TestHandleARFile(t *testing.T) { defer rdr.Close() handler := newARHandler() - dataOrErrChan := handler.HandleFile(context.AddLogger(ctx), rdr) + dataOrErrChan := handler.HandleFile(trContext.AddLogger(ctx), rdr) assert.NoError(t, err) wantChunkCount := 102 @@ -54,7 +53,7 @@ func TestARHandler_NonArchiveErrPreservesIdentity(t *testing.T) { require.NoError(t, err) defer file.Close() - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := trContext.WithCancel(trContext.Background()) defer cancel() rdr, err := newFileReader(ctx, file) @@ -66,7 +65,7 @@ func TestARHandler_NonArchiveErrPreservesIdentity(t *testing.T) { // chunk's data sees the cancelled context and returns context.Canceled, // which handleNonArchiveContent returns and processARFiles wraps. This // is the only path that exercises the ar.go:107-110 wrap. - cancellingChunkReader := sources.ChunkReader(func(_ context.Context, _ io.Reader) <-chan sources.ChunkResult { + cancellingChunkReader := sources.ChunkReader(func(_ trContext.Context, _ io.Reader) <-chan sources.ChunkResult { ch := make(chan sources.ChunkResult, 1) cancel() ch <- sources.NewChunkResult([]byte("data"), 4) @@ -79,22 +78,22 @@ func TestARHandler_NonArchiveErrPreservesIdentity(t *testing.T) { } var got []DataOrErr - for d := range handler.HandleFile(context.AddLogger(ctx), rdr) { + for d := range handler.HandleFile(trContext.AddLogger(ctx), rdr) { got = append(got, d) } var warnErr error for _, d := range got { - if d.Err != nil && errors.Is(d.Err, ErrProcessingWarning) { + if d.Err != nil { warnErr = d.Err break } } require.Error(t, warnErr, "expected wrapped warning from ar.go non-archive error path") - assert.True(t, errors.Is(warnErr, ErrProcessingWarning), + assert.ErrorIs(t, warnErr, ErrProcessingWarning, "outer ErrProcessingWarning wrap should be preserved") - assert.True(t, errors.Is(warnErr, stdctx.Canceled), + assert.ErrorIs(t, warnErr, context.Canceled, "inner cancellation cause should be inspectable via errors.Is") assert.True(t, isFatal(warnErr), "isFatal should classify the wrapped error based on the inner cause") diff --git a/pkg/handlers/rpm_test.go b/pkg/handlers/rpm_test.go index 9faa5053b88b..4c3d19dc4830 100644 --- a/pkg/handlers/rpm_test.go +++ b/pkg/handlers/rpm_test.go @@ -1,8 +1,7 @@ package handlers import ( - stdctx "context" - "errors" + "context" "io" "os" "testing" @@ -11,7 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/trufflesecurity/trufflehog/v3/pkg/context" + trContext "github.com/trufflesecurity/trufflehog/v3/pkg/context" "github.com/trufflesecurity/trufflehog/v3/pkg/sources" ) @@ -20,7 +19,7 @@ func TestHandleRPMFile(t *testing.T) { assert.Nil(t, err) defer file.Close() - ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + ctx, cancel := trContext.WithTimeout(trContext.Background(), 3*time.Second) defer cancel() rdr, err := newFileReader(ctx, file) @@ -28,7 +27,7 @@ func TestHandleRPMFile(t *testing.T) { defer rdr.Close() handler := newRPMHandler() - dataOrErrChan := handler.HandleFile(context.AddLogger(ctx), rdr) + dataOrErrChan := handler.HandleFile(trContext.AddLogger(ctx), rdr) assert.NoError(t, err) wantChunkCount := 179 @@ -51,14 +50,14 @@ func TestRPMHandler_NonArchiveErrPreservesIdentity(t *testing.T) { require.NoError(t, err) defer file.Close() - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := trContext.WithCancel(trContext.Background()) defer cancel() rdr, err := newFileReader(ctx, file) require.NoError(t, err) defer rdr.Close() - cancellingChunkReader := sources.ChunkReader(func(_ context.Context, _ io.Reader) <-chan sources.ChunkResult { + cancellingChunkReader := sources.ChunkReader(func(_ trContext.Context, _ io.Reader) <-chan sources.ChunkResult { ch := make(chan sources.ChunkResult, 1) cancel() ch <- sources.NewChunkResult([]byte("data"), 4) @@ -71,22 +70,22 @@ func TestRPMHandler_NonArchiveErrPreservesIdentity(t *testing.T) { } var got []DataOrErr - for d := range handler.HandleFile(context.AddLogger(ctx), rdr) { + for d := range handler.HandleFile(trContext.AddLogger(ctx), rdr) { got = append(got, d) } var warnErr error for _, d := range got { - if d.Err != nil && errors.Is(d.Err, ErrProcessingWarning) { + if d.Err != nil { warnErr = d.Err break } } require.Error(t, warnErr, "expected wrapped warning from rpm.go non-archive error path") - assert.True(t, errors.Is(warnErr, ErrProcessingWarning), + assert.ErrorIs(t, warnErr, ErrProcessingWarning, "outer ErrProcessingWarning wrap should be preserved") - assert.True(t, errors.Is(warnErr, stdctx.Canceled), + assert.ErrorIs(t, warnErr, context.Canceled, "inner cancellation cause should be inspectable via errors.Is") assert.True(t, isFatal(warnErr), "isFatal should classify the wrapped error based on the inner cause") From 7b66333863d2bf71eef63b2b124188ec6cac91de Mon Sep 17 00:00:00 2001 From: mayor Date: Wed, 29 Apr 2026 14:50:21 -0400 Subject: [PATCH 3/3] test: replace stale line numbers in test docstrings with symbolic refs Address review nit on PR #4931: line numbers in regression-test docstrings will go stale as the file evolves. Reference the symbolic shape instead (dataOrErrChan ErrProcessingWarning send in processARFiles/processRPMFiles' handleNonArchiveContent error path). --- pkg/handlers/ar_test.go | 7 +++++-- pkg/handlers/rpm_test.go | 4 +++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/pkg/handlers/ar_test.go b/pkg/handlers/ar_test.go index bf18d0891863..65b10c4987b3 100644 --- a/pkg/handlers/ar_test.go +++ b/pkg/handlers/ar_test.go @@ -40,7 +40,9 @@ func TestHandleARFile(t *testing.T) { } // TestARHandler_NonArchiveErrPreservesIdentity is a regression test for the -// %v wrap at ar.go:109. Before the fix, wrapping handleNonArchiveContent's +// %v wrap on the ErrProcessingWarning send to dataOrErrChan in +// processARFiles' handleNonArchiveContent error path. Before the fix, wrapping +// handleNonArchiveContent's // return value with %v dropped the inner error's identity from the errors.Is // chain, causing isFatal in handleChunksWithError to misclassify cancellation // (and other fatal causes) as a non-fatal warning. The test injects a @@ -64,7 +66,8 @@ func TestARHandler_NonArchiveErrPreservesIdentity(t *testing.T) { // chunks, then deliver a single non-error chunk. CancellableWrite of that // chunk's data sees the cancelled context and returns context.Canceled, // which handleNonArchiveContent returns and processARFiles wraps. This - // is the only path that exercises the ar.go:107-110 wrap. + // is the only path that exercises the processARFiles dataOrErrChan + // ErrProcessingWarning wrap. cancellingChunkReader := sources.ChunkReader(func(_ trContext.Context, _ io.Reader) <-chan sources.ChunkResult { ch := make(chan sources.ChunkResult, 1) cancel() diff --git a/pkg/handlers/rpm_test.go b/pkg/handlers/rpm_test.go index 4c3d19dc4830..d9ee608b4d6c 100644 --- a/pkg/handlers/rpm_test.go +++ b/pkg/handlers/rpm_test.go @@ -40,7 +40,9 @@ func TestHandleRPMFile(t *testing.T) { } // TestRPMHandler_NonArchiveErrPreservesIdentity is a regression test for the -// %v wrap at rpm.go:120. Same shape as TestARHandler_NonArchiveErrPreservesIdentity: +// %v wrap on the ErrProcessingWarning send to dataOrErrChan in +// processRPMFiles' handleNonArchiveContent error path. Same shape as +// TestARHandler_NonArchiveErrPreservesIdentity: // inject a cancelling chunk reader so handleNonArchiveContent's CancellableWrite // returns context.Canceled, which processRPMFiles wraps with ErrProcessingWarning. // Asserts the outer warning wrap and inner cancellation cause are both