fix: empty root-owned bind workspaces for non-root runners (#1399)

A job running as root leaves root-owned files in the bind-mounted workspace. A non-root runner on Linux cannot remove them, so workspaces pile up with only a warning. Such a runner now empties the workspace as root from inside the job container, right before removing the container. Root runners and Docker Desktop already remove these files themselves and skip this.

The workspace is only emptied when it is exactly the runner's own mountpoint with no mount below it. Otherwise, or when the command fails, the existing removal and warning apply. Jobs of a reusable workflow share the workspace, so they are not covered and still end in the existing warning.

Fixes: https://gitea.com/gitea/runner/issues/879
Co-authored-by: silverwind <[email protected]>
Co-authored-by: silverwind <[email protected]>
Reviewed-on: https://gitea.com/gitea/runner/pulls/1399
Reviewed-by: silverwind <[email protected]>
Co-authored-by: bircni <[email protected]>

act/container/container_types.go

@@ -15,6 +15,7 @@ import (
 	"strings"
 	"sync"
 	"sync/atomic"
+	"time"
 
 	"gitea.com/gitea/runner/act/common"
 
@@ -24,6 +25,13 @@ import (
 
 var hostOS = runtime.GOOS // replaced in tests
 
+// DrainGracePeriod bounds how long we wait for an output-copy goroutine to
+// finish draining a container's output before returning, so that neither a
+// cancellation (waitForCommand) nor a normal container exit (wait) truncates
+// the tail of the log. It is a safety bound: in the common case the stream
+// reaches EOF and the goroutine returns well before this elapses.
+const DrainGracePeriod = 2 * time.Second
+
 // ExitCodeError reports a non-zero process exit code from a container command.
 type ExitCodeError int
 

act/container/docker_run.go

@@ -41,13 +41,6 @@ import (
 	specs "github.com/opencontainers/image-spec/specs-go/v1"
 )
 
-// drainGracePeriod bounds how long we wait for an output-copy goroutine to
-// finish draining a container's output before returning, so that neither a
-// cancellation (waitForCommand) nor a normal container exit (wait) truncates
-// the tail of the log. It is a safety bound: in the common case the stream
-// reaches EOF and the goroutine returns well before this elapses.
-const drainGracePeriod = 2 * time.Second
-
 // NewContainer creates a reference to a container
 func NewContainer(input *NewContainerInput) ExecutionsEnvironment {
 	// Resolved up front because the image pull runs before the container is created.
@@ -947,7 +940,7 @@ func (cr *containerReference) waitForCommand(ctx context.Context, resp client.Hi
 			if err != nil {
 				logger.Debugf("Failed to drain command output after cancellation: %v", err)
 			}
-		case <-time.After(drainGracePeriod):
+		case <-time.After(DrainGracePeriod):
 			logger.Warn("Timed out draining command output after cancellation")
 		}
 
@@ -1122,7 +1115,7 @@ func (cr *containerReference) wait() common.Executor {
 		if cr.attachDone != nil {
 			select {
 			case <-cr.attachDone:
-			case <-time.After(drainGracePeriod):
+			case <-time.After(DrainGracePeriod):
 				logger.Warn("Timed out draining container output")
 			}
 			cr.attachDone = nil

act/runner/action.go

@@ -472,7 +472,7 @@ func newStepContainer(ctx context.Context, step step, image string, cmd, entrypo
 
 	envList = append(envList, rc.runnerEnv(ctx)...)
 
-	binds, mounts := rc.GetBindsAndMounts(ctx)
+	binds, mounts, _ := rc.GetBindsAndMounts(ctx)
 	networkMode := "container:" + rc.jobContainerName()
 	if rc.IsHostEnv() {
 		networkMode = "default"

act/runner/container_mock_test.go

@@ -92,3 +92,8 @@ func (cm *containerMock) Inspect(ctx context.Context) (*container.Info, error) {
 func (cm *containerMock) CopyTarStream(ctx context.Context, destPath string, write func(*container.TarWriter) error) error {
 	return cm.Called(ctx, destPath, write).Error(0)
 }
+
+func (cm *containerMock) ReplaceLogWriter(stdout, stderr io.Writer) (io.Writer, io.Writer) {
+	args := cm.Called(stdout, stderr)
+	return args.Get(0).(io.Writer), args.Get(1).(io.Writer)
+}

act/runner/run_context.go

@@ -30,6 +30,7 @@ import (
 	"time"
 
 	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/act/common/logging"
 	"gitea.com/gitea/runner/act/container"
 	"gitea.com/gitea/runner/act/ghcontext"
 	"gitea.com/gitea/runner/internal/pkg/lock"
@@ -75,6 +76,7 @@ type RunContext struct {
 	actionInputs        map[string]any // inputs of the composite action this runs, nil for a job
 	Masks               []string
 	cleanUpJobContainer common.Executor
+	taskWorkspace       string
 	deferVolumeCleanup  func(common.Executor)
 	caller              *caller           // job calling this RunContext (reusable workflows)
 	workflowCallInputs  map[string]any    // the caller's with:, resolved once by resolveWorkflowCall
@@ -378,8 +380,8 @@ func splitVolumes(specs []string) ([]string, map[string]string, map[string]bool)
 	return binds, mounts, targets
 }
 
-// Returns the binds and mounts for the container, resolving paths as appopriate
+// Returns the binds and mounts for the container, resolving paths as appropriate, and the task workspace the runner binds
-func (rc *RunContext) GetBindsAndMounts(ctx context.Context) ([]string, map[string]string) {
+func (rc *RunContext) GetBindsAndMounts(ctx context.Context) ([]string, map[string]string, string) {
 	name := rc.jobContainerName()
 	ext := container.LinuxContainerEnvironmentExtensions{}
 
@@ -399,12 +401,16 @@ func (rc *RunContext) GetBindsAndMounts(ctx context.Context) ([]string, map[stri
 	}
 	mounts[name+"-env"] = ext.GetActPath() // runner-internal, never overridable
 
+	taskWorkspace := ""
 	if workdir := ext.ToContainerPath(rc.Config.Workdir); !claimed[workdir] {
 		source := rc.workdirMountRoot()
 		target := ext.ToContainerPath(source)
 		if claimed[target] {
 			source, target = rc.Config.Workdir, workdir
 		}
+		if rc.Config.BindWorkdir && rc.Config.PresetGitHubContext != nil {
+			taskWorkspace = target
+		}
 		if rc.Config.BindWorkdir {
 			bindModifiers := ""
 			if runtime.GOOS == "darwin" {
@@ -419,7 +425,7 @@ func (rc *RunContext) GetBindsAndMounts(ctx context.Context) ([]string, map[stri
 		}
 	}
 
-	return binds, mounts
+	return binds, mounts, taskWorkspace
 }
 
 func (rc *RunContext) startHostEnvironment() common.Executor {
@@ -672,7 +678,8 @@ func (rc *RunContext) startJobContainer() common.Executor {
 			return err
 		}
 		rc.startDockerProxy(ctx)
-		containerInput.Binds, containerInput.Mounts = rc.GetBindsAndMounts(ctx)
+		var taskWorkspace string
+		containerInput.Binds, containerInput.Mounts, taskWorkspace = rc.GetBindsAndMounts(ctx)
 		containerInput.ValidVolumes = rc.validVolumes()
 
 		rc.jobNetworkName = networkName
@@ -687,6 +694,7 @@ func (rc *RunContext) startJobContainer() common.Executor {
 			func(ctx context.Context) error { return rc.createJobVolumes(ctx, containerInput.Mounts) },
 			rc.JobContainer.Create(rc.Config.ContainerCapAdd, rc.Config.ContainerCapDrop),
 			rc.JobContainer.Start(false),
+			func(context.Context) error { rc.taskWorkspace = taskWorkspace; return nil },
 			rc.captureJobContainerInfo(),
 			rc.copyWorkflowFiles(),
 			rc.waitForServiceContainers(),
@@ -801,7 +809,10 @@ func (rc *RunContext) cleanupJobResources(networkName string, createAndDeleteNet
 		errs := []error{rc.closeDockerProxy(ctx)}
 		var containers []common.Executor
 		if rc.JobContainer != nil {
-			containers = append(containers, rc.JobContainer.Remove())
+			containers = append(containers, func(ctx context.Context) error {
+				rc.emptyTaskWorkspace(ctx)
+				return rc.JobContainer.Remove()(ctx)
+			})
 		}
 		if len(rc.serviceContainers) > 0 {
 			logger.Infof("Cleaning up services for job %s", rc.JobName)
@@ -831,6 +842,42 @@ func (rc *RunContext) cleanupJobResources(networkName string, createAndDeleteNet
 	}
 }
 
+// exits 3 unless $1 is a mountpoint in mountinfo $2, at least two levels deep, with no mount below it for rm to cross into
+const emptyTaskWorkspaceScript = `case "$1" in /?*/?*) ;; *) exit 3 ;; esac
+mounted=
+while read -r _ _ _ _ point _; do
+	case "$point" in "$1") mounted=1 ;; "$1"/*) exit 3 ;; esac
+done <"$2"
+[ "$mounted" ] || exit 3
+rm -rf "$1"/* "$1"/.[!.]* "$1"/..?*`
+
+var runnerCanRemoveJobFiles = runtime.GOOS != "linux" || os.Geteuid() == 0 // root, or a desktop VM mapping container files to the user, replaced in tests
+
+// empties the workspace as root, on root-owned PATH dirs only, for a runner that may not own what the job wrote, called workflows share it
+func (rc *RunContext) emptyTaskWorkspace(ctx context.Context) {
+	if rc.taskWorkspace == "" || rc.caller != nil || runnerCanRemoveJobFiles {
+		return
+	}
+	budget := 30 * time.Second
+	if deadline, ok := ctx.Deadline(); ok {
+		budget = min(budget, time.Until(deadline)/2-container.DrainGracePeriod)
+	}
+	ctx, cancel := context.WithTimeout(ctx, budget)
+	defer cancel()
+	stdout, stderr := rc.JobContainer.ReplaceLogWriter(io.Discard, io.Discard)
+	defer rc.JobContainer.ReplaceLogWriter(stdout, stderr)
+	err := rc.JobContainer.Exec(
+		[]string{"sh", "-c", emptyTaskWorkspaceScript, "sh", rc.taskWorkspace, "/proc/self/mountinfo"},
+		map[string]string{"PATH": "/usr/sbin:/usr/bin:/sbin:/bin"}, "0", "/",
+	)(common.WithLogger(ctx, logging.New(logging.DiscardHandler)))
+	switch {
+	case errors.Is(err, container.ExitCodeError(3)):
+		logging.Infof("not emptying workspace %s of %s: under two levels deep, not a mountpoint as given, or a mount below it", rc.taskWorkspace, rc.jobContainerName())
+	case err != nil:
+		logging.Infof("could not empty workspace %s of %s: %v", rc.taskWorkspace, rc.jobContainerName(), err)
+	}
+}
+
 func runAll(ctx context.Context, executors []common.Executor) error {
 	errs := make([]error, len(executors))
 	var wg sync.WaitGroup

act/runner/run_context_test.go

@@ -16,6 +16,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"runtime"
 	"slices"
@@ -455,7 +456,7 @@ func TestRunContext_GetBindsAndMounts(t *testing.T) {
 			t.Run(testcase.name+testBindSuffix, func(t *testing.T) {
 				rctemplate.Config.Workdir = testcase.name
 				rctemplate.Config.BindWorkdir = bindWorkDir
-				gotbind, gotmount := rctemplate.GetBindsAndMounts(t.Context())
+				gotbind, gotmount, _ := rctemplate.GetBindsAndMounts(t.Context())
 
 				if bindWorkDir {
 					fullBind := testcase.name + ":" + testcase.want
@@ -507,7 +508,7 @@ func TestRunContext_GetBindsAndMounts(t *testing.T) {
 			rc.ExprEval = rc.NewExpressionEvaluator(t.Context())
 			require.NoError(t, rc.resolvePlatformImage(t.Context()))
 
-			gotbind, gotmount := rc.GetBindsAndMounts(t.Context())
+			gotbind, gotmount, _ := rc.GetBindsAndMounts(t.Context())
 			assert.Contains(t, gotbind, "/host/mame/roms:/root/.mame/roms:ro")
 			assert.NotContains(t, gotbind, "${{ secrets.MAME }}")
 			assert.NotContains(t, gotmount, "${{ secrets.MAME }}")
@@ -522,7 +523,7 @@ func TestRunContext_GetBindsAndMounts(t *testing.T) {
 				rc.containerSpec.Volumes = testcase.volumes
 				rc.Run.JobID = "job1"
 
-				gotbind, gotmount := rc.GetBindsAndMounts(t.Context())
+				gotbind, gotmount, _ := rc.GetBindsAndMounts(t.Context())
 
 				if len(testcase.wantbind) > 0 {
 					assert.Contains(t, gotbind, testcase.wantbind)
@@ -551,12 +552,12 @@ func TestRunContext_GetBindsAndMounts(t *testing.T) {
 	t.Run("ToolCacheMount", func(t *testing.T) {
 		rc := newRunContext(&Config{})
 
-		_, gotmount := rc.GetBindsAndMounts(t.Context())
+		_, gotmount, _ := rc.GetBindsAndMounts(t.Context())
 		assert.NotContains(t, gotmount, sharedToolCacheVolume)
 
 		rc.Config.SharedToolCache = true
 		rc.containerSpec.Volumes = []string{"/unapproved:" + container.DefaultToolCache}
-		_, gotmount = rc.GetBindsAndMounts(t.Context())
+		_, gotmount, _ = rc.GetBindsAndMounts(t.Context())
 		assert.Equal(t, container.DefaultToolCache, gotmount[sharedToolCacheVolume])
 	})
 
@@ -569,16 +570,26 @@ func TestRunContext_GetBindsAndMounts(t *testing.T) {
 			Config: &Config{BindWorkdir: true, Workdir: "/workspace/1/owner/repo", PresetGitHubContext: &model.GithubContext{}, ValidVolumes: []string{"claimed"}},
 		}
 
-		gotbind, _ := rc.GetBindsAndMounts(t.Context())
+		gotbind, _, workspace := rc.GetBindsAndMounts(t.Context())
 		assert.True(t, slices.ContainsFunc(gotbind, func(bind string) bool { return strings.HasPrefix(bind, "/workspace/1:/workspace/1") }), gotbind)
+		assert.Equal(t, "/workspace/1", workspace)
 
 		rc.Config.BindWorkdir = false
-		_, gotmount := rc.GetBindsAndMounts(t.Context())
+		_, gotmount, workspace := rc.GetBindsAndMounts(t.Context())
 		assert.Equal(t, "/workspace/1", gotmount[rc.jobContainerName()])
+		assert.Empty(t, workspace)
 
 		rc.containerSpec.Volumes = []string{"claimed:/workspace/1"}
-		_, gotmount = rc.GetBindsAndMounts(t.Context())
+		_, gotmount, _ = rc.GetBindsAndMounts(t.Context())
 		assert.Equal(t, "/workspace/1/owner/repo", gotmount[rc.jobContainerName()])
+		rc.Config.BindWorkdir = true
+		_, _, workspace = rc.GetBindsAndMounts(t.Context())
+		assert.Equal(t, "/workspace/1/owner/repo", workspace)
+
+		rc.containerSpec.Volumes = nil
+		rc.Config.PresetGitHubContext = nil
+		_, _, workspace = rc.GetBindsAndMounts(t.Context())
+		assert.Empty(t, workspace)
 	})
 }
 
@@ -611,7 +622,7 @@ func TestRunContextValidVolumes(t *testing.T) {
 	rc.JobContainer = hostEnv
 	rc.Config.BindWorkdir = false
 	rc.Config.SharedToolCache = true
-	binds, mounts := rc.GetBindsAndMounts(t.Context())
+	binds, mounts, _ := rc.GetBindsAndMounts(t.Context())
 	assert.Subset(t, binds, []string{hostEnv.Path + ":" + hostEnv.Path, hostEnv.GetActPath() + ":" + hostEnv.GetActPath()})
 	assert.Contains(t, mounts, sharedToolCacheVolume)
 	assert.Equal(t, binds, container.SanitizeBinds(t.Context(), rc.validVolumes(), binds))
@@ -683,6 +694,110 @@ func TestCleanupJobVolumesReapsAbandonedDeferredCleanup(t *testing.T) {
 	assert.ElementsMatch(t, []string{"became-active", "remove-failed", "foreign", "fresh", "unknown-age"}, slices.Collect(maps.Keys(volumes)))
 }
 
+func TestCleanupJobResourcesEmptiesTaskWorkspace(t *testing.T) {
+	defer func(previous bool) { runnerCanRemoveJobFiles = previous }(runnerCanRemoveJobFiles)
+	for _, tt := range []struct {
+		name, workspace string
+		caller          *caller
+		runnerCanRemove bool
+		execErr         error
+		log             string
+	}{
+		{name: "refusal is logged", workspace: "/workspace/1", execErr: container.ExitCodeError(3), log: "not emptying workspace /workspace/1"},
+		{name: "failure does not fail the job", workspace: "/workspace/1", execErr: errors.New("no rm"), log: "could not empty workspace /workspace/1"},
+		{name: "no runner bind"},
+		{name: "called workflow shares it", workspace: "/workspace/1", caller: &caller{}},
+		{name: "runner removes it itself", workspace: "/workspace/1", runnerCanRemove: true},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			runnerCanRemoveJobFiles = tt.runnerCanRemove
+			processLog := logtest.SetDefault(t)
+			var jobLog bytes.Buffer
+			var calls []string
+			job := &containerMock{}
+			if tt.log != "" {
+				job.On("ReplaceLogWriter", io.Discard, io.Discard).Return(&jobLog, &jobLog).Once()
+				job.On("ReplaceLogWriter", &jobLog, &jobLog).Return(io.Discard, io.Discard).Once()
+				job.On("Exec", mock.MatchedBy(func(command []string) bool { return command[4] == tt.workspace }), map[string]string{"PATH": "/usr/sbin:/usr/bin:/sbin:/bin"}, "0", "/").Return(func(ctx context.Context) error {
+					calls = append(calls, "exec")
+					deadline, ok := ctx.Deadline()
+					assert.True(t, ok)
+					assert.LessOrEqual(t, time.Until(deadline), 10*time.Second-container.DrainGracePeriod)
+					common.Logger(ctx).Infof("workspace script output")
+					return tt.execErr
+				})
+			}
+			job.On("Remove").Return(func(context.Context) error {
+				calls = append(calls, "remove")
+				return nil
+			})
+			rc := &RunContext{
+				Config:             &Config{},
+				Run:                &model.Run{Workflow: &model.Workflow{Name: "workflow"}, JobID: "job"},
+				JobContainer:       job,
+				taskWorkspace:      tt.workspace,
+				caller:             tt.caller,
+				deferVolumeCleanup: func(common.Executor) {},
+			}
+			ctx, cancel := context.WithTimeout(t.Context(), 20*time.Second)
+			defer cancel()
+			ctx = common.WithLogger(ctx, logging.New(logging.NewLevelHandler(newJobLogHandler(&jobLog, cyan), logging.LevelInfo)))
+			require.NoError(t, rc.cleanupJobResources("", false, false)(ctx))
+			job.AssertExpectations(t)
+			assert.NotContains(t, jobLog.String(), "workspace")
+			if tt.log == "" {
+				assert.Equal(t, []string{"remove"}, calls)
+				return
+			}
+			assert.Equal(t, []string{"exec", "remove"}, calls)
+			assert.True(t, slices.ContainsFunc(processLog.AllRecords(), func(record logging.Record) bool {
+				return strings.HasPrefix(record.Message, tt.log)
+			}), processLog.AllRecords())
+		})
+	}
+}
+
+func TestEmptyTaskWorkspaceScript(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("the script runs in the linux job container")
+	}
+	run := func(t *testing.T, workspace string, mountpoints ...string) error {
+		t.Helper()
+		var mountinfo strings.Builder
+		for _, mountpoint := range mountpoints {
+			fmt.Fprintf(&mountinfo, "36 35 98:0 /src %s rw - ext4 /dev/sda1 rw\n", mountpoint)
+		}
+		info := filepath.Join(t.TempDir(), "mountinfo")
+		require.NoError(t, os.WriteFile(info, []byte(mountinfo.String()), 0o600))
+		return exec.Command("sh", "-c", emptyTaskWorkspaceScript, "sh", workspace, info).Run()
+	}
+
+	workspace := t.TempDir()
+	for _, file := range []string{"owner/repo/file", ".hidden", "..odd"} {
+		require.NoError(t, os.MkdirAll(filepath.Dir(filepath.Join(workspace, file)), 0o755))
+		require.NoError(t, os.WriteFile(filepath.Join(workspace, file), nil, 0o600))
+	}
+	require.NoError(t, run(t, workspace, workspace))
+	entries, err := os.ReadDir(workspace)
+	require.NoError(t, err)
+	assert.Empty(t, entries)
+
+	for _, tt := range []struct {
+		name, workspace string
+		mountpoints     []string
+	}{
+		{"a mount below it", "/nonexistent/1", []string{"/nonexistent/1", "/nonexistent/1/owner/repo/cache"}},
+		{"a workspace that is not a mountpoint", "/nonexistent/1", []string{"/nonexistent/2"}},
+		{"a top-level directory", "/nonexistent", []string{"/nonexistent"}},
+	} {
+		t.Run("refuses "+tt.name, func(t *testing.T) {
+			var exitErr *exec.ExitError
+			require.ErrorAs(t, run(t, tt.workspace, tt.mountpoints...), &exitErr)
+			assert.Equal(t, 3, exitErr.ExitCode())
+		})
+	}
+}
+
 func TestRemoveDockerActionImagesByTag(t *testing.T) {
 	var removed []string
 	fakeDockerDaemon(t, func(writer http.ResponseWriter, request *http.Request) {

docs/post-task-script.md

@@ -9,7 +9,7 @@ It is configured under `runner.post_task_script` in the runner YAML config (see
 For each task, execution order is:
 
 1. Workflow runs (steps, actions, containers).
-2. In-job cleanup (action `post:` steps, container stop/remove).
+2. In-job cleanup (action `post:` steps, emptying a bind-mounted workspace from inside the job container when a non-root runner on Linux may not own its files, container stop/remove).
 3. Job outputs are reported to Gitea.
 4. Bind-workdir workspace removal, when `container.bind_workdir` is enabled.
 5. **Post-task script** (this hook).