diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index cfa941216..982ec7f9d 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -881,6 +881,13 @@ fk_referencing fk_referencing the conversion helper is refused identically 15;16 fk_referencing fk_referencing the parent is still a row store 15;16;17;18;19 2026-10-16 columnar_tableam.c: converting an FK-referenced table to columnar no longer refused fk_referencing fk_referencing the primary key is still enforced 15;16;17;18;19 never - fk_referencing fk_referencing the refused table was not left behind 15;16;17;18;19 2026-10-16 columnar_tableam.c: a foreign key referencing a columnar table no longer refused;columnar_tableam.c: the foreign-key guard keyed on the referencing table, not the referenced one +hardening hardening backend alive after corrupt bloom 15;16;17;18;19 never - +hardening hardening backend alive after invalid descriptor 15;16;17;18;19 2026-10-17 columnar_reader.c: an unrecognized native encoding descriptor aborts the backend, with the value-stream bounds before it off +hardening hardening corrupt bloom absent 15;16;17;18;19 never - +hardening hardening corrupt bloom present 15;16;17;18;19 2026-10-17 columnar_bloom.c: a bloom filter too short for its header reads as definitely absent +hardening hardening invalid descriptor raises error 15;16;17;18;19 2026-10-17 columnar_reader.c: an unrecognized native encoding descriptor aborts the backend, with the value-stream bounds before it off +hardening hardening premise: the corrupt descriptor replaces one chunk's 15;16;17;18;19 never - +hardening hardening premise: the malformed bloom replaces k's filter 15;16;17;18;19 never - harness_selftest 030-assertions nothing leaked into the squatter 15;16;17;18;19 never - harness_selftest 030-assertions pgc_port_free says the squatter's port is busy 15;16;17;18;19 never - harness_selftest 030-assertions squatter survived untouched 15;16;17;18;19 never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index 55824d0d1..85fa0b1cc 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -15,7 +15,7 @@ # # Adding a check to a suite that is already covered does not move it, which is # what makes it safe to bound. -suites_not_covered 182 +suites_not_covered 181 # # checks_never_observed_red -- A CENSUS. NOT a ceiling, and it must not become # one. @@ -3746,4 +3746,24 @@ suites_not_covered 182 # never 3085 + 2 = 3087 # dated 1402 + 3 = 1405 # 3087 + 1405 == 4492 -checks_never_observed_red 3087 +# +# #1341, hardening seeded, with its twin (OffgridwithJD): 3087 -> 3091, and +# suites_not_covered 182 -> 181. The suite had no rows. It enters with 7 checks from +# five-major runs (7/7 on each of 15-19), after three fixes and two new premises. +# "invalid descriptor raises error" read an empty result as the error, which a crash +# also gives: with the descriptor's refusal turned into abort() it passed, and the +# crash failed the bloom section's setup and both its arms instead. It now reads the +# SQLSTATE (XX001), survival counts crashes in the server log, and each survival +# check waits recovery out, so a crash fails its own two arms only. A premise per +# corruption asserts its UPDATE replaced one row. 3 are dated by two mutations: an +# unrecognized descriptor aborting with the value-stream bounds before it off (2), +# and a too-short bloom filter read as definitely absent (1). Four arrive never: the +# two premises, the absent probe (424242424 lies outside k's range, so a bloom +# read as absent changes no answer), and survival of the bloom. Removing the descriptor's backstops one at a +# time reddens nothing, rightly: each layer left refuses with the same XX001. +# +# ledger rows 4492 + 7 = 4499 +# never 3087 + 4 = 3091 +# dated 1405 + 3 = 1408 +# 3091 + 1408 == 4499 +checks_never_observed_red 3091 diff --git a/test/hardening.sh b/test/hardening.sh index d6afc876d..8a23549fe 100755 --- a/test/hardening.sh +++ b/test/hardening.sh @@ -15,25 +15,63 @@ set -uo pipefail pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" +errcode() { # SQL -> the 5-char SQLSTATE it raises, or empty: a crash raises none + env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" \ + -qtA -v VERBOSITY=sqlstate -c "$1" 2>&1 \ + | sed -n 's/^ERROR: \([0-9A-Z]\{5\}\).*/\1/p' | head -1 +} +# A liveness probe alone passes once crash recovery finishes, so survival also +# requires that no backend has died since the scenario started, read from the +# server log rather than from timing. +crashes() { + # a number, or nothing when the log can't be read: grep -c prints 0 itself when + # nothing matches (and exits 1), but nothing at all for a file it cannot open, + # and a blind count must not read as "no crashes" + local _n + _n="$(pgc_pg "grep -c 'terminated by signal' '$PGC_LOGFILE'" 2>/dev/null)" + case "$_n" in '' | *[!0-9]*) ;; *) echo "$_n" ;; esac +} +alive() { # since the scenario's corruption, which notes the count + # a crash puts the server through recovery: wait it out, so the next scenario + # starts on a server that answers and one crash cannot fail the checks after it + local _i + for _i in $(seq 60); do [ "$(q 'SELECT 1;' 2>/dev/null)" = "1" ] && break; sleep 1; done + local _now; _now="$(crashes)" + if ! [[ "$CRASHES_BEFORE" =~ ^[0-9]+$ && "$_now" =~ ^[0-9]+$ ]]; then + echo "no (crash count unreadable)" + elif [ "$(q 'SELECT 1;')" = "1" ] && [ "$_now" = "$CRASHES_BEFORE" ]; then + echo yes + else + echo "no ($(( _now - CRASHES_BEFORE )) backend crashes)" + fi +} + # An invalid encoding descriptor must raise a clean error, not crash the backend. +# "Clean" is read as its SQLSTATE: an empty result, which this arm used to take +# for an error, is also what a crash and a missing table give. echo "-- invalid encoding descriptor" make_pair "id int, v bigint" load_pair "SELECT g, g*2 FROM generate_series(1,3000) g" -psql_run "UPDATE pgcolumnar.column_chunk SET encoding_descriptor = '\\\\xffffffff'::bytea - WHERE storage_id = pgcolumnar.get_storage_id('t_col') AND column_index = 1;" -badresult="$(q "SELECT * FROM t_col;")" -check "invalid descriptor raises error" "$([ -z "$badresult" ] && echo errored)" "errored" -check "backend alive after invalid descriptor" "$(q "SELECT 1;")" "1" +check "premise: the corrupt descriptor replaces one chunk's" \ + "$(q "UPDATE pgcolumnar.column_chunk SET encoding_descriptor = '\\\\xffffffff'::bytea + WHERE storage_id = pgcolumnar.get_storage_id('t_col') AND column_index = 1;")" \ + "UPDATE 1" +CRASHES_BEFORE="$(crashes)" +check "invalid descriptor raises error" "$(errcode "SELECT * FROM t_col;")" "XX001" +check "backend alive after invalid descriptor" "$(alive)" "yes" # A malformed bloom filter must be ignored, keeping results correct. echo "-- malformed bloom filter" make_pair "id int, k bigint" q "SELECT pgcolumnar.set_options('t_col', chunk_group_row_limit => 1000, stripe_row_limit => 20000);" >/dev/null load_pair "SELECT g, ((g*2654435761)%100000)::bigint FROM generate_series(1,8000) g" -psql_run "UPDATE pgcolumnar.bloom SET filter = '\\\\x00'::bytea - WHERE storage_id = pgcolumnar.get_storage_id('t_col') AND column_index = 1;" +check "premise: the malformed bloom replaces k's filter" \ + "$(q "UPDATE pgcolumnar.bloom SET filter = '\\\\x00'::bytea + WHERE storage_id = pgcolumnar.get_storage_id('t_col') AND column_index = 1;")" \ + "UPDATE 1" +CRASHES_BEFORE="$(crashes)" diff_query "corrupt bloom present" "SELECT id FROM %T WHERE k = ((7*2654435761)%100000)::bigint" diff_query "corrupt bloom absent" "SELECT count(*) FROM %T WHERE k = 424242424" -check "backend alive after corrupt bloom" "$(q "SELECT 1;")" "1" +check "backend alive after corrupt bloom" "$(alive)" "yes" pgc_summary diff --git a/test/native_varlena_bound.sh b/test/native_varlena_bound.sh index 5315b3bcd..1b55a67f9 100755 --- a/test/native_varlena_bound.sh +++ b/test/native_varlena_bound.sh @@ -69,16 +69,26 @@ refusal() { # SQL -> "SQLSTATE: message" of the error it raises, or NOERR's empt # requires that no backend has died since its scenario started, read from the server # log rather than from timing (corruption.sh measured the same hole). crashes() { - # grep -c prints 0 itself when nothing matches, and then exits 1 - pgc_pg "grep -c 'terminated by signal' '$PGC_LOGFILE'" 2>/dev/null || true + # a number, or nothing when the log can't be read: grep -c prints 0 itself when + # nothing matches (and exits 1), but nothing at all for a file it cannot open, + # and a blind count must not read as "no crashes" + local _n + _n="$(pgc_pg "grep -c 'terminated by signal' '$PGC_LOGFILE'" 2>/dev/null)" + case "$_n" in '' | *[!0-9]*) ;; *) echo "$_n" ;; esac } alive() { # since this scenario's corruption, which notes the count # a crash puts the server through recovery: wait it out, so the next scenario # starts on a server that answers and one crash cannot fail the checks after it local _i for _i in $(seq 60); do [ "$(q 'SELECT 1;' 2>/dev/null)" = "1" ] && break; sleep 1; done - [ "$(q 'SELECT 1;')" = "1" ] && [ "$(crashes)" = "$CRASHES_BEFORE" ] && echo yes \ - || echo "no ($(( $(crashes) - CRASHES_BEFORE )) backend crashes)" + local _now; _now="$(crashes)" + if ! [[ "$CRASHES_BEFORE" =~ ^[0-9]+$ && "$_now" =~ ^[0-9]+$ ]]; then + echo "no (crash count unreadable)" + elif [ "$(q 'SELECT 1;')" = "1" ] && [ "$_now" = "$CRASHES_BEFORE" ]; then + echo yes + else + echo "no ($(( _now - CRASHES_BEFORE )) backend crashes)" + fi } # the zone map's untouched bounds, to put back: native_zone_excludes decodes the # minimum BEFORE the maximum, so a corrupt minimum left in place answers for the diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index b7c5b9fc8..8ab46fb54 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -229,6 +229,7 @@ behaviour, the source of that number is named. - [181. test_native_varlena_bound.py: a corrupt varlena length is refused, not read past its buffer](#181-test_native_varlena_boundpy-a-corrupt-varlena-length-is-refused-not-read-past-its-buffer) - [182. test_native_agg_addcolumn.py: a metadata aggregate over an added column reads what the metadata cannot hold](#182-test_native_agg_addcolumnpy-a-metadata-aggregate-over-an-added-column-reads-what-the-metadata-cannot-hold) - [183. test_native_groupagg_wide_cost.py: the grouped fold prices its own input scan as the scan node does](#183-test_native_groupagg_wide_costpy-the-grouped-fold-prices-its-own-input-scan-as-the-scan-node-does) +- [184. test_hardening.py: corrupted native metadata is refused or ignored, never trusted](#184-test_hardeningpy-corrupted-native-metadata-is-refused-or-ignored-never-trusted) ## 1. How to read a test in here @@ -8064,3 +8065,21 @@ Each check is its own test over a module fixture that takes every reading in the | `test_with_parallelism_free_a_gather_displaces_the_serial_scan` | premise: with parallelism free the plan is a Gather, the state in which the fallback prices the scan | | `test_the_fallback_prices_the_fold_as_a_surviving_serial_scan_does` | the fold node's cost through the fallback equals its cost when a serial scan survives (800.19 on PG17; 503.01 with the bare seqscan formula) | | `test_backend_alive` | the backend still answers | + +## 184. test_hardening.py: corrupted native metadata is refused or ignored, never trusted + +The pytest twin of `test/hardening.sh` (ported for #1341), every check by the shell's name. A plain UPDATE corrupts a chunk's encoding descriptor, then a bloom filter, both catalog byteas. The descriptor must be refused with a clean XX001 and a surviving backend; the malformed bloom must be read as "may match", so the probe for a present value still finds it. An empty result is not read as the error, since a crash and a missing table give one too, and a crash is counted from the server log: a liveness probe alone passes once recovery has finished. On a private cluster, because a crash drops every session on the server. + +### Every test + +Each check is its own test over a module fixture that runs every step in the shell's order, each reading on a connection of its own opened once any recovery is over. A failed query is a `query_error()`, refused on either side by name. + +| test | what it holds | +| --- | --- | +| `test_the_corrupt_descriptor_replaces_one_chunks` | premise: the descriptor UPDATE reports `UPDATE 1` | +| `test_an_invalid_descriptor_raises_an_error` | reading the table raises XX001 | +| `test_the_backend_is_alive_after_an_invalid_descriptor` | the server answers and no backend died since the corruption | +| `test_the_malformed_bloom_replaces_ks_filter` | premise: the bloom UPDATE reports `UPDATE 1` | +| `test_a_corrupt_bloom_still_finds_a_present_value` | the equality probe returns the heap's rows | +| `test_a_corrupt_bloom_still_counts_an_absent_value` | the absent probe's count equals the heap's | +| `test_the_backend_is_alive_after_a_corrupt_bloom` | the server answers and no backend died since the corruption | diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index 3e3569beb..0301996d0 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -854,4 +854,6 @@ guard_tests 442 # per test (5 -> 21 tests), by collection over the cluster files. # cluster_tests 1006 -> 1011 for #1341's test_native_groupagg_wide_cost.py, the twin of # native_groupagg_wide_cost.sh: five tests, one per check, by collection over the cluster files. -cluster_tests 1011 +# cluster_tests 1011 -> 1018 for #1341's test_hardening.py, the twin of hardening.sh: seven +# tests, one per check, by collection over the cluster files. +cluster_tests 1018 diff --git a/test/pytest/test_compare_to_bash.py b/test/pytest/test_compare_to_bash.py index 2fd489c8a..e532b7139 100644 --- a/test/pytest/test_compare_to_bash.py +++ b/test/pytest/test_compare_to_bash.py @@ -112,6 +112,7 @@ "entry_point_privilege", "fk_referencing", "format_downgrade", + "hardening", "hilbert_cluster", "hilbert_locality", "iceberg_fdw", diff --git a/test/pytest/test_hardening.py b/test/pytest/test_hardening.py new file mode 100644 index 000000000..8c5f6709b --- /dev/null +++ b/test/pytest/test_hardening.py @@ -0,0 +1,194 @@ +"""Corrupted native metadata is refused or ignored, never trusted. The pytest twin of +`test/hardening.sh`, every check by the shell's name (#1341). + +An invalid encoding descriptor on a chunk must raise a clean error (XX001), not crash +the backend. A malformed bloom filter must be ignored, read conservatively as "may +match", so a probe for a value that is present still finds it. Both are catalog +byteas, so a plain UPDATE reaches them; a premise asserts each UPDATE replaced exactly +one row, or the checks after it would read uncorrupted metadata and pass. + +"Clean" is read as the SQLSTATE: an empty result is also what a crash and a missing +table give. A crash is read from the server log, counted from the corruption: a +liveness probe alone passes once crash recovery has finished. Each reading has a +connection of its own, opened once any recovery is over, as the shell's psql calls +are, so one crash cannot fail the scenario after it. + +A PRIVATE CLUSTER, because a crash would drop every session on the server. +""" +import pathlib +import shutil +import tempfile +import time + +import psycopg +import pytest + +from pgc_vacuity import query_error + +# the shell's bytes exactly: '\\x...' in a standard string is the ASCII text "\x...", +# which bytea's escape format stores as written +BAD_DESCRIPTOR = r"'\\xffffffff'::bytea" +BAD_BLOOM = r"'\\x00'::bytea" +PRESENT = "SELECT id FROM {t} WHERE k = ((7*2654435761)%100000)::bigint" +ABSENT = "SELECT count(*) FROM {t} WHERE k = 424242424" + + +def _connect(cluster): + """A connection, after waiting out any crash recovery.""" + deadline = time.monotonic() + 60 + while True: + try: + return psycopg.connect(cluster.dsn(), autocommit=True, connect_timeout=5) + except psycopg.OperationalError: + if time.monotonic() > deadline: + raise + time.sleep(0.5) + + +def _failed(exc): + return query_error(str(exc).splitlines()[0] if str(exc) else type(exc).__name__) + + +def _one(cluster, sql): + try: + with _connect(cluster) as c: + return str(c.execute(sql).fetchone()[0]) + except psycopg.Error as exc: + return _failed(exc) + + +def _tag(cluster, sql): + """-> the command tag sql returns, as psql prints it ("UPDATE 1").""" + try: + with _connect(cluster) as c: + cur = c.execute(sql) + return cur.statusmessage or "" + except psycopg.Error as exc: + return _failed(exc) + + +def _rows(cluster, sql): + try: + with _connect(cluster) as c: + return c.execute(sql).fetchall() + except psycopg.Error as exc: + return _failed(exc) + + +def _sqlstate(cluster, sql): + """-> the SQLSTATE sql raised, or what happened instead: a crash ends the + connection with no SQLSTATE at all.""" + try: + with _connect(cluster) as c: + rows = c.execute(sql).fetchall() + return f"no error ({len(rows)} rows)" + except psycopg.Error as exc: + return exc.sqlstate or _failed(exc) + + +def _crashes(cluster): + """-> how many backends the server log says died, or a failed reading when the + log can't be read: a blind count must not read as "no crashes".""" + try: + return (cluster.datadir / "server.log").read_text(errors="replace").count("terminated by signal") + except OSError as exc: + return query_error(f"crash count unreadable: {exc}") + + +def _alive(cluster, before): + """-> "yes" when the server answers and no backend died since `before`.""" + # probe first: the postmaster logs a backend's death when it reaps it, so a + # count read before the server answers again can miss the crash it is for + up = _one(cluster, "SELECT 1") == "1" + now = _crashes(cluster) + if not (isinstance(before, int) and isinstance(now, int)): + return now if isinstance(before, int) else before + died = now - before + return "yes" if up and died == 0 else f"no ({died} backend crashes)" + + +def _steps(cluster, *sqls): + # a step that fails is printed and left for the checks to see + for sql in sqls: + try: + with _connect(cluster) as c: + c.execute(sql) + except psycopg.Error as exc: + print(f"-- step failed: {str(exc).splitlines()[0] if str(exc) else exc!r}") + + +def _pair(cluster, defs, load, *between): + _steps(cluster, "DROP TABLE IF EXISTS t_heap", "DROP TABLE IF EXISTS t_col", + f"CREATE TABLE t_heap ({defs}) USING heap", + f"CREATE TABLE t_col ({defs}) USING pgcolumnar", *between, + f"INSERT INTO t_heap {load}", "INSERT INTO t_col SELECT * FROM t_heap") + + +@pytest.fixture(scope="module") +def hard(pgc_cluster): + """Every step of the shell, in its order, each reading recorded as a value so each + check is its own test and one red cannot stop the rest.""" + from pgc_cluster import Cluster, pick_port + + work = pathlib.Path(tempfile.mkdtemp(prefix="pgc-hardening-")) + work.chmod(0o777) + datadir = work / "data" + datadir.mkdir() + cluster = Cluster(pgc_cluster.pg_config, "hardening", datadir, pick_port(25)) + try: + cluster.initdb() + cluster.start() + _steps(cluster, "CREATE EXTENSION IF NOT EXISTS pgcolumnar") + r = {} + + _pair(cluster, "id int, v bigint", "SELECT g, g*2 FROM generate_series(1,3000) g") + r["descriptor lands"] = _tag( + cluster, f"UPDATE pgcolumnar.column_chunk SET encoding_descriptor = {BAD_DESCRIPTOR} " + "WHERE storage_id = pgcolumnar.get_storage_id('t_col') AND column_index = 1") + before = _crashes(cluster) + r["descriptor"] = _sqlstate(cluster, "SELECT * FROM t_col") + r["descriptor alive"] = _alive(cluster, before) + + _pair(cluster, "id int, k bigint", + "SELECT g, ((g*2654435761)%100000)::bigint FROM generate_series(1,8000) g", + "SELECT pgcolumnar.set_options('t_col', chunk_group_row_limit => 1000, " + "stripe_row_limit => 20000)") + r["bloom lands"] = _tag( + cluster, f"UPDATE pgcolumnar.bloom SET filter = {BAD_BLOOM} " + "WHERE storage_id = pgcolumnar.get_storage_id('t_col') AND column_index = 1") + before = _crashes(cluster) + for key, sql in (("present", PRESENT), ("absent", ABSENT)): + r[key] = (_rows(cluster, sql.format(t="t_col")), _rows(cluster, sql.format(t="t_heap"))) + r["bloom alive"] = _alive(cluster, before) + yield r + finally: + cluster.stop() + shutil.rmtree(work, ignore_errors=True) + + +def test_the_corrupt_descriptor_replaces_one_chunks(hard, expect): + expect.text(hard["descriptor lands"], "UPDATE 1", "premise: the corrupt descriptor replaces one chunk's") + + +def test_an_invalid_descriptor_raises_an_error(hard, expect): + expect.text(hard["descriptor"], "XX001", "invalid descriptor raises error") + + +def test_the_backend_is_alive_after_an_invalid_descriptor(hard, expect): + expect.text(hard["descriptor alive"], "yes", "backend alive after invalid descriptor") + + +def test_the_malformed_bloom_replaces_ks_filter(hard, expect): + expect.text(hard["bloom lands"], "UPDATE 1", "premise: the malformed bloom replaces k's filter") + + +def test_a_corrupt_bloom_still_finds_a_present_value(hard, expect): + expect.row_set(*hard["present"], "corrupt bloom present") + + +def test_a_corrupt_bloom_still_counts_an_absent_value(hard, expect): + expect.row_set(*hard["absent"], "corrupt bloom absent") + + +def test_the_backend_is_alive_after_a_corrupt_bloom(hard, expect): + expect.text(hard["bloom alive"], "yes", "backend alive after corrupt bloom") diff --git a/test/pytest/test_native_varlena_bound.py b/test/pytest/test_native_varlena_bound.py index 31d3eeb8c..6edb16d27 100644 --- a/test/pytest/test_native_varlena_bound.py +++ b/test/pytest/test_native_varlena_bound.py @@ -80,18 +80,25 @@ def _refusal(cluster, sql): def _crashes(cluster): + """-> how many backends the server log says died, or a failed reading when the + log can't be read: a blind count must not read as "no crashes".""" try: return (cluster.datadir / "server.log").read_text(errors="replace").count("terminated by signal") - except OSError: - return 0 + except OSError as exc: + return query_error(f"crash count unreadable: {exc}") def _scenario(cluster, sql): """-> (the SQLSTATE sql raised, whether the backend survived it).""" before = _crashes(cluster) state = _refused(cluster, sql) + # probe first: the postmaster logs a backend's death when it reaps it, so a + # count read before the server answers again can miss the crash it is for up = _one(cluster, "SELECT 1") == "1" - died = _crashes(cluster) - before + now = _crashes(cluster) + if not (isinstance(before, int) and isinstance(now, int)): + return state, now if isinstance(before, int) else before + died = now - before return state, "yes" if up and died == 0 else f"no ({died} backend crashes)" diff --git a/test/pytest/unported_suites.txt b/test/pytest/unported_suites.txt index 433ef552c..4bd8ab06c 100644 --- a/test/pytest/unported_suites.txt +++ b/test/pytest/unported_suites.txt @@ -45,7 +45,6 @@ fuzz_listing fuzz_parquet generated_columns groupagg_table_sizing -hardening harness_selftest hilbert_curve iceberg_catalog