Repository navigation
Merge security + reliability PRs (11): explain SQLi, dep CVEs, pool fixes, introspection - #2
Merged
Merged
Conversation
AGENTS.md provides universal rules for all AI agents: safety guardrails (get_sql_driver() hard rule), CI commands, project structure, and code conventions. CLAUDE.md adds Claude-specific tool preferences, MCP-aware validation workflows, and commit style guidance — referencing AGENTS.md for shared rules rather than duplicating them. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ools Add PostgreSQL COMMENT support to list_objects and get_object_details: - list_objects: include obj_description() for tables, views, and materialized views - get_object_details: include col_description() for column comments and obj_description() for table/view/materialized view comments Add materialized view as a new object_type: - list_objects: query pg_class with relkind='m' for materialized views - get_object_details: query pg_attribute for columns (since materialized views are not in information_schema.columns) This enables LLMs using the MCP server to discover schema documentation stored as COMMENT ON TABLE/VIEW/COLUMN statements, which is the standard PostgreSQL mechanism for schema documentation. Closes crystaldba#71
These pg_catalog functions are read-only and needed for the schema comment feature to work in restricted mode.
Add _validate_explain_input() using pglast to parse and validate SQL before f-string concatenation. Prevents multi-statement injection and restricts EXPLAIN ANALYZE to SELECT-only.
17 unit tests + 5 integration tests covering valid queries, multi-statement injection rejection, empty/invalid input, EXPLAIN ANALYZE DML restrictions, and COPY exfiltration prevention.
Add "PostgreSQL" keyword to all tool descriptions so LLM search can find them. Make get_object_details and list_objects descriptions explicit about returned data (columns, comments, constraints, indexes).
Replace legacy typing imports (List, Dict, Optional, Union) with built-in generics (list, dict, X | None, X | Y) across 7 source files. Removes the UP006/UP035/UP045 ruff ignore entries and the TODO comment referencing issue crystaldba#129.
Establish the connection pool on first tool use instead of at server startup, and let idle connections be reaped, so defining many database MCP configs no longer means every server holds an open Postgres connection from session start. - server.py: store the database URL without connecting eagerly; the lazy path in SqlDriver.execute_query opens the pool on first use. - sql_driver.py: pool now uses min_size=0 with a max_idle reaper so unused/idle connections release. - Add --max-idle flag and DATABASE_MAX_IDLE env var (default 300s) to tune the idle timeout; invalid values fall back to the default via a validating max_idle property. - README: document lazy connections and the idle timeout with examples. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two defects in DbConnPool/SqlDriver combined to produce the behavior reported in crystaldba#98 (ever-incrementing pool-N names, 'the pool is closed' errors, and memory growth proportional to usage): 1. SqlDriver.execute_query marked the shared pool invalid on *every* exception, including query-level errors like bad SQL or a statement timeout. The next call then tore down and rebuilt the pool, so each malformed query churned a pool. Now only connection-level errors (OperationalError/InterfaceError, excluding QueryCanceled) invalidate the pool. 2. DbConnPool.pool_connect had no synchronization: concurrent callers that both observed an invalid pool each built an AsyncConnectionPool, and every overwritten pool was left open forever - leaking its connections and worker tasks - while callers holding a pool that a later winner closed failed with 'the pool ... is closed'. Pool (re)creation is now serialized with an asyncio.Lock and re-checked under the lock. Also pass check=AsyncConnectionPool.check_connection so connections broken while idle (server restart, idle timeout) are discarded on checkout instead of surfacing as query errors that would previously have poisoned the pool. In production we observed this as steady ~6MiB/day memory growth in a long-lived streamable-http deployment, resetting only on pod restart. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
uv lock --upgrade across the board, constraining mcp to <2 (2.0 removed mcp.server.fastmcp, which server.py is built on; 1.29.0 is the newest 1.x and satisfies every advisory). Base image bookworm -> trixie for both stages, and the runtime stage upgrades the base image's pip, which ships with known CVEs despite never being used at runtime. Headline bumps: aiohttp 3.11.16->3.14.3, cryptography 46.0.3->50.0.0, starlette 0.46.1->1.3.1, h11 0.14->0.16, pyjwt 2.10.1->2.13.0, urllib3 2.3.0->2.7.0, python-multipart 0.0.21->0.0.32. Test suite passes (206 passed, 9 skipped, 1 xfailed); streamable-http transport smoke-tested against the built image. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…indexes Pooled backends reused by execute_query kept session GUCs after COMMIT and leftover HypoPG indexes after hypothetical-index analysis. Later MCP requests that hit the same PID inherited that state. Give AsyncConnectionPool a reset callback that runs DISCARD ALL outside a transaction and, if HypoPG is installed, schema-qualified hypopg_reset(). Cleanup errors propagate so the pool discards the connection. Combine DTA hypopg_create_index and hypopg_list_indexes into one checkout so size estimates still work after reset-on-return. Closes crystaldba#203
…dba#143 not-exist error)
…rror-classification + check_connection)
… line length) Merging crystaldba#173 (which removed the UP006/UP035 ruff ignores) together with PRs that still used legacy typing (`List`/`Dict`, `typing.List`) left the tree failing ruff. Applies crystaldba#173's modernization to the stragglers (test_dta_calc.py, tests/utils.py, conftest.py, index_opt_base.py) and wraps three long tool-description strings from crystaldba#160 (E501) via implicit string concatenation (text unchanged). ruff clean, pyright 0 errors, pytest 208 passed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D6w353jusev2fgmWQY1mS1
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
11 participants
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.
Integrates worthwhile upstream improvements after the security work, weighted to a read-only, local-stdio deployment. Checks: ruff clean, pyright 0 errors,
pytest tests/unit= 208 passed / 27 skipped.timezone()in restricted mode.🤖 Generated with Claude Code