Skip to content

Commit d19fb5c

Browse files
fix(verify,plugin-security,cli): bootStack honours the app-declared default permission set (#7001) (#7091)
两条启动路径对「应用声明的默认权限集是否存在」给出了不同答案: `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 夹具不受影响。 Claude-Session: https://claude.ai/code/session_01F8q5J1MQyocgtNspb15fSn Co-authored-by: Claude <noreply@anthropic.com>
1 parent ac244ad commit d19fb5c

9 files changed

Lines changed: 613 additions & 14 deletions
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
---
2+
"@objectstack/plugin-security": minor
3+
"@objectstack/verify": minor
4+
"@objectstack/cli": patch
5+
---
6+
7+
fix(verify,plugin-security,cli): `bootStack` honours the app-declared default permission set, like `serve` always did (#7001)
8+
9+
Two boot paths disagreed about whether an application's declared default
10+
permission profile exists.
11+
12+
- **`objectstack serve` honoured it** — it read the permission set marked
13+
`isDefault: true` off `config.permissions` and passed the name as the
14+
`SecurityPlugin` `fallbackPermissionSet`.
15+
- **`bootStack` did not**`@objectstack/verify` constructed a vanilla
16+
`new SecurityPlugin()` and never read `config.permissions` at all.
17+
18+
So the profile an app declares was in force when a human ran the CLI and
19+
silently absent when the app's own suite booted it: a `declared ≠ enforced`
20+
split inside the harness that exists to catch that split. Green tests,
21+
different production behaviour.
22+
23+
It was invisible until #5491. Until then the platform's `member_default`
24+
carried an `object_permissions['*']` wildcard, so a member with no application
25+
profile reached every object anyway and the declared fallback was never
26+
load-bearing. #5491 removed that floor deliberately and its Migration section
27+
prescribes exactly one consumer action — ship an app default profile via
28+
`isDefault: true` — which `bootStack` had no way to express. Measured in
29+
cloud's `ee-group-showcase`, adding the prescribed profile changed nothing: the
30+
same acceptance cases still failed at the object gate.
31+
32+
**What changed.** The resolution now lives in one place and both boot paths call
33+
it: `appSecurityPluginOptions(config)`, new in `@objectstack/plugin-security`
34+
next to the existing `appDefaultPermissionSetName`. It answers the question a
35+
booter actually has — *what do I hand the `SecurityPlugin` constructor for this
36+
config* — rather than just the name, because the second half
37+
(`name ? { fallbackPermissionSet: name } : undefined`) is a decision, not
38+
formatting, and while `serve.ts` had open-coded it, `bootStack` had simply never
39+
grown one. `serve.ts` is converged onto the same helper, so the two now agree by
40+
construction rather than by each caller remembering.
41+
42+
**Behavioural change, `@objectstack/verify` only.** `bootStack(config)` on an
43+
app that declares an `isDefault` permission set now boots with that profile as
44+
the additive per-request baseline (ADR-0090 D5), matching `objectstack dev`. An
45+
app that declares no such set is unaffected — the resolution yields `undefined`
46+
and the plugin keeps deriving `member_default` from its built-in sets, exactly
47+
as before.
48+
49+
A suite that deliberately wants the platform's own baseline over an app that
50+
declares a default now says so: `bootStack(config, { security: new SecurityPlugin() })`.
51+
A plugin passed in `opts.security` still wins whole and is never merged into —
52+
it arrives carrying its own constructor options, and silently rewriting one of
53+
them would be a worse surprise than the bug being fixed.
54+
55+
Measured blast radius across the framework's own suites: of 86 dogfood files and
56+
524 tests, exactly one assertion moved — `me-apps-and-everyone-baseline`, which
57+
asserts the bootstrap binds `member_default` to the `everyone` anchor and whose
58+
header already read "Deliberately VANILLA". That dependence was real but silent,
59+
expressed only by the harness default; it is now stated in the argument. The
60+
showcase fixtures that needed the app profile were already hand-wiring a
61+
`SecurityPlugin` for it (`test/showcase-security.ts`, added by #5491) — the
62+
"custom security code" these dogfood apps exist to prove unnecessary — and are
63+
unchanged by this release.
Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// `objectstack serve` ↔ `@objectstack/verify`'s `bootStack`: the two boot paths
4+
// must construct their `SecurityPlugin` from the SAME resolution — #7001.
5+
//
6+
// The defect this file mechanises: two boot paths disagreed about whether an
7+
// application's declared default permission profile exists.
8+
//
9+
// • `serve.ts` read `appDefaultPermissionSetName(config.permissions)` and
10+
// passed it as `fallbackPermissionSet`.
11+
// • `bootStack` constructed a vanilla `new SecurityPlugin()` and never read
12+
// `config.permissions` at all.
13+
//
14+
// So the profile an app declared was in force when a human ran the CLI and
15+
// silently absent when the app's own dogfood suite booted it — a
16+
// `declared ≠ enforced` split inside the harness that exists to catch that
17+
// split. Green tests, different production behaviour. It stayed invisible until
18+
// #5491 removed `member_default`'s `'*'` wildcard, because until then the floor
19+
// underneath granted everything anyway and the fallback was never load-bearing.
20+
//
21+
// The runtime halves are pinned where they run: the harness's wiring and its
22+
// behavioural consequence in `packages/verify/src/harness.app-default-profile.test.ts`,
23+
// the helper's own contract in
24+
// `packages/plugins/plugin-security/src/app-default-permission-set.test.ts`.
25+
// Neither can see THIS file's failure mode, which is the one that actually
26+
// happened: nothing in the repo pins `serve.ts`'s side, so re-open-coding the
27+
// wiring here — or dropping it — would be green everywhere while the paths
28+
// separate again. Hence a source scan, in the shape of this package's
29+
// `serve-email-config-parity.contract.test.ts`: the grep that would have caught
30+
// it, mechanised, so the second divergence fails a build instead of waiting for
31+
// someone to run it.
32+
//
33+
// It is deliberately a scan of BOTH files rather than an assertion about one.
34+
// A one-sided pin is satisfiable by editing the other side, which is exactly
35+
// how two mirrored literals drift.
36+
37+
import { describe, it, expect } from 'vitest';
38+
import { readFileSync } from 'node:fs';
39+
import path from 'node:path';
40+
import { fileURLToPath } from 'node:url';
41+
import { appSecurityPluginOptions, appDefaultPermissionSetName } from '@objectstack/plugin-security';
42+
43+
const HERE = path.dirname(fileURLToPath(import.meta.url));
44+
45+
/**
46+
* `packages/cli/src/commands/` → `packages/`. The sibling read is what makes
47+
* this a PARITY assertion instead of a single-file lint; `@objectstack/verify`
48+
* is a real dependency of this package, and the comparison is test-only, so no
49+
* runtime edge is added. Tests never ship (`files: ["dist"]`).
50+
*/
51+
const PACKAGES_DIR = path.resolve(HERE, '../../..');
52+
53+
/**
54+
* Absence must be loud (AGENTS.md, Route & surface ownership §3). A scan that
55+
* silently reports success because it could not find the file it scans is worse
56+
* than no scan — it is this very gate's failure mode, one level up.
57+
*/
58+
function readBootPath(relative: string): string {
59+
const full = path.join(PACKAGES_DIR, relative);
60+
try {
61+
return readFileSync(full, 'utf8');
62+
} catch (e) {
63+
throw new Error(
64+
`serve↔verify parity scan cannot read its subject '${relative}' (looked at ${full}). ` +
65+
'The file moved or was renamed — repoint this scan; do NOT delete it, the two boot ' +
66+
`paths still have to agree. (${(e as Error).message})`,
67+
);
68+
}
69+
}
70+
71+
/**
72+
* Comments stripped, because this scan is about what the two files DO.
73+
*
74+
* Both boot sites are heavily commented — with the very construction shapes
75+
* being asserted about, since each explains what it replaced — so a scan over
76+
* raw text measures the prose and reports on it. (It did: the first run of this
77+
* file counted six constructions where the code has two.) Worse, the
78+
* comment-inclusive form would forbid the next author from ever *describing*
79+
* the old wiring, which is the opposite of what these files need.
80+
*
81+
* Approximate by design, and safe here: the result feeds nothing but the
82+
* `new SecurityPlugin(...)` regex below, so a `//` mangled out of a string
83+
* literal (`'http://localhost:3000'` in `harness.ts`) cannot affect a verdict.
84+
* Do not reuse this for anything that reads string contents.
85+
*/
86+
function stripComments(source: string): string {
87+
return source.replace(/\/\*[\s\S]*?\*\//g, ' ').replace(/(^|[^:])\/\/[^\n]*/g, '$1');
88+
}
89+
90+
const BOOT_PATHS: Array<{ label: string; relative: string; source: string }> = [
91+
{ label: 'objectstack serve', relative: 'cli/src/commands/serve.ts' },
92+
{ label: 'verify bootStack', relative: 'verify/src/harness.ts' },
93+
].map((p) => ({ ...p, source: stripComments(readBootPath(p.relative)) }));
94+
95+
/**
96+
* Every `new SecurityPlugin(...)` construction in a file, with its argument.
97+
*
98+
* Walks parentheses rather than matching `\(([^)]*)\)` — the argument is itself
99+
* a call (`appSecurityPluginOptions(config)`), so a non-nesting match stops at
100+
* the INNER `)` and silently reports `appSecurityPluginOptions(config`. That
101+
* truncation compares equal across both files, so the naive form would have
102+
* passed while measuring something that is not the argument.
103+
*/
104+
function securityPluginConstructions(source: string): string[] {
105+
const NEW = 'new SecurityPlugin(';
106+
const found: string[] = [];
107+
for (let i = source.indexOf(NEW); i !== -1; i = source.indexOf(NEW, i + 1)) {
108+
let depth = 1;
109+
let j = i + NEW.length;
110+
for (; j < source.length && depth > 0; j++) {
111+
if (source[j] === '(') depth++;
112+
else if (source[j] === ')') depth--;
113+
}
114+
if (depth !== 0) throw new Error(`unbalanced \`${NEW}…\` at offset ${i} — the scan cannot read this file`);
115+
found.push(source.slice(i + NEW.length, j - 1).trim());
116+
}
117+
return found;
118+
}
119+
120+
describe('serve ↔ bootStack construct SecurityPlugin from one resolution (#7001)', () => {
121+
// Asserting on the CONSTRUCTION rather than on the file's text, because the
122+
// text form does not go red on the defect. Reverting `harness.ts` to its
123+
// pre-#7001 `new SecurityPlugin()` left the `appSecurityPluginOptions` import
124+
// standing, so a `expect(source).toContain('appSecurityPluginOptions')` check
125+
// stayed GREEN over a boot path that had stopped calling it — a mention is
126+
// not a call. Measured, not reasoned: that ablation was run, and this is the
127+
// shape that failed correctly.
128+
it.each(BOOT_PATHS)('$label constructs SecurityPlugin exactly once, via the helper', ({ source }) => {
129+
expect(securityPluginConstructions(source)).toEqual(['appSecurityPluginOptions(config)']);
130+
});
131+
132+
it.each(BOOT_PATHS)('$label does not re-open-code the resolution', ({ source }) => {
133+
// The open-coded shape #7001 replaced, in either spelling. Reaching for the
134+
// NAME helper at a boot site means rebuilding `name ? {...} : undefined` by
135+
// hand — the half that was a decision, not formatting, and the half the
136+
// other path never grew.
137+
expect(source).not.toContain('appDefaultPermissionSetName');
138+
expect(source).not.toMatch(/fallbackPermissionSet\s*:/);
139+
});
140+
141+
it('and the two paths agree with EACH OTHER, not merely with a literal', () => {
142+
// The parity claim proper. The per-path assertions above both compare to
143+
// the same hard-coded string, which a single careless edit could "fix" on
144+
// both sides at once; this one compares the paths to one another, so
145+
// divergence is red however the argument is spelled.
146+
const [serve, verify] = BOOT_PATHS.map(({ source }) => securityPluginConstructions(source));
147+
// A path that stopped constructing one at all would otherwise satisfy any
148+
// "the two agree" claim over two empty sets.
149+
expect(serve.length, 'serve constructs a SecurityPlugin').toBeGreaterThan(0);
150+
expect(verify).toEqual(serve);
151+
});
152+
});
153+
154+
describe('the shared resolution, exercised (#7001)', () => {
155+
// The scan above proves both paths call one helper; these prove the helper
156+
// they call answers correctly. Neither claim implies the other, and a source
157+
// scan alone would be green over a helper that returned nonsense.
158+
const declared = {
159+
permissions: [
160+
{ name: 'ignored_not_default', isDefault: false },
161+
{ name: 'app_member_default', isDefault: true },
162+
],
163+
};
164+
165+
it('carries an app-declared isDefault profile into the constructor options', () => {
166+
expect(appSecurityPluginOptions(declared)).toEqual({ fallbackPermissionSet: 'app_member_default' });
167+
expect(appDefaultPermissionSetName(declared.permissions)).toBe('app_member_default');
168+
});
169+
170+
it('yields undefined when nothing is declared, so the plugin keeps its own derivation', () => {
171+
// Deliberately NOT `{ fallbackPermissionSet: undefined }`: the constructor
172+
// reads an explicit `undefined` as "derive from the built-in sets" only
173+
// because the KEY is absent — `fallbackPermissionSet: null` means "no
174+
// baseline at all". Passing the object shape would work today and is one
175+
// refactor away from silently disabling the platform baseline.
176+
expect(appSecurityPluginOptions({ permissions: [{ name: 'plain' }] })).toBeUndefined();
177+
expect(appSecurityPluginOptions({})).toBeUndefined();
178+
expect(appSecurityPluginOptions(undefined)).toBeUndefined();
179+
});
180+
});

packages/cli/src/commands/serve.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2003,14 +2003,22 @@ export default class Serve extends Command {
20032003
// Pair: SecurityPlugin (RBAC) — optional
20042004
try {
20052005
const securityPkg = '@objectstack/plugin-security';
2006-
const { SecurityPlugin, appDefaultPermissionSetName } = await import(/* webpackIgnore: true */ securityPkg);
2006+
const { SecurityPlugin, appSecurityPluginOptions } = await import(/* webpackIgnore: true */ securityPkg);
20072007
// ADR-0056 D7 — honor an app-declared default profile. A stack
2008-
// permission set marked `isDefault` becomes the
2009-
// fallback for users with no explicit grants. The SecurityPlugin's
2010-
// own scan only sees its built-in sets, so the CLI passes the
2011-
// declared name through explicitly (undefined → built-in default).
2012-
const appDefaultProfile = appDefaultPermissionSetName((config as any)?.permissions);
2013-
await kernel.use(new SecurityPlugin(appDefaultProfile ? { fallbackPermissionSet: appDefaultProfile } : undefined));
2008+
// permission set marked `isDefault` becomes the baseline for
2009+
// users with no explicit grants. The SecurityPlugin's own scan
2010+
// only sees its built-in sets, so the declared name is passed
2011+
// through explicitly (undefined → built-in default).
2012+
//
2013+
// [#7001] Resolved through the SHARED helper rather than
2014+
// open-coded here. This was the only boot path that did it at
2015+
// all: `@objectstack/verify`'s `bootStack` constructed a vanilla
2016+
// `new SecurityPlugin()`, so an app's own dogfood suite ran
2017+
// against a boot without the profile the CLI gave its users. The
2018+
// two now agree by construction — one helper, one call shape, and
2019+
// `serve-verify-security-parity.contract.test.ts` fails if either
2020+
// side open-codes its way back out.
2021+
await kernel.use(new SecurityPlugin(appSecurityPluginOptions(config)));
20142022
trackPlugin('Security');
20152023
} catch {
20162024
// optional

packages/plugins/plugin-security/src/app-default-permission-set.test.ts

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
22
import { describe, it, expect } from 'vitest';
3-
import { appDefaultPermissionSetName } from './app-default-permission-set';
3+
import { appDefaultPermissionSetName, appSecurityPluginOptions } from './app-default-permission-set';
4+
import { SecurityPlugin } from './security-plugin';
45

56
describe('appDefaultPermissionSetName (ADR-0090 D5)', () => {
67
it('returns the name of the first isDefault permission set', () => {
@@ -25,3 +26,77 @@ describe('appDefaultPermissionSetName (ADR-0090 D5)', () => {
2526
).toBe('ok');
2627
});
2728
});
29+
30+
/**
31+
* [#7001] `appSecurityPluginOptions` — the whole constructor argument, so every
32+
* boot path spells the wiring once.
33+
*
34+
* `appDefaultPermissionSetName` above answers "which profile did the app
35+
* declare". That left the second half — turning a name into constructor options
36+
* — open-coded at each call site, and only ONE site ever had it: `objectstack
37+
* serve`. `@objectstack/verify`'s `bootStack` built a vanilla
38+
* `new SecurityPlugin()`, so an app's own suite ran against a boot without the
39+
* profile the CLI gave its users.
40+
*/
41+
describe('appSecurityPluginOptions (#7001)', () => {
42+
it('reads the declared default off a stack config', () => {
43+
expect(
44+
appSecurityPluginOptions({
45+
permissions: [{ name: 'read_only' }, { name: 'app_member_default', isDefault: true }],
46+
}),
47+
).toEqual({ fallbackPermissionSet: 'app_member_default' });
48+
});
49+
50+
it('returns undefined — NOT { fallbackPermissionSet: undefined } — when nothing is declared', () => {
51+
// The distinction is load-bearing, not stylistic. The constructor reads an
52+
// ABSENT key as "derive my own default from the built-in sets" and an
53+
// explicit `null` as "no baseline at all"; returning the object shape works
54+
// today only because `undefined` happens to hit the same branch, and is one
55+
// refactor away from silently disabling the platform baseline.
56+
for (const config of [{ permissions: [{ name: 'plain' }] }, { permissions: [] }, {}, null, undefined, 'nonsense']) {
57+
expect(appSecurityPluginOptions(config)).toBeUndefined();
58+
}
59+
});
60+
61+
it('reads `permissions` top-level, exactly where serve.ts has always read it', () => {
62+
// Being cleverer here (also looking inside `manifest`) would re-open the
63+
// #7001 gap in the other direction: the harness would honour a declaration
64+
// the CLI ignores, and a suite would again prove something production does
65+
// not do.
66+
expect(appSecurityPluginOptions({ manifest: { permissions: [{ name: 'buried', isDefault: true }] } }))
67+
.toBeUndefined();
68+
});
69+
});
70+
71+
/**
72+
* The options do not merely describe the wiring — they land on the plugin. A
73+
* fake `PluginContext` captures what `init()` publishes as the
74+
* `security.fallbackPermissionSet` service, which is the value the runtime
75+
* resolves every authenticated request's additive baseline from (ADR-0090 D5).
76+
*/
77+
describe('the resolved options reach the constructed plugin (#7001)', () => {
78+
const initAndReadBaseline = async (plugin: SecurityPlugin): Promise<unknown> => {
79+
const services = new Map<string, unknown>();
80+
await plugin.init({
81+
logger: { info() {}, warn() {}, error() {}, debug() {} },
82+
registerService: (name: string, value: unknown) => services.set(name, value),
83+
getService: (name: string) => {
84+
if (name === 'manifest') return { register() {} };
85+
return undefined;
86+
},
87+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
88+
} as any);
89+
return services.get('security.fallbackPermissionSet');
90+
};
91+
92+
it('an app-declared default becomes the plugin baseline', async () => {
93+
const config = { permissions: [{ name: 'app_member_default', isDefault: true }] };
94+
await expect(initAndReadBaseline(new SecurityPlugin(appSecurityPluginOptions(config))))
95+
.resolves.toBe('app_member_default');
96+
});
97+
98+
it('and no declaration leaves the built-in member_default standing', async () => {
99+
await expect(initAndReadBaseline(new SecurityPlugin(appSecurityPluginOptions({}))))
100+
.resolves.toBe('member_default');
101+
});
102+
});

0 commit comments

Comments
 (0)