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([]); + }); });