fix(plugin-dev): ask the published security service, in start(), whether anything is enforcing

.changeset/dev-plugin-security-enforcement-warning.md

@@ -0,0 +1,41 @@
+---
+"@objectstack/plugin-dev": patch
+---
+
+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)
+
+DevPlugin's security warning probed `security.permissions` / `security.rls` /
+`security.fieldMasker` from `init()`. Both halves were wrong, and wrong in the
+direction that is hardest to notice — silence read as health.
+
+**Wrong signal.** Those three are `SecurityPlugin.init()` registrations. The
+`ISecurityService` contract in `@objectstack/spec` names them "implementation
+internals and deliberately NOT part of this contract"; the published `security`
+service is the contract. Their presence answers "is SecurityPlugin loaded?",
+not "is anything being enforced?" — and the two answers come apart at
+`SecurityPlugin.start()`, which returns early (when `objectql`/`metadata` will
+not resolve, and when the engine cannot take middleware) **before** it publishes
+`security` and before it registers a single enforcement middleware. A stack in
+that state holds all three internal handles and enforces nothing, so the warning
+stayed silent in the one state where its own text is literally true.
+
+**Wrong phase.** `security` is registered in `SecurityPlugin.start()`, which
+DevPlugin runs in its own `start()`. Probing it from `init()` would find it
+absent on *every* stack, healthy ones included — so swapping only the service
+name turns a false negative into a permanent false positive. The check now runs
+after the child-start loop, in the boot banner an operator actually reads
+(the placement #3900 already established for the production-override brand).
+
+Observable behaviour change, both directions:
+
+- A stack whose `SecurityPlugin.start()` bailed now gets a warning that names
+ that state ("LOADED but did not finish starting"), where it previously got
+ silence. The internal handles keep their one honest use — telling "never
+ loaded" apart from "loaded, then failed to start" — so the operator is
+ pointed at the right fix.
+- The absent-plugin warning is unchanged in meaning and wording, but is now
+ emitted from `start()` rather than `init()`.
+
+This is the same move #10035 made for the other consumer this signal misled
+(`plugin-hono-server`'s `/auth/me/permissions` and `/me/apps`). Two consumers,
+two packages, one misread — a property of the signal, not of either reader.

packages/plugins/plugin-dev/src/dev-plugin-security-enforcement-warning.test.ts

@@ -0,0 +1,147 @@
+import { describe, it, expect, vi } from 'vitest';
+import { DevPlugin } from './dev-plugin';
+
+// [#10036] The state under test is "SecurityPlugin LOADED but its start()
+// bailed", so `@objectstack/plugin-security` is deliberately NOT mocked here —
+// the real plugin's real `init()`/`start()` phase split is what constructs the
+// state. Every OTHER optional dependency is mocked away for the same reason as
+// #3060 (their vite transforms alone can blow the timeout), and their absence
+// is irrelevant to this file's subject.
+//
+// One FURTHER omission, measured rather than assumed:
+// `@objectstack/driver-memory` is deliberately not mocked here either,
+// because DevPlugin imports
+// `@objectstack/runtime` on the line BEFORE it and that import throws, so the
+// driver import is never evaluated and a mock for it would be dead weight. It
+// is a frozen driver under a retirement census (#5499/#5704/#6664), where an
+// unnecessary module binding is the defect the census exists to catch, so the
+// dead mock is not harmless bookkeeping. Probed, not reasoned: a marker in the
+// factory printed 0 times across the whole file while the same marker in the
+// `@objectstack/runtime` factory printed 8 times in the same run. ⛔ Do not add
+// one back.
+vi.mock('@objectstack/objectql', () => { throw Object.assign(new Error("Cannot find package '@objectstack/objectql'"), { code: 'ERR_MODULE_NOT_FOUND' }); });
+vi.mock('@objectstack/runtime', () => { throw Object.assign(new Error("Cannot find package '@objectstack/runtime'"), { code: 'ERR_MODULE_NOT_FOUND' }); });
+vi.mock('@objectstack/service-i18n', () => { throw Object.assign(new Error("Cannot find package '@objectstack/service-i18n'"), { code: 'ERR_MODULE_NOT_FOUND' }); });
+vi.mock('@objectstack/service-storage', () => { throw Object.assign(new Error("Cannot find package '@objectstack/service-storage'"), { code: 'ERR_MODULE_NOT_FOUND' }); });
+vi.mock('@objectstack/service-realtime', () => { throw Object.assign(new Error("Cannot find package '@objectstack/service-realtime'"), { code: 'ERR_MODULE_NOT_FOUND' }); });
+vi.mock('@objectstack/plugin-auth', () => { throw Object.assign(new Error("Cannot find package '@objectstack/plugin-auth'"), { code: 'ERR_MODULE_NOT_FOUND' }); });
+vi.mock('@objectstack/plugin-hono-server', () => { throw Object.assign(new Error("Cannot find package '@objectstack/plugin-hono-server'"), { code: 'ERR_MODULE_NOT_FOUND' }); });
+vi.mock('@objectstack/rest', () => { throw Object.assign(new Error("Cannot find package '@objectstack/rest'"), { code: 'ERR_MODULE_NOT_FOUND' }); });
+vi.mock('@objectstack/setup', () => { throw Object.assign(new Error("Cannot find package '@objectstack/setup'"), { code: 'ERR_MODULE_NOT_FOUND' }); });
+vi.mock('@objectstack/account', () => { throw Object.assign(new Error("Cannot find package '@objectstack/account'"), { code: 'ERR_MODULE_NOT_FOUND' }); });
+
+function mockCtx() {
+ const registeredServices = new Map<string, any>();
+ const ctx: any = {
+ logger: { info: vi.fn(), debug: vi.fn(), warn: vi.fn(), error: vi.fn() },
+ getService: vi.fn().mockImplementation((name: string) => {
+ if (registeredServices.has(name)) return registeredServices.get(name);
+ throw new Error(`service '${name}' not found`);
+ }),
+ getServices: vi.fn().mockReturnValue(new Map()),
+ registerService: vi.fn().mockImplementation((name: string, svc: any) => {
+ registeredServices.set(name, svc);
+ }),
+ hook: vi.fn(),
+ trigger: vi.fn(),
+ getKernel: vi.fn(),
+ };
+ // SecurityPlugin.init() contributes to the manifest; without this its init()
+ // throws midway and the state we want to construct is only half-built.
+ registeredServices.set('manifest', { register: () => {} });
+ return { ctx, registeredServices };
+}
+
+/** Every `logger.warn` line that claims security is not being enforced. */
+function enforcementWarnings(ctx: any): string[] {
+ return ctx.logger.warn.mock.calls
+ .map((call: any[]) => (typeof call[0] === 'string' ? call[0] : ''))
+ .filter((msg: string) => msg.includes('NOT enforced'));
+}
+
+async function boot(ctx: any, options: Record<string, unknown> = {}) {
+ const plugin = new DevPlugin({ seedAdminUser: false, ...options });
+ await plugin.init(ctx);
+ await plugin.start(ctx);
+ return plugin;
+}
+
+describe('[#10036] the "nothing is enforced" warning must fire when SecurityPlugin.start() bailed', () => {
+ // ── The state the warning describes, constructed for real ───────────────
+ //
+ // SecurityPlugin registers `security.permissions` / `security.rls` /
+ // `security.fieldMasker` in `init()` and the published `security` service
+ // (plus every enforcement middleware) only in `start()`, which returns early
+ // when the engine cannot take middleware. So a stack can hold all three
+ // internal handles while NOTHING is enforced — and that is precisely the
+ // state the warning's own text describes.
+
+ it('bail #1 (no objectql/metadata service): the three init() handles resolve, `security` does not, and the warning fires', async () => {
+ const { ctx, registeredServices } = mockCtx();
+ // No `objectql` service at all → SecurityPlugin.start() takes its FIRST
+ // early return.
+ await boot(ctx);
+
+ // Precondition — the state really is the one the card describes.
+ expect(registeredServices.has('security.permissions'), 'SecurityPlugin.init() ran').toBe(true);
+ expect(registeredServices.has('security.rls')).toBe(true);
+ expect(registeredServices.has('security.fieldMasker')).toBe(true);
+ expect(registeredServices.has('security'), 'start() bailed before publishing the service').toBe(false);
+
+ // The real plugin logged its own bail, from inside itself.
+ const bail = ctx.logger.warn.mock.calls.find(
+ (call: any[]) => typeof call[0] === 'string' && call[0].includes('security middleware not registered'),
+ );
+ expect(bail, 'SecurityPlugin.start() must have bailed').toBeDefined();
+
+ // …and the dev assembly says so out loud.
+ const warnings = enforcementWarnings(ctx);
+ expect(warnings.length, 'the dev assembly must warn that nothing is enforced').toBe(1);
+ expect(warnings[0]).toContain('LOADED');
+ });
+
+ it('bail #2 (engine cannot take middleware): same — handles present, no enforcement, warning fires', async () => {
+ const { ctx, registeredServices } = mockCtx();
+ // An engine that resolves but has no `registerMiddleware` → SecurityPlugin
+ // .start() takes its SECOND early return.
+ registeredServices.set('objectql', { find: () => [] });
+ registeredServices.set('metadata', { list: () => [] });
+
+ await boot(ctx);
+
+ expect(registeredServices.has('security.permissions')).toBe(true);
+ expect(registeredServices.has('security')).toBe(false);
+
+ const bail = ctx.logger.warn.mock.calls.find(
+ (call: any[]) => typeof call[0] === 'string' && call[0].includes('does not support middleware'),
+ );
+ expect(bail, 'the engine-cannot-take-middleware bail must be the one taken').toBeDefined();
+
+ const warnings = enforcementWarnings(ctx);
+ expect(warnings.length).toBe(1);
+ expect(warnings[0]).toContain('LOADED');
+ });
+
+ it('does not fire when SecurityPlugin.start() completed and published the `security` service', async () => {
+ const { ctx, registeredServices } = mockCtx();
+ // An engine that CAN take middleware → start() runs to completion and
+ // registers the published `security` service alongside the middleware.
+ const registerMiddleware = vi.fn();
+ registeredServices.set('objectql', { registerMiddleware, find: () => [] });
+ registeredServices.set('metadata', { list: () => [], get: () => undefined });
+
+ await boot(ctx);
+
+ // Precondition — this really is the healthy state.
+ expect(registeredServices.has('security'), 'start() published the service').toBe(true);
+ expect(registerMiddleware, 'enforcement middleware was installed').toHaveBeenCalled();
+
+ expect(enforcementWarnings(ctx)).toEqual([]);
+ });
+
+ it('stays silent when the operator disabled security explicitly', async () => {
+ const { ctx } = mockCtx();
+ await boot(ctx, { services: { security: false } });
+ expect(enforcementWarnings(ctx)).toEqual([]);
+ });
+});

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

@@ -85,7 +85,14 @@ describe('DevPlugin', () => {
  it('registers no service implementation of its own — every unfilled slot stays empty', async () => {
  const { ctx, registeredServices } = mockCtx();
 
- await new DevPlugin({ seedAdminUser: false }).init(ctx);
+ const plugin = new DevPlugin({ seedAdminUser: false });
+ await plugin.init(ctx);
+ // [#10036] `start()` too: the "nothing is enforcing security" warning
+ // asserted at the bottom of this test moved to the start phase, because
+ // `security` — the published service that means enforcement, as opposed
+ // to the `init()`-registered internals that only mean "plugin loaded" —
+ // is not registered until SecurityPlugin.start() has run.
+ await plugin.start(ctx);
 
  // Not one slot was filled by this plugin itself.
  expect(ctx.registerService).not.toHaveBeenCalled();
@@ -116,6 +123,9 @@ describe('DevPlugin', () => {
  (call: any[]) => typeof call[0] === 'string' && call[0].includes('NOT enforced'),
  );
  expect(securityWarn).toBeDefined();
+ // …and with the plugin genuinely absent it says so, rather than reporting
+ // the loaded-but-failed-to-start state (#10036).
+ expect(securityWarn![0]).toContain('SecurityPlugin is not loaded');
  });
 
  describe('production guard (ADR-0115 D6)', () => {

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

@@ -888,24 +888,10 @@ export class DevPlugin implements Plugin {
  // in-process consumers handle absence exactly as they already must in
  // production. To use a capability locally, install its real service.
 
- // The security slots deserve one loud line when empty (#4126): faking an
- // authorization decision is the one thing ADR-0076 D12 forbids a fallback
- // to do, so the slots stay empty rather than stubbed — but "no RBAC/RLS/
- // masking is being enforced" is worth saying in the boot log of a stack
- // that expected them.
- if (enabled('security')) {
- const missing = ['security.permissions', 'security.rls', 'security.fieldMasker'].filter((svc) => {
- try { ctx.getService(svc); return false; } catch { return true; }
- });
- if (missing.length > 0) {
- ctx.logger.warn(
- ` ⚠ No security services (${missing.join(', ')}) — SecurityPlugin is not loaded, so RBAC, `
- + 'row-level security and field masking are NOT enforced. The slots stay empty rather than '
- + 'being stubbed: a fake that answers "allowed" is worse than an absent one. Install '
- + '@objectstack/plugin-security to enforce them.',
- );
- }
- }
+ // The security slots deserve one loud line when nothing is enforcing
+ // (#4126) — but that question cannot be answered HERE. It is asked in
+ // `start()` instead; see `warnIfNothingIsEnforcingSecurity` below for why
+ // the phase, and not just the service name, is load-bearing.
 
  ctx.logger.info(`DevPlugin initialized ${this.childPlugins.length} plugin(s)`);
  }
@@ -950,13 +936,86 @@ export class DevPlugin implements Plugin {
  + 'this is NOT a production stack',
  );
  }
+ // Same reasoning, same surface: "nothing is enforcing security" belongs
+ // next to the banner, not buried in the init log (#10036, #3900).
+ this.warnIfNothingIsEnforcingSecurity(ctx);
  ctx.logger.info('');
  ctx.logger.info(' API: /api/v1/data/:object');
  ctx.logger.info(' Metadata: /api/v1/meta/:type/:name');
  ctx.logger.info(' Discovery: /.well-known/objectstack');
  ctx.logger.info('─────────────────────────────────────────');
  }
 
+ /**
+ * One loud line when the stack is enforcing no security at all (#4126,
+ * ADR-0076 D12: a fake that answers "allowed" is worse than an absent one,
+ * so the slots stay empty — but silence about unenforced RBAC/RLS/masking
+ * would be its own kind of fake).
+ *
+ * ## Why this asks for `security`, and why it asks in `start()` (#10036)
+ *
+ * This used to probe `security.permissions` / `security.rls` /
+ * `security.fieldMasker` from `init()`. Both halves of that were wrong, and
+ * they were wrong in the direction that is hardest to notice — silence read
+ * as health:
+ *
+ * - **Wrong signal.** Those three are `SecurityPlugin.init()` registrations.
+ * The `ISecurityService` contract in `@objectstack/spec` names them
+ * "implementation internals and deliberately NOT part of this contract";
+ * the published `security` service is the contract. Their presence answers
+ * "is SecurityPlugin loaded?", which is not the question this warning
+ * asks. The two answers come apart at `start()`: it returns early — when
+ * `objectql`/`metadata` will not resolve, and when the engine cannot take
+ * middleware — BEFORE it publishes `security` and before it registers a
+ * single enforcement middleware. A stack in that state holds all three
+ * internal handles and enforces nothing, so the warning stayed silent in
+ * the one state where its text is literally true. (The same presence
+ * signal misled `plugin-hono-server`'s `/auth/me/permissions`, fixed in
+ * #10035 by this same move — two consumers, two packages, one misread:
+ * that is a property of the signal, not of either reader.)
+ *
+ * - **Wrong phase.** `security` is registered in `SecurityPlugin.start()`,
+ * which this plugin runs in its OWN `start()`. Asking from `init()` would
+ * find it absent on every stack, healthy ones included — so swapping only
+ * the service name would have turned a false negative into a permanent
+ * false positive. The question is answerable only after the child-start
+ * loop has run.
+ *
+ * The internal handles keep exactly one honest use, and it is the one they
+ * can support: telling "SecurityPlugin was never loaded" apart from
+ * "SecurityPlugin loaded and then failed to start", so the operator is
+ * pointed at the right fix.
+ */
+ private warnIfNothingIsEnforcingSecurity(ctx: PluginContext): void {
+ if (this.options.services?.['security'] === false) return; // opted out
+
+ // An absent slot may throw OR resolve to undefined depending on the
+ // kernel; both mean "nothing is there".
+ const resolves = (name: string): boolean => {
+ try { return ctx.getService(name) != null; } catch { return false; }
+ };
+
+ if (resolves('security')) return; // enforcement middleware is installed
+
+ const loadedButNotEnforcing = ['security.permissions', 'security.rls', 'security.fieldMasker']
+ .some(resolves);
+
+ ctx.logger.warn(
+ loadedButNotEnforcing
+ ? ' ⚠ SecurityPlugin is LOADED but did not finish starting — it published no `security` '
+ + 'service, so no enforcement middleware was registered and RBAC, row-level security and '
+ + 'field masking are NOT enforced. Its own start() warning above says why (the objectql or '
+ + 'metadata service could not be resolved, or the engine does not accept middleware). The '
+ + '`security.permissions` / `security.rls` / `security.fieldMasker` handles ARE present — '
+ + 'they are registered in init() and mean the plugin loaded, never that anything is being '
+ + 'enforced.'
+ : ' ⚠ No `security` service — SecurityPlugin is not loaded, so RBAC, row-level security '
+ + 'and field masking are NOT enforced. The slots stay empty rather than being stubbed: a '
+ + 'fake that answers "allowed" is worse than an absent one. Install '
+ + '@objectstack/plugin-security to enforce them.',
+ );
+ }
+
  /**
  * Destroy Phase
  *