fix(runtime, plugin-auth)!: the environment-membership gate and the organization slug guard fail closed when their own read faults

.changeset/21941-access-guards-fail-closed.md

@@ -0,0 +1,17 @@
+---
+'@objectstack/runtime': minor
+'@objectstack/plugin-auth': minor
+---
+
+fix(runtime, plugin-auth)!: two access guards refuse, instead of admitting, when their own read cannot answer
+
+Clause-②: no (narrowing)
+
+<!-- adr-0087: not-required (no-migration-prescription) A runtime refusal narrowing on two access guards, not a metadata change: no spec key, export, option or stored shape is removed, renamed or re-shaped, so there is no tombstone and nothing for `objectstack migrate meta` to rewrite. What narrows is which requests the two guards admit: a request admitted only because the guard's own read faulted is now refused with 503, and every answer from a healthy read is unchanged. The other categories are closed on facts: both packages publish (not unpublished); no ADR-0087 id covers either guard and this diff adds none (not registered / already-registered); and no published interface or type changes (not runtime-interface-only / type-surface-only). -->
+
+**BREAKING** (an accept-set narrowing), shipped as `minor` under the launch-window convention: a request that one of these two guards let through only because the guard's own read faulted is now refused. Nothing an author or caller writes changes shape.
+
+- **`@objectstack/runtime` — the dispatcher's environment-membership gate.** When its `sys_environment_member` read throws, or no ObjectQL engine resolves on the request's kernel, the request is refused with `503 SERVICE_UNAVAILABLE` — the `AuthzStoreUnavailableError` answer the identity step and the domain gates already give an authorization input they could not read. Before, the gate logged at debug level and let the request through. A member is still admitted, and a non-member is still refused with `403 PROJECT_MEMBERSHIP_REQUIRED`. An engine whose registry does not register `sys_environment_member` declares the gate inapplicable, and nothing is read.
+- **`@objectstack/plugin-auth` — the organization slug guard** (`organizationHooks.beforeUpdateOrganization`). When its `sys_organization` or `sys_environment` read throws, the organization update is refused with `503 SERVICE_UNAVAILABLE`. Before, the hook ended without refusing and the slug changed. A slug change while an active environment references the organization is still refused (`403 FORBIDDEN`), and any other change is still allowed. An engine that does not register `sys_environment` — the open-source composition, where it is a cloud-provided object — declares the guard inapplicable from its registry (`getSchema`): nothing is read and the update proceeds as before. Without a data engine the guard does not apply either.
+
+What changes for you: nothing in what you write. A `503 SERVICE_UNAVAILABLE` on these doors is a store outage that used to be hidden behind an admitted request; it clears when the store answers again.

packages/plugins/plugin-auth/src/auth-manager.org-slug-guard-system-context.test.ts

@@ -18,13 +18,19 @@
  *  2. the guard KEEPS refusing on an engine that refuses a principal-less,
  *     non-system context — its reads are not one, so the catch below never
  *     turns the refusal into a skipped guard;
- *  3. the catches around the two reads are unchanged: a read that throws ends
- *     the hook without refusing, as it did before. That is pre-existing
- *     behaviour, pinned as it stands, not endorsed here.
+ *  3. the guard FAILS CLOSED when a read it makes cannot answer (#21941): a
+ *     read that throws is refused with better-auth's `SERVICE_UNAVAILABLE`
+ *     (503) — never the guard's own `FORBIDDEN` (403), and never "ends the hook
+ *     without refusing" — while an engine that does not register
+ *     `sys_environment` (the open-source composition) declares the guard
+ *     inapplicable without reading anything. The real ObjectQL registry
+ *     answers that question in the last section.
  */
 
 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import { ObjectQL, assertEngineFindOnePredicate } from '@objectstack/objectql';
 import { AuthManager } from './auth-manager';
+import { authIdentityObjects } from './manifest';
 
 vi.mock('better-auth', () => ({
   betterAuth: vi.fn(() => ({ handler: vi.fn(), api: {} })),
@@ -172,27 +178,149 @@ describe('organization slug guard — on an engine that refuses a principal-less
   });
 });
 
-describe('organization slug guard — the catches around the reads are unchanged (pre-existing)', () => {
-  it('an organization read that throws ends the hook without refusing, as before', async () => {
+/**
+ * A read that could not answer, asserted on its envelope: better-auth's
+ * `SERVICE_UNAVAILABLE` / 503, naming the object whose read failed — and
+ * explicitly NOT the guard's `FORBIDDEN` / 403, which is a verdict about the
+ * slug change that the guard never reached.
+ */
+async function expectReadFaultRefusal(attempt: Promise<unknown>, object: string) {
+  const err = (await attempt.then(
+    () => undefined,
+    (e: unknown) => e,
+  )) as { status?: unknown; statusCode?: unknown; body?: { message?: unknown }; cause?: unknown } | undefined;
+  expect(err, 'the slug change was let through on a read that could not answer').toBeTruthy();
+  expect(err?.statusCode).toBe(503);
+  expect(err?.status).toBe('SERVICE_UNAVAILABLE');
+  expect(String(err?.body?.message)).toContain(`\`${object}\``);
+  return err;
+}
+
+/** The registration answer an ObjectQL engine gives through `getSchema`. */
+const schemaFor = (...registered: string[]) => vi.fn((object: string) => (registered.includes(object) ? { name: object } : undefined));
+
+/**
+ * An engine double that answers the registration question (`getSchema`) beside
+ * its two reads. Its `findOne` keeps ObjectQL's own predicate contract
+ * (`assertEngineFindOnePredicate`), so it is no looser than the engine it
+ * stands in for.
+ */
+function registeringEngine(
+  registered: string[],
+  answers: { findOne: () => Promise<unknown>; find: () => Promise<unknown> },
+) {
+  return {
+    getSchema: schemaFor(...registered),
+    findOne: vi.fn(async (object: string, q?: Query) => {
+      assertEngineFindOnePredicate(object, q as never);
+      return answers.findOne();
+    }),
+    find: vi.fn(answers.find),
+  };
+}
+
+describe('organization slug guard — fails closed when a read cannot answer (#21941)', () => {
+  it('an organization read that throws is refused (503), and the environment read is never made', async () => {
+    // SUPERSEDED PIN, quoted — what the guard answered before:
+    //     await expect(update('acme-new')).resolves.toBeUndefined();   // ends the hook without refusing
     const engine = {
       findOne: vi.fn(async () => {
         throw new Error('store unavailable');
       }),
       find: vi.fn(async () => ENVS),
     };
     const update = await slugGuard(engine);
-    await expect(update('acme-new')).resolves.toBeUndefined();
+    const err = await expectReadFaultRefusal(update('acme-new'), 'sys_organization');
+    expect((err?.cause as Error | undefined)?.message).toBe('store unavailable');
     expect(engine.find).not.toHaveBeenCalled();
   });
 
-  it('an environment read that throws ends the hook without refusing, as before', async () => {
+  it('an environment read that throws is refused (503) — on an engine that registers the object', async () => {
+    // SUPERSEDED PIN, quoted — `find` threw and the hook resolved `undefined`.
+    const engine = registeringEngine(['sys_organization', 'sys_environment'], {
+      findOne: async () => ORG,
+      find: async () => {
+        throw new Error('connection reset');
+      },
+    });
+    const update = await slugGuard(engine);
+    await expectReadFaultRefusal(update('acme-new'), 'sys_environment');
+    expect(engine.find).toHaveBeenCalledTimes(1);
+  });
+
+  it('an environment read that throws on an engine whose registry cannot be asked is refused too — it is asked, and the fault refuses', async () => {
     const engine = {
       findOne: vi.fn(async () => ORG),
       find: vi.fn(async () => {
         throw new Error('object sys_environment is not registered');
       }),
     };
     const update = await slugGuard(engine);
+    await expectReadFaultRefusal(update('acme-new'), 'sys_environment');
+  });
+
+  it('a healthy read on a registered object still refuses the change while an active environment references the org, and allows it otherwise', async () => {
+    const refusing = registeringEngine(['sys_organization', 'sys_environment'], {
+      findOne: async () => ORG,
+      find: async () => ENVS,
+    });
+    await expectSlugRefusal((await slugGuard(refusing))('acme-new'));
+    const allowing = registeringEngine(['sys_organization', 'sys_environment'], {
+      findOne: async () => ORG,
+      find: async () => [{ id: 'e2', status: 'archived' }],
+    });
+    await expect((await slugGuard(allowing))('acme-new')).resolves.toBeUndefined();
+  });
+
+  it('an engine that does not register `sys_environment` declares the guard inapplicable — nothing is read', async () => {
+    const engine = registeringEngine(['sys_organization'], {
+      findOne: async () => {
+        throw new Error('must not be read');
+      },
+      find: async () => {
+        throw new Error('must not be read');
+      },
+    });
+    const update = await slugGuard(engine);
+    await expect(update('acme-new')).resolves.toBeUndefined();
+    expect(engine.getSchema).toHaveBeenCalledWith('sys_environment');
+    expect(engine.findOne).not.toHaveBeenCalled();
+    expect(engine.find).not.toHaveBeenCalled();
+  });
+});
+
+describe('organization slug guard — the real ObjectQL registry answers the composition question', () => {
+  /** A real engine holding exactly the identity objects this package registers — no `sys_environment`. */
+  function engineWithAuthObjects(): ObjectQL {
+    const engine = new ObjectQL({ logger: { debug() {}, info() {}, warn() {}, error() {}, child() { return this; } } } as never);
+    for (const object of authIdentityObjects) {
+      engine.registry.registerObject(object as never, '@objectstack/plugin-auth');
+    }
+    return engine;
+  }
+
+  it('this package registers no `sys_environment`, so on its own object set the guard does not apply and reads nothing', async () => {
+    const engine = engineWithAuthObjects();
+    expect(engine.getSchema('sys_organization'), 'POSITIVE CONTROL: the registry does answer for an object this package registers').toBeTruthy();
+    expect(engine.getSchema('sys_environment')).toBeUndefined();
+    const findOne = vi.spyOn(engine, 'findOne');
+    const find = vi.spyOn(engine, 'find');
+    const update = await slugGuard(engine);
     await expect(update('acme-new')).resolves.toBeUndefined();
+    expect(findOne).not.toHaveBeenCalled();
+    expect(find).not.toHaveBeenCalled();
+  });
+
+  it('once `sys_environment` is registered the guard reads, and a real engine fault on that read refuses (503)', async () => {
+    // No driver is registered, so the engine itself cannot serve the read: a
+    // genuine fault of the real engine, not a double's.
+    const engine = engineWithAuthObjects();
+    engine.registry.registerObject(
+      { name: 'sys_environment', label: 'Environment', fields: { organization_id: { name: 'organization_id', type: 'text' } } } as never,
+      '@objectstack/test-cloud-objects',
+    );
+    expect(engine.getSchema('sys_environment')).toBeTruthy();
+    const update = await slugGuard(engine);
+    await expectReadFaultRefusal(update('acme-new'), 'sys_organization');
   });
 });

packages/plugins/plugin-auth/src/auth-manager.ts

@@ -1275,6 +1275,28 @@ async function smsQuotaExceededApiError(message: string): Promise<Error> {
   return new APIError(SMS_QUOTA_EXCEEDED_CODE, { message });
 }
 
+/**
+ * [#21941] The organization slug guard's answer when one of its own reads
+ * cannot answer: `503 SERVICE_UNAVAILABLE`, the existing ADR-0112 code for a
+ * store that is temporarily down.
+ *
+ * Not the guard's `403 FORBIDDEN`: a read that could not answer is not a
+ * verdict about the slug change, so it must not wear the refusal's words — and
+ * never "allowed", which is what swallowing the fault used to answer. The
+ * driver's own failure rides `cause`, so its diagnostic is not lost.
+ */
+async function slugGuardReadFaultApiError(object: string, cause: unknown): Promise<Error> {
+  const { APIError } = await import('better-auth/api');
+  const err = new APIError('SERVICE_UNAVAILABLE', {
+    message:
+      `The organization slug guard could not read \`${object}\`, so whether an active environment ` +
+      `still references this organization was never determined, and the slug was not changed. ` +
+      `This is a server-side outage, not a refusal of the new slug: retry once the data store is reachable.`,
+  });
+  (err as { cause?: unknown }).cause = cause;
+  return err;
+}
+
 export class AuthManager {
   private auth: Auth<any> | null = null;
   /**
@@ -3334,13 +3356,35 @@ export class AuthManager {
             const orgId = member?.organizationId;
             if (!newSlug || !orgId) return;
 
+            // [#21941] The guard reads only what this composition registers,
+            // and FAILS CLOSED when a registered read cannot answer.
+            //
+            //  - NO DATA ENGINE ⇒ the guard does not apply. Only a standalone
+            //    `AuthManager` lacks one (`AuthPlugin` declares ObjectQL a hard
+            //    dependency), and it then runs on better-auth's in-memory store:
+            //    no engine registers `sys_environment`, so no environment can
+            //    reference the organization.
+            //  - `sys_environment` NOT REGISTERED ⇒ the guard does not apply,
+            //    for the same reason, and nothing is read. It is a cloud-provided
+            //    object (`CLOUD_PROVIDED_OBJECT_NAMES`), so this is the
+            //    open-source composition's answer. The question is the
+            //    registry's own (`getSchema`), never a caught throw; an engine
+            //    that cannot be asked reads as before — it is asked, and a fault
+            //    refuses.
+            //  - A REGISTERED READ THAT THROWS ⇒ refused, `503
+            //    SERVICE_UNAVAILABLE` (`slugGuardReadFaultApiError`). It used to
+            //    end the hook without refusing, which let the slug change through
+            //    whenever either read faulted.
+            const rawEngine = this.config.dataEngine;
+            if (!rawEngine) return;
+            const schemaOf = (rawEngine as { getSchema?: (object: string) => unknown }).getSchema;
+            if (typeof schemaOf === 'function' && !schemaOf.call(rawEngine, 'sys_environment')) return;
+
             // Both reads run as the platform (`withSystemContext`): this hook
             // IS the slug guard — the organization id is the `where`, not the
             // reader — so neither read reaches the engine with no principal
             // and no opt-in (the security middleware's principal-less
-            // hand-off, ADR-0096). The catches below are unchanged.
-            const rawEngine = this.config.dataEngine;
-            if (!rawEngine) return;
+            // hand-off, ADR-0096).
             const dataEngine = withSystemContext(rawEngine) as any;
 
             let currentSlug: string | undefined;
@@ -3349,8 +3393,8 @@ export class AuthManager {
                 where: { id: orgId },
               });
               currentSlug = current?.slug;
-            } catch {
-              return;
+            } catch (err) {
+              throw await slugGuardReadFaultApiError('sys_organization', err);
             }
             if (!currentSlug || currentSlug === newSlug) return;
 
@@ -3362,8 +3406,8 @@ export class AuthManager {
               activeEnvs = (envs ?? []).filter(
                 (e: any) => e?.status !== 'archived' && e?.status !== 'failed',
               ).length;
-            } catch {
-              return;
+            } catch (err) {
+              throw await slugGuardReadFaultApiError('sys_environment', err);
             }
 
             if (activeEnvs > 0) {

packages/runtime/src/domains/meta-verb-fallthrough.test.ts

@@ -129,7 +129,12 @@ function boot() {
     } as any;
 
     return {
-        dispatcher: new HttpDispatcher(kernel),
+        // The environment-membership gate is not this file's subject, and this
+        // fixture composes no ObjectQL engine — which the gate refuses as an
+        // outage (503) rather than letting the request through (#21941). It is
+        // switched off here the way the dispatcher option documents for tests
+        // that seed no membership.
+        dispatcher: new HttpDispatcher(kernel, undefined, { enforceProjectMembership: false }),
         getMetaItem,
         saveMetaItem,
         deleteMetaItem,

packages/runtime/src/http-dispatcher.membership-skip-boundary.test.ts

@@ -51,7 +51,14 @@ function makeGate() {
     const objectql = {
         find,
         getObjects: vi.fn().mockReturnValue({}),
-        registry: { getObject: vi.fn().mockReturnValue(null), getRegisteredTypes: vi.fn().mockReturnValue([]) },
+        // The engine REGISTERS `sys_environment_member`: the gate asks the
+        // registry before it reads, and an engine that registers no such object
+        // declares the gate inapplicable (#21941) — which would make "skipped"
+        // and "checked" both return `null` again.
+        registry: {
+            getObject: vi.fn((name: string) => (name === 'sys_environment_member' ? { name } : null)),
+            getRegisteredTypes: vi.fn().mockReturnValue([]),
+        },
     };
     const auth = {
         getApi: async () => ({