Skip to content

Commit c2d42cb

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 6e1604c commit c2d42cb

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
@@ -225,7 +225,7 @@ func (b *BuildahCli) Build(args *BuildahBuildArgs) error {
225225
// Context directory must be the last argument
226226
buildahArgs = append(buildahArgs, args.ContextDir)
227227

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

230230
_, _, _, err := b.Executor.ExecuteWithOutput("buildah", buildahArgs...)
231231
if err != nil {
@@ -263,7 +263,7 @@ func (b *BuildahCli) Push(args *BuildahPushArgs) (string, error) {
263263
buildahArgs = append(buildahArgs, args.Destination)
264264
}
265265

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

268268
retryer := NewRetryer(func() (string, string, int, error) {
269269
return b.Executor.ExecuteWithOutput("buildah", buildahArgs...)
@@ -300,7 +300,7 @@ func (b *BuildahCli) Pull(args *BuildahPullArgs) error {
300300

301301
buildahArgs := []string{"pull", args.Image}
302302

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

305305
retryer := NewRetryer(func() (string, string, int, error) {
306306
return b.Executor.ExecuteWithOutput("buildah", buildahArgs...)
@@ -338,7 +338,7 @@ func (b *BuildahCli) Inspect(args *BuildahInspectArgs) (string, error) {
338338

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

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

343343
stdout, stderr, _, err := b.Executor.Execute("buildah", buildahArgs...)
344344
if err != nil {
@@ -384,7 +384,7 @@ type BuildahVersionInfo struct {
384384
func (b *BuildahCli) Version() (BuildahVersionInfo, error) {
385385
buildahArgs := []string{"version", "--json"}
386386

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

389389
stdout, stderr, _, err := b.Executor.Execute("buildah", buildahArgs...)
390390
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)