fix: the configuration must not be used as temporary storage (#849)

rc.Config should be treated as read-only and not as a temporary storage for the variable list of valid volumes for containers sharing this configuration.

Refs forgejo/runner#848

<!--start release-notes-assistant-->
<!--URL:https://code.forgejo.org/forgejo/runner-->
- bug fixes
  - [PR](https://code.forgejo.org/forgejo/runner/pulls/849): <!--number 849 --><!--line 0 --><!--description Zml4OiB0aGUgY29uZmlndXJhdGlvbiBtdXN0IG5vdCBiZSB1c2VkIGFzIHRlbXBvcmFyeSBzdG9yYWdl-->fix: the configuration must not be used as temporary storage<!--description-->
<!--end release-notes-assistant-->

Reviewed-on: https://code.forgejo.org/forgejo/runner/pulls/849
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.code.forgejo.org>
Reviewed-by: Michael Kriese <michael.kriese@gmx.de>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Co-committed-by: Earl Warren <contact@earl-warren.org>

act/runner/action.go

@@ -428,7 +428,7 @@ func newStepContainer(ctx context.Context, step step, image string, cmd, entrypo
 	envList = append(envList, fmt.Sprintf("%s=%s", "RUNNER_ARCH", container.RunnerArch(ctx)))
 	envList = append(envList, fmt.Sprintf("%s=%s", "RUNNER_TEMP", "/tmp"))
 
-	binds, mounts := rc.GetBindsAndMounts()
+	binds, mounts, validVolumes := rc.GetBindsAndMounts()
 	networkMode := fmt.Sprintf("container:%s", rc.jobContainerName())
 	if rc.IsHostEnv(ctx) {
 		networkMode = "default"
@@ -452,7 +452,7 @@ func newStepContainer(ctx context.Context, step step, image string, cmd, entrypo
 		UsernsMode:   rc.Config.UsernsMode,
 		Platform:     rc.Config.ContainerArchitecture,
 		AutoRemove:   rc.Config.AutoRemove,
-		ValidVolumes: rc.Config.ValidVolumes,
+		ValidVolumes: validVolumes,
 
 		ConfigOptions: rc.Config.ContainerOptions,
 	})

act/runner/run_context.go

@@ -124,7 +124,7 @@ func getDockerDaemonSocketMountPath(daemonPath string) string {
 }
 
 // Returns the binds and mounts for the container, resolving paths as appopriate
-func (rc *RunContext) GetBindsAndMounts() ([]string, map[string]string) {
+func (rc *RunContext) GetBindsAndMounts() ([]string, map[string]string, []string) {
 	name := rc.jobContainerName()
 
 	if rc.Config.ContainerDaemonSocket == "" {
@@ -171,13 +171,13 @@ func (rc *RunContext) GetBindsAndMounts() ([]string, map[string]string) {
 		mounts[name] = ext.ToContainerPath(rc.Config.Workdir)
 	}
 
-	// add some default binds and mounts to ValidVolumes
+	validVolumes := []string{
-	rc.Config.ValidVolumes = append(rc.Config.ValidVolumes, name)
+		name,
-	rc.Config.ValidVolumes = append(rc.Config.ValidVolumes, name+"-env")
+		name + "-env",
-	// TODO: add a new configuration to control whether the docker daemon can be mounted
+		getDockerDaemonSocketMountPath(rc.Config.ContainerDaemonSocket),
-	rc.Config.ValidVolumes = append(rc.Config.ValidVolumes, getDockerDaemonSocketMountPath(rc.Config.ContainerDaemonSocket))
+	}
-
+	validVolumes = append(validVolumes, rc.Config.ValidVolumes...)
-	return binds, mounts
+	return binds, mounts, validVolumes
 }
 
 //go:embed lxc-helpers-lib.sh
@@ -446,7 +446,7 @@ func (rc *RunContext) prepareJobContainer(ctx context.Context) error {
 	envList = append(envList, fmt.Sprintf("%s=%s", "LANG", "C.UTF-8")) // Use same locale as GitHub Actions
 
 	ext := container.LinuxContainerEnvironmentExtensions{}
-	binds, mounts := rc.GetBindsAndMounts()
+	binds, mounts, validVolumes := rc.GetBindsAndMounts()
 
 	networkName, createAndDeleteNetwork := rc.getNetworkName(ctx)
 	// add service containers
@@ -570,7 +570,7 @@ func (rc *RunContext) prepareJobContainer(ctx context.Context) error {
 		UsernsMode:     rc.Config.UsernsMode,
 		Platform:       rc.Config.ContainerArchitecture,
 		AutoRemove:     rc.Config.AutoRemove,
-		ValidVolumes:   rc.Config.ValidVolumes,
+		ValidVolumes:   validVolumes,
 
 		JobOptions:    rc.options(ctx),
 		ConfigOptions: rc.Config.ContainerOptions,

act/runner/run_context_test.go

@@ -219,7 +219,7 @@ func TestRunContext_GetBindsAndMounts(t *testing.T) {
 					config := testcase.rc.Config
 					config.Workdir = testcase.name
 					config.BindWorkdir = bindWorkDir
-					gotbind, gotmount := rctemplate.GetBindsAndMounts()
+					gotbind, gotmount, _ := rctemplate.GetBindsAndMounts()
 
 					// Name binds/mounts are either/or
 					if config.BindWorkdir {
@@ -271,7 +271,7 @@ func TestRunContext_GetBindsAndMounts(t *testing.T) {
 				rc.Run.JobID = "job1"
 				rc.Run.Workflow.Jobs = map[string]*model.Job{"job1": job}
 
-				gotbind, gotmount := rc.GetBindsAndMounts()
+				gotbind, gotmount, _ := rc.GetBindsAndMounts()
 
 				if len(testcase.wantbind) > 0 {
 					assert.Contains(t, gotbind, testcase.wantbind)
@@ -764,11 +764,6 @@ jobs:
 					ConfigOptions:  "",
 					JobOptions:     "",
 					AutoRemove:     false,
-					ValidVolumes: []string{
-						"WORKFLOW-e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855_JOB",
-						"WORKFLOW-e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855_JOB-env",
-						"/var/run/docker.sock",
-					},
 				},
 				{
 					Name:           "WORKFLOW-e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca49599-c233cf913e1d0c90cc1404ee09917e625f9cb82156ca3d7cb10b729d563728ea",
@@ -792,11 +787,6 @@ jobs:
 					ConfigOptions:  "",
 					JobOptions:     "",
 					AutoRemove:     false,
-					ValidVolumes: []string{
-						"WORKFLOW-e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855_JOB",
-						"WORKFLOW-e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855_JOB-env",
-						"/var/run/docker.sock",
-					},
 				},
 			},
 		},

act/runner/step_docker.go

@@ -110,7 +110,7 @@ func (sd *stepDocker) newStepContainer(ctx context.Context, image string, cmd, e
 	envList = append(envList, fmt.Sprintf("%s=%s", "RUNNER_ARCH", container.RunnerArch(ctx)))
 	envList = append(envList, fmt.Sprintf("%s=%s", "RUNNER_TEMP", "/tmp"))
 
-	binds, mounts := rc.GetBindsAndMounts()
+	binds, mounts, validVolumes := rc.GetBindsAndMounts()
 	stepContainer := ContainerNewContainer(&container.NewContainerInput{
 		Cmd:          cmd,
 		Entrypoint:   entrypoint,
@@ -130,7 +130,7 @@ func (sd *stepDocker) newStepContainer(ctx context.Context, image string, cmd, e
 		UsernsMode:   rc.Config.UsernsMode,
 		Platform:     rc.Config.ContainerArchitecture,
 		AutoRemove:   rc.Config.AutoRemove,
-		ValidVolumes: rc.Config.ValidVolumes,
+		ValidVolumes: validVolumes,
 	})
 	return stepContainer
 }