fix: mask_proxy/mask_url never return a credential in the clear (structure-preserving)

three credential-leak shapes where a redactor returned the secret verbatim (all 718c8a7):
- mask_proxy host:port:cred (3 parts) fell through to a verbatim return; and a >4-part spec
  bulleted only the LAST field, leaking a colon-bearing password's earlier halves. now
  everything after host:port is credential material and collapses to a single :**** for any
  shape (3/4/n parts), so no credential field survives. plain host:port/bare host unchanged.
- mask_url's fragment used a lossless parse_qsl->urlencode round-trip gate that a =-padded
  base64 token failed, passing the whole fragment (token included) through verbatim. _mask_qs
  is now structure-preserving: it splits on & and the first =, replaces only a sensitive key's
  value with *** (covering a =-padded/embedded-= token whole), and leaves every other segment
  byte-identical - so a non-secret value is never corrupted and the round-trip gate (the root of
  the leak) is gone. fragment and query mask identically now.
invariant: never emit a credential in the clear; over-mask when the shape is ambiguous.

Signed-off-by: disqualifier <dev@disqualifier.me>

src/commons/masking.py

@@ -3,7 +3,7 @@
 DISPLAY only, not a security control: the underlying value is unchanged and still
 needs proper handling (encryption at rest, etc.).
 """
-from urllib.parse import parse_qsl, quote_plus, urlencode, urlsplit, urlunsplit
+from urllib.parse import urlsplit, urlunsplit
 
 SENSITIVE_QUERY_KEYS = frozenset(
     {"apikey", "api_key", "key", "token", "access_token", "refresh_token",
@@ -66,24 +66,23 @@ def provider(card_number: str) -> str:
 
 
 def _mask_qs(qs: str, sensitive: "frozenset[str]") -> str:
-    """mask sensitive key=value pairs in a query-string-shaped blob (query or fragment)"""
-    pairs = parse_qsl(qs, keep_blank_values=True)
-    return urlencode(
-        [(k, "***" if k.lower() in sensitive else v) for k, v in pairs],
-        safe="*",
-        quote_via=quote_plus,
-    )
+    """mask sensitive ``key=value`` values in a query/fragment blob, structure-preserving
 
-
-def _is_query_shaped(qs: str) -> bool:
-    """true when a blob is genuinely ``key=value&key=value`` shaped, not merely containing ``=``
-
-    a lossless parse_qsl -> urlencode round-trip proves the blob was already in canonical
-    query form; anything else (a spa hash route like ``#/page?x=1``, a value that itself
-    contains ``=`` like ``#a=b=c``) fails the round-trip and must not be rewritten.
+    splits on ``&`` and each pair on its FIRST ``=``; a sensitive key's value (everything
+    after that first ``=``, so a ``=``-padded base64 token is covered whole) becomes ``***``.
+    non-sensitive segments are left BYTE-IDENTICAL - no re-encoding, so a value with a
+    reserved char, an embedded ``=``, or a non-kv segment (a spa hash route like ``/page``)
+    is never corrupted. a sensitive token in the fragment is therefore always masked, never
+    passed through verbatim.
     """
-    pairs = parse_qsl(qs, keep_blank_values=True)
-    return urlencode(pairs, safe="*", quote_via=quote_plus) == qs
+    out = []
+    for segment in qs.split("&"):
+        key, sep, _ = segment.partition("=")
+        if sep and key.lower() in sensitive:
+            out.append(f"{key}=***")
+        else:
+            out.append(segment)
+    return "&".join(out)
 
 
 def mask_url(url: str, *, keys: "frozenset[str] | None" = None) -> str:
@@ -91,16 +90,12 @@ def mask_url(url: str, *, keys: "frozenset[str] | None" = None) -> str:
     and fragment values
 
     param-name matching is case-insensitive; ``keys`` overrides the default sensitive-name
-    set. non-url or unparseable input is returned unchanged. non-sensitive query values are
-    re-percent-encoded on the way out (``quote_plus``), so a value containing a reserved
-    character (e.g. ``&``, ``=``, a space) round-trips correctly instead of corrupting the
-    rebuilt url. the fragment is masked the same way as the query WHEN it is genuinely
-    ``key=value&key=value`` shaped - this covers oauth implicit-grant callbacks that put
-    ``access_token`` in the fragment instead of the query. a fragment that merely contains
-    an ``=`` without being canonical query shape (a spa hash route like ``#/page?x=1``, a
-    value with an embedded ``=`` like ``#a=b=c``) is left untouched rather than risking a
-    lossy rewrite of a non-secret value; a plain anchor fragment (e.g. ``#section``, no
-    ``=`` at all) also passes through unchanged.
+    set. non-url or unparseable input is returned unchanged. masking is structure-preserving
+    (see ``_mask_qs``): only a sensitive key's value is replaced, everything else is left
+    byte-identical, so a non-secret value is never corrupted and a non-kv fragment (a spa
+    hash route like ``#/page``, a plain anchor ``#section``) passes through unchanged. the
+    fragment is masked the same way as the query, covering oauth implicit-grant callbacks
+    that put ``access_token`` (including a ``=``-padded base64 token) in the fragment.
     """
     sensitive = SENSITIVE_QUERY_KEYS if keys is None else frozenset(k.lower() for k in keys)
     try:
@@ -113,9 +108,7 @@ def mask_url(url: str, *, keys: "frozenset[str] | None" = None) -> str:
     if "@" in netloc:
         netloc = netloc.rsplit("@", 1)[1]
     query = _mask_qs(parts.query, sensitive) if parts.query else parts.query
-    fragment = parts.fragment
-    if fragment and "=" in fragment and _is_query_shaped(fragment):
-        fragment = _mask_qs(fragment, sensitive)
+    fragment = _mask_qs(parts.fragment, sensitive) if parts.fragment else parts.fragment
     return urlunsplit((parts.scheme, netloc, parts.path, query, fragment))
 
 
@@ -134,32 +127,32 @@ def _split_host(rest: str) -> "tuple[str, list[str]]":
 
 
 def _bullet_trailing_fields(rest: str) -> str:
-    """bullet the trailing colon-separated credential field(s) of a ``host:port[:...]`` tail"""
+    """redact any credential fields of a ``host:port[:user:password...]`` tail
+
+    keeps ``host`` and the ``port`` (the first colon field after host) in the clear; every
+    field AFTER the port is credential material and is replaced with a single ``****``,
+    regardless of how many there are (so a colon-bearing password can't leak a fragment).
+    a bare ``host`` or ``host:port`` (no trailing fields) passes through unchanged - fails
+    safe: anything with a credential field is bulleted, never returned verbatim.
+    """
     host, fields = _split_host(rest)
     if fields == [""]:
         return host
     if len(fields) == 1:
         return f"{host}:{fields[0]}"
-    if len(fields) == 3:
-        port, user, _ = fields
-        return f"{host}:{port}:{user}:****"
-    if len(fields) > 3:
-        return host + ":" + ":".join(fields[:-1]) + ":****"
-    return rest
+    port = fields[0]
+    return f"{host}:{port}:****"
 
 
 def mask_proxy(spec: str) -> str:
     """redact credentials in a proxy spec for logging
 
-    the canonical ``host:port:user:password`` (exactly 4 colon-separated parts) bullets
-    the password to ``:****``. a plain ``host:port`` (no auth) passes through unchanged.
-    any other shape that carries credentials - ``user:pass@host:port`` userinfo, a
-    scheme-prefixed url with trailing colon fields, or a colon-separated spec with more
-    than 4 parts - is masked rather than ever returned verbatim: a ``scheme://`` prefix
-    is dropped, then any ``user:pass@`` userinfo is dropped, then the remaining
-    ``host:port[:user:password]`` tail has its trailing field(s) bulleted when there are
-    more than 2 colon-separated parts. fails safe: this masker never returns a still-
-    credential-bearing string verbatim.
+    drops a ``scheme://`` prefix and any ``user:pass@`` userinfo, then keeps only
+    ``host:port`` in the clear and bullets EVERY trailing colon field (user/password/etc.)
+    to a single ``:****`` - so ``host:port:user:password`` and any longer spec (a
+    colon-bearing password, extra fields) all collapse to ``host:port:****``. a plain
+    ``host:port`` or bare ``host`` (no auth) passes through unchanged. fails safe: whenever
+    a credential field is present it is bulleted, never returned verbatim.
     """
     rest = spec
     if "://" in rest: