fix(approvals): guard self-service OOO delegation writes + i18n (#3255)

os-zhuang · claude · web-flow · commit 611402f3c0a3 · 2026-07-19T14:39:10.000+08:00
Follow-up hardening on the merged OOO delegation feature (#3235). Security — delegator forge guard: sys_approval_delegation is apiEnabled CRUD, but as a system object it gets no owner_id anchor and defaults to a `public` sharing model, so an unguarded member could create a delegation naming SOMEONE ELSE as delegator and reroute that victim's individually-routed approvals to themselves. bindDelegationWrite- Guard (plugin-approvals beforeInsert/beforeUpdate, mirroring the ADR-0092 identity write-guard) forces a normal user's writes to name themselves as delegator: system context bypasses, admins (roles include 'admin') may set any delegator, everyone else is stamped-to-self on insert and rejected on a foreign delegator. Row ownership on update/delete is already covered by member_default's wildcard `created_by == current_user.id` RLS. i18n: Register sys_approval_delegation in the plugin's i18n extract config and add its object/field/view translations. zh-CN fully translated; en/ja-JP/es-ES carry the English baseline pending translation. (Blocks hand-added to avoid a full re-extract dropping existing enum/view keys.) Tests: 9 new guard cases (self-create, forge reject on insert/update, stamp on omit, unauthenticated reject, system + admin bypass, batch). plugin-approvals 123 passed. Claude-Session: https://claude.ai/code/session_016ypkQikZ55oWUHUnMebwXA Co-authored-by: Claude <noreply@anthropic.com>
diff --git a/packages/plugins/plugin-approvals/scripts/i18n-extract.config.ts b/packages/plugins/plugin-approvals/scripts/i18n-extract.config.ts
@@ -15,14 +15,15 @@
 import { defineStack } from '@objectstack/spec';
 import { SysApprovalRequest } from '../src/sys-approval-request.object.js';
 import { SysApprovalAction } from '../src/sys-approval-action.object.js';
+import { SysApprovalDelegation } from '../src/sys-approval-delegation.object.js';
 import { enObjects } from '../src/translations/en.objects.generated.js';
 import { zhCNObjects } from '../src/translations/zh-CN.objects.generated.js';
 import { jaJPObjects } from '../src/translations/ja-JP.objects.generated.js';
 import { esESObjects } from '../src/translations/es-ES.objects.generated.js';
 
 export default defineStack({
   name: 'plugin-approvals-i18n-extract',
-  objects: [SysApprovalRequest, SysApprovalAction] as any,
+  objects: [SysApprovalRequest, SysApprovalAction, SysApprovalDelegation] as any,
   translations: [
     { en: { objects: enObjects } },
     { 'zh-CN': { objects: zhCNObjects } },
diff --git a/packages/plugins/plugin-approvals/src/approval-service.test.ts b/packages/plugins/plugin-approvals/src/approval-service.test.ts
@@ -11,7 +11,7 @@
 
 import { describe, it, expect, beforeEach } from 'vitest';
 import { ApprovalService, REMIND_COOLDOWN_MS } from './approval-service.js';
-import { bindApprovalLockHook, unbindAllHooks } from './lifecycle-hooks.js';
+import { bindApprovalLockHook, bindDelegationWriteGuard, unbindAllHooks } from './lifecycle-hooks.js';
 
 interface FakeRow { [k: string]: any }
 
@@ -1223,3 +1223,67 @@ describe('ApprovalService — out-of-office delegation (#1322)', () => {
     expect(req.pending_approvers).toEqual(['bob']);
   });
 });
+
+// ── Delegation self-service write guard (#1322 follow-up) ─────────────
+//
+// sys_approval_delegation is apiEnabled CRUD; a member must not be able to
+// forge a delegation for someone else (delegator_id = victim) and reroute the
+// victim's approvals. The guard forces delegator_id == acting user for normal
+// writes; system/admin contexts bypass. Row-ownership on update/delete is the
+// platform's created_by RLS (not exercised here).
+describe('sys_approval_delegation write guard (#1322)', () => {
+  const DEL = 'sys_approval_delegation';
+  let engine: ReturnType<typeof makeFakeEngine>;
+
+  beforeEach(() => {
+    engine = makeFakeEngine();
+    bindDelegationWriteGuard(engine as any);
+  });
+
+  const fireInsert = (data: any, session: any) =>
+    (engine as any).fire('beforeInsert', { object: DEL, input: { data }, session });
+  const fireUpdate = (data: any, session: any) =>
+    (engine as any).fire('beforeUpdate', { object: DEL, input: { id: data?.id ?? 'd1', data }, session });
+  const member = (userId?: string) => ({ isSystem: false, roles: [], ...(userId ? { userId } : {}) });
+
+  it('allows a member to create their own delegation', async () => {
+    await expect(fireInsert({ delegator_id: 'u1', delegate_id: 'u2' }, member('u1'))).resolves.toBeUndefined();
+  });
+
+  it('rejects a member forging a delegation for someone else', async () => {
+    await expect(fireInsert({ delegator_id: 'victim', delegate_id: 'u1' }, member('u1'))).rejects.toThrow(/FORBIDDEN/);
+  });
+
+  it('stamps the caller as delegator when omitted on insert', async () => {
+    const data: any = { delegate_id: 'u2' };
+    await fireInsert(data, member('u1'));
+    expect(data.delegator_id).toBe('u1');
+  });
+
+  it('rejects an unauthenticated non-system insert', async () => {
+    await expect(fireInsert({ delegate_id: 'u2' }, member())).rejects.toThrow(/FORBIDDEN/);
+  });
+
+  it('bypasses the guard for system context', async () => {
+    await expect(fireInsert({ delegator_id: 'victim', delegate_id: 'u1' }, { isSystem: true })).resolves.toBeUndefined();
+  });
+
+  it('lets an admin set the delegator to anyone', async () => {
+    await expect(fireInsert({ delegator_id: 'victim', delegate_id: 'u2' }, { isSystem: false, roles: ['admin'], userId: 'admin1' })).resolves.toBeUndefined();
+  });
+
+  it('rejects a member relabelling delegator on update', async () => {
+    await expect(fireUpdate({ id: 'd1', delegator_id: 'victim' }, member('u1'))).rejects.toThrow(/FORBIDDEN/);
+  });
+
+  it('allows a member update that does not touch delegator_id', async () => {
+    await expect(fireUpdate({ id: 'd1', valid_until: '2026-06-01T00:00:00Z' }, member('u1'))).resolves.toBeUndefined();
+  });
+
+  it('rejects a batch insert if any row names a foreign delegator', async () => {
+    await expect(fireInsert(
+      [{ delegator_id: 'u1', delegate_id: 'u2' }, { delegator_id: 'victim', delegate_id: 'u3' }],
+      member('u1'),
+    )).rejects.toThrow(/FORBIDDEN/);
+  });
+});
diff --git a/packages/plugins/plugin-approvals/src/approvals-plugin.ts b/packages/plugins/plugin-approvals/src/approvals-plugin.ts
@@ -13,7 +13,7 @@ import {
   ESCALATION_SCAN_INTERVAL_MS,
   type ApprovalEngine,
 } from './approval-service.js';
-import { bindApprovalLockHook, unbindAllHooks } from './lifecycle-hooks.js';
+import { bindApprovalLockHook, bindDelegationWriteGuard, unbindAllHooks } from './lifecycle-hooks.js';
 import { registerApprovalNode, type ApprovalAutomationSurface } from './approval-node.js';
 
 export interface ApprovalsPluginOptions {
@@ -124,12 +124,16 @@ export class ApprovalsServicePlugin implements Plugin {
     });
 
     // Record lock: block edits to a record while it has a pending request.
+    // Delegation write-guard: a self-service OOO delegation may only name the
+    // acting user as delegator (#1322 follow-up). Both bind under the same
+    // package id, so unbindAllHooks clears them together.
     if (!this.options.disableAutoHooks) {
       try {
         unbindAllHooks(engine);
         bindApprovalLockHook(engine, ctx.logger);
+        bindDelegationWriteGuard(engine, ctx.logger);
       } catch (err: any) {
-        ctx.logger.warn?.('[approvals] failed to bind record-lock hook', { error: err?.message });
+        ctx.logger.warn?.('[approvals] failed to bind approval hooks', { error: err?.message });
       }
     }
 
diff --git a/packages/plugins/plugin-approvals/src/lifecycle-hooks.ts b/packages/plugins/plugin-approvals/src/lifecycle-hooks.ts
@@ -109,6 +109,70 @@ export function bindApprovalLockHook(engine: MinimalEngine, logger?: MinimalLogg
   logger?.info?.('[approvals] record-lock hook bound');
 }
 
+/** The self-service out-of-office delegation object (#1322). */
+export const DELEGATION_OBJECT = 'sys_approval_delegation';
+
+/**
+ * Self-service write guard for `sys_approval_delegation` (#1322 follow-up).
+ *
+ * The object is `apiEnabled` CRUD so a user can declare their own out-of-office
+ * delegation. But it is a system object: it gets no auto `owner_id` anchor and
+ * (with no `sharingModel`) defaults to a `public` sharing model, so an
+ * unguarded member could **forge a delegation for someone else**
+ * (`delegator_id = victim`) and reroute the victim's individually-routed
+ * approvals to themselves. This guard forces a normal user's writes to name
+ * themselves as the delegator:
+ *
+ *   - **system** context (service / seed / import) → bypass;
+ *   - **admin** (`roles` includes `'admin'`) → may set `delegator_id` to anyone;
+ *   - otherwise `delegator_id` must equal the acting user — an absent delegator
+ *     on insert is stamped to the caller, a foreign delegator is rejected.
+ *
+ * Row-level ownership on update/delete (you can only touch a delegation you
+ * created) is already enforced by `member_default`'s wildcard
+ * `created_by == current_user.id` RLS; this guard adds the delegator-identity
+ * check that RLS alone can't express. Mirrors the ADR-0092 identity write-guard
+ * shape and the security plugin's `owner_id` anchor guard, scoped to this one
+ * object.
+ */
+export function bindDelegationWriteGuard(engine: MinimalEngine, logger?: MinimalLogger): void {
+  const makeGuard = (isInsert: boolean) => async (ctx: any) => {
+    const session = (ctx?.session ?? {}) as any;
+    if (session.isSystem) return;                                    // service / seed / import
+    const roles = (session.roles ?? []) as unknown[];
+    if (Array.isArray(roles) && roles.includes('admin')) return;     // admin may act for anyone
+    const userId = session.userId != null ? String(session.userId) : '';
+    const data = ctx?.input?.data;
+    const rows = Array.isArray(data) ? data : (data && typeof data === 'object' ? [data] : []);
+    const deny = (): never => {
+      const err: any = new Error(
+        'FORBIDDEN: you may only manage out-of-office delegations where you are the delegator'
+        + (userId ? ` ('${userId}')` : ''),
+      );
+      err.code = 'FORBIDDEN';
+      err.statusCode = 403;
+      throw err;
+    };
+    for (const row of rows) {
+      if (!row || typeof row !== 'object' || Array.isArray(row)) continue;
+      const has = Object.prototype.hasOwnProperty.call(row, 'delegator_id');
+      const supplied = has ? String((row as any).delegator_id ?? '') : '';
+      if (isInsert && (!has || supplied === '')) {
+        // Self-service: stamp the caller as delegator when omitted (the schema's
+        // `required` is the fallback if the engine doesn't persist the stamp).
+        if (!userId) deny();
+        (row as any).delegator_id = userId;
+        continue;
+      }
+      // A foreign delegator on insert (forge) or update (relabel/hijack) → deny.
+      if (has && supplied !== userId) deny();
+    }
+  };
+  engine.registerHook('beforeInsert', makeGuard(true), { object: DELEGATION_OBJECT, packageId: APPROVALS_HOOK_PACKAGE, priority: 50 });
+  engine.registerHook('beforeUpdate', makeGuard(false), { object: DELEGATION_OBJECT, packageId: APPROVALS_HOOK_PACKAGE, priority: 50 });
+  logger?.info?.('[approvals] delegation write-guard bound');
+}
+
 /** Unregister every hook the lock module registered. */
 export function unbindAllHooks(engine: MinimalEngine): number {
   return engine.unregisterHooksByPackage(APPROVALS_HOOK_PACKAGE);
diff --git a/packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts
@@ -152,5 +152,54 @@ export const enObjects: NonNullable<TranslationData['objects']> = {
         label: "All"
       }
     }
+  },
+  sys_approval_delegation: {
+    label: "Approval Delegation",
+    pluralLabel: "Approval Delegations",
+    description: "Self-service out-of-office rule: route this user's approver slots to a delegate within a time window (#1322 M1).",
+    fields: {
+      id: {
+        label: "Delegation ID"
+      },
+      delegator_id: {
+        label: "Delegator",
+        help: "The user going out of office; their individually-routed approver slots are rerouted while active."
+      },
+      delegate_id: {
+        label: "Delegate",
+        help: "The backup who receives the delegator's approvals while this rule is active. Acts under their own identity."
+      },
+      valid_from: {
+        label: "Valid From",
+        help: "Rule is inactive before this instant. Null = active immediately. Enforced at resolution time via isGrantActive (ADR-0091 D2 predicate) — never by a background job."
+      },
+      valid_until: {
+        label: "Valid Until",
+        help: "Rule is inactive AT and AFTER this instant (half-open [from, until), UTC). Null = never expires."
+      },
+      reason: {
+        label: "Reason",
+        help: "Why the delegation exists (e.g. \"Annual leave 5/26\u20135/30\"). Recorded on the substitution audit row."
+      },
+      organization_id: {
+        label: "Organization",
+        help: "Tenant that owns this rule; null = applies across tenants for this delegator."
+      },
+      created_at: {
+        label: "Created At"
+      },
+      updated_at: {
+        label: "Updated At"
+      }
+    },
+    _views: {
+      active: {
+        label: "Active",
+        emptyState: {
+          title: "No delegations",
+          message: "Declare an out-of-office delegation so approvals route to a backup while you are away."
+        }
+      }
+    }
   }
 };
diff --git a/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts
@@ -152,5 +152,54 @@ export const esESObjects: NonNullable<TranslationData['objects']> = {
         label: "Todas"
       }
     }
+  },
+  sys_approval_delegation: {
+    label: "Approval Delegation",
+    pluralLabel: "Approval Delegations",
+    description: "Self-service out-of-office rule: route this user's approver slots to a delegate within a time window (#1322 M1).",
+    fields: {
+      id: {
+        label: "Delegation ID"
+      },
+      delegator_id: {
+        label: "Delegator",
+        help: "The user going out of office; their individually-routed approver slots are rerouted while active."
+      },
+      delegate_id: {
+        label: "Delegate",
+        help: "The backup who receives the delegator's approvals while this rule is active. Acts under their own identity."
+      },
+      valid_from: {
+        label: "Valid From",
+        help: "Rule is inactive before this instant. Null = active immediately. Enforced at resolution time via isGrantActive (ADR-0091 D2 predicate) — never by a background job."
+      },
+      valid_until: {
+        label: "Valid Until",
+        help: "Rule is inactive AT and AFTER this instant (half-open [from, until), UTC). Null = never expires."
+      },
+      reason: {
+        label: "Reason",
+        help: "Why the delegation exists (e.g. \"Annual leave 5/26\u20135/30\"). Recorded on the substitution audit row."
+      },
+      organization_id: {
+        label: "Organization",
+        help: "Tenant that owns this rule; null = applies across tenants for this delegator."
+      },
+      created_at: {
+        label: "Created At"
+      },
+      updated_at: {
+        label: "Updated At"
+      }
+    },
+    _views: {
+      active: {
+        label: "Active",
+        emptyState: {
+          title: "No delegations",
+          message: "Declare an out-of-office delegation so approvals route to a backup while you are away."
+        }
+      }
+    }
   }
 };
diff --git a/packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts
@@ -152,5 +152,54 @@ export const jaJPObjects: NonNullable<TranslationData['objects']> = {
         label: "すべて"
       }
     }
+  },
+  sys_approval_delegation: {
+    label: "Approval Delegation",
+    pluralLabel: "Approval Delegations",
+    description: "Self-service out-of-office rule: route this user's approver slots to a delegate within a time window (#1322 M1).",
+    fields: {
+      id: {
+        label: "Delegation ID"
+      },
+      delegator_id: {
+        label: "Delegator",
+        help: "The user going out of office; their individually-routed approver slots are rerouted while active."
+      },
+      delegate_id: {
+        label: "Delegate",
+        help: "The backup who receives the delegator's approvals while this rule is active. Acts under their own identity."
+      },
+      valid_from: {
+        label: "Valid From",
+        help: "Rule is inactive before this instant. Null = active immediately. Enforced at resolution time via isGrantActive (ADR-0091 D2 predicate) — never by a background job."
+      },
+      valid_until: {
+        label: "Valid Until",
+        help: "Rule is inactive AT and AFTER this instant (half-open [from, until), UTC). Null = never expires."
+      },
+      reason: {
+        label: "Reason",
+        help: "Why the delegation exists (e.g. \"Annual leave 5/26\u20135/30\"). Recorded on the substitution audit row."
+      },
+      organization_id: {
+        label: "Organization",
+        help: "Tenant that owns this rule; null = applies across tenants for this delegator."
+      },
+      created_at: {
+        label: "Created At"
+      },
+      updated_at: {
+        label: "Updated At"
+      }
+    },
+    _views: {
+      active: {
+        label: "Active",
+        emptyState: {
+          title: "No delegations",
+          message: "Declare an out-of-office delegation so approvals route to a backup while you are away."
+        }
+      }
+    }
   }
 };
diff --git a/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts
