linter: add lint rule for required json arguments

This lint requires json arguments to be used when the `SHELL` command
isn't used previously for `ENTRYPOINT` and `CMD`. This is because using
non-json arguments can block signals from properly being handled when
used in shell mode.

This uses the `Shell` attribute on the image configuration and therefore
requires the stage to be included in the build for it to properly run.
This is done because we don't want to force image config resolution on
every stage even if the stage isn't part of the build, but the only way
to definitively know if a custom shell is set is to look at the image
configuration.

Signed-off-by: Jonathan A. Sternberg <[email protected]>

frontend/dockerfile/dockerfile2llb/convert.go

@@ -611,6 +611,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
 			cgroupParent:      opt.CgroupParent,
 			llbCaps:           opt.LLBCaps,
 			sourceMap:         opt.SourceMap,
+			lintWarn:          opt.Warn,
 		}
 
 		if err = dispatchOnBuildTriggers(d, d.image.Config.OnBuild, opt); err != nil {
@@ -746,6 +747,7 @@ type dispatchOpt struct {
 	cgroupParent      string
 	llbCaps           *apicaps.CapSet
 	sourceMap         *llb.SourceMap
+	lintWarn          linter.LintWarnFunc
 }
 
 func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error {
@@ -820,9 +822,9 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error {
 	case *instructions.OnbuildCommand:
 		err = dispatchOnbuild(d, c)
 	case *instructions.CmdCommand:
-		err = dispatchCmd(d, c)
+		err = dispatchCmd(d, c, opt.lintWarn)
 	case *instructions.EntrypointCommand:
-		err = dispatchEntrypoint(d, c)
+		err = dispatchEntrypoint(d, c, opt.lintWarn)
 	case *instructions.HealthCheckCommand:
 		err = dispatchHealthcheck(d, c)
 	case *instructions.ExposeCommand:
@@ -1051,6 +1053,8 @@ func dispatchRun(d *dispatchState, c *instructions.RunCommand, proxy *llb.ProxyE
 		}
 	}
 	if c.PrependShell {
+		// Don't pass the linter function because we do not report a warning for
+		// shell usage on run commands.
 		args = withShell(d.image, args)
 	}
 
@@ -1435,9 +1439,13 @@ func dispatchOnbuild(d *dispatchState, c *instructions.OnbuildCommand) error {
 	return nil
 }
 
-func dispatchCmd(d *dispatchState, c *instructions.CmdCommand) error {
+func dispatchCmd(d *dispatchState, c *instructions.CmdCommand, warn linter.LintWarnFunc) error {
 	var args []string = c.CmdLine
 	if c.PrependShell {
+		if len(d.image.Config.Shell) == 0 {
+			msg := linter.RuleJSONArgsRecommended.Format(c.Name())
+			linter.RuleJSONArgsRecommended.Run(warn, c.Location(), msg)
+		}
 		args = withShell(d.image, args)
 	}
 	d.image.Config.Cmd = args
@@ -1446,9 +1454,13 @@ func dispatchCmd(d *dispatchState, c *instructions.CmdCommand) error {
 	return commitToHistory(&d.image, fmt.Sprintf("CMD %q", args), false, nil, d.epoch)
 }
 
-func dispatchEntrypoint(d *dispatchState, c *instructions.EntrypointCommand) error {
+func dispatchEntrypoint(d *dispatchState, c *instructions.EntrypointCommand, warn linter.LintWarnFunc) error {
 	var args []string = c.CmdLine
 	if c.PrependShell {
+		if len(d.image.Config.Shell) == 0 {
+			msg := linter.RuleJSONArgsRecommended.Format(c.Name())
+			linter.RuleJSONArgsRecommended.Run(warn, c.Location(), msg)
+		}
 		args = withShell(d.image, args)
 	}
 	d.image.Config.Entrypoint = args

frontend/dockerfile/dockerfile_lint_test.go

@@ -28,6 +28,7 @@ var lintTests = integration.TestFuncs(
 	testFileConsistentCommandCasing,
 	testDuplicateStageName,
 	testReservedStageName,
+	testJSONArgsRecommended,
 	testMaintainerDeprecated,
 	testWarningsBeforeError,
 	testUndeclaredArg,
@@ -302,6 +303,74 @@ FROM scratch AS a
 	checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
 }
 
+func testJSONArgsRecommended(t *testing.T, sb integration.Sandbox) {
+	dockerfile := []byte(`
+FROM scratch
+CMD mycommand
+`)
+	checkLinterWarnings(t, sb, &lintTestParams{
+		Dockerfile: dockerfile,
+		Warnings: []expectedLintWarning{
+			{
+				RuleName:    "JSONArgsRecommended",
+				Description: "JSON arguments recommended for ENTRYPOINT/CMD to prevent unintended behavior related to OS signals",
+				Detail:      "JSON arguments recommended for CMD to prevent unintended behavior related to OS signals",
+				Level:       1,
+				Line:        3,
+			},
+		},
+	})
+
+	dockerfile = []byte(`
+FROM scratch
+ENTRYPOINT mycommand
+`)
+	checkLinterWarnings(t, sb, &lintTestParams{
+		Dockerfile: dockerfile,
+		Warnings: []expectedLintWarning{
+			{
+				RuleName:    "JSONArgsRecommended",
+				Description: "JSON arguments recommended for ENTRYPOINT/CMD to prevent unintended behavior related to OS signals",
+				Detail:      "JSON arguments recommended for ENTRYPOINT to prevent unintended behavior related to OS signals",
+				Level:       1,
+				Line:        3,
+			},
+		},
+	})
+
+	dockerfile = []byte(`
+FROM scratch
+SHELL ["/usr/bin/customshell"]
+CMD mycommand
+`)
+	checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
+
+	dockerfile = []byte(`
+FROM scratch
+SHELL ["/usr/bin/customshell"]
+ENTRYPOINT mycommand
+`)
+	checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
+
+	dockerfile = []byte(`
+FROM scratch AS base
+SHELL ["/usr/bin/customshell"]
+
+FROM base
+CMD mycommand
+`)
+	checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
+
+	dockerfile = []byte(`
+FROM scratch AS base
+SHELL ["/usr/bin/customshell"]
+
+FROM base
+ENTRYPOINT mycommand
+`)
+	checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
+}
+
 func testMaintainerDeprecated(t *testing.T, sb integration.Sandbox) {
 	dockerfile := []byte(`
 FROM scratch

frontend/dockerfile/linter/ruleset.go

@@ -55,6 +55,13 @@ var (
 			return fmt.Sprintf("Stage name should not use the same name as reserved stage %q", reservedStageName)
 		},
 	}
+	RuleJSONArgsRecommended = LinterRule[func(instructionName string) string]{
+		Name:        "JSONArgsRecommended",
+		Description: "JSON arguments recommended for ENTRYPOINT/CMD to prevent unintended behavior related to OS signals",
+		Format: func(instructionName string) string {
+			return fmt.Sprintf("JSON arguments recommended for %s to prevent unintended behavior related to OS signals", instructionName)
+		},
+	}
 	RuleMaintainerDeprecated = LinterRule[func() string]{
 		Name:        "MaintainerDeprecated",
 		Description: "The maintainer instruction is deprecated, use a label instead to define an image author",