Repository navigation
docs: specify command execution as two tiers - #565
Merged
Merged
Conversation
A command is an argv vector. A script is a file that an argv vector runs. There is no third thing, and in particular no command string assembled by concatenation and handed to an interpreter's -c. The rule removes a defect class rather than defending against it. Today three places build command strings by concatenation, and each is exploitable, broken, or both: pkg/platform reaches sudo -n bash -c through seventy concatenating call sites while ParsePURL validates no charset and decodes %3B to a semicolon; shell.exec and powershell.exec each take an opaque string; and two fixtures interpolated a native path into an sh -c string, where windows backslashes were consumed as escapes. Tier 1 is exec.Command with an argv vector. All seventy platform commands fit it once two habits are unlearned: format strings quoted only to hide them from sh are plain arguments, and a pipe whose right side is a text utility is a Go expression -- grep is strings.Contains, wc -l is a line count. Tier 2 accepts a script as a string, writes it to a file, and runs it through tier 1. Pipes, loops and redirects work because they are script content rather than argv. Its body stays static and values arrive as $1, $2: interpolating into the body would relocate the escaping bug rather than remove it, since script text is still shell source. Static bodies also need no quoter for either dialect, which matters because sh and PowerShell disagree about how to escape a quote. The file is created through the scratch root rather than os.CreateTemp, for confinement, a self-removing lifetime, and the restrictive mode a script carrying a secret needs on both platforms. The executable bit is never set; the interpreter is named explicitly. Result.Command carries the argv vector for tier 1 and the script source for tier 2 -- never the temp path, which is unauditable once scratch is reclaimed, and never a joined string, which is not what the kernel received. Five questions are left open rather than assumed, the sharpest being whether shell.exec(command=) survives at all: it is the only shape that requires escaping, so keeping it keeps the defect class alive, and retiring it breaks an authoring surface. Refs #561.
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.
A command is an argv vector. A script is a file that an argv vector runs. No third thing, and in particular
no command string assembled by concatenation and handed to an interpreter's
-c.docs/architecture/6.3-command-execution.md, with its status companion, indexed after6.2.Why now
Three places build command strings by concatenation, and each is exploitable, broken, or both:
pkg/platform— 70 concatenating call sites reachingsudo -n bash -c, withParsePURLapplying no charsetvalidation and decoding
%3Bto;shell.exec/powershell.exec— both take an opaque string, with different escaping dialects, so onequoting helper cannot serve both
sh -cstring, where Windows backslashes were consumedas escapes
The two tiers
Tier 1 is
exec.Commandwith an argv vector. All 70 platform commands fit, once two habits are unlearned:a format string quoted only to hide it from
shis a plain argument, and a pipe whose right-hand side is a textutility is a Go expression —
| grepisstrings.Contains,| wc -lis a line count.Tier 2 takes a script string, writes it to a file, and runs it through tier 1. Pipes, loops and redirects
work because they're script content.
Tier 2's body stays static; values arrive as
$1,$2. This is the half that makes it safe rather thanmerely tidier — interpolating into the body would relocate the escaping bug, since script text is still shell
source. It also means no quoter is needed in either dialect.
Two details worth the review
os.CreateTemp— confinement, self-removing lifetime, and therestrictive mode a script carrying a secret needs on both platforms. The executable bit is never set; the
interpreter is named explicitly.
Result.Commandcarries the argv vector for tier 1 and the script source for tier 2. Never the temppath, which is unauditable once scratch is reclaimed; never a joined string, which isn't what the kernel
received.
Open
Five questions are recorded rather than assumed. The sharpest: does
shell.exec(command=…)survive? It's theonly shape requiring escaping, so keeping it keeps the defect class alive, and retiring it breaks an authoring
surface.
#561'spkg/platformrewrite is tier 1 and can proceed as the first delivery without waiting for thetiers to be built.
Refs #561.