fix(secrets): check URL secrets for placeholders and host:port addresses

docs/plugins.md

@@ -255,6 +255,10 @@ class InternalSecrets(SecretDetector):
     credential_names: ClassVar[tuple[str, ...]] = (*DEFAULT_CREDENTIAL_NAMES, "acme_key")
 ```
 
+A pattern whose regex names a `secret` group (`(?P<secret>...)`) is judged on that group:
+a match whose secret is a placeholder (`...`, `<token>`, `${VAR}`, fewer than four
+characters) is not reported, as with credential names.
+
 `entropy_threshold`, `hex_entropy_threshold` and `minimum_length` tune the high-entropy
 rule. Findings carry a redacted preview only.
 

src/coretrace_python/plugins/secrets.py

@@ -178,13 +178,19 @@ def _bound_name(target: nodes.Target) -> str | None:
 
 @dataclass(frozen=True)
 class SecretPattern:
-    """A provider-specific secret format."""
+    """A provider-specific secret format. When the regex names a ``secret`` group, that
+    group is the secret itself, and a match whose secret is a placeholder is not one;
+    punctuation that ends a sentence around it (``(see ?token=...)``) is not part of it."""
 
     provider: str
     regex: str
 
     def matches(self, text: str) -> bool:
-        return re.search(self.regex, text) is not None
+        for match in re.finditer(self.regex, text):
+            secret = match.groupdict().get("secret")
+            if secret is None or not is_placeholder(secret.rstrip(".,;:!?)")):
+                return True
+        return False
 
 
 DEFAULT_PATTERNS: tuple[SecretPattern, ...] = (
@@ -198,10 +204,21 @@ def matches(self, text: str) -> bool:
     SecretPattern("jwt", r"\beyJ[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\b"),
     SecretPattern("sendgrid", r"\bSG\.[A-Za-z0-9_-]{22}\.[A-Za-z0-9_-]{43}\b"),
     SecretPattern("twilio", r"\bSK[0-9a-fA-F]{32}\b"),
+    # A password in the user information, up to the last ``@`` before the host, since a
+    # password may hold one. An IP address or ``localhost`` and a port before the ``@``
+    # are an address, as in proxy specifications (``udp://127.0.0.1:[email protected]:0``);
+    # a dotted name is not, since user names such as ``john.doe`` have dots.
+    SecretPattern(
+        "url",
+        r"://(?!(?:\d{1,3}(?:\.\d{1,3}){3}|localhost):\d+@)"
+        r"[^/\s:@'\"]+:(?P<secret>[^/\s'\"]+)@",
+    ),
+    # A secret in a query parameter, up to a character no URL holds unescaped (RFC 3986):
+    # prose around it, such as Markdown quotes, is not part of it.
     SecretPattern(
         "url",
-        r"://[^/\s:@'\"]+:[^/\s@'\"]+@"
-        r"|[?&](?:password|passwd|pwd|token|api_key|apikey|secret|access_key)=[^&\s'\"]{3,}",
+        r"[?&](?:password|passwd|pwd|token|api_key|apikey|secret|access_key)="
+        r"(?P<secret>[^&\s'\"`<>{}|\\^]{3,})",
     ),
 )
 

tests/regression/expected/full-stack-fastapi-template.json

@@ -24,12 +24,6 @@
       "rule": "hardcoded-credential",
       "function": null
     },
-    {
-      "file": ".env",
-      "line": 18,
-      "rule": "hardcoded-secret",
-      "function": null
-    },
     {
       "file": "backend/app/api/deps.py",
       "line": 1,

tests/regression/expected/healthchecks.json

@@ -220,24 +220,6 @@
       "rule": "hardcoded-credential",
       "function": "test_it_updates_channel"
     },
-    {
-      "file": "hc/integrations/gotify/tests/test_notify.py",
-      "line": 50,
-      "rule": "hardcoded-secret",
-      "function": "test_it_works"
-    },
-    {
-      "file": "hc/integrations/gotify/tests/test_notify.py",
-      "line": 93,
-      "rule": "hardcoded-secret",
-      "function": "test_it_handles_subpath"
-    },
-    {
-      "file": "hc/integrations/gotify/tests/test_notify.py",
-      "line": 105,
-      "rule": "hardcoded-secret",
-      "function": "test_it_handles_subpath_with_missing_slash"
-    },
     {
       "file": "hc/integrations/matrix/tests/test_notify.py",
       "line": 15,

tests/test_url_secret_placeholders.py

@@ -0,0 +1,66 @@
+"""Acceptance tests for issue #209: credentials in URLs, not their placeholders.
+
+The ``url`` provider pattern finds a password in the user information of a URL and a
+secret in a query parameter. The secret it matches goes through the placeholder check
+that credentials already get: ``?token=...``, ``?token=<token>`` or ``?token={token}`` in
+help text are not secrets. A ``host:port@`` pair is an address, as in proxy mode
+specifications (``reverse:udp://127.0.0.1:[email protected]:0``), not a user and a password.
+Real credentials in URLs are still reported.
+"""
+
+from __future__ import annotations
+
+from pathlib import Path
+
+import pytest
+
+from coretrace_python import engine
+from coretrace_python.findings import Finding
+from coretrace_python.source import SourceManager
+
+PLUGINS = Path(__file__).resolve().parent.parent / "src" / "coretrace_python" / "bundled"
+
+
+def url_secrets(value: str) -> list[Finding]:
+    findings = engine.check(
+        SourceManager().add_source("app/cli.py", f"TEXT = {value!r}\n"), [PLUGINS]
+    )
+    return [f for f in findings if f.rule_id == "hardcoded-secret"]
+
+
+@pytest.mark.parametrize(
+    "value",
+    [
+        "--upload needs --token (the ?token=... mitmweb prints at startup)",
+        "The token is the `?token=...` in the URL mitmweb prints on startup.",
+        "open http://127.0.0.1:8081/?token=<token> in a browser",
+        "open http://127.0.0.1:8081/?token={token} in a browser",
+        "https://host.example/api?password=password",
+        "https://user:<password>@db.example/app",
+        "reverse:udp://127.0.0.1:[email protected]:0",
+        "reverse:http://localhost:[email protected]:80",
+        "(the ?token=...)",
+    ],
+)
+def test_placeholders_and_addresses_are_not_url_secrets(value: str) -> None:
+    assert url_secrets(value) == []
+
+
+@pytest.mark.parametrize(
+    "value",
+    [
+        "https://alice:[email protected]/",
+        "postgres://admin:[email protected]/app",
+        "https://admin:[email protected]/",
+        "https://host.example/api?token=a1b2c3d4e5f6",
+        "see ?token=... then https://host.example/api?api_key=a1b2c3d4e5f6",
+        "call `https://host.example/api?token=a1b2c3d4e5f6` to sync",
+        "mongodb://admin:P@ssw0rd123@mongo:27017/app",
+        "https://john.doe:[email protected]/",
+        "postgres://postgres.abcdefghij:[email protected]:6543/postgres",
+    ],
+)
+def test_real_credentials_in_urls_are_still_reported(value: str) -> None:
+    (finding,) = url_secrets(value)
+
+    assert finding.metadata["provider"] == "url"