Standardize server-file privilege on roles (#330)

[ChronicallyJD]

Resolves #330. Standardizes every server-file function on the core roles (option 1 from the issue), the model core COPY ... FROM/TO 'file' uses.

Change

Relaxed the seven superuser() checks to has_privs_of_role(...):

function	was	now
import_parquet, read_parquet, parquet_schema, parquet FDW scan, import_arrow	superuser()	pg_read_server_files
export_parquet, export_arrow	superuser()	pg_write_server_files

parallel_copy and file_split_offsets already used pg_read_server_files; parallel_export_parquet (this stack) uses pg_write_server_files. A superuser holds both roles, so this only widens access to role holders.

• docs/administration.md — the blanket "requires superuser" sentence replaced, the table completed with all ten entry points and their role, the untrusted-input note updated.
• CHANGELOG.md — the deliberate-loosening entry.
• test/server_file_privilege.sh — rebuilt over the full set. Per point it asserts a role-less caller is refused by the role name and a role holder gets past the gate; plus a coverage check that turns red if a SQL function declaring a file-path argument is missing from the list, so a new server-file function can't slip past. This is the durable half you asked for.

Validation

• Gate green: preflight 0-warning on all five majors (15–19); full matrix ALL VERSIONS PASSED on pg18a/pg19a; server_file_privilege, parquet_export/parquet_import, arrow_export/arrow_import, native_read_parquet, native_parquet_fdw, parallel_copy, parallel_export_parquet all pass. server_file_privilege.sh is 30/30.

The security call I want you to make

This is a deliberate loosening, and the sharp edge is the read side: import_parquet, read_parquet, parquet_schema, the FDW, and import_arrow parse files this project wrote, so a role short of superuser can now reach those parsers. #216 kept them at superuser precisely until the parser fuzzing is done, and Arrow fuzzing (#214) is still open. I implemented option 1 because you leaned that way and it is the coherent model, and I documented the exposure in docs/administration.md and the CHANGELOG. But if you'd rather keep the untrusted-parser readers at superuser for now and only role-gate the writers + the text-path readers (parallel_copy, file_split_offsets, which use core's trusted COPY parser), that is a one-line-per-site revert — say the word and I'll adjust.

Stacked on #329 (export_parquet's privilege is coupled to that PR's refactor + parallel_export_parquet lives there). This PR therefore shows #329's two commits as well as the #330 commit (0cf8fb4) until #329 merges, after which the diff reduces to #330 alone. Review the 0cf8fb4 commit for the #330 change; #329 you already verified. No sanitizer run: the change is privilege checks, and #329's code already went through pg18_san.

[jdatcmd]

Review: the right resolution, and the durable half actually works

This is option 1 from #330 and I think it is the correct call: the role gate is
what core COPY ... FROM/TO 'file' uses, and the roles exist precisely so this
capability can be delegated without superuser.

Verified myself

The role mapping is right. I enumerated every gate in the branch: writers
(export_parquet, export_arrow, parallel_export_parquet) take
pg_write_server_files; readers (import_parquet, read_parquet,
parquet_schema, the FDW scan, import_arrow, file_split_offsets,
parallel_copy) take pg_read_server_files. No writer is gated on the read role,
which was the thing worth checking.

The coverage check works, proved by removal. I added an ungated function to the
extension SQL:

CREATE FUNCTION pgcolumnar.peek_server_file(path text) RETURNS bigint ...

FAIL  server-file function is covered by the boundary test: peek_server_file: got [no] want [yes]

30 checks clean, 31 with the injection, and it names the offending function. That
is the part of #330 I actually cared about: the docs drifted once and would drift
again, and now they cannot do so silently.

The anti-vacuity assertion on the discovery itself ("coverage check found the file
functions (not an empty scan)", requiring at least 7) is the right instinct. Without
it a broken awk would make the whole coverage loop pass by finding nothing.

The CHANGELOG says the quiet part. "This is a deliberate loosening", plus the
note that the read functions parse files this project wrote and are now reachable
below superuser while Arrow parser fuzzing (#214) is incomplete. That is the
sentence a reader needs and the easiest one to omit.

One leftover, and it is now self-contradicting

src/columnar_parquet_reader.c:3547, in pqfdwGetForeignRelSize:

/* ... The stat() is gated on superuser -- the same bar the scan
 * enforces -- so a non-privileged planner cannot use the EXPLAIN estimate to
 * probe whether a server-side path exists or how big it is. */
if (superuser() && path != NULL && stat(path, &st) == 0 && st.st_size > 0)

The guard itself is good thinking: it stops the row estimate becoming a
file-existence side channel. But its stated justification, "the same bar the scan
enforces", is exactly what this PR changes. The scan now enforces
pg_read_server_files.

Consequence is not a security hole, it is the reverse: a user who legitimately
holds pg_read_server_files can read the file through the FDW but gets the flat
1000-row default instead of the size-derived estimate, so they plan worse than
before for no reason. The gate should follow the scan to pg_read_server_files,
and the comment updated with it.

It also escaped the new coverage check, which is fair, since that check scans the
extension SQL for file-path arguments and this is an internal planner hook rather
than a SQL function. Worth knowing the check's boundary rather than assuming it
catches everything of this shape.

Note on scope

This branch also carries #329's parallel_export_parquet (+653 and its suite), so
merging this merges that. The snapshot blocker there is fixed and I verified it,
but the open items from my #329 review still apply: the parent-versus-leaf tupdesc
check, the leaf list re-derived from the live catalog, BgWorkerStart_RecoveryFinished
blocking hot-standby export, the missing dsm-detach callback, per-call buffer
leaks, and no cleanup of partial output on failure.

If you would rather land the privilege standardisation on its own, splitting it out
would make both easier to reason about, since one is a security-model change and
the other is a feature.

[ChronicallyJD]

Fixed the FDW estimate gate in c395c39: pqfdwGetForeignRelSize's stat() now gates on pg_read_server_files, matching the scan, and the comment is corrected. Good catch — my grep was for !superuser(), so this positive-form guard slipped past; it is now the last superuser() in the server-file paths (grep confirms zero remain). A role holding pg_read_server_files gets the size-derived estimate again instead of the flat default. Re-gated: build clean, server_file_privilege 30/30, native_parquet_fdw and native_parquet_pushdown green.

On the #329 items in your scope note

The parent-versus-leaf tupdesc check, the leaf list re-derived from the live catalog, BgWorkerStart_RecoveryFinished, the dsm-detach callback, and the per-call buffer leaks are all addressed in 0a3f87d — the commit you verified the snapshot fix on — so I think the note crossed with my #329 reply. Concretely on that branch: the dispatcher validates each leaf and ships the oid-sorted leaf list in the DSM (workers no longer re-derive it); workers start at BgWorkerStart_ConsistentState; the spawn/wait is wrapped in PG_ENSURE_ERROR_CLEANUP; each partition runs in its own memory context.

The one I did not change is partial output on failure: a failed export leaves its part-*.parquet behind, the same as a failed COPY TO, and the create-or-require-empty directory policy stops a later run from mixing them. Happy to add best-effort unlink on failure if you want it.

On splitting the privilege change from the feature

I'd like to, and the coupling is only two spots: export_parquet's role change sits on #329's refactored wrapper, and the boundary test's data list includes parallel_export_parquet, which lives in #329. A #331 rebased straight onto main without #329 would leave export_parquet at superuser and drop that one test row until #329 lands. So the clean separation falls out of merge order: merge #329, then #331's diff auto-reduces to the model change alone (0cf8fb4 + c395c39). If you'd rather I physically split them now, I can, at the cost of that temporary inconsistency — your call.