Fix out-of-range @ref result references in program interpreter - #393
Merged
Conversation
Result references were validated only with 'index < length', accepting negative and non-integer indices. A negative @ref such as -1 passed the check and resolved to undefined, silently substituting undefined where a validated value was expected. Add Number.isInteger(index) && index >= 0 bounds checks to both evaluateJsonProgram and createModuleTextFromProgram, and throw on an invalid reference in the interpreter instead of falling through to undefined. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens validation and runtime handling of result references ({ "@ref": n }) in the TypeChat program interpreter and module generator so invalid indices (negative/non-integer/out-of-range) no longer silently resolve to undefined.
Changes:
- Strengthen
@refbounds checks inevaluateJsonProgramandcreateModuleTextFromProgramto require integer, non-negative indices within bounds. - Make the interpreter throw an explicit
Invalid result referenceerror on invalid indices. - Add a new TypeScript test file for
@refbehavior and wire it into thetypescriptpackage test script.
Show a summary per file
| File | Description |
|---|---|
| typescript/src/ts/program.ts | Adds stricter @ref validation and throws on invalid references during interpretation. |
| typescript/test/program.test.ts | Introduces tests covering valid and invalid result references for both APIs. |
| typescript/package.json | Updates the test script to execute the new compiled test. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 3
- Review effort level: Low
robgruen
approved these changes
Jul 27, 2026
…coverage - Require Object.keys(obj).length === 1 in the interpreter's @ref path so it matches createModuleTextFromProgram and does not treat multi-key objects as references. - Use String(index) instead of JSON.stringify(index) to avoid throwing on non-JSON values (e.g. bigint) when formatting the error. - Rename the misleading 'forward reference' test and add an out-of-upper-bound test for createModuleTextFromProgram. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Tal Zaccai (TalZaccai)
added a commit
that referenced
this pull request
Jul 27, 2026
* Reject unsafe @func names in program validator and interpreter Add a shared isValidFunctionName predicate used by both createModuleTextFromProgram and evaluateJsonProgram so a program can never be validated by one path but dispatched by the other. - createModuleTextFromProgram now rejects @func values that aren't strict identifiers, closing a source-injection hole where comment tokens (/* ... */) in @func could comment out later steps so the type-checker-based validator never saw them. - evaluateJsonProgram now throws for @func values that aren't strict identifiers or that resolve to an Object.prototype member (constructor, __proto__, toString, valueOf, hasOwnProperty, etc.), instead of forwarding them to the host onCall dispatcher. @ref bounds checking is unrelated and already handled by #393. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Address review: fix JSDoc, mirror shape checks in interpreter, reindent tests - Reword isValidFunctionName JSDoc to describe the actual ASCII identifier constraint instead of implying full JS identifier support. - evaluateJsonProgram's @func branch now mirrors the generator's shape checks (args must be an array when present, no unexpected extra keys) and throws instead of silently returning undefined when they don't hold. - Re-indent the @func name validation test block to 4 spaces. - Add tests covering non-array @Args and extra keys for both createModuleTextFromProgram and evaluateJsonProgram. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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.
Problem
Result references (
{ "@ref": n }) were validated only withtypeof index === "number" && index < length. A negative index like-1passed the check, andresults[-1]silently resolved toundefined— substitutingundefinedwhere a validated value from a preceding step was expected.createModuleTextFromProgramhad the same gap.evaluateJsonProgramis a public export usable without validation, so this is reachable in practice.Fix
Number.isInteger(index) && index >= 0 && index < …bounds checks in bothevaluateJsonProgramandcreateModuleTextFromProgram.Invalid result referenceon a bad index instead of falling through to an implicitundefined— otherwise it would still silently yieldundefined.Tests
typescript/test/program.test.tscovering valid, negative, non-integer, and out-of-upper-bound references for both functions, and wired it into thetestscript.