diff --git a/act/container/container_types.go b/act/container/container_types.go index fe1abb9d..4fa25896 100644 --- a/act/container/container_types.go +++ b/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 diff --git a/act/container/docker_run.go b/act/container/docker_run.go index 7f56956d..b2266ee8 100644 --- a/act/container/docker_run.go +++ b/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 diff --git a/act/runner/action.go b/act/runner/action.go index bf801b3a..05781287 100644 --- a/act/runner/action.go +++ b/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" diff --git a/act/runner/container_mock_test.go b/act/runner/container_mock_test.go index 30317c40..73f8001d 100644 --- a/act/runner/container_mock_test.go +++ b/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) +} diff --git a/act/runner/run_context.go b/act/runner/run_context.go index e038ab4c..383e012c 100644 --- a/act/runner/run_context.go +++ b/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 -func (rc *RunContext) GetBindsAndMounts(ctx context.Context) ([]string, map[string]string) { +// 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, 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 diff --git a/act/runner/run_context_test.go b/act/runner/run_context_test.go index c81b3ec9..6c5248cf 100644 --- a/act/runner/run_context_test.go +++ b/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) { diff --git a/docs/post-task-script.md b/docs/post-task-script.md index cca3f4bb..b40fb071 100644 --- a/docs/post-task-script.md +++ b/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).