fix: version compare

Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>

Author: ulemons

services/apps/packages_worker/src/blast-radius/clients/nugetSource.ts

@@ -47,12 +47,8 @@ function codeloadTarballUrl(owner: string, repo: string, ref: string): string {
   return `https://codeload.github.com/${owner}/${repo}/tar.gz/${ref}`
 }
 
-// .nupkg ships compiled DLLs, not C# source (unlike Maven's -sources.jar), so source
-// must come from GitHub. Ordered candidates: the exact commit the nuspec <repository>
-// element records (most precise), then a couple of common version-tag conventions
-// against the same repo. Only GitHub repos are supported — GitLab/Bitbucket tarballs
-// don't share codeload's single-wrapper-directory layout that downloadAndExtractTarball
-// (strip: 1) relies on.
+// .nupkg has no source (unlike Maven's -sources.jar), so fetch from GitHub repo.
+// Try exact commit first, then common version-tag conventions.
 async function candidateSourceTarballUrls(packageId: string, version: string): Promise<string[]> {
   const nuspec = await fetchNuspec(packageId, version)
   if (isNuGetFetchError(nuspec)) return []
@@ -66,11 +62,14 @@ async function candidateSourceTarballUrls(packageId: string, version: string): P
   const ownerRepo = githubOwnerRepo(canonical.url)
   if (!ownerRepo) return []
 
-  const candidates: string[] = []
-  if (commit) candidates.push(codeloadTarballUrl(ownerRepo.owner, ownerRepo.repo, commit))
-  candidates.push(codeloadTarballUrl(ownerRepo.owner, ownerRepo.repo, `v${version}`))
-  candidates.push(codeloadTarballUrl(ownerRepo.owner, ownerRepo.repo, version))
-  return candidates
+  // An authoritative commit must never fall through to guessed tags, which could
+  // resolve to a different revision and produce a verdict from mismatched source.
+  if (commit) return [codeloadTarballUrl(ownerRepo.owner, ownerRepo.repo, commit)]
+
+  return [
+    codeloadTarballUrl(ownerRepo.owner, ownerRepo.repo, `v${version}`),
+    codeloadTarballUrl(ownerRepo.owner, ownerRepo.repo, version),
+  ]
 }
 
 export async function downloadAndExtractNuGetSource(

services/apps/packages_worker/src/blast-radius/packageIdentifier.ts

@@ -1,16 +1,12 @@
-// Strip query string and fragment from a purl or identifier string.
 function stripQueryAndFragment(input: string): string {
   const q = input.indexOf('?')
   const h = input.indexOf('#')
   const cut = q === -1 ? h : h === -1 ? q : Math.min(q, h)
   return cut === -1 ? input : input.slice(0, cut)
 }
 
-// The blast-radius submit endpoint accepts either a bare npm package name
-// ("lodash", "@babel/core") or a full purl ("pkg:npm/lodash", "pkg:npm/%40babel/core@4.17.21")
-// for the `package` field — see blastRadiusJobRequestSchema. OSV affected-package entries and
-// the npm registry only ever use bare names, so a purl must be reduced to that form before
-// it's compared against them (raw string equality otherwise never matches a purl input).
+// OSV/npm registry use bare names only, so purls must be normalized before comparison.
+// See blastRadiusJobRequestSchema for accepted formats.
 export function toBareNpmName(input: string): string {
   let name = input.trim()
 

services/apps/packages_worker/src/blast-radius/stages/ecosystemVersions.ts

@@ -3,10 +3,8 @@ import { OsvAffectedPackage } from '../clients/osvClient'
 
 import { compareNuGetVersion } from './nuget/nugetVersionCompare'
 
-// Shared by any ecosystem whose OSV advisories use ECOSYSTEM-typed ranges (ordered via
-// compareVersion(ecosystem, …) rather than node-semver) instead of SEMVER-typed ranges —
-// currently Maven and NuGet. Parameterized by ecosystem so the two don't duplicate this
-// logic; see mavenVersions.ts for the ecosystem-bound wrappers Maven's stage files import.
+// Shared range type for Maven and NuGet (ecosystems with ECOSYSTEM-typed OSV ranges).
+// Parameterized by ecosystem to avoid duplication; see mavenVersions.ts for wrappers.
 export interface EcosystemRange {
   introduced: string | null
   fixed: string | null

services/apps/packages_worker/src/blast-radius/stages/selectAdvisoryEntry.ts

@@ -13,30 +13,28 @@ export function selectAdvisoryEntry<T extends { package: { name: string } }>(
   matchesRequested: (entry: T) => boolean,
   advisoryOsvId: string,
 ): SelectedAdvisoryEntry<T> {
+  const affectedNames = entries.map((e) => e.package.name)
+
   // `!== null`, not truthiness — an empty string is still an explicit (if malformed)
   // request and must go through matching/rejection, not be treated as "none requested".
   if (requestedPackageName !== null) {
     const entry = entries.find(matchesRequested)
     if (!entry) {
-      const affectedNames = entries.map((e) => e.package.name).join(', ')
       throw ApplicationFailure.nonRetryable(
         `Requested package ${requestedPackageName} not found in advisory ${advisoryOsvId} ` +
-          `(affected: ${affectedNames})`,
+          `(affected: ${affectedNames.join(', ')})`,
         'ADVISORY_PACKAGE_NOT_FOUND',
       )
     }
     return {
       entry,
-      relatedAffectedPackages: entries
-        .map((e) => e.package.name)
-        .filter((name) => name !== entry.package.name),
+      relatedAffectedPackages: affectedNames.filter((name) => name !== entry.package.name),
     }
   }
 
   if (entries.length > 1) {
-    const affectedNames = entries.map((e) => e.package.name).join(', ')
     throw ApplicationFailure.nonRetryable(
-      `Advisory ${advisoryOsvId} affects ${entries.length} packages (${affectedNames}); ` +
+      `Advisory ${advisoryOsvId} affects ${entries.length} packages (${affectedNames.join(', ')}); ` +
         `advisory-wide analysis is not supported for multi-artifact advisories — specify one via 'package'`,
       'ADVISORY_MULTI_ARTIFACT_AMBIGUOUS',
     )