From e80acebd1624b8e00fc9a4d981454cfb2e5d355b Mon Sep 17 00:00:00 2001 From: Rhianna Litchfield Date: Tue, 14 Jul 2026 10:42:19 -0400 Subject: [PATCH 1/3] fix(pii): never echo original PII and generate guaranteed-invalid dummies Part of #465 (the lookup-first mapping half already landed in #468). - GenerateReplacement now guarantees the replacement differs from the original, falling back to the [REDACTED_...] placeholder after repeated generator collisions, so a dummy can never leak the PII it is meant to mask. - StreetGenerator uses pickExcluding like the other list generators. - SSNs use the never-issued 900-999 area range, phone numbers the reserved 555-0100..0199 fictional block, and credit card numbers deliberately fail the Luhn checksum, so dummies look realistic but can never collide with a real person's data. - Tests for the new invariants; gofmt pass over the touched test file. Co-Authored-By: Claude Fable 5 --- .changeset/pii-generator-invalid-formats.md | 5 + src/backend/pii/generator_service.go | 17 ++- src/backend/pii/generator_service_test.go | 42 ++++++ src/backend/pii/generators/pii_generators.go | 73 +++++++--- .../pii/generators/pii_generators_test.go | 131 +++++++++++++----- 5 files changed, 214 insertions(+), 54 deletions(-) create mode 100644 .changeset/pii-generator-invalid-formats.md create mode 100644 src/backend/pii/generator_service_test.go diff --git a/.changeset/pii-generator-invalid-formats.md b/.changeset/pii-generator-invalid-formats.md new file mode 100644 index 00000000..780e824b --- /dev/null +++ b/.changeset/pii-generator-invalid-formats.md @@ -0,0 +1,5 @@ +--- +"kiji-privacy-proxy": patch +--- + +- PII replacements are now guaranteed to differ from the original value (falling back to a `[REDACTED_...]` placeholder after repeated collisions), and generated SSNs, phone numbers, and credit card numbers use realistic-looking but guaranteed-invalid formats: never-issued 900-999 SSN area numbers, the reserved 555-01XX fictional phone block, and card numbers that deliberately fail the Luhn checksum. Street replacements no longer echo the original street name. diff --git a/src/backend/pii/generator_service.go b/src/backend/pii/generator_service.go index 9067b05c..9ab1224b 100644 --- a/src/backend/pii/generator_service.go +++ b/src/backend/pii/generator_service.go @@ -50,10 +50,23 @@ func NewGeneratorService() *GeneratorService { } } -// GenerateReplacement generates a replacement for the given PII label and original text +// generateAttempts is how many times a label's generator may collide with the +// original before we fall back to the deterministic placeholder. +const generateAttempts = 3 + +// GenerateReplacement generates a replacement for the given PII label and +// original text. The replacement is guaranteed to differ from the original: +// returning it unchanged would leak the very PII we are masking, so after a +// few collisions we fall back to GenericGenerator's [REDACTED_...] placeholder, +// which embeds a hash of the original and therefore never equals it. func (s *GeneratorService) GenerateReplacement(label, originalText string) string { generator := s.getGeneratorForLabel(label) - return generator(originalText) + for attempt := 0; attempt < generateAttempts; attempt++ { + if replacement := generator(originalText); replacement != originalText { + return replacement + } + } + return piiGenerators.GenericGenerator(label, originalText) } // getGeneratorForLabel returns the appropriate generator function for the given label diff --git a/src/backend/pii/generator_service_test.go b/src/backend/pii/generator_service_test.go new file mode 100644 index 00000000..83bb1880 --- /dev/null +++ b/src/backend/pii/generator_service_test.go @@ -0,0 +1,42 @@ +package pii + +import ( + "strings" + "testing" +) + +// generatorLabels is every label GenerateReplacement routes to a dedicated +// generator, plus an unknown label to cover the generic fallback path. +var generatorLabels = []string{ + labelSurname, labelFirstName, labelBuildingNum, labelDateOfBirth, + labelEmail, labelPhoneNumber, labelCity, labelURL, labelCompanyName, + labelState, labelZip, labelStreet, labelCountry, labelSSN, + labelDriverLicenseNum, labelPassportID, labelNationalID, labelIDCardNum, + labelTaxNum, labelLicensePlateNum, labelPassword, labelIBAN, labelAge, + labelSecurityToken, labelCreditCardNumber, labelUsername, + "UNKNOWNLABEL", +} + +// A replacement equal to the original would leak the PII it is supposed to +// mask, so GenerateReplacement guarantees inequality. Feeding a previous +// output back in as the original is the most collision-prone input possible, +// and must still always produce something different. +func TestGenerateReplacementNeverReturnsOriginal(t *testing.T) { + service := NewGeneratorService() + for _, label := range generatorLabels { + original := service.GenerateReplacement(label, "") + for i := 0; i < 100; i++ { + if replacement := service.GenerateReplacement(label, original); replacement == original { + t.Fatalf("GenerateReplacement(%q) returned the original %q", label, original) + } + } + } +} + +func TestGenerateReplacementUnknownLabelUsesPlaceholder(t *testing.T) { + service := NewGeneratorService() + replacement := service.GenerateReplacement("SOMETHINGELSE", "raw value") + if !strings.HasPrefix(replacement, "[REDACTED_SOMETHINGELSE_") { + t.Errorf("expected generic placeholder for unknown label, got %q", replacement) + } +} diff --git a/src/backend/pii/generators/pii_generators.go b/src/backend/pii/generators/pii_generators.go index bee67d2b..af86ad09 100644 --- a/src/backend/pii/generators/pii_generators.go +++ b/src/backend/pii/generators/pii_generators.go @@ -71,44 +71,75 @@ func EmailGenerator(rng *rand.Rand, original string) string { return fmt.Sprintf("%s.%s@%s", firstName, lastName, domain) } -// PhoneGenerator generates dummy phone numbers +// PhoneGenerator generates dummy phone numbers. The exchange is fixed to 555 +// and the line number to 0100-0199 — the NANP block reserved for fictional +// use — so the output looks real but can never be a dialable subscriber +// number belonging to someone. func PhoneGenerator(rng *rand.Rand, original string) string { // Generate a random 3-digit area code (200-999) areaCode := 200 + rng.Intn(800) - // Generate a random 3-digit exchange (200-999) - exchange := 200 + rng.Intn(800) - - // Generate a random 4-digit number - number := 1000 + rng.Intn(9000) + // Line number in the reserved fictional range 0100-0199 + line := 100 + rng.Intn(100) // Randomly choose format - formats := []string{"%d-%d-%d", "%d.%d.%d", "(%d) %d-%d"} + formats := []string{"%d-555-%04d", "%d.555.%04d", "(%d) 555-%04d"} format := formats[rng.Intn(len(formats))] - return fmt.Sprintf(format, areaCode, exchange, number) + return fmt.Sprintf(format, areaCode, line) } -// SSNGenerator generates dummy SSN numbers (SOCIALNUM) +// SSNGenerator generates dummy SSN numbers (SOCIALNUM). The area number is +// drawn from 900-999, which the SSA has never issued and has reserved against +// future issuance, so the output looks real but can never collide with an +// actual person's SSN. func SSNGenerator(rng *rand.Rand, original string) string { - // Generate random numbers, avoiding obvious patterns - first := 100 + rng.Intn(900) // 100-999 - second := 10 + rng.Intn(90) // 10-99 - third := 1000 + rng.Intn(9000) // 1000-9999 + area := 900 + rng.Intn(100) // 900-999: never issued + group := 10 + rng.Intn(90) // 10-99 + serial := 1000 + rng.Intn(9000) // 1000-9999 + + return fmt.Sprintf("%d-%d-%d", area, group, serial) +} - return fmt.Sprintf("%d-%d-%d", first, second, third) +// luhnCheckDigit returns the check digit that would make digits followed by +// that digit pass the Luhn checksum. +func luhnCheckDigit(digits []int) int { + sum := 0 + // Walk right to left; with the check digit occupying the final position, + // the rightmost digit here sits in a doubled position. + for i := 0; i < len(digits); i++ { + d := digits[len(digits)-1-i] + if i%2 == 0 { + d *= 2 + if d > 9 { + d -= 9 + } + } + sum += d + } + return (10 - sum%10) % 10 } -// CreditCardGenerator generates dummy credit card numbers +// CreditCardGenerator generates dummy credit card numbers that deliberately +// fail the Luhn checksum: the final digit is chosen to be anything except the +// valid check digit, so the output looks real but is never an issuable card +// number. func CreditCardGenerator(rng *rand.Rand, original string) string { - // Generate 4 groups of 4 digits - groups := make([]int, 4) - for i := range groups { - groups[i] = 1000 + rng.Intn(9000) + digits := make([]int, 16) + // Lead with a real-looking network digit (Visa/Mastercard/Discover style) + digits[0] = 4 + rng.Intn(3) + for i := 1; i < 15; i++ { + digits[i] = rng.Intn(10) + } + digits[15] = (luhnCheckDigit(digits[:15]) + 1 + rng.Intn(9)) % 10 + + var groups [4]string + for g := 0; g < 4; g++ { + groups[g] = fmt.Sprintf("%d%d%d%d", digits[g*4], digits[g*4+1], digits[g*4+2], digits[g*4+3]) } // Randomly choose format - formats := []string{"%d %d %d %d", "%d-%d-%d-%d"} + formats := []string{"%s %s %s %s", "%s-%s-%s-%s"} format := formats[rng.Intn(len(formats))] return fmt.Sprintf(format, groups[0], groups[1], groups[2], groups[3]) @@ -158,7 +189,7 @@ func StreetGenerator(rng *rand.Rand, original string) string { "King Street", "Manor Road", "Park Lane", "The Crescent", "Green Lane", "Mill Lane", "New Road", "Chapel Street", "West End", "North Terrace", } - return streetNames[rng.Intn(len(streetNames))] + return pickExcluding(rng, streetNames, original) } // ZipCodeGenerator generates dummy zip codes diff --git a/src/backend/pii/generators/pii_generators_test.go b/src/backend/pii/generators/pii_generators_test.go index 2b3d1ab0..7a09e935 100644 --- a/src/backend/pii/generators/pii_generators_test.go +++ b/src/backend/pii/generators/pii_generators_test.go @@ -103,7 +103,7 @@ func TestUsernameGenerator(t *testing.T) { func TestDateOfBirthGenerator(t *testing.T) { rng := getTestRand(42) - + // Test with empty original result := DateOfBirthGenerator(rng, "") datePattern := regexp.MustCompile(`^\d{2}/\d{2}/\d{4}$`) @@ -141,7 +141,7 @@ func TestStreetGenerator(t *testing.T) { func TestZipCodeGenerator(t *testing.T) { rng := getTestRand(42) - + // Test basic 5-digit zip result := ZipCodeGenerator(rng, "") zipPattern := regexp.MustCompile(`^\d{5}$`) @@ -255,9 +255,9 @@ func TestUrlGenerator(t *testing.T) { } // Check that it uses reserved domains - hasReservedDomain := strings.Contains(result, "example.") || - strings.Contains(result, "test.") || - strings.Contains(result, "invalid.") || + hasReservedDomain := strings.Contains(result, "example.") || + strings.Contains(result, "test.") || + strings.Contains(result, "invalid.") || strings.Contains(result, "localhost.") if !hasReservedDomain { t.Errorf("UrlGenerator returned URL with non-reserved domain: %s", result) @@ -435,6 +435,75 @@ func TestGenericGenerator(t *testing.T) { } } +// The exchange must be 555 and the line number 0100-0199 — the NANP block +// reserved for fictional use — so a generated number is never dialable. +func TestPhoneGeneratorFictionalRange(t *testing.T) { + phonePattern := regexp.MustCompile(`^(\(\d{3}\) 555-01\d{2}|\d{3}-555-01\d{2}|\d{3}\.555\.01\d{2})$`) + for seed := int64(0); seed < 50; seed++ { + result := PhoneGenerator(getTestRand(seed), "") + if !phonePattern.MatchString(result) { + t.Errorf("PhoneGenerator returned number outside the 555-01XX fictional range: %s", result) + } + } +} + +// The area number must fall in 900-999, which the SSA has never issued, so a +// generated SSN can never collide with a real person's. +func TestSSNGeneratorNeverIssuedArea(t *testing.T) { + for seed := int64(0); seed < 50; seed++ { + result := SSNGenerator(getTestRand(seed), "") + var area, group, serial int + if _, err := fmt.Sscanf(result, "%d-%d-%d", &area, &group, &serial); err != nil { + t.Fatalf("SSNGenerator returned unparseable SSN: %s", result) + } + if area < 900 || area > 999 { + t.Errorf("SSNGenerator returned area %d, expected never-issued range 900-999: %s", area, result) + } + } +} + +// luhnValid reports whether a full card number (check digit included) passes +// the Luhn checksum. +func luhnValid(digits []int) bool { + sum := 0 + for i := 0; i < len(digits); i++ { + d := digits[len(digits)-1-i] + if i%2 == 1 { + d *= 2 + if d > 9 { + d -= 9 + } + } + sum += d + } + return sum%10 == 0 +} + +// Generated card numbers must deliberately fail the Luhn checksum so they can +// never be an issuable card number. +func TestCreditCardGeneratorFailsLuhn(t *testing.T) { + for seed := int64(0); seed < 100; seed++ { + result := CreditCardGenerator(getTestRand(seed), "") + cleaned := strings.NewReplacer(" ", "", "-", "").Replace(result) + digits := make([]int, 0, len(cleaned)) + for _, ch := range cleaned { + digits = append(digits, int(ch-'0')) + } + if luhnValid(digits) { + t.Errorf("CreditCardGenerator returned a Luhn-valid number: %s", result) + } + } +} + +// Echoing the original street back would leak the PII we are masking. +func TestStreetGeneratorExcludesOriginal(t *testing.T) { + for seed := int64(0); seed < 100; seed++ { + if result := StreetGenerator(getTestRand(seed), "Main St"); result == "Main St" { + t.Errorf("StreetGenerator returned the original street name (seed %d)", seed) + } + } +} + // Test that generators produce different outputs with different seeds func TestGeneratorsDifferentSeeds(t *testing.T) { rng1 := getTestRand(1) @@ -464,32 +533,32 @@ func TestGeneratorsSameSeed(t *testing.T) { // Test all generators return non-empty strings func TestAllGeneratorsNonEmpty(t *testing.T) { generators := map[string]func(*rand.Rand, string) string{ - "EmailGenerator": EmailGenerator, - "PhoneGenerator": PhoneGenerator, - "SSNGenerator": SSNGenerator, - "CreditCardGenerator": CreditCardGenerator, - "UsernameGenerator": UsernameGenerator, - "DateOfBirthGenerator": DateOfBirthGenerator, - "StreetGenerator": StreetGenerator, - "ZipCodeGenerator": ZipCodeGenerator, - "CityGenerator": CityGenerator, - "BuildingNumGenerator": BuildingNumGenerator, - "FirstNameGenerator": FirstNameGenerator, - "SurnameGenerator": SurnameGenerator, - "IDCardNumGenerator": IDCardNumGenerator, - "DriverLicenseNumGenerator": DriverLicenseNumGenerator, - "TaxNumGenerator": TaxNumGenerator, - "UrlGenerator": UrlGenerator, - "CompanyNameGenerator": CompanyNameGenerator, - "StateGenerator": StateGenerator, - "CountryGenerator": CountryGenerator, - "PassportIdGenerator": PassportIdGenerator, - "NationalIdGenerator": NationalIdGenerator, - "LicensePlateNumGenerator": LicensePlateNumGenerator, - "PasswordGenerator": PasswordGenerator, - "IbanGenerator": IbanGenerator, - "AgeGenerator": AgeGenerator, - "SecurityTokenGenerator": SecurityTokenGenerator, + "EmailGenerator": EmailGenerator, + "PhoneGenerator": PhoneGenerator, + "SSNGenerator": SSNGenerator, + "CreditCardGenerator": CreditCardGenerator, + "UsernameGenerator": UsernameGenerator, + "DateOfBirthGenerator": DateOfBirthGenerator, + "StreetGenerator": StreetGenerator, + "ZipCodeGenerator": ZipCodeGenerator, + "CityGenerator": CityGenerator, + "BuildingNumGenerator": BuildingNumGenerator, + "FirstNameGenerator": FirstNameGenerator, + "SurnameGenerator": SurnameGenerator, + "IDCardNumGenerator": IDCardNumGenerator, + "DriverLicenseNumGenerator": DriverLicenseNumGenerator, + "TaxNumGenerator": TaxNumGenerator, + "UrlGenerator": UrlGenerator, + "CompanyNameGenerator": CompanyNameGenerator, + "StateGenerator": StateGenerator, + "CountryGenerator": CountryGenerator, + "PassportIdGenerator": PassportIdGenerator, + "NationalIdGenerator": NationalIdGenerator, + "LicensePlateNumGenerator": LicensePlateNumGenerator, + "PasswordGenerator": PasswordGenerator, + "IbanGenerator": IbanGenerator, + "AgeGenerator": AgeGenerator, + "SecurityTokenGenerator": SecurityTokenGenerator, } for name, generator := range generators { From 571d443521819251f81e347ceec4744283b346d9 Mon Sep 17 00:00:00 2001 From: Rhianna Litchfield Date: Thu, 16 Jul 2026 14:39:54 -0400 Subject: [PATCH 2/3] chore: drop changeset, maintainers add it at release time Requested in review: changesets for this repo are set by the maintainers when cutting the next version. Co-Authored-By: Claude Fable 5 --- .changeset/pii-generator-invalid-formats.md | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 .changeset/pii-generator-invalid-formats.md diff --git a/.changeset/pii-generator-invalid-formats.md b/.changeset/pii-generator-invalid-formats.md deleted file mode 100644 index 780e824b..00000000 --- a/.changeset/pii-generator-invalid-formats.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"kiji-privacy-proxy": patch ---- - -- PII replacements are now guaranteed to differ from the original value (falling back to a `[REDACTED_...]` placeholder after repeated collisions), and generated SSNs, phone numbers, and credit card numbers use realistic-looking but guaranteed-invalid formats: never-issued 900-999 SSN area numbers, the reserved 555-01XX fictional phone block, and card numbers that deliberately fail the Luhn checksum. Street replacements no longer echo the original street name. From 6d677d8644b479433e2064133693196d62fc39db Mon Sep 17 00:00:00 2001 From: Rhianna Litchfield Date: Thu, 16 Jul 2026 14:46:06 -0400 Subject: [PATCH 3/3] fix(pii): keep dummy SSN group digits outside ITIN ranges Review follow-up: 9XX numbers with group digits 50-65, 70-88, 90-92, or 94-99 are the IRS ITIN space, so a generated dummy could match a real person's ITIN. Constrain the group to 10-49 so the output avoids both issued SSN areas (900-999 was never assigned) and all ITIN ranges. Co-Authored-By: Claude Fable 5 --- src/backend/pii/generators/pii_generators.go | 11 +++++++---- src/backend/pii/generators/pii_generators_test.go | 7 ++++++- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/backend/pii/generators/pii_generators.go b/src/backend/pii/generators/pii_generators.go index af86ad09..08ab79e1 100644 --- a/src/backend/pii/generators/pii_generators.go +++ b/src/backend/pii/generators/pii_generators.go @@ -91,11 +91,14 @@ func PhoneGenerator(rng *rand.Rand, original string) string { // SSNGenerator generates dummy SSN numbers (SOCIALNUM). The area number is // drawn from 900-999, which the SSA has never issued and has reserved against -// future issuance, so the output looks real but can never collide with an -// actual person's SSN. +// future issuance (every issued prefix, including everything in the SSA high +// group list, lies in 001-899), so the output looks real but can never +// collide with an actual person's SSN. The group number stays in 10-49 +// because IRS ITINs also start with 9 and use group ranges 50-65, 70-88, +// 90-92, and 94-99; staying below 50 avoids colliding with those too. func SSNGenerator(rng *rand.Rand, original string) string { - area := 900 + rng.Intn(100) // 900-999: never issued - group := 10 + rng.Intn(90) // 10-99 + area := 900 + rng.Intn(100) // 900-999: never issued as SSNs + group := 10 + rng.Intn(40) // 10-49: outside all ITIN group ranges serial := 1000 + rng.Intn(9000) // 1000-9999 return fmt.Sprintf("%d-%d-%d", area, group, serial) diff --git a/src/backend/pii/generators/pii_generators_test.go b/src/backend/pii/generators/pii_generators_test.go index 7a09e935..993a983c 100644 --- a/src/backend/pii/generators/pii_generators_test.go +++ b/src/backend/pii/generators/pii_generators_test.go @@ -448,7 +448,9 @@ func TestPhoneGeneratorFictionalRange(t *testing.T) { } // The area number must fall in 900-999, which the SSA has never issued, so a -// generated SSN can never collide with a real person's. +// generated SSN can never collide with a real person's. The group number must +// stay in 10-49: IRS ITINs also start with 9 and use groups 50-65, 70-88, +// 90-92, and 94-99, so anything 50+ risks colliding with a real ITIN. func TestSSNGeneratorNeverIssuedArea(t *testing.T) { for seed := int64(0); seed < 50; seed++ { result := SSNGenerator(getTestRand(seed), "") @@ -459,6 +461,9 @@ func TestSSNGeneratorNeverIssuedArea(t *testing.T) { if area < 900 || area > 999 { t.Errorf("SSNGenerator returned area %d, expected never-issued range 900-999: %s", area, result) } + if group < 10 || group > 49 { + t.Errorf("SSNGenerator returned group %d, expected 10-49 to avoid ITIN ranges: %s", group, result) + } } }