Skip to content

Commit 53cbad9

Browse files
os-litantclaude
andauthored
fix(rest): consume the parsed api sub-config so RestApiConfigSchema owns its defaults (#15673)
* fix(rest): consume the parsed `api` sub-config instead of discarding it `RestServer.normalizeConfig` ran `RestApiConfigSchema` over `config.api` and threw the parsed output away, rebuilding the block from a `??` chain over the raw cast. That chain restated the schema's eleven top-level `z.default(...)`s as eleven literals in `packages/rest`, with nothing pinning that the two stayed equal — a `packages/spec` default change would silently fail to propagate. #11637 made the parse validate-only for two measured reasons; both have since expired (#11983 gave `enableSearch` a declared seat, #12450 withdrew the `projectResolution` omit). Re-measured here: the 14 keys the method reads and the 14 the schema declares after `.omit({ requireAuth: true })` are the same 14 in both directions, so a consumed parse cannot strip anything the runtime honours. `requireAuth` stays omitted and stays warn-and-ignore in the plugin, which reads it off the RAW config. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * test(rest): pin that the `api` defaults follow `RestApiConfigSchema` Two halves, deliberately split by whether the schema is mocked. `rest-api-config-defaults-follow-spec.pin.test.ts` is the DISCRIMINATING pin: it moves five `z.default(...)`s to values that differ from both the shipped schema's and the deleted `??` chain's literals, then drives a real RestServer construction. Asserting today's values would have been vacuous — the chain's literals and the schema's defaults agreed key for key, which is the defect. `rest-config-parse-not-cast.test.ts` §D is the unmocked half: the shipped defaults are the schema's own output (derived, never restated), the normalized key set equals the declared key set, `requireAuth: false` still constructs and still warns through the plugin, and the one bounded behaviour delta — an authored `documentation` / `responseFormat` now carrying its declared inner defaults — is pinned rather than left to be rediscovered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * chore(changeset): rest api config defaults now come from the schema Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * docs(permissions): re-anchor the system-context citations after the line shift `content/docs/permissions/system-context.mdx` cites `packages/rest/src/rest-server.ts` by absolute line number, so this change's +44 net lines rotted ten anchors. Rewritten by the gate's own `node scripts/check-system-context-census.mjs --fix`, which rewrites a pure shift and refuses a population change. Verified structurally rather than by line content (several anchored lines are the same string): the census JSON re-derived at the merge base and at head is identical in population — 106 sites both sides, 0 arrived, 0 vanished, same 45 files and 20 packages, `classified` 462 = 462. The six `rest-server.ts` sites carry identical text at +12 / +44. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * docs(rest): retire the two comments that still called `api` validate-only Both were true until this branch consumed the parse, and both now contradict the code beside them: the seam's own sibling paragraph in `rest-server.ts` and the `api` aside in the #11984 pin file's header. Rewritten to say what is now true, keeping the history in past tense. Repo-wide grep for the stale claim returns only that one past-tense mention. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * docs(changeset): state the subtractive half of the `api` parse delta The changeset described the behaviour change on an authored `api.documentation` / `api.responseFormat` as additive only — the objects now carry their declared inner defaults. Measured against the built schema, the delta is also subtractive: `RestApiConfigSchema`'s nested objects are non-strict `z.object()`s, so inner keys they do not declare are stripped, at both depths (`documentation.logo`, `documentation.contact.phone`, `documentation.license.spdxId`, `responseFormat.extra`), where the deleted `??` chain passed the authored object through by reference and kept them. Documentation accuracy only — no code, test or pin is touched, and the `patch` level is unchanged: the normalized block is `private` to `RestServer` and neither key has a read site, so nothing public widens or narrows. 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 after merging main `content/docs/permissions/system-context.mdx` cites `rest-server.ts` by absolute line number, and both sides of this merge moved lines in that file: main by +32 and this branch by +44, in disjoint regions. Row 50's five REST anchors were the one hunk a text merge could not resolve. Re-anchored with the gate's own `--fix`, which rewrites a pure shift and refuses a population change. Verified structurally rather than by comparing line content (several anchored lines are the same string): the census re-derived with one instrument over main, this branch's pre-merge tip and the merged tree agrees on every population number — 106 sites, 20 packages, 45 files, 871 identifier appearances, 462 classified, 0 stale ledger rows — and site identity keyed shift-invariantly shows 0 arrived and 0 vanished in both directions. Only `file:line` identity moves, on exactly the six rest-server sites `--fix` rewrote. 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 after the second merge `origin/main` moved again while this branch was being verified, and its new tip moves `rest-server.ts` a second time — so row 50's five REST anchors conflicted for exactly the same reason as the first merge. Re-anchored with the gate's own `--fix`, a pure shift again. Structural verification repeated with one instrument over main, this branch's previous tip and the merged tree: 106 sites, 20 packages, 45 files, 871 identifier appearances, 462 classified, 0 stale ledger rows on all three, and shift-invariant site identity shows 0 arrived / 0 vanished in both directions. Only `file:line` identity moves. 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 after the main merge `node scripts/check-system-context-census.mjs --fix` rewrote the ten `packages/rest/src/rest-server.ts` anchors displaced by this branch's +44-line change to that file. Pure line rot: the census population is unchanged against `origin/main` — 107 sites, 873 identifier appearances, 463 classified, 0 arrived, 0 vanished — and the gate now reports 141 anchors resolving with 27 declared non-read. 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 after the second merge `node scripts/check-system-context-census.mjs --fix` rewrote the same ten `packages/rest/src/rest-server.ts` anchors, displaced again by this branch's +44-line change to that file. The instrument itself moved on main in this window, so every figure was re-derived with the merged tree's census rather than carried over: pure line rot, `--fix` did not refuse, and the population is unchanged against `origin/main` — 106 sites, 885 identifier appearances, 462 classified, 0 arrived, 0 vanished. The gate now reports 140 anchors resolving with 27 declared non-read. 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 after the third merge Main landed another `packages/rest/src/rest-server.ts` comment change while the previous sync was being verified, displacing the same ten anchors again. `node scripts/check-system-context-census.mjs --fix` rewrote them; it did not refuse, so this is a pure shift. Re-derived against `origin/main` abdceef with the merged tree's own census instrument: 106 sites, 885 identifier appearances, 462 classified, 19 packages, 44 files, 0 arrived, 0 vanished, and scannedFiles 293 as a non-zero control on both sides. Gate: 140 anchors resolve, 27 declared non-read. 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 after the fourth merge `--fix` rewrote 5 anchors on row 50 and REFUSED ZERO files — the signal that this is a pure line shift, not a population change. Verified structurally against `origin/main`'s own census rather than by reading the rewritten lines: same 105 elevation read sites, same 44 files, same 19 packages, same per-site identity text, `staleLedgerRows` empty on both sides, and the only delta a +12 / +44 shift inside `packages/rest/src/rest-server.ts` — this branch's two insertion points. Zero sites arrived, zero vanished. 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 e1d4f9e commit 53cbad9

6 files changed

Lines changed: 363 additions & 39 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 server's `api` configuration defaults now come from `RestApiConfigSchema` alone, instead of being restated in `packages/rest`.
6+
7+
`RestServer.normalizeConfig` already parsed `config.api` against `RestApiConfigSchema` — and then discarded the result, rebuilding the block from a `??` chain over the raw input. That chain restated the schema's eleven top-level `z.default(...)`s as eleven literals in a second package. They agreed key for key, and nothing measured that they would keep agreeing: changing a default in `@objectstack/spec` silently failed to propagate, because `api.enableUi ?? true` answers `true` for an absent key whatever the schema declares. Consuming the parse deletes the duplicate and makes the schema authoritative.
8+
9+
The parse itself is unchanged, so **nothing new is accepted or refused**: the same schema, with the same `.omit({ requireAuth: true })`, already ran at construction. `api.requireAuth` keeps its retired warn-and-ignore posture (`@objectstack/rest`'s plugin reads it off the raw config, so the warning is untouched), and every authored value still wins over the default.
10+
11+
One bounded behaviour change, for a caller who writes `api.documentation` or `api.responseFormat` — and it runs in two directions, not one. **Filled in:** those objects now arrive carrying their own declared inner defaults — `documentation.enabled` / `.title`, and `responseFormat.envelope` / `.includeMetadata` / `.includePagination`. **Stripped:** inner keys the schema does not declare no longer survive, at either depth — an authored `documentation.logo`, a `documentation.contact.phone` or a `documentation.license.spdxId` inside the nested objects, a `responseFormat.extra` — where the `??` chain passed the authored object through by reference and kept every key on it. Both halves are the same parse: `documentation` / `responseFormat` (and their `contact` / `license`) are non-strict `z.object()`s, which fill in their `.default()`s and drop what they do not name — dropped silently, so this is a strip and not a new refusal. An object left unwritten stays absent, and nothing in the platform reads either key today: the normalized block is `private` to `RestServer`, which reads only scalars off it (`apiPath` / `basePath` / `version` in `getApiBasePath`, the `enable*` flags, `projectResolution`), and the repo has no other read site for either key — so no consumer observes either half.

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:1553`, `:1582`), and neither
67+
cannot set it (`packages/rest/src/rest-server.ts:1565`, `:1594`), 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:1585` |
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` |
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:5101`, `:6527`, `:6775`, `:7206`, `:7399` |
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` |
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:1581` (#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:1553`, `:1582`; `domains/actions.ts:414` |
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` |
203203

204204
---
205205

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#14366] `RestApiConfigSchema` is the SINGLE SOURCE of the `api` sub-object's
5+
* defaults — `RestServer.normalizeConfig` follows a change to a
6+
* `z.default(...)` in `packages/spec` rather than restating it.
7+
*
8+
* ⛔ ANTI-VACUITY, and this file exists because the ordinary spelling of this
9+
* pin is vacuous. A test that asserts today's VALUES — `version === 'v1'`,
10+
* `enableProjectScoping === false` — passes just as well with the deleted `??`
11+
* chain still in place, because the chain's literals and the schema's defaults
12+
* agreed key for key on the day the chain was written. That agreement is the
13+
* whole defect: two sources that happen to match, with nothing measuring that
14+
* they keep matching. Asserting the matched value measures neither source.
15+
*
16+
* So this file MOVES the schema and asks where the server lands. The mock
17+
* below re-declares five `z.default(...)`s to values that differ from both the
18+
* real schema's and the deleted chain's literals, then drives a REAL
19+
* `RestServer` construction and reads the normalized config back:
20+
*
21+
* key real default deleted `??` literal mutated to
22+
* version 'v1' 'v1' 'v9-mutated'
23+
* basePath '/api' '/api' '/mutated'
24+
* enableUi true true false
25+
* enableProjectScoping false false true
26+
* projectResolution 'auto' 'auto' 'required'
27+
*
28+
* Pre-change tree: all five answer the `??` literal, because the chain read the
29+
* RAW input (`api.version ?? 'v1'`) and an absent key is nullish whatever the
30+
* schema says — the parsed output was discarded. Post-change: all five answer
31+
* the mutated default. That gap is what makes each case below a measurement of
32+
* the propagation rather than of a coincidence. Measured, both directions, in
33+
* this change's own reverse verification.
34+
*
35+
* ⚠️ This file mocks `@objectstack/spec/api` module-wide, so the schema it
36+
* drives is NOT the shipped one. The complementary pins that need the REAL
37+
* schema — that the shipped defaults are the schema's, that `requireAuth`
38+
* keeps its warn-and-ignore posture, and that the parse's inner defaults now
39+
* reach `documentation` / `responseFormat` — live in
40+
* `rest-config-parse-not-cast.test.ts` §D, which is deliberately unmocked.
41+
*/
42+
43+
import { describe, it, expect, vi } from 'vitest';
44+
45+
vi.mock('@objectstack/spec/api', async (importOriginal) => {
46+
const actual = await importOriginal<typeof import('@objectstack/spec/api')>();
47+
const { z } = await import('zod');
48+
return {
49+
...actual,
50+
// `.extend()` on the real schema, not a hand-built stand-in: every
51+
// other key — and the `requireAuth` tombstone the seam `.omit()`s —
52+
// must survive, or this would measure a shape change rather than a
53+
// default change. Only the five defaults move.
54+
RestApiConfigSchema: (actual.RestApiConfigSchema as any).extend({
55+
version: z.string().regex(/^[a-zA-Z0-9_\-\.]+$/).default('v9-mutated'),
56+
basePath: z.string().default('/mutated'),
57+
enableUi: z.boolean().default(false),
58+
enableProjectScoping: z.boolean().default(true),
59+
projectResolution: z.enum(['required', 'optional', 'auto']).default('required'),
60+
}),
61+
};
62+
});
63+
64+
const { RestServer } = await import('./rest-server.js');
65+
// The MOCKED schema, imported through the same specifier the seam uses, so the
66+
// control case below reads the very object the server was handed.
67+
const { RestApiConfigSchema } = await import('@objectstack/spec/api');
68+
69+
function makeServer() {
70+
return {
71+
get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(),
72+
use: vi.fn(), listen: vi.fn(), close: vi.fn(),
73+
} as any;
74+
}
75+
76+
function makeProtocol() {
77+
return {
78+
getMetaItems: vi.fn(async ({ type }: { type: string }) => ({ type, items: [] })),
79+
} as any;
80+
}
81+
82+
/** Construct the real server with `api` as given — the seam under test. */
83+
function construct(api: Record<string, unknown> = {}) {
84+
return new RestServer(makeServer(), makeProtocol(), { api } as any);
85+
}
86+
87+
/** The normalized `api` block, read off the constructed server. */
88+
function normalizedApi(rest: unknown): Record<string, unknown> {
89+
return (rest as { config: { api: Record<string, unknown> } }).config.api;
90+
}
91+
92+
describe('[#14366] normalizeConfig follows the SCHEMA default, not a local literal', () => {
93+
it('CONTROL: the mock really did move the schema', () => {
94+
// Not decoration. Every assertion below is "the server answers X"; if
95+
// the mock silently failed to apply, the real default would be `'v1'`
96+
// and a `not.toBe('v1')` style pin could pass for the wrong reason.
97+
// This proves the premise the rest of the file rests on.
98+
const parsed = (RestApiConfigSchema as any).omit({ requireAuth: true }).parse({});
99+
expect(parsed.version, 'the mocked schema must carry the mutated default').toBe('v9-mutated');
100+
expect(parsed.basePath).toBe('/mutated');
101+
expect(parsed.enableUi).toBe(false);
102+
expect(parsed.enableProjectScoping).toBe(true);
103+
expect(parsed.projectResolution).toBe('required');
104+
});
105+
106+
it('a moved `version` default reaches the normalized config', () => {
107+
// Pre-change: `'v1'` — `api.version ?? 'v1'` never consulted the schema.
108+
expect(normalizedApi(construct()).version).toBe('v9-mutated');
109+
});
110+
111+
it('a moved `basePath` default reaches the normalized config', () => {
112+
expect(normalizedApi(construct()).basePath).toBe('/mutated');
113+
});
114+
115+
it('a moved BOOLEAN default reaches it too — the `??` chain could not express this', () => {
116+
// The sharpest of the five. `api.enableUi ?? true` yields `true` for an
117+
// absent key no matter what the schema declares, so a spec change from
118+
// `default(true)` to `default(false)` was UNREPRESENTABLE downstream:
119+
// silently dropped, with every test still green. This is the drift the
120+
// card was filed about, stated as an executable case.
121+
expect(normalizedApi(construct()).enableUi).toBe(false);
122+
expect(normalizedApi(construct()).enableProjectScoping).toBe(true);
123+
});
124+
125+
it('a moved ENUM default reaches it', () => {
126+
expect(normalizedApi(construct()).projectResolution).toBe('required');
127+
});
128+
129+
it('the moved defaults reach the MOUNT, not just the config object', () => {
130+
// Read through the behaviour, not only the structure: a default that
131+
// landed in the normalized config but was not threaded would still be
132+
// a half-fix. `getApiBasePath()` composes `${basePath}/${version}`.
133+
expect(construct().getApiBasePath()).toBe('/mutated/v9-mutated');
134+
});
135+
136+
it('an AUTHORED value still wins over the schema default — the change is defaults only', () => {
137+
// The bound. Consuming the parse must not start overriding what the
138+
// caller wrote; zod `.default()` applies to `undefined` alone.
139+
const rest = construct({ version: 'v3', basePath: '/authored', enableUi: true });
140+
expect(normalizedApi(rest).version).toBe('v3');
141+
expect(normalizedApi(rest).basePath).toBe('/authored');
142+
expect(normalizedApi(rest).enableUi).toBe(true);
143+
expect(rest.getApiBasePath()).toBe('/authored/v3');
144+
});
145+
146+
it('an authored FALSE still survives — `??` and the parse agree here, and must keep agreeing', () => {
147+
// `false` is not nullish, so the deleted chain honoured it too. Kept as
148+
// a regression guard: the failure this pin guards against is a future
149+
// author "simplifying" the parse into a truthiness check.
150+
const rest = construct({ enableProjectScoping: false });
151+
expect(normalizedApi(rest).enableProjectScoping).toBe(false);
152+
});
153+
});

0 commit comments

Comments
 (0)