Skip to content

Five more instances of the #8323 class: admin- and user-authored names on tenant-scoped objects still carry installation-wide unique indexes #8554

Description

@os-zhuang

Found while implementing #8468 (the third instance), running the real lint rule over every platform object rather than reading declarations by eye. Not fixed there — #8468's scope is sys_position alone. Filed unassigned.

Per the maintainer ruling of 2026-08-13 on #8468, an instance of this shape inherits that answer and goes straight to the lane queue rather than back to the decision box. None of the five below carries a semantic argument for an installation-wide namespace; where one might exist I have filed separately instead (see "Related").

Why this contradicts the "closed set" claim on #8468

The #8468 triage comment stated that the sweep behind it had bounded the rest of the platform's objects, so "whatever is ruled here closes the class for the platform's own metadata rather than leaving a tail." That is not the case. The original sweep was a source read; this one executes lintUnscopedDeclaredIndexes from packages/lint/src/data-model-rules.ts over all 76 loadable *.object.ts definitions and cross-references each finding against resolveInjectedSystemColumns — i.e. the rule's own verdict, filtered by whether organization_id is actually injected.

The rule fires on the spelling alone and deliberately does no tenancy inference, so its raw output includes many legitimately-global better-auth objects. The tenancy filter is what separates them.

The five instances

All are tenant-scoped (organization_id injected, no tenancy opt-out) and declare a bare unique: true on a declared index over authored content — the positional spelling of 'global', i.e. the listed columns verbatim.

object package declared index managedBy authored by
sys_permission_set plugin-security { fields: ['name'], unique: true } config admins, in Setup
sys_sharing_rule plugin-sharing { fields: ['name'], unique: true } config admins, via the Studio criteria builder
sys_webhook plugin-webhooks { fields: ['name'], unique: true } config admins, from the UI
sys_email_template platform-objects { fields: ['name', 'locale'], unique: true } config admins
sys_notification_preference service-messaging { fields: ['user_id', 'topic', 'channel'], unique: true } system-data per-user toggles

sys_permission_set is the strongest case and the most awkward one: it sits in the same directory as the two objects already fixed, and together with sys_capability and sys_position it is the third leg of the ADR-0090 RBAC triad. Its own header comment reads "tenants may add custom rows (created via UI / API) but the schema itself is locked" — the same sentence that made sys_position a defect. Two organizations cannot both have a permission set named sales_readonly.

sys_notification_preference is the near-exact analogue of the already-ruled sys_user_preference: same archetype (a per-user K/V row), same defect shape. Note that sys_user_preference is managedBy: 'system-data' too, so managedBy is not the discriminator — the ruling's phrase is admin-authored content, which is about provenance of the rows, not the management mode of the object.

Expected consequence

The same one measured on #8468 and #8323: a per-value 409 on a row the caller cannot read (a cross-tenant existence oracle over another organization's naming), plus a functional dead end — the second organization simply cannot use the name and the refusal does not say why. For sys_notification_preference the shape matches sys_user_preference's measured symptom instead: a user in two organizations cannot hold independent per-topic toggles.

⚠️ Not yet measured live per object. #8468 established that the static read is reliable for this shape by reproducing the oracle on a real engine, but the ruling's own discipline is that the probe comes first. Whoever takes this should run the 409/201 probe per object before migrating, exactly as #8468 did.

Fix shape (unchanged from #8461 / #8468, and free)

  1. Respell each to unique: 'organization'.
  2. The replace_unique_index migration arm generalized to declared indexes by fix(platform-objects,plugin-security,driver-sql): scope sys_user_preference and sys_capability uniqueness per organization (#8323) #8461 covers these unchanged — verified on sys_position in sys_position.name is the third instance of the #8323 class: an admin-authored name on a tenant-scoped RBAC object carries an installation-wide unique index #8468 with no modification to schema-drift.ts.
  3. Check each object's published text for a uniqueness claim, as sys_position.name is the third instance of the #8323 class: an admin-authored name on a tenant-scoped RBAC object carries an installation-wide unique index #8468 had to (describe() plus the generated reference page).
  4. ⚠️ The pin must exercise the deployed-installation migration path, not only a fresh database. Respelling changes the index's generated name, so on a deployed database drift otherwise reads as two findings — composite missing (safe) and old global index orphaned (destructive) — and an operator applying only the safe half keeps the defect while the plan reads as applied.

Related

Found by session session_012WMpuAfA2KSdDjGF6tm1bH.


Generated by Claude Code

Activity

  1. added theissue type on Aug 13, 2026
  2. hotlong commented on Aug 13, 2026

    @hotlong
    Contributor

    Triage (triage seat Routine, 2026-08-13 ~20:10Z): confirmed as filed — pm:queue + type Bug set (the shape inherits the maintainer's 2026-08-13 ruling on #8468: it violates the declared tenancy contract, no fresh decision needed).

    Routing — cross-domain exception path. The five objects span four packages in three lanes: plugin-security + plugin-sharing (identity), plugin-webhooks + service-messaging (services), platform-objects (metadata). Splitting five identical one-line respellings plus one shared migration-pin discipline into three lane cards would triple the overhead of a mechanical batch, so this stays one card under the sweep-batch rules. Designated claiming lane: domain:metadata — it holds the class lineage (#8323 → #8461 → #8468), the replace_unique_index arm knowledge, and the probe discipline. Conditions of the exception path apply: the claim comment must declare the full file surface (all four packages), and the claimer runs the targeted in-flight check against the identity and services lanes' open claims before starting.

    Serial constraints for whoever claims:

    Correction accepted, on the record: the #8468 triage comment claimed the sweep behind it had closed the class for the platform's own metadata. This card's executed-lint sweep (rule verdict × tenancy filter over all 76 loadable objects) proves that claim wrong — the earlier sweep was a source read, not a rule execution. The closed-set assertion is retracted; this card plus #8555's explicitly-triaged negative list is now the bounded set, and it is measurement-backed.

    Dispatch expectations carry over from the ruling unchanged: live 409/201 probe per object first; the deployed-installation migration path is the load-bearing pin (per this card's ⚠️ on the orphaned-index split-plan hazard); check each object's published text for uniqueness claims.


    Generated by Claude Code

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

    @os-zhuang
    ContributorAuthor

    Claim

    Claiming this card for implementation, under the cross-domain exception path the triage attached.

    Full file surface — all four packages, as the exception path requires

    package file declared index today
    plugin-security packages/plugins/plugin-security/src/objects/sys-permission-set.object.ts { fields: ['name'], unique: true }
    plugin-sharing packages/plugins/plugin-sharing/src/objects/sys-sharing-rule.object.ts { fields: ['name'], unique: true }
    plugin-webhooks packages/plugins/plugin-webhooks/src/sys-webhook.object.ts { fields: ['name'], unique: true }
    platform-objects packages/platform-objects/src/audit/sys-email-template.object.ts { fields: ['name', 'locale'], unique: true }
    service-messaging packages/services/service-messaging/src/objects/notification-preference.object.ts { fields: ['user_id', 'topic', 'channel'], unique: true }

    Plus, expected but not yet certain: per-package declaration pins beside each object, one driver-level suite in packages/drivers/driver-sql/ for the live probe and the deployed-installation migration, published-text sources whose describe() / description / helpText carry a uniqueness claim (A3, to be surveyed per object), any content/docs/references/** page regenerated from those sources, and a changeset.

    Note the card's table says sys_webhook is in plugin-webhooks, and it is — packages/plugins/plugin-webhooks/src/sys-webhook.object.ts. There is no platform-objects/src/system/sys-webhook.object.ts on current main; an older serial note (#8081, 2026-08-12) referenced that path.

    Serial checks, run at claim time

    Order of work

    Per R2 the probe comes first, per object, with the prediction recorded before each run — the ruling's premise was established for sys_position, not for these five. If any object's probe contradicts the static read I will stop and report the fork here rather than migrating it or switching spelling.

    ⚠️ One thing already measured, before any edit, because it bears on the PM's assumption A1: sys_notification_preference's replacement index name would be uniq_sys_notification_preference_organization_id_user_id_topic_channel — 70 characters, past INDEX_NAME_MAX = 60 — so buildIndexName hash-suffixes it while the 54-character legacy name is emitted verbatim. The two composite cases are exactly where a single-column assumption would hide, and this one also lands on the name-truncation path. Reporting the measured behaviour of the replace_unique_index arm per object.


    Generated by Claude Code

  5. os-zhuang commented on Aug 14, 2026

    @os-zhuang
    ContributorAuthor
    {
      "issue": 8554,
      "status": "done",
      "branch": "claude/issue-8554-unscoped-unique-indexes",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/8599",
      "premise_still_valid": true,
      "summary": "PROBE FIRST, PER OBJECT, AND ALL FIVE CONFIRMED THE PREMISE — no fork to report. Predictions recorded before each run; measured 201 / 409 UNIQUE_VIOLATION / 201 control / GET 0 on every one, on a real SqlDriver under OS_TENANCY_POSTURE=isolated, driving the REAL shipped declarations imported from source with organization_id derived from resolveInjectedSystemColumns rather than hand-copied. Respelled all five to unique:'organization', spelled explicitly; corrected every published uniqueness claim at its source and regenerated three reference pages (permission.mdx, webhook.mdx, and connector.mdx which embeds the same webhook schema — the third was unexpected and is correct). schema-drift.ts is UNTOUCHED and proven byte-identical to origin/main by object hash 3a16c6f5e297d81eb63b07781664c6dbddf2a1fe. THE CARD'S BOUNDED-SET CLAIM IS FALSE: its stated method (rule verdict x tenancy filter) yields 20 tenant-scoped findings, not 6 — an unstated judgement step about authored names was applied on top, and that step dropped two more clean inheritors, filed as #8577 after probing them live. One of those, sys_audience_binding_suggestion, is worse than a naming oracle: its key is the same triple for every tenant installing the same package, so the second and every later organization to install a package silently never gets its binding-suggestion row. Also measured: #8543 is live and already bit the merged #8556 — editing a field description does not update the translation bundles, so sys_position's en bundle on main still says 'Unique machine name for the position'.",
      "tests": "NEW: driver-sql/src/sql-driver-tenant-scoped-declared-unique.test.ts 85 tests covering all five (materialized shape, the live oracle, the deployed-installation migration, the #8461 guards, posture independence, the name-truncation boundary); five per-package declaration pins, 25 tests. FULL SUITES ON THE MERGED TREE, all green: @objectstack/spec 10520/397 files; plugin-security 1084/56; plugin-sharing 574/22; plugin-webhooks 86/7; platform-objects 362/19; service-messaging 224/21; driver-sql 1565 passed + 52 skipped/97. Typecheck green on all seven. ABLATION A (revert only the five declarations to bare true) — predicted 'each package pin RED 3 of 5 (15 total), driver suite GREEN 85/85'; measured exactly that. The driver suite staying green is the finding, not a miss: it carries its own copied fixtures, which is why each package pin asserts 'matches the fixture the driver suite copies'. As on #8556 the test named 'declares exactly one unique index' stayed GREEN under ablation because true is truthy — the scope assertion is toBe('organization'), never a truthiness check. ABLATION B (revert only #8461's declared-index arm in schema-drift.ts) — predicted '26 red / 59 green: fresh-database sections stay green, the deployed-installation block goes red, decisive assertion answers 409 where 201 expected, and the three negative guards stay green FOR THE WRONG REASON since they assert emptiness'; measured exactly 26 red / 59 green with 'after applying, BOTH halves hold on the MIGRATED database: expected 409 to be 201'. That is the #8323 finding reproduced for all five: with the arm ablated a FULLY APPLIED non-destructive migration still answers 409 cross-organization, while every fresh-database test stays green. Both ablations used commit-then-revert; never git stash. HARNESS NON-VACUITY: a named guard per object asserts the seeded database really carries the pre-fix index, holds 3 rows, and that the defect is live on it; anti-vacuity twins throughout (every 409-to-201 has a same-organization duplicate that must STILL be refused, plus an organization-less pair unique among themselves); the two composites carry a trailing-column control that is 201 before and after, which is what proves the key was the composite rather than its leading column. One harness bug found and fixed honestly: seeding the platform row with the same key as the tenant row threw a raw UNIQUE violation and took 30 tests red — under the pre-fix global index those rows genuinely collide, which is the defect, so the fixture now carries a distinct platformKey. GATES: all 23 derived gates green on the merged tree — nul-bytes, changeset-gate-self-tests, cross-package-test-inputs, docs-audit-scope, merge-driver, objectui-changeset, quick-reference-counts, role-word, spec-parsed-alias, test-source-alias, type-source-resolution, query-options-erasure, i18n, doc-formula-expressions, dev-prereqs, adr-0087-registration, changeset-no-major, empty-changeset, authorable-surface, docs, generated, api-surface, export-origins. ONE GATE ACTUALLY FAILED: check:query-options-erasure grew the test surface 240 to 245 via '{} as any' on query options; fixed AT THE SOURCE by removing the casts (find/count accept {}), back to 240 at the ceiling — no ledger grown anywhere. check:type-check-debt's --re-measure half first REFUSED to run, naming platform-objects and service-realtime as having type entry points older than their sources (#6376) — the gate working, not failing; built both directly and re-ran: 33 ledger entries, 1969 raw errors, none above its recorded number, surplus none. Gate list re-derived from actual changed paths with scripts/pm/dispatch-gates.mjs, which surfaced check:doc-formula-expressions, check:dev-prereqs, check:merge-driver, check:spec-parsed-alias, check:adr-0087-registration, check:changeset-no-major, check:empty-changeset and check:query-options-erasure that the dispatch prompt did not name — and the last of those was the one that failed. origin/main merged and the entire union plus every suite re-run after the final commit (#8550). Four gates I first recorded as FAIL were my own wrong invocation (authorable-surface / generated / api-surface / export-origins are spec-scoped, not root scripts); re-run correctly, all green — reporting that rather than the false reds.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #8577: TWO more clean inheritors of the ruled class, both PROBED LIVE rather than read — sys_notification_subscription [topic, principal] (the direct sibling of one of my five, same package and directory, same ADR-0030 layer, admin/user-authored from the Setup grid) and sys_audience_binding_suggestion [package_id, permission_set_name, anchor]. Labelled pm:queue per the ruling's standing consequence. ⚠️ The second is the most serious thing I found: the key is identical for every tenant that installs the same package, so the second and every later organization to install it silently never gets its binding-suggestion row and its admin is never asked to bind the package's default permission set — a functional dead end on the package-install path, live on main today and independent of this PR.",
        "filed as #8578: the #8554 triage's 'measurement-backed bounded set' is not bounded — the stated method yields 41 raw findings of which 20 are tenant-scoped, not 6. Records the full three-category triage (7 ADR-0120 S6 hand-written composites, 9 opaque-id/token/dedup keys that are globally unique by construction, and the defect class) so the next sweep need not re-derive it, plus sys_job as a second #8555-shaped judgement case (engine-owned; if the job catalogue is a property of the deployment the correct answer is the OPPOSITE fix, an explicit 'global'). Filed WITHOUT pm:queue with the semantic argument stated, exactly as the ruling requires for a non-inheriting instance.",
        "reported, not filed (another seat owns it): #8543 is live and already bit the merged #8556. `node scripts/check-i18n-bundles.mjs --write` printed 'regenerated' for all nine packages and changed NOTHING, and check:i18n reports 'all bundles in sync' either way — so an edited field description never reaches the shipped bundles. Consequence on main right now: #8556 corrected sys_position's source text but its en bundle still says 'Unique machine name for the position', i.e. the corrected source and the shipped English text disagree. I did not touch the generator (#8543 owns it) and did not touch sys_position (out of scope); for my own objects I followed the bundle header's own documented workflow ('Edit translations in place... only the leaf string values') and corrected all four shipped locales, without which this PR would have claimed to fix the published text while Setup still showed the old contract.",
        "observation, no issue filed: sys_notification_preference's replacement index name is HASH-SUFFIXED (uniq_sys_notification_preference_a22d7d27) because the natural name is 70 characters against INDEX_NAME_MAX=60. The existing arm handles it — measured — but if the legacy and replacement names had ever collapsed to one string the `legacyName === replacement.name` guard would have read the respelling as 'nothing was superseded' and emitted NO migration at all, with a fresh database still looking correct. Pinned as its own section rather than filed."
      ]
    }

    Generated by Claude Code

  6. os-zhuang commented on Aug 14, 2026

    @os-zhuang
    ContributorAuthor

    Landed — b45c71e8 on origin/main (PR #8599, squash-merged). Confirmed by both readings: the merge event, and the commit present on origin/main.

    Fourth act of the class, after #8461 (sys_user_preference, sys_capability) and #8556 (sys_position).

    The probe ran first, per object, and all five reproduced

    Predictions recorded before each run; measured 201 / 409 UNIQUE_VIOLATION / 201 control / GET 0 on every one — real SqlDriver under OS_TENANCY_POSTURE=isolated, driving the real shipped declarations imported from source with organization_id derived from resolveInjectedSystemColumns rather than typed in. No object contradicted the static read, so there was no fork to report.

    The two composite keys got a second control varying only a trailing column (locale; channel), accepted before and after — which is what proves the installation-wide key was the composite rather than its leading column.

    An honest process note from the PR worth preserving: sys_email_template's first probe answered 500 on every insert. That was the harness (a filler naming body instead of body_html), not the object. Fixed and re-run before any conclusion was drawn — "a 500 is not a 409 and I did not count it as one."

    ⚠️ This card's bounded-set claim was false, and that is the finding

    This card stated its method as rule verdict × tenancy filter, and the triage comment accepted it as a measurement-backed bounded set, retracting #8468's earlier "closed the class" claim in the process.

    Executed on origin/main: lintUnscopedDeclaredIndexes over 76 loadable definitions gives 41 raw findings, of which 20 are tenant-scoped — not 6. The stated method is not what produced the five-object table; an unstated human judgement about which keys count as authored names was applied on top, and that undeclared step is where instances were dropped.

    So the correction chain ran twice: #8468's triage claimed a closed set from a source read, this card's sweep disproved it — and this card's own claim was disproved the same way one act later. A method is measurement-backed only when every step of it is written down; "I ran the rule" and "I ran the rule and then filtered by judgement" are indistinguishable in a report.

    Chasing that down produced:

    The migration half, again

    Ablation B (reverting only #8461's declared-index arm) measured 26 red / 59 green with the decisive expected 409 to be 201: with that arm ablated, a fully applied non-destructive migration still answers 409 cross-organization, on all five. Every fresh-database test stayed green throughout.

    ⚠️ sys_notification_preference also lands on the name-truncation path — its natural replacement name is 70 characters against INDEX_NAME_MAX = 60, so it is hash-suffixed while the 54-character legacy name is emitted verbatim. The two therefore differ and the legacyName === replacement.name guard correctly does not fire. Had they ever collapsed to one string, the guard would have read the respelling as "nothing was superseded" and emitted no migration at all — declaration changed, fresh database correct, every deployed installation keeping the global index forever. Pinned.

    Gates

    check:query-options-erasure went red — the new suite grew the test surface 240 → 245 via {} as any on query options — and was repaired at the source by removing casts that were never needed, back to 240 at the ceiling. No ledger grown anywhere.

    The gate list was re-derived with dispatch-gates.mjs and surfaced eight gates the dispatch brief did not name — including the one that actually failed. Second time today the derivation caught what the brief missed; the brief's list is a floor, never a list.

    Residue

    #8601 — #8556 corrected sys_position's source text but its shipped en bundle still asserts the old claim on main, because editing a field description does not update the translation bundles (#8543) and check:i18n reports "in sync" either way.


    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

Labels

bugSomething isn't workingsecurity

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions