learningcircuit--local-deep-research/.pre-commit-hooks/check-env-vars.py

#!/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 <file1> <file2> ...")
        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()