Repository navigation
fix(privacy): detect cards after non-card reference numbers - #194
Conversation
johnny4young
left a comment
There was a problem hiding this comment.
Summary: SensitiveDataDetector now runs Luhn on every 13–19 digit candidate in the text, not only the first. An order or reference number that fails the checksum can no longer hide a real card later in the same clip.
Verdict: good to merge as an improvement. There is one should-fix false negative in the same area, which predates this PR.
🟠 A card followed by a short number joins into one candidate and is missed
The regex is greedy and allows a single space or dash between any two digits. A card followed by a CVV or a short code, separated only by a space, therefore becomes a single 19-digit candidate, and that candidate fails Luhn. The 16 digits of the card are never tested on their own. I reproduced this by porting the pattern and Luhn to Python:
| input | detected |
|---|---|
4111 1111 1111 1111 123 (card + CVV) |
❌ |
4111111111111111 123 |
❌ |
Tel 555 1234 4111 1111 1111 1111 |
❌ (the candidate is 555 1234 4111 1111 1111; the trailing 1111 is lost) |
acct 12 4111 1111 1111 1111 |
❌ |
card 4111 1111 1111 1111 cvv 123 |
✅ |
5555-5555-5555-4444 737 |
✅ only by chance: the 19-digit run happens to pass Luhn |
"Card number, space, CVV" is a very common thing to copy. Suggested direction, which is a product trade-off so I did not push it:
- Match maximal digit runs with separators, without the
{12,18}cap:(?<[0-9])[0-9](?:[ -]?[0-9])*(?[0-9]). - Split each run into separator-delimited groups.
- Run Luhn on contiguous group spans of 13–19 digits.
Group boundaries keep the number of extra windows small. Each extra window still has about a 10% chance of passing Luhn, so long space-separated number lists may need a cap, for example "at most 4 groups dropped from either end". A test row for card + CVV belongs in cardAfterNonCard.
🟡 Nits
SensitiveDataDetector.swift: the old(13...19).contains(digits.count)guard is gone. The regex still bounds this to 13–19 digits, so this is fine, but a short comment noting that the bound now comes only from the pattern would help the next reader.- The tests cover the reported case well, including the masked preview and expiry in
ClipItemFactoryTests.
CI is green.
Generated by Claude Code
A card followed by its CVV, or preceded by a phone number or short prefix, joined the neighbouring digits into one 19-digit candidate that failed Luhn, so the card was never checked on its own. Match whole digit runs, split them on single spaces or dashes, and run Luhn on every contiguous span of 13-19 digits. Multi-group spans only count when every group is 3-6 digits, the shapes cards are printed in, so number lists, dates and order segments do not turn into extra Luhn draws.
- Always try the longest leading 13-19 digit span of each run, so 8-8 and other unusual layouts that main already detected stay masked - Compile the digit-run pattern once and enumerate matches in a single pass; re-running String.range per run made large numeric pastes quadratic - Check only the newly added group when extending a span, and keep the span helper private - Cover unusual groupings with a regression test
Summary
Secret detection now inspects every digit run in a clip, not only the first card-shaped one, and tries card-shaped spans inside longer runs. A card is masked and given the short sensitive expiry when it follows an order/reference number that fails Luhn, when it is followed by its CVV, or when it is preceded by a phone number or short prefix.
What changed
SensitiveDataDetector.containsValidCardwalks every maximal digit run (single spaces/dashes allowed between digits).containsCardSpansplits a run on its separators and runs Luhn on every contiguous span of groups totalling 13–19 digits. A single group is a bare card number; a multi-group span only counts when every group has 3–6 digits (4-4-4-4, 4-6-5, 4-4-4-4-3).Why
The previous regex was greedy and allowed a separator between any two digits, so
4111 1111 1111 1111 123(card + CVV) orTel 555 1234 4111 1111 1111 1111became one 19-digit candidate that failed Luhn, and the card was never checked on its own. The group-length rule keeps lists of small numbers, ISO dates and 7-digit order segments from turning into many extra Luhn draws (each extra span is a ~10% false-positive chance, and a false positive expires the clip).How it was tested
swift:6.0.3-noble,-swift-version 6) on Linux in a standalone harness and all 14 positive and 16 negative fixtures passed;swift-format lint --strictpasses on the changed files. A Python port of the algorithm was used to choose fixtures.Review follow-ups
e97157b— fix(privacy): keep legacy card groupings and scan digit runs in one pass (restores the old leading-span check; compiles the run pattern once so large numeric pastes stay linear).8db0669— fix(privacy): try grouped card spans inside longer digit runs (card + CVV / phone prefix false negatives from the review).Decisions taken
41111111 11111111) stay detected.Known limitations & follow-ups
Merge order
Independent of the other PRs; merge before the v0.10.0 release PR #193.
CI
macOS jobs for
8db0669were queued when this description was written; check the PR checks tab.Original description (state at
7c53712):Problem
Secret detection stopped after the first 13–19-digit candidate. If a copied receipt or note began with a reference number that failed Luhn, a later valid card was never checked. That mixed clip therefore missed sensitive masking and short expiry.
Change
Verification
git diff --checkpasses7c537124d63925f46260a200544b01483cce4828: swift-format, SwiftLint, 1,503 Swift tests across 232 suites, 83.9% production package-source coverage (80% floor), StoreKit tests, macOS build and iOS buildScope
No release metadata, database schema, sync policy, dependency version, or existing release-candidate PR changes. No merge or release performed.
Human-review readiness
Scoped implementation and regression coverage are complete. The exact published head remains 7c53712; both required native jobs and GitGuardian are successful, with the expected non-blocking beta-canary skip. No discussion comments, submitted reviews or unresolved inline threads are pending. This status requests source review only; it does not authorize merge, release or deployment.
🤖 Generated with Claude Code
https://claude.ai/code/session_01ExmnfUngrDYjAheGvYwMnn
Generated by Claude Code