test: every registered preset renders a recipe that parses back - #69
Merged
Conversation
Finding S8 of the outside security review: the preset renderer builds its source by writing strings into a template, and what keeps that safe is a whitelist of the characters a size is written with - added after fuzzing found the hole in the first place. The review's point was that the whitelist has to be remembered by whoever writes the next preset. The review's remedy was to compose through recipe.Compose. Measured with tools/probes/composeeject and turned down: the source comes out 627 B rather than 965 B, the comment header saying where the file came from disappears, and every number is quoted because the draft type holds strings. internal/cli/preset.go records recipe.Hash of exactly those bytes as recipe_hash in the manifest - checked on a real run - so that is a breaking change in other people's records rather than a tidy. The property the whitelist provides is asked for directly instead: whatever a preset is given, the source it produces is either refused with a sentence or parses back into a recipe with targets. FuzzPresetExpansion already asks that, well, and asks it of "size-boundaries" by name - so a second preset would arrive uncovered. This asks it of every registered preset and every parameter it declares, with fourteen hostile values, and it is deterministic rather than fuzzed because a fuzz target runs where somebody runs it. One mutation: taking the whitelist off turns it red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Finding
S8of the outside security review, and the last one. The premise is true and the proposed remedy was measured and turned down.The premise
internal/preset/sizeboundaries.gobuilds its recipe source by writing strings into a template. What keeps that safe isfirstUnusable, a whitelist of the characters a size is written with - added after fuzzing found the hole on 2026-08-05, where1<CR>Bparsed as one byte because the size parser trims the ends, and the carriage return then reached the document raw. The review's point stands: that whitelist has to be remembered by whoever writes the next preset.The remedy, measured and turned down
The review suggested composing through
recipe.Compose, calling it a cheap structural tidy. Measured withtools/probes/composeeject, onsize-boundariesatlimit=4mb,spread=1B,1kb:ComposeThe comment header that tells a reader where the file came from disappears, the list indentation changes, and every number comes out quoted (
count: "1",size: "4194303") becauseTargetDraftholds strings.🔴 And those bytes are a contract.
internal/cli/preset.go:329computesrecipe.Hash(expanded.Source)and records it asrecipe_hashin the manifest - checked on a real run. A renderer that moves one byte moves that hash for everybody who runs a preset. That is a breaking change in other people's records, not a tidy.What this does instead
Asks for the property the whitelist exists to provide: whatever a preset is given, the source it produces is either refused with a sentence or parses back into a recipe with targets.
FuzzPresetExpansionalready asks exactly that, well, with"1kb\n id: injected"among its seeds - and asks it ofsize-boundariesby name, so a second preset would arrive uncovered. This asks every registered preset and every parameter it declares, with fourteen hostile values (28 tries today), and it is deterministic rather than fuzzed because a fuzz target runs where somebody runs it and this runs on every push.One mutation: taking the whitelist off turns it red.
🤖 Generated with Claude Code