Skip to content

Commit fc0272a

Browse files
baozhoutaoclaude
andauthored
fix(actions): apply the ADR-0066 D4 capability gate on every action surface (framework#3923) (#2965)
`requiredPermissions` is meant to be one declaration with two enforcement surfaces: 403 on the server, hidden button in the UI. The UI half only ever ran inside `ActionEngine.getActionsForLocation` — and none of the surfaces those locations actually render on go through the engine. `page:header` (record_header / record_more), `action:bar` (list_toolbar) and the grid's `RowActionMenu` (list_item) each filter their own action list, so a button declaring a capability nobody holds rendered live and clickable on all three. For a `type: 'api'` action pointed at a self-authored endpoint nothing else was checking either — the platform action route, which is where the 403 comes from, never sees that request. All three now gate through one shared `useCapabilityGate()`, so they cannot drift from the engine or from each other. The rule is the engine's, unchanged: hide unless the caller holds ALL declared capabilities; an empty held set means "holds nothing" and gates; unknown — no action runtime, nothing resolved — fails OPEN, because the server is the authority and hiding a permitted user's button on missing client data is the worse failure. The record surface was also feeding the gate nothing to work with. `RecordDetailView` mounts its own `<ActionProvider>` that shadows the shell's for every action on that page, and seeded it with identity alone — no `systemPermissions`. Since unknown fails open, that by itself un-gated every record_header / record_more / record_section action on the one page those locations exist on. It now forwards the caller's resolved capabilities, and only once they have actually resolved, so a standalone embed with no PermissionProvider keeps failing open instead of hiding everything. `useRecordEditable`'s record-level explain probe went out on a bare `fetch(..., { credentials: 'include' })`. A bearer-token session carries its credential in the Authorization header, not a cookie, so the probe came back 401 on a valid admin session and the verdict silently failed open — the hook was inert in exactly the deployments it was written for. It now rides the host's authenticated fetch (`SchemaRendererProvider`'s `apiFetch`), falling back to the global one for standalone embeds. Verified against a real showcase app (probe actions declaring a held vs an unheld capability, checked on all four surfaces, both sides of the gate). Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 29ba3a8 commit fc0272a

16 files changed

Lines changed: 740 additions & 14 deletions
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
---
2+
"@object-ui/app-shell": patch
3+
"@object-ui/components": patch
4+
"@object-ui/plugin-detail": patch
5+
"@object-ui/plugin-grid": patch
6+
"@object-ui/react": patch
7+
---
8+
9+
fix(actions): apply the ADR-0066 D4 capability gate on every action surface (framework#3923)
10+
11+
An action declaring `requiredPermissions` is supposed to be one declaration with
12+
two enforcement surfaces: 403 on the server, hidden button in the UI. The UI half
13+
only ever ran inside `ActionEngine.getActionsForLocation` — and the surfaces
14+
`record_header`, `record_more`, `list_item` and `list_toolbar` actually render on
15+
do not go through the engine. They filter their own action lists. So a button
16+
declaring a capability nobody holds rendered, live and clickable, on the record
17+
header, in every grid row menu, and on the list toolbar. For a `type: 'api'`
18+
action pointed at a self-authored endpoint, nothing else was checking either: the
19+
platform's action route (which is where the 403 comes from) never sees that
20+
request.
21+
22+
`page:header`, `action:bar` (business *and* `systemActions`) and the grid's
23+
`RowActionMenu` now apply the same gate, via a shared `useCapabilityGate()` so
24+
the surfaces cannot drift apart. The rule is the engine's, unchanged: hide unless
25+
the caller holds **all** declared capabilities; an empty held set is "holds
26+
nothing" and gates; **unknown** — no action runtime, no resolved capabilities —
27+
fails OPEN, because the server is the authority and hiding a permitted user's
28+
button on missing client data is the worse failure.
29+
30+
The record surface was also feeding the gate nothing to work with.
31+
`RecordDetailView` mounts its own `<ActionProvider>`, which shadows the shell's
32+
for every action on that page, and seeded it with identity only — no
33+
`systemPermissions`. Since unknown fails open, that alone un-gated every
34+
`record_header` / `record_more` / `record_section` action on the one page those
35+
locations exist on. It now forwards the caller's resolved capabilities (and only
36+
once they have actually resolved, so a standalone embed without a
37+
`PermissionProvider` keeps failing open rather than hiding everything).
38+
39+
`useRecordEditable`'s record-level explain probe went out on a bare
40+
`fetch(..., { credentials: 'include' })`. A bearer-token session carries its
41+
credential in the `Authorization` header, not a cookie, so the probe came back
42+
401 on a perfectly valid admin session and the verdict silently failed open —
43+
the hook was inert in exactly the deployments it was written for. It now rides
44+
the host's authenticated fetch (`SchemaRendererProvider`'s `apiFetch`), falling
45+
back to the global one for standalone embeds.
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* RecordDetailView — the `user` seeded into the record surface's own
11+
* `<ActionProvider>` (ADR-0066 D4 / framework#3923).
12+
*
13+
* The record page mounts an ActionProvider that SHADOWS the shell-level one, so
14+
* the capability gate the engine reads (`user.systemPermissions`) is whatever
15+
* this object carries. It used to carry identity only — `{id, name, avatar}` —
16+
* and since `ActionEngine.getActionsForLocation` fails OPEN on `undefined`, every
17+
* `record_header` / `record_more` action's `requiredPermissions` was silently
18+
* unenforced on the one surface those locations exist on.
19+
*
20+
* Two properties are pinned here, and they pull in opposite directions:
21+
* • loaded permissions MUST reach the engine (otherwise: no gate at all);
22+
* • unloaded permissions MUST NOT be forwarded as `[]` (otherwise: every gated
23+
* action hides in a standalone embed that never mounts a PermissionProvider).
24+
*/
25+
26+
import { describe, it, expect } from 'vitest';
27+
import { resolveActionUser } from './RecordDetailView';
28+
29+
const user = { id: 'u1', name: 'Ada', image: 'https://cdn/a.png' };
30+
31+
describe('resolveActionUser — ADR-0066 D4 capability gate seeding (#3923)', () => {
32+
it('forwards loaded systemPermissions so the action gate can enforce', () => {
33+
expect(resolveActionUser(user, true, ['manage_platform_settings'])).toEqual({
34+
id: 'u1',
35+
name: 'Ada',
36+
avatar: 'https://cdn/a.png',
37+
systemPermissions: ['manage_platform_settings'],
38+
});
39+
});
40+
41+
it('forwards an EMPTY loaded set — "holds nothing" must gate, not fail open', () => {
42+
const resolved = resolveActionUser(user, true, []);
43+
expect(resolved.systemPermissions).toEqual([]);
44+
// The distinction the engine keys off: present-but-empty ≠ absent.
45+
expect(Array.isArray(resolved.systemPermissions)).toBe(true);
46+
});
47+
48+
it('omits systemPermissions while permissions are still loading (fail-open)', () => {
49+
const resolved = resolveActionUser(user, false, []);
50+
expect(resolved).toEqual({ id: 'u1', name: 'Ada', avatar: 'https://cdn/a.png' });
51+
expect('systemPermissions' in resolved).toBe(false);
52+
});
53+
54+
it('treats a loaded-but-undefined set as "holds nothing", not as unknown', () => {
55+
expect(resolveActionUser(user, true, undefined).systemPermissions).toEqual([]);
56+
});
57+
58+
it('keeps the anonymous fallback identity and still carries the gate', () => {
59+
const resolved = resolveActionUser(null, true, ['setup.access']);
60+
expect(resolved.id).toBe('current-user');
61+
expect(resolved.systemPermissions).toEqual(['setup.access']);
62+
});
63+
64+
it('never mutates the shared FALLBACK_USER constant', () => {
65+
resolveActionUser(undefined, true, ['a']);
66+
const second = resolveActionUser(undefined, false, undefined);
67+
expect('systemPermissions' in second).toBe(false);
68+
});
69+
});

packages/app-shell/src/views/RecordDetailView.tsx

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,38 @@ interface RecordDetailViewProps {
7373

7474
const FALLBACK_USER = { id: 'current-user', name: 'Demo User' };
7575

76+
/**
77+
* The `user` seeded into this view's own `<ActionProvider>` — identity plus, once
78+
* resolved, the caller's system capabilities.
79+
*
80+
* [ADR-0066 D4 / framework#3923] `systemPermissions` is not decoration here: this
81+
* provider SHADOWS the shell-level one (`useConsoleActionRuntime`) for every
82+
* action on the record surface, and `ActionEngine.getActionsForLocation` reads the
83+
* capability gate off `runner.getContext().user.systemPermissions`. The engine
84+
* fails OPEN on `undefined` (unknown ≠ denied), so shipping identity alone
85+
* silently un-gated every `record_header` / `record_more` action that declared
86+
* `requiredPermissions` — the button rendered, and only the server's 403 stopped
87+
* it (and only for platform action routes at that).
88+
*
89+
* The `permissionsLoaded` gate keeps the two states apart: `usePermissions()`
90+
* returns `[]` both for "holds no capabilities" and for "no PermissionProvider /
91+
* still resolving". Forwarding the latter as `[]` would flip the gate fail-CLOSED
92+
* and hide gated actions in a standalone embed, so it stays `undefined` until the
93+
* answer is real.
94+
*/
95+
export function resolveActionUser(
96+
user: { id: string; name: string; image?: string } | null | undefined,
97+
permissionsLoaded: boolean,
98+
systemPermissions: string[] | undefined,
99+
): { id: string; name: string; avatar?: string; systemPermissions?: string[] } {
100+
const identity = user
101+
? { id: user.id, name: user.name, avatar: user.image }
102+
: FALLBACK_USER;
103+
return permissionsLoaded
104+
? { ...identity, systemPermissions: systemPermissions ?? [] }
105+
: identity;
106+
}
107+
76108
/**
77109
* Audit field names auto-injected by the framework's `applySystemFields`.
78110
* Filtered out of the auto-generated body sections — they are rendered
@@ -948,7 +980,7 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri
948980
// are still loading (`isLoaded === false`, e.g. no PermissionProvider in a
949981
// standalone embed) the gate stays open — fail-open is safe because the
950982
// server enforces data access regardless; this is purely a UI/DX filter.
951-
const { can: canOnObject, isLoaded: permissionsLoaded, getObjectApiOperations } = usePermissions();
983+
const { can: canOnObject, isLoaded: permissionsLoaded, getObjectApiOperations, systemPermissions } = usePermissions();
952984
// [#3546] Server-resolved effective API operation set for this object
953985
// (`/me/permissions` `apiOperations`). Threaded as the 2nd arg into
954986
// `resolveRecordHeaderActionGates` for the detail header's Edit/Delete and
@@ -1216,9 +1248,12 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri
12161248
// Memoize so the object identity is stable across renders — otherwise
12171249
// any effect that depends on it (e.g. the feed loader below) would
12181250
// re-fire every render and create an infinite request loop.
1251+
//
1252+
// Carries the ADR-0066 D4 capability gate into this view's ActionProvider —
1253+
// see `resolveActionUser` for why that matters (framework#3923).
12191254
const currentUser = useMemo(
1220-
() => (user ? { id: user.id, name: user.name, avatar: user.image } : FALLBACK_USER),
1221-
[user?.id, user?.name, user?.image],
1255+
() => resolveActionUser(user, permissionsLoaded, systemPermissions),
1256+
[user?.id, user?.name, user?.image, permissionsLoaded, systemPermissions],
12221257
);
12231258

12241259
// Fetch comments from API.

packages/components/src/__tests__/action-bar.test.tsx

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import { describe, it, expect } from 'vitest';
66
import { render, screen } from '@testing-library/react';
77
import { ComponentRegistry } from '@object-ui/core';
8+
import { ActionProvider } from '@object-ui/react';
89
import { renderComponent, validateComponentRegistration } from './test-utils';
910

1011
// Ensure action renderers are loaded (side-effect imports via vitest.setup.tsx)
@@ -381,4 +382,54 @@ describe('ActionBar (action:bar)', () => {
381382
expect(toolbar?.className).toContain('flex-row');
382383
});
383384
});
385+
386+
/**
387+
* [ADR-0066 D4 / framework#3923] The bar filters its own action list instead
388+
* of going through `ActionEngine.getActionsForLocation`, so the engine's
389+
* capability gate never reached `list_toolbar`: an action declaring a
390+
* capability nobody holds rendered as a live button, with nothing behind it
391+
* for a `type: 'api'` action pointed at a custom endpoint (the platform's
392+
* action route — the source of the 403 — never sees that request).
393+
*/
394+
describe('requiredPermissions capability gate', () => {
395+
const Bar = ({ actions, systemActions }: { actions?: any[]; systemActions?: any[] }) => {
396+
const Component = ComponentRegistry.get('action:bar')!;
397+
// eslint-disable-next-line react-hooks/static-components -- registry component is stable
398+
return <Component schema={{ type: 'action:bar', location: 'list_toolbar', actions, systemActions }} />;
399+
};
400+
const withUser = (user: unknown, props: { actions?: any[]; systemActions?: any[] }) =>
401+
render(<ActionProvider context={{ user } as any}><Bar {...props} /></ActionProvider>);
402+
403+
const gated = { name: 'gated', label: 'Bulk Reassign', type: 'api', requiredPermissions: ['manage_users'] };
404+
const plain = { name: 'plain', label: 'Export', type: 'api' };
405+
406+
it('hides an action whose capability the caller lacks', () => {
407+
const { container } = withUser({ id: 'u1', systemPermissions: ['setup.access'] }, { actions: [gated, plain] });
408+
expect(container.textContent).not.toContain('Bulk Reassign');
409+
expect(container.textContent).toContain('Export');
410+
});
411+
412+
it('shows it when the capability is held', () => {
413+
const { container } = withUser({ id: 'u1', systemPermissions: ['manage_users'] }, { actions: [gated] });
414+
expect(container.textContent).toContain('Bulk Reassign');
415+
});
416+
417+
it('gates on an EMPTY held set — "holds nothing" is known, not unknown', () => {
418+
const { container } = withUser({ id: 'u1', systemPermissions: [] }, { actions: [gated] });
419+
expect(container.textContent).not.toContain('Bulk Reassign');
420+
});
421+
422+
it('fails OPEN when the host never resolved capabilities', () => {
423+
const { container } = withUser({ id: 'u1' }, { actions: [gated] });
424+
expect(container.textContent).toContain('Bulk Reassign');
425+
});
426+
427+
it('gates systemActions too — the overflow slot is not a bypass', () => {
428+
const { container } = withUser(
429+
{ id: 'u1', systemPermissions: [] },
430+
{ actions: [plain], systemActions: [{ ...gated, label: 'Purge All' }] },
431+
);
432+
expect(container.textContent).not.toContain('Purge All');
433+
});
434+
});
384435
});
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* `page:header` — ADR-0066 D4 `requiredPermissions` gate (framework#3923).
11+
*
12+
* `record_header` / `record_more` actions render through THIS component, which
13+
* filters its own action list rather than calling
14+
* `ActionEngine.getActionsForLocation`. So the engine's capability gate never
15+
* ran on the one surface those locations exist on: an action declaring a
16+
* capability nobody holds rendered as a live button, and the only thing behind
17+
* it was the server's 403 — which covers platform action routes only, never a
18+
* `type: 'api'` action pointed at a custom endpoint.
19+
*
20+
* The gate reads the ActionProvider's runner user (the same object the engine
21+
* reads), falls back to the ambient predicate scope, and fails OPEN when
22+
* neither knows — unknown is not denied.
23+
*/
24+
25+
import { describe, it, expect } from 'vitest';
26+
import { render, screen } from '@testing-library/react';
27+
import { ComponentRegistry } from '@object-ui/core';
28+
import { ActionProvider, PredicateScopeProvider } from '@object-ui/react';
29+
30+
function PageHeader({ schema }: { schema: any }) {
31+
const Component = ComponentRegistry.get('page:header');
32+
if (!Component) throw new Error('page:header not registered');
33+
// eslint-disable-next-line react-hooks/static-components -- registry component is stable
34+
return <Component schema={schema} />;
35+
}
36+
37+
const schema = {
38+
type: 'page:header',
39+
title: 'Invoice',
40+
actions: [
41+
{ name: 'gated', label: 'Set Parallel Reviewers', type: 'api', requiredPermissions: ['ehr_probe_capability'] },
42+
{ name: 'plain', label: 'Print', type: 'api' },
43+
],
44+
};
45+
46+
const renderWithUser = (user: unknown, scope?: Record<string, any>) => {
47+
const tree = <ActionProvider context={{ user } as any}><PageHeader schema={schema} /></ActionProvider>;
48+
return render(scope ? <PredicateScopeProvider scope={scope}>{tree}</PredicateScopeProvider> : tree);
49+
};
50+
51+
const gatedShown = () => !!screen.queryByRole('button', { name: /Set Parallel Reviewers/i });
52+
53+
describe('page:header — ADR-0066 D4 capability gate (#3923)', () => {
54+
it('hides an action whose requiredPermissions the caller lacks', () => {
55+
renderWithUser({ id: 'u1', systemPermissions: ['setup.access'] });
56+
expect(gatedShown()).toBe(false);
57+
// The ungated action is untouched — this filters, it does not blank the bar.
58+
expect(screen.getByRole('button', { name: /Print/i })).toBeTruthy();
59+
});
60+
61+
it('shows the action when every declared capability is held', () => {
62+
renderWithUser({ id: 'u1', systemPermissions: ['ehr_probe_capability', 'setup.access'] });
63+
expect(gatedShown()).toBe(true);
64+
});
65+
66+
it('requires ALL declared capabilities (AND, not OR)', () => {
67+
render(
68+
<ActionProvider context={{ user: { id: 'u1', systemPermissions: ['a'] } } as any}>
69+
<PageHeader
70+
schema={{
71+
type: 'page:header',
72+
title: 'Invoice',
73+
actions: [{ name: 'both', label: 'Needs Both', type: 'api', requiredPermissions: ['a', 'b'] }],
74+
}}
75+
/>
76+
</ActionProvider>,
77+
);
78+
expect(screen.queryByRole('button', { name: /Needs Both/i })).toBeNull();
79+
});
80+
81+
it('gates on an EMPTY held set — "holds nothing" is known, not unknown', () => {
82+
renderWithUser({ id: 'u1', systemPermissions: [] });
83+
expect(gatedShown()).toBe(false);
84+
});
85+
86+
it('fails OPEN when the host never resolved capabilities', () => {
87+
renderWithUser({ id: 'u1' });
88+
expect(gatedShown()).toBe(true);
89+
});
90+
91+
it('falls back to the ambient predicate scope when there is no runner user', () => {
92+
renderWithUser(undefined, { user: { id: 'u1', systemPermissions: ['setup.access'] } });
93+
expect(gatedShown()).toBe(false);
94+
});
95+
});

packages/components/src/renderers/action/action-bar.tsx

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ import React, { forwardRef, useMemo } from 'react';
3535
import { ComponentRegistry } from '@object-ui/core';
3636
import type { ActionSchema, ActionLocation, ActionComponent } from '@object-ui/types';
3737
import { ACTION_LOCATIONS } from '@object-ui/types';
38-
import { useCondition, toPredicateInput } from '@object-ui/react';
38+
import { useCondition, toPredicateInput, useCapabilityGate } from '@object-ui/react';
3939
import { useObjectTranslation } from '@object-ui/i18n';
4040
import { cn } from '../../lib/utils';
4141
import { useIsMobile } from '../../hooks/use-mobile';
@@ -119,13 +119,21 @@ const ActionBarRenderer = forwardRef<HTMLDivElement, { schema: ActionBarSchema;
119119
label: `action:bar${schema.location ? ` (${schema.location})` : ''} (visible)`,
120120
});
121121
const isMobile = useIsMobile();
122+
// [ADR-0066 D4 / framework#3923] Shared capability gate — see below.
123+
const mayInvoke = useCapabilityGate();
122124

123125
// Filter business actions by location and deduplicate by name
124126
const filteredActions = useMemo(() => {
125127
const actions = schema.actions || [];
128+
// [ADR-0066 D4 / framework#3923] Capability gate — this bar filters its
129+
// own set instead of going through `ActionEngine.getActionsForLocation`,
130+
// so without this a `list_toolbar` action declaring a capability nobody
131+
// holds rendered as a live button. Same rule as the engine; unknown
132+
// capabilities fail OPEN (see `useCapabilityGate`).
133+
const permitted = actions.filter(a => mayInvoke((a as any)?.requiredPermissions));
126134
const located = !schema.location
127-
? actions
128-
: actions.filter(
135+
? permitted
136+
: permitted.filter(
129137
a => !a.locations || a.locations.length === 0 || a.locations.includes(schema.location!),
130138
);
131139
// Deduplicate by action name — keep first occurrence
@@ -163,20 +171,23 @@ const ActionBarRenderer = forwardRef<HTMLDivElement, { schema: ActionBarSchema;
163171
});
164172
}
165173
return deduped;
166-
}, [schema.actions, schema.location]);
174+
}, [schema.actions, schema.location, mayInvoke]);
167175

168176
// System actions: always go into the overflow menu, deduped by name,
169177
// never filtered by location (they're chrome, not business logic).
170178
const systemActions = useMemo(() => {
171179
const actions = schema.systemActions || [];
172180
const seen = new Set<string>();
181+
// Chrome or not, a declared capability gates it (ADR-0066 D4) — a host
182+
// that puts a gated action in this slot means the same thing by it.
173183
return actions.filter(a => {
184+
if (!mayInvoke((a as any)?.requiredPermissions)) return false;
174185
if (!a.name) return true;
175186
if (seen.has(a.name)) return false;
176187
seen.add(a.name);
177188
return true;
178189
});
179-
}, [schema.systemActions]);
190+
}, [schema.systemActions, mayInvoke]);
180191

181192
// Split business actions into visible inline and overflow.
182193
// On mobile, show fewer actions inline (default: 1).

0 commit comments

Comments
 (0)