Redaction: find credential names by their parts, not one app's list

packages/flow/openadapt_flow/network_capture.py

@@ -75,7 +75,7 @@ def _page_of(request: Any) -> Any:
     re.I,
 )
 SECRET_NAME = re.compile(
-    r"pass(word|wd|phrase)?\b|passwd|clearpass|pwd|secret|private[_-]?key|^pin$|^otp$",
+    r"pass(word|wd|phrase)?\b|passwd|pwd|secret|private[_-]?key|^pin$|^otp$",
     re.I,
 )
 SECRET = "<secret>"

packages/flow/openadapt_flow/report_redaction.py

@@ -2,16 +2,18 @@
 
 A run report (``report.json``) is a shareable artifact: an offline Seal signs
 its exact bytes and the signed report can be published as a receipt. URLs the
-run saw can carry the application's session: OpenEMR puts its per-session
-CSRF token in ``main.php?token_main=...``, OAuth redirects carry ``code`` and
+run saw can carry the application's session: an app may put its per-session
+CSRF token in a query parameter such as ``main.php?csrf_token=...``, OAuth redirects carry ``code`` and
 ``state``, and a captured header can hold ``Authorization: Bearer ...`` or a
 ``Cookie``. None of that belongs in a receipt.
 
 :func:`redact_report` walks every string in the report and replaces
 
-* the value of a secret query, fragment or form parameter (``token_main=``,
-  ``csrf_token_form=``, ``PHPSESSID=``, ``access_token=``, ``code=``,
-  ``state=``, ``signature=`` and the rest of :data:`DEFAULT_PARAMS`);
+* the value of a secret query, fragment or form parameter: one in
+  :data:`DEFAULT_PARAMS` (``PHPSESSID=``, ``access_token=``, ``code=``,
+  ``state=``, ``signature=``, ...) or one whose name has a CSRF, XSRF,
+  token or session-id part (:func:`is_credential_name`: ``csrf_token_form=``,
+  ``token_main=``, ``auth_token=``);
 * the value of a credential header (``Authorization``, ``Cookie``,
   ``Set-Cookie`` and the rest of :data:`DEFAULT_HEADERS`), in a
   ``Name: value`` line, a JSON key, or a ``{"name": ..., "value": ...}`` pair;
@@ -40,7 +42,7 @@
 
 ``keep`` may only name the parameters that are also ordinary words in some
 apps (:data:`KEEPABLE_PARAMS`, such as ``state`` for a US state field); a
-credential name such as ``token_main`` or ``access_token`` is always redacted.
+credential name such as ``csrf_token`` or ``access_token`` is always redacted.
 Nothing here touches the network.
 """
 
@@ -65,9 +67,8 @@
 #: credential, OAuth grant or request signature. Compared case-insensitively.
 DEFAULT_PARAMS: Final = frozenset(
     {
-        # CSRF and per-session tokens (OpenEMR, Laravel, Rails, Django, ...)
-        "token_main",
-        "csrf_token_form",
+        # CSRF and per-session tokens (Laravel, Rails, Django, ...); any other
+        # name with such a part is caught by is_credential_name.
         "csrf_token",
         "csrf",
         "_csrf",
@@ -128,9 +129,7 @@
 #: JSON object keys whose whole string value is a credential, besides headers.
 _SECRET_KEYS: Final = frozenset(
     {
-        "token_main",
         "csrf_token",
-        "csrf_token_form",
         "access_token",
         "id_token",
         "refresh_token",
@@ -214,6 +213,47 @@ def params_for(self, host: Optional[str]) -> frozenset[str]:
 
 DEFAULT_POLICY: Final = RedactionPolicy()
 
+#: Name parts that make a parameter or JSON key a credential in any app.
+_CREDENTIAL_PARTS: Final = frozenset(
+    {
+        "token",
+        "csrf",
+        "xsrf",
+        "csrftoken",
+        "xsrftoken",
+        "sessid",
+        "sessionid",
+    }
+)
+_NAME_PARTS: Final = re.compile(r"[a-z0-9]+")
+_CAMEL_BOUNDARY: Final = re.compile(r"(?<=[a-z0-9])(?=[A-Z])")
+
+
+def is_credential_name(name: str, *, header: bool = False) -> bool:
+    """Whether a parameter, key or header name says its value is a credential.
+
+    The name is split into parts on anything but letters and digits; a part
+    that is a token, CSRF/XSRF token or session id makes it one
+    (``csrf_token_form``, ``X-Frappe-CSRF-Token``, ``app_sessid``). For a
+    header (``header=True``) an ``authorization`` part does too
+    (``Espo-Authorization``); elsewhere that word names an approval, not a
+    credential. A parameter or header named so is redacted in every app and
+    cannot be kept.
+    """
+    # camelCase splits too: sessionToken, accessToken, RequestVerificationToken.
+    spaced = _CAMEL_BOUNDARY.sub(" ", name)
+    parts = _NAME_PARTS.findall(spaced.lower())
+    # A session id header spelled as two parts (Session-Id, x-session-id).
+    # Only headers: a report's own fields such as dispatch_session_id name a
+    # run's internal session, not a credential.
+    joined = {a + b for a, b in zip(parts, parts[1:])} if header else set()
+    return any(
+        part in _CREDENTIAL_PARTS
+        or part.startswith(("csrf", "xsrf"))
+        or (header and part == "authorization")
+        for part in [*parts, *joined]
+    )
+
 
 @dataclass(frozen=True)
 class Redaction:
@@ -349,7 +389,9 @@ def _redact_params(text: str, policy: RedactionPolicy, reasons: list[str]) -> st
         if match.start() < cursor:
             continue  # inside a value already redacted
         name = match.group("name").lower()
-        if name not in policy.params_for(_host_at(text, match.start())):
+        if name not in policy.params_for(
+            _host_at(text, match.start())
+        ) and not is_credential_name(match.group("name")):
             continue
         value = _PARAM_VALUE.match(text, match.end())
         if value is None:
@@ -370,7 +412,9 @@ def redact_text(
 
     def header(match: re.Match[str]) -> str:
         name = match.group("name")
-        if name.lower() not in policy.headers:
+        if name.lower() not in policy.headers and not is_credential_name(
+            name, header=True
+        ):
             return match.group(0)
         new = _header_value(name, match.group("value"))
         if new == match.group("value") or new.strip() == match.group("value").strip():
@@ -415,7 +459,10 @@ def _redact_node(
     pair_header = (
         pair_name.lower()
         if isinstance(pair_name, str)
-        and pair_name.lower() in policy.headers
+        and (
+            pair_name.lower() in policy.headers
+            or is_credential_name(pair_name, header=True)
+        )
         and isinstance(node.get("value"), str)
         else None
     )
@@ -425,7 +472,8 @@ def _redact_node(
         header_key = lowered if lowered in policy.headers else None
         if key == "value" and pair_header:
             header_key = pair_header
-        if isinstance(item, str) and (header_key or lowered in _SECRET_KEYS):
+        secret_key = lowered in _SECRET_KEYS or is_credential_name(str(key))
+        if isinstance(item, str) and (header_key or secret_key):
             new = _header_value(header_key or lowered, item) if item else item
             if item and new != item:
                 reason = f"header:{header_key}" if header_key else f"key:{lowered}"

packages/flow/tests/test_report_redaction.py

@@ -378,3 +378,69 @@ def test_hosted_validation_matches_a_redacted_entry_url():
     url = _openemr_url(_token())
     assert _openemr_url(REDACTED) in _entry_url_forms(url)
     assert url in _entry_url_forms(url)
+
+
+@pytest.mark.parametrize(
+    ("app", "text", "name"),
+    [
+        # The motivating case: a per-session CSRF token in a query parameter.
+        ("OpenEMR", "/interface/main/tabs/main.php?token_main={v}&x=1", "token_main"),
+        ("OpenEMR", "csrf_token_form={v}&pid=3", "csrf_token_form"),
+        # The same rule, with no per-app list, on other apps' credential names.
+        ("Frappe", "X-Frappe-CSRF-Token: {v}", "x-frappe-csrf-token"),
+        ("EspoCRM", "Espo-Authorization: {v}", "espo-authorization"),
+        ("any", "/list?next=2&app_sessid={v}", "app_sessid"),
+    ],
+)
+def test_credential_names_are_found_by_their_parts_in_any_app(app, text, name):
+    value = _token()
+    out, reasons = redact_text(text.format(v=value))
+    assert value not in out, app
+    assert any(reason.endswith(name) for reason in reasons), reasons
+
+
+def test_credential_name_parts_leave_ordinary_names_alone():
+    assert not report_redaction.is_credential_name("tokenizer")
+    assert not report_redaction.is_credential_name("input_tokens")
+    assert not report_redaction.is_credential_name("patient_id")
+    text = "/list?tokenizer=bpe&page=2&input_tokens=12"
+    assert redact_text(text) == (text, [])
+
+
+def test_a_credential_pair_in_a_har_entry_is_redacted():
+    value = _token()
+    report = {"request": {"headers": [{"name": "X-Frappe-CSRF-Token", "value": value}]}}
+    redacted, found = redact_report(report)
+    assert value not in json.dumps(redacted)
+    assert found and found[0].pointer == "/request/headers/0/value"
+
+
+def test_an_approval_named_authorization_is_not_a_credential():
+    """Outside a header, 'authorization' names a governed approval."""
+    report = {"governed_authorization_id": "approval-123"}
+    redacted, found = redact_report(report)
+    assert redacted["governed_authorization_id"] == "approval-123"
+    assert not found
+
+
+@pytest.mark.parametrize(
+    "text",
+    [
+        "/app?sessionToken={v}&x=1",
+        "/app?accessToken={v}",
+        "/app?__RequestVerificationToken={v}",
+        "Session-Id: {v}",
+        "x-session-id: {v}",
+    ],
+)
+def test_camel_case_and_two_part_session_names_are_credentials(text):
+    value = _token()
+    out, reasons = redact_text(text.format(v=value))
+    assert value not in out and reasons
+
+
+def test_report_fields_naming_a_session_stay_readable():
+    report = {"dispatch_session_id": "run-42", "authenticated_session_id_sha256": "ab"}
+    redacted, found = redact_report(report)
+    assert redacted == report
+    assert not found