fix(runtime): tighten run_command metachar reject to ContainsAny set#67
Merged
Conversation
The ad-hoc substring loop in isCommandAllowed missed:
- bare tab (\t) — only "\t&" was rejected; a tab on its own (e.g.
"go\ttest ./...") passed the metachar check then failed the prefix
match, but the inconsistency was a footgun;
- NUL byte (\x00) — "go build\x00evil" passed metachar and failed
prefix, again silent rejection-as-denial rather than rejection-as-
intent;
- bare > and < — only "$" / "$(" caught command substitution but
redirection had no entry;
- \ (backslash) — could be used to escape characters.
Replace the substring loop with one strings.ContainsAny over the
canonical forbidden set: `;&|$`<>\n\r\t\x00\\`. Faster (single linear
scan), and closes the gaps. ; & | $ ` already covered chaining +
expansion + command substitution; > < add redirection; \n \r \t \x00
cover control chars; \\ catches escapes.
7 new table-driven cases in TestIsCommandAllowed_RejectsExtendedMetachars
cover bare tab, NUL, > / <, newline, carriage return, backslash, and
bare $.
Surfaced by the 2026-06-11 security audit (SEC-M3).
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.
Summary
The ad-hoc substring loop in
isCommandAllowedmissed several dangerous characters:\t) — only\t&was rejected;\talone passed the metachar check then failed the prefix match. Inconsistent rejection-as-denial.\x00) —"go build\x00evil"slipped past the metachar list.>and<—$/ ``` caught command substitution but redirection had no entry.\\(backslash) — could escape characters past the prefix match.Changes
Replace the loop with one
strings.ContainsAny(command, ";&|$\<>\n\r\t\x00\\"). Faster (single linear scan), and closes the gaps.;already covered chaining + expansion + command substitution;>add redirection;\ncover control chars;\` catches escapes.Test plan
TestIsCommandAllowed_RejectsExtendedMetachars: bare tab, NUL, redirection out/in, newline, carriage return, backslash, bare$.go build ./...,go vet ./...,go test ./... -count=1 -timeout 240sall green locally.Audit traceability
Security finding SEC-M3 (2026-06-11 sweep).