fix: redact fragment and strip userinfo, flag hidden query in path

The query string was not the only place a recipient identifier can hide.
A fragment (#e=victim@...) is published as-is since we report the URL's
literal text, not what a browser would send. Userinfo (user:pass@host)
leaks a credential as well as an identifier, so it is stripped outright
rather than redacted in place. A path segment can also smuggle an
encoded query (%3Fe=victim@...); suspect_path_segments now flags a
segment that decodes to something containing '=' or '@', still leaving
the decision to redact or not to human review.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KphFXTc2QajxXsHWyvGJ4R

2 files changed, 111 insertions, 11 deletions

diff --git a/abusectl/redact.py b/abusectl/redact.py
index 3ca744f..ffc63a8 100644
--- a/abusectl/redact.py
+++ b/abusectl/redact.py
@@ -34,6 +34,19 @@ a value that decodes to an http(s) URL is recovered separately by
 output: the redirector's own query string still redacts every value,
 including the one the target was recovered from, because recovery reports an
 extra indicator, it does not license leaving the recipient's token in place.
+
+``url()`` redacts three places a recipient identifier can hide, not only the
+query string. The FRAGMENT is redacted the same way as the query, because we
+publish the URL's text as it appeared in the email, and a browser's habit of
+not sending the fragment to the server is irrelevant to that: a phishing kit
+can and does put a token after ``#``. Fragment key=value pairs keep their
+names and lose their values, same as the query; an opaque fragment carries no
+kit fingerprint worth keeping, so it is replaced whole. USERINFO
+(``user:pass@host``) is stripped from the netloc entirely rather than
+redacted in place, because it is often a credential as well as an
+identifier and no part of it is safe to publish, not even a placeholder
+shape; ``has_userinfo()`` lets a caller record that a URL carried one, since
+that fact is itself an indicator.
 """
 
 import re
@@ -56,34 +69,79 @@ _MIN_SUSPECT_LENGTH = 16
 _SUSPECT_CHARS = set(string.ascii_letters + string.digits + "+/=_-")
 
 
+def _redact_kv_string(value: str) -> str:
+    """Redact a query- or fragment-style key=value string, keeping names."""
+    pairs = parse_qsl(value, keep_blank_values=True)
+    return urlencode([(name, REDACTED) for name, _ in pairs])
+
+
+def _redact_fragment(fragment: str) -> str:
+    """Redact a fragment: key=value pairs keep names, an opaque one is wiped.
+
+    An opaque fragment fingerprints nothing the way a parameter name does,
+    so there is no reason to keep any part of it once it might carry an
+    identifier.
+    """
+    if "=" in fragment:
+        return _redact_kv_string(fragment)
+    return REDACTED
+
+
+def _netloc_without_userinfo(parts) -> str:
+    """Return scheme://host[:port] worth of netloc, dropping any userinfo."""
+    netloc = parts.hostname or ""
+    if parts.port is not None:
+        netloc = f"{netloc}:{parts.port}"
+    return netloc
+
+
+def has_userinfo(raw: str) -> bool:
+    """Return True if raw's netloc carries a user:pass@ component.
+
+    Userinfo is stripped rather than redacted in ``url()``, since it is
+    often a credential and not safe to publish even as a placeholder. This
+    lets a caller record separately that the URL carried one, which is
+    itself an indicator.
+    """
+    return "@" in urlsplit(raw).netloc
+
+
 def url(raw: str) -> str:
-    """Return raw with every query parameter value replaced by REDACTED.
+    """Return raw with every recipient-identifying part replaced.
 
-    Parameter names, scheme, host and path are left untouched. A URL with no
-    query string is returned unchanged.
+    Query and fragment key=value pairs keep their names and lose their
+    values; an opaque fragment is replaced whole. Userinfo is stripped from
+    the netloc entirely. Scheme, host, port and path are left untouched. A
+    URL with none of query, fragment or userinfo is returned unchanged.
     """
     parts = urlsplit(raw)
-    if not parts.query:
+    if not parts.query and not parts.fragment and not has_userinfo(raw):
         return raw
-    pairs = parse_qsl(parts.query, keep_blank_values=True)
-    redacted_query = urlencode([(name, REDACTED) for name, _ in pairs])
-    return urlunsplit(parts._replace(query=redacted_query))
+    redacted_query = _redact_kv_string(parts.query) if parts.query else parts.query
+    redacted_fragment = _redact_fragment(parts.fragment) if parts.fragment else parts.fragment
+    netloc = _netloc_without_userinfo(parts) if has_userinfo(raw) else parts.netloc
+    return urlunsplit(
+        parts._replace(netloc=netloc, query=redacted_query, fragment=redacted_fragment)
+    )
 
 
 def suspect_path_segments(raw: str) -> list[str]:
     """Return path segments that look like an encoded identifier.
 
     A segment made only of base64/hex-shaped characters and at least
-    ``_MIN_SUSPECT_LENGTH`` long is flagged for human review, never redacted:
-    unlike a query value, a path segment may be the very thing being
-    reported.
+    ``_MIN_SUSPECT_LENGTH`` long is flagged, and so is a segment that
+    URL-decodes to something containing ``=`` or ``@``, which is how a whole
+    query string smuggled into one path segment (``%3F`` and all) is caught.
+    Flagging is never redaction: unlike a query value, a path segment may be
+    the very thing being reported, so the decision is left to human review.
     """
     parts = urlsplit(raw)
     segments = [seg for seg in parts.path.split("/") if seg]
     return [
         seg
         for seg in segments
-        if len(seg) >= _MIN_SUSPECT_LENGTH and set(seg) <= _SUSPECT_CHARS
+        if (len(seg) >= _MIN_SUSPECT_LENGTH and set(seg) <= _SUSPECT_CHARS)
+        or ("=" in unquote(seg) or "@" in unquote(seg))
     ]
 
 
diff --git a/tests/test_redact.py b/tests/test_redact.py
index 91ab291..23c2a45 100644
--- a/tests/test_redact.py
+++ b/tests/test_redact.py
@@ -102,5 +102,47 @@ class TestUrlValuedParameters(unittest.TestCase):
         )
 
 
+class TestLeakResistance(unittest.TestCase):
+    def test_a_fragment_key_value_pair_is_redacted(self):
+        out = redact.url("http://a.invalid/p?x=1#e=victim@example.org")
+        self.assertNotIn("victim@example.org", out)
+
+    def test_userinfo_is_stripped_not_redacted_in_place(self):
+        out = redact.url("http://victim%40example.org:pw@a.invalid/p")
+        self.assertNotIn("victim%40example.org", out)
+        self.assertNotIn("pw", out)
+
+    def test_a_query_hidden_inside_a_path_segment_is_flagged(self):
+        found = redact.suspect_path_segments(
+            "http://a.invalid/p%3Fe=victim@example.org"
+        )
+        self.assertNotEqual(found, [])
+
+    def test_no_recipient_marker_survives_any_placement(self):
+        # The same address, placed everywhere a URL can hide one.
+        marker = "victim@example.org"
+        encoded = "victim%40example.org"
+        for raw in (
+            f"http://a.invalid/p?e={marker}",
+            f"http://a.invalid/p?x=1#e={marker}",
+            f"http://{encoded}:pw@a.invalid/p",
+            f"http://a.invalid/p?a=1&b=2#{marker}",
+        ):
+            with self.subTest(raw=raw):
+                out = redact.url(raw)
+                self.assertNotIn(marker, out)
+                self.assertNotIn(encoded, out)
+
+
+class TestHasUserinfo(unittest.TestCase):
+    def test_userinfo_present_is_reported(self):
+        self.assertTrue(
+            redact.has_userinfo("http://victim%40example.org:pw@a.invalid/p")
+        )
+
+    def test_no_userinfo_is_reported_absent(self):
+        self.assertFalse(redact.has_userinfo("http://a.invalid/p"))
+
+
 if __name__ == "__main__":
     unittest.main()