diff --git a/.claude/commands/address-pr-feedback.md b/.claude/commands/address-pr-feedback.md new file mode 100644 index 00000000..c12704d3 --- /dev/null +++ b/.claude/commands/address-pr-feedback.md @@ -0,0 +1,213 @@ +# /address-pr-feedback - Address PR Feedback + +Fetch and address all review bot feedback on the current PR. + +## Instructions + +### 1. Identify the PR + +```bash +gh pr view --json number,url,state --jq '{number, url, state}' +``` + +If no PR exists for the current branch, abort with a message. + +### 2. Check that review bots are done + +CodeRabbit reviews asynchronously. Before addressing feedback, verify it has finished. + +```bash +# Get all reviews on the PR +gh pr view --json reviews --jq '.reviews[] | {author: .author.login, state: .state}' + +# Check PR comments for bot activity (MUST use --paginate for large PRs) +gh api "repos/{owner}/{repo}/pulls/{pr}/comments" --paginate \ + --jq '[.[] | .user.login] | unique' +``` + +**CodeRabbit**: Look for a PR comment containing "Walkthrough" or a review with `coderabbitai[bot]` as author. If not present, inform the user: + +> "CodeRabbit hasn't reviewed this PR yet. Wait for its review or run `@coderabbitai review` as a PR comment, then re-run this command." + +**If the bot hasn't finished, stop here.** Do not proceed to fixing issues with incomplete feedback. + +### 3. Fetch ALL review comments + +**CRITICAL**: Always use `--paginate` with `gh api` for review comments. The default page size is 30, which is easily exceeded when CodeRabbit posts 16+ inline comments plus replies. Without `--paginate`, you will miss comments from later review passes. + +#### 3a. Inline review comments + +```bash +# Get ALL review comments โ€” MUST use --paginate +gh api "repos/{owner}/{repo}/pulls/{pr}/comments" --paginate --jq '.[] | { + id: .id, + author: .user.login, + path: .path, + line: .line, + body: .body, + in_reply_to_id: .in_reply_to_id, + created_at: .created_at +}' +``` + +To identify **new unaddressed root comments**, filter by: + +- `in_reply_to_id == null` (root comment, not a reply) +- `user.login == "coderabbitai[bot]"` +- No reply from the PR author (`gh pr view --json author --jq '.author.login'`) exists with matching `in_reply_to_id` + +Useful shortcut to see how many batches exist: + +```bash +gh api "repos/{owner}/{repo}/pulls/{pr}/comments" --paginate \ + --jq '.[] | select(.user.login == "coderabbitai[bot]") | select(.in_reply_to_id == null) | .created_at' \ + | sort | uniq -c | sort -rn +``` + +Each unique timestamp cluster represents one review pass. + +#### 3b. Outside-diff-range comments (in review body) + +CodeRabbit posts "outside diff range" comments in the **review body**, not as inline comments. These are easy to miss. + +```bash +# Get ALL CodeRabbit reviews with non-empty bodies (includes CHANGES_REQUESTED and COMMENTED states) +gh api "repos/{owner}/{repo}/pulls/{pr}/reviews" --paginate \ + --jq '.[] | select(.user.login == "coderabbitai[bot]") | select(.body | length > 0) | {id: .id, state: .state, submitted_at: .submitted_at}' +``` + +Then fetch each review body: + +```bash +gh api "repos/{owner}/{repo}/pulls/{pr}/reviews/{review_id}" --jq '.body' +``` + +Look for the `โš ๏ธ Outside diff range comments (N)` section in the body. Parse these โ€” they contain file paths, line numbers, and the same comment format as inline comments. + +Also look for these sections in review bodies: + +- **"๐Ÿงน Nitpick comments (N)"** โ€” valid code quality items +- **"โ™ป๏ธ Duplicate comments (N)"** โ€” re-raised from prior reviews +- **"๐Ÿค– Prompt for AI Agents"** โ€” structured fix instructions + +#### 3c. General PR comments + +```bash +gh api "repos/{owner}/{repo}/issues/{pr}/comments" --paginate \ + --jq '.[] | {id: .id, author: .user.login, body: .body, created_at: .created_at}' +``` + +### 4. Identify actionable feedback + +Collect ALL comments from: + +- Inline review comments (3a) +- Outside-diff-range / nitpick / duplicate comments from review bodies (3b) +- General PR comments (3c) + +Filter to unaddressed items from `coderabbitai[bot]`. + +**Before applying any fix**, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. + +For each comment, determine: + +1. **Valid concern** โ€” fix it +2. **False positive** โ€” reply explaining why +3. **Stale** โ€” code was already changed/removed since the comment was posted +4. **Ambiguous** โ€” ask the user which direction to take + +### 5. Present decisions for approval + +**STOP and present a table** before making any changes. The user must approve the plan first. + +| # | Source | File:Line | Comment Summary | Decision | Rationale | +| --- | --------------- | -------------------- | ---------------------------- | --------------------- | ----------------- | +| 1 | CR inline | `path/to/file.ts:42` | Brief summary of the comment | Fix / Dismiss / Stale | Why this decision | +| 2 | CR nitpick | `path/to/file.ts:10` | Brief summary | Fix / Dismiss / Stale | Why | +| 3 | CR outside-diff | `path/to/file.ts:78` | Brief summary | Fix / Dismiss / Stale | Why | + +Wait for the user to: + +- **Approve all** โ€” proceed with all decisions as proposed +- **Override specific rows** โ€” change the decision for individual items (e.g., "dismiss #3 instead of fixing") +- **Ask questions** โ€” clarify any items before approving + +**Do not proceed to step 6 until the user approves.** + +### 6. Apply fixes locally (do NOT reply to bots yet) + +**CRITICAL โ€” DO NOT reply to bot threads in this step.** Bot replies must happen _after_ push so the bot can verify against the actual remote. Replying with "Fixed" before the commit is on the remote causes the bot to re-flag the comment as unfixed (it reads the remote, not your working tree). + +For each item that needs a code change: + +1. Read the file and understand the context around the flagged line. +2. Apply the fix. + +Do not post any thread replies, PR comments, or "Fixed" messages yet. Just edit code. + +### 7. Run quality gates + +After all fixes are applied: + +```bash +bun tsc +``` + +All must pass before committing. + +### 8. Commit and push + +If any code changes were made: + +```bash +git add +git commit -m "fix: address PR review feedback from CodeRabbit" +git push +``` + +**Verify the push landed before moving on.** Confirm `git log origin/ -1` matches your local HEAD, or check `gh api repos/{owner}/{repo}/pulls/{pr} --jq .head.sha`. The bots will read this SHA when re-evaluating, so the reply in step 9 must reference code that is actually on it. + +### 9. Reply to bot threads + +Now that the fix is on the remote, post replies. Reference the new commit SHA in "Fixed" replies so the bot can verify and so the timeline is auditable later. + +**Reply rules:** + +1. **Inline review comments (3a)** โ€” reply on the conversation thread: + + ```bash + gh api "repos/{owner}/{repo}/pulls/{pr}/comments/{comment_id}/replies" \ + -X POST -f body="@coderabbitai Fixed in โ€” " + ``` + +2. **Review-body / outside-diff items (3b)** โ€” no inline thread exists. Post a top-level PR comment: + + ```bash + gh pr comment {pr} --body "@coderabbitai Addressed in : + - Fixed: + - Dismissed: " + ``` + +3. **General PR comments (3c)** โ€” issue comments don't support threaded replies. Post a new PR comment referencing the original: + ```bash + gh pr comment {pr} --body "@coderabbitai Re: comment {comment_id} โ€” Fixed in โ€” " + ``` + +**CodeRabbit replies MUST start with `@coderabbitai`** or the bot will not see them. + +For false positives / stale, use the same reply mechanism with an explanation instead of "Fixed": + +```bash +gh api "repos/{owner}/{repo}/pulls/{pr}/comments/{comment_id}/replies" \ + -X POST -f body="@coderabbitai " +``` + +### 10. Report summary + +Present a summary to the user: + +- **Fixed**: List of issues that were fixed with brief descriptions +- **Dismissed**: List of false positives with reasoning +- **Stale**: Comments on code that was already changed/removed +- **Needs input**: Any ambiguous items requiring user decision +- **Quality gates**: Pass/fail status diff --git a/.claude/commands/review.md b/.claude/commands/review.md index 778e999d..188e4521 100644 --- a/.claude/commands/review.md +++ b/.claude/commands/review.md @@ -5,6 +5,7 @@ Review all commits on the current branch since diverging from main. ## Prerequisites **IMPORTANT**: Before starting the review, check if this is a fresh context/session: + - If there is prior conversation history in this session (e.g., you helped write the code being reviewed), STOP immediately - Inform the user: "Code reviews should be done in a fresh context to avoid bias. Please start a new Claude Code session and run /review there." - A reviewer should not be the same "person" who wrote the code @@ -32,3 +33,17 @@ When activated (in a fresh session), perform a full code review of the commits s - Recommended actions (if any) Run `bun tsc` to verify the code compiles. + +## Follow-up + +After presenting the review, present a **fix plan table** for the user to approve before making any changes: + +| # | Severity | File:Line | Issue | Proposed Action | +| --- | -------- | ------------------ | ----------------- | ---------------- | +| 1 | Medium | path/to/file.ts:42 | Brief description | Fix / Skip / Ask | + +- **Fix**: Will apply the change +- **Skip**: Not worth changing (explain why) +- **Ask**: Ambiguous, needs user input on approach + +**Wait for the user to approve the plan** (they may want to skip or modify items). Then apply only the approved fixes. Run `bun tsc` after all fixes are applied to verify everything is clean. diff --git a/example/src/tests/subtle/encrypt_decrypt.ts b/example/src/tests/subtle/encrypt_decrypt.ts index 3c8ff08c..b4f908de 100644 --- a/example/src/tests/subtle/encrypt_decrypt.ts +++ b/example/src/tests/subtle/encrypt_decrypt.ts @@ -603,7 +603,7 @@ test(SUITE, 'ChaCha20-Poly1305 RF C 8439 vector', async () => { const expectedTagHex = '1ae10b594f09e26a7e902ecbd0600691'; const key = await subtle.importKey( - 'raw', + 'raw-secret', Buffer.from(keyHex, 'hex'), { name: 'ChaCha20-Poly1305' } as any, // eslint-disable-line @typescript-eslint/no-explicit-any true, @@ -763,18 +763,19 @@ test(SUITE, 'ChaCha20-Poly1305 large plaintext', async () => { ); }); -test(SUITE, 'ChaCha20-Poly1305 key import/export raw', async () => { +test(SUITE, 'ChaCha20-Poly1305 key import/export raw-secret', async () => { const keyMaterial = getRandomValues(new Uint8Array(32)); // 256 bits + // ChaCha20-Poly1305 accepts only 'raw-secret' (Node chacha20_poly1305.js:104-134) const key = await subtle.importKey( - 'raw', + 'raw-secret', keyMaterial, { name: 'ChaCha20-Poly1305' } as any, // eslint-disable-line @typescript-eslint/no-explicit-any true, ['encrypt', 'decrypt'], ); - const exported = await subtle.exportKey('raw', key as CryptoKey); + const exported = await subtle.exportKey('raw-secret', key as CryptoKey); expect(Buffer.from(exported as ArrayBuffer).toString('hex')).to.equal( Buffer.from(keyMaterial as Uint8Array).toString('hex'), @@ -1367,19 +1368,20 @@ test(SUITE, 'AES-OCB empty plaintext', async () => { expect(decrypted.byteLength).to.equal(0); }); -// Test AES-OCB key import/export -test(SUITE, 'AES-OCB key import/export raw', async () => { +// Test AES-OCB key import/export โ€” Node accepts only 'raw-secret' for AES-OCB +// (aes.js:243-249, webcrypto.js:550-560). +test(SUITE, 'AES-OCB key import/export raw-secret', async () => { const keyMaterial = getRandomValues(new Uint8Array(32)); const key = await subtle.importKey( - 'raw', + 'raw-secret', keyMaterial, { name: 'AES-OCB' }, true, ['encrypt', 'decrypt'], ); - const exported = await subtle.exportKey('raw', key as CryptoKey); + const exported = await subtle.exportKey('raw-secret', key as CryptoKey); expect(Buffer.from(exported as ArrayBuffer).toString('hex')).to.equal( Buffer.from(keyMaterial as Uint8Array).toString('hex'), diff --git a/example/src/tests/subtle/import_export.ts b/example/src/tests/subtle/import_export.ts index 12c8fc4f..fff5edf2 100644 --- a/example/src/tests/subtle/import_export.ts +++ b/example/src/tests/subtle/import_export.ts @@ -2733,11 +2733,11 @@ for (const variant of MLKEM_VARIANTS) { ); } -test(SUITE, 'ML-KEM-768 importKey raw rejects bad usages', async () => { +test(SUITE, 'ML-KEM-768 importKey raw-public rejects bad usages', async () => { await assertThrowsAsync( async () => await subtle.importKey( - 'raw', + 'raw-public', new Uint8Array(1184), { name: 'ML-KEM-768' }, true, @@ -3093,8 +3093,12 @@ test(SUITE, 'AES-OCB export/import jwk', async () => { ['encrypt', 'decrypt'], ); - const exportedRaw1 = await subtle.exportKey('raw', key as CryptoKey); - const exportedRaw2 = await subtle.exportKey('raw', imported as CryptoKey); + // AES-OCB does not accept plain 'raw' โ€” use 'raw-secret' (Node aes.js:243-249) + const exportedRaw1 = await subtle.exportKey('raw-secret', key as CryptoKey); + const exportedRaw2 = await subtle.exportKey( + 'raw-secret', + imported as CryptoKey, + ); expect(Buffer.from(exportedRaw1 as ArrayBuffer).toString('hex')).to.equal( Buffer.from(exportedRaw2 as ArrayBuffer).toString('hex'), ); @@ -3497,19 +3501,20 @@ test(SUITE, 'ML-KEM-768 unwrapKey with AES-GCM', async () => { // --- KMAC Import/Export Tests --- for (const algorithm of ['KMAC128', 'KMAC256'] as const) { - test(SUITE, `KMAC import/export raw (${algorithm})`, async () => { + // KMAC accepts only 'raw-secret' (Node mac.js:141-145, webcrypto.js:552-560) + test(SUITE, `KMAC import/export raw-secret (${algorithm})`, async () => { const keyBytes = new Uint8Array(32); globalThis.crypto.getRandomValues(keyBytes); const key = await subtle.importKey( - 'raw', + 'raw-secret', keyBytes, { name: algorithm }, true, ['sign', 'verify'], ); - const exported = await subtle.exportKey('raw', key); + const exported = await subtle.exportKey('raw-secret', key); expect(ab2str(exported as ArrayBuffer, 'hex')).to.equal( ab2str(keyBytes.buffer as ArrayBuffer, 'hex'), ); @@ -3573,3 +3578,299 @@ test( expect(err.message).to.contain('not extractable'); }, ); + +// --- Format aliasing per-algorithm tests (gh#1002) --- +// +// Mirrors Node's per-algorithm format matrix +// (lib/internal/crypto/webcrypto.js:472-563, 734-742, 813-822; +// lib/internal/crypto/aes.js:243-249; +// lib/internal/crypto/chacha20_poly1305.js:104-134; +// lib/internal/crypto/mac.js:141-145). + +test(SUITE, 'AES-OCB importKey rejects plain raw format', async () => { + await assertThrowsAsync( + async () => + await subtle.importKey( + 'raw', + getRandomValues(new Uint8Array(32)), + { name: 'AES-OCB' }, + true, + ['encrypt', 'decrypt'], + ), + 'Unable to import AES-OCB key with format raw', + ); +}); + +test(SUITE, 'AES-OCB exportKey rejects plain raw format', async () => { + const key = (await subtle.generateKey( + { name: 'AES-OCB', length: 256 }, + true, + ['encrypt', 'decrypt'], + )) as CryptoKey; + await assertThrowsAsync( + async () => await subtle.exportKey('raw', key), + 'Unable to export AES-OCB secret key using raw format', + ); +}); + +test( + SUITE, + 'ChaCha20-Poly1305 importKey rejects plain raw format', + async () => { + await assertThrowsAsync( + async () => + await subtle.importKey( + 'raw', + getRandomValues(new Uint8Array(32)), + // eslint-disable-next-line @typescript-eslint/no-explicit-any + { name: 'ChaCha20-Poly1305' } as any, + true, + ['encrypt', 'decrypt'], + ), + 'Unable to import ChaCha20-Poly1305 key with format raw', + ); + }, +); + +test( + SUITE, + 'ChaCha20-Poly1305 exportKey rejects plain raw format', + async () => { + const key = (await subtle.generateKey( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + { name: 'ChaCha20-Poly1305', length: 256 } as any, + true, + ['encrypt', 'decrypt'], + )) as CryptoKey; + await assertThrowsAsync( + async () => await subtle.exportKey('raw', key), + 'Unable to export ChaCha20-Poly1305 secret key using raw format', + ); + }, +); + +for (const algorithm of ['KMAC128', 'KMAC256'] as const) { + test(SUITE, `${algorithm} importKey rejects plain raw format`, async () => { + await assertThrowsAsync( + async () => + await subtle.importKey( + 'raw', + getRandomValues(new Uint8Array(32)), + { name: algorithm }, + true, + ['sign', 'verify'], + ), + `Unable to import ${algorithm} key with format raw`, + ); + }); + + test(SUITE, `${algorithm} exportKey rejects plain raw format`, async () => { + const key = (await subtle.generateKey({ name: algorithm }, true, [ + 'sign', + 'verify', + ])) as CryptoKey; + await assertThrowsAsync( + async () => await subtle.exportKey('raw', key), + `Unable to export ${algorithm} secret key using raw format`, + ); + }); +} + +for (const algorithm of ['Argon2d', 'Argon2i', 'Argon2id'] as const) { + test(SUITE, `${algorithm} importKey rejects plain raw format`, async () => { + await assertThrowsAsync( + async () => + await subtle.importKey( + 'raw', + getRandomValues(new Uint8Array(32)), + { name: algorithm }, + false, + ['deriveBits'], + ), + `Unable to import ${algorithm} key with format raw`, + ); + }); +} + +// AES-CTR/CBC/GCM/KW + HMAC accept BOTH 'raw' and 'raw-secret' (Node aliases). +test(SUITE, 'AES-GCM accepts raw-secret import/export', async () => { + const keyData = getRandomValues(new Uint8Array(32)); + const key = await subtle.importKey( + 'raw-secret', + keyData, + { name: 'AES-GCM' }, + true, + ['encrypt', 'decrypt'], + ); + const exported = (await subtle.exportKey('raw-secret', key)) as ArrayBuffer; + expect(Buffer.from(exported).toString('hex')).to.equal( + Buffer.from(keyData as Uint8Array).toString('hex'), + ); + const exportedRaw = (await subtle.exportKey('raw', key)) as ArrayBuffer; + expect(Buffer.from(exportedRaw).toString('hex')).to.equal( + Buffer.from(keyData as Uint8Array).toString('hex'), + ); +}); + +// ML-DSA / ML-KEM reject plain 'raw' โ€” only 'raw-public' is accepted for +// public-key import/export (Node webcrypto.js:493-499, 506-511). +for (const variant of [ + 'ML-DSA-44', + 'ML-DSA-65', + 'ML-DSA-87', + 'ML-KEM-512', + 'ML-KEM-768', + 'ML-KEM-1024', +] as const) { + test(SUITE, `${variant} exportKey rejects plain raw format`, async () => { + const isMlKem = variant.startsWith('ML-KEM'); + const usages: KeyUsage[] = isMlKem + ? ['encapsulateBits', 'decapsulateBits'] + : ['sign', 'verify']; + const keyPair = (await subtle.generateKey( + { name: variant }, + true, + usages, + )) as CryptoKeyPair; + await assertThrowsAsync( + async () => await subtle.exportKey('raw', keyPair.publicKey as CryptoKey), + `Unable to export ${variant} public key using raw format`, + ); + }); + + test(SUITE, `${variant} importKey rejects plain raw format`, async () => { + const isMlKem = variant.startsWith('ML-KEM'); + const usages: KeyUsage[] = isMlKem ? ['encapsulateBits'] : ['verify']; + const keyPair = (await subtle.generateKey( + { name: variant }, + true, + isMlKem ? ['encapsulateBits', 'decapsulateBits'] : ['sign', 'verify'], + )) as CryptoKeyPair; + const exported = (await subtle.exportKey( + 'raw-public', + keyPair.publicKey as CryptoKey, + )) as ArrayBuffer; + await assertThrowsAsync( + async () => + await subtle.importKey( + 'raw', + exported, + { name: variant }, + true, + usages, + ), + `Unsupported format for ${variant} import: raw`, + ); + }); +} + +// HMAC accepts BOTH 'raw' and 'raw-secret' (Node mac.js:141-145). +test(SUITE, 'HMAC accepts raw-secret import/export', async () => { + const keyData = getRandomValues(new Uint8Array(32)); + const key = await subtle.importKey( + 'raw-secret', + keyData, + { name: 'HMAC', hash: 'SHA-256' }, + true, + ['sign', 'verify'], + ); + const exportedSecret = (await subtle.exportKey( + 'raw-secret', + key, + )) as ArrayBuffer; + expect(Buffer.from(exportedSecret).toString('hex')).to.equal( + Buffer.from(keyData as Uint8Array).toString('hex'), + ); + const exportedRaw = (await subtle.exportKey('raw', key)) as ArrayBuffer; + expect(Buffer.from(exportedRaw).toString('hex')).to.equal( + Buffer.from(keyData as Uint8Array).toString('hex'), + ); +}); + +// HMAC rejects 'raw-public' (Node mac.js:172-173 returns undefined โ†’ parent +// throws NotSupportedError). Without this gate, raw-public would silently be +// accepted via aliasing. +test(SUITE, 'HMAC importKey rejects raw-public format', async () => { + await assertThrowsAsync( + async () => + await subtle.importKey( + 'raw-public', + getRandomValues(new Uint8Array(32)), + { name: 'HMAC', hash: 'SHA-256' }, + true, + ['sign', 'verify'], + ), + 'Unable to import HMAC key with format raw-public', + ); +}); + +// Ed25519 / Ed448 / X25519 / X448 public keys accept both 'raw' and +// 'raw-public' (Node webcrypto.js:763-773 aliases for CFRG). +for (const variant of ['Ed25519', 'Ed448', 'X25519', 'X448'] as const) { + test(SUITE, `${variant} accepts raw-public import/export`, async () => { + const isX = variant === 'X25519' || variant === 'X448'; + const generateUsages: KeyUsage[] = isX + ? ['deriveBits'] + : ['sign', 'verify']; + const keyPair = (await subtle.generateKey( + { name: variant }, + true, + generateUsages, + )) as CryptoKeyPair; + const exportedPublic = (await subtle.exportKey( + 'raw-public', + keyPair.publicKey as CryptoKey, + )) as ArrayBuffer; + const exportedRaw = (await subtle.exportKey( + 'raw', + keyPair.publicKey as CryptoKey, + )) as ArrayBuffer; + expect(Buffer.from(exportedPublic).toString('hex')).to.equal( + Buffer.from(exportedRaw).toString('hex'), + ); + const reimported = await subtle.importKey( + 'raw-public', + exportedPublic, + { name: variant }, + true, + isX ? [] : ['verify'], + ); + expect((reimported as CryptoKey).type).to.equal('public'); + expect((reimported as CryptoKey).algorithm.name).to.equal(variant); + }); +} + +// ECDSA / ECDH public keys accept both 'raw' and 'raw-public' (Node +// webcrypto.js:756-762 aliases for EC). +for (const variant of ['ECDSA', 'ECDH'] as const) { + test(SUITE, `${variant} accepts raw-public import/export`, async () => { + const usages: KeyUsage[] = + variant === 'ECDSA' ? ['sign', 'verify'] : ['deriveBits']; + const importUsages: KeyUsage[] = variant === 'ECDSA' ? ['verify'] : []; + const keyPair = (await subtle.generateKey( + { name: variant, namedCurve: 'P-256' }, + true, + usages, + )) as CryptoKeyPair; + const exportedPublic = (await subtle.exportKey( + 'raw-public', + keyPair.publicKey as CryptoKey, + )) as ArrayBuffer; + const exportedRaw = (await subtle.exportKey( + 'raw', + keyPair.publicKey as CryptoKey, + )) as ArrayBuffer; + expect(Buffer.from(exportedPublic).toString('hex')).to.equal( + Buffer.from(exportedRaw).toString('hex'), + ); + const reimported = await subtle.importKey( + 'raw-public', + exportedPublic, + { name: variant, namedCurve: 'P-256' }, + true, + importUsages, + ); + expect((reimported as CryptoKey).type).to.equal('public'); + expect((reimported as CryptoKey).algorithm.name).to.equal(variant); + }); +} diff --git a/example/src/tests/subtle/sign_verify.ts b/example/src/tests/subtle/sign_verify.ts index 71313be5..cb8f32a6 100644 --- a/example/src/tests/subtle/sign_verify.ts +++ b/example/src/tests/subtle/sign_verify.ts @@ -1199,8 +1199,9 @@ const kmacNistVectors = [ for (const vec of kmacNistVectors) { test(SUITE, `NIST KMAC sign: ${vec.name}`, async () => { + // KMAC accepts only 'raw-secret' (Node mac.js:141-145). const key = await subtle.importKey( - 'raw', + 'raw-secret', vec.key, { name: vec.algorithm }, false, @@ -1224,7 +1225,7 @@ for (const vec of kmacNistVectors) { for (const vec of kmacNistVectors) { test(SUITE, `NIST KMAC verify: ${vec.name}`, async () => { const key = await subtle.importKey( - 'raw', + 'raw-secret', vec.key, { name: vec.algorithm }, false, diff --git a/packages/react-native-quick-crypto/src/subtle.ts b/packages/react-native-quick-crypto/src/subtle.ts index 2a4c08b7..cda31664 100644 --- a/packages/react-native-quick-crypto/src/subtle.ts +++ b/packages/react-native-quick-crypto/src/subtle.ts @@ -146,6 +146,16 @@ function getAlgorithmName(name: string, length: number): string { } } +// Mirrors Node's aliasKeyFormat (lib/internal/crypto/webcrypto.js): for +// algorithms whose import/export accepts both 'raw' and the disambiguated +// 'raw-secret' / 'raw-public', collapse the latter to 'raw'. Used per-algorithm +// โ€” algorithms that demand the disambiguated form (KMAC, AES-OCB, +// ChaCha20-Poly1305, Argon2*, ML-DSA, ML-KEM) MUST NOT alias. +function aliasKeyFormat(format: ImportFormat): ImportFormat { + if (format === 'raw-secret' || format === 'raw-public') return 'raw'; + return format; +} + // Placeholder implementations for missing functions function ecExportKey(key: CryptoKey, format: KWebCryptoKeyFormat): ArrayBuffer { const keyObject = key.keyObject; @@ -878,7 +888,9 @@ async function kmacImportKey( } keyObject = new SecretKeyObject(handle); - } else if (format === 'raw' || format === 'raw-secret') { + } else if (format === 'raw-secret') { + // KMAC accepts only the disambiguated 'raw-secret' form (Node mac.js:141-145 + // returns undefined for plain 'raw' when not HMAC). if (hasAnyNotIn(keyUsages, ['sign', 'verify'])) { throw lazyDOMException( `Unsupported key usage for ${name} key`, @@ -1114,11 +1126,15 @@ async function hmacImportKey( } keyObject = new SecretKeyObject(handle); - } else if (format === 'raw') { + } else if (format === 'raw' || format === 'raw-secret') { + // HMAC accepts both 'raw' and 'raw-secret' (Node mac.js:141-145). checkUsages(); keyObject = createSecretKey(data as BinaryLike); } else { - throw new Error(`Unable to import HMAC key with format ${format}`); + throw lazyDOMException( + `Unable to import HMAC key with format ${format}`, + 'NotSupportedError', + ); } // Normalize hash to { name: string } format per WebCrypto spec @@ -1153,6 +1169,13 @@ async function aesImportKey( } }; + // AES-OCB and ChaCha20-Poly1305 require the disambiguated 'raw-secret' form + // and reject 'raw' (Node aes.js:243-249, chacha20_poly1305.js:104-134). + // Other AES variants accept both 'raw' and 'raw-secret'. + const requiresRawSecret = name === 'AES-OCB' || name === 'ChaCha20-Poly1305'; + const acceptsRaw = + format === 'raw-secret' || (format === 'raw' && !requiresRawSecret); + let keyObject: KeyObject; let actualLength: number; @@ -1184,18 +1207,28 @@ async function aesImportKey( const exported = keyObject.export(); actualLength = exported.byteLength * 8; - } else if (format === 'raw') { + } else if (acceptsRaw) { checkUsages(); const keyData = bufferLikeToArrayBuffer(data as BufferLike); actualLength = keyData.byteLength * 8; - if (![128, 192, 256].includes(actualLength)) { + if (name === 'ChaCha20-Poly1305') { + if (actualLength !== 256) { + throw lazyDOMException( + 'Invalid ChaCha20-Poly1305 key length', + 'DataError', + ); + } + } else if (![128, 192, 256].includes(actualLength)) { throw new Error('Invalid AES key length'); } keyObject = createSecretKey(keyData); } else { - throw new Error(`Unsupported format for AES import: ${format}`); + throw lazyDOMException( + `Unable to import ${name} key with format ${format}`, + 'NotSupportedError', + ); } // Validate length if specified @@ -1380,7 +1413,9 @@ function pqcImportKeyObject( ), isPublic: false, }; - } else if (format === 'raw') { + } else if (format === 'raw-public') { + // ML-DSA / ML-KEM reject plain 'raw' โ€” only 'raw-public' is accepted for + // public-key import (Node webcrypto.js:493-499, 506-511). const handle = NitroModules.createHybridObject('KeyObjectHandle'); if ( @@ -1452,7 +1487,7 @@ function pqcIsPublicImport( (data as JWK).priv === undefined ); } - return format === 'spki' || format === 'raw'; + return format === 'spki' || format === 'raw-public'; } function validatePqcJwk( @@ -1478,6 +1513,23 @@ function validatePqcJwk( } } +// Validates that `format` is one of the formats PQC algorithms accept; rejects +// plain 'raw' early so the format error wins over usage-based errors. +function validatePqcFormat(format: ImportFormat, name: string): void { + if ( + format !== 'spki' && + format !== 'pkcs8' && + format !== 'raw-public' && + format !== 'raw-seed' && + format !== 'jwk' + ) { + throw lazyDOMException( + `Unsupported format for ${name} import: ${format}`, + 'NotSupportedError', + ); + } +} + function mldsaImportKey( format: ImportFormat, data: BufferLike | JWK, @@ -1486,6 +1538,7 @@ function mldsaImportKey( keyUsages: KeyUsage[], ): CryptoKey { const { name } = algorithm; + validatePqcFormat(format, name); if (format === 'jwk') { validatePqcJwk(data, name, extractable, keyUsages, 'sig'); } @@ -1508,6 +1561,7 @@ function mlkemImportKey( keyUsages: KeyUsage[], ): CryptoKey { const { name } = algorithm; + validatePqcFormat(format, name); if (format === 'jwk') { validatePqcJwk(data, name, extractable, keyUsages, 'enc'); } @@ -1657,73 +1711,83 @@ const exportKeyPkcs8 = async ( ); }; -const exportKeyRaw = (key: CryptoKey): ArrayBuffer | unknown => { - switch (key.algorithm.name) { +// Mirrors Node's export key matrix (lib/internal/crypto/webcrypto.js +// exportKeyRawSecret / exportKeyRawPublic, lines 472-563): +// +// raw โ€” AES-CTR/CBC/GCM/KW + HMAC (secret); ECDSA/ECDH/Ed/X (public) +// raw-secret โ€” AES-CTR/CBC/GCM/KW + HMAC + AES-OCB + KMAC + ChaCha20-Poly1305 +// raw-public โ€” ECDSA/ECDH + Ed/X + ML-DSA + ML-KEM (public) +const exportKeyRaw = ( + key: CryptoKey, + format: 'raw' | 'raw-secret' | 'raw-public', +): ArrayBuffer => { + const name = key.algorithm.name; + const isPublic = key.type === 'public'; + const isSecret = key.type === 'secret'; + + const exportSecret = (): ArrayBuffer => { + const exported = key.keyObject.export(); + return exported.buffer.slice( + exported.byteOffset, + exported.byteOffset + exported.byteLength, + ) as ArrayBuffer; + }; + const exportRawPublic = (): ArrayBuffer => + bufferLikeToArrayBuffer(key.keyObject.handle.exportKey()); + + const fail = (): never => { + throw lazyDOMException( + `Unable to export ${name} ${key.type} key using ${format} format`, + 'NotSupportedError', + ); + }; + + // Symmetric: AES-CTR/CBC/GCM/KW and HMAC accept both 'raw' and 'raw-secret'; + // AES-OCB / KMAC* / ChaCha20-Poly1305 only 'raw-secret'. + switch (name) { + case 'AES-CTR': + case 'AES-CBC': + case 'AES-GCM': + case 'AES-KW': + case 'HMAC': + if (!isSecret) return fail(); + if (format === 'raw' || format === 'raw-secret') return exportSecret(); + return fail(); + case 'AES-OCB': + case 'KMAC128': + case 'KMAC256': + case 'ChaCha20-Poly1305': + if (!isSecret) return fail(); + if (format === 'raw-secret') return exportSecret(); + return fail(); case 'ECDSA': - // Fall through case 'ECDH': - if (key.type === 'public') { + if (!isPublic) return fail(); + if (format === 'raw' || format === 'raw-public') { return ecExportKey(key, KWebCryptoKeyFormat.kWebCryptoKeyFormatRaw); } - break; + return fail(); case 'Ed25519': - // Fall through case 'Ed448': - // Fall through case 'X25519': - // Fall through case 'X448': - if (key.type === 'public') { - const exported = key.keyObject.handle.exportKey(); - return bufferLikeToArrayBuffer(exported); - } - break; - case 'ML-KEM-512': - // Fall through - case 'ML-KEM-768': - // Fall through - case 'ML-KEM-1024': - // Fall through + if (!isPublic) return fail(); + if (format === 'raw' || format === 'raw-public') return exportRawPublic(); + return fail(); case 'ML-DSA-44': - // Fall through case 'ML-DSA-65': - // Fall through case 'ML-DSA-87': - if (key.type === 'public') { - const exported = key.keyObject.handle.exportKey(); - return bufferLikeToArrayBuffer(exported); - } - break; - case 'AES-CTR': - // Fall through - case 'AES-CBC': - // Fall through - case 'AES-GCM': - // Fall through - case 'AES-KW': - // Fall through - case 'AES-OCB': - // Fall through - case 'ChaCha20-Poly1305': - // Fall through - case 'HMAC': - // Fall through - case 'KMAC128': - // Fall through - case 'KMAC256': { - const exported = key.keyObject.export(); - // Convert Buffer to ArrayBuffer - return exported.buffer.slice( - exported.byteOffset, - exported.byteOffset + exported.byteLength, - ); - } + case 'ML-KEM-512': + case 'ML-KEM-768': + case 'ML-KEM-1024': + // ML-DSA / ML-KEM keys do not recognize plain 'raw' (Node webcrypto.js + // lines 488-510). + if (!isPublic) return fail(); + if (format === 'raw-public') return exportRawPublic(); + return fail(); } - throw lazyDOMException( - `Unable to export a raw ${key.algorithm.name} ${key.type} key`, - 'InvalidAccessError', - ); + return fail(); }; const exportKeyJWK = (key: CryptoKey): ArrayBuffer | unknown => { @@ -1807,7 +1871,11 @@ const exportKeyJWK = (key: CryptoKey): ArrayBuffer | unknown => { ); }; -const importGenericSecretKey = async ( +// PBKDF2 import. Mirrors Node's importGenericSecretKey ordering +// (keys.js:945-971): extractable โ†’ usage โ†’ format โ†’ length. Callers pre-alias +// 'raw-secret' / 'raw-public' to 'raw' via aliasKeyFormat +// (webcrypto.js:798-808). +const pbkdf2ImportKey = async ( { name, length }: SubtleAlgorithm, format: ImportFormat, keyData: BufferLike | BinaryLike, @@ -1815,33 +1883,70 @@ const importGenericSecretKey = async ( keyUsages: KeyUsage[], ): Promise => { if (extractable) { - throw new Error(`${name} keys are not extractable`); + throw lazyDOMException(`${name} keys are not extractable`, 'SyntaxError'); } if (hasAnyNotIn(keyUsages, ['deriveKey', 'deriveBits'])) { - throw new Error(`Unsupported key usage for a ${name} key`); + throw lazyDOMException( + `Unsupported key usage for a ${name} key`, + 'SyntaxError', + ); + } + if (format !== 'raw') { + throw lazyDOMException( + `Unable to import ${name} key with format ${format}`, + 'NotSupportedError', + ); } - switch (format) { - case 'raw': { - if (hasAnyNotIn(keyUsages, ['deriveKey', 'deriveBits'])) { - throw new Error(`Unsupported key usage for a ${name} key`); - } + const checkLength = + typeof keyData === 'string' || SBuffer.isBuffer(keyData) + ? keyData.length * 8 + : keyData.byteLength * 8; + if (length !== undefined && length !== checkLength) { + throw lazyDOMException('Invalid key length', 'DataError'); + } - const checkLength = - typeof keyData === 'string' || SBuffer.isBuffer(keyData) - ? keyData.length * 8 - : keyData.byteLength * 8; + const keyObject = createSecretKey(keyData as BinaryLike); + return new CryptoKey(keyObject, { name }, keyUsages, false); +}; - if (length !== undefined && length !== checkLength) { - throw new Error('Invalid key length'); - } +// Argon2 import. Node gates the format at the dispatcher level โ€” only +// 'raw-secret' enters importGenericSecretKey (webcrypto.js:813-822). To match +// that, format is the first check here; remaining ordering matches Node's +// importGenericSecretKey. +const argon2ImportKey = async ( + { name, length }: SubtleAlgorithm, + format: ImportFormat, + keyData: BufferLike | BinaryLike, + extractable: boolean, + keyUsages: KeyUsage[], +): Promise => { + if (format !== 'raw-secret') { + throw lazyDOMException( + `Unable to import ${name} key with format ${format}`, + 'NotSupportedError', + ); + } + if (extractable) { + throw lazyDOMException(`${name} keys are not extractable`, 'SyntaxError'); + } + if (hasAnyNotIn(keyUsages, ['deriveKey', 'deriveBits'])) { + throw lazyDOMException( + `Unsupported key usage for a ${name} key`, + 'SyntaxError', + ); + } - const keyObject = createSecretKey(keyData as BinaryLike); - return new CryptoKey(keyObject, { name }, keyUsages, false); - } + const checkLength = + typeof keyData === 'string' || SBuffer.isBuffer(keyData) + ? keyData.length * 8 + : keyData.byteLength * 8; + if (length !== undefined && length !== checkLength) { + throw lazyDOMException('Invalid key length', 'DataError'); } - throw new Error(`Unable to import ${name} key with format ${format}`); + const keyObject = createSecretKey(keyData as BinaryLike); + return new CryptoKey(keyObject, { name }, keyUsages, false); }; const hkdfImportKey = async ( @@ -2526,9 +2631,10 @@ export class Subtle { ); } - // Step 2: Import as key + // Step 2: Import as key. Use 'raw-secret' so derived material flows into + // AEADs / KMAC correctly โ€” they reject plain 'raw' (Node webcrypto.js:381-385). return this.importKey( - 'raw', + 'raw-secret', derivedBits, derivedKeyAlgorithm, extractable, @@ -2582,9 +2688,6 @@ export class Subtle { return bufferLikeToArrayBuffer(key.keyObject.handle.exportKey()); } - // Note: 'raw-seed' is handled above; do NOT normalize it here - if (format === 'raw-secret' || format === 'raw-public') format = 'raw'; - switch (format) { case 'spki': return (await exportKeySpki(key)) as ArrayBuffer; @@ -2593,7 +2696,9 @@ export class Subtle { case 'jwk': return exportKeyJWK(key) as JWK; case 'raw': - return exportKeyRaw(key) as ArrayBuffer; + case 'raw-secret': + case 'raw-public': + return exportKeyRaw(key, format) as ArrayBuffer; } } @@ -2851,8 +2956,10 @@ export class Subtle { extractable: boolean, keyUsages: KeyUsage[], ): Promise { - // Note: 'raw-seed' is NOT normalized โ€” PQC import functions handle it directly - if (format === 'raw-secret' || format === 'raw-public') format = 'raw'; + // Per-algorithm format handling. Some algorithms alias raw-secret/raw-public + // to 'raw' (RSA, EC, Ed/X, HMAC, HKDF, PBKDF2); others demand the + // disambiguated form (KMAC, AES-OCB, ChaCha20-Poly1305, Argon2, ML-DSA, + // ML-KEM). 'raw-seed' is never normalized โ€” PQC import handles it directly. const normalizedAlgorithm = normalizeAlgorithm(algorithm, 'importKey'); let result: CryptoKey; switch (normalizedAlgorithm.name) { @@ -2862,7 +2969,7 @@ export class Subtle { // Fall through case 'RSA-OAEP': result = rsaImportKey( - format, + aliasKeyFormat(format), data as BufferLike | JWK, normalizedAlgorithm, extractable, @@ -2873,7 +2980,7 @@ export class Subtle { // Fall through case 'ECDH': result = ecImportKey( - format, + aliasKeyFormat(format), data, normalizedAlgorithm, extractable, @@ -2881,6 +2988,9 @@ export class Subtle { ); break; case 'HMAC': + // No aliasing โ€” Node routes HMAC straight into mac.js, which accepts + // 'raw' / 'raw-secret' / 'jwk' and rejects everything else + // (webcrypto.js:774-781, mac.js:136-174). result = await hmacImportKey( normalizedAlgorithm, format, @@ -2920,10 +3030,18 @@ export class Subtle { ); break; case 'PBKDF2': + result = await pbkdf2ImportKey( + normalizedAlgorithm, + aliasKeyFormat(format), + data as BufferLike | BinaryLike, + extractable, + keyUsages, + ); + break; case 'Argon2d': case 'Argon2i': case 'Argon2id': - result = await importGenericSecretKey( + result = await argon2ImportKey( normalizedAlgorithm, format, data as BufferLike | BinaryLike, @@ -2933,7 +3051,7 @@ export class Subtle { break; case 'HKDF': result = await hkdfImportKey( - format, + aliasKeyFormat(format), data as BufferLike | BinaryLike, normalizedAlgorithm, extractable, @@ -2948,7 +3066,7 @@ export class Subtle { // Fall through case 'Ed448': result = edImportKey( - format, + aliasKeyFormat(format), data as BufferLike | JWK, normalizedAlgorithm, extractable, @@ -3113,8 +3231,10 @@ export class Subtle { } const { sharedKey, ciphertext } = this._encapsulateCore(algorithm, key); + // Node imports the encapsulated shared bits as 'raw-secret' + // (webcrypto.js:1370-1374) so AEADs / KMAC accept the result. const importedKey = await this.importKey( - 'raw', + 'raw-secret', sharedKey, sharedKeyAlgorithm, extractable, @@ -3155,8 +3275,10 @@ export class Subtle { } const sharedKey = this._decapsulateCore(algorithm, key, ciphertext); + // Node imports the decapsulated shared bits as 'raw-secret' + // (webcrypto.js:1490-1494). return this.importKey( - 'raw', + 'raw-secret', sharedKey, sharedKeyAlgorithm, extractable,