Skip to content

Commit 488f4f5

Browse files
Samclaude
andauthored
fix(spec): refuse a top-level __proto__ on the assignment node config (the catchall site) (#19419)
Fixes #19151 Clause-②: yes (narrowing) ⚠️ **This diverges from the claim comment, deliberately and on the record.** Claim 5751323556 declares `Clause-②: no`; the criterion as I read it says `yes (narrowing)`. What lands here is a **refusal newly added to a published parse surface** — the same act, on the same family, that the sibling PR #19147 declared `Clause-②: yes (narrowing)` in `.changeset/17852-record-proto-key-preparse-guard.md`. Grading a sibling site differently from its family is how a family stops being one, and of the two possible errors, declaring `no` on a real narrowing is the one that lets a contract narrowing land without contract review. The seat owns the correction if it reads the criterion the other way; I write this line once, here, and nowhere else. --- ## STEP ONE — the vendor line, re-read first-hand. It holds. The card's premise arrived half second-hand, so this was the gate before any edit. **Which zod, and how it was resolved.** `packages/spec/package.json` declares `"zod": "^4.4.3"`. Resolved from the package's own entry rather than from the manifest text: ``` node -e "const {createRequire}=require('module'); const r=createRequire('.../packages/spec/src/index.ts'); const p=r.resolve('zod/package.json'); console.log(p, require(p).version);" => /home/user/objectstack-issue-19151/node_modules/.pnpm/zod@4.4.3/node_modules/zod/package.json 4.4.3 ``` `pnpm --filter @objectstack/spec why zod` reports **`Found 1 version of zod`** — 4.4.3 — so the resolution is not one of two. (The lockfile does carry a second, 4.6.1, reached only through the better-auth family; `packages/spec` never sees it.) **The line, at the cited coordinates.** `node_modules/.pnpm/zod@4.4.3/node_modules/zod/v4/core/schemas.js`, `handleCatchall` opens at 759 and the skip is at **767-769**, exactly as reported: ```js function handleCatchall(proms, input, payload, ctx, def, inst) { // 759 ... for (const key in input) { // skip __proto__ so it can't replace the result prototype via the // 767 // assignment setter on the plain {} we build into // 768 if (key === "__proto__") // 769 continue; // 770 if (keySet.has(key)) continue; ... const r = _catchall.run({ value: input[key], issues: [] }, ctx); ``` `grep -n '__proto__' v4/core/schemas.js` returns exactly two sites in the file: **767-769** here, and **1496** in `$ZodRecord`'s open-key branch — the one #17852 measured and PR #19147 guarded. One function apart, same shape, and the `continue` sits above the schema that would judge the key in both. ⇒ **`premise_still_valid: true`.** The card's quotation was not accepted as evidence; it was reproduced. ## The defect, reproduced end to end on this tree At `origin/main` `0870fb5418`, through the real exported schema: ``` input (JSON.parse) own enumerable keys : [ 'total', '__proto__', 'other' ] AssignmentConfigSchema.safeParse => success: true parsed own keys : [ 'total', 'other' ] ``` with two lit controls in the same run: the identical config **without** `__proto__` round-trips both keys (so the instrument can see keys at all), and the same `__proto__` **inside** `assignments` is already refused loudly by #19147's record guard (so the instrument can see a refusal). `JSON.parse` is what makes `__proto__` an own enumerable key; an object literal's `{ __proto__: … }` sets the prototype and never reaches either loop. This lands on data an author wrote on purpose: the schema's own docblock says its top-level keys may be flow variables, and the descriptor declares `additionalProperties: true`. ## The fix `refuseCatchallProtoKey` in `packages/spec/src/shared/record-proto-key-guard.ts` — a **sibling** of `refuseRecordProtoKey`, both now calling one private `refuseProtoOwnKey`. Identical mechanism, identical refused name, identical issue shape (`custom`, `path: ['__proto__']`). `refuseRecordProtoKey`'s message bytes and behaviour are unchanged. Why a second wrapper rather than a second call site of the first: the refusal **names the parser that would otherwise drop the key**, and here that is `.catchall()`, not `z.record()`. An author told their top-level flow variable was dropped by "z.record()" would go looking at the `assignments` map — a different slot, one level down, with a different guard. A pin asserts the two messages name their own parser and not the other's. ## Why a pre-parse guard — the two alternatives, eliminated by measurement 1. **A key/catchall schema cannot see it.** The `continue` at 769 is above `_catchall.run`, so no catchall — not even `z.never()`, whose `unrecognized_keys` list is built inside the loop the `continue` already left — ever receives the key. Same structural unreachability the record guard's docblock records. 2. **Declaring `__proto__` in the object's own shape refuses every config.** Measured: zod reads a declared key as `input["__proto__"]` and tests presence as `"__proto__" in input`; on an ordinary object both answer through the **inherited accessor**, so the value is `Object.prototype` and the key is always "present". A plain config with no `__proto__` authored came back `success: false` with an `invalid_type` at `['__proto__']`. (It is also unwritable as an object literal at all — `{ __proto__: schema }` sets the shape object's prototype rather than adding a key, measured: the shape had one key, `assignments`.) That leaves the raw input, ahead of the parse. ## Why `__proto__` only — re-derived, not copied The record guard refuses `__proto__` alone on the ground that `constructor` and `prototype` reach the key schema unskipped. That ground had to be re-established at this position, because it is a different loop. Measured at the catchall, top level: | authored top-level key | parse | key in the output | |---|---|---| | `constructor` | success | kept | | `prototype` | success | kept | | `toString` | success | kept | | `__proto__` | success | **dropped** | ⇒ the reasoning transfers exactly, and for the same reason it was true below: only `__proto__` is structurally unrepresentable. Everything else round-trips, so refusing it here would be a narrowing no ruling ordered. The `assignments` slot keeps its own guard; the two are different parsers at different depths and neither covers the other. ## The pins, and their ablation The sharpest pin asserts **behaviour**, through one `classify()` helper that discriminates the three outcomes an authored key can meet — `refused` / `silently-dropped` / `silently-kept`. A bare `success === false` would pass for a schema that refused every config; a bare key check would pass for one that kept the key and reported success. Both the defect and its over-correction are named, not assumed. Beside them: an unguarded-object CONTROL that must stay `silently-dropped` on this exact zod; a preservation row per reserved-looking name; the previously-accepted shapes (empty config, bare legacy config, the CEL envelope and its malformed counterpart); the `assignments` guard and the array-form prescription still firing at their own paths; and an invariance pin on the JSON projection. **Ablation** (`scripts/ablation-replace.mjs`, anchor declared and hit exactly once, mutation verified against the disk): ``` anchor x1 -> x0 ; blob 50724ef -> 21c448025a16 (mutation landed) result Tests 6 failed | 88 passed (94) restore blob after restore 50724ef == blob at HEAD 50724ef, `git diff HEAD` empty ``` Direction observed: **red**, as expected. The six that turn red are exactly the six `#19151` assertions. **The `#17852` / `#18847` record-guard pins stay green under the same mutation** — which is the pin that the two guards are independent, and that these six are not riding on the other one's work. The fix was committed before the ablation, so the restore leg points at a commit that really exists; the subject resolves through the package's own `src` (a same-package relative import), so no `dist` leg is involved and none is claimed. ## What else moved, and why `packages/spec/dropped-refinements.baseline.json` — the guard wraps the object in a `z.preprocess` pipe, so the `AssignmentValue` refinement the JSON projection already dropped sits one segment deeper: `assignments.out.valueType` becomes `out.assignments.out.valueType`. **Same single site, same gap, no new one.** The build gate caught it and printed the corrected entry verbatim; this is that entry. Nothing else regenerated: `pnpm --filter @objectstack/spec check:generated` reports **all 15 generated artifacts up to date**, and the JSON projection is byte-identical to the pre-change baseline — same `type`, same single `properties.assignments`, same `xExpression: 'value'` on the map value, same `additionalProperties`. The expression ledger still derives `assignments.*` through `getSchemalessNodeConfigJsonSchemas()`, because every spec walker resolves a preprocess pipe to its OUT side (`pipeAuthorableSide`). All four are pinned, not merely observed. ## Verification Every exit code captured before any pipe. | what | verdict | |---|---| | `pnpm --filter @objectstack/spec build && … check:generated` | exit 0 — all 15 artifacts current | | `pnpm --filter @objectstack/spec test` | exit 0 — **504 files / 14754 tests** | | `pnpm --filter @objectstack/spec typecheck` | exit 0 (test layer included) | | service-automation reconciliation suites (8 files: form↔Zod ledger, expression ledger, config parse/schemas/unknown-keys, assignment envelope ×2, logic nodes) | exit 0 — 117 tests | | `dispatch-gates --commands` then `--ran` | **81 derived, 81 run, 0 UNRUN** | | `pnpm lint` (repo-wide `eslint . --no-inline-config`) | exit 0 | Three of the 81 answered **exit 3 — PREREQUISITE NOT MET, which is not a red and not a pass**: `check-plugin-teardown-shape --self-test` (its positive control is pinned to a commit outside this shallow clone), `check:dual-build-cjs-loads` and `check:type-check-debt` (both read a whole-repo `dist/` this box did not build within the foreground cap). CI builds and runs all three. Two families were derived from a base four commits behind `origin/main` and are named rather than assumed: `check:merged-result` and `check:issue-citations` were wired into `lint.yml` after this branch's base. Both were run anyway — green, after the citation-spelling correction described below. Measured for the changeset's disposition: **zero** authored use of `__proto__` as a top-level key on an `assignment` node config, across this repo, `examples/` and the `objectui` sibling — against a **lit control of 100 authored `assignment` node declarations in 26 files here and 11 files there**. The census is a working-tree reading at `1418799698`, not a history question; this clone is shallow (boundary `ae8edd2c4f71d6f6fea5261e8284997f5546392f`) and no count here depends on history. ## Acceptance notes — noted, not filed 1. **`scripts/check-issue-citations.mjs` cannot resolve a same-repo citation written `objectstack#N`.** `buildBoard` builds its probe set as `rows.filter((r) => !r.qualifier)`, so every **qualified** citation is excluded from the probe — while `classifyCitation` treats `objectstack#N` as naming this repo and resolves it against that same board. The number is therefore never on the board and always reports `allocated-but-absent`. Two-leg measurement on this diff, same six citations, same run mode: with `objectstack#17852` → `board: probed (1 citations)`, `2 allocated-but-absent`, exit 2; with `#17852` → `board: probed (2 citations)`, `6 resolves`, exit 0. Independent control: `GET /repos//issues/17852` answers **HTTP 200** (state `closed`), so the number resolves and the gate's own transport would have found it had it asked. This diff's added citations use the bare spelling, which is this repo's documented form for its own issues and what the gate's failure text itself prescribes. The gate is not otherwise touched here. 2. **`treeifyError` / `error.format()` throw on any issue path containing `__proto__`.** Reproduced first-hand against #19147's landed record guard on zod 4.4.3: both throw `TypeError: Cannot read properties of undefined (reading 'push')` on the `['assignments','__proto__']` path, while the same call on an ordinary refusal path succeeds (lit control). This guard uses the same `path: ['__proto__']` shape as its landed sibling, deliberately — it adds no new exposure class, and changing the path shape for one of the two would create two dialects and pre-empt a decision that belongs to whoever takes that question. #19151's body already records this connection; it is not this change's subject and not its acceptance condition. 3. **Two open PRs also hold `packages/spec/dropped-refinements.baseline.json`** — #19373 and #19335. That file is deliberately **not** `merge=os-regen` (recomputing a shrink-only ratchet can widen it), so whichever lands second reads the conflict by hand. No open PR holds either source file this change edits; lit control on the same scan, 18:25:35Z: 11 open PRs touch `packages/spec/` at all and 1 touches `packages/spec/src/automation/`. Not filed here: the PM files what is worth filing, per ruling A-narrow's own instruction that a further site is its own card **when measured**. ## Scope One site. ⛔ No sweep over the 398 records, ⛔ no re-opening of #17852's ruling, and #19147's `assignments` guard is untouched — it is correct and it is not this change's. #18670, the JSON-Schema projection gap, is not addressed here and stays open. --- _Generated by [Claude Code](https://claude.ai/code/session_01HnRAeVTLJevtQ5iCPX6JSm)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f9e16d8 commit 488f4f5

6 files changed

Lines changed: 386 additions & 53 deletions
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
'@objectstack/spec': minor
3+
---
4+
5+
**BREAKING** for authored metadata — an `assignment` flow node's config refuses a **top-level** key named `__proto__`, with a named, located error at parse time, instead of accepting the document and silently returning one without that key (objectstack#19151).
6+
7+
## Why
8+
9+
`AssignmentConfigSchema` is deliberately open at the top level: an `assignment` node is exempt from `registerFlow()`'s undeclared-key walk by design, because its top-level keys may themselves be flow variables (the bare legacy `{ <variable>: <value> }` config), and the descriptor declares `additionalProperties: true`. That openness is spelled `.catchall(z.unknown())`.
10+
11+
zod has two open-key branches and both skip a `__proto__` own key before anything author-facing can judge it. `z.record()`'s branch skips it above the key schema — that is objectstack#17852, fixed for the `assignments` map one level down. `handleCatchall` skips it above the **catchall** schema, one function over in the same file. So a flow variable named `__proto__` declared at the top level of an `assignment` node config parsed as SUCCESS and came back missing: the contract accepted a document and handed back a different one, on a surface whose own keys are author-named by design.
12+
13+
`JSON.parse` is what produces `__proto__` as an own key, so stored flow metadata reaches this door routinely; an object literal's `{ __proto__: … }` sets the prototype instead and never reaches either loop.
14+
15+
## What is refused, and what is not
16+
17+
`__proto__` only, in this position as in the sibling one. `constructor`, `prototype` and every other reserved-looking name reach the catchall unskipped and round-trip intact — measured — so they remain legal top-level flow-variable names and nothing narrows for them. The refusal is a `z.preprocess` guard on the raw input (`refuseCatchallProtoKey`, a sibling of `refuseRecordProtoKey` sharing one mechanism), because that is the only place the key is still visible: declaring it in the object's own shape was measured to refuse *every* config, since zod reads a declared key through `input["__proto__"]` and `"__proto__" in input`, which on an ordinary object both answer through the inherited accessor.
18+
19+
The `assignments` map keeps its own guard. The two are different parsers at different depths and neither covers the other.
20+
21+
Measured: zero authored use of `__proto__` as a top-level key on an `assignment` node config, across this repo, `examples/` and `objectui` — against a lit control of 100 authored `assignment` node declarations in 26 files here and 11 files there.
22+
23+
## Known gap, left open on purpose
24+
25+
Like the sibling guard, this runs at parse time only and does not project into the published JSON Schema (`packages/spec/json-schema/**`) — the general gap tracked as objectstack#18670, which stays open after this change.
26+
27+
Clause-②: yes (narrowing)
28+
29+
<!-- adr-0087: not-required (no-migration-prescription) zero authored use of `__proto__` as a top-level key on an `assignment` node config across this repo, examples/ and objectui — nobody has anything to rewrite, so there is no prescription to give. -->

‎packages/spec/dropped-refinements.baseline.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@
383383
},
384384
"automation/AssignmentConfig": {
385385
"sites": [
386-
"assignments.out.valueType"
386+
"out.assignments.out.valueType"
387387
]
388388
},
389389
"automation/AssignmentValue": {

‎packages/spec/src/automation/builtin-node-config.test.ts‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -665,3 +665,129 @@ describe('AssignmentConfigSchema.assignments — __proto__ pre-parse guard, cons
665665
expect(AssignmentConfigSchema.safeParse({ assignments: { total: '{amount}' } }).success).toBe(true);
666666
});
667667
});
668+
669+
/**
670+
* `AssignmentConfigSchema`'s OWN `.catchall()` — the third `__proto__` site
671+
* of the #17852 family and the one maintainer ruling A-narrow (comment
672+
* 5725370319) said would be «its own card when measured» (#19151).
673+
*
674+
* Different parser, different depth from the `assignments` guard above:
675+
* `z.record()`'s open-key branch skips `__proto__` above the KEY schema,
676+
* `handleCatchall` skips it above the CATCHALL schema (zod 4.4.3,
677+
* `v4/core/schemas.js` — the two `continue`s are one function apart). Neither
678+
* guard covers the other, and this object's top-level keys are an authoring
679+
* surface for flow-variable names BY DESIGN (the schema's own docblock:
680+
* «its top-level keys may be variables»), so the drop landed on data an
681+
* author wrote on purpose.
682+
*
683+
* The pins below assert BEHAVIOUR, not field presence: an authored top-level
684+
* `__proto__` must be refused LOUDLY — told apart here from the two silent
685+
* outcomes it could otherwise have (silently dropped, silently kept).
686+
*/
687+
describe('AssignmentConfigSchema — top-level __proto__ refused at the catchall (#19151)', () => {
688+
/**
689+
* The three outcomes an authored key can meet, discriminated by one
690+
* function so a test cannot accidentally assert the wrong one. A bare
691+
* `success === false` would pass for a schema that refused EVERY config,
692+
* and a bare key check would pass for one that kept the key and reported
693+
* success — the defect and its over-correction both.
694+
*/
695+
function classify(config: unknown, key: string): 'refused' | 'silently-dropped' | 'silently-kept' {
696+
const result = AssignmentConfigSchema.safeParse(config);
697+
if (!result.success) return 'refused';
698+
return Object.prototype.hasOwnProperty.call(result.data as object, key)
699+
? 'silently-kept'
700+
: 'silently-dropped';
701+
}
702+
703+
// `JSON.parse` is what makes `__proto__` an OWN enumerable key — an object
704+
// literal's `{ __proto__: ... }` sets the actual prototype instead and
705+
// never reaches `handleCatchall`'s `for (const key in input)` as a key at
706+
// all. Stored flow metadata arrives through exactly this door.
707+
const AUTHORED = '{"total":"{amount}","__proto__":"{evil}","other":1}';
708+
709+
it('REFUSES it — neither silently dropped nor silently kept', () => {
710+
expect(classify(JSON.parse(AUTHORED), '__proto__')).toBe('refused');
711+
});
712+
713+
it('names the key and locates it at the document root', () => {
714+
const result = AssignmentConfigSchema.safeParse(JSON.parse(AUTHORED));
715+
expect(result.success).toBe(false);
716+
if (result.success) return;
717+
const issue = result.error.issues.find((i) => i.path.join('.') === '__proto__');
718+
expect(issue).toBeDefined();
719+
expect(issue?.code).toBe('custom');
720+
expect(issue?.message).toContain('__proto__');
721+
// The refusal names the parser that would have dropped it. An author sent
722+
// to `z.record()` here would go looking at `assignments`, a different slot
723+
// with a different guard.
724+
expect(issue?.message).toContain('catchall');
725+
expect(issue?.message).toContain('assignment');
726+
});
727+
728+
it('refuses a config that is `__proto__` ALONE — no sibling key masks the drop', () => {
729+
expect(classify(JSON.parse('{"__proto__":"{evil}"}'), '__proto__')).toBe('refused');
730+
});
731+
732+
it('CONTROL — the same parse WITHOUT the guard silently drops it, on this exact zod', () => {
733+
// The instrument can see the defect: an unguarded object of the same
734+
// shape accepts the document and returns a different one. Without this
735+
// leg the pin above could be green because nothing ever reached the
736+
// catchall at all.
737+
const Unguarded = z.object({ assignments: z.record(z.string().min(1), z.unknown()).optional() })
738+
.catchall(z.unknown());
739+
const result = Unguarded.safeParse(JSON.parse(AUTHORED));
740+
expect(result.success).toBe(true);
741+
if (!result.success) return;
742+
expect(Object.keys(result.data)).toEqual(['total', 'other']);
743+
});
744+
745+
it.each(['constructor', 'prototype', 'toString'])(
746+
'PRESERVATION: `%s` remains a legal TOP-LEVEL variable name — only `__proto__` is refused',
747+
(name) => {
748+
// These reach the catchall unskipped and round-trip intact, so no
749+
// ruling narrows them. The guard refusing them would be a narrowing
750+
// nobody ordered.
751+
expect(classify(JSON.parse(JSON.stringify({ [name]: '{x}', keep: 1 })), name)).toBe('silently-kept');
752+
},
753+
);
754+
755+
it('PRESERVATION: every previously accepted shape still parses', () => {
756+
expect(AssignmentConfigSchema.safeParse({}).success).toBe(true);
757+
expect(AssignmentConfigSchema.safeParse({ assignments: {} }).success).toBe(true);
758+
expect(AssignmentConfigSchema.safeParse({ assignments: { total: '{amount}' } }).success).toBe(true);
759+
// The bare legacy config (no `assignments` wrapper) — the shape that makes
760+
// this top level an authoring surface in the first place.
761+
expect(AssignmentConfigSchema.safeParse({ decision: 'approved', digest: { dialect: 'cel' } }).success).toBe(true);
762+
// The canonical map's CEL envelope, and its malformed counterpart.
763+
expect(AssignmentConfigSchema.safeParse({ assignments: { d: { dialect: 'cel', source: 'a + b' } } }).success).toBe(true);
764+
expect(AssignmentConfigSchema.safeParse({ assignments: { d: { dialect: 'cel' } } }).success).toBe(false);
765+
});
766+
767+
it('leaves the `assignments` slot\'s OWN guard and prescriptions untouched', () => {
768+
// Two guards, two depths: the nested one still fires at its own path, and
769+
// the array-form prescription still rides the record's `invalid_type`.
770+
const nested = AssignmentConfigSchema.safeParse(JSON.parse('{"assignments":{"__proto__":"{evil}"}}'));
771+
expect(nested.success).toBe(false);
772+
if (nested.success) return;
773+
expect(nested.error.issues.some((i) => i.path.join('.') === 'assignments.__proto__')).toBe(true);
774+
expect(AssignmentConfigSchema.safeParse({ assignments: [{ variable: 'digest', value: 1 }] }).error!.issues[0]!.message)
775+
.toBe(ASSIGNMENT_ARRAY_FORM_PRESCRIPTION);
776+
});
777+
778+
it('the JSON projection is UNCHANGED — the guard adds no structure the Studio or the ledger can see', () => {
779+
// The wrapper is a `z.preprocess` pipe, which every spec walker resolves
780+
// through (`pipeAuthorableSide`). If it did not, the expression ledger
781+
// would lose `assignments.*` and the authorable surface would lose the
782+
// `assignments` row — silently, in both cases.
783+
const json = z.toJSONSchema(AssignmentConfigSchema, {
784+
target: 'draft-2020-12', io: 'input', unrepresentable: 'any',
785+
}) as { type?: string; properties?: Record<string, { additionalProperties?: Record<string, unknown> }> };
786+
expect(json.type).toBe('object');
787+
expect(Object.keys(json.properties ?? {})).toEqual(['assignments']);
788+
expect(json.properties?.assignments?.additionalProperties?.xExpression).toBe('value');
789+
const projected = getSchemalessNodeConfigJsonSchemas().assignment as
790+
{ properties?: Record<string, { additionalProperties?: Record<string, unknown> }> };
791+
expect(projected.properties?.assignments?.additionalProperties?.xExpression).toBe('value');
792+
});
793+
});

‎packages/spec/src/automation/builtin-node-config.zod.ts‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ import { z } from 'zod';
8282
import { EvaluatedExpressionSchema } from '../shared/expression.zod';
8383
import { lazySchema } from '../shared/lazy-schema';
8484
import { strictObject } from '../shared/strict-object';
85-
import { refuseRecordProtoKey } from '../shared/record-proto-key-guard';
85+
import { refuseCatchallProtoKey, refuseRecordProtoKey } from '../shared/record-proto-key-guard';
8686
import { isExpressionEnvelopeShaped } from './flow-node-expression-paths';
8787

8888
/** What a rejected key on these contracts silently did before #4001 批 9. */
@@ -918,8 +918,17 @@ export const ASSIGNMENT_ARRAY_FORM_PRESCRIPTION =
918918
* undeclared-key walk by design (its top-level keys may be variables), and a
919919
* closed shape here would contradict the descriptor's `additionalProperties:
920920
* true` the form↔Zod ledger pins.
921+
*
922+
* That openness is exactly why the top level needs its own `__proto__` guard
923+
* (#19151). `.catchall()`'s branch skips a `__proto__` own key above the
924+
* catchall schema, the same way `z.record()`'s branch skips it above the key
925+
* schema — so on a surface whose own keys ARE author-named variables, a
926+
* variable named `__proto__` parsed as SUCCESS and came back missing. It is
927+
* refused here instead; `assignments` one level down keeps its own guard,
928+
* because the two are different parsers at different depths and neither
929+
* covers the other.
921930
*/
922-
export const AssignmentConfigSchema = lazySchema(() => z.object({
931+
export const AssignmentConfigSchema = lazySchema(() => refuseCatchallProtoKey(z.object({
923932
/** Variable name → value; the canonical authoring surface. */
924933
assignments: refuseRecordProtoKey(
925934
z.record(z.string().min(1), AssignmentValueSchema, {
@@ -938,7 +947,19 @@ export const AssignmentConfigSchema = lazySchema(() => z.object({
938947
).optional()
939948
.describe('Variables to set: each key is a variable name, each value a `{token}` template, a CEL value envelope, or a literal'),
940949
})
941-
.catchall(z.unknown()));
950+
// Open by design: the bare legacy `{ <variable>: <value> }` config and any
951+
// top-level key an author names live here.
952+
.catchall(z.unknown()),
953+
// [#19151] `__proto__` ONLY, and for the same structural reason
954+
// the `assignments` slot above refuses it: `handleCatchall`'s
955+
// `if (key === "__proto__") continue;` runs above `_catchall.run`, so no
956+
// catchall schema can ever see the key. `constructor`, `prototype` and
957+
// every other reserved-looking name reach the catchall unskipped and
958+
// round-trip intact (measured), so they stay legal top-level variable
959+
// names — nothing here narrows this surface's accept set beyond the one
960+
// name zod makes unrepresentable.
961+
'an `assignment` node config',
962+
));
942963

943964
export type AssignmentConfig = z.input<typeof AssignmentConfigSchema>;
944965
export type AssignmentConfigParsed = z.infer<typeof AssignmentConfigSchema>;

‎packages/spec/src/shared/record-proto-key-guard.test.ts‎

Lines changed: 86 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,15 @@
33
import { z } from 'zod';
44
import { describe, expect, it } from 'vitest';
55

6-
import { refuseRecordProtoKey } from './record-proto-key-guard';
6+
import { refuseCatchallProtoKey, refuseRecordProtoKey } from './record-proto-key-guard';
77

88
/**
9-
* Isolated pin for the guard itself, independent of either real consumer
10-
* (`ObjectSchema.fields`, `AssignmentConfigSchema.assignments` — objectstack
11-
* #17852 / #18847). Those two files pin the guard wired into a real
12-
* authoring surface; this file pins the guard's own contract against a
13-
* minimal record so a future change to either consumer schema cannot mask a
14-
* regression here.
9+
* Isolated pins for the guards themselves, independent of their real
10+
* consumers (`ObjectSchema.fields`, `AssignmentConfigSchema.assignments` and
11+
* `AssignmentConfigSchema`'s own catchall — objectstack #17852 / #18847 /
12+
* #19151). Those files pin each guard wired into a real authoring surface;
13+
* this file pins the guards' own contracts against minimal schemas so a
14+
* future change to a consumer schema cannot mask a regression here.
1515
*
1616
* These assertions are BEHAVIOUR pins, not a zod-version pin (as ordered):
1717
* `packages/spec/package.json` pins `zod` at `^4.4.3`, and a bump inside
@@ -91,3 +91,82 @@ describe('refuseRecordProtoKey', () => {
9191
expect(result.error.issues[0]!.message).toBe('no arrays here');
9292
});
9393
});
94+
95+
/**
96+
* `refuseCatchallProtoKey` — the sibling for zod's OTHER open-key branch
97+
* (objectstack#19151). Same mechanism, same refused name, same issue shape;
98+
* what differs is the parser the message names, and that difference is the
99+
* reason it is a second wrapper rather than a second call site of the first.
100+
*/
101+
describe('refuseCatchallProtoKey', () => {
102+
const Open = () => z.object({ known: z.string().optional() }).catchall(z.unknown());
103+
const Guarded = refuseCatchallProtoKey(Open(), 'a widget config');
104+
105+
it('refuses a `__proto__` own key with a named, self-locating issue', () => {
106+
const input = JSON.parse('{"known":"a","__proto__":"2"}');
107+
const result = Guarded.safeParse(input);
108+
expect(result.success).toBe(false);
109+
if (result.success) return;
110+
expect(result.error.issues).toHaveLength(1);
111+
const [issue] = result.error.issues;
112+
expect(issue.code).toBe('custom');
113+
expect(issue.path).toEqual(['__proto__']);
114+
expect(issue.message).toContain('a widget config');
115+
expect(issue.message).toContain('__proto__');
116+
});
117+
118+
it('names `.catchall()`, NOT `z.record()` — the two positions drop the key for different reasons', () => {
119+
// An author sent to the wrong parser looks in the wrong place. This is
120+
// the whole reason the wrapper is not a relabelled `refuseRecordProtoKey`.
121+
const catchallMessage = Guarded.safeParse(JSON.parse('{"__proto__":1}')).error!.issues[0]!.message;
122+
const recordMessage = refuseRecordProtoKey(z.record(z.string(), z.string()), 'things')
123+
.safeParse(JSON.parse('{"__proto__":"1"}')).error!.issues[0]!.message;
124+
expect(catchallMessage).toContain('catchall');
125+
expect(catchallMessage).not.toContain('z.record()');
126+
expect(recordMessage).toContain('z.record()');
127+
expect(recordMessage).not.toContain('catchall');
128+
});
129+
130+
it('the underlying catchall really would have silently dropped it — the control this guard exists to fail', () => {
131+
// Proves the defect on the pinned zod rather than from the docblock's
132+
// quoted excerpt: the unguarded object accepts the document and returns a
133+
// DIFFERENT one.
134+
const result = Open().safeParse(JSON.parse('{"known":"a","__proto__":"2"}'));
135+
expect(result.success).toBe(true);
136+
if (!result.success) return;
137+
expect(Object.keys(result.data)).toEqual(['known']);
138+
});
139+
140+
it('leaves an object with no `__proto__` key untouched, catchall keys included', () => {
141+
const result = Guarded.safeParse({ known: 'a', anything: 1, constructor: 'c', prototype: 'p' });
142+
expect(result.success).toBe(true);
143+
if (!result.success) return;
144+
expect(result.data).toEqual({ known: 'a', anything: 1, constructor: 'c', prototype: 'p' });
145+
});
146+
147+
it('does not choke on non-object input — the wrapped schema still reports its own type error', () => {
148+
const result = Guarded.safeParse('nope');
149+
expect(result.success).toBe(false);
150+
if (result.success) return;
151+
expect(result.error.issues[0]!.code).toBe('invalid_type');
152+
});
153+
154+
it('does not choke on null or an array', () => {
155+
expect(Guarded.safeParse(null).success).toBe(false);
156+
expect(Guarded.safeParse([1, 2]).success).toBe(false);
157+
});
158+
159+
it('composes with `.optional()` — undefined never runs the guard', () => {
160+
expect(refuseCatchallProtoKey(Open(), 'a widget config').optional().safeParse(undefined).success).toBe(true);
161+
});
162+
163+
it('keeps the wrapped shape readable through the spec walkers\' pipe resolution', () => {
164+
// `zodShapeOf`/`pipeAuthorableSide` resolve a preprocess pipe to its OUT
165+
// side; if the guard broke that, a guarded schema would walk "to no
166+
// shape" and silently stop being governed (the #4488 blind spot).
167+
const def = (Guarded as unknown as { _zod: { def: Record<string, unknown> } })._zod.def;
168+
expect(def.type).toBe('pipe');
169+
const out = def.out as z.ZodObject<z.ZodRawShape>;
170+
expect(Object.keys(out.shape)).toEqual(['known']);
171+
});
172+
});

0 commit comments

Comments
 (0)