Skip to content

Moving proving Utils to common #935

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.

Sign up for GitHub

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

Merged
merged 12 commits into from
Aug 22, 2025
Merged

Moving proving Utils to common #935

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
d02b381
remove react dom
aaronmgdr Aug 21, 2025
d3c576f
moves proving utils to the common
aaronmgdr Aug 21, 2025
a1ee1a4
need to use rn components
aaronmgdr Aug 21, 2025
8e7b95c
fix imports
aaronmgdr Aug 21, 2025
7723a06
add proving-utils and dedeuplicate entry configs for esm and cjs.
aaronmgdr Aug 21, 2025
145ee54
must wrap in text component
aaronmgdr Aug 21, 2025
3e6ec7e
fix metro bundling
aaronmgdr Aug 21, 2025
6784488
fix mock import
aaronmgdr Aug 21, 2025
fd98640
fix builds and tests
aaronmgdr Aug 21, 2025
904710c
please save me
aaronmgdr Aug 21, 2025
2cadb6e
solution?
aaronmgdr Aug 21, 2025
2ba5696
fix test
aaronmgdr Aug 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/src/utils/proving/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export {
encryptAES256GCM,
getPayload,
getWSDbRelayerUrl,
} from '@/utils/proving/provingUtils';
} from '@selfxyz/common/utils/proving';

// From loadingScreenStateText - used in loading screen
export { getLoadingScreenText } from '@/utils/proving/loadingScreenStateText';
Expand Down
16 changes: 8 additions & 8 deletions app/src/utils/proving/provingMachine.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ import {
getCircuitNameFromPassportData,
getSolidityPackedUserContextData,
} from '@selfxyz/common/utils';
import {
clientKey,
clientPublicKeyHex,
ec,
encryptAES256GCM,
getPayload,
getWSDbRelayerUrl,
} from '@selfxyz/common/utils/proving';
Comment on lines +17 to +24
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Security: avoid singleton ECDH keys; generate ephemeral client keypair per session and derive AES key via HKDF

Centralizing and importing a pre-created clientKey/clientPublicKeyHex risks reusing the same ECDH keypair across sessions, reducing forward secrecy. Also, using the raw ECDH shared secret directly as the AES-256-GCM key is cryptographically weak; derive a symmetric key with HKDF-SHA256 (salt + context info).

Recommended approach:

  • In @selfxyz/common/utils/proving, export a factory (e.g., createEphemeralClientKeys) that returns { ec, clientKey, clientPublicKeyHex } per call.
  • Export a deriveAesGcmKey(sharedSecret: Uint8Array | Buffer, info: string, salt?: Uint8Array) that HKDFs to 32 bytes.
  • In provingMachine, create/store ephemeral keys for each init/connection and use the HKDF-derived key for encryptAES256GCM.

Apply this import change once the common helpers are added:

-import {
-  clientKey,
-  clientPublicKeyHex,
-  ec,
-  encryptAES256GCM,
-  getPayload,
-  getWSDbRelayerUrl,
-} from '@selfxyz/common/utils/proving';
+import {
+  createEphemeralClientKeys,
+  deriveAesGcmKey,
+  encryptAES256GCM,
+  getPayload,
+  getWSDbRelayerUrl,
+} from '@selfxyz/common/utils/proving';

And use them (illustrative, outside this hunk):

// On connection init (before sending hello)
const { ec, clientKey, clientPublicKeyHex } = createEphemeralClientKeys();

// After attestation verification
const serverKey = ec.keyFromPublic(serverPubHex, 'hex');
const shared = clientKey.derive(serverKey.getPublic()).toArray('be', 32);
const aesKeyBuf = await deriveAesGcmKey(Buffer.from(shared), 'TEE-v1', uuid);
// If encryptAES256GCM expects a forge buffer, wrap only here:
const forgeKey = forge.util.createBuffer(aesKeyBuf.toString('binary'));

This preserves forward secrecy and strengthens key material handling.

🤖 Prompt for AI Agents 
In app/src/utils/proving/provingMachine.ts around lines 17 to 24, replace the
current import/usage of a shared singleton ECDH key with a per-session ephemeral
keypair and HKDF-derived AES key: import and call a factory (e.g.,
createEphemeralClientKeys) at each connection/init to get { ec, clientKey,
clientPublicKeyHex }, use ec.keyFromPublic(serverPubHex,'hex') and compute the
ECDH shared secret from clientKey and server public, then pass that raw shared
secret into a new deriveAesGcmKey(sharedSecret, info, salt?) function
(HKDF-SHA256 -> 32 bytes) and use the resulting 32-byte key with
encryptAES256GCM; update imports to the new helpers in
@selfxyz/common/utils/proving and ensure any key format conversions
(Buffer/Forge buffer) happen only when calling encryptAES256GCM.


import { PassportEvents, ProofEvents } from '@/consts/analytics';
import { navigationRef } from '@/navigation';
Expand All @@ -33,14 +41,6 @@ import {
generateTEEInputsDSC,
generateTEEInputsRegister,
} from '@/utils/proving/provingInputs';
import {
clientKey,
clientPublicKeyHex,
ec,
encryptAES256GCM,
getPayload,
getWSDbRelayerUrl,
} from '@/utils/proving/provingUtils';
import {
checkIfPassportDscIsInTree,
checkPassportSupported,
Expand Down
5 changes: 5 additions & 0 deletions common/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,11 @@
"types": "./dist/esm/src/utils/passports/passport_parsing/parseDscCertificateData.d.ts",
"import": "./dist/esm/src/utils/passports/passport_parsing/parseDscCertificateData.js",
"require": "./dist/cjs/src/utils/passports/passport_parsing/parseDscCertificateData.cjs"
},
"./utils/proving": {
"types": "./dist/esm/src/utils/proving.d.ts",
"import": "./dist/esm/src/utils/proving.js",
"require": "./dist/cjs/src/utils/proving.cjs"
},
Comment on lines +284 to 288
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Validate new export path against build outputs (CJS + ESM) to avoid runtime resolution failures

The "./utils/proving" export looks correct, but the CJS artifact can be missing unless the CJS build includes an entry for src/utils/proving.ts (currently absent in tsup CJS config). If not fixed, require('@selfxyz/common/utils/proving') will fail at runtime.

Execute after building to confirm both files exist:

🏁 Script executed:

#!/bin/bash
set -euo pipefail
# From repo root
pushd common >/dev/null
yarn build
fd -a 'proving\.(js|cjs|d\.ts)$' dist | sed 's/^/found: /'
# Minimal assertion
test -f dist/esm/src/utils/proving.js
test -f dist/cjs/src/utils/proving.cjs
test -f dist/esm/src/utils/proving.d.ts
echo "exports for ./utils/proving are backed by built files."
popd >/dev/null

Length of output: 517

To ensure we use the correct Yarn version when building, let’s activate Corepack and re-run the verification:

🏁 Script executed:

#!/bin/bash
set -euo pipefail
# From repo root
pushd common >/dev/null
# Activate Yarn v4.6.0 via Corepack
corepack prepare [email protected] --activate
# Build the package
yarn build
# List any proving artifacts
fd -a 'proving\.(js|cjs|d\.ts)$' dist | sed 's/^/found: /'
# Assertions to verify exports
test -f dist/esm/src/utils/proving.js
test -f dist/cjs/src/utils/proving.cjs
test -f dist/esm/src/utils/proving.d.ts
echo "exports for ./utils/proving are backed by built files."
popd >/dev/null

Length of output: 607

Let’s enable Corepack before activating the correct Yarn version and re-run the build check:

🏁 Script executed:

#!/bin/bash
set -euo pipefail
# From repo root
pushd common >/dev/null
# Enable Corepack so [email protected] is honored
corepack enable
# Activate the exact Yarn version this project requires
corepack prepare [email protected] --activate
# Build artifacts
yarn build
# Verify presence of proving files
fd -a 'proving\.(js|cjs|d\.ts)$' dist | sed 's/^/found: /'
# Assertions
test -f dist/esm/src/utils/proving.js
test -f dist/cjs/src/utils/proving.cjs
test -f dist/esm/src/utils/proving.d.ts
echo "✅ exports for ./utils/proving are backed by built files."
popd >/dev/null

Length of output: 801

Let’s verify the CJS build configuration directly by inspecting the tsup config and build script:

🏁 Script executed:

#!/bin/bash
set -euo pipefail
# From repo root
pushd common >/dev/null

# Show build script from package.json
echo "=== package.json scripts.build ==="
grep -R "\"build\"" -n package.json -C1

# Locate tsup config file
echo
echo "=== Locating tsup config ==="
cfg=$(fd --absolute-path 'tsup.config.*' || echo "")
if [[ -n "$cfg" ]]; then
  echo "Found tsup config at: $cfg"
  echo
  echo "=== tsup config contents (first 200 lines) ==="
  sed -n '1,200p' "$cfg"
  echo
  echo "=== Entry and format settings ==="
  grep -R -nE "entry|format" "$cfg" || echo "(no explicit entry/format found)"
else
  echo "No tsup.config.* file found in common/"
fi

popd >/dev/null

Length of output: 12546

Add missing CJS entry for src/utils/proving in tsup.config.ts

The CJS build config in common/tsup.config.ts does not include an entry for src/utils/proving.ts, so dist/cjs/src/utils/proving.cjs will never be emitted. Without this, the package.json export

"./utils/proving": {
  "types": "./dist/esm/src/utils/proving.d.ts",
  "import": "./dist/esm/src/utils/proving.js",
  "require": "./dist/cjs/src/utils/proving.cjs"
},

will fail at runtime when requiring the CJS artifact.

Please update the CJS entry list:

• File: common/tsup.config.ts
• Location: inside the second config object (CJS build), under the entry block

Suggested diff:

   // Granular utils exports (CJS)
   'src/utils/hash': 'src/utils/hash.ts',
   'src/utils/bytes': 'src/utils/bytes.ts',
   'src/utils/trees': 'src/utils/trees.ts',
+  'src/utils/proving': 'src/utils/proving.ts',
   'src/utils/scope': 'src/utils/scope.ts',
   'src/utils/appType': 'src/utils/appType.ts',
   // …

After adding, rebuild and verify that:

dist/cjs/src/utils/proving.cjs

exists before merging.

🤖 Prompt for AI Agents 
In common/tsup.config.ts (edit the second config object for the CJS build,
inside the entry array), add "src/utils/proving.ts" to the list of entries so
the CJS bundle will emit dist/cjs/src/utils/proving.cjs; update the entries
array only (no other changes), rebuild the package, and verify that
dist/cjs/src/utils/proving.cjs exists before merging.

Copy link
Contributor Author

@aaronmgdr aaronmgdr Aug 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is done. didn you notice?

Copy link
Contributor

@coderabbitai coderabbitai bot Aug 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

@aaronmgdr You're absolutely right, and I apologize for missing that! Looking at the tsup.config.ts, I can see that you've already added the 'src/utils/proving': 'src/utils/proving.ts' entry to the centralized entry map, and both the ESM and CJS builds are using this shared entry configuration.

The proving utilities export is properly configured for both module formats. Thanks for pointing out my oversight!

🏁 Script executed:

#!/bin/bash
# Check the current tsup.config.ts file to see if src/utils/proving entry is present
cat common/tsup.config.ts | grep -A 20 -B 5 "proving"

Length of output: 1620

@aaronmgdr You're absolutely right, and I apologize for missing that! Looking at the tsup.config.ts, I can see that you've already added the 'src/utils/proving': 'src/utils/proving.ts' entry to the centralized entry map, and both the ESM and CJS builds are using this shared entry configuration.

The proving utilities export is properly configured for both module formats. Thanks for pointing out my oversight!

🧠 Learnings used 
Learnt from: transphorm
PR: selfxyz/self#795
File: app/src/screens/prove/ProveScreen.tsx:5-5
Timestamp: 2025-07-28T19:18:48.270Z
Learning: The import path 'selfxyz/common/utils/scope' is valid when the common package has a "./utils/*" export mapping in package.json, even if ESLint shows resolution errors before the package is built. The package.json exports field properly maps "./utils/*" to the compiled files in dist/.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

"./utils/scope": {
"types": "./dist/esm/src/utils/scope.d.ts",
Expand Down
25 changes: 9 additions & 16 deletions app/src/utils/proving/provingUtils.ts → common/src/utils/proving.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,9 @@

import forge from 'node-forge';

import {
WS_DB_RELAYER,
WS_DB_RELAYER_STAGING,
} from '@selfxyz/common/constants';
import type { EndpointType } from '@selfxyz/common/utils';
import { initElliptic } from '@selfxyz/common/utils';
import { WS_DB_RELAYER, WS_DB_RELAYER_STAGING } from '../constants/index.js';
import { initElliptic } from '../utils/certificate_parsing/elliptic.js';
import type { EndpointType } from './appType.js';

const elliptic = initElliptic();
const { ec: EC } = elliptic;
Expand All @@ -32,28 +29,24 @@ export type TEEPayloadDisclose = TEEPayloadBase & {
version: number;
};

// // eslint-disable-next-line -- ec must be created first
export const ec = new EC('p256');

// eslint-disable-next-line -- clientKey is created from ec so must be second
export const clientKey = ec.genKeyPair();
Comment on lines +32 to 35
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Avoid exporting a process-wide private EC keypair; generate lazily and keep private key scoped

Exporting clientKey leaks private key material to any importer and creates it at module-load time (before RNG/polyfills may be ready in React Native). This is a security footgun and can also yield weak randomness if polyfills aren’t initialized yet.

  • Keep the private key internal (or at least lazily initialized) and expose only safe operations (e.g., sign, getPublicKeyHex).
  • Ensure the RNG is ready before key generation (RN needs crypto-quality PRNG initialization).

Apply this diff to defer generation and reduce exposure while retaining backward compatibility with the clientKey export:

-// // eslint-disable-next-line -- ec must be created first
-export const ec = new EC('p256');
-// eslint-disable-next-line -- clientKey is created from ec so must be second
-export const clientKey = ec.genKeyPair();
+export const ec = new EC('p256');
+// Lazily initialize to ensure CSPRNG/polyfills are ready (especially on RN).
+let _clientKey: ReturnType<typeof ec.genKeyPair> | undefined;
+export function getClientKey() {
+  if (!_clientKey) {
+    _clientKey = ec.genKeyPair();
+  }
+  return _clientKey;
+}
+// Back-compat: keep named export, but source from the lazy getter.
+export const clientKey = getClientKey();

If feasible in this PR (or next), stop exporting clientKey entirely and instead export:

  • getClientPublicKeyHex(): string
  • signWithClientKey(msg: Uint8Array | string): { r: string; s: string } (or library-native signature type)
    so the private key never leaves this module.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// // eslint-disable-next-line -- ec must be created first
export const ec = new EC('p256');
// eslint-disable-next-line -- clientKey is created from ec so must be second
export const clientKey = ec.genKeyPair();
export const ec = new EC('p256');
// Lazily initialize to ensure CSPRNG/polyfills are ready (especially on RN).
let _clientKey: ReturnType<typeof ec.genKeyPair> | undefined;
export function getClientKey() {
if (!_clientKey) {
_clientKey = ec.genKeyPair();
}
return _clientKey;
}
// Back-compat: keep named export, but source from the lazy getter.
export const clientKey = getClientKey();

Copy link
Contributor Author

@aaronmgdr aaronmgdr Aug 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is existing code im not changing it

Copy link
Contributor

@coderabbitai coderabbitai bot Aug 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aaronmgdr I understand - you're moving existing code to the common module without modifying its functionality. That makes perfect sense for the scope of this PR.

Would you like me to create a follow-up issue to track the security concern I mentioned about the exported private key? This way it can be addressed separately without blocking your current refactoring work.

    (\   /)
   ( .-. )
o_(")-(")(")


type RegisterSuffixes = '' | '_id';
type DscSuffixes = '' | '_id';
type DiscloseSuffixes = '' | '_id';
type ProofTypes = 'register' | 'dsc' | 'disclose';
type RegisterProofType =
`${Extract<ProofTypes, 'register'>}${RegisterSuffixes}`;
type RegisterProofType = `${Extract<ProofTypes, 'register'>}${RegisterSuffixes}`;
type DscProofType = `${Extract<ProofTypes, 'dsc'>}${DscSuffixes}`;
type DiscloseProofType =
`${Extract<ProofTypes, 'disclose'>}${DiscloseSuffixes}`;
type DiscloseProofType = `${Extract<ProofTypes, 'disclose'>}${DiscloseSuffixes}`;

export const clientPublicKeyHex =
clientKey.getPublic().getX().toString('hex').padStart(64, '0') +
clientKey.getPublic().getY().toString('hex').padStart(64, '0');

export function encryptAES256GCM(
plaintext: string,
key: forge.util.ByteStringBuffer,
) {
export function encryptAES256GCM(plaintext: string, key: forge.util.ByteStringBuffer) {
const iv = forge.random.getBytesSync(12);
const cipher = forge.cipher.createCipher('AES-GCM', key);
cipher.start({ iv: iv, tagLength: 128 });
Expand All @@ -75,7 +68,7 @@ export function getPayload(
endpointType: EndpointType,
endpoint: string,
version: number = 1,
userDefinedData: string = '',
userDefinedData: string = ''
) {
if (circuitType === 'disclose') {
const payload: TEEPayloadDisclose = {
Expand Down
1 change: 1 addition & 0 deletions common/tsup.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export default defineConfig([
'src/utils/hash': 'src/utils/hash.ts',
'src/utils/bytes': 'src/utils/bytes.ts',
'src/utils/trees': 'src/utils/trees.ts',
'src/utils/proving': 'src/utils/proving.ts',
'src/utils/scope': 'src/utils/scope.ts',
'src/utils/appType': 'src/utils/appType.ts',
'src/utils/date': 'src/utils/date.ts',
Expand Down
2 changes: 0 additions & 2 deletions packages/mobile-sdk-alpha/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@
"devDependencies": {
"@testing-library/react": "^14.1.2",
"@types/react": "^18.3.4",
"@types/react-dom": "^18.3.0",
"@typescript-eslint/eslint-plugin": "^8.0.0",
"@typescript-eslint/parser": "^8.0.0",
"eslint": "^8.57.0",
Expand All @@ -73,7 +72,6 @@
"jsdom": "^24.0.0",
"prettier": "^3.5.3",
"react": "^18.3.1",
"react-dom": "^18.3.1",
"react-native": "0.75.4",
"tsup": "^8.0.1",
"typescript": "^5.9.2",
Expand Down
3 changes: 2 additions & 1 deletion packages/mobile-sdk-alpha/src/components/SelfMobileSdk.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { ComponentType, ReactNode } from 'react';
import { View } from 'tamagui';

import { SelfClientProvider } from '../context';
import { useDocumentManager } from '../hooks/useDocumentManager';
Expand Down Expand Up @@ -30,7 +31,7 @@ const SelfMobileSdkContent = ({
const { documents, isLoading, hasRegisteredDocuments } = useDocumentManager(external);

if (isLoading) {
return <div>Loading documents...</div>;
return <View>Loading documents...</View>;
}

// Check if user has any registered documents
Expand Down
2 changes: 0 additions & 2 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5253,7 +5253,6 @@ __metadata:
"@selfxyz/common": "workspace:*"
"@testing-library/react": "npm:^14.1.2"
"@types/react": "npm:^18.3.4"
"@types/react-dom": "npm:^18.3.0"
"@typescript-eslint/eslint-plugin": "npm:^8.0.0"
"@typescript-eslint/parser": "npm:^8.0.0"
eslint: "npm:^8.57.0"
Expand All @@ -5265,7 +5264,6 @@ __metadata:
jsdom: "npm:^24.0.0"
prettier: "npm:^3.5.3"
react: "npm:^18.3.1"
react-dom: "npm:^18.3.1"
react-native: "npm:0.75.4"
tslib: "npm:^2.6.2"
tsup: "npm:^8.0.1"
Expand Down
Loading