Lock a task's source, owner and business unit against callers

src/hooks/task.hook.ts

@@ -353,6 +353,37 @@ const stampTaskLifecycle = async (ctx: HookContext): Promise<void> => {
     if (input.status === 'done') {
       input.completed_late = completedLate(input.completed_at, input.late_after);
     }
+
+    // ── 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;
+    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 owner\'s business unit cannot be read.');
+      }
+      const owner = await ctx.api.object('sys_user').findOne({
+        where: { id: ownerId },
+        fields: ['primary_business_unit_id'],
+      });
+      const unit = owner?.primary_business_unit_id;
+      if (typeof unit === 'string' && unit !== '') input.business_unit = unit;
+    }
     return;
   }
 
@@ -563,15 +594,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) 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 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 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.
   runAs: 'system',
   handler: stampTaskLifecycle,

src/objects/task.object.ts

@@ -113,6 +113,12 @@ export const Task = ObjectSchema.create({
       description: 'Empty for a bare one-off that was never modelled as a duty.',
     }),
 
+    // `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',
@@ -123,7 +129,7 @@ export const Task = ObjectSchema.create({
     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.',
+      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', {

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<string, ObjectPermission>;
 
+/**
+ * `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
+ * [<field>]`, 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' },
 });

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);
   });

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 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
     // 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
+    // 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('));
-    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: ownerId }, fields: ["primary_business_unit_id"] })');
   });
 
   it('stamps a task on a one-off duty with the duty\'s grace (#100)', async () => {