Skip to content

Commit 40cc978

Browse files
ibuildthecloudibuildthecloud
committed
bug: change quoting behavior
When reading the #! interpreter line variables in quotes "${FOO}" will be evaluated and passed as a single argument to the command. Unquoted variables like ${FOO} may result in multiple argument. For example, if FOO="Hello World" that will result in two arguments "Hello" and "World", where "${FOO}" will result in one argument "Hello World"
1 parent dc4dcfc commit 40cc978

File tree

2 files changed

+228
-0
lines changed

2 files changed

+228
-0
lines changed

Diff for: ‎pkg/engine/cmd.go

+93
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,9 @@ func (e *Engine) runCommand(ctx Context, tool types.Tool, input string, toolCate
123123
}
124124
cmd, stop, err := e.newCommand(ctx.Ctx, extraEnv, tool, input)
125125
if err != nil {
126+
if toolCategory == NoCategory {
127+
return fmt.Sprintf("ERROR: got (%v) while parsing command", err), nil
128+
}
126129
return "", err
127130
}
128131
defer stop()
@@ -268,6 +271,12 @@ func (e *Engine) newCommand(ctx context.Context, extraEnv []string, tool types.T
268271
})
269272
}
270273

274+
// After we determined the interpreter we again interpret the args by env vars
275+
args, err = replaceVariablesForInterpreter(interpreter, envMap)
276+
if err != nil {
277+
return nil, nil, err
278+
}
279+
271280
if runtime.GOOS == "windows" && (args[0] == "/bin/bash" || args[0] == "/bin/sh") {
272281
args[0] = path.Base(args[0])
273282
}
@@ -314,3 +323,87 @@ func (e *Engine) newCommand(ctx context.Context, extraEnv []string, tool types.T
314323
cmd.Env = compressEnv(envvars)
315324
return cmd, stop, nil
316325
}
326+
327+
func replaceVariablesForInterpreter(interpreter string, envMap map[string]string) ([]string, error) {
328+
var parts []string
329+
for i, part := range splitByQuotes(interpreter) {
330+
if i%2 == 0 {
331+
part = os.Expand(part, func(s string) string {
332+
return envMap[s]
333+
})
334+
// We protect newly resolved env vars from getting replaced when we do the second Expand
335+
// after shlex. Yeah, crazy. I'm guessing this isn't secure, but just trying to avoid a foot gun.
336+
part = os.Expand(part, func(s string) string {
337+
return "${__" + s + "}"
338+
})
339+
}
340+
parts = append(parts, part)
341+
}
342+
343+
parts, err := shlex.Split(strings.Join(parts, ""))
344+
if err != nil {
345+
return nil, err
346+
}
347+
348+
for i, part := range parts {
349+
parts[i] = os.Expand(part, func(s string) string {
350+
if strings.HasPrefix(s, "__") {
351+
return "${" + s[2:] + "}"
352+
}
353+
return envMap[s]
354+
})
355+
}
356+
357+
return parts, nil
358+
}
359+
360+
// splitByQuotes will split a string by parsing matching double quotes (with \ as the escape character).
361+
// The return value conforms to the following properties
362+
// 1. s == strings.Join(result, "")
363+
// 2. Even indexes are strings that were not in quotes.
364+
// 3. Odd indexes are strings that were quoted.
365+
//
366+
// Example: s = `In a "quoted string" quotes can be escaped with \"`
367+
//
368+
// result = [`In a `, `"quoted string"`, ` quotes can be escaped with \"`]
369+
func splitByQuotes(s string) (result []string) {
370+
var (
371+
buf strings.Builder
372+
inEscape, inQuote bool
373+
)
374+
375+
for _, c := range s {
376+
if inEscape {
377+
buf.WriteRune(c)
378+
inEscape = false
379+
continue
380+
}
381+
382+
switch c {
383+
case '"':
384+
if inQuote {
385+
buf.WriteRune(c)
386+
}
387+
result = append(result, buf.String())
388+
buf.Reset()
389+
if !inQuote {
390+
buf.WriteRune(c)
391+
}
392+
inQuote = !inQuote
393+
case '\\':
394+
inEscape = true
395+
buf.WriteRune(c)
396+
default:
397+
buf.WriteRune(c)
398+
}
399+
}
400+
401+
if buf.Len() > 0 {
402+
if inQuote {
403+
result = append(result, "")
404+
}
405+
result = append(result, buf.String())
406+
}
407+
408+
return
409+
}

Diff for: ‎pkg/engine/cmd_test.go

+135
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
// File: cmd_test.go
2+
package engine
3+
4+
import "testing"
5+
6+
func TestSplitByQuotes(t *testing.T) {
7+
tests := []struct {
8+
name string
9+
input string
10+
expected []string
11+
}{
12+
{
13+
name: "NoQuotes",
14+
input: "Hello World",
15+
expected: []string{"Hello World"},
16+
},
17+
{
18+
name: "ValidQuote",
19+
input: `"Hello" "World"`,
20+
expected: []string{``, `"Hello"`, ` `, `"World"`},
21+
},
22+
{
23+
name: "ValidQuoteWithEscape",
24+
input: `"Hello\" World"`,
25+
expected: []string{``, `"Hello\" World"`},
26+
},
27+
{
28+
name: "Nothing",
29+
input: "",
30+
expected: []string{},
31+
},
32+
{
33+
name: "SpaceInsideQuote",
34+
input: `"Hello World"`,
35+
expected: []string{``, `"Hello World"`},
36+
},
37+
{
38+
name: "SingleChar",
39+
input: "H",
40+
expected: []string{"H"},
41+
},
42+
{
43+
name: "SingleQuote",
44+
input: `"Hello`,
45+
expected: []string{``, ``, `"Hello`},
46+
},
47+
{
48+
name: "ThreeQuotes",
49+
input: `Test "Hello "World" End\"`,
50+
expected: []string{`Test `, `"Hello "`, `World`, ``, `" End\"`},
51+
},
52+
}
53+
54+
for _, tt := range tests {
55+
t.Run(tt.name, func(t *testing.T) {
56+
got := splitByQuotes(tt.input)
57+
if !equal(got, tt.expected) {
58+
t.Errorf("splitByQuotes() = %v, want %v", got, tt.expected)
59+
}
60+
})
61+
}
62+
}
63+
64+
// Helper function to assert equality of two string slices.
65+
func equal(a, b []string) bool {
66+
if len(a) != len(b) {
67+
return false
68+
}
69+
for i, v := range a {
70+
if v != b[i] {
71+
return false
72+
}
73+
}
74+
return true
75+
}
76+
77+
// Testing for replaceVariablesForInterpreter
78+
func TestReplaceVariablesForInterpreter(t *testing.T) {
79+
tests := []struct {
80+
name string
81+
interpreter string
82+
envMap map[string]string
83+
expected []string
84+
shouldFail bool
85+
}{
86+
{
87+
name: "No quotes",
88+
interpreter: "/bin/bash -c ${COMMAND} tail",
89+
envMap: map[string]string{"COMMAND": "echo Hello!"},
90+
expected: []string{"/bin/bash", "-c", "echo", "Hello!", "tail"},
91+
},
92+
{
93+
name: "Quotes Variables",
94+
interpreter: `/bin/bash -c "${COMMAND}" tail`,
95+
envMap: map[string]string{"COMMAND": "Hello, World!"},
96+
expected: []string{"/bin/bash", "-c", "Hello, World!", "tail"},
97+
},
98+
{
99+
name: "Double escape",
100+
interpreter: `/bin/bash -c "${COMMAND}" ${TWO} tail`,
101+
envMap: map[string]string{
102+
"COMMAND": "Hello, World!",
103+
"TWO": "${COMMAND}",
104+
},
105+
expected: []string{"/bin/bash", "-c", "Hello, World!", "${COMMAND}", "tail"},
106+
},
107+
{
108+
name: "aws cli issue",
109+
interpreter: "aws ${ARGS}",
110+
envMap: map[string]string{
111+
"ARGS": `ec2 describe-instances --region us-east-1 --query 'Reservations[*].Instances[*].{Instance:InstanceId,State:State.Name}'`,
112+
},
113+
expected: []string{
114+
`aws`,
115+
`ec2`,
116+
`describe-instances`,
117+
`--region`, `us-east-1`,
118+
`--query`, `Reservations[*].Instances[*].{Instance:InstanceId,State:State.Name}`,
119+
},
120+
},
121+
}
122+
123+
for _, tt := range tests {
124+
t.Run(tt.name, func(t *testing.T) {
125+
got, err := replaceVariablesForInterpreter(tt.interpreter, tt.envMap)
126+
if (err != nil) != tt.shouldFail {
127+
t.Errorf("replaceVariablesForInterpreter() error = %v, want %v", err, tt.shouldFail)
128+
return
129+
}
130+
if !equal(got, tt.expected) {
131+
t.Errorf("replaceVariablesForInterpreter() = %v, want %v", got, tt.expected)
132+
}
133+
})
134+
}
135+
}

0 commit comments

Comments
 (0)