Skip to content

fix(isVAT): escape dot separators in ID and BR VAT numbers - #2833

Open
pacocartones wants to merge 1 commit into
validatorjs:masterfrom
pacocartones:prep/isVAT-dot-escape
Open

fix(isVAT): escape dot separators in ID and BR VAT numbers#2833
pacocartones wants to merge 1 commit into
validatorjs:masterfrom
pacocartones:prep/isVAT-dot-escape

Conversation

@pacocartones

Copy link
Copy Markdown

Problem

The ID (Indonesia) and BR (Brazil) matchers in src/lib/isVAT.js write their dot separators as a bare .. Outside a character class that is the "any character" metacharacter, not a literal dot:

ID: str => /^(ID)?(\d{15}|(\d{2}.\d{3}.\d{3}.\d{1}-\d{3}.\d{3}))$/.test(str),
BR: str => /^(BR)?((\d{2}.\d{3}.\d{3}\/\d{4}-\d{2})|(\d{3}.\d{3}.\d{3}-\d{2}))$/.test(str),

So any single character is accepted wherever the formatted number requires a dot. On current master:

isVAT('12X345.678.9-012.345', 'ID'); // => true (should be false)
isVAT('12/345.678.9-012.345', 'ID'); // => true (should be false)
isVAT('12.345X678.9-012.345', 'ID'); // => true (should be false)
isVAT('12X345.678/9012-34',   'BR'); // => true (should be false)
isVAT('123X456.789-01',       'BR'); // => true (should be false)

To be clear about severity: this is format hardening, not a security issue. Nothing is bypassed or leaked — isVAT simply 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 in ID, 4 in BR). The \/ in the BR CNPJ branch was already escaped and is untouched.

ID: str => /^(ID)?(\d{15}|(\d{2}\.\d{3}\.\d{3}\.\d{1}-\d{3}\.\d{3}))$/.test(str),
BR: str => /^(BR)?((\d{2}\.\d{3}\.\d{3}\/\d{4}-\d{2})|(\d{3}\.\d{3}\.\d{3}-\d{2}))$/.test(str),

I checked every matcher in the file: ID and BR are the only two with unescaped literal dots, so this closes the whole class in isVAT.js rather than leaving siblings behind.

References

The clearest evidence that the dot was always meant literally is inside this repo. The CH matcher 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/BR fixture in test/validators.test.js uses 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) and XXX.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 true today and false after this PR:

isVAT('12-345-678-9-012-345', 'ID'); // true -> false
isVAT('12 345 678 9-012 345', 'ID'); // true -> false
isVAT('123 456 789-01',       'BR'); // true -> false
isVAT('12-345-678/9012-34',   'BR'); // true -> false

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 ID fixtures and the 4 valid and 2 invalid BR fixtures give identical results before and after.

Tests

Added the malformed strings to the ID and BR invalid lists in test/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.js and re-running:

    $ git stash push src/lib/isVAT.js
    $ npx mocha --require @babel/register test/validators.test.js --grep "should validate VAT numbers"
    
      Validators
        1) should validate VAT numbers
    
      0 passing (42ms)
      1 failing
    
      1) Validators
           should validate VAT numbers:
         Error: validator.isVAT("12X345.678.9-012.345", "ID") passed but should have failed
    
  • With the fix the same command passes (and passes again after git stash pop), npm run lint is clean, and the full npm test is green:

      Validators
        √ should validate VAT numbers
    
      1 passing (51ms)
    
    321 passing (260ms)
    
    =============================== Coverage summary ===============================
    Statements   : 100% ( 2743/2743 )
    Branches     : 96.59% ( 1697/1757 )
    Functions    : 100% ( 427/427 )
    Lines        : 100% ( 2428/2428 )
    ================================================================================
    

Checklist

  • PR contains only changes related; no stray files, etc.
  • README updated (where applicable) — n/a: no signature change and no change to the documented countryCode list
  • Tests written (where applicable)
  • References provided in PR (where applicable)

Disclosure: 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 master before opening this PR.

@codecov

codecov Bot commented Aug 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (3576b41) to head (8ed8a08).

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.
📢 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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
pacocartones force-pushed the prep/isVAT-dot-escape branch from 5bcc372 to 8ed8a08 Compare August 2, 2026 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant