Three least-privilege database roles (task 42) #28
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/database-provisioning"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
A
provisioncommand onPlaceMark.Databasecreates three roles. The application role holdsSELECT/INSERT/UPDATE/DELETE, owns nothing and is refused every form of DDL; the schema-upgrade role is the only one that may create anything; the seed role reads and inserts only. Recorded in ADR-0031.Two mechanisms keep the separation true as the schema grows — this is the part worth reviewing:
ALTER DEFAULT PRIVILEGES, not per-script grants. A table created by a future DbUp script would otherwise be owned by the upgrade role and unreachable by the application, months after anyone remembers why.publicinto aschema_historyschema the application role has noUSAGEon, because default privileges cannot exclude a single table.The seed guard's
LOCK TABLE … SHARE ROW EXCLUSIVEis unblocked by grantingMAINTAIN— the only qualifying privilege that does not also let a seed credential change or delete a row.Proved against real PostgreSQL rather than described:
RoleProvisionerTestsprovisions a container, upgrades the schema as the upgrade role, and asserts the refusals, the future-table grant and the seed running end to end. Removing theMAINTAINgrant or the default privileges each fails exactly the test that claims it.Production provisioning is not done and cannot be from here — no cluster, no host, the same reason task 45 is parked. What it needs is not code: a PostgreSQL cluster with an empty database, an administrative credential to run
provisionagainst it once with the two passwords and no seed password, and a host holdingConnectionStrings__PlaceMarkasplacemark_appplus the upgrade role's credential wherever ADR-0023's out-of-band upgrade runs from.docs/adr/README.mdwill conflict with the PRs in flight — keep the rows in numeric order.The DDL refusals hold — I could not find a form the application role gets through, including temp tables,
pg_tempobject creation,COPY … FROM/TO PROGRAM,CREATE EXTENSION,SET ROLE,lo_import,DOblocks and grant/owner changes.REVOKE ALL … FROM PUBLICremovingTEMPis what closes thepg_tempsearch-path route, which is worth more than it looks. Findings below.log_min_error_statement=errora failed run logsSTATEMENT: ALTER ROLE placemark_app WITH … PASSWORD 'SUPERSECRETAPPPW'verbatim (observed against 18.4), and on any cluster withlog_statement=ddl|allevery run does. Send a pre-computedSCRAM-SHA-256$…verifier as the password literal instead of the cleartext, or state the exposure in ADR-0031.RoleProvisioner.ProvisionAsyncnever removes the seed role: re-provisioning a database that already hasplacemark_seedwhile supplying noPLACEMARK_SEED_ROLE_PASSWORDleaves it able to log in with its old password and read every row (verified). "Absence is the switch" holds only on first provisioning — drop the role when no password is supplied, or say so in ADR-0031 and the README.ALTER DEFAULT PRIVILEGES … IN SCHEMA publicreproduces its own trap one level up: a future script doingCREATE SCHEMA x; CREATE TABLE x.tleaves the application role with noUSAGEand no grant, deployment reports success, and theON ALL TABLESfallback does not reach it either (verified). Worth an ADR consequence at minimum, better a test that fails if the scripts ever create a schema other thanpublic.SECURITY DEFINERfunction created inpublicbyplacemark_upgradeis executable byplacemark_app(EXECUTE goes toPUBLICby default) and hands it the upgrade role's DDL — verified end to end with a helper that creates a table. AddALTER DEFAULT PRIVILEGES FOR ROLE placemark_upgrade IN SCHEMA public REVOKE EXECUTE ON FUNCTIONS FROM PUBLIC.SeedSchemaPrivilegesSqlgrants the seed role nothing on sequences, so the first table with aserial/identity column fails the seed withpermission denied for sequence …(verified); mirror the application role'sUSAGE, SELECT ON SEQUENCESin both the default privileges and theON ALLgrant.MAINTAIN… isVACUUM,ANALYZE,REINDEXandLOCK, none of which is a write" is incomplete: it also carriesCLUSTERandREFRESH MATERIALIZED VIEW, and the seed role successfully refreshed a matview owned by the upgrade role — a content-replacing write on any matview a future script adds.VACUUM FULL/CLUSTER/LOCK … ACCESS EXCLUSIVEalso let the seed stall the API. Adjust the sentence and theCannotcolumn rather than the grant.ON ALL TABLESgrant does reach tables owned by someone else — I provisioned over a superuser-created table and the application role read it. What genuinely is not rescued is future tables from the wrong role and the journal's location; the "throw it away" advice is right for the second reason only.placemark_appcan runALTER ROLE placemark_app PASSWORD …andALTER ROLE placemark_app SET search_path …(both verified), locking the API out persistently. Nothing to fix, one clause to soften.Verdict: changes required
Paused mid-fix. Partial and unverified work is on
wip/database-provisioning-review-fixes(36f1970), committed only because the worktree was session-scoped — rebase or discard it, do not merge it. All eight findings above still stand againstb566fe8.A provisioning run no longer sends PostgreSQL a password. It computes the SCRAM-SHA-256 verifier itself and sends that, because a role statement that fails is written to the server log verbatim under PostgreSQL's own defaults — so a failed run used to leave every role password in a file that outlives it. The cost is that passwords must be printable ASCII, refused before a connection is opened, since the SASLprep normalisation the server would apply is not reproduced here and is the identity only over that range. Three narrower gaps, each found by attack rather than by reading: - A run that supplies no seed password now drops the seed role rather than merely not creating it, and the command names what it dropped. Absence was the switch on the first run only; a seed credential added once survived, with its old password, the run meant to remove it. - EXECUTE on functions is revoked from PUBLIC, so a SECURITY DEFINER function a future script creates cannot hand the application role the schema-upgrade role's DDL. The revoke is written *without* IN SCHEMA: a schema-qualified default-privilege entry starts from an empty ACL, so the IN SCHEMA form reports success, stores nothing, and leaves the next function executable by everyone. - The seed role is granted USAGE on sequences, which the first `serial` column would otherwise have failed on. ALTER DEFAULT PRIVILEGES still covers `public` alone, and no default privilege can grant USAGE on a schema that does not exist yet. That limit is now stated in ADR-0031, with what an operator must do, and a test fails the build if a script ever creates a schema beyond `public` and the journal's. Four claims in ADR-0031 and the README are corrected to what was measured: MAINTAIN also carries CLUSTER, VACUUM FULL, REFRESH MATERIALIZED VIEW and every lock mode; the ON ALL TABLES grant does reach tables another role created, so what a legacy database loses is future tables and the journal's location; and the application role can always alter itself, so a compromised credential can lock the API out even though it can escalate nothing.All eight actioned in
d5c78ef(the WIP branch was cherry-picked, then finished and corrected — itsALTER DEFAULT PRIVILEGES … IN SCHEMA public REVOKE EXECUTE ON FUNCTIONS FROM PUBLICis a silent no-op, since a schema-qualified default-privilege entry starts from an empty ACL; the database-wide form is what closes it).wip/database-provisioning-review-fixescan be deleted.All eight are closed: the verifier authenticates (including a password full of
$ ' \ ; --), no password fragment survives anywhere in the log withlog_statement=allplus a failing role statement on both the CREATE and ALTER paths, theSECURITY DEFINERroute is shut for functions and procedures and in schemas created later, theIN SCHEMA publicform really does store nothing, the seed drop cleans its default-ACL entries and round-trips, and the schema guard test reddens naming the offending schema when a script adds one. Two things left.ALTER DEFAULT PRIVILEGES … GRANT USAGE ON SCHEMASdoes exist (database-wide form, noIN SCHEMA), so ADR-0031's "PostgreSQL has no default privileges for schemas, so no provisioning written in advance can fix it" is wrong: set forplacemark_upgradeon 18.4 it stores adefaclobjtype='n'row and the application role hadUSAGEon a schema the upgrade role created afterwards, with nothing written in advance about its name. Add it beside the function revoke and narrow the consequence — the residual limit is then only the table and sequence grants inside a new schema, which is a smaller thing to have to remember than the paragraph currently describes..env.exampleinvites aplacemark_scratch), provisioning the second with no seed password fails on2BP01: role "placemark_seed" cannot be dropped because some objects depend on it, and Npgsql redacts the DETAIL that would name the other database — so the production-shaped invocation fails with a message that names no cause and no remedy. The transaction rolls back cleanly and the first database is untouched, so this is message quality only: catch2BP01inProvisionCommandand say that the seed role holds privileges in another database of the cluster.Verdict: changes required
Both done in
98a0ef0:GRANT USAGE ON SCHEMASadded for the application and seed roles, with the consequence narrowed to the table and sequence grants inside a new schema, and2BP01caught inProvisionCommandwith the cause and the three remedies.One knock-on worth knowing: that grant follows every schema the upgrade role creates, so it would have handed the application
USAGEonschema_history— provisioning now creates the journal schema itself (administrative-role-owned, outside the upgrade role's default privileges), and the existing journal test reddens if that is undone.Both closed, and the journal knock-on is handled correctly: on a fresh database
schema_historycomes out owned by the administrative role with{postgres=UC,placemark_upgrade=UC}and neitherplacemark_appnorplacemark_seedholdingUSAGE, the refusal still names the schema, and a re-provision does not change that. The narrowed limit is exactly right — a schema the upgrade role adds later is reachable while its table is not, and the refusal names the table. Removing the provisioning-side creation reddens the journal test as claimed. The2BP01message names the cause, all three remedies and that nothing changed; I agree it needs no README section, since it appears precisely when it is relevant and nowhere else is a reader looking.DROP SCHEMA schema_historyfollowed by a schema upgrade recreates it as the upgrade role, where it picks up the database-wideUSAGEfor both roles, and a laterprovisionleaves that in place (CREATE SCHEMA IF NOT EXISTSno-ops, nothing revokes). No data is exposed — the journal table still carries no grant because the table defaults areIN SCHEMA public, so the app is refused at the table instead of at the schema — so this is hardening rather than a hole:REVOKE ALL ON SCHEMA {JournalSchema} FROM {ApplicationRole}, {SeedRole};after theCREATE SCHEMA IF NOT EXISTSwould make it an invariant every run re-establishes rather than a property of who created it first. Take it or leave it; it does not need to hold up the merge.Verdict: mergeable
Merge confirmed at
e277870: two parents, the only conflict resolved as one added row in numeric order, and relative to3d42fc0the branch delta is its own 17 files and nothing else. The architecture rules judgePlaceMark.Domain,PlaceMark.Contracts,PlaceMark.WebUI,PlaceMark.Infrastructureas the positive control, and every tracked.props/.targets/.rsp/.user— this branch touches none of those projects, adds no build file and no package, and the provisioning work needs nothing they forbid (the SCRAM verifier is in-boxSystem.Security.CryptographyinsidePlaceMark.Database, which nothing judged reaches). Clean Release rebuild,dotnet formatclean, and all 220 tests pass in Release including the 27 Testcontainers-backed provisioning tests, so the integration path still runs rather than only compiling.Verdict: mergeable