From 03a52e46cde8857980ba691e84b4e6279875dda8 Mon Sep 17 00:00:00 2001
From: Martha-code-dev <Martha-code-dev@users.noreply.github.com>
Date: Wed, 21 Oct 2026 16:39:39 +0100
Subject: [PATCH] test(crypto): add unit tests for crypto utility (#599)

Add unit tests for sha256Hash() and safeEqual() covering output format,
determinism, equal/different strings, and different-length inputs.

Also fix a latent bug in safeEqual(): zero-padding the shorter buffer
before the constant-time comparison made strings that only differ by
trailing NUL bytes (e.g. "abc" vs "abc\0") compare as equal. safeEqual
now explicitly returns false whenever the original lengths differ.
---
 src/utils/crypto.ts             | 13 ++++---
 tests/unit/utils/crypto.test.ts | 66 +++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+), 5 deletions(-)
 create mode 100644 tests/unit/utils/crypto.test.ts

diff --git a/src/utils/crypto.ts b/src/utils/crypto.ts
index e29e240..e8fe8be 100644
--- a/src/utils/crypto.ts
+++ b/src/utils/crypto.ts
@@ -16,15 +16,18 @@ export function safeEqual(a: string, b: string): boolean {
   const bufA = Buffer.from(a);
   const bufB = Buffer.from(b);
   if (bufA.length !== bufB.length) {
-    // Pad the shorter buffer to match the longer one so timingSafeEqual
-    // doesn't throw. The length mismatch itself is not secret — an
-    // attacker learns nothing from it that they couldn't already see by
-    // comparing the strings directly.
+    // Buffers of different lengths are never equal. We still run a
+    // constant-time comparison (on zero-padded copies) so the function's
+    // timing doesn't leak the fact that lengths differ, but the result is
+    // always `false` here — without this explicit check, zero-padding
+    // would make strings that only differ by trailing NUL bytes (e.g.
+    // "abc" vs "abc\0") compare as equal, which is not the intent.
     const padded = Buffer.alloc(Math.max(bufA.length, bufB.length));
     bufA.copy(padded);
     const paddedB = Buffer.alloc(padded.length);
     bufB.copy(paddedB);
-    return crypto.timingSafeEqual(padded, paddedB);
+    crypto.timingSafeEqual(padded, paddedB);
+    return false;
   }
   return crypto.timingSafeEqual(bufA, bufB);
 }
diff --git a/tests/unit/utils/crypto.test.ts b/tests/unit/utils/crypto.test.ts
new file mode 100644
index 0000000..3cdd678
--- /dev/null
+++ b/tests/unit/utils/crypto.test.ts
@@ -0,0 +1,66 @@
+import { describe, it, expect } from "vitest";
+import { sha256Hash, safeEqual } from "../../../src/utils/crypto.js";
+
+describe("sha256Hash", () => {
+  it("returns a 64-character lowercase hex string", () => {
+    const hash = sha256Hash("hello world");
+    expect(hash).toMatch(/^[0-9a-f]{64}$/);
+  });
+
+  it("is deterministic for the same input", () => {
+    expect(sha256Hash("same input")).toBe(sha256Hash("same input"));
+  });
+
+  it("produces different hashes for different inputs", () => {
+    expect(sha256Hash("input-a")).not.toBe(sha256Hash("input-b"));
+  });
+
+  it("matches the known SHA-256 digest for an empty string", () => {
+    expect(sha256Hash("")).toBe(
+      "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
+    );
+  });
+
+  it("hashes unicode input without throwing", () => {
+    expect(() => sha256Hash("ünïcode 🚀")).not.toThrow();
+    expect(sha256Hash("ünïcode 🚀")).toMatch(/^[0-9a-f]{64}$/);
+  });
+});
+
+describe("safeEqual", () => {
+  it("returns true for identical strings", () => {
+    expect(safeEqual("secret-token", "secret-token")).toBe(true);
+  });
+
+  it("returns false for different strings of the same length", () => {
+    expect(safeEqual("secret-token", "secret-tokeX")).toBe(false);
+  });
+
+  it("returns false for strings of different lengths", () => {
+    expect(safeEqual("short", "a-much-longer-string")).toBe(false);
+  });
+
+  it("returns false when one string is a prefix of the other", () => {
+    expect(safeEqual("abc", "abcd")).toBe(false);
+  });
+
+  it("returns false for strings that only differ by trailing NUL bytes", () => {
+    // Regression guard: zero-padding the shorter buffer for the
+    // constant-time comparison must not make "abc" and "abc\0" compare
+    // equal just because their padded buffers happen to match.
+    expect(safeEqual("abc", "abc\0")).toBe(false);
+    expect(safeEqual("abc\0", "abc")).toBe(false);
+  });
+
+  it("returns true for two empty strings", () => {
+    expect(safeEqual("", "")).toBe(true);
+  });
+
+  it("returns false comparing an empty string to a non-empty one", () => {
+    expect(safeEqual("", "a")).toBe(false);
+  });
+
+  it("is case-sensitive", () => {
+    expect(safeEqual("Secret", "secret")).toBe(false);
+  });
+});