fix: address Copilot round-2 review for PR #3526

- Reject empty regex at config load (ARGS:// → error, not silent no-op);
  add parser_error regression tests for ById and ByTag.
- Fast-fail when a collection-scoped regex target is matched against a
  scalar variable (no colon in candidate).
- Remove redundant src/utils/regex.h include from ctl action .cc files
  (already provided by their .h; avoids PCRE2_CODE_UNIT_WIDTH redefinition).

Made-with: Cursor

Author: etiennemunnich

src/actions/ctl/rule_remove_target_by_id.cc

@@ -24,7 +24,6 @@
 #include "modsecurity/transaction.h"
 #include "modsecurity/rule_remove_target_entry.h"
 #include "src/utils/string.h"
-#include "src/utils/regex.h"
 
 
 namespace modsecurity {
@@ -67,6 +66,10 @@ bool RuleRemoveTargetById::init(std::string *error) {
                         m_target);
                     return false;
                 }
+            } else {
+                error->assign("Empty regex in ctl:ruleRemoveTargetById: " +
+                    m_target);
+                return false;
             }
         }
     }

src/actions/ctl/rule_remove_target_by_tag.cc

@@ -24,7 +24,6 @@
 #include "modsecurity/transaction.h"
 #include "modsecurity/rule_remove_target_entry.h"
 #include "src/utils/string.h"
-#include "src/utils/regex.h"
 
 
 namespace modsecurity {
@@ -44,6 +43,7 @@ bool RuleRemoveTargetByTag::init(std::string *error) {
     m_tag = param[0];
     m_target = param[1];
 
+    // Detect regex format: COLLECTION:/pattern/ (e.g. ARGS:/mixpanel$/)
     if (m_target.size() >= 4) {
         size_t colon = m_target.find(':');
         if (colon != std::string::npos && colon + 2 < m_target.size() &&
@@ -59,6 +59,10 @@ bool RuleRemoveTargetByTag::init(std::string *error) {
                         m_target);
                     return false;
                 }
+            } else {
+                error->assign("Empty regex in ctl:ruleRemoveTargetByTag: " +
+                    m_target);
+                return false;
             }
         }
     }

src/rule_remove_target_entry.cc

@@ -46,6 +46,12 @@ bool RuleRemoveTargetSpec::matchesKeyWithCollection(
     if (regex) {
         size_t litColon = literal.find(':');
         size_t kwcColon = keyWithCollection.find(':');
+        // Collection-scoped target cannot match a scalar variable.
+        if (litColon != std::string::npos && kwcColon == std::string::npos) {
+            return false;
+        }
+        // Regex targets match only the key portion; verify collection prefix
+        // separately so ARGS:/.../ does not exclude REQUEST_HEADERS variables.
         if (litColon != std::string::npos && kwcColon != std::string::npos) {
             if (!collectionPrefixMatches(literal, litColon,
                                          keyWithCollection, kwcColon)) {
@@ -62,6 +68,12 @@ bool RuleRemoveTargetSpec::matchesFullName(const std::string &fullName) const {
     if (regex) {
         size_t litColon = literal.find(':');
         size_t fullColon = fullName.find(':');
+        // Collection-scoped target cannot match a scalar variable.
+        if (litColon != std::string::npos && fullColon == std::string::npos) {
+            return false;
+        }
+        // Regex targets match only the key portion; verify collection prefix
+        // separately so ARGS:/.../ does not exclude REQUEST_HEADERS variables.
         if (litColon != std::string::npos && fullColon != std::string::npos) {
             if (!collectionPrefixMatches(literal, litColon,
                                          fullName, fullColon)) {

test/test-cases/regression/issue-3505.json

@@ -253,5 +253,91 @@
       "SecRule REQUEST_FILENAME \"@unconditionalMatch\" \"id:100,phase:1,pass,nolog,ctl:ruleRemoveTargetById=1;ARGS:/^X-Evil$/\"",
       "SecRule REQUEST_HEADERS|ARGS \"@contains attack\" \"id:1,phase:2,deny,status:403,tag:'test'\""
     ]
+  },
+  {
+    "enabled": 1,
+    "version_min": 300000,
+    "title": "Issue 3505: empty regex ARGS:// in ctl:ruleRemoveTargetById must fail at config load",
+    "client": {
+      "ip": "200.249.12.31",
+      "port": 123
+    },
+    "server": {
+      "ip": "200.249.12.31",
+      "port": 80
+    },
+    "request": {
+      "headers": {
+        "Host": "localhost",
+        "User-Agent": "curl/7.38.0",
+        "Accept": "*/*",
+        "Content-Length": "0"
+      },
+      "uri": "/",
+      "method": "GET",
+      "body": [
+        ""
+      ]
+    },
+    "response": {
+      "headers": {
+        "Content-Length": "0"
+      },
+      "body": [
+        ""
+      ]
+    },
+    "expected": {
+      "http_code": 200,
+      "parser_error": "Empty regex in ctl:ruleRemoveTargetById: ARGS://"
+    },
+    "rules": [
+      "SecRuleEngine On",
+      "SecRule REQUEST_FILENAME \"@unconditionalMatch\" \"id:100,phase:1,pass,nolog,ctl:ruleRemoveTargetById=1;ARGS://\"",
+      "SecRule ARGS \"@contains test\" \"id:1,phase:2,deny,status:403,tag:'test'\""
+    ]
+  },
+  {
+    "enabled": 1,
+    "version_min": 300000,
+    "title": "Issue 3505: empty regex ARGS:// in ctl:ruleRemoveTargetByTag must fail at config load",
+    "client": {
+      "ip": "200.249.12.31",
+      "port": 123
+    },
+    "server": {
+      "ip": "200.249.12.31",
+      "port": 80
+    },
+    "request": {
+      "headers": {
+        "Host": "localhost",
+        "User-Agent": "curl/7.38.0",
+        "Accept": "*/*",
+        "Content-Length": "0"
+      },
+      "uri": "/",
+      "method": "GET",
+      "body": [
+        ""
+      ]
+    },
+    "response": {
+      "headers": {
+        "Content-Length": "0"
+      },
+      "body": [
+        ""
+      ]
+    },
+    "expected": {
+      "http_code": 200,
+      "parser_error": "Empty regex in ctl:ruleRemoveTargetByTag: ARGS://"
+    },
+    "rules": [
+      "SecRuleEngine On",
+      "SecRule REQUEST_FILENAME \"@unconditionalMatch\" \"id:100,phase:1,pass,nolog,ctl:ruleRemoveTargetByTag=test;ARGS://\"",
+      "SecRule ARGS \"@contains test\" \"id:1,phase:2,deny,status:403,tag:'test'\""
+    ]
   }
 ]