Skip to content

Commit 79c35d4

Browse files
fix(service-settings)!: settings rows carry the caller's organization, and the data API read of the settings stores applies each namespace's readPermission (#22295)
Fixes #22261 Clause-②: no (narrowing) Executes option A as ruled on the card. This body stays at the level of classes, positions and functions, as the card asks; the detailed measurement went to the dispatching seat privately. ## What changes ### `SettingsService` (`packages/services/service-settings/src/settings-service.ts`) The service reads and writes `sys_setting` under its own system context, and that context names no organization. So no driver tenant scope and no organization wall reaches those calls. The organization now travels in the service's own query and row identity. - **Row identity.** `rowIdentity` keys a `tenant` or `user` row by the identity `sys_setting` declares, organization included. `setMany` writes every such row with the caller's organization (`SettingsContext.tenantId`). A `global` row is unchanged: it lives in `sys_platform_setting` and has no organization column. - **Reads.** `loadScopedRows` filters explicitly, in the query itself, by the caller's organization plus rows stored with no organization. The in-memory store applies the same reach (`OrganizationReach`). The global rung (`loadGlobalRows`) is unchanged. - **Cascade.** `preferredRow` makes the tenant and user rungs take the caller organization's own row ahead of an organization-less one. The lock pre-flight in `setMany` reads the same row, so a lock applies only where the cascade reads. - **Refusal.** Under a walled posture (`group` / `isolated`), a tenant-scope write that names no organization is refused whole, before anything is written. It reuses the vocabulary the ownerless user-key refusal already has: `SettingsValidationError`, `code: SETTINGS_VALIDATION`, HTTP 400 at the settings routes, one field entry per key with `code: invalid_value` and `constraint: { scope: 'tenant' }`. A reset is refused the same way. - **Posture.** The posture comes from the `tenancy` service. The plugin passes it through `bindEngine` (`tenancyPosture`), read the same way its HTTP door reads it. The service only asks when the caller names no organization. ### The generic read door (`settings-read-door.ts`, registered by `SettingsServicePlugin`) - An engine middleware registered by object name on `sys_setting`, `sys_setting_audit` and `sys_platform_setting`. It ANDs a `namespace` predicate into every non-system read (`find`, `findOne`, `count`, `aggregate`). It is a filter, not a pass over the result, so counts, aggregates and pages see exactly the rows a list returns. - The predicate is `SettingsService.namespaceReadScope`. It uses the same `requiredCapability` table the settings door enforces. A namespace with no registered manifest reads at the default capability, `setup.access`. - **Seam (H4).** The seam sits inside `service-settings`. No file in `objectql`, `runtime` or `plugin-security` is edited. ### Census page `content/docs/permissions/system-context.mdx` gains row 18b for the middleware's `isSystem` read. `check:system-context-census` requires a row for every elevation read site. The counts were regenerated with `pnpm gen:system-context-census`. ## Posture `single` The default organization keeps the answers it had. These are pinned in `settings-organization-isolation.pin.test.ts`: - a value stored before rows carried an organization is still read by the default organization; - the default organization's new write is read by itself and by a process-wide reader that names no organization; - a reset reads back the default for both; - an organization-less tenant-scope write is not refused under `single`, nor where no posture is reported. ## Existing rows (H3) No stored row is rewritten, as the dispatch fences. What stored rows carry today, and the decision that follows from it, went to the seat privately. ## Tests Every run in this table is on HEAD `61911cf17a`, after `service-settings` was rebuilt and its `dist/` was proven to carry the HEAD source. | Suite | Result | |:--|:--| | `pnpm --filter @objectstack/service-settings exec vitest run --maxWorkers=2` | 41 files, 752 passed | | `settings-organization-isolation.pin.test.ts` (part of the suite above) | 18 passed | | `settings-read-door.pin.test.ts` (part of the suite above) | 27 passed | | `test/settings-organization-isolation.dogfood.test.ts`, real stack over HTTP, two organizations under a non-degraded `isolated` posture | 6 passed | | `single`-posture HTTP regression: the existing dogfood files that write and read settings (`settings-config-change-audit`, `analytics-timezone`, `audit-log-parent-read-gate`) | 3 files, 14 passed | | `pnpm --filter @objectstack/service-settings typecheck` | exit 0 | | `pnpm --filter @objectstack/dogfood typecheck` | exit 0 | ### Over HTTP, with a positive control per refusal - Each organization sets and reads its own tenant-scope value. One organization's write and its reset leave the other's value unchanged. A `global` value is read by both. - An organization-less tenant-scope write under the walled posture answers `400` with `error.code` `SETTINGS_VALIDATION` and a field entry `invalid_value`, and writes nothing. Control: the identical write from inside an organization answers 200. - The data-API read of `sys_setting` hides a namespace's row from a principal lacking that namespace's `readPermission`: the list returns 0 rows and the by-id read answers 404. Control 1: the same principal reads its own row of a namespace whose capability it holds. Control 2: a holder of the withheld capability reads the withheld row (200). ### Ablations Each ablation started from a committed fix, restored from `HEAD` under a trap, and was proven restored by blob hash and an empty `git diff HEAD`. All ablations ran at `e1407dc55f`, except the two dogfood rows, which ran at `672e54e08e`. | Mutation | Result | |:--|:--| | `settings-service.ts` set to the base commit | isolation pin 11 red, 5 green. The 5 that stay green are the regression guards: the global row, and the three `single` cases plus its no-refusal control. | | `namespaceReadScope` answers no predicate | read-door pin 19 red, 8 green. The 8 green: system context, registration by name, writes untouched, refusal of a read with no query. | | `preferredRow` made positional | isolation pin 2 red (both preference cases) | | Dogfood, service and plugin set to the base commit, package rebuilt, `ablation-dist-preflight --absent` passed | 4 of 6 red. Green: the guard, and the global row read by both. Restored, rebuilt, marker present again: 6 of 6 green. | | Dogfood, only the door predicate disabled, plant proven in `dist/` | only the read-door case red (1 of 6). Restored, rebuilt, `--absent` passed, tree clean. | ### Gates - **Derived set.** `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 94 commands at HEAD `61911cf17a`. All 94 were run and exit 0, plus `check:settings-bind-window` as dispatched. - **Reconciliation.** `--ran` reports 94 derived, 94 run, 0 NOT-MEASURED, 0 UNRUN. - **Fixed on the way.** `check:system-context-census` went red on the first pass for the new read site. It is green after the row-18b edit. - **Lint, narrowed.** The 9 changed `.ts` files lint with 0 errors and 0 warnings at `61911cf17a` (`eslint --no-inline-config --format json`). The repository runs no type-aware lint, so this change cannot move an untouched file's lint verdict. The full lint run is CI's. - **Base.** The branch sits on `c8bb3c8d`. `origin/main` gained 4 commits since, and none of them touches `service-settings`. CI on the merge ref is the arbiter. ## Acceptance notes - **Changeset**: one `@objectstack/service-settings` `minor` changeset, declared breaking, with ADR-0087 disposition `not-required (no-migration-prescription)`. `check-adr-0087-registration` and `check-changeset-no-major` are green. - **Fence**: - `settings-service.types.ts` changes only `SettingsRow` (the `organization_id` row-shape field). PR #22266 edits a different region of it. - `settings-service-plugin.ts` is touched for wiring only: the posture source and the read-door registration. - Two existing test fixtures were triaged. `settings-getmany.test.ts` rows now spell `organization_id: null` the way a real driver returns it. `settings-routes.test.ts` passes the reach its private `loadRows` call now takes. - **Out of scope, not changed here**: - The organization attribution of the settings-specific audit trail rows belongs to #15207's family (audit ledgers); that card remains open. - **For later**: `sys-setting.object.ts` (platform-objects) quotes a `loadRows` comment sentence this change retires. The quote is prose only; no gate reads it. > Seat's append (`domain:services` seat 1, #6021), carried verbatim from the dev's round-2 report `6060893787`; the dev never edits a PR body. ## Round 2 - **Merge.** `origin/main` `d1dbe70ebd` is merged with a merge commit (`dcbf66b41a`), with no rebase and no force-push. The only conflict was the derived counts on `content/docs/permissions/system-context.mdx`. Both rows are kept, 18b from this branch and 23d from main, and the counts were regenerated with `pnpm gen:system-context-census`. `check:system-context-census` is green: 118 elevation read sites in 20 packages across 55 files. - **Re-verified at `dcbf66b41a`.** All results below follow a rebuild of what the merge touched. | Check | Result | |:--|:--| | `service-settings` suite | 41 files, 752 passed | | `settings-organization-isolation.dogfood.test.ts` | 6 passed | | single-posture settings dogfood files | 3 files, 14 passed | | typecheck of `service-settings` and `dogfood` | exit 0 | - **Gates.** `dispatch-gates --commands`, run with no paths, derives 94 commands at `dcbf66b41a` with no stale-tree warning. All 94 were run, plus `check:settings-bind-window`. `--ran` reports 94 derived, 94 run, 0 NOT-MEASURED and 0 UNRUN. Two gates first refused with exit 3 on unbuilt packages; they were re-run after those packages were built and exit 0. - **Measurement.** Under the isolated posture, with the real tenant wall in the composition, a non-system read of `sys_setting` or `sys_setting_audit` by one organization's administrator does not return another organization's organization-stamped row. The harness is a temporary test, removed after the run. --- _Generated by [Claude Code](https://claude.ai/code/session_01WkL6Eijt432S1Y7ekb6ovQ)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6729e10 commit 79c35d4

11 files changed

Lines changed: 1318 additions & 55 deletions
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
'@objectstack/service-settings': minor
3+
---
4+
5+
fix(service-settings)!: tenant- and user-scope settings rows carry the caller's organization, and the data API read of the settings stores applies each namespace's readPermission
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) A runtime narrowing inside SettingsService and its plugin, not a metadata change: no spec key, export, option, response field or stored shape is removed, renamed or re-shaped (`organization_id` is the column `sys_setting` already declares in its row identity), so there is no tombstone and nothing for `objectstack migrate meta` to rewrite. What narrows is the service's accept set (a tenant-scope write naming no organization under a walled posture is refused) and the generic read door's row set (a namespace's rows are withheld from a principal lacking its readPermission). The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id covers this service and this diff adds none (not registered / already-registered); and no published interface or type is removed or narrowed (not runtime-interface-only / type-surface-only). -->
10+
11+
**BREAKING** (an accept-set narrowing), shipped as `minor` under the launch-window convention for breaking changes.
12+
13+
`sys_setting` declares its row identity as `(organization_id, namespace, key, scope, user_id)`. `SettingsService` now carries the organization in that identity itself, on every read and write of a `tenant` or `user` row, because it reads and writes the store under its own system context, which no driver tenant scope or organization wall reaches.
14+
15+
- **Writes.** A `tenant` or `user` row is written with the caller's organization (`SettingsContext.tenantId`) in its key and in its stored `organization_id`. A write by one organization updates only that organization's row.
16+
- **Reads.** The tenant and user rungs draw on the caller organization's rows and on rows stored with no organization, and take the caller organization's own row when it has one. A row stored with no organization stays the fallback for every organization until that organization writes its own. A caller that names no organization reads only the rows stored with no organization under a walled posture (`group`, `isolated`), and every row under `single`. The global rung (`sys_platform_setting`) is unchanged and read by every organization.
17+
- **Locks.** The lock check on a write reads the same upper rows the caller's cascade reads, so a lock on one organization's tenant row locks nothing for another organization.
18+
- **Refused now.** Under a walled posture, `set` and `setMany` refuse a key declared `scope: 'tenant'` when the context names no organization, a `null` reset of one included. The refusal is a `SettingsValidationError` (`code: 'SETTINGS_VALIDATION'`, HTTP 400 at the settings routes) with one `fields` entry per such key (`code: 'invalid_value'`, `constraint: { scope: 'tenant' }`). It refuses the whole batch, before anything is written. The posture is the one the `tenancy` service reports; `SettingsServicePlugin` supplies it through `bindEngine`.
19+
- **Generic read door.** `SettingsServicePlugin` registers an engine middleware on `sys_setting`, `sys_setting_audit` and `sys_platform_setting`: every non-system read (`find`, `findOne`, `count`, `aggregate`) is narrowed to the namespaces whose `readPermission` the principal holds, by the same rule `GET /api/settings/:namespace` applies. A namespace with no registered manifest reads at the default capability, `setup.access`.
20+
- **Unchanged.** Under `single`, a caller in the default organization reads every value it read before, and a process-wide reader that names no organization reads the organization's current value. Keys declared at `scope: 'global'` resolve and write the same for every caller.
21+
22+
What changes for you: write a tenant-scope setting from inside the organization it belongs to (an active organization on the session, or `SettingsContext.tenantId` in process). Rows already stored with no organization are not rewritten.

‎content/docs/permissions/system-context.mdx‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ the seed loader replaying package fixtures, a plugin's boot reconciler, a
1010
service self-write, a migration.
1111

1212
This page is **the authority** for what that flag actually does. It exists
13-
because the flag is not one concept: it is a single boolean read at **117
14-
distinct sites across 19 packages**, and knowing three of those behaviours gives
13+
because the flag is not one concept: it is a single boolean read at **118
14+
distinct sites across 20 packages**, and knowing three of those behaviours gives
1515
no hint that the other hundred-and-four exist. Every documented app-side bug
1616
traced to `isSystem` had the same shape — the metadata was complete and correct,
1717
and the gap was observable only by querying the resulting rows.
@@ -115,6 +115,7 @@ that silently does not happen.
115115
| 16 | **Read-audit rows are not written** | plugin-audit | Lose: the "a person opened this record" trail. `sudo()` keeps the caller's `userId`, so this flag is the only thing separating a human read from a platform one | `packages/plugins/plugin-audit/src/read-audit.ts#installReadAuditWriter` |
116116
| 17 | Approval snapshot payload redaction skipped, and so is the snapshot query guard | plugin-approvals | Get: the whole snapshot on `find` / `findOne` — the audit/replay channel — and a filter, sort or grouping by it on any read. Lose: field-visibility redaction over approval payloads, and the refusal of a query over the snapshot for a reader withheld a field of the objects it can reach | `packages/plugins/plugin-approvals/src/payload-redaction-middleware.ts#bindSnapshotRedactionMiddleware`, `packages/plugins/plugin-approvals/src/payload-predicate-guard.ts#bindSnapshotPredicateGuard` |
117117
| 18 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `packages/rest/src/rest-server.ts#enforceAuth` |
118+
| 18b | Settings namespace read scope not applied | service-settings | Get: a read of `sys_setting`, `sys_setting_audit` or `sys_platform_setting` across every namespace, whatever each namespace's `readPermission` names — the settings service's own reads of its stores take this path. Lose: the per-namespace read gate the generic data API applies to every other caller, the same rule the settings routes enforce | `packages/services/service-settings/src/settings-read-door.ts#settingsReadDoorMiddleware` |
118119

119120
### 2. Write pipeline and data integrity
120121

@@ -140,7 +141,7 @@ that silently does not happen.
140141

141142
### 3. Sharing (`plugin-sharing`)
142143

143-
The largest single consumer — **17 of the 117 sites**.
144+
The largest single consumer — **17 of the 118 sites**.
144145

145146
| # | Behaviour when `isSystem` | What you get / what you lose | Anchor |
146147
|:--|:---|:---|:---|
@@ -283,8 +284,8 @@ Ownership injection, `readonly` bypass and sharing materialisation are
283284
independent decisions, and a seed loader plausibly wants the first two but not
284285
the third. The concept is nevertheless **staying as one boolean**:
285286

286-
- **Shipped semantics.** `isSystem` is a published contract with 117 read sites
287-
in 19 packages. Splitting it is a breaking contract change across all of them.
287+
- **Shipped semantics.** `isSystem` is a published contract with 118 read sites
288+
in 20 packages. Splitting it is a breaking contract change across all of them.
288289
(The ruling was taken when the census read 80 sites in 18 packages; the count
289290
has grown, which strengthens rather than weakens the argument.)
290291
- **No business pull.** No app has asked for the combinations a split would
@@ -357,16 +358,16 @@ still holds equal to the census on every pull request:
357358
| Appearances of the bare identifier `isSystem` in non-test sources | 813 | — |
358359
| — parsed as a declaration | 27 | ✅ |
359360
| — parsed as an object-literal / type key (producers and option objects) | 310 | — |
360-
| — parsed as a property **read** | 123 | ✅ |
361+
| — parsed as a property **read** | 124 | ✅ |
361362
| — parsed in some other syntactic position (a local, a cast, a conditional) | 9 | ✅ |
362363
| — the remainder: text inside comments and string literals | 358 | — |
363364
| Of those reads: reads of one of the unrelated metadata fields | 6 | ✅ |
364-
| Of those reads: reads of `ExecutionContext.isSystem` | **117** | ✅ |
365-
| — behaviour-bearing (rows 1–61 above) | 114 | ✅ |
365+
| Of those reads: reads of `ExecutionContext.isSystem` | **118** | ✅ |
366+
| — behaviour-bearing (rows 1–61 above) | 115 | ✅ |
366367
| — carry the flag onward only (rows 62–64 above) | 3 | ✅ |
367-
| Packages containing at least one elevation read | **19** | ✅ |
368-
| Files containing at least one elevation read | 54 | ✅ |
369-
| — the distinct symbols those reads live in — what this page anchors | 100 | ✅ |
368+
| Packages containing at least one elevation read | **20** | ✅ |
369+
| Files containing at least one elevation read | 55 | ✅ |
370+
| — the distinct symbols those reads live in — what this page anchors | 101 | ✅ |
370371
| — of those files, the ones holding more than one read in one symbol | 8 | ✅ |
371372

372373
The six rows marked — are a **dated decomposition, not a live claim**: they were
@@ -430,7 +431,7 @@ same resolver, and the same registration shape, that holds `docs/adr/**`.
430431
Renaming a symbol is now a loud red instead of a silent misdirection.
431432

432433
⚠️ **The precision that costs, priced here rather than buried.** A symbol anchor
433-
cannot say WHICH read inside a function it means, and **8** of the **54**
434+
cannot say WHICH read inside a function it means, and **8** of the **55**
434435
anchored files hold more than one read inside a single symbol. So the population
435436
check runs per file at symbol granularity: every file the census finds a read in
436437
must be anchored, and the set of symbols this page cites into that file must

0 commit comments

Comments
 (0)