From be255341e58878567684b2dccb9efd79b9754dfa Mon Sep 17 00:00:00 2001 From: squid Date: Mon, 20 Jul 2026 21:00:24 +0200 Subject: [PATCH] Require confirmation and block deleting library actions still referenced by node groups. --- service/internal/api/actions_handlers.go | 11 ++ service/internal/api/actions_handlers_test.go | 144 ++++++++++++++++++ .../internal/settings/node_action_groups.go | 17 +++ .../settings/node_action_groups_test.go | 42 +++++ webui/src/App.tsx | 3 + 5 files changed, 217 insertions(+) diff --git a/service/internal/api/actions_handlers.go b/service/internal/api/actions_handlers.go index 709d8e6..cc3dd26 100644 --- a/service/internal/api/actions_handlers.go +++ b/service/internal/api/actions_handlers.go @@ -49,6 +49,7 @@ var ( errActionIDRequired = errors.New("action id is required") errActionWidgetTypeNeeded = errors.New("widget_type is required when show_widget is true") errActionWidgetTypeBad = errors.New("widget_type must be memory, disk, or raw") + errActionInUse = errors.New("action is used by node action groups; remove or clone to customize first") ) func actionsGetHandler(configDir string) http.HandlerFunc { @@ -221,6 +222,16 @@ func actionsDeleteHandler(configDir string) http.HandlerFunc { return } + groups, err := settings.LoadNodeActionGroupsOrEmpty(configDir) + if err != nil { + writeJSON(writer, http.StatusInternalServerError, apiErrorResponse{Error: err.Error()}) + return + } + if settings.LibraryActionInUse(groups, actionID) { + writeJSON(writer, http.StatusConflict, apiErrorResponse{Error: errActionInUse.Error()}) + return + } + store.Actions = append(store.Actions[:index], store.Actions[index+1:]...) if err := settings.SaveActions(configDir, store); err != nil { writeJSON(writer, http.StatusInternalServerError, apiErrorResponse{Error: err.Error()}) diff --git a/service/internal/api/actions_handlers_test.go b/service/internal/api/actions_handlers_test.go index 017574d..807a3f1 100644 --- a/service/internal/api/actions_handlers_test.go +++ b/service/internal/api/actions_handlers_test.go @@ -257,3 +257,147 @@ func TestActionsMutationsRequirePermissions(t *testing.T) { t.Fatalf("expected delete 403, got %d", deleteRec.Code) } } + +func TestActionsDeleteBlockedWhenLibraryReferenced(t *testing.T) { + configDir := t.TempDir() + cookie := seedCompletedSetup(t, configDir) + node := seedNodeForActions(t, configDir) + router := NewRouter(configDir) + + createBody := []byte(`{"name":"Echo host","kind":"shell","body":"echo hi"}`) + createReq := withSession(httptest.NewRequest(http.MethodPost, "/api/v1/actions", bytes.NewReader(createBody)), cookie) + createRec := httptest.NewRecorder() + router.ServeHTTP(createRec, createReq) + if createRec.Code != http.StatusOK { + t.Fatalf("create status %d body=%s", createRec.Code, createRec.Body.String()) + } + var created actionsResponseTest + if err := json.NewDecoder(createRec.Body).Decode(&created); err != nil { + t.Fatalf("decode create: %v", err) + } + customID := "" + for _, action := range created.Actions { + if !action.Builtin { + customID = action.ID + break + } + } + if customID == "" { + t.Fatal("custom action missing") + } + + groupBody, _ := json.Marshal(map[string]any{ + "name": "Nightly", + "schedule": map[string]any{ + "enabled": false, + "kind": "interval", + "every_seconds": 3600, + "times": []any{}, + }, + }) + groupReq := withSession( + httptest.NewRequest(http.MethodPost, "/api/v1/nodes/"+node.ID+"/action-groups", bytes.NewReader(groupBody)), + cookie, + ) + groupRec := httptest.NewRecorder() + router.ServeHTTP(groupRec, groupReq) + if groupRec.Code != http.StatusOK { + t.Fatalf("create group status=%d body=%s", groupRec.Code, groupRec.Body.String()) + } + var groupResp actionGroupResponse + if err := json.Unmarshal(groupRec.Body.Bytes(), &groupResp); err != nil { + t.Fatalf("decode group: %v", err) + } + groupID := groupResp.Group.ID + + addBody, _ := json.Marshal(map[string]any{ + "source": "library", + "library_action_id": customID, + }) + addReq := withSession( + httptest.NewRequest( + http.MethodPost, + "/api/v1/nodes/"+node.ID+"/action-groups/"+groupID+"/items", + bytes.NewReader(addBody), + ), + cookie, + ) + addRec := httptest.NewRecorder() + router.ServeHTTP(addRec, addReq) + if addRec.Code != http.StatusOK { + t.Fatalf("add item status=%d body=%s", addRec.Code, addRec.Body.String()) + } + var withItem actionGroupResponse + if err := json.Unmarshal(addRec.Body.Bytes(), &withItem); err != nil { + t.Fatalf("decode add: %v", err) + } + itemID := withItem.Group.Items[0].ID + + blockedReq := withSession( + httptest.NewRequest(http.MethodDelete, "/api/v1/actions?id="+customID, nil), + cookie, + ) + blockedRec := httptest.NewRecorder() + router.ServeHTTP(blockedRec, blockedReq) + if blockedRec.Code != http.StatusConflict { + t.Fatalf("expected delete conflict %d, got %d body=%s", http.StatusConflict, blockedRec.Code, blockedRec.Body.String()) + } + var blockedErr apiErrorResponse + if err := json.NewDecoder(blockedRec.Body).Decode(&blockedErr); err != nil { + t.Fatalf("decode conflict: %v", err) + } + if blockedErr.Error != errActionInUse.Error() { + t.Fatalf("conflict error = %q, want %q", blockedErr.Error, errActionInUse.Error()) + } + + listReq := withSession(httptest.NewRequest(http.MethodGet, "/api/v1/actions", nil), cookie) + listRec := httptest.NewRecorder() + router.ServeHTTP(listRec, listReq) + var listed actionsResponseTest + if err := json.NewDecoder(listRec.Body).Decode(&listed); err != nil { + t.Fatalf("decode list: %v", err) + } + stillPresent := false + for _, action := range listed.Actions { + if action.ID == customID { + stillPresent = true + break + } + } + if !stillPresent { + t.Fatal("action should remain after blocked delete") + } + + cloneReq := withSession( + httptest.NewRequest( + http.MethodPost, + "/api/v1/nodes/"+node.ID+"/action-groups/"+groupID+"/items/"+itemID+"/clone", + nil, + ), + cookie, + ) + cloneRec := httptest.NewRecorder() + router.ServeHTTP(cloneRec, cloneReq) + if cloneRec.Code != http.StatusOK { + t.Fatalf("clone status=%d body=%s", cloneRec.Code, cloneRec.Body.String()) + } + + deleteReq := withSession( + httptest.NewRequest(http.MethodDelete, "/api/v1/actions?id="+customID, nil), + cookie, + ) + deleteRec := httptest.NewRecorder() + router.ServeHTTP(deleteRec, deleteReq) + if deleteRec.Code != http.StatusOK { + t.Fatalf("delete after clone status %d body=%s", deleteRec.Code, deleteRec.Body.String()) + } + var deleted actionsResponseTest + if err := json.NewDecoder(deleteRec.Body).Decode(&deleted); err != nil { + t.Fatalf("decode delete: %v", err) + } + for _, action := range deleted.Actions { + if action.ID == customID { + t.Fatal("custom action should be gone after delete") + } + } +} diff --git a/service/internal/settings/node_action_groups.go b/service/internal/settings/node_action_groups.go index 96948d9..a155e61 100644 --- a/service/internal/settings/node_action_groups.go +++ b/service/internal/settings/node_action_groups.go @@ -128,6 +128,23 @@ func GroupsForNode(store NodeActionGroupStore, nodeID string) []NodeActionGroup return result } +// LibraryActionInUse reports whether any group item still references actionID as a library source. +// Local clones (source local) do not count even if they previously came from that action. +func LibraryActionInUse(store NodeActionGroupStore, actionID string) bool { + actionID = strings.TrimSpace(actionID) + if actionID == "" { + return false + } + for _, group := range store.Groups { + for _, item := range group.Items { + if item.Source == ActionItemSourceLibrary && item.LibraryActionID == actionID { + return true + } + } + } + return false +} + // UpdateGroupLastRunAt sets LastRunAt and UpdatedAt for a group and saves. func UpdateGroupLastRunAt(dir string, groupID string, at time.Time) error { store, err := LoadNodeActionGroupsOrEmpty(dir) diff --git a/service/internal/settings/node_action_groups_test.go b/service/internal/settings/node_action_groups_test.go index 5d23f43..f080002 100644 --- a/service/internal/settings/node_action_groups_test.go +++ b/service/internal/settings/node_action_groups_test.go @@ -66,6 +66,48 @@ func TestNodeActionGroupsRoundTrip(t *testing.T) { } } +func TestLibraryActionInUse(t *testing.T) { + store := NodeActionGroupStore{ + Groups: []NodeActionGroup{ + { + ID: "g1", + NodeID: "n1", + Name: "Nightly", + Items: []NodeActionItem{ + { + ID: "lib-item", + Source: ActionItemSourceLibrary, + LibraryActionID: "custom-1", + }, + { + ID: "local-item", + Source: ActionItemSourceLocal, + Name: "Echo", + Kind: ActionKindShell, + Body: "echo hi", + }, + }, + }, + }, + } + if !LibraryActionInUse(store, "custom-1") { + t.Fatal("expected library action custom-1 to be in use") + } + if LibraryActionInUse(store, "other") { + t.Fatal("expected unrelated action not in use") + } + store.Groups[0].Items[0] = NodeActionItem{ + ID: "cloned", + Source: ActionItemSourceLocal, + Name: "Echo host", + Kind: ActionKindShell, + Body: "echo", + } + if LibraryActionInUse(store, "custom-1") { + t.Fatal("expected cloned local item not to count as in use") + } +} + func TestIsScheduleDueInterval(t *testing.T) { now := time.Date(2026, 7, 19, 12, 0, 0, 0, time.Local) group := NodeActionGroup{ diff --git a/webui/src/App.tsx b/webui/src/App.tsx index c77af48..7f5828b 100644 --- a/webui/src/App.tsx +++ b/webui/src/App.tsx @@ -1318,6 +1318,9 @@ function ActionsConfigTab() { if (action.builtin || !canDeleteActions) { return } + if (!window.confirm(`Delete action “${action.name}”?`)) { + return + } setSubmitError(null) try { const response = await deleteAction(action.id)