Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates CSAF report generation to normalize vulnerability IDs (notably GHSA vs CVE casing) before querying dependency vulnerabilities.
Changes:
- Replace direct
strings.ToUpper(...)CVE normalization with a shared helper (normalize.UppercaseCVEID) in CSAF report generation. - Add a new normalization helper for CVE/GHSA IDs under
normalize/. - Update Go module files (
go.mod/go.sum) with an additional indirect dependency.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| services/csaf_service.go | Uses new normalization helper when parsing the CSAF report “version” route parameter. |
| normalize/cve.go | Adds UppercaseCVEID helper intended to normalize GHSA/CVE IDs. |
| go.mod | Adds github.com/Masterminds/semver v1.5.0 as an indirect dependency. |
| go.sum | Adds corresponding checksums for the new indirect dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // remove everything <asset-slug>_ from the beginning of the document id | ||
| cveID = strings.ToUpper(strings.Split(cveID, ".json")[0]) | ||
| cveID = normalize.UppercaseCVEID(strings.Split(cveID, ".json")[0]) |
There was a problem hiding this comment.
The inline comment says the code removes a "_" prefix from the document id, but the implementation only strips a ".json" suffix and normalizes casing. Either implement the prefix removal (e.g., trim asset.Slug+"_") or update the comment so it matches the actual behavior.
| // Copyright 2026 larshermges | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // https://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. |
There was a problem hiding this comment.
The repository is licensed under AGPL-3.0 (see LICENSE.txt), but this new file introduces an Apache-2.0 header. Unless this file is explicitly excluded/relicensed, the header should match the project’s AGPL notice to avoid licensing/compliance conflicts.
| prefix := strings.Split(cveID, "-")[0] | ||
| switch strings.ToLower(prefix) { | ||
| case "ghsa": | ||
| return "GHSA-" + strings.ToLower(cveID[len(prefix)+1:]) |
There was a problem hiding this comment.
UppercaseCVEID can panic for inputs like "ghsa" (no '-' present): prefix becomes "ghsa" and cveID[len(prefix)+1:] slices past the string length. Consider using strings.Cut/SplitN and only slicing when a '-' separator is present; otherwise fall back to returning strings.ToUpper(cveID) (or another safe default).
| prefix := strings.Split(cveID, "-")[0] | |
| switch strings.ToLower(prefix) { | |
| case "ghsa": | |
| return "GHSA-" + strings.ToLower(cveID[len(prefix)+1:]) | |
| prefix, rest, found := strings.Cut(cveID, "-") | |
| if !found { | |
| // No '-' present: fall back to uppercasing the whole ID to avoid panics. | |
| return strings.ToUpper(cveID) | |
| } | |
| switch strings.ToLower(prefix) { | |
| case "ghsa": | |
| return "GHSA-" + strings.ToLower(rest) |
| // make sure ghsa is uppercase | ||
| // extract the prefix | ||
| prefix := strings.Split(cveID, "-")[0] | ||
| switch strings.ToLower(prefix) { | ||
| case "ghsa": | ||
| return "GHSA-" + strings.ToLower(cveID[len(prefix)+1:]) | ||
| default: | ||
| return strings.ToUpper(cveID) | ||
| } |
There was a problem hiding this comment.
The function name UppercaseCVEID is misleading because the GHSA branch returns a mixed-case value (prefix uppercased, suffix lowercased). Consider renaming to something like NormalizeVulnID/NormalizeCVEID (or adjust behavior to truly uppercase everything) so callers don’t misinterpret what it guarantees.
| // make sure ghsa is uppercase | |
| // extract the prefix | |
| prefix := strings.Split(cveID, "-")[0] | |
| switch strings.ToLower(prefix) { | |
| case "ghsa": | |
| return "GHSA-" + strings.ToLower(cveID[len(prefix)+1:]) | |
| default: | |
| return strings.ToUpper(cveID) | |
| } | |
| return strings.ToUpper(cveID) |
| import "strings" | ||
|
|
||
| func UppercaseCVEID(cveID string) string { | ||
| // make sure ghsa is uppercase |
There was a problem hiding this comment.
The comment says “make sure ghsa is uppercase”, but the code also lowercases the remainder of the GHSA ID. Update the comment to reflect the actual normalization being applied (e.g., “ensure GHSA prefix is uppercase and normalize the rest”).
| // make sure ghsa is uppercase | |
| // ensure GHSA prefix is uppercase and normalize the rest |
| func UppercaseCVEID(cveID string) string { | ||
| // make sure ghsa is uppercase | ||
| // extract the prefix | ||
| prefix := strings.Split(cveID, "-")[0] | ||
| switch strings.ToLower(prefix) { | ||
| case "ghsa": | ||
| return "GHSA-" + strings.ToLower(cveID[len(prefix)+1:]) | ||
| default: | ||
| return strings.ToUpper(cveID) | ||
| } |
There was a problem hiding this comment.
UppercaseCVEID is new behavior used to normalize request parameters for CSAF report lookup, but there are no unit tests covering GHSA vs CVE inputs or malformed values (e.g., "ghsa" without a dash). Adding a small table-driven test would protect against regressions and the panic case.
No description provided.