fix(analytics,runtime,types): gate cube auto-inference on object existence; stop the dispatcher boundary returning raw SQL (#3867) (#3875)

Two independent defects on the `/analytics` surface, found while verifying
#3770 against a real server. Before this change, on an authenticated CRM dev
server:

    POST /api/v1/analytics/query
    {"cube":"sqlite_master","measures":["count"],"dimensions":["type"]}
    → 200 {"rows":[{"type":"index","count":262},{"type":"table","count":71},
                   {"type":"view","count":1}]}

SQLite's internal schema table — never a registered object — read through the
analytics endpoint. Any table the connection can see was readable.

① CUBE NAME REACHED THE DRIVER AS A TABLE NAME. `AnalyticsService.ensureCube`
auto-infers a minimal Cube when none is registered, with `cube.sql = <queried
name>`. That is the intended "metric over an object" path (an `object-metric`
KPI widget queries `crm_account` with no authored Cube) but it accepted ANY
string. Not covered by #3770: that gated the protocol's `analyticsQuery`, the
degraded fallback; a deployment with @objectstack/service-analytics installed
runs the real engine instead (`ctx.replaceService`).

Inference is now gated on the same schema registry the data path consults, via
a new optional `AnalyticsServiceConfig.isRegisteredObject` wired in `plugin.ts`
from the `data` engine's `getObject`. A registered Cube runs untouched; an
unregistered name that IS an object still auto-infers as before; neither →
`CUBE_NOT_FOUND` / 404 raised before any SQL exists. `generateSql` is gated
too. With no probe configured the gate stands down and warns once.

② THE DISPATCHER BOUNDARY RETURNED `err.message` VERBATIM. `errorResponseBase`
is the single error exit for EVERY route the dispatcher plugin mounts —
/analytics, /packages, /i18n, /storage, /automation, /auth, /notifications,
/mcp. @objectstack/rest has guarded its data routes forever (`mapDataError`);
this boundary guarded nothing. Unlike ①, this half is unconditional.

The heuristic moved to @objectstack/types as `looksLikeInternalErrorLeak` and
applies at both boundaries. `mapDataError` is unchanged. At the dispatcher it
applies ONLY to 5xx — a 4xx message is a deliberate answer. Diagnostics are
preserved via the existing `__obsRecordedError` side-channel.

③ ALSO FIXED: `errorResponseBase` read only `err.statusCode`, while domain
errors carry `status` (as `errorFromThrown` already assumes). Every deliberate
4xx thrown through a dispatcher route rendered as a 500 — including #3770's
`OBJECT_NOT_FOUND` on the analytics fallback path.

Verified live, before → after:
  crm_account KPI            200 rows      → 200 rows (unchanged)
  sqlite_master count        200 {334}     → 404 CUBE_NOT_FOUND
  sqlite_master grouped      200 real rows → 404 CUBE_NOT_FOUND
  malformed payload          raw SQL body  → "Internal server error"
  /analytics/sql             ungated       → 404 CUBE_NOT_FOUND

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: os-zhuang

.changeset/analytics-cube-gate-and-error-leak.md

@@ -0,0 +1,71 @@
+---
+"@objectstack/types": minor
+"@objectstack/service-analytics": minor
+"@objectstack/runtime": minor
+"@objectstack/rest": patch
+---
+
+fix(analytics,runtime,types): gate cube auto-inference on object existence; stop the dispatcher boundary returning raw SQL (#3867)
+
+Two independent defects on the `/analytics` surface, found while verifying #3770
+against a real server. On an authenticated CRM dev server, before this change:
+
+```
+POST /api/v1/analytics/query {"cube":"sqlite_master","measures":["count"],"dimensions":["type"]}
+→ 200 {"rows":[{"type":"index","count":262},{"type":"table","count":71},{"type":"view","count":1}],
+       "sql":"SELECT type AS \"type\", COUNT(*) AS \"count\" FROM \"sqlite_master\" GROUP BY type"}
+```
+
+That is SQLite's internal schema table — never a registered object — read
+successfully through the analytics endpoint. Not merely "the name reaches the
+driver and errors": **any table the connection can see was readable.**
+
+**① The cube name reached the driver as a table name.** `AnalyticsService.ensureCube`
+auto-infers a minimal Cube when none is registered, with `cube.sql = <the queried
+name>`. That is the intended "metric over an object" path — an `object-metric` KPI
+widget queries `crm_account` with no authored Cube — but it accepted *any* string,
+so the endpoint could aggregate over an arbitrary physical table. The
+analytics-side twin of the data-path gap #3770 closed, and it was not covered by
+that fix: #3770 gated the protocol's `analyticsQuery`, which is the *degraded
+fallback*; a deployment with `@objectstack/service-analytics` installed runs the
+real engine instead (`ctx.replaceService`).
+
+Inference is now gated on the same schema registry the data path consults, via a
+new optional `AnalyticsServiceConfig.isRegisteredObject` that `plugin.ts` wires
+from the `data` engine's `getObject`. Three-way rule: a registered Cube runs
+untouched (its `sql` is whatever it declares); an unregistered name that IS an
+object still auto-infers exactly as before; neither → `CUBE_NOT_FOUND` / 404
+raised before any SQL exists, naming both ways to make the request valid. With no
+probe configured the gate stands down and warns once — the same tiering #3770
+took for a missing registry. `generateSql` (`/analytics/sql`) is gated too.
+
+**② The dispatcher boundary returned `err.message` verbatim.** `errorResponseBase`
+is the single error exit for *every* route the dispatcher plugin mounts —
+`/analytics`, `/packages`, `/i18n`, `/storage`, `/automation`, `/auth`,
+`/notifications`, `/mcp`. `@objectstack/rest` has guarded its data routes against
+driver dumps forever (`mapDataError`); this boundary guarded nothing, so any
+driver error on any of those routes shipped its SQL to the client. Unlike ①, this
+half is unconditional — it does not depend on the cube being invalid.
+
+The leak heuristic moved out of `rest-server.ts` into `@objectstack/types` as
+`looksLikeInternalErrorLeak` (both packages already depend on it) and is now
+applied at both boundaries — one predicate, one place to widen when a new
+dialect's phrasing shows up. `mapDataError`'s behaviour is unchanged. At the
+dispatcher it applies **only to 5xx**: a 4xx message is a deliberate
+business/validation answer and must reach the caller intact. Sanitising costs no
+diagnostics — the untouched error still reaches `errorReporter` through the
+existing `__obsRecordedError` side-channel.
+
+**Also fixed in the same function:** `errorResponseBase` read only
+`err.statusCode`, while domain errors across this codebase carry `status` (and
+`HttpDispatcher.errorFromThrown` already reads `status` first). Every deliberate
+4xx thrown through a dispatcher route — including #3770's `OBJECT_NOT_FOUND` on
+the analytics fallback path — was rendered as a **500**. It now reads `status`
+then `statusCode`.
+
+**Behaviour change.** `/analytics/query` and `/analytics/sql` return 404
+`CUBE_NOT_FOUND` for a cube that is neither registered nor a registered object;
+previously the name was passed to the driver. Dashboards and KPI widgets pointed
+at real objects or authored cubes are unaffected. A 5xx on a dispatcher route
+whose message looks like a driver dump now reads `Internal server error` — check
+server logs or your error reporter for the original.

packages/rest/src/rest-server.ts

@@ -4,7 +4,7 @@ import {
     IHttpServer, resolveAuthzContext, resolveLocalizationContext, isAuthGateAllowlisted,
     shouldDenyAnonymous, ANONYMOUS_DENY_BODY, ANONYMOUS_DENY_STATUS,
 } from '@objectstack/core';
-import { isMcpServerEnabled } from '@objectstack/types';
+import { isMcpServerEnabled, looksLikeInternalErrorLeak } from '@objectstack/types';
 import { allowPerfDisclosure, isPerfDisclosurePrincipal } from '@objectstack/observability';
 import { RouteManager } from './route-manager.js';
 import { RestServerConfig, RestApiConfig, CrudEndpointsConfig, MetadataEndpointsConfig, BatchEndpointsConfig, RouteGenerationConfig } from '@objectstack/spec/api';
@@ -401,17 +401,14 @@ export function mapDataError(error: any, object?: string): { status: number; bod
     // Default: do NOT leak raw SQL or driver internals. If the message
     // looks like a SQL/driver dump, replace it with a generic envelope
     // and rely on server logs for the full diagnostic.
-    const looksLikeSqlLeak =
-        lower.includes('sqlite_') ||
-        lower.includes('sqlstate') ||
-        lower.startsWith('insert into ') ||
-        lower.startsWith('update ') ||
-        lower.startsWith('select ') ||
-        lower.startsWith('delete from ') ||
-        lower.includes('constraint failed') ||
-        lower.includes('unique constraint') ||
-        lower.includes('foreign key');
-    if (looksLikeSqlLeak) {
+    //
+    // [#3867] The heuristic itself now lives in `@objectstack/types`
+    // (`looksLikeInternalErrorLeak`) so the OTHER HTTP boundary — the
+    // dispatcher-plugin routes (`/analytics`, `/packages`, `/i18n`, …) — can
+    // apply the same rule. Before #3867 that boundary applied none and
+    // returned raw SQL to clients. Behaviour here is unchanged; only the
+    // predicate's home moved.
+    if (looksLikeInternalErrorLeak(raw)) {
         // Surface unique-constraint violations as a structured 409 so
         // the UI can map them to "this value already exists".
         if (lower.includes('unique constraint') || lower.includes('unique violation')) {

packages/runtime/src/dispatcher-plugin.error-envelope.test.ts

@@ -0,0 +1,156 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #3867 — the dispatcher-plugin error exit.
+ *
+ * `errorResponseBase` is the single error exit for EVERY route this plugin
+ * mounts (`/analytics`, `/packages`, `/i18n`, `/storage`, `/automation`,
+ * `/auth`, `/notifications`, `/mcp`, …): each handler catches and calls it
+ * rather than re-throwing. Two defects lived there, and neither is visible
+ * until a real error actually reaches it:
+ *
+ *  1. It returned `err.message` verbatim. `@objectstack/rest` has guarded its
+ *     data routes against driver dumps forever (`mapDataError`); this boundary
+ *     guarded nothing, so `POST /analytics/query` on an unresolvable cube
+ *     answered with a real SQL statement in the body.
+ *  2. It read only `err.statusCode`, while domain errors across this codebase
+ *     carry `status` (and `HttpDispatcher.errorFromThrown` already reads
+ *     `status` first). A deliberate 404 rendered as a 500.
+ *
+ * These drive the REAL route handler the plugin registers, through the real
+ * dispatcher, so the assertions are about what an HTTP client receives.
+ */
+
+import { describe, it, expect } from 'vitest';
+
+import { createDispatcherPlugin } from './dispatcher-plugin.js';
+
+function makeFakeServer() {
+    const handlers: Record<string, (req: any, res: any) => any> = {};
+    const rec = (verb: string) => (path: string, handler: any) => {
+        handlers[`${verb} ${path}`] = handler;
+    };
+    return {
+        handlers,
+        server: {
+            get: rec('GET'),
+            post: rec('POST'),
+            put: rec('PUT'),
+            delete: rec('DELETE'),
+            patch: rec('PATCH'),
+        },
+    };
+}
+
+/** A kernel whose `analytics` service throws whatever the test hands it. */
+function makeCtx(fakeServer: any, analyticsError: unknown) {
+    const analytics = {
+        query: async () => { throw analyticsError; },
+        getMeta: async () => ({ cubes: [] }),
+        generateSql: async () => ({ sql: null }),
+    };
+    const kernel = {
+        getService: (name: string) => (name === 'analytics' ? analytics : undefined),
+        getServiceAsync: async (name: string) => (name === 'analytics' ? analytics : undefined),
+    };
+    return {
+        getKernel: () => kernel,
+        getService: (name: string) => (name === 'http.server' ? fakeServer : undefined),
+        environmentId: undefined,
+        logger: { info() {}, warn() {}, error() {}, debug() {} },
+        hook: () => {},
+        on: () => {},
+    } as any;
+}
+
+function makeRes() {
+    const res: any = {
+        statusCode: undefined as number | undefined,
+        body: undefined as any,
+        status(c: number) { res.statusCode = c; return res; },
+        header() { return res; },
+        json(b: any) { res.body = b; return res; },
+    };
+    return res;
+}
+
+/** Drive `POST /analytics/query` with an analytics service that throws `err`. */
+async function postAnalyticsQuery(err: unknown) {
+    const { server, handlers } = makeFakeServer();
+    const plugin = createDispatcherPlugin({ prefix: '/api/v1', securityHeaders: false });
+    await plugin.start?.(makeCtx(server, err));
+
+    const handler = handlers['POST /api/v1/analytics/query'];
+    expect(handler, 'POST /api/v1/analytics/query must be mounted').toBeTypeOf('function');
+
+    const res = makeRes();
+    await handler({ body: { cube: 'x', query: {} }, query: {} }, res);
+    return res;
+}
+
+describe('#3867 — dispatcher-plugin error envelope', () => {
+    it('does not return raw SQL to the client (the message that motivated the issue)', async () => {
+        const res = await postAnalyticsQuery(
+            new Error('SELECT  FROM "sqlite_sequence" - near "FROM": syntax error'),
+        );
+
+        expect(res.statusCode).toBe(500);
+        expect(res.body.success).toBe(false);
+        expect(res.body.error.message).toBe('Internal server error');
+        // The specifics a client must never see.
+        expect(String(res.body.error.message)).not.toContain('SELECT');
+        expect(String(res.body.error.message)).not.toContain('sqlite_sequence');
+    });
+
+    it('still hands the UNSANITISED error to the observability side-channel', async () => {
+        // Sanitising the response must not cost server-side diagnostics: the
+        // error reporter reads `__obsRecordedError`, not the response body.
+        const original = new Error('UNIQUE constraint failed: sys_user.email');
+        const res = await postAnalyticsQuery(original);
+
+        expect(res.statusCode).toBe(500);
+        expect(res.body.error.message).toBe('Internal server error');
+        expect((res as any).__obsRecordedError).toBe(original);
+    });
+
+    it('leaves an ordinary 5xx message alone — only leaks are replaced', async () => {
+        const res = await postAnalyticsQuery(new Error('analytics engine unavailable'));
+
+        expect(res.statusCode).toBe(500);
+        expect(res.body.error.message).toBe('analytics engine unavailable');
+    });
+
+    it('honours `status` (not just `statusCode`) so a domain 404 is not a 500', async () => {
+        // What the #3867 cube gate throws, and the shape every protocol-layer
+        // domain error uses (`OBJECT_NOT_FOUND`, `RECORD_NOT_FOUND`, …).
+        const err = Object.assign(new Error("Cube 'ghost' not found: no cube is registered"), {
+            code: 'CUBE_NOT_FOUND',
+            status: 404,
+        });
+        const res = await postAnalyticsQuery(err);
+
+        expect(res.statusCode).toBe(404);
+        // A 4xx message is a deliberate answer — it must reach the caller intact.
+        expect(res.body.error.message).toContain("Cube 'ghost' not found");
+    });
+
+    it('still honours `statusCode` for callers that use it', async () => {
+        const err = Object.assign(new Error('bad request'), { statusCode: 400 });
+        const res = await postAnalyticsQuery(err);
+
+        expect(res.statusCode).toBe(400);
+        expect(res.body.error.message).toBe('bad request');
+    });
+
+    it('does not sanitise a 4xx even when its message resembles SQL', async () => {
+        // Anti-regression for the tier: the guard is scoped to 5xx precisely so
+        // a deliberate client-facing answer is never swallowed.
+        const err = Object.assign(new Error('unique constraint on email — pick another'), {
+            status: 409,
+        });
+        const res = await postAnalyticsQuery(err);
+
+        expect(res.statusCode).toBe(409);
+        expect(res.body.error.message).toBe('unique constraint on email — pick another');
+    });
+});

packages/runtime/src/dispatcher-plugin.ts

@@ -1,6 +1,7 @@
 // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
 
 import { Plugin, PluginContext, IHttpServer } from '@objectstack/core';
+import { looksLikeInternalErrorLeak, INTERNAL_ERROR_MESSAGE } from '@objectstack/types';
 import { HttpDispatcher, HttpDispatcherResult } from './http-dispatcher.js';
 import {
     buildSecurityHeaders,
@@ -347,8 +348,38 @@ function sendResultBase(
     });
 }
 
+/**
+ * The single error exit for EVERY dispatcher-plugin route — `/analytics`,
+ * `/packages`, `/i18n`, `/storage`, `/automation`, `/auth`, `/notifications`,
+ * `/mcp`, … Each handler catches and calls here rather than re-throwing.
+ *
+ * [#3867] Two things were wrong with it, both invisible until a driver error
+ * actually reached this path:
+ *
+ * 1. **It only honoured `statusCode`.** Domain errors across this codebase
+ *    carry their HTTP status as `status` (the protocol layer's
+ *    `OBJECT_NOT_FOUND`/`RECORD_NOT_FOUND`/`CLONE_DISABLED`, plugin-sharing's
+ *    `FORBIDDEN`, …); `HttpDispatcher.errorFromThrown` already reads `status`
+ *    first, `statusCode` second. Here a deliberate 404 was rendered as a
+ *    **500** — the wrong code, and it dragged the message through the
+ *    sanitiser below for no reason. Now aligned with `errorFromThrown`.
+ *
+ * 2. **It returned `err.message` verbatim.** `@objectstack/rest` has guarded
+ *    its data routes against driver dumps since forever (`mapDataError`), but
+ *    this boundary had no equivalent, so `POST /analytics/query` on an
+ *    unresolvable cube answered with a real SQL statement in the body. The
+ *    shared predicate now applies here too — but ONLY on a 5xx: a 4xx message
+ *    is a deliberate business/validation answer and must reach the caller
+ *    intact.
+ *
+ * Sanitising costs no diagnostics: the untouched error is still handed to
+ * `errorReporter` through the `__obsRecordedError` side-channel below.
+ */
 function errorResponseBase(err: any, res: any, securityHeaders?: Record<string, string>): void {
-    const code = err.statusCode || 500;
+    const code =
+        (typeof err?.status === 'number' ? err.status : undefined) ??
+        (typeof err?.statusCode === 'number' ? err.statusCode : undefined) ??
+        500;
     res.status(code);
     if (securityHeaders) {
         for (const [k, v] of Object.entries(securityHeaders)) {
@@ -366,9 +397,14 @@ function errorResponseBase(err: any, res: any, securityHeaders?: Record<string,
             // res is a frozen / proxy object — skip
         }
     }
+    const raw = err?.message;
+    const message =
+        code >= 500 && looksLikeInternalErrorLeak(raw)
+            ? INTERNAL_ERROR_MESSAGE
+            : raw || 'Internal Server Error';
     res.json({
         success: false,
-        error: { message: err.message || 'Internal Server Error', code },
+        error: { message, code },
     });
 }
 

packages/services/service-analytics/src/__tests__/analytics-service.test.ts

@@ -725,7 +725,15 @@ describe('AnalyticsService — auto-inferred cube log level', () => {
   it('logs at debug (not warn) for a scalar metric over an unregistered cube', async () => {
     const logger = { info: vi.fn(), debug: vi.fn(), warn: vi.fn(), error: vi.fn(), child: vi.fn().mockReturnThis() } as any;
     await makeService(logger).query({ cube: 'showcase_task', measures: ['count'] });
-    expect(logger.warn).not.toHaveBeenCalled();
+    // [#3867] Asserted against THIS message rather than "warn was never called
+    // at all". These services are built without `isRegisteredObject`, so the
+    // cube-existence gate correctly stands down and warns once about being
+    // inactive — a different message, and not what this pair is about. The
+    // contract here is the LEVEL of the no-cube-registered log, which is
+    // exactly how the sibling case below already asserts it.
+    expect(logger.warn).not.toHaveBeenCalledWith(
+      expect.stringContaining('No cube registered for "showcase_task"'),
+    );
     expect(logger.debug).toHaveBeenCalledWith(expect.stringContaining('No cube registered for "showcase_task"'));
   });