fix: core masking correctness (FF3-1, double-encryption, rekey, detection, fail-loud API)#33
Merged
Merged
Conversation
…c pos 6, alpha-only pos 8-9)
… extend candidate gate
…t and 15-34 char lengths
…ep bare matching under explicit selection
…ce radix-10 domain minimum of 6 digits
…ttern stages Thread a shared claim-set through the FPE and consistent pipelines so a later pattern never re-encrypts a span an earlier pattern already masked (e.g. Argentine DNI re-matched by pe_dni, bare CPF re-matched by pesel). All digit maskers are now length-preserving via reinsert_digits, keeping byte offsets stable across stages and preserving the documented original format.
…-rotatable cells mask_pii_fpe_rekey treated the whole cell as one digit string, so any token carrying a separator (SSN, phone, RUT, ...) failed to parse and was silently returned under the OLD key. Rekey now extracts a cell's encrypted digits, rotates them, and reinserts with the original format. A new pattern= argument names the column's token family so rut/co_nit (check digit) and phone (country prefix), which leave some digits unencrypted, rotate exactly. Cells that cannot be rotated now raise instead of passing through unrotated.
BREAKING CHANGE: FF3-1 output differs from prior releases (tokens masked by an older version must be decrypted with that version before re-masking); FPE now rejects digit runs shorter than 6 digits (the FF3-1 minimum domain); unknown patterns= entries and unknown mask_pii mode values now raise instead of returning data unmasked; and CPF/CNPJ/card FPE output is now format-preserving. These changes break the 2.x API contract and warrant a 3.0.0 release.
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
1 participant
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.
Situation
A high-effort multi-agent review of the MaskOps core (not the marketing tooling) surfaced ten verified correctness defects in the masking engine itself — several of which contradict the compliance guarantees the docs advertise and would surface in a buyer's due-diligence review.
Task
Fix the confirmed defects across all three masking modes (asterisk, FPE, consistent) plus detection and API-ergonomics issues, with regression tests and cross-implementation verification where cryptography is involved. Product-level decisions (FF3-1 approach, PE_DNI default, rekey API) were confirmed with the owner before implementing.
Action
pe_dni, bare CPF viapesel). All digit maskers are now length-preserving via a single shared helper, keeping the documented original format.rekey_pii_fpenow rotates separator-bearing tokens (SSN, phone, RUT) instead of silently leaving them under the old key, fails loudly on un-rotatable cells, and takes apattern=argument so families that keep a check digit / country prefix rotate exactly.patterns=names, unknownmask_piimodevalues, and misusedsaltnow raise (ComputeError/ValueError) instead of returning data unmasked.pe_dnicontext-gated in the default pipeline.Result
Unreleasedin the changelog. No version bump is included — releasing (likely a major, given the breaking changes) is a separate decision.