fix(webpack): give the browser bundle the node builtins node-rsa needs - #11
Conversation
There was a problem hiding this comment.
Pull request overview
Restores webpack browser builds after adding node-rsa by supplying targeted Node.js polyfills and CI coverage.
Changes:
- Adds lightweight crypto and Node builtin fallbacks.
- Adds required browser-build dependencies.
- Runs
build-webin CI.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/ci.yml |
Builds the browser bundle in CI. |
package.json |
Adds browser polyfill dependencies. |
webpack.config.js |
Configures process and builtin fallbacks. |
webpack/crypto-shim.js |
Implements minimal crypto support for node-rsa. |
yarn.lock |
Locks the added dependencies. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| - name: Build web | ||
| run: yarn run build-web |
There was a problem hiding this comment.
Agreed, and the smoke test is in: scripts/browser-bundle.test.js loads the emitted chunks into a context with no Buffer, no process, no global and no require, then drives verify_rsa_sha256_sig through importKey(..., 'components-public') and verify(...), asserting 1 for a valid signature and 0 for one tampered byte.
It was run against the bundle as it stood before the Buffer fix and reports 'not ok 2 the bundle accepts a valid RSA signature: returned 0, wanted 1', so it fails on the defect it exists to catch. CI runs it right after the build step.
| Buffer: ["buffer", "Buffer"], | ||
| // node-rsa reads process.title to tell a browser from node, and webpack | ||
| // no longer supplies a process object of its own. | ||
| process: "process/browser", |
There was a problem hiding this comment.
Correct, and it was worse than a throw. Webpack rewrites global to globalThis rather than leaving it undefined, so there is no ReferenceError: globalThis.Buffer is simply undefined, Buffer.from throws a TypeError, and the catch around the host function turns that into a return of 0. A valid signature therefore read as invalid with nothing logged.
The bare identifier is not the fix here either, because Buffer in vm.ts is this package's own Uint8Array subclass from src/buffer.ts. Now importing the buffer module by name, which gives node its builtin and the bundle the package that replaces it.
The browser bundle target has not compiled since node-rsa joined the runtime dependencies. Webpack 5 stopped supplying node builtins on its own, and nineteen resolution errors follow: crypto and constants through node-rsa, fs and path through rustbn.js, os through colors. Nothing caught it, because no workflow ran the target and the bundle it writes is neither committed nor published. The usual answer, a whole crypto implementation compiled for the browser, buys far more than this package needs and drags the unpatched elliptic advisory in through three further paths. node-rsa reaches for crypto at load time in every scheme file, but the branch it takes when its environment is browser calls only createHash and randomBytes, because the RSA arithmetic runs on its own BigInteger. Mapping crypto to a two-function module holds the bundle to those, and the engine selector finds no publicEncrypt on it and settles on the pure JavaScript engine, which is the one a browser has to use anyway. The three node-only builtins resolve to false, and process joins Buffer in the provide plugin, because node-rsa reads process.title to tell a browser from node. Compiling was not enough on its own. The RSA host function reached node's Buffer through the global object, since the bare name here belongs to this package's own Uint8Array subclass, and webpack rewrites that global to globalThis, where a browser carries no Buffer. The call threw, the surrounding catch turned the throw into a zero, and a valid signature came back invalid with nothing logged as wrong. Importing the buffer module by name hands node its builtin and the bundle the package that stands in for it. So the new check drives the host function rather than the compiler: it loads the emitted chunks into a context holding no Buffer, no process, no global and no require, then verifies a signature and rejects a tampered one. It fails against the bundle as it stood. CI runs the build and the check from here on, so the next break surfaces on the pull request that causes it.
a4014f7 to
10259da
Compare
Why
yarn run build-webhas been broken sincenode-rsajoined the runtime dependencies. Webpack 5 stopped supplying node builtins on its own, and nineteen resolution errors follow:cryptoandconstantsthrough node-rsa,fsandpaththrough rustbn.js,osthrough colors. Nothing caught it. No workflow ran the target, anddist-webis neither committed nor published, so the failure was invisible.The conventional fix,
crypto-browserify, costs more than this package needs. It adds 43 packages and pulls the unpatchedellipticadvisory GHSA-848j-6mx2-7j84 in throughbrowserify-sign,create-ecdhandcrypto-browserify, which would take the repository from two open alerts back to about five rows of the same advisory.node-rsa requires
cryptoat load time in every scheme file, but the branch it takes when its environment isbrowsercalls onlycreateHashandrandomBytes. The RSA arithmetic runs on its own BigInteger, andcreateSignandcreateVerifysit on the node branch a browser never reaches. Socryptomaps to a two-function module instead.fs,pathandosresolve tofalse, since rustbn.js and colors reach for them only on their node branches.processjoinsBufferin the provide plugin, becauseutils.jsreadsprocess.titleto tell a browser from node.The bundle compiled and still did not work
vm.tsreached node'sBufferthrough the global object, because the bare name in that file belongs to this package's ownUint8Arraysubclass fromsrc/buffer.ts. Webpack rewritesglobaltoglobalThis, where a browser carries noBuffer, soBuffer.fromthrew, the surroundingcatchturned the throw into a0, andverify_rsa_sha256_sigreported a valid signature as invalid with nothing logged as wrong. Importing thebuffermodule by name hands node its builtin and the bundle the package that stands in for it. Node behaviour is unchanged, since both names reach the same object there.Thanks for both review points. They were correct, and the second one was the reason this needed more than a fallback map.
Validation
The bundle compiles: nineteen errors to zero, three size warnings, writing
vert.min.jsat 86 KB andexternals.min.jsat 1.3 MB.scripts/browser-bundle.test.jsis the answer to "a compiling bundle is not a working one". It loads the emitted chunks into a context holding noBuffer, noprocess, noglobaland norequire, then drivesverify_rsa_sha256_sigitself: it accepts a valid signature and rejects the same signature with its first byte flipped. It was run against the bundle as it stood before theBufferfix and reportsnot ok 2 ... returned 0, wanted 1, so it fails on the defect it exists to catch.Root gates: install frozen, build, 35 unit tests passing including both RSA cases, packaging 8/8, release notes 12/12.
npm pack --dry-runships none ofdist-web,webpack/orscripts/, so the published package is unchanged apart from thevm.tsimport.Dependency cost measured against
main: the root lockfile goes from 297 packages to 322, all dev scope.yarn auditreports no advisory on any of the 25 additions. The four it does report (micromatch,cross-spawn,diff,elliptic) all predate this branch; Dependabot has auto-dismissed the first three andellipticis the known unpatched one.