diff --git a/CHANGELOG.md b/CHANGELOG.md index 42e7578..f5987c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - Fix: what agents are told now follows the enforced access mode and database lock. In restricted mode the `execute_sql` description no longer advertises `read_only=false` writes and says they are refused; in unrestricted mode it explains the write opt-in. Under a database lock, `list_databases` no longer claims new databases need no configuration edit, and both it and `execute_sql` say only listed databases can be selected. `initialize` now returns server instructions naming the access mode, the database lock and result/time limits; `list_databases` adds `database_lock`. A `25006` read-only rejection now tells unrestricted-mode agents to retry with `read_only=false` if a write is intended (the target must also accept writes, so not a standby), and the restricted-mode refusal names the access mode. The misleading `ReadOnly = true, Destructive = false` declaration on `execute_sql` is removed; annotations were and remain set from the access mode at registration (unrestricted: `readOnlyHint=false`, `destructiveHint=true`). - Every tool parameter now has a short description listing allowed values or defaults (`section`, `format`, `order_by`, `limit` and its `max_rows` cap, `offset`, `target`, …), so agents no longer discover them from error round trips. The restricted `tools/list` array grows from 6,780 to 10,516 bytes. - SQL errors are now actionable without exposing data values. Errors for statements submitted to `execute_sql` and `explain_query` include `position`, PostgreSQL's 1-based character position mapped into the submitted statement (previously absent; the server's paging/EXPLAIN wrapper would also have shifted it). SQLSTATE class `42` errors (syntax, undefined/ambiguous objects, privileges) that PostgreSQL positions inside the submitted statement, and `42501` privilege errors, add `server_message` and `server_hint` when not raised inside a routine or internal query; execution-time class `42` errors that can echo row values (`current_setting(col)`, `col::regclass`, `query_to_xml`) stay withheld. Class `23` integrity errors not raised from a routine add `constraint`. All other PostgreSQL messages, hints and details remain withheld, and the fallback summary no longer claims the message is withheld when `server_message` is returned. Existing `code`, `message` and `sql_state` fields are unchanged. +- `execute_sql` and `explain_query` accept optional `parameters`: up to 256 JSON scalars bound to `$1..$n`, never interpolated into the SQL. Each value is sent as PostgreSQL `unknown`, so its type is inferred from context like a quoted literal; cast when ambiguous. Objects and arrays return `invalid_parameters`, and new fixed summaries cover too few values (`08P01`) and an undeterminable parameter type (`42P18`, which also results from extra values or a skipped `$n`). Paging with `next_offset` must resend the same `sql` and `parameters`. Existing calls are unchanged. - Show a grouped README badge table covering build/tests, security scanning, supply chain, registries, platforms, stack versions, features and project activity. Feature and practice badges link to the documentation or configuration that supports them; stack versions are read live from `global.json` and `Directory.Packages.props`. - Release npm job: install npm 12.2.0 from its registry tarball after a pinned SHA-512 check matching the published `dist.integrity`, replacing the unpinned `npm install --global` flagged by Scorecard Pinned-Dependencies. - Release npm job: accept npm 12's one-element JSON array from `npm view @ version --json`. Previously an already-published version failed with "requires an element of type 'String'" instead of being reported as not republished. diff --git a/README.md b/README.md index 79a4c9f..b382137 100644 --- a/README.md +++ b/README.md @@ -360,12 +360,14 @@ Except `list_databases`, every tool **requires `database`**. Every tool accepts | `list_schemas` | Schemas with USAGE privilege | literal `prefix`, `include_system`, page | | `list_objects` | Tables, views, materialized views, sequences, functions, procedures and extensions | `schema`, `type`, literal `search`, `include_system`, page | | `get_object_details` | One object's metadata section | `schema`, `name`, `section`: columns/constraints/indexes/triggers/definition/parameters; `type`, `identity_arguments`, page | -| `execute_sql` | One SQL statement; bounded results or explicitly enabled writes | `sql`, `read_only`, page | -| `explain_query` | Estimated/actual JSON plan and compact major-node summary | `sql`, `format`: summary/json, `analyze`, optional HypoPG `indexes` | +| `execute_sql` | One SQL statement; bounded results or explicitly enabled writes | `sql`, `parameters` (values for `$1..$n`), `read_only`, page | +| `explain_query` | Estimated/actual JSON plan and compact major-node summary | `sql`, `parameters`, `format`: summary/json, `analyze`, optional HypoPG `indexes` | | `analyze_indexes` | Index size, usage, validity, constraints and structural duplicate evidence | `schema`, `table`, page | | `get_top_queries` | Current-database `pg_stat_statements` workload | `order_by`: total_time/mean_time/calls/rows/reads, page | | `analyze_db_health` | Summary or focused PostgreSQL health evidence | `section`: summary/vacuum/index/constraints/sequences/replication/blocking; `schema` where applicable, page | +`parameters` binds up to 256 JSON scalars (string, number, boolean or `null`) to `$1..$n` in order. Values are sent separately from the SQL text, never interpolated, so quotes and statement-like text in a value stay data. Each value is sent as PostgreSQL's untyped `unknown`, so the server infers its type from context exactly as for a quoted literal (`WHERE created_at > $1` with `"2026-01-20"`). Cast where context is ambiguous (`$1::int`, `$1::jsonb`); objects and arrays are refused, so pass JSON documents as strings and cast. Too few values for the highest `$n` returns SQLSTATE `08P01`; extra values, or a skipped placeholder number such as `$2` without `$1`, return `42P18` (the server cannot infer a type for an unreferenced parameter, so cast the placeholders you use and send no more values than the statement references). The server is stateless: to fetch the next page, resend the same `sql` and `parameters` with `next_offset`, which are re-bound on each call. Parameters do not change read-only enforcement. + Routine overloads require the exact `identity_arguments` from `list_objects`, including parameter names; an empty string selects zero arguments. Use `type` to disambiguate relation/routine name collisions. Discovery filters by role privileges; missing or hidden objects return an error. Table definitions are structural fragments, not a round-trip DDL export. What clients and agents are told follows the enforced configuration. In unrestricted mode `execute_sql` is annotated `readOnlyHint: false`, `destructiveHint: true` and its description explains the `read_only=false` write opt-in; in restricted mode it is annotated read-only and its description says writes are refused. All other tools are annotated read-only in both modes. The MCP `initialize` result carries server instructions naming the access mode, whether a database lock applies, and the result/time limits; under a lock, `list_databases` and `execute_sql` descriptions say only listed databases can be selected. Annotations are hints about this server's own enforcement, not a guarantee about side effects of privileged PostgreSQL functions; see [access boundaries](#access-and-resource-boundaries). diff --git a/src/PostgreSqlMcp.Core/SqlExecutor.cs b/src/PostgreSqlMcp.Core/SqlExecutor.cs index 8cb28c8..49efa38 100644 --- a/src/PostgreSqlMcp.Core/SqlExecutor.cs +++ b/src/PostgreSqlMcp.Core/SqlExecutor.cs @@ -10,8 +10,9 @@ public sealed class SqlExecutor(ServerOptions options, DatabaseRegistry registry private readonly SemaphoreSlim _calls = new(options.MaxConcurrentCalls); public Task QueryAsync(string database, string sql, IReadOnlyDictionary? parameters = null, - int? limit = null, int offset = 0, bool readOnly = true, CancellationToken ct = default, string? target = null, bool callerStatement = false) => - WithSessionAsync(database, (session, token) => session.QueryAsync(sql, parameters, limit, offset, token, callerStatement), readOnly, ct, target); + int? limit = null, int offset = 0, bool readOnly = true, CancellationToken ct = default, string? target = null, bool callerStatement = false, + IReadOnlyList? positional = null) => + WithSessionAsync(database, (session, token) => session.QueryAsync(sql, parameters, limit, offset, token, callerStatement, positional), readOnly, ct, target); public async Task WithSessionAsync(string database, Func> action, bool readOnly = true, CancellationToken ct = default, string? target = null) @@ -61,8 +62,9 @@ public sealed class SqlSession(string database, NpgsqlConnection connection, Npg public NpgsqlTransaction Transaction { get; } = transaction; /// The SQL came from the tool caller, so PostgreSQL error positions are reported relative to it. + /// Caller values for $1..$n (see ); exclusive with named parameters. public async Task QueryAsync(string sql, IReadOnlyDictionary? parameters = null, - int? limit = null, int offset = 0, CancellationToken ct = default, bool callerStatement = false) + int? limit = null, int offset = 0, CancellationToken ct = default, bool callerStatement = false, IReadOnlyList? positional = null) { sql = SqlGuard.Validate(sql, out string kind); string statement = sql; @@ -77,18 +79,20 @@ public async Task QueryAsync(string sql, IReadOnlyDictionary ReadAsync(string sql, IReadOnlyDictionary? parameters, int rowLimit, int offset, - bool serverPage, CancellationToken ct) + private async Task ReadAsync(string sql, IReadOnlyDictionary? parameters, IReadOnlyList? positional, + int rowLimit, int offset, bool serverPage, CancellationToken ct) { await using var command = new NpgsqlCommand(sql, Connection, Transaction) { CommandTimeout = options.QueryTimeout }; if (parameters is not null) foreach (var (name, value) in parameters) command.Parameters.AddWithValue(name, value ?? DBNull.Value); + else if (positional is not null) + SqlParameters.Bind(command.Parameters, positional); await using var reader = await command.ExecuteReaderAsync(CommandBehavior.SequentialAccess, ct).ConfigureAwait(false); if (reader.FieldCount > 128) throw new ToolException("result_too_wide", "Select at most 128 columns."); var columns = new ColumnInfo[reader.FieldCount]; diff --git a/src/PostgreSqlMcp.Core/SqlParameters.cs b/src/PostgreSqlMcp.Core/SqlParameters.cs new file mode 100644 index 0000000..3f39774 --- /dev/null +++ b/src/PostgreSqlMcp.Core/SqlParameters.cs @@ -0,0 +1,57 @@ +using System.Text; +using System.Text.Json; +using Npgsql; +using NpgsqlTypes; + +namespace PostgreSqlMcp.Core; + +/// +/// Positional values for caller SQL placeholders ($1..$n). Every value is sent as PostgreSQL type unknown +/// in text form, so the server infers its type from context exactly as for a quoted literal; callers cast +/// ($1::int, $1::jsonb) where the context is ambiguous. Values are never interpolated into SQL text. +/// +public static class SqlParameters +{ + public const int MaxCount = 256; + public const int MaxBytes = 131072; + + /// Converts JSON scalars to their text form; null stays SQL NULL. Objects and arrays are refused. + public static IReadOnlyList? Normalize(JsonElement[]? values) + { + if (values is null || values.Length == 0) return null; + if (values.Length > MaxCount) + throw new ToolException("invalid_parameters", $"Supply at most {MaxCount} parameters."); + var texts = new string?[values.Length]; + long bytes = 0; + for (int i = 0; i < values.Length; i++) + { + JsonElement value = values[i]; + texts[i] = value.ValueKind switch + { + JsonValueKind.Null or JsonValueKind.Undefined => null, + JsonValueKind.String => value.GetString(), + JsonValueKind.Number => value.GetRawText(), + JsonValueKind.True => "true", + JsonValueKind.False => "false", + _ => throw new ToolException("invalid_parameters", + $"Parameter ${i + 1} must be a string, number, boolean or null. Pass JSON documents and arrays as a string and cast, for example ${i + 1}::jsonb.") + }; + if (texts[i] is { } text) + { + if (text.Contains('\0')) + throw new ToolException("invalid_parameters", $"Parameter ${i + 1} must not contain NUL characters."); + bytes += Encoding.UTF8.GetByteCount(text); + } + if (bytes > MaxBytes) + throw new ToolException("invalid_parameters", $"Parameters must total at most {MaxBytes} UTF-8 bytes."); + } + return texts; + } + + /// Adds the values as unnamed (positional) parameters, so Npgsql sends the SQL text unrewritten. + public static void Bind(NpgsqlParameterCollection target, IReadOnlyList values) + { + foreach (string? value in values) + target.Add(new NpgsqlParameter { NpgsqlDbType = NpgsqlDbType.Unknown, Value = (object?)value ?? DBNull.Value }); + } +} diff --git a/src/PostgreSqlMcp.Core/ToolReply.cs b/src/PostgreSqlMcp.Core/ToolReply.cs index f5132f2..708f3b7 100644 --- a/src/PostgreSqlMcp.Core/ToolReply.cs +++ b/src/PostgreSqlMcp.Core/ToolReply.cs @@ -58,6 +58,8 @@ public static async Task Run(string database, Func> "42P01" => "A referenced relation does not exist or is not visible in the current search path.", "42703" => "A referenced column does not exist.", "42883" => "No matching PostgreSQL function or operator was found. Check argument types and search path.", + "08P01" => "PostgreSQL rejected the request as a protocol violation. With parameters, supply a value for every $n placeholder in the statement.", + "42P18" => "PostgreSQL could not determine a parameter's data type. Cast the placeholder, for example $1::int or $1::text. Supplying more values than the highest $n used, or leaving a $n number unused, also causes this: supply exactly the values the statement references.", "22P02" => "A value has invalid syntax for its PostgreSQL type. Check input values and casts.", "22003" => "A numeric value is outside the PostgreSQL type's range.", "22012" => "PostgreSQL rejected division by zero.", diff --git a/src/PostgreSqlMcp.Tools/ParameterText.cs b/src/PostgreSqlMcp.Tools/ParameterText.cs index 90182d7..229f764 100644 --- a/src/PostgreSqlMcp.Tools/ParameterText.cs +++ b/src/PostgreSqlMcp.Tools/ParameterText.cs @@ -10,4 +10,5 @@ internal static class ParameterText public const string IncludeSystem = "Include pg_catalog, information_schema and pg_* schemas."; public const string Sql = "One SQL statement."; public const string SchemaFilter = "Exact schema name; all if omitted."; + public const string Values = "Values for $1..$n placeholders, in order: strings, numbers, booleans or null (max 256). Never interpolated; PostgreSQL infers each type from context like a quoted literal, so cast when ambiguous ($1::int, $1::jsonb)."; } diff --git a/src/PostgreSqlMcp.Tools/PlanTools.cs b/src/PostgreSqlMcp.Tools/PlanTools.cs index fc36e89..c80c744 100644 --- a/src/PostgreSqlMcp.Tools/PlanTools.cs +++ b/src/PostgreSqlMcp.Tools/PlanTools.cs @@ -22,10 +22,12 @@ public Task ExplainQuery(CancellationToken ct, [Description("summary (default): ranked major plan nodes. json: the complete planner JSON within the byte limit.")] string format = "summary", [Description("true executes the statement with EXPLAIN ANALYZE, BUFFERS inside a READ ONLY transaction; not combinable with indexes.")] bool analyze = false, [Description("Up to 16 hypothetical CREATE INDEX statements compared together through the installed hypopg extension.")] string[]? indexes = null, - [Description(ParameterText.Target)] string? target = null) => + [Description(ParameterText.Target)] string? target = null, + [Description(ParameterText.Values)] JsonElement[]? parameters = null) => ToolReply.Run(database, async () => { string statement = SqlGuard.Validate(sql); + IReadOnlyList? values = SqlParameters.Normalize(parameters); bool json = format.Equals("json", StringComparison.OrdinalIgnoreCase); if (!json && !format.Equals("summary", StringComparison.OrdinalIgnoreCase)) throw new ToolException("invalid_format", "format must be summary or json."); @@ -36,7 +38,7 @@ public Task ExplainQuery(CancellationToken ct, { if (candidates.Length == 0) { - using JsonDocument plan = await ReadPlan(session, statement, analyze, token); + using JsonDocument plan = await ReadPlan(session, statement, values, analyze, token); return new { database, analyzed = analyze, format, @@ -61,7 +63,7 @@ public Task ExplainQuery(CancellationToken ct, try { await ExecuteControl(session, reset, token); - using JsonDocument baseline = await ReadPlan(session, statement, false, token); + using JsonDocument baseline = await ReadPlan(session, statement, values, false, token); var hypotheticalIndexes = new List(); // Candidates are independent; send one batch rather than one round trip per index. await using var batch = new NpgsqlBatch(session.Connection, session.Transaction) { Timeout = options.QueryTimeout }; @@ -90,7 +92,7 @@ public Task ExplainQuery(CancellationToken ct, if (candidate + 1 < candidates.Length && !await reader.NextResultAsync(token)) throw new ToolException("invalid_index", "HypoPG did not return a result for each candidate."); } - using JsonDocument evaluated = await ReadPlan(session, statement, false, token); + using JsonDocument evaluated = await ReadPlan(session, statement, values, false, token); PlanSummary before = Summarize(baseline.RootElement); PlanSummary after = Summarize(evaluated.RootElement); return new @@ -160,10 +162,12 @@ private string[] ValidateCandidates(string[]? indexes) [GeneratedRegex(@"\A\s*CREATE\s+(?:UNIQUE\s+)?INDEX\b", RegexOptions.IgnoreCase | RegexOptions.CultureInvariant)] private static partial Regex IndexDefinition { get; } - private async Task ReadPlan(SqlSession session, string sql, bool analyze, CancellationToken ct) + private async Task ReadPlan(SqlSession session, string sql, IReadOnlyList? values, bool analyze, CancellationToken ct) { string prefix = analyze ? "EXPLAIN (FORMAT JSON, ANALYZE TRUE, BUFFERS TRUE, VERBOSE FALSE) " : "EXPLAIN (FORMAT JSON, VERBOSE FALSE) "; await using var command = Command(session, prefix + sql); + // Bound values give PostgreSQL a custom plan for these values, as for the same statement with literals. + if (values is not null) SqlParameters.Bind(command.Parameters, values); NpgsqlDataReader reader; try { reader = await command.ExecuteReaderAsync(CommandBehavior.SequentialAccess | CommandBehavior.SingleRow, ct); } catch (PostgresException ex) when (ToolReply.MarkStatementPosition(ex, prefix.Length, sql)) { throw; } diff --git a/src/PostgreSqlMcp.Tools/SqlTools.cs b/src/PostgreSqlMcp.Tools/SqlTools.cs index eafcb1a..acaad64 100644 --- a/src/PostgreSqlMcp.Tools/SqlTools.cs +++ b/src/PostgreSqlMcp.Tools/SqlTools.cs @@ -1,4 +1,5 @@ using System.ComponentModel; +using System.Text.Json; using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; using PostgreSqlMcp.Core; @@ -10,13 +11,15 @@ public sealed class SqlTools(SqlExecutor executor) { // Annotations and the access-mode part of the description are set by ToolRegistration from the enforced configuration. [McpServerTool(Name = "execute_sql", OpenWorld = false)] - [Description("Run one SQL statement on a physical database or configured profile alias. Optional target selects a connection profile. Returns bounded column/row arrays; for read-only SQL, next_offset re-executes the statement for the next page, so use a stable ORDER BY.")] + [Description("Run one SQL statement on a physical database or configured profile alias. Optional target selects a connection profile. Returns bounded column/row arrays; for read-only SQL, next_offset re-executes the statement for the next page, so use a stable ORDER BY and resend the same sql and parameters. Pass values as parameters ($1..$n) instead of inlining literals.")] public Task ExecuteSql(CancellationToken ct, [Description(ParameterText.Database)] string database, [Description(ParameterText.Sql)] string sql, [Description(ParameterText.Limit)] int? limit = null, [Description(ParameterText.Offset + " Read-only statements only.")] int offset = 0, [Description("true (default): READ ONLY transaction, always rolled back. false: commit a write; refused in restricted access mode.")] bool read_only = true, - [Description(ParameterText.Target)] string? target = null) => - ToolReply.Run(database, async () => await executor.QueryAsync(database, sql, limit: limit, offset: offset, readOnly: read_only, ct: ct, target: target, callerStatement: true).ConfigureAwait(false)); + [Description(ParameterText.Target)] string? target = null, + [Description(ParameterText.Values)] JsonElement[]? parameters = null) => + ToolReply.Run(database, async () => await executor.QueryAsync(database, sql, limit: limit, offset: offset, readOnly: read_only, ct: ct, target: target, + callerStatement: true, positional: SqlParameters.Normalize(parameters)).ConfigureAwait(false)); } diff --git a/tools/PostgreSqlMcp.Verify/Integration.cs b/tools/PostgreSqlMcp.Verify/Integration.cs index 140c9db..eaf536c 100644 --- a/tools/PostgreSqlMcp.Verify/Integration.cs +++ b/tools/PostgreSqlMcp.Verify/Integration.cs @@ -40,6 +40,7 @@ public static async Task RunAsync(string root) await VerifyBoundariesAsync(client); await VerifyPaginationAsync(client); await VerifySqlAsync(client); + await VerifyParametersAsync(client); await FuzzChecks.RunAsync(client); await VerifyCatalogAsync(client); await VerifyOperationsAsync(client, fixture); @@ -243,6 +244,36 @@ private static async Task VerifySqlAsync(McpClient client) Console.WriteLine("PASS SQL guards, read-only DML/CTE protection, quoted literals, syntax errors and timeout"); } + private static async Task VerifyParametersAsync(McpClient client) + { + // Values are bound, never interpolated: a statement-shaped string comes back verbatim. + const string hostile = "x'); DROP TABLE marker; -- \\ \"$1\""; + Check.Equal((await client.OkAsync("execute_sql", new { database = "a", sql = "SELECT $1 AS v, '@named' AS literal", parameters = new object[] { hostile } }))["rows"], + new[] { new[] { hostile, "@named" } }, "A bound string or literal @-text was altered."); + // Untyped values take their type from context, like quoted literals; null and booleans bind as SQL values. + Check.Equal((await client.OkAsync("execute_sql", new + { + database = "a", sql = "SELECT count(*)::int AS c, $3::int IS NULL AS n, $4::bool AS b FROM orders WHERE id > $1 AND id <= $2", + parameters = new object?[] { 1, "3", null, true } + }))["rows"], new[] { new object[] { 2, true, true } }, "Context-typed parameters returned the wrong rows."); + JsonNode paged = await client.OkAsync("execute_sql", new { database = "a", sql = "SELECT g FROM generate_series(1, $1::int) g ORDER BY g", parameters = new object[] { 5 }, limit = 2, offset = 2 }); + Check.That(Check.Rows(paged).Select(row => row["g"]!.GetValue()).SequenceEqual(new[] { 3, 4 }) && paged["next_offset"]!.GetValue() == 4, + "Parameterized pagination did not re-bind values for the next page."); + Check.Equal((await client.OkAsync("execute_sql", new { database = "a", sql = "SELECT $1::jsonb->>'k' AS k", parameters = new object[] { "{\"k\":\"v\"}" } }))["rows"], + new[] { new[] { "v" } }, "A JSON document passed as a string did not cast to jsonb."); + await client.FailsAsync("execute_sql", new { database = "a", sql = "SELECT $1", parameters = new object[] { new { k = 1 } } }, "invalid_parameters"); + await client.FailsAsync("execute_sql", new { database = "a", sql = "SELECT $1", parameters = Enumerable.Repeat(1, 257).ToArray() }, "invalid_parameters"); + await client.FailsAsync("execute_sql", new { database = "a", sql = "SELECT $1, $2", parameters = new object[] { 1 } }, "postgresql_error", "08P01"); + // Surplus values and skipped placeholders leave a parameter untypeable, so PostgreSQL answers 42P18, not 08P01. + await client.FailsAsync("execute_sql", new { database = "a", sql = "SELECT $1", parameters = new object[] { 1, 2 } }, "postgresql_error", "42P18"); + await client.FailsAsync("execute_sql", new { database = "a", sql = "SELECT $2", parameters = new object[] { 1, 2 } }, "postgresql_error", "42P18"); + // Parameters do not bypass the read-only transaction. + await client.FailsAsync("execute_sql", new { database = "a", sql = "INSERT INTO marker VALUES ($1)", parameters = new object[] { "BAD" } }, "postgresql_error", "25006"); + JsonNode plan = await client.OkAsync("explain_query", new { database = "a", sql = "SELECT * FROM orders WHERE id = $1", parameters = new object[] { 1 } }); + Check.That(plan["plan"]?["node_count"]?.GetValue() >= 1, "explain_query did not plan a parameterized statement."); + Console.WriteLine("PASS bound parameters: verbatim strings, context typing, NULL/boolean, re-bound pages, jsonb, limits, count mismatch, read-only and EXPLAIN"); + } + private static async Task VerifyCatalogAsync(McpClient client) { var schemas = Check.Rows(await client.OkAsync("list_schemas", new { database = "a", prefix = "app" })).Select(row => row["schema_name"].Text()); @@ -363,6 +394,12 @@ private static async Task VerifyWritesAsync(Command command, Dictionary