From 0f6bafae8ee15dc71ad08bb72123636474eac700 Mon Sep 17 00:00:00 2001
From: hsiegeln <37154749+hsiegeln@users.noreply.github.com>
Date: Wed, 22 Apr 2026 17:31:11 +0200
Subject: [PATCH] alerting(api): cross-field validation for PER_EXCHANGE +
 empty-targets guard

PER_EXCHANGE rules: 400 if reNotifyMinutes != 0 or forDurationSeconds != 0.
Any rule: 400 if webhooks + targets are both empty (never notifies anyone).

Turns green: AlertRuleControllerIT#createPerExchangeRule_with*NonZero_returns400,
AlertRuleControllerIT#createAnyRule_withEmptyWebhooksAndTargets_returns400.
---
 .../controller/AlertRuleController.java       | 33 +++++++++++++++++++
 .../controller/AlertRuleControllerIT.java     |  7 ++--
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/controller/AlertRuleController.java b/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/controller/AlertRuleController.java
index 73477466..792b87ca 100644
--- a/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/controller/AlertRuleController.java
+++ b/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/controller/AlertRuleController.java
@@ -22,6 +22,7 @@ import com.cameleer.server.core.alerting.AlertRuleRepository;
 import com.cameleer.server.core.alerting.AlertRuleTarget;
 import com.cameleer.server.core.alerting.ConditionKind;
 import com.cameleer.server.core.alerting.ExchangeMatchCondition;
+import com.cameleer.server.core.alerting.FireMode;
 import com.cameleer.server.core.alerting.WebhookBinding;
 import com.cameleer.server.core.outbound.OutboundConnection;
 import com.cameleer.server.core.outbound.OutboundConnectionService;
@@ -126,6 +127,7 @@ public class AlertRuleController {
             HttpServletRequest httpRequest) {
 
         validateAttributeKeys(req.condition());
+        validateBusinessRules(req);
         validateWebhooks(req.webhooks(), env.id());
 
         AlertRule draft = buildRule(null, env.id(), req, currentUserId());
@@ -147,6 +149,7 @@ public class AlertRuleController {
 
         AlertRule existing = requireRule(id, env.id());
         validateAttributeKeys(req.condition());
+        validateBusinessRules(req);
         validateWebhooks(req.webhooks(), env.id());
 
         AlertRule updated = buildRule(existing, env.id(), req, currentUserId());
@@ -258,6 +261,36 @@ public class AlertRuleController {
     // Helpers
     // -------------------------------------------------------------------------
 
+    /**
+     * Cross-field business-rule validation for {@link AlertRuleRequest}.
+     *
+     * <p>PER_EXCHANGE rules: re-notify and for-duration are nonsensical (each fire is its own
+     * exchange — there's no "still firing" window and nothing to re-notify about). Reject 400
+     * if either is non-zero.
+     *
+     * <p>All rules: reject 400 if both webhooks and targets are empty — such a rule can never
+     * notify anyone and is a pure footgun.
+     */
+    private void validateBusinessRules(AlertRuleRequest req) {
+        if (req.condition() instanceof ExchangeMatchCondition ex
+                && ex.fireMode() == FireMode.PER_EXCHANGE) {
+            if (req.reNotifyMinutes() != null && req.reNotifyMinutes() != 0) {
+                throw new ResponseStatusException(HttpStatus.BAD_REQUEST,
+                        "reNotifyMinutes must be 0 for PER_EXCHANGE rules (re-notify does not apply)");
+            }
+            if (req.forDurationSeconds() != null && req.forDurationSeconds() != 0) {
+                throw new ResponseStatusException(HttpStatus.BAD_REQUEST,
+                        "forDurationSeconds must be 0 for PER_EXCHANGE rules");
+            }
+        }
+        boolean noWebhooks = req.webhooks() == null || req.webhooks().isEmpty();
+        boolean noTargets  = req.targets()  == null || req.targets().isEmpty();
+        if (noWebhooks && noTargets) {
+            throw new ResponseStatusException(HttpStatus.BAD_REQUEST,
+                    "rule must have at least one webhook or target — otherwise it never notifies anyone");
+        }
+    }
+
     /**
      * Validates that all attribute keys in an {@link ExchangeMatchCondition} match
      * {@code ^[a-zA-Z0-9._-]+$}. Keys are inlined into ClickHouse SQL, making this
diff --git a/cameleer-server-app/src/test/java/com/cameleer/server/app/alerting/controller/AlertRuleControllerIT.java b/cameleer-server-app/src/test/java/com/cameleer/server/app/alerting/controller/AlertRuleControllerIT.java
index 76392912..56d7b9a2 100644
--- a/cameleer-server-app/src/test/java/com/cameleer/server/app/alerting/controller/AlertRuleControllerIT.java
+++ b/cameleer-server-app/src/test/java/com/cameleer/server/app/alerting/controller/AlertRuleControllerIT.java
@@ -164,7 +164,8 @@ class AlertRuleControllerIT extends AbstractPostgresIT {
                 {"name":"valid-attr","severity":"WARNING","conditionKind":"EXCHANGE_MATCH",
                  "condition":{"kind":"EXCHANGE_MATCH","scope":{},
                    "filter":{"status":"FAILED","attributes":{"order.type":"x"}},
-                   "fireMode":"PER_EXCHANGE"}}
+                   "fireMode":"PER_EXCHANGE"},
+                 "targets":[{"kind":"USER","targetId":"test-operator"}]}
                 """;
 
         ResponseEntity<String> resp = restTemplate.exchange(
@@ -324,10 +325,12 @@ class AlertRuleControllerIT extends AbstractPostgresIT {
     }
 
     private static String routeMetricRuleBody(String name) {
+        // Includes a USER target so the rule passes the "at least one webhook or target" guard.
         return """
                 {"name":"%s","severity":"WARNING","conditionKind":"ROUTE_METRIC",
                  "condition":{"kind":"ROUTE_METRIC","scope":{},
-                   "metric":"ERROR_RATE","comparator":"GT","threshold":0.05,"windowSeconds":60}}
+                   "metric":"ERROR_RATE","comparator":"GT","threshold":0.05,"windowSeconds":60},
+                 "targets":[{"kind":"USER","targetId":"test-operator"}]}
                 """.formatted(name);
     }