roles phase 3: dev tools are a row, not a hardcoded name
rolePermitsDevTools() was `role === 'Developer'` — one line gating two real grants, a Postgres login role with CREATEDB and a rootless Docker daemon. A role the owner created at runtime could never have either, which defeats the point of roles being data. Two boolean columns on `roles` now carry it, and roleDevGrants() reads the row. A role that no longer exists reads as neither grant, matching "absence denies" everywhere else. NOT the `provisioningScript` D8 asked for, and that column is removed rather than left dead. A shell script stored on a role row and run at provisioning time is arbitrary root-level code in a database column, and D8's own warning says why that is worse than it looks: `usermod -aG docker` is root by a longer route. So the row CHOOSES from a vocabulary the platform already knows how to carry out — the same shape as `multiarch` on a plugin's osDependencies, for the same reason. The seed re-asserts both flags on every run for system roles, and only for them. Without that, existing rows take the column default of false the moment it lands and Developer silently loses its tools on the next deploy — every host already has these four rows, so an insert-only seed would have shipped that. `description` stays insert-only, because an edit made on one server must survive a deploy. `db:push` could not apply this and said nothing. Dropping a column while adding another makes drizzle-kit ask whether it is a rename; with no terminal it bails and the wrapper still exits 0, leaving the seed to fail against an unchanged schema. Recorded in src/databases/CLAUDE.md with the SQL, because it will hit every host on the next deploy and no agent driving a rollout has a terminal. Verified here: SQL applied guarded, Developer keeps both grants and no other role gained any, second push emits zero DDL and no prompt, 1423 pass / 0 fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Officer-Session: 656f5c85-e49a-4f1f-b9f3-042a04b4d043
This commit is contained in:
@@ -2,7 +2,7 @@ import type { Handler } from 'hono';
|
||||
import { getUsers, getUserById, updateUser, deleteUser, listRoles, getRole, OWNER_USER_ID } from 'officerdb';
|
||||
import * as errors from '@@/custom-errors';
|
||||
import { deprovisionOsAccount } from '@@/os-user-deprovision';
|
||||
import { dropPostgresRole, provisionPostgresRole, rolePermitsDevTools } from '@@/os-user-postgres';
|
||||
import { dropPostgresRole, provisionPostgresRole } from '@@/os-user-postgres';
|
||||
import { lookupOsUser } from '@@/os-user';
|
||||
import { DATA_PATH } from '@@/data-path';
|
||||
import { broadcastProvisionableUsers } from '@@/plugins/user-provisioning';
|
||||
@@ -71,9 +71,11 @@ export const updateUserRoleHandler: Handler = async function (ctx) {
|
||||
// The runtime replacement for what USER_ROLES used to guard at the type level (docs/roles-and-teams.md
|
||||
// phase 2 part 4): a role that is not a row in `roles` must never reach `updateUser`, or the FK on
|
||||
// `users.role` throws a raw Postgres error instead of this message.
|
||||
if (typeof role !== 'string' || !(await getRole(role))) {
|
||||
throw errors.BAD_REQUEST(`Unknown role '${String(role)}'`);
|
||||
}
|
||||
if (typeof role !== 'string') throw errors.BAD_REQUEST(`Unknown role '${String(role)}'`);
|
||||
// Kept rather than re-fetched below: this is the one read of the DESTINATION role's row this handler
|
||||
// needs, and `wants` reuses it instead of asking the database the same question twice.
|
||||
const newRole = await getRole(role);
|
||||
if (!newRole) throw errors.BAD_REQUEST(`Unknown role '${role}'`);
|
||||
|
||||
// ck_users_owner_is_super_admin would reject this anyway — the point of catching it here is the
|
||||
// message. A raw CHECK violation surfaces as a 500 with Postgres wording, which tells the person
|
||||
@@ -101,7 +103,7 @@ export const updateUserRoleHandler: Handler = async function (ctx) {
|
||||
|
||||
// ── The database role follows the platform role ──
|
||||
//
|
||||
// Without this the gate in `rolePermitsDevTools` is decorative: it would decide what a Developer gets at
|
||||
// Without this, `grantsPostgres` on the role row is decorative: it would decide what a Developer gets at
|
||||
// creation and then never look again, so demoting one would leave their Postgres role, their databases
|
||||
// and a working password in their ~/.zshenv. A permission that survives its own revocation is worse than
|
||||
// not having gated it, because the UI then says something untrue.
|
||||
@@ -109,8 +111,13 @@ export const updateUserRoleHandler: Handler = async function (ctx) {
|
||||
// REVOKE BEFORE RECORDING, grant after. Dropping first means a failure aborts with the role unchanged, so
|
||||
// the account still says Developer and the whole thing can be retried. Doing it the other way round would
|
||||
// leave a Member holding database access with nothing in the row to indicate it.
|
||||
const had = rolePermitsDevTools(existing.role);
|
||||
const wants = rolePermitsDevTools(role);
|
||||
//
|
||||
// Docker does NOT follow the role here — only Postgres does. That is a pre-existing gap (docker was
|
||||
// never wired into role change, only into initial provisioning in provision-os.ts), not something this
|
||||
// change introduces or was asked to close.
|
||||
const oldRole = await getRole(existing.role);
|
||||
const had = oldRole?.grantsPostgres ?? false;
|
||||
const wants = newRole.grantsPostgres;
|
||||
|
||||
if (had && !wants && existing.osUser) {
|
||||
const dropped = await dropPostgresRole(existing.osUser);
|
||||
|
||||
Reference in New Issue
Block a user