fix: complete the ecosystem-case split (URL titlecase, storage lowercase)

The previous "lowercase Maven" commit (1d77fd3) was incomplete:
versionCompare.compareVersion still keyed on 'Maven' (titlecase) so
deriveCriticalFlag's lowercase 'maven' fell through to the null
branch, isInRange treated every Maven range as "no match," and
Maven packages could not have has_critical_vulnerability flipped
TRUE. The same commit also lowercased OSV_ECOSYSTEMS in the env,
which broke the OSV bucket URL (Maven/all.zip is case-sensitive on
the GCS side — maven/all.zip 404s).

Cursor and Copilot both caught this on the latest push; 8 bot
threads were filed against the partial fix. This commit closes
all of them.

Changes:

- versionCompare.ts: compareVersion now matches lowercase 'maven'
  per ADR-0001 §OSV. compareMaven returns null for empty / punctuation-
  only input (Cursor 3312194073) so isInRange treats unparseable
  versions as "no match" rather than silently ordering them as 0.
- versionCompare.ts callers were already lowercase (deriveCriticalFlag
  via the packages-db column, parseOsvRecord via the .toLowerCase()
  at the OSV boundary). No change needed there.
- activities.ts: requirePositiveInt env validator replaces parseInt,
  fails fast on non-numeric or <= 0 input so an upstream typo can't
  silently turn into NaN flush-thresholds or LIMIT NaN SQL.
- activities.ts: the allowlist is built from
  allowedEcosystems.map(toLowerCase) so the activity layer keeps the
  caller-supplied ecosystem (used as a case-sensitive bucket URL
  segment) separate from the lowercase matching set used by
  parseOsvRecord.
- .env.dist.local: OSV_ECOSYSTEMS back to npm,Maven (OSV's canonical
  bucket case). Comment clarifies that the URL is case-sensitive
  while the allowlist + DB storage normalize to lowercase internally.
- workflows.ts: comment now states the dual handling explicitly so
  the next reader doesn't trip on the same casing seam.
- versionCompare.test.ts: 19 Maven test cases moved to 'maven'
  matching production; +3 cases pinning the new compareMaven null
  contract (empty, swap empty, '...'); +1 regression guard rejecting
  titlecase 'Maven' through compareVersion so a future "forgot to
  lowercase" caller fails loudly instead of silently missing
  vulnerabilities.
- deriveCriticalFlag.integration.test.ts: finalQuirk fixture moved
  to 'maven' to match what production stores.

All 70 unit tests pass.

Signed-off-by: Joan Reyero <joan@reyero.io>

Author: joanreyero

backend/.env.dist.local

@@ -185,8 +185,12 @@ ENRICHER_REPO_UPDATE_INTERVAL_HOURS=24
 ENRICHER_IDLE_SLEEP_SEC=60
 
 # osv-sync (Temporal-scheduled; see services/apps/packages_worker/src/osv/schedule.ts)
+# OSV_ECOSYSTEMS uses OSV's canonical bucket case (npm lowercase, Maven titlecase) because
+# the bucket URL <BASE>/<ecosystem>/all.zip is case-sensitive (Maven/all.zip exists,
+# maven/all.zip 404s). The allowlist check and DB storage normalize to lowercase
+# internally per ADR-0001 §OSV "Ecosystem normalization", so downstream stays lowercase.
 OSV_BULK_BASE_URL=https://osv-vulnerabilities.storage.googleapis.com
-OSV_ECOSYSTEMS=npm,maven
+OSV_ECOSYSTEMS=npm,Maven
 OSV_TMP_DIR=/tmp/osv
 OSV_BATCH_SIZE=500
 OSV_DERIVE_BATCH_SIZE=1000

services/apps/packages_worker/src/osv/__tests__/deriveCriticalFlag.integration.test.ts

@@ -50,7 +50,7 @@ const FIXTURES: Record<string, FixturePackage> = {
   // Regression guard for the implicit-empty-qualifier Maven bug. There's no
   // critical advisory for fixture.test:final-quirk, so the flag must be FALSE.
   finalQuirk: {
-    ecosystem: 'Maven',
+    ecosystem: 'maven',
     namespace: 'fixture.test',
     name: 'final-quirk',
     latest_version: '1.0-final',

services/apps/packages_worker/src/osv/__tests__/versionCompare.test.ts

@@ -30,7 +30,7 @@ describe('compareVersion — npm (semver)', () => {
   })
 })
 
-describe('compareVersion — Maven (ComparableVersion-style)', () => {
+describe('compareVersion — maven (ComparableVersion-style)', () => {
   it.each([
     // Basic numeric ordering
     ['1.0', '1.1', -1],
@@ -56,8 +56,16 @@ describe('compareVersion — Maven (ComparableVersion-style)', () => {
     ['1.0-m1', '1.0-milestone1', 0],
     // Numeric beats alpha at same depth
     ['1.0-1', '1.0-alpha', 1],
-  ])('compareVersion("Maven", %s, %s) sign = %s', (a, b, expected) => {
-    expect(sign(compareVersion('Maven', a, b))).toBe(expected)
+  ])('compareVersion("maven", %s, %s) sign = %s', (a, b, expected) => {
+    expect(sign(compareVersion('maven', a, b))).toBe(expected)
+  })
+
+  it('returns null for unparseable maven versions', () => {
+    // Empty / punctuation-only strings tokenize to [] and used to silently
+    // compare as version 0. Per the comparator contract, return null instead.
+    expect(compareVersion('maven', '', '1.0')).toBeNull()
+    expect(compareVersion('maven', '1.0', '')).toBeNull()
+    expect(compareVersion('maven', '...', '1.0')).toBeNull()
   })
 })
 
@@ -66,4 +74,13 @@ describe('compareVersion — unsupported ecosystems', () => {
     expect(compareVersion('PyPI', '1.0.0', '2.0.0')).toBeNull()
     expect(compareVersion('crates.io', '0.1', '0.2')).toBeNull()
   })
+
+  it('rejects titlecase "Maven" — production storage is always lowercase', () => {
+    // Regression guard for the casing bug Fix 1 missed: deriveCriticalFlag
+    // reads `ecosystem` from packages-db where it's lowercase. The comparator
+    // is keyed on the same lowercase form per ADR-0001 §OSV "Ecosystem
+    // normalization". A titlecase 'Maven' call indicates the caller forgot
+    // to normalize.
+    expect(compareVersion('Maven', '1.0', '2.0')).toBeNull()
+  })
 })

services/apps/packages_worker/src/osv/activities.ts

@@ -22,8 +22,8 @@ function getActivityConfig() {
   return {
     bulkBaseUrl: required('OSV_BULK_BASE_URL'),
     tmpDir: required('OSV_TMP_DIR'),
-    upsertBatchSize: parseInt(required('OSV_BATCH_SIZE'), 10),
-    deriveBatchSize: parseInt(required('OSV_DERIVE_BATCH_SIZE'), 10),
+    upsertBatchSize: requirePositiveInt('OSV_BATCH_SIZE'),
+    deriveBatchSize: requirePositiveInt('OSV_DERIVE_BATCH_SIZE'),
   }
 }
 
@@ -33,6 +33,19 @@ function required(name: string): string {
   return value
 }
 
+// requirePositiveInt fails fast on non-numeric or zero/negative values so an
+// upstream env-config typo can't turn into NaN propagating through the upsert
+// buffer flush threshold (`buffer.length >= NaN` is always false → unbounded
+// growth) or SQL `LIMIT NaN` errors in the derive loop.
+function requirePositiveInt(name: string): number {
+  const raw = required(name)
+  const parsed = parseInt(raw, 10)
+  if (!Number.isFinite(parsed) || parsed <= 0) {
+    throw new Error(`Env ${name} must be a positive integer, got: ${raw}`)
+  }
+  return parsed
+}
+
 export interface OsvSyncEcosystemInput {
   ecosystem: string
   allowedEcosystems: string[]
@@ -66,7 +79,11 @@ export async function osvSyncEcosystem(
 ): Promise<OsvSyncEcosystemResult> {
   const { ecosystem, allowedEcosystems } = input
   const config = getActivityConfig()
-  const allowed = new Set(allowedEcosystems)
+  // OSV's bucket uses case-sensitive paths (Maven/all.zip, not maven/all.zip),
+  // so `ecosystem` (used for the URL) keeps OSV's canonical case. The allowlist
+  // is matched in parseOsvRecord after lowercasing each record's ecosystem per
+  // ADR-0001 §OSV "Ecosystem normalization", so the set is built lowercase here.
+  const allowed = new Set(allowedEcosystems.map((e) => e.toLowerCase()))
   const start = Date.now()
 
   const ecoDir = path.join(config.tmpDir, ecosystem)

services/apps/packages_worker/src/osv/versionCompare.ts

@@ -94,9 +94,15 @@ function padFor(other: Token): Token {
   return other.kind === 'num' ? { kind: 'num', value: 0 } : { kind: 'str', value: '' }
 }
 
-function compareMaven(a: string, b: string): number {
+function compareMaven(a: string, b: string): number | null {
   const ta = tokenizeMaven(a)
   const tb = tokenizeMaven(b)
+  // Empty or punctuation-only inputs (e.g. '', '...', '---') tokenize to []
+  // and would otherwise be treated as version 0. Per the compareVersion
+  // contract ("returns null when either operand cannot be parsed"), reject
+  // them here so isInRange treats them as "no match" — safer than silently
+  // ordering garbage as 0.
+  if (ta.length === 0 || tb.length === 0) return null
   const max = Math.max(ta.length, tb.length)
   for (let i = 0; i < max; i++) {
     if (i >= ta.length) {
@@ -115,8 +121,11 @@ function compareMaven(a: string, b: string): number {
   return 0
 }
 
+// Ecosystem names are stored lowercase in packages-db per ADR-0001 §OSV
+// "Ecosystem normalization" — 'npm' and 'maven'. Callers (deriveCriticalFlag)
+// pull the value straight from the DB so the literals here must match.
 export function compareVersion(ecosystem: string, a: string, b: string): number | null {
   if (ecosystem === 'npm') return compareNpm(a, b)
-  if (ecosystem === 'Maven') return compareMaven(a, b)
+  if (ecosystem === 'maven') return compareMaven(a, b)
   return null
 }

services/apps/packages_worker/src/osv/workflows.ts

@@ -28,9 +28,12 @@ const { osvSyncEcosystem, osvDeriveCriticalFlag } = proxyActivities<typeof activ
 })
 
 export interface OsvSyncWorkflowInput {
-  // OSV ecosystem labels (case-sensitive: 'npm', 'Maven', 'PyPI', …). The same
-  // list is used both for the per-ecosystem download loop and as the
-  // post-parse allowlist filter inside parseOsvRecord.
+  // OSV ecosystem labels in OSV's canonical case ('npm', 'Maven', 'PyPI', …).
+  // Same list is used by the per-ecosystem download loop, where OSV's bucket
+  // is case-sensitive (Maven/all.zip), and as the allowlist source for
+  // parseOsvRecord. The activity lowercases the allowlist internally before
+  // matching parsed records, since storage normalizes to lowercase per
+  // ADR-0001 §OSV "Ecosystem normalization".
   ecosystems: string[]
 }