From 5b523740ac8a6a31ec3d34cac4e692eb941f0f8e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 17:15:06 +0000 Subject: [PATCH] fix(plugin-security): let a member revoke their OWN API key (#8053) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The residual of #7727, one layer down. That fix opened the method gate and registered the ADR-0092 D2 column whitelist, but the object-CRUD layer was untouched: `member_default` granted only `allowRead` across the better-auth managed identity tables, so `update` on `sys_api_key` resolved for `admin_full_access` alone. A member could mint a personal key and then not revoke it — 403 PERMISSION_DENIED, row unchanged, key still authenticating — while the `revoke_api_key` row action rendered in their own My Keys grid. `member_default` now carries an explicit `sys_api_key` entry with `allowEdit`. The grant is bounded by two pre-existing mechanisms rather than by the permission-set boolean: the `sys_api_key_self` RLS carve-out decides which rows (cross-owner revocation still 403), and ADR-0092 D2's column whitelist decides which fields (`revoked` alone; `key` and `user_id` stripped even when smuggled alongside a legal `revoked`). `create`/`delete` stay closed at 405. 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. Verified by removing the grant and re-running: the four owner-path cases go red, the refusal cases stay green. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73 --- .changeset/member-revoke-own-api-key.md | 52 ++++ .../src/member-default-explicit-allow.test.ts | 26 +- .../objects/default-permission-sets.test.ts | 36 ++- .../src/objects/default-permission-sets.ts | 46 +++ .../test/api-key-owner-revoke.dogfood.test.ts | 287 ++++++++++++++++++ 5 files changed, 445 insertions(+), 2 deletions(-) create mode 100644 .changeset/member-revoke-own-api-key.md create mode 100644 packages/qa/dogfood/test/api-key-owner-revoke.dogfood.test.ts diff --git a/.changeset/member-revoke-own-api-key.md b/.changeset/member-revoke-own-api-key.md new file mode 100644 index 0000000000..96d61bb4e3 --- /dev/null +++ b/.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. diff --git a/packages/plugins/plugin-security/src/member-default-explicit-allow.test.ts b/packages/plugins/plugin-security/src/member-default-explicit-allow.test.ts index 199ae32850..fbc65dab95 100644 --- a/packages/plugins/plugin-security/src/member-default-explicit-allow.test.ts +++ b/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 diff --git a/packages/plugins/plugin-security/src/objects/default-permission-sets.test.ts b/packages/plugins/plugin-security/src/objects/default-permission-sets.test.ts index ead92fd4c7..abdb5124f6 100644 --- a/packages/plugins/plugin-security/src/objects/default-permission-sets.test.ts +++ b/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(); diff --git a/packages/plugins/plugin-security/src/objects/default-permission-sets.ts b/packages/plugins/plugin-security/src/objects/default-permission-sets.ts index 1c9dfdb329..d296c973e7 100644 --- a/packages/plugins/plugin-security/src/objects/default-permission-sets.ts +++ b/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 diff --git a/packages/qa/dogfood/test/api-key-owner-revoke.dogfood.test.ts b/packages/qa/dogfood/test/api-key-owner-revoke.dogfood.test.ts new file mode 100644 index 0000000000..689d6758ec --- /dev/null +++ b/packages/qa/dogfood/test/api-key-owner-revoke.dogfood.test.ts @@ -0,0 +1,287 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #8053 — an ordinary member revoking their OWN API key. + * + * The residual of #7727. That fix opened the METHOD gate (`apiMethods` gained + * `update`) and registered the ADR-0092 D2 column whitelist (`revoked` alone), + * and `api-key-revoke-lifecycle.dogfood.test.ts` pins both — but every + * assertion in that file drives the seeded ADMIN. One layer down, in + * object-CRUD, the platform `member_default` set granted only `allowRead` on + * the `BETTER_AUTH_MANAGED_OBJECTS` list, so `update` on `sys_api_key` + * resolved for `admin_full_access` and nobody else. A member could mint a + * personal key, watch it authenticate, and then not revoke it: 403 + * PERMISSION_DENIED, row unchanged, key still live. Their only remedy for a + * leaked credential that "acts as you" was to find an admin. + * + * ## Why this file exists SEPARATELY from the admin one + * + * The persona IS the gate, and it is the whole reason the defect survived its + * own fix's test suite. A revoke assertion written as admin passes against the + * unfixed build, passes against the fixed build, and certifies nothing about + * the persona the checklist item names first. So this file signs up a plain + * member (`stack.signUp` — the first user is the seeded admin, so a fresh + * sign-up carries no roles or grants) and drives every case as them. + * + * `[persona]` below is not ceremony: it asserts the fixture's principal really + * is an ordinary member resolving the platform baseline. Two ways this file + * could go quietly vacuous, both live: + * + * - the principal turns out to be privileged, so every case passes on the + * admin path this file exists to avoid; + * - `member_default` stops resolving for a showcase member at all, so the + * grant under test is not in force and the file proves nothing about it. + * The showcase declares its own `isDefault` profile + * (`showcase_member_default`), and #7555's `composeHumanBaselinePermissionSets` + * is the only reason that profile COMPOSES with the platform baseline + * instead of replacing it. If that composition regresses, this file must go + * red loudly rather than keep reporting green about a set nobody resolved. + * + * ## What is deliberately NOT widened + * + * The grant is owner-scoped-plus-one-column, and `sys_api_key` rows act as the + * user — the console's mint screen says to treat one like a password. So the + * refusals below are as load-bearing as the success: + * + * - `[cross-owner]` — a member may not revoke the ADMIN's key. The row scope + * is the pre-existing `sys_api_key_self` RLS carve-out, not the grant. + * - `[column]` — a non-`revoked` column stays refused, as the owner. + * - `[method]` — `create` / `delete` stay 405 at the method gate. + * + * ⚠️ Read those three honestly: on the UNFIXED build they pass **vacuously**, + * because nothing granted the member `update` at all and so every PATCH was + * 403 regardless of row or column. They only become load-bearing once the + * grant exists, which is precisely when they are the assertions that prove it + * is narrow. A reverse verification of this file must therefore expect the + * OWNER cases to move and these to sit still — their stillness is not evidence. + * + * Refusal cases assert `code` AND `status` (ADR-0112): a bare + * `rejects.toThrow()` / status-only assertion stays green against an + * implementation that throws a naked `Error`. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import showcaseStack from '@objectstack/example-showcase'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; + +describe('#8053: a member revokes their OWN sys_api_key', () => { + let stack: VerifyStack; + /** The seeded platform admin — used ONLY to mint the cross-owner key. */ + let adminToken: string; + /** The persona under test: an ordinary member, no roles, no grants. */ + let memberToken: string; + + const MEMBER_EMAIL = 'apikey.owner.8053@verify.test'; + + /** Mint a key through the ONE mint path, as whoever holds `token`. */ + const mintKey = async (token: string, name: string): Promise<{ id: string; raw: string }> => { + const res = await stack.apiAs(token, 'POST', '/keys', { name }); + expect(res.status, await res.clone().text()).toBe(201); + const body: any = await res.json(); + expect(body?.data?.id).toBeTruthy(); + expect(body?.data?.key).toBeTruthy(); + return { id: String(body.data.id), raw: String(body.data.key) }; + }; + + /** + * Does this key still authenticate? Asked through a real read with NO bearer + * token, so the key is the only credential present. `showcase_task` is the + * card's own probe and is readable by a member via `showcase_member_default` + * — deliberately NOT `sys_api_key`, so a permission change on the object + * under test can never be mistaken for a credential verdict. + */ + const keyStillAuthenticates = async (raw: string): Promise => { + const res = await stack.api('/data/showcase_task?$top=1', { headers: { 'x-api-key': raw } }); + if (res.status === 200) return true; + // 401 = credential rejected. Anything else (e.g. 403) would mean the key + // WAS accepted and something later refused — a different fact entirely. + expect(res.status, `revoked-key probe should be 401, got ${res.status}`).toBe(401); + return false; + }; + + /** Read a key row back with the caller's own credentials. */ + const readKey = async (token: string, id: string) => { + const res = await stack.apiAs(token, 'GET', `/data/sys_api_key/${id}`); + const body: any = res.status === 200 ? await res.json() : {}; + return { status: res.status, row: body.record ?? body.data ?? {} }; + }; + + beforeAll(async () => { + stack = await bootStack(showcaseStack, {}); + adminToken = await stack.signIn(); + memberToken = await stack.signUp(MEMBER_EMAIL); + }, 180_000); + + afterAll(async () => { await stack?.stop?.(); }); + + // ── the fixture's own preconditions ─────────────────────────────────────── + + it('[persona] the principal is an ordinary member resolving the platform baseline', async () => { + const res = await stack.apiAs(memberToken, 'GET', '/security/explain?object=sys_api_key&operation=read'); + expect(res.status, await res.clone().text()).toBe(200); + const body: any = await res.json(); + + const setNames: string[] = (body.layers ?? []) + .flatMap((l: any) => l.contributors ?? []) + .filter((c: any) => c?.kind === 'permission_set') + .map((c: any) => String(c.name)); + + // The grant under test lives in `member_default`. If a showcase member + // stops resolving it, every other assertion here would be measuring a set + // that is not in force — the failure mode this guard exists to make loud. + expect( + setNames, + 'a showcase member must resolve the PLATFORM baseline `member_default` — ' + + '#7555 composition (app `isDefault` profile ∪ platform baseline), not replacement', + ).toContain('member_default'); + + // …and must NOT be an admin, or this file silently re-tests the #7727 path. + expect( + setNames, + 'the fixture principal must not hold admin_full_access — that is the persona #7727 already covers', + ).not.toContain('admin_full_access'); + }); + + it('[persona] the member can SEE their own key row but that is a read grant, not a write one', async () => { + const { id } = await mintKey(memberToken, 'visible-to-me'); + const { status, row } = await readKey(memberToken, id); + // The `sys_api_key_self` RLS carve-out already made the row owner-visible; + // the card's point is that no `allowEdit` came with it. + expect(status).toBe(200); + expect(row.name).toBe('visible-to-me'); + }); + + // ── the defect ──────────────────────────────────────────────────────────── + + it('[owner] revokes their OWN key through the declared route, and it stops authenticating', async () => { + const { id, raw } = await mintKey(memberToken, 'my-leaked-key'); + + // Baseline: without this the post-revoke 401 is unfalsifiable — a key that + // never worked also "stops working". + expect(await keyStillAuthenticates(raw)).toBe(true); + + // The exact request the `revoke_api_key` row action declares, issued by the + // persona whose My Keys grid renders that action. + const revoked = await stack.apiAs(memberToken, 'PATCH', `/data/sys_api_key/${id}`, { revoked: true }); + expect(revoked.status, await revoked.clone().text()).toBe(200); + + // The consequence — a 200 that leaves the key live is the defect wearing a + // success code, and would pass a status-only check. + expect(await keyStillAuthenticates(raw)).toBe(false); + + const { row } = await readKey(memberToken, id); + expect(row.revoked).toBe(true); + }); + + it('[owner] restores their own key through the same route', async () => { + const { id, raw } = await mintKey(memberToken, 'my-restorable-key'); + + const off = await stack.apiAs(memberToken, 'PATCH', `/data/sys_api_key/${id}`, { revoked: true }); + expect(off.status).toBe(200); + expect(await keyStillAuthenticates(raw)).toBe(false); + + const on = await stack.apiAs(memberToken, 'PATCH', `/data/sys_api_key/${id}`, { revoked: false }); + expect(on.status).toBe(200); + expect(await keyStillAuthenticates(raw)).toBe(true); + }); + + it('[explain] the object_crud layer now grants update, and names `member_default`', async () => { + // The card's second measurement, and a second consumer of the same + // decision: `/security/explain` read as that member said "No resolved + // permission set grants update on sys_api_key". If the fix is right, this + // output has to move with it or the two consumers disagree. + const res = await stack.apiAs(memberToken, 'GET', '/security/explain?object=sys_api_key&operation=update'); + expect(res.status, await res.clone().text()).toBe(200); + const body: any = await res.json(); + + const crud = (body.layers ?? []).find((l: any) => l.layer === 'object_crud'); + expect(crud, 'explain must report an object_crud layer').toBeTruthy(); + expect(crud.verdict).toBe('grants'); + expect(String(crud.detail)).not.toContain('No resolved permission set grants'); + expect( + (crud.contributors ?? []).map((c: any) => String(c.name)), + 'the grant must be attributed to the platform baseline, not to an admin set', + ).toContain('member_default'); + }); + + // ── the "Not affected" list: still not affected ─────────────────────────── + + it('[cross-owner] a member may NOT revoke the admin\'s key — 403, row unchanged, key still live', async () => { + // The row scope is the pre-existing `sys_api_key_self` RLS carve-out. This + // is the assertion that proves the new grant is OWNER-scoped and not a + // table-wide `update` on a credential table. + const { id, raw } = await mintKey(adminToken, 'admins-key'); + expect(await keyStillAuthenticates(raw)).toBe(true); + + const res = await stack.apiAs(memberToken, 'PATCH', `/data/sys_api_key/${id}`, { revoked: true }); + expect(res.status).toBe(403); + const body: any = await res.json(); + expect(body.code ?? body.error?.code).toBe('PERMISSION_DENIED'); + + // Refused, not merely reported as refused: the admin's key is untouched. + const { row } = await readKey(adminToken, id); + expect(row.revoked).toBe(false); + expect(await keyStillAuthenticates(raw)).toBe(true); + }); + + it('[column] a non-`revoked` column is still refused for the owner — 403 PERMISSION_DENIED', async () => { + // The grant opens the ROW, the ADR-0092 D2 whitelist opens the COLUMN, and + // the two are independent. `name` is innocuous — it is refused because it + // is not whitelisted, not because it is dangerous. + const { id, raw } = await mintKey(memberToken, 'dont-rename-me'); + + const res = await stack.apiAs(memberToken, 'PATCH', `/data/sys_api_key/${id}`, { name: 'renamed' }); + expect(res.status).toBe(403); + const body: any = await res.json(); + expect(body.code ?? body.error?.code).toBe('PERMISSION_DENIED'); + + // Refused, not silently degraded into a timestamp touch. + const { row } = await readKey(memberToken, id); + expect(row.name).toBe('dont-rename-me'); + expect(await keyStillAuthenticates(raw)).toBe(true); + }); + + it('[column] credential columns are stripped even when smuggled alongside a legal `revoked`', async () => { + // The whitelist STRIPS non-listed keys rather than rejecting the payload, + // so a mixed patch is what decides whether the opening is column-scoped in + // practice — now on the OWNER path, where the whitelist had never run. + const { id, raw } = await mintKey(memberToken, 'smuggle-mine'); + const before = await readKey(memberToken, id); + const originalOwner = before.row.user_id; + expect(originalOwner).toBeTruthy(); + + const res = await stack.apiAs(memberToken, 'PATCH', `/data/sys_api_key/${id}`, { + revoked: true, + key: 'forged-hash-value', + user_id: 'usr_someone_else', + }); + expect(res.status).toBe(200); + + const after = await readKey(memberToken, id); + expect(after.row.user_id, 're-owning a key is privilege transfer').toBe(originalOwner); + expect(after.row.revoked).toBe(true); + + // The decisive proof that `key` was stripped: the ORIGINAL secret is still + // what the row hashes to. A forged value would make this key unrecognised + // rather than recognised-and-revoked — both answer 401, so assert it from + // the other side by restoring and re-authenticating. + const restored = await stack.apiAs(memberToken, 'PATCH', `/data/sys_api_key/${id}`, { revoked: false }); + expect(restored.status).toBe(200); + expect(await keyStillAuthenticates(raw)).toBe(true); + }); + + it('[method] create and delete stay closed for the member (405, method gate)', async () => { + // `update` was opened for the owner; `create` / `delete` were opened for + // nobody. Minting stays on `POST /api/v1/keys` and rows are retired by + // revoking, not deleting. + const { id } = await mintKey(memberToken, 'immortal-mine'); + + const created = await stack.apiAs(memberToken, 'POST', '/data/sys_api_key', { name: 'forged' }); + expect(created.status).toBe(405); + expect((await created.json()).code).toBe('OBJECT_API_METHOD_NOT_ALLOWED'); + + const deleted = await stack.apiAs(memberToken, 'DELETE', `/data/sys_api_key/${id}`); + expect(deleted.status).toBe(405); + expect((await deleted.json()).code).toBe('OBJECT_API_METHOD_NOT_ALLOWED'); + }); +});