diff --git a/src/config/config-paths.test.ts b/src/config/config-paths.test.ts new file mode 100644 index 000000000000..4947a20b16f5 --- /dev/null +++ b/src/config/config-paths.test.ts @@ -0,0 +1,91 @@ +import { describe, expect, it, vi } from "vitest"; +import { + getConfigValueAtPath, + setConfigValueAtPath, + unsetConfigValueAtPath, +} from "./config-paths.js"; + +describe("config path own-property traversal", () => { + for (const key of ["toString", "valueOf", "hasOwnProperty"]) { + it(`does not treat inherited ${key} as config`, () => { + const parent: Record = {}; + const root: Record = { parent }; + + expect(getConfigValueAtPath(root, ["parent", key])).toBeUndefined(); + expect(unsetConfigValueAtPath(root, ["parent", key])).toBe(false); + expect(root).toEqual({ parent: {} }); + + setConfigValueAtPath(root, ["parent", key], "own"); + expect(Object.hasOwn(parent, key)).toBe(true); + expect(getConfigValueAtPath(root, ["parent", key])).toBe("own"); + expect(unsetConfigValueAtPath(root, ["parent", key])).toBe(true); + expect(root).toEqual({}); + }); + } + + it("replaces an inherited parent instead of traversing it", () => { + const prototypeBranch = { leaf: "prototype" }; + const root = Object.create({ branch: prototypeBranch }) as Record; + + expect(getConfigValueAtPath(root, ["branch", "leaf"])).toBeUndefined(); + expect(unsetConfigValueAtPath(root, ["branch", "leaf"])).toBe(false); + expect(prototypeBranch).toEqual({ leaf: "prototype" }); + + setConfigValueAtPath(root, ["branch", "leaf"], "own"); + expect(Object.hasOwn(root, "branch")).toBe(true); + expect(root.branch).toEqual({ leaf: "own" }); + expect(prototypeBranch).toEqual({ leaf: "prototype" }); + + expect(unsetConfigValueAtPath(root, ["branch", "leaf"])).toBe(true); + expect(Object.hasOwn(root, "branch")).toBe(false); + expect(getConfigValueAtPath(root, ["branch", "leaf"])).toBeUndefined(); + expect(prototypeBranch).toEqual({ leaf: "prototype" }); + }); + + for (const inheritedKind of ["setter", "non-writable"] as const) { + it(`creates an own parent over an inherited ${inheritedKind} property`, () => { + const setter = vi.fn(); + const prototype = {}; + Object.defineProperty( + prototype, + "branch", + inheritedKind === "setter" + ? { configurable: true, set: setter } + : { configurable: true, value: { leaf: "prototype" }, writable: false }, + ); + const root = Object.create(prototype) as Record; + + expect(() => setConfigValueAtPath(root, ["branch", "leaf"], "own")).not.toThrow(); + expect(setter).not.toHaveBeenCalled(); + expect(Object.getOwnPropertyDescriptor(root, "branch")).toMatchObject({ + configurable: true, + enumerable: true, + value: { leaf: "own" }, + writable: true, + }); + }); + + it(`creates an own leaf over an inherited ${inheritedKind} property`, () => { + const setter = vi.fn(); + const prototype = {}; + Object.defineProperty( + prototype, + "leaf", + inheritedKind === "setter" + ? { configurable: true, set: setter } + : { configurable: true, value: "prototype", writable: false }, + ); + const parent = Object.create(prototype) as Record; + const root = { parent }; + + expect(() => setConfigValueAtPath(root, ["parent", "leaf"], "own")).not.toThrow(); + expect(setter).not.toHaveBeenCalled(); + expect(Object.getOwnPropertyDescriptor(parent, "leaf")).toMatchObject({ + configurable: true, + enumerable: true, + value: "own", + writable: true, + }); + }); + } +}); diff --git a/src/config/config-paths.ts b/src/config/config-paths.ts index dee4feda26e0..6037dde34bd8 100644 --- a/src/config/config-paths.ts +++ b/src/config/config-paths.ts @@ -1,9 +1,22 @@ +import { isBlockedObjectKey } from "../infra/prototype-keys.js"; // Resolves and classifies config paths for reads, writes, and metadata. import { isPlainObject } from "../utils.js"; -import { isBlockedObjectKey } from "../infra/prototype-keys.js"; type PathNode = Record; +function setOwnConfigProperty(node: PathNode, key: string, value: unknown): void { + if (Object.hasOwn(node, key)) { + node[key] = value; + return; + } + Object.defineProperty(node, key, { + configurable: true, + enumerable: true, + value, + writable: true, + }); +} + /** Parses CLI/config dot-notation paths and rejects unsafe object-key segments. */ export function parseConfigPath(raw: string): { ok: boolean; @@ -37,13 +50,14 @@ export function setConfigValueAtPath(root: PathNode, path: string[], value: unkn let cursor: PathNode = root; for (let idx = 0; idx < path.length - 1; idx += 1) { const key = path[idx]; - const next = cursor[key]; - if (!isPlainObject(next)) { - cursor[key] = {}; + const existing = Object.hasOwn(cursor, key) ? cursor[key] : undefined; + const next: PathNode = isPlainObject(existing) ? existing : {}; + if (next !== existing) { + setOwnConfigProperty(cursor, key, next); } - cursor = cursor[key] as PathNode; + cursor = next; } - cursor[path[path.length - 1]] = value; + setOwnConfigProperty(cursor, path[path.length - 1], value); } /** Removes a value at a config path and prunes empty parent objects created by setters. */ @@ -52,6 +66,9 @@ export function unsetConfigValueAtPath(root: PathNode, path: string[]): boolean let cursor: PathNode = root; for (let idx = 0; idx < path.length - 1; idx += 1) { const key = path[idx]; + if (!Object.hasOwn(cursor, key)) { + return false; + } const next = cursor[key]; if (!isPlainObject(next)) { return false; @@ -60,7 +77,7 @@ export function unsetConfigValueAtPath(root: PathNode, path: string[]): boolean cursor = next; } const leafKey = path[path.length - 1]; - if (!(leafKey in cursor)) { + if (!Object.hasOwn(cursor, leafKey)) { return false; } delete cursor[leafKey]; @@ -82,7 +99,7 @@ export function unsetConfigValueAtPath(root: PathNode, path: string[]): boolean export function getConfigValueAtPath(root: PathNode, path: string[]): unknown { let cursor: unknown = root; for (const key of path) { - if (!isPlainObject(cursor)) { + if (!isPlainObject(cursor) || !Object.hasOwn(cursor, key)) { return undefined; } cursor = cursor[key];