Skip to content

Commit fb2bccf

Browse files
os-billclaude
andauthored
fix(spec): close the shared rate-limit budget — one declaration was answering two doors, and one of them dropped the key in silence (#18861)
Fixes #18578 Clause-②: yes (widening) `ServerRateLimitConfigSchema` was declared `strictObject({ … guidance: { keyBy, store } }, RateLimitConfigSchema.shape)` — built from the OPEN schema's own shape object. One declaration therefore answered for **two** emitted defs with opposite doors: | def | door before | an authored `keyBy` | |---|---|---| | `system/ServerRateLimitConfig` | closed | refused, loudly, with the prescription | | `shared/RateLimitConfig` | plain open `z.object` | accepted, then dropped in silence | The two `guidance` entries prescribed to nobody on the open twin. And a misspelled budget was the same story one key over: on the bare mount `windowSeconds: 60` parsed green and metered the 60000 ms default — a thousandfold miss on the one key whose job is to bound spend, reported as success. ## The census — the method, re-run on current `main` ⛔ Not re-derived by grep: the card records why a `CONTRACT_REVIEW_TIER` sweep with live controls found nothing here (it hunted an OPEN clone built from a STRICT schema's shape; this is the STRICT one built from the OPEN one's shape, which every grep shape in that sweep is blind to by construction). The instrument is the gate's own — `computeGuidanceRoutes()` in `packages/spec/scripts/build-schemas.ts`: match every emitted def to its declaration by sorted key set plus per-entry instance identity, then write the key at the def and read what comes back. Driven over every emitted def at `42f8df1723` (current `main` when this branch was cut), and again on this branch: | reading | before | after | |---|---|---| | emitted defs | 1527 | 1527 | | `strictObject` declarations registered | 551 | 551 | | defs resolving to exactly one declaration | 258 | 258 | | … of those, naming at least one undeclared key | 147 | 147 | | keys promised | 779 | 779 | | keys **delivered** | 770 | **772** | | keys **not delivered** | 9 | **7** | The filer's figures reproduce exactly (258 / 779 / 770 / 9); the def total moved 1525 → 1527 with the tree. One refinement worth recording: **258 is the count of defs resolving to exactly one declaration**, and the subset whose declaration also names an undeclared key is **147** — the card's sentence folds the two. The 9 not-delivered, itemised: - **2** — `shared/RateLimitConfig:keyBy` and `:store`, the live members this card is about. Probe verdict `accepted-and-stripped`. - **7** — `ui/ChartGroupBy:function`, `ui/ChartGroupBy:groupBy`, `ui/RecordHighlightsField:icon`, `ui/ViewItem:confg`, `ui/ViewItem:isPinned`, `ui/ViewItem:sortOrder`, `ui/ViewItem:columnState`. **An acknowledged probe boundary, ⛔ not a clean zero and ⛔ not a finding** — and the mechanism is now measured rather than assumed. Each is a discriminated union; the probe writes `{ [key]: null }` and nothing else, so the DISCRIMINATOR is missing and the union answers `invalid_union` on `viewKind` before any arm's door is reached. Written whole, the same document DOES raise the prescription. So they are refused loudly today, just not through a door this instrument can watch. ## The shape chosen, and why The strictness and both tables move to the **shared** schema; `ServerRateLimitConfigSchema` keeps only what is genuinely server-only — its two bounds checks — and is now `RateLimitConfigSchema.superRefine(…)` rather than a second `strictObject` over the same shape object. That leaves **one declaration and one door for both defs**, which is load-bearing in three ways: 1. the declaration match still resolves to exactly **one** declaration, so `matched.length !== 1` never fires and proof 4 keeps working for both defs. Declaring a second `strictObject` over the same shape object would have restored the ambiguity in the other direction, where the match resolves to neither and both defs silently read "no evidence"; 2. the closed twin's verdict is untouched — same acceptances, same refusals, same prescription bullet, same bounds; 3. the `shared/` ledger row's own rationale — *strictness decided at the consuming schema* — is false for this shape, exactly as it was false for `shared/protection.zod.ts`. Of the two mounts only one re-postured; `api/endpoint.zod.ts` mounts it bare on `apis[].rateLimit`, a registered metadata type authored through `defineStack({ apis })`, the Studio form and `PUT /meta/api/:name`, where nothing re-postures it. The row is annotated for what is now the fourth instance of a shape that ledger has recorded three times. ## Controls **LIT — behaviour flips.** Same probe, run against this branch and against the two source files restored to the base commit (ablation with an `EXIT`/`INT`/`TERM` restore trap; both ablated blobs verified by `git hash-object` against the base blob hashes, and the restore verified by an empty `git diff HEAD`): | written on `shared/RateLimitConfig` | before | after | |---|---|---| | `{ …valid, keyBy: 'ip' }` | PARSED OK → `{enabled, windowMs, maxRequests}` — key gone | REFUSED, `Unrecognized key(s) on this rate-limit budget …: keyBy.` + the `keyBy` prescription | | `{ …valid, store: 'redis' }` | PARSED OK, key gone | REFUSED + the `store` prescription | | `{ enabled: true, windowSeconds: 60, maxRequests: 100 }` | PARSED OK → `windowMs: 60000` | REFUSED, `Did you mean windowSeconds → windowMs?` | | an `api` endpoint whose `rateLimit` carries `keyBy` | PARSED OK, key gone | REFUSED with the same prescription, at the author's own path | Census leg of the same flip: 770 → 772 delivered, 9 → 7 not delivered. **DARK — reads what it must.** `system/ServerRateLimitConfig`, before and after: a legitimate budget parses to the same document; `{ enabled: true }` still materialises the same defaults; `max: 5` is still renamed to `maxRequests`; `maxRequests: 0` and `windowMs: 0` are still refused on `path: ['maxRequests']` / `['windowMs']` with their own messages; `{ …valid, keyBy: 'ip' }` is still refused carrying the prescription bullet. A legitimate endpoint document with a legitimate `rateLimit` still parses, before and after. The census's 258 / 779 are unchanged, so the closed twin's declaration resolution did not move either. ⚠️ **The two defs are told apart, as the card requires.** The probe reads them by def key through separate schema instances, and the discriminating reading is the PARSE ANSWER, not the emitted artefact: both still emit `additionalProperties: false`, and both still match one declaration by per-entry instance identity — neither of the two cheap instruments moved, and neither was used. ⚠️ **One disclosure inside DARK.** The server key's refusal MESSAGE changes, because the two surfaces now share one declaration: the surface prose reads `this rate-limit budget (server.security.rateLimit, or an endpoint's rateLimit)` rather than `server.security.rateLimit` alone, and the `history` sentence is the shared one. The verdict, the issue codes, the accept set and the prescription text are unchanged, and no test pinned the old prose. It is named here rather than left for a reviewer to find. ## The pin this rots, and where it went `scripts/build-schemas-check-mode.test.ts`'s `#18301` DOOR pin used this defect as a LIVE fixture, and its guard fired with its own prescription — *"its door closed, so this fixture no longer models an open def sharing a closed declaration's shape; re-pick the pair"*. Re-picked: - **DARK leg** — the rate-limit twins, now one declaration, two defs and one door. Both rows must be ADMITTED by proof 4, so re-opening the shared shape turns this test red. That is this card's regression guard. - **LIT leg** — `ui/ViewItem:confg`, a def whose declaration NAMES the key and whose delivery the probe cannot watch. Proof 4 must refuse the deletion, in the words that say a declaration exists, and must not be waived by proof 2 either. Both halves of the fixture's own validity are guarded loudly: the bare document must carry no `unrecognized_keys` issue at all, and the whole document must raise the prescription. The census says there is no remaining def that ACCEPTS a promised key and drops it, so no live pair could model the original shape — which is the point of the card. ## Verification - `pnpm --filter @objectstack/spec build` — green - `pnpm --filter @objectstack/spec check:generated` — 15 artefacts; only `strictness-ledger.counts.md` was stale (regenerated with `gen:strictness-ledger`, `system/` 351 → 350). ⭐ `check:authorable-surface`, `check:api-surface`, `check:docs`, `check:declaration-map` and `check:export-origins` all pass with **no regeneration**: the published JSON Schema and the API surface are byte-unchanged, because in `io: 'output'` zod already emitted `additionalProperties: false` for the stripping shape. - `pnpm --filter @objectstack/spec typecheck` — green - `pnpm --filter @objectstack/spec test` — 486 files / 14053 tests pass, 1 skipped - `pnpm --filter @objectstack/spec test:repo` — 31 files / 536 tests pass (the re-picked `#18301` pin is in here) - `check:strictness-ledger` · `check:yaml-examples` · `check:cross-package-test-inputs` · `check:test-source-alias` · `check:spec-parsed-alias` · `check:doc-authoring` · `check:spec-docblock-symbol-anchors` · `check:pm-widening-tells` · `check:nul-bytes` — green - `check-adr-0087-registration --base origin/main` — green; 1 declared-breaking changeset carrying `not-required (no-migration-prescription)` - `check-changeset-no-major --base origin/main` · `check-empty-changeset --base origin/main` — green - `check:skill-examples` — NOT MEASURED: it refuses before judging any surface because `packages/client-react/dist` holds no declarations in this worktree. A prerequisite, not a verdict. Blast radius, measured: every shipped `rateLimit` block writes only declared keys — three in `content/docs/`, one in `skills/objectstack-api`, none at all in `examples/`, the `os init` templates or the `create-objectstack` blank template. ## Acceptance notes - **The ADR-0087 boundary was measured, and it does not fire.** The dispatch fenced `packages/spec/src/migrations/registry.ts` (held by another seat). ⛔ That file is untouched, and the technical choice was not bent to avoid it: the disposition is `not-required (no-migration-prescription)` on its own merits, and it is the same disposition, on the same stored metadata type, that the close of `ApiEndpointSchema` itself took one level up — an undeclared key was never honoured, so nothing exists for `objectstack migrate meta` to rewrite, and the refused set is an open set of author typos rather than a renamed key. - **The declaration line above is copied verbatim from the dispatch, and the measurement disagrees with it.** No key is added to a published payload here; what moves is the accept set, and it moves DOWN. On the arms this repo uses, the measured reading is `no (narrowing)`. The line is left exactly as dispatched, per the charter that the declaration is the seat's to align; the correction is named in the report rather than made here. - Noted, not filed: the retired outbound connector vocabulary (`strategy`, `burstCapacity`, `respectUpstreamLimits`, `rateLimitHeaders`) is refused by the closed budget with no wrong-layer pointer — the rejection is correct and loud, and a `guidance` entry naming where outbound throttling belongs would be an improvement rather than a defect repair. Carrier: whoever next touches `shared/http.zod.ts`. - Noted, not filed: the `keyBy` prescription points at `server.trustProxy` for how the caller IP is read, which is accurate on both mounts but is written in server language; it now reaches endpoint authors too. Carrier: none — no PR or person is queued on this file. - Noted, not filed: the dispatch's file surface named `packages/spec/src/system/http-server.zod.ts`, and `ServerRateLimitConfigSchema` actually lives in `packages/spec/src/system/stack-server.zod.ts` (`http-server.zod.ts`'s shape was retired). The premise otherwise verified exactly. --- _Generated by [Claude Code](https://claude.ai/code/session_01JbZnqu8bt6YqfJsr9vaFb3)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent a43b9d0 commit fb2bccf

7 files changed

Lines changed: 297 additions & 125 deletions

File tree

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
---
2+
"@objectstack/spec": minor
3+
---
4+
5+
feat(spec): refuse unknown keys inside a rate-limit budget — `RateLimitConfigSchema` goes strict, so one declaration stops answering for two doors
6+
7+
**BREAKING** accept-set narrowing on a published spec schema, landing after the
8+
v17.0.0 cut (the lockstep launch-window convention ships it as `minor`).
9+
10+
Clause-②: no (narrowing)
11+
12+
<!-- adr-0087: not-required (no-migration-prescription) this change retires NO key. The budget vocabulary is byte-identical and only the unknown-key POSTURE moves, from strip to reject, on one of its two mounts. Nothing exists for `objectstack migrate meta` to rewrite, because an undeclared key was never honoured: it was dropped at parse, so neither the inbound token bucket (`@objectstack/runtime` `security/inbound-rate-limit.ts`), nor the endpoint policy chain, nor the publish gate ever read one — measured with the gate's own instrument, which reports this def as accepting the key and returning a document without it. There is no single FROM/TO rule a ledger entry could state either, since what is now refused is an open set of author typos rather than a renamed key. The upgrade channel is the schema rejection itself, which is strictly more specific than any ledger line: it names the offending key at the author's own path and carries either the canonical spelling or the wrong-layer pointer. This is the same disposition, on the same stored metadata type, that #5384 took one level up when it closed `ApiEndpointSchema` itself; the `declarative-apis-endpoints-live` entry that governs this surface is already registered for protocol 17 and needs no change here. -->
13+
14+
`ServerRateLimitConfigSchema` was declared
15+
`strictObject({ … guidance: { keyBy, store } }, RateLimitConfigSchema.shape)` —
16+
built from the OPEN schema's own shape object. One declaration therefore answered
17+
for TWO emitted defs with opposite doors: `system/ServerRateLimitConfig` refused
18+
an undeclared `keyBy` and handed back the prescription, while
19+
`shared/RateLimitConfig` — the same shape, mounted bare on `apis[].rateLimit` —
20+
accepted the key and dropped it in silence. Both guidance entries prescribed to
21+
nobody there. A misspelled budget was the same story one key over:
22+
`windowSeconds: 60` parsed green and metered the 60000 ms default, a
23+
thousandfold miss on the one key whose job is to bound spend, reported as
24+
success.
25+
26+
**What is refused:** any key the budget does not declare, wherever it is mounted,
27+
with a message naming the surface and the offending key. A near miss carries the
28+
declared spelling (`window` / `windowSeconds` are answered with `windowMs`;
29+
`max` / `maxRequest` / `limit` with `maxRequests`). `keyBy` and `store` keep
30+
their wrong-layer prescriptions — the limiter's key is the resolved principal
31+
falling back to the caller IP, and its counters live in the kernel `cache`
32+
service (ADR-0069 D2) — and those two now reach the author on both mounts
33+
instead of one.
34+
35+
**What stays accepted:** every declared key, byte-identically, with the same
36+
defaults. `server.security.rateLimit` keeps its two bounds checks
37+
(`maxRequests > 0`, `windowMs > 0`) and answers exactly as before. The published
38+
JSON Schema, the authorable surface and the API surface are all unchanged —
39+
`check:authorable-surface`, `check:api-surface` and `check:docs` pass with no
40+
regeneration, because in `io: 'output'` zod already emitted
41+
`additionalProperties: false` for the stripping shape too.
42+
43+
**Breaking for metadata that was already silently broken.** An `apis[].rateLimit`
44+
carrying an undeclared key now fails `objectstack validate`, `objectstack build`
45+
and the metadata write path instead of publishing with the key discarded.
46+
Measured blast radius before landing: every shipped `rateLimit` block writes
47+
only declared keys — three in `content/docs/`, one in `skills/objectstack-api`,
48+
and none at all in `examples/`, the `os init` templates or the
49+
`create-objectstack` blank template, which declare no budget.
50+

‎docs/audits/2026-07-unknown-key-strictness-ledger.counts.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,4 +264,4 @@ directory rather than per file.
264264
| `marketplace/` | 29 |
265265
| `qa/` | 6 |
266266
| `shared/` | 20 |
267-
| `system/` | 351 |
267+
| `system/` | 350 |

‎docs/audits/2026-07-unknown-key-strictness-ledger.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1374,7 +1374,7 @@ rest at #5107.
13741374
| `ai/` | mixed | agent/tool/skill definitions authored (partially strict already); model/provider payloads wire |
13751375
| `integration/` | wire | connector payloads — upstream adds fields freely |
13761376
| `identity/` | mixed | position/user shapes authored (`PositionSchema` **strict as of #4001 step 2**, with the ADR-0010 envelope declared); auth payloads wire. **34 → 33 in #4641**: `identity.zod.ts` lost its `SessionSchema` site — a second, importerless declaration of a name `api/auth.zod.ts` already owned (the #4411 dual-source trap), deleted rather than reclassified |
1377-
| `shared/` | **mixed · `protection.zod.ts` authorable, the rest as written** | ⚠️ **Annotated at #16845 — the flat `n/a` verdict this row carried was right for the directory and wrong for one file in it, the `api/` and `kernel/` rows' finding a third time.** ⛔ **This directory is still UNTRIAGED and this is not a per-file row** — `shared/` has never had per-file rows, so `shared/protection.zod.ts` was **never enumerated** by this campaign; it was not deliberately accepted, because no verdict was ever taken on it. Two measurements say why the silence carried no information. **①** This row's own rationale — *strictness decided at the consuming schema* — is false for this file: all **16** `protection: ProtectionSchema` mounts across **14** files are the bare `ProtectionSchema.optional()`, not one `.extend()` / `.merge()` / `.omit()` / re-posture among them, so both the strictness and the MESSAGE are decided here in `shared/` and inherited verbatim by objects, views, dashboards, datasets, reports, apps, flows, webhooks, permissions, positions, email templates, agents, tools and skills — very nearly every authorable metadata type in the platform. A row is read as licence, which is what this ledger's own gate exists to prevent. **②** Even a per-file row would not have surfaced the defect, and this is the part worth keeping: `ProtectionSchema` has been `.strict()` since it was introduced, so it counts in the `strict` column and could never carry a remaining-strip row. **This ratchet's axis is CLOSURE; the defect was MESSAGE QUALITY** — a closed shape with no error map, refusing `lockk` with zod's bare `Unrecognized key: "lockk"`: no surface, no declared-key list, no rename, on every one of those mounts. `strictObject` at #16845 (the `PluginPermissionsSchema` conversion at #16328 one directory over is the precedent). ⛔ The population of *closed shapes still carrying zod's bare message* is **NOT MEASURED** here and is not this row's claim — #14722's rule stands that a sweep needs its own card with its own measured count. The rest of `shared/` is unchanged: utilities and building blocks whose strictness really is decided at the consuming schema |
1377+
| `shared/` | **mixed · `protection.zod.ts` and `http.zod.ts`'s rate-limit budget authorable, the rest as written** | ⚠️ **Annotated again at #18578 — a FOURTH instance of the same shape, and this one was invisible to a sweep that went looking for it.** `shared/http.zod.ts`'s `RateLimitConfigSchema` is a building block with two mounts, and this row's rationale — *strictness decided at the consuming schema* — held for exactly one of them: `system/stack-server.zod.ts` re-postured it closed, and it did so by building `strictObject(…, RateLimitConfigSchema.shape)` **from this shape object**, so ONE declaration answered for TWO emitted defs with OPPOSITE doors. `api/endpoint.zod.ts` mounts the same schema BARE on `apis[].rateLimit` — a registered metadata type since #5312, authored through `defineStack({ apis })`, the Studio form and `PUT /meta/api/:name` — where nothing re-postures it, so the two `guidance` entries (`keyBy`, `store`) prescribed to nobody and a misspelled budget was worse: `windowSeconds: 60` parsed green and metered the 60000 ms default, a thousandfold miss reported as success. ⚠️ **The sweep that should have caught it could not.** A `CONTRACT_REVIEW_TIER` review swept this exact class with live controls (5 `.strip()` sites, 0 `z.object(XSchema.shape…)`, 0 `z.object(bareIdentifier)`) and found nothing, because the sharing runs the other way round here: it hunted an OPEN clone built from a STRICT schema's shape, and this is the STRICT one built from the OPEN one's shape — every grep shape in that sweep is blind to that direction by construction. It was found instead by driving the gate's own instrument (`computeGuidanceRoutes()` in `scripts/build-schemas.ts`) over every emitted def: match each def to its declaration, then ask the def what it actually does with the key. Census on `42f8df1723`: 1527 emitted defs, 258 resolving to exactly one declaration, 779 keys promised, **770 delivered, 9 not** — 2 of them this live silent strip, the other 7 union defs whose discriminator the probe's one-key document cannot supply (they DO deliver to an author who writes a whole document). The close moves the strictness and both tables onto the shared schema, leaving ONE declaration and ONE door for both defs: 772 delivered, 7 not. ⚠️ The residual 7 are a PROBE boundary, ⛔ not a clean zero and ⛔ not a finding. ⚠️ **Annotated at #16845 — the flat `n/a` verdict this row carried was right for the directory and wrong for one file in it, the `api/` and `kernel/` rows' finding a third time.** — the flat `n/a` verdict this row carried was right for the directory and wrong for one file in it, the `api/` and `kernel/` rows' finding a third time.** ⛔ **This directory is still UNTRIAGED and this is not a per-file row** — `shared/` has never had per-file rows, so `shared/protection.zod.ts` was **never enumerated** by this campaign; it was not deliberately accepted, because no verdict was ever taken on it. Two measurements say why the silence carried no information. **①** This row's own rationale — *strictness decided at the consuming schema* — is false for this file: all **16** `protection: ProtectionSchema` mounts across **14** files are the bare `ProtectionSchema.optional()`, not one `.extend()` / `.merge()` / `.omit()` / re-posture among them, so both the strictness and the MESSAGE are decided here in `shared/` and inherited verbatim by objects, views, dashboards, datasets, reports, apps, flows, webhooks, permissions, positions, email templates, agents, tools and skills — very nearly every authorable metadata type in the platform. A row is read as licence, which is what this ledger's own gate exists to prevent. **②** Even a per-file row would not have surfaced the defect, and this is the part worth keeping: `ProtectionSchema` has been `.strict()` since it was introduced, so it counts in the `strict` column and could never carry a remaining-strip row. **This ratchet's axis is CLOSURE; the defect was MESSAGE QUALITY** — a closed shape with no error map, refusing `lockk` with zod's bare `Unrecognized key: "lockk"`: no surface, no declared-key list, no rename, on every one of those mounts. `strictObject` at #16845 (the `PluginPermissionsSchema` conversion at #16328 one directory over is the precedent). ⛔ The population of *closed shapes still carrying zod's bare message* is **NOT MEASURED** here and is not this row's claim — #14722's rule stands that a sweep needs its own card with its own measured count. The rest of `shared/` is unchanged: utilities and building blocks whose strictness really is decided at the consuming schema |
13781378
| `qa/` | n/a | test fixtures |
13791379

13801380
## Next steps (verify-then-enforce, one shape at a time)

0 commit comments

Comments
 (0)