diff --git a/.changeset/tenant-scoped-declared-unique-indexes.md b/.changeset/tenant-scoped-declared-unique-indexes.md new file mode 100644 index 0000000000..4ae4db3d10 --- /dev/null +++ b/.changeset/tenant-scoped-declared-unique-indexes.md @@ -0,0 +1,98 @@ +--- +"@objectstack/plugin-security": patch +"@objectstack/plugin-sharing": patch +"@objectstack/plugin-webhooks": patch +"@objectstack/platform-objects": patch +"@objectstack/service-messaging": patch +"@objectstack/spec": patch +--- + +fix(plugin-security,plugin-sharing,plugin-webhooks,platform-objects,service-messaging,spec): five tenant-scoped declared unique indexes become per-organization (#8554) + +Five platform objects declared their uniqueness as a table-level index with bare +`unique: true`. At the DECLARED-index level that is the positional spelling of +`'global'` — the listed columns verbatim — so on a tenant-scoped object each +materialized an **installation-wide** unique index. (Field-level `unique: true` +means the opposite, per-organization, and has since #3696; `packages/lint` names +that divergence "the #4986 trap" and warns on it via +`unique/unscoped-declared-index`.) These are the fourth act of the class ruled on +2026-08-13, after `sys_user_preference` / `sys_capability` (#8461) and +`sys_position` (#8556). + +| object | package | was | now | +|---|---|---|---| +| `sys_permission_set` | `plugin-security` | `[name]` global | `[name]` per organization | +| `sys_sharing_rule` | `plugin-sharing` | `[name]` global | `[name]` per organization | +| `sys_webhook` | `plugin-webhooks` | `[name]` global | `[name]` per organization | +| `sys_email_template` | `platform-objects` | `[name, locale]` global | `[name, locale]` per organization | +| `sys_notification_preference` | `service-messaging` | `[user_id, topic, channel]` global | same, per organization | + +Measured live on a real engine before the fix — two organizations, the same key, +`OS_TENANCY_POSTURE=isolated`, driving the real shipped declarations. All five +reproduced identically: + +``` +org_jia POST the key → 201 +org_yi POST the SAME → 409 UNIQUE_VIOLATION +org_yi POST an unused → 201 ← the control that makes it an oracle +org_yi GET the key → total 0 ← refused by a row it cannot see +``` + +Two consequences, both removed. **A cross-tenant existence oracle:** the 409 is a +per-value answer about a row the caller cannot read, so an organization could +enumerate another organization's permission-set, sharing-rule, webhook and +template naming. **A functional dead end:** the second organization simply could +not use the name, and the refusal did not say why. For +`sys_notification_preference` the shape is the one #8323 measured on +`sys_user_preference` — a user belonging to two organizations could not hold +independent per-topic delivery toggles. + +## ⚠️ Operators: a migration is REQUIRED, and deploying this release is not it + +Respelling a declared index changes its generated **name**. On an existing +database `initObjects` is additive: it creates the new per-organization composite +at boot and **never drops the old global index**, which goes on enforcing. Until +the retirement is applied, a deployed installation that has taken this release is +still enumerable — that is asserted as a test, not assumed. + +Run the migration: + +``` +os migrate plan # shows one `replace_unique_index` per object, categorised `safe` +os migrate apply # no --allow-destructive needed +``` + +Each object plans as **one pure relaxation**, not as two findings. That matters: +if it read as "composite missing" (safe) plus "old global index orphaned" +(destructive, opt-in), an operator applying only the safe half would keep the +global index — keep the defect — while the plan read as applied. The `#8461` +`replace_unique_index` arm covers all five unchanged (no driver change in this +release), applies CREATE-before-DROP so uniqueness is never unenforced in +between, drops the legacy index only once the replacement is confirmed present, +preserves every row, and converges to no drift. + +Two columns are worth an operator's attention: + +- `sys_notification_preference`'s replacement index name is **hash-suffixed** — + `uniq_sys_notification_preference_a22d7d27` — because the natural name is 70 + characters and the limit is 60. That is expected, not corruption. +- Rows with no `organization_id` (platform/seed rows) stay unique **among + themselves**: the organization key part is NULL-safe + (`COALESCE(organization_id, '__global__')`, ADR-0120 D3), so seeding by name + keeps working and a tenant may hold its own row of the same name. + +## Not breaking + +A relaxation admits key pairs that were previously refused and refuses nothing +that previously succeeded, so no caller that worked before fails now. Every read +path for these five objects goes through the tenant-scoped data API, so no +consumer resolves one of these names across organizations expecting at most one +row. Shipped as `patch` for that reason — the same call #8556 made for the same +shape. + +Published text carrying the bare uniqueness claim was corrected at its source and +the generated reference pages regenerated (`security/permission.mdx`, +`automation/webhook.mdx`, and `integration/connector.mdx`, which embeds the same +webhook schema), together with the `sys_permission_set` field description, its +clone-dialog help text, the `sys_webhook` field description, and the matching +translation bundles in all four shipped locales. diff --git a/content/docs/automation/webhooks.mdx b/content/docs/automation/webhooks.mdx index c9bfad8606..81422245c8 100644 --- a/content/docs/automation/webhooks.mdx +++ b/content/docs/automation/webhooks.mdx @@ -93,7 +93,7 @@ its own encrypted column, see below. | Field | Type | Notes | |-------------------|-----------|------------------------------------------------------------------------------------| | `id` | text | Primary key. | -| `name` | text | Unique snake_case name — referenced in logs and audit. | +| `name` | text | snake_case name, unique **per organization** ([#8554](https://github.com/objectstack-ai/objectstack/issues/8554)) — referenced in logs and audit. | | `label` | text | Optional display label. | | `object_name` | text | Short object name whose record events fire this webhook. | | `triggers` | select | Multi-select of `create` / `update` / `delete` plus the opt-in bulk pair `bulk_update` / `bulk_delete` ([see below](#bulk-writes-bulk_update-and-bulk_delete)), stored as an array (the enqueuer also accepts a legacy comma-separated string). | diff --git a/content/docs/references/automation/webhook.mdx b/content/docs/references/automation/webhook.mdx index 4612018d36..7209fc811a 100644 --- a/content/docs/references/automation/webhook.mdx +++ b/content/docs/references/automation/webhook.mdx @@ -58,7 +58,7 @@ const result = WebhookSchema.parse(data); | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | -| **name** | `string` | ✅ | Webhook unique name (lowercase snake_case) | +| **name** | `string` | ✅ | Webhook name, unique per organization (lowercase snake_case) | | **label** | `string` | optional | Human-readable webhook label | | **object** | `string` | optional | Object whose record events (create/update/delete, bulk_update/bulk_delete) trigger this webhook | | **triggers** | `Enum<'create' \| 'update' \| 'delete' \| 'bulk_update' \| 'bulk_delete'>[]` | optional | Events that trigger execution | diff --git a/content/docs/references/integration/connector.mdx b/content/docs/references/integration/connector.mdx index b9ba752fd3..a7c5243bca 100644 --- a/content/docs/references/integration/connector.mdx +++ b/content/docs/references/integration/connector.mdx @@ -610,7 +610,7 @@ Synchronization strategy | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | -| **name** | `string` | ✅ | Webhook unique name (lowercase snake_case) | +| **name** | `string` | ✅ | Webhook name, unique per organization (lowercase snake_case) | | **label** | `string` | optional | Human-readable webhook label | | **object** | `string` | optional | Object whose record events (create/update/delete, bulk_update/bulk_delete) trigger this webhook | | **triggers** | `Enum<'create' \| 'update' \| 'delete' \| 'bulk_update' \| 'bulk_delete'>[]` | optional | Events that trigger execution | diff --git a/content/docs/references/security/permission.mdx b/content/docs/references/security/permission.mdx index af1fdf9e9e..9c177b124a 100644 --- a/content/docs/references/security/permission.mdx +++ b/content/docs/references/security/permission.mdx @@ -121,7 +121,7 @@ const result = AdminScopeSchema.parse(data); | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | -| **name** | `string` | ✅ | Permission set unique name (lowercase snake_case) | +| **name** | `string` | ✅ | Permission set name, unique per organization (lowercase snake_case) | | **label** | `string` | optional | Display label | | **description** | `string` | optional | Human-readable description shown in Setup (persisted as sys_permission_set.description) | | **packageId** | `string` | optional | [ADR-0086 D3] Owning package id for a package-shipped set (absent = env-authored) | diff --git a/packages/drivers/driver-sql/src/sql-driver-tenant-scoped-declared-unique.test.ts b/packages/drivers/driver-sql/src/sql-driver-tenant-scoped-declared-unique.test.ts new file mode 100644 index 0000000000..3fb9824742 --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-tenant-scoped-declared-unique.test.ts @@ -0,0 +1,723 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, afterEach, beforeEach, vi } from 'vitest'; +import { isUniqueViolationError } from '@objectstack/types'; +import { SqlDriver, classifyIndexKeyPart, parseIndexDdl, legacyUniqueReplacements } from '../src/index.js'; + +/** + * #8554 — the FOURTH act of the #8323 class: five more tenant-scoped objects + * whose declared unique indexes were installation-wide. + * + * ## What was measured here, live, before the fix — per object + * + * The card was filed from an executed lint sweep but was explicit that no live + * probe had been run against any of the five, and the maintainer ruling of + * 2026-08-13 makes the probe the required first step. It reproduced on all five, + * exactly as predicted, driving the REAL shipped declarations through this + * driver on `OS_TENANCY_POSTURE=isolated`: + * + * ``` + * sys_permission_set uniq_sys_permission_set_name (name) + * sys_sharing_rule uniq_sys_sharing_rule_name (name) + * sys_webhook uniq_sys_webhook_name (name) + * sys_email_template uniq_sys_email_template_name_locale (name, locale) + * sys_notification_preference uniq_sys_notification_preference_… (user_id, topic, channel) + * + * org_jia POST the key → 201 + * org_yi POST the SAME → 409 UNIQUE_VIOLATION + * org_yi POST an unused → 201 ← the control that makes it an ORACLE + * org_yi GET the key → total 0 ← refused by a row it cannot see + * ``` + * + * A per-value refusal on a row the caller cannot read is a cross-tenant + * existence oracle. It is also a plain dead end: two organizations could not + * both name a permission set `sales_readonly`, and — the `sys_user_preference` + * symptom, inherited by its near-twin `sys_notification_preference` — a user in + * two organizations could not hold independent per-topic toggles. + * + * ## Why the declarations were global + * + * The deliberate divergence #8323 documents: a DECLARED index's bare + * `unique: true` is the positional spelling of `'global'` and takes the listed + * columns VERBATIM, while FIELD-level `unique: true` has meant per-organization + * since #3696. `packages/lint/src/data-model-rules.ts` calls it "the #4986 trap". + * + * ⚠️ `managedBy` is NOT the discriminator. `sys_notification_preference` is + * `managedBy: 'system-data'`, and so is the already-ruled `sys_user_preference`. + * The ruling's phrase is ADMIN-AUTHORED CONTENT — the provenance of the rows, + * not the management mode of the object. + * + * ## Why this suite is at the DRIVER level + * + * The driver is the only layer that materializes a unique constraint and the + * only layer that can insert the violating row. `rest-server.ts` maps any error + * satisfying `isUniqueViolationError` to `409 UNIQUE_VIOLATION` and a successful + * create to `201`, so each case asserts the ENVELOPE the API would put on the + * wire — never a bare `.toThrow()`, which a driver throwing a plain `Error` + * would satisfy while REST answered 500. + * + * ## The half a fresh-database suite cannot see + * + * #8323's most expensive finding, re-measured for this card in Ablation B: + * respelling changes the index's generated NAME, so on a DEPLOYED database drift + * reads as two findings — composite missing (safe, auto-applied) and old global + * index orphaned (destructive, opt-in) — and an operator applying only the safe + * half keeps the defect while the plan reads as applied. Section 3 is therefore + * the load-bearing part of this file: it builds installations that ALREADY HAVE + * the old index and real rows, then migrates them. + */ + +/** The wire shape a duplicate insert must produce, per `rest-server.ts`. */ +const CONFLICT_ENVELOPE = { status: 409, code: 'UNIQUE_VIOLATION' } as const; + +async function createAsApi( + driver: SqlDriver, + object: string, + record: Record, +): Promise<{ status: number; code?: string; raw?: unknown }> { + try { + await driver.create(object, record as any); + return { status: 201 }; + } catch (error) { + if (isUniqueViolationError(error)) return { status: 409, code: 'UNIQUE_VIOLATION', raw: error }; + return { status: 500, code: 'INTERNAL_ERROR', raw: error }; + } +} + +/** + * One object under test. + * + * ⚠️ `preIndexes` / `fixedIndexes` are hand-copied from the shipped + * declarations, keeping the package boundary (the shape #8461 and #8556 used — + * `driver-sql` must not depend on five plugin packages). The copy is guarded in + * ONE direction only: each package carries a pin asserting its real + * `Xxx.indexes` against its own inline literal, so a change to a SHIPPED + * declaration that is not mirrored here goes red over there. The reverse is + * unguarded — edit a fixture below without touching its declaration and nothing + * compares them. Treat these as hand-maintained and change them only together + * with the declaration. + * + * The `unique: false` on the non-unique entries is not decoration: + * `ObjectSchema.create` normalizes an authored `{ fields: ['active'] }` into + * that shape, so this is what a driver is actually handed at registration. + */ +interface Subject { + table: string; + /** Physical columns the constraint and the rows need. */ + fields: Record; + preIndexes: Array>; + fixedIndexes: Array>; + /** The declared unique index's listed columns, in key order. */ + keyColumns: string[]; + /** The colliding value, column by column. */ + key: Record; + /** Differs from `key` in its FIRST column — the 201 control. */ + control: Record; + /** Differs from `key` in a NON-leading column. Composites only. */ + trailingControl?: Record; + /** + * A third distinct value, used for the organization-LESS (platform/seed) row. + * + * ⚠️ It must differ from `key`: the seeded database carries the PRE-FIX global + * index, under which an organization-less row and an organization's row + * sharing the key columns genuinely collide — that is the defect, not a + * harness accident. Seeding both with `key` made `seedDeployed` throw a raw + * UNIQUE violation and took 30 tests red at once. + */ + platformKey: Record; + /** Other columns a row needs to be insertable. */ + filler?: Record; + /** Legacy (pre-fix) index name, as `buildIndexName` emits it. */ + legacyName: string; + /** Replacement index name — hash-suffixed when the base passes 60 chars. */ + replacementName: string; +} + +const s = { type: 'string' }; +const b = { type: 'boolean' }; + +const SUBJECTS: Subject[] = [ + { + table: 'sys_permission_set', + fields: { id: s, organization_id: s, name: s, label: s, active: b, package_id: s }, + preIndexes: [ + { fields: ['name'], unique: true }, // ← the defect + { fields: ['active'], unique: false }, + { fields: ['package_id'], unique: false }, + ], + fixedIndexes: [ + { fields: ['name'], unique: 'organization' }, + { fields: ['active'], unique: false }, + { fields: ['package_id'], unique: false }, + ], + keyColumns: ['name'], + key: { name: 'sales_readonly' }, + control: { name: 'only_in_org_yi' }, + filler: { label: 'Sales Readonly' }, + platformKey: { name: 'platform_baseline' }, + legacyName: 'uniq_sys_permission_set_name', + replacementName: 'uniq_sys_permission_set_organization_id_name', + }, + { + table: 'sys_sharing_rule', + fields: { id: s, organization_id: s, name: s, label: s, object_name: s, active: b }, + preIndexes: [ + { fields: ['object_name', 'active'], unique: false }, + { fields: ['name'], unique: true }, + { fields: ['organization_id'], unique: false }, + ], + fixedIndexes: [ + { fields: ['object_name', 'active'], unique: false }, + { fields: ['name'], unique: 'organization' }, + { fields: ['organization_id'], unique: false }, + ], + keyColumns: ['name'], + key: { name: 'share_west_region' }, + control: { name: 'only_in_org_yi' }, + filler: { label: 'Share West Region', object_name: 'account' }, + platformKey: { name: 'platform_baseline_rule' }, + legacyName: 'uniq_sys_sharing_rule_name', + replacementName: 'uniq_sys_sharing_rule_organization_id_name', + }, + { + table: 'sys_webhook', + fields: { id: s, organization_id: s, name: s, label: s, url: s, object_name: s, active: b }, + preIndexes: [ + { fields: ['name'], unique: true }, + { fields: ['object_name'], unique: false }, + { fields: ['active', 'object_name'], unique: false }, + ], + fixedIndexes: [ + { fields: ['name'], unique: 'organization' }, + { fields: ['object_name'], unique: false }, + { fields: ['active', 'object_name'], unique: false }, + ], + keyColumns: ['name'], + key: { name: 'order_created_hook' }, + control: { name: 'only_in_org_yi' }, + filler: { label: 'Order Created', url: 'https://example.test/hook' }, + platformKey: { name: 'platform_baseline_hook' }, + legacyName: 'uniq_sys_webhook_name', + replacementName: 'uniq_sys_webhook_organization_id_name', + }, + { + table: 'sys_email_template', + fields: { id: s, organization_id: s, name: s, locale: s, label: s, subject: s, body_html: s, category: s, active: b }, + preIndexes: [ + { fields: ['name', 'locale'], unique: true }, + { fields: ['category'], unique: false }, + { fields: ['active'], unique: false }, + ], + fixedIndexes: [ + { fields: ['name', 'locale'], unique: 'organization' }, + { fields: ['category'], unique: false }, + { fields: ['active'], unique: false }, + ], + keyColumns: ['name', 'locale'], + key: { name: 'welcome', locale: 'en-US' }, + control: { name: 'only_in_org_yi', locale: 'en-US' }, + // Varies the TRAILING column only: 201 even before the fix, because the + // installation-wide key was the composite rather than `name` alone. Without + // this, "the key is (name, locale)" would rest on the fixture's spelling. + trailingControl: { name: 'welcome', locale: 'zh-CN' }, + filler: { label: 'Welcome Email', category: 'auth', subject: 'Welcome', body_html: 'Hello', active: true }, + platformKey: { name: 'platform_welcome', locale: 'en-US' }, + legacyName: 'uniq_sys_email_template_name_locale', + replacementName: 'uniq_sys_email_template_organization_id_name_locale', + }, + { + table: 'sys_notification_preference', + fields: { id: s, organization_id: s, user_id: s, topic: s, channel: s, enabled: b }, + preIndexes: [ + { fields: ['user_id', 'topic', 'channel'], unique: true }, + { fields: ['topic'], unique: false }, + ], + fixedIndexes: [ + { fields: ['user_id', 'topic', 'channel'], unique: 'organization' }, + { fields: ['topic'], unique: false }, + ], + keyColumns: ['user_id', 'topic', 'channel'], + key: { user_id: 'user_u1', topic: 'billing.invoice', channel: 'email' }, + control: { user_id: 'user_only_yi', topic: 'billing.invoice', channel: 'email' }, + trailingControl: { user_id: 'user_u1', topic: 'billing.invoice', channel: 'push' }, + filler: {}, + platformKey: { user_id: '*', topic: 'billing.invoice', channel: 'email' }, + legacyName: 'uniq_sys_notification_preference_user_id_topic_channel', + // ⚠️ HASH-SUFFIXED, and this is the case the PM's A1 flagged as the one + // where a single-column assumption would hide. The base name + // `uniq_sys_notification_preference_organization_id_user_id_topic_channel` + // is 70 characters, past `INDEX_NAME_MAX = 60`, so `buildIndexName` + // truncates and appends a sha1 prefix. The 54-character LEGACY name is + // emitted verbatim, so the two differ and the `legacyName === replacement.name` + // guard correctly does not fire. Pinned literally rather than recomputed: + // recomputing it with the same helper the code uses would assert nothing. + replacementName: 'uniq_sys_notification_preference_a22d7d27', + }, +]; + +describe('#8554 — five tenant-scoped declared unique indexes become per-organization', () => { + let driver: SqlDriver | undefined; + let savedPosture: string | undefined; + let savedMultiOrg: string | undefined; + + const makeDriver = () => { + const d = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + (d as any).logger = { warn: vi.fn(), info: vi.fn(), error: vi.fn(), debug: vi.fn() }; + driver = d; + return d; + }; + + const app = (sub: Subject, which: 'pre' | 'fixed') => [ + { name: sub.table, fields: sub.fields, indexes: which === 'pre' ? sub.preIndexes : sub.fixedIndexes }, + ]; + + beforeEach(() => { + savedPosture = process.env.OS_TENANCY_POSTURE; + savedMultiOrg = process.env.OS_MULTI_ORG_ENABLED; + // The posture the probe was run on. ADR-0120's invariant is that no index + // shape reads the posture — section 5 pins that — so this is context for the + // reader, not an input the assertions depend on. + process.env.OS_TENANCY_POSTURE = 'isolated'; + process.env.OS_MULTI_ORG_ENABLED = 'true'; + }); + + afterEach(async () => { + await driver?.disconnect(); + driver = undefined; + if (savedPosture === undefined) delete process.env.OS_TENANCY_POSTURE; + else process.env.OS_TENANCY_POSTURE = savedPosture; + if (savedMultiOrg === undefined) delete process.env.OS_MULTI_ORG_ENABLED; + else process.env.OS_MULTI_ORG_ENABLED = savedMultiOrg; + }); + + /** Unique index name → canonical key parts, COALESCE literal elided. */ + async function uniqueKeyParts(table: string): Promise> { + const k = (driver as any).knex; + const list: any = await k.raw(`PRAGMA index_list(${table})`); + const master: any = await k.raw( + `SELECT name, sql FROM sqlite_master WHERE type = 'index' AND tbl_name = ?`, + [table], + ); + const ddlByName = new Map(); + for (const r of Array.isArray(master) ? master : (master?.rows ?? [])) { + if (typeof r?.sql === 'string' && r.sql) ddlByName.set(r.name, r.sql); + } + const out: Record = {}; + for (const idx of list) { + if (idx.origin === 'pk' || idx.unique !== 1) continue; + const parsed = parseIndexDdl(ddlByName.get(idx.name) ?? ''); + if (parsed) { + out[idx.name] = parsed.keyParts.map((p) => { + const part = classifyIndexKeyPart(p); + if (part.kind === 'column') return part.column; + return part.column === null ? p : `COALESCE(${part.column})`; + }); + } else { + const info: any = await k.raw(`PRAGMA index_info("${idx.name}")`); + out[idx.name] = info.map((c: any) => c.name); + } + } + return out; + } + + const row = (sub: Subject, id: string, org: string | undefined, value: Record) => ({ + id, + ...(org ? { organization_id: org } : {}), + ...(sub.filler ?? {}), + ...value, + }); + + for (const sub of SUBJECTS) { + describe(sub.table, () => { + // ─────────────────────────────────────────────────────────────────── + // 1. Materialized shape — both spellings, kept side by side + // ─────────────────────────────────────────────────────────────────── + + it('the fixed declaration keys on the NULL-safe organization part', async () => { + const d = makeDriver(); + await d.initObjects(app(sub, 'fixed') as any); + + expect(await uniqueKeyParts(sub.table)).toEqual({ + [sub.replacementName]: ['COALESCE(organization_id)', ...sub.keyColumns], + }); + }); + + it('the pre-fix declaration keyed on the bare business columns — installation-wide', async () => { + // Kept permanently rather than measured once: this is the contrast that + // makes every "AFTER" assertion below mean something. + const d = makeDriver(); + await d.initObjects(app(sub, 'pre') as any); + + expect(await uniqueKeyParts(sub.table)).toEqual({ [sub.legacyName]: sub.keyColumns }); + }); + + // ─────────────────────────────────────────────────────────────────── + // 2. The card's reproduction — the live probe the ruling required + // ─────────────────────────────────────────────────────────────────── + + it('BEFORE: a key held by another organization is refused — 409 on a row you cannot read', async () => { + const d = makeDriver(); + await d.initObjects(app(sub, 'pre') as any); + + expect((await createAsApi(d, sub.table, row(sub, 'a', 'org_jia', sub.key))).status).toBe(201); + expect(await createAsApi(d, sub.table, row(sub, 'b', 'org_yi', sub.key))).toMatchObject( + CONFLICT_ENVELOPE, + ); + + // The control that makes the refusal an ORACLE rather than a blanket + // rejection: an unused value from the same caller is accepted, so the + // 409 is a per-value answer about another tenant's data. + expect((await createAsApi(d, sub.table, row(sub, 'c', 'org_yi', sub.control))).status).toBe(201); + + // …and the other half of the oracle: the caller's own read of the + // colliding key returns nothing. It is refused by a row it cannot see. + const visible = (await d.find(sub.table, {})).filter( + (r: any) => + r.organization_id === 'org_yi' && Object.entries(sub.key).every(([k, v]) => r[k] === v), + ); + expect(visible).toHaveLength(0); + }); + + it('AFTER: the same cross-organization create is accepted — 409 flips to 201', async () => { + const d = makeDriver(); + await d.initObjects(app(sub, 'fixed') as any); + + expect((await createAsApi(d, sub.table, row(sub, 'a', 'org_jia', sub.key))).status).toBe(201); + expect((await createAsApi(d, sub.table, row(sub, 'b', 'org_yi', sub.key))).status).toBe(201); + + const held = (await d.find(sub.table, {})).filter((r: any) => + Object.entries(sub.key).every(([k, v]) => r[k] === v), + ); + expect(held.map((r: any) => r.organization_id).sort()).toEqual(['org_jia', 'org_yi']); + }); + + it('AFTER (anti-vacuity): a SAME-organization duplicate is still refused', async () => { + // If this ever goes green-by-acceptance the constraint was REMOVED, not + // scoped — strictly worse than the defect being fixed, and + // indistinguishable from the fix by the 409-flips-to-201 assertion alone. + const d = makeDriver(); + await d.initObjects(app(sub, 'fixed') as any); + + expect((await createAsApi(d, sub.table, row(sub, 'a', 'org_jia', sub.key))).status).toBe(201); + expect(await createAsApi(d, sub.table, row(sub, 'b', 'org_jia', sub.key))).toMatchObject( + CONFLICT_ENVELOPE, + ); + expect(await d.count(sub.table, {})).toBe(1); + }); + + it('AFTER: rows with no organization stay unique among THEMSELVES (ADR-0120 D3)', async () => { + // Platform/seed rows carry no organization. A bare + // `(organization_id, …)` composite would be NULL-DISTINCT under SQL, so + // every seeded row could be duplicated at will; the NULL-safe key part + // is what prevents that, and seed upsert-by-name relies on it. + const d = makeDriver(); + await d.initObjects(app(sub, 'fixed') as any); + + expect((await createAsApi(d, sub.table, row(sub, 'seed1', undefined, sub.key))).status).toBe(201); + expect(await createAsApi(d, sub.table, row(sub, 'seed2', undefined, sub.key))).toMatchObject( + CONFLICT_ENVELOPE, + ); + + // A tenant may still hold its OWN row of the same key — the platform + // bucket and the organization buckets are separate namespaces. + expect((await createAsApi(d, sub.table, row(sub, 'own1', 'org_jia', sub.key))).status).toBe(201); + }); + + if (sub.trailingControl) { + it('the key is the whole COMPOSITE — varying a trailing column was always accepted', async () => { + // Guards the fixture's own claim about which columns the constraint + // spans. Green BEFORE and AFTER by design: it describes behaviour this + // card does not change, which is exactly what makes it a control. If + // the key were silently narrowed to the leading column this goes red. + const d = makeDriver(); + await d.initObjects(app(sub, 'pre') as any); + + expect((await createAsApi(d, sub.table, row(sub, 'a', 'org_jia', sub.key))).status).toBe(201); + expect( + (await createAsApi(d, sub.table, row(sub, 'b', 'org_yi', sub.trailingControl!))).status, + ).toBe(201); + }); + } + + // ─────────────────────────────────────────────────────────────────── + // 3. The deployed-installation half — the #8323 trap, per object + // ─────────────────────────────────────────────────────────────────── + + describe('migration on a database built before the respelling', () => { + /** + * An installation that ALREADY HAS the old global index and real rows — + * i.e. every deployment in the field. A fresh-provision test cannot + * reach any of the assertions in this block. + */ + const seedDeployed = async (d: SqlDriver) => { + await d.initObjects(app(sub, 'pre') as any); + await d.create(sub.table, row(sub, 'r1', 'org_jia', sub.key) as any); + await d.create(sub.table, row(sub, 'r2', 'org_jia', sub.control) as any); + await d.create(sub.table, row(sub, 'r3', undefined, sub.platformKey) as any); + }; + + it('the seeded database really carries the pre-fix index, and the defect is live on it (harness guard)', async () => { + // Without this the whole block could be exercising a fresh schema and + // every assertion below would still pass. Named as a guard on purpose. + const d = makeDriver(); + await seedDeployed(d); + + expect(await uniqueKeyParts(sub.table)).toEqual({ [sub.legacyName]: sub.keyColumns }); + expect(await d.count(sub.table, {})).toBe(3); + expect(await createAsApi(d, sub.table, row(sub, 'x', 'org_yi', sub.key))).toMatchObject( + CONFLICT_ENVELOPE, + ); + }); + + it('is planned as ONE pure relaxation, categorised safe — not a destructive orphan drop', async () => { + const d = makeDriver(); + await seedDeployed(d); + // The new metadata arrives (a deploy), same database. + await d.initObjects(app(sub, 'fixed') as any); + + const drift = await d.detectManagedDrift(); + expect(drift.filter((e) => e.table === sub.table)).toHaveLength(1); + const entry = drift.find( + (e) => e.table === sub.table && e.op.type === 'replace_unique_index', + ); + expect(entry, 'the respelling must be a replacement, not two unrelated findings').toBeDefined(); + expect(entry!.category).toBe('safe'); + expect(entry!.op).toMatchObject({ + dropIndexNames: [sub.legacyName], + createIndexName: sub.replacementName, + createColumns: ['organization_id', ...sub.keyColumns], + nullSafeColumns: ['organization_id'], + }); + + // ⛔ The old index must NOT ALSO surface as an orphan. An orphan drop + // is `destructive`, so an operator applying only the safe half would + // keep the global index — keep the defect — while the plan read as + // applied. That is the exact failure #8323 measured. + expect( + drift.filter( + (e) => e.op.type === 'drop_index' && (e.op as any).indexName === sub.legacyName, + ), + ).toHaveLength(0); + }); + + it('applies WITHOUT --allow-destructive, keeps every row, and converges', async () => { + const d = makeDriver(); + await seedDeployed(d); + await d.initObjects(app(sub, 'fixed') as any); + + const drift = await d.detectManagedDrift(); + const { applied, skipped } = await d.applyMigrationEntries(drift, { allowDestructive: false }); + expect(applied.some((e) => e.op.type === 'replace_unique_index')).toBe(true); + expect(skipped).toHaveLength(0); + + expect(await uniqueKeyParts(sub.table)).toEqual({ + [sub.replacementName]: ['COALESCE(organization_id)', ...sub.keyColumns], + }); + expect(await d.count(sub.table, {})).toBe(3); + + // Re-running finds nothing: the plan is not a drop/create cycle. + expect(await d.detectManagedDrift()).toHaveLength(0); + }); + + it('after applying, BOTH halves hold on the MIGRATED database', async () => { + // The assertion the card is actually about: the fix reaches a deployed + // installation, not merely a freshly provisioned one. + const d = makeDriver(); + await seedDeployed(d); + await d.initObjects(app(sub, 'fixed') as any); + await d.applyMigrationEntries(await d.detectManagedDrift(), { allowDestructive: false }); + + expect((await createAsApi(d, sub.table, row(sub, 'z1', 'org_yi', sub.key))).status).toBe(201); + + // The anti-vacuity arm, on the SAME migrated index. + expect(await createAsApi(d, sub.table, row(sub, 'z2', 'org_jia', sub.key))).toMatchObject( + CONFLICT_ENVELOPE, + ); + + // …and the organization-less bucket survived the migration intact: + // the seeded platform row still blocks a duplicate of ITSELF. + expect(await createAsApi(d, sub.table, row(sub, 'z3', undefined, sub.platformKey))).toMatchObject( + CONFLICT_ENVELOPE, + ); + }); + + it('boot creates the replacement ADDITIVELY, so the defect is STILL LIVE until the plan runs', async () => { + // `initObjects` is additive-only: it materializes the newly-declared + // composite at boot and never drops anything. So a deployed + // installation that has taken the new code but not run the plan is + // still enumerable — deploying the respelling is not, by itself, the + // fix. This is the sentence an operator needs, stated as an assertion. + const d = makeDriver(); + await seedDeployed(d); + await d.initObjects(app(sub, 'fixed') as any); + + expect(Object.keys(await uniqueKeyParts(sub.table)).sort()).toEqual( + [sub.legacyName, sub.replacementName].sort(), + ); + expect(await createAsApi(d, sub.table, row(sub, 'y', 'org_yi', sub.key))).toMatchObject( + CONFLICT_ENVELOPE, + ); + }); + + it('DROP happens only once the replacement is confirmed present', async () => { + // The safety argument in the direction that can actually go wrong: if + // the replacement is not there, the legacy index must be left alone + // rather than dropped into a gap with no uniqueness at all. + const d = makeDriver(); + await seedDeployed(d); + await d.initObjects(app(sub, 'fixed') as any); + const drift = await d.detectManagedDrift(); + + await (d as any).knex.raw(`DROP INDEX ${sub.replacementName}`); + (d as any).syncDeclaredIndexes = async () => undefined; + + const { applied, skipped } = await d.applyMigrationEntries(drift, { allowDestructive: false }); + const isReplace = (e: { table: string; op: { type: string } }) => + e.table === sub.table && e.op.type === 'replace_unique_index'; + expect(applied.some(isReplace)).toBe(false); + expect(skipped.some(isReplace)).toBe(true); + + // The pre-migration constraint is intact: never left with neither index. + expect(Object.keys(await uniqueKeyParts(sub.table))).toEqual([sub.legacyName]); + }); + }); + + // ─────────────────────────────────────────────────────────────────── + // 4. The #8461 arm and its guards, exercised on THIS object (A1) + // ─────────────────────────────────────────────────────────────────── + + describe('the declared-index replacement arm', () => { + const physicalColumns = () => new Set(Object.keys(sub.fields)); + + it('proposes exactly one retirement, keyed on the listed columns', () => { + const [entry, ...rest] = legacyUniqueReplacements({ + table: sub.table, + fields: {}, + tenantField: 'organization_id', + physicalColumns: physicalColumns(), + declaredIndexes: sub.fixedIndexes, + } as any); + expect(rest).toHaveLength(0); + expect(entry).toMatchObject({ + // ⚠️ `legacyColumns` is the whole listed key, not the leading + // column. `column` is the LEADING one and is reporting only — the + // two composites here are what make that distinction observable. + legacyColumns: sub.keyColumns, + legacyNames: [sub.legacyName], + replacement: { + name: sub.replacementName, + columns: ['organization_id', ...sub.keyColumns], + unique: true, + nullSafeColumns: ['organization_id'], + }, + }); + }); + + it('claims nothing for an EXPLICITLY NAMED index — that transition is a recreate', () => { + // #8461 guard 1. If this ever starts proposing a replacement it would + // ask to drop the very index `recreate_index` is rebuilding. + expect( + legacyUniqueReplacements({ + table: sub.table, + fields: {}, + tenantField: 'organization_id', + physicalColumns: physicalColumns(), + declaredIndexes: [{ name: 'uq_hand_named', fields: sub.keyColumns, unique: 'organization' }], + } as any), + ).toHaveLength(0); + }); + + it('claims nothing when the legacy name IS the replacement name (the S6 composite)', () => { + // #8461 guard 2 — what protects sys_team / sys_business_unit / + // sys_member, and the seven S6 objects the #8554 sweep re-triaged. + expect( + legacyUniqueReplacements({ + table: sub.table, + fields: {}, + tenantField: 'organization_id', + physicalColumns: physicalColumns(), + declaredIndexes: [ + { fields: ['organization_id', ...sub.keyColumns], unique: 'organization' }, + ], + } as any), + ).toHaveLength(0); + }); + + it('claims nothing for the BARE spelling — an unrespelled declaration is untouched (#5082)', () => { + expect( + legacyUniqueReplacements({ + table: sub.table, + fields: {}, + tenantField: 'organization_id', + physicalColumns: physicalColumns(), + declaredIndexes: [{ fields: sub.keyColumns, unique: true }], + } as any), + ).toHaveLength(0); + }); + }); + }); + } + + // ───────────────────────────────────────────────────────────────────────── + // 5. ADR-0120: no index shape reads the posture + // ───────────────────────────────────────────────────────────────────────── + + describe('postureIndependence', () => { + it('materializes the same key parts under single / group / isolated, on all five', async () => { + for (const sub of SUBJECTS) { + for (const posture of ['single', 'group', 'isolated']) { + process.env.OS_TENANCY_POSTURE = posture; + const d = makeDriver(); + await d.initObjects(app(sub, 'fixed') as any); + expect(await uniqueKeyParts(sub.table), `${sub.table} posture=${posture}`).toEqual({ + [sub.replacementName]: ['COALESCE(organization_id)', ...sub.keyColumns], + }); + await d.disconnect(); + driver = undefined; + } + } + }); + }); + + // ───────────────────────────────────────────────────────────────────────── + // 6. The index-name truncation boundary (A1's hidden case) + // ───────────────────────────────────────────────────────────────────────── + + describe('the hash-suffixed replacement name', () => { + /** + * `sys_notification_preference` is the only one of the five whose + * replacement name passes `INDEX_NAME_MAX = 60` and is therefore truncated + * to `uniq_` plus a sha1 prefix. It is worth its own assertions + * because a name-based retirement is precisely where this would go wrong: + * if the legacy name and the replacement name ever collapsed to the same + * string, the `legacyName === replacement.name` guard would silently treat + * the respelling as "nothing was superseded" and emit NO migration — the + * declaration would change, a fresh database would look right, and every + * deployed installation would keep the global index forever. + */ + const sub = SUBJECTS.find((x) => x.table === 'sys_notification_preference')!; + + it('the legacy and replacement names are DIFFERENT, so the S6 guard does not swallow the retirement', () => { + expect(sub.legacyName.length).toBeLessThanOrEqual(60); + expect(sub.replacementName).not.toBe(sub.legacyName); + expect(sub.replacementName).toMatch(/^uniq_sys_notification_preference_[0-9a-f]{8}$/); + // The un-truncated form, for the reader: 70 characters. + expect('uniq_sys_notification_preference_organization_id_user_id_topic_channel'.length).toBe(70); + }); + + it('the truncated name is what actually materializes, and the migration converges on it', async () => { + const d = makeDriver(); + await d.initObjects(app(sub, 'pre') as any); + await d.create(sub.table, row(sub, 'r1', 'org_jia', sub.key) as any); + await d.initObjects(app(sub, 'fixed') as any); + await d.applyMigrationEntries(await d.detectManagedDrift(), { allowDestructive: false }); + + expect(Object.keys(await uniqueKeyParts(sub.table))).toEqual([sub.replacementName]); + expect(await d.detectManagedDrift()).toHaveLength(0); + }); + }); +}); diff --git a/packages/platform-objects/src/audit/sys-email-template.object.ts b/packages/platform-objects/src/audit/sys-email-template.object.ts index 83f991ba7b..c8183e48e0 100644 --- a/packages/platform-objects/src/audit/sys-email-template.object.ts +++ b/packages/platform-objects/src/audit/sys-email-template.object.ts @@ -190,7 +190,17 @@ export const SysEmailTemplate = ObjectSchema.create({ }, indexes: [ - { fields: ['name', 'locale'], unique: true }, + // [#8554] Scope spelled EXPLICITLY (ADR-0120 D1). On a DECLARED index bare + // `unique: true` is the positional spelling of `'global'` — the listed + // columns VERBATIM, composite included — so `(name, locale)` was an + // installation-wide key on a tenant-scoped object. Measured live before the + // fix: org_jia creates (welcome, en-US) 201 / org_yi the SAME pair 409 + // UNIQUE_VIOLATION / org_yi (other_tpl, en-US) 201 / org_yi (welcome, zh-CN) + // 201 / org_yi's own GET on the colliding pair 0 rows. The last two controls + // are what prove the key was the composite rather than `name` alone. + // Admins author and overlay templates per tenant, so two organizations both + // holding a `welcome` / `en-US` template is the normal case. + { fields: ['name', 'locale'], unique: 'organization' }, { fields: ['category'] }, { fields: ['active'] }, ], diff --git a/packages/platform-objects/src/audit/sys-email-template.organization-unique.test.ts b/packages/platform-objects/src/audit/sys-email-template.organization-unique.test.ts new file mode 100644 index 0000000000..19e61570d2 --- /dev/null +++ b/packages/platform-objects/src/audit/sys-email-template.organization-unique.test.ts @@ -0,0 +1,102 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { IndexSchema } from '@objectstack/spec/data'; +import { resolveInjectedSystemColumns } from '@objectstack/spec/data'; +import { SysEmailTemplate } from './sys-email-template.object.js'; + +/** + * #8554 — `sys_email_template`'s declared uniqueness is organization-scoped. + * + * ## What the bare spelling cost + * + * A DECLARED index's `unique: true` is the positional spelling of `'global'` + * (the listed columns verbatim), so the composite `(name, locale)` was an installation-wide key on a + * tenant-scoped object. Measured live on a real engine BEFORE the fix, driving + * this shipped declaration through `SqlDriver` under + * `OS_TENANCY_POSTURE=isolated`: + * + * ``` + * CREATE UNIQUE INDEX uniq_sys_email_template_name_locale + * on sys_email_template (name, locale) + * + * org_jia POST (welcome, en-US) → 201 + * org_yi POST the SAME → 409 UNIQUE_VIOLATION + * org_yi POST an unused → 201 ← the control that makes it an ORACLE + * org_yi GET the key → total 0 ← refused by a row it cannot see + * ``` + * + * ## Why per-organization is the CORRECT boundary, not merely the safe one + * + * The object's own header says administrators author and edit templates in + * Studio and that tenants may overlay specific rows. Two organizations both + * holding a `welcome` template in `en-US` is the normal case. + * + * ⚠️ This is one of the two COMPOSITE cases in #8554 — the shapes the + * `replace_unique_index` arm had not been exercised on before this card, and + * where a single-column assumption would have hidden. The probe therefore ran a + * second control varying only `locale` (accepted, before and after), which is + * what proves the installation-wide key was the composite rather than `name` + * alone. + * + * The materialized shape, the anti-vacuity twin (a SAME-organization duplicate + * must still be refused), and the migration of an installation that already + * carries the old index are pinned driver-side in + * `driver-sql/src/sql-driver-tenant-scoped-declared-unique.test.ts`. This test + * pins the declaration that suite's fixture copies. + */ +describe('sys_email_template — declared uniqueness is organization-scoped (#8554)', () => { + const uniqueIndexes = (SysEmailTemplate.indexes ?? []).filter((i: any) => i.unique); + + it('declares exactly one unique index, on (name, locale)', () => { + expect(uniqueIndexes).toHaveLength(1); + expect((uniqueIndexes[0] as any).fields).toEqual(['name', 'locale']); + }); + + it("spells the scope 'organization' — NOT bare `true`", () => { + // ⛔ Bare `true` here is `'global'` — the installation-wide key that made + // the 409 an oracle over other tenants' rows. Asserted by EQUALITY, never + // by truthiness: a truthy check accepts the very spelling that was the bug, + // which is how #8556's equivalent pin stayed green under its own ablation. + expect((uniqueIndexes[0] as any).unique).toBe('organization'); + }); + + it('is a valid IndexSchema — the spec accepts the explicit vocabulary', () => { + expect(IndexSchema.parse(uniqueIndexes[0])).toMatchObject({ + fields: ['name', 'locale'], + unique: 'organization', + }); + }); + + it('matches the fixture the driver suite copies, entry for entry', () => { + // The driver suite hand-copies this declaration to keep the package + // boundary (the shape #8461 and #8556 used). This assertion catches ONE + // direction of drift: + // + // caught — the shipped declaration changes and the driver fixture + // does not. This test goes red. + // NOT caught — the DRIVER fixture is edited and this declaration is not. + // Nothing compares the two copies directly; they are only + // ever checked against this third spelling. + // + // Asserted on the BUILT value, which is what a driver is handed: + // `ObjectSchema.create` normalizes an authored `{ fields: [...] }` into + // `{ fields: [...], unique: false }`. A fixture copied from the source + // text alone would be subtly wrong about what the driver sees. + expect(SysEmailTemplate.indexes).toEqual([ + { fields: ['name', 'locale'], unique: 'organization' }, + { fields: ['category'], unique: false }, + { fields: ['active'], unique: false }, + ]); + }); + + it('takes no tenancy opt-out, so organization_id is injected (the scope has a column)', () => { + // Derived from the BUILT value, not a regex over source. If this ever goes + // false the `'organization'` spelling has no column to key on and the whole + // fix is silently inert — a failure the index assertions above cannot see. + const plan = resolveInjectedSystemColumns(SysEmailTemplate); + expect(SysEmailTemplate.tenancy).toBeUndefined(); + expect(plan.tenant).toBe(true); + expect(plan.names.has('organization_id')).toBe(true); + }); +}); diff --git a/packages/plugins/plugin-security/src/objects/sys-permission-set.object.ts b/packages/plugins/plugin-security/src/objects/sys-permission-set.object.ts index cff65a4348..29ad2353bd 100644 --- a/packages/plugins/plugin-security/src/objects/sys-permission-set.object.ts +++ b/packages/plugins/plugin-security/src/objects/sys-permission-set.object.ts @@ -83,7 +83,10 @@ export const SysPermissionSet = ObjectSchema.create({ refreshAfter: true, params: [ { name: 'label', label: 'New Display Name', type: 'text', required: true }, - { name: 'name', label: 'New API Name', type: 'text', required: true, helpText: 'Unique snake_case machine name' }, + // [#8554] The clone dialog is the exact moment an admin types a NEW + // name, so the scope has to be right here — a bare "Unique" tells the + // author the name is taken installation-wide when it is not. + { name: 'name', label: 'New API Name', type: 'text', required: true, helpText: 'snake_case machine name, unique per organization' }, { field: 'description', defaultFromRow: true }, { field: 'object_permissions', defaultFromRow: true }, { field: 'field_permissions', defaultFromRow: true }, @@ -140,10 +143,12 @@ export const SysPermissionSet = ObjectSchema.create({ required: true, searchable: true, maxLength: 100, + // [#8554] "unique per organization", not bare "unique". The bare wording + // described the installation-wide index this card removed. description: - 'Unique machine name for the permission set. This is the set’s metadata identity ' + - '(ADR-0094) and cannot be changed after creation — the data door rejects a rename; ' + - 'clone the set to a new name instead.', + 'Machine name for the permission set, unique per organization. This is the set’s ' + + 'metadata identity (ADR-0094) and cannot be changed after creation — the data door ' + + 'rejects a rename; clone the set to a new name instead.', // [ADR-0094] The name is the metadata key the record projects from, so it // is immutable once the record exists. `record.id` is server-assigned: // absent on the create form (editable), present on edit (locked). The @@ -289,7 +294,15 @@ export const SysPermissionSet = ObjectSchema.create({ }, indexes: [ - { fields: ['name'], unique: true }, + // [#8554] Scope spelled EXPLICITLY (ADR-0120 D1). On a DECLARED index bare + // `unique: true` is the positional spelling of `'global'` — the listed + // columns verbatim — so this was an installation-wide key on a tenant-scoped + // object. Measured live before the fix, two organizations and the same name: + // org_jia 201 / org_yi 409 UNIQUE_VIOLATION / org_yi unused name 201 / + // org_yi's own GET on the colliding name 0 rows. A per-value refusal on a + // row the caller cannot read is the #8323 cross-tenant existence oracle, and + // two tenants could not both name a set `sales_readonly`. + { fields: ['name'], unique: 'organization' }, { fields: ['active'] }, // ADR-0086 D3 — uninstall/upgrade query: "this package's own sets". { fields: ['package_id'] }, diff --git a/packages/plugins/plugin-security/src/objects/sys-permission-set.organization-unique.test.ts b/packages/plugins/plugin-security/src/objects/sys-permission-set.organization-unique.test.ts new file mode 100644 index 0000000000..f90c819c6d --- /dev/null +++ b/packages/plugins/plugin-security/src/objects/sys-permission-set.organization-unique.test.ts @@ -0,0 +1,98 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { IndexSchema } from '@objectstack/spec/data'; +import { resolveInjectedSystemColumns } from '@objectstack/spec/data'; +import { SysPermissionSet } from './sys-permission-set.object.js'; + +/** + * #8554 — `sys_permission_set`'s declared uniqueness is organization-scoped. + * + * ## What the bare spelling cost + * + * A DECLARED index's `unique: true` is the positional spelling of `'global'` + * (the listed columns verbatim), so `name` was an installation-wide key on a + * tenant-scoped object. Measured live on a real engine BEFORE the fix, driving + * this shipped declaration through `SqlDriver` under + * `OS_TENANCY_POSTURE=isolated`: + * + * ``` + * CREATE UNIQUE INDEX uniq_sys_permission_set_name + * on sys_permission_set (name) + * + * org_jia POST name=sales_readonly → 201 + * org_yi POST the SAME → 409 UNIQUE_VIOLATION + * org_yi POST an unused → 201 ← the control that makes it an ORACLE + * org_yi GET the key → total 0 ← refused by a row it cannot see + * ``` + * + * ## Why per-organization is the CORRECT boundary, not merely the safe one + * + * Permission sets are admin-authored: the object's own header says tenants may + * add custom rows created via UI / API while the schema itself is locked — the + * same sentence that made `sys_position` a defect (#8468). Two organizations + * naming a set `sales_readonly` are not in conflict. It is the third leg of the + * ADR-0090 RBAC triad, after `sys_capability` (#8461) and `sys_position` + * (#8556), and it sits in the same directory as both. + * + * The materialized shape, the anti-vacuity twin (a SAME-organization duplicate + * must still be refused), and the migration of an installation that already + * carries the old index are pinned driver-side in + * `driver-sql/src/sql-driver-tenant-scoped-declared-unique.test.ts`. This test + * pins the declaration that suite's fixture copies. + */ +describe('sys_permission_set — declared uniqueness is organization-scoped (#8554)', () => { + const uniqueIndexes = (SysPermissionSet.indexes ?? []).filter((i: any) => i.unique); + + it('declares exactly one unique index, on (name)', () => { + expect(uniqueIndexes).toHaveLength(1); + expect((uniqueIndexes[0] as any).fields).toEqual(['name']); + }); + + it("spells the scope 'organization' — NOT bare `true`", () => { + // ⛔ Bare `true` here is `'global'` — the installation-wide key that made + // the 409 an oracle over other tenants' rows. Asserted by EQUALITY, never + // by truthiness: a truthy check accepts the very spelling that was the bug, + // which is how #8556's equivalent pin stayed green under its own ablation. + expect((uniqueIndexes[0] as any).unique).toBe('organization'); + }); + + it('is a valid IndexSchema — the spec accepts the explicit vocabulary', () => { + expect(IndexSchema.parse(uniqueIndexes[0])).toMatchObject({ + fields: ['name'], + unique: 'organization', + }); + }); + + it('matches the fixture the driver suite copies, entry for entry', () => { + // The driver suite hand-copies this declaration to keep the package + // boundary (the shape #8461 and #8556 used). This assertion catches ONE + // direction of drift: + // + // caught — the shipped declaration changes and the driver fixture + // does not. This test goes red. + // NOT caught — the DRIVER fixture is edited and this declaration is not. + // Nothing compares the two copies directly; they are only + // ever checked against this third spelling. + // + // Asserted on the BUILT value, which is what a driver is handed: + // `ObjectSchema.create` normalizes an authored `{ fields: [...] }` into + // `{ fields: [...], unique: false }`. A fixture copied from the source + // text alone would be subtly wrong about what the driver sees. + expect(SysPermissionSet.indexes).toEqual([ + { fields: ['name'], unique: 'organization' }, + { fields: ['active'], unique: false }, + { fields: ['package_id'], unique: false }, + ]); + }); + + it('takes no tenancy opt-out, so organization_id is injected (the scope has a column)', () => { + // Derived from the BUILT value, not a regex over source. If this ever goes + // false the `'organization'` spelling has no column to key on and the whole + // fix is silently inert — a failure the index assertions above cannot see. + const plan = resolveInjectedSystemColumns(SysPermissionSet); + expect(SysPermissionSet.tenancy).toBeUndefined(); + expect(plan.tenant).toBe(true); + expect(plan.names.has('organization_id')).toBe(true); + }); +}); diff --git a/packages/plugins/plugin-security/src/translations/en.objects.generated.ts b/packages/plugins/plugin-security/src/translations/en.objects.generated.ts index 7acd67268e..cd2a0e1023 100644 --- a/packages/plugins/plugin-security/src/translations/en.objects.generated.ts +++ b/packages/plugins/plugin-security/src/translations/en.objects.generated.ts @@ -184,7 +184,7 @@ export const enObjects: NonNullable = { }, name: { label: "API Name", - help: "Unique machine name for the permission set" + help: "Machine name for the permission set, unique per organization" }, description: { label: "Description" @@ -273,7 +273,7 @@ export const enObjects: NonNullable = { }, name: { label: "New API Name", - helpText: "Unique snake_case machine name" + helpText: "snake_case machine name, unique per organization" } } } diff --git a/packages/plugins/plugin-security/src/translations/es-ES.objects.generated.ts b/packages/plugins/plugin-security/src/translations/es-ES.objects.generated.ts index ec2b70b7f6..ee1891744a 100644 --- a/packages/plugins/plugin-security/src/translations/es-ES.objects.generated.ts +++ b/packages/plugins/plugin-security/src/translations/es-ES.objects.generated.ts @@ -184,7 +184,7 @@ export const esESObjects: NonNullable = { }, name: { label: "Nombre de API", - help: "Nombre técnico único del conjunto de permisos." + help: "Nombre técnico del conjunto de permisos, único por organización." }, description: { label: "Descripción" @@ -273,7 +273,7 @@ export const esESObjects: NonNullable = { }, name: { label: "Nuevo nombre de API", - helpText: "Nombre de máquina snake_case único" + helpText: "Nombre de máquina snake_case, único por organización" } } } diff --git a/packages/plugins/plugin-security/src/translations/ja-JP.objects.generated.ts b/packages/plugins/plugin-security/src/translations/ja-JP.objects.generated.ts index f3468a6932..40a0deb500 100644 --- a/packages/plugins/plugin-security/src/translations/ja-JP.objects.generated.ts +++ b/packages/plugins/plugin-security/src/translations/ja-JP.objects.generated.ts @@ -184,7 +184,7 @@ export const jaJPObjects: NonNullable = { }, name: { label: "API 名", - help: "権限セットの一意の機械名" + help: "権限セットのマシン名(組織ごとに一意)" }, description: { label: "説明" @@ -273,7 +273,7 @@ export const jaJPObjects: NonNullable = { }, name: { label: "新しい API 名", - helpText: "一意の snake_case マシン名" + helpText: "snake_case マシン名(組織ごとに一意)" } } } diff --git a/packages/plugins/plugin-security/src/translations/zh-CN.objects.generated.ts b/packages/plugins/plugin-security/src/translations/zh-CN.objects.generated.ts index 35826b91ce..772b28189c 100644 --- a/packages/plugins/plugin-security/src/translations/zh-CN.objects.generated.ts +++ b/packages/plugins/plugin-security/src/translations/zh-CN.objects.generated.ts @@ -184,7 +184,7 @@ export const zhCNObjects: NonNullable = { }, name: { label: "API 名称", - help: "权限集的唯一机器名称" + help: "权限集的机器名称,在每个组织内唯一" }, description: { label: "描述" @@ -273,7 +273,7 @@ export const zhCNObjects: NonNullable = { }, name: { label: "新 API 名称", - helpText: "唯一的 snake_case 机器名称" + helpText: "snake_case 机器名称,在每个组织内唯一" } } } diff --git a/packages/plugins/plugin-sharing/src/objects/sys-sharing-rule.object.ts b/packages/plugins/plugin-sharing/src/objects/sys-sharing-rule.object.ts index c712446e27..c7a85768b3 100644 --- a/packages/plugins/plugin-sharing/src/objects/sys-sharing-rule.object.ts +++ b/packages/plugins/plugin-sharing/src/objects/sys-sharing-rule.object.ts @@ -271,7 +271,15 @@ export const SysSharingRule = ObjectSchema.create({ indexes: [ { fields: ['object_name', 'active'] }, - { fields: ['name'], unique: true }, + // [#8554] Scope spelled EXPLICITLY (ADR-0120 D1). On a DECLARED index bare + // `unique: true` is the positional spelling of `'global'` — the listed + // columns verbatim — so this was an installation-wide key on a tenant-scoped + // object. Measured live before the fix: org_jia 201 / org_yi 409 + // UNIQUE_VIOLATION on the same name / org_yi unused name 201 / org_yi's own + // GET on the colliding name 0 rows. Sharing rules are authored by admins in + // the Studio criteria builder, so two organizations naming a rule + // `share_west_region` are not in conflict. + { fields: ['name'], unique: 'organization' }, { fields: ['organization_id'] }, ], }); diff --git a/packages/plugins/plugin-sharing/src/objects/sys-sharing-rule.organization-unique.test.ts b/packages/plugins/plugin-sharing/src/objects/sys-sharing-rule.organization-unique.test.ts new file mode 100644 index 0000000000..e20c59eb1a --- /dev/null +++ b/packages/plugins/plugin-sharing/src/objects/sys-sharing-rule.organization-unique.test.ts @@ -0,0 +1,100 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { IndexSchema } from '@objectstack/spec/data'; +import { resolveInjectedSystemColumns } from '@objectstack/spec/data'; +import { SysSharingRule } from './sys-sharing-rule.object.js'; + +/** + * #8554 — `sys_sharing_rule`'s declared uniqueness is organization-scoped. + * + * ## What the bare spelling cost + * + * A DECLARED index's `unique: true` is the positional spelling of `'global'` + * (the listed columns verbatim), so `name` was an installation-wide key on a + * tenant-scoped object. Measured live on a real engine BEFORE the fix, driving + * this shipped declaration through `SqlDriver` under + * `OS_TENANCY_POSTURE=isolated`: + * + * ``` + * CREATE UNIQUE INDEX uniq_sys_sharing_rule_name + * on sys_sharing_rule (name) + * + * org_jia POST name=share_west_region → 201 + * org_yi POST the SAME → 409 UNIQUE_VIOLATION + * org_yi POST an unused → 201 ← the control that makes it an ORACLE + * org_yi GET the key → total 0 ← refused by a row it cannot see + * ``` + * + * ## Why per-organization is the CORRECT boundary, not merely the safe one + * + * Sharing rules are authored by admins through the Studio criteria builder, and + * a rule's whole subject matter — which records of which object are shared with + * whom — is tenant-local. Two organizations naming a rule `share_west_region` + * are not in conflict. + * + * ⚠️ Note the neighbouring `{ fields: ['organization_id'] }` entry: this object + * already indexed the tenant column, non-uniquely, while its UNIQUE key ignored + * it. That is the clearest single illustration of the #4986 trap in the tree. + * + * The materialized shape, the anti-vacuity twin (a SAME-organization duplicate + * must still be refused), and the migration of an installation that already + * carries the old index are pinned driver-side in + * `driver-sql/src/sql-driver-tenant-scoped-declared-unique.test.ts`. This test + * pins the declaration that suite's fixture copies. + */ +describe('sys_sharing_rule — declared uniqueness is organization-scoped (#8554)', () => { + const uniqueIndexes = (SysSharingRule.indexes ?? []).filter((i: any) => i.unique); + + it('declares exactly one unique index, on (name)', () => { + expect(uniqueIndexes).toHaveLength(1); + expect((uniqueIndexes[0] as any).fields).toEqual(['name']); + }); + + it("spells the scope 'organization' — NOT bare `true`", () => { + // ⛔ Bare `true` here is `'global'` — the installation-wide key that made + // the 409 an oracle over other tenants' rows. Asserted by EQUALITY, never + // by truthiness: a truthy check accepts the very spelling that was the bug, + // which is how #8556's equivalent pin stayed green under its own ablation. + expect((uniqueIndexes[0] as any).unique).toBe('organization'); + }); + + it('is a valid IndexSchema — the spec accepts the explicit vocabulary', () => { + expect(IndexSchema.parse(uniqueIndexes[0])).toMatchObject({ + fields: ['name'], + unique: 'organization', + }); + }); + + it('matches the fixture the driver suite copies, entry for entry', () => { + // The driver suite hand-copies this declaration to keep the package + // boundary (the shape #8461 and #8556 used). This assertion catches ONE + // direction of drift: + // + // caught — the shipped declaration changes and the driver fixture + // does not. This test goes red. + // NOT caught — the DRIVER fixture is edited and this declaration is not. + // Nothing compares the two copies directly; they are only + // ever checked against this third spelling. + // + // Asserted on the BUILT value, which is what a driver is handed: + // `ObjectSchema.create` normalizes an authored `{ fields: [...] }` into + // `{ fields: [...], unique: false }`. A fixture copied from the source + // text alone would be subtly wrong about what the driver sees. + expect(SysSharingRule.indexes).toEqual([ + { fields: ['object_name', 'active'], unique: false }, + { fields: ['name'], unique: 'organization' }, + { fields: ['organization_id'], unique: false }, + ]); + }); + + it('takes no tenancy opt-out, so organization_id is injected (the scope has a column)', () => { + // Derived from the BUILT value, not a regex over source. If this ever goes + // false the `'organization'` spelling has no column to key on and the whole + // fix is silently inert — a failure the index assertions above cannot see. + const plan = resolveInjectedSystemColumns(SysSharingRule); + expect(SysSharingRule.tenancy).toBeUndefined(); + expect(plan.tenant).toBe(true); + expect(plan.names.has('organization_id')).toBe(true); + }); +}); diff --git a/packages/plugins/plugin-webhooks/src/sys-webhook.object.ts b/packages/plugins/plugin-webhooks/src/sys-webhook.object.ts index cd490af04b..31a75f1b21 100644 --- a/packages/plugins/plugin-webhooks/src/sys-webhook.object.ts +++ b/packages/plugins/plugin-webhooks/src/sys-webhook.object.ts @@ -104,7 +104,9 @@ export const SysWebhook = ObjectSchema.create({ label: 'Name', required: true, maxLength: 100, - description: 'Unique snake_case name — referenced in logs and audit', + // [#8554] "unique per organization", not bare "unique" — the bare wording + // described the installation-wide index this card removed. + description: 'snake_case name, unique per organization — referenced in logs and audit', group: 'Definition', }), @@ -301,7 +303,14 @@ export const SysWebhook = ObjectSchema.create({ }, indexes: [ - { fields: ['name'], unique: true }, + // [#8554] Scope spelled EXPLICITLY (ADR-0120 D1). On a DECLARED index bare + // `unique: true` is the positional spelling of `'global'` — the listed + // columns verbatim — so this was an installation-wide key on a tenant-scoped + // object. Measured live before the fix: org_jia 201 / org_yi 409 + // UNIQUE_VIOLATION on the same name / org_yi unused name 201 / org_yi's own + // GET on the colliding name 0 rows. Webhooks are named by admins from the + // UI, so two organizations both wanting `order_created_hook` is ordinary. + { fields: ['name'], unique: 'organization' }, { fields: ['object_name'] }, { fields: ['active', 'object_name'] }, ], diff --git a/packages/plugins/plugin-webhooks/src/sys-webhook.organization-unique.test.ts b/packages/plugins/plugin-webhooks/src/sys-webhook.organization-unique.test.ts new file mode 100644 index 0000000000..0db8634f81 --- /dev/null +++ b/packages/plugins/plugin-webhooks/src/sys-webhook.organization-unique.test.ts @@ -0,0 +1,96 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { IndexSchema } from '@objectstack/spec/data'; +import { resolveInjectedSystemColumns } from '@objectstack/spec/data'; +import { SysWebhook } from './sys-webhook.object.js'; + +/** + * #8554 — `sys_webhook`'s declared uniqueness is organization-scoped. + * + * ## What the bare spelling cost + * + * A DECLARED index's `unique: true` is the positional spelling of `'global'` + * (the listed columns verbatim), so `name` was an installation-wide key on a + * tenant-scoped object. Measured live on a real engine BEFORE the fix, driving + * this shipped declaration through `SqlDriver` under + * `OS_TENANCY_POSTURE=isolated`: + * + * ``` + * CREATE UNIQUE INDEX uniq_sys_webhook_name + * on sys_webhook (name) + * + * org_jia POST name=order_created_hook → 201 + * org_yi POST the SAME → 409 UNIQUE_VIOLATION + * org_yi POST an unused → 201 ← the control that makes it an ORACLE + * org_yi GET the key → total 0 ← refused by a row it cannot see + * ``` + * + * ## Why per-organization is the CORRECT boundary, not merely the safe one + * + * Webhooks are named by admins from the UI and point at that tenant's own + * endpoint. Two organizations both wanting `order_created_hook` is the ordinary + * case, not a collision — and before this fix the second one was told only + * `409`, with no way to discover why. + * + * The materialized shape, the anti-vacuity twin (a SAME-organization duplicate + * must still be refused), and the migration of an installation that already + * carries the old index are pinned driver-side in + * `driver-sql/src/sql-driver-tenant-scoped-declared-unique.test.ts`. This test + * pins the declaration that suite's fixture copies. + */ +describe('sys_webhook — declared uniqueness is organization-scoped (#8554)', () => { + const uniqueIndexes = (SysWebhook.indexes ?? []).filter((i: any) => i.unique); + + it('declares exactly one unique index, on (name)', () => { + expect(uniqueIndexes).toHaveLength(1); + expect((uniqueIndexes[0] as any).fields).toEqual(['name']); + }); + + it("spells the scope 'organization' — NOT bare `true`", () => { + // ⛔ Bare `true` here is `'global'` — the installation-wide key that made + // the 409 an oracle over other tenants' rows. Asserted by EQUALITY, never + // by truthiness: a truthy check accepts the very spelling that was the bug, + // which is how #8556's equivalent pin stayed green under its own ablation. + expect((uniqueIndexes[0] as any).unique).toBe('organization'); + }); + + it('is a valid IndexSchema — the spec accepts the explicit vocabulary', () => { + expect(IndexSchema.parse(uniqueIndexes[0])).toMatchObject({ + fields: ['name'], + unique: 'organization', + }); + }); + + it('matches the fixture the driver suite copies, entry for entry', () => { + // The driver suite hand-copies this declaration to keep the package + // boundary (the shape #8461 and #8556 used). This assertion catches ONE + // direction of drift: + // + // caught — the shipped declaration changes and the driver fixture + // does not. This test goes red. + // NOT caught — the DRIVER fixture is edited and this declaration is not. + // Nothing compares the two copies directly; they are only + // ever checked against this third spelling. + // + // Asserted on the BUILT value, which is what a driver is handed: + // `ObjectSchema.create` normalizes an authored `{ fields: [...] }` into + // `{ fields: [...], unique: false }`. A fixture copied from the source + // text alone would be subtly wrong about what the driver sees. + expect(SysWebhook.indexes).toEqual([ + { fields: ['name'], unique: 'organization' }, + { fields: ['object_name'], unique: false }, + { fields: ['active', 'object_name'], unique: false }, + ]); + }); + + it('takes no tenancy opt-out, so organization_id is injected (the scope has a column)', () => { + // Derived from the BUILT value, not a regex over source. If this ever goes + // false the `'organization'` spelling has no column to key on and the whole + // fix is silently inert — a failure the index assertions above cannot see. + const plan = resolveInjectedSystemColumns(SysWebhook); + expect(SysWebhook.tenancy).toBeUndefined(); + expect(plan.tenant).toBe(true); + expect(plan.names.has('organization_id')).toBe(true); + }); +}); diff --git a/packages/plugins/plugin-webhooks/src/translations/en.objects.generated.ts b/packages/plugins/plugin-webhooks/src/translations/en.objects.generated.ts index cae6601e96..4ef00fb4c9 100644 --- a/packages/plugins/plugin-webhooks/src/translations/en.objects.generated.ts +++ b/packages/plugins/plugin-webhooks/src/translations/en.objects.generated.ts @@ -19,7 +19,7 @@ export const enObjects: NonNullable = { }, name: { label: "Name", - help: "Unique snake_case name — referenced in logs and audit" + help: "snake_case name, unique per organization — referenced in logs and audit" }, label: { label: "Display Label" diff --git a/packages/plugins/plugin-webhooks/src/translations/es-ES.objects.generated.ts b/packages/plugins/plugin-webhooks/src/translations/es-ES.objects.generated.ts index 3c8f4ff750..37c8497ef5 100644 --- a/packages/plugins/plugin-webhooks/src/translations/es-ES.objects.generated.ts +++ b/packages/plugins/plugin-webhooks/src/translations/es-ES.objects.generated.ts @@ -19,7 +19,7 @@ export const esESObjects: NonNullable = { }, name: { label: "Nombre", - help: "Nombre snake_case único; se usa en los registros y en la auditoría." + help: "Nombre snake_case, único por organización; se usa en los registros y en la auditoría." }, label: { label: "Nombre visible" diff --git a/packages/plugins/plugin-webhooks/src/translations/ja-JP.objects.generated.ts b/packages/plugins/plugin-webhooks/src/translations/ja-JP.objects.generated.ts index 47c992c59b..27793f9a79 100644 --- a/packages/plugins/plugin-webhooks/src/translations/ja-JP.objects.generated.ts +++ b/packages/plugins/plugin-webhooks/src/translations/ja-JP.objects.generated.ts @@ -19,7 +19,7 @@ export const jaJPObjects: NonNullable = { }, name: { label: "名前", - help: "一意の snake_case 名 — ログおよび監査で参照" + help: "snake_case 名(組織ごとに一意)— ログおよび監査で参照" }, label: { label: "表示名" diff --git a/packages/plugins/plugin-webhooks/src/translations/zh-CN.objects.generated.ts b/packages/plugins/plugin-webhooks/src/translations/zh-CN.objects.generated.ts index af20881855..c28a1a872b 100644 --- a/packages/plugins/plugin-webhooks/src/translations/zh-CN.objects.generated.ts +++ b/packages/plugins/plugin-webhooks/src/translations/zh-CN.objects.generated.ts @@ -19,7 +19,7 @@ export const zhCNObjects: NonNullable = { }, name: { label: "名称", - help: "唯一的 snake_case 名称——用于日志和审计引用" + help: "snake_case 名称,在每个组织内唯一——用于日志和审计引用" }, label: { label: "显示标签" diff --git a/packages/services/service-messaging/src/objects/notification-preference.object.ts b/packages/services/service-messaging/src/objects/notification-preference.object.ts index 2029d55910..cba3007854 100644 --- a/packages/services/service-messaging/src/objects/notification-preference.object.ts +++ b/packages/services/service-messaging/src/objects/notification-preference.object.ts @@ -83,7 +83,24 @@ export const NotificationPreference = ObjectSchema.create({ }, indexes: [ - { fields: ['user_id', 'topic', 'channel'], unique: true }, + // [#8554] Scope spelled EXPLICITLY (ADR-0120 D1). On a DECLARED index + // bare `unique: true` is the positional spelling of `'global'` — the + // listed columns VERBATIM — so `(user_id, topic, channel)` was an + // installation-wide key on a tenant-scoped object. This is the near-exact + // analogue of `sys_user_preference` (#8323): a user who belongs to two + // organizations could not hold INDEPENDENT per-topic toggles, because the + // first organization's row claimed the triple for the whole installation. + // Measured live before the fix: org_jia creates + // (user_u1, billing.invoice, email) 201 / org_yi the SAME triple 409 + // UNIQUE_VIOLATION / org_yi the same pair on `push` 201 / org_yi a + // different topic on `email` 201 / org_yi's own GET on the colliding + // triple 0 rows. + // + // ⚠️ `managedBy: 'system-data'` is NOT a reason to exempt this object. + // The already-ruled `sys_user_preference` is `system-data` too; the + // ruling's phrase is ADMIN-AUTHORED CONTENT — the provenance of the + // rows, not the management mode of the object. + { fields: ['user_id', 'topic', 'channel'], unique: 'organization' }, { fields: ['topic'] }, ], }); diff --git a/packages/services/service-messaging/src/objects/notification-preference.organization-unique.test.ts b/packages/services/service-messaging/src/objects/notification-preference.organization-unique.test.ts new file mode 100644 index 0000000000..fd752aa35d --- /dev/null +++ b/packages/services/service-messaging/src/objects/notification-preference.organization-unique.test.ts @@ -0,0 +1,110 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { IndexSchema } from '@objectstack/spec/data'; +import { resolveInjectedSystemColumns } from '@objectstack/spec/data'; +// Explicit `.js` extension: this package resolves under NodeNext, where an +// extensionless relative import does not typecheck (its sibling suites all +// spell it this way). +import { NotificationPreference } from './notification-preference.object.js'; + +/** + * #8554 — `sys_notification_preference`'s declared uniqueness is organization-scoped. + * + * ## What the bare spelling cost + * + * A DECLARED index's `unique: true` is the positional spelling of `'global'` + * (the listed columns verbatim), so the composite `(user_id, topic, channel)` was an installation-wide key on a + * tenant-scoped object. Measured live on a real engine BEFORE the fix, driving + * this shipped declaration through `SqlDriver` under + * `OS_TENANCY_POSTURE=isolated`: + * + * ``` + * CREATE UNIQUE INDEX uniq_sys_notification_preference_user_id_topic_channel + * on sys_notification_preference (user_id, topic, channel) + * + * org_jia POST (user_u1, billing.invoice, email) → 201 + * org_yi POST the SAME → 409 UNIQUE_VIOLATION + * org_yi POST an unused → 201 ← the control that makes it an ORACLE + * org_yi GET the key → total 0 ← refused by a row it cannot see + * ``` + * + * ## Why per-organization is the CORRECT boundary, not merely the safe one + * + * This is the near-exact analogue of `sys_user_preference`, one of the two + * objects the 2026-08-13 ruling named: the same archetype (a per-user K/V row) + * and the same defect shape. The measured symptom is the one #8323 recorded — + * a user who belongs to two organizations could not hold INDEPENDENT per-topic + * toggles, because the first organization's row claimed the triple for the + * whole installation. + * + * ⚠️ `managedBy: 'system-data'` is NOT a reason to exempt this object. The + * already-ruled `sys_user_preference` is `system-data` too; the ruling's phrase + * is ADMIN-AUTHORED CONTENT — the provenance of the ROWS, not the management + * mode of the object. This object's own header records that a user authors + * their own mute/allow rows from the Setup grid. + * + * ⚠️ The other COMPOSITE case, and the only one of the five whose replacement + * index name passes `INDEX_NAME_MAX = 60` and is hash-suffixed + * (`uniq_sys_notification_preference_a22d7d27`). Pinned driver-side. + * + * The materialized shape, the anti-vacuity twin (a SAME-organization duplicate + * must still be refused), and the migration of an installation that already + * carries the old index are pinned driver-side in + * `driver-sql/src/sql-driver-tenant-scoped-declared-unique.test.ts`. This test + * pins the declaration that suite's fixture copies. + */ +describe('sys_notification_preference — declared uniqueness is organization-scoped (#8554)', () => { + const uniqueIndexes = (NotificationPreference.indexes ?? []).filter((i: any) => i.unique); + + it('declares exactly one unique index, on (user_id, topic, channel)', () => { + expect(uniqueIndexes).toHaveLength(1); + expect((uniqueIndexes[0] as any).fields).toEqual(['user_id', 'topic', 'channel']); + }); + + it("spells the scope 'organization' — NOT bare `true`", () => { + // ⛔ Bare `true` here is `'global'` — the installation-wide key that made + // the 409 an oracle over other tenants' rows. Asserted by EQUALITY, never + // by truthiness: a truthy check accepts the very spelling that was the bug, + // which is how #8556's equivalent pin stayed green under its own ablation. + expect((uniqueIndexes[0] as any).unique).toBe('organization'); + }); + + it('is a valid IndexSchema — the spec accepts the explicit vocabulary', () => { + expect(IndexSchema.parse(uniqueIndexes[0])).toMatchObject({ + fields: ['user_id', 'topic', 'channel'], + unique: 'organization', + }); + }); + + it('matches the fixture the driver suite copies, entry for entry', () => { + // The driver suite hand-copies this declaration to keep the package + // boundary (the shape #8461 and #8556 used). This assertion catches ONE + // direction of drift: + // + // caught — the shipped declaration changes and the driver fixture + // does not. This test goes red. + // NOT caught — the DRIVER fixture is edited and this declaration is not. + // Nothing compares the two copies directly; they are only + // ever checked against this third spelling. + // + // Asserted on the BUILT value, which is what a driver is handed: + // `ObjectSchema.create` normalizes an authored `{ fields: [...] }` into + // `{ fields: [...], unique: false }`. A fixture copied from the source + // text alone would be subtly wrong about what the driver sees. + expect(NotificationPreference.indexes).toEqual([ + { fields: ['user_id', 'topic', 'channel'], unique: 'organization' }, + { fields: ['topic'], unique: false }, + ]); + }); + + it('takes no tenancy opt-out, so organization_id is injected (the scope has a column)', () => { + // Derived from the BUILT value, not a regex over source. If this ever goes + // false the `'organization'` spelling has no column to key on and the whole + // fix is silently inert — a failure the index assertions above cannot see. + const plan = resolveInjectedSystemColumns(NotificationPreference); + expect(NotificationPreference.tenancy).toBeUndefined(); + expect(plan.tenant).toBe(true); + expect(plan.names.has('organization_id')).toBe(true); + }); +}); diff --git a/packages/spec/src/automation/webhook.zod.ts b/packages/spec/src/automation/webhook.zod.ts index 9c0076809b..0cee0c3be1 100644 --- a/packages/spec/src/automation/webhook.zod.ts +++ b/packages/spec/src/automation/webhook.zod.ts @@ -180,7 +180,12 @@ export const WebhookSchema = lazySchema(() => strictObject({ // it is never the unrecognized key there anyway. extraKeys: ['signatureAlgorithm'], }, { - name: SnakeCaseIdentifierSchema.describe('Webhook unique name (lowercase snake_case)'), + // [#8554] "unique per organization", not bare "unique". This `describe()` is + // the SOURCE of the generated reference page + // (`content/docs/references/automation/webhook.mdx`), so the bare wording had + // already reached authors as published contract. `sys_webhook` is + // tenant-scoped and the declared index is now `unique: 'organization'`. + name: SnakeCaseIdentifierSchema.describe('Webhook name, unique per organization (lowercase snake_case)'), label: z.string().optional().describe('Human-readable webhook label'), /** Scope */ diff --git a/packages/spec/src/security/permission.zod.ts b/packages/spec/src/security/permission.zod.ts index a718a1c282..fd887b7c8d 100644 --- a/packages/spec/src/security/permission.zod.ts +++ b/packages/spec/src/security/permission.zod.ts @@ -380,8 +380,14 @@ export const PermissionSetSchema = lazySchema(() => strictObject( 'believed a capability boundary was declared that the runtime never saw.', }, { - /** Unique permission set name */ - name: SnakeCaseIdentifierSchema.describe('Permission set unique name (lowercase snake_case)'), + // [#8554] "unique per organization", not bare "unique". This `describe()` is + // the SOURCE of the generated reference page + // (`content/docs/references/security/permission.mdx`), so the bare wording had + // already reached authors as published contract — the same route the + // `sys_position` accident took (#8468). `sys_permission_set` is tenant-scoped + // and the declared index is now `unique: 'organization'`. + /** Permission set name, unique per organization */ + name: SnakeCaseIdentifierSchema.describe('Permission set name, unique per organization (lowercase snake_case)'), /** Display label */ label: z.string().optional().describe('Display label'),