-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Cosmosdb/4.3.0-fixes #33355
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cosmosdb/4.3.0-fixes #33355
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes client‐side encryption issues in the Cosmos DB SDK by converting key data types from Buffer to Uint8Array, renaming parameters for clarity, and updating type definitions and sample code.
- Methods in key wrapping/unwrapping now use Uint8Array for consistency.
- Parameter names have been updated (e.g. from “id” to “clientEncryptionKeyId”) and client encryption metadata and included paths are now defined as object literals.
- The API and sample documentation have been updated accordingly.
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
sdk/cosmosdb/cosmos/src/encryption/EncryptionKeyStoreProvider.ts | Converts keys to/from Uint8Array and Buffer for proper wrapping/unwrapping. |
sdk/cosmosdb/cosmos/src/encryption/EncryptionProcessor.ts | Updates wrapped key conversion to Uint8Array. |
sdk/cosmosdb/cosmos/review/cosmos.api.md | Updates method signatures to use Uint8Array instead of Buffer. |
sdk/cosmosdb/cosmos/src/client/Database/Database.ts | Renames parameters (id → clientEncryptionKeyId) and updates key conversion logic. |
sdk/cosmosdb/cosmos/src/encryption/Cache/ProtectedDataEncryptionKeyCache.ts | Adjusts parameter conversion from Buffer to Uint8Array. |
sdk/cosmosdb/cosmos/src/encryption/EncryptionKeyWrapMetadata.ts | Switches from a class to an interface definition with updated properties. |
sdk/cosmosdb/cosmos/src/encryption/ClientEncryptionIncludedPath.ts | Changes from a class to an interface and standardizes property names. |
sdk/cosmosdb/cosmos/src/encryption/EncryptionKeyResolver/AzureKeyVaultEncryptionKeyResolver.ts | Updates key wrapping/unwrapping methods to use Uint8Array. |
Samples & CHANGELOG | Updates sample code and changelog snippets to use object literal style for metadata and paths. |
sdk/cosmosdb/cosmos/src/encryption/EncryptionKeyResolver/EncryptionKeyResolver.ts | Adjusts interface definitions to use Uint8Array. |
sdk/cosmosdb/cosmos/src/encryption/ClientEncryptionKey/ClientEncryptionKeyProperties.ts | Changes the type of wrappedDataEncryptionKey from Buffer to Uint8Array. |
Tests | Updates test constructors to align with new object literal definitions for encryption settings. |
Comments suppressed due to low confidence (1)
sdk/cosmosdb/cosmos/samples-dev/ClientSideEncryption.ts:147
- There is a spelling error in the property name; 'algoruthm' should be 'algorithm'.
algoruthm: KeyEncryptionAlgorithm.RSA_OAEP,
API change check APIView has identified API level changes in this PR and created following API reviews. |
sdk/cosmosdb/cosmos/samples/v4/javascript/ClientSideEncryption.js
Outdated
Show resolved
Hide resolved
sdk/cosmosdb/cosmos/src/encryption/EncryptionKeyResolver/EncryptionKeyResolver.ts
Outdated
Show resolved
Hide resolved
Add one more change, rename |
makes more sense but the public API name will differ from .NET. Are we fine with that? |
/azp run js - cosmosdb - ci |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR addresses fixes and enhancements to client-side encryption functionality in the Cosmos DB SDK. The changes include improved validation for encryption policies, updates to TTL configuration (from hours to seconds), and refactoring of various encryption-related APIs and samples for consistency and type safety.
- Improved client encryption policy validation and error handling.
- Updated encryption options and key wrapping APIs to use consistent types (e.g. Uint8Array) and naming.
- Refactored sample code and public API documentation to reflect these changes.
Reviewed Changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
sdk/cosmosdb/cosmos/src/common/helper.ts | Added and exposed validateClientEncryptionPolicy function for better policy validation. |
sdk/cosmosdb/cosmos/src/common/constants.ts | Updated encryption cache TTL from hours to seconds. |
sdk/cosmosdb/cosmos/samples/v4/javascript/ClientSideEncryption.js | Updated sample to use clientEncryptionOptions and new key wrap metadata structure. |
sdk/cosmosdb/cosmos/samples/v4/typescript/src/ClientSideEncryption.ts | Refactored TypeScript sample to use object literals for encryption metadata. |
sdk/cosmosdb/cosmos/src/client/Item/Items.ts | Revised extraction of item id with updated type casting. |
sdk/cosmosdb/cosmos/src/client/Database/Database.ts | Renamed parameters from id to clientEncryptionKeyId and updated buffer conversion. |
sdk/cosmosdb/cosmos/src/encryption/ClientEncryptionOptions.ts | Changed encryptionKeyTimeToLive prop to use seconds. |
sdk/cosmosdb/cosmos/review/cosmos.api.md | Updated public API documentation to reflect interface changes. |
sdk/cosmosdb/cosmos/src/encryption/Cache/ProtectedDataEncryptionKeyCache.ts | Updated method signature converting Buffer to Uint8Array for encrypted values. |
sdk/cosmosdb/cosmos/src/client/Container/Containers.ts | Adjusted client encryption policy validation by using validateClientEncryptionPolicy. |
sdk/cosmosdb/cosmos/src/CosmosClient.ts | Updated client configuration for encryption TTL in seconds. |
sdk/cosmosdb/cosmos/src/encryption/ClientEncryptionKey/ClientEncryptionKeyProperties.ts | Changed wrappedDataEncryptionKey type from Buffer to Uint8Array. |
sdk/cosmosdb/cosmos/src/client/Container/Container.ts | Updated constructor visibility and method signature for item retrieval. |
sdk/cosmosdb/cosmos/samples-dev/ClientSideEncryption.ts | Refactored sample to use updated encryption metadata and options. |
Comments suppressed due to low confidence (2)
sdk/cosmosdb/cosmos/src/client/Item/Items.ts:461
- [nitpick] The use of 'as any' to extract the item id may hide potential type issues. Consider defining a proper interface for 'response.result' and extracting the id in a type-safe manner.
(response.result as any).id,
sdk/cosmosdb/cosmos/src/client/Container/Containers.ts:195
- [nitpick] Assigning a default policyFormatVersion of 1 may be misleading if the encryption policy includes paths that require version 2. Ensure that the default value aligns with the encryption requirements or is explicitly set by the caller.
body.clientEncryptionPolicy.policyFormatVersion = body.clientEncryptionPolicy.policyFormatVersion ?? 1;
|
Packages impacted by this PR
@azure/cosmos
Issues associated with this PR
Describe the problem that is addressed by this PR
This PR fixes issues related to client side encryption
What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?
Are there test cases added in this PR? (If not, why?)
Provide a list of related PRs (if any)
Command used to generate this PR:**(Applicable only to SDK release request PRs)
Checklists