fix(spec): isValueDomainMember refuses an off-vocabulary domain instead of failing open - #16899
Conversation
…tead of failing open `DOMAIN_MEMBERSHIP` is an object literal, so it inherits `Object.prototype`, and the predicate indexed it with no own-property guard. Measured against the built artifact on the Node 22 baseline (v22.22.2), a `domain` outside the vocabulary did one of two wrong things, and one of them was a membership FALSE POSITIVE out of a function whose whole job is to refuse non-members: `toString` answered the truthy string '[object Object]', `valueOf` and `constructor` answered truthy objects, while `__proto__`, `nope` and '' threw a TypeError off a non-callable. The parameter is typed and every in-repo call site names a member, but "unreachable in-repo" is not "unreachable": `isValueDomainMember` is published on `@objectstack/spec/shared` (`packages/spec/api-surface/shared.json`), so a plain-JS consumer, or any caller passing a domain string read from metadata rather than written in source, reaches it with no type checking at all. An `Object.prototype.hasOwnProperty.call` guard — the same spelling the `iso_4217_currency` definition in this module already uses — now returns `false` for a domain that is not an own key. This narrows and widens nothing: every accepted domain is an own key, so no previously accepted value is refused, and the three real domains answer from their own definitions unmoved. `false` rather than a thrown refusal is the narrowing reading; throwing would change published behaviour for callers who today receive a truthy value. A null-prototype record was the other available shape and was not taken: it converts the truthy answers into throws rather than into `false`, and it costs the `Readonly<Record<ValueDomain, ...>>` annotation that makes a vocabulary member added without a definition fail to compile. The pin that existed could not have caught this. It asserted the return `typeof` was `boolean` but iterated `ValueDomainSchema.options` only — exactly the domains that behave. The fix is as much about the pin's POPULATION as about the guard: new pins put `toString`, `valueOf`, `constructor`, `hasOwnProperty`, `isPrototypeOf`, `propertyIsEnumerable`, `__proto__` and plainly absent words into the population, and a third pin holds that population honest by asserting each is still outside the vocabulary. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016N6xmWt5hYm94ffVEwGH8x
📓 Docs Drift Check1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. What this run could not see
Coarse fallback — 131 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 889415ed262978daa10ba29712810d07e2c8caae && git checkout 889415ed262978daa10ba29712810d07e2c8caae
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 68aee4c990e3abcaa3c9241d546d035563f49841 0fad76341222c14ac83b029745b717b293edde96 && git checkout -B drift-repro 68aee4c990e3abcaa3c9241d546d035563f49841 && git merge --no-ff 0fad76341222c14ac83b029745b717b293edde96
node scripts/docs-audit/affected-docs.mjs --json 68aee4c990e3abcaa3c9241d546d035563f49841 |
ACCEPT — the dev found three shapes the card's own table missed, and one of them explains why the existing pin could never have caught this
⭐ The finding that matters most is why the old pin was blind
⇒ ⭐ A Two more shapes beyond the card's table:
⭐ The population-honesty pinBeyond widening the population, the dev added a pin asserting every one of those words is still outside the vocabulary. ⇒ if a future vocabulary gains a member named like one of the probes, the pin fails loudly instead of silently becoming vacuous. That is the failure mode that produced this card, pinned against recurrence. The lane call was implemented, ⛔ not re-litigatedReturn Verified by this seat, ⛔ not taken on the report
⭐ #16903 — the same defect one function over, and the reason it was NOT ridden here is the right oneThe dev filed #16903: ⛔ Correctly not taken under the bounded in-place exemption, and the reasoning is exactly the fence I set: that exemption needs a mechanical change whose shape is already pinned, and this one is not — the function's return is non-optional, so refusing requires either widening the published return type to an optional or throwing. The first widens a published signature, which ⛔ cannot get an at-tier review while fable is exhausted. ⇒ filed, not ridden.
⛔ Not enqueued yet: 31 checks, 15 still running, 0 non-green. Enqueueing when every name closes. Generated by Claude Code |
Fixes #15315
What was wrong
isValueDomainMember(domain, value)indexedDOMAIN_MEMBERSHIPwith no own-propertyguard. That record is an object literal, so it inherits
Object.prototype, and adomainword naming a prototype member resolved through it. A validation predicatewhose whole job is to refuse non-members therefore failed OPEN.
Measured against the built artifact
packages/spec/dist/shared/index.mjson the repo'sNode 22 baseline (v22.22.2), value
'UTC':domaintypeofiana_time_zone(in vocabulary)truetrue— unmovedtoString'[object Object]'falsevalueOffalseconstructorObject('UTC'), a boxed StringfalsehasOwnPropertyfalse— right answer, wrong reasonfalse__proto__TypeErrorfalsenope,''TypeErrorfalseTwo rows here are not on the card and were found while re-measuring.
valueOfis athird truthy answer, so the fail-open family is larger than the two names the card
lists. And
hasOwnPropertyanswered a plain booleanfalseby accident — it was invokedwith the membership record as its receiver — which is why a
typeofassertion alonecannot detect this family. The card characterises
constructoras returning the record;measured, it returns
Object(value), a boxed String. Same class of wrong, truthy eitherway.
Why it is reachable
"Unreachable in-repo" is not "unreachable". The parameter is typed and every in-repo call
site names a member, but
isValueDomainMemberis published on@objectstack/spec/shared— it is inpackages/spec/api-surface/shared.json. A plain-JSconsumer, or any caller handing over a domain string read from metadata rather than
written in source, reaches it with no type checking at all, and metadata-sourced strings
are exactly where
constructorandtoStringshow up.The fix
One
Object.prototype.hasOwnProperty.call(DOMAIN_MEMBERSHIP, domain)guard returningfalse— the same spelling theiso_4217_currencydefinition in the same module alreadyuses.
Unknown domain answers
false; it does not throw. This implements the lane call thetriage seat left to the claim rather than re-opening it.
falseis the narrowing reading:it refuses more and accepts nothing new, whereas a thrown refusal would change published
behaviour for callers who today receive a truthy value. It is also the sister ruling's
settled third branch — list reject / own-member value / prototype-resolvable gives reject.
What this does to
__proto__, stated and pinned__proto__today throws aTypeError; after this change it answersfalse, likeevery other non-member. The card leaves this open and both answers are defensible; this PR
chooses
falsefor one reason — a caller cannot tell__proto__apart fromnope, andtwo different refusal mechanisms for two indistinguishable non-members is the surface the
guard exists to collapse. It is pinned explicitly, not left implicit.
A null-prototype record was the other shape the card names and was not taken, for
two reasons: it converts the truthy answers into throws rather than into
false(theopposite of the lane call), and it costs the
ReadonlyRecordannotation overValueDomainthat makes a vocabulary member added without a definition fail to compile— a guarantee the record's own doc comment exists to state.
The pin changes its POPULATION, not just its assertions
The pin that existed asserted the return
typeofwasbooleanbut iteratedValueDomainSchema.optionsonly — exactly the domains that behave, so it could neverhave caught this. The old loop is kept (it is the right population for its own claim, that
every member has a definition) and a comment now says so. The new pins put the failing
shapes into the population:
toString,valueOf,constructor,hasOwnProperty,isPrototypeOf,propertyIsEnumerable,__proto__, plusnope,''and threeplausible near-miss words — each crossed with five values.
A third pin holds that population honest: it asserts every one of those words is still
outside the vocabulary. Without it, a word later promoted into
ValueDomainSchemawouldleave the other two asserting
falsefor a legal domain — passing while meaning theopposite of what they say.
Ablation — the new pin fails on the unfixed code
Run from the committed state, guard line deleted, restored under
trap ... EXIT INT TERM:Direction was predicted before the run and matched: the two new behavioural pins go red,
the population-honesty pin and the in-vocabulary pin stay green — they do not depend on the
guard. Mutation proved landed by anchor count and blob hash, never by an editor's exit
code. Afterwards
git diff HEADwas empty andgit status --porcelainwas empty.This narrows; it widens nothing
The enqueue-gate fence, measured rather than asserted:
api-surfaceorexport-originspath is touched;isValueDomainMemberappearsonce in
shared.jsonbefore and once after.check:api-surfaceandcheck:export-originsboth green after rebuildingdist.domainisan own key, so nothing accepted before is refused now.
Verification
pnpm --filter @objectstack/spec test— 466 files, 13016 tests, 0 failures.pnpm --filter @objectstack/spec typecheck— green. Proved it actually reaches the newtest:
tsc -p tsconfig.test.json --listFileslistssrc/shared/value-domain.test.ts(positive and negative controls both behaved); 0 errors in the touched files.
service-settings(25),coretime-zone-domain pin (30),objectqlrecord-validator value-domain (17),platform-objects(14).scripts/pm/dispatch-gates.mjs, reconciled with--ran:75 derived, 75 run, 0 unrun. 73 green.
pnpm lintequivalent — the full unioneslint . --no-inline-configran, 6377 files,0 errors, 0 warnings, at
0fad76341. Not a narrowing: the whole population was linted.Two gates were NOT MEASURED and are declared to CI, both exit code 3
(prerequisite-not-met, which each script distinguishes from a finding's 1):
check:dual-build-cjs-loadsandcheck:type-check-debt. Both require the entiremonorepo built (75+ packages with no
dist), which is a repo-wide run CI owns. Neither isa pass and neither is a finding.
Six further gates first reported exit 1 against a
diststill built from pre-editsources; each printed its own prerequisite banner rather than a verdict. After
pnpm exec turbo run build, all six were re-run and are green(
check:api-surface,check:browser-reachable-entries,check:dual-source-exports,check:entry-nameability,check:exported-any,check:doc-formula-expressions).🤖 Generated with Claude Code
https://claude.ai/code/session_016N6xmWt5hYm94ffVEwGH8x
Generated by Claude Code