fix(daemon): make the credential scan provably terminating - #84
Merged
Merged
Conversation
The mutation gate on main has been failing, and not because a mutant survived. Three mutants in sanitize_tool_input hung the job outright: guard.rs:91 replace += with *= in sanitize_tool_input (j *= 1) guard.rs:93 replace >= with < in sanitize_tool_input guard.rs:98 replace += with *= in sanitize_tool_input (i *= 1) Each one stops the scan making progress. `j *= 1` freezes the inner cursor, so the run-counting loop never reaches the end of the input. `run < 40` sends a zero-length run down the redact branch, which then advances by `i += run`, i.e. not at all. `i *= 1` freezes the outer cursor directly. cargo-mutants times each out after 58s of a 63s build, the shard exits 3, and the workflow reports "Raise timeout_multiplier in .cargo/mutants.toml, or the mutant caused a genuine hang." It is the second of those. Raising timeout_multiplier would only make the job take longer before failing, and would weaken the gate for every other mutant in the suite, so the fix is to remove the hang. All three share one cause: a hand-written cursor walked by arithmetic inside `while i < chars.len()`, with three separate expressions responsible for making progress and nothing enforcing that any of them does. Grouping the input into maximal runs of same-class characters removes the cursor entirely, so progress is structural and no arithmetic slip can turn the scan into an infinite loop. It is also less code: 28 lines replacing 175. Behaviour is unchanged. A differential test reproduces the original cursor walk verbatim and asserts the two agree across fourteen inputs covering every branch of both, including runs at the threshold and one either side of it, adjacent runs, runs at each end of the string, and non-ASCII text. The magic numbers gain names: the 40-character threshold is MIN_REDACTED_RUN and the alphabet test is is_base64_char, which the tests now assert against rather than restating. All 12 mutants of the rewritten code were hand-applied and the suite confirmed to fail: 12 killed, 0 survived, 0 hung. That includes the `>= MIN_REDACTED_RUN` -> `<` mutation that used to hang, which is now an ordinary caught mutant. Two of the kills are worth naming. A run of 40+ characters that are NOT base64 (punctuation, say) must not be redacted, which pins the class check next to the length check. And `+` and `/` must keep counting: if they stopped, a real base64 key would split into runs of 17, 14 and 7, none long enough to redact, and the whole key would be written to the audit log in clear.
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.
The mutation gate on
mainhas been failing — and not because a mutant survived. Three mutants insanitize_tool_inputhung the job outright, so the run produced no kill rate at all.Each stops the scan making progress:
j *= 1run < 40i += run— i.e. not at alli *= 1cargo-mutants times each out after 58s of a 63s build, the shard exits 3, and the workflow prints "Raise
timeout_multiplierin.cargo/mutants.toml, or the mutant caused a genuine hang."It is the second of those. Raising
timeout_multiplierwould only make the job take longer before failing, and would weaken the gate for every other mutant in the suite. So the fix is to remove the hang.One cause, three symptoms
All three share a root: a hand-written cursor walked by arithmetic inside
while i < chars.len(), with three separate expressions responsible for making progress and nothing enforcing that any of them does.Grouping the input into maximal runs of same-class characters removes the cursor entirely. Progress becomes structural — the loop is over groups — so no arithmetic slip can turn the scan into an infinite loop. It is also less code: 28 lines replacing 175.
Behaviour is unchanged, and that is tested
A differential test reproduces the original cursor walk verbatim and asserts the two agree across fourteen inputs covering every branch of both: runs at the threshold and one either side, adjacent runs, runs at each end of the string, punctuation-only input, and non-ASCII text.
The magic numbers also gain names —
MIN_REDACTED_RUNfor the 40-character threshold andis_base64_charfor the alphabet test — which the tests assert against rather than restating.Mutants
All 12 mutants of the rewritten code were hand-applied and the suite confirmed to fail:
12 killed, 0 survived, 0 hung.
That includes the
>= MIN_REDACTED_RUN→<mutation that used to hang; it is now an ordinary caught mutant.Two kills are worth naming, because both are real leak paths:
+and/must keep counting. If they stopped, a real base64 key would split into runs of 17, 14 and 7 — none long enough to redact — and the whole key would be written to the audit log in clear.cargo fmt --check,cargo clippy --all-targets -- -D warningsand the full 881-test lib suite are clean.