Merge pull request #4903 from jsternberg/lint-workdir-relative-path

linter: add rule for relative path used in workdir

frontend/dockerfile/dockerfile2llb/convert.go

@@ -549,18 +549,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
 		if !isReachable(target, d) || d.noinit {
 			continue
 		}
-		// mark as initialized, used to determine states that have not been dispatched yet
-		d.noinit = true
-
-		if d.base != nil {
-			d.state = d.base.state
-			d.platform = d.base.platform
-			d.image = clone(d.base.image)
-			d.baseImg = cloneX(d.base.baseImg)
-			// Utilize the same path index as our base image so we propagate
-			// the paths we use back to the base image.
-			d.paths = d.base.paths
-		}
+		d.init()
 
 		// Ensure platform is set.
 		if d.platform == nil {
@@ -908,12 +897,35 @@ type dispatchState struct {
 	epoch          *time.Time
 	scanStage      bool
 	scanContext    bool
+	// workdirSet is set to true if a workdir has been set
+	// within the current dockerfile.
+	workdirSet bool
 }
 
 func (ds *dispatchState) asyncLocalOpts() []llb.LocalOption {
 	return filterPaths(ds.paths)
 }
 
+// init is invoked when the dispatch state inherits its attributes
+// from the base image.
+func (ds *dispatchState) init() {
+	// mark as initialized, used to determine states that have not been dispatched yet
+	ds.noinit = true
+
+	if ds.base == nil {
+		return
+	}
+
+	ds.state = ds.base.state
+	ds.platform = ds.base.platform
+	ds.image = clone(ds.base.image)
+	ds.baseImg = cloneX(ds.base.baseImg)
+	// Utilize the same path index as our base image so we propagate
+	// the paths we use back to the base image.
+	ds.paths = ds.base.paths
+	ds.workdirSet = ds.base.workdirSet
+}
+
 type dispatchStates struct {
 	states       []*dispatchState
 	statesByName map[string]*dispatchState
@@ -1128,6 +1140,23 @@ func dispatchRun(d *dispatchState, c *instructions.RunCommand, proxy *llb.ProxyE
 }
 
 func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bool, opt *dispatchOpt) error {
+	if commit {
+		// This linter rule checks if workdir has been set to an absolute value locally
+		// within the current dockerfile. Absolute paths in base images are ignored
+		// because they might change and it is not advised to rely on them.
+		//
+		// We only run this check when commit is true. Commit is true when we are performing
+		// this operation on a local call to workdir rather than one coming from
+		// the base image. We only check the first instance of workdir being set
+		// so successive relative paths are ignored because every instance is fixed
+		// by fixing the first one.
+		if !d.workdirSet && !system.IsAbs(c.Path, d.platform.OS) {
+			msg := linter.RuleWorkdirRelativePath.Format(c.Path)
+			linter.RuleWorkdirRelativePath.Run(opt.lintWarn, c.Location(), msg)
+		}
+		d.workdirSet = true
+	}
+
 	wd, err := system.NormalizeWorkdir(d.image.Config.WorkingDir, c.Path, d.platform.OS)
 	if err != nil {
 		return errors.Wrap(err, "normalizing workdir")

frontend/dockerfile/dockerfile_lint_test.go

@@ -32,6 +32,7 @@ var lintTests = integration.TestFuncs(
 	testMaintainerDeprecated,
 	testWarningsBeforeError,
 	testUndeclaredArg,
+	testWorkdirRelativePath,
 )
 
 func testStageName(t *testing.T, sb integration.Sandbox) {
@@ -487,6 +488,34 @@ COPY Dockerfile .
 	})
 }
 
+func testWorkdirRelativePath(t *testing.T, sb integration.Sandbox) {
+	dockerfile := []byte(`
+FROM scratch
+WORKDIR app/
+`)
+	checkLinterWarnings(t, sb, &lintTestParams{
+		Dockerfile: dockerfile,
+		Warnings: []expectedLintWarning{
+			{
+				RuleName:    "WorkdirRelativePath",
+				Description: "Relative workdir without an absolute workdir declared within the build can have unexpected results if the base image changes",
+				Detail:      "Relative workdir \"app/\" can have unexpected results if the base image changes",
+				Level:       1,
+				Line:        3,
+			},
+		},
+	})
+
+	dockerfile = []byte(`
+FROM scratch AS a
+WORKDIR /app
+
+FROM a AS b
+WORKDIR subdir/
+`)
+	checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
+}
+
 func checkUnmarshal(t *testing.T, sb integration.Sandbox, lintTest *lintTestParams) {
 	destDir, err := os.MkdirTemp("", "buildkit")
 	require.NoError(t, err)

frontend/dockerfile/linter/ruleset.go

@@ -77,4 +77,11 @@ var (
 			return fmt.Sprintf("FROM argument '%s' is not declared", baseArg)
 		},
 	}
+	RuleWorkdirRelativePath = LinterRule[func(workdir string) string]{
+		Name:        "WorkdirRelativePath",
+		Description: "Relative workdir without an absolute workdir declared within the build can have unexpected results if the base image changes",
+		Format: func(workdir string) string {
+			return fmt.Sprintf("Relative workdir %q can have unexpected results if the base image changes", workdir)
+		},
+	}
 )