Bug/sp 3974 missing GitHub packages #31
Open
+362
−183
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
📝 WalkthroughWalkthroughAdds Git-hosted PURL handling with fallback for OSV lookups, factors OSV HTTP logic into a helper, introduces concurrent worker-pool component version resolution, upgrades go-models and adds go-purl-helper, updates changelog and tests. Changes
Sequence DiagramssequenceDiagram
participant Client as Client
participant Vuln as VulnerabilityUseCase
participant Pool as WorkerPool
participant SCANOSS as SCANOSS
participant OSVService as OSVService
Client->>Vuln: Execute(components)
Vuln->>Pool: getComponentsVersion(components)
loop concurrent workers
Pool->>SCANOSS: query component version
alt version resolved
SCANOSS-->>Pool: resolved version
Pool-->>Vuln: processed component
else error/no match
SCANOSS-->>Pool: no match / error
Pool-->>Vuln: original component
end
end
Vuln->>OSVService: vulnerability lookups using processedComponents
OSVService-->>Vuln: results
Vuln-->>Client: final vulnerabilities
sequenceDiagram
participant Client as Client
participant OSV as OSVUseCase
participant PURL as PURLParser
participant OSVService as OSVService
Client->>OSV: Execute(components)
OSV->>PURL: getRepoURL(originalPurl)
alt repo URL found (Git)
PURL-->>OSV: repoURL
OSV->>OSVService: queryOSV(request with Ecosystem=GIT, repo URL)
OSVService-->>OSV: vulnerabilities
alt no results
OSV->>OSVService: queryOSV(request with original PURL fallback)
OSVService-->>OSV: vulnerabilities
end
else no repo URL
PURL-->>OSV: nil
OSV->>OSVService: queryOSV(request with original PURL)
OSVService-->>OSV: vulnerabilities
end
OSV-->>Client: results (Purl set to OriginalPurl)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@go.mod`:
- Around line 17-18: The go.mod references github.com/scanoss/go-models v0.3.0
which does not exist; update the module line for github.com/scanoss/go-models to
the valid tag v0.2.0 (or confirm and set a newer existing tag if intended) and
then run go tooling to refresh modules (e.g., go get
github.com/scanoss/go-models@v0.2.0 && go mod tidy) to ensure go.sum and
dependencies are consistent; leave github.com/scanoss/go-purl-helper at v0.2.1
as-is unless you intentionally want to change it.
In `@pkg/usecase/OSV_use_case.go`:
- Around line 71-100: In getOSVRequestsFromDTO, the code builds a Git repo URL
from a parsed PURL without verifying purl.Namespace and purl.Name; add guards
after packageurl.FromString to ensure both purl.Namespace and purl.Name are
non-empty (and non-whitespace) before using gitHosts[purl.Type] to construct
repoURL and assign osvRequest.Package = OSVPackageRequest{Name: repoURL,
Ecosystem: "GIT"}; if either field is empty, skip the Git branch so the existing
fallback that sets osvRequest.Package = OSVPackageRequest{Purl: c.Purl} remains
used. Also ensure you reference the same symbols (packageurl.FromString,
gitHosts, osvRequest.Package, OSVPackageRequest) when making the change.
In `@pkg/usecase/vulnerability_use_case.go`:
- Around line 81-107: The getComponentsVersion function can end up with
numWorkers == 0 when config.Source.SCANOSS.MaxWorkers is 0, causing no workers
to spawn and results to be empty; fix this by enforcing at least one worker when
there are components to process (e.g., compute numWorkers using max(1,
min(us.config.Source.SCANOSS.MaxWorkers, numJobs)) or if numJobs>0 &&
numWorkers==0 set numWorkers=1) before starting the worker goroutines in
getComponentsVersion so workers are always spawned when components exist.
🧹 Nitpick comments (1)
pkg/usecase/vulnerability_use_case.go (1)
52-79: Consider emitting the original component on SCANOSS lookup errors.
Right now an error drops the component entirely, which can suppress vulnerability results for that input. If best‑effort behavior is desired, emit the original component before continuing.♻️ Suggested fallback behavior
if err != nil { us.s.Warnf("Failed to get component: %s, %s", j.Purl, j.Requirement) - continue + results <- processedComponent + continue }
agustingroh
force-pushed
the
bug/SP-3974-missing-github-packages
branch
2 times, most recently
from
1443fb3 to
11bd257
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@pkg/usecase/OSV_use_case.go`:
- Around line 99-109: In getRepoURL, avoid using purl before checking
packageurl.FromString's error and eliminate the err shadowing; first call purl,
err := packageurl.FromString(purlString) and immediately return nil on non-nil
err, then access purl.Qualifiers.Map() to get repoURL; when calling
url.QueryUnescape(repoURL) use a differently named variable (e.g., decodedErr
for the error or reuse err after the earlier check) to prevent shadowing and
return nil on unescape error, otherwise return &decoded.
In `@pkg/usecase/vulnerability_use_case.go`:
- Around line 70-73: The worker loop in vulnerability_use_case.go currently logs
and continues when GetComponent returns an error (the block containing
us.s.Warnf("Failed to get component: %s, %s", j.Purl, j.Requirement)), which
skips sending to the results channel and causes processRequests to block waiting
for numJobs results; change the error path so it still sends a result for that
job (for example send the original job j or a result struct containing j with
the existing version/empty vulnerability info and the error flagged) to the
results channel instead of continue, ensuring GetComponent, the results channel,
and processRequests receive a result for every job to avoid deadlock.
🧹 Nitpick comments (1)
pkg/usecase/OSV_use_case_test.go (1)
72-87: Good test coverage, but context setup is unused.The test provides excellent coverage of
getRepoURLwith various PURL formats. However,ctx(line 78) is created but never used sincegetRepoURLdoesn't take a context parameter. Consider removing the unused setup for clarity.♻️ Suggested simplification
func TestGetRepoURL(t *testing.T) { err := zlog.NewSugaredDevLogger() if err != nil { t.Fatalf("an error '%s' was not expected when opening a sugared logger", err) } defer zlog.SyncZap() - ctx := ctxzap.ToContext(context.Background(), zlog.L) - s := ctxzap.Extract(ctx).Sugar() + s := zlog.S serverConfig, err := config.NewServerConfig(nil)
agustingroh
force-pushed
the
bug/SP-3974-missing-github-packages
branch
from
11bd257 to
931289b
Compare
agustingroh
force-pushed
the
bug/SP-3974-missing-github-packages
branch
from
931289b to
ff771b7
Compare
agustingroh
force-pushed
the
bug/SP-3974-missing-github-packages
branch
from
ff771b7 to
af07fa9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@CHANGELOG.md`:
- Around line 12-16: The CHANGELOG entry "## [0.9.0] - 2026/02/02" is dated in
the future; move the three bullet items under an "[Unreleased]" header (or
change the header to "## [0.9.0] - Planned" / add "(planned)" after the date) so
the release notes aren't presented as already released; specifically update the
header line "## [0.9.0] - 2026/02/02" and ensure the three bullets about GitHub
PURLs, concurrent worker pool, and `/scanoss/go-models v0.3.0` remain under the
new "[Unreleased]" or clearly marked planned section.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Changed
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.