fix(service-analytics): gate the /analytics/query SQL echo on debug, as the contract declares (#8286)

.changeset/analytics-sql-echo-debug-gate.md

@@ -0,0 +1,63 @@
+---
+"@objectstack/service-analytics": patch
+---
+
+fix(service-analytics): gate the `/analytics/query` SQL echo on debug, as the contract has always declared (#8286)
+
+`POST /api/v1/analytics/query` returned the executed statement to the caller in
+`data.sql` on every deployment, `NODE_ENV=production` included, with no debug
+flag requested and none available to request. The contract had declared the
+field debug-only since it was introduced — `AnalyticsResultResponseSchema`
+(`spec/api/analytics.zod.ts`) types it `optional()` and describes it as
+"Executed SQL (if debug enabled)" — but no implementation ever read a debug
+switch. This restores declared = enforced. **The contract is unchanged; the
+response now matches it.**
+
+**What was disclosed.** More than table and column names. The echoed statement
+carries the compiled read scope, so it describes the SHAPE of the tenant
+isolation predicate: on the reported deployment it showed that `sys_user` is
+walled by an enumerated `"sys_user"."id" IN ($2, $3, …)` member list rather than
+by an `organization_id` comparison — that is, which column the wall is built on
+and how — plus the bound-parameter arity, which counts the caller's own
+organization's membership and hands a prober the exact query surface to work
+against.
+
+**No wall was breached.** This is information disclosure and nothing more. The
+reporter ran the isolation probes on the same deployment and every one held:
+cross-tenant read answered 404, cross-tenant update and delete answered 403 at
+row-level security, a `filter`/`where` naming another organization came back
+empty, a batch write by foreign id answered per-row `PERMISSION_DENIED`, and the
+audit log and activity stream were partitioned cleanly. The wall works; it
+simply should not have been describing itself to callers.
+
+**The gate is one gate.** It lives at the response-assembly seam —
+`AnalyticsService.query`, the single point every strategy's result leaves
+through — not on any one strategy. `NativeSQLStrategy` returns the statement it
+ran, `ObjectQLStrategy` renders a representative one, and the fallback delegate
+passes through whatever the service it delegates to minted (the in-memory
+analytics service always echoes); gating one of the three would have left the
+others serving. `queryDataset` reaches the same seam through `DatasetExecutor`,
+so dataset-backed dashboard and report responses inherit the verdict without a
+second gate to keep in step.
+
+**The switch, and its default.** New `debugSql` option on
+`AnalyticsServicePlugin` (forwarded to `AnalyticsServiceConfig`). Unset means no
+host choice, which resolves to `NODE_ENV === 'development'` — and only that: an
+**unset** `NODE_ENV` counts as production and the echo stays off, matching how
+`os start`, `os serve` and `os doctor` already read that absence. Of the two ways
+to be wrong, disclosing on a production deployment whose operator forgot the
+variable is the dangerous one.
+
+It is deliberately a HOST switch with no request field behind it: a
+caller-settable debug flag would let any tenant reopen the disclosure on demand,
+which is the shape of the defect rather than a fix for it. It is also
+deliberately separate from the plugin's existing `debug` option, which stays
+server-side log verbosity only — raising log level on a live deployment must not
+widen what travels to a tenant.
+
+**Unaffected.** `POST /api/v1/analytics/sql` — the dedicated dry-run route that
+exists to hand back a statement — is not gated and behaves exactly as before; it
+is where an author debugging a widget should look. Rows, `fields`, `totals`,
+drill-through metadata, error envelopes and every gate on the query path are
+untouched, and no shipped consumer read the echo (the Studio console does not
+render it).

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

@@ -422,11 +422,18 @@ describe('AnalyticsService', () => {
  logger: silentLogger,
  queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: true, inMemory: false }),
  executeRawSql: vi.fn().mockResolvedValue(mockRows),
+ // [#8286] The claim under test is ROUTING — which strategy served — and
+ // the echo is only the witness it reads: `ObjectQLStrategy` would have
+ // died here for want of an aggregate bridge, so a statement coming back
+ // means NativeSQL won. Since #8286 that witness has a precondition, so
+ // the precondition is stated. The echo's OWN behaviour, both sides of
+ // this switch, is pinned in `sql-echo-debug-gate.test.ts`.
+ debugSql: true,
  });
 
  const result = await service.query(baseQuery);
  expect(result.rows).toEqual(mockRows);
- expect(result.sql).toBeDefined(); // NativeSQL always includes sql
+ expect(result.sql).toBeDefined(); // with the echo enabled, NativeSQL reports its statement
  });
 
  it('should fall back to ObjectQLStrategy when nativeSql is false', async () => {

packages/services/service-analytics/src/__tests__/cross-field-engine-fallback.test.ts

@@ -110,6 +110,16 @@ describe('[#7598] cross-field `$field` on the analytics face — served via the
  readScope = null;
  service = new AnalyticsService({
  cubes: [CUBE],
+ // [#8286] The response-level SQL echo is now debug-gated and OFF by
+ // default outside development. It is enabled here on purpose, and the
+ // reason is the same one the header gives for declaring both
+ // capabilities: an assertion has to be a MEASUREMENT. The pin below —
+ // "`/analytics/query` serves the query while the echo declines" — would
+ // pass against a service that never echoes anything at all, i.e. it would
+ // stop measuring the renderer's decline and start measuring the gate.
+ // With the echo on, `sql` being absent again means what this file says it
+ // means: `generateSql` refused and `execute()` swallowed the refusal.
+ debugSql: true,
  // BOTH paths available — see the header. Native SQL wins unless it declines.
  queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: true, inMemory: false }),
  executeRawSql: async (_object, sql) => {

packages/services/service-analytics/src/__tests__/dataset-selection-window.test.ts

@@ -41,14 +41,26 @@ const accounts = DatasetSchema.parse({
 
 type AggCall = { object: string; options: Record<string, unknown> };
 
-/** ObjectQL-aggregate service; captures every `executeAggregate` call. */
-function aggService(rows: Record<string, unknown>[], calls: AggCall[] = []) {
+/**
+ * ObjectQL-aggregate service; captures every `executeAggregate` call.
+ *
+ * [#8286] `debugSql` is the response-level SQL echo, off by default outside
+ * development. Only the tests that read `result.sql` — the block asserting the
+ * echo tells the truth — need it on; the rest of this file measures the CALL
+ * the executor made, which the gate does not touch.
+ */
+function aggService(
+ rows: Record<string, unknown>[],
+ calls: AggCall[] = [],
+ options: { debugSql?: boolean } = {},
+) {
  const svc = new AnalyticsService({
  queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }),
- executeAggregate: async (object, options) => {
- calls.push({ object, options: options as Record<string, unknown> });
+ executeAggregate: async (object, opts) => {
+ calls.push({ object, options: opts as Record<string, unknown> });
  return rows;
  },
+ debugSql: options.debugSql,
  });
  return { svc, calls };
 }
@@ -327,9 +339,16 @@ describe('#3588 — ordering never corrupts a multi-query selection', () => {
  });
 });
 
+/**
+ * [#8286] Every service in this block enables the echo explicitly. The claim —
+ * that the echoed statement is an honest account of the query that ran — is
+ * unchanged and still worth pinning; what changed is that the echo now has a
+ * precondition, and a block whose whole subject is the echo's CONTENT must
+ * state it rather than inherit whatever `NODE_ENV` the runner happens to have.
+ */
 describe('#3588 — the echoed SQL tells the truth on the ObjectQL path', () => {
  it('renders date_trunc for a bucketed dimension instead of the bare column', async () => {
- const { svc } = aggService([{ created_at: '2026-06', account_count: 4 }]);
+ const { svc } = aggService([{ created_at: '2026-06', account_count: 4 }], [], { debugSql: true });
  const result = await svc.queryDataset(
  accounts,
  { dimensions: ['created_at'], measures: ['account_count'], dateGranularity: 'month' },
@@ -341,7 +360,7 @@ describe('#3588 — the echoed SQL tells the truth on the ObjectQL path', () =>
  });
 
  it('renders the ordering and window that the response rows actually reflect', async () => {
- const { svc } = aggService([{ industry: 'Tech', annual_revenue_sum: 1 }]);
+ const { svc } = aggService([{ industry: 'Tech', annual_revenue_sum: 1 }], [], { debugSql: true });
  const result = await svc.queryDataset(
  accounts,
  { dimensions: ['industry'], measures: ['annual_revenue_sum'], order: { annual_revenue_sum: 'desc' }, limit: 10 },
@@ -352,7 +371,7 @@ describe('#3588 — the echoed SQL tells the truth on the ObjectQL path', () =>
  });
 
  it('parameterizes filter values rather than inlining them into the echoed statement', async () => {
- const { svc } = aggService([{ industry: 'Tech', account_count: 1 }]);
+ const { svc } = aggService([{ industry: 'Tech', account_count: 1 }], [], { debugSql: true });
  const result = await svc.queryDataset(
  accounts,
  { dimensions: ['industry'], measures: ['account_count'], runtimeFilter: { industry: 'Tech' } },