diff --git a/docs/webhooks.md b/docs/webhooks.md index 76fde4c..7bb9b43 100644 --- a/docs/webhooks.md +++ b/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= 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=<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. ``` import { diff --git a/package.json b/package.json index 361e120..fc14342 100644 --- a/package.json +++ b/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", diff --git a/site/build.mjs b/site/build.mjs index 1f2cef9..321998a 100644 --- a/site/build.mjs +++ b/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(/
  • /g, "- ") - .replace(/<\/li>/g, "\n") - .replace(/<[^>]+>/g, "") - .replaceAll("&", "&") - .replaceAll("<", "<") - .replaceAll(">", ">") - .replaceAll(""", '"') - .trim(); -} - function sitemap() { const paths = [ "/", diff --git a/site/html-text.mjs b/site/html-text.mjs new file mode 100644 index 0000000..8ba0f37 --- /dev/null +++ b/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]); +} diff --git a/site/html-text.test.mjs b/site/html-text.test.mjs new file mode 100644 index 0000000..e5468cf --- /dev/null +++ b/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(''), + '- One & two\n- "Three" \'four\'' + ); +}); + +test("does not reconstruct markup from nested tags or encoded entities", () => { + for (const input of [ + "ipt>alert(1)", + "<script>alert(1)</script>", + "&lt;script&gt;alert(1)", + "safe 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=<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. ### 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. diff --git a/src/client.ts b/src/client.ts index bfd8be5..5dc5ecf 100644 --- a/src/client.ts +++ b/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); +} diff --git a/src/runtime.ts b/src/runtime.ts index f0ee5e6..3e3fdd7 100644 --- a/src/runtime.ts +++ b/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" diff --git a/test/browser-smoke.test.ts b/test/browser-smoke.test.ts index 5db63e5..fb30dfe 100644 --- a/test/browser-smoke.test.ts +++ b/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", diff --git a/test/client.test.ts b/test/client.test.ts new file mode 100644 index 0000000..30780d3 --- /dev/null +++ b/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"); +});