|
| 1 | +--- |
| 2 | +"@objectstack/service-analytics": patch |
| 3 | +"@objectstack/rest": patch |
| 4 | +"@objectstack/spec": patch |
| 5 | +--- |
| 6 | + |
| 7 | +fix(analytics,rest)!: an RLS read-scope lowering failure is a `500`, not the caller's `400` — and its policy detail no longer reaches the response (#5367) |
| 8 | + |
| 9 | +**Observable behaviour change — read this if you alert, retry, or assert on status.** |
| 10 | +A request whose dataset carries an RLS read scope that `read-scope-sql.ts` cannot |
| 11 | +lower used to answer `400 DATASET_INVALID` with the refusal message echoed |
| 12 | +verbatim. It now answers `500 ANALYTICS_QUERY_FAILED` with the message withheld |
| 13 | +(`"Internal server error"`); the full text goes to the server log. Monitoring that |
| 14 | +counted these as client errors will see a 4xx disappear and a 5xx appear, and a |
| 15 | +client retrying on 5xx will now retry a request that cannot succeed until an |
| 16 | +administrator fixes the policy. Both follow from the correction below and are |
| 17 | +stated rather than buried. |
| 18 | + |
| 19 | +## What was wrong |
| 20 | + |
| 21 | +These ten fail-closed refusals were the last family `/analytics/dataset/query` |
| 22 | +classified by **prose** — the final entry of the hardcoded message-substring list |
| 23 | +#5352 introduced, which #5367's first PR had already shrunk from six entries to |
| 24 | +one. Two defects in one verdict: |
| 25 | + |
| 26 | +- **Misattribution.** `compileScopedFilterToSql(filter, alias)` receives an RLS |
| 27 | + `FilterCondition` the security service compiled from an **administrator's** |
| 28 | + sharing rule / permission set, and a join alias the **dataset compiler** |
| 29 | + generated. Neither is caller input — the caller's own predicate goes through |
| 30 | + `filter-normalizer.ts` and has answered `INVALID_FILTER` / 400 since #5352. So |
| 31 | + what can arrive here is a broken policy, or drift between two of our own |
| 32 | + components (#5557's `$regex` was literally the second case). For this request's |
| 33 | + caller both are a **server** fault; `400` told them to fix a request that was |
| 34 | + never wrong and kept the real fault out of 5xx alerting. |
| 35 | +- **Disclosure.** A 400 echoed the message, so |
| 36 | + `unsafe field identifier "secret_policy_field"` and |
| 37 | + `unsupported operator "$regex" on "owner_email"` handed a tenant the field names |
| 38 | + and comparands of the RLS policy governing them. |
| 39 | + |
| 40 | +The maintainer ruled on 2026-08-06 (option B on #5367's decision card; option A |
| 41 | +was `READ_SCOPE_INVALID` / 422, rejected because no consumer reads a code on this |
| 42 | +path, a 4xx misreports a condition the client cannot fix, and 422 would have left |
| 43 | +the disclosure question to be re-decided message by message). |
| 44 | + |
| 45 | +## What changed |
| 46 | + |
| 47 | +- `read-scope-sql.ts` gains a module-local `readScopeCompileError` — the twin of |
| 48 | + `filter-normalizer.ts`'s `invalidFilterError`, and likewise **the only way the |
| 49 | + module refuses**. All ten sites carry `READ_SCOPE_COMPILE_FAILED` / **500**. |
| 50 | + `:104`'s alias-vs-field split (option C on the card) collapses under B: both |
| 51 | + branches answer the same verdict, pinned so the collapse is a recorded decision. |
| 52 | +- `rest-server.ts` loses branch ② entirely. **The message-sniffing mechanism is |
| 53 | + fully retired** — nothing in this catch reads prose any more, and #5367's |
| 54 | + Prime-Directive-#12 retirement schedule ("declared, loud, tested AND removable |
| 55 | + on a schedule") is paid off. |
| 56 | +- The route's 5xx branch now withholds the message of any producer that |
| 57 | + **declares** a server fault (`status >= 500` with a `code`). This was needed |
| 58 | + rather than inherited: `looksLikeInternalErrorLeak` (#3867/#5520) is a heuristic |
| 59 | + over SQL/driver *phrasing*, and measured, every read-scope message returns |
| 60 | + `false` from it — so retiring the list alone would have moved the policy content |
| 61 | + from a 400 body into a 500 body instead of out of the response. Teaching that |
| 62 | + heuristic to recognise `[read-scope-sql]` would have been *more* message |
| 63 | + sniffing, so the rule keys on the ADR-0112 envelope instead. **Undeclared** 5xx |
| 64 | + errors keep #5667's tiering, so a self-authored fault ("no strategy can handle |
| 65 | + query …") stays readable. |
| 66 | +- `READ_SCOPE_COMPILE_FAILED` is registered in `ERROR_CODE_LEDGER` under |
| 67 | + `@objectstack/service-analytics` (ADR-0112 D3) and typed as |
| 68 | + `RegisteredErrorCode` at the constructor, so an unregistered code is a compile |
| 69 | + error. It is legible on the wire through the sibling `/analytics/query` exit, |
| 70 | + which puts a thrown `err.code` in `error.details.code` (#3842). |
| 71 | + |
| 72 | +**Which inputs are refused did not change.** No refusal condition moved: nothing |
| 73 | +that used to lower now throws, and nothing that used to throw now lowers. That is |
| 74 | +pinned input-by-input — refusals *and* accepted read scopes with their compiled |
| 75 | +SQL and bind params — in `read-scope-refusal-envelope.test.ts`, which is green both |
| 76 | +before and after; only the envelope assertions move. |
| 77 | + |
| 78 | +Coverage: `read-scope-refusal-envelope.test.ts` (service-analytics) drives all ten |
| 79 | +sites through the real compiler; `analytics-read-scope-refusal-envelope.test.ts` |
| 80 | +(rest) drives five policy shapes end-to-end through a real `AnalyticsService`, |
| 81 | +asserting the 500, that the body contains no policy detail, and that the withheld |
| 82 | +text is present in the log — plus a positive control and both sides of the |
| 83 | +declared-vs-undeclared withhold. |
0 commit comments