From 970438e05c5f1211426766e239c92b7094eb0d0d Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Sun, 23 Aug 2026 08:27:28 -0500 Subject: [PATCH 1/3] 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/3] 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/3] 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