Reject empty command names in the parser - #2
Merged
Conversation
A script line that starts with a quoted empty string, such as '' or '' e, parses into a Command with an empty name. The tokenizer emits empty tokens for quoted empty strings on purpose, because cmd '' must pass an empty argument. But parse_command_line takes the first token as the command name without a check. Return a parse error with the line number when the command-name token is empty, in both the plain case and the ! negation case. The tokenizer does not change, so empty quoted arguments still work. Three fuzz targets (parser, tokens, structured) crash on this input. See the failing fuzz run 30781132837. A regression test covers the crash inputs and the still-valid empty-argument case. Co-Authored-By: Claude <noreply@anthropic.com>
test_update_scripts_via_env_var sets UPDATE_SCRIPTS and then reads it inside execute(). test_normal_mode_still_fails removes the same variable on a parallel test thread. The variable is process-global, so the removal can land between the set and the read. The test then runs in normal mode and fails, as seen in CI run 30787133800. Add a shared mutex and hold it for the whole body of both tests. The lock ignores poisoning, so one failed test does not cascade into the other. The two other tests in the file set update_scripts explicitly and never read the variable, so they need no lock. Co-Authored-By: Claude <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.
parser::parsereturns aCommandwith an emptynamewhen the first token of a script line is a quoted empty string. The minimal inputs are''and'' e; the second one parses intoCommand { name: "", args: ["e"] }. The tokenizer emits empty tokens for quoted empty strings on purpose, becausecmd ''must pass an empty argument. Butparse_command_linetakes the first token (or the token after!) as the command name without a non-empty check. Three fuzz targets (parser, tokens, structured) crash on this input; their assertion that a command name is never empty is a correct invariant, and Go's testscript rejects such lines. The crash surfaced in fuzz run https://github.com/antithesishq/testscript-rs/actions/runs/30781132837.The fix is in
parse_command_lineonly. When the command-name token is empty, in both the plain case and the!negation case, the parser now returns a parse error with the line number and the message "empty command name".parse_command_tokensdoes not change, so empty quoted arguments keep working.A regression test checks that
''and'' efail to parse with the empty-command-name error, that"" argand! ''fail the same way, and thatcmd ''still parses with one empty-string argument.cargo testpasses across all suites. A standalone reproducer feeds the fuzz crash inputs throughparser::parseand now getsErr(Parse error at line 1: empty command name)for both.cargo fmt --checkandcargo clippy --all-targets -- -D warningspass. cargo-fuzz is not installed in this environment, so the fuzz targets did not run locally.🤖 Generated with Claude Code
https://claude.ai/code/session_01S9UYm4w97yWTsvMeRHC2uZ