fix: mask_proxy and mask_url no longer leak credentials in non-4-part specs or url fragments

Signed-off-by: disqualifier <dev@disqualifier.me>
2026-07-03 19:08:39 -04:00
parent 33ade498da
commit 718c8a79b0
3 changed files with 98 additions and 26 deletions
@@ -12,15 +12,15 @@ Small sync helpers shared across projects. Base is stdlib only — **no dependen
 ## Install

 ```
-commons @ git+ssh://git@git.rethinkstudios.io/rethink-public/commons.git@v0.3.4
+commons @ git+ssh://git@git.rethinkstudios.io/rethink-public/commons.git@v0.3.5
 # async address/geo lookups (fetch_ip / ip_location / fetch_location) need the extra:
-commons[addr] @ git+ssh://git@git.rethinkstudios.io/rethink-public/commons.git@v0.3.4
+commons[addr] @ git+ssh://git@git.rethinkstudios.io/rethink-public/commons.git@v0.3.5
 ```

 The base install pulls **nothing** (stdlib). Only `commons[addr]` adds `aiohttp`, and
 only for the geo lookups — the pure `commons.addr.ip` utilities ship in base.

-Drop the `@v0.3.4` suffix from the line above to install the latest unpinned.
+Drop the `@v0.3.5` suffix from the line above to install the latest unpinned.

 ## timing

@@ -129,16 +129,28 @@ provider("4111²111111111234")    # "VISA"  (unicode digit lookalikes ignored, n
 mask_url("https://u:pw@api.x/v2?apiKey=SECRET&ip=8.8.8.8")
 #   -> "https://api.x/v2?apiKey=***&ip=8.8.8.8"   (userinfo dropped, secret query masked)
 mask_url("redis://:pw@127.0.0.1:6379/0")     # -> "redis://127.0.0.1:6379/0"
+mask_url("https://x/cb#access_token=SECRET&token_type=bearer")
+#   -> "https://x/cb#access_token=***&token_type=bearer"   (oauth implicit-grant fragment masked)
 mask_proxy("1.2.3.4:8080:user:supersecret")  # -> "1.2.3.4:8080:user:****"
 mask_proxy("1.2.3.4:8080")                    # -> "1.2.3.4:8080"  (no auth, untouched)
+mask_proxy("user:supersecret@1.2.3.4:8080")   # -> "1.2.3.4:8080"  (userinfo shape, also masked)
 ```

 `mask_url` strips `user:pass@` userinfo and replaces the values of sensitive query
 params (`apiKey`, `token`, `password`, `secret`, …; override via `keys=`) with `***`.
 Non-sensitive query values are re-percent-encoded on the way out, so a value with a
 reserved character (`&`, `=`, a space, …) round-trips correctly instead of corrupting
-the rebuilt URL. `mask_proxy` bullets the password of a `host:port:user:password`
-spec. Non-URL / non-conforming input is returned unchanged.
+the rebuilt URL. The URL fragment is masked the same way as the query when it is
+genuinely `key=value&key=value` shaped (e.g. an OAuth implicit-grant callback) — a
+fragment that only incidentally contains `=` (an SPA hash route like `#/page?x=1`) is
+left untouched rather than risking a lossy rewrite of a non-secret value, and a plain
+anchor (`#section`) always passes through unchanged.
+
+`mask_proxy` bullets the password of a `host:port:user:password` spec, and a plain
+`host:port` (no auth, including a bracketed IPv6 host) passes through unchanged. Any
+other credential-bearing shape — `user:pass@host:port` userinfo, a `scheme://`-prefixed
+URL, or a colon spec with more than 4 parts — is masked rather than ever returned
+verbatim; it never logs a password in the clear.

 ## retry

@@ -4,7 +4,7 @@ build-backend = "hatchling.build"

 [project]
 name = "commons"
-version = "0.3.4"
+version = "0.3.5"
 description = "small stdlib-based sync helpers: time/timezone deltas, dotted-path dict access, display masking, ip/address tooling, and retry/backoff"
 requires-python = ">=3.10"
 dependencies = []
@@ -65,14 +65,42 @@ def provider(card_number: str) -> str:
    return "UNKW"


+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,
+    )
+
+
+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.
+    """
+    pairs = parse_qsl(qs, keep_blank_values=True)
+    return urlencode(pairs, safe="*", quote_via=quote_plus) == qs
+
+
 def mask_url(url: str, *, keys: "frozenset[str] | None" = None) -> str:
-    """redact credentials in a url for logging: drop userinfo, ``***`` sensitive query values
+    """redact credentials in a url for logging: drop userinfo, ``***`` sensitive query
+    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.
+    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.
    """
    sensitive = SENSITIVE_QUERY_KEYS if keys is None else frozenset(k.lower() for k in keys)
    try:
@@ -84,26 +112,58 @@ def mask_url(url: str, *, keys: "frozenset[str] | None" = None) -> str:
    netloc = parts.netloc
    if "@" in netloc:
        netloc = netloc.rsplit("@", 1)[1]
-    query = parts.query
-    if query:
-        pairs = parse_qsl(query, keep_blank_values=True)
-        query = urlencode(
-            [(k, "***" if k.lower() in sensitive else v) for k, v in pairs],
-            safe="*",
-            quote_via=quote_plus,
-        )
-    return urlunsplit((parts.scheme, netloc, parts.path, query, parts.fragment))
+    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)
+    return urlunsplit((parts.scheme, netloc, parts.path, query, fragment))
+
+
+def _split_host(rest: str) -> "tuple[str, list[str]]":
+    """split a ``host:port[:...]`` tail into its host token and the remaining colon fields
+
+    a leading ``[bracketed-ipv6-literal]`` is kept intact as the host token, since its
+    own internal colons are not field separators; anything else splits on the first colon.
+    """
+    if rest.startswith("["):
+        end = rest.find("]")
+        if end != -1:
+            return rest[: end + 1], rest[end + 1:].lstrip(":").split(":")
+    host, _, tail = rest.partition(":")
+    return host, tail.split(":")
+
+
+def _bullet_trailing_fields(rest: str) -> str:
+    """bullet the trailing colon-separated credential field(s) of a ``host:port[:...]`` tail"""
+    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


 def mask_proxy(spec: str) -> str:
-    """redact the password of a ``host:port:user:password`` proxy string to ``:****``
+    """redact credentials in a proxy spec for logging

-    a plain ``host:port`` (no auth) and any other non-conforming input pass through unchanged.
+    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.
    """
-    parts = spec.split(":")
-    if len(parts) == 2:
-        return spec
-    if len(parts) == 4:
-        host, port, user, _ = parts
-        return f"{host}:{port}:{user}:****"
-    return spec
+    rest = spec
+    if "://" in rest:
+        rest = rest.split("://", 1)[1]
+    if "@" in rest:
+        rest = rest.rsplit("@", 1)[1]
+    return _bullet_trailing_fields(rest)