From 6a3d964df1637d41c91f430a3f6522f920efa10c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Burak=20Emre=20Kabakc=C4=B1?= <emrekabakci@gmail.com>
Date: Wed, 13 May 2026 06:03:44 +0100
Subject: [PATCH] fix(core): harden ENCRYPTION_KEY parsing and make
 worker-token clock skew configurable

---
 .../encryption-key-validation.test.ts         | 58 +++++++++++++++++++
 packages/core/src/utils/encryption.ts         | 29 ++++++----
 packages/core/src/worker/auth.ts              |  8 ++-
 3 files changed, 83 insertions(+), 12 deletions(-)
 create mode 100644 packages/core/src/__tests__/encryption-key-validation.test.ts

diff --git a/packages/core/src/__tests__/encryption-key-validation.test.ts b/packages/core/src/__tests__/encryption-key-validation.test.ts
new file mode 100644
index 000000000..228a4c13e
--- /dev/null
+++ b/packages/core/src/__tests__/encryption-key-validation.test.ts
@@ -0,0 +1,58 @@
+import { afterEach, beforeEach, describe, expect, test } from "bun:test";
+import { decrypt, encrypt } from "../utils/encryption";
+
+// Regression coverage for ENCRYPTION_KEY parsing: `Buffer.from(x, "base64")`
+// silently drops invalid characters, so a typo'd key could yield a short or
+// garbled key. Parsing must round-trip and length-check before trusting it.
+describe("ENCRYPTION_KEY validation", () => {
+  let originalKey: string | undefined;
+
+  beforeEach(() => {
+    originalKey = process.env.ENCRYPTION_KEY;
+  });
+
+  afterEach(() => {
+    if (originalKey !== undefined) {
+      process.env.ENCRYPTION_KEY = originalKey;
+    } else {
+      delete process.env.ENCRYPTION_KEY;
+    }
+  });
+
+  test("non-base64, non-hex junk key throws", () => {
+    process.env.ENCRYPTION_KEY = "this is not a valid key!! @#$%";
+    expect(() => encrypt("x")).toThrow("ENCRYPTION_KEY");
+  });
+
+  test("base64 string decoding to fewer than 32 bytes throws", () => {
+    // 16 bytes → 24-char canonical base64; passes the regex but is too short.
+    process.env.ENCRYPTION_KEY = Buffer.alloc(16, 9).toString("base64");
+    expect(() => encrypt("x")).toThrow("ENCRYPTION_KEY");
+  });
+
+  test("non-canonical base64 (chars that get silently dropped) throws", () => {
+    // Contains chars outside [A-Za-z0-9+/]; old code would drop them.
+    process.env.ENCRYPTION_KEY = `${Buffer.alloc(32, 1).toString("base64")}!!`;
+    expect(() => encrypt("x")).toThrow("ENCRYPTION_KEY");
+  });
+
+  test("valid 32-byte base64 key round-trips encrypt/decrypt", () => {
+    process.env.ENCRYPTION_KEY = Buffer.alloc(32, 42).toString("base64");
+    const enc = encrypt("base64 secret");
+    expect(decrypt(enc)).toBe("base64 secret");
+  });
+
+  test("valid 64-char hex key round-trips encrypt/decrypt", () => {
+    process.env.ENCRYPTION_KEY =
+      "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef";
+    const enc = encrypt("hex secret");
+    expect(decrypt(enc)).toBe("hex secret");
+  });
+
+  test("uppercase 64-char hex key round-trips encrypt/decrypt", () => {
+    process.env.ENCRYPTION_KEY =
+      "0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF";
+    const enc = encrypt("hex upper secret");
+    expect(decrypt(enc)).toBe("hex upper secret");
+  });
+});
diff --git a/packages/core/src/utils/encryption.ts b/packages/core/src/utils/encryption.ts
index be5039357..495e8ddda 100644
--- a/packages/core/src/utils/encryption.ts
+++ b/packages/core/src/utils/encryption.ts
@@ -16,25 +16,32 @@ function getEncryptionKey(): Buffer {
     );
   }
 
-  // Try to decode as base64 first (most common format).
-  // Buffer.from with "base64" does not throw on invalid input — it silently
-  // discards non-base64 chars — so we only need a length check here.
-  const base64Buffer = Buffer.from(key, "base64");
-  if (base64Buffer.length === 32) {
-    return base64Buffer;
+  // Try to decode as base64 first (most common format). `Buffer.from(x,
+  // "base64")` silently drops non-base64 chars rather than throwing, so a
+  // typo'd key can yield a short/garbled buffer. Require canonical base64 and
+  // a clean round-trip before trusting the decoded bytes.
+  if (/^[A-Za-z0-9+/]+={0,2}$/.test(key) && key.length % 4 === 0) {
+    const base64Buffer = Buffer.from(key, "base64");
+    if (base64Buffer.length === 32 && base64Buffer.toString("base64") === key) {
+      return base64Buffer;
+    }
   }
 
-  // Try as hex (must be exactly 64 hex characters for 32 bytes)
-  if (/^[0-9a-fA-F]{64}$/.test(key)) {
+  // Try as hex (must be exactly 64 hex characters for 32 bytes), again
+  // verifying the round-trip so partially-valid input is rejected.
+  if (/^[0-9a-fA-F]+$/.test(key) && key.length % 2 === 0) {
     const hexBuffer = Buffer.from(key, "hex");
-    if (hexBuffer.length === 32) {
+    if (
+      hexBuffer.length === 32 &&
+      hexBuffer.toString("hex") === key.toLowerCase()
+    ) {
       return hexBuffer;
     }
   }
 
   throw new Error(
-    "ENCRYPTION_KEY must be a base64 or hex encoded 32-byte key. " +
-      "Generate a valid key with: openssl rand -base64 32"
+    "ENCRYPTION_KEY must be a canonical base64 or hex encoded 32-byte key. " +
+      "Generate a valid key with: openssl rand -base64 32 (or openssl rand -hex 32)"
   );
 }
 
diff --git a/packages/core/src/worker/auth.ts b/packages/core/src/worker/auth.ts
index d4a5166df..958b92ef6 100644
--- a/packages/core/src/worker/auth.ts
+++ b/packages/core/src/worker/auth.ts
@@ -93,7 +93,13 @@ export function verifyWorkerToken(token: string): WorkerTokenData | null {
       !Number.isNaN(parsedTtl) && parsedTtl > 0
         ? parsedTtl
         : 2 * 60 * 60 * 1000;
-    const skewMs = 30 * 1000;
+    // Clock-skew tolerance between gateway and worker; override with WORKER_TOKEN_CLOCK_SKEW_MS.
+    const parsedSkew = parseInt(
+      process.env.WORKER_TOKEN_CLOCK_SKEW_MS ?? "",
+      10
+    );
+    const skewMs =
+      !Number.isNaN(parsedSkew) && parsedSkew >= 0 ? parsedSkew : 30 * 1000;
     if (Date.now() - data.timestamp > ttl + skewMs) {
       logger.error("Worker token rejected: expired");
       return null;