From e5548acb984ca4f8b2ae8381f879a0285094257f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B0=A2=E6=A0=8B=E6=A2=81?= Date: Mon, 15 Dec 2025 20:24:44 +0800 Subject: [PATCH] fix: allow home subdirectories through is_dangerous_path() Split DANGEROUS_PATHS into two categories: 1. DANGEROUS_SYSTEM_PATHS: Block path AND all subdirectories (e.g., /etc, /etc/passwd, /var/log/auth.log) 2. DANGEROUS_HOME_CONTAINERS: Block ONLY exact match (e.g., /home is blocked but /home/user/project passes through) This fixes the issue where /home/user/project was incorrectly blocked by is_dangerous_path(). Subdirectory access control for home directories is properly delegated to is_home_directory_root() in resolve_and_validate_path(). Addresses review feedback from @chatgpt-codex-connector about blocking all home directory subpaths. --- tests/test_path_traversal_security.py | 41 ++++++++++++++++++++++-- utils/security_config.py | 46 ++++++++++++++++++++------- 2 files changed, 72 insertions(+), 15 deletions(-) diff --git a/tests/test_path_traversal_security.py b/tests/test_path_traversal_security.py index f281c58..57509dd 100644 --- a/tests/test_path_traversal_security.py +++ b/tests/test_path_traversal_security.py @@ -7,6 +7,11 @@ Fixes vulnerability reported in: The vulnerability: is_dangerous_path() only did exact string matching, so /etc was blocked but /etc/passwd was allowed. + +Additionally, this fix properly handles home directory containers: +- /home and C:\\Users are blocked (exact match only) +- /home/user/project paths are allowed through is_dangerous_path() + and handled by is_home_directory_root() in resolve_and_validate_path() """ from pathlib import Path @@ -15,7 +20,7 @@ from utils.security_config import is_dangerous_path class TestPathTraversalFix: - """Test that subdirectories of dangerous paths are now blocked.""" + """Test that subdirectories of dangerous system paths are blocked.""" def test_exact_match_still_works(self): """Test that exact dangerous paths are still blocked.""" @@ -24,7 +29,7 @@ class TestPathTraversalFix: assert is_dangerous_path(Path("/var")) is True def test_subdirectory_now_blocked(self): - """Test that subdirectories are now blocked (the fix).""" + """Test that subdirectories of system paths are blocked (the fix).""" # These were allowed before the fix assert is_dangerous_path(Path("/etc/passwd")) is True assert is_dangerous_path(Path("/etc/shadow")) is True @@ -32,7 +37,7 @@ class TestPathTraversalFix: assert is_dangerous_path(Path("/var/log/auth.log")) is True def test_deeply_nested_blocked(self): - """Test that deeply nested paths are blocked.""" + """Test that deeply nested system paths are blocked.""" assert is_dangerous_path(Path("/etc/ssh/sshd_config")) is True assert is_dangerous_path(Path("/usr/local/bin/python")) is True @@ -53,6 +58,36 @@ class TestPathTraversalFix: assert is_dangerous_path(Path("/tmp/my_etc_files")) is False +class TestHomeDirectoryHandling: + """Test that home directory containers are handled correctly. + + Home containers (/home, C:\\Users) should only block the exact path, + not subdirectories. Subdirectory access control is delegated to + is_home_directory_root() in resolve_and_validate_path(). + """ + + def test_home_container_blocked(self): + """Test that /home itself is blocked.""" + assert is_dangerous_path(Path("/home")) is True + + def test_home_subdirectories_allowed(self): + """Test that /home subdirectories pass through is_dangerous_path(). + + These paths should NOT be blocked by is_dangerous_path() because: + 1. /home/user/project is a valid user workspace + 2. Access control for /home/username is handled by is_home_directory_root() + """ + # User home directories should pass is_dangerous_path() + # (they are handled by is_home_directory_root() separately) + assert is_dangerous_path(Path("/home/user")) is False + assert is_dangerous_path(Path("/home/user/project")) is False + assert is_dangerous_path(Path("/home/user/project/src/main.py")) is False + + def test_home_deeply_nested_allowed(self): + """Test that deeply nested home paths are allowed.""" + assert is_dangerous_path(Path("/home/user/documents/work/project/src")) is False + + class TestRegressionPrevention: """Regression tests for the specific vulnerability.""" diff --git a/utils/security_config.py b/utils/security_config.py index 34140b1..4f42b49 100644 --- a/utils/security_config.py +++ b/utils/security_config.py @@ -7,22 +7,31 @@ for file access control. from pathlib import Path -# Dangerous paths that should never be scanned -# These would give overly broad access and pose security risks -DANGEROUS_PATHS = { +# Dangerous system paths - block these AND all their subdirectories +# These are system directories where user code should never reside +DANGEROUS_SYSTEM_PATHS = { "/", "/etc", "/usr", "/bin", "/var", "/root", - "/home", "C:\\", "C:\\Windows", "C:\\Program Files", +} + +# User home container paths - block ONLY the exact path, not subdirectories +# Subdirectory access (e.g., /home/user/project) is controlled by is_home_directory_root() +# This allows users to work in their home subdirectories while blocking overly broad access +DANGEROUS_HOME_CONTAINERS = { + "/home", "C:\\Users", } +# Combined set for backward compatibility +DANGEROUS_PATHS = DANGEROUS_SYSTEM_PATHS | DANGEROUS_HOME_CONTAINERS + # Directories to exclude from recursive file search # These typically contain generated code, dependencies, or build artifacts EXCLUDED_DIRS = { @@ -91,9 +100,14 @@ def is_dangerous_path(path: Path) -> bool: """ Check if a path is in or under a dangerous directory. - Uses Path.is_relative_to() to block dangerous directories AND their subdirectories. - For example, if "/etc" is in DANGEROUS_PATHS, both "/etc" and "/etc/passwd" - will be blocked. + This function handles two categories of dangerous paths differently: + + 1. System paths (DANGEROUS_SYSTEM_PATHS): Block the path AND all subdirectories. + Example: /etc is dangerous, so /etc/passwd is also blocked. + + 2. Home containers (DANGEROUS_HOME_CONTAINERS): Block ONLY the exact path. + Example: /home is blocked, but /home/user/project is allowed. + Subdirectory access control is delegated to is_home_directory_root(). Args: path: Path to check @@ -102,7 +116,8 @@ def is_dangerous_path(path: Path) -> bool: True if the path is dangerous and should not be accessed Security: - Fixes path traversal vulnerability (CWE-22) + Fixes path traversal vulnerability (CWE-22) while preserving + user access to home subdirectories. """ try: resolved = path.resolve() @@ -111,19 +126,26 @@ def is_dangerous_path(path: Path) -> bool: if resolved.parent == resolved: return True - # Check 2: Exact match or subdirectory of dangerous paths - # Use Path.is_relative_to() for correct cross-platform path comparison - for dangerous in DANGEROUS_PATHS: + # Check 2: System paths - block exact match AND all subdirectories + for dangerous in DANGEROUS_SYSTEM_PATHS: # Skip root "/" - already handled above if dangerous == "/": continue dangerous_path = Path(dangerous) # is_relative_to() correctly handles both exact matches and subdirectories - # Works properly on Windows with paths like "C:\" and "C:\Users" + # Works properly on Windows with paths like "C:\" and "C:\Windows" if resolved == dangerous_path or resolved.is_relative_to(dangerous_path): return True + # Check 3: Home containers - block ONLY exact match + # Subdirectories like /home/user/project should pass through here + # and be handled by is_home_directory_root() in resolve_and_validate_path() + for container in DANGEROUS_HOME_CONTAINERS: + container_path = Path(container) + if resolved == container_path: + return True + return False except Exception: