Skip to content

fix(cli): refuse an unrecognised --redaction-strategy or FERRET_PRECOMMIT_EXIT_ON (#772) - #791

Merged
rustic merged 2 commits into
mainfrom
fix/772-strict-enum-flags
Oct 20, 2026
Merged

rustic merged 2 commits into
mainfrom
fix/772-strict-enum-flags

Conversation

@sectoramen

Copy link
Copy Markdown
Contributor

Closes #772. Independent of #786/#789/#790: no file in common except CHANGELOG.md (the usual ### 🐛 Bug Fixes heading hunk).

What was wrong

Two enum-valued inputs silently became their default on any unrecognised value, both measured on v2.5.4:

  • --redaction-strategy, on --file and --stdin, in a profile, in an explicit --config, or in an auto-discovered .ferret-scan.yaml: anything other than exactly simple, format_preserving or synthetic redacted with format_preserving at exit 0, empty stderr. SIMPLE wrote SSN: ***-**-8901 where simple writes SSN: [SSN-REDACTED] (last four digits, email's first character and domain left in the clear). A typo on the flag threw away a valid strategy: simple in the config. The explicit config's schema trimmed and comma-split, so " simple ", "simple,", "simple,synthetic" and " " passed it and reached a parser that matched none of them. No report named the strategy applied.
  • FERRET_PRECOMMIT_EXIT_ON ignored any value outside high, medium, low, none: with one MEDIUM finding, medium blocked the commit (rc 1) and MEDIUM, Medium, LOW let it through (rc 0). A gate failing open on a typo.

Root causes (main@e8c6fe2): redactors.ParseRedactionStrategy had no error return and a default: of format_preserving, called from four sites; the flag was an unvalidated flag.String; validateEnumField was lenient where the parser was exact; detector.go's switch had no default; pkg/scan.ParseStrategy/RedactFile mapped out-of-range values to format_preserving.

What changed (the issue's Resolution 1–9)

  1. Exact match only, three strategy names, four exit-on names. No folding, trimming or lists: None/NONE block a HIGH finding today (the gate stays at high) and folding them to none would let that commit through after an upgrade; SIMPLE runs would switch from masked to placeholder output. Refusing keeps every rc-0 run byte-identical. A did you mean "simple"? hint is printed for a case/whitespace variant and the value is still refused.
  2. Strict parser redactors.ParseRedactionStrategy(s) (RedactionStrategy, error); RedactionStrategyNames exported; RedactionStrategy.Valid(). The lenient parser is removed, so the compiler found every fallback site.
  3. Typed carrier. finalConfiguration.redactionStrategy, parallel.JobConfig.RedactionStrategy and core.ScanConfig.RedactionStrategy are redactors.RedactionStrategy; the worker refuses a value outside the definition and reports it as a redaction failure for the file (nothing written). Zero value stays simple (audit-log integers unchanged).
  4. One CLI helper applyRedactionStrategy, called on both entry points right after resolveConfiguration, before any scan, output directory or report. resolveConfiguration records the winning name and its layer; the message names the source: --redaction-strategy, profile "share" redaction.strategy in <path>, or redaction.strategy in <path> (auto-discovered) (Trust-boundary decision for a config discovered inside the scan target: opt-in gating, a THREAT_MODEL entry, and the pre-commit gate that silences the provenance disclosure #293).
  5. Unconditional, effective value only. Validated whether or not this run redacts (redaction can be enabled by a config or profile); the stated cost is that a run that does not redact but carries a typo now exits 1.
  6. Schema uses the parser's rule for redaction.strategy (validateExactEnumField, defaults and profiles); --format keeps the lenient rule since it already exits 1 downstream. TestRedactionStrategySpellingsAgreeWithTheRedactor pins the two together.
  7. FERRET_PRECOMMIT_EXIT_ON: an unknown value is recorded as PrecommitDetector.ConfigError(); in pre-commit mode (--pre-commit-mode or a detected hook) both paths exit 1 before scanning, naming the variable, without "commit blocked" text (gate integrity: three ways the tool reports success on work it did not do (consolidates exit resolver, coverage ledger, --exclude) #667). Outside pre-commit mode the variable is not read. ShouldExitOnFindings keeps its in-code default: with the comment corrected.
  8. pkg/scan: ParseStrategyStrict added; ParseStrategy Deprecated: and unchanged. RedactText/RedactFile reject RedactStrategy(7) before writing, as pkg/redact.NewEngine does.
  9. Sequencing: resolveConfig returns an error for a pinned ConfigPath that exists and fails the strict load (parse or schema), so ScanText/ScanFile/RedactFile surface it; a missing path keeps the fix(config): name a config discovered in the working directory, and let pkg/scan pin or disable discovery #317 fallback. Without this, tightening the schema turned a pinned config with a custom pattern plus strategy: " simple " from 1 finding into 0 with err=nil.

One deviation from the issue text, stated: core.RedactFile's Strategy: "" keeps its documented meaning (format_preserving). It is an in-process Go API whose empty value means "not set", mirrored by its own tests; any set value that does not match is refused before the output path exists.

Measured, main@e8c6fe2 vs this PR

--file --enable-redaction --redaction-strategy <v>, SSN line of the redacted copy:

value main PR
simple rc 0, SSN: [SSN-REDACTED] identical
format_preserving rc 0, SSN: ***-**-8901 identical
SIMPLE rc 0, ***-**-8901 (format_preserving applied) rc 1, no output dir, invalid redaction strategy "SIMPLE": valid values are simple, format_preserving, synthetic (did you mean "simple"?) (from --redaction-strategy)
simpel, " simple", bogus rc 0, ***-**-8901 rc 1, no output dir
--stdin with SIMPLE/bogus rc 0, 102 bytes redacted under format_preserving rc 1, 0 bytes on stdout
explicit config " simple ", "simple,", "simple,synthetic", " " rc 0, format_preserving rc 1 from the schema
explicit config strategy: simple + --redaction-strategy bogus rc 0, format_preserving (config discarded) rc 1
auto-discovered .ferret-scan.yaml strategy: simpel rc 0, format_preserving rc 1, (from redaction.strategy in .ferret-scan.yaml (auto-discovered))

--pre-commit-mode, exit code on a MEDIUM-only fixture / a HIGH-only fixture:

FERRET_PRECOMMIT_EXIT_ON main PR
unset, "", high 0 / 1 0 / 1
medium, low 1 / 1 1 / 1
none 0 / 0 0 / 0
MEDIUM, Medium, meduim, " medium", LOW, None, NONE, off 0 / 1 (gate stays at high) 1 / 1 before scanning: FERRET_PRECOMMIT_EXIT_ON="MEDIUM" is not one of high, medium, low, none (did you mean "medium"?)
--stdin --pre-commit-mode with MEDIUM 0 1
outside pre-commit mode, MEDIUM 0 0 (no effect)

Golden corpus: UPDATE_GOLDEN=1 changes 0 files. make score: PASS, identical to main. No finding text, line or type changes, so suppression hashes are unchanged.

Tests, and that each fails on main

Test Pins On main
cmd TestAnUnusableRedactionStrategyIsRefusedOnEveryChannel 9 refused spellings × --file (rc 1, value + 3 names + source quoted, no output dir, empty stdout), --stdin (0 stdout bytes), explicit config defaults and profile, config-vs-flag precedence, auto-discovered file (message says auto-discovered), unconditional validation; canonical names rc 0 with the v2.5.4 bytes (non-vacuity: simple and format_preserving must differ) rc 0 everywhere, output dir created, stdout written
cmd TestPreCommitExitOnIsRefusedWhenMisspelled the 4 names keep 0/1/1/0 and 1/1/1/0; 10 misspellings rc 1 on both fixtures and --stdin, variable named, no "commit blocked", empty stdout; no effect outside pre-commit mode rc 0 on every misspelling, scan ran
cmd TestEveryEnumValuedInputRejectsAnOutOfDomainValue the derived class guard (below) selects 8 inputs, fails on --redaction-strategy and FERRET_PRECOMMIT_EXIT_ON
config_test TestRedactionStrategySpellingsAgreeWithTheRedactor 24 spellings: schema (defaults and profile) and parser agree; "" unset for the schema, refused by the parser main's schema accepts " simple ", "simple,", "simple,synthetic", " "
redactors TestParseRedactionStrategyIsExact, TestRedactionStrategyValid the parser table, hints, round-trip with String(), Valid, zero value new API
parallel TestRedaction_OutOfRangeStrategyIsRefusedNotDefaulted a job with RedactionStrategy(7) is reported in UnredactedFiles, nothing written; zero value redacts field was a string
core TestRedactFile_MisspelledStrategyIsRefusedBeforeAnyOutput simpel/SIMPLE/" simple"/bogus error before the output dir exists; "" default unchanged wrote format_preserving copies
pkg/scan TestParseStrategyStrict…, TestRedactTextRejects…, TestRedactFileRejects…BeforeWriting, TestASchemaRejectedPinnedConfigPathIsAnError strict parser, deprecated parser unchanged, RedactStrategy(7) refused without writing, the sequencing guard with an exact-spelling control new API / silent fallback

The class guard derives its set rather than keeping a list: every flag.String in cmd/main.go whose usage text enumerates values (parsed with go/ast; a strings.Join over a name list counts, which is how --checks is registered) and every FERRET_* row of docs/PRE_COMMIT_INTEGRATION.md's environment table whose options cell lists backticked values. Today that is --checks, --confidence, --disable-ip-types, --format, --redaction-strategy, FERRET_PRECOMMIT, FERRET_PRECOMMIT_EXIT_ON, FERRET_PRECOMMIT_EXIT_ON_FIRST, exactly the eight the issue predicted. Three exemptions carry a reason and fail if their input stops being selected: --disable-ip-types (warns and keeps detecting; the --stdin half is a sibling item), FERRET_PRECOMMIT_EXIT_ON_FIRST (nothing reads it), FERRET_PRECOMMIT (an unrecognised opt-out keeps the gate on).

Compatibility

  • rc 1 is the existing usage-error code, so hooks fail closed. The repo's own docs and examples use only canonical values.
  • Runs that exit 0 today are byte-identical (canonical names only). Runs with a non-canonical spelling exit 1 where they previously wrote output under a strategy they did not ask for; that is the fix.
  • pkg/scan: only callers passing an undefined RedactStrategy constant, or a pinned ConfigPath naming a file that exists and is invalid, see a new error. ParseStrategy is unchanged.
  • Audit-log strategy integers unchanged (no renumbering).

Docs

docs/user-guides/README-Redaction.md, docs/user-guides/README-Stdin.md, docs/configuration.md, docs/PRE_COMMIT_INTEGRATION.md (table row), docs/TEAM_SETUP_GUIDE.md, the --redaction-strategy usage text and internal/help/help.go now state the exact-spelling rule and that other values exit 1 naming the source.

Test plan

-- changelog                       RAN — entry present under [Unreleased] (42 line(s)); required because it changes what a release 
1 gofmt                            RAN — clean
1 go vet                           RAN — clean
1 go build                         RAN — ok
2 full suite (-count=1)            RAN — 89 packages ok
3 golden regen                     RAN — 0 files changed
4 redaction                        NEEDS-MANUAL — scan a fixture with --enable-redaction for EVERY strategy; for xlsx/docx read the part INSIDE the zip (grepping the compressed bytes returns 0 and looks like a pass)
5 suppression                      NEEDS-MANUAL — generate with the PARENT binary, apply with YOURS — same-binary round-tripping passes even when the hash inputs changed
6 timing / O(n^2)                  NEEDS-MANUAL — scaling curve at N/2N/4N with a NON-VACUITY floor (assert findings > 0 at every size) and distinct values (identical repeats let a quadratic measure as linear)
7 all 7 formats                    NEEDS-MANUAL — text json yaml csv junit gitlab-sast sarif
8 web / CSP                        N/A — no web/template/asset file touched
10 non-vacuity                     NEEDS-MANUAL — new tests MUST fail on the parent commit; prove it by reverting the production change (or the guard) and re-running
11 cross-platform vet              RAN — linux darwin windows
12 flake (3 repeats)               RAN — ./cmd ./internal/config ./internal/core ./internal/help ./internal/parallel ./internal/precommit ./internal/redactors ./internal/redactors/plaintext ./pkg/scan 
13 race (whole module)             RAN — 89 packages ok
-- -short mode                     FAIL — fails under -short
-- tree clean                      RAN — this run dirtied nothing
  • -short FAIL: TestGrowthRatioMissesASparseQuadratic (goldencorpus), the perfguard host flake (8.15x against 8.0 on this Cloud Desktop; CI reads 4.6–5.0x), untouched by this PR. Re-run with output kept: that is the only failure. The full suite, the race run and the 3x flake repeats on every touched package were green.
  • 4 redaction: the measured table above covers simple, format_preserving (byte-identical to main) and synthetic (accepted, random output) on --file and --stdin; the refused spellings create no output directory and write 0 bytes on --stdin.
  • 5 suppression: N/A for hashes (no finding text/line/type changes; 0 golden files changed). The one hash-adjacent surface, redaction.strategy in a config, is now refused rather than re-hashed.
  • 6 timing: N/A, no scanning path changed; the new work is one string comparison per run.
  • 7 formats: N/A, no formatter touched; golden regeneration across all 9 outputs changed 0 files.
  • 10 non-vacuity: the table above; the three CLI tests fail on main as listed, and the class guard on main selects the same eight inputs and fails on exactly the two this issue is about.

@sectoramen sectoramen added this to the v2.5.5 milestone Oct 16, 2026
@sectoramen sectoramen added bug Something isn't working go Pull requests that update go code P0 Audit tier P0: fail-open or cleartext leak; fix first labels Oct 16, 2026
@sectoramen
sectoramen requested a review from rustic October 16, 2026 19:01
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Ferret Scan Results

✅ No sensitive data detected in the changed files.


🤖 This comment was automatically generated by Ferret Scan

…MMIT_EXIT_ON instead of defaulting

Any value other than exactly simple, format_preserving or synthetic, given
to --redaction-strategy (--file or --stdin), in a profile, in an explicit
config or in an auto-discovered .ferret-scan.yaml, redacted with
format_preserving at exit 0 with an empty stderr: `SIMPLE` wrote masked
values (***-**-8901) where `simple` writes placeholders ([SSN-REDACTED]).
A typo on the flag threw away a valid `strategy: simple` from the config,
and the explicit config's schema trimmed and comma-split, so " simple ",
"simple," and "simple,synthetic" passed it and reached a parser that
matched none of them. FERRET_PRECOMMIT_EXIT_ON ignored any value outside
high/medium/low/none, so with one MEDIUM finding `medium` blocked the
commit and `MEDIUM` let it through: a gate failing open on a typo.

Changes:

- redactors.ParseRedactionStrategy returns (RedactionStrategy, error),
  exact match only, with a did-you-mean hint for a case or whitespace
  variant; RedactionStrategyNames is exported; RedactionStrategy.Valid.
  The lenient parser is gone, so the compiler found every fallback site.
- The strategy travels typed: finalConfiguration, parallel.JobConfig and
  core.ScanConfig hold a redactors.RedactionStrategy; the worker refuses a
  value outside the definition; core.RedactFile parses its name strictly
  before any output path exists ("" keeps its documented default).
- One CLI helper, applyRedactionStrategy, runs on both entry points right
  after resolveConfiguration, before any scan, output directory or report,
  unconditionally, and names the source: --redaction-strategy, profile "x"
  redaction.strategy in <path>, or redaction.strategy in <path>
  (auto-discovered). resolveConfiguration records the winning name and its
  layer; only the literal empty string means unset.
- The config schema applies the parser's exact rule to redaction.strategy
  (validateExactEnumField); --format keeps the lenient rule since it exits
  1 downstream. An agreement test pins schema and parser together.
- precommit: an unknown FERRET_PRECOMMIT_EXIT_ON is recorded as
  ConfigError; in pre-commit mode both paths exit 1 before scanning,
  naming the variable, without "commit blocked" text. The in-code default
  branch of ShouldExitOnFindings stays, with its comment corrected.
- pkg/scan: ParseStrategyStrict added; ParseStrategy deprecated,
  unchanged. RedactText/RedactFile reject an out-of-range RedactStrategy
  before writing. resolveConfig returns an error for a pinned ConfigPath
  that exists and fails the strict load (schema or parse), which is what
  made the schema safe to tighten; a missing path still falls back (#317).

Measured on v2.5.4 vs this build: every refused spelling now exits 1 with
no output directory and 0 bytes on --stdin; the canonical names write
byte-identical output; the four pre-commit names keep rc 0/1/1/0 (MEDIUM
fixture) and 1/1/1/0 (HIGH fixture); the eight misspellings exit 1 on
both paths; outside pre-commit mode the variable has no effect. Golden
corpus: 0 files changed. make score unchanged. No finding text, line or
type changes, so suppression hashes are unchanged.

Tests: CLI contract tests across the five channels and the pre-commit
matrix (all fail on main); a derived class guard that selects every
flag.String whose usage lists values (via go/ast) and every FERRET_* row
of the pre-commit docs table that lists values, and requires each to
refuse an out-of-domain value -- on main it selects eight inputs and
fails on exactly --redaction-strategy and FERRET_PRECOMMIT_EXIT_ON, with
three reasoned exemptions that fail if stale; the schema/parser agreement
test; parser, worker, core and pkg/scan unit tests; a sequencing guard
for the schema-rejected pinned ConfigPath.

Docs: the redaction guide, configuration reference, stdin guide,
pre-commit integration table, team setup guide and --help state the
exact-spelling rule.

Closes #772
@sectoramen
sectoramen force-pushed the fix/772-strict-enum-flags branch from 21ae95d to 4bf34d4 Compare October 20, 2026 17:17
@sectoramen

Copy link
Copy Markdown
Contributor Author

Rebased onto main after #786 merged (21ae95d -> 4bf34d4). GitHub flagged the CHANGELOG as conflicting because both sides opened the ### 🐛 Bug Fixes section under [Unreleased]; the bullet now sits after #786's. Everything outside CHANGELOG.md is byte-identical to the previous head (same git patch-id). Build, vet, the touched packages and goldencorpus pass on the rebased head (the only red in repeated runs is TestGrowthRatioMissesASparseQuadratic, which fails at the same rate on unchanged main on this host); CI is re-running.

@github-actions

Copy link
Copy Markdown
Contributor

🔍 Ferret Scan Results

✅ No sensitive data detected in the changed files.


🤖 This comment was automatically generated by Ferret Scan

Catches up with #792, #789 and #790. CHANGELOG.md merged without conflict
through the merge=union attribute (#781), but union put the result in the
wrong place: this branch adds a new ### Features subsection after Bug Fixes,
and union emits this side's lines before main's, so #790's and #789's
bug-fix bullets landed under Features. [Unreleased] is rebuilt by hand:
Bug Fixes holds main's three bullets in main's order and then this branch's,
and Features holds this branch's one. Every bullet is moved byte-for-byte;
the section's multiset of non-blank lines is unchanged.
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Ferret Scan Results

Gate mode: report — findings are reported here and do not fail this check; a scan that fails or is incomplete still does.

⚠️ Sensitive data detected in 12 scanned file(s):

  • High confidence: 4 findings
  • Medium confidence: 1 findings

Findings Summary:

File Type Confidence Line
main.go SSN HIGH 1709
main.go SSN HIGH 3440
stdin.go PERSON_NAME HIGH 111
stdin.go INSURANCE_MEMBER_ID MEDIUM 116
apply.go INTELLECTUAL_PROPERTY HIGH 102

Next Steps:

  1. Review the findings above
  2. Remove or redact any actual sensitive data
  3. Add suppressions for false positives
  4. Re-run the scan after making changes

🤖 This comment was automatically generated by Ferret Scan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working go Pull requests that update go code P0 Audit tier P0: fail-open or cleartext leak; fix first

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cli: an unrecognised --redaction-strategy or FERRET_PRECOMMIT_EXIT_ON value silently becomes the default

2 participants