fix(sops-audit): allowlist non-secret values, and skip TOML-encrypted files - #75
Merged
Conversation
… files Closes #74. The plaintext-secret scan matched any 8+ character value after a secret-ish key, with no notion of what a value MEANS. Ordinary configuration tripped it: `update_password: on_create` matches on the `password` substring, and `on_create` is nine characters. That is the documented API of ansible.builtin.user, so a consumer cannot reword their way out of it, and their only lever was plaintext-scan-exclude on a whole file — disarming real detection across a file that manages credentials to silence one line. Two classes are now suppressed, both cases where the value CANNOT be a secret: 1. Indirection — the value names something rather than being it. Jinja/Go templates, shell and env expansion, Ansible vault refs, lookups, and SOPS ENC[...] ciphertext. 2. Config enums — a closed list of setting values: on_create, always, never, true/false, enabled/disabled, and similar. Deliberately NOT allowlisted: changeme, placeholder, password, secret. Those are plausible real bad values and keep failing. The allowlist only ever suppresses; it never widens what counts as a secret. Also fixes the SOPS skip missing TOML. The regex covered the YAML, JSON and dotenv metadata markers but not `[sops]`, so an encrypted .toml was scanned rather than skipped even though .toml is in the scanned extension list. No false positive today only because ENC[...] breaks the value character class. tests/plaintext-scan.test.sh, wired into self-test.yml. 21 fixtures, and the important half are real secrets that must STILL be caught — an AWS key with slashes, a base64 private key, a github token, and changeme. This matters more than usual. The workflow reaches every consumer repo through the floating @v2 tag, and an allowlist can only make the scan LESS sensitive, which is the direction that fails silently: a scanner that stops detecting reports success. So the test extracts KEY/SEP/Q/ENUMS from the workflow itself rather than restating them, because a drifted copy would prove nothing about what actually runs. Verified the guard bites: adding `changeme` to the enum list turns the suite red rather than quietly green. Note for whoever merges: @v2 is a floating tag, so re-pointing it publishes this to every consumer at once. Worth landing deliberately.
nkg
added a commit
that referenced
this pull request
Sep 1, 2026
PR #75 landed the sops-audit allowlist, the TOML skip fix and the plaintext-scan fixtures without a changelog entry; write it now as the 3.2.1 section so release.yml has notes to publish. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QHRYacEb7ABoYsNn7X71VY
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
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.
Closes #74.
The problem
The plaintext-secret scan matches any 8+ character value after a secret-ish
key, with no notion of what a value means. Ordinary configuration trips it:
passwordmatches as a substring ofupdate_password, andon_createisnine characters. That is the documented API of
ansible.builtin.user— aconsumer cannot reword their way out of it, and their only lever was
plaintext-scan-excludeon a whole file, disarming real detection across afile that manages credentials to silence one line.
Found the hard way in
HordiaLabs/platform, wheremainwas red for hourson a comment containing that pair.
The fix
Two classes are suppressed, both cases where the value cannot be a secret:
{{ vault_pw }},${API_KEY},$TOKEN,!vault,lookup(...),ENC[...]on_create,always,never,true/false,enabled/disabledDeliberately not allowlisted:
changeme,placeholder,password,secret. Those are plausible real bad values and keep failing. The allowlistonly ever suppresses — it never widens what counts as a secret.
Also: TOML-encrypted files were being scanned
The SOPS skip covered the YAML, JSON and dotenv markers but not
[sops], soan encrypted
.tomlwas scanned rather than skipped — even though.tomlisin the scanned extension list. Verified against all four forms; only the
plain
.tomlis now scanned.No false positive today, because
ENC[AES256_GCM,...]breaks the valuecharacter class. Fragile rather than broken, which is worth fixing before it
becomes the latter.
Tests
tests/plaintext-scan.test.sh, wired intoself-test.yml. 21 fixtures,and the important half are real secrets that must still be caught:
This matters more than usual here. An allowlist can only make a scanner
less sensitive, and that is the direction that fails silently — a scanner
that stops detecting reports success. So:
KEY/SEP/Q/ENUMSfrom the workflow itselfrather than restating them. A copy would drift, and a drifted test proves
nothing about what actually runs.
changemeto the enum list turns thesuite red, not quietly green.
For whoever merges
@v2is a floating tag, so re-pointing it publishes this to every consumerrepo at once. Worth landing deliberately rather than on autopilot — that is
why this is a PR and not a push.
🤖 Generated with Claude Code
https://claude.ai/code/session_017EDkXRcCrVDfu2e8V9Yy52