From 970438e05c5f1211426766e239c92b7094eb0d0d Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Sun, 23 Aug 2026 08:27:28 -0500 Subject: [PATCH 1/4] feat(security): detect a credential defaulted into a shell fallback (BACKLOG #1091) The two required secret gates share a blind spot over most of the places a credential would actually live. bandit is a Python AST scanner and does not parse .yaml or .ps1 at all. gitleaks' operative generic rule is ENTROPY-GATED, so `changeme`, a dev default or a short site code falls below the threshold and is never reported. SEVERITY, STATED PRECISELY: this is NOT a live exposure. Nothing is deployed and the tracked values are development defaults. The defect is that the CONTROL CANNOT SEE THE CLASS, so the next credential -- added by someone trusting the gate -- lands the same way. THE ENV INDIRECTION IS NOT THE DEFECT, THE FALLBACK IS. `${MEFOR_STORE_PASSWORD}` is correct usage and must never match. `${MEFOR_STORE_PASSWORD:-mefor-dev-password}` ships that literal the moment the variable is unset, which on a first deployment is exactly what happens. The rule keys on that shape and is deliberately entropy-free. MEASURED IN BOTH DIRECTIONS against gitleaks 8.30.1: FIRES docker/compose.yaml:158, the tracked demonstration FIRES a fresh ${REAL_SECRET:-changeme}, in .yaml and in .ps1 SILENT ${VAR} with no fallback -- correct usage SILENT .github/workflows/ -- NINE `id-token: write` lines live there. Those are GitHub PERMISSIONS, not credentials, and a naive `token\s*[:=]` rule reddens a REQUIRED gate nine times on its first run, which is how a control gets disabled not fixed. DIFFERENTIALLY VERIFIED: against the real tree this adds EXACTLY ONE finding. The repo config alone already reports one pre-existing hit elsewhere that is not this rule's. THE TESTS RUN THE SCANNER RATHER THAN READING THE CONFIG. A text assertion that a rule is PRESENT cannot tell a working rule from a regex that matches nothing -- the same "control that cannot fire" defect BACKLOG #1313 found in the sdist leak gate. Each test builds a fixture tree and runs real gitleaks against the real config, and a config gitleaks REFUSES to load fails the test loudly instead of reading as "no findings". THE ALLOWLIST TEST CAUGHT A REAL DEFECT BEFORE IT SHIPPED. gitleaks reports the FIRST CAPTURE GROUP as `Secret` and matches allowlist regexes against it. With the key name captured, every finding reported `Secret: "password"` -- useless in a report, and it silently broke the allowlist, since no placeholder regex can match the literal word "password". The key-name group is now non-capturing and `secretGroup` names the default value, so a finding reports `mefor-dev-password`. AND THE RULE BLOCKED THE COMMIT THAT INTRODUCED IT. The pre-commit hook fired three times on the test fixtures, which ARE the detected shape by design -- the same class as BACKLOG #1086, where rule 3c refuses the commit documenting it. Resolved with the NARROWEST exemption that works: a `paths` entry scoped to THIS rule and THIS file, so the file stays covered by every other rule and a real high-entropy credential dropped into it is still caught. Widening the pattern to miss the fixtures would have bought the same convenience by making the rule worse. Verified narrow: the rule still fires on docker/compose.yaml:158. Two traps measured on the way: `[[rules.allowlist]]` builds an ARRAY and gitleaks 8.30 refuses the whole config with "expected a map, got 'slice'" -- loudly, which is correct for a scanner. And `--no-git --source ` scans the DIRECTORY, not the tracked tree; a first run attributed ten .venv third-party findings to this change until the baseline was run separately. --- .gitleaks.toml | 57 +++++++++++ tests/test_gitleaks_credential_rule.py | 128 +++++++++++++++++++++++++ 2 files changed, 185 insertions(+) create mode 100644 tests/test_gitleaks_credential_rule.py diff --git a/.gitleaks.toml b/.gitleaks.toml index 08cf3435..a43ea415 100644 --- a/.gitleaks.toml +++ b/.gitleaks.toml @@ -53,3 +53,60 @@ regexes = [ # the startup error (#1183). Never a real credential; it is chosen to FAIL int() on purpose. '''pw-C4st_Val-66''', ] + +# --- BACKLOG #1091 -- a credential-named key given a hard-coded literal --------------------------- +# +# WHY A RULE AND NOT A THRESHOLD. gitleaks' operative generic rule is ENTROPY-GATED, and the +# credentials that matter here sit below any useful threshold -- `changeme`, a dev default, a short +# site code. bandit cannot help: it is a Python AST scanner and does not parse .yaml or .ps1 at all. +# So the two required gates share a blind spot over most of the places a credential would actually +# live. Lowering the entropy floor would trade that blind spot for a flood of false positives in a +# REQUIRED gate, which is how a security control gets disabled rather than fixed. +# +# THE ENV INDIRECTION IS NOT THE DEFECT -- THE FALLBACK IS. `${MEFOR_STORE_PASSWORD}` is correct +# usage and must never match. `${MEFOR_STORE_PASSWORD:-mefor-dev-password}` SHIPS that literal the +# moment the variable is unset, which on a first deployment is exactly what happens. +# +# MEASURED BOTH DIRECTIONS against gitleaks 8.30.1 before this landed. Fires on docker/compose.yaml +# and on a fresh `${REAL_SECRET:-changeme}`. Silent on: `${VAR}` with no fallback; the documented +# REPLACE_or_omit placeholders; and .github/workflows/release.yml, which carries NINE `id-token: +# write` lines. That last one is the load-bearing negative -- those are GitHub PERMISSIONS, not +# credentials, and a naive `token\s*[:=]` rule reddens a required gate nine times on its first run. +# +# DIFFERENTIALLY VERIFIED: against the real tree this rule adds EXACTLY ONE finding. The repo config +# alone already reports one pre-existing hit in docs/testing/master-test-plan/ that is NOT this rule +# and NOT this change. +[[rules]] +id = "mefor-defaulted-credential" +description = "A credential-named key given a hard-coded literal via a shell default -- ${VAR:-literal}" +# THE KEY-NAME GROUP IS NON-CAPTURING, AND THAT IS NOT COSMETIC. gitleaks reports the FIRST CAPTURE +# GROUP as `Secret`, and allowlist regexes are matched against it. With the key name captured, every +# finding reported `Secret: "password"` -- useless in a report, and it SILENTLY BROKE THE ALLOWLIST, +# because no placeholder regex can match the literal word "password". Caught by the allowlist test in +# tests/test_gitleaks_credential_rule.py, which is the only reason it was not shipped that way. +# `secretGroup` names the DEFAULT VALUE, which is the part a reader needs to see and the part an +# allowlist needs to test. +regex = '''(?i)(?:password|passwd|secret|token|api[_-]?key)[^\n:=]*[:=][^\n]*\$\{[A-Za-z_][A-Za-z0-9_]*:-([^}\s]+)\}''' +secretGroup = 1 +tags = ["credential", "default", "mefor"] + +# SINGLE bracket. `[[rules.allowlist]]` builds an ARRAY and gitleaks 8.30 refuses the config outright +# with "expected a map, got 'slice'" -- loudly, which is the right behaviour for a scanner and is how +# this was caught. +[rules.allowlist] +description = "Documented placeholders that exist to be replaced. NEVER allowlist a real credential." +# THE RULE'S OWN TEST FILE IS EXEMPT, AND THIS EXEMPTION IS THE NARROWEST ONE THAT WORKS. Its +# fixtures ARE the detected shape -- that is what they are for -- so on the first commit this rule +# blocked the commit that introduced it, exactly as BACKLOG #1086 describes rule 3c refusing the +# commit that documents it. Scoped to THIS rule and THIS path: the file stays covered by every other +# rule and by the top-level allowlist, so a real high-entropy credential dropped into it is still +# caught. A repository-wide exemption, or widening the pattern to miss the fixtures, would both have +# bought the same convenience by making the rule worse. +paths = ['''tests/test_gitleaks_credential_rule\.py'''] +regexes = [ + # Fill-me-in markers of the shape docker/k8s/secret.example.yaml uses. NOTE: that file's own + # placeholders carry no `${VAR:-...}` wrapper today, so this rule does not currently see them -- + # these entries cover the shape appearing INSIDE a default, which is the form that would ship. + '''REPLACE_or_omit''', + '''CHANGE_?ME_?BEFORE''', +] diff --git a/tests/test_gitleaks_credential_rule.py b/tests/test_gitleaks_credential_rule.py new file mode 100644 index 00000000..f604aa2b --- /dev/null +++ b/tests/test_gitleaks_credential_rule.py @@ -0,0 +1,128 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Guard the `mefor-defaulted-credential` gitleaks rule (BACKLOG #1091) by RUNNING it. + +The two required secret gates share a blind spot over most of the places a credential would actually +live: bandit is a Python AST scanner and does not parse `.yaml` or `.ps1` at all, and gitleaks' +operative generic rule is ENTROPY-GATED, so `changeme` or a short dev default falls below the +threshold. The rule added for #1091 keys on the SHAPE of a defaulted secret instead. + +WHY THESE TESTS EXECUTE THE SCANNER RATHER THAN READING THE CONFIG. A text assertion that the rule +is PRESENT cannot tell a working rule from a regex that matches nothing -- which is the same +"control that cannot fire" defect BACKLOG #1313 found in the sdist leak gate, where every text check +passed over a step that reported clean without inspecting anything. So each test builds a fixture +tree and runs the real `gitleaks` against the repository's real `.gitleaks.toml`. + +THE MUST-BE-SILENT CASES ARE NOT DECORATION. `.github/workflows/release.yml` carries NINE +`id-token: write` lines. Those are GitHub PERMISSIONS, not credentials, and a naive `token\\s*[:=]` +rule reddens a REQUIRED gate nine times on its first run -- which is how a security control gets +disabled rather than fixed. The negative rows are what keep the rule narrow enough to survive. +""" + +from __future__ import annotations + +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + +_REPO = Path(__file__).resolve().parents[1] +CONFIG = _REPO / ".gitleaks.toml" +RULE_ID = "mefor-defaulted-credential" + +pytestmark = pytest.mark.skipif( + shutil.which("gitleaks") is None, + reason="gitleaks is not on PATH; this guard needs the real scanner, not a text match", +) + + +def _findings(tmp_path: Path, files: dict[str, str]) -> list[dict]: + """Run the REAL gitleaks with the REPO's config over a fixture tree; return this rule's hits.""" + src = tmp_path / "src" + src.mkdir() + for name, body in files.items(): + (src / name).write_text(body, encoding="utf-8") + report = tmp_path / "report.json" + proc = subprocess.run( + [ + "gitleaks", + "detect", + "--no-git", + "--source", + str(src), + "--config", + str(CONFIG), + "--report-format", + "json", + "--report-path", + str(report), + ], + capture_output=True, + text=True, + timeout=120, + ) + # A config gitleaks refuses to load exits non-zero with FTL and writes no report. That must fail + # the test loudly rather than read as "no findings" -- the whole point of this module. + assert report.exists(), ( + f"gitleaks wrote no report; it likely rejected the config:\n{proc.stderr}" + ) + return [f for f in json.loads(report.read_text(encoding="utf-8")) if f["RuleID"] == RULE_ID] + + +#: The defect shape. The env indirection is fine; the FALLBACK is what ships on first deployment. +_DEFECT = ( + 'services:\n db:\n environment:\n DB_PASSWORD: "${REAL_SECRET:-mefor-dev-password}"\n' +) + + +def test_a_defaulted_credential_is_detected(tmp_path: Path) -> None: + """The positive control for this whole module. If this fails, every silence below means nothing.""" + hits = _findings(tmp_path, {"compose.yaml": _DEFECT}) + assert len(hits) == 1, f"expected the defaulted credential to be caught, got {hits}" + + +def test_it_catches_the_shape_in_a_powershell_file_too(tmp_path: Path) -> None: + """bandit cannot parse .ps1 at all, which is half of why this rule exists.""" + body = '$env:API_TOKEN = "${API_TOKEN:-changeme}"\n' + assert len(_findings(tmp_path, {"dev.ps1": body})) == 1 + + +def test_a_low_entropy_value_is_still_caught(tmp_path: Path) -> None: + """The entropy gate is the other half. `changeme` is below any useful threshold and is exactly + the credential a first deployment would carry.""" + body = 'db_password: "${DB_PASSWORD:-changeme}"\n' + assert len(_findings(tmp_path, {"values.yaml": body})) == 1 + + +def test_a_plain_env_reference_is_not_a_finding(tmp_path: Path) -> None: + """`${VAR}` with no fallback is CORRECT usage -- it is what the rule wants people to write.""" + body = 'services:\n db:\n environment:\n DB_PASSWORD: "${REAL_SECRET}"\n' + assert _findings(tmp_path, {"compose.yaml": body}) == [] + + +def test_github_permission_blocks_are_not_credentials(tmp_path: Path) -> None: + """THE LOAD-BEARING NEGATIVE. `id-token: write` is a GitHub permission, and release.yml carries + nine of them. A rule that matches these reddens a required gate on every run.""" + body = ( + "jobs:\n publish:\n permissions:\n" + " contents: write\n id-token: write # PyPI Trusted Publishing (OIDC)\n" + ) + assert _findings(tmp_path, {"release.yml": body}) == [] + + +def test_documented_placeholders_are_allowlisted(tmp_path: Path) -> None: + """The example manifest's fill-me-in markers exist to be replaced and are not secrets.""" + body = 'stringData:\n store-password: "${STORE_PASSWORD:-REPLACE_or_omit_for_sqlite}"\n' + assert _findings(tmp_path, {"secret.example.yaml": body}) == [] + + +def test_the_tracked_demonstration_is_caught(tmp_path: Path) -> None: + """docker/compose.yaml carries the shape today. If it is ever cleaned up this test should be + retired deliberately rather than left asserting over a file that no longer demonstrates it.""" + tracked = _REPO / "docker" / "compose.yaml" + if not tracked.exists(): # pragma: no cover - the file is tracked; this is a honest guard + pytest.skip("docker/compose.yaml is absent from this checkout") + hits = _findings(tmp_path, {"compose.yaml": tracked.read_text(encoding="utf-8")}) + assert len(hits) >= 1, "the tracked demonstration stopped being detected" From 99650504870392feb9e35cfff751deb13c443616 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Sun, 23 Aug 2026 08:55:21 -0500 Subject: [PATCH 2/4] fix(compose): the postgres password shipped a literal default the secret gates cannot see (BACKLOG #1091) The rule added in this branch fires on docker/compose.yaml, which is the rule working: `${MEFOR_STORE_PASSWORD:-mefor-dev-password}` ships its literal the moment the variable is unset, and neither required gate can see it -- bandit does not parse yaml, and gitleaks' generic rule is entropy-gated. Fixed rather than allowlisted. Exempting the only instance the rule finds would restore, for exactly that instance, the blindness the rule exists to remove -- and an allowlist entry written in the same commit as the rule is indistinguishable later from a rule that never worked. `:?` rather than a bare `${VAR}`: bare substitution passes an EMPTY password and postgres fails with a confusing error, while `:?` refuses to start and names what to supply. The error message names the MECHANISM, because the message this replaces did not. The line already on main read "override via the env_file / your secret store" for a service that has no env_file -- and compose substitution is not fed by env_file at all. env_file sets variables inside the container; `${VAR}` is resolved at parse time from the shell environment or a .env beside compose.yaml. A reader who followed the old comment would have done exactly what it said and seen no change. Measured with gitleaks 8.30.1 against this branch's own config, both directions: a `:-` fallback fixture still fires (the rule is intact), and compose.yaml no longer appears. The one remaining finding is a pre-existing generic-api-key hit in docs/testing/master-test-plan/09-engine-api.md, unrelated to this change. A local --no-git run reports it and CI's run did not, which is evidence for a git-mode difference and is NOT a measurement of one. Not a live exposure: zero deployments, the value was a dev default, the service is behind profiles: ["ha"], and the value is redacted in the scan output. AUTHORSHIP: the change is builder-1's, applied by the lander at their request because the worktree gate refused their branch switch and the branch was already checked out elsewhere. The BACKLOG #1091 claim was RELEASED BY ITS HOLDER for this commit and re-taken transiently by the lander to satisfy the claim gate, which requires the committer to hold it. It was not judged stale and -Force was declined. ADR 0165 establishes a paired commit authored by the Lander as a legitimate shape; this is that shape pointed the other way. Co-Authored-By: Claude Opus 5 --- docker/compose.yaml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/docker/compose.yaml b/docker/compose.yaml index 0351149a..a2c518c1 100644 --- a/docker/compose.yaml +++ b/docker/compose.yaml @@ -154,8 +154,12 @@ services: environment: POSTGRES_DB: messagefoundry POSTGRES_USER: mefor - # DEV password — override via the env_file / your secret store for anything real. - POSTGRES_PASSWORD: "${MEFOR_STORE_PASSWORD:-mefor-dev-password}" + # NO DEFAULT, DELIBERATELY (BACKLOG #1091). A `:-` fallback SHIPS its literal the moment the + # variable is unset, and the two required secret gates cannot see it -- bandit does not parse + # yaml, gitleaks' generic rule is entropy-gated. NOTE this is COMPOSE SUBSTITUTION, which is a + # different mechanism from the `env_file:` the other services use: env_file sets variables + # INSIDE the container and does not feed substitution at all. + POSTGRES_PASSWORD: "${MEFOR_STORE_PASSWORD:?required. Compose SUBSTITUTION reads the shell environment or a .env beside compose.yaml -- an env_file: does NOT feed it. Export it, or pass --env-file docker/secrets.env}" volumes: - mefor-pg:/var/lib/postgresql/data healthcheck: From b5a66abf00284a53dc1cde79b758aa80013c9dac Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Sun, 23 Aug 2026 10:05:42 -0500 Subject: [PATCH 3/4] test(tooling): register the gitleaks credential-rule test in the manifest (BACKLOG #1091) `test_every_non_engine_test_is_classified` refuses a `tests/*.py` that imports no engine module and appears in neither `tooling_manifest.txt` nor the explicit stay-list. `tests/test_gitleaks_credential_rule.py` is exactly that shape: it runs the real gitleaks binary against fixture trees and imports nothing from `messagefoundry`, so the manifest is where it belongs. Measured rather than assumed, both directions: manifest entry present 0 (control: an existing entry returns 1) engine imports in the file 0 (control: test_wiring.py returns 21) and the gate itself as the positive control -- without this line `test_every_non_engine_test_is_classified` fails with `assert not ['test_gitleaks_credential_rule.py']`; with it, it passes. It also satisfies the manifest header's own admission rule. The header says to LEAVE A TEST OFF when membership is ambiguous, because a test that reads real engine source must stay on the engine leg. This one reads no engine source and spawns a real child process per case, which is the cost profile the tooling tier exists to move off the engine legs. PLACED IN SORTED POSITION, which corrects something I told the fleet earlier. I had reported this list as "not sorted" after three insertion attempts landed in three different places on a sibling branch. Measured: 107 entries with 5 out of order, so it is sorted with local exceptions, and sorted insertion is the right rule. This entry sits between `test_gate_rule_scan_agreement.py` and `test_ide_licence_packaging.py`, leaving the out-of-order count unchanged at 5. Fourth branch to hit this gate (542, 544, #1040, this one), four authors. The registration is invisible to a content review, to ruff, to mypy, and to a targeted local run of the suite you actually changed -- it surfaces only when someone runs test_tooling_partition. Co-Authored-By: Claude Opus 5 --- tests/tooling_manifest.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/tooling_manifest.txt b/tests/tooling_manifest.txt index c9030de7..e95e290b 100644 --- a/tests/tooling_manifest.txt +++ b/tests/tooling_manifest.txt @@ -75,6 +75,7 @@ tests/test_freethread_smoke_liveness.py tests/test_gate_installed_parity.py tests/test_gate_liveness.py tests/test_gate_rule_scan_agreement.py +tests/test_gitleaks_credential_rule.py tests/test_ide_licence_packaging.py tests/test_incomplete_run_banner.py tests/test_install_gate_wiring.py From 33b291ea082f64f2439952a7335f4fb42297f9c2 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Sun, 23 Aug 2026 13:54:48 -0500 Subject: [PATCH 4/4] fix(security): move the defaulted-credential check out of gitleaks to the tip (BACKLOG #1091) PR 545 could never go green and it was not a tuning problem. `security.yml` runs gitleaks at `fetch-depth: 0`, and this rule's pattern matches 12 of 14 historical revisions of `docker/compose.yaml`. The tip was already fixed; the gate stayed red BY CONSTRUCTION. THE PLACEMENT WAS THE DEFECT, NOT THE RULE. A LEAKED SECRET and a DEFAULTED CREDENTIAL mean opposite things by history: a leaked value is compromised the moment it is pushed full history is CORRECT for it a defaulted credential is a property of the SHIPPED TIP fix the tip and it is gone So a tip-property lint inside a full-history secret scanner is a category error, and the permanently-red gate was that error reporting itself accurately. The rule moves to where tip properties are checked; the scanner keeps the job it is actually for. Every default gitleaks rule still scans full history, which is right for them, and a tombstone comment in .gitleaks.toml says so where the next person will look. THIS NEEDED NOTHING I COULD NOT RUN. The earlier candidate was a rule-scoped commit allowlist, which required a gitleaks 8.30.1 feature test that was DENIED, and I would not recommend on an unrun test. That constraint disappears once the rule is not a gitleaks rule -- I stopped needing the answer rather than routing around the denial. NOT A COVERAGE LOSS. The regex is carried over byte-identical, including the non-capturing key-name group whose absence once made every finding report `Secret: "password"` and silently broke the allowlist -- pinned now by its own test so the relocation cannot reintroduce it. CONTROLS BOTH WAYS, because a repo scan returning zero is indistinguishable from a pattern that matches nothing: 4 positive the pattern DOES match a defaulted credential 4 negative `${VAR}`, `${VAR:?msg}`, and `id-token: write` are NOT flagged 1 denominator the scan reads >500 tracked files, so the zero is over a real corpus `id-token: write` is the load-bearing negative: release.yml carries nine of them, they are GitHub PERMISSIONS rather than credentials, and a naive `token\s*[:=]` rule reddens a required gate nine times on its first run. MEASURED AFTER THE CHANGE: gitleaks still accepts the trimmed config, and reports the same ONE pre-existing finding in docs/testing/ that the original comment recorded as neither this rule nor this change. Unchanged, not merely unmeasured. MANIFEST ENTRY PLACED BY SORT ORDER, AND I GOT IT WRONG FIRST. My insertion landed at line 47 -- immediately before `tests/test_hook_prose_folding.py`, which is MY OWN misplaced #1040 entry from this morning. I landed next to the exception I created. Correct position is after `test_dast_claims.py`; the file's out-of-order count goes 8 -> 7. I did NOT move the #1040 entry: it is unrelated to this item and belongs in its own change. 20 passed. Co-Authored-By: Claude Opus 5 --- .gitleaks.toml | 63 ++------- tests/test_defaulted_credential_lint.py | 162 ++++++++++++++++++++++++ tests/test_gitleaks_credential_rule.py | 128 ------------------- tests/tooling_manifest.txt | 2 +- 4 files changed, 172 insertions(+), 183 deletions(-) create mode 100644 tests/test_defaulted_credential_lint.py delete mode 100644 tests/test_gitleaks_credential_rule.py diff --git a/.gitleaks.toml b/.gitleaks.toml index a43ea415..c4dab544 100644 --- a/.gitleaks.toml +++ b/.gitleaks.toml @@ -54,59 +54,14 @@ regexes = [ '''pw-C4st_Val-66''', ] -# --- BACKLOG #1091 -- a credential-named key given a hard-coded literal --------------------------- +# BACKLOG #1091's `mefor-defaulted-credential` RULE LIVED HERE AND WAS DELIBERATELY REMOVED, not +# abandoned. It is now a TIP-SCOPED check in tests/test_defaulted_credential_lint.py. # -# WHY A RULE AND NOT A THRESHOLD. gitleaks' operative generic rule is ENTROPY-GATED, and the -# credentials that matter here sit below any useful threshold -- `changeme`, a dev default, a short -# site code. bandit cannot help: it is a Python AST scanner and does not parse .yaml or .ps1 at all. -# So the two required gates share a blind spot over most of the places a credential would actually -# live. Lowering the entropy floor would trade that blind spot for a flood of false positives in a -# REQUIRED gate, which is how a security control gets disabled rather than fixed. +# THE PLACEMENT WAS THE DEFECT, NOT THE RULE. A LEAKED SECRET and a DEFAULTED CREDENTIAL mean +# opposite things by history: a leaked value is compromised the moment it is pushed, so scanning all +# history is CORRECT for it; a defaulted credential is a property of the SHIPPED ARTIFACT, so fixing +# the tip removes it. security.yml runs gitleaks at fetch-depth 0, and the pattern matched 12 of 14 +# historical revisions of docker/compose.yaml -- so a clean tip could never turn the gate green. That +# was the scanner doing its job on a rule that was not its business, not a tuning problem. # -# THE ENV INDIRECTION IS NOT THE DEFECT -- THE FALLBACK IS. `${MEFOR_STORE_PASSWORD}` is correct -# usage and must never match. `${MEFOR_STORE_PASSWORD:-mefor-dev-password}` SHIPS that literal the -# moment the variable is unset, which on a first deployment is exactly what happens. -# -# MEASURED BOTH DIRECTIONS against gitleaks 8.30.1 before this landed. Fires on docker/compose.yaml -# and on a fresh `${REAL_SECRET:-changeme}`. Silent on: `${VAR}` with no fallback; the documented -# REPLACE_or_omit placeholders; and .github/workflows/release.yml, which carries NINE `id-token: -# write` lines. That last one is the load-bearing negative -- those are GitHub PERMISSIONS, not -# credentials, and a naive `token\s*[:=]` rule reddens a required gate nine times on its first run. -# -# DIFFERENTIALLY VERIFIED: against the real tree this rule adds EXACTLY ONE finding. The repo config -# alone already reports one pre-existing hit in docs/testing/master-test-plan/ that is NOT this rule -# and NOT this change. -[[rules]] -id = "mefor-defaulted-credential" -description = "A credential-named key given a hard-coded literal via a shell default -- ${VAR:-literal}" -# THE KEY-NAME GROUP IS NON-CAPTURING, AND THAT IS NOT COSMETIC. gitleaks reports the FIRST CAPTURE -# GROUP as `Secret`, and allowlist regexes are matched against it. With the key name captured, every -# finding reported `Secret: "password"` -- useless in a report, and it SILENTLY BROKE THE ALLOWLIST, -# because no placeholder regex can match the literal word "password". Caught by the allowlist test in -# tests/test_gitleaks_credential_rule.py, which is the only reason it was not shipped that way. -# `secretGroup` names the DEFAULT VALUE, which is the part a reader needs to see and the part an -# allowlist needs to test. -regex = '''(?i)(?:password|passwd|secret|token|api[_-]?key)[^\n:=]*[:=][^\n]*\$\{[A-Za-z_][A-Za-z0-9_]*:-([^}\s]+)\}''' -secretGroup = 1 -tags = ["credential", "default", "mefor"] - -# SINGLE bracket. `[[rules.allowlist]]` builds an ARRAY and gitleaks 8.30 refuses the config outright -# with "expected a map, got 'slice'" -- loudly, which is the right behaviour for a scanner and is how -# this was caught. -[rules.allowlist] -description = "Documented placeholders that exist to be replaced. NEVER allowlist a real credential." -# THE RULE'S OWN TEST FILE IS EXEMPT, AND THIS EXEMPTION IS THE NARROWEST ONE THAT WORKS. Its -# fixtures ARE the detected shape -- that is what they are for -- so on the first commit this rule -# blocked the commit that introduced it, exactly as BACKLOG #1086 describes rule 3c refusing the -# commit that documents it. Scoped to THIS rule and THIS path: the file stays covered by every other -# rule and by the top-level allowlist, so a real high-entropy credential dropped into it is still -# caught. A repository-wide exemption, or widening the pattern to miss the fixtures, would both have -# bought the same convenience by making the rule worse. -paths = ['''tests/test_gitleaks_credential_rule\.py'''] -regexes = [ - # Fill-me-in markers of the shape docker/k8s/secret.example.yaml uses. NOTE: that file's own - # placeholders carry no `${VAR:-...}` wrapper today, so this rule does not currently see them -- - # these entries cover the shape appearing INSIDE a default, which is the form that would ship. - '''REPLACE_or_omit''', - '''CHANGE_?ME_?BEFORE''', -] +# DO NOT RE-ADD IT HERE. Every default rule below still scans full history, which is right for them. diff --git a/tests/test_defaulted_credential_lint.py b/tests/test_defaulted_credential_lint.py new file mode 100644 index 00000000..0816456a --- /dev/null +++ b/tests/test_defaulted_credential_lint.py @@ -0,0 +1,162 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""A credential-named key must not ship a hard-coded literal default (BACKLOG #1091). + +The shape is `POSTGRES_PASSWORD: "${MEFOR_STORE_PASSWORD:-changeme}"` -- a shell default that makes a +deployment come up with a working password nobody chose. Compose substitution reads the shell +environment or a `.env` beside `compose.yaml`; an `env_file:` does NOT feed it, so the fallback is +what actually runs. + +THIS WAS A GITLEAKS RULE AND THE PLACEMENT WAS THE DEFECT. A LEAKED SECRET and a DEFAULTED CREDENTIAL +mean opposite things by history: + + a leaked value is compromised the moment it is pushed -> scanning ALL history is correct + a defaulted credential is a property of the SHIPPED TIP -> fixing the tip removes it + +`security.yml` runs gitleaks at `fetch-depth: 0`, and the pattern matched 12 of 14 historical +revisions of `docker/compose.yaml`. A clean tip could therefore never turn that gate green -- not a +tuning problem, a category error, with the permanently-red gate reporting it correctly. So the check +moved to where tip-properties are checked and the scanner kept the job it is actually for. + +WHY A RULE AND NOT AN ENTROPY THRESHOLD, unchanged from the original finding: gitleaks' generic rule +is entropy-gated, and the credentials that matter here sit below any useful threshold. `changeme` is +the whole problem and it has almost no entropy. +""" + +from __future__ import annotations + +import re +import subprocess +from pathlib import Path + +import pytest + +_REPO = Path(__file__).resolve().parents[1] + +#: Byte-identical to the retired gitleaks rule's regex. The key-name group is NON-CAPTURING and the +#: DEFAULT VALUE is group 1: that is the part a reader needs to see and the part an allowlist tests. +#: Capturing the key name instead made every finding report the literal word "password". +_DEFAULTED_CREDENTIAL = re.compile( + r"(?i)(?:password|passwd|secret|token|api[_-]?key)[^\n:=]*[:=][^\n]*" + r"\$\{[A-Za-z_][A-Za-z0-9_]*:-([^}\s]+)\}" +) + +#: Documented fill-me-in markers. NEVER add a real credential here -- a real one must not exist in +#: the tree at all, which is what this check is for. +_PLACEHOLDERS = ( + re.compile(r"REPLACE_or_omit"), + re.compile(r"CHANGE_?ME_?BEFORE"), +) + +#: This file's own fixtures ARE the detected shape -- that is what they are for. The exemption is +#: scoped to this ONE path, exactly as the retired rule's was, so every other file stays covered. +_EXEMPT_PATHS = {"tests/test_defaulted_credential_lint.py"} + + +def _tracked_files() -> list[str]: + out = subprocess.run( + ["git", "-C", str(_REPO), "ls-files", "-z"], + capture_output=True, + text=True, + check=True, + timeout=120, + ).stdout + return [p for p in out.split("\0") if p] + + +def _findings() -> list[tuple[str, int, str]]: + """Every (path, line number, default value) in the TIP that ships a defaulted credential.""" + hits: list[tuple[str, int, str]] = [] + for rel in _tracked_files(): + if rel in _EXEMPT_PATHS: + continue + f = _REPO / rel + try: + text = f.read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError): + # Binary or unreadable. Not a silent skip: the count is asserted below so a tree that + # became entirely unreadable cannot pass as clean. + continue + for n, line in enumerate(text.splitlines(), 1): + m = _DEFAULTED_CREDENTIAL.search(line) + if not m: + continue + if any(p.search(line) for p in _PLACEHOLDERS): + continue + hits.append((rel, n, m.group(1))) + return hits + + +# --- the pattern itself, before it is pointed at the repo ---------------------- +# +# A repo scan that returns zero is indistinguishable from a pattern that matches nothing. These run +# first so the zero below is a RESULT rather than a broken instrument. + + +@pytest.mark.parametrize( + "line", + [ + pytest.param('POSTGRES_PASSWORD: "${MEFOR_STORE_PASSWORD:-changeme}"', id="compose-yaml"), + pytest.param("API_KEY=${SOME_KEY:-dev-key}", id="env-assignment"), + pytest.param(' secret: "${APP_SECRET:-s3cr3t}"', id="indented-yaml"), + pytest.param("api-key: ${K:-x}", id="hyphenated-key-name"), + ], +) +def test_the_pattern_MATCHES_a_defaulted_credential(line: str) -> None: + """POSITIVE CONTROL. Without these the repo scan proves nothing.""" + assert _DEFAULTED_CREDENTIAL.search(line), ( + "the pattern is inert; the repo scan below is vacuous" + ) + + +@pytest.mark.parametrize( + "line", + [ + pytest.param('POSTGRES_PASSWORD: "${MEFOR_STORE_PASSWORD}"', id="no-default-at-all"), + pytest.param( + 'POSTGRES_PASSWORD: "${MEFOR_STORE_PASSWORD:?required}"', id="required-not-defaulted" + ), + pytest.param(" id-token: write", id="github-permission-not-a-credential"), + pytest.param("contents: read", id="another-github-permission"), + ], +) +def test_the_pattern_IGNORES_shapes_that_are_not_defaulted_credentials(line: str) -> None: + """NEGATIVE CONTROLS, and `id-token: write` is the load-bearing one. + + `.github/workflows/release.yml` carries nine `id-token: write` lines. Those are GitHub + PERMISSIONS, not credentials, and a naive `token\\s*[:=]` rule reddens a required gate nine times + on its first run. `:?` is the OPPOSITE of the defect -- it fails closed with a message rather + than supplying a value -- so it must not be flagged either. + """ + assert not _DEFAULTED_CREDENTIAL.search(line) + + +def test_the_group_reported_is_the_DEFAULT_not_the_key_name() -> None: + """The retired gitleaks rule shipped this backwards once and it silently broke its allowlist: + with the key name captured, every finding reported `Secret: "password"`, which no placeholder + regex can match. Pinned here so the relocation does not reintroduce it.""" + m = _DEFAULTED_CREDENTIAL.search('POSTGRES_PASSWORD: "${MEFOR_STORE_PASSWORD:-changeme}"') + assert m and m.group(1) == "changeme" + + +# --- the tree ------------------------------------------------------------------ + + +def test_the_scan_actually_reads_a_meaningful_number_of_files() -> None: + """The scan's own denominator. A `git ls-files` that returned nothing, or a tree of unreadable + files, would make the assertion below pass over an empty set.""" + files = _tracked_files() + assert len(files) > 500, f"only {len(files)} tracked files; the scan is not seeing the repo" + readable = sum(1 for r in files if (_REPO / r).is_file()) + assert readable > 500, f"only {readable} readable; the scan would pass over almost nothing" + + +def test_no_tracked_file_ships_a_defaulted_credential() -> None: + hits = _findings() + rendered = "\n".join(f" {p}:{n} -> default {v!r}" for p, n, v in hits) + assert not hits, ( + "a credential-named key ships a hard-coded default, so a deployment comes up with a working " + "password nobody chose:\n" + rendered + "\n\n" + "Use `${VAR:?message}` so it fails closed with an explanation. Note an `env_file:` does NOT " + "feed compose substitution -- that reads the shell environment or a .env beside compose.yaml." + ) diff --git a/tests/test_gitleaks_credential_rule.py b/tests/test_gitleaks_credential_rule.py deleted file mode 100644 index f604aa2b..00000000 --- a/tests/test_gitleaks_credential_rule.py +++ /dev/null @@ -1,128 +0,0 @@ -# SPDX-License-Identifier: AGPL-3.0-or-later -# Copyright (C) 2026 MessageFoundry Organization and contributors -"""Guard the `mefor-defaulted-credential` gitleaks rule (BACKLOG #1091) by RUNNING it. - -The two required secret gates share a blind spot over most of the places a credential would actually -live: bandit is a Python AST scanner and does not parse `.yaml` or `.ps1` at all, and gitleaks' -operative generic rule is ENTROPY-GATED, so `changeme` or a short dev default falls below the -threshold. The rule added for #1091 keys on the SHAPE of a defaulted secret instead. - -WHY THESE TESTS EXECUTE THE SCANNER RATHER THAN READING THE CONFIG. A text assertion that the rule -is PRESENT cannot tell a working rule from a regex that matches nothing -- which is the same -"control that cannot fire" defect BACKLOG #1313 found in the sdist leak gate, where every text check -passed over a step that reported clean without inspecting anything. So each test builds a fixture -tree and runs the real `gitleaks` against the repository's real `.gitleaks.toml`. - -THE MUST-BE-SILENT CASES ARE NOT DECORATION. `.github/workflows/release.yml` carries NINE -`id-token: write` lines. Those are GitHub PERMISSIONS, not credentials, and a naive `token\\s*[:=]` -rule reddens a REQUIRED gate nine times on its first run -- which is how a security control gets -disabled rather than fixed. The negative rows are what keep the rule narrow enough to survive. -""" - -from __future__ import annotations - -import json -import shutil -import subprocess -from pathlib import Path - -import pytest - -_REPO = Path(__file__).resolve().parents[1] -CONFIG = _REPO / ".gitleaks.toml" -RULE_ID = "mefor-defaulted-credential" - -pytestmark = pytest.mark.skipif( - shutil.which("gitleaks") is None, - reason="gitleaks is not on PATH; this guard needs the real scanner, not a text match", -) - - -def _findings(tmp_path: Path, files: dict[str, str]) -> list[dict]: - """Run the REAL gitleaks with the REPO's config over a fixture tree; return this rule's hits.""" - src = tmp_path / "src" - src.mkdir() - for name, body in files.items(): - (src / name).write_text(body, encoding="utf-8") - report = tmp_path / "report.json" - proc = subprocess.run( - [ - "gitleaks", - "detect", - "--no-git", - "--source", - str(src), - "--config", - str(CONFIG), - "--report-format", - "json", - "--report-path", - str(report), - ], - capture_output=True, - text=True, - timeout=120, - ) - # A config gitleaks refuses to load exits non-zero with FTL and writes no report. That must fail - # the test loudly rather than read as "no findings" -- the whole point of this module. - assert report.exists(), ( - f"gitleaks wrote no report; it likely rejected the config:\n{proc.stderr}" - ) - return [f for f in json.loads(report.read_text(encoding="utf-8")) if f["RuleID"] == RULE_ID] - - -#: The defect shape. The env indirection is fine; the FALLBACK is what ships on first deployment. -_DEFECT = ( - 'services:\n db:\n environment:\n DB_PASSWORD: "${REAL_SECRET:-mefor-dev-password}"\n' -) - - -def test_a_defaulted_credential_is_detected(tmp_path: Path) -> None: - """The positive control for this whole module. If this fails, every silence below means nothing.""" - hits = _findings(tmp_path, {"compose.yaml": _DEFECT}) - assert len(hits) == 1, f"expected the defaulted credential to be caught, got {hits}" - - -def test_it_catches_the_shape_in_a_powershell_file_too(tmp_path: Path) -> None: - """bandit cannot parse .ps1 at all, which is half of why this rule exists.""" - body = '$env:API_TOKEN = "${API_TOKEN:-changeme}"\n' - assert len(_findings(tmp_path, {"dev.ps1": body})) == 1 - - -def test_a_low_entropy_value_is_still_caught(tmp_path: Path) -> None: - """The entropy gate is the other half. `changeme` is below any useful threshold and is exactly - the credential a first deployment would carry.""" - body = 'db_password: "${DB_PASSWORD:-changeme}"\n' - assert len(_findings(tmp_path, {"values.yaml": body})) == 1 - - -def test_a_plain_env_reference_is_not_a_finding(tmp_path: Path) -> None: - """`${VAR}` with no fallback is CORRECT usage -- it is what the rule wants people to write.""" - body = 'services:\n db:\n environment:\n DB_PASSWORD: "${REAL_SECRET}"\n' - assert _findings(tmp_path, {"compose.yaml": body}) == [] - - -def test_github_permission_blocks_are_not_credentials(tmp_path: Path) -> None: - """THE LOAD-BEARING NEGATIVE. `id-token: write` is a GitHub permission, and release.yml carries - nine of them. A rule that matches these reddens a required gate on every run.""" - body = ( - "jobs:\n publish:\n permissions:\n" - " contents: write\n id-token: write # PyPI Trusted Publishing (OIDC)\n" - ) - assert _findings(tmp_path, {"release.yml": body}) == [] - - -def test_documented_placeholders_are_allowlisted(tmp_path: Path) -> None: - """The example manifest's fill-me-in markers exist to be replaced and are not secrets.""" - body = 'stringData:\n store-password: "${STORE_PASSWORD:-REPLACE_or_omit_for_sqlite}"\n' - assert _findings(tmp_path, {"secret.example.yaml": body}) == [] - - -def test_the_tracked_demonstration_is_caught(tmp_path: Path) -> None: - """docker/compose.yaml carries the shape today. If it is ever cleaned up this test should be - retired deliberately rather than left asserting over a file that no longer demonstrates it.""" - tracked = _REPO / "docker" / "compose.yaml" - if not tracked.exists(): # pragma: no cover - the file is tracked; this is a honest guard - pytest.skip("docker/compose.yaml is absent from this checkout") - hits = _findings(tmp_path, {"compose.yaml": tracked.read_text(encoding="utf-8")}) - assert len(hits) >= 1, "the tracked demonstration stopped being detected" diff --git a/tests/tooling_manifest.txt b/tests/tooling_manifest.txt index 49bcf296..8c40b8fd 100644 --- a/tests/tooling_manifest.txt +++ b/tests/tooling_manifest.txt @@ -72,6 +72,7 @@ tests/test_coord_usage.py tests/test_cutover_slug_rot.py tests/test_dangling_citation_check.py tests/test_dast_claims.py +tests/test_defaulted_credential_lint.py tests/test_dep1_lock_resync_lockstep.py tests/test_dependabot_automerge_guardrails.py tests/test_docs_runbooks.py @@ -80,7 +81,6 @@ tests/test_freethread_smoke_liveness.py tests/test_gate_installed_parity.py tests/test_gate_liveness.py tests/test_gate_rule_scan_agreement.py -tests/test_gitleaks_credential_rule.py tests/test_ide_licence_packaging.py tests/test_incomplete_run_banner.py tests/test_install_gate_wiring.py