Skip to content

Conversation

@transphorm
Copy link
Member

@transphorm transphorm commented Aug 11, 2025

Summary

  • expand migration checklist into structured chapters and link to detailed prompts
  • add migration prompts file with granular task stubs

Testing

  • yarn workspace @selfxyz/mobile-sdk-alpha nice
  • yarn workspace @selfxyz/common nice (fails: Require statement not part of import statement)
  • yarn workspace @selfxyz/app nice (workspace not found; ran @selfxyz/mobile-app nice instead)
  • yarn lint
  • yarn build
  • yarn workspace @selfxyz/contracts build (fails: Hardhat HH8 config errors)
  • yarn types
  • yarn workspace @selfxyz/mobile-sdk-alpha test

https://chatgpt.com/codex/tasks/task_b_689a35128b2c832db3120ad4d1b8ac83

Summary by CodeRabbit

  • Documentation
    • Replaced the flat Migration Checklist with a structured 13-section migration plan and added a companion, task-based migration guide with granular, actionable steps and cross-references.
    • Added partner guidance and prompts detailing integration, verification, branding, sample apps, and security/callback requirements.
  • New Features
    • Added partner requirements (bundle-size targets, US and Aadhaar support, runnable example, webhook/callback guidance) and clarified iOS OpenPassport URL scheme wording.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 11, 2025

Walkthrough

Replaces a flat migration checklist with a structured, task-driven migration plan (13 sections) and adds detailed migration and partner prompt documents plus partner feedback guidelines. All changes are documentation-only; no source code or public API modifications.

Changes

Cohort / File(s) Summary
Migration checklist rewritten
packages/mobile-sdk-alpha/MIGRATION_CHECKLIST.md
Replaced the prior top-level checklist with a 13-section, action-oriented migration plan; adds cross-reference to MIGRATION_PROMPTS.md; enumerates concrete tasks (processing helpers, validation, proof input, crypto adapters, TEE/session, attestation, protocol sync, artifact management, samples, SDK integration, lightweight in-SDK demo, partner feedback, iOS OpenPassport URL scheme clarification). Documentation-only.
Detailed migration prompts
packages/mobile-sdk-alpha/MIGRATION_PROMPTS.md
New file: granular, task-level blueprint covering scanner interfaces/adapters (MRZ/NFC), processing helpers, validation module, paginated protocol sync with TTL cache and root verification, proof-input generation, WebSocket-based TEE session wrapper (AbortSignal/timeout/progress), attestation and cert-chain checks, runtime crypto adapters, manifest/downloader with caching/verification, sample apps, SDK integration steps, and an embedded demo scaffold. Documentation-only.
Partner feedback & requirements
packages/mobile-sdk-alpha/PARTNER_FEEDBACK.md
New file: partner integration guidelines covering OAuth-style branding requirements, verification workflow expansions (US driver's licenses, Aadhaar), bundle-size and runnable example targets, iOS/Android min versions, device compatibility, and secure webhook/callback requirements (auth, retries, signature verification). Documentation-only.
Partner prompts
packages/mobile-sdk-alpha/PARTNER_PROMPTS.md
New file: partner-focused prompts and checklist for implementing branding props, US/state license adapters and tests, Aadhaar support specifics, bundle-size measurement/optimization guidance, runnable example app requirements, and callback/webhook security patterns (HMAC, retries, rate limiting). Documentation-only.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Suggested reviewers

  • remicolin
  • aaronmgdr

Poem

A mapped-out path in careful lines,
Prompts for scanners, certs, and signs.
Docs that steer each testing chore,
Hooks for partners, samples, more.
Read the plan — then ship and shine.

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex/expand-migration-checklist-for-mobile-sdk

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 15

🔭 Outside diff range comments (1)
packages/mobile-sdk-alpha/MIGRATION_CHECKLIST.md (1)

1-64: Cross-file link verified; convert :::task-stub blocks for GitHub compatibility

  • ✅ packages/mobile-sdk-alpha/MIGRATION_CHECKLIST.md correctly links to MIGRATION_PROMPTS.md
  • ⚠️ packages/mobile-sdk-alpha/MIGRATION_PROMPTS.md uses multiple nonstandard :::task-stub{…} containers (e.g. lines 7, 14, 21, 28, …) that GitHub won’t render
  • Please replace each stub with a collapsible <details> section. For example:
- :::task-stub{title="Create scanning adapter interface"}
+ <details>
+   <summary>Create scanning adapter interface</summary>

Then wrap the prompt content inside the <details>…</details> block to ensure proper rendering.

🧹 Nitpick comments (12)
packages/mobile-sdk-alpha/MIGRATION_PROMPTS.md (5)

16-19: MRZ adapter: permissions and platform caveats

  • iOS: NSCameraUsageDescription required; test on both simulators and real devices (MRZ scanning requires real device).
  • Android: Camera permission + handling background restrictions; consider frame throttling to reduce CPU.
  • Provide a frame-rate cap and backpressure handling to avoid battery drain.

39-41: MRZ fixtures: cite ICAO 9303 and sanitize

  • Link to ICAO Doc 9303 examples and ensure synthetic test data (no real PII).
  • Add fuzz tests for invalid lengths, checksums, and unexpected characters.
-2. Use sample MRZ strings from ICAO specs for fixtures.
+2. Use synthetic MRZ strings modeled on ICAO Doc 9303; include checksum edge cases and malformed inputs. Avoid real PII.

135-138: Certificate chain verification: environment-safe crypto and policy

  • Avoid Node-only APIs; rely on your CryptoAdapter.
  • Enforce EKU/policy OIDs where applicable.
  • Include negative tests: expired, wrong issuer, weak signatures.

175-178: Samples: guardrails for secrets and telemetry

  • Do not embed API keys, test certificates, or seed phrases.
  • Disable analytics/telemetry by default in samples.
  • Include a “Security Footguns” section in the sample README.

1-4: Add a Table of Contents and dependency graph

Improve navigability and parallelization by adding:

  • ToC linking to all 12 sections.
  • A dependency matrix (what must be done before each task).
packages/mobile-sdk-alpha/MIGRATION_CHECKLIST.md (7)

3-3: Cross-reference sections with anchors to MIGRATION_PROMPTS

Link each checklist section to its corresponding detailed prompt anchor for quick navigation.

-> Detailed task prompts are listed in [MIGRATION_PROMPTS.md](./MIGRATION_PROMPTS.md).
+> Detailed task prompts are listed in [MIGRATION_PROMPTS.md](./MIGRATION_PROMPTS.md). Each section below links to a matching prompt anchor for quick navigation.

5-10: Scanning: add explicit acceptance criteria and PII policy

Include checkboxes:

  • No MRZ/NFC data in logs
  • Single active NFC session with cleanup
  • iOS/Android permissions configured and verified on devices

31-33: TEE session: add heartbeat and reconnection policy to checklist

Add criteria:

  • Heartbeat (ping/pong) interval configured
  • Reconnect capped with jitter
  • EventEmitter listeners cleaned on close/error/abort

36-38: Attestation: trust anchors and time validation

Checklist additions:

  • Attestation time within skew window
  • Trust anchors documented and pinned
  • Optional OCSP/CRL policy defined

51-53: URL scheme: add Android intent filters and collision considerations

Add a bullet for Android and scheme uniqueness across installed apps to prevent routing conflicts.


56-59: Integration gating: make CI checks explicit

Add gating items:

  • yarn lint
  • yarn types
  • unit tests pass across workspaces
  • bundle size guardrails where applicable

60-64: In-SDK demo: publishing guardrails

Confirm:

  • Demo excluded from npm package
  • Separate tsconfig/build to avoid leaking dev deps
  • CI check on npm pack contents
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e5fd37d and 206d829.

📒 Files selected for processing (2)
  • packages/mobile-sdk-alpha/MIGRATION_CHECKLIST.md (1 hunks)
  • packages/mobile-sdk-alpha/MIGRATION_PROMPTS.md (1 hunks)

Comment on lines 21 to 25
## 4. Protocol synchronization

- Fetch protocol trees with pagination and a TTL cache.
- Verify computed roots against server data.

Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Protocol sync: rate limiting, backoff, and memory bounds

Make explicit:

  • Max page size and total items
  • Exponential backoff with jitter, honor Retry-After
  • Memory budget for concatenated trees

Comment on lines 206 to 210
1. Scaffold `demo/` under the SDK as a minimal React Native project.
2. Use SDK APIs for MRZ → NFC → registration flow.
3. Expose simple theming configuration.
4. Add `demo/README.md` with build/run instructions.
:::
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Embedded demo: exclude from published package and isolate build

  • Exclude demo/ from npm package via files field or .npmignore.
  • Use a separate tsconfig and no hoisting to avoid leaking dev deps into the SDK.
  • Add a CI check to prevent publishing the demo.
 4. Add `demo/README.md` with build/run instructions.
+5. Add publishing guardrails: exclude `demo/` from npm and add a CI step to verify the published tarball contents.
📝 Committable suggestion

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

Suggested change
1. Scaffold `demo/` under the SDK as a minimal React Native project.
2. Use SDK APIs for MRZ → NFC → registration flow.
3. Expose simple theming configuration.
4. Add `demo/README.md` with build/run instructions.
:::
1. Scaffold `demo/` under the SDK as a minimal React Native project.
2. Use SDK APIs for MRZ → NFC → registration flow.
3. Expose simple theming configuration.
4. Add `demo/README.md` with build/run instructions.
5. Add publishing guardrails: exclude `demo/` from npm and add a CI step to verify the published tarball contents.
:::

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

♻️ Duplicate comments (2)
packages/mobile-sdk-alpha/MIGRATION_CHECKLIST.md (2)

21-27: Protocol sync: good additions; add cache validation and resilience.

You’ve addressed earlier feedback on backoff/jitter, memory bounds, and Retry-After. Consider adding cache validation and resilience controls.

 ## 4. Protocol synchronization

 - Fetch protocol trees with pagination and a TTL cache.
 - Verify computed roots against server data.
 - Implement rate limiting with exponential backoff and jitter.
 - Set memory bounds for concatenated trees and honor Retry-After headers.
+ - Use ETag/If-None-Match to avoid unnecessary transfers; respect 304.
+ - Skew-tolerant TTL using monotonic time; clamp negative cache durations.
+ - Circuit breaker for repeated failures; cap max pagination depth/items.
+ - Metrics: request latency, retries, cache hit ratio, OOM/abort counters.

41-48: Crypto adapters: nice—environment detection, CSPRNG, timing-safe compare are covered.

Add key handling and AEAD safety footguns to the checklist.

 ## 8. Crypto adapters

 - Runtime-selectable adapter using WebCrypto with `@noble/*` fallbacks.
 - Parity tests across implementations.
 - Detect WebCrypto availability on React Native/Hermes environments.
 - Ensure CSPRNG-backed random number generation.
 - Use timing-safe comparison for secret values.
+ - Enforce AEAD nonce/IV uniqueness (e.g., AES-GCM) and include AAD where applicable.
+ - Zeroize secrets and private keys after use; avoid lingering JS references.
+ - Use vetted KDFs (HKDF/Argon2) with explicit salt/info; document params.
+ - Confirm BigInt/TypedArray behavior parity across Hermes/JSC; add tests.
🧹 Nitpick comments (4)
packages/mobile-sdk-alpha/MIGRATION_CHECKLIST.md (4)

16-20: Validation module: enumerate scope, errors, and tests.

Make validation deterministic and diagnosable.

 ## 3. Validation module

 - Port stateless document checks.
 - Cover validation logic with unit tests.
+ - Enumerate supported document types and policy toggles (strict/lenient modes).
+ - Define stable error codes/messages for UX and telemetry.
+ - Add property-based tests for boundary inputs and localized data formats.

57-61: Samples: expand to cover deep links, E2E, and privacy.

Ensure samples are production-safe and instructive.

 ## 10. Sample applications

 - React Native and web demos showcasing core flows.
 - iOS `OpenPassport` URL scheme.
+ - Test deep links across cold/warm starts; handle duplicate intents safely.
+ - Add Detox/Cypress E2E covering MRZ→NFC→proof flows.
+ - Document privacy prompts and data handling; disable analytics by default.

62-67: Integration plan: add rollout controls and observability.

Enable safe migration in /app.

 ## 11. Integrate SDK into `/app`

 - Consume `@selfxyz/mobile-sdk-alpha` inside the `app` workspace.
 - Replace MRZ/NFC modules with SDK adapters and wire processing helpers.
 - Validate builds and unit tests.
+ - Gate with feature flags and staged rollout; provide kill switch.
+ - Add tracing/metrics around new flows; define SLOs and error budgets.
+ - Migration checklist for removing legacy modules post-rollout.

68-71: In-SDK demo: ensure it’s dev-only and trimmed in prod.

Avoid shipping demo code in production builds.

 ## 12. In-SDK lightweight demo

 - Embedded React Native demo inside the SDK with theming hooks.
 - Provide build and run instructions.
+ - Guard behind dev flag and tree-shake from production bundles.
+ - CI check to ensure demo modules are excluded from release artifacts.
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 206d829 and ac10508.

📒 Files selected for processing (2)
  • packages/mobile-sdk-alpha/MIGRATION_CHECKLIST.md (1 hunks)
  • packages/mobile-sdk-alpha/MIGRATION_PROMPTS.md (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/mobile-sdk-alpha/MIGRATION_PROMPTS.md
🔇 Additional comments (1)
packages/mobile-sdk-alpha/MIGRATION_CHECKLIST.md (1)

3-4: Link to MIGRATION_PROMPTS.md is valid in its current form

The file packages/mobile-sdk-alpha/MIGRATION_PROMPTS.md exists alongside MIGRATION_CHECKLIST.md, and the inline link (./MIGRATION_PROMPTS.md) will resolve correctly. There are no numbered “## 1.”-style headings in MIGRATION_PROMPTS.md, so there are no chapter‐number anchors to match—no further changes are required unless you intend to add or reference numbered sections.

Comment on lines 5 to 10
## 1. Scanning adapters & NFC lifecycle

- Define cross-platform scanner interfaces.
- Implement React Native MRZ and NFC adapters with screen-on hooks.
- Provide a sample flow chaining MRZ to NFC scanning.

Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Strengthen scanning adapters with permissions, lifecycle, error taxonomy, and privacy.

Add explicit permission/lifecycle handling and PII-safe logging.

 ## 1. Scanning adapters & NFC lifecycle

 - Define cross-platform scanner interfaces.
 - Implement React Native MRZ and NFC adapters with screen-on hooks.
 - Provide a sample flow chaining MRZ to NFC scanning.
+ - Handle runtime permissions and denial flows (camera/NFC) with retry UX.
+ - Manage app lifecycle: cancel NFC/MRZ sessions on background/unmount; resume cleanly.
+ - Define error taxonomy (retryable vs terminal) with codes surfaced to the app.
+ - Avoid logging PII: redact MRZ/NFC payloads; keep buffers ephemeral and zeroized.
+ - Android: use ReaderMode with flags to minimize power; respect Doze/idle restrictions.
+ - iOS: add required Info.plist keys; handle NFC session invalidation callbacks.
📝 Committable suggestion

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

Suggested change
## 1. Scanning adapters & NFC lifecycle
- Define cross-platform scanner interfaces.
- Implement React Native MRZ and NFC adapters with screen-on hooks.
- Provide a sample flow chaining MRZ to NFC scanning.
## 1. Scanning adapters & NFC lifecycle
- Define cross-platform scanner interfaces.
- Implement React Native MRZ and NFC adapters with screen-on hooks.
- Provide a sample flow chaining MRZ to NFC scanning.
- Handle runtime permissions and denial flows (camera/NFC) with retry UX.
- Manage app lifecycle: cancel NFC/MRZ sessions on background/unmount; resume cleanly.
- Define error taxonomy (retryable vs terminal) with codes surfaced to the app.
- Avoid logging PII: redact MRZ/NFC payloads; keep buffers ephemeral and zeroized.
- Android: use ReaderMode with flags to minimize power; respect Doze/idle restrictions.
- iOS: add required Info.plist keys; handle NFC session invalidation callbacks.

Comment on lines 11 to 15
## 2. Processing helpers (MRZ & NFC)

- Finalize MRZ utilities and add an NFC response parser.
- Re-export helpers through the SDK entry point.

Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Parsing helpers: add test vectors and robustness requirements.

Cover edge-cases and spec compliance to harden MRZ/NFC parsing.

 ## 2. Processing helpers (MRZ & NFC)

 - Finalize MRZ utilities and add an NFC response parser.
 - Re-export helpers through the SDK entry point.
+ - Provide MRZ test vectors (ICAO sample docs) incl. checksum and date edge cases (YY <-> 19/20 century).
+ - Normalize and validate character sets (ICAO 9303, fill chars, diacritics/Unicode).
+ - Validate NFC TLV lengths and bounds; reject oversized/fragmented payloads.
+ - Fuzz/property tests for parsers; reject malformed inputs without OOM.
📝 Committable suggestion

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

Suggested change
## 2. Processing helpers (MRZ & NFC)
- Finalize MRZ utilities and add an NFC response parser.
- Re-export helpers through the SDK entry point.
## 2. Processing helpers (MRZ & NFC)
- Finalize MRZ utilities and add an NFC response parser.
- Re-export helpers through the SDK entry point.
- Provide MRZ test vectors (ICAO sample docs) incl. checksum and date edge cases (YY <-> 19/20 century).
- Normalize and validate character sets (ICAO 9303, fill chars, diacritics/Unicode).
- Validate NFC TLV lengths and bounds; reject oversized/fragmented payloads.
- Fuzz/property tests for parsers; reject malformed inputs without OOM.
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/MIGRATION_CHECKLIST.md around lines 11 to 15, the
MRZ/NFC parsing helpers lack explicit test vectors and robustness requirements;
add concrete test vectors (valid and invalid MRZ samples, NFC TLV/BER-TLV
examples, edge-cases like missing fields, truncated input, extra padding,
unexpected character encodings), specify validation rules per spec (checksum
verification, field lengths, allowed character sets), require defensive parsing
(graceful error returns, clear error codes/messages, input normalization,
strict/lenient modes), and add unit/integration test requirements that cover all
listed edge cases and ensure helpers are re-exported through the SDK entry point
so tests exercise the public API.

Comment on lines 28 to 31
## 5. Proof input generation

- Port register and disclose TEE input helpers.

Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Proof inputs: enforce determinism, encoding, and zeroization.

Prevent subtle cross-platform divergences.

 ## 5. Proof input generation

 - Port register and disclose TEE input helpers.
+ - Specify canonical serialization (endianness, byte order, field ordering).
+ - Use domain-separation tags and versioned schemas for forward-compat.
+ - Validate inputs (ranges, lengths) and zeroize secrets after use.
📝 Committable suggestion

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

Suggested change
## 5. Proof input generation
- Port register and disclose TEE input helpers.
## 5. Proof input generation
- Port register and disclose TEE input helpers.
- Specify canonical serialization (endianness, byte order, field ordering).
- Use domain-separation tags and versioned schemas for forward-compat.
- Validate inputs (ranges, lengths) and zeroize secrets after use.
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/MIGRATION_CHECKLIST.md around lines 28 to 31, the
checklist item "Port register and disclose TEE input helpers" needs concrete
requirements to enforce deterministic proof input generation: specify a
canonical, platform-independent encoding (e.g., explicit endianness, canonical
JSON/CBOR/Protobuf schema or fixed-length byte serialization), mandate
deterministic field ordering and canonicalization, add unit tests with known
cross-platform test vectors and fuzz tests to catch divergence, and require
secure zeroization of all transient buffers and secrets (use explicit zeroing
APIs or RAII-style wipe in all code paths including error/early returns); also
add a note to implement constant-time comparisons where applicable and to
document the chosen encoding/schema in the checklist so ports can conform
exactly.

Comment on lines 36 to 40
## 7. Attestation verification

- PCR0 check and public-key extraction.
- Minimal certificate-chain validation.

Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Attestation: extend chain validation, freshness, and revocation checks.

Make the trust decision auditable and time-bound.

 ## 7. Attestation verification

 - PCR0 check and public-key extraction.
 - Minimal certificate-chain validation.
+ - Bind attestation to a challenge/nonce; verify time validity with skew tolerance.
+ - Validate CRL/OCSP or vendor revocation mechanisms; cache with TTL.
+ - Maintain allowlist/mapping of expected measurements per version/device class.
+ - Document supported TEE vendors and flows; add negative test vectors.
📝 Committable suggestion

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

Suggested change
## 7. Attestation verification
- PCR0 check and public-key extraction.
- Minimal certificate-chain validation.
## 7. Attestation verification
- PCR0 check and public-key extraction.
- Minimal certificate-chain validation.
- Bind attestation to a challenge/nonce; verify time validity with skew tolerance.
- Validate CRL/OCSP or vendor revocation mechanisms; cache with TTL.
- Maintain allowlist/mapping of expected measurements per version/device class.
- Document supported TEE vendors and flows; add negative test vectors.
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/MIGRATION_CHECKLIST.md around lines 36 to 40, the
attestation verification note is too minimal; update the implementation to
perform full certificate-chain path validation to a configured trust anchor
(verify signatures, constraints and EKU), implement freshness checks (require
and validate attestation timestamps and/or nonces/challenges and enforce an
acceptable skew/window), add revocation checks (OCSP/CRL/CRLite or equivalent)
for each cert in the chain, and make the trust decision auditable and time‑bound
by logging the verification inputs, chain, revocation responses, timestamp,
decision and TTL (decision expiry) to an append-only audit store; ensure
configuration options for trust anchors, allowed clock skew, revocation
endpoints and decision TTL and fail closed if necessary.

Comment on lines 49 to 56
## 9. Artifact management

- Manifest schema & integrity verification.
- CDN downloads with caching and storage adapter.
- Verify manifest signature with pinned public key before caching.
- Enforce CDN allowlist and Content-Length checks.
- Stream hashing to avoid buffering large files.

Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Artifacts: strong improvements; add supply-chain hardening and safe I/O.

Prevent path traversal and ensure atomic, resumable downloads.

 ## 9. Artifact management

 - Manifest schema & integrity verification.
 - CDN downloads with caching and storage adapter.
 - Verify manifest signature with pinned public key before caching.
 - Enforce CDN allowlist and Content-Length checks.
 - Stream hashing to avoid buffering large files.
+ - Prevent path traversal (normalize and validate paths); sandbox destination dirs.
+ - Use atomic writes (temp file + fsync + rename) to avoid torn files.
+ - Support resumable downloads with range requests and hash verification.
+ - Version and rotate pinned keys; support multi-key verification windows.
+ - Validate Content-Type; set sane timeouts; cap concurrent downloads.
📝 Committable suggestion

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

Suggested change
## 9. Artifact management
- Manifest schema & integrity verification.
- CDN downloads with caching and storage adapter.
- Verify manifest signature with pinned public key before caching.
- Enforce CDN allowlist and Content-Length checks.
- Stream hashing to avoid buffering large files.
## 9. Artifact management
- Manifest schema & integrity verification.
- CDN downloads with caching and storage adapter.
- Verify manifest signature with pinned public key before caching.
- Enforce CDN allowlist and Content-Length checks.
- Stream hashing to avoid buffering large files.
- Prevent path traversal (normalize and validate paths); sandbox destination dirs.
- Use atomic writes (temp file + fsync + rename) to avoid torn files.
- Support resumable downloads with range requests and hash verification.
- Version and rotate pinned keys; support multi-key verification windows.
- Validate Content-Type; set sane timeouts; cap concurrent downloads.
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/MIGRATION_CHECKLIST.md around lines 49 to 56, the
artifact management notes need concrete supply-chain and safe I/O steps:
validate and sanitize all artifact paths and filenames to prevent path traversal
(reject ../, absolute paths, and normalize to a confined extraction root),
enforce CDN allowlist and Content-Length/size limits, stream downloads to a
temporary file in a dedicated temp directory and use atomic rename after
successful verification, support resumable downloads via Range and partial-file
tracking so downloads can resume without corrupting final files, verify manifest
signatures and checksums before moving/caching artifacts, set restrictive file
permissions and limit disk writes, and implement timeouts and max file size
checks to avoid resource exhaustion.

Comment on lines +57 to +61
## 10. Sample applications

- React Native and web demos showcasing core flows.
- iOS `OpenPassport` URL scheme.

Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

iOS URL scheme uniqueness and collision avoidance.

Verify OpenPassport scheme is unique and documented for integrators to avoid conflicts.


🏁 Script executed:

#!/bin/bash
# Search for existing URL scheme docs/usages to avoid duplication across packages
rg -n 'OpenPassport|CFBundleURLSchemes|URL scheme' -A 2 -B 2

Length of output: 37523


Clarify and document the iOS URL scheme in the migration checklist

The checklist currently calls out an “iOS OpenPassport URL scheme,” but your Info.plist (and existing tests) actually register the scheme as proofofpassport. Let’s keep everything in sync and remind integrators to pick a collision-safe name:

• packages/mobile-sdk-alpha/MIGRATION_CHECKLIST.md (around line 60):

- - iOS `OpenPassport` URL scheme.
+ - iOS `proofofpassport` URL scheme – ensure you choose a unique name (e.g. reverse-domain prefix).  
+   See packages/mobile-sdk-alpha/MIGRATION_PROMPTS.md Step 1 for details on configuring and avoiding collisions.
📝 Committable suggestion

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

Suggested change
## 10. Sample applications
- React Native and web demos showcasing core flows.
- iOS `OpenPassport` URL scheme.
## 10. Sample applications
- React Native and web demos showcasing core flows.
- iOS `proofofpassport` URL scheme – ensure you choose a unique name (e.g. reverse-domain prefix).
See packages/mobile-sdk-alpha/MIGRATION_PROMPTS.md Step 1 for details on configuring and avoiding collisions.
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/MIGRATION_CHECKLIST.md around lines 57 to 61, the
checklist refers to an iOS "OpenPassport" URL scheme but the project registers
the scheme as "proofofpassport"; update the checklist text to reference
"proofofpassport" (or explicitly instruct integrators to choose a collision-safe
custom scheme and keep it consistent), and add a short note to remind
maintainers to synchronize the chosen scheme across Info.plist, tests, and
sample apps to avoid mismatches.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🔭 Outside diff range comments (1)
packages/mobile-sdk-alpha/MIGRATION_CHECKLIST.md (1)

73-79: Partner feedback: fix “AADHAR” spelling and clarify callback security expectation

Keep terminology consistent and set security expectations here, too.

Apply this diff:

 - Callback hook so the host app can trigger push notifications after async proof verification.
-- Expand ID document coverage for US digital licenses and AADHAR.
+- Expand ID document coverage for US digital licenses and Aadhaar (India).
 - Document bundle size targets and provide a runnable integration example.
+ - Callback security: require HMAC-signed webhooks with timestamp/nonce and idempotency keys.
♻️ Duplicate comments (7)
packages/mobile-sdk-alpha/MIGRATION_CHECKLIST.md (7)

59-61: Fix iOS URL scheme name and add collision-avoidance guidance

Prior reviews flagged mismatch; keep docs consistent and collision-safe.

Apply this diff:

 - iOS `OpenPassport` URL scheme.
+ - iOS `proofofpassport` URL scheme (or your own collision-safe, reverse‑DNS scheme).
+   Ensure the chosen scheme is synchronized across Info.plist, tests, and sample apps.

5-9: Parsing helpers: add test vectors, spec compliance, and robustness

Harden MRZ/NFC parsers to prevent subtle bugs and OOMs.

Apply this diff:

 - Finalize MRZ utilities and add an NFC response parser.
 - Re-export helpers through the SDK entry point.
+ - Provide MRZ test vectors (ICAO 9303 samples) incl. checksum and date edge cases (YY century rollover).
+ - Validate character sets and field lengths; normalize inputs and reject malformed data.
+ - Validate NFC TLV/BER-TLV lengths and bounds; reject oversized/fragmented payloads.
+ - Add fuzz/property tests; ensure parsers fail fast without excessive memory use.

15-19: Proof inputs: enforce canonical encoding and zeroization

Ensure determinism across platforms and avoid secret leaks.

Apply this diff:

 - Port register and disclose TEE input helpers.
+ - Specify canonical serialization (endianness, field ordering, schema/version tags).
+ - Use domain‑separation tags; validate all input ranges/lengths.
+ - Zeroize transient buffers and secrets after use; use timing‑safe comparisons where applicable.
+ - Add cross‑platform test vectors to detect divergences.

27-31: TEE sessions: security, limits, and cleanup requirements

Expand WS wrapper requirements for safety and resilience.

Apply this diff:

 - WebSocket wrapper supporting abort, timeout, and progress events.
+ - Authenticate the WS connection (token binding) and enforce URL/domain allowlist.
+ - Keepalive (ping/pong) and auto‑reconnect with capped exponential backoff.
+ - Enforce max frame/message sizes and apply backpressure to prevent OOM.
+ - On abort/timeout, close sockets, remove listeners, and free buffers deterministically.
+ - Close sessions on app lifecycle events (background/unmount); cap concurrent sessions.

31-35: Attestation: freshness, revocation, and auditable trust decision

Minimal chain validation is insufficient on its own.

Apply this diff:

 - PCR0 check and public-key extraction.
 - Minimal certificate-chain validation.
+ - Bind attestation to a challenge/nonce; validate time/skew bounds.
+ - Validate revocation (OCSP/CRL/vendor); cache responses with TTL.
+ - Maintain allowlist of expected measurements per version/device class.
+ - Log inputs/chain/decision/TTL to an auditable store; fail closed on validation errors.

43-50: Artifacts: supply‑chain hardening and safe I/O

Add path safety, atomic writes, resumption, and key rotation.

Apply this diff:

 - Manifest schema & integrity verification.
 - CDN downloads with caching and storage adapter.
 - Verify manifest signature with pinned public key before caching.
 - Enforce CDN allowlist and Content-Length checks.
 - Stream hashing to avoid buffering large files.
+ - Prevent path traversal (normalize/validate paths) and sandbox destination dirs.
+ - Use atomic writes (temp file + fsync + rename) to avoid torn files.
+ - Support resumable downloads (Range) and verify final hash.
+ - Version and rotate pinned keys; support multi-key verification windows.
+ - Validate Content-Type; set timeouts; cap concurrent downloads and file sizes.

51-56: Scanning adapters: permissions, lifecycle, error taxonomy, and privacy

Operational hardening to avoid leaks and UX dead-ends.

Apply this diff:

 - Define cross-platform scanner interfaces.
 - Implement React Native MRZ and NFC adapters with screen-on hooks.
 - Provide a sample flow chaining MRZ to NFC scanning.
+ - Handle runtime permissions and denial flows (camera/NFC) with retry UX.
+ - Manage app lifecycle: cancel sessions on background/unmount; resume cleanly.
+ - Define error taxonomy (retryable vs terminal) with codes surfaced to the app.
+ - Avoid logging PII; redact payloads; keep buffers ephemeral and zeroized.
+ - Android: use ReaderMode with power-aware flags; respect Doze/idle restrictions.
+ - iOS: add required Info.plist keys; handle NFC session invalidation callbacks.
🧹 Nitpick comments (2)
packages/mobile-sdk-alpha/PARTNER_FEEDBACK.md (2)

13-13: Correct “AADHAR” to the official spelling “Aadhaar (India)”

Standardize terminology; “Aadhaar” is the official name.

Apply this diff:

-- Add AADHAR identity support.
+- Add Aadhaar (India) identity support.

18-21: Bundle size and platform/version support: define measurement method and targets

Make expectations testable and unambiguous.

Apply this diff:

 - Clarify acceptable SDK bundle size with partner (target: <500KB gzipped for core functionality).
+  - Define measurement method (e.g., metro production build, minified, gzipped; exclude sample apps).
+  - Track platform-specific budgets separately (JS bundle vs native binaries) with CI checks.
 - Provide a runnable example React Native app to speed up integration.
-- Document minimum iOS/Android version requirements and device compatibility.
+- Document minimum iOS/Android version requirements and device compatibility.
+  - Specify minimum OS versions, CPU architectures, and required device capabilities (camera, NFC).
+  - List known unsupported combinations and graceful-degradation behavior.
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ec00f9a and 7dfac90.

📒 Files selected for processing (3)
  • packages/mobile-sdk-alpha/MIGRATION_CHECKLIST.md (1 hunks)
  • packages/mobile-sdk-alpha/PARTNER_FEEDBACK.md (1 hunks)
  • packages/mobile-sdk-alpha/PARTNER_PROMPTS.md (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • packages/mobile-sdk-alpha/PARTNER_PROMPTS.md
🔇 Additional comments (2)
packages/mobile-sdk-alpha/MIGRATION_CHECKLIST.md (2)

21-26: Crypto adapters section looks solid

Includes environment detection, CSPRNG, and timing-safe comparisons.


36-42: Protocol sync section covers the key controls

Pagination, roots verification, backoff with jitter, Retry-After, and memory bounds are in place.

Comment on lines +5 to +7
- Expect OAuth-style branding inside the host app. Logo is required; colors, copy, and fonts are optional bonuses.
- Verification flow should run seamlessly on devices that already have the host app installed.
- Ensure branding configuration supports dark/light mode and accessibility requirements.
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Accessibility: specify WCAG, dynamic type, and assistive tech requirements

Be explicit so partners ship accessible experiences.

Apply this diff:

 - Ensure branding configuration supports dark/light mode and accessibility requirements.
+  - Meet WCAG 2.1 AA contrast ratios (normal and large text).
+  - Support Dynamic Type (iOS) / font scaling (Android), high-contrast and reduced-motion settings.
+  - Verify screen reader labels and focus order (VoiceOver/TalkBack); avoid conveying information by color alone.
+  - Localize all user-facing strings; support RTL layouts where applicable.
📝 Committable suggestion

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

Suggested change
- Expect OAuth-style branding inside the host app. Logo is required; colors, copy, and fonts are optional bonuses.
- Verification flow should run seamlessly on devices that already have the host app installed.
- Ensure branding configuration supports dark/light mode and accessibility requirements.
- Expect OAuth-style branding inside the host app. Logo is required; colors, copy, and fonts are optional bonuses.
- Verification flow should run seamlessly on devices that already have the host app installed.
- Meet WCAG 2.1 AA contrast ratios (normal and large text).
- Support Dynamic Type (iOS) / font scaling (Android), high-contrast and reduced-motion settings.
- Verify screen reader labels and focus order (VoiceOver/TalkBack); avoid conveying information by color alone.
- Localize all user-facing strings; support RTL layouts where applicable.
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/PARTNER_FEEDBACK.md around lines 5 to 7, the
guidance is too high-level on accessibility; update these lines to explicitly
require WCAG conformance (specify WCAG 2.1 AA minimum or WCAG 2.2 AA preferred),
support for dynamic type / font scaling (respect system text size and provide
scalable layouts), proper assistive‑tech support (VoiceOver and TalkBack with
fully localized semantic labels, traits, and accessible hints), color and
contrast requirements (minimum 4.5:1 text, 3:1 large text and UI components),
touch target and spacing minimums (44x44pt), logical focus and navigation order,
keyboard and switch control support, reduced‑motion preference respect, and
include acceptance/testing notes (list of required manual and automated tests,
screenshots in light/dark modes, and a11y audit tools to run).

Comment on lines +11 to +14
- Expand coverage to US driver's licenses, prioritizing states that support digital verification.
- Support both physical licenses and digital wallet credentials when available.
- Add AADHAR identity support.
- Implement proper error handling for unsupported document types with clear user messaging.
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Document handling: add standards alignment and PII minimization

Clarify mDL standards and privacy requirements.

Apply this diff:

 - Support both physical licenses and digital wallet credentials when available.
+- Where applicable, align digital license flows with ISO/IEC 18013-5/7 (mDL) capabilities and constraints.
 - Add Aadhaar (India) identity support.
 - Implement proper error handling for unsupported document types with clear user messaging.
+ - PII minimization: avoid persisting raw document images/biometrics; redact logs; define retention and deletion policy.

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines +24 to +27
- Allow the host backend to receive a callback when proofs finish so it can trigger push notifications for users.
- Implement secure callback authentication to prevent unauthorized notifications.
- Add retry logic and timeout handling for callback delivery.
- Provide webhook signature verification for callback security.
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Strengthen webhook/callback security: HMAC signatures, replay protection, idempotency, rotation

Callbacks can be abused without strict verification. Add concrete requirements.

Apply this diff:

 - Implement secure callback authentication to prevent unauthorized notifications.
-- Add retry logic and timeout handling for callback delivery.
-- Provide webhook signature verification for callback security.
+- Provide webhook signature verification: include an HMAC-SHA256 signature header over the raw body using a versioned secret; publish the exact canonicalization rules.
+- Enforce replay protection: require timestamped signed headers, reject stale (>5 min) requests, and support a nonce to prevent replays.
+- Require idempotency keys so partners can safely retry without duplicate side effects.
+- Restrict to HTTPS only; recommend optional mTLS and/or IP allowlisting.
+- Define secret rotation policy and overlapping acceptance window for old/new secrets.
+- Add exponential backoff with jitter for retries; cap max attempts and overall retry window.
+- Clearly document payload schema (versioned), correlation IDs, and expected 2xx ACK semantics.
📝 Committable suggestion

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

Suggested change
- Allow the host backend to receive a callback when proofs finish so it can trigger push notifications for users.
- Implement secure callback authentication to prevent unauthorized notifications.
- Add retry logic and timeout handling for callback delivery.
- Provide webhook signature verification for callback security.
- Allow the host backend to receive a callback when proofs finish so it can trigger push notifications for users.
- Implement secure callback authentication to prevent unauthorized notifications.
- Provide webhook signature verification: include an HMAC-SHA256 signature header over the raw body using a versioned secret; publish the exact canonicalization rules.
- Enforce replay protection: require timestamped signed headers, reject stale (>5 min) requests, and support a nonce to prevent replays.
- Require idempotency keys so partners can safely retry without duplicate side effects.
- Restrict to HTTPS only; recommend optional mTLS and/or IP allowlisting.
- Define secret rotation policy and overlapping acceptance window for old/new secrets.
- Add exponential backoff with jitter for retries; cap max attempts and overall retry window.
- Clearly document payload schema (versioned), correlation IDs, and expected 2xx ACK semantics.
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/PARTNER_FEEDBACK.md around lines 24 to 27, the
callback/webhook requirements are underspecified and need concrete security and
reliability controls; update the document to require: HMAC signatures (specify
algorithm e.g. HMAC-SHA256) over a canonicalized request body and selected
headers, include a signature header and a signature version header, include a
timestamp and unique nonce/ID header to enable replay protection (reject
requests older than a configurable skew and store recent nonces for
deduplication), mandate idempotency handling on the receiver side
(idempotency-key header or idempotent delivery semantics) and clear verification
steps, describe key management and rotation (support multiple active keys with
key IDs in the signature header and a rotation procedure), require TLS for
delivery, specify timeout and retry/backoff policy (max retries, exponential
backoff, jitter) and a dead-letter/alerting path, document expected HTTP
response codes for success vs retryable/fatal failures, and add test
vectors/examples and acceptance criteria for implementers to validate signature
generation/verification and replay/idempotency behavior.

@transphorm transphorm merged commit b2c79f3 into dev Aug 12, 2025
9 checks passed
@transphorm transphorm deleted the codex/expand-migration-checklist-for-mobile-sdk branch August 12, 2025 00:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants