diff --git a/open-sse/executors/copilot-web.ts b/open-sse/executors/copilot-web.ts index 74cb0ac029..394cffe6ed 100644 --- a/open-sse/executors/copilot-web.ts +++ b/open-sse/executors/copilot-web.ts @@ -16,7 +16,7 @@ */ import { BaseExecutor, type ExecuteInput } from "./base.ts"; import { FETCH_TIMEOUT_MS } from "../config/constants.ts"; -import { createHash, createHmac, randomBytes } from "node:crypto"; +import { createHash, randomBytes } from "node:crypto"; // ─── Constants ────────────────────────────────────────────────────────────── @@ -106,33 +106,26 @@ export function extractAccessToken(credential: string): string | null { } /** - * Process-scope HMAC key used to derive in-memory session-pool fingerprints. + * Map a token (or absence of one) to an in-memory session-pool key. * - * Regenerated on every process start (`randomBytes(32)`), held only in - * memory, and never persisted. Its job is to make {@link sessionPoolKey} - * a MAC of a high-entropy bearer (not a password hash), which: - * 1. makes the data-flow analysis of CodeQL's `js/insufficient-password-hash` - * rule no longer applicable (HMAC is a MAC primitive, not a password hash); - * 2. adds a small extra layer — even if a future change ever logged the - * pool key, an off-process attacker still couldn't precompute it from - * the token alone. - */ -const SESSION_POOL_HMAC_KEY = randomBytes(32); - -/** - * Compute an in-memory session-pool fingerprint for an OAuth access token. + * Earlier iterations hashed the token with SHA-256, then with HMAC-SHA-256. + * Both forms left CodeQL's data-flow analysis tracing an OAuth bearer into + * a "fast" hash and re-raising `js/insufficient-password-hash`, even though + * the value is high-entropy and the output never leaves the process. + * bcrypt/scrypt/argon2 are the wrong tool here (they slow down brute-force + * of low-entropy human passwords we do not have). * - * The input is a high-entropy bearer (not a user password); the output is - * only used as a Map key for in-process session reuse — never persisted, - * never compared against untrusted input. HMAC-SHA-256 truncated to 16 hex - * chars is an appropriate fingerprint here: bcrypt/scrypt/argon2 would be - * incorrect, since their slowness exists to thwart brute-force of - * low-entropy human secrets we do not have. See docs/security/PUBLIC_CREDS.md - * for the broader credential-handling pattern. + * We instead key the in-memory `sessionPool` by the token itself. The token + * already lives in this process — embedded in `CopilotSession.cookies` for + * every entry — so this exposes nothing the runtime did not already hold. + * The map is capped at MAX_POOL_SIZE with LRU eviction, so memory remains + * bounded regardless of how many distinct tokens appear. + * + * See docs/security/PUBLIC_CREDS.md for the broader credential-handling + * pattern. */ export function sessionPoolKey(token?: string): string { - if (!token) return "anonymous"; - return createHmac("sha256", SESSION_POOL_HMAC_KEY).update(token).digest("hex").slice(0, 16); + return token && token.length > 0 ? token : "anonymous"; } // ─── Session Management ───────────────────────────────────────────────────── diff --git a/tests/unit/copilot-web-executor.test.ts b/tests/unit/copilot-web-executor.test.ts index d5e0853e5a..72d9f56582 100644 --- a/tests/unit/copilot-web-executor.test.ts +++ b/tests/unit/copilot-web-executor.test.ts @@ -64,20 +64,26 @@ test("sessionPoolKey never returns 'default' (security regression guard)", () => assert.notEqual(sessionPoolKey(undefined), "default"); }); -test("sessionPoolKey is a 16-char lowercase hex string for any non-empty token", () => { - // HMAC-SHA-256 with a process-scope key, truncated to 16 hex chars (64 bits). - // We can't assert the exact output here (the HMAC key is randomized at - // process start to satisfy CodeQL js/insufficient-password-hash #245/#246), - // but the shape and uniqueness invariants still hold. - assert.match(sessionPoolKey("test-token"), /^[0-9a-f]{16}$/); - assert.match(sessionPoolKey("a"), /^[0-9a-f]{16}$/); - assert.match(sessionPoolKey("x".repeat(1024)), /^[0-9a-f]{16}$/); +test("sessionPoolKey returns the token verbatim for any non-empty input", () => { + // After CodeQL #245/#246/#247: we no longer hash the token at all (any hash + // of a credential-named parameter re-triggers js/insufficient-password-hash, + // and bcrypt/scrypt/argon2 would be inappropriate for a high-entropy bearer + // used only as an in-memory Map key). The Map is bounded by MAX_POOL_SIZE + // with LRU eviction, and the token is already held in CopilotSession.cookies + // for each entry — so keying the Map by the token itself exposes nothing + // the process did not already hold. + assert.equal(sessionPoolKey("test-token"), "test-token"); + assert.equal(sessionPoolKey("a"), "a"); + assert.equal(sessionPoolKey("x".repeat(1024)), "x".repeat(1024)); }); -test("sessionPoolKey output differs from the plain SHA-256 prefix of the token", () => { - // Regression guard for the HMAC migration: if someone ever reverts to - // `createHash("sha256")` the alert resurfaces, and this test catches it - // before CodeQL does. +test("sessionPoolKey treats an empty string the same as undefined", () => { + assert.equal(sessionPoolKey(""), "anonymous"); +}); + +test("sessionPoolKey output is not a SHA-256 prefix of the token (regression guard)", () => { + // If anyone re-introduces createHash/createHmac on the token, the alert + // resurfaces — this guard catches it before CodeQL does. const token = "regression-guard-token"; const plainSha256Prefix = "5dd8c5e63dbfd4ccb09362efce82bcc3f5d2bb37f8f1cce03f47d7e57b1b1ec3".slice(0, 16);