From bf0ad7d9b7876efc95080048678b8ad69063e3e1 Mon Sep 17 00:00:00 2001
From: sy0u1ti <110846601+sy0u1ti@users.noreply.github.com>
Date: Mon, 7 Sep 2026 14:46:07 +0800
Subject: [PATCH] fix(otel): validate card numbers with Luhn check and protect
 context identifiers from PII redaction

---
 .../instrumentation/config.py                 |  8 ++-
 .../instrumentation/pii_redaction.py          | 46 +++++++++++-----
 python/tests/test_pii_redaction.py            | 53 +++++++++++++++++++
 3 files changed, 92 insertions(+), 15 deletions(-)
 create mode 100644 python/tests/test_pii_redaction.py

diff --git a/python/fi_instrumentation/instrumentation/config.py b/python/fi_instrumentation/instrumentation/config.py
index f209c210..ce5e46a6 100644
--- a/python/fi_instrumentation/instrumentation/config.py
+++ b/python/fi_instrumentation/instrumentation/config.py
@@ -283,7 +283,13 @@ def mask(
         ):
             return None
         resolved = value() if callable(value) else value
-        if self.pii_redaction and resolved is not None:
+        if (
+            self.pii_redaction
+            and resolved is not None
+            and key != SpanAttributes.SESSION_ID
+            and key != SpanAttributes.USER_ID
+            and key != SpanAttributes.GEN_AI_CONVERSATION_ID
+        ):
             resolved = redact_pii_in_value(resolved)
         return resolved
 
diff --git a/python/fi_instrumentation/instrumentation/pii_redaction.py b/python/fi_instrumentation/instrumentation/pii_redaction.py
index 6d8c782e..ac9f6ea9 100644
--- a/python/fi_instrumentation/instrumentation/pii_redaction.py
+++ b/python/fi_instrumentation/instrumentation/pii_redaction.py
@@ -24,29 +24,21 @@
 # ---------------------------------------------------------------------------
 # Individual PII patterns — order matters (more specific first).
 # ---------------------------------------------------------------------------
-_EMAIL_RE = re.compile(
-    r"\b[A-Za-z0-9._%+\-]+@[A-Za-z0-9.\-]+\.[A-Za-z]{2,}\b"
-)
+_EMAIL_RE = re.compile(r"\b[A-Za-z0-9._%+\-]+@[A-Za-z0-9.\-]+\.[A-Za-z]{2,}\b")
 
-_SSN_RE = re.compile(
-    r"\b\d{3}[\-\.\s]\d{2}[\-\.\s]\d{4}\b"
-)
+_SSN_RE = re.compile(r"\b\d{3}[\-\.\s]\d{2}[\-\.\s]\d{4}\b")
 
-_CREDIT_CARD_RE = re.compile(
-    r"\b(?:\d[ \-]*?){13,19}\b"
-)
+_CREDIT_CARD_RE = re.compile(r"\b(?:\d[ \-]*?){13,19}\b")
 
 _PHONE_RE = re.compile(
-    r"(?:\+?1[\s\-\.]?)?\(?\d{3}\)?[\s\-\.]?\d{3}[\s\-\.]?\d{4}\b"
+    r"(?<!\d)(?:\+?1[\s\-\.]?)?\(?\d{3}\)?[\s\-\.]?\d{3}[\s\-\.]?\d{4}(?!\d)"
 )
 
 _IP_RE = re.compile(
     r"\b(?:(?:25[0-5]|2[0-4]\d|[01]?\d\d?)\.){3}(?:25[0-5]|2[0-4]\d|[01]?\d\d?)\b"
 )
 
-_API_KEY_RE = re.compile(
-    r"\b(?:sk|pk)[-_](?:live|test|prod)[-_][A-Za-z0-9]{20,}\b"
-)
+_API_KEY_RE = re.compile(r"\b(?:sk|pk)[-_](?:live|test|prod)[-_][A-Za-z0-9]{20,}\b")
 
 # Ordered: most specific → least specific to avoid partial overlaps.
 _PII_PATTERNS: list[tuple[re.Pattern[str], str]] = [
@@ -59,12 +51,38 @@
 ]
 
 
+def _luhn_check(digits: str) -> bool:
+    """Verify Luhn algorithm checksum for credit card numbers."""
+    if not (13 <= len(digits) <= 19):
+        return False
+    total = 0
+    for i, d in enumerate(reversed(digits)):
+        n = int(d)
+        if i % 2 == 1:
+            n *= 2
+            if n > 9:
+                n -= 9
+        total += n
+    return total % 10 == 0
+
+
+def _replace_credit_card(match: re.Match[str]) -> str:
+    matched = match.group(0)
+    digits = re.sub(r"\D", "", matched)
+    if _luhn_check(digits):
+        return "<CREDIT_CARD>"
+    return matched
+
+
 def redact_pii_in_string(text: str) -> str:
     """Scan *text* for PII patterns and replace each match with its entity token."""
     if not text or not _QUICK_CHECK.search(text):
         return text
     for pattern, replacement in _PII_PATTERNS:
-        text = pattern.sub(replacement, text)
+        if pattern is _CREDIT_CARD_RE:
+            text = pattern.sub(_replace_credit_card, text)
+        else:
+            text = pattern.sub(replacement, text)
     return text
 
 
diff --git a/python/tests/test_pii_redaction.py b/python/tests/test_pii_redaction.py
new file mode 100644
index 00000000..aa86e7e9
--- /dev/null
+++ b/python/tests/test_pii_redaction.py
@@ -0,0 +1,53 @@
+from fi_instrumentation.fi_types import SpanAttributes
+from fi_instrumentation.instrumentation.config import TraceConfig
+from fi_instrumentation.instrumentation.pii_redaction import redact_pii_in_string
+
+
+def test_valid_credit_card_is_redacted():
+    # Valid Visa card number passing Luhn algorithm
+    card_text = "Please charge my card 4012-8888-8888-1881 for the subscription."
+    redacted = redact_pii_in_string(card_text)
+    assert "<CREDIT_CARD>" in redacted
+    assert "4012" not in redacted
+
+
+def test_uuid_with_numeric_segments_is_not_corrupted():
+    # UUID from issue #195 whose digits would previously match bare 13-19 digit regex
+    uuid_str = "73630065-0794-4450-a1f9-8cc987a02b09"
+    redacted = redact_pii_in_string(uuid_str)
+    assert redacted == uuid_str
+    assert "<CREDIT_CARD>" not in redacted
+
+
+def test_numeric_timestamps_are_preserved():
+    ts_text = "Event occurred at timestamp 1725700000000 in cluster."
+    redacted = redact_pii_in_string(ts_text)
+    assert redacted == ts_text
+    assert "1725700000000" in redacted
+
+
+def test_trace_config_preserves_session_and_user_ids():
+    cfg = TraceConfig(pii_redaction=True)
+
+    # session.id must remain intact
+    session_id = "73630065-0794-4450-a1f9-8cc987a02b09"
+    masked_session = cfg.mask(SpanAttributes.SESSION_ID, session_id)
+    assert masked_session == session_id
+
+    # user.id must remain intact
+    user_id = "1234567890123"
+    masked_user = cfg.mask(SpanAttributes.USER_ID, user_id)
+    assert masked_user == user_id
+
+
+def test_other_pii_types_continue_to_redact():
+    text = (
+        "User test.user@example.com with SSN 123-45-6789 and IP 192.168.1.10 "
+        "called from +1 (555) 123-4567 with key sk-live-123456789012345678901234."
+    )
+    redacted = redact_pii_in_string(text)
+    assert "<EMAIL_ADDRESS>" in redacted
+    assert "<SSN>" in redacted
+    assert "<IP_ADDRESS>" in redacted
+    assert "<PHONE_NUMBER>" in redacted
+    assert "<API_KEY>" in redacted