fix(service-settings)!: no tenant- or user-rung settings read admits an organization-less row (ADR-0131 D8)

.changeset/25202-settings-no-organization-less-rung.md

@@ -0,0 +1,22 @@
+---
+'@objectstack/service-settings': patch
+---
+
+fix(service-settings)!: a tenant- or user-scope setting stored with no organization no longer answers every organization's read, and a caller with no organization reads and writes as the one organization the deployment derives
+
+Clause-②: yes (narrowing)
+Metadata-protocol: none
+
+<!-- adr-0087: not-required (no-migration-prescription) Which stored `sys_setting` row answers a settings read changes. No key of any metadata schema is removed, renamed or re-shaped, so `objectstack migrate meta` has nothing to rewrite and no tombstone is needed. No export, type, error code or route is removed or renamed: `bindEngine` gains one optional input, and the plugin supplies it. The remedy for the rows this stops reading is the existing organization-ownership ceremony, stated below. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id covers this read (not registered or already-registered); and nothing exported is removed or narrowed (not runtime-interface-only, type-surface-only or runtime-export-retired). -->
+
+**BREAKING** read narrowing (ADR-0131 D1 "nothing reads a NULL tenant value as an owner", and D8 "no layer has a NULL arm"). It changes which stored row a settings read takes, not what a metadata author can write, so it ships as `patch` with this banner.
+
+- FROM: a `tenant`- or `user`-scope `sys_setting` row written with no organization was every organization's fallback. A caller that named an organization read its own row, or failing that the organization-less one. A caller that named none read only organization-less rows behind a wall (`group` / `isolated`). Under `single` it read every row, an organization's first. Under `single` such a caller also wrote with no organization, so its write's identity probe missed the row the engine stamped with the Default Organization. The second save of a key then inserted a second row beside the first. On a SQL store the identity index refuses that insert and the save fails. On a store with no such index the save was lost: every reader kept answering the first value.
+- TO: a caller reads its organization's rows and no others. For a caller that names no organization, the deployment names one (ADR-0131 D9, the predicate the engine applies to a system write):
+  - Under `single` with one organization, the caller reads and writes as that organization. This covers the process-wide readers: plugin-auth's email brand name and auth locale, and the anonymous `runtime/config` branding read.
+  - Behind a wall, or under `single` with no organization or several, the caller has no tenant or user rung. Its read answers from the global rung or the default, and its tenant-scope write is refused (`SETTINGS_VALIDATION`, `constraint: { scope: 'tenant' }`).
+  - A composition that registers no organization object (a lean embedding, kept by ruling) still stores and reads its rows with no organization.
+
+**Who is affected.** Only a store holding a tenant- or user-scope `sys_setting` row with a NULL `organization_id`. A fresh install holds none, nor does an 18.0.0-born store written through `os serve`: the measured 18.0.0 fixture holds two `sys_setting` rows, both owned. A v17 store holds none after `os migrate organization-ownership` under `single`, which gives such rows to the Default Organization. Under a walled posture the ceremony leaves such a row NULL and reports it, and from this release that row stops answering any organization.
+
+**The remedy.** Run `os migrate organization-ownership --out PLAN`, review the reported `sys_setting` rows, and save each value again from inside the organization it belongs to (Setup → the settings page, or `PUT /api/settings/:namespace` with that organization active). A value meant for every organization belongs on a key declared at scope `global`.

packages/qa/dogfood/test/upgrade-18x-harness.dogfood.test.ts

@@ -399,6 +399,26 @@ describe(`[#24884] an 18.0.0 database (fixture from ${gaManifest.ref} @ ${gaMani
     expect(rowsOf(await res.json()).map((r) => String(r?.title))).toContain(`${FIXTURE_NAMES.task} — edited`);
   }, 240_000);
 
+  // [ADR-0131 D8] Its settings rows carry their organization, and no reach
+  // reads a row written with none: they still answer, through the admin's
+  // Setup door and for a process-wide reader that names no organization (the
+  // email brand name, the anonymous runtime config), which reads as the one
+  // organization D9 derives.
+  it('[ADR-0131 D8] its settings still answer: through the Setup door, and for a reader that names no organization', async () => {
+    expect(stack, 'HEAD is booted').toBeDefined();
+    const adminTok = await stack!.signIn(FIXTURE_PEOPLE.admin.email, FIXTURE_PEOPLE.admin.password);
+    const res = await stack!.raw('/api/settings/branding', { headers: { Authorization: `Bearer ${adminTok}` } });
+    expect(res.status).toBe(200);
+    const body = (await res.json()) as { data: { values: Record<string, { value: unknown; source: string }> } };
+    expect(body.data.values.workspace_name).toMatchObject({ value: FIXTURE_NAMES.workspaceName, source: 'tenant' });
+
+    const settings = stack!.kernel.getService('settings') as {
+      get(namespace: string, key: string, ctx?: object): Promise<{ value: unknown; source: string }>;
+    };
+    expect(await settings.get('branding', 'workspace_name', {})).toMatchObject({ value: FIXTURE_NAMES.workspaceName, source: 'tenant' });
+    expect(await settings.get('localization', 'timezone', {})).toMatchObject({ value: 'UTC', source: 'tenant' });
+  }, 120_000);
+
   it('every 18.0.0 row of every reshaped table is still there, with its key columns', async () => {
     await stack?.stop();
     stack = undefined;

packages/services/service-settings/src/settings-getmany.test.ts

@@ -224,7 +224,9 @@ describe('[#10826] SettingsService.getMany', () => {
   // codec, which is why the fixture must already hold the decoded value.
   it('[#12172] resolves the LITERAL stored value — no doubled JSON encoding', async () => {
     const { svc } = await makeService();
-    const ctx = { userId: 'u1', tenantId: 't1' };
+    // No organization: the fixture's rows carry none, and an organization's
+    // read does not fall back to them (ADR-0131 D8).
+    const ctx = { userId: 'u1' };
     const many = await svc.getMany('localization', ['timezone', 'locale', 'currency'], ctx);
 
     // Global row, user-scope key, no user/tenant override.

packages/services/service-settings/src/settings-organization-isolation.pin.test.ts

@@ -19,15 +19,21 @@
  *  - identity: a tenant-scope or user-scope row carries the writing caller's
  *    organization, and a write by one organization neither reads, nor
  *    replaces, nor resets another's;
- *  - the cascade: a caller's own organization row is preferred over a row
- *    written with no organization, which stays every organization's fallback,
- *    and the global rung is read by every organization;
+ *  - the cascade: a caller reads its own organization's row and no other — a
+ *    row written with no organization is no organization's fallback (ADR-0131
+ *    D8), under a wall and under `single` alike — and the global rung is read
+ *    by every organization;
  *  - the lock pre-flight reads the same row the cascade does;
  *  - the refusal: under a walled posture a tenant-scope write that names no
  *    organization is refused whole, and writes nothing; under `single` it is
  *    not;
- *  - `single`: the default organization keeps its answers, including for a
- *    process-wide reader that names no organization;
+ *  - `single`: a process-wide reader that names no organization reads and
+ *    writes as the one organization ADR-0131 D9 derives, so the default
+ *    organization keeps its answers and a second organization-less write
+ *    replaces the first instead of inserting beside it; where D9 derives none,
+ *    such a reader has no tenant rung and its tenant write is refused;
+ *  - a composition with no organization object (the lean embedding) keeps
+ *    storing and reading its rows with no organization;
  *  - the posture is asked only for a caller that names no organization, on a
  *    key below the global rung — and when it cannot be read, that read or
  *    write fails rather than guessing.
@@ -43,7 +49,7 @@ import { ObjectQL } from '@objectstack/objectql';
 import { SysUser } from '@objectstack/platform-objects/identity';
 import { SysPlatformSetting, SysSetting } from '@objectstack/platform-objects/system';
 import type { TenancyPosture } from '@objectstack/spec/security';
-import { SettingsService } from './settings-service.js';
+import { SettingsService, type SettingsOrganizationSource } from './settings-service.js';
 import { wrapEngineAsSettingsEngine } from './settings-service-plugin.js';
 import { SettingsLockedError, type SettingsContext } from './settings-service.types.js';
 
@@ -139,12 +145,30 @@ beforeEach(async () => {
   userId = String((user as any).id);
 });
 
-/** A service bound to the real engine, under the given posture. */
-function settings(posture: TenancyPosture | undefined): SettingsService {
+/**
+ * ADR-0131 D9's answers, as the plugin's source hands them to the service: the
+ * one organization derived, none derivable, or no organization object at all.
+ */
+const derived = (organizationId: string): SettingsOrganizationSource =>
+  async () => ({ kind: 'derived', organizationId });
+const NONE_DERIVABLE: SettingsOrganizationSource =
+  async () => ({ kind: 'refuse', reason: 'no-organization', organizationCount: 0 });
+const NO_ORGANIZATION_OBJECT: SettingsOrganizationSource = async () => ({ kind: 'no-organization-object' });
+
+/**
+ * A service bound to the real engine, under the given posture and with the
+ * given answer for a caller that names no organization — by default a
+ * composition with no organization object, which is what this rig registers.
+ */
+function settings(
+  posture: TenancyPosture | undefined,
+  organization: SettingsOrganizationSource = NO_ORGANIZATION_OBJECT,
+): SettingsService {
   const svc = new SettingsService({ env: {} });
   svc.registerManifest(MANIFEST);
   svc.bindEngine(wrapEngineAsSettingsEngine(engine as any), undefined, {
     tenancyPosture: () => posture,
+    organization,
   });
   return svc;
 }
@@ -221,24 +245,35 @@ describe('a tenant-scope value belongs to the organization that wrote it (walled
     }
   });
 
-  it('a row written with no organization is every organization\'s fallback, and an organization\'s own row is preferred over it', async () => {
+  it('a row written with no organization answers no organization, and an organization\'s write lands beside it', async () => {
     await seed({ key: 'motto', scope: 'tenant', value: 'Legacy', organization_id: null });
+    await seed({ key: 'theme', scope: 'user', user_id: userId, value: 'Legacy-dark', organization_id: null });
     const svc = settings('isolated');
 
     for (const org of [ORG_A, ORG_B]) {
       const got = await svc.get(NS, 'motto', inOrg(org));
-      expect(got.value).toBe('Legacy');
-      expect(got.source).toBe('tenant');
+      expect(got.value).toBe('none');
+      expect(got.source).toBe('default');
+      expect(got.cascadeChain?.some((e) => e.scope === 'tenant')).toBe(false);
+      // The user rung likewise: the same user's organization-less row is not
+      // their value in any organization.
+      expect((await svc.getMany(NS, ['theme'], inOrg(org, { userId }))).theme.value).toBe('light');
     }
 
     await svc.set(NS, 'motto', 'Alpha', inOrg(ORG_A));
-    const own = await svc.get(NS, 'motto', inOrg(ORG_A));
-    expect(own.value).toBe('Alpha');
-    // ONE tenant entry in the chain — the preferred row, not both.
-    expect(own.cascadeChain?.filter((e) => e.scope === 'tenant')).toHaveLength(1);
-    expect((await svc.get(NS, 'motto', inOrg(ORG_B))).value).toBe('Legacy');
-    // The organization-less row was not rewritten by the organization's write.
-    expect(settingRows().find((r) => r.organization_id == null)?.value).toBe('Legacy');
+    expect((await svc.get(NS, 'motto', inOrg(ORG_A))).value).toBe('Alpha');
+    expect((await svc.get(NS, 'motto', inOrg(ORG_B))).source).toBe('default');
+    // The organization's write did not find, and rewrite, the organization-less row.
+    expect(
+      settingRows().filter((r) => r.key === 'motto').map((r) => [r.organization_id ?? null, r.value]).sort(),
+    ).toEqual([[null, 'Legacy'], [ORG_A, 'Alpha']]);
+  });
+
+  it('a lock on a row written with no organization locks no organization\'s write', async () => {
+    await seed({ key: 'theme', scope: 'tenant', value: 'dark', organization_id: null, locked: true });
+    const svc = settings('isolated');
+    await expect(svc.set(NS, 'theme', 'blue', inOrg(ORG_A, { userId }))).resolves.toBeDefined();
+    expect((await svc.get(NS, 'theme', inOrg(ORG_A, { userId }))).value).toBe('blue');
   });
 
   it('a user-scope value carries the organization too: the same user reads it only in the organization it was set in', async () => {
@@ -283,23 +318,40 @@ describe('a tenant-scope write that names no organization', () => {
     });
   }
 
-  it("is not refused under the 'single' posture, nor where no posture is reported", async () => {
+  it("is not refused under the 'single' posture, nor where no posture is reported: it lands in the one organization D9 derives", async () => {
     for (const posture of ['single', undefined] as const) {
-      const svc = settings(posture);
+      const svc = settings(posture, derived(ORG_A));
       await svc.set(NS, 'motto', `free-${String(posture)}`, {});
       expect((await svc.get(NS, 'motto', {})).value).toBe(`free-${String(posture)}`);
+      expect((await svc.get(NS, 'motto', inOrg(ORG_A))).value).toBe(`free-${String(posture)}`);
     }
-    expect(settingRows().every((r) => r.organization_id == null)).toBe(true);
+    // One row, the organization's: the second write found the first.
+    expect(settingRows().map((r) => [r.organization_id, r.value])).toEqual([[ORG_A, 'free-undefined']]);
   });
 
-  it('under a walled posture, a reader that names no organization reads only the organization-less rows', async () => {
-    const svc = settings('isolated');
-    await svc.set(NS, 'motto', 'Alpha', inOrg(ORG_A));
-    expect((await svc.get(NS, 'motto', {})).source).toBe('default');
-
-    await seed({ key: 'motto', scope: 'tenant', value: 'Legacy', organization_id: null });
-    expect((await svc.get(NS, 'motto', {})).value).toBe('Legacy');
+  it("is refused under 'single' where D9 derives no organization, and nothing is written", async () => {
+    const svc = settings('single', NONE_DERIVABLE);
+    const refusal = await svc.setMany(NS, { motto: 'Nobody' }, { userId }).then(() => null, (e) => e);
+    expect(refusal?.code).toBe('SETTINGS_VALIDATION');
+    expect(settingRows()).toHaveLength(0);
   });
+
+  for (const posture of ['isolated', 'group'] as const) {
+    it(`under the '${posture}' posture, a reader that names no organization reads no tenant or user row — neither an organization's nor one written with none`, async () => {
+      const svc = settings(posture);
+      await svc.set(NS, 'motto', 'Alpha', inOrg(ORG_A));
+      await svc.set(NS, 'theme', 'dark', inOrg(ORG_A, { userId }));
+      expect((await svc.get(NS, 'motto', {})).source).toBe('default');
+
+      await seed({ key: 'motto', scope: 'tenant', value: 'Legacy', organization_id: null });
+      await seed({ key: 'theme', scope: 'user', user_id: userId, value: 'Legacy-dark', organization_id: null });
+      const got = await svc.get(NS, 'motto', {});
+      expect(got.source).toBe('default');
+      expect(got.cascadeChain?.some((e) => e.scope === 'tenant')).toBe(false);
+      expect((await svc.get(NS, 'theme', { userId })).source).toBe('default');
+      expect((await svc.getNamespace(NS, { userId })).values.motto.source).toBe('default');
+    });
+  }
 });
 
 describe('the posture is read only where it decides something', () => {
@@ -332,25 +384,44 @@ describe('the posture is read only where it decides something', () => {
 describe("posture 'single': the default organization keeps its answers", () => {
   const DEFAULT_ORG = 'org_default';
 
-  it('a value stored before rows carried an organization is still read by the default organization', async () => {
+  it('a row stored with no organization is not the default organization\'s, for either reader', async () => {
     await seed({ key: 'motto', scope: 'tenant', value: 'Acme', organization_id: null });
-    const svc = settings('single');
-    const got = await svc.get(NS, 'motto', inOrg(DEFAULT_ORG));
-    expect(got.value).toBe('Acme');
-    expect(got.source).toBe('tenant');
+    await seed({ key: 'theme', scope: 'user', user_id: userId, value: 'Acme-dark', organization_id: null });
+    const svc = settings('single', derived(DEFAULT_ORG));
+    for (const ctx of [inOrg(DEFAULT_ORG), {}]) {
+      const got = await svc.get(NS, 'motto', ctx);
+      expect(got.value).toBe('none');
+      expect(got.source).toBe('default');
+      expect((await svc.get(NS, 'theme', { ...ctx, userId })).value).toBe('light');
+    }
   });
 
   it('the default organization\'s write is read by itself and by a process-wide reader that names no organization', async () => {
     await seed({ key: 'motto', scope: 'tenant', value: 'Acme', organization_id: null });
-    const svc = settings('single');
+    const svc = settings('single', derived(DEFAULT_ORG));
     await svc.set(NS, 'motto', 'Acme Corp', inOrg(DEFAULT_ORG));
 
     expect((await svc.get(NS, 'motto', inOrg(DEFAULT_ORG))).value).toBe('Acme Corp');
     expect((await svc.get(NS, 'motto', {})).value).toBe('Acme Corp');
   });
 
+  it('a process-wide writer that names no organization writes the default organization\'s row, and its second write replaces the first', async () => {
+    const svc = settings('single', derived(DEFAULT_ORG));
+    await svc.set(NS, 'motto', 'one', {});
+    await svc.set(NS, 'motto', 'two', {});
+    await svc.set(NS, 'theme', 'dark', { userId });
+
+    expect((await svc.get(NS, 'motto', {})).value).toBe('two');
+    expect((await svc.get(NS, 'motto', inOrg(DEFAULT_ORG))).value).toBe('two');
+    expect((await svc.get(NS, 'theme', inOrg(DEFAULT_ORG, { userId }))).value).toBe('dark');
+    expect(settingRows().map((r) => [r.scope, r.organization_id, r.value]).sort()).toEqual([
+      ['tenant', DEFAULT_ORG, 'two'],
+      ['user', DEFAULT_ORG, 'dark'],
+    ]);
+  });
+
   it('the default organization\'s reset reads back the default for both readers', async () => {
-    const svc = settings('single');
+    const svc = settings('single', derived(DEFAULT_ORG));
     await svc.set(NS, 'motto', 'Acme Corp', inOrg(DEFAULT_ORG));
     await svc.runAction(NS, 'reset', null, inOrg(DEFAULT_ORG));
 
@@ -359,6 +430,17 @@ describe("posture 'single': the default organization keeps its answers", () => {
   });
 });
 
+describe('a composition with no organization object (the lean embedding, ADR-0131 D9 note)', () => {
+  it('stores and reads its rows with no organization, which no organization reads', async () => {
+    const svc = settings('single', NO_ORGANIZATION_OBJECT);
+    await svc.set(NS, 'motto', 'Lean', {});
+    await svc.set(NS, 'motto', 'Lean again', {});
+    expect((await svc.get(NS, 'motto', {})).value).toBe('Lean again');
+    expect(settingRows().map((r) => [r.organization_id ?? null, r.value])).toEqual([[null, 'Lean again']]);
+    expect((await svc.get(NS, 'motto', inOrg(ORG_A))).source).toBe('default');
+  });
+});
+
 describe('the in-memory store keeps the same identity', () => {
   it('two organizations each read their own value with no engine bound', async () => {
     const svc = new SettingsService({ env: {} });