diff --git a/docs/06-field-notes.md b/docs/06-field-notes.md index c27d7cc..e955cf9 100644 --- a/docs/06-field-notes.md +++ b/docs/06-field-notes.md @@ -175,6 +175,32 @@ Base: `~/.vscode-server/data/User/` cooldown change is warranted (the proxy lag itself adds real-age protection and Dependabot's 7d gates the update flow); just be aware the local gate ≈ npm-publish + mirror-lag + 7d. +- **A security override only helps if a PATCHED version is mirrored — and a still-vulnerable + override actively fails `dependency-review` (2026-07-22).** `dependency-review-action` + (`fail-on-severity: high`) reviews only the **diff**: a pre-existing vulnerable transitive left + untouched passes (it stays a Dependabot _alert_, not a PR blocker), but the moment a PR **changes** + that dep to another version still inside the advisory range it fails the check. Learned the hard way + on `fast-uri`: the alert cited `<= 3.1.3` (GHSA-cq4c-9wjx-4gp7), but a newer advisory + (GHSA-v2hh-gcrm-f6hx / CVE-2026-16221) extended the affected range to `>= 4.0.0, <= 4.1.0`, so an + `overrides: fast-uri: ^4.1.0` resolved to the **still-vulnerable** 4.1.0 — no fix, plus a red gate. + The only patched lines are **2.4.3 / 3.1.4 / 4.1.1**, and **none of them is mirrored on the MS feed + proxy yet** (its fast-uri history is 3.1.3 → 4.0.0 → 4.0.1 → 4.1.0), so the lockfile **cannot** be + regenerated with the fix locally. Correct play: **do not land a security override until a patched + version is actually resolvable on your registry** — ship the independent fixes, leave the + transitive unchanged (Dependabot handles it against real npm once the point-fix lands / the proxy + mirrors it), and always **cross-check the CURRENT advisory range** (not just the version the alert + first cited) before picking the target. +- **Correlation/frame ids must be `crypto.randomUUID()`, never `Math.random()` — CodeQL + `js/insecure-randomness` (2026-07-22).** Any random value that flows into a request/frame id (which a + scanner treats as a security sink) trips the High CodeQL alert even for our benign RPC-correlation + use — and even when the flagged file is a _downstream_ sink (the alert pointed at the dev-only + `web-playground` echo, but the tainted **source** was `web`'s `Math.random` ids). Fix at the source: + browser + Node ≥19 both expose `crypto.randomUUID()` (web has it via the DOM lib in a secure/localhost + context; in the extension's Node context `import { randomUUID } from "node:crypto"` — do **not** reach + for `globalThis.crypto` there). Genuinely non-security `Math.random` is fine and should stay: + reconnect-backoff **jitter** and the crypto-first, clearly-local-only `newTraceId` **fallback** are + not sinks — don't cargo-cult them into UUIDs. + - **esbuild CLI shim is broken under pnpm (persistent).** pnpm's `.bin/esbuild` cmd-shim hardcodes `exec node `, but esbuild's postinstall overwrites its own `bin/esbuild` (a Node stub in the tarball) with the native Go binary → `node ` `SyntaxError`. `pnpm rebuild esbuild` does diff --git a/packages/extension/src/provider-auth.ts b/packages/extension/src/provider-auth.ts index 6473fc8..0d034e4 100644 --- a/packages/extension/src/provider-auth.ts +++ b/packages/extension/src/provider-auth.ts @@ -1,3 +1,4 @@ +import { randomUUID } from "node:crypto"; import { WebSocket } from "ws"; import { rpcErrorSchema, sessionAuthResponseSchema } from "@cloakcode/protocol"; @@ -58,7 +59,7 @@ export function exchangeCodeForToken( ): Promise { return new Promise((resolve, reject) => { const ws = new WebSocket(url); - const id = `auth-${Math.random().toString(36).slice(2)}`; + const id = `auth-${randomUUID()}`; const timer = setTimeout(() => { ws.close(); reject(new Error("gateway timed out")); diff --git a/packages/web/src/auth.test.ts b/packages/web/src/auth.test.ts index fd353c0..53b90a3 100644 --- a/packages/web/src/auth.test.ts +++ b/packages/web/src/auth.test.ts @@ -46,7 +46,6 @@ class MockWebSocket { } beforeEach(() => { - vi.spyOn(Math, "random").mockReturnValue(0); MockWebSocket.instances = []; vi.stubGlobal("WebSocket", MockWebSocket); localStorage.clear(); @@ -86,6 +85,13 @@ describe("tokenAuthFrame", () => { expect(frame.op).toBe("auth"); expect(frame.params).toEqual({ token: "tok" }); }); + + it("uses a cryptographically-random (UUID) correlation id, not Math.random", () => { + const id = JSON.parse(tokenAuthFrame("tok")).id as string; + expect(id).toMatch( + /^auth-[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i, + ); + }); }); describe("needs-auth bus", () => { diff --git a/packages/web/src/auth.ts b/packages/web/src/auth.ts index 996f328..6add037 100644 --- a/packages/web/src/auth.ts +++ b/packages/web/src/auth.ts @@ -44,7 +44,7 @@ export function clearStoredToken(): void { /** A fresh `auth` frame resuming with a stored token, for a new socket. */ export function tokenAuthFrame(token: string): string { return JSON.stringify({ - id: `auth-${Math.random().toString(36).slice(2)}`, + id: `auth-${crypto.randomUUID()}`, op: "auth", params: { token }, }); @@ -107,7 +107,7 @@ export function submitAuthCode( ): Promise { return new Promise((resolve, reject) => { const ws = new WebSocket(url); - const id = `auth-${Math.random().toString(36).slice(2)}`; + const id = `auth-${crypto.randomUUID()}`; const timer = setTimeout(() => { ws.close(); reject(new Error("bridge timed out")); @@ -165,7 +165,7 @@ export function beginEnrolment( ): Promise { return new Promise((resolve, reject) => { const ws = new WebSocket(url); - const id = `enrol-${Math.random().toString(36).slice(2)}`; + const id = `enrol-${crypto.randomUUID()}`; const timer = setTimeout(() => { ws.close(); reject(new Error("bridge timed out")); diff --git a/packages/web/src/bridge.test.ts b/packages/web/src/bridge.test.ts index 4105bf3..2ea72b6 100644 --- a/packages/web/src/bridge.test.ts +++ b/packages/web/src/bridge.test.ts @@ -87,7 +87,7 @@ function appendFrame(seq: number): unknown { beforeEach(() => { vi.useFakeTimers(); - vi.spyOn(Math, "random").mockReturnValue(0); // deterministic id + zero jitter + vi.spyOn(Math, "random").mockReturnValue(0); // zero backoff jitter (ids are crypto UUIDs) MockWebSocket.instances = []; vi.stubGlobal("WebSocket", MockWebSocket); }); diff --git a/packages/web/src/bridge.ts b/packages/web/src/bridge.ts index b878bd7..8140458 100644 --- a/packages/web/src/bridge.ts +++ b/packages/web/src/bridge.ts @@ -66,7 +66,7 @@ export function fetchSessions( ): Promise { return new Promise((resolve, reject) => { const ws = new WebSocket(url); - const id = Math.random().toString(36).slice(2); + const id = crypto.randomUUID(); const timer = setTimeout(() => { ws.close(); reject(new Error("bridge timed out")); @@ -168,7 +168,7 @@ export function subscribeSession( onStatus(attempt === 0 ? "connecting" : "reconnecting"); const socket = new WebSocket(url); ws = socket; - const id = Math.random().toString(36).slice(2); + const id = crypto.randomUUID(); socket.addEventListener("open", () => { attempt = 0; @@ -266,7 +266,7 @@ function oneShotRpc( ): Promise { return new Promise((resolve, reject) => { const ws = new WebSocket(url); - const id = Math.random().toString(36).slice(2); + const id = crypto.randomUUID(); const timer = setTimeout(() => { ws.close(); reject(new Error("bridge timed out"));