Skip to content

Commit 5c38994

Browse files
committed
fix(run-command): prevent nil pointer dereference on missing params flag
relates to #1289
1 parent 19b9b6d commit 5c38994

File tree

4 files changed

+40
-17
lines changed

4 files changed

+40
-17
lines changed

internal/cmd/server/command/create/create.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,14 +119,15 @@ func parseInput(p *print.Printer, cmd *cobra.Command, _ []string) (*inputModel,
119119
CommandTemplateName: flags.FlagToStringValue(p, cmd, commandTemplateNameFlag),
120120
Params: flags.FlagToStringToStringPointer(p, cmd, paramsFlag),
121121
}
122-
parsedParams, err := runcommandUtils.ParseScriptParams(*model.Params)
122+
123+
var err error
124+
model.Params, err = runcommandUtils.ParseScriptParams(model.Params)
123125
if err != nil {
124126
return nil, &cliErr.FlagValidationError{
125127
Flag: paramsFlag,
126128
Details: err.Error(),
127129
}
128130
}
129-
model.Params = &parsedParams
130131

131132
p.DebugInputModel(model)
132133
return &model, nil

internal/cmd/server/command/create/create_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,16 @@ func TestParseInput(t *testing.T) {
105105
flagValues: map[string]string{},
106106
isValid: false,
107107
},
108+
{
109+
description: "params flag missing",
110+
flagValues: fixtureFlagValues(func(flagValues map[string]string) {
111+
delete(flagValues, paramsFlag)
112+
}),
113+
isValid: true,
114+
expectedModel: fixtureInputModel(func(model *inputModel) {
115+
model.Params = nil
116+
}),
117+
},
108118
{
109119
description: "project id missing",
110120
flagValues: fixtureFlagValues(func(flagValues map[string]string) {

internal/pkg/services/runcommand/utils/utils.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,15 @@ import (
55
"strings"
66
)
77

8-
func ParseScriptParams(params map[string]string) (map[string]string, error) {
8+
func ParseScriptParams(params *map[string]string) (*map[string]string, error) {
99
if params == nil {
1010
return nil, nil
1111
}
12+
1213
parsed := map[string]string{}
13-
for k, v := range params {
14+
for k, v := range *params {
1415
parsed[k] = v
16+
1517
if k == "script" && strings.HasPrefix(v, "@{") && strings.HasSuffix(v, "}") {
1618
// Check if a script file path was specified, like: --params script=@{/tmp/test.sh}
1719
fileContents, err := os.ReadFile(v[2 : len(v)-1])
@@ -21,5 +23,6 @@ func ParseScriptParams(params map[string]string) (map[string]string, error) {
2123
parsed[k] = string(fileContents)
2224
}
2325
}
24-
return parsed, nil
26+
27+
return &parsed, nil
2528
}

internal/pkg/services/runcommand/utils/utils_test.go

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,26 +2,34 @@ package utils
22

33
import (
44
"testing"
5+
6+
"github.com/google/go-cmp/cmp"
57
)
68

79
func TestParseScriptParams(t *testing.T) {
810
tests := []struct {
911
description string
10-
input map[string]string
11-
expectedOutput map[string]string
12+
input *map[string]string
13+
expectedOutput *map[string]string
1214
isValid bool
1315
}{
1416
{
15-
"base-ok",
16-
map[string]string{"script": "ls /"},
17-
map[string]string{"script": "ls /"},
18-
true,
17+
description: "base-ok",
18+
input: &map[string]string{"script": "ls /"},
19+
expectedOutput: &map[string]string{"script": "ls /"},
20+
isValid: true,
21+
},
22+
{
23+
description: "nil input",
24+
input: nil,
25+
expectedOutput: nil,
26+
isValid: true,
1927
},
2028
{
21-
"not-ok-nonexistant-file-specified-for-script",
22-
map[string]string{"script": "@{/some/file/which/does/not/exist/and/thus/fails}"},
23-
nil,
24-
false,
29+
description: "not-ok-nonexistant-file-specified-for-script",
30+
input: &map[string]string{"script": "@{/some/file/which/does/not/exist/and/thus/fails}"},
31+
expectedOutput: nil,
32+
isValid: false,
2533
},
2634
}
2735

@@ -38,8 +46,9 @@ func TestParseScriptParams(t *testing.T) {
3846
if !tt.isValid {
3947
return
4048
}
41-
if output["script"] != tt.expectedOutput["script"] {
42-
t.Errorf("expected output to be %s, got %s", tt.expectedOutput["script"], output["script"])
49+
diff := cmp.Diff(output, tt.expectedOutput)
50+
if diff != "" {
51+
t.Fatalf("ParseScriptParams() output mismatch (-want +got):\n%s", diff)
4352
}
4453
})
4554
}

0 commit comments

Comments
 (0)