Skip to content

fix(platform-objects): source sys_oauth_resource.identifier's bound from its producer — 1024 → 255, and the referring column with it - #12595

Open
os-warren wants to merge 2 commits into
mainfrom
claude/issue-12313-oauth-identifier-sourced-255
Open

fix(platform-objects): source sys_oauth_resource.identifier's bound from its producer — 1024 → 255, and the referring column with it#12595
os-warren wants to merge 2 commits into
mainfrom
claude/issue-12313-oauth-identifier-sourced-255

Conversation

@os-warren

Copy link
Copy Markdown
Collaborator

Fixes #12313

Implements the maintainer ruling of 2026-08-26 (verbatim 「同意」 on option B, with its stock-reading precondition): sys_oauth_resource.identifier and sys_oauth_client_resource.resource_id both narrow to a sourced 255. Options A (768, unsourced alignment) and C (keep 1024 + citation) were weighed and rejected upstream of this PR and are not taken here.

Step 1 — the binding precondition, run before anything changed

Measure existing rows: any stored identifier / resource_id value longer than 255 in reachable deployments' system tables.

Verdict: clean — but vacuously so, and that distinction is load-bearing.

Method. Enumerated every reachable engine, not just the convenient one. MySQL 8.0.46 is live in this container; information_schema was queried directly rather than inferred. Postgres: pg_isreadyno response, TCP connect refused. SQLite: no ObjectStack database file anywhere under /home, /root, /var, /opt, /srv (the only .db files present are Docker's own boltdb/containerd internals). So MySQL was the whole reachable population.

Across all four schemas holding these tables (os11374probe, os11627, os11627b, os11627f — prior agents' probe databases):

schema column rows max chars > 255
all four sys_oauth_resource.identifier 0 0 0
all four sys_oauth_client_resource.resource_id 0 0 0

Control. A zero-hit is not a reading until a positive control fires in the same scan, so one did. A fifth schema was created with the same table shapes and planted with identifiers of 256, 300, 900 and 1024 characters plus one short value, then the identical scan was run over all five schemas in one pass:

schema column rows max chars > 255
os12313probe (control) sys_oauth_resource.identifier 5 1024 4
os12313probe (control) sys_oauth_client_resource.resource_id 2 900 1

The scan finds long values when they exist, and correctly excludes the short one. The four real schemas are clean.

⚠️ What this reading does and does not settle. Every reachable table is empty, so this is a clean reading over a population of zero rows. It does not disprove the self-hosted-bypass possibility the decision analysis flagged — no customer or production deployment is reachable from this container. It establishes only that nothing reachable here holds such a value. Flagged for the PM rather than buried.

Step 2 — the narrowing, each column with a producer citation

The card's whole complaint is that 1024 cites no producer, so a narrowing that landed without a citation would reproduce the defect at a smaller number. Both citations were measured firsthand, by running better-auth 1.7.1's own migration generator against live MySQL 8.0.46 and then reading information_schema.COLUMNS as its own query:

upstream column measured why that branch
oauthResource.identifier varchar(255) (1020 octets) { unique: true }getType's field.unique arm; oauthResource declares no table-level indexes, so tableIndexStringLength is undefined
oauthClient.clientId varchar(255) confirms the existing client_id citation
oauthClientResource.resourceId varchar(191) not varchar(36) — see below

sys_oauth_resource.identifier therefore takes 255 directly from its producer. sys_oauth_client_resource.resource_id takes 255 from the referent, by the same referring-column derivation client_id already uses — which the oauthClient.clientId measurement above independently confirms is this family's established rule.

⚠️ A correction to the #11701 citation, carried in this PR

The in-tree comment claimed upstream emits the referring column as varchar(36) via getType's field.references arm. It does not. Measured: varchar(191). That arm never runs for this column — resourceId participates in table-level indexes, so getType receives a tableIndexStringLength argument, which takes precedence over every field.* arm, and getDatabaseIndexStringLength seeds its reduce at MySQL's 191-character default and can only shrink from there.

That 191 is an artifact of upstream's index budget on upstream's own physical schema, and is deliberately not inherited: ObjectStack emits its own schema. Taking the referent's 255 is also what keeps the pair symmetric. (Upstream's own 255/191 split means a native better-auth deployment has a (191, 255] band it can register but never link — an upstream defect, noted in the field comment, not reproduced here.)

Hash-shadow route — measured entry and exit, not predicted

The ruling requires this be stated rather than discovered from a migration diff. sys_oauth_resource was synced onto live MySQL by the real driver at each declaration, and the physical facts read from information_schema as their own query:

before (1024) after (255)
identifier physical text (65535 octets) varchar(255) (1020 octets)
shadow column uniq_sys_oauth_resource_identifier__hash varbinary(32) present absent
UNIQUE index keys on the shadow column identifier directly, SUB_PART NULL

255 × 4 = 1020 bytes sits under the 3072-byte key-part ceiling, so the object leaves the #11627/#12198 hash shadow, as the ruling anticipated. Uniqueness is unchanged and still enforced over the full value — the replacement is a direct full-value UNIQUE, not a prefix index. The referrer was measured too: resource_id is now varchar(255)/1020 octets with its non-unique index direct and SUB_PART NULL.

Clause ② — yes

An accept/reject narrowing on a published object, in both directions of a referring pair. git diff --stat behind it:

 .changeset/oauth-resource-identifier-sourced-255.md |  66 ++++++++++++
 .../identity/sys-oauth-client-resource.object.ts    |  62 +++++++-----
 .../sys-oauth-resource-sourced-bounds.test.ts       | 111 +++++++++++++++++++++
 .../src/identity/sys-oauth-resource.object.ts       |  28 +++++-
 .../src/platform-keyed-text-bounds.test.ts          |  18 +++-
 5 files changed, 255 insertions(+), 30 deletions(-)

Precisely what the narrowing rejects that today's tree accepts: values in (255, 768] move from "the referrer accepts" to "both refuse". Values in (768, 1024] were already refused by the referrer and are now refused by the referent too. Nothing upstream can emit either band.

Tests

New pin sys-oauth-resource-sourced-bounds.test.ts (4 tests). The third is the one that matters: it asserts the referrer admits exactly the referent's domain, because any gap between them is a register-then-never-authorize dead end by construction. Pinning two integers pins two integers; pinning the invariant is what a future re-narrowing has to stay honest against.

Ablations — direction and exact failure count predicted in writing first, mutation proved on disk with anchored grep -cF counts before any result was read, restored under trap … EXIT INT TERM, restore verified with an empty git diff. No rebuild needed, justified by import form: the pin imports ./sys-oauth-resource.object — a relative path inside the same package, which vitest resolves to src/*.ts, never through the package exports map or dist/.

ablation predicted observed
referent reverted to 1024 3 failed / 8 passed 3 failed / 8 passed — exactly the three named tests
referrer reverted to 768 1 failed / 10 passed 1 failed / 10 passed — only the dead-end invariant

No existing assertion was changed. platform-keyed-text-bounds.test.ts carries comment-only edits recording that #11701's 768 is now 255; its assertions are untouched and still pass (255 ≤ 768, so the column stays in its enumerated set).

Gates

Every exit captured before any pipe; each verdict quoted from the gate's own output, never from a wrapper's $? (the verify-lock wrapper printed 0 for a batch in which two gates exited 1 — caught exactly this way).

Green (own verdict lines): check:nul-bytes, check:changeset-gate-self-tests, check:cross-package-test-inputs (+ the ci.yml variant), check:objectql-double-limit, check:objectui-changeset, check:page-declaration-shape, check:published-files, check:slot-lookup, check:test-source-alias, check:type-source-resolution, check-changeset-no-major, check-ci-filter-parity, check-comment-mask-adoption, check-empty-changeset, check-plugin-teardown-shape, release-rehearsal-clone --self-test, check:query-options-erasure, check:engine-double-contract, check:where-matcher, check:i18n-stale-fill, check:type-check-coverage, check:i18n, plus @objectstack/platform-objects typecheck and test (32 files / 519 tests).

Two needed real work rather than a green tick:

  • check-adr-0087-registration exited 1 — a genuine red. The changeset declares BREAKING and carried no ledger disposition. Answered with not-required (no-migration-prescription); the gate now reports "1 declared-breaking changeset(s), each carrying an ADR-0087 disposition" and accepts the exemption (its prescription detector does not refuse the body).
  • check:i18n exited 1 saying "PREREQUISITE NOT MET … Nothing was checked"not measured, not red. The CLI was built and it was re-run for a real result: "check-i18n-bundles: OK (9 package(s) — all bundles in sync, no undeclared authoring keys)."

check:type-check-debt --re-measure was narrowed, and the narrowing is a measurement. platform-objects hides its tests from tsc and carries TEST_DEBT = 3, so the ratchet is load-bearing for the new test file. Rather than rebuild the whole workspace closure, the entry was re-measured the way the gate does — the package's own config with the test exclusion lifted — with the exclusion-lift confirmed by a positive control (32 test files in the program). Result: 3 raw errors, TS2339 ×2 + TS7006 ×1, matching the ledger to the unit, all in the pre-existing feature-gate-guard.test.ts and 0 in the new file. The ratchet cannot move for this card.

Gate union derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack over the real changeset (merge-base semantics, 5 paths). The union was re-run on the final head 3fda8cb404 for the changeset family — the only family the last commit could move.


Generated by Claude Code

claude added 2 commits August 26, 2026 14:42
…rom its producer — 1024 -> 255, and the referring column with it

sys_oauth_resource.identifier declared maxLength: 1024 citing no producer.
better-auth 1.7.1 — the sole writer (managedBy: 'better-auth',
protection.lock: 'full') — emits it as varchar(255) on MySQL, measured by
running its own migration generator against live MySQL 8.0.46 and reading
information_schema.COLUMNS as its own query.

Both halves of the foreign key now declare 255, closing the silent
register-then-never-authorize dead end: on PG/SQLite a 900-char identifier
could be registered by the referent and then never granted, because the
referrer (narrowed to 768 by #11701) refused it.

Measured consequence: 255 x 4 = 1020 bytes is under the 3072-byte key-part
ceiling, so sys_oauth_resource LEAVES the hash-shadow route — the
varbinary(32) shadow column is gone and the UNIQUE index is carried directly
on identifier with SUB_PART NULL.

Also corrects the #11701 citation: upstream emits the referring column as
varchar(191), not varchar(36) — the field.references arm never runs because
resourceId participates in table-level indexes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W6HFzyH98W1YaQXhJUJt6o
…und narrowing

no-migration-prescription: the change narrows two field bounds on objects whose
protection.lock is 'full'. No metadata key is removed or renamed, so no authored
metadata can name the discarded band, and schema sync applies the physical
column change itself.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W6HFzyH98W1YaQXhJUJt6o

Copy link
Copy Markdown
Collaborator Author

PM review — accepted, flipped ready, auto-merge armed. Two things need the maintainer's eye and neither blocks the merge.

Measured: 5 files, +255/−30, all in-scope. Verified independently: the ADR-0087 disposition is present in the changeset (not-required (no-migration-prescription), with its reasoning), and the hash-shadow outcome is stated there "measured rather than predicted" — both of which the ruling required.

⭐ The precondition came back clean, and the dev refused to let that stand unqualified

This is the part worth reading twice. The stock reading found zero over-255 values — with a positive control firing in the same scan: a fifth schema cloned from the real table shapes and planted with 256/300/900/1024-character identifiers, then the identical query run over all five in one pass. It found 4 planted over-255 identifiers and 1 planted resource_id, and 0 in the four real schemas, correctly excluding a planted short value. A long value would have been found had one existed.

And then:

CLEAN — but VACUOUSLY so, and the distinction is load-bearing. All eight real tables hold ZERO rows.

A clean reading over a population of zero is not the same evidence as a clean reading over a populated corpus, and it does not disprove the self-hosted-bypass possibility the precondition exists for — no customer or production deployment is reachable from this container. Postgres was measured absent (pg_isready no response, TCP refused) and no SQLite ObjectStack database exists on disk, so MySQL genuinely was the whole reachable population. Enumerating every engine rather than reporting the convenient one, and reading row counts alongside max-length so an empty table could not be mistaken for a clean corpus, is what makes the vacuity visible instead of invisible.

Merge posture: the ruling said "a hit forks back; a clean reading proceeds". The reading was clean, so by the ruling's letter this proceeds, and it is armed. But the caveat is real and it is yours:

  • A (the dev's recommendation, and mine): treat the precondition as discharged. The bypass risk requires a writer other than better-auth, and both objects are protection.lock: 'full' with no supported path writing them; the sole writer stores the identifier in varchar(255), so no better-auth-managed row can exceed 255 regardless of stock.
  • B: hold until someone runs the same scan against a real deployment. The scan is a single portable query, reproduced verbatim in the PR body.

If you want B, disarm auto-merge — I have not treated an empty scan as if it were a populated one, and neither did the dev.

⚠️ The ruling's citation was partly wrong, and the dev implemented the number while correcting the rationale

The ruling sourced both columns to "get-migration.mjs's varchar branch". Measured firsthand against better-auth 1.7.1: that is exactly right for the referent (varchar(255)) and wrong for the referrer — upstream emits oauthClientResource.resourceId as varchar(191), which is neither the ruling's implied 255 nor the varchar(36) the in-tree #11701 comment claimed.

The dev implemented the ruling's number (255) via the referring-column derivation already established for client_id — independently confirmed by measuring oauthClient.clientId = varchar(255) — rather than copying upstream's 191, which is an artifact of upstream's own index budget. That is the right call: 191 would reopen a dead end at (191, 255] inside ObjectStack for no gain, importing an upstream defect we do not have. Flagged so the ruling's rationale and the implementation stay reconciled in the record rather than silently diverging.

The in-tree varchar(36) miscitation was fixed in place, under the bounded-exemption test and named in both the PR body and the changeset: same defect class as this card (a miscited bound on this exact field pair), mechanical, in a file this card already rewrites, same gate family. Correct handling — and I confirmed the removal in the diff.

Also correctly not filed: upstream better-auth 1.7.1 has its own (191, 255] register-but-never-link band. Real, but with no ObjectStack-side consequence since we emit our own schema at 255/255. Documented in the field comment rather than minted as an issue against someone else's repo.

Hash shadow — measured entry and exit

BEFORE (1024): identifier = text (65535 octets); uniq_…__hash varbinary(32) PRESENT; UNIQUE on the shadow
AFTER  (255) : identifier = varchar(255) (1020 octets); NO __hash column; UNIQUE direct, SUB_PART NULL

Read from information_schema as its own query, never inferred from emitted DDL. The object leaves the shadow exactly as the ruling anticipated, and — the part that matters — uniqueness is unchanged and still enforced over the full value: a direct full-value UNIQUE, never a prefix index.

⚠️ Third wrapper lie today, and the first that hid a genuine red

the verify-lock wrapper printed VERDICT command-exit 0 for a batch in which check-adr-0087-registration and check:i18n had each exited 1.

The first was a real failure — a breaking changeset with no ADR-0087 disposition. Had the wrapper's zero been trusted, this PR would have gone to CI carrying a genuine red. The second was PREREQUISITE NOT MET … Nothing was checked, correctly recorded as not measured rather than red, then resolved by building the CLI and re-run for a real verdict. Cross-posting to #12365: the two earlier instances today cost a re-run; this one would have cost a red CI cycle.

Ablations

Two legs, both matching their written predictions exactly (3/8 and 1/10). Leg B is the one that earns its keep: reverting only the referrer reds one test — the dead-end invariant — proving that assertion catches the asymmetry on its own rather than riding on the referent-width assertion. The pin asserts the referent/referrer symmetry as an invariant instead of pinning two integers, which is why it survives the next legitimate re-bounding.

No existing assertions changed; the neighbouring bounds test got comment-only edits and still passes on its own terms.


Generated by Claude Code

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sys_oauth_resource.identifier declares an uncited maxLength: 1024, and now over-declares relative to every column that references it

2 participants