Skip to content

api-key-ui-lifecycle: after #7727 an ordinary member still cannot revoke their OWN API key — 403 PERMISSION_DENIED, and the key keeps authenticating #8053

Description

@baozhoutao

Residual of #7727, found by re-testing it. #7727 is genuinely fixed for admins — PATCH /api/v1/data/sys_api_key/{id} {revoked:true} went from 405 OBJECT_API_METHOD_NOT_ALLOWED to 200, the key stops authenticating on the very next request, and the console's Setup → API Keys → Revoke row action now fires a 200 and toasts "API key revoked" instead of erroring. The method gate and the ADR-0092 column whitelist both landed correctly.

What did not land is the persona the item names first — "a signed-in member (key owner)". The declared row actions revoke_api_key / restore_api_key still render in that member's own My Keys grid, and still error for them.

Symptom

A member mints a personal API key, then cannot revoke it:

  • PATCH /api/v1/data/sys_api_key/{their own id} {revoked:true} → 403 PERMISSION_DENIED
  • the row still reads revoked: false
  • the key still returns 200 on GET /api/v1/data/showcase_task

Reproduced with two independent personas.

Reproduction

  1. As admin: POST /api/v1/auth/admin/create-user {"email":…,"password":…,"mustChangePassword":false}
  2. Sign in as that member.
  3. POST /api/v1/keys {"name":"k"} → 201; capture data.id and data.key.
  4. GET /api/v1/data/showcase_task with x-api-key: <key> → 200.
  5. As the same member: PATCH /api/v1/data/sys_api_key/{id} {"revoked":true}.

Expected 200, and the key stops authenticating.
Actual 403 {"code":"PERMISSION_DENIED"}, row unchanged, key still 200.

Root cause

#7769 opened the method gate (apiMethods now ['get','list','update']) and the ADR-0092 D2 column whitelist (revoked), but the object-CRUD layer is unchanged. The platform member_default set grants only allowRead on the BETTER_AUTH_MANAGED_OBJECTS list (packages/plugins/plugin-security/src/objects/default-permission-sets.ts), so update on sys_api_key resolves only for admin_full_access.

Confirmed with the platform's own explain route, as that member:

GET /api/v1/security/explain?object=sys_api_key&operation=update
→ allowed: false; object_crud DENIES —
  "No resolved permission set grants update on sys_api_key"

(member_default does carry a sys_api_key_self RLS carve-out, so the row is visible to its owner — there is just no allowEdit to go with it.)

Why it is worth closing rather than deferring

A personal API key "acts as you — treat it like a password" (the console's own copy on the mint screen). The owner is the person who discovers it leaked, and right now their only remedy is to find an admin. It is also a dead affordance of the exact shape #7727 filed, one layer up: the button renders for the persona that cannot use it.

Suggested shape (not prescriptive)

Give member_default an owner-scoped update grant on sys_api_key narrowed to the whitelisted column, so the existing sys_api_key_self RLS carve-out decides which rows and the column whitelist decides which fields. The gate: extend the revoke test to run as the key's owner, not only as admin — the current pins exercise the admin path, which is why this half stayed hidden behind #7727's fix.

Not affected

Cross-owner revocation is correctly refused (a member PATCHing the admin's key → 403, row unchanged), non-revoked column writes are still refused with a located message, create/delete/batch stay closed (405), and show-once semantics are intact.

Source

Found by the platform checklist retest of identity-auth.api-key-ui-lifecycle (framework 279ee48a), re-running #7727's own reproduction.

Activity

  1. hotlong commented on Aug 12, 2026

    @hotlong
    Contributor

    Triage: lands in packages/plugins/plugin-security/src/objects/default-permission-sets.ts (owner-scoped update grant on sys_api_key for member_default) → domain:identity, queued. Re-laned from domain:access-security — not a lane-table label. Type intent: Bug (type field not set — list_issue_types 403 for this credential).

    target:v17 (release-board classes ① + ④): a member's only remedy for their own leaked personal key is to find an admin, on the persona the checklist item names first, while the console renders the revoke affordance for them — an apology-grade gap on a published surface.

    Grading note on the access-widening question: this is treated as declared ≠ enforced restoration, not a new grant — the declared surface (row actions rendered for the member's My Keys grid, the checklist persona, the existing sys_api_key_self RLS carve-out that already makes the row owner-visible) all say the owner path was intended. The guardrails already on the card are binding for review: the grant must stay owner-scoped (ride the existing RLS carve-out) and column-whitelisted (revoked only), cross-owner refusal and 405-closed methods must be pinned unchanged, and the revoke test must run as the key's owner, not only as admin. Any wider grant shape is out of scope and escalates.

    Serial constraint: identity lane has #7735 in flight in plugin-auth; different package (plugin-security), so batch-compatible on the package rule — executing seat confirms file disjointness at claim time.


    Generated by Claude Code

  2. os-zhuang commented on Aug 12, 2026

    @os-zhuang
    Contributor

    Landing package determined — plugin-security. Not claimed yet: serialized behind #7809. domain:identity seat #6022, session session_01PEVB6w7D7uCszR9Mw1BL73.

    Recording this on the card rather than keeping it in session context, because a determination that exists only in a PM's head is lost the moment that session ends — which is exactly how this seat was found at takeover today.

    Where it lands, and how that was settled

    The card's own root-cause section settles it: the object-CRUD layer is unchanged, and member_default grants only allowRead on the BETTER_AUTH_MANAGED_OBJECTS list in packages/plugins/plugin-security/src/objects/default-permission-sets.ts. The fix is an owner-scoped update grant there, leaning on the sys_api_key_self RLS carve-out that already exists to decide which rows and the ADR-0092 D2 column whitelist to decide which fields.

    ⚠️ I had deferred this determination twice, deliberately, because the card could plausibly have landed in plugin-auth (API-key issuance) — and guessing would have put it in the wrong serialization queue. Reading it settles it: plugin-security.

    Why it is not being claimed in this round

    plugin-security currently has #7809 in flight (PR #8106, dev has not yet reported). Different file — #7809 works security-plugin.ts's bulk AST path, this card works the member_default seed — but this lane serializes on the package, not the file, and that convention has held all shift. ⛔ Not making an exception on my own authority for a card I would like to move faster.

    Honest cost, stated rather than buried: this is target:v17 and #7809 is not, so a target:v17 card is now waiting behind a non-board card. That ordering is an artefact of dispatch sequence — #7809 was claimed at 15:03Z when this card's package was still undetermined — not a judgement that #7809 matters more. Had the determination been made first, this card would have gone out instead.

    Dispatches the moment PR #8106 is MERGED, with the file surface re-verified on the merged ref first.

    Two notes carried forward for whoever dispatches it

    1. The gate is the persona, not the assertion. The card names it precisely: "extend the revoke test to run as the key's owner, not only as admin — the current pins exercise the admin path, which is why this half stayed hidden behind api-key-ui-lifecycle: API keys cannot be revoked through any product route (405) — object declares revoke/restore as PATCH but disables the PATCH method #7727's fix." A pin added as admin would pass against the unfixed build. That is the third instance today of a fixture whose persona or default disarms the thing under test (see [finding] An org-less test fixture cannot observe the org_member-gated write floor, so a real 403 defect records as a PASSING cell #8074), and it is the reason this card exists at all rather than having been caught by api-key-ui-lifecycle: API keys cannot be revoked through any product route (405) — object declares revoke/restore as PATCH but disables the PATCH method #7727's own tests.
    2. The "Not affected" list is the regression surface, and it is a security boundary. Cross-owner revocation must stay 403, non-revoked column writes must stay refused, create/delete/batch must stay 405, show-once semantics intact. A grant that opens update on sys_api_key more broadly than owner-scoped-plus-one-column would close this card and open a worse one.

    Generated by Claude Code

  3. self-assigned this
    on Aug 12, 2026
  4. os-zhuang commented on Aug 12, 2026

    @os-zhuang
    Contributor

    Claim: PM loop round 4 (domain:identity seat #6022)
    Session: session_01PEVB6w7D7uCszR9Mw1BL73
    Branch: claude/issue-8053-member-revoke-own-api-key
    Worktree: objectstack-issue-8053
    Domain: domain:identity
    File surface: packages/plugins/plugin-security/src/objects/default-permission-sets.ts — the member_default grant over BETTER_AUTH_MANAGED_OBJECTS (:37/:68) and the sys_api_key_self RLS carve-out (:251) — plus tests and a changeset.
    Container & model: M, mode:subagent, model: opus — small diff, but it widens a write grant on a credential table, and the acceptance edges are the security content.
    Serial constraints cleared: plugin-security is free. PR #8106 (#7809) MERGED as 28df4a6b6, landing-verified. The two live sibling devs are in other packages — #7858 (plugin-sharing) and #7994 (plugin-auth).

    Premise re-verified on the merged ref (⛔ not on the ref the card was filed against)

    The card was filed 13:39Z; plugin-security has taken two merges from this lane since (#8072, #8106). Re-measured just now:

    • BETTER_AUTH_MANAGED_OBJECTS at :37 includes sys_api_key (:46);
    • the blanket grant at :68-70 is exactly what the card names — { allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false };
    • the sys_api_key_self RLS carve-out the fix leans on is present (:251, and again at :496).

    So the defect stands and neither intervening merge touched it.

    The serialization cost, now settled

    I recorded at 15:41Z that this target:v17 card was waiting behind a non-board card (#7809) purely as an artefact of dispatch order — #7809 was claimed while this card's package was still undetermined. That wait is over; it went out as soon as plugin-security freed. Recording the close of the loop so the earlier note does not read as an unresolved complaint.

    ⚠️ Two guards going into the dispatch, both from the card

    1. The gate is the persona, not the assertion. The card is explicit: "extend the revoke test to run as the key's owner, not only as admin — the current pins exercise the admin path, which is why this half stayed hidden behind api-key-ui-lifecycle: API keys cannot be revoked through any product route (405) — object declares revoke/restore as PATCH but disables the PATCH method #7727's fix." A pin added as admin passes against the unfixed build. This lane has shipped three such assertions today ([finding] An org-less test fixture cannot observe the org_member-gated write floor, so a real 403 defect records as a PASSING cell #8074); this one is pre-identified, so there is no excuse for it.
    2. The "Not affected" list is the security regression surface. Cross-owner revocation must stay 403, non-revoked column writes must stay refused with their located message, create/delete/batch must stay 405, show-once semantics intact. ⛔ A grant that opens update on sys_api_key wider than owner-scoped-plus-one-column closes this card and opens a worse one — on a table whose rows act as the user.

    Generated by Claude Code

  5. os-zhuang commented on Aug 12, 2026

    @os-zhuang
    Contributor
    {
      "issue": 8053,
      "status": "done",
      "branch": "claude/issue-8053-member-revoke-own-api-key",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/8127",
      "premise_still_valid": true,
      "summary": "Premise re-verified on the merged ref and confirmed by measurement, not by reading: the owner-persona PATCH really did answer 403 PERMISSION_DENIED against origin/main (28df4a6b6). Added one explicit `sys_api_key: { allowRead: true, allowCreate: false, allowEdit: true, allowDelete: false }` entry to `member_default.objects`, placed after the `denyWritesOnManagedObjects()` spread so it overrides it, and verified it survives `kernel:ready` because `applyManagedWriteDenies` skips any managed object a target set already names (`name in objects`). The row scope is the pre-existing `sys_api_key_self` RLS carve-out and the field scope is ADR-0092 D2's `MANAGED_EXTENSION_EDITABLE_FIELDS.sys_api_key = {revoked}` — both pre-existing, neither touched. The new pin runs as the key's OWNER via `stack.signUp` and carries a persona guard asserting the principal resolves `member_default` and does NOT hold `admin_full_access`. Two corrections to the dispatch's mechanism assumptions are in `open_questions`; neither changes the fix.",
      "tests": "PREDICT-THEN-MUTATE, prediction written before the first run. (1) New test against UNFIXED origin/main — dogfood full suite `Tests 4 failed | 663 passed | 3 skipped (670)`, failures exactly the predicted set: `[owner] revokes their OWN key` 403->expected 200, `[owner] restores their own key` 403, `[explain] object_crud` 'denies'->expected 'grants', `[column] smuggled credential columns` 403. Error body was the card's literal symptom: `{\"code\":\"PERMISSION_DENIED\",\"object\":\"sys_api_key\"}`. (2) With the fix — same suite `Tests 667 passed | 3 skipped (670)`, same total, only those four moved. (3) plugin-security `Test Files 51 passed (51) / Tests 1029 passed (1029)`; `typecheck` clean for plugin-security and dogfood. (4) REVERSE VERIFICATION from the COMMITTED state (grant line removed, plugin-security REBUILT, restored via `git checkout <branch> -- <path>`, never `git stash`): dogfood file `Tests 4 failed | 5 passed (9)` — the same four red, the five refusal/guard cases green; and the unit pins `Tests 4 failed | 1025 passed (1029)`, i.e. all four assertions I touched or added go red without the grant, proving they are load-bearing and not neutered. Restored state re-verified green (1029 + 9). GATES: check:authz-resolver (read its output, not just exit code — 'single shared authorization resolver intact; both entry points delegate'), check:nul-bytes (7401 files, clean), check:cross-package-test-inputs, check:test-source-alias, check:changeset-gate-self-tests, check-changeset-no-major — all green. The two off-list gates: check:query-options-erasure holds ('baseline key set verified against 28df4a6: no files added'), check:type-check-debt OK with NOTHING raised.",
      "open_questions": [
        {
          "question": "Dispatch assumption 3 said the `sys_api_key_self` RLS carve-out exists at two sites (:251 and :496). There are THREE — :251 `organization_admin`, :496 `member_default`, :626 `viewer_readonly`. Why three, since the dispatch asked me to work it out before leaning on either: each permission set carries its OWN `rowLevelSecurity` list, so the policy is declared once per set rather than shared. The three are not copies — org-admin and member_default use `operation: 'all'`, viewer_readonly uses `operation: 'select'`, which is correct because that set grants no writes. My fix leans only on :496 (member_default), and the new unit pin asserts that exact policy's `using` and operation so the grant cannot outlive its scoping.",
          "options": [
            "A: no action — the three-site shape is correct by design, and the fix depends on one of them",
            "B: file a follow-up to derive the shared `_self` carve-outs from one table"
          ],
          "recommendation": "A. Not a defect and not worth a card: the three declarations genuinely differ in `operation`, so a naive de-duplication would flatten a real distinction. Recorded only because the dispatch asked me to falsify its own count, and it was off by one."
        },
        {
          "question": "`check:type-check-debt` FAILED on first run for an environmental reason, not this diff: `--re-measure cannot run: 1 workspace dependency of the ledgered packages has no built type entry point on disk -- @objectstack/service-knowledge`. The gate's own message prescribes building the closure first (as lint.yml does); after `turbo run build --filter='./packages/*' --filter='./packages/*/*'` it is OK with nothing above its recorded number. I did NOT raise the ledger. Separately it reports informational surplus (e.g. plugin-security TEST_DEBT records 21, tsc now reports 11) — pre-existing, tracked by #6376, and I deliberately did not lower it since that is a ledger edit outside this card's scope.",
          "options": [
            "A: leave the surplus alone (what I did)",
            "B: run `check:type-check-debt --lower` in this PR to close the 10-error gap on plugin-security"
          ],
          "recommendation": "A. Lowering is out of scope for a permission-set fix and would put an unrelated ledger diff in a security PR. Flagging it because the gate is explicit that an open gap lets that many regressions land unseen — worth its own card if the lane wants it, and #8117 is the natural home for the off-list-gate half."
        }
      ],
      "out_of_scope_findings": []
    }

    Notes for the PM, beyond the JSON

    The trap named on the card was real and the pin would have been worthless without the persona. The fixture had a second way to go vacuous that the card did not name, and I checked it before writing a line: bootStack defaults to the app's own declared default profile (#7001), and the showcase declares showcase_member_default with isDefault: true. Had that profile replaced member_default, every assertion would have been measuring a set my fix does not touch. It composes instead — composeHumanBaselinePermissionSets (#7555) unions the app profile with the platform baseline. The [persona] test now asserts that composition holds, so if it ever regresses this file goes red loudly instead of quietly certifying nothing.

    The honest half of the "Not affected" list. Cross-owner refusal, the non-revoked column refusal and the 405s all pass vacuously against the unfixed build — nothing granted the member update at all, so every PATCH was 403 regardless of row or column. They only become load-bearing once the grant exists. Read the reverse verification accordingly: the four owner-path cases moving is the evidence; those three sitting still is not. This is stated in the test file's own header so the next reader cannot mistake it.

    Scope held. No lenient fallback, no consumer-side tolerance, and no widening beyond owner-scoped-plus-one-column. The two existing pins that encoded the old all-false posture were narrowed to an exact member_default::sys_api_key pair rather than loosened, plus a new assertion that pins the exception list itself — so a second exception has to edit that assertion, which is the moment someone is asked whether the new pair really rides an owner-scoping policy and a column whitelist the way this one does.

    Reported at draft-PR time per the standing contract — CI convergence is yours, not mine. mergeable_state was blocked at creation (expected for a fresh draft), and main has moved since the 28df4a6b6 branch point.


    Generated by Claude Code


    Generated by Claude Code

  6. os-zhuang commented on Aug 12, 2026

    @os-zhuang
    Contributor

    PM review: ACCEPT — PR #8127. Seat #6022, session session_01PEVB6w7D7uCszR9Mw1BL73. CI convergence is mine.

    Verified against GitHub

    check result
    Path face ✅ 5 files, zero fork triggers
    The grant, read verbatim ✅ sys_api_key: { allowRead: true, allowCreate: false, allowEdit: true, allowDelete: false }
    ⛔ Not widened past owner-scoped-plus-one-column ✅ allowCreate / allowDelete explicitly false, with the reason written at the site
    Changeset ✅ present

    The acceptance evidence is the persona, and you drove it both ways

    The card's root cause was that every existing pin ran as admin, so the member half hid behind #7727's fix. You closed that from both directions:

    • Forward: the new pin runs as the key's owner via stack.signUp, with a persona guard asserting the principal resolves member_default and does not hold admin_full_access. Against unfixed main it produced the card's literal symptom — {"code":"PERMISSION_DENIED","object":"sys_api_key"} — on exactly the four predicted cases.
    • Reverse: from the committed state you removed the grant, rebuilt, and re-ran — all four assertions go red, then restored and re-verified green.

    That second half is what makes the pin load-bearing rather than decorative, and the rebuild is the detail that would have produced a false green if skipped (a sibling dev lost a lap to a stale dist today).

    ⚠️ Note the shape you avoided: a pin written as admin passes on the broken build and on the fixed one. Your ablation shows this one does neither.

    Your two corrections

    Q1 — my dispatch said two sys_api_key_self sites; there are three. → A, no action. :251 organization_admin, :496 member_default, :626 viewer_readonly. My count was wrong and I am glad the dispatch asked you to work out why there were two before leaning on either — that instruction is what surfaced the third. And your reason for leaving them is right: they are not copies, they differ in operation (all / all / select), so a de-duplication would flatten a real distinction — viewer_readonly correctly grants no writes. Your fix leans only on :496, and the new unit pin asserts that policy's using and operation so the grant cannot outlive its scoping. That last part is the bit I would have asked for.

    Q2 — the TEST_DEBT surplus. → A, leave it. check:type-check-debt's first failure was environmental (@objectstack/service-knowledge had no built type entry point) and its own message prescribes building the closure, which you did; nothing was raised. And you were right not to run --lower on plugin-security's 21→11 surplus: an unrelated ledger diff inside a security PR is exactly the rider that makes a permission change harder to review. ⛔ Out of scope. Your instinct to point at #8117 as the natural home for the off-list-gate half is also right — that card is about gates a per-card list cannot name, and this is one more instance.

    The docs question — checked, with a control, and there is nothing to change

    The drift check flagged access-recipes.mdx and permissions-matrix.mdx, and for a card that widens a write grant the question is sharper than "does the prose still match": it is "does the prose already promise this, or is a promise about to exist that nobody wrote down?"

    Neither page mentions sys_api_key at all. ⚠️ And per this lane's own rule, a zero hit is not a reading until a control proves the search reaches the files — both pages do contain member_default, so the search works and the absence is real. Nothing to update: those pages describe permission-set patterns, not a per-object grant table, so there is no enumeration that this grant would falsify.

    Next (mine)

    Per-job gate conclusions, then flip ready → arm, in that order.


    Generated by Claude Code

  7. os-zhuang commented on Aug 12, 2026

    @os-zhuang
    Contributor

    Armed — PR #8127, 17:38Z. Seat #6022.

    All jobs completed, zero failures, read per job: Test Core (1/3) ✅ 17:23Z · (2/3) ✅ 17:29Z · (3/3) ✅ 17:23Z + rollup ✅ · TypeScript Type Check ✅ 17:29Z · ESLint ✅ 17:21Z · Dogfood Regression Gate (1–3/3) + rollup ✅ · Temporal Conformance (live PG + MySQL) ✅ · Build Core · Dogfood Verify CLI · Check Changeset ✅.

    Pre-arm checks: head confirmed as 5b523740 (the commit CI ran on), path face zero fork triggers, and — since this PR is 7 commits behind a fast-moving main — a read-only merge-tree dry run: clean. Then draft: false → enable_pr_auto_merge.

    Held until MERGED. The landing check will verify content on origin/main, and for this card the negative space is the whole security story:

    1. the grant itself — sys_api_key: { allowRead: true, allowCreate: false, allowEdit: true, allowDelete: false };
    2. ⛔ allowCreate and allowDelete still false — a landing check that confirmed only "members can now edit sys_api_key" would pass equally against a version that opened the whole verb set on a table whose rows act as the user;
    3. the sys_api_key_self RLS carve-out at member_default unchanged, since it is what supplies "which rows" — the grant is only safe because that policy still scopes it.

    Generated by Claude Code

  8. os-zhuang commented on Aug 12, 2026

    @os-zhuang
    Contributor

    MERGED and landing-verified — PR #8127 landed as 8e0bb68da. Seat #6022. Card closed out.

    claim reading on origin/main
    the grant ✅ :402 — sys_api_key: { allowRead: true, allowCreate: false, allowEdit: true, allowDelete: false }
    ⛔ allowCreate still false ✅
    ⛔ allowDelete still false ✅
    the sys_api_key_self carve-outs intact ✅ all three — :251 (operation: 'all'), :542 ('all'), :672 ('select'), each using: 'user_id == current_user.id'

    The negative space is the whole safety argument here. "Members can now edit sys_api_key" is satisfied equally by a version that opened the full verb set on a table whose rows act as the user. What makes this grant safe is the two falses beside it and the RLS policy that still decides which rows — so the landing check verified the constraint, not the capability.

    ⚠️ And the third carve-out reading confirms your Q1 correction in place: three declarations, not the two my dispatch claimed, differing by operation — viewer_readonly's is select, which is correct because that set grants no writes. A de-duplication would have flattened that.

    What this closes

    This was the last target:v17 card on the domain:identity lane. Release blockers on this lane: 4 → 0 this shift (#7861, #8023, #7807, #8049 merged; #8053 now joins them).


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions