From dfc434b2c2f795dacc08b6ed1c3ed6e4bfe8e7e6 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Mon, 28 Sep 2026 14:27:31 +0100 Subject: [PATCH 1/4] feat: enforce custom rule hermeticity at runtime Replace the call-name ban with a runtime audit-hook guard in the custom rule host, launch the host with a scrubbed environment and a fixed hash seed, widen the import allowlist to pure deterministic stdlib modules, limit the static import check to the fingerprinted import closure, and make the public rule harness detect state leaking between evaluations. --- scripts/rules_benchmark/_helpers/workflow.py | 17 +- scripts/rules_benchmark/constants.py | 2 +- .../custom-rules/testing-and-determinism.md | 37 ++- .../_helpers/engine/hermeticity.py | 107 ++---- .../rule_engine/_helpers/engine/native.py | 8 +- .../_helpers/engine/rule_harness.py | 1 + .../rule_engine/_helpers/engine/ruleset.py | 8 +- .../{engine => host}/custom_evaluation.py | 109 ++++++- .../_helpers/{engine => host}/custom_host.py | 22 +- .../_helpers/host/custom_host_launch.py | 32 ++ .../rule_engine/classes/runtime_guard.py | 154 +++++++++ src/sqlbuild/rule_engine/constants.py | 47 +++ src/sqlbuild/rule_engine/exceptions.py | 4 + src/sqlbuild/rule_engine/main/_evaluate.py | 2 + .../cli/commands/main/rules/_test_types.py | 46 +++ .../cli/commands/main/rules/helpers.py | 97 ++++++ .../rules/test_custom_rule_hermeticity.py | 306 ++++++++++++++++++ .../_helpers/engine/_test_types.py | 11 + .../rule_engine/_helpers/engine/helpers.py | 18 ++ .../_helpers/engine/test_custom_rules.py | 5 +- .../_helpers/engine/test_hermeticity.py | 162 ++++++++++ .../main/evaluate_rule/_test_types.py | 7 + .../rule_engine/main/evaluate_rule/helpers.py | 14 + .../main/evaluate_rule/test_evaluate_rule.py | 33 +- .../custom-rules/testing-and-determinism.mdx | 37 ++- 25 files changed, 1168 insertions(+), 118 deletions(-) rename src/sqlbuild/rule_engine/_helpers/{engine => host}/custom_evaluation.py (52%) rename src/sqlbuild/rule_engine/_helpers/{engine => host}/custom_host.py (83%) create mode 100644 src/sqlbuild/rule_engine/_helpers/host/custom_host_launch.py create mode 100644 src/sqlbuild/rule_engine/classes/runtime_guard.py create mode 100644 tests/e2e/src/sqlbuild/cli/commands/main/rules/test_custom_rule_hermeticity.py create mode 100644 tests/unit/src/sqlbuild/rule_engine/_helpers/engine/test_hermeticity.py 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..0fbc3b497f 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,30 @@ 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 the +helpers it imports, which 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 +62,20 @@ 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. +- While Rule code runs, opening files, 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. These checks keep cached results correct. They are not a +security sandbox. Read supported project text and structure through `ctx.project.tree`: 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 83% rename from src/sqlbuild/rule_engine/_helpers/engine/custom_host.py rename to src/sqlbuild/rule_engine/_helpers/host/custom_host.py index 988ce2e70c..c46acadd32 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,13 @@ def main() -> int: if isinstance(raw_model_paths, list) else None ) + os.environ.clear() + guard = RuntimeGuard(project_dir=project_dir) + sys.addaudithook(guard) 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 +74,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..cedd037242 --- /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_INHERITED_ENVIRONMENT, + CUSTOM_HOST_MODULE, +) + + +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_INHERITED_ENVIRONMENT if name in os.environ + } + environment["PYTHONHASHSEED"] = CUSTOM_HOST_HASH_SEED + environment["PYTHONUTF8"] = "1" + arguments: list[str] = [sys.executable, "-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..a89a4ebc0e --- /dev/null +++ b/src/sqlbuild/rule_engine/classes/runtime_guard.py @@ -0,0 +1,154 @@ +"""Runtime hermeticity audit hook for the custom-rule host.""" + +from __future__ import annotations + +import importlib +import os +import sys +from collections.abc import Iterator +from contextlib import contextmanager +from pathlib import Path +from types import CodeType, FrameType + +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: frozenset[CodeType] = frozenset({ProjectTree.read_text.__code__}) + self._rule_files: dict[str, bool] = {} + 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)) + + @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.""" + + while frame is not None: + code: CodeType = frame.f_code + if self._is_rule_file(code.co_filename): + return frame + if code in self._mediated_codes 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 _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..3dbadf870b 100644 --- a/src/sqlbuild/rule_engine/constants.py +++ b/src/sqlbuild/rule_engine/constants.py @@ -10,6 +10,53 @@ 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" +# Interpreter start-up only: the host clears os.environ before it loads any Rule. +CUSTOM_HOST_INHERITED_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..5bcab4ede4 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,49 @@ 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 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..040b768cd4 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,98 @@ 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], ...] +) -> 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", + 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], ...] = () +) -> subprocess.CompletedProcess[str]: + """Run the real `sqb compile --json` command with extra parent environment variables.""" + + 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)}, + ) + + +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() 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..12b5b4dd67 --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_custom_rule_hermeticity.py @@ -0,0 +1,306 @@ +"""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, + HermeticRuleCase, + NonHermeticRuleCase, + ProjectTreeCacheCase, + ScrubbedEnvironmentCase, +) +from tests.e2e.src.sqlbuild.cli.commands.main.rules.helpers import ( + custom_rule_codes, + custom_rule_diagnostics, + 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", +) + + +@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="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 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..69821133f1 --- /dev/null +++ b/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/test_hermeticity.py @@ -0,0 +1,162 @@ +"""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="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) 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..6b416868e0 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,30 @@ 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 the +helpers it imports, which 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 +60,20 @@ 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. +- While Rule code runs, opening files, 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. These checks keep cached results correct. They are not a +security sandbox. Read supported project text and structure through `ctx.project.tree`: From 11872fd7bc2d8d196ee39b2757e82dcf08eecf89 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Mon, 28 Sep 2026 14:38:35 +0100 Subject: [PATCH 2/4] fix: fingerprint whole custom rule files and pin host working directory Hash each Rule's full source file plus its import closure so any module statement edit invalidates cached results, pin the custom host's working directory to the resolved project directory and drop the invocation directory from its import path, and state in the docs that the guard is for cache correctness rather than a sandbox. --- .../custom-rules/testing-and-determinism.md | 16 ++-- .../_helpers/engine/custom_rule_evidence.py | 72 ++--------------- .../rule_engine/_helpers/host/custom_host.py | 1 + .../_helpers/host/custom_host_launch.py | 6 +- src/sqlbuild/rule_engine/constants.py | 3 +- .../cli/commands/main/rules/_test_types.py | 19 +++++ .../cli/commands/main/rules/helpers.py | 22 ++++- .../rules/test_custom_rule_hermeticity.py | 81 +++++++++++++++++++ .../custom-rules/testing-and-determinism.mdx | 16 ++-- 9 files changed, 152 insertions(+), 84 deletions(-) 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 0fbc3b497f..45966755e0 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 @@ -69,13 +69,18 @@ custom Rules in a separate host process and enforces that at runtime: 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, 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. These checks keep cached results correct. They are not a -security sandbox. +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`: @@ -89,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. 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..9befd97724 100644 --- a/src/sqlbuild/rule_engine/_helpers/engine/custom_rule_evidence.py +++ b/src/sqlbuild/rule_engine/_helpers/engine/custom_rule_evidence.py @@ -31,26 +31,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 +67,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( diff --git a/src/sqlbuild/rule_engine/_helpers/host/custom_host.py b/src/sqlbuild/rule_engine/_helpers/host/custom_host.py index c46acadd32..3cedf90003 100644 --- a/src/sqlbuild/rule_engine/_helpers/host/custom_host.py +++ b/src/sqlbuild/rule_engine/_helpers/host/custom_host.py @@ -59,6 +59,7 @@ def main() -> int: else None ) os.environ.clear() + os.chdir(project_dir) guard = RuntimeGuard(project_dir=project_dir) sys.addaudithook(guard) messages: io.StringIO = io.StringIO() diff --git a/src/sqlbuild/rule_engine/_helpers/host/custom_host_launch.py b/src/sqlbuild/rule_engine/_helpers/host/custom_host_launch.py index cedd037242..f9c1a049c6 100644 --- a/src/sqlbuild/rule_engine/_helpers/host/custom_host_launch.py +++ b/src/sqlbuild/rule_engine/_helpers/host/custom_host_launch.py @@ -9,8 +9,8 @@ from sqlbuild.rule_engine.constants import ( CUSTOM_HOST_EXEC_OS_NAME, CUSTOM_HOST_HASH_SEED, - CUSTOM_HOST_INHERITED_ENVIRONMENT, CUSTOM_HOST_MODULE, + CUSTOM_HOST_STARTUP_ENVIRONMENT, ) @@ -18,11 +18,11 @@ 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_INHERITED_ENVIRONMENT if name in os.environ + 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, "-m", CUSTOM_HOST_MODULE] + 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 diff --git a/src/sqlbuild/rule_engine/constants.py b/src/sqlbuild/rule_engine/constants.py index 3dbadf870b..ba04b7294c 100644 --- a/src/sqlbuild/rule_engine/constants.py +++ b/src/sqlbuild/rule_engine/constants.py @@ -14,8 +14,7 @@ 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" -# Interpreter start-up only: the host clears os.environ before it loads any Rule. -CUSTOM_HOST_INHERITED_ENVIRONMENT: tuple[str, ...] = ( +CUSTOM_HOST_STARTUP_ENVIRONMENT: tuple[str, ...] = ( "APPDATA", "HOME", "PYTHONHOME", 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 5bcab4ede4..d0e707d2a1 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 @@ -101,3 +101,22 @@ class ProjectTreeCacheCase: 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 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 040b768cd4..659ca60fa4 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/rules/helpers.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/rules/helpers.py @@ -59,13 +59,17 @@ def run_unevaluated_rules_cli(project_dir: Path, *args: str) -> subprocess.Compl def write_custom_rule_project( - *, project_dir: Path, selected_rules: tuple[str, ...], files: tuple[tuple[str, str], ...] + *, + 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", + "\n[rules.thresholds]\nmin_custom_rule_test_cases = 0\n" + configuration, encoding="utf-8", ) orders: Path = project_dir / "models" / "orders.sql" @@ -96,9 +100,12 @@ def custom_rule_source(*, code: str, body: str, header: str = CUSTOM_RULE_HEADER def run_compile_cli( - project_dir: Path, *, environment: tuple[tuple[str, str], ...] = () + project_dir: Path, + *, + environment: tuple[tuple[str, str], ...] = (), + working_directory: Path | None = None, ) -> subprocess.CompletedProcess[str]: - """Run the real `sqb compile --json` command with extra parent environment variables.""" + """Run the real `sqb compile --json` command from an optional invocation directory.""" return subprocess.run( [ @@ -113,6 +120,7 @@ def run_compile_cli( text=True, check=False, env={**os.environ, **dict(environment)}, + cwd=working_directory, ) @@ -151,3 +159,9 @@ def string_set_order(*, names: tuple[str, ...], hash_seed: str) -> str: 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 index 12b5b4dd67..98f0425967 100644 --- 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 @@ -8,13 +8,16 @@ from tests.e2e.src.sqlbuild.cli.commands.main.rules._test_types import ( HashSeedCase, 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, @@ -304,3 +307,81 @@ def test_given_project_tree_rule_when_inputs_change_then_reads_work_and_cache_in 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) 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 6b416868e0..0d7a3bc7b2 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 @@ -67,13 +67,18 @@ custom Rules in a separate host process and enforces that at runtime: 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, 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. These checks keep cached results correct. They are not a -security sandbox. +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`: @@ -87,7 +92,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. From 8b6562e524eed7083646f8827c84ee44d933d23e Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Mon, 28 Sep 2026 15:01:04 +0100 Subject: [PATCH 3/4] fix: guard filesystem metadata and complete custom rule import closure Route unaudited os and os.path metadata calls through the runtime guard so Rule code probing file existence or stat results is rejected with the non-hermetic error, and resolve every repository-owned rules module a Rule can execute (relative and multi-name imports, package initializers, and function-local imports) for both the static import check and the implementation fingerprint. --- .../custom-rules/testing-and-determinism.md | 10 +- .../_helpers/engine/custom_rule_evidence.py | 93 +++++++++------ .../rule_engine/_helpers/host/custom_host.py | 1 + .../rule_engine/classes/runtime_guard.py | 97 +++++++++++++++- .../cli/commands/main/rules/_test_types.py | 10 ++ .../rules/test_custom_rule_hermeticity.py | 106 ++++++++++++++++++ .../_helpers/engine/test_hermeticity.py | 48 ++++++++ .../custom-rules/testing-and-determinism.mdx | 10 +- 8 files changed, 326 insertions(+), 49 deletions(-) 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 45966755e0..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 @@ -51,8 +51,9 @@ library modules: 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 the -helpers it imports, which are the files the Rules cache fingerprints. Other Python files under +`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. @@ -71,8 +72,9 @@ custom Rules in a separate host process and enforces that at runtime: 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, listing directories, running commands, forking, network - access, and `ctypes` are rejected, however they are reached. Loading a module outside the +- 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 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 9befd97724..28b7750352 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" @@ -124,62 +125,82 @@ 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.with_suffix(".py") if parts else None, candidate / "__init__.py"): + 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 init.is_file() and init.resolve() != path: + inits.append(init.resolve()) + return tuple(inits) def _file_evidence( diff --git a/src/sqlbuild/rule_engine/_helpers/host/custom_host.py b/src/sqlbuild/rule_engine/_helpers/host/custom_host.py index 3cedf90003..89b37e8877 100644 --- a/src/sqlbuild/rule_engine/_helpers/host/custom_host.py +++ b/src/sqlbuild/rule_engine/_helpers/host/custom_host.py @@ -62,6 +62,7 @@ def main() -> int: 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) diff --git a/src/sqlbuild/rule_engine/classes/runtime_guard.py b/src/sqlbuild/rule_engine/classes/runtime_guard.py index a89a4ebc0e..847fc4b1a2 100644 --- a/src/sqlbuild/rule_engine/classes/runtime_guard.py +++ b/src/sqlbuild/rule_engine/classes/runtime_guard.py @@ -2,13 +2,15 @@ from __future__ import annotations +import functools import importlib import os import sys -from collections.abc import Iterator +import threading +from collections.abc import Callable, Iterator from contextlib import contextmanager from pathlib import Path -from types import CodeType, FrameType +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 @@ -41,6 +43,30 @@ ) _GUARDED_EVENTS: frozenset[str] = frozenset({_IMPORT_EVENT, *_FILESYSTEM_EVENTS, *_PROCESS_EVENTS}) _GUARDED_PREFIXES: tuple[str, ...] = tuple(prefix for prefix, _ in _PREFIX_EVENTS) +_METADATA_ACTION: str = "reading file metadata" +_OS_METADATA_FUNCTIONS: tuple[str, ...] = ("access", "lstat", "readlink", "stat", "statvfs") +_PATH_METADATA_FUNCTIONS: tuple[str, ...] = ( + "exists", + "getatime", + "getctime", + "getmtime", + "getsize", + "isdevdrive", + "isdir", + "isfile", + "isjunction", + "islink", + "ismount", + "lexists", + "realpath", + "samefile", +) +_OS_CAPABILITY_SETS: tuple[str, ...] = ( + "supports_dir_fd", + "supports_effective_ids", + "supports_fd", + "supports_follow_symlinks", +) _REMEDIATION: str = "read project inputs through ctx.project.tree and compiler facts" @@ -51,8 +77,10 @@ def __init__(self, *, project_dir: Path) -> 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: frozenset[CodeType] = frozenset({ProjectTree.read_text.__code__}) + 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 @@ -71,6 +99,47 @@ def __call__(self, event: str, arguments: tuple[object, ...]) -> None: 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.""" @@ -110,11 +179,16 @@ def _reject(self, *, rule_frame: FrameType, action: str) -> None: 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 - if self._is_rule_file(code.co_filename): + 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 code in self._mediated_codes or ( + if id(code) in mediated or ( code.co_name == _IMPORT_ENTRY_FUNCTION and self._is_import_machinery(code.co_filename) ): @@ -140,6 +214,19 @@ 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}" 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 d0e707d2a1..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 @@ -120,3 +120,13 @@ class WorkingDirectoryCase: 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/test_custom_rule_hermeticity.py b/tests/e2e/src/sqlbuild/cli/commands/main/rules/test_custom_rule_hermeticity.py index 98f0425967..3b1486680c 100644 --- 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 @@ -7,6 +7,7 @@ from tests.e2e.src.sqlbuild.cli.commands.main.rules._test_types import ( HashSeedCase, + HelperImportCase, HermeticRuleCase, ModuleStatementEditCase, NonHermeticRuleCase, @@ -38,6 +39,19 @@ ) +_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", [ @@ -146,6 +160,32 @@ def test_given_hermetic_rule_when_compiling_then_rule_is_evaluated( 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", @@ -385,3 +425,69 @@ def test_given_rule_observing_cwd_when_invoked_from_different_directories_then_v 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_hermeticity.py b/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/test_hermeticity.py index 69821133f1..1fdb57f4cd 100644 --- a/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/test_hermeticity.py +++ b/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/test_hermeticity.py @@ -131,6 +131,54 @@ def test_given_closure_with_allowed_imports_when_verifying_then_accepts_rule( 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="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", 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 0d7a3bc7b2..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 @@ -49,8 +49,9 @@ library modules: 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 the -helpers it imports, which are the files the Rules cache fingerprints. Other Python files under +`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. @@ -69,8 +70,9 @@ custom Rules in a separate host process and enforces that at runtime: 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, listing directories, running commands, forking, network - access, and `ctypes` are rejected, however they are reached. Loading a module outside the +- 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 From a1e09cfff308861dc6137857ad586e16943a5e23 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Mon, 28 Sep 2026 15:07:53 +0100 Subject: [PATCH 4/4] fix: resolve helper packages before same-named modules and keep closure inside rules --- .../_helpers/engine/custom_rule_evidence.py | 9 +++-- .../_helpers/engine/test_hermeticity.py | 36 +++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) 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 28b7750352..d049e0ee9d 100644 --- a/src/sqlbuild/rule_engine/_helpers/engine/custom_rule_evidence.py +++ b/src/sqlbuild/rule_engine/_helpers/engine/custom_rule_evidence.py @@ -183,7 +183,7 @@ def _local_import_paths(*, node: ast.AST, source_path: Path, rules_root: Path) - def _module_file(*, base: Path, parts: tuple[str, ...], rules_root: Path) -> Path | None: candidate: Path = base.joinpath(*parts) - for path in (candidate.with_suffix(".py") if parts else None, candidate / "__init__.py"): + 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 @@ -198,8 +198,11 @@ def _package_inits(*, path: Path, rules_root: Path) -> tuple[Path, ...]: if not parent.is_relative_to(rules_root): break init: Path = parent / "__init__.py" - if init.is_file() and init.resolve() != path: - inits.append(init.resolve()) + 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) 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 index 1fdb57f4cd..35ccfe213f 100644 --- a/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/test_hermeticity.py +++ b/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/test_hermeticity.py @@ -167,6 +167,15 @@ def test_given_closure_with_allowed_imports_when_verifying_then_accepts_rule( ), 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", @@ -208,3 +217,30 @@ def test_given_closure_with_disallowed_import_when_verifying_then_raises_locatio 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