Skip to content

chore(deps): refresh brace-expansion to patched releases across all three majors - #548

Closed
mobmob719 wants to merge 3 commits into
bytefolk:mainfrom
mobmob719:chore/lockfile-brace-expansion
Closed

mobmob719 wants to merge 3 commits into
bytefolk:mainfrom
mobmob719:chore/lockfile-brace-expansion

Conversation

@mobmob719

@mobmob719 mobmob719 commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

This PR does not turn the Node 24 legs green — but it does drop the high count

npm run security:check runs npm audit --audit-level=high. Before this change the audit listed 4 findings: 2 moderate + 2 high:

  • brace-expansion carries three advisories — GHSA-q2hr-2g5m-vwhr (moderate, quadratic-time expansion of the {a},b} rewrite) plus GHSA-qhr7-859c-m2p7 and GHSA-6j4f-fj2g-mc7p (both high, uncontrolled recursion in nested brace groups / parseCommaParts causing stack exhaustion).
  • electron GHSA-qmv3-fv6v-rmhq (high, needs 43.7.6, outside the stated dependency range).

This PR refreshes brace-expansion to patched releases, which closes all three of its advisories — the two high ones included — so the high count moves 2 → 1 and the only high left is electron:

  • GHSA-qmv3-fv6v-rmhq — electron, high, needs 43.7.6, outside the stated dependency range.

So expect the Node 24 legs (ubuntu / macos) to stay red until the electron version policy is decided. That is the intended outcome here: every finding that can be closed without a version decision is now closed, and what remains needs a human call rather than a lockfile refresh. See #547.

Summary

Refresh brace-expansion to patched releases across all three major lines present in package-lock.json:

Line From To Dependents
1.x 1.1.18 1.1.21 @electron/asar, dir-compare, glob
2.x 2.1.4 2.1.7 @electron/universal, filelist
5.x 5.0.9 5.0.12 direct

Advisory GHSA-q2hr-2g5m-vwhr covers < 1.1.21, >= 2.0.0 < 2.1.7, >= 3.0.0 < 3.0.9, >= 4.0.0 < 5.0.12. This repo has no 3.x or 4.x entries. The bug is quadratic-time expansion of the {a},b} rewrite causing CPU denial of service — glob and filelist sit on test and packaging paths.

All three are semver-compatible patch bumps. package.json is unchanged; every edit is inside package-lock.json and touches only version / resolved / integrity on the six brace-expansion entries (18 insertions, 18 deletions).

Closes #547

A note on how the lockfile was edited

The obvious route is npm audit fix --package-lock-only, and it produces these same six version bumps. It also rewrites one unrelated line: the recorded vitest range for ../design-system, an out-of-tree workspace dependency, from ^3.2.4 to ^5.0.0.

That hunk is arguably correct — ../design-system/package.json already declares ^5.0.0 — but it is someone else's dependency record, it is not what this PR is for, and carrying it here would make the diff of a security refresh harder to audit. The six entries were therefore applied directly from the advisory's published first_patched_version values and their registry integrity digests, leaving that line untouched.

Explicitly left alone:

  • electron — needs 43.7.6, outside the declared range. Version policy; tracked separately.
  • vitest / @vitest/mocker — GHSA-82fw-gwwq-j7x9, moderate, needs vitest@5.0.2. Breaking; fold into the next deliberate dependency round. Note it is not currently blocking, since --audit-level=high does not gate on moderate findings.

Verification

  • Every brace-expansion entry resolves to a version outside the advisory's vulnerable range: 1.1.21, 2.1.7, 5.0.12.
  • No package.json changed; diff is confined to package-lock.json, 18 insertions / 18 deletions.
  • git diff --check passed; LF endings preserved.
  • Version and integrity values were taken from the advisory and the registry, not hand-derived.
  • CI performs npm ci, so the resolved tree is validated against the edited lockfile there.

Release note / limitation

No runtime or API change — dependency-only. The local environment runs against registry.npmmirror.com, which does not implement the audit endpoint, so npm audit cannot be executed here; the before/after figures come from CI's own security:check step and from the advisory metadata fetched over the GitHub API. Because electron still trips the high threshold, this PR should be judged by whether it closes everything that can be closed without a version decision, not by whether the Node 24 legs turn green.

@mobmob719

Copy link
Copy Markdown
Contributor Author

Windows 那条腿重跑通过了:Unpacked staging smoke / Windows x64 已转绿。

原始红灯是 prepare:electron:staging 里 node node_modules/electron/install.js 抛 TypeError: fetch failed——下 electron 二进制时网络抖动,与本次 lockfile 改动无关,重跑即过。

fork 账号在 upstream 没有 rerun 权限,用一个空提交把 CI 重新触发了一轮来重跑,PR 上会多一个 ci 提交,内容无变化。

@PeterGuy326 PeterGuy326 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed. Clean, surgical, and correctly scoped — my only merges are the six brace-expansion entries.

What this does

  • Refreshes brace-expansion across the three lines actually present in package-lock.json (1.x -> 1.1.21, 2.x -> 2.1.7, 5.x -> 5.0.12). All three are semver-compatible patch bumps and all land outside the advisories' vulnerable ranges.
  • Diff is confined to package-lock.json, exactly 18 insertions / 18 deletions, touching only version / resolved / integrity on the six brace-expansion entries. package.json is unchanged.

Why the manual lockfile edit is the right call

  • Skipping npm audit fix --package-lock-only avoids carrying an unrelated ../design-system vitest range rewrite (^3.2.4 -> ^5.0.0) inside a security refresh. Keeping that line out makes this diff auditable as a pure dependency patch, and the version/integrity values were taken from the advisory's first_patched_version and the registry rather than hand-derived.
  • CI runs npm ci, so the edited tree is validated on the runner.

Current CI state

  • With #543 now on main (electron 43.7.6), the electron advisory GHSA-qmv3-fv6v-rmhq is already closed. This PR closes the remaining brace-expansion advisories, so after this lands the npm audit --audit-level=high gate should be fully green on both Node 24 legs. I verified Node 24 on ubuntu and macos pass on the current head.

Left alone on purpose, agreed with the body

  • vitest / @vitest/mocker (GHSA-82fw-gwwq-j7x9, moderate) — needs vitest@5.0.2, a breaking change; correct to defer to a deliberate dependency round, and --audit-level=high does not gate on moderate findings.

Approving. This also lets the fork PRs #536 and #540 that were blocked on the audit gate proceed on their own merits (they have a separate structural CodeQL block, reported separately).

One process note for the author, non-blocking: this repository does not enforce the nine-section governance body that digital-employee does, so the Closes #547 here is fine.

@PeterGuy326

Copy link
Copy Markdown
Contributor

Closing as already incorporated by current main (#543). The current PR files list is empty, and package-lock.json has the identical Git blob 4868de69c24889de6f24004c9f104cd266343df5 on main and this head. There is no remaining dependency diff to merge.

@PeterGuy326 PeterGuy326 closed this Oct 4, 2026
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.

security: refresh brace-expansion to patched releases across all three major lines

2 participants