🛡️ Sentinel: [HIGH] Validate rule input to prevent XSS by google-labs-jules[bot] · Pull Request #117 · abhimehro/ctrld-sync

google-labs-jules · 2026-01-20T10:47:37Z

Implemented is_valid_rule helper function and integrated it into push_rules to filter out potentially malicious inputs (e.g. <script>) from external blocklists.
Added tests in tests/test_security.py.

PR created automatically by Jules for task 17576703938918876849 started by @abhimehro

Validates rule content from external JSON lists to ensure they do not contain malicious characters (XSS payloads, control characters) before pushing to Control D API. Filters out unsafe rules and logs a warning. Fixes potential Stored XSS vulnerability via malicious blocklists.

google-labs-jules · 2026-01-20T10:47:38Z

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

For security, I will only act on instructions from the user who triggered this task.

trunk-io · 2026-01-20T10:47:41Z

Merging to main in this repository is managed by Trunk.

To merge this pull request, check the box to the left or comment /trunk merge below.

tests/test_security.py

+        )
+
+        # Check what was sent
+        assert mock_post_form.called


tests/test_security.py

+                    sent_rules.append(v)
+
+        # EXPECTED BEHAVIOR: Malicious rules are NOT sent
+        assert "<script>alert(1)</script>" not in sent_rules


tests/test_security.py

+
+        # EXPECTED BEHAVIOR: Malicious rules are NOT sent
+        assert "<script>alert(1)</script>" not in sent_rules
+        assert "javascript:void(0)" not in sent_rules # Contains parenthesis/colon?


tests/test_security.py

+        assert "<script>alert(1)</script>" not in sent_rules
+        assert "javascript:void(0)" not in sent_rules # Contains parenthesis/colon?
+        # Wait, 'javascript:void(0)' has '('. My validator blocks '('.
+        assert "fail' OR '1'='1" not in sent_rules # Contains '


tests/test_security.py

+        assert "javascript:void(0)" not in sent_rules # Contains parenthesis/colon?
+        # Wait, 'javascript:void(0)' has '('. My validator blocks '('.
+        assert "fail' OR '1'='1" not in sent_rules # Contains '
+        assert "img src=x onerror=alert(1)" not in sent_rules # Contains ( ) or =? No = is allowed?


tests/test_security.py

+
+        # Valid rules MUST be sent
+        assert "valid.com" in sent_rules
+        assert "safe-domain.com" in sent_rules


tests/test_security.py

+        # Valid rules MUST be sent
+        assert "valid.com" in sent_rules
+        assert "safe-domain.com" in sent_rules
+        assert "1.1.1.1" in sent_rules


tests/test_security.py

+        assert "valid.com" in sent_rules
+        assert "safe-domain.com" in sent_rules
+        assert "1.1.1.1" in sent_rules
+        assert "*.wildcard.com" in sent_rules


tests/test_security.py

+
+        # Check logs for warnings
+        # We expect 4 skipped rules
+        assert mock_log.warning.call_count >= 1


tests/test_security.py

+        for call in mock_log.warning.call_args_list:
+            if "Skipping unsafe rule" in str(call):
+                found_unsafe_log = True
+        assert found_unsafe_log


tests/test_security.py

.jules/sentinel.md

 **Prevention:**
 1. Always use `if not ip.is_global or ip.is_multicast:` for strict SSRF filtering, rather than manual blacklists of private ranges.
+
+## 2026-05-14 - [Indirect XSS via Third-Party Data]


.jules/sentinel.md

 **Prevention:**
 1. Always use `if not ip.is_global or ip.is_multicast:` for strict SSRF filtering, rather than manual blacklists of private ranges.
+
+## 2026-05-14 - [Indirect XSS via Third-Party Data]


main.py

+
    for h in hostnames:
+        if not is_valid_rule(h):
+            log.warning(f"Skipping unsafe rule in {sanitize_for_log(folder_name)}: {sanitize_for_log(h)}")


main.py


-    duplicates_count = original_count - len(filtered_hostnames)
+    if skipped_unsafe > 0:
+        log.warning(f"Folder {sanitize_for_log(folder_name)}: skipped {skipped_unsafe} unsafe rules")


tests/test_security.py

@@ -0,0 +1,87 @@
+import pytest


tests/test_security.py

@@ -0,0 +1,87 @@
+import pytest


tests/test_security.py

+    mock_post_form = MagicMock()
+
+    # Patch the API call
+    original_post_form = main._api_post_form


tests/test_security.py

+
+    # Patch the API call
+    original_post_form = main._api_post_form
+    main._api_post_form = mock_post_form


tests/test_security.py

+
+        sent_rules = []
+        for call in calls:
+            args, kwargs = call


tests/test_security.py

+        assert found_unsafe_log
+
+    finally:
+        main._api_post_form = original_post_form


tests/test_security.py

+        )
+
+        # Check what was sent
+        assert mock_post_form.called


tests/test_security.py

+                    sent_rules.append(v)
+
+        # EXPECTED BEHAVIOR: Malicious rules are NOT sent
+        assert "<script>alert(1)</script>" not in sent_rules


tests/test_security.py

+
+        # EXPECTED BEHAVIOR: Malicious rules are NOT sent
+        assert "<script>alert(1)</script>" not in sent_rules
+        assert "javascript:void(0)" not in sent_rules # Contains parenthesis/colon?


tests/test_security.py

+        assert "<script>alert(1)</script>" not in sent_rules
+        assert "javascript:void(0)" not in sent_rules # Contains parenthesis/colon?
+        # Wait, 'javascript:void(0)' has '('. My validator blocks '('.
+        assert "fail' OR '1'='1" not in sent_rules # Contains '


tests/test_security.py

+        assert "javascript:void(0)" not in sent_rules # Contains parenthesis/colon?
+        # Wait, 'javascript:void(0)' has '('. My validator blocks '('.
+        assert "fail' OR '1'='1" not in sent_rules # Contains '
+        assert "img src=x onerror=alert(1)" not in sent_rules # Contains ( ) or =? No = is allowed?


tests/test_security.py

+
+        # Valid rules MUST be sent
+        assert "valid.com" in sent_rules
+        assert "safe-domain.com" in sent_rules


tests/test_security.py

+        # Valid rules MUST be sent
+        assert "valid.com" in sent_rules
+        assert "safe-domain.com" in sent_rules
+        assert "1.1.1.1" in sent_rules


tests/test_security.py

+        assert "valid.com" in sent_rules
+        assert "safe-domain.com" in sent_rules
+        assert "1.1.1.1" in sent_rules
+        assert "*.wildcard.com" in sent_rules


tests/test_security.py

+
+        # Check logs for warnings
+        # We expect 4 skipped rules
+        assert mock_log.warning.call_count >= 1


tests/test_security.py

+        for call in mock_log.warning.call_args_list:
+            if "Skipping unsafe rule" in str(call):
+                found_unsafe_log = True
+        assert found_unsafe_log


main.py

+
    for h in hostnames:
+        if not is_valid_rule(h):
+            log.warning(f"Skipping unsafe rule in {sanitize_for_log(folder_name)}: {sanitize_for_log(h)}")


main.py


-    duplicates_count = original_count - len(filtered_hostnames)
+    if skipped_unsafe > 0:
+        log.warning(f"Folder {sanitize_for_log(folder_name)}: skipped {skipped_unsafe} unsafe rules")


tests/test_security.py

@@ -0,0 +1,87 @@
+import pytest


tests/test_security.py

@@ -0,0 +1,87 @@
+import pytest
+from unittest.mock import MagicMock


tests/test_security.py

+        for call in calls:
+            args, kwargs = call
+            data = kwargs['data']
+            for k, v in data.items():


tests/test_security.py

@@ -0,0 +1,87 @@
+import pytest


tests/test_security.py

+    mock_post_form = MagicMock()
+
+    # Patch the API call
+    original_post_form = main._api_post_form


tests/test_security.py

+
+    # Patch the API call
+    original_post_form = main._api_post_form
+    main._api_post_form = mock_post_form


tests/test_security.py

+
+        sent_rules = []
+        for call in calls:
+            args, kwargs = call


tests/test_security.py

+        assert found_unsafe_log
+
+    finally:
+        main._api_post_form = original_post_form


main.py

+
    for h in hostnames:
+        if not is_valid_rule(h):
+            log.warning(f"Skipping unsafe rule in {sanitize_for_log(folder_name)}: {sanitize_for_log(h)}")


main.py


-    duplicates_count = original_count - len(filtered_hostnames)
+    if skipped_unsafe > 0:
+        log.warning(f"Folder {sanitize_for_log(folder_name)}: skipped {skipped_unsafe} unsafe rules")


tests/test_security.py

@@ -0,0 +1,87 @@
+import pytest


tests/test_security.py

@@ -0,0 +1,87 @@
+import pytest
+from unittest.mock import MagicMock


tests/test_security.py

+        for call in calls:
+            args, kwargs = call
+            data = kwargs['data']
+            for k, v in data.items():


tests/test_security.py

@@ -0,0 +1,87 @@
+import pytest


tests/test_security.py

+    mock_post_form = MagicMock()
+
+    # Patch the API call
+    original_post_form = main._api_post_form


tests/test_security.py

+
+    # Patch the API call
+    original_post_form = main._api_post_form
+    main._api_post_form = mock_post_form


tests/test_security.py

+
+        sent_rules = []
+        for call in calls:
+            args, kwargs = call


tests/test_security.py

+        assert found_unsafe_log
+
+    finally:
+        main._api_post_form = original_post_form


Copilot

Pull request overview

This PR implements XSS validation for rule inputs from external blocklists by adding a new is_valid_rule() helper function and integrating it into the push_rules() function. The implementation filters out potentially malicious inputs (e.g., <script>, javascript:void(0)) from external blocklists before they can be stored in user profiles.

Changes:

Added is_valid_rule() validation function to block dangerous characters commonly used in XSS and injection attacks
Integrated validation into push_rules() with logging for skipped unsafe rules
Added integration test in tests/test_security.py to verify XSS payloads are filtered
Documented the vulnerability and prevention strategy in .jules/sentinel.md

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
main.py	Added `is_valid_rule()` function to validate rule safety and integrated it into `push_rules()` to filter out malicious inputs before API submission
tests/test_security.py	Added integration test to verify that XSS payloads are filtered while legitimate domain rules are allowed through
.jules/sentinel.md	Documented the indirect XSS vulnerability via third-party data and prevention measures

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

main.py

Copilot · 2026-01-21T18:45:19Z

tests/test_security.py

+def test_push_rules_filters_xss_payloads():
+    """
+    Verify that push_rules filters out malicious strings (XSS payloads).
+    """
+    mock_client = MagicMock()
+    mock_post_form = MagicMock()
+
+    # Patch the API call
+    original_post_form = main._api_post_form
+    main._api_post_form = mock_post_form
+
+    # Patch the logger to verify warnings
+    mock_log = MagicMock()
+    original_log = main.log
+    main.log = mock_log
+
+    try:
+        malicious_rules = [
+            "<script>alert(1)</script>",
+            "valid.com",
+            "javascript:void(0)",
+            "fail' OR '1'='1",
+            "img src=x onerror=alert(1)",
+            "safe-domain.com",
+            "1.1.1.1",
+            "*.wildcard.com"
+        ]
+
+        main.push_rules(
+            profile_id="p1",
+            folder_name="f1",
+            folder_id="fid1",
+            do=1,
+            status=1,
+            hostnames=malicious_rules,
+            existing_rules=set(),
+            client=mock_client
+        )
+
+        # Check what was sent
+        assert mock_post_form.called
+        calls = mock_post_form.call_args_list
+
+        sent_rules = []
+        for call in calls:
+            args, kwargs = call
+            data = kwargs['data']
+            for k, v in data.items():
+                if k.startswith("hostnames["):
+                    sent_rules.append(v)
+
+        # EXPECTED BEHAVIOR: Malicious rules are NOT sent
+        assert "<script>alert(1)</script>" not in sent_rules
+        assert "javascript:void(0)" not in sent_rules # Contains parenthesis/colon?
+        # Wait, 'javascript:void(0)' has '('. My validator blocks '('.
+        assert "fail' OR '1'='1" not in sent_rules # Contains '
+        assert "img src=x onerror=alert(1)" not in sent_rules # Contains ( ) or =? No = is allowed?
+        # "img src=x onerror=alert(1)" contains spaces?
+        # My validator: isprintable() is True.
+        # dangerous_chars: set("<>\"'`();{}[]")
+        # <script> has < >
+        # javascript:void(0) has ( )
+        # fail' has '
+        # img src=x ... has ( )
+
+        # Valid rules MUST be sent
+        assert "valid.com" in sent_rules
+        assert "safe-domain.com" in sent_rules
+        assert "1.1.1.1" in sent_rules
+        assert "*.wildcard.com" in sent_rules
+
+        # Check logs for warnings
+        # We expect 4 skipped rules
+        assert mock_log.warning.call_count >= 1
+        found_unsafe_log = False
+        for call in mock_log.warning.call_args_list:
+            if "Skipping unsafe rule" in str(call):
+                found_unsafe_log = True
+        assert found_unsafe_log
+
+    finally:
+        main._api_post_form = original_post_form
+        main.log = original_log


The test file uses a plain function with manual mocking rather than following the unittest.TestCase pattern used consistently in other test files (test_ssrf.py, test_ssrf_enhanced.py, test_ux.py). This inconsistency makes the test suite harder to maintain and run. Consider refactoring to use unittest.TestCase with proper setUp/tearDown methods for consistent test structure.

Copilot · 2026-01-21T18:45:19Z

tests/test_security.py

+import pytest
+from unittest.mock import MagicMock
+import main
+
+def test_push_rules_filters_xss_payloads():
+    """
+    Verify that push_rules filters out malicious strings (XSS payloads).
+    """
+    mock_client = MagicMock()
+    mock_post_form = MagicMock()
+
+    # Patch the API call
+    original_post_form = main._api_post_form
+    main._api_post_form = mock_post_form
+
+    # Patch the logger to verify warnings
+    mock_log = MagicMock()
+    original_log = main.log
+    main.log = mock_log
+
+    try:
+        malicious_rules = [
+            "<script>alert(1)</script>",
+            "valid.com",
+            "javascript:void(0)",
+            "fail' OR '1'='1",
+            "img src=x onerror=alert(1)",
+            "safe-domain.com",
+            "1.1.1.1",
+            "*.wildcard.com"
+        ]
+
+        main.push_rules(
+            profile_id="p1",
+            folder_name="f1",
+            folder_id="fid1",
+            do=1,
+            status=1,
+            hostnames=malicious_rules,
+            existing_rules=set(),
+            client=mock_client
+        )
+
+        # Check what was sent
+        assert mock_post_form.called
+        calls = mock_post_form.call_args_list
+
+        sent_rules = []
+        for call in calls:
+            args, kwargs = call
+            data = kwargs['data']
+            for k, v in data.items():
+                if k.startswith("hostnames["):
+                    sent_rules.append(v)
+
+        # EXPECTED BEHAVIOR: Malicious rules are NOT sent
+        assert "<script>alert(1)</script>" not in sent_rules
+        assert "javascript:void(0)" not in sent_rules # Contains parenthesis/colon?
+        # Wait, 'javascript:void(0)' has '('. My validator blocks '('.
+        assert "fail' OR '1'='1" not in sent_rules # Contains '
+        assert "img src=x onerror=alert(1)" not in sent_rules # Contains ( ) or =? No = is allowed?
+        # "img src=x onerror=alert(1)" contains spaces?
+        # My validator: isprintable() is True.
+        # dangerous_chars: set("<>\"'`();{}[]")
+        # <script> has < >
+        # javascript:void(0) has ( )
+        # fail' has '
+        # img src=x ... has ( )
+
+        # Valid rules MUST be sent
+        assert "valid.com" in sent_rules
+        assert "safe-domain.com" in sent_rules
+        assert "1.1.1.1" in sent_rules
+        assert "*.wildcard.com" in sent_rules
+
+        # Check logs for warnings
+        # We expect 4 skipped rules
+        assert mock_log.warning.call_count >= 1
+        found_unsafe_log = False
+        for call in mock_log.warning.call_args_list:
+            if "Skipping unsafe rule" in str(call):
+                found_unsafe_log = True
+        assert found_unsafe_log
+
+    finally:
+        main._api_post_form = original_post_form
+        main.log = original_log


There is no direct unit test for the is_valid_rule function itself. While the integration test checks the behavior through push_rules, adding dedicated unit tests for is_valid_rule would improve test coverage and make it easier to verify edge cases like empty strings, non-printable characters, and each dangerous character individually.

tests/test_security.py

…ng sanitization Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

tests/test_security.py

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

abhimehro · 2026-01-22T10:14:48Z

Hi @google-lab-jules, please open a new pull request to apply any remaining changes based on the comments in this thread. Thanks!

tests/test_security.py

google-labs-jules · 2026-01-22T10:15:53Z

Hi @google-lab-jules, please open a new pull request to apply any remaining changes based on the comments in this thread. Thanks!

Acknowledged. I have implemented the security fix for validating rules to prevent XSS and have submitted the changes in the sentinel-validate-rules branch.

Validates rule content from external JSON lists to ensure they do not contain malicious characters (XSS payloads, control characters) before pushing to Control D API. Filters out unsafe rules and logs a warning. Fixes potential Stored XSS vulnerability via malicious blocklists.

github-actions bot added the python label Jan 20, 2026

github-advanced-security bot found potential problems Jan 20, 2026

View reviewed changes

tests/test_security.py Dismissed Show dismissed Hide dismissed

tests/test_security.py Dismissed Show dismissed Hide dismissed

github-advanced-security bot found potential problems Jan 20, 2026

View reviewed changes

abhimehro marked this pull request as ready for review January 21, 2026 18:39

Copilot AI review requested due to automatic review settings January 21, 2026 18:39

abhimehro assigned abhimehro and Copilot Jan 21, 2026

Copilot started reviewing on behalf of abhimehro January 21, 2026 18:40 View session

Copilot AI reviewed Jan 21, 2026

View reviewed changes

Potential fix for code scanning alert no. 920: Incomplete URL substri…

0c761c6

…ng sanitization Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

github-advanced-security bot found potential problems Jan 21, 2026

View reviewed changes