From dffd95ce7cd8ac183e84af45c7ac0f9796ea5301 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Mon, 31 Aug 2026 20:38:02 -0400 Subject: [PATCH] fix(codemode): reject Object.assign cycles (#46076) --- .../codemode/src/interpreter/references.ts | 24 ++- packages/codemode/src/stdlib/object.ts | 35 ++-- packages/codemode/test/stdlib.test.ts | 170 ++++++++++++++++++ 3 files changed, 205 insertions(+), 24 deletions(-) diff --git a/packages/codemode/src/interpreter/references.ts b/packages/codemode/src/interpreter/references.ts index bfc7663a7e3..120afc7f28f 100644 --- a/packages/codemode/src/interpreter/references.ts +++ b/packages/codemode/src/interpreter/references.ts @@ -1,5 +1,6 @@ import { type AstNode, + AsyncIteratorSymbol, CodeModeFunction, CodeModeGenerator, CoercionFunction, @@ -9,6 +10,7 @@ import { GeneratorMethodReference, InterpreterRuntimeError, IntrinsicReference, + IteratorSymbol, JsonMethodReference, PromiseCapabilityFunction, PromiseInstanceMethodReference, @@ -42,13 +44,12 @@ export const isRuntimeReference = (value: unknown): boolean => value instanceof SymbolNamespace || isCodeModeValue(value) -function* childValues(value: object): Generator { - if (Array.isArray(value)) { - const length = value.length - for (let index = 0; index < length; index++) yield value[index] - return +function* childValues(value: object): Generator { + for (const key of Reflect.ownKeys(value)) { + if (!Object.prototype.propertyIsEnumerable.call(value, key)) continue + if (typeof key === "symbol" && key !== AsyncIteratorSymbol && key !== IteratorSymbol) continue + yield Reflect.get(value, key) } - yield* Object.values(value) } export const containsRuntimeReference = (value: unknown): boolean => { @@ -90,9 +91,14 @@ export const containsOpaqueReference = (value: unknown): boolean => { } // Reject cycles before mutation so later boundary walks remain safe. -export const rejectCircularInsertion = (container: object, value: unknown, label: string, node: AstNode): void => { +export const rejectCircularInsertion = ( + container: object, + value: unknown, + label: string, + node: AstNode, + seen = new Set(), +): void => { const pending: Array> = [[value].values()] - const seen = new Set() while (pending.length > 0) { const next = pending.at(-1)!.next() if (next.done) { @@ -104,7 +110,7 @@ export const rejectCircularInsertion = (container: object, value: unknown, label throw new InterpreterRuntimeError(`${label} contains a circular value.`, node, "InvalidDataValue") if (current === null || typeof current !== "object" || isRuntimeReference(current) || seen.has(current)) continue seen.add(current) - pending.push(Array.isArray(current) ? current[Symbol.iterator]() : childValues(current)) + pending.push(childValues(current)) } } diff --git a/packages/codemode/src/stdlib/object.ts b/packages/codemode/src/stdlib/object.ts index 583a4b4a42b..e5f48db291b 100644 --- a/packages/codemode/src/stdlib/object.ts +++ b/packages/codemode/src/stdlib/object.ts @@ -1,12 +1,6 @@ import { Effect } from "effect" -import { - type AstNode, - AsyncIteratorSymbol, - InterpreterRuntimeError, - IteratorSymbol, - IteratorSymbols, -} from "../interpreter/model.js" -import { containsOpaqueReference } from "../interpreter/references.js" +import { type AstNode, AsyncIteratorSymbol, InterpreterRuntimeError, IteratorSymbol } from "../interpreter/model.js" +import { containsOpaqueReference, rejectCircularInsertion } from "../interpreter/references.js" import { isBlockedMember } from "../tool-runtime.js" import { isCodeModeValue, CodeModePromise } from "../values.js" import { boundedData, coerceToString } from "./value.js" @@ -37,10 +31,6 @@ export const invokeObjectMethod = (name: string, args: Array, node: Ast } return input as Record } - const guardedSet = (out: Record, key: string, item: unknown): void => { - if (isBlockedMember(key)) throw new InterpreterRuntimeError(`Property '${key}' is not available.`, node) - out[key] = item - } switch (name) { case "keys": return Object.keys(requireObject()) @@ -64,14 +54,29 @@ export const invokeObjectMethod = (name: string, args: Array, node: Ast throw new InterpreterRuntimeError("Object.assign expects a data object target.", node) } const out = target as Record + const seen = new Set() + const guardedSet = (key: PropertyKey, item: unknown): void => { + if (typeof key === "string" && isBlockedMember(key)) + throw new InterpreterRuntimeError(`Property '${key}' is not available.`, node) + rejectCircularInsertion(out, item, "Object.assign result", node, seen) + if (!Reflect.set(out, key, item)) + throw new InterpreterRuntimeError(`Object.assign could not assign property '${String(key)}'.`, node).as( + "TypeError", + ) + } for (const source of args.slice(1)) { if (source === null || source === undefined || isCodeModeValue(source)) continue if (typeof source !== "object" || Array.isArray(source)) { throw new InterpreterRuntimeError("Object.assign expects data objects.", node) } - for (const [key, item] of Object.entries(source)) guardedSet(out, key, item) - for (const symbol of IteratorSymbols) { - if (Object.hasOwn(source, symbol)) Reflect.set(out, symbol, Reflect.get(source, symbol)) + for (const key of Reflect.ownKeys(source)) { + if (typeof key === "string") { + if (Object.prototype.propertyIsEnumerable.call(source, key)) guardedSet(key, Reflect.get(source, key)) + continue + } + if (key !== AsyncIteratorSymbol && key !== IteratorSymbol) continue + if (!Object.prototype.propertyIsEnumerable.call(source, key)) continue + guardedSet(key, Reflect.get(source, key)) } } return out diff --git a/packages/codemode/test/stdlib.test.ts b/packages/codemode/test/stdlib.test.ts index e3a840fd0ff..5846743d615 100644 --- a/packages/codemode/test/stdlib.test.ts +++ b/packages/codemode/test/stdlib.test.ts @@ -17,6 +17,8 @@ import { describe, expect, test } from "bun:test" import { Effect, Schema } from "effect" import { CodeMode, Tool } from "../src/index.js" +import { AsyncIteratorSymbol, IteratorSymbol } from "../src/interpreter/model.js" +import { invokeObjectMethod } from "../src/stdlib/object.js" // Standard-library value types: Date, RegExp, Map, Set. Programs use them as ordinary JS; // intra-CodeMode checkpoints (Object.* helpers, spread, coercion inputs) preserve the live @@ -824,6 +826,174 @@ describe("stdlib integration", () => { expect(await value(`try { Object.assign(null, { a: 1 }); return false } catch { return true }`)).toBe(true) }) + test("Object.assign ignores non-enumerable supported symbols without reading them", () => { + const target = {} + const reads: Array = [] + const source = Object.defineProperty({}, IteratorSymbol, { + get() { + reads.push(true) + return target + }, + }) + expect(invokeObjectMethod("assign", [target, source], { type: "CallExpression" })).toBe(target) + expect(reads).toEqual([]) + expect(Object.hasOwn(target, IteratorSymbol)).toBe(false) + }) + + test("Object.assign ignores nested non-enumerable supported symbols during cycle checks", () => { + const target = {} + const reads: Array = [] + const nested = Object.defineProperty({}, IteratorSymbol, { + get() { + reads.push(true) + return target + }, + }) + expect(invokeObjectMethod("assign", [target, { nested }], { type: "CallExpression" })).toBe(target) + expect(reads).toEqual([]) + expect(target).toEqual({ nested }) + }) + + test("Object.assign rejects cycles through supported symbols on nested arrays", () => { + const target = {} + const nested = Object.defineProperty([], IteratorSymbol, { enumerable: true, value: target }) + expect(() => invokeObjectMethod("assign", [target, { nested }], { type: "CallExpression" })).toThrow( + "Object.assign result contains a circular value.", + ) + expect(Object.hasOwn(target, "nested")).toBe(false) + }) + + test("Object.assign cycle checks traverse sparse keys lazily", () => { + const target = {} + const reads: Array = [] + const nested = Object.defineProperties([], { + 4294967294: { enumerable: true, value: target }, + later: { + enumerable: true, + get() { + reads.push(true) + return null + }, + }, + }) + expect(() => invokeObjectMethod("assign", [target, { nested }], { type: "CallExpression" })).toThrow( + "Object.assign result contains a circular value.", + ) + expect(reads).toEqual([]) + }) + + test("Object.assign stops after a supported symbol write fails", () => { + const previous = () => ({ done: true }) + const target = Object.defineProperty({}, IteratorSymbol, { value: previous }) + const reads: Array = [] + const source = Object.defineProperties( + {}, + { + [IteratorSymbol]: { enumerable: true, value: () => ({ done: false }) }, + [AsyncIteratorSymbol]: { + enumerable: true, + get() { + reads.push(true) + return () => ({ done: true }) + }, + }, + }, + ) + expect(() => invokeObjectMethod("assign", [target, source], { type: "CallExpression" })).toThrow( + "Object.assign could not assign property", + ) + expect(Reflect.get(target, IteratorSymbol)).toBe(previous) + expect(reads).toEqual([]) + }) + + test("Object.assign rejects direct and nested cycles", async () => { + expect( + await value(` + const target = { kept: true } + try { Object.assign(target, { self: target }) } catch { return target } + return null + `), + ).toEqual({ kept: true }) + expect( + await value(` + const target = { kept: true } + const nested = { target } + try { Object.assign(target, { nested }) } catch { return target } + return null + `), + ).toEqual({ kept: true }) + expect( + await value(` + const target = {} + const source = {} + source[Symbol.iterator] = target + try { Object.assign(target, source) } catch { return Object.hasOwn(target, Symbol.iterator) } + return true + `), + ).toBe(false) + expect( + await value(` + const target = {} + const nested = {} + nested[Symbol.iterator] = target + try { Object.assign(target, { nested }) } catch { return Object.hasOwn(target, "nested") } + return true + `), + ).toBe(false) + }) + + test("Object.assign preserves mutations before a circular field", async () => { + expect( + await value(` + const target = {} + try { Object.assign(target, { before: 1, cycle: { target }, after: 2 }) } catch { return target } + return null + `), + ).toEqual({ before: 1 }) + expect( + await value(` + const target = {} + const marker = {} + const source = {} + source[Symbol.iterator] = marker + source[Symbol.asyncIterator] = target + try { Object.assign(target, source) } catch { + return [target[Symbol.iterator] === marker, Object.hasOwn(target, Symbol.asyncIterator)] + } + return null + `), + ).toEqual([true, false]) + }) + + test("Object.assign preserves target identity and acyclic shared aliases", async () => { + expect( + await value(` + const shared = { count: 1 } + const target = {} + const result = Object.assign(target, { left: shared, right: shared }) + result.left.count = 2 + return [result === target, result.left === shared, result.left === result.right, shared.count] + `), + ).toEqual([true, true, true, 2]) + }) + + test("Object.assign traverses shared aliases once", () => { + const reads: Array = [] + const shared = Object.defineProperty({}, "value", { + enumerable: true, + get() { + reads.push(true) + return 1 + }, + }) + const target = {} + expect(invokeObjectMethod("assign", [target, { left: shared, right: shared }], { type: "CallExpression" })).toBe( + target, + ) + expect(target).toEqual({ left: shared, right: shared }) + expect(reads).toEqual([true]) + }) + test("assignment resolves and reads its left side before evaluating the right side", async () => { expect(await value(`let x = 1; x += (x = 5); return x`)).toBe(6) expect(await value(`let i = 0; const values = [9]; values[i++] = i; return [values, i]`)).toEqual([[1], 1])