fix(actions): prevent bulk actions from affecting all runners (#38453) (#38457)

Backport #38453

Co-authored-by: xkm <fzc_study@163.com>

routers/web/shared/actions/runners.go

@@ -373,16 +373,18 @@ func RunnerBulkActionPost(ctx *context.Context) {
 		return
 	}
 
-	var runnerIDs []int64
-	if rCtx.IsAdmin {
-		// ATTENTION: it completely depends on the assumption that the doer is "site admin"
-		// So it doesn't do extra permission check to the runner IDs
-		// In the future, if you need to support such operation on non-admin pages, be careful!
-		runnerIDs = ctx.FormStringInt64s("ids")
-	} else {
+	if !rCtx.IsAdmin {
 		ctx.HTTPError(http.StatusForbidden, "bulk actions are admin-only")
 		return
 	}
+	// ATTENTION: it completely depends on the assumption that the doer is "site admin"
+	// So it doesn't do extra permission check to the runner IDs
+	// In the future, if you need to support such operation on non-admin pages, be careful!
+	runnerIDs := ctx.FormStringInt64s("ids")
+	if len(runnerIDs) == 0 {
+		ctx.HTTPError(http.StatusBadRequest, "missing runner IDs")
+		return
+	}
 
 	action := ctx.FormString("action")
 	var successKey, failedKey string

services/actions/schedule_tasks.go

@@ -6,16 +6,22 @@ package actions
 import (
 	"context"
 	"fmt"
+	"maps"
 	"time"
 
 	actions_model "gitea.dev/models/actions"
 	"gitea.dev/models/db"
+	"gitea.dev/models/organization"
+	perm_model "gitea.dev/models/perm"
+	access_model "gitea.dev/models/perm/access"
 	repo_model "gitea.dev/models/repo"
 	"gitea.dev/models/unit"
+	user_model "gitea.dev/models/user"
 	"gitea.dev/modules/json"
 	"gitea.dev/modules/log"
 	"gitea.dev/modules/timeutil"
 	webhook_module "gitea.dev/modules/webhook"
+	"gitea.dev/services/convert"
 )
 
 // StartScheduleTasks start the task
@@ -102,7 +108,19 @@ func startTasks(ctx context.Context) error {
 // It creates an action run based on the schedule, inserts it into the database, and creates commit statuses for each job.
 func CreateScheduleTask(ctx context.Context, spec *actions_model.ActionScheduleSpec) error {
 	cron := spec.Schedule
-	eventPayload := withScheduleInEventPayload(cron.EventPayload, spec.Spec)
+
+	// Scheduled runs carry no webhook payload; synthesize what github.event.* expects.
+	if err := spec.Repo.LoadOwner(ctx); err != nil {
+		return fmt.Errorf("LoadOwner: %w", err)
+	}
+	fields := map[string]any{
+		"repository": convert.ToRepo(ctx, spec.Repo, access_model.Permission{AccessMode: perm_model.AccessModeRead}),
+		"sender":     convert.ToUser(ctx, user_model.NewActionsUser(), nil),
+	}
+	if spec.Repo.Owner.IsOrganization() {
+		fields["organization"] = convert.ToOrganization(ctx, organization.OrgFromUser(spec.Repo.Owner))
+	}
+	eventPayload := withScheduleInEventPayload(cron.EventPayload, spec.Spec, fields)
 
 	// Create a new action run based on the schedule
 	run := &actions_model.ActionRun{
@@ -134,7 +152,7 @@ func CreateScheduleTask(ctx context.Context, spec *actions_model.ActionScheduleS
 	return nil
 }
 
-func withScheduleInEventPayload(eventPayload, schedule string) string {
+func withScheduleInEventPayload(eventPayload, schedule string, fields map[string]any) string {
 	if schedule == "" {
 		return eventPayload
 	}
@@ -153,6 +171,7 @@ func withScheduleInEventPayload(eventPayload, schedule string) string {
 		event = map[string]any{}
 	}
 
+	maps.Copy(event, fields)
 	event["schedule"] = schedule
 	updatedPayload, err := json.Marshal(event)
 	if err != nil {

services/actions/schedule_tasks_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"gitea.dev/modules/json"
+	api "gitea.dev/modules/structs"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -14,7 +15,7 @@ import (
 func TestWithScheduleInEventPayload(t *testing.T) {
 	t.Run("adds schedule to existing payload", func(t *testing.T) {
 		payload := `{"ref":"refs/heads/main"}`
-		updated := withScheduleInEventPayload(payload, "*/5 * * * *")
+		updated := withScheduleInEventPayload(payload, "*/5 * * * *", nil)
 
 		event := map[string]any{}
 		assert.NoError(t, json.Unmarshal([]byte(updated), &event))
@@ -23,7 +24,7 @@ func TestWithScheduleInEventPayload(t *testing.T) {
 	})
 
 	t.Run("adds schedule to null payload", func(t *testing.T) {
-		updated := withScheduleInEventPayload("null", "37 12 5 1 2")
+		updated := withScheduleInEventPayload("null", "37 12 5 1 2", nil)
 
 		event := map[string]any{}
 		assert.NoError(t, json.Unmarshal([]byte(updated), &event))
@@ -31,22 +32,37 @@ func TestWithScheduleInEventPayload(t *testing.T) {
 	})
 
 	t.Run("adds schedule to empty payload", func(t *testing.T) {
-		updated := withScheduleInEventPayload("", "37 12 5 1 2")
+		updated := withScheduleInEventPayload("", "37 12 5 1 2", nil)
 
 		event := map[string]any{}
 		assert.NoError(t, json.Unmarshal([]byte(updated), &event))
 		assert.Equal(t, "37 12 5 1 2", event["schedule"])
 	})
 
+	t.Run("adds schedule with repository, sender, organization", func(t *testing.T) {
+		updated := withScheduleInEventPayload("null", "@weekly", map[string]any{
+			"repository":   &api.Repository{Name: "test-repo"},
+			"sender":       &api.User{UserName: "test-user"},
+			"organization": &api.Organization{Name: "test-org"},
+		})
+
+		event := map[string]any{}
+		assert.NoError(t, json.Unmarshal([]byte(updated), &event))
+		assert.Equal(t, "@weekly", event["schedule"])
+		assert.Equal(t, "test-repo", event["repository"].(map[string]any)["name"])
+		assert.Equal(t, "test-user", event["sender"].(map[string]any)["login"])
+		assert.Equal(t, "test-org", event["organization"].(map[string]any)["name"])
+	})
+
 	t.Run("keeps payload when schedule empty", func(t *testing.T) {
 		payload := `{"ref":"refs/heads/main"}`
-		updated := withScheduleInEventPayload(payload, "")
+		updated := withScheduleInEventPayload(payload, "", nil)
 		assert.Equal(t, payload, updated)
 	})
 
 	t.Run("keeps payload when malformed JSON", func(t *testing.T) {
 		payload := `not a json object`
-		updated := withScheduleInEventPayload(payload, "*/5 * * * *")
+		updated := withScheduleInEventPayload(payload, "*/5 * * * *", nil)
 		assert.Equal(t, payload, updated)
 	})
 }

templates/devtest/flex-list.tmpl

@@ -1,7 +1,17 @@
 {{template "devtest/devtest-header"}}
 <div class="page-content devtest">
 	<div class="ui container">
-		<h3>Flex List (standalone)</h3>
+		<h3>Flex Relaxed List</h3>
+		<div class="flex-container tw-border">
+			<div class="flex-relaxed-list tw-flex-1">
+				<div class="flex-left-right">
+					<span class="gt-ellipsis">left looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong</span>
+					<span>right</span>
+				</div>
+			</div>
+		</div>
+
+		<h3>Flex Divided List (standalone)</h3>
 		<div class="divider"></div>
 		<div class="flex-divided-list items-with-main">
 			<div class="item">
@@ -87,7 +97,7 @@
 
 		<div class="divider"></div>
 
-		<h3>Flex List (with "ui segment")</h3>
+		<h3>Flex Divided List (with "ui segment")</h3>
 		<div class="ui attached segment">
 			<div class="flex-divided-list">
 				<div class="item">item 1</div>
@@ -101,7 +111,7 @@
 				<div class="item">item 2</div>
 			</div>
 		</div>
-		<h3>Flex List (with "ui segment fitted", items have their own padding)</h3>
+		<h3>Flex Divided List (with "ui segment fitted", items have their own padding)</h3>
 		<div class="ui fitted segment">
 			<div class="flex-divided-list items-px-default">
 				<div class="item">item 1</div>

templates/org/header.tmpl

@@ -1,6 +1,6 @@
-<div class="ui container tw-flex tw-gap-4">
+<div class="ui container flex-container">
 	<div>{{ctx.AvatarUtils.Avatar .Org 100}}</div>
-	<div class="flex-relaxed-list">
+	<div class="flex-relaxed-list tw-flex-1">
 		<div class="ui header flex-left-right tw-m-0">
 			<div class="flex-text-block">
 				<span class="tw-text-2xl">{{.Org.DisplayName}}</span>

templates/shared/actions/runner_list.tmpl

@@ -44,9 +44,9 @@
 	<div class="ui attached segment tw-hidden" data-global-init="initRunnerBulkToolbar">
 		<form action="{{$.Link}}/bulk" method="post" class="form-fetch-action">
 			<input type="hidden" name="ids">
-			<button class="ui small button" name="action" value="disable">{{ctx.Locale.Tr "actions.runners.disable_runner"}} <span class="runner-bulk-count"></span></button>
-			<button class="ui small button" name="action" value="enable">{{ctx.Locale.Tr "actions.runners.enable_runner"}} <span class="runner-bulk-count"></span></button>
-			<button class="ui small red button" name="action" value="delete"
+			<button class="ui small button runner-bulk-action" name="action" value="disable">{{ctx.Locale.Tr "actions.runners.disable_runner"}} <span class="runner-bulk-count"></span></button>
+			<button class="ui small button runner-bulk-action" name="action" value="enable">{{ctx.Locale.Tr "actions.runners.enable_runner"}} <span class="runner-bulk-count"></span></button>
+			<button class="ui small red button runner-bulk-action" name="action" value="delete"
 				data-modal-confirm-header="{{ctx.Locale.Tr "actions.runners.delete_runner_header"}}"
 				data-modal-confirm-content="{{ctx.Locale.Tr "actions.runners.delete_runner_notice"}}"
 				>{{ctx.Locale.Tr "actions.runners.delete_runner"}} <span class="runner-bulk-count"></span>

tests/integration/actions_runner_modify_test.go

@@ -196,6 +196,13 @@ func TestActionsRunnerModify(t *testing.T) {
 			doBulk(t, sessionAdmin, "evict", allIDs, http.StatusBadRequest)
 		})
 
+		t.Run("EmptyIDs", func(t *testing.T) {
+			doBulk(t, sessionAdmin, "delete", nil, http.StatusBadRequest)
+			for _, id := range allIDs {
+				unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunner{ID: id})
+			}
+		})
+
 		t.Run("DisableEnable", func(t *testing.T) {
 			doBulk(t, sessionAdmin, "disable", allIDs, http.StatusOK)
 			for _, id := range allIDs {

web_src/css/shared/flex-list.css

@@ -3,6 +3,8 @@
   display: flex;
   flex-direction: column;
   gap: var(--gap-block);
+  min-width: 0; /* keep the same style as "flex-text-block" etc, make the text content wrap/ellipse correctly */
+  max-width: 100%;
 }
 
 .flex-relaxed-list > .divider {

web_src/js/features/admin/common.ts

@@ -36,6 +36,7 @@ function initAdminRunnerBulk(toolbar: HTMLElement) {
 
   const refresh = () => {
     const checked = Array.from(rowCheckboxes).filter((c) => c.checked);
+    formRunnerIds.value = checked.map((c) => c.getAttribute('data-runner-id')!).join(',');
     toggleElem(toolbar, checked.length > 0);
     for (const btn of actionButtons) {
       btn.querySelector<HTMLElement>('.runner-bulk-count')!.textContent = `(${checked.length})`;
@@ -50,15 +51,6 @@ function initAdminRunnerBulk(toolbar: HTMLElement) {
   });
   for (const cb of rowCheckboxes) cb.addEventListener('change', refresh);
   refresh();
-
-  const collectSelectedIds = () => {
-    const ids = [];
-    for (const cb of rowCheckboxes) {
-      if (cb.checked) ids.push(cb.getAttribute('data-runner-id')!);
-    }
-    return ids.join(',');
-  };
-  formRunnerIds.value = collectSelectedIds();
 }
 
 function initAdminUser() {