diff --git a/SECURITY.md b/SECURITY.md index 86bd27d..4ac893c 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -46,7 +46,7 @@ The following are in scope: - **PII leakage** -- sensitive fields (marked `sensitive: true`) appearing in logs, event bus payloads, or rendered output. - **Redaction failures** -- the redactor (`@mobile-reality/mdma-runtime`) failing to mask, hash, or omit PII as configured. - **Event log integrity** -- tampering with or forging audit log entries (hash-chain breaks). -- **Approval gate bypass** -- approval-gate components accepting unauthorized approvers or skipping required approvals. +- **Approval gate bypass** -- the approval-gate component's `allowedRoles`, `requiredApprovers`, and `requireReason` fields are advisory metadata surfaced to the host application; the MDMA runtime does not enforce them. Host-side authentication and authorization of approvers is the responsibility of the embedding application and is out of scope for this project. Reports that hinge on those fields being enforced at the runtime layer will be closed as not-a-bug. - **Injection via bindings** -- binding expressions (`{{variable.path}}`) that resolve to executable code or leak cross-component state. - **Dependency vulnerabilities** -- known CVEs in direct dependencies that are exploitable in MDMA's usage context. diff --git a/docs/guides/creating-blueprints.md b/docs/guides/creating-blueprints.md index 398a2ad..170da6f 100644 --- a/docs/guides/creating-blueprints.md +++ b/docs/guides/creating-blueprints.md @@ -46,7 +46,7 @@ integrations: checklists: security: - PII fields (reporter_email) marked sensitive - - Approval gate enforces manager/director role + - Approval gate declares manager/director role (host application enforces — see approval-gate security model in `docs/reference/component-catalog.md`) logging: - All form submissions are logged with timestamp - Approval decisions are audit-logged @@ -234,7 +234,7 @@ The `checklists` section documents what has been verified. This serves as a comp checklists: security: - All PII fields marked sensitive - - Approval gate enforces authorized roles + - Approval gate declares authorized roles (host application enforces — MDMA does not verify approver identity) - Data encrypted at rest and in transit logging: - All form submissions audit-logged diff --git a/docs/guides/creating-documents.md b/docs/guides/creating-documents.md index 262deaa..d48780b 100644 --- a/docs/guides/creating-documents.md +++ b/docs/guides/creating-documents.md @@ -77,6 +77,8 @@ allowedRoles: In this example, the approval gate's `visible` property is bound to the `user_role` field. When `user_role` has a truthy value, the gate becomes visible. +> **Advisory.** `allowedRoles` is metadata MDMA surfaces to the host application; it is **not** a control the runtime enforces. Authenticating the user, verifying their role, and gating downstream actions are the host application's responsibility. See [approval-gate](../reference/component-catalog.md#approval-gate) for the security model. + ### Binding Syntax Rules - Must start with `{{` and end with `}}` @@ -269,6 +271,8 @@ onDeny: reopen-case requireReason: true ``` +> **Important.** `allowedRoles`, `requiredApprovers`, and `requireReason` are advisory — MDMA does not verify approver identity, enforce role membership, or require multiple distinct approvers. Host applications must authenticate users, check their roles before dispatching `APPROVAL_GRANTED`, and enforce downstream authorization on their own backend. See [approval-gate](../reference/component-catalog.md#approval-gate) for the security model. + ## Validation Checklist Before finalizing a document, verify: diff --git a/docs/guides/enterprise-features.md b/docs/guides/enterprise-features.md index 8861706..d70c99a 100644 --- a/docs/guides/enterprise-features.md +++ b/docs/guides/enterprise-features.md @@ -316,7 +316,12 @@ if (report.summary.failed > 0) { throw new Error(`Compliance check failed: ${report.summary.failed} failures`); } -// 3. Create store with policy enforcement +// 3. Create the store. +// Note: MDMA's runtime enforces policy only for the webhook integration +// (action 'webhook_call'). The approval-gate fields `allowedRoles`, +// `requiredApprovers`, and `requireReason` are advisory — the host application +// is responsible for verifying approver identity and quorum before dispatching +// APPROVAL_GRANTED. See docs/reference/component-catalog.md#approval-gate. const registry = new AttachableRegistry(); registerAllCoreAttachables(registry); diff --git a/docs/reference/component-catalog.md b/docs/reference/component-catalog.md index 369bc85..bd7a182 100644 --- a/docs/reference/component-catalog.md +++ b/docs/reference/component-catalog.md @@ -294,7 +294,20 @@ dismissible: true ## approval-gate -Blocks workflow progression until the required number of approvals is received. Supports role-based access control. +Surfaces an approval-workflow UI element. The gate's status (`pending` / `approved` / `denied`) is exposed as a binding (`{{.status}}`) for downstream components. + +> **Security model — read first.** +> +> The MDMA runtime does **not** verify approver identity, enforce role membership, or require multiple distinct approvers. The `allowedRoles`, `requiredApprovers`, and `requireReason` fields below are **advisory metadata only** — they describe the policy a host application should enforce, but MDMA itself does not enforce them. Anyone able to dispatch an `APPROVAL_GRANTED` action against the store can flip a gate to `approved` regardless of role, count, or reason. +> +> Host applications embedding MDMA are responsible for: +> +> 1. Authenticating the user and supplying a verified actor identity. +> 2. Checking that identity against `allowedRoles` before invoking `store.dispatch({ type: 'APPROVAL_GRANTED', ... })`. +> 3. Tracking distinct approvers if `requiredApprovers > 1` is intended to be meaningful. +> 4. Enforcing end-to-end authorization on any backend the approval triggers (defense in depth — client-side controls are bypassable in any browser). +> +> When the runtime detects any of these advisory fields on a gate, it emits a `console.warn` once per gate at store initialization to make the responsibility explicit. ### Properties @@ -303,11 +316,11 @@ Blocks workflow progression until the required number of approvals is received. | `type` | `"approval-gate"` | *required* | Must be `"approval-gate"`. | | `title` | `string` | *required* | Gate title. Min length 1. | | `description` | `string` | -- | Additional details about the approval. | -| `requiredApprovers` | `number` | `1` | Number of approvals needed. Positive integer. | -| `allowedRoles` | `string[]` | -- | Restrict who can approve. If omitted, anyone can approve. | +| `requiredApprovers` | `number` | `1` | **Advisory only** — not enforced by MDMA. The host application is responsible for tracking distinct approvers. Positive integer. | +| `allowedRoles` | `string[]` | -- | **Advisory only** — not enforced by MDMA. The host application is responsible for verifying the actor's role before dispatching `APPROVAL_GRANTED`. | | `onApprove` | `string` | -- | Action ID triggered when approval is granted. | | `onDeny` | `string` | -- | Action ID triggered when approval is denied. | -| `requireReason` | `boolean` | `false` | Require the denier to provide a reason. | +| `requireReason` | `boolean` | `false` | **Advisory only** — UI may surface a reason input on denial; the MDMA runtime does not block dispatch when a reason is omitted. | ### Example @@ -316,8 +329,9 @@ id: dual-approval type: approval-gate title: Production Change Approval description: > - Requires sign-off from both a tech lead and a manager. - This is a SOX compliance requirement for all production changes. + Surfaces the approval workflow for a production change. Authenticating + the signers and enforcing the role/quorum constraints below is the host + application's responsibility — see the "Security model" callout above. requiredApprovers: 2 allowedRoles: - tech-lead diff --git a/packages/runtime/src/core/document-store.ts b/packages/runtime/src/core/document-store.ts index 4229a09..7d2219c 100644 --- a/packages/runtime/src/core/document-store.ts +++ b/packages/runtime/src/core/document-store.ts @@ -75,6 +75,8 @@ export function createDocumentStore( redactionCtx.sensitiveComponents.add(comp.id); } + warnIfAdvisoryApprovalFields(comp); + // Extract sensitive fields from form components if (comp.type === 'form') { for (const field of comp.fields) { @@ -286,6 +288,8 @@ export function createDocumentStore( redactionCtx.sensitiveComponents.add(comp.id); } + warnIfAdvisoryApprovalFields(comp); + if (comp.type === 'form') { for (const field of comp.fields) { if (field.sensitive) { @@ -340,6 +344,44 @@ function isPendingMdmaBlock(node: unknown): boolean { ); } +/** + * approval-gate's `allowedRoles`, `requiredApprovers`, and `requireReason` fields + * are accepted by the schema and surfaced to the host UI, but the MDMA runtime + * does NOT verify approver identity, enforce role membership, or require multiple + * distinct approvers. If a document declares any of these fields above their + * schema defaults, warn so that embedding applications are not misled into + * treating the gate as a security control. + * + * Note: `requiredApprovers: 1` is the schema default and is suppressed here to + * avoid noise on gates that carry no quorum intent. It is still advisory at the + * runtime layer regardless of value — the warning text below makes that explicit. + * + * Emitted once per gate per store instance the first time it enters the store + * (init + updateAst's "new component" branch only — re-renders do not re-warn). + * The warning is delivered via `console.warn`, which is suppressed in some + * embedder pipelines (e.g. vitest --silent, SSR redirected to a structured + * logger). If embedders need durable advisory signalling, a structured-logger + * integration is the natural follow-up; that is intentionally out of scope here + * because `packages/runtime` does not currently take a logger dependency. + */ +function warnIfAdvisoryApprovalFields(comp: MdmaBlock['component']): void { + if (comp.type !== 'approval-gate') return; + const hasAllowedRoles = Array.isArray(comp.allowedRoles) && comp.allowedRoles.length > 0; + const hasMultiApproverRequirement = + typeof comp.requiredApprovers === 'number' && comp.requiredApprovers > 1; + const hasRequireReason = comp.requireReason === true; + if (!hasAllowedRoles && !hasMultiApproverRequirement && !hasRequireReason) return; + + const fields: string[] = []; + if (hasAllowedRoles) fields.push('allowedRoles'); + if (hasMultiApproverRequirement) fields.push('requiredApprovers'); + if (hasRequireReason) fields.push('requireReason'); + + console.warn( + `[mdma] approval-gate "${comp.id}" declares ${fields.join(', ')}, but ALL approval-gate role/quorum/reason fields are advisory at the runtime layer. The MDMA runtime does not verify approver identity, enforce role membership, track distinct approvers (regardless of requiredApprovers value, including 1), or block dispatch when a reason is omitted. The host application is responsible for authentication and authorization. See docs/reference/component-catalog.md#approval-gate.`, + ); +} + /** Extract the `id` field from partial YAML content. */ function extractIdFromYaml(yaml?: string): string | null { if (!yaml) return null; diff --git a/packages/runtime/tests/document-store.test.ts b/packages/runtime/tests/document-store.test.ts index 12e327a..8814ac8 100644 --- a/packages/runtime/tests/document-store.test.ts +++ b/packages/runtime/tests/document-store.test.ts @@ -196,3 +196,119 @@ describe('DocumentStore', () => { expect(comp?.values.deniedReason).toBe('Not ready'); }); }); + +describe('warnIfAdvisoryApprovalFields', () => { + it('warns at init when an approval-gate declares allowedRoles', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const ast = makeAst([ + { + id: 'gate-with-roles', + type: 'approval-gate', + title: 'Approve', + sensitive: false, + disabled: false, + visible: true, + allowedRoles: ['manager'], + requiredApprovers: 1, + requireReason: false, + }, + ]); + + createDocumentStore(ast); + + expect(warn).toHaveBeenCalledTimes(1); + const message = warn.mock.calls[0][0] as string; + expect(message).toContain('gate-with-roles'); + expect(message).toContain('allowedRoles'); + expect(message).toContain('advisory'); + warn.mockRestore(); + }); + + it('warns when requiredApprovers > 1 or requireReason: true', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const ast = makeAst([ + { + id: 'gate-quorum', + type: 'approval-gate', + title: 'Approve', + sensitive: false, + disabled: false, + visible: true, + requiredApprovers: 2, + requireReason: true, + }, + ]); + + createDocumentStore(ast); + + expect(warn).toHaveBeenCalledTimes(1); + const message = warn.mock.calls[0][0] as string; + expect(message).toContain('requiredApprovers'); + expect(message).toContain('requireReason'); + warn.mockRestore(); + }); + + it('does NOT warn for a bare approval-gate with no advisory fields set', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const ast = makeAst([ + { + id: 'bare-gate', + type: 'approval-gate', + title: 'Approve', + sensitive: false, + disabled: false, + visible: true, + }, + ]); + + createDocumentStore(ast); + + expect(warn).not.toHaveBeenCalled(); + warn.mockRestore(); + }); + + it('warns when an approval-gate with advisory fields is added via updateAst', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const initialAst = makeAst([ + { + id: 'form1', + type: 'form', + sensitive: false, + disabled: false, + visible: true, + fields: [], + }, + ]); + const store = createDocumentStore(initialAst); + expect(warn).not.toHaveBeenCalled(); + + const updatedAst = makeAst([ + { + id: 'form1', + type: 'form', + sensitive: false, + disabled: false, + visible: true, + fields: [], + }, + { + id: 'streamed-gate', + type: 'approval-gate', + title: 'Approve', + sensitive: false, + disabled: false, + visible: true, + allowedRoles: ['admin'], + requiredApprovers: 1, + requireReason: false, + }, + ]); + store.updateAst(updatedAst); + + expect(warn).toHaveBeenCalledTimes(1); + const message = warn.mock.calls[0][0] as string; + expect(message).toContain('streamed-gate'); + expect(message).toContain('allowedRoles'); + warn.mockRestore(); + }); +}); diff --git a/packages/spec/src/schemas/components/approval-gate.ts b/packages/spec/src/schemas/components/approval-gate.ts index dd27994..1b1c154 100644 --- a/packages/spec/src/schemas/components/approval-gate.ts +++ b/packages/spec/src/schemas/components/approval-gate.ts @@ -5,11 +5,32 @@ export const ApprovalGateComponentSchema = ComponentBaseSchema.extend({ type: z.literal('approval-gate'), title: z.string().min(1), description: z.string().optional(), - requiredApprovers: z.number().int().positive().default(1), - allowedRoles: z.array(z.string()).optional(), + requiredApprovers: z + .number() + .int() + .positive() + .default(1) + .describe( + 'Advisory only — not enforced by the MDMA runtime, regardless of value (including the ' + + 'default of 1). The host application is responsible for tracking distinct approvers. ' + + 'See docs/reference/component-catalog.md#approval-gate for the security model.', + ), + allowedRoles: z + .array(z.string()) + .optional() + .describe( + 'Advisory list of approver roles surfaced to the host UI — not enforced by the MDMA ' + + 'runtime. The host application is responsible for verifying actor identity and role membership.', + ), onApprove: z.string().optional().describe('Action ID dispatched on approval'), onDeny: z.string().optional().describe('Action ID dispatched on denial'), - requireReason: z.boolean().default(false).describe('Require reason on denial'), + requireReason: z + .boolean() + .default(false) + .describe( + 'Advisory only — surfaces a reason input on denial; the MDMA runtime does not block ' + + 'dispatch when a reason is omitted. Enforcement is the host application’s responsibility.', + ), }); export type ApprovalGateComponent = z.infer;