diff --git a/CLAUDE.md b/CLAUDE.md index 8f2184e..fb39e5e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -85,7 +85,7 @@ scrubbed = scrub_dict(action, scrubber) | EMAIL_ADDRESS | john@example.com | | | PHONE_NUMBER | 555-123-4567 | | | US_SSN | 923-45-6789 | | -| CREDIT_CARD | 4532-1234-5678-9012 | | +| CREDIT_CARD | 4111111111111111 | | | DATE_TIME | 01/15/1985 | | | LOCATION | Toronto, ON | | diff --git a/README.md b/README.md index ec3e0d2..6de490d 100644 --- a/README.md +++ b/README.md @@ -38,15 +38,16 @@ Scrubbing is one control inside a reviewed egress process. It is not a guarantee that an artifact is free of protected data, and the evidence behind it is synthetic, not clinical. -`tests/test_phi_recall.py` is the regression gate: 24 synthetic identifiers +`tests/test_phi_recall.py` is the regression gate: 26 synthetic identifiers across names, contact details, financial identifiers, dates of birth, addresses, network identifiers, medical record numbers, member IDs, and -provider licenses. It requires 24 out of 24, and it also checks that ordinary -operational UI text comes back untouched. +provider licenses. It requires 26 out of 26, it pins the entity type each one +must produce, and it also checks that ordinary operational UI text comes back +untouched. Detection is contextual, so the same value scrubs differently depending on what -surrounds it. Every output below was produced by running 1.0.3 on 2026-08-28, -not written by hand. Some of it will surprise you: +surrounds it. Every output below was produced by running this code on +2026-08-28, not written by hand. Some of it will surprise you: ```python >>> from openadapt_privacy.providers.presidio import PresidioScrubbingProvider @@ -59,14 +60,20 @@ not written by hand. Some of it will surprise you: ': ' >>> s.scrub_text("Card 4111111111111111 on file") -' on file' +'Card on file' + +>>> s.scrub_text("Card: 4532-1234-5678-9012") +'Card: ' ``` -The last one is the important one. A card number gets redacted, but as -`DATE_TIME`, not as `CREDIT_CARD`, and the label "Card" goes with it. The -redaction holds; the entity type you get back is not the one you would predict. -Do not build a policy that keys off the placeholder name without measuring it -against your own data first. +The last two are the interesting pair. Both card numbers get redacted, but only +the first is labelled `CREDIT_CARD`, because only the first passes a Luhn +check. An identifier that no recognizer can validate falls to whatever the +spaCy model makes of it, which for a run of digits is usually `DATE_TIME`. + +So: the redaction holds either way, and a validated identifier now carries its +own type. An unvalidated one does not. Measure the placeholder names against +your own data before you route on them. For production egress: scrub a copy, verify every output file, and bind the human or policy approval to the verified artifact. A model that ran without @@ -115,10 +122,23 @@ Only the keys in `PrivacyConfig.SCRUB_KEYS_HTML` are scrubbed, and non-string values pass through, which is why the coordinates survive. Pass `scrub_all=True` to scrub every string regardless of key. -One caveat in 1.0.3: the `text` key is treated as character-separated action -text, joined by `ACTION_TEXT_SEP` (`-`). Scrubbing a plain sentence under that -key returns it hyphenated, one character at a time. Use `value` or `title` for -ordinary prose until that's fixed. +The keys in `PrivacyConfig.SCRUB_KEYS_SEPARATED` (`text` and `canonical_text`) +can also hold recorded keystrokes: one typed character per `ACTION_TEXT_SEP`, +as in `j-o-h-n-@-e-x-a-m-p-l-e-.-c-o-m`. Those are reassembled before analysis, +scrubbed, and separated again, because the PII is invisible in the split form. +Whether that happens is decided by the value, not the key, so prose under +`text` is scrubbed as prose: + +```python +scrub_dict({"text": "Email: john@example.com"}, scrubber) +# {'text': ': '} + +scrub_dict({"text": "-".join("john@example.com")}, scrubber) +# {'text': '<-E-M-A-I-L-_-A-D-D-R-E-S-S->'} +``` + +Pass `separated_keys=[]` to turn keystroke handling off for a call, or +`separated_keys=["keys"]` to move it to your own field name. ## Scrubbing screenshots @@ -183,9 +203,9 @@ PrivacyConfig( ``` The full field list is `SCRUB_CHAR`, `SCRUB_LANGUAGE`, `SCRUB_FILL_COLOR`, -`SCRUB_KEYS_HTML`, `ACTION_TEXT_NAME_PREFIX`, `ACTION_TEXT_NAME_SUFFIX`, -`ACTION_TEXT_SEP`, `SCRUB_CONFIG_TRF`, `SCRUB_PRESIDIO_IGNORE_ENTITIES`, and -`SPACY_MODEL_NAME`. +`SCRUB_KEYS_HTML`, `SCRUB_KEYS_SEPARATED`, `ACTION_TEXT_NAME_PREFIX`, +`ACTION_TEXT_NAME_SUFFIX`, `ACTION_TEXT_SEP`, `SCRUB_CONFIG_TRF`, +`SCRUB_PRESIDIO_IGNORE_ENTITIES`, and `SPACY_MODEL_NAME`. The analyzer's supported entity set comes from Presidio: `CREDIT_CARD`, `CRYPTO`, `DATE_TIME`, `EMAIL_ADDRESS`, `IBAN_CODE`, `IP_ADDRESS`, `LOCATION`, diff --git a/openadapt_privacy/base.py b/openadapt_privacy/base.py index c60d266..2c836b8 100644 --- a/openadapt_privacy/base.py +++ b/openadapt_privacy/base.py @@ -191,6 +191,7 @@ def scrub_dict( list_keys: list[str] | None = None, scrub_all: bool = False, force_scrub_children: bool = False, + separated_keys: list[str] | None = None, ) -> dict[str, Any]: """Scrub PII/PHI from a nested dictionary. @@ -204,18 +205,29 @@ def scrub_dict( scrub_all: If True, scrub all string values regardless of key. force_scrub_children: If True, use aggressive scrubbing for child values after PII is detected in parent. + separated_keys: Keys that may hold character-separated action text. + Defaults to config.SCRUB_KEYS_SEPARATED. Pass an empty list to + scrub every value as prose. Returns: Scrubbed dictionary with PII/PHI removed. """ + policy = effective_config() if list_keys is None: - list_keys = effective_config().SCRUB_KEYS_HTML + list_keys = policy.SCRUB_KEYS_HTML + if separated_keys is None: + separated_keys = policy.SCRUB_KEYS_SEPARATED scrubbed_dict: dict[str, Any] = {} for key, value in input_dict.items(): if self._should_scrub_text(key, value, list_keys, scrub_all): - scrubbed_text = self._scrub_text_item(value, key, force_scrub_children) - if key in ("text", "canonical_text") and self._is_scrubbed(value, scrubbed_text): + scrubbed_text = self._scrub_text_item( + value, + key, + force_scrub_children, + separated_keys=separated_keys, + ) + if key in separated_keys and self._is_scrubbed(value, scrubbed_text): force_scrub_children = True scrubbed_dict[key] = scrubbed_text elif isinstance(value, list): @@ -228,6 +240,7 @@ def scrub_dict( list_keys, scrub_all=list_scrub_all, force_scrub_children=force_scrub_children, + separated_keys=separated_keys, ) if self._should_scrub_list_item( item, @@ -246,6 +259,7 @@ def scrub_dict( value, list_keys, scrub_all=scrub_all or (isinstance(key, str) and key == "state"), + separated_keys=separated_keys, ) else: scrubbed_dict[key] = value @@ -256,17 +270,22 @@ def scrub_list_dicts( self, input_list: list[dict[str, Any]], list_keys: list[str] | None = None, + separated_keys: list[str] | None = None, ) -> list[dict[str, Any]]: """Scrub PII/PHI from a list of dictionaries. Args: input_list: List of dictionaries to be scrubbed. list_keys: List of keys whose values should be scrubbed. + separated_keys: Keys that may hold character-separated action text. Returns: List of scrubbed dictionaries. """ - return [self.scrub_dict(input_dict, list_keys) for input_dict in input_list] + return [ + self.scrub_dict(input_dict, list_keys, separated_keys=separated_keys) + for input_dict in input_list + ] def _should_scrub_text( self, @@ -305,6 +324,7 @@ def _scrub_text_item( value: str, key: str, force_scrub_children: bool = False, + separated_keys: list[str] | None = None, ) -> str: """Scrub a single text value. @@ -312,11 +332,16 @@ def _scrub_text_item( value: Text value to scrub. key: Dictionary key associated with the value. force_scrub_children: If True, use aggressive scrubbing. + separated_keys: Keys that may hold character-separated action text. Returns: Scrubbed text. """ - if key in ("text", "canonical_text"): + if separated_keys is None: + separated_keys = effective_config().SCRUB_KEYS_SEPARATED + if key in separated_keys: + # A permission, not an instruction: the provider applies separated + # handling only to a value that is genuinely a key sequence. return self.scrub_text(value, is_separated=True) if force_scrub_children: return self.scrub_text_all(value) @@ -351,6 +376,7 @@ def _scrub_list_item( list_keys: list[str], force_scrub_children: bool = False, scrub_all: bool = False, + separated_keys: list[str] | None = None, ) -> Any: """Scrub a single list item. @@ -360,6 +386,7 @@ def _scrub_list_item( list_keys: List of keys that should be scrubbed. force_scrub_children: If True, use aggressive scrubbing. scrub_all: If True, scrub every string at every list depth. + separated_keys: Keys that may hold character-separated action text. Returns: Scrubbed item. @@ -370,6 +397,7 @@ def _scrub_list_item( list_keys, scrub_all=scrub_all, force_scrub_children=force_scrub_children, + separated_keys=separated_keys, ) if isinstance(item, list): return [ @@ -380,6 +408,7 @@ def _scrub_list_item( list_keys, scrub_all=scrub_all, force_scrub_children=force_scrub_children, + separated_keys=separated_keys, ) if self._should_scrub_list_item( nested_item, @@ -391,7 +420,7 @@ def _scrub_list_item( ) for nested_item in item ] - return self._scrub_text_item(item, key) + return self._scrub_text_item(item, key, separated_keys=separated_keys) class ScrubbingProviderFactory: diff --git a/openadapt_privacy/config.py b/openadapt_privacy/config.py index 3f33492..c56a2ea 100644 --- a/openadapt_privacy/config.py +++ b/openadapt_privacy/config.py @@ -67,6 +67,10 @@ class PrivacyConfig: SCRUB_LANGUAGE: Language code for NLP analysis (default: "en"). SCRUB_FILL_COLOR: BGR color value for image redaction (default: blue 0x0000FF). SCRUB_KEYS_HTML: List of dict keys that should be scrubbed. + SCRUB_KEYS_SEPARATED: Dict keys whose values may hold character-separated + action text rather than prose. A value under one of these keys is + reassembled before analysis only when it is actually in separated + form; prose under the same key is scrubbed as prose. ACTION_TEXT_NAME_PREFIX: Prefix for action text names (e.g., "<"). ACTION_TEXT_NAME_SUFFIX: Suffix for action text names (e.g., ">"). ACTION_TEXT_SEP: Separator for action text sequences (e.g., "-"). @@ -103,6 +107,16 @@ class PrivacyConfig: ] ) + # Keys whose values may hold a key sequence joined by ACTION_TEXT_SEP. + # Membership here only permits separated handling; the value's own shape + # decides whether it is applied. + SCRUB_KEYS_SEPARATED: list[str] = field( + default_factory=lambda: [ + "text", + "canonical_text", + ] + ) + # Action text formatting (for handling separated text like key sequences) ACTION_TEXT_NAME_PREFIX: str = "<" ACTION_TEXT_NAME_SUFFIX: str = ">" diff --git a/openadapt_privacy/pipelines/dicts.py b/openadapt_privacy/pipelines/dicts.py index 7219744..9973c32 100644 --- a/openadapt_privacy/pipelines/dicts.py +++ b/openadapt_privacy/pipelines/dicts.py @@ -50,6 +50,7 @@ def scrub_dict( scrubber: ScrubbingProvider, list_keys: list[str] | None = None, scrub_all: bool = False, + separated_keys: list[str] | None = None, ) -> dict[str, Any]: """Scrub PII/PHI from a nested dictionary. @@ -61,6 +62,10 @@ def scrub_dict( list_keys: List of keys whose values should be scrubbed. Defaults to config.SCRUB_KEYS_HTML. scrub_all: If True, scrub all string values regardless of key. + separated_keys: Keys that may hold character-separated action text + (a key sequence joined by ACTION_TEXT_SEP). Defaults to + config.SCRUB_KEYS_SEPARATED. Pass an empty list to scrub every + value as prose. Returns: Scrubbed dictionary with PII/PHI removed. @@ -75,13 +80,19 @@ def scrub_dict( >>> scrubbed = scrub_dict(event, scrubber) """ helper = DictScrubber(scrubber) - return helper.scrub_dict(input_dict, list_keys=list_keys, scrub_all=scrub_all) + return helper.scrub_dict( + input_dict, + list_keys=list_keys, + scrub_all=scrub_all, + separated_keys=separated_keys, + ) def scrub_list_dicts( input_list: list[dict[str, Any]], scrubber: ScrubbingProvider, list_keys: list[str] | None = None, + separated_keys: list[str] | None = None, ) -> list[dict[str, Any]]: """Scrub PII/PHI from a list of dictionaries. @@ -91,6 +102,7 @@ def scrub_list_dicts( input_list: List of dictionaries to be scrubbed. scrubber: The ScrubbingProvider to use for text scrubbing. list_keys: List of keys whose values should be scrubbed. + separated_keys: Keys that may hold character-separated action text. Returns: List of scrubbed dictionaries. @@ -105,4 +117,8 @@ def scrub_list_dicts( >>> scrubbed = scrub_list_dicts(events, scrubber) """ helper = DictScrubber(scrubber) - return helper.scrub_list_dicts(input_list, list_keys=list_keys) + return helper.scrub_list_dicts( + input_list, + list_keys=list_keys, + separated_keys=separated_keys, + ) diff --git a/openadapt_privacy/providers/presidio.py b/openadapt_privacy/providers/presidio.py index 5f02ad1..afb1dcf 100644 --- a/openadapt_privacy/providers/presidio.py +++ b/openadapt_privacy/providers/presidio.py @@ -37,6 +37,24 @@ } ) +# Entity types that reach us from the spaCy pipeline. Their spans are model +# output: the model picks the boundary, so a span that overlaps a deterministic +# identifier match is the model reading an identifier plus the words near it. +_STATISTICAL_ENTITIES = frozenset( + { + "DATE_TIME", + "LOCATION", + "NRP", + "ORGANIZATION", + "PERSON", + } +) + +# Presidio scores an unvalidated, context-free digit run at 0.01-0.05. Below +# this floor a pattern recognizer is guessing too, and must not take a label +# away from the NER result. +_MIN_DETERMINISTIC_SCORE = 0.4 + class PrivacyModelUnavailable(RuntimeError): """The required, allowlisted local NLP model is unavailable or invalid.""" @@ -142,6 +160,70 @@ def _filter_automation_false_positives(text: str, analyzer_results: list) -> lis return filtered +def _covers_every_digit(text: str, result, deterministic: list) -> bool: + """Report whether pattern matches claim every digit inside `result`'s span. + + Returns False when nothing overlaps, so a statistical result that stands + alone is never touched. + """ + covered: set[int] = set() + overlaps = False + for other in deterministic: + if result.start < other.end and other.start < result.end: + overlaps = True + covered.update(range(max(result.start, other.start), min(result.end, other.end))) + if not overlaps: + return False + return not any( + text[index].isdigit() for index in range(result.start, result.end) if index not in covered + ) + + +def _prefer_deterministic_identifier_labels(text: str, analyzer_results: list) -> list: + """Give an overlapped span the label of the recognizer that validated it. + + Presidio's anonymizer settles an overlap by span before score, so a wide + spaCy `DATE_TIME` at 0.85 replaced a Luhn-checked `CREDIT_CARD` at 1.0 and + the output named the wrong entity type. A card number that reads as + `` is still redacted, but no policy can route on it. + + The statistical result is dropped only when the pattern matches cover every + digit it claimed, so this can never expose part of an identifier: where the + NER span reaches past the pattern match into more digits, the wider span + stays. + """ + deterministic = [ + result + for result in analyzer_results + if result.entity_type not in _STATISTICAL_ENTITIES + and result.score >= _MIN_DETERMINISTIC_SCORE + ] + if not deterministic: + return analyzer_results + + return [ + result + for result in analyzer_results + if not ( + result.entity_type in _STATISTICAL_ENTITIES + and _covers_every_digit(text, result, deterministic) + ) + ] + + +def _is_separated_form(text: str, separator: str) -> bool: + """Report whether `text` is a key sequence rather than prose. + + Action text is one typed character per separator: `ACTION_TEXT_SEP.join( + "john")` gives `j-o-h-n`. Prose that holds a separator ("follow-up", + "555-123-4567") has chunks longer than one character, so it is prose. + """ + if not separator or separator not in text: + return False + chunks = text.split(separator) + return len(chunks) >= 2 and all(len(chunk) <= 1 for chunk in chunks) + + def _get_analyzer_engine(): """Get or create the Presidio analyzer engine (lazy initialization).""" global _analyzer_engine, _scrubbing_entities @@ -246,8 +328,10 @@ def scrub_text(self, text: str, is_separated: bool = False) -> str: Args: text: Text to be scrubbed. - is_separated: Whether the text contains separated characters - (e.g., key sequences like "a-b-c"). + is_separated: Permit key-sequence handling (e.g. "a-b-c"). The + text is reassembled and re-separated only when it is actually + in that form; prose passes through unchanged, because splitting + prose one character at a time destroys it. Returns: Scrubbed text with PII/PHI replaced by entity type placeholders. @@ -261,11 +345,15 @@ def scrub_text(self, text: str, is_separated: bool = False) -> str: entities = _get_scrubbing_entities() # Handle separated text (e.g., key sequences) - original_text = text - if is_separated and not ( - text.startswith(policy.ACTION_TEXT_NAME_PREFIX) - or text.endswith(policy.ACTION_TEXT_NAME_SUFFIX) - ): + separated = ( + is_separated + and not ( + text.startswith(policy.ACTION_TEXT_NAME_PREFIX) + or text.endswith(policy.ACTION_TEXT_NAME_SUFFIX) + ) + and _is_separated_form(text, policy.ACTION_TEXT_SEP) + ) + if separated: text = "".join(text.split(policy.ACTION_TEXT_SEP)) # Analyze and anonymize @@ -275,6 +363,7 @@ def scrub_text(self, text: str, is_separated: bool = False) -> str: language=policy.SCRUB_LANGUAGE, ) analyzer_results = _filter_automation_false_positives(text, analyzer_results) + analyzer_results = _prefer_deterministic_identifier_labels(text, analyzer_results) logger.debug(f"analyzer_results: {analyzer_results}") @@ -288,10 +377,7 @@ def scrub_text(self, text: str, is_separated: bool = False) -> str: result_text = anonymized_results.text # Restore separator format if needed - if is_separated and not ( - original_text.startswith(policy.ACTION_TEXT_NAME_PREFIX) - or original_text.endswith(policy.ACTION_TEXT_NAME_SUFFIX) - ): + if separated: result_text = policy.ACTION_TEXT_SEP.join(result_text) return result_text diff --git a/tests/test_phi_recall.py b/tests/test_phi_recall.py index 02ac292..53892db 100644 --- a/tests/test_phi_recall.py +++ b/tests/test_phi_recall.py @@ -1,4 +1,8 @@ -"""Quantitative synthetic PHI recall and clean-text regression gate.""" +"""Quantitative synthetic PHI recall and clean-text regression gate. + +Recall alone is not enough. A caller that routes on the entity type needs the +type to be right, so every case below pins the placeholder it must produce. +""" from __future__ import annotations @@ -20,47 +24,64 @@ pytest.skip("Presidio dependencies not installed", allow_module_level=True) +# (category, text, identifier that must disappear, entity type that must fire) PHI_CASES = [ - ("person", "Patient John Smith is ready.", "John Smith"), - ("person", "Schedule Amelia Earhart for follow-up.", "Amelia Earhart"), - ("person", "The chart belongs to Aisha Rahman.", "Aisha Rahman"), - ("person", "Emergency contact is José Alvarez.", "José Alvarez"), - ("person", "Seen by physician Chidi Okafor today.", "Chidi Okafor"), - ("person", "Send the referral to Mei Lin.", "Mei Lin"), - ("person", "Patient Olivia O'Connor arrived.", "Olivia O'Connor"), - ("person", "Discuss results with François Dupont.", "François Dupont"), - ("email", "Email jane.doe@example.com with results.", "jane.doe@example.com"), + ("person", "Patient John Smith is ready.", "John Smith", "PERSON"), + ("person", "Schedule Amelia Earhart for follow-up.", "Amelia Earhart", "PERSON"), + ("person", "The chart belongs to Aisha Rahman.", "Aisha Rahman", "PERSON"), + ("person", "Emergency contact is José Alvarez.", "José Alvarez", "PERSON"), + ("person", "Seen by physician Chidi Okafor today.", "Chidi Okafor", "PERSON"), + ("person", "Send the referral to Mei Lin.", "Mei Lin", "PERSON"), + ("person", "Patient Olivia O'Connor arrived.", "Olivia O'Connor", "PERSON"), + ("person", "Discuss results with François Dupont.", "François Dupont", "PERSON"), + ("email", "Email jane.doe@example.com with results.", "jane.doe@example.com", "EMAIL_ADDRESS"), ( "email", "Portal contact: care-team+east@clinic.example.org", "care-team+east@clinic.example.org", + "EMAIL_ADDRESS", ), - ("phone", "Call the patient at 555-123-4567.", "555-123-4567"), - ("phone", "Mobile: +1 (416) 555-0199", "+1 (416) 555-0199"), - ("ssn", "Social security number 923-45-6789.", "923-45-6789"), + ("phone", "Call the patient at 555-123-4567.", "555-123-4567", "PHONE_NUMBER"), + ("phone", "Mobile: +1 (416) 555-0199", "+1 (416) 555-0199", "PHONE_NUMBER"), + ("ssn", "Social security number 923-45-6789.", "923-45-6789", "US_SSN"), ( "credit_card", - "Card 4532-1234-5678-9012 is on file.", - "4532-1234-5678-9012", + "Card 4532-1234-5678-9014 is on file.", + "4532-1234-5678-9014", + "CREDIT_CARD", ), - ("dob", "Date of birth: 01/15/1985.", "01/15/1985"), - ("dob", "DOB is January 5, 1974.", "January 5, 1974"), + ( + "credit_card", + "Card 4111111111111111 on file", + "4111111111111111", + "CREDIT_CARD", + ), + ( + "bank_account", + "My bank account number is 635526789012.", + "635526789012", + "US_BANK_NUMBER", + ), + ("dob", "Date of birth: 01/15/1985.", "01/15/1985", "DATE_TIME"), + ("dob", "DOB is January 5, 1974.", "January 5, 1974", "DATE_TIME"), ( "address", "Home address: 123 Main Street, Boston, MA 02110.", "123 Main Street", + "STREET_ADDRESS", ), - ("address", "Mail to 88 King St W, Toronto, ON M5H 1J9.", "88 King St W"), - ("ip", "Last login from 192.168.10.22.", "192.168.10.22"), + ("address", "Mail to 88 King St W, Toronto, ON M5H 1J9.", "88 King St W", "STREET_ADDRESS"), + ("ip", "Last login from 192.168.10.22.", "192.168.10.22", "IP_ADDRESS"), ( "url", "Portal is https://patient.example.org/chart/42", "https://patient.example.org/chart/42", + "URL", ), - ("mrn", "Medical record number MRN: 00123456.", "00123456"), - ("mrn", "Patient ID: AB-902771.", "AB-902771"), - ("member_id", "Health plan member ID ZXQ-443-991.", "ZXQ-443-991"), - ("license", "Provider license number ME123456.", "ME123456"), + ("mrn", "Medical record number MRN: 00123456.", "00123456", "MEDICAL_RECORD_NUMBER"), + ("mrn", "Patient ID: AB-902771.", "AB-902771", "MEDICAL_RECORD_NUMBER"), + ("member_id", "Health plan member ID ZXQ-443-991.", "ZXQ-443-991", "MEDICAL_RECORD_NUMBER"), + ("license", "Provider license number ME123456.", "ME123456", "US_DRIVER_LICENSE"), ] CLEAN_CASES = [ @@ -83,7 +104,7 @@ def scrubber() -> PresidioScrubbingProvider: def test_synthetic_phi_identifier_recall_is_complete(scrubber) -> None: misses = [] - for category, text, identifier in PHI_CASES: + for category, text, identifier, _entity in PHI_CASES: scrubbed = scrubber.scrub_text(text) if identifier in scrubbed: misses.append((category, identifier, scrubbed)) @@ -95,6 +116,84 @@ def test_synthetic_phi_identifier_recall_is_complete(scrubber) -> None: ) +def test_synthetic_phi_entity_labels_are_correct(scrubber) -> None: + """The placeholder must name the entity a policy would route on. + + Asserting only that the identifier disappeared hid a systematic + mislabelling: a card number came back as ````. + """ + wrong = [] + for category, text, _identifier, entity in PHI_CASES: + scrubbed = scrubber.scrub_text(text) + if f"<{entity}>" not in scrubbed: + wrong.append((category, entity, scrubbed)) + + assert not wrong, f"{len(wrong)}/{len(PHI_CASES)} cases carry the wrong entity type: {wrong}" + + @pytest.mark.parametrize("text", CLEAN_CASES) def test_clean_operational_text_is_not_redacted(scrubber, text: str) -> None: assert scrubber.scrub_text(text) == text + + +def test_a_card_number_that_fails_the_luhn_check_is_still_redacted(scrubber) -> None: + """Documented limit: no checksum, no ``CREDIT_CARD`` label. + + ``4532-1234-5678-9012`` is not a valid card number, so Presidio's card + recognizer invalidates it. Something else claims the span, and the digits + still leave the output, but the type is not ``CREDIT_CARD``. + """ + scrubbed = scrubber.scrub_text("Card: 4532-1234-5678-9012") + + assert "4532-1234-5678-9012" not in scrubbed + assert "" not in scrubbed + + +def test_a_wider_ner_span_survives_when_it_holds_digits_the_pattern_missed() -> None: + """Relabelling must never shrink what gets redacted. + + Presidio's driver-license recognizer matches only the leading ``A123`` of + ``A123-456-789-012``. If an overlapping ``ORGANIZATION`` span covering the + whole identifier were dropped in favour of that partial match, the trailing + digits would leave the scrubber unredacted. + """ + from presidio_analyzer import RecognizerResult + + from openadapt_privacy.providers.presidio import _prefer_deterministic_identifier_labels + + text = "A123-456-789-012" + partial = RecognizerResult(entity_type="US_DRIVER_LICENSE", start=0, end=4, score=0.65) + whole = RecognizerResult(entity_type="ORGANIZATION", start=0, end=16, score=0.85) + + kept = _prefer_deterministic_identifier_labels(text, [partial, whole]) + + assert whole in kept + + +def test_a_validated_pattern_takes_the_label_from_an_overlapping_ner_span() -> None: + from presidio_analyzer import RecognizerResult + + from openadapt_privacy.providers.presidio import _prefer_deterministic_identifier_labels + + text = "Card 4111111111111111 on file" + card = RecognizerResult(entity_type="CREDIT_CARD", start=5, end=21, score=1.0) + date = RecognizerResult(entity_type="DATE_TIME", start=0, end=21, score=0.85) + + kept = _prefer_deterministic_identifier_labels(text, [card, date]) + + assert kept == [card] + + +def test_a_low_confidence_pattern_does_not_take_a_label() -> None: + """An unvalidated 0.05 digit-run match is a guess, not an identifier.""" + from presidio_analyzer import RecognizerResult + + from openadapt_privacy.providers.presidio import _prefer_deterministic_identifier_labels + + text = "Card 4111111111111111 on file" + weak = RecognizerResult(entity_type="US_BANK_NUMBER", start=5, end=21, score=0.05) + date = RecognizerResult(entity_type="DATE_TIME", start=0, end=21, score=0.85) + + kept = _prefer_deterministic_identifier_labels(text, [weak, date]) + + assert date in kept diff --git a/tests/test_separated_action_text.py b/tests/test_separated_action_text.py new file mode 100644 index 0000000..201e672 --- /dev/null +++ b/tests/test_separated_action_text.py @@ -0,0 +1,88 @@ +"""Separated action text must be decided by the value's shape, not the key name. + +``SCRUB_KEYS_HTML`` contains ``text``, which is also the most common key name in +an arbitrary dict. Treating every ``text`` value as a hyphen-joined key sequence +turned a plain sentence into one character per hyphen. +""" + +from __future__ import annotations + +import pytest + +try: + import spacy + + from openadapt_privacy.config import config + + if not spacy.util.is_package(config.SPACY_MODEL_NAME): + pytest.skip( + f"SpaCy model {config.SPACY_MODEL_NAME} not installed", + allow_module_level=True, + ) + + from openadapt_privacy.pipelines.dicts import scrub_dict + from openadapt_privacy.providers.presidio import PresidioScrubbingProvider +except ImportError: # pragma: no cover - exercised only without the extra + pytest.skip("Presidio dependencies not installed", allow_module_level=True) + + +@pytest.fixture(scope="module") +def scrubber() -> PresidioScrubbingProvider: + return PresidioScrubbingProvider() + + +PROSE_UNDER_TEXT_KEY = "Email: john@example.com" + + +def test_plain_prose_under_the_text_key_is_not_character_separated(scrubber) -> None: + """A sentence under ``text`` comes back as a sentence.""" + result = scrub_dict({"text": PROSE_UNDER_TEXT_KEY}, scrubber)["text"] + + assert "john@example.com" not in result + assert "" in result + assert "-E-M-A-I-L" not in result + assert result.count("-") == PROSE_UNDER_TEXT_KEY.count("-") + + +def test_hyphenated_prose_under_the_text_key_keeps_its_words(scrubber) -> None: + """A single hyphen in ordinary prose does not make the value action text.""" + result = scrub_dict({"text": "Book the follow-up for jane.doe@example.com"}, scrubber)["text"] + + assert "follow-up" in result + assert "" in result + + +def test_separated_action_text_still_round_trips(scrubber) -> None: + """A genuine key sequence is still joined before analysis and re-separated.""" + typed = "-".join("john@example.com") + + result = scrubber.scrub_text(typed, is_separated=True) + + assert "".join(result.split("-")) == "" + + +def test_separated_action_text_under_the_text_key_still_finds_pii(scrubber) -> None: + """The dict path must not lose the separated analysis that hides PII.""" + typed = "-".join("john@example.com") + + result = scrub_dict({"text": typed}, scrubber)["text"] + + assert "john@example.com" not in "".join(result.split("-")) + assert "".join(result.split("-")) == "" + + +def test_separated_handling_can_be_turned_off_per_call(scrubber) -> None: + """``separated_keys`` makes the behaviour explicit instead of implied.""" + typed = "-".join("john@example.com") + + result = scrub_dict({"text": typed}, scrubber, separated_keys=[])["text"] + + assert result == typed + + +def test_a_phone_number_under_the_text_key_survives_as_a_phone_number(scrubber) -> None: + """Hyphen groups of more than one character are not a key sequence.""" + result = scrub_dict({"text": "Call 555-123-4567 now"}, scrubber)["text"] + + assert "555-123-4567" not in result + assert "" in result