From 6d219d9652f7a30823d618b7876be790173517de Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 14:37:04 +0000 Subject: [PATCH] fix(verify,plugin-security,cli): bootStack honours the app-declared default permission set (#7001) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 两条启动路径对「应用声明的默认权限集是否存在」给出了不同答案: `objectstack serve` 会读取 `config.permissions` 中标记 `isDefault: true` 的权限集, 并作为 SecurityPlugin 的 `fallbackPermissionSet` 传入;而 `@objectstack/verify` 的 `bootStack` 直接构造了一个 vanilla `new SecurityPlugin()`,从不读取 `config.permissions`。于是应用声明的 profile 在真人执行 CLI 时生效,在该应用自己的 测试套件启动时却静默缺席 —— 这正是「declared ≠ enforced」,而且发生在专门用来捕捉 这类偏差的测试载体内部:测试全绿,生产行为却不同。 #5491 之前这一点不可见:平台的 `member_default` 带有 `object_permissions['*']` 通配符,没有任何应用 profile 的成员照样能访问所有对象,fallback 从来不承重。#5491 有意移除了这层地板,其 Migration 章节给出的唯一消费者动作 —— 通过 `isDefault: true` 提供应用默认 profile —— 恰恰是 `bootStack` 无法表达的。 解析逻辑现在只有一处,两条路径都调用它:`appSecurityPluginOptions(config)`,新增于 `@objectstack/plugin-security`,与既有的 `appDefaultPermissionSetName` 并列。它回答 启动方真正的问题 —— 「这份 config 该给 SecurityPlugin 构造函数传什么」—— 而不只是 名字,因为后半截 `name ? { fallbackPermissionSet: name } : undefined` 是一个决策而非 格式选择:serve.ts 曾把它写死在原地,而 bootStack 压根没长出来过。serve.ts 一并收敛 到同一个 helper,两条路径从此按构造一致,而不是靠各自记得。 行为变化仅限 `@objectstack/verify`:对声明了 `isDefault` 权限集的应用, `bootStack(config)` 现在以该 profile 作为每请求可加性基线(ADR-0090 D5),与 `objectstack dev` 一致;未声明的应用完全不受影响(解析返回 `undefined`,插件继续从 内置集推导 `member_default`)。刻意需要平台原生基线的套件现在显式表达: `bootStack(config, { security: new SecurityPlugin() })`;`opts.security` 传入的实例 整体胜出,永不被合并改写。 反向验证(两个方向都按预测): - 还原 serve.ts 的原地写法 → parity 契约测试 3 红,verify 自身 6 绿(除该契约外, 仓库里没有任何东西盯着 serve 这一侧)。 - 还原 harness.ts → parity 2 红 + verify 2 红。该消融还暴露出扫描本身的弱点: 未使用的 import 让 `toContain('appSecurityPluginOptions')` 保持绿,故断言改为 测量构造式而非字符串。 实测影响面:dogfood 86 个文件 / 524 个用例中,仅 1 条断言移动 —— `me-apps-and-everyone-baseline`,其文件头本就写着「Deliberately VANILLA」。该依赖 真实存在但此前只由 harness 默认值静默表达,现在写进参数里。#5491 时已手工搭建 `test/showcase-security.ts` 来补这个洞的 showcase 夹具不受影响。 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01F8q5J1MQyocgtNspb15fSn --- ...fy-bootstack-app-default-permission-set.md | 63 ++++++ ...ve-verify-security-parity.contract.test.ts | 180 ++++++++++++++++ packages/cli/src/commands/serve.ts | 22 +- .../src/app-default-permission-set.test.ts | 77 ++++++- .../src/app-default-permission-set.ts | 41 ++++ packages/plugins/plugin-security/src/index.ts | 2 +- ...apps-and-everyone-baseline.dogfood.test.ts | 12 +- .../src/harness.app-default-profile.test.ts | 197 ++++++++++++++++++ packages/verify/src/harness.ts | 33 ++- 9 files changed, 613 insertions(+), 14 deletions(-) create mode 100644 .changeset/verify-bootstack-app-default-permission-set.md create mode 100644 packages/cli/src/commands/serve-verify-security-parity.contract.test.ts create mode 100644 packages/verify/src/harness.app-default-profile.test.ts diff --git a/.changeset/verify-bootstack-app-default-permission-set.md b/.changeset/verify-bootstack-app-default-permission-set.md new file mode 100644 index 0000000000..c5501908e4 --- /dev/null +++ b/.changeset/verify-bootstack-app-default-permission-set.md @@ -0,0 +1,63 @@ +--- +"@objectstack/plugin-security": minor +"@objectstack/verify": minor +"@objectstack/cli": patch +--- + +fix(verify,plugin-security,cli): `bootStack` honours the app-declared default permission set, like `serve` always did (#7001) + +Two boot paths disagreed about whether an application's declared default +permission profile exists. + +- **`objectstack serve` honoured it** — it read the permission set marked + `isDefault: true` off `config.permissions` and passed the name as the + `SecurityPlugin` `fallbackPermissionSet`. +- **`bootStack` did not** — `@objectstack/verify` constructed a vanilla + `new SecurityPlugin()` and never read `config.permissions` at all. + +So the profile an app declares was in force when a human ran the CLI and +silently absent when the app's own suite booted it: a `declared ≠ enforced` +split inside the harness that exists to catch that split. Green tests, +different production behaviour. + +It was invisible until #5491. Until then the platform's `member_default` +carried an `object_permissions['*']` wildcard, so a member with no application +profile reached every object anyway and the declared fallback was never +load-bearing. #5491 removed that floor deliberately and its Migration section +prescribes exactly one consumer action — ship an app default profile via +`isDefault: true` — which `bootStack` had no way to express. Measured in +cloud's `ee-group-showcase`, adding the prescribed profile changed nothing: the +same acceptance cases still failed at the object gate. + +**What changed.** The resolution now lives in one place and both boot paths call +it: `appSecurityPluginOptions(config)`, new in `@objectstack/plugin-security` +next to the existing `appDefaultPermissionSetName`. It answers the question a +booter actually has — *what do I hand the `SecurityPlugin` constructor for this +config* — rather than just the name, because the second half +(`name ? { fallbackPermissionSet: name } : undefined`) is a decision, not +formatting, and while `serve.ts` had open-coded it, `bootStack` had simply never +grown one. `serve.ts` is converged onto the same helper, so the two now agree by +construction rather than by each caller remembering. + +**Behavioural change, `@objectstack/verify` only.** `bootStack(config)` on an +app that declares an `isDefault` permission set now boots with that profile as +the additive per-request baseline (ADR-0090 D5), matching `objectstack dev`. An +app that declares no such set is unaffected — the resolution yields `undefined` +and the plugin keeps deriving `member_default` from its built-in sets, exactly +as before. + +A suite that deliberately wants the platform's own baseline over an app that +declares a default now says so: `bootStack(config, { security: new SecurityPlugin() })`. +A plugin passed in `opts.security` still wins whole and is never merged into — +it arrives carrying its own constructor options, and silently rewriting one of +them would be a worse surprise than the bug being fixed. + +Measured blast radius across the framework's own suites: of 86 dogfood files and +524 tests, exactly one assertion moved — `me-apps-and-everyone-baseline`, which +asserts the bootstrap binds `member_default` to the `everyone` anchor and whose +header already read "Deliberately VANILLA". That dependence was real but silent, +expressed only by the harness default; it is now stated in the argument. The +showcase fixtures that needed the app profile were already hand-wiring a +`SecurityPlugin` for it (`test/showcase-security.ts`, added by #5491) — the +"custom security code" these dogfood apps exist to prove unnecessary — and are +unchanged by this release. diff --git a/packages/cli/src/commands/serve-verify-security-parity.contract.test.ts b/packages/cli/src/commands/serve-verify-security-parity.contract.test.ts new file mode 100644 index 0000000000..59b45b2fef --- /dev/null +++ b/packages/cli/src/commands/serve-verify-security-parity.contract.test.ts @@ -0,0 +1,180 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// `objectstack serve` ↔ `@objectstack/verify`'s `bootStack`: the two boot paths +// must construct their `SecurityPlugin` from the SAME resolution — #7001. +// +// The defect this file mechanises: two boot paths disagreed about whether an +// application's declared default permission profile exists. +// +// • `serve.ts` read `appDefaultPermissionSetName(config.permissions)` and +// passed it as `fallbackPermissionSet`. +// • `bootStack` constructed a vanilla `new SecurityPlugin()` and never read +// `config.permissions` at all. +// +// So the profile an app declared was in force when a human ran the CLI and +// silently absent when the app's own dogfood suite booted it — a +// `declared ≠ enforced` split inside the harness that exists to catch that +// split. Green tests, different production behaviour. It stayed invisible until +// #5491 removed `member_default`'s `'*'` wildcard, because until then the floor +// underneath granted everything anyway and the fallback was never load-bearing. +// +// The runtime halves are pinned where they run: the harness's wiring and its +// behavioural consequence in `packages/verify/src/harness.app-default-profile.test.ts`, +// the helper's own contract in +// `packages/plugins/plugin-security/src/app-default-permission-set.test.ts`. +// Neither can see THIS file's failure mode, which is the one that actually +// happened: nothing in the repo pins `serve.ts`'s side, so re-open-coding the +// wiring here — or dropping it — would be green everywhere while the paths +// separate again. Hence a source scan, in the shape of this package's +// `serve-email-config-parity.contract.test.ts`: the grep that would have caught +// it, mechanised, so the second divergence fails a build instead of waiting for +// someone to run it. +// +// It is deliberately a scan of BOTH files rather than an assertion about one. +// A one-sided pin is satisfiable by editing the other side, which is exactly +// how two mirrored literals drift. + +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { appSecurityPluginOptions, appDefaultPermissionSetName } from '@objectstack/plugin-security'; + +const HERE = path.dirname(fileURLToPath(import.meta.url)); + +/** + * `packages/cli/src/commands/` → `packages/`. The sibling read is what makes + * this a PARITY assertion instead of a single-file lint; `@objectstack/verify` + * is a real dependency of this package, and the comparison is test-only, so no + * runtime edge is added. Tests never ship (`files: ["dist"]`). + */ +const PACKAGES_DIR = path.resolve(HERE, '../../..'); + +/** + * Absence must be loud (AGENTS.md, Route & surface ownership §3). A scan that + * silently reports success because it could not find the file it scans is worse + * than no scan — it is this very gate's failure mode, one level up. + */ +function readBootPath(relative: string): string { + const full = path.join(PACKAGES_DIR, relative); + try { + return readFileSync(full, 'utf8'); + } catch (e) { + throw new Error( + `serve↔verify parity scan cannot read its subject '${relative}' (looked at ${full}). ` + + 'The file moved or was renamed — repoint this scan; do NOT delete it, the two boot ' + + `paths still have to agree. (${(e as Error).message})`, + ); + } +} + +/** + * Comments stripped, because this scan is about what the two files DO. + * + * Both boot sites are heavily commented — with the very construction shapes + * being asserted about, since each explains what it replaced — so a scan over + * raw text measures the prose and reports on it. (It did: the first run of this + * file counted six constructions where the code has two.) Worse, the + * comment-inclusive form would forbid the next author from ever *describing* + * the old wiring, which is the opposite of what these files need. + * + * Approximate by design, and safe here: the result feeds nothing but the + * `new SecurityPlugin(...)` regex below, so a `//` mangled out of a string + * literal (`'http://localhost:3000'` in `harness.ts`) cannot affect a verdict. + * Do not reuse this for anything that reads string contents. + */ +function stripComments(source: string): string { + return source.replace(/\/\*[\s\S]*?\*\//g, ' ').replace(/(^|[^:])\/\/[^\n]*/g, '$1'); +} + +const BOOT_PATHS: Array<{ label: string; relative: string; source: string }> = [ + { label: 'objectstack serve', relative: 'cli/src/commands/serve.ts' }, + { label: 'verify bootStack', relative: 'verify/src/harness.ts' }, +].map((p) => ({ ...p, source: stripComments(readBootPath(p.relative)) })); + +/** + * Every `new SecurityPlugin(...)` construction in a file, with its argument. + * + * Walks parentheses rather than matching `\(([^)]*)\)` — the argument is itself + * a call (`appSecurityPluginOptions(config)`), so a non-nesting match stops at + * the INNER `)` and silently reports `appSecurityPluginOptions(config`. That + * truncation compares equal across both files, so the naive form would have + * passed while measuring something that is not the argument. + */ +function securityPluginConstructions(source: string): string[] { + const NEW = 'new SecurityPlugin('; + const found: string[] = []; + for (let i = source.indexOf(NEW); i !== -1; i = source.indexOf(NEW, i + 1)) { + let depth = 1; + let j = i + NEW.length; + for (; j < source.length && depth > 0; j++) { + if (source[j] === '(') depth++; + else if (source[j] === ')') depth--; + } + if (depth !== 0) throw new Error(`unbalanced \`${NEW}…\` at offset ${i} — the scan cannot read this file`); + found.push(source.slice(i + NEW.length, j - 1).trim()); + } + return found; +} + +describe('serve ↔ bootStack construct SecurityPlugin from one resolution (#7001)', () => { + // Asserting on the CONSTRUCTION rather than on the file's text, because the + // text form does not go red on the defect. Reverting `harness.ts` to its + // pre-#7001 `new SecurityPlugin()` left the `appSecurityPluginOptions` import + // standing, so a `expect(source).toContain('appSecurityPluginOptions')` check + // stayed GREEN over a boot path that had stopped calling it — a mention is + // not a call. Measured, not reasoned: that ablation was run, and this is the + // shape that failed correctly. + it.each(BOOT_PATHS)('$label constructs SecurityPlugin exactly once, via the helper', ({ source }) => { + expect(securityPluginConstructions(source)).toEqual(['appSecurityPluginOptions(config)']); + }); + + it.each(BOOT_PATHS)('$label does not re-open-code the resolution', ({ source }) => { + // The open-coded shape #7001 replaced, in either spelling. Reaching for the + // NAME helper at a boot site means rebuilding `name ? {...} : undefined` by + // hand — the half that was a decision, not formatting, and the half the + // other path never grew. + expect(source).not.toContain('appDefaultPermissionSetName'); + expect(source).not.toMatch(/fallbackPermissionSet\s*:/); + }); + + it('and the two paths agree with EACH OTHER, not merely with a literal', () => { + // The parity claim proper. The per-path assertions above both compare to + // the same hard-coded string, which a single careless edit could "fix" on + // both sides at once; this one compares the paths to one another, so + // divergence is red however the argument is spelled. + const [serve, verify] = BOOT_PATHS.map(({ source }) => securityPluginConstructions(source)); + // A path that stopped constructing one at all would otherwise satisfy any + // "the two agree" claim over two empty sets. + expect(serve.length, 'serve constructs a SecurityPlugin').toBeGreaterThan(0); + expect(verify).toEqual(serve); + }); +}); + +describe('the shared resolution, exercised (#7001)', () => { + // The scan above proves both paths call one helper; these prove the helper + // they call answers correctly. Neither claim implies the other, and a source + // scan alone would be green over a helper that returned nonsense. + const declared = { + permissions: [ + { name: 'ignored_not_default', isDefault: false }, + { name: 'app_member_default', isDefault: true }, + ], + }; + + it('carries an app-declared isDefault profile into the constructor options', () => { + expect(appSecurityPluginOptions(declared)).toEqual({ fallbackPermissionSet: 'app_member_default' }); + expect(appDefaultPermissionSetName(declared.permissions)).toBe('app_member_default'); + }); + + it('yields undefined when nothing is declared, so the plugin keeps its own derivation', () => { + // Deliberately NOT `{ fallbackPermissionSet: undefined }`: the constructor + // reads an explicit `undefined` as "derive from the built-in sets" only + // because the KEY is absent — `fallbackPermissionSet: null` means "no + // baseline at all". Passing the object shape would work today and is one + // refactor away from silently disabling the platform baseline. + expect(appSecurityPluginOptions({ permissions: [{ name: 'plain' }] })).toBeUndefined(); + expect(appSecurityPluginOptions({})).toBeUndefined(); + expect(appSecurityPluginOptions(undefined)).toBeUndefined(); + }); +}); diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index 724a9bd8ee..bb6f575c6b 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -2003,14 +2003,22 @@ export default class Serve extends Command { // Pair: SecurityPlugin (RBAC) — optional try { const securityPkg = '@objectstack/plugin-security'; - const { SecurityPlugin, appDefaultPermissionSetName } = await import(/* webpackIgnore: true */ securityPkg); + const { SecurityPlugin, appSecurityPluginOptions } = await import(/* webpackIgnore: true */ securityPkg); // ADR-0056 D7 — honor an app-declared default profile. A stack - // permission set marked `isDefault` becomes the - // fallback for users with no explicit grants. The SecurityPlugin's - // own scan only sees its built-in sets, so the CLI passes the - // declared name through explicitly (undefined → built-in default). - const appDefaultProfile = appDefaultPermissionSetName((config as any)?.permissions); - await kernel.use(new SecurityPlugin(appDefaultProfile ? { fallbackPermissionSet: appDefaultProfile } : undefined)); + // permission set marked `isDefault` becomes the baseline for + // users with no explicit grants. The SecurityPlugin's own scan + // only sees its built-in sets, so the declared name is passed + // through explicitly (undefined → built-in default). + // + // [#7001] Resolved through the SHARED helper rather than + // open-coded here. This was the only boot path that did it at + // all: `@objectstack/verify`'s `bootStack` constructed a vanilla + // `new SecurityPlugin()`, so an app's own dogfood suite ran + // against a boot without the profile the CLI gave its users. The + // two now agree by construction — one helper, one call shape, and + // `serve-verify-security-parity.contract.test.ts` fails if either + // side open-codes its way back out. + await kernel.use(new SecurityPlugin(appSecurityPluginOptions(config))); trackPlugin('Security'); } catch { // optional diff --git a/packages/plugins/plugin-security/src/app-default-permission-set.test.ts b/packages/plugins/plugin-security/src/app-default-permission-set.test.ts index c38c7610cd..e626ee53ae 100644 --- a/packages/plugins/plugin-security/src/app-default-permission-set.test.ts +++ b/packages/plugins/plugin-security/src/app-default-permission-set.test.ts @@ -1,6 +1,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect } from 'vitest'; -import { appDefaultPermissionSetName } from './app-default-permission-set'; +import { appDefaultPermissionSetName, appSecurityPluginOptions } from './app-default-permission-set'; +import { SecurityPlugin } from './security-plugin'; describe('appDefaultPermissionSetName (ADR-0090 D5)', () => { it('returns the name of the first isDefault permission set', () => { @@ -25,3 +26,77 @@ describe('appDefaultPermissionSetName (ADR-0090 D5)', () => { ).toBe('ok'); }); }); + +/** + * [#7001] `appSecurityPluginOptions` — the whole constructor argument, so every + * boot path spells the wiring once. + * + * `appDefaultPermissionSetName` above answers "which profile did the app + * declare". That left the second half — turning a name into constructor options + * — open-coded at each call site, and only ONE site ever had it: `objectstack + * serve`. `@objectstack/verify`'s `bootStack` built a vanilla + * `new SecurityPlugin()`, so an app's own suite ran against a boot without the + * profile the CLI gave its users. + */ +describe('appSecurityPluginOptions (#7001)', () => { + it('reads the declared default off a stack config', () => { + expect( + appSecurityPluginOptions({ + permissions: [{ name: 'read_only' }, { name: 'app_member_default', isDefault: true }], + }), + ).toEqual({ fallbackPermissionSet: 'app_member_default' }); + }); + + it('returns undefined — NOT { fallbackPermissionSet: undefined } — when nothing is declared', () => { + // The distinction is load-bearing, not stylistic. The constructor reads an + // ABSENT key as "derive my own default from the built-in sets" and an + // explicit `null` as "no baseline at all"; returning the object shape works + // today only because `undefined` happens to hit the same branch, and is one + // refactor away from silently disabling the platform baseline. + for (const config of [{ permissions: [{ name: 'plain' }] }, { permissions: [] }, {}, null, undefined, 'nonsense']) { + expect(appSecurityPluginOptions(config)).toBeUndefined(); + } + }); + + it('reads `permissions` top-level, exactly where serve.ts has always read it', () => { + // Being cleverer here (also looking inside `manifest`) would re-open the + // #7001 gap in the other direction: the harness would honour a declaration + // the CLI ignores, and a suite would again prove something production does + // not do. + expect(appSecurityPluginOptions({ manifest: { permissions: [{ name: 'buried', isDefault: true }] } })) + .toBeUndefined(); + }); +}); + +/** + * The options do not merely describe the wiring — they land on the plugin. A + * fake `PluginContext` captures what `init()` publishes as the + * `security.fallbackPermissionSet` service, which is the value the runtime + * resolves every authenticated request's additive baseline from (ADR-0090 D5). + */ +describe('the resolved options reach the constructed plugin (#7001)', () => { + const initAndReadBaseline = async (plugin: SecurityPlugin): Promise => { + const services = new Map(); + await plugin.init({ + logger: { info() {}, warn() {}, error() {}, debug() {} }, + registerService: (name: string, value: unknown) => services.set(name, value), + getService: (name: string) => { + if (name === 'manifest') return { register() {} }; + return undefined; + }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any); + return services.get('security.fallbackPermissionSet'); + }; + + it('an app-declared default becomes the plugin baseline', async () => { + const config = { permissions: [{ name: 'app_member_default', isDefault: true }] }; + await expect(initAndReadBaseline(new SecurityPlugin(appSecurityPluginOptions(config)))) + .resolves.toBe('app_member_default'); + }); + + it('and no declaration leaves the built-in member_default standing', async () => { + await expect(initAndReadBaseline(new SecurityPlugin(appSecurityPluginOptions({})))) + .resolves.toBe('member_default'); + }); +}); diff --git a/packages/plugins/plugin-security/src/app-default-permission-set.ts b/packages/plugins/plugin-security/src/app-default-permission-set.ts index 23c03c56ac..f2a6161f40 100644 --- a/packages/plugins/plugin-security/src/app-default-permission-set.ts +++ b/packages/plugins/plugin-security/src/app-default-permission-set.ts @@ -25,3 +25,44 @@ export function appDefaultPermissionSetName(permissions: unknown): string | unde } return undefined; } + +/** + * [#7001] The `SecurityPlugin` options a stack config implies — ONE resolution + * for EVERY boot path. + * + * `appDefaultPermissionSetName` above answers "which profile did the app + * declare"; this answers the question every booter actually has: "what do I + * hand the `SecurityPlugin` constructor for this config". The difference + * sounds cosmetic and is not — the second half (`name ? { fallbackPermissionSet: + * name } : undefined`) is a decision, not a formatting choice, and while it was + * open-coded at the one call site that had it, the other boot path simply never + * grew one: + * + * • `objectstack serve` honoured an app's `isDefault` profile. + * • `@objectstack/verify`'s `bootStack` constructed a vanilla + * `new SecurityPlugin()` and never read `config.permissions`. + * + * So an app could declare a profile, ship it to users through the CLI, and have + * every one of its own tests run against a boot that did not include it. That + * stayed invisible until #5491 removed `member_default`'s `'*'` wildcard: before + * it, the floor underneath granted everything anyway, so the fallback was never + * load-bearing. #5491's Migration section prescribes shipping an `isDefault` + * profile — a prescription `bootStack` had no way to express. + * + * Returning `undefined` (rather than `{ fallbackPermissionSet: undefined }`) is + * deliberate: it lets the constructor apply its OWN derivation from the + * built-in sets, which is not the same thing as being told "no fallback". Pass + * the result straight through — `new SecurityPlugin(appSecurityPluginOptions(config))` + * — and a caller cannot get the undefined case subtly wrong. + * + * Reads `config.permissions`, top-level, exactly as `serve.ts` always has. + * Being cleverer here (also looking inside `manifest`) would re-open the gap it + * closes, in the other direction. + */ +export function appSecurityPluginOptions( + config: unknown, +): { fallbackPermissionSet: string } | undefined { + const permissions = (config as { permissions?: unknown } | null | undefined)?.permissions; + const name = appDefaultPermissionSetName(permissions); + return name ? { fallbackPermissionSet: name } : undefined; +} diff --git a/packages/plugins/plugin-security/src/index.ts b/packages/plugins/plugin-security/src/index.ts index e4b097b51e..8d6efd3e17 100644 --- a/packages/plugins/plugin-security/src/index.ts +++ b/packages/plugins/plugin-security/src/index.ts @@ -64,7 +64,7 @@ export { objectPostureGate, registerObjectPostureGate } from './object-posture-g export type { ObjectPostureGateContext } from './object-posture-gate.js'; export { claimSeedOwnership } from './claim-seed-ownership.js'; export { normalizeManagedByVocab } from './normalize-managed-by.js'; -export { appDefaultPermissionSetName } from './app-default-permission-set.js'; +export { appDefaultPermissionSetName, appSecurityPluginOptions } from './app-default-permission-set.js'; export { DelegatedAdminGate, isTenantAdmin } from './delegated-admin-gate.js'; export { assertEngineOwnedWriteAllowed, ENGINE_OWNED_BUCKETS } from './system-write-guard.js'; export type { EngineOwnedSchemaLike } from './system-write-guard.js'; diff --git a/packages/qa/dogfood/test/me-apps-and-everyone-baseline.dogfood.test.ts b/packages/qa/dogfood/test/me-apps-and-everyone-baseline.dogfood.test.ts index e2e7542c6c..87f760a9ff 100644 --- a/packages/qa/dogfood/test/me-apps-and-everyone-baseline.dogfood.test.ts +++ b/packages/qa/dogfood/test/me-apps-and-everyone-baseline.dogfood.test.ts @@ -31,6 +31,7 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import showcaseStack from '@objectstack/example-showcase'; import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { SecurityPlugin } from '@objectstack/plugin-security'; const SYS = { isSystem: true } as const; @@ -46,7 +47,16 @@ describe('ADR-0090 D5 closures: /me/apps + anchor-bindable baseline', () => { // THAT set to the `everyone` anchor. The #5491 consequence — the baseline no // longer grants objects — is handled where it bites, by declaring the probe's // create grant instead of inheriting it from a wildcard (see the delete case). - stack = await bootStack(showcaseStack); + // + // [#7001] SAID OUT LOUD, in the argument, because it is a real dependency of + // this file and not a background fact. It used to be silent: `bootStack` + // built a vanilla `new SecurityPlugin()` for everyone, so this suite got the + // platform baseline by DEFAULT while `objectstack dev` gave the same + // showcase its own declared `showcase_member_default` — the boot-path + // asymmetry #7001 closed. The harness now honours an app's declared default, + // so the file that genuinely wants the platform's own baseline asks for it. + // Nothing about the claim below changed; only who is saying it. + stack = await bootStack(showcaseStack, { security: new SecurityPlugin() }); adminTok = await stack.signIn(); memberTok = await stack.signUp('baseline-member@verify.test'); ql = await stack.kernel.getServiceAsync('objectql'); diff --git a/packages/verify/src/harness.app-default-profile.test.ts b/packages/verify/src/harness.app-default-profile.test.ts new file mode 100644 index 0000000000..3cb9d87ad4 --- /dev/null +++ b/packages/verify/src/harness.app-default-profile.test.ts @@ -0,0 +1,197 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#7001] `bootStack` must wire an app's DECLARED default permission set — the +// same profile `objectstack serve` wires for the same config. +// +// The asymmetry this file pins, measured on `origin/main` before the fix: +// +// • `objectstack serve` (packages/cli/src/commands/serve.ts) reads +// `appDefaultPermissionSetName(config.permissions)` and passes it as the +// SecurityPlugin `fallbackPermissionSet`. +// • `bootStack` constructed a vanilla `new SecurityPlugin()` and never read +// `config.permissions` at all. +// +// So the profile an app declares was in force when a human ran the CLI and +// silently absent when the app's own suite booted it — a `declared ≠ enforced` +// split INSIDE the harness that exists to catch that split. +// +// It was invisible until #5491 (`9e9445ba`): the platform's `member_default` +// carried an `object_permissions['*']` wildcard, so a member with no +// application profile reached every object anyway and the declared fallback was +// never load-bearing. #5491 removed that floor deliberately and prescribed +// exactly one consumer action — ship an app default profile via +// `isDefault: true` — which `bootStack` had no way to express. +// +// The behavioural case below is the honest red: with the wildcard gone, the +// built-in baseline grants NO object access, so a fresh member reading the +// fixture object is denied unless the app's declared profile is actually wired. +// A name-only assertion could not tell "wired" from "wired but inert". + +import { describe, it, expect, afterAll } from 'vitest'; +import { defineStack } from '@objectstack/spec'; +import { ObjectSchema, Field } from '@objectstack/spec/data'; +import { PermissionSetSchema } from '@objectstack/spec/security'; +import { + SecurityPlugin, + appDefaultPermissionSetName, + appSecurityPluginOptions, +} from '@objectstack/plugin-security'; +// `.js` extension deliberate: under `moduleResolution: NodeNext` the +// extensionless form does not resolve, so every symbol it names silently +// becomes `any` (AGENTS.md §Build & Test). This package's older test files +// still carry that shape as measured TEST_DEBT — a shrink-only ratchet — so a +// new file must not add to it. +import { bootStack, type VerifyStack } from './harness.js'; + +// Booting the full in-process stack runs well past vitest's 5s default. +const BOOT_TIMEOUT = 120_000; + +const APP_DEFAULT_SET = 'appdefault_member_default'; + +// `defineStack` enforces `${manifest.namespace}_${shortName}` on every object +// name, so the two fixtures below need one memo object each. +const memo = (namespace: string) => + ObjectSchema.create({ + name: `${namespace}_memo`, + // [ADR-0090 D1] grandfather stamp: the gate under test is the BASELINE + // profile the harness wires, not owner-sharing. `public_read_write` keeps + // the OWD out of the way so the only thing that can deny the read is the + // permission set — which is exactly the variable this file measures. + sharingModel: 'public_read_write', + label: 'Memo', + pluralLabel: 'Memos', + fields: { + name: Field.text({ label: 'Name', required: true }), + }, + }); + +/** + * The app's declared default profile. Deliberately low-privilege: an + * `isDefault` set is the `everyone` baseline suggestion (ADR-0090 D5) and the + * D7 anchor gate refuses high-privilege bits on one. + */ +const AppDefaultProfile = PermissionSetSchema.parse({ + name: APP_DEFAULT_SET, + label: 'App Member (Default)', + isDefault: true, + objects: { + appdefault_memo: { allowRead: true, allowCreate: true }, + }, +}); + +/** Declares an `isDefault` profile — the #5491 migration the CLI already honours. */ +const appWithDeclaredDefault = defineStack({ + manifest: { + id: 'com.example.app-default-profile', + namespace: 'appdefault', + version: '0.0.1', + type: 'app', + name: 'App Default Profile Fixture', + }, + objects: [memo('appdefault')], + permissions: [AppDefaultProfile], +}); + +/** Declares NO default profile — the shape the vast majority of apps still have. */ +const appWithoutDeclaredDefault = defineStack({ + manifest: { + id: 'com.example.no-default-profile', + namespace: 'nodefault', + version: '0.0.1', + type: 'app', + name: 'No Default Profile Fixture', + }, + objects: [memo('nodefault')], +}); + +const started: VerifyStack[] = []; +const boot = async (...args: Parameters): Promise => { + const stack = await bootStack(...args); + started.push(stack); + return stack; +}; + +afterAll(async () => { + for (const stack of started) await stack.stop().catch(() => undefined); +}); + +/** The baseline profile a booted kernel actually runs with. */ +const wiredBaseline = (stack: VerifyStack): Promise => + stack.kernel.getServiceAsync('security.fallbackPermissionSet'); + +describe('bootStack honours the app-declared default permission set (#7001)', () => { + it( + 'wires the SAME profile `serve` wires for the same config', + async () => { + const stack = await boot(appWithDeclaredDefault); + + // The exact expression `serve.ts` evaluates for its `fallbackPermissionSet`. + const servesChoice = appDefaultPermissionSetName(appWithDeclaredDefault.permissions); + expect(servesChoice, 'fixture precondition: the config declares an isDefault set') + .toBe(APP_DEFAULT_SET); + + // RED before the fix: 'member_default' — the vanilla constructor's + // derivation from the BUILT-IN sets, which cannot see the app's. + await expect(wiredBaseline(stack)).resolves.toBe(servesChoice); + }, + BOOT_TIMEOUT, + ); + + it( + 'and that profile is load-bearing: a fresh member holds the declared grants', + async () => { + const stack = await boot(appWithDeclaredDefault); + await stack.signIn(); // first user is the seeded dev admin + const memberToken = await stack.signUp('appdefault-member@verify.test'); + + // RED before the fix: the built-in baseline grants no object access since + // #5491 removed its `'*'` wildcard, so this was a denial. + const read = await stack.apiAs(memberToken, 'GET', '/data/appdefault_memo'); + expect(read.status, 'the declared default grants memo read').toBe(200); + }, + BOOT_TIMEOUT, + ); + + it( + 'leaves an app that declares no default on the built-in baseline', + async () => { + const stack = await boot(appWithoutDeclaredDefault); + + expect(appDefaultPermissionSetName(appWithoutDeclaredDefault.permissions)).toBeUndefined(); + // Unchanged by this fix — the resolution yields `undefined`, so the + // SecurityPlugin keeps deriving its own default from the built-in sets. + await expect(wiredBaseline(stack)).resolves.toBe('member_default'); + }, + BOOT_TIMEOUT, + ); + + it( + 'lets a suite opt OUT explicitly, by passing its own SecurityPlugin', + async () => { + // The vanilla baseline is still reachable — it is now the EXPLICIT + // choice rather than the silent default. A caller-supplied plugin wins + // whole: it already carries its own constructor options, and quietly + // rewriting one of them would be a second, worse surprise. + const stack = await boot(appWithDeclaredDefault, { security: new SecurityPlugin() }); + await expect(wiredBaseline(stack)).resolves.toBe('member_default'); + }, + BOOT_TIMEOUT, + ); +}); + +describe('the resolution helper is shared, not mirrored (#7001)', () => { + // Both boot paths call `appSecurityPluginOptions`; a second copy of + // `x ? { fallbackPermissionSet: x } : undefined` is how they drifted apart + // the first time. The source-level pin that BOTH callers still route through + // it lives in `packages/cli/src/commands/serve-verify-security-parity.contract.test.ts` + // — this pair only fixes the helper's own contract. + it('derives the plugin options from a config the same way the name helper does', () => { + expect(appSecurityPluginOptions(appWithDeclaredDefault)).toEqual({ + fallbackPermissionSet: appDefaultPermissionSetName(appWithDeclaredDefault.permissions), + }); + }); + + it('yields undefined for a config with no declared default (built-in derivation kept)', () => { + expect(appSecurityPluginOptions(appWithoutDeclaredDefault)).toBeUndefined(); + }); +}); diff --git a/packages/verify/src/harness.ts b/packages/verify/src/harness.ts index 0249d446e0..98bd61341e 100644 --- a/packages/verify/src/harness.ts +++ b/packages/verify/src/harness.ts @@ -25,7 +25,7 @@ import { ObjectQLPlugin } from '@objectstack/objectql'; import { HonoServerPlugin } from '@objectstack/plugin-hono-server'; import { createRestApiPlugin } from '@objectstack/rest'; import { AuthPlugin } from '@objectstack/plugin-auth'; -import { SecurityPlugin } from '@objectstack/plugin-security'; +import { SecurityPlugin, appSecurityPluginOptions } from '@objectstack/plugin-security'; import { SharingServicePlugin } from '@objectstack/plugin-sharing'; import { SettingsServicePlugin, LocalCryptoProvider } from '@objectstack/service-settings'; import { AnalyticsServicePlugin } from '@objectstack/service-analytics'; @@ -103,8 +103,22 @@ export interface BootOptions { * Override the SecurityPlugin instance. Pass a `new SecurityPlugin({...})` * to carry a custom `fallbackPermissionSet` / extra permission sets — this * is how an owner-isolated RLS fixture makes a fresh member fall back to a - * permission set that carries `RLS.ownerPolicy(...)` instead of the broad-read - * `member_default`. Defaults to a vanilla `new SecurityPlugin()`. + * permission set that carries `RLS.ownerPolicy(...)` instead of the + * platform's `member_default`. + * + * **Default (since #7001): the app's own declared default profile** — + * `new SecurityPlugin(appSecurityPluginOptions(config))`, i.e. the permission + * set the config marks `isDefault: true`, wired exactly as `objectstack + * serve` wires it. A config declaring no such set is unaffected: the + * resolution yields `undefined` and the plugin keeps deriving its own default + * (`member_default`) from the built-in sets. + * + * A plugin passed here wins WHOLE — the harness never merges the app's + * declared default into it. An instance arrives carrying its own constructor + * options, and silently rewriting one of them would be a second, worse + * surprise than the one #7001 fixed. So this is also the explicit opt-out: + * a suite that deliberately wants the vanilla platform baseline over an app + * that declares a default asks for it with `security: new SecurityPlugin()`. */ security?: SecurityPlugin; /** @@ -416,7 +430,18 @@ export async function bootStack( await kernel.use(plugin as any); } - await kernel.use(opts.security ?? new SecurityPlugin()); + // [#7001] The app's DECLARED default profile, resolved the one way every boot + // path resolves it. Character-for-character what `objectstack serve` does + // (`packages/cli/src/commands/serve.ts`) — the same helper, the same argument + // — because the two disagreeing was the defect: an app could declare a + // profile, ship it to users through the CLI, and have every one of its own + // tests run against a boot that did not include it. A harness whose context + // differs from the seam it verifies reports green on a difference in + // production behaviour, which is the one thing it exists not to do. + // + // `opts.security` still wins whole — see BootOptions.security for why a + // caller-supplied plugin is never partially rewritten. + await kernel.use(opts.security ?? new SecurityPlugin(appSecurityPluginOptions(config))); // Sharing service — apps that declare `requires: ['sharing']` rely on it for // record-share grants; without it their RLS/sharing rules are inert and the // verifier would under-report authorization.