This repository was archived by the owner on May 7, 2026. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 199
feat(wren): add strict query mode for SQL policy enforcement #1500
Merged
goldmedal
merged 4 commits into
Canner:feat/cli-0.2.0
from
goldmedal:claude/elated-almeida
Apr 4, 2026
Merged
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
cfb1432
feat(wren): add strict query mode for SQL policy enforcement
goldmedal 636d360
fix(wren): address code review — type validation, CTE scope, TVF bloc…
goldmedal 0d479da
fix(wren): expanduser for WREN_HOME and fail closed on denied_functions
goldmedal f7d03e0
fix(wren): catch OSError alongside WrenError in CLI config loading
goldmedal File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| """Wren CLI configuration loaded from ~/.wren/config.json.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import json | ||
| from dataclasses import dataclass, field | ||
| from pathlib import Path | ||
|
|
||
| from wren.model.error import ErrorCode, WrenError | ||
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class WrenConfig: | ||
| """Immutable configuration for the Wren CLI. | ||
|
|
||
| Attributes | ||
| ---------- | ||
| strict_mode: | ||
| When ``True``, all table references in SQL must be defined in the MDL | ||
| manifest. Queries referencing non-MDL tables are rejected. | ||
| denied_functions: | ||
| Set of function names (lowercase) that are forbidden in SQL queries. | ||
| Matching is case-insensitive. | ||
| """ | ||
|
|
||
| strict_mode: bool = False | ||
| denied_functions: frozenset[str] = field(default_factory=frozenset) | ||
|
|
||
|
|
||
| def load_config(wren_home: Path) -> WrenConfig: | ||
| """Load configuration from ``wren_home/config.json``. | ||
|
|
||
| Returns default ``WrenConfig`` when the file does not exist. | ||
| Raises ``WrenError`` when the file exists but contains invalid JSON. | ||
| """ | ||
| config_path = wren_home / "config.json" | ||
| if not config_path.exists(): | ||
| return WrenConfig() | ||
|
|
||
| try: | ||
| raw = json.loads(config_path.read_text()) | ||
| except (json.JSONDecodeError, OSError) as e: | ||
| raise WrenError( | ||
| ErrorCode.GENERIC_USER_ERROR, | ||
| f"Failed to read {config_path}: {e}", | ||
| ) from e | ||
|
|
||
| if not isinstance(raw, dict): | ||
| raise WrenError( | ||
| ErrorCode.GENERIC_USER_ERROR, | ||
| f"{config_path} must contain a JSON object.", | ||
| ) | ||
|
|
||
| strict_mode = bool(raw.get("strict_mode", False)) | ||
|
|
||
| denied_raw = raw.get("denied_functions", []) | ||
| if not isinstance(denied_raw, list): | ||
| raise WrenError( | ||
| ErrorCode.GENERIC_USER_ERROR, | ||
| f"{config_path}: 'denied_functions' must be a JSON array.", | ||
| ) | ||
| denied_functions = frozenset(str(f).lower() for f in denied_raw) | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| return WrenConfig(strict_mode=strict_mode, denied_functions=denied_functions) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| """SQL policy validation for strict query mode. | ||
|
|
||
| Validates that a parsed SQL AST only references tables defined in the MDL | ||
| manifest and does not use any denied functions. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from sqlglot import exp | ||
|
|
||
| from wren.config import WrenConfig | ||
| from wren.model.error import ErrorCode, ErrorPhase, WrenError | ||
|
|
||
|
|
||
| def validate_sql_policy( | ||
| ast: exp.Expression, | ||
| model_names: set[str], | ||
| user_cte_names: set[str], | ||
| config: WrenConfig, | ||
| ) -> None: | ||
| """Raise ``WrenError`` if the SQL violates strict-mode policies. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| ast: | ||
| Parsed sqlglot AST of the user query. | ||
| model_names: | ||
| Set of model names defined in the MDL manifest. | ||
| user_cte_names: | ||
| Set of CTE names defined in the user's SQL (excluded from table checks). | ||
| config: | ||
| Wren configuration with strict_mode and denied_functions settings. | ||
| """ | ||
| if config.strict_mode: | ||
| _check_tables(ast, model_names, user_cte_names) | ||
| if config.denied_functions: | ||
| _check_functions(ast, config.denied_functions) | ||
|
|
||
|
|
||
| def _check_tables( | ||
| ast: exp.Expression, | ||
| model_names: set[str], | ||
| user_cte_names: set[str], | ||
| ) -> None: | ||
| model_names_lower = {n.lower() for n in model_names} | ||
| user_cte_names_lower = {n.lower() for n in user_cte_names} | ||
| for table in ast.find_all(exp.Table): | ||
| name = table.name | ||
| if not name: | ||
| continue | ||
| if name.lower() in user_cte_names_lower: | ||
| continue | ||
| if name.lower() not in model_names_lower: | ||
| raise WrenError( | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| ErrorCode.MODEL_NOT_FOUND, | ||
| f"Table '{name}' is not defined in the MDL manifest. " | ||
| "In strict mode, all table references must correspond to MDL models.", | ||
| phase=ErrorPhase.SQL_POLICY_CHECK, | ||
| ) | ||
|
|
||
|
|
||
| def _check_functions( | ||
| ast: exp.Expression, | ||
| denied: frozenset[str], | ||
| ) -> None: | ||
| for func in ast.find_all(exp.Func): | ||
| if isinstance(func, exp.Anonymous): | ||
| name = func.name | ||
| else: | ||
| name = type(func).key | ||
| if name.lower() in denied: | ||
| raise WrenError( | ||
| ErrorCode.BLOCKED_FUNCTION, | ||
| f"Function '{name}' is not allowed. " | ||
| "This function is on the denied list.", | ||
| phase=ErrorPhase.SQL_POLICY_CHECK, | ||
| ) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| """Unit tests for wren.config — config loading from ~/.wren/config.json.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import json | ||
|
|
||
| import pytest | ||
|
|
||
| from wren.config import WrenConfig, load_config | ||
| from wren.model.error import WrenError | ||
|
|
||
| pytestmark = pytest.mark.unit | ||
|
|
||
|
|
||
| def test_load_config_no_file(tmp_path): | ||
| config = load_config(tmp_path) | ||
| assert config == WrenConfig() | ||
| assert config.strict_mode is False | ||
| assert config.denied_functions == frozenset() | ||
|
|
||
|
|
||
| def test_load_config_strict_enabled(tmp_path): | ||
| (tmp_path / "config.json").write_text(json.dumps({"strict_mode": True})) | ||
| config = load_config(tmp_path) | ||
| assert config.strict_mode is True | ||
| assert config.denied_functions == frozenset() | ||
|
|
||
|
|
||
| def test_load_config_with_denied_functions(tmp_path): | ||
| data = { | ||
| "strict_mode": True, | ||
| "denied_functions": ["pg_read_file", "DBLINK", "Lo_Import"], | ||
| } | ||
| (tmp_path / "config.json").write_text(json.dumps(data)) | ||
| config = load_config(tmp_path) | ||
| assert config.strict_mode is True | ||
| assert config.denied_functions == frozenset( | ||
| ["pg_read_file", "dblink", "lo_import"] | ||
| ) | ||
|
|
||
|
|
||
| def test_load_config_function_names_lowercased(tmp_path): | ||
| data = {"denied_functions": ["PG_READ_FILE"]} | ||
| (tmp_path / "config.json").write_text(json.dumps(data)) | ||
| config = load_config(tmp_path) | ||
| assert "pg_read_file" in config.denied_functions | ||
|
|
||
|
|
||
| def test_load_config_malformed_json(tmp_path): | ||
| (tmp_path / "config.json").write_text("not valid json{{{") | ||
| with pytest.raises(WrenError): | ||
| load_config(tmp_path) | ||
|
|
||
|
|
||
| def test_load_config_not_a_dict(tmp_path): | ||
| (tmp_path / "config.json").write_text(json.dumps([1, 2, 3])) | ||
| with pytest.raises(WrenError): | ||
| load_config(tmp_path) | ||
|
|
||
|
|
||
| def test_load_config_denied_functions_not_array(tmp_path): | ||
| data = {"denied_functions": "pg_read_file"} | ||
| (tmp_path / "config.json").write_text(json.dumps(data)) | ||
| with pytest.raises(WrenError): | ||
| load_config(tmp_path) | ||
|
|
||
|
|
||
| def test_load_config_unknown_keys_ignored(tmp_path): | ||
| data = {"strict_mode": True, "unknown_key": "value", "another": 42} | ||
| (tmp_path / "config.json").write_text(json.dumps(data)) | ||
| config = load_config(tmp_path) | ||
| assert config.strict_mode is True | ||
|
|
||
|
|
||
| def test_load_config_partial_only_denied_functions(tmp_path): | ||
| data = {"denied_functions": ["dblink"]} | ||
| (tmp_path / "config.json").write_text(json.dumps(data)) | ||
| config = load_config(tmp_path) | ||
| assert config.strict_mode is False | ||
| assert config.denied_functions == frozenset(["dblink"]) | ||
|
|
||
|
|
||
| def test_load_config_empty_object(tmp_path): | ||
| (tmp_path / "config.json").write_text(json.dumps({})) | ||
| config = load_config(tmp_path) | ||
| assert config == WrenConfig() |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.