Skip to content

ci: excuse the unfixable extract-zip advisory by id instead of by hand - #192

Open
Sev7eNup wants to merge 1 commit into
mainfrom
ci/desktop-audit-advisory-allowlist
Open

ci: excuse the unfixable extract-zip advisory by id instead of by hand#192
Sev7eNup wants to merge 1 commit into
mainfrom
ci/desktop-audit-advisory-allowlist

Conversation

@Sev7eNup

Copy link
Copy Markdown
Owner

The desktop job went red on a newly reported advisory: GHSA-jmr9-qjv8-65gv, extract-zip unvalidated symlink path traversal, high. It reaches this package only through Electron Forge's dev tree, fans out across 15 packages of that chain, and has no patched release at all (first_patched_version: null) — so neither an overrides pin nor npm audit fix can clear it, and it will fail every future PR until Forge drops the dependency.

Both blunt escapes destroy the check rather than narrow it:

  • --omit=dev audits nothing here. dependencies is empty and electron itself is a devDependency, because Forge requires that. Omitting dev dependencies would silently stop reporting Electron CVEs — the one package in this tree that reaches users.
  • --audit-level=critical would wave through every future high advisory.

So the audit stays full and single advisories are excused by id. scripts/audit-gate.mjs folds npm audit --json into one entry per advisory, fails on anything at moderate or above that is not on a reviewed allowlist, and prints a stale line when an entry stops matching — the signal to delete it. Every entry carries the reason it exists and the condition for dropping it again.

Verified locally: the gate excuses exactly the one advisory and exits 0; 86 tests pass (4 files, up from 3); typecheck clean.

The desktop job went red on a new npm audit finding: GHSA-jmr9-qjv8-65gv,
extract-zip unvalidated symlink path traversal, high. It reaches this package
only through Electron Forge's dev tree, it fans out across 15 packages of that
chain, and it has no patched release at all — first_patched_version is null, so
neither an `overrides` pin nor `npm audit fix` can clear it.

Both blunt escapes destroy the check rather than narrow it:

  - `--omit=dev` audits nothing here. `dependencies` is empty and electron
    itself is a devDependency, because Forge requires that. Omitting dev
    dependencies would silently stop reporting Electron CVEs — the one package
    in this tree that reaches users.
  - `--audit-level=critical` would wave through every future high advisory.

So the audit stays full and single advisories are excused by id. scripts/
audit-gate.mjs folds `npm audit --json` into one entry per advisory, fails on
anything at moderate or above that is not on a reviewed allowlist, and prints a
`stale` line when an allowlist entry stops matching, which is the signal to
delete it. Each entry carries the reason it exists and the condition for
dropping it again.

The gate's decision logic is pure and tested alongside the rest of the shell's
pure logic; vitest's include now covers scripts/ as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant