From adec152d6129ca29fbbc1f2b7a9541a3ef5db03d Mon Sep 17 00:00:00 2001 From: abrichr Date: Tue, 3 Nov 2026 12:45:23 -0500 Subject: [PATCH 1/4] Redaction: find credential names by their parts, not one app's list report_redaction listed OpenEMR's token_main and csrf_token_form by name. A parameter, header, JSON key or HAR name/value pair whose name has a token, CSRF, XSRF, session-id or authorization part is now a credential in every app (token_main, csrf_token_form, X-Frappe-CSRF-Token, Espo-Authorization). Such names are never keepable. network_capture's secret pattern listed OpenEMR's clearpass; its pass alternative already matches it. --- .../flow/openadapt_flow/network_capture.py | 2 +- .../flow/openadapt_flow/report_redaction.py | 62 ++++++++++++++----- packages/flow/tests/test_report_redaction.py | 35 +++++++++++ 3 files changed, 83 insertions(+), 16 deletions(-) diff --git a/packages/flow/openadapt_flow/network_capture.py b/packages/flow/openadapt_flow/network_capture.py index 2a707b9c92..14b1e9f12f 100644 --- a/packages/flow/openadapt_flow/network_capture.py +++ b/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 = "" diff --git a/packages/flow/openadapt_flow/report_redaction.py b/packages/flow/openadapt_flow/report_redaction.py index 4b1be3b91d..82e70dada0 100644 --- a/packages/flow/openadapt_flow/report_redaction.py +++ b/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,36 @@ 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", + "authorization", + } +) +_NAME_PARTS: Final = re.compile(r"[a-z0-9]+") + + +def is_credential_name(name: str) -> bool: + """Whether a parameter or key 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``, ``Espo-Authorization``, + ``app_sessid``). A parameter or header named so is redacted in every app + and cannot be kept. + """ + parts = _NAME_PARTS.findall(name.lower()) + return any( + part in _CREDENTIAL_PARTS or part.startswith(("csrf", "xsrf")) for part in parts + ) + @dataclass(frozen=True) class Redaction: @@ -349,7 +378,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(name): continue value = _PARAM_VALUE.match(text, match.end()) if value is None: @@ -370,7 +401,7 @@ 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): 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 +446,7 @@ 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)) and isinstance(node.get("value"), str) else None ) @@ -425,7 +456,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(lowered) + 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}" diff --git a/packages/flow/tests/test_report_redaction.py b/packages/flow/tests/test_report_redaction.py index ef483d4b50..8f41c30787 100644 --- a/packages/flow/tests/test_report_redaction.py +++ b/packages/flow/tests/test_report_redaction.py @@ -378,3 +378,38 @@ 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" From 92618724d52f068cf98d48923989aa9a2ca2620c Mon Sep 17 00:00:00 2001 From: abrichr Date: Tue, 3 Nov 2026 13:48:14 -0500 Subject: [PATCH 2/4] Redaction: 'authorization' marks a credential only in a header name A run report's governed_authorization_id is an approval id, not a secret; CI caught the report losing it. --- .../flow/openadapt_flow/report_redaction.py | 27 ++++++++++++------- packages/flow/tests/test_report_redaction.py | 8 ++++++ 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/packages/flow/openadapt_flow/report_redaction.py b/packages/flow/openadapt_flow/report_redaction.py index 82e70dada0..80478f0e48 100644 --- a/packages/flow/openadapt_flow/report_redaction.py +++ b/packages/flow/openadapt_flow/report_redaction.py @@ -223,24 +223,28 @@ def params_for(self, host: Optional[str]) -> frozenset[str]: "xsrftoken", "sessid", "sessionid", - "authorization", } ) _NAME_PARTS: Final = re.compile(r"[a-z0-9]+") -def is_credential_name(name: str) -> bool: - """Whether a parameter or key name says its value is a credential. +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``, ``Espo-Authorization``, - ``app_sessid``). A parameter or header named so is redacted in every app - and cannot be kept. + (``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. """ parts = _NAME_PARTS.findall(name.lower()) return any( - part in _CREDENTIAL_PARTS or part.startswith(("csrf", "xsrf")) for part in parts + part in _CREDENTIAL_PARTS + or part.startswith(("csrf", "xsrf")) + or (header and part == "authorization") + for part in parts ) @@ -401,7 +405,9 @@ def redact_text( def header(match: re.Match[str]) -> str: name = match.group("name") - if name.lower() not in policy.headers and not is_credential_name(name): + 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(): @@ -446,7 +452,10 @@ def _redact_node( pair_header = ( pair_name.lower() if isinstance(pair_name, str) - and (pair_name.lower() in policy.headers or is_credential_name(pair_name)) + and ( + pair_name.lower() in policy.headers + or is_credential_name(pair_name, header=True) + ) and isinstance(node.get("value"), str) else None ) diff --git a/packages/flow/tests/test_report_redaction.py b/packages/flow/tests/test_report_redaction.py index 8f41c30787..6f436537ef 100644 --- a/packages/flow/tests/test_report_redaction.py +++ b/packages/flow/tests/test_report_redaction.py @@ -413,3 +413,11 @@ def test_a_credential_pair_in_a_har_entry_is_redacted(): 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 From dd8a129b081a2b07ca81f0a09f32af1b92cba9b9 Mon Sep 17 00:00:00 2001 From: abrichr Date: Tue, 3 Nov 2026 15:50:05 -0500 Subject: [PATCH 3/4] Redaction: camelCase credential names and two-part session headers Review of the credential-name rule: sessionToken, accessToken and __RequestVerificationToken split on case now, and a Session-Id or x-session-id header counts as a session id. The two-part rule is for header names only, so a report's own dispatch_session_id stays readable. --- .../flow/openadapt_flow/report_redaction.py | 11 +++++++-- packages/flow/tests/test_report_redaction.py | 23 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/packages/flow/openadapt_flow/report_redaction.py b/packages/flow/openadapt_flow/report_redaction.py index 80478f0e48..415986a78a 100644 --- a/packages/flow/openadapt_flow/report_redaction.py +++ b/packages/flow/openadapt_flow/report_redaction.py @@ -226,6 +226,7 @@ def params_for(self, host: Optional[str]) -> frozenset[str]: } ) _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: @@ -239,12 +240,18 @@ def is_credential_name(name: str, *, header: bool = False) -> bool: credential. A parameter or header named so is redacted in every app and cannot be kept. """ - parts = _NAME_PARTS.findall(name.lower()) + # 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 + for part in [*parts, *joined] ) diff --git a/packages/flow/tests/test_report_redaction.py b/packages/flow/tests/test_report_redaction.py index 6f436537ef..afb70e7b57 100644 --- a/packages/flow/tests/test_report_redaction.py +++ b/packages/flow/tests/test_report_redaction.py @@ -421,3 +421,26 @@ def test_an_approval_named_authorization_is_not_a_credential(): 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 From d332040ce21fa33dd5094f379f1ea1cfaff22e2b Mon Sep 17 00:00:00 2001 From: abrichr Date: Tue, 3 Nov 2026 17:48:12 -0500 Subject: [PATCH 4/4] Redaction: check a query or JSON name's camelCase parts before lowercasing --- packages/flow/openadapt_flow/report_redaction.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/flow/openadapt_flow/report_redaction.py b/packages/flow/openadapt_flow/report_redaction.py index 415986a78a..1770cb75b7 100644 --- a/packages/flow/openadapt_flow/report_redaction.py +++ b/packages/flow/openadapt_flow/report_redaction.py @@ -391,7 +391,7 @@ def _redact_params(text: str, policy: RedactionPolicy, reasons: list[str]) -> st name = match.group("name").lower() if name not in policy.params_for( _host_at(text, match.start()) - ) and not is_credential_name(name): + ) and not is_credential_name(match.group("name")): continue value = _PARAM_VALUE.match(text, match.end()) if value is None: @@ -472,7 +472,7 @@ def _redact_node( header_key = lowered if lowered in policy.headers else None if key == "value" and pair_header: header_key = pair_header - secret_key = lowered in _SECRET_KEYS or is_credential_name(lowered) + 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: