Skip to content

[PM-39358] fix: Reject partial card scans via Luhn and brand-length validation - #2845

Merged
fedemkr merged 4 commits into
mainfrom
PM-39358/fix-partial-card-scan
Jul 16, 2026
Merged

[PM-39358] fix: Reject partial card scans via Luhn and brand-length validation#2845
fedemkr merged 4 commits into
mainfrom
PM-39358/fix-partial-card-scan

Conversation

@fedemkr

@fedemkr fedemkr commented Jun 30, 2026

Copy link
Copy Markdown
Member

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-39358

📔 Objective

Partial OCR card scans could produce digit sequences that matched the regex in CardTextParser and were auto-filled into the card-number field despite being incomplete (e.g. the card was not fully in frame). This PR adds two validation layers inside CardTextParser.extractCardNumber(from:):

  1. Luhn mod-10 checksum — rejects numbers that fail the standard Luhn algorithm, which rules out nearly all truncated card numbers.
  2. Brand-specific digit-length enforcement — after inferring the brand from leading digits, confirms the digit count matches the expected PAN length for that network (e.g. Amex = 15, Visa = 13/16/19, Diners = 14).

CardComponent.Brand is extended with a validDigitLengths: Set<Int> property that drives the length check.

No UI changes; scanner behaviour is unchanged from the user's perspective — the form simply never populates from an invalid number.

…alidation

Partial OCR scans could produce digit sequences that matched the card-number
regex and were auto-filled despite being incomplete. CardTextParser now rejects
any candidate number that fails the Luhn mod-10 checksum or whose digit count
does not match the expected length for the detected brand.
@fedemkr fedemkr added the ai-review Request a Claude code review label Jun 30, 2026
@github-actions github-actions Bot added app:password-manager Bitwarden Password Manager app context t:bug Change Type - Bug labels Jun 30, 2026
@fedemkr
fedemkr marked this pull request as ready for review June 30, 2026 20:33
@fedemkr
fedemkr requested review from a team and matt-livefront as code owners June 30, 2026 20:33
@fedemkr fedemkr added the enhancement New feature or request label Jun 30, 2026
@github-actions

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the addition of Luhn mod-10 checksum and brand-specific digit-length validation to CardTextParser.extractCardNumber(from:), plus the new CardComponent.Brand.validDigitLengths property. The Luhn implementation is correct (check digit never doubled, alternating digits doubled from the right with the standard -9 fold), and the per-brand length table aligns with the prefixes handled in Brand.detect(from:). Both the single-line and merged-fragment extraction paths route through the same validation, so behavior is applied uniformly. Test coverage is thorough and follows Swift Testing / XCTest conventions, and the updated Discover test value is a legitimate fix to satisfy the new Luhn check.

Code Review Details

No findings. The change is well-scoped, correctly implemented, and appropriately tested. No security concerns (no card data is logged or persisted), no architecture violations, and no breaking changes.

Comment thread BitwardenShared/Core/Vault/Services/CardTextParser.swift Outdated
@fedemkr
fedemkr requested a review from matt-livefront July 13, 2026 21:02
matt-livefront
matt-livefront previously approved these changes Jul 13, 2026
@fedemkr
fedemkr merged commit b46f978 into main Jul 16, 2026
14 checks passed
@fedemkr
fedemkr deleted the PM-39358/fix-partial-card-scan branch July 16, 2026 21:29
@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.25%. Comparing base (dae7094) to head (94b8255).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2845   +/-   ##
=======================================
  Coverage   81.25%   81.25%           
=======================================
  Files        1028     1028           
  Lines       66164    66194   +30     
=======================================
+ Hits        53761    53788   +27     
- Misses      12403    12406    +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review app:password-manager Bitwarden Password Manager app context enhancement New feature or request t:bug Change Type - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants