Repository navigation
Description
.github/scripts/vuln-gate.mjs on main carries three defects. The first is a
fail-open: the gate reports success while a critical advisory is present.
This script came from the same lineage as mforce/repo-template,
which is now public and has all three fixed with mutation checks. Everything
below links to real upstream code — port the logic, do not re-derive it.
Part 1 — the defects (required)
P2 — an unknown --level makes the gate stop gating
gate() computes its floor with severityRank(level). That function
deliberately ranks an unrecognised string above critical, which is right
for a finding (a tool inventing a new severity must still block) and exactly
backwards for the threshold: an inflated floor is unreachable, so every advisory
reads as "below threshold".
Reproduced on main:
$ printf '{"auditReportVersion":2,"vulnerabilities":{"m":{"name":"m","severity":"critical","via":[{"source":1,"name":"m","severity":"critical","title":"x","url":"https://github.com/advisories/GHSA-93q8-gq69-wqmw"}]}}}' \
| node .github/scripts/vuln-gate.mjs --ecosystem npm --level hihg --exceptions /nonexistent
[npm] no advisories at or above "hihg" (0 excepted, 1 below threshold).
exit=0
A critical advisory, a green run, no warning — from a typo.
A finding's severity comes from a tool we do not control, so unknown must sort
high. The threshold is one of our own strings, so unknown is a bad
invocation and must be refused.
Upstream fix: levelProblem —
validated in main() (exit 2) and thrown from
gate(), with the floor computed by
SEVERITIES.indexOf so the
wrong-function mistake cannot recur. Validating only in main() leaves the
landmine in the exported API for the next caller — that residual is
filed against collectify, do
not repeat it here.
P3 — a lapsed "ANY" exception never gets its "remove me" warning
vuln-gate.mjs:191:
const inScope = (e) => e.ecosystem === "any" || e.ecosystem.toLowerCase() === ecosystem;Suppression (matches) lower-cases the scope; this does not. An exception
written "ecosystem": "ANY" correctly stops suppressing once it lapses, but is
never reported stale — it lingers as dead config nobody is told to delete, which
is the entire point of the staleness warning.
Upstream fix: one scopeOf helper
used by both, so the two cannot disagree again.
P3 — a corrupt exceptions file is swallowed silently
vuln-gate.mjs:298:
function loadExceptions(path) {
try {
return JSON.parse(readFileSync(path, "utf8")).exceptions ?? [];
} catch {
return []; // no file → empty allowlist, the normal case
}
}The catch cannot tell "no file" (normal) from "the file is corrupt". Returning
[] is the safe direction — nothing is suppressed — but silently: every entry
someone believes is muting an advisory is doing nothing, with no signal. It also
contradicts the documented "reported as a warning" contract.
Upstream fix: loadExceptions —
only ENOENT stays quiet; a parse failure, an unreadable file, or an
exceptions key that is not an array each warn and return []. Without the
array check a wrong-shaped file crashes with a raw TypeError from
exceptions.filter instead of a reason.
Definition of done
-
--level hihgexits 2; a critical advisory at the default level still exits 1. - The
--levelguard is end to end, spawning the real CLI —
example, helper at
runCli. Every unit here
behaves correctly on its own; only the assembled exit code shows the hole,
so a unit test of the predicate does not pin the regression. This was
measured upstream: deleting the whole fix left the sibling repo's suite
28/28 green. - Mutation-checked per
docs/decisions/guard rules: green baseline, each
mutant red on its own named assertion, baseline green after restoring.
Part 2 — alignment with the template (optional, separate PRs)
Do not blind-copy the upstream file over this one. The two have drifted 175
lines and cluckwork's 26 tests are written against its own shape:
| cluckwork | repo-template | |
|---|---|---|
| ecosystem dispatch | ECOSYSTEMS hardcoded (vuln-gate.mjs:35) + a ternary at :339 |
a PARSERS registry; ECOSYSTEMS derived from it |
| adding an ecosystem | edits in 3+ places | one entry in PARSERS |
| threshold predicate | none | levelProblem (reason string or null) |
loadExceptions |
private | exported, takes an injectable warn |
Port Part 1 as a minimal diff against cluckwork's existing shape. Adopting
the PARSERS registry is a legitimate follow-up, but it is a refactor and
belongs in its own PR — not bundled with a security fix.
Template files cluckwork does not have, if you want them (each its own PR, none
blocking this issue):
.gitattributes—* text=auto eol=lf. The highest-value one: cluckwork's
.githooks/*are POSIXshrun via their shebang, and a CRLF checkout makes
the kernel look for/bin/shwith a trailing CR, failing with a message that
names neither the hook nor the cause.actionlint+shellcheckinpre-commit— cluckwork's has neither, and the
repo carries a lot of workflow and shell. Warns and continues when the tools
are absent, so it is safe to add.SECURITY.md,CONTRIBUTING.md,.github/pull_request_template.md,
.editorconfig— absent here..github/CODEOWNERS— the only in-repo lever on "a self-merge to.github/
rewrites what every gate means". Ships inert upstream on purpose: an entry
naming an unresolvable user is silently ignored by GitHub, so a
plausible-looking broken file is worse than an obviously empty one.
Activity
[-]vuln-gate: a typo'd --level opens the gate (fail-open), plus two hygiene defects[/-][+]vuln-gate: a typo'd --level opens the gate (fail-open) — port the fixed logic from repo-template[/+]mforce commentedon Aug 31, 2026
Filed the follow-up this issue named: #634 — replacing vuln-gate's four hand-dispatched ecosystem sites with the
PARSERSregistry.Recorded here because that sentence ("a legitimate follow-up, but it is a refactor and belongs in its own PR") had no issue behind it, and a closed issue is not a place a follow-up survives.
For the record, the other residual in this body is already placed and #634 explicitly leaves it alone: the
gate()-level--levelvalidation is filed at collectify#117. Worth noting the precise behaviour, since "fail-open" in this issue's title refers to themain()path that was fixed here — an unvalidatedlevelreaching the exportedgate()yieldsfloor = -1atvuln-gate.mjs:198, so every finding ranks at or above it and the gate blocks everything. Wrong, but fail-closed.chore(ci): replace vuln-gate's four hand-dispatched ecosystem sites w…