From 22c9ab1c41afa52f41011fed7809b485aa4813be Mon Sep 17 00:00:00 2001 From: Christopher Phillips <32073428+spiffcs@users.noreply.github.com> Date: Thu, 2 Jul 2026 13:39:37 -0400 Subject: [PATCH] pr: feedback Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com> --- cmd/syft/internal/options/golang.go | 7 +-- cmd/syft/internal/options/golang_test.go | 12 ++-- syft/cataloging/symbols.go | 7 +-- syft/cataloging/symbols_test.go | 6 +- syft/pkg/cataloger/golang/parse_go_binary.go | 2 +- syft/pkg/cataloger/golang/stdlib_package.go | 6 +- syft/pkg/cataloger/golang/symbols.go | 61 ++++++++++++++++++-- syft/pkg/cataloger/golang/symbols_test.go | 12 ++++ 8 files changed, 84 insertions(+), 29 deletions(-) diff --git a/cmd/syft/internal/options/golang.go b/cmd/syft/internal/options/golang.go index 7d13d289d..c811a34c3 100644 --- a/cmd/syft/internal/options/golang.go +++ b/cmd/syft/internal/options/golang.go @@ -1,7 +1,6 @@ package options import ( - "fmt" "strings" "github.com/anchore/clio" @@ -52,11 +51,7 @@ a more accurate version from the binary.`) } func (o *golangConfig) PostLoad() error { - parsed := o.CaptureSymbols.Parse() - if parsed == "" { - return fmt.Errorf("invalid value %q for golang.capture-symbols; valid values are: none, stdlib, all", o.CaptureSymbols) - } - o.CaptureSymbols = parsed + o.CaptureSymbols = o.CaptureSymbols.Parse() return nil } diff --git a/cmd/syft/internal/options/golang_test.go b/cmd/syft/internal/options/golang_test.go index 8a4b7e997..643349904 100644 --- a/cmd/syft/internal/options/golang_test.go +++ b/cmd/syft/internal/options/golang_test.go @@ -31,14 +31,14 @@ func Test_golangConfig_PostLoad(t *testing.T) { expected: cataloging.SymbolScopeNone, }, { - name: "error on invalid value", - cfg: golangConfig{CaptureSymbols: "bogus"}, - wantErr: assert.Error, + name: "invalid value defaults to none", + cfg: golangConfig{CaptureSymbols: "bogus"}, + expected: cataloging.SymbolScopeNone, }, { - name: "boolean spellings are not valid", - cfg: golangConfig{CaptureSymbols: "true"}, - wantErr: assert.Error, + name: "boolean spellings default to none", + cfg: golangConfig{CaptureSymbols: "true"}, + expected: cataloging.SymbolScopeNone, }, } for _, tt := range tests { diff --git a/syft/cataloging/symbols.go b/syft/cataloging/symbols.go index 77f41050c..4e892fcee 100644 --- a/syft/cataloging/symbols.go +++ b/syft/cataloging/symbols.go @@ -16,16 +16,13 @@ const ( SymbolScopeAll SymbolScope = "all" ) -// Parse normalizes a SymbolScope, treating an empty (unset) value as SymbolScopeNone. It returns an empty -// SymbolScope to signal an unrecognized value, which callers validate against. +// Parse normalizes a SymbolScope, treating empty (unset) and unrecognized values as SymbolScopeNone. func (s SymbolScope) Parse() SymbolScope { switch strings.ToLower(strings.TrimSpace(string(s))) { case string(SymbolScopeAll): return SymbolScopeAll case string(SymbolScopeStdlib): return SymbolScopeStdlib - case string(SymbolScopeNone), "": - return SymbolScopeNone } - return "" + return SymbolScopeNone } diff --git a/syft/cataloging/symbols_test.go b/syft/cataloging/symbols_test.go index b610e7d73..307119a15 100644 --- a/syft/cataloging/symbols_test.go +++ b/syft/cataloging/symbols_test.go @@ -18,9 +18,9 @@ func Test_SymbolScope_Parse(t *testing.T) { {"Stdlib", SymbolScopeStdlib}, {"none", SymbolScopeNone}, {"", SymbolScopeNone}, - {"true", ""}, - {"false", ""}, - {"bogus", ""}, + {"true", SymbolScopeNone}, + {"false", SymbolScopeNone}, + {"bogus", SymbolScopeNone}, } for _, test := range tests { t.Run(test.input, func(t *testing.T) { diff --git a/syft/pkg/cataloger/golang/parse_go_binary.go b/syft/pkg/cataloger/golang/parse_go_binary.go index 8031cc65b..8553ec4be 100644 --- a/syft/pkg/cataloger/golang/parse_go_binary.go +++ b/syft/pkg/cataloger/golang/parse_go_binary.go @@ -64,7 +64,7 @@ func newGoBinaryCataloger(opts CatalogerConfig) *goBinaryCataloger { return &goBinaryCataloger{ licenseResolver: newGoLicenseResolver(binaryCatalogerName, opts), mainModuleVersion: opts.MainModuleVersion, - symbolScope: opts.CaptureSymbols.Parse(), + symbolScope: opts.CaptureSymbols, stdlibSymbols: make(map[file.Coordinates][]string), } } diff --git a/syft/pkg/cataloger/golang/stdlib_package.go b/syft/pkg/cataloger/golang/stdlib_package.go index 9205a781e..1743372e5 100644 --- a/syft/pkg/cataloger/golang/stdlib_package.go +++ b/syft/pkg/cataloger/golang/stdlib_package.go @@ -80,10 +80,10 @@ func generateStdlibCpe(version string) (stdlibCpe cpe.CPE, err error) { // we also need to trim starting from the first + to // correctly extract potential rc candidate information for cpe generation - // ex: 2.0.0-rc.1+build.123 -> 2.0.0-rc.1; if no + is found then + is returned - after, _, found := strings.Cut("+", version) + // ex: 2.0.0-rc.1+build.123 -> 2.0.0-rc.1; if no + is found version is unchanged + before, _, found := strings.Cut(version, "+") if found { - version = after + version = before } // extracting and diff --git a/syft/pkg/cataloger/golang/symbols.go b/syft/pkg/cataloger/golang/symbols.go index d2dc244cd..3d553a739 100644 --- a/syft/pkg/cataloger/golang/symbols.go +++ b/syft/pkg/cataloger/golang/symbols.go @@ -31,6 +31,7 @@ func getSymbols(r io.ReaderAt) (syms []binarySymbol, err error) { defer func() { if r := recover(); r != nil { // the gosym package can panic on malformed pclntab data + syms = nil err = fmt.Errorf("recovered from panic while reading pclntab: %v", r) } }() @@ -84,6 +85,7 @@ func packagePathFromSymbolName(name string) string { if isCompilerGeneratedName(name) { return "" } + name = nameWithoutTypeArgs(name) slash := strings.LastIndex(name, "/") dot := strings.IndexByte(name[slash+1:], '.') if dot < 0 { @@ -92,13 +94,62 @@ func packagePathFromSymbolName(name string) string { return name[:slash+1+dot] } +// nameWithoutTypeArgs strips the type-argument portion from an instantiated generic symbol name, e.g. +// "foo/bar.Do[net/url.Values]" -> "foo/bar.Do". The slashes and dots inside the brackets would otherwise +// corrupt package-path derivation (yielding "foo/bar.Do[net" for the example above). Mirrors +// debug/gosym's (*Sym).nameWithoutInst. +func nameWithoutTypeArgs(name string) string { + start := strings.IndexByte(name, '[') + if start < 0 { + return name + } + end := strings.LastIndexByte(name, ']') + if end < 0 { + // malformed: an opening bracket should always have a closing one + return name + } + return name[:start] + name[end+1:] +} + +// oldStyleCompilerGeneratedPrefixes match compiler/linker-generated symbols from toolchains older than +// go1.20, which used "." where newer toolchains use ":" (e.g. "go.buildid" is now "go:buildid", +// "go.type.*" is now "go:type.*"). These must be prefix (not substring) matches, and a bare "go." prefix +// is not enough: legitimate module paths such as "go.uber.org/zap" also start with "go.". +var oldStyleCompilerGeneratedPrefixes = []string{ + "go.buildid", + "go.builtin.", + "go.constinfo.", + "go.cuinfo.", + "go.func.", + "go.importpath.", + "go.info.", + "go.interface.", + "go.itab.", + "go.itablink.", + "go.map.", + "go.shape.", + "go.string.", + "go.type.", + "go.typelink.", + "type.", +} + // isCompilerGeneratedName reports whether a symbol name was synthesized by the compiler or linker rather -// than declared in Go source. These names use ':' or '..' (e.g. "type:.eq.*", "type..hash.*", -// "go:string.*") — byte sequences that never appear in a real Go import path or identifier — so they -// belong to no package and are dropped rather than mis-attributed (e.g. bucketed under a bogus "type" -// stdlib package). +// than declared in Go source. Since go1.20 these names contain ':' or '..' (e.g. "type:.eq.*", +// "go:string.*") — byte sequences that never appear in a real Go import path or identifier. Older +// toolchains used '.' as the separator (e.g. "go.type.*", "type..hash.*"), which is matched against the +// known reserved prefixes. Such names belong to no package and are dropped rather than mis-attributed +// (e.g. bucketed under a bogus "type" stdlib package). func isCompilerGeneratedName(name string) bool { - return strings.Contains(name, ":") || strings.Contains(name, "..") + if strings.Contains(name, ":") || strings.Contains(name, "..") { + return true + } + for _, prefix := range oldStyleCompilerGeneratedPrefixes { + if strings.HasPrefix(name, prefix) { + return true + } + } + return false } // funcNameTable returns every function name recorded in the pclntab's funcname table, including the diff --git a/syft/pkg/cataloger/golang/symbols_test.go b/syft/pkg/cataloger/golang/symbols_test.go index b9569108f..703c3210a 100644 --- a/syft/pkg/cataloger/golang/symbols_test.go +++ b/syft/pkg/cataloger/golang/symbols_test.go @@ -174,12 +174,24 @@ func Test_packagePathFromSymbolName(t *testing.T) { {"github.com/foo/bar.Parse.func1", "github.com/foo/bar"}, {"main.main", "main"}, {"runtime.gcBgMarkWorker", "runtime"}, + // generic instantiations: type arguments must not corrupt the package path + {"foo/bar.Do[net/url.Values]", "foo/bar"}, + {"github.com/foo/bar.Map[go.shape.int,go.shape.string]", "github.com/foo/bar"}, + {"main.Do[go.shape.int]", "main"}, // no package-qualifying dot {"runtime", ""}, // compiler/linker-generated symbols belong to no package {"type:.eq.[]string", ""}, {"type..hash.runtime._type", ""}, {"go:string.\"foo\"", ""}, + // pre-go1.20 toolchains generated symbols with "." where newer ones use ":" + {"go.buildid", ""}, + {"go.type.*runtime._type", ""}, + {"go.itab.*os.File,io.Reader", ""}, + {"go.string.\"foo\"", ""}, + // module paths that begin with "go." are not compiler-generated + {"go.uber.org/zap.(*Logger).Info", "go.uber.org/zap"}, + {"go.opentelemetry.io/otel.Tracer", "go.opentelemetry.io/otel"}, } for _, test := range tests { t.Run(test.name, func(t *testing.T) {