Skip to content

managed-extension-fields D7 collision guard silently skips sys_api_key (no apikey entry in MODEL_TO_OBJECT) #7770

Closed

Description

@claude

Observation

managed-extension-fields.test.ts (the ADR-0105 D7 collision guard) derives better-auth's real field surface from getAuthTables() and fails on any overlap with MANAGED_EXTENSION_FIELDS. Its MODEL_TO_OBJECT map carries nine models — user, session, account, verification, organization, member, invitation, team, teamMember — and no apikey entry. Any object absent from that map is skipped outright:

const owned = byObject[object];
if (!owned) continue;

sys_api_key declares managedBy: 'better-auth', so it looks like it is covered. It is not — its extension fields get zero collision coverage.

Why this is dormant today, not a live defect

better-auth's apiKey plugin is not loaded (auth-manager.ts assembles bearer + the optional set; apiKey is not among them). sys_api_key is hand-rolled: packages/core/src/security/api-key.ts mints and verifies it, and POST /api/v1/keys writes the row. So better-auth owns none of these columns, there is nothing to collide with, and the guard skipping the object is currently the correct answer.

Adding apikey: 'sys_api_key' to MODEL_TO_OBJECT today would change nothing either, since getAuthTables() is called without that plugin and would not emit an apikey model.

The latent part

If better-auth's apiKey plugin is ever enabled, its own model carries name, prefix, key, userId, enabled, expiresAt, permissions, metadata and the refill/rate-limit columns — several of which are exactly the columns sys_api_key already defines and writes. That is the ownership-transfer hazard D7 exists to catch, and the gate that is supposed to catch it would stay silent, because the object is not in its map.

Worth noting the shape: D7's guard is not wrong, it is unreachable for this object. That is the same class as objectstack#7727 (a declared row action behind a method gate that answered 405 first) — a check that reads as coverage and evaluates to nothing.

Suggested direction (not decided here)

Whoever enables better-auth's apiKey plugin must, in the same change, add apikey: 'sys_api_key' to MODEL_TO_OBJECT, pass the plugin into that test's getAuthTables() call, and reconcile the overlapping columns — most likely by keeping the hand-rolled table and NOT adopting the plugin's model, or by renaming ours. A cheaper interim option is a comment in MODEL_TO_OBJECT recording why apikey is deliberately absent, so its absence reads as a decision rather than an oversight.

Filing unassigned and finding-labeled: nothing a user hits today, and the trigger is a plugin-enablement decision nobody has taken.

Source

Found while implementing #7727 (PR #7769), which registers sys_api_key → {revoked} in MANAGED_EXTENSION_FIELDS / MANAGED_EXTENSION_EDITABLE_FIELDS. revoked is not among better-auth's apikey columns, so that entry is safe either way — but it is the first entry this object has ever had in that registry, which is what surfaced the gap.


Generated by Claude Code

Activity

  1. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    Contributor

    Findings triage round, 2026-08-11 (PM session, maintainer-directed: 「跑一轮集中定级」).

    Graded: promote. A guard that silently skips an object it appears to cover is a gate-integrity gap (the declared managedBy: 'better-auth' surface reads as guarded and is not). Dormant today (apiKey plugin unloaded), but the fix is small — add the mapping or fail on unmapped managedBy: 'better-auth' objects — and restores the guard's own claim. Routing: better-auth managed-surface collision coverage ⇒ domain:identity.

    finding → pm:queue.


    Generated by Claude Code

  2. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    Contributor

    PM note from the #7727 dispatch (domain:metadata seat), where this was filed. The finding holds — I traced it in managed-extension-fields.test.ts rather than taking the report's word for it — and the fix is bigger than the card's framing suggests.

    The skip is two-stage, and both stages have to be fixed or the check goes green while verifying nothing:

    1. betterAuthFieldsByObject() never produces a sys_api_key key. It iterates getAuthTables(...) and drops any model missing from MODEL_TO_OBJECT (if (!object) continue). There is no apikey entry, so nothing is produced.
    2. The collision test then skips the object outright — const owned = byObject[object]; if (!owned) continue;. So 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 new sys_api_key: new Set(['revoked']) entry gets zero collision coverage, exactly as reported.

    The part worth adding, because it makes the obvious fix a trap. Suppose someone closes this by adding apikey: 'sys_api_key' to MODEL_TO_OBJECT. The test would then compare better-auth's apiKey surface against the fields listed in MANAGED_EXTENSION_FIELDS — which for sys_api_key is revoked, and only revoked. The columns that would actually collide on a library upgrade — name, prefix, key, user_id, expires_at — are not listed, so they would never be compared. The result is a green D7 check over a table with essentially no coverage: the same "reads as coverage, evaluates to nothing" shape this card is about, surviving its own fix.

    That is not an argument against the revoked-only entry in #7727. That entry is correct for its purpose — MANAGED_EXTENSION_FIELDS feeds the ADR-0092 D2 write whitelist, and revoked genuinely is the only column a generic write surface may touch. It is an argument that this card's fix needs the extension-field list for sys_api_key to be made complete at the same time, since the same map is doing double duty for D7 collision coverage. #7727's own code comment already states the premise for that completion: the table is hand-rolled (packages/core/src/security/api-key.ts mints and verifies it), better-auth's apiKey plugin is not loaded, so every column here is an extension field.

    Third step, easy to miss: the test builds its table surface from getAuthTables({ plugins: [organization({ teams: { enabled: true } })] }). apiKey() is not among those plugins, so adding the MODEL_TO_OBJECT entry alone still yields no apiKey table and the object stays silently skipped. Closing this properly means all three: pass the apiKey plugin to getAuthTables, add the MODEL_TO_OBJECT entry, and complete sys_api_key's declared extension fields.

    Worth pointing at the sibling gate too: the header notes D7 catches only overlap, while the complementary direction (every column better-auth writes must be provisioned) is better-auth-schema-parity.test.ts. If apiKey starts being modelled here, check whether it needs to be in scope there as well, or the same blind spot reopens from the other side.

    Left unassigned and un-routed — recording analysis, not claiming it.


    Generated by Claude Code

  3. self-assigned this
    on Aug 11, 2026
  4. os-help commented on Aug 11, 2026

    @os-help
    Collaborator

    Claim: PM loop (domain:identity seat, #6022)
    Session: session_01BVc1ekPpi6yaWywAUhfzfd
    Branch: claude/issue-7770-d7-guard-unmapped-managed-objects
    Worktree: objectstack-issue-7770
    Mode & model: M card, mode:subagent (sanctioned fallback), model: opus.

    Direction of record — the triage seat's second option, generalised: make the guard fail loudly on any unmapped managedBy: 'better-auth' object, with an explicit exemption registry carrying a reason per entry, and register sys_api_key there with the hand-rolled-table reason. That closes the class rather than this one instance, and it turns an absence that currently reads as an oversight into a recorded decision. It also matches a pattern this repo already uses for exactly this shape (SINGLE_RECORD_WRITE_ONLY in the api-methods conformance test — the same "if it genuinely must not, register it with the reason" channel).

    ⛔ The obvious fix is a trap, and the dispatch forbids walking into it. Per the domain:metadata seat's analysis on this thread (traced in the file, not inferred): adding apikey: 'sys_api_key' to MODEL_TO_OBJECT would make the D7 check compare better-auth's apiKey surface against MANAGED_EXTENSION_FIELDS[sys_api_key], which lists only revoked — a green check over a table with essentially no coverage, i.e. the very "reads as coverage, evaluates to nothing" shape this card is about, surviving its own fix.

    And the completion that would make that comparison meaningful is not a free move: MANAGED_EXTENSION_FIELDS does double duty as the ADR-0092 D2 write whitelist, so adding name / prefix / key / user_id / expires_at to it would widen what a generic write surface may touch on a credential table. That is a security change, not a test fix. The dev must not make it; if it concludes the guard cannot be made honest without it, that goes under escalations on this card.

    Also in scope as a reporting duty, not a change: the third step the metadata seat identified (the test builds its surface from getAuthTables({plugins: [organization(...)]}), so apiKey() is absent and any mapping alone still yields nothing), and the sibling gate better-auth-schema-parity.test.ts — state whether the same blind spot exists from the parity direction, without fixing it here.


    Generated by Claude Code

  5. os-help commented on Aug 11, 2026

    @os-help
    Collaborator
    {
      "issue": 7770,
      "status": "done",
      "premise_still_valid": true,
      "pr": "https://github.com/objectstack-ai/objectstack/pull/7822",
      "branch": "claude/issue-7770-d7-guard-unmapped-managed-objects",
      "commits": [
        "9a6236a88 test(plugin-auth): fail the D7 collision guard on unmapped better-auth-managed objects",
        "96b4bf047 test(plugin-auth): pin the covered-object list independently of MODEL_TO_OBJECT",
        "1d9e358d7 test(plugin-auth): point at #7820, the second axis of the same blindness"
      ],
      "files_changed": [
        "packages/plugins/plugin-auth/src/managed-extension-fields.test.ts (+325, -0)"
      ],
      "summary": "Implemented the direction of record: the D7 guard now scans every *.object.ts under packages/ for a line-anchored `managedBy: 'better-auth'` (22 objects today, 9 mapped) and fails unless each is in MODEL_TO_OBJECT or in a new UNMAPPED_MANAGED_OBJECTS registry carrying a reason per entry. `sys_api_key` is registered with the hand-rolled-table reason plus a `noBetterAuthColumns` flag, which is the only thing that lets an unmapped object carry MANAGED_EXTENSION_FIELDS entries — a second assertion fails when a declared extension field is skipped by the collision loop without that flag. A staleness companion fails on an entry that is no longer a declared managed object or is now mapped, and a targeted premise test pins the one claim `noBetterAuthColumns` makes for sys_api_key. The trap was avoided: nothing was added to MODEL_TO_OBJECT and nothing to MANAGED_EXTENSION_FIELDS, and both failure messages tell a future reader explicitly not to widen that map, because it is also the ADR-0092 D2 write whitelist. The diff is pure insertion — every pre-existing assertion survives byte-for-byte.",
      "failure_message": "these objects declare managedBy: 'better-auth' but are absent from MODEL_TO_OBJECT, so this guard skips them entirely and any extension field declared on them gets ZERO collision coverage while reading as covered: sys_api_key (packages/platform-objects/src/identity/sys-api-key.object.ts). Pick one deliberately:\n  (a) MAP IT — and it takes both halves, in this order: 1. pass the plugin that owns the model into the getAuthTables() call in betterAuthFieldsByObject(), because a map entry alone derives no table and the object stays silently skipped; 2. add the model: 'object_name' entry to MODEL_TO_OBJECT.\n  (b) REGISTER IT in UNMAPPED_MANAGED_OBJECTS with the reason better-auth owns no derivable surface for it here.\n  Do NOT complete MANAGED_EXTENSION_FIELDS just to make the comparison look meaningful: that map is also the ADR-0092 D2 write whitelist, so adding a column there widens what a generic write surface may touch. On a credential table that is a security change, not a test fix. Coverage of the columns better-auth itself owns is the parity gate's job (better-auth-schema-parity.test.ts), not D7's.  ||  [silent-skip assertion] these objects declare extension fields in MANAGED_EXTENSION_FIELDS but no better-auth surface was derived for them, so the collision assertion skipped them and proved nothing about their fields: sys_api_key. Either map the object (see the two-step recipe in the previous assertion), or — only if better-auth genuinely owns no column on that table at the pinned version — register it in UNMAPPED_MANAGED_OBJECTS with noBetterAuthColumns: true and a tripwire that fails when that stops being true.  ||  [staleness] stale UNMAPPED_MANAGED_OBJECTS entries: sys_user is now mapped in MODEL_TO_OBJECT — drop its exemption  ||  [premise tripwire] better-auth now exports an apiKey plugin, so sys_api_key CAN acquire better-auth-owned columns and its UNMAPPED_MANAGED_OBJECTS exemption no longer holds. Re-decide: keep the hand-rolled table and do not load the plugin (restate the reason and repoint this tripwire at the auth manager's plugin list), or adopt the plugin — in which case map apikey: 'sys_api_key' AND pass apiKey() to getAuthTables() here, add sys_api_key to better-auth-schema-parity.test.ts, and reconcile the overlapping columns (name, prefix, key, user_id, expires_at) as an ownership decision, NOT by widening MANAGED_EXTENSION_FIELDS, which is also the ADR-0092 D2 write whitelist.",
      "parity_gate_finding": "better-auth-schema-parity.test.ts does NOT share the same blind spot, because it is driven by the derived model set rather than by an object map: it iterates Object.entries(tables), so an object missing from its PLATFORM_OBJECTS map cannot be silently skipped — it would surface as a loud 'better-auth would write tables no platform object provisions: apikey -> apikey' from its 'every better-auth model maps to a platform object' assertion. Today it derives no apikey model (apiKey() is not in its plugin list) and therefore asserts nothing at all about sys_api_key, which is the correct answer while better-auth writes no column there, and it self-announces on the day the model appears. What both gates DO share is one upstream precondition worth recording: each hand-maintains its own getAuthTables() plugin list and nothing reconciles either list against auth-manager.ts's actual assembly, so enabling a plugin in the auth manager while forgetting a test's list leaves both gates silent. Reported only; not fixed here.",
      "apikey_enablement_behaviour": "Actionable, and it fires one step earlier than the card anticipated. The premise test goes red at the DEPENDENCY BUMP that makes an apiKey plugin available, before anyone can enable it, and its message names the fork and — for the adopt branch — all four steps: map apikey, pass apiKey() to getAuthTables() here, add sys_api_key to better-auth-schema-parity.test.ts, and reconcile name/prefix/key/user_id/expires_at as an ownership decision rather than by widening the D2 whitelist. If instead someone adds the mapping without the premise expiring, the staleness assertion fails and names the entry to drop. If someone declares a new extension field on any exempted object, the silent-skip assertion fails and names the two-step recipe.",
      "tests": "Dependency closure built first: pnpm --workspace-concurrency=2 --filter '@objectstack/plugin-auth^...' build -> Done.  pnpm --filter @objectstack/plugin-auth test -> 'Test Files 40 passed (40) / Tests 1003 passed (1003)' (re-run after the final commit and after merging origin/main, which was already up to date).  Targeted: npx vitest run src/managed-extension-fields.test.ts -> 'Test Files 1 passed (1) / Tests 12 passed (12)', 11 in the D7 describe plus the premise test.  pnpm --filter @objectstack/plugin-auth typecheck -> tsc --noEmit, clean.  npx eslint on the changed file -> exit 0.  node scripts/check-nul-bytes.mjs -> 'OK (scanned 7136 text file(s) ... no raw ASCII control bytes)'; targeted self-scan grep -naP over the changed file -> clean.  CI at report time (draft-PR-time reporting, #6644 L2): Check Changeset SUCCESS (the skip-changeset label landed ~50s ahead of its re-read), Check PR Size / ADR maintainer approval / duplicate-issue guard / Console Pin Freshness / doc links all success; ESLint and TypeScript Type Check still in_progress, not waited on.",
      "ablation": "Five mutations, each with the direction predicted before running. A) delete the sys_api_key exemption -> PREDICTED red on 'mapped or registered' and on 'silently skipped', with the collision assertion staying GREEN; OBSERVED exactly that, plus a third red because the mutation renamed the key rather than deleting it so the staleness assertion fired too. The collision assertion's green through this ablation is the whole point — that green alone is what #7770 reports. B) keep the entry, drop noBetterAuthColumns -> PREDICTED red on 'silently skipped' only; OBSERVED as predicted. C) exempt sys_user, an object that IS mapped -> PREDICTED red on staleness with the 'now mapped' branch; OBSERVED as predicted. D) point the premise tripwire at a symbol that does exist -> PREDICTED red with the re-decide message; OBSERVED as predicted. E) delete teamMember: 'sys_team_member' from MODEL_TO_OBJECT -> PREDICTED two reds (coverage pin plus accounting); OBSERVED ONE — INVERTED, and the inversion was a real defect in my own assertion, not in the prediction. The coverage pin compared the derived surface against Object.values(MODEL_TO_OBJECT), so both sides shrank with the map and the pin could never see a deleted mapping. Fixed in commit 96b4bf047 by writing the nine covered objects out as a literal and asserting the map against it; re-running ablation E after the fix produced the two reds originally predicted. All mutations were made and reverted with edits plus git checkout inside the worktree — no git stash at any point.",
      "neighbors": "Stayed entirely out of #7732's territory: no edit to auth-manager.ts, no session-revocation path, no sys_session tombstoning. The whole diff is one file, managed-extension-fields.test.ts, which #7732 has been told to stay out of — boundary held from both sides. merged origin/main before the final push: 'Already up to date' (origin/main at 098b62914). Did not flip ready, did not arm auto-merge; PR left as draft per the poisoned-queue instruction.",
      "escalations": [
        "None blocking. The guard was made honest WITHOUT the forbidden move: it now fails loudly on the unmapped class and records sys_api_key's absence as a reasoned decision, so completing MANAGED_EXTENSION_FIELDS (the ADR-0092 D2 write whitelist widening) was never needed. Recording the shape anyway, since it is the decision a future reader will face: D7 coverage of sys_api_key's real columns is unobtainable without either widening that whitelist on a credential table or giving D7 a second, read-only field source separate from the D2 registry. The second option is the one worth considering if it ever becomes live, and it is a maintainer call, not mine.",
        "For awareness, not action on this card: better-auth 1.7.0-rc.2 ships NO apiKey plugin at all — no 'better-auth/plugins/api-key' export subpath, no apiKey export from 'better-auth/plugins', and no @better-auth/api-key package in the lockfile. The issue's latent-hazard framing assumes a plugin that is not in the dependency graph. This does not falsify the card (the skip is real and verified in the file), it narrows the trigger: the hazard arrives at a dependency bump, which is what the new premise test now watches."
      ],
      "out_of_scope_findings": [
        "filed as #7820: the second axis of the same blindness — this file's getAuthTables() loads `organization` only while auth-manager.ts loads many more plugins behind flags, so a column an unloaded plugin owns is invisible even for a MAPPED object. sys_user.phone_number is a live instance: it is declared in MANAGED_EXTENSION_FIELDS.sys_user AND is the column better-auth's phoneNumber plugin writes (mapped explicitly by AUTH_PHONE_NUMBER_USER_FIELDS in auth-schema-config.ts). Measured: with D7's plugin set the user surface is name/email/emailVerified/image/createdAt/updatedAt; with the auth manager's set it also carries twoFactorEnabled, role, banned, banReason, banExpires, phone_number, phone_number_verified. Not fixed here because widening the plugin set turns the collision assertion RED on an ownership question nobody has decided. Searched open issues by keyword and file path first — no existing report. It is the only overlap: every other declared extension field on sys_user, sys_organization and sys_invitation is clean against the wider surface."
      ],
      "deviations": [
        "Scope of the exemption registry is 13 entries, not 1. The direction of record is 'fail on any object that declares managedBy: better-auth but is absent from MODEL_TO_OBJECT', and 22 objects declare it while 9 are mapped — so closing the class required a reason for each of the other 12 alongside sys_api_key. The reasons are grouped by family (opt-in core plugins this file does not load; sso/scim, which expose no schema option for getAuthTables() to read, #3653; the seven @better-auth/oauth-provider objects, which have their own gate) and each says what makes the absence tolerable, which for all 12 is that no extension field is declared on them — the moment one is, the silent-skip assertion turns red.",
        "Enumeration is a source scan over packages/**/*.object.ts rather than an import of the platform-objects barrel. This follows the exemplar the dispatch named (api-methods-batch-conformance.test.ts), whose own header records why: an audit whose scope is a package name misses the declaration that lands in a package nobody thought to open. Every managedBy: 'better-auth' object lives in platform-objects/identity today; nothing makes that permanent.",
        "Added one assertion beyond the card's list — a floor on the scan size — because a scan that matches nothing would pass the two new assertions vacuously, which is the same 'reads as coverage, evaluates to nothing' defect this card is about."
      ],
      "changeset": "None; `skip-changeset` label applied and read back as ['size/m','skip-changeset']. Justified: the entire diff is one *.test.ts file. No shipped code, no published artifact, nothing user-visible — so a patch changeset would announce a release note for a change no consumer can observe. Check Changeset ran green on the first run (the label landed ahead of its live re-read)."
    }

    Generated by Claude Code


    Generated by Claude Code

  6. os-help commented on Aug 11, 2026

    @os-help
    Collaborator

    Review: ACCEPT — PR #7822 (PM session session_01BVc1ekPpi6yaWywAUhfzfd; diff read, not the report).

    The trap was avoided and the refusal is written into the failure text, which is the right place for it: nothing was added to MODEL_TO_OBJECT, nothing to MANAGED_EXTENSION_FIELDS, and the message a future reader hits names the two-step map recipe, the register alternative, and why completing the extension list is a security change on a credential table rather than a test fix. The failure message is the deliverable here and it was treated that way.

    Verified in the diff, not taken from the report: one file, 325 pure insertions, every pre-existing assertion byte-for-byte intact; COVERED_OBJECTS is a hard-coded literal with both MODEL_TO_OBJECT's values and the derived surface asserted against it; the scan carries a floor (> 15, must contain sys_api_key) so an empty sweep cannot satisfy the new assertions vacuously.

    Ablation E is the find, and it is a good one. The prediction was wrong because the assertion was: the coverage pin compared the derived surface against Object.values(MODEL_TO_OBJECT), so deleting a mapping shrank expectation and reality together and the pin could never fail. That is an assertion that cannot fail, caught by a mutation, in a PR whose entire subject is a check that reads as coverage and evaluates to nothing — and it was self-caught and fixed (96b4bf047) rather than explained away, with the ablation re-run afterwards producing the originally predicted reds. Second instance this shift of that defect class; the first was #7665's vacuous explain pin.

    The premise correction is accepted and improves the card. better-auth 1.7.0-rc.2 ships no apiKey plugin at all — no subpath export, no named export, nothing in the lockfile — so the card's (and my dispatch's) "the day someone enables it" framing named a trigger that cannot occur yet. Turning that into a tripwire that fires at the dependency bump is strictly better: it expires the exemption one step before anyone can make the mistake, and its message carries all four reconciliation steps. Checking the premise instead of inheriting it is exactly what the dispatch asked for.

    13 registry entries rather than 1 is a correct reading of the direction, not scope creep: closing the class required a reason for each of the 22 − 9 unmapped objects, grouped by family, each stating what makes the absence tolerable and each armed by the silent-skip assertion the moment an extension field appears on it.

    Parity-gate answer is properly negative: better-auth-schema-parity.test.ts is driven by the derived model set, so an object missing from its map fails loudly rather than being skipped — no shared blind spot. The shared precondition it identified is the more valuable half: both gates hand-maintain their own getAuthTables() plugin list and nothing reconciles either against auth-manager.ts's real assembly. Recorded on the seat post.

    #7820 (filed from this work) is triaged into the decision box — sys_user.phone_number is simultaneously a declared ObjectStack extension field and the column better-auth's phoneNumber plugin writes, per the repo's own AUTH_PHONE_NUMBER_USER_FIELDS mapping. Refusing to guess an ownership call on an identity column was right.

    ⚠️ Not arming. #7822 stays draft with the lane's other four reviewed PRs while the merge queue is poisoned by #7793 (fix PR #7803). It will be flipped ready and armed automatically once that lands.


    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

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions