Skip to content

Commit 2daa6c0

Browse files
os-teslaclaude
andauthored
fix(app-shell): DeclaredActionsBar mirrors the ADR-0066 D4 capability gate (#9805)
Fixes #9572 Clause-②: no `@objectstack/spec` declares `requiredPermissions` as "enforced with 403 on the platform action route (script/flow/modal + MCP) and **mirrored as a UI hide**". `DeclaredActionsBar` filters its own declared-action list instead of routing through `ActionEngine.getActionsForLocation`, so the engine's gate never reached it and the key was **inert** on every action it drew — the last object-bound surface that did not mirror it, after `action:bar`, the grid row menu, the selection bar and the data-table row menu (objectui#9623). ⛔ **This restores a MIRROR, it does not move enforcement into the UI.** The server stays the sole authority and still answers 403; the request this bar sends is byte-identical either way. The fail-open-on-unknown doctrine in `useCapabilityGate` is untouched. ## The repair The shared `useCapabilityGate` is applied **once** to the located set, inside the bar's **own** `ActionProvider` — the provider `useConsoleActionRuntime` seeds with the same `user.systemPermissions` the engine reads, and the same one the dispatch this bar is about to make will carry. ⭐ **The placement is the repair, not an incidental.** `useCapabilityGate` reads the nearest provider **above** its caller. This bar is self-contained: it mounts its own provider and renders the buttons under it. A gate called in the bar's outer body would therefore read a *different* provider — the host's, if there is one — and in a standalone host would read none at all and fail open on every action forever: a gate that compiles, reviews as correct, and mirrors nothing. The new suite supplies the held set **only** through `useConsoleActionRuntime().actionProviderProps.context`, with no ambient provider and no predicate scope above the render, so the outer placement cannot pass it. Applying it to the **list** rather than per button is the objectui#3562 invariant: the toolbar chrome and the buttons read one filtered source, so a wholly denied set draws nothing instead of leaving the host an orphan divider and section label. ## Why the fail-open / fail-closed boundary composes The card fenced itself on this, correctly. It resolves by measurement, not argument: the capability gate is **ANDed in front of** the existing fail-closed `visible` CEL, so the composition is **monotone** — it can only ever hide more than before, and can never expose something that is hidden today. That is what makes a fail-OPEN gate safe to put in front of a fail-CLOSED one, and it is pinned in both directions (a `visible`-hidden action stays hidden even when the capability IS held). ## The `can_override` arm (objectui#5178) — re-derived, not inherited `can_override` is a per-**record** viewer flag an action's own `visible` CEL reads; `requiredPermissions` is a per-**caller** capability list checked against `user.systemPermissions`. They are orthogonal, they are ANDed, and `isOverrideDecision` still runs unchanged on every action that survives the filter. Pinned both ways: an override-only viewer keeps the override affordance on an action that declares no capability, and gets **no exemption** from one that does. ⚠️ One correction to the card's reading: objectui#3314 appears in this file's docblock about two `toPredicateInput` normalizations drifting, not about the `can_override` arm. The `can_override` history is objectui#5178 (the affordance) and objectui#5414 (approver names). Both were read before editing. ## Evidence Reverse verification and ablation are one-shot proofs; no permanent test artifact is left behind. **RED before the repair** — the new suite on the unrepaired tree: `Tests 5 failed | 6 passed (11)`. The five reds are the four hide arms plus the orphan-chrome arm. The six greens are the guards that must stay green: capability held, fail-OPEN on unknown `systemPermissions`, fail-OPEN with no user at all, chrome kept when one action survives, and the `can_override` affordance preserved. **Placement ablation** — the gate moved to the bar's outer body, everything else identical. Mutation proved on disk before the run (injected marker present, removed text absent, `git hash-object` differs from the `HEAD` blob), restored afterwards and verified byte-for-byte (`git diff HEAD` empty, restored blob hash equals the `HEAD` blob hash, zero marker residue). Result: the **same five** tests fail, the six guards stay green. Vitest resolves `@object-ui/*` through the root config's alias map to each package's `src`, so no rebuild step sits between the mutation and the measurement. **Gates** (exit codes captured to a file before reading, never through a pipe): | gate | verdict | |---|---| | `packages/app-shell` `type-check` (`tsc --noEmit` + `tsc -p tsconfig.test.json`) | exit 0 | | `pnpm exec vitest run packages/app-shell/` | exit 0 | | `node scripts/check-changeset-presence.mjs` | exit 0 | | `check-changeset-no-major` · `check-changeset-overwrite` | exit 0 | | `check-control-bytes` | exit 0 | | `check-vi-mock-specifiers` · `check-vi-mock-inherit` · `check-vi-mock-override-shape` | exit 0 | | `check-test-path-roots` | exit 0 | | `check-new-cross-file-line-citations --base origin/main` | exit 0 — 0 citations added | | `check-governed-queue-guard` surface membership | not governed (0 of 5 surfaces matched) | The dependency closure was built first (`turbo run build --filter '@object-ui/app-shell^...'`, 28/28) — an unbuilt closure makes `type-check` report `TS2307` on every workspace import, which is a missing prerequisite and not a red gate. **Lint — a declared narrowing, measured at `998c8fd`.** The repo-wide `pnpm lint` (`turbo run lint`, every package) is CI's run. Locally this was narrowed to the two source files the diff touches, with all three pieces of evidence: 1. the population is the root `eslint.config.js` (`files: ['**/*.{ts,tsx}']`); `packages/app-shell` has no config of its own; 2. the file count is read from `--format json`: **2 files linted**, 0 errors; 3. type-aware linting is **not** enabled — `eslint.config.js` sets no `parserOptions.project` and no `projectService` — so no rule in this config reads another file's types, and this diff cannot move the verdict on any file it does not contain. The diff also changes no eslint config and adds no inline disable. The modified source file's warning profile is byte-identical to `origin/main`'s (13 `no-explicit-any` + 1 `react-hooks/exhaustive-deps`, measured against the base revision of the same file): the change adds **zero** warnings to it. The new test file carries 3 `no-explicit-any` warnings, all inside test doubles copied verbatim from the two sibling `DeclaredActionsBar` suites — a divergent spelling in one of three sibling doubles would be worse than the warning. ## Clause ② `Clause-②: no`, and the delivered diff matches the declaration: no exported symbol is added (`DeclaredActionsToolbar` is module-private), no member is added to `DeclaredActionsBarProps` or to any published interface, no contract face is touched. The new component's props are `Pick`ed from the existing `DeclaredActionsBarProps` rather than restated. The change narrows a render set onto already-published contract text. ## Acceptance notes Out-of-scope observations, ⛔ none filed, ⛔ none fixed here: - The card's probe table reports all four probes at 0 hits on this file. Re-derived: three are 0; `getActionsForLocation` has exactly **one** hit, in a prose comment, not a call. The substance of the finding is unchanged. Noted, not filed — a card-accuracy nit, not a code defect. Carrier: this pull request's reader. - The card attributes the `can_override` OR to objectui#3314 "referenced in its own docblock". That reference is about `toPredicateInput` normalization drift; the `can_override` arm is objectui#5178. Noted, not filed, same class. Carrier: this pull request's reader. - **Reachability was flagged unmeasured by the card, and it stays partly unmeasurable from this repository.** `DeclaredActionsBar` is exported from `@object-ui/app-shell` with a public `actions?: ActionDef[]` prop whose element type declares `requiredPermissions`, so the surface is reachable by construction for any host. In-repo it is mounted only by `ApprovalsInboxPage`, twice, for `sys_approval_request` at `record_section`, and it passes no `actions` prop — the set it draws is whatever the backend declares, which is framework metadata this repository cannot read. So: reachable as a published API, and whether a capability-gated action reaches the console host **today** is a server-side fact, not an objectui one. This bears on grading, ⛔ not on the correctness of the repair. - `useCapabilityGate` has no variant that takes the held set explicitly, so every surface filtering its own list must be rendered under a provider that carries it — the ambient-context foot-gun this change had to step over. Measured: `DeclaredActionsBar` is the **only** file in the tree that both mounts an `ActionProvider` and calls the hook (lit control: `ObjectView` / `RecordDetailView` mount one and do not call it; `data-table.tsx` calls it and only names the provider in prose). No sibling carries the hazard, so there is nothing to file. Noted, not filed. Carrier: the docblock now states the rule in the file where the next reader will meet it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) _Generated by [Claude Code](https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq)_ --- _Generated by [Claude Code](https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0b26acf commit 2daa6c0

3 files changed

Lines changed: 399 additions & 20 deletions

File tree

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
---
4+
5+
`DeclaredActionsBar` now mirrors ADR-0066 D4 `requiredPermissions` as a UI hide
6+
(objectui#9572).
7+
8+
`@objectstack/spec` declares `requiredPermissions` as "enforced with 403 on the
9+
platform action route (script/flow/modal + MCP) and **mirrored as a UI hide**".
10+
This bar filters its own action list instead of routing through
11+
`ActionEngine.getActionsForLocation`, so the engine's gate never reached it and
12+
the declared key was inert on every action it drew — the one remaining
13+
object-bound surface that did not mirror it, after `action:bar`, the grid row
14+
menu, the selection bar and the data-table row menu (objectui#9623).
15+
16+
The shared `useCapabilityGate` is now applied **once** to the located set,
17+
inside the bar's own `<ActionProvider>` — the provider `useConsoleActionRuntime`
18+
seeds with the same `user.systemPermissions` the engine reads, and the same one
19+
the dispatch this bar makes will carry. A gate called in the bar's outer body
20+
would read a different provider (the host's, or none at all in a standalone
21+
host) and fail open forever, so the placement is pinned by the new suite rather
22+
than left to review.
23+
24+
Nothing is enforced in the console and no request changes: the server remains
25+
the sole authority and still answers 403. Unknown capabilities fail **open**
26+
(an absent `systemPermissions` is not a denial), an empty held set means "holds
27+
nothing" and gates normally, and because the capability gate is ANDed in front
28+
of the existing fail-closed `visible` CEL the composition is monotone — it can
29+
only ever hide more than before, never expose something that was hidden. The
30+
approvals admin-override affordance (objectui#5178) is unchanged: `can_override`
31+
is a per-record viewer flag an action's own `visible` reads, `requiredPermissions`
32+
is a per-caller capability list, and an override-only viewer gets neither an
33+
exemption from a declared capability nor any new exposure.
34+
35+
Applying the gate to the list rather than per button also keeps the toolbar
36+
chrome and the buttons reading one filtered source, so a wholly denied set draws
37+
nothing instead of leaving the host an orphan divider and section label.

‎packages/app-shell/src/views/DeclaredActionsBar.tsx‎

Lines changed: 104 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import { Button, Separator, cn, hasDeclaredVisibilityGate } from '@object-ui/com
3535
import {
3636
ActionProvider,
3737
useAction,
38+
useCapabilityGate,
3839
useCondition,
3940
toPredicateInput,
4041
usePredicateRecordContext,
@@ -421,6 +422,95 @@ const DeclaredActionButton: React.FC<{
421422
);
422423
};
423424

425+
/**
426+
* The located set, capability-gated — and the toolbar it draws.
427+
*
428+
* ## Why this is a component at all, and why it lives HERE
429+
*
430+
* [ADR-0066 D4 / objectui#9572] `@objectstack/spec` declares
431+
* `requiredPermissions` as "enforced with 403 on the platform action route …
432+
* and mirrored as a UI hide". This bar filters its own action list instead of
433+
* routing through `ActionEngine.getActionsForLocation`, so the engine's gate
434+
* never reached it and the declared key was INERT on every action it draws —
435+
* the one object-bound surface that did not mirror it, after `action:bar`, the
436+
* grid row menu, the selection bar and (objectui#9623) the data-table row menu.
437+
*
438+
* ⛔ A UI MIRROR of a decision the SERVER still enforces, and nothing more.
439+
* Unknown capabilities fail OPEN (see `useCapabilityGate`), an EMPTY held set
440+
* means "holds nothing" and gates normally, and the request this bar sends is
441+
* byte-identical either way. ⛔ No enforcement moves into the console.
442+
*
443+
* ⭐ PLACEMENT is load-bearing. `useCapabilityGate` reads the nearest
444+
* `<ActionProvider>` ABOVE its caller, and this bar is self-contained: it
445+
* mounts its OWN provider, seeded by `useConsoleActionRuntime` with the same
446+
* `user.systemPermissions` the engine reads. Calling the gate in the bar's
447+
* outer body would therefore read a DIFFERENT provider — the host's, if it has
448+
* one at all — and in a standalone host would read none and fail open on every
449+
* action forever: a gate that compiles, reviews as correct, and mirrors
450+
* nothing. Evaluated here, under this bar's own provider, it mirrors exactly
451+
* the capabilities the dispatch it is about to make will carry.
452+
* `DeclaredActionsBar.capabilityGate-9572.test.tsx` supplies the held set
453+
* through that provider ALONE, so the outer placement fails it.
454+
*
455+
* ## Why ONCE, over the list
456+
*
457+
* The objectui#3562 invariant: the chrome and the items must read one filtered
458+
* source. Gating per button would leave a host holding a divider + section
459+
* label with nothing under it whenever the whole declared set is denied —
460+
* exactly the orphan divider `label` is documented never to produce.
461+
*
462+
* ## How it composes with the two gates already here
463+
*
464+
* `visible` (fail-CLOSED) and `disabled` are evaluated per button, downstream
465+
* of this filter, and the three are ANDed. The composition is therefore
466+
* MONOTONE: it can only ever hide more than before, never show something that
467+
* was hidden — which is what makes a fail-OPEN gate safe to put in front of a
468+
* fail-CLOSED one. The approvals `can_override` arm (objectui#5178) is
469+
* untouched by construction: `can_override` is a per-RECORD viewer flag an
470+
* action's own `visible` CEL reads, `requiredPermissions` is a per-CALLER
471+
* capability list, and `isOverrideDecision` still runs unchanged on every
472+
* action that survives. An override-only viewer gets no exemption from a
473+
* declared capability, and gets no new exposure either.
474+
*/
475+
const DeclaredActionsToolbar: React.FC<
476+
Pick<DeclaredActionsBarProps, 'objectName' | 'record' | 'className' | 'label'> & {
477+
/** The located set, pre-filtered by `location` / `exclude`, ungated. */
478+
actions: ActionDef[];
479+
}
480+
> = ({ actions, objectName, record, className, label }) => {
481+
const { t } = useObjectTranslation();
482+
const mayInvoke = useCapabilityGate();
483+
const permitted = useMemo(
484+
() => actions.filter((action) => mayInvoke(action.requiredPermissions)),
485+
[actions, mayInvoke],
486+
);
487+
488+
// Nothing this viewer may invoke renders nothing — no toolbar chrome, no
489+
// orphan divider, same degrade as "nothing declared at this location".
490+
if (permitted.length === 0) return null;
491+
492+
return (
493+
<div className={cn('space-y-2', className)}>
494+
{label && (
495+
<>
496+
<Separator />
497+
<div className="text-xs font-medium text-muted-foreground">{label}</div>
498+
</>
499+
)}
500+
<div role="toolbar" aria-label={label || t('common.actions')} className="flex flex-row flex-wrap items-center gap-2">
501+
{permitted.map((action) => (
502+
<DeclaredActionButton
503+
key={action.name}
504+
action={action}
505+
objectName={objectName}
506+
record={record}
507+
/>
508+
))}
509+
</div>
510+
</div>
511+
);
512+
};
513+
424514
export function DeclaredActionsBar({
425515
objectName,
426516
record,
@@ -432,7 +522,6 @@ export function DeclaredActionsBar({
432522
label,
433523
}: DeclaredActionsBarProps) {
434524
const dataSource = useAdapter();
435-
const { t } = useObjectTranslation();
436525
// Fetch the object def (and its declared actions) unless the host passed
437526
// them in. `useMetadataItem` no-ops when `name` is undefined.
438527
const { item: objectDef } = useMetadataItem('object', actionsProp ? undefined : objectName);
@@ -468,29 +557,24 @@ export function DeclaredActionsBar({
468557
});
469558

470559
// Degrade gracefully — nothing declared at this location renders nothing (no
471-
// toolbar chrome, no provider churn).
560+
// toolbar chrome, no provider churn). This early exit stays on the LOCATION
561+
// filter alone, deliberately: the capability gate below needs this provider
562+
// above it, so it cannot also decide whether to mount it. A set that is
563+
// located but wholly denied therefore mounts an inert provider and its
564+
// (closed) dialogs, and draws no chrome — see `DeclaredActionsToolbar`.
472565
if (located.length === 0) return null;
473566

474567
return (
475568
<ActionProvider {...runtime.actionProviderProps}>
476-
<div className={cn('space-y-2', className)}>
477-
{label && (
478-
<>
479-
<Separator />
480-
<div className="text-xs font-medium text-muted-foreground">{label}</div>
481-
</>
482-
)}
483-
<div role="toolbar" aria-label={label || t('common.actions')} className="flex flex-row flex-wrap items-center gap-2">
484-
{located.map((action) => (
485-
<DeclaredActionButton
486-
key={action.name}
487-
action={action}
488-
objectName={objectName}
489-
record={record}
490-
/>
491-
))}
492-
</div>
493-
</div>
569+
{/* The capability gate runs INSIDE this provider — see the toolbar's own
570+
docblock for why the placement is the whole point. */}
571+
<DeclaredActionsToolbar
572+
actions={located}
573+
objectName={objectName}
574+
record={record}
575+
className={className}
576+
label={label}
577+
/>
494578
{runtime.dialogs}
495579
</ActionProvider>
496580
);

0 commit comments

Comments
 (0)