diff --git a/.changeset/action-param-carryover-declaration.md b/.changeset/action-param-carryover-declaration.md new file mode 100644 index 0000000000..f3a91225ee --- /dev/null +++ b/.changeset/action-param-carryover-declaration.md @@ -0,0 +1,35 @@ +--- +'@objectstack/spec': minor +'@objectstack/plugin-security': patch +--- + +feat(spec): `ActionParamSchema.carryOver` — the declared carry-over param: seeded from the row, rendered as a non-editable summary, submitted verbatim (#11753 ruling, spec half; #11992) + + + +The maintainer's 2026-08-25 ruling on #11753 (recommendation A) declares ONE +carry-over contract instead of a rendering convention: a param may state, in +metadata, that its value is carried through the action dialog rather than +collected from the user. + +- `carryOver: true` — seed from the current row (`defaultFromRow: true` is + required alongside, enforced at parse time), render as a NON-EDITABLE + summary, submit VERBATIM. Unlike `visible: false` — the measured non-answer, + which omits the param from the submission entirely — a carry-over param is + always sent. +- Aliases: `readonly` / `disabled` are refused with guidance naming + `carryOver` (a field's `readonly` means write-path strip, which is exactly + the wrong half here). +- Exemplar (`@objectstack/plugin-security`): the five `clone_permission_set` + JSON facet params (`object_permissions`, `field_permissions`, + `system_permissions`, `row_level_security`, `tab_permissions`) declare it, + so the sanctioned clone path stops offering five prefilled raw-JSON + textareas an admin could hand-mangle into a clone that grants MORE than its + base. `description` stays an ordinary editable param. The send-side contract + is unchanged (#11703 pin 6 stays green). + +The objectui renderer leg (honouring the declaration in `ActionParamDialog`) +is the downstream card tracked on #11753. diff --git a/content/docs/references/ui/action.mdx b/content/docs/references/ui/action.mdx index d78624a814..c335ad6c93 100644 --- a/content/docs/references/ui/action.mdx +++ b/content/docs/references/ui/action.mdx @@ -146,6 +146,7 @@ L2 sandboxed JS body — runs inside an isolated VM with declared capabilities | **maxSize** | `integer` | optional | Max upload size in bytes for file/image params. | | **reference** | `string` | optional | Reference target object for inline lookup/master_detail params; mirrors FieldSchema.reference. | | **defaultFromRow** | `boolean` | optional | | +| **carryOver** | `boolean` | optional | Carry-over param: seed the value from the current row (requires defaultFromRow: true), render it as a non-editable summary in the dialog, and submit it verbatim in the request body. Unlike `visible: false` (which omits the param from the submission entirely), a carry-over param is always sent. | | **visible** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Param visibility predicate (CEL); omits the param when false. | | **requiresFeature** | `Enum<'twoFactor' \| 'organization' \| 'multiOrgEnabled' \| 'degradedTenancy' \| …>` | optional | Public auth feature flag gating this param; lowered into `visible` at parse time. | @@ -239,6 +240,7 @@ L2 sandboxed JS body — runs inside an isolated VM with declared capabilities | **maxSize** | `integer` | optional | Max upload size in bytes for file/image params. | | **reference** | `string` | optional | Reference target object for inline lookup/master_detail params; mirrors FieldSchema.reference. | | **defaultFromRow** | `boolean` | optional | | +| **carryOver** | `boolean` | optional | Carry-over param: seed the value from the current row (requires defaultFromRow: true), render it as a non-editable summary in the dialog, and submit it verbatim in the request body. Unlike `visible: false` (which omits the param from the submission entirely), a carry-over param is always sent. | | **visible** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Param visibility predicate (CEL); omits the param when false. | | **requiresFeature** | `Enum<'twoFactor' \| 'organization' \| 'multiOrgEnabled' \| 'degradedTenancy' \| 'oidcProvider' \| 'sso' \| 'ssoEnforced' \| 'deviceAuthorization' \| 'admin' \| 'phoneNumber' \| 'phoneNumberOtp'>` | optional | Public auth feature flag gating this param; lowered into `visible` at parse time. | @@ -358,6 +360,7 @@ L2 sandboxed JS body — runs inside an isolated VM with declared capabilities | **maxSize** | `integer` | optional | Max upload size in bytes for file/image params. | | **reference** | `string` | optional | Reference target object for inline lookup/master_detail params; mirrors FieldSchema.reference. | | **defaultFromRow** | `boolean` | optional | | +| **carryOver** | `boolean` | optional | Carry-over param: seed the value from the current row (requires defaultFromRow: true), render it as a non-editable summary in the dialog, and submit it verbatim in the request body. Unlike `visible: false` (which omits the param from the submission entirely), a carry-over param is always sent. | | **visible** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Param visibility predicate (CEL); omits the param when false. | | **requiresFeature** | `Enum<'twoFactor' \| 'organization' \| 'multiOrgEnabled' \| 'degradedTenancy' \| …>` | optional | Public auth feature flag gating this param; lowered into `visible` at parse time. | diff --git a/packages/plugins/plugin-security/src/objects/clone-carryover.test.ts b/packages/plugins/plugin-security/src/objects/clone-carryover.test.ts new file mode 100644 index 0000000000..350efd73cc --- /dev/null +++ b/packages/plugins/plugin-security/src/objects/clone-carryover.test.ts @@ -0,0 +1,65 @@ +// #11992 — the exemplar half of the #11753 ruling (recommendation A, +// maintainer 2026-08-25): the five `clone_permission_set` facet params DECLARE +// the spec's `carryOver` key, so the clone dialog's JSON facets are copied +// verbatim, shown read-only, and never offered as prefilled textareas an admin +// could hand-mangle into a clone that grants MORE than its base. +// +// ⭐ IDENTITIES, NOT COUNTS (same discipline as the #11703 pins one file over): +// "five params declare it" holds constant while two of them swap. Every facet +// is asserted by NAME, and the deliberate non-member (`description` — prose, +// not a permission facet) is asserted NOT to carry the key, so the boundary of +// the declaration is pinned from both sides. +// +// The SEND side is deliberately not restated here — that is +// `packaged-permission-set-lock.test.ts`'s clone-payload suite (#11703 pin 6), +// which reads the params list and must stay green under this declaration +// precisely because `carryOver` changes what the dialog RENDERS, never what it +// SENDS. +import { describe, it, expect } from 'vitest'; +import { ActionParamSchema } from '@objectstack/spec/ui'; +import { SysPermissionSet } from './sys-permission-set.object.js'; + +/** The five JSON-serialized definition facets the clone carries (#11703). */ +const CARRIED_FACETS = [ + 'object_permissions', + 'field_permissions', + 'system_permissions', + 'row_level_security', + 'tab_permissions', +] as const; + +const cloneParams = (): any[] => { + const action = (SysPermissionSet.actions ?? []).find( + (a: any) => a.name === 'clone_permission_set', + ); + if (!action) throw new Error('clone_permission_set is missing from SysPermissionSet.actions'); + return (action as any).params ?? []; +}; + +describe('clone_permission_set carry-over declaration (#11992)', () => { + it.each(CARRIED_FACETS)('%s declares carryOver: true alongside its row seed', (facet) => { + const p = cloneParams().find((x) => x.field === facet); + expect(p, `param { field: '${facet}' } is missing from clone_permission_set`).toBeDefined(); + expect(p.carryOver).toBe(true); + // The co-requirement the spec enforces at parse time — asserted here too + // so a future edit that drops the seed fails THIS suite by name instead of + // only tripping a schema refusal somewhere in a stack build. + expect(p.defaultFromRow).toBe(true); + }); + + it('description stays an ordinary editable param (no carryOver)', () => { + const p = cloneParams().find((x) => x.field === 'description'); + expect(p, 'param { field: "description" } is missing').toBeDefined(); + expect(p.carryOver).toBeUndefined(); + }); + + it('every clone param parses under ActionParamSchema (the declaration is spec-legal, not local dialect)', () => { + for (const p of cloneParams()) { + const r = ActionParamSchema.safeParse(p); + expect( + r.success, + `param ${JSON.stringify(p)} refused: ${JSON.stringify((r as { error?: unknown }).error)}`, + ).toBe(true); + } + }); +}); diff --git a/packages/plugins/plugin-security/src/objects/sys-permission-set.object.ts b/packages/plugins/plugin-security/src/objects/sys-permission-set.object.ts index 4e12344254..e726f25162 100644 --- a/packages/plugins/plugin-security/src/objects/sys-permission-set.object.ts +++ b/packages/plugins/plugin-security/src/objects/sys-permission-set.object.ts @@ -140,16 +140,22 @@ export const SysPermissionSet = ObjectSchema.create({ // name, so the scope has to be right here — a bare "Unique" tells the // author the name is taken installation-wide when it is not. { name: 'name', label: 'New API Name', type: 'text', required: true, helpText: 'snake_case machine name, unique per organization' }, + // `description` is prose, not a permission facet: it stays editable + // (renaming a clone's description is legitimate), while the five JSON + // facets below are declared `carryOver` — the #11753 ruling's + // non-editable carry-over. Copied verbatim, shown read-only, never + // offered as a prefilled JSON textarea an admin could hand-mangle into + // a clone that grants MORE than its base. { field: 'description', defaultFromRow: true }, - { field: 'object_permissions', defaultFromRow: true }, - { field: 'field_permissions', defaultFromRow: true }, + { field: 'object_permissions', defaultFromRow: true, carryOver: true }, + { field: 'field_permissions', defaultFromRow: true, carryOver: true }, // [#11703] The three facets the clone silently dropped. Same // JSON-string shape as the two above: `permissionSetRowFields()` // writes all five with `JSON.stringify`, and the data door parses all // five back — the accept surface did not move, only what is SENT. - { field: 'system_permissions', defaultFromRow: true }, - { field: 'row_level_security', defaultFromRow: true }, - { field: 'tab_permissions', defaultFromRow: true }, + { field: 'system_permissions', defaultFromRow: true, carryOver: true }, + { field: 'row_level_security', defaultFromRow: true, carryOver: true }, + { field: 'tab_permissions', defaultFromRow: true, carryOver: true }, // ⛔ `admin_scope` is deliberately absent — see `description` above. ], }, diff --git a/packages/spec/authorable-surface/ui.json b/packages/spec/authorable-surface/ui.json index 6fb11733e4..3247344c83 100644 --- a/packages/spec/authorable-surface/ui.json +++ b/packages/spec/authorable-surface/ui.json @@ -70,6 +70,7 @@ "ui/ActionNavItem:type", "ui/ActionNavItem:visible", "ui/ActionParam:accept", + "ui/ActionParam:carryOver", "ui/ActionParam:defaultFromRow", "ui/ActionParam:defaultValue", "ui/ActionParam:field", diff --git a/packages/spec/src/ui/action-param-carryover.test.ts b/packages/spec/src/ui/action-param-carryover.test.ts new file mode 100644 index 0000000000..fd69409aca --- /dev/null +++ b/packages/spec/src/ui/action-param-carryover.test.ts @@ -0,0 +1,160 @@ +// #11992 — `ActionParamSchema.carryOver`, the #11753 ruling's spec half +// (maintainer 2026-08-25, recommendation A): a declared carry-over param is +// seeded from the row, rendered as a NON-EDITABLE summary, and submitted +// VERBATIM. These pins hold the ruled shape: the accept set (key + parsed +// output), the parse-time `defaultFromRow` co-requirement, the alias +// prescriptions for the words authors will actually try (`readonly` / +// `disabled`), and the describe() contract the renderer leg and the docs are +// generated from. +// +// Measured constraint restated from the parent card, because it is the reason +// the key exists at all: `visible: false` is NOT this contract — it omits the +// param from the dialog AND from the submission, which is the #11703 +// silent-drop shape. `carryOver` must keep the param in the submission. +import { describe, it, expect } from 'vitest'; +import { ActionParamSchema } from './action.zod'; + +describe('ActionParamSchema.carryOver (#11992, #11753 ruling)', () => { + describe('accept pins', () => { + it('accepts carryOver on a field-backed defaultFromRow param and carries it in the parse output', () => { + const r = ActionParamSchema.safeParse({ + field: 'row_level_security', + defaultFromRow: true, + carryOver: true, + }); + expect(r.success, JSON.stringify((r as { error?: unknown }).error)).toBe(true); + // The renderer leg reads this member off the parsed shape; nothing may + // strip or rename it on the way through (contrast `requiresFeature`, + // which IS lowered away — this key is not sugar, it is the contract). + expect((r.data as { carryOver?: boolean }).carryOver).toBe(true); + }); + + it('accepts carryOver: false as an explicit no-op', () => { + const r = ActionParamSchema.safeParse({ + field: 'description', + defaultFromRow: true, + carryOver: false, + }); + expect(r.success, JSON.stringify((r as { error?: unknown }).error)).toBe(true); + }); + + it('accepts carryOver on an inline param when the row seed is declared', () => { + const r = ActionParamSchema.safeParse({ + name: 'tab_permissions', + type: 'textarea', + defaultFromRow: true, + carryOver: true, + }); + expect(r.success, JSON.stringify((r as { error?: unknown }).error)).toBe(true); + }); + }); + + describe('co-requirement pin — carryOver without its row seed is an authoring error', () => { + it('refuses carryOver: true without defaultFromRow, on the carryOver path, naming the missing seed', () => { + const r = ActionParamSchema.safeParse({ + field: 'row_level_security', + carryOver: true, + }); + expect(r.success).toBe(false); + if (r.success) return; + const issue = r.error.issues.find((i) => i.path.join('.') === 'carryOver'); + expect(issue, JSON.stringify(r.error.issues)).toBeDefined(); + // The message must carry the repair (`defaultFromRow: true`) and the + // fixed-value alternative (`bodyExtra`) — the refusal is the docs at the + // moment of the mistake. + expect(issue!.message).toContain('defaultFromRow: true'); + expect(issue!.message).toContain('bodyExtra'); + }); + + it('refuses carryOver: true with defaultFromRow explicitly false', () => { + const r = ActionParamSchema.safeParse({ + field: 'row_level_security', + defaultFromRow: false, + carryOver: true, + }); + expect(r.success).toBe(false); + }); + + it('a carryOver: false param does NOT require the seed (no phantom check on the disabled spelling)', () => { + const r = ActionParamSchema.safeParse({ + name: 'note', + type: 'text', + carryOver: false, + }); + expect(r.success, JSON.stringify((r as { error?: unknown }).error)).toBe(true); + }); + }); + + describe('alias pins — the borrowed words point at the declared key', () => { + // The parent card's option A was literally titled "a readonly / carryOver + // flag", and `FieldSchema.readonly` / widget `disabled` are the spellings + // an author will reach for first. Both must land on the strict-unknown-key + // path with a suggestion naming `carryOver` — never parse clean (this + // schema is strict) and never dead-end without a pointer. + it.each(['readonly', 'disabled'] as const)('rejects %s with a suggestion naming carryOver', (word) => { + const r = ActionParamSchema.safeParse({ + field: 'row_level_security', + defaultFromRow: true, + [word]: true, + }); + expect(r.success).toBe(false); + if (r.success) return; + const text = JSON.stringify(r.error.issues); + expect(text).toContain(word); + expect(text).toContain('carryOver'); + }); + }); + + describe('describe pin — the three ruled semantics are stated on the key', () => { + it('the .describe() text states seed-from-row, non-editable render, and verbatim submission', () => { + // The describe string is what the generated reference docs and the + // authorable-surface baseline carry — an author (or an AI writing + // metadata in bulk) reads THIS, so all three halves of the ruled + // contract must be in it, including the contrast with `visible: false` + // (the measured non-answer). + const shape = (ActionParamSchema as unknown as { + def: { getter?: () => unknown }; + }); + // `lazySchema` wraps the pipeline; walk to the inner object's shape via + // a parse-independent probe: JSON-schema-free, so just read the + // description off a parsed-known-good source — the schema graph. + const description = findCarryOverDescription(shape); + expect(description).toBeTruthy(); + expect(description).toContain('seed the value from the current row'); + expect(description).toContain('non-editable summary'); + expect(description).toContain('submit it verbatim'); + expect(description).toContain('visible: false'); + }); + }); +}); + +/** + * Walk the (lazy, refined, transformed) schema graph down to the strict object + * and read `carryOver`'s description. Kept structural rather than importing + * zod internals: every wrapper layer exposes its inner schema on `def` + * (`innerType` / `schema` / `getter()`), and the object layer exposes `shape`. + */ +function findCarryOverDescription(node: unknown, depth = 0): string | undefined { + // `lazySchema` returns a Proxy over a FUNCTION target (structurally a + // ZodType, `typeof` says 'function'), so both object and function nodes are + // walkable — an object-only guard silently skips the schema root. + if (!node || (typeof node !== 'object' && typeof node !== 'function') || depth > 12) return undefined; + const n = node as Record; + const shape = typeof n.shape === 'object' ? n.shape : n.def?.shape; + if (shape?.carryOver) { + const co = shape.carryOver as Record; + return co.description ?? co.def?.description ?? co.meta?.()?.description; + } + const d = n.def ?? {}; + for (const next of [ + typeof d.getter === 'function' ? d.getter() : undefined, + d.innerType, + d.schema, + d.in, + n.innerType, + ]) { + const found = findCarryOverDescription(next, depth + 1); + if (found) return found; + } + return undefined; +} diff --git a/packages/spec/src/ui/action.zod.ts b/packages/spec/src/ui/action.zod.ts index 73fa164ff5..a9db2dcbbd 100644 --- a/packages/spec/src/ui/action.zod.ts +++ b/packages/spec/src/ui/action.zod.ts @@ -88,6 +88,13 @@ const ACTION_PARAM_KEY_ALIASES: Readonly> = { description: 'helpText', help: 'helpText', default: 'defaultValue', + // The words an author borrows from `FieldSchema` (`readonly`) or widget + // vocabulary (`disabled`) for "the user must not edit this". On a param the + // declared contract is `carryOver` (#11753): non-editable AND still + // submitted verbatim — which is the half `readonly`'s field semantics + // (write-path strip) would get exactly wrong here. + readonly: 'carryOver', + disabled: 'carryOver', }; /** @@ -372,6 +379,40 @@ export const ActionParamSchema = lazySchema(() => strictObject( * context. Useful for edit dialogs that pre-fill from the selected row. */ defaultFromRow: z.boolean().optional(), + /** + * Carry-over declaration (#11753 ruling, 2026-08-25): the param's value is + * carried through the dialog rather than collected from the user — seeded + * from the current row (`defaultFromRow` is required alongside), rendered as + * a NON-EDITABLE summary, and submitted VERBATIM in the request body. + * + * The knob exists because neither neighbour expresses this contract: + * + * - `visible: false` omits the param from the dialog AND from the submission + * — measured on #11753; a clone action that hid its facet params this way + * would silently stop copying them, which is exactly the #11703 defect + * shape. + * - Leaving the param editable invites the failure the ruling names: the + * clone dialog offered `member_default`'s `row_level_security` — a JSON + * array of 17+ policy objects — as a prefilled textarea on the platform's + * SANCTIONED clone path, where a hand-mangled-but-valid-JSON edit produces + * a clone granting MORE than its base, accepted without a word + * (`PermissionSetSchema` validates shape, not intent). + * + * "Not editable" is expressed by contract and enforced by the renderer + * (maintainer ruling on #11753, recommendation A): objectui's + * `ActionParamDialog` renders a declared carry-over as a read-only summary + * while keeping the seeded value in its submit state, so what is declared is + * what is sent. Requiring `defaultFromRow: true` is the declared = enforced + * half at authoring time — a carry-over with no row seed would render an + * empty locked control and submit nothing, which is an authoring error, not + * a rendering decision (ADR-0078). + */ + carryOver: z.boolean().optional().describe( + 'Carry-over param: seed the value from the current row (requires defaultFromRow: true), ' + + 'render it as a non-editable summary in the dialog, and submit it verbatim in the request ' + + 'body. Unlike `visible: false` (which omits the param from the submission entirely), a ' + + 'carry-over param is always sent.', + ), /** * Visibility predicate (CEL) — same scope as the action-level `visible` * (`current_user` / `app` / `data` / `features`). When it evaluates false the @@ -405,6 +446,22 @@ export const ActionParamSchema = lazySchema(() => strictObject( message: 'ActionParam with type "lookup"/"master_detail" requires "reference" (the target object) when declared inline — without it the param dialog degrades to a raw record-id text input. Set `reference: \'\'`, or use a field-backed param (`{ field: \'\' }`) to inherit it.', }, +).refine( + // A carry-over param must have its row seed declared. The pair is checked at + // parse time because the failure it prevents is silent at runtime: a + // `carryOver: true` param with no `defaultFromRow` would render an empty + // read-only control and submit `undefined` — the #11703 silent-drop shape, + // reintroduced through the very key added to close it. + (p) => !p.carryOver || p.defaultFromRow === true, + { + path: ['carryOver'], + message: + 'ActionParam with "carryOver" requires "defaultFromRow: true" — a carry-over param is ' + + 'seeded from the current row, rendered read-only and submitted verbatim; without the row ' + + 'seed it would render an empty locked control and submit nothing. Declare ' + + '`defaultFromRow: true`, or (for a fixed value the user should not see) use the action\'s ' + + '`bodyExtra` instead.', + }, ).superRefine((p, ctx) => { // #6970 — an authored `defaultValue` is checked against the param's OWN // declared value contract, through the SAME `valueSchemaFor` the dispatcher