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

[os-zhuang]

Fixes#8286

POST /api/v1/analytics/query returned the executed statement to callers in data.sql on every deployment — NODE_ENV=production included — with no debug flag requested and none available to request. The contract has declared the field debug-only since it was introduced (packages/spec/src/api/analytics.zod.ts:75, sql typed optional() and described as "Executed SQL (if debug enabled)"); no implementation ever read a switch. This restores declared = enforced. packages/spec is untouched — the contract was already right.

What was disclosed, precisely

More than table and column names. The echoed statement carries the compiled read scope, so it describes the shape of the isolation predicate: on the reported deployment it showed sys_user walled by an enumerated "sys_user"."id" IN ($2, $3, …) member list rather than by an organization_id comparison — which column the wall is built on, and how — plus the bound-parameter arity, which counts the caller's own organization's membership.

No wall was breached. Information disclosure only. The reporter ran the isolation probes on that deployment and every one held: cross-tenant read 404, cross-tenant update/delete 403 at row-level security, a foreign-org filter/where empty, batch write by foreign id per-row PERMISSION_DENIED, audit log and activity stream partitioned. The wall works; it should not have been describing itself.

Where the gate lands

At the response-assembly seam — AnalyticsService.query, the single point every strategy's result leaves through — not on any one strategy. Triage's concern was measured and correct:

Strategy	How it mints sql
NativeSQLStrategy	returns the statement it ran
ObjectQLStrategy	renders a representative one (best-effort, inside try/catch)
FallbackDelegateStrategy	passes through whatever the delegated service minted — the in-memory analytics service always echoes

Gating one would have left the other two serving. queryDataset reaches the same seam through DatasetExecutor, so dataset-backed dashboard and report responses inherit the verdict with no second gate to keep in step. generateSql — the dedicated /api/v1/analytics/sql dry-run route — is deliberately not gated: handing back a statement is that route's entire purpose, and it is where a debugging author should look.

The switch

New debugSql on AnalyticsServicePlugin, forwarded to AnalyticsServiceConfig. Unset means "no host choice", which resolves to NODE_ENV === 'development' and nothing else — an unsetNODE_ENV counts as production and the echo stays off, matching how os start, os serve and os doctor already read that absence (the 2026-08-06 ruling for machine-readable environment answers). Of the two ways to be wrong, disclosing on a production deployment whose operator forgot the variable is the dangerous one.

Two deliberate non-choices:

• No request field. A caller-settable debug flag would let any tenant reopen the disclosure on demand — the shape of the defect, not a fix for it. Adding one would also have meant editing packages/spec, which this card forbids and does not need.
• Not the plugin's existing debug option, which stays server-side log verbosity. Folding them together means a support engineer raising log level on a live deployment silently reopens the disclosure.

I took the debug-gate route rather than the card's alternative (drop sql from this response entirely). Measured, not assumed: nothing consumes the echo today — no reader in this repo outside tests, and ../objectui has no analytics sql consumer at all (its only SQL references are SqlDriverInterface / SqlQueryAST type names in the data protocol). So removal would not have broken a consumer — but the field is declared in the response contract, and deleting a declared field to solve an enforcement bug is a contract change to fix an implementation defect. Gating it enforces what the contract already says, and keeps the echo reachable where an author legitimately needs it.

Tests — every absence pin is paired

"sql is absent" is trivially green against any fixture that never minted one. New src/__tests__/sql-echo-debug-gate.test.ts (15 pins) is built so no absence claim stands alone:

• each arm captures the statement server-side (executeRawSql / the delegated service / the rendered echo) and asserts it is real, proving the fixture capable of disclosure before absence is claimed of it;
• each absence arm has a presence twin on the same cube, same query, same rows, differing only in the switch;
• all three strategies are driven on both sides, plus queryDataset, plus the production default with debugSql not passed at all (the repro), plus NODE_ENV unset and NODE_ENV=development, plus the ungated dry-run route.

Absence is asserted as toBeUndefined() with cross-field-engine-fallback.test.ts's own message — absent, not falsy, and not a second phrasing for one property.

Three existing suites, triaged individually

• analytics-service.test.ts:429 (NativeSQL always includes sql) — its subject is routing, and the echo was only the witness it read; ObjectQLStrategy would have died there for want of an aggregate bridge, so a statement coming back means NativeSQL won. Re-pointed: the witness now states its precondition (debugSql: true) and the trailing comment no longer claims the echo is unconditional.
• dataset-selection-window.test.ts, the #3588 — the echoed SQL tells the truth block — its subject is the echo's content, which still exists as behaviour with a precondition. The three tests enable the echo explicitly rather than inheriting whatever NODE_ENV the runner has.
• cross-field-engine-fallback.test.ts:469 — the important one. Its absence pin passes on this branch for free, because a gated-off service echoes nothing; it would have stopped measuring the renderer's decline and started measuring the gate. That service now enables the echo, so sql being absent again means what the file says it means.

Reverse verification

Direction predicted before running, then measured on origin/main (source files reverted, new tests kept):

• 8 RED — every absence arm, e.g. NODE_ENV=production: the echo is absent (the reported deployment): AssertionError: the echo must be absent, not half-rendered: expected 'SELECT COUNT(*) AS "count" FROM "sys_…' to be undefined. These are the reds that prove the fix.
• 7 GREEN before and after — every presence arm and the dry-run route. On main they pass for the wrong reason (an unconditional echo satisfies "present when debug is on"); after, for the right one. They accompany the fix rather than prove it.
• The other 73 test files in the package stay green in both states, the re-pointed suites included: debugSql is simply an unknown key on main, so those edits change no reading there.

Verification run

• pnpm --filter @objectstack/service-analytics test — 74 files, 1670 tests, all pass.
• Downstream consumers (prefix filter — the packages that consume this one, where a response narrowing lands): @objectstack/rest 108 files / 1800 tests and @objectstack/runtime 150 files / 2306 tests, all pass against the rebuilt dist.
• tsc --noEmit on the package with its closure built: 10 errors, identical to the check-type-check-coverage DEBT ledger entry — none from this diff.
• Gates green: check:nul-bytes, check:docs-audit-scope, check:test-source-alias, check:type-source-resolution, check:changeset-gate-self-tests, check:objectui-changeset, check:query-options-erasure, check:type-check-coverage, check-changeset-fixed.mjs, check-changeset-no-major.mjs. The gate list was re-derived against the actual changed paths with scripts/pm/dispatch-gates.mjs, which added five families the dispatch list did not name; all five are above.

Two local reds that are not this diff, reported rather than worked around:

• check:type-check-debt refuses to run locally without the whole workspace build closure on disk (--re-measure ... 47 workspace dependenc(ies) ... have no built type entry point) — the known local-ergonomics limitation tracked in check:type-check-debt --re-measure trusts stale dist/: phantom upward drift for ledgered packages whose deps resolve to build artifacts, unless the caller builds the closure first #8271, and CI builds that closure before the step. The number it would measure is pinned directly above.
• check-objectui-pin-fresh is red because .objectui-sha no longer describes objectui main. That file is byte-identical to origin/main here and this branch touches no objectui file; it matched only because the diff adds a .changeset entry. Pre-existing repo state, and moving the console pin is a separate procedure.

Scope

packages/services/service-analytics/src/** plus one changeset. packages/spec/src/**, docs/adr/**, .claude/skills/**, skills/** and content/docs/releases/** untouched. Nothing here goes near service-analytics' comparand allow-list files, so #8186 is not addressed here and its surface stays clear.

---

Generated by Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
objectstack	Ignored		Aug 13, 2026 4:28am

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-analytics.

5 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

• content/docs/api/data-api.mdx(via @objectstack/service-analytics)
• content/docs/api/index.mdx(via @objectstack/service-analytics)
• content/docs/kernel/services-checklist.mdx(via @objectstack/service-analytics)
• content/docs/permissions/sharing-rules.mdx(via @objectstack/service-analytics)
• content/docs/plugins/packages.mdx(via @objectstack/service-analytics)

⛔ 3 release-owned page(s) also reference the affected code. These are read-only:

• content/docs/releases/implementation-status.mdx(via @objectstack/service-analytics)
• content/docs/releases/v17.mdx(via @objectstack/service-analytics)
• content/docs/releases/v9.mdx(via @objectstack/service-analytics)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.