Skip to content

feat(permissions): let permissionKey express anyOf/allOf key sets - #1765

Merged
sarajkrishnasingh merged 2 commits into
masterfrom
fm/sistent-1747-permissionkey-set
Aug 4, 2026
Merged

feat(permissions): let permissionKey express anyOf/allOf key sets#1765
sarajkrishnasingh merged 2 commits into
masterfrom
fm/sistent-1747-permissionkey-set

Conversation

@willcalcote

@willcalcote willcalcote commented Aug 4, 2026

Copy link
Copy Markdown
Member

Intent

Fix #1747: permissionKey cannot express an OR of keys, so any affordance gated on more than one permission key cannot adopt PermissionShield at all. The issue contains a fully specified, backwards-compatible design that was approved; implement that design.

What to build: extend permissionKey to accept a key set with an explicit combinator, defaulting to today's single-key behaviour: permissionKey?: Key | { anyOf: Key[] } | { allOf: Key[] }.

  1. useHasPermission resolves anyOf with .some(...) and allOf with .every(...). A bare Key must keep working byte-for-byte as it does today.
  2. PermissionShield lists EVERY unmet key in its tooltip, not just one. For anyOf that means "needs any of: A, B"; for allOf, "needs all of: A, B". This is the whole point of the issue - the user currently gets no explanation at all.
  3. NavigationNavbar previously dropped the legacy permission boolean as soon as a key was present (disabled={hasKey ? undefined : !permission}). A key SET must flow through that same path correctly and still render the shield.

Section/parent items - required, and the reason this task exists: the downstream consumer needs this to work on a parent/section nav item, not only leaves. A section is reachable exactly when the user can reach at least one of its children, which is { anyOf: [...every child key...] }. Two things must hold for a section whose keys are all unmet: it renders a PermissionShield explaining why, and it cannot be expanded or clicked at all. If NavigationNavbar let a disabled section still toggle its children open, fixing that is part of this task. The exact complaint is that clicking such a section dumps the user on a "you don't have permission" error page.

Acceptance criteria:

  • permissionKey accepts Key, { anyOf: Key[] }, and { allOf: Key[] }; types exported.
  • Bare-Key behaviour is unchanged - proven with a regression test. This library has many consumers; a silent narrowing of access here is the worst possible outcome.
  • Empty/malformed sets are handled explicitly rather than silently granting access. The safe semantic was decided, stated in a comment, and tested.
  • Unit tests cover: single key, anyOf hit/miss, allOf hit/miss, and the shield tooltip listing all unmet keys.
  • A disabled section item cannot be expanded or activated; covered by a test.
  • Storybook/docs updated if this repo documents these props there.
  • No behaviour change for any component that does not pass a key set.

Context: this blocks layer5io/meshery-cloud, whose Identity -> Organizations nav item is gated on ViewAllOrganizations || ViewOrg and is the one entry stuck on the legacy boolean path, so it shows as disabled with no explanation. Once this lands and is released, meshery-cloud bumps the sistent dependency and adopts key sets for both that item and the Identity section header. meshery-cloud is NOT changed here - that is a separate task.

Keep the change proportional to the defect: a focused API extension plus its tests, not a permissions framework. Do not refactor unrelated permission code.

Decisions and tradeoffs made while implementing, which a reviewer reading only the diff would not know:

  • Empty/malformed sets DENY. An empty, non-array, nullish-member, or double-combinator ({anyOf, allOf} both present) set resolves to false. Rationale, stated in a code comment: supplying a set is an explicit statement that the affordance IS gated, so a set that cannot be evaluated must not fall through to "permitted"; and [].every(...) is vacuously true, so { allOf: [] } would otherwise silently grant access to everyone. This is deliberate and covered by a five-case parameterised test.
  • The no-provider default stays unconditionally true, INCLUDING for a malformed set. This was a deliberate choice, not an oversight: PermissionProvider's documented contract is "if no PermissionProvider is present, all permission checks default to permitted - ensuring full backward compatibility". With no provider mounted the host has not wired up authorization at all, so nothing is evaluated and nothing is gated; the empty/malformed denial applies wherever permissions are actually evaluated. Both branches are tested.
  • NavigationNavbar was restructured into per-item components (NavigationNavbarItem / NavigationNavbarSubItem) because evaluating each item's permission requires a hook, and hooks cannot be called inside a .map callback. This is required by the fix, not gratuitous refactoring.
  • An unreachable nav item is now made genuinely INERT in JS - it withholds onClick, withholds the expand-toggle handler, and forces the section collapsed (expanded = isPermitted && isOpen) - rather than only being styled disabled. This is the actual root cause: MUI enforces disabled on a non-<button> element (a MenuItem renders <li>) purely with pointer-events: none in the Mui-disabled class, and PermissionShield blocks its children the same way, so the handlers stay attached and a "disabled" section could still be toggled open and activated. This also means legacy permission: false nav items no longer fire onClick on the programmatic path; in a real browser that is unobservable because MUI's CSS already blocked the click, and it is covered by its own test. This is intentional and is the defect being fixed.
  • The two nav-inertness tests were verified to FAIL against the pre-fix navbar logic and pass with it, so they are not vacuous.
  • PermissionShield now derives the unmet keys itself via a new useUnmetPermissionKeys hook reading the provider context. It remains a pure visual component in the sense that it never second-guesses the caller's disabled decision: if the caller shields a key the user does in fact hold, it falls back to rendering the declared keys, which keeps the single-key rendering identical to today. A set that names no keys falls back to the original "Missing requisite key" wording rather than emitting a dangling "Needs any of: ".
  • Category/subcategory chips in the tooltip are deduped across the displayed keys, which reduces to exactly today's output for a single key.
  • New public exports (PermissionKeySet, PermissionKeySpec, isPermissionKeySet, useUnmetPermissionKeys) were added explicitly to src/index.tsx, per the repo's documented rollup-plugin-dts nested-barrel drop rule, and their presence in the built dist/index.d.ts was verified.
  • No Storybook/docs update was needed: this repo has a single stories file (DangerConfirmationModal) and no docs surface covering these props.
  • One section was added to AGENTS.md recording the MUI-disabled/jsdom sharp edge, because a future agent testing disabled state would otherwise write a test that proves nothing.

Verified locally: 22 new tests in src/__testing__/permissionKeySet.test.tsx, full suite 492 passing across 27 suites, npm run lint clean, npm run build clean. Pre-existing repo state that is NOT caused by this change: prettier --check fails on many files and tsc --noEmit reports errors across src/ - neither is a CI gate (see AGENTS.md); files already unformatted before this change were deliberately left unformatted rather than mass-reformatted.

What Changed

  • permissionKey (on Button, IconButton, MenuItem, ListItem, ListItemButton, PermissionShield, and useHasPermission) now accepts Key | { anyOf: Key[] } | { allOf: Key[] }, resolved with .some(...) / .every(...). A bare Key behaves exactly as before; an empty, non-array, nullish-member, or double-combinator set denies rather than falling through to permitted (so { allOf: [] } cannot vacuously grant access), while the no-provider default stays unconditionally permitted. PermissionKeySet, PermissionKeySpec, isPermissionKeySet, and useUnmetPermissionKeys are re-exported explicitly from src/index.tsx.
  • PermissionShield derives the unmet keys from provider context via the new useUnmetPermissionKeys hook and lists all of them in its tooltip — Needs any of: A, B for anyOf, Needs all of: A, B for allOf — with category/subcategory chips deduped across displayed keys. It never overrides the caller's disabled decision, and a set that resolves to no keys falls back to the original "Missing requisite key" wording.
  • NavigationNavbar was split into per-item NavigationNavbarItem / NavigationNavbarSubItem components so each item can evaluate its own permission hook (hooks can't run inside a .map callback). An unpermitted item is now genuinely inert in JS — onClick and the expand toggle are withheld and the section is forced collapsed (expanded = isPermitted && isOpen) — instead of relying on MUI's pointer-events: none styling, so a shielded section can no longer be opened or activated. Covered by 22 new tests in src/__testing__/permissionKeySet.test.tsx (the two nav-inertness tests were confirmed to fail against the pre-fix navbar), plus an AGENTS.md note recording the MUI-disabled/jsdom sharp edge.

Risk Assessment

✅ Low: A well-bounded, backwards-compatible widening of an approved API: the bare-Key path is provably unchanged, the deny-on-malformed semantics and the nav-inertness fix are explicitly decided and covered by tests, new exports follow the repo's root re-export rule, and only informational nits remain — none blocking.

Testing

Ran the focused suite for this change (permissionKeySet.test.tsx, 22 tests) plus the adjacent permission, nav-type and dist-backed export guards — all green — and confirmed the new tests are not vacuous by replaying the two nav-inertness cases against the pre-fix navbar, where exactly those two fail. Verified the API is actually published by building and finding PermissionKeySet, PermissionKeySpec, isPermissionKeySet and useUnmetPermissionKeys in dist/index.d.ts and the runtime bundle. For end-user evidence I bundled the library from src/ and drove it in real headless Chromium reproducing the meshery-cloud Identity/Organizations case with real generated permission keys, capturing eight screenshots, a screen recording and a click-by-click transcript: the tooltip now names every unmet key for both combinators, bare-Key wording is byte-identical to before, a malformed { allOf: [] } denies, and an unreachable section resists both real pointer clicks and programmatic click dispatches that bypass MUI's pointer-events: none, while satisfying one arm of the anyOf restores expansion and navigation. Removed the dist/ build output afterwards; the working tree is clean.

  • Evidence: anyOf tooltip lists every unmet key (the fix's whole point) (local file: /tmp/no-mistakes-evidence/01KZ5MGH8XCB94Z0TMMXHBJT4M/03-anyOf-tooltip-lists-every-unmet-key.png)
  • Evidence: Unreachable section is inert — real + programmatic clicks leave it collapsed, no navigation (local file: /tmp/no-mistakes-evidence/01KZ5MGH8XCB94Z0TMMXHBJT4M/02-no-permissions-section-cannot-expand.png)
  • Evidence: One arm of the anyOf held: section unshields, expands and navigates; bare-Key child still shielded (local file: /tmp/no-mistakes-evidence/01KZ5MGH8XCB94Z0TMMXHBJT4M/07-anyOf-satisfied-navigates.png)
  • Evidence: allOf tooltip — Needs all of: View Organizations, Create Organization (local file: /tmp/no-mistakes-evidence/01KZ5MGH8XCB94Z0TMMXHBJT4M/04-allOf-tooltip.png)
  • Evidence: Bare-Key regression — original "Missing requisite key" wording, single key (local file: /tmp/no-mistakes-evidence/01KZ5MGH8XCB94Z0TMMXHBJT4M/05-bare-key-tooltip-unchanged.png)
  • Evidence: Member with no org permissions — every key-set affordance shielded (local file: /tmp/no-mistakes-evidence/01KZ5MGH8XCB94Z0TMMXHBJT4M/01-no-permissions-overview.png)
  • Evidence: anyOf satisfied — section expands on a real pointer click (local file: /tmp/no-mistakes-evidence/01KZ5MGH8XCB94Z0TMMXHBJT4M/06-anyOf-satisfied-section-expands.png)
  • Evidence: Admin holding both keys — allOf unlocks; only the malformed { allOf: [] } stays denied (local file: /tmp/no-mistakes-evidence/01KZ5MGH8XCB94Z0TMMXHBJT4M/08-admin-profile-allOf-unlocked.png)
  • Evidence: Recorded walkthrough of the full browser run (local file: /tmp/no-mistakes-evidence/01KZ5MGH8XCB94Z0TMMXHBJT4M/09-walkthrough.webm)
Evidence: Browser transcript — every click and the exact tooltip text read from the live DOM

anyOf section tooltip : Authorization Required | Needs any of: View All Organizations, View Organizations | View All Organizations — See all organizations within a Cloud deployment... | View Organizations — See all organizations of which you are an administrator... allOf button tooltip : Needs all of: View Organizations, Create Organization bare-Key tooltip : Missing requisite key | Create Organization empty-set tooltip : Missing requisite key (i.e. { allOf: [] } denies, no dangling combinator text) real pointer click at the chevron -> intercepted by the shield overlay; child items rendered: 0 programmatic click, chevron -> child items rendered: 0 programmatic click, section row -> last navigation event: "—" [profile holding only View Organizations] real pointer click on chevron -> child items rendered: 1 real pointer click, Organizations -> last navigation event: "NAVIGATED: Identity -> Organizations clicked" real pointer click, Identity row -> last navigation event: "NAVIGATED: Identity section clicked" Teams (bare Key, still unheld) -> shielded + disabled

=== PROFILE A — member holds NEITHER org key (the meshery-cloud complaint) ===
   [screenshot] 01-no-permissions-overview.png
Identity section renders a PermissionShield: true

-- tooltips (the explanation the user previously never got) --
anyOf section tooltip : Authorization RequiredNeeds any of: View All Organizations, View OrganizationsView All OrganizationsSee all organizations within a Cloud deployment. See all organizations, teams, and users.View OrganizationsSee all organizations of which you are an administrator. See all members of those organizations.Identity & Access ManagementOrganizationsKey ReferenceUserLee CalcoteOrgLayer5Role(s)MemberSeeing this message in error? Contact your Admins to request access.
   [screenshot] 03-anyOf-tooltip-lists-every-unmet-key.png
allOf button tooltip  : Authorization RequiredNeeds all of: View Organizations, Create OrganizationView OrganizationsSee all organizations of which you are an administrator. See all members of those organizations.Create OrganizationEstablish new organization for organizing teams, users, and resource access.Identity & Access ManagementOrganizationsKey ReferenceUserLee CalcoteOrgLayer5Role(s)MemberSeeing this message in error? Contact your Admins to request access.
   [screenshot] 04-allOf-tooltip.png
bare-Key tooltip      : Authorization RequiredMissing requisite keyCreate OrganizationEstablish new organization for organizing teams, users, and resource access.Identity & Access ManagementOrganizationsKey ReferenceUserLee CalcoteOrgLayer5Role(s)MemberSeeing this message in error? Contact your Admins to request access.
   [screenshot] 05-bare-key-tooltip-unchanged.png
empty-set tooltip     : Authorization RequiredMissing requisite keyKey ReferenceUserLee CalcoteOrgLayer5Role(s)MemberSeeing this message in error? Contact your Admins to request access.

-- attempt to expand / activate the unreachable section --
real pointer click at the chevron -> intercepted by the shield overlay; child items rendered: 0
programmatic click, chevron      -> child items rendered: 0
programmatic click, section row  -> last navigation event: "—"
   [screenshot] 02-no-permissions-section-cannot-expand.png

=== PROFILE B — member holds ONLY "View Organizations" (one arm of the anyOf) ===
Identity section still shielded: false
real pointer click on chevron    -> child items rendered: 1
Teams (bare Key, still unheld) shielded inside the open section: shielded + disabled
   [screenshot] 06-anyOf-satisfied-section-expands.png
real pointer click, Organizations -> last navigation event: "NAVIGATED: Identity -> Organizations clicked"
real pointer click, Identity row  -> last navigation event: "NAVIGATED: Identity section clicked"
   [screenshot] 07-anyOf-satisfied-navigates.png

=== PROFILE C — admin holds View Organizations + Create Organization ===
shields left on the 4 buttons: 1 (only the malformed { allOf: [] } stays denied)
   [screenshot] 08-admin-profile-allOf-unlocked.png

page errors: console.error: In HTML, %s cannot be a descendant of <%s>.
This will cause a hydration error.%s <li> li 

  ...
    <ClickAwayListener2 onClickAway={function handleClose}>
      <ClickAwayListener onClickAway={function handleClose}>
        <Box4 sx={{position:"...", ...}} onTouchEnd={function} onClick={function} ref={function}>
          <Styled(div) as="div" ref={function} className="MuiBox-root" theme={{...}} sx={{position:"...", ...}} ...>
            <Insertion4>
            <div className="MuiBox-roo..." onTouchEnd={function} onClick={function} ref={function}>
              <Box4 sx={{width:"100%", ...}} ref={null}>
                <Styled(div) as="div" ref={null} className="MuiBox-root" theme={{...}} sx={{width:"100%", ...}}>
                  <Insertion4>
                  <div className="MuiBox-roo...">
                    <MenuItem2 onClick={function noop6} data-testid="nav-item-i..." disabled={true}>
                      <MuiMenuItem-root ref={function} role="menuitem" tabIndex={-1} component="li" ...>
                        <Insertion4>
                        <ButtonBase2 role="menuitem" tabIndex={-1} component="li" internalNativeButton={false} ...>
                          <MuiButtonBase-root as="li" className="MuiButtonB..." ownerState={{role:"menu...", ...}} ...>
                            <Insertion4>
>                           <li
>                             className="MuiButtonBase-root Mui-disabled MuiMenuItem-root Mui-disabled MuiMenuItem-gut..."
>                             onBlur={function}
>                             onClick={function handleClick}
>                             onContextMenu={function}
>                             onFocus={function}
>                             onKeyDown={function handleKeyDown}
>                             onKeyUp={function handleKeyUp}
>                             onMouseDown={function}
>                             onMouseLeave={function}
>                             onMouseUp={function}
>                             onDragLeave={function}
>                             onTouchEnd={function}
>                             onTouchMove={function}
>                             onTouchStart={function}
>                             tabIndex={-1}
>                             role="menuitem"
>                             aria-disabled={true}
>                             data-testid="nav-item-identity"
>                             ref={function}
>                           >
                              ...
                                <ListItem2 className="css-16k2adt" ref={null}>
                                  <MuiListItem-root className="MuiListIte..." as="li" ref={null} ...>
                                    <Insertion4>
>                                   <li
>                                     className="MuiListItem-root MuiListItem-dense MuiListItem-gutters MuiListItem-pa..."
>                                   >
              ...
 | console.error: <%s> cannot contain a nested %s.
See this log for the ancestor stack trace. li <li> | console.error: In HTML, %s cannot be a descendant of <%s>.
This will cause a hydration error.%s <li> li 

  ...
    <ClickAwayListener2 onClickAway={function handleClose}>
      <ClickAwayListener onClickAway={function handleClose}>
        <Box4 sx={{position:"...", ...}} onTouchEnd={function} onClick={function} ref={function}>
          <Styled(div) as="div" ref={function} className="MuiBox-root" theme={{...}} sx={{position:"...", ...}} ...>
            <Insertion4>
            <div className="MuiBox-roo..." onTouchEnd={function} onClick={function} ref={function}>
              <Box4 sx={{width:"100%", ...}} ref={null}>
                <Styled(div) as="div" ref={null} className="MuiBox-root" theme={{...}} sx={{width:"100%", ...}}>
                  <Insertion4>
                  <div className="MuiBox-roo...">
                    <MenuItem2 onClick={function noop6} data-testid="nav-item-i..." disabled={true}>
                      <MuiMenuItem-root ref={function} role="menuitem" tabIndex={-1} component="li" ...>
                        <Insertion4>
                        <ButtonBase2 role="menuitem" tabIndex={-1} component="li" internalNativeButton={false} ...>
                          <MuiButtonBase-root as="li" className="MuiButtonB..." ownerState={{role:"menu...", ...}} ...>
                            <Insertion4>
>                           <li
>                             className="MuiButtonBase-root Mui-disabled MuiMenuItem-root Mui-disabled MuiMenuItem-gut..."
>                             onBlur={function}
>                             onClick={function handleClick}
>                             onContextMenu={function}
>                             onFocus={function}
>                             onKeyDown={function handleKeyDown}
>                             onKeyUp={function handleKeyUp}
>                             onMouseDown={function}
>                             onMouseLeave={function}
>                             onMouseUp={function}
>                             onDragLeave={function}
>                             onTouchEnd={function}
>                             onTouchMove={function}
>                             onTouchStart={function}
>                             tabIndex={-1}
>                             role="menuitem"
>                             aria-disabled={true}
>                             data-testid="nav-item-identity"
>                             ref={function}
>                           >
                              ...
                                <ListItem2 className="css-16k2adt" ref={null}>
                                  <MuiListItem-root className="MuiListIte..." as="li" ref={null} ...>
                                    <Insertion4>
>                                   <li
>                                     className="MuiListItem-root MuiListItem-dense MuiListItem-gutters MuiListItem-pa..."
>                                   >
              ...
 | console.error: <%s> cannot contain a nested %s.
See this log for the ancestor stack trace. li <li>
Evidence: Pre-fix non-vacuity check for the two nav-inertness tests
$ git checkout a57f6b42 -- src/custom/NavigationNavbar/navigationNavbar.tsx
$ npx jest src/__testing__/permissionKeySet.test.tsx -t "NavigationNavbar"
● NavigationNavbar — key sets on a section item › cannot be expanded or activated while every key is unmet
● NavigationNavbar — key sets on a section item › keeps a legacy `permission: false` section inert as well
Tests: 2 failed, 17 skipped, 3 passed, 22 total
$ git checkout HEAD -- src/custom/NavigationNavbar/navigationNavbar.tsx # restored, tree clean
Evidence: New public exports present in the built declaration + runtime bundles
$ npm run build && grep -nE "PermissionKeySet|PermissionKeySpec|isPermissionKeySet|useUnmetPermissionKeys" dist/index.d.ts
642:type PermissionKeySet = {
651:type PermissionKeySpec = Key | PermissionKeySet;
659:declare const isPermissionKeySet: (spec: PermissionKeySpec) => spec is PermissionKeySet;
740:declare const useHasPermission: (key?: PermissionKeySpec) => boolean;
750:declare const useUnmetPermissionKeys: (spec?: PermissionKeySpec) => Key[];
3333:export { ... type PermissionKeySet, type PermissionKeySpec, ... isPermissionKeySet, ... useUnmetPermissionKeys ... }

$ node -e "require('./dist/index.js')"
isPermissionKeySet: function
useUnmetPermissionKeys: function
Evidence: Evidence index + reproduction harness
# Evidence — `permissionKey` key sets (layer5io/sistent#1747)

The library is rendered straight from `src/` into a real headless Chromium
(`demo/` is the harness: `build.mjs` bundles `app.tsx` with esbuild, `shots.mjs`
drives the page with Playwright). The demo mirrors the downstream meshery-cloud
case: an **Identity** section header and an **Identity → Organizations** item
gated on `{ anyOf: [View All Organizations, View Organizations] }`, using the
real generated keys from `@meshery/schemas/permissions`.

| Artifact | Shows |
| --- | --- |
| `01-no-permissions-overview.png` | Member holding neither key: Identity section and every key-set button shielded. |
| `02-no-permissions-section-cannot-expand.png` | The unreachable section is inert — a real pointer click **and** two programmatic `MouseEvent("click")` dispatches (which bypass MUI's `pointer-events: none`) leave it collapsed with no navigation fired. |
| `03-anyOf-tooltip-lists-every-unmet-key.png` | `Needs any of: View All Organizations, View Organizations` — both keys, both descriptions, deduped category/subcategory chips. |
| `04-allOf-tooltip.png` | `Needs all of: View Organizations, Create Organization`. |
| `05-bare-key-tooltip-unchanged.png` | Bare `Key` regression: original `Missing requisite key` wording, one key. |
| `06-anyOf-satisfied-section-expands.png` | Holding only *View Organizations* satisfies the `anyOf`: the section unshields and expands on a real click. |
| `07-anyOf-satisfied-navigates.png` | Both the section row and the Organizations child navigate; the `Teams` child (bare `Key`, still unheld) stays shielded and disabled. |
| `08-admin-profile-allOf-unlocked.png` | Admin holding both keys: `allOf` unlocks; only the malformed `{ allOf: [] }` stays denied. |
| `09-walkthrough.webm` | Full recorded run of the above. |
| `browser-transcript.txt` | Console transcript of every click and the exact tooltip text read out of the live DOM. |

Reproduce: `REPO=<checkout> node demo/build.mjs && PW_PKG=<playwright> node demo/shots.mjs`

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

⚠️ **Review** - 4 infos
  • ℹ️ src/custom/permissions.tsx:198 - The tooltip now renders one copy button per unmet key but still shares a single copiedKeyId plus an unconditional setTimeout(() =&gt; setCopiedKeyId(null), 1500). Each click schedules its own timer that clears whatever is current, so a second copy is cut short by the first key's timer. Fix: setCopiedKeyId((cur) =&gt; (cur === thisId ? null : cur)) in the timeout. Newly reachable only because a set can now display multiple keys.
  • ℹ️ src/custom/PermissionProvider.tsx:50 - getPermissionKeys (line 50), resolvePermissionKeySet (line 67) and getPermissionKeyCombinator (line 80) each independently re-cast the spec to { anyOf?: unknown; allOf?: unknown }, re-apply the double-combinator rule and (in two of three) re-apply isKeyList. They agree today, but the resolver decides whether access is granted while the other two decide what the shield says, so any future edit to one rule silently desynchronises the gate from its explanation. A single normalizeKeySpec(spec) -&gt; { combinator, keys } that all three consume would make that drift impossible; both helpers are module-internal to the public API (neither is re-exported from src/index.tsx), so the refactor is contained.
  • ℹ️ src/custom/PermissionProvider.tsx:45 - isKeyList requires every member to be a truthy object, so one bad member denies the whole set — including anyOf. Concretely: { anyOf: [Keys.ViewAllOrganizations, Keys.ViewOrg] } where one constant resolved to undefined because meshery/schemas renamed it (AGENTS.md records that a patch release renamed 10 key constants with UUIDs unchanged) denies the section outright, even for a user who holds the surviving key, and the shield then shows the bare 'Missing requisite key' wording with no keys listed. This is the explicitly decided fail-closed semantic and is tested, so no change is requested — noting it because the downstream adopter (meshery-cloud) will hit it as a silent access loss, not a build error, on a schemas bump.
  • ℹ️ src/custom/permissions.tsx:85 - The component docstring still asserts "This is a pure visual component — it does NOT check permissions itself." It now calls useUnmetPermissionKeys (line 100) and reads the provider context to choose which keys to list. The inline comment at lines 136-139 states the refined contract correctly (it never overrides the caller's disabled decision, only picks what to display); the header comment above it should say the same so consumers reading the IDE tooltip aren't told the component ignores the provider.
✅ **Test** - passed

✅ No issues found.

  • npx jest src/__testing__/permissionKeySet.test.tsx — 22 tests, all passing
  • npx jest src/__testing__/permissions.test.ts src/__testing__/navigationItemTitleTypes.test.ts — adjacent permission/nav guards
  • Non-vacuity check: git checkout a57f6b42 -- src/custom/NavigationNavbar/navigationNavbar.tsx then npx jest src/__testing__/permissionKeySet.test.tsx -t &#34;NavigationNavbar&#34; — exactly the 2 inertness tests fail pre-fix; navbar restored and tree left clean
  • npm run build then grep -nE &#34;PermissionKeySet|PermissionKeySpec|isPermissionKeySet|useUnmetPermissionKeys&#34; dist/index.d.ts — all four new symbols in the declaration bundle and root export list
  • node -e &#34;require(&#39;./dist/index.js&#39;)&#34; export check — isPermissionKeySet / useUnmetPermissionKeys present at runtime
  • npx jest src/__testing__/publishedTypeSurfaceDependencies.test.ts src/__testing__/optionalPeerDependencies.test.ts — dist-backed public-surface guards against the newly added exports
  • Manual browser verification: esbuild-bundled demo of src/ driven by Playwright in headless Chromium (demo/build.mjs, demo/shots.mjs) — Identity section + Organizations item gated on { anyOf: [ViewAllOrganizations, ViewOrganizations] } using real @meshery/schemas/permissions keys, across three permission profiles
  • Browser: opened the anyOf section shield tooltip and read live DOM text — Needs any of: View All Organizations, View Organizations plus both key descriptions
  • Browser: allOf tooltip reads Needs all of: …; bare Key tooltip still reads Missing requisite key; { allOf: [] } denies and falls back to the original wording with no dangling combinator text
  • Browser: unreachable section attacked with a real pointer click on the expand chevron and two programmatic MouseEvent(&#39;click&#39;) dispatches at the chevron and the row (bypassing pointer-events: none) — 0 child items rendered, no navigation fired
  • Browser: profile holding only View Organizations unshields the section, expands it on a real click and navigates; the Teams child on a bare unheld Key stays shielded and disabled
  • find src -name &#34;*.stories.*&#34; -o -name &#34;*.mdx&#34; — confirms DangerConfirmationModal is the only stories file, so no Storybook surface documents these props
⚠️ **Document** - 1 info
  • ℹ️ AGENTS.md:108 - Out-of-scope consolidation worth a follow-up: the paragraph explaining that Key is the permission-key contract behind permissionKey on Button/IconButton/MenuItem/ListItem/ListItemButton, PermissionShield, PermissionProvider and useHasPermission, and that a silent collapse to any un-checks those props, exists in full in two places — AGENTS.md:108-110 and the header comment of src/testing/publishedTypeSurfaceDependencies.test.ts:16-21. The duplication predates this change and both copies are still accurate (PermissionKeySpec is built from Key, so the collapse consequence is unchanged), so nothing here is stale. Not touched under scope discipline; the follow-up would reduce the AGENTS.md copy to a pointer at the guard test, which the placement policy names as the owner of that detail.
⚠️ **Lint** - 1 info
  • ℹ️ src/custom/permissions.tsx:321 - prettier --check still fails on three files this change touched (src/custom/permissions.tsx, src/custom/PermissionProvider.tsx, src/base/MenuItem/MenuItem.tsx). Each deviation was verified to be present at base commit a57f6b4 by running prettier over the base blobs, so none was introduced here: an unwrapped &lt;Box sx={...}&gt;/&lt;UsersIcon&gt; block in the tooltip user-context section, two double blank lines in PermissionProvider.tsx, and organize-imports wanting to delete import React from &#39;react&#39; in MenuItem.tsx. Left unfixed deliberately: prettier is not a CI gate (AGENTS.md "Repo state that looks broken but is pre-existing"), fixing them would mass-reformat regions this change never touched, and the MenuItem React-import removal is not a safe mechanical edit. The one prettier deviation this change did introduce — named-import ordering in permissions.tsx — was fixed. eslint, the actual gate, is clean.
✅ **Push** - passed

✅ No issues found.

Summary by CodeRabbit

  • New Features
    • Added support for permission requirements using a single key or anyOf/allOf key sets.
    • Updated buttons, menus, list items, navigation, and permission shields to support combined permissions.
    • Unauthorized navigation items are now non-interactive, preventing clicks, expansion, and toggling.
    • Permission tooltips now show unmet keys and related details.
  • Bug Fixes
    • Improved handling of empty, malformed, missing, or denied permission requirements.
  • Tests
    • Added comprehensive coverage for permission evaluation, tooltips, and navigation behavior.

alexquincy and others added 2 commits August 4, 2026 05:36
`permissionKey` accepted exactly one `Key`, so an affordance legitimately
gated on more than one key could not adopt the declarative path at all:
supplying either key alone narrows access, and supplying none forfeits the
`PermissionShield` that explains the block.

It now also accepts a set with an explicit combinator —
`{ anyOf: Key[] }` (any one key suffices) or `{ allOf: Key[] }` (every key
required) — resolved by `useHasPermission` with `.some(...)` / `.every(...)`.
A bare `Key` takes the original code path untouched.

An empty, non-array, or double-combinator set is denied rather than granted:
a set is an explicit statement that the affordance IS gated, and
`[].every(...)` is vacuously true, so `{ allOf: [] }` would otherwise open
the affordance to everyone.

`PermissionShield` now lists every key the user is missing — "Needs any
of: A, B" / "Needs all of: A, B" plus a row and description per key — instead
of naming one. Single-key tooltips are unchanged.

`NavigationNavbar` makes an item the user cannot reach genuinely inert
rather than merely styled as disabled: it withholds both `onClick` and the
expand toggle, and forces the section collapsed. MUI enforces `disabled` on
a `<li>` MenuItem with `pointer-events` alone, so a disabled section could
still be toggled open and activated — which is how an unpermitted section
landed the user on a "you don't have permission" error page. A section is
reachable exactly when one of its children is, i.e. `{ anyOf: [...child
keys...] }`.

Fixes #1747

Signed-off-by: Alex Quinn <227241865+alexquincy@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds anyOf and allOf permission specifications. Permission-aware components accept the new type. Permission shields show unmet keys. Navigation blocks unauthorized clicks, expansion, and toggles.

Changes

Permission key sets

Layer / File(s) Summary
Permission resolution
src/custom/PermissionProvider.tsx, src/__testing__/permissionKeySet.test.tsx
Adds key-set types, validation, combinator resolution, unmet-key lookup, and tests for provider and denial behavior.
Permission-aware component contracts
src/base/{Button,IconButton,ListItem,ListItemButton,MenuItem}/*.tsx
Updates permissionKey props to accept single keys and anyOf or allOf specifications.
Permission shield rendering
src/custom/permissions.tsx, src/index.tsx, src/__testing__/permissionKeySet.test.tsx
Renders unmet keys with per-key copy actions, combinator text, and deduplicated labels. Re-exports the new helpers and types.
Navigation permission enforcement
src/custom/NavigationNavbar/navigationNavbar.tsx, src/__testing__/permissionKeySet.test.tsx, AGENTS.md
Suppresses unauthorized navigation, expansion, and toggle handlers. Tests cover permitted, denied, unrestricted, and legacy sections. Documentation records jsdom disabled-state limitations.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant NavigationNavbarItem
  participant useHasPermission
  participant MenuItem
  NavigationNavbarItem->>useHasPermission: resolve permissionKey
  useHasPermission-->>NavigationNavbarItem: permitted or denied
  NavigationNavbarItem->>MenuItem: pass permission metadata and handlers
  MenuItem-->>NavigationNavbarItem: click or expansion event
  NavigationNavbarItem-->>NavigationNavbarItem: execute or suppress interaction
Loading

Possibly related issues

Possibly related PRs

Suggested reviewers: rishiraj38

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: supporting anyOf and allOf permission key sets.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fm/sistent-1747-permissionkey-set

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (1)
src/custom/NavigationNavbar/navigationNavbar.tsx (1)

75-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Withhold denied-item handlers instead of assigning noop.

When isPermitted is false, MenuItem and the expand icons still receive click callbacks. Omit onClick for denied items and toggles.

  • src/custom/NavigationNavbar/navigationNavbar.tsx#L75-L76,L96,L127,L137-L139: Replace denied-path noop callbacks with omitted or undefined handlers. Remove noop when it has no remaining use.
  • AGENTS.md#L156-L159: Keep this statement only after the implementation actually withholds both handlers.
Proposed fix
-const noop = () => {};
-
-      onClick={isPermitted ? item.onClick : noop}
+      onClick={isPermitted ? item.onClick : undefined}

-        onClick={isPermitted ? item.onClick : noop}
+        onClick={isPermitted ? item.onClick : undefined}

-              <ExpandLessIcon onClick={isPermitted ? (e) => onToggle(item.id, e) : noop} />
+              <ExpandLessIcon onClick={isPermitted ? (e) => onToggle(item.id, e) : undefined} />

-              <ExpandMoreIcon onClick={isPermitted ? (e) => onToggle(item.id, e) : noop} />
+              <ExpandMoreIcon onClick={isPermitted ? (e) => onToggle(item.id, e) : undefined} />

As per coding guidelines, “For navigation items that are not permitted, withhold both onClick and the expand toggle in JavaScript.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/custom/NavigationNavbar/navigationNavbar.tsx` around lines 75 - 76, In
navigationNavbar.tsx, update the denied-item paths in MenuItem and the
expand-toggle handlers to pass omitted or undefined onClick values when
isPermitted is false, then remove the unused noop helper; in AGENTS.md, retain
the existing guidance only once both handlers are withheld in the
implementation.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/__testing__/permissionKeySet.test.tsx`:
- Line 1: Update the Key import in permissionKeySet.test.tsx to be type-only,
since it is used solely for TypeScript annotations. Preserve the existing module
source and import usage while preventing runtime resolution of the optional peer
dependency.

In `@src/custom/PermissionProvider.tsx`:
- Around line 43-45: Replace the broad object check in isKeyList with runtime
validation of every required Key field, and centralize parsing for
getPermissionKeys, resolvePermissionKeySet, and getPermissionKeyCombinator.
Require a non-empty key array, exactly one defined combinator property, and
reject malformed members before userHasPermission; add regression coverage for {
anyOf: [{} as Key] } and { anyOf: [VIEW_ORG], allOf: undefined }.

In `@src/custom/permissions.tsx`:
- Around line 152-153: Update the metadata label preparation in the permissions
rendering flow: combine the category and subcategory values from displayedKeys
into one list, call uniqueDefined once on that combined list, and use the
resulting deduplicated labels where the chips are rendered instead of
concatenating separately deduplicated categories and subcategories.
- Around line 193-213: Update the clickable Box containing KeyIcon in the
key-copy action to use semantic button or icon-button behavior with a focusable
target and an appropriate aria-label. Preserve the existing copy logic and
ensure the click handler continues to call stopPropagation; add keyboard
activation through the chosen button semantics so keyboard users can copy the
key ID.
- Around line 202-210: Replace the raw hex and RGBA color literals in the `sx`
blocks for the copied key row and the related ranges with Sistent theme exports
or semantic palette tokens. Update both the default `color` values and `&:hover`
colors while preserving the existing copied-state and hover behavior across the
affected key rows and chips.

---

Nitpick comments:
In `@src/custom/NavigationNavbar/navigationNavbar.tsx`:
- Around line 75-76: In navigationNavbar.tsx, update the denied-item paths in
MenuItem and the expand-toggle handlers to pass omitted or undefined onClick
values when isPermitted is false, then remove the unused noop helper; in
AGENTS.md, retain the existing guidance only once both handlers are withheld in
the implementation.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 811bf8c8-3017-4c72-ab26-3f94c7f26ef8

📥 Commits

Reviewing files that changed from the base of the PR and between a57f6b4 and f306539.

📒 Files selected for processing (11)
  • AGENTS.md
  • src/__testing__/permissionKeySet.test.tsx
  • src/base/Button/Button.tsx
  • src/base/IconButton/IconButton.tsx
  • src/base/ListItem/ListItem.tsx
  • src/base/ListItemButton/ListItemButton.tsx
  • src/base/MenuItem/MenuItem.tsx
  • src/custom/NavigationNavbar/navigationNavbar.tsx
  • src/custom/PermissionProvider.tsx
  • src/custom/permissions.tsx
  • src/index.tsx

@@ -0,0 +1,277 @@
import { Key } from '@meshery/schemas/permissions';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 2 \
  -g 'package.json' \
  -g 'tsconfig*.json' \
  '"`@meshery/schemas`"|verbatimModuleSyntax|importsNotUsedAsValues|preserveValueImports' . || true

Repository: layer5io/sistent

Length of output: 643


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== permission test file =="
wc -l src/__testing__/permissionKeySet.test.tsx
sed -n '1,180p' src/__testing__/permissionKeySet.test.tsx

echo
echo "== package.json relevant sections =="
sed -n '1,180p' package.json

echo
echo "== tsconfig files =="
fd -a 'tsconfig.*\.json$' . | sed 's#^\./##' | while read -r f; do
  echo "--- $f"
  sed -n '1,220p' "$f"
done

echo
echo "== Key usages across repo =="
rg -n "import [^{]*\{[^}]*Key|import type.*Key|from ['\"]`@meshery/schemas/permissions`['\"]" .

Repository: layer5io/sistent

Length of output: 14402


Make the Key import type-only.

Key is only used for TypeScript annotations in this test file. Use import type { Key } from '@meshery/schemas/permissions' so import-preserving builds do not resolve the optional peer dependency at runtime.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/__testing__/permissionKeySet.test.tsx` at line 1, Update the Key import
in permissionKeySet.test.tsx to be type-only, since it is used solely for
TypeScript annotations. Preserve the existing module source and import usage
while preventing runtime resolution of the optional peer dependency.

Source: Coding guidelines

Comment on lines +43 to +45
/** A non-empty array of key-shaped values. */
const isKeyList = (value: unknown): value is Key[] =>
Array.isArray(value) && value.length > 0 && value.every((k) => !!k && typeof k === 'object');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Reject malformed permission key sets before host evaluation.

isKeyList accepts any non-null object as a Key, so malformed members can be passed to userHasPermission. Also, the current parser treats { anyOf: [...], allOf: undefined } as a valid anyOf because it checks undefined instead of requiring exactly one combinator property. Use one parser for getPermissionKeys, resolvePermissionKeySet, and getPermissionKeyCombinator that requires a non-empty key array, one combinator property, and valid runtime Key fields; add regression cases for { anyOf: [{} as Key] } and { anyOf: [VIEW_ORG], allOf: undefined }.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/custom/PermissionProvider.tsx` around lines 43 - 45, Replace the broad
object check in isKeyList with runtime validation of every required Key field,
and centralize parsing for getPermissionKeys, resolvePermissionKeySet, and
getPermissionKeyCombinator. Require a non-empty key array, exactly one defined
combinator property, and reject malformed members before userHasPermission; add
regression coverage for { anyOf: [{} as Key] } and { anyOf: [VIEW_ORG], allOf:
undefined }.

Comment on lines +152 to +153
const categories = uniqueDefined(displayedKeys.map((key) => key.category));
const subcategories = uniqueDefined(displayedKeys.map((key) => key.subcategory));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Deduplicate metadata labels after combining them.

Lines 152-153 deduplicate categories and subcategories separately. Line 259 concatenates both arrays, so the same label appears twice when it occurs in both fields.

Build one combined value list, then call uniqueDefined once before rendering the chips.

Proposed fix
-  const categories = uniqueDefined(displayedKeys.map((key) => key.category));
-  const subcategories = uniqueDefined(displayedKeys.map((key) => key.subcategory));
+  const metadataLabels = uniqueDefined([
+    ...displayedKeys.map((key) => key.category),
+    ...displayedKeys.map((key) => key.subcategory)
+  ]);
...
-          {[...categories, ...subcategories].map((label, index) => (
+          {metadataLabels.map((label, index) => (

Also applies to: 259-271

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/custom/permissions.tsx` around lines 152 - 153, Update the metadata label
preparation in the permissions rendering flow: combine the category and
subcategory values from displayedKeys into one list, call uniqueDefined once on
that combined list, and use the resulting deduplicated labels where the chips
are rendered instead of concatenating separately deduplicated categories and
subcategories.

Comment on lines +193 to +213
<Tooltip title={copied ? 'Copied!' : 'Copy key ID to clipboard'} placement="top">
<Box
component="span"
onClick={(e: React.MouseEvent) => {
e.stopPropagation();
navigator.clipboard.writeText(key.id || '');
setCopiedKeyId(key.id || `#${index}`);
setTimeout(() => setCopiedKeyId(null), 1500);
}}
sx={{
display: 'inline-flex',
cursor: 'pointer',
color: copied ? '#EBC024' : 'rgba(255, 255, 255, 0.7)',
transition: 'color 0.2s ease',
'&:hover': {
color: '#EBC024'
}
}}
>
<KeyIcon sx={{ fontSize: '1rem' }} />
</Box>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Make each key-copy action keyboard-operable.

The new Box at Line 194 is a clickable span. It has no button semantics, focus target, accessible name, or keyboard handler. Keyboard-only users cannot copy a permission key ID.

Use a semantic button or an accessible icon button with an aria-label. Keep the click propagation stop in the new handler.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/custom/permissions.tsx` around lines 193 - 213, Update the clickable Box
containing KeyIcon in the key-copy action to use semantic button or icon-button
behavior with a focusable target and an appropriate aria-label. Preserve the
existing copy logic and ensure the click handler continues to call
stopPropagation; add keyboard activation through the chosen button semantics so
keyboard users can copy the key ID.

Comment on lines +202 to +210
sx={{
display: 'inline-flex',
cursor: 'pointer',
color: copied ? '#EBC024' : 'rgba(255, 255, 255, 0.7)',
transition: 'color 0.2s ease',
'&:hover': {
color: '#EBC024'
}
}}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Replace new raw colors with Sistent semantic tokens.

These new key rows and chips use literal hex and RGBA colors. The values will not follow the active Sistent theme. Use Sistent theme exports and semantic palette tokens in these sx values.

As per coding guidelines, “Theme-aware UI must use Sistent theme exports and semantic palette tokens rather than raw MUI defaults.”

Also applies to: 215-238, 264-269

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/custom/permissions.tsx` around lines 202 - 210, Replace the raw hex and
RGBA color literals in the `sx` blocks for the copied key row and the related
ranges with Sistent theme exports or semantic palette tokens. Update both the
default `color` values and `&:hover` colors while preserving the existing
copied-state and hover behavior across the affected key rows and chips.

Source: Coding guidelines

@sarajkrishnasingh
sarajkrishnasingh merged commit d29007c into master Aug 4, 2026
6 checks passed
@sarajkrishnasingh
sarajkrishnasingh deleted the fm/sistent-1747-permissionkey-set branch August 4, 2026 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

permissionKey cannot express an OR of keys, so multi-key affordances cannot adopt PermissionShield

4 participants