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/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: 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" diff --git a/tests/tooling_manifest.txt b/tests/tooling_manifest.txt index 57e2d31e..e0359637 100644 --- a/tests/tooling_manifest.txt +++ b/tests/tooling_manifest.txt @@ -77,6 +77,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