Skip to content

Commit 7552e03

Browse files
Elon Muskclaude
andauthored
fix(plugin-dev): ask the published security service, in start(), whether anything is enforcing (#10092)
* fix(plugin-dev): probe the published `security` service in start() so the "not enforced" warning can fire (#10036) The warning probed `security.permissions` / `security.rls` / `security.fieldMasker` from init(). Those are SecurityPlugin.init() registrations that the spec contract names implementation internals; the published `security` service is the contract, and it is registered only in SecurityPlugin.start(), after both of that method's early returns and alongside the enforcement middleware. A stack whose start() bailed holds all three handles and enforces nothing, so the warning was silent in exactly the state its text describes. Probing `security` from init() would have been a permanent false positive (start() has not run yet), so the check moves to DevPlugin.start(), after the child-start loop and into the boot banner. The internal handles keep one honest use: telling "never loaded" apart from "loaded, then failed to start". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r * fix(plugin-dev): drop the dead `@objectstack/driver-memory` mock from the new test (#10036) The `driver-memory` census gate (#5499/#5704/#6664) flagged the new test file as a thirteenth module binding on a frozen driver. It was dead weight, not a consumer: DevPlugin imports `@objectstack/runtime` on the line BEFORE the driver import, and that import is mocked to throw, so the driver import is never evaluated. Measured rather than reasoned, with a control: a marker written from the `driver-memory` mock factory printed 0 times across the whole file, while the same marker in the `@objectstack/runtime` factory printed 8 times in the same run of the same harness. Removing the mock leaves the suite green (58/58) at the same duration. No ledger entry added and no ruling assumed — the census stays at 2 ruled consumers, which is the disposition that needs no maintainer ruling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 907c11d commit 7552e03

4 files changed

Lines changed: 276 additions & 19 deletions

File tree

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
---
2+
"@objectstack/plugin-dev": patch
3+
---
4+
5+
fix(plugin-dev): ask the published `security` service — in `start()` — whether anything is enforcing, so the "RBAC/RLS/masking are NOT enforced" warning can fire in the state it describes (#10036)
6+
7+
DevPlugin's security warning probed `security.permissions` / `security.rls` /
8+
`security.fieldMasker` from `init()`. Both halves were wrong, and wrong in the
9+
direction that is hardest to notice — silence read as health.
10+
11+
**Wrong signal.** Those three are `SecurityPlugin.init()` registrations. The
12+
`ISecurityService` contract in `@objectstack/spec` names them "implementation
13+
internals and deliberately NOT part of this contract"; the published `security`
14+
service is the contract. Their presence answers "is SecurityPlugin loaded?",
15+
not "is anything being enforced?" — and the two answers come apart at
16+
`SecurityPlugin.start()`, which returns early (when `objectql`/`metadata` will
17+
not resolve, and when the engine cannot take middleware) **before** it publishes
18+
`security` and before it registers a single enforcement middleware. A stack in
19+
that state holds all three internal handles and enforces nothing, so the warning
20+
stayed silent in the one state where its own text is literally true.
21+
22+
**Wrong phase.** `security` is registered in `SecurityPlugin.start()`, which
23+
DevPlugin runs in its own `start()`. Probing it from `init()` would find it
24+
absent on *every* stack, healthy ones included — so swapping only the service
25+
name turns a false negative into a permanent false positive. The check now runs
26+
after the child-start loop, in the boot banner an operator actually reads
27+
(the placement #3900 already established for the production-override brand).
28+
29+
Observable behaviour change, both directions:
30+
31+
- A stack whose `SecurityPlugin.start()` bailed now gets a warning that names
32+
that state ("LOADED but did not finish starting"), where it previously got
33+
silence. The internal handles keep their one honest use — telling "never
34+
loaded" apart from "loaded, then failed to start" — so the operator is
35+
pointed at the right fix.
36+
- The absent-plugin warning is unchanged in meaning and wording, but is now
37+
emitted from `start()` rather than `init()`.
38+
39+
This is the same move #10035 made for the other consumer this signal misled
40+
(`plugin-hono-server`'s `/auth/me/permissions` and `/me/apps`). Two consumers,
41+
two packages, one misread — a property of the signal, not of either reader.
Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
import { describe, it, expect, vi } from 'vitest';
2+
import { DevPlugin } from './dev-plugin';
3+
4+
// [#10036] The state under test is "SecurityPlugin LOADED but its start()
5+
// bailed", so `@objectstack/plugin-security` is deliberately NOT mocked here —
6+
// the real plugin's real `init()`/`start()` phase split is what constructs the
7+
// state. Every OTHER optional dependency is mocked away for the same reason as
8+
// #3060 (their vite transforms alone can blow the timeout), and their absence
9+
// is irrelevant to this file's subject.
10+
//
11+
// One FURTHER omission, measured rather than assumed:
12+
// `@objectstack/driver-memory` is deliberately not mocked here either,
13+
// because DevPlugin imports
14+
// `@objectstack/runtime` on the line BEFORE it and that import throws, so the
15+
// driver import is never evaluated and a mock for it would be dead weight. It
16+
// is a frozen driver under a retirement census (#5499/#5704/#6664), where an
17+
// unnecessary module binding is the defect the census exists to catch, so the
18+
// dead mock is not harmless bookkeeping. Probed, not reasoned: a marker in the
19+
// factory printed 0 times across the whole file while the same marker in the
20+
// `@objectstack/runtime` factory printed 8 times in the same run. ⛔ Do not add
21+
// one back.
22+
vi.mock('@objectstack/objectql', () => { throw Object.assign(new Error("Cannot find package '@objectstack/objectql'"), { code: 'ERR_MODULE_NOT_FOUND' }); });
23+
vi.mock('@objectstack/runtime', () => { throw Object.assign(new Error("Cannot find package '@objectstack/runtime'"), { code: 'ERR_MODULE_NOT_FOUND' }); });
24+
vi.mock('@objectstack/service-i18n', () => { throw Object.assign(new Error("Cannot find package '@objectstack/service-i18n'"), { code: 'ERR_MODULE_NOT_FOUND' }); });
25+
vi.mock('@objectstack/service-storage', () => { throw Object.assign(new Error("Cannot find package '@objectstack/service-storage'"), { code: 'ERR_MODULE_NOT_FOUND' }); });
26+
vi.mock('@objectstack/service-realtime', () => { throw Object.assign(new Error("Cannot find package '@objectstack/service-realtime'"), { code: 'ERR_MODULE_NOT_FOUND' }); });
27+
vi.mock('@objectstack/plugin-auth', () => { throw Object.assign(new Error("Cannot find package '@objectstack/plugin-auth'"), { code: 'ERR_MODULE_NOT_FOUND' }); });
28+
vi.mock('@objectstack/plugin-hono-server', () => { throw Object.assign(new Error("Cannot find package '@objectstack/plugin-hono-server'"), { code: 'ERR_MODULE_NOT_FOUND' }); });
29+
vi.mock('@objectstack/rest', () => { throw Object.assign(new Error("Cannot find package '@objectstack/rest'"), { code: 'ERR_MODULE_NOT_FOUND' }); });
30+
vi.mock('@objectstack/setup', () => { throw Object.assign(new Error("Cannot find package '@objectstack/setup'"), { code: 'ERR_MODULE_NOT_FOUND' }); });
31+
vi.mock('@objectstack/account', () => { throw Object.assign(new Error("Cannot find package '@objectstack/account'"), { code: 'ERR_MODULE_NOT_FOUND' }); });
32+
33+
function mockCtx() {
34+
const registeredServices = new Map<string, any>();
35+
const ctx: any = {
36+
logger: { info: vi.fn(), debug: vi.fn(), warn: vi.fn(), error: vi.fn() },
37+
getService: vi.fn().mockImplementation((name: string) => {
38+
if (registeredServices.has(name)) return registeredServices.get(name);
39+
throw new Error(`service '${name}' not found`);
40+
}),
41+
getServices: vi.fn().mockReturnValue(new Map()),
42+
registerService: vi.fn().mockImplementation((name: string, svc: any) => {
43+
registeredServices.set(name, svc);
44+
}),
45+
hook: vi.fn(),
46+
trigger: vi.fn(),
47+
getKernel: vi.fn(),
48+
};
49+
// SecurityPlugin.init() contributes to the manifest; without this its init()
50+
// throws midway and the state we want to construct is only half-built.
51+
registeredServices.set('manifest', { register: () => {} });
52+
return { ctx, registeredServices };
53+
}
54+
55+
/** Every `logger.warn` line that claims security is not being enforced. */
56+
function enforcementWarnings(ctx: any): string[] {
57+
return ctx.logger.warn.mock.calls
58+
.map((call: any[]) => (typeof call[0] === 'string' ? call[0] : ''))
59+
.filter((msg: string) => msg.includes('NOT enforced'));
60+
}
61+
62+
async function boot(ctx: any, options: Record<string, unknown> = {}) {
63+
const plugin = new DevPlugin({ seedAdminUser: false, ...options });
64+
await plugin.init(ctx);
65+
await plugin.start(ctx);
66+
return plugin;
67+
}
68+
69+
describe('[#10036] the "nothing is enforced" warning must fire when SecurityPlugin.start() bailed', () => {
70+
// ── The state the warning describes, constructed for real ───────────────
71+
//
72+
// SecurityPlugin registers `security.permissions` / `security.rls` /
73+
// `security.fieldMasker` in `init()` and the published `security` service
74+
// (plus every enforcement middleware) only in `start()`, which returns early
75+
// when the engine cannot take middleware. So a stack can hold all three
76+
// internal handles while NOTHING is enforced — and that is precisely the
77+
// state the warning's own text describes.
78+
79+
it('bail #1 (no objectql/metadata service): the three init() handles resolve, `security` does not, and the warning fires', async () => {
80+
const { ctx, registeredServices } = mockCtx();
81+
// No `objectql` service at all → SecurityPlugin.start() takes its FIRST
82+
// early return.
83+
await boot(ctx);
84+
85+
// Precondition — the state really is the one the card describes.
86+
expect(registeredServices.has('security.permissions'), 'SecurityPlugin.init() ran').toBe(true);
87+
expect(registeredServices.has('security.rls')).toBe(true);
88+
expect(registeredServices.has('security.fieldMasker')).toBe(true);
89+
expect(registeredServices.has('security'), 'start() bailed before publishing the service').toBe(false);
90+
91+
// The real plugin logged its own bail, from inside itself.
92+
const bail = ctx.logger.warn.mock.calls.find(
93+
(call: any[]) => typeof call[0] === 'string' && call[0].includes('security middleware not registered'),
94+
);
95+
expect(bail, 'SecurityPlugin.start() must have bailed').toBeDefined();
96+
97+
// …and the dev assembly says so out loud.
98+
const warnings = enforcementWarnings(ctx);
99+
expect(warnings.length, 'the dev assembly must warn that nothing is enforced').toBe(1);
100+
expect(warnings[0]).toContain('LOADED');
101+
});
102+
103+
it('bail #2 (engine cannot take middleware): same — handles present, no enforcement, warning fires', async () => {
104+
const { ctx, registeredServices } = mockCtx();
105+
// An engine that resolves but has no `registerMiddleware` → SecurityPlugin
106+
// .start() takes its SECOND early return.
107+
registeredServices.set('objectql', { find: () => [] });
108+
registeredServices.set('metadata', { list: () => [] });
109+
110+
await boot(ctx);
111+
112+
expect(registeredServices.has('security.permissions')).toBe(true);
113+
expect(registeredServices.has('security')).toBe(false);
114+
115+
const bail = ctx.logger.warn.mock.calls.find(
116+
(call: any[]) => typeof call[0] === 'string' && call[0].includes('does not support middleware'),
117+
);
118+
expect(bail, 'the engine-cannot-take-middleware bail must be the one taken').toBeDefined();
119+
120+
const warnings = enforcementWarnings(ctx);
121+
expect(warnings.length).toBe(1);
122+
expect(warnings[0]).toContain('LOADED');
123+
});
124+
125+
it('does not fire when SecurityPlugin.start() completed and published the `security` service', async () => {
126+
const { ctx, registeredServices } = mockCtx();
127+
// An engine that CAN take middleware → start() runs to completion and
128+
// registers the published `security` service alongside the middleware.
129+
const registerMiddleware = vi.fn();
130+
registeredServices.set('objectql', { registerMiddleware, find: () => [] });
131+
registeredServices.set('metadata', { list: () => [], get: () => undefined });
132+
133+
await boot(ctx);
134+
135+
// Precondition — this really is the healthy state.
136+
expect(registeredServices.has('security'), 'start() published the service').toBe(true);
137+
expect(registerMiddleware, 'enforcement middleware was installed').toHaveBeenCalled();
138+
139+
expect(enforcementWarnings(ctx)).toEqual([]);
140+
});
141+
142+
it('stays silent when the operator disabled security explicitly', async () => {
143+
const { ctx } = mockCtx();
144+
await boot(ctx, { services: { security: false } });
145+
expect(enforcementWarnings(ctx)).toEqual([]);
146+
});
147+
});

‎packages/plugins/plugin-dev/src/dev-plugin.test.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,14 @@ describe('DevPlugin', () => {
8585
it('registers no service implementation of its own — every unfilled slot stays empty', async () => {
8686
const { ctx, registeredServices } = mockCtx();
8787

88-
await new DevPlugin({ seedAdminUser: false }).init(ctx);
88+
const plugin = new DevPlugin({ seedAdminUser: false });
89+
await plugin.init(ctx);
90+
// [#10036] `start()` too: the "nothing is enforcing security" warning
91+
// asserted at the bottom of this test moved to the start phase, because
92+
// `security` — the published service that means enforcement, as opposed
93+
// to the `init()`-registered internals that only mean "plugin loaded" —
94+
// is not registered until SecurityPlugin.start() has run.
95+
await plugin.start(ctx);
8996

9097
// Not one slot was filled by this plugin itself.
9198
expect(ctx.registerService).not.toHaveBeenCalled();
@@ -116,6 +123,9 @@ describe('DevPlugin', () => {
116123
(call: any[]) => typeof call[0] === 'string' && call[0].includes('NOT enforced'),
117124
);
118125
expect(securityWarn).toBeDefined();
126+
// …and with the plugin genuinely absent it says so, rather than reporting
127+
// the loaded-but-failed-to-start state (#10036).
128+
expect(securityWarn![0]).toContain('SecurityPlugin is not loaded');
119129
});
120130

121131
describe('production guard (ADR-0115 D6)', () => {

‎packages/plugins/plugin-dev/src/dev-plugin.ts‎

Lines changed: 77 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -888,24 +888,10 @@ export class DevPlugin implements Plugin {
888888
// in-process consumers handle absence exactly as they already must in
889889
// production. To use a capability locally, install its real service.
890890

891-
// The security slots deserve one loud line when empty (#4126): faking an
892-
// authorization decision is the one thing ADR-0076 D12 forbids a fallback
893-
// to do, so the slots stay empty rather than stubbed — but "no RBAC/RLS/
894-
// masking is being enforced" is worth saying in the boot log of a stack
895-
// that expected them.
896-
if (enabled('security')) {
897-
const missing = ['security.permissions', 'security.rls', 'security.fieldMasker'].filter((svc) => {
898-
try { ctx.getService(svc); return false; } catch { return true; }
899-
});
900-
if (missing.length > 0) {
901-
ctx.logger.warn(
902-
` ⚠ No security services (${missing.join(', ')}) — SecurityPlugin is not loaded, so RBAC, `
903-
+ 'row-level security and field masking are NOT enforced. The slots stay empty rather than '
904-
+ 'being stubbed: a fake that answers "allowed" is worse than an absent one. Install '
905-
+ '@objectstack/plugin-security to enforce them.',
906-
);
907-
}
908-
}
891+
// The security slots deserve one loud line when nothing is enforcing
892+
// (#4126) — but that question cannot be answered HERE. It is asked in
893+
// `start()` instead; see `warnIfNothingIsEnforcingSecurity` below for why
894+
// the phase, and not just the service name, is load-bearing.
909895

910896
ctx.logger.info(`DevPlugin initialized ${this.childPlugins.length} plugin(s)`);
911897
}
@@ -950,13 +936,86 @@ export class DevPlugin implements Plugin {
950936
+ 'this is NOT a production stack',
951937
);
952938
}
939+
// Same reasoning, same surface: "nothing is enforcing security" belongs
940+
// next to the banner, not buried in the init log (#10036, #3900).
941+
this.warnIfNothingIsEnforcingSecurity(ctx);
953942
ctx.logger.info('');
954943
ctx.logger.info(' API: /api/v1/data/:object');
955944
ctx.logger.info(' Metadata: /api/v1/meta/:type/:name');
956945
ctx.logger.info(' Discovery: /.well-known/objectstack');
957946
ctx.logger.info('─────────────────────────────────────────');
958947
}
959948

949+
/**
950+
* One loud line when the stack is enforcing no security at all (#4126,
951+
* ADR-0076 D12: a fake that answers "allowed" is worse than an absent one,
952+
* so the slots stay empty — but silence about unenforced RBAC/RLS/masking
953+
* would be its own kind of fake).
954+
*
955+
* ## Why this asks for `security`, and why it asks in `start()` (#10036)
956+
*
957+
* This used to probe `security.permissions` / `security.rls` /
958+
* `security.fieldMasker` from `init()`. Both halves of that were wrong, and
959+
* they were wrong in the direction that is hardest to notice — silence read
960+
* as health:
961+
*
962+
* - **Wrong signal.** Those three are `SecurityPlugin.init()` registrations.
963+
* The `ISecurityService` contract in `@objectstack/spec` names them
964+
* "implementation internals and deliberately NOT part of this contract";
965+
* the published `security` service is the contract. Their presence answers
966+
* "is SecurityPlugin loaded?", which is not the question this warning
967+
* asks. The two answers come apart at `start()`: it returns early — when
968+
* `objectql`/`metadata` will not resolve, and when the engine cannot take
969+
* middleware — BEFORE it publishes `security` and before it registers a
970+
* single enforcement middleware. A stack in that state holds all three
971+
* internal handles and enforces nothing, so the warning stayed silent in
972+
* the one state where its text is literally true. (The same presence
973+
* signal misled `plugin-hono-server`'s `/auth/me/permissions`, fixed in
974+
* #10035 by this same move — two consumers, two packages, one misread:
975+
* that is a property of the signal, not of either reader.)
976+
*
977+
* - **Wrong phase.** `security` is registered in `SecurityPlugin.start()`,
978+
* which this plugin runs in its OWN `start()`. Asking from `init()` would
979+
* find it absent on every stack, healthy ones included — so swapping only
980+
* the service name would have turned a false negative into a permanent
981+
* false positive. The question is answerable only after the child-start
982+
* loop has run.
983+
*
984+
* The internal handles keep exactly one honest use, and it is the one they
985+
* can support: telling "SecurityPlugin was never loaded" apart from
986+
* "SecurityPlugin loaded and then failed to start", so the operator is
987+
* pointed at the right fix.
988+
*/
989+
private warnIfNothingIsEnforcingSecurity(ctx: PluginContext): void {
990+
if (this.options.services?.['security'] === false) return; // opted out
991+
992+
// An absent slot may throw OR resolve to undefined depending on the
993+
// kernel; both mean "nothing is there".
994+
const resolves = (name: string): boolean => {
995+
try { return ctx.getService(name) != null; } catch { return false; }
996+
};
997+
998+
if (resolves('security')) return; // enforcement middleware is installed
999+
1000+
const loadedButNotEnforcing = ['security.permissions', 'security.rls', 'security.fieldMasker']
1001+
.some(resolves);
1002+
1003+
ctx.logger.warn(
1004+
loadedButNotEnforcing
1005+
? ' ⚠ SecurityPlugin is LOADED but did not finish starting — it published no `security` '
1006+
+ 'service, so no enforcement middleware was registered and RBAC, row-level security and '
1007+
+ 'field masking are NOT enforced. Its own start() warning above says why (the objectql or '
1008+
+ 'metadata service could not be resolved, or the engine does not accept middleware). The '
1009+
+ '`security.permissions` / `security.rls` / `security.fieldMasker` handles ARE present — '
1010+
+ 'they are registered in init() and mean the plugin loaded, never that anything is being '
1011+
+ 'enforced.'
1012+
: ' ⚠ No `security` service — SecurityPlugin is not loaded, so RBAC, row-level security '
1013+
+ 'and field masking are NOT enforced. The slots stay empty rather than being stubbed: a '
1014+
+ 'fake that answers "allowed" is worse than an absent one. Install '
1015+
+ '@objectstack/plugin-security to enforce them.',
1016+
);
1017+
}
1018+
9601019
/**
9611020
* Destroy Phase
9621021
*

0 commit comments

Comments
 (0)