Repository navigation
fix(recipe): host steps left dir: and script: unsubstituted - #216
Merged
kuny0707 merged 1 commit intoAug 6, 2026
Merged
Conversation
`kind: host` wired substitution for args and run[1:] and env:, and missed
the other two fields it accepts. Found by writing the first real recipe
that uses them — a private chain, a TRC20 deploy and a load run — where
both failed, and only one failed visibly.
**script: was the dangerous one.** The template text reached the shell
verbatim, so
script: until curl -sf {{ params.node_url }}/wallet/getnowblock; do sleep 2; done
curled a literal "{{ params.node_url }}" forever. The step ran, nothing
errored, and the recipe hung until the operator gave up — a silent wrong
result in the most literal sense.
Substituting it is not the fix: interpolating a value into a shell body
is command injection by construction, and a param of "; rm -rf /" would
execute. env: is already substituted and the shell sees it as data, so
{{ }} inside script: is now rejected at load time with a message naming
that alternative:
env: {NODE_URL: "{{ params.node_url }}"}
script: curl -sf "$NODE_URL/wallet/getnowblock"
**dir: gets the opposite treatment and is now substituted.** It becomes
cmd.Dir, a path handed to chdir where a hostile value can only fail to
open — no shell, no injection surface. It failed loudly ("chdir
{{ params.lab_dir }}/contract: no such file or directory") rather than
silently, which is the only reason it was the second bug found and not
the first.
The two fields differing is the point, and the rule is where the value
lands: a path substitutes, code does not.
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.
kind: host(#214) wired substitution forargs,run[1:]andenv:— and missed the other two fields it accepts. Found by writing the first real recipe that uses them: a private chain, a TRC20 deploy, and a load run. Both fields were broken, and only one broke visibly.script:— the silent oneThe template text reached the shell verbatim:
That curled a literal
{{ params.node_url }}forever. The step ran, nothing errored, and the recipe hung until I gave up and killed it.Substituting it is not the fix. Interpolating a value into a shell body is command injection by construction — a param of
; rm -rf /would execute.env:is already substituted and the shell sees it as data, so{{ }}insidescript:is now rejected at load time with a message naming that alternative:dir:— the opposite treatmentNow substituted. It becomes
cmd.Dir, a path handed to chdir where a hostile value can only fail to open. No shell, no injection surface.It failed loudly —
chdir {{ params.lab_dir }}/contract: no such file or directory— which is the only reason it was the second bug found rather than the first.The two fields differing is the point, and the rule is where the value lands: a path substitutes, code does not. Both are documented on the struct fields so the next field added has a rule to follow.
Testing
make test -race— 25 packages, 0 failuresmake lint— no findings{{ }}inscript:is rejected and the error namesenv:;env:actually carries substituted values into the shell;dir:substitutesstatus=successin 12.6s, ending in an assertion that TRC20 receivers hold exactlytx_count * amountStill open
kind: hosthas no per-step timeout, which is why thescript:bug presented as an unbounded hang rather than a failure. That was deliberately deferred in #214 (the runner has no timeout for command steps either, and it wants to arrive for both at once) — but this is the second time its absence turned a bug into a wedged run, so it is worth doing.