Skip to content

test: attribute a refusal by SQLSTATE, and fix the tool that grades it (#432) #1036

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jdatcmd merged 2 commits into from
Sep 13, 2026
Merged

test: attribute a refusal by SQLSTATE, and fix the tool that grades it (#432) #1036

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a23ff5d
test: attribute a refusal by SQLSTATE, and fix the tool that grades i…
jdatcmd Sep 13, 2026
ebcfabf
Merge main into the projection-privilege branch
jdatcmd Sep 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,59 @@ true until the next version shipped.

### Added

- `projection_privilege.sh` has a pytest twin, and both halves now attribute a refusal
by SQLSTATE instead of by error text (#432, #562, #563).

The bash suite decided four things by matching the message, and two of them were
load-bearing rather than decorative. Its own comment says why: both ACL layers raise
42501, so a bare "refused" stays true if the SQL `REVOKE` is deleted and the C check
catches it instead. Measured there: with the `REVOKE` removed the suite still passed
14 of 14.

The fixture is what separates them, not the wording. Called with a projection name
that does not exist, on a table the role may read, a caller stopped by the SQL grant
never runs the body and gets 42501; one that gets past the grant reaches the lookup
and gets 42704. The refusal is attributed by what the code REACHED. RLS is a third
code again, `0A000` from `ERRCODE_FEATURE_NOT_SUPPORTED`, which is a different
SQLSTATE class from either ACL refusal.

Two orderings that `src/columnar_projection.c` and `src/columnar_vacuum.c` assert had
no test in either harness: the base ACL is checked before the projection is looked
up, so a caller with no SELECT cannot learn whether a projection exists on a table it
may not read; and the ACL is checked before RLS, so a caller with no privilege is not
told the table has row-level security enabled. The second is a correction the source
records being made in review.

Five mutations, each asserted to apply at both call sites. Moving the ACL check below
the projection lookup but above its raise reddens nothing, correctly -- 42501 still
wins. Moving it below the raise reddens both ordering arms, and the shell half prints
the disclosure: `got [42704] want [42501]`.

- `compare_to_bash.py` reads the assertion's NAME (#432, #897).

The parity tool decides whether a port is one-for-one with its bash suite, which is
#432's definition of done, and it was reading the wrong argument. The python side was
matched with `expect\.\w+\([^)]*?"([^"]+)"`, whose lazy `[^)]*?` stops at the FIRST
quoted argument. For `expect.num(got, 1, NAME)` that is the name, so the tool looked
correct. For `expect.sqlstate(err, "42501", NAME)` it is `42501`.

Every SQLSTATE assertion was therefore read as the literal `42501`, reported as an
"extra" name the bash suite does not have, while the real property was reported
MISSING. #432's ports are exactly the ones replacing a grep on a message with a
SQLSTATE assertion, so the tool went blind in proportion to the work being done well.

It parses with `ast` now and takes the last string argument, resolving f-strings to
templates, both arms of a conditional, and the `name` column of a
`@pytest.mark.parametrize`. Bash interpolations reduce to the same template, including
`$1`, which is the commonest one in a check name and which the first version of the
reducer missed because its pattern required a letter after the dollar.

Measured over every pair in the tree: **61 bash properties reported missing, now 0.**
34 were never missing. The rest were real and are closed here: `stats_privilege` had
invented a name for a property the bash suite already named, and `zonemap_boundaries`
was missing its `backend alive` premise outright. Neither was visible while the tool
was reporting the wrong string.

- A UNIQUE-constraint check passed on any psql failure, and a recursive sweep passed on a
tree it never read (#1033).

Expand Down
83 changes: 72 additions & 11 deletions test/projection_privilege.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,15 @@ for r in t_prjexec t_prjsel; do
done
psql_run "GRANT SELECT ON secret TO t_prjsel;"

# A table EVERY role may read. It carries no projection, which is what lets the
# arms below attribute a refusal to a LAYER without reading the message: a caller
# stopped by the SQL grant never reaches the projection lookup, and one that gets
# past the grant does. See test/pytest/test_projection_privilege.py, which decides
# the same two questions the same way in a different language.
psql_run "CREATE TABLE prjopen (id int) USING pgcolumnar;"
psql_run "INSERT INTO prjopen SELECT generate_series(1,10);"
psql_run "GRANT SELECT ON prjopen TO t_prjnone, t_prjexec, t_prjsel;"

as() { # as <role> <sql>
env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U "$1" \
-d "$PGC_DB" -At -c "$2" 2>&1
Expand All @@ -82,6 +91,22 @@ count_as() { # count_as <role> <function>
esac
}

# state_as <role> <sql> -> the SQLSTATE, or the literal noerror.
#
# VERBOSITY is set with -v, NOT with -c "\\set ...". psql treats a -c argument
# beginning with a backslash as a meta-command and takes a different code path,
# which is how a sibling suite ended up with deny arms that could never go green.
# The SQLSTATE is read from the ERROR line because psql prefixes it, and it is
# extracted into a variable rather than tested through a pipeline whose STATUS
# would be the answer.
state_as() { # state_as <role> <sql>
local out _sqlstate
out="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U "$1" \
-d "$PGC_DB" -At -v VERBOSITY=sqlstate -v ON_ERROR_STOP=0 -c "$2" 2>&1)"
_sqlstate="$(sed -n 's/^.*ERROR:[[:space:]]*\([0-9A-Z]\{5\}\).*$/\1/p' <<<"$out" | head -1)"
if [ -n "$_sqlstate" ]; then printf '%s\n' "$_sqlstate"; else echo noerror; fi
}

# ---- premises: the fixture, and that the functions WORK ----------------------
#
# The owner arms are not decoration. A DENIED result and a BROKEN result are
Expand Down Expand Up @@ -125,11 +150,19 @@ check "and is refused reconstruct_via_projection" \
# WHICH layer refused, not merely that one did. Both layers reject this role, so
# a bare "refused" stays true if the REVOKE is deleted and the C check catches it
# instead -- measured: with the REVOKE removed this suite still passed 14 of 14.
# The error text is the only thing that attributes the refusal, so it is asserted
# here to tell the layers apart rather than in place of behaviour.
check "and the refusal comes from the SQL grant, naming the function" \
"$(as t_prjnone "SELECT count(*) FROM pgcolumnar.read_projection('secret','p1');" | grep -c 'permission denied for function')" \
"1"
#
# THE ERROR TEXT USED TO BE THE ONLY THING THAT COULD ATTRIBUTE IT, because both
# layers raise 42501. It is not, once the call names a table every role may read
# and a projection that does not exist: a caller stopped by the SQL grant never
# runs the body, so it never reaches the lookup, and one that gets past the grant
# does. The two outcomes are then different SQLSTATEs and the wording is free to
# change without moving the arm.
check "the no-EXECUTE role never reaches the body, so the grant is what stopped it" \
"$(state_as t_prjnone "SELECT count(*) FROM pgcolumnar.read_projection('prjopen','no_such_proj');")" \
"42501"
check "while the EXECUTE role reaches the projection lookup on the same call" \
"$(state_as t_prjexec "SELECT count(*) FROM pgcolumnar.read_projection('prjopen','no_such_proj');")" \
"42704"

# ---- layer two: the C check, reached only because EXECUTE was granted --------
#
Expand All @@ -150,9 +183,20 @@ check "a role with EXECUTE but no SELECT is refused read_projection" \
"$(count_as t_prjexec read_projection)" "refused"
check "and is refused reconstruct_via_projection, which leaks non-covered columns" \
"$(count_as t_prjexec reconstruct_via_projection)" "refused"
check "and THAT refusal names the table, so it is the C check and not the grant" \
"$(as t_prjexec "SELECT count(*) FROM pgcolumnar.read_projection('secret','p1');" | grep -c 'permission denied for table')" \
"1"
check "and THAT refusal is the C check, which raises 42501 from aclcheck_error" \
"$(state_as t_prjexec "SELECT count(*) FROM pgcolumnar.read_projection('secret','p1');")" \
"42501"

# AN ORDERING THE SOURCE ASSERTS AND NOTHING TESTED. The ACL check is the first
# statement of the body; the projection lookup is well below it. Swapped, a caller
# with no SELECT would learn whether a named projection exists on a table it may
# not read -- existence disclosure, from a function whose purpose is to stop
# disclosure. The arm above is the control: the same role, the same bogus name, on
# a table it MAY read, returns 42704, so 42501 here is the ACL check winning rather
# than the lookup being unreachable.
check "the base ACL is checked before the projection name is looked up" \
"$(state_as t_prjexec "SELECT count(*) FROM pgcolumnar.read_projection('secret','no_such_proj');")" \
"42501"

# ---- the bar is SELECT, not ownership ---------------------------------------
#
Expand Down Expand Up @@ -185,8 +229,25 @@ check "read_projection now refuses a policy-restricted caller (#563)" \
"$(count_as t_prjsel read_projection)" "refused"
check "and so does reconstruct_via_projection" \
"$(count_as t_prjsel reconstruct_via_projection)" "refused"
check "and the refusal names row-level security, not the table ACL" \
"$(as t_prjsel "SELECT count(*) FROM pgcolumnar.read_projection('secret','p1');" | grep -c '^ERROR:.*row-level security')" \
"1"
# 0A000, not a phrase. PgColumnarRequireNoRowSecurity raises
# ERRCODE_FEATURE_NOT_SUPPORTED, which is a different SQLSTATE CLASS from either
# ACL refusal -- 0A against 42 -- so no rewording of any message can confuse them.
check "and the refusal says the feature is not supported, not that a privilege is missing" \
"$(state_as t_prjsel "SELECT count(*) FROM pgcolumnar.read_projection('secret','p1');")" \
"0A000"

# THE SECOND ORDERING, recorded in src/columnar_vacuum.c as a correction made in
# review. RLS is checked AFTER the relation ACL. With it first, a caller holding
# no privilege at all was told the table has RLS enabled, which ordinary SQL does
# not disclose and which disagrees with core. Measured there:
#
# RLS on, no-select, ordinary SQL -> permission denied for table
# RLS on, no-select, read_projection -> row-level security is in force
#
# The arm above is its control: same table, same policy, a role that DOES hold
# SELECT, and 0A000. So 42501 here is the ordering, not RLS being switched off.
check "a caller with no SELECT is told a privilege is missing, not that RLS is in force" \
"$(state_as t_prjexec "SELECT count(*) FROM pgcolumnar.read_projection('secret','p1');")" \
"42501"

pgc_summary
149 changes: 149 additions & 0 deletions test/pytest/TESTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ behaviour, the source of that number is named.
- [32. test_stats_privilege.py: stats is readable only by a caller who may read the table](#32-test_stats_privilegepy-stats-is-readable-only-by-a-caller-who-may-read-the-table)
- [33. test_docs_table_structure.py: a table must stay a table](#33-test_docs_table_structurepy-a-table-must-stay-a-table)
- [34. test_docs_stripe_floor.py: the stripe floor is below a vector](#34-test_docs_stripe_floorpy-the-stripe-floor-is-below-a-vector)
- [35. test_projection_privilege.py: the projection read helpers are a privilege boundary](#35-test_projection_privilegepy-the-projection-read-helpers-are-a-privilege-boundary)
- [36. test_compare_to_bash.py: the parity tool reads the NAME](#36-test_compare_to_bashpy-the-parity-tool-reads-the-name)

## 1. How to read a test in here

Expand Down Expand Up @@ -3274,3 +3276,150 @@ anything.

The shell twin is three arms in `docs_style.sh`: `grep` for the two one-line pages and
an awk heading walker for `administration.md`. The two halves share no code.

## 35. test_projection_privilege.py: the projection read helpers are a privilege boundary

`read_projection()` and `reconstruct_via_projection()` opened a caller-supplied regclass
and returned its contents with no privilege check, and `CREATE FUNCTION` grants EXECUTE to
PUBLIC. `reconstruct` rebuilds NON-COVERED columns from the base by row number, so the
projection was never the bound on what leaked: one projection on any column exposed the
whole row (#562). A caller holding SELECT but restricted by an RLS policy received every
row (#563).

### Four refusals, four SQLSTATEs, and why the bash suite needed the message

`projection_privilege.sh` decided four things by matching error text, and two of them were
load-bearing rather than decorative. Its own comment says why:

> Both layers reject this role, so a bare "refused" stays true if the REVOKE is deleted and
> the C check catches it instead -- measured: with the REVOKE removed this suite still
> passed 14 of 14.

Both ACL layers raise 42501, so the code alone does not separate them. What separates them
is the fixture:

| the caller | the call | outcome |
| --- | --- | --- |
| no EXECUTE | a projection that does not exist, on a table it MAY read | `42501` -- the body never ran |
| EXECUTE | the same call | `42704` -- it ran and reached the lookup |
| EXECUTE, no SELECT | a real projection on a table it may NOT read | `42501` -- the base ACL |
| SELECT, under a policy | the same | `0A000` -- `ERRCODE_FEATURE_NOT_SUPPORTED` |

The refusal is attributed by what the code REACHED. That is a fact about execution, and no
rephrasing of either message can move it. Both harnesses now do this: the shell half reads
the SQLSTATE through `psql -v VERBOSITY=sqlstate`.

### Two orderings the source asserts and nothing tested

| arm | what breaks without it |
| --- | --- |
| `test_the_base_acl_is_checked_before_the_projection_is_looked_up` | a caller with no SELECT learns whether a named projection exists on a table it may not read |
| `test_the_acl_is_checked_before_rls_so_no_privilege_discloses_no_rls_state` | a caller with no privilege is told the table has RLS enabled, which ordinary SQL does not disclose |

### Every arm

| test | what it holds |
| --- | --- |
| `test_the_premises_the_fixture_is_what_the_suite_assumes` | each role opens its own session, the owner gets rows from both helpers, and reconstruct really does return the non-covered column |
| `test_a_role_with_only_schema_usage_is_refused` | layer one, the SQL grant, per function |
| `test_a_role_with_execute_but_no_select_is_refused` | layer two, the C check, reached only because EXECUTE was granted |
| `test_which_layer_refused_without_reading_the_message` | which of the two, decided by what the code reached rather than by wording |
| `test_the_base_acl_is_checked_before_the_projection_is_looked_up` | the first ordering |
| `test_a_role_with_select_still_reads` | THE CONTROL: a bar that refused everyone is not a fix |
| `test_a_policy_restricted_caller_is_refused` | RLS, as `0A000` rather than a phrase |
| `test_the_acl_is_checked_before_rls_so_no_privilege_discloses_no_rls_state` | the second ordering |

The second is a correction `src/columnar_vacuum.c` records being made in review. Neither
had a test in either harness.

### Removal proof

Five mutations, each asserted to apply at both call sites before the run:

| mutation | pytest | shell |
| --- | --- | --- |
| drop the base ACL check | 4 arms red | -- |
| drop the RLS refusal | 2 arms red | -- |
| RLS **before** the ACL check | the RLS-ordering arm alone | the RLS-ordering arm alone |
| ACL below the projection lookup but above its raise | no arm red, correctly: 42501 still wins | same |
| ACL below the not-found **raise** | both ordering arms | both, reporting `got [42704] want [42501]` |

The fourth row is the one that says the ordering arms measure an ordering rather than the
presence of a check. The fifth is the disclosure itself, printed.

## 36. test_compare_to_bash.py: the parity tool reads the NAME

`compare_to_bash.py` decides whether a port is one-for-one with its bash suite, which is
#432's definition of done. It was reading the wrong argument.

The python side was matched with `expect\.\w+\([^)]*?"([^"]+)"...`, and `[^)]*?` is lazy,
so it stopped at the FIRST quoted argument:

| call | name read |
| --- | --- |
| `expect.num(got, 1, NAME)` | `NAME` -- correct, which is why it looked right |
| `expect.sqlstate(err, "42501", NAME)` | `"42501"` |
| `expect.text(got, "none", NAME)` | `"none"` |

So every SQLSTATE assertion was read as the literal `42501`, reported as an "extra" the
bash suite lacks, while the real property was reported MISSING. #432's ports are exactly
the ones replacing a grep on a message with a SQLSTATE assertion, so **the tool went blind
in proportion to the work being done well.**

### Measured over the tree

| pair | missing before | after |
| --- | --- | --- |
| differential | 6 | 0 |
| hilbert_locality | 13 | 0 |
| native_ownership | 1 | 0 |
| native_projection | 0 | 0 |
| projection_privilege | 23 | 0 |
| stats_privilege | 9 | 0 |
| zonemap_boundaries | 9 | 0 |
| **total** | **61** | **0** |

Of the 61, 34 were never missing. The rest were real, and four of them are closed here:
`stats_privilege` had invented a name for a property the bash suite already named, and
`zonemap_boundaries` was missing its liveness premise outright. Neither was visible while
the tool was reporting the wrong string.

### What the parser reads

| shape | read as |
| --- | --- |
| the last string argument | the name |
| an f-string | a `{}` template, matched against bash interpolations reduced the same way |
| `"a" if cond else "b"` | both arms |
| `@pytest.mark.parametrize("func,name", ROWS)` | the `name` column, resolved through module constants |
| anything else | nothing -- absent is better than wrong |

`$1` is the commonest interpolation in a bash check name and the first version of the
template reducer missed every one of them, because its pattern required `[A-Za-z_]` after
the dollar.

### Removal proof

| mutation | red |
| --- | --- |
| take the first string argument, as the regex did | the regression arm, the unreadable-name arm, and the whole-tree arm |
| drop the conditional-name case | its own arm, and the whole-tree arm |
| drop parametrize resolution | its own arm, and the whole-tree arm |
| read the name column by position instead of by its declared name | its own arm, and the whole-tree arm |

`test_the_ported_suites_in_this_tree_are_graded_one_for_one` catches all four. It is the
arm that matters: a guard over invented sources proves the extractor reads python, not that
the tool grades THIS tree.

### Every arm

| test | what it holds |
| --- | --- |
| `test_the_name_is_the_last_argument_not_the_first_string` | the regression, over three helpers, one of which always worked |
| `test_a_call_whose_name_is_not_a_literal_contributes_nothing` | absent beats wrong: a false green on a parity tool loses a property in both harnesses |
| `test_an_fstring_name_becomes_a_template` | a runtime-built name is compared by shape |
| `test_a_conditional_name_carries_both_of_its_arms` | `"a" if c else "b"` states two properties |
| `test_a_parametrized_name_is_resolved_from_the_decorator` | the idiom a repeated bash property should be ported to, with a no-`name` decorator as the control |
| `test_the_parametrize_reader_takes_the_column_called_name` | the declared column, not position |
| `test_the_two_harnesses_interpolations_land_on_one_template` | bash and python spell interpolation differently and must meet |
| `test_the_ported_suites_in_this_tree_are_graded_one_for_one` | the standing arm: every pair in the tree, graded |
Loading
Loading