Skip to content

fix: correct format verbs in diagnostic messages#10805

Open
eran132 wants to merge 1 commit into
aquasecurity:mainfrom
eran132:fix/diagnostic-format-verbs
Open

fix: correct format verbs in diagnostic messages#10805
eran132 wants to merge 1 commit into
aquasecurity:mainfrom
eran132:fix/diagnostic-format-verbs

Conversation

@eran132
Copy link
Copy Markdown

@eran132 eran132 commented Jun 6, 2026

Description

While reviewing the codebase I found two small bugs that produce misleading diagnostic output:

  1. pkg/vex/sbomref.go — when fetching an external VEX document, a non-2xx HTTP response is reported with:

    xerrors.Errorf("did not receive 2xx status code: %w", res.StatusCode)

    res.StatusCode is an int, but %w expects an error. So a failed fetch renders as:

    did not receive 2xx status code: %!w(int=404)
    

    Switched to %d so the actual status code is shown. (go vet flags this class of mistake.)

  2. pkg/sbom/spdx/marshal.go — the "Unsupported hash algorithm" warning logs the alg variable from the default branch of the switch, where it is still the zero value:

    var alg spdx.ChecksumAlgorithm
    switch d.Algorithm() {
    ...
    default:
        m.logger.Warn("Unsupported hash algorithm", log.String("algorithm", string(alg))) // always ""
    }

    So the warning never names the offending algorithm. Changed it to log d.Algorithm().

I also strengthened the existing "vex not found" test case to assert that the status code appears in the returned error message. The previous test only checked require.Error, which still passes with the garbled %!w(int=404) output — so it would not have caught this regression.

Related issues

N/A

Checklist

  • I've read the guidelines for contributing to this repository.
  • I've followed the conventions in the PR title.
  • I've added tests that prove my fix is effective or that my feature works.
  • I've updated the documentation with the relevant information (if needed).
  • I've added usage information (if the PR introduces new options)
  • I've included a "before" and "after" example to the description (if the PR is a user interface change).

Two diagnostic-output bugs that produce misleading messages:

- pkg/vex/sbomref.go: the external VEX fetch used %w to format
  res.StatusCode (an int) in xerrors.Errorf. %w expects an error, so a
  non-2xx response rendered as "...status code: %!w(int=404)". Use %d.

- pkg/sbom/spdx/marshal.go: the "Unsupported hash algorithm" warning
  logged the (still zero-valued) alg variable from the default switch
  branch, always emitting an empty algorithm. Log d.Algorithm() instead.

Strengthen the existing "vex not found" test to assert the status code
appears in the error message, which guards against the %w regression
(the previous require.Error check passed even with a garbled error).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@eran132 eran132 requested a review from knqyf263 as a code owner June 6, 2026 09:46
Copilot AI review requested due to automatic review settings June 6, 2026 09:46
@eran132 eran132 requested a review from DmitriyLewen as a code owner June 6, 2026 09:46
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Jun 6, 2026

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR tightens error handling and observability around external VEX document retrieval and SPDX checksum marshaling, and extends tests to validate the updated behavior.

Changes:

  • Fix HTTP status code error formatting when retrieving external VEX documents.
  • Extend VEX retrieval tests to assert on error message content for non-2xx responses.
  • Improve log output for unsupported checksum algorithms by logging the actual digest algorithm string.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
pkg/vex/sbomref_test.go Adds per-test expected error substring assertions for failed external VEX fetches.
pkg/vex/sbomref.go Fixes incorrect error formatting for non-OK HTTP responses.
pkg/sbom/spdx/marshal.go Logs the correct unsupported hash algorithm (from the digest) in warnings.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/vex/sbomref.go
Comment on lines 86 to 88
if res.StatusCode != http.StatusOK {
return nil, xerrors.Errorf("did not receive 2xx status code: %w", res.StatusCode)
return nil, xerrors.Errorf("did not receive 2xx status code: %d", res.StatusCode)
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants