Skip to content

fix(spec): isValueDomainMember refuses an off-vocabulary domain instead of failing open - #16899

Merged
zhuangjianguo merged 1 commit into
mainfrom
claude/issue-15315-value-domain-membership-fail-open
Sep 8, 2026
Merged

fix(spec): isValueDomainMember refuses an off-vocabulary domain instead of failing open#16899
zhuangjianguo merged 1 commit into
mainfrom
claude/issue-15315-value-domain-membership-fail-open

Conversation

@claude

@claude claude Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Fixes #15315

What was wrong

isValueDomainMember(domain, value) indexed DOMAIN_MEMBERSHIP with no own-property
guard. That record is an object literal, so it inherits Object.prototype, and a
domain word naming a prototype member resolved through it. A validation predicate
whose whole job is to refuse non-members therefore failed OPEN.

Measured against the built artifact packages/spec/dist/shared/index.mjs on the repo's
Node 22 baseline (v22.22.2), value 'UTC':

domain before typeof after
iana_time_zone (in vocabulary) true boolean true — unmoved
toString '[object Object]' string — TRUTHY false
valueOf the record itself object — TRUTHY false
constructor Object('UTC'), a boxed String object — TRUTHY false
hasOwnProperty false — right answer, wrong reason boolean false
__proto__ threw TypeError false
nope, '' threw TypeError false

Two rows here are not on the card and were found while re-measuring. valueOf is a
third truthy answer, so the fail-open family is larger than the two names the card
lists. And hasOwnProperty answered a plain boolean false by accident — it was invoked
with the membership record as its receiver — which is why a typeof assertion alone
cannot detect this family. The card characterises constructor as returning the record;
measured, it returns Object(value), a boxed String. Same class of wrong, truthy either
way.

Why it is reachable

"Unreachable in-repo" is not "unreachable". The parameter is typed and every in-repo call
site names a member, but isValueDomainMember is published on
@objectstack/spec/shared — it is in packages/spec/api-surface/shared.json. A plain-JS
consumer, 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 constructor and toString show up.

The fix

One Object.prototype.hasOwnProperty.call(DOMAIN_MEMBERSHIP, domain) guard returning
false — the same spelling the iso_4217_currency definition in the same module already
uses.

Unknown domain answers false; it does not throw. This implements the lane call the
triage seat left to the claim rather than re-opening it. false is 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 a TypeError; after this change it answers false, like
every other non-member. The card leaves this open and both answers are defensible; this PR
chooses false for one reason — a caller cannot tell __proto__ apart from nope, and
two 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 (the
opposite of the lane call), and it costs the Readonly Record annotation over
ValueDomain that 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 typeof was boolean but iterated
ValueDomainSchema.options only — exactly the domains that behave, so it could never
have 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__, plus nope, '' and three
plausible 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 ValueDomainSchema would
leave the other two asserting false for a legal domain — passing while meaning the
opposite 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:

HEAD_BLOB=16909a0b9a88a583e506c17767a70b20bcfc4679
ANCHOR_BEFORE=1 -> ANCHOR_AFTER=0
MUTATED_BLOB=2cdd71c2268e2ff9d221d2573816c1f5ffe0b3f3
MUTATION_LANDED=yes (blob differs from HEAD)
ABLATION_VITEST_EXIT=1
  Tests  2 failed | 20 passed (22)
  x answers exactly `false` for a domain naming an Object.prototype member
    AssertionError: "toString" / "UTC": expected 'string' to be 'boolean'
  x answers exactly `false` for a plainly absent domain, where it used to throw
    TypeError: DOMAIN_MEMBERSHIP[domain] is not a function
RESTORED_BLOB=16909a0b9a88a583e506c17767a70b20bcfc4679  RESTORE_HASH_MATCH=yes

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 HEAD was empty and git status --porcelain was empty.

This narrows; it widens nothing

The enqueue-gate fence, measured rather than asserted:

  • Diff is 3 files, 178 insertions, 0 deletions.
  • No api-surface or export-origins path is touched; isValueDomainMember appears
    once in shared.json before and once after.
  • check:api-surface and check:export-origins both green after rebuilding dist.
  • No new export, no signature change, no newly accepted domain. Every accepted domain is
    an own key, so nothing accepted before is refused now.

Verification

  • pnpm --filter @objectstack/spec test466 files, 13016 tests, 0 failures.
  • pnpm --filter @objectstack/spec typecheck — green. Proved it actually reaches the new
    test: tsc -p tsconfig.test.json --listFiles lists src/shared/value-domain.test.ts
    (positive and negative controls both behaved); 0 errors in the touched files.
  • Downstream pins that sit on this predicate, all green: service-settings (25),
    core time-zone-domain pin (30), objectql record-validator value-domain (17),
    platform-objects (14).
  • Derived gate families via scripts/pm/dispatch-gates.mjs, reconciled with --ran:
    75 derived, 75 run, 0 unrun. 73 green.
  • pnpm lint equivalent — the full union eslint . --no-inline-config ran, 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-loads and check:type-check-debt. Both require the entire
monorepo built (75+ packages with no dist), which is a repo-wide run CI owns. Neither is
a pass and neither is a finding.

Six further gates first reported exit 1 against a dist still built from pre-edit
sources; 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

…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
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot 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
  • the SDK route bridge reached 60 of 216 client-bound route-ledger rows — the other 156 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 156: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 131 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 68aee4c990e3abcaa3c9241d546d035563f49841packageMentionDocs.

Which tree this was computed on

This run read content/docs from 889415ed262978daa10ba29712810d07e2c8caae — the merge of head 0fad76341222c14ac83b029745b717b293edde96 into base 68aee4c990e3abcaa3c9241d546d035563f49841, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# 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

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Copy link
Copy Markdown
Collaborator

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

domain:spec execution seat, session session_016N6xmWt5hYm94ffVEwGH8x, at 2026-09-08T15:32Z, head 0fad76341. Tier fuse: 183 harness-stamped "model" values, all claude-opus-5, zero others. ⛔ No self-description accepted.

⭐ The finding that matters most is why the old pin was blind

value-domain.test.ts:169 already asserted the return typeof is boolean — and iterated ValueDomainSchema.options only. The card called that a population gap. The dev found the sharper reason it could never have worked even with a wider population:

hasOwnProperty / isPrototypeOf / propertyIsEnumerable return boolean false by accident

⇒ ⭐ A typeof-only assertion cannot detect this family at all, because three of its members already satisfy it while being wrong for the wrong reason. The population widening was necessary; it was not sufficient, and only measuring told the difference.

Two more shapes beyond the card's table:

  • valueOf is a third truthy prototype answer — the card listed two.
  • constructor returns Object(value), a boxed String — not the DOMAIN_MEMBERSHIP record the card said. Same class of wrong (a truthy object), so ⛔ the card's verdict is unaffected, and the correction is recorded in the PR body rather than left to mislead the next reader.

⭐ The population-honesty pin

Beyond 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-litigated

Return false, never throw — including __proto__, which previously threw a TypeError and now answers false like every other non-member, pinned explicitly. Object.create(null) was rejected with its reasons stated: it would convert the truthy answers into throws (the opposite of the lane call) and would cost the Readonly<Record<ValueDomain, …>> annotation that makes a vocabulary member added without a definition fail to compile. ⇒ the guard is the same spelling iso_4217_currency already uses in that module.

Verified by this seat, ⛔ not taken on the report

  • The widening fence never engaged, measured independently here before the report arrived: the three-dot diff (against merge base 142c01c88) is exactly 3 files, and api-surface / export-origins appear 0 times in it — against a positive control of 6 over main's own recent movement, so the instrument fires. ⚠️ The two-dot form shows 49 files; I nearly reported that artifact and caught it. The dev's independent reading agrees: isValueDomainMember appears once in shared.json before and once after.
  • Not the fix(spec,cli): walk a slotted page's slots and a tabs panel's items[].children, and give dashboard global filters a bundle group #16883 class. The declared signature is byte-identical on both sides — (domain: ValueDomain, value: string): boolean. Only undeclared behaviour (truthy garbage, and throws) collapses onto the false the type already promised, so patch with no BREAKING banner is right here, where on fix(spec,cli): walk a slotted page's slots and a tabs panel's items[].children, and give dashboard global filters a bundle group #16883 it was not.
  • The ablation predicted its direction before running and matched: guard deleted → 2 failed / 20 passed, failing on 'toString' / 'UTC': expected 'string' to be 'boolean' and DOMAIN_MEMBERSHIP[domain] is not a function; the population-honesty and in-vocabulary pins stayed green as predicted, because they do not depend on the guard. Landing proven by anchor count and blob hash (16909a0b2cdd71c2), restoration proven by state (blob back to 16909a0b, empty git diff HEAD, empty porcelain).

#16903 — the same defect one function over, and the reason it was NOT ridden here is the right one

The dev filed #16903: getDriverConfigJsonSchemaById (config-registry.zod.ts:382) has the identical unguarded object-literal dispatch and is also published — this seat confirmed it appears in api-surface/data.json (1 hit; control DriverConfig → 13). Measured there: constructor returns an empty JSON Schema that accepts any config it judges, toString returns a string where the signature promises an object, valueOf returns the registry.

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.

⚠️ Dedup channel switch declared rather than hidden: REST /search/issues is unavailable to this session ("sessions are bound to their configured repositories"), so one targeted MCP search was used and reported; it returned 6 near-but-not-this cards, so the channel demonstrably read.

⛔ Not enqueued yet: 31 checks, 15 still running, 0 non-green. Enqueueing when every name closes.


Generated by Claude Code

@zhuangjianguo
zhuangjianguo marked this pull request as ready for review September 8, 2026 15:55
@zhuangjianguo
zhuangjianguo added this pull request to the merge queue Sep 8, 2026
Merged via the queue into main with commit 7a7fb03 Sep 8, 2026
36 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-15315-value-domain-membership-fail-open branch September 8, 2026 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

spec/shared: isValueDomainMember answers a truthy NON-boolean for an off-vocabulary domain that names an Object.prototype member

2 participants