🛡️ Sentinel: Enforce strict whitelist for rule validation

.jules/sentinel.md

@@ -60,3 +60,10 @@
 **Prevention:**
 1. Implement strict validation on all data items from external lists (`is_valid_rule`).
 2. Filter out items containing dangerous characters (`<`, `>`, `"` etc.) or control codes.
+
+## 2026-05-24 - [Weak Blacklist Validation for Rules]
+**Vulnerability:** The `is_valid_rule` function used a blacklist of dangerous characters (`<>\"'`();{}[]`) to sanitize rules. This missed several dangerous characters like `&`, `|`, and ` ` which could allow command injection or other attacks if the backend processed rules unsafely.
+**Learning:** Blacklists are inherently fragile because they require knowing every possible dangerous character. Whitelists are robust because they define exactly what is allowed.
+**Prevention:**
+1. Replace blacklists with strict whitelists whenever possible.
+2. Use regex to enforce allowable character sets (e.g., `^[a-zA-Z0-9.\-_:*\/]+$`).

main.py

@@ -259,16 +259,15 @@ def validate_profile_id(profile_id: str) -> bool:
 def is_valid_rule(rule: str) -> bool:
     """
     Validates that a rule is safe to use.
-    Rejects potential XSS payloads or control characters.
+    Enforces a strict whitelist of allowed characters.
+    Allowed: Alphanumeric, hyphen, dot, underscore, asterisk, colon (IPv6), slash (CIDR)
     """
-    if not rule or not rule.isprintable():
+    if not rule:
         return False
 
-    # Block characters common in XSS and injection attacks
-    # Allowed: Alphanumeric, hyphen, dot, underscore, asterisk, colon (IPv6), slash (CIDR)
-    # Block: < > " ' ` ( ) ; { } [ ]
-    dangerous_chars = set("<>\"'`();{}[]")
-    if any(c in dangerous_chars for c in rule):
+    # Strict whitelist to prevent injection
+    # ^[a-zA-Z0-9.\-_:*\/]+$
+    if not re.match(r"^[a-zA-Z0-9.\-_:*\/]+$", rule):
         return False
 
     return True

tests/test_security.py

@@ -1,6 +1,8 @@
+
 import pytest
 from unittest.mock import MagicMock
 import main
+from main import is_valid_rule
 
 def test_push_rules_filters_xss_payloads():
     """
@@ -85,3 +87,35 @@
     finally:
         main._api_post_form = original_post_form
         main.log = original_log
+
+@pytest.mark.parametrize("rule,expected_validity", [
+    # Valid Inputs (should pass)
+    ("example.com", True),
+    ("1.2.3.4", True),
+    ("2001:db8::1", True),
+    ("192.168.1.0/24", True),
+    ("*.example.com", True),
+    ("_sip._tcp.example.com", True),
+    ("valid-domain-with-hyphens.com", True),
+    ("sub.domain.example.com", True),
+
+    # Invalid Inputs (should fail)
+    # These contain characters NOT in the whitelist (and some not in the old blacklist)
+    ("example.com && rm -rf /", False),  # Contains spaces and &
+    ("$(whoami).example.com", False),    # Contains $ and ( )
+    ("example.com|nc 1.2.3.4 80", False),# Contains | and space
+    ("example.com; ls", False),          # Contains ; and space
+    ("<script>alert(1)</script>", False),# Contains < > ( )
+    ("invalid@email.com", False),        # Contains @ (not allowed in whitelist)
+    ("http://example.com", True),        # Safe from injection (contains only allowed chars)
+    ("example.com/path", True),          # Allowed (CIDR uses /)
+    ("foo bar", False),                  # Space not allowed
+    ("foo\tbar", False),                 # Tab not allowed
+    ("foo\nbar", False),                 # Newline not allowed
+    ("`touch hacked`", False),           # Backtick not allowed
+])
+def test_is_valid_rule_strict(rule, expected_validity):
+    """
+    Tests the is_valid_rule function against a strict whitelist of inputs.
+    """
+    assert is_valid_rule(rule) == expected_validity, f"Failed for rule: {rule}"