feat: enable CryptoKeyPair support across jose and core auth#157
Merged
halvaradop merged 7 commits intomasterfrom Apr 27, 2026
Merged
feat: enable CryptoKeyPair support across jose and core auth#157halvaradop merged 7 commits intomasterfrom
CryptoKeyPair support across jose and core auth#157halvaradop merged 7 commits intomasterfrom
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds asymmetric cryptography support using Web Crypto Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant JOSE as JOSE API
participant KeyGuard as Key Dispatcher
participant JWS as JWS Handler
participant JWE as JWE Handler
Client->>JOSE: createJWS(payload, CryptoKeyPair)
JOSE->>KeyGuard: isCryptoKeyPair(secret)
KeyGuard-->>JOSE: true
JOSE->>JOSE: select privateKey (sign) & publicKey (verify)
Client->>JWS: signJWS(payload, privateKey)
JWS-->>Client: signed token
Client->>JWS: verifyJWS(token, publicKey)
JWS-->>Client: verified payload
Client->>JOSE: createJWE(payload, CryptoKeyPair)
JOSE->>KeyGuard: isCryptoKeyPair(secret)
KeyGuard-->>JOSE: true
JOSE->>JOSE: select publicKey (encrypt) & privateKey (decrypt)
Client->>JWE: encryptJWE(payload, publicKey)
JWE-->>Client: encrypted token
Client->>JWE: decryptJWE(token, privateKey)
JWE-->>Client: decrypted payload
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/core/src/@types/session.ts (1)
20-27:⚠️ Potential issue | 🟡 MinorUpdate the doc to reference
CryptoKeyPairinstead of the removedKeyPair.The
KeyPairalias was removed, but the JSDoc on Line 25 still says "KeyPair: asymmetric signing (RS256, ES256, EdDSA, etc.)". Rename for consistency.📝 Proposed fix
/** * A symmetric secret or asymmetric key pair used for JWT operations. * * - string / Uint8Array: used as-is for HMAC (signed) or AES (encrypted) * - CryptoKey: Web Crypto API key, for environments that support it - * - KeyPair: asymmetric signing (RS256, ES256, EdDSA, etc.) + * - CryptoKeyPair: asymmetric signing/encryption (RS256, ES256, EdDSA, RSA-OAEP, etc.) */ export type SecretKey = string | Uint8Array | CryptoKey | CryptoKeyPair🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/`@types/session.ts around lines 20 - 27, The JSDoc for the SecretKey type still references the removed KeyPair alias; update the comment to mention CryptoKeyPair instead: change the "KeyPair: asymmetric signing (RS256, ES256, EdDSA, etc.)" entry in the JSDoc above the export type SecretKey to read "CryptoKeyPair: asymmetric signing (RS256, ES256, EdDSA, etc.)" so the documentation matches the actual union type.packages/jose/src/index.ts (1)
88-99:⚠️ Potential issue | 🟡 MinorUpdate stale JSDoc for
decodeJWTsecret parameter.The param description still enumerates
CryptoKey, KeyObject, string or Uint8Array, which is now outdated (noCryptoKeyPair, andKeyObjectwas never actually inSecretInput). Consider aligning with the newJWTSecretInput | DerivedKeyInputtype.📝 Suggested doc tweak
- * `@param` secret - Secret key used for both decrypting and verifying the JWT (CryptoKey, KeyObject, string or Uint8Array) + * `@param` secret - Secret used for decrypting and verifying the JWT. Accepts a `SecretInput` (string, Uint8Array, CryptoKey), a `CryptoKeyPair` for asymmetric keys, or a `DerivedKeyInput` with separate `sign`/`encrypt` entries.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/jose/src/index.ts` around lines 88 - 99, Update the JSDoc for the decodeJWT function to correctly describe the secret parameter type: replace the outdated enumeration ("CryptoKey, KeyObject, string or Uint8Array" / references to CryptoKeyPair/KeyObject) with the current union type (JWTSecretInput | DerivedKeyInput), and briefly note that the secret can be a raw key, passphrase-derived key input, or other shapes covered by those types; update the `@param` secret line in the decodeJWT comment to reference these exact type names (JWTSecretInput | DerivedKeyInput) so it matches the implementation.
🧹 Nitpick comments (5)
packages/core/test/instance.test.ts (2)
22-56: Matrix only validates construction, not runtime sign/verify with each algorithm.
testJWTAlgorithmsassertscreateAuth(...)returns a defined instance across all 11 signing algorithms, but never exercisesencodeJWT/decodeJWT. That means combinations which would fail at runtime (e.g.,RS256with aUint8Arraysecret, orHS256with aCryptoKeyPair) still pass this test. Consider extending one of the matrices to sign+verify a token so algorithm/secret mismatches are actually caught; otherwise this mostly tests thatcreateAuthis lazy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/test/instance.test.ts` around lines 22 - 56, The current testJWTAlgorithms only checks that createAuth(...) constructs an instance for each signing algorithm but doesn't exercise runtime signing/verification; update testJWTAlgorithms to actually sign and verify a JWT for each alg by calling authInstance.encodeJWT(...) and authInstance.decodeJWT(...). For HMAC algs (HS256/384/512) pass a Uint8Array secret via createSecretValue(32), for RSA/PS algs generate or stub an RSA CryptoKeyPair, for EC (ES*) and EdDSA generate appropriate EC/Ed25519 key pairs, then createAuth({ session: { jwt: { mode: "signed", signingAlgorithm: alg, ... } }, secret/keys }) and assert that a token produced by encodeJWT decodes successfully with decodeJWT and that decoding fails for mismatched key/secret; locate the logic around testJWTAlgorithms, createAuth, encodeJWT, and decodeJWT to add these sign+verify assertions.
131-131: Redundant describe-levelvi.stubEnvcalls are overridden bybeforeEach.The
vi.stubEnv("AURA_AUTH_SALT", createSecretValue(32))statements inside each describe body run at collection time, butbeforeEachthen stubs it back toundefinedbefore every test, and the test body (insidetestJWTAlgorithms) stubs it again prior tocreateAuth. The describe-level calls are effectively dead code — remove them to avoid misleading readers.Also applies to: 138-138, 152-152, 169-169
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/test/instance.test.ts` at line 131, Remove the redundant describe-level vi.stubEnv("AURA_AUTH_SALT", createSecretValue(32)) calls that run at collection time (they're overridden by the beforeEach that stubs it to undefined and subsequently by the testJWTAlgorithms test which re-stubs it before createAuth); locate and delete those vi.stubEnv lines (also at the other occurrences noted) and leave the beforeEach and test-level stubs intact so behavior is unchanged.packages/jose/src/assert.ts (1)
27-29: Type guard is shape-only — consider tightening if false positives matter.
isCryptoKeyPaironly checks that the value haspublicKeyandprivateKeykeys, so any{ publicKey: any, privateKey: any }object narrows toCryptoKeyPair. This is fine for the library's intent (downstreamjosecalls will reject non-CryptoKey values), but if a caller passes e.g.{ publicKey: "pem", privateKey: "pem" }thinking it's supported, they'll get a less descriptive downstream error. Consider also verifying property types (e.g.,instanceof CryptoKey) if stricter runtime typing is desired.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/jose/src/assert.ts` around lines 27 - 29, The current isCryptoKeyPair type guard only checks for shape (presence of publicKey/privateKey) which can yield false positives; update isCryptoKeyPair to also validate the property types at runtime (e.g., check that value.publicKey and value.privateKey are CryptoKey instances where available: use typeof CryptoKey !== "undefined" and instanceof CryptoKey, or if CryptoKey isn't defined fall back to verifying expected object shape/interface like presence of algorithm/type usages) so callers are narrowed to actual CryptoKey objects; modify the isCryptoKeyPair function to perform these additional checks and return false for plain POJOs with string PEMs while keeping the original signature.packages/core/src/@types/config.ts (1)
73-73: Optional: alignAuthConfig.secretandRouterGlobalContext.secreton the same type alias.
AuthConfig.secretis now typedSecretKey(Line 73) whileRouterGlobalContext.secreton Line 320 remains typedJWTKey. SinceJWTKey = SecretKey(seepackages/core/src/@types/session.ts), they're equivalent today, but using the same alias on both sides makes the intent clearer and removes a redundant indirection.Also applies to: 320-320
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/`@types/config.ts at line 73, AuthConfig.secret is using the SecretKey alias while RouterGlobalContext.secret uses JWTKey; make them consistent by changing RouterGlobalContext.secret to SecretKey (or alternatively change AuthConfig.secret to JWTKey) so both use the same type alias (SecretKey/JWTKey are currently identical), update the RouterGlobalContext.secret declaration to use SecretKey, and run type checks to ensure no other references break.packages/jose/src/secret.ts (1)
57-83: Crypto routing logic is correct; minor duplication opportunity.The key selection is semantically correct: JWS uses
privateKeyfor signing /publicKeyfor verification, and JWE is inverted (publicKeyfor encryption /privateKeyfor decryption). Both helpers are near-mirror images of each other, so you could optionally collapse them into a single parameterized helper if you want to reduce duplication.♻️ Optional dedup
-const getJWSSecrets = (secret: JWTSecretInput) => { - if (!isCryptoKeyPair(secret)) { - return { - encode: secret, - decode: secret, - } - } - - return { - encode: secret.privateKey, - decode: secret.publicKey, - } -} - -const getJWESecrets = (secret: JWTSecretInput) => { - if (!isCryptoKeyPair(secret)) { - return { - encode: secret, - decode: secret, - } - } - - return { - encode: secret.publicKey, - decode: secret.privateKey, - } -} +const pickKeys = (secret: JWTSecretInput, encodeKey: "privateKey" | "publicKey") => { + if (!isCryptoKeyPair(secret)) { + return { encode: secret, decode: secret } + } + const decodeKey = encodeKey === "privateKey" ? "publicKey" : "privateKey" + return { encode: secret[encodeKey], decode: secret[decodeKey] } +} + +const getJWSSecrets = (secret: JWTSecretInput) => pickKeys(secret, "privateKey") +const getJWESecrets = (secret: JWTSecretInput) => pickKeys(secret, "publicKey")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/jose/src/secret.ts` around lines 57 - 83, The two near-identical helpers getJWSSecrets and getJWESecrets can be collapsed into a single parameterized helper to remove duplication: create a function (e.g., getJWTSecrets(secret: JWTSecretInput, mode: 'JWS'|'JWE')) that uses isCryptoKeyPair(secret) and returns { encode, decode } with the mapping switched based on mode (for 'JWS' use privateKey for encode and publicKey for decode; for 'JWE' use publicKey for encode and privateKey for decode), then replace calls to getJWSSecrets and getJWESecrets with calls to the new helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/jose/src/encrypt.ts`:
- Around line 151-163: The createJWE (and createCompactJWE) factory currently
assigns encrypt/decrypt secrets reversed for asymmetric keys; when secret is a
CryptoKeyPair, set encryptSecret to secret.publicKey (used to encrypt/wrap) and
decryptSecret to secret.privateKey (used to decrypt/unwrap) instead of the
current private/public mapping, and update any tests that relied on the reversed
naming (e.g., swap decryptPrivateKey/encryptPublicKey test variables) so they
reflect publicKey for encryption and privateKey for decryption; change the
assignments in createJWE/createCompactJWE accordingly.
In `@packages/jose/src/secret.ts`:
- Around line 85-101: The current getSecrets function branches on presence of
"sign" / "encrypt" individually which can misclassify future secret shapes;
change the DerivedKeyInput detection in getSecrets to require both properties
(e.g., isObject(secret) && "sign" in secret && "encrypt" in secret) before
treating it as a DerivedKeyInput, and then derive jwsSource from secret.sign and
jweSource from secret.encrypt only when that stricter guard is true (keeping
getJWSSecrets/getJWESecrets usage unchanged); update any related type-narrowing
or comments around getSecrets/DerivedKeyInput/JWTSecretInput to reflect the
stricter check.
In `@packages/jose/test/index.test.ts`:
- Around line 280-334: Tests fail because encrypt.ts inverts keys for JWE:
createJWE/getJWESecrets are passing privateKey to the encrypt path and publicKey
to the decrypt path; update the logic so encryption uses the recipient's public
key and decryption uses the corresponding private key. Locate createJWE and
getJWESecrets in packages/jose/src/encrypt.ts and swap the key
assignment/arguments (use publicKey for encryption/creating JWE and privateKey
for decrypting/getJWESecrets) and ensure any helper names (e.g., encrypt,
decrypt, createJWE, getJWESecrets) are consistently using the corrected key
roles.
---
Outside diff comments:
In `@packages/core/src/`@types/session.ts:
- Around line 20-27: The JSDoc for the SecretKey type still references the
removed KeyPair alias; update the comment to mention CryptoKeyPair instead:
change the "KeyPair: asymmetric signing (RS256, ES256, EdDSA, etc.)" entry in
the JSDoc above the export type SecretKey to read "CryptoKeyPair: asymmetric
signing (RS256, ES256, EdDSA, etc.)" so the documentation matches the actual
union type.
In `@packages/jose/src/index.ts`:
- Around line 88-99: Update the JSDoc for the decodeJWT function to correctly
describe the secret parameter type: replace the outdated enumeration
("CryptoKey, KeyObject, string or Uint8Array" / references to
CryptoKeyPair/KeyObject) with the current union type (JWTSecretInput |
DerivedKeyInput), and briefly note that the secret can be a raw key,
passphrase-derived key input, or other shapes covered by those types; update the
`@param` secret line in the decodeJWT comment to reference these exact type names
(JWTSecretInput | DerivedKeyInput) so it matches the implementation.
---
Nitpick comments:
In `@packages/core/src/`@types/config.ts:
- Line 73: AuthConfig.secret is using the SecretKey alias while
RouterGlobalContext.secret uses JWTKey; make them consistent by changing
RouterGlobalContext.secret to SecretKey (or alternatively change
AuthConfig.secret to JWTKey) so both use the same type alias (SecretKey/JWTKey
are currently identical), update the RouterGlobalContext.secret declaration to
use SecretKey, and run type checks to ensure no other references break.
In `@packages/core/test/instance.test.ts`:
- Around line 22-56: The current testJWTAlgorithms only checks that
createAuth(...) constructs an instance for each signing algorithm but doesn't
exercise runtime signing/verification; update testJWTAlgorithms to actually sign
and verify a JWT for each alg by calling authInstance.encodeJWT(...) and
authInstance.decodeJWT(...). For HMAC algs (HS256/384/512) pass a Uint8Array
secret via createSecretValue(32), for RSA/PS algs generate or stub an RSA
CryptoKeyPair, for EC (ES*) and EdDSA generate appropriate EC/Ed25519 key pairs,
then createAuth({ session: { jwt: { mode: "signed", signingAlgorithm: alg, ... }
}, secret/keys }) and assert that a token produced by encodeJWT decodes
successfully with decodeJWT and that decoding fails for mismatched key/secret;
locate the logic around testJWTAlgorithms, createAuth, encodeJWT, and decodeJWT
to add these sign+verify assertions.
- Line 131: Remove the redundant describe-level vi.stubEnv("AURA_AUTH_SALT",
createSecretValue(32)) calls that run at collection time (they're overridden by
the beforeEach that stubs it to undefined and subsequently by the
testJWTAlgorithms test which re-stubs it before createAuth); locate and delete
those vi.stubEnv lines (also at the other occurrences noted) and leave the
beforeEach and test-level stubs intact so behavior is unchanged.
In `@packages/jose/src/assert.ts`:
- Around line 27-29: The current isCryptoKeyPair type guard only checks for
shape (presence of publicKey/privateKey) which can yield false positives; update
isCryptoKeyPair to also validate the property types at runtime (e.g., check that
value.publicKey and value.privateKey are CryptoKey instances where available:
use typeof CryptoKey !== "undefined" and instanceof CryptoKey, or if CryptoKey
isn't defined fall back to verifying expected object shape/interface like
presence of algorithm/type usages) so callers are narrowed to actual CryptoKey
objects; modify the isCryptoKeyPair function to perform these additional checks
and return false for plain POJOs with string PEMs while keeping the original
signature.
In `@packages/jose/src/secret.ts`:
- Around line 57-83: The two near-identical helpers getJWSSecrets and
getJWESecrets can be collapsed into a single parameterized helper to remove
duplication: create a function (e.g., getJWTSecrets(secret: JWTSecretInput,
mode: 'JWS'|'JWE')) that uses isCryptoKeyPair(secret) and returns { encode,
decode } with the mapping switched based on mode (for 'JWS' use privateKey for
encode and publicKey for decode; for 'JWE' use publicKey for encode and
privateKey for decode), then replace calls to getJWSSecrets and getJWESecrets
with calls to the new helper.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 08871330-eb05-4a60-84ae-7bd28bc559d1
📒 Files selected for processing (10)
packages/core/src/@types/config.tspackages/core/src/@types/session.tspackages/core/test/instance.test.tspackages/jose/src/assert.tspackages/jose/src/encrypt.tspackages/jose/src/index.tspackages/jose/src/secret.tspackages/jose/src/sign.tspackages/jose/test/index.test.tspackages/jose/test/types.test-d.ts
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
packages/jose/src/encrypt.ts (1)
151-179: LGTM — past review's swap is correctly fixed; one usability footgun to note.
encryptSecret = secret.publicKeyanddecryptSecret = secret.privateKeyis now correct for asymmetric JWE.One thing to be mindful of:
encryptJWE/compactEncryptJWEdefault toalg: "dir"andenc: "A256GCM"(Lines 45 and 73). When a caller passes aCryptoKeyPairfor an asymmetric algorithm (e.g.RSA-OAEP-256), they must remember to overridealgvia theoptionsargument, otherwise jose will reject the call. Consider documenting this oncreateJWE/createCompactJWEJSDoc, or detecting an asymmetric secret and preferring an asymmetric defaultalg(e.g."RSA-OAEP-256").🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/jose/src/encrypt.ts` around lines 151 - 179, Update the JSDoc for createJWE and createCompactJWE to warn callers that encryptJWE/compactEncryptJWE default to alg: "dir" and enc: "A256GCM" and that when passing a CryptoKeyPair for asymmetric algorithms (e.g., RSA) callers must override alg via the options argument; alternatively, detect an asymmetric CryptoKeyPair inside createJWE/createCompactJWE and set a more appropriate default alg (for example "RSA-OAEP-256") when building the encryptJWE/compactEncryptJWE calls so callers using publicKey/privateKey pairs won’t be rejected by jose if they forget to pass options.packages/core/src/jose.ts (1)
170-179: Errors from the async IIFE are silently swallowed.
jose.catch(() => {})at Line 179 prevents unhandled-rejection warnings, but it also hides genuine misconfiguration errors (invalid CryptoKey usages, KDF failures, etc.). The promise is then awaited again inside each public method (Lines 182–217), where the original error will eventually surface — but only on first call, not at instance creation.If the intent is just to suppress the unhandled-rejection warning, that's fine; otherwise consider failing fast (or at least logging) so consumers learn about misconfiguration immediately rather than on the first request.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/jose.ts` around lines 170 - 179, The async IIFE promise "jose" is currently swallowing errors via jose.catch(() => {}), which hides failures from getSecrets/createJWT/createJWS/createJWE; remove the empty catch and instead handle errors explicitly (either fail fast by awaiting the IIFE at module init or attach a catch that logs the error and rethrows) so misconfiguration surfaces immediately—update the code around the jose declaration and its catch to log (e.g., using your logger) and rethrow or await the promise so createJWT/createJWS/createJWE errors are not silently ignored.packages/core/test/jose.test.ts (1)
306-364:vi.stubEnvcalls inside these describe blocks (but outside any test/beforeEach) are effectively no-ops at test time.Lines 307, 316, 332, 351 run synchronously during describe registration. Then the global
beforeEachat Lines 15–24 wipes them before each test, and each generated test insidetestJWS/JWE/JWTAlgorithmsre-stubsAURA_AUTH_SALTitself (Lines 47, 90, 138). Net effect: the describe-level stubs do nothing visible to tests and just add confusing noise. Either drop them or move them into a scopedbeforeEachif they were intended to set per-suite defaults.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/test/jose.test.ts` around lines 306 - 364, The vi.stubEnv("AURA_AUTH_SALT", ...) calls placed directly inside the describe blocks ("crypto.getRandomValues", "crypto.generateKey", "crypto.importKey", "uint8array secret", "symmetric key") are executed at describe-registration time and are wiped by the global beforeEach, so remove those describe-level vi.stubEnv calls or move each into a per-suite beforeEach so the stub is applied for tests run inside testJWSAlgorithms, testJWEAlgorithms, and testJWTAlgorithms; update the describe blocks to either omit the stub entirely or add a beforeEach that calls vi.stubEnv("AURA_AUTH_SALT", createSecretValue(32)) so the stub is active during the actual tests.packages/core/src/shared/assert.ts (1)
113-119: Optional: preferinstanceofover duck-typing for built-in WebCrypto types.
CryptoKeyandCryptoKeyPairare well-defined WebCrypto types available globally in Node 19+, Deno, Bun, and Cloudflare Workers. While duck-typing on field names works for your trusted internal JWT flow, usinginstanceof CryptoKeywhere available provides stronger type safety and eliminates false positives from arbitrary objects. Consider:♻️ More robust guards
export const isCryptoKeyPair = (value: unknown): value is CryptoKeyPair => { - return typeof value === "object" && value !== null && "publicKey" in value && "privateKey" in value + return ( + typeof value === "object" && + value !== null && + "publicKey" in value && + "privateKey" in value && + isCryptoKey((value as CryptoKeyPair).publicKey) && + isCryptoKey((value as CryptoKeyPair).privateKey) + ) } export const isCryptoKey = (value: unknown): value is CryptoKey => { - return typeof value === "object" && value !== null && "algorithm" in value && "extractable" in value + return typeof CryptoKey !== "undefined" ? value instanceof CryptoKey : ( + typeof value === "object" && value !== null && "algorithm" in value && "extractable" in value && "type" in value && "usages" in value + ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/shared/assert.ts` around lines 113 - 119, The current type-guards isCryptoKeyPair and isCryptoKey rely solely on duck-typing and can yield false positives; update them to prefer using the native constructor when present: check typeof globalThis.CryptoKey !== "undefined" and use value instanceof CryptoKey for isCryptoKey (and for isCryptoKeyPair ensure publicKey/privateKey are instances of CryptoKey), falling back to the existing property checks if CryptoKey is not available. Keep the function names isCryptoKey and isCryptoKeyPair and preserve the original duck-typing as the fallback to maintain compatibility across runtimes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/jose.ts`:
- Around line 106-113: getSecrets currently assigns the same
CryptoKey/CryptoKeyPair to jwsSecret, jweSecret and jwtSecret which can misuse
keys and hides failures; update the isCryptoKeyPair/isCryptoKey branch to either
accept an explicit object with separate signing/encryption keys (e.g., { sign?:
CryptoKey|CryptoKeyPair, encrypt?: CryptoKey|CryptoKeyPair }) or validate the
provided key(s) before returning: check CryptoKey.usages (or pair roles) to
ensure sign/verify usages for JWS and encrypt/decrypt or wrapKey/unwrapKey for
JWE, and if validations fail throw a synchronous AuthInternalError with a clear
message (so failures surface immediately instead of being swallowed by the later
jose.catch(() => {})); reference getSecrets, createJWS and createJWE when
choosing how to split/assign jwsSecret vs jweSecret.
- Around line 115-128: The public signJWS/verifyJWS functions currently use
jwsSecret (set to derivedCsrfTokenKey) while encodeJWT uses jwtSecret.sign
(derivedSigningKey), causing cryptographic incompatibility; either align the
keys or make the CSRF intent explicit — the quickest, safest fix is to rename
signJWS/verifyJWS to signCsrfToken/verifyCsrfToken (update createJoseInstance to
expose those names instead of signJWS/verifyJWS), change jwsSecret -> csrfSecret
or keep jwsSecret but clarify in the JSDoc for createJoseInstance that these
functions are CSRF-specific and use derivedCsrfTokenKey, and update any
callsites/tests (e.g., shared/crypto.ts and session/stateless.ts) and
documentation to use the new names so consumers won’t mistakenly use them for
general-purpose JWS operations.
In `@packages/core/src/oauth/index.ts`:
- Line 23: The import/filename mismatch: rename the module file from dribble.ts
to dribbble.ts and update the import in packages/core/src/oauth/index.ts (import
{ dribbble } from "./dribbble.ts") so filename matches the exported symbol
dribbble; then ensure the builtInOAuthProviders entry that references dribbble
(and any usages of the string key or the type BuiltInOAuthProvider) is
consistent with the new identifier and update any environment variable naming
references (DRIBBLE_CLIENT_ID → DRIBBBLE_CLIENT_ID) or add a migration note to
preserve backwards compatibility.
In `@packages/core/test/jose.test.ts`:
- Around line 111-112: The describe label inside the test helper
testJWTAlgorithms is incorrect and duplicates the JWE block; update the describe
call in the testJWTAlgorithms function so it reads a distinct, accurate label
such as "JWT algorithms" or "Sealed (JWS + JWE)" instead of "JWE algorithms" to
avoid shadowing testJWEAlgorithms in reports; locate the describe in
testJWTAlgorithms and change the string label accordingly.
- Around line 359-364: The test block labeled describe("symmetric key", ...) is
misnamed and uses generateKeyPair("RS256") (an asymmetric RSA key) while calling
testJWSAlgorithms, testJWEAlgorithms, and testJWTAlgorithms which expect
symmetric keys/algorithms; rename the describe to reflect an RSA/asymmetric key
(e.g., "asymmetric RSA key") or change the key generation to a symmetric key
generator, and additionally filter or restrict the algorithm lists passed into
testJWSAlgorithms/testJWEAlgorithms/testJWTAlgorithms so only algorithms
compatible with the RSA key (or with the symmetric key if you switch generators)
are exercised to avoid invalid algorithm/key combinations; locate the block
using the identifiers describe("symmetric key", ...), generateKeyPair("RS256"),
testJWSAlgorithms, testJWEAlgorithms, and testJWTAlgorithms and apply the rename
or generator change plus algorithm filtering accordingly.
- Around line 30-158: The matrix tests (testJWSAlgorithms, testJWEAlgorithms,
testJWTAlgorithms) only assert createAuth(...).toBeDefined() so they never
exercise crypto; change each test to build a real round-trip using the jose
instance produced by createAuth/createJoseInstance: after calling
createAuth(...) await the internal jose initialization promise (do not swallow
errors with .catch(() => {})), obtain the signing/encryption CryptoKey/material
from getSecrets (or the created jose instance), sign and then verify (for JWS)
or encrypt then decrypt (for JWE/sealed) using the selected algorithms, and
assert the original payload is recovered; also add gating logic so only
compatible (secret, signingAlgorithm, encryptionAlgorithm, keyAlgorithm) pairs
run (e.g., HMAC algs require symmetric/raw key, RSA-OAEP requires RSA key pair,
dir requires symmetric key), and surface any initialization errors instead of
hiding them.
---
Nitpick comments:
In `@packages/core/src/jose.ts`:
- Around line 170-179: The async IIFE promise "jose" is currently swallowing
errors via jose.catch(() => {}), which hides failures from
getSecrets/createJWT/createJWS/createJWE; remove the empty catch and instead
handle errors explicitly (either fail fast by awaiting the IIFE at module init
or attach a catch that logs the error and rethrows) so misconfiguration surfaces
immediately—update the code around the jose declaration and its catch to log
(e.g., using your logger) and rethrow or await the promise so
createJWT/createJWS/createJWE errors are not silently ignored.
In `@packages/core/src/shared/assert.ts`:
- Around line 113-119: The current type-guards isCryptoKeyPair and isCryptoKey
rely solely on duck-typing and can yield false positives; update them to prefer
using the native constructor when present: check typeof globalThis.CryptoKey !==
"undefined" and use value instanceof CryptoKey for isCryptoKey (and for
isCryptoKeyPair ensure publicKey/privateKey are instances of CryptoKey), falling
back to the existing property checks if CryptoKey is not available. Keep the
function names isCryptoKey and isCryptoKeyPair and preserve the original
duck-typing as the fallback to maintain compatibility across runtimes.
In `@packages/core/test/jose.test.ts`:
- Around line 306-364: The vi.stubEnv("AURA_AUTH_SALT", ...) calls placed
directly inside the describe blocks ("crypto.getRandomValues",
"crypto.generateKey", "crypto.importKey", "uint8array secret", "symmetric key")
are executed at describe-registration time and are wiped by the global
beforeEach, so remove those describe-level vi.stubEnv calls or move each into a
per-suite beforeEach so the stub is applied for tests run inside
testJWSAlgorithms, testJWEAlgorithms, and testJWTAlgorithms; update the describe
blocks to either omit the stub entirely or add a beforeEach that calls
vi.stubEnv("AURA_AUTH_SALT", createSecretValue(32)) so the stub is active during
the actual tests.
In `@packages/jose/src/encrypt.ts`:
- Around line 151-179: Update the JSDoc for createJWE and createCompactJWE to
warn callers that encryptJWE/compactEncryptJWE default to alg: "dir" and enc:
"A256GCM" and that when passing a CryptoKeyPair for asymmetric algorithms (e.g.,
RSA) callers must override alg via the options argument; alternatively, detect
an asymmetric CryptoKeyPair inside createJWE/createCompactJWE and set a more
appropriate default alg (for example "RSA-OAEP-256") when building the
encryptJWE/compactEncryptJWE calls so callers using publicKey/privateKey pairs
won’t be rejected by jose if they forget to pass options.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6cde32d3-d125-4ae0-89ee-f3fd7c59f477
📒 Files selected for processing (12)
packages/core/CHANGELOG.mdpackages/core/src/@types/session.tspackages/core/src/jose.tspackages/core/src/oauth/index.tspackages/core/src/shared/assert.tspackages/core/test/jose.test.tspackages/jose/CHANGELOG.mdpackages/jose/src/deriveKey.tspackages/jose/src/encrypt.tspackages/jose/src/index.tspackages/jose/src/secret.tspackages/jose/test/index.test.ts
✅ Files skipped from review due to trivial changes (3)
- packages/jose/CHANGELOG.md
- packages/jose/src/deriveKey.ts
- packages/core/CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/core/src/@types/session.ts
- packages/jose/src/secret.ts
- packages/jose/test/index.test.ts
- packages/jose/src/index.ts
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/core/src/@types/session.ts (1)
34-37: Redundant union in the@todoannotation.Since
SecretKeyalready includesCryptoKeyPair, writingSecretKey | CryptoKeyPair | [SecretKey | CryptoKeyPair, ...]is equivalent toSecretKey | [SecretKey, ...SecretKey[]]. Minor doc cleanup.📝 Proposed fix
/** - * `@todo`: add key rotation support for "SecretKey | CryptoKeyPair | [SecretKey | CryptoKeyPair, ...(SecretKey | CryptoKeyPair)[]]" + * `@todo`: add key rotation support for "SecretKey | [SecretKey, ...SecretKey[]]" */ export type JWTKey = SecretKey🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/`@types/session.ts around lines 34 - 37, The JSDoc `@todo` for the JWTKey type contains a redundant union; update the comment to a concise form that reflects SecretKey already encompassing CryptoKeyPair by replacing `SecretKey | CryptoKeyPair | [SecretKey | CryptoKeyPair, ...(SecretKey | CryptoKeyPair)[]]` with a simplified description such as `SecretKey | [SecretKey, ...SecretKey[]]` or equivalent, and keep the exported type declaration export type JWTKey = SecretKey unchanged so the TODO accurately documents intended key-rotation support without redundant types.packages/core/src/shared/assert.ts (1)
113-130: Strengthen the runtime guards with instanceof checks and property validation.These guards rely solely on property existence, which means any plain object with
algorithm/extractableorpublicKey/privateKeywill pass—including misconfigured inputs that are not real Web Crypto objects. They then flow intogetSecretsand only fail much later, which is hard to debug.Two reasonable improvements:
- For
isCryptoKey, useinstanceof CryptoKeywhen available (Node 20+ and all modern browsers expose it), falling back to duck typing. Also validate additional properties liketypeandusagesto strengthen duck typing.- In
isCryptoKeyPair, validate thatpublicKeyandprivateKeyare themselvesCryptoKey-shaped. This preventsisCryptoSecretfrom accepting arbitrary{ sign, encrypt }objects as valid.Note:
CryptoKeyPairis not a global constructor (it's a dictionary), soinstanceof CryptoKeyPairis not available.♻️ Proposed refactor
export const isCryptoKey = (value: unknown): value is CryptoKey => { - return typeof value === "object" && value !== null && "algorithm" in value && "extractable" in value + if (typeof value !== "object" || value === null) return false + if (typeof CryptoKey !== "undefined" && value instanceof CryptoKey) return true + return "algorithm" in value && "extractable" in value && "type" in value && "usages" in value } export const isCryptoKeyPair = (value: unknown): value is CryptoKeyPair => { - return typeof value === "object" && value !== null && "publicKey" in value && "privateKey" in value + if (typeof value !== "object" || value === null) return false + return ( + "publicKey" in value && + "privateKey" in value && + isCryptoKey((value as { publicKey: unknown }).publicKey) && + isCryptoKey((value as { privateKey: unknown }).privateKey) + ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/shared/assert.ts` around lines 113 - 130, The runtime guards are too weak—update isCryptoKey to prefer runtime instanceof CryptoKey when available and otherwise perform stronger duck-typing by validating additional properties (algorithm, extractable, type, and usages) and their expected shapes; change isCryptoKeyPair so it not only checks for "publicKey" and "privateKey" but also verifies that both are crypto-key-shaped by calling isCryptoKey; and update isCryptoSecret to rely on the strengthened isCryptoKey/isCryptoKeyPair checks for the sign and encrypt fields (and remove plain property-only acceptance). Ensure all three functions (isCryptoKey, isCryptoKeyPair, isCryptoSecret) use these improved checks so misconfigured plain objects no longer pass.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/src/content/docs/api-reference/core.mdx`:
- Line 312: Update the docs to use the actual exported type name: replace the
incorrect type alias "Secret" with "SecretKey" (the union should remain "string
| Uint8Array | CryptoKey | CryptoKeyPair | CryptoSecret") so it matches the
exported symbol and the JWTKey alias referenced by AuthConfig.secret; also widen
the parameters table entry that currently lists "secret" as just "string" to the
same union type to keep documentation consistent.
In `@docs/src/content/docs/configuration/options.mdx`:
- Line 198: Rewrite the awkward sentence in the Aura Auth docs: instead of "But
requires additional setup to their correct generation and usage" change to a
clearer phrasing that uses the preposition "for" and does not start with "But"
(e.g., "This requires additional setup for their correct generation and
usage."); also add a brief mention that Aura Auth additionally accepts a
CryptoSecret-shaped object so readers see all three accepted key shapes
(CryptoKey, CryptoKeyPair, CryptoSecret) in the `jwt.mode` discussion to improve
clarity.
In `@packages/core/test/jose.test.ts`:
- Around line 159-179: The test "same cryptoKeyPair secret for JWS and JWE" is
vacuous and pairs incompatible alg+key; update it to perform a real async
round-trip: generate an RSA key pair (used in your snippet), call
createJoseInstance with a compatible JWS alg (e.g., "PS256") and a JWE alg that
matches RSA keys (e.g., "RSA-OAEP"), await the jose instance initialization
(don't rely on the internal .catch swallowing errors), then exercise both JWS
and JWE paths (sign and verify a JWT and encrypt and decrypt a payload) to
assert correctness instead of only expect(jose).toBeDefined(); ensure the key
usages and algorithm names match the crypto key (refer to createJoseInstance,
signingAlgorithm, and the RSA CryptoKeyPair you generate).
---
Nitpick comments:
In `@packages/core/src/`@types/session.ts:
- Around line 34-37: The JSDoc `@todo` for the JWTKey type contains a redundant
union; update the comment to a concise form that reflects SecretKey already
encompassing CryptoKeyPair by replacing `SecretKey | CryptoKeyPair | [SecretKey
| CryptoKeyPair, ...(SecretKey | CryptoKeyPair)[]]` with a simplified
description such as `SecretKey | [SecretKey, ...SecretKey[]]` or equivalent, and
keep the exported type declaration export type JWTKey = SecretKey unchanged so
the TODO accurately documents intended key-rotation support without redundant
types.
In `@packages/core/src/shared/assert.ts`:
- Around line 113-130: The runtime guards are too weak—update isCryptoKey to
prefer runtime instanceof CryptoKey when available and otherwise perform
stronger duck-typing by validating additional properties (algorithm,
extractable, type, and usages) and their expected shapes; change isCryptoKeyPair
so it not only checks for "publicKey" and "privateKey" but also verifies that
both are crypto-key-shaped by calling isCryptoKey; and update isCryptoSecret to
rely on the strengthened isCryptoKey/isCryptoKeyPair checks for the sign and
encrypt fields (and remove plain property-only acceptance). Ensure all three
functions (isCryptoKey, isCryptoKeyPair, isCryptoSecret) use these improved
checks so misconfigured plain objects no longer pass.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9937c6b5-1d40-4e6c-b95f-7cfbd0710bec
📒 Files selected for processing (11)
docs/src/content/docs/api-reference/core.mdxdocs/src/content/docs/configuration/options.mdxpackages/core/src/@types/config.tspackages/core/src/@types/session.tspackages/core/src/jose.tspackages/core/src/oauth/dribbble.tspackages/core/src/oauth/index.tspackages/core/src/shared/assert.tspackages/core/src/shared/crypto.tspackages/core/test/jose.test.tspackages/jose/test/index.test.ts
✅ Files skipped from review due to trivial changes (1)
- packages/core/src/@types/config.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/oauth/index.ts
This was referenced Apr 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This pull request adds support to CryptokeyPair across jose and core packages, the new feature allows to pass asymmetric Adds support for
CryptoKeyPairas a validsecretinput across bothcoreandjosepackages, enabling the use of asymmetric keys for signing and encryption operations.All relevant utilities now accept
CryptoKeyandCryptoKeyPair, allowing more flexible and standards-aligned cryptographic configurations.Changes
CryptoKeyPairsupport acrosscoreandjosepackagesstringUint8ArrayCryptoKeyCryptoKeyPairCryptoKeyorCryptoKeyPairis providedHKDF Derivation Behavior
Previously, when a
stringorUint8Arraysecret was provided, the core package applied HKDF-based key derivation to produce multiple cryptographic keys as a hardening measure.With the introduction of
CryptoKeyandCryptoKeyPair:stringandUint8ArrayinputsCryptoKeyandCryptoKeyPairThis is required because:
CryptoKey/CryptoKeyPairis opaqueUsage
String or Uint8Array
CryptoKey
CryptoKeyPair
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation