fix/batch-changes: validate files target paths in changeset hooks and executor (#1352)

Author: cbrnrd

internal/batches/executor/run_steps.go

@@ -540,6 +540,16 @@ func createFilesToMount(tempDir string, step batcheslib.Step, stepContext *templ
 	// can mount them into the container.
 	filesToMount := make(map[string]*os.File, len(files))
 	for name, content := range files {
+		// Defense-in-depth: the mount target name is concatenated directly
+		// into a Docker `--mount type=bind,...,target=<name>` argument. A comma
+		// in the name would let an attacker break out of the mount spec and
+		// inject arbitrary source=/target= pairs (e.g. /var/run/docker.sock).
+		// Parse-time validation rejects this, but the executor receives
+		// pre-parsed steps via JSON and never re-runs that validation, so we
+		// re-check here at the sink.
+		if strings.Contains(name, ",") {
+			return nil, cleanup, errors.Newf("files target path %q contains invalid characters", name)
+		}
 		fp, err := os.CreateTemp(tempDir, "")
 		if err != nil {
 			return nil, cleanup, errors.Wrap(err, "creating temporary file")

internal/batches/executor/run_steps_test.go

@@ -5,9 +5,25 @@ import (
 
 	"github.com/stretchr/testify/require"
 
+	batcheslib "github.com/sourcegraph/sourcegraph/lib/batches"
 	"github.com/sourcegraph/sourcegraph/lib/batches/template"
 )
 
+func TestCreateFilesToMount_RejectsCommaInTargetPath(t *testing.T) {
+	step := batcheslib.Step{
+		Files: map[string]string{
+			"/tmp/x,source=/var/run/docker.sock,target=/var/run/docker.sock": "IGNORED",
+		},
+	}
+
+	_, cleanup, err := createFilesToMount(t.TempDir(), step, &template.StepContext{})
+	if cleanup != nil {
+		cleanup()
+	}
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "contains invalid characters")
+}
+
 func TestRenderStepContainer(t *testing.T) {
 	t.Run("static image", func(t *testing.T) {
 		got, err := renderStepContainer("alpine:3", &template.StepContext{})