From cf5611b02e9dc67473a20851baf394a523bf12d3 Mon Sep 17 00:00:00 2001 From: maherm Date: Tue, 18 Aug 2026 17:03:02 +0200 Subject: [PATCH] fix(core): don't rely on crypto.randomUUID for asset ids saveAsset() (called from reference-panel.tsx and local-guide-image.ts, i.e. on every file upload/drop) used crypto.randomUUID() to generate the IndexedDB key. That method requires a secure context (HTTPS or localhost) and is undefined otherwise, so it throws on plain-HTTP origins such as an internal-network deployment reached over a bare IP or hostname - every upload failed immediately there. Switched to nanoid's customAlphabet, which has no secure-context requirement and is already used for ids elsewhere in this package (schema/base.ts). Preferred that over the narrower typeof-check + counter fallback used in packages/nodes/src/cabinet/stack.ts: that counter resets to 0 on every reload, which is fine for stack.ts's transient in-memory compartment ids but not here - asset ids are persisted as IndexedDB keys and as asset:// references inside saved/exported scenes, so a colliding fallback could silently overwrite a different asset. nanoid gives the same collision resistance on every code path instead of degrading on insecure origins. Also adds a regression test. There was no existing test for this module because idb-keyval (used here) needs a real indexedDB global, which Bun's test runtime doesn't provide - added fake-indexeddb as a devDependency to fill that gap. The new test stubs crypto.randomUUID as unavailable and asserts saveAsset()/loadAssetUrl() still round-trip correctly; confirmed it fails with the old implementation (TypeError: crypto.randomUUID is not a function) and passes with the fix. Full packages/core suite (957 tests) passes. --- bun.lock | 3 + packages/core/package.json | 1 + packages/core/src/lib/asset-storage.test.ts | 63 +++++++++++++++++++++ packages/core/src/lib/asset-storage.ts | 6 +- 4 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 packages/core/src/lib/asset-storage.test.ts diff --git a/bun.lock b/bun.lock index f32d84c143..efafcd58f5 100644 --- a/bun.lock +++ b/bun.lock @@ -129,6 +129,7 @@ "@types/bun": "^1.3.0", "@types/react": "^19.2.2", "@types/three": "^0.184.0", + "fake-indexeddb": "^6.2.5", "typescript": "6.0.3", }, "peerDependencies": { @@ -1263,6 +1264,8 @@ "express-rate-limit": ["express-rate-limit@8.5.2", "", { "dependencies": { "ip-address": "^10.2.0" }, "peerDependencies": { "express": ">= 4.11" } }, "sha512-5Kb34ipNX694DH48vN9irak1Qx30nb0PLYHXfJgw4YEjiC3ZEmZJhwOp+VfiCYwFzvFTdB9QkArYS5kXa2cx2A=="], + "fake-indexeddb": ["fake-indexeddb@6.2.5", "", {}, "sha512-CGnyrvbhPlWYMngksqrSSUT1BAVP49dZocrHuK0SvtR0D5TMs5wP0o3j7jexDJW01KSadjBp1M/71o/KR3nD1w=="], + "fast-check": ["fast-check@4.8.0", "", { "dependencies": { "pure-rand": "^8.0.0" } }, "sha512-GOJ158CUMnN6cSahsv4+ExARvIDuzzinFjkp0E9WtiBa5zcVeLozVkWaE4IzFcc+Y48Wp1EDlUZsXRyAztQcSg=="], "fast-deep-equal": ["fast-deep-equal@3.1.3", "", {}, "sha512-f3qQ9oQy9j2AhBe/H9VC91wLmKBCCU/gDOnKNAYG5hswO7BLKj09Hc5HYNz9cGI++xlpDCIgDaitVs03ATR84Q=="], diff --git a/packages/core/package.json b/packages/core/package.json index 8612ea2e7c..2338fadb9a 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -88,6 +88,7 @@ "@types/bun": "^1.3.0", "@types/react": "^19.2.2", "@types/three": "^0.184.0", + "fake-indexeddb": "^6.2.5", "typescript": "6.0.3" }, "keywords": [ diff --git a/packages/core/src/lib/asset-storage.test.ts b/packages/core/src/lib/asset-storage.test.ts new file mode 100644 index 0000000000..432b79f900 --- /dev/null +++ b/packages/core/src/lib/asset-storage.test.ts @@ -0,0 +1,63 @@ +import 'fake-indexeddb/auto' +import { afterEach, describe, expect, test } from 'bun:test' +import { loadAssetUrl, saveAsset } from './asset-storage' + +function file(contents: string, name = 'test.txt'): File { + return new File([contents], name, { type: 'text/plain' }) +} + +describe('saveAsset', () => { + const originalRandomUUID = crypto.randomUUID + + afterEach(() => { + crypto.randomUUID = originalRandomUUID + }) + + test('returns an asset:// URL', async () => { + const url = await saveAsset(file('hello')) + expect(url.startsWith('asset://')).toBe(true) + }) + + test('generates distinct ids across calls', async () => { + const [a, b] = await Promise.all([saveAsset(file('a')), saveAsset(file('b'))]) + expect(a).not.toBe(b) + }) + + // Regression test: crypto.randomUUID() throws/`undefined`s on plain-HTTP + // origins because it requires a secure context (HTTPS or localhost). Every + // upload used to fail on such deployments (see packages/editor's + // reference-panel.tsx, local-guide-image.ts, both of which call saveAsset). + test('still works when crypto.randomUUID is unavailable (insecure context)', async () => { + // @ts-expect-error simulating a browser without Web Crypto's randomUUID + crypto.randomUUID = undefined + + const url = await saveAsset(file('insecure-context')) + expect(url.startsWith('asset://')).toBe(true) + + const loaded = await loadAssetUrl(url) + expect(loaded).not.toBeNull() + }) +}) + +describe('loadAssetUrl', () => { + test('round-trips a saved asset back to an object URL', async () => { + const url = await saveAsset(file('round-trip')) + const objectUrl = await loadAssetUrl(url) + expect(objectUrl?.startsWith('blob:')).toBe(true) + }) + + test('passes through blob: and http(s) URLs unchanged', async () => { + expect(await loadAssetUrl('blob:http://example.com/1234')).toBe('blob:http://example.com/1234') + expect(await loadAssetUrl('https://cdn.example.com/a.glb')).toBe( + 'https://cdn.example.com/a.glb', + ) + }) + + test('returns null for an unknown asset id', async () => { + expect(await loadAssetUrl('asset://does-not-exist')).toBeNull() + }) + + test('returns null for an empty URL', async () => { + expect(await loadAssetUrl('')).toBeNull() + }) +}) diff --git a/packages/core/src/lib/asset-storage.ts b/packages/core/src/lib/asset-storage.ts index 72f577a348..7f2213f445 100644 --- a/packages/core/src/lib/asset-storage.ts +++ b/packages/core/src/lib/asset-storage.ts @@ -1,15 +1,19 @@ import { get, set } from 'idb-keyval' +import { customAlphabet } from 'nanoid' export const ASSET_PREFIX = 'asset_data:' // Cache for active object URLs to prevent leaks and flickering const urlCache = new Map() +// Unlike crypto.randomUUID(), nanoid works outside secure contexts. +const nanoAssetId = customAlphabet('0123456789abcdefghijklmnopqrstuvwxyz', 16) + /** * Save a file to IndexedDB and return a custom protocol URL */ export async function saveAsset(file: File): Promise { - const id = crypto.randomUUID() + const id = nanoAssetId() await set(`${ASSET_PREFIX}${id}`, file) return `asset://${id}` }