Repository navigation
fix(config): the built-in precommit profile no longer overrides what a config set - #796
Merged
Merged
Conversation
added 2 commits
October 20, 2026 20:07
…a config set
Under --pre-commit-mode, the four hook environment variables, or the
published hook's --profile precommit, the built-in precommit profile's
checks, confidence_levels, redaction.strategy and redaction.output_dir
replaced what the config wrote, on both input paths and for every config
source. A config naming PASSPORT got six other types, so a HIGH passport
passed the hook at exit 0, and strategy: simple came back as
format_preserving with the passport in cleartext. Only a config's own
profiles.precommit escaped, and a null profiles: wiped the built-in
profile while profiles: {} kept it. Separately, fail_on_incomplete was
missing from the profile bool registry, so every profile turned a
config's fail_on_incomplete: true off, and the suppression bools were
backfilled from defaults.* only.
internal/config: the built-in profile is added after the file is decoded,
only when the file defines no profiles.precommit (missing, null and {}
agree), and yields to the file: no redaction strategy or output_dir of
its own; high,medium only when the file lacks defaults.confidence_levels;
checks are the six plus every type defaults.checks names, so a config can
widen a hook and cannot narrow it; format stays text. fail_on_incomplete
is registered, and show_suppressed / generate_suppressions backfill from
defaults.X or the suppressions: key. The uncalled GetPrecommitProfile is
removed; BuiltinPrecommitChecks holds the six.
cmd: one stderr line names the remaining override when a config's
defaults.checks is all (withheld where --stdin stderr is the findings
document); --debug names the active profile, why it was selected and what
it supplied, on both input paths; --list-profiles reads the loaded
config, auto-discovered ones included, and marks the built-in profile.
Tests: reflection guards that fail on an unclassified Profile field or an
unregistered Profile bool, and a binary matrix of 6 routes x 2 input
paths x 5 config sources.
Closes #768
…768) No doc listed the built-in precommit profile's contents. docs/configuration.md presented examples/ferret.yaml's own profile (PASSPORT, PERSON_NAME) as the one a hook runs, docs/INSTALLATION.md showed a user profile as the built-in one, docs/PRE_COMMIT_INTEGRATION.md said the profile sets respect_gitignore (it does not), and the shipped config.yaml said the redaction flags have no config equivalent although its own redaction: block is honoured. The pre-commit and configuration guides now state the six checks, how the built-in profile yields to a config, the settings precedence, and that a profile inherits every key it leaves out, fail_on_incomplete included. TestDocsStateTheBuiltinPrecommitChecks holds both guides to config.BuiltinPrecommitChecks. Refs #768
sectoramen
added
bug
Contributor
🔍 Ferret Scan ResultsGate mode:
Findings Summary:
... and 5 more findings Next Steps:
🤖 This comment was automatically generated by Ferret Scan |
rustic
approved these changes
Oct 22, 2026
Contributor
🔍 Ferret Scan ResultsGate mode:
Findings Summary:
... and 5 more findings Next Steps:
🤖 This comment was automatically generated by Ferret Scan |
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
2 participants
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 #768. Independent of #771 and #766 (no file in common except
CHANGELOG.md, where each stream appends one bullet to### 🐛 Bug Fixes, anddocs/configuration.md, where this PR stays out of the--validator-budgetparagraph #771 edits). Builds on #791: the strategy source strings andapplyRedactionStrategyare reused unchanged.Summary
Under
--pre-commit-mode,PRE_COMMIT/_PRE_COMMIT_RUNNING/PRE_COMMIT_HOOK/GIT_HOOK_TYPE, or the published hook's--profile precommit, the built-inprecommitprofile'schecks,confidence_levels,redaction.strategyandredaction.output_dirreplaced what the config wrote. Onmain@46e2918: a config namingPASSPORTgot the profile's six types, so a HIGH passport passed the hook at rc 0 with 0 bytes of output, and--stdinwithstrategy: simplereturnedSSN: ***-**-0001/Passport number: X00000001. All 6 routes, both input paths and all 5 config sources reproduced (60 of 60 cells); only a config's ownprofiles.precommitescaped, andprofiles:null and{}disagreed. Separately, every profile that omittedfail_on_incomplete, the built-in one included, turned a config'sfail_on_incomplete: trueoff (rc 0 instead of 3 with an unreadable file), and--profile otherswitched offsuppressions.show_suppressed/suppressions.generate_on_scan. After this PR the built-in profile yields to the config, every profile bool inherits it, and the one override left (defaults.checks: allstill runs the six) is disclosed on one stderr line.What changed (the issue's Resolution 1–4)
1. The built-in profile yields (
internal/config/config.go)injectBuiltinPrecommitProfilereplaces the pre-decode injection. It runs afteryaml.Unmarshal, only when the file has noprofiles.precommit(missing,profiles: {}and a nullprofiles:now agree; null used to wipe it), and beforebackfillProfileBools, so the profile's bools come from the file exactly as for any profile that omits them.containsField:redaction.strategyandredaction.output_dirare no longer set (they equalled the global defaults);confidence_levels: high,mediumonly when the file lacksdefaults.confidence_levels;checks= the six uniondefaults.checks(upper-cased, deduplicated, unknown names kept so they are refused downstream as without a profile;all, any case, or empty adds nothing);formatstaystext(exempt). With no config file the profile is the one it always was.2. Disclose the remaining override (
cmd/active_profile.go, call sites incmd/main.go,cmd/stdin.go)reportBuiltinProfileChecks: when the built-in profile is active and the loaded file wrotedefaults.checksas every check, one stderr line:Note: the built-in precommit profile runs 6 checks (CREDIT_CARD, SECRETS, SSN, EMAIL, PHONE, IP_ADDRESS), not the "all" that defaults.checks sets in <config>; set profiles.precommit.checks in that file to choose what pre-commit runs.Ungated likereportDisabledDetectionTypes/reportConfigProvenance(printed beside the provenance note on both paths); withheld only where--stdinstderr is the findings document; silent without a config file, withoutdefaults.checks, under a profile the file defines, and when--checkschose the checks.--debugrecord on both input paths (profileDebugRecord): the active profile, built-in ordefined in <config>, why it was selected (--profile,--pre-commit-mode, or a detected hook environment), and what it supplied (format=text; confidence_levels=high,medium; checks=...for the built-in profile; the keys its YAML block sets for a file-defined one, viaConfig.ProfileKeys).3. Every profile bool is registered (
profileBoolFields)fail_on_incompleteadded.show_suppressed/generate_suppressionsbackfill fromdefaults.X || suppressions.Y, the same ORresolveConfigurationapplies with no profile. Each entry gained agetProfileaccessor (used by the debug record; pinned against the reflected field).4. One definition, one listing
Config.GetPrecommitProfile(uncalled duplicate) is deleted;config.BuiltinPrecommitChecksis the one exported 6-type constant.Config.BuiltinProfile(name)reports whether a profile is built-in, what it supplied and whether the file'sdefaults.checkswasall.--list-profiles(writeProfileList) reads the loaded config, auto-discovered ones included (it tested the--configflag and printedNo configuration file foundfor a discovered file), and marksprecommit (built-in).Docs (Docs & CHANGELOG list):
docs/PRE_COMMIT_INTEGRATION.mdgains "The precommit profile" (contents, how it yields, norespect_gitignore,--list-profiles/--debug) and therespect_gitignoreclaim at the old:785is corrected;docs/configuration.mdgains the settings precedence and "The built-inprecommitprofile", says profiles inheritfail_on_incomplete, and marks theprecommitentry of "Pre-Configured Profiles" asexamples/ferret.yaml's own profile;docs/INSTALLATION.md:325no longer presents a user profile as the built-in one (and dropsbatch_size: 50, a key profiles do not have; main warnsunknown config key "batch_size");config.yaml:5-7no longer says the redaction flags are CLI-only;examples/ferret.yamlnotes that itsprecommitreplaces the built-in profile.cmd/discovery_scope_test.go: comment only (format is the one value the built-in profile keeps). CHANGELOG: one bullet at the end of### 🐛 Bug Fixeswith the behaviour-change note.Deviations from the issue, and why
cmd/, nottests/integration, beside the other CLI contract tests and on their helpers (buildScanner,isolatedEnv,runCLI,precommitTriggers); the file-ownership split for the parallel streams put new contract tests there.defaults.checkstoo, sinceresolveConfigurationand the--checksparser both read""as every check.allmatching is case-insensitive, as the parser's is.--debugalso names a file-defined profile (and an undefined--profileon--stdin, which that path silently ignores today) rather than only the built-in one; the issue's "own-precommitruns byte-identical outside--debug" implies the record covers them.--list-profileswithout any config file lists the built-in profile (No configuration file found. Built-in profiles:) instead ofNo profiles available:--profile precommitworks there, so the old line was wrong. Header text with a file:Available profiles in <config>:.config.yaml:5-7: also corrects--suppression-fileand--disable-ip-types, listed as CLI-only in the same three lines althoughsuppressions.fileandvalidators.intellectual_property.disabled_typesare honoured.Measured:
main@46e2918vs this branchEvery run:
env -i, emptyHOME/FERRET_CONFIG_DIR/XDG_CONFIG_HOME, empty--suppression-file,timeout 60, the same script against both binaries (the issue's matrix plus its "other fields" and second-member blocks, and a byte-identity loop).Matrix A (
pp.txt= one passport,pp.yaml=defaults.checks: PASSPORT,SSN,--format json):[PASSPORT:HIGH][PASSPORT:HIGH]--pre-commit-mode[][PASSPORT:HIGH]PRE_COMMIT=1,_PRE_COMMIT_RUNNING=1,PRE_COMMIT_HOOK=1,GIT_HOOK_TYPE=1[](each)[PASSPORT:HIGH](each)--profile precommit(no pre-commit mode)[][PASSPORT:HIGH]--pre-commit-mode --profile precommit --respect-gitignore)[][PASSPORT:HIGH]pp.txt: 1 high confidence issues found/Passport number detectedPRE_COMMIT=1 FERRET_PRECOMMIT=0[PASSPORT:HIGH]--pre-commit-mode --checks PASSPORT[PASSPORT:HIGH]own.yaml(ownprecommit)[PASSPORT:HIGH]other.yaml(profiles: {ci: …}),empty.yaml({})[][PASSPORT:HIGH]null.yaml(profiles:)[PASSPORT:HIGH]{}now agree)--pre-commit-mode[], 0 B stderrMatrix B/C (
in.txt= SSN + passport,red.yaml=enabled: true, strategy: simple, checks: PASSPORT,SSN), 5 sources (--config,./config.yaml,./ferret.yaml,./.ferret-scan.yaml,$FERRET_CONFIG_DIR/config.yamloutside the working dir) × 7 routes (none + the 6) ×--stdin/--file:SSN: [SSN-REDACTED]andPassport number: [PASSPORT-REDACTED]X00000001in cleartextSSN: ***-**-0001|Passport number: X00000001)--stdinpre-commit:<stdin>: 1 high confidence issues found; three working-dir sources: the provenance note2 high confidence issues found; the same provenance note, nothing elseOther fields:
strategy: synthetic,--stdin --pre-commit-modeSSN: ***-**-0001--redaction-strategy simple[SSN-REDACTED]output_dir: <dir>/cfgout,--file --pre-commit-mode./redacted, 0 incfgoutcfgout, 0 in./redactedprecommitcfgoutconfidence_levels: all, one LOW email,--quiet=false --pre-commit-modeNo matches found at the specified confidence levels.FERRET_PRECOMMIT_EXIT_ON=lowconfidence_levels: low, HIGH SSN, pre-commitprofiles:none /{ci}/{}on--stdin --pre-commit-mode,strategy: simple***-**-0001[SSN-REDACTED]profiles:null / own /profiles.precommit.redaction.strategy: simple[SSN-REDACTED].ferret-scan.yamlPASSPORT,SSN,SECRETS, stagedtraveller.txt,PRE_COMMIT=1 … --profile precommit --respect-gitignore)config.yamlin the user dir,enabled: true,strategy: "synthetic",--stdinPRE_COMMIT=1rc 1***-**-0001PRE_COMMIT=1 --filestderrchecks: allin that file)--verbose --pre-commit-mode,pp.yamlvs no config--debug,--file/--stdinPre-commit environment detected…, no profile named / nothingactive profile "precommit" (built-in), selected because --pre-commit-mode was passed+supplied format=text; confidence_levels=high,medium; checks=CREDIT_CARD,SECRETS,SSN,EMAIL,PHONE,IP_ADDRESS,PASSPORTon both--list-profileswith--config/ discovered / noneprecommit: …/No configuration file found. No profiles available./ sameprecommit (built-in): …underAvailable profiles in pp.yaml:/… in .ferret-scan.yaml (auto-discovered):/No configuration file found. Built-in profiles:defaults.checks: EMAILbeside an SSN[], pre-commit rc 1[SSN:HIGH]profiles.precommit.checks: EMAIL/.redaction.output_direlsewhereSecond member (
ok.txt+ mode-000locked.txt):foi.yaml(fail_on_incomplete: true), no profilefoi.yaml --pre-commit-mode/--profile precommitfoiown.yaml --pre-commit-mode(ownprecommit) /--profile otherfoi.yaml --pre-commit-mode --fail-on-incompletesuppressions.generate_on_scan: true: no profile / pre-commit /--profile otherdefaults.generate_suppressions: true, pre-commitsuppressions.show_suppressed: true, one enabled rule: no profile /--profile otherAcceptance criteria
[SSN-REDACTED]and[PASSPORT-REDACTED], as do--file,--profile precommit, the 4 env signals and 4 discovered sources--pre-commit-moderuns exactly the six;pp.txtrc 0, no new stderrValidators to run: CREDIT_CARD,SECRETS,SSN,EMAIL,PHONE,IP_ADDRESS; rc 0, 0 B / 0 Bdefaults.checks: allruns the six with exactly one stderr line namingprofiles.precommit.checks; none under--stdin --enable-redactionwithout--output--file: 1 stderr line, rc 0;--stdin: 1 line;--stdin --enable-redaction: 0 lines (with--output: 1);--checks SSN: 0; file withoutdefaults.checks: 0 B.ferret-scan.yamldefaults.checks: EMAILleaves the SSN at rc 1output_dirandconfidence_levels: allapply under pre-commitfail_on_incompleterc 3 under the built-in profile, a userprecommitand--profile other; suppression bools survive--profile other{}agree; both guards fail on an unclassified field (mutation);GetPrecommitProfilegone; docs and CHANGELOGByte identity (durations normalised): no config, own
precommit(two configs) andexamples/ferret.yaml× 6 routes (X=1,PRE_COMMIT=1,GIT_HOOK_TYPE=1,--pre-commit-mode,--profile precommit, published hook args) × 7 modes (--filetext/json/verbose,--stdintext/json/redaction,--file --enable-redaction): 168 runs, 0 differ in stdout, stderr, rc or redacted copy.Golden corpus and
make scoreUPDATE_GOLDEN=1 go test ./internal/goldencorpus/: 0 files changed (explicitChecks, below this layer), as the issue predicts.make score: PASS on both, identical report:recall_all=1.0000 recall_hm=0.9906 prec_hm=0.9589 whole_leak=0; redaction sink simple 0/0, format_preserving 0/873; suppression layer 65/65/0/0.Suppression hashes
Unchanged: no finding's text, line or type changes for any check that ran before. Hooks gain findings only for types the config names; those are new findings, not re-hashed ones.
Non-vacuity: each new test on
maincmd/precommit_profile_precedence_test.goandinternal/config/profile_precedence_test.gouse only symbols that exist on main; both were copied into amain@46e2918worktree and run there.mainconfigTestEveryProfileBoolInheritsFromTheConfigProfile bool "fail_on_incomplete" is not in profileBoolFields, so every profile that omits it -- the built-in precommit one included -- sets it to false over whatever the config wrote;config suppressions.show_suppressed: true, profile "other" omits show_suppressed and has false; same forsuppressions.generate_on_scananddefaults.fail_on_incomplete(built-in andother)configTestBuiltinPrecommitProfileYieldsToTheConfigbuilt-in precommit profile sets redaction.strategy = format_preserving; resolveConfiguration lets a non-empty profile value outrank the config; same forredaction.output_dir = redacted;config sets defaults.confidence_levels, yet the built-in profile carries "high,medium";built-in precommit checks = [CREDIT_CARD EMAIL IP_ADDRESS PHONE SECRETS SSN], want [… PASSPORT …];fail_on_incomplete=falseconfigTestBuiltinPrecommitChecksNeverNarrowconfigTestNullAndEmptyProfilesAgreeconfig "profiles:\n": no precommit profile, so pre-commit mode runs without oneconfigTestGetPrecommitProfileIsGoneConfig.GetPrecommitProfile still existscmdTestBuiltinPrecommitProfileYieldsOnEveryRoute--config / --pre-commit-mode / --stdin: redacted output "SSN: ***-**-0001\nPassport number: X00000001\n"; the no-route controls passcmdTestPrecommitHookReportsThePassportTheConfigNamedpp.yaml / --pre-commit-mode: findings [], want exactly [PASSPORT:HIGH](18: 6 routes × pp/other/empty; null passes on main) andrc = 0, want 1(15)cmdTestBuiltinPrecommitChecksAllIsDisclosedOncechecks: all, --file: rc 0 stderr "", want rc 0 and exactly one line naming profiles.precommit.checkscmdTestConfigOutputDirAndConfidenceApplyUnderPrecommitthe redacted copy did not land in the config's output_dir;stdout "No matches found at the specified confidence levels.\n", want the LOW email finding listedcmdTestProfileBoolsInheritTheConfigAtTheBinary[--config foi.yaml --pre-commit-mode] with an unreadable file: rc 0, want 3(and the other three);suppressions.generate_on_scan: true under --profile other wrote no rule;show_suppressed … 0 suppressed entries, want 1cmdTestActiveProfileIsNamed--list-profiles with an auto-discovered config: "No configuration file found. No profiles available.\n";--debug: stderr does not contain "active profile \"precommit\" (built-in), …"on--fileand--stdincmdTestBuiltinPrecommitProfileDecoys,configTestAConfigsOwnPrecommitProfileIsKeptAsWritten,TestBuiltinPrecommitProfileWithoutConfigIsUnchangedprecommit,FERRET_PRECOMMIT=0, plantedEMAIL)TestProfileBoolRegistryAccessorsAgree,TestBuiltinPrecommitChecksConstant,TestBuiltinProfileReport,TestProfileKeys,cmdTestDocsStateTheBuiltinPrecommitChecksquiet: getProfile = false after setting true; main's two docs contain the six-check string 0 timesMutation (guards on an unclassified field): adding
MutantBool boolandMutantText stringtoProfilefails both guards:Profile bool "mutant_bool" is unclassified: register it in profileBoolFields …andProfile field "mutant_bool" / "mutant_text" is unclassified for the built-in precommit profile …. Deleting thefail_on_incompleteregistry entry failsTestEveryProfileBoolInheritsFromTheConfigwith the main-side messages above. Both reverted.scripts/pr-checklist.sh origin/mainRun with its
/tmp/_*log paths redirected to the session scratch dir (same commands otherwise).-short modeFAIL: the script discards that run's output. Re-run three times with output kept on the same commit: 3/3ok(89 packages). The one test this host fails intermittently isTestGrowthRatioMissesASparseQuadratic(internal/goldencorpus, a CPU-clock growth ratio against an 8.0 threshold, not skipped under-short); see the flake counts below, which are the same on main. The checklist's-shortpass ran straight after the whole-module race run.simpleis the strategy the matrix asserts on 70 cells (--filecopies and--stdinstreams);format_preservingis the no-config path, byte-identical to main in 168 runs including--enable-redactionon both inputs;syntheticis measured above (--stdin, and the shipped config in the user dir). No redactor or extractor code changed: this PR changes which configured strategy reaches it, so office containers are not a separate path.mainbinary (--generate-suppressions, 3 rules: SSN, PASSPORT, EMAIL), enabled, then applied by the branch binary:pp.yamlnormal and--pre-commit-modegive 0 results / 3 suppressed (the pre-commit run now includes PASSPORT); own-precommitconfig 0 / 2. Hash inputs unchanged.defaults.checks: alland no-config hooks run the same six as before.precommit} × {normal,--pre-commit-mode} on main vs branch: 28 runs, 0 differ (durations normalised). Golden regeneration across every output: 0 files.Other gates from the worktree, before the checklist:
gofmt -l .empty,go vet ./...clean,go build ./...ok;go test ./...89 packages ok and one FAIL,TestGrowthRatioMissesASparseQuadratic(9.29x against 8.0), the known host timing flake, untouched (the checklist's own full-suite run above was clean). Same minute, interleaved: isolated-run5/5 pass on the branch and 5/5 on main; whole-package runs failed it 1/3 on the branch (8.84x) and 2/3 on main (8.08x, 8.32x).go test -raceon./internal/config/ ./cmd/: ok.Open questions for the reviewer
config.yamlsetschecks: all, so a user who copies it into the user config dir (asdocs/INSTALLATION.mdsuggests) now sees the disclosure line on every hook run, and LOW findings because it also setsconfidence_levels: all. That is the issue's stated trade-off; flagging it because it is the most common way users will meet the line.defaults.checks: alland--stdin --enable-redactionwith no--output, the hook still runs only the six and the line is withheld (stderr is the findings document), so a passport stays in cleartext in that stream with no disclosure. The issue's acceptance criterion asks for exactly this; a follow-up could carry the note inside the findings document instead.--stdinsilently ignores an undefined--profile(the--filepath exits 1). Out of scope here; the debug record now saysprofile "x" is not defined in …; no profile activeon that path.