diff --git a/scripts/rules_benchmark/_helpers/workflow.py b/scripts/rules_benchmark/_helpers/workflow.py index d6fb84d897..f13c5daa2d 100644 --- a/scripts/rules_benchmark/_helpers/workflow.py +++ b/scripts/rules_benchmark/_helpers/workflow.py @@ -39,6 +39,21 @@ _CI_THRESHOLDS_PATH: Path = Path(__file__).parents[1] / "ci_thresholds.json" _GNU_TIME_PATH: Path = Path("/usr/bin/time") _PEAK_RSS_MARKER: str = "__SQLBUILD_BENCHMARK_PEAK_RSS_KIB__=" +_NON_CACHEABLE_PROBE_RULE: str = """ +import pathlib + + +@rule( + code="XSQBRB999", + slug="non-cacheable-probe", + message="Non-cacheable probe observed the rules directory", + remediation="Remove the non-cacheable probe.", + enabled_by_default=True, +) +def non_cacheable_probe(*, model: Model, ctx: RuleContext) -> list[Finding]: + listed = pathlib.os.listdir(pathlib.Path(__file__).parent) + return [ctx.finding(subject=model)] * int(not listed) +""" @dataclass(frozen=True) @@ -890,7 +905,7 @@ def _extract_peak_rss(stderr: str) -> tuple[str, int | None]: def _measure_rejection(*, project_dir: Path, iterations: int) -> BenchmarkResult: rule_file: Path = project_dir / "rules" / "benchmark_rules.py" with rule_file.open("a", encoding="utf-8") as handle: - handle.write("\ndef non_cacheable_probe():\n return open('untracked.txt')\n") + handle.write(_NON_CACHEABLE_PROBE_RULE) elapsed: list[float] = [] peak_rss_bytes: list[int] = [] for _ in range(iterations): diff --git a/scripts/rules_benchmark/constants.py b/scripts/rules_benchmark/constants.py index 10a3838dd8..1d901c4d70 100644 --- a/scripts/rules_benchmark/constants.py +++ b/scripts/rules_benchmark/constants.py @@ -8,5 +8,5 @@ CI_STRESS_WARM_ITERATIONS: int = 3 BENCHMARK_HELPER_RULE_INDEX: int = 14 MAX_CUSTOM_RULE_COUNT: int = 999 -NON_CACHEABLE_REJECTION: str = "call to open must go through RuleContext" +NON_CACHEABLE_REJECTION: str = "listing a directory is not allowed" SQL_SUFFIX: str = ".sql" diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/custom-rules/testing-and-determinism.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/custom-rules/testing-and-determinism.md index dca63b7350..c3641a48d4 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/custom-rules/testing-and-determinism.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/custom-rules/testing-and-determinism.md @@ -31,11 +31,31 @@ def test_given_missing_order_id_when_evaluating_then_reports_finding() -> None: `RuleCase.files` adds supporting project files. `RuleCase.config` supplies option values. Include passing cases, failing cases, near misses, and exact source-position assertions. +Each case also checks determinism. The harness evaluates every subject once, then evaluates every +subject again in reverse order in the same process. If the Rule's findings differ between the two +passes, the case fails with `custom rule is not deterministic`. The usual cause is +module-level state, such as a set of already-seen models, that carries over from one subject to +the next. Compute findings from the subject and `ctx` alone. + Repository pytest files remain outside the SQLBuild project's SQL `tests/` directory. ## Helpers -Custom Rules can import repository-owned helpers under `rules/` and supported pure Python modules. +Custom Rules can import repository-owned helpers under `rules/` and these deterministic standard +library modules: + +`abc`, `bisect`, `collections`, `copy`, `dataclasses`, `decimal`, `difflib`, `enum`, `fnmatch`, +`fractions`, `functools`, `graphlib`, `hashlib`, `heapq`, `itertools`, `json`, `math`, +`operator`, `pathlib`, `re`, `statistics`, `string`, `textwrap`, `types`, `typing`, and +`unicodedata`, plus `__future__` and `sqlbuild.rules`. + +Modules that expose the clock, randomness, the process, or the network are rejected, including +`time`, `datetime`, `random`, `uuid`, `secrets`, `os`, `sys`, `subprocess`, `socket`, and +`importlib`. SQLBuild checks the `import` statements in each selected Rule's file and in every +helper under `rules/` it imports, directly or through other helpers, with absolute or relative +imports anywhere in the module. These are the files the Rules cache fingerprints. Other Python files under +`rules/`, such as tests or scratch scripts, are not checked. + Changing an imported helper invalidates the Rules that depend on it. Only decorated functions register. Ordinary functions, constants, dataclasses, and classes remain @@ -43,8 +63,26 @@ helpers. ## Deterministic inputs -Direct filesystem, environment, clock, randomness, network, and subprocess access is rejected. -Those inputs cannot be reproduced safely by the Rules cache. +The Rules cache can only reuse a result when the Rule depends on tracked inputs. SQLBuild runs +custom Rules in a separate host process and enforces that at runtime: + +- Rules run with an empty environment, so `os.environ` is empty even when it is reached through + an allowed module, for example `pathlib.os.environ`. +- The host starts with `PYTHONHASHSEED=0`, so iterating over a set of strings gives the same order + on every run. +- The working directory is the resolved project directory, wherever `sqb` was invoked, so + `Path.cwd()` and relative paths do not vary between runs. +- While Rule code runs, opening files, reading file metadata such as `Path.exists()` or + `Path.stat()`, listing directories, running commands, forking, network access, and `ctypes` are + rejected, however they are reached. Loading a module outside the + allowed list for the first time is rejected too. + +A rejected operation fails the command with `non-hermetic custom rule at :`, even +if the Rule catches the exception. + +The guard exists for cache correctness: it stops Rules from depending on inputs the cache cannot +track by accident. It is not a security sandbox. Deliberately evasive code can bypass it, so only +run Rules you trust. Read supported project text and structure through `ctx.project.tree`: @@ -58,7 +96,8 @@ path invalidates the cached result. ## Cache granularity -Cache identity incorporates the Rule implementation, imported helper closure, configured options, -subject, accessed compiler facts, tracked project observations, dialect, and compatibility -versions. Prefer model subjects for independent per-model checks and project subjects for genuine +Cache identity incorporates the whole Rule source file, imported helper closure, configured +options, subject, accessed compiler facts, tracked project observations, dialect, and +compatibility versions. Editing any statement in a Rule file invalidates cached results for every +Rule defined in that file, so keep unrelated Rules in separate files. Prefer model subjects for independent per-model checks and project subjects for genuine cross-project invariants. diff --git a/src/sqlbuild/rule_engine/_helpers/engine/custom_rule_evidence.py b/src/sqlbuild/rule_engine/_helpers/engine/custom_rule_evidence.py index 3c79408b52..d049e0ee9d 100644 --- a/src/sqlbuild/rule_engine/_helpers/engine/custom_rule_evidence.py +++ b/src/sqlbuild/rule_engine/_helpers/engine/custom_rule_evidence.py @@ -14,6 +14,7 @@ _PUBLIC_HARNESS_MODULES: frozenset[str] = frozenset({"sqlbuild.rules", "sqlbuild.rules.testing"}) _INIT_MODULE_NAME: str = "__init__" _RULES_DIRECTORY_NAME: str = "rules" +_STAR_IMPORT_NAME: str = "*" _MIN_PARAMETRIZE_ARGUMENTS: int = 2 _PARAMETRIZE_ATTRIBUTE: str = "parametrize" _PYTEST_MARK_ATTRIBUTE: str = "mark" @@ -31,26 +32,6 @@ class _RuleTestEvidence: owner: str -class _ModuleBindingVisitor(ast.NodeVisitor): - """Collect names bound in module control flow without entering local scopes.""" - - def __init__(self) -> None: - self.names: set[str] = set() - - def visit_Name(self, node: ast.Name) -> None: - if isinstance(node.ctx, ast.Store): - self.names.add(node.id) - - def visit_FunctionDef(self, node: ast.FunctionDef) -> None: - self.names.add(node.name) - - def visit_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> None: - self.names.add(node.name) - - def visit_ClassDef(self, node: ast.ClassDef) -> None: - self.names.add(node.name) - - def custom_rule_test_evidence(*, rule: Rule, project_dir: Path) -> tuple[_RuleTestEvidence, ...]: """Return statically associated public-harness cases for one exact custom rule.""" @@ -87,66 +68,28 @@ def custom_rule_import_closure(*, rule: Rule, project_dir: Path) -> tuple[Path, def custom_rule_implementation_fingerprint( *, rule: Rule, project_dir: Path, import_closure: tuple[Path, ...] | None = None ) -> str: - """Fingerprint one rule function and its repository-owned imported helper closure.""" + """Fingerprint one rule's whole source file and its repository-owned imported helper closure.""" digest: Any = hashlib.sha256() if rule.source is None: digest.update(inspect.getsource(rule.check).encode()) return digest.hexdigest() - digest.update(inspect.getsource(rule.check).encode()) - digest.update(_module_binding_fingerprint(rule=rule)) root: Path = project_dir.resolve() + source_path: Path = Path(rule.source).resolve() closure: tuple[Path, ...] = ( custom_rule_import_closure(rule=rule, project_dir=root) if import_closure is None else import_closure ) - for path in closure: - digest.update(path.relative_to(root).as_posix().encode()) + for path in (source_path, *closure): + digest.update(_fingerprint_label(path=path, root=root).encode()) + digest.update(b"\0") digest.update(path.read_bytes()) return digest.hexdigest() -def _module_binding_fingerprint(*, rule: Rule) -> bytes: - if rule.source is None: - return b"" - source_path: Path = Path(rule.source).resolve() - try: - source: str = source_path.read_text(encoding="utf-8") - tree: ast.Module = ast.parse(source, filename=str(source_path)) - except (OSError, UnicodeError, SyntaxError): - return b"" - definitions: dict[str, ast.AST] = {} - for statement in tree.body: - visitor: _ModuleBindingVisitor = _ModuleBindingVisitor() - visitor.visit(statement) - for name in visitor.names: - definitions[name] = statement - check_name: str = getattr(rule.check, "__name__", "") - check_node: ast.AST | None = definitions.get(check_name) - pending: list[str] = [] if check_node is None else list(_loaded_names(check_node)) - visited: set[str] = {check_name} - parts: list[bytes] = [] - while pending: - name: str = pending.pop() - if name in visited: - continue - visited.add(name) - definition: ast.AST | None = definitions.get(name) - if definition is None: - continue - parts.append(name.encode()) - parts.append(ast.dump(definition, include_attributes=False).encode()) - pending.extend(_loaded_names(definition)) - return b"\0".join(parts) - - -def _loaded_names(node: ast.AST) -> tuple[str, ...]: - return tuple( - child.id - for child in ast.walk(node) - if isinstance(child, ast.Name) and isinstance(child.ctx, ast.Load) - ) +def _fingerprint_label(*, path: Path, root: Path) -> str: + return path.relative_to(root).as_posix() if path.is_relative_to(root) else path.as_posix() def custom_rule_project_fact_attributes( @@ -182,62 +125,85 @@ def custom_rule_project_fact_attributes( def _import_closure(*, source_path: Path, project_dir: Path) -> tuple[Path, ...]: + rules_root: Path = (project_dir / _RULES_DIRECTORY_NAME).resolve() pending: list[Path] = [source_path] visited: set[Path] = {source_path} helpers: set[Path] = set() while pending: current: Path = pending.pop() + candidates: set[Path] = set(_package_inits(path=current, rules_root=rules_root)) try: tree: ast.Module = ast.parse(current.read_text(encoding="utf-8"), filename=str(current)) except (OSError, UnicodeError, SyntaxError): - continue - for node in tree.body: - candidate: Path | None = _local_import_path( - node=node, - source_path=current, - project_dir=project_dir, + tree = ast.Module(body=[], type_ignores=[]) + for node in ast.walk(tree): + candidates.update( + _local_import_paths(node=node, source_path=current, rules_root=rules_root) ) - if candidate is None or candidate in visited: - continue + for candidate in candidates - visited: visited.add(candidate) helpers.add(candidate) pending.append(candidate) return tuple(sorted(helpers)) -def _local_import_path(*, node: ast.stmt, source_path: Path, project_dir: Path) -> Path | None: - module: str | None = None - base: Path - level: int = 0 - if isinstance(node, ast.ImportFrom): - module = node.module - level = node.level +def _local_import_paths(*, node: ast.AST, source_path: Path, rules_root: Path) -> tuple[Path, ...]: + """Resolve every repository-owned rules module one import statement can execute.""" + + targets: list[tuple[Path, tuple[str, ...]]] = [] + if isinstance(node, ast.Import): + targets.extend( + (rules_root.parent, tuple(alias.name.split("."))) + for alias in node.names + if alias.name.split(".")[0] == _RULES_DIRECTORY_NAME + ) + elif isinstance(node, ast.ImportFrom): + base: Path = rules_root.parent + module_parts: tuple[str, ...] = tuple((node.module or "").split(".")) if node.module else () if node.level: base = source_path.parent for _ in range(node.level - 1): base = base.parent - else: - base = project_dir - elif isinstance(node, ast.Import) and len(node.names) == 1: - module = node.names[0].name - base = project_dir - else: - return None - if module is None: - return None - parts: tuple[str, ...] = tuple(module.split(".")) - if not level and (not parts or parts[0] != _RULES_DIRECTORY_NAME): - return None + elif not module_parts or module_parts[0] != _RULES_DIRECTORY_NAME: + return () + targets.append((base, module_parts)) + targets.extend( + (base, (*module_parts, alias.name)) + for alias in node.names + if alias.name != _STAR_IMPORT_NAME + ) + resolved: list[Path] = [] + for base, parts in targets: + module: Path | None = _module_file(base=base, parts=parts, rules_root=rules_root) + if module is not None: + resolved.append(module) + resolved.extend(_package_inits(path=module, rules_root=rules_root)) + return tuple(resolved) + + +def _module_file(*, base: Path, parts: tuple[str, ...], rules_root: Path) -> Path | None: candidate: Path = base.joinpath(*parts) - file_candidate: Path = candidate.with_suffix(".py") - package_candidate: Path = candidate / "__init__.py" - resolved: Path | None = file_candidate if file_candidate.is_file() else None - if resolved is None and package_candidate.is_file(): - resolved = package_candidate - if resolved is None: - return None - resolved = resolved.resolve() - return resolved if resolved.is_relative_to(project_dir) else None + for path in (candidate / "__init__.py", candidate.with_suffix(".py") if parts else None): + if path is not None and path.is_file(): + resolved: Path = path.resolve() + return resolved if resolved.is_relative_to(rules_root) else None + return None + + +def _package_inits(*, path: Path, rules_root: Path) -> tuple[Path, ...]: + """Return package initializers executed before one module under the rules root.""" + + inits: list[Path] = [] + for parent in path.parents: + if not parent.is_relative_to(rules_root): + break + init: Path = parent / "__init__.py" + if not init.is_file(): + continue + resolved: Path = init.resolve() + if resolved != path and resolved.is_relative_to(rules_root): + inits.append(resolved) + return tuple(inits) def _file_evidence( diff --git a/src/sqlbuild/rule_engine/_helpers/engine/hermeticity.py b/src/sqlbuild/rule_engine/_helpers/engine/hermeticity.py index 2824321c91..b7fc4c16e4 100644 --- a/src/sqlbuild/rule_engine/_helpers/engine/hermeticity.py +++ b/src/sqlbuild/rule_engine/_helpers/engine/hermeticity.py @@ -1,79 +1,41 @@ -"""Static hermeticity verification for cacheable custom rules.""" +"""Static import verification for the fingerprinted custom-rule source closure.""" from __future__ import annotations import ast from pathlib import Path -from sqlbuild.rule_engine.exceptions import RulesError +from sqlbuild.rule_engine._helpers.engine.custom_rule_evidence import custom_rule_import_closure +from sqlbuild.rule_engine.constants import CUSTOM_RULE_IMPORT_ROOTS +from sqlbuild.rule_engine.exceptions import NonHermeticRuleError, RulesError from sqlbuild.rule_engine.models import Rule -_CONTEXT_TREE_CALLS: frozenset[str] = frozenset({"glob", "read_text"}) - -_ALLOWED_IMPORT_ROOTS: frozenset[str] = frozenset( - { - "__future__", - "collections", - "dataclasses", - "decimal", - "enum", - "functools", - "graphlib", - "itertools", - "math", - "pathlib", - "re", - "rules", - "sqlbuild.rules", - "statistics", - "typing", - } -) -_BANNED_CALLS: frozenset[str] = frozenset( - { - "__import__", - "compile", - "eval", - "exec", - "input", - "open", - "exists", - "glob", - "iterdir", - "read_bytes", - "read_text", - "rglob", - "stat", - "write_bytes", - "write_text", - } -) - def verify_custom_rules(*, rules: tuple[Rule, ...], project_dir: Path) -> None: - """Reject custom-rule source that can observe untracked process or filesystem state.""" + """Reject imports outside the deterministic allowlist in every fingerprinted rule file.""" + root: Path = project_dir.resolve() checked: set[Path] = set() for rule in rules: if not rule.custom or rule.source is None: continue source_path: Path = Path(rule.source).resolve() - source_root: Path = _source_root(source_path=source_path, project_dir=project_dir) - paths: tuple[Path, ...] = tuple(sorted(source_root.rglob("*.py"))) - for path in paths: + closure: tuple[Path, ...] = custom_rule_import_closure(rule=rule, project_dir=root) + for path in (source_path, *closure): if path in checked: continue checked.add(path) - _verify_source(path=path, project_dir=project_dir) + _verify_source(path=path, project_dir=root) -def _source_root(*, source_path: Path, project_dir: Path) -> Path: - rules_root: Path = project_dir.resolve() / "rules" - return rules_root if source_path.is_relative_to(rules_root) else source_path.parent +def allowed_custom_rule_import(module: str) -> bool: + """Return whether custom-rule code may import one absolute module name.""" + + return any(module == root or module.startswith(f"{root}.") for root in CUSTOM_RULE_IMPORT_ROOTS) def _verify_source(*, path: Path, project_dir: Path) -> None: - if not path.is_relative_to(project_dir.resolve()): + if not path.is_relative_to(project_dir): raise RulesError(f"custom rule source must be repository-owned: {path}") source: str = path.read_text(encoding="utf-8") tree: ast.Module = ast.parse(source, filename=str(path)) @@ -81,45 +43,12 @@ def _verify_source(*, path: Path, project_dir: Path) -> None: if isinstance(node, ast.Import): for alias in node.names: _verify_import(path=path, line=node.lineno, module=alias.name) - elif isinstance(node, ast.ImportFrom): - if node.level == 0: - _verify_import(path=path, line=node.lineno, module=node.module or "") - elif isinstance(node, ast.Call): - call_name: str | None = _call_name(node.func) - mediated: bool = ( - isinstance(node.func, ast.Attribute) - and call_name in _CONTEXT_TREE_CALLS - and _attribute_path(node.func).endswith(".project.tree." + str(call_name)) - ) - if call_name in _BANNED_CALLS and not mediated: - raise RulesError( - f"non-hermetic custom rule at {path}:{node.lineno}: " - f"call to {call_name} must go through RuleContext" - ) + elif isinstance(node, ast.ImportFrom) and node.level == 0: + _verify_import(path=path, line=node.lineno, module=node.module or "") def _verify_import(*, path: Path, line: int, module: str) -> None: - allowed: bool = any( - module == root or module.startswith(f"{root}.") for root in _ALLOWED_IMPORT_ROOTS - ) - if not allowed: - raise RulesError( + if not allowed_custom_rule_import(module): + raise NonHermeticRuleError( f"non-hermetic custom rule at {path}:{line}: import {module!r} is not allowed" ) - - -def _call_name(node: ast.expr) -> str | None: - if isinstance(node, ast.Name): - return node.id - if isinstance(node, ast.Attribute): - return node.attr - return None - - -def _attribute_path(node: ast.expr) -> str: - if isinstance(node, ast.Name): - return node.id - if isinstance(node, ast.Attribute): - prefix: str = _attribute_path(node.value) - return f"{prefix}.{node.attr}" if prefix else node.attr - return "" diff --git a/src/sqlbuild/rule_engine/_helpers/engine/native.py b/src/sqlbuild/rule_engine/_helpers/engine/native.py index 34933180c2..cc055cd92b 100644 --- a/src/sqlbuild/rule_engine/_helpers/engine/native.py +++ b/src/sqlbuild/rule_engine/_helpers/engine/native.py @@ -41,6 +41,7 @@ custom_rule_test_evidence, ) from sqlbuild.rule_engine.constants import ( + CUSTOM_HOST_LAUNCH_MODULE, CUSTOM_HOST_RUNTIME_VERSION, RULE_CONTEXT_AUDITS_FACT, RULE_CONTEXT_DECLARATIONS_FACT, @@ -73,6 +74,7 @@ def evaluate_native( dialect: str = "generic", initial_findings: tuple[Finding, ...] = (), defer_suppressions: bool = False, + verify_determinism: bool = False, ) -> RulesResult: """Evaluate one compiled model batch through the native engine.""" @@ -122,6 +124,7 @@ def evaluate_native( project_dir=project_dir, catalogue=tuple(selected_catalogue), dialect=dialect, + verify_determinism=verify_determinism, ) request["custom_host"] = custom_host try: @@ -137,6 +140,7 @@ def evaluate_native( project_dir=project_dir, catalogue=tuple(selected_catalogue), dialect=dialect, + verify_determinism=verify_determinism, ) request["custom_host"] = custom_host response_json = _evaluate_request(request) @@ -731,6 +735,7 @@ def _custom_host_payload( project_dir: Path, catalogue: tuple[Rule, ...], dialect: str, + verify_determinism: bool, ) -> tuple[dict[str, object] | None, Path | None]: if not any(rule.custom for rule in catalogue): return None, None @@ -765,13 +770,14 @@ def _custom_host_payload( return ( { "program": sys.executable, - "arguments": ["-m", "sqlbuild.rule_engine._helpers.engine.custom_host"], + "arguments": ["-m", CUSTOM_HOST_LAUNCH_MODULE], "timeout_millis": 120_000, "runtime_version": CUSTOM_HOST_RUNTIME_VERSION, "payload": { "project_pickle_path": str(input_path.resolve()), "project_dir": str(project_dir.resolve()), "dialect": dialect, + "verify_determinism": verify_determinism, }, }, input_path, diff --git a/src/sqlbuild/rule_engine/_helpers/engine/rule_harness.py b/src/sqlbuild/rule_engine/_helpers/engine/rule_harness.py index 0eeeec54bc..bd2b1853fa 100644 --- a/src/sqlbuild/rule_engine/_helpers/engine/rule_harness.py +++ b/src/sqlbuild/rule_engine/_helpers/engine/rule_harness.py @@ -45,6 +45,7 @@ def run_rule_case(*, rule: RuleCheck | Rule, test_case: RuleCase) -> RuleResult: project=project, config=config, project_dir=root, + verify_determinism=True, ) if len(result.findings) != test_case.expected_finding_count: raise RuleAssertionError( diff --git a/src/sqlbuild/rule_engine/_helpers/engine/ruleset.py b/src/sqlbuild/rule_engine/_helpers/engine/ruleset.py index 5df5356bce..1455afdfa1 100644 --- a/src/sqlbuild/rule_engine/_helpers/engine/ruleset.py +++ b/src/sqlbuild/rule_engine/_helpers/engine/ruleset.py @@ -19,7 +19,12 @@ def evaluate_project( - *, project: CompiledProject, config: RulesConfig, project_dir: Path, dialect: str = "generic" + *, + project: CompiledProject, + config: RulesConfig, + project_dir: Path, + dialect: str = "generic", + verify_determinism: bool = False, ) -> RulesResult: """Evaluate selected rules through one native project boundary.""" @@ -40,6 +45,7 @@ def evaluate_project( project_dir=project_dir, catalogue=catalogue, dialect=dialect, + verify_determinism=verify_determinism, ) diff --git a/src/sqlbuild/rule_engine/_helpers/engine/custom_evaluation.py b/src/sqlbuild/rule_engine/_helpers/host/custom_evaluation.py similarity index 52% rename from src/sqlbuild/rule_engine/_helpers/engine/custom_evaluation.py rename to src/sqlbuild/rule_engine/_helpers/host/custom_evaluation.py index 47e48f48e0..b77e900b88 100644 --- a/src/sqlbuild/rule_engine/_helpers/engine/custom_evaluation.py +++ b/src/sqlbuild/rule_engine/_helpers/host/custom_evaluation.py @@ -2,6 +2,8 @@ from __future__ import annotations +from collections import Counter +from contextlib import AbstractContextManager, nullcontext from pathlib import Path from sqlbuild.compiler.compile.models import CompiledModel, CompiledProject @@ -11,10 +13,13 @@ RuleFactViews, build_rule_fact_views, ) -from sqlbuild.rule_engine.exceptions import RulesError +from sqlbuild.rule_engine.classes.runtime_guard import RuntimeGuard +from sqlbuild.rule_engine.exceptions import NonHermeticRuleError, RulesError from sqlbuild.rule_engine.models import Finding, Model, Project, Rule, RulesConfig from sqlbuild.rule_engine.types import RuleSubject +type _FindingKey = tuple[str, str, int, int, str, str] + def evaluate_custom_rules( *, @@ -24,26 +29,61 @@ def evaluate_custom_rules( selected_rules: tuple[Rule, ...], dialect: str = "generic", selected_model_paths: frozenset[str] | None = None, + verify_determinism: bool = False, + guard: RuntimeGuard | None = None, ) -> list[Finding]: """Run only selected custom rules through the Python authoring API.""" custom_rules: tuple[Rule, ...] = tuple(rule for rule in selected_rules if rule.custom) if not custom_rules: return [] - findings: list[Finding] = [] models: tuple[CompiledModel, ...] = tuple( model for model in sorted(project.models, key=lambda item: item.relative_path.as_posix()) if selected_model_paths is None or model.relative_path.as_posix() in selected_model_paths ) + facts: RuleFactViews = build_rule_fact_views( + project=project, project_dir=project_dir, dialect=dialect + ) + + def run_pass(*, rules: tuple[Rule, ...], subjects: tuple[CompiledModel, ...]) -> list[Finding]: + return _evaluate_pass( + project=project, + config=config, + project_dir=project_dir, + selected_rules=selected_rules, + dialect=dialect, + facts=facts, + rules=rules, + models=subjects, + guard=guard, + ) + + findings: list[Finding] = run_pass(rules=custom_rules, subjects=models) + if verify_determinism: + repeated: list[Finding] = run_pass(rules=custom_rules[::-1], subjects=models[::-1]) + _verify_repeatable(rules=custom_rules, first=findings, repeated=repeated) + return findings + + +def _evaluate_pass( + *, + project: CompiledProject, + config: RulesConfig, + project_dir: Path, + selected_rules: tuple[Rule, ...], + dialect: str, + facts: RuleFactViews, + rules: tuple[Rule, ...], + models: tuple[CompiledModel, ...], + guard: RuntimeGuard | None, +) -> list[Finding]: + findings: list[Finding] = [] project_rules: tuple[Rule, ...] = tuple( - rule for rule in custom_rules if rule.subject is RuleSubject.PROJECT + rule for rule in rules if rule.subject is RuleSubject.PROJECT ) model_rules: tuple[Rule, ...] = tuple( - rule for rule in custom_rules if rule.subject is RuleSubject.MODEL - ) - facts: RuleFactViews = build_rule_fact_views( - project=project, project_dir=project_dir, dialect=dialect + rule for rule in rules if rule.subject is RuleSubject.MODEL ) model_contexts: tuple[tuple[Rule, EvaluationRuleContext], ...] = tuple( ( @@ -81,15 +121,20 @@ def evaluate_custom_rules( target_name=project.effective_target_name, ), ctx=ctx, + guard=guard, ) ) + except NonHermeticRuleError: + raise except Exception as error: raise RulesError(f"rule {rule.code} failed for the project: {error}") from error for model in models: subject: Model = public_model(model) for rule, ctx in model_contexts: try: - findings.extend(_invoke(rule=rule, subject=subject, ctx=ctx)) + findings.extend(_invoke(rule=rule, subject=subject, ctx=ctx, guard=guard)) + except NonHermeticRuleError: + raise except Exception as error: raise RulesError( f"rule {rule.code} failed for {model.relative_path}: {error}" @@ -97,15 +142,49 @@ def evaluate_custom_rules( return findings -def _invoke(*, rule: Rule, subject: object, ctx: EvaluationRuleContext) -> list[Finding]: +def _invoke( + *, rule: Rule, subject: object, ctx: EvaluationRuleContext, guard: RuntimeGuard | None +) -> list[Finding]: if rule.subject_parameter is None or rule.context_parameter is None: raise RulesError(f"custom rule {rule.code} has no resolved typed signature") - result: object = rule.check( - **{ - rule.subject_parameter: subject, - rule.context_parameter: ctx, - } - ) + scope: AbstractContextManager[None] = nullcontext() if guard is None else guard.rule(rule.code) + with scope: + result: object = rule.check( + **{ + rule.subject_parameter: subject, + rule.context_parameter: ctx, + } + ) if not isinstance(result, list) or any(not isinstance(finding, Finding) for finding in result): raise RulesError(f"custom rule {rule.code} must return list[Finding]") return result + + +def _verify_repeatable( + *, rules: tuple[Rule, ...], first: list[Finding], repeated: list[Finding] +) -> None: + for rule in rules: + before: Counter[_FindingKey] = _finding_keys(findings=first, code=rule.code) + after: Counter[_FindingKey] = _finding_keys(findings=repeated, code=rule.code) + if before != after: + raise RulesError( + f"custom rule {rule.code} is not deterministic: evaluating every subject again " + f"after the others changed its findings from {before.total()} to " + f"{after.total()}; keep per-subject results independent of module-level state " + "and evaluation order" + ) + + +def _finding_keys(*, findings: list[Finding], code: str) -> Counter[_FindingKey]: + return Counter( + ( + finding.code, + finding.path.as_posix(), + finding.line, + finding.column, + finding.message, + finding.remediation, + ) + for finding in findings + if finding.code == code + ) diff --git a/src/sqlbuild/rule_engine/_helpers/engine/custom_host.py b/src/sqlbuild/rule_engine/_helpers/host/custom_host.py similarity index 82% rename from src/sqlbuild/rule_engine/_helpers/engine/custom_host.py rename to src/sqlbuild/rule_engine/_helpers/host/custom_host.py index 988ce2e70c..89b37e8877 100644 --- a/src/sqlbuild/rule_engine/_helpers/engine/custom_host.py +++ b/src/sqlbuild/rule_engine/_helpers/host/custom_host.py @@ -5,6 +5,7 @@ import contextlib import io import json +import os import pickle import sys from dataclasses import asdict @@ -13,9 +14,12 @@ from sqlbuild.compiler.compile.models import CompiledProject from sqlbuild.rule_engine._helpers.engine.catalogue import build_catalogue -from sqlbuild.rule_engine._helpers.engine.custom_evaluation import evaluate_custom_rules +from sqlbuild.rule_engine._helpers.host.custom_evaluation import evaluate_custom_rules +from sqlbuild.rule_engine.classes.runtime_guard import RuntimeGuard from sqlbuild.rule_engine.constants import ( + CUSTOM_HOST_HASH_SEED, CUSTOM_HOST_INPUT_TUPLE_SIZE, + CUSTOM_HOST_LAUNCH_MODULE, CUSTOM_HOST_PROTOCOL_VERSION, CUSTOM_HOST_RUNTIME_VERSION, ) @@ -26,6 +30,11 @@ def main() -> int: """Read one Fensu host request and write exactly one response.""" + if sys.flags.hash_randomization: + return _write_error( + f"custom host must be started by {CUSTOM_HOST_LAUNCH_MODULE} " + f"with PYTHONHASHSEED={CUSTOM_HOST_HASH_SEED}" + ) request: object = json.load(sys.stdin) if not isinstance(request, dict): return _write_error("custom host request must be an object") @@ -33,6 +42,7 @@ def main() -> int: runtime_version: object = request.get("runtime_version") if protocol != CUSTOM_HOST_PROTOCOL_VERSION or runtime_version != CUSTOM_HOST_RUNTIME_VERSION: return _write_error("unsupported custom host protocol or runtime") + guard: RuntimeGuard | None = None try: payload: object = request["payload"] if not isinstance(payload, dict): @@ -40,6 +50,7 @@ def main() -> int: project, config = _decode_inputs(payload) project_dir: Path = Path(str(payload["project_dir"])).resolve() dialect: str = str(payload.get("dialect", "generic")) + verify_determinism: bool = payload.get("verify_determinism") is True selected_codes: tuple[str, ...] = tuple(str(code) for code in payload["selected_codes"]) raw_model_paths: object = payload.get("selected_model_paths") selected_model_paths: frozenset[str] | None = ( @@ -47,9 +58,15 @@ def main() -> int: if isinstance(raw_model_paths, list) else None ) + os.environ.clear() + os.chdir(project_dir) + guard = RuntimeGuard(project_dir=project_dir) + sys.addaudithook(guard) + guard.guard_filesystem_metadata() messages: io.StringIO = io.StringIO() with contextlib.redirect_stdout(messages): catalogue: tuple[Rule, ...] = build_catalogue(config=config, project_dir=project_dir) + guard.raise_violation() by_code: dict[str, Rule] = {rule.code: rule for rule in catalogue} selected: tuple[Rule, ...] = tuple(by_code[code] for code in selected_codes) findings: list[Finding] = evaluate_custom_rules( @@ -59,9 +76,12 @@ def main() -> int: selected_rules=selected, dialect=dialect, selected_model_paths=selected_model_paths, + verify_determinism=verify_determinism, + guard=guard, ) except Exception as error: - return _write_error(str(error)) + violation: Exception | None = None if guard is None else guard.violation + return _write_error(str(violation or error)) response: dict[str, object] = { "protocol": CUSTOM_HOST_PROTOCOL_VERSION, "runtime_version": CUSTOM_HOST_RUNTIME_VERSION, diff --git a/src/sqlbuild/rule_engine/_helpers/host/custom_host_launch.py b/src/sqlbuild/rule_engine/_helpers/host/custom_host_launch.py new file mode 100644 index 0000000000..f9c1a049c6 --- /dev/null +++ b/src/sqlbuild/rule_engine/_helpers/host/custom_host_launch.py @@ -0,0 +1,32 @@ +"""Start the custom-rule host with a scrubbed environment and a fixed hash seed.""" + +from __future__ import annotations + +import os +import subprocess +import sys + +from sqlbuild.rule_engine.constants import ( + CUSTOM_HOST_EXEC_OS_NAME, + CUSTOM_HOST_HASH_SEED, + CUSTOM_HOST_MODULE, + CUSTOM_HOST_STARTUP_ENVIRONMENT, +) + + +def main() -> int: + """Replace this launcher with the host, or relay to it where exec is unavailable.""" + + environment: dict[str, str] = { + name: os.environ[name] for name in CUSTOM_HOST_STARTUP_ENVIRONMENT if name in os.environ + } + environment["PYTHONHASHSEED"] = CUSTOM_HOST_HASH_SEED + environment["PYTHONUTF8"] = "1" + arguments: list[str] = [sys.executable, "-P", "-m", CUSTOM_HOST_MODULE] + if os.name == CUSTOM_HOST_EXEC_OS_NAME: + os.execve(sys.executable, arguments, environment) + return subprocess.run(arguments, env=environment, check=False).returncode + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/src/sqlbuild/rule_engine/classes/runtime_guard.py b/src/sqlbuild/rule_engine/classes/runtime_guard.py new file mode 100644 index 0000000000..847fc4b1a2 --- /dev/null +++ b/src/sqlbuild/rule_engine/classes/runtime_guard.py @@ -0,0 +1,241 @@ +"""Runtime hermeticity audit hook for the custom-rule host.""" + +from __future__ import annotations + +import functools +import importlib +import os +import sys +import threading +from collections.abc import Callable, Iterator +from contextlib import contextmanager +from pathlib import Path +from types import CodeType, FrameType, FunctionType, ModuleType + +from sqlbuild.rule_engine._helpers.engine.hermeticity import allowed_custom_rule_import +from sqlbuild.rule_engine.classes.project_tree import ProjectTree +from sqlbuild.rule_engine.exceptions import NonHermeticRuleError + +_IMPORT_EVENT: str = "import" +_OPEN_EVENT: str = "open" +_IMPORT_ENTRY_FUNCTION: str = "_find_and_load" +_FROZEN_IMPORT_PREFIX: str = " None: + self._project_dir: Path = project_dir.resolve() + self._rules_prefix: str = f"{self._project_dir / 'rules'}{os.sep}" + self._importlib_prefix: str = f"{Path(importlib.__file__).resolve().parent}{os.sep}" + self._mediated_codes: tuple[CodeType, ...] = _project_tree_codes() + self._mediated_code_ids: frozenset[int] = frozenset(map(id, self._mediated_codes)) + self._rule_files: dict[str, bool] = {} + self._metadata_scope: threading.local = threading.local() + self.active_rule: str | None = None + self.violation: NonHermeticRuleError | None = None + + def __call__(self, event: str, arguments: tuple[object, ...]) -> None: + if event not in _GUARDED_EVENTS and not event.startswith(_GUARDED_PREFIXES): + return + frame: FrameType = sys._getframe(1) + rule_frame: FrameType | None = ( + self._importing_rule_frame(frame) + if event == _IMPORT_EVENT + else self._responsible_rule_frame(frame) + ) + if rule_frame is None: + return + if event == _IMPORT_EVENT and allowed_custom_rule_import(str(arguments[0])): + return + self._reject(rule_frame=rule_frame, action=_action(event=event, arguments=arguments)) + + def guard_filesystem_metadata(self) -> None: + """Route unaudited os and os.path metadata calls through the Rule attribution check.""" + + self._wrap(module=os, names=_OS_METADATA_FUNCTIONS) + self._wrap(module=os.path, names=_PATH_METADATA_FUNCTIONS) + + def _wrap(self, *, module: ModuleType, names: tuple[str, ...]) -> None: + for name in names: + original: object = getattr(module, name, None) + if callable(original): + guarded: Callable[..., object] = self._guarded(original) + setattr(module, name, guarded) + for capability in _OS_CAPABILITY_SETS: + supported: set[object] = getattr(os, capability) + if original in supported: + supported.add(guarded) + + def _guarded(self, function: Callable[..., object]) -> Callable[..., object]: + guarded: functools.partial[object] = functools.partial( + self._call_metadata, guarded_function=function + ) + return functools.update_wrapper(guarded, function) + + def _call_metadata( + self, *arguments: object, guarded_function: Callable[..., object], **keywords: object + ) -> object: + function: Callable[..., object] = guarded_function + if getattr(self._metadata_scope, "trusted", False): + return function(*arguments, **keywords) + paths: tuple[object, ...] = tuple(map(_plain_path, arguments)) + named: dict[str, object] = {key: _plain_path(value) for key, value in keywords.items()} + rule_frame: FrameType | None = self._responsible_rule_frame(sys._getframe(1)) + if rule_frame is not None: + target: str = f" for {paths[0]!r}" if paths else "" + self._reject(rule_frame=rule_frame, action=f"{_METADATA_ACTION}{target}") + self._metadata_scope.trusted = True + try: + return function(*paths, **named) + finally: + self._metadata_scope.trusted = False + + @contextmanager + def rule(self, code: str) -> Iterator[None]: + """Attribute violations to one Rule and surface them even when Rule code catches them.""" + + self.active_rule = code + try: + yield + except Exception: + self.raise_violation() + raise + finally: + self.active_rule = None + self.raise_violation() + + def raise_violation(self) -> None: + """Raise the first recorded violation, if any.""" + + if self.violation is not None: + raise self.violation + + def _reject(self, *, rule_frame: FrameType, action: str) -> None: + filename: Path = Path(rule_frame.f_code.co_filename) + location: str = ( + filename.relative_to(self._project_dir).as_posix() + if filename.is_relative_to(self._project_dir) + else filename.as_posix() + ) + owner: str = "" if self.active_rule is None else f" {self.active_rule}" + error: NonHermeticRuleError = NonHermeticRuleError( + f"non-hermetic custom rule{owner} at {location}:{rule_frame.f_lineno}: " + f"{action} is not allowed; {_REMEDIATION}" + ) + if self.violation is None: + self.violation = error + raise error + + def _responsible_rule_frame(self, frame: FrameType | None) -> FrameType | None: + """Return the innermost Rule frame not shielded by module import or a mediated read.""" + + rule_files: dict[str, bool] = self._rule_files + mediated: frozenset[int] = self._mediated_code_ids + while frame is not None: + code: CodeType = frame.f_code + is_rule: bool | None = rule_files.get(code.co_filename) + if is_rule is None: + is_rule = self._is_rule_file(code.co_filename) + if is_rule: + return frame + if id(code) in mediated or ( + code.co_name == _IMPORT_ENTRY_FUNCTION + and self._is_import_machinery(code.co_filename) + ): + return None + frame = frame.f_back + return None + + def _importing_rule_frame(self, frame: FrameType | None) -> FrameType | None: + while frame is not None and self._is_import_machinery(frame.f_code.co_filename): + frame = frame.f_back + if frame is None or not self._is_rule_file(frame.f_code.co_filename): + return None + return frame + + def _is_rule_file(self, filename: str) -> bool: + cached: bool | None = self._rule_files.get(filename) + if cached is None: + cached = filename.startswith(self._rules_prefix) + self._rule_files[filename] = cached + return cached + + def _is_import_machinery(self, filename: str) -> bool: + return filename.startswith((_FROZEN_IMPORT_PREFIX, self._importlib_prefix)) + + +def _project_tree_codes() -> tuple[CodeType, ...]: + codes: list[CodeType] = [] + for value in vars(ProjectTree).values(): + member: object = getattr(value, "__func__", value) + if isinstance(member, FunctionType): + codes.append(member.__code__) + return tuple(codes) + + +def _plain_path(value: object) -> object: + return os.fspath(value) if isinstance(value, os.PathLike) else value + + +def _action(*, event: str, arguments: tuple[object, ...]) -> str: + if event == _OPEN_EVENT and arguments: + return f"opening {arguments[0]!r}" + if event == _IMPORT_EVENT and arguments: + return f"import {arguments[0]!r}" + action: str | None = _FILESYSTEM_EVENTS.get(event) or _PROCESS_EVENTS.get(event) + if action is not None: + return action + return next( + (prefix_action for prefix, prefix_action in _PREFIX_EVENTS if event.startswith(prefix)), + event, + ) diff --git a/src/sqlbuild/rule_engine/constants.py b/src/sqlbuild/rule_engine/constants.py index 3561e0ae4a..ba04b7294c 100644 --- a/src/sqlbuild/rule_engine/constants.py +++ b/src/sqlbuild/rule_engine/constants.py @@ -10,6 +10,52 @@ CUSTOM_HOST_PROTOCOL_VERSION: int = 1 CUSTOM_HOST_RUNTIME_VERSION: str = "sqlbuild-rules-custom-v1" CUSTOM_HOST_INPUT_TUPLE_SIZE: int = 2 +CUSTOM_HOST_MODULE: str = "sqlbuild.rule_engine._helpers.host.custom_host" +CUSTOM_HOST_LAUNCH_MODULE: str = "sqlbuild.rule_engine._helpers.host.custom_host_launch" +CUSTOM_HOST_HASH_SEED: str = "0" +CUSTOM_HOST_EXEC_OS_NAME: str = "posix" +CUSTOM_HOST_STARTUP_ENVIRONMENT: tuple[str, ...] = ( + "APPDATA", + "HOME", + "PYTHONHOME", + "PYTHONNOUSERSITE", + "PYTHONPATH", + "PYTHONUSERBASE", + "SYSTEMROOT", +) +CUSTOM_RULE_IMPORT_ROOTS: frozenset[str] = frozenset( + { + "__future__", + "abc", + "bisect", + "collections", + "copy", + "dataclasses", + "decimal", + "difflib", + "enum", + "fnmatch", + "fractions", + "functools", + "graphlib", + "hashlib", + "heapq", + "itertools", + "json", + "math", + "operator", + "pathlib", + "re", + "rules", + "sqlbuild.rules", + "statistics", + "string", + "textwrap", + "types", + "typing", + "unicodedata", + } +) SKILL_FRESH: str = "fresh" SKILL_MISSING: str = "missing" SKILL_STALE: str = "stale" diff --git a/src/sqlbuild/rule_engine/exceptions.py b/src/sqlbuild/rule_engine/exceptions.py index 25d1738fe5..2d4ace09b4 100644 --- a/src/sqlbuild/rule_engine/exceptions.py +++ b/src/sqlbuild/rule_engine/exceptions.py @@ -9,5 +9,9 @@ class RuleUsageError(RulesError): """Raised when a rule or rule test uses the public API incorrectly.""" +class NonHermeticRuleError(RulesError): + """Raised when a custom rule observes state the Rules cache cannot track.""" + + class RuleAssertionError(RulesError): """Raised when a custom-rule harness case produces an unexpected result.""" diff --git a/src/sqlbuild/rule_engine/main/_evaluate.py b/src/sqlbuild/rule_engine/main/_evaluate.py index 535b1f9f0d..f6e6a4036d 100644 --- a/src/sqlbuild/rule_engine/main/_evaluate.py +++ b/src/sqlbuild/rule_engine/main/_evaluate.py @@ -13,6 +13,7 @@ def evaluate( config: RulesConfig, project_dir: Path, dialect: str = "generic", + verify_determinism: bool = False, ) -> RulesResult: """Evaluate the selected compiler rules over compiled models.""" @@ -21,4 +22,5 @@ def evaluate( config=config, project_dir=project_dir, dialect=dialect, + verify_determinism=verify_determinism, ) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/rules/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/rules/_test_types.py index 279f1cc6c1..5d281bde90 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/rules/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/rules/_test_types.py @@ -55,3 +55,78 @@ class UnusedCteContextTestCase: files: dict[str, str] expected_returncode: int expected_finding_count: int + + +@dataclass(frozen=True) +class HermeticRuleCase: + description: str + code: str + files: tuple[tuple[str, str], ...] + expected_rule_codes: tuple[str, ...] + + +@dataclass(frozen=True) +class NonHermeticRuleCase: + description: str + code: str + source: str + expected_line: int + expected_action: str + + +@dataclass(frozen=True) +class ScrubbedEnvironmentCase: + description: str + code: str + source: str + parent_environment: tuple[tuple[str, str], ...] + expected_returncode: int + expected_rule_codes: tuple[str, ...] + + +@dataclass(frozen=True) +class HashSeedCase: + description: str + code: str + names: tuple[str, ...] + expected_hash_seed: str + + +@dataclass(frozen=True) +class ProjectTreeCacheCase: + description: str + code: str + source: str + policy_path: str + added_model_path: str + expected_paths_after_glob_change: tuple[str, ...] + expected_minimum_cache_hits: int + + +@dataclass(frozen=True) +class ModuleStatementEditCase: + description: str + code: str + source: str + appended_statement: str + expected_rule_codes_before_edit: tuple[str, ...] + expected_rule_codes_after_edit: tuple[str, ...] + + +@dataclass(frozen=True) +class WorkingDirectoryCase: + description: str + code: str + source: str + invocation_directories: tuple[str, ...] + expected_working_directory: str + + +@dataclass(frozen=True) +class HelperImportCase: + description: str + code: str + rule_source: str + helper_path: str + helper_source: str + expected_error: str diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/rules/helpers.py b/tests/e2e/src/sqlbuild/cli/commands/main/rules/helpers.py index 575bc1e80d..659ca60fa4 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/rules/helpers.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/rules/helpers.py @@ -1,9 +1,11 @@ """Compiler Rules end-to-end fixture helpers.""" import json +import os import subprocess import sys from pathlib import Path +from typing import Any from scripts.rules_benchmark._helpers.custom_rules import write_custom_rules @@ -54,3 +56,112 @@ def run_unevaluated_rules_cli(project_dir: Path, *args: str) -> subprocess.Compl text=True, check=False, ) + + +def write_custom_rule_project( + *, + project_dir: Path, + selected_rules: tuple[str, ...], + files: tuple[tuple[str, str], ...], + configuration: str = "", +) -> None: + """Write a DuckDB project whose selected custom Rules live in the given files.""" + + (project_dir / "sqlbuild_project.toml").write_text( + f'name = "orders"\nadapter = "duckdb"\n\n[rules]\nselect = {json.dumps(selected_rules)}\n' + "\n[rules.thresholds]\nmin_custom_rule_test_cases = 0\n" + configuration, + encoding="utf-8", + ) + orders: Path = project_dir / "models" / "orders.sql" + orders.parent.mkdir(parents=True, exist_ok=True) + orders.write_text('MODEL (description "Orders");\nSELECT 1 AS order_id\n', encoding="utf-8") + relative_path: str + contents: str + for relative_path, contents in files: + path: Path = project_dir / relative_path + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(contents, encoding="utf-8") + + +CUSTOM_RULE_HEADER: str = ( + "import pathlib\n\nfrom sqlbuild.rules import Finding, Model, RuleContext, rule\n" +) + + +def custom_rule_source(*, code: str, body: str, header: str = CUSTOM_RULE_HEADER) -> str: + """Return one model-subject custom Rule module with the given check body.""" + + return ( + f"{header}\n\n" + f'@rule(code="{code}", message="orders rule", remediation="Adjust the orders model.")\n' + "def check(*, model: Model, ctx: RuleContext) -> list[Finding]:\n" + f"{body}" + ) + + +def run_compile_cli( + project_dir: Path, + *, + environment: tuple[tuple[str, str], ...] = (), + working_directory: Path | None = None, +) -> subprocess.CompletedProcess[str]: + """Run the real `sqb compile --json` command from an optional invocation directory.""" + + return subprocess.run( + [ + str(Path(sys.executable).with_name("sqb")), + "--no-color", + "--project-dir", + str(project_dir), + "compile", + "--json", + ], + capture_output=True, + text=True, + check=False, + env={**os.environ, **dict(environment)}, + cwd=working_directory, + ) + + +def custom_rule_diagnostics(result: subprocess.CompletedProcess[str]) -> list[dict[str, Any]]: + """Return custom-rule diagnostics from one JSON compile result.""" + + payload: dict[str, Any] = json.loads(result.stdout) + return list(filter(lambda item: str(item["code"]).startswith("XSQBR"), payload["diagnostics"])) + + +def custom_rule_codes(result: subprocess.CompletedProcess[str]) -> tuple[str, ...]: + """Return custom-rule diagnostic codes from one JSON compile result.""" + + return tuple(str(item["code"]) for item in custom_rule_diagnostics(result)) + + +def custom_rule_paths(result: subprocess.CompletedProcess[str]) -> tuple[str, ...]: + """Return sorted custom-rule diagnostic paths from one JSON compile result.""" + + return tuple(sorted(str(item["path"]) for item in custom_rule_diagnostics(result))) + + +def rule_cache_hits(result: subprocess.CompletedProcess[str]) -> int: + """Return the Rules cache hit count from one JSON compile result.""" + + return int(json.loads(result.stdout)["compile_timings"]["rule_cache_hits"]) + + +def string_set_order(*, names: tuple[str, ...], hash_seed: str) -> str: + """Return the comma-joined iteration order of a string set under one hash seed.""" + + return subprocess.run( + [sys.executable, "-c", f"print(','.join({{*{list(names)!r}}}))"], + capture_output=True, + text=True, + check=True, + env={"PYTHONHASHSEED": hash_seed}, + ).stdout.strip() + + +def custom_rule_messages(result: subprocess.CompletedProcess[str]) -> tuple[str, ...]: + """Return custom-rule diagnostic messages from one JSON compile result.""" + + return tuple(str(item["message"]) for item in custom_rule_diagnostics(result)) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_custom_rule_hermeticity.py b/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_custom_rule_hermeticity.py new file mode 100644 index 0000000000..3b1486680c --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_custom_rule_hermeticity.py @@ -0,0 +1,493 @@ +"""Real CLI coverage of cache-safe custom Rules evaluated by the guarded host.""" + +import subprocess +from pathlib import Path + +import pytest + +from tests.e2e.src.sqlbuild.cli.commands.main.rules._test_types import ( + HashSeedCase, + HelperImportCase, + HermeticRuleCase, + ModuleStatementEditCase, + NonHermeticRuleCase, + ProjectTreeCacheCase, + ScrubbedEnvironmentCase, + WorkingDirectoryCase, +) +from tests.e2e.src.sqlbuild.cli.commands.main.rules.helpers import ( + custom_rule_codes, + custom_rule_diagnostics, + custom_rule_messages, + custom_rule_paths, + custom_rule_source, + rule_cache_hits, + run_compile_cli, + string_set_order, + write_custom_rule_project, +) + +_ENTITY_NAMES: tuple[str, ...] = ( + "customers", + "orders", + "products", + "inventory", + "fulfillment", + "support_tickets", + "shipments", + "returns", +) + + +_RELATIVE_HELPER_RULE: str = custom_rule_source( + code="XSQBRHELP001", + header=( + "from sqlbuild.rules import Finding, Model, RuleContext, rule\n\n" + "from . import order_limits\n" + ), + body=( + " over_limit = len(model.name) > order_limits.MAX_NAME_LENGTH\n" + " return [ctx.finding(subject=model)] * int(over_limit)\n" + ), +) + + +@pytest.mark.parametrize( + "test_case", + [ + HermeticRuleCase( + description="pure rule using re.compile, fnmatch, and json is evaluated", + code="XSQBRPURE001", + files=( + ( + "rules/orders.py", + custom_rule_source( + code="XSQBRPURE001", + header=( + "import fnmatch\nimport json\nimport re\n\n" + "from sqlbuild.rules import Finding, Model, RuleContext, rule\n\n" + '_ORDERS = re.compile(r"^ord")\n' + ), + body=( + ' payload = json.loads(json.dumps({"name": model.name}))\n' + ' matched = fnmatch.fnmatchcase(payload["name"], "*ers")\n' + " if matched and _ORDERS.match(model.name):\n" + " return [ctx.finding(subject=model)]\n" + " return []\n" + ), + ), + ), + ), + expected_rule_codes=("XSQBRPURE001",), + ), + HermeticRuleCase( + description="unrelated rules file importing a disallowed module is ignored", + code="XSQBRNAME001", + files=( + ( + "rules/orders.py", + custom_rule_source( + code="XSQBRNAME001", + body=" return [ctx.finding(subject=model)]\n", + ), + ), + ( + "rules/scratch/test_orders.py", + "import subprocess\n\n\ndef test_orders() -> None:\n" + ' assert subprocess.run(["true"], check=False).returncode == 0\n', + ), + ), + expected_rule_codes=("XSQBRNAME001",), + ), + ], + ids=lambda case: case.description, +) +def test_given_hermetic_rule_when_compiling_then_rule_is_evaluated( + tmp_path: Path, test_case: HermeticRuleCase +) -> None: + write_custom_rule_project( + project_dir=tmp_path, selected_rules=(test_case.code,), files=test_case.files + ) + + result: subprocess.CompletedProcess[str] = run_compile_cli(tmp_path) + + assert result.returncode == 1, result.stdout + result.stderr + assert "non-hermetic" not in result.stderr + assert custom_rule_codes(result) == test_case.expected_rule_codes + + +@pytest.mark.parametrize( + "test_case", + [ + NonHermeticRuleCase( + description="directory listing through an allowed module is rejected", + code="XSQBRLIST001", + source=custom_rule_source( + code="XSQBRLIST001", + body=' return [ctx.finding(subject=model)] * len(pathlib.os.listdir("models"))\n', + ), + expected_line=8, + expected_action="listing a directory", + ), + NonHermeticRuleCase( + description="file opened through an indirect builtin spelling is rejected", + code="XSQBROPEN001", + source=custom_rule_source( + code="XSQBROPEN001", + body=( + ' reader = getattr(pathlib, "__builtins__")["op" + "en"]\n' + ' reader("models/orders.sql").close()\n' + " return []\n" + ), + ), + expected_line=9, + expected_action="opening 'models/orders.sql'", + ), + NonHermeticRuleCase( + description="file read through an aliased method is rejected even when caught", + code="XSQBROPEN002", + source=custom_rule_source( + code="XSQBROPEN002", + body=( + " reader = pathlib.Path.read_text\n" + " try:\n" + ' reader(pathlib.Path("models/orders.sql"))\n' + " except Exception:\n" + " pass\n" + " return []\n" + ), + ), + expected_line=10, + expected_action="opening 'models/orders.sql'", + ), + NonHermeticRuleCase( + description="file existence probe through pathlib is rejected", + code="XSQBRMETA001", + source=custom_rule_source( + code="XSQBRMETA001", + body=( + ' ready = pathlib.Path("target/orders_ready").exists()\n' + " return [ctx.finding(subject=model)] * int(ready)\n" + ), + ), + expected_line=8, + expected_action="reading file metadata for 'target/orders_ready'", + ), + NonHermeticRuleCase( + description="file modification time through pathlib is rejected", + code="XSQBRMETA002", + source=custom_rule_source( + code="XSQBRMETA002", + body=( + ' modified = pathlib.Path("models/orders.sql").stat().st_mtime\n' + " return [ctx.finding(subject=model)] * int(modified < 0)\n" + ), + ), + expected_line=8, + expected_action="reading file metadata for 'models/orders.sql'", + ), + NonHermeticRuleCase( + description="command run through an allowed module is rejected", + code="XSQBRCMD001", + source=custom_rule_source( + code="XSQBRCMD001", + body=' pathlib.os.system("true")\n return []\n', + ), + expected_line=8, + expected_action="running a command", + ), + ], + ids=lambda case: case.description, +) +def test_given_non_hermetic_rule_when_compiling_then_fails_naming_rule_and_location( + tmp_path: Path, test_case: NonHermeticRuleCase +) -> None: + write_custom_rule_project( + project_dir=tmp_path, + selected_rules=(test_case.code,), + files=(("rules/orders.py", test_case.source),), + ) + + result: subprocess.CompletedProcess[str] = run_compile_cli(tmp_path) + + assert result.returncode == 1, result.stdout + result.stderr + expected: str = ( + f"non-hermetic custom rule {test_case.code} at rules/orders.py:{test_case.expected_line}: " + f"{test_case.expected_action} is not allowed" + ) + assert expected in result.stderr, result.stderr + + +@pytest.mark.parametrize( + "test_case", + [ + ScrubbedEnvironmentCase( + description="environment read through an allowed module sees no parent variables", + code="XSQBRENV001", + source=custom_rule_source( + code="XSQBRENV001", + body=( + ' flag = pathlib.os.environ.get("ORDERS_RULE_FLAG")\n' + " return [ctx.finding(subject=model)] * int(bool(flag))\n" + ), + ), + parent_environment=(("ORDERS_RULE_FLAG", "enabled"),), + expected_returncode=0, + expected_rule_codes=(), + ) + ], + ids=lambda case: case.description, +) +def test_given_parent_environment_variable_when_rule_reads_environment_then_host_is_scrubbed( + tmp_path: Path, test_case: ScrubbedEnvironmentCase +) -> None: + write_custom_rule_project( + project_dir=tmp_path, + selected_rules=(test_case.code,), + files=(("rules/orders.py", test_case.source),), + ) + + result: subprocess.CompletedProcess[str] = run_compile_cli( + tmp_path, environment=test_case.parent_environment + ) + + assert result.returncode == test_case.expected_returncode, result.stdout + result.stderr + assert custom_rule_codes(result) == test_case.expected_rule_codes + + +@pytest.mark.parametrize( + "test_case", + [ + HashSeedCase( + description="string set iteration follows the fixed host hash seed", + code="XSQBRSET001", + names=_ENTITY_NAMES, + expected_hash_seed="0", + ) + ], + ids=lambda case: case.description, +) +def test_given_rule_iterating_a_string_set_when_compiling_then_order_uses_fixed_hash_seed( + tmp_path: Path, test_case: HashSeedCase +) -> None: + write_custom_rule_project( + project_dir=tmp_path, + selected_rules=(test_case.code,), + files=( + ( + "rules/orders.py", + custom_rule_source( + code=test_case.code, + body=( + f" names = {{*{list(test_case.names)!r}}}\n" + ' return [ctx.finding(subject=model, message=",".join(names))]\n' + ), + ), + ), + ), + ) + + result: subprocess.CompletedProcess[str] = run_compile_cli(tmp_path) + + assert result.returncode == 1, result.stdout + result.stderr + (finding,) = custom_rule_diagnostics(result) + assert string_set_order(names=test_case.names, hash_seed=test_case.expected_hash_seed) in str( + finding["message"] + ) + + +@pytest.mark.parametrize( + "test_case", + [ + ProjectTreeCacheCase( + description="mediated read and glob work and invalidate cached results", + code="XSQBRTREE001", + source=custom_rule_source( + code="XSQBRTREE001", + body=( + ' policy = ctx.project.tree.read_text("rules/orders_policy.yaml")\n' + ' models = ctx.project.tree.glob("models/*.sql")\n' + ' required = "required: true" in policy and len(models) > 1\n' + " return [ctx.finding(subject=model)] * int(required)\n" + ), + ), + policy_path="rules/orders_policy.yaml", + added_model_path="models/customers.sql", + expected_paths_after_glob_change=("models/customers.sql", "models/orders.sql"), + expected_minimum_cache_hits=1, + ) + ], + ids=lambda case: case.description, +) +def test_given_project_tree_rule_when_inputs_change_then_reads_work_and_cache_invalidates( + tmp_path: Path, test_case: ProjectTreeCacheCase +) -> None: + write_custom_rule_project( + project_dir=tmp_path, + selected_rules=(test_case.code,), + files=( + (test_case.policy_path, "required: true\n"), + ("rules/orders.py", test_case.source), + ), + ) + + single_model: subprocess.CompletedProcess[str] = run_compile_cli(tmp_path) + (tmp_path / test_case.added_model_path).write_text( + 'MODEL (description "Customers");\nSELECT 1 AS customer_id\n', encoding="utf-8" + ) + two_models: subprocess.CompletedProcess[str] = run_compile_cli(tmp_path) + (tmp_path / test_case.policy_path).write_text("required: false\n", encoding="utf-8") + relaxed: subprocess.CompletedProcess[str] = run_compile_cli(tmp_path) + repeated: subprocess.CompletedProcess[str] = run_compile_cli(tmp_path) + + assert single_model.returncode == 0, single_model.stdout + single_model.stderr + assert custom_rule_paths(single_model) == () + assert two_models.returncode == 1, two_models.stdout + two_models.stderr + assert custom_rule_paths(two_models) == test_case.expected_paths_after_glob_change + assert relaxed.returncode == 0, relaxed.stdout + relaxed.stderr + assert custom_rule_paths(relaxed) == () + assert rule_cache_hits(repeated) >= test_case.expected_minimum_cache_hits + + +@pytest.mark.parametrize( + "test_case", + [ + ModuleStatementEditCase( + description="appended module statement invalidates the cached rule result", + code="XSQBRSTMT001", + source=custom_rule_source( + code="XSQBRSTMT001", + header=( + "from sqlbuild.rules import Finding, Model, RuleContext, rule\n\n" + "_NAMES: set[str] = set()\n" + ), + body=" return [ctx.finding(subject=model)] * int(model.name in _NAMES)\n", + ), + appended_statement='\n_NAMES.add("orders")\n', + expected_rule_codes_before_edit=(), + expected_rule_codes_after_edit=("XSQBRSTMT001",), + ) + ], + ids=lambda case: case.description, +) +def test_given_cached_rule_when_module_statement_is_appended_then_rule_is_reevaluated( + tmp_path: Path, test_case: ModuleStatementEditCase +) -> None: + write_custom_rule_project( + project_dir=tmp_path, + selected_rules=(test_case.code,), + files=(("rules/orders.py", test_case.source),), + ) + before: subprocess.CompletedProcess[str] = run_compile_cli(tmp_path) + rule_file: Path = tmp_path / "rules" / "orders.py" + rule_file.write_text(test_case.source + test_case.appended_statement, encoding="utf-8") + + after: subprocess.CompletedProcess[str] = run_compile_cli(tmp_path) + + assert custom_rule_codes(before) == test_case.expected_rule_codes_before_edit + assert custom_rule_codes(after) == test_case.expected_rule_codes_after_edit, after.stderr + + +@pytest.mark.parametrize( + "test_case", + [ + WorkingDirectoryCase( + description="rules observe the project directory wherever sqb is invoked", + code="XSQBRCWD001", + source=custom_rule_source( + code="XSQBRCWD001", + body=" return [ctx.finding(subject=model, message=str(pathlib.Path.cwd()))]\n", + ), + invocation_directories=("invoked/first", "invoked/second"), + expected_working_directory="project", + ) + ], + ids=lambda case: case.description, +) +def test_given_rule_observing_cwd_when_invoked_from_different_directories_then_value_is_fixed( + tmp_path: Path, test_case: WorkingDirectoryCase +) -> None: + project_dir: Path = tmp_path / "project" + project_dir.mkdir() + write_custom_rule_project( + project_dir=project_dir, + selected_rules=(test_case.code,), + files=(("rules/orders.py", test_case.source),), + configuration="\n[rules.cache]\nenabled = false\n", + ) + messages: list[tuple[str, ...]] = [] + for directory in test_case.invocation_directories: + invocation: Path = tmp_path / directory + invocation.mkdir(parents=True) + messages.append( + custom_rule_messages(run_compile_cli(project_dir, working_directory=invocation)) + ) + + expected: str = str((tmp_path / test_case.expected_working_directory).resolve()) + assert messages == [(expected,)] * len(test_case.invocation_directories) + + +@pytest.mark.parametrize( + "test_case", + [ + HelperImportCase( + description="relative helper importing time is rejected statically", + code="XSQBRHELP001", + rule_source=_RELATIVE_HELPER_RULE, + helper_path="rules/order_limits.py", + helper_source="import time\n\nMAX_NAME_LENGTH = 100\n", + expected_error="rules/order_limits.py:1: import 'time' is not allowed", + ) + ], + ids=lambda case: case.description, +) +def test_given_relative_helper_with_disallowed_import_when_compiling_then_fails_import_check( + tmp_path: Path, test_case: HelperImportCase +) -> None: + write_custom_rule_project( + project_dir=tmp_path, + selected_rules=(test_case.code,), + files=( + ("rules/orders.py", test_case.rule_source), + (test_case.helper_path, test_case.helper_source), + ), + ) + + result: subprocess.CompletedProcess[str] = run_compile_cli(tmp_path) + + assert result.returncode == 1, result.stdout + result.stderr + assert "non-hermetic custom rule at" in result.stderr, result.stderr + assert test_case.expected_error in result.stderr, result.stderr + + +@pytest.mark.parametrize( + "test_case", + [ + ModuleStatementEditCase( + description="editing a relatively imported helper invalidates the cached result", + code="XSQBRHELP001", + source=_RELATIVE_HELPER_RULE, + appended_statement="\nMAX_NAME_LENGTH = 3\n", + expected_rule_codes_before_edit=(), + expected_rule_codes_after_edit=("XSQBRHELP001",), + ) + ], + ids=lambda case: case.description, +) +def test_given_cached_rule_when_relative_helper_is_edited_then_rule_is_reevaluated( + tmp_path: Path, test_case: ModuleStatementEditCase +) -> None: + helper_source: str = "MAX_NAME_LENGTH = 100\n" + write_custom_rule_project( + project_dir=tmp_path, + selected_rules=(test_case.code,), + files=(("rules/orders.py", test_case.source), ("rules/order_limits.py", helper_source)), + ) + before: subprocess.CompletedProcess[str] = run_compile_cli(tmp_path) + helper: Path = tmp_path / "rules" / "order_limits.py" + helper.write_text(helper_source + test_case.appended_statement, encoding="utf-8") + + after: subprocess.CompletedProcess[str] = run_compile_cli(tmp_path) + + assert custom_rule_codes(before) == test_case.expected_rule_codes_before_edit, before.stderr + assert custom_rule_codes(after) == test_case.expected_rule_codes_after_edit, after.stderr diff --git a/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/_test_types.py b/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/_test_types.py index b2502f0467..ed35db4454 100644 --- a/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/_test_types.py +++ b/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/_test_types.py @@ -144,3 +144,14 @@ class PolicyLayoutConfigTestCase: source: str expected_levels: tuple[str, ...] expected_thresholds: dict[str, int] + + +@dataclass(frozen=True) +class CustomRuleImportTestCase: + """One static import-allowlist expectation over the fingerprinted rule closure.""" + + description: str + module_import: str + extra_files: tuple[tuple[str, str], ...] = () + expected_rule_codes: tuple[str, ...] = ("XSQBRT101",) + expected_error_pattern: str = "" diff --git a/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/helpers.py b/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/helpers.py index be1c2f5a6e..bb91a7a532 100644 --- a/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/helpers.py +++ b/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/helpers.py @@ -208,3 +208,21 @@ def custom_rule_inputs( cache=RulesCacheConfig(enabled=True), ) return project, config + + +def custom_rules_with_imports( + *, project_dir: Path, module_import: str, extra_files: tuple[tuple[str, str], ...] +) -> tuple[Rule, ...]: + """Write one custom rule with an import line plus extra files and load its catalogue.""" + + _ = write_rule( + root=project_dir, body="del model, ctx\n return []", module_import=module_import + ) + for relative_path, source in extra_files: + path: Path = project_dir / relative_path + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(source, encoding="utf-8") + catalogue: tuple[Rule, ...] = build_catalogue( + config=RulesConfig(select=("XSQBRT101",)), project_dir=project_dir + ) + return tuple(filter(lambda rule: rule.custom, catalogue)) diff --git a/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/test_custom_rules.py b/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/test_custom_rules.py index bae24a0ce8..584fe81a61 100644 --- a/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/test_custom_rules.py +++ b/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/test_custom_rules.py @@ -213,7 +213,10 @@ def test_given_project_file_change_when_repeating_custom_rule_then_dependencies_ CustomRuleTestCase( description="nonhermetic cacheable rule fails", body='open("policy.txt")\n return []', - expected_error_pattern=r"custom.py:\d+: call to open", + expected_error_pattern=( + r"non-hermetic custom rule XSQBRT101 at rules/custom.py:\d+: " + r"opening 'policy.txt' is not allowed" + ), ), CustomRuleTestCase( description="project-wide cacheable rule rejects untracked file types", diff --git a/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/test_hermeticity.py b/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/test_hermeticity.py new file mode 100644 index 0000000000..35ccfe213f --- /dev/null +++ b/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/test_hermeticity.py @@ -0,0 +1,246 @@ +"""Static import verification over the fingerprinted custom-rule closure.""" + +from pathlib import Path + +import pytest + +from sqlbuild.rule_engine._helpers.engine.hermeticity import verify_custom_rules +from sqlbuild.rule_engine.exceptions import NonHermeticRuleError +from sqlbuild.rule_engine.models import Rule +from tests.unit.src.sqlbuild.rule_engine._helpers.engine._test_types import ( + CustomRuleImportTestCase, +) +from tests.unit.src.sqlbuild.rule_engine._helpers.engine.helpers import custom_rules_with_imports + + +@pytest.mark.parametrize( + "test_case", + [ + CustomRuleImportTestCase( + description="pure deterministic stdlib modules are allowed", + module_import=( + "import abc, bisect, copy, difflib, fnmatch, fractions, hashlib, heapq, json\n" + "import operator, string, textwrap, types, unicodedata" + ), + ), + CustomRuleImportTestCase( + description="helper names shared with filesystem calls are not banned", + module_import="from rules.order_paths import exists, glob", + extra_files=( + ( + "rules/order_paths.py", + "import re\n\n\ndef exists(name: str) -> bool:\n" + " return re.compile('^orders').match(name) is not None\n\n\n" + "def glob(names: tuple[str, ...]) -> tuple[str, ...]:\n" + " return tuple(name for name in names if exists(name))\n", + ), + ), + ), + CustomRuleImportTestCase( + description="rules file outside the import closure is not checked", + module_import="", + extra_files=(("rules/scratch/test_orders.py", "import subprocess\nimport time\n"),), + ), + ], + ids=lambda case: case.description, +) +def test_given_closure_with_allowed_imports_when_verifying_then_accepts_rule( + tmp_path: Path, test_case: CustomRuleImportTestCase +) -> None: + rules: tuple[Rule, ...] = custom_rules_with_imports( + project_dir=tmp_path, + module_import=test_case.module_import, + extra_files=test_case.extra_files, + ) + + verify_custom_rules(rules=rules, project_dir=tmp_path) + + assert tuple(rule.code for rule in rules) == test_case.expected_rule_codes + + +@pytest.mark.parametrize( + "test_case", + [ + CustomRuleImportTestCase( + description="time stays excluded", + module_import="import time", + expected_error_pattern=( + r"non-hermetic custom rule at .*rules/custom.py:2: import 'time' is not allowed" + ), + ), + CustomRuleImportTestCase( + description="datetime stays excluded", + module_import="import datetime", + expected_error_pattern=( + r"non-hermetic custom rule at .*rules/custom.py:2: import 'datetime' is not allowed" + ), + ), + CustomRuleImportTestCase( + description="random stays excluded", + module_import="import random", + expected_error_pattern=( + r"non-hermetic custom rule at .*rules/custom.py:2: import 'random' is not allowed" + ), + ), + CustomRuleImportTestCase( + description="uuid stays excluded", + module_import="import uuid", + expected_error_pattern=( + r"non-hermetic custom rule at .*rules/custom.py:2: import 'uuid' is not allowed" + ), + ), + CustomRuleImportTestCase( + description="secrets stays excluded", + module_import="import secrets", + expected_error_pattern=( + r"non-hermetic custom rule at .*rules/custom.py:2: import 'secrets' is not allowed" + ), + ), + CustomRuleImportTestCase( + description="os stays excluded", + module_import="import os", + expected_error_pattern=( + r"non-hermetic custom rule at .*rules/custom.py:2: import 'os' is not allowed" + ), + ), + CustomRuleImportTestCase( + description="sys stays excluded", + module_import="import sys", + expected_error_pattern=( + r"non-hermetic custom rule at .*rules/custom.py:2: import 'sys' is not allowed" + ), + ), + CustomRuleImportTestCase( + description="subprocess stays excluded", + module_import="import subprocess", + expected_error_pattern=( + r"non-hermetic custom rule at .*rules/custom.py:2: import 'subprocess' is not allowed" + ), + ), + CustomRuleImportTestCase( + description="socket stays excluded", + module_import="import socket", + expected_error_pattern=( + r"non-hermetic custom rule at .*rules/custom.py:2: import 'socket' is not allowed" + ), + ), + CustomRuleImportTestCase( + description="importlib stays excluded", + module_import="import importlib", + expected_error_pattern=( + r"non-hermetic custom rule at .*rules/custom.py:2: import 'importlib' is not allowed" + ), + ), + CustomRuleImportTestCase( + description="second name of a relative multi-name import is checked", + module_import="from . import order_names, order_clock", + extra_files=( + ("rules/order_names.py", "NAMES = ('orders',)\n"), + ("rules/order_clock.py", "import time\n"), + ), + expected_error_pattern=r"non-hermetic custom rule at .*rules/order_clock.py:1: import 'time' is not allowed", + ), + CustomRuleImportTestCase( + description="relative submodule of a helper package is checked", + module_import="from .order_pkg import clock", + extra_files=( + ("rules/order_pkg/__init__.py", ""), + ("rules/order_pkg/clock.py", "import time\n"), + ), + expected_error_pattern=r"non-hermetic custom rule at .*rules/order_pkg/clock.py:1: import 'time' is not allowed", + ), + CustomRuleImportTestCase( + description="helper package initializer is checked", + module_import="from .order_pkg import clock", + extra_files=( + ("rules/order_pkg/__init__.py", "import random\n"), + ("rules/order_pkg/clock.py", "VALUE = 1\n"), + ), + expected_error_pattern=r"non-hermetic custom rule at .*rules/order_pkg/__init__.py:1: import 'random' is not allowed", + ), + CustomRuleImportTestCase( + description="second absolute module of a multi-name import is checked", + module_import="import rules.order_names, rules.order_clock", + extra_files=( + ("rules/order_names.py", "NAMES = ('orders',)\n"), + ("rules/order_clock.py", "import time\n"), + ), + expected_error_pattern=r"non-hermetic custom rule at .*rules/order_clock.py:1: import 'time' is not allowed", + ), + CustomRuleImportTestCase( + description="helper package takes precedence over a same-named module", + module_import="from . import order_clock", + extra_files=( + ("rules/order_clock.py", "VALUE = 1\n"), + ("rules/order_clock/__init__.py", "import time\n\nVALUE = time.time()\n"), + ), + expected_error_pattern=r"non-hermetic custom rule at .*rules/order_clock/__init__.py:1: import 'time' is not allowed", + ), + CustomRuleImportTestCase( + description="function-local helper import is checked", + module_import="from rules.order_names import names", + extra_files=( + ( + "rules/order_names.py", + "def names() -> tuple[str, ...]:\n from rules import order_clock\n\n return order_clock.NAMES\n", + ), + ("rules/order_clock.py", "import uuid\n\nNAMES = ('orders',)\n"), + ), + expected_error_pattern=r"non-hermetic custom rule at .*rules/order_clock.py:1: import 'uuid' is not allowed", + ), + CustomRuleImportTestCase( + description="imported helper in the closure is checked", + module_import="from rules.order_clock import today", + extra_files=( + ( + "rules/order_clock.py", + "def today() -> str:\n import datetime\n\n" + " return datetime.date.today().isoformat()\n", + ), + ), + expected_error_pattern=( + r"non-hermetic custom rule at .*rules/order_clock.py:2: " + r"import 'datetime' is not allowed" + ), + ), + ], + ids=lambda case: case.description, +) +def test_given_closure_with_disallowed_import_when_verifying_then_raises_location( + tmp_path: Path, test_case: CustomRuleImportTestCase +) -> None: + rules: tuple[Rule, ...] = custom_rules_with_imports( + project_dir=tmp_path, + module_import=test_case.module_import, + extra_files=test_case.extra_files, + ) + + with pytest.raises(NonHermeticRuleError, match=test_case.expected_error_pattern): + verify_custom_rules(rules=rules, project_dir=tmp_path) + + +@pytest.mark.parametrize( + "test_case", + [ + CustomRuleImportTestCase( + description="package initializer symlinked outside rules is not part of the closure", + module_import="import re", + extra_files=(("shared/__init__.py", "import time\n"),), + ), + ], + ids=lambda case: case.description, +) +def test_given_rules_package_initializer_outside_rules_when_verifying_then_accepts_rule( + tmp_path: Path, test_case: CustomRuleImportTestCase +) -> None: + (tmp_path / "rules").mkdir() + (tmp_path / "rules" / "__init__.py").symlink_to(tmp_path / "shared" / "__init__.py") + rules: tuple[Rule, ...] = custom_rules_with_imports( + project_dir=tmp_path, + module_import=test_case.module_import, + extra_files=test_case.extra_files, + ) + + verify_custom_rules(rules=rules, project_dir=tmp_path) + + assert tuple(rule.code for rule in rules) == test_case.expected_rule_codes diff --git a/tests/unit/src/sqlbuild/rule_engine/main/evaluate_rule/_test_types.py b/tests/unit/src/sqlbuild/rule_engine/main/evaluate_rule/_test_types.py index ef506343e2..2b384ced2d 100644 --- a/tests/unit/src/sqlbuild/rule_engine/main/evaluate_rule/_test_types.py +++ b/tests/unit/src/sqlbuild/rule_engine/main/evaluate_rule/_test_types.py @@ -18,3 +18,10 @@ class EvaluateRuleParityTestCase: description: str rule_case: RuleCase expected_finding_count: int + + +@dataclass(frozen=True) +class EvaluateRuleDeterminismTestCase: + description: str + rule_case: RuleCase + expected_error_pattern: str diff --git a/tests/unit/src/sqlbuild/rule_engine/main/evaluate_rule/helpers.py b/tests/unit/src/sqlbuild/rule_engine/main/evaluate_rule/helpers.py index 22b7f568c1..7a650c16b8 100644 --- a/tests/unit/src/sqlbuild/rule_engine/main/evaluate_rule/helpers.py +++ b/tests/unit/src/sqlbuild/rule_engine/main/evaluate_rule/helpers.py @@ -2,6 +2,8 @@ from sqlbuild.rules import Finding, Model, RuleContext, RuleOption, rule +_EVALUATED_MODELS: set[str] = set() + REQUIRED_DOMAIN: RuleOption[str] = RuleOption.string( name="required_domain", default="commerce", @@ -19,3 +21,15 @@ def required_domain(*, model: Model, ctx: RuleContext) -> list[Finding]: matches: bool = model.name.startswith(f"{ctx.option(REQUIRED_DOMAIN)}__") return [ctx.finding(subject=model)] * int(not matches) + + +@rule( + code="XSQBRS001", + slug="first-evaluation-only", + message="model was already evaluated by this process", + remediation="Derive findings from the subject and context, not module-level state.", +) +def first_evaluation_only(*, model: Model, ctx: RuleContext) -> list[Finding]: + repeated: bool = model.name in _EVALUATED_MODELS + _EVALUATED_MODELS.add(model.name) + return [ctx.finding(subject=model)] * int(repeated) diff --git a/tests/unit/src/sqlbuild/rule_engine/main/evaluate_rule/test_evaluate_rule.py b/tests/unit/src/sqlbuild/rule_engine/main/evaluate_rule/test_evaluate_rule.py index e7aa2b31d3..6530d6355b 100644 --- a/tests/unit/src/sqlbuild/rule_engine/main/evaluate_rule/test_evaluate_rule.py +++ b/tests/unit/src/sqlbuild/rule_engine/main/evaluate_rule/test_evaluate_rule.py @@ -8,16 +8,21 @@ from sqlbuild.compiler.compile.models import CompiledProject from sqlbuild.rule_engine.constants import MIN_CUSTOM_RULE_TEST_CASES +from sqlbuild.rule_engine.exceptions import RulesError from sqlbuild.rule_engine.main._evaluate import evaluate from sqlbuild.rule_engine.models import RulesCacheConfig, RulesConfig, RulesResult from sqlbuild.rule_engine.types import RuleOptionValue from sqlbuild.rules.testing import RuleCase, RuleResult, evaluate_rule from tests.unit.src.sqlbuild.rule_engine.main.evaluate.helpers import build_project from tests.unit.src.sqlbuild.rule_engine.main.evaluate_rule._test_types import ( + EvaluateRuleDeterminismTestCase, EvaluateRuleParityTestCase, EvaluateRuleTestCase, ) -from tests.unit.src.sqlbuild.rule_engine.main.evaluate_rule.helpers import required_domain +from tests.unit.src.sqlbuild.rule_engine.main.evaluate_rule.helpers import ( + first_evaluation_only, + required_domain, +) @pytest.mark.parametrize( @@ -115,3 +120,29 @@ def test_given_same_custom_rule_when_using_public_and_native_paths_then_faults_h assert native_result.findings == public_result.findings assert repeated_native_result.findings == native_result.findings assert native_result.findings[0] == public_result.findings[0] + + +@pytest.mark.parametrize( + "test_case", + [ + EvaluateRuleDeterminismTestCase( + description="module-level state leaking between evaluations fails the harness", + rule_case=RuleCase( + description="first evaluation reports nothing", + source="MODEL (materialized table);\n\nSELECT 1 AS id\n", + path="models/mart/commerce__mart__orders.sql", + expected_finding_count=0, + ), + expected_error_pattern=( + r"custom rule XSQBRS001 is not deterministic: evaluating every subject again " + r"after the others changed its findings from 0 to 1" + ), + ) + ], + ids=lambda case: case.description, +) +def test_given_rule_with_module_state_when_evaluating_case_then_determinism_check_fails( + test_case: EvaluateRuleDeterminismTestCase, +) -> None: + with pytest.raises(RulesError, match=test_case.expected_error_pattern): + evaluate_rule(rule=first_evaluation_only, test_case=test_case.rule_case) diff --git a/website/src/content/docs/docs/concepts/rules/custom-rules/testing-and-determinism.mdx b/website/src/content/docs/docs/concepts/rules/custom-rules/testing-and-determinism.mdx index 9198c43b4a..aa6facc605 100644 --- a/website/src/content/docs/docs/concepts/rules/custom-rules/testing-and-determinism.mdx +++ b/website/src/content/docs/docs/concepts/rules/custom-rules/testing-and-determinism.mdx @@ -29,11 +29,31 @@ def test_given_missing_order_id_when_evaluating_then_reports_finding() -> None: `RuleCase.files` adds supporting project files. `RuleCase.config` supplies option values. Include passing cases, failing cases, near misses, and exact source-position assertions. +Each case also checks determinism. The harness evaluates every subject once, then evaluates every +subject again in reverse order in the same process. If the Rule's findings differ between the two +passes, the case fails with `custom rule is not deterministic`. The usual cause is +module-level state, such as a set of already-seen models, that carries over from one subject to +the next. Compute findings from the subject and `ctx` alone. + Repository pytest files remain outside the SQLBuild project's SQL `tests/` directory. ## Helpers -Custom Rules can import repository-owned helpers under `rules/` and supported pure Python modules. +Custom Rules can import repository-owned helpers under `rules/` and these deterministic standard +library modules: + +`abc`, `bisect`, `collections`, `copy`, `dataclasses`, `decimal`, `difflib`, `enum`, `fnmatch`, +`fractions`, `functools`, `graphlib`, `hashlib`, `heapq`, `itertools`, `json`, `math`, +`operator`, `pathlib`, `re`, `statistics`, `string`, `textwrap`, `types`, `typing`, and +`unicodedata`, plus `__future__` and `sqlbuild.rules`. + +Modules that expose the clock, randomness, the process, or the network are rejected, including +`time`, `datetime`, `random`, `uuid`, `secrets`, `os`, `sys`, `subprocess`, `socket`, and +`importlib`. SQLBuild checks the `import` statements in each selected Rule's file and in every +helper under `rules/` it imports, directly or through other helpers, with absolute or relative +imports anywhere in the module. These are the files the Rules cache fingerprints. Other Python files under +`rules/`, such as tests or scratch scripts, are not checked. + Changing an imported helper invalidates the Rules that depend on it. Only decorated functions register. Ordinary functions, constants, dataclasses, and classes remain @@ -41,8 +61,26 @@ helpers. ## Deterministic inputs -Direct filesystem, environment, clock, randomness, network, and subprocess access is rejected. -Those inputs cannot be reproduced safely by the Rules cache. +The Rules cache can only reuse a result when the Rule depends on tracked inputs. SQLBuild runs +custom Rules in a separate host process and enforces that at runtime: + +- Rules run with an empty environment, so `os.environ` is empty even when it is reached through + an allowed module, for example `pathlib.os.environ`. +- The host starts with `PYTHONHASHSEED=0`, so iterating over a set of strings gives the same order + on every run. +- The working directory is the resolved project directory, wherever `sqb` was invoked, so + `Path.cwd()` and relative paths do not vary between runs. +- While Rule code runs, opening files, reading file metadata such as `Path.exists()` or + `Path.stat()`, listing directories, running commands, forking, network access, and `ctypes` are + rejected, however they are reached. Loading a module outside the + allowed list for the first time is rejected too. + +A rejected operation fails the command with `non-hermetic custom rule at :`, even +if the Rule catches the exception. + +The guard exists for cache correctness: it stops Rules from depending on inputs the cache cannot +track by accident. It is not a security sandbox. Deliberately evasive code can bypass it, so only +run Rules you trust. Read supported project text and structure through `ctx.project.tree`: @@ -56,7 +94,8 @@ path invalidates the cached result. ## Cache granularity -Cache identity incorporates the Rule implementation, imported helper closure, configured options, -subject, accessed compiler facts, tracked project observations, dialect, and compatibility -versions. Prefer model subjects for independent per-model checks and project subjects for genuine +Cache identity incorporates the whole Rule source file, imported helper closure, configured +options, subject, accessed compiler facts, tracked project observations, dialect, and +compatibility versions. Editing any statement in a Rule file invalidates cached results for every +Rule defined in that file, so keep unrelated Rules in separate files. Prefer model subjects for independent per-model checks and project subjects for genuine cross-project invariants.