fix(plugin-security): let a member revoke their OWN API key — owner-scoped update on sys_api_key

.changeset/member-revoke-own-api-key.md

@@ -0,0 +1,52 @@
+---
+"@objectstack/plugin-security": patch
+---
+
+fix(plugin-security): a member can revoke their OWN API key — owner-scoped `update` on `sys_api_key` for `member_default` (#8053)
+
+An ordinary member who minted a personal API key could not revoke it.
+`PATCH /api/v1/data/sys_api_key/{their own id} {"revoked": true}` answered **403
+`PERMISSION_DENIED`**, the row stayed `revoked: false`, and the key kept
+authenticating. The `revoke_api_key` / `restore_api_key` row actions rendered in
+that member's own **My Keys** grid the whole time — a dead affordance on the
+persona the surface is built for.
+
+A personal API key acts as its owner ("treat it like a password", per the
+console's own mint screen), and the owner is the person who discovers it leaked.
+Their only remedy was to find an admin.
+
+This is the residual of #7727, one layer down. That fix was correct as far as it
+went: the method gate opened (`enable.apiMethods` gained `update`) and ADR-0092
+D2's column whitelist registered `revoked`. But the **object-CRUD** layer was
+untouched — the platform `member_default` set granted only `allowRead` across the
+better-auth-managed identity tables, so `update` on `sys_api_key` resolved for
+`admin_full_access` and nobody else. `GET /api/v1/security/explain` said so
+outright, as that member: *"No resolved permission set grants update on
+sys_api_key"*. Because #7727's tests all drove the admin, the member half stayed
+hidden behind its fix.
+
+`member_default` now carries an explicit `sys_api_key` entry with `allowEdit`.
+Two pre-existing mechanisms bound it, and the grant is deliberately not bounded
+by the permission-set boolean alone:
+
+- **which rows** — the `sys_api_key_self` RLS carve-out
+ (`user_id == current_user.id`), which already made the row owner-*visible*;
+ there was simply no `allowEdit` to go with it. A member PATCHing another
+ user's key is still refused **403**, row unchanged.
+- **which fields** — ADR-0092 D2's identity write guard, whose per-object update
+ whitelist for this table lists `revoked` alone. `key` stays unwritable (a
+ rotated hash would mint a credential nobody holds) and `user_id` stays
+ unwritable (re-owning a key is privilege transfer) — both are stripped even
+ when smuggled alongside a legal `revoked`.
+
+**Unaffected, and pinned as such:** cross-owner revocation stays 403; a
+non-`revoked` column stays refused for the owner too; `create` / `delete` stay
+**405** at the method gate (minting remains `POST /api/v1/keys`, the only path
+that returns the raw secret once, and rows retire by revoking, not deleting);
+show-once semantics are intact. Every other better-auth-managed identity table
+stays write-denied — `sys_api_key` is the one exception, and it is one because
+that table is hand-rolled ObjectStack rather than better-auth-owned, with a
+registered whitelist already governing its single platform-owned column.
+
+The regression pin runs as the key's **owner**, not as an admin — the persona
+gap that let this survive #7727's own test suite.

packages/plugins/plugin-security/src/member-default-explicit-allow.test.ts

@@ -114,11 +114,35 @@ describe('[#5491] what the baseline still declares, it still enforces', () => {
  it('better-auth identity tables stay WRITE-DENIED (the door is better-auth, not CRUD)', () => {
  for (const object of BETTER_AUTH_MANAGED_OBJECTS) {
  expect(allows('insert', [MEMBER_DEFAULT], object), `${object} insert`).toBe(false);
- expect(allows('update', [MEMBER_DEFAULT], object), `${object} update`).toBe(false);
+ // [#8053] `sys_api_key` is the ONE update exception: a member revokes
+ // their own personal key. It is not a hole in the "door is better-auth"
+ // rule — that table is hand-rolled ObjectStack (better-auth's `apiKey`
+ // plugin is not loaded), and the write is narrowed to the owner's rows by
+ // the `sys_api_key_self` RLS policy and to `revoked` by ADR-0092 D2's
+ // column whitelist. Neither narrowing is expressible as a permission-set
+ // boolean, which is why this axis has to be asserted per object here.
+ expect(allows('update', [MEMBER_DEFAULT], object), `${object} update`).toBe(
+ object === 'sys_api_key',
+ );
  expect(allows('delete', [MEMBER_DEFAULT], object), `${object} delete`).toBe(false);
  }
  });
 
+ it('[#8053] the update exception is `sys_api_key` alone, and it does not leak onto the other axes', () => {
+ // Stated positively and separately so the loop above cannot be "fixed" by
+ // widening the condition: every other managed table must still refuse
+ // update, and `sys_api_key` itself must still refuse insert and delete.
+ const alsoUpdatable = BETTER_AUTH_MANAGED_OBJECTS.filter(
+ (o) => o !== 'sys_api_key' && allows('update', [MEMBER_DEFAULT], o),
+ );
+ expect(alsoUpdatable, 'no other managed identity table may become updatable').toEqual([]);
+
+ expect(allows('update', [MEMBER_DEFAULT], 'sys_api_key')).toBe(true);
+ expect(allows('insert', [MEMBER_DEFAULT], 'sys_api_key'), 'minting stays POST /keys').toBe(false);
+ expect(allows('delete', [MEMBER_DEFAULT], 'sys_api_key'), 'rows retire by revoking').toBe(false);
+ expect(allows('find', [MEMBER_DEFAULT], 'sys_api_key')).toBe(true);
+ });
+
  it('self-service preferences survive the wildcard removal as an EXPLICIT grant', () => {
  // `sys_user_preference` is not a better-auth table, so the managed-deny
  // block does not cover it, and its `sys_user_preference_self` RLS policy

packages/plugins/plugin-security/src/objects/default-permission-sets.test.ts

@@ -38,6 +38,19 @@ describe('BETTER_AUTH_MANAGED_OBJECTS ↔ schemas (drift pin, #3325)', () => {
  });
 });
 
+/**
+ * [#8053] The single, deliberate exception to the blanket managed-object edit
+ * deny: `member_default` may EDIT `sys_api_key`, so a member can revoke their
+ * own personal key. Bounded elsewhere and not by the permission-set boolean —
+ * the `sys_api_key_self` RLS carve-out decides which rows, ADR-0092 D2's column
+ * whitelist (`revoked` alone) decides which fields.
+ *
+ * Encoded as an exact (set, object) pair rather than by loosening the loop, so
+ * a second entry — or the same one on another set — still fails this pin. The
+ * create/delete/read axes are NOT excepted and are still asserted below.
+ */
+const EDIT_EXCEPTIONS = new Set(['member_default::sys_api_key']);
+
 describe('default permission sets carry the managed denies (static baseline)', () => {
  it('each write-granting target set denies create/edit/delete on every managed object', () => {
  for (const setName of MANAGED_DENY_TARGET_SETS) {
@@ -47,13 +60,34 @@ describe('default permission sets carry the managed denies (static baseline)', (
  const entry = set.objects[obj];
  expect(entry, `${setName} has entry for ${obj}`).toBeTruthy();
  expect(entry.allowCreate).toBe(false);
- expect(entry.allowEdit).toBe(false);
+ expect(entry.allowEdit, `${setName}.${obj} allowEdit`).toBe(
+ EDIT_EXCEPTIONS.has(`${setName}::${obj}`),
+ );
  expect(entry.allowDelete).toBe(false);
  expect(entry.allowRead).toBe(true);
  }
  }
  });
 
+ it('the edit exception is exactly one (set, object) pair, and it is the API-key one', () => {
+ // The exception list is itself pinned: a future widening has to edit THIS
+ // assertion, which is the moment someone is asked whether the new pair
+ // really rides an owner-scoping RLS policy and a column whitelist the way
+ // `sys_api_key` does. Without this, `EDIT_EXCEPTIONS` could grow silently.
+ expect([...EDIT_EXCEPTIONS]).toEqual(['member_default::sys_api_key']);
+
+ const member = setByName('member_default');
+ expect(member.objects.sys_api_key.allowEdit).toBe(true);
+ // The owner scoping the grant leans on must exist, or the edit bit is
+ // table-wide on a credential table.
+ const selfPolicy = (member.rowLevelSecurity ?? []).find(
+ (p: any) => p.object === 'sys_api_key' && p.name === 'sys_api_key_self',
+ );
+ expect(selfPolicy, 'member_default must keep the sys_api_key_self RLS carve-out').toBeTruthy();
+ expect(selfPolicy.using).toBe('user_id == current_user.id');
+ expect(['all', 'update']).toContain(selfPolicy.operation);
+ });
+
  it('admin_full_access keeps its bare wildcard (zero per-object entries) — admin rescue path', () => {
  const admin = setByName('admin_full_access');
  expect(admin).toBeTruthy();

packages/plugins/plugin-security/src/objects/default-permission-sets.ts

@@ -354,6 +354,52 @@ const baseDefaultPermissionSets: PermissionSet[] = [
  // but the grant itself: it is what keeps `/auth/me`, the org switcher and
  // the Account app working for a member with no application profile.
  ...denyWritesOnManagedObjects(),
+ // [#8053] The ONE override of the block just above: a member may revoke
+ // their OWN API key. #7727 opened the method gate and registered the
+ // ADR-0092 D2 column whitelist, but left this object-CRUD layer
+ // untouched, so `update` on `sys_api_key` resolved for
+ // `admin_full_access` alone — the `revoke_api_key` row action rendered in
+ // the member's own My Keys grid and answered 403 for them. A personal key
+ // "acts as you — treat it like a password", and the owner is the person
+ // who discovers it leaked; their only remedy was to find an admin.
+ //
+ // This is a restoration of declared-≠-enforced intent, not a new grant:
+ // the row action, the checklist persona and the `sys_api_key_self` policy
+ // below (which already makes the row owner-VISIBLE) all say the owner
+ // path was intended — there was simply no `allowEdit` to go with it.
+ //
+ // The opening is bounded by TWO pre-existing mechanisms, and it is
+ // deliberately not bounded by this line alone:
+ // - WHICH ROWS: the `sys_api_key_self` RLS carve-out below
+ // (`user_id == current_user.id`, `operation: 'all'`), enforced on
+ // by-id writes through the security middleware's pre-image check. A
+ // member PATCHing another user's key is still refused.
+ // - WHICH FIELDS: ADR-0092 D2's identity write guard, whose per-object
+ // update whitelist for this table lists `revoked` alone
+ // (plugin-auth `MANAGED_EXTENSION_EDITABLE_FIELDS`). `key` stays
+ // unwritable (a rotated hash mints a credential nobody holds) and
+ // `user_id` stays unwritable (re-owning a key is privilege transfer).
+ //
+ // `allowCreate` / `allowDelete` stay false and are NOT an oversight:
+ // minting is `POST /api/v1/keys` (the only path that returns the raw
+ // secret once) and rows are retired by revoking, not deleting, so history
+ // survives. `allowDelete` also stays false because this set is bound to
+ // the `everyone` anchor and must remain anchor-safe (ADR-0090 D5).
+ //
+ // ⚠️ Not a pattern to copy across the managed list. Every OTHER
+ // better-auth table here stays write-denied because its mutations must
+ // flow through an auth endpoint; this one is a hand-rolled ObjectStack
+ // table (`packages/core/src/security/api-key.ts` mints and verifies it,
+ // better-auth's `apiKey` plugin is not loaded) whose one platform-owned
+ // column already has a registered whitelist. Widening `update` on
+ // `sys_api_key` beyond owner-scoped-plus-one-column would close #8053 and
+ // open a worse defect on a table whose rows act as the user.
+ //
+ // Being an EXPLICIT entry is what makes it survive `kernel:ready`:
+ // `applyManagedWriteDenies` injects its deny only for managed objects a
+ // target set does not already name (`name in objects` → skip), so this
+ // line is preserved rather than overwritten.
+ sys_api_key: { allowRead: true, allowCreate: false, allowEdit: true, allowDelete: false },
  // Self-service preferences. NOT a better-auth table, so it is not covered
  // by the block above, and its `sys_user_preference_self` RLS policy below
  // (`operation: 'all'`) declares exactly this intent: a member reads and