fix(deps): patch browserslist to 4.28.8 to clear the HIGH advisory blocking every PR - #501
Conversation
…ocking every PR The dependency advisory gate has been failing on every open pull request, including #498, which changes only two submodule gitlinks. Two HIGH advisories against browserslist had no allowlist entry: GHSA-c83g-rgw3-j3cx unbounded memory growth (no cache eviction) via distinct query results, leading to eventual OOM GHSA-73wf-gq98-2v4g uncaught crash / prototype write via untrusted browserslist-stats.json custom stats (normalizeStats) ## Not caused by any open PR Three independent lines of evidence: - #498 fails identically and changes only two submodule SHAs. A pointer commit cannot introduce a dependency advisory. - Every affected branch has a package-lock.json byte-identical to dev's. - dev's last green run is 2026-08-22; these runs are 2026-09-02. Same dependency tree, opposite result, eleven days apart. The gate queries a live advisory database, so the advisories were published in that window. ## Upgrade, not an allowlist entry The gate offers both remedies. Upgrade is the right one here: the vulnerable range is <=4.28.6 and 4.28.8 is published, so this is a PATCH bump inside the same minor, not a tolerated exposure. An allowlist entry would have documented around a fix that exists. The reachability argument would have been easy to write -- browserslist resolves through exactly one path, @vitejs/plugin-react -> @babel/core -> @babel/helper-compilation-targets, which npm marks dev, and neither advisory is reachable without attacker-controlled input to a build tool. That is precisely why an entry would have been the wrong call: the allowlist is for advisories we choose to CARRY, and there is no reason to carry this one. package.json is untouched; only the lockfile resolution moved. Footprint is browserslist and its own data packages: browserslist 4.28.2 -> 4.28.8 caniuse-lite 1.0.30001790 -> 1.0.30001810 electron-to-chromium 1.5.344 -> 1.5.420 node-releases 2.0.38 -> 2.0.54 update-browserslist-db 1.2.3 -> 1.3.2 ## Verification check-deps-audit.ts PASS -- 11 advisory packages, 11 allowlisted, 0 unexpected npm run build pass npm run build:frontend pass (this is the build that actually consumes browserslist, via autoprefixer targets, so the data-package bumps are exercised rather than merely installed) npm test 8410 tests, 8384 pass, 0 fail, 26 skipped One attribution note for anyone reproducing the test run: a first pass showed 10 failures, all of them skill-file tests reading skills_ref (WAIS-001..004, EMP-008, P37-CU-010, and others). The submodule was checked out on a stale local branch predating the jaw- rename, so those tests were looking for jaw-* paths that did not exist in that tree. Aligning skills_ref to the SHA dev records turns the same two files from 0/6 to 6/6, and the full suite to 0 fail. Nothing to do with this change. Co-authored-by: Cursor <cursoragent@cursor.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b4dcbfa67a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "version": "4.28.2", | ||
| "resolved": "https://registry.npmjs.org/browserslist/-/browserslist-4.28.2.tgz", | ||
| "integrity": "sha512-48xSriZYYg+8qXna9kwqjIVzuQxi+KYWp2+5nCYnYKPTr0LvD89Jqk2Or5ogxz0NUMfIjhh2lIUX/LyX9B4oIg==", | ||
| "version": "4.28.8", |
There was a problem hiding this comment.
When desktop dependencies are installed, this upgrade has no effect on that tree: .github/workflows/desktop-release.yml runs npm ci --prefix electron, while electron/package-lock.json still pins browserslist 4.28.2, which is within the commit's cited vulnerable range (<=4.28.6). The root advisory gate becomes green, but the desktop build continues installing the vulnerable copy, so regenerate or update the Electron lockfile as part of this remediation.
Useful? React with 👍 / 👎.
020 was rewritten: the advisory blocker is fixed by upgrade in #501 rather than carried, and the doc now records why the allowlist entry was the wrong instrument despite being the easier one to write. devlog 382c004 -> 79e6f4e Co-authored-by: Cursor <cursoragent@cursor.com>
1 participant
Summary
The dependency advisory gate has been failing on every open pull request on this repository — including #498, which changes only two submodule gitlinks, and pre-existing #473. Two HIGH advisories against
browserslisthad no allowlist entry:browserslist-stats.jsoncustom stats (normalizeStats)This was not caused by any open PR
Three independent lines of evidence, because "CI went red" invites the assumption that someone's change did it:
package-lock.jsonbyte-identical todev's (git diff --quiet origin/dev HEAD -- package-lock.json).dev's last green run is 2026-08-22; these runs are 2026-09-02. Same dependency tree, opposite result, eleven days apart. The gate queries a live advisory database, so the advisories were published in that window.Why upgrade rather than an allowlist entry
The gate offers both remedies: "Either upgrade the package, or add an entry to
scripts/audit-allowlist.jsonstating why the vulnerable path is unreachable."Upgrade is the right one. The vulnerable range is
<=4.28.6and 4.28.8 is published, so this is a patch bump inside the same minor — not a tolerated exposure.The reachability argument would have been easy to write:
browserslistresolves through exactly one path,@vitejs/plugin-react→@babel/core→@babel/helper-compilation-targets, whichnpm explainmarksdev, and neither advisory is reachable without attacker-controlled input to a build tool. That is precisely why an entry would have been the wrong call —audit-allowlist.jsonsays an entry "is a decision, not a mute," and it is for advisories we choose to carry. There is no reason to carry one that has a published patch.package.jsonis untouched; only the lockfile resolution moved. The footprint isbrowserslistplus its own data packages:Verification
npx tsx scripts/check-deps-audit.tsnpm run buildnpm run build:frontendbrowserslist(autoprefixer targets), so the data-package bumps are exercised rather than merely installednpm testAttribution note for anyone reproducing the test run. A first pass showed 10 failures, all of them skill-file tests reading
skills_ref(WAIS-001..004,EMP-008,P37-CU-010, and others). The submodule was checked out on a stale local branch predating thejaw-rename, so those tests were looking forjaw-*paths absent from that tree. Aligningskills_refto the SHAdevrecords turns the same two files from 0/6 to 6/6, and the full suite to 0 fail. Unrelated to this change, but worth recording so the number is not mistaken for a regression later.Security review requested
AGENTS.mdreserves dependency changes for explicit security review, so this is deliberately a separate PR rather than folded into the skill or model-catalog stacks. It touches no credential, auth, or quota path — lockfile only.Checklist
devnpm run buildrunnpm run build:frontendrunnpm testrun (a lockfile change is repo-wide, so the scoped default does not apply): 0 failurespackage.jsonunchanged — no new or removed dependency declarationAGENTS.md(dependency change)Made with Cursor