diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 038829e..483ebee 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -61,3 +61,11 @@ **Learning:** Logic errors in security controls often lead to "fail-closed" states that break functionality entirely, or "fail-open" states that bypass security. Implicit returns in Python (`None`) can be dangerous when boolean validation is expected. **Prevention:** Always use explicit return statements for both success and failure paths in validation functions. Use static analysis (linting) to catch unreachable code and implicit returns. Ensure unit tests cover positive cases (valid inputs) as rigorously as negative cases (attack vectors). + +## 2026-10-24 - Malformed Code Disabling Input Validation + +**Vulnerability:** A critical syntax error and malformed code structure in `validate_folder_data` disabled input validation for nested rules. The code block intended to validate that rules are dictionaries was corrupt (syntax error) and placed outside the logic loop, preventing the application from running (DoS) and potentially allowing invalid data structures if the syntax error were bypassed or ignored. + +**Learning:** Broken code or copy-paste errors can silently disable security controls or cause application crashes. Visual inspection is not enough; static analysis (linting) and comprehensive testing are essential. + +**Prevention:** Ensure all code paths are covered by tests. Use linters to catch syntax errors and unreachable code. Verify that input validation logic is correctly scoped within loops and conditional blocks. diff --git a/main.py b/main.py index 7e75861..a67d9d7 100644 --- a/main.py +++ b/main.py @@ -1131,19 +1131,22 @@ def validate_folder_data(data: Dict[str, Any], url: str) -> bool: ) return False if "rules" in rg: - if not isinstance (rg["rules"], list): - log. error ( - f"Invalid data from {sanitize_for_log(url)} : rule_groups[fil].rules must be a list." + 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 (rgi"rules"1): -if not isinstance (rule, dict): - log. error ( - f"Invalid data from {sanitize_for_log(u rl)}: rule_groups[fiłl.rules[kił] must be an object." - ) - 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 + + return True + # Lock to protect updates to _api_stats in multi-threaded contexts. # Without this, concurrent increments can lose updates because `+=` is not atomic. diff --git a/tests/test_fix_broken_validation.py b/tests/test_fix_broken_validation.py new file mode 100644 index 0000000..7f47236 --- /dev/null +++ b/tests/test_fix_broken_validation.py @@ -0,0 +1,78 @@ +import unittest +from unittest.mock import MagicMock +import sys +import os + +# Add root to path to import main +sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +import main + +class TestFixBrokenValidation(unittest.TestCase): + def setUp(self): + self.original_log = main.log + main.log = MagicMock() + + def tearDown(self): + main.log = self.original_log + + def test_invalid_rule_type_in_rule_groups(self): + """ + Verify that validate_folder_data correctly identifies and rejects + non-dict rules inside rule_groups. + This tests the fix for the broken syntax block. + """ + # Data with invalid rule (string instead of dict) inside rule_groups + invalid_data = { + "group": {"group": "Test Group"}, + "rule_groups": [ + { + "rules": [ + {"PK": "valid.com"}, + "invalid_string_rule" # Should trigger the error + ] + } + ] + } + + result = main.validate_folder_data(invalid_data, "http://test.com") + + self.assertFalse(result, "Should return False for invalid rule type") + + # Verify the error log message + # We expect: "Invalid data from http://test.com: rule_groups[0].rules[1] must be an object." + main.log.error.assert_called() + found = False + for call in main.log.error.call_args_list: + if "rule_groups[0].rules[1] must be an object" in call[0][0]: + found = True + break + self.assertTrue(found, "Did not find expected error message for invalid rule type") + + def test_invalid_rules_list_type(self): + """ + Verify that if 'rules' is not a list, it is caught. + This tests the fix for the malformed logging block above the loop. + """ + invalid_data = { + "group": {"group": "Test Group"}, + "rule_groups": [ + { + "rules": "not_a_list" # Should trigger error + } + ] + } + + result = main.validate_folder_data(invalid_data, "http://test.com") + self.assertFalse(result) + + main.log.error.assert_called() + found = False + for call in main.log.error.call_args_list: + if "rule_groups[0].rules must be a list" in call[0][0]: + found = True + break + self.assertTrue(found, "Did not find expected error message for invalid rules list type") + +if __name__ == '__main__': + unittest.main()