From 2735450aedc0c0cd0e57fe358eee84648fee91b5 Mon Sep 17 00:00:00 2001 From: DSE Builder Date: Fri, 4 Sep 2026 21:04:15 +0000 Subject: [PATCH 1/4] =?UTF-8?q?fix(auth-audit):=20placeholder=20secrets,?= =?UTF-8?q?=20shell-default/Windows=20refs,=20secret-manager=20URIs=20?= =?UTF-8?q?=E2=80=94=20from=20a=20463-config=20public=20scan?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Running auth audit over a 463-config public corpus showed 74% of WRD-AUTH-TOKEN-IN-CONFIG hits were template fill-me-ins, and several correct reference shapes were reported as committed credentials. - New WRD-AUTH-PLACEHOLDER-SECRET (low) for obvious fill-me-ins. Built so it cannot hide a real secret: vendor/entropy hits are never downgraded, words match whole tokens (adherenceTokenValue is not `here`), the short-bare-word rule rejects digits/punctuation (hunter2! stays high), and a beside a literal is still a credential. - References no longer flagged: ${VAR:-default}/${VAR:?msg}, %VAR%, op:// vault:// awssm:// gcpsm:// azkv:// secretref:// keyring:// pass://, and credential-file paths (bare absolute paths need >= 2 directories so a base64 blob starting with / is still a literal). - Vendor scan runs once per value and gates the classification. - docs/AGENT_GATES.md rule table + rationale, CHANGELOG, DOCUMENTATION_INDEX. --- CHANGELOG.md | 18 ++++- DOCUMENTATION_INDEX.md | 2 +- docs/AGENT_GATES.md | 30 +++++++- src/mcp_warden/auth_audit.py | 133 +++++++++++++++++++++++++++++++---- tests/test_auth_audit.py | 94 +++++++++++++++++++++++++ 5 files changed, 259 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c379b6..7897c7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,7 +30,23 @@ Streamable HTTP; the v0.3 `guard` proxy adds deterministic runtime *result* insp ## [Unreleased] -_Nothing yet._ +### Changed + +- **`auth audit` / `doctor`: template placeholders are `WRD-AUTH-PLACEHOLDER-SECRET` + (low), not committed credentials.** Running the audit over a 463-config public + corpus showed 74 % of `WRD-AUTH-TOKEN-IN-CONFIG` hits were fill-me-ins + (``, `YOUR KEY GOES HERE`, `changeme`, `xxx`) — the false highs that + get a gate switched off. The downgrade cannot hide a real secret: any value the + vendor patterns or entropy heuristic recognise stays high; placeholder words match + whole tokens, never substrings (`adherenceTokenValue` is not `here`); the + short-bare-word rule rejects digits and punctuation (`hunter2!` stays high); and + a bracketed slot beside a literal (`Bearer aB3x…`) is a credential. +- **Reference forms found in the same corpus are no longer flagged as literals:** + `${VAR:-default}` / `${VAR:?msg}` shell expansions, `%VAR%` (Windows), + secret-manager URIs (`op://`, `vault://`, `awssm://`, `gcpsm://`, `azkv://`, + `secretref://`, `keyring://`, `pass://`), and credential-file paths. A bare + absolute path must have at least two directory segments so a base64 blob that + starts with `/` is still a literal. ## [1.2.0] — 2026-09-04 diff --git a/DOCUMENTATION_INDEX.md b/DOCUMENTATION_INDEX.md index 84b0a4e..229ba37 100644 --- a/DOCUMENTATION_INDEX.md +++ b/DOCUMENTATION_INDEX.md @@ -40,7 +40,7 @@ is the MCP auth posture sound. Both reuse the `check` exit-code contract (0 clea | [`docs/AGENT_GATES.md`](docs/AGENT_GATES.md) | Security contract for both gates — policy/evidence schemas, full rule tables, scope honesty, and the two load-bearing design decisions (evidence-adjudication, static-only) | | [`src/mcp_warden/deploy_gate.py`](src/mcp_warden/deploy_gate.py) | `WRD-GATE-*` engine: eval thresholds, guardrail presence, budget, approval receipt | | [`src/mcp_warden/cli_deploy_gate.py`](src/mcp_warden/cli_deploy_gate.py) | `deploy-gate` command body (register idiom) | -| [`src/mcp_warden/auth_audit.py`](src/mcp_warden/auth_audit.py) | `WRD-AUTH-*` static config audit; reuses `checks_secret.scan_field` for vendor patterns | +| [`src/mcp_warden/auth_audit.py`](src/mcp_warden/auth_audit.py) | `WRD-AUTH-*` static config audit; reuses `checks_secret.scan_field` for vendor patterns (vendor hits are never downgraded); template fill-me-ins are `WRD-AUTH-PLACEHOLDER-SECRET` (low), matched on whole tokens | | [`src/mcp_warden/cli_auth.py`](src/mcp_warden/cli_auth.py) | `auth audit` sub-app command body | | [`tests/test_deploy_gate.py`](tests/test_deploy_gate.py) | Engine per-control pass/fail + CLI exit codes + fail-closed on malformed evidence | | [`tests/test_auth_audit.py`](tests/test_auth_audit.py) | Every rule, the deliberate non-flags (loopback, `${VAR}` refs, stdio), redaction, CLI/JSON/SARIF | diff --git a/docs/AGENT_GATES.md b/docs/AGENT_GATES.md index d5ac7ab..b9584a3 100644 --- a/docs/AGENT_GATES.md +++ b/docs/AGENT_GATES.md @@ -111,6 +111,7 @@ and tracked separately (DSE-725). | `WRD-AUTH-NOAUTH` | medium | A remote endpoint declares no auth material | | `WRD-AUTH-PLAINTEXT-HTTP` | high | A remote endpoint uses `http://` | | `WRD-AUTH-TOKEN-IN-CONFIG` | high | An auth-bearing key holds a literal credential | +| `WRD-AUTH-PLACEHOLDER-SECRET` | low | An auth-bearing key holds an obvious template fill-me-in (``, `YOUR KEY GOES HERE`, `changeme`, `xxx`) — a config that cannot work, not a committed credential | | `WRD-AUTH-URL-CREDENTIAL` | high | The endpoint URL embeds a `user:pass@` userinfo credential | | `WRD-SEC-*` | varies | Vendor secret patterns found in any config value (shared with `check`) | @@ -121,7 +122,11 @@ Precision matters more than recall for a gate that blocks CI: - **Loopback servers** (`localhost`, `127.0.0.1`, `::1`) — not remotely reachable, so missing auth is not an exposure. - **Secret references, including embedded ones** — `${TOKEN}`, `$TOKEN`, - `{{ secret }}` are the correct pattern and are never flagged as literals, and + `${TOKEN:-default}` / `${TOKEN:?msg}` (shell expansion forms), `%TOKEN%` + (Windows), `{{ secret }}`, secret-manager URIs (`op://`, `vault://`, + `awssm://`, `gcpsm://`, `azkv://`, `secretref://`, `keyring://`, `pass://`) and + paths to a credential file (`~/.config/app/keys.json`, `/etc/app/secrets/token`) + are the correct pattern and are never flagged as literals, and that holds when the reference sits *inside* a larger value. **`Bearer ${TOKEN}` is correct configuration and is not a finding** — it is the most common shape an Authorization header takes, and flagging it was a real false positive found @@ -134,6 +139,29 @@ Precision matters more than recall for a gate that blocks CI: - **Local stdio servers** — a `command`/`args` entry with no URL and no remote transport has no auth posture to audit. +### Placeholders are a separate, low-severity rule + +Running the audit over a 463-config public corpus showed that **74 % of +`WRD-AUTH-TOKEN-IN-CONFIG` hits were template fill-me-ins** — ``, +`YOUR KEY GOES HERE`, `changeme`, `xxx`. Calling those committed credentials is +false, and false highs are what get a gate switched off. They are now +`WRD-AUTH-PLACEHOLDER-SECRET` (low): a shipped config that cannot work is still a +finding, just not a leak. + +The downgrade is built so it cannot hide a real secret: + +- **Vendor scan first.** Any value the `WRD-SEC-*` vendor patterns or the entropy + heuristic recognise is reported as a credential regardless of what else it + contains — a `ghp_…` token that happens to spell `example` stays high. +- **Whole-token matching.** Placeholder words match whole tokens of the value + (split on non-alphanumerics), never substrings: `adherenceTokenValue` is not + `here`, `fillmoreStreetPass` is not `fill`. +- **Short bare words only.** The "scheme/type word" rule (`admin`, `basic`, + `api-key`) accepts at most 11 purely alphabetic characters plus `-`/`_`. A digit + or any other punctuation (`hunter2!`, `p@ssword`) disqualifies the value. +- **Bracketed slots must stand alone.** `Bearer ` is a template; + `Bearer aB3x…` — a literal sitting beside the slot — is a credential. + ### Usage ```bash diff --git a/src/mcp_warden/auth_audit.py b/src/mcp_warden/auth_audit.py index 35f63e5..7f21891 100644 --- a/src/mcp_warden/auth_audit.py +++ b/src/mcp_warden/auth_audit.py @@ -71,7 +71,85 @@ def _safe_url(url: str) -> str: #: An env/secret-manager reference anywhere in the value: ``${TOKEN}``, ``$TOKEN``, #: or ``{{ secret }}``. -_SECRET_REF = re.compile(r"\$\{[A-Za-z_][A-Za-z0-9_]*\}|\$[A-Za-z_][A-Za-z0-9_]*|\{\{[^}]+\}\}") +#: ``${VAR}``, ``${VAR:-default}``/``${VAR:?msg}`` (shell expansion with a default), +#: ``$VAR``, ``{{ secret }}``, and ``%VAR%`` (Windows). +_SECRET_REF = re.compile( + r"\$\{[A-Za-z_][A-Za-z0-9_]*(?:[:\-+?][^}]*)?\}" + r"|\$[A-Za-z_][A-Za-z0-9_]*" + r"|\{\{[^}]+\}\}" + r"|%[A-Za-z_][A-Za-z0-9_]*%" +) + +#: Secret-manager reference URIs — the value names where the secret lives rather +#: than carrying it. Flagging these punishes operators for doing it right. +_SECRET_URI = re.compile( + r"^(?:op|vault|awssm|gcpsm|azkv|secretref|keyring|pass)://\S+$", re.IGNORECASE +) + +#: A filesystem path pointing at a credential file. The path is not the secret. +#: A bare absolute path needs >= 2 directory segments of path-safe characters: +#: a base64 blob that happens to start with ``/`` (``/9j/4AAQ...``) is a literal, +#: not a path, and must stay a finding. +_PATH_LIKE = re.compile( + r"^(?:~/|\./|\.\./|[A-Za-z]:\\)[^\s]*$" + r"|^/(?:[A-Za-z0-9._-]+/){2,}[A-Za-z0-9._-]+$" +) + +#: Obvious fill-me-in values shipped in template/example configs. These are NOT +#: committed credentials and reporting them as such is the noise that gets a gate +#: switched off — 74% of the WRD-AUTH-TOKEN-IN-CONFIG hits in a 463-config public +#: corpus were this, which is why they get their own low-severity rule. +#: +#: Matching is on whole tokens (the value split on non-alphanumerics), never on +#: raw substrings: ``adherenceTokenValue`` does not match ``here`` and +#: ``fillmoreStreetPass`` does not match ``fill``. Anything a vendor pattern or the +#: entropy heuristic recognises as a real secret is never downgraded (see +#: :func:`_scan_mapping_for_literals`). +_PLACEHOLDER_TOKENS = frozenset({ + "your", "yours", "placeholder", "example", "changeme", "todo", "tbd", "here", + "insert", "replace", "dummy", "sample", "fake", "fill", "redacted", + "abc123", "12345", "123456", "1234567890", +}) +#: Multi-token placeholder phrases, matched as whole consecutive tokens. +_PLACEHOLDER_PHRASES = ("change-me", "goes-here", "add-your", "put-your", "api-key-here") +#: Whole-value placeholders that do not tokenise usefully. +_PLACEHOLDER_EXACT = frozenset({"n/a", "none", "null", "-", "...", "sk-..."}) +_TOKEN_SPLIT = re.compile(r"[^a-z0-9]+") +_X_RUN = re.compile(r"^x{3,}$") +#: ```` / ```` — an angle-bracketed slot anywhere in the value. +_ANGLE_SLOT = re.compile(r"<[^<>]{1,64}>") +#: A short bare word: purely alphabetic with ``-``/``_`` and no digits or other +#: punctuation (``admin``, ``basic``, ``api-key``). A scheme/type token, not a +#: credential — real secrets carry entropy, length, or both. ``hunter2!`` and +#: ``p@ssword`` are NOT matched (digit / punctuation) and stay high severity. +_SHORT_BARE_WORD = re.compile(r"^[a-z][a-z_-]{0,10}$") + + +def _looks_like_placeholder(value: str) -> bool: + """True when the value is an obvious fill-me-in, not a real credential.""" + v = value.strip().lower() + if not v: + return False + if v in _PLACEHOLDER_EXACT or "..." in v: + return True + if _ANGLE_SLOT.search(v): + # A bracketed slot counts only when nothing but scheme words sits around + # it: ``Bearer `` is a template, ``Bearer aB3x…`` is not. + remainder = _ANGLE_SLOT.sub(" ", v) + if all(_REF_SURROUND_OK.match(tok) for tok in remainder.split()): + return True + tokens = [t for t in _TOKEN_SPLIT.split(v) if t] + if not tokens: + return False + if tokens[0] == "my" and len(tokens) > 1: + return True + if any(t in _PLACEHOLDER_TOKENS or _X_RUN.match(t) for t in tokens): + return True + joined = "-" + "-".join(tokens) + "-" + if any(f"-{phrase}-" in joined for phrase in _PLACEHOLDER_PHRASES): + return True + return bool(_SHORT_BARE_WORD.match(v)) + #: What may sit around a reference and still count as "no literal here": an auth #: scheme word. Real schemes (Bearer, Token, Basic, ApiKey, Negotiate) are short @@ -96,6 +174,10 @@ def _looks_like_secret_ref(value: str) -> bool: a real literal is still sitting there next to the reference. """ v = value.strip() + if _SECRET_URI.match(v) or _PATH_LIKE.match(v): + # `op://vault/item/field` or `~/.config/app/keys.json` — the value says + # where the secret lives, it does not carry it. + return True if not _SECRET_REF.search(v): return False remainder = _SECRET_REF.sub(" ", v) @@ -132,21 +214,42 @@ def _scan_mapping_for_literals(mapping: dict[str, Any], target: str) -> list[Fin for key, value in mapping.items(): if not isinstance(value, str) or not value: continue - if _is_auth_key(key) and not _looks_like_secret_ref(value): - findings.append( - Finding( - rule_id="WRD-AUTH-TOKEN-IN-CONFIG", - severity="high", - target=target, - message=( - f"auth key '{key}' holds a literal credential in config; " - "reference a secret manager (${VAR}) instead" - ), - snippet=_redact(value), + # Value-level secret scan (already redacts). Computed first: a value the + # vendor patterns or entropy heuristic recognise as a secret is a + # credential no matter how it is dressed up, and is never downgraded to + # a placeholder or excused as a reference. + vendor = scan_field(value, target) + if _is_auth_key(key) and (vendor or not _looks_like_secret_ref(value)): + if not vendor and _looks_like_placeholder(value): + # A template's fill-me-in slot is a real (low) finding — shipping + # a config that cannot work — but calling it a committed + # credential is false and is what gets the gate switched off. + findings.append( + Finding( + rule_id="WRD-AUTH-PLACEHOLDER-SECRET", + severity="low", + target=target, + message=( + f"auth key '{key}' holds a placeholder, not a working " + "credential; wire it to a secret reference (${VAR})" + ), + snippet=_redact(value), + ) ) - ) - # Value-level secret scan (already redacts). - findings.extend(scan_field(value, target)) + else: + findings.append( + Finding( + rule_id="WRD-AUTH-TOKEN-IN-CONFIG", + severity="high", + target=target, + message=( + f"auth key '{key}' holds a literal credential in config; " + "reference a secret manager (${VAR}) instead" + ), + snippet=_redact(value), + ) + ) + findings.extend(vendor) return findings diff --git a/tests/test_auth_audit.py b/tests/test_auth_audit.py index 0a84180..9549d81 100644 --- a/tests/test_auth_audit.py +++ b/tests/test_auth_audit.py @@ -105,6 +105,100 @@ def test_cli_audit_malformed_config_exit_two(tmp_path): assert result.exit_code == 2 +def test_reference_forms_found_in_a_real_public_corpus(): + # Every shape here appeared in a 463-config public scan and was WRONGLY + # reported as a committed credential. Each names where a secret lives; none + # carries one. + for good in ( + "${GITHUB_TOKEN:-}", # shell default-expansion + "${API_KEY:?required}", # shell error-if-unset + "op://Private/GitHub/token", # 1Password reference URI + "vault://secret/data/app#key", # Vault reference URI + "%USERPROFILE_TOKEN%", # Windows env expansion + "~/.config/app/keys.json", # path to a credential file + "/Users/me/.secrets/token", # absolute path + ): + server = {"url": "https://x.example.com", "headers": {"Authorization": good}} + assert audit_server("ok", server) == [], f"false positive on {good!r}" + + +def test_placeholders_are_low_severity_not_committed_credentials(): + # 74% of TOKEN-IN-CONFIG hits in the public corpus were template fill-me-ins. + # They are a real (low) finding, but calling them committed credentials is + # false and is the noise that gets a gate switched off. + for ph in ("YOUR KEY GOES HERE", "", "xxx", "changeme", "admin", "basic"): + server = {"url": "https://x.example.com", "headers": {"Authorization": ph}} + rules = _rules(audit_server("ph", server)) + assert "WRD-AUTH-PLACEHOLDER-SECRET" in rules, f"{ph!r} should be a placeholder" + assert "WRD-AUTH-TOKEN-IN-CONFIG" not in rules, f"{ph!r} wrongly called a credential" + + +def test_a_real_opaque_credential_is_still_high_severity(): + # The placeholder path must not become a bypass for real secrets. + server = {"url": "https://x.example.com", "headers": {"Authorization": "aB3xK9mQ7pL2wR5tY8vN4jH6"}} + assert "WRD-AUTH-TOKEN-IN-CONFIG" in _rules(audit_server("real", server)) + + +def test_short_real_secret_shapes_are_not_downgraded_to_placeholder(): + # The short-bare-word rule must not swallow a short REAL secret: a digit or any + # punctuation beyond -/_ takes the value out of the "scheme word" class. + for real in ("hunter2!", "p@ss-w0rd", "Xk9#mQ2p", "a1b2c3d4e5", "t0ken", "p@ssword"): + server = {"url": "https://x.example.com", "headers": {"Authorization": real}} + rules = _rules(audit_server("real", server)) + assert "WRD-AUTH-TOKEN-IN-CONFIG" in rules, f"{real!r} wrongly downgraded" + assert "WRD-AUTH-PLACEHOLDER-SECRET" not in rules + + +def test_placeholder_words_match_whole_tokens_not_substrings(): + # `here` must not match inside `adherence`, `fill` not inside `fillmore`. + for real in ("adherenceTokenValue", "fillmoreStreetPass", "nonesuchCredential", "exampledotcomPass"): + server = {"url": "https://x.example.com", "headers": {"Authorization": real}} + rules = _rules(audit_server("sub", server)) + assert "WRD-AUTH-TOKEN-IN-CONFIG" in rules, f"{real!r} wrongly downgraded" + # ...while the same words as WHOLE tokens are placeholders. + for ph in ("goes-here", "fill-me-in", "my-api-key", "change_me", "Bearer ", "sk-..."): + server = {"url": "https://x.example.com", "headers": {"Authorization": ph}} + rules = _rules(audit_server("ph", server)) + assert "WRD-AUTH-PLACEHOLDER-SECRET" in rules, f"{ph!r} should be a placeholder" + + +def test_vendor_shaped_secret_is_never_downgraded_even_if_it_contains_placeholder_words(): + # A real ghp_ token that happens to spell `example` is still a committed + # credential: the vendor scan is the guard, the placeholder heuristic cannot + # be used to hide a secret. + ghp = "ghp_" + "example" + "A1b2C3d4E5f6G7h8I9j0K1l2M3n4O" + assert len(ghp) == 4 + 36 + server = {"url": "https://x.example.com", "headers": {"Authorization": ghp}} + rules = _rules(audit_server("ghp", server)) + assert "WRD-SEC-GITHUB" in rules + assert "WRD-AUTH-TOKEN-IN-CONFIG" in rules + assert "WRD-AUTH-PLACEHOLDER-SECRET" not in rules + + +def test_angle_slot_beside_a_real_literal_is_still_a_credential(): + server = {"url": "https://x.example.com", "headers": {"Authorization": "Bearer aB3xK9mQ7pL2wR5t"}} + rules = _rules(audit_server("mixed", server)) + assert "WRD-AUTH-TOKEN-IN-CONFIG" in rules + assert "WRD-AUTH-PLACEHOLDER-SECRET" not in rules + + +def test_base64_blob_starting_with_slash_is_not_a_credential_path(): + # `/9j/4AAQ…` is a literal (one directory segment), not a path to a key file. + for blob in ("/9j/4AAQSkZJRgABAQAAAQABAAD", "/abc+def=="): + server = {"url": "https://x.example.com", "headers": {"Authorization": blob}} + assert "WRD-AUTH-TOKEN-IN-CONFIG" in _rules(audit_server("b64", server)), blob + # ...while a real credential-file path (>= 2 directories) is a reference. + server = {"url": "https://x.example.com", "headers": {"Authorization": "/etc/app/secrets/token"}} + assert audit_server("path", server) == [] + + +def test_placeholder_finding_is_redacted_like_every_other_snippet(): + server = {"url": "https://x.example.com", "headers": {"Authorization": "YOUR-KEY-GOES-HERE"}} + (f,) = [x for x in audit_server("ph", server) if x.rule_id == "WRD-AUTH-PLACEHOLDER-SECRET"] + assert f.severity == "low" + assert "YOUR-KEY-GOES-HERE" not in f.snippet and "…" in f.snippet + + def test_bearer_prefixed_reference_is_not_a_literal(): # `Bearer ${TOKEN}` is the single most common CORRECT shape for an # Authorization header. Flagging it as a committed credential is the From 1e4aced38aaa4cc0783018b93e6e3294285b73e0 Mon Sep 17 00:00:00 2001 From: DSE Builder Date: Fri, 4 Sep 2026 21:27:26 +0000 Subject: [PATCH 2/4] =?UTF-8?q?fix(auth-audit):=20bound=20the=20placeholde?= =?UTF-8?q?r=20downgrade=20=E2=80=94=20CSO=20review=20of=20#104=20(F1?= =?UTF-8?q?=E2=80=93F6)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - F1 ${VAR:-D} / ${VAR:+D} is a reference only when D is empty, itself a reference, or itself a placeholder; otherwise D is a literal. ${VAR:?msg} stays a reference. - F2 placeholder requires every token to be a placeholder or filler word, plus a hard floor: a 16+ char value with a digit and any non-placeholder token is never downgraded. - F3 only a closed scheme set (bearer|token|basic|apikey|negotiate|digest) may sit beside a reference or ; `correcthorse ${T}` is a literal. - F4 default credentials (admin, password, letmein, changeit, postgres, root, ...) never take the short-bare-word downgrade. - F5 every locator branch (op://, ~/, ./, ../, /, C:\) needs >= 2 segments and no mixed-case alphanumeric segment of 16+ chars. - F6 `...` is anchored (whole value or tail). - docs/AGENT_GATES.md states the true bound: vendor pattern or entropy (24+ chars at >= 4.0 bits/char) is guaranteed; shorter/low-entropy keys rely on the bounds above. Each bound is pinned by a test with the bypass string, plus a negative-twin test (every excused prefix + a real token stays high). --- CHANGELOG.md | 14 +-- docs/AGENT_GATES.md | 43 +++++++--- src/mcp_warden/auth_audit.py | 160 ++++++++++++++++++++++------------- tests/test_auth_audit.py | 85 ++++++++++++++++++- 4 files changed, 222 insertions(+), 80 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7897c7e..f91d1a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,11 +36,15 @@ Streamable HTTP; the v0.3 `guard` proxy adds deterministic runtime *result* insp (low), not committed credentials.** Running the audit over a 463-config public corpus showed 74 % of `WRD-AUTH-TOKEN-IN-CONFIG` hits were fill-me-ins (``, `YOUR KEY GOES HERE`, `changeme`, `xxx`) — the false highs that - get a gate switched off. The downgrade cannot hide a real secret: any value the - vendor patterns or entropy heuristic recognise stays high; placeholder words match - whole tokens, never substrings (`adherenceTokenValue` is not `here`); the - short-bare-word rule rejects digits and punctuation (`hunter2!` stays high); and - a bracketed slot beside a literal (`Bearer aB3x…`) is a credential. + get a gate switched off. The downgrade is bounded, and the bound is stated in + [`docs/AGENT_GATES.md`](docs/AGENT_GATES.md): vendor-pattern and entropy hits + (24+ chars at >= 4.0 bits/char) are classified as credentials before it runs; + below that, a value is a placeholder only when every token is a placeholder or + filler word, a 16+-char value with a digit and any real token is never + downgraded, only a closed scheme set may sit beside a reference, + `${VAR:-default}` counts only for an empty/reference/placeholder default, + locators need two segments with none token-shaped, and default credentials + (`admin`, `letmein`, …) stay high. - **Reference forms found in the same corpus are no longer flagged as literals:** `${VAR:-default}` / `${VAR:?msg}` shell expansions, `%VAR%` (Windows), secret-manager URIs (`op://`, `vault://`, `awssm://`, `gcpsm://`, `azkv://`, diff --git a/docs/AGENT_GATES.md b/docs/AGENT_GATES.md index b9584a3..5c071fb 100644 --- a/docs/AGENT_GATES.md +++ b/docs/AGENT_GATES.md @@ -148,19 +148,36 @@ false, and false highs are what get a gate switched off. They are now `WRD-AUTH-PLACEHOLDER-SECRET` (low): a shipped config that cannot work is still a finding, just not a leak. -The downgrade is built so it cannot hide a real secret: - -- **Vendor scan first.** Any value the `WRD-SEC-*` vendor patterns or the entropy - heuristic recognise is reported as a credential regardless of what else it - contains — a `ghp_…` token that happens to spell `example` stays high. -- **Whole-token matching.** Placeholder words match whole tokens of the value - (split on non-alphanumerics), never substrings: `adherenceTokenValue` is not - `here`, `fillmoreStreetPass` is not `fill`. -- **Short bare words only.** The "scheme/type word" rule (`admin`, `basic`, - `api-key`) accepts at most 11 purely alphabetic characters plus `-`/`_`. A digit - or any other punctuation (`hunter2!`, `p@ssword`) disqualifies the value. -- **Bracketed slots must stand alone.** `Bearer ` is a template; - `Bearer aB3x…` — a literal sitting beside the slot — is a credential. +**The bound, stated honestly.** The downgrade is guaranteed not to hide a value +the `WRD-SEC-*` vendor patterns match, or one the entropy heuristic catches +(**24+ characters at >= 4.0 bits/char, >= 80 % alphanumeric**) — those are +reported as credentials before the placeholder logic runs. A *shorter* or +*low-entropy* secret (a lowercase hex key, a passphrase) is outside that guard, +and for those the placeholder heuristic is the only line. It is bounded as +follows, each bound pinned by a test with the concrete bypass string: + +- **Whole-token matching, every token accounted for.** A value is a placeholder + only if at least one *strong* placeholder token (`your`, `example`, `changeme`, + `xxx`, …) is present as a whole token **and every other token is filler** + (`key`, `token`, `goes`, a vendor name). One placeholder word does not launder + the rest: `example-aB3xK9` is a credential. Hard floor: a value of 16+ + characters containing a digit and any non-placeholder token is never + downgraded. +- **Closed scheme set.** Only `Bearer`, `Token`, `Basic`, `ApiKey`, `Negotiate`, + `Digest` may sit beside a reference or a `` — `correcthorse ${TOKEN}` is + a literal. +- **`${VAR:-default}` is a reference only when the default is empty, itself a + reference, or itself a placeholder.** `${TOKEN:-aB3x…}` is a committed + credential wearing a reference; `${VAR:?msg}` stays a reference. +- **Locators need >= 2 segments and no token-shaped segment** (mixed-case + alphanumeric, 16+ chars): `op://Private/GitHub/token` and + `~/.config/app/keys.json` are references; `op://aB3x…`, `~/aB3x…`, `/9j/4AAQ…` + are literals. +- **Default credentials are secrets.** `admin`, `password`, `letmein`, `changeit`, + `postgres`, `root`, … never take the short-bare-word downgrade. The rule that + does (`basic`, `api-key`) accepts at most 11 purely alphabetic characters plus + `-`/`_`; a digit or other punctuation (`hunter2!`) disqualifies. +- **`...` is anchored** — the whole value or its tail (`sk-...`), never a substring. ### Usage diff --git a/src/mcp_warden/auth_audit.py b/src/mcp_warden/auth_audit.py index 7f21891..7f86a70 100644 --- a/src/mcp_warden/auth_audit.py +++ b/src/mcp_warden/auth_audit.py @@ -69,94 +69,129 @@ def _safe_url(url: str) -> str: return f"{scheme}://{_host_of(url)}" -#: An env/secret-manager reference anywhere in the value: ``${TOKEN}``, ``$TOKEN``, -#: or ``{{ secret }}``. -#: ``${VAR}``, ``${VAR:-default}``/``${VAR:?msg}`` (shell expansion with a default), -#: ``$VAR``, ``{{ secret }}``, and ``%VAR%`` (Windows). +#: An env/secret-manager reference anywhere in the value: ``${TOKEN}``, +#: ``${TOKEN:-default}`` / ``${TOKEN:?msg}`` (shell expansion forms, one level of +#: nesting), ``$TOKEN``, ``{{ secret }}``, and ``%TOKEN%`` (Windows). _SECRET_REF = re.compile( - r"\$\{[A-Za-z_][A-Za-z0-9_]*(?:[:\-+?][^}]*)?\}" + r"\$\{[A-Za-z_][A-Za-z0-9_]*(?:[:\-+?](?:[^{}]|\$\{[^{}]*\})*)?\}" r"|\$[A-Za-z_][A-Za-z0-9_]*" r"|\{\{[^}]+\}\}" r"|%[A-Za-z_][A-Za-z0-9_]*%" ) +#: ``${VAR:-D}`` / ``${VAR:+D}`` (colon optional): D is *substituted text* and is a +#: literal unless it is empty, itself a reference, or itself a placeholder (CSO F1 — +#: ``${TOKEN:-aB3x…}`` is a committed credential wearing a reference). ``${VAR:?msg}`` +#: names an error message, never a value, and stays a plain reference. +_REF_WITH_DEFAULT = re.compile(r"\$\{[A-Za-z_][A-Za-z0-9_]*:?[-+]((?:[^{}]|\$\{[^{}]*\})*)\}") + +#: A path segment / URI segment that is itself token-shaped: mixed-case +#: alphanumeric, 16+ chars. ``op://…/aB3xK9mQ7pL2wR5tY8vN`` and +#: ``~/.config/aB3xK9mQ7pL2wR5tY8vN`` carry the secret; they do not point at it. +_TOKENISH_SEGMENT = re.compile(r"(?=.*[a-z])(?=.*[A-Z])[A-Za-z0-9]{16,}") +_PATH_SEGMENT = re.compile(r"[A-Za-z0-9._~-]+") #: Secret-manager reference URIs — the value names where the secret lives rather -#: than carrying it. Flagging these punishes operators for doing it right. +#: than carrying it. Requires a ``/``-separated path of >= 2 segments after the +#: scheme, none of them token-shaped (CSO F5). _SECRET_URI = re.compile( - r"^(?:op|vault|awssm|gcpsm|azkv|secretref|keyring|pass)://\S+$", re.IGNORECASE + r"^(?:op|vault|awssm|gcpsm|azkv|secretref|keyring|pass)://(\S+)$", re.IGNORECASE ) +#: A filesystem path pointing at a credential file. Every branch (``~/``, ``./``, +#: ``../``, ``/``, ``C:\``) needs >= 2 path-safe segments, none token-shaped, so a +#: base64 blob that happens to start with ``/`` stays a literal (CSO F5). +_PATH_PREFIX = re.compile(r"^(?:~/|\./|\.\./|/|[A-Za-z]:[\\/])") -#: A filesystem path pointing at a credential file. The path is not the secret. -#: A bare absolute path needs >= 2 directory segments of path-safe characters: -#: a base64 blob that happens to start with ``/`` (``/9j/4AAQ...``) is a literal, -#: not a path, and must stay a finding. -_PATH_LIKE = re.compile( - r"^(?:~/|\./|\.\./|[A-Za-z]:\\)[^\s]*$" - r"|^/(?:[A-Za-z0-9._-]+/){2,}[A-Za-z0-9._-]+$" -) -#: Obvious fill-me-in values shipped in template/example configs. These are NOT -#: committed credentials and reporting them as such is the noise that gets a gate -#: switched off — 74% of the WRD-AUTH-TOKEN-IN-CONFIG hits in a 463-config public -#: corpus were this, which is why they get their own low-severity rule. -#: -#: Matching is on whole tokens (the value split on non-alphanumerics), never on -#: raw substrings: ``adherenceTokenValue`` does not match ``here`` and -#: ``fillmoreStreetPass`` does not match ``fill``. Anything a vendor pattern or the -#: entropy heuristic recognises as a real secret is never downgraded (see -#: :func:`_scan_mapping_for_literals`). +def _segments_are_a_locator(rest: str) -> bool: + segs = [seg for seg in re.split(r"[\\/]+", rest) if seg] + if len(segs) < 2: + return False + return all(_PATH_SEGMENT.fullmatch(seg) and not _TOKENISH_SEGMENT.fullmatch(seg) for seg in segs) + + +def _is_secret_locator(v: str) -> bool: + """A secret-manager URI or credential-file path: names where a secret lives.""" + m = _SECRET_URI.match(v) + if m: + return _segments_are_a_locator(m.group(1).split("#", 1)[0]) + m = _PATH_PREFIX.match(v) + return bool(m) and _segments_are_a_locator(v[m.end():]) + + +#: Strong placeholder tokens — a value is a fill-me-in only if at least one of +#: these is present as a WHOLE token and every other token is filler (CSO F2). _PLACEHOLDER_TOKENS = frozenset({ "your", "yours", "placeholder", "example", "changeme", "todo", "tbd", "here", "insert", "replace", "dummy", "sample", "fake", "fill", "redacted", "abc123", "12345", "123456", "1234567890", }) +#: Filler that may accompany a strong token without making the value a secret: +#: the thing being named (``key``, ``token``), connectives, and vendor names. +_FILLER_TOKENS = frozenset({ + "key", "api", "apikey", "token", "secret", "password", "pass", "access", "auth", + "id", "value", "string", "goes", "me", "in", "the", "a", "an", "to", "with", "my", + "name", "bearer", "github", "gitlab", "openai", "anthropic", "slack", "aws", + "google", "azure", "notion", "stripe", "brave", "tavily", "exa", "firecrawl", +}) #: Multi-token placeholder phrases, matched as whole consecutive tokens. _PLACEHOLDER_PHRASES = ("change-me", "goes-here", "add-your", "put-your", "api-key-here") #: Whole-value placeholders that do not tokenise usefully. -_PLACEHOLDER_EXACT = frozenset({"n/a", "none", "null", "-", "...", "sk-..."}) +_PLACEHOLDER_EXACT = frozenset({"n/a", "none", "null", "-"}) +#: Default credentials are real, working secrets (CSO F4). They never take the +#: short-bare-word downgrade; ``changeme`` is a strong placeholder token instead. +_DEFAULT_CREDENTIALS = frozenset({ + "changeit", "raspberry", "postgres", "grafana", "elastic", "letmein", "guest", + "toor", "admin", "password", "root", "secret", "test", +}) _TOKEN_SPLIT = re.compile(r"[^a-z0-9]+") _X_RUN = re.compile(r"^x{3,}$") -#: ```` / ```` — an angle-bracketed slot anywhere in the value. +#: ```` / ```` — an angle-bracketed slot. _ANGLE_SLOT = re.compile(r"<[^<>]{1,64}>") -#: A short bare word: purely alphabetic with ``-``/``_`` and no digits or other -#: punctuation (``admin``, ``basic``, ``api-key``). A scheme/type token, not a -#: credential — real secrets carry entropy, length, or both. ``hunter2!`` and -#: ``p@ssword`` are NOT matched (digit / punctuation) and stay high severity. +#: A short bare word: purely alphabetic with ``-``/``_``, no digits or other +#: punctuation (``basic``, ``api-key``). A scheme/type token, not a credential. +#: ``hunter2!`` and ``p@ssword`` are NOT matched and stay high severity. _SHORT_BARE_WORD = re.compile(r"^[a-z][a-z_-]{0,10}$") +#: Auth scheme words that may sit beside a reference or a slot and still count as +#: "no literal here". A closed set (CSO F3): any other alphabetic run — including +#: a short alphabetic secret such as ``correcthorse`` — is a literal. +_SCHEME_WORDS = frozenset({"bearer", "token", "basic", "apikey", "negotiate", "digest"}) + + +def _only_scheme_words(text: str) -> bool: + return all(tok.lower() in _SCHEME_WORDS for tok in text.split()) def _looks_like_placeholder(value: str) -> bool: - """True when the value is an obvious fill-me-in, not a real credential.""" + """True when the value is an obvious fill-me-in, not a real credential. + + Bounded on purpose (CSO F2): a value of 16+ characters that contains a digit + and any non-placeholder token is never downgraded, whatever else it says. + """ v = value.strip().lower() if not v: return False - if v in _PLACEHOLDER_EXACT or "..." in v: - return True - if _ANGLE_SLOT.search(v): - # A bracketed slot counts only when nothing but scheme words sits around - # it: ``Bearer `` is a template, ``Bearer aB3x…`` is not. - remainder = _ANGLE_SLOT.sub(" ", v) - if all(_REF_SURROUND_OK.match(tok) for tok in remainder.split()): - return True tokens = [t for t in _TOKEN_SPLIT.split(v) if t] - if not tokens: + strong = [t for t in tokens if t in _PLACEHOLDER_TOKENS or _X_RUN.match(t)] + if len(v) >= 16 and any(ch.isdigit() for ch in v) and len(strong) < len(tokens): return False - if tokens[0] == "my" and len(tokens) > 1: + if v in _PLACEHOLDER_EXACT or v == "..." or v.endswith("..."): return True - if any(t in _PLACEHOLDER_TOKENS or _X_RUN.match(t) for t in tokens): + if _ANGLE_SLOT.search(v) and _only_scheme_words(_ANGLE_SLOT.sub(" ", v)): return True + if not tokens: + return False joined = "-" + "-".join(tokens) + "-" - if any(f"-{phrase}-" in joined for phrase in _PLACEHOLDER_PHRASES): + phrase_tokens: set[str] = set() + for phrase in _PLACEHOLDER_PHRASES: + if f"-{phrase}-" in joined: + phrase_tokens.update(phrase.split("-")) + filler_ok = all( + t in _PLACEHOLDER_TOKENS or t in _FILLER_TOKENS or t in phrase_tokens or _X_RUN.match(t) + for t in tokens + ) + if filler_ok and (strong or phrase_tokens or (tokens[0] == "my" and len(tokens) > 1)): return True - return bool(_SHORT_BARE_WORD.match(v)) - - -#: What may sit around a reference and still count as "no literal here": an auth -#: scheme word. Real schemes (Bearer, Token, Basic, ApiKey, Negotiate) are short -#: and purely alphabetic — deliberately strict, because anything longer or -#: containing digits/punctuation is far more likely to BE the credential than to -#: name the scheme carrying it. -_REF_SURROUND_OK = re.compile(r"^[A-Za-z]{1,12}$") + return bool(_SHORT_BARE_WORD.match(v)) and v not in _DEFAULT_CREDENTIALS def _looks_like_secret_ref(value: str) -> bool: @@ -168,20 +203,23 @@ def _looks_like_secret_ref(value: str) -> bool: as a committed credential is a false positive that gets the whole gate switched off. - Conservative by construction: a reference must be present, and once every - reference is removed, whatever remains may only be scheme words and - separators. ``Bearer ${T}`` passes; ``Bearer abc123 ${T}`` does not, because - a real literal is still sitting there next to the reference. + Conservative by construction: a reference must be present; once every + reference is removed, whatever remains may only be auth scheme words + (``Bearer ${T}`` passes; ``Bearer abc123 ${T}`` and ``correcthorse ${T}`` do + not); and a ``${VAR:-default}`` counts only when the default is empty, itself + a reference, or itself a placeholder. """ v = value.strip() - if _SECRET_URI.match(v) or _PATH_LIKE.match(v): - # `op://vault/item/field` or `~/.config/app/keys.json` — the value says - # where the secret lives, it does not carry it. + if _is_secret_locator(v): return True if not _SECRET_REF.search(v): return False + for default in _REF_WITH_DEFAULT.findall(v): + d = default.strip() + if d and not _SECRET_REF.fullmatch(d) and not _looks_like_placeholder(d): + return False remainder = _SECRET_REF.sub(" ", v) - return all(_REF_SURROUND_OK.match(tok) for tok in remainder.split()) + return _only_scheme_words(remainder) def _server_has_auth(server: dict[str, Any]) -> bool: diff --git a/tests/test_auth_audit.py b/tests/test_auth_audit.py index 9549d81..f7e86a5 100644 --- a/tests/test_auth_audit.py +++ b/tests/test_auth_audit.py @@ -126,7 +126,7 @@ def test_placeholders_are_low_severity_not_committed_credentials(): # 74% of TOKEN-IN-CONFIG hits in the public corpus were template fill-me-ins. # They are a real (low) finding, but calling them committed credentials is # false and is the noise that gets a gate switched off. - for ph in ("YOUR KEY GOES HERE", "", "xxx", "changeme", "admin", "basic"): + for ph in ("YOUR KEY GOES HERE", "", "xxx", "changeme", "basic", "your_github_token", "api-key"): server = {"url": "https://x.example.com", "headers": {"Authorization": ph}} rules = _rules(audit_server("ph", server)) assert "WRD-AUTH-PLACEHOLDER-SECRET" in rules, f"{ph!r} should be a placeholder" @@ -199,6 +199,89 @@ def test_placeholder_finding_is_redacted_like_every_other_snippet(): assert "YOUR-KEY-GOES-HERE" not in f.snippet and "…" in f.snippet +REAL = "aB3xK9mQ7pL2wR5tY8vN4jH6" # 24 mixed-case alnum: token-shaped, no vendor prefix + + +def _auth(value): + return _rules(audit_server("s", {"url": "https://x.example.com", "headers": {"Authorization": value}})) + + +def _is_high(value): + rules = _auth(value) + return "WRD-AUTH-TOKEN-IN-CONFIG" in rules and "WRD-AUTH-PLACEHOLDER-SECRET" not in rules + + +def test_f1_shell_default_is_a_literal_unless_empty_reference_or_placeholder(): + # CSO F1: `${VAR:-D}` substitutes D — a secret hidden as the default is a credential. + assert _is_high("${TOKEN:-" + REAL + "}") + assert _is_high("${TOKEN-" + REAL + "}") + assert _is_high("${TOKEN:+" + REAL + "}") + assert _is_high("Bearer ${TOKEN:-hunter2!}") + for ref in ("${TOKEN:-}", "${TOKEN:-${OTHER}}", "${TOKEN:-$OTHER}", "${TOKEN:?required}", "${TOKEN?required}"): + assert not _auth(ref), ref + assert not _auth("${TOKEN:-changeme}") # placeholder default -> still a reference + + +def test_f2_placeholder_needs_every_token_to_be_filler_and_has_a_hard_floor(): + # CSO F2: one placeholder word does not launder the rest of the value. + assert _is_high("example-aB3xK9") # < 16 chars, still not all filler + assert _is_high("your-key-" + REAL) # >= 16, digit, non-placeholder token -> floor + assert _is_high("placeholder" + REAL) # strong token glued to a real one + assert _is_high("YOUR KEY GOES HERE 12345 " + REAL) + assert "WRD-AUTH-PLACEHOLDER-SECRET" in _auth("your-github-token") + assert "WRD-AUTH-PLACEHOLDER-SECRET" in _auth("YOUR KEY GOES HERE") + + +def test_f3_only_a_closed_set_of_scheme_words_may_sit_beside_a_reference(): + # CSO F3: a short alphabetic SECRET beside a reference is a literal. + assert _is_high("correcthorse ${TOKEN}") + assert _is_high("hunter ${TOKEN}") + assert _is_high("correcthorse ") + for scheme in ("Bearer", "Token", "Basic", "ApiKey", "Negotiate", "Digest", "bearer"): + assert not _auth(scheme + " ${TOKEN}"), scheme + assert "WRD-AUTH-PLACEHOLDER-SECRET" in _auth(scheme + " "), scheme + + +def test_f4_default_credentials_are_never_downgraded(): + # CSO F4: `admin` / `letmein` are working secrets, not template slots. + for cred in ("changeit", "raspberry", "postgres", "grafana", "elastic", "letmein", + "guest", "toor", "admin", "password", "root", "secret", "test"): + assert _is_high(cred), cred + assert "WRD-AUTH-PLACEHOLDER-SECRET" in _auth("changeme") + + +def test_f5_locators_need_two_segments_and_no_token_shaped_segment(): + # CSO F5: a URI/path that CARRIES the secret is not a locator. + for lit in ("op://" + REAL, "vault://" + REAL, "op://a/" + REAL, "~/" + REAL, "~/.config/" + REAL, + "/etc/" + REAL, "/a/b/" + REAL, "./" + REAL, "C:\\" + REAL, "C:\\Users\\" + REAL, + "/9j/4AAQSkZJRgABAQAAAQABAAD", "/abc+def=="): + assert _is_high(lit), lit + for loc in ("op://Private/GitHub/token", "vault://secret/data/app#key", "~/.config/app/keys.json", + "/etc/app/secrets/token", "./secrets/token.txt", "../keys/app.json", "C:\\Users\\me\\keys.json"): + assert not _auth(loc), loc + + +def test_f6_ellipsis_is_anchored(): + # CSO F6: `...` marks a placeholder only as the whole value or its tail. + assert "WRD-AUTH-PLACEHOLDER-SECRET" in _auth("sk-...") + assert "WRD-AUTH-PLACEHOLDER-SECRET" in _auth("your-key...") + assert "WRD-AUTH-PLACEHOLDER-SECRET" in _auth("...") + assert _is_high("abc..." + REAL) + assert _is_high(REAL + "...x") + + +def test_negative_twin_every_benign_prefix_plus_a_real_token_is_still_high(): + # For each shape the audit deliberately excuses, the same shape carrying a real + # token must still be a committed credential. + for prefix, suffix in ( + ("Bearer ", ""), ("Token ", ""), ("Basic ", ""), ("ApiKey ", ""), ("Negotiate ", ""), ("Digest ", ""), + ("${TOKEN:-", "}"), (" ", ""), ("op://Private/GitHub/", ""), ("~/.config/app/", ""), + ("your-key-", ""), ("changeme-", ""), ("example/", ""), ("", "..."), ("", ""), + ): + value = prefix + REAL + suffix + assert _is_high(value), value + + def test_bearer_prefixed_reference_is_not_a_literal(): # `Bearer ${TOKEN}` is the single most common CORRECT shape for an # Authorization header. Flagging it as a committed credential is the From ab93a464a99f92dfcbf6a70a7228a37138fc844a Mon Sep 17 00:00:00 2001 From: DSE Builder Date: Fri, 4 Sep 2026 21:48:55 +0000 Subject: [PATCH 3/4] =?UTF-8?q?fix(auth-audit):=20close=20the=20re-verify?= =?UTF-8?q?=20gaps=20=E2=80=94=20CSO=20pass=203=20of=20#104=20(N1=E2=80=93?= =?UTF-8?q?N5)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - N1 a ${VAR:-D} default is checked recursively: ${T:-${U:-hunter2!}} and ${T:-${U:-9f8e7d6c5b4a}} are literals. - N2 ${VAR:=D} is an expansion form too; a non-benign default is a literal. - N3 token-shaped locator segments are case-blind: alphanumeric-only and 20+ chars, or 16+ at >= 3.5 bits/char (shannon). ~/.config/<24 hex> and ~/.config/GHSAT0AAAAAABCDEFGHIJ are literals; file names with dots or underscores (application_default_credentials.json) stay locators. - N4 the short-bare-word downgrade is an allowlist of scheme/type slots (basic, bearer, token, apikey/api-key, digest, negotiate, oauth, none); _DEFAULT_CREDENTIALS deleted — admin, qwerty, welcome, letmein are high. - N5 the 16+/digit hard floor applies only when a non-filler token is present: your-api-key-goes-here-12345 and YOUR_API_KEY_1234567890 stay low. - Tests now use sub-threshold literals (9f8e7d6c5b4a, 24-hex at H~3.92) so each assertion fails on the pre-fix code; one REAL case kept as the entropy-path control. docs/AGENT_GATES.md + CHANGELOG describe the allowlist rule. --- CHANGELOG.md | 8 +-- docs/AGENT_GATES.md | 37 +++++++------ src/mcp_warden/auth_audit.py | 86 ++++++++++++++++-------------- tests/test_auth_audit.py | 100 +++++++++++++++++++++++------------ 4 files changed, 138 insertions(+), 93 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7acaf62..ba3938a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,9 +42,11 @@ Streamable HTTP; the v0.3 `guard` proxy adds deterministic runtime *result* insp below that, a value is a placeholder only when every token is a placeholder or filler word, a 16+-char value with a digit and any real token is never downgraded, only a closed scheme set may sit beside a reference, - `${VAR:-default}` counts only for an empty/reference/placeholder default, - locators need two segments with none token-shaped, and default credentials - (`admin`, `letmein`, …) stay high. + `${VAR:-default}` / `${VAR:=default}` counts only for an empty, reference + (recursively) or placeholder default, locators need two segments with none + token-shaped (case-blind: 20+ alphanumerics or 16+ at >= 3.5 bits/char), and + short bare words are downgraded only from a closed allowlist of scheme/type + slots — `admin`, `qwerty`, `letmein` stay high. - **Reference forms found in the same corpus are no longer flagged as literals:** `${VAR:-default}` / `${VAR:?msg}` shell expansions, `%VAR%` (Windows), secret-manager URIs (`op://`, `vault://`, `awssm://`, `gcpsm://`, `azkv://`, diff --git a/docs/AGENT_GATES.md b/docs/AGENT_GATES.md index 5c071fb..bef053b 100644 --- a/docs/AGENT_GATES.md +++ b/docs/AGENT_GATES.md @@ -154,30 +154,35 @@ the `WRD-SEC-*` vendor patterns match, or one the entropy heuristic catches reported as credentials before the placeholder logic runs. A *shorter* or *low-entropy* secret (a lowercase hex key, a passphrase) is outside that guard, and for those the placeholder heuristic is the only line. It is bounded as -follows, each bound pinned by a test with the concrete bypass string: +follows, each bound pinned by a test with a sub-threshold bypass string: - **Whole-token matching, every token accounted for.** A value is a placeholder only if at least one *strong* placeholder token (`your`, `example`, `changeme`, `xxx`, …) is present as a whole token **and every other token is filler** (`key`, `token`, `goes`, a vendor name). One placeholder word does not launder - the rest: `example-aB3xK9` is a credential. Hard floor: a value of 16+ - characters containing a digit and any non-placeholder token is never - downgraded. + the rest: `example-9f8e7d6c5b4a` is a credential. Hard floor: unless every + token is placeholder/filler, a value of 16+ characters containing a digit is + never downgraded (`YOUR_API_KEY_1234567890` is all filler and stays low). - **Closed scheme set.** Only `Bearer`, `Token`, `Basic`, `ApiKey`, `Negotiate`, `Digest` may sit beside a reference or a `` — `correcthorse ${TOKEN}` is a literal. -- **`${VAR:-default}` is a reference only when the default is empty, itself a - reference, or itself a placeholder.** `${TOKEN:-aB3x…}` is a committed - credential wearing a reference; `${VAR:?msg}` stays a reference. -- **Locators need >= 2 segments and no token-shaped segment** (mixed-case - alphanumeric, 16+ chars): `op://Private/GitHub/token` and - `~/.config/app/keys.json` are references; `op://aB3x…`, `~/aB3x…`, `/9j/4AAQ…` - are literals. -- **Default credentials are secrets.** `admin`, `password`, `letmein`, `changeit`, - `postgres`, `root`, … never take the short-bare-word downgrade. The rule that - does (`basic`, `api-key`) accepts at most 11 purely alphabetic characters plus - `-`/`_`; a digit or other punctuation (`hunter2!`) disqualifies. -- **`...` is anchored** — the whole value or its tail (`sk-...`), never a substring. +- **`${VAR:-default}` / `${VAR:=default}` is a reference only when the default is + empty, itself a reference (recursively), or itself a placeholder.** + `${TOKEN:-9f8e7d6c5b4a}` and `${T:-${U:-hunter2!}}` are committed credentials + wearing a reference; `${VAR:?msg}` stays a reference. +- **Locators need >= 2 segments and no token-shaped segment.** Token-shaped is + case-blind: an alphanumeric-only segment of 20+ characters, or 16+ at + >= 3.5 bits/char. `op://Private/GitHub/token`, `~/.config/app/keys.json` and + `~/.config/gcloud/application_default_credentials.json` are references; + `op://9f8e…`, `~/9f8e…`, `~/.config/9f8e7d6c5b4a3e2d1c0b9a8f`, + `~/.config/GHSAT0AAAAAABCDEFGHIJ` and `/9j/4AAQ…` are literals. +- **Short bare words are an allowlist, not a heuristic.** Only `basic`, `bearer`, + `token`, `apikey`/`api-key`, `digest`, `negotiate`, `oauth`, `none` are treated + as scheme/type slots. Everything else — `admin`, `password`, `letmein`, + `qwerty`, `welcome`, `hunter2!` — is a working secret and stays high. +- **`...` is anchored** — the whole value, or the tail of a short stub (`sk-...`) + or an all-filler value (`your-key...`); never a substring, never behind a real + token. ### Usage diff --git a/src/mcp_warden/auth_audit.py b/src/mcp_warden/auth_audit.py index 7f86a70..1cc2ce7 100644 --- a/src/mcp_warden/auth_audit.py +++ b/src/mcp_warden/auth_audit.py @@ -21,7 +21,7 @@ from pathlib import Path from typing import Any -from .checks_secret import scan_field +from .checks_secret import scan_field, shannon_entropy from .models import Finding from .redact import redact_secret @@ -70,35 +70,46 @@ def _safe_url(url: str) -> str: #: An env/secret-manager reference anywhere in the value: ``${TOKEN}``, -#: ``${TOKEN:-default}`` / ``${TOKEN:?msg}`` (shell expansion forms, one level of -#: nesting), ``$TOKEN``, ``{{ secret }}``, and ``%TOKEN%`` (Windows). +#: ``${TOKEN:-default}`` / ``${TOKEN:=default}`` / ``${TOKEN:?msg}`` (shell expansion +#: forms, one level of nesting), ``$TOKEN``, ``{{ secret }}``, ``%TOKEN%`` (Windows). _SECRET_REF = re.compile( - r"\$\{[A-Za-z_][A-Za-z0-9_]*(?:[:\-+?](?:[^{}]|\$\{[^{}]*\})*)?\}" + r"\$\{[A-Za-z_][A-Za-z0-9_]*(?:[:\-+?=](?:[^{}]|\$\{[^{}]*\})*)?\}" r"|\$[A-Za-z_][A-Za-z0-9_]*" r"|\{\{[^}]+\}\}" r"|%[A-Za-z_][A-Za-z0-9_]*%" ) -#: ``${VAR:-D}`` / ``${VAR:+D}`` (colon optional): D is *substituted text* and is a -#: literal unless it is empty, itself a reference, or itself a placeholder (CSO F1 — -#: ``${TOKEN:-aB3x…}`` is a committed credential wearing a reference). ``${VAR:?msg}`` -#: names an error message, never a value, and stays a plain reference. -_REF_WITH_DEFAULT = re.compile(r"\$\{[A-Za-z_][A-Za-z0-9_]*:?[-+]((?:[^{}]|\$\{[^{}]*\})*)\}") - -#: A path segment / URI segment that is itself token-shaped: mixed-case -#: alphanumeric, 16+ chars. ``op://…/aB3xK9mQ7pL2wR5tY8vN`` and -#: ``~/.config/aB3xK9mQ7pL2wR5tY8vN`` carry the secret; they do not point at it. -_TOKENISH_SEGMENT = re.compile(r"(?=.*[a-z])(?=.*[A-Z])[A-Za-z0-9]{16,}") +#: ``${VAR:-D}`` / ``${VAR:+D}`` / ``${VAR:=D}`` (colon optional): D is *substituted +#: text* and is a literal unless it is empty, itself a reference (recursively — +#: ``${T:-${U:-hunter2!}}`` bottoms out on a literal), or itself a placeholder +#: (CSO F1/N1/N2). ``${VAR:?msg}`` names an error message, never a value. +_REF_WITH_DEFAULT = re.compile(r"\$\{[A-Za-z_][A-Za-z0-9_]*:?[-+=]((?:[^{}]|\$\{[^{}]*\})*)\}") + +#: A path/URI segment that is itself token-shaped (CSO F5/N3): alphanumeric only, +#: case-blind, and either 20+ characters or 16+ at >= 3.5 bits/char — a lowercase +#: hex key or a GHSAT-style token is carried, not pointed at. Segments with dots +#: or underscores (``application_default_credentials.json``) are file names. +_ALNUM_SEGMENT = re.compile(r"[A-Za-z0-9]+") _PATH_SEGMENT = re.compile(r"[A-Za-z0-9._~-]+") +_TOKENISH_MIN_LEN, _TOKENISH_LONG, _TOKENISH_ENTROPY = 16, 20, 3.5 + + +def _tokenish_segment(seg: str) -> bool: + if not _ALNUM_SEGMENT.fullmatch(seg): + return False + if len(seg) >= _TOKENISH_LONG: + return True + return len(seg) >= _TOKENISH_MIN_LEN and shannon_entropy(seg) >= _TOKENISH_ENTROPY + #: Secret-manager reference URIs — the value names where the secret lives rather #: than carrying it. Requires a ``/``-separated path of >= 2 segments after the -#: scheme, none of them token-shaped (CSO F5). +#: scheme, none of them token-shaped. _SECRET_URI = re.compile( r"^(?:op|vault|awssm|gcpsm|azkv|secretref|keyring|pass)://(\S+)$", re.IGNORECASE ) #: A filesystem path pointing at a credential file. Every branch (``~/``, ``./``, #: ``../``, ``/``, ``C:\``) needs >= 2 path-safe segments, none token-shaped, so a -#: base64 blob that happens to start with ``/`` stays a literal (CSO F5). +#: base64 blob that happens to start with ``/`` stays a literal. _PATH_PREFIX = re.compile(r"^(?:~/|\./|\.\./|/|[A-Za-z]:[\\/])") @@ -106,7 +117,7 @@ def _segments_are_a_locator(rest: str) -> bool: segs = [seg for seg in re.split(r"[\\/]+", rest) if seg] if len(segs) < 2: return False - return all(_PATH_SEGMENT.fullmatch(seg) and not _TOKENISH_SEGMENT.fullmatch(seg) for seg in segs) + return all(_PATH_SEGMENT.fullmatch(seg) and not _tokenish_segment(seg) for seg in segs) def _is_secret_locator(v: str) -> bool: @@ -137,20 +148,15 @@ def _is_secret_locator(v: str) -> bool: _PLACEHOLDER_PHRASES = ("change-me", "goes-here", "add-your", "put-your", "api-key-here") #: Whole-value placeholders that do not tokenise usefully. _PLACEHOLDER_EXACT = frozenset({"n/a", "none", "null", "-"}) -#: Default credentials are real, working secrets (CSO F4). They never take the -#: short-bare-word downgrade; ``changeme`` is a strong placeholder token instead. -_DEFAULT_CREDENTIALS = frozenset({ - "changeit", "raspberry", "postgres", "grafana", "elastic", "letmein", "guest", - "toor", "admin", "password", "root", "secret", "test", +#: The ONLY short bare words that are a scheme/type slot rather than a credential +#: (CSO N4 — an allowlist; ``admin``, ``qwerty``, ``letmein`` are working secrets). +_SCHEME_TYPE_WORDS = frozenset({ + "basic", "bearer", "token", "apikey", "api-key", "digest", "negotiate", "oauth", "none", }) _TOKEN_SPLIT = re.compile(r"[^a-z0-9]+") _X_RUN = re.compile(r"^x{3,}$") #: ```` / ```` — an angle-bracketed slot. _ANGLE_SLOT = re.compile(r"<[^<>]{1,64}>") -#: A short bare word: purely alphabetic with ``-``/``_``, no digits or other -#: punctuation (``basic``, ``api-key``). A scheme/type token, not a credential. -#: ``hunter2!`` and ``p@ssword`` are NOT matched and stay high severity. -_SHORT_BARE_WORD = re.compile(r"^[a-z][a-z_-]{0,10}$") #: Auth scheme words that may sit beside a reference or a slot and still count as #: "no literal here". A closed set (CSO F3): any other alphabetic run — including #: a short alphabetic secret such as ``correcthorse`` — is a literal. @@ -164,34 +170,34 @@ def _only_scheme_words(text: str) -> bool: def _looks_like_placeholder(value: str) -> bool: """True when the value is an obvious fill-me-in, not a real credential. - Bounded on purpose (CSO F2): a value of 16+ characters that contains a digit - and any non-placeholder token is never downgraded, whatever else it says. + Bounded on purpose (CSO F2/N5): unless every token is a placeholder or filler + word, a value of 16+ characters that contains a digit is never downgraded. """ v = value.strip().lower() if not v: return False tokens = [t for t in _TOKEN_SPLIT.split(v) if t] strong = [t for t in tokens if t in _PLACEHOLDER_TOKENS or _X_RUN.match(t)] - if len(v) >= 16 and any(ch.isdigit() for ch in v) and len(strong) < len(tokens): - return False - if v in _PLACEHOLDER_EXACT or v == "..." or v.endswith("..."): - return True - if _ANGLE_SLOT.search(v) and _only_scheme_words(_ANGLE_SLOT.sub(" ", v)): - return True - if not tokens: - return False joined = "-" + "-".join(tokens) + "-" phrase_tokens: set[str] = set() for phrase in _PLACEHOLDER_PHRASES: if f"-{phrase}-" in joined: phrase_tokens.update(phrase.split("-")) - filler_ok = all( + filler_ok = bool(tokens) and all( t in _PLACEHOLDER_TOKENS or t in _FILLER_TOKENS or t in phrase_tokens or _X_RUN.match(t) for t in tokens ) if filler_ok and (strong or phrase_tokens or (tokens[0] == "my" and len(tokens) > 1)): return True - return bool(_SHORT_BARE_WORD.match(v)) and v not in _DEFAULT_CREDENTIALS + if len(v) >= 16 and any(ch.isdigit() for ch in v): + return False # hard floor: a long value with a digit and a real token + if v in _PLACEHOLDER_EXACT or v == "...": + return True + if v.endswith("...") and (len(v) <= 12 or filler_ok): + return True # `sk-...`, `your-key...` — never `...` + if _ANGLE_SLOT.search(v) and _only_scheme_words(_ANGLE_SLOT.sub(" ", v)): + return True + return v.replace("_", "-") in _SCHEME_TYPE_WORDS def _looks_like_secret_ref(value: str) -> bool: @@ -207,7 +213,7 @@ def _looks_like_secret_ref(value: str) -> bool: reference is removed, whatever remains may only be auth scheme words (``Bearer ${T}`` passes; ``Bearer abc123 ${T}`` and ``correcthorse ${T}`` do not); and a ``${VAR:-default}`` counts only when the default is empty, itself - a reference, or itself a placeholder. + a reference (recursively), or itself a placeholder. """ v = value.strip() if _is_secret_locator(v): @@ -216,7 +222,7 @@ def _looks_like_secret_ref(value: str) -> bool: return False for default in _REF_WITH_DEFAULT.findall(v): d = default.strip() - if d and not _SECRET_REF.fullmatch(d) and not _looks_like_placeholder(d): + if d and not _looks_like_secret_ref(d) and not _looks_like_placeholder(d): return False remainder = _SECRET_REF.sub(" ", v) return _only_scheme_words(remainder) diff --git a/tests/test_auth_audit.py b/tests/test_auth_audit.py index f7e86a5..5e5f1e6 100644 --- a/tests/test_auth_audit.py +++ b/tests/test_auth_audit.py @@ -199,7 +199,13 @@ def test_placeholder_finding_is_redacted_like_every_other_snippet(): assert "YOUR-KEY-GOES-HERE" not in f.snippet and "…" in f.snippet -REAL = "aB3xK9mQ7pL2wR5tY8vN4jH6" # 24 mixed-case alnum: token-shaped, no vendor prefix +# Sub-threshold literals: NOT caught by the vendor patterns or the entropy guard +# (24 chars at >= 4.0 bits/char), so every assertion below exercises the +# placeholder/reference logic itself and fails on the pre-fix code. +LIT = "9f8e7d6c5b4a" # 12-char lowercase hex +LIT24 = "9f8e7d6c5b4a3e2d1c0b9a8f" # 24-char lowercase hex, H ~ 3.92 bits/char +# Entropy-path control: 24 mixed-case alnum, H >= 4.0 — caught by checks_secret. +REAL = "aB3xK9mQ7pL2wR5tY8vN4jH6" def _auth(value): @@ -211,25 +217,38 @@ def _is_high(value): return "WRD-AUTH-TOKEN-IN-CONFIG" in rules and "WRD-AUTH-PLACEHOLDER-SECRET" not in rules +def _is_low(value): + return "WRD-AUTH-PLACEHOLDER-SECRET" in _auth(value) + + def test_f1_shell_default_is_a_literal_unless_empty_reference_or_placeholder(): - # CSO F1: `${VAR:-D}` substitutes D — a secret hidden as the default is a credential. - assert _is_high("${TOKEN:-" + REAL + "}") - assert _is_high("${TOKEN-" + REAL + "}") - assert _is_high("${TOKEN:+" + REAL + "}") + # CSO F1/N1/N2: `${VAR:-D}` substitutes D — a secret hidden as the default is a + # credential, at any nesting depth, and `:=` is an assignment form too. + assert _is_high("${TOKEN:-" + LIT + "}") + assert _is_high("${TOKEN-" + LIT + "}") + assert _is_high("${TOKEN:+" + LIT + "}") + assert _is_high("${TOKEN:=hunter2!}") + assert _is_high("${T:-${U:-hunter2!}}") + assert _is_high("${T:-${U:-" + LIT + "}}") assert _is_high("Bearer ${TOKEN:-hunter2!}") - for ref in ("${TOKEN:-}", "${TOKEN:-${OTHER}}", "${TOKEN:-$OTHER}", "${TOKEN:?required}", "${TOKEN?required}"): + for ref in ("${TOKEN:-}", "${TOKEN:-${OTHER}}", "${TOKEN:-$OTHER}", "${TOKEN:?required}", + "${TOKEN?required}", "${T:-${U:-}}", "${T:-${U:-changeme}}"): assert not _auth(ref), ref assert not _auth("${TOKEN:-changeme}") # placeholder default -> still a reference def test_f2_placeholder_needs_every_token_to_be_filler_and_has_a_hard_floor(): # CSO F2: one placeholder word does not launder the rest of the value. - assert _is_high("example-aB3xK9") # < 16 chars, still not all filler - assert _is_high("your-key-" + REAL) # >= 16, digit, non-placeholder token -> floor - assert _is_high("placeholder" + REAL) # strong token glued to a real one - assert _is_high("YOUR KEY GOES HERE 12345 " + REAL) - assert "WRD-AUTH-PLACEHOLDER-SECRET" in _auth("your-github-token") - assert "WRD-AUTH-PLACEHOLDER-SECRET" in _auth("YOUR KEY GOES HERE") + assert _is_high("example-" + LIT) # strong token + a real one + assert _is_high("your-key-" + LIT) + assert _is_high("placeholder" + LIT) + assert _is_high("YOUR KEY GOES HERE 12345 " + LIT) + # CSO N5: the floor applies only when a non-filler token is present — an all- + # filler value stays a placeholder however long or digit-laden it is. + assert _is_low("your-api-key-goes-here-12345") + assert _is_low("YOUR_API_KEY_1234567890") + assert _is_low("your-github-token") + assert _is_low("YOUR KEY GOES HERE") def test_f3_only_a_closed_set_of_scheme_words_may_sit_beside_a_reference(): @@ -239,47 +258,60 @@ def test_f3_only_a_closed_set_of_scheme_words_may_sit_beside_a_reference(): assert _is_high("correcthorse ") for scheme in ("Bearer", "Token", "Basic", "ApiKey", "Negotiate", "Digest", "bearer"): assert not _auth(scheme + " ${TOKEN}"), scheme - assert "WRD-AUTH-PLACEHOLDER-SECRET" in _auth(scheme + " "), scheme + assert _is_low(scheme + " "), scheme -def test_f4_default_credentials_are_never_downgraded(): - # CSO F4: `admin` / `letmein` are working secrets, not template slots. - for cred in ("changeit", "raspberry", "postgres", "grafana", "elastic", "letmein", - "guest", "toor", "admin", "password", "root", "secret", "test"): +def test_f4_short_bare_words_are_placeholders_only_from_a_closed_allowlist(): + # CSO F4/N4: the downgrade is an allowlist of scheme/type slots. Everything else + # — default and dictionary passwords included — is a working secret. + for cred in ("changeit", "raspberry", "postgres", "grafana", "elastic", "letmein", "guest", + "toor", "admin", "password", "root", "secret", "test", "qwerty", "welcome", + "dragon", "iloveyou", "hunter", "monkey"): assert _is_high(cred), cred - assert "WRD-AUTH-PLACEHOLDER-SECRET" in _auth("changeme") + for slot in ("basic", "Bearer", "token", "apikey", "api-key", "api_key", "digest", "negotiate", "oauth", "none"): + assert _is_low(slot), slot + assert _is_low("changeme") # strong placeholder token, not the allowlist def test_f5_locators_need_two_segments_and_no_token_shaped_segment(): - # CSO F5: a URI/path that CARRIES the secret is not a locator. - for lit in ("op://" + REAL, "vault://" + REAL, "op://a/" + REAL, "~/" + REAL, "~/.config/" + REAL, - "/etc/" + REAL, "/a/b/" + REAL, "./" + REAL, "C:\\" + REAL, "C:\\Users\\" + REAL, - "/9j/4AAQSkZJRgABAQAAAQABAAD", "/abc+def=="): + # CSO F5/N3: a URI/path that CARRIES the secret is not a locator. Token-shaped + # is case-blind: 20+ alphanumerics, or 16+ at >= 3.5 bits/char. + for lit in ("op://" + LIT, "vault://" + LIT, "~/" + LIT, "/etc/" + LIT24, "./" + LIT, "C:\\" + LIT, + "op://a/" + LIT24, "~/.config/" + LIT24, "/a/b/" + LIT24, "C:\\Users\\" + LIT24, + "~/.config/GHSAT0AAAAAABCDEFGHIJ", "/9j/4AAQSkZJRgABAQAAAQABAAD", "/abc+def=="): assert _is_high(lit), lit for loc in ("op://Private/GitHub/token", "vault://secret/data/app#key", "~/.config/app/keys.json", - "/etc/app/secrets/token", "./secrets/token.txt", "../keys/app.json", "C:\\Users\\me\\keys.json"): + "/etc/app/secrets/token", "./secrets/token.txt", "../keys/app.json", "C:\\Users\\me\\keys.json", + "~/.config/gcloud/application_default_credentials.json"): assert not _auth(loc), loc def test_f6_ellipsis_is_anchored(): - # CSO F6: `...` marks a placeholder only as the whole value or its tail. - assert "WRD-AUTH-PLACEHOLDER-SECRET" in _auth("sk-...") - assert "WRD-AUTH-PLACEHOLDER-SECRET" in _auth("your-key...") - assert "WRD-AUTH-PLACEHOLDER-SECRET" in _auth("...") - assert _is_high("abc..." + REAL) - assert _is_high(REAL + "...x") + # CSO F6: `...` marks a placeholder only as the whole value or the tail of a + # stub / all-filler value — never a substring, never behind a real token. + assert _is_low("sk-...") + assert _is_low("your-key...") + assert _is_low("...") + assert _is_high("abc..." + LIT) + assert _is_high(LIT24 + "...") + assert _is_high(LIT + "...x") def test_negative_twin_every_benign_prefix_plus_a_real_token_is_still_high(): - # For each shape the audit deliberately excuses, the same shape carrying a real - # token must still be a committed credential. + # For each shape the audit deliberately excuses, the same shape carrying a + # sub-threshold literal must still be a committed credential. for prefix, suffix in ( ("Bearer ", ""), ("Token ", ""), ("Basic ", ""), ("ApiKey ", ""), ("Negotiate ", ""), ("Digest ", ""), - ("${TOKEN:-", "}"), (" ", ""), ("op://Private/GitHub/", ""), ("~/.config/app/", ""), - ("your-key-", ""), ("changeme-", ""), ("example/", ""), ("", "..."), ("", ""), + ("${TOKEN:-", "}"), ("${T:-${U:-", "}}"), (" ", ""), ("op://Private/GitHub/", ""), + ("~/.config/app/", ""), ("your-key-", ""), ("changeme-", ""), ("example/", ""), ("", "..."), + ("", ""), ): - value = prefix + REAL + suffix + value = prefix + LIT24 + suffix assert _is_high(value), value + # Entropy-path control: the same shapes with a >= 4.0 bits/char token are caught + # by checks_secret before the placeholder logic runs. + assert _is_high("op://Private/GitHub/" + REAL) + assert "WRD-SEC-ENTROPY" in _auth("your-key-" + REAL) def test_bearer_prefixed_reference_is_not_a_literal(): From 2e57352a26fcacf9b81f1d35d72776ad419a3fa6 Mon Sep 17 00:00:00 2001 From: DSE Builder Date: Fri, 4 Sep 2026 22:09:43 +0000 Subject: [PATCH 4/4] fix(auth-audit): fold in CSO final-pass notes on #104 (NF-1, NF-3, coverage) - NF-1 a 16+ char mixed-case alphanumeric segment is token-shaped even below 3.5 bits/char: /etc/Passw0rdPassw0rd is a literal. - NF-3 a URI fragment and a ${VAR:?msg} message are checked for token-shaped runs: op://a/b#<24hex> and ${API_KEY:?<24hex>} are literals; #key and :?required stay references. - Coverage: ~/.config/9f8e7d6c5b4a3e2d (entropy branch), a 16-char low-entropy directory name (locator), correcthorsebatterystaple... (F6 tail rule). - docs/AGENT_GATES.md: hyphens are file-name separators too; NF-2 (a token containing a separator) recorded as a known residual, tracked as a follow-up. --- docs/AGENT_GATES.md | 12 +++++++++--- src/mcp_warden/auth_audit.py | 35 +++++++++++++++++++++++++++-------- tests/test_auth_audit.py | 17 ++++++++++++++--- 3 files changed, 50 insertions(+), 14 deletions(-) diff --git a/docs/AGENT_GATES.md b/docs/AGENT_GATES.md index bef053b..2a97384 100644 --- a/docs/AGENT_GATES.md +++ b/docs/AGENT_GATES.md @@ -171,11 +171,17 @@ follows, each bound pinned by a test with a sub-threshold bypass string: `${TOKEN:-9f8e7d6c5b4a}` and `${T:-${U:-hunter2!}}` are committed credentials wearing a reference; `${VAR:?msg}` stays a reference. - **Locators need >= 2 segments and no token-shaped segment.** Token-shaped is - case-blind: an alphanumeric-only segment of 20+ characters, or 16+ at - >= 3.5 bits/char. `op://Private/GitHub/token`, `~/.config/app/keys.json` and + an alphanumeric-only segment that is 20+ characters, or 16+ at >= 3.5 bits/char, + or 16+ mixed-case (`Passw0rdPassw0rd`). Segments with dots, underscores or + hyphens are file names. A URI fragment or a `${VAR:?msg}` message is checked + the same way: `#key` and `:?required` are fine, `#9f8e…` and `:?9f8e…` are + literals. `op://Private/GitHub/token`, `~/.config/app/keys.json` and `~/.config/gcloud/application_default_credentials.json` are references; `op://9f8e…`, `~/9f8e…`, `~/.config/9f8e7d6c5b4a3e2d1c0b9a8f`, - `~/.config/GHSAT0AAAAAABCDEFGHIJ` and `/9j/4AAQ…` are literals. + `~/.config/GHSAT0AAAAAABCDEFGHIJ`, `/etc/Passw0rdPassw0rd` and `/9j/4AAQ…` are + literals. **Known residual:** a token that itself contains a separator + (`9f8e-7d6c-5b4a-…`) reads as a file name and is not caught by this rule — + tracked as a follow-up. - **Short bare words are an allowlist, not a heuristic.** Only `basic`, `bearer`, `token`, `apikey`/`api-key`, `digest`, `negotiate`, `oauth`, `none` are treated as scheme/type slots. Everything else — `admin`, `password`, `letmein`, diff --git a/src/mcp_warden/auth_audit.py b/src/mcp_warden/auth_audit.py index 1cc2ce7..c271543 100644 --- a/src/mcp_warden/auth_audit.py +++ b/src/mcp_warden/auth_audit.py @@ -83,22 +83,36 @@ def _safe_url(url: str) -> str: #: ``${T:-${U:-hunter2!}}`` bottoms out on a literal), or itself a placeholder #: (CSO F1/N1/N2). ``${VAR:?msg}`` names an error message, never a value. _REF_WITH_DEFAULT = re.compile(r"\$\{[A-Za-z_][A-Za-z0-9_]*:?[-+=]((?:[^{}]|\$\{[^{}]*\})*)\}") - -#: A path/URI segment that is itself token-shaped (CSO F5/N3): alphanumeric only, -#: case-blind, and either 20+ characters or 16+ at >= 3.5 bits/char — a lowercase -#: hex key or a GHSAT-style token is carried, not pointed at. Segments with dots -#: or underscores (``application_default_credentials.json``) are file names. +#: ``${VAR:?msg}``: the message is an error string, but a token-shaped run inside it +#: (``${API_KEY:?9f8e…}``) is a secret parked where the scanner looks away (NF-3). +_REF_WITH_MESSAGE = re.compile(r"\$\{[A-Za-z_][A-Za-z0-9_]*:?\?((?:[^{}]|\$\{[^{}]*\})*)\}") + +#: A path/URI segment that is itself token-shaped (CSO F5/N3/NF-1): alphanumeric +#: only and any of — 20+ characters; 16+ at >= 3.5 bits/char; 16+ mixed-case +#: (``Passw0rdPassw0rd`` is low-entropy but no directory is named that). A +#: lowercase hex key or a GHSAT-style token is carried, not pointed at. Segments +#: with dots, underscores or hyphens (``application_default_credentials.json``, +#: ``my-app``) are file names. Known residual (NF-2): a token that itself contains +#: a separator (``9f8e-7d6c-…``) is not caught here — tracked as a follow-up. _ALNUM_SEGMENT = re.compile(r"[A-Za-z0-9]+") +_ALNUM_RUN = re.compile(r"[A-Za-z0-9]+") _PATH_SEGMENT = re.compile(r"[A-Za-z0-9._~-]+") _TOKENISH_MIN_LEN, _TOKENISH_LONG, _TOKENISH_ENTROPY = 16, 20, 3.5 def _tokenish_segment(seg: str) -> bool: - if not _ALNUM_SEGMENT.fullmatch(seg): + if not _ALNUM_SEGMENT.fullmatch(seg) or len(seg) < _TOKENISH_MIN_LEN: return False if len(seg) >= _TOKENISH_LONG: return True - return len(seg) >= _TOKENISH_MIN_LEN and shannon_entropy(seg) >= _TOKENISH_ENTROPY + mixed_case = seg.lower() != seg and seg.upper() != seg + return mixed_case or shannon_entropy(seg) >= _TOKENISH_ENTROPY + + +def _text_carries_a_token(text: str) -> bool: + """Any alphanumeric run inside free text (a URI fragment, a ``:?`` message) that + is token-shaped (CSO NF-3): ``#9f8e…`` / ``${K:?9f8e…}`` carry the secret.""" + return any(_tokenish_segment(run) for run in _ALNUM_RUN.findall(text)) #: Secret-manager reference URIs — the value names where the secret lives rather @@ -124,7 +138,10 @@ def _is_secret_locator(v: str) -> bool: """A secret-manager URI or credential-file path: names where a secret lives.""" m = _SECRET_URI.match(v) if m: - return _segments_are_a_locator(m.group(1).split("#", 1)[0]) + path, _, fragment = m.group(1).partition("#") + if fragment and (not _PATH_SEGMENT.fullmatch(fragment) or _text_carries_a_token(fragment)): + return False # `#key` names a field; `#9f8e…` carries the secret + return _segments_are_a_locator(path) m = _PATH_PREFIX.match(v) return bool(m) and _segments_are_a_locator(v[m.end():]) @@ -224,6 +241,8 @@ def _looks_like_secret_ref(value: str) -> bool: d = default.strip() if d and not _looks_like_secret_ref(d) and not _looks_like_placeholder(d): return False + if any(_text_carries_a_token(msg) for msg in _REF_WITH_MESSAGE.findall(v)): + return False remainder = _SECRET_REF.sub(" ", v) return _only_scheme_words(remainder) diff --git a/tests/test_auth_audit.py b/tests/test_auth_audit.py index 5e5f1e6..4f92ea8 100644 --- a/tests/test_auth_audit.py +++ b/tests/test_auth_audit.py @@ -232,8 +232,12 @@ def test_f1_shell_default_is_a_literal_unless_empty_reference_or_placeholder(): assert _is_high("${T:-${U:-" + LIT + "}}") assert _is_high("Bearer ${TOKEN:-hunter2!}") for ref in ("${TOKEN:-}", "${TOKEN:-${OTHER}}", "${TOKEN:-$OTHER}", "${TOKEN:?required}", - "${TOKEN?required}", "${T:-${U:-}}", "${T:-${U:-changeme}}"): + "${TOKEN?required}", "${T:-${U:-}}", "${T:-${U:-changeme}}", + "${API_KEY:?set API_KEY in your shell}"): assert not _auth(ref), ref + # CSO NF-3: a token parked in the `:?` message is a secret, not an error string. + assert _is_high("${API_KEY:?" + LIT24 + "}") + assert _is_high("${API_KEY:?Passw0rdPassw0rd}") assert not _auth("${TOKEN:-changeme}") # placeholder default -> still a reference @@ -278,11 +282,17 @@ def test_f5_locators_need_two_segments_and_no_token_shaped_segment(): # is case-blind: 20+ alphanumerics, or 16+ at >= 3.5 bits/char. for lit in ("op://" + LIT, "vault://" + LIT, "~/" + LIT, "/etc/" + LIT24, "./" + LIT, "C:\\" + LIT, "op://a/" + LIT24, "~/.config/" + LIT24, "/a/b/" + LIT24, "C:\\Users\\" + LIT24, - "~/.config/GHSAT0AAAAAABCDEFGHIJ", "/9j/4AAQSkZJRgABAQAAAQABAAD", "/abc+def=="): + "~/.config/GHSAT0AAAAAABCDEFGHIJ", "/9j/4AAQSkZJRgABAQAAAQABAAD", "/abc+def==", + "/etc/Passw0rdPassw0rd", # NF-1: 16 mixed-case, low entropy + "~/.config/9f8e7d6c5b4a3e2d", # entropy branch: 16 hex, H ~ 3.75 + "op://a/b#" + LIT24, # NF-3: token in the URI fragment + "vault://secret/data/app#Passw0rdPassw0rd"): assert _is_high(lit), lit for loc in ("op://Private/GitHub/token", "vault://secret/data/app#key", "~/.config/app/keys.json", "/etc/app/secrets/token", "./secrets/token.txt", "../keys/app.json", "C:\\Users\\me\\keys.json", - "~/.config/gcloud/application_default_credentials.json"): + "~/.config/gcloud/application_default_credentials.json", + "~/.config/aaaabbbbccccdddd/keys.json", # 16-char low-entropy dir name + "~/.config/my-app/keys.json", "op://Private/GitHub/token#credential"): assert not _auth(loc), loc @@ -295,6 +305,7 @@ def test_f6_ellipsis_is_anchored(): assert _is_high("abc..." + LIT) assert _is_high(LIT24 + "...") assert _is_high(LIT + "...x") + assert _is_high("correcthorsebatterystaple...") # tail rule: not a stub, not filler def test_negative_twin_every_benign_prefix_plus_a_real_token_is_still_high():