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

.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"'));
  });