step 2/4: role_capabilities becomes role_permissions, without losing a grant
The dangerous one. `drizzle-kit push` does not understand renames — it sees a
table gone and a table added, and with --force it resolves that by dropping and
creating. The 9 live grants would have vanished silently and every member would
have lost terminal, chat and files until someone noticed and re-granted by hand.
So the database moved FIRST, by explicit ALTER, and the code followed:
ALTER TABLE role_capabilities RENAME TO role_permissions
ALTER TABLE role_permissions RENAME COLUMN capability TO permission
ALTER INDEX uq_role_capabilities_role_capability RENAME TO uq_role_permissions_role_permission
ALTER TABLE ... RENAME CONSTRAINT ck_role_capabilities_{role,level,not_owner} TO ck_role_permissions_*
ALTER INDEX role_capabilities_pkey RENAME TO role_permissions_pkey
Seven objects, not one, and all in a single transaction: a partial rename would
leave drizzle-kit seeing a table it half-recognised, which is the same drop.
The names were read out of pg_indexes and pg_constraint rather than assumed.
`bun db:push` then reported **No changes detected** — which is the whole proof.
It means the ALTERs matched the schema code exactly, so there was nothing for
push to reconcile and nothing for --force to destroy.
Grants verified against a JSON backup taken before the first ALTER: 9 rows,
byte-identical, `role_capabilities` gone from the database.
No client noticed. Step 1 had already moved the wire to `permission` by mapping
in the route while the column still said `capability`, so this step deleted the
mapping rather than changing any response. The two call sites that carried a
`capability` key with a comment explaining why now say `permission` and the
comments are gone.
Also renamed officer_db/src/capabilities/ → permissions/, roleCapabilities →
rolePermissions, CapabilityLevelValue → PermissionLevelValue.
Verified live after restart: self 200, admin list 200, gated route 200, no
token 401, grants read back as {role, permission, level}, and a PUT round trip
left 9 grants intact.
tsgo clean. 797 tests, 787 pass, same 7.
This commit is contained in:
@@ -31,7 +31,7 @@ export * from './api-keys';
|
|||||||
export * from './app-store';
|
export * from './app-store';
|
||||||
export * from './plugins';
|
export * from './plugins';
|
||||||
export * from './auth';
|
export * from './auth';
|
||||||
export * from './capabilities';
|
export * from './permissions';
|
||||||
export * from './chat-events';
|
export * from './chat-events';
|
||||||
export * from './dashboards';
|
export * from './dashboards';
|
||||||
export * from './integrations';
|
export * from './integrations';
|
||||||
|
|||||||
+1
-1
@@ -2,4 +2,4 @@ export { getAllRoleGrants, getRoleGrants, setRoleGrant, revokeRoleGrant, replace
|
|||||||
|
|
||||||
export type { RoleGrant } from './queries';
|
export type { RoleGrant } from './queries';
|
||||||
|
|
||||||
export type { CapabilityLevelValue } from './schema';
|
export type { PermissionLevelValue } from './schema';
|
||||||
+25
-25
@@ -1,60 +1,60 @@
|
|||||||
import { eq, and } from 'drizzle-orm';
|
import { eq, and } from 'drizzle-orm';
|
||||||
import { db } from '../db';
|
import { db } from '../db';
|
||||||
import { roleCapabilities } from './schema';
|
import { rolePermissions } from './schema';
|
||||||
import type { UserRole } from '../auth/schema';
|
import type { UserRole } from '../auth/schema';
|
||||||
import type { CapabilityLevelValue } from './schema';
|
import type { PermissionLevelValue } from './schema';
|
||||||
|
|
||||||
// Grants, keyed on role. Absence denies — see the table comment.
|
// Grants, keyed on role. Absence denies — see the table comment.
|
||||||
|
|
||||||
export type RoleGrant = {
|
export type RoleGrant = {
|
||||||
role: UserRole;
|
role: UserRole;
|
||||||
capability: string;
|
permission: string;
|
||||||
level: CapabilityLevelValue;
|
level: PermissionLevelValue;
|
||||||
};
|
};
|
||||||
|
|
||||||
/** Every grant, for the settings UI. Ordered so the UI never has to sort. */
|
/** Every grant, for the settings UI. Ordered so the UI never has to sort. */
|
||||||
export async function getAllRoleGrants(): Promise<RoleGrant[]> {
|
export async function getAllRoleGrants(): Promise<RoleGrant[]> {
|
||||||
const rows = await db
|
const rows = await db
|
||||||
.select({
|
.select({
|
||||||
role: roleCapabilities.role,
|
role: rolePermissions.role,
|
||||||
capability: roleCapabilities.capability,
|
permission: rolePermissions.permission,
|
||||||
level: roleCapabilities.level,
|
level: rolePermissions.level,
|
||||||
})
|
})
|
||||||
.from(roleCapabilities)
|
.from(rolePermissions)
|
||||||
.orderBy(roleCapabilities.role, roleCapabilities.capability);
|
.orderBy(rolePermissions.role, rolePermissions.permission);
|
||||||
return rows;
|
return rows;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* What one role may reach, as capability key → level.
|
* What one role may reach, as permission key → level.
|
||||||
*
|
*
|
||||||
* A Map rather than an array because every caller is asking "does this role hold X, and at what level",
|
* A Map rather than an array because every caller is asking "does this role hold X, and at what level",
|
||||||
* and the hot path is a per-request lookup.
|
* and the hot path is a per-request lookup.
|
||||||
*/
|
*/
|
||||||
export async function getRoleGrants(role: UserRole): Promise<Map<string, CapabilityLevelValue>> {
|
export async function getRoleGrants(role: UserRole): Promise<Map<string, PermissionLevelValue>> {
|
||||||
const rows = await db
|
const rows = await db
|
||||||
.select({ capability: roleCapabilities.capability, level: roleCapabilities.level })
|
.select({ permission: rolePermissions.permission, level: rolePermissions.level })
|
||||||
.from(roleCapabilities)
|
.from(rolePermissions)
|
||||||
.where(eq(roleCapabilities.role, role));
|
.where(eq(rolePermissions.role, role));
|
||||||
return new Map(rows.map((r) => [r.capability, r.level]));
|
return new Map(rows.map((r) => [r.permission, r.level]));
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Grant, or change the level of an existing grant. Upsert: the UI has one control, not two. */
|
/** Grant, or change the level of an existing grant. Upsert: the UI has one control, not two. */
|
||||||
export async function setRoleGrant(role: UserRole, capability: string, level: CapabilityLevelValue): Promise<void> {
|
export async function setRoleGrant(role: UserRole, permission: string, level: PermissionLevelValue): Promise<void> {
|
||||||
await db
|
await db
|
||||||
.insert(roleCapabilities)
|
.insert(rolePermissions)
|
||||||
.values({ role, capability, level })
|
.values({ role, permission, level })
|
||||||
.onConflictDoUpdate({
|
.onConflictDoUpdate({
|
||||||
target: [roleCapabilities.role, roleCapabilities.capability],
|
target: [rolePermissions.role, rolePermissions.permission],
|
||||||
set: { level, updatedAt: new Date() },
|
set: { level, updatedAt: new Date() },
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Revoke. Deleting rather than writing a `none` level is what keeps "absence denies" the only rule. */
|
/** Revoke. Deleting rather than writing a `none` level is what keeps "absence denies" the only rule. */
|
||||||
export async function revokeRoleGrant(role: UserRole, capability: string): Promise<void> {
|
export async function revokeRoleGrant(role: UserRole, permission: string): Promise<void> {
|
||||||
await db
|
await db
|
||||||
.delete(roleCapabilities)
|
.delete(rolePermissions)
|
||||||
.where(and(eq(roleCapabilities.role, role), eq(roleCapabilities.capability, capability)));
|
.where(and(eq(rolePermissions.role, role), eq(rolePermissions.permission, permission)));
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -66,12 +66,12 @@ export async function revokeRoleGrant(role: UserRole, capability: string): Promi
|
|||||||
*/
|
*/
|
||||||
export async function replaceRoleGrants(
|
export async function replaceRoleGrants(
|
||||||
role: UserRole,
|
role: UserRole,
|
||||||
grants: { capability: string; level: CapabilityLevelValue }[],
|
grants: { permission: string; level: PermissionLevelValue }[],
|
||||||
): Promise<void> {
|
): Promise<void> {
|
||||||
await db.transaction(async (tx) => {
|
await db.transaction(async (tx) => {
|
||||||
await tx.delete(roleCapabilities).where(eq(roleCapabilities.role, role));
|
await tx.delete(rolePermissions).where(eq(rolePermissions.role, role));
|
||||||
if (grants.length > 0) {
|
if (grants.length > 0) {
|
||||||
await tx.insert(roleCapabilities).values(grants.map((g) => ({ role, capability: g.capability, level: g.level })));
|
await tx.insert(rolePermissions).values(grants.map((g) => ({ role, permission: g.permission, level: g.level })));
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
+16
-16
@@ -15,26 +15,26 @@ import { USER_ROLES } from '../auth/schema';
|
|||||||
// safe to read as the whole truth — an empty table is a platform where members can reach nothing but their
|
// safe to read as the whole truth — an empty table is a platform where members can reach nothing but their
|
||||||
// own account, which is the correct state for a server that has just been installed.
|
// own account, which is the correct state for a server that has just been installed.
|
||||||
//
|
//
|
||||||
// `capability` is a key from src/servers/capabilities/registry.ts and is deliberately NOT constrained here.
|
// `permission` is a key from src/servers/permissions/registry.ts and is deliberately NOT constrained here.
|
||||||
// A CHECK listing the keys would put the registry in two places and make adding a capability a schema
|
// A CHECK listing the keys would put the registry in two places and make adding a permission a schema
|
||||||
// change; worse, renaming one would fail the push rather than telling you what actually broke. The registry
|
// change; worse, renaming one would fail the push rather than telling you what actually broke. The registry
|
||||||
// is the source of truth and the API layer rejects unknown keys on write. What the database does enforce is
|
// is the source of truth and the API layer rejects unknown keys on write. What the database does enforce is
|
||||||
// the shape: a legal role, a legal level, and at most one grant per pair.
|
// the shape: a legal role, a legal level, and at most one grant per pair.
|
||||||
const CAPABILITY_LEVELS = ['read', 'write'] as const;
|
const PERMISSION_LEVELS = ['read', 'write'] as const;
|
||||||
export type CapabilityLevelValue = (typeof CAPABILITY_LEVELS)[number];
|
export type PermissionLevelValue = (typeof PERMISSION_LEVELS)[number];
|
||||||
|
|
||||||
export const roleCapabilities = pgTable(
|
export const rolePermissions = pgTable(
|
||||||
'role_capabilities',
|
'role_permissions',
|
||||||
{
|
{
|
||||||
id: serial('id').primaryKey(),
|
id: serial('id').primaryKey(),
|
||||||
role: text('role', { enum: USER_ROLES }).notNull(),
|
role: text('role', { enum: USER_ROLES }).notNull(),
|
||||||
/** A capability key from the registry. Validated by the API, not by a constraint — see above. */
|
/** A permission key from the registry. Validated by the API, not by a constraint — see above. */
|
||||||
capability: text('capability').notNull(),
|
permission: text('permission').notNull(),
|
||||||
/**
|
/**
|
||||||
* `read` permits safe methods plus mutations under the capability's `personal` sub-paths — the ones
|
* `read` permits safe methods plus mutations under the permission's `personal` sub-paths — the ones
|
||||||
* holding the caller's own data. `write` is unconditional within the capability.
|
* holding the caller's own data. `write` is unconditional within the permission.
|
||||||
*/
|
*/
|
||||||
level: text('level', { enum: CAPABILITY_LEVELS }).notNull().default('read'),
|
level: text('level', { enum: PERMISSION_LEVELS }).notNull().default('read'),
|
||||||
createdAt: timestamp('created_at', { withTimezone: true }).notNull().defaultNow(),
|
createdAt: timestamp('created_at', { withTimezone: true }).notNull().defaultNow(),
|
||||||
updatedAt: timestamp('updated_at', { withTimezone: true }).notNull().defaultNow(),
|
updatedAt: timestamp('updated_at', { withTimezone: true }).notNull().defaultNow(),
|
||||||
},
|
},
|
||||||
@@ -42,27 +42,27 @@ export const roleCapabilities = pgTable(
|
|||||||
// uniqueIndex, not unique().on() — drizzle-kit mis-diffs named composite unique CONSTRAINTS and
|
// uniqueIndex, not unique().on() — drizzle-kit mis-diffs named composite unique CONSTRAINTS and
|
||||||
// re-creates them on every push, which stops db:push on an unanswerable truncate prompt. See
|
// re-creates them on every push, which stops db:push on an unanswerable truncate prompt. See
|
||||||
// src/databases/CLAUDE.md → "Composite keys".
|
// src/databases/CLAUDE.md → "Composite keys".
|
||||||
uniqueIndex('uq_role_capabilities_role_capability').on(table.role, table.capability),
|
uniqueIndex('uq_role_permissions_role_permission').on(table.role, table.permission),
|
||||||
// Postgres will not reject an unknown string in a text column just because TypeScript narrowed it, so
|
// Postgres will not reject an unknown string in a text column just because TypeScript narrowed it, so
|
||||||
// the legal sets are stated here too. sql.raw for the literals: an interpolated JS string binds as a
|
// the legal sets are stated here too. sql.raw for the literals: an interpolated JS string binds as a
|
||||||
// parameter and Postgres refuses a parameter inside a CHECK, which breaks push for the WHOLE schema.
|
// parameter and Postgres refuses a parameter inside a CHECK, which breaks push for the WHOLE schema.
|
||||||
check(
|
check(
|
||||||
'ck_role_capabilities_role',
|
'ck_role_permissions_role',
|
||||||
sql`${table.role} IN (${sql.join(
|
sql`${table.role} IN (${sql.join(
|
||||||
USER_ROLES.map((r) => sql.raw(`'${r}'`)),
|
USER_ROLES.map((r) => sql.raw(`'${r}'`)),
|
||||||
sql`, `,
|
sql`, `,
|
||||||
)})`,
|
)})`,
|
||||||
),
|
),
|
||||||
check(
|
check(
|
||||||
'ck_role_capabilities_level',
|
'ck_role_permissions_level',
|
||||||
sql`${table.level} IN (${sql.join(
|
sql`${table.level} IN (${sql.join(
|
||||||
CAPABILITY_LEVELS.map((l) => sql.raw(`'${l}'`)),
|
PERMISSION_LEVELS.map((l) => sql.raw(`'${l}'`)),
|
||||||
sql`, `,
|
sql`, `,
|
||||||
)})`,
|
)})`,
|
||||||
),
|
),
|
||||||
// The owner is not granted anything, because the owner is not gated by this table at all — isSuperAdmin
|
// The owner is not granted anything, because the owner is not gated by this table at all — isSuperAdmin
|
||||||
// short-circuits every check. A 'Super Admin' row here would be inert, and an inert row that looks
|
// short-circuits every check. A 'Super Admin' row here would be inert, and an inert row that looks
|
||||||
// meaningful is worse than no row: someone would eventually revoke one and expect it to do something.
|
// meaningful is worse than no row: someone would eventually revoke one and expect it to do something.
|
||||||
check('ck_role_capabilities_not_owner', sql`${table.role} <> ${sql.raw("'Super Admin'")}`),
|
check('ck_role_permissions_not_owner', sql`${table.role} <> ${sql.raw("'Super Admin'")}`),
|
||||||
],
|
],
|
||||||
);
|
);
|
||||||
@@ -20,7 +20,7 @@ import { pgTable, serial, text, boolean, timestamp, uniqueIndex } from 'drizzle-
|
|||||||
// ── No userId, for the same reason as `sidecar_installs` ──
|
// ── No userId, for the same reason as `sidecar_installs` ──
|
||||||
//
|
//
|
||||||
// installed server-level, owner-only — this row
|
// installed server-level, owner-only — this row
|
||||||
// permitted per role — role_capabilities
|
// permitted per role — role_permissions
|
||||||
// configured per user — the plugin's own tables
|
// configured per user — the plugin's own tables
|
||||||
//
|
//
|
||||||
// ── `enabled` is not `installed` ──
|
// ── `enabled` is not `installed` ──
|
||||||
|
|||||||
@@ -24,7 +24,7 @@
|
|||||||
// ── Core ─────────────────────────────────────────────────────────────────────────────────────────
|
// ── Core ─────────────────────────────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
export * from './auth/schema'; // users, passkeys, passkey_challenges, token_blacklist
|
export * from './auth/schema'; // users, passkeys, passkey_challenges, token_blacklist
|
||||||
export * from './capabilities/schema'; // role_capabilities — what each ROLE may reach
|
export * from './permissions/schema'; // role_permissions — what each ROLE may reach
|
||||||
export * from './api-keys/schema'; // api_keys
|
export * from './api-keys/schema'; // api_keys
|
||||||
export * from './user-data/schema'; // user_settings, user_state, user_integrations, dock_configs
|
export * from './user-data/schema'; // user_settings, user_state, user_integrations, dock_configs
|
||||||
export * from './dashboards/schema'; // dashboards, screens, dashboard_defaults
|
export * from './dashboards/schema'; // dashboards, screens, dashboard_defaults
|
||||||
|
|||||||
@@ -55,8 +55,7 @@ export const bootstrapHandler: Handler = async function (ctx) {
|
|||||||
for (const role of USER_ROLES.filter((r) => r !== 'Super Admin')) {
|
for (const role of USER_ROLES.filter((r) => r !== 'Super Admin')) {
|
||||||
await replaceRoleGrants(
|
await replaceRoleGrants(
|
||||||
role,
|
role,
|
||||||
// `capability` is the DATABASE's column name, renamed in the step that renames the table.
|
DEFAULT_ROLE_PERMISSIONS.map((permission) => ({ permission, level: 'write' as const })),
|
||||||
DEFAULT_ROLE_PERMISSIONS.map((permission) => ({ capability: permission, level: 'write' as const })),
|
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
} catch (ex) {
|
} catch (ex) {
|
||||||
|
|||||||
@@ -108,16 +108,10 @@ permissionAdminRouter.get('/permissions', ownerGate, async (ctx) => {
|
|||||||
// Roles a grant may name. Super Admin is excluded: the owner bypasses this table entirely, and the
|
// Roles a grant may name. Super Admin is excluded: the owner bypasses this table entirely, and the
|
||||||
// database refuses a row for that role.
|
// database refuses a row for that role.
|
||||||
roles: USER_ROLES.filter((r) => r !== 'Super Admin'),
|
roles: USER_ROLES.filter((r) => r !== 'Super Admin'),
|
||||||
// Mapped rather than passed through: `capability` is the DATABASE's column name, and the wire should
|
// The column is `permission` now too, so this is a straight pass-through again. It was briefly a
|
||||||
// not leak it. Doing it here means renaming the column changes nothing any client can see — and the
|
// mapping — the wire said `permission` while the column still said `capability` — which is what let
|
||||||
// round trip below already expects `permission`, so passing the row straight out left the screen
|
// the table be renamed without any client noticing.
|
||||||
// reading `grant.permission` on an object that only had `grant.capability`. Every role rendered as
|
grants: await getAllRoleGrants(),
|
||||||
// holding nothing, with no error anywhere.
|
|
||||||
grants: (await getAllRoleGrants()).map(({ role, capability, level }) => ({
|
|
||||||
role,
|
|
||||||
permission: capability,
|
|
||||||
level,
|
|
||||||
})),
|
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -130,8 +124,7 @@ permissionAdminRouter.put('/permissions/:role', ownerGate, async (ctx) => {
|
|||||||
const raw = body?.grants;
|
const raw = body?.grants;
|
||||||
if (!Array.isArray(raw)) throw errors.BAD_REQUEST('Expected { grants: [{ permission, level }] }');
|
if (!Array.isArray(raw)) throw errors.BAD_REQUEST('Expected { grants: [{ permission, level }] }');
|
||||||
|
|
||||||
// `capability` is the DATABASE's column name; it becomes `permission` when the table is renamed.
|
const grants: { permission: string; level: 'read' | 'write' }[] = [];
|
||||||
const grants: { capability: string; level: 'read' | 'write' }[] = [];
|
|
||||||
for (const entry of raw) {
|
for (const entry of raw) {
|
||||||
const { permission, level } = (entry ?? {}) as { permission?: unknown; level?: unknown };
|
const { permission, level } = (entry ?? {}) as { permission?: unknown; level?: unknown };
|
||||||
if (typeof permission !== 'string') throw errors.BAD_REQUEST('Each grant needs a permission key');
|
if (typeof permission !== 'string') throw errors.BAD_REQUEST('Each grant needs a permission key');
|
||||||
@@ -149,8 +142,7 @@ permissionAdminRouter.put('/permissions/:role', ownerGate, async (ctx) => {
|
|||||||
: `${known.label} is not grantable`,
|
: `${known.label} is not grantable`,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
// `capability` is the DATABASE's column name until the table rename.
|
grants.push({ permission, level });
|
||||||
grants.push({ capability: permission, level });
|
|
||||||
}
|
}
|
||||||
|
|
||||||
await replaceRoleGrants(role, grants);
|
await replaceRoleGrants(role, grants);
|
||||||
|
|||||||
Reference in New Issue
Block a user