|
| 1 | +--- |
| 2 | +"@objectstack/types": minor |
| 3 | +"@objectstack/runtime": minor |
| 4 | +"@objectstack/rest": patch |
| 5 | +--- |
| 6 | + |
| 7 | +fix(runtime,types)!: `/analytics/query` no longer echoes RLS policy field names — the declared-server-fault withhold is shared by both HTTP boundaries (#5811) |
| 8 | + |
| 9 | +**Observable behaviour change — read this if you read, log, or assert on |
| 10 | +`error.message` from a dispatcher-plugin route.** An error that **declares a |
| 11 | +server fault** in the ADR-0112 envelope (`status >= 500` *and* a non-empty |
| 12 | +`code`) now leaves `dispatcher-plugin.errorResponseBase` with its message |
| 13 | +replaced by `"Internal server error"`. It previously reached the caller verbatim |
| 14 | +unless it happened to *sound* like a SQL/driver dump. This applies to every route |
| 15 | +that plugin mounts — `/analytics`, `/packages`, `/i18n`, `/automation`, `/auth`, |
| 16 | +`/notifications`, `/mcp`, … — not only the one that motivated it. Nothing a |
| 17 | +machine reads changed: the producer's `code` still arrives in the response |
| 18 | +(`error.code`, promoted there from `details` by the shared envelope builder, |
| 19 | +#3842), the status is untouched, and the full original text still goes to the |
| 20 | +server log and `errorReporter` via `__obsRecordedError`. |
| 21 | + |
| 22 | +## What was wrong |
| 23 | + |
| 24 | +#5367 (maintainer ruling 2026-08-06) made `read-scope-sql.ts`'s ten fail-closed |
| 25 | +RLS lowering refusals `READ_SCOPE_COMPILE_FAILED` / 500 and taught |
| 26 | +`POST /analytics/dataset/query` to withhold their message, because those messages |
| 27 | +name the field names and comparands of an **administrator's** sharing rule: |
| 28 | + |
| 29 | +``` |
| 30 | +[read-scope-sql] unsafe field identifier "secret_policy_field" — refusing to |
| 31 | +build read scope (fail-closed). |
| 32 | +``` |
| 33 | + |
| 34 | +The caller never wrote that field name and must not be able to read it out of an |
| 35 | +error body. But the **sibling** analytics face was never closed. |
| 36 | +`compileScopedFilterToSql` runs on both `NativeSQLStrategy.applyReadScope` and |
| 37 | +`ObjectQLStrategy`'s echoed SQL, both of which serve `POST /analytics/query`, |
| 38 | +which exits through `dispatcher-plugin.errorResponseBase`. That exit's only |
| 39 | +message guard was `looksLikeInternalErrorLeak` — a heuristic over SQL/driver |
| 40 | +*phrasing* — and all eleven read-scope message shapes return `false` from it. |
| 41 | +Measured at that boundary: **11 of 11 echoed verbatim**, at 500, with the policy |
| 42 | +content in `error.message`. A real reachable disclosure, not a theoretical one. |
| 43 | + |
| 44 | +## What changed |
| 45 | + |
| 46 | +- **`@objectstack/types` gains `declaresServerFault(err)`**, exported from |
| 47 | + `error-leak.ts` beside `looksLikeInternalErrorLeak`. The heuristic asks whether |
| 48 | + a message *sounds* internal; the declaration asks whether the producer *said |
| 49 | + so*. `error-leak.ts`'s own file header already states the principle — "do not |
| 50 | + ship driver internals to clients" is a property of the HTTP boundary, not of |
| 51 | + one router — and this is the second predicate that principle asks for. |
| 52 | +- **Both boundaries read it.** `dispatcher-plugin.errorResponseBase` gains the |
| 53 | + withhold (the fix); `rest-server.ts`'s `/analytics/dataset/query` catch drops |
| 54 | + its in-line copy of the same test in favour of the shared one. #5808 wrote that |
| 55 | + rule in-line on purpose — promoting a rule with one consumer is a speculative |
| 56 | + surface — and this is the second consumer, so it was promoted rather than |
| 57 | + duplicated (`#3843`/`#3867` paid for the two-implementations shape twice). |
| 58 | + The REST face's verdict is unchanged in every case: same `status >= 500` plus |
| 59 | + non-empty `code` test, over the same two fields. |
| 60 | + |
| 61 | +## What deliberately did NOT change |
| 62 | + |
| 63 | +- ⛔ **This is not "withhold every 5xx".** #5667 kept **undeclared** 5xx errors |
| 64 | + legible on purpose: a bare `Error` from our own code ("no strategy can handle |
| 65 | + query …") is the operator's own bug report, names nothing tenant-sensitive, and |
| 66 | + still falls to `looksLikeInternalErrorLeak` alone. A 5xx carrying only half an |
| 67 | + envelope (a status with no code) is likewise still readable — inventing the |
| 68 | + withhold for it would be the consumer-side leniency Prime Directive #12 removes. |
| 69 | +- **4xx is untouched.** `declaresServerFault` requires `status >= 500`, so a |
| 70 | + deliberate business/validation answer can never be swallowed by it. |
| 71 | +- **`statusCode` is not accepted as a substitute for `status`.** `status` is the |
| 72 | + channel ADR-0112 declares; making a disclosure rule depend on which spelling a |
| 73 | + producer reached for would be the same leniency in a different place. |
| 74 | +- **The heuristic was not taught to recognise `[read-scope-sql]`.** That would be |
| 75 | + more prose sniffing — the mechanism #5352/#5367 exist to remove — and would only |
| 76 | + ever cover the family someone remembered to add. |
| 77 | + |
| 78 | +Coverage: `analytics-query-read-scope-withhold.test.ts` (runtime) drives six RLS |
| 79 | +policy shapes end-to-end through a **real** `AnalyticsService` on the real |
| 80 | +native-SQL path and the real mounted route, asserting the 500, that the whole |
| 81 | +serialized body contains no policy detail, that `error.code` still carries |
| 82 | +`READ_SCOPE_COMPILE_FAILED`, and that the full text is still on the |
| 83 | +`__obsRecordedError` side-channel — plus a positive control and both sides of the |
| 84 | +declared-vs-undeclared tiering. `error-leak.test.ts` (types) pins the predicate |
| 85 | +directly, including that all eleven read-scope shapes stay invisible to the |
| 86 | +heuristic. The REST face's existing `analytics-read-scope-refusal-envelope.test.ts` |
| 87 | +is green before and after, unchanged, which is the pin on the refactor. |
0 commit comments