Chore/sp 4308 add info message and info code vulnerability response#34
Merged
agustingroh merged 4 commits intoApr 17, 2026
Conversation
agustingroh
requested review from
isasmendiagus,
mscasso-scanoss and
scanoss-qg
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 12 minutes and 12 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (21)
📝 WalkthroughWalkthroughThis PR upgrades the Go toolchain from 1.24.0 to 1.25.0, updates dependencies including linter tools (golangci-lint v3→v9, actions/setup-go v3→v5), restructures linting configuration to v2 format, adds package-level documentation, and refactors status code handling by replacing Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pkg/helpers/vulnerabilty_helper.go (1)
43-43:⚠️ Potential issue | 🟡 MinorReplace
ComponentWithoutInfowithNoInfoto complete the refactoring.The codebase uses
NoInfoacross multiple files (6 usages in vulnerability_use_case.go, cpe.go, and OSV_use_case.go), but this line still uses the legacyComponentWithoutInfo. Update line 43 to usedomain.NoInfofor consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/helpers/vulnerabilty_helper.go` at line 43, Replace the legacy enum value domain.ComponentWithoutInfo with the new refactored constant domain.NoInfo in the StatusCode assignment inside the helper that sets component vulnerability info (look for StatusCode: domain.ComponentWithoutInfo in the function in vulnerabilty_helper.go) so it matches other usages (vulnerability_use_case.go, cpe.go, OSV_use_case.go); update that single reference to domain.NoInfo and run tests/compile to confirm consistency.pkg/adapters/vulnerability_support.go (1)
150-159:⚠️ Potential issue | 🟡 MinorSuccessful components will emit
info_code="Success"with an emptyinfo_message.The guard
status.StatusCode != ""will pass fordomain.SuccesssinceStatusCodeis defined astype StatusCode stringwithconst Success StatusCode = "Success", which is non-empty. For successful components,info.InfoCodewill be set to"Success"andinfo.InfoMessageto a pointer to an empty string. The comment states "Set error code and message," but the fields are nowInfoCode/InfoMessage, suggesting the logic may not have been updated alongside the naming change. Consider whether clients expect aSuccessinfo code on every successful component response, or if the Success case should be skipped explicitly.♻️ Suggested gate
- if status, ok := statusByKey[key]; ok && status.StatusCode != "" { + if status, ok := statusByKey[key]; ok && status.StatusCode != "" && status.StatusCode != domain.Success { code := status.StatusCode.String() message := status.Message info.InfoCode = &code info.InfoMessage = &message }Also applies to: 199-208
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/adapters/vulnerability_support.go` around lines 150 - 159, The loop that sets info.InfoCode/info.InfoMessage for each entry (the componentsInfo iteration using key := info.Purl + info.Requirement + info.Version and lookup in statusByKey) currently treats any non-empty status.StatusCode (including domain.Success) the same and assigns an InfoMessage pointer to an empty string; change the logic to explicitly skip or treat the Success case differently: detect domain.Success (status.StatusCode == domain.Success) and either do not set InfoCode/InfoMessage at all or set InfoMessage to nil while leaving InfoCode unset, so successful components do not emit info_code="Success" with an empty info_message. Ensure the check is updated where similar logic appears (the other block around the same pattern) to avoid regressing the behavior.
🧹 Nitpick comments (4)
pkg/usecase/OSV_use_case.go (1)
269-269: Misleadingnolint:gosecjustification.The comment states the URL is built from a trusted PURL type/namespace/name, but the actual request URL here is
us.OSVAPIBaseURL+"/query"— it's derived from server configuration, not from PURL data. The PURL-derived value is only placed in the request body. Consider rewording to accurately reflect why G107 is being suppressed (e.g., "URL base is sourced from trusted server configuration, not user input").♻️ Proposed wording
- resp, err := us.client.Do(req) //nolint:gosec // G107: URL is built from a trusted, validated PURL type/namespace/name, not user input + resp, err := us.client.Do(req) //nolint:gosec // G107: OSVAPIBaseURL is sourced from trusted server configuration, not user input🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/usecase/OSV_use_case.go` at line 269, The nolint justification on the us.client.Do(req) call is misleading because the request URL is constructed from us.OSVAPIBaseURL+"/query" (server configuration) not PURL data; update the inline nolint comment to accurately state that the URL base is sourced from trusted server configuration (not user input) and that any PURL-derived data is only included in the request body—e.g., replace the existing comment on the us.client.Do(req) line with a concise explanation like "URL base is sourced from trusted server configuration, not user input" while keeping the nolint:gosec suppression..github/workflows/golangci-lint.yml (2)
15-15: Consider also bumpingactions/checkout.While the PR updates
actions/setup-goto v5 andgolangci-lint-actionto v9,actions/checkout@v3is still on an older major (v4/v5 are current). Non-blocking, but aligning the upgrade keeps Node.js runtime requirements consistent on GitHub runners.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/golangci-lint.yml at line 15, Update the checkout action to a current major version: replace the uses entry referencing actions/checkout@v3 with a newer supported release (e.g., actions/checkout@v4 or actions/checkout@v5) so the workflow aligns with the other updated actions (refer to the existing uses: actions/checkout@v3 line) and ensure the workflow YAML still passes linting and any runtime compatibility checks after the change.
28-30: Pingolangci-lintversion to match the Makefile.Without an explicit
version:input, golangci/golangci-lint-action@v9 will use the latest version of golangci-lint, which can differ from thev2.10.1pinned in the Makefile'slint_dockertarget. This creates a risk of local-vs-CI discrepancies when new golangci-lint releases introduce new lints or change defaults.Suggested fix
- name: golangci-lint uses: golangci/golangci-lint-action@v9 with: + version: v2.10.1 args: --timeout=5m🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/golangci-lint.yml around lines 28 - 30, The GitHub Actions step using golangci/golangci-lint-action@v9 should be pinned to the same golangci-lint release used in the Makefile (v2.10.1) to avoid CI vs local mismatches; update the action step that references golangci/golangci-lint-action@v9 and add the version: v2.10.1 input (alongside the existing args: --timeout=5m) so the action runs that specific golangci-lint release and matches the Makefile's lint_docker target.Makefile (1)
3-3: Consider pinning CI to the same linter version.
LINTER_VERSION := v2.10.1is great for local reproducibility, but.github/workflows/golangci-lint.ymldoesn't specify aversion:input forgolangci-lint-action@v9, so CI will install whatever the action's default resolves to. This can cause local vs. CI drift (different findings/false positives). Consider exposing this version to CI (e.g., a.golangci-lint-versionfile consumed viaversion-file:, or a shared constant).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` at line 3, The Makefile defines LINTER_VERSION := v2.10.1 but CI (golangci-lint-action@v9 in .github/workflows/golangci-lint.yml) isn't pinned, causing drift; fix by exposing and reusing the same pinned version in CI: either add a .golangci-lint-version file with the same value and point the workflow's golangci-lint-action to version-file: .golangci-lint-version, or update the workflow to set the action's version: input to LINTER_VERSION (or otherwise reference a shared constant) so both local Makefile (LINTER_VERSION) and the workflow use the identical linter version.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.golangci.yml:
- Line 5: The config sets run.tests: false which disables linting for all
*_test.go files; revert or remove run.tests: false to re-enable test analysis
and instead add targeted per-rule exclusions if you need to relax checks in
tests (use exclusions.rules with path: _test\.go for specific linters like
gosec, gocritic, errcheck) and review the current exclusions.paths: [tests] to
avoid fully skipping test code; update the .golangci.yml to enable test runs and
add per-rule exclusions referencing run.tests and exclusions.rules (path:
_test\.go) as needed.
- Around line 62-68: The global gosec excludes block currently suppresses many
important rules (the gosec "excludes" list containing G117, G304, G118, G401,
G501) which hides weak-crypto and taint issues; remove the broad excludes and
either (1) delete G401/G501/G304/G118 from the excludes list so gosec will flag
them, or (2) restrict exclusions to specific files via per-site "//nosec Gxxx --
reason" comments in the exact source locations that need suppression and add
explanatory comments in the config only for any remaining necessary,
narrowly-scoped excludes; ensure you update the ".golangci.yml" gosec.excludes
section to include rationale if any IDs must remain and prefer path_patterns or
per-file annotations over project-wide suppression.
- Around line 84-85: The global suppression of SA1019 in the staticcheck
configuration should be removed: undo the "-SA1019" entry under staticcheck
checks so SA1019 runs again, and instead restore targeted per-site suppressions
(use //nolint:staticcheck with short justification) at the specific call sites
that previously required it; search for places that had been converted (mentions
of SA1019 or prior //nolint comments) and add back the localized
//nolint:staticcheck annotations adjacent to the deprecated-API calls so only
reviewed exceptions remain.
- Around line 87-89: The current exclusions.paths entry uses a bare "tests"
pattern which will match any path containing that substring; update the
.golangci.yml exclusions.paths to use anchored or more specific regexes (e.g.,
'^tests/' for a top-level tests directory or '^pkg/(config|models)/tests/' to
target the existing pkg/config/tests and pkg/models/tests directories) so that
only the intended test fixture directories are excluded; modify the
exclusions.paths value accordingly while keeping the existing run.tests: false
behavior.
In `@CHANGELOG.md`:
- Around line 11-13: Update the CHANGELOG entry under "### Changed" to
explicitly list the user-visible API changes: note that component
vulnerability/CPE responses replaced error_code/error_message with
info_code/info_message, and that the status code ComponentWithoutInfo was
renamed to NoInfo; optionally add a separate bullet for the Go toolchain bump to
1.25.0 so the changelog accurately reflects these PR changes.
In `@pkg/usecase/vulnerability_use_case.go`:
- Line 87: The codebase mixes two status codes for the same semantic
state—domain.ComponentWithoutInfo and the newly used domain.NoInfo—causing
inconsistent info_code in responses; update pkg/helpers/vulnerabilty_helper.go
(the logic used by helpers.MergeOSVAndLocalVulnerabilities) to replace any usage
of domain.ComponentWithoutInfo with domain.NoInfo in the "No vulnerabilities
found" / no-info branch and ensure any returned structs or StatusCode
assignments there and in helper functions consistently use domain.NoInfo to
match changes made in OSV_use_case.go, cpe.go, and vulnerability_use_case.go.
---
Outside diff comments:
In `@pkg/adapters/vulnerability_support.go`:
- Around line 150-159: The loop that sets info.InfoCode/info.InfoMessage for
each entry (the componentsInfo iteration using key := info.Purl +
info.Requirement + info.Version and lookup in statusByKey) currently treats any
non-empty status.StatusCode (including domain.Success) the same and assigns an
InfoMessage pointer to an empty string; change the logic to explicitly skip or
treat the Success case differently: detect domain.Success (status.StatusCode ==
domain.Success) and either do not set InfoCode/InfoMessage at all or set
InfoMessage to nil while leaving InfoCode unset, so successful components do not
emit info_code="Success" with an empty info_message. Ensure the check is updated
where similar logic appears (the other block around the same pattern) to avoid
regressing the behavior.
In `@pkg/helpers/vulnerabilty_helper.go`:
- Line 43: Replace the legacy enum value domain.ComponentWithoutInfo with the
new refactored constant domain.NoInfo in the StatusCode assignment inside the
helper that sets component vulnerability info (look for StatusCode:
domain.ComponentWithoutInfo in the function in vulnerabilty_helper.go) so it
matches other usages (vulnerability_use_case.go, cpe.go, OSV_use_case.go);
update that single reference to domain.NoInfo and run tests/compile to confirm
consistency.
---
Nitpick comments:
In @.github/workflows/golangci-lint.yml:
- Line 15: Update the checkout action to a current major version: replace the
uses entry referencing actions/checkout@v3 with a newer supported release (e.g.,
actions/checkout@v4 or actions/checkout@v5) so the workflow aligns with the
other updated actions (refer to the existing uses: actions/checkout@v3 line) and
ensure the workflow YAML still passes linting and any runtime compatibility
checks after the change.
- Around line 28-30: The GitHub Actions step using
golangci/golangci-lint-action@v9 should be pinned to the same golangci-lint
release used in the Makefile (v2.10.1) to avoid CI vs local mismatches; update
the action step that references golangci/golangci-lint-action@v9 and add the
version: v2.10.1 input (alongside the existing args: --timeout=5m) so the action
runs that specific golangci-lint release and matches the Makefile's lint_docker
target.
In `@Makefile`:
- Line 3: The Makefile defines LINTER_VERSION := v2.10.1 but CI
(golangci-lint-action@v9 in .github/workflows/golangci-lint.yml) isn't pinned,
causing drift; fix by exposing and reusing the same pinned version in CI: either
add a .golangci-lint-version file with the same value and point the workflow's
golangci-lint-action to version-file: .golangci-lint-version, or update the
workflow to set the action's version: input to LINTER_VERSION (or otherwise
reference a shared constant) so both local Makefile (LINTER_VERSION) and the
workflow use the identical linter version.
In `@pkg/usecase/OSV_use_case.go`:
- Line 269: The nolint justification on the us.client.Do(req) call is misleading
because the request URL is constructed from us.OSVAPIBaseURL+"/query" (server
configuration) not PURL data; update the inline nolint comment to accurately
state that the URL base is sourced from trusted server configuration (not user
input) and that any PURL-derived data is only included in the request body—e.g.,
replace the existing comment on the us.client.Do(req) line with a concise
explanation like "URL base is sourced from trusted server configuration, not
user input" while keeping the nolint:gosec suppression.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3f290521-9fda-4fbc-a820-81f174feeebe
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (21)
.github/workflows/go-ci.yml.github/workflows/golangci-lint.yml.golangci.ymlCHANGELOG.mdMakefilego.modpkg/adapters/vulnerability_support.gopkg/dtos/component_dto.gopkg/dtos/vulnerability_output.gopkg/entities/component.gopkg/helpers/vulnerabilty_helper.gopkg/models/epss.gopkg/models/versions.gopkg/models/vulns_purl.gopkg/protocol/rest/server.gopkg/service/vulnerability_service.gopkg/usecase/OSV_use_case.gopkg/usecase/cpe.gopkg/usecase/local_use_case.gopkg/usecase/vulnerability_use_case.gopkg/utils/cvss.go
💤 Files with no reviewable changes (2)
- pkg/models/versions.go
- pkg/dtos/vulnerability_output.go
agustingroh
force-pushed
the
chore/SP-4308-add-info-message-and-info-code-vulnerability-response
branch
3 times, most recently
from
78dfacf to
01fb7d2
Compare
agustingroh
force-pushed
the
chore/SP-4308-add-info-message-and-info-code-vulnerability-response
branch
from
01fb7d2 to
528e159
Compare
agustingroh
deleted the
chore/SP-4308-add-info-message-and-info-code-vulnerability-response
branch
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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
Release Notes
Chores
Changed
Info_message & info_code [vulnerability response]
Info_message & info_code [cpe response]