From 7a2e8376fe0dff60edffba69f1b7bfaf0f68a64b Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 19 Jul 2026 06:21:55 +0000 Subject: [PATCH] fix(approvals): guard self-service OOO delegation writes + i18n (#1322) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_016ypkQikZ55oWUHUnMebwXA --- .../scripts/i18n-extract.config.ts | 3 +- .../src/approval-service.test.ts | 66 ++++++++++++++++++- .../plugin-approvals/src/approvals-plugin.ts | 8 ++- .../plugin-approvals/src/lifecycle-hooks.ts | 64 ++++++++++++++++++ .../src/translations/en.objects.generated.ts | 49 ++++++++++++++ .../translations/es-ES.objects.generated.ts | 49 ++++++++++++++ .../translations/ja-JP.objects.generated.ts | 49 ++++++++++++++ .../translations/zh-CN.objects.generated.ts | 49 ++++++++++++++ 8 files changed, 333 insertions(+), 4 deletions(-) diff --git a/packages/plugins/plugin-approvals/scripts/i18n-extract.config.ts b/packages/plugins/plugin-approvals/scripts/i18n-extract.config.ts index 3ef5114ab2..50f5158894 100644 --- a/packages/plugins/plugin-approvals/scripts/i18n-extract.config.ts +++ b/packages/plugins/plugin-approvals/scripts/i18n-extract.config.ts @@ -15,6 +15,7 @@ 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'; @@ -22,7 +23,7 @@ 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 index 5f9df404a8..e0171883f7 100644 --- 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; + + 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 index 8d1ade49dd..2ca815626e 100644 --- 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 index 2e315229ae..caa038916f 100644 --- 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 index a99416620a..043dc20a01 100644 --- 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 = { 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 index 0bc73c7222..c045cfcdae 100644 --- 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 = { 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 index e4dbfeb98f..4896d7ab71 100644 --- 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 = { 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 index 02cf827d40..76b4c04755 100644 --- a/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts +++ b/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts @@ -152,5 +152,54 @@ export const zhCNObjects: NonNullable = { label: "全部" } } + }, + sys_approval_delegation: { + label: "审批委派", + pluralLabel: "审批委派", + description: "自助不在岗规则：在时间窗内把该用户的审批人槽位改派给候补人（#1322 M1）。", + fields: { + id: { + label: "委派 ID" + }, + delegator_id: { + label: "委派人", + help: "即将不在岗的用户；规则生效期间，路由到其个人的审批人槽位将被改派。" + }, + delegate_id: { + label: "候补人", + help: "规则生效期间接收委派人审批的候补人。以其本人身份行使。" + }, + valid_from: { + label: "生效时间", + help: "此刻之前规则不生效。留空 = 立即生效。在解析时通过 isGrantActive 强制（ADR-0091 D2 判定式）——绝不依赖后台任务。" + }, + valid_until: { + label: "失效时间", + help: "此刻（含）及之后规则不生效（半开区间 [from, until)，UTC）。留空 = 永不过期。" + }, + reason: { + label: "原因", + help: "委派存在的原因（例如“年假 5/26–5/30”）。会记入改派审计行。" + }, + organization_id: { + label: "组织", + help: "拥有此规则的租户；留空 = 对该委派人跨租户生效。" + }, + created_at: { + label: "创建时间" + }, + updated_at: { + label: "更新时间" + } + }, + _views: { + active: { + label: "生效中", + emptyState: { + title: "暂无委派", + message: "声明一条不在岗委派，休假期间审批自动改派给候补人。" + } + } + } } };