Skip to content

fix(supply-chain): SC4 must not claim a vulnerability it did not verify (#318) - #319

Open
Mark2Mac wants to merge 1 commit into
NVIDIA:mainfrom
Mark2Mac:fix/sc4-unresolved-version-severity
Open

fix(supply-chain): SC4 must not claim a vulnerability it did not verify (#318)#319
Mark2Mac wants to merge 1 commit into
NVIDIA:mainfrom
Mark2Mac:fix/sc4-unresolved-version-severity

Conversation

@Mark2Mac

Copy link
Copy Markdown
Contributor

Fixes #318.

The problem

When a manifest admits a range, no version is resolved and query_batch is called with (name, None). OSV then answers a different question than the one SC4 asks: not "is this release affected?" but "has this package ever had an advisory?". _sc4_from_osv takes the worst severity of that history and reports it as the finding's severity, so:

[build-system]
requires = ["setuptools>=61"]
CRITICAL SC4 — Known Vulnerable Dependency: setuptools — N advisory(ies)

The floor excludes every affected release. Nothing here was verified.

Why the severity, specifically

The lack of pinning is already reported by SC1 ("Dependencies lack version pinning"), so SC4 is not the only signal and nothing is lost by making it honest. What is left for SC4 to say in this case is "could not verify" — and that must not outrank a finding where a version genuinely matched a vulnerable release.

So: severity capped at LOW, confidence 0.4, and wording that states the limit (Unverifiable Dependency: … the manifest does not pin a version, so it is unknown whether the installed release is affected) instead of implying a match. matched_text no longer carries a == that the manifest never had. Version-matched findings are completely unchanged.

Field data

Scanning 65 skill/plugin units: every SC4 finding in the corpus came from this behaviour or from a range being read as a pin (#294 / #302). Not one manifest pinned a vulnerable release. Two of the affected files are first-party marketplaces, so the practical effect today is that a user either fixes a non-problem or learns to ignore SC4 entirely.

Relationship to #302

Complementary halves of the same defect, and neither is sufficient alone — measured on the same inputs:

manifest today #302 alone this PR alone both
setuptools>=61 CRITICAL CRITICAL LOW LOW
"shell-quote": "^1.8.3" CRITICAL ==1.8.3 CRITICAL CRITICAL ==1.8.3 LOW
jinja2==2.4.1 (really vulnerable) HIGH HIGH HIGH HIGH

#302 stops inventing a version; this PR stops claiming a match that was never made.

Tests

Three regressions on _sc4_from_osv: pinned version keeps the OSV severity and the name==version wording; unresolved version is capped, de-confidenced and reworded; no advisories still emits nothing. Full unit suite green (696 passed, 14 skipped).

When a manifest admits a range, no version is resolved and OSV is queried by name
alone. The advisories that come back are the package's history, not a match against
the release that will be installed: the worst of them may predate every version the
range admits. Using that as the finding's severity turns 'setuptools>=61' into a
CRITICAL 'Known Vulnerable Dependency'.

Scanning 65 skill/plugin units, every SC4 finding in the corpus came from this or
from a range being read as a pin (NVIDIA#294/NVIDIA#302). Not one manifest pinned a vulnerable
release.

The lack of pinning is already reported by SC1, so what is left for SC4 to say is
'could not verify', and it must not outrank a real version match: severity capped at
LOW, confidence 0.4, and wording that states the limit instead of implying a match.
Version-matched findings are unchanged.

Closes NVIDIA#318

Signed-off-by: Mark2Mac <Mark2Mac@users.noreply.github.com>
@Mark2Mac

Copy link
Copy Markdown
Contributor Author

Cross-PR note, from merging all my open PRs together locally.

This PR and #302 interact, and it is worth seeing before either lands. Real example from the corpus:

# requirements.txt
anthropic>=0.40.0

Today the >= is read as a pin, OSV is queried for anthropic==0.40.0, that release is clean, and nothing is reported. With #302 the version becomes unknown, the query goes out by name, advisories come back — and it is this PR that decides what to say about them: SC4 LOW — unverifiable, instead of the package's worst-ever severity.

In other words the false pin was accidentally producing silence in this case, and the two PRs together turn it into an honest low-severity note rather than either a wrong CRITICAL or nothing at all. Neither ordering is broken; both applied, the full suite is green (1585 passed, 14 skipped, 6 xfailed).

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Automated SkillSpector Review]

Approved. A name-only OSV query is now reported as unverifiable rather than as a version-matched vulnerability: severity is capped at LOW, confidence is reduced, and the message/matched text no longer invent an exact version. Exact-version matches preserve the OSV severity and wording. The focused supply-chain suite passes (242 tests).

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.

[BUG] SC4 has no notion of version ranges: "^1.8.3" becomes an exact pin, and an unversioned dep gets the package's worst-ever advisory as CRITICAL

2 participants