Skip to content

Commit 6f35b74

Browse files
fix(plugin-audit, service-storage)!: a row's author or uploader is read-only after creation, like the comment's display name (#29868)
security#1 · os-justin · session_01QSeKYn5CmmdShCJifm6WYj Clause-②: no (narrowing) Metadata-protocol: none Skills-docs: `content/docs/permissions/attachments-access.mdx` now states that a row's uploader (`sys_attachment.uploaded_by`, `sys_file.owner_id`) is read-only after creation and that an attachment create with no signed-in user is refused. No published skill teaches writing `author_id`, `uploaded_by` or `owner_id` (`git grep` over `skills/` finds none), and no doc teaches writing a comment's `author_id`. ## 维护者速读 - **改了什么**:评论作者(`sys_comment.author_id`)、附件上传者(`sys_attachment.uploaded_by`)与文件上传者(`sys_file.owner_id`)在创建后只读,与评论显示名同一规则;没有登录用户的评论或附件创建被拒绝(403)。系统上下文写入不变。 - **为什么改**:一条记录的出处(谁写的、谁传的)只在创建时记录一次,之后的编辑不改变它。 - **风险与回滚**:收窄。编辑请求里带的这三列不落库(仍答 200,`droppedFields` 报告),持有所有权转移授权的管理员经数据接口写 `sys_file.owner_id` 同样不落库;系统上下文创建评论时漏写 `author_id` 不再被引擎拦下。控制台无需改动。回滚即 revert 本 PR。 - **先看**:`packages/plugins/plugin-audit/src/comment-access-hooks.ts`(创建钩子的作者盖章与无用户拒绝)、`packages/services/service-storage/src/attachment-access-hooks.ts`(同样的盖章与拒绝,及更新段的规则注释)、三个对象定义里新加的 `readonly: true`。 - **两分钟验证**:`pnpm --filter @objectstack/dogfood exec vitest run test/comment-author-display-identity.dogfood.test.ts test/attachments-permission-matrix.dogfood.test.ts`,预期 `Test Files 2 passed (2)`、`Tests 30 passed | 1 skipped (31)`。 - **需要你做的**:无,席位落地。 ## What changes A row's author or uploader is read-only after creation, like the comment's display name. - **Update.** `sys_comment.author_id` (`@objectstack/plugin-audit`), `sys_attachment.uploaded_by` (`@objectstack/platform-objects`) and `sys_file.owner_id` (`@objectstack/service-storage`) declare `readonly: true`. The engine's update-side strip drops a non-system caller's value and reports it in `droppedFields` and `X-ObjectStack-Dropped-Fields`; the rest of the write lands and the write is not refused. This is the mechanism the comment's display name already uses. - **Create.** The comment and attachment create hooks refuse a non-system create whose session names no signed-in user, with 403 `RECORD_NOT_ACCESSIBLE`, the answer a comment reaction with no signed-in user already gets. A signed-in create is stamped with the session user as before. The `sys_file` upload doors already answer a request with no signed-in user 401 `AUTH_REQUIRED` and take `owner_id` from the session only, so they are unchanged. - **Kept.** System-context writes, and a historical import's update (`preserveAudit`, which only the system principal may start), keep the values they supply. - **Comments.** The attachment kit's update-side note now cites this rule and gives provenance integrity as its reason. - **Docs.** `content/docs/permissions/attachments-access.mdx`. ## Tests All readings are on head `c2de5ab501` unless a line says otherwise. - `packages/plugins/plugin-audit/src/comment-author-display-identity.integration.test.ts`: a new top-level describe on the same rig (real kernel, ObjectQL, AuditPlugin, SQLite; the boot moved to module scope so both describes share it). The author's own edit and a parent editor's edit naming `author_id` leave it unchanged and report it with reason `readonly`. Controls: an ordinary edit lands and drops nothing; a system write keeps the author it supplies; a historical import's update (`preserveAudit`) keeps it. A create whose session names no user is refused (`code` and `status`) and stores nothing. - `packages/plugins/plugin-audit/src/comment-access-hooks.test.ts` and `packages/services/service-storage/src/attachment-access-hooks.test.ts`: the create hook refuses a session with no user (`code` and `status`); the attachment hook refuses it before the parent is judged. - `packages/qa/dogfood/test/comment-author-display-identity.dogfood.test.ts`: a new top-level describe at the REST data door (the boot moved to module scope, one stack for both describes). A PATCH naming `author_id` answers 200, `droppedFields` names it, the body edit lands and the served author is unchanged. Control: a system write keeps the author it supplies. - `packages/qa/dogfood/test/attachments-permission-matrix.dogfood.test.ts`: a new top-level describe beside the matrix that pins both create stamps. The uploader's PATCH of its `sys_attachment` naming `uploaded_by` answers 200, `droppedFields` names it and the description lands; an administrator's PATCH of a `sys_file` naming `owner_id` answers 200, `droppedFields` names it and the name lands. Control: a system write keeps the uploader it supplies, on both rows. <details><summary>Package suites and typechecks</summary> ``` head c2de5ab (origin/main 97f8f6f merged), each part's exit captured before any pipe pnpm --filter @objectstack/plugin-audit test exit 0 Test Files 69 passed (69) Tests 1127 passed (1127) pnpm --filter @objectstack/plugin-audit typecheck exit 0 pnpm --filter @objectstack/service-storage test exit 0 Test Files 71 passed (71) Tests 1327 passed (1327) pnpm --filter @objectstack/service-storage typecheck exit 0 pnpm --filter @objectstack/platform-objects test exit 0 Test Files 110 passed (110) Tests 1512 passed (1512) pnpm --filter @objectstack/platform-objects typecheck exit 0 pnpm --filter @objectstack/dogfood typecheck exit 0 targeted dogfood (the two files above, after the closure build): Test Files 2 passed (2) Tests 30 passed | 1 skipped (31) ``` </details> <details><summary>Full dogfood battery (the title carries `!`)</summary> ``` c2de5ab · os-verify-lock VERDICT command-exit 0 · Test Files 407 passed | 3 skipped (410) · Tests 3364 passed | 28 skipped (3392) · no failures ``` </details> <details><summary>Gates</summary> `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` on c2de5ab derived 94 commands from the 12 changed paths; all 94 ran locally with each exit code captured before any pipe. 92 exited 0 on the first run. `pnpm --filter @objectstack/spec run check:skill-examples` and `pnpm check:dual-build-cjs-loads` first exited 3 (PREREQUISITE NOT MET: packages outside this diff had no `dist/`, nothing measured); after building those packages both exited 0. ``` dispatch-gates --ran: Run reconciliation — 94 derived, 94 run, 0 NOT-MEASURED, 0 UNRUN. EXIT CODES — all 94 accounted famil(ies) carry one ``` Every family CI runs is read from this pull's check-runs; none is re-run here. </details> ## Acceptance notes - `sys_comment.author_id` keeps `required: true`, but the engine's record validator does not enforce `required` on a `readonly` field. A system-context create that omits it is now stored with no author instead of refused, the posture `sys_file`'s storage-owned `key` and `status` already record. No in-repo system writer creates comments. - `sys_file.owner_id` is also the column the security layer reads as a record's owner. A holder of the ownership-transfer grant is a non-system caller like any other here: its data-door write of the column is stripped and reported, because the column records who uploaded the file. A system-context write still sets it. - The refusal of a create with no signed-in user is pinned at the hook and engine layers. Through the composed stack's REST door such a request is answered 401 before any hook runs. - The console's comment edit sends `{ body, mentions }`, and its attachments panel sends `uploaded_by` only on create, so no UI change is needed. - Not run locally, declared to CI: `test:repo` (the vitest `repo` project), which the derived list does not name. ## Verification <details><summary>Ablation (one shot, all three declarations and both refusals)</summary> One shot, on 7971a64 (the merge of main that followed touched none of these packages). Five sites were mutated in nested `scripts/ablation-replace.mjs` WRAP layers, each anchor hit once and each blob changed: the three `readonly: true` declarations removed, and both create hooks returned to stamping only when a user is present. The three packages were rebuilt, and `scripts/ablation-dist-preflight.mjs` read each of the five markers in built `.js` (exit 0 each). service-storage's declaration step exited 1 on TS6133, the constant the ablation left unused; its JavaScript was emitted before that step, as the preflight shows. ``` mutated: plugin-audit unit + integration Tests 4 failed | 76 passed (80) service-storage unit Tests 1 failed | 150 passed (151) dogfood (the two files) Tests 3 failed | 27 passed | 1 skipped (31) the 8 red are the 8 rule pins; every control stayed green restored: 5 blobs == HEAD, git diff HEAD empty, porcelain clean; rebuilt (exit 0); preflight --absent exit 0 for all 5 markers plugin-audit Tests 80 passed (80) service-storage Tests 151 passed (151) dogfood Tests 30 passed | 1 skipped (31) ``` </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01QSeKYn5CmmdShCJifm6WYj)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2bf71ce commit 6f35b74

12 files changed

Lines changed: 455 additions & 125 deletions
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
'@objectstack/plugin-audit': patch
3+
'@objectstack/service-storage': patch
4+
'@objectstack/platform-objects': patch
5+
---
6+
7+
fix(plugin-audit, service-storage)!: a row's author or uploader is read-only after creation, like the comment's display name
8+
9+
Clause-②: no (narrowing) — on a write by a non-system caller, `sys_comment.author_id`, `sys_attachment.uploaded_by` and `sys_file.owner_id` are read-only on update and dropped (reported in `droppedFields`), and a comment or attachment create whose session names no signed-in user is refused. Nothing is widened.
10+
Metadata-protocol: none
11+
12+
<!-- adr-0087: not-required (no-migration-prescription) No metadata moves: no spec key, authorable spelling, export or stored shape is removed, renamed or re-shaped, and no stored row is read, rewritten, converted or dropped, so there is nothing for `objectstack migrate meta` to rewrite. What narrows is a runtime accept set on three platform-written columns of three platform objects, `sys_comment.author_id`, `sys_attachment.uploaded_by` and `sys_file.owner_id`, whose value is recorded once at creation; the remedy for a writer that needs another value recorded is a system-context write, which no ledger entry can derive. The other categories are closed on facts: the bumped packages publish (not unpublished); no ADR-0087 id covers these columns and this diff adds none (not registered, not already-registered); no export is added, removed or retyped, so no interface or type narrows (not runtime-export-retired, runtime-interface-only or type-surface-only). -->
13+
14+
**BREAKING** accept-set narrowing on `sys_comment`, `sys_attachment` and `sys_file` writes (`@objectstack/plugin-audit`, `@objectstack/platform-objects`, `@objectstack/service-storage`), served by every data door (REST, MCP, the console).
15+
16+
- **Now.** A row's author or uploader is recorded once, at creation, and is read-only after it, like the comment's display name. `sys_comment.author_id`, `sys_attachment.uploaded_by` and `sys_file.owner_id` declare `readonly: true`. On update a non-system caller's value is dropped and reported in `droppedFields` and the `X-ObjectStack-Dropped-Fields` header, the rest of the write lands, and the write is not refused. This holds for every non-system caller: the author's or uploader's own edit, a parent editor's, and, on `sys_file.owner_id`, a holder of the ownership-transfer grant.
17+
- **Refused.** A non-system create of a comment or an attachment whose session names no signed-in user is refused with 403 `RECORD_NOT_ACCESSIBLE`, as a comment reaction with no signed-in user already is: there is no user to record as its author or uploader. A signed-in create is unchanged: the session user is recorded, whatever the request carries.
18+
- **Unchanged.** System-context writes (seeds, imports, migrations) and a historical import's update (`preserveAudit`, which only the system principal may start) keep the values they supply. The upload doors already refuse a request with no signed-in user. Stored rows keep what they were written with. The console needs no change: it never sends these columns on an edit.
19+
- **Also.** `sys_comment.author_id` stays `required`, but as a `readonly` field its presence on a system-context create is the writer's to supply: the engine's record validator does not refuse a system create that omits it.
20+
21+
**The remedy.** None for the console. A client that sends one of these columns on an edit gets the same answer as before, with the column reported in `droppedFields`; to record a different author or uploader on an existing row, write it from a system context (a migration or an import). A server-side caller that creates comments or attachments with no signed-in user runs as the system principal or names the user it acts for.

‎content/docs/permissions/attachments-access.mdx‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,15 @@ depth, then, for a `controlled_by_parent` parent, the master-detail check
6666
(`ISecurityService.checkControlledByParentWrite`). A store fault keeps its own
6767
status (a `503` stays a `503`); it is never reported as a refusal.
6868
`uploaded_by` is **server-stamped** from the session; a client-supplied value
69-
is ignored.
69+
is ignored, and a create whose session names no signed-in user is refused. A
70+
row's uploader is read-only after creation, like a comment's author: an update
71+
drops a non-system caller's `uploaded_by` and reports it in `droppedFields`,
72+
and the rest of the write lands. System-context writes keep the value they
73+
supply.
7074

7175
| Code | Status | When |
7276
| --- | --- | --- |
77+
| `RECORD_NOT_ACCESSIBLE` | 403 | The session names no signed-in user to record as the uploader |
7378
| `ATTACHMENT_PARENT_ACCESS` | 403 | The parent record does not exist, or a by-id update of it by the caller would not be admitted (on a re-point, the new parent) |
7479
| `OBJECT_NOT_FOUND` | 404 | The `parent_object` names an object the registry does not hold — the answer a by-id update of it gets |
7580

@@ -174,7 +179,10 @@ rendering these files. Only a file marked `acl: 'public_read'` is served to a
174179
caller with no session: mark a file that way when it must render before sign-in.
175180

176181
The upload entry points (presigned / chunked) likewise require a session when
177-
an auth service is wired, and stamp `owner_id` on the new `sys_file`.
182+
an auth service is wired, and stamp `owner_id` on the new `sys_file`. The
183+
file's uploader is read-only after creation, as an attachment's is: an update
184+
drops a non-system caller's `owner_id` (a holder of the ownership-transfer
185+
grant included) and reports it in `droppedFields`.
178186

179187
## Storage-byte lifecycle
180188

‎packages/platform-objects/src/audit/sys-attachment.object.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,16 @@ export const SysAttachment = ObjectSchema.create({
9797
}),
9898

9999
// ── Authoring ──────────────────────────────────────────────
100+
// Platform-written: a row's uploader is read-only after creation, like a
101+
// comment's author. service-storage's create hook stamps the session user
102+
// (and refuses a non-system create whose session names no user);
103+
// `readonly` makes the engine's update-side strip drop a non-system
104+
// caller's value and report it (`droppedFields`). System writes keep the
105+
// value they supply.
100106
uploaded_by: Field.lookup('sys_user', {
101107
label: 'Uploaded By',
102108
required: false,
109+
readonly: true,
103110
group: 'System',
104111
}),
105112

‎packages/plugins/plugin-audit/src/comment-access-hooks.test.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,14 @@ describe('comment access — beforeInsert (parent readability + provenance)', ()
203203
expect(data.author_id).toBe('rep1');
204204
});
205205

206+
it('refuses a create whose session names no user: the author is the signed-in user, never the request\'s', async () => {
207+
const { beforeInsert } = install({});
208+
const data = { thread_id: 'crm_opportunity:opp1', body: 'hi', author_id: 'someone-else' };
209+
await expect(
210+
beforeInsert({ ...insertCtx(data, { visible: ['crm_opportunity/opp1'] }), session: { positions: ['org_member'] } }),
211+
).rejects.toMatchObject({ code: 'RECORD_NOT_ACCESSIBLE', status: 403 });
212+
});
213+
206214
it('bypasses for system context and context-less calls', async () => {
207215
const { beforeInsert } = install({});
208216
await expect(

‎packages/plugins/plugin-audit/src/comment-access-hooks.ts‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,13 @@
2222
* * `beforeInsert`: the caller must be able to READ the parent record —
2323
* verified with a caller-scoped `findOne`, so the parent's own
2424
* OWD/sharing, RLS and object-level CRUD all apply. `author_id` is
25-
* server-stamped from the session; a client-supplied value never wins.
26-
* So is the display identity beside it: `author_name` and
27-
* `author_avatar_url` are the session user's own name and avatar
28-
* ({@link CommentAuthorIdentityResolver}), never the client's, and both
29-
* are read-only on update (`objects/sys-comment.object.ts`).
25+
* server-stamped from the session; a client-supplied value never wins,
26+
* and a non-system create whose session names no user is refused. So is
27+
* the display identity beside it: `author_name` and `author_avatar_url`
28+
* are the session user's own name and avatar
29+
* ({@link CommentAuthorIdentityResolver}), never the client's. All three
30+
* are read-only on update (`objects/sys-comment.object.ts`): a row's
31+
* author is read-only after creation.
3032
* * `beforeUpdate` / `beforeDelete`: the caller must be the comment's
3133
* AUTHOR, or hold EDIT on the parent record (see "Parent EDIT" below) —
3234
* the attachment kit's uploader-or-parent-editor rule. A multi-row write
@@ -915,10 +917,17 @@ export function installCommentAccessHooks(
915917
return;
916918
}
917919

918-
// Server stamps provenance: the session identity wins over whatever the
919-
// client sent. Without this the author-or-parent-editor rule below is
920-
// spoofable — a caller could post as anyone, then "author-delete" it.
921-
if (ctx.session.userId) data.author_id = ctx.session.userId;
920+
// The author is the session's user, recorded over whatever the client
921+
// sent: the row's provenance names who wrote it. A session with no user
922+
// has nobody to record, so the create is refused, as a reaction's is —
923+
// never left to the client's value. The column is read-only on update
924+
// (`objects/sys-comment.object.ts`), so this stamp is the row's author
925+
// for life.
926+
const authorId: unknown = ctx.session.userId;
927+
if (typeof authorId !== 'string' || authorId === '') {
928+
forbid('Cannot comment: there is no signed-in user to record as the author');
929+
}
930+
data.author_id = authorId;
922931

923932
// [#23124] A new comment is not an edited one, whatever the client sent.
924933
// The engine's create-side `readonly` strip leaves `sys_` objects to their

‎packages/plugins/plugin-audit/src/comment-author-display-identity.integration.test.ts‎

Lines changed: 107 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,13 @@
1212
* and reports it (`droppedFields`), so an edit never changes whose name a
1313
* comment shows. A system write keeps what it supplies, as for `author_id`.
1414
*
15+
* `author_id` is held the same way (the second describe): a row's author is
16+
* read-only after creation, like its display name. The engine strips a
17+
* non-system caller's value on update and reports it, the author's edit and a
18+
* parent editor's alike; a system write and a historical import's update
19+
* (`preserveAudit`) keep the value they supply; and a create whose session
20+
* names no user is refused, since there is nobody to record.
21+
*
1522
* Same rig as `comment-edit-stamp.integration.test.ts` (real kernel, real
1623
* ObjectQL, real AuditPlugin, real SQLite driver), plus the platform's
1724
* `sys_user` object so the profile is a real row. Assertions are on the
@@ -47,47 +54,49 @@ const boardObject = {
4754

4855
type Row = Record<string, any>;
4956

50-
describe('sys_comment — the display name and avatar are stamped from the caller\'s session, like the author id', () => {
51-
let kernel: ObjectKernel;
52-
let engine: ObjectQL;
53-
54-
beforeAll(async () => {
55-
kernel = new ObjectKernel({ logger: { level: 'silent' } });
56-
await kernel.use(new ObjectQLPlugin());
57-
await kernel.use(new AuditPlugin());
58-
await kernel.bootstrap();
59-
60-
engine = kernel.getService<ObjectQL>('objectql');
61-
const driver = new SqliteWasmDriver({ filename: ':memory:' });
62-
await driver.connect();
63-
engine.registerDriver(driver, true);
64-
engine.registry.registerObject(SysUser as any, HARNESS_PACKAGE);
65-
engine.registry.registerObject(boardObject as any, HARNESS_PACKAGE);
66-
await engine.syncSchemas();
67-
await installDefaultOrganization(engine);
68-
for (const user of [ADA, GRACE]) await engine.insert('sys_user', { ...user }, { context: SYS });
69-
}, 120_000);
70-
71-
afterAll(async () => {
72-
if (kernel) {
73-
await Promise.race([kernel.shutdown(), new Promise<void>((r) => setTimeout(r, 10_000))]);
74-
}
75-
}, 30_000);
76-
77-
const newThread = async (): Promise<string> => {
78-
const board = await engine.insert(BOARD, { name: 'board' }, { context: SYS });
79-
return `${BOARD}:${board.id}`;
80-
};
81-
const post = async (userId: string, payload: Row): Promise<string> => {
82-
const created = await engine.insert(COMMENT, { thread_id: await newThread(), body: 'hello', ...payload }, { context: as(userId) });
83-
return String(created.id);
84-
};
85-
/** The stored identity columns, and nothing else, so a failure prints exactly them. */
86-
const identity = async (id: string) => {
87-
const row = (await engine.findOne(COMMENT, { where: { id }, context: SYS })) as Row;
88-
return { author_id: row.author_id, author_name: row.author_name ?? null, author_avatar_url: row.author_avatar_url ?? null };
89-
};
57+
// One rig for both describes below: the display identity and the author id it
58+
// sits beside are one component.
59+
let kernel: ObjectKernel;
60+
let engine: ObjectQL;
61+
62+
beforeAll(async () => {
63+
kernel = new ObjectKernel({ logger: { level: 'silent' } });
64+
await kernel.use(new ObjectQLPlugin());
65+
await kernel.use(new AuditPlugin());
66+
await kernel.bootstrap();
67+
68+
engine = kernel.getService<ObjectQL>('objectql');
69+
const driver = new SqliteWasmDriver({ filename: ':memory:' });
70+
await driver.connect();
71+
engine.registerDriver(driver, true);
72+
engine.registry.registerObject(SysUser as any, HARNESS_PACKAGE);
73+
engine.registry.registerObject(boardObject as any, HARNESS_PACKAGE);
74+
await engine.syncSchemas();
75+
await installDefaultOrganization(engine);
76+
for (const user of [ADA, GRACE]) await engine.insert('sys_user', { ...user }, { context: SYS });
77+
}, 120_000);
78+
79+
afterAll(async () => {
80+
if (kernel) {
81+
await Promise.race([kernel.shutdown(), new Promise<void>((r) => setTimeout(r, 10_000))]);
82+
}
83+
}, 30_000);
84+
85+
const newThread = async (): Promise<string> => {
86+
const board = await engine.insert(BOARD, { name: 'board' }, { context: SYS });
87+
return `${BOARD}:${board.id}`;
88+
};
89+
const post = async (userId: string, payload: Row): Promise<string> => {
90+
const created = await engine.insert(COMMENT, { thread_id: await newThread(), body: 'hello', ...payload }, { context: as(userId) });
91+
return String(created.id);
92+
};
93+
/** The stored identity columns, and nothing else, so a failure prints exactly them. */
94+
const identity = async (id: string) => {
95+
const row = (await engine.findOne(COMMENT, { where: { id }, context: SYS })) as Row;
96+
return { author_id: row.author_id, author_name: row.author_name ?? null, author_avatar_url: row.author_avatar_url ?? null };
97+
};
9098

99+
describe('sys_comment — the display name and avatar are stamped from the caller\'s session, like the author id', () => {
91100
it('a comment shows the caller\'s own name and avatar whatever the client sent, as its author id does', async () => {
92101
const id = await post(ADA.id, { author_id: GRACE.id, author_name: GRACE.name, author_avatar_url: GRACE.image });
93102
expect(await identity(id)).toEqual({ author_id: ADA.id, author_name: ADA.name, author_avatar_url: ADA.image });
@@ -142,3 +151,61 @@ describe('sys_comment — the display name and avatar are stamped from the calle
142151
expect((await identity(id)).author_name).toBe('Re-imported Name');
143152
});
144153
});
154+
155+
describe('sys_comment — a row\'s author is read-only after creation, like its display name', () => {
156+
/** Run one update and collect what the engine reports it dropped. */
157+
const updateCollectingDrops = async (id: string, data: Row, context: Row): Promise<Row[]> => {
158+
const drops: Row[] = [];
159+
await engine.update(COMMENT, data, { where: { id }, context, onFieldsDropped: (e: Row) => drops.push(e) } as any);
160+
return drops;
161+
};
162+
const authorOf = async (id: string): Promise<unknown> =>
163+
((await engine.findOne(COMMENT, { where: { id }, context: SYS })) as Row).author_id;
164+
165+
it('the author\'s own edit never changes the author: the value is stripped and reported, and the body lands', async () => {
166+
const id = await post(ADA.id, {});
167+
const drops = await updateCollectingDrops(id, { body: 'edited by the author', author_id: GRACE.id }, as(ADA.id));
168+
expect(await authorOf(id)).toBe(ADA.id);
169+
expect(((await engine.findOne(COMMENT, { where: { id }, context: SYS })) as Row).body).toBe('edited by the author');
170+
expect(drops).toEqual([
171+
expect.objectContaining({ object: COMMENT, reason: 'readonly', fields: expect.arrayContaining(['author_id']) }),
172+
]);
173+
});
174+
175+
it('a parent editor\'s edit never changes the author either', async () => {
176+
const id = await post(ADA.id, {});
177+
const drops = await updateCollectingDrops(id, { body: 'moderated', author_id: GRACE.id }, as(GRACE.id));
178+
expect(await authorOf(id)).toBe(ADA.id);
179+
expect(drops).toEqual([
180+
expect.objectContaining({ object: COMMENT, reason: 'readonly', fields: expect.arrayContaining(['author_id']) }),
181+
]);
182+
});
183+
184+
it('control: an ordinary edit that does not name the author lands and drops nothing', async () => {
185+
const id = await post(ADA.id, {});
186+
const drops = await updateCollectingDrops(id, { body: 'a plain edit' }, as(ADA.id));
187+
expect(((await engine.findOne(COMMENT, { where: { id }, context: SYS })) as Row).body).toBe('a plain edit');
188+
expect(await authorOf(id)).toBe(ADA.id);
189+
expect(drops).toEqual([]);
190+
});
191+
192+
it('control: a system write keeps the author it supplies', async () => {
193+
const id = await post(ADA.id, {});
194+
await engine.update(COMMENT, { author_id: GRACE.id }, { where: { id }, context: SYS });
195+
expect(await authorOf(id)).toBe(GRACE.id);
196+
});
197+
198+
it('control: a historical import\'s update (preserveAudit) keeps the author it supplies', async () => {
199+
const id = await post(ADA.id, {});
200+
await engine.update(COMMENT, { author_id: GRACE.id }, { where: { id }, context: { ...as(ADA.id), preserveAudit: true } });
201+
expect(await authorOf(id)).toBe(GRACE.id);
202+
});
203+
204+
it('a create whose session names no user is refused: there is nobody to record as the author', async () => {
205+
const thread = await newThread();
206+
await expect(
207+
engine.insert(COMMENT, { thread_id: thread, body: 'nobody wrote this', author_id: GRACE.id }, { context: { positions: ['org_member'] } }),
208+
).rejects.toMatchObject({ code: 'RECORD_NOT_ACCESSIBLE', status: 403 });
209+
expect(await engine.find(COMMENT, { where: { thread_id: thread }, context: SYS })).toEqual([]);
210+
});
211+
});

‎packages/plugins/plugin-audit/src/objects/sys-comment.object.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,26 @@ import { ObjectSchema, Field } from '@objectstack/spec/data';
122122
* gate answers them with the bounded thread probe (fail closed), never a
123123
* leaf.
124124
*
125+
* ## Platform-written: the author `author_id`
126+
*
127+
* A comment's author is the server's, recorded once, at creation: a row's
128+
* author is read-only after creation, like the comment's display name below.
129+
* The reason is provenance integrity: the row names the user who wrote it, so
130+
* it keeps the value the create stamped for the life of the row.
131+
*
132+
* - **The stamp.** The create hook in `comment-access-hooks.ts` assigns the
133+
* session user over any value the client sent. A non-system create whose
134+
* session names no user has nobody to record, and is refused (403
135+
* `RECORD_NOT_ACCESSIBLE`), as a reaction with no signed-in user is.
136+
* - **Read-only on update.** `readonly: true`, so the engine's update-side
137+
* strip drops a non-system caller's value and reports it (`droppedFields`),
138+
* the author's and a parent editor's alike; the rest of the write lands.
139+
* - **System writes** (seeds, imports, migrations) and a `preserveAudit`
140+
* update (a historical import, which only the system principal may start)
141+
* keep the value they supply. On those writes the column's presence is the writer's
142+
* to supply: the engine's record validator does not enforce `required` on a
143+
* `readonly` field, as `sys_file`'s storage-owned `key` and `status` record.
144+
*
125145
* ## Platform-written: the author's display identity `author_name` / `author_avatar_url`
126146
*
127147
* The name and avatar a reader is shown beside a comment are the server's,
@@ -204,9 +224,12 @@ export const SysComment = ObjectSchema.create({
204224
}),
205225

206226
// ── Author ───────────────────────────────────────────────────
227+
// Platform-written (see "Platform-written: the author" above): stamped on
228+
// create from the session user, read-only on update.
207229
author_id: Field.lookup('sys_user', {
208230
label: 'Author',
209231
required: true,
232+
readonly: true,
210233
searchable: true,
211234
group: 'Author',
212235
}),

0 commit comments

Comments
 (0)