#!/usr/bin/env python3 """ Simple pre-commit hook to check for direct os.environ usage. This is a lightweight check - comprehensive validation happens in CI. """ import ast import sys from pathlib import Path # Allowlist entries, partitioned by matching strategy. # # Previously this was a single ALLOWED_PATTERNS set checked with # `pattern in self.filename` — a bare substring match. That incorrectly # exempted production files whose path happened to contain an allowlist # substring: e.g. "test_" matched protest_handler.py, "settings/" matched # foo_settings_override.py, and so on. Partition the list instead so each # entry matches at the right granularity. # Directory subtrees where env-var access is allowed (settings / tests / # scripts / examples / migrations / the hooks themselves). ALLOWED_PATH_SEGMENTS = { "settings", "config", "tests", "scripts", "examples", "migrations", ".pre-commit-hooks", } # Filename prefixes for test files. ALLOWED_NAME_PREFIXES = ("test_",) # Filename suffixes for Go-style *_test.py convention. ALLOWED_NAME_SUFFIXES = ("_test.py",) # Exact basenames for bootstrap / infrastructure modules that run before # SettingsManager is initialized. ALLOWED_NAMES = { "log_utils.py", # Logger init before DB/SettingsManager "server_config.py", # Fail-closed security validation for LDR_APP_ALLOW_REGISTRATIONS "sqlcipher_utils.py", # Encryption init needs LDR_TEST_MODE before SettingsManager } # Path-anchored entries where the basename alone is too generic to match # reliably. # Leading "/" anchors the path segment so endswith() cannot match lookalike # paths (e.g. "notsecurity/secure_logging.py"). ALLOWED_PATH_ENDINGS = ( "/security/rate_limiter.py", # Module-level RATE_LIMIT_FAIL_CLOSED at decorator time "/security/secure_logging.py", # Diagnose-mode gate must work before/without SettingsManager (same rationale as log_utils.py) ) # System environment variables that are always allowed SYSTEM_VARS = { "PATH", "HOME", "USER", "PYTHONPATH", "TMPDIR", "TEMP", "TZ", # Standard POSIX timezone variable "CI", "GITHUB_ACTIONS", "TESTING", # External testing flag "PYTEST_CURRENT_TEST", # Pytest test detection "WERKZEUG_RUN_MAIN", # Flask/Werkzeug debug reloader detection } class EnvVarChecker(ast.NodeVisitor): def __init__(self, filename: str): self.filename = filename self.errors = [] def visit_ImportFrom(self, node): """Check for 'from os import environ/getenv' direct imports.""" if not self._is_file_allowed() and node.module == "os" and node.names: for alias in node.names: if alias.name in ("environ", "getenv"): self.errors.append( ( node.lineno, f"Direct import 'from os import {alias.name}' — use SettingsManager instead of direct env var access", ) ) self.generic_visit(node) def visit_Assign(self, node): """Check for aliasing: env = os.environ.""" if not self._is_file_allowed(): # Check the right-hand side for os.environ attribute access if ( isinstance(node.value, ast.Attribute) and node.value.attr == "environ" and isinstance(node.value.value, ast.Name) and node.value.value.id == "os" ): self.errors.append( ( node.lineno, "Aliasing os.environ to a local variable — use SettingsManager instead", ) ) self.generic_visit(node) def visit_Call(self, node): # Check for os.environ.get() or os.getenv() is_environ_get = False env_var_name = None # Pattern 1: os.environ.get("VAR_NAME") or os.environ.get(variable) if ( isinstance(node.func, ast.Attribute) and node.func.attr == "get" and isinstance(node.func.value, ast.Attribute) and node.func.value.attr == "environ" and isinstance(node.func.value.value, ast.Name) and node.func.value.value.id == "os" ): is_environ_get = True if node.args and isinstance(node.args[0], ast.Constant): env_var_name = node.args[0].value # Pattern 2: os.getenv("VAR_NAME") or os.getenv(variable) elif ( isinstance(node.func, ast.Attribute) and node.func.attr == "getenv" and isinstance(node.func.value, ast.Name) and node.func.value.id == "os" ): is_environ_get = True if node.args and isinstance(node.args[0], ast.Constant): env_var_name = node.args[0].value # Pattern 3: os.environ.pop() / .setdefault() / .update() if ( isinstance(node.func, ast.Attribute) and node.func.attr in ("pop", "setdefault", "update") and isinstance(node.func.value, ast.Attribute) and node.func.value.attr == "environ" and isinstance(node.func.value.value, ast.Name) and node.func.value.value.id == "os" ): if not self._is_file_allowed(): self.errors.append( ( node.lineno, f"Direct os.environ.{node.func.attr}() call — use SettingsManager instead", ) ) if is_environ_get: if env_var_name: # Known constant key — allow system vars if env_var_name in SYSTEM_VARS: return self.generic_visit(node) if not self._is_file_allowed(): if env_var_name.startswith("LDR_"): self.errors.append( ( node.lineno, f"Environment variable '{env_var_name}' should be accessed through SettingsManager, not os.environ", ) ) else: self.errors.append( ( node.lineno, f"Direct access to environment variable '{env_var_name}' - consider using SettingsManager", ) ) else: # Dynamic key (variable, not a string literal) if not self._is_file_allowed(): self.errors.append( ( node.lineno, "Dynamic environment variable access (variable key) — use SettingsManager instead", ) ) self.generic_visit(node) def visit_Subscript(self, node): # Check for os.environ["VAR_NAME"] pattern if ( isinstance(node.value, ast.Attribute) and node.value.attr == "environ" and isinstance(node.value.value, ast.Name) and node.value.value.id == "os" ): if isinstance(node.slice, ast.Constant): env_var_name = node.slice.value # Allow system vars if env_var_name in SYSTEM_VARS: return self.generic_visit(node) if not self._is_file_allowed(): if env_var_name.startswith("LDR_"): self.errors.append( ( node.lineno, f"Environment variable '{env_var_name}' should be accessed through SettingsManager, not os.environ", ) ) else: self.errors.append( ( node.lineno, f"Direct access to environment variable '{env_var_name}' - consider using SettingsManager", ) ) else: # Dynamic subscript: os.environ[variable] if not self._is_file_allowed(): self.errors.append( ( node.lineno, "Dynamic os.environ[variable] access — use SettingsManager instead", ) ) self.generic_visit(node) def visit_Compare(self, node): """Check for 'KEY in os.environ' containment checks.""" if not self._is_file_allowed(): for comparator in node.comparators: if ( isinstance(comparator, ast.Attribute) and comparator.attr == "environ" and isinstance(comparator.value, ast.Name) and comparator.value.id == "os" ): # Check if the left side is a system var if ( isinstance(node.left, ast.Constant) and node.left.value in SYSTEM_VARS ): continue self.errors.append( ( node.lineno, "'... in os.environ' check — use SettingsManager instead of direct env var access", ) ) self.generic_visit(node) def _is_file_allowed(self) -> bool: """Check if this file is allowed to use os.environ directly.""" p = Path(self.filename) if p.name in ALLOWED_NAMES: return True if p.name.startswith(ALLOWED_NAME_PREFIXES): return True if p.name.endswith(ALLOWED_NAME_SUFFIXES): return True if ALLOWED_PATH_SEGMENTS.intersection(p.parts): return True for ending in ALLOWED_PATH_ENDINGS: if self.filename.endswith(ending): return True return False def check_file(filename: str) -> bool: """Check a single Python file for direct env var access.""" if not filename.endswith(".py"): return True try: with open(filename, "r", encoding="utf-8") as f: content = f.read() except Exception as e: print(f"Error reading {filename}: {e}") return False try: tree = ast.parse(content, filename=filename) checker = EnvVarChecker(filename) checker.visit(tree) if checker.errors: print(f"\n{filename}:") for line_num, error in checker.errors: print(f" Line {line_num}: {error}") return False except SyntaxError: # Skip files with syntax errors pass except Exception as e: print(f"Error parsing {filename}: {e}") return False return True def main(): """Main function to check all staged Python files.""" if len(sys.argv) < 2: print("Usage: check-env-vars.py ...") sys.exit(1) files_to_check = sys.argv[1:] has_errors = False for filename in files_to_check: if not check_file(filename): has_errors = True if has_errors: print("\n⚠️ Direct environment variable access detected!") print("\nFor LDR_ variables, use SettingsManager instead of os.environ") print("See issue #598 for migration details") print("\nNote: Full validation runs in CI") sys.exit(1) else: sys.exit(0) if __name__ == "__main__": main()