Skip to content

Commit 8ff93d8

Browse files
chmeliikchmeliik
committed
pkg/cliwrappers: shell-escape log messages
The commands we execute may include shell control characters (most commonly whitespace). If we log the arguments separated by space, the log messages will look confusing and will not be copy-paste-able into a terminal. Shell-quote the arguments that need quoting to make the log messages copy-paste-able. Use a similar approach as alessio/shellescape [1] or shlex [2] from the Python standard library. Compared to those, use a slighly nicer escape style for single quotes. Our approach: it's => 'it'\''s' Their approach: it's => 'it'"'"'s' [1]: https://github.com/alessio/shellescape/blob/59ee74454256aaf5478a6283be4c480c13fd3152/shellescape.go#L26-L42 [2]: https://github.com/python/cpython/blob/485699216f2186c63f85fc546301e5edbe6b2f22/Lib/shlex.py#L320-L338 Assisted-by: Claude Signed-off-by: Adam Cmiel <acmiel@redhat.com>
1 parent da0cde9 commit 8ff93d8

File tree

6 files changed

+112
-14
lines changed

6 files changed

+112
-14
lines changed

‎pkg/cliwrappers/buildah.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ func (b *BuildahCli) Build(args *BuildahBuildArgs) error {
231231
executable, buildahArgs = args.Wrapper.Wrap(executable, buildahArgs)
232232
}
233233

234-
buildahLog.Debugf("Running command:\n%s %s", executable, strings.Join(buildahArgs, " "))
234+
buildahLog.Debugf("Running command:\n%s", shellJoin(executable, buildahArgs...))
235235

236236
_, _, _, err := b.Executor.ExecuteWithOutput(executable, buildahArgs...)
237237
if err != nil {
@@ -269,7 +269,7 @@ func (b *BuildahCli) Push(args *BuildahPushArgs) (string, error) {
269269
buildahArgs = append(buildahArgs, args.Destination)
270270
}
271271

272-
buildahLog.Debugf("Running command:\nbuildah %s", strings.Join(buildahArgs, " "))
272+
buildahLog.Debugf("Running command:\n%s", shellJoin("buildah", buildahArgs...))
273273

274274
retryer := NewRetryer(func() (string, string, int, error) {
275275
return b.Executor.ExecuteWithOutput("buildah", buildahArgs...)
@@ -306,7 +306,7 @@ func (b *BuildahCli) Pull(args *BuildahPullArgs) error {
306306

307307
buildahArgs := []string{"pull", args.Image}
308308

309-
buildahLog.Debugf("Running command:\nbuildah %s", strings.Join(buildahArgs, " "))
309+
buildahLog.Debugf("Running command:\n%s", shellJoin("buildah", buildahArgs...))
310310

311311
retryer := NewRetryer(func() (string, string, int, error) {
312312
return b.Executor.ExecuteWithOutput("buildah", buildahArgs...)
@@ -344,7 +344,7 @@ func (b *BuildahCli) Inspect(args *BuildahInspectArgs) (string, error) {
344344

345345
buildahArgs := []string{"inspect", "--type", args.Type, args.Name}
346346

347-
buildahLog.Debugf("Running command:\nbuildah %s", strings.Join(buildahArgs, " "))
347+
buildahLog.Debugf("Running command:\n%s", shellJoin("buildah", buildahArgs...))
348348

349349
stdout, stderr, _, err := b.Executor.Execute("buildah", buildahArgs...)
350350
if err != nil {
@@ -390,7 +390,7 @@ type BuildahVersionInfo struct {
390390
func (b *BuildahCli) Version() (BuildahVersionInfo, error) {
391391
buildahArgs := []string{"version", "--json"}
392392

393-
buildahLog.Debugf("Running command:\nbuildah %s", strings.Join(buildahArgs, " "))
393+
buildahLog.Debugf("Running command:\n%s", shellJoin("buildah", buildahArgs...))
394394

395395
stdout, stderr, _, err := b.Executor.Execute("buildah", buildahArgs...)
396396
if err != nil {

‎pkg/cliwrappers/hermeto.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ package cliwrappers
44

55
import (
66
"errors"
7-
"strings"
87

98
"github.com/konflux-ci/konflux-build-cli/pkg/logger"
109
)
@@ -79,7 +78,7 @@ func (hc *HermetoCli) FetchDeps(params *HermetoFetchDepsParams) error {
7978
params.OutputDir,
8079
)
8180

82-
log.Debugf("Executing hermeto %s", strings.Join(args, " "))
81+
log.Debugf("Executing %s", shellJoin("hermeto", args...))
8382
_, _, _, err := hc.Executor.ExecuteWithOutput("hermeto", args...)
8483
return err
8584
}
@@ -108,7 +107,7 @@ func (hc *HermetoCli) GenerateEnv(params *HermetoGenerateEnvParams) error {
108107
params.Output,
109108
}
110109

111-
log.Debugf("Executing hermeto %s", strings.Join(args, " "))
110+
log.Debugf("Executing %s", shellJoin("hermeto", args...))
112111
_, _, _, err := hc.Executor.ExecuteWithOutput("hermeto", args...)
113112
return err
114113
}
@@ -131,7 +130,7 @@ func (hc *HermetoCli) InjectFiles(params *HermetoInjectFilesParams) error {
131130
params.ForOutputDir,
132131
}
133132

134-
log.Debugf("Executing hermeto %s", strings.Join(args, " "))
133+
log.Debugf("Executing %s", shellJoin("hermeto", args...))
135134
_, _, _, err := hc.Executor.ExecuteWithOutput("hermeto", args...)
136135
return err
137136
}

‎pkg/cliwrappers/oras.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package cliwrappers
22

33
import (
44
"fmt"
5-
"strings"
65

76
l "github.com/konflux-ci/konflux-build-cli/pkg/logger"
87
)
@@ -66,7 +65,7 @@ func (b *OrasCli) Push(args *OrasPushArgs) (string, string, error) {
6665
}
6766
orasArgs = append(orasArgs, args.DestinationImage, args.FileName)
6867

69-
orasLog.Debugf("Running command:\noras %s", strings.Join(orasArgs, " "))
68+
orasLog.Debugf("Running command:\n%s", shellJoin("oras", orasArgs...))
7069

7170
stdout, stderr, _, err := b.Executor.ExecuteWithOutput("oras", orasArgs...)
7271

‎pkg/cliwrappers/shell_quote.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
package cliwrappers
2+
3+
import (
4+
"regexp"
5+
"strings"
6+
)
7+
8+
// Use an explicit ASCII set instead of \w, which in Go matches Unicode letters.
9+
// Non-ASCII characters should be quoted to avoid locale-dependent shell behavior.
10+
var shellUnsafe = regexp.MustCompile(`[^a-zA-Z0-9_%+,\-./:=@]`)
11+
12+
// Quotes command arguments as needed and joins them with spaces.
13+
// The output should be human-readable and copy-paste-able into a POSIX shell.
14+
// Try to avoid using this to execute shell commands, the intended use case is logging.
15+
func shellJoin(cmdName string, args ...string) string {
16+
cmd := make([]string, len(args)+1)
17+
cmd[0] = shellQuote(cmdName)
18+
for i, arg := range args {
19+
cmd[i+1] = shellQuote(arg)
20+
}
21+
return strings.Join(cmd, " ")
22+
}
23+
24+
func shellQuote(arg string) string {
25+
if arg == "" || shellUnsafe.MatchString(arg) {
26+
return "'" + strings.ReplaceAll(arg, "'", "'\\''") + "'"
27+
}
28+
return arg
29+
}

‎pkg/cliwrappers/shell_quote_test.go‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
package cliwrappers
2+
3+
import "testing"
4+
5+
func TestShellQuote(t *testing.T) {
6+
tests := []struct {
7+
name string
8+
input string
9+
expected string
10+
}{
11+
{"simple", "hello", "hello"},
12+
{"with space", "hello world", "'hello world'"},
13+
{"with single quote", "it's", `'it'\''s'`},
14+
{"empty string", "", "''"},
15+
{"non-ascii", "čau", "'čau'"},
16+
{"dollar sign", "$HOME", "'$HOME'"},
17+
{"backtick", "`cmd`", "'`cmd`'"},
18+
{"pipe", "a|b", "'a|b'"},
19+
{"semicolon", "a;b", "'a;b'"},
20+
{"ampersand", "a&b", "'a&b'"},
21+
{"parentheses", "(cmd)", "'(cmd)'"},
22+
{"safe chars", "a-b_c+d,e.f/g:h=i@j%k", "a-b_c+d,e.f/g:h=i@j%k"},
23+
}
24+
25+
for _, tt := range tests {
26+
t.Run(tt.name, func(t *testing.T) {
27+
got := shellQuote(tt.input)
28+
if got != tt.expected {
29+
t.Errorf("shellQuote(%q) = %q, want %q", tt.input, got, tt.expected)
30+
}
31+
})
32+
}
33+
}
34+
35+
func TestShellJoin(t *testing.T) {
36+
tests := []struct {
37+
name string
38+
cmd string
39+
args []string
40+
expected string
41+
}{
42+
{
43+
"simple command",
44+
"buildah", []string{"build", "-t", "myimage"},
45+
"buildah build -t myimage",
46+
},
47+
{
48+
"arg with space",
49+
"buildah", []string{"build", "--build-arg", "FOO=hello world"},
50+
"buildah build --build-arg 'FOO=hello world'",
51+
},
52+
{
53+
"no args",
54+
"buildah", nil,
55+
"buildah",
56+
},
57+
{
58+
"multiple special args",
59+
"echo", []string{"it's", "a $var", ""},
60+
`echo 'it'\''s' 'a $var' ''`,
61+
},
62+
}
63+
64+
for _, tt := range tests {
65+
t.Run(tt.name, func(t *testing.T) {
66+
got := shellJoin(tt.cmd, tt.args...)
67+
if got != tt.expected {
68+
t.Errorf("shellJoin(%q, %v) = %q, want %q", tt.cmd, tt.args, got, tt.expected)
69+
}
70+
})
71+
}
72+
}

‎pkg/cliwrappers/skopeo.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"errors"
55
"fmt"
66
"strconv"
7-
"strings"
87

98
l "github.com/konflux-ci/konflux-build-cli/pkg/logger"
109
)
@@ -80,7 +79,7 @@ func (s *SkopeoCli) Copy(args *SkopeoCopyArgs) error {
8079
dockerPrefix := "docker://"
8180
scopeoArgs = append(scopeoArgs, dockerPrefix+args.SourceImage, dockerPrefix+args.DestinationImage)
8281

83-
skopeoLog.Debugf("Running command:\nskopeo %s", strings.Join(scopeoArgs, " "))
82+
skopeoLog.Debugf("Running command:\n%s", shellJoin("skopeo", scopeoArgs...))
8483

8584
retryer := NewRetryer(func() (string, string, int, error) {
8685
return s.Executor.Execute("skopeo", scopeoArgs...)
@@ -136,7 +135,7 @@ func (s *SkopeoCli) Inspect(args *SkopeoInspectArgs) (string, error) {
136135
dockerPrefix := "docker://"
137136
scopeoArgs = append(scopeoArgs, dockerPrefix+args.ImageRef)
138137

139-
skopeoLog.Debugf("Running command:\nskopeo %s", strings.Join(scopeoArgs, " "))
138+
skopeoLog.Debugf("Running command:\n%s", shellJoin("skopeo", scopeoArgs...))
140139

141140
retryer := NewRetryer(func() (string, string, int, error) {
142141
return s.Executor.Execute("skopeo", scopeoArgs...)

0 commit comments

Comments
 (0)