From 352a1e133d1edbcd837bace056494b33ab560291 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 10:04:30 +0000 Subject: [PATCH 1/5] feat(app-shell,core,permissions): bind current_user.can(object, verb) into the predicate scope once permissions have loaded (objectui#4421) The acting subject carries the loaded /auth/me/permissions objects map under a symbol; evalFieldPredicate hands it to the engine as EvalContext.permissions. Not loaded: nothing is bound and the engine refuses can() loudly, so each surface applies its own fault policy. Claude-Session: https://claude.ai/code/session_0122Knsowci76D2rBWReCzzZ Co-authored-by: Claude --- packages/app-shell/package.json | 2 +- packages/app-shell/src/console/AppContent.tsx | 8 +- .../src/providers/ExpressionProvider.tsx | 101 ++++++++++++++++-- .../app-shell/src/views/RecordFormPage.tsx | 7 +- packages/core/package.json | 2 +- packages/core/src/evaluator/fieldRules.ts | 10 ++ packages/core/src/evaluator/index.ts | 1 + .../core/src/evaluator/subjectPermissions.ts | 99 +++++++++++++++++ .../permissions/src/MePermissionsProvider.tsx | 4 + packages/permissions/src/PermissionContext.ts | 20 ++++ .../permissions/src/PermissionProvider.tsx | 4 + pnpm-lock.yaml | 4 +- 12 files changed, 250 insertions(+), 12 deletions(-) create mode 100644 packages/core/src/evaluator/subjectPermissions.ts diff --git a/packages/app-shell/package.json b/packages/app-shell/package.json index c1a1e8f276..c7dcba3277 100644 --- a/packages/app-shell/package.json +++ b/packages/app-shell/package.json @@ -83,7 +83,7 @@ "@object-ui/providers": "workspace:*", "@object-ui/react": "workspace:*", "@object-ui/types": "workspace:*", - "@objectstack/formula": "^17.0.0", + "@objectstack/formula": "^17.5.0", "@objectstack/lint": "^17.0.0", "@objectstack/spec": "^17.5.0", "@sentry/react": "^10.70.0", diff --git a/packages/app-shell/src/console/AppContent.tsx b/packages/app-shell/src/console/AppContent.tsx index e29dc9b2d0..a00bd53220 100644 --- a/packages/app-shell/src/console/AppContent.tsx +++ b/packages/app-shell/src/console/AppContent.tsx @@ -26,6 +26,7 @@ import { ExpressionProvider, createExpressionEvaluator, isObjectFieldVisible, + useExpressionPermissions, } from '../providers/ExpressionProvider.js'; import { buildExpressionUser } from '../providers/expressionUser.js'; import { useTrackRouteAsRecent } from '../hooks/useTrackRouteAsRecent.js'; @@ -692,6 +693,10 @@ export function AppContent({ extraRoutes, extraRoutesNoApp }: AppContentProps = // object) and `features`. Its `user` was hand-rolled too, without // `positions` — so `'sales' in current_user.positions`, the gate the server // enforces on write, faulted here rather than hiding the field. + // objectui#4421 — the same `permissions` input the provider below reads, so + // `current_user.can(...)` in a field's `visibleWhen` answers here exactly as + // it does under the provider (one bag, objectui#6493). + const expressionPermissions = useExpressionPermissions(); const expressionEvaluator = useMemo( // ⛔ No `app`: objectui#8155 removed it from the predicate scope, because // neither ADR-0068 nor the engine's `SCOPE_ROOTS` declares such a root. @@ -709,8 +714,9 @@ export function AppContent({ extraRoutes, extraRoutesNoApp }: AppContentProps = () => createExpressionEvaluator({ user: buildExpressionUser(user), features, + permissions: expressionPermissions, }), - [user, features], + [user, features, expressionPermissions], ); // objectui#5619 — `isWorkspaceAdminResolved` belongs in this readiness gate diff --git a/packages/app-shell/src/providers/ExpressionProvider.tsx b/packages/app-shell/src/providers/ExpressionProvider.tsx index c347bcffaf..ff3deb40ae 100644 --- a/packages/app-shell/src/providers/ExpressionProvider.tsx +++ b/packages/app-shell/src/providers/ExpressionProvider.tsx @@ -15,8 +15,10 @@ */ import React, { createContext, useContext, useMemo } from 'react'; -import { ExpressionEvaluator } from '@object-ui/core'; +import { ExpressionEvaluator, bindSubjectPermissions } from '@object-ui/core'; import { PredicateScopeProvider, reportUnresolvableVisibilityPredicate } from '@object-ui/react'; +import { usePermissions } from '@object-ui/permissions'; +import { toEvalPermissions, type EvalPermissions } from '@objectstack/formula'; export interface ExpressionContextValue { /** Current authenticated user */ @@ -59,6 +61,16 @@ export interface ExpressionScopeInput { * and still publishes it on the React context value. */ features?: Record; + /** + * The caller's effective object permissions — the data + * `current_user.can(object, verb)` is answered from (objectui#4421) — or + * `undefined` while there is no LOADED payload to answer from. Take it from + * {@link useExpressionPermissions}, which is what decides "loaded"; ⛔ never + * pass `{}` for "not loaded yet": an empty map is a real answer ("holds + * nothing"), and the engine refuses loudly precisely so that a missing + * payload cannot pass for one. + */ + permissions?: EvalPermissions; } /** @@ -169,8 +181,81 @@ export interface ExpressionScopeInput { export function buildExpressionScope({ user = {}, features = {}, + permissions, }: ExpressionScopeInput = {}): Record { - return { current_user: user, user, ctx: { user }, os: { user }, features }; + // ONE subject object under all four spellings, and it is the one carrying the + // permissions: the engine answers `can` only for a receiver IDENTICAL to the + // bound `current_user`, so a second copy under any alias would be refused. + const subject = bindSubjectPermissions(user, permissions); + return { current_user: subject, user: subject, ctx: { user: subject }, os: { user: subject }, features }; +} + +/** + * One adapted map per payload object, kept outside React (AGENTS.md #10): the + * provider publishes the SAME `objects` object for the same response, so this + * is keyed on the payload, never on a memoised identity. `null` records a + * payload `toEvalPermissions` refused, so the refusal is reported once. + */ +const ADAPTED_PERMISSIONS = new WeakMap(); + +function adaptEffectiveObjects(objects: object): EvalPermissions | undefined { + let adapted = ADAPTED_PERMISSIONS.get(objects); + if (adapted === undefined) { + try { + adapted = toEvalPermissions(objects); + } catch (err) { + // Refused, not repaired: the formula package's adapter has no lenient + // arm, and neither does this one. The binding stays UNBOUND, so every + // `current_user.can(...)` faults (and each surface applies its own fault + // policy) instead of answering from a map that is not the contract. + console.error( + '[object-ui] current_user.can(object, verb) is not bound: the permissions ' + + `payload was refused — ${err instanceof Error ? err.message : String(err)}`, + ); + adapted = null; + } + ADAPTED_PERMISSIONS.set(objects, adapted); + } + return adapted ?? undefined; +} + +/** + * The effective object permissions the predicate scope binds for + * `current_user.can(object, verb)` (objectui#4421), or `undefined` when there + * is no loaded payload to answer from. + * + * ## Rider 1: absent while not loaded, never "empty" + * + * The maintainer's ruling makes a permission-shaped binding fail-CLOSED while + * the permissions payload has not loaded. The map is therefore handed on only + * when `usePermissions().isLoaded` is true AND the provider holds an + * `/auth/me/permissions` answer; otherwise nothing is bound and the engine + * REFUSES `can` (`ok: false`, a fault naming the missing input). What happens + * next is the evaluating surface's own FAULT policy, unchanged by this binding: + * a leg that evaluates fail-closed (`throwOnError: true` on `useCondition`, + * `fallback: false` on `evalRowPredicate`) hides the action, a fail-soft leg + * does not. Which legs are which is deliberately not listed here (AGENTS.md + * #9); `providers/__tests__/currentUserCan-4421.render.test.tsx` re-measures + * the three states on the surfaces it pins. ⛔ No `{}` stand-in: it would + * answer "holds nothing" — a quiet `false` indistinguishable from a real + * denial, which the engine's contract forbids a caller to fabricate. + * + * `isLoaded` is the gate rather than "some map is present" because a + * refetching `MePermissionsProvider` still holds its previous map while + * `isLoaded` is false; the ruling's "has not loaded" is read as the provider's + * own flag. + * + * ## Why the global no-provider default is not involved + * + * `usePermissions()` with no provider answers `can: () => true` (fail-open, the + * built-in affordances' standalone-embed contract). This binding never calls + * it: it reads `effectiveObjects`, which the no-provider answer does not carry, + * so under no provider `current_user.can(...)` faults — it does not inherit + * that `true`. + */ +export function useExpressionPermissions(): EvalPermissions | undefined { + const { isLoaded, effectiveObjects } = usePermissions(); + return isLoaded && effectiveObjects ? adaptEffectiveObjects(effectiveObjects) : undefined; } /** @@ -194,13 +279,17 @@ interface ExpressionProviderProps { } export function ExpressionProvider({ children, user = {}, app = {}, data = {}, features = {} }: ExpressionProviderProps) { + // objectui#4421 — read HERE, once, so every mount of this provider (and every + // surface under it) binds `current_user.can(...)` from the same payload with + // no prop to thread and no per-surface copy of the hand-off. + const permissions = useExpressionPermissions(); const value = useMemo(() => { - const evaluator = createExpressionEvaluator({ user, features }); + const evaluator = createExpressionEvaluator({ user, features, permissions }); // `app` and `data` are still published on the context value — `DashboardView` // reads `app` as a plain value. Neither is handed to the evaluator: // objectui#8155 (`app`), objectui#8166 (`data`). return { user, app, data, features, evaluator }; - }, [user, app, data, features]); + }, [user, app, data, features, permissions]); // Also feed the predicate scope used by useCondition/useExpression in // @object-ui/react so action visibility predicates (e.g. on toolbar @@ -208,8 +297,8 @@ export function ExpressionProvider({ children, user = {}, app = {}, data = {}, f // The SAME bag the evaluator above got — one builder, so the imperative and // the hook-driven halves of this provider cannot drift apart either. const scope = useMemo( - () => buildExpressionScope({ user, features }), - [user, features], + () => buildExpressionScope({ user, features, permissions }), + [user, features, permissions], ); return ( diff --git a/packages/app-shell/src/views/RecordFormPage.tsx b/packages/app-shell/src/views/RecordFormPage.tsx index b49b2b52d2..09acdddc34 100644 --- a/packages/app-shell/src/views/RecordFormPage.tsx +++ b/packages/app-shell/src/views/RecordFormPage.tsx @@ -41,6 +41,7 @@ import { ExpressionProvider, createExpressionEvaluator, isObjectFieldVisible, + useExpressionPermissions, } from '../providers/ExpressionProvider.js'; import { buildExpressionUser } from '../providers/expressionUser.js'; import { SkeletonDetail } from '../skeletons/index.js'; @@ -200,6 +201,9 @@ export function RecordFormPage({ mode }: RecordFormPageProps) { // The private bag this replaced bound `user` alone, so an authored // `current_user` / `ctx.user` / `os.user` / `features` gate on a field // faulted here and failed OPEN while resolving normally on a nav item. + // objectui#4421 — the `permissions` input the `ExpressionProvider` below + // reads for itself, so `current_user.can(...)` answers the same on both. + const expressionPermissions = useExpressionPermissions(); const expressionEvaluator = useMemo( () => // ⛔ No `app`: objectui#8155 removed it from the predicate scope, because @@ -213,8 +217,9 @@ export function RecordFormPage({ mode }: RecordFormPageProps) { // pass it through unconditionally. user: expressionUser, features, + permissions: expressionPermissions, }), - [expressionUser, features], + [expressionUser, features, expressionPermissions], ); // Resolve the field list using the same visibility-aware logic as the diff --git a/packages/core/package.json b/packages/core/package.json index 32b5d8f92c..c45ee4ca93 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -32,7 +32,7 @@ }, "dependencies": { "@object-ui/types": "workspace:*", - "@objectstack/formula": "^17.0.0", + "@objectstack/formula": "^17.5.0", "@objectstack/spec": "^17.5.0" }, "devDependencies": { diff --git a/packages/core/src/evaluator/fieldRules.ts b/packages/core/src/evaluator/fieldRules.ts index 83605f713b..4028931a8f 100644 --- a/packages/core/src/evaluator/fieldRules.ts +++ b/packages/core/src/evaluator/fieldRules.ts @@ -75,6 +75,7 @@ * site as every other fault; the VERDICT is unchanged for every input. */ import { ExpressionEngine } from '@objectstack/formula'; +import { subjectPermissionsOf } from './subjectPermissions.js'; import type { Expression } from '@objectstack/spec'; import { isBlankPredicateText } from './declaredPredicate.js'; @@ -257,10 +258,19 @@ export function evalFieldPredicate( reason = BLANK_PREDICATE_REASON; } else { try { + // objectui#4421 — the acting subject's effective object permissions, the + // one input `current_user.can(object, verb)` is answered from. They ride + // on the subject the scope already carries (`subjectPermissions.ts`), + // never under a scope key, because every key of `scope` is a CEL root + // and the engine keeps this map out of the variable namespace. A subject + // with none passes none, and `can` then refuses loudly: that fault goes + // through the caller's `fallback` like any other. + const permissions = subjectPermissionsOf(scope?.current_user); const res = ExpressionEngine.evaluate(expr, { record, previous, ...(scope ? { extra: scope } : {}), + ...(permissions !== undefined ? { permissions } : {}), }); // Parse error, type error, unbound identifier, engine fault … — every // not-ok verdict resolves to the fallback, but never silently (objectstack-ai/objectstack#5149). diff --git a/packages/core/src/evaluator/index.ts b/packages/core/src/evaluator/index.ts index 6308ba33a5..de2b2b39ee 100644 --- a/packages/core/src/evaluator/index.ts +++ b/packages/core/src/evaluator/index.ts @@ -11,6 +11,7 @@ export * from './ExpressionEvaluator.js'; export * from './predicateInput.js'; export * from './declaredPredicate.js'; export * from './fieldRules.js'; +export * from './subjectPermissions.js'; export * from './rowPredicateCanon.js'; export * from './listConditional.js'; export * from './optionRules.js'; diff --git a/packages/core/src/evaluator/subjectPermissions.ts b/packages/core/src/evaluator/subjectPermissions.ts new file mode 100644 index 0000000000..4d192ab611 --- /dev/null +++ b/packages/core/src/evaluator/subjectPermissions.ts @@ -0,0 +1,99 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * The acting subject's effective object permissions, carried to the one place + * this repo hands a predicate to the CEL engine (objectui#4421). + * + * ## What it is for + * + * `@objectstack/formula` registers `current_user.can(object, verb)` as a + * receiver method on the acting subject, and answers it from + * `EvalContext.permissions` — the verbatim `objects` map of the + * `/auth/me/permissions` response. That map is a separate field of the engine + * context, and the engine is explicit that it is ⛔ NOT a CEL variable: an + * authored predicate must not be able to read `permissions.crm_lead.allowEdit` + * and step around the closed verb table. + * + * This repo's evaluation seam (`evalFieldPredicate`) is not handed an engine + * context by its callers, though. Every action surface — the row menu, the + * record header, the `action:*` renderers, `DeclaredActionsBar` — hands it a + * variable BAG (the host's predicate scope, spread, merged, pushed through an + * `ExpressionContext`), and the bag's string keys all become CEL roots. So the + * permissions cannot ride on the bag under a name: any name would be a root. + * + * They ride on the SUBJECT instead, under a symbol: + * + * - The engine models the map as the subject's own answer sheet — it binds + * `permissions` only alongside a user, and `can` refuses any receiver that + * is not that user (`receiver !== subject`). The subject is the one object + * every bag already carries by reference under `current_user` (and the + * `user` / `ctx.user` / `os.user` aliases of it), through every spread and + * every `ExpressionContext` scope, so no surface needs its own copy of the + * hand-off. + * - A symbol is the one kind of key no predicate dialect can name: CEL + * identifiers and member names are strings, and so are the legacy + * evaluator's. `current_user.permissions` still faults as an unknown key. + * + * ## Absent is not empty + * + * A subject that carries NO map — the payload has not loaded, no permission + * provider is mounted, or the provider has no `/auth/me/permissions` answer at + * all — hands the engine no `permissions`, and `can` then refuses loudly + * (`ok: false`, `kind: 'runtime'`). That refusal is a FAULT, so each surface + * applies its own fault policy to it; it is never turned into a quiet `false` + * here, because the engine's contract is that an empty map means "holds + * nothing" and a caller that has no data must not claim it has that answer. + */ + +import type { EvalPermissions } from '@objectstack/formula'; + +/** + * The key the subject carries its effective object permissions under. + * + * `Symbol.for` rather than `Symbol()` so two copies of this module in one page + * (a host that bundles `@object-ui/core` twice) still agree on the key; the + * namespace keeps it from colliding with anyone else's registry entry. + */ +export const SUBJECT_PERMISSIONS: unique symbol = Symbol.for( + '@object-ui/core:subject-permissions', +) as never; + +type PermissionBearing = { [SUBJECT_PERMISSIONS]?: EvalPermissions }; + +/** + * The subject to publish as `current_user`, carrying `permissions` when there + * are any. + * + * Returns a shallow copy so the caller's own user object is never mutated; the + * copy is what the whole predicate scope must use for every alias of the + * subject, because the engine compares the `can` receiver by IDENTITY. With no + * `permissions` the result carries none — a subject built from one that did is + * stripped rather than left answering from a stale map. + */ +export function bindSubjectPermissions( + subject: T, + permissions: EvalPermissions | undefined, +): T { + if (permissions === undefined) { + if (!Object.prototype.hasOwnProperty.call(subject, SUBJECT_PERMISSIONS)) return subject; + const stripped = { ...subject } as T & PermissionBearing; + delete stripped[SUBJECT_PERMISSIONS]; + return stripped; + } + return { ...subject, [SUBJECT_PERMISSIONS]: permissions }; +} + +/** + * The effective object permissions a subject carries, or `undefined` when it + * carries none (see "Absent is not empty" above). + */ +export function subjectPermissionsOf(subject: unknown): EvalPermissions | undefined { + if (subject === null || typeof subject !== 'object') return undefined; + return (subject as PermissionBearing)[SUBJECT_PERMISSIONS]; +} diff --git a/packages/permissions/src/MePermissionsProvider.tsx b/packages/permissions/src/MePermissionsProvider.tsx index 9e878bd51d..b9d4ceab10 100644 --- a/packages/permissions/src/MePermissionsProvider.tsx +++ b/packages/permissions/src/MePermissionsProvider.tsx @@ -415,6 +415,10 @@ export function MePermissionsProvider({ const held = new Set(perms); return required.every((p) => held.has(p)); }, + // [objectui#4421] The response's `objects` map, verbatim — the data + // `current_user.can(object, verb)` is answered from. Keyed on `dataKey` + // like every member here, so it is the SAME object for the same payload. + effectiveObjects: data?.objects, isLoaded, })); diff --git a/packages/permissions/src/PermissionContext.ts b/packages/permissions/src/PermissionContext.ts index d83bb91240..c3b066eaf5 100644 --- a/packages/permissions/src/PermissionContext.ts +++ b/packages/permissions/src/PermissionContext.ts @@ -69,6 +69,26 @@ export interface PermissionContextValue { * enforceable answer and must not be read as "unknown". */ hasCapabilities: (required: string[]) => boolean; + /** + * [objectui#4421] The `objects` map of the `/auth/me/permissions` response + * this provider holds — object name → the server-resolved effective object + * permission — handed on VERBATIM, or `undefined` when the provider holds no + * such response: `MePermissionsProvider` before its first answer, the + * role-based `PermissionProvider` (it resolves roles from config and has no + * server payload), or no provider at all. + * + * It is DATA for the predicate binding `current_user.can(object, verb)`, + * which `@objectstack/formula` answers from exactly this map, and nothing + * else. It is not a verdict: a consumer asking "may the caller do X" calls + * `check` / `can`. `undefined` and `{}` are different answers — "no payload" + * versus "a payload that grants nothing" — and must not be collapsed into + * each other (the same absent-vs-empty rule `systemPermissions` follows). + * + * Pair it with `isLoaded`: while `isLoaded` is false a refetching provider + * can still hold its previous map here, and the predicate binding does not + * answer from it (see app-shell's `useExpressionPermissions`). + */ + effectiveObjects?: Readonly>>>; /** Whether permissions are loaded */ isLoaded: boolean; } diff --git a/packages/permissions/src/PermissionProvider.tsx b/packages/permissions/src/PermissionProvider.tsx index e05855d2c2..9bdddca471 100644 --- a/packages/permissions/src/PermissionProvider.tsx +++ b/packages/permissions/src/PermissionProvider.tsx @@ -182,6 +182,10 @@ export function PermissionProvider({ // `ALL_CAPABILITIES` above for the full reasoning. systemPermissions: undefined, hasCapabilities: ALL_CAPABILITIES, + // [objectui#4421] No `/auth/me/permissions` payload: this provider resolves + // roles from config, so `current_user.can(object, verb)` has no map to be + // answered from here and refuses loudly rather than guessing. + effectiveObjects: undefined, isLoaded: true, })); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a92e1dcd2b..c5a5b03755 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -774,7 +774,7 @@ importers: specifier: workspace:* version: link:../types '@objectstack/formula': - specifier: ^17.0.0 + specifier: ^17.5.0 version: 17.5.0(ai@7.0.65(zod@4.6.5)) '@objectstack/lint': specifier: ^17.0.0 @@ -1190,7 +1190,7 @@ importers: specifier: workspace:* version: link:../types '@objectstack/formula': - specifier: ^17.0.0 + specifier: ^17.5.0 version: 17.5.0(ai@7.0.65(zod@4.6.5)) '@objectstack/spec': specifier: ^17.5.0 From e1c001f97fdc57d08d555d5f7954581c49e6609f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 10:12:59 +0000 Subject: [PATCH 2/5] test,docs(app-shell,core,permissions): pin current_user.can three states per surface family and document it as client-only UI gating (objectui#4421) Claude-Session: https://claude.ai/code/session_0122Knsowci76D2rBWReCzzZ Co-authored-by: Claude --- .changeset/4421-current-user-can-binding.md | 34 +++ content/docs/layout/page-header.mdx | 38 +++ .../currentUserCan-4421.render.test.tsx | 227 ++++++++++++++++++ .../__tests__/subjectPermissions-4421.test.ts | 189 +++++++++++++++ .../__tests__/effectiveObjects-4421.test.tsx | 87 +++++++ 5 files changed, 575 insertions(+) create mode 100644 .changeset/4421-current-user-can-binding.md create mode 100644 packages/app-shell/src/providers/__tests__/currentUserCan-4421.render.test.tsx create mode 100644 packages/core/src/evaluator/__tests__/subjectPermissions-4421.test.ts create mode 100644 packages/permissions/src/__tests__/effectiveObjects-4421.test.tsx diff --git a/.changeset/4421-current-user-can-binding.md b/.changeset/4421-current-user-can-binding.md new file mode 100644 index 0000000000..2391615474 --- /dev/null +++ b/.changeset/4421-current-user-can-binding.md @@ -0,0 +1,34 @@ +--- +'@object-ui/core': minor +'@object-ui/permissions': minor +'@object-ui/app-shell': minor +--- + +feat: an action's `visible` / `disabled` predicate can ask `current_user.can(object, verb)` — the caller's object permissions, from the payload the built-in Edit / Delete buttons are already gated by + +A custom action that replaces a built-in CRUD button — a logical delete that archives +instead of deleting, say — can now carry the same gate the built-in button had: + +```yaml +visible: current_user.can('account', 'delete') +``` + +It answers from the signed-in user's `/auth/me/permissions` payload once that payload +has loaded, on the record header and the row menu alike. Until then it gives no answer: +the predicate faults, and a surface that evaluates `visible` fail-closed does not render +the action. The `user` / `ctx.user` / `os.user` aliases are the same call. + +This is client-side UI gating only. It decides whether the button is shown; the server +still enforces object permissions on the request the action sends and answers 403 when +they are not held. + +- `@object-ui/permissions`: the permission context carries `effectiveObjects`, the + response's `objects` map verbatim (`undefined` when the provider holds no such + response — the role-based `PermissionProvider`, or no provider at all). +- `@object-ui/core`: `evalFieldPredicate` hands the acting subject's permissions to the + CEL engine as `EvalContext.permissions`; `bindSubjectPermissions` / + `subjectPermissionsOf` are the carrier. `@objectstack/formula` is now required at + `^17.5.0`, the first release that answers `can`. +- `@object-ui/app-shell`: `ExpressionProvider` binds the map into the predicate scope it + publishes only while `usePermissions().isLoaded` is true, and the record-form field + evaluators take the same input, so one predicate answers the same everywhere. diff --git a/content/docs/layout/page-header.mdx b/content/docs/layout/page-header.mdx index 1e3fd0db8e..6fd2d3f45a 100644 --- a/content/docs/layout/page-header.mdx +++ b/content/docs/layout/page-header.mdx @@ -133,6 +133,44 @@ The identity side of the comparison is `os.user` — the spec's canonical CEL id scope, and the same one the server binds — with `current_user`, `user` and `ctx.user` available as aliases of it. +## Gating an action on the caller's object permissions + +`current_user.can(object, verb)` answers "may the signed-in user do *verb* on *object*" +from the caller's effective object permissions — the `/auth/me/permissions` payload the +console already holds, the one its built-in Edit and Delete buttons are gated by. It is +the gate to write when a custom action replaces a built-in one, for example a logical +delete that archives the record instead of removing it: + +```yaml +actions: + - name: account_void + label: Void + locations: [record_header, list_item] + visible: current_user.can('account', 'delete') +``` + +The row menu (`list_item`) evaluates the same predicate against the same scope, so one +declaration gates both places. + +- `object` is the object's API name. `verb` must come from the closed verb table + `OBJECT_PERMISSION_VERBS` in `@objectstack/spec/security`; a verb outside it is a fault, + not a `false`. +- Call it on the signed-in user. `user.can(…)`, `ctx.user.can(…)` and `os.user.can(…)` + are the same call; a bare `can(…)`, or `record.can(…)`, is a fault. +- Until the permissions payload has loaded there is no answer: the call faults, and a + header action whose `visible` faults is not rendered (see above). Gate on `visible`, + not `disabled` — a `disabled` predicate that faults leaves the action enabled. +- `requiredPermissions` is a different gate: it names system capabilities, which the + platform action route enforces as well as the UI. `current_user.can` asks about + object permissions and does nothing on the server. + +**Client-side UI gating only.** This first phase is client-side: `current_user.can` decides +whether the button is shown, nothing more. It is not an authorization boundary — the +server still enforces the caller's object permissions on the request the action sends, +and answers 403 when they are not held. Server-side evaluation of the same call (formula +fields, validation rules, row-level security) is tracked separately in objectstack#18783; +until it lands, do not rely on `can` in an expression the server evaluates. + ## Layout The renderer picks one of two layouts when it renders: diff --git a/packages/app-shell/src/providers/__tests__/currentUserCan-4421.render.test.tsx b/packages/app-shell/src/providers/__tests__/currentUserCan-4421.render.test.tsx new file mode 100644 index 0000000000..385b3be90d --- /dev/null +++ b/packages/app-shell/src/providers/__tests__/currentUserCan-4421.render.test.tsx @@ -0,0 +1,227 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#4421 — a custom action gated on `current_user.can(object, verb)`, + * the card's own shape: an app turns the built-in Delete off and ships a + * logical delete (archive / void) in its place, and wants it gated by the SAME + * verdict the Delete it replaced was gated by. + * + * Everything on the path is real: `MePermissionsProvider` holding a + * `/auth/me/permissions` payload, `ExpressionProvider` publishing the predicate + * scope, and three action surfaces, one per evaluation family: + * + * - the row menu (`RowActionMenu`, `@object-ui/plugin-grid`) — the + * `evalRowPredicate` family, `fallback: false`; + * - the record header (`page:header`, `@object-ui/components`) — the same + * family, reached through its own merged scope (`headerPredicateScope`); + * - `action:button` (`@object-ui/components`) — the `useCondition` family, + * `throwOnError: true`. + * + * Each surface is driven through the three states on ONE verb (`delete`): + * + * 1. NOT LOADED — no permission provider answers, which is the state a + * mount outside `MePermissionsProvider` is in permanently (the console's + * `/forms/:name` route, a standalone embed). Rider 1 of the ruling: the + * action is HIDDEN, and the console says why. + * 2. LOADED, GRANTED — shown. + * 3. LOADED, DENIED — hidden, and SILENTLY: a denial is a verdict, not a + * fault. That is what keeps state 1 from collapsing into state 3 — the + * same hidden button, told apart by whether the engine reported a missing + * payload. + * + * State 1 is also checked against the built-in affordances' own answer in the + * same tree: `usePermissions().can` with no provider is `true` (the embed + * contract `rowCrudAffordances.ts` documents), and the binding does NOT + * inherit it. + * + * ## Reverse verification (direction predicted before running) + * + * Drop the `permissions` hand-off in `evalFieldPredicate` (`fieldRules.ts`): + * every GRANTED arm goes red (the engine refuses `can` with no data, so the + * fail-closed surfaces hide the action for a grant holder too); the NOT-LOADED + * and DENIED arms stay green — they hide either way, which is why the denied + * arm also asserts silence and the not-loaded arm asserts the named reason. + */ + +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, cleanup } from '@testing-library/react'; +import { ComponentRegistry } from '@object-ui/core'; +import { ActionProvider, RecordContextProvider } from '@object-ui/react'; +import { MePermissionsProvider, usePermissions, type MePermissionsResponse } from '@object-ui/permissions'; +import { ExpressionProvider } from '../ExpressionProvider'; +// Module-scope side-effect imports: the two registry renderers must be +// registered before the first render (AGENTS.md §测试纪律 — import phase, not a +// hook). Deep paths into `src`, the same module instances the aliases resolve. +import '../../../../components/src/renderers/action/action-button'; +import '../../../../components/src/renderers/layout/containers'; +import { RowActionMenu } from '../../../../plugin-grid/src/components/RowActionMenu'; + +const h = React.createElement; + +/** The logical delete the app ships in place of the built-in Delete. */ +const CAN_DELETE = { dialect: 'cel', source: "current_user.can('account', 'delete')" }; +const LOGICAL_DELETE = { + name: 'account_logical_delete', + label: 'Void account', + visible: CAN_DELETE, +}; +/** An ungated companion, so "not rendered" can never mean "nothing rendered". */ +const COMPANION = { name: 'print', label: 'Print' }; + +const USER = { id: 'u1', name: 'Ada', email: 'ada@example.com', role: 'user', positions: ['everyone'] }; +const RECORD = { id: 'acc_1', name: 'Northwind', status: 'open' }; + +type State = 'not-loaded' | 'granted' | 'denied'; + +function payload(allowDelete: boolean): MePermissionsResponse { + return { + authenticated: true, + userId: 'u1', + tenantId: null, + roles: [], + permissionSets: ['account_clerk'], + // `allowEdit` is granted in BOTH loaded states, so a verb mix-up (reading + // edit for delete) would show the action in the denied state. + objects: { account: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete } }, + fields: {}, + }; +} + +/** What the built-in affordances would answer for the same verb in the same tree. */ +function BuiltinVerdict() { + const { can, isLoaded } = usePermissions(); + return h('output', { 'data-testid': 'builtin-verdict' }, `${String(isLoaded)}:${String(can('account', 'delete'))}`); +} + +function withState(state: State, surface: React.ReactNode) { + const tree = h( + ExpressionProvider, + { user: USER }, + h(ActionProvider, null, surface, h(BuiltinVerdict)), + ); + if (state === 'not-loaded') return tree; + return h(MePermissionsProvider, { initialPermissions: payload(state === 'granted') }, tree); +} + +function registered(type: string): React.ComponentType { + const R = ComponentRegistry.get(type); + if (!R) throw new Error(`${type} is not registered`); + return R as React.ComponentType; +} + +/** + * What each family's console line says about the NOT-LOADED fault. The + * `evalRowPredicate` family forwards the engine's own reason; the throwing + * `useCondition` leg reports only that the predicate threw (its `throwOnError` + * probe runs the engine with `warn: false` and throws a generic message), so + * there the line is matched on the predicate it names. + */ +const ENGINE_REASON = /carries no permission data/; +const THREW = /was hidden\/disabled: its predicate threw — CEL predicate failed to evaluate: current_user\.can\('account', 'delete'\)/; + +const SURFACES: Record React.ReactNode; + find: (name: string) => HTMLElement | null; + fault: RegExp; +}> = { + 'row menu (RowActionMenu, evalRowPredicate)': { + mount: () => + h(RowActionMenu, { + row: RECORD, + onActionDef: () => {}, + // `variant: 'primary'` renders the defs as always-mounted inline + // buttons, so the assertion does not depend on opening the menu. + rowActionDefs: [ + { ...LOGICAL_DELETE, variant: 'primary' }, + { ...COMPANION, variant: 'primary' }, + ] as never, + maxInlineActions: 2, + }), + find: (name) => screen.queryByTestId(`row-action-inline-${name}`), + fault: ENGINE_REASON, + }, + 'record header (page:header, evalRowPredicate)': { + mount: () => + h( + RecordContextProvider, + { objectName: 'account', recordId: RECORD.id, data: RECORD, objectSchema: { name: 'account', fields: {} } } as never, + h(registered('page:header'), { + schema: { + type: 'page:header', + title: 'Northwind', + maxVisible: 10, + actions: [ + { ...LOGICAL_DELETE, type: 'api', locations: ['record_header'] }, + { ...COMPANION, type: 'api', locations: ['record_header'] }, + ], + }, + }), + ), + find: (name) => + screen.queryByText(name === LOGICAL_DELETE.name ? LOGICAL_DELETE.label : COMPANION.label), + fault: ENGINE_REASON, + }, + 'action:button (useCondition, throwOnError)': { + mount: () => + h( + React.Fragment, + null, + h(registered('action:button'), { + schema: { type: 'action:button', actionType: 'script', ...LOGICAL_DELETE }, + }), + h(registered('action:button'), { + schema: { type: 'action:button', actionType: 'script', ...COMPANION }, + }), + ), + find: (name) => + screen.queryByText(name === LOGICAL_DELETE.name ? LOGICAL_DELETE.label : COMPANION.label), + fault: THREW, + }, +}; + +function warnings(spy: { mock: { calls: unknown[][] } }): string { + return spy.mock.calls.map((c) => c.map(String).join(' ')).join('\n'); +} + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +describe.each(Object.entries(SURFACES))( + 'current_user.can(object, verb) on the %s (objectui#4421)', + (_label, surface) => { + it('NOT LOADED: hidden, reported as a missing payload — the built-in no-provider `true` is not inherited', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + render(withState('not-loaded', surface.mount())); + expect(surface.find(COMPANION.name)).toBeInTheDocument(); + expect(surface.find(LOGICAL_DELETE.name)).not.toBeInTheDocument(); + expect(warnings(warn)).toMatch(surface.fault); + // Same tree, same verb: the built-in answer is the fail-open embed default. + expect(screen.getByTestId('builtin-verdict')).toHaveTextContent('false:true'); + }); + + it('LOADED, GRANTED: shown', () => { + render(withState('granted', surface.mount())); + expect(surface.find(LOGICAL_DELETE.name)).toBeInTheDocument(); + expect(screen.getByTestId('builtin-verdict')).toHaveTextContent('true:true'); + }); + + it('LOADED, DENIED: hidden, with nothing reported — a verdict, not a fault', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + render(withState('denied', surface.mount())); + expect(surface.find(COMPANION.name)).toBeInTheDocument(); + expect(surface.find(LOGICAL_DELETE.name)).not.toBeInTheDocument(); + expect(warnings(warn)).not.toMatch(/permission data|can\(|predicate threw/); + expect(screen.getByTestId('builtin-verdict')).toHaveTextContent('true:false'); + }); + }, +); diff --git a/packages/core/src/evaluator/__tests__/subjectPermissions-4421.test.ts b/packages/core/src/evaluator/__tests__/subjectPermissions-4421.test.ts new file mode 100644 index 0000000000..e3d517c09b --- /dev/null +++ b/packages/core/src/evaluator/__tests__/subjectPermissions-4421.test.ts @@ -0,0 +1,189 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * `current_user.can(object, verb)` reaches the engine through the ONE seam + * every action surface evaluates through (objectui#4421). + * + * The engine (`@objectstack/formula` >= 17.5.0) registers `can` as a receiver + * method and answers it from `EvalContext.permissions`. This file pins the + * hand-off `evalFieldPredicate` makes — the subject the scope already carries + * brings its permissions with it — across the three entries the surfaces use: + * + * - `ExpressionEvaluator.evaluateCondition` with `throwOnError` — the + * fail-closed `useCondition` leg (`action:button` / `action:menu` / + * `action:bar` / `DeclaredActionsBar` `visible`), and `ActionEngine`; + * - the same entry WITHOUT `throwOnError` — the fail-soft `useCondition` + * legs (every `disabled` / `enabled`, and the renderers that evaluate + * `visible` fail-soft); + * - `evalRowPredicate` with `fallback: false` — the row menu, the record + * header, the selection bar, the data-table rows. + * + * Each is driven through the SAME three states on the SAME verb (`delete`): + * not loaded (a subject carrying no map), loaded-and-granted, and + * loaded-and-denied. State 1 cannot collapse into state 3 here because the two + * are told apart by the engine itself — a denial is a clean `false`, the + * missing payload is a FAULT — and the tests assert which one each is. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { ExpressionEvaluator } from '../ExpressionEvaluator'; +import { evalRowPredicate } from '../listConditional'; +import { evalFieldPredicate } from '../fieldRules'; +import { + SUBJECT_PERMISSIONS, + bindSubjectPermissions, + subjectPermissionsOf, +} from '../subjectPermissions'; + +const GRANTED = { account: { allowRead: true, allowEdit: true, allowDelete: true } }; +const DENIED = { account: { allowRead: true, allowEdit: true, allowDelete: false } }; + +const USER = { id: 'u1', name: 'Ada', positions: ['everyone'] }; + +/** The predicate scope `buildExpressionScope` publishes: ONE subject, four spellings. */ +function scopeFor(permissions?: Record>) { + const subject = bindSubjectPermissions(USER, permissions as never); + return { current_user: subject, user: subject, ctx: { user: subject }, os: { user: subject }, features: {} }; +} + +const cel = (source: string) => ({ dialect: 'cel', source }); +const CAN_DELETE = "current_user.can('account', 'delete')"; + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('bindSubjectPermissions / subjectPermissionsOf (objectui#4421)', () => { + it('returns a copy carrying the map, and never mutates the caller\'s user', () => { + const bound = bindSubjectPermissions(USER, GRANTED as never); + expect(bound).not.toBe(USER); + expect(subjectPermissionsOf(bound)).toBe(GRANTED); + expect(subjectPermissionsOf(USER)).toBeUndefined(); + expect(bound).toMatchObject(USER); + }); + + it('with no map, hands back the subject untouched — or strips a stale one', () => { + expect(bindSubjectPermissions(USER, undefined)).toBe(USER); + const stale = bindSubjectPermissions(USER, GRANTED as never); + const cleared = bindSubjectPermissions(stale, undefined); + expect(subjectPermissionsOf(cleared)).toBeUndefined(); + expect(Object.getOwnPropertySymbols(cleared)).not.toContain(SUBJECT_PERMISSIONS); + }); + + it('is not a name any predicate can read: the map is data for the seam, not a root', () => { + const scope = scopeFor(GRANTED); + // The symbol is invisible to string-keyed access, so the map the engine + // keeps out of the variable namespace stays out of it here too. + expect(Object.keys(scope.current_user)).not.toContain('permissions'); + const evaluator = new ExpressionEvaluator(scope); + expect(evaluator.evaluateCondition(cel('has(current_user.permissions)'), { throwOnError: true })).toBe(false); + }); +}); + +describe('the throwing useCondition leg — fail-closed `visible` (objectui#4421)', () => { + const visible = (scope: Record) => + new ExpressionEvaluator(scope).evaluateCondition(cel(CAN_DELETE), { throwOnError: true }); + + it('not loaded: the predicate FAULTS, so the fail-closed caller hides', () => { + expect(() => visible(scopeFor(undefined))).toThrow(/CEL predicate failed to evaluate/); + }); + + it('loaded and granted: true', () => { + expect(visible(scopeFor(GRANTED))).toBe(true); + }); + + it('loaded and denied: a clean false — not a fault', () => { + expect(visible(scopeFor(DENIED))).toBe(false); + }); + + it('answers identically on every alias of the subject', () => { + for (const receiver of ['current_user', 'user', 'ctx.user', 'os.user']) { + const src = cel(`${receiver}.can('account', 'delete')`); + expect(new ExpressionEvaluator(scopeFor(GRANTED)).evaluateCondition(src, { throwOnError: true })).toBe(true); + expect(new ExpressionEvaluator(scopeFor(DENIED)).evaluateCondition(src, { throwOnError: true })).toBe(false); + } + }); + + it('survives the `{ ...scope, ...context }` merge useCondition performs', () => { + const merged = { ...scopeFor(GRANTED), record: { id: 'r1' } }; + expect(new ExpressionEvaluator(merged).evaluateCondition(cel(CAN_DELETE), { throwOnError: true })).toBe(true); + }); +}); + +describe('the fail-soft useCondition legs report the missing payload (objectui#4421)', () => { + it('not loaded: the fault goes to the caller\'s fallback AND is reported, naming the missing input', () => { + const faults: string[] = []; + const verdict = new ExpressionEvaluator(scopeFor(undefined)).evaluateCondition(cel(CAN_DELETE), { + onFault: (reason) => faults.push(reason), + }); + // The fail-soft fallback of this entry is `true`: on a `disabled` leg that + // is DISABLED, on a fail-soft `visible` leg it is SHOWN. That is the + // surface's fault policy, not this binding's answer. + expect(verdict).toBe(true); + expect(faults).toHaveLength(1); + expect(faults[0]).toMatch(/carries no permission data/); + }); + + it('loaded: the verdict itself, with nothing reported', () => { + const faults: string[] = []; + const onFault = (reason: string) => faults.push(reason); + expect(new ExpressionEvaluator(scopeFor(GRANTED)).evaluateCondition(cel(`!${CAN_DELETE}`), { onFault })).toBe(false); + expect(new ExpressionEvaluator(scopeFor(DENIED)).evaluateCondition(cel(`!${CAN_DELETE}`), { onFault })).toBe(true); + expect(faults).toEqual([]); + }); +}); + +describe('the evalRowPredicate leg — `fallback: false` (objectui#4421)', () => { + const ROW = { id: 'r1', status: 'open' }; + const row = (scope: Record, warnOnError = false) => + evalRowPredicate(CAN_DELETE, ROW, { fallback: false, scope, warnOnError, label: 'custom_delete' }); + + it('not loaded: hidden, and the warning names the missing payload', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + expect(row(scopeFor(undefined), true)).toBe(false); + expect(warn.mock.calls.map((c) => String(c[0])).join('\n')).toMatch(/carries no permission data/); + }); + + it('loaded and granted: shown', () => { + expect(row(scopeFor(GRANTED))).toBe(true); + }); + + it('loaded and denied: hidden, with nothing to warn about', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + expect(row(scopeFor(DENIED), true)).toBe(false); + expect(warn).not.toHaveBeenCalled(); + }); + + it('combines with the row the way the card\'s logical-delete action needs', () => { + const src = `record.status == 'open' && ${CAN_DELETE}`; + expect(evalRowPredicate(src, ROW, { fallback: false, scope: scopeFor(GRANTED) })).toBe(true); + expect(evalRowPredicate(src, { ...ROW, status: 'void' }, { fallback: false, scope: scopeFor(GRANTED) })).toBe(false); + expect(evalRowPredicate(src, ROW, { fallback: false, scope: scopeFor(DENIED) })).toBe(false); + }); +}); + +describe('what the seam does NOT do (objectui#4421)', () => { + it('a map carried under a string key is not read — only the subject\'s own carries', () => { + const scope = { ...scopeFor(undefined), permissions: GRANTED }; + expect(evalFieldPredicate(CAN_DELETE, {}, false, undefined, scope, { warn: false })).toBe(false); + }); + + it('a receiver that is a COPY of the subject is refused, so every alias must be the one object', () => { + const scope = scopeFor(GRANTED); + const drifted = { ...scope, user: { ...scope.current_user } }; + const faults: string[] = []; + expect( + evalFieldPredicate("user.can('account', 'delete')", {}, false, undefined, drifted, { + warn: false, + onFault: (r) => faults.push(r), + }), + ).toBe(false); + expect(faults[0]).toMatch(/ACTING SUBJECT/); + }); +}); diff --git a/packages/permissions/src/__tests__/effectiveObjects-4421.test.tsx b/packages/permissions/src/__tests__/effectiveObjects-4421.test.tsx new file mode 100644 index 0000000000..04952c3089 --- /dev/null +++ b/packages/permissions/src/__tests__/effectiveObjects-4421.test.tsx @@ -0,0 +1,87 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * objectui#4421 — `effectiveObjects`: the `objects` map of the + * `/auth/me/permissions` response, handed on verbatim for the predicate + * binding `current_user.can(object, verb)`. + * + * The truth table, one row per `PermissionContextValue` source: + * - `MePermissionsProvider` with a response → the response's OWN `objects` + * object (identity, not a copy: the binding caches its adapted map on the + * payload object, AGENTS.md #10), with `isLoaded` true. + * - `MePermissionsProvider` with a response that grants nothing → `{}`, a + * real answer, NOT collapsed into `undefined`. + * - `PermissionProvider` (roles from config, no server payload) → `undefined`. + * - no provider at all → `undefined`, while `can` keeps its fail-open `true` + * for the built-in affordances — the two are separate answers. + */ + +import { describe, it, expect } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import React from 'react'; +import { MePermissionsProvider, type MePermissionsResponse } from '../MePermissionsProvider'; +import { PermissionProvider } from '../PermissionProvider'; +import { usePermissions } from '../usePermissions'; + +const seen: Array> = []; + +function Probe() { + const perms = usePermissions(); + seen.push(perms); + return {String(perms.isLoaded)}; +} + +function response(objects: MePermissionsResponse['objects']): MePermissionsResponse { + return { + authenticated: true, + userId: 'u1', + tenantId: null, + roles: [], + permissionSets: ['account_clerk'], + objects, + fields: {}, + }; +} + +describe('objectui#4421 — effectiveObjects', () => { + it('MePermissionsProvider hands on the response\'s own objects map', () => { + seen.length = 0; + const payload = response({ account: { allowRead: true, allowDelete: false } }); + render( + + + , + ); + expect(screen.getByTestId('loaded')).toHaveTextContent('true'); + expect(seen.at(-1)?.effectiveObjects).toBe(payload.objects); + }); + + it('a response that grants nothing is `{}`, not "no payload"', () => { + seen.length = 0; + render( + + + , + ); + expect(seen.at(-1)?.effectiveObjects).toEqual({}); + }); + + it('PermissionProvider has no server payload: undefined', () => { + seen.length = 0; + render( + + + , + ); + expect(seen.at(-1)?.isLoaded).toBe(true); + expect(seen.at(-1)?.effectiveObjects).toBeUndefined(); + }); + + it('no provider: undefined — while the built-in `can` stays fail-open', () => { + seen.length = 0; + render(); + expect(seen.at(-1)?.effectiveObjects).toBeUndefined(); + expect(seen.at(-1)?.can('account', 'delete')).toBe(true); + }); +}); From e5cbccde282e75421ad973ebee6a905e794b954c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 10:29:57 +0000 Subject: [PATCH 3/5] test(app-shell,permissions): type the 4421 pins for the test tsconfigs (objectui#4421) Claude-Session: https://claude.ai/code/session_0122Knsowci76D2rBWReCzzZ Co-authored-by: Claude --- .../__tests__/currentUserCan-4421.render.test.tsx | 13 ++++++++----- .../src/__tests__/effectiveObjects-4421.test.tsx | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/app-shell/src/providers/__tests__/currentUserCan-4421.render.test.tsx b/packages/app-shell/src/providers/__tests__/currentUserCan-4421.render.test.tsx index 385b3be90d..c6362d3af9 100644 --- a/packages/app-shell/src/providers/__tests__/currentUserCan-4421.render.test.tsx +++ b/packages/app-shell/src/providers/__tests__/currentUserCan-4421.render.test.tsx @@ -102,13 +102,16 @@ function BuiltinVerdict() { } function withState(state: State, surface: React.ReactNode) { - const tree = h( - ExpressionProvider, - { user: USER }, - h(ActionProvider, null, surface, h(BuiltinVerdict)), + const tree = ( + + + {surface} + + + ); if (state === 'not-loaded') return tree; - return h(MePermissionsProvider, { initialPermissions: payload(state === 'granted') }, tree); + return {tree}; } function registered(type: string): React.ComponentType { diff --git a/packages/permissions/src/__tests__/effectiveObjects-4421.test.tsx b/packages/permissions/src/__tests__/effectiveObjects-4421.test.tsx index 04952c3089..ab4aada2b0 100644 --- a/packages/permissions/src/__tests__/effectiveObjects-4421.test.tsx +++ b/packages/permissions/src/__tests__/effectiveObjects-4421.test.tsx @@ -53,7 +53,7 @@ describe('objectui#4421 — effectiveObjects', () => { , ); - expect(screen.getByTestId('loaded')).toHaveTextContent('true'); + expect(screen.getByTestId('loaded').textContent).toBe('true'); expect(seen.at(-1)?.effectiveObjects).toBe(payload.objects); }); From 76bef128ab69f0a2eeee41648fc954ac470afa14 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 11:33:16 +0000 Subject: [PATCH 4/5] test(app-shell): state the ablation's predicted direction per arm (objectui#4421) Claude-Session: https://claude.ai/code/session_0122Knsowci76D2rBWReCzzZ Co-authored-by: Claude --- .../__tests__/currentUserCan-4421.render.test.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/app-shell/src/providers/__tests__/currentUserCan-4421.render.test.tsx b/packages/app-shell/src/providers/__tests__/currentUserCan-4421.render.test.tsx index c6362d3af9..119622a703 100644 --- a/packages/app-shell/src/providers/__tests__/currentUserCan-4421.render.test.tsx +++ b/packages/app-shell/src/providers/__tests__/currentUserCan-4421.render.test.tsx @@ -44,9 +44,10 @@ * * Drop the `permissions` hand-off in `evalFieldPredicate` (`fieldRules.ts`): * every GRANTED arm goes red (the engine refuses `can` with no data, so the - * fail-closed surfaces hide the action for a grant holder too); the NOT-LOADED - * and DENIED arms stay green — they hide either way, which is why the denied - * arm also asserts silence and the not-loaded arm asserts the named reason. + * fail-closed surfaces hide the action for a grant holder too), and every + * DENIED arm goes red through its SILENCE assertion only — the action is + * hidden either way, and it is the reported fault that tells the ablated tree + * apart. The NOT-LOADED arms stay green: nothing was handed off there to drop. */ import '@testing-library/jest-dom/vitest'; From d4bf5612394bbece7538580def5ded28352d5b9c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 11:38:17 +0000 Subject: [PATCH 5/5] test(app-shell,core): one locator per arm, so the denied arm's silence is measured, not inherited from warn-once (objectui#4421) Claude-Session: https://claude.ai/code/session_0122Knsowci76D2rBWReCzzZ Co-authored-by: Claude --- .../currentUserCan-4421.render.test.tsx | 57 ++++++++++--------- .../__tests__/subjectPermissions-4421.test.ts | 14 +++-- 2 files changed, 40 insertions(+), 31 deletions(-) diff --git a/packages/app-shell/src/providers/__tests__/currentUserCan-4421.render.test.tsx b/packages/app-shell/src/providers/__tests__/currentUserCan-4421.render.test.tsx index 119622a703..b6eb03862b 100644 --- a/packages/app-shell/src/providers/__tests__/currentUserCan-4421.render.test.tsx +++ b/packages/app-shell/src/providers/__tests__/currentUserCan-4421.render.test.tsx @@ -69,11 +69,17 @@ const h = React.createElement; /** The logical delete the app ships in place of the built-in Delete. */ const CAN_DELETE = { dialect: 'cel', source: "current_user.can('account', 'delete')" }; -const LOGICAL_DELETE = { - name: 'account_logical_delete', - label: 'Void account', - visible: CAN_DELETE, -}; +/** + * One action NAME per arm, the predicate text identical in all three. Every + * fault report on these surfaces is warn-once per (locator, predicate) for the + * life of the module, and the locator carries the action name — so with one + * shared name, the NOT-LOADED arm's report would silence the same report in + * any later arm, and the DENIED arm's "nothing reported" would pass for a + * reason that has nothing to do with the verdict. + */ +function logicalDelete(state: State) { + return { name: `account_void_${state.replace('-', '_')}`, label: 'Void account', visible: CAN_DELETE }; +} /** An ungated companion, so "not rendered" can never mean "nothing rendered". */ const COMPANION = { name: 'print', label: 'Print' }; @@ -132,28 +138,29 @@ const ENGINE_REASON = /carries no permission data/; const THREW = /was hidden\/disabled: its predicate threw — CEL predicate failed to evaluate: current_user\.can\('account', 'delete'\)/; const SURFACES: Record React.ReactNode; - find: (name: string) => HTMLElement | null; + mount: (state: State) => React.ReactNode; + find: (state: State | 'companion') => HTMLElement | null; fault: RegExp; }> = { 'row menu (RowActionMenu, evalRowPredicate)': { - mount: () => + mount: (state) => h(RowActionMenu, { row: RECORD, onActionDef: () => {}, // `variant: 'primary'` renders the defs as always-mounted inline // buttons, so the assertion does not depend on opening the menu. rowActionDefs: [ - { ...LOGICAL_DELETE, variant: 'primary' }, + { ...logicalDelete(state), variant: 'primary' }, { ...COMPANION, variant: 'primary' }, ] as never, maxInlineActions: 2, }), - find: (name) => screen.queryByTestId(`row-action-inline-${name}`), + find: (state) => + screen.queryByTestId(`row-action-inline-${state === 'companion' ? COMPANION.name : logicalDelete(state).name}`), fault: ENGINE_REASON, }, 'record header (page:header, evalRowPredicate)': { - mount: () => + mount: (state) => h( RecordContextProvider, { objectName: 'account', recordId: RECORD.id, data: RECORD, objectSchema: { name: 'account', fields: {} } } as never, @@ -163,30 +170,28 @@ const SURFACES: Record - screen.queryByText(name === LOGICAL_DELETE.name ? LOGICAL_DELETE.label : COMPANION.label), + find: (state) => screen.queryByText(state === 'companion' ? COMPANION.label : 'Void account'), fault: ENGINE_REASON, }, 'action:button (useCondition, throwOnError)': { - mount: () => + mount: (state) => h( React.Fragment, null, h(registered('action:button'), { - schema: { type: 'action:button', actionType: 'script', ...LOGICAL_DELETE }, + schema: { type: 'action:button', actionType: 'script', ...logicalDelete(state) }, }), h(registered('action:button'), { schema: { type: 'action:button', actionType: 'script', ...COMPANION }, }), ), - find: (name) => - screen.queryByText(name === LOGICAL_DELETE.name ? LOGICAL_DELETE.label : COMPANION.label), + find: (state) => screen.queryByText(state === 'companion' ? COMPANION.label : 'Void account'), fault: THREW, }, }; @@ -205,25 +210,25 @@ describe.each(Object.entries(SURFACES))( (_label, surface) => { it('NOT LOADED: hidden, reported as a missing payload — the built-in no-provider `true` is not inherited', () => { const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); - render(withState('not-loaded', surface.mount())); - expect(surface.find(COMPANION.name)).toBeInTheDocument(); - expect(surface.find(LOGICAL_DELETE.name)).not.toBeInTheDocument(); + render(withState('not-loaded', surface.mount('not-loaded'))); + expect(surface.find('companion')).toBeInTheDocument(); + expect(surface.find('not-loaded')).not.toBeInTheDocument(); expect(warnings(warn)).toMatch(surface.fault); // Same tree, same verb: the built-in answer is the fail-open embed default. expect(screen.getByTestId('builtin-verdict')).toHaveTextContent('false:true'); }); it('LOADED, GRANTED: shown', () => { - render(withState('granted', surface.mount())); - expect(surface.find(LOGICAL_DELETE.name)).toBeInTheDocument(); + render(withState('granted', surface.mount('granted'))); + expect(surface.find('granted')).toBeInTheDocument(); expect(screen.getByTestId('builtin-verdict')).toHaveTextContent('true:true'); }); it('LOADED, DENIED: hidden, with nothing reported — a verdict, not a fault', () => { const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); - render(withState('denied', surface.mount())); - expect(surface.find(COMPANION.name)).toBeInTheDocument(); - expect(surface.find(LOGICAL_DELETE.name)).not.toBeInTheDocument(); + render(withState('denied', surface.mount('denied'))); + expect(surface.find('companion')).toBeInTheDocument(); + expect(surface.find('denied')).not.toBeInTheDocument(); expect(warnings(warn)).not.toMatch(/permission data|can\(|predicate threw/); expect(screen.getByTestId('builtin-verdict')).toHaveTextContent('true:false'); }); diff --git a/packages/core/src/evaluator/__tests__/subjectPermissions-4421.test.ts b/packages/core/src/evaluator/__tests__/subjectPermissions-4421.test.ts index e3d517c09b..204276bf3c 100644 --- a/packages/core/src/evaluator/__tests__/subjectPermissions-4421.test.ts +++ b/packages/core/src/evaluator/__tests__/subjectPermissions-4421.test.ts @@ -141,22 +141,26 @@ describe('the fail-soft useCondition legs report the missing payload (objectui#4 describe('the evalRowPredicate leg — `fallback: false` (objectui#4421)', () => { const ROW = { id: 'r1', status: 'open' }; - const row = (scope: Record, warnOnError = false) => - evalRowPredicate(CAN_DELETE, ROW, { fallback: false, scope, warnOnError, label: 'custom_delete' }); + // One locator per arm: the row warning is warn-once per (locator, predicate) + // for the module's life, so a shared locator would let the not-loaded arm's + // report silence the denied arm's — and "nothing to warn about" would pass + // for a reason unrelated to the verdict. + const row = (scope: Record, label: string, warnOnError = false) => + evalRowPredicate(CAN_DELETE, ROW, { fallback: false, scope, warnOnError, label }); it('not loaded: hidden, and the warning names the missing payload', () => { const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); - expect(row(scopeFor(undefined), true)).toBe(false); + expect(row(scopeFor(undefined), 'custom_delete:not-loaded', true)).toBe(false); expect(warn.mock.calls.map((c) => String(c[0])).join('\n')).toMatch(/carries no permission data/); }); it('loaded and granted: shown', () => { - expect(row(scopeFor(GRANTED))).toBe(true); + expect(row(scopeFor(GRANTED), 'custom_delete:granted')).toBe(true); }); it('loaded and denied: hidden, with nothing to warn about', () => { const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); - expect(row(scopeFor(DENIED), true)).toBe(false); + expect(row(scopeFor(DENIED), 'custom_delete:denied', true)).toBe(false); expect(warn).not.toHaveBeenCalled(); });