bug: better errors handling

Author: denis256

pkg/config/config_helpers.go

@@ -536,6 +536,11 @@ func runCommandImpl(ctx context.Context, pctx *ParsingContext, l log.Logger, arg
 		}
 	}
 
+	// Re-check after option-stripping; otherwise args[0] / args[1:] below panics.
+	if len(args) == 0 {
+		return "", errors.New(EmptyStringNotAllowedError("command parameter to the run_cmd function (only option flags were supplied)"))
+	}
+
 	// To avoid re-run of the same run_cmd command, is used in memory cache for command results, with caching key path + arguments
 	// see: https://github.com/gruntwork-io/terragrunt/issues/1427
 	cacheKey := fmt.Sprintf("%v-%v", cachePath, args)
@@ -1440,8 +1445,8 @@ func StartsWith(ctx context.Context, pctx *ParsingContext, args []string) (bool,
 	var result bool
 
 	err := telemetry.TelemeterFromContext(ctx).Collect(ctx, "hcl_fn_startswith", attrs, func(childCtx context.Context) error {
-		if len(args) == 0 {
-			return errors.New(EmptyStringNotAllowedError("parameter to the startswith function"))
+		if len(args) != matchedPats {
+			return errors.New(WrongNumberOfParamsError{Func: FuncNameStartsWith, Expected: "2", Actual: len(args)})
 		}
 
 		str := args[0]
@@ -1474,8 +1479,8 @@ func EndsWith(ctx context.Context, pctx *ParsingContext, args []string) (bool, e
 	var result bool
 
 	err := telemetry.TelemeterFromContext(ctx).Collect(ctx, "hcl_fn_endswith", attrs, func(childCtx context.Context) error {
-		if len(args) == 0 {
-			return errors.New(EmptyStringNotAllowedError("parameter to the endswith function"))
+		if len(args) != matchedPats {
+			return errors.New(WrongNumberOfParamsError{Func: FuncNameEndsWith, Expected: "2", Actual: len(args)})
 		}
 
 		str := args[0]
@@ -1555,8 +1560,8 @@ func StrContains(ctx context.Context, pctx *ParsingContext, args []string) (bool
 	var result bool
 
 	err := telemetry.TelemeterFromContext(ctx).Collect(ctx, "hcl_fn_strcontains", attrs, func(childCtx context.Context) error {
-		if len(args) == 0 {
-			return errors.New(EmptyStringNotAllowedError("parameter to the strcontains function"))
+		if len(args) != matchedPats {
+			return errors.New(WrongNumberOfParamsError{Func: FuncNameStrContains, Expected: "2", Actual: len(args)})
 		}
 
 		str := args[0]

pkg/config/config_helpers_test.go

@@ -1401,3 +1401,127 @@ func TestConstraintCheck(t *testing.T) {
 		})
 	}
 }
+
+// TestStartsWithArityRegression: startswith with wrong arity must return WrongNumberOfParamsError, not panic.
+func TestStartsWithArityRegression(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		name string
+		args []string
+	}{
+		{name: "no args", args: []string{}},
+		{name: "one arg (the bug trigger)", args: []string{"foo"}},
+		{name: "three args", args: []string{"foo", "bar", "baz"}},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			ctx, pctx := newTestParsingContext(t, "")
+
+			require.NotPanics(t, func() {
+				_, err := config.StartsWith(ctx, pctx, tc.args)
+				require.Error(t, err, "must return error for wrong arity (%d args)", len(tc.args))
+				require.True(t, assertErrorType(t, config.WrongNumberOfParamsError{}, err),
+					"expected WrongNumberOfParamsError, got %T: %v", err, err)
+			}, "startswith with %d args must not panic", len(tc.args))
+		})
+	}
+}
+
+// TestEndsWithArityRegression: endswith with wrong arity must return WrongNumberOfParamsError, not panic.
+func TestEndsWithArityRegression(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		name string
+		args []string
+	}{
+		{name: "no args", args: []string{}},
+		{name: "one arg (the bug trigger)", args: []string{"foo"}},
+		{name: "three args", args: []string{"foo", "bar", "baz"}},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			ctx, pctx := newTestParsingContext(t, "")
+
+			require.NotPanics(t, func() {
+				_, err := config.EndsWith(ctx, pctx, tc.args)
+				require.Error(t, err, "must return error for wrong arity (%d args)", len(tc.args))
+				require.True(t, assertErrorType(t, config.WrongNumberOfParamsError{}, err),
+					"expected WrongNumberOfParamsError, got %T: %v", err, err)
+			}, "endswith with %d args must not panic", len(tc.args))
+		})
+	}
+}
+
+// TestStrContainsArityRegression: strcontains with wrong arity must return WrongNumberOfParamsError, not panic.
+func TestStrContainsArityRegression(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		name string
+		args []string
+	}{
+		{name: "no args", args: []string{}},
+		{name: "one arg (the bug trigger)", args: []string{"hello"}},
+		{name: "three args", args: []string{"hello", "world", "extra"}},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			ctx, pctx := newTestParsingContext(t, "")
+
+			require.NotPanics(t, func() {
+				_, err := config.StrContains(ctx, pctx, tc.args)
+				require.Error(t, err, "must return error for wrong arity (%d args)", len(tc.args))
+				require.True(t, assertErrorType(t, config.WrongNumberOfParamsError{}, err),
+					"expected WrongNumberOfParamsError, got %T: %v", err, err)
+			}, "strcontains with %d args must not panic", len(tc.args))
+		})
+	}
+}
+
+// TestRunCommandOptionsOnlyArityRegression: run_cmd with only option flags must return EmptyStringNotAllowedError, not panic.
+func TestRunCommandOptionsOnlyArityRegression(t *testing.T) {
+	t.Parallel()
+
+	if runtime.GOOS == "windows" {
+		t.Skip("Skipping test on Windows because it doesn't support bash")
+	}
+
+	testCases := []struct {
+		name   string
+		params []string
+	}{
+		{name: "single quiet flag", params: []string{"--terragrunt-quiet"}},
+		{name: "single no-cache flag", params: []string{"--terragrunt-no-cache"}},
+		{name: "single global-cache flag", params: []string{"--terragrunt-global-cache"}},
+		{name: "two compatible flags", params: []string{"--terragrunt-quiet", "--terragrunt-no-cache"}},
+		{name: "two compatible flags reversed", params: []string{"--terragrunt-no-cache", "--terragrunt-quiet"}},
+		{name: "duplicate quiet", params: []string{"--terragrunt-quiet", "--terragrunt-quiet"}},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			l := logger.CreateLogger()
+			ctx, pctx := newTestParsingContext(t, "")
+
+			require.NotPanics(t, func() {
+				_, err := config.RunCommand(ctx, pctx, l, tc.params)
+				require.Error(t, err, "must return error when only option flags are supplied (%v)", tc.params)
+				require.True(t, assertErrorType(t, config.EmptyStringNotAllowedError(""), err),
+					"expected EmptyStringNotAllowedError, got %T: %v", err, err)
+			}, "run_cmd with options-only %v must not panic", tc.params)
+		})
+	}
+}

pkg/config/fuzz_test.go

@@ -0,0 +1,77 @@
+package config_test
+
+import (
+	"runtime"
+	"strings"
+	"testing"
+
+	"github.com/gruntwork-io/terragrunt/pkg/config"
+	"github.com/gruntwork-io/terragrunt/test/helpers/logger"
+)
+
+// FuzzHCLStringHelpers: startswith / endswith / strcontains must never panic on any args shape.
+func FuzzHCLStringHelpers(f *testing.F) {
+	seeds := []string{
+		"",
+		"foo",
+		"foo\x00bar",
+		"foo\x00bar\x00baz",
+		"\x00",
+		"\x00\x00",
+		"a\x00b\x00c\x00d\x00e",
+		"hello world\x00world",
+		"hello world\x00hello",
+		"hello world\x00wor",
+	}
+	for _, s := range seeds {
+		f.Add(s)
+	}
+
+	f.Fuzz(func(t *testing.T, raw string) {
+		var args []string
+		if raw != "" {
+			args = strings.Split(raw, "\x00")
+		}
+
+		ctx, pctx := newTestParsingContext(t, "")
+
+		_, _ = config.StartsWith(ctx, pctx, args)
+		_, _ = config.EndsWith(ctx, pctx, args)
+		_, _ = config.StrContains(ctx, pctx, args)
+	})
+}
+
+// FuzzHCLRunCommandOptions: run_cmd must never panic on any mix of option flags and commands.
+func FuzzHCLRunCommandOptions(f *testing.F) {
+	if runtime.GOOS == "windows" {
+		f.Skip("run_cmd happy-path requires bash; skip on Windows")
+	}
+
+	seeds := []string{
+		"--terragrunt-quiet",
+		"--terragrunt-no-cache",
+		"--terragrunt-global-cache",
+		"--terragrunt-quiet\x00--terragrunt-no-cache",
+		"--terragrunt-quiet\x00--terragrunt-quiet",
+		"--terragrunt-quiet\x00/bin/echo\x00hi",
+		"/bin/echo\x00hi",
+		"--terragrunt-no-cache\x00--terragrunt-global-cache",
+		"--unknown-flag",
+		"",
+	}
+	for _, s := range seeds {
+		f.Add(s)
+	}
+
+	f.Fuzz(func(t *testing.T, raw string) {
+		var args []string
+		if raw != "" {
+			args = strings.Split(raw, "\x00")
+		}
+
+		l := logger.CreateLogger()
+		ctx, pctx := newTestParsingContext(t, "")
+
+		_, _ = config.RunCommand(ctx, pctx, l, args)
+	})
+}