feat(plugins): give the sandbox a CSPRNG - #390
Open
lovepixel-git wants to merge 1 commit into
Open
Conversation
The QuickJS crypto shim exposed only `crypto.subtle` digest and HMAC, so `Math.random` and `Date` were the only entropy inside a plugin VM. A plugin that has to mint a bearer token, nonce, invitation code or one-time link had no safe way to do it server-side. CoreBunch#387 reports working around this by generating tokens in the browser and posting them to the server, so a capability the server owns depended on a client for its entropy. Verified on main before this change: inside the VM, `typeof crypto.getRandomValues` and `typeof crypto.randomUUID` are both `undefined`, while `crypto.subtle.digest` is a function. The load-bearing constraint is that `getRandomValues` is synchronous by spec. Every existing crypto call goes through `__hostCall`, which vm.ts documents as returning a VM-side Promise, so a shim built on it could not be awaited by a spec-shaped caller. This adds a dedicated synchronous host function, `__hostRandomBytes`, alongside `__hostSleep` and `__log`, returning base64 the shim decodes with the existing codec. Ungated, matching the reasoning already documented for `crypto.digest` and `crypto.signHmac`: pure computation, no I/O, nothing to escalate. Spec conformance where it costs nothing: integer-typed views only (`TypeMismatchError` for float views and non-views), the WebCrypto 65536 byte per-call quota (`QuotaExceededError` above it, enforced in the shim AND the host function so a plugin cannot ask the host for an unbounded allocation), fills through a byte view so element width and `byteOffset` are respected, and an RFC 9562 version-4 `randomUUID`. QuickJS has no DOMException, so the spec's error `name` rides on a plain Error. The shim augments `globalThis.crypto` rather than replacing it, so it is order-independent against the subtle shim; it only has to follow the base64 codec. Fixes CoreBunch#387
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #387.
Verified before touching anything
Probed a live VM on
main:After:
So
Math.randomandDatereally were the only entropy in the sandbox, which is why the reporter had to mint MCP access tokens in the browser and POST them to the server — a capability the server owns depending on a client for its entropy.The constraint that shapes the fix
crypto.getRandomValuesis synchronous by spec. Every existing crypto path goes through__hostCall, whichvm.tsdocuments as returning a VM-side Promise:A shim built on that bridge would hand back a Promise, so
crypto.getRandomValues(new Uint8Array(8))could not work without anawaitno real caller writes. So this adds a dedicated synchronous host function,__hostRandomBytes, next to__hostSleepand__log, returning base64 that the shim decodes with the codec already shared by fetch and the digest path.Left ungated, matching the reasoning the crypto handlers already document: pure computation, no I/O, nothing to escalate. Same model as
MathandJSON.Spec conformance where it was free
Float32Array,Float64Array, plain arrays andundefinedall throwTypeMismatchError.QuotaExceededErrorabove it. Enforced in the shim and independently in the host function, so a plugin that reaches__hostRandomBytesdirectly still cannot ask the host for an unbounded allocation. One exported constant backs both, so they cannot drift.byteOffsetare honoured. There is a test asserting aUint8Array(buffer, 4, 8)view leaves the surrounding bytes untouched.randomUUIDsets the RFC 9562 version and variant bits.DOMException, so the spec's errornamerides on a plainError. Plugins branching onerr.namebehave the same.The shim augments
globalThis.cryptorather than replacing it, so it is order-independent against the subtle shim and only has to follow the base64 codec. One test assertscrypto.subtle's three methods still exist afterwards.Tests
src/__tests__/server/pluginSandboxCsprng.test.ts, 12 cases in a real VM through the existingcreatePluginVmrecorder harness. Beyond presence they cover: synchronous return of the same array instance (a Promise there would mean it went through__hostCall), varied non-zero bytes (an all-zero fill is the signature of a bridge that silently no-ops), different bytes across successive calls, wide element types,byteOffsetisolation, zero-length arrays, both quota boundaries, everyTypeMismatchErrorcase, and 200 UUIDs with no collision.bunx tsc -bexit 0bun run lintcleanbun run buildcleanbun test6628 pass, 0 failOne note in case it saves you time: the bundle-size budget test fails against a stale
dist/. Rebuild before running the full suite and it is green. I chased that one down and it is not related to this change.Deliberately not in scope
The reporter also mentions
crypto.subtlebeing HMAC-only, so plugins cannot sign an RS256 JWT. That is a separate surface and a bigger call, and they offered to file it separately.getRandomValuesalone unblocks everything else they listed.