diff --git a/.changeset/slot-lookup-sweep-auth-session.md b/.changeset/slot-lookup-sweep-auth-session.md new file mode 100644 index 0000000000..34c9530d73 --- /dev/null +++ b/.changeset/slot-lookup-sweep-auth-session.md @@ -0,0 +1,61 @@ +--- +"@objectstack/spec": patch +"@objectstack/plugin-auth": patch +"@objectstack/plugin-hono-server": patch +"@objectstack/plugin-security": patch +--- + +fix(spec,plugins): sweep the auth/session slot lookups — 31 sites typed, and the user-import metadata reader was pointed at a service that never had the method (#4251) + +Batch B2 of the #4251 sweep: every service-lookup erasure in the auth/session +family. `plugin-auth/auth-plugin.ts` (20), `plugin-hono-server/current-user-endpoints.ts` +(10) and `plugin-security/security-plugin.ts` (1) now pass the slot's contract +type; the ratchet baseline drops **171 → 140 sites, 40 → 37 files**. + +**The yield.** `POST /admin/import-users` resolved the `metadata` slot and probed +`metadataService?.getMetaItem` to decide whether to pass the import's field-coercion +dependency. `getMetaItem` is a **protocol** method — `ObjectStackProtocolImplementation`, +registered by MetadataProtocolPlugin under the `protocol` slot. `MetadataManager`, +which occupies `metadata`, has never had it. So the probe was false on every +deployment and the dep was never passed: imported rows reached `sys_user` +uncoerced, with the branch that says otherwise sitting right there. This is the +same shape as #4127's dead `automation.trigger` and #4321's `registerInMemory` +probes — a capability the code advertises and the runtime cannot deliver, kept +invisible by the `any`. Typing the lookup to `IMetadataService` is what turned it +into a compile error. The route reads `protocol` now. + +`/me/apps` reached ObjectQL's **private** `_registry` through `as any` while +`/auth/me/permissions`, two handlers up in the same file, read the public +`registry` getter over the same field of the same object. Both read the public +accessor now; the one test that stubbed `_registry` was pinning the private reach +and stubs `registry` instead. + +**Contract, from evidence.** `IDataEngine`'s read methods (`find` / `findOne` / +`count` / `aggregate`) declare the trailing `options?: BaseEngineOptions` +argument they have always accepted. ObjectQL's own doc explains why it exists: +reads once took their context inside the query while writes took it in trailing +`options.context`, so the same `{ context }` object was correct as `insert`'s 3rd +argument and **silently dropped** as `find`'s — "an intended `isSystem` bypass +just vanished". The engine accepts both channels; the contract exposed only the +query one, so callers using the trailing channel — the current-user endpoints' +permission-set loader among them — could only reach it by erasing the lookup. +Adding an optional trailing parameter breaks no implementor (the existing +minimal-implementation test proves it) and no caller. `BaseEngineOptions` was +already exported, sitting unused under the "legacy/deprecated" heading, which is +why the contract went looking and did not find it; it moves up beside the other +QueryAST-aligned types with the rationale attached. One new spec test pins the +trailing argument at the call site — the position where the old contract rejected it. + +**Where the contract does not reach, the escape hatch is named.** Three slots +resist a spec type today and each gets a narrow, documented local interface +instead of `any`: `security.permissions` (plugin-security's `PermissionEvaluator` +— plugin-hono-server must not depend on an optional plugin), `settings` +(service-settings' resolver, same reason), and ObjectQL beyond `IDataEngine` +(`registry` / `getSchema` / `registerHook` / `registerMiddleware`). That last one +is deliberate scope: the standing record on `getObjectQL` in `@objectstack/runtime` +says ObjectQL is genuinely wider than `IDataEngine` and nobody has written the +wider contract, so typing the whole thing `IDataEngine` would be "the more +comfortable-looking lie". These declarations are what that contract gets written +from, and what it deletes. + +No behavior changes beyond the two fixes above. diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index a34867707b..0628721199 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -2,7 +2,14 @@ import { Plugin, PluginContext, IHttpServer } from '@objectstack/core'; import type { BetterAuthOptions } from 'better-auth'; -import { AuthConfig, type SocialProviderConfig, SystemObjectName, SystemUserId } from '@objectstack/spec/system'; +import { + AuthConfig, + type SocialProviderConfig, + type SettingsChangeHandler, + type SettingsUnsubscribe, + SystemObjectName, + SystemUserId, +} from '@objectstack/spec/system'; import { // ADR-0048 — the Setup/Studio/Account apps moved to their own packages // (@objectstack/{setup,studio,account}); plugin-auth no longer registers them. @@ -12,6 +19,7 @@ import { import { SysOrganizationDetailPage, SysUserDetailPage } from '@objectstack/platform-objects/pages'; import { resolveTenancyPosture } from '@objectstack/types'; import { postureEnforcesWall, type OrgScopingEntitlement } from '@objectstack/spec/security'; +import type { IDataEngine, IEmailService, ISmsService } from '@objectstack/spec/contracts'; import { AuthManager, resolveOidcProviderEnabled, @@ -36,6 +44,87 @@ import { authPluginManifestHeader, } from './manifest.js'; +/** + * The `objectql` slot BEYOND `IDataEngine` — the hook and middleware seams this + * plugin installs on the engine. + * + * [#4251] The slot's ledger entry is `IDataEngine` and it covers every read and + * write below; it does not cover `registerHook` / `registerMiddleware`, and no + * contract has been written for the wider ObjectQL surface yet (the standing + * record of why is on `getObjectQL` in `@objectstack/runtime`'s + * `DomainHandlerContext`). Declared here, narrow and named, so the extension is + * legible instead of hidden under `any` — and so it is deleted, not migrated, + * when that contract lands. + * + * Both members are optional and every call site already guards with + * `typeof … === 'function'`: the slot is satisfiable by engines that implement + * neither (mock mode), and this plugin degrades rather than fails there. + */ +interface EngineExtensionSurface { + registerHook?( + event: string, + handler: (context: any) => Promise | void, + options?: { object?: string | string[]; priority?: number; packageId?: string }, + ): void; + registerMiddleware?( + fn: (opCtx: any, next: () => Promise) => Promise, + options?: { object?: string }, + ): void; +} + +/** The engine as this plugin uses it: the data contract plus those two seams. */ +type AuthEngine = IDataEngine & EngineExtensionSurface; + +/** + * The `settings` slot, as this plugin reads it. + * + * [#4251] `service-settings` registers its `SettingsService` here and the slot + * carries no `packages/spec` contract, so this declares the two resolver methods + * this plugin calls. Structural on purpose — plugin-auth must not depend on + * service-settings, which is optional (both readers below already return early + * when the slot is empty or the method is missing). + */ +interface SettingsReadSurface { + /** Resolve one key; `source` distinguishes a stored/env value from a manifest default. */ + get( + namespace: string, + key: string, + ctx?: Record, + ): Promise<{ value?: unknown; source?: string } | undefined>; + /** Resolve a whole namespace as `key → { value, source }`. */ + getNamespace( + namespace: string, + ctx?: Record, + ): Promise<{ values: Record }>; + /** + * Re-run a binding when the namespace changes — how the brand name, SMS + * locale and auth namespace stay live without a redeploy. Optional: an + * implementation without a change bus leaves the initial read in place, which + * is what the `typeof … === 'function'` guards at each call site already say. + * + * The handler and unsubscribe types are the SPEC's + * (`@objectstack/spec/system`), not re-declared here — the change bus has a + * published contract even though the slot does not. + */ + subscribe?( + namespace: string | undefined, + handler: SettingsChangeHandler, + ): SettingsUnsubscribe; +} + +/** + * The `protocol` slot's metadata reader, as the user-import route uses it. + * + * [#4251] `protocol` is a deliberately UNCONTRACTED slot (see + * `UNCONTRACTED_SLOTS` in `eslint.config.mjs`), so the one method this route + * needs is declared here rather than erased. It reads `sys_user`'s field + * definitions so imported values are coerced to their declared types — + * best-effort by contract: without it the import still runs, uncoerced. + */ +interface MetaItemReaderSurface { + getMetaItem(ref: { type: string; name: string }): Promise; +} + /** * Auth Plugin Options * Extends AuthConfig from spec with additional runtime options @@ -222,7 +311,7 @@ export class AuthPlugin implements Plugin { // that advertises a tolerance the composition forbids is worse than no // branch — it reads as a supported degraded mode (#4187). AuthManager keeps // its own `dataEngine?` guards because it is usable outside this plugin. - const dataEngine = ctx.getService('data'); + const dataEngine = ctx.getService('data'); const authConfig: AuthManagerOptions & AuthPluginOptions = { ...this.options, @@ -451,8 +540,8 @@ export class AuthPlugin implements Plugin { if (this.authManager) { await this.bindAuthSettings(ctx); - let emailSvc: any; - try { emailSvc = ctx.getService('email'); } catch { emailSvc = undefined; } + let emailSvc: IEmailService | undefined; + try { emailSvc = ctx.getService('email'); } catch { emailSvc = undefined; } if (emailSvc) { this.authManager.setEmailService(emailSvc); ctx.logger.info('Auth: email service wired (transactional mail enabled)'); @@ -480,8 +569,8 @@ export class AuthPlugin implements Plugin { // SMS-invite path can deliver. Same lazy-resolution contract as // the email service: absent ⇒ OTP endpoints keep failing loudly // (NOT_SUPPORTED) while phone+password sign-in still works. - let smsSvc: any; - try { smsSvc = ctx.getService('sms'); } catch { smsSvc = undefined; } + let smsSvc: ISmsService | undefined; + try { smsSvc = ctx.getService('sms'); } catch { smsSvc = undefined; } if (smsSvc) { this.authManager.setSmsService(smsSvc); if (this.authManager.isPhoneNumberEnabled()) { @@ -503,7 +592,7 @@ export class AuthPlugin implements Plugin { // the override so the deployment's `appName` (e.g. `OS_APP_NAME`) // keeps precedence. Mirrors EmailServicePlugin's settings binding. try { - const settings = ctx.getService('settings'); + const settings = ctx.getService('settings'); if (settings && typeof settings.get === 'function') { const applyBrand = async () => { try { @@ -628,7 +717,7 @@ export class AuthPlugin implements Plugin { // whose rows already carry an issuer costs one empty query. ctx.hook('kernel:ready', async () => { try { - const ql: any = ctx.getService('objectql'); + const ql = ctx.getService('objectql'); if (!ql) return; const { backfillAccountIssuer } = await import('./backfill-account-issuer.js'); await backfillAccountIssuer(ql, { @@ -656,7 +745,7 @@ export class AuthPlugin implements Plugin { if (this.options.autoDefaultOrganization !== false && !postureEnforcesWall(resolveTenancyPosture())) { const runEnsure = async () => { try { - const ql: any = ctx.getService('objectql'); + const ql = ctx.getService('objectql'); if (!ql) return; const res = await ensureDefaultOrganization(ql, { logger: ctx.logger }); if (res.defaultOrgCreated) { @@ -675,7 +764,7 @@ export class AuthPlugin implements Plugin { // to platform admin" case where kernel:ready fired before any user // existed (same wiring the multi-org bootstrap uses). try { - const ql: any = ctx.getService('objectql'); + const ql = ctx.getService('objectql'); if (ql && typeof ql.registerMiddleware === 'function') { ql.registerMiddleware(async (opCtx: any, next: () => Promise) => { await next(); @@ -708,7 +797,7 @@ export class AuthPlugin implements Plugin { const runBackfill = (source: string): Promise => { backfillChain = backfillChain.then(async () => { try { - const ql: any = ctx.getService('objectql'); + const ql = ctx.getService('objectql'); const tenancy = this.tenancy; if (!ql || !tenancy) return; const res = await backfillMemberships(ql, { @@ -752,7 +841,7 @@ export class AuthPlugin implements Plugin { try { // Use the kernel's ObjectQL engine (available + hookable at kernel:ready); // the auth manager's getDataEngine() is not yet wired this early. - const engine: any = ctx.getService('objectql'); + const engine = ctx.getService('objectql'); if (!engine || typeof engine.registerHook !== 'function') return; const SYSTEM_CTX = { isSystem: true, roles: [], permissions: [] }; engine.registerHook('afterInsert', async (hookCtx: any) => { @@ -794,7 +883,7 @@ export class AuthPlugin implements Plugin { // bypass — see identity-write-guard.ts for the full contract. ctx.hook('kernel:ready', async () => { try { - const engine: any = ctx.getService('objectql'); + const engine = ctx.getService('objectql'); if (!engine || typeof engine.registerHook !== 'function') return; registerManagedUpdateWhitelist(SystemObjectName.USER, SYS_USER_PROFILE_EDIT_FIELDS); // [ADR-0105 D7] Extension fields ObjectStack adds to better-auth-managed @@ -821,7 +910,7 @@ export class AuthPlugin implements Plugin { // Register auth middleware on ObjectQL engine (if available) try { - const ql = ctx.getService('objectql'); + const ql = ctx.getService('objectql'); if (ql && typeof ql.registerMiddleware === 'function') { ql.registerMiddleware(async (opCtx: any, next: () => Promise) => { // If context already has userId or isSystem, skip auth resolution @@ -850,9 +939,9 @@ export class AuthPlugin implements Plugin { private async bindAuthSettings(ctx: PluginContext): Promise { if (!this.authManager) return; - let settings: any; + let settings: SettingsReadSurface | undefined; try { - settings = ctx.getService('settings'); + settings = ctx.getService('settings'); } catch { return; } @@ -1136,8 +1225,8 @@ export class AuthPlugin implements Plugin { const password = process.env.OS_SEED_ADMIN_PASSWORD?.trim() || 'admin123'; const name = process.env.OS_SEED_ADMIN_NAME?.trim() || 'Dev Admin'; - let ql: any; - try { ql = ctx.getService('objectql'); } catch { /* unavailable */ } + let ql: IDataEngine | undefined; + try { ql = ctx.getService('objectql'); } catch { /* unavailable */ } if (!ql || typeof ql.find !== 'function') return; try { @@ -1627,15 +1716,24 @@ export class AuthPlugin implements Plugin { const actor = await gateAdmin(c); if (actor instanceof Response) return actor; const { runAdminImportUsers } = await import('./admin-import-users.js'); - const metadataService: any = (() => { - try { return ctx.getService?.('metadata'); } catch { return undefined; } + // [#4251] Resolved from `protocol`, not `metadata`. `getMetaItem` is a + // PROTOCOL method (`ObjectStackProtocolImplementation`, registered by + // MetadataProtocolPlugin); the `metadata` slot holds MetadataManager, + // which has never had it. So `metadataService?.getMetaItem` was always + // falsy and the field-coercion dep was never passed — an import row's + // values reached `sys_user` uncoerced, with the branch that says + // otherwise sitting right here. Invisible while the lookup was `any`: + // typing it to the slot's contract is what turned the dead probe into + // a compile error. + const metaReader = (() => { + try { return ctx.getService?.('protocol'); } catch { return undefined; } })(); const { status, body } = await runAdminImportUsers( { getAuthApi: () => this.authManager!.getApi() as any, getDataEngine: () => this.authManager!.getDataEngine(), - ...(metadataService?.getMetaItem - ? { getMetaItem: (ref: { type: string; name: string }) => metadataService.getMetaItem(ref) } + ...(typeof metaReader?.getMetaItem === 'function' + ? { getMetaItem: (ref: { type: string; name: string }) => metaReader.getMetaItem(ref) } : {}), phoneNumberEnabled: () => this.authManager!.isPhoneNumberEnabled(), emailServiceAvailable: () => this.authManager!.isEmailServiceAvailable(), diff --git a/packages/plugins/plugin-hono-server/src/current-user-endpoints-multi-tenant.test.ts b/packages/plugins/plugin-hono-server/src/current-user-endpoints-multi-tenant.test.ts index 681ce4f0af..b4ff8dde0b 100644 --- a/packages/plugins/plugin-hono-server/src/current-user-endpoints-multi-tenant.test.ts +++ b/packages/plugins/plugin-hono-server/src/current-user-endpoints-multi-tenant.test.ts @@ -96,7 +96,9 @@ describe('the answer comes from the kernel that OWNS the request (cloud#927)', ( it('routes /auth/me/localization and /me/apps through the same resolution', async () => { const envKernel = kernelWith({ auth: authFor('usr_env'), - objectql: { _registry: { getAllApps: () => [{ name: 'env_app' }] } }, + // [#4251] `registry`, the public accessor — this stub pinned the + // private `_registry` the handler used to reach through `as any`. + objectql: { registry: { getAllApps: () => [{ name: 'env_app' }] } }, }); const { app } = mount({}, async () => envKernel); diff --git a/packages/plugins/plugin-hono-server/src/current-user-endpoints.ts b/packages/plugins/plugin-hono-server/src/current-user-endpoints.ts index 94a58ed672..9085197655 100644 --- a/packages/plugins/plugin-hono-server/src/current-user-endpoints.ts +++ b/packages/plugins/plugin-hono-server/src/current-user-endpoints.ts @@ -45,7 +45,7 @@ import { type EnableLike, } from '@objectstack/spec/data'; import type { ExecutionContext } from '@objectstack/spec/kernel'; -import type { Logger } from '@objectstack/spec/contracts'; +import type { IAuthService, IMetadataService, Logger } from '@objectstack/spec/contracts'; import { allowPerfDisclosure, isPerfDisclosurePrincipal } from '@objectstack/observability'; /** API prefix these endpoints mount under unless the host overrides it. */ @@ -261,6 +261,71 @@ export function foldWildcardSuperUser(objects: Record): void { } } +/** + * The `objectql` slot BEYOND `IDataEngine` — the schema registry these + * endpoints read. + * + * [#4251] The slot's ledger entry is `IDataEngine` (ObjectQL registers the SAME + * instance under `data` and `objectql`), and that covers the reads below. It + * does NOT cover `registry` / `getSchema`, and the honest record of why lives + * on `getObjectQL` in `@objectstack/runtime`'s `DomainHandlerContext`: ObjectQL + * is genuinely wider than `IDataEngine`, nobody has written a contract for the + * wider part, and typing the whole thing `IDataEngine` would be "the more + * comfortable-looking lie". So the extra surface is declared here, named and + * narrow, instead of the lookup being erased to `any` — the wider contract, when + * someone writes it, absorbs this and the declaration is deleted. + * + * Every member is optional and every call site probes with `?.`: the slot is + * satisfied by engines with no registry at all (test fakes, remote engines), and + * these endpoints degrade rather than fail when it is absent. + */ +interface EngineRegistrySurface { + /** + * The engine's schema registry — the PUBLIC accessor. ObjectQL exposes it as + * `get registry()` over the private `_registry` field; `_registry` is what + * `/me/apps` used to reach through `as any`, two handlers away from the + * `/auth/me/permissions` reach for the public one, for the same object. + */ + readonly registry?: { + getAllObjects?(): ApiExposureSchemaLike[]; + getAllApps?(): unknown[]; + }; + getSchema?(objectName: string): unknown; +} + +/** + * The `security.permissions` slot, as these two handlers use it. + * + * [#4251] plugin-security registers its `PermissionEvaluator` under this name; + * the slot has no `packages/spec` contract, so this declares the ONE method both + * handlers call rather than erasing the lookup. Structural on purpose — + * plugin-hono-server must not take a runtime dependency on plugin-security, + * which is OPTIONAL in the stacks these endpoints serve (the `!evaluator` + * branches below are exactly its absence). + * + * The parameter types are the loose shapes this caller passes, not the + * evaluator's own: it takes `metadataService: any` and resolves to parsed + * `PermissionSet`s, while the DB loader here yields the projected subset the + * merges below read. Declaring what is passed and read keeps the claim honest. + */ +interface PermissionEvaluatorSurface { + resolvePermissionSets( + identifiers: string[], + metadataService: unknown, + bootstrapPermissionSets?: unknown[], + dbLoader?: (unresolved: string[]) => Promise, + ): Promise; +} + +/** The permission-set fields the two handlers merge out of a resolution. */ +interface ResolvedPermissionSetLike { + name?: string; + objects?: Record; + fields?: Record; + systemPermissions?: unknown; + tabPermissions?: unknown; +} + /** Minimal schema shape the managed-write clamp needs. */ export interface ManagedSchemaLike { managedBy?: string; @@ -426,9 +491,9 @@ export function makeExecutionContextResolver(ctx: CurrentUserEndpointsContext) { // promotion seeded by `bootstrapPlatformAdmin`. const resolveCtx = async (c: any): Promise => { try { - const authService: any = ctx.getService('auth'); + const authService = ctx.getService('auth'); if (!authService) return undefined; - let api: any = authService.api; + let api = authService.api; if (!api && typeof authService.getApi === 'function') { api = await authService.getApi(); } @@ -685,11 +750,12 @@ export function registerCurrentUserEndpoints( // logged as "/auth/me/permissions failed", which reads as a // fault on every console navigation instead of the deliberate // `!evaluator` branch right below. - const metadata: any = (() => { - try { return ctx.getService('metadata'); } catch { return null; } + const metadata = (() => { + try { return ctx.getService('metadata') ?? null; } catch { return null; } })(); - const evaluator: any = (() => { - try { return ctx.getService('security.permissions'); } catch { return null; } + const evaluator = (() => { + try { return ctx.getService('security.permissions') ?? null; } + catch { return null; } })(); const bootstrap: any[] = (() => { try { return ctx.getService('security.bootstrapPermissionSets') ?? []; } @@ -702,8 +768,9 @@ export function registerCurrentUserEndpoints( // DB loader: surfaces user-defined permission sets // (created via the admin UI as `sys_permission_set` // rows) that aren't in metadata or bootstrap. - const ql: any = (() => { - try { return ctx.getService('objectql'); } catch { return null; } + const ql = (() => { + try { return ctx.getService('objectql') ?? null; } + catch { return null; } })(); const dbLoader = ql ? async (names: string[]) => { @@ -763,7 +830,7 @@ export function registerCurrentUserEndpoints( ...(execCtx.positions ?? []), ...(execCtx.permissions ?? []), ]; - let resolved: any[] = await evaluator + let resolved: ResolvedPermissionSetLike[] = await evaluator .resolvePermissionSets(requested, metadata, bootstrap, dbLoader) .catch(() => []); if (resolved.length === 0 && fallbackName) { @@ -831,7 +898,7 @@ export function registerCurrentUserEndpoints( // here must never drop the whole response. try { const allSchemas: ApiExposureSchemaLike[] = (() => { - try { return (ql as any)?.registry?.getAllObjects?.() ?? []; } + try { return ql?.registry?.getAllObjects?.() ?? []; } catch { return []; } })(); seedSuperUserRestrictedObjects(objects, allSchemas); @@ -909,13 +976,17 @@ export function registerCurrentUserEndpoints( try { const byName = new Map(); try { - const registry: any = (ctx.getService('objectql') as any)?._registry; + // The PUBLIC accessor. This read used to reach ObjectQL's + // private `_registry` while `/auth/me/permissions` read the + // `registry` getter over the same field, on the same object, + // in the same file — visible only once both were typed. + const registry = ctx.getService('objectql')?.registry; for (const app of registry?.getAllApps?.() ?? []) { - if (app?.name) byName.set(String(app.name), app); + if ((app as { name?: unknown })?.name) byName.set(String((app as { name: unknown }).name), app); } } catch { /* registry unavailable — fall through to metadata */ } try { - const metadata: any = ctx.getService('metadata'); + const metadata = ctx.getService('metadata'); for (const app of ((await metadata?.list?.('app')) ?? []) as any[]) { if (app?.name && !byName.has(String(app.name))) byName.set(String(app.name), app); } @@ -929,10 +1000,10 @@ export function registerCurrentUserEndpoints( const tabs: Record = { ...((execCtx as any).tabPermissions ?? {}) }; let failOpen = true; try { - const evaluator: any = ctx.getService('security.permissions'); + const evaluator = ctx.getService('security.permissions'); failOpen = !evaluator; if (evaluator) { - const metadata: any = ctx.getService('metadata'); + const metadata = ctx.getService('metadata'); const bootstrap: any[] = (() => { try { return ctx.getService('security.bootstrapPermissionSets') ?? []; } catch { return []; } @@ -945,7 +1016,9 @@ export function registerCurrentUserEndpoints( ...((execCtx as any).positions ?? []), ...((execCtx as any).permissions ?? []), ]; - const qlSvc: any = (() => { try { return ctx.getService('objectql'); } catch { return null; } })(); + const qlSvc = (() => { + try { return ctx.getService('objectql') ?? null; } catch { return null; } + })(); const dbLoader = qlSvc ? async (names: string[]) => { let rows: any; @@ -968,7 +1041,7 @@ export function registerCurrentUserEndpoints( })); } : undefined; - let resolved: any[] = await evaluator + let resolved: ResolvedPermissionSetLike[] = await evaluator .resolvePermissionSets(requested, metadata, bootstrap, dbLoader) .catch(() => []); if (resolved.length === 0 && fallbackName) { diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 1bf8bd29d0..8a4d1eaf10 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -17,6 +17,7 @@ import { intersectFieldMasks, } from './explain-engine.js'; import type { ExplainDecision, ExplainOperation } from '@objectstack/spec/security'; +import type { II18nService } from '@objectstack/spec/contracts'; import { bootstrapDeclaredPositions } from './bootstrap-declared-positions.js'; import { bootstrapDeclaredPermissions, upsertPackagePermissionSet, readDeclared } from './bootstrap-declared-permissions.js'; import { applyManagedWriteDenies } from './managed-object-write-denies.js'; @@ -451,7 +452,7 @@ export class SecurityPlugin implements Plugin { if (typeof (ctx as any).hook === 'function') { (ctx as any).hook('kernel:ready', async () => { try { - const i18n = ctx.getService('i18n'); + const i18n = ctx.getService('i18n'); if (i18n && typeof i18n.loadTranslations === 'function') { const { SecurityTranslations } = await import('./translations/index.js'); for (const [locale, data] of Object.entries(SecurityTranslations)) { diff --git a/packages/spec/src/contracts/data-engine.test.ts b/packages/spec/src/contracts/data-engine.test.ts index 301dd3475f..8c920f200c 100644 --- a/packages/spec/src/contracts/data-engine.test.ts +++ b/packages/spec/src/contracts/data-engine.test.ts @@ -90,6 +90,52 @@ describe('Data Engine Contract', () => { expect(count).toBe(2); }); + it('takes the execution context in a trailing options argument on every read', async () => { + // [#4251] The read methods' 3rd argument. It is what ObjectQL has taken + // since reads and writes were unified on "context goes in the trailing + // options" — passing it as the 3rd arg to `insert` was correct while the + // same object as the 3rd arg to `find` was SILENTLY DROPPED, and an + // intended `isSystem` bypass just vanished. The contract declared only + // `query.context`, so callers reaching the trailing channel had to erase + // the lookup to `any` to do it. Pinned here in the position that matters: + // at the CALL, which is where the old contract rejected it. + const seen: Array<{ method: string; isSystem?: boolean }> = []; + const engine: IDataEngine = { + find: async (_obj, _query, options) => { + seen.push({ method: 'find', isSystem: options?.context?.isSystem }); + return []; + }, + findOne: async (_obj, _query, options) => { + seen.push({ method: 'findOne', isSystem: options?.context?.isSystem }); + return null; + }, + insert: async (_obj, data) => data, + update: async (_obj, data) => data, + delete: async () => ({}), + count: async (_obj, _query, options) => { + seen.push({ method: 'count', isSystem: options?.context?.isSystem }); + return 0; + }, + aggregate: async (_obj, _query, options) => { + seen.push({ method: 'aggregate', isSystem: options?.context?.isSystem }); + return []; + }, + }; + + const sys = { context: { isSystem: true } }; + await engine.find('sys_permission_set', { where: {} }, sys); + await engine.findOne('sys_user', { where: {} }, sys); + await engine.count('sys_account', { where: {} }, sys); + await engine.aggregate('sys_user', {} as any, sys); + + expect(seen).toEqual([ + { method: 'find', isSystem: true }, + { method: 'findOne', isSystem: true }, + { method: 'count', isSystem: true }, + { method: 'aggregate', isSystem: true }, + ]); + }); + it('should support optional vectorFind', async () => { const engine: IDataEngine = { find: async () => [], diff --git a/packages/spec/src/contracts/data-engine.ts b/packages/spec/src/contracts/data-engine.ts index f24b6e0344..571d57704d 100644 --- a/packages/spec/src/contracts/data-engine.ts +++ b/packages/spec/src/contracts/data-engine.ts @@ -1,6 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { + BaseEngineOptions, EngineQueryOptions, DataEngineInsertOptions, EngineUpdateOptions, @@ -50,13 +51,29 @@ export interface WriteObservabilityOptions { */ export interface IDataEngine { - find(objectName: string, query?: EngineQueryOptions): Promise; - findOne(objectName: string, query?: EngineQueryOptions): Promise; + /** + * Read methods take the execution context in a TRAILING options argument, + * the same position the write methods take theirs. + * + * [#4251] Declared because the implementation and its callers were already + * there: ObjectQL's `find`/`findOne`/`count`/`aggregate` have taken this + * argument since the read/write split was unified, and its own doc explains + * why — the same `{ context }` object was correct as the 3rd arg to `insert` + * but SILENTLY DROPPED as the 3rd arg to `find`, so an intended `isSystem` + * bypass just vanished (control-plane reads coming back empty once + * org-scoping hooks landed). The contract kept only `query.context`, so + * every caller passing the trailing one — the current-user endpoints' + * permission-set loader among them — had to reach it through `any`, which is + * exactly the erasure this issue is sweeping. `query.context` remains + * supported; when both are given, `options.context` wins. + */ + find(objectName: string, query?: EngineQueryOptions, options?: BaseEngineOptions): Promise; + findOne(objectName: string, query?: EngineQueryOptions, options?: BaseEngineOptions): Promise; insert(objectName: string, data: any | any[], options?: DataEngineInsertOptions & WriteObservabilityOptions): Promise; update(objectName: string, data: any, options?: EngineUpdateOptions & WriteObservabilityOptions): Promise; delete(objectName: string, options?: EngineDeleteOptions): Promise; - count(objectName: string, query?: EngineCountOptions): Promise; - aggregate(objectName: string, query: EngineAggregateOptions): Promise; + count(objectName: string, query?: EngineCountOptions, options?: BaseEngineOptions): Promise; + aggregate(objectName: string, query: EngineAggregateOptions, options?: BaseEngineOptions): Promise; /** * Vector Search (AI/RAG) diff --git a/packages/spec/src/data/data-engine.zod.ts b/packages/spec/src/data/data-engine.zod.ts index f3d1c4c453..f55cf2c738 100644 --- a/packages/spec/src/data/data-engine.zod.ts +++ b/packages/spec/src/data/data-engine.zod.ts @@ -704,6 +704,24 @@ export const DataEngineRequestSchema = lazySchema(() => z.discriminatedUnion('me // ========================================================================== // --- New: QueryAST-aligned types (preferred) --- +/** + * Trailing options for the READ methods (`find` / `findOne` / `count` / + * `aggregate`) — the execution context, and nothing else. + * + * [#4251] The schema always existed; the exported type did not, so + * `IDataEngine`'s read methods could not declare the trailing argument their + * implementation has taken since the split was unified. Reads once took their + * context INSIDE the query while writes took it in trailing `options.context`, + * and passing the write shape to a read SILENTLY DROPPED it — an intended + * `isSystem` bypass just vanished. The engine accepts both channels now + * (`options.context` wins), but a caller typed to the contract could not reach + * the trailing one at all, so the callers that use it were reaching it through + * `any`. Same shape as {@link BaseEngineOptionsSchema} by construction: this is + * naming what is already there, not widening it. (The type itself is not new — + * it sat unused under the "legacy/deprecated" heading below, which is why the + * contract went looking for it and did not find it.) + */ +export type BaseEngineOptions = z.infer; export type EngineQueryOptions = z.infer; export type EngineUpdateOptions = z.infer; export type DroppedFieldsEvent = z.infer; @@ -715,7 +733,6 @@ export type EngineCountOptions = z.infer; export type DataEngineFilter = z.infer; /** @deprecated Use standard `SortNode[]` from QueryAST instead. */ export type DataEngineSort = z.infer; -export type BaseEngineOptions = z.infer; /** @deprecated Use `EngineQueryOptions` instead. */ export type DataEngineQueryOptions = z.infer; export type DataEngineInsertOptions = z.infer; diff --git a/scripts/slot-lookup-baseline.json b/scripts/slot-lookup-baseline.json index 937ed74e55..df999668b9 100644 --- a/scripts/slot-lookup-baseline.json +++ b/scripts/slot-lookup-baseline.json @@ -13,12 +13,9 @@ "packages/plugins/plugin-approvals/src/approvals-plugin.ts": 7, "packages/plugins/plugin-approvals/src/status-mirror-cascade.integration.test.ts": 2, "packages/plugins/plugin-audit/src/audit-plugin.ts": 1, - "packages/plugins/plugin-auth/src/auth-plugin.ts": 20, "packages/plugins/plugin-email/src/email-plugin.ts": 3, - "packages/plugins/plugin-hono-server/src/current-user-endpoints.ts": 10, "packages/plugins/plugin-pinyin-search/src/pinyin-search-plugin.ts": 2, "packages/plugins/plugin-reports/src/reports-plugin.ts": 5, - "packages/plugins/plugin-security/src/security-plugin.ts": 1, "packages/plugins/plugin-sharing/src/sharing-plugin.ts": 7, "packages/plugins/plugin-webhooks/src/webhook-outbox-plugin.ts": 1, "packages/qa/dogfood/test/showcase-agent-intersection.dogfood.test.ts": 1,