From fb9e6e451ef5f7ddaa000f0cf106ca2c79de3397 Mon Sep 17 00:00:00 2001
From: Kieran Klukas <kieran@dunkirk.sh>
Date: Mon, 27 Jul 2026 04:56:55 -0400
Subject: [PATCH] feat: use timing safe comparison

---
 src/lib/secrets.ts        | 27 +++++++++++++++++++++++++++
 src/routes/clients.ts     |  6 +-----
 src/routes/oauth/token.ts |  8 ++------
 test/secrets.test.ts      | 33 +++++++++++++++++++++++++++++++++
 4 files changed, 63 insertions(+), 11 deletions(-)
 create mode 100644 src/lib/secrets.ts
 create mode 100644 test/secrets.test.ts

diff --git a/src/lib/secrets.ts b/src/lib/secrets.ts
new file mode 100644
index 0000000..54e90c2
--- /dev/null
+++ b/src/lib/secrets.ts
@@ -0,0 +1,27 @@
+import crypto from "node:crypto";
+
+// Client secrets are stored as sha256 hex digests.
+export function hashSecret(secret: string): string {
+	return crypto.createHash("sha256").update(secret).digest("hex");
+}
+
+// Compare a presented secret against a stored sha256 hex digest in constant
+// time. String equality short-circuits on the first differing byte, which
+// leaks how much of the secret is correct; compare the raw digests instead.
+export function verifySecret(presented: string, storedHashHex: string): boolean {
+	const presentedHash = crypto
+		.createHash("sha256")
+		.update(presented)
+		.digest();
+
+	const storedHash = Buffer.from(storedHashHex, "hex");
+
+	// Different lengths can't match, but still burn a comparison to avoid
+	// leaking length via early return.
+	if (storedHash.length !== presentedHash.length) {
+		crypto.timingSafeEqual(presentedHash, presentedHash);
+		return false;
+	}
+
+	return crypto.timingSafeEqual(presentedHash, storedHash);
+}
diff --git a/src/routes/clients.ts b/src/routes/clients.ts
index a44fce1..c41ceba 100644
--- a/src/routes/clients.ts
+++ b/src/routes/clients.ts
@@ -1,12 +1,8 @@
-import crypto from "node:crypto";
 import { nanoid } from "nanoid";
 import { db } from "../db";
+import { hashSecret } from "../lib/secrets";
 import { getSessionUser } from "../lib/session";
 
-function hashSecret(secret: string): string {
-	return crypto.createHash("sha256").update(secret).digest("hex");
-}
-
 function generateClientSecret(): string {
 	return `iks_${nanoid(43)}`; // indiko secret
 }
diff --git a/src/routes/oauth/token.ts b/src/routes/oauth/token.ts
index 8a47c0f..e663a98 100644
--- a/src/routes/oauth/token.ts
+++ b/src/routes/oauth/token.ts
@@ -7,6 +7,7 @@ import {
 	unauthorizedResponse,
 } from "../../lib/oauth/errors";
 import { canonicalizeURL, verifyPKCE } from "../../lib/oauth/urls";
+import { verifySecret } from "../../lib/secrets";
 import { signIDToken } from "../../oidc";
 
 const ACCESS_TOKEN_TTL = 3600; // 1 hour
@@ -343,12 +344,7 @@ function verifyClientCredentials(
 		return oauthError(500, "server_error", "Client secret not configured");
 	}
 
-	const providedSecretHash = crypto
-		.createHash("sha256")
-		.update(client_secret)
-		.digest("hex");
-
-	if (providedSecretHash !== app.client_secret_hash) {
+	if (!verifySecret(client_secret, app.client_secret_hash)) {
 		return unauthorizedResponse("invalid_client", "Invalid client_secret");
 	}
 
diff --git a/test/secrets.test.ts b/test/secrets.test.ts
new file mode 100644
index 0000000..cef9dbb
--- /dev/null
+++ b/test/secrets.test.ts
@@ -0,0 +1,33 @@
+import { describe, expect, test } from "bun:test";
+import { hashSecret, verifySecret } from "../src/lib/secrets";
+
+describe("hashSecret", () => {
+	test("produces a sha256 hex digest", () => {
+		expect(hashSecret("iks_test")).toMatch(/^[0-9a-f]{64}$/);
+	});
+
+	test("is deterministic", () => {
+		expect(hashSecret("abc")).toBe(hashSecret("abc"));
+	});
+});
+
+describe("verifySecret", () => {
+	test("accepts the correct secret", () => {
+		const hash = hashSecret("iks_correct-horse-battery-staple");
+		expect(verifySecret("iks_correct-horse-battery-staple", hash)).toBe(true);
+	});
+
+	test("rejects a wrong secret", () => {
+		const hash = hashSecret("correct");
+		expect(verifySecret("wrong", hash)).toBe(false);
+	});
+
+	test("rejects a secret differing only in the last char (no early-exit leak)", () => {
+		const hash = hashSecret("almost-the-same-secret");
+		expect(verifySecret("almost-the-same-secreX", hash)).toBe(false);
+	});
+
+	test("handles a malformed stored hash without throwing", () => {
+		expect(verifySecret("anything", "not-valid-hex")).toBe(false);
+	});
+});
-- 
2.51.2