build(deps): bump github.com/google/osv-scalibr from 0.3.4 to 0.4.5 - #6179
build(deps): bump github.com/google/osv-scalibr from 0.3.4 to 0.4.5#6179dependabot[bot] wants to merge 5 commits into
Conversation
dependabot
Bot
added
dependencies
ef68922 to
6a1c5e9
Compare
80adf4f to
74901c5
Compare
74901c5 to
c955573
Compare
c955573 to
4e2b677
Compare
Bumps [github.com/google/osv-scalibr](https://github.com/google/osv-scalibr) from 0.3.4 to 0.4.5. - [Release notes](https://github.com/google/osv-scalibr/releases) - [Commits](google/osv-scalibr@v0.3.4...v0.4.5) --- updated-dependencies: - dependency-name: github.com/google/osv-scalibr dependency-version: 0.4.5 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
4e2b677 to
da5b197
Compare
|
Coverage is 60.908% — dependabot/go_modules/github.com/google/osv-scalibr-0.4.5 into main. No base build found for |
da5b197 to
ca585a0
Compare
| if scanResults.Status.Status == scalibr_plugin.ScanStatusSucceeded { | ||
| return scanResults.Inventory.Packages, nil | ||
| } | ||
|
|
||
| return scanResults.Inventory.Packages, nil | ||
| // Scalibr runs a lot of plugins and aggregates the result. Some of these are picky, and | ||
| // fail for random reasons. Accept partial success, but log the failing plugins. | ||
| if scanResults.Status.Status == scalibr_plugin.ScanStatusPartiallySucceeded { |
There was a problem hiding this comment.
Do you prefer this set of if statements, or a switch like in scalibr.go, line 94?
(Yeah, we need to unify these two code paths or retire one)
|
@JAORMX - can you take a look at this? I had to do some code updates to support the new scalibr evaluation (and ignore a couple scanners that were over-sensitive). |
|
This PR needs additional information before we can continue. It is now marked as stale because it has been open for 30 days with no activity. Please provide the necessary details to continue or it will be closed in 30 days. |
…es/github.com/google/osv-scalibr-0.4.5
|
@dakshhhhh16 or @krrish175-byte Could you take a look at this? I'd like to get scalibr updated to their most recent release in prep for clearing out the pin on OCI v1.2.1 spec. |
| _ = os.RemoveAll(tmpDir) | ||
| }() | ||
| cfg := scalibr_cfg.PluginConfig{ | ||
| MaxFileSizeBytes: 1024 * 1024, |
There was a problem hiding this comment.
Could we avoid setting a global 1 MiB file-size limit here and in internal/engine/ingester/diff/diff.go?
This configuration is passed to every selected plugin, including the package-lock.json extractor, which silently skips valid lockfiles larger than this limit. I reproduced a valid lockfile over 1 MiB returning an empty dependency inventory. Previously, an unset value of 0 meant no limit.
If this cap is intended only for binary extractors, could we configure those plugins specifically and add a large-lockfile regression test?
There was a problem hiding this comment.
Good point! We're trying to avoid blowing up memory during rule evaluation on the Minder server side, so let me bump this to 2MB, and see if we can check the stats for ignored files.
GitHub security alerts apparently only trigger for <0.5MB, so there is precedent for this sort of limit.
| return nil, err | ||
| } | ||
| // unknownbinariesextr uses file extension to determine "binary-ness", and triggers on e.g. .py files | ||
| skipPlugins := []string{"ffa/unknownbinariesextr"} |
There was a problem hiding this comment.
Should go/binary remain in this skip list?
Before this PR, Minder explicitly removed the Go-binary extractor because it can panic on certain files. The v0.4.5 implementation retains the same extraction path, and SCALIBR does not recover extractor panics, so removing that exclusion appears to reintroduce a process-level crash risk.
Could we keep it excluded unless there is an upstream fix we can reference and cover with a regression test?
There was a problem hiding this comment.
It looks like google/osv-scalibr#2316 may have (incidentally) fixed this. I added a regression test.
evankanderson
left a comment
evankanderson
left a comment
There was a problem hiding this comment.
Feedback addressed and test cases added! Thanks for the review!
| _ = os.RemoveAll(tmpDir) | ||
| }() | ||
| cfg := scalibr_cfg.PluginConfig{ | ||
| MaxFileSizeBytes: 1024 * 1024, |
There was a problem hiding this comment.
Good point! We're trying to avoid blowing up memory during rule evaluation on the Minder server side, so let me bump this to 2MB, and see if we can check the stats for ignored files.
GitHub security alerts apparently only trigger for <0.5MB, so there is precedent for this sort of limit.
| return nil, err | ||
| } | ||
| // unknownbinariesextr uses file extension to determine "binary-ness", and triggers on e.g. .py files | ||
| skipPlugins := []string{"ffa/unknownbinariesextr"} |
There was a problem hiding this comment.
It looks like google/osv-scalibr#2316 may have (incidentally) fixed this. I added a regression test.
4 participants
Bumps github.com/google/osv-scalibr from 0.3.4 to 0.4.5.
Release notes
Sourced from github.com/google/osv-scalibr's releases.
... (truncated)
Commits
6510600Bump SCALIBR version in preparation for a new release.9c92834Merge pull request #1870 from cuixq:npmea5d428fix: escape package names in manifest writerc7f2e9bAdd new structured Location fields to protos.352fc38Merge pull request #1855 from cleanstart-community-admin:feat/cleanstart-ecos...6742f9cMerge pull request #1862 from ackama:refactor/remove-unneeded-parenthesesafc5764Merge pull request #1861 from ackama:refactor/use-packagist-directly0bf9e87Merge pull request #1853 from ackama:adjust-version886caf3Merge pull request #1859 from ackama:enrichers-root8bd4660Merge pull request #1858 from ackama:adjust-comment