diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index ce94bc74b..e5e80d92d 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -7103,7 +7103,9 @@ sort_status sort_status unsorted table reports every row appended 15;16;17;18;19 sort_status sort_status unsorted table reports no sorted groups 15;16;17;18;19 2026-10-20 sort_status counts every group as sorted when the table has no mark sort_status sort_status unsorted table reports no sorted rows 15;16;17;18;19 2026-10-20 sort_status counts every group as sorted when the table has no mark sort_status_privilege sort_status_privilege a role merely GRANTed select can read it too 15;16;17;18;19 2026-10-20 sort_status back to invoker rights, so the owner is false-denied on the catalogs (#608) +sort_status_privilege sort_status_privilege a role with no privilege on a heap table is refused for privilege, not told it is not columnar 15;16;17;18;19 2026-10-22 pgcolumnar--1.0-alpha5.sql: sort_status() reads the storage id before the caller check (the two lines swapped) sort_status_privilege sort_status_privilege a role with no privilege on the table is refused 15;16;17;18;19 2026-10-20 sort_status back to invoker rights, so the owner is false-denied on the catalogs (#608);sort_status no longer checks the caller's SELECT privilege (require_caller_select removed) +sort_status_privilege sort_status_privilege a superuser asking about a heap table is told it is not columnar 15;16;17;18;19 never - sort_status_privilege sort_status_privilege a superuser still reads sort status 15;16;17;18;19 never - sort_status_privilege sort_status_privilege and the refusal names the TABLE, not a catalog table it never asked about 15;16;17;18;19 2026-10-20 sort_status back to invoker rights, so the owner is false-denied on the catalogs (#608);sort_status no longer checks the caller's SELECT privilege (require_caller_select removed) sort_status_privilege sort_status_privilege premise: none of these roles is a superuser 15;16;17;18;19 never - @@ -7199,6 +7201,21 @@ statement_self_visibility statement_self_visibility and it read only the rows th statement_self_visibility statement_self_visibility and the table holds them once, as heap does 15;16;17;18;19 2026-10-15 a serial scan re-reads its row-group list when it reaches the end, so it picks up groups its own statement flushed (src/columnar_reader.c pgcolumnar_next_group_index) statement_self_visibility statement_self_visibility in one transaction, an INSERT, an UPDATE of every row and an INSERT SELECT run to completion 15;16;17;18;19 2026-10-15 a serial scan re-reads its row-group list when it reaches the end, so it picks up groups its own statement flushed (src/columnar_reader.c pgcolumnar_next_group_index) statement_self_visibility statement_self_visibility the INSERT flushed row groups while its scan was still reading 15;16;17;18;19 2026-10-15 a serial scan re-reads its row-group list when it reaches the end, so it picks up groups its own statement flushed (src/columnar_reader.c pgcolumnar_next_group_index) +stats_privilege stats_privilege a role merely GRANTed select can read them too 15;16;17;18;19 never - +stats_privilege stats_privilege a role with no privilege on a heap table is refused for privilege, not told it is not columnar 15;16;17;18;19 2026-10-22 pgcolumnar--1.0-alpha5.sql: stats() reads the storage id before the caller check (the two lines swapped) +stats_privilege stats_privilege a role with no privilege on the table is refused 15;16;17;18;19 never - +stats_privilege stats_privilege a superuser asking about a heap table is told it is not columnar 15;16;17;18;19 never - +stats_privilege stats_privilege a superuser still reads stats 15;16;17;18;19 never - +stats_privilege stats_privilege and the refusal names the TABLE, not a catalog table it never asked about 15;16;17;18;19 never - +stats_privilege stats_privilege premise: none of these roles is a superuser 15;16;17;18;19 never - +stats_privilege stats_privilege premise: t_stowner really owns the table 15;16;17;18;19 never - +stats_privilege stats_privilege premise: the catalog tables are NOT readable by these roles, so a GRANT is not the fix 15;16;17;18;19 never - +stats_privilege stats_privilege premise: the granted role can open a session 15;16;17;18;19 never - +stats_privilege stats_privilege premise: the no-privilege role can open a session 15;16;17;18;19 never - +stats_privilege stats_privilege premise: the no-privilege role cannot read it by ordinary SQL 15;16;17;18;19 never - +stats_privilege stats_privilege premise: the owner can open a session 15;16;17;18;19 never - +stats_privilege stats_privilege premise: the owner can read its own table by ordinary SQL 15;16;17;18;19 never - +stats_privilege stats_privilege the OWNER of the table can read its stats 15;16;17;18;19 never - storage_row_repair storage_row_repair REFRESH MATERIALIZED VIEW repairs the view's row 15;16;17;18;19 2026-10-12 PgColumnarRetargetStorageRelation returns at once, so no repair repoints a storage row storage_row_repair storage_row_repair a same-type ALTER COLUMN ... USING repairs the table's row 15;16;17;18;19 2026-10-12 PgColumnarRetargetStorageRelation returns at once, so no repair repoints a storage row storage_row_repair storage_row_repair after both repairs the notes' query finds nothing 15;16;17;18;19 2026-10-12 PgColumnarRetargetStorageRelation returns at once, so no repair repoints a storage row;the release notes' query is edited from NOT EXISTS to EXISTS diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index 5d2a3cb83..8b62c6794 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 28 +suites_not_covered 27 # # checks_never_observed_red -- A CENSUS. NOT a ceiling, and it must not become # one. @@ -7039,4 +7039,21 @@ suites_not_covered 28 # never 4545 - 25 = 4520 # dated 3418 + 25 = 3443 # 4520 + 3443 == 7963 -checks_never_observed_red 4520 +# #1657 follow-up, stats() and sort_status() refuse before they read the storage id +# (OffgridwithJD): 4520 -> 4535, and suites_not_covered 28 -> 27. get_storage_id +# checks SELECT with GetUserId(), which inside these SECURITY DEFINER functions is the +# definer, so it passes for anyone and says "not a columnar table" for a heap table. +# Only the explicit caller check refuses, and only while it runs first. With the two +# lines swapped ("sidfirst"), both privilege suites stayed 13/13 in shell and twin on +# main, because on their columnar table the caller check still refused afterwards. +# Each suite gains a heap table no test role may read: refusing it for privilege is +# dated by sidfirst, red by the same name in both twins, and the superuser control +# ("not a columnar table") arrives never. sort_status_privilege already had its rows +# and gains these 2. stats_privilege had none, so it enters whole with its 15 checks +# from five-major runs (all green on 15-19, shell and twin alike): 1 dated, 14 never. +# +# ledger rows 7963 + 17 = 7980 +# never 4520 + 15 = 4535 +# dated 3443 + 2 = 3445 +# 4535 + 3445 == 7980 +checks_never_observed_red 4535 diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index cedfcb647..6048fbf14 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -3931,6 +3931,8 @@ property this suite tests, so `SET ROLE` would assert it away. Each role connect | `test_the_premises_each_role_is_what_the_suite_assumes` | each premise run BY the role it is about | | `test_who_may_read_the_stats` | owner, GRANTed reader and superuser all succeed | | `test_a_role_with_no_privilege_is_refused` | the bar: 42501 **and** the table named | +| `test_a_heap_table_is_refused_for_privilege_before_its_kind_is_read_by_stats` | a heap table the caller may not read is refused for privilege, not called non-columnar: the caller check runs before the storage id is read | +| `test_a_superuser_is_told_a_heap_table_is_not_columnar_by_stats` | control: a caller who may read it is told "not a columnar table" | ### A helper that turned a driver detail into a product claim @@ -8975,6 +8977,8 @@ A call made as a role reads the first column of its first row, or its failure as | `test_a_role_with_no_privilege_is_refused_its_sort_status` | a role with no privilege on the table is refused | | `test_the_refusal_names_the_table` | and the refusal names the TABLE, not a catalog table it never asked about | | `test_a_superuser_still_reads_the_sort_status` | a superuser still reads sort status | +| `test_a_heap_table_is_refused_for_privilege_before_its_kind_is_read` | a role with no privilege on a heap table is refused for privilege, not told it is not columnar | +| `test_a_superuser_is_told_a_heap_table_is_not_columnar` | a superuser asking about a heap table is told it is not columnar | ## 210. test_bloom_lazy.py: a scan reads the bloom filters of the groups and columns it examines The pytest twin of `test/bloom_lazy.sh` (ported for #1341), every check by the shell's name. Two tables differ only in how many groups a one-group equality scan skips (6 and 30), so the bloom blocks they fetch, read from `pg_stat_get_blocks_fetched` on `pgcolumnar.bloom`, must not grow with the skipped groups (#310); on a wide table where no group is excluded, six equality predicates must fetch more than one (#314). The answers are compared with a heap mirror, with and without bloom filters. diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index 38492c1df..c2ef5cf38 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -1186,6 +1186,8 @@ guard_tests 839 # 22 checks in 22 tests, by collection over the cluster files. # cluster_tests 2670 -> 2677 for #1637's test_catalog_filter_reads.py, the twin of catalog_filter_reads.sh: # 7 tests, one per check, by collection over the cluster files. +# cluster_tests 4227 -> 4231 for the #1657 follow-up: a heap-table refusal and its control in +# test_stats_privilege.py and test_sort_status_privilege.py, by collection over the cluster files. # cluster_tests 4176 -> 4200 for #1339's hilbert batch: test_hilbert_cluster.py's S1 and (a2) tests split # into one check per test (S1 8 -> 24, (a2) 1 -> 7) and two daemon tests on a decayed zorder table, by collection. # cluster_tests 2748 -> 2753 for #1706's five timestamp-flag tests in test_native_parquet_schema.py, by collection. @@ -1208,4 +1210,4 @@ guard_tests 839 # 54 tests, one per check, by collection over the cluster files. # cluster_tests 4225 -> 4226 for #1753's arm 5 in test_parallel_flush_optin.py (folded into #1746): # one structural test, by collection over the cluster files. -cluster_tests 4227 +cluster_tests 4231 diff --git a/test/pytest/test_sort_status_privilege.py b/test/pytest/test_sort_status_privilege.py index 205669dd7..04c1e56f8 100644 --- a/test/pytest/test_sort_status_privilege.py +++ b/test/pytest/test_sort_status_privilege.py @@ -61,6 +61,8 @@ def ss(pgc_cluster): conn.execute("ALTER TABLE ss_t OWNER TO t_ssowner") conn.execute("REVOKE ALL ON ss_t FROM PUBLIC") conn.execute("GRANT SELECT ON ss_t TO t_sssel") + conn.execute("CREATE TABLE ss_h (id int)") # a heap table, readable by no test role + conn.execute("REVOKE ALL ON ss_h FROM PUBLIC") owner = conn.execute("SELECT pg_get_userbyid(relowner) FROM pg_class WHERE relname='ss_t'").fetchone()[0] nosuper = conn.execute("SELECT bool_and(NOT rolsuper) FROM pg_roles WHERE rolname LIKE 't_ss%'").fetchone()[0] superuser = conn.execute("SELECT count(*) FROM pgcolumnar.sort_status('ss_t')").fetchone()[0] @@ -80,6 +82,10 @@ def ss(pgc_cluster): "none refused": _refused(none_status, "permission denied for table"), "none names table": _refused(none_status, "ss_t"), "super status": superuser, + "none heap": _refused(_as(dsn, "t_ssnone", "SELECT count(*) FROM pgcolumnar.sort_status('ss_h')"), + "permission denied for table ss_h"), + "super heap": _refused(_as(dsn, "postgres", "SELECT count(*) FROM pgcolumnar.sort_status('ss_h')"), + 'relation "ss_h" is not a columnar table'), } finally: with psycopg.connect(pgc_cluster.dsn(), autocommit=True) as a: @@ -143,3 +149,14 @@ def test_the_refusal_names_the_table(ss, expect): def test_a_superuser_still_reads_the_sort_status(ss, expect): expect.num(ss["super status"], 1, "a superuser still reads sort status") + + +def test_a_heap_table_is_refused_for_privilege_before_its_kind_is_read(ss, expect): + """get_storage_id checks the DEFINER inside this SECURITY DEFINER function, so only + the explicit caller check refuses, and only while it runs first (#1657 review).""" + expect.num(ss["none heap"], 1, + "a role with no privilege on a heap table is refused for privilege, not told it is not columnar") + + +def test_a_superuser_is_told_a_heap_table_is_not_columnar(ss, expect): + expect.num(ss["super heap"], 1, "a superuser asking about a heap table is told it is not columnar") diff --git a/test/pytest/test_stats_privilege.py b/test/pytest/test_stats_privilege.py index a64d52f6d..7a0536218 100644 --- a/test/pytest/test_stats_privilege.py +++ b/test/pytest/test_stats_privilege.py @@ -75,6 +75,9 @@ def _fixture(cur): cur.execute(f"ALTER TABLE st_t OWNER TO {OWNER}") cur.execute("REVOKE ALL ON st_t FROM PUBLIC") cur.execute(f"GRANT SELECT ON st_t TO {SEL}") + cur.execute("DROP TABLE IF EXISTS st_h") + cur.execute("CREATE TABLE st_h (id int)") # a heap table, readable by no test role + cur.execute("REVOKE ALL ON st_h FROM PUBLIC") return schema @@ -170,3 +173,28 @@ def test_a_role_with_no_privilege_is_refused(pgc_cluster, pgc_conn, expect): expect.contains( str(err), "st_t", "and the refusal names the TABLE, not a catalog table it never asked about") + + +def _says(err, text): + """-> 1 when the statement failed with `text` in its message, else 0, as the shell's grep -c.""" + return 1 if err is not None and text in str(err) else 0 + + +def test_a_heap_table_is_refused_for_privilege_before_its_kind_is_read_by_stats(pgc_cluster, pgc_conn, expect): + """get_storage_id checks SELECT with GetUserId(), the DEFINER here, so it passes for + anyone and then says "not a columnar table". Only the explicit caller check refuses, + and only while it runs first: with the two swapped (#1657 review) every arm above + stays green, because on st_t the caller check still refuses afterwards.""" + with pgc_conn.cursor() as cur: + schema = _fixture(cur) + _, err = _as(pgc_cluster, NONE, "SELECT count(*) FROM pgcolumnar.stats('st_h')", schema) + expect.num(_says(err, "permission denied for table st_h"), 1, + "a role with no privilege on a heap table is refused for privilege, not told it is not columnar") + + +def test_a_superuser_is_told_a_heap_table_is_not_columnar_by_stats(pgc_cluster, pgc_conn, expect): + with pgc_conn.cursor() as cur: + schema = _fixture(cur) + _, err = _as(pgc_cluster, "postgres", "SELECT count(*) FROM pgcolumnar.stats('st_h')", schema) + expect.num(_says(err, 'relation "st_h" is not a columnar table'), 1, + "a superuser asking about a heap table is told it is not columnar") diff --git a/test/sort_status_privilege.sh b/test/sort_status_privilege.sh index 9bdcf64bb..217c2ae8b 100755 --- a/test/sort_status_privilege.sh +++ b/test/sort_status_privilege.sh @@ -42,6 +42,8 @@ psql_run "SELECT pgcolumnar.vacuum_sorted('ss_t','ts');" -- give it a sorted r psql_run "ALTER TABLE ss_t OWNER TO t_ssowner;" psql_run "REVOKE ALL ON ss_t FROM PUBLIC;" psql_run "GRANT SELECT ON ss_t TO t_sssel;" +psql_run "CREATE TABLE ss_h (id int);" # a heap table, readable by no test role +psql_run "REVOKE ALL ON ss_h FROM PUBLIC;" as() { env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U "$1" \ -d "$PGC_DB" -At -v ON_ERROR_STOP=0 -c "$2" 2>&1 | head -1; } @@ -75,6 +77,16 @@ check "a role with no privilege on the table is refused" \ check "and the refusal names the TABLE, not a catalog table it never asked about" \ "$(as t_ssnone "SELECT count(*) FROM pgcolumnar.sort_status('ss_t');" | grep -c 'ss_t')" "1" +# ---- the order: privilege before anything else ------------------------------ +# +# As in stats_privilege.sh: get_storage_id checks the DEFINER, so only the explicit +# caller check refuses, and only while it runs first ("sidfirst" swaps them and +# every check above stays green). A heap table must be refused for privilege. +check "a role with no privilege on a heap table is refused for privilege, not told it is not columnar" \ + "$(as t_ssnone "SELECT count(*) FROM pgcolumnar.sort_status('ss_h');" | grep -c 'permission denied for table ss_h')" "1" +check "a superuser asking about a heap table is told it is not columnar" \ + "$(as postgres "SELECT count(*) FROM pgcolumnar.sort_status('ss_h');" | grep -c 'relation "ss_h" is not a columnar table')" "1" + # ---- the superuser path still works ---------------------------------------- check_num "a superuser still reads sort status" \ "$(q "SELECT count(*) FROM pgcolumnar.sort_status('ss_t');")" "1" diff --git a/test/stats_privilege.sh b/test/stats_privilege.sh index e14890ae5..86868a07c 100755 --- a/test/stats_privilege.sh +++ b/test/stats_privilege.sh @@ -51,6 +51,8 @@ psql_run "INSERT INTO st_t SELECT g, 'v'||g FROM generate_series(1,$ROWS) g;" psql_run "ALTER TABLE st_t OWNER TO t_stowner;" psql_run "REVOKE ALL ON st_t FROM PUBLIC;" psql_run "GRANT SELECT ON st_t TO t_stsel;" +psql_run "CREATE TABLE st_h (id int);" # a heap table, readable by no test role +psql_run "REVOKE ALL ON st_h FROM PUBLIC;" as() { env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U "$1" \ -d "$PGC_DB" -At -v ON_ERROR_STOP=0 -c "$2" 2>&1 | head -1; } @@ -101,6 +103,20 @@ check "a role with no privilege on the table is refused" \ check "and the refusal names the TABLE, not a catalog table it never asked about" \ "$(as t_stnone "SELECT count(*) FROM pgcolumnar.stats('st_t');" | grep -c 'st_t')" "1" +# ---- the order: privilege before anything else ------------------------------ +# +# get_storage_id checks SELECT with GetUserId(), which inside this SECURITY DEFINER +# function is the definer, so it passes for anyone and then says "not a columnar +# table" for a heap table. Only the explicit caller check refuses, and only while +# it runs FIRST: with the two lines swapped (#1657 review, "sidfirst") every check +# above stays green, because on st_t the caller check still refuses afterwards. +# A heap table the caller may not read must be refused for privilege; the control +# shows the same call does say "not a columnar table" to a caller who may read it. +check "a role with no privilege on a heap table is refused for privilege, not told it is not columnar" \ + "$(as t_stnone "SELECT count(*) FROM pgcolumnar.stats('st_h');" | grep -c 'permission denied for table st_h')" "1" +check "a superuser asking about a heap table is told it is not columnar" \ + "$(as postgres "SELECT count(*) FROM pgcolumnar.stats('st_h');" | grep -c 'relation "st_h" is not a columnar table')" "1" + # ---- the superuser path still works ---------------------------------------- check_num "a superuser still reads stats" "$(q "SELECT count(*) FROM pgcolumnar.stats('st_t');")" "1"