feat: TD1 MRZ Overflow Format #1337
Conversation
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on November 17. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
WalkthroughAdds TD1 overflow handling by introducing Changes
Sequence Diagram(s)sequenceDiagram
participant Camera as LiveMRZScannerView
participant TD1Proc as processTD1DocumentWithOverflow
participant Extractor as extractAndValidateTD1DocumentNumber
participant CheckCalc as calculateMRZCheckDigit
participant Parser as QKMRZParser
participant Mapper as mapVisionResultToDictionary
Camera->>TD1Proc: MRZ result (TD1 / possible overflow)
TD1Proc->>Extractor: parse TD1 line1 for overflow pattern
Extractor->>CheckCalc: compute MRZ check digit(s)
CheckCalc-->>Extractor: check digit result
Extractor-->>TD1Proc: (documentNumber, isValid)
alt valid
TD1Proc->>Parser: parse remaining MRZ lines (use validated doc if needed)
Parser-->>TD1Proc: parsed MRZ object
TD1Proc->>Camera: set overrideDocumentNumber
Camera->>Mapper: map result (uses overrideDocumentNumber)
else invalid
TD1Proc-->>Camera: log invalid / fallback
Camera->>Mapper: map result (no override, default flow)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/ios/LiveMRZScannerView.swift(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
app/ios/**/*.{m,mm,swift}
📄 CodeRabbit inference engine (app/AGENTS.md)
Document complex native iOS module changes in the PR
Files:
app/ios/LiveMRZScannerView.swift
app/ios/**/*
⚙️ CodeRabbit configuration file
app/ios/**/*: Review iOS-specific code for:
- Platform-specific implementations
- Performance considerations
- Security best practices for mobile
Files:
app/ios/LiveMRZScannerView.swift
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)
Applied to files:
app/ios/LiveMRZScannerView.swift
📚 Learning: 2025-09-22T11:10:57.879Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: app/AGENTS.md:0-0
Timestamp: 2025-09-22T11:10:57.879Z
Learning: Applies to app/ios/**/*.{m,mm,swift} : Document complex native iOS module changes in the PR
Applied to files:
app/ios/LiveMRZScannerView.swift
🧬 Code graph analysis (1)
app/ios/LiveMRZScannerView.swift (1)
packages/mobile-sdk-alpha/ios/SelfSDK/SelfLiveMRZScannerView.swift (1)
processBelgiumDocument(137-190)
🔇 Additional comments (2)
app/ios/LiveMRZScannerView.swift (2)
83-106: LGTM! Solid ICAO 9303 implementation.The check digit calculation correctly implements the ICAO 9303 standard with proper character-to-value mappings and weighted sum modulo 10.
189-238: Add test coverage for Belgian overflow format against ICAO 9303 sample MRZ strings.The custom Belgian overflow format validation bypasses
QKMRZParserwithout test verification against published ICAO standards. Current test suite (packages/mobile-sdk-alpha/tests/processing/mrz.test.ts) includes only TD1/TD3 samples for FRA, ESP, USA—no Belgian cases.Implement test cases using ICAO 9303 Belgian TD1 sample MRZ strings, including edge cases (all-numeric vs. alphanumeric overflow sequences) to verify
extractAndValidateBelgianDocumentNumber()correctness.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/ios/LiveMRZScannerView.swift (1)
214-214: Remove or gate PII logging for production—compliance risk.Lines 214 and 227 log sensitive PII (full MRZ line and document number) without debug compilation guards. This creates GDPR/CCPA compliance risks. Based on learnings, document numbers are primary PII in this passport verification domain.
Wrap these prints in debug flags:
- print("[LiveMRZScannerView] Line 1: \(line1)") +#if DEBUG + print("[LiveMRZScannerView] Line 1: \(line1)") +#endif- print("[LiveMRZScannerView] Belgian document number validated: \(documentNumber) ✓") +#if DEBUG + print("[LiveMRZScannerView] Belgian document number validated: \(documentNumber) ✓") +#endifAlso applies to: 227-227
🧹 Nitpick comments (1)
app/ios/LiveMRZScannerView.swift (1)
13-13: Optional: Remove redundant nil initialization.Swift optionals default to
nil, making the explicit initialization redundant.- @State private var overrideDocumentNumber: String? = nil // for Belgian ID overflow format + @State private var overrideDocumentNumber: String? // for Belgian ID overflow format
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/ios/LiveMRZScannerView.swift(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
app/ios/**/*.{m,mm,swift}
📄 CodeRabbit inference engine (app/AGENTS.md)
Document complex native iOS module changes in the PR
Files:
app/ios/LiveMRZScannerView.swift
app/ios/**/*
⚙️ CodeRabbit configuration file
app/ios/**/*: Review iOS-specific code for:
- Platform-specific implementations
- Performance considerations
- Security best practices for mobile
Files:
app/ios/LiveMRZScannerView.swift
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to noir/crates/dg1/src/dg1/dg1.nr : Document Verification Processing validates international travel documents using ICAO standards, processes DSC verification, and handles multiple signature algorithms.
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)
Applied to files:
app/ios/LiveMRZScannerView.swift
📚 Learning: 2025-09-22T11:10:57.879Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: app/AGENTS.md:0-0
Timestamp: 2025-09-22T11:10:57.879Z
Learning: Applies to app/ios/**/*.{m,mm,swift} : Document complex native iOS module changes in the PR
Applied to files:
app/ios/LiveMRZScannerView.swift
📚 Learning: 2025-08-25T14:07:52.997Z
Learnt from: aaronmgdr
Repo: selfxyz/self PR: 889
File: app/src/utils/utils.ts:20-29
Timestamp: 2025-08-25T14:07:52.997Z
Learning: In the SELF passport/identity verification app, the primary PII concerns are MRZ data from passports and document numbers. SSNs and credit cards are not relevant to this domain, so PII redaction should focus on passport-specific data patterns rather than general financial/personal identifiers.
Applied to files:
app/ios/LiveMRZScannerView.swift
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to **/*.{js,ts,tsx,jsx,sol,nr} : NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
Applied to files:
app/ios/LiveMRZScannerView.swift
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to **/*.{js,ts,tsx,jsx,sol,nr} : ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g., `***-***-1234` for passport numbers, `J*** D***` for names).
Applied to files:
app/ios/LiveMRZScannerView.swift
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Never log PII, credentials, or private keys (especially in production); redact sensitive fields consistently
Applied to files:
app/ios/LiveMRZScannerView.swift
📚 Learning: 2025-10-23T12:08:55.529Z
Learnt from: shazarre
Repo: selfxyz/self PR: 1236
File: packages/mobile-sdk-alpha/src/flows/onboarding/document-nfc-screen.tsx:356-378
Timestamp: 2025-10-23T12:08:55.529Z
Learning: In packages/mobile-sdk-alpha/src/flows/onboarding/document-nfc-screen.tsx, the NFC native events emitted via NativeEventEmitter are generic status strings (e.g., "PACE succeeded", "BAC failed", "Reading DG1 succeeded") and do not contain sensitive MRZ data or passport numbers, so they do not require sanitization before logging.
Applied to files:
app/ios/LiveMRZScannerView.swift
📚 Learning: 2025-09-22T11:10:57.879Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: app/AGENTS.md:0-0
Timestamp: 2025-09-22T11:10:57.879Z
Learning: Ensure no sensitive data appears in logs (PII, credentials, tokens)
Applied to files:
app/ios/LiveMRZScannerView.swift
🧬 Code graph analysis (1)
app/ios/LiveMRZScannerView.swift (1)
packages/mobile-sdk-alpha/ios/SelfSDK/SelfLiveMRZScannerView.swift (1)
processBelgiumDocument(137-190)
🪛 SwiftLint (0.57.0)
app/ios/LiveMRZScannerView.swift
[Warning] 13-13: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
🔇 Additional comments (4)
app/ios/LiveMRZScannerView.swift (4)
63-87: LGTM: Clean override pattern for Belgian document numbers.The implementation correctly prioritizes the manually validated document number for Belgian IDs while falling back to the parser result for other countries. This ensures NFC chip authentication receives the correct full document number.
89-112: LGTM: Correct ICAO 9303 check digit implementation.The algorithm correctly implements the ICAO 9303 standard with proper weights [7, 3, 1], character-to-value mapping (digits 0-9, letters A-Z → 10-35, filler '<' → 0), and modulo 10 checksum calculation.
114-188: Excellent: Belgian overflow format handler with proper safety.The implementation correctly handles both standard and overflow Belgian TD1 formats, with proper validation of check digits per ICAO 9303. Previous review concerns have been addressed:
- Proper optional binding eliminates force-unwrap crash risk
- PII logging properly gated with
#if DEBUG
201-264: Strong implementation: Manual Belgian validation with proper field checks.The rewritten function correctly:
- Manually extracts and validates the Belgian document number using the overflow handler
- Validates birthdate and expiry dates (addressing previous review concern)
- Uses QKMRZParser only for non-document-number fields
- Properly sets
overrideDocumentNumberfor downstream NFC authentication- Gates verbose debug output with
#if DEBUG(lines 242-253)
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/ios/LiveMRZScannerView.swift(5 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
app/ios/**/*.{m,mm,swift}
📄 CodeRabbit inference engine (app/AGENTS.md)
Document complex native iOS module changes in the PR
Files:
app/ios/LiveMRZScannerView.swift
app/ios/**/*
⚙️ CodeRabbit configuration file
app/ios/**/*: Review iOS-specific code for:
- Platform-specific implementations
- Performance considerations
- Security best practices for mobile
Files:
app/ios/LiveMRZScannerView.swift
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to noir/crates/dg1/src/dg1/dg1.nr : Document Verification Processing validates international travel documents using ICAO standards, processes DSC verification, and handles multiple signature algorithms.
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)
Applied to files:
app/ios/LiveMRZScannerView.swift
📚 Learning: 2025-10-23T12:08:55.529Z
Learnt from: shazarre
Repo: selfxyz/self PR: 1236
File: packages/mobile-sdk-alpha/src/flows/onboarding/document-nfc-screen.tsx:356-378
Timestamp: 2025-10-23T12:08:55.529Z
Learning: In packages/mobile-sdk-alpha/src/flows/onboarding/document-nfc-screen.tsx, the NFC native events emitted via NativeEventEmitter are generic status strings (e.g., "PACE succeeded", "BAC failed", "Reading DG1 succeeded") and do not contain sensitive MRZ data or passport numbers, so they do not require sanitization before logging.
Applied to files:
app/ios/LiveMRZScannerView.swift
📚 Learning: 2025-09-22T11:10:57.879Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: app/AGENTS.md:0-0
Timestamp: 2025-09-22T11:10:57.879Z
Learning: Applies to app/ios/**/*.{m,mm,swift} : Document complex native iOS module changes in the PR
Applied to files:
app/ios/LiveMRZScannerView.swift
📚 Learning: 2025-08-25T14:07:52.997Z
Learnt from: aaronmgdr
Repo: selfxyz/self PR: 889
File: app/src/utils/utils.ts:20-29
Timestamp: 2025-08-25T14:07:52.997Z
Learning: In the SELF passport/identity verification app, the primary PII concerns are MRZ data from passports and document numbers. SSNs and credit cards are not relevant to this domain, so PII redaction should focus on passport-specific data patterns rather than general financial/personal identifiers.
Applied to files:
app/ios/LiveMRZScannerView.swift
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to **/*.{js,ts,tsx,jsx,sol,nr} : ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g., `***-***-1234` for passport numbers, `J*** D***` for names).
Applied to files:
app/ios/LiveMRZScannerView.swift
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Never log PII, credentials, or private keys (especially in production); redact sensitive fields consistently
Applied to files:
app/ios/LiveMRZScannerView.swift
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to **/*.{js,ts,tsx,jsx,sol,nr} : NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
Applied to files:
app/ios/LiveMRZScannerView.swift
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Address CodeRabbitAI feedback and resolve security warnings during review
Applied to files:
app/ios/LiveMRZScannerView.swift
📚 Learning: 2025-09-22T11:10:57.879Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: app/AGENTS.md:0-0
Timestamp: 2025-09-22T11:10:57.879Z
Learning: Ensure no sensitive data appears in logs (PII, credentials, tokens)
Applied to files:
app/ios/LiveMRZScannerView.swift
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to noir/crates/dg1/src/dg1/dg1.nr : Document Verification Processing validates international travel documents using ICAO standards, processes DSC verification, and handles multiple signature algorithms.
Applied to files:
app/ios/LiveMRZScannerView.swift
🧬 Code graph analysis (1)
app/ios/LiveMRZScannerView.swift (1)
packages/mobile-sdk-alpha/ios/SelfSDK/SelfLiveMRZScannerView.swift (1)
handleValidMRZResult(130-135)
🪛 SwiftLint (0.57.0)
app/ios/LiveMRZScannerView.swift
[Warning] 13-13: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
| if pos15 != "<" { | ||
| // handling standard format where position 15 is the check digit | ||
| let checkDigit = Int(String(pos15)) ?? -1 | ||
| let calculatedCheck = calculateMRZCheckDigit(principalPart) | ||
| let isValid = (checkDigit == calculatedCheck) | ||
| print("[extractAndValidateTD1DocumentNumber] Standard format: \(principalPart), check=\(checkDigit), calculated=\(calculatedCheck), valid=\(isValid)") | ||
| return (principalPart, isValid) | ||
| } |
There was a problem hiding this comment.
Strip PII from standard-format logging.
This print dumps the raw document number and check digit into logs, which violates our “never log PII” rule for MRZ data and creates GDPR/CCPA exposure in release builds. Please remove or properly gate/redact the output.
[suggested fix]
- print("[extractAndValidateTD1DocumentNumber] Standard format: \(principalPart), check=\(checkDigit), calculated=\(calculatedCheck), valid=\(isValid)")
+ #if DEBUG
+ let maskedDoc = String(principalPart.prefix(3)) + String(repeating: "•", count: Swift.max(principalPart.count - 6, 0)) + String(principalPart.suffix(3))
+ print("[extractAndValidateTD1DocumentNumber] Standard format valid=\(isValid) (doc=\(maskedDoc))")
+ #endifAs per coding guidelines.
📝 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 pos15 != "<" { | |
| // handling standard format where position 15 is the check digit | |
| let checkDigit = Int(String(pos15)) ?? -1 | |
| let calculatedCheck = calculateMRZCheckDigit(principalPart) | |
| let isValid = (checkDigit == calculatedCheck) | |
| print("[extractAndValidateTD1DocumentNumber] Standard format: \(principalPart), check=\(checkDigit), calculated=\(calculatedCheck), valid=\(isValid)") | |
| return (principalPart, isValid) | |
| } | |
| if pos15 != "<" { | |
| // handling standard format where position 15 is the check digit | |
| let checkDigit = Int(String(pos15)) ?? -1 | |
| let calculatedCheck = calculateMRZCheckDigit(principalPart) | |
| let isValid = (checkDigit == calculatedCheck) | |
| #if DEBUG | |
| let maskedDoc = String(principalPart.prefix(3)) + String(repeating: "•", count: Swift.max(principalPart.count - 6, 0)) + String(principalPart.suffix(3)) | |
| print("[extractAndValidateTD1DocumentNumber] Standard format valid=\(isValid) (doc=\(maskedDoc))") | |
| #endif | |
| return (principalPart, isValid) | |
| } |
🤖 Prompt for AI Agents
In app/ios/LiveMRZScannerView.swift around lines 132 to 139, the current print
call outputs raw MRZ PII (document number and check digits); remove that raw
output and replace it with a non-PII-safe logging strategy: either completely
remove the print in release builds or gate it behind a debug-only flag (e.g.,
#if DEBUG) and, if you must log, redact the document number (mask all but last
1–2 characters) and only log a boolean validity result and non-sensitive
context. Ensure no raw principalPart or digits appear in any release logs.
| let line1 = lines[0] | ||
| print("[LiveMRZScannerView] Line 1: \(line1)") | ||
|
|
||
| // Reconstruct the MRZ with the corrected first line | ||
| var correctedLines = lines | ||
| correctedLines[0] = paddedCorrectedLine | ||
| let correctedMRZString = correctedLines.joined(separator: "\n") | ||
| // print("[LiveMRZScannerView] Corrected MRZ string: \(correctedMRZString)") | ||
| // extracting and validating document number manually using overflow format handler | ||
| guard let (documentNumber, isDocNumberValid) = extractAndValidateTD1DocumentNumber(line1: line1) else { | ||
| print("[LiveMRZScannerView] Failed to extract TD1 document number") | ||
| return nil | ||
| } | ||
|
|
||
| guard let belgiumMRZResult = parser.parse(mrzString: correctedMRZString) else { | ||
| print("[LiveMRZScannerView] Belgium MRZ result is not valid") | ||
| if !isDocNumberValid { | ||
| print("[LiveMRZScannerView] TD1 document number check digit is INVALID") | ||
| return nil | ||
| } | ||
|
|
||
| // print("[LiveMRZScannerView] Belgium MRZ result: \(belgiumMRZResult)") | ||
| print("[LiveMRZScannerView] TD1 document number validated: \(documentNumber) ✓") | ||
|
|
||
| // Try the corrected MRZ first | ||
| if isValidMRZResult(belgiumMRZResult) { | ||
| return belgiumMRZResult | ||
| // parsing the original MRZ to get all other fields (name, birthdate, etc.) |
There was a problem hiding this comment.
Stop logging full MRZ line/document number.
print is emitting the entire MRZ line and the validated document number in plaintext. That’s sensitive PII and must not reach production logs. Wrap these diagnostics in #if DEBUG and mask the values before logging.
[suggested fix]
- let line1 = lines[0]
- print("[LiveMRZScannerView] Line 1: \(line1)")
+ let line1 = lines[0]
+ #if DEBUG
+ let maskedLine1 = "\(line1.prefix(4))…\(line1.suffix(4))"
+ print("[LiveMRZScannerView] Line 1: \(maskedLine1)")
+ #endif
@@
- print("[LiveMRZScannerView] TD1 document number validated: \(documentNumber) ✓")
+ #if DEBUG
+ let maskedDoc = String(documentNumber.prefix(3)) + String(repeating: "•", count: Swift.max(documentNumber.count - 6, 0)) + String(documentNumber.suffix(3))
+ print("[LiveMRZScannerView] TD1 document number validated ✓ (doc=\(maskedDoc))")
+ #endifAs per coding guidelines.
🤖 Prompt for AI Agents
In app/ios/LiveMRZScannerView.swift around lines 214 to 230, the code prints
full MRZ line and document number (PII) to logs; wrap these diagnostic prints in
a compile-time conditional (#if DEBUG ... #endif) so they are only emitted in
debug builds, and mask sensitive values before logging (e.g., replace all but
last 2–4 chars of the document number with asterisks and redact the MRZ line to
show only safe metadata like length or masked segments). Ensure the
guard/failure logs do not expose raw PII and keep only non-sensitive status text
in production.
ArmanKolozyan
changed the title
|
Hey @ArmanKolozyan, Thank you for the PR. |
|
We also need to update this check here to complete the flow. |
|
Good point, thanks! I have updated |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/mobile-sdk-alpha/src/processing/mrz.ts (1)
188-202: Add maximum length validation as a safety check.While TD1 overflow format allows document numbers >9 characters, there should still be a reasonable upper bound to prevent malformed data from passing through. ICAO 9303 specifies that overflow uses the optional data field (15 characters), so the practical maximum is 9 + 15 = 24 characters.
Apply this diff:
export function checkScannedInfo(passportNumber: string, dateOfBirth: string, dateOfExpiry: string): boolean { // TD1 overflow format allows document numbers > 9 characters per ICAO 9303: // When document numbers exceed 9 characters, the overflow digits are stored in the optional data // field and the full document number is reconstructed during MRZ parsing. - // if (passportNumber.length > 9) { - // return false; - // } + // Maximum length: 9 (principal) + 15 (optional data) = 24 characters + if (passportNumber.length > 24) { + return false; + } if (dateOfBirth.length !== 6) { return false; } if (dateOfExpiry.length !== 6) { return false; } return true; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/mobile-sdk-alpha/src/processing/mrz.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
packages/mobile-sdk-alpha/src/processing/**
📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)
Place MRZ processing helpers in packages/mobile-sdk-alpha/src/processing/
Files:
packages/mobile-sdk-alpha/src/processing/mrz.ts
packages/mobile-sdk-alpha/**/*.{ts,tsx}
📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)
packages/mobile-sdk-alpha/**/*.{ts,tsx}: Use strict TypeScript type checking across the codebase
Follow ESLint TypeScript-specific rules
Avoid introducing circular dependencies
Files:
packages/mobile-sdk-alpha/src/processing/mrz.ts
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g.,***-***-1234for passport numbers,J*** D***for names).
Files:
packages/mobile-sdk-alpha/src/processing/mrz.ts
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}: Review alpha mobile SDK code for:
- API consistency with core SDK
- Platform-neutral abstractions
- Performance considerations
- Clear experimental notes or TODOs
Files:
packages/mobile-sdk-alpha/src/processing/mrz.ts
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)
Learnt from: shazarre
Repo: selfxyz/self PR: 1236
File: packages/mobile-sdk-alpha/src/flows/onboarding/document-nfc-screen.tsx:356-378
Timestamp: 2025-10-23T12:08:55.529Z
Learning: In packages/mobile-sdk-alpha/src/flows/onboarding/document-nfc-screen.tsx, the NFC native events emitted via NativeEventEmitter are generic status strings (e.g., "PACE succeeded", "BAC failed", "Reading DG1 succeeded") and do not contain sensitive MRZ data or passport numbers, so they do not require sanitization before logging.
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to noir/crates/dg1/src/dg1/dg1.nr : Document Verification Processing validates international travel documents using ICAO standards, processes DSC verification, and handles multiple signature algorithms.
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)
Applied to files:
packages/mobile-sdk-alpha/src/processing/mrz.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Test isPassportDataValid() with realistic synthetic passport data (never real user data)
Applied to files:
packages/mobile-sdk-alpha/src/processing/mrz.ts
📚 Learning: 2025-10-23T12:08:55.529Z
Learnt from: shazarre
Repo: selfxyz/self PR: 1236
File: packages/mobile-sdk-alpha/src/flows/onboarding/document-nfc-screen.tsx:356-378
Timestamp: 2025-10-23T12:08:55.529Z
Learning: In packages/mobile-sdk-alpha/src/flows/onboarding/document-nfc-screen.tsx, the NFC native events emitted via NativeEventEmitter are generic status strings (e.g., "PACE succeeded", "BAC failed", "Reading DG1 succeeded") and do not contain sensitive MRZ data or passport numbers, so they do not require sanitization before logging.
Applied to files:
packages/mobile-sdk-alpha/src/processing/mrz.ts
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/processing/** : Place MRZ processing helpers in packages/mobile-sdk-alpha/src/processing/
Applied to files:
packages/mobile-sdk-alpha/src/processing/mrz.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Ensure parseNFCResponse() works with representative, synthetic NFC data
Applied to files:
packages/mobile-sdk-alpha/src/processing/mrz.ts
📚 Learning: 2025-08-24T18:53:11.220Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-08-24T18:53:11.220Z
Learning: Normalize passport numbers by removing whitespace/punctuation and validate against country-specific formats
Applied to files:
packages/mobile-sdk-alpha/src/processing/mrz.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Never use real user PII in tests; use only synthetic, anonymized, or approved test vectors
Applied to files:
packages/mobile-sdk-alpha/src/processing/mrz.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Use strict TypeScript type checking across the codebase
Applied to files:
packages/mobile-sdk-alpha/src/processing/mrz.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/package.json : Verify package conditions are valid (e.g., exports conditions)
Applied to files:
packages/mobile-sdk-alpha/src/processing/mrz.ts
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/index.ts : Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts
Applied to files:
packages/mobile-sdk-alpha/src/processing/mrz.ts
📚 Learning: 2025-08-24T18:53:11.220Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursor/rules/compliance-verification.mdc:0-0
Timestamp: 2025-08-24T18:53:11.220Z
Learning: Implement OFAC passport number check: validate passport numbers directly against OFAC lists
Applied to files:
packages/mobile-sdk-alpha/src/processing/mrz.ts
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
Repo: selfxyz/self PR: 0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Always validate certificates for passport data.
Applied to files:
packages/mobile-sdk-alpha/src/processing/mrz.ts
| // TD1 overflow format allows document numbers > 9 characters per ICAO 9303: | ||
| // When document numbers exceed 9 characters, the overflow digits are stored in the optional data | ||
| // field and the full document number is reconstructed during MRZ parsing. | ||
| // if (passportNumber.length > 9) { | ||
| // return false; | ||
| // } |
There was a problem hiding this comment.
Critical: Incomplete TD1 overflow implementation.
Removing the length check without updating the TD1 extraction and validation logic creates an incomplete implementation. The iOS code (per PR objectives) added extractAndValidateTD1DocumentNumber to detect overflow (position 15 == '<'), extract overflow characters from optional data (positions 16+), and validate using the correct check digit. This TypeScript implementation allows long document numbers to pass checkScannedInfo but lacks the corresponding overflow extraction and validation logic in extractTD1Info and validateTD1CheckDigits.
This could lead to:
- Incorrect document numbers being extracted (missing overflow characters)
- Failed check digit validation (validating against wrong data)
- NFC/BAC authentication failures
Apply these fixes:
1. Update extractTD1Info to handle overflow:
function extractTD1Info(lines: string[]): Omit<MRZInfo, 'validation'> {
const line1 = lines[0];
const line2 = lines[1];
const concatenatedLines = line1 + line2;
+
+ // Handle TD1 overflow format per ICAO 9303
+ // When document number exceeds 9 chars, overflow is in optional data
+ let documentNumber = concatenatedLines.slice(5, 14).replace(/</g, '').trim();
+ const documentNumberCheckDigitPos = concatenatedLines.slice(14, 15);
+
+ // Detect overflow: if position 15 is '<', overflow exists in optional data
+ if (documentNumberCheckDigitPos === '<') {
+ // Extract overflow from optional data (positions 15-29, which is indices 15-29)
+ const optionalData = concatenatedLines.slice(15, 30);
+ // Find the check digit (last non-< character in overflow)
+ let overflowChars = '';
+ for (let i = 0; i < optionalData.length; i++) {
+ if (optionalData[i] !== '<') {
+ overflowChars += optionalData[i];
+ } else {
+ break;
+ }
+ }
+ // Append overflow to document number (excluding last char which is check digit)
+ if (overflowChars.length > 0) {
+ documentNumber += overflowChars.slice(0, -1);
+ }
+ }
return {
documentType: concatenatedLines.slice(0, 2),
issuingCountry: concatenatedLines.slice(2, 5),
- documentNumber: concatenatedLines.slice(5, 14).replace(/</g, '').trim(),
+ documentNumber,
dateOfBirth: concatenatedLines.slice(30, 36),
dateOfExpiry: concatenatedLines.slice(38, 44),
};
}2. Update validateTD1CheckDigits to validate overflow:
function validateTD1CheckDigits(lines: string[]): Omit<MRZValidation, 'format' | 'overall'> {
const line1 = lines[0];
const line2 = lines[1];
const concatenatedLines = line1 + line2;
- const documentNumber = concatenatedLines.slice(5, 14);
- const documentNumberCheckDigit = concatenatedLines.slice(14, 15);
+ // Handle TD1 overflow format
+ let documentNumber = concatenatedLines.slice(5, 14);
+ let documentNumberCheckDigit = concatenatedLines.slice(14, 15);
+
+ // Detect overflow: if position 15 is '<', check digit is in optional data
+ if (documentNumberCheckDigit === '<') {
+ const optionalData = concatenatedLines.slice(15, 30);
+ // Find overflow characters and check digit
+ let overflowChars = '';
+ for (let i = 0; i < optionalData.length; i++) {
+ if (optionalData[i] !== '<') {
+ overflowChars += optionalData[i];
+ } else {
+ break;
+ }
+ }
+ if (overflowChars.length > 0) {
+ // Last char is check digit, rest is overflow
+ documentNumberCheckDigit = overflowChars.slice(-1);
+ documentNumber += overflowChars.slice(0, -1);
+ }
+ }
+
const dateOfBirth = concatenatedLines.slice(30, 36);
const dobCheckDigit = concatenatedLines.slice(36, 37);
const dateOfExpiry = concatenatedLines.slice(38, 44);
const expiryCheckDigit = concatenatedLines.slice(44, 45);
return {
passportNumberChecksum: verifyCheckDigit(documentNumber, documentNumberCheckDigit),
dateOfBirthChecksum: verifyCheckDigit(dateOfBirth, dobCheckDigit),
dateOfExpiryChecksum: verifyCheckDigit(dateOfExpiry, expiryCheckDigit),
compositeChecksum: true, // TD1 doesn't have a composite check digit like TD3
};
}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
This specific code path is only reached on Android. This PR addresses only the iOS platform.
There was a problem hiding this comment.
@seshanthS Thank you for the clarification! I understand now—this TypeScript code path is Android-only, and since this PR addresses only iOS, the Android TD1 overflow implementation is appropriately deferred.
The change to comment out the length check makes sense as an interim step: it prevents Android from prematurely rejecting document numbers > 9 characters, while the full TD1 overflow extraction and validation logic (similar to what was implemented in the iOS Swift code) would be added when Android support is implemented later—as noted in the PR TODOs.
For the iOS flow, the Swift implementation in LiveMRZScannerView.swift with extractAndValidateTD1DocumentNumber and processTD1DocumentWithOverflow provides the complete overflow handling before the data reaches this TypeScript layer.
✏️ Learnings added
Learnt from: seshanthS
Repo: selfxyz/self PR: 1337
File: packages/mobile-sdk-alpha/src/processing/mrz.ts:189-194
Timestamp: 2025-11-18T12:17:14.797Z
Learning: In packages/mobile-sdk-alpha/src/processing/mrz.ts, the checkScannedInfo function and related TD1 extraction/validation logic are only reached on Android. iOS uses native Swift parsing (LiveMRZScannerView.swift) that bypasses this TypeScript layer.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: shazarre
Repo: selfxyz/self PR: 1236
File: packages/mobile-sdk-alpha/src/flows/onboarding/document-nfc-screen.tsx:356-378
Timestamp: 2025-10-23T12:08:55.529Z
Learning: In packages/mobile-sdk-alpha/src/flows/onboarding/document-nfc-screen.tsx, the NFC native events emitted via NativeEventEmitter are generic status strings (e.g., "PACE succeeded", "BAC failed", "Reading DG1 succeeded") and do not contain sensitive MRZ data or passport numbers, so they do not require sanitization before logging.
Learnt from: aaronmgdr
Repo: selfxyz/self PR: 889
File: app/src/utils/utils.ts:20-29
Timestamp: 2025-08-25T14:07:52.997Z
Learning: In the SELF passport/identity verification app, the primary PII concerns are MRZ data from passports and document numbers. SSNs and credit cards are not relevant to this domain, so PII redaction should focus on passport-specific data patterns rather than general financial/personal identifiers.
2 participants
Self enables users to prove their identity in a privacy-preserving manner by generating zero-knowledge proofs from their passport or ID card data. To read the ID card data, Self scans the Machine Readable Zone (MRZ) at the bottom of the document. The MRZ contains essential information like document number, date of birth, and expiry date encoded in a standardized format defined by ICAO 9303. The MRZ embeds check digits (single-digit numbers calculated from the data using a formula) throughout the encoded fields to detect OCR scanning errors.
Accurately scanning the MRZ is critical for two reasons. First, the identity data parsed from the MRZ is required for proof generation. Second, the NFC chip on modern IDs protects its data behind access control. With Basic Access Control (BAC), the reader and chip derive session keys from the exact MRZ values (document number, date of birth, and expiry date) and then establish encrypted, authenticated communication. If any MRZ field, such as the document number, is incorrect, the derived keys will not match, mutual authentication will fail, and the chip’s data cannot be read.
Travel documents use different MRZ formats depending on type.
Passports use the TD3 format, while ID cards use the TD1 format. The TD1 format consists of three lines of exactly 30 characters each.
For document numbers exceeding 9 characters, ICAO 9303 Part 5 defines a special "overflow mechanism":
This case was discovered when testing Self with a Belgian ID card that failed to scan properly. The MRZ scanner would detect the text but validation consistently failed. Upon investigation on an iOS device, it was determined that the existing MRZ parsing library (QKMRZParser) does not correctly handle the TD1 overflow format defined in the ICAO 9303 standard.
This overflow format is used by ID cards worldwide when document numbers exceed 9 characters, affecting multiple countries including Belgium, Portugal, Spain, and others. Without proper overflow handling, these ID cards cannot be scanned in Self, and even if scanned, the truncated document number would cause NFC chip authentication to fail.
Problem
Three critical issues were identified with the existing MRZ validation logic:
1. Parser Hard-Coded Indices Breaking Initial Attempted Fix
An initial fix was attempted that detected Belgian IDs by country code (
countryCode == "BEL"), then manually extracted overflow digits from the optional data field (positions 16+) and concatenated them with the principal part to reconstruct the full document number. This approach seemed logical: detect Belgian IDs, manually handle the overflow format, and pass the corrected document number to the rest of the system.However, this fix was broken because the underlying QKMRZParser (iOS) uses hard-coded field indices that don't account for the overflow mechanism. The parser extracts the document number from positions 6-14 only and validates it with the check digit at position 15, completely ignoring the overflow digits at positions 16+.
Result: The custom overflow extraction logic had no effect.
2. Country-Specific Implementation
The initial fix was hard-coded to only work with Belgian IDs (
countryCode == "BEL"). However, the TD1 overflow format is part of the ICAO 9303 international standard and is used by many countries.Result: ID cards from other countries with long document numbers would still fail validation.
3. Stripping First 3 Characters for No Specific Reason
The initial implementation was stripping the first 3 characters from the document number. This wasn't based on any ICAO specification.
Result: NFC authentication would fail because the modified document number wouldn't match the one stored in the chip.
Solution
Changes in
LiveMRZScannerView.swift1. Added ICAO 9303 Check Digit Calculator
0-9→ 0-9,A-Z→ 10-35,<→ 02. Added TD1 Overflow Format Handler
<3. Added TD1 Document Processor
nilif manual check digit validation failsoverrideDocumentNumberstate variable4. Updated Document Number Mapping
overrideDocumentNumberif available (for TD1 overflow cases)TODO
Test NFC authentication on iOS - Validate that the full untruncated document number works correctly for NFC chip authentication (BAC key derivation). Justin is creating a TestFlight build for physical device testing.
Check if same issue exists on Android - Test Android MRZ scanning with ID cards that use TD1 overflow format to verify if the same limitation exists.
Implement Android solution if needed - If Android has the same issue, implement the TD1 overflow validation following the same approach as iOS (manual check digit validation bypassing the parser's hard-coded indices).
Consider updating the parser library - Alternatively, investigate whether QKMRZParser (iOS) and jmrtd (Android) can be updated or forked to natively support TD1 overflow format, which would be a more sustainable long-term solution than the current workaround.
Contact
For questions or issues related to this implementation, please contact: @ArmanKolozyan on Telegram (https://armankolozyan.com).
Summary by CodeRabbit