Skip to content

test: parallel_copy and parallel_export_parquet refuse a table the caller holds no grant on; import_export_privilege gets its twin and its ledger (#1341) - #1362

Merged
jdatcmd merged 2 commits into
commandprompt:mainfrom
OffgridwithJD:test/1341-parallel-io-privilege
Oct 14, 2026
Merged

jdatcmd merged 2 commits into
commandprompt:mainfrom
OffgridwithJD:test/1341-parallel-io-privilege

Conversation

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

Part of #1341: the High item "parallel_copy requires INSERT on the target, and parallel_export_parquet requires SELECT", and the twin-parity item for import_export_privilege, which had no pytest twin. Both are fixed here.

The arms

import_export_privilege.sh gains two deny arms, two owner controls, and a file check for the parallel export.

  • Same attacker as the serial arms: USAGE on schema pgcolumnar, EXECUTE, both server-file roles, and no grant on victim. So the table ACL is the only thing that can refuse.
  • Why these checks matter: the parallel functions' loaders and workers skip the executor's permission check, so the check before anything is spawned is the only one.

PG17, main 1933577 against this branch:

mutation main this branch
parallel_copy without its INSERT check sh 19/19 "parallel_copy refuses INSERT…: got [allowed]", and "victim still holds exactly its own rows: got [250] want [200]": the attacker's 50 rows landed
parallel_export_parquet without its SELECT check sh 19/19 "parallel_export_parquet refuses SELECT…: got [allowed]", and "nor by the parallel export: got [written]"

The pytest twin is red under both, too.

The pytest twin

test_import_export_privilege.py covers every shell check by the same name. compare_to_bash grades it complete, and it is added to COMPLETE.

Like test_stats_privilege.py:

  • every attacker call is a real login, not SET ROLE;
  • a refusal is SQLSTATE 42501 and a message naming the table, where the shell greps "permission denied for table";
  • files go in server_dir.

The deny arms run in the shell's order in one test, and the consequence checks are asked once after them. So under a mutation, the hard assert stops on the arm whose check is gone.

Bookkeeping and evidence

  • Clean: import_export_privilege 24/24 and the twin's 4 tests passed on each of PG 15–19.
  • Ledger seeding: this suite had no ledger rows, so it is seeded with all 24 checks. 2343 → 2367 rows, the census 1796 → 1816, and suites_not_covered 246 → 245.
  • Dated rows: the four checks above. The five serial deny arms enter never-red. I didn't re-mutate their checks here, and can in a follow-up.
  • Gate: rc=0, and the ceiling falls.
  • Counts: TESTS.md section 105, cluster_tests 533 → 537, database-free set 429 passed.

🤖 Generated with Claude Code

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving the content at d1159aa. On PG17 I reproduced the claim with parallel_copy's ACL_INSERT check replaced by ACLCHECK_OK:

  • main: import_export_privilege passes 19/19.
  • this head: "parallel_copy refuses INSERT into a table the caller cannot write: got [allowed]" and "victim still holds exactly its own rows: got [250] want [200]". The attacker's rows landed.

The twin's real logins and its message-plus-SQLSTATE refusals are right. Before merge: rebase onto the main of its turn and restate the budget and the ceiling (246 → 245, or lower if another seeding PR lands first).

jdatcmd and others added 2 commits October 14, 2026 02:54
…ller holds no grant on; import_export_privilege gets its pytest twin (commandprompt#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ns' arms dated (commandprompt#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

@jdatcmd, rebased onto 4027b8d (#1358 merged). The new head is 67e5d32, replacing d1159aa.

Restated:

Conflicts:

Evidence still applies: since its base, main has changed no product file and neither of this suite's files, so 24/24 on 15–19 and both mutant results stand.

Checks: the gate gives rc=0, with the census agreeing and the ceiling falling. The database-free set gives 429 passed, collection 538.

I'll ping on green CI.

@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

@jdatcmd, CI is green on 67e5d32 (15/15). Ready for your merge; #1363 is next, and I'll restate it as soon as this lands.

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approving the rebased head 67e5d32. I checked it against the head I reviewed (d1159aa):

  • Patch: identical outside the three count files. Only index hashes and hunk offsets differ.
  • Ledger: 2385 rows / 1821 never (main 2361/1801). No duplicate keys, no dated premise rows, and none of main's rows dropped or changed.
  • Ceiling: suites_not_covered 245.
  • CI: 15/15 green.

Merging.

@jdatcmd
jdatcmd merged commit 9b3c181 into commandprompt:main Oct 14, 2026
15 checks passed
@jdatcmd

jdatcmd commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator

@OffgridwithJD, merged as 9b3c181.

Main now: 2385 rows / 1821 never, suites_not_covered 245, cluster_tests 538.

Next: #1363, once you restate it on 9b3c181.

The nightly went red (#1382): the coverage job installs neither pg_partman, hll nor tdigest. #1291 and #1296 added those packages to every other suite job. I have a fix and a selftest guard, red first, and its ledger runs are in progress. I'll open it as a PR within the hour. It touches only nightly.yml, the selftest and the ledger, so it can land between your PRs without colliding with them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants