Flag left-out packages, known-range conversions and version spelling

CHANGELOG.md

@@ -1,5 +1,21 @@
 # Changelog
 
+## Unreleased
+
+This changes finding codes and allow-list coverage. The cases come from running the tool on real advisory edits.
+
+- New `sibling-package-unchanged` finding: version ranges changed for one package but not for another in the same
+  advisory that had the same version data. It needs review, because packages often share a fix but can really
+  differ. A `--allow` list that used to pass such an edit now also has to name this code.
+- New `known-range-converted` finding: `last_known_affected_version_range` replaced by a range's one new end. This
+  was reported as an added event and a removed field, and the second suggested the value may have been dropped by
+  accident. The finding says whether the two name the same end, comparing strings only. A `--allow` list naming
+  `range-event-added,field-removed` no longer covers this case.
+- New `version-spelling-changed` finding: a range value that changes only in leading zeros, such as `2025.08.14` to
+  `2025.8.14`. `--allow range-event-changed` no longer covers this case.
+- New `version-leading-zero` warning from `validate`, for range events in `npm` packages and `SEMVER` ranges. The
+  file is still valid OSV, so `validate` still exits 0.
+
 ## 0.2.0
 
 This release changes finding codes, allow-list coverage and some exit codes. See the

README.md

@@ -197,7 +197,7 @@ On top of the schema it adds a few checks, each reported with its own code:
 - GitHub REST/GraphQL advisory objects (not OSV) are recognised and rejected with a pointer to the OSV format.
 
 Problems are `error` (invalid, exit 2) or `warning` (valid but unusual: missing severity, duplicate
-references, scheme-less URLs, non-semver versions in a SEMVER range, ...). JSON Schema
+references, scheme-less URLs, non-semver versions in a SEMVER range, leading zeros in an npm or SEMVER version, ...). JSON Schema
 `format` keywords (`uri`, `date-time`) are treated as annotations, as in the specification's default
 mode, so a genuine GitHub record with a scheme-less reference URL is valid with a warning.
 
@@ -209,7 +209,7 @@ the indexes in paths:
 | --- | --- |
 | `severity[]` (also `affected[].severity`) | Matched by type, so CVSS v3 and v4 (and several entries) stay distinct. Removed = review, changed = review (with metric delta and v3 base score), added = info. |
 | `affected[]` | Matched by ecosystem+name; a lone unmatched package in the same ecosystem is treated as a **rename** so its ranges/severity are still compared. Removed/added packages = review. |
-| ranges / events | `introduced`, `fixed`, `last_affected`, `limit` added/removed/changed = review, with an explanation (e.g. removing the only `fixed` leaves the range open-ended). Replacing `last_affected` with `fixed` (or the reverse) in a range, with the same number of values on both sides, = one review finding (`range-bound-converted`), because `last_affected` is an inclusive end and `fixed` an exclusive one, so affected status may change and needs checking against the package's release history (the repository's history for a GIT range). Several values of one kind in a range are listed without being paired. Explicit `versions[]` additions/removals = review. |
+| ranges / events | `introduced`, `fixed`, `last_affected`, `limit` added/removed/changed = review, with an explanation (e.g. removing the only `fixed` leaves the range open-ended). Replacing `last_affected` with `fixed` (or the reverse) in a range, with the same number of values on both sides, = one review finding (`range-bound-converted`), because `last_affected` is an inclusive end and `fixed` an exclusive one, so affected status may change and needs checking against the package's release history (the repository's history for a GIT range). Several values of one kind in a range are listed without being paired. Replacing `database_specific.last_known_affected_version_range` with a range's one new end = one review finding (`known-range-converted`), which says whether the two name the same end. A value that changes only in leading zeros = review (`version-spelling-changed`). Version ranges changing for one package but not for another that had the same version data = review (`sibling-package-unchanged`). Explicit `versions[]` additions/removals = review. |
 | references, aliases, related, upstream | Removal = review (lost provenance / cross-references); addition = info. URLs compared ignoring trailing slash and host case. |
 | CWE (`database_specific.cwe_ids`), GitHub severity label | Removal/change = review; addition = info. |
 | `id`, `published`, `withdrawn` | Changes = review (withdrawing is flagged). Two exceptions = info: setting a missing `published`, and changing the date of an existing `withdrawn`. |
@@ -262,6 +262,14 @@ and every such finding says why.
   of events, severity entries and references have only the per-list limit. None of this is a run-time
   guarantee: a crafted custom list of about 1.5 MB took over a minute in one local measurement, and a file
   near the 25 MB size limit can take far longer.
+- `sibling-package-unchanged` compares each package's version data as written, so it can only ask whether a
+  package was left out. Packages can really differ, and it does not notice packages whose ranges already differed
+  before the edit. Allow the code when the difference is intended.
+- `known-range-converted` compares strings only. It reads `< Y` and `<= Y`; any other form is reported as not
+  naming the same end, even if it does.
+- The leading-zero checks read spelling only, and cover range events in `npm` packages and `SEMVER` ranges. They
+  cannot tell whether a version was published. Other ecosystems can allow these spellings, so `validate` says
+  nothing about them there.
 - The tool does not compare versions, so a `range-bound-converted` finding cannot say whether any release
   changes status. Check the package's release history. If a value stays the same on both sides, the finding
   says that this end now includes or excludes it. Several values of one kind in a range are listed without

src/core/diff/affected.ts

@@ -1,7 +1,8 @@
 import { MAX_MATCHING_WORK, PAIRING_LIMIT_WHY, overlap, pairItems, setMinus } from "../pairing.js";
 import type { WorkBudget } from "../pairing.js";
 import type { Finding, JsonObject, JsonValue } from "../types.js";
-import { asArray, asObjectArray, canonical, clone, getOwn, isObject, isString, shorten } from "../util.js";
+import { asArray, asObjectArray, canonical, clone, getOwn, hasOwn, isObject, isString, shorten } from "../util.js";
+import { differsOnlyInLeadingZeros, hasLeadingZero, rejectsLeadingZeros } from "../versionSpelling.js";
 import { diffGeneric, diffOtherKeys } from "./generic.js";
 import { diffSeverity } from "./severity.js";
 
@@ -152,6 +153,88 @@ function boundConvertedWhy(git: boolean, valueKept: boolean): string {
     : `${ends} Replacing one with the other may change which releases are affected, or may change nothing, depending on what the package has released. AdvisoryGuard does not compare versions, so check the package's release history.`;
 }
 
+const KNOWN_RANGE = "last_known_affected_version_range";
+
+function knownRangeOf(entry: JsonObject): JsonValue | undefined {
+  const specific = getOwn(entry, "database_specific");
+  return isObject(specific) ? getOwn(specific, KNOWN_RANGE) : undefined;
+}
+
+/** The known range an edit removed from this entry, if it was a string. */
+function knownRangeRemoved(b: JsonObject, a: JsonObject): string | undefined {
+  const known = knownRangeOf(b);
+  if (!isString(known)) return undefined;
+  const specific = getOwn(a, "database_specific");
+  return isObject(specific) && hasOwn(specific, KNOWN_RANGE) ? undefined : known;
+}
+
+function withoutKnownRange(entry: JsonValue): JsonValue {
+  if (!isObject(entry)) return entry;
+  const specific = getOwn(entry, "database_specific");
+  if (isObject(specific)) {
+    delete specific["last_known_affected_version_range"];
+    if (Object.keys(specific).length === 0) delete entry["database_specific"];
+  }
+  return entry;
+}
+
+interface AddedEnd {
+  kind: "fixed" | "last_affected";
+  value: string;
+}
+
+/** The one end a range gained, when that is the only change to its events. */
+function soleAddedEnd(gone: Record<EventKind, string[]>, fresh: Record<EventKind, string[]>): AddedEnd | undefined {
+  if (EVENT_KINDS.some((k) => gone[k].length > 0) || fresh.introduced.length > 0 || fresh.limit.length > 0) return undefined;
+  const ends: AddedEnd[] = [
+    ...fresh.fixed.map((value) => ({ kind: "fixed" as const, value })),
+    ...fresh.last_affected.map((value) => ({ kind: "last_affected" as const, value })),
+  ];
+  return ends.length === 1 ? ends[0] : undefined;
+}
+
+/** Whether the text names exactly this end. This compares strings only, not versions. */
+function knownRangeNames(known: string, end: AddedEnd): boolean {
+  const m = /^\s*(<=|<)\s*(\S+)\s*$/.exec(known);
+  if (!m) return false;
+  return m[2] === end.value && m[1] === (end.kind === "fixed" ? "<" : "<=");
+}
+
+function knownRangeConverted(known: string, end: AddedEnd, where: string, path: string): Finding {
+  const same = knownRangeNames(known, end);
+  const seen = "Tools that read only the range events now see this end, which they did not before, so versions past it may stop being flagged.";
+  return {
+    code: "known-range-converted",
+    level: "review",
+    path,
+    message: `${KNOWN_RANGE} "${shorten(known, 80)}" was replaced by "${end.kind}" ${end.value} in ${where}: ${same ? "both name the same end" : "the two do not name the same end"}.`,
+    why: same
+      ? `${seen} Confirm the value is right.`
+      : `${seen} AdvisoryGuard does not compare versions, so check the package's release history to confirm the new end is right.`,
+    before: known,
+    after: end.value,
+  };
+}
+
+function spellingChanged(kind: EventKind, from: string, to: string, where: string, path: string, strict: boolean): Finding {
+  const fromZero = hasLeadingZero(from);
+  const toZero = hasLeadingZero(to);
+  const change = fromZero && !toZero ? ", with leading zeros removed" : !fromZero && toZero ? ", with leading zeros added" : "";
+  return {
+    code: "version-spelling-changed",
+    level: "review",
+    path,
+    message: change
+      ? `"${kind}" value changed spelling in ${where}: ${from} -> ${to}${change}. The numbers are the same.`
+      : `"${kind}" values in ${where} differ only in leading zeros: ${from} -> ${to}.`,
+    why: strict
+      ? "Semantic versions cannot have leading zeros, so a value with them may not match any published release, and tools may read the two spellings differently. Confirm the spelling that was published."
+      : "Some version schemes treat these spellings as the same version and others do not, so tools may read them differently. Confirm the spelling that was published.",
+    before: from,
+    after: to,
+  };
+}
+
 function reportBoundConversions(
   gone: Record<EventKind, string[]>,
   fresh: Record<EventKind, string[]>,
@@ -202,7 +285,20 @@ function reportBoundConversions(
   }
 }
 
-function diffRanges(b: JsonObject, a: JsonObject, label: string, path: string, out: Finding[], ctx: MatchContext): void {
+/**
+ * Compare the ranges of a pair of entries. `known` is a known range the edit removed; returns whether a
+ * range's new end was reported as replacing it, in which case the caller should not report it as removed.
+ */
+function diffRanges(
+  b: JsonObject,
+  a: JsonObject,
+  label: string,
+  path: string,
+  ecosystem: string | undefined,
+  known: string | undefined,
+  out: Finding[],
+  ctx: MatchContext,
+): boolean {
   const bs = asObjectArray(getOwn(b, "ranges"));
   const as = asObjectArray(getOwn(a, "ranges"));
   const rangeId = (r: JsonObject): string => `${textOf(getOwn(r, "type"), "?")}|${textOf(getOwn(r, "repo"), "")}`;
@@ -220,32 +316,58 @@ function diffRanges(b: JsonObject, a: JsonObject, label: string, path: string, o
     if (restB.some((bi) => idsA.has(rangeId(bs[bi] as JsonObject)))) ctx.rangesSkipped = true;
   }
 
-  for (const { bi, ai, exact } of pairs) {
+  const valuesOf = (r: JsonObject, kind: EventKind): string[] =>
+    asObjectArray(getOwn(r, "events")).map((e) => getOwn(e, kind)).filter(isString);
+  const eventDelta = (x: JsonObject, y: JsonObject): Record<EventKind, string[]> => ({
+    introduced: setMinus(valuesOf(x, "introduced"), valuesOf(y, "introduced")).sort(),
+    fixed: setMinus(valuesOf(x, "fixed"), valuesOf(y, "fixed")).sort(),
+    last_affected: setMinus(valuesOf(x, "last_affected"), valuesOf(y, "last_affected")).sort(),
+    limit: setMinus(valuesOf(x, "limit"), valuesOf(y, "limit")).sort(),
+  });
+
+  // If several ranges each gained an end, which one replaced the known range cannot be known, and
+  // array order must not decide. They are then reported as ordinary additions.
+  const candidates = known === undefined
+    ? []
+    : pairs.filter(({ bi, ai, exact }) => {
+        const rb = bs[bi];
+        const ra = as[ai];
+        return !exact && rb && ra && soleAddedEnd(eventDelta(rb, ra), eventDelta(ra, rb)) !== undefined;
+      });
+  const replacing = candidates.length === 1 ? candidates[0] : undefined;
+
+  for (const pair of pairs) {
+    const { bi, ai, exact } = pair;
     if (exact) continue;
     const rb = bs[bi];
     const ra = as[ai];
     if (!rb || !ra) continue;
     const rtype = textOf(getOwn(rb, "type"), "range");
     const where = `${rtype} range of ${label}`;
-    const valuesOf = (r: JsonObject, kind: EventKind): string[] =>
-      asObjectArray(getOwn(r, "events")).map((e) => getOwn(e, kind)).filter(isString);
-    const eventDelta = (x: JsonObject, y: JsonObject): Record<EventKind, string[]> => ({
-      introduced: setMinus(valuesOf(x, "introduced"), valuesOf(y, "introduced")).sort(),
-      fixed: setMinus(valuesOf(x, "fixed"), valuesOf(y, "fixed")).sort(),
-      last_affected: setMinus(valuesOf(x, "last_affected"), valuesOf(y, "last_affected")).sort(),
-      limit: setMinus(valuesOf(x, "limit"), valuesOf(y, "limit")).sort(),
-    });
     const goneByKind = eventDelta(rb, ra);
     const freshByKind = eventDelta(ra, rb);
     const afterNamesFixed = valuesOf(ra, "fixed").length > 0;
     const afterHasEnd = afterNamesFixed || valuesOf(ra, "last_affected").length > 0;
     reportBoundConversions(goneByKind, freshByKind, where, path, rtype === "GIT", afterNamesFixed, out);
+    if (pair === replacing && known !== undefined) {
+      const end = soleAddedEnd(goneByKind, freshByKind);
+      if (end) {
+        out.push(knownRangeConverted(known, end, where, path));
+        freshByKind[end.kind] = [];
+      }
+    }
     for (const kind of EVENT_KINDS) {
       const gone = goneByKind[kind];
       const fresh = freshByKind[kind];
       // Which of several values replaced which cannot be known without comparing versions.
       const n = gone.length === 1 && fresh.length === 1 ? 1 : 0;
       for (let k = 0; k < n; k++) {
+        const from = gone[k] as string;
+        const to = fresh[k] as string;
+        if (rtype !== "GIT" && differsOnlyInLeadingZeros(from, to)) {
+          out.push(spellingChanged(kind, from, to, where, path, rejectsLeadingZeros(ecosystem, rtype)));
+          continue;
+        }
         out.push({
           code: "range-event-changed",
           level: "review",
@@ -316,6 +438,7 @@ function diffRanges(b: JsonObject, a: JsonObject, label: string, path: string, o
       after: canonical(r),
     });
   }
+  return replacing !== undefined;
 }
 
 function diffVersions(b: JsonObject, a: JsonObject, label: string, path: string, out: Finding[]): void {
@@ -423,9 +546,56 @@ function diffPair(b: JsonObject, a: JsonObject, ai: number, out: Finding[], ctx:
   }
 
   diffSeverity(getOwn(b, "severity"), getOwn(a, "severity"), `${path}.severity`, ` (affected ${label})`, out);
-  diffRanges(b, a, label, path, out, ctx);
+  const replaced = diffRanges(b, a, label, path, pa?.ecosystem ?? pb?.ecosystem, knownRangeRemoved(b, a), out, ctx);
   diffVersions(b, a, label, path, out);
-  diffGeneric(remainder(b), remainder(a), path, out);
+  // Every other key of database_specific is still compared, so a value dropped alongside it is still reported.
+  const rest = (entry: JsonObject): JsonValue => (replaced ? withoutKnownRange(remainder(entry)) : remainder(entry));
+  diffGeneric(rest(b), rest(a), path, out);
+}
+
+/**
+ * Packages in one advisory often share a fix. Report any that had the same version data as a package
+ * this edit changed, but did not change themselves. One finding per group keeps long lists readable.
+ */
+function reportLeftOutPackages(bs: JsonObject[], as: JsonObject[], matched: { bi: number; ai: number }[], out: Finding[]): void {
+  const dataKey = (entry: JsonObject): string | null => {
+    const ranges = asArray(getOwn(entry, "ranges"));
+    const known = knownRangeOf(entry);
+    return ranges.length === 0 && known === undefined ? null : JSON.stringify([canonical(ranges), canonical(known)]);
+  };
+  const groups = new Map<string, { changed: Set<string>; same: Set<string> }>();
+  for (const { bi, ai } of matched) {
+    const b = bs[bi];
+    const a = as[ai];
+    if (!b || !a) continue;
+    const p = pkgOf(a);
+    if (!p || (!p.name && !p.ecosystem)) continue;
+    const key = dataKey(b);
+    if (key === null) continue;
+    let group = groups.get(key);
+    if (!group) {
+      group = { changed: new Set(), same: new Set() };
+      groups.set(key, group);
+    }
+    // Only the ranges count as a change: a known range note can go on its own without changing what the advisory covers.
+    const rangesChanged = canonical(asArray(getOwn(a, "ranges"))) !== canonical(asArray(getOwn(b, "ranges")));
+    (rangesChanged ? group.changed : group.same).add(labelOf(a, ai));
+  }
+  for (const key of [...groups.keys()].sort()) {
+    const group = groups.get(key);
+    if (!group || group.changed.size === 0) continue;
+    // Two entries for one package can differ on purpose, so that is not a package left out.
+    const leftOut = [...group.same].filter((l) => !group.changed.has(l)).sort();
+    if (leftOut.length === 0) continue;
+    out.push({
+      code: "sibling-package-unchanged",
+      level: "review",
+      path: `affected[${leftOut[0] as string}]`,
+      message: `Version ranges changed for ${previewList([...group.changed].sort())} but not for ${previewList(leftOut)}, which had the same version data before this edit.`,
+      why: "Packages in one advisory often share a fix. If these do, they need the same change. If they really differ, this can be allowed.",
+      after: leftOut,
+    });
+  }
 }
 
 /**
@@ -546,6 +716,7 @@ export function diffAffected(
   for (const { bi } of changed) {
     for (const f of reports.get(bi) ?? []) out.push(f);
   }
+  reportLeftOutPackages(bs, as, matched, out);
 
   if (ctx.rangesSkipped) {
     out.push({