🛡️ Sentinel: [HIGH] Fix Stored XSS risk in folder names by google-labs-jules[bot] · Pull Request #131 · abhimehro/ctrld-sync

google-labs-jules · 2026-01-25T11:07:51Z

Implemented strict input validation for folder names in external JSON files. This prevents potential Stored XSS attacks where a malicious folder name could be injected into the Control D dashboard.

Validation rules:

Must be a non-empty string.
Must be printable.
Must NOT contain < > " ' `.

Added comprehensive unit tests covering valid names, XSS payloads, non-string types, and empty strings.

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

- Added `is_valid_folder_name` to validate folder names against dangerous characters (<, >, ", ', `) and non-printable characters. - Updated `validate_folder_data` to enforce string type and folder name validity. - Added `tests/test_folder_validation.py` to verify security constraints. Risk: Low (Validation only) Impact: High (Prevents Stored XSS)

google-labs-jules · 2026-01-25T11:07:53Z

👋 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-25T11:07:55Z

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.

.jules/sentinel.md

 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-15 - [Stored XSS in Folder Names]


.jules/sentinel.md

 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-15 - [Stored XSS in Folder Names]


main.py

+
+    folder_name = data["group"]["group"]
+    if not isinstance(folder_name, str):
+        log.error(f"Invalid data from {sanitize_for_log(url)}: Folder name must be a string.")


main.py

+        return False
+
+    if not is_valid_folder_name(folder_name):
+        log.error(f"Invalid data from {sanitize_for_log(url)}: Invalid folder name (empty, unsafe characters, or non-printable).")


tests/test_folder_validation.py

+
+sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
+
+import main


main.py

+        return False
+
+    if not is_valid_folder_name(folder_name):
+        log.error(f"Invalid data from {sanitize_for_log(url)}: Invalid folder name (empty, unsafe characters, or non-printable).")


tests/test_folder_validation.py

@@ -0,0 +1,44 @@
+import unittest


tests/test_folder_validation.py

+
+sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
+
+import main


tests/test_folder_validation.py

+
+import main
+
+class TestFolderValidation(unittest.TestCase):


tests/test_folder_validation.py

+    def tearDown(self):
+        main.log = self.original_log
+
+    def test_valid_folder_name(self):


tests/test_folder_validation.py

+        data = {"group": {"group": "Safe Folder"}}
+        self.assertTrue(main.validate_folder_data(data, "test_url"))
+
+    def test_xss_folder_name(self):


tests/test_folder_validation.py

+        # Assuming we will log "Invalid folder name" or similar
+        self.assertTrue("Invalid folder name" in args or "Unsafe characters" in args)
+
+    def test_non_string_folder_name(self):


tests/test_folder_validation.py

+        args = str(self.mock_log.error.call_args_list)
+        self.assertTrue("must be a string" in args)
+
+    def test_empty_folder_name(self):


main.py

+        return False
+
+    if not is_valid_folder_name(folder_name):
+        log.error(f"Invalid data from {sanitize_for_log(url)}: Invalid folder name (empty, unsafe characters, or non-printable).")


tests/test_folder_validation.py

@@ -0,0 +1,44 @@
+import unittest


tests/test_folder_validation.py

+
+sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
+
+import main


tests/test_folder_validation.py

+
+import main
+
+class TestFolderValidation(unittest.TestCase):


tests/test_folder_validation.py

+    def tearDown(self):
+        main.log = self.original_log
+
+    def test_valid_folder_name(self):


tests/test_folder_validation.py

+        data = {"group": {"group": "Safe Folder"}}
+        self.assertTrue(main.validate_folder_data(data, "test_url"))
+
+    def test_xss_folder_name(self):


tests/test_folder_validation.py

+        # Assuming we will log "Invalid folder name" or similar
+        self.assertTrue("Invalid folder name" in args or "Unsafe characters" in args)
+
+    def test_non_string_folder_name(self):


tests/test_folder_validation.py

+        args = str(self.mock_log.error.call_args_list)
+        self.assertTrue("must be a string" in args)
+
+    def test_empty_folder_name(self):


main.py

+
+    folder_name = data["group"]["group"]
+    if not isinstance(folder_name, str):
+        log.error(f"Invalid data from {sanitize_for_log(url)}: Folder name must be a string.")


main.py

+        return False
+
+    if not is_valid_folder_name(folder_name):
+        log.error(f"Invalid data from {sanitize_for_log(url)}: Invalid folder name (empty, unsafe characters, or non-printable).")


Copilot

Pull request overview

This PR addresses a Stored XSS security vulnerability by implementing input validation for folder names loaded from external JSON files. Previously, while rule content was validated using is_valid_rule(), folder names were not validated, allowing malicious JSON files to inject XSS payloads that could be stored via the Control D API.

Changes:

Added is_valid_folder_name() validation function that rejects XSS-dangerous characters (<>"'`) while allowing printable characters and brackets/parentheses
Integrated validation into validate_folder_data() to check folder names before use
Added comprehensive unit tests covering valid names, XSS payloads, non-string types, and empty strings

Reviewed changes

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

File	Description
main.py	Implements `is_valid_folder_name()` validation function and integrates it into `validate_folder_data()` to prevent XSS in folder names
tests/test_folder_validation.py	Adds unit tests for folder name validation covering XSS payloads, type validation, and empty strings
.jules/sentinel.md	Documents the vulnerability, learning, and prevention measures for this security fix

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

Copilot · 2026-01-27T07:33:03Z

tests/test_folder_validation.py

+class TestFolderValidation(unittest.TestCase):
+    def setUp(self):
+        self.mock_log = MagicMock()
+        self.original_log = main.log
+        main.log = self.mock_log
+
+    def tearDown(self):
+        main.log = self.original_log
+
+    def test_valid_folder_name(self):
+        data = {"group": {"group": "Safe Folder"}}
+        self.assertTrue(main.validate_folder_data(data, "test_url"))
+
+    def test_xss_folder_name(self):
+        # Should be rejected
+        data = {"group": {"group": "<script>alert(1)</script>"}}
+        self.assertFalse(main.validate_folder_data(data, "test_url"))
+        # Check that error was logged
+        self.assertTrue(self.mock_log.error.called)
+        # We expect a specific error message about unsafe characters or invalid name
+        args = str(self.mock_log.error.call_args_list)
+        # Assuming we will log "Invalid folder name" or similar
+        self.assertTrue("Invalid folder name" in args or "Unsafe characters" in args)
+
+    def test_non_string_folder_name(self):
+        data = {"group": {"group": 123}}
+        self.assertFalse(main.validate_folder_data(data, "test_url"))
+        self.assertTrue(self.mock_log.error.called)
+        args = str(self.mock_log.error.call_args_list)
+        self.assertTrue("must be a string" in args)
+
+    def test_empty_folder_name(self):
+        data = {"group": {"group": "   "}}
+        self.assertFalse(main.validate_folder_data(data, "test_url"))
+        self.assertTrue(self.mock_log.error.called)


This test file uses unittest.TestCase while the existing test files in the codebase use pytest (see tests/test_security.py, tests/test_ssrf.py, etc.). For consistency with the established codebase convention, this test should be refactored to use pytest style with plain functions and assert statements instead of unittest.TestCase. Examples: tests/test_security.py:5-88, tests/test_ssrf.py

Copilot · 2026-01-27T07:33:03Z

tests/test_folder_validation.py

+    def test_valid_folder_name(self):
+        data = {"group": {"group": "Safe Folder"}}
+        self.assertTrue(main.validate_folder_data(data, "test_url"))
+
+    def test_xss_folder_name(self):
+        # Should be rejected
+        data = {"group": {"group": "<script>alert(1)</script>"}}
+        self.assertFalse(main.validate_folder_data(data, "test_url"))
+        # Check that error was logged
+        self.assertTrue(self.mock_log.error.called)
+        # We expect a specific error message about unsafe characters or invalid name
+        args = str(self.mock_log.error.call_args_list)
+        # Assuming we will log "Invalid folder name" or similar
+        self.assertTrue("Invalid folder name" in args or "Unsafe characters" in args)
+
+    def test_non_string_folder_name(self):
+        data = {"group": {"group": 123}}
+        self.assertFalse(main.validate_folder_data(data, "test_url"))
+        self.assertTrue(self.mock_log.error.called)
+        args = str(self.mock_log.error.call_args_list)
+        self.assertTrue("must be a string" in args)
+
+    def test_empty_folder_name(self):
+        data = {"group": {"group": "   "}}
+        self.assertFalse(main.validate_folder_data(data, "test_url"))
+        self.assertTrue(self.mock_log.error.called)


The test coverage is incomplete. While validate_folder_data is tested, the is_valid_folder_name function should also have direct unit tests to cover edge cases like folder names with allowed special characters (parentheses, brackets, braces) and various combinations of dangerous characters. This would provide better isolation and more comprehensive coverage of the validation logic. Similar functions like is_valid_rule (main.py:259) are tested directly in the codebase.

Copilot · 2026-01-27T07:33:03Z

tests/test_folder_validation.py

+        args = str(self.mock_log.error.call_args_list)
+        # Assuming we will log "Invalid folder name" or similar
+        self.assertTrue("Invalid folder name" in args or "Unsafe characters" in args)
+
+    def test_non_string_folder_name(self):
+        data = {"group": {"group": 123}}
+        self.assertFalse(main.validate_folder_data(data, "test_url"))
+        self.assertTrue(self.mock_log.error.called)
+        args = str(self.mock_log.error.call_args_list)
+        self.assertTrue("must be a string" in args)


The test assertion is brittle because it checks for error messages by converting call_args_list to a string and using substring matching. This approach is fragile and could break if the error message format changes. Consider checking the actual error messages directly using mock_log.error.call_args or mock_log.error.call_args_list[0][0][0], similar to how the existing security tests verify specific logged content (see tests/test_security.py:76-83 for an example pattern).

Suggested change

args = str(self.mock_log.error.call_args_list)

# Assuming we will log "Invalid folder name" or similar

self.assertTrue("Invalid folder name" in args or "Unsafe characters" in args)

def test_non_string_folder_name(self):

data = {"group": {"group": 123}}

self.assertFalse(main.validate_folder_data(data, "test_url"))

self.assertTrue(self.mock_log.error.called)

args = str(self.mock_log.error.call_args_list)

self.assertTrue("must be a string" in args)

error_calls = self.mock_log.error.call_args_list

self.assertGreaterEqual(len(error_calls), 1)

logged_message = error_calls[0][0][0]

self.assertIsInstance(logged_message, str)

# Assuming we will log "Invalid folder name" or similar

self.assertTrue(

"Invalid folder name" in logged_message

or "Unsafe characters" in logged_message

)

def test_non_string_folder_name(self):

data = {"group": {"group": 123}}

self.assertFalse(main.validate_folder_data(data, "test_url"))

self.assertTrue(self.mock_log.error.called)

error_calls = self.mock_log.error.call_args_list

self.assertGreaterEqual(len(error_calls), 1)

logged_message = error_calls[0][0][0]

self.assertIsInstance(logged_message, str)

self.assertIn("must be a string", logged_message)

Copilot · 2026-01-27T07:33:04Z

tests/test_folder_validation.py

+    def test_valid_folder_name(self):
+        data = {"group": {"group": "Safe Folder"}}
+        self.assertTrue(main.validate_folder_data(data, "test_url"))
+
+    def test_xss_folder_name(self):
+        # Should be rejected
+        data = {"group": {"group": "<script>alert(1)</script>"}}
+        self.assertFalse(main.validate_folder_data(data, "test_url"))
+        # Check that error was logged
+        self.assertTrue(self.mock_log.error.called)
+        # We expect a specific error message about unsafe characters or invalid name
+        args = str(self.mock_log.error.call_args_list)
+        # Assuming we will log "Invalid folder name" or similar
+        self.assertTrue("Invalid folder name" in args or "Unsafe characters" in args)
+
+    def test_non_string_folder_name(self):
+        data = {"group": {"group": 123}}
+        self.assertFalse(main.validate_folder_data(data, "test_url"))
+        self.assertTrue(self.mock_log.error.called)
+        args = str(self.mock_log.error.call_args_list)
+        self.assertTrue("must be a string" in args)
+
+    def test_empty_folder_name(self):
+        data = {"group": {"group": "   "}}
+        self.assertFalse(main.validate_folder_data(data, "test_url"))
+        self.assertTrue(self.mock_log.error.called)


The test coverage should include test cases for folder names with allowed special characters that are mentioned in the validation comments. For example, "Work (Private)" with parentheses, "Tasks [2024]" with brackets, or "Config {prod}" with braces. This would verify that the intentionally allowed characters actually work as expected and document the validation behavior.

Copilot · 2026-01-27T07:33:04Z

tests/test_folder_validation.py

+    def test_xss_folder_name(self):
+        # Should be rejected
+        data = {"group": {"group": "<script>alert(1)</script>"}}
+        self.assertFalse(main.validate_folder_data(data, "test_url"))
+        # Check that error was logged
+        self.assertTrue(self.mock_log.error.called)
+        # We expect a specific error message about unsafe characters or invalid name
+        args = str(self.mock_log.error.call_args_list)
+        # Assuming we will log "Invalid folder name" or similar
+        self.assertTrue("Invalid folder name" in args or "Unsafe characters" in args)


The XSS test coverage should be expanded to test all the dangerous characters individually. Currently only one XSS payload with script tags is tested. Add test cases for each dangerous character mentioned in the validation: angle brackets (<, >), double quotes ("), single quotes ('), and backticks (). Examples: "test<div>", "test\"quote", "test'quote", "testbacktick". This ensures comprehensive coverage of all blocked XSS-related characters.

Copilot · 2026-01-27T07:33:04Z

tests/test_folder_validation.py

+        self.assertFalse(main.validate_folder_data(data, "test_url"))
+        self.assertTrue(self.mock_log.error.called)
+        args = str(self.mock_log.error.call_args_list)
+        self.assertTrue("must be a string" in args)


assertTrue(a in b) cannot provide an informative message. Using assertIn(a, b) instead will give more informative messages.

Suggested change

self.assertTrue("must be a string" in args)

self.assertIn("must be a string", args)

Reverting all changes as this work has been superseded by PR #131. Co-authored-by: abhimehro <84992105+abhimehro@users.noreply.github.com>

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

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

View reviewed changes

abhimehro assigned Copilot and abhimehro Jan 27, 2026

abhimehro marked this pull request as ready for review January 27, 2026 07:28

Copilot AI review requested due to automatic review settings January 27, 2026 07:28

Copilot started reviewing on behalf of abhimehro January 27, 2026 07:28 View session

Copilot AI reviewed Jan 27, 2026

View reviewed changes

abhimehro merged commit d6923da into main Jan 27, 2026
24 checks passed

abhimehro deleted the sentinel-folder-validation-10716619568147792189 branch January 27, 2026 13:27

abhimehro mentioned this pull request Jan 27, 2026

🛡️ Sentinel: [HIGH] Fix XSS risk in folder names #128

Closed

abhimehro added a commit that referenced this pull request Jan 27, 2026

Revert changes - PR Superseded

5482988

Reverting all changes as this work has been superseded by PR #131. Co-authored-by: abhimehro <84992105+abhimehro@users.noreply.github.com>


		sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))

		import main

	self.assertTrue("must be a string" in args)
	self.assertIn("must be a string", args)

Conversation

google-labs-jules bot commented Jan 25, 2026

Uh oh!

google-labs-jules bot commented Jan 25, 2026

Uh oh!

trunk-io bot commented Jan 25, 2026

Uh oh!

Check notice

Check notice

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check notice

Check notice

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants