From b366c6e7f04e67b9151baa55df83fd21377baf6a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 01:02:26 +0000 Subject: [PATCH 1/2] 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 Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r --- ...dev-plugin-security-enforcement-warning.md | 41 ++++++ ...lugin-security-enforcement-warning.test.ts | 136 ++++++++++++++++++ .../plugins/plugin-dev/src/dev-plugin.test.ts | 12 +- packages/plugins/plugin-dev/src/dev-plugin.ts | 95 +++++++++--- 4 files changed, 265 insertions(+), 19 deletions(-) create mode 100644 .changeset/dev-plugin-security-enforcement-warning.md create mode 100644 packages/plugins/plugin-dev/src/dev-plugin-security-enforcement-warning.test.ts diff --git a/.changeset/dev-plugin-security-enforcement-warning.md b/.changeset/dev-plugin-security-enforcement-warning.md new file mode 100644 index 0000000000..1dc792cbd3 --- /dev/null +++ b/.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. diff --git a/packages/plugins/plugin-dev/src/dev-plugin-security-enforcement-warning.test.ts b/packages/plugins/plugin-dev/src/dev-plugin-security-enforcement-warning.test.ts new file mode 100644 index 0000000000..c4598c51eb --- /dev/null +++ b/packages/plugins/plugin-dev/src/dev-plugin-security-enforcement-warning.test.ts @@ -0,0 +1,136 @@ +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. +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/driver-memory', () => { throw Object.assign(new Error("Cannot find package '@objectstack/driver-memory'"), { 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(); + 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 = {}) { + 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([]); + }); +}); diff --git a/packages/plugins/plugin-dev/src/dev-plugin.test.ts b/packages/plugins/plugin-dev/src/dev-plugin.test.ts index f7b7309164..6d539ae9d7 100644 --- a/packages/plugins/plugin-dev/src/dev-plugin.test.ts +++ b/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)', () => { diff --git a/packages/plugins/plugin-dev/src/dev-plugin.ts b/packages/plugins/plugin-dev/src/dev-plugin.ts index cb05409182..83fc932a7b 100644 --- a/packages/plugins/plugin-dev/src/dev-plugin.ts +++ b/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,6 +936,9 @@ 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'); @@ -957,6 +946,76 @@ export class DevPlugin implements Plugin { 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 * From 897f8350626027f9cc94fc7bc0240cd6a092516a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 01:33:07 +0000 Subject: [PATCH 2/2] fix(plugin-dev): drop the dead `@objectstack/driver-memory` mock from the new test (#10036) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r --- .../dev-plugin-security-enforcement-warning.test.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/plugins/plugin-dev/src/dev-plugin-security-enforcement-warning.test.ts b/packages/plugins/plugin-dev/src/dev-plugin-security-enforcement-warning.test.ts index c4598c51eb..3dc5e0ad08 100644 --- a/packages/plugins/plugin-dev/src/dev-plugin-security-enforcement-warning.test.ts +++ b/packages/plugins/plugin-dev/src/dev-plugin-security-enforcement-warning.test.ts @@ -7,9 +7,20 @@ import { DevPlugin } from './dev-plugin'; // 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/driver-memory', () => { throw Object.assign(new Error("Cannot find package '@objectstack/driver-memory'"), { 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' }); });