From 85a1f5b274fd84ad37de5dd935607ca9d3c6ec20 Mon Sep 17 00:00:00 2001 From: AG <288536783+codegiveness@users.noreply.github.com> Date: Sat, 10 Oct 2026 20:51:02 +0700 Subject: [PATCH 1/2] feat: warn about privileged logins and unverified remote TLS Startup and --validate warn when a remote profile does not verify the server certificate; --validate warns when a login holds superuser, server-file, signal, checkpoint, REPLICATION or BYPASSRLS capabilities that READ ONLY transactions do not contain. Warnings never block startup. --- CHANGELOG.md | 1 + README.md | 2 +- SECURITY.md | 4 +- src/PostgreSqlMcp.Core/SecurityWarnings.cs | 92 ++++++++++++++++++++++ src/PostgreSqlMcp/Program.cs | 4 + tools/PostgreSqlMcp.Verify/Integration.cs | 34 ++++++++ 6 files changed, 135 insertions(+), 2 deletions(-) create mode 100644 src/PostgreSqlMcp.Core/SecurityWarnings.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 67b7f41..9d2bfa3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- Security warnings that never block startup: startup and `--validate` print a `[warning]` on stderr for every profile that reaches a non-loopback host without certificate verification (`SSL Mode` other than `VerifyCA`/`VerifyFull`, including the Npgsql default `Prefer`); `--validate` also warns when a configured login is or can become a superuser, belongs to `pg_execute_server_program`, `pg_write_server_files`, `pg_read_server_files`, `pg_signal_backend` or `pg_checkpoint`, or reaches a `REPLICATION`/`BYPASSRLS` role. These capabilities act through `SELECT` inside READ ONLY transactions, so restricted mode and read-only annotations do not contain them. Warnings name profiles and roles only, never connection strings. - 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 d39e277..55db829 100644 --- a/README.md +++ b/README.md @@ -228,7 +228,7 @@ At the hidden prompt, quoting has **one layer: Npgsql**, not JSON or shell synta If your provider supplies only a PostgreSQL URL, map its host, port, database, username and password to the example's fields, decode URL-escaped values, and preserve required TLS/authentication options. The URL itself cannot be entered unchanged. -For a remote server needing a provider CA certificate, retain `SSL Mode=VerifyFull` and add `Root Certificate=`. Follow your provider's [TLS requirements](https://www.npgsql.org/doc/security.html); do not disable TLS to work around a remote certificate error. For PostgreSQL on **this same computer** without TLS, use `Host=127.0.0.1` and replace `SSL Mode=VerifyFull` with `SSL Mode=Disable`. +For a remote server needing a provider CA certificate, retain `SSL Mode=VerifyFull` and add `Root Certificate=`. Follow your provider's [TLS requirements](https://www.npgsql.org/doc/security.html); do not disable TLS to work around a remote certificate error. A remote profile without `VerifyCA`/`VerifyFull` (including the Npgsql default, `Prefer`) still works but prints a `[warning]` at startup and in `--validate`; `--validate` also warns when the login is a superuser or holds server-file, signal or checkpoint roles ([details](SECURITY.md#credentials-and-diagnostics)). For PostgreSQL on **this same computer** without TLS, use `Host=127.0.0.1` and replace `SSL Mode=VerifyFull` with `SSL Mode=Disable`. ### Upgrading an existing installation diff --git a/SECURITY.md b/SECURITY.md index 9d51d9c..ae95cd0 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -12,6 +12,8 @@ Live `list_databases` uses the catalog of the profile's bootstrap database (or, Use dedicated, least-privileged PostgreSQL roles, not superusers or database owners. Grant only the required database connection, schema usage, table access, and routine execution privileges. Configure row-level security and separate roles where required by the application's data model. Review role memberships, default privileges, `PUBLIC` grants, `SECURITY DEFINER` routines, extensions, foreign servers, and monitoring privileges. +Read-only transactions and restricted mode stop SQL writes, not every side effect a privileged login can cause: a superuser or a member of `pg_execute_server_program`, `pg_write_server_files`, `pg_read_server_files`, `pg_signal_backend` or `pg_checkpoint` can read or write server files, run programs, terminate other sessions or force checkpoints from a `SELECT`, and roles with `REPLICATION` or `BYPASSRLS` widen what can be read. `--validate` prints a non-blocking `[warning]` line for each such capability of every configured login, so tool annotations that say read-only can be checked against what the role can actually do. + **Unrestricted access is the default when access mode is omitted.** SQL requests still default to `read_only=true`; read-only operations use a server-owned read-only transaction and roll it back after reading the result. Writes require `read_only=false` on `execute_sql`, unrestricted mode and sufficient PostgreSQL privileges. Opt into `POSTGRES_ACCESS_MODE=restricted` or `--access-mode restricted` to refuse write requests; existing explicit restricted settings remain effective. Enabled writes use a transaction that commits after successful execution and result reading; failures dispose the transaction without committing. Other tools remain read-only. Connection loss or cancellation during commit can leave the caller uncertain whether a write committed; inspect database state before retrying. The server does not automatically replay writes. The SQL guard is a statement-boundary lexer, **not an authorization parser or SQL sandbox**. It accepts one statement, handles quoted SQL and comments, and rejects direct transaction/session control plus unsupported operations such as `COPY`, `DO`, `CALL`, and `VACUUM`. PostgreSQL transactions and role permissions enforce the actual access restrictions. @@ -131,7 +133,7 @@ These protections are not content redaction for successful results. Queries, pla ## Connection and resource settings -TLS and certificate trust are operator-controlled Npgsql connection-string settings. The server does not force TLS or verify certificates on the operator's behalf. For remote production connections, configure certificate validation appropriate to the deployment, such as `SSL Mode=VerifyFull`, and provision the necessary trust material. Use network controls to restrict database reachability. +TLS and certificate trust are operator-controlled Npgsql connection-string settings. The server does not force TLS or verify certificates on the operator's behalf. For remote production connections, configure certificate validation appropriate to the deployment, such as `SSL Mode=VerifyFull`, and provision the necessary trust material. Npgsql's default `SSL Mode=Prefer`, like `Require`, encrypts without verifying the server, so an on-path attacker can impersonate it and capture the login; startup and `--validate` print a non-blocking `[warning]` for each profile that reaches a non-loopback host without `VerifyCA` or `VerifyFull` (Unix sockets and loopback addresses are exempt). Use network controls to restrict database reachability. The server owns pool bounds, reset-on-close, transaction enlistment, multiplexing, application name, command timeouts, and logging safety settings. Configuration with `No Reset On Close=true` or multiplexing is rejected. Operation deadlines include queue and connection-pool wait; transactions also set statement and lock timeouts. Result row, byte, cell, and column limits bound responses. These are resource controls, not a guarantee against expensive queries, external side effects, or denial of service. Apply PostgreSQL and operating-system resource controls where needed. diff --git a/src/PostgreSqlMcp.Core/SecurityWarnings.cs b/src/PostgreSqlMcp.Core/SecurityWarnings.cs new file mode 100644 index 0000000..bca6a50 --- /dev/null +++ b/src/PostgreSqlMcp.Core/SecurityWarnings.cs @@ -0,0 +1,92 @@ +using System.Net; +using Npgsql; + +namespace PostgreSqlMcp.Core; + +/// +/// Non-fatal operator warnings for configurations whose enforced behavior is weaker than the advertised one: +/// unverified TLS to remote hosts, and logins whose privileges reach past READ ONLY transactions. +/// Messages name profiles and roles only, never connection strings or credentials. +/// +public static class SecurityWarnings +{ + // Each capability can cause effects that SET TRANSACTION READ ONLY and restricted mode do not prevent. + // A superuser is a "member" of every role, so its other rows would only repeat the first finding. + private const string RoleSql = """ + WITH super AS ( + SELECT r.rolname FROM pg_catalog.pg_roles r + WHERE r.rolsuper AND pg_catalog.pg_has_role(session_user, r.oid, 'MEMBER') + ) + SELECT warning FROM ( + SELECT 1, 'login is or can SET ROLE to superuser ' || quote_ident(rolname) FROM super + UNION ALL + SELECT 2, 'login is a member of ' || r.rolname FROM pg_catalog.pg_roles r + WHERE r.rolname IN ('pg_execute_server_program', 'pg_write_server_files', 'pg_read_server_files', 'pg_signal_backend', 'pg_checkpoint') + AND pg_catalog.pg_has_role(session_user, r.oid, 'MEMBER') + UNION ALL + SELECT 3, 'role ' || quote_ident(r.rolname) || ' has REPLICATION' FROM pg_catalog.pg_roles r + WHERE r.rolreplication AND pg_catalog.pg_has_role(session_user, r.oid, 'MEMBER') + UNION ALL + SELECT 4, 'role ' || quote_ident(r.rolname) || ' has BYPASSRLS' FROM pg_catalog.pg_roles r + WHERE r.rolbypassrls AND pg_catalog.pg_has_role(session_user, r.oid, 'MEMBER') + ) AS warnings(severity, warning) + WHERE severity = 1 OR NOT EXISTS (SELECT 1 FROM super) + ORDER BY severity, warning COLLATE "C" + """; + + /// Profiles that reach a non-local host without certificate verification (Npgsql's default SSL Mode is Prefer). + public static IReadOnlyList Tls(ServerOptions options) + { + var warnings = new List(); + var seen = new HashSet<(string Host, SslMode Mode)>(); + foreach (var (alias, connectionString) in options.Targets.OrderBy(pair => pair.Key, StringComparer.Ordinal)) + { + var builder = new NpgsqlConnectionStringBuilder(connectionString); + if (builder.SslMode is SslMode.VerifyCA or SslMode.VerifyFull) continue; + string host = builder.Host ?? ""; + if (host.Split(',').All(IsLocal) || !seen.Add((host, builder.SslMode))) continue; + warnings.Add($"profile {alias}: SSL Mode={builder.SslMode} to a non-local host does not verify the server certificate, " + + "so a network attacker can impersonate the server. Use SSL Mode=VerifyFull with the provider's CA certificate."); + } + return warnings; + } + + /// Privileged capabilities of each distinct login, checked through the database the server would open. + public static async Task> RolesAsync(DatabaseRegistry registry, SqlExecutor executor, CancellationToken ct = default) + { + var warnings = new List(); + foreach (var (target, _) in registry.LoginTargets()) + { + try + { + List found = await executor.WithSessionAsync(registry.GetDiscoveryDatabase(target), async (session, token) => + { + await using var command = new NpgsqlCommand(RoleSql, session.Connection, session.Transaction); + await using var reader = await command.ExecuteReaderAsync(token).ConfigureAwait(false); + var values = new List(); + while (await reader.ReadAsync(token).ConfigureAwait(false)) values.Add(reader.GetString(0)); + return values; + }, ct: ct, target: target).ConfigureAwait(false); + foreach (string warning in found) + warnings.Add($"target {target}: {warning}. READ ONLY transactions and restricted mode do not stop its privileged functions or side effects; use a least-privileged role."); + } + // Connectivity is reported by --validate itself; an unreadable catalog only means no warning. + catch (Exception ex) when (ex is ToolException or NpgsqlException or TimeoutException) { } + } + return warnings; + } + + private static bool IsLocal(string entry) + { + string host = entry.Trim(); + if (host.Length == 0 || host[0] is '/' or '@') return true; // Unix-domain socket directory or abstract socket. + if (host.StartsWith('[')) + { + int close = host.IndexOf(']'); + host = close > 0 ? host[1..close] : host; + } + else if (host.Count(c => c == ':') == 1) host = host[..host.IndexOf(':')]; // host:port + return host.Equals("localhost", StringComparison.OrdinalIgnoreCase) + || (IPAddress.TryParse(host, out IPAddress? address) && IPAddress.IsLoopback(address)); + } +} diff --git a/src/PostgreSqlMcp/Program.cs b/src/PostgreSqlMcp/Program.cs index 05feb4d..edcac07 100644 --- a/src/PostgreSqlMcp/Program.cs +++ b/src/PostgreSqlMcp/Program.cs @@ -45,6 +45,8 @@ Targets JSON wins over file. if (!Enum.TryParse(options.LogLevel, true, out var logLevel) || !Enum.IsDefined(logLevel)) { Console.Error.WriteLine("[startup] Invalid log level."); return 1; } ToolReply.Configure(options); +// Warnings never block startup: they name configurations whose real exposure exceeds what tools advertise. +foreach (string warning in SecurityWarnings.Tls(options)) Console.Error.WriteLine($"[warning] {warning}"); static async Task CheckDatabaseLockAsync(DatabaseRegistry registry, SqlExecutor executor) { if (registry.AllowedDatabaseNames is null) return true; @@ -78,6 +80,8 @@ static async Task CheckDatabaseLockAsync(DatabaseRegistry registry, SqlExe Console.Error.WriteLine(result.StructuredContent?.GetRawText()); ok &= result.IsError != true; } + foreach (string warning in await SecurityWarnings.RolesAsync(registry, executor)) + Console.Error.WriteLine($"[warning] {warning}"); ok &= await CheckDatabaseLockAsync(registry, executor); return ok ? 0 : 1; } diff --git a/tools/PostgreSqlMcp.Verify/Integration.cs b/tools/PostgreSqlMcp.Verify/Integration.cs index bbfeb8f..d0a5b2e 100644 --- a/tools/PostgreSqlMcp.Verify/Integration.cs +++ b/tools/PostgreSqlMcp.Verify/Integration.cs @@ -50,6 +50,7 @@ public static async Task RunAsync(string root) Console.WriteLine("PASS PostgreSQL error and trace-level request/result confidentiality"); await VerifyWritesAsync(command, environment, fixture.Port); await VerifyConfigurationAsync(command, environment, targets, connection); + await VerifySecurityWarningsAsync(command, environment, fixture); await VerifyDiscoveryAsync(command, environment, fixture, connection); await VerifyPoolCapacityAsync(command, environment, fixture, connection); // Last: hardening revokes PUBLIC CONNECT on every fixture database. @@ -493,6 +494,7 @@ private static async Task VerifyConfigurationAsync(Command command, Dictionary JsonNode.Parse(line)!).ToArray(); Check.That(reports.Select(report => report["database"].Text()).ToHashSet().SetEquals(new[] { "a", "b" }) && reports.Select(report => report["rows"]![0]![0].Text()).ToHashSet().SetEquals(new[] { "tenant_a", "tenant_b" }), "Preflight did not validate both configured targets."); @@ -540,4 +542,36 @@ private static async Task VerifyConfigurationAsync(Command command, Dictionary environment, PostgresFixture fixture) + { + // Reuses the fixture's synthetic reader password, which the secret-scanner configuration already allows here. + await fixture.SqlAsync("postgres", "CREATE ROLE mcp_signal LOGIN PASSWORD 'reader-disposable' IN ROLE pg_signal_backend; GRANT CONNECT ON DATABASE tenant_a TO mcp_signal;"); + try + { + var privileged = new Dictionary(environment) + { + ["POSTGRES_TARGETS"] = JsonSerializer.Serialize(new { a = $"Host=127.0.0.1;Port={fixture.Port};Username=mcp_signal;Password=reader-disposable;Database=tenant_a" }) + }; + ProcessResult result = await Processes.RunAsync(command.With("--validate"), privileged); + Check.That(result.Output.Length == 0, "Privilege warning contaminated protocol stdout."); + Check.That(result.Error.Contains("[warning] target a: login is a member of pg_signal_backend.", StringComparison.Ordinal), + "--validate did not warn about a login whose privileges exceed READ ONLY transactions."); + Check.Confidential(result.Error, "reader-disposable"); + } + finally { await fixture.SqlAsync("postgres", "DROP OWNED BY mcp_signal; DROP ROLE mcp_signal;"); } + // Name resolution of .invalid fails without network traffic; the warning comes from configuration alone. + // The login name is the marker: no part of the connection string may appear in a warning. + const string remote = "Host=tls-warning.invalid;Username=sensitive-tls-marker;Database=d;Timeout=2"; + foreach (var (mode, warns) in new[] { ("", true), (";SSL Mode=Require", true), (";SSL Mode=VerifyFull", false) }) + { + var tls = new Dictionary(environment) { ["POSTGRES_TARGETS"] = JsonSerializer.Serialize(new { remote = remote + mode }) }; + ProcessResult result = await Processes.RunAsync(command.With("--validate"), tls, expected: 1); + Check.That(result.Error.Contains("[warning] profile remote: SSL Mode=", StringComparison.Ordinal) == warns, + $"Unverified-TLS warning was {(warns ? "missing" : "emitted")} for '{mode}'."); + Check.Confidential(result.Error, "sensitive-tls-marker"); + } + Console.WriteLine("PASS privileged-login and unverified-TLS warnings, non-blocking and credential-free"); + } } From 3df701fe67dd5df57b31272bf3bbdb604b8115a6 Mon Sep 17 00:00:00 2001 From: AG <288536783+codegiveness@users.noreply.github.com> Date: Sat, 10 Oct 2026 22:52:57 +0700 Subject: [PATCH 2/2] fix: name every unverified-TLS profile, never crash on warning checks --- src/PostgreSqlMcp.Core/SecurityWarnings.cs | 22 ++++++++++++++++------ tools/PostgreSqlMcp.Verify/Integration.cs | 16 ++++++++++++++++ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/PostgreSqlMcp.Core/SecurityWarnings.cs b/src/PostgreSqlMcp.Core/SecurityWarnings.cs index bca6a50..64b5600 100644 --- a/src/PostgreSqlMcp.Core/SecurityWarnings.cs +++ b/src/PostgreSqlMcp.Core/SecurityWarnings.cs @@ -37,15 +37,25 @@ WHERE r.rolbypassrls AND pg_catalog.pg_has_role(session_user, r.oid, 'MEMBER') /// Profiles that reach a non-local host without certificate verification (Npgsql's default SSL Mode is Prefer). public static IReadOnlyList Tls(ServerOptions options) { - var warnings = new List(); - var seen = new HashSet<(string Host, SslMode Mode)>(); + // One line per distinct server login (host, port, user, mode) naming its profiles: lock aliases of one connection + // string share a line, while profiles that reach different ports or users are each named. + var groups = new Dictionary<(string Host, int Port, string? User, SslMode Mode), List>(); foreach (var (alias, connectionString) in options.Targets.OrderBy(pair => pair.Key, StringComparer.Ordinal)) { var builder = new NpgsqlConnectionStringBuilder(connectionString); if (builder.SslMode is SslMode.VerifyCA or SslMode.VerifyFull) continue; string host = builder.Host ?? ""; - if (host.Split(',').All(IsLocal) || !seen.Add((host, builder.SslMode))) continue; - warnings.Add($"profile {alias}: SSL Mode={builder.SslMode} to a non-local host does not verify the server certificate, " + + if (host.Split(',').All(IsLocal)) continue; + var key = (host, builder.Port, builder.Username, builder.SslMode); + if (!groups.TryGetValue(key, out List? aliases)) groups[key] = aliases = []; + aliases.Add(alias); + } + var warnings = new List(); + foreach (var ((_, _, _, mode), aliases) in groups) + { + string names = aliases.Count == 1 ? $"profile {aliases[0]}" + : $"profiles {string.Join(", ", aliases.Take(5))}{(aliases.Count > 5 ? $" and {aliases.Count - 5} more" : "")}"; + warnings.Add($"{names}: SSL Mode={mode} to a non-local host does not verify the server certificate, " + "so a network attacker can impersonate the server. Use SSL Mode=VerifyFull with the provider's CA certificate."); } return warnings; @@ -70,8 +80,8 @@ public static async Task> RolesAsync(DatabaseRegistry regi foreach (string warning in found) warnings.Add($"target {target}: {warning}. READ ONLY transactions and restricted mode do not stop its privileged functions or side effects; use a least-privileged role."); } - // Connectivity is reported by --validate itself; an unreadable catalog only means no warning. - catch (Exception ex) when (ex is ToolException or NpgsqlException or TimeoutException) { } + // Connectivity is reported by --validate itself; any failure here only means no warning, never a crash. + catch (Exception) when (!ct.IsCancellationRequested) { } } return warnings; } diff --git a/tools/PostgreSqlMcp.Verify/Integration.cs b/tools/PostgreSqlMcp.Verify/Integration.cs index d0a5b2e..ce1cec2 100644 --- a/tools/PostgreSqlMcp.Verify/Integration.cs +++ b/tools/PostgreSqlMcp.Verify/Integration.cs @@ -572,6 +572,22 @@ private static async Task VerifySecurityWarningsAsync(Command command, Dictionar $"Unverified-TLS warning was {(warns ? "missing" : "emitted")} for '{mode}'."); Check.Confidential(result.Error, "sensitive-tls-marker"); } + // Profiles on the same host but different ports are different servers: both are named. A connection-string lock's + // aliases share one connection, so they share one line instead of repeating it per locked name. + var ports = new Dictionary(environment) + { + ["POSTGRES_TARGETS"] = JsonSerializer.Serialize(new { one = remote, two = remote + ";Port=6543" }) + }; + ProcessResult grouped = await Processes.RunAsync(command.With("--validate"), ports, expected: 1); + Check.That(grouped.Error.Contains("[warning] profile one: SSL Mode=", StringComparison.Ordinal) + && grouped.Error.Contains("[warning] profile two: SSL Mode=", StringComparison.Ordinal), + "A remote profile on another port was not named in an unverified-TLS warning."); + var locked = new Dictionary(environment) { ["POSTGRES_CONNECTION_STRING"] = remote, ["POSTGRES_DATABASES"] = "[\"d1\",\"d2\"]" }; + locked.Remove("POSTGRES_TARGETS"); + ProcessResult shared = await Processes.RunAsync(command.With("--validate"), locked, expected: 1); + Check.That(shared.Error.Contains("[warning] profiles d1, d2: SSL Mode=", StringComparison.Ordinal), + "Lock aliases of one connection string did not share one unverified-TLS warning."); + Check.Confidential(grouped.Error + shared.Error, "sensitive-tls-marker"); Console.WriteLine("PASS privileged-login and unverified-TLS warnings, non-blocking and credential-free"); } }