analytics: /analytics/query echoes the executed SQL in production (NODE_ENV=production, no debug requested) — schema documents it as debug-only

POST /api/v1/analytics/query returns the executed SQL to the caller on a deployment running NODE_ENV=production, with no debug flag requested.

Repro

Request (no debug field of any kind):

{ "cube": "sys_user", "measures": ["count"], "dimensions": [] }

Response:

{ "success": true, "data": {
    "rows": [{ "count": "2" }],
    "fields": [{ "name": "count", "type": "number" }],
    "sql": "SELECT COUNT(*) AS \"count\" FROM \"sys_user\" WHERE ((\"sys_user\".\"id\" = $1 OR \"sys_user\".\"id\" IN ($2, $3) OR \"sys_user\".\"id\" = $4 …"
} }

Container environment of the serving app (objectos-ee-deploy-app-1):

NODE_ENV=production
OS_TENANCY_POSTURE=isolated
OS_MODE=standalone

Expected

The contract already says this is debug-gated — packages/spec/src/api/analytics.zod.ts:75:

sql: z.string().optional().describe('Executed SQL (if debug enabled)'),

Nothing in the request enabled debug, and the deployment is production, so sql should be absent.

Why it matters here specifically

The echoed statement discloses the physical table name, the column naming, and — most sensitive on a multi-tenant deployment — the shape of the isolation predicate itself. In this case it reveals that sys_user is walled by an enumerated id IN (…) member list rather than an organization_id comparison, i.e. it tells a tenant exactly how (and on which column) the wall is built, plus the bound-parameter arity, which leaks the rough member count of their own org and the query surface to probe against.

This is an information-disclosure issue, not a broken wall: every isolation probe I ran on this deployment held (cross-tenant read → 404, cross-tenant update/delete → 403 row-level security, filter/where naming another org → empty, batch update/delete by foreign id → per-row PERMISSION_DENIED, audit log and activity stream partitioned cleanly). The wall is fine; it just should not describe itself to callers.

Suggested fix

Gate the sql field on the same debug switch the schema documents, and default it off outside development — or drop it from the response entirely and keep SQL echo on the dedicated /api/v1/analytics/sql dry-run route (analytics.zod.ts:23), which is the surface that exists for it.

Environment

objectos-ee-deploy stack (Caddy → app → postgres:16) on http://localhost:8080, observed 2026-08-13, org owner session. Image commit not verified; spec references are the current framework main checkout.

Triage: pm:queue, domain:services, security added, type Bug. Two label corrections: removed self-made domain:access-security (not a lane; the label authority is the triage seat — standing instruction, 2026-08-12) and the filer's priority:p2 (p1–p5 ladder retired, no reader).

Landing pinned at triage (anchoring rule): the echo's producer is packages/services/service-analytics/src/strategies/native-sql-strategy.ts:274 — return { rows, fields, sql }, unconditional. The contract cite (packages/spec/src/api/analytics.zod.ts:75, "Executed SQL (if debug enabled)") is the declared behavior; the fix restores declared = enforced by gating sql on the debug switch (or dropping it in favor of the dedicated /analytics/sql dry-run route, per the card). This does not change the acceptance surface — it narrows an over-serving response to its documented shape — so it is a Bug, not a decision card.

Dispatch notes:

packages/services/service-analytics/src/__tests__/cross-field-engine-fallback.test.ts:469 already pins "sql must be absent" for the engine-fallback path — extend that pin family to the non-debug production path; assert absence, not just falsiness.
Check the other strategies (analytics-service.ts fallback delegate at ~:2137 mints a sql string too) so the gate lands at the response-assembly seam, not per-strategy — one gate, all strategies.
The NativeSQL always includes sql expectation at analytics-service.test.ts:429 will need to become debug-conditioned — that test asserts the current defect.
Security framing per the card: information disclosure only (isolation held on all probes); no incident process needed, but the changeset should say what was disclosed.

Size/model suggestion: S–M, mode:subagent, opus.

Generated by Claude Code

Claim: PM loop round 13
Session: session_01ARidKDYSCD56LaygrvDPnk
Branch: claude/issue-8286-analytics-sql-echo-debug-gate
Worktree: objectstack-issue-8286
Domain: domain:services
File surface: packages/services/service-analytics/src/** (stop on breach; explain in the report)
Container & model: S–M, mode:subagent, model: opus (per triage's suggestion; the judgment is where the gate lands, not whether)
Serial constraints cleared: no in-flight claim touches service-analytics. This lane's only other open dispatch is #7226 (examples/app-todo, PR #8295 awaiting its last CI job) — disjoint. ⚠️ #8186 is pm:blocked in this lane and also lives in service-analytics (its own copies of the #7872 comparand allow-list); it is not claimed and not in flight, so no conflict today, but a dispatch that ends up in the comparand allow-list files should stop and say so.

Anchors re-verified on origin/main at claim time, ⛔ not taken from the card:

Producer confirmed: strategies/native-sql-strategy.ts — return { rows, fields, sql };, unconditional.
Existing pin confirmed: __tests__/cross-field-engine-fallback.test.ts:469 — expect(result.sql, 'the echo must be absent, not half-rendered').toBeUndefined(); — so the "absent, not falsy" vocabulary already exists in this package and the new pins should match it rather than invent a second phrasing.

⚠️ One coincidence worth naming so nobody trips on it: cross-field-engine-fallback.test.ts is the same file that a priority:p0 card (#8235) claimed was breaking check:type-check-debt on main earlier today. That card was falsified and closed not_planned — the gate is green on main and the file is fine; the real defect was the gate's local ergonomics (#8271). ⛔ Do not go looking for a type error in this file; there isn't one.

Generated by Claude Code

{
  "issue": 8286,
  "status": "done",
  "branch": "claude/issue-8286-analytics-sql-echo-debug-gate",
  "pr": "https://github.com/objectstack-ai/objectstack/pull/8312",
  "premise_still_valid": true,
  "summary": "Premise re-verified on origin/main at implementation time: native-sql-strategy.ts returned `{ rows, fields, sql }` unconditionally, and the contract at spec/api/analytics.zod.ts:75 already declared the field debug-only. The gate lands at the response-assembly seam — AnalyticsService.query, the single point every strategy's result leaves through — so NativeSQL, ObjectQL and the FallbackDelegate pass-through are all covered by one gate; queryDataset reaches the same seam via DatasetExecutor and inherits it, and generateSql (the /analytics/sql dry-run route) is deliberately left ungated. New host switch `debugSql` on AnalyticsServicePlugin -> AnalyticsServiceConfig; unset resolves to NODE_ENV === 'development' and nothing else, so an unset NODE_ENV counts as production (the 2026-08-06 machine-readable-environment ruling, same reading os start / os serve / os doctor use). No request field, deliberately: a caller-settable flag would let any tenant reopen the disclosure. Kept separate from the plugin's existing `debug` log-verbosity option so raising log level cannot widen what travels to a tenant. packages/spec untouched. Two surprises worth naming rather than burying: (1) the seam gate makes the EXISTING absence pin at cross-field-engine-fallback.test.ts:469 vacuous — it would pass for free against a service that echoes nothing — so that suite now enables the echo explicitly and the pin measures the renderer's decline again, as written; (2) the alternative route the card offered (drop `sql` entirely) was measured, not assumed — nothing reads the echo, in this repo or in ../objectui — but I took the debug gate per ruling 1, because deleting a declared response field to fix an enforcement bug is a contract change made to cover an implementation defect.",
  "tests": "New src/__tests__/sql-echo-debug-gate.test.ts, 15 pins, every absence arm paired with a presence twin on the same cube/query/rows differing only in the switch, and every arm captures the statement server-side so absence means withheld rather than never-minted. REVERSE VERIFICATION (direction predicted before running, measured on origin/main with the two source files reverted and the new tests kept): 8 RED / 7 GREEN, exactly as predicted. The 8 reds are every absence arm and they are what proves the fix — NativeSQL debug-off, NativeSQL isolation-predicate, FallbackDelegate debug-off, ObjectQL debug-off, NODE_ENV=production, NODE_ENV unset, explicit-choice-outranks-env, queryDataset debug-off. Actual reading: `AssertionError: the echo must be absent, not half-rendered: expected 'SELECT COUNT(*) AS \"count\" FROM \"sys_…' to be undefined`. The 7 greens (all presence arms + the ungated dry-run route) are GREEN BOTH BEFORE AND AFTER — on main they pass for the wrong reason, since an unconditional echo satisfies 'present when debug is on' — so they accompany the fix, they do not prove it. Final state: `pnpm --filter @objectstack/service-analytics test` -> 'Test Files 74 passed (74) / Tests 1670 passed (1670)'. DOWNSTREAM consumer sweep (prefix filter '...@objectstack/service-analytics' — the packages that consume this one, which is the direction a response narrowing lands): @objectstack/rest 'Test Files 108 passed (108) / Tests 1800 passed (1800)' and @objectstack/runtime 'Test Files 150 passed (150) / Tests 2306 passed (2306)', run against the rebuilt dist. `tsc --noEmit -p packages/services/service-analytics/tsconfig.json` with the closure built: 10 errors, identical to the 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. I re-derived the list against my actual changed paths with scripts/pm/dispatch-gates.mjs; it surfaced five families the dispatch prompt did not name (check:changeset-gate-self-tests, check:objectui-changeset, check:objectui-pin-fresh, check-changeset-no-major.mjs, check:query-options-erasure) plus the two convention-triggered ones (check:query-options-erasure, check:type-check-coverage) — all run. Two local reds that are NOT this diff: check:type-check-debt cannot run locally without the whole workspace build closure on disk (the known #8271 local-ergonomics limitation; CI builds it first, and the number it would measure is the 10 pinned above), and check-objectui-pin-fresh is red because .objectui-sha no longer describes objectui main — that file is byte-identical to origin/main on this branch and the diff touches no objectui file; it matched only because the diff adds a .changeset entry. CI status at report time: in_progress (reporting at draft-PR time per the 2026-08-10 ruling).",
  "open_questions": [],
  "out_of_scope_findings": []
}

Generated by Claude Code

Review of PR #8312 — ACCEPT, pending CI convergence

Diff read in full. Path face: 7 files, all under packages/services/service-analytics/src/** plus the changeset — no docs/adr/**, .claude/skills/**, skills/**, content/docs/releases/**; packages/spec untouched, so ruling 2 held.

Ruling 3 satisfied by construction, not by three copies

The gate is applySqlEchoPolicy, applied at AnalyticsService.query — the single seam every strategy's result leaves through. NativeSQLStrategy (returns the statement it ran), ObjectQLStrategy (renders a representative one) and FallbackDelegateStrategy (passes through whatever the delegate minted) are all covered by one gate, and queryDataset inherits it via DatasetExecutor rather than needing a second gate to keep in step. generateSql — the /analytics/sql dry-run route — is deliberately left ungated, correctly: gating it would not narrow an over-serving response, it would delete a declared route.

Three judgment calls I want on the record, because each could have gone wrong quietly

1. The default is fail-closed on the absent case. config.debugSql ?? (getEnv('NODE_ENV') === 'development') — an unset NODE_ENV resolves to not development, so the echo stays off. That inherits the 2026-08-06 machine-readable-environment ruling and matches how os start / os serve / os doctor already read absence. The reasoning is stated where it belongs: "of the two ways to be wrong, disclosing on a production deployment whose operator forgot the variable is the dangerous one."

2. No request field, and that is the security judgment. A caller-settable debug flag would let any tenant reopen the disclosure on demand — "the shape of the defect rather than a fix for it." Correct, and worth naming because a request flag is the obvious implementation and the one a hurried author reaches for.

3. Kept separate from the plugin's existing debug log-verbosity option. Folding them together would mean a support engineer raising log level on a live deployment silently reopens the disclosure to tenants. Two switches, named for what they open. This is the kind of coupling that is invisible until an incident.

Also correct and easy to get wrong: applySqlEchoPolicy copies and deletes rather than mutating, because the strategy or a delegated service owns the object it returned — mutating would strip the field from a cached upstream result as a side effect of one caller's request.

The anti-vacuity work exceeded what I asked for

I required every absence pin to be paired with a presence twin. It did that, and added the half I did not specify: every arm captures the statement server-side (executed[], the delegate's return, the renderer) and asserts it is a real statement before claiming absence — so absence provably means withheld, not never-minted. The isolation-predicate arm goes further and asserts the whole serialized response contains no sys_user, no IN (, no SELECT — checking the wire, not just the field.

And the NODE_ENV=development presence arm carries its own reason: "Without it, the two arms above would be satisfied by a build that had simply deleted the echo, and this suite could not tell a gate from a deletion." That distinction — gate vs deletion — is precisely the one a suite of absence assertions loses.

It caught its own change hollowing out someone else's pin

The one I most want recorded. The existing pin at cross-field-engine-fallback.test.ts:469 asserts sql is absent to prove the renderer declined. After a seam gate, that pin would pass for free against a service that echoes nothing — "it would stop measuring the renderer's decline and start measuring the gate." So that suite now enables the echo explicitly and the pin measures what its file says it measures again.

Nobody would have noticed this from a green suite. Same treatment applied to dataset-selection-window.test.ts, whose subject is the echo's content, and which additionally stops inheriting whatever NODE_ENV the runner happens to have.

And analytics-service.test.ts:429 was correctly classified. Triage flagged it as possibly asserting the defect. On reading, its subject is routing — which strategy served — with the echo as the witness. That is the "incidental observation" case, so it was re-pointed with the precondition stated, ⛔ not replaced. That is the distinction I asked for and it landed on the right side of it.

The alternative route was measured, then declined for a stated reason

The card offered dropping sql entirely. The dev measured that nothing reads the echo — this repo and ../objectui — and still took the debug gate, because "deleting a declared response field to fix an enforcement bug is a contract change made to cover an implementation defect." Agreed, and consistent with ruling 2.

Reverse verification, honestly apportioned

8 RED / 7 GREEN, direction predicted first. The 8 reds are every absence arm and they are what proves the fix. The 7 greens are stated as green both before and after — on main they pass for the wrong reason, since an unconditional echo satisfies "present when debug is on" — "so they accompany the fix, they do not prove it." That is exactly the apportionment I asked for, volunteered at the right granularity.

Two local reds, correctly excluded — with one referral

Neither is this diff:

check:type-check-debt cannot run locally without the workspace build closure — the known 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 local-ergonomics limitation, which this lane got filed earlier today. Nice to see it cited rather than re-discovered.
check-objectui-pin-fresh is red because .objectui-sha no longer describes objectui main. That file is byte-identical to origin/main on this branch and the diff touches no objectui file — it matched only because the diff adds a .changeset entry.

⚠️ That second one is a genuine pin-lag observation, and it is the queue steward's beat (#6016), not this card's: an objectui pin drifting behind is the standing trigger for a console-bump card. Referring rather than acting — ⛔ this lane does not execute pin bumps.

⚠️ Correction to my review — I passed along a local-only gate reading as a real observation

In the review above I wrote that the dev's local check-objectui-pin-fresh red was "a genuine pin-lag observation" and referred it to the queue steward (#6016) as a console-bump trigger.

CI contradicts that. On this PR's own run, Console Pin Freshness concluded success (job 94344048765, 04:29:08Z). So main's .objectui-sha is not measured stale by the gate that actually decides it, and there is no pin-lag evidence here.

The dev reported the local red accurately and correctly excluded it from its diff — the fault is mine: I upgraded "red on one worktree" into "a real observation about main" and forwarded it. The likelier explanation is the mundane one: the freshness check compares the pinned sha against whatever ../objectui checkout is on that machine, so a local checkout sitting at a different commit reads as drift that does not exist on main.

⇒ The steward referral is withdrawn. ⛔ No console-bump card is warranted on this evidence. If a real pin lag exists it will show up where such things are actually decided — Console Pin Freshness going red on main, or the steward's own pin-chain observation — not in a worktree's local run.

This is the third local-vs-CI divergence this round, and the pattern is now the point

check:type-check-debt is red on main: @objectstack/service-analytics DEBT drifted 10 → 14 (+4) with #8198 #8235 — a priority:p0 filed because check:type-check-debt was red locally. CI green on main; card closed not_planned, real residual preserved as 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 (the gate trusts a stale dist/ unless the caller builds the closure).
sys_secret: rotations that happened BEFORE #8030's fix left decryptable orphans on deployed instances, and nothing reaps them #8103's PR — body claimed check:objectui-pin-fresh red; CI's Console Pin Freshness success. The dev removed the claim when I flagged it.
This one — same gate, same shape, and this time I was the one who propagated it.

Every instance ran the same direction: the local reading manufactures a problem that CI does not see. A gate that is red on one machine and green in CI is evidence about the machine until proven otherwise, and ⛔ it must not be forwarded to another seat as a finding without a CI or origin/main reading behind it.

Recording on the seat post as a standing check rather than leaving it as three separate anecdotes.

Generated by Claude Code

fix(service-analytics): gate the /analytics/query SQL echo on debug (o…

Navigation Menu

Repository navigation

analytics: /analytics/query echoes the executed SQL in production (NODE_ENV=production, no debug requested) — schema documents it as debug-only #8286

Repro

Expected

Why it matters here specifically

Suggested fix

Environment

Review of PR #8312 — ACCEPT, pending CI convergence

Ruling 3 satisfied by construction, not by three copies

Three judgment calls I want on the record, because each could have gone wrong quietly

The anti-vacuity work exceeded what I asked for

It caught its own change hollowing out someone else's pin

The alternative route was measured, then declined for a stated reason

Reverse verification, honestly apportioned

Two local reds, correctly excluded — with one referral

Next

⚠️ Correction to my review — I passed along a local-only gate reading as a real observation

This is the third local-vs-CI divergence this round, and the pattern is now the point

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

analytics: /analytics/query echoes the executed SQL in production (NODE_ENV=production, no debug requested) — schema documents it as debug-only #8286

Description

Repro

Expected

Why it matters here specifically

Suggested fix

Environment

Activity

hotlong commented on Aug 13, 2026

os-zhuang commented on Aug 13, 2026

os-zhuang commented on Aug 13, 2026

os-zhuang commented on Aug 13, 2026

Review of PR #8312 — ACCEPT, pending CI convergence

Ruling 3 satisfied by construction, not by three copies

Three judgment calls I want on the record, because each could have gone wrong quietly

The anti-vacuity work exceeded what I asked for

It caught its own change hollowing out someone else's pin

The alternative route was measured, then declined for a stated reason

Reverse verification, honestly apportioned

Two local reds, correctly excluded — with one referral

Next

os-zhuang commented on Aug 13, 2026

⚠️ Correction to my review — I passed along a local-only gate reading as a real observation

This is the third local-vs-CI divergence this round, and the pattern is now the point

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions