fix(ts): fix TS SDK build + export encryption module (v3.3.0) — closes #1404 - #1409
Conversation
… (v3.3.0) - Remove ignoreDeprecations: '6.0' from tsconfig.json (invalid in TS 6) - Replace @noble/ciphers/utils.js randomBytes import with node:crypto webcrypto.getRandomValues (noble dropped randomBytes export) - Bump version to 3.3.0 — encryption module (X3DH, MeshClient, SecureChannel, DoubleRatchet) now in published dist/ This unblocks Azure/kars#29 which depends on encryption exports. Closes #1404 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🤖 AI Agent: security-scanner — Security Review for PR: fix(ts): fix TS SDK build + export encryption module (v3.3.0)Security Review for PR: fix(ts): fix TS SDK build + export encryption module (v3.3.0)1. Prompt Injection Defense BypassNo changes in this PR directly relate to prompt injection or input sanitization mechanisms. The changes focus on cryptographic utilities and TypeScript build configurations. 2. Policy Engine CircumventionNo policy evaluation logic or enforcement mechanisms are modified in this PR. The changes are isolated to cryptographic utilities and build configurations. 3. Trust Chain WeaknessesThe PR modifies cryptographic random number generation by replacing
if (!webcrypto || typeof webcrypto.getRandomValues !== 'function') {
throw new Error("Secure random number generation is not available.");
}Rating: 🟠 HIGH 4. Credential ExposureNo sensitive credentials or secrets are introduced or exposed in this PR. 5. Sandbox EscapeThe PR does not introduce any changes that could lead to a sandbox escape. The changes are limited to cryptographic utilities and TypeScript build configurations. 6. Deserialization AttacksNo deserialization logic or changes to data parsing mechanisms are introduced in this PR. 7. Race ConditionsNo concurrent operations or time-of-check-to-time-of-use (TOCTOU) vulnerabilities are introduced in this PR. 8. Supply ChainThe PR replaces the
Additionally, the PR does not introduce any new dependencies, so risks related to dependency confusion or typosquatting are minimal. Summary of Findings
Recommendations
Final Rating: 🟠 HIGHThe primary concern is the potential for insecure randomness if |
There was a problem hiding this comment.
🤖 AI Agent: code-reviewer
Review Summary
This pull request addresses compatibility issues with TypeScript 6, replaces the deprecated randomBytes function from @noble/ciphers/utils.js with a custom implementation using webcrypto.getRandomValues, and updates the package version to 3.3.0. It also exports the encryption module to unblock a dependent project.
While the changes are generally well-implemented, there are a few areas of concern, particularly around cryptographic operations and backward compatibility. Below are detailed comments and recommendations.
🔴 CRITICAL
-
Insecure Random Number Generation
- The replacement of
@noble/ciphers/utils.js'srandomByteswith a custom implementation usingwebcrypto.getRandomValuesintroduces a potential security risk if not handled carefully. - While
webcrypto.getRandomValuesis a cryptographically secure random number generator, the implementation should ensure that it is used consistently and correctly across all environments, including Node.js and browser contexts. - Recommendation: Add a runtime check to ensure that
webcryptois available and functional. If it is not, fail gracefully with an appropriate error message. Additionally, document the requirement forwebcryptoin the SDK's README or documentation.
- The replacement of
-
Lack of Tests for
randomBytesReplacement- The new
randomBytesimplementation is a critical part of cryptographic operations. However, there are no tests to verify its correctness or behavior in different environments. - Recommendation: Add unit tests to validate the behavior of the
randomBytesfunction. Ensure it produces random values of the correct length and works as expected in all supported environments.
- The new
🟡 WARNING
-
Potential Breaking Change in
randomBytesImplementation- The replacement of
@noble/ciphers/utils.js'srandomByteswith a custom implementation may lead to subtle differences in behavior, especially if the old implementation had specific characteristics (e.g., entropy source, distribution) that are not replicated in the new implementation. - Recommendation: Clearly document this change in the release notes and communicate it to users of the SDK. Highlight any potential differences in behavior and provide guidance for migration if necessary.
- The replacement of
-
Version Bump to
3.3.0- The version bump from
3.2.2to3.3.0suggests a minor version update. However, the change in therandomBytesimplementation could be considered a breaking change, depending on how it is used by downstream consumers. - Recommendation: Consider whether this should be a major version bump (
4.0.0) instead of a minor version bump, as it may affect backward compatibility.
- The version bump from
💡 SUGGESTIONS
-
Centralize
randomBytesImplementation- The
randomBytesfunction is defined twice (inratchet.tsandx3dh.ts). This duplication increases the risk of inconsistencies and makes future maintenance harder. - Recommendation: Extract the
randomBytesfunction into a shared utility module and import it where needed.
- The
-
Improve Documentation
- The PR description mentions that the encryption module is now exported, but there is no mention of this in the code diff or any accompanying documentation updates.
- Recommendation: Update the SDK documentation to reflect the new encryption module export. Include usage examples and any relevant details about the new functionality.
-
Test Coverage for Encryption Modules
- The
ratchet.tsandx3dh.tsfiles contain critical cryptographic logic, but there is no indication in this PR that additional tests were added or updated to cover these changes. - Recommendation: Ensure that the encryption modules have comprehensive test coverage, including edge cases and failure scenarios.
- The
-
TypeScript Configuration
- The
tsconfig.jsonfile removes theignoreDeprecationsoption for TypeScript 6. While this is likely intentional, it could lead to warnings or errors if deprecated APIs are used in the codebase. - Recommendation: Audit the codebase for any usage of deprecated TypeScript APIs and address them proactively.
- The
-
Changelog Update
- The PR does not include an update to the changelog to document the changes made in this release.
- Recommendation: Update the changelog to include details about the TypeScript 6 compatibility fix, the
randomBytesreplacement, and the new encryption module export.
Summary of Actions
- 🔴 Add runtime checks and documentation for
webcryptousage. - 🔴 Add unit tests for the new
randomBytesimplementation. - 🟡 Consider whether the version bump should be major instead of minor.
- 💡 Centralize the
randomBytesfunction in a shared utility module. - 💡 Update SDK documentation to reflect the new encryption module export.
- 💡 Ensure comprehensive test coverage for encryption modules.
- 💡 Audit the codebase for deprecated TypeScript APIs.
- 💡 Update the changelog to document the changes.
Let me know if you need further clarification or assistance!
… (v3.3.0) (microsoft#1409) - Remove ignoreDeprecations: '6.0' from tsconfig.json (invalid in TS 6) - Replace @noble/ciphers/utils.js randomBytes import with node:crypto webcrypto.getRandomValues (noble dropped randomBytes export) - Bump version to 3.3.0 — encryption module (X3DH, MeshClient, SecureChannel, DoubleRatchet) now in published dist/ This unblocks Azure/kars#29 which depends on encryption exports. Closes microsoft#1404 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes TS build for TypeScript 6, replaces dropped noble randomBytes, bumps to v3.3.0 with encryption exports. Unblocks Azure/kars#29.