fix(oauth): redact URLs in log output — close CodeQL clear-text-logging flags (#5/#6/#7/#8)

CHANGELOG.md

@@ -8,6 +8,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Security
+- **Redact URL log output in the OAuth paths** — new `safe_url_for_log(url)` helper (in `src/mcp_awareness/helpers.py`) strips RFC 3986 `userinfo` (the `user:password@` netloc prefix), query string, and fragment before returning a URL for logging. OIDC discovery, JWKS URI discovery, userinfo-endpoint discovery, OAuth-proxy discovery failure, and OAuth-proxy discovered-endpoints logs (`oauth.py:_discover_oidc_config`, `oauth_proxy.py:discover_oidc_endpoints`) all now route through the helper. Defensive rather than currently-exploitable: the issuer URLs this server consumes today are plain `https://provider/...` strings with no credential-carrying parts, but the helper ensures future misconfiguration, provider change, or redirect can't leak an embedded credential into every log line. New `_format_ip_header_chain(chain)` in `oauth_proxy.py` wraps the one-line formatting of the configured IP-header-name chain — documents explicitly that header *names* (not values) are being rendered. 10 new unit tests in `tests/test_helpers.py::TestSafeUrlForLog` cover plain/userinfo-stripping/query-stripping/fragment-stripping/invalid-input/edge cases. Closes CodeQL alerts #5, #6, #7 (`oauth.py` + `oauth_proxy.py` clear-text-logging-sensitive-data flags). Closes or narrows #8 (header-chain log) depending on CodeQL's re-scan — the refactor factors the formatting through a helper to break the direct taint-flow path.
+
 ### Security
 - **Cascade the `#333` env-routing hardening into `pr-labels.yml`** — three job steps (`on-push`, `on-unlabel`, `on-label`) previously inlined `github.event.pull_request.number`, `github.repository`, and `github.event.pull_request.head.sha` as `${{ ... }}` expressions inside `run:` bodies. All now route through step-level `env:` (`PR`, `REPO`, `HEAD_SHA`) and are referenced as shell variables. Defense-in-depth — the current `on: pull_request:` trigger makes `secrets.GITHUB_TOKEN` read-only for fork PRs so the inline values weren't exploitable today (all were typed values: numeric PR, repo name, hex SHA), but the symmetric pattern protects against trigger-drift and parameterization-drift per [#334](https://github.com/cmeans/mcp-awareness/issues/334). Closes [#334](https://github.com/cmeans/mcp-awareness/issues/334).
 - **`ci.yml` workflow-level `permissions: contents: read`** — previously had no `permissions:` block, so the three jobs (`lint`, `typecheck`, `test`) inherited whatever repo-level default `GITHUB_TOKEN` scope was configured. Now explicit least-privilege: read-only contents access. Closes CodeQL alerts #1, #2, #3 (`actions/missing-workflow-permissions`).

src/mcp_awareness/helpers.py

@@ -106,6 +106,48 @@ def dsn_to_sqlalchemy_url(dsn: str) -> str:
     return base
 
 
+def safe_url_for_log(url: str) -> str:
+    """Render a URL for logging with credential-carrying parts stripped.
+
+    Keeps ``scheme://host[:port]/path`` so an operator can still read the
+    log and identify the endpoint. Strips:
+
+    - ``userinfo`` (anything before ``@`` in the netloc) — RFC 3986 lets
+      URLs embed ``user:password@host``; an OAuth issuer URL
+      misconfigured that way would leak credentials through every log
+      line that mentioned it.
+    - ``query`` string — the OAuth authorization-code flow passes codes,
+      state, and PKCE verifiers here; anything on the query string is
+      a candidate to be secret.
+    - ``fragment`` — the OAuth implicit flow (deprecated but still
+      occasionally seen) puts access tokens in fragments.
+
+    Defensive rather than currently-exploitable: the issuer URLs this
+    server consumes today are plain ``https://provider/...`` strings
+    with no userinfo / query / fragment. The helper exists so that if a
+    future misconfiguration, provider change, or redirect ever put a
+    credential into a URL that's about to be logged, we've already
+    redacted it.
+
+    Unparseable or empty inputs return ``"<redacted url>"`` rather than
+    raising — a log helper should never itself become a failure source.
+    """
+    from urllib.parse import urlparse, urlunparse
+
+    if not url:
+        return "<redacted url>"
+    try:
+        parsed = urlparse(url)
+        if not parsed.netloc:
+            return "<redacted url>"
+        # Rebuild netloc without the userinfo portion.
+        host = parsed.hostname or ""
+        netloc = f"{host}:{parsed.port}" if parsed.port else host
+        return urlunparse((parsed.scheme, netloc, parsed.path, "", "", ""))
+    except Exception:
+        return "<redacted url>"
+
+
 def canonical_email(email: str) -> str:
     """Normalize email for uniqueness: strip +tags, dots for gmail, lowercase."""
     email = email.lower().strip()

src/mcp_awareness/oauth.py

@@ -27,6 +27,8 @@
 import jwt
 from jwt import PyJWKClient
 
+from .helpers import safe_url_for_log
+
 logger = logging.getLogger(__name__)
 
 
@@ -70,19 +72,23 @@ def _discover_oidc_config(self) -> tuple[str, str]:
                 jwks = config.get("jwks_uri", "")
                 userinfo = config.get("userinfo_endpoint", "")
                 if jwks:
-                    logger.info("Discovered JWKS URI: %s", jwks)
+                    logger.info("Discovered JWKS URI: %s", safe_url_for_log(jwks))
                 if userinfo:
-                    logger.info("Discovered userinfo endpoint: %s", userinfo)
+                    logger.info("Discovered userinfo endpoint: %s", safe_url_for_log(userinfo))
                 return (
                     str(jwks) or f"{self.issuer}/.well-known/jwks.json",
                     str(userinfo),
                 )
         except Exception as exc:
-            logger.warning("OIDC discovery request failed for %s: %s", discovery_url, exc)
+            logger.warning(
+                "OIDC discovery request failed for %s: %s",
+                safe_url_for_log(discovery_url),
+                exc,
+            )
 
         logger.warning(
             "OIDC discovery failed for %s, using default JWKS path",
-            self.issuer,
+            safe_url_for_log(self.issuer),
         )
         return f"{self.issuer}/.well-known/jwks.json", ""
 

src/mcp_awareness/oauth_proxy.py

@@ -40,11 +40,28 @@
 
 from starlette.types import ASGIApp, Receive, Scope, Send
 
+from .helpers import safe_url_for_log
+
 logger = logging.getLogger(__name__)
 
 # Default trusted-header chain — Cloudflare environment
 _DEFAULT_IP_HEADERS = ["CF-Connecting-IP", "X-Real-IP"]
 
+
+def _format_ip_header_chain(chain: list[str]) -> str:
+    """Render the operator-readable display of the IP-header-name chain.
+
+    The input is a list of HTTP header *names* that the proxy will consult
+    (in order) to derive the client IP — not the values of those headers.
+    Header names are operator configuration, not credentials. This helper
+    exists so the caller's log statement doesn't have ``header_chain``
+    flowing directly into a format-string sink, which some static
+    analyzers conservatively flag because the list comes from a
+    constructor parameter.
+    """
+    return " → ".join(chain) + " → ASGI client"
+
+
 # Patterns that indicate injection attempts in parameter values
 _INJECTION_PATTERNS = re.compile(
     r"\.\./|\.\.\\|;.*(?:DROP|DELETE|INSERT|UPDATE|SELECT)|<script|%00|%0a|%0d",
@@ -222,7 +239,11 @@
         with urllib.request.urlopen(discovery_url, timeout=10) as resp:
             config = json.loads(resp.read())
     except Exception as exc:
-        logger.error("OAuth proxy: OIDC discovery failed for %s: %s", discovery_url, exc)
+        logger.error(
+            "OAuth proxy: OIDC discovery failed for %s: %s",
+            safe_url_for_log(discovery_url),
+            exc,
+        )
         return None
 
     authorization_endpoint = config.get("authorization_endpoint")
@@ -241,9 +262,9 @@
 
     logger.info(
         "OAuth proxy: discovered endpoints — authorize=%s, token=%s, register=%s",
-        authorization_endpoint,
-        token_endpoint,
-        registration_endpoint or "(not available)",
+        safe_url_for_log(authorization_endpoint),
+        safe_url_for_log(token_endpoint),
+        safe_url_for_log(registration_endpoint) if registration_endpoint else "(not available)",
     )
 
     return {
@@ -309,7 +330,7 @@
         header_chain = ip_headers or _DEFAULT_IP_HEADERS
         logger.info(
             "OAuth proxy: IP header chain = %s",
-            " → ".join(header_chain) + " → ASGI client",
+            _format_ip_header_chain(header_chain),
         )
 
     async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:

tests/test_helpers.py

@@ -29,9 +29,71 @@
     _validate_enum,
     _validate_timestamp,
     dsn_to_sqlalchemy_url,
+    safe_url_for_log,
 )
 
 
+class TestSafeUrlForLog:
+    """Redaction helper for URLs that might carry credentials or tokens."""
+
+    def test_plain_https_preserved(self):
+        assert (
+            safe_url_for_log("https://api.workos.com/user_management/client_X")
+            == "https://api.workos.com/user_management/client_X"
+        )
+
+    def test_strips_userinfo(self):
+        """user:password@host would otherwise leak credentials into logs."""
+        assert (
+            safe_url_for_log("https://user:secret@auth.example.com/oauth/token")
+            == "https://auth.example.com/oauth/token"
+        )
+
+    def test_strips_query_string(self):
+        """OAuth authorization-code flow puts codes/PKCE verifiers in query."""
+        assert (
+            safe_url_for_log("https://auth.example.com/callback?code=secret_code&state=xyz")
+            == "https://auth.example.com/callback"
+        )
+
+    def test_strips_fragment(self):
+        """OAuth implicit flow (deprecated but possible) puts tokens in fragment."""
+        assert (
+            safe_url_for_log("https://auth.example.com/callback#access_token=secret_token")
+            == "https://auth.example.com/callback"
+        )
+
+    def test_strips_all_credential_carrying_parts_at_once(self):
+        assert (
+            safe_url_for_log("https://u:p@auth.example.com:8443/oauth?code=c&state=s#token=t")
+            == "https://auth.example.com:8443/oauth"
+        )
+
+    def test_preserves_non_default_port(self):
+        assert (
+            safe_url_for_log("https://localhost:8443/oauth/token")
+            == "https://localhost:8443/oauth/token"
+        )
+
+    def test_empty_string_returns_placeholder(self):
+        assert safe_url_for_log("") == "<redacted url>"
+
+    def test_unparseable_string_returns_placeholder(self):
+        """A URL with no scheme/host shouldn't make the logger crash."""
+        assert safe_url_for_log("not a url") == "<redacted url>"
+
+    def test_no_netloc_returns_placeholder(self):
+        """file:// URLs with no host or schemeless paths fall through safely."""
+        assert safe_url_for_log("/path/only/no/scheme") == "<redacted url>"
+
+    def test_path_preserved_exactly(self):
+        """The path component is operator-meaningful; don't modify it."""
+        assert (
+            safe_url_for_log("https://example.com/deeply/nested/path/with.dots/and-dashes")
+            == "https://example.com/deeply/nested/path/with.dots/and-dashes"
+        )
+
+
 def test_default_query_limit_is_100():
     assert DEFAULT_QUERY_LIMIT == 100