GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||
GeezerVison is showing github.com — fix(scan,get): rank patches by severity, then merge state, then patch recency by mikolalysenko · Pull Request #147 · SocketDev/socket-patch Conversation
|
| merged patch | rival patch | winner | why |
|---|---|---|---|
| high | critical | rival | higher severity available |
| critical | high | merged | merged already covers the worst |
| high | high | merged | severities tie → breadth decides |
tier is an access filter, not a ranking keyA free critical outranks a paid low. This deliberately reverses select_paid_user_prefers_paid_over_free_same_purl, which now holds only when everything above tier ties.
Never the upstream package's release date. [email protected] shipped to npm 2023-10-26 and carries patches published 2026-03-27 and 2026-08-03.
api/ranking.rs (new) — the single comparator. Ordering is normalized at the API-client boundary, so the table, --json arrays, get's listing, the interactive prompt and detect_updates all inherit it rather than each re-deriving it.utils/date.rs (new) — parses RFC 2822, RFC 3339 and bare dates to epoch seconds. No new dependency; reuses the Hinnant calendar algorithm already in vex::time, and pins the inverse against it.selection_required JSON error. Only the presented order changed — which is what fixes scan for them too, since select_one auto-selects index 0 in non-TTY and defaults to it in a TTY.The batch endpoint omits publishedAt; by-package carries it. Ranks 1, 2 and 4 agree across both, so they diverge only when top candidates tie on severity and merge state — there the batch side falls to the UUID tiebreak while apply correctly uses the date.
Live: pkg:npm/[email protected] has two free HIGH patches; packages[0].patches[0] reports 0bc312a6… (2026-03-27) while --apply installs the newer 83f5a654… (2026-08-03), the correct choice. Only the reported ordering is affected — never which patch lands on disk. Documented in CLI_CONTRACT.md; BatchPatchInfo.published_at is already wired with #[serde(default)], so it closes once the batch endpoint emits it.
3891 pass, 0 fail. CI clippy (--workspace --all-features -D warnings) clean; changed files rustfmt-clean.
Both new rungs are mutation-checked:
| mutation | tests that die |
|---|---|
| coverage rung stubbed to a constant | 9 across core, cli, wiremock e2e |
| coverage promoted above severity (loses the "unless higher severity" rule) | 3, exactly the exception cases |
| patch-date rung stubbed to a constant | 8 across core, cli, wiremock e2e |
That exercise also caught two of my own recency tests passing vacuously — their UUIDs happened to agree with the correct answer, so the tiebreak was silently supplying the result. They now use adversarial UUIDs where the tiebreak points the wrong way.
Coverage highlights:
scan picks with multiple candidates — every prior scan fixture mocked exactly one patch per package.merged_patch_wins_when_severities_tie / a_higher_severity_patch_beats_the_merged_one / merged_patch_wins_when_it_already_covers_the_worst_advisory — the three rows of the table above.coverage_counts_advisories_not_cve_aliases + batch twin, and batch_falls_back_to_cve_ids_when_no_ghsa_is_named.scan_apply_picks_critical_over_more_recent_low_for_paid_user, the free/non-TTY twin, and scan_apply_picks_the_more_recently_published_patch_when_severity_ties.published_at_is_per_patch_not_per_package — verbatim production payload: one package version, two patches, two dates.#[ignore], verified passing):
canary_patches_name_advisories_so_merge_state_is_inferable — guards the inference signal itself. If production stopped populating vulnerabilities, coverage would collapse to 0, the merge rung would go permanently inert, and selection would fall through to recency with no error anywhere. Asserts only that the signal exists, never a count, so publishing a genuinely merged patch does not fail it — that is reported informationally.canary_published_at_is_a_patch_date_not_a_package_date — fails if the API switches publishedAt to a package-level date.utils::date suite: all 12 RFC-2822 months, zone aliases and offsets, RFC-3339 variants, rejection cases, adversarial non-ASCII input, a monotonicity sweep, and an exhaustive cross-check of the calendar inverse over ~1265 years.The manifest still holds one patch record per PURL, so a package with two applicable patches still gets only its best one ([email protected] has two free HIGH patches; applying one reverts the other — they conflict on lib/adapters/http.js). Pre-existing and tracked separately.
🤖 Generated with Claude Code
Sorry, something went wrong.
There was an error while loading. Please reload this page.
When a package has more than one available patch, `select_patches` never
consulted severity. For an authorized user (`can_access_paid == true`) the
pick was "newest paid patch, else newest patch":
group.sort_by(|a, b| b.published_at.cmp(&a.published_at));
let choice = group.iter().find(|p| p.tier == "paid").or_else(|| group.first())
so a package whose newest patch fixed a `low` got that one and its `critical`
patch was silently dropped.
The date sort was broken outright besides. `publishedAt` is RFC 2822 /
HTTP-date on the wire (`Fri, 27 Mar 2026 19:12:42 GMT`, verified live across
npm, PyPI, cargo and gem), so a raw `String::cmp` orders by *day-of-week
name* — `Fri` < `Mon` < `Sat` < `Sun` < `Thu` < `Tue` < `Wed`. Every fixture
in this repo uses ISO-8601, which is why it was never caught.
A second, independent chooser — `detect_updates` — took `.first()` off the
raw batch response with no ordering at all, so `updates[].newUuid` could name
a different patch than `--apply` installed.
The new order, best first:
1. merged patches
2. severity: critical > high > medium/moderate > low > unknown
3. patch publish date, most recent first
4. tier (paid), then uuid — tiebreaks only, so the order is total and
output is reproducible
`tier` is now an access filter, not a ranking key: a free `critical` outranks
a paid `low`. This reverses `select_paid_user_prefers_paid_over_free_same_purl`,
which now holds only when everything above tier ties.
Rank 3 is the date *the patch* was published, never the upstream package's
release date. The two are unrelated — [email protected] shipped 2023-10-26 and
carries patches published 2026-03-27 and 2026-08-03.
Implementation notes:
* `api::ranking` is the one comparator; ordering is normalized at the API
client boundary so the table, `--json` arrays, `get`'s listing, the
interactive prompt and `detect_updates` all inherit it instead of each
re-deriving it.
* `utils::date` parses RFC 2822, RFC 3339 and bare dates to epoch seconds
with no new dependency (reuses the Hinnant calendar algorithm already in
`vex::time`).
* The three duplicated severity ladders now delegate to one.
* `merged` is not emitted by any endpoint yet. It deserializes defensively
(`de_truthy_flag` + aliases) so a `mergedAt` *string* payload cannot
hard-error an entire patch-list response. Confirm the real JSON key and
prune the aliases before this ships.
* Free/unauthorized callers keep the interactive picker and the
`selection_required` JSON error; only the presented order changed — which
is what fixes `scan` for them, since `select_one` auto-selects index 0 in
non-TTY and defaults to it in a TTY.
Known gap, documented in CLI_CONTRACT.md: the batch endpoint omits
`publishedAt` while by-package carries it, so scan's *listing* and apply's
*selection* can differ when top candidates tie on merged AND severity. Only
the reported order is affected, never what lands on disk.
`BatchPatchInfo.published_at` is already wired, so this closes server-side.
Tests: the date rung is mutation-checked — stubbing it out fails 8 tests
across core, cli and the wiremock e2e. Adds a live production canary
(`canary_published_at_is_a_patch_date_not_a_package_date`) that fails if the
API ever switches `publishedAt` to a package-level date, which would silently
disable recency ranking with no error anywhere.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
…anks it
Replaces the speculative `merged` API field with an inference, and reorders
the ranking so a merged patch can never shadow a higher-severity fix.
## Merge state is inferable today
The previous commit added a `merged: bool` deserialized from a guessed key
(`merged`/`isMerged`/`mergedAt`/`upstreamMerged`) behind a type-tolerant
deserializer, because no endpoint emits one. That guesswork is unnecessary:
a merged patch is by definition one that folds several fixes into a single
blob, so it NAMES several advisories — which every endpoint already returns.
Merge state is now the count of distinct advisories a patch remediates:
`vulnerabilities` map keys on by-package/view, `ghsaIds` on batch (falling
back to `cveIds` only when no GHSA is named). Advisories are counted, not
CVE ids: one advisory routinely carries several CVE aliases, and counting
those would inflate a single-fix patch into a phantom merged one.
Removes `merged`, `de_truthy_flag` and `is_false` — no speculative API
surface, nothing to confirm before merge, and no server change needed.
Surveyed production 2026-08-05: all 28 patches sampled across
npm/PyPI/gem/cargo cover exactly one advisory, so the rung is inert today
and ranking falls through to recency. Also confirmed the patches for a
package are built against a shared pristine baseline (identical
`beforeHash`) and conflict rather than chain — there is no merged patch to
find yet, not a detection failure.
## Severity now outranks merge state
Order was merged -> severity -> recency; it is now:
1. severity 2. merge state 3. patch recency 4. tier, uuid
The merged patch stays the general preference — breadth is what an operator
wants when only one patch per PURL can be applied — but it must not shadow
a worse vulnerability. Because a patch's severity is the worst advisory it
fixes, putting severity on top expresses that exactly:
| merged | rival | winner | why |
| high | critical | rival | higher severity available |
| critical | high | merged | merged covers the worst |
| high | high | merged | tie -> breadth decides |
## Testing
Both rungs are mutation-checked:
* coverage rung stubbed to a constant -> 9 tests fail across core, cli
and the wiremock e2e;
* coverage promoted above severity (losing the "unless higher severity"
rule) -> 3 tests fail, exactly the ones expressing the exception.
An earlier mutation run also exposed two recency tests passing vacuously
(their UUIDs agreed with the correct answer); they now use adversarial
UUIDs where the tiebreak points the wrong way.
Adds a live production canary,
`canary_patches_name_advisories_so_merge_state_is_inferable`, guarding the
inference signal itself: if production ever stopped populating
`vulnerabilities`, coverage would collapse to 0, the merge rung would go
permanently inert, and selection would fall through to recency with no
error anywhere. It asserts only that the signal exists (>= 1 advisory per
patch), never a count, so publishing a genuinely merged patch does not fail
it — that case is reported informationally instead.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.