diff --git a/CHANGELOG.md b/CHANGELOG.md index 50dfd9b58..87ffdb1cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ ### Bug Fixes: +- fix(compute): Add bounds checking around starter kit selection to avoid a panic and provide a more useful error message ([#1879](https://github.com/fastly/cli/pull/1879)) + ### Enhancements: ### Dependencies: diff --git a/pkg/commands/compute/init.go b/pkg/commands/compute/init.go index 9f0be5356..9c0fdcceb 100644 --- a/pkg/commands/compute/init.go +++ b/pkg/commands/compute/init.go @@ -817,6 +817,9 @@ func (c *InitCommand) PromptForStarterKit(kits []config.StarterKit, in io.Reader var i int if i, err = strconv.Atoi(option); err == nil { + if i < 1 || i > len(kits) { + return "", "", "", fmt.Errorf("invalid starter kit option: %s", option) + } template := kits[i-1] return template.Path, template.Branch, template.Tag, nil } @@ -831,7 +834,7 @@ func validateTemplateOptionOrURL(templates []config.StarterKit) func(string) err return nil } if option, err := strconv.Atoi(input); err == nil { - if option > len(templates) { + if option < 1 || option > len(templates) { return errors.New(msg) } return nil diff --git a/pkg/commands/compute/init_test.go b/pkg/commands/compute/init_test.go index 1669e3af2..3cd7cc3fa 100644 --- a/pkg/commands/compute/init_test.go +++ b/pkg/commands/compute/init_test.go @@ -13,6 +13,8 @@ import ( "github.com/fastly/go-fastly/v17/fastly" "github.com/fastly/cli/pkg/app" + "github.com/fastly/cli/pkg/argparser" + "github.com/fastly/cli/pkg/commands/compute" "github.com/fastly/cli/pkg/config" "github.com/fastly/cli/pkg/global" "github.com/fastly/cli/pkg/manifest" @@ -847,3 +849,118 @@ func TestInit_ExistingService(t *testing.T) { }) } } + +// TestPromptForStarterKitBounds verifies that bounds checks are applied to +// starter kit selection in the interactive mode prompt. +func TestPromptForStarterKitBounds(t *testing.T) { + kits := []config.StarterKit{ + { + Name: "Default", + Path: "https://github.com/fastly/compute-starter-kit-rust-default", + Branch: "main", + }, + { + Name: "Empty", + Path: "https://github.com/fastly/compute-starter-kit-rust-empty", + Branch: "main", + }, + } + + scenarios := []struct { + name string + // stdin is the input given at the starter kit prompt. An invalid entry + // is rejected and the prompt repeats, so those cases supply a valid + // follow-up value. + stdin string + wantPath string + wantBranch string + // wantRejected asserts the validation message was shown to the user. + wantRejected bool + }{ + { + name: "first option", + stdin: "1\n", + wantPath: kits[0].Path, + wantBranch: "main", + }, + { + name: "last option", + stdin: "2\n", + wantPath: kits[1].Path, + wantBranch: "main", + }, + { + name: "no input defaults to the first option", + stdin: "\n", + wantPath: kits[0].Path, + wantBranch: "main", + }, + { + name: "git URL is passed through", + stdin: "https://github.com/fastly/compute-starter-kit-rust-websockets\n", + wantPath: "https://github.com/fastly/compute-starter-kit-rust-websockets", + }, + { + // Without the lower bound this indexed kits[-1] and panicked. + name: "zero is rejected", + stdin: "0\n1\n", + wantPath: kits[0].Path, + wantBranch: "main", + wantRejected: true, + }, + { + // Without the lower bound this indexed kits[-2] and panicked. + name: "negative is rejected", + stdin: "-1\n2\n", + wantPath: kits[1].Path, + wantBranch: "main", + wantRejected: true, + }, + { + name: "above the upper bound is rejected", + stdin: "3\n1\n", + wantPath: kits[0].Path, + wantBranch: "main", + wantRejected: true, + }, + } + + for _, testcase := range scenarios { + t.Run(testcase.name, func(t *testing.T) { + var stdout threadsafe.Buffer + c := compute.InitCommand{ + Base: argparser.Base{ + Globals: testutil.MockGlobalData(testutil.SplitArgs("compute init"), &stdout), + }, + } + + from, branch, _, err := c.PromptForStarterKit(kits, strings.NewReader(testcase.stdin), &stdout) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + testutil.AssertEqual(t, testcase.wantPath, from) + testutil.AssertEqual(t, testcase.wantBranch, branch) + + if testcase.wantRejected { + testutil.AssertStringContains(t, stdout.String(), "must be a valid option or git URL") + } + }) + } +} + +// TestPromptForStarterKitBoundsNonInteractive verifies that bounds checks are +// applied to starter kit selection when either the AcceptDefaults flag or the +// NonInteractive flag are true, which skips the prompt and prompt validation. +func TestPromptForStarterKitBoundsNonInteractive(t *testing.T) { + var stdout threadsafe.Buffer + g := testutil.MockGlobalData(testutil.SplitArgs("compute init"), &stdout) + g.Flags.AcceptDefaults = true + + c := compute.InitCommand{Base: argparser.Base{Globals: g}} + + // With defaults accepted and no kits configured, the option falls back to + // "1" with an empty slice, which is out of range. + _, _, _, err := c.PromptForStarterKit([]config.StarterKit{}, strings.NewReader(""), &stdout) + testutil.AssertErrorContains(t, err, "no default starter kits configured for this language") +}