Skip to content

Commit dd999ce

Browse files
os-litantclaude
andauthored
fix(runtime): type the packages-domain protocol service handle so undeclared request keys are compile errors (#15215)
* wip(runtime): type the packages-domain protocol service handle Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * wip(runtime): add the packages-domain protocol handle typing pin Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * chore(changeset): patch note for the packages-domain protocol handle typing 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 rows moved by the typing block Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * docs(permissions): regenerate the system-context census from the merged tree 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 97bcd99 commit dd999ce

4 files changed

Lines changed: 359 additions & 37 deletions

File tree

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
"@objectstack/runtime": patch
3+
---
4+
5+
fix(runtime): the packages domain reaches the `protocol` service through a typed handle (#13598)
6+
7+
`deps.resolveService(context, 'protocol')` answers `any``protocol` is
8+
deliberately left unmapped in `ServiceSlotContracts` — so every request literal
9+
downstream of that seam compiled against nothing. Twelve sites in
10+
`domains/packages.ts` held that `any` (two of them on the variable declaration
11+
rather than the call), and an undeclared or misspelt key in the ADR-0045
12+
publish-visibility flip's `getMetaItems` / `saveMetaItem` literals compiled
13+
silently. Measured on the base tree: injecting `bogusUndeclaredKey: true` into
14+
the `saveMetaItem` literal gave `tsc --noEmit` exit 0 and zero diagnostics.
15+
16+
The slot is now narrowed once, at one helper, to a handle `Pick`ed from the
17+
DECLARED contracts — `MetadataProtocol` / `PackageProtocol` from
18+
`@objectstack/spec`, plus the producer's own exported `DeletePackageRequest`
19+
so the same injection is now `error TS2353`. Every member is OPTIONAL and every
20+
`typeof protocol.<verb> === 'function'` capability probe is unchanged: a host
21+
may occupy the slot with a partial object, and the type answers "is this key
22+
declared?" while the probe still answers "did this host bring the verb?".
23+
24+
Compile-layer signal only — no request is newly accepted or refused, no
25+
response shape moves, and the eight verbs no contract declares keep an explicit
26+
`any` request rather than a private restatement nothing verifies.

content/docs/permissions/system-context.mdx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,10 +160,10 @@ The largest single consumer — **17 of the 106 sites**.
160160
| 49 | Action `requiredPermissions` bypassed | runtime | Get: engine self-invocation runs any action | `action-execution.ts:401` |
161161
| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:4888`, `:6302`, `:6550`, `:6981`, `:7174` |
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` |
163-
| 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `domains/actions.ts:411`, `domains/ai.ts:60`, `domains/automation.ts:989`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:326`, `external-datasource-routes.ts:302`, `package-routes.ts:97` |
163+
| 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `domains/actions.ts:411`, `domains/ai.ts:60`, `domains/automation.ts:989`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:422`, `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` |
165165
| 54 | Package REST route capability gate bypassed | rest | Get: package read/write over REST without `manage_metadata` / `studio.access` / `setup.access` | `package-routes.ts:102` |
166-
| 55 | Package domain capability gates bypassed | runtime | Get: package management and package-inventory reads without the capability | `domains/packages.ts:145`, `:178` |
166+
| 55 | Package domain capability gates bypassed | runtime | Get: package management and package-inventory reads without the capability | `domains/packages.ts:241`, `:274` |
167167
| 56 | Activation write / authoring refusals do not fire | runtime | Get: activation artifacts writable and authorable without the activation-authoring capability | `activation-gate.ts:138`, `:189` |
168168
| 57 | Automation run-state read, flow-authoring write and unrelated-screen read all pass | runtime | Get: run state, flow writes and screen reads with no grant | `domains/automation.ts:254`, `:545`, `:635` |
169169
| 58 | Audience-binding suggestion recording skipped | plugin-security | Lose: install-time suggestions are not recorded for system callers | `suggested-audience-bindings.ts:703` |
Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #13598 — the packages domain reaches the `protocol` service through a TYPED
5+
* handle, and the runtime capability probes survive that typing.
6+
*
7+
* Two halves, because the card has two halves that pull in opposite directions
8+
* and either one alone is a regression:
9+
*
10+
* 1. **Compile-time** (section 1). An undeclared key in one of this domain's
11+
* request literals must be a COMPILE ERROR. That is the #11006 series' end
12+
* state, and it stopped one seam short here.
13+
* 2. **Runtime** (section 2). ⛔ A host may occupy the `protocol` slot with a
14+
* PARTIAL object. Tightening the type and then deleting a
15+
* `typeof … === 'function'` probe would trade the compile-time improvement
16+
* for a runtime crash, so section 2 drives a real dispatcher whose protocol
17+
* brings none of the verbs and pins the documented 501s.
18+
*
19+
* ## The defect, measured on the base tree with the same instrument
20+
*
21+
* `deps.resolveService(context, 'protocol')` answers `any` — `protocol` is
22+
* deliberately unmapped in `ServiceSlotContracts`. Downstream of that seam
23+
* nothing compiled against a contract at all. Measured at `25a59bd`, injecting
24+
* one undeclared key (`bogusUndeclaredKey: true`) into the `saveMetaItem`
25+
* literal of the ADR-0045 visibility flip:
26+
*
27+
* tsc --noEmit -p packages/runtime/tsconfig.json -> exit 0, ZERO diagnostics
28+
*
29+
* The same injection into the same literal after this change:
30+
*
31+
* ... -> exit 2
32+
* packages.ts(727,41): error TS2353: Object literal may only specify known
33+
* properties, and 'bogusUndeclaredKey' does not exist in type
34+
* '{ type: string; name: string; item: unknown; organizationId?: … }'
35+
*
36+
* Section 1 is that measurement made DURABLE. Each `@ts-expect-error` below is
37+
* itself checked: if the seam ever goes back to `any` the directive stops
38+
* matching an error and tsc reports TS2578 (unused directive) — so this file
39+
* cannot rot into a green no-op the way an assertion-only pin could.
40+
*
41+
* ⚠️ These directives are NOT phantom checks: `packages/runtime`'s BUILD
42+
* tsconfig excludes every `.test.ts` under `src`, but the sibling
43+
* `tsconfig.test.json`
44+
* compiles this layer and `package.json`'s `typecheck` script names it via
45+
* `check:test-typecheck`. This file carries no entry in
46+
* `test-typecheck-debt.json`, so any error it gains beyond the expected ones is
47+
* red on arrival.
48+
*
49+
* ## Reverse verification — direction predicted BEFORE running
50+
*
51+
* Reverting `domains/packages.ts` to the base tree makes section 1 red as
52+
* TS2578 x4 (every directive becomes unused, because the `any` handle accepts
53+
* everything) — the reversal shape, not a plain "assertion failed", which is
54+
* why the directives are the pin and not `expectTypeOf` assertions. Section 2
55+
* is GREEN IN BOTH DIRECTIONS by construction: the probes it exercises are
56+
* unchanged by this card, so it is the control that says the 501s were never
57+
* bought with a behaviour change.
58+
*/
59+
import { describe, expect, it } from 'vitest';
60+
import { HttpDispatcher } from '../http-dispatcher.js';
61+
import type { PackagesDomainProtocol } from './packages.js';
62+
63+
// ---------------------------------------------------------------------------
64+
// Section 1 — compile-time pins (never executed; the checker is the assertion)
65+
// ---------------------------------------------------------------------------
66+
67+
/**
68+
* The literals this domain actually sends, spelled exactly as the handlers
69+
* spell them. A positive control for the four `@ts-expect-error`s below: if
70+
* this body ever stopped compiling, those directives could be "satisfied" by a
71+
* type that rejects everything, which pins nothing.
72+
*/
73+
function declaredKeysCompile(protocol: PackagesDomainProtocol) {
74+
return [
75+
// ADR-0045 visibility flip — `GET` half.
76+
protocol.getMetaItems?.({ type: 'app', packageId: 'crm', organizationId: 'org_1' }),
77+
// ADR-0045 visibility flip — `SAVE` half. `packageId` is declared
78+
// `nullable().optional()`, `actor` optional; both are load-bearing here.
79+
protocol.saveMetaItem?.({
80+
type: 'app',
81+
name: 'crm_console',
82+
item: { _unpublished: false },
83+
packageId: 'crm',
84+
organizationId: 'org_1',
85+
actor: 'u_publisher',
86+
}),
87+
// `applyPublishedSeeds`' seed body read-back, both attempts.
88+
protocol.getMetaItem?.({ type: 'seed', name: 'crm_seed', organizationId: 'org_1' }),
89+
protocol.getMetaItem?.({ type: 'seed', name: 'crm_seed' }),
90+
// The manifest-export read.
91+
protocol.getMetaItems?.({ type: 'view', packageId: 'crm', organizationId: undefined }),
92+
];
93+
}
94+
95+
/**
96+
* ⛔ THE PIN. Each directive must match a real diagnostic; an unused one is
97+
* TS2578 and fails `check:test-typecheck`.
98+
*/
99+
function undeclaredKeysAreCompileErrors(protocol: PackagesDomainProtocol) {
100+
return [
101+
protocol.saveMetaItem?.({
102+
type: 'app',
103+
name: 'crm_console',
104+
item: {},
105+
// @ts-expect-error [#13598] `packagId` is a misspelling of the
106+
// declared `packageId`. Through the pre-change `any` handle this
107+
// compiled, and the write silently landed unbound to the package.
108+
packagId: 'crm',
109+
}),
110+
protocol.getMetaItems?.({
111+
type: 'app',
112+
// @ts-expect-error [#13598] not a member of `GetMetaItemsRequest` —
113+
// the read has no `packageIds` plural.
114+
packageIds: ['crm'],
115+
}),
116+
// A misspelt VERB, which is what the untyped handle could never catch:
117+
// any property access on `any` is a property access on `any`.
118+
// @ts-expect-error [#13598] `rollbackToPackageCommit` has three `m`s in
119+
// neither of the two places this one puts them.
120+
protocol.rollbackToPackageCommmit?.({ commitId: 'c1' }),
121+
// ⛔ Every member is OPTIONAL and STAYS optional: a filled slot is not a
122+
// promise that the verb is there. This directive is what would go
123+
// unused if someone "simplified" the handle to a non-partial
124+
// `MetadataProtocol` — which is exactly the change that deletes the
125+
// reason the runtime probes in section 2 exist.
126+
// @ts-expect-error [#13598] possibly `undefined` — call it behind the probe.
127+
protocol.getMetaItems({ type: 'app' }),
128+
];
129+
}
130+
131+
// ---------------------------------------------------------------------------
132+
// Section 2 — runtime control: the capability probes SURVIVE the typing
133+
// ---------------------------------------------------------------------------
134+
135+
/** `/packages` state changes demand `manage_metadata` (#7033 / #7023). */
136+
const PKG_ADMIN = () => ({
137+
request: {},
138+
executionContext: {
139+
userId: 'u_pkg_admin',
140+
systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'],
141+
},
142+
}) as any;
143+
144+
/**
145+
* A host that OCCUPIES the `protocol` slot with an object carrying none of the
146+
* verbs — the documented reason every call site probes rather than calls. Not
147+
* an empty slot: an empty slot would take the `!protocol` arm of each guard and
148+
* prove nothing about the `typeof … === 'function'` half.
149+
*/
150+
function partialProtocolDoor() {
151+
const kernel: any = {
152+
getService: (name: string) => {
153+
if (name === 'protocol') return Promise.resolve({ someUnrelatedVerb: () => undefined });
154+
if (name === 'objectql') {
155+
return Promise.resolve({
156+
registry: { getAllPackages: () => [], getPackage: () => undefined },
157+
});
158+
}
159+
return null;
160+
},
161+
context: { getService: () => null },
162+
};
163+
return new HttpDispatcher(kernel);
164+
}
165+
166+
describe('#13598 · 1 · the compile-time pins are type-level only', () => {
167+
it('neither pin function is invoked — tsc is the assertion', () => {
168+
expect(typeof declaredKeysCompile).toBe('function');
169+
expect(typeof undeclaredKeysAreCompileErrors).toBe('function');
170+
});
171+
});
172+
173+
describe('#13598 · 2 · a PARTIAL protocol host is still answered, never crashed', () => {
174+
const cases: Array<[string, string, string, string]> = [
175+
['publish-drafts', '/crm/publish-drafts', 'POST', 'Draft publishing not supported'],
176+
['discard-drafts', '/crm/discard-drafts', 'POST', 'Draft discarding not supported'],
177+
['commits', '/crm/commits', 'GET', 'Commit history not supported'],
178+
['commit revert', '/crm/commits/c1/revert', 'POST', 'Commit revert not supported'],
179+
['rollback', '/crm/rollback', 'POST', 'Commit rollback not supported'],
180+
['adopt-orphans', '/crm/adopt-orphans', 'POST', 'Orphan adoption not supported'],
181+
['duplicate', '/crm/duplicate', 'POST', 'Package duplication not supported'],
182+
];
183+
184+
for (const [label, path, method, message] of cases) {
185+
it(`${label} answers 501 from the capability probe`, async () => {
186+
const result = await partialProtocolDoor().handlePackages(
187+
path, method, { commitId: 'c1', targetPackageId: 'crm_copy' }, {}, PKG_ADMIN(),
188+
);
189+
expect(result.response?.status).toBe(501);
190+
expect(JSON.stringify(result.response?.body)).toContain(message);
191+
});
192+
}
193+
});

0 commit comments

Comments
 (0)