fix: resolve high-severity CodeQL findings

docs/webhooks.md

@@ -85,7 +85,7 @@ Endpoint filters can select browser.session.created, browser.session.closed, bro
 
 ## Verify the signature before parsing or dispatching
 
-Each request includes the event type, stable delivery ID, timestamp, 128-bit nonce, key ID, and v1=<hex> signature. The signature is HMAC-SHA256 over the domain string cockroach-browser.webhook.v1, timestamp, nonce, delivery ID, key ID, and exact body, separated by newlines. verifyWebhookSignature() checks syntax, timestamp tolerance, key binding, and the signature with a timing-safe comparison. The built-in WebhookReplayGuard is a bounded in-process nonce guard and fails closed when full. A multi-process or restart-safe receiver should place the same delivery ID and nonce checks in its durable store.
+Each request includes the event type, stable delivery ID, timestamp, 128-bit nonce, key ID, and v1=&lt;hex&gt; signature. The signature is HMAC-SHA256 over the domain string cockroach-browser.webhook.v1, timestamp, nonce, delivery ID, key ID, and exact body, separated by newlines. verifyWebhookSignature() checks syntax, timestamp tolerance, key binding, and the signature with a timing-safe comparison. The built-in WebhookReplayGuard is a bounded in-process nonce guard and fails closed when full. A multi-process or restart-safe receiver should place the same delivery ID and nonce checks in its durable store.
 
 ```
 import {

package.json

@@ -108,7 +108,7 @@
     "test:source": "npm test",
     "typecheck": "tsc -p tsconfig.json --noEmit",
     "check:package": "node scripts/check-package.mjs",
-    "check:site": "node scripts/check-site.mjs",
+    "check:site": "node --test site/html-text.test.mjs && node scripts/check-site.mjs",
     "api-surface:build": "node scripts/build-api-surface.mjs",
     "api-surface:check": "node scripts/build-api-surface.mjs --check",
     "check": "npm run typecheck && npm run build && npm test && npm run api-surface:check && npm run check:package && npm run check:site && npm audit --omit=dev && npm pack --dry-run --ignore-scripts",

site/build.mjs

@@ -15,6 +15,7 @@ import {
   site,
   snippets
 } from "./content.mjs";
+import { stripHtml } from "./html-text.mjs";
 
 const root = resolve(import.meta.dirname);
 const sourceRoot = resolve(root, "..");
@@ -1536,18 +1537,6 @@ function escapeAttr(value) {
   return escapeHtml(value).replaceAll("'", "'");
 }
 
-function stripHtml(value) {
-  return String(value)
-    .replace(/<li>/g, "- ")
-    .replace(/<\/li>/g, "\n")
-    .replace(/<[^>]+>/g, "")
-    .replaceAll("&amp;", "&")
-    .replaceAll("&lt;", "<")
-    .replaceAll("&gt;", ">")
-    .replaceAll("&quot;", '"')
-    .trim();
-}
-
 function sitemap() {
   const paths = [
     "/",

site/html-text.mjs

@@ -0,0 +1,33 @@
+const SAFE_TEXT_ENTITIES = Object.freeze({
+  amp: "&",
+  quot: '"',
+  "#39": "'"
+});
+
+export function stripHtml(value) {
+  const source = String(value);
+  let text = "";
+  let cursor = 0;
+
+  while (cursor < source.length) {
+    const tagStart = source.indexOf("<", cursor);
+    if (tagStart === -1) {
+      text += source.slice(cursor);
+      break;
+    }
+    text += source.slice(cursor, tagStart);
+    const tagEnd = source.indexOf(">", tagStart + 1);
+    if (tagEnd === -1) break;
+
+    const tag = source.slice(tagStart + 1, tagEnd).trim().toLowerCase();
+    if (tag === "li" || tag.startsWith("li ")) text += "- ";
+    if (tag === "/li" || tag.startsWith("/li ")) text += "\n";
+    cursor = tagEnd + 1;
+  }
+
+  return decodeSafeTextEntities(text).trim();
+}
+
+function decodeSafeTextEntities(value) {
+  return value.replace(/&(amp|quot|#39);/g, (_match, entity) => SAFE_TEXT_ENTITIES[entity]);
+}

site/html-text.test.mjs

@@ -0,0 +1,23 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+import { stripHtml } from "./html-text.mjs";
+
+test("extracts list text and decodes safe entities once", () => {
+  assert.equal(
+    stripHtml('<ul><li>One &amp; two</li><li>&quot;Three&quot; &#39;four&#39;</li></ul>'),
+    '- One & two\n- "Three" \'four\''
+  );
+});
+
+test("does not reconstruct markup from nested tags or encoded entities", () => {
+  for (const input of [
+    "<scr<script>ipt>alert(1)</script>",
+    "&lt;script&gt;alert(1)&lt;/script&gt;",
+    "&amp;lt;script&amp;gt;alert(1)",
+    "safe<script"
+  ]) {
+    const text = stripHtml(input);
+    assert.doesNotMatch(text, /<script/i);
+    assert.doesNotMatch(text, /<scr<script/i);
+  }
+});

site/llms-full.txt

@@ -545,7 +545,7 @@ publish() validates and sanitizes the event, applies endpoint event filters, enf
 Endpoint filters can select browser.session.created, browser.session.closed, browser.action.completed, browser.challenge.detected, browser.challenge.resolved, and browser.evidence.recorded. Event metadata is allowlisted by type. Control characters, credential-bearing URLs, bearer values, tokens, passwords, API keys, cookies, and secret-shaped text are removed or redacted before canonicalization. Payload size is checked after sanitation.
 
 ### Verify the signature before parsing or dispatching
-Each request includes the event type, stable delivery ID, timestamp, 128-bit nonce, key ID, and v1=<hex> signature. The signature is HMAC-SHA256 over the domain string cockroach-browser.webhook.v1, timestamp, nonce, delivery ID, key ID, and exact body, separated by newlines. verifyWebhookSignature() checks syntax, timestamp tolerance, key binding, and the signature with a timing-safe comparison. The built-in WebhookReplayGuard is a bounded in-process nonce guard and fails closed when full. A multi-process or restart-safe receiver should place the same delivery ID and nonce checks in its durable store.
+Each request includes the event type, stable delivery ID, timestamp, 128-bit nonce, key ID, and v1=&lt;hex&gt; signature. The signature is HMAC-SHA256 over the domain string cockroach-browser.webhook.v1, timestamp, nonce, delivery ID, key ID, and exact body, separated by newlines. verifyWebhookSignature() checks syntax, timestamp tolerance, key binding, and the signature with a timing-safe comparison. The built-in WebhookReplayGuard is a bounded in-process nonce guard and fails closed when full. A multi-process or restart-safe receiver should place the same delivery ID and nonce checks in its durable store.
 
 ### Deduplicate stable delivery IDs
 The normal retry path keeps one deterministic delivery ID for an event and endpoint while creating a fresh timestamp and nonce on each request. Verify the request, begin a receiver transaction, return success immediately when that delivery ID was already committed, otherwise apply the event and commit the ID with the result. This makes at-least-once attempts safe at the receiver. A manual retryDeadLetter() intentionally creates a new delivery ID. Keep the original event ID in application-level reconciliation when an operator needs to connect both attempts.

src/client.ts

@@ -50,7 +50,7 @@ export class BrowserClient {
   readonly fetcher: typeof globalThis.fetch;
 
   constructor(options: BrowserClientOptions) {
-    this.baseUrl = (options.baseUrl ?? "http://127.0.0.1:43110").replace(/\/+$/, "");
+    this.baseUrl = trimTrailingSlashes(options.baseUrl ?? "http://127.0.0.1:43110");
     this.token = options.token;
     this.fetcher = options.fetch ?? globalThis.fetch;
   }
@@ -211,3 +211,9 @@ export class BrowserClient {
     return value as T;
   }
 }
+
+function trimTrailingSlashes(value: string): string {
+  let end = value.length;
+  while (end > 0 && value.charCodeAt(end - 1) === 47) end -= 1;
+  return value.slice(0, end);
+}

src/runtime.ts

@@ -1,5 +1,5 @@
 import { lstat, mkdir, readFile, readdir, realpath, stat } from "node:fs/promises";
-import { timingSafeEqual } from "node:crypto";
+import { randomBytes, scryptSync, timingSafeEqual } from "node:crypto";
 import { homedir } from "node:os";
 import { basename, extname, isAbsolute, join, relative, resolve } from "node:path";
 import pixelmatch from "pixelmatch";
@@ -94,6 +94,7 @@ export interface BrowserRuntimeOptions {
 
 interface InternalTabLock extends TabLockSummary {
   tokenDigest: string;
+  tokenSalt: string;
 }
 
 interface ConsoleRecord {
@@ -1844,12 +1845,14 @@ export class BrowserRuntime {
           "TAB_LOCK_TTL_INVALID"
         );
         const acquiredAt = nowIso();
+        const tokenSalt = randomBytes(16);
         const lock: InternalTabLock = {
           tabId,
           owner,
           acquiredAt,
           expiresAt: new Date(Date.now() + ttlMs).toISOString(),
-          tokenDigest: sha256(token)
+          tokenDigest: deriveTabLockDigest(token, tokenSalt),
+          tokenSalt: tokenSalt.toString("base64")
         };
         session.tabLocks.set(tabId, lock);
         return { lock: publicTabLock(lock) };
@@ -2154,9 +2157,12 @@ export class BrowserRuntime {
       );
     }
     const token = await this.#resolveSecret(tokenReference);
-    const actual = Buffer.from(sha256(token), "hex");
+    const actual = Buffer.from(deriveTabLockDigest(token, Buffer.from(lock.tokenSalt, "base64")), "hex");
     const expected = Buffer.from(lock.tokenDigest, "hex");
-    if (actual.length !== expected.length || !timingSafeEqual(actual, expected)) {
+    const accepted = actual.length === expected.length && timingSafeEqual(actual, expected);
+    actual.fill(0);
+    expected.fill(0);
+    if (!accepted) {
       throw new CockroachBrowserError(
         "TAB_LOCK_DENIED",
         `Tab ${tabId} is exclusively locked by ${lock.owner} until ${lock.expiresAt}.`
@@ -2575,6 +2581,15 @@ function publicTabLock(lock: InternalTabLock): TabLockSummary {
   };
 }
 
+function deriveTabLockDigest(token: string, salt: Buffer): string {
+  const digest = scryptSync(token, salt, 32);
+  try {
+    return digest.toString("hex");
+  } finally {
+    digest.fill(0);
+  }
+}
+
 function serializeNetworkExport(
   records: BrowserNetworkRecord[],
   format: "json" | "ndjson" | "har"

test/browser-smoke.test.ts

@@ -39,6 +39,7 @@ test(
     const secrets = new Map([
       ["ref:profile-passphrase", "a-strong-profile-passphrase"],
       ["ref:tab-lock", "one-exclusive-tab-token"],
+      ["ref:wrong-tab-lock", "a-different-exclusive-tab-token"],
       ["ref:clipboard", "clipboard fixture value"],
       ["ref:storage-original", JSON.stringify({ localStorage: { fixture: "original" } })]
     ]);
@@ -329,11 +330,26 @@ test(
         error && typeof error === "object" && "code" in error && error.code === "TAB_LOCK_DENIED"
       )
     );
+    await assert.rejects(
+      runtime.act(session.id, {
+        kind: "snapshot",
+        lockTokenRef: "ref:wrong-tab-lock",
+        purpose: "Verify that a different lock secret cannot enter the locked tab"
+      }),
+      (error: unknown) => Boolean(
+        error && typeof error === "object" && "code" in error && error.code === "TAB_LOCK_DENIED"
+      )
+    );
     const lock = await runtime.act(session.id, {
       kind: "tab.lock.status",
       purpose: "Inspect the exclusive fixture tab lock"
     });
-    assert.equal((lock.output as { lock: { owner: string } }).lock.owner, "fixture-worker");
+    const lockSummary = (lock.output as { lock: { owner: string } }).lock;
+    assert.equal(lockSummary.owner, "fixture-worker");
+    assert.deepEqual(
+      Object.keys(lockSummary).sort(),
+      ["acquiredAt", "expiresAt", "owner", "tabId"]
+    );
     await runtime.act(session.id, {
       kind: "tab.unlock",
       lockTokenRef: "ref:tab-lock",

test/client.test.ts

@@ -0,0 +1,21 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+import { BrowserClient } from "../src/client.js";
+
+test("normalizes arbitrarily long trailing slash runs in one pass", () => {
+  const client = new BrowserClient({
+    baseUrl: `https://browser.example${"/".repeat(100_000)}`,
+    token: "fixture-token"
+  });
+
+  assert.equal(client.baseUrl, "https://browser.example");
+});
+
+test("preserves non-trailing slashes in the configured base URL", () => {
+  const client = new BrowserClient({
+    baseUrl: "https://browser.example/tenant/api///",
+    token: "fixture-token"
+  });
+
+  assert.equal(client.baseUrl, "https://browser.example/tenant/api");
+});