Conversation
* `miscreant` → `@noble/ciphers` * `rfc4648` → `@scure/base`
There was a problem hiding this comment.
Pull request overview
This pull request modernizes the frontend's cryptography stack by replacing deprecated libraries (miscreant and rfc4648) with actively maintained alternatives (@noble/ciphers and @scure/base). The migration includes:
- Replacing all base encoding/decoding operations throughout the codebase
- Migrating AES-SIV implementation to a new library
- Updating multiple dependencies to their latest versions
- Adding
skipLibCheck: trueto TypeScript configuration
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
frontend/package.json |
Removed miscreant and rfc4648, added @noble/ciphers and @scure/base, updated vitest, vite, vue, and other dependencies |
frontend/package-lock.json |
Updated lockfile with new dependencies and removed old ones |
frontend/tsconfig.json |
Added skipLibCheck: true compiler option |
frontend/src/common/crypto.ts |
Migrated to new APIs: base64.decode() instead of base64.parse(), aessiv() instead of miscreant.SIV, updated all encoding operations |
frontend/src/common/jwe.ts |
Updated all base64url encoding/decoding to use base64urlnopad from @scure/base |
frontend/src/common/jwt.ts |
Migrated JWT encoding/decoding to new base64url API |
frontend/src/common/backend.ts |
Updated base64 import and usage |
frontend/src/common/util.ts |
Added Uint8Array<ArrayBuffer> return type annotations |
frontend/src/common/userdata.ts |
Updated type annotations and decode calls |
frontend/src/common/wot.ts |
Migrated base64 decoding with type casts |
frontend/src/components/*.vue |
Updated base64 imports and decode calls in Vue components |
frontend/test/common/*.spec.ts |
Updated test files to use new encoding APIs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
WalkthroughThis PR replaces rfc4648 and miscreant with Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/common/crypto.ts (1)
207-222: Incorrect AES-SIV key handling—pass raw key directly to @noble/ciphers.@noble/ciphers expects a single raw AES key (16/24/32 bytes) and internally derives/splits subkeys for SIV. The code manually splits the raw key into macKey and encKey, then reorders them before passing to
aessiv(). This manual key manipulation contradicts the library's design and should be removed. Pass the raw key directly:aessiv(rawkey).encrypt(dirHash).
🧹 Nitpick comments (2)
frontend/tsconfig.json (1)
19-19: Stale comment referencing removed dependency.The comment references
miscreantas a workaround, but this PR removesmiscreantin favor of@noble/ciphers. Consider removing this comment or updating it to reflect the current reason foruseUnknownInCatchVariables: false.- "useUnknownInCatchVariables": false, // Workaround for `node_modules/miscreant/src/providers/webcrypto.ts:21:11 - error TS18046: 'e' is of type 'unknown'.` + "useUnknownInCatchVariables": false,frontend/src/common/jwe.ts (1)
15-15: Typed-array generics (Uint8Array<ArrayBuffer>) are wired through correctly; minor JSDoc nitUpdating
ConcatKDF.kdf,JWEParser’s internal buffers, andECDH_ES.lengthPrefixedto useUint8Array<ArrayBuffer>keeps them compatible with WebCrypto’sBufferSourceexpectations while making the backing buffer type explicit; current call sites (deriveContentKey, AES‑GCM encrypt/decrypt, unwrapKey, etc.) all remain valid.The only nit is that
otherInfoinConcatKDF.kdfis documented as “Optional” but is required by the signature and always passed in; consider updating the comment (or adding a default) so the docs reflect actual usage.Also applies to: 56-59, 240-245
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
frontend/package.json(3 hunks)frontend/src/common/backend.ts(2 hunks)frontend/src/common/crypto.ts(13 hunks)frontend/src/common/jwe.ts(10 hunks)frontend/src/common/jwt.ts(4 hunks)frontend/src/common/userdata.ts(4 hunks)frontend/src/common/util.ts(2 hunks)frontend/src/common/wot.ts(3 hunks)frontend/src/components/AuditLogDetailsSignedWotId.vue(2 hunks)frontend/src/components/GrantPermissionDialog.vue(2 hunks)frontend/test/common/crypto.spec.ts(4 hunks)frontend/test/common/jwe.spec.ts(2 hunks)frontend/tsconfig.json(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-07-27T13:14:40.963Z
Learnt from: overheadhunter
Repo: cryptomator/hub PR: 282
File: frontend/src/common/userdata.ts:82-88
Timestamp: 2024-07-27T13:14:40.963Z
Learning: The error handling suggestion for `createBrowserKeys` in `frontend/src/common/userdata.ts` was discussed and resolved in PR #282, and should not be repeated in future reviews.
Applied to files:
frontend/src/common/userdata.ts
📚 Learning: 2025-02-13T10:33:38.555Z
Learnt from: overheadhunter
Repo: cryptomator/hub PR: 315
File: backend/src/main/java/org/cryptomator/hub/api/UserDto.java:0-0
Timestamp: 2025-02-13T10:33:38.555Z
Learning: In the UserDto class of the Cryptomator Hub project, the `JsonCreator` constructor is used to handle both new and legacy field names (e.g., `ecdhPublicKey`/`publicKey` and `privateKeys`/`privateKey`) during JSON deserialization, while a separate regular constructor is used for programmatic object creation with only the new field names.
Applied to files:
frontend/src/common/crypto.ts
🧬 Code graph analysis (4)
frontend/src/common/wot.ts (2)
frontend/src/common/crypto.ts (2)
asPublicKey(458-464)UserKeys(260-383)frontend/src/common/userdata.ts (2)
ecdhPublicKey(68-75)ecdsaPublicKey(82-86)
frontend/src/common/jwt.ts (1)
frontend/src/common/util.ts (1)
UTF8(3-26)
frontend/src/common/userdata.ts (1)
frontend/src/common/backend.ts (1)
me(268-270)
frontend/src/common/crypto.ts (2)
frontend/src/common/util.ts (1)
UTF8(3-26)frontend/src/common/userdata.ts (1)
ecdsaPublicKey(82-86)
🔇 Additional comments (38)
frontend/src/common/backend.ts (2)
324-324: Switch tobase64.encode(bytes)for SVG data URL is appropriateEncoding the UTF‑8 SVG bytes with
base64.encode(bytes)to build thedata:image/svg+xml;base64,...URL is correct and matches the intended deterministic behavior of the previous Base64 implementation.
1-1: Import ofbase64from@scure/baseis correctThe named
base64export from@scure/baseis a documented API that providesencode(bytes: Uint8Array): stringanddecode(str: string): Uint8Arraymethods. This import is appropriate for the use case.frontend/package.json (3)
25-25: LGTM on minor/patch version updates.The patch bumps for @intlify/devtools-types (^11.1.12 → ^11.2.2) and vite (^7.1.10 → ^7.2.7), along with runtime updates (vue, vue-i18n) are low-risk and appropriately scoped.
Also applies to: 43-43
34-34: Verify vitest v4 breaking changes and compatibility.The vitest ecosystem jump from v3 to v4 is a major version bump. Confirm that the test suite and any vitest plugins/integrations (e.g., @vitest/coverage-v8, vue integration) are compatible with v4.0.15 and that no breaking changes affect existing test configuration or test code.
Run the following shell script to check for test suite compatibility with vitest v4:
#!/bin/bash # Description: Verify vitest v4 migration compatibility # Check if vitest config file exists and show its content fd -e 'vitest.config|vite.config' -t f | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && cat {}' # Check package.json for vitest-related scripts and plugins echo -e "\n=== Test Scripts ===" && jq '.scripts | select(. != null) | to_entries[] | select(.value | contains("vitest"))' frontend/package.json # Check for test files to understand test patterns echo -e "\n=== Test Files ===" && fd -e '\.test\.(ts|js)$|\.spec\.(ts|js)$' frontend/ -t f | head -10Also applies to: 44-44
51-52: @noble/ciphers and @scure/base are production-ready and well-maintained.Both libraries are actively maintained with recent releases (2025), independently audited by Cure53 (@scure/base in Jan 2022, @noble/ciphers in Sep 2024), and follow strong supply-chain practices with PGP-signed commits and CI provenance. No known vulnerabilities or malicious activity reported. These are appropriate replacements for miscreant and rfc4648.
frontend/src/common/wot.ts (3)
1-1: LGTM!The import migration from
rfc4648to@scure/baseis correct and consistent with the rest of the codebase changes.
73-73: Consistent encoding migration.The change from
base64.parsetobase64.decodewith theUint8Array<ArrayBuffer>cast follows the established pattern across the codebase. The decoded key is correctly passed toasPublicKey.
87-88: Fingerprint computation updated correctly.Both ECDH and ECDSA public key decoding uses the same consistent pattern as line 73.
frontend/src/components/AuditLogDetailsSignedWotId.vue (2)
41-41: LGTM!Import migration is consistent with the codebase-wide change.
78-78: Decoding updated correctly for signature verification.The
base64.decodewithUint8Array<ArrayBuffer>cast matches the pattern used elsewhere for key material decoding.frontend/test/common/crypto.spec.ts (4)
1-1: LGTM!Test imports correctly updated to use
@scure/basewith bothbase64andbase64urlnopadvariants.
113-113: Test data decoding updated correctly.The test key decoding uses the same pattern as production code.
194-214: Key export encoding tests updated correctly.All
base64.stringifycalls replaced withbase64.encode. The expected base64 strings remain unchanged, confirming encoding compatibility.
244-244: JWK thumbprint test updated correctly.Using
base64urlnopad.encodeinstead ofbase64url.stringify(..., { pad: false })is the correct migration. The expected value matches RFC 7638 Section 3.1.frontend/src/common/util.ts (2)
12-14: LGTM!The return type annotation
Uint8Array<ArrayBuffer>accurately reflects whatTextEncoder.encode()returns and improves type consistency across the codebase.
141-164: LGTM!The return type annotation change is correct since
new Uint8Array(length)creates anArrayBuffer-backed typed array.frontend/src/common/jwt.ts (5)
1-1: LGTM!Correct import for unpadded base64url encoding as required by JWT specification.
23-27: JWT building correctly updated.The header and payload encoding using
base64urlnopad.encodeis correct for JWT format per RFC 7519.
40-40: Signature encoding updated correctly.The signature is correctly encoded using unpadded base64url.
50-61: JWT parsing correctly updated.Header and payload decoding using
base64urlnopad.decodeis consistent with the encoding changes.
68-68: Signature decoding correctly updated.The cast to
Uint8Array<ArrayBuffer>ensures type compatibility withcrypto.subtle.verify.frontend/src/common/userdata.ts (4)
1-1: LGTM!Import migration is consistent with the codebase-wide change.
68-75: LGTM!The return type update and decoding change are correct. The cast ensures type compatibility with downstream crypto operations.
82-86: LGTM!Consistent with the
ecdhPublicKeygetter pattern, handling the optional case correctly.
168-168: LGTM!Device public key decoding correctly migrated for the ECDSA key migration flow.
frontend/src/common/crypto.ts (10)
1-4: LGTM!Import changes correctly bring in the new cryptographic primitives from
@noble/ciphersand encoding utilities from@scure/base.
87-92: LGTM!Key decoding from JWE payload correctly migrated with proper zeroing of sensitive data in the finally block.
115-126: LGTM!PBKDF2 salt and wrapped key decoding correctly updated for the legacy admin password flow.
148-148: LGTM!Public key cast ensures type compatibility with
crypto.subtle.importKey.
197-204: LGTM!Vault config JWT creation correctly uses
base64urlnopadfor header, payload, and signature encoding.
313-319: LGTM!Private key decoding in
createFromJwecorrectly usesbase64.decodewith proper type casting forcrypto.subtle.importKey.
334-344: LGTM!Public key encoding methods correctly migrated from
base64.stringifytobase64.encode.
363-367: LGTM!The signature change to
Uint8Array<ArrayBuffer>is consistent with the type annotations used throughout the codebase for decoded key material.
374-377: LGTM!Private key encoding for JWE payload correctly migrated.
449-454: LGTM!Browser key ID and public key encoding correctly updated to use
base16.encodeandbase64.encoderespectively.frontend/test/common/jwe.spec.ts (2)
1-1: Switching test base64 import to@scure/basematches the runtime codeUsing
base64urlnopadhere keeps the test harness aligned with the implementation insrc/common/jwe.ts, so encoded values should stay in lockstep with production behavior.Please confirm that vitest runs against this branch use the same
@scure/baseversion as the app build so tests don’t mask version‑specific differences.
115-123: apu/apv encoding viabase64urlnopad.encodepreserves the ECDH-ES test vectorEncoding the
apu/apvUint8Arrays withbase64urlnopad.encodematches the JOSE requirement for unpadded base64url and should yield the same strings as the previousstringify(..., { pad: false })for this vector.Please re‑run the ECDH‑ES vector test and, if you have older artifacts, spot‑check that
apu/apvencodings are identical across the library swap.frontend/src/common/jwe.ts (1)
1-1: Base64 migration to@scure/baselooks consistent and JOSE‑compliantAll JWE‑related encoding/decoding (header, IV, encrypted key, ciphertext, tag, apu/apv, and p2s) now uses
base64urlnopad.encode/.decode, and builder/parser remain symmetric, which aligns with RFC 7516/7518’s requirement for unpadded base64url encodings.One behavioral change to be aware of is that
base64urlnopad.decodemay be stricter than the previousrfc4648helper (especially around padding or “loose” inputs), so if you accept JWEs from third parties that might emit padded or non‑canonical encodings, it’s worth double‑checking that they still parse cleanly or normalizing inputs before decode.If you have sample JWEs from external producers (esp. for PBES2 and ECDH‑ES flows), please run decryption against this branch to confirm there are no regressions in how non‑canonical base64url inputs are handled.
Also applies to: 62-66, 103-104, 145-151, 166-176, 189-193, 206-207, 218-221
`Type 'unknown' is not assignable to type 'Error | null | undefined'`
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/components/SignUserKeysDialog.vue (1)
119-120: Good defensive error handling with logging.The addition of
console.errorimproves debugging visibility, and theinstanceof Errorcheck ensures type-safe error handling for the template.Optionally, consider preserving the original error value when it's not an Error instance:
- onSignError.value = error instanceof Error ? error : new Error('Unknown Error'); + onSignError.value = error instanceof Error ? error : new Error(`Unknown Error: ${error}`);This would capture more diagnostic information if an unexpected non-Error value is thrown.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/SignUserKeysDialog.vue(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-07-27T13:14:40.963Z
Learnt from: overheadhunter
Repo: cryptomator/hub PR: 282
File: frontend/src/common/userdata.ts:82-88
Timestamp: 2024-07-27T13:14:40.963Z
Learning: The error handling suggestion for `createBrowserKeys` in `frontend/src/common/userdata.ts` was discussed and resolved in PR #282, and should not be repeated in future reviews.
Applied to files:
frontend/src/components/SignUserKeysDialog.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run Tests
This pull request updates the frontend's cryptography and encoding libraries to improve security, compatibility, and maintainability. The main changes involve replacing the deprecated
miscreantandrfc4648libraries with@noble/ciphersand@scure/base, updating related code to use the new APIs, and upgrading several dependencies to their latest versions.Dependency updates and library replacements:
Replaced the
miscreantandrfc4648libraries with@noble/ciphersand@scure/basefor cryptographic operations and base encoding/decoding. Updated all related imports and method calls infrontend/src/common/crypto.ts,frontend/src/common/jwe.ts, andfrontend/src/common/backend.ts. [1] [2] [3] [4]Upgraded several dev and runtime dependencies in
frontend/package.json, includingvite,vitest,vue,vue-i18n,@intlify/devtools-types, and removed unused/deprecated packages. [1] [2] [3]Code refactoring for new libraries:
@scure/basemethods (encode,decode, etc.), replacing previous usage ofstringify/parseand changing method signatures where necessary. This affects key import/export, JWT and JWE handling, and cryptographic utilities in multiple classes (VaultKeys,UserKeys,BrowserKeys,JWEParser,JWEBuilder, etc.). [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16] [17] [18] [19]Cryptography improvements:
miscreantto@noble/ciphers/aes.jsfor deterministic encryption, updating the relevant logic and comments inVaultKeys.hashDirectoryId.Type and compatibility fixes:
Uint8Array<ArrayBuffer>where required). [1] [2] [3]These changes modernize the codebase, remove deprecated dependencies, and ensure more robust cryptographic handling.