fix(security): handle macOS symlinked system dirs
Follow-up on PR #353 to keep dangerous-path blocking correct on macOS (/etc -> /private/etc) while avoiding overblocking Windows workspaces (C:\).
This commit is contained in:
@@ -6,10 +6,18 @@ import asyncio
|
|||||||
import importlib
|
import importlib
|
||||||
import os
|
import os
|
||||||
import sys
|
import sys
|
||||||
|
import tempfile
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
|
# On macOS, the default pytest temp dir is typically under /var (e.g. /private/var/folders/...).
|
||||||
|
# If /var is considered a dangerous system path, tests must use a safe temp root (like /tmp).
|
||||||
|
if sys.platform == "darwin":
|
||||||
|
os.environ["TMPDIR"] = "/tmp"
|
||||||
|
# tempfile caches the temp dir after first lookup; clear it so pytest fixtures pick up TMPDIR.
|
||||||
|
tempfile.tempdir = None
|
||||||
|
|
||||||
# Ensure the parent directory is in the Python path for imports
|
# Ensure the parent directory is in the Python path for imports
|
||||||
parent_dir = Path(__file__).resolve().parent.parent
|
parent_dir = Path(__file__).resolve().parent.parent
|
||||||
if str(parent_dir) not in sys.path:
|
if str(parent_dir) not in sys.path:
|
||||||
|
|||||||
@@ -16,7 +16,6 @@ DANGEROUS_SYSTEM_PATHS = {
|
|||||||
"/bin",
|
"/bin",
|
||||||
"/var",
|
"/var",
|
||||||
"/root",
|
"/root",
|
||||||
"C:\\",
|
|
||||||
"C:\\Windows",
|
"C:\\Windows",
|
||||||
"C:\\Program Files",
|
"C:\\Program Files",
|
||||||
}
|
}
|
||||||
@@ -122,6 +121,17 @@ def is_dangerous_path(path: Path) -> bool:
|
|||||||
try:
|
try:
|
||||||
resolved = path.resolve()
|
resolved = path.resolve()
|
||||||
|
|
||||||
|
def _dangerous_variants(p: Path) -> set[Path]:
|
||||||
|
variants = {p}
|
||||||
|
# Only resolve paths that are absolute on the current platform.
|
||||||
|
# This avoids turning Windows-style strings into nonsense absolute paths on POSIX.
|
||||||
|
if p.is_absolute():
|
||||||
|
try:
|
||||||
|
variants.add(p.resolve())
|
||||||
|
except Exception:
|
||||||
|
pass
|
||||||
|
return variants
|
||||||
|
|
||||||
# Check 1: Root directory (filesystem root)
|
# Check 1: Root directory (filesystem root)
|
||||||
if resolved.parent == resolved:
|
if resolved.parent == resolved:
|
||||||
return True
|
return True
|
||||||
@@ -132,9 +142,10 @@ def is_dangerous_path(path: Path) -> bool:
|
|||||||
if dangerous == "/":
|
if dangerous == "/":
|
||||||
continue
|
continue
|
||||||
|
|
||||||
dangerous_path = Path(dangerous)
|
for dangerous_path in _dangerous_variants(Path(dangerous)):
|
||||||
# is_relative_to() correctly handles both exact matches and subdirectories
|
# is_relative_to() correctly handles both exact matches and subdirectories.
|
||||||
# Works properly on Windows with paths like "C:\" and "C:\Windows"
|
# Resolving the dangerous base path also handles platform symlinks
|
||||||
|
# (e.g., macOS /etc -> /private/etc, /var -> /private/var).
|
||||||
if resolved == dangerous_path or resolved.is_relative_to(dangerous_path):
|
if resolved == dangerous_path or resolved.is_relative_to(dangerous_path):
|
||||||
return True
|
return True
|
||||||
|
|
||||||
@@ -142,7 +153,7 @@ def is_dangerous_path(path: Path) -> bool:
|
|||||||
# Subdirectories like /home/user/project should pass through here
|
# Subdirectories like /home/user/project should pass through here
|
||||||
# and be handled by is_home_directory_root() in resolve_and_validate_path()
|
# and be handled by is_home_directory_root() in resolve_and_validate_path()
|
||||||
for container in DANGEROUS_HOME_CONTAINERS:
|
for container in DANGEROUS_HOME_CONTAINERS:
|
||||||
container_path = Path(container)
|
for container_path in _dangerous_variants(Path(container)):
|
||||||
if resolved == container_path:
|
if resolved == container_path:
|
||||||
return True
|
return True
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user