batches: improve handling of non-root steps (#886)

LawnGnome · web-flow · commit c9902bf2accc · 2022-11-17T16:27:22.000-08:00
This changes the default for `src batch exec` (used by SSBC) to run all
step containers as root, and adds a flag to `src batch apply` and `src
batch preview` to optionally do the same thing.

Fixes sourcegraph/sourcegraph#41836.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,8 +13,12 @@ All notable changes to `src-cli` are documented in this file.
 
 ### Added
 
+- Batch specs being run locally with `src batch preview` or `src batch apply` can now be run with the `-run-as-root` flag, which will run all step containers as root instead of the default user for the image. This is off by default. [#886](https://github.com/sourcegraph/src-cli/pull/886)
+
 ### Changed
 
+- Batch specs being run from the server using this version of `src-cli` now run all step containers as root, rather than as the default user for the image. [#886](https://github.com/sourcegraph/src-cli/pull/886)
+
 ### Fixed
 
 ### Removed
diff --git a/cmd/src/batch_common.go b/cmd/src/batch_common.go
@@ -85,6 +85,7 @@ type batchExecuteFlags struct {
 	workspace     string
 	cleanArchives bool
 	skipErrors    bool
+	runAsRoot     bool
 
 	// EXPERIMENTAL
 	textOnly bool
@@ -152,6 +153,11 @@ func newBatchExecuteFlags(flagSet *flag.FlagSet, cacheDir, tempDir string) *batc
 
 	flagSet.BoolVar(verbose, "v", false, "print verbose output")
 
+	flagSet.BoolVar(
+		&caf.runAsRoot, "run-as-root", false,
+		"If true, forces all step containers to run as root.",
+	)
+
 	return caf
 }
 
@@ -382,6 +388,7 @@ func executeBatchSpec(ctx context.Context, ui ui.ExecUI, opts executeBatchSpecOp
 				Timeout:             opts.flags.timeout,
 				TempDir:             opts.flags.tempDir,
 				GlobalEnv:           os.Environ(),
+				ForceRoot:           opts.flags.runAsRoot,
 			},
 			Logger:    logManager,
 			Cache:     executor.NewDiskCache(opts.flags.cacheDir),
diff --git a/cmd/src/batch_exec.go b/cmd/src/batch_exec.go
@@ -33,6 +33,7 @@ const (
 type executorModeFlags struct {
 	timeout           time.Duration
 	file              string
+	runAsImageUser    bool
 	tempDir           string
 	repoDir           string
 	workspaceFilesDir string
@@ -42,6 +43,7 @@ func newExecutorModeFlags(flagSet *flag.FlagSet) (f *executorModeFlags) {
 	f = &executorModeFlags{}
 	flagSet.DurationVar(&f.timeout, "timeout", 60*time.Minute, "The maximum duration a single batch spec step can take.")
 	flagSet.StringVar(&f.file, "f", "", "The workspace execution input file to read.")
+	flagSet.BoolVar(&f.runAsImageUser, "run-as-image-user", false, "True to run step containers as the default image user; if false or omitted, containers are always run as root.")
 	flagSet.StringVar(&f.tempDir, "tmp", "", "Directory for storing temporary data.")
 	flagSet.StringVar(&f.repoDir, "repo", "", "Path of the checked out repo on disk.")
 	flagSet.StringVar(&f.workspaceFilesDir, "workspaceFiles", "", "Path of workspace files on disk.")
@@ -208,6 +210,7 @@ func executeBatchSpecInWorkspaces(ctx context.Context, flags *executorModeFlags)
 		GlobalEnv:        globalEnv,
 		RepoArchive:      &repozip.NoopArchive{},
 		UI:               taskExecUI.StepsExecutionUI(task),
+		ForceRoot:        !flags.runAsImageUser,
 	}
 	results, err := executor.RunSteps(ctx, opts)
 
diff --git a/internal/batches/executor/executor.go b/internal/batches/executor/executor.go
@@ -68,6 +68,7 @@ type NewExecutorOpts struct {
 	TempDir          string
 	IsRemote         bool
 	GlobalEnv        []string
+	ForceRoot        bool
 }
 
 type executor struct {
@@ -178,6 +179,7 @@ func (x *executor) do(ctx context.Context, task *Task, ui TaskExecutionUI) (err
 		Timeout:          x.opts.Timeout,
 		RepoArchive:      repoArchive,
 		WorkingDirectory: x.opts.WorkingDirectory,
+		ForceRoot:        x.opts.ForceRoot,
 
 		UI: ui.StepsExecutionUI(task),
 	}
diff --git a/internal/batches/executor/run_steps.go b/internal/batches/executor/run_steps.go
@@ -51,6 +51,9 @@ type RunStepsOpts struct {
 	// GlobalEnv is the os.Environ() for the execution. We don't read from os.Environ()
 	// directly to allow injecting variables and hiding others.
 	GlobalEnv []string
+	// ForceRoot forces Docker containers to be run as root:root, rather than
+	// whatever the image's default user and group are.
+	ForceRoot bool
 }
 
 func RunSteps(ctx context.Context, opts *RunStepsOpts) (stepResults []execution.AfterStepResult, err error) {
@@ -317,6 +320,10 @@ func executeSingleStep(
 		"--mount", fmt.Sprintf("type=bind,source=%s,target=%s,ro", runScriptFile, containerTemp),
 	}, workspaceOpts...)
 
+	if opts.ForceRoot {
+		args = append(args, "--user", "0:0")
+	}
+
 	for target, source := range filesToMount {
 		args = append(args, "--mount", fmt.Sprintf("type=bind,source=%s,target=%s,ro", source.Name(), target))
 	}

Original file line number	Diff line number	Diff line change
`@@ -68,6 +68,7 @@ type NewExecutorOpts struct {`
`68`	`68`	`TempDir string`
`69`	`69`	`IsRemote bool`
`70`	`70`	`GlobalEnv []string`
	`71`	`+ ForceRoot bool`
`71`	`72`	`}`
`72`	`73`
`73`	`74`	`type executor struct {`
`@@ -178,6 +179,7 @@ func (x executor) do(ctx context.Context, task Task, ui TaskExecutionUI) (err`
`178`	`179`	`Timeout: x.opts.Timeout,`
`179`	`180`	`RepoArchive: repoArchive,`
`180`	`181`	`WorkingDirectory: x.opts.WorkingDirectory,`
	`182`	`+ ForceRoot: x.opts.ForceRoot,`
`181`	`183`
`182`	`184`	`UI: ui.StepsExecutionUI(task),`
`183`	`185`	`}`