From 718c8a79b0e41fe2364f8d886fedccfea2c0b72c Mon Sep 17 00:00:00 2001 From: disqualifier Date: Fri, 3 Jul 2026 19:08:39 -0400 Subject: [PATCH] fix: mask_proxy and mask_url no longer leak credentials in non-4-part specs or url fragments Signed-off-by: disqualifier --- README.md | 22 ++++++--- pyproject.toml | 2 +- src/commons/masking.py | 100 ++++++++++++++++++++++++++++++++--------- 3 files changed, 98 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index 18b4134..3decbae 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/pyproject.toml b/pyproject.toml index 38605d0..2f36325 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -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 = [] diff --git a/src/commons/masking.py b/src/commons/masking.py index d637377..9cd6c9b 100644 --- a/src/commons/masking.py +++ b/src/commons/masking.py @@ -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)