Merge pull request moby#4889 from jsternberg/lint-json-args-required-…

…sans-shell

linter: add lint rule for required json arguments

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",