From 5cbc85cd67d335e603af15e5b81a1fe1dcb83a2f Mon Sep 17 00:00:00 2001 From: Ridvan Sirma Date: Mon, 21 Sep 2026 13:14:32 +0000 Subject: [PATCH 1/4] pgduck: log the engine error class from pgduck_server pgduck_server already maps DuckDB error types onto the ErrorResponse SQLSTATE, but the canned class derived from it was only logged by Postgres. Emit the same class from the engine too, so it is available in the engine's own log for failures that never reach a client cleanly. The line holds a class from a fixed literal set and nothing else. Every other line here may carry the DuckDB message, up to 500 characters of the statement, or the client address, so anything collecting these lines has to match the prefix rather than the process. --no_log_engine_errors turns the line off; the SQLSTATE and error text are unaffected either way. Startup announces the disabled state alongside the other non-default options, since an absent class line otherwise reads as no errors having occurred. That announcement names pgduck_engine_error without the trailing colon so it does not match a prefix-based collector itself. Signed-off-by: Ridvan Sirma Co-authored-by: Cursor --- .../include/command_line/command_line.h | 1 + pgduck_server/include/pgsession/pgsession.h | 1 + pgduck_server/src/command_line/command_line.c | 9 + pgduck_server/src/main.c | 1 + pgduck_server/src/pgsession/pgsession.c | 53 +++++ .../pytests/test_engine_error_class_log.py | 209 ++++++++++++++++++ 6 files changed, 274 insertions(+) create mode 100644 pgduck_server/tests/pytests/test_engine_error_class_log.py diff --git a/pgduck_server/include/command_line/command_line.h b/pgduck_server/include/command_line/command_line.h index 3631cf460..c7622314c 100644 --- a/pgduck_server/include/command_line/command_line.h +++ b/pgduck_server/include/command_line/command_line.h @@ -43,6 +43,7 @@ typedef struct char *cache_dir; char *extensions_dir; bool no_extension_install; + bool no_log_engine_errors; bool debug; char *init_file_path; char *pidfile_path; diff --git a/pgduck_server/include/pgsession/pgsession.h b/pgduck_server/include/pgsession/pgsession.h index 604388f5c..018ed0fb7 100644 --- a/pgduck_server/include/pgsession/pgsession.h +++ b/pgduck_server/include/pgsession/pgsession.h @@ -181,5 +181,6 @@ typedef struct PGSession extern void *pgsession_handle_connection(void *input); extern int oom_is_fatal; +extern bool log_engine_errors; #endif /* // PGDUCK_PG_SESSION_H */ diff --git a/pgduck_server/src/command_line/command_line.c b/pgduck_server/src/command_line/command_line.c index 6f01c9c1d..3cd16ed06 100644 --- a/pgduck_server/src/command_line/command_line.c +++ b/pgduck_server/src/command_line/command_line.c @@ -119,6 +119,7 @@ print_usage() printf(" --extensions_dir Install and load extensions in the specified directory\n"); printf(" --pidfile Write the pid of this program to the given path\n"); printf(" --no_extension_install Disable extension installation\n"); + printf(" --no_log_engine_errors Do not log an error class for failing queries\n"); printf(" --debug Include debug-level log messages (including full queries) in server output\n"); printf(" --verbose Run in verbose mode\n"); printf(" --help Display this help and exit\n"); @@ -146,6 +147,7 @@ parse_arguments(int argc, char *argv[]) .cache_dir = NULL, .extensions_dir = NULL, .no_extension_install = false, + .no_log_engine_errors = false, .debug = false, }; int opt; @@ -167,6 +169,7 @@ parse_arguments(int argc, char *argv[]) {"cache_dir", required_argument, NULL, 'C'}, {"extensions_dir", required_argument, NULL, 'E'}, {"no_extension_install", no_argument, NULL, 'n'}, + {"no_log_engine_errors", no_argument, NULL, 'e'}, {"init_file_path", required_argument, NULL, 'i'}, {"pidfile", required_argument, NULL, 'p'}, {"debug", no_argument, NULL, 'd'}, @@ -233,6 +236,9 @@ parse_arguments(int argc, char *argv[]) case 'n': options.no_extension_install = true; break; + case 'e': + options.no_log_engine_errors = true; + break; case 'P': { int inputPort = 0; @@ -320,6 +326,9 @@ parse_arguments(int argc, char *argv[]) if (options.no_extension_install) PGDUCK_SERVER_LOG("Using local extension binaries only"); + if (options.no_log_engine_errors) + PGDUCK_SERVER_LOG("Engine error classification is off; no pgduck_engine_error lines will be emitted"); + if (options.debug) PGDUCK_SERVER_LOG("Debugging mode on; will log all queries"); diff --git a/pgduck_server/src/main.c b/pgduck_server/src/main.c index 678bfd26f..236d6ed25 100644 --- a/pgduck_server/src/main.c +++ b/pgduck_server/src/main.c @@ -63,6 +63,7 @@ main(int argc, char *argv[]) pgduck_log_min_messages = DEBUG1; oom_is_fatal = !options.continue_on_oom; + log_engine_errors = !options.no_log_engine_errors; /* first, make sure duckdb is accessible */ DuckDBStatus duckDbStatus = duckdb_global_init(options.duckdb_database_file_path, diff --git a/pgduck_server/src/pgsession/pgsession.c b/pgduck_server/src/pgsession/pgsession.c index 452450c39..7d67a21d5 100644 --- a/pgduck_server/src/pgsession/pgsession.c +++ b/pgduck_server/src/pgsession/pgsession.c @@ -121,6 +121,9 @@ static bool is_transmit_query(const char *queryString); /* global flag on whether to exit on OOM */ int oom_is_fatal = true; +/* global flag on whether to log a class for engine errors */ +bool log_engine_errors = true; + /* * Per-client entrance point for the pgsession logic. * @@ -878,6 +881,46 @@ process_execute_message(PGSession * pgSession, StringInfo inputMessage) } +/* + * PGDUCK_ENGINE_ERROR_PREFIX marks log lines that carry only a canned error + * class, so a log collector can match on the prefix alone and collect nothing + * else from this process. The text after the prefix must always come from the + * fixed literal set in error_class_for_sqlstate -- never interpolate DuckDB + * text, a statement, a URL, or client identity into it. Every other log line + * here may carry all of those. + */ +#define PGDUCK_ENGINE_ERROR_PREFIX "pgduck_engine_error: " + +/* + * error_class_for_sqlstate maps a SQLSTATE this server reports to a PII-free + * class. Codes we do not map are "other", and NULL is one of them. + * + * NULL means no error type was recorded; pgsession_send_postgres_error reports + * it to the client as feature_not_supported, which is not a category worth a + * class of its own. + */ +static const char * +error_class_for_sqlstate(const char *sqlState) +{ + if (sqlState == NULL) + return "other"; + + if (strcmp(sqlState, PGDUCK_SQLSTATE_OUT_OF_MEMORY) == 0) + return "out_of_memory"; + + if (strcmp(sqlState, PGDUCK_SQLSTATE_IO_ERROR) == 0) + return "io_error"; + + if (strcmp(sqlState, PGDUCK_SQLSTATE_INVALID_PARAMETER) == 0) + return "invalid_input"; + + if (strcmp(sqlState, PGDUCK_SQLSTATE_INTERNAL_ERROR) == 0) + return "internal_error"; + + return "other"; +} + + /* * sqlstate_for_status maps the statuses that are raised without a DuckDB error * type, so no SQLSTATE was recorded on the session. NULL leaves the default. @@ -913,6 +956,16 @@ handle_pgsession_error_message(DuckDBStatus status, PGSession * pgSession, char pgSession->duckSession.errorSqlState = NULL; + /* + * Every reportable status passes through here, including the fatal ones + * the caller exits on, so one line here covers all of them. + */ + if (log_engine_errors) + { + PGDUCK_SERVER_LOG(PGDUCK_ENGINE_ERROR_PREFIX "%s", + error_class_for_sqlstate(sqlState)); + } + switch (status) { case DUCKDB_QUERY_ERROR: diff --git a/pgduck_server/tests/pytests/test_engine_error_class_log.py b/pgduck_server/tests/pytests/test_engine_error_class_log.py new file mode 100644 index 000000000..57c07f3c4 --- /dev/null +++ b/pgduck_server/tests/pytests/test_engine_error_class_log.py @@ -0,0 +1,209 @@ +"""Coverage for the classified log line pgduck_server emits for engine errors. + +The line carries a canned class and nothing else, so that a log collector can +match on its prefix and collect nothing else from this process. Every other line +pgduck_server writes may legitimately contain the DuckDB message, the failing +statement, or the client address, which is why the assertions here isolate the +classified line instead of searching the whole output. +""" + +import os +import tempfile +from contextlib import contextmanager + +import pytest +from utils_pytest import * + + +PGDUCK_UNIX_DOMAIN_PATH = "/tmp" +PGDUCK_PORT = 8259 # its own port, so a shared server cannot answer instead +DUCKDB_DATABASE_FILE_PATH = "/tmp/pgduck_engine_error_class.db" + +ENGINE_ERROR_PREFIX = "pgduck_engine_error: " + +# Spill setup copied from test_server_start.py: a 0-byte temp cap makes the +# overflow deterministic, while a memory_limit well above DuckDB's block size and +# threads=1 keep it a recoverable temp-cap OOM rather than a fatal memory OOM. +SPILL_MEMORY_LIMIT = "32MB" +SPILL_QUERY = "SELECT count(*) FROM (SELECT i FROM range(10000000) t(i) GROUP BY i) g" + +# A path that does not exist, distinctive enough that finding it on the classified +# line can only mean the line leaked its query. +MISSING_FILE = "/tmp/pgduck_class_log_absent_9f3c1d.parquet" + + +def _connect(): + conn = psycopg2.connect(host=PGDUCK_UNIX_DOMAIN_PATH, port=PGDUCK_PORT) + conn.autocommit = True + return conn + + +def _start_server(need_output=True, extra_args=None): + """Start a server and wait for it to accept connections. + + pgduck_server binds its socket only after DuckDB is initialized, so + connecting without this wait races that startup. + """ + server = PgDuckServer( + port=PGDUCK_PORT, + duckdb_database_file_path=DUCKDB_DATABASE_FILE_PATH, + need_output=need_output, + extra_args=extra_args, + ) + assert is_server_listening(server.socket_path), "pgduck_server did not start" + return server + + +@contextmanager +def _spill_server(extra_args=None): + """Start a server whose spill knobs come from an init file, as production does.""" + with tempfile.TemporaryDirectory(dir="/tmp") as cfg_dir: + init_file = os.path.join(cfg_dir, "init.sql") + with open(init_file, "w") as f: + f.write(f"SET GLOBAL memory_limit='{SPILL_MEMORY_LIMIT}';\n") + f.write("SET GLOBAL threads='1';\n") + f.write("SET GLOBAL max_temp_directory_size='0KiB';\n") + yield _start_server( + extra_args=["--init_file_path", init_file] + list(extra_args or []) + ) + + +def _class_lines(server): + output = get_server_output(server.output_queue) + return [line for line in output.splitlines() if ENGINE_ERROR_PREFIX in line] + + +def _assert_class(server, expected, *must_not_appear): + """Assert the expected class was logged, and that the line carries nothing else. + + *must_not_appear* holds fragments of the statement and of the DuckDB message. + They are checked against the classified lines only: the neighbouring WARNING + lines contain all of them by design and would mask a leak. + """ + lines = _class_lines(server) + + assert lines, f"pgduck_server logged no {ENGINE_ERROR_PREFIX!r} line" + + assert any( + ENGINE_ERROR_PREFIX + expected in line for line in lines + ), f"expected class {expected!r}, got: {lines!r}" + + for line in lines: + for fragment in must_not_appear: + assert fragment not in line, ( + f"classified line leaked {fragment!r}, which must never reach a " + f"collected log line: {line!r}" + ) + + +def test_io_error_class_is_logged_without_the_path(): + """A missing file is an IO error, and the path must not ride along.""" + server = _start_server() + + cur = _connect().cursor() + with pytest.raises(psycopg2.Error) as exc_info: + cur.execute(f"SELECT * FROM read_parquet('{MISSING_FILE}')") + + assert ( + exc_info.value.pgcode == "58030" + ), f"missing file should report SQLSTATE 58030, got {exc_info.value.pgcode}" + + _assert_class(server, "io_error", MISSING_FILE, "read_parquet") + + +def test_invalid_input_class_is_logged_without_the_value(): + """A bad format string is invalid input; neither it nor the value may appear.""" + server = _start_server() + cur = _connect().cursor() + with pytest.raises(psycopg2.Error) as exc_info: + cur.execute("SELECT strptime('not-a-date', '%Y-%m-%d')") + + assert ( + exc_info.value.pgcode == "22023" + ), f"invalid input should report SQLSTATE 22023, got {exc_info.value.pgcode}" + + _assert_class(server, "invalid_input", "not-a-date", "strptime") + + +def test_invalid_input_class_is_logged_over_extended_protocol(): + """A bind parameter puts psycopg2 on Parse/Bind/Execute, the path pg_lake uses.""" + server = _start_server() + cur = _connect().cursor() + with pytest.raises(psycopg2.Error) as exc_info: + cur.execute("SELECT strptime(%s, %s)", ("not-a-date", "%Y-%m-%d")) + + assert ( + exc_info.value.pgcode == "22023" + ), f"invalid input should report SQLSTATE 22023, got {exc_info.value.pgcode}" + + _assert_class(server, "invalid_input", "not-a-date", "strptime") + + +def test_unmapped_error_is_logged_as_other(): + """A catalog error is not a class we distinguish, so it lands in "other".""" + server = _start_server() + cur = _connect().cursor() + with pytest.raises(psycopg2.Error) as exc_info: + cur.execute("SELECT * FROM no_such_table_1a2b3c") + + assert ( + exc_info.value.pgcode == "0A000" + ), f"unmapped error should stay SQLSTATE 0A000, got {exc_info.value.pgcode}" + + _assert_class(server, "other", "no_such_table_1a2b3c") + + +def test_recoverable_out_of_memory_class_is_logged(): + """A temp-cap overflow is a recoverable OOM: classified, and the server lives.""" + with _spill_server() as server: + assert is_server_listening(server.socket_path) + + cur = _connect().cursor() + with pytest.raises(psycopg2.Error) as exc_info: + cur.execute(SPILL_QUERY) + + assert ( + exc_info.value.pgcode == "53200" + ), f"recoverable OOM should report SQLSTATE 53200, got {exc_info.value.pgcode}" + + _assert_class(server, "out_of_memory", "range(10000000)") + + assert is_server_listening( + server.socket_path + ), "pgduck_server stopped accepting connections after a recoverable OOM" + assert server.process.poll() is None, "pgduck_server process exited" + + +def test_no_log_engine_errors_suppresses_the_class_line(): + """--no_log_engine_errors drops the line but not the error itself.""" + server = _start_server(extra_args=["--no_log_engine_errors"]) + cur = _connect().cursor() + with pytest.raises(psycopg2.Error) as exc_info: + cur.execute(f"SELECT * FROM read_parquet('{MISSING_FILE}')") + + # The client still learns what went wrong; only the log line is gone. + assert exc_info.value.pgcode == "58030" + + assert not _class_lines( + server + ), "--no_log_engine_errors still logged a classified line" + + +def test_no_log_engine_errors_is_announced_at_startup(): + """Disabling classification must be visible, since silence otherwise reads + as "no errors occurred" rather than "nothing is being classified". + + The announcement must also stay invisible to a collector allow-listing the + "pgduck_engine_error: " prefix, so it carries the bare name without the + colon. + """ + server = _start_server(extra_args=["--no_log_engine_errors"]) + output = get_server_output(server.output_queue) + + assert ( + "Engine error classification is off" in output + ), f"startup did not announce the disabled classification: {output!r}" + + assert ( + ENGINE_ERROR_PREFIX not in output + ), f"startup line matches the collected prefix {ENGINE_ERROR_PREFIX!r}: {output!r}" From 783cfa9871c0015e2af061e21a6deaa44d49fef1 Mon Sep 17 00:00:00 2001 From: Ridvan Sirma Date: Mon, 21 Sep 2026 14:02:09 +0000 Subject: [PATCH 2/4] pgduck_server: classify fatal OOM before the process exits A genuine RAM OOM is the case a client often only sees as lost_connection. Assert the canned out_of_memory record is on stderr before exit(), and that the statement does not ride along. Signed-off-by: Ridvan Sirma Co-authored-by: Cursor --- .../tests/pytests/test_server_start.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/pgduck_server/tests/pytests/test_server_start.py b/pgduck_server/tests/pytests/test_server_start.py index 280f1f496..83b27c109 100644 --- a/pgduck_server/tests/pytests/test_server_start.py +++ b/pgduck_server/tests/pytests/test_server_start.py @@ -1,6 +1,7 @@ import pytest import subprocess import os +import re import signal import time import tempfile @@ -692,6 +693,23 @@ def test_genuine_oom_over_extended_protocol_terminates_server(): "Out of Memory Error" in server_output ), f"expected the genuine OOM message to be surfaced, got: {server_output}" + # The class line must be written before exit(): after the process dies + # the client often only sees lost_connection, so this record is the + # one a collector can still attribute to OOM. + classified = [ + line + for line in server_output.splitlines() + if re.match(r"^\S+ LOG pgduck_engine_error: out_of_memory$", line) + ] + assert classified, ( + "expected a classified out_of_memory record before the fatal exit, " + f"got: {server_output}" + ) + for line in classified: + assert ( + "range(100000000)" not in line + ), f"classified line leaked the statement: {line!r}" + # --------------------------------------------------------------------------- # Spill location via the init file From 3f8bf61a59e6395fc69f20184f004c5f42c4bc69 Mon Sep 17 00:00:00 2001 From: Ridvan Sirma Date: Tue, 22 Sep 2026 10:00:15 +0000 Subject: [PATCH 3/4] pgduck_server: always emit the engine error class pgduck_server stderr is already a niche stream; an opt-out flag is extra surface without a use. The class line is always written. Signed-off-by: Ridvan Sirma Co-authored-by: Cursor --- .../include/command_line/command_line.h | 1 - pgduck_server/include/pgsession/pgsession.h | 1 - pgduck_server/src/command_line/command_line.c | 9 ----- pgduck_server/src/main.c | 1 - pgduck_server/src/pgsession/pgsession.c | 10 ++---- .../pytests/test_engine_error_class_log.py | 35 ------------------- 6 files changed, 2 insertions(+), 55 deletions(-) diff --git a/pgduck_server/include/command_line/command_line.h b/pgduck_server/include/command_line/command_line.h index c7622314c..3631cf460 100644 --- a/pgduck_server/include/command_line/command_line.h +++ b/pgduck_server/include/command_line/command_line.h @@ -43,7 +43,6 @@ typedef struct char *cache_dir; char *extensions_dir; bool no_extension_install; - bool no_log_engine_errors; bool debug; char *init_file_path; char *pidfile_path; diff --git a/pgduck_server/include/pgsession/pgsession.h b/pgduck_server/include/pgsession/pgsession.h index 018ed0fb7..604388f5c 100644 --- a/pgduck_server/include/pgsession/pgsession.h +++ b/pgduck_server/include/pgsession/pgsession.h @@ -181,6 +181,5 @@ typedef struct PGSession extern void *pgsession_handle_connection(void *input); extern int oom_is_fatal; -extern bool log_engine_errors; #endif /* // PGDUCK_PG_SESSION_H */ diff --git a/pgduck_server/src/command_line/command_line.c b/pgduck_server/src/command_line/command_line.c index 3cd16ed06..6f01c9c1d 100644 --- a/pgduck_server/src/command_line/command_line.c +++ b/pgduck_server/src/command_line/command_line.c @@ -119,7 +119,6 @@ print_usage() printf(" --extensions_dir Install and load extensions in the specified directory\n"); printf(" --pidfile Write the pid of this program to the given path\n"); printf(" --no_extension_install Disable extension installation\n"); - printf(" --no_log_engine_errors Do not log an error class for failing queries\n"); printf(" --debug Include debug-level log messages (including full queries) in server output\n"); printf(" --verbose Run in verbose mode\n"); printf(" --help Display this help and exit\n"); @@ -147,7 +146,6 @@ parse_arguments(int argc, char *argv[]) .cache_dir = NULL, .extensions_dir = NULL, .no_extension_install = false, - .no_log_engine_errors = false, .debug = false, }; int opt; @@ -169,7 +167,6 @@ parse_arguments(int argc, char *argv[]) {"cache_dir", required_argument, NULL, 'C'}, {"extensions_dir", required_argument, NULL, 'E'}, {"no_extension_install", no_argument, NULL, 'n'}, - {"no_log_engine_errors", no_argument, NULL, 'e'}, {"init_file_path", required_argument, NULL, 'i'}, {"pidfile", required_argument, NULL, 'p'}, {"debug", no_argument, NULL, 'd'}, @@ -236,9 +233,6 @@ parse_arguments(int argc, char *argv[]) case 'n': options.no_extension_install = true; break; - case 'e': - options.no_log_engine_errors = true; - break; case 'P': { int inputPort = 0; @@ -326,9 +320,6 @@ parse_arguments(int argc, char *argv[]) if (options.no_extension_install) PGDUCK_SERVER_LOG("Using local extension binaries only"); - if (options.no_log_engine_errors) - PGDUCK_SERVER_LOG("Engine error classification is off; no pgduck_engine_error lines will be emitted"); - if (options.debug) PGDUCK_SERVER_LOG("Debugging mode on; will log all queries"); diff --git a/pgduck_server/src/main.c b/pgduck_server/src/main.c index 236d6ed25..678bfd26f 100644 --- a/pgduck_server/src/main.c +++ b/pgduck_server/src/main.c @@ -63,7 +63,6 @@ main(int argc, char *argv[]) pgduck_log_min_messages = DEBUG1; oom_is_fatal = !options.continue_on_oom; - log_engine_errors = !options.no_log_engine_errors; /* first, make sure duckdb is accessible */ DuckDBStatus duckDbStatus = duckdb_global_init(options.duckdb_database_file_path, diff --git a/pgduck_server/src/pgsession/pgsession.c b/pgduck_server/src/pgsession/pgsession.c index 7d67a21d5..28a18f3c2 100644 --- a/pgduck_server/src/pgsession/pgsession.c +++ b/pgduck_server/src/pgsession/pgsession.c @@ -121,9 +121,6 @@ static bool is_transmit_query(const char *queryString); /* global flag on whether to exit on OOM */ int oom_is_fatal = true; -/* global flag on whether to log a class for engine errors */ -bool log_engine_errors = true; - /* * Per-client entrance point for the pgsession logic. * @@ -960,11 +957,8 @@ handle_pgsession_error_message(DuckDBStatus status, PGSession * pgSession, char * Every reportable status passes through here, including the fatal ones * the caller exits on, so one line here covers all of them. */ - if (log_engine_errors) - { - PGDUCK_SERVER_LOG(PGDUCK_ENGINE_ERROR_PREFIX "%s", - error_class_for_sqlstate(sqlState)); - } + PGDUCK_SERVER_LOG(PGDUCK_ENGINE_ERROR_PREFIX "%s", + error_class_for_sqlstate(sqlState)); switch (status) { diff --git a/pgduck_server/tests/pytests/test_engine_error_class_log.py b/pgduck_server/tests/pytests/test_engine_error_class_log.py index 57c07f3c4..8d70f8cd3 100644 --- a/pgduck_server/tests/pytests/test_engine_error_class_log.py +++ b/pgduck_server/tests/pytests/test_engine_error_class_log.py @@ -172,38 +172,3 @@ def test_recoverable_out_of_memory_class_is_logged(): server.socket_path ), "pgduck_server stopped accepting connections after a recoverable OOM" assert server.process.poll() is None, "pgduck_server process exited" - - -def test_no_log_engine_errors_suppresses_the_class_line(): - """--no_log_engine_errors drops the line but not the error itself.""" - server = _start_server(extra_args=["--no_log_engine_errors"]) - cur = _connect().cursor() - with pytest.raises(psycopg2.Error) as exc_info: - cur.execute(f"SELECT * FROM read_parquet('{MISSING_FILE}')") - - # The client still learns what went wrong; only the log line is gone. - assert exc_info.value.pgcode == "58030" - - assert not _class_lines( - server - ), "--no_log_engine_errors still logged a classified line" - - -def test_no_log_engine_errors_is_announced_at_startup(): - """Disabling classification must be visible, since silence otherwise reads - as "no errors occurred" rather than "nothing is being classified". - - The announcement must also stay invisible to a collector allow-listing the - "pgduck_engine_error: " prefix, so it carries the bare name without the - colon. - """ - server = _start_server(extra_args=["--no_log_engine_errors"]) - output = get_server_output(server.output_queue) - - assert ( - "Engine error classification is off" in output - ), f"startup did not announce the disabled classification: {output!r}" - - assert ( - ENGINE_ERROR_PREFIX not in output - ), f"startup line matches the collected prefix {ENGINE_ERROR_PREFIX!r}: {output!r}" From 2acc2bff611c9e13082f76413e1b60c6ab687736 Mon Sep 17 00:00:00 2001 From: Ridvan Sirma Date: Wed, 23 Sep 2026 11:41:28 +0000 Subject: [PATCH 4/4] pgduck_server: verify failed CREATE SECRET doesn't leak credentials in error class log Add a pytest that issues an invalid CREATE SECRET with distinctive KEY_ID/SECRET/SESSION_TOKEN values and asserts the classified engine error line carries the error class but none of the credential strings. Signed-off-by: Ridvan Sirma Co-authored-by: Cursor --- .../pytests/test_engine_error_class_log.py | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/pgduck_server/tests/pytests/test_engine_error_class_log.py b/pgduck_server/tests/pytests/test_engine_error_class_log.py index 8d70f8cd3..c8d5e53ae 100644 --- a/pgduck_server/tests/pytests/test_engine_error_class_log.py +++ b/pgduck_server/tests/pytests/test_engine_error_class_log.py @@ -172,3 +172,32 @@ def test_recoverable_out_of_memory_class_is_logged(): server.socket_path ), "pgduck_server stopped accepting connections after a recoverable OOM" assert server.process.poll() is None, "pgduck_server process exited" + + +# Distinctive so a leak on the classified line cannot be a coincidence. +SECRET_KEY_ID = "leaky-key-id-9f3c1d" +SECRET_SECRET = "leaky-secret-9f3c1d" +SECRET_TOKEN = "leaky-session-token-9f3c1d" + + +def test_failed_create_secret_class_line_does_not_carry_credentials(): + """A bad CREATE SECRET still logs a class, without KEY_ID/SECRET/token.""" + server = _start_server() + cur = _connect().cursor() + with pytest.raises(psycopg2.Error): + cur.execute( + "CREATE SECRET leak_probe_9f3c1d (" + "TYPE NOT_A_SECRET_TYPE, " + f"KEY_ID '{SECRET_KEY_ID}', " + f"SECRET '{SECRET_SECRET}', " + f"SESSION_TOKEN '{SECRET_TOKEN}'" + ")" + ) + + _assert_class( + server, + "invalid_input", + SECRET_KEY_ID, + SECRET_SECRET, + SECRET_TOKEN, + )