Skip to content

test(utils): cover generateId's insecure-context fallback - #7189

Open
reddynitish wants to merge 1 commit into
simstudioai:stagingfrom
reddynitish:test/3393-generateid-fallback-coverage
Open

test(utils): cover generateId's insecure-context fallback#7189
reddynitish wants to merge 1 commit into
simstudioai:stagingfrom
reddynitish:test/3393-generateid-fallback-coverage

Conversation

@reddynitish

Copy link
Copy Markdown

Summary

  • Issue [Bug] White screen / "Application error" - crypto.randomUUID is not a function #3393 reports a white-screen crash on self-hosted deployments accessed over plain HTTP (a LAN IP, not localhost/HTTPS): TypeError: crypto.randomUUID is not a function. Browsers only expose crypto.randomUUID() in secure contexts.
  • This is already fixed on current staginggenerateId() (packages/utils/src/id.ts) has feature-detected crypto.randomUUID and fallen back to a crypto.getRandomValues()-based UUID v4 since b5674d9ed4 (improvement(codebase): centralize test mocks, extract @sim/utils, remove dead code #4228). A repo-wide grep confirms no client-side code in apps/sim calls crypto.randomUUID directly anymore, and scripts/check-utils-enforcement.ts bans direct usage outside that one definition file. This PR does not fix a bug — nothing here changes production behavior.
  • What it does close: the existing test suite for generateId() only ever exercised the "happy path" — Node/Bun's crypto.randomUUID is always present in the test environment, so the fallback branch that actually fixes [Bug] White screen / "Application error" - crypto.randomUUID is not a function #3393 had zero coverage. A future refactor could silently reintroduce the exact crash with nothing catching it.
  • Adds two tests to packages/utils/src/id.test.ts that stub crypto.randomUUID to undefined (restored in afterEach, verified not to leak across tests or files) and assert generateId() still returns a valid, unique UUID v4. Verified these tests genuinely fail with the original TypeError when generateId() is temporarily reverted to a naive crypto.randomUUID() call, and pass again once restored.

Refs #3393 (using Refs, not Fixes — the underlying bug isn't something this PR resolves; it already isn't reproducible on staging).

Type of Change

  • Other: test-only regression coverage for an already-fixed bug

Testing

  • cd packages/utils && bunx vitest run src/id.test.tsPASS, 12/12.
  • Reverted generateId() to the pre-fix return crypto.randomUUID() (no guard) and re-ran the same file — the two new tests failed with TypeError: crypto.randomUUID is not a function, the exact error from [Bug] White screen / "Application error" - crypto.randomUUID is not a function #3393; all other tests still passed. Restored the file and confirmed git diff clean before committing.
  • cd packages/utils && bunx vitest run (full workspace, all 14 files) — PASS, 194/194, both before and after the regression check above — the afterEach restore doesn't leak across tests or files.
  • bunx tsc --noEmit / bunx turbo run type-check --filter=@sim/utilsPASS. Confirms the two @ts-expect-error directives suppress a real type error rather than being stale (an unused directive is itself a type error).
  • bunx biome check packages/utils/src/id.test.tsPASS.
  • bun run check:utils — pre-existing baseline failures only (13 hits, all in files already on the allowlist — id.ts's own definition, helpers.ts, random.ts, etc.); none are in id.test.ts, which is already allowlisted at scripts/check-utils-enforcement.ts:37 for referencing the banned pattern directly (needed to test it).
  • Full grep of apps/sim for crypto.randomUUID / randomUUID — 6 hits, all vi.fn() test mocks; zero production callsites.

An independent adversarial review (Claude subagent, no session context, in place of Codex which isn't installed on this machine) reproduced all of the above itself — including a stress test running the full suite with --no-isolate --no-file-parallelism to rule out cross-file leakage from the crypto.randomUUID stub — and confirmed the diff is correct as-is. It also independently traced the fix to b5674d9ed4 and flagged that the PR should be framed as coverage rather than a fix (reflected above), which is the only change it asked for.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Screenshots/Videos

N/A — test-only change, no UI.

Refs simstudioai#3393

The crash reported there (crypto.randomUUID is not a function, hit on
self-hosted deployments served over plain HTTP — browsers only expose
crypto.randomUUID() in secure contexts) is already fixed on staging:
generateId() has feature-detected crypto.randomUUID and fallen back to
a crypto.getRandomValues()-based UUID v4 since b5674d9 (simstudioai#4228), and
no client-side code calls crypto.randomUUID directly anymore
(enforced by scripts/check-utils-enforcement.ts).

What was missing: the existing test suite never exercised the fallback
branch, since Node/Bun's crypto.randomUUID is always present in the test
environment. A future refactor could silently break the exact path that
fixes simstudioai#3393 with nothing catching it. Adds two tests that stub
crypto.randomUUID to undefined (restored in afterEach) and assert the
fallback still produces valid, unique UUID v4s. Verified these tests
actually fail with the pre-fix TypeError when generateId() is reverted
to a naive crypto.randomUUID() call.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Aug 28, 2026

Copy link
Copy Markdown

@reddynitish is attempting to deploy a commit to the Sim Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps

greptile-apps Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds regression coverage for generateId() when crypto.randomUUID is unavailable, without changing production behavior.

  • Adds fallback-path UUID v4 validation.
  • Verifies fallback-generated IDs remain unique across 100 calls.
  • Restores the global crypto method after each test to preserve isolation.

Confidence Score: 5/5

The test-only change appears safe to merge.

The new tests exercise the existing fallback path and restore the temporarily modified crypto method after every test, with no production-code changes or accepted defects.

Important Files Changed

Filename Overview
packages/utils/src/id.test.ts Adds focused, isolated tests for the existing insecure-context UUID fallback; no actionable issues found.

Reviews (1): Last reviewed commit: "test(utils): cover generateId's insecure..." | Re-trigger Greptile

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.

1 participant