fix: redact a valueless query token whole, not as a kept name

parse_qsl reads "?victim@example.org" as the pair
("victim@example.org", ""), and the code kept parameter NAMES because
they fingerprint the phishing kit. A name that is itself an address is
not a name, so keeping it published the recipient's identifier
verbatim (percent-decoded, no less: %40 fools no consumer).

_redact_kv_string now splits the query/fragment string by hand on "&"
and ";" and inspects each token's own name: one containing "@" is a
value that landed in name position and is redacted WHOLE ("?REDACTED"
rather than "?victim%40example.org=REDACTED"); an ordinary name still
keeps its shape ("?flag" stays "?flag=REDACTED", "?t=1&t=2" stays
"?t=REDACTED&t=REDACTED").

This also exposed a second leak reachable through the same fixture:
parse._suspect_segments() ran redact.suspect_path_segments()'s
decode-and-check predicate (meant for a querystring smuggled past
percent-encoding into a PATH segment) against a raw query VALUE, so a
plaintext "?e=you@example.org" reproduced the address in the
manifest's suspect_path_segments flag even though the URL itself was
correctly redacted. redact.suspect_path_segments() now exposes the
opaque-shape half of its check as _looks_opaque(), and parse.py uses
only that half against query values: a query value is always fully
redacted regardless, so the flag may hint at its shape but must never
reproduce it.

Turns tests.test_parse.TestIocAssembly.test_the_address_does_not_survive_any_url_shape
green, and closes the gap test_no_ioc_holds_a_recipient_address had
been passing over with URL redaction fully disabled.

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

3 files changed, 63 insertions, 5 deletions

diff --git a/abusectl/parse.py b/abusectl/parse.py
index 79cccc9..468e35e 100644
--- a/abusectl/parse.py
+++ b/abusectl/parse.py
@@ -322,6 +322,15 @@ def _suspect_segments(raw_url: str) -> list[str]:
     is exactly this: ``?id=<base64 of the address>``). Checking it there
     too is what lets review see the shape of the token redaction removes.
 
+    Only the opaque-SHAPE check applies to a query value, via
+    redact._looks_opaque(); the decode-and-check half of
+    suspect_path_segments() is for a querystring smuggled into a PATH
+    segment, and running it against a query value would reproduce a
+    plaintext address (``?e=you@example.org``) in the manifest instead of
+    merely flagging that a value is suspect. Query values are always fully
+    redacted in the reported URL; this flag is a shape hint for review, not
+    a second place for the value itself to leak through.
+
     Must be called on the ORIGINAL url, before redact.url() replaces every
     query value with REDACTED: past that point there is nothing left to
     recognise as suspect.
@@ -329,7 +338,8 @@ def _suspect_segments(raw_url: str) -> list[str]:
     found = list(redact.suspect_path_segments(raw_url))
     query = urlsplit(raw_url).query
     for _, value in parse_qsl(query, keep_blank_values=True):
-        found.extend(redact.suspect_path_segments(value))
+        if redact._looks_opaque(value):
+            found.append(value)
     return found
 
 
diff --git a/abusectl/redact.py b/abusectl/redact.py
index ffc63a8..5e7c4d3 100644
--- a/abusectl/redact.py
+++ b/abusectl/redact.py
@@ -70,9 +70,26 @@ _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])
+    """Redact a query- or fragment-style key=value string, keeping names.
+
+    parse_qsl cannot tell a bare flag ("?flag") from a nameless token that
+    merely looks like one ("?victim@example.org"): both parse as a pair
+    whose value is empty, so keeping "the name" kept the recipient address
+    verbatim in the second case. The two are told apart here instead: a
+    parameter name that fingerprints a phishing kit ("flag", "id", "src")
+    is never itself an email address, so a token containing "@" is a value
+    that landed in name position, not a name, and the WHOLE token is
+    redacted rather than only "the value" it does not actually have.
+    """
+    tokens = [t for t in re.split(r"[&;]", value) if t]
+    redacted = []
+    for token in tokens:
+        name = token.partition("=")[0]
+        if "@" in name:
+            redacted.append(REDACTED)
+        else:
+            redacted.append(f"{name}={REDACTED}")
+    return "&".join(redacted)
 
 
 def _redact_fragment(fragment: str) -> str:
@@ -125,6 +142,17 @@ def url(raw: str) -> str:
     )
 
 
+def _looks_opaque(segment: str) -> bool:
+    """Return True if segment is long and shaped like a base64/hex blob.
+
+    This is the one predicate safe to run against a query VALUE as well as
+    a path segment: it flags the SHAPE of a token without decoding or
+    reproducing anything readable, so a plaintext address sitting directly
+    in a value never comes back out through this check.
+    """
+    return len(segment) >= _MIN_SUSPECT_LENGTH and set(segment) <= _SUSPECT_CHARS
+
+
 def suspect_path_segments(raw: str) -> list[str]:
     """Return path segments that look like an encoded identifier.
 
@@ -134,13 +162,19 @@ def suspect_path_segments(raw: str) -> list[str]:
     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.
+
+    The decode-and-check half of this is deliberately PATH-only (see
+    ``_looks_opaque`` for the half that is safe on a query value too): it
+    exists to catch a querystring smuggled past percent-encoding, and
+    running it against an already-plaintext query value would reproduce a
+    recipient address in cleartext instead of merely flagging its shape.
     """
     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 _looks_opaque(seg)
         or ("=" in unquote(seg) or "@" in unquote(seg))
     ]
 
diff --git a/tests/test_redact.py b/tests/test_redact.py
index 23c2a45..e6d65c0 100644
--- a/tests/test_redact.py
+++ b/tests/test_redact.py
@@ -51,6 +51,20 @@ class TestRedactUrl(unittest.TestCase):
             "http://a.example.invalid/p?t=REDACTED&t=REDACTED",
         )
 
+    def test_a_valueless_token_is_redacted_whole(self):
+        # parse_qsl reads ?victim@example.org as a NAME, and names are kept.
+        # A token with no "=" is a value, not a fingerprint.
+        self.assertEqual(
+            redact.url("http://a.invalid/p?victim@example.org"),
+            "http://a.invalid/p?REDACTED",
+        )
+
+    def test_a_valueless_token_in_a_fragment_is_redacted_whole(self):
+        self.assertEqual(
+            redact.url("http://a.invalid/p#victim@example.org"),
+            "http://a.invalid/p#REDACTED",
+        )
+
 
 class TestSuspectPathSegments(unittest.TestCase):
     def test_a_base64_looking_segment_is_flagged(self):