fix(auth-audit): placeholder secrets, shell-default/Windows refs, secret-manager URIs — from a 463-config public scan - #104
Merged
Conversation
…ret-manager URIs — from a 463-config public scan
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 <slot> 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.
…(F1–F6)
- 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 <slot>; `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).
- 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.
…verage)
- 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Turns the orphaned auth-audit placeholder work (uncommitted on the laptop since the 463-config public scan) into a reviewed change, tightened so the downgrade path cannot hide a real credential.
Why
Running
mcp-warden auth auditover a 463-config public corpus showed 74 % ofWRD-AUTH-TOKEN-IN-CONFIGhits were template fill-me-ins (<your-api-key>,YOUR KEY GOES HERE,changeme,xxx) and several correct reference shapes were reported as committed credentials. False highs are what get a gate switched off.What
WRD-AUTH-PLACEHOLDER-SECRET(low) — a shipped config that cannot work is still a finding, but not a leak.${VAR:-default}/${VAR:?msg},%VAR%(Windows), secret-manager URIs (op://,vault://,awssm://,gcpsm://,azkv://,secretref://,keyring://,pass://), credential-file paths.docs/AGENT_GATES.mdrule table + a "Placeholders are a separate rule" section; CHANGELOG[Unreleased]; DOCUMENTATION_INDEX.Tightened during review (vs the original WIP)
here⊂adherence,fill⊂fillmore)<dropped in favour of a<slot>ruletest_placeholder_words_match_whole_tokens_not_substringslen < 12 and no digitswallowed short real secrets with punctuation^[a-z][a-z_-]{0,10}$— a digit or any other punctuation disqualifies (hunter2!,p@sswordstay high)test_short_real_secret_shapes_are_not_downgraded_to_placeholderghp_example…) would be downgradedWRD-SEC-*hit is a credential, never a placeholder, never a "reference"test_vendor_shaped_secret_is_never_downgraded_even_if_it_contains_placeholder_wordsBearer <token> aB3x…(slot beside a literal) counted as placeholdertest_angle_slot_beside_a_real_literal_is_still_a_credential_PATH_LIKEaccepted any value starting with/— a base64 blob (/9j/4AAQ…) vanished as a "path"test_base64_blob_starting_with_slash_is_not_a_credential_pathtest_placeholder_finding_is_redacted_like_every_other_snippetdoctorinherits all of this unchanged (doctor_funnelimports_is_auth_key/_looks_like_secret_ref); its 65 tests pass.Verification (dse-builder-01, py3.12)
ruff check .cleanpython= system 3.12 withoutmcp); re-run with the venv on PATH →test_precommit+test_signing52 passed / 1 skipped. CI'ssetup-pythonis unaffected.test_auth_audit+test_doctor*97 passed.Note:
src/mcp_warden/auth_audit.pyis 355 LOC (was 314) — over the 300-LOC guideline before this change; not split here to keep the diff reviewable.Security review remediation (CSO RED → fixes, second commit)
${VAR:-D}accepted any default as a reference${TOKEN:-aB3xK9mQ7pL2wR5tY8vN4jH6}_REF_WITH_DEFAULT: D must be empty, itself a reference, or itself a placeholder, else the value is a literal;${VAR:?msg}stays a referencetest_f1_shell_default_is_a_literal_unless_empty_reference_or_placeholderany())example-aB3xK9,your-key-<real>test_f2_placeholder_needs_every_token_to_be_filler_and_has_a_hard_floorcorrecthorse ${TOKEN}bearer|token|basic|apikey|negotiate|digest(case-insensitive) for both the reference and<slot>remaindertest_f3_only_a_closed_set_of_scheme_words_may_sit_beside_a_referenceadmin,letmein,changeit,postgres, …_DEFAULT_CREDENTIALSdeny-list subtracted from the downgrade (changemestays a strong placeholder)test_f4_default_credentials_are_never_downgraded~/,./,C:\branches accepted one segment;op://<token>was a "URI"~/<real>,op://<real>,/a/b/<real>test_f5_locators_need_two_segments_and_no_token_shaped_segment...matched as a substringabc...<real>test_f6_ellipsis_is_anchoreddocs/AGENT_GATES.mdnow states the true bound: vendor pattern or entropy (24+ chars at ≥ 4.0 bits/char, ≥ 80 % alnum) is guaranteed; shorter/low-entropy keys rely on the bounds aboveaB3xK9mQ7pL2wR5tY8vN4jH6test_negative_twin_every_benign_prefix_plus_a_real_token_is_still_highF7 (doctor absent) was a stale-checkout artefact —
doctoris on main and its 65 tests pass against this branch.Verification after remediation (builder, venv on PATH): ruff clean · full suite with coverage 1109 passed / 2 skipped / 0 failed, TOTAL 88 % · targeted
test_auth_audit+test_doctor*104 passed.auth_audit.pyis now 393 LOC — a split intoauth_refs.pyis the obvious follow-up.Re-verification remediation (CSO pass 3, N1–N5)
${T:-${U:-hunter2!}},${T:-${U:-9f8e7d6c5b4a}}_looks_like_secret_refrecursively, then placeholdertest_f1_…:=not an expansion form${TOKEN:=hunter2!}_REF_WITH_DEFAULT/_SECRET_REFaccept:?[-+=]test_f1_…~/.config/9f8e7d6c5b4a3e2d1c0b9a8f,~/.config/GHSAT0AAAAAABCDEFGHIJshannon_entropy); dotted/underscored file names (application_default_credentials.json) stay locatorstest_f5_…qwerty,welcome,dragon,iloveyou{basic, bearer, token, apikey, api-key, digest, negotiate, oauth, none};_DEFAULT_CREDENTIALSdeletedtest_f4_short_bare_words_are_placeholders_only_from_a_closed_allowlistyour-api-key-goes-here-12345,YOUR_API_KEY_1234567890test_f2_…9f8e7d6c5b4a(12-hex) and9f8e7d6c5b4a3e2d1c0b9a8f(24-hex, H ≈ 3.92) so each fails on the pre-fix code; oneREALcase kept and labelled as the entropy-path controltest_negative_twin_…Must-pass confirmed:
${TOKEN:-9f8e7d6c5b4a}HIGH ·your-key-9f8e7d6c5b4aHIGH ·~/9f8e7d6c5b4aHIGH · the 7 benign corpus shapes still[].Verification after pass 3 (builder, venv on PATH): ruff clean · full suite with coverage 1119 passed / 2 skipped / 0 failed, TOTAL 88 % · targeted
test_auth_audit+test_doctor*114 passed.auth_audit.pyis 399 LOC.Final-pass fold-in (CSO GREEN notes NF-1 / NF-3 / coverage)
/etc/Passw0rdPassw0rd_tokenish_segment: 16+ mixed-case alphanumeric is token-shaped regardless of entropy (OR with the ≥ 20 / ≥ 3.5-bit clauses)test_f5_…:?messageop://a/b#9f8e7d6c5b4a3e2d1c0b9a8f,${API_KEY:?9f8e7d6c5b4a3e2d1c0b9a8f}_text_carries_a_token()over alphanumeric runs in the fragment and the:?message;#key,#credential,:?required,:?set API_KEY in your shellstay referencestest_f5_…,test_f1_…~/.config/9f8e7d6c5b4a3e2d(16 hex, H ≈ 3.75) → HIGHtest_f5_…~/.config/aaaabbbbccccdddd/keys.json,~/.config/my-app/keys.json→ locatortest_f5_…correcthorsebatterystaple...→ HIGHtest_f6_ellipsis_is_anchoredAGENT_GATES.md: hyphens are file-name separators; NF-2 recorded as a known residual (a token that itself contains a separator, e.g.9f8e-7d6c-…, reads as a file name) — tracked as a follow-upVerification after fold-in (builder, venv on PATH): ruff clean · full suite with coverage 1119 passed / 2 skipped / 0 failed, TOTAL 88 % · targeted
test_auth_audit+test_doctor*114 passed.🤖 Generated with Claude Code
https://claude.ai/code/session_01Lu4ZLyWWL7TXMGYpNFU5Dc