Backport login-CSRF fix (GHSA-xf67-jxfx-jf88) to 1.x (Harper 4)

[heskew]

What this ports

This is the 1.x / Harper 4 backport of the login-CSRF fix already released on the 2.x line in v2.5.0 under GHSA-xf67-jxfx-jf88.

Two security items and one low-severity hardening fix are included:

Browser-secret binding (__Host-oauth_browser cookie) — handleLogin mints or reuses a stable per-browser secret, stores hash(secret) as browserNonceHash in the CSRF state, and handleCallback verifies the hash constant-time before any code exchange or session write. Tokens minted before the upgrade (in-flight at deploy time) pass through without the check. Implemented in the new self-contained src/lib/browserBinding.ts module.

State↔session binding — the existing session-id binding introduced in #185 is preserved unchanged (tokenData.sessionId && tokenData.sessionId !== request.session?.id).

CRLF log injection fix (CWE-117) — browser-controlled error and error_description callback parameters are now passed through JSON.stringify before logging.

Scope

This branch contains only the security backport. The peerDependencies.harperdb engine-pin tightening (>=4.6.0 <5.0.0) is tracked separately in #214 and is not included here; the package.json in this branch is identical to 1.6.1's.

Version bump and release steps are handled separately — this PR targets v1.x as a pre-release checkpoint.

Test results

tests 390 | pass 388 | fail 0 | cancelled 0 | skipped 2

All 388 non-skipped tests pass. The 2 skipped tests are pre-existing environment-dependent skips (Harper table availability in unit context).

Verification

Build: npm run build — clean (tsc emits no errors).
Tests: npm test on the worktree — 388/388 passing.

Complexity: Low — isolated module addition + targeted handler changes; no new external dependencies.

[github-actions]

Reviewed; no blockers found.

[gemini-code-assist]

Code Review

This pull request introduces a stable per-browser secret cookie (__Host-oauth_browser) to bind the human login flow to the initiating browser, mitigating login-CSRF attacks for logged-out requests. It adds helpers for generating, hashing, reading, and matching the secret, integrates them into the login and callback handlers, and includes comprehensive unit tests. The review feedback suggests validating the format and length of the read cookie value to prevent CPU exhaustion attacks from hashing excessively long inputs, and defensively verifying that both arguments in browserSecretMatches are strings to avoid runtime TypeErrors.

[gemini-code-assist]

To prevent potential CPU exhaustion attacks from hashing extremely long cookie values, and to ensure we only reuse well-formed secrets, we should validate the format and length of the cookie value before returning it. Since the secret is generated as a 32-byte cryptographically secure base64url string (which is 43 characters), we can enforce a safe regex pattern (e.g., /^[A-Za-z0-9_-]{1,64}$/) to allow standard test mocks while rejecting any excessively long or malformed inputs.

if (part.slice(0, eq).trim() === BROWSER_SECRET_COOKIE_NAME) {
	const value = part.slice(eq + 1).trim();
	return /^[A-Za-z0-9_-]{1,64}$/.test(value) ? value : undefined;
}

[gemini-code-assist]

In JavaScript/TypeScript, any value can be thrown or passed at runtime. If secret or expectedHash is not a string (e.g., due to unexpected database values or malformed inputs), calling Buffer.from or hashBrowserSecret on them could throw a runtime TypeError, potentially leading to an unhandled exception in the callback handler. We should defensively verify that both arguments are non-empty strings before performing the constant-time comparison.

Suggested change

	export function browserSecretMatches(secret: string | undefined, expectedHash: string | undefined): boolean {
	if (!secret || !expectedHash) return false;
	const actual = Buffer.from(hashBrowserSecret(secret));
	const expected = Buffer.from(expectedHash);
	return actual.length === expected.length && timingSafeEqual(actual, expected);
	export function browserSecretMatches(secret: string | undefined, expectedHash: string | undefined): boolean {
	if (typeof secret !== 'string' || typeof expectedHash !== 'string' || !secret || !expectedHash) return false;
	const actual = Buffer.from(hashBrowserSecret(secret));
	const expected = Buffer.from(expectedHash);
	return actual.length === expected.length && timingSafeEqual(actual, expected);
	}

References

1. For functions that guarantee a 'never throws' contract (such as cryptographic verification or key selection), defensively guard against null, primitive, or malformed elements before accessing their properties to prevent runtime TypeErrors.