Skip to content

Commit 9b459b7

Browse files
os-litantclaude
andauthored
fix(rest): compile the data doors' protocol requests against the declared contract (#16071)
* fix(rest): compile the data doors' request literals against the declared protocol contract The DELETE and PATCH data handlers dispatched through `p.deleteData({…} as any)` / `p.updateData({…} as any)`, which erased TypeScript's check of the assembled request against `DeleteDataRequest` / `UpdateDataRequest`. A sweep found the same erasure in two forms across 22 protocol-dispatch sites in this file, not two. The casts were load-bearing, as filed: `environmentId` and `context` are passed at these call sites and are members of no data request schema. Neither belongs in one: `environmentId` is the transport routing key already ruled out of the protocol request shape (2026-08-18, #9741), and `context` is the SERVER-DERIVED execution context whose caller-supplied form is a privilege escalation the ingress deletes unconditionally. So both are declared on a typed envelope beside the ruled `TransportScopedMetaRequest`, and every other member of every literal is now compiled against the spec contract. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * test(rest): record the execctx census mention move, and add the changeset The `ServerScopedDataRequest` doc-comment names `resolveExecCtx` to say where its `context` member comes from, which moves the census's prose-mention control from 98 to 99. The invocation-site control is UNCHANGED at 77 — no consumer was added, moved or removed — which is the split that census exists to keep visible, and the entry records it in the block's house style. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * docs(permissions): re-anchor the system-context census citations after the line shift Pure line rot, applied with the gate's own `--fix`: the typed-envelope declaration and its imports sit near the top of `rest-server.ts`, so every `isSystem` citation below them moved by a constant +86. No prose, no row and no verdict changed — only the `file:line` anchors. `check-system-context-census` was verified GREEN at the merge base first, so this is a shift this branch caused rather than one it inherited. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * docs(permissions): restore the sharing-rule-service anchors the merge reverted ⚠️ The merge with `origin/main` was textually CLEAN and semantically wrong, which on a line-anchor page is the failure mode to expect rather than a surprise: `main` had re-anchored row 39 to `sharing-rule-service.ts:278`/`:503`, the merge resolved that line to this branch's older `:202`/`:427`, and nothing about the merge said so. Only `check-system-context-census` did — it was verified GREEN at `origin/main` (3140345) first, so the four findings were the merge's and not inherited. Re-derived from the merged sources with the gate's own `--fix`; the two anchors now read what `main` set them to. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 64011dd commit 9b459b7

4 files changed

Lines changed: 191 additions & 52 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
"@objectstack/rest": patch
3+
---
4+
5+
The REST data doors' protocol requests are compiled against the declared contract again, so a field added to a data request schema reddens the build instead of going silently unsent.
6+
7+
No runtime behaviour changes — every door assembles and forwards exactly the object it did before. What changes is what the compiler is allowed to see. `packages/rest/src/rest-server.ts` dispatched to the protocol through two erasing forms: `p.deleteData({ … } as any)` on the argument, and the stronger `(p as any).updateData({ … })` on the protocol object itself, which erases the check on *every* member — a misspelled method name would not have errored. Across the file that was 22 dispatch sites spanning `findData` / `getData` / `createData` / `updateData` / `deleteData`, their `*Many` and batch siblings, and `getUiView`.
8+
9+
The casts were load-bearing rather than lazy: these call sites pass `environmentId` and `context`, and neither is a member of any data request schema. Neither should become one. `environmentId` is the transport routing key that selects the kernel *before* the protocol call and is already ruled out of the request shape; `context` is the server-derived execution context, and a caller-supplied `context` is a privilege escalation the ingress deletes unconditionally — putting it in the published request schema would re-open that door. Both are now declared on a typed envelope alongside the request type, so they stay server-side *and* compiled, and every other member of every literal is checked against the spec.
10+
11+
One slot stays deliberately untyped and is now named rather than diffuse: `findData`'s `query` accepts both the declared AST and an undeclared wire dialect (`$top`, `$orderby`, `filters`, …) that the protocol normalizer folds. Three server-built literals speak that dialect; the erasure there is confined to the query slot alone, and the declared-versus-shipped mismatch is filed as its own question.

content/docs/permissions/system-context.mdx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ not on any flag.
6464
## How the flag is set
6565

6666
`isSystem` is **server-constructed and never client-supplied**. Inbound HTTP
67-
cannot set it (`packages/rest/src/rest-server.ts:1565`, `:1594`), and neither
67+
cannot set it (`packages/rest/src/rest-server.ts:1651`, `:1680`), and neither
6868
can an action body (`packages/runtime/src/domains/actions.ts:414`). It is
6969
written by internal callers only, as an option on the engine call:
7070

@@ -103,7 +103,7 @@ that silently does not happen.
103103
| 14 | MCP stdio bridge skips the object API-exposure gate | mcp | Get: the bridge reaches objects whose `apiEnabled` / `apiMethods` would refuse an external caller | `stdio-data-bridge.ts:250` |
104104
| 15 | **Read-audit rows are not written** | plugin-audit | Lose: the "a person opened this record" trail. `sudo()` keeps the caller's `userId`, so this flag is the only thing separating a human read from a platform one | `read-audit.ts:556` |
105105
| 16 | Approval snapshot payload redaction skipped | plugin-approvals | Get: the whole snapshot on `find` / `findOne` — the audit/replay channel. Lose: field-visibility redaction over approval payloads | `payload-redaction-middleware.ts:115` |
106-
| 17 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `rest-server.ts:1597` |
106+
| 17 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `rest-server.ts:1683` |
107107

108108
### 2. Write pipeline and data integrity
109109

@@ -158,7 +158,7 @@ The largest single consumer — **17 of the 105 sites**.
158158
|:--|:---|:---|:---|:---|
159159
| 48 | Object API-exposure gate bypassed (`apiEnabled` / `apiMethods`) | runtime | Get: internal self-writes ignore exposure declarations — these govern **external** exposure, not engine self-writes | `action-execution.ts:138` |
160160
| 49 | Action `requiredPermissions` bypassed | runtime | Get: engine self-invocation runs any action | `action-execution.ts:401` |
161-
| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:5145`, `:6571`, `:6819`, `:7250`, `:7443` |
161+
| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:5231`, `:6657`, `:6905`, `:7336`, `:7529` |
162162
| 51 | The shared metadata-write verdict itself returns `allowed` | metadata-core | Get: the one function all of row 50's doors consult answers yes before any capability is examined | `meta-write-capability.ts:134` |
163163
| 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `domains/actions.ts:421`, `domains/ai.ts:60`, `domains/automation.ts:989`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:535`, `external-datasource-routes.ts:302`, `package-routes.ts:97` |
164164
| 53 | MCP principal check satisfied | runtime | Get: MCP surface reachable with no user | `domains/mcp.ts:61` |
@@ -199,7 +199,7 @@ assuming `isSystem` covers it is a documented source of bugs.
199199
| "It preserves a supplied `updated_at` / `updated_by`" | **No.** That is `preserveAudit`, a separate opt-in — and an UPDATE-path exemption only | `field.zod.ts:1590` (#3493 / #6640) |
200200
| "It stamps `created_by`" | **No.** Audit stamping reads `userId` from the context. A user-less system write stamps nothing — that is today's behaviour, not an error | `runtime-identity.ts:280``281` |
201201
| "It bypasses every guard" | **No.** The last-admin guard applies to **every** context, `isSystem` included — the deprovision path that actually locks an org out is the system one | `last-admin-guard.ts:299` |
202-
| "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1565`, `:1594`; `domains/actions.ts:414` |
202+
| "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1651`, `:1680`; `domains/actions.ts:414` |
203203

204204
---
205205

packages/rest/src/execctx-consumer-census.test.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,22 @@ describe('[#13160] §1 the production supplier fulfils with `undefined` rather t
309309
// ---------------------------------------------------------------------------
310310

311311
describe('[#13160] §2 the consumer surface, counted from the tree', () => {
312-
it('77 invocation sites, 98 mentions — the thread\'s two control numbers hold', () => {
312+
it('77 invocation sites, 99 mentions — the thread\'s two control numbers hold', () => {
313+
// [#15866] 77 sites UNCHANGED / 98 → 99 mentions — the fourth pattern,
314+
// and the first entry here that moves the mention count while adding no
315+
// consumer at all. That card retired the `as any` casts on this file's
316+
// protocol-dispatch sites, so each data door's request literal is now
317+
// compiled against the declared spec contract through a typed envelope
318+
// (`ServerScopedDataRequest`). The envelope's doc-comment has to say
319+
// where its `context` member comes from — it is the SERVER-DERIVED
320+
// execution context, which is the whole reason it may not join the
321+
// published request schema — and naming {@link RestServer.resolveExecCtx}
322+
// is how it says so. ⚠️ No call site was added, moved or removed: the
323+
// repair is a type annotation, and `SITES.length` staying at 77 across
324+
// it is the assertion that says so. A reader who sees only the mention
325+
// count move should read it as documentation about the consumers, never
326+
// as a consumer.
327+
//
313328
// [#13753, the `/references` half] 76 → 77 sites / 97 → 98 mentions.
314329
// `GET /meta/:type/:name/references` resolved NO identity, so the
315330
// reference sweep behind the admin "Used by" panel read the env
@@ -378,7 +393,7 @@ describe('[#13160] §2 the consumer surface, counted from the tree', () => {
378393
// that tracked the site count exactly would be measuring one thing
379394
// twice.
380395
expect(SITES.length).toBe(77);
381-
expect(SOURCE.split('resolveExecCtx').length - 1).toBe(98);
396+
expect(SOURCE.split('resolveExecCtx').length - 1).toBe(99);
382397
});
383398

384399
it('the split is 24 locally caught / 53 bare — NOT 16 / 53, which does not add to 77', () => {

0 commit comments

Comments
 (0)