Skip to content

scrub_text's truncation cannot see the guard rules keyed on caller-supplied values #654

Description

@pbean

Follow-up from PR #653 (#481), raised by a codex review on that PR. Out of scope there; filed rather than widening that diff.

What holds today

sanitize._truncate_line retracts a cut out of any _TRUNCATION_HAZARD_RES match it would split whose whole trips assert_no_leak while its surviving prefix does not. That closes the two shapes reachable through the guard's static rules:

  • a credential-shaped token (looks_like_secret's entropy arm needs a contiguous alnum run of _SECRET_RUN_MIN), and
  • a URL credential (_URL_CRED_RE's match ends at the @).

Both are pinned by tests/test_sanitize.py::test_scrub_text_max_chars_never_leaves_a_guard_invisible_fragment.

What does not

The guard also fires on values scrub_text has no access to, and a cut splitting one of those still costs the guard its verdict while leaving a fragment in the emitted bytes:

  1. Caller-supplied extra values. _truncate_line consults assert_no_leak without its extra argument, so a value the caller passes at egress (a project basename, a Pseudonymizer.legend value) does not influence retraction.
  2. A standalone username at the guard's floor. assert_no_leak's username rule fires at len(user) >= 5, but both hazard patterns need six characters, so a five-character username is matched by neither. Split it and the rule goes quiet.

Confirmed empirically on the PR branch. Note (2) is narrower than it first looks — a six-character username is caught today, because it matches _LEAK_TOKEN_RE and assert_no_leak fires on the whole match; the gap is usernames of exactly five characters.

Why it was not fixed in #653

Closing it means threading the caller's sensitive values into scrub_text, which is currently a pure text function whose two uncapped call sites depend on it being a byte-identical passthrough. That is a signature and layering change, not a bound fix, and #481 was about bounding volume.

Scope note

This is a defense-in-depth gap, not a demonstrated leak of a released value: the guard is a re-scan behind per-field routing, and PR #653's own "What this does NOT claim" section already states that a shape it does not know is outside it. Sizing this against that boundary is part of the work.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    P3Robustness, enhancement, tests, or docs worth schedulingarea:engineOrchestrator engine and run lifecyclebugSomething isn't workingneeds-designAwaiting a maintainer design decision before code

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions