Skip to content

Commit 2ea6042

Browse files
fix(plugin-auth)!: a user preference's owner is fixed when it is saved (#29685)
<!-- claude-projects-attribution: {"github_login":"os-warren"} --> _Requested by **Warren** · [project thread](https://claude.ai/code/project/chan_01U5RdnAAmkSQmZbqh6FtqZ8?thread=cmsg_01U5RdnAAmkSQmZbqh6FtqZ82oFrZdNaDiqbKbJU8Jh3ja)_ Fixes #29630 Clause-②: no (narrowing) Metadata-protocol: none Skills-docs: none — no skill or doc teaches changing a saved preference's owner; the one page that names the rule, `content/docs/permissions/system-context.mdx` row 26c, is updated here ## 维护者速读 - 改了什么:`sys_user_preference` 加一条保存规则:更新时若把 `user_id` 改成另一个用户,一律 403 `PERMISSION_DENIED`(系统写除外),与 #29575 给 `sys_personal_view` 的规则同形;两个对象现在共用 plugin-auth 里的一份检查 `owner-fixed-rule.ts`。 - 为什么改:用户偏好属于保存它的那个用户,规则此前没有对这个对象成立。 - 风险与回滚:只收窄一种写入(改偏好的属主);只改 `value` 的写入、回显原属主的写入、系统写入都不受影响。回滚即 revert 本 PR。 - 先看:`packages/plugins/plugin-auth/src/owner-fixed-rule.ts`(共用检查)、`user-preference-hooks.ts`(注册与对象判定)、`personal-view-hooks.ts`(改为调用共用检查,文案不变)。 - 两分钟验证:`pnpm --filter @objectstack/dogfood exec vitest run --maxWorkers=2 test/user-preference-owner-fixed.dogfood.test.ts`,预期 7/7 通过,三种调用者各一条 403。 - 需要你做的:无,席位落地。 ## What changes - **New rule on `sys_user_preference`.** A non-system update that sends a `user_id` other than the stored one is refused `403 PERMISSION_DENIED`, for a member, an organization administrator and a platform administrator alike. The refusal names the row and the `user_id` column and echoes no other id. An update that echoes the stored `user_id`, or does not send it, saves as before; a system write is not judged. - **One shared check.** `packages/plugins/plugin-auth/src/owner-fixed-rule.ts` (`isOwnerChange`, `ownerFixedError`, `assertOwnerUnchanged`) now holds the rule for both `sys_personal_view` and `sys_user_preference`. `personal-view-hooks.ts` calls it instead of its inline copy; its behaviour and message are byte-identical. Not exported from a published entry. - **Registration.** `user-preference-hooks.ts` registers one `beforeUpdate` hook on `sys_user_preference`, in `AuthPlugin` and in `IdentityObjectsPlugin`, beside the personal-view hooks, so the rule travels with the table. - **Docs.** `content/docs/permissions/system-context.mdx` row 26c covers both objects and anchors on the shared check (register regenerated). - **Changeset.** `.changeset/29630-user-preference-owner-fixed.md`: `@objectstack/plugin-auth` patch, BREAKING banner, both declaration lines, ADR-0087 `not-required`. Resumed from `claude/project-thread-rq4y7m@34568e1` (the previous card thread's hand-off), merged with `origin/main` `7dae3b3c`. The last commit trims the shared check's header and the changeset's "Who sees it" line to the rule alone. ## Why The card asks for the `sys_personal_view` shape (#29575) on this object: a user's preferences are their own UI state, and their owner is fixed when they are saved. ## Tests On `424e9295`: - `packages/plugins/plugin-auth/src/user-preference-hooks.test.ts`: registration; a member, an org admin and a platform admin are refused 403; the message names the row and column and no other id; clearing the owner is a change of owner; controls: echo, value-only update, system write, another object. - `packages/qa/dogfood/test/user-preference-owner-fixed.dogfood.test.ts` (REST): member, org admin and platform admin cannot hand their own preference to another user (403, the row keeps its owner); controls: echo saves; the owner reads, lists and updates their own preference. - `pnpm --filter @objectstack/plugin-auth build && test && typecheck` under the lock: VERDICT command-exit 0, Test Files 216 passed (216), Tests 3703 passed, 10 skipped. - Full dogfood battery (`pnpm --filter @objectstack/dogfood test`, the `(narrowing)` arm) under the lock on the full build: VERDICT command-exit 0, Test Files 400 passed, 3 skipped (403); Tests 3304 passed, 28 skipped (3332). - Ablations (previous card thread, on `3bbd5da8`, each red then restored with the blob equal to HEAD and `git diff HEAD` empty): dropping the rule call reds the 5 unit refusal pins and keeps the 5 controls green; making the shared check always false reds 6 pins across both objects; guarding the registration (plugin-auth rebuilt, dist-preflight marker present, then `--absent` after restore) reds the member and org-admin REST pins. The platform-admin REST pin stays green under that last ablation because an existing write check already refuses that caller's write there; the unit pin holds the rule on its own for that caller. Since then only comments and the changeset changed in tested files. ## Acceptance notes - `Clause-②: no (narrowing)`: one accepted write is now refused, and no public export or published type moves. - The ablation readings are carried from the hand-off report on the card; the tested code is unchanged since. ## Verification <details><summary>Derived gates on 424e929</summary> - Full build under the lock (`pnpm build --concurrency=2`): VERDICT command-exit 0, Tasks 117 successful, 117 total. History unshallowed for the teardown self-test fixture. - `node scripts/pm/dispatch-gates.mjs --commands` (9 paths vs merge base `7dae3b3c`): 95 families. Each run once, stdin from /dev/null, exit captured before any pipe: 95 exit 0, including `check-plugin-teardown-shape.mjs --self-test` (48 cases), `spec check:skill-examples` (282 prose examples), `check:dual-build-cjs-loads` (115 entry points across 70 packages), `check:system-context-census` and `check:type-check-debt` (0 raw tsc errors above the ledger). - `dispatch-gates --ran`: 95 derived, 95 run, 0 NOT-MEASURED, 0 UNRUN. - `pnpm lint`: exit 0. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01JQwxbWZsNmNDugrjfzMoJW)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent c76cdeb commit 2ea6042

9 files changed

Lines changed: 405 additions & 33 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
'@objectstack/plugin-auth': patch
3+
---
4+
5+
fix(plugin-auth)!: a user preference's owner is fixed when the preference is saved
6+
7+
Clause-②: no (narrowing) — a non-system update of a `sys_user_preference` row that sends a `user_id` other than the stored one is refused `403 PERMISSION_DENIED`, which some callers' writes were admitted before. Nothing is widened.
8+
Metadata-protocol: none
9+
10+
<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and no export or type changes: a save rule on one platform object refuses a change of the row's owner. The refusal is a 403 a caller cannot work around by rewriting metadata, so there is nothing for a migration to rewrite and no tombstone. -->
11+
12+
**BREAKING** accept-set narrowing at the generic data door for `sys_user_preference` (`@objectstack/plugin-auth`).
13+
14+
- **Now.** A preference belongs to the user it was saved for. An update that sends a different `user_id` is refused `403 PERMISSION_DENIED`, for every caller but the system: a member, an organization administrator and a platform administrator alike. The refusal names the row and the `user_id` column. The row is unchanged.
15+
- **Unchanged.** An update that echoes the stored `user_id`, or does not send it, saves as before. The console's preference writer updates `value` only and creates rows in the caller's own name, so it is not affected. A system write (the platform's own writers) is not judged.
16+
- **Shared rule.** `sys_personal_view` and `sys_user_preference` now hold this with one check, `owner-fixed-rule.ts`. The personal view's behaviour and message are unchanged.
17+
- **Who sees it.** A client that changes the `user_id` of a saved preference.
18+
19+
**The remedy.** Save a new preference in the other user's name, as that user, instead of moving one.

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ bugs — it is the protection or the side effect that silently does not happen.
143143
| 25 | Engine-owned / append-only write guard bypassed | plugin-security | Get: generic writes to `managedBy` engine-owned objects | `packages/plugins/plugin-security/src/system-write-guard.ts#isUserContextWrite`, `packages/plugins/plugin-security/src/system-write-guard.ts#assertEngineOwnedWriteAllowed` |
144144
| 26 | Identity write guard bypassed (ADR-0092) | plugin-auth | Get: direct writes to identity tables through the generic data path | `packages/plugins/plugin-auth/src/identity-write-guard.ts#isUserContextWrite` |
145145
| 26b | API key restore guard does not refuse | plugin-auth | Get: a write setting `revoked: false` on a `sys_api_key` row an administrator's forced password reset revoked is admitted without the platform-administrator check a user-context restore must pass; the reason is still cleared. Lose: nothing a caller reaches: no system writer restores a key, and the mint, the last-used stamp and the forced reset itself never set `revoked: false` | `packages/plugins/plugin-auth/src/api-key-restore-guard.ts#isUserContextWrite` |
146-
| 26c | **Personal-view owner rule does not refuse** — a `sys_personal_view` update that changes `user_id` is admitted | plugin-auth | Get: a system writer can move a personal view to another owner (no shipped system writer does). Lose: nothing a caller reaches. A non-system update whose `user_id` differs from the stored owner is refused `403 PERMISSION_DENIED`, for every caller, a platform administrator included; an echo of the stored owner is no change. The rule exists because, for an organization member, `member_default`'s `'*'` update floor (`created_by == current_user.id`) applies to an update in place of the object's own row scope | `packages/plugins/plugin-auth/src/personal-view-hooks.ts#registerPersonalViewHooks` |
146+
| 26c | **Per-user owner rule does not refuse** — a `sys_personal_view` or `sys_user_preference` update that changes `user_id` is admitted | plugin-auth | Get: a system writer can move a personal view or a preference to another owner (no shipped system writer does). Lose: nothing a caller reaches. A non-system update whose `user_id` differs from the stored owner is refused `403 PERMISSION_DENIED`, for every caller, a platform administrator included; an echo of the stored owner is no change. One shared check holds it for both objects. The rule exists because, for an organization member, `member_default`'s `'*'` update floor (`created_by == current_user.id`) applies to an update beside the object's own row scope | `packages/plugins/plugin-auth/src/owner-fixed-rule.ts#isOwnerChange` |
147147
| 27 | Search-companion column **kept** in a read's rows when it was explicitly requested | objectql | Get: the internal companion column is readable. Lose: nothing for app code — this is the engine reading its own index | `stripSearchCompanionFromRead` |
148148
| 28 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `dependentCountIsDisclosable` |
149149
| 29 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `recordReferenceCheckElevation` |
@@ -575,7 +575,7 @@ and CI refuses one that differs from what the tree produces.
575575
| `packages/plugins/plugin-auth/src/api-key-restore-guard.ts#isUserContextWrite` | 1 | 26b |
576576
| `packages/plugins/plugin-auth/src/auth-plugin.ts#start` | 1 | 12 |
577577
| `packages/plugins/plugin-auth/src/identity-write-guard.ts#isUserContextWrite` | 1 | 26 |
578-
| `packages/plugins/plugin-auth/src/personal-view-hooks.ts#registerPersonalViewHooks` | 1 | 26c |
578+
| `packages/plugins/plugin-auth/src/owner-fixed-rule.ts#isOwnerChange` | 1 | 26c |
579579
| `packages/plugins/plugin-auth/src/principal-session-mint.ts#refuseUpFront` | 1 | 61b |
580580
| `packages/plugins/plugin-email/src/email-template-door.ts#isOrganizationWrite` | 1 | 59 |
581581
| `packages/plugins/plugin-security/src/flow-system-identity-write-refusal.ts#elevationSource` | 1 | 1b |

‎packages/plugins/plugin-auth/src/auth-plugin.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ import {
6969
import { registerLastAdminGuard } from './last-admin-guard.js';
7070
import { registerApiKeyRestoreGuard } from './api-key-restore-guard.js';
7171
import { registerPersonalViewHooks } from './personal-view-hooks.js';
72+
import { registerUserPreferenceHooks } from './user-preference-hooks.js';
7273
import { personalViewCallerResolver, registerPersonalViewRoutes, PERSONAL_VIEW_REMOVE_ROUTE, type PersonalViewEngine } from './personal-view-routes.js';
7374
import { registerMembershipEndedSessionTrigger } from './membership-ended-session.js';
7475
import { registerMemberRoleCanonicalization } from './member-role-canonical.js';
@@ -1872,6 +1873,12 @@ export class AuthPlugin implements Plugin {
18721873
packageId: 'com.objectstack.plugin-auth.personal-view',
18731874
logger: ctx.logger,
18741875
});
1876+
// A user's own preferences: the owner (user_id) is fixed when the row
1877+
// is saved, for every caller but the system. See user-preference-hooks.ts.
1878+
registerUserPreferenceHooks(engine, {
1879+
packageId: 'com.objectstack.plugin-auth.user-preference',
1880+
logger: ctx.logger,
1881+
});
18751882
// [cloud ADR-0024 D5.2] Break-glass — the same identity write
18761883
// chokepoints, guarding a different question: not "may this caller
18771884
// write identity tables" (above, and system writes bypass it by

‎packages/plugins/plugin-auth/src/identity-objects-plugin.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ import { resolveTenancyPosture } from '@objectstack/types';
6464
import { ensureDefaultOrganizationExists } from './default-organization-invariant.js';
6565
import { authIdentityManifest } from './manifest.js';
6666
import { registerPersonalViewHooks } from './personal-view-hooks.js';
67+
import { registerUserPreferenceHooks } from './user-preference-hooks.js';
6768

6869
/** The kernel plugin name of {@link IdentityObjectsPlugin}. */
6970
export const IDENTITY_OBJECTS_PLUGIN_NAME = 'com.objectstack.auth.identity-objects';
@@ -123,11 +124,12 @@ export class IdentityObjectsPlugin implements Plugin {
123124
* plugin keeps it. A failure throws and fails the boot, never warn-only.
124125
*/
125126
async start(ctx: PluginContext): Promise<void> {
126-
// The manifest above registers `sys_personal_view`; its two write rules
127-
// travel with it, exactly as `AuthPlugin` registers them.
127+
// The manifest above registers `sys_personal_view` and `sys_user_preference`;
128+
// their write rules travel with them, exactly as `AuthPlugin` registers them.
128129
const engine = ctx.getService<{ registerHook?: unknown }>('objectql');
129130
if (engine && typeof engine.registerHook === 'function') {
130131
registerPersonalViewHooks(engine, { packageId: `${IDENTITY_OBJECTS_PLUGIN_NAME}.personal-view` });
132+
registerUserPreferenceHooks(engine, { packageId: `${IDENTITY_OBJECTS_PLUGIN_NAME}.user-preference` });
131133
}
132134
if (postureInForce(ctx) !== 'single') return;
133135
const invariant = await ensureDefaultOrganizationExists(ctx.getService<IDataEngine>('objectql'), {
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* **The owner of a per-user row is fixed when the row is saved.** The one
5+
* spelling of that rule, shared by `sys_personal_view`
6+
* (`personal-view-hooks.ts`) and `sys_user_preference`
7+
* (`user-preference-hooks.ts`), whose owner column is `user_id`.
8+
*
9+
* An update that sends an owner other than the stored one is refused
10+
* `403 PERMISSION_DENIED`, for every caller but the system: a member, an
11+
* organization administrator and a platform administrator alike. An echo of
12+
* the stored owner is no change, and saves.
13+
*
14+
* A save rule is the one place that judges the CHANGE of owner; why the row
15+
* scope alone does not is row 26c of `content/docs/permissions/system-context.mdx`.
16+
*
17+
* The refusal names the row and the owner column, and never the id the write
18+
* sent.
19+
*/
20+
21+
/** The string form two stored ids are compared by (an echo of the stored owner is no change). */
22+
function sameId(a: unknown, b: unknown): boolean {
23+
const norm = (v: unknown) => (v === undefined || v === null || v === '' ? null : String(v));
24+
return norm(a) === norm(b);
25+
}
26+
27+
const sets = (data: unknown, key: string): boolean =>
28+
!!data && typeof data === 'object' && Object.prototype.hasOwnProperty.call(data, key);
29+
30+
/**
31+
* Whether a `beforeUpdate` hook context is a non-system write handing a row to
32+
* another owner: it sends `ownerField` with a value other than the stored one
33+
* (`ctx.previous`, present on the by-id path and per row on a multi-row
34+
* update). A system write is never one: the platform's own writers are
35+
* trusted to mean it.
36+
*/
37+
export function isOwnerChange(ctx: any, ownerField = 'user_id'): boolean {
38+
if (ctx?.session?.isSystem === true) return false;
39+
const data = ctx?.input?.data;
40+
const previous = ctx?.previous;
41+
if (!sets(data, ownerField) || !previous || typeof previous !== 'object') return false;
42+
return !sameId((data as Record<string, unknown>)[ownerField], (previous as Record<string, unknown>)[ownerField]);
43+
}
44+
45+
/** How a refusal names the row it guards. */
46+
export interface OwnerFixedSubject {
47+
/** The row's kind as a sentence starts it: `Personal view`, `User preference`. */
48+
label: string;
49+
/** The row's kind inside a sentence: `view`, `preference`. */
50+
noun: string;
51+
}
52+
53+
/** The refusal for an update that would hand a row to another user: `PERMISSION_DENIED` / `403`. */
54+
export function ownerFixedError(subject: OwnerFixedSubject, id: unknown, ownerField = 'user_id'): Error {
55+
const { label, noun } = subject;
56+
const err = new Error(
57+
`${label} ${String(id ?? '')} was not changed: a ${noun}'s owner (${ownerField}) is fixed when the ${noun} is saved, `
58+
+ `and nobody can hand a ${noun} to another user. Save a new ${noun} in that user's name instead. `
59+
+ `The ${noun} is unchanged.`,
60+
) as Error & { code?: string; status?: number };
61+
err.code = 'PERMISSION_DENIED';
62+
err.status = 403;
63+
return err;
64+
}
65+
66+
/** Throw {@link ownerFixedError} when {@link isOwnerChange} holds for `ctx`. */
67+
export function assertOwnerUnchanged(ctx: any, subject: OwnerFixedSubject, ownerField = 'user_id'): void {
68+
if (isOwnerChange(ctx, ownerField)) {
69+
throw ownerFixedError(subject, ctx?.previous?.id ?? ctx?.input?.id, ownerField);
70+
}
71+
}

‎packages/plugins/plugin-auth/src/personal-view-hooks.ts‎

Lines changed: 6 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,8 @@
2020
*
2121
* **The owner is fixed when the view is saved.** An update that sends a
2222
* `user_id` other than the stored one is refused `403 PERMISSION_DENIED`, for
23-
* every caller but the system. The row scope alone does not hold this: for an
24-
* organization member, `member_default`'s `'*'` update floor
25-
* (`created_by == current_user.id`) is a held update policy, so the object's
26-
* declared default does not apply to an update, and the post-image of a
27-
* re-parent still carries the member's own `created_by`.
23+
* every caller but the system. The rule, and why the row scope alone does not
24+
* hold it, is `owner-fixed-rule.ts`'s, shared with `sys_user_preference`.
2825
*
2926
* Which view opens by default is not this object's: the reader's existing
3027
* per-object pin holds a view id (seat ruling 6587007982 on #29443).
@@ -37,6 +34,7 @@
3734
import { ListViewSchema } from '@objectstack/spec/ui';
3835
import { zodIssuesToFields } from '@objectstack/spec/api';
3936
import { validationFailure } from '@objectstack/types';
37+
import { assertOwnerUnchanged } from './owner-fixed-rule.js';
4038

4139
/** The object these hooks guard. */
4240
export const PERSONAL_VIEW_OBJECT = 'sys_personal_view';
@@ -95,22 +93,8 @@ export function assertPersonalViewConfig(config: unknown, objectName?: unknown):
9593
);
9694
}
9795

98-
/** The string form two stored ids are compared by (an echo of the stored owner is no change). */
99-
function sameId(a: unknown, b: unknown): boolean {
100-
const norm = (v: unknown) => (v === undefined || v === null || v === '' ? null : String(v));
101-
return norm(a) === norm(b);
102-
}
103-
104-
/** The refusal for an update that would hand a view to another user: `PERMISSION_DENIED` / `403`. */
105-
export function ownerChangeError(id: unknown): Error {
106-
const err = new Error(
107-
`Personal view ${String(id ?? '')} was not changed: a view's owner (user_id) is fixed when the view is saved, `
108-
+ 'and nobody can hand a view to another user. Save a new view in that user\'s name instead. The view is unchanged.',
109-
) as Error & { code?: string; status?: number };
110-
err.code = 'PERMISSION_DENIED';
111-
err.status = 403;
112-
return err;
113-
}
96+
/** How the shared owner rule names this object's rows. */
97+
const PERSONAL_VIEW_SUBJECT = { label: 'Personal view', noun: 'view' } as const;
11498

11599
const sets = (data: Record<string, unknown> | null | undefined, key: string): boolean =>
116100
!!data && typeof data === 'object' && Object.prototype.hasOwnProperty.call(data, key);
@@ -133,14 +117,7 @@ export function registerPersonalViewHooks(engine: any, opts: PersonalViewHooksOp
133117
// The row as it will be: what the write sends, over what is stored
134118
// (`ctx.previous`, present on the by-id path and per row on a multi-row update).
135119
const previous = (ctx.previous ?? undefined) as Record<string, unknown> | undefined;
136-
if (
137-
ctx?.session?.isSystem !== true
138-
&& sets(data, 'user_id')
139-
&& previous
140-
&& !sameId(data!.user_id, previous.user_id)
141-
) {
142-
throw ownerChangeError(previous.id ?? ctx.input?.id);
143-
}
120+
assertOwnerUnchanged(ctx, PERSONAL_VIEW_SUBJECT);
144121
const sendsConfig = sets(data, CONFIG_FIELD);
145122
if (!sendsConfig && !sets(data, 'object_name')) return;
146123
const config = sendsConfig ? data![CONFIG_FIELD] : previous?.[CONFIG_FIELD];

0 commit comments

Comments
 (0)