Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ This changes finding codes and allow-list coverage. The cases come from running
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 `known-range-removed` finding: `last_known_affected_version_range` removed while the ranges still have an
end. This was `field-removed`, which suggested the value may have been dropped by accident, although the end
had usually moved too. It still needs review, and it says whether the end also changed. With no end left it
stays `field-removed`. `--allow 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
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. 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. |
| 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. Removing that note while the ranges still have an end = review (`known-range-removed`), which says whether the end also changed. With no end left it is an ordinary removed field. 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`. |
Expand Down
40 changes: 38 additions & 2 deletions src/core/diff/affected.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,33 @@ function knownRangeConverted(known: string, end: AddedEnd, where: string, path:
};
}

/** Every end in an entry's ranges, sorted and without repeats so that array order cannot decide anything. */
function endsOf(entry: JsonObject): string[] {
const ends = new Set<string>();
for (const range of asObjectArray(getOwn(entry, "ranges"))) {
for (const event of asObjectArray(getOwn(range, "events"))) {
for (const kind of ["fixed", "last_affected"] as const) {
const value = getOwn(event, kind);
if (isString(value)) ends.add(`"${kind}" ${value}`);
}
}
}
return [...ends].sort();
}

function knownRangeNoteRemoved(known: string, ends: string[], endMoved: boolean, label: string, path: string): Finding {
return {
code: "known-range-removed",
level: "review",
path: `${path}.database_specific`,
message: `${KNOWN_RANGE} "${shorten(known, 80)}" was removed from ${label}, whose ranges still end at ${previewList(ends)}.`,
why: endMoved
? "The end also changed in this edit, so a note about the old end may no longer apply. Confirm the new end is right, and that the note recorded nothing else."
: "The ranges keep the same end, so tools that read only the range events are not affected. Confirm the note recorded nothing the end does not say.",
before: known,
};
}

function spellingChanged(kind: EventKind, from: string, to: string, where: string, path: string, strict: boolean): Finding {
const fromZero = hasLeadingZero(from);
const toZero = hasLeadingZero(to);
Expand Down Expand Up @@ -546,10 +573,19 @@ function diffPair(b: JsonObject, a: JsonObject, ai: number, out: Finding[], ctx:
}

diffSeverity(getOwn(b, "severity"), getOwn(a, "severity"), `${path}.severity`, ` (affected ${label})`, out);
const replaced = diffRanges(b, a, label, path, pa?.ecosystem ?? pb?.ecosystem, knownRangeRemoved(b, a), out, ctx);
const known = knownRangeRemoved(b, a);
const replaced = diffRanges(b, a, label, path, pa?.ecosystem ?? pb?.ecosystem, known, out, ctx);
diffVersions(b, a, label, path, out);
let reported = replaced;
const endsAfter = endsOf(a);
// With no end left, the note may be all that said where the range stops, so that stays an ordinary removed field.
if (known !== undefined && !replaced && endsAfter.length > 0) {
const endMoved = JSON.stringify(endsOf(b)) !== JSON.stringify(endsAfter);
out.push(knownRangeNoteRemoved(known, endsAfter, endMoved, label, path));
reported = true;
}
// 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));
const rest = (entry: JsonObject): JsonValue => (reported ? withoutKnownRange(remainder(entry)) : remainder(entry));
diffGeneric(rest(b), rest(a), path, out);
}

Expand Down
86 changes: 83 additions & 3 deletions test/real-edits.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ describe("a known range replaced by an end event", () => {

it("does not apply when an end was also removed", () => {
const r = diff(record(entry("a:lib", lastAt("2.0.0"), { known: "<= 2.0.0" })), record(entry("a:lib", fixedAt("2.1.0"))));
expect(reviewCodes(r).sort()).toEqual(["field-removed", "range-bound-converted"]);
expect(reviewCodes(r).sort()).toEqual(["known-range-removed", "range-bound-converted"]);
});

it("does not apply when the known range is not a string", () => {
Expand All @@ -155,11 +155,91 @@ describe("a known range replaced by an end event", () => {
record({ package: { ecosystem: "Maven", name: "a:lib" }, ranges, ...(known ? { database_specific: { [KNOWN]: known } } : {}) } as JsonObject);
const r1 = diff(mk([rangeA, rangeB], "< 6.0.0"), mk([endA, endB]));
const r2 = diff(mk([rangeB, rangeA], "< 6.0.0"), mk([endB, endA]));
expect(reviewCodes(r1).sort()).toEqual(["field-removed", "range-event-added", "range-event-added"]);
expect(reviewCodes(r1).sort()).toEqual(["known-range-removed", "range-event-added", "range-event-added"]);
expect(reviewCodes(r2).sort()).toEqual(reviewCodes(r1).sort());
});
});

describe("a known range removed while the range keeps an end", () => {
const before = record(entry("a:lib", fixedAt("8.6.1"), { known: "<= 8.6.0" }));
const after = record(entry("a:lib", fixedAt("8.6.1")));

it("is its own review finding, without the dropped by accident wording", () => {
const r = diff(before, after);
expect(reviewCodes(r)).toEqual(["known-range-removed"]);
expect(codes(r)).not.toContain("field-removed");
const f = r.findings[0];
expect(f?.before).toBe("<= 8.6.0");
expect(f?.message).toContain("<= 8.6.0");
expect(f?.message).toContain("fixed");
expect(f?.message).toContain("8.6.1");
expect(f?.why).not.toContain("accident");
expect(f?.why).toContain("same end");
});

it("does not loosen the exit code", () => {
expect(diff(before, after).status).toBe("review");
expect(cliDiff(before, after).code).toBe(1);
});

it("says so when the end also moved", () => {
const r = diff(record(entry("a:lib", fixedAt("8.0.32"), { known: "<= 8.0.31" })), record(entry("a:lib", fixedAt("8.0.33"))));
expect(reviewCodes(r).sort()).toEqual(["known-range-removed", "range-event-changed"]);
const f = r.findings.find((x) => x.code === "known-range-removed");
expect(f?.why).toContain("also changed");
expect(f?.why).not.toContain("same end");
});

it("does not call a reordered range a moved end", () => {
const events: JsonObject[] = [{ introduced: "0" }, { fixed: "1" }, { introduced: "2" }, { last_affected: "3" }];
const r = diff(record(entry("a:lib", events, { known: "<= 3" })), record(entry("a:lib", [...events].reverse())));
expect(r.findings.find((x) => x.code === "known-range-removed")?.why).toContain("same end");
});

it("works with last_affected as the end", () => {
const r = diff(record(entry("a:lib", lastAt("2"), { known: "< 3" })), record(entry("a:lib", lastAt("2"))));
expect(reviewCodes(r)).toEqual(["known-range-removed"]);
expect(r.findings[0]?.message).toContain("last_affected");
});

it("stays a dropped field when no end remains, because that really can lose data", () => {
const r = diff(record(entry("a:lib", OPEN, { known: "< 2" })), record(entry("a:lib", OPEN)));
expect(reviewCodes(r)).toEqual(["field-removed"]);
expect(r.findings[0]?.why).toContain("accident");
});

it("does not replace the conversion finding when the note became the end", () => {
const r = diff(record(entry("a:lib", OPEN, { known: "< 2" })), record(entry("a:lib", fixedAt("2"))));
expect(reviewCodes(r)).toEqual(["known-range-converted"]);
});

it("stays quiet when the note stays", () => {
const r = diff(before, record(entry("a:lib", fixedAt("8.6.1"), { known: "<= 8.6.0" })));
expect(codes(r)).toEqual([]);
});

it("treats a note that is not a string as an ordinary dropped field", () => {
const r = diff(record(entry("a:lib", fixedAt("2"), { known: 5 })), record(entry("a:lib", fixedAt("2"))));
expect(reviewCodes(r)).toEqual(["field-removed"]);
});

it("needs its own code in an allow list, which does not hide other removed fields", () => {
expect(diff(before, after, ["known-range-removed"]).status).toBe("clean");
expect(diff(before, after, ["field-removed"]).status).toBe("review");
expect(cliDiff(before, after, ["--allow", "known-range-removed"]).code).toBe(0);
const b = record(entry("a:lib", fixedAt("2"), { extra: { database_specific: { [KNOWN]: "< 2", note: "kept" } } }));
expect(diff(b, record(entry("a:lib", fixedAt("2"))), ["known-range-removed"]).status).toBe("review");
});

it("still reports another key dropped from database_specific", () => {
const b = record(entry("a:lib", fixedAt("2"), { extra: { database_specific: { [KNOWN]: "<= 1", note: "kept" } } }));
const r = diff(b, record(entry("a:lib", fixedAt("2"))));
expect(reviewCodes(r).sort()).toEqual(["field-removed", "known-range-removed"]);
const removed = r.findings.find((f) => f.code === "field-removed");
expect(removed?.before).toEqual({ note: "kept" });
});
});

describe("packages left out of a version change", () => {
it("names the package that kept the old range", () => {
const r = diff(
Expand Down Expand Up @@ -228,7 +308,7 @@ describe("packages left out of a version change", () => {
record(entry("a:one", fixedAt("2")), entry("a:two", fixedAt("2"), { known: "<= 1" })),
);
expect(codes(r)).not.toContain("sibling-package-unchanged");
expect(reviewCodes(r)).toEqual(["field-removed"]);
expect(reviewCodes(r)).toEqual(["known-range-removed"]);
});

it("counts a package whose note changed but whose ranges did not as left out", () => {
Expand Down
Loading