fix: return useful error instead of 500 when attestation verification raises - #6675
Open
mvanhorn wants to merge 2 commits into
Open
fix: return useful error instead of 500 when attestation verification raises#6675mvanhorn wants to merge 2 commits into
mvanhorn wants to merge 2 commits into
Conversation
…rors; keep 500 for server failures
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
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
A gem push with an attestation whose verification raises an unexpected error no longer returns a bare
500. Known Sigstore validation failures return a useful422with the specific message (as before), while unexpected errors from the verifier are reported for observability and return a generic, client-safe message rather than a raw exception dump.Why this matters
Issue #6369 reports that pushing a gem with a bad/unexpected attestation can produce an opaque
500with no actionable detail. The push path only rescuedSigstore::Error; anything else propagated as a generic server error, so a user with a malformed attestation just saw "500" and could not tell what went wrong.This keeps the existing
Sigstore::Errorbranch (specific, client-safe422validation message) and narrows a catch-all rescue to the Sigstore verifier call itself. An unexpected error there is reported viaRails.error.report(e, handled: true)and returned as a genericAttestation verification failed.message. Crucially, it does not leak the raw exception text to the public API and does not reclassify genuine server-side failures (policy setup, association, metrics) as client errors — those still surface as the normal500.Testing
Added
pusher_testcoverage: a knownSigstore::Errorreturns the specific validation message; an unexpected error during verification is reported, returns the generic message, and does not include the raw exception text. Note: the repo requires Ruby 4.0.5 / Bundler 4, which was not available in this environment, so the suite was not executed locally; the change is a small, self-contained rescue-scoping fix reviewed against the surrounding push flow.Fixes #6369