feat(cloud-sync): integrate keyless credential into sync credential system #9894
Conversation
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
sidmorizon
requested review from
ezailWang,
huhuanming,
originalix and
revan-zhang
as code owners
Contributor
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
sidmorizon
force-pushed
the
feat/keyless-sync
branch
4 times, most recently
from
415e922 to
18d0364
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🟡 excludeKeylessWallet flag is ignored, so callers cannot actually exclude keyless wallets
ServiceAccount.getAllWallets({ excludeKeylessWallet: true }) is used by code that expects keyless wallets to be filtered out, but the implementation contains a no-op branch.
Actual behavior: keyless wallets are still returned even when exclusion is requested.
Expected: when excludeKeylessWallet is true, return a list with wallet.isKeyless !== true.
Click to expand
packages/kit-bg/src/services/ServiceAccount/ServiceAccount.ts:371-396
// Filter out keyless wallets if excludeKeylessWallet is true
if (excludeKeylessWallet) {
// do nothing
}
return { wallets, allDevices };Impact: any feature relying on this filter (e.g., HW/QR device lists built from getAllWallets) may include keyless wallets unexpectedly.
(Refers to lines 371-396)
Recommendation: Implement the filter, e.g. if (excludeKeylessWallet) wallets = wallets.filter(w => !w.isKeyless);.
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
48
to
104
| async upload(params: { | ||
| client: AxiosInstance; | ||
| signatureHeader: string; | ||
| postData: ICloudSyncUploadPostData; | ||
| }): Promise<AxiosResponse<IApiClientResponse<ICloudSyncUploadResult>, any>> { | ||
| return this.postToMockServer<ICloudSyncUploadResult>({ | ||
| client: params.client, | ||
| url: '/prime/v1/sync/upload', | ||
| signatureHeader: params.signatureHeader, | ||
| postData: params.postData, | ||
| }); | ||
| } | ||
|
|
||
| async checkStatus(params: { | ||
| client: AxiosInstance; | ||
| signatureHeader: string; | ||
| postData: ICloudSyncCheckServerStatusPostData; | ||
| }): Promise< | ||
| AxiosResponse<IApiClientResponse<ICloudSyncCheckServerStatusResult>, any> | ||
| > { | ||
| return this.postToMockServer<ICloudSyncCheckServerStatusResult>({ | ||
| client: params.client, | ||
| url: '/prime/v1/sync/check', | ||
| signatureHeader: params.signatureHeader, | ||
| postData: params.postData, | ||
| }); | ||
| } | ||
|
|
||
| async download(params: { | ||
| client: AxiosInstance; | ||
| signatureHeader?: string; | ||
| postData: ICloudSyncDownloadPostData; | ||
| }): Promise< | ||
| AxiosResponse<IApiClientResponse<ICloudSyncDownloadResult>, any> | ||
| > { | ||
| if (params.signatureHeader) { | ||
| return this.postToMockServer<ICloudSyncDownloadResult>({ | ||
| client: params.client, | ||
| url: '/prime/v1/sync/download', | ||
| signatureHeader: params.signatureHeader, | ||
| postData: params.postData, | ||
| }); | ||
| } | ||
|
|
||
| throw new OneKeyLocalError('Signature header is not set'); | ||
| } | ||
|
|
||
| async clear(params: { | ||
| client: AxiosInstance; | ||
| signatureHeader: string; | ||
| }): Promise<void> { | ||
| await this.postToMockServer<{ cleared: boolean }>({ | ||
| client: params.client, | ||
| url: '/prime/v1/sync/clear', | ||
| signatureHeader: params.signatureHeader, | ||
| postData: {}, | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
🔴 Keyless mock client calls non-keyless endpoints that the mock server never serves
Keyless sync is wired to keylessMockApi, but the mock client posts to /prime/v1/sync/upload, /prime/v1/sync/check, /prime/v1/sync/download, /prime/v1/sync/clear while the mock server only handles *-keyless routes (/prime/v1/sync/upload-keyless, /prime/v1/sync/check-keyless, /prime/v1/sync/download-keyless, /prime/v1/sync/clear-keyless).
Actual behavior: all Keyless calls will get 404 Not found from the mock server.
Expected: client and server should use the same paths.
Click to expand
- Client mock API uses non-keyless URLs:
packages/kit-bg/src/services/ServicePrimeCloudSync/keylessCloudSyncMockApi.ts:48-105url: '/prime/v1/sync/upload' url: '/prime/v1/sync/check' url: '/prime/v1/sync/download' url: '/prime/v1/sync/clear'
- Mock server routes are
*-keylessonly:packages/kit-bg/src/services/ServicePrimeCloudSync/keylessCloudSyncMockServer/index.ts:286-461if (url === '/prime/v1/sync/upload-keyless') { ... } if (url === '/prime/v1/sync/check-keyless') { ... } if (url === '/prime/v1/sync/download-keyless') { ... } if (url === '/prime/v1/sync/clear-keyless') { ... }
Impact: Keyless cloud sync can’t function at all when using the included mock server.
Recommendation: Update keylessCloudSyncMockApi URLs to match the mock server routes (use .../upload-keyless, .../check-keyless, .../download-keyless, .../clear-keyless), or change the server to also serve the non-suffixed routes.
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+888
to
+913
| const localData: ICloudSyncServerItem[] = localItems | ||
| .map((item) => { | ||
| let dataTimestamp = item.dataTime; | ||
| if (setUndefinedTimeToNow && isNil(dataTimestamp)) { | ||
| dataTimestamp = now; | ||
| } | ||
| const serverItem = this.convertLocalItemToServerItem({ | ||
| localItem: item, | ||
| dataTimestamp, | ||
| }); | ||
| if (process.env.NODE_ENV !== 'production') { | ||
| // @ts-ignore | ||
| serverItem.$$dataTimestampStr = new Date( | ||
| serverItem?.dataTimestamp || 0, | ||
| ).toLocaleString(); | ||
| } | ||
| return serverItem; |
Contributor
There was a problem hiding this comment.
🟡 Dev-only timestamp debug property assignment can throw when convertLocalItemToServerItem returns null
convertLocalItemToServerItem() can return null (it explicitly skips Lock items when pwdHash is keyless). _callApiUploadItems unconditionally writes serverItem.$$dataTimestampStr = ... in dev builds, which will throw if serverItem is null.
Actual behavior: in dev builds, uploading a local sync item that gets skipped (e.g., a Lock item with keyless pwdHash) crashes the upload path.
Expected: skipped items should be handled without exceptions.
Click to expand
convertLocalItemToServerItemreturns null for keyless Lock:packages/kit-bg/src/services/ServicePrimeCloudSync/ServicePrimeCloudSync.tsx:2343-2354if (localItem.dataType === Lock && isKeylessPwdHash(localItem.pwdHash)) { return null; }
- Caller assigns a property on possibly-null value:
packages/kit-bg/src/services/ServicePrimeCloudSync/ServicePrimeCloudSync.tsx:888-904const serverItem = this.convertLocalItemToServerItem(...); if (process.env.NODE_ENV !== 'production') { // @ts-ignore serverItem.$$dataTimestampStr = ...; // throws if serverItem is null }
Recommendation: Guard the debug assignment: if (serverItem && process.env.NODE_ENV !== 'production') { ... }, or move the debug augmentation after .filter(Boolean).
Was this helpful? React with 👍 or 👎 to provide feedback.
sidmorizon
force-pushed
the
feat/keyless-sync
branch
from
890f2e6 to
92f0b2c
Compare
Add keylessData and keylessDataTime fields to cloud sync schemas and types to support keyless wallet encryption alongside existing password-based encryption.
Add comprehensive keyless cloud sync methods to ServicePrimeCloudSync: - Keyless credential derivation and caching - Active sync mode detection (OnekeyId/Keyless/None) - Data source determination based on timestamps - Bidirectional encryption conversion between modes - Mode switch handling with automatic data migration This enables seamless switching between OneKey ID sync and Keyless wallet sync while preserving user data integrity.
Implement local mock storage and API methods for keyless cloud sync: - Add keylessMockApi with query, upload, and clear operations - Add mockApiDownloadItemsKeyless, mockApiUploadItemsKeyless methods - Add mockApiCheckServerStatusKeyless for status checking - Add getKeylessSyncAuth for authentication headers - Skip Lock/Reset/Flush operations in keyless mode - Fix null safety for convertServerItemToLocalItem return type
Allow cloud sync mode to proceed when enabled and register a new developer gallery route.
…ystem - Add keylessCredential to ICloudSyncCredential type for unified credential handling - Update cloudSyncItemBuilder to support keyless encryption/decryption - Modify getSyncCredentialWithCache to return keyless credential in Keyless mode - Update sync flow to properly handle keylessData and keylessDataTime fields - Fix LocalDbBase to persist keylessData and keylessDataTime on updates - Change IDBCloudSyncItem keyless fields from optional to explicit undefined type
- Disable swift-lsp plugin in settings - Update spell checker skip words with 'pwdhash' - Remove keylessData and keylessDataTime fields from LocalDbBase, RealmSchemaCloudSyncItem, and related types - Introduce pwdHash in relevant data structures for cloud sync
- Introduce a unified method for computing pwdHash based on sync mode. - Refactor cloud sync item handling to utilize the new pwdHash logic. - Update ServicePrimeCloudSync to streamline data conversion and encryption processes. - Remove deprecated keylessData and keylessDataTime fields, focusing on pwdHash for validation. - Enhance keyless credential management by integrating pwdHash computation into the sync workflow.
- Add keyless mock server build and start scripts to root package.json - Implement HTTP-based mock API with signature verification - Add onPressLoadingEnabled prop to Button component for async operations - Connect CloudSyncGallery to use loading-enabled buttons - Replace in-memory storage with mock server HTTP endpoints
- Create standalone keyless cloud sync mock server with HTTP API endpoints - Add build scripts and TypeScript config for mock server - Refactor keylessCloudSyncMockApi to use HTTP client instead of in-memory storage - Add environment variable support for mock server URL configuration - Support upload, download, check-status, and clear operations via HTTP - Add CLI entry point for starting mock server independently Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…g and unified pwdHash - Add comprehensive BIP32 key encryption documentation - Fix keyless credential derivation to use deterministic decrypted keys - Remove keyless wallet filtering from sync flow managers - Unify pwdHash retrieval from sync credential across all modes - Clean up duplicate API calls in mock server - Add debug tools and UI improvements for keyless sync testing - Remove unused import in Button component
…ation - Add a guideline to avoid using JSON.stringify() for cryptographic operations, promoting the use of stringUtils.stableStringify() for deterministic serialization. - Refactor keylessCloudSyncUtils to export functions directly, improving usability and clarity. - Update BIP32 key documentation to clarify encryption states and usage. - Clean up ServiceMasterPassword and CloudSyncItemBuilder by removing unnecessary checks and improving error handling. - Adjust keyless sync constants to reflect updated derivation paths for better clarity and consistency.
…ylessWallet - Comment out the createKeylessWallet function calls in both useKeylessWallet and FinalizeWalletSetupPage to prevent unintended execution during onboarding. - Add a new property `skipAddHDNextIndexedAccount` to IDBCreateHDWalletParams for improved wallet creation flexibility.
…ndling - Comment out the wait timer in getKeylessWallet for future removal. - Add checks for deviceId in context to enhance flow control. - Introduce skipAddHDNextIndexedAccount property to improve wallet creation parameters.
- Introduce timing logs for `isCloudSyncIsAvailable` and `getSyncCredentialSafe` methods to monitor performance. - Update `LocalDbBase` to handle existing sync items more robustly with optional chaining. - Add `IExistingSyncItemsInfo` type for improved type safety in sync item handling. - Implement a conditional check for `skipAddHDNextIndexedAccount` during indexed account addition to enhance wallet creation flexibility.
…and mock server enhancements - Add methods in LocalDbBase for retrieving keyless cloud sync credentials and wallets, improving credential management. - Update ServicePrimeCloudSync to delegate credential derivation to the database layer, enhancing modularity. - Introduce a keyless cloud sync mock server with signature verification and persistent storage for testing. - Enhance keyless cloud sync utilities with deterministic JSON serialization and improved error handling. - Update .gitignore to include mock server data files for better project hygiene.
…e dataHash requirement - Eliminate publicKey parameter from keyless cloud sync API methods, as it is now included in the signature header. - Update methods in KeylessCloudSyncMockApi and ServicePrimeCloudSync to reflect this change. - Make dataHash a required parameter in signature generation and verification to enhance security against tampering and replay attacks. - Improve documentation to emphasize the importance of dataHash in request handling.
- Introduce a comprehensive guide detailing the authentication methods for the cloud sync server, including OneKey ID and Keyless signature authentication. - Outline the priority of authentication methods, header formats, user identification processes, and signature verification steps. - Provide code examples for client implementation and emphasize the importance of data integrity and security in the authentication process.
…arameters - Update `ServicePrimeCloudSync` to include `pwdHash` in API responses and adjust related methods for better credential management. - Modify `CloudSyncFlowManagerBase` to ensure `tx` can be undefined, preventing premature transaction commits. - Refactor `keylessCloudSyncMockApi` to use a constant for the sync signature header, improving code clarity. - Enhance `LocalDbBase` methods to handle undefined transactions and improve sync item retrieval. - Update CORS headers in the mock server for broader compatibility. - Refactor `isKeylessPwdHash` function to handle undefined input gracefully.
… registration - Add a method to build the keyless signature header for client registration in `ServiceNotification`, enhancing the integration with keyless cloud sync. - Update the registration process to include the signature header in API requests when keyless sync is active. - Refactor `getKeylessSyncAuth` in `ServicePrimeCloudSync` to accept a generic postData parameter, improving flexibility in handling different data types. - Update `.gitignore` to exclude local markdown files for better project hygiene.
… operations - Add a debounced method `updateClientBasicAppInfoDebounced` in `ServiceNotification` to optimize client info updates. - Integrate calls to this method in `ServiceAccount` and `ServicePassword` to ensure timely updates when keyless operations occur. - Refactor `keylessCloudSyncMockApi` and `ServicePrimeCloudSync` to improve API response handling and streamline data management. - Update `ICloudSyncUploadPostData` type to include a nonce for enhanced data integrity.
- Deleted the KeylessForWeb.html and KeylessForWeb.md files, which contained the prototype implementation and documentation for the Keyless for Web project. This cleanup reflects a shift in focus away from the previous prototype structure.
- Introduced a new button in CloudSyncApiTests for flushing server data, improving testing capabilities. - Updated EnableOneKeyCloudSwitchListItem to include a keyless cloud sync switch, allowing users to enable or disable keyless sync. - Refactored ServicePrimeCloudSync to streamline cloud sync mode determination and improve handling of keyless credentials. - Enhanced keyless cloud sync API methods to accept dynamic URL paths for upload and flush operations, improving flexibility. - Added state management for keyless sync in prime atom, ensuring proper initialization and tracking of sync status.
- Simplified import statement in CloudSyncGallery for better readability. - Reformatted code in SwapProviderListPanel and SwapOldSwapBridgeLimitContainer to enhance consistency and maintainability, including adjustments to indentation and line breaks.
…support - Added `toggleCloudSync` and `toggleCloudSyncKeyless` methods in `ServicePrimeCloudSync` to simplify enabling/disabling cloud sync. - Updated `EnableOneKeyCloudSwitchListItem` to utilize the new toggle methods, improving code clarity and reducing redundancy. - Implemented automatic cloud sync enabling for keyless wallets in `autoEnableCloudSyncKeyless`, enhancing user experience.
- Updated `eslint-config-wesbos` to version 4.3.2 in `package.json` for enhanced linting capabilities. - Modified `yarn.lock` to reflect updated dependencies, including `@babel/eslint-parser` and various Babel plugins for improved compatibility and performance. - Cleaned up code formatting in multiple files, enhancing readability and maintainability, including adjustments in `LottieView`, `HomePageView`, and `ExportButton` components. - Refactored `ServicePrimeCloudSync` methods to improve clarity and consistency in handling cloud sync operations.
sidmorizon
force-pushed
the
feat/keyless-sync
branch
from
92f0b2c to
f375c2e
Compare
- Updated error handling in `ServicePrimeCloudSync` to conditionally enable or disable cloud sync based on the `enabled` and `forceEnable` flags during exceptions. - Ensured that cloud sync is disabled by default unless explicitly forced to enable, improving reliability in error scenarios.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
keylessCredentialtoICloudSyncCredentialtype for unified credential handlingcloudSyncItemBuilderto support keyless encryption/decryption based on credential typegetSyncCredentialWithCacheto return keyless credential when in Keyless sync modekeylessDataandkeylessDataTimefieldsLocalDbBaseto persist keyless data fields on updatesIDBCloudSyncItemkeyless fields from optional (?) to explicit| undefinedtypeTest plan
🤖 Generated with Claude Code