Skip to content

Conversation

@halvaradop
Copy link
Member

@halvaradop halvaradop commented Feb 5, 2026

Description

This pull request adds entropy verification for weak secrets via the createSecret function, which is used internally by createJWT, createJWE, createJWS, and createDeriveKeys. The validation ensures that secrets meet minimum security requirements: at least 32 bits of length and a minimum of 4 bits of entropy per character, helping prevent the use of weak or guessable secrets.

Additionally, all utilities defined in secret.ts are now re-exported from the entry module of the @aura-stack/jose package.

Following the same approach, the createSecret function was also implemented in the core module of @aura-stack/auth to validate the salt used during key derivation, ensuring it meets the same strength and entropy requirements.

Summary by CodeRabbit

  • New Features

    • Secret validation now enforces minimum byte-length and per-character entropy.
  • Improvements

    • Better error reporting for invalid salt/secret values, with a dedicated internal error code.
    • Secret-related utilities are publicly exposed for reuse.
  • Documentation

    • Tests and changelogs updated to reflect entropy and validation rules.

@vercel
Copy link

vercel bot commented Feb 5, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
auth-nextjs-demo Ready Ready Preview, Comment Feb 5, 2026 5:28pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
auth Skipped Skipped Feb 5, 2026 5:28pm

@coderabbitai
Copy link

coderabbitai bot commented Feb 5, 2026

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Adds runtime secret entropy and byte-length validation: new error code INVALID_SALT_SECRET_VALUE; core salt fallback and validation via createSecret; getEntropy and MIN_SECRET_ENTROPY_BITS = 4; Vitest SALT env wiring; removal of secret from a test preset.

Changes

Cohort / File(s) Summary
Auth types
packages/core/src/@types/index.ts
Added new internal error code literal "INVALID_SALT_SECRET_VALUE".
Core salt wiring & validation
packages/core/src/jose.ts
Salt derivation now falls back to "Not found" when unset and calls createSecret(salt); validation failures throw AuthInternalError with code INVALID_SALT_SECRET_VALUE.
Test presets / env
packages/core/test/presets.ts, packages/core/vitest.config.ts
Removed secret from test preset; added SALT_KEY and injected it as AURA_AUTH_SALT in Vitest env.
Secret utilities & exports
packages/jose/src/secret.ts, packages/jose/src/index.ts, packages/jose/CHANGELOG.md
Added getEntropy(secret: string) and MIN_SECRET_ENTROPY_BITS = 4; createSecret(secret, length = 32) now rejects falsy secrets, enforces byte-length >= length, and enforces entropy >= MIN_SECRET_ENTROPY_BITS; re-exported secret utilities; changelog documents entropy checks.
Tests
packages/jose/test/index.test.ts
Updated tests to expect 32‑byte length and entropy validation errors; added null/undefined rejection expectations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I counted bytes beneath the moon,
Four bits per hop, a secret tune,
Thirty-two bytes and entropy bright,
I guard the salt through day and night,
A tiny rabbit keeps keys tight 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main change: adding entropy verification for weak secrets, which is the primary feature across the @aura-stack/jose package.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/add-entropy-verification

Comment @coderabbitai help to get the list of available commands and usage tips.

@halvaradop halvaradop added enhancement New feature or request security Security-related changes, vulnerability fixes, or hardening measures. labels Feb 5, 2026
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In `@packages/core/src/jose.ts`:
- Around line 26-28: Wrap the call to createSecret(salt) in a try/catch so any
InvalidSecretError thrown by the jose library is caught and rethrown as the core
error code INVALID_SALT_SECRET_VALUE (preserving the original error
message/stack as inner/cause). Specifically, surround the existing
createSecret(salt) invocation with error handling that detects
jose.InvalidSecretError (or any thrown error) and throws a new core error (use
your project's core error class/constructor and code INVALID_SALT_SECRET_VALUE)
so salt validation failures are reported through the core taxonomy; keep
createDerivedSalt(secret) and the salt assignment unchanged.

In `@packages/jose/CHANGELOG.md`:
- Line 13: Update the changelog entry text to accurately reflect createSecret's
validation: change "32 characters" to "32 bytes", correct "6 bits" to "4 bits"
of entropy per character, and fix the typo "signin" to "signing"; reference the
createSecret function name in the entry so readers know the behavior is enforced
by that function.

In `@packages/jose/src/secret.ts`:
- Around line 29-38: The validation mismatches: change the byteLength check and
entropy threshold/messages to be consistent—use secret.length (character count)
instead of new TextEncoder().encode(secret).byteLength when enforcing the "at
least 32 characters" rule, update the entropy threshold check on
getEntropy(secret) to compare against 6 (not 4) if getEntropy returns bits per
character, and revise the thrown InvalidSecretError messages to accurately
reference "characters" and "6 bits per character" for the checks in the secret
validation block (functions/identifiers: secret, TextEncoder usage, getEntropy,
InvalidSecretError).
🧹 Nitpick comments (1)
packages/jose/test/index.test.ts (1)

213-216: Rename test to reflect entropy failure.
The test name says “with at least 32 bytes,” but it now expects a low-entropy rejection. Renaming avoids confusion when scanning failures.

✏️ Suggested rename
-    test("createSecret with string secret with at least 32 bytes", () => {
+    test("createSecret rejects low-entropy string secret (>= 32 bytes)", () => {
         const secretString = "this-is-a-very-secure-and-long-secret"
         expect(() => createSecret(secretString)).toThrow("Secret string must have an entropy of at least 6 bits per character")
     })

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/jose/src/secret.ts (1)

30-43: ⚠️ Potential issue | 🟠 Major

Validate byte length for non-string secrets too.
Uint8Array secrets bypass the minimum-length enforcement, so very short binary secrets pass validation. That undermines the new security guarantees.

🔒 Proposed fix
 export const createSecret = (secret: SecretInput, length: number = 32) => {
     if (!Boolean(secret)) throw new InvalidSecretError("Secret is required")
     if (typeof secret === "string") {
         const byteLength = new TextEncoder().encode(secret).byteLength
         if (byteLength < length) {
             throw new InvalidSecretError(`Secret string must be at least ${length} bytes long`)
         }
         const entropy = getEntropy(secret)
         if (entropy < MIN_SECRET_ENTROPY_BITS) {
             throw new InvalidSecretError("Secret string must have an entropy of at least 6 bits per character")
         }
         return new Uint8Array(Buffer.from(secret, "utf-8"))
     }
-    return secret
+    if (secret instanceof Uint8Array && secret.byteLength < length) {
+        throw new InvalidSecretError(`Secret must be at least ${length} bytes long`)
+    }
+    return secret
 }

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@packages/jose/test/index.test.ts`:
- Around line 213-216: The test expectation and the thrown error string are
inconsistent with the MIN_SECRET_ENTROPY_BITS constant (set to 4); update the
error text in the createSecret implementation (packages/jose/src/secret.ts -
function createSecret) to say "Secret string must have an entropy of at least 4
bits per character" and then update the test (packages/jose/test/index.test.ts -
the test "createSecret with string secret with at least 32 bytes") to expect the
same exact message so both the thrown error and the assertion match the
MIN_SECRET_ENTROPY_BITS value.
🧹 Nitpick comments (1)
packages/jose/test/index.test.ts (1)

207-237: Redundant undefined-secret coverage—consider consolidating.
The new undefined test duplicates the earlier “createSecret without secret” case. Optional: merge null/undefined cases into a single table-driven test for concision.

♻️ Consolidation idea
-    test("createSecret without secret", () => {
-        const secret = undefined
-        expect(() => createSecret(secret as unknown as string)).toThrow("Secret is required")
-    })
...
-    test("createSecret with null secret", () => {
-        const secret = null
-        expect(() => createSecret(secret as unknown as string)).toThrow("Secret is required")
-    })
-
-    test("createSecret with undefined secret", () => {
-        const secret = undefined
-        expect(() => createSecret(secret as unknown as string)).toThrow("Secret is required")
-    })
+    test.each([
+        ["null", null],
+        ["undefined", undefined],
+    ])("createSecret with %s secret", (_label, secret) => {
+        expect(() => createSecret(secret as unknown as string)).toThrow("Secret is required")
+    })

@vercel vercel bot temporarily deployed to Preview – auth February 5, 2026 17:28 Inactive
@halvaradop halvaradop merged commit 13265a0 into master Feb 5, 2026
9 of 10 checks passed
@halvaradop halvaradop deleted the feat/add-entropy-verification branch February 5, 2026 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request security Security-related changes, vulnerability fixes, or hardening measures.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant