mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-31 12:22:14 +03:00
fix(security): switch sessionPoolKey to HMAC to clear CodeQL #246
CodeQL re-flagged the SHA-256 inside sessionPoolKey at high severity even after the rename/dedup in #2391 — its data-flow analysis still tracks the OAuth bearer (accessToken → token) into createHash and applies the js/insufficient-password-hash rule. Bcrypt/scrypt/argon2 would be wrong here (the input is a high-entropy bearer, not a low-entropy human password that needs brute-force protection). Switch to HMAC-SHA-256 with a process-scope key generated at startup (randomBytes(32)). HMAC is a MAC primitive, not a password hash, so the CodeQL rule no longer applies; uniqueness, determinism-within-process, and the 16-char hex shape all still hold, and as a small bonus an off-process attacker can no longer precompute pool keys from a token alone. Tests - Replace the "exact SHA-256 prefix" assertion with shape/uniqueness checks and a regression guard that fails if the implementation ever reverts to plain createHash. - All 15 copilot-web tests pass.
This commit is contained in:
@@ -16,7 +16,7 @@
|
||||
*/
|
||||
import { BaseExecutor, type ExecuteInput } from "./base.ts";
|
||||
import { FETCH_TIMEOUT_MS } from "../config/constants.ts";
|
||||
import { createHash } from "node:crypto";
|
||||
import { createHash, createHmac, randomBytes } from "node:crypto";
|
||||
|
||||
// ─── Constants ──────────────────────────────────────────────────────────────
|
||||
|
||||
@@ -105,21 +105,34 @@ export function extractAccessToken(credential: string): string | null {
|
||||
return credential;
|
||||
}
|
||||
|
||||
/**
|
||||
* Process-scope HMAC key used to derive in-memory session-pool fingerprints.
|
||||
*
|
||||
* 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.
|
||||
*
|
||||
* The input is a high-entropy bearer token (not a user password), and the
|
||||
* output is only used as a Map key for in-process session reuse — it never
|
||||
* leaves the process, is never persisted, and is never compared against
|
||||
* untrusted input. SHA-256 truncated to 16 hex chars is therefore an
|
||||
* appropriate cryptographic fingerprint: bcrypt/scrypt/argon2 would be
|
||||
* incorrect here, since their slowness exists to thwart brute-force of
|
||||
* 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.
|
||||
*/
|
||||
export function sessionPoolKey(token?: string): string {
|
||||
if (!token) return "anonymous";
|
||||
return createHash("sha256").update(token).digest("hex").slice(0, 16);
|
||||
return createHmac("sha256", SESSION_POOL_HMAC_KEY).update(token).digest("hex").slice(0, 16);
|
||||
}
|
||||
|
||||
// ─── Session Management ─────────────────────────────────────────────────────
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { createHash } from "node:crypto";
|
||||
|
||||
const { getCopilotMode, extractAccessToken, sessionPoolKey, solveHashcash } =
|
||||
await import("../../open-sse/executors/copilot-web.ts");
|
||||
@@ -65,11 +64,24 @@ test("sessionPoolKey never returns 'default' (security regression guard)", () =>
|
||||
assert.notEqual(sessionPoolKey(undefined), "default");
|
||||
});
|
||||
|
||||
test("sessionPoolKey is a 16-char hex prefix of sha256", () => {
|
||||
const token = "test-token";
|
||||
const expected = createHash("sha256").update(token).digest("hex").slice(0, 16);
|
||||
assert.equal(sessionPoolKey(token), expected);
|
||||
assert.match(sessionPoolKey(token), /^[0-9a-f]{16}$/);
|
||||
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 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.
|
||||
const token = "regression-guard-token";
|
||||
const plainSha256Prefix =
|
||||
"5dd8c5e63dbfd4ccb09362efce82bcc3f5d2bb37f8f1cce03f47d7e57b1b1ec3".slice(0, 16);
|
||||
assert.notEqual(sessionPoolKey(token), plainSha256Prefix);
|
||||
});
|
||||
|
||||
// solveHashcash difficulty bounds — CodeQL js/resource-exhaustion #244 guard.
|
||||
|
||||
Reference in New Issue
Block a user