feat: support Expo Go with software-backed device signer fallback [WAL-9469]#1728
feat: support Expo Go with software-backed device signer fallback [WAL-9469]#1728albertoelias-crossmint wants to merge 5 commits intomainfrom
Conversation
…L-9469] Co-Authored-By: Alberto Elias <alberto.elias@paella.dev>
…clarify prehash behavior Co-Authored-By: Alberto Elias <alberto.elias@paella.dev>
Original prompt from Alberto Elias
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
🦋 Changeset detectedLatest commit: 3f19313 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Prompt To Fix All With AIThis is a comment left during a code review.
Path: packages/client/device-signer-expo/src/createDeviceSignerKeyStorage.ts
Line: 18-20
Comment:
**Silent fallback to software-backed keys**
When the native module is unavailable, this factory silently falls back to the software-backed implementation. Since this is a security-relevant decision (software-backed keys lack hardware-backed protections like Secure Enclave / Android Keystore), a `console.warn` would help developers notice when they are running with the software fallback — especially if a production build accidentally ships without the native module.
```suggestion
console.warn(
"[CrossmintDeviceSigner] Native module not available — using software-backed key storage. " +
"This is suitable for development in Expo Go but not recommended for production."
);
const { SoftwareDeviceSignerKeyStorage } = require("./SoftwareDeviceSignerKeyStorage");
return new SoftwareDeviceSignerKeyStorage();
```
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: "fix: address PR revi..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| const { SoftwareDeviceSignerKeyStorage } = require("./SoftwareDeviceSignerKeyStorage"); | ||
| return new SoftwareDeviceSignerKeyStorage(); |
There was a problem hiding this comment.
When the native module is unavailable, this factory silently falls back to the software-backed implementation. Since this is a security-relevant decision (software-backed keys lack hardware-backed protections like Secure Enclave / Android Keystore), a console.warn would help developers notice when they are running with the software fallback — especially if a production build accidentally ships without the native module.
| const { SoftwareDeviceSignerKeyStorage } = require("./SoftwareDeviceSignerKeyStorage"); | |
| return new SoftwareDeviceSignerKeyStorage(); | |
| console.warn( | |
| "[CrossmintDeviceSigner] Native module not available — using software-backed key storage. " + | |
| "This is suitable for development in Expo Go but not recommended for production." | |
| ); | |
| const { SoftwareDeviceSignerKeyStorage } = require("./SoftwareDeviceSignerKeyStorage"); | |
| return new SoftwareDeviceSignerKeyStorage(); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/client/device-signer-expo/src/createDeviceSignerKeyStorage.ts
Line: 18-20
Comment:
**Silent fallback to software-backed keys**
When the native module is unavailable, this factory silently falls back to the software-backed implementation. Since this is a security-relevant decision (software-backed keys lack hardware-backed protections like Secure Enclave / Android Keystore), a `console.warn` would help developers notice when they are running with the software fallback — especially if a production build accidentally ships without the native module.
```suggestion
console.warn(
"[CrossmintDeviceSigner] Native module not available — using software-backed key storage. " +
"This is suitable for development in Expo Go but not recommended for production."
);
const { SoftwareDeviceSignerKeyStorage } = require("./SoftwareDeviceSignerKeyStorage");
return new SoftwareDeviceSignerKeyStorage();
```
How can I resolve this? If you propose a fix, please make it concise.
Co-Authored-By: Alberto Elias <alberto.elias@paella.dev>
Prompt To Fix All With AIThis is a comment left during a code review.
Path: packages/client/device-signer-expo/src/SoftwareDeviceSignerKeyStorage.ts
Line: 222-228
Comment:
**Race condition in public key index read-modify-write**
`trackPublicKey` (and `untrackPublicKey`) performs a read-modify-write on the shared `PUBLIC_KEY_INDEX_KEY` SecureStore entry without any serialization. If two concurrent `generateKey` calls overlap, both will read the same index, each appends its key, and the second `savePublicKeyIndex` call will overwrite the first — losing a key from the index.
While concurrent `generateKey` calls may be unlikely in practice, this could be guarded with a simple in-memory mutex/lock or by serializing writes through a queue.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: packages/client/device-signer-expo/src/SoftwareDeviceSignerKeyStorage.ts
Line: 123
Comment:
**`generateKey` silently ignores biometric parameters**
The base class `DeviceSignerKeyStorage` defines `generateKey` with overloads accepting `biometricPolicy` and `biometricExpirationTime` parameters, but this implementation only destructures `address` and silently discards the rest. While the software fallback can't enforce biometric policies, callers have no indication their security requirement was dropped.
Consider adding a `console.warn` when `biometricPolicy` is passed with a value other than `"none"` so developers are aware the software fallback cannot enforce it.
How can I resolve this? If you propose a fix, please make it concise.Reviews (2): Last reviewed commit: "fix: add 0x prefix to signature r/s in S..." | Re-trigger Greptile |
| private async trackPublicKey(publicKeyBase64: string): Promise<void> { | ||
| const index = await this.getPublicKeyIndex(); | ||
| if (!index.includes(publicKeyBase64)) { | ||
| index.push(publicKeyBase64); | ||
| await this.savePublicKeyIndex(index); | ||
| } | ||
| } |
There was a problem hiding this comment.
trackPublicKey (and untrackPublicKey) performs a read-modify-write on the shared PUBLIC_KEY_INDEX_KEY SecureStore entry without any serialization. If two concurrent generateKey calls overlap, both will read the same index, each appends its key, and the second savePublicKeyIndex call will overwrite the first — losing a key from the index.
While concurrent generateKey calls may be unlikely in practice, this could be guarded with a simple in-memory mutex/lock or by serializing writes through a queue.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/client/device-signer-expo/src/SoftwareDeviceSignerKeyStorage.ts
Line: 222-228
Comment:
**Race condition in public key index read-modify-write**
`trackPublicKey` (and `untrackPublicKey`) performs a read-modify-write on the shared `PUBLIC_KEY_INDEX_KEY` SecureStore entry without any serialization. If two concurrent `generateKey` calls overlap, both will read the same index, each appends its key, and the second `savePublicKeyIndex` call will overwrite the first — losing a key from the index.
While concurrent `generateKey` calls may be unlikely in practice, this could be guarded with a simple in-memory mutex/lock or by serializing writes through a queue.
How can I resolve this? If you propose a fix, please make it concise.| super(""); | ||
| } | ||
|
|
||
| async generateKey(params: { address?: string }): Promise<string> { |
There was a problem hiding this comment.
generateKey silently ignores biometric parameters
The base class DeviceSignerKeyStorage defines generateKey with overloads accepting biometricPolicy and biometricExpirationTime parameters, but this implementation only destructures address and silently discards the rest. While the software fallback can't enforce biometric policies, callers have no indication their security requirement was dropped.
Consider adding a console.warn when biometricPolicy is passed with a value other than "none" so developers are aware the software fallback cannot enforce it.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/client/device-signer-expo/src/SoftwareDeviceSignerKeyStorage.ts
Line: 123
Comment:
**`generateKey` silently ignores biometric parameters**
The base class `DeviceSignerKeyStorage` defines `generateKey` with overloads accepting `biometricPolicy` and `biometricExpirationTime` parameters, but this implementation only destructures `address` and silently discards the rest. While the software fallback can't enforce biometric policies, callers have no indication their security requirement was dropped.
Consider adding a `console.warn` when `biometricPolicy` is passed with a value other than `"none"` so developers are aware the software fallback cannot enforce it.
How can I resolve this? If you propose a fix, please make it concise.![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟡 isSignerApproved not updated to recognize new "active" status as approved
The PR adds "active" to the SignerStatus type (packages/wallets/src/wallets/types.ts:115) and introduces isApprovedDeviceSignerStatus in recover() (packages/wallets/src/wallets/wallet.ts:915-916) that correctly treats both "success" and "active" as approved states. However, the public isSignerApproved() method at line 869 still only checks status === "success", meaning it will return false for signers with "active" status even though they are approved. This creates an inconsistency: the internal recovery flow considers "active" approved, but the public API does not.
(Refers to line 869)
Was this helpful? React with 👍 or 👎 to provide feedback.
🔥 Smoke Test Results✅ Status: Passed Statistics
✅ All smoke tests passed!All critical flows are working correctly. This is a non-blocking smoke test. Full regression tests run separately. |
Prompt To Fix All With AIThis is a comment left during a code review.
Path: packages/client/device-signer-expo/src/SoftwareDeviceSignerKeyStorage.ts
Line: 200-204
Comment:
**Clear raw key bytes after use**
In `signMessage`, the raw bytes from `hexToBytes(...)` remain in memory after signing. Similarly in `generateKey` (line 149–152), the raw bytes are not zeroed after being converted to hex. While JavaScript doesn't guarantee immediate garbage collection, zeroing `Uint8Array` buffers after use is standard practice in cryptographic code to reduce the window of exposure — consider adding `fill(0)` on the byte arrays after they're no longer needed.
For example after the sign call on line 204, zero out the bytes. Same for `generateKey` after line 152.
How can I resolve this? If you propose a fix, please make it concise.Reviews (3): Last reviewed commit: "Fixes software device signer key managem..." | Re-trigger Greptile |
| const privateKey = hexToBytes(privateKeyHex); | ||
| const messageBytes = base64ToBytes(message); | ||
| // Match the native implementations, which sign the decoded message bytes using the | ||
| // platform P-256 primitives. Those primitives apply SHA-256 before ECDSA signing. | ||
| const signature = p256.sign(messageBytes, privateKey, { lowS: true, prehash: true }); |
There was a problem hiding this comment.
In signMessage, the raw bytes from hexToBytes(...) remain in memory after signing. Similarly in generateKey (line 149–152), the raw bytes are not zeroed after being converted to hex. While JavaScript doesn't guarantee immediate garbage collection, zeroing Uint8Array buffers after use is standard practice in cryptographic code to reduce the window of exposure — consider adding fill(0) on the byte arrays after they're no longer needed.
For example after the sign call on line 204, zero out the bytes. Same for generateKey after line 152.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/client/device-signer-expo/src/SoftwareDeviceSignerKeyStorage.ts
Line: 200-204
Comment:
**Clear raw key bytes after use**
In `signMessage`, the raw bytes from `hexToBytes(...)` remain in memory after signing. Similarly in `generateKey` (line 149–152), the raw bytes are not zeroed after being converted to hex. While JavaScript doesn't guarantee immediate garbage collection, zeroing `Uint8Array` buffers after use is standard practice in cryptographic code to reduce the window of exposure — consider adding `fill(0)` on the byte arrays after they're no longer needed.
For example after the sign call on line 204, zero out the bytes. Same for `generateKey` after line 152.
How can I resolve this? If you propose a fix, please make it concise.2 participants
Description
CrossmintWalletProvidercrashes in Expo Go because it eagerly instantiatesNativeDeviceSignerKeyStorage, which callsrequireNativeModule("CrossmintDeviceSigner")— a custom native module unavailable in Expo Go.This PR adds a software-backed fallback so the SDK works in Expo Go without a dev build:
SoftwareDeviceSignerKeyStorage: Pure JS implementation ofDeviceSignerKeyStorageusing@noble/curves/p256for key generation/signing andexpo-secure-storefor encrypted persistence. Same interface as the native version but without hardware-backed security.createDeviceSignerKeyStorage(): Factory that probes for the native module and auto-falls back to the software implementation.CrossmintWalletProvider: Now uses the factory instead of directly constructingNativeDeviceSignerKeyStorage.NativeDeviceSignerKeyStorage: Improved error message when native module is missing.SoftwareDeviceSignerKeyStorage.signMessage()formatsrandsas0x-prefixed hex strings (e.g.0x00ab...), matching the format used byIframeDeviceSignerKeyStorageand required by the backend'sPositiveBigIntStringschema. Without the0xprefix, the backend rejects the signature but surfaces a misleading error from a different union branch ("Invalid type: device"from the passkey schema instead of the actual BigInt validation error).Items requiring human verification
signMessagesigns raw bytes withprehash: false, assuming the message is already a hash digest (perwallet.tscomment about keccak256). Verify this matches the nativeCrossmintDeviceSignerSwift/Kotlin package behavior. If the native side hashes internally (e.g.ecdsaSignatureMessageX962SHA256), the software fallback needsprehash: trueto produce compatible signatures.expo-modules-corestatic import:NativeDeviceSignerKeyStorage.tsstill has a top-levelimport { requireNativeModule } from "expo-modules-core". The factory uses dynamicrequire()to avoid this, but ifNativeDeviceSignerKeyStorageis ever imported directly in Expo Go, it would fail at the import level before reaching the try/catch. Confirm this is acceptable givenexpo-modules-coreis always present in Expo projects.SoftwareDeviceSignerKeyStoragehas no unit tests. Should be tested manually in Expo Go.hasKey()stores all public keys as a JSON array in one SecureStore entry — may approach the 2KB value limit with many keys.Test plan
pnpm build:libspasses)pnpm lintpasses)pnpm test:vitest)Package updates
@crossmint/expo-device-signer: minor — newSoftwareDeviceSignerKeyStorage,createDeviceSignerKeyStorage,isNativeModuleAvailableexports; added@noble/curvesdep;expo-secure-store+expo-modules-coreas optional peer deps@crossmint/client-sdk-react-native-ui: patch — uses factory instead of directNativeDeviceSignerKeyStorage.changeset/expo-go-device-signer-fallback.mdLink to Devin session: https://crossmint.devinenterprise.com/sessions/d7ddc53fd2ed434989652db7b8824c21
Requested by: @albertoelias-crossmint