Skip to content

🛡️ Sentinel: [MEDIUM] Fix missing input validation on external JSON data #409

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
Feb 22, 2026
Merged

🛡️ Sentinel: [MEDIUM] Fix missing input validation on external JSON data #409

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
24 changes: 24 additions & 0 deletions main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -955,7 +955,7 @@
try:
parsed = urlparse(url)
hostname = parsed.hostname
if not hostname:

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)
return False

return validate_hostname(hostname)
Expand Down Expand Up @@ -1089,7 +1089,7 @@
log.error(f"Invalid data from {sanitize_for_log(url)}: Missing 'group' key.")
return False
if not isinstance(data["group"], dict):
log.error(

Check notice

Code scanning / Pylintpython3 (reported by Codacy)

Use lazy % formatting in logging functions Note

Use lazy % formatting in logging functions
f"Invalid data from {sanitize_for_log(url)}: 'group' must be an object."
)
return False
Expand All @@ -1112,6 +1112,30 @@
)
return False

# Validate 'rules' if present (must be a list)
if "rules" in data and not isinstance(data["rules"], list):
log.error(f"Invalid data from {sanitize_for_log(url)}: 'rules' must be a list.")

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
Comment on lines +1115 to +1118
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The validation checks if 'rules' is a list, but doesn't validate that each item in the list is a dictionary. The code at lines 2138, 2152, and 2258 uses r.get("PK") which will throw an AttributeError if r is not a dict (e.g., if it's a string, int, or None). This leaves the DoS vulnerability partially unfixed.

Add validation to ensure each rule item is a dict:

if "rules" in data:
    if not isinstance(data["rules"], list):
        log.error(f"Invalid data from {sanitize_for_log(url)}: 'rules' must be a list.")
        return False
    for i, rule in enumerate(data["rules"]):
        if not isinstance(rule, dict):
            log.error(
                f"Invalid data from {sanitize_for_log(url)}: rules[{i}] must be an object."
            )
            return False

Copilot uses AI. Check for mistakes.
Comment on lines +1115 to +1118
Copy link

@gemini-code-assist gemini-code-assist bot Feb 20, 2026

Choose a reason for hiding this comment

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

medium

This validation correctly checks if 'rules' is present and, if so, ensures it's a list. This is a good step towards preventing TypeError if a non-list value is provided, as described in the PR description.


# Validate 'rule_groups' if present (must be a list of dicts)
if "rule_groups" in data:
if not isinstance(data["rule_groups"], list):
log.error(
f"Invalid data from {sanitize_for_log(url)}: 'rule_groups' must be a list."
)
return False
for i, rg in enumerate(data["rule_groups"]):

Check warning

Code scanning / Pylint (reported by Codacy)

Variable name "rg" doesn't conform to snake_case naming style Warning

Variable name "rg" doesn't conform to snake_case naming style

Check warning

Code scanning / Pylintpython3 (reported by Codacy)

Variable name "rg" doesn't conform to snake_case naming style Warning

Variable name "rg" doesn't conform to snake_case naming style
if not isinstance(rg, dict):
log.error(
f"Invalid data from {sanitize_for_log(url)}: rule_groups[{i}] must be an object."

Check warning

Code scanning / Pylint (reported by Codacy)

Line too long (101/100) Warning

Line too long (101/100)

Check warning

Code scanning / Pylintpython3 (reported by Codacy)

Line too long (101/100) Warning

Line too long (101/100)
)
return False
if "rules" in rg and not isinstance(rg["rules"], list):
log.error(
f"Invalid data from {sanitize_for_log(url)}: rule_groups[{i}].rules must be a list."

Check warning

Code scanning / Pylint (reported by Codacy)

Line too long (108/100) Warning

Line too long (108/100)

Check warning

Code scanning / Pylintpython3 (reported by Codacy)

Line too long (108/100) Warning

Line too long (108/100)
)
return False
Comment on lines +1133 to +1137
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

Similar to the 'rules' validation above, this code validates that rules within rule_groups is a list, but doesn't validate that each item in that list is a dictionary. The code at line 2138 uses r.get("PK") which will throw an AttributeError if any rule item is not a dict.

Add validation to ensure each rule item within rule_groups is a dict:

if "rules" in rg:
    if not isinstance(rg["rules"], list):
        log.error(
            f"Invalid data from {sanitize_for_log(url)}: rule_groups[{i}].rules must be a list."
        )
        return False
    for j, rule in enumerate(rg["rules"]):
        if not isinstance(rule, dict):
            log.error(
                f"Invalid data from {sanitize_for_log(url)}: rule_groups[{i}].rules[{j}] must be an object."
            )
            return False
Suggested change
if "rules" in rg and not isinstance(rg["rules"], list):
log.error(
f"Invalid data from {sanitize_for_log(url)}: rule_groups[{i}].rules must be a list."
)
return False
if "rules" in rg:
if not isinstance(rg["rules"], list):
log.error(
f"Invalid data from {sanitize_for_log(url)}: rule_groups[{i}].rules must be a list."
)
return False
# Ensure each rule within the group is an object (dict),
# because later code treats each rule as a mapping (e.g., rule.get(...)).
for j, rule in enumerate(rg["rules"]):
if not isinstance(rule, dict):
log.error(
f"Invalid data from {sanitize_for_log(url)}: rule_groups[{i}].rules[{j}] must be an object."
)
return False

Copilot uses AI. Check for mistakes.
Comment on lines +1120 to +1137
Copy link

@gemini-code-assist gemini-code-assist bot Feb 20, 2026

Choose a reason for hiding this comment

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

medium

The validation for 'rule_groups' is thorough, checking both the top-level type (must be a list) and iterating through its elements to ensure each is a dictionary. Furthermore, it correctly validates the 'rules' key within each rule group. This nested validation is critical for preventing crashes when processing complex, malformed JSON structures.


return True


Expand Down Expand Up @@ -1788,7 +1812,7 @@
# Restore progress bar after warning
render_progress_bar(completed, total, "Warming up cache", prefix="⏳")

if USE_COLORS:

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
sys.stderr.write(
f"\r\033[K{Colors.GREEN}✅ Warming up cache: Done!{Colors.ENDC}\n"
)
Expand Down Expand Up @@ -1882,7 +1906,7 @@
if not validate_folder_id(pk, log_errors=False):
log.error(f"API returned invalid folder ID: {sanitize_for_log(pk)}")
return None
log.info(

Check notice

Code scanning / Pylintpython3 (reported by Codacy)

Use lazy % formatting in logging functions Note

Use lazy % formatting in logging functions
"Created folder %s (ID %s) [Polled]",
sanitize_for_log(name),
sanitize_for_log(pk),
Expand Down
50 changes: 50 additions & 0 deletions test_main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@
mock_args.dry_run = False
mock_args.no_delete = False
mock_args.plan_json = None
mock_args.clear_cache = False
Copy link

@gemini-code-assist gemini-code-assist bot Feb 20, 2026

Choose a reason for hiding this comment

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

medium

Adding mock_args.clear_cache = False ensures that the test environment for test_interactive_prompts_show_hints accurately reflects the default state of clear_cache when not explicitly set by the user. This prevents unintended side effects or test failures if the default behavior of clear_cache were to change or if the test were run in an environment where clear_cache might be implicitly true.

monkeypatch.setattr(m, "parse_args", lambda: mock_args)

# Mock internal functions to abort execution safely after prompts
Expand Down Expand Up @@ -434,6 +435,7 @@
mock_args.dry_run = False
mock_args.no_delete = False
mock_args.plan_json = None
mock_args.clear_cache = False
Copy link

@gemini-code-assist gemini-code-assist bot Feb 20, 2026

Choose a reason for hiding this comment

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

medium

Similar to the previous comment, explicitly setting mock_args.clear_cache = False in test_interactive_input_extracts_id ensures test isolation and predictability. This is good practice for robust unit testing.

monkeypatch.setattr(m, "parse_args", lambda: mock_args)

# Mock sync_profile to catch the call
Expand Down Expand Up @@ -495,7 +497,7 @@
assert "Error message" in captured.out


# Case 12: get_password works with getpass

Check warning

Code scanning / Pylintpython3 (reported by Codacy)

Variable name "m" doesn't conform to snake_case naming style Warning test

Variable name "m" doesn't conform to snake_case naming style
def test_get_password(monkeypatch):
m = reload_main_with_env(monkeypatch)

Expand Down Expand Up @@ -661,7 +663,7 @@
assert "█" * 24 in combined or len([c for c in combined if c == "█"]) == 24


# Case 18: check_env_permissions uses secure file operations

Check warning

Code scanning / Pylint (reported by Codacy)

Variable name "m" doesn't conform to snake_case naming style Warning test

Variable name "m" doesn't conform to snake_case naming style
def test_check_env_permissions_secure(monkeypatch):
m = reload_main_with_env(monkeypatch)

Expand Down Expand Up @@ -723,3 +725,51 @@
assert mock_open.called
assert not mock_fchmod.called
mock_close.assert_called_with(123)


def test_validate_folder_data_structure(monkeypatch):
"""Test validation of 'rules' and 'rule_groups' structures."""
m = reload_main_with_env(monkeypatch)
mock_log = MagicMock()
monkeypatch.setattr(m, "log", mock_log)

valid_base = {
"group": {"group": "ValidFolder"}
}

# 1. Invalid 'rules' type (string instead of list)
invalid_rules = valid_base.copy()
invalid_rules["rules"] = "not_a_list"
assert m.validate_folder_data(invalid_rules, "url") 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.
assert "rules" in str(mock_log.error.call_args)

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.
mock_log.reset_mock()

# 2. Invalid 'rule_groups' type (string instead of list)
invalid_rg_type = valid_base.copy()
invalid_rg_type["rule_groups"] = "not_a_list"
assert m.validate_folder_data(invalid_rg_type, "url") 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.
assert "rule_groups" in str(mock_log.error.call_args)

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.
mock_log.reset_mock()

# 3. Invalid 'rule_groups' content (list of strings instead of dicts)
invalid_rg_content = valid_base.copy()
invalid_rg_content["rule_groups"] = ["not_a_dict"]
assert m.validate_folder_data(invalid_rg_content, "url") 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.
assert "must be an object" in str(mock_log.error.call_args)

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.
mock_log.reset_mock()

# 4. Invalid 'rules' inside 'rule_groups'
invalid_rg_rules = valid_base.copy()
invalid_rg_rules["rule_groups"] = [{"rules": "not_a_list"}]
assert m.validate_folder_data(invalid_rg_rules, "url") 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.
assert "must be a list" in str(mock_log.error.call_args)

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.
mock_log.reset_mock()

# 5. Valid cases
valid_rules = valid_base.copy()
valid_rules["rules"] = [{"PK": "rule1"}]
assert m.validate_folder_data(valid_rules, "url") 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.

valid_rg = valid_base.copy()
valid_rg["rule_groups"] = [{"rules": [{"PK": "rule1"}]}]
assert m.validate_folder_data(valid_rg, "url") 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.
Comment on lines +730 to +775
Copy link

@gemini-code-assist gemini-code-assist bot Feb 20, 2026

Choose a reason for hiding this comment

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

medium

This new test function test_validate_folder_data_structure is excellent. It covers various invalid scenarios for rules and rule_groups, including incorrect types and malformed nested structures. This directly verifies the effectiveness of the input validation added in main.py and ensures the application's resilience against malformed external JSON data. The use of mock_log to assert error messages is also a good approach.

Loading