Repository navigation
Realm-safe verdicts: hook-aware scans, brand checks, union passes-gate - #317
Merged
Merged
Conversation
…g trust (#316) Panel diffs values shared live across iframe boundaries, where instanceof Array/Map/Set fails because each realm owns its constructors. Dense clean arrays then scored exactly one fallback problem and lost every Vec|number[] union, printing confident 'closest is number[]' rows for anchor/margin the checker had passed. isArrayValue/isMapValue/isSetValue (brand checks, one function per file, barrel-exported, realmBrand.spec.js) replace instanceof in all verdict paths: validators, windowed map scan, and the differ. Tagged snapshots get the validator's nominal trust in the differ too: a union member matching the $type tag accepts outright, and the object branch exempts transport-stripped methods by tag equality even when materialization erased the class name or the panel registry lacks the classes. crossRealmDiagnosis.spec.js pins the production payload (foreign arrays plus tagged snapshot diff to exactly the alignment row), mutation-verified per fix line.
snapshotTag rejected any object whose prototype is not the local Object.prototype, so snapshots shared live across iframe boundaries kept their home realm's prototype and lost their tag. Every nominal path in the differ went blind and counted transport-stripped methods as defects against the 12-method engine shapes. The gate now accepts any root prototype (proto null, or proto-of-proto null): live instances still fail it in every realm. unwrapTransparent folds Readonly/NonNullable/NoInfer members to the nominal name the harvest spells (getter Readonly<Color> vs bare tag), in the union shortcut and the object-branch method exemption. crossRealmDiagnosis pins the wrapped member; snapshotTag.spec pins foreign tags, foreign instances, and impostors.
…n passes-gate (#316) firstBadIndex skipped element-level userland hooks: a clean primitive scan returned -1 and validateArray trusted it, so customTypes.number rejecting [1,2] still passed. The scan now bails to 0 when customValidations is non-empty or customTypes holds the scanned primitive. firstBadEntry had the same hole one level up: it trusted instanceof past a customTypes hook shadowing the class name, so Map<string, LiveBox> with a rejecting hook passed. All three class branches bail to -2 on a shadowing hook. Realm-safe container checks everywhere a verdict depends on them: isPromiseValue (brand, not constructor; thenables still fail), validateGlobalConstructor (instanceof, then constructor-name fallback for live-shared platform instances), validateIArguments (typeof the iterator function, not instanceof Function), collectCandidates tuple and array branches (isArrayValue), tagSnapshot traversal and tagName exclusions plus stringifyValue Map/Set branches (brand, so foreign entries survive tagging and logging). explainMismatch gains the last-resort union passes-gate: when every member fails structurally but the validator accepts one, the row would contradict the verdict, so it diffs clean (informational extras on clean members keep flowing, only would-be rows convert). Corrects the windowing spend-budget doc to what the code does: the total restarts on size changes; same-size replacements ride until the next scheduled full scan, at most one window away. Covered by liveSharing.spec.js (21 hostile local-vs-foreign pairs), validateGlobalConstructor.spec.js, and hook-bypass pins in firstBadIndex.spec.js/firstBadEntry.spec.js; every fix verified by stash-revert failing exactly its tests.
Hand-written object/union/reference literals rot when the parser shape changes and teach a construction path real code never uses. The tower shapes and the Readonly-wrapped member now come from the same parser the transpiler feeds, with the empty-args reference kept literal (no valid spelling parses to it).
Measured the own-tag guard at ~60ns per promise check against zero threat: spoofing Symbol.toStringTag requires deliberate craft in a realm that already runs code, while the slot-probing alternative manufactures unhandled rejections on ordinary rejected promises. The JSDoc now states the tradeoff instead of over-claiming; the spec pins cross-realm, subclass and thenable behavior.
diffValue carried three inline realm/nominal branches with their own essays: the union tag shortcut, the union validator last-resort, and the object-branch method exemption, plus a local classMatches twin. Each is now one function per file with its own JSDoc @example and its own spec (unionNominalTag, unionValidatorPass, snapshotNominalMatch, classMatches), wired into test_runtime.js and the barrel. diffValue keeps three one-line calls; behavior is unchanged (every helper spec fails when its helper is stubbed, full suite green before and after).
validateIArguments accepted anything iterable; tsc requires indexed elements plus integer length plus callee, so arrays and generators passed RTI while failing tsc-strict. The check now requires all three, with guarded reads: strict-mode arguments throw on .callee access, so an iterator plus length with a throwing callee still accepts (genuine strict arguments), while a readable non-function callee fails. Fixture gains the array case with @ts-expect-error (consumed, tsc run stays green); output regenerated from the real emit; errors.json extended. Stash-reverting the validator fails exactly the array and generator specs.
Trusted application code: accidental cross-realm incompatibility and accidental misclassification are in scope, deliberate spoofing (Symbol.toStringTag, fabricated constructor names) is explicitly out. Brand and constructor checks stay pragmatic compatibility heuristics, never security boundaries.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #316
Primer
Runtime Type Inspector validates values where the engine runs and reports where the panel reads, often across an iframe boundary that shares values live without cloning. Two prototype systems then disagree: the checking realm's
Array(orMap,Set,Promise, class) constructor is not the panel realm's, so everyinstanceofcheck against a realm-local constructor fails for a genuine foreign value, and every prototype-identity gate (Object.getPrototypeOf(value) === Object.prototype) rejects foreign plain objects. The validators and the on-click differ both contained such gates at exactly the places where a verdict turns: container acceptance (validateArray,validateMap,validateSet,validateTuple,validatePromise,validateIArguments,validateGlobalConstructor), template inference collection (collectCandidates), snapshot tagging and traversal (tagSnapshot,snapshotTag), and log serialization (stringifyValue). The fast-path scans added a second, independent verdict hazard:firstBadIndexandfirstBadEntryskip the full validation loop when their cheap predicates report clean, which is only sound when the predicates mirror the full loop exactly — and neither consulted element-level userland hooks (customTypes,customValidations), so a rejecting hook was silently bypassed. The union differ had the mirror-image flaw: it picked the closest structural match with no validator consultation, printing "matches no union member" for values the validator accepted (nominal trust, option leniency), which is exactly the phantom-row class from production (data.anchor,data.color,data.margineach flaggedclosest is number[] with 1 problemwhile the checker passed them).What changed
src-runtime/isPromiseValue.js(new, barrel-exported, spec-pinned inrealmBrand.spec.js): brand check (Object.prototype.toStringis[object Promise]) replacinginstanceof PromiseinvalidatePromise. Thenables still fail (no duck-typing); foreign promises pass.src-runtime/validateGlobalConstructor.jskeepsinstanceoffor same-realm subclasses, then falls back to constructor-name equality for live-shared platform instances (foreignFloat32Arrayvalidates; plain impostor shapes fail).validateGlobalConstructor.spec.js(new) pins local, foreign, impostor, and unknown-name cases.src-runtime/validateIArguments.jscheckstypeof value[Symbol.iterator] === 'function'instead ofinstanceof Function, so foreign iterables pass the protocol check.src-runtime/collectCandidates.jsusesisArrayValuein its object, tuple, and array branches, so template inference collects positionally from foreign arrays and stops treating foreign arrays as objects with named properties.src-runtime/tagSnapshot.jstraverses and excludes by brand (isMapValue/isSetValue) instead ofinstanceof Map/Set, so instances nested in foreign maps keep their$typetags and foreign maps serialize by entries.src-runtime/stringifyValue.jsgates its Map/Set branches on the same brands, so foreign maps log as{Map, entries}instead of collapsing to keyless{}.src-runtime/firstBadIndex.jsbails to a full scan (returns 0) whencustomValidationsis non-empty orcustomTypesholds the scanned primitive (customTypes.numberfornumber, and so on). Reproduced before the fix:customTypes.number = () => falseplus[1, 2]vsnumber[]validatedtrue.src-runtime/firstBadEntry.jsbails to-2when acustomTypeshook shadows a registered class name, in all three class branches (keys, bare values, single-arg references). Reproduced before the fix:customTypes.LiveBox = () => falseplusMap<string, LiveBox>of genuine instances validated clean.src-runtime/explainMismatch.jsgains the last-resort union passes-gate: when every member fails structurally, each member is probed with the validator itself, and a passing member diffs clean. The gate only ever converts would-be rows to clean, never the reverse, so structurally clean members (with their informational children) keep flowing through the existing loop untouched. Reproduced before the fix:{a: 1, typo: 2}vs{a: number} | {b: string}with Exact objects off printed "matches no union member" for a passing value.src-runtime/plugin-windowed/windowedOptions.jscorrects the spend-budget doc to what the code does: the accumulated scan total restarts on size changes; same-size value replacements keep riding until the next scheduled full scan (at most one window away, bounded bymapCheckWindowCap), because observing content without scanning is exactly what riding skips.src-runtime/liveSharing.spec.js(new, 21 tests) is the hostile local-vs-foreign battery: every verdict runs the same shape twice (local build plusvm.runInNewContextlive-shared build) and asserts agreement — arrays, tuples, nested members, Maps/Sets with entry checks, promises vs thenables, foreign iterables,IArguments, positional template candidates, tag traversal through foreign maps, Map serialization, mistyped-foreign still fails, wrong-tag fails, foreign same-spelled instances underPartial, deep foreign nesting pinpointing, frozen/sparse/proxied arrays, array-likes, constructor-name platform checks, and display survival.firstBadIndex.spec.jsgains the hook-bypass pins;firstBadEntry.spec.jsgains the class-hook shadowing pin.Proof
tmp/repro-windowing.mjsharness (scratch, not committed) showedgot true, want falseon the hook-rejection cases before the fix andgot falseafter, with the production cross-realm matrix (tmp/repro-matrix.mjs, foreign arrays plus tagged snapshot) diffing to exactly the one genuine alignment row in all four environments (same-realm, cloned, foreign live-shared, missing classes).firstBadIndex.jsfails exactly the two hook tests; stashingfirstBadEntry.jsfails exactly the shadowing test; reverting any brand swap fails exactly its live-sharing tests; dropping the union gate fails exactlytestExcessUnionStaysClean; revertingsnapshotTag.jsfails 13 tag specs; narrowingunwrapTransparentfails 8 nominal tests. Restores verified green after each.tmp/repro-real-color.mjs(scratch) diffs the real 12-method engineColorharvest (parsed out ofplaycanvas-engine's own.rti.mjs) against the tagged snapshot with both registries present and absent: clean in both, matching the worker's nominal accept.Latent holes
windowedOptionsstates the latency bound; rides stay off by default with a 100ms earn budget). Verdict: tolerated deliberately — detection is bounded, never lost, and false positives stay impossible.LiveBoxdiffsdata.rows[0].get("a").wwhile validation trusts the identity, same as tsc trusting the declared type). The union gate suppresses only the misleading "no member matches" verdict; a direct member row names a real defect. Verdict: tolerated deliberately and pinned (testDeepForeignNestingPinpointsasserts both halves).describeValue,Warning,typeTree, andDisplayAnythingkeep realm-localinstanceofbranches: they render, never decide, and foreign values degrade to generic-but-correct displays instead of throwing (pinned bytestForeignDisplaysSurvive). Verdict: tolerated deliberately — display-only, no verdict path reads them.validateTypeofkeeps nominal identity for class targets: a foreign same-named class correctly fails, since nominal identity cannot cross realms. Verdict: tolerated deliberately —typeof Xis about the registered constructor, not the brand.