fix(security): move the defaulted-credential check out of gitleaks to a tip-scoped lint (BACKLOG #1091) - #555
Merged
Conversation
…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 <dir>` 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.
…ret 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 <noreply@anthropic.com>
…fest (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 <noreply@anthropic.com>
… 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 <noreply@anthropic.com>
wshallwshall
enabled auto-merge (squash)
August 23, 2026 20:42
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.
Supersedes PR 545, which carried the same branch and could not be reopened: I force-pushed the branch before attempting the reopen, and GitHub refuses to reopen a pull request whose closing head no longer exists. Reopening first would have worked. The order was mine and it is recorded here rather than quietly worked around.
I closed 545 earlier today and the close was right about the OLD approach. The approach has since changed, and the objection no longer applies.
Why it was closed
The check lived as a rule in
.gitleaks.toml, and the gitleaks gate scans full history. A rule that fires on a defaulted credential anywhere in the past cannot be cleared by fixing the tip. That was a category error in the design, not a tuning problem, so there was no version of "iterate on the config" that would ever go green.What changed
The check moved out of gitleaks entirely, to a tip-scoped lint:
tests/test_defaulted_credential_lint.py. It inspects the current tree, so it is answerable by fixing the current tree..gitleaks.tomlkeeps only the trimmed allowlist. The author reports gitleaks still accepts the config and returns the same one pre-existing finding underdocs/testing/that predates all of this, measured after the change rather than assumed. That measurement is theirs and I have not re-run it.The commits
970438e099650504b5a66abftests/tooling_manifest.txt33b291ea6ba61d2cf5c464e3Four layered commits on one item. The last is meaningless without the first, so this one is deliberately not split.
The manifest line is present at
tests/tooling_manifest.txt:75. The tooling-partition gate refuses atests/*.pythat imports no engine module and is absent from the manifest, and that gate cost PR 542 an afternoon. It is handled here.Landing notes
The branch was force-pushed, replacing a superseded merge commit (
cac18909) whose only unique content was a merge resolution. Measured before pushing: it was not an ancestor of the new tip, and the newer merge supersedes it.backlog-hygienedoes not fire on this branch. Itstouches_codetest matches onlymessagefoundry/,ide/, andmessagefoundry_webconsole/paths, and this PR touches.gitleaks.toml,docker/compose.yamlandtests/— verified by running the gate's own logic against the pushed tip, not inferred.Co-Authored-By: Claude Opus 5 noreply@anthropic.com