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

Backport #38453

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

Author: GiteaBot

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

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/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() {