Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions packages/core/src/__tests__/encryption-key-validation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,15 @@ describe("ENCRYPTION_KEY validation", () => {
expect(decrypt(enc)).toBe("base64 secret");
});

test("valid 32-byte URL-safe base64 key round-trips encrypt/decrypt", () => {
// Historical keys were sometimes generated with `openssl rand -base64 32 |
// tr +/ -_` and stored in URL-safe form (alphabet [A-Za-z0-9_-], no
// padding). Same 32 bytes — must be accepted.
process.env.ENCRYPTION_KEY = Buffer.alloc(32, 99).toString("base64url");
const enc = encrypt("urlsafe secret");
expect(decrypt(enc)).toBe("urlsafe secret");
});
Comment on lines +45 to +52
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

This regression can pass without exercising the URL-safe parsing path.

Because getEncryptionKey() caches the first valid key, this test may reuse the previous test’s base64 key instead of the URL-safe key set on Line 49.

Proposed patch
-import { decrypt, encrypt } from "../utils/encryption";
+import {
+  __resetEncryptionKeyCacheForTests,
+  decrypt,
+  encrypt,
+} from "../utils/encryption";
@@
   beforeEach(() => {
     originalKey = process.env.ENCRYPTION_KEY;
+    __resetEncryptionKeyCacheForTests();
   });
@@
   afterEach(() => {
     if (originalKey !== undefined) {
       process.env.ENCRYPTION_KEY = originalKey;
     } else {
       delete process.env.ENCRYPTION_KEY;
     }
+    __resetEncryptionKeyCacheForTests();
   });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/__tests__/encryption-key-validation.test.ts` around lines
45 - 52, The test is intermittently passing because getEncryptionKey() caches
the first valid key, so set process.env.ENCRYPTION_KEY after clearing that
cache; either call jest.resetModules() before requiring/importing
encrypt/decrypt or add and call a small exported reset function (e.g.,
resetEncryptionKeyCache) that clears the module-level cached key used by
getEncryptionKey() so the URL-safe key is actually parsed during this test.


test("valid 64-char hex key round-trips encrypt/decrypt", () => {
process.env.ENCRYPTION_KEY =
"0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef";
Expand Down
15 changes: 15 additions & 0 deletions packages/core/src/utils/encryption.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,21 @@ function getEncryptionKey(): Buffer {
}
}

// Try as URL-safe base64 (alphabet [A-Za-z0-9_-], no padding). Historically
// some keys were generated as `openssl rand -base64 32 | tr +/ -_` and stored
// in this form; same 32 bytes, just a different alphabet. Apply the same
// round-trip check so typos still get rejected.
if (/^[A-Za-z0-9_-]+$/.test(key)) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Accept the padded URL-safe keys this hotfix targets

For URL-safe keys generated by the documented historical command openssl rand -base64 32 | tr +/ -_, the trailing base64 padding is preserved (=), so 32-byte keys are 44 chars and end in =. This regex rejects those keys before decoding, and the later round-trip would also compare Node's unpadded base64url output to the padded input, so deployments with the stated production key format will continue throwing from getEncryptionKey() and 500ing on encrypt/decrypt paths.

Useful? React with 👍 / 👎.

const urlsafeBuffer = Buffer.from(key, "base64url");
if (
urlsafeBuffer.length === 32 &&
urlsafeBuffer.toString("base64url") === key
) {
cachedKey = urlsafeBuffer;
return urlsafeBuffer;
}
}
Comment on lines +37 to +50
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update the validation error text to mention URL-safe base64.

Line 66 still says only canonical base64 or hex are accepted, which is now outdated and can mislead incident triage.

Proposed patch
-  throw new Error(
-    "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)"
-  );
+  throw new Error(
+    "ENCRYPTION_KEY must be a canonical base64, URL-safe base64 (unpadded), or hex encoded 32-byte key. " +
+      "Generate a valid key with: openssl rand -base64 32, openssl rand -base64 32 | tr +/ -_, or openssl rand -hex 32"
+  );

Also applies to: 65-68

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/utils/encryption.ts` around lines 37 - 50, The validation
error message that currently claims only "canonical base64 or hex" are accepted
must be updated to mention URL-safe base64 as well; locate the key
validation/throw site in the same function where cachedKey is set (the URL-safe
check using /^[A-Za-z0-9_-]+$/ and Buffer.from(key, "base64url") is performed)
and change the thrown/returned error text to include "URL-safe base64" (or
"base64url") alongside canonical base64 and hex so the error accurately reflects
all accepted formats.


// 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) {
Expand Down
Loading