Skip to content

Commit eea4aeb

Browse files
olivermeyerclaude
andcommitted
fix(database): hide bound parameters in errors
The engine did not set hide_parameters. So a StatementError or a DBAPIError message contained "[parameters: (...)]" with the values of the statement. These messages go to logs and Sentry. The engine now sets hide_parameters=True. Refs: VPTHP-128 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 952100d commit eea4aeb

3 files changed

Lines changed: 23 additions & 1 deletion

File tree

‎src/aignostics_foundry_core/AGENTS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ This file provides an overview of all modules in `aignostics_foundry_core`, thei
371371

372372
- **Purpose**: Manages a process-level async database engine singleton, providing session injection for FastAPI routes, background jobs, and CLI commands. All public functions accept optional DB-config params and fall back to the active `FoundryContext.database` when they are `None`.
373373
- **Key Features**:
374-
- `init_engine(db_url=None, pool_size=None, pool_max_overflow=None, pool_timeout=None)` — initialises the global `AsyncEngine` and `async_sessionmaker`; subsequent calls are silent no-ops. When `db_url` is `None`, the URL and pool settings are resolved from `get_context().database`; raises `RuntimeError` if no context is installed or `ctx.database` is `None`. Pool parameters are omitted automatically for SQLite (which does not use `QueuePool`).
374+
- `init_engine(db_url=None, pool_size=None, pool_max_overflow=None, pool_timeout=None)` — initialises the global `AsyncEngine` and `async_sessionmaker`; subsequent calls are silent no-ops. When `db_url` is `None`, the URL and pool settings are resolved from `get_context().database`; raises `RuntimeError` if no context is installed or `ctx.database` is `None`. Pool parameters are omitted automatically for SQLite (which does not use `QueuePool`). The engine sets `hide_parameters=True`, so SQLAlchemy error messages do not contain bound parameter values.
375375
- `dispose_engine()` — async; disposes the engine; called during application shutdown.
376376
- `get_db_session()` — async generator; yields an `AsyncSession`; raises `RuntimeError` if engine not initialised. Use as a FastAPI `Depends` target.
377377
- `execute_with_session(async_func, *args, **kwargs)` — async; runs `async_func` with a session injected as the `session` keyword argument. For background jobs and CLI helpers.

‎src/aignostics_foundry_core/database.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,9 @@ def init_engine(
211211
Called during FastAPI lifespan startup or first job execution.
212212
Subsequent calls are no-ops (engine is already initialized).
213213
214+
The engine hides bound parameter values in SQLAlchemy error messages, because
215+
these messages go to logs and Sentry.
216+
214217
For multiprocessing: Engine is automatically reset in child processes via
215218
multiprocessing.util.register_after_fork().
216219
@@ -255,6 +258,8 @@ def init_engine(
255258
"url": db_url,
256259
"pool_pre_ping": True,
257260
"echo": False,
261+
# Keep bound parameter values out of error messages. They go to logs and Sentry.
262+
"hide_parameters": True,
258263
}
259264
if not db_url.startswith("sqlite"):
260265
engine_kwargs["pool_size"] = pool_size

‎tests/aignostics_foundry_core/database_test.py‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
from pathlib import Path
66

77
import pytest
8+
from sqlalchemy import text
9+
from sqlalchemy.exc import OperationalError
810
from sqlalchemy.ext.asyncio import AsyncSession
911

1012
from aignostics_foundry_core.database import (
@@ -24,6 +26,7 @@
2426
DB_URL_ERROR_FRAGMENT = "DB_URL"
2527

2628
SESSION_KWARG = "session"
29+
SENSITIVE_PARAMETER_VALUE = "sensitive-parameter-value-vpthp-128"
2730

2831

2932
@pytest.fixture(autouse=True)
@@ -74,6 +77,20 @@ async def test_init_engine_is_idempotent(self, sqlite_url: str) -> None:
7477
init_engine(sqlite_url) # Must not raise; second call is a silent no-op
7578
await execute_with_session(noop) # Session maker still functional
7679

80+
@pytest.mark.unit
81+
async def test_failed_statement_error_hides_parameter_values(self, sqlite_url: str) -> None:
82+
"""A failed statement gives an error message without the bound parameter values."""
83+
84+
async def select_from_missing_table(session: AsyncSession) -> None:
85+
await session.execute(text("SELECT * FROM missing_table WHERE x = :v"), {"v": SENSITIVE_PARAMETER_VALUE})
86+
87+
init_engine(sqlite_url)
88+
with pytest.raises(OperationalError) as exc_info:
89+
await execute_with_session(select_from_missing_table)
90+
91+
assert SENSITIVE_PARAMETER_VALUE not in str(exc_info.value)
92+
assert "hide_parameters=True" in str(exc_info.value)
93+
7794
@pytest.mark.unit
7895
async def test_init_engine_non_sqlite_url_accepted(self) -> None:
7996
"""init_engine accepts a non-SQLite URL without raising (engine creation is lazy)."""

0 commit comments

Comments
 (0)