From cfbfd84d44acd1db09a182d55ec9f2b8a940d078 Mon Sep 17 00:00:00 2001 From: Hussain Sultan Date: Fri, 10 Jul 2026 06:29:37 -0400 Subject: [PATCH 1/3] fix(security): constrain safe_eval to query DSL --- src/boring_semantic_layer/tests/test_utils.py | 67 ++++ src/boring_semantic_layer/utils.py | 287 +++++++++++++++++- 2 files changed, 343 insertions(+), 11 deletions(-) diff --git a/src/boring_semantic_layer/tests/test_utils.py b/src/boring_semantic_layer/tests/test_utils.py index 4abbb82d..1723774a 100644 --- a/src/boring_semantic_layer/tests/test_utils.py +++ b/src/boring_semantic_layer/tests/test_utils.py @@ -1,5 +1,6 @@ from __future__ import annotations +import ibis from ibis import _ from returns.result import Failure, Success @@ -44,6 +45,72 @@ def test_safe_eval_disallowed_names(): assert isinstance(result, Failure) +def test_safe_eval_blocks_dunder_attribute_escape(): + result = safe_eval("().__class__.__mro__[1].__subclasses__()") + assert isinstance(result, Failure) + + +def test_safe_eval_blocks_dunder_escape_even_with_allowed_names(): + result = safe_eval( + "().__class__.__mro__[1].__subclasses__()", + allowed_names={"_"}, + ) + assert isinstance(result, Failure) + + +def test_safe_eval_blocks_dunder_inside_lambda_body(): + result = safe_eval("lambda t: t.__class__") + assert isinstance(result, Failure) + + +def test_safe_eval_blocks_private_names_from_context(): + result = safe_eval("__import__('os')", context={"__import__": __import__}) + assert isinstance(result, Failure) + + +def test_safe_eval_blocks_effectful_method_calls(): + class MockModel: + def execute(self): + raise AssertionError("execute should not be called") + + result = safe_eval("model.execute()", context={"model": MockModel()}) + assert isinstance(result, Failure) + + +def test_safe_eval_blocks_arbitrary_model_attribute_chains(): + class MockModel: + table = object() + + result = safe_eval("model.table", context={"model": MockModel()}) + assert isinstance(result, Failure) + + +def test_safe_eval_allows_query_metadata_attributes(): + class MockModel: + dimensions = ("origin", "destination") + measures = ("flight_count",) + + dimensions = safe_eval("model.dimensions", context={"model": MockModel()}) + measures = safe_eval("model.measures", context={"model": MockModel()}) + + assert dimensions.unwrap() == ("origin", "destination") + assert measures.unwrap() == ("flight_count",) + + +def test_safe_eval_allows_curated_ibis_helpers(): + result = safe_eval("ibis.literal(1)", context={"ibis": ibis}, allowed_names={"ibis"}) + assert isinstance(result, Success) + + +def test_safe_eval_blocks_ibis_io_helpers(): + result = safe_eval( + "ibis.duckdb.connect(':memory:')", + context={"ibis": ibis}, + allowed_names={"ibis"}, + ) + assert isinstance(result, Failure) + + def test_safe_eval_ibis_column_access(): result = safe_eval("_.column_name", context={"_": _}, allowed_names={"_"}) assert isinstance(result, Success) diff --git a/src/boring_semantic_layer/utils.py b/src/boring_semantic_layer/utils.py index 1c67f66f..5c2672c3 100644 --- a/src/boring_semantic_layer/utils.py +++ b/src/boring_semantic_layer/utils.py @@ -69,17 +69,275 @@ class SafeEvalError(Exception): } -def _validate_ast(node: ast.AST, allowed_names: set[str] | None = None) -> None: - if type(node) not in SAFE_NODES: - raise SafeEvalError( - f"Unsafe node type: {type(node).__name__}. Only whitelisted operations are allowed." - ) +# Helpers exposed by the Ibis modules in ``ibis_string_to_expr`` and agent +# query contexts. In particular, backend/IO namespaces (duckdb, postgres, +# read_*, connect, ...) are intentionally absent. +SAFE_MODULE_CALLS = frozenset( + { + "and_", + "asc", + "cases", + "coalesce", + "cume_dist", + "date", + "dense_rank", + "desc", + "greatest", + "ifelse", + "interval", + "least", + "literal", + "now", + "ntile", + "or_", + "param", + "percent_rank", + "random", + "rank", + "row_number", + "time", + "timestamp", + "today", + "uuid", + "window", + } +) + + +# Pure expression operations plus the BSL query-building operations accepted +# in agent-generated query chains. Calls that execute, compile, perform IO, +# access a backend, or expose an underlying operation are deliberately absent. +SAFE_METHOD_CALLS = frozenset( + { + "abs", + "aggregate", + "all", + "any", + "approx_median", + "approx_nunique", + "arbitrary", + "argmax", + "argmin", + "as_table", + "between", + "capitalize", + "cast", + "ceil", + "coalesce", + "collect", + "contains", + "count", + "cummax", + "cummean", + "cummin", + "cumsum", + "date", + "day", + "day_of_week", + "distinct", + "drop", + "endswith", + "epoch_seconds", + "fill_null", + "filter", + "find", + "first", + "floor", + "group_by", + "hour", + "identical_to", + "ifelse", + "isin", + "isnull", + "lag", + "last", + "lead", + "length", + "like", + "limit", + "lower", + "lpad", + "lstrip", + "max", + "mean", + "median", + "microsecond", + "millisecond", + "min", + "minute", + "mode", + "month", + "mutate", + "name", + "notin", + "notnull", + "nullif", + "nunique", + "order_by", + "over", + "quarter", + "quantile", + "re_extract", + "re_replace", + "re_search", + "re_split", + "rename", + "repeat", + "replace", + "reverse", + "right", + "round", + "rpad", + "rstrip", + "second", + "select", + "sign", + "split", + "startswith", + "std", + "strftime", + "strip", + "substr", + "sum", + "time", + "timestamp", + "translate", + "truncate", + "typeof", + "unique", + "unnest", + "upper", + "var", + "week_of_year", + "with_dimensions", + "with_measures", + "year", + } +) + +SAFE_QUERY_METHOD_CALLS = frozenset( + { + "aggregate", + "distinct", + "drop", + "filter", + "group_by", + "limit", + "mutate", + "order_by", + "rename", + "select", + "unnest", + "with_dimensions", + "with_measures", + } +) + +SAFE_QUERY_ATTRIBUTES = frozenset({"dimensions", "measures"}) + +_SAFE_MODULE_ROOTS = frozenset({"ibis", "xorq_ibis", "xo"}) + + +class _SafeEvalValidator(ast.NodeVisitor): + """Validate the small, expression-only DSL accepted by ``safe_eval``.""" + + def __init__(self, allowed_names: set[str]): + self._allowed_names = allowed_names + self._lambda_names: list[set[str]] = [] + + @staticmethod + def _root_name(node: ast.AST) -> str | None: + while True: + if isinstance(node, ast.Name): + return node.id + if isinstance(node, ast.Attribute): + node = node.value + elif isinstance(node, ast.Call): + node = node.func + elif isinstance(node, ast.Subscript): + node = node.value + else: + return None + + def _is_expression_root(self, name: str | None) -> bool: + lambda_names = set().union(*self._lambda_names) if self._lambda_names else set() + return name == "_" or name in lambda_names or name in _SAFE_MODULE_ROOTS + + def generic_visit(self, node: ast.AST) -> None: + if type(node) not in SAFE_NODES: + raise SafeEvalError( + f"Unsafe node type: {type(node).__name__}. Only whitelisted operations are allowed." + ) + super().generic_visit(node) + + def visit_Name(self, node: ast.Name) -> None: # noqa: N802 + if node.id.startswith("_") and node.id != "_": + raise SafeEvalError(f"Private name '{node.id}' is not allowed") + lambda_names = set().union(*self._lambda_names) if self._lambda_names else set() + if node.id not in self._allowed_names and node.id not in lambda_names: + raise SafeEvalError( + f"Name '{node.id}' is not in the allowed names: {self._allowed_names}" + ) + + def visit_Attribute(self, node: ast.Attribute) -> None: # noqa: N802 + if node.attr.startswith("_"): + raise SafeEvalError(f"Private attribute '{node.attr}' is not allowed") + if ( + isinstance(node.value, ast.Name) + and node.value.id in _SAFE_MODULE_ROOTS + and node.attr not in SAFE_MODULE_CALLS + ): + raise SafeEvalError(f"Module attribute '{node.value.id}.{node.attr}' is not allowed") + if ( + isinstance(node.value, ast.Name) + and not self._is_expression_root(node.value.id) + and node.attr not in SAFE_QUERY_METHOD_CALLS | SAFE_QUERY_ATTRIBUTES + ): + raise SafeEvalError( + f"Attribute '{node.value.id}.{node.attr}' is not an allowed query operation" + ) + self.generic_visit(node) + + def visit_Call(self, node: ast.Call) -> None: # noqa: N802 + if not isinstance(node.func, ast.Attribute): + raise SafeEvalError("Only allowlisted DSL method calls are allowed") + + root_name = self._root_name(node.func) + if isinstance(node.func.value, ast.Name) and node.func.value.id in _SAFE_MODULE_ROOTS: + if node.func.attr not in SAFE_MODULE_CALLS: + raise SafeEvalError( + f"Module call '{node.func.value.id}.{node.func.attr}' is not allowed" + ) + elif ( + root_name is not None + and not self._is_expression_root(root_name) + and node.func.attr not in SAFE_QUERY_METHOD_CALLS + ): + raise SafeEvalError(f"Query method call '{node.func.attr}' is not allowed") + elif node.func.attr not in SAFE_METHOD_CALLS: + raise SafeEvalError(f"Method call '{node.func.attr}' is not allowed") + + for keyword in node.keywords: + if keyword.arg is None or keyword.arg.startswith("_"): + raise SafeEvalError("Private or expanded keyword arguments are not allowed") + self.generic_visit(node) + + def visit_Lambda(self, node: ast.Lambda) -> None: # noqa: N802 + args = node.args + if args.vararg or args.kwarg or args.kwonlyargs or args.defaults or args.kw_defaults: + raise SafeEvalError("Lambda defaults and variadic arguments are not allowed") + lambda_names = {arg.arg for arg in [*args.posonlyargs, *args.args]} + if any(name.startswith("_") for name in lambda_names): + raise SafeEvalError("Private lambda argument names are not allowed") + self._lambda_names.append(lambda_names) + try: + self.visit(node.body) + finally: + self._lambda_names.pop() - if isinstance(node, ast.Name) and allowed_names is not None and node.id not in allowed_names: - raise SafeEvalError(f"Name '{node.id}' is not in the allowed names: {allowed_names}") - for child in ast.iter_child_nodes(node): - _validate_ast(child, allowed_names) +def _validate_ast(node: ast.AST, allowed_names: set[str]) -> None: + _SafeEvalValidator(allowed_names).visit(node) def _parse_expr(expr_str: str) -> ast.AST: @@ -111,12 +369,19 @@ def safe_eval( context: dict[str, Any] | None = None, allowed_names: set[str] | None = None, ) -> Result[Any, Exception]: - eval_context = {"__builtins__": {}, **(context or {})} + context = context or {} + names = set(context) if allowed_names is None else set(allowed_names) + # ``_`` is the one intentionally public DSL identifier beginning with an + # underscore. All caller-provided private names remain inaccessible. + names = {name for name in names if name == "_" or not name.startswith("_")} + # Keep builtins empty even if an untrusted caller supplied a conflicting + # ``__builtins__`` context entry. + eval_context = {**context, "__builtins__": {}} @safe def do_eval(): tree = _parse_expr(expr_str) - _validate_ast(tree, allowed_names) + _validate_ast(tree, names) code = _compile_validated(tree) return _eval_in_context(eval_context, code) From 718139e6009b102edff1b7a19745f13a17af5c51 Mon Sep 17 00:00:00 2001 From: Hussain Sultan Date: Fri, 10 Jul 2026 06:30:06 -0400 Subject: [PATCH 2/3] fix(server): harden HTTP defaults and authentication --- src/boring_semantic_layer/agents/cli.py | 6 +- src/boring_semantic_layer/server/__init__.py | 4 +- src/boring_semantic_layer/server/api.py | 78 +++++++++++++++++-- .../tests/test_server_api.py | 59 ++++++++++++++ 4 files changed, 134 insertions(+), 13 deletions(-) diff --git a/src/boring_semantic_layer/agents/cli.py b/src/boring_semantic_layer/agents/cli.py index 58349eea..7ca00cb2 100644 --- a/src/boring_semantic_layer/agents/cli.py +++ b/src/boring_semantic_layer/agents/cli.py @@ -432,8 +432,8 @@ def main(): ) serve_parser.add_argument( "--host", - default="0.0.0.0", - help="Host to bind to (default: 0.0.0.0)", + default="127.0.0.1", + help="Host to bind to (default: 127.0.0.1)", ) serve_parser.add_argument( "--port", @@ -448,7 +448,7 @@ def main(): ) serve_parser.add_argument( "--cors-origins", - help="Comma-separated CORS allowlist. Defaults to BSL_CORS_ORIGINS or '*' if unset.", + help="Comma-separated CORS allowlist. Defaults to BSL_CORS_ORIGINS or no origins.", ) serve_parser.set_defaults(func=cmd_serve) diff --git a/src/boring_semantic_layer/server/__init__.py b/src/boring_semantic_layer/server/__init__.py index 83225079..428c09d2 100644 --- a/src/boring_semantic_layer/server/__init__.py +++ b/src/boring_semantic_layer/server/__init__.py @@ -3,8 +3,8 @@ from __future__ import annotations import os +from collections.abc import Sequence from pathlib import Path -from typing import Sequence from .api import create_app @@ -13,7 +13,7 @@ def main( config: str | None = None, - host: str = "0.0.0.0", + host: str = "127.0.0.1", port: int = 8000, reload: bool = False, cors_origins: Sequence[str] | None = None, diff --git a/src/boring_semantic_layer/server/api.py b/src/boring_semantic_layer/server/api.py index 9bfef517..4f7dfe4e 100644 --- a/src/boring_semantic_layer/server/api.py +++ b/src/boring_semantic_layer/server/api.py @@ -2,11 +2,13 @@ from __future__ import annotations +import inspect import json import logging import os import re -from collections.abc import Mapping, Sequence +import secrets +from collections.abc import Awaitable, Callable, Mapping, Sequence from contextlib import asynccontextmanager from typing import Any @@ -22,6 +24,8 @@ logger = logging.getLogger(__name__) +AuthHook = Callable[[Request], bool | None | Awaitable[bool | None]] + class QueryRequest(BaseModel): """HTTP request body for the query endpoint.""" @@ -88,9 +92,21 @@ def _check_grain_fields(self) -> ComparePeriodsRequest: def _default_cors_origins() -> list[str]: raw = os.environ.get("BSL_CORS_ORIGINS") if not raw: - return ["*"] + return [] origins = [origin.strip() for origin in raw.split(",") if origin.strip()] - return origins or ["*"] + return origins + + +def _api_key_auth_hook(api_key: str) -> AuthHook: + """Build a constant-time API-key hook for simple deployments.""" + + def authenticate(request: Request) -> bool: + authorization = request.headers.get("authorization", "") + bearer = authorization[7:] if authorization.lower().startswith("bearer ") else "" + supplied = request.headers.get("x-bsl-api-key") or bearer + return bool(supplied) and secrets.compare_digest(supplied, api_key) + + return authenticate def _get_models(request: Request) -> Mapping[str, Any]: @@ -227,8 +243,23 @@ def create_app( models: Mapping[str, Any] | None = None, config_path: str | None = None, cors_origins: Sequence[str] | None = None, + auth_hook: AuthHook | None = None, + api_key: str | None = None, ) -> FastAPI: - """Create the FastAPI app for the BSL HTTP server.""" + """Create the FastAPI app for the BSL HTTP server. + + ``auth_hook`` is called before every endpoint except health checks and CORS + preflight requests. It may raise ``HTTPException`` or return ``False`` to + reject a request. For simple deployments, ``api_key`` (or ``BSL_API_KEY``) + enables Bearer and ``X-BSL-API-Key`` authentication. + """ + + if auth_hook is not None and api_key: + raise ValueError("Specify either auth_hook or api_key, not both") + configured_api_key = api_key or (os.environ.get("BSL_API_KEY") if auth_hook is None else None) + effective_auth_hook = auth_hook or ( + _api_key_auth_hook(configured_api_key) if configured_api_key else None + ) @asynccontextmanager async def lifespan(app: FastAPI): @@ -236,11 +267,42 @@ async def lifespan(app: FastAPI): yield app = FastAPI(title="Boring Semantic Layer HTTP API", version="0.1.0", lifespan=lifespan) + + @app.middleware("http") + async def authenticate_request(request: Request, call_next): + if ( + effective_auth_hook is None + or request.url.path == "/health" + or request.method == "OPTIONS" + ): + return await call_next(request) + try: + authorized = effective_auth_hook(request) + if inspect.isawaitable(authorized): + authorized = await authorized + except HTTPException as exc: + return JSONResponse( + status_code=exc.status_code, + content={"detail": exc.detail}, + headers=exc.headers, + ) + except Exception: + logger.exception("HTTP authentication hook failed") + return JSONResponse(status_code=500, content={"detail": "Internal server error"}) + if authorized is False: + return JSONResponse( + status_code=401, + content={"detail": "Unauthorized"}, + headers={"WWW-Authenticate": "Bearer"}, + ) + return await call_next(request) + + allowed_origins = list(_default_cors_origins() if cors_origins is None else cors_origins) app.add_middleware( CORSMiddleware, - allow_origins=list(cors_origins or _default_cors_origins()), - allow_methods=["*"], - allow_headers=["*"], + allow_origins=allowed_origins, + allow_methods=["GET", "POST"], + allow_headers=["Authorization", "Content-Type", "X-BSL-API-Key"], ) @app.exception_handler(ValueError) @@ -250,7 +312,7 @@ async def value_error_handler(_request: Request, exc: ValueError) -> JSONRespons @app.exception_handler(Exception) async def unhandled_exception_handler(_request: Request, exc: Exception) -> JSONResponse: logger.exception("Unhandled HTTP API error") - return JSONResponse(status_code=500, content={"detail": str(exc)}) + return JSONResponse(status_code=500, content={"detail": "Internal server error"}) @app.get("/health") def health() -> dict[str, str]: diff --git a/src/boring_semantic_layer/tests/test_server_api.py b/src/boring_semantic_layer/tests/test_server_api.py index 59a3f7b1..5e64c59e 100644 --- a/src/boring_semantic_layer/tests/test_server_api.py +++ b/src/boring_semantic_layer/tests/test_server_api.py @@ -2,6 +2,8 @@ from __future__ import annotations +import inspect + import ibis import pandas as pd import pytest @@ -9,6 +11,7 @@ from boring_semantic_layer import to_semantic_table from boring_semantic_layer.server import create_app +from boring_semantic_layer.server import main as serve from boring_semantic_layer.server.loader import load_models @@ -127,6 +130,10 @@ def test_health(client): assert response.json() == {"status": "ok"} +def test_server_defaults_to_loopback(): + assert inspect.signature(serve).parameters["host"].default == "127.0.0.1" + + def test_list_models(client): response = client.get("/models") @@ -297,3 +304,55 @@ def test_missing_model_returns_404(client): assert response.status_code == 404 assert "not found" in response.json()["detail"].lower() + + +def test_auth_hook_protects_data_endpoints_but_not_health(sample_models): + app = create_app( + models=sample_models, + auth_hook=lambda request: request.headers.get("x-test-token") == "secret", + ) + with TestClient(app) as auth_client: + assert auth_client.get("/health").status_code == 200 + assert auth_client.get("/models").status_code == 401 + authorized = auth_client.get("/models", headers={"x-test-token": "secret"}) + + assert authorized.status_code == 200 + + +def test_api_key_authentication(sample_models): + with TestClient(create_app(models=sample_models, api_key="secret")) as auth_client: + assert auth_client.get("/models").status_code == 401 + bearer = auth_client.get("/models", headers={"Authorization": "Bearer secret"}) + custom_header = auth_client.get("/models", headers={"X-BSL-API-Key": "secret"}) + + assert bearer.status_code == 200 + assert custom_header.status_code == 200 + + +def test_default_cors_does_not_allow_arbitrary_origins(sample_models): + with TestClient(create_app(models=sample_models)) as cors_client: + response = cors_client.options( + "/models", + headers={ + "Origin": "https://untrusted.example", + "Access-Control-Request-Method": "GET", + }, + ) + + assert "access-control-allow-origin" not in response.headers + + +def test_unhandled_errors_do_not_leak_exception_text(): + class BrokenModel: + def query(self, **_kwargs): + raise RuntimeError("database password is hunter2") + + app = create_app(models={"broken": BrokenModel()}) + with TestClient(app, raise_server_exceptions=False) as error_client: + response = error_client.post( + "/query", + json={"model_name": "broken", "measures": ["count"], "get_chart": False}, + ) + + assert response.status_code == 500 + assert response.json() == {"detail": "Internal server error"} From 54c8fc28a8d9c481fa72624d114842940b40fa0d Mon Sep 17 00:00:00 2001 From: Hussain Sultan Date: Fri, 10 Jul 2026 06:30:36 -0400 Subject: [PATCH 3/3] fix(server): evaluate derived time dimensions --- src/boring_semantic_layer/server/api.py | 17 +++++++---- .../tests/test_server_api.py | 28 +++++++++++++++++++ 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/src/boring_semantic_layer/server/api.py b/src/boring_semantic_layer/server/api.py index 4f7dfe4e..9ba3ece1 100644 --- a/src/boring_semantic_layer/server/api.py +++ b/src/boring_semantic_layer/server/api.py @@ -145,18 +145,25 @@ def _build_model_response(model: Any) -> dict[str, Any]: def _get_time_range_response(model: Any, model_name: str) -> dict[str, str]: - time_dim_name = _find_time_dimension(model, list(model.dimensions)) + dimensions = model.get_dimensions() + time_dim_name = _find_time_dimension(model, list(dimensions)) if not time_dim_name: raise HTTPException(status_code=400, detail=f"Model '{model_name}' has no time dimension") tbl = model.table - col_name = time_dim_name.split(".")[-1] if "." in time_dim_name else time_dim_name - time_col = tbl[col_name] + time_col = dimensions[time_dim_name](tbl) result = tbl.aggregate(start=time_col.min(), end=time_col.max()).execute() + def to_iso(value: Any) -> str: + # Some backends materialize an Ibis date aggregate as a midnight + # pandas Timestamp. Preserve the semantic datatype in the HTTP value. + if time_col.type().is_date() and callable(as_date := getattr(value, "date", None)): + value = as_date() + return value.isoformat() + return { - "start": result["start"].iloc[0].isoformat(), - "end": result["end"].iloc[0].isoformat(), + "start": to_iso(result["start"].iloc[0]), + "end": to_iso(result["end"].iloc[0]), } diff --git a/src/boring_semantic_layer/tests/test_server_api.py b/src/boring_semantic_layer/tests/test_server_api.py index 5e64c59e..468edc68 100644 --- a/src/boring_semantic_layer/tests/test_server_api.py +++ b/src/boring_semantic_layer/tests/test_server_api.py @@ -164,6 +164,34 @@ def test_get_time_range(client): assert data["end"].startswith("2024-01-30") +def test_get_time_range_evaluates_derived_dimension(): + con = ibis.duckdb.connect(":memory:") + tbl = con.create_table( + "derived_time_http", + pd.DataFrame( + { + "ts": pd.to_datetime( + ["2024-03-01 12:30:00", "2024-03-04 08:15:00", "2024-03-09 18:45:00"] + ) + } + ), + overwrite=True, + ) + model = to_semantic_table(tbl, name="events").with_dimensions( + event_date={ + "expr": lambda t: t.ts.date(), + "is_time_dimension": True, + "smallest_time_grain": "day", + } + ) + + with TestClient(create_app(models={"events": model})) as derived_client: + response = derived_client.get("/models/events/time-range") + + assert response.status_code == 200 + assert response.json() == {"start": "2024-03-01", "end": "2024-03-09"} + + def test_search_dimension_values(client): response = client.get("/models/flights/dimensions/carrier/values", params={"limit": 2})