test(diffanalysis): gate sensitive patterns on structural mutants #91
Loading…
Reference in a new issue
No description provided.
Delete branch "test/sensitive-path-mutation"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Stacked on #88 — that PR's commit is included in this diff until it merges. Review #88 first.
Follows up on the regex fix by asking whether the tests would catch that bug coming back. They wouldn't.
The measurement
Scoring the corpus that shipped with #88 against patterns rewritten the way a careless edit would rewrite them: 11 of 63 mutants killed (17%). The important survivors:
ungroup-alternationsurvived on security and build system — the exact bug #88 fixes could be reintroduced on those two patterns with CI green.drop-trailing-boundarysurvived on security, database migration, and Helm.drop-leading-boundarysurvived on 11 of 18 patterns.Worth correcting something I claimed in #88's description: on the security pattern,
signup.cssis kept clean by dropping baresignfrom the keyword list, not by the trailing boundary; on Helm,charts.gois kept clean by requiringcharts/with the slash. The trailing boundary is load-bearing only on the auth pattern (tokenizer) — and that mutant was already dying. The fix is right, but two of its three defenses had no test behind them.What this adds
sensitiveCorpus— one table mapping path to exact expected tags — plusTestSensitivePatternMutants, which regenerates every pattern minus a boundary or minus the alternation grouping and fails when the corpus can't distinguish the mutant from the real pattern. Killing them needs deliberate near misses:internal/migratefs/,pkg/verifying/,tools/helmfile/,tools/GNUMakefile,templates/dot.github/. All 26 structural mutants now die.The property that makes it worth gating: a new pattern added without corpus entries matches nothing, so its mutants match nothing either, they compare equal, and the gate fails until someone adds coverage. Verified both directions — reintroducing the ungrouped alternation is caught, and adding an uncovered
(kafka|redis)pattern is caught.Off-the-shelf Go mutation tools (
gremlins,go-mutesting) were not used and wouldn't help: they mutate AST nodes, and all the logic here lives inside string literals they don't model. This is ~80 lines, no new dependencies, runs in milliseconds.Scope
Structural devices only. Requiring a positive example per keyword (
oidc,authz,decrypted, ...) is ~40 more mutants and roughly triples the corpus; deliberately not gated here.Two side effects
.envgot fixed. It was(^|/)\.env, which required the filename to start with.envand so missed the commonapp.env/prod.envconvention (docker --env-file). Writing its near-miss entry forced the question. Now\.(env|envrc)([^a-zA-Z]|$)— covers those, still rejectsdev.environment.md, and deliberately carries no leading anchor:[^/]*can always reach back to the segment start, so anchoring would produce a mutant no corpus could ever kill (an equivalent mutant).Tag collection moved into
sensitiveTagsFor, so the corpus scores the same code pathAnalyzeuses instead of a copy that can drift.TestSensitiveTagsRespectPathBoundariesfrom #88 is removed — fully subsumed by the corpus table.🤖 Generated with Claude Code
`|` binds looser than concatenation, so `(^|/)auth|oauth|...` parsed as `((^|/)auth)|(oauth)|...` — only the first branch was anchored to a path segment and the rest were bare substring matches. internal/design/ was tagged "security", cmd/immigration/ "database migration". Grouping the alternation fixes the leading end. That alone left the prefix case: with the boundary satisfied, "token" still matched pkg/tokenizer/ and "chart" matched dashboard/charts.go. A trailing ([^a-zA-Z]|$) closes it, at the cost of spelling out derived forms, since nothing distinguishes "tokens" from "tokenizer" by shape. Bare "sign" is dropped rather than enumerated — signup/signal/assign swamped the code-signing sense — with "signing" and "signature" left to carry it. Helm now keys off helm/, charts/, or Chart.yaml specifically. These tags reach the model as flat assertions ("Touches **security**"), so a false one aims the review at a threat model the diff has nothing to do with. Tags are also deduped per file, since Helm now has three spellings and a path matching two of them would be listed twice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>Automated review by pr-reviewer v0.41.4 | Safety Check | Claude Sonnet 5 | tracking id
r-682b06-a58c95This is an AI-generated review and may contain mistakes.
Status: ✅ Completed
✅ Verdict: LGTM — Test-only PR (plus carried-forward #88 pattern fix) adding a mutation-testing gate; changes are self-contained, well-explained, and show no signs of bugs, secrets, or accidental commits.
This is a test-only PR (plus the carried-forward #88 regex fix) that adds a mutation-testing style gate over the sensitive-path patterns. I checked for logic errors, security issues, and accidental commits:
sensitivePatternsare consistent with the corpus and the PR's stated rationale; the.envregex change (\.(env|envrc)([^a-zA-Z]|$)) is explained and matches the stated intent (catchingapp.env/prod.env), and its lack of a leading anchor is deliberately justified as avoiding an equivalent mutant.sensitiveTagsForcorrectly dedupes tags (verified viaTestSensitiveTagsDeduped) and is now shared by bothAnalyzeand the test corpus, which is a reasonable refactor reducing drift risk.structuralMutants,matched,TestSensitivePatternMutants) is self-contained, only touches test files, and doesn't affect production code paths.Analyze's public behavior beyond the pattern refinements already covered by #88, which this PR says is stacked on and should be reviewed separately.I don't see anything that looks unsafe or accidental here — it's a well-scoped test addition with clear rationale, and the diff matches the description closely.
1b8f8fcae3f8105ad835Automated review by pr-reviewer v0.42.0 | Safety Check | Claude Sonnet 5 | tracking id
r-684970-2df8deThis is an AI-generated review and may contain mistakes.
Status: ✅ Completed
✅ Verdict: LGTM — Test-only PR (plus a well-justified regex fix) adding a mutation-testing gate; no bugs, secrets, or breaking changes found.
This is a test-only PR (plus one small regex/refactor change) that adds a mutation-testing style gate over the sensitive-path patterns. I checked the substantive code change:
.envpattern change from(^|/)\.envto\.(env|envrc)([^a-zA-Z]|$)is explained clearly and the corpus includes a check thatdocs/dev.environment.mdstays untagged, which is the right regression guard for dropping the leading anchor.sensitiveTagsForextraction is a straightforward refactor that keepsAnalyze's behavior identical (dedup logic preserved), and the new mutation test exercises this exact function, so there's no drift risk between test and production code path.structuralMutants,keywordAlternation,matched) is self-contained, doesn't touch production logic, and only runs in the test binary.TestSensitivePatternMutants's predecessor (TestSensitivePatternRespectPathBoundaries) is justified as subsumed by the new corpus table, and spot-checking a few entries confirms the same paths/tags are still covered.No secrets, no committed binaries, no breaking changes to runtime behavior — this only affects an internal diff-analysis heuristic used for tagging, not anything security-critical in the auth/execution sense. Nothing here looks accidentally pushed or alarming.