From 8c48b2a4b12a316de8293e07e664162e97be158f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 06:47:59 +0000 Subject: [PATCH 1/2] Keep where a task came from, whose it is and its unit server-owned (#176) A duly_member or duly_manager could write source, owner and business_unit on a task through every door (data POST/PATCH, MCP create/update), and so put self-made work into the governed readings or a row into someone else's, which that person could not see (#143). - source and business_unit are readonly: the platform strips a caller's value on insert and update (source re-defaults to self). The dispatcher, the assignment fan-out and the seed write as the system and keep writing both. - The lifecycle hook stamps business_unit on a caller's insert from the writer's primary business unit, the rung the fan-out already reads. - owner stays writable, because a readonly field skips required and the #148 fan-out relies on an ownerless row being refused. A new guard, duly_task_owner_is_writer, refuses a caller's insert naming someone else and an update changing the owner. Managers follow the same rule: their door for a report's work is the assignment. Fixes #176 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01JQwxbaGDrSsQpv7GZxtDyZ --- src/hooks/index.ts | 6 +- src/hooks/task.hook.ts | 126 +++++++++- src/objects/task.object.ts | 24 +- test/invariants.test.ts | 15 +- test/task-hook-body.test.ts | 14 +- test/task-hook.test.ts | 6 +- test/task-provenance.test.ts | 438 +++++++++++++++++++++++++++++++++++ 7 files changed, 614 insertions(+), 15 deletions(-) create mode 100644 test/task-provenance.test.ts diff --git a/src/hooks/index.ts b/src/hooks/index.ts index baa64ab..16cc244 100644 --- a/src/hooks/index.ts +++ b/src/hooks/index.ts @@ -14,8 +14,8 @@ // while empty and infers correctly the moment something is pushed into it. import { DutyTimezoneGuard } from './duty.hook.js'; -import { TaskDutyReadGuard, TaskLifecycleHook } from './task.hook.js'; +import { TaskDutyReadGuard, TaskLifecycleHook, TaskOwnerGuard } from './task.hook.js'; -export { DutyTimezoneGuard, TaskDutyReadGuard, TaskLifecycleHook }; +export { DutyTimezoneGuard, TaskDutyReadGuard, TaskLifecycleHook, TaskOwnerGuard }; -export const dulyHooks = [DutyTimezoneGuard, TaskLifecycleHook, TaskDutyReadGuard]; +export const dulyHooks = [DutyTimezoneGuard, TaskLifecycleHook, TaskDutyReadGuard, TaskOwnerGuard]; diff --git a/src/hooks/task.hook.ts b/src/hooks/task.hook.ts index 66a4dd6..55d6d7b 100644 --- a/src/hooks/task.hook.ts +++ b/src/hooks/task.hook.ts @@ -353,6 +353,29 @@ const stampTaskLifecycle = async (ctx: HookContext): Promise => { if (input.status === 'done') { input.completed_late = completedLate(input.completed_at, input.late_after); } + + // ── business_unit — the writer's unit, on a caller's insert (#176) ────── + // `business_unit` is `readonly`, so a caller's value is stripped AFTER this + // hook and nothing would be left. The system writers carry their own: the + // dispatcher copies the duty's, the fan-out reads the assignee's, the seed + // writes history. A caller's own task is the remaining case, and its owner + // is the caller (`duly_task_owner_is_writer` refuses anyone else), so the + // unit is the caller's `primary_business_unit_id` — the rung the fan-out + // already reads for a task with no duty. A key this hook writes is not a + // caller write, so the strip keeps it, even when it equals what the + // caller sent (the form echoes the field back). + const session = ctx.session; + if (session?.isSystem !== true && typeof session?.userId === 'string' && session.userId !== '') { + if (!ctx.api) { + throw new Error('duly_task_lifecycle_stamps: ctx.api is unavailable, so the writer\'s business unit cannot be read.'); + } + const writer = await ctx.api.object('sys_user').findOne({ + where: { id: session.userId }, + fields: ['primary_business_unit_id'], + }); + const unit = writer?.primary_business_unit_id; + input.business_unit = typeof unit === 'string' && unit !== '' ? unit : null; + } return; } @@ -563,15 +586,17 @@ export const TaskLifecycleHook: Hook = { + 'moves them. On a predicate (bulk) write the row-conditional stamps are handled by ' + 'ADR-0058 Addendum II D3: one payload for the whole batch, so a re-stamp of an ' + 'already-done row and a completion that would be stamped late are both refused, and ' - + 'last_update_at is not stamped at all.', + + 'last_update_at is not stamped at all. On an insert by a caller (not a system write), ' + + 'business_unit is the writer\'s primary business unit, read elevated.', // Explicit because it is load-bearing rather than a default worth inheriting: // if this handler throws, the write MUST be refused. Committing a task whose // stamps were not applied is the exact silent corruption the // `completed_at_required_when_done` rule exists to make impossible. onError: 'abort', - // Elevated for the one `ctx.api` call this hook makes: reading `grace_days` - // off the duty the row names. The deadline a row is born with must not - // depend on whether its creator can see that duty — see the module header. + // Elevated for the two `ctx.api` reads this hook makes: `grace_days` off the + // duty the row names, and the writer's own `primary_business_unit_id`. The + // deadline a row is born with must not depend on whether its creator can see + // that duty — see the module header. // Governs `ctx.api` only; the readonly strip on this payload is unchanged. runAs: 'system', handler: stampTaskLifecycle, @@ -709,3 +734,96 @@ export const TaskDutyReadGuard: Hook = { runAs: 'inherit', handler: refuseUnreadableDuty, }; + +/** + * `duly_task.owner` names the writer, or the write is refused (#176). + * + * Measured on 18.2.0 against a live `pnpm demo`: a `duly_member` and a + * `duly_manager` could each create a task, or move their own, with `owner` + * set to somebody else — through data POST and PATCH and MCP create and + * update. The row then sat in that person's governed readings (Overdue, + * on-time) while that person could not see it, because `readScope: 'own'` + * resolves through `owner_id`, which is the writer (#143). + * + * ── Who may name whom ──────────────────────────────────────────────────── + * A caller names only themselves. That holds for a manager too: "Assigning is + * their only write" (AGENTS.md product invariants), and the door for a + * report's work is `duly_assignment` and its fan-out. The system writers — + * the dispatcher, the fan-out (`runAs: 'system'`), the seed — are who put work + * on other people, and `session.isSystem` lets them through. + * + * ── Why a hook, and why it refuses ────────────────────────────────────── + * `source` and `business_unit` are `readonly` on the object and the platform + * strips a caller's value. `owner` cannot be: a `readonly` field is validated + * shape-only, so its `required` stops being checked, and a system insert with + * `owner: null` then commits an ownerless task (measured). The #148 fan-out + * depends on that refusal to report a removed assignee. So `owner` stays + * writable and this guard judges it. + * + * It refuses rather than rewrites. The console form does not carry `owner` + * at all, so the only caller who meets this is one who meant somebody else, + * and a silent rewrite would hide that mistake from them. + * + * Re-sending the stored owner is not judged, so a re-save of a row whose + * owner is already someone else (written before this guard) keeps working. + * A write with no caller identity at all is not judged either: there is + * nobody to compare with, and the security layer refuses a principal-less + * write before it gets here. + * + * Self-contained, like the hooks above: the build lowers it into a sandboxed + * `body`, which carries `ctx.session`. + */ +const refuseForeignOwner = async (ctx: HookContext): Promise => { + const input = ctx.input as Record; + if (!('owner' in input)) return; + + const session = ctx.session; + if (session?.isSystem === true) return; + const writer = typeof session?.userId === 'string' ? session.userId : ''; + if (writer === '') return; + + const idOf = (value: unknown): string => { + if (typeof value === 'string') return value; + if (value !== null && typeof value === 'object' && typeof (value as { id?: unknown }).id === 'string') { + return (value as { id: string }).id; + } + return ''; + }; + + const owner = idOf(input.owner); + if (owner === writer) return; + + if (ctx.event === 'beforeUpdate') { + const previous = ctx.previous as Record | undefined; + if (previous && idOf(previous.owner) === owner) return; + } + + const message = 'Owner: you can only create or keep tasks for yourself. ' + + 'To give work to someone else, send them an assignment.'; + const error = new Error(message) as Error & { code: string; fields: unknown[] }; + error.name = 'ValidationError'; + error.code = 'VALIDATION_FAILED'; + error.fields = [{ + field: 'owner', + code: 'owner_not_writer', + message, + label: 'Owner', + value: owner, + }]; + throw error; +}; + +export const TaskOwnerGuard: Hook = { + name: 'duly_task_owner_is_writer', + label: 'Task owner is the writer', + object: 'duly_task', + events: ['beforeInsert', 'beforeUpdate'], + description: + 'Refuses a duly_task write by a caller that names someone else as owner — an insert, or an ' + + 'update changing owner. A caller creates tasks only for themselves; work for another ' + + 'person is an assignment. System writes (the dispatcher, the assignment fan-out, the seed) ' + + 'pass, and re-sending the stored owner is not checked.', + // A guard that fails open is not a guard. + onError: 'abort', + handler: refuseForeignOwner, +}; diff --git a/src/objects/task.object.ts b/src/objects/task.object.ts index bc4a390..48fa7b7 100644 --- a/src/objects/task.object.ts +++ b/src/objects/task.object.ts @@ -113,6 +113,15 @@ export const Task = ObjectSchema.create({ description: 'Empty for a bare one-off that was never modelled as a duty.', }), + // A caller names only themselves (#176): `duly_task_owner_is_writer` + // refuses anyone else. The dispatcher, the fan-out and the seed are the + // writers that put work on other people. + // + // ⛔ NOT `readonly`, unlike `source` and `business_unit` beside it. A + // readonly field is validated shape-only, so `required` stops being + // checked and a system insert with `owner: null` commits an ownerless task + // (measured on 18.2.0). The #148 fan-out relies on that refusal to report a + // removed assignee, and "one task has exactly one owner" is an invariant. owner: Field.user({ label: 'Owner', group: 'basics', @@ -120,10 +129,16 @@ export const Task = ObjectSchema.create({ defaultValue: 'current_user', }), + // Server-owned (#176). `readonly` strips a caller's value on insert and + // update; the system writers are exempt and carry their own — the + // dispatcher copies the duty's, the fan-out reads the assignee's. A + // caller's own task gets the writer's primary business unit from + // `task.hook.ts`. Before this, a member could file a row under any unit. business_unit: Field.lookup('sys_business_unit', { label: 'Business unit', group: 'basics', - description: 'Denormalised from the owner at dispatch so rollups survive a later transfer.', + readonly: true, + description: 'Denormalised from the owner when the task is created, so rollups survive a later transfer. Set by the server, never by hand.', }), assignment: Field.lookup('duly_assignment', { @@ -134,10 +149,17 @@ export const Task = ObjectSchema.create({ // Same caliber rule as the duty. Metrics read this column and only this // column; `self` never enters an on-time rate or a comparison. + // + // Server-owned (#176). A member could mark their own work `catalog` and + // enter every governed reading. `readonly` strips a caller's value on + // insert (re-defaulted to `self`) and on update; the dispatcher, the + // fan-out and the seed write as the system and are exempt. So where a task + // came from is decided by how it was made, never by who typed it. source: Field.select({ label: 'Source', group: 'basics', required: true, + readonly: true, options: [ { label: 'Role catalog', value: 'catalog', color: '#16515F' }, { label: 'Assigned by manager', value: 'assigned', color: '#8C6512' }, diff --git a/test/invariants.test.ts b/test/invariants.test.ts index 7106cf4..323f852 100644 --- a/test/invariants.test.ts +++ b/test/invariants.test.ts @@ -123,7 +123,20 @@ describe('product invariants', () => { ).toBeDefined(); } - const readonlyFields = entries.filter(([, f]) => f.readonly === true).map(([name]) => name).sort(); + // #176 — two readonly columns are PROVENANCE, not stamps: where the task + // came from and which unit it is filed under. They are server-owned so a + // caller cannot forge them, and they stay in Basics because they say what + // the task is. Named here, so a third one is a decision and not a drift. + const PROVENANCE = ['business_unit', 'source']; + for (const name of PROVENANCE) { + const field = (Task.fields as Record)[name]; + expect(field?.readonly, `duly_task.${name} is server-owned (#176)`).toBe(true); + expect(field?.group, `duly_task.${name} stays in Basics`).toBe('basics'); + } + + const readonlyFields = entries + .filter(([name, f]) => f.readonly === true && !PROVENANCE.includes(name)) + .map(([name]) => name).sort(); const historyFields = entries.filter(([, f]) => f.group === 'history').map(([name]) => name).sort(); expect(historyFields, 'the history section IS the set of server-owned stamps') .toEqual(readonlyFields); diff --git a/test/task-hook-body.test.ts b/test/task-hook-body.test.ts index d01a102..e7e4edd 100644 --- a/test/task-hook-body.test.ts +++ b/test/task-hook-body.test.ts @@ -103,18 +103,22 @@ describe('the lowered body', () => { expect(lowered.source).toContain("ctx.api.object(\"duly_duty\").findOne"); }); - it('makes exactly one elevated call: grace_days off the duty the row names', () => { + it('makes exactly two elevated calls: grace_days off the duty, and the writer\'s unit (#176)', () => { // `runAs: 'system'` elevates the body's own `ctx.api` calls and nothing // else — the triggering task write keeps its caller's authorization (the // spec's `runAs` scope note; measured live on `pnpm demo` for #100: a // member who gets 404 reading the duty still gets the grace on their // task, and still gets 404 updating a task that is not theirs). So the - // whole of what the elevation reaches is this one read, and it is pinned. + // whole of what the elevation reaches is these two reads, and they are + // pinned: the duty's grace, and — on a caller's insert only (#176) — the + // caller's OWN `primary_business_unit_id`, keyed on `session.userId`. const calls = lowered.source.match(/ctx\.api\b[^;]*/g) ?? []; - const apiCalls = calls.filter((c) => c.includes('.object(')); - expect(apiCalls, 'one ctx.api data call in the whole body').toHaveLength(1); - expect(apiCalls[0].replace(/\s+/g, ' ')) + const apiCalls = calls.filter((c) => c.includes('.object(')).map((c) => c.replace(/\s+/g, ' ')); + expect(apiCalls, 'two ctx.api data calls in the whole body').toHaveLength(2); + expect(apiCalls[0]) .toBe('ctx.api.object("duly_duty").findOne({ where: { id }, fields: ["grace_days"] })'); + expect(apiCalls[1]) + .toBe('ctx.api.object("sys_user").findOne({ where: { id: session.userId }, fields: ["primary_business_unit_id"] })'); }); it('stamps a task on a one-off duty with the duty\'s grace (#100)', async () => { diff --git a/test/task-hook.test.ts b/test/task-hook.test.ts index f97f0ac..44cb0da 100644 --- a/test/task-hook.test.ts +++ b/test/task-hook.test.ts @@ -394,10 +394,12 @@ describe('last_update_at — the stagnation signal', () => { const before = task.last_update_at as string; await tick(); + // An administrative write is a system one: `business_unit` is readonly + // (#176), so a caller's value is stripped and only the system lands it. const rebadged = await data.update('duly_task', { id: task.id, business_unit: 'bu_north', - }); + }, { context: { isSystem: true } }); expect(rebadged.business_unit).toBe('bu_north'); expect( @@ -590,6 +592,8 @@ describe('completed_at on a predicate write — one payload, N rows', () => { await data.update('duly_task', { business_unit: 'bu_north' }, { multi: true, where: { id: { $in: [alreadyDone] } }, + // A backfill is a system write: `business_unit` is readonly (#176). + context: { isSystem: true }, }); const row = await read(alreadyDone); diff --git a/test/task-provenance.test.ts b/test/task-provenance.test.ts new file mode 100644 index 0000000..f9df7fe --- /dev/null +++ b/test/task-provenance.test.ts @@ -0,0 +1,438 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { AppPlugin, HttpDispatcher, ObjectKernel, createStandaloneStack } from '@objectstack/runtime'; +import type { HttpProtocolContext } from '@objectstack/runtime'; +import { createIdentityObjectsPlugin } from '@objectstack/plugin-auth'; +import { SecurityPlugin, buildContextForUser } from '@objectstack/plugin-security'; +import { SharingServicePlugin } from '@objectstack/plugin-sharing'; +import { AutomationServicePlugin } from '@objectstack/service-automation'; +import { JobServicePlugin } from '@objectstack/service-job'; +import { extractHookBody } from '@objectstack/cli/hook-body'; + +import stack from '../objectstack.config.js'; +import { Task } from '../src/objects/task.object.js'; +import { TaskOwnerGuard, dulyHooks } from '../src/hooks/index.js'; +import { runDispatch, unbindDispatchEngine } from '../src/jobs/dispatch.job.js'; +import { ensureUsers } from './support/users.js'; + +/** + * Where a task came from, whose it is, and which unit it is filed under (#176). + * + * Measured first on 18.2.0 against a live `pnpm demo`: a `duly_member` and a + * `duly_manager` could each write `source`, `owner` and `business_unit` on a + * task through every door — data POST and PATCH, MCP create and update — and + * so put self-made work into the governed readings, or a row into somebody + * else's. This suite proves the fix door by door, as a real member AND a real + * manager resolved by the platform's own `buildContextForUser`, with + * `SecurityPlugin` and `SharingServicePlugin` mounted: + * + * - `source` is `readonly`: a caller's value is STRIPPED, on insert + * re-defaulted to `self`. + * - `business_unit` is `readonly`: stripped on update; on insert the + * lifecycle hook writes the writer's own primary unit. + * - `owner` naming anyone but the writer is REFUSED by + * `duly_task_owner_is_writer` (it cannot be readonly — see the hook). + * + * And the writers that put work on other people keep writing all three: the + * dispatcher, the assignment fan-out and the seed. + */ + +const SYSTEM = { isSystem: true } as const; +const HISTORY = { isSystem: true, skipStateMachine: true } as const; +const DUE = '2026-11-10'; + +let kernel: any; +let data: any; +let dispatcher: HttpDispatcher; +let unitOwen: string; +let unitMarek: string; +let unitOther: string; +const callers: Record = {}; + +const attempt = async (fn: () => Promise): Promise<{ ok?: any; thrown?: any }> => { + try { + const ok = await fn(); + const status = ok?.response?.status; + if (typeof status === 'number' && status >= 400) return { thrown: ok.response.body?.error ?? ok.response.body }; + return { ok }; + } catch (thrown) { + return { thrown }; + } +}; + +const createdId = (res: { ok?: any }): string => { + const body = res.ok?.response?.body; + return String(body?.data?.id ?? body?.data?.record?.id ?? body?.id ?? body?.record?.id ?? res.ok?.id ?? res.ok?.record?.id); +}; + +const stored = async (id: string) => data.findOne('duly_task', { where: { id } }, { context: SYSTEM }); + +const expectOwnerRefused = (thrown: any) => { + expect(thrown, 'the write was refused').toBeDefined(); + const message = String(thrown?.message ?? thrown?.error?.message ?? ''); + expect(message).toContain('you can only create or keep tasks for yourself'); + const fields = thrown?.fields ?? thrown?.error?.fields ?? thrown?.details?.fields; + if (fields) expect(fields).toEqual(expect.arrayContaining([expect.objectContaining({ field: 'owner', code: 'owner_not_writer' })])); +}; + +beforeAll(async () => { + const { plugins } = await createStandaloneStack({ + databaseDriver: 'sqlite', + databaseUrl: ':memory:', + skipSeedData: true, + artifactPath: 'dist/objectstack.this-suite-must-not-load-an-artifact.json', + }); + kernel = new ObjectKernel(); + for (const plugin of plugins) await kernel.use(plugin); + await kernel.use(createIdentityObjectsPlugin()); + await kernel.use(new SharingServicePlugin()); + await kernel.use(new SecurityPlugin()); + await kernel.use(new AppPlugin(stack, undefined, { skipSeedData: true })); + await kernel.use(new JobServicePlugin()); + await kernel.use(new AutomationServicePlugin()); + await kernel.bootstrap(); + data = kernel.getService('data'); + const ql = kernel.getService('objectql'); + dispatcher = new HttpDispatcher(kernel); + + await ensureUsers(data, ['user_owen', 'user_marek', 'user_sam']); + const unit = async (name: string, code: string) => + String((await data.insert('sys_business_unit', { name, code }, { context: SYSTEM })).id); + unitOwen = await unit('Central Office', 'CEN'); + unitMarek = await unit('Northgate Operations', 'OPS'); + unitOther = await unit('Northgate Quality', 'QA'); + // `primary_business_unit_id` is the platform's projection; a fixture writes + // it directly, from the system, the way `test/business-unit-membership` + // reads it back. + for (const [user, bu] of [['user_owen', unitOwen], ['user_marek', unitMarek], ['user_sam', unitOther]]) { + await data.update('sys_user', { id: user, primary_business_unit_id: bu }, { context: SYSTEM }); + } + + const bind = async (user: string, sets: string[]) => { + let organization: string | undefined; + for (const set of sets) { + const grant = await data.insert('sys_user_permission_set', { user_id: user, permission_set: set }, { context: SYSTEM }); + organization = grant.organization_id; + } + return { request: {}, executionContext: await buildContextForUser(ql, user, Date.now(), organization) }; + }; + callers.member = await bind('user_owen', ['duly_member']); + callers.manager = await bind('user_marek', ['duly_member', 'duly_manager']); +}, 180_000); + +afterAll(async () => { + unbindDispatchEngine?.(); + await kernel?.shutdown?.(); +}); + +describe('the metadata', () => { + it('source and business_unit are server-owned; owner is not readonly, so required still bites', () => { + const fields = Task.fields as Record; + expect(fields.source.readonly).toBe(true); + expect(fields.business_unit.readonly).toBe(true); + expect(fields.owner.readonly, 'a readonly field skips required — see the owner guard').toBeFalsy(); + expect(fields.owner.required).toBe(true); + }); + + it('registers the owner guard, failing closed', () => { + expect(dulyHooks).toContain(TaskOwnerGuard); + expect(TaskOwnerGuard.object).toBe('duly_task'); + expect(TaskOwnerGuard.events).toEqual(['beforeInsert', 'beforeUpdate']); + expect(TaskOwnerGuard.onError).toBe('abort'); + }); +}); + +describe.each([ + ['member', 'user_owen', () => unitOwen], + ['manager', 'user_marek', () => unitMarek], +])('as a %s', (who, self, ownUnit) => { + const ctx = () => callers[who]!; + const post = (body: Record) => + attempt(() => dispatcher.handleData('/duly_task', 'POST', body, {}, ctx())); + const patch = (id: string, body: Record) => + attempt(() => dispatcher.handleData(`/duly_task/${id}`, 'PATCH', body, {}, ctx())); + const mcp = () => (dispatcher as any).buildMcpBridge(ctx()); + const ownTask = async (subject: string): Promise => { + const res = await post({ subject, due_date: DUE }); + expect(res.thrown, 'creating their own task').toBeUndefined(); + return createdId(res); + }; + + describe('a plain task of their own', () => { + it('is theirs, self-declared, and filed under their own unit', async () => { + const row = await stored(await ownTask(`${who}: plain`)); + expect(row.owner).toBe(self); + expect(row.owner_id).toBe(self); + expect(row.source).toBe('self'); + expect(row.business_unit).toBe(ownUnit()); + }); + + it('saves from the console form, which echoes source and business_unit back', async () => { + // The create form's own body (measured: `{ source: 'self', status, subject }`), + // plus an edit form's re-save of every field it shows. + const res = await post({ subject: `${who}: form`, source: 'self', status: 'open', business_unit: ownUnit() }); + expect(res.thrown).toBeUndefined(); + const id = createdId(res); + expect((await stored(id)).business_unit).toBe(ownUnit()); + const resave = await patch(id, { subject: `${who}: form, edited`, source: 'self', business_unit: ownUnit(), owner: self, note: 'n' }); + expect(resave.thrown).toBeUndefined(); + const row = await stored(id); + expect(row.subject).toBe(`${who}: form, edited`); + expect(row.note).toBe('n'); + expect(row.business_unit).toBe(ownUnit()); + }); + }); + + describe('source — stripped on every door', () => { + it('data POST: a forged catalog lands self', async () => { + const res = await post({ subject: `${who}: forged catalog`, source: 'catalog', due_date: DUE }); + expect(res.thrown).toBeUndefined(); + expect((await stored(createdId(res))).source).toBe('self'); + }); + + it('data PATCH: moving their own task to assigned leaves it self', async () => { + const id = await ownTask(`${who}: patch source`); + expect((await patch(id, { source: 'assigned', note: 'still saved' })).thrown).toBeUndefined(); + const row = await stored(id); + expect(row.source).toBe('self'); + expect(row.note, 'the rest of the write lands').toBe('still saved'); + }); + + it('MCP create_record and update_record', async () => { + const created = await attempt(() => mcp().create('duly_task', { subject: `${who}: mcp source`, source: 'catalog' })); + expect(created.thrown).toBeUndefined(); + const id = createdId(created); + expect((await stored(id)).source).toBe('self'); + expect((await attempt(() => mcp().update('duly_task', id, { source: 'catalog' }))).thrown).toBeUndefined(); + expect((await stored(id)).source).toBe('self'); + }); + + it('bulk updateMany', async () => { + const a = await ownTask(`${who}: bulk source`); + await attempt(() => kernel.getService('protocol').updateManyData({ + object: 'duly_task', + records: [{ id: a, data: { source: 'catalog' } }], + context: ctx().executionContext, + })); + expect((await stored(a)).source).toBe('self'); + }); + }); + + describe('business_unit — the writer\'s own, on every door', () => { + it('data POST: a foreign unit lands as their own', async () => { + const res = await post({ subject: `${who}: foreign unit`, business_unit: unitOther }); + expect(res.thrown).toBeUndefined(); + expect((await stored(createdId(res))).business_unit).toBe(ownUnit()); + }); + + it('data PATCH: moving their own task to a foreign unit leaves it', async () => { + const id = await ownTask(`${who}: patch unit`); + expect((await patch(id, { business_unit: unitOther })).thrown).toBeUndefined(); + expect((await stored(id)).business_unit).toBe(ownUnit()); + }); + + it('MCP create_record and update_record', async () => { + const created = await attempt(() => mcp().create('duly_task', { subject: `${who}: mcp unit`, business_unit: unitOther })); + expect(created.thrown).toBeUndefined(); + const id = createdId(created); + expect((await stored(id)).business_unit).toBe(ownUnit()); + expect((await attempt(() => mcp().update('duly_task', id, { business_unit: unitOther }))).thrown).toBeUndefined(); + expect((await stored(id)).business_unit).toBe(ownUnit()); + }); + + it('bulk updateMany', async () => { + const a = await ownTask(`${who}: bulk unit`); + await attempt(() => kernel.getService('protocol').updateManyData({ + object: 'duly_task', + records: [{ id: a, data: { business_unit: unitOther } }], + context: ctx().executionContext, + })); + expect((await stored(a)).business_unit).toBe(ownUnit()); + }); + }); + + describe('owner — refused on every door', () => { + it('data POST naming someone else', async () => { + const res = await post({ subject: `${who}: for Sam`, owner: 'user_sam' }); + expectOwnerRefused(res.thrown); + const rows = await data.find('duly_task', { where: { subject: `${who}: for Sam` } }, { context: SYSTEM }); + expect(rows ?? []).toHaveLength(0); + }); + + it('data POST naming themselves is fine', async () => { + const res = await post({ subject: `${who}: for me`, owner: self }); + expect(res.thrown).toBeUndefined(); + expect((await stored(createdId(res))).owner).toBe(self); + }); + + it('data PATCH handing their own task to someone else', async () => { + const id = await ownTask(`${who}: hand over`); + expectOwnerRefused((await patch(id, { owner: 'user_sam' })).thrown); + expect((await stored(id)).owner).toBe(self); + }); + + it('MCP create_record and update_record', async () => { + expectOwnerRefused((await attempt(() => mcp().create('duly_task', { subject: `${who}: mcp for Sam`, owner: 'user_sam' }))).thrown); + const id = await ownTask(`${who}: mcp hand over`); + expectOwnerRefused((await attempt(() => mcp().update('duly_task', id, { owner: 'user_sam' }))).thrown); + expect((await stored(id)).owner).toBe(self); + }); + + it('bulk updateMany', async () => { + const a = await ownTask(`${who}: bulk hand over`); + const res = await attempt(() => kernel.getService('protocol').updateManyData({ + object: 'duly_task', + records: [{ id: a, data: { owner: 'user_sam' } }], + context: ctx().executionContext, + })); + if (res.thrown === undefined) { + const rows = res.ok?.results ?? res.ok?.records ?? []; + expect(rows.length, JSON.stringify(res.ok)).toBe(1); + expect(rows[0].success).toBe(false); + } else { + expectOwnerRefused(res.thrown); + } + expect((await stored(a)).owner).toBe(self); + }); + + it('a predicate (multi) update over their own tasks', async () => { + const subject = `${who}: predicate hand over`; + const a = await ownTask(subject); + const res = await attempt(() => data.update('duly_task', { owner: 'user_sam' }, { + where: { subject }, multi: true, context: ctx().executionContext, + })); + expectOwnerRefused(res.thrown); + expect((await stored(a)).owner).toBe(self); + }); + + it('a re-save of a row whose stored owner is already someone else passes', async () => { + // Rows written before this guard can carry a foreign owner. A re-save + // that re-sends it is not a change, so it is not judged. + const id = await ownTask(`${who}: legacy row`); + await data.update('duly_task', { id, owner: 'user_sam' }, { context: SYSTEM }); + const res = await patch(id, { owner: 'user_sam', note: 'touched' }); + expect(res.thrown).toBeUndefined(); + expect((await stored(id)).note).toBe('touched'); + }); + }); +}); + +describe('the writers that put work on other people keep writing all three', () => { + it('the dispatcher copies the duty\'s source, owner and unit', async () => { + const duty = await data.insert('duly_duty', { + name: 'Sam\'s monthly return', + owner: 'user_sam', + business_unit: unitOther, + form: 'recurring', + frequency: 'monthly', + status: 'active', + source: 'catalog', + review_status: 'approved', + timezone: 'UTC', + }, { context: HISTORY }); + const result = await runDispatch(data, { now: new Date('2026-11-02T06:00:00Z') }); + expect(result.degradedReason).toBeUndefined(); + const tasks = await data.find('duly_task', { where: { duty: String(duty.id) } }, { context: SYSTEM }); + expect(tasks.length).toBeGreaterThan(0); + for (const task of tasks) { + expect(task.owner).toBe('user_sam'); + expect(task.source).toBe('catalog'); + expect(task.business_unit).toBe(unitOther); + } + }); + + it('the assignment fan-out, triggered by a manager, writes assigned + the assignee\'s unit', async () => { + const assignment = await data.insert('duly_assignment', { + subject: 'Supplier attestation, #176', assigner: 'user_marek', assignees: ['user_sam'], + due_date: '2026-11-20', status: 'dispatched', needs_collection: false, + }, { context: SYSTEM }); + const row = await data.findOne('duly_assignment', { where: { id: assignment.id } }, { context: SYSTEM }); + // A record-change trigger carries the user whose write fired it: a + // `runAs: 'system'` run is then `{ isSystem: true, userId }`. That user is + // the manager — and the owner guard and the unit stamp must still stand + // aside, because the write is the system's. + await kernel.getService('automation').execute('duly_assignment_fanout', { + record: row, object: 'duly_assignment', event: 'afterUpdate', userId: 'user_marek', + }); + const tasks = await data.find('duly_task', { where: { assignment: assignment.id } }, { context: SYSTEM }); + const sams = tasks.filter((t: any) => t.owner === 'user_sam'); + expect(sams, JSON.stringify(tasks)).toHaveLength(1); + expect(sams[0].source).toBe('assigned'); + expect(sams[0].business_unit).toBe(unitOther); + }); + + it('a fan-out row with no owner is still refused — owner keeps its required', async () => { + const res = await attempt(() => data.insert('duly_task', { subject: 'Ownerless', owner: null, source: 'assigned' }, { context: SYSTEM })); + expect(res.thrown, 'a system insert with no owner').toBeDefined(); + expect(String(res.thrown?.message)).toMatch(/Owner is required/); + }); + + it('a seed write, from the seed loader\'s own context', async () => { + const row = await data.insert('duly_task', { + subject: 'Seeded for Sam', owner: 'user_sam', source: 'catalog', business_unit: unitOther, due_date: DUE, + }, { context: { isSystem: true, skipTriggers: true, seedReplay: true } }); + const back = await stored(String(row.id)); + expect(back.owner).toBe('user_sam'); + expect(back.source).toBe('catalog'); + expect(back.business_unit).toBe(unitOther); + }); + + it('an administrative system update can still re-file a task', async () => { + const row = await data.insert('duly_task', { subject: 'Re-filed', owner: 'user_sam', source: 'catalog' }, { context: SYSTEM }); + await data.update('duly_task', { id: row.id, owner: 'user_owen', business_unit: unitOwen, source: 'assigned' }, { context: SYSTEM }); + const back = await stored(String(row.id)); + expect([back.owner, back.business_unit, back.source]).toEqual(['user_owen', unitOwen, 'assigned']); + }); +}); + +describe('the owner guard as it ships — lowered, and run in QuickJS', () => { + let bodyKernel: any; + let bodyData: any; + let bodyDispatcher: HttpDispatcher; + let bodyOwen: HttpProtocolContext; + + beforeAll(async () => { + const lowered = extractHookBody(TaskOwnerGuard.handler as (...a: unknown[]) => unknown, 'task.hook.ts'); + expect(lowered.isExpression).toBe(false); + expect(lowered.capabilities, 'the guard reads nothing').toEqual([]); + const bodyOnly = { + ...stack, + hooks: (stack.hooks ?? []).map((hook: any) => + hook.name === TaskOwnerGuard.name + ? { ...hook, handler: undefined, body: { language: 'js', source: lowered.source, capabilities: lowered.capabilities } } + : hook), + }; + const { plugins } = await createStandaloneStack({ + databaseDriver: 'sqlite', + databaseUrl: ':memory:', + skipSeedData: true, + artifactPath: 'dist/objectstack.this-suite-must-not-load-an-artifact.json', + }); + bodyKernel = new ObjectKernel(); + for (const plugin of plugins) await bodyKernel.use(plugin); + await bodyKernel.use(createIdentityObjectsPlugin()); + await bodyKernel.use(new SharingServicePlugin()); + await bodyKernel.use(new SecurityPlugin()); + await bodyKernel.use(new AppPlugin(bodyOnly as any, undefined, { skipSeedData: true })); + await bodyKernel.bootstrap(); + bodyData = bodyKernel.getService('data'); + bodyDispatcher = new HttpDispatcher(bodyKernel); + await ensureUsers(bodyData, ['user_owen', 'user_sam']); + const grant = await bodyData.insert('sys_user_permission_set', { user_id: 'user_owen', permission_set: 'duly_member' }, { context: SYSTEM }); + bodyOwen = { request: {}, executionContext: await buildContextForUser(bodyKernel.getService('objectql'), 'user_owen', Date.now(), grant.organization_id) }; + }, 180_000); + + afterAll(async () => { + await bodyKernel?.shutdown?.(); + }); + + it('refuses a foreign owner, allows their own, and lets the system through', async () => { + const refused = await attempt(() => bodyDispatcher.handleData('/duly_task', 'POST', { subject: 'Body: for Sam', owner: 'user_sam' }, {}, bodyOwen)); + expectOwnerRefused(refused.thrown); + const allowed = await attempt(() => bodyDispatcher.handleData('/duly_task', 'POST', { subject: 'Body: mine', owner: 'user_owen' }, {}, bodyOwen)); + expect(allowed.thrown).toBeUndefined(); + const system = await bodyData.insert('duly_task', { subject: 'Body: system for Sam', owner: 'user_sam', source: 'assigned' }, { context: SYSTEM }); + expect(system.owner).toBe('user_sam'); + }); +}); From 5c46eb19368b9fd244736542df6d888ae8598df3 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 07:00:26 +0000 Subject: [PATCH 2/2] Lock task provenance with field-level security instead of readonly (#176) #178 measured that the console honours a permission set's field locks: a locked field renders read-only and is left out of the form's POST and PATCH. Re-measured on duly_task: that holds, and a payload naming a locked field is refused on every door (403 "Field write denied"). - TASK_PROVENANCE_LOCK on duly_member, duly_manager and duly_admin locks duly_task.source, owner and business_unit. A loud refusal on all three, as the seat ruled for owner, and owner keeps its required (a readonly field is validated shape-only). - readonly on the object and the duly_task_owner_is_writer guard are dropped; the lock does both jobs declaratively. - The lifecycle stamp now fills business_unit from the OWNER's primary unit, only when the insert carries none, so a caller the lock does not bind keeps a unit it names. - Three #174 tests sent source or owner, which now meet the lock first. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01JQwxbaGDrSsQpv7GZxtDyZ --- src/hooks/index.ts | 6 +- src/hooks/task.hook.ts | 139 +++----------- src/objects/task.object.ts | 28 +-- src/security/permission-sets.ts | 59 +++++- test/invariants.test.ts | 15 +- test/task-duty-access.test.ts | 11 +- test/task-hook-body.test.ts | 6 +- test/task-hook.test.ts | 6 +- test/task-provenance.test.ts | 311 ++++++++++---------------------- 9 files changed, 203 insertions(+), 378 deletions(-) diff --git a/src/hooks/index.ts b/src/hooks/index.ts index 16cc244..baa64ab 100644 --- a/src/hooks/index.ts +++ b/src/hooks/index.ts @@ -14,8 +14,8 @@ // while empty and infers correctly the moment something is pushed into it. import { DutyTimezoneGuard } from './duty.hook.js'; -import { TaskDutyReadGuard, TaskLifecycleHook, TaskOwnerGuard } from './task.hook.js'; +import { TaskDutyReadGuard, TaskLifecycleHook } from './task.hook.js'; -export { DutyTimezoneGuard, TaskDutyReadGuard, TaskLifecycleHook, TaskOwnerGuard }; +export { DutyTimezoneGuard, TaskDutyReadGuard, TaskLifecycleHook }; -export const dulyHooks = [DutyTimezoneGuard, TaskLifecycleHook, TaskDutyReadGuard, TaskOwnerGuard]; +export const dulyHooks = [DutyTimezoneGuard, TaskLifecycleHook, TaskDutyReadGuard]; diff --git a/src/hooks/task.hook.ts b/src/hooks/task.hook.ts index 55d6d7b..690933b 100644 --- a/src/hooks/task.hook.ts +++ b/src/hooks/task.hook.ts @@ -354,27 +354,35 @@ const stampTaskLifecycle = async (ctx: HookContext): Promise => { input.completed_late = completedLate(input.completed_at, input.late_after); } - // ── business_unit — the writer's unit, on a caller's insert (#176) ────── - // `business_unit` is `readonly`, so a caller's value is stripped AFTER this - // hook and nothing would be left. The system writers carry their own: the - // dispatcher copies the duty's, the fan-out reads the assignee's, the seed - // writes history. A caller's own task is the remaining case, and its owner - // is the caller (`duly_task_owner_is_writer` refuses anyone else), so the - // unit is the caller's `primary_business_unit_id` — the rung the fan-out - // already reads for a task with no duty. A key this hook writes is not a - // caller write, so the strip keeps it, even when it equals what the - // caller sent (the form echoes the field back). + // ── business_unit — the owner's unit, on a caller's insert (#176) ─────── + // A member or manager cannot name `business_unit`, `source` or `owner`: + // the Duly permission sets lock all three (`TASK_PROVENANCE_LOCK`, + // `src/security/permission-sets.ts`), so the platform refuses a payload + // that names one, and `owner` is its default — the writer. The system + // writers carry their own unit: the dispatcher copies the duty's, the + // fan-out reads the assignee's, the seed writes history. A caller's task + // with no unit is the remaining case, and it gets the owner's + // `primary_business_unit_id` — the rung the fan-out already reads for a + // task with no duty. The field lock judges the caller's payload, never + // what a hook derives, so this write lands. A caller the lock does not + // bind (a platform administrator) and who names a unit keeps it. const session = ctx.session; - if (session?.isSystem !== true && typeof session?.userId === 'string' && session.userId !== '') { + const ownerId = typeof input.owner === 'string' + ? input.owner + : input.owner !== null && typeof input.owner === 'object' && typeof (input.owner as { id?: unknown }).id === 'string' + ? (input.owner as { id: string }).id + : ''; + const unitGiven = typeof input.business_unit === 'string' && input.business_unit !== ''; + if (session?.isSystem !== true && !unitGiven && ownerId !== '') { if (!ctx.api) { - throw new Error('duly_task_lifecycle_stamps: ctx.api is unavailable, so the writer\'s business unit cannot be read.'); + throw new Error('duly_task_lifecycle_stamps: ctx.api is unavailable, so the owner\'s business unit cannot be read.'); } - const writer = await ctx.api.object('sys_user').findOne({ - where: { id: session.userId }, + const owner = await ctx.api.object('sys_user').findOne({ + where: { id: ownerId }, fields: ['primary_business_unit_id'], }); - const unit = writer?.primary_business_unit_id; - input.business_unit = typeof unit === 'string' && unit !== '' ? unit : null; + const unit = owner?.primary_business_unit_id; + if (typeof unit === 'string' && unit !== '') input.business_unit = unit; } return; } @@ -586,15 +594,15 @@ export const TaskLifecycleHook: Hook = { + 'moves them. On a predicate (bulk) write the row-conditional stamps are handled by ' + 'ADR-0058 Addendum II D3: one payload for the whole batch, so a re-stamp of an ' + 'already-done row and a completion that would be stamped late are both refused, and ' - + 'last_update_at is not stamped at all. On an insert by a caller (not a system write), ' - + 'business_unit is the writer\'s primary business unit, read elevated.', + + 'last_update_at is not stamped at all. On an insert by a caller (not a system write) that ' + + 'carries no business_unit, it is the owner\'s primary business unit, read elevated.', // Explicit because it is load-bearing rather than a default worth inheriting: // if this handler throws, the write MUST be refused. Committing a task whose // stamps were not applied is the exact silent corruption the // `completed_at_required_when_done` rule exists to make impossible. onError: 'abort', // Elevated for the two `ctx.api` reads this hook makes: `grace_days` off the - // duty the row names, and the writer's own `primary_business_unit_id`. The + // duty the row names, and the owner's `primary_business_unit_id`. The // deadline a row is born with must not depend on whether its creator can see // that duty — see the module header. // Governs `ctx.api` only; the readonly strip on this payload is unchanged. @@ -734,96 +742,3 @@ export const TaskDutyReadGuard: Hook = { runAs: 'inherit', handler: refuseUnreadableDuty, }; - -/** - * `duly_task.owner` names the writer, or the write is refused (#176). - * - * Measured on 18.2.0 against a live `pnpm demo`: a `duly_member` and a - * `duly_manager` could each create a task, or move their own, with `owner` - * set to somebody else — through data POST and PATCH and MCP create and - * update. The row then sat in that person's governed readings (Overdue, - * on-time) while that person could not see it, because `readScope: 'own'` - * resolves through `owner_id`, which is the writer (#143). - * - * ── Who may name whom ──────────────────────────────────────────────────── - * A caller names only themselves. That holds for a manager too: "Assigning is - * their only write" (AGENTS.md product invariants), and the door for a - * report's work is `duly_assignment` and its fan-out. The system writers — - * the dispatcher, the fan-out (`runAs: 'system'`), the seed — are who put work - * on other people, and `session.isSystem` lets them through. - * - * ── Why a hook, and why it refuses ────────────────────────────────────── - * `source` and `business_unit` are `readonly` on the object and the platform - * strips a caller's value. `owner` cannot be: a `readonly` field is validated - * shape-only, so its `required` stops being checked, and a system insert with - * `owner: null` then commits an ownerless task (measured). The #148 fan-out - * depends on that refusal to report a removed assignee. So `owner` stays - * writable and this guard judges it. - * - * It refuses rather than rewrites. The console form does not carry `owner` - * at all, so the only caller who meets this is one who meant somebody else, - * and a silent rewrite would hide that mistake from them. - * - * Re-sending the stored owner is not judged, so a re-save of a row whose - * owner is already someone else (written before this guard) keeps working. - * A write with no caller identity at all is not judged either: there is - * nobody to compare with, and the security layer refuses a principal-less - * write before it gets here. - * - * Self-contained, like the hooks above: the build lowers it into a sandboxed - * `body`, which carries `ctx.session`. - */ -const refuseForeignOwner = async (ctx: HookContext): Promise => { - const input = ctx.input as Record; - if (!('owner' in input)) return; - - const session = ctx.session; - if (session?.isSystem === true) return; - const writer = typeof session?.userId === 'string' ? session.userId : ''; - if (writer === '') return; - - const idOf = (value: unknown): string => { - if (typeof value === 'string') return value; - if (value !== null && typeof value === 'object' && typeof (value as { id?: unknown }).id === 'string') { - return (value as { id: string }).id; - } - return ''; - }; - - const owner = idOf(input.owner); - if (owner === writer) return; - - if (ctx.event === 'beforeUpdate') { - const previous = ctx.previous as Record | undefined; - if (previous && idOf(previous.owner) === owner) return; - } - - const message = 'Owner: you can only create or keep tasks for yourself. ' - + 'To give work to someone else, send them an assignment.'; - const error = new Error(message) as Error & { code: string; fields: unknown[] }; - error.name = 'ValidationError'; - error.code = 'VALIDATION_FAILED'; - error.fields = [{ - field: 'owner', - code: 'owner_not_writer', - message, - label: 'Owner', - value: owner, - }]; - throw error; -}; - -export const TaskOwnerGuard: Hook = { - name: 'duly_task_owner_is_writer', - label: 'Task owner is the writer', - object: 'duly_task', - events: ['beforeInsert', 'beforeUpdate'], - description: - 'Refuses a duly_task write by a caller that names someone else as owner — an insert, or an ' - + 'update changing owner. A caller creates tasks only for themselves; work for another ' - + 'person is an assignment. System writes (the dispatcher, the assignment fan-out, the seed) ' - + 'pass, and re-sending the stored owner is not checked.', - // A guard that fails open is not a guard. - onError: 'abort', - handler: refuseForeignOwner, -}; diff --git a/src/objects/task.object.ts b/src/objects/task.object.ts index 48fa7b7..3611a0a 100644 --- a/src/objects/task.object.ts +++ b/src/objects/task.object.ts @@ -113,15 +113,12 @@ export const Task = ObjectSchema.create({ description: 'Empty for a bare one-off that was never modelled as a duty.', }), - // A caller names only themselves (#176): `duly_task_owner_is_writer` - // refuses anyone else. The dispatcher, the fan-out and the seed are the - // writers that put work on other people. - // - // ⛔ NOT `readonly`, unlike `source` and `business_unit` beside it. A - // readonly field is validated shape-only, so `required` stops being - // checked and a system insert with `owner: null` commits an ownerless task - // (measured on 18.2.0). The #148 fan-out relies on that refusal to report a - // removed assignee, and "one task has exactly one owner" is an invariant. + // `owner`, `source` and `business_unit` are not a caller's to write + // (#176): the Duly permission sets lock all three + // (`TASK_PROVENANCE_LOCK`, `src/security/permission-sets.ts`). The + // defaults and `task.hook.ts` fill them for a caller; the dispatcher, the + // fan-out and the seed write them as the system. Not `readonly` here — a + // readonly field skips `required`, and `owner` must stay required. owner: Field.user({ label: 'Owner', group: 'basics', @@ -129,15 +126,9 @@ export const Task = ObjectSchema.create({ defaultValue: 'current_user', }), - // Server-owned (#176). `readonly` strips a caller's value on insert and - // update; the system writers are exempt and carry their own — the - // dispatcher copies the duty's, the fan-out reads the assignee's. A - // caller's own task gets the writer's primary business unit from - // `task.hook.ts`. Before this, a member could file a row under any unit. business_unit: Field.lookup('sys_business_unit', { label: 'Business unit', group: 'basics', - readonly: true, description: 'Denormalised from the owner when the task is created, so rollups survive a later transfer. Set by the server, never by hand.', }), @@ -149,17 +140,10 @@ export const Task = ObjectSchema.create({ // Same caliber rule as the duty. Metrics read this column and only this // column; `self` never enters an on-time rate or a comparison. - // - // Server-owned (#176). A member could mark their own work `catalog` and - // enter every governed reading. `readonly` strips a caller's value on - // insert (re-defaulted to `self`) and on update; the dispatcher, the - // fan-out and the seed write as the system and are exempt. So where a task - // came from is decided by how it was made, never by who typed it. source: Field.select({ label: 'Source', group: 'basics', required: true, - readonly: true, options: [ { label: 'Role catalog', value: 'catalog', color: '#16515F' }, { label: 'Assigned by manager', value: 'assigned', color: '#8C6512' }, diff --git a/src/security/permission-sets.ts b/src/security/permission-sets.ts index fd517fc..631dbbc 100644 --- a/src/security/permission-sets.ts +++ b/src/security/permission-sets.ts @@ -64,11 +64,14 @@ import type { ObjectPermission } from '@objectstack/spec/security'; * read-open, write-owned), which is why the admin entry has to say `org` for * "full control" to mean anything at all. * - * ── No `fields` block ──────────────────────────────────────────────────── - * Nothing in this schema needs masking: the server-owned columns + * ── One `fields` block: the task's provenance (#176) ─────────────────── + * Nothing in this schema needs MASKING: the server-owned columns * (`completed_at`, `last_update_at`, `last_dispatched_period`) are already * `readonly` on the object, which is the enforced surface. An FLS entry that * repeats a readonly flag is declared-and-unenforced twice over. + * + * The one field map is a WRITE lock, `TASK_PROVENANCE_LOCK` below, and it is + * there because `readonly` cannot do that job. See its note. */ /** @@ -252,6 +255,55 @@ const ADMIN_OBJECTS = { }, } satisfies Record; +/** + * `duly_task.source`, `owner` and `business_unit` — where a task came from, + * whose it is, and which unit it is filed under — are not a caller's to write + * (#176). + * + * Measured on 18.2.0 before this lock: a member and a manager could each set + * all three on their own tasks through data POST and PATCH and MCP create and + * update. `source: catalog` put self-made work into every governed reading + * (`src/datasets/governed.ts`). `owner` naming someone else put a row into + * that person's Overdue that they could not see (#143), and `business_unit` + * filed it under any unit. + * + * With this lock the platform's field-level write gate refuses any payload + * that NAMES one of the three: `403 Field write denied: not permitted to edit + * []`, on every door. Nobody needs to name them: + * + * - `owner` defaults to the writer (`current_user`), so a caller's task is + * theirs. A manager is no different: "Assigning is their only write", and + * the door for a report's work is `duly_assignment` and its fan-out. + * - `source` defaults to `self`, which is what a hand-made task is. + * - `business_unit` is stamped by `duly_task_lifecycle_stamps` from the + * owner's unit. The gate judges the caller's payload, not a hook's write. + * + * The console honours it: on the task form Source and Business unit render + * read-only and are left out of the POST and PATCH, so the form keeps saving + * (measured in en-US and zh-CN). Owner is not on the form at all. + * + * The writers that put work on other people are SYSTEM writes, which the gate + * does not judge: the dispatcher, the assignment fan-out (`runAs: 'system'`), + * and the seed. + * + * ⛔ Why not `readonly` on the object. Measured: a `readonly` field is + * validated shape-only, so `required` stops being checked, and a system + * insert with `owner: null` then commits an ownerless task. The #148 fan-out + * depends on that refusal to report a removed assignee. And a readonly strip + * is silent, where this is a refusal the caller can see. + * + * The same map is on all three sets, because the gate is a most-permissive + * merge in which only declaring sets vote: a set that says nothing lifts + * nothing, while a set that said `editable: true` would lift the others. + * No Duly role hand-writes another person's task, so no set lifts it. A + * platform administrator is not bound by field permissions. + */ +const TASK_PROVENANCE_LOCK = { + 'duly_task.source': { readable: true, editable: false }, + 'duly_task.owner': { readable: true, editable: false }, + 'duly_task.business_unit': { readable: true, editable: false }, +}; + /** * `duly_member` — the baseline every Duly user holds. * @@ -267,6 +319,7 @@ export const MemberPermissionSet = definePermissionSet({ description: 'Owns duties, works tasks, keeps a personal work log. Everything is scoped to the holder’s own records.', objects: MEMBER_OBJECTS, + fields: TASK_PROVENANCE_LOCK, systemPermissions: [DULY_TASK_UPDATE_STATUS], tabPermissions: { duly_app: 'visible' }, }); @@ -280,6 +333,7 @@ export const ManagerPermissionSet = definePermissionSet({ description: 'Everything a team member has, plus unit-and-below read on tasks and duties and the ability to raise assignments. Writes no one else’s status.', objects: MANAGER_OBJECTS, + fields: TASK_PROVENANCE_LOCK, systemPermissions: [DULY_TASK_UPDATE_STATUS], tabPermissions: { duly_app: 'visible' }, }); @@ -297,6 +351,7 @@ export const AdminPermissionSet = definePermissionSet({ description: 'Everything a manager has, plus full control of the catalog and an org-wide read of duties. No View All / Modify All, and no widened read on the personal work log.', objects: ADMIN_OBJECTS, + fields: TASK_PROVENANCE_LOCK, systemPermissions: [DULY_TASK_UPDATE_STATUS, DULY_CATALOG_APPLY, DULY_CATALOG_SYNC], tabPermissions: { duly_app: 'visible' }, }); diff --git a/test/invariants.test.ts b/test/invariants.test.ts index 323f852..7106cf4 100644 --- a/test/invariants.test.ts +++ b/test/invariants.test.ts @@ -123,20 +123,7 @@ describe('product invariants', () => { ).toBeDefined(); } - // #176 — two readonly columns are PROVENANCE, not stamps: where the task - // came from and which unit it is filed under. They are server-owned so a - // caller cannot forge them, and they stay in Basics because they say what - // the task is. Named here, so a third one is a decision and not a drift. - const PROVENANCE = ['business_unit', 'source']; - for (const name of PROVENANCE) { - const field = (Task.fields as Record)[name]; - expect(field?.readonly, `duly_task.${name} is server-owned (#176)`).toBe(true); - expect(field?.group, `duly_task.${name} stays in Basics`).toBe('basics'); - } - - const readonlyFields = entries - .filter(([name, f]) => f.readonly === true && !PROVENANCE.includes(name)) - .map(([name]) => name).sort(); + const readonlyFields = entries.filter(([, f]) => f.readonly === true).map(([name]) => name).sort(); const historyFields = entries.filter(([, f]) => f.group === 'history').map(([name]) => name).sort(); expect(historyFields, 'the history section IS the set of server-owned stamps') .toEqual(readonlyFields); diff --git a/test/task-duty-access.test.ts b/test/task-duty-access.test.ts index 7e13f49..cfe3375 100644 --- a/test/task-duty-access.test.ts +++ b/test/task-duty-access.test.ts @@ -165,13 +165,18 @@ describe('the premise, measured on this boot', () => { describe('every door refuses a duty the writer cannot read', () => { it('data POST', async () => { const before = (await tasksOn(otherDuty)).length; - const res = await post({ subject: 'On Sam\'s duty', duty: otherDuty, due_date: DUE, source: 'catalog' }); + const res = await post({ subject: 'On Sam\'s duty', duty: otherDuty, due_date: DUE }); expectNotFound(res.thrown, otherDuty); expect((await tasksOn(otherDuty)).length).toBe(before); }); it('data POST naming the duty\'s own owner — the write that would pre-empt their dispatch', async () => { - const res = await post({ subject: 'As Sam', duty: otherDuty, owner: 'user_sam', period_key: '2026-11', due_date: DUE }); + // Since #176 the owner key alone is refused, before this guard is asked: + // a member may not name anyone else as owner (`TASK_PROVENANCE_LOCK`). + const named = await post({ subject: 'As Sam', duty: otherDuty, owner: 'user_sam', period_key: '2026-11', due_date: DUE }); + expect(String(named.thrown?.message ?? named.thrown?.error?.message ?? named.thrown)).toContain('Field write denied'); + // And without it, the duty is still refused here. + const res = await post({ subject: 'Next period', duty: otherDuty, period_key: '2026-11', due_date: DUE }); expectNotFound(res.thrown, otherDuty); }); @@ -232,7 +237,7 @@ describe('every door refuses a duty the writer cannot read', () => { }); it('MCP create_record', async () => { - const res = await attempt(() => mcp().create('duly_task', { subject: 'Via MCP', duty: otherDuty, source: 'catalog' })); + const res = await attempt(() => mcp().create('duly_task', { subject: 'Via MCP', duty: otherDuty })); expect(res.ok?.id ?? res.ok?.record?.id, 'no row came back').toBeUndefined(); expectNotFound(res.thrown, otherDuty); }); diff --git a/test/task-hook-body.test.ts b/test/task-hook-body.test.ts index e7e4edd..9e57ada 100644 --- a/test/task-hook-body.test.ts +++ b/test/task-hook-body.test.ts @@ -103,7 +103,7 @@ describe('the lowered body', () => { expect(lowered.source).toContain("ctx.api.object(\"duly_duty\").findOne"); }); - it('makes exactly two elevated calls: grace_days off the duty, and the writer\'s unit (#176)', () => { + it('makes exactly two elevated calls: grace_days off the duty, and the owner\'s unit (#176)', () => { // `runAs: 'system'` elevates the body's own `ctx.api` calls and nothing // else — the triggering task write keeps its caller's authorization (the // spec's `runAs` scope note; measured live on `pnpm demo` for #100: a @@ -111,14 +111,14 @@ describe('the lowered body', () => { // task, and still gets 404 updating a task that is not theirs). So the // whole of what the elevation reaches is these two reads, and they are // pinned: the duty's grace, and — on a caller's insert only (#176) — the - // caller's OWN `primary_business_unit_id`, keyed on `session.userId`. + // owner's `primary_business_unit_id` (the caller, unless the caller is unlocked). const calls = lowered.source.match(/ctx\.api\b[^;]*/g) ?? []; const apiCalls = calls.filter((c) => c.includes('.object(')).map((c) => c.replace(/\s+/g, ' ')); expect(apiCalls, 'two ctx.api data calls in the whole body').toHaveLength(2); expect(apiCalls[0]) .toBe('ctx.api.object("duly_duty").findOne({ where: { id }, fields: ["grace_days"] })'); expect(apiCalls[1]) - .toBe('ctx.api.object("sys_user").findOne({ where: { id: session.userId }, fields: ["primary_business_unit_id"] })'); + .toBe('ctx.api.object("sys_user").findOne({ where: { id: ownerId }, fields: ["primary_business_unit_id"] })'); }); it('stamps a task on a one-off duty with the duty\'s grace (#100)', async () => { diff --git a/test/task-hook.test.ts b/test/task-hook.test.ts index 44cb0da..f97f0ac 100644 --- a/test/task-hook.test.ts +++ b/test/task-hook.test.ts @@ -394,12 +394,10 @@ describe('last_update_at — the stagnation signal', () => { const before = task.last_update_at as string; await tick(); - // An administrative write is a system one: `business_unit` is readonly - // (#176), so a caller's value is stripped and only the system lands it. const rebadged = await data.update('duly_task', { id: task.id, business_unit: 'bu_north', - }, { context: { isSystem: true } }); + }); expect(rebadged.business_unit).toBe('bu_north'); expect( @@ -592,8 +590,6 @@ describe('completed_at on a predicate write — one payload, N rows', () => { await data.update('duly_task', { business_unit: 'bu_north' }, { multi: true, where: { id: { $in: [alreadyDone] } }, - // A backfill is a system write: `business_unit` is readonly (#176). - context: { isSystem: true }, }); const row = await read(alreadyDone); diff --git a/test/task-provenance.test.ts b/test/task-provenance.test.ts index f9df7fe..c27666a 100644 --- a/test/task-provenance.test.ts +++ b/test/task-provenance.test.ts @@ -8,11 +8,10 @@ import { SecurityPlugin, buildContextForUser } from '@objectstack/plugin-securit import { SharingServicePlugin } from '@objectstack/plugin-sharing'; import { AutomationServicePlugin } from '@objectstack/service-automation'; import { JobServicePlugin } from '@objectstack/service-job'; -import { extractHookBody } from '@objectstack/cli/hook-body'; import stack from '../objectstack.config.js'; import { Task } from '../src/objects/task.object.js'; -import { TaskOwnerGuard, dulyHooks } from '../src/hooks/index.js'; +import { AdminPermissionSet, ManagerPermissionSet, MemberPermissionSet } from '../src/security/permission-sets.js'; import { runDispatch, unbindDispatchEngine } from '../src/jobs/dispatch.job.js'; import { ensureUsers } from './support/users.js'; @@ -23,16 +22,15 @@ import { ensureUsers } from './support/users.js'; * `duly_manager` could each write `source`, `owner` and `business_unit` on a * task through every door — data POST and PATCH, MCP create and update — and * so put self-made work into the governed readings, or a row into somebody - * else's. This suite proves the fix door by door, as a real member AND a real - * manager resolved by the platform's own `buildContextForUser`, with - * `SecurityPlugin` and `SharingServicePlugin` mounted: + * else's. This suite proves the fix door by door, as a real member, a real + * manager and a member who also holds `duly_admin`, each resolved by the + * platform's own `buildContextForUser` with `SecurityPlugin` and + * `SharingServicePlugin` mounted: * - * - `source` is `readonly`: a caller's value is STRIPPED, on insert - * re-defaulted to `self`. - * - `business_unit` is `readonly`: stripped on update; on insert the - * lifecycle hook writes the writer's own primary unit. - * - `owner` naming anyone but the writer is REFUSED by - * `duly_task_owner_is_writer` (it cannot be readonly — see the hook). + * - all three fields are locked by `TASK_PROVENANCE_LOCK` on every Duly set, + * so a payload that NAMES one is refused by the field-level write gate; + * - a task that names none of them is the caller's, `self`, and filed under + * the owner's unit (the lifecycle hook's stamp). * * And the writers that put work on other people keep writing all three: the * dispatcher, the assignment fan-out and the seed. @@ -48,6 +46,7 @@ let dispatcher: HttpDispatcher; let unitOwen: string; let unitMarek: string; let unitOther: string; +let unitNadia: string; const callers: Record = {}; const attempt = async (fn: () => Promise): Promise<{ ok?: any; thrown?: any }> => { @@ -68,12 +67,14 @@ const createdId = (res: { ok?: any }): string => { const stored = async (id: string) => data.findOne('duly_task', { where: { id } }, { context: SYSTEM }); -const expectOwnerRefused = (thrown: any) => { - expect(thrown, 'the write was refused').toBeDefined(); - const message = String(thrown?.message ?? thrown?.error?.message ?? ''); - expect(message).toContain('you can only create or keep tasks for yourself'); - const fields = thrown?.fields ?? thrown?.error?.fields ?? thrown?.details?.fields; - if (fields) expect(fields).toEqual(expect.arrayContaining([expect.objectContaining({ field: 'owner', code: 'owner_not_writer' })])); +const LOCKED = ['source', 'owner', 'business_unit'] as const; + +/** The field-level write gate's refusal, naming the field. */ +const expectLocked = (thrown: any, field: string) => { + expect(thrown, `the write naming ${field} was refused`).toBeDefined(); + const message = String(thrown?.message ?? thrown?.error?.message ?? thrown?.error ?? JSON.stringify(thrown)); + expect(message).toContain('Field write denied'); + expect(message).toContain(field); }; beforeAll(async () => { @@ -96,16 +97,17 @@ beforeAll(async () => { const ql = kernel.getService('objectql'); dispatcher = new HttpDispatcher(kernel); - await ensureUsers(data, ['user_owen', 'user_marek', 'user_sam']); + await ensureUsers(data, ['user_owen', 'user_marek', 'user_nadia', 'user_sam']); const unit = async (name: string, code: string) => String((await data.insert('sys_business_unit', { name, code }, { context: SYSTEM })).id); unitOwen = await unit('Central Office', 'CEN'); unitMarek = await unit('Northgate Operations', 'OPS'); unitOther = await unit('Northgate Quality', 'QA'); + unitNadia = await unit('Ardenline Group', 'ARD'); // `primary_business_unit_id` is the platform's projection; a fixture writes // it directly, from the system, the way `test/business-unit-membership` // reads it back. - for (const [user, bu] of [['user_owen', unitOwen], ['user_marek', unitMarek], ['user_sam', unitOther]]) { + for (const [user, bu] of [['user_owen', unitOwen], ['user_marek', unitMarek], ['user_nadia', unitNadia], ['user_sam', unitOther]]) { await data.update('sys_user', { id: user, primary_business_unit_id: bu }, { context: SYSTEM }); } @@ -119,6 +121,7 @@ beforeAll(async () => { }; callers.member = await bind('user_owen', ['duly_member']); callers.manager = await bind('user_marek', ['duly_member', 'duly_manager']); + callers.admin = await bind('user_nadia', ['duly_member', 'duly_admin']); }, 180_000); afterAll(async () => { @@ -127,27 +130,31 @@ afterAll(async () => { }); describe('the metadata', () => { - it('source and business_unit are server-owned; owner is not readonly, so required still bites', () => { - const fields = Task.fields as Record; - expect(fields.source.readonly).toBe(true); - expect(fields.business_unit.readonly).toBe(true); - expect(fields.owner.readonly, 'a readonly field skips required — see the owner guard').toBeFalsy(); - expect(fields.owner.required).toBe(true); + it('every Duly set locks the three fields, and none lifts them', () => { + for (const set of [MemberPermissionSet, ManagerPermissionSet, AdminPermissionSet]) { + for (const field of LOCKED) { + const entry = (set.fields as Record | undefined)?.[`duly_task.${field}`]; + expect(entry, `${set.name} declares duly_task.${field}`).toBeDefined(); + expect(entry?.editable, `${set.name} may not edit duly_task.${field}`).toBe(false); + expect(entry?.readable, `${set.name} still reads duly_task.${field}`).toBe(true); + } + } }); - it('registers the owner guard, failing closed', () => { - expect(dulyHooks).toContain(TaskOwnerGuard); - expect(TaskOwnerGuard.object).toBe('duly_task'); - expect(TaskOwnerGuard.events).toEqual(['beforeInsert', 'beforeUpdate']); - expect(TaskOwnerGuard.onError).toBe('abort'); + it('owner is not readonly on the object, so required still bites', () => { + const fields = Task.fields as Record; + for (const field of LOCKED) expect(fields[field].readonly, `duly_task.${field}`).toBeFalsy(); + expect(fields.owner.required).toBe(true); }); }); describe.each([ ['member', 'user_owen', () => unitOwen], ['manager', 'user_marek', () => unitMarek], + ['member who also holds duly_admin', 'user_nadia', () => unitNadia], ])('as a %s', (who, self, ownUnit) => { - const ctx = () => callers[who]!; + const key = who.startsWith('member who') ? 'admin' : who; + const ctx = () => callers[key]!; const post = (body: Record) => attempt(() => dispatcher.handleData('/duly_task', 'POST', body, {}, ctx())); const patch = (id: string, body: Record) => @@ -158,163 +165,90 @@ describe.each([ expect(res.thrown, 'creating their own task').toBeUndefined(); return createdId(res); }; + const forged: Record<(typeof LOCKED)[number], () => unknown> = { + source: () => 'catalog', + owner: () => 'user_sam', + business_unit: () => unitOther, + }; - describe('a plain task of their own', () => { - it('is theirs, self-declared, and filed under their own unit', async () => { - const row = await stored(await ownTask(`${who}: plain`)); - expect(row.owner).toBe(self); - expect(row.owner_id).toBe(self); - expect(row.source).toBe('self'); - expect(row.business_unit).toBe(ownUnit()); - }); - - it('saves from the console form, which echoes source and business_unit back', async () => { - // The create form's own body (measured: `{ source: 'self', status, subject }`), - // plus an edit form's re-save of every field it shows. - const res = await post({ subject: `${who}: form`, source: 'self', status: 'open', business_unit: ownUnit() }); - expect(res.thrown).toBeUndefined(); - const id = createdId(res); - expect((await stored(id)).business_unit).toBe(ownUnit()); - const resave = await patch(id, { subject: `${who}: form, edited`, source: 'self', business_unit: ownUnit(), owner: self, note: 'n' }); - expect(resave.thrown).toBeUndefined(); - const row = await stored(id); - expect(row.subject).toBe(`${who}: form, edited`); - expect(row.note).toBe('n'); - expect(row.business_unit).toBe(ownUnit()); - }); - }); - - describe('source — stripped on every door', () => { - it('data POST: a forged catalog lands self', async () => { - const res = await post({ subject: `${who}: forged catalog`, source: 'catalog', due_date: DUE }); - expect(res.thrown).toBeUndefined(); - expect((await stored(createdId(res))).source).toBe('self'); - }); - - it('data PATCH: moving their own task to assigned leaves it self', async () => { - const id = await ownTask(`${who}: patch source`); - expect((await patch(id, { source: 'assigned', note: 'still saved' })).thrown).toBeUndefined(); - const row = await stored(id); - expect(row.source).toBe('self'); - expect(row.note, 'the rest of the write lands').toBe('still saved'); - }); - - it('MCP create_record and update_record', async () => { - const created = await attempt(() => mcp().create('duly_task', { subject: `${who}: mcp source`, source: 'catalog' })); - expect(created.thrown).toBeUndefined(); - const id = createdId(created); - expect((await stored(id)).source).toBe('self'); - expect((await attempt(() => mcp().update('duly_task', id, { source: 'catalog' }))).thrown).toBeUndefined(); - expect((await stored(id)).source).toBe('self'); - }); - - it('bulk updateMany', async () => { - const a = await ownTask(`${who}: bulk source`); - await attempt(() => kernel.getService('protocol').updateManyData({ - object: 'duly_task', - records: [{ id: a, data: { source: 'catalog' } }], - context: ctx().executionContext, - })); - expect((await stored(a)).source).toBe('self'); - }); + it('a task naming none of them is theirs, self-declared, and filed under their unit', async () => { + // The console form's own body: Source and Business unit are left out. + const res = await post({ subject: `${who}: plain`, status: 'open' }); + expect(res.thrown).toBeUndefined(); + const row = await stored(createdId(res)); + expect(row.owner).toBe(self); + expect(row.owner_id).toBe(self); + expect(row.source).toBe('self'); + expect(row.business_unit).toBe(ownUnit()); }); - describe('business_unit — the writer\'s own, on every door', () => { - it('data POST: a foreign unit lands as their own', async () => { - const res = await post({ subject: `${who}: foreign unit`, business_unit: unitOther }); - expect(res.thrown).toBeUndefined(); - expect((await stored(createdId(res))).business_unit).toBe(ownUnit()); - }); - - it('data PATCH: moving their own task to a foreign unit leaves it', async () => { - const id = await ownTask(`${who}: patch unit`); - expect((await patch(id, { business_unit: unitOther })).thrown).toBeUndefined(); - expect((await stored(id)).business_unit).toBe(ownUnit()); - }); - - it('MCP create_record and update_record', async () => { - const created = await attempt(() => mcp().create('duly_task', { subject: `${who}: mcp unit`, business_unit: unitOther })); - expect(created.thrown).toBeUndefined(); - const id = createdId(created); - expect((await stored(id)).business_unit).toBe(ownUnit()); - expect((await attempt(() => mcp().update('duly_task', id, { business_unit: unitOther }))).thrown).toBeUndefined(); - expect((await stored(id)).business_unit).toBe(ownUnit()); - }); - - it('bulk updateMany', async () => { - const a = await ownTask(`${who}: bulk unit`); - await attempt(() => kernel.getService('protocol').updateManyData({ - object: 'duly_task', - records: [{ id: a, data: { business_unit: unitOther } }], - context: ctx().executionContext, - })); - expect((await stored(a)).business_unit).toBe(ownUnit()); - }); + it('an edit that names none of them saves', async () => { + const id = await ownTask(`${who}: edit`); + const res = await patch(id, { subject: `${who}: edited`, note: 'n' }); + expect(res.thrown).toBeUndefined(); + const row = await stored(id); + expect([row.subject, row.note, row.business_unit]).toEqual([`${who}: edited`, 'n', ownUnit()]); }); - describe('owner — refused on every door', () => { - it('data POST naming someone else', async () => { - const res = await post({ subject: `${who}: for Sam`, owner: 'user_sam' }); - expectOwnerRefused(res.thrown); - const rows = await data.find('duly_task', { where: { subject: `${who}: for Sam` } }, { context: SYSTEM }); - expect(rows ?? []).toHaveLength(0); - }); - - it('data POST naming themselves is fine', async () => { - const res = await post({ subject: `${who}: for me`, owner: self }); - expect(res.thrown).toBeUndefined(); - expect((await stored(createdId(res))).owner).toBe(self); + describe.each(LOCKED)('%s', (field) => { + it('data POST naming it is refused, and nothing is written', async () => { + const subject = `${who}: POST ${field}`; + expectLocked((await post({ subject, [field]: forged[field]() })).thrown, field); + expect((await data.find('duly_task', { where: { subject } }, { context: SYSTEM })) ?? []).toHaveLength(0); }); - it('data PATCH handing their own task to someone else', async () => { - const id = await ownTask(`${who}: hand over`); - expectOwnerRefused((await patch(id, { owner: 'user_sam' })).thrown); - expect((await stored(id)).owner).toBe(self); + it('data PATCH naming it is refused', async () => { + const id = await ownTask(`${who}: PATCH ${field}`); + const before = await stored(id); + expectLocked((await patch(id, { [field]: forged[field](), note: 'with it' })).thrown, field); + const after = await stored(id); + expect(after[field]).toEqual(before[field]); + expect(after.note ?? null, 'nothing of the write lands').toBeNull(); }); - it('MCP create_record and update_record', async () => { - expectOwnerRefused((await attempt(() => mcp().create('duly_task', { subject: `${who}: mcp for Sam`, owner: 'user_sam' }))).thrown); - const id = await ownTask(`${who}: mcp hand over`); - expectOwnerRefused((await attempt(() => mcp().update('duly_task', id, { owner: 'user_sam' }))).thrown); - expect((await stored(id)).owner).toBe(self); + it('MCP create_record and update_record naming it are refused', async () => { + expectLocked((await attempt(() => mcp().create('duly_task', { subject: `${who}: MCP ${field}`, [field]: forged[field]() }))).thrown, field); + const id = await ownTask(`${who}: MCP update ${field}`); + const before = await stored(id); + expectLocked((await attempt(() => mcp().update('duly_task', id, { [field]: forged[field]() }))).thrown, field); + expect((await stored(id))[field]).toEqual(before[field]); }); - it('bulk updateMany', async () => { - const a = await ownTask(`${who}: bulk hand over`); + it('bulk updateMany naming it does not land', async () => { + const id = await ownTask(`${who}: bulk ${field}`); + const before = await stored(id); const res = await attempt(() => kernel.getService('protocol').updateManyData({ object: 'duly_task', - records: [{ id: a, data: { owner: 'user_sam' } }], + records: [{ id, data: { [field]: forged[field]() } }], context: ctx().executionContext, })); if (res.thrown === undefined) { const rows = res.ok?.results ?? res.ok?.records ?? []; expect(rows.length, JSON.stringify(res.ok)).toBe(1); - expect(rows[0].success).toBe(false); + expect(rows[0].success, JSON.stringify(rows[0])).toBe(false); } else { - expectOwnerRefused(res.thrown); + expectLocked(res.thrown, field); } - expect((await stored(a)).owner).toBe(self); + expect((await stored(id))[field]).toEqual(before[field]); }); - it('a predicate (multi) update over their own tasks', async () => { - const subject = `${who}: predicate hand over`; - const a = await ownTask(subject); - const res = await attempt(() => data.update('duly_task', { owner: 'user_sam' }, { + it('a predicate (multi) update naming it is refused', async () => { + const subject = `${who}: predicate ${field}`; + const id = await ownTask(subject); + const before = await stored(id); + expectLocked((await attempt(() => data.update('duly_task', { [field]: forged[field]() }, { where: { subject }, multi: true, context: ctx().executionContext, - })); - expectOwnerRefused(res.thrown); - expect((await stored(a)).owner).toBe(self); + }))).thrown, field); + expect((await stored(id))[field]).toEqual(before[field]); }); + }); - it('a re-save of a row whose stored owner is already someone else passes', async () => { - // Rows written before this guard can carry a foreign owner. A re-save - // that re-sends it is not a change, so it is not judged. - const id = await ownTask(`${who}: legacy row`); - await data.update('duly_task', { id, owner: 'user_sam' }, { context: SYSTEM }); - const res = await patch(id, { owner: 'user_sam', note: 'touched' }); - expect(res.thrown).toBeUndefined(); - expect((await stored(id)).note).toBe('touched'); - }); + it('naming the stored or their own value is refused too: the lock is on the key, not the value', async () => { + // Documented, not incidental. The console leaves locked fields out, so + // only an API caller meets this, and the answer is to leave them out. + expectLocked((await post({ subject: `${who}: own owner named`, owner: self })).thrown, 'owner'); + const id = await ownTask(`${who}: resend`); + expectLocked((await patch(id, { source: 'self' })).thrown, 'source'); }); }); @@ -385,54 +319,3 @@ describe('the writers that put work on other people keep writing all three', () expect([back.owner, back.business_unit, back.source]).toEqual(['user_owen', unitOwen, 'assigned']); }); }); - -describe('the owner guard as it ships — lowered, and run in QuickJS', () => { - let bodyKernel: any; - let bodyData: any; - let bodyDispatcher: HttpDispatcher; - let bodyOwen: HttpProtocolContext; - - beforeAll(async () => { - const lowered = extractHookBody(TaskOwnerGuard.handler as (...a: unknown[]) => unknown, 'task.hook.ts'); - expect(lowered.isExpression).toBe(false); - expect(lowered.capabilities, 'the guard reads nothing').toEqual([]); - const bodyOnly = { - ...stack, - hooks: (stack.hooks ?? []).map((hook: any) => - hook.name === TaskOwnerGuard.name - ? { ...hook, handler: undefined, body: { language: 'js', source: lowered.source, capabilities: lowered.capabilities } } - : hook), - }; - const { plugins } = await createStandaloneStack({ - databaseDriver: 'sqlite', - databaseUrl: ':memory:', - skipSeedData: true, - artifactPath: 'dist/objectstack.this-suite-must-not-load-an-artifact.json', - }); - bodyKernel = new ObjectKernel(); - for (const plugin of plugins) await bodyKernel.use(plugin); - await bodyKernel.use(createIdentityObjectsPlugin()); - await bodyKernel.use(new SharingServicePlugin()); - await bodyKernel.use(new SecurityPlugin()); - await bodyKernel.use(new AppPlugin(bodyOnly as any, undefined, { skipSeedData: true })); - await bodyKernel.bootstrap(); - bodyData = bodyKernel.getService('data'); - bodyDispatcher = new HttpDispatcher(bodyKernel); - await ensureUsers(bodyData, ['user_owen', 'user_sam']); - const grant = await bodyData.insert('sys_user_permission_set', { user_id: 'user_owen', permission_set: 'duly_member' }, { context: SYSTEM }); - bodyOwen = { request: {}, executionContext: await buildContextForUser(bodyKernel.getService('objectql'), 'user_owen', Date.now(), grant.organization_id) }; - }, 180_000); - - afterAll(async () => { - await bodyKernel?.shutdown?.(); - }); - - it('refuses a foreign owner, allows their own, and lets the system through', async () => { - const refused = await attempt(() => bodyDispatcher.handleData('/duly_task', 'POST', { subject: 'Body: for Sam', owner: 'user_sam' }, {}, bodyOwen)); - expectOwnerRefused(refused.thrown); - const allowed = await attempt(() => bodyDispatcher.handleData('/duly_task', 'POST', { subject: 'Body: mine', owner: 'user_owen' }, {}, bodyOwen)); - expect(allowed.thrown).toBeUndefined(); - const system = await bodyData.insert('duly_task', { subject: 'Body: system for Sam', owner: 'user_sam', source: 'assigned' }, { context: SYSTEM }); - expect(system.owner).toBe('user_sam'); - }); -});