fix(lib): stop depending on a secure-context API for ids - #1283
Open
joepio wants to merge 1 commit into
Open
Conversation
`crypto.randomUUID` is gated on a secure context, so over plain HTTP it is undefined on anything but localhost — which is exactly how a self-hosted server on a LAN gets reached (`http://homeassistant.local`, `http://192.168.1.x`). `DrivePresenceManager`'s constructor called it bare, and that runs on every drive load, so the app did not degrade a feature: it threw before first paint and rendered "Error loading resource". That is the Home Assistant add-on needing HTTPS or hard- crashing. The data-browser entrypoint had been carrying a global `window.crypto.randomUUID = ...` patch to paper over this. A polyfill hides the constraint from every future caller and put a monkey-patch in the app's first statement, so replace it with an explicit `randomUUID()` in `@tomic/lib`: `crypto.randomUUID` when it exists, `getRandomValues` otherwise. `getRandomValues` carries no secure-context gate, so the fallback is as random as the real thing — only the convenience wrapper is missing. Migrated all fifteen call sites so nothing relies on the patch that is now gone. Two had already grown their own guards, which is a sign this had bitten before: `client-db.ts` fell back to `Math.random().toString(36)` for its tab id — neither a UUID nor cryptographically random, and colliding for tabs opened in the same millisecond — and `OpenRouterLoginButton` fell back to `randomString(32)` for a PKCE verifier. Tested against a stubbed insecure context where `crypto.randomUUID` is absent, since that is the case the whole thing exists for and it cannot be observed on localhost.
2 tasks
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.
crypto.randomUUIDis gated on a secure context, so over plain HTTP it is undefined on anything but localhost — which is exactly how a self-hosted server on a LAN gets reached (http://homeassistant.local,http://192.168.1.x).DrivePresenceManager's constructor called it bare:and that runs on every drive load. So the app did not degrade a feature — it threw before first paint and rendered "Error loading resource". This is the Home Assistant add-on needing HTTPS or hard-crashing.
What was there
The data-browser entrypoint carried a global
window.crypto.randomUUID = ...patch to paper over it. That works, but it hides the constraint from every future caller and puts a monkey-patch in the app's first statement.What changed
An explicit
randomUUID()in@tomic/lib— realcrypto.randomUUIDwhen it exists,getRandomValuesotherwise.getRandomValuescarries no secure-context gate, so the fallback is as random as the real thing; only the convenience wrapper is absent.All fifteen call sites migrated, so nothing relies on the patch that is now gone.
Two had already grown their own guards, which suggests this had bitten before — and both fallbacks were worse than they look:
client-db.tstab idMath.random().toString(36)OpenRouterLoginButtonrandomString(32)Testing
random-uuid.test.tsstubs an insecure context wherecrypto.randomUUIDis absent — the case the whole thing exists for, and one that cannot be observed on localhost. Plus v4 shape and a collision check.oxfmt)Checklist
Note
Medium Risk
Touches core init paths (presence, client-db tab RPC) and auth (OpenRouter PKCE); behavior change is intentional and low blast radius outside HTTP/insecure contexts, but incorrect UUID generation would affect session/tab identity.
Overview
Fixes first-paint crashes on plain HTTP (e.g. LAN self-host) where
crypto.randomUUIDis missing because it is secure-context-only.DrivePresenceManagercalled it on every drive load, which surfaced as “Error loading resource” instead of a degraded feature.Adds an explicit
randomUUID()in@tomic/lib: nativecrypto.randomUUIDwhen present, otherwise RFC 4122 v4 viacrypto.getRandomValues. It is exported from the package and wired through@tomic/react.Replaces the data-browser entrypoint
window.crypto.randomUUIDpolyfill and migrates ~15 call sites (AI context IDs, MCP config, canvas branches, device IDs, OpenRouter PKCE verifier,ClientDbWorkertab IDs, etc.). Ad-hoc fallbacks (Math.random()tab ids, non-UUID PKCE verifiers) go away in favor of one helper.Adds
random-uuid.test.tsfor v4 shape, insecure-context stubbing, and collision checks.Reviewed by Cursor Bugbot for commit bdecf66. Bugbot is set up for automated code reviews on this repo. Configure here.