fix(isVAT): escape dot separators in ID and BR VAT numbers - #2833
Open
pacocartones wants to merge 1 commit into
Open
fix(isVAT): escape dot separators in ID and BR VAT numbers#2833pacocartones wants to merge 1 commit into
pacocartones wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2833 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 114 114
Lines 2598 2598
Branches 658 658
=========================================
Hits 2598 2598 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The ID (Indonesia) and BR (Brazil) matchers wrote their dot separators as
a bare `.`, which outside a character class is the "any character"
metacharacter rather than a literal dot. As a result malformed numbers
were accepted, e.g. isVAT('12X345.678.9-012.345', 'ID') and
isVAT('123X456.789-01', 'BR') both returned true.
Escape the separators (`.` -> `\.`) so only the printed NPWP
(XX.XXX.XXX.X-XXX.XXX), CNPJ (XX.XXX.XXX/XXXX-XX) and CPF
(XXX.XXX.XXX-XX) forms match. This matches the CH matcher in the same
file, which already writes `\d{3}\.\d{3}\.\d{3}`. Regression tests cover
each separator position.
Note this also stops hyphen- and space-separated variants that passed by
accident (e.g. '123 456 789-01' for BR); those are not valid renderings
of these numbers.
pacocartones
force-pushed
the
prep/isVAT-dot-escape
branch
from
August 2, 2026 09:00
5bcc372 to
8ed8a08
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The
ID(Indonesia) andBR(Brazil) matchers insrc/lib/isVAT.jswrite their dot separators as a bare.. Outside a character class that is the "any character" metacharacter, not a literal dot:So any single character is accepted wherever the formatted number requires a dot. On current
master:To be clear about severity: this is format hardening, not a security issue. Nothing is bypassed or leaked —
isVATsimply accepts malformed strings that no tax authority would ever print. It is the same class of over-permissive matcher as the literal comma in[1,4,5]/[J,G,V,E]fixed in #2814.Fix
Escape the separators,
.→\.(4 occurrences inID, 4 inBR). The\/in theBRCNPJ branch was already escaped and is untouched.I checked every matcher in the file:
IDandBRare the only two with unescaped literal dots, so this closes the whole class inisVAT.jsrather than leaving siblings behind.References
The clearest evidence that the dot was always meant literally is inside this repo. The
CHmatcher a few lines above already escapes exactly this kind of separator:/^(CHE[- ]?)?(\d{9}|(\d{3}\.\d{3}\.\d{3})|(\d{3} \d{3} \d{3})) ?(TVA|MWST|IVA)?$/and every existing
ID/BRfixture intest/validators.test.jsuses real dots —'12.345.678.9-012.345','12.345.678/9012-34','123.456.789-01'.The printed masks —
XX.XXX.XXX.X-XXX.XXX(NPWP),XX.XXX.XXX/XXXX-XX(CNPJ) andXXX.XXX.XXX-XX(CPF) — are the renderings the tax authorities put on the documents themselves. Their sites are linked for reference (they are service portals, not format specifications):Behaviour change worth flagging
Escaping also removes some accidental permissiveness. These return
truetoday andfalseafter this PR:Hyphen- and space-separated renderings are not valid NPWP/CNPJ/CPF formats, so rejecting them is the intended outcome — but it is a tightening, not a pure bug fix, and anyone relying on the accidental permissiveness would notice. Flagging it rather than burying it. Happy to drop the change and keep only the "junk character" cases if you'd rather not tighten this in a patch release.
Everything that is supposed to keep working does: the 4 valid and 2 invalid
IDfixtures and the 4 valid and 2 invalidBRfixtures give identical results before and after.Tests
Added the malformed strings to the
IDandBRinvalidlists intest/validators.test.js— 5 and 4 entries, one per separator position, plus one using/to show it is genuinely "any character" and not just letters.Without the src change the new fixtures genuinely fail — verified by stashing only
src/lib/isVAT.jsand re-running:With the fix the same command passes (and passes again after
git stash pop),npm run lintis clean, and the fullnpm testis green:Checklist
countryCodelistDisclosure: this patch was prepared with AI assistance (Claude). The two regexes, the nine new fixtures and the revert-and-fail check were each executed against
masterbefore opening this PR.