|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | +// |
| 3 | +// #16746 — the Connect-an-Agent page must be reachable by the principal the |
| 4 | +// page's own promise is about. |
| 5 | +// |
| 6 | +// `POST /api/v1/keys` mints a `sys_api_key` bound to the CALLER and the page |
| 7 | +// says the key "acts as you". Reached only through Setup it could not keep that |
| 8 | +// promise: `SETUP_APP` declares `requiredPermissions: ['setup.access']`, so |
| 9 | +// every non-admin following the shipped two-step guide — and the runtime's own |
| 10 | +// error text (`plugin.ts`, `README.md`: "mint a key in Setup → Connect an |
| 11 | +// Agent") — stopped at step 1 while the endpoint behind the button accepted |
| 12 | +// them all along. Maintainer ruling 2026-09-08, option A: open the key card to |
| 13 | +// every signed-in user; backend, authorization and the published promise do |
| 14 | +// not move. |
| 15 | +// |
| 16 | +// --------------------------------------------------------------------------- |
| 17 | +// What this file pins, and what it deliberately does NOT claim |
| 18 | +// --------------------------------------------------------------------------- |
| 19 | +// ⚠️ The acceptance criterion is a WIRE fact about two apps — a permissionless |
| 20 | +// principal sees `nav_connect_agent` in the `account` app's nav while |
| 21 | +// `GET /api/v1/meta/apps/setup` keeps answering 403 PERMISSION_DENIED — and |
| 22 | +// that fact cannot be measured from this package: `@objectstack/mcp` declares |
| 23 | +// no dependency on `@objectstack/rest` (the RBAC-by-route harness), on |
| 24 | +// `@objectstack/objectql` (`SchemaRegistry.applyNavContributions`, the fold) or |
| 25 | +// on `@objectstack/platform-objects` (`SETUP_APP` / `ACCOUNT_APP`). ⛔ So this |
| 26 | +// file does not reimplement any of them — a second copy of the fold is exactly |
| 27 | +// the divergence `cli/src/utils/nav-contribution-groups.ts` refuses to write. |
| 28 | +// |
| 29 | +// What it pins instead is the half this package OWNS, stated as the two |
| 30 | +// properties the wire fact rests on: |
| 31 | +// |
| 32 | +// 1. the contribution is aimed at the ungated app and group, and carries |
| 33 | +// nothing the server-side nav filter could strip for a permissionless |
| 34 | +// caller (`requiredPermissions` / `requiresService`); |
| 35 | +// 2. this bundle cannot widen Setup — the accident that would make a |
| 36 | +// "the entry is visible" assertion pass for the wrong reason. |
| 37 | +// |
| 38 | +// The second is the load-bearing one. Ungating Setup was measured on the real |
| 39 | +// composition and refused: the app-level `setup.access` gate fires BEFORE the |
| 40 | +// group gate (so dropping `group_integrations`' gate alone changes nothing), |
| 41 | +// and dropping both serves 14+ unrelated Setup surfaces to every signed-in |
| 42 | +// user. Nothing in this bundle may reintroduce that, so the walk below asserts |
| 43 | +// the bundle declares no permission key anywhere and no `apps` collection that |
| 44 | +// could redeclare `SETUP_APP` without its gate. |
| 45 | + |
| 46 | +import { describe, it, expect } from 'vitest'; |
| 47 | +import { NavigationContributionSchema } from '@objectstack/spec/ui'; |
| 48 | + |
| 49 | +import { CONNECT_AGENT_UI_BUNDLE } from './connect-ui.js'; |
| 50 | + |
| 51 | +type AnyRec = Record<string, unknown>; |
| 52 | + |
| 53 | +const contributions = CONNECT_AGENT_UI_BUNDLE.navigationContributions as AnyRec[]; |
| 54 | +const byApp = (app: string): AnyRec[] => contributions.filter((c) => c.app === app); |
| 55 | +const itemsOf = (c: AnyRec): AnyRec[] => (c.items ?? []) as AnyRec[]; |
| 56 | + |
| 57 | +/** Every key present anywhere in a value, depth-first — objects and arrays. */ |
| 58 | +function everyKey(value: unknown, out: string[] = []): string[] { |
| 59 | + if (Array.isArray(value)) { |
| 60 | + for (const entry of value) everyKey(entry, out); |
| 61 | + } else if (value && typeof value === 'object') { |
| 62 | + for (const [key, child] of Object.entries(value)) { |
| 63 | + out.push(key); |
| 64 | + everyKey(child, out); |
| 65 | + } |
| 66 | + } |
| 67 | + return out; |
| 68 | +} |
| 69 | + |
| 70 | +describe('#16746 — Connect an Agent reaches the per-user Account app', () => { |
| 71 | + it('contributes into the `account` app, into the group that already ships API Keys', () => { |
| 72 | + const account = byApp('account'); |
| 73 | + expect(account).toHaveLength(1); |
| 74 | + // `ACCOUNT_APP` declares no `requiredPermissions` (deliberately: every |
| 75 | + // authenticated user must reach their own security surface, RLS scopes the |
| 76 | + // rows), and `grp_account_developer` is the group already carrying |
| 77 | + // `nav_account_api_keys`. Both are read by NAME here — this card edits |
| 78 | + // nothing in `@objectstack/platform-objects`. |
| 79 | + expect(account[0]).toMatchObject({ app: 'account', group: 'grp_account_developer' }); |
| 80 | + }); |
| 81 | + |
| 82 | + it('aims one `page` item at `connect_agent`, the page this same bundle registers', () => { |
| 83 | + const items = itemsOf(byApp('account')[0]); |
| 84 | + expect(items).toHaveLength(1); |
| 85 | + expect(items[0]).toMatchObject({ |
| 86 | + id: 'nav_connect_agent', |
| 87 | + type: 'page', |
| 88 | + pageName: 'connect_agent', |
| 89 | + }); |
| 90 | + // The destination must be a page this bundle actually ships, or the entry |
| 91 | + // is a 404-when-clicked shown to every signed-in user — the precise defect |
| 92 | + // that ruled out gating an `account.app.ts` entry on `requiresService: |
| 93 | + // 'mcp'` (the service registers unconditionally in `init()`; this bundle |
| 94 | + // registers behind `isMcpServerEnabled()`). |
| 95 | + const pageNames = (CONNECT_AGENT_UI_BUNDLE.pages ?? []).map((p) => p.name); |
| 96 | + expect(pageNames).toContain(items[0].pageName); |
| 97 | + }); |
| 98 | + |
| 99 | + it('carries nothing the per-user caller could be filtered on', () => { |
| 100 | + // THE criterion, stated at the layer this package owns. The server-side |
| 101 | + // nav filter strips an item for a caller missing its `requiredPermissions` |
| 102 | + // and for an absent `requiresService` capability. An entry carrying either |
| 103 | + // would be invisible to exactly the principal this card is about — a |
| 104 | + // permissionless one — while every assertion above still passed. |
| 105 | + const item = itemsOf(byApp('account')[0])[0]; |
| 106 | + expect(item).not.toHaveProperty('requiredPermissions'); |
| 107 | + expect(item).not.toHaveProperty('requiresService'); |
| 108 | + expect(item).not.toHaveProperty('visible'); |
| 109 | + }); |
| 110 | + |
| 111 | + it('leaves the Setup entry exactly as it was — admins keep the page where the guide points', () => { |
| 112 | + const setup = byApp('setup'); |
| 113 | + expect(setup).toHaveLength(1); |
| 114 | + expect(setup[0]).toMatchObject({ app: 'setup', group: 'group_integrations', priority: 110 }); |
| 115 | + expect(itemsOf(setup[0])).toEqual([ |
| 116 | + { id: 'nav_connect_agent', type: 'page', pageName: 'connect_agent', label: 'Connect an Agent', icon: 'bot' }, |
| 117 | + ]); |
| 118 | + }); |
| 119 | + |
| 120 | + it('⛔ cannot widen Setup — no permission key and no app redeclaration anywhere in the bundle', () => { |
| 121 | + // The risk the green has to prove, not merely pass. A test asserting only |
| 122 | + // "the account entry exists" would go green just as happily on a diff that |
| 123 | + // reached the card by ungating Setup instead — measured to serve 14+ |
| 124 | + // unrelated Setup surfaces (Users, Organization, Branding, Feature Flags, |
| 125 | + // …) to every signed-in user. This bundle is the one file that changed, so |
| 126 | + // it is where that accident would have to be written. |
| 127 | + const keys = new Set(everyKey(JSON.parse(JSON.stringify(CONNECT_AGENT_UI_BUNDLE)))); |
| 128 | + expect([...keys].filter((k) => k === 'requiredPermissions')).toEqual([]); |
| 129 | + // An `apps: [...]` collection here could redeclare `SETUP_APP` — last |
| 130 | + // registration wins — and drop its `setup.access` gate without touching |
| 131 | + // `platform-objects` at all. |
| 132 | + expect(CONNECT_AGENT_UI_BUNDLE).not.toHaveProperty('apps'); |
| 133 | + // Every contribution aims at a named group. A contribution with no `group` |
| 134 | + // appends at the app's TOP level, which for Setup would put the entry |
| 135 | + // outside `group_integrations`' gate. |
| 136 | + for (const c of contributions) expect(typeof c.group).toBe('string'); |
| 137 | + }); |
| 138 | + |
| 139 | + it('shares the item id across the two apps, and the fold says that is scoped per app', () => { |
| 140 | + // Answered from the fold rather than from taste, because a wrong answer |
| 141 | + // here is a silent one. `SchemaRegistry` keys contributions by TARGET APP |
| 142 | + // (`appNavContributions: Map<string, …>`) and `applyNavContributions(app)` |
| 143 | + // consults only `get(app.name)`, so a nav item id is unique within ONE |
| 144 | + // app's navigation tree; nothing indexes it across apps (no id-keyed |
| 145 | + // registry, no de-duplication by id), and the translation bundles are |
| 146 | + // keyed `apps.<app>.navigation.<id>`, which makes one shared id two |
| 147 | + // distinct keys. One destination therefore keeps one identity. |
| 148 | + const targets = contributions.map((c) => c.app); |
| 149 | + expect(new Set(targets).size).toBe(targets.length); |
| 150 | + expect(contributions.flatMap((c) => itemsOf(c).map((i) => i.id))).toEqual([ |
| 151 | + 'nav_connect_agent', |
| 152 | + 'nav_connect_agent', |
| 153 | + ]); |
| 154 | + // ⛔ …and they are two literals, not one shared const. The fold |
| 155 | + // `structuredClone`s the APP but pushes `...c.items` BY REFERENCE, so one |
| 156 | + // shared object would sit in two apps' navigation trees at once and any |
| 157 | + // in-place consumer edit would leak from one app into the other. |
| 158 | + expect(itemsOf(byApp('setup')[0])[0]).not.toBe(itemsOf(byApp('account')[0])[0]); |
| 159 | + }); |
| 160 | + |
| 161 | + it('both contributions parse against the real spec contract', () => { |
| 162 | + // The shared id is not merely unenforced — it is accepted by the schema |
| 163 | + // that governs the surface, checked against the spec rather than asserted |
| 164 | + // about it. |
| 165 | + for (const c of contributions) { |
| 166 | + const parsed = NavigationContributionSchema.safeParse(c); |
| 167 | + expect(parsed.error?.issues ?? []).toEqual([]); |
| 168 | + expect(parsed.success).toBe(true); |
| 169 | + } |
| 170 | + }); |
| 171 | +}); |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?v=4&size=40){kind=avatar}
0 commit comments