Skip to content

Commit 9690d11

Browse files
hotlongclaude
andauthored
fix(plugin-security): the app default permission set resolves from packages[] (#15007) (#15226)
Reader card 4/4 of the ADR-0130 D4 option-B program ruled on #14512. This is the site both earlier enumerations missed: `appSecurityPluginOptions` read `config.permissions` top-level and nothing else, so a multi-package artifact in the option-B shape resolves no `isDefault` profile, boots clean, and leaves every member of the app on the platform baseline alone. `declaredPermissionSets` now reads the flattened top level FIRST — so every artifact emitted today answers bit-identically — and then each package body in `resolveArtifactPackageOrder` order (`@objectstack/core`, ADR-0130 D4+D5). The singular `manifest` is still not consulted (#7001), and a malformed `packages` raises the loader's own ADR-0112 envelope rather than being skipped. The #15004 acceptance pin's ledger shrinks by exactly one row, the B2 from-source one. Its B1 sibling stays: that row reads `appSecurityPluginOptions(createStandaloneStack(...))`, and the standalone result surfaces neither `permissions` nor `packages` under option B — it is downstream of card #15005, not of this reader. Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m Co-authored-by: Claude <noreply@anthropic.com>
1 parent 445a25f commit 9690d11

4 files changed

Lines changed: 328 additions & 6 deletions

File tree

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
---
2+
"@objectstack/plugin-security": patch
3+
---
4+
5+
fix(plugin-security): the app default permission set resolves from `packages[]`, not only the flattened top level (#15007)
6+
7+
`appSecurityPluginOptions(config)` read `config.permissions` and nothing else.
8+
For a multi-package artifact under the ADR-0130 D4 option-B shape — where
9+
`packages[]` carries each definition exactly once and the flattened top-level
10+
copy is gone — that read returns `undefined`, the reader concludes "this app
11+
declared no default profile", and the boot continues. Nothing throws and
12+
nothing logs.
13+
14+
That silence has a security posture attached. The name this resolves becomes
15+
the `SecurityPlugin`'s `fallbackPermissionSet`, i.e. the app's half of every
16+
authenticated human principal's additive baseline
17+
(`composeHumanBaselinePermissionSets`, ADR-0090 D5). Losing it does not deny
18+
anyone the boot — the deployment simply runs on the platform floor alone, and
19+
every member of a multi-package app quietly holds less access than the app
20+
declared for them. #7555 measured what that looks like from the outside: nav
21+
entries served, 403 behind them.
22+
23+
The resolution now reads the flattened top level FIRST and then each package
24+
body, in the order `resolveArtifactPackageOrder` (`@objectstack/core`,
25+
ADR-0130 D4+D5) registers them:
26+
27+
- **Every artifact the platform emits today answers bit-identically.** The
28+
flattened level still answers first, so the `packages[]` pass can only supply
29+
a set where the top level had none. This is the reader half of the ruled
30+
order (readers first, emitter last, artifact stays additive throughout), so
31+
it lands with no change to what any command emits.
32+
- **Order is the platform's one package order, not the array's.**
33+
`appDefaultPermissionSetName` resolves the FIRST `isDefault` set, so with two
34+
packages declaring one, "first" has to mean here what it means at every other
35+
artifact reader: dependency-topological, so a package that extends another is
36+
read after it whichever array slot it occupies.
37+
- **The singular `manifest` is still not consulted** (#7001 — the harness must
38+
not honour a declaration `serve.ts` ignores). That is not a special case: an
39+
artifact carrying no `packages` key makes `resolveArtifactPackageOrder`
40+
return the caller's own object as the single package body, so that branch
41+
reads `permissions` from exactly where the old code read it.
42+
- **A malformed `packages` is refused, not skipped.** A non-array `packages`,
43+
an entry inlined instead of wrapped under `manifest:`, or a duplicate package
44+
id raises the same ADR-0112 envelope (`code` + `status: 422`) the manifest
45+
service raises when it registers that artifact. Catching it would resolve a
46+
permission surface out of an artifact the loader refuses to load.
47+
48+
Every boot path that already funnelled through this one function picks the fix
49+
up unchanged: `objectstack serve`'s artifact and from-source paths, and
50+
`@objectstack/verify`'s `bootStack` / RLS harness.

packages/cli/test/option-b-reader-acceptance.pin.test.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,6 @@ const OPTION_B_LOSSES: readonly string[] = [
143143
'B2 · AppPlugin ql.setDatasourceMapping (object routing) (from source) · datasourceMapping',
144144
'B2 · AppPlugin seed datasets merged (from source) · data',
145145
'B2 · AppPlugin translation loading into the i18n service (from source) · translations',
146-
'B2 · plugin-security appSecurityPluginOptions over the from-source config (default permission set) · permissions',
147146
'B2 · runtime collectBundleActions over the from-source config · actions + objects[].actions',
148147
'B2 · runtime collectBundleFunctionEntries over the from-source config · functions',
149148
'B2 · runtime collectBundleHooks over the from-source config · hooks',

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

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
22
import { describe, it, expect } from 'vitest';
3+
import {
4+
AssembledPackageBodySchema,
5+
ObjectStackDefinitionSchema,
6+
composeStacks,
7+
defineStack,
8+
} from '@objectstack/spec';
39
import { appDefaultPermissionSetName, appSecurityPluginOptions } from './app-default-permission-set';
410
import { SecurityPlugin } from './security-plugin';
511

@@ -100,3 +106,192 @@ describe('the resolved options reach the constructed plugin (#7001)', () => {
100106
.resolves.toBe('member_default');
101107
});
102108
});
109+
110+
/**
111+
* [ADR-0130 D4, #15007] The reader resolves `packages[]`.
112+
*
113+
* Reader card 4/4 of the option-B program ruled on #14512. A multi-package
114+
* artifact carries each definition twice today — flattened at the top level and
115+
* again under `packages[]` — and option B removes the flattened copy. Every
116+
* assertion below is about the SAME declaration read out of both shapes, which
117+
* is what "the artifact stays additive while the readers learn" means.
118+
*
119+
* The two shapes are built by the REAL composer (`composeStacks`, the one
120+
* `examples/app-multi-package` uses) rather than hand-written, so a package
121+
* entry that stopped looking the way this file assumes fails here instead of
122+
* passing against a shape the platform never emits. The option-B shape is
123+
* derived from it by stripping the package-owned keys — and that key set is
124+
* read off the two schemas, never transcribed, so a collection family added to
125+
* the stack schema next month is stripped too.
126+
*/
127+
describe('appSecurityPluginOptions over `packages[]` (ADR-0130 D4, #15007)', () => {
128+
const CORE_ID = 'com.example.security.core';
129+
const ADDON_ID = 'com.example.security.addon';
130+
const CORE_PROFILE = 'core_member_default';
131+
const ADDON_PROFILE = 'addon_member_default';
132+
133+
const shapeKeys = (schema: unknown): string[] =>
134+
Object.keys((schema as { shape: Record<string, unknown> }).shape);
135+
136+
/** Exactly the keys an option-B artifact no longer carries at the top level. */
137+
const PACKAGE_OWNED_KEYS: readonly string[] = (() => {
138+
const body = new Set(shapeKeys(AssembledPackageBodySchema));
139+
return shapeKeys(ObjectStackDefinitionSchema).filter((k) => body.has(k));
140+
})();
141+
142+
const permissionSet = (name: string) => ({
143+
name,
144+
label: name,
145+
isDefault: true,
146+
objects: {},
147+
});
148+
149+
const coreStack = () =>
150+
defineStack({
151+
manifest: {
152+
id: CORE_ID, name: 'Security Probe Core', namespace: 'secprobe',
153+
version: '1.0.0', type: 'app',
154+
},
155+
permissions: [permissionSet(CORE_PROFILE)],
156+
});
157+
158+
/** Declared SECOND in composition order, and depends on the app package. */
159+
const addonStack = () =>
160+
defineStack({
161+
manifest: {
162+
id: ADDON_ID, name: 'Security Probe Addon', namespace: 'secprobe',
163+
version: '1.0.0', type: 'module',
164+
dependencies: { [CORE_ID]: '^1.0.0' },
165+
},
166+
});
167+
168+
/** Today's emitted shape: flattened top level PLUS `packages[]`. */
169+
const additive = () => composeStacks([addonStack(), coreStack()], { manifest: 'preserve' });
170+
171+
/** The ruled shape: `packages[]` only. */
172+
const optionB = () => {
173+
const composed = additive() as unknown as Record<string, unknown>;
174+
const owned = new Set(PACKAGE_OWNED_KEYS);
175+
const out: Record<string, unknown> = {};
176+
for (const [key, value] of Object.entries(composed)) if (!owned.has(key)) out[key] = value;
177+
return out;
178+
};
179+
180+
it('CONTROL — the additive shape really does carry the flattened copy', () => {
181+
// Without this, the option-B case below could pass because the fixture
182+
// never had a flattened level to lose.
183+
const composed = additive() as unknown as Record<string, unknown>;
184+
expect(Array.isArray(composed.permissions)).toBe(true);
185+
expect((composed.permissions as unknown[]).length).toBeGreaterThan(0);
186+
expect((composed.packages as unknown[]).length).toBe(2);
187+
expect(PACKAGE_OWNED_KEYS).toContain('permissions');
188+
});
189+
190+
it('the additive shape answers exactly what it answered before this card', () => {
191+
expect(appSecurityPluginOptions(additive())).toEqual({ fallbackPermissionSet: CORE_PROFILE });
192+
});
193+
194+
it('OPTION B — the flattened level is gone and the packaged declaration is still resolved', () => {
195+
const stripped = optionB();
196+
expect(stripped.permissions).toBeUndefined();
197+
expect((stripped.packages as unknown[]).length).toBe(2);
198+
199+
// The pre-#15007 reader returned `undefined` here — no throw, no log, and
200+
// every member of the app silently down to the platform floor alone.
201+
expect(appSecurityPluginOptions(stripped)).toEqual({ fallbackPermissionSet: CORE_PROFILE });
202+
});
203+
204+
it('the flattened level still answers FIRST when both shapes carry a set', () => {
205+
// The reader half lands while the artifact is still additive, so this
206+
// function must be a superset of the old read and never a replacement:
207+
// whatever the top level said, it still says.
208+
expect(
209+
appSecurityPluginOptions({
210+
permissions: [permissionSet('flattened_wins')],
211+
packages: [{ manifest: { id: CORE_ID, name: 'Core', version: '1.0.0', type: 'app', permissions: [permissionSet(CORE_PROFILE)] } }],
212+
}),
213+
).toEqual({ fallbackPermissionSet: 'flattened_wins' });
214+
});
215+
216+
it('package order is `resolveArtifactPackageOrder`\'s, not the array\'s', () => {
217+
// Both packages declare an `isDefault` set and the DEPENDENT one is listed
218+
// first. "The first isDefault set" has to mean the same thing here as at
219+
// every other artifact reader, so the depended-upon package answers —
220+
// dependency-topological order (ADR-0130 D5), not authoring accident.
221+
expect(
222+
appSecurityPluginOptions({
223+
packages: [
224+
{ manifest: { id: ADDON_ID, name: 'Addon', version: '1.0.0', type: 'module', dependencies: { [CORE_ID]: '^1.0.0' }, permissions: [permissionSet(ADDON_PROFILE)] } },
225+
{ manifest: { id: CORE_ID, name: 'Core', version: '1.0.0', type: 'app', permissions: [permissionSet(CORE_PROFILE)] } },
226+
],
227+
}),
228+
).toEqual({ fallbackPermissionSet: CORE_PROFILE });
229+
230+
// …and with the dependency edge removed, declared order is what is left.
231+
expect(
232+
appSecurityPluginOptions({
233+
packages: [
234+
{ manifest: { id: ADDON_ID, name: 'Addon', version: '1.0.0', type: 'module', permissions: [permissionSet(ADDON_PROFILE)] } },
235+
{ manifest: { id: CORE_ID, name: 'Core', version: '1.0.0', type: 'app', permissions: [permissionSet(CORE_PROFILE)] } },
236+
],
237+
}),
238+
).toEqual({ fallbackPermissionSet: ADDON_PROFILE });
239+
});
240+
241+
it('a package that declares no default does not shadow one that does', () => {
242+
expect(
243+
appSecurityPluginOptions({
244+
packages: [
245+
{ manifest: { id: ADDON_ID, name: 'Addon', version: '1.0.0', type: 'module', permissions: [{ name: 'addon_read_only', label: 'RO', objects: {} }] } },
246+
{ manifest: { id: CORE_ID, name: 'Core', version: '1.0.0', type: 'app', permissions: [permissionSet(CORE_PROFILE)] } },
247+
],
248+
}),
249+
).toEqual({ fallbackPermissionSet: CORE_PROFILE });
250+
});
251+
252+
it('an artifact with no `packages` key still reads the top level and NOTHING else', () => {
253+
// D4's second branch hands `resolveArtifactPackageOrder` the caller's own
254+
// object back as the single package body, so this path is the pre-#15007
255+
// read exactly — including its refusal to look inside the singular
256+
// `manifest` (#7001, pinned above).
257+
expect(appSecurityPluginOptions({ manifest: { permissions: [permissionSet('buried')] } })).toBeUndefined();
258+
expect(appSecurityPluginOptions({ packages: [] })).toBeUndefined();
259+
expect(appSecurityPluginOptions({ permissions: [permissionSet('top')] })).toEqual({ fallbackPermissionSet: 'top' });
260+
});
261+
262+
/**
263+
* The gate travels with the read: `resolveArtifactPackageOrder` refuses a
264+
* malformed `packages` with an ADR-0112 envelope, and this reader does not
265+
* catch it. Swallowing it would resolve a permission surface out of an
266+
* artifact the loader refuses to load.
267+
*/
268+
describe('a malformed `packages` is refused, not silently skipped', () => {
269+
const refusalOf = (config: unknown): { code?: string; status?: number; message?: string } => {
270+
try {
271+
appSecurityPluginOptions(config);
272+
return {};
273+
} catch (e) {
274+
return e as { code?: string; status?: number; message?: string };
275+
}
276+
};
277+
278+
it('`packages` that is not an array', () => {
279+
const err = refusalOf({ packages: 'nope' });
280+
expect(err.code).toBe('INVALID_ARTIFACT_PACKAGES');
281+
expect(err.status).toBe(422);
282+
});
283+
284+
it('an entry inlined instead of wrapped under `manifest:`', () => {
285+
const err = refusalOf({ packages: [{ id: CORE_ID, name: 'Core', version: '1.0.0', type: 'app', permissions: [permissionSet(CORE_PROFILE)] }] });
286+
expect(err.code).toBe('INVALID_ARTIFACT_PACKAGE_ENTRY');
287+
expect(err.status).toBe(422);
288+
});
289+
290+
it('the same package id twice', () => {
291+
const entry = { manifest: { id: CORE_ID, name: 'Core', version: '1.0.0', type: 'app', permissions: [permissionSet(CORE_PROFILE)] } };
292+
const err = refusalOf({ packages: [entry, entry] });
293+
expect(err.code).toBe('DUPLICATE_ARTIFACT_PACKAGE');
294+
expect(err.status).toBe(422);
295+
});
296+
});
297+
});

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

Lines changed: 83 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
22

3+
import { resolveArtifactPackageOrder } from '@objectstack/core';
4+
35
/**
46
* [ADR-0090 D5, #7555] The PLATFORM's own human baseline permission set.
57
*
@@ -83,6 +85,83 @@ export function appDefaultPermissionSetName(permissions: unknown): string | unde
8385
return undefined;
8486
}
8587

88+
/**
89+
* [ADR-0130 D4, #15007] Every permission set a stack config DECLARES — from the
90+
* flattened top level, and from `packages[]`.
91+
*
92+
* ## What this exists to stop
93+
*
94+
* A multi-package artifact carries each definition twice today: flattened onto
95+
* the top level, and again inside `packages[i]`. Option B (the ADR-0130 D4
96+
* ruling on #14512) removes the flattened copy, so `packages[]` carries it
97+
* once. A reader that only ever looked at the top level does not fail when that
98+
* happens — it reads `undefined`, finds no `isDefault` set, and answers "the app
99+
* declared no default profile".
100+
*
101+
* For THIS reader that silence has a security posture attached. The name it
102+
* resolves becomes the `SecurityPlugin`'s `fallbackPermissionSet`, i.e. the
103+
* app's half of every authenticated human's additive baseline
104+
* ({@link composeHumanBaselinePermissionSets}). Losing it does not deny the
105+
* boot and does not log: the deployment simply runs on the platform floor
106+
* alone, and every member of a multi-package app quietly holds less than the
107+
* app declared they should. #7555 measured what that looks like from the
108+
* outside — nav entries served, 403 behind them — and could only measure it
109+
* because someone went looking.
110+
*
111+
* ## Top level FIRST, `packages[]` second — and why that order is the contract
112+
*
113+
* The reader half of the program lands while the artifact is still ADDITIVE, so
114+
* this function has to be a superset of the old read rather than a replacement
115+
* for it: for every artifact the platform emits today the flattened level
116+
* answers first and this returns exactly what it returned before. The
117+
* `packages[]` pass only supplies a set where the top level had none — which is
118+
* precisely the option-B artifact. That is what makes this card revertible on
119+
* its own and safe to land before the emitter half (#14512).
120+
*
121+
* ## The order is `resolveArtifactPackageOrder`'s, not the array's
122+
*
123+
* `appDefaultPermissionSetName` resolves the FIRST `isDefault` set, so with more
124+
* than one package declaring one, "first" has to mean the same thing here as it
125+
* does everywhere else the artifact is read. `resolveArtifactPackageOrder`
126+
* (`@objectstack/core`, ADR-0130 D4+D5, #14643) is the ONE place that turns an
127+
* artifact into its ordered package list — dependency-topological, so a package
128+
* that extends another is read after it regardless of which array slot it
129+
* occupies. ⛔ Do not iterate `config.packages` directly here; a second
130+
* traversal is a second ordering, and the depended-upon package would win or
131+
* lose by authoring accident.
132+
*
133+
* ## Two things it deliberately does NOT do
134+
*
135+
* • It does not look inside the SINGULAR `manifest`. That constraint is
136+
* #7001's and it still holds — the harness must not honour a declaration
137+
* `serve.ts` ignores. Note this is not a special case bolted on: an
138+
* artifact carrying no `packages` key makes `resolveArtifactPackageOrder`
139+
* return the caller's own object as the single package body (D4's second
140+
* branch, D7's compatibility term), so that branch reads `permissions` from
141+
* exactly where the old code read it and nowhere else.
142+
* • It does not catch `resolveArtifactPackageOrder`'s refusals. A malformed
143+
* `packages` (not an array, an unwrapped entry, a duplicate package id)
144+
* raises an ADR-0112 envelope here, the same one the manifest service
145+
* raises when it registers that artifact moments later. Swallowing it would
146+
* resolve a permission surface out of an artifact the loader refuses to
147+
* load — the gate travels with the read.
148+
*/
149+
function declaredPermissionSets(config: unknown): unknown[] {
150+
const sets: unknown[] = [];
151+
152+
const flattened = (config as { permissions?: unknown } | null | undefined)?.permissions;
153+
if (Array.isArray(flattened)) sets.push(...flattened);
154+
155+
const packages = (config as { packages?: unknown } | null | undefined)?.packages;
156+
if (packages === undefined || packages === null) return sets;
157+
158+
for (const body of resolveArtifactPackageOrder(config)) {
159+
const declared = (body as { permissions?: unknown } | null | undefined)?.permissions;
160+
if (Array.isArray(declared)) sets.push(...declared);
161+
}
162+
return sets;
163+
}
164+
86165
/**
87166
* [#7001] The `SecurityPlugin` options a stack config implies — ONE resolution
88167
* for EVERY boot path.
@@ -112,14 +191,13 @@ export function appDefaultPermissionSetName(permissions: unknown): string | unde
112191
* the result straight through — `new SecurityPlugin(appSecurityPluginOptions(config))`
113192
* — and a caller cannot get the undefined case subtly wrong.
114193
*
115-
* Reads `config.permissions`, top-level, exactly as `serve.ts` always has.
116-
* Being cleverer here (also looking inside `manifest`) would re-open the gap it
117-
* closes, in the other direction.
194+
* Reads the sets through {@link declaredPermissionSets} — the flattened top
195+
* level `serve.ts` has always read, and, for a multi-package artifact, the
196+
* `packages[]` bodies that carry the same declaration under ADR-0130 D4.
118197
*/
119198
export function appSecurityPluginOptions(
120199
config: unknown,
121200
): { fallbackPermissionSet: string } | undefined {
122-
const permissions = (config as { permissions?: unknown } | null | undefined)?.permissions;
123-
const name = appDefaultPermissionSetName(permissions);
201+
const name = appDefaultPermissionSetName(declaredPermissionSets(config));
124202
return name ? { fallbackPermissionSet: name } : undefined;
125203
}

0 commit comments

Comments
 (0)