Skip to content

Commit 5597ed9

Browse files
committed
fix(service-settings)!: the user rung answers only its owner; a user-scoped write must name its user
The read half: resolveKeyFromRows takes the caller's user id and answers the user rung only for a row whose user_id equals it. A resolve whose context names no user has no user rung and falls through to the tenant rung, then global, then the default. get and getMany thread the id; getNamespace, createClient, runAction and resetNamespace reach the cascade through them. The write half: setMany refuses a user-scoped key when the context names no user, with a SettingsValidationError (invalid_value, constraint scope user) collected in the pre-flight, before any write. Pins on both halves over the engine-bound and in-memory stores, with per-user and tenant/global controls. Minor changeset, declared narrowing. Claude-Session: https://claude.ai/code/session_01WkL6Eijt432S1Y7ekb6ovQ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0e9371f commit 5597ed9

3 files changed

Lines changed: 361 additions & 12 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
'@objectstack/service-settings': minor
3+
---
4+
5+
fix(service-settings)!: the user rung answers only its owner, and a write of a user-scoped key must name its user
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) A runtime narrowing inside SettingsService, not a metadata change: no spec key, export, option, response field or stored shape is removed, renamed or re-shaped, so there is no tombstone and nothing for `objectstack migrate meta` to rewrite. What narrows is the service's accept set: `set` and `setMany` now refuse a key declared `scope: 'user'` when the caller's context names no user. 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 changes (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+
A settings key declared `scope: 'user'` belongs to one user. `SettingsService` now holds that on both its read and its write side.
14+
15+
- **Reads.** The user rung answers only a caller whose `SettingsContext.userId` is present and equals the row's owner. A resolve whose context names no user (no `userId`, or an empty one) has no user rung: it answers from the tenant rung, then global, then the manifest default. This holds on every read path that reaches the cascade: `get`, `getMany`, `getNamespace`, `createClient`, `runAction` and `resetNamespace`.
16+
- **Refused now.** `set` and `setMany` refuse a user-scoped key when the context names no user, a `null` reset of one included. The refusal is a `SettingsValidationError` (`code: 'SETTINGS_VALIDATION'`) with one `fields` entry per such key (`code: 'invalid_value'`, `constraint: { scope: 'user' }`). It refuses the whole batch, before anything is written.
17+
- **Unchanged.** A caller with a user id reads and writes only their own rows, as before. Keys declared at `scope: 'tenant'` or `'global'` resolve and write the same with or without a user id. The settings HTTP routes resolve a signed-in user before they read or write any row, so they behave as before.
18+
19+
What changes for you: when your code calls the settings service for a user-scoped key, pass the user id of the person the value belongs to. A value meant for everyone belongs on a key declared at `scope: 'tenant'` or `'global'`. None of the settings namespaces this repository ships declares a user-scoped key.

‎packages/services/service-settings/src/settings-service.ts‎

Lines changed: 84 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,21 @@ const PLATFORM_SETTING_OBJECT = 'sys_platform_setting';
106106
*/
107107
const SETTINGS_SYSTEM_CONTEXT = Object.freeze({ isSystem: true as const });
108108

109+
/**
110+
* The caller's user id, or `null` when the context names no user.
111+
*
112+
* The user rung is per owner: a `scope: 'user'` row belongs to the user its
113+
* `user_id` names. So the rung answers a resolve only for that same user
114+
* ({@link SettingsService.resolveKeyFromRows}), and a write of a user-scoped
115+
* key needs a user to pin the row to ({@link SettingsService.setMany} refuses
116+
* one without). An absent or empty id names nobody, and both read as "no user"
117+
* here: the read falls through to the tenant rung, then global, then the
118+
* default, and the write is refused.
119+
*/
120+
function callerUserIdOf(ctx: SettingsContext): string | null {
121+
return typeof ctx.userId === 'string' && ctx.userId !== '' ? ctx.userId : null;
122+
}
123+
109124
/**
110125
* Value-bearing specifier types — drives which entries we expect to
111126
* find in the K/V store. Keeps the resolver in sync with the spec
@@ -1283,8 +1298,10 @@ export class SettingsService {
12831298
const scope = reg.scopes.get(key)!;
12841299
// For 'user' scope we pre-filter by user_id; for 'tenant' and 'global'
12851300
// we load everything for the namespace and pick the right row below.
1286-
const rows = await this.loadRows(namespace, scope === 'user' ? ctx.userId ?? null : null);
1287-
return this.resolveKeyFromRows<T>(reg, key, scope, rows);
1301+
// The user rung's pick compares the owner too (see `resolveKeyFromRows`).
1302+
const userId = scope === 'user' ? callerUserIdOf(ctx) : null;
1303+
const rows = await this.loadRows(namespace, userId);
1304+
return this.resolveKeyFromRows<T>(reg, key, scope, rows, userId);
12881305
}
12891306

12901307
/**
@@ -1379,18 +1396,19 @@ export class SettingsService {
13791396
// [ADR-0131 D7] One load per required `loadRows` argument, as before —
13801397
// and the global rung, which no argument changes, is read ONCE for both
13811398
// (see `loadRowSets`).
1399+
const userId = callerUserIdOf(ctx);
13821400
const groups: Array<string | null> = [
1383-
...(userKeys.length > 0 ? [ctx.userId ?? null] : []),
1401+
...(userKeys.length > 0 ? [userId] : []),
13841402
...(otherKeys.length > 0 ? [null] : []),
13851403
];
13861404
const sets = await this.loadRowSets(namespace, groups);
13871405
const userRows = userKeys.length > 0 ? sets[0] : [];
13881406
const otherRows = otherKeys.length > 0 ? sets[sets.length - 1] : [];
13891407
for (const { key, scope } of userKeys) {
1390-
out[key] = await this.resolveKeyFromRows(reg, key, scope, userRows);
1408+
out[key] = await this.resolveKeyFromRows(reg, key, scope, userRows, userId);
13911409
}
13921410
for (const { key, scope } of otherKeys) {
1393-
out[key] = await this.resolveKeyFromRows(reg, key, scope, otherRows);
1411+
out[key] = await this.resolveKeyFromRows(reg, key, scope, otherRows, null);
13941412
}
13951413
}
13961414
return out;
@@ -1401,12 +1419,18 @@ export class SettingsService {
14011419
* verbatim from {@link get} for #10826 so `get`, `getMany` and
14021420
* `getNamespace` share ONE implementation of the resolution order
14031421
* (env is handled by the callers BEFORE the row load, exactly as before).
1422+
*
1423+
* `userId` is the caller's ({@link callerUserIdOf}), or `null` for a caller
1424+
* that names no user and for every key not declared `scope: 'user'`. The user
1425+
* rung answers only a row whose `user_id` equals it; with `null` there is no
1426+
* user rung at all.
14041427
*/
14051428
private async resolveKeyFromRows<T = unknown>(
14061429
reg: RegisteredManifest,
14071430
key: string,
14081431
scope: SpecifierScope,
14091432
rows: SettingsRow[],
1433+
userId: string | null,
14101434
): Promise<ResolvedSettingValue<T>> {
14111435
// 2. cascade walk — OS_* env (handled by callers) > global > tenant > user > default
14121436
//
@@ -1438,8 +1462,12 @@ export class SettingsService {
14381462
}
14391463
}
14401464

1441-
if (scope === 'user') {
1442-
const userRow = rows.find((r) => r.key === key && r.scope === 'user');
1465+
// The user rung answers its OWNER only. The pick compares `user_id` itself
1466+
// rather than trusting the row set to be the caller's alone, and a caller
1467+
// that names no user gets no user rung: the walk falls through to tenant,
1468+
// global, then the default, exactly as if no user row existed.
1469+
if (scope === 'user' && userId !== null) {
1470+
const userRow = rows.find((r) => r.key === key && r.scope === 'user' && r.user_id === userId);
14431471
if (userRow) {
14441472
chain.push({
14451473
scope: 'user',
@@ -1674,6 +1702,30 @@ export class SettingsService {
16741702
return null;
16751703
}
16761704

1705+
/**
1706+
* The per-key entry {@link setMany} refuses a user-scoped key with when the
1707+
* caller names no user. `invalid_value` is ADR-0114's declared slot for
1708+
* "rejected for a reason no other member names" (the closed catalog has no
1709+
* member for a missing owner, and a service does not invent one); the
1710+
* constraint carries the declared scope, spelled as the specifier property
1711+
* it comes from.
1712+
*/
1713+
private ownerlessUserKeyError(reg: RegisteredManifest, key: string): FieldError {
1714+
const spec = ((reg.manifest.specifiers ?? []) as Array<Record<string, unknown>>)
1715+
.find((s) => s.key === key);
1716+
const label = typeof spec?.label === 'string' ? spec.label : key;
1717+
return {
1718+
field: key,
1719+
code: 'invalid_value',
1720+
message:
1721+
`${label} is a per-user setting, and this write names no user to store it for. ` +
1722+
'Pass the user id of the person the value belongs to (`SettingsContext.userId`); ' +
1723+
"a value meant for everyone belongs on a key declared at scope 'tenant' or 'global'.",
1724+
label,
1725+
constraint: { scope: 'user' },
1726+
};
1727+
}
1728+
16771729
/** Persist a single key. Throws SettingsLockedError when env-locked. */
16781730
async set(
16791731
namespace: string,
@@ -1684,7 +1736,16 @@ export class SettingsService {
16841736
return (await this.setMany(namespace, { [key]: value }, ctx))[key];
16851737
}
16861738

1687-
/** Persist multiple keys atomically (best-effort). */
1739+
/**
1740+
* Persist multiple keys atomically (best-effort).
1741+
*
1742+
* A key declared `scope: 'user'` is written for one user, the caller
1743+
* ({@link callerUserIdOf}). A batch that writes one with no user id is
1744+
* refused WHOLE, before any write, with a {@link SettingsValidationError}
1745+
* naming each such key (`invalid_value`, `constraint: { scope: 'user' }`):
1746+
* the row it would store names no owner, and the user rung answers only its
1747+
* owner, so no reader would ever see it while the write reported success.
1748+
*/
16881749
async setMany(
16891750
namespace: string,
16901751
patch: Record<string, unknown>,
@@ -1701,7 +1762,10 @@ export class SettingsService {
17011762
// missing — the write path previously trusted a spoofable header identity).
17021763
this.assertPermitted(reg.manifest, 'write', ctx);
17031764

1704-
// Pre-flight: reject the whole batch if any key is locked or unknown.
1765+
// Pre-flight: reject the whole batch if any key is locked or unknown, or
1766+
// is user-scoped while the caller names no user (collected, refused below).
1767+
const callerUserId = callerUserIdOf(ctx);
1768+
const ownerless: FieldError[] = [];
17051769
for (const key of Object.keys(patch)) {
17061770
if (!reg.scopes.has(key)) throw new UnknownKeyError(namespace, key);
17071771
// An env override pins the key against writes — but only one that is IN
@@ -1720,7 +1784,11 @@ export class SettingsService {
17201784
// scope as the lock is still permitted (i.e. a platform admin
17211785
// can edit a globally-locked value; a tenant admin cannot).
17221786
const scope = reg.scopes.get(key)!;
1723-
const rows = await this.loadRows(namespace, scope === 'user' ? ctx.userId ?? null : null);
1787+
if (scope === 'user' && callerUserId === null) {
1788+
ownerless.push(this.ownerlessUserKeyError(reg, key));
1789+
continue;
1790+
}
1791+
const rows = await this.loadRows(namespace, scope === 'user' ? callerUserId : null);
17241792
const upper = rows.find(
17251793
(r) =>
17261794
r.key === key &&
@@ -1731,6 +1799,9 @@ export class SettingsService {
17311799
throw new SettingsLockedError(namespace, key, `locked-by-${upper.scope}`);
17321800
}
17331801
}
1802+
if (ownerless.length > 0) {
1803+
throw new SettingsValidationError(namespace, ownerless);
1804+
}
17341805

17351806
// Reject writes that would leave the namespace in a known-broken
17361807
// state (visible required field empty / pattern mismatch). See
@@ -1761,9 +1832,10 @@ export class SettingsService {
17611832
const scope = reg.scopes.get(key)!;
17621833
// global rows are deployment-wide and land in `sys_platform_setting`,
17631834
// which has no organization and no user column (ADR-0131 D7, see
1764-
// `rowIdentity`); user rows pin to ctx.userId; tenant rows leave user_id
1835+
// `rowIdentity`); user rows pin to the caller's user id, which the
1836+
// pre-flight above guarantees is present; tenant rows leave user_id
17651837
// null and let the engine's tenant scoping fill in tenant_id from ctx.
1766-
const userId = scope === 'user' ? ctx.userId ?? null : null;
1838+
const userId = scope === 'user' ? callerUserId : null;
17671839
const isEncrypted = reg.encryptedKeys.has(key);
17681840
const isNull = rawValue === null || typeof rawValue === 'undefined';
17691841

0 commit comments

Comments
 (0)