Skip to content

App nav item visible (CEL) is served to the client but never evaluated — a silently inert gate (17.2.0) #15135

Description

@baozhoutao

Summary

An app navigation item's visible CEL predicate is not evaluated by the Console shell. The predicate is authored, validated, persisted, and served to the browser intact — and then ignored. The entry renders for every user, including the ones the predicate excludes.

This is the exact failure shape ADR-0049 / ADR-0078 legislate against ("a gate that validates and then silently does nothing is an authoring trap"), and the spec's own docblocks assert the opposite behaviour, so an author has no way to discover it short of measuring.

Environment

  • @objectstack/* 17.2.0 (runtime, spec, rest, plugin-security, plugin-sharing), Console UI as shipped by objectstack dev
  • Node 22, better-sqlite3 driver, single-tenant, NODE_ENV=development

What the contract says

spec/src/ui/app.zod.ts, on the nav-item surface:

visible: ExpressionInputSchema.optional()
  .describe('Visibility predicate (CEL). e.g. P`\'org_admin\' in current_user.positions`'),

and AREA_VISIBLE_RETIRED (the message shown when someone authors the retired area-level key) prescribes the item-level key as the working replacement:

"…gate the items INSIDE the area — a navigation ITEM's visible takes the same CEL expression and IS evaluated per item by the shell."

rest/dist/index.js (filterAppForUserWithReason) is consistent with that division of labour and says so:

"NOT gated here: visible (CEL) at any level, and requiresObject — both are still evaluated client-side only."

So the server deliberately forwards visible, on the stated understanding that the shell applies it. The shell does not.

Minimal reproduction

  1. Author an app with one nav group and one leaf item, both carrying an item-level visible predicate that is FALSE for the signed-in user:
import { P } from '@objectstack/spec';
import { App } from '@objectstack/spec/ui';

export const DemoApp = App.create({
  name: 'demo_app',
  label: 'Demo',
  navigation: [
    { id: 'nav_home', type: 'page', pageName: 'demo_home', label: 'Home' },
    {
      id: 'group_admin', type: 'group', label: 'Admin Only',
      visible: P`!('demo_member' in current_user.positions)`,
      children: [
        { id: 'nav_secret', type: 'object', objectName: 'demo_thing', label: 'Secret',
          visible: P`!('demo_member' in current_user.positions)` },
      ],
    },
  ],
});
  1. Sign in as a user who holds the demo_member position (so both predicates evaluate to false).
  2. Observe the sidebar.

Observed

Both the group and the leaf render. GET /api/v1/meta/app/demo_app returns them with the predicate attached and uncompiled:

group_admin group visible={"dialect":"cel","source":"!('demo_member' in current_user.positions)"}
  nav_secret object visible={"dialect":"cel","source":"!('demo_member' in current_user.positions)"}

Measured in a real app with six positions: the predicate was false for the signed-in user on both a group and a leaf, and the sidebar was byte-identical to a user for whom it was true — same six top-level entries, same children after expanding every group.

Expected

One of the two, either is fine — what is not fine is the current silence:

  • the shell evaluates item-level visible against the session (what the spec's own text promises); or
  • visible is retired from the nav-item surface the way areas[].visible already was in 17.0.0, with the same kind of hard error naming requiredPermissions as the replacement.

Impact / workaround

requiredPermissions does work, and is a strictly better gate for anything security-adjacent — it is applied server-side in filterAppForUser, so the entry never reaches the browser. We switched to it and got correct per-role menus.

Two sharp edges worth documenting wherever visible is (or is not) fixed, both hit while migrating:

  1. requiredPermissions is AND over exact strings (req.every(p => sysPerms.has(p))) — no wildcard, and no superuser bypass. A platform admin holding admin_full_access (manage_metadata, manage_platform_settings, studio.access, …) is pruned from an entry gated on an app-defined capability just like an ordinary member. That is defensible, but it is surprising enough that it cost us a debugging cycle: the admin silently lost two menu groups. If that is intended, the docblock next to the every() is the place to say so.
  2. Because the gate is AND-only, an app cannot express "admins OR this capability" declaratively; it has to grant the capability to the admins as data.

No behaviour change is being requested urgently on our side — we are unblocked via requiredPermissions. Filing because a silently-inert authoring key is the category this project has repeatedly chosen to eliminate rather than document.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions