Fixes #121.
What changes
getPath (shared/core/schema/json/path.ts) validated the container type and then indexed current[key] directly. Plain property access walks the prototype chain, so:
getPath({}, "$.constructor"); // Object [Function: Object] on main
getPath({}, "$.toString"); // function toString() { ... } on mainNumeric traversal had the same gap: an array index set on Object.prototype (or Array.prototype) resolves through the chain and surfaces as payload data, violating the Json return contract that utils.json.get / optionalGet expose.
Both traversal steps now require Object.hasOwn before indexing, so a path absent from the payload stays absent. Own members and own elements read exactly as before.
Verification
No deno on this host, so I ran the module itself under Node 26 with type stripping and a two-method zod stub (z.string().regex, .refine are the only calls the module makes at import time), comparing the vendored copy of main and the patched copy:
| case | main | patched |
|---|---|---|
getPath({}, "$.constructor") | inherited Object function | undefined |
getPath({}, "$.toString") | inherited toString function | undefined |
getPath({ nested: {} }, "$.nested.constructor") | inherited function | undefined |
getPath({ a: [{}] }, "$.a[0].hasOwnProperty") | inherited function | undefined |
Object.prototype[0] = "inherited"; getPath({ a: [] }, "$.a[0]") | "inherited" | undefined |
same pollution; getPath({ a: ["own"] }, "$.a[0]") | "own" | "own" |
getPath({ cost: { total: 5 } }, "$.cost.total") | 5 | 5 |
getPath({}, "$.missing.deep") | undefined | undefined |
getPath({}, "$..recursive") / "$.results[*]" | undefined (pattern guard) | undefined |
The two new Deno.test cases in shared/core/core.test.ts mirror the first five rows (the prototype pollution is scoped and restored in a finally); I could not run deno test locally, so CI is the authoritative run for them.
Summary by CodeRabbit
- Bug Fixes
- Path lookups now return no value for properties inherited from an object’s prototype, including inherited array indexes. Values directly present on objects and arrays continue to be returned as expected. This makes lookup results more predictable when navigating nested data and prevents inherited values from being mistaken for values explicitly provided in that data.
No actionable comments were generated in the recent review.
115135ad-e882-4e92-847d-b3174ea2676eReviewing files that changed from the base of the PR and between 09d54479142e36eff876383beacfbd37b33b5908 and 3e0ddf60190f35ac8ff98db4c672210dc1aaa4ac.
shared/core/core.test.tsshared/core/schema/json/path.tsIncluded review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
Walkthrough
getPathnow returnsundefinedfor inherited object members and array indexes. New tests verify that inherited values are ignored and own values remain accessible.Priority: Low
Severity of issue fixed: Low
Merge Risk: Minimal · up to
3e0ddNo merge-blocking issue is evident, so this change is mergeable pending normal checks.
Pre-merge checks: 4
#121requires that getPath return undefined for inherited object members and array indices, and that traversal use only own members and own elements. The whole-PR diff (mergeBase 09d5447 to head…#121. The test h…Full details: Linked Issues check
Explanation
Issue
#121requires that getPath return undefined for inherited object members and array indices, and that traversal use only own members and own elements. The whole-PR diff (mergeBase 09d5447 to head 3e0ddf6) in shared/core/schema/json/path.ts adds Object.hasOwn checks at both traversal steps. The object-member branch checks Object.hasOwn(current, key) before reading current[key]. The array-index branch checks Object.hasOwn(current, index) before reading current[index]. Both branches return undefined when the check fails. Own reads are unchanged. Path validation via PATH_PATTERN still runs first, and the existing container-type checks remain. Regression coverage for#121is in shared/core/core.test.ts. The first test asserts getPath({}, "$.constructor") and getPath({}, "$.toString") return undefined, plus nested and array-element inherited members. The second test sets Object.prototype[0] and asserts getPath({ a: [] }, "$.a[0]") returns undefined, with the prototype restored in a finally block. Own-value assertions are included. The author states the tests were not run locally. The code evidence supports the behavior, but CI has not confirmed the test run.Full details: Out of Scope Changes check
Explanation
The whole-PR diff touches only shared/core/schema/json/path.ts and shared/core/core.test.ts. Every hunk in path.ts changes the ownership check in getPath, which is the defect named in
#121. The test hunks add regression coverage for the same defect. No unrelated behavior, refactoring, or API change appears in the diff. The changed comments describe the ownership rule and do not add unrelated scope.Comment
@coderabbitai helpto get the list of available commands.