.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
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 POSIX sh run via their shebang, and a CRLF checkout makes
the kernel look for /bin/sh with a trailing CR, failing with a message that
names neither the hook nor the cause.
actionlint + shellcheck in pre-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.
.github/scripts/vuln-gate.mjsonmaincarries three defects. The first is afail-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
--levelmakes the gate stop gatinggate()computes its floor withseverityRank(level). That functiondeliberately ranks an unrecognised string above
critical, which is rightfor 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: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 fromgate(), with the floor computed bySEVERITIES.indexOfso thewrong-function mistake cannot recur. Validating only in
main()leaves thelandmine 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" warningvuln-gate.mjs:191:Suppression (
matches) lower-cases the scope; this does not. An exceptionwritten
"ecosystem": "ANY"correctly stops suppressing once it lapses, but isnever reported stale — it lingers as dead config nobody is told to delete, which
is the entire point of the staleness warning.
Upstream fix: one
scopeOfhelperused by both, so the two cannot disagree again.
P3 — a corrupt exceptions file is swallowed silently
vuln-gate.mjs:298:The
catchcannot tell "no file" (normal) from "the file is corrupt". Returning[]is the safe direction — nothing is suppressed — but silently: every entrysomeone 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
ENOENTstays quiet; a parse failure, an unreadable file, or anexceptionskey that is not an array each warn and return[]. Without thearray check a wrong-shaped file crashes with a raw
TypeErrorfromexceptions.filterinstead of a reason.Definition of done
--level hihgexits 2; a critical advisory at the default level still exits 1.--levelguard is end to end, spawning the real CLI —example, helper at
runCli. Every unit herebehaves 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.
docs/decisions/guard rules: green baseline, eachmutant 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:
ECOSYSTEMShardcoded (vuln-gate.mjs:35) + a ternary at:339PARSERSregistry;ECOSYSTEMSderived from itPARSERSlevelProblem(reason string ornull)loadExceptionswarnPort Part 1 as a minimal diff against cluckwork's existing shape. Adopting
the
PARSERSregistry is a legitimate follow-up, but it is a refactor andbelongs 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 makesthe kernel look for
/bin/shwith a trailing CR, failing with a message thatnames neither the hook nor the cause.
actionlint+shellcheckinpre-commit— cluckwork's has neither, and therepo 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.