fix: preserve argv boundaries in spin run and spin exec#185
Open
yusufkandemir wants to merge 1 commit into
Open
fix: preserve argv boundaries in spin run and spin exec#185yusufkandemir wants to merge 1 commit into
yusufkandemir wants to merge 1 commit into
Conversation
filter_out_spin_arguments returned its result via echo (space-joined, unquoted) and the caller word-split it back on IFS, destroying any argument containing whitespace. `spin run --quiet php php -r 'echo "hi";'` reached PHP as `echo` + `"hi";` (two tokens), causing parse errors.
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.
Problem
spin runandspin execlose argument boundaries when any argument contains a space or quoted string. Minimal reproduction:The single-quoted PHP source arrives at the container split into two tokens (
echoand"hi";), so PHP only seesecho. Same problem causes anyspin run php php artisan tinker --execute '...'to fail with similar errors as well. This is especially a big deal with AI agents since Tinker and PHP REPL are important to quickly test/verify certain things.Cause
spin/lib/actions/run.sh
Line 5 in e205e41
filter_out_spin_argumentsreturned its result viaecho "${filtered[@]}"(space-joined, unquoted), and the caller word-split that back on IFS. Every argument containing whitespace is gone before reachingdocker compose run.lib/actions/exec.shhad a smaller variant of the same problem: bare$@instead of"$@".Fix
Use a global-array return-based solution. The same pattern is also used by the existing
prepare_ansible_run.filter_out_spin_argumentsnow writes toSPIN_FILTERED_ARGSrun.shcopies it into a local array to make the usage easier, preserving the previous DXexec.shquotes"$@".Bash 3.2 compatibility is required as noted in AGENTS.md(I'd recommend creating a contributing.md or similar, or putting the same requirement elsewhere too). So, I didn't use the nameref(
local -n) approach, which would've helped avoid introducing global variables, but it only works in Bash 4.3+.