fix(actions): correctness and hardening fixes (#38518)

Various fixes to actions

1. **Cap total jobs per run in reusable-workflow expansion** — only
nesting depth was capped, so fan-out + nested reusable workflows could
explode job-row inserts and exhaust the DB from a single push. Now
enforces `MaxJobNumPerRun` in the insert path.
2. **Reject rerun-failed when a run has no failed jobs** — an empty job
list meant "re-run everything", so `rerun-failed` on a green run re-ran
all jobs. Now errors (web + API).
3. **Don't adopt external commit statuses into the legacy hash** — the
pre-#35699 Context-only hash matched API-posted statuses too, collapsing
two same-named workflows into one check. Now limited to Actions-user
rows.
4. **Don't cut post-cancel cleanup short in `StopEndlessTasks`** — the
sweep force-stopped just-cancelled jobs mid-cleanup. Now targets
`StatusRunning` only; stalled cancels stay covered by `StopZombieTasks`.
5. **Avoid redundant run reload in `GenerateGiteaContext`** — resolving
`github.triggering_actor` reloaded the run already passed in. Now loads
only the trigger user via new `ActionRunAttempt.LoadTriggerUser`.

---------

Co-authored-by: Zettat123 <zettat123@gmail.com>
Co-authored-by: silverwind <me@silverwind.io>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

services/actions/clear_tasks.go

@@ -26,11 +26,13 @@ func StopZombieTasks(ctx context.Context) error {
 	}, actions_model.StatusRunning, actions_model.StatusCancelling)
 }
 
-// StopEndlessTasks stops tasks in running/cancelling status with continuous updates that don't end for a long time
+// StopEndlessTasks stops running tasks with continuous updates that don't end for a long time.
+// StatusRunning only: the threshold is the task's *start* time, so including StatusCancelling would kill a
+// task mid post-cancel cleanup. StopZombieTasks covers a stalled one, keying off the last update instead.
 func StopEndlessTasks(ctx context.Context) error {
 	return stopTasksByStatuses(ctx, actions_model.FindTaskOptions{
 		StartedBefore: timeutil.TimeStamp(time.Now().Add(-setting.Actions.EndlessTaskTimeout).Unix()),
-	}, actions_model.StatusRunning, actions_model.StatusCancelling)
+	}, actions_model.StatusRunning)
 }
 
 func stopTasksByStatuses(ctx context.Context, opts actions_model.FindTaskOptions, statuses ...actions_model.Status) error {

services/actions/clear_tasks_test.go

@@ -4,11 +4,16 @@
 package actions
 
 import (
+	"fmt"
 	"testing"
+	"time"
 
 	actions_model "gitea.dev/models/actions"
 	"gitea.dev/models/db"
 	"gitea.dev/models/unittest"
+	"gitea.dev/modules/setting"
+	"gitea.dev/modules/test"
+	"gitea.dev/modules/timeutil"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -88,3 +93,51 @@ func TestShouldBlockRunByConcurrency_CancellingJobBlocks(t *testing.T) {
 	require.NoError(t, err)
 	assert.True(t, shouldBlock)
 }
+
+// TestStopEndlessTasksSkipsCancelling verifies that a task running its post-cancel cleanup is not
+// force-stopped by the endless-task sweep just because the job started long ago.
+func TestStopEndlessTasksSkipsCancelling(t *testing.T) {
+	require.NoError(t, unittest.PrepareTestDatabase())
+
+	// StopEndlessTasks emits ready jobs onto the emitter queue, mock it
+	defer test.MockVariableValue(&EmitJobsIfReadyByRun, func(runID int64) error { return nil })()
+
+	// well past the endless-task threshold, keyed on the task's start time
+	longAgo := timeutil.TimeStamp(time.Now().Add(-2 * setting.Actions.EndlessTaskTimeout).Unix())
+
+	var seq int64
+	newTaskWithJob := func(status actions_model.Status) *actions_model.ActionTask {
+		seq++
+		run := &actions_model.ActionRun{
+			RepoID: 1, OwnerID: 2, TriggerUserID: 2, WorkflowID: "test.yml",
+			Index: 99500 + seq, Ref: "refs/heads/main", Status: actions_model.StatusRunning,
+		}
+		require.NoError(t, db.Insert(t.Context(), run))
+		attempt := &actions_model.ActionRunAttempt{
+			RepoID: run.RepoID, RunID: run.ID, Attempt: 1, TriggerUserID: run.TriggerUserID, Status: actions_model.StatusRunning,
+		}
+		require.NoError(t, db.Insert(t.Context(), attempt))
+		job := &actions_model.ActionRunJob{
+			RunID: run.ID, RunAttemptID: attempt.ID, AttemptJobID: 1, RepoID: run.RepoID, OwnerID: run.OwnerID,
+			CommitSHA: "c2d72f548424103f01ee1dc02889c1e2bff816b0", Name: "j", JobID: "j", Status: status,
+		}
+		require.NoError(t, db.Insert(t.Context(), job))
+		task := &actions_model.ActionTask{
+			JobID: job.ID, RepoID: run.RepoID, OwnerID: run.OwnerID,
+			CommitSHA: job.CommitSHA, Status: status, Started: longAgo,
+			TokenHash: fmt.Sprintf("endless-test-token-%d", seq), TokenSalt: "salt",
+		}
+		require.NoError(t, db.Insert(t.Context(), task))
+		return task
+	}
+
+	running := newTaskWithJob(actions_model.StatusRunning)
+	cancelling := newTaskWithJob(actions_model.StatusCancelling)
+
+	require.NoError(t, StopEndlessTasks(t.Context()))
+
+	runningAfter := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: running.ID})
+	cancellingAfter := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: cancelling.ID})
+	assert.Equal(t, actions_model.StatusFailure, runningAfter.Status, "long-running task should be force-stopped")
+	assert.Equal(t, actions_model.StatusCancelling, cancellingAfter.Status, "cancelling task should keep running its cleanup")
+}

services/actions/commit_status.go

@@ -265,7 +265,10 @@ func createWorkflowCommitStatus(ctx context.Context, repo *repo_model.Repository
 		return fmt.Errorf("GetLatestCommitStatus: %w", err)
 	}
 	for _, v := range statuses {
-		if v.ContextHash == legacyHash && v.Context == ctxName {
+		// Only adopt the legacy (Context-only) hash from a row the Actions user itself created before the
+		// #35699 fix, i.e. a pre-upgrade in-flight run. Adopting it from an external integration or the API
+		// would collapse two same-named workflows back into a single check.
+		if v.ContextHash == legacyHash && v.Context == ctxName && v.CreatorID == user_model.ActionsUserID {
 			ctxHash = legacyHash
 			break
 		}

services/actions/commit_status_test.go

@@ -217,7 +217,8 @@ func TestCreateCommitStatus_LegacyHashRecovery(t *testing.T) {
 	legacyHash := git_model.HashCommitStatusContext(ctxName)
 	sha, err := git.NewIDFromString(branch.CommitID)
 	require.NoError(t, err)
-	creator := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
+	// Pre-#35699 in-flight rows were posted by the Actions user with the Context-only hash.
+	creator := user_model.NewActionsUser()
 	require.NoError(t, git_model.NewCommitStatus(t.Context(), git_model.NewCommitStatusOptions{
 		Repo:    repo,
 		Creator: creator,
@@ -258,6 +259,65 @@ func TestCreateCommitStatus_LegacyHashRecovery(t *testing.T) {
 	assert.Equal(t, 1, matches)
 }
 
+// TestCreateCommitStatus_LegacyHashExternalNotAdopted: a status from a non-Actions creator sharing a
+// workflow's Context must not pull the workflow into the legacy Context-only hash group.
+func TestCreateCommitStatus_LegacyHashExternalNotAdopted(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4})
+	branch := unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo.ID, Name: repo.DefaultBranch})
+
+	workflowID := "external.yaml"
+	ctxName := "external.yaml / my-job (push)"
+	legacyHash := git_model.HashCommitStatusContext(ctxName)
+	distinctHash := git_model.HashCommitStatusContext(ctxName + "\x00" + workflowID)
+	sha, err := git.NewIDFromString(branch.CommitID)
+	require.NoError(t, err)
+
+	// An external status (posted by a real user, not the Actions user) sharing the same Context.
+	externalCreator := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
+	require.NoError(t, git_model.NewCommitStatus(t.Context(), git_model.NewCommitStatusOptions{
+		Repo:    repo,
+		Creator: externalCreator,
+		SHA:     sha,
+		CommitStatus: &git_model.CommitStatus{
+			State:       commitstatus.CommitStatusSuccess,
+			Context:     ctxName,
+			ContextHash: legacyHash,
+			TargetURL:   "https://example.invalid/external",
+			Description: "external check",
+		},
+	}))
+
+	run := &actions_model.ActionRun{
+		ID: 99311, Index: 99311, RepoID: repo.ID, Repo: repo, OwnerID: repo.OwnerID, TriggerUserID: repo.OwnerID,
+		WorkflowID: workflowID, CommitSHA: branch.CommitID,
+	}
+	require.NoError(t, db.Insert(t.Context(), run))
+	job := &actions_model.ActionRunJob{
+		ID: 99312, RunID: run.ID, RepoID: repo.ID, OwnerID: repo.OwnerID,
+		Name: "my-job", Status: actions_model.StatusSuccess,
+	}
+	require.NoError(t, db.Insert(t.Context(), job))
+	require.NoError(t, createCommitStatus(t.Context(), repo, "push", branch.CommitID, "", run, job))
+
+	latest, err := git_model.GetLatestCommitStatus(t.Context(), repo.ID, branch.CommitID, db.ListOptionsAll)
+	require.NoError(t, err)
+	// The external status and the workflow status must coexist under distinct hashes.
+	var external, workflow *git_model.CommitStatus
+	for _, s := range latest {
+		switch s.ContextHash {
+		case legacyHash:
+			external = s
+		case distinctHash:
+			workflow = s
+		}
+	}
+	require.NotNil(t, external, "external status must be preserved under the legacy hash")
+	require.NotNil(t, workflow, "workflow status must use its own distinct hash, not the external legacy hash")
+	assert.Equal(t, "https://example.invalid/external", external.TargetURL)
+}
+
 // TestCreateCommitStatus_UnnamedWorkflowUsesFileName: a workflow with no
 // non-blank `name:` uses the file name in the Context, not an empty
 // "/ job (event)" — covers both an omitted and a whitespace-only name.

services/actions/context.go

@@ -134,7 +134,12 @@ func GenerateGiteaContext(ctx context.Context, run *actions_model.ActionRun, att
 
 	if attempt != nil {
 		gitContext["run_attempt"] = strconv.FormatInt(attempt.Attempt, 10)
-		if err := attempt.LoadAttributes(ctx); err == nil {
+		// Only the trigger user is needed for triggering_actor. attempt.LoadAttributes would also re-load
+		// the run this function already holds as a parameter, on the hot task-dispatch path.
+		if err := attempt.LoadTriggerUser(ctx); err != nil {
+			log.Error("GenerateGiteaContext: load trigger user of attempt %d: %v", attempt.ID, err)
+		}
+		if attempt.TriggerUser != nil {
 			gitContext["triggering_actor"] = attempt.TriggerUser.Name
 		}
 	}

services/actions/context_test.go

@@ -9,7 +9,9 @@ import (
 
 	actions_model "gitea.dev/models/actions"
 	"gitea.dev/models/db"
+	repo_model "gitea.dev/models/repo"
 	"gitea.dev/models/unittest"
+	user_model "gitea.dev/models/user"
 	"gitea.dev/modules/json"
 	api "gitea.dev/modules/structs"
 
@@ -318,3 +320,39 @@ func TestFindTaskNeeds(t *testing.T) {
 	assert.Equal(t, "abc", ret["job1"].Outputs["output_a"])
 	assert.Equal(t, "bbb", ret["job1"].Outputs["output_b"])
 }
+
+// TestGenerateGiteaContext_NilAttempt verifies that, with no explicit attempt,
+// use GetLatestAttempt to load the latest attempt and resolve attempt-related context variables.
+func TestGenerateGiteaContext_NilAttempt(t *testing.T) {
+	require.NoError(t, unittest.PrepareTestDatabase())
+
+	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4})
+	require.NoError(t, repo.LoadOwner(t.Context()))
+	actor := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})     // initiated the run
+	triggerer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) // initiated the latest attempt
+
+	run := &actions_model.ActionRun{
+		RepoID: repo.ID, Repo: repo, OwnerID: repo.OwnerID,
+		TriggerUserID: actor.ID, TriggerUser: actor,
+		WorkflowID: "test.yml", Index: 99600, Ref: "refs/heads/main",
+		CommitSHA: "c2d72f548424103f01ee1dc02889c1e2bff816b0", TriggerEvent: "push",
+		Status: actions_model.StatusRunning,
+	}
+	require.NoError(t, db.Insert(t.Context(), run))
+	attempt := &actions_model.ActionRunAttempt{
+		RepoID: repo.ID, RunID: run.ID, Attempt: 3, TriggerUserID: triggerer.ID, Status: actions_model.StatusRunning,
+	}
+	require.NoError(t, db.Insert(t.Context(), attempt))
+	run.LatestAttemptID = attempt.ID
+	job := &actions_model.ActionRunJob{
+		RunID: run.ID, RunAttemptID: attempt.ID, AttemptJobID: 1, RepoID: repo.ID, OwnerID: repo.OwnerID,
+		Name: "j", JobID: "j", Attempt: attempt.Attempt, Status: actions_model.StatusRunning,
+	}
+	require.NoError(t, db.Insert(t.Context(), job))
+
+	// attempt == nil forces the fallback lookup via run.GetLatestAttempt.
+	gitCtx := GenerateGiteaContext(t.Context(), run, nil, job)
+	assert.Equal(t, actor.Name, gitCtx["actor"])
+	assert.Equal(t, triggerer.Name, gitCtx["triggering_actor"])
+	assert.Equal(t, "3", gitCtx["run_attempt"])
+}

services/actions/job_emitter.go

@@ -27,7 +27,7 @@ type jobUpdate struct {
 	RunID int64
 }
 
-func EmitJobsIfReadyByRun(runID int64) error {
+var EmitJobsIfReadyByRun = func(runID int64) error {
 	err := jobEmitterQueue.Push(&jobUpdate{
 		RunID: runID,
 	})

services/actions/reusable_workflow.go

@@ -32,6 +32,20 @@ import (
 // a top-level caller may have at most MaxReusableCallLevels nested callers below it.
 const MaxReusableCallLevels = 9
 
+// checkRunJobLimit rejects an expansion that would push the attempt over actions_model.MaxJobNumPerRun.
+// checkCallerChain bounds nesting *depth*, but a reusable graph also fans out in *breadth*: without a
+// cumulative cap a tiny set of files can drive exponential job-row insertion and exhaust the database.
+func checkRunJobLimit(ctx context.Context, runID, attemptID int64, adding int) error {
+	existing, err := actions_model.CountRunJobsByRunAndAttemptID(ctx, runID, attemptID)
+	if err != nil {
+		return fmt.Errorf("count existing jobs of run %d attempt %d: %w", runID, attemptID, err)
+	}
+	if existing+int64(adding) > actions_model.MaxJobNumPerRun {
+		return fmt.Errorf("workflow run exceeds the maximum of %d jobs", actions_model.MaxJobNumPerRun)
+	}
+	return nil
+}
+
 // loadReusableWorkflowSource resolves the workflow file referenced by a caller's `uses:` and returns its raw bytes,
 // along with the (repo_id, commit_sha) the file was loaded from.
 func loadReusableWorkflowSource(ctx context.Context, run *actions_model.ActionRun, caller *actions_model.ActionRunJob, ref *jobparser.UsesRef) (content []byte, sourceRepoID int64, sourceCommitSHA string, err error) {
@@ -289,6 +303,10 @@ func insertCallerChildren(ctx context.Context, run *actions_model.ActionRun, att
 		return fmt.Errorf("called workflow for caller %d (uses %q) has no jobs", caller.ID, caller.CallUses)
 	}
 
+	if err := checkRunJobLimit(ctx, run.ID, attempt.ID, len(childWorkflows)); err != nil {
+		return err
+	}
+
 	priorChildren, err := actions_model.GetPriorAttemptChildrenByParent(ctx, run.ID, attempt.ID, caller.AttemptJobID)
 	if err != nil {
 		return fmt.Errorf("lookup prior-attempt children of caller %d: %w", caller.ID, err)

services/actions/reusable_workflow_test.go

@@ -207,6 +207,47 @@ func TestResolveUses(t *testing.T) {
 	})
 }
 
+func TestCheckRunJobLimit(t *testing.T) {
+	require.NoError(t, unittest.PrepareTestDatabase())
+
+	const (
+		runID    = 900100
+		attemptA = 910001
+		attemptB = 910002
+	)
+
+	seed := func(attemptID int64, n int) {
+		for i := range n {
+			name := fmt.Sprintf("job-%d-%d", attemptID, i)
+			require.NoError(t, db.Insert(t.Context(), &actions_model.ActionRunJob{
+				RunID:        runID,
+				RunAttemptID: attemptID,
+				RepoID:       1,
+				OwnerID:      1,
+				CommitSHA:    "abcdef",
+				Name:         name,
+				JobID:        name,
+				AttemptJobID: attemptID*1000 + int64(i),
+				Status:       actions_model.StatusBlocked,
+			}))
+		}
+	}
+
+	seed(attemptA, 5)
+	seed(attemptB, 3) // a different attempt of the same run must not count toward attempt A
+
+	limit := actions_model.MaxJobNumPerRun
+
+	// attempt A already holds 5 jobs: filling up to the cap is allowed, one more is rejected.
+	require.NoError(t, checkRunJobLimit(t.Context(), runID, attemptA, limit-5))
+	require.ErrorContains(t, checkRunJobLimit(t.Context(), runID, attemptA, limit-4), "maximum")
+	require.ErrorContains(t, checkRunJobLimit(t.Context(), runID, attemptA, limit), "maximum")
+
+	// the count is scoped to the attempt: attempt B only holds 3 jobs, so attempt A's 5 must not leak in.
+	require.NoError(t, checkRunJobLimit(t.Context(), runID, attemptB, limit-3))
+	require.ErrorContains(t, checkRunJobLimit(t.Context(), runID, attemptB, limit-2), "maximum")
+}
+
 func TestUndoExpansion(t *testing.T) {
 	require.NoError(t, unittest.PrepareTestDatabase())
 	ctx := t.Context()