Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
abhimehro merged 1 commit into from
Jan 27, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .jules/sentinel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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-06-01 - [Incomplete Input Validation Scope]

Check notice

Code scanning / Remark-lint (reported by Codacy)

Warn when references to undefined definitions are found. Note

[no-undefined-references] Found reference to undefined definition

Check notice

Code scanning / Remark-lint (reported by Codacy)

Warn when shortcut reference links are used. Note

[no-shortcut-reference-link] Use the trailing [] on reference links
**Vulnerability:** While rule content was validated for XSS characters, the `folder_name` (parsed from the same JSON structure) was not. This created an inconsistency where one part of the untrusted input was sanitized, but another (which also appears in the UI) was left vulnerable to Stored XSS.
**Learning:** Validation MUST cover the entire schema of untrusted input. Focusing only on the "payload" (rules) while ignoring metadata (names, descriptions) leaves gaps.
**Prevention:**
1. Map out the full JSON schema of external inputs.
2. Apply validation functions to ALL string fields, not just the primary data arrays.
26 changes: 26 additions & 0 deletions main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,21 @@

return True

def is_valid_folder_name(name: str) -> bool:
"""
Validates folder name to prevent XSS and ensure printability.
Allowed: Anything printable except < > " ' `
"""
if not name or not name.isprintable():
return False

# Block XSS and HTML injection characters
dangerous_chars = set("<>\"'`")
if any(c in dangerous_chars for c in name):
Comment on lines +281 to +286
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validation in is_valid_folder_name checks the folder name before it's stripped, but the actual usage throughout the codebase (lines 721, 841, 872 in main.py) strips the folder name with .strip() before using it. This creates a validation bypass: a folder name consisting only of whitespace characters or one that becomes empty after stripping could pass validation but then cause issues or unexpected behavior when processed. The validation should either strip the name before checking, or explicitly reject names that would become empty after stripping. Consider modifying the function to: strip the name first, then check if it's empty.

Suggested change
if not name or not name.isprintable():
return False
# Block XSS and HTML injection characters
dangerous_chars = set("<>\"'`")
if any(c in dangerous_chars for c in name):
# Normalize the folder name the same way it is used elsewhere
stripped = name.strip()
# Reject names that are empty or only whitespace after stripping
if not stripped or not stripped.isprintable():
return False
# Block XSS and HTML injection characters
dangerous_chars = set("<>\"'`")
if any(c in dangerous_chars for c in stripped):

Copilot uses AI. Check for mistakes.
return False

return True

def validate_folder_data(data: Dict[str, Any], url: str) -> bool:
if not isinstance(data, dict):
log.error(f"Invalid data from {sanitize_for_log(url)}: Root must be a JSON object.")
Expand All @@ -286,6 +301,17 @@
if "group" not in data["group"]:
log.error(f"Invalid data from {sanitize_for_log(url)}: Missing 'group.group' (folder name).")
return False

# Security: Validate folder name
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.")

Check warning

Code scanning / Prospector (reported by Codacy)

Use lazy % formatting in logging functions (logging-fstring-interpolation) Warning

Use lazy % formatting in logging functions (logging-fstring-interpolation)

Check notice

Code scanning / Pylintpython3 (reported by Codacy)

Use lazy % formatting in logging functions Note

Use lazy % formatting in logging functions
return False

if not is_valid_folder_name(folder_name):
log.error(f"Invalid data from {sanitize_for_log(url)}: Unsafe folder name detected.")

Check warning

Code scanning / Prospector (reported by Codacy)

Use lazy % formatting in logging functions (logging-fstring-interpolation) Warning

Use lazy % formatting in logging functions (logging-fstring-interpolation)

Check notice

Code scanning / Pylintpython3 (reported by Codacy)

Use lazy % formatting in logging functions Note

Use lazy % formatting in logging functions
return False

return True

def _api_get(client: httpx.Client, url: str) -> httpx.Response:
Expand Down
32 changes: 32 additions & 0 deletions tests/test_folder_validation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import pytest

Check warning

Code scanning / Pylint (reported by Codacy)

Missing module docstring Warning test

Missing module docstring

Check warning

Code scanning / Prospector (reported by Codacy)

Unable to import 'pytest' (import-error) Warning test

Unable to import 'pytest' (import-error)

Check warning

Code scanning / Pylintpython3 (reported by Codacy)

Missing module docstring Warning test

Missing module docstring

Check warning

Code scanning / Prospector (reported by Codacy)

Unused import pytest (unused-import) Warning test

Unused import pytest (unused-import)

Check notice

Code scanning / Pylint (reported by Codacy)

Unused import pytest Note test

Unused import pytest

Check notice

Code scanning / Pylintpython3 (reported by Codacy)

Unused import pytest Note test

Unused import pytest
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import of 'pytest' is not used.

Suggested change
import pytest

Copilot uses AI. Check for mistakes.
from unittest.mock import MagicMock

Check warning

Code scanning / Pylint (reported by Codacy)

standard import "from unittest.mock import MagicMock" should be placed before "import pytest" Warning test

standard import "from unittest.mock import MagicMock" should be placed before "import pytest"

Check warning

Code scanning / Pylintpython3 (reported by Codacy)

standard import "from unittest.mock import MagicMock" should be placed before "import pytest" Warning test

standard import "from unittest.mock import MagicMock" should be placed before "import pytest"
import main

def test_folder_name_security():
"""
Verify that validate_folder_data enforces security checks on folder names.
"""
# Mock logger to verify errors
mock_log = MagicMock()
original_log = main.log
main.log = mock_log

try:
# Case 1: Valid Folder Name
valid_data = {"group": {"group": "Safe Folder Name (Work)"}}
assert main.validate_folder_data(valid_data, "http://valid.com") is True

Check notice

Code scanning / Bandit

Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test

Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.

Check notice

Code scanning / Bandit (reported by Codacy)

Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test

Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.

# Case 2: XSS Payload
xss_data = {"group": {"group": "<script>alert(1)</script>"}}
assert main.validate_folder_data(xss_data, "http://evil.com") is False

Check notice

Code scanning / Bandit

Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test

Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.

Check notice

Code scanning / Bandit (reported by Codacy)

Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test

Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.

# Case 3: Invalid Type
invalid_type_data = {"group": {"group": 123}}
assert main.validate_folder_data(invalid_type_data, "http://badtype.com") is False

Check notice

Code scanning / Bandit

Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test

Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.

Check notice

Code scanning / Bandit (reported by Codacy)

Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test

Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.

# Case 4: Dangerous characters
dangerous_data = {"group": {"group": "Folder\"Name"}}
assert main.validate_folder_data(dangerous_data, "http://dangerous.com") is False

Check notice

Code scanning / Bandit

Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test

Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.

Check notice

Code scanning / Bandit (reported by Codacy)

Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test

Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.
Comment on lines +19 to +29
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test should verify additional XSS vectors and edge cases beyond the basic script tag. Consider adding tests for: HTML event handlers without script tags (e.g., "onload=alert(1)"), HTML entities (e.g., "<script>"), mixed case attempts (though these may not bypass the character check), and Unicode normalization attacks if applicable. Also consider testing the backtick character explicitly since it's in the dangerous_chars set but not tested.

Copilot uses AI. Check for mistakes.

finally:
main.log = original_log
Comment on lines +5 to +32
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test cases should include edge cases for whitespace handling since the actual code usage strips folder names with .strip(). Consider adding test cases for: folder names with leading/trailing whitespace (e.g., " Valid Name "), folder names that are only whitespace (e.g., " "), and folder names that become empty after stripping. These edge cases are important to verify that the validation properly handles the stripping behavior seen in the main code.

Copilot uses AI. Check for mistakes.
Loading