Skip to content

Commit

Permalink
chore: prevent file resolver from bubbling errors in binary cataloger (
Browse files Browse the repository at this point in the history
…#3410)

Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com>
Signed-off-by: Keith Zantow <kzantow@gmail.com>
Co-authored-by: Keith Zantow <kzantow@gmail.com>
  • Loading branch information
spiffcs and kzantow authored Nov 4, 2024
1 parent eb56f2e commit 8a41d77
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 6 deletions.
17 changes: 12 additions & 5 deletions internal/task/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"runtime/debug"
"slices"
"sync"
"time"

Expand Down Expand Up @@ -57,14 +58,14 @@ func (p *Executor) Execute(ctx context.Context, resolver file.Resolver, s sbomsy
}

err := runTaskSafely(ctx, tsk, resolver, s)
unknowns, err := unknown.ExtractCoordinateErrors(err)
unknowns, remainingErrors := unknown.ExtractCoordinateErrors(err)
if len(unknowns) > 0 {
appendUnknowns(s, tsk.Name(), unknowns)
}
if err != nil {
if remainingErrors != nil {
withLock(func() {
errs = multierror.Append(errs, fmt.Errorf("failed to run task: %w", err))
prog.SetError(err)
errs = multierror.Append(errs, fmt.Errorf("failed to run task: %w", remainingErrors))
prog.SetError(remainingErrors)
})
}
prog.Increment()
Expand All @@ -84,7 +85,13 @@ func appendUnknowns(builder sbomsync.Builder, taskName string, unknowns []unknow
if sb.Artifacts.Unknowns == nil {
sb.Artifacts.Unknowns = map[file.Coordinates][]string{}
}
sb.Artifacts.Unknowns[u.Coordinates] = append(sb.Artifacts.Unknowns[u.Coordinates], formatUnknown(u.Reason.Error(), taskName))
unknownText := formatUnknown(u.Reason.Error(), taskName)
existing := sb.Artifacts.Unknowns[u.Coordinates]
// don't include duplicate unknowns
if slices.Contains(existing, unknownText) {
continue
}
sb.Artifacts.Unknowns[u.Coordinates] = append(existing, unknownText)
}
})
}
Expand Down
33 changes: 33 additions & 0 deletions internal/unknown/path_error.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package unknown

import (
"regexp"

"github.com/anchore/syft/internal/log"
"github.com/anchore/syft/syft/file"
)

var pathErrorRegex = regexp.MustCompile(`.*path="([^"]+)".*`)

// ProcessPathErrors replaces "path" errors returned from the file.Resolver into unknowns,
// and warn logs non-unknown errors, returning only the unknown errors
func ProcessPathErrors(err error) error {
if err == nil {
return nil
}
errText := err.Error()
if pathErrorRegex.MatchString(errText) {
foundPath := pathErrorRegex.ReplaceAllString(err.Error(), "$1")
if foundPath != "" {
return New(file.NewLocation(foundPath), err)
}
}
unknowns, remainingErrors := ExtractCoordinateErrors(err)
log.Warn(remainingErrors)

var out []error
for _, u := range unknowns {
out = append(out, &u)
}
return Join(out...)
}
56 changes: 56 additions & 0 deletions internal/unknown/path_error_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package unknown

import (
"fmt"
"testing"

"github.com/stretchr/testify/require"

"github.com/anchore/syft/syft/file"
)

func Test_ProcessPathErrors(t *testing.T) {
tests := []struct {
errorText string
expected error
}{
{
errorText: `prefix path="/var/lib/thing" suffix`,
expected: &CoordinateError{
Coordinates: file.Coordinates{
RealPath: "/var/lib/thing",
},
Reason: fmt.Errorf(`prefix path="/var/lib/thing" suffix`),
},
},
{
errorText: `prefix path="/var/lib/thing"`,
expected: &CoordinateError{
Coordinates: file.Coordinates{
RealPath: "/var/lib/thing",
},
Reason: fmt.Errorf(`prefix path="/var/lib/thing"`),
},
},
{
errorText: `path="/var/lib/thing" suffix`,
expected: &CoordinateError{
Coordinates: file.Coordinates{
RealPath: "/var/lib/thing",
},
Reason: fmt.Errorf(`path="/var/lib/thing" suffix`),
},
},
{
errorText: "all your base are belong to us",
expected: nil,
},
}

for _, test := range tests {
t.Run(test.errorText, func(t *testing.T) {
got := ProcessPathErrors(fmt.Errorf("%s", test.errorText))
require.Equal(t, test.expected, got)
})
}
}
1 change: 1 addition & 0 deletions syft/pkg/cataloger/binary/classifier_cataloger.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ func catalog(resolver file.Resolver, cls Classifier) (packages []pkg.Package, er
var errs error
locations, err := resolver.FilesByGlob(cls.FileGlob)
if err != nil {
err = unknown.ProcessPathErrors(err) // convert any file.Resolver path errors to unknowns with locations
return nil, err
}
for _, location := range locations {
Expand Down
2 changes: 1 addition & 1 deletion syft/pkg/cataloger/binary/classifier_cataloger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1668,7 +1668,7 @@ func Test_Cataloger_ResilientToErrors(t *testing.T) {

resolver := &panicyResolver{}
_, _, err := c.Catalog(context.Background(), resolver)
assert.Error(t, err)
assert.Nil(t, err) // non-coordinate-based FindBy* errors are now logged and not returned
assert.True(t, resolver.searchCalled)
}

Expand Down
2 changes: 2 additions & 0 deletions test/cli/test-fixtures/image-unknowns/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
FROM alpine@sha256:c5c5fda71656f28e49ac9c5416b3643eaa6a108a8093151d6d1afc9463be8e33
RUN rm -rf /lib/apk/db/installed
COPY . /home/files
# add a circular reference that will result in a failure while executing FindByGlob:
RUN mkdir -p /etc/alternatives && ln -s /etc/alternatives/java2 /etc/alternatives/java && ln -s /etc/alternatives/java /etc/alternatives/java2
1 change: 1 addition & 0 deletions test/cli/unknowns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ func Test_Unknowns(t *testing.T) {
assertInOutput(`no package identified in executable file`),
assertInOutput(`unable to read files from java archive`),
assertInOutput(`no package identified in archive`),
assertInOutput(`cycle during symlink resolution`),
assertSuccessfulReturnCode,
},
},
Expand Down

0 comments on commit 8a41d77

Please sign in to comment.