feat: capture golang binary lib symbols#4988
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com>
Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com>
Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com>
Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com>
8028f3f to
7050c09
Compare
Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com>
Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com>
Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com>
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
There was a problem hiding this comment.
I was getting warnings when regenerating the json schema, so I went ahead and added descriptions for these.
| case string(SymbolScopeNone), "": | ||
| return SymbolScopeNone | ||
| } | ||
| return "" |
There was a problem hiding this comment.
see the compliance parsing for an example; we tend to fail towards defaults instead of require checking and error by the caller. So when we use it in syft/pkg/cataloger/golang/parse_go_binary, we should trust the value at that point (not call Parse() again)
There was a problem hiding this comment.
Ahh nice call I totally missed that compliance pattern. The options layer's PostLoad just assigns the parsed value (no error path anymore). I think this is the same shape as complianceConfig.PostLoad() now.
| return &goBinaryCataloger{ | ||
| licenseResolver: newGoLicenseResolver(binaryCatalogerName, opts), | ||
| mainModuleVersion: opts.MainModuleVersion, | ||
| symbolScope: opts.CaptureSymbols.Parse(), |
There was a problem hiding this comment.
no need to call Parse() again
There was a problem hiding this comment.
I updated this so the cataloger now takes opts.CaptureSymbols as-is and trusts that normalization happened at the options layer.
| defer func() { | ||
| if r := recover(); r != nil { | ||
| // the gosym package can panic on malformed pclntab data | ||
| err = fmt.Errorf("recovered from panic while reading pclntab: %v", r) |
There was a problem hiding this comment.
we should clear syms at this point too
There was a problem hiding this comment.
👍 I updated this so the recover block now sets syms = nil before setting the error. A panic mid-parse can't return partial results alongside the error.
| // 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, "..") |
There was a problem hiding this comment.
I think this isn't accounting for the go.type case that I think older compiler versions have
There was a problem hiding this comment.
Yea I confirmed pre-1.20 toolchains used . where newer ones use :
I added prefix matches to account for the above. Real module paths like go.uber.org/zap and go.opentelemetry.io/otel also start with go. and shouldn't be dropped. I added test cases to cover this.
| if isCompilerGeneratedName(name) { | ||
| return "" | ||
| } | ||
| slash := strings.LastIndex(name, "/") |
There was a problem hiding this comment.
I think this doesn't account for cases when there are type arguments like foo/bar.Do[net/url.Values] ... we'd find that the package is foo/bar.Do[net
There was a problem hiding this comment.
Right, I found that the last "/" search was landing inside the type arguments
foo/bar.Do[net/url.Values] derived foo/bar.Do[net.
I fixed this by stripping the [...] portion before deriving the package path.
New test cases have been added to symbols_test
There was a problem hiding this comment.
I realize this is outside of review, but noticed it in the review locally... I think this is swapped
| after, _, found := strings.Cut(version, "+") |
There was a problem hiding this comment.
Nice catch! Fixed this apologies for the oversight.
Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com>
Summary
Adds optional extraction of function symbols from Go binaries cataloging. Symbol extraction attributes them to the module that owns them which come out in the SBOM as syft packages. When enabled, syft reads the binary's pclntab and records the function symbols compiled into the binary.
Symbols are attributed to their owning module and on that package's metadata. Standard-library symbols are attached to the synthetic stdlib package. Inlined functions are also recovered from the pclntab funcname table so a vulnerable but inlined function can still be reported.
Changes
GolangBinaryBuildinfoEntry.Symbols(syft/pkg/golang.go): new symbols JSON field (omitempty).Configuration: default off
capture-symbols is a three-value scope enum. Disabled by default, so upgrading does not change existing Go SBOM output or increase its size unless explicitly enabled.