From dab5649c7e96ecd48492cbdba2dce483fc54fe3e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 16:52:11 +0000 Subject: [PATCH] fix(platform-objects): make the last two non-unique keyed text indexes expressible on MySQL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A non-unique index over a text column MySQL cannot key is refused outright (ER_BLOB_KEY_WITHOUT_LENGTH), failing the whole object's syncSchema and leaving it registered with its declared index absent. #11627's hash shadow closed the UNIQUE half of this class; it structurally cannot serve the non-unique half, because an index over a digest accelerates no WHERE col = ? the planner can reach. Two members remained, ruled separately by the maintainer on 2026-08-25: - sys_verification.value: the declared index is REMOVED. The column is genuinely unboundable (oauth-provider writes OIDC authorization-code payloads there as a JSON blob) and the index was measured dead — better-auth 1.7.1 keys verification lookups on identifier/id/expiresAt, upstream declares the field unindexed and unbounded, and no in-repo query filters by value. - sys_oauth_client_resource.resource_id: the declared bound narrows 1024 -> 768. This is a live access path (FK side of sys_oauth_resource.identifier), so it keeps its index and becomes keyable instead. Narrowing below the referent's 1024 is safe because upstream better-auth stores that same identifier as varchar(255) on MySQL and this referring column as varchar(36), so nothing the producing contract can emit lives in the discarded band. The package pin gains the executable form of "the class is closed": it now rejects a non-unique index over any text column MySQL cannot key, enumerated across the whole package rather than naming these two. Its UNBOUNDABLE allowlist, which existed to excuse sys_verification.value, moves with the change and is empty; a synthetic control keeps its excusing branch exercised. Part of #11701 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01W6HFzyH98W1YaQXhJUJt6o --- .../non-unique-keyed-text-remainders.md | 49 ++++ .../sys-oauth-client-resource.object.ts | 30 ++- .../src/identity/sys-verification.object.ts | 41 +++- .../src/platform-keyed-text-bounds.test.ts | 209 +++++++++++++++--- 4 files changed, 296 insertions(+), 33 deletions(-) create mode 100644 .changeset/non-unique-keyed-text-remainders.md diff --git a/.changeset/non-unique-keyed-text-remainders.md b/.changeset/non-unique-keyed-text-remainders.md new file mode 100644 index 0000000000..fefc02e2fd --- /dev/null +++ b/.changeset/non-unique-keyed-text-remainders.md @@ -0,0 +1,49 @@ +--- +'@objectstack/platform-objects': minor +--- + +Make the last two non-unique keyed text indexes expressible on MySQL — remove one, +narrow one + +`driver-sql` emits a keyed text-family column as `varchar(maxLength)` only when the +declared bound is one MySQL can key (768 characters on utf8mb4, the 3072-byte key-part +ceiling); otherwise the column stays `TEXT`, MySQL refuses it as an index key +(`ER_BLOB_KEY_WITHOUT_LENGTH`), and the object's whole `syncSchema` fails — it lands +registered with its declared index absent. #11374 declared sourced bounds for thirteen +such columns and #11627 carried the over-long UNIQUE ones on a SHA-256 hash-shadow +column, taking live MySQL 8.0.46 from 12/44 → 8/44 → 2/44 failing objects. + +The two that remained are **non-unique**, and a hash shadow structurally cannot serve +them: a UNIQUE constraint is an equality-only predicate that survives hashing exactly, +but a non-unique index exists for an access path, and an index over a digest +accelerates no `WHERE col = ?` the planner can reach without rewriting the read side. +They are ruled separately (maintainer, 2026-08-25) because they are different problems: + +- **`sys_verification.value` — the declared index is removed.** The column is + genuinely unboundable (better-auth's oauth-provider writes OIDC authorization-code + payloads there as a JSON blob), and the index was measured dead: better-auth 1.7.1 + keys every verification lookup on `identifier`, `id` or `expiresAt` + (`internal-adapter.mjs`), upstream declares the field unindexed and unbounded, and no + in-repo query filters `sys_verification` by `value`. An index that silently does not + exist on one dialect is the worst of both worlds; removing it makes the metadata match + reality. +- **`sys_oauth_client_resource.resource_id` — the declared bound narrows 1024 → 768.** + This one is a live access path (the FK side of `sys_oauth_resource.identifier`, read + as a predicate by upstream's client-registration collision path), so it keeps its + index and becomes keyable instead. 768 is the widest utf8mb4 value a MySQL key part + holds, and the smallest narrowing that works. + +This is an enforcement change on published objects — hence the minor grade. On MySQL and +SQL Server a `resource_id` longer than 768 characters is now refused rather than stored, +and on PostgreSQL and SQLite the `sys_verification` `[value]` index is dropped on the +next schema sync (on MySQL it never existed). Neither narrows what the producing +contract can emit: the value is an RFC 8707 resource-indicator URI, and upstream +better-auth 1.7.1 stores that same identifier as `varchar(255)` on MySQL +(`get-migration.mjs`) and this referring column as `varchar(36)`, so a resource whose +identifier exceeded 768 characters could never have been registered upstream at all. + +The pin that enumerated the package for unbounded keyed text columns now also rejects a +non-unique index over any text column MySQL cannot key, so a third member of the class +fails at test time rather than on a live server. Its `UNBOUNDABLE` allowlist — which +existed to excuse `sys_verification.value` — is empty as a result, and a synthetic +control keeps the excusing branch exercised rather than letting it rot. diff --git a/packages/platform-objects/src/identity/sys-oauth-client-resource.object.ts b/packages/platform-objects/src/identity/sys-oauth-client-resource.object.ts index 7275fd7389..efe3b9ffa2 100644 --- a/packages/platform-objects/src/identity/sys-oauth-client-resource.object.ts +++ b/packages/platform-objects/src/identity/sys-oauth-client-resource.object.ts @@ -52,7 +52,35 @@ export const SysOauthClientResource = ObjectSchema.create({ resource_id: Field.text({ label: 'Resource ID', required: true, - maxLength: 1024, + // [#11701] Narrowed 1024 → 768 so the declared `[resource_id]` index can + // exist at all. At 1024 the column stays TEXT on MySQL and the index is + // refused (`ER_BLOB_KEY_WITHOUT_LENGTH`), taking the whole object's + // schema-sync down with it; 768 characters is the widest utf8mb4 value a + // MySQL key part can hold (768 × 4 = 3072 bytes, exactly the ceiling). + // + // ⚠️ This is the one bound in the family that does NOT simply take its + // referenced column's width: `sys_oauth_resource.identifier` declares + // 1024. Narrowing below the referent is safe here because the + // (768, 1024] band holds nothing the PRODUCING contract can emit. The + // value is an RFC 8707 resource-indicator URI, and upstream better-auth + // 1.7.1 — the sole writer of this table (`managedBy: 'better-auth'`) — + // stores that same identifier in `oauthResource.identifier` as + // **varchar(255)** on MySQL (`better-auth/dist/db/get-migration.mjs`, + // `getType`: a unique string column → varchar(255)) and this referring + // column as varchar(36) (its `field.references` branch). A resource + // whose identifier exceeded 768 characters could never have been + // registered upstream in the first place. + // + // 768 rather than upstream's 255 on purpose: it is the SMALLEST + // narrowing that makes the index expressible, so it rejects the least of + // the referent's declared domain. Guessing a tighter number to make a + // key fit is what `sys_account.issuer` refuses to do. + // + // ⛔ Unlike `sys_verification.value`, this index is NOT removable: this + // is the FK side of `sys_oauth_resource.identifier` and upstream reads it + // as a predicate (`findOne({ clientId, resourceId })` on the client + // registration collision path), so it is a live access path. + maxLength: 768, description: 'Foreign key to sys_oauth_resource.identifier', }), diff --git a/packages/platform-objects/src/identity/sys-verification.object.ts b/packages/platform-objects/src/identity/sys-verification.object.ts index c9d3e79d7a..38882d823e 100644 --- a/packages/platform-objects/src/identity/sys-verification.object.ts +++ b/packages/platform-objects/src/identity/sys-verification.object.ts @@ -57,6 +57,12 @@ export const SysVerification = ObjectSchema.create({ value: Field.text({ label: 'Verification Token', required: true, + // [#11374/#11701] Deliberately UNBOUNDED: better-auth's oauth-provider + // writes OIDC authorization-code payloads here as a JSON blob, so no + // bound provably admits every value it may write. That is only + // survivable because the column carries no index — see the `indexes` + // note below, which is what makes an unbounded TEXT column safe on + // MySQL. description: 'Token or code for verification', }), @@ -81,13 +87,34 @@ export const SysVerification = ObjectSchema.create({ }, indexes: [ - // `value` must NOT be unique. better-auth's oauth-provider stores OIDC - // authorization codes in this table with `value` = a JSON blob keyed by - // user+client+state, which can legitimately repeat. A UNIQUE constraint - // makes `/api/v1/auth/oauth2/authorize` fail (`UNIQUE constraint failed: - // sys_verification.value`) → 503, breaking cloud-as-IdP SSO entirely. - // better-auth keys verification lookups on `identifier`, not `value`. - { fields: ['value'], unique: false }, + // [#11701] `value` carries NO index — and must not gain one. + // + // Removing the index it used to declare is the maintainer's 2026-08-25 + // ruling, taken on MEASURED liveness rather than on convenience: + // + // • better-auth 1.7.1 keys every verification lookup on `identifier` + // (or on `id`, or on `expiresAt` for cleanup) — see + // `internal-adapter.mjs`'s `findByIdentifier` / `consumeByIdentifier`; + // • upstream declares the field unindexed and unbounded; + // • no in-repo query filters `sys_verification` by `value`. + // + // ⛔ It could not be indexed here even if a reader wanted it. `value` is + // UNBOUNDABLE — better-auth's oauth-provider stores OIDC + // authorization-code payloads in it as a JSON blob, so no defensible + // `maxLength` exists — so on MySQL the column stays TEXT and ANY index + // over it is refused (`ER_BLOB_KEY_WITHOUT_LENGTH`), failing the whole + // object's schema-sync over an index nothing reads. #11627's hash-shadow + // route cannot rescue it either: a shadow carries a UNIQUE constraint, + // and an index over a digest accelerates no `WHERE value = ?` the planner + // can reach. An index that silently does not exist on one dialect is the + // worst of both worlds; removing it makes the metadata match reality, + // which is the `declared = enforced` property this family restores. + // + // ⛔ A UNIQUE index here would be wrong twice over: those JSON payloads + // legitimately repeat (they are keyed by user+client+state), and a unique + // constraint made `/api/v1/auth/oauth2/authorize` fail (`UNIQUE + // constraint failed: sys_verification.value`) → 503, breaking + // cloud-as-IdP SSO entirely. { fields: ['identifier'], unique: false }, { fields: ['expires_at'], unique: false }, ], diff --git a/packages/platform-objects/src/platform-keyed-text-bounds.test.ts b/packages/platform-objects/src/platform-keyed-text-bounds.test.ts index adf7248c73..6317aea798 100644 --- a/packages/platform-objects/src/platform-keyed-text-bounds.test.ts +++ b/packages/platform-objects/src/platform-keyed-text-bounds.test.ts @@ -43,15 +43,44 @@ import * as PlatformObjects from './index'; * A new keyed text-family field arrived without a `maxLength`. Do not silence * the assertion — derive a bound from the value's producer (upstream * better-auth schema/constraints, IdP norms, or the in-repo producer) and - * declare it, or, if the value source genuinely cannot be bounded (the - * `sys_verification.value` case below), extend the allowlist WITH a comment - * naming why and where the keyability debt is tracked. + * declare it. If the value source genuinely cannot be bounded, extend + * `UNBOUNDABLE` WITH a comment naming why — but read the #11701 block below + * first: an unboundable column may only be keyed by a UNIQUE index, because a + * UNIQUE index is the only kind #11627's hash shadow can carry. * * A bound may legitimately exceed 768 chars (the utf8mb4 index-key ceiling — * e.g. `sys_account.issuer` at 2048, the oauth token columns at 1024): the - * column then stays TEXT and its index still cannot exist on MySQL. That debt - * is #11627's (hash-shadow keys), and this pin does not police it — it polices - * only "keyed text declares its bound". + * column then stays TEXT and its index still cannot exist on MySQL directly. + * That debt was #11627's, and #11627 discharged it for the UNIQUE half — such + * an index is now carried on a hash-shadow column. The first `describe` below + * still polices only "keyed text declares its bound". + * + * ## #11701 — the NON-UNIQUE half, which a hash shadow cannot serve + * + * The second `describe` polices the case #11627 deliberately left refused. A + * UNIQUE constraint is an equality-only predicate, so hashing the value + * preserves it exactly; a NON-UNIQUE index exists for an ACCESS PATH, and an + * index over a digest accelerates no `WHERE col = ?` the planner can reach + * without rewriting the read side. So for a non-unique index there is no + * shadow to fall back on: the column must be KEYABLE — bounded, and bounded at + * or under 768 — or the index cannot exist on MySQL at all and the object's + * whole schema-sync is refused. + * + * That left exactly two platform members, and the maintainer ruled them + * separately on 2026-08-25 because they are different problems: + * + * • `sys_verification.value` — unboundable AND unread. The declared index was + * REMOVED, on measured liveness (better-auth keys verification lookups on + * `identifier`; no in-repo query filters by `value`). Removing it is what + * emptied `UNBOUNDABLE` below. + * • `sys_oauth_client_resource.resource_id` — a LIVE access path (the FK side + * of `sys_oauth_resource.identifier`), so its bound was narrowed + * 1024 → 768 instead. See the field's own comment for the evidence that + * nothing legitimate lives in the discarded band. + * + * The pin below is the executable form of "the class is closed": it does not + * name those two, it enumerates the whole package, so a THIRD member arriving + * later fails here rather than being found on a live MySQL months on. */ const TEXT_FAMILY = new Set(['text', 'textarea', 'html', 'markdown']); @@ -59,17 +88,33 @@ const TEXT_FAMILY = new Set(['text', 'textarea', 'html', 'markdown']); /** * Keyed text-family columns with NO defensible bound. Every entry must name * why. Entries that stop matching a real keyed unbounded column fail the - * third test, so the list cannot rot. + * fourth test, so the list cannot rot. + * + * ⚠️ EMPTY since #11701 — and empty here is a RESULT, not a default. The list + * held exactly one entry, `sys_verification.value`, allowlisted because + * better-auth's oauth-provider writes OIDC authorization-code payloads there as + * a JSON blob and no bound provably admits all of them. That entry was written + * to explain why the column could not be BOUNDED, and the maintainer's + * 2026-08-25 ruling did not bound it — it removed the column's declared INDEX, + * on measured liveness. An unindexed column is not a keyed column, so the entry + * stopped describing anything real and moved with the change rather than being + * left to rot. (The fourth test enforces exactly that: it is what would have + * gone red had the entry been left behind.) + * + * ⚠️ Before adding an entry: an unboundable column may only be keyed by a + * UNIQUE index, which #11627 carries on a hash shadow. A NON-UNIQUE index over + * an unboundable column is not "debt" — it is unfixable, and the #11701 + * `describe` below rejects it. + */ +const UNBOUNDABLE: ReadonlySet = new Set([]); + +/** + * MySQL's utf8mb4 key-part ceiling, in CHARACTERS: 768 × 4 = 3072 bytes, the + * whole key-part budget. A declared bound at or under this makes `driver-sql` + * emit `varchar(n)`, which MySQL can key; anything wider stays TEXT, which it + * refuses to key without a prefix length. */ -const UNBOUNDABLE: ReadonlySet = new Set([ - // better-auth's oauth-provider stores OIDC authorization-code payloads in - // `verification.value` as a JSON blob (see the index comment in - // sys-verification.object.ts), and upstream deliberately declares the field - // unindexed and unbounded — no bound exists that provably admits every value - // better-auth may write. Its ObjectStack-declared index therefore still - // cannot exist on MySQL; that keyability debt is tracked with #11627. - 'sys_verification.value', -]); +const MAX_KEYABLE_CHARS = 768; type AnyObject = { name: string; @@ -96,6 +141,26 @@ function keyedTextColumns(o: AnyObject): Array<{ column: string; maxLength: unkn .map(([column, def]) => ({ column: `${o.name}.${column}`, maxLength: def.maxLength })); } +/** + * The rule the third test enforces, as a pure function of (objects, allowlist). + * + * Extracted rather than inlined because #11701 emptied `UNBOUNDABLE`: with the + * allowlist empty, the `allowlist.has(column)` branch is never taken against the + * real objects, so it would sit unexecuted and free to rot until the next agent + * needed it. The synthetic control below drives both of its outcomes. + */ +function unboundedKeyedColumns(objects: AnyObject[], allowlist: ReadonlySet): string[] { + const offenders: string[] = []; + for (const o of objects) { + for (const { column, maxLength } of keyedTextColumns(o)) { + if (allowlist.has(column)) continue; + const bounded = typeof maxLength === 'number' && Number.isInteger(maxLength) && maxLength > 0; + if (!bounded) offenders.push(`${column} (maxLength: ${String(maxLength)})`); + } + } + return offenders; +} + describe('platform keyed text-family columns declare their bound (#11374)', () => { it('enumerates a real surface — the probe itself is not vacuous', () => { // Positive control: if the export shape or field/index spelling changes so @@ -120,15 +185,7 @@ describe('platform keyed text-family columns declare their bound (#11374)', () = }); it('every keyed text-family column declares a positive integer maxLength, or is allowlisted by name', () => { - const offenders: string[] = []; - for (const o of platformObjects) { - for (const { column, maxLength } of keyedTextColumns(o)) { - if (UNBOUNDABLE.has(column)) continue; - const bounded = - typeof maxLength === 'number' && Number.isInteger(maxLength) && maxLength > 0; - if (!bounded) offenders.push(`${column} (maxLength: ${String(maxLength)})`); - } - } + const offenders = unboundedKeyedColumns(platformObjects, UNBOUNDABLE); expect( offenders, `keyed text-family column(s) without a declared maxLength — on MySQL their ` + @@ -150,4 +207,106 @@ describe('platform keyed text-family columns declare their bound (#11374)', () = ).toBeUndefined(); } }); + + /** + * ⚠️ The control that keeps the test above honest now that #11701 emptied the + * allowlist. An empty `for` loop passes, so with a real-objects-only check the + * excusing branch of the rule would be dead code that nobody notices rotting. + * This drives BOTH outcomes on a synthetic object, so the mechanism a future + * unboundable column will rely on is proven to work while the list is empty. + */ + it('the allowlist mechanism still excuses and still accuses — driven on a synthetic object', () => { + const synthetic: AnyObject[] = [ + { + name: 'sys_probe', + fields: { blob: { type: 'text' } }, + indexes: [{ fields: ['blob'], unique: true }], + }, + ]; + // Keyed + unbounded, excused by nothing → an offender, named with its value. + expect(unboundedKeyedColumns(synthetic, new Set())).toEqual([ + 'sys_probe.blob (maxLength: undefined)', + ]); + // …and named in the allowlist → excused. The branch the real objects no + // longer reach. + expect(unboundedKeyedColumns(synthetic, new Set(['sys_probe.blob']))).toEqual([]); + }); +}); + +/** + * #11701 — a NON-UNIQUE index over a text column MySQL cannot key. + * + * See this file's header for why this is a different defect from #11374's: + * a UNIQUE index over an unkeyable column is EXPRESSIBLE after #11627 (it moves + * onto a SHA-256 hash-shadow column), but a non-unique one is not — hashing + * destroys the ordering and prefix structure an access path is for, so there is + * no fallback and the column itself must be keyable. + * + * Measured on live MySQL 8.0.46: while these two members existed, each one + * failed `syncSchema` for its whole object with `ER_BLOB_KEY_WITHOUT_LENGTH` — + * `CREATE TABLE` succeeded and the following `ALTER TABLE … ADD INDEX` did not, + * so the object landed registered with its declared index absent. + */ +describe('platform non-unique text indexes are keyable on MySQL (#11701)', () => { + /** Every (object, column) a NON-UNIQUE declared index keys on a text field. */ + const nonUniqueKeyedTextColumns = (): Array<{ column: string; maxLength: unknown }> => { + const out: Array<{ column: string; maxLength: unknown }> = []; + for (const o of platformObjects) { + for (const ix of o.indexes ?? []) { + if (ix.unique) continue; // UNIQUE → carried on a hash shadow (#11627) + for (const f of ix.fields ?? []) { + const def = o.fields[f]; + if (!TEXT_FAMILY.has(def?.type ?? '')) continue; + out.push({ column: `${o.name}.${f}`, maxLength: def.maxLength }); + } + } + } + return out; + }; + + /** + * Positive control FIRST, and it does double duty: it proves the probe sees a + * real population rather than passing empty, and it pins both of #11701's + * dispositions by name — the index that was removed must stay removed, and + * the column that kept its index must stay in the enumerated set (the rule + * below then holds it to a keyable bound). + */ + it('enumerates a real surface, and pins both #11701 dispositions by name', () => { + const columns = nonUniqueKeyedTextColumns().map((c) => c.column); + // Measured at 55 on this tree; the floor is set just under it so an + // enumeration that collapses (a changed export or index spelling) fails + // here rather than passing over a surface it can no longer see. + expect(columns.length).toBeGreaterThanOrEqual(50); + + // Kept: bounded at 255, a live better-auth lookup key. + expect(columns).toContain('sys_verification.identifier'); + // ⛔ REMOVED by the 2026-08-25 ruling — unboundable and unread. If this + // comes back, it comes back with a live reader and a keyable bound, or it + // fails here and in the rule below. + expect(columns).not.toContain('sys_verification.value'); + // Kept: a live access path (FK side of sys_oauth_resource.identifier), + // narrowed 1024 → 768 so the index can exist at all. + expect(columns).toContain('sys_oauth_client_resource.resource_id'); + }); + + it('every non-unique-keyed text column is bounded at or under the utf8mb4 key ceiling', () => { + const offenders = nonUniqueKeyedTextColumns() + .filter(({ maxLength: n }) => { + const keyable = + typeof n === 'number' && Number.isInteger(n) && n > 0 && n <= MAX_KEYABLE_CHARS; + return !keyable; + }) + .map(({ column, maxLength }) => `${column} (maxLength: ${String(maxLength)})`); + + expect( + offenders, + `non-unique declared index/indexes over a text column MySQL cannot key. Unlike the ` + + `UNIQUE case there is NO hash-shadow fallback (#11627) — an index over a digest ` + + `accelerates no 'WHERE col = ?' — so the whole object fails syncSchema with ` + + `ER_BLOB_KEY_WITHOUT_LENGTH. Either bound the column at <= ${MAX_KEYABLE_CHARS} ` + + `characters from its producer, or, if nothing reads it as a predicate, remove the ` + + `index and say so (the two routes #11701 took): ` + + offenders.join(', '), + ).toEqual([]); + }); });