fix: escape --host option in serve command#10203
Merged
Merged
Conversation
michalsn
approved these changes
May 15, 2026
samsonasik
approved these changes
May 15, 2026
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.
Description
spark serveconcatenated the user-supplied--hostoption directly into the shell command passed topassthru(), while sibling arguments (--php, document root, rewrite script) were already wrapped withescapeshellarg(). A locally-crafted host such as$(id)would be expanded by/bin/sh -cbefore reachingphp -S. Threat model is limited (local CLI argv control); this is hardening on par with the surrounding code.Replaces and closes #10156.
Changes:
Serve::buildServeCommand()that wraps every interpolated argument withescapeshellarg(), thensprintf's them into thephp -S host:port -t docroot rewritetemplate.ServeTestcovering the builder with literal expected strings: a happy path plus eight injection vectors (command substitution, shell separators, redirection, embedded quote, newline). Gated to POSIX shells via#[RequiresOperatingSystem('^(?!WIN)')]becauseescapeshellarg()output differs on Windows.Bugs Fixedentry underCommandsin the v4.7.3 changelog.Note on 4.8 forward-merge
This bug only exists on
develop. The4.8branch rewritesServeto extendAbstractCommandand already escapes the host inside itsexecute()method, so the vulnerability is not present there. Whendevelopis forward-merged into4.8:system/Commands/Server/Serve.php: keep 4.8's version; the rewrite supersedes this fix.tests/system/Commands/Server/ServeTest.php: should be deleted during conflict resolution. The test reflects on theBaseCommand-basedbuildServeCommand()method, which does not exist on 4.8.Checklist: