From 901d7bbab2eb32ba46ad57e517e3ba2d54a842ec Mon Sep 17 00:00:00 2001 From: Alex Goodman Date: Wed, 21 Oct 2026 11:28:53 -0400 Subject: [PATCH] fix: don't infer archive nesting from paths in error text `ProcessPathErrors` cut the `path="..."` it pulled out of an error at the first `!/` and built nested coordinates from it, so a host file with `!/` in its name was reported as a file inside an archive. - paths from error text are taken as plain real paths again, as on main - the php extension reader already has the location in hand, so it records the unknown against that instead of re-parsing the error (this is the one caller that could see a nested path). It also no longer defers `Close` on a nil reader when the read fails Signed-off-by: Alex Goodman --- internal/unknown/path_error.go | 15 +++------------ internal/unknown/path_error_test.go | 7 +++++++ syft/pkg/cataloger/php/interpreter_cataloger.go | 4 ++-- .../cataloger/php/interpreter_cataloger_test.go | 16 ++++++++++++++++ 4 files changed, 28 insertions(+), 14 deletions(-) diff --git a/internal/unknown/path_error.go b/internal/unknown/path_error.go index 1c74193dc..05b8cf085 100644 --- a/internal/unknown/path_error.go +++ b/internal/unknown/path_error.go @@ -2,7 +2,6 @@ package unknown import ( "regexp" - "strings" "github.com/anchore/syft/internal/log" "github.com/anchore/syft/syft/file" @@ -20,7 +19,9 @@ func ProcessPathErrors(err error) error { if pathErrorRegex.MatchString(errText) { foundPath := pathErrorRegex.ReplaceAllString(err.Error(), "$1") if foundPath != "" { - return New(file.NewLocationFromCoordinates(pathCoordinates(foundPath)), err) + // a path in an error text is taken as is: nesting is never inferred from "!/" in it, since a real + // path can hold one too. A caller with the location in hand should use New instead + return New(file.NewLocation(foundPath), err) } } unknowns, remainingErrors := ExtractCoordinateErrors(err) @@ -32,13 +33,3 @@ func ProcessPathErrors(err error) error { } return Join(out...) } - -// pathCoordinates names the file at a path from a resolver error, which for a file within an archive is -// a full path: the real path is what precedes the first archive separator. -func pathCoordinates(p string) file.Coordinates { - realPath, _, nested := strings.Cut(p, file.ArchivePathSeparator) - if !nested { - return file.NewCoordinates(p, "") - } - return file.Coordinates{RealPath: realPath, FullPath: p} -} diff --git a/internal/unknown/path_error_test.go b/internal/unknown/path_error_test.go index 53a82c6b2..c6bbe688a 100644 --- a/internal/unknown/path_error_test.go +++ b/internal/unknown/path_error_test.go @@ -35,6 +35,13 @@ func Test_ProcessPathErrors(t *testing.T) { Reason: fmt.Errorf(`path="/var/lib/thing" suffix`), }, }, + { + errorText: `path="/data/Hello!/world.txt"`, + expected: &CoordinateError{ + Coordinates: file.NewCoordinates("/data/Hello!/world.txt", ""), + Reason: fmt.Errorf(`path="/data/Hello!/world.txt"`), + }, + }, { errorText: "all your base are belong to us", expected: nil, diff --git a/syft/pkg/cataloger/php/interpreter_cataloger.go b/syft/pkg/cataloger/php/interpreter_cataloger.go index 4cea616a2..016db66b4 100644 --- a/syft/pkg/cataloger/php/interpreter_cataloger.go +++ b/syft/pkg/cataloger/php/interpreter_cataloger.go @@ -182,10 +182,10 @@ func (p interpreterCataloger) catalogExtensions(resolver file.Resolver) ([]pkg.P func (p interpreterCataloger) catalogExtension(resolver file.Resolver, location file.Location) ([]pkg.Package, error) { reader, err := resolver.FileContentsByLocation(location) - defer internal.CloseAndLogError(reader, location.FullNestedPath()) if err != nil { - return nil, unknown.ProcessPathErrors(err) + return nil, unknown.New(location, err) } + defer internal.CloseAndLogError(reader, location.FullNestedPath()) name, cls := p.getClassifier(location.FullNestedPath()) if name == "" || cls == nil { diff --git a/syft/pkg/cataloger/php/interpreter_cataloger_test.go b/syft/pkg/cataloger/php/interpreter_cataloger_test.go index bff331d5c..5fa5d06e1 100644 --- a/syft/pkg/cataloger/php/interpreter_cataloger_test.go +++ b/syft/pkg/cataloger/php/interpreter_cataloger_test.go @@ -6,7 +6,9 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/anchore/syft/internal/unknown" "github.com/anchore/syft/syft/cpe" + "github.com/anchore/syft/syft/file" "github.com/anchore/syft/syft/pkg/cataloger/internal/pkgtest" ) @@ -164,3 +166,17 @@ func Test_getClassifier_declaredCPESource(t *testing.T) { require.Len(t, cls.CPEs, 1) assert.Equal(t, cpe.DeclaredSource, cls.CPEs[0].Source) } + +func Test_catalogExtension_unreadableFileKeepsItsCoordinates(t *testing.T) { + nested := file.NewLocationFromCoordinates(file.Coordinates{ + RealPath: "/app.tar", + FullPath: "/app.tar!/usr/lib/php83/modules/bcmath.so", + }) + c := NewInterpreterCataloger().(interpreterCataloger) + + _, err := c.catalogExtension(file.NewMockResolverForPaths(), nested) + + var coordinateErr *unknown.CoordinateError + require.ErrorAs(t, err, &coordinateErr) + assert.Equal(t, nested.Coordinates, coordinateErr.Coordinates) +}