Conversation
* merge dev branch into main (#624) * remove sdk/tests (#622) * remove sdk/tests * chore: update yarn.lock --------- Co-authored-by: Ayman <[email protected]> * fix: add range check on paddedInLength of shaBytesDynamic (#623) * fix ci (#626) --------- Co-authored-by: Ayman <[email protected]> Co-authored-by: Vishalkulkarni45 <[email protected]> * update contracts (#628) * remove sdk/tests (#622) * remove sdk/tests * chore: update yarn.lock --------- Co-authored-by: Ayman <[email protected]> * fix: add range check on paddedInLength of shaBytesDynamic (#623) * fix ci (#626) * implement self uups upgradeable (#592) * implement self uups upgradeable * small changes in identityVerificationHubImplV2 * delete aderyn.toml * chore: add custom verifier * chnage return output * feat: use self structs and a Generic output struct * feat: add userIdentifier, nullifier, forbiddencountries to returned output * add root view functions from registry * fix: build and compilation errors * add userDefined data into selfVerificationRoot * "resolve conflicts" * fix compilation problem * fix how to register verification config * test: CustomVerifier * fix verification root and hub integration * add scope check in hub impl * replace poseidon hash to ripemd+sha256 * add todo list * feat: refactor and add test cases for generic formatter * add performUserIdentifierCheck in basicVerification * change how to handle additionalData and fix stack too deep * start adding test codes * fix dependency problems in monorepo * fix: forbidden countries (#612) LGTM! * able to run test code * pass happy path * delete unused codes * change error code name, add caller address validation and add scripts to run test and build in monorepo * add all test cases in vcAndDisclose flow * remove comment out * chore: use actual user identifier outputs * success in registration tests * cover all cases * pass contractVersion instead of circuitVersion * fix disclose test * chore: add natspecs for ImplHubV2, CustomVerifier and GenericFormatter * change val name and remove unused lines * add val name change * remove userIdentifier from return data * feat: use GenericDiscloseOutput struct in verfication hook fix test cases for user identifier * chore: change the function order for Hub Impl V2 (#625) * fix nat specs * add nat spec in SelfStructs --------- Co-authored-by: Ayman <[email protected]> Co-authored-by: Nesopie <[email protected]> * prettier (#629) --------- Co-authored-by: Ayman <[email protected]> Co-authored-by: Vishalkulkarni45 <[email protected]> Co-authored-by: nicoshark <[email protected]> Co-authored-by: Nesopie <[email protected]> * fix: vc_and_disclose_id test (#640) * fix: vc_and_disclose_id test * chore: yarn prettier * fix: check if a config id exists * chore: change the function where the config not set verification is happening * fix: add await * feat: add getConfigId function in SelfVerificationRoot (#650) * feat: add getConfigId function in SelfVerificationRoot * update comment --------- Co-authored-by: motemotech <[email protected]> * chore: fix ofac end index in eu id cards * chore: fix tests * fix: example contracts and tests --------- Co-authored-by: turnoffthiscomputer <[email protected]> Co-authored-by: Vishalkulkarni45 <[email protected]> Co-authored-by: nicoshark <[email protected]>
…ed documentation and library linkage for CustomVerifier. Update initialization process to reflect changes in V2 implementation, ensuring proper setup for proxy deployment. (#658)
* fix build issues * generate disclosure proof with euids * generate disclosure proof with euids * Eu id updates 2 (#648) * update vc_and_disclose_id test (dev branch) (#641) * fix: vc_and_disclose_id test * chore: yarn prettier * Show modal on NFC scan error (#642) * Add help button and error modal actions * fix the screen management * yarn nice * Bump build v2.5.4: ios 132; android 71 (#631) * bump version and build numbers * remove tamagui/toast * fix marketing version * fix: update TD1 and TD3 checks (#643) * bum yarn.lock * add version and user defined data --------- Co-authored-by: Vishalkulkarni45 <[email protected]> Co-authored-by: Justin Hernandez <[email protected]> Co-authored-by: Seshanth.S🐺 <[email protected]> * remove the mock user define data * get the useridentifier as a hash from the user defined data * chore: add version and userDefinedData * feat: use the version in register / dsc proofs as well * update calculateUserIdentifierHash * yarn nice * refactor: consolidate user context data handling and update payload structure * fix typing issues on sha1 * remove console.log(sha1) * fix sha1 import * refactor: streamline userDefinedData handling and adjust payload type for circuit * refactor: update sha1 usage and enhance logging in calculateUserIdentifierHash * yarn nice * yarn lint common * use ts-ignore for sha1 import * fix app ci tests * fix typing issue * remove unused ts-ignore * cast uuid before calling generateinputs * bump qrcode version * add tsup on the qrcode sdk * fix: exports on selfxyz/qrcode * update how we define config.version * fix yarn imports * yarn format --------- Co-authored-by: Vishalkulkarni45 <[email protected]> Co-authored-by: Justin Hernandez <[email protected]> Co-authored-by: Seshanth.S🐺 <[email protected]> Co-authored-by: Ayman <[email protected]>
* Fix previous rebase error * Refactor deployment module for Identity Verification Hub V2.
* fix: sdk build configs * chore: SelfBackendVerifier (WIP) * feat: add custom verification * feat: consider destination chain in user defined data * chore: export attestation id * chore: export attestation id * chore: export config storage * chore: don't throw an error if the proof is not valid * chore: trim abi and rm typechain types * refactor * chore: rm unnecessary exports * 📝 Add docstrings to `fix/sdk` (#653) Docstrings generation was requested by @remicolin. * #652 (comment) The following files were modified: * `sdk/core/src/utils/hash.ts` * `sdk/core/src/utils/proof.ts` * `sdk/core/src/utils/utils.ts` Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * review fixes * chore: fix package.json cjs types * chore: add minor changes to checks * feat: add InMemoryConfigStore, allIds constant and verificationResult type * chore: export Verification config * feat: change the verification config types * fix: throw issues early if verification config is null * fix: update yarn.lock file * chore: lint * fix: rm ts expect error directive * fix: contract tests * use excluded countries instead forbidden countries list * chore: change types in constnats --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* Update deployment module for Identity Verification Hub V2 to correct file paths and module name for deployment commands. * Add troubleshooting documentation for verification issues in deployHubV2.ts. Include manual verification steps and common failure reasons to assist users during deployment. * Change visibility of getVerificationConfigV2 function from internal to public in IdentityVerificationHubImplV2 contract to allow external access.
* Add BSL license headers to app sources * prettier * fix license reference - https://spdx.org/licenses/BUSL-1.1.html
* update scripts * deploy vc and disclose id * fix the deployment scripts on staging * update yarn.lock
* tweak coderabbit * bump * more thorough test spec
* Clean up root license wording * Simplify SPDX header * simplify license and rename BSL to BUSL * fix merge issues * fix missing method --------- Co-authored-by: Justin Hernandez <[email protected]>
* apply recommended app settings from xcode * stick to portrait orientation and update target settings * remove app clip references
* feat: add range checks before use of LessEqThan and SelectSubArray * fix: Num2Bits_strict to constrain virtualKey
* remove sdk/tests (#622) * remove sdk/tests * chore: update yarn.lock --------- Co-authored-by: Ayman <[email protected]> * fix: add range check on paddedInLength of shaBytesDynamic (#623) * fix ci (#626) * implement self uups upgradeable (#592) * implement self uups upgradeable * small changes in identityVerificationHubImplV2 * delete aderyn.toml * chore: add custom verifier * chnage return output * feat: use self structs and a Generic output struct * feat: add userIdentifier, nullifier, forbiddencountries to returned output * add root view functions from registry * fix: build and compilation errors * add userDefined data into selfVerificationRoot * "resolve conflicts" * fix compilation problem * fix how to register verification config * test: CustomVerifier * fix verification root and hub integration * add scope check in hub impl * replace poseidon hash to ripemd+sha256 * add todo list * feat: refactor and add test cases for generic formatter * add performUserIdentifierCheck in basicVerification * change how to handle additionalData and fix stack too deep * start adding test codes * fix dependency problems in monorepo * fix: forbidden countries (#612) LGTM! * able to run test code * pass happy path * delete unused codes * change error code name, add caller address validation and add scripts to run test and build in monorepo * add all test cases in vcAndDisclose flow * remove comment out * chore: use actual user identifier outputs * success in registration tests * cover all cases * pass contractVersion instead of circuitVersion * fix disclose test * chore: add natspecs for ImplHubV2, CustomVerifier and GenericFormatter * change val name and remove unused lines * add val name change * remove userIdentifier from return data * feat: use GenericDiscloseOutput struct in verfication hook fix test cases for user identifier * chore: change the function order for Hub Impl V2 (#625) * fix nat specs * add nat spec in SelfStructs --------- Co-authored-by: Ayman <[email protected]> Co-authored-by: Nesopie <[email protected]> * prettier (#629) * CAN auth - android (#613) * add missed files * add NFCMethodSelectionScreen * bump android build --------- Co-authored-by: Justin Hernandez <[email protected]> * feat: add MRZ correction method to NFCMethodSelectionScreen (#627) * add npm auth token env (#632) * bump sdk version (#633) * publish npm package when merging on dev * bump common sdk version * replace yarn publish by npm publish * update common package version * Simplify dev mode gesture (#635) * Simplify developer mode gesture * Enable dev mode on MockData screen with five taps * add build smt function to common sdk * update vc_and_disclose_id test (dev branch) (#641) * fix: vc_and_disclose_id test * chore: yarn prettier * Show modal on NFC scan error (#642) * Add help button and error modal actions * fix the screen management * yarn nice * Bump build v2.5.4: ios 132; android 71 (#631) * bump version and build numbers * remove tamagui/toast * fix marketing version * fix: update TD1 and TD3 checks (#643) * bum yarn.lock * Bump build: ios 133; android 72 and build fixes (#654) * update gesture version and bump android build * bump and fix ios build * update lock files * fixes * fix fotoapparat library source * Update example contracts to include EUID usage (#656) * refactor: update HappyBirthday contract to V2 with support for E-Passport and EUID cards, introduce bonus multipliers, and enhance verification logic * refactor: update Airdrop contract to V2 with support for E-Passport and EU ID Card attestations * refactor: remove BASIS_POINTS constant from Airdrop contract * feat: introduce SelfIdentityERC721 contract for issuing NFTs based on verified identity credentials, replacing SelfPassportERC721 * fix: update verification functions in Airdrop, HappyBirthday, and SelfIdentityERC721 contracts to use customVerificationHook * cherry pick commit from add-test-self-verification... * block non-dev pr to main branch * audit fixes (#645) * merge dev branch into main (#624) * remove sdk/tests (#622) * remove sdk/tests * chore: update yarn.lock --------- Co-authored-by: Ayman <[email protected]> * fix: add range check on paddedInLength of shaBytesDynamic (#623) * fix ci (#626) --------- Co-authored-by: Ayman <[email protected]> Co-authored-by: Vishalkulkarni45 <[email protected]> * update contracts (#628) * remove sdk/tests (#622) * remove sdk/tests * chore: update yarn.lock --------- Co-authored-by: Ayman <[email protected]> * fix: add range check on paddedInLength of shaBytesDynamic (#623) * fix ci (#626) * implement self uups upgradeable (#592) * implement self uups upgradeable * small changes in identityVerificationHubImplV2 * delete aderyn.toml * chore: add custom verifier * chnage return output * feat: use self structs and a Generic output struct * feat: add userIdentifier, nullifier, forbiddencountries to returned output * add root view functions from registry * fix: build and compilation errors * add userDefined data into selfVerificationRoot * "resolve conflicts" * fix compilation problem * fix how to register verification config * test: CustomVerifier * fix verification root and hub integration * add scope check in hub impl * replace poseidon hash to ripemd+sha256 * add todo list * feat: refactor and add test cases for generic formatter * add performUserIdentifierCheck in basicVerification * change how to handle additionalData and fix stack too deep * start adding test codes * fix dependency problems in monorepo * fix: forbidden countries (#612) LGTM! * able to run test code * pass happy path * delete unused codes * change error code name, add caller address validation and add scripts to run test and build in monorepo * add all test cases in vcAndDisclose flow * remove comment out * chore: use actual user identifier outputs * success in registration tests * cover all cases * pass contractVersion instead of circuitVersion * fix disclose test * chore: add natspecs for ImplHubV2, CustomVerifier and GenericFormatter * change val name and remove unused lines * add val name change * remove userIdentifier from return data * feat: use GenericDiscloseOutput struct in verfication hook fix test cases for user identifier * chore: change the function order for Hub Impl V2 (#625) * fix nat specs * add nat spec in SelfStructs --------- Co-authored-by: Ayman <[email protected]> Co-authored-by: Nesopie <[email protected]> * prettier (#629) --------- Co-authored-by: Ayman <[email protected]> Co-authored-by: Vishalkulkarni45 <[email protected]> Co-authored-by: nicoshark <[email protected]> Co-authored-by: Nesopie <[email protected]> * fix: vc_and_disclose_id test (#640) * fix: vc_and_disclose_id test * chore: yarn prettier * fix: check if a config id exists * chore: change the function where the config not set verification is happening * fix: add await * feat: add getConfigId function in SelfVerificationRoot (#650) * feat: add getConfigId function in SelfVerificationRoot * update comment --------- Co-authored-by: motemotech <[email protected]> * chore: fix ofac end index in eu id cards * chore: fix tests * fix: example contracts and tests --------- Co-authored-by: turnoffthiscomputer <[email protected]> Co-authored-by: Vishalkulkarni45 <[email protected]> Co-authored-by: nicoshark <[email protected]> * Update deployment module for Identity Verification Hub V2 with detailed documentation and library linkage for CustomVerifier. Update initialization process to reflect changes in V2 implementation, ensuring proper setup for proxy deployment. (#658) * publish npm-package (#651) * App/eu id updates (#638) * fix build issues * generate disclosure proof with euids * generate disclosure proof with euids * Eu id updates 2 (#648) * update vc_and_disclose_id test (dev branch) (#641) * fix: vc_and_disclose_id test * chore: yarn prettier * Show modal on NFC scan error (#642) * Add help button and error modal actions * fix the screen management * yarn nice * Bump build v2.5.4: ios 132; android 71 (#631) * bump version and build numbers * remove tamagui/toast * fix marketing version * fix: update TD1 and TD3 checks (#643) * bum yarn.lock * add version and user defined data --------- Co-authored-by: Vishalkulkarni45 <[email protected]> Co-authored-by: Justin Hernandez <[email protected]> Co-authored-by: Seshanth.S🐺 <[email protected]> * remove the mock user define data * get the useridentifier as a hash from the user defined data * chore: add version and userDefinedData * feat: use the version in register / dsc proofs as well * update calculateUserIdentifierHash * yarn nice * refactor: consolidate user context data handling and update payload structure * fix typing issues on sha1 * remove console.log(sha1) * fix sha1 import * refactor: streamline userDefinedData handling and adjust payload type for circuit * refactor: update sha1 usage and enhance logging in calculateUserIdentifierHash * yarn nice * yarn lint common * use ts-ignore for sha1 import * fix app ci tests * fix typing issue * remove unused ts-ignore * cast uuid before calling generateinputs * bump qrcode version * add tsup on the qrcode sdk * fix: exports on selfxyz/qrcode * update how we define config.version * fix yarn imports * yarn format --------- Co-authored-by: Vishalkulkarni45 <[email protected]> Co-authored-by: Justin Hernandez <[email protected]> Co-authored-by: Seshanth.S🐺 <[email protected]> Co-authored-by: Ayman <[email protected]> * Hotfix contract compile error (#660) * Fix previous rebase error * Refactor deployment module for Identity Verification Hub V2. * Fix/sdk (#652) * fix: sdk build configs * chore: SelfBackendVerifier (WIP) * feat: add custom verification * feat: consider destination chain in user defined data * chore: export attestation id * chore: export attestation id * chore: export config storage * chore: don't throw an error if the proof is not valid * chore: trim abi and rm typechain types * refactor * chore: rm unnecessary exports * 📝 Add docstrings to `fix/sdk` (#653) Docstrings generation was requested by @remicolin. * #652 (comment) The following files were modified: * `sdk/core/src/utils/hash.ts` * `sdk/core/src/utils/proof.ts` * `sdk/core/src/utils/utils.ts` Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * review fixes * chore: fix package.json cjs types * chore: add minor changes to checks * feat: add InMemoryConfigStore, allIds constant and verificationResult type * chore: export Verification config * feat: change the verification config types * fix: throw issues early if verification config is null * fix: update yarn.lock file * chore: lint * fix: rm ts expect error directive * fix: contract tests * use excluded countries instead forbidden countries list * chore: change types in constnats --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update npm-publish workflow and bump core package version to 1.0.0 (#661) * update import * Update get verification config visibility (#664) * Update deployment module for Identity Verification Hub V2 to correct file paths and module name for deployment commands. * Add troubleshooting documentation for verification issues in deployHubV2.ts. Include manual verification steps and common failure reasons to assist users during deployment. * Change visibility of getVerificationConfigV2 function from internal to public in IdentityVerificationHubImplV2 contract to allow external access. * Apply BUSL v1.1 license headers to app (#665) * Add BSL license headers to app sources * prettier * fix license reference - https://spdx.org/licenses/BUSL-1.1.html * bump build: android 73 (#659) * Contracts/deploy staging (#668) * update scripts * deploy vc and disclose id * fix the deployment scripts on staging * update yarn.lock * bump ios build and version (#669) * configure coderabbitai (#670) * tweak coderabbit * bump * more thorough test spec * Apply BSL to app codebase (#639) * Clean up root license wording * Simplify SPDX header * simplify license and rename BSL to BUSL * fix merge issues * fix missing method --------- Co-authored-by: Justin Hernandez <[email protected]> * SEL-423 apply xcode build suggestions (#671) * apply recommended app settings from xcode * stick to portrait orientation and update target settings * remove app clip references * Circuit audit fixes (#644) * feat: add range checks before use of LessEqThan and SelectSubArray * fix: Num2Bits_strict to constrain virtualKey * bump core version * bump core version and fix ci * chore: use npm_auth_token in yarnrc * chroe: rm yarnrc changes * chore: update npm publish * chore: run npm publish manually * chore: change hub contract address (#675) * Update npm-publish.yml --------- Co-authored-by: Ayman <[email protected]> Co-authored-by: Vishalkulkarni45 <[email protected]> Co-authored-by: nicoshark <[email protected]> Co-authored-by: Nesopie <[email protected]> Co-authored-by: Seshanth.S🐺 <[email protected]> Co-authored-by: Justin Hernandez <[email protected]> Co-authored-by: Justin Hernandez <[email protected]> Co-authored-by: Kevin Lin <[email protected]> Co-authored-by: kevinsslin <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Eric Nakagawa <[email protected]>
* update hardhat config * update vc and disclose verifier * update vc and disclose verifier script and run it * update test self verification root
* docs: add architecture guide and reorganize docs * docs: add architecture prompt tasks
* chore: add TypeScript tooling * chore: remove unused tsconfig and migration script * update settings * add knip config * format * more reliable circuits tests * fix workflow * fix types * updates * ts updates * more updates * fix upgrade * fixes
… safety (#895) * cr feedback * update based on feedback * typing updates * unify yarn package version * update lock
* feat: stub mrz, nfc, and qr modules * feat: export mergeConfig and browser feature modules * refactor: use named exports for feature modules * arch feedback
* chore: trim lint warnings and ignore dev paths * fix: restore navigation param types * fixes * fixes * tweaks and cr feedback * final improvements * fix nav types
* docs: align migration rules with updated checklist * docs: restore demo sample sections
* chore: add package version mismatch check * chore: check workflow node versions * update script and add tests * fix scripts * fixes
* chore: focus coderabbit on meaningful suggestions * chore: expand coderabbit paths * chore: fix CodeRabbit tone length * Update .coderabbit.yaml Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* feat: add passport validation * refactor: consume sdk passport validator in app * feat: add tracking hook to passport validation * feat: add validation callbacks * docs: clarify passport validator * fix: validate passport hash algorithms * abstract properly * fix tests * format * fix logger and bundler * fix tests * fix tests and bundling * fix format * update lock * fix types * fixes * fixes * don't leak pii * done * revert error
* feat: add native event bridge * refactor(native-events): validate module and harden tests (#903)
* docs: expand React Native component migration * fix bad merge
* chore: standardize test tooling and type defs * fix bad merge * move to dev dependencies * fix version
* Enforce separate type imports * chore: enforce separate type exports * refactor: flatten sdk index exports * updates * fix improperly sorted types * fixes and prettier * prettier
* chore: unify lightweight deps and workflow node version * gigamind flow updates * standardize workflows * fix workflow * improvements * Gigamind tweaks (#911) * save gigamind updates * keep manual review on for now * pr feedback * fix pipeline * make runner more dynamic * fix regex * fixes * update simulator * fixes * fixes * fix regex * try again * test * revert back to dev settings * mobile fixes * fixes * fix runtime * just use latest * pr feedback * remove template * Update .github/workflows/mobile-e2e.yml Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* chore: streamline ios e2e workflow * fix version * fix * fixes * fix path * fix path issue * revert back to dev. test locally first * fix missing dep * update lock * fix version * fix ndk * fix scheme * fixes * fix e2e * fixes * fix pipeline * fix and unify xcode version * update workflow * fix pipeline * implement pipeline fix * fix ios version * remove || true
* feat: add typed errors * docs: add jsdoc comments to error classes * docs: document sdk error helpers * pr feedback
* implement deferred linking * yarn nice
* simplify pr template * update ignore pr action items line * add full analysis workflow export file * fix template * fixes
* feat: expose MRZ parsing on client * feat: add SelfClient provider * feat: add react native entry * feat: add mobile sdk entry wrapper * upgrade packages * save wip * clean up tests to support new arch * abstract SelfMobileSdk * pr feedback * updates * pr updates * update lock * updates * fix types * updates * fix tests * test: verify provider memoization and add jsdoc (#929) * pr feedback
* feat: add Self SDK provider * test: cover passport camera and self client provider * allow this to fail for now cuz the runner doesn't support 16.4 * sort imports * upgrade and fix * fix header issue * pr feedback * fix linter * Update app/scripts/check-license-headers.mjs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update app/scripts/check-license-headers.mjs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * update extensions and lock file * simplify call * use caching --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
WalkthroughLarge-scale CI/CD, deployment, and platform updates: new/updated GitHub workflows and composite actions; Fastlane modularization with version management and auto-deploy flows; Android NDK 27, Hermes, deep link/OAuth changes, and NFC analytics; iOS Hermes enablement, RNFB config, logging bridge, APNs prod, E2E stubs; app config/scripts, logging providers; security scanning; extensive docs/specs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub
participant WF as mobile-deploy-auto.yml
participant MD as mobile-deploy.yml (workflow_call)
participant FL as Fastlane
participant Store as App Store / Play Store
participant GHRel as GitHub Release
Dev->>GH: Merge PR to main/dev
GH-->>WF: Trigger on closed PR (merged)
WF->>WF: Check labels/skip-deploy/branch
alt should_deploy
WF->>MD: Call with inputs (platform, track, version_bump, test_mode)
par iOS
MD->>FL: Build/Upload iOS (track/test_mode)
FL->>Store: Upload/TestFlight or App Store
and Android
MD->>FL: Build/Upload Android (track/test_mode)
FL->>Store: Upload/Internal or Production
end
MD->>GHRel: Tag and create release (production)
else skip
WF->>GH: Log skip message
end
sequenceDiagram
autonumber
participant JS as JS App
participant PR as PassportReader (iOS/Android)
participant NFC as NFC Chip
participant LogBr as NativeLoggerBridge
participant Sinks as Loki/Sentry
JS->>PR: scanPassport(..., usePacePolling)
PR-->>LogBr: logEvent("scan_started", flags)
PR->>NFC: Open session / PACE/BAC / Read DG1/DG2/SOD
PR-->>LogBr: logEvent(status updates, metrics/errors)
PR-->>JS: Resolve with data or reject on error
LogBr-->>Sinks: Forward events (async)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 82
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (17)
app/android/app/src/main/AndroidManifest.xml (1)
21-24: Restrict Cleartext Traffic for Production BuildsThe manifest currently allows cleartext HTTP globally—a material security risk. Limit cleartext to development hosts only by:
• Using a manifest placeholder for
android:usesCleartextTraffic(true in debug, false in release)
• Addingandroid:networkSecurityConfig="@xml/network_security_config"to your<application>tag
• Creatingapp/src/main/res/xml/network_security_config.xmlthat only permits cleartext for known dev domainsSuggested diff:
<application android:theme="@style/AppTheme" - android:usesCleartextTraffic="true" + android:usesCleartextTraffic="${usesCleartextTraffic}" + android:networkSecurityConfig="@xml/network_security_config" >Example Gradle (Groovy):
android { buildTypes { debug { manifestPlaceholders = [ usesCleartextTraffic: "true" ] } release { manifestPlaceholders = [ usesCleartextTraffic: "false" ] } } }Script check output:
NOTE: network_security_config.xml not found; please create it under
app/src/main/res/xml/to scope cleartext traffic.Ensure
network_security_config.xmluses<domain-config>to whitelist only your development endpoints.app/android/react-native-passport-reader/android/src/main/java/io/tradle/nfc/RNPassportReaderModule.kt (2)
229-233: Remove PII-heavy debug logging of raw optionsopts likely includes documentNumber, DOB, DOE, and CAN. Logging raw opts to Logcat risks PII leakage in production builds and during device logs collection.
- Log.d("RNPassportReaderModule", "opts set to: " + opts.toString()) + // Avoid logging raw opts to prevent PII leakage
704-708: Remove logging of full document signing certificate (Base64)Logging the entire certificate (Base64) to Logcat is sensitive and unnecessarily increases the attack surface and log size.
- Log.d(TAG, "certificateBase64: ${certificateBase64}")app/ios/OpenPassport/AppDelegate.mm (2)
112-118: Deep link handling bypasses React Native Linking; forward to RCTLinkingManagerReturning YES here without forwarding means JS may never receive custom scheme deep links. Forward to RCTLinkingManager so RN can route the URL.
Apply this diff:
- [AnalyticsReactNative trackDeepLink:url withOptions:options]; - return YES; + [AnalyticsReactNative trackDeepLink:url withOptions:options]; + // Forward to React Native Linking so JS receives the deep link + return [RCTLinkingManager application:application openURL:url options:options];
71-75: Avoid logging raw APNs device token in release buildsAPNs tokens are sensitive; logging them in production risks leakage. Gate logs behind DEBUG.
Apply this diff:
- NSString *token = [self stringFromDeviceToken:deviceToken]; - NSLog(@"APNs device token: %@", token); + #ifdef DEBUG + NSString *token = [self stringFromDeviceToken:deviceToken]; + NSLog(@"APNs device token: %@", token); + #endif.gitignore (1)
12-16: Scope the .yarnrc.yml ignore to the repo root to avoid silently ignoring workspace-level Yarn configsThe unrooted pattern will ignore any .yarnrc.yml anywhere. If you ever add per-package Yarn configs (e.g., new workspaces), they’ll be unintentionally untracked, leading to drift between local and CI installs.
Apply this diff to constrain the ignore to the root only:
-.yarnrc.yml +/.yarnrc.ymlapp/.gitignore (1)
91-98: Reintroduce proper ignores for source maps and Android Kotlin error artifactsThe previous conflated pattern was broken; removing it is correct. However, without a replacement, there’s a risk of accidentally committing source maps (which can bloat the repo and expose source) and Android error outputs.
Consider adding explicit, separate ignores:
# Maestro maestro-results.xml + +# Bundle analyzer/source maps and Android error artifacts +*-sourcemap.json +android/.kotlin/errors/app/android/build.gradle (1)
23-33: Remove duplicated Gradle plugins for deterministic buildsYou’ve declared multiple versions of key Gradle plugins, which can lead to nondeterministic resolution and subtle build failures:
• Android Gradle Plugin: 7.3.1 vs 8.1.0
• Kotlin Gradle Plugin:$kotlinVersion(1.9.24) vs hard-coded 1.7.0
• Google Services Plugin: 4.3.14 vs 4.4.0Consolidate each to a single, up-to-date version. For example, in
app/android/build.gradle:buildscript { dependencies { - classpath("com.android.tools.build:gradle:7.3.1") classpath("com.facebook.react:react-native-gradle-plugin") - // classpath("org.jetbrains.kotlin:kotlin-gradle-plugin") classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlinVersion" - classpath 'com.android.tools.build:gradle:8.1.0' - classpath 'com.google.gms:google-services:4.3.14' + classpath 'com.android.tools.build:gradle:8.1.0' + classpath 'com.google.gms:google-services:4.4.0' classpath 'com.google.firebase:firebase-crashlytics-gradle:2.9.2' classpath 'org.mozilla.rust-android-gradle:plugin:0.9.3' - classpath("org.jetbrains.kotlin:kotlin-gradle-plugin:1.7.0") + // removed hard-coded Kotlin plugin } }• Remove the older AGP and Google-services entries.
• Keep onlyorg.jetbrains.kotlin:kotlin-gradle-plugin:$kotlinVersion(1.9.24).After applying, run a Gradle sync locally and validate on CI to confirm no downstream breakage.
app/env.sample (1)
1-12: Ensure env.sample mirrors env.ts by adding the missing variableThe code in app/env.ts (and your test setup) reads
GOOGLE_SIGNIN_WEB_CLIENT_ID, so we must include it in app/env.sample to avoid confusing local builds.• File: app/env.sample
• Insert immediately afterGOOGLE_SIGNIN_ANDROID_CLIENT_ID=ENABLE_DEBUG_LOGS= GITGUARDIAN_API_KEY= GITLEAKS_LICENSE= GOOGLE_SIGNIN_ANDROID_CLIENT_ID= +GOOGLE_SIGNIN_WEB_CLIENT_ID= GRAFANA_LOKI_URL= GRAFANA_LOKI_USERNAME= GRAFANA_LOKI_PASSWORD= IS_TEST_BUILD= MIXPANEL_NFC_PROJECT_TOKEN= SEGMENT_KEY= SENTRY_DSN=Let’s keep these in sync so everyone’s dev environment stays happy and error-free!
.github/actions/mobile-setup/action.yml (1)
55-103: Fail fast and add a preflight check in the install stepHarden the install script with strict shell flags and a guard on app_path to avoid confusing failures and ensure consistent behavior across runners.
- name: Install app dependencies shell: bash run: | - cd ${{ inputs.app_path }} + set -euo pipefail + APP_PATH="${{ inputs.app_path }}" + if [ ! -d "$APP_PATH" ]; then + echo "❌ ERROR: app_path '$APP_PATH' does not exist. Ensure actions/checkout has run and the path is correct." + exit 1 + fi + cd "$APP_PATH" # Configure Yarn corepack enable yarn set version 4.6.0.github/workflows/npm-publish.yml (3)
64-72: Do not echo secrets in logs; pass NPM token via environment for Yarn publishUsing “yarn config set … npmAuthToken” prints the token (albeit masked) to logs and persists it to config. Prefer environment variables to reduce exposure and avoid stateful mutations.
- name: Publish to npm working-directory: sdk/core run: | - yarn config set npmScopes.selfxyz.npmAuthToken ${{ secrets.NPM_TOKEN }} - yarn config set npmPublishAccess public yarn npm publish --access public env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} NPM_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} + YARN_NPM_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} + YARN_NPM_REGISTRY_SERVER: https://registry.npmjs.org
93-101: Repeat: avoid logging NPM token; use env for Yarn publish (qrcode)Same security concern in the qrcode job.
- name: Publish to npm working-directory: sdk/qrcode run: | - yarn config set npmScopes.selfxyz.npmAuthToken ${{ secrets.NPM_TOKEN }} - yarn config set npmPublishAccess public yarn npm publish --access public env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} NPM_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} + YARN_NPM_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} + YARN_NPM_REGISTRY_SERVER: https://registry.npmjs.org
122-131: Repeat: avoid logging NPM token; use env for Yarn publish (common)Same security concern in the common job.
- name: Publish to npm working-directory: common run: | - yarn config set npmScopes.selfxyz.npmAuthToken ${{ secrets.NPM_TOKEN }} - yarn config set npmPublishAccess public yarn npm publish --access public env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} NPM_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} + YARN_NPM_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} + YARN_NPM_REGISTRY_SERVER: https://registry.npmjs.orgapp/README.md (1)
199-205: Replace external “chat.openai.com” link with project-appropriate guidance.A vendor-specific share link is brittle and not appropriate for a public README. Provide neutral instructions or an official Apple reference instead.
Apply this diff:
- Find your [development team id](https://chat.openai.com/share/9d52c37f-d9da-4a62-acb9-9e4ee8179f95) and run: + Find your Apple Developer Team ID (Apple Developer account → Membership details) and run:Optionally, add a quick CLI hint for local discovery:
- In Xcode: Settings → Accounts → select your team → Team ID
- Or grep the project for DevelopmentTeam to see current IDs:
rg -n "DevelopmentTeam" ios.github/workflows/gitleaks.yml (1)
6-21: Implement least-privilege permissions, correct SARIF upload, and fork-safe license handlingGreat to have Gitleaks on PRs! A few critical updates:
- Add minimal permissions for Code Scanning
- Remove unsupported
report-format/report-pathinputs and use the action’s built-in SARIF upload- Upload the generated SARIF with CodeQL’s
upload-sarif@v3usingsarif_file- Gate the licensed Gitleaks step in forked PRs (fallback to CLI scan)
Apply this diff to .github/workflows/gitleaks.yml:
jobs: gitleaks: runs-on: ubuntu-latest + permissions: + contents: read + security-events: write steps: - uses: actions/checkout@v4 with: fetch-depth: 0 - name: Gitleaks scan (org repos) - uses: gitleaks/[email protected] - with: - config-path: .gitleaks.toml - fail: true - report-format: sarif - report-path: gitleaks.sarif + uses: gitleaks/[email protected] + if: ${{ !github.event.pull_request.head.repo.fork }} + with: + config-path: .gitleaks.toml + fail: true + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GITLEAKS_LICENSE: ${{ secrets.GITLEAKS_LICENSE }} + GITLEAKS_ENABLE_UPLOAD_ARTIFACT: true - name: Upload SARIF to GitHub Code Scanning - uses: github/codeql-action/upload-sarif@v3 - with: - sarif_file: gitleaks.sarif + uses: github/codeql-action/upload-sarif@v3 + if: ${{ !github.event.pull_request.head.repo.fork }} + with: + sarif_file: results.sarif + category: gitleaks-org + - name: Fork-safe Gitleaks scan + if: ${{ github.event.pull_request.head.repo.fork }} + run: | + gitleaks detect \ + --config-path .gitleaks.toml \ + --report-format sarif \ + --report-path fork-results.sarif \ + --fail + + - name: Upload SARIF for forked PR + if: ${{ github.event.pull_request.head.repo.fork }} + uses: github/codeql-action/upload-sarif@v3 + with: + sarif_file: fork-results.sarif + category: gitleaks-forkThis ensures secure defaults, correct SARIF integration, and avoids failures in forked PRs.
.github/workflows/mobile-deploy.yml (2)
243-249: Harden secret material permissions (600) after decodingCertificates, keystores, and API keys are sensitive. Write them with owner-only permissions to reduce exposure on shared runners and to align with least-privilege.
iOS secrets:
- name: Decode certificate and profile if: inputs.platform != 'android' run: | mkdir -p "${{ env.APP_PATH }}$(dirname "${{ env.IOS_DIST_CERT_PATH }}")" echo "${{ secrets.IOS_DIST_CERT_BASE64 }}" | base64 --decode > ${{ env.APP_PATH }}${{ env.IOS_DIST_CERT_PATH }} echo "${{ secrets.IOS_PROV_PROFILE_BASE64 }}" | base64 --decode > ${{ env.APP_PATH }}${{ env.IOS_PROV_PROFILE_PROJ_PATH }} echo "${{ secrets.IOS_CONNECT_API_KEY_BASE64 }}" | base64 --decode > ${{ env.APP_PATH }}${{ env.IOS_CONNECT_API_KEY_PATH }} + chmod 600 ${{ env.APP_PATH }}${{ env.IOS_DIST_CERT_PATH }} + chmod 600 ${{ env.APP_PATH }}${{ env.IOS_CONNECT_API_KEY_PATH }}Android secrets:
- name: Decode Android Secrets if: inputs.platform != 'ios' run: | echo "${{ secrets.ANDROID_KEYSTORE }}" | base64 --decode > ${{ env.APP_PATH }}${{ env.ANDROID_KEYSTORE_PATH }} echo "${{ secrets.ANDROID_PLAY_STORE_JSON_KEY_BASE64 }}" | base64 --decode > ${{ env.APP_PATH }}${{ env.ANDROID_PLAY_STORE_JSON_KEY_PATH }} + chmod 600 ${{ env.APP_PATH }}${{ env.ANDROID_KEYSTORE_PATH }} + chmod 600 ${{ env.APP_PATH }}${{ env.ANDROID_PLAY_STORE_JSON_KEY_PATH }}Also applies to: 697-701
289-296: Align certificate permission check with secure mode (600), not 644With the above chmod 600, update the check to enforce owner-only permissions for the distribution certificate.
- CERT_PERMS=$(ls -l "${{ env.APP_PATH }}${{ env.IOS_DIST_CERT_PATH }}" | awk '{print $1}') - if [ "$CERT_PERMS" != "-rw-r--r--" ]; then - echo "❌ Error: Distribution certificate has incorrect permissions: $CERT_PERMS" - echo "Expected: -rw-r--r--" - exit 1 - fi + CERT_PERMS=$(ls -l "${{ env.APP_PATH }}${{ env.IOS_DIST_CERT_PATH }}" | awk '{print $1}') + if [ "$CERT_PERMS" != "-rw-------" ]; then + echo "❌ Error: Distribution certificate has incorrect permissions: $CERT_PERMS" + echo "Expected: -rw-------" + exit 1 + fi
🧹 Nitpick comments (35)
app/android/app/google-services.json (2)
15-31: OAuth clients added: verify API key restrictions in Google Cloud ConsoleWith Google Sign-In/OAuth now configured, ensure the exposed API key is restricted to:
- Application restrictions: Android apps only, bound to package name and SHA-1(s).
- API restrictions: only the specific Google APIs you use (e.g., Firebase, Identity Toolkit).
Unrestricted keys are routinely scraped from public repos and abused. Even though this file is expected to be committed, lack of restrictions is a medium-risk security issue.
21-24: Include Play App Signing SHA-1 to avoid prod login failuresThe certificate_hash entries likely correspond to debug and upload keystores. If Play App Signing is enabled, the runtime app uses the Play signing cert, whose SHA-1 must also be added to your Firebase/Google OAuth credentials. Without it, Google Sign-In and Phone Auth can fail in production only.
app/android/android-passport-reader/app/src/main/java/example/jllarraz/com/passportreader/utils/OcrUtils.kt (1)
70-82: Gender is parsed but ignored; consider propagating it into MRZInfoYou parse
genderbut always setGender.MALEin MRZ output. If downstream relies on MRZ fields, this is misleading.Option A (local change): Inline MRZInfo creation here and map
gendertoGender:
- "M" -> Gender.MALE
- "F" -> Gender.FEMALE
- "<" -> a neutral/unknown value if available in your Gender enum
Option B (preferred for consistency): Extend
createDummyMrzto accept aGenderparameter and pass the parsed value.Example (outside this hunk):
// Update signature private fun createDummyMrz( documentType: String, issuingState: String = "ESP", documentNumber: String, dateOfBirthDay: String, expirationDate: String, nationality: String = "ESP", gender: Gender = Gender.MALE // TODO: map parsed char to Gender ): MRZInfo { /* ... */ }Then pass the mapped
genderwhen calling it in the ID branch.app/metro.config.cjs (1)
20-34: Unify ‘@selfxyz/common’ to ESM output to avoid dual‐package instancesMapping the base alias to
dist(CJS root) while subpaths point atdist/esmcan cause two copies of the same module (CJS vs ESM), leading to identity mismatches (singletons, instanceof checks, etc.). Pick one build format across all aliases:• In app/metro.config.cjs
- '@selfxyz/common': path.resolve(commonPath, 'dist'), + '@selfxyz/common': path.resolve(commonPath, 'dist/esm'),• Build common and verify you have
dist/esm/index.js(and all thedist/esm/src/...files).If you’d rather stick with CJS, instead point your subpath aliases at
dist/cjs.This keeps module identity consistent and avoids unpredictable behavior.
app/android/build.gradle (1)
40-40: Remove jcenter(): repository is deprecated and increases supply-chain riskjcenter() has been sunset and should be removed to avoid unexpected dependency resolution from an unmaintained source. Prefer mavenCentral() and vendor repos.
Apply this minimal change:
- jcenter()If any artifact still relies on jcenter, pin it to a maintained source or consider vendoring.
.cursor/mcp.json (1)
12-15: Gate MCP usage via settings to reduce unintended activation.Good to see
disableAutoPRAnalysisandmanualReviewEnabled. Consider adding a documented activation path (e.g., environment flag) and a short SECURITY.md note so contributors understand when connecting to external MCP is acceptable..github/workflows/gitguardian.yml (1)
15-15: Pin the GitGuardian action to a commit SHA to mitigate supply-chain risk.Version tags can be retagged. Pin to a verified commit digest and periodically rotate.
Example (replace with the published commit for v1.41.0):
uses: GitGuardian/ggshield/actions/secret@<commit-sha>app/docs/examples/tree-shaking/level2-optimal-example.ts (1)
14-14: Import the specific hash implementation for stronger tree-shaking.Avoid barrel/module-level imports; deep import the exact function to minimize bundle surface.
-import { hash } from '@selfxyz/common/utils/hash'; +import { hash } from '@selfxyz/common/utils/hash/sha';app/docs/examples/tree-shaking/granular-circuits-example.ts (1)
7-7: Deep-import the circuit utility to maximize tree-shaking.Importing from the circuits index can pull more than needed, depending on bundler/exports. Deep-import the file that defines generateCircuitInputsDSC.
-import { generateCircuitInputsDSC } from '@selfxyz/common/utils/circuits'; +import { generateCircuitInputsDSC } from '@selfxyz/common/utils/circuits/dscInputs';app/android/gradle.properties (1)
13-13: UseStringDeduplication requires G1GC; current ParallelGC setting nullifies it.With -XX:+UseParallelGC, -XX:+UseStringDeduplication is ignored. Either switch to G1GC to benefit from string dedup, or drop the dedup flag. G1GC generally works well for Gradle builds on JDK 17.
Recommended (enable G1 + dedup):
-org.gradle.jvmargs=-Xmx4096m -XX:MaxMetaspaceSize=1024m -XX:+HeapDumpOnOutOfMemoryError -Dfile.encoding=UTF-8 -XX:+UseParallelGC -XX:+UseStringDeduplication +org.gradle.jvmargs=-Xmx4096m -XX:MaxMetaspaceSize=1024m -XX:+HeapDumpOnOutOfMemoryError -Dfile.encoding=UTF-8 -XX:+UseG1GC -XX:+UseStringDeduplicationIf you want to keep ParallelGC, remove dedup to avoid confusion:
org.gradle.jvmargs=-Xmx4096m -XX:MaxMetaspaceSize=1024m -XX:+HeapDumpOnOutOfMemoryError -Dfile.encoding=UTF-8 -XX:+UseParallelGCAction: Please confirm GC choice in CI/dev and we can tune pause/throughput flags accordingly (e.g., -XX:MaxGCPauseMillis for G1).
app/Gemfile (1)
15-15: Extend Bundler cache keys to include Ruby version & correct lockfileYour cache-bundler action already partitions by OS (
${{ runner.os }}), which prevents cross-platform pollution. To also avoid stale or incompatible native extensions when bumping Ruby or gems:• In each
.github/workflows/mobile-*.ymlstep that does
uses: ./.github/actions/cache-bundler
add thelock-fileandcache-versioninputs:- uses: ./.github/actions/cache-bundler with: path: app/vendor/bundle lock-file: app/Gemfile.lock # ensure we hash the right Gemfile.lock cache-version: ${{ env.RUBY_VERSION }} # bust when Ruby version changes• Confirm
app/Gemfile.lockis committed (so its hash changes when dependencies update).This guarantees your cache keys reflect both OS and Ruby version, and bust on lockfile upgrades.
.github/actions/mobile-setup/action.yml (1)
47-54: Optional improvements: Harden Bundler setup and speed CI installsWe’ve verified that every workflow using
.github/actions/mobile-setupincludesactions/checkoutbeforehand, so the “missing checkout” risk is mitigated. To further bulletproof and optimize your CI:• Enable strict shell mode (
set -euo pipefail)
• Guard against a missing or mis-typedapp_pathdirectory
• Exclude development/test gems to cut install timeApply the following patch in
.github/actions/mobile-setup/action.yml(lines 47–54):--- a/.github/actions/mobile-setup/action.yml +++ b/.github/actions/mobile-setup/action.yml @@ -47,7 +47,14 @@ - name: Configure bundler shell: bash run: | - cd ${{ inputs.app_path }} + set -euo pipefail + APP_PATH="${{ inputs.app_path }}" + if [ ! -d "$APP_PATH" ]; then + echo "❌ ERROR: app_path '$APP_PATH' does not exist. Make sure actions/checkout has run." + exit 1 + fi + cd "$APP_PATH" bundle config set --local path 'vendor/bundle' bundle config set --local deployment 'true' + bundle config set --local without 'development test' echo "✅ Bundler configured for strict mode (deployment=true)".github/workflows/npm-publish.yml (1)
112-116: Remove duplicate checkout in publish-commonThere are two checkout steps in this job; the second is redundant and slows the workflow.
with: node-version-file: .nvmrc registry-url: "https://registry.npmjs.org" - - uses: actions/checkout@v4 - name: Install Dependencies uses: ./.github/actions/yarn-installapp/docs/examples/tree-shaking/optimal-pattern-example.ts (2)
4-13: Remove unused imports to prevent accidental side effects and to stay true to “optimal” tree-shaking.
PASSPORT_ATTESTATION_ID,generateCommitment, andhashare not used. Keeping unused imports can force module evaluation if utils/constants have any side effects, which undermines the example’s goal.Apply this diff:
import { API_URL, countryCodes, - PASSPORT_ATTESTATION_ID, } from '@selfxyz/common/constants'; // Import types (these are eliminated during compilation anyway) import type { PassportData } from '@selfxyz/common/types'; -// Import utilities from the utils module -import { generateCommitment, hash } from '@selfxyz/common/utils'; +// Keep imports minimal to maximize tree-shaking; add only when actually used
26-29: Avoid Object.keys(...).includes(...) in hot paths; use a direct membership check.Building a keys array each call is unnecessary and slower. A direct hasOwnProperty check is faster and avoids extra allocations; it also reads as intent: “is code in the map?”.
Apply this diff:
- // Extract country code from MRZ (positions 2-4 in passport MRZ) - isValidCountry: Object.keys(countryCodes).includes( - passportData.mrz?.slice(2, 5) || '', - ), + // Extract country code from MRZ (positions 2-4 in passport MRZ) + isValidCountry: (() => { + const code = passportData.mrz?.slice(2, 5); + return !!code && Object.prototype.hasOwnProperty.call(countryCodes, code); + })(),.cursor/rules/mobile-sdk-migration.mdc (1)
264-268: Lock in tree-shaking with explicit exports mapping and sideEffects=false.To reliably keep bundles lean for partners, ensure the SDK package declares explicit export paths and no side-effects, aligning with our prior learning on export mappings for better tree shaking.
Apply this diff:
### Bundle Size - **Tree shaking**: Ensure all exports support tree shaking - **Code splitting**: Split large modules into smaller chunks - **Dependency analysis**: Monitor and optimize dependencies - **Bundle analysis**: Regular bundle size monitoring + - **Explicit exports**: Use precise `package.json#exports` mappings (avoid wildcards) in `packages/mobile-sdk-alpha` + - **No side effects**: Set `"sideEffects": false` in `packages/mobile-sdk-alpha/package.json` and avoid barrel `index.ts` wildcards.vscode/settings.json (1)
78-83: Configure ESLint working directories for a monorepo instead of path-specific editor blocks.To ensure the ESLint server resolves configs per workspace, declare working directories. This delivers the intended per-package lint/fix behavior without unsupported glob-scoped editor settings.
Apply this diff:
// ESLint Configuration - Optimized for Performance "eslint.run": "onSave", + "eslint.workingDirectories": [ + { "pattern": "app" }, + { "pattern": "packages/*" } + ],.github/actions/cache-gradle/action.yml (1)
30-33: Include gradle.properties in cache key to prevent stale or incorrect restoresGradle builds often depend on gradle.properties; not hashing it can yield stale caches when flags or versions change there. Lowers CI reliability and can cause hard-to-debug build behavior.
Apply this diff to incorporate gradle.properties into the cache key:
- key: ${{ runner.os }}-gradle-${{ inputs.cache-version }}-${{ hashFiles('**/*.gradle*', '**/gradle-wrapper.properties') }} + key: ${{ runner.os }}-gradle-${{ inputs.cache-version }}-${{ hashFiles('**/*.gradle*', '**/gradle-wrapper.properties', '**/gradle.properties') }}.github/actions/cache-yarn/action.yml (1)
31-36: Broaden Yarn cache key to include Yarn config/plugins for correctness across upgradesYarn Berry changes (plugins/releases/yarnrc) won’t invalidate the cache with the current key. This can cause broken or inconsistent installs after Yarn upgrades or plugin changes.
Update the key to hash common Yarn metadata in addition to the lockfile:
- key: ${{ runner.os }}-yarn-${{ inputs.cache-version }}-${{ hashFiles(inputs.lock-file) }} + key: ${{ runner.os }}-yarn-${{ inputs.cache-version }}-${{ hashFiles(inputs.lock-file, '.yarnrc.yml', '.yarn/plugins/**', '.yarn/releases/**', '.yarn/install-state.gz', 'package.json') }}Optional: consider adding an input for node-version to segment caches across Node majors used in CI matrices.
app/docs/TREE_SHAKING.md (1)
54-75: Add language to fenced code block to satisfy markdownlint and improve readability.The code fence with output lacks a language identifier and is flagged by MD040. Use a generic language like "text".
Apply this diff:
-``` +```text 🧪 Testing: full-import 📝 Import everything from @selfxyz/common (worst case) 📊 Bundle size: 2.45 MB @@ 🎯 Maximum tree shaking benefit: 93.6% reduction (2.29 MB saved)</blockquote></details> <details> <summary>app/package.json (1)</summary><blockquote> `116-139`: **Ensure web-only dependencies are not leaking into native bundles** Packages such as react-dom, react-native-web, react-native-svg-web, react-qr-barcode-scanner, vite-plugin-svgr, and vite should be excluded from native bundles. If shared code imports web libs behind feature flags without proper platform guards, Metro may still pull them in, increasing app size and risking runtime errors. - Confirm platform-specific entry files and/or conditional exports are configured so Metro excludes web libs from native builds. - Validate with bundle analysis scripts you added: - yarn analyze:bundle:android - yarn analyze:bundle:ios - If you see leakage, consider: - Using .native.ts/.web.ts suffixes for split modules. - Metro resolver with extraNodeModules aliases mapping web-only packages to stub modules for native. - A “browser” or conditional exports map for Vite/web that points to web implementations and keeps native paths clean. Also applies to: 143-145 </blockquote></details> <details> <summary>.github/workflows/common-ci.yml (1)</summary><blockquote> `3-5`: **Prevent redundant macOS/Linux minutes: add concurrency cancellation** Without concurrency, multiple runs for the same ref can overlap and waste CI time. Add a workflow-level concurrency group to auto-cancel in-flight runs for the same branch/PR. ```diff name: Common CI on: pull_request: +concurrency: + group: common-ci-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true jobs:app/fastlane/helpers/slack.rb (1)
23-26: Avoid dumping full Ruby backtraces into CI logsPrinting e.backtrace leaks internal paths and can bloat logs. Consider gating verbose output behind an env flag or only surfacing e.class and e.message.
- rescue => e - report_error("Error during Slack upload process: #{e.message}", e.backtrace.join("\n"), "Slack Upload Failed") + rescue => e + debug = ENV["CI_DEBUG"].to_s == "1" + suggestion = debug ? e.backtrace.join("\n") : nil + report_error("Error during Slack upload process: #{e.class}: #{e.message}", suggestion, "Slack Upload Failed") false end.github/workflows/mobile-bundle-analysis.yml (1)
9-16: Add concurrency to prevent duplicate, expensive macOS runsBundle analysis on macOS is costly. Concurrency ensures new pushes cancel in-flight runs for the same ref.
on: pull_request: paths: - "app/**" - ".github/workflows/mobile-bundle-analysis.yml" - ".github/actions/**" workflow_dispatch: + +concurrency: + group: mobile-bundle-analysis-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: trueapp/fastlane/helpers/common.rb (2)
7-9: Consider additional CI platform detectionThe current implementation only checks for GitHub Actions CI (
ENV["CI"] == "true"). Consider detecting other CI platforms that might be used in the future (e.g., Jenkins, GitLab CI, CircleCI) for better portability.def is_ci_environment? - ENV["CI"] == "true" && ENV["ACT"] != "true" + # Check multiple CI environment variables + ci_detected = ENV["CI"] == "true" || + ENV["GITHUB_ACTIONS"] == "true" || + ENV["JENKINS_URL"] || + ENV["GITLAB_CI"] == "true" || + ENV["CIRCLECI"] == "true" + + ci_detected && ENV["ACT"] != "true" end
52-77: Add jitter to exponential backoff for rate limitsThe exponential backoff implementation for rate limits could benefit from adding jitter to prevent thundering herd problems when multiple instances hit rate limits simultaneously.
if is_rate_limit # Exponential backoff for rate limits: 5s, 10s, 20s, 40s... backoff_delay = delay * (2 ** (attempts - 1)) + # Add jitter (±20% randomization) to prevent thundering herd + jitter = backoff_delay * 0.2 * (rand() - 0.5) + backoff_delay = (backoff_delay + jitter).round UI.important("Rate limit hit. Retry ##{attempts} after #{backoff_delay}s: #{e.message}") sleep(backoff_delay).github/workflows/core-sdk-ci.yml (1)
58-60: Redundant conditional check for dependency installationThe condition
if: steps.build-cache.outputs.cache-hit != 'true'seems unnecessary sincefail-on-cache-miss: trueis set on line 57, which would fail the job if the cache is not found.-- name: Install Dependencies - if: steps.build-cache.outputs.cache-hit != 'true' - uses: ./.github/actions/yarn-install.github/workflows/circuits-build.yml (1)
130-134: Use array for build targets to improve maintainabilityThe circuit build commands are repeated with different targets. Consider using an array or loop to make this more maintainable and less error-prone.
- name: Build cpp circuits run: | chmod +x circuits/scripts/build/build_cpp.sh && \ - ./circuits/scripts/build/build_cpp.sh register && - ./circuits/scripts/build/build_cpp.sh register_id && - ./circuits/scripts/build/build_cpp.sh disclose && - ./circuits/scripts/build/build_cpp.sh dsc + for target in register register_id disclose dsc; do + echo "Building circuit: $target" + ./circuits/scripts/build/build_cpp.sh "$target" || exit 1 + done.gitleaks.toml (1)
508-517: Generic API Key rule could be noisy; ensure CI gating behavior is acceptableThe entropy threshold (3.5) and broad keywords are useful for catching unknown patterns but can increase false positives. Confirm that your workflow treats detections as blocking only when high-confidence, or that reviewers know how to triage.
If false positives become painful, consider tightening keywords or adding file/path-level allowlists for known noisy areas (e.g., changelogs, docs).
.github/workflows/mobile-e2e.yml (2)
268-276: Avoid caching iOS Simulator and DeviceSupport directories to reduce flakiness and cache bloatCaching:
- ~/Library/Developer/CoreSimulator/Devices
- ~/Library/Developer/Xcode/iOS DeviceSupport
is prone to incompatibilities across runner images/Xcode versions and can cause odd simulator boot failures. It also creates large, slow caches.
Recommend removing this step or drastically scoping it down.
Apply this diff to remove the simulator/device support cache:
- - name: Cache iOS Simulator - uses: actions/cache@v4 - with: - path: | - ~/Library/Developer/CoreSimulator/Devices - ~/Library/Developer/Xcode/iOS DeviceSupport - key: ${{ runner.os }}-simulator-v1 - restore-keys: | - ${{ runner.os }}-simulator-
212-221: Double-switching Xcode is redundant and can mask issuessetup-xcode sets the developer dir; switching again via xcode-select adds complexity. If the runner-image issue is fully resolved, consider removing the manual xcode-select to keep environment control in a single step.
app/fastlane/helpers/version_manager.rb (3)
47-54: Defensive cast for build number to avoid type surprisesIf version.json ever stores build as a string (e.g., via non-Ruby tooling), current + 1 will raise. Cast to integer on read, and write back as integer.
- current = data["ios"]["build"] - data["ios"]["build"] = current + 1 + current = Integer(data["ios"]["build"]) + data["ios"]["build"] = current + 1
56-63: Same here for Android build numberMirror the defensive cast for Android:
- current = data["android"]["build"] - data["android"]["build"] = current + 1 + current = Integer(data["android"]["build"]) + data["android"]["build"] = current + 1
65-77: Guard against missing platform key in version.json when updating timestampIf data["ios"] or data["android"] is missing, data[platform]["lastDeployed"] will raise. Initialize when absent.
- data[platform]["lastDeployed"] = timestamp + data[platform] ||= {} + data[platform]["lastDeployed"] = timestampapp/fastlane/Fastfile (1)
438-444: Android Slack message may reference ambiguous track labelYou compute target_platform earlier, but here you mix deployment_track and target_platform. If deployment_track is nil, the interpolation can read oddly. Since you already compute deployment_track above for Slack, default to options[:deployment_track] || options[:track] explicitly.
- initial_comment: "#{track_emoji} Android v#{package_version} (Build #{version_code}) deployed to #{deployment_track || target_platform} via #{deploy_source}", + initial_comment: "#{track_emoji} Android v#{package_version} (Build #{version_code}) deployed to #{(deployment_track || options[:track])} via #{deploy_source}",
| # Deployment addresses (might contain sensitive info) | ||
| contracts/deployed_addresses.json | ||
| contracts/ignition/deployments/staging/deployed_addresses.json | ||
| contracts/ignition/deployments/*/artifacts/ | ||
| contracts/deployments/*/ | ||
| !contracts/deployments/prod/ | ||
|
|
There was a problem hiding this comment.
Do not re-include prod deployment data into AI indexing
You’re ignoring all deployments but explicitly un-ignoring prod with “!contracts/deployments/prod/”. That can surface sensitive on-chain deployment metadata to the AI index. Keep prod deployments ignored.
contracts/ignition/deployments/*/artifacts/
contracts/deployments/*/
-!contracts/deployments/prod/📝 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.
| # Deployment addresses (might contain sensitive info) | |
| contracts/deployed_addresses.json | |
| contracts/ignition/deployments/staging/deployed_addresses.json | |
| contracts/ignition/deployments/*/artifacts/ | |
| contracts/deployments/*/ | |
| !contracts/deployments/prod/ | |
| # Deployment addresses (might contain sensitive info) | |
| contracts/deployed_addresses.json | |
| contracts/ignition/deployments/staging/deployed_addresses.json | |
| contracts/ignition/deployments/*/artifacts/ | |
| contracts/deployments/*/ |
🤖 Prompt for AI Agents
.cursorignore around lines 229-235: the file currently negates ignoring prod
deployments with "!contracts/deployments/prod/" which re-includes sensitive prod
deployment metadata into AI indexing; remove that negation (delete the
"!contracts/deployments/prod/" line) or replace it with an explicit ignore
pattern (e.g., "contracts/deployments/prod/" or "contracts/deployments/prod/**")
so prod deployments remain excluded from indexing.
| # Ensure source code is accessible | ||
| !**/*.ts | ||
| !**/*.tsx | ||
| !**/*.js | ||
| !**/*.jsx | ||
| !**/*.sol | ||
| !**/*.circom | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid re-including node_modules by narrowing allowed source globs
The broad allow-list “!**/*.(ts|tsx|js|jsx|sol|circom)” can re-include files under node_modules (negations override earlier ignores), ballooning index size and increasing risk of unrelated code surfacing in AI suggestions. Restrict to project source roots.
-# Ensure source code is accessible
-!**/*.ts
-!**/*.tsx
-!**/*.js
-!**/*.jsx
-!**/*.sol
-!**/*.circom
+# Ensure source code is accessible (limit to repo sources, not node_modules)
+!app/**/*.ts
+!app/**/*.tsx
+!app/**/*.js
+!app/**/*.jsx
+!common/**/*.ts
+!common/**/*.tsx
+!sdk/**/*.ts
+!sdk/**/*.tsx
+!contracts/**/*.sol
+!circuits/**/*.circom📝 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.
| # Ensure source code is accessible | |
| !**/*.ts | |
| !**/*.tsx | |
| !**/*.js | |
| !**/*.jsx | |
| !**/*.sol | |
| !**/*.circom | |
| # Ensure source code is accessible (limit to repo sources, not node_modules) | |
| !app/**/*.ts | |
| !app/**/*.tsx | |
| !app/**/*.js | |
| !app/**/*.jsx | |
| !common/**/*.ts | |
| !common/**/*.tsx | |
| !sdk/**/*.ts | |
| !sdk/**/*.tsx | |
| !contracts/**/*.sol | |
| !circuits/**/*.circom |
| $END$ | ||
|
|
There was a problem hiding this comment.
Remove $END$ sentinel to avoid truncating rule parsing.
Having $END$ mid-file risks downstream tooling ignoring everything that follows (Self App Development Rules). Keep the document continuous.
Apply this diff:
-$END$
-📝 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.
| $END$ |
🤖 Prompt for AI Agents
.cursorrules around lines 87-88: remove the literal "$END$" sentinel that was
inserted mid-file because it causes downstream tooling to stop parsing and
truncate the remainder of the rules; delete that line (and any stray whitespace
or blank line it creates) so the file is continuous and parsing continues into
the "Self App Development Rules" section.
| version: 2 | ||
| exclusion_globs: | ||
| - "common/src/mock_certificates/**" | ||
| - "common/src/constants/mockCertificates.ts" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Validate ggshield config key and scope of exclusions to prevent blind spots
Excluding mock certificate paths is reasonable to reduce false positives. However:
- Ensure the key name "exclusion_globs" is valid for .gitguardian.yml version: 2 (ggshield). A wrong key silently disables your intent.
- Confirm the globs aren’t overly broad and won’t mask real secrets if non-mock materials get added under similarly named paths.
If needed, I can align this file with the official ggshield schema (including per-scan-type sections) and mirror the same exclusions in GitLeaks to keep tooling consistent.
🌐 Web query:
Is "exclusion_globs" a valid key for ggshield's .gitguardian.yml with version: 2, and what is the recommended way to exclude paths from secret scanning?
💡 Result:
"exclusion_globs" is not a valid key for ggshield's .gitguardian.yml configuration file with version: 2; the recommended way to exclude paths from secret scanning is to use the ignored-paths key.
The official configuration documentation and multiple sources indicate that path exclusion in ggshield YAML (version 2) is managed using the ignored-paths key, which expects a list of glob patterns—these patterns let you specify files or directories to ignore during secret scanning[5][3]. For example:
version: 2
ignored-paths:
- '**/README.md'
- 'doc/*'
- 'LICENSE'This configuration will cause ggshield to skip all matching files or directories when scanning for secrets[5][3].
There is no mention of an exclusion_globs key in the configuration specification or in example configs from the official documentation, PyPI, or public cheat-sheets[5][3][1]. The authoritative sources provide ignored-paths as the correct and supported way to handle exclusions.
To exclude paths from secret scanning:
- Add glob patterns to the
ignored-pathslist in your.gitguardian.yml(version 2 format). - Alternatively, use the
--excludeCLI option for one-off exclusions if you do not want to use the YAML config[5].
Example:
version: 2
ignored-paths:
- '**/node_modules/**'
- 'tests/**'
- '*.md'Summary Table
| Key | Supported? | Recommended for exclusions? |
|---|---|---|
| exclusion_globs | No | Not documented or supported |
| ignored-paths | Yes | Official method—list of glob patterns |
If you need to exclude paths, always use the ignored-paths key in your configuration[5][3][1].
Citations:
- 1: https://docs.gitguardian.com/ggshield-docs/configuration
- 2: https://blog.gitguardian.com/how-to-use-ggshield-to-avoid-hardcoded-secrets-cheat-sheet-included/
- 3: https://pypi.org/project/ggshield/1.4.0/
- 4: https://www.youtube.com/watch?v=8bDKn3y7Br4
- 5: https://pypi.org/project/ggshield/1.13.3/
- 6: https://docs.gitguardian.com/ggshield-docs/integrations/git-hooks/pre-receive
- 7: https://github.com/GitGuardian/ggshield
- 8: https://docs.gitguardian.com/releases/self-hosted
- 9: https://www.youtube.com/watch?v=ZzzPSfqla_U
- 10: https://circleci.com/blog/detect-hardcoded-secrets-with-gitguardian/
Update ggshield config: replace invalid exclusion_globs with ignored-paths
The key exclusion_globs isn’t recognized in .gitguardian.yml version 2, so your intended exclusions won’t be applied. Switch to the supported ignored-paths key to ensure mock certificates are skipped without masking real secrets.
• File: .gitguardian.yml (lines 1–4)
• Replace the unsupported block with the following:
version: 2
-exclusion_globs:
- - "common/src/mock_certificates/**"
- - "common/src/constants/mockCertificates.ts"
+ignored-paths:
+ - "common/src/mock_certificates/**"
+ - "common/src/constants/mockCertificates.ts"📝 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.
| version: 2 | |
| exclusion_globs: | |
| - "common/src/mock_certificates/**" | |
| - "common/src/constants/mockCertificates.ts" | |
| version: 2 | |
| ignored-paths: | |
| - "common/src/mock_certificates/**" | |
| - "common/src/constants/mockCertificates.ts" |
🤖 Prompt for AI Agents
In .gitguardian.yml around lines 1 to 4, the config uses the unsupported key
`exclusion_globs`; replace that block with the supported `ignored-paths` key and
add the same glob patterns under it (e.g., common/src/mock_certificates/** and
common/src/constants/mockCertificates.ts) preserving YAML indentation and
version: 2 so GitGuardian will actually ignore those paths.
| inputs: | ||
| path: | ||
| description: Paths to cache | ||
| required: false | ||
| default: vendor/bundle | ||
| lock-file: | ||
| description: Path to Gemfile.lock | ||
| required: false | ||
| default: Gemfile.lock | ||
| cache-version: | ||
| description: Additional cache version segment | ||
| required: false | ||
| default: v1 | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Scope cache by Ruby version to avoid cross-version native gem conflicts
Caching gems across different Ruby versions can restore incompatible native extensions leading to flaky builds or forced reinstalls. Include Ruby version in the cache key (plumbed via an input).
inputs:
path:
description: Paths to cache
required: false
default: vendor/bundle
lock-file:
description: Path to Gemfile.lock
required: false
default: Gemfile.lock
cache-version:
description: Additional cache version segment
required: false
default: v1
+ ruby-version:
+ description: Ruby version to scope cache (e.g., 3.3.x)
+ required: false
+ default: ""
@@
with:
path: ${{ inputs.path }}
- key: ${{ runner.os }}-gems-${{ inputs.cache-version }}-${{ hashFiles(inputs['lock-file']) }}
+ key: ${{ runner.os }}-ruby-${{ inputs['ruby-version'] }}-gems-${{ inputs.cache-version }}-${{ hashFiles(inputs['lock-file']) }}
restore-keys: |
- ${{ runner.os }}-gems-${{ inputs.cache-version }}-
- ${{ runner.os }}-gems-
+ ${{ runner.os }}-ruby-${{ inputs['ruby-version'] }}-gems-${{ inputs.cache-version }}-
+ ${{ runner.os }}-ruby-${{ inputs['ruby-version'] }}-gems-
+ ${{ runner.os }}-gems-${{ inputs.cache-version }}-
+ ${{ runner.os }}-gems-Also applies to: 31-35
🤖 Prompt for AI Agents
.github/actions/cache-bundler/action.yml around lines 5-18 (also apply same
change to lines 31-35): the cache key is not scoped by Ruby version which can
restore incompatible native gems across different Ruby runtimes; add a new input
(e.g., ruby-version) that accepts the Ruby version string (required: false,
default: output of `ruby -v` or a placeholder like 2.7.0) and update the cache
key generation to include this input (append it as a segment alongside
cache-version) so cache entries are partitioned per Ruby version; ensure the
input is documented and defaulted so existing workflows remain backward
compatible.
| public override func trackEvent(_ name: String, properties: Properties? = nil) { | ||
| super.trackEvent(name, properties: properties) | ||
|
|
||
| print("[NFC Analytics] Event: \(name), Properties: \(properties ?? [:])") | ||
|
|
||
| if let logger = NativeLoggerBridge.shared { | ||
| logger.sendEvent(withName: "logEvent", body: [ | ||
| "level": "info", | ||
| "category": "NFC", | ||
| "message": "Analytics Event: \(name)", | ||
| "data": properties ?? [:] | ||
| ]) | ||
| } | ||
| } |
There was a problem hiding this comment.
Unconditional event logging can leak PII and add overhead in production; gate and sanitize.
trackEvent prints to console and forwards full properties to JS via NativeLoggerBridge on every event. Properties may include sensitive data. Gate logs behind enableDebugLogs (or DEBUG) and sanitize payloads.
Apply this diff within the selected lines:
public override func trackEvent(_ name: String, properties: Properties? = nil) {
super.trackEvent(name, properties: properties)
-
- print("[NFC Analytics] Event: \(name), Properties: \(properties ?? [:])")
-
- if let logger = NativeLoggerBridge.shared {
- logger.sendEvent(withName: "logEvent", body: [
- "level": "info",
- "category": "NFC",
- "message": "Analytics Event: \(name)",
- "data": properties ?? [:]
- ])
- }
+ if enableDebugLogs {
+ print("[NFC Analytics] Event: \(name), Properties: \(properties ?? [:])")
+ }
+ if let logger = NativeLoggerBridge.shared {
+ logger.sendEvent(withName: "logEvent", body: [
+ "level": "info",
+ "category": "NFC",
+ "message": "Analytics Event: \(name)",
+ "data": sanitizedProperties(properties)
+ ])
+ }
}Add this helper to sanitize known sensitive keys (outside the selected range, e.g., near the bottom of the file):
private func sanitizedProperties(_ dict: Properties?) -> [String: Any] {
let sensitiveKeys: Set<String> = [
"passportNumber", "dateOfBirth", "dateOfExpiry", "canNumber",
"firstName", "lastName", "phoneNumber", "residenceAddress",
"personalNumber", "passportMRZ"
]
guard let props = dict as? [String: Any] else { return [:] }
return props.reduce(into: [String: Any]()) { acc, kv in
acc[kv.key] = sensitiveKeys.contains(kv.key) ? "[redacted]" : kv.value
}
}🤖 Prompt for AI Agents
In app/ios/SelfAnalytics.swift around lines 15-28, the current trackEvent
unconditionally prints and forwards full properties which can leak PII and cause
overhead; gate console/log forwarding behind a debug flag (e.g., enableDebugLogs
or #if DEBUG) and replace raw properties with a sanitizedProperties(_:) result
before printing or sending to NativeLoggerBridge; add the provided
sanitizedProperties helper near the bottom of the file, then change the print
and logger.sendEvent body to use sanitizedProperties(properties) and only
execute them when debugging is enabled.
| public override func trackError(_ error: Error, context: String) { | ||
| super.trackError(error, context: context) | ||
|
|
||
| print("[NFC Analytics Error] Context: \(context), Error: \(error.localizedDescription)") | ||
|
|
||
| if let logger = NativeLoggerBridge.shared { | ||
| logger.sendEvent(withName: "logEvent", body: [ | ||
| "level": "error", | ||
| "category": "NFC", | ||
| "message": "Analytics Error: \(error.localizedDescription)", | ||
| "data": [ | ||
| "error_type": String(describing: type(of: error)), | ||
| "error_description": error.localizedDescription, | ||
| "context": context | ||
| ] | ||
| ]) | ||
| } | ||
| } |
There was a problem hiding this comment.
Error logs may expose sensitive details; avoid console prints in prod.
Keep NativeLoggerBridge error events, but avoid unconditional print statements to console in production.
Apply this diff:
public override func trackError(_ error: Error, context: String) {
super.trackError(error, context: context)
-
- print("[NFC Analytics Error] Context: \(context), Error: \(error.localizedDescription)")
-
+ if enableDebugLogs {
+ print("[NFC Analytics Error] Context: \(context), Error: \(error.localizedDescription)")
+ }
if let logger = NativeLoggerBridge.shared {
logger.sendEvent(withName: "logEvent", body: [
"level": "error",
"category": "NFC",
"message": "Analytics Error: \(error.localizedDescription)",
"data": [
"error_type": String(describing: type(of: error)),
- "error_description": error.localizedDescription,
+ "error_description": error.localizedDescription,
"context": context
]
])
}
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/ios/SelfAnalytics.swift around lines 47 to 64, there is an unconditional
console print of the error which may expose sensitive details in production;
keep the NativeLoggerBridge event but remove or guard the print so it only runs
in non-production builds. Wrap the print call in a compile-time check (e.g. #if
DEBUG ... #endif) or remove it entirely, leaving the existing
logger.sendEvent(...) intact so production builds do not write error details to
the console.
| moduleNameMapper: { | ||
| '^@env$': '<rootDir>/tests/__setup__/@env.js', | ||
| '\\.svg$': '<rootDir>/tests/__setup__/svgMock.js', | ||
| '^@/(.*)$': '<rootDir>/src/$1', | ||
| '^@$': '<rootDir>/src', | ||
| '^@tests/(.*)$': '<rootDir>/tests/src/$1', | ||
| '^@tests$': '<rootDir>/tests/src', | ||
| }, |
There was a problem hiding this comment.
Jest cannot resolve monorepo packages without moduleNameMapper entries
You intentionally do not mock @selfxyz/mobile-sdk-alpha (see jest.setup.js). Without a mapper, Jest won’t resolve it (or @selfxyz/common) from the monorepo during tests, leading to “Cannot find module …” or ESM parsing errors.
Add mappers pointing to the built outputs so tests run against real packages:
moduleNameMapper: {
'^@env$': '<rootDir>/tests/__setup__/@env.js',
'\\.svg$': '<rootDir>/tests/__setup__/svgMock.js',
'^@/(.*)$': '<rootDir>/src/$1',
'^@$': '<rootDir>/src',
'^@tests/(.*)$': '<rootDir>/tests/src/$1',
'^@tests$': '<rootDir>/tests/src',
+ '^@selfxyz/mobile-sdk-alpha(.*)$': '<rootDir>/../packages/mobile-sdk-alpha/dist$1',
+ '^@selfxyz/common(.*)$': '<rootDir>/../common/dist$1',
},If your tests rely on ESM builds, update the targets above to dist/esm accordingly.
🤖 Prompt for AI Agents
In app/jest.config.cjs around lines 11 to 18, add moduleNameMapper entries for
the monorepo packages so Jest can resolve them during tests: map
'@selfxyz/mobile-sdk-alpha' and '@selfxyz/common' to their built output paths
(e.g. <rootDir>/../packages/mobile-sdk-alpha/dist/cjs or dist/esm and
<rootDir>/../packages/common/dist/cjs or dist/esm as appropriate), and if your
tests require ESM change the targets to the dist/esm builds; ensure the paths
are relative from the Jest root and match the actual package build outputs.
| // Note: @selfxyz/mobile-sdk-alpha is NOT mocked to allow testing real package methods | ||
| // This is intentional for the mobile-sdk-alpha migration testing | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Not mocking @selfxyz/mobile-sdk-alpha requires Jest resolution to real package
Per this note, Jest must resolve the monorepo package. Ensure the moduleNameMapper additions exist (see comment in app/jest.config.cjs) or tests will fail with “Cannot find module”.
Run this to confirm imports and mapping are in place:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for imports of @selfxyz/mobile-sdk-alpha and @selfxyz/common..."
rg -nP -C2 "from\s+['\"]@selfxyz/(mobile-sdk-alpha|common)['\"]|require\(['\"]@selfxyz/(mobile-sdk-alpha|common)['\"]\)" app
echo
echo "Checking Jest mappers..."
rg -nP "moduleNameMapper:\s*{[^}]*@selfxyz\/mobile-sdk-alpha|@selfxyz\/common" app/jest.config.cjs || echo "Mappers not found"
echo
echo "Do we still import deprecated Google Sign-In anywhere?"
rg -nP -C2 "@react-native-google-signin/google-signin|@react-native-google-signin\b" -g '!**/node_modules/**' -g '!**/dist/**' || echo "No references found"Length of output: 8464
Ensure Jest config maps @selfxyz packages (tests will break otherwise)
We have multiple direct imports of @selfxyz/mobile-sdk-alpha and @selfxyz/common (in app/src and tests), but no corresponding moduleNameMapper entries in app/jest.config.cjs. Without these, Jest will error “Cannot find module”.
Please update app/jest.config.cjs:
moduleNameMapper: {
+ '^@selfxyz/common/(.*)$': '<rootDir>/../common/src/$1',
+ '^@selfxyz/mobile-sdk-alpha/(.*)$': '<rootDir>/../mobile-sdk-alpha/src/$1',
// …existing mappers
},No deprecated Google Sign-In imports were found in source code.
Committable suggestion skipped: line range outside the PR's diff.
| Checkout our [coverage map here](http://map.self.xyz/). | ||
|
|
There was a problem hiding this comment.
Use HTTPS for external links to avoid MITM and mixed-content issues
The coverage map links use HTTP. If the site supports HTTPS, switch to it to prevent downgrade/mitM risks and mixed-content blocking when embedded.
Apply this diff:
-Checkout our [coverage map here](http://map.self.xyz/).
+Checkout our [coverage map here](https://map.self.xyz/).
-Countries use different signature algorithms to sign ID documents. Check out our [coverage map](http://map.self.xyz/) to see which.
+Countries use different signature algorithms to sign ID documents. Check out our [coverage map](https://map.self.xyz/) to see which.Also applies to: 44-45
🤖 Prompt for AI Agents
In README.md around lines 26-27 (and also lines 44-45), update the external
coverage map links that currently use http to use https instead; replace
"http://map.self.xyz/" with "https://map.self.xyz/" (and any other http
occurrences on the specified lines), and verify the HTTPS URL resolves
successfully so there are no mixed-content or MITM issues when embedded.
| interface VerificationConfig { | ||
| circuitVersion: string; // Semantic versioning | ||
| complianceRules: ComplianceRule[]; | ||
| timeWindow: number; // 24-hour window in seconds | ||
| clockDrift: number; // ±5 minutes tolerance | ||
| trustAnchors: string[]; // Trusted certificate authority roots | ||
| revocationRoots: string[]; // Certificate revocation list roots | ||
| timeSource: string; // NTP server for time validation | ||
| nullifierScope: string; // Domain separation for nullifiers | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align time source guidance with mobile constraints: avoid direct NTP.
Direct NTP from mobile apps is often blocked/unreliable (UDP, captive portals) and conflicts with the compliance doc’s “trusted time tokens” guidance. Prefer server-signed time tokens or blockchain time anchors.
Apply this doc change (comment-only, non-breaking) to clarify:
interface VerificationConfig {
circuitVersion: string; // Semantic versioning
complianceRules: ComplianceRule[];
timeWindow: number; // 24-hour window in seconds
clockDrift: number; // ±5 minutes tolerance
trustAnchors: string[]; // Trusted certificate authority roots
revocationRoots: string[]; // Certificate revocation list roots
- timeSource: string; // NTP server for time validation
+ timeSource: string; // Trusted time authority (server-signed time token or chain block time); avoid direct NTP from mobile clients
nullifierScope: string; // Domain separation for nullifiers
}📝 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.
| interface VerificationConfig { | |
| circuitVersion: string; // Semantic versioning | |
| complianceRules: ComplianceRule[]; | |
| timeWindow: number; // 24-hour window in seconds | |
| clockDrift: number; // ±5 minutes tolerance | |
| trustAnchors: string[]; // Trusted certificate authority roots | |
| revocationRoots: string[]; // Certificate revocation list roots | |
| timeSource: string; // NTP server for time validation | |
| nullifierScope: string; // Domain separation for nullifiers | |
| } | |
| ``` | |
| interface VerificationConfig { | |
| circuitVersion: string; // Semantic versioning | |
| complianceRules: ComplianceRule[]; | |
| timeWindow: number; // 24-hour window in seconds | |
| clockDrift: number; // ±5 minutes tolerance | |
| trustAnchors: string[]; // Trusted certificate authority roots | |
| revocationRoots: string[]; // Certificate revocation list roots | |
| timeSource: string; // Trusted time authority (server-signed time token or chain block time); avoid direct NTP from mobile clients | |
| nullifierScope: string; // Domain separation for nullifiers | |
| } |
🤖 Prompt for AI Agents
In .cursor/rules/technical-specification.mdc around lines 58 to 68, update the
comment for the timeSource field to clarify that mobile clients must not rely on
direct NTP (UDP/captive-portal issues) and instead reference trusted
server-signed time tokens or blockchain time anchors as the canonical time
source; make this a non-breaking, comment-only clarification (no schema change),
explicitly mention that timeSource should identify a trusted time-token provider
or anchor and that nullifier/time validation must use server-signed tokens or
anchors rather than direct NTP from mobile.
| - id: cache | ||
| name: Cache Pods | ||
| uses: actions/cache@v4 | ||
| with: | ||
| path: ${{ inputs.path }} | ||
| key: ${{ runner.os }}-pods-${{ inputs.cache-version }}-${{ hashFiles(inputs.lock-file) }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-pods-${{ inputs.cache-version }}- | ||
| ${{ runner.os }}-pods- |
There was a problem hiding this comment.
Pin actions/cache to a commit SHA for security.
Composite action uses actions/cache@v4 by tag. Pin this to a specific commit SHA to reduce supply-chain risk. This action is used across CI and controls restoration of cached artifacts.
Example:
- uses: actions/cache@v4
+ uses: actions/cache@0c45773b623bea8c8e75f6c82b208c3cf94ea4f9 # v4.0.2🤖 Prompt for AI Agents
.github/actions/cache-pods/action.yml around lines 29-37: the composite action
is referencing actions/cache by tag (actions/cache@v4); replace the tag with a
specific commit SHA (actions/cache@<full-commit-sha>) to pin the dependency and
reduce supply-chain risk, update the uses field to that SHA, and optionally add
a short comment with the upstream tag and date for maintainability.
| - "common/**" | ||
| jobs: | ||
| run_circuit_tests: | ||
| if: github.event.pull_request.draft == false |
There was a problem hiding this comment.
Job condition skips push events unintentionally
The condition references pull_request.draft even for push events, causing the job to be skipped on pushes. Ensure tests run on pushes to dev/main.
Apply this diff:
- if: github.event.pull_request.draft == false
+ if: ${{ github.event_name != 'pull_request' || github.event.pull_request.draft == false }}📝 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.
| if: github.event.pull_request.draft == false | |
| if: ${{ github.event_name != 'pull_request' || github.event.pull_request.draft == false }} |
🤖 Prompt for AI Agents
.github/workflows/circuits.yml around line 21: the current condition "if:
github.event.pull_request.draft == false" references pull_request fields for all
events and therefore skips the job on push events; replace it with a condition
that only checks draft status for pull_request events and allows push events
through, e.g. use a compound expression that permits pushes and non-draft PRs
(github.event_name == 'push' || (github.event_name == 'pull_request' &&
github.event.pull_request.draft == false)), so tests run on pushes to dev/main
while still skipping draft PRs.
| id: circom-cache | ||
| uses: actions/cache/restore@v4 | ||
| with: | ||
| path: ~/.cache/circom | ||
| key: circom-v2.1.9 | ||
|
|
||
| - name: Download Circom Binary v2.1.9 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Cache key should include OS/version/hash to avoid cross-runner pollution and ensure integrity
Using a generic key risks reusing a wrong binary across environments and versions. Include OS, version, and checksum in the key for precise caching.
Apply this diff:
- with:
- path: ~/.cache/circom
- key: circom-v2.1.9
+ with:
+ path: ~/.cache/circom
+ key: circom-${{ runner.os }}-v${{ env.CIRCOM_VERSION }}-${{ env.CIRCOM_SHA256 }}- with:
- path: ~/.cache/circom
- key: circom-v2.1.9
+ with:
+ path: ~/.cache/circom
+ key: circom-${{ runner.os }}-v${{ env.CIRCOM_VERSION }}-${{ env.CIRCOM_SHA256 }}Also applies to: 100-106
🤖 Prompt for AI Agents
.github/workflows/circuits.yml around lines 46-52 (and also update 100-106): the
cache key is too generic and may cause cross-runner/OS pollution; change the key
to include runner OS, circom version, and a checksum/hash of the binary or
source used to build it (e.g., include matrix.os, the circom version variable,
and a computed checksum like sha256 of the downloaded tarball) so each
runner/version gets a unique cache entry; update both cache steps to construct
the key from OS/version/checksum and the restore/save steps to use the same
composite key and a fallback restore key if desired.
| if: steps.circom-cache.outputs.cache-hit == 'true' | ||
| run: | | ||
| wget -qO /home/runner/work/circom https://github.com/iden3/circom/releases/download/v2.1.9/circom-linux-amd64 | ||
| chmod +x /home/runner/work/circom | ||
| sudo mv /home/runner/work/circom /bin/circom | ||
| echo "${{ env.CIRCOM_SHA256 }} $HOME/.cache/circom/circom" | sha256sum -c - | ||
|
|
||
| - name: Add Circom to PATH |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure cached circom binary is executable on cache hit
Execute bit may not survive caching consistently. Add chmod to avoid “Permission denied” at circom --version.
Apply this diff:
- name: Verify Circom checksum (cache hit)
if: steps.circom-cache.outputs.cache-hit == 'true'
run: |
echo "${{ env.CIRCOM_SHA256 }} $HOME/.cache/circom/circom" | sha256sum -c -
+ chmod +x "$HOME/.cache/circom/circom"📝 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.
| - name: Verify Circom checksum (cache hit) | |
| if: steps.circom-cache.outputs.cache-hit == 'true' | |
| run: | | |
| wget -qO /home/runner/work/circom https://github.com/iden3/circom/releases/download/v2.1.9/circom-linux-amd64 | |
| chmod +x /home/runner/work/circom | |
| sudo mv /home/runner/work/circom /bin/circom | |
| echo "${{ env.CIRCOM_SHA256 }} $HOME/.cache/circom/circom" | sha256sum -c - | |
| - name: Verify Circom checksum (cache hit) | |
| if: steps.circom-cache.outputs.cache-hit == 'true' | |
| run: | | |
| echo "${{ env.CIRCOM_SHA256 }} $HOME/.cache/circom/circom" | sha256sum -c - | |
| chmod +x "$HOME/.cache/circom/circom" |
🤖 Prompt for AI Agents
In .github/workflows/circuits.yml around lines 107-111, the workflow verifies
the cached circom binary but doesn't ensure the executable bit, which can be
lost in cache; update the cache-hit branch to set the executable permission (run
chmod +x $HOME/.cache/circom/circom) before any circom usage (and you can place
it just before the existing sha256sum check or before any circom --version
invocation) so the cached binary won’t produce “Permission denied”.
| def request_upload_url(slack_token, file_name, file_size) | ||
| upload_url = nil | ||
| file_id = nil | ||
| with_retry(max_retries: 3, delay: 5) do | ||
| uri = URI.parse("https://slack.com/api/files.getUploadURLExternal") | ||
| request = Net::HTTP::Post.new(uri) | ||
| request["Authorization"] = "Bearer #{slack_token}" | ||
| request.set_form_data(filename: file_name, length: file_size) | ||
| http = Net::HTTP.new(uri.host, uri.port) | ||
| http.use_ssl = true | ||
| response = http.request(request) | ||
|
|
||
| # Handle rate limiting specifically | ||
| if response.code == "429" | ||
| raise "HTTP 429 Rate limit exceeded for Slack API" | ||
| end | ||
|
|
||
| raise "Slack API failed: #{response.code} #{response.body}" unless response.is_a?(Net::HTTPSuccess) | ||
| json = JSON.parse(response.body) | ||
| raise "Slack API Error: #{json["error"]}" unless json["ok"] | ||
| upload_url = json["upload_url"] | ||
| file_id = json["file_id"] | ||
| end | ||
| [upload_url, file_id] | ||
| end |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add network timeouts for Slack API calls
Net::HTTP defaults can hang indefinitely on network stalls. Add open/read timeouts for resilience.
http = Net::HTTP.new(uri.host, uri.port)
http.use_ssl = true
+ http.open_timeout = 10
+ http.read_timeout = 30
response = http.request(request)Apply similarly to the complete_upload HTTP client:
- complete_http = Net::HTTP.new(complete_uri.host, complete_uri.port)
- complete_http.use_ssl = true
+ complete_http = Net::HTTP.new(complete_uri.host, complete_uri.port)
+ complete_http.use_ssl = true
+ complete_http.open_timeout = 10
+ complete_http.read_timeout = 30Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 RuboCop (1.76.1)
[convention] 30-54: Assignment Branch Condition size for request_upload_url is too high. [<11, 23, 4> 25.81/23]
(Metrics/AbcSize)
🤖 Prompt for AI Agents
In app/fastlane/helpers/slack.rb around lines 30 to 54, the Net::HTTP client in
request_upload_url has no timeouts and can hang; set explicit open_timeout and
read_timeout on the Net::HTTP instance (e.g. open_timeout = 5, read_timeout =
30) before calling request, and propagate the same timeout configuration to the
complete_upload HTTP client as well so both Slack API calls fail fast on network
stalls.
| def upload_file_content(upload_url, file_path, file_size) | ||
| with_retry(max_retries: 3, delay: 5) do | ||
| upload_uri = URI.parse(upload_url) | ||
| upload_request = Net::HTTP::Post.new(upload_uri) | ||
| upload_request.body = File.binread(file_path) | ||
| upload_request["Content-Type"] = "application/octet-stream" | ||
| upload_request["Content-Length"] = file_size.to_s | ||
| upload_http = Net::HTTP.new(upload_uri.host, upload_uri.port) | ||
| upload_http.use_ssl = true | ||
| upload_response = upload_http.request(upload_request) | ||
|
|
||
| # Handle rate limiting specifically | ||
| if upload_response.code == "429" | ||
| raise "HTTP 429 Rate limit exceeded for file upload" | ||
| end | ||
|
|
||
| raise "File upload failed: #{upload_response.code} #{upload_response.message}" unless upload_response.is_a?(Net::HTTPOK) | ||
| end | ||
| end |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid loading entire file into memory and accept 2xx responses
Reading the whole file into memory is risky for large artifacts (OOM risk). Also, pre-signed upload endpoints often return 204 No Content on success; restricting to 200 OK can cause false failures.
def upload_file_content(upload_url, file_path, file_size)
with_retry(max_retries: 3, delay: 5) do
upload_uri = URI.parse(upload_url)
- upload_request = Net::HTTP::Post.new(upload_uri)
- upload_request.body = File.binread(file_path)
- upload_request["Content-Type"] = "application/octet-stream"
- upload_request["Content-Length"] = file_size.to_s
- upload_http = Net::HTTP.new(upload_uri.host, upload_uri.port)
- upload_http.use_ssl = true
- upload_response = upload_http.request(upload_request)
+ upload_request = Net::HTTP::Post.new(upload_uri)
+ upload_request["Content-Type"] = "application/octet-stream"
+ upload_request["Content-Length"] = file_size.to_s
+ upload_http = Net::HTTP.new(upload_uri.host, upload_uri.port)
+ upload_http.use_ssl = true
+ upload_http.open_timeout = 30
+ upload_http.read_timeout = 300
+ File.open(file_path, "rb") do |io|
+ upload_request.body_stream = io
+ upload_response = upload_http.request(upload_request)
+ # Accept any 2xx as success (some endpoints return 204)
+ unless upload_response.code.to_i.between?(200, 299)
+ raise "File upload failed: #{upload_response.code} #{upload_response.message}"
+ end
+ end
- raise "File upload failed: #{upload_response.code} #{upload_response.message}" unless upload_response.is_a?(Net::HTTPOK)
end
end📝 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.
| def upload_file_content(upload_url, file_path, file_size) | |
| with_retry(max_retries: 3, delay: 5) do | |
| upload_uri = URI.parse(upload_url) | |
| upload_request = Net::HTTP::Post.new(upload_uri) | |
| upload_request.body = File.binread(file_path) | |
| upload_request["Content-Type"] = "application/octet-stream" | |
| upload_request["Content-Length"] = file_size.to_s | |
| upload_http = Net::HTTP.new(upload_uri.host, upload_uri.port) | |
| upload_http.use_ssl = true | |
| upload_response = upload_http.request(upload_request) | |
| # Handle rate limiting specifically | |
| if upload_response.code == "429" | |
| raise "HTTP 429 Rate limit exceeded for file upload" | |
| end | |
| raise "File upload failed: #{upload_response.code} #{upload_response.message}" unless upload_response.is_a?(Net::HTTPOK) | |
| end | |
| end | |
| def upload_file_content(upload_url, file_path, file_size) | |
| with_retry(max_retries: 3, delay: 5) do | |
| upload_uri = URI.parse(upload_url) | |
| upload_request = Net::HTTP::Post.new(upload_uri) | |
| upload_request["Content-Type"] = "application/octet-stream" | |
| upload_request["Content-Length"] = file_size.to_s | |
| upload_http = Net::HTTP.new(upload_uri.host, upload_uri.port) | |
| upload_http.use_ssl = true | |
| upload_http.open_timeout = 30 | |
| upload_http.read_timeout = 300 | |
| File.open(file_path, "rb") do |io| | |
| upload_request.body_stream = io | |
| upload_response = upload_http.request(upload_request) | |
| # Accept any 2xx as success (some endpoints return 204) | |
| unless upload_response.code.to_i.between?(200, 299) | |
| raise "File upload failed: #{upload_response.code} #{upload_response.message}" | |
| end | |
| end | |
| # Handle rate limiting specifically | |
| if upload_response.code == "429" | |
| raise "HTTP 429 Rate limit exceeded for file upload" | |
| end | |
| end | |
| end |
| assert_raises(FastlaneCore::Interface::FastlaneCommonException) do | ||
| Fastlane::Helpers.upload_file_to_slack(file_path: file.path, channel_id: "") | ||
| end | ||
| ensure | ||
| file.unlink | ||
| ENV.delete("SLACK_API_TOKEN") | ||
| end | ||
|
|
||
| def test_upload_file_to_slack_missing_token | ||
| ENV.delete("SLACK_API_TOKEN") | ||
| file = Tempfile.new(["artifact", ".txt"]) | ||
| file.write("data") | ||
| file.close | ||
|
|
||
| assert_raises(FastlaneCore::Interface::FastlaneCommonException) do | ||
| Fastlane::Helpers.upload_file_to_slack(file_path: file.path, channel_id: "C123") | ||
| end | ||
| ensure |
There was a problem hiding this comment.
Fix expected exception type for Slack error paths.
report_error calls UI.abort_with_message!, which raises FastlaneCore::Interface::FastlaneError. Tests currently expect FastlaneCommonException, leading to false failures.
Apply this diff:
- assert_raises(FastlaneCore::Interface::FastlaneCommonException) do
+ assert_raises(FastlaneCore::Interface::FastlaneError) do
Fastlane::Helpers.upload_file_to_slack(file_path: file.path, channel_id: "")
end
...
- assert_raises(FastlaneCore::Interface::FastlaneCommonException) do
+ assert_raises(FastlaneCore::Interface::FastlaneError) do
Fastlane::Helpers.upload_file_to_slack(file_path: file.path, channel_id: "C123")
endIf you prefer to keep FastlaneCommonException, change report_error to use UI.user_error! instead; align tests and implementation consistently either way.
📝 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.
| assert_raises(FastlaneCore::Interface::FastlaneCommonException) do | |
| Fastlane::Helpers.upload_file_to_slack(file_path: file.path, channel_id: "") | |
| end | |
| ensure | |
| file.unlink | |
| ENV.delete("SLACK_API_TOKEN") | |
| end | |
| def test_upload_file_to_slack_missing_token | |
| ENV.delete("SLACK_API_TOKEN") | |
| file = Tempfile.new(["artifact", ".txt"]) | |
| file.write("data") | |
| file.close | |
| assert_raises(FastlaneCore::Interface::FastlaneCommonException) do | |
| Fastlane::Helpers.upload_file_to_slack(file_path: file.path, channel_id: "C123") | |
| end | |
| ensure | |
| assert_raises(FastlaneCore::Interface::FastlaneError) do | |
| Fastlane::Helpers.upload_file_to_slack(file_path: file.path, channel_id: "") | |
| end | |
| ensure | |
| file.unlink | |
| ENV.delete("SLACK_API_TOKEN") | |
| end | |
| def test_upload_file_to_slack_missing_token | |
| ENV.delete("SLACK_API_TOKEN") | |
| file = Tempfile.new(["artifact", ".txt"]) | |
| file.write("data") | |
| file.close | |
| assert_raises(FastlaneCore::Interface::FastlaneError) do | |
| Fastlane::Helpers.upload_file_to_slack(file_path: file.path, channel_id: "C123") | |
| end | |
| ensure |
🤖 Prompt for AI Agents
In app/fastlane/test/helpers_test.rb around lines 339 to 356, the tests
assert_raises(FastlaneCore::Interface::FastlaneCommonException) for Slack error
paths but the code uses report_error which calls UI.abort_with_message! and
raises FastlaneCore::Interface::FastlaneError; update the tests to
assert_raises(FastlaneCore::Interface::FastlaneError) for those blocks (or
alternatively change report_error to use UI.user_error! and keep the tests
as-is) so test expectation matches the actual exception type.
| "@sentry/react": "^9.32.0", | ||
| "@sentry/react-native": "7.0.0-beta.1", | ||
| "@stablelib/cbor": "^2.0.1", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Avoid shipping both @sentry/react-native (beta) and @sentry/react; pin to stable and enforce platform-specific imports
Carrying both Sentry packages increases bundle size and risks runtime conflicts; additionally, relying on a beta for production mobile is risky. Ensure mobile uses only @sentry/react-native and web uses @sentry/react via platform-specific entry points or aliasing, and prefer a stable Sentry release unless 7.x features are strictly required.
Recommended actions:
- Gate imports by platform (e.g., index.native.tsx uses @sentry/react-native, index.web.tsx uses @sentry/react).
- If you must keep both, verify the native bundle does not include @sentry/react and the web bundle does not include @sentry/react-native using:
- yarn analyze:bundle:android
- yarn analyze:bundle:ios
- Consider pinning @sentry/react-native to a stable version (6.x stable) pending 7.x GA. Do you want a PR-ready diff once you confirm the target versions?
🏁 Script executed:
#!/bin/bash
# Confirm the Sentry entries in app/package.json
echo "---- app/package.json (lines 80-110) ----"
sed -n '80,110p' app/package.json || echo "app/package.json not found at expected path"
# Find all occurrences of Sentry imports in the codebase
echo "---- @sentry/react-native imports ----"
rg -n "@sentry/react-native" -g "*.ts" -g "*.tsx" -g "*.js" -g "*.jsx" || echo "No @sentry/react-native imports found"
echo "---- @sentry/react imports ----"
rg -n "@sentry/react" -g "*.ts" -g "*.tsx" -g "*.js" -g "*.jsx" || echo "No @sentry/react imports found"
# Check for platform-specific entry points
echo "---- entry files matching index.*.tsx ----"
fd "index.*.tsx" || echo "No platform-specific entry files found"Length of output: 2249
🏁 Script executed:
#!/bin/bash
echo "---- app/src/Sentry.ts ----"
sed -n '1,200p' app/src/Sentry.ts || echo "Not found: app/src/Sentry.ts"
echo
echo "---- app/src/Sentry.web.ts ----"
sed -n '1,200p' app/src/Sentry.web.ts || echo "Not found: app/src/Sentry.web.ts"
echo
echo "---- app/src/navigation/index.tsx ----"
sed -n '1,200p' app/src/navigation/index.tsx || echo "Not found: app/src/navigation/index.tsx"Length of output: 6831
Refactor Vite config to exclude @sentry/react-native from your web bundle
It looks like you’ve already gated Sentry imports via app/src/Sentry.ts and app/src/Sentry.web.ts, so mobile will import @sentry/react-native and web will import @sentry/react. However, your Vite manual‐chunk definition is still bundling both packages together for web:
• In app/vite.config.ts (around line 129), change:
- 'vendor-analytics-sentry': ['@sentry/react', '@sentry/react-native'],
+ 'vendor-analytics-sentry': ['@sentry/react'],• If you need a separate chunk for @sentry/react-native on web (e.g. fallback), create a dedicated manual chunk instead of lumping them together.
• Run bundle analysis to confirm:
- yarn analyze:web
- yarn analyze:bundle:android
- yarn analyze:bundle:ios
Ensure only@sentry/reactis in your web bundle and only@sentry/react-nativein your native bundles.
• Since you’re on the 7.0.0-beta for mobile, plan to upgrade to a GA release (6.x or 7.x stable) as soon as it’s available to avoid shipping beta code in production.
🤖 Prompt for AI Agents
In app/vite.config.ts around line 129, the current manualChunks groups
@sentry/react and @sentry/react-native together which causes the native package
to be bundled into the web output; update the manualChunks config to place
@sentry/react and @sentry/react-native into separate named chunks (or exclude
@sentry/react-native entirely from the web build), ensure your Sentry import
gating (app/src/Sentry.ts and app/src/Sentry.web.ts) remains, then run the
recommended bundle analyses (yarn analyze:web, yarn analyze:bundle:android, yarn
analyze:bundle:ios) to verify only @sentry/react is in the web bundle and only
@sentry/react-native is in native bundles; additionally plan to bump
@sentry/react-native off the 7.0.0-beta to a GA release (6.x/7.x stable) before
shipping to production.
| "react-native-logs": "^5.3.0", | ||
| "react-native-nfc-manager": "^3.15.1", |
There was a problem hiding this comment.
Guard logs to avoid PII leakage in production
react-native-logs is useful, but verbose logs in production can leak PII and tokens. Ensure logging level is lowered to warn/error (or disabled) for release builds, and scrub sensitive payloads before logging.
- Wire the logger level to build variants or remote config with a strict upper bound on release.
- Add a sanitizer for auth headers, access tokens, QR content, and NFC payloads before logs.
| with: | ||
| token: ${{ github.token }} | ||
| fetch-depth: 0 | ||
| - name: Read and sanitize Node.js version |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use secrets.GITHUB_TOKEN for checkout token
github.token is not a documented context; this may result in an empty token and break pushing commits. Use the standard GITHUB_TOKEN secret.
- - uses: actions/checkout@v4
- with:
- token: ${{ github.token }}
- fetch-depth: 0
+ - uses: actions/checkout@v4
+ with:
+ token: ${{ secrets.GITHUB_TOKEN }}
+ fetch-depth: 0📝 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.
| with: | |
| token: ${{ github.token }} | |
| fetch-depth: 0 | |
| - name: Read and sanitize Node.js version | |
| - uses: actions/checkout@v4 | |
| with: | |
| token: ${{ secrets.GITHUB_TOKEN }} | |
| fetch-depth: 0 | |
| - name: Read and sanitize Node.js version |
🤖 Prompt for AI Agents
.github/workflows/mobile-deploy.yml around lines 837 to 840: the checkout action
is using the undocumented github.token context which can be empty; replace the
token input to use the standard secret by changing the checkout step to pass
token: ${{ secrets.GITHUB_TOKEN }} so the action uses the documented
GITHUB_TOKEN secret and will be able to push commits reliably.
|
|
||
| - name: Create GitHub Release | ||
| uses: softprops/action-gh-release@v1 | ||
| with: |
There was a problem hiding this comment.
Update action to Node 20 runtime (v1 is deprecated on GitHub runners)
softprops/action-gh-release@v1 uses an old Node runtime and is flagged by actionlint. Upgrade to v2.
- - name: Create GitHub Release
- uses: softprops/action-gh-release@v1
+ - name: Create GitHub Release
+ uses: softprops/action-gh-release@v2📝 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.
| uses: softprops/action-gh-release@v1 | |
| - name: Create GitHub Release | |
| uses: softprops/action-gh-release@v2 |
🧰 Tools
🪛 actionlint (1.7.7)
1004-1004: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
.github/workflows/mobile-deploy.yml around line 1004: the workflow uses
softprops/action-gh-release@v1 which relies on an outdated Node runtime; update
the action reference to softprops/action-gh-release@v2 to move to Node 20
runtime, update any inputs/outputs if the v2 version changed (check action
README for breaking changes), run actionlint or a dry run to verify the
workflow, and commit the updated uses line.
15 participants
Summary by CodeRabbit
New Features
Improvements
Documentation
Tests
Chores