feat(defaults): drop the schema-wide public grants, add verify/revert/tests - #113
Merged
Merged
Conversation
…/tests The module revoked PUBLIC's database and function privileges and then handed schema public straight back to authenticated, anonymous and administrator, reopening as a blanket role grant what the revoke had just closed. Schema public holds only extension functions (which keep PUBLIC=X from before the revoke) and the pgpm-verify deploy helpers, so the grants added redundant ACL entries and one real hole. A function in public now needs an explicit grant. verify/ and revert/ were still stubs: verify now proves the four properties the deploy claims (no PUBLIC database CREATE, no schema public CREATE, a database-wide function default for the owner that excludes PUBLIC, no schema-wide default in public), and revert restores PostgreSQL's out-of-the-box PUBLIC privileges. Tests cover the fail-closed grant behavior and the ordering requirement: an extension created after this module has no PUBLIC execution, so the module must be deployed last.
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
defaults/publicrevoked PUBLIC's privileges and then immediately handed schemapublicback to three roles wholesale:Measured on a real deploy, schema
publicholds 1,499 extension functions + 19pgpm-verifydeploy helpers and zero application functions — and all 1,518 already carry=Xfrom before the revoke, because extensions are created earlier. So the first grant only added redundant ACL entries, while the default grant kept every future function inpublicreachable byanonymous— a blanket anonymous surface, which is exactly what the revoke above exists to prevent. Both are gone; a function inpublicnow needs an explicit grant, which fails closed.verify/andrevert/were still-- XXXstubs. Verify now proves the four properties the deploy claims, the load-bearing one being that the database-wide default exists and excludes PUBLIC — only the schema-less, role-scoped form ofALTER DEFAULT PRIVILEGESreaches schemas other thanpublic, so the check is ondefaclnamespace = 0:Revert restores PostgreSQL's out-of-the-box PUBLIC privileges (
CREATEon schemapublic, the function default,CONNECT/TEMPORARYon the database).Ordering (why the new extension test matters)
ALTER DEFAULT PRIVILEGESis prospective, so an extension created after this module never getsPUBLIC=Xand its functions become uncallable — deployingpgpm-defaultsbefore extensions breaks 47citext+ 37pgcryptofunctions, and'Abc'::citext = 'abc'::citextfails with permission denied for function citext_eq, which looks nothing like a permissions bug at the call site. The newextension orderingtest pins that failure mode so the requirement (deploy this module last) can't be silently dropped.17/17 tests pass; the snapshot's
default_func_acl_countdrops 2 → 1 with the schema-scoped default gone.Context
Split out of constructive-io/constructive-db#2734, which wires
pgpm-defaultsinto the deployment graph — the module is vendored there, and constructive-db's CI doesn't runpgpm-modules/**jest suites, so the change and its tests belong here.Link to Devin session: https://app.devin.ai/sessions/76e3c8aa95f24cd1af720e76280ddf90
Requested by: @pyramation