Repository navigation
fix: move Node and Bun past the Brotli decoder advisory - #357
Conversation
GHSA-9v8h-qqgm-4m35 (CVE-2026-111497) covers the Brotli decoder in 1.0.3 through 1.2.0, which both runtimes ship. Node fixed it in 22.23.4, 24.22.1, and 26.12.1. Bun 1.3.14 vendors Brotli 1.1.0 and 1.4.3 still ships 1.2.0, so 1.4.4, with Brotli 1.2.1, is the first fixed Bun. .nvmrc pins 24.22.1, since setup-node otherwise reuses whatever 24.x the runner has cached. Bun moves to 1.4.4 in packageManager, every workflow, the root README, and the create-sei template. mcp-server, which reaches the decoder through fetch responses, now declares ^22.23.4 || ^24.22.1 || >=26.12.1. bun.lock is unchanged: Bun 1.4.4 accepts it under --frozen-lockfile. Two checks needed changes under Bun 1.4.4. Its rejects matcher never settles a Streamable HTTP stop() that has to close a live request, so that test awaits the rejection directly. The consumer verifier's edge check strictly compared an object created in a vm realm, which 1.3.14 allowed and 1.4.4 rejects the way Node does, so it compares a copy. Co-authored-by: Cursor <cursoragent@cursor.com>
PR SummaryMedium Risk Overview Bun is bumped from 1.3.14 → 1.4.4 in root Node for development/publishing is pinned to 24.22.1 in Test/script fixes for Bun 1.4.4: streamable HTTP transport test avoids Patch changesets for Reviewed by Cursor Bugbot for commit bda9a50. Bugbot is set up for automated code reviews on this repo. Configure here. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #357 +/- ##
==========================================
- Coverage 97.82% 96.53% -1.30%
==========================================
Files 81 81
Lines 5652 5563 -89
==========================================
- Hits 5529 5370 -159
- Misses 123 193 +70
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR moves the Bun and Node runtimes past the Brotli advisory. The Bun pin and Node floor are bumped together across the workflows, packageManager, the READMEs and the mcp-server engines field, and both affected packages get patch changesets. The two test adjustments are narrow and keep their assertions meaningful. Nothing here blocks merging; one doc reference to Bun 1.3.14 is now stale and the engines change could use a more explicit release note.
Findings: 0 blocking | 7 non-blocking | 2 posted inline
Blockers
- None at the file/PR level.
Non-blocking
packages/sei-global-wallet/README.md:75still says "Bun 1.3.14 does not support nested overrides", but the workspace now pins Bun 1.4.4. Please check whether this still holds on 1.4.4 and update the version reference, so readers aren't pointed at a Bun release this PR flags as vulnerable.@sei-js/mcp-serverengines.nodedrops Node 20 and Node 22 before 22.23.4 in a patch release. npm only warns unlessengine-strictis set, so patch is defensible. Still, users withengine-strictset, or on Yarn, which fails installs on an engines mismatch, will see install failures after a patch upgrade. Consider calling this out at the top of the changeset or release note.- The advisory and CVE identifiers (GHSA-9v8h-qqgm-4m35 / CVE-2026-111497) and the fixed versions (Node 22.23.4 / 24.22.1 / 26.12.1, Bun 1.4.4 with Brotli 1.2.1) could not be checked from the diff. A maintainer should confirm them against the linked Node.js security release post and the Bun release notes before merging.
- The merge order matters: per the description, #355 and #356 must land first, or the wallet smoke and create-sei audit will fail.
- Cursor's review file was empty, so that pass produced no output. Codex reported no material issues.
- 2 suggestion(s)/nit(s) flagged inline on specific lines.
| await withTimeout(started, 'request startup'); | ||
|
|
||
| await expect(transport.stop()).rejects.toBeInstanceOf(AggregateError); | ||
| // Under Bun 1.4, `expect(...).rejects` never settles a stop() that has to close a live request. |
There was a problem hiding this comment.
[nit] This workaround is fine, and if stop() resolves, stopError is undefined and toBeInstanceOf still fails, so the assertion stays meaningful. Consider linking an upstream Bun issue in the comment so the workaround can be reverted to .rejects once Bun fixes it.
| }); | ||
| // The result object belongs to the vm realm, and a strict deep comparison rejects its foreign prototype. | ||
| assert.deepEqual( | ||
| { ...(context.__edgeCheck as Record<string, unknown>) }, |
There was a problem hiding this comment.
[nit] The shallow spread works here because both values are primitives. If __edgeCheck ever gains nested objects, those would still carry vm-realm prototypes and hit the same strict-compare failure; something like structuredClone or a JSON round-trip would cover that case too. If __edgeCheck is undefined, spreading it gives {}, and the comparison still fails as it should.
Yarn 1, and npm with engine-strict set, refuse to install on a Node release outside the new engines range, so a patch upgrade can fail for those users. Say so at the top of the release note instead of in its last paragraph. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
This PR moves Node and Bun past the Brotli advisory: it pins Node in .nvmrc, bumps Bun in CI, the root and template packageManager fields and the READMEs, tightens mcp-server engines.node, and adds changesets for both affected packages. The changes are correct and consistent. The notes below are minor, and Codex found no issues.
Findings: 0 blocking | 6 non-blocking | 2 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- The Cursor review file was empty, so that pass produced no output.
packages/sei-global-wallet/README.md:75still says "Bun 1.3.14 does not support nested overrides". The repo now requires Bun 1.4.4, so either check whether 1.4.4 behaves the same and update the version, or phrase the note so it doesn't name a version the repo no longer uses.- The new
mcp-serverengines range is a breaking change for users on Node 20 and on older 22.x, 24.x and 26.x releases, yet it ships as a patch. The changeset explains this clearly. Patch looks deliberate for a security floor, but the maintainers should confirm they accept that on the release-notes side. - The PR description says #355 and #356 must merge first, or the smoke workflows fail. Keep this PR out of the merge queue until both have landed and this branch has been rebased.
- 2 suggestion(s)/nit(s) flagged inline on specific lines.
| await withTimeout(started, 'request startup'); | ||
|
|
||
| await expect(transport.stop()).rejects.toBeInstanceOf(AggregateError); | ||
| // Under Bun 1.4, `expect(...).rejects` never settles a stop() that has to close a live request. |
There was a problem hiding this comment.
[nit] The workaround is sound: if stop() resolves, stopError is undefined and the assertion still fails. Consider linking an upstream Bun issue (or adding a TODO) so this can go back to expect(...).rejects once Bun fixes it.
| }); | ||
| // The result object belongs to the vm realm, and a strict deep comparison rejects its foreign prototype. | ||
| assert.deepEqual( | ||
| { ...(context.__edgeCheck as Record<string, unknown>) }, |
There was a problem hiding this comment.
[nit] Spreading into a main-realm object still catches missing or extra own keys, and an undefined __edgeCheck spreads to {}, which fails as it should. Non-enumerable or inherited props are now ignored, which is fine for this check.
Moves our runtimes off the Brotli decoder in GHSA-9v8h-qqgm-4m35 (CVE-2026-111497), an out-of-bounds write in Google's Brotli 1.0.3 through 1.2.0. The advisory lists no npm packages, so Dependabot won't raise it: the vulnerable decoder ships inside Node and Bun. Node fixed it in the October 21 security releases (22.23.4, 24.22.1, 26.12.1). Bun 1.3.14, our pin, vendors Brotli 1.1.0, and 1.4.3 still ships 1.2.0, so 1.4.4 (Brotli 1.2.1) is the first fixed Bun.
Nothing in the repo bundles Brotli.
mcp-serverreaches the decoder throughfetch, which decompresses Brotli responses from its RPC endpoints anddocs.sei.io/mcp. Its HTTP transports don't, because Express 4's body parser rejectsContent-Encoding: br.Changes
.nvmrcto24.22.1. With a bare24,setup-nodereuses whatever 24.x the runner image has cached.1.4.4inpackageManager, all five workflows, and the root README.@sei-js/create-sei: the template'spackageManagerand both READMEs now require Bun 1.4.4.@sei-js/mcp-server:engines.nodemoves from>=20.0.0to^22.23.4 || ^24.22.1 || >=26.12.1, and the README explains why. Node 20 is end-of-life and gets no fix.bun.lockis unchanged; Bun 1.4.4 accepts it under--frozen-lockfile.Two checks needed changes for Bun 1.4.4:
streamable-http.test.ts: Bun 1.4.4'sexpect(...).rejectsnever settles astop()that has to close a live request, while a plainawaitof the same promise settles in milliseconds. 1.3.14 was fine. The test hit its hook timeout and kept the file running for six minutes. It now awaits the rejection and asserts on it.check-sei-global-wallet.ts: the edge check strictly deep-compared an object created byvm.runInNewContext. Bun 1.3.14 ignored its foreign prototype; Bun 1.4.4 rejects it the way Node does. It now compares a main-realm copy, which still fails on missing or extra keys.Merge order
Merge #355 and #356 first, then rebase this. Both smoke workflows run here because the PR touches
package.jsonand.changeset/:maintoday and will fail here until fix(sei-global-wallet): clear the sharp, Vite, and stream-json drift #356 lands.Verification
On Bun 1.4.4 and Node 26.12.1:
bun install --frozen-lockfile,bun run check,bun run build,bun run test, andbun run lint:pack:allpass, and the coverage commands produce lcov for all four packages.bun run test:create-sei-releasepasses; both variants install, check, build, and run.bun run test:sei-global-wallet-releaserun passes: the wallet-only npm consumer audits clean, and the npm and Bun AA consumers stay within the waiver.