Skip and warn on malformed OSV records#19496
Conversation
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
| match response.json::<Vulnerability>().await { | ||
| Ok(vuln) => Ok(Some(vuln)), | ||
| Err(err) if err.is_decode() => { | ||
| warn_user!("Skipping malformed OSV record: {id}"); |
There was a problem hiding this comment.
This doesn't make sense for warn_user! imo — it's not an actionable warning for a user.
There was a problem hiding this comment.
Yeah, I'm not sure if there's a better fit ATM. It's not directly actionable, but we do want users to warn us (or better yet, OSV) if this ever regresses.
There was a problem hiding this comment.
I don't think we're going to get reports of this though given that message. hm.
This is in uv audit?
There was a problem hiding this comment.
Yep, in uv audit.
My 0.02c is that I'm equally fine with keeping this as a hard error (maybe with a better error message than the current JSON one) -- this is squarely a regression in OSV and I agree that users are less likely to tell us about it if we're suppressing it, even with a warning. If you're -1 on this then I'll switch to making the error message itself better instead.
There was a problem hiding this comment.
Yeah I think uv audit should hard fail here if it can't perform an audit.
I think we could add an --skip-malformed-entries flag or something?
There was a problem hiding this comment.
OK, works for me. I'll follow up with a better error PR.
I think we could add an
--skip-malformed-entriesflag or something?
Maybe, although my worry with a flag would be similar (users will habituate themselves into using it). Maybe that's not a realistic concern on my part.
uv test inventory changesThis PR changes the tests when compared with the latest successful
|
Temporarily skip the workflow when `uv audit` exits non-zero due to the upstream OSV/uv deserialization bug (astral-sh/uv#19492). The bad OSV record (PYSEC-2026-89 and similar) contains an empty `{}` in `affected.ranges.events`, which uv's strict Rust deserializer rejects, aborting the entire audit before vulnerabilities can be reported. The workflow now: - Captures uv audit's stderr separately and forwards it to the run log. - Detects the specific `error decoding response body` failure mode. - Emits a GitHub Actions ::warning:: annotation plus a Step Summary explaining what was skipped, why, and how to remove the shim. - Exits successfully so unrelated PRs aren't blocked while we wait for the upstream fix (astral-sh/uv#19496 or equivalent). All other uv audit error exit codes (>1) still fail loudly. Once the `setup-uv` version is bumped past 0.11.15 to a release containing the upstream fix, the workaround branch should be removed so future audit regressions surface immediately. Signed-off-by: Steve Scargall <37674041+sscargal@users.noreply.github.com>
Summary
We now skip (and warn) if we encounter a malformed OSV record. This is (IMO) more correct that trying to suss out the intent behind a malformed record, since OSV is a strictly-defined format and the only reason we're seeing this at the moment is because OSV appears to be having a regression that their tests didn't catch.
Ref: pypa/advisory-database#284
Fixes #19492.
Test Plan
Added an integration test.