Fix actions concurrency groups cross-branch leak (#37311)

services/actions/context.go

@@ -73,7 +73,7 @@ func GenerateGiteaContext(run *actions_model.ActionRun, job *actions_model.Actio
 		"repository_owner":  run.Repo.OwnerName,                       // string, The repository owner's name. For example, Codertocat.
 		"repositoryUrl":     run.Repo.HTMLURL(),                       // string, The Git URL to the repository. For example, git://github.com/codertocat/hello-world.git.
 		"retention_days":    "",                                       // string, The number of days that workflow run logs and artifacts are kept.
-		"run_id":            "",                                       // string, A unique number for each workflow run within a repository. This number does not change if you re-run the workflow run.
+		"run_id":            strconv.FormatInt(run.ID, 10),            // string, A unique number for each workflow run within a repository. This number does not change if you re-run the workflow run.
 		"run_number":        strconv.FormatInt(run.Index, 10),         // string, A unique number for each run of a particular workflow in a repository. This number begins at 1 for the workflow's first run, and increments with each new run. This number does not change if you re-run the workflow run.
 		"run_attempt":       "",                                       // string, A unique number for each attempt of a particular workflow run in a repository. This number begins at 1 for the workflow run's first attempt, and increments with each re-run.
 		"secret_source":     "Actions",                                // string, The source of a secret used in a workflow. Possible values are None, Actions, Dependabot, or Codespaces.

services/actions/context_test.go

@@ -4,14 +4,86 @@
 package actions
 
 import (
+	"strconv"
 	"testing"
 
 	actions_model "code.gitea.io/gitea/models/actions"
 	"code.gitea.io/gitea/models/unittest"
 
+	act_model "github.com/nektos/act/pkg/model"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
+func TestEvaluateRunConcurrency_RunIDFallback(t *testing.T) {
+	// Unit-level check that EvaluateRunConcurrencyFillModel resolves
+	// github.run_id from run.ID. The full-flow regression — that run.ID is
+	// non-zero by the time evaluation happens — is in
+	// TestPrepareRunAndInsert_ExpressionsSeeRunID.
+	assert.NoError(t, unittest.PrepareTestDatabase())
+	ctx := t.Context()
+
+	runA := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: 791})
+	runB := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: 792})
+
+	expr := &act_model.RawConcurrency{
+		Group:            "${{ github.workflow }}-${{ github.head_ref || github.run_id }}",
+		CancelInProgress: "true",
+	}
+
+	assert.NoError(t, EvaluateRunConcurrencyFillModel(ctx, runA, expr, nil, nil))
+	assert.NoError(t, EvaluateRunConcurrencyFillModel(ctx, runB, expr, nil, nil))
+
+	assert.Contains(t, runA.ConcurrencyGroup, "791")
+	assert.Contains(t, runB.ConcurrencyGroup, "792")
+	assert.NotEqual(t, runA.ConcurrencyGroup, runB.ConcurrencyGroup)
+}
+
+func TestPrepareRunAndInsert_ExpressionsSeeRunID(t *testing.T) {
+	// Regression for the cross-branch concurrency leak: github.run_id must
+	// be available during BOTH jobparser.Parse (run-name) and workflow-level
+	// concurrency evaluation. Re-ordering db.Insert relative to either step
+	// would leave run.ID at 0 and break this test.
+	assert.NoError(t, unittest.PrepareTestDatabase())
+	ctx := t.Context()
+
+	content := []byte(`name: cross-branch
+run-name: "Run ${{ github.run_id }}"
+on: push
+concurrency:
+  group: group-${{ github.run_id }}
+  cancel-in-progress: true
+jobs:
+  hello:
+    runs-on: ubuntu-latest
+    steps:
+      - run: echo hi
+`)
+
+	run := &actions_model.ActionRun{
+		Title:         "before parse",
+		RepoID:        4,
+		OwnerID:       1,
+		WorkflowID:    "expr-runid.yaml",
+		TriggerUserID: 1,
+		Ref:           "refs/heads/master",
+		CommitSHA:     "c2d72f548424103f01ee1dc02889c1e2bff816b0",
+		Event:         "push",
+		TriggerEvent:  "push",
+		EventPayload:  "{}",
+	}
+	require.NoError(t, PrepareRunAndInsert(ctx, content, run, nil))
+	require.Positive(t, run.ID)
+
+	persisted := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: run.ID})
+	runIDStr := strconv.FormatInt(run.ID, 10)
+	assert.Equal(t, "Run "+runIDStr, persisted.Title)
+	assert.Equal(t, "group-"+runIDStr, persisted.ConcurrencyGroup)
+	// Rerun reads raw_concurrency from the DB to re-evaluate the group;
+	// see services/actions/rerun.go. Must survive the insert.
+	assert.NotEmpty(t, persisted.RawConcurrency)
+}
+
 func TestFindTaskNeeds(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 

services/actions/run.go

@@ -13,6 +13,7 @@ import (
 	"code.gitea.io/gitea/modules/util"
 	notify_service "code.gitea.io/gitea/services/notify"
 
+	act_model "github.com/nektos/act/pkg/model"
 	"go.yaml.in/yaml/v4"
 )
 
@@ -34,25 +35,7 @@ func PrepareRunAndInsert(ctx context.Context, content []byte, run *actions_model
 		return fmt.Errorf("ReadWorkflowRawConcurrency: %w", err)
 	}
 
-	if wfRawConcurrency != nil {
-		err = EvaluateRunConcurrencyFillModel(ctx, run, wfRawConcurrency, vars, inputsWithDefaults)
-		if err != nil {
-			return fmt.Errorf("EvaluateRunConcurrencyFillModel: %w", err)
-		}
-	}
-
-	giteaCtx := GenerateGiteaContext(run, nil)
-
-	jobs, err := jobparser.Parse(content, jobparser.WithVars(vars), jobparser.WithGitContext(giteaCtx.ToGitHubContext()), jobparser.WithInputs(inputsWithDefaults))
-	if err != nil {
-		return fmt.Errorf("parse workflow: %w", err)
-	}
-
-	if len(jobs) > 0 && jobs[0].RunName != "" {
-		run.Title = jobs[0].RunName
-	}
-
-	if err = InsertRun(ctx, run, jobs, vars, inputsWithDefaults); err != nil {
+	if err = InsertRun(ctx, run, content, vars, inputsWithDefaults, wfRawConcurrency); err != nil {
 		return fmt.Errorf("InsertRun: %w", err)
 	}
 
@@ -74,31 +57,44 @@ func PrepareRunAndInsert(ctx context.Context, content []byte, run *actions_model
 
 // InsertRun inserts a run
 // The title will be cut off at 255 characters if it's longer than 255 characters.
-func InsertRun(ctx context.Context, run *actions_model.ActionRun, jobs []*jobparser.SingleWorkflow, vars map[string]string, inputs map[string]any) error {
+func InsertRun(ctx context.Context, run *actions_model.ActionRun, content []byte, vars map[string]string, inputs map[string]any, wfRawConcurrency *act_model.RawConcurrency) error {
 	return db.WithTx(ctx, func(ctx context.Context) error {
 		index, err := db.GetNextResourceIndex(ctx, "action_run_index", run.RepoID)
 		if err != nil {
 			return err
 		}
 		run.Index = index
 		run.Title = util.EllipsisDisplayString(run.Title, 255)
+		run.Status = actions_model.StatusWaiting
 
-		// check run (workflow-level) concurrency
-		run.Status, err = PrepareToStartRunWithConcurrency(ctx, run)
-		if err != nil {
+		// Insert before parsing jobs or evaluating workflow-level concurrency
+		// so that run.ID is populated. Expressions referencing github.run_id —
+		// in run-name, job names, runs-on, or a workflow-level concurrency
+		// group like `${{ github.head_ref || github.run_id }}` — would otherwise
+		// interpolate to an empty string.
+		if err := db.Insert(ctx, run); err != nil {
 			return err
 		}
 
-		if err := db.Insert(ctx, run); err != nil {
-			return err
+		giteaCtx := GenerateGiteaContext(run, nil)
+		jobs, err := jobparser.Parse(content, jobparser.WithVars(vars), jobparser.WithGitContext(giteaCtx.ToGitHubContext()), jobparser.WithInputs(inputs))
+		if err != nil {
+			return fmt.Errorf("parse workflow: %w", err)
 		}
 
-		if err := run.LoadRepo(ctx); err != nil {
-			return err
+		titleChanged := len(jobs) > 0 && jobs[0].RunName != ""
+		if titleChanged {
+			run.Title = util.EllipsisDisplayString(jobs[0].RunName, 255)
 		}
 
-		if err := actions_model.UpdateRepoRunsNumbers(ctx, run.Repo); err != nil {
-			return err
+		if wfRawConcurrency != nil {
+			if err := EvaluateRunConcurrencyFillModel(ctx, run, wfRawConcurrency, vars, inputs); err != nil {
+				return fmt.Errorf("EvaluateRunConcurrencyFillModel: %w", err)
+			}
+			run.Status, err = PrepareToStartRunWithConcurrency(ctx, run)
+			if err != nil {
+				return err
+			}
 		}
 
 		runJobs := make([]*actions_model.ActionRunJob, 0, len(jobs))
@@ -168,7 +164,14 @@ func InsertRun(ctx context.Context, run *actions_model.ActionRun, jobs []*jobpar
 		}
 
 		run.Status = actions_model.AggregateJobStatus(runJobs)
-		if err := actions_model.UpdateRun(ctx, run, "status"); err != nil {
+		cols := []string{"status"}
+		if titleChanged {
+			cols = append(cols, "title")
+		}
+		if wfRawConcurrency != nil {
+			cols = append(cols, "raw_concurrency", "concurrency_group", "concurrency_cancel")
+		}
+		if err := actions_model.UpdateRun(ctx, run, cols...); err != nil {
 			return err
 		}