#207

fix(core): treat inherited members as absent in JSONPath lookup

Openrahul05ranjanwants to merge intofrom

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 main

Numeric 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:

casemainpatched
getPath({}, "$.constructor")inherited Object functionundefined
getPath({}, "$.toString")inherited toString functionundefined
getPath({ nested: {} }, "$.nested.constructor")inherited functionundefined
getPath({ a: [{}] }, "$.a[0].hasOwnProperty")inherited functionundefined
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")55
getPath({}, "$.missing.deep")undefinedundefined
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.

Activity

2
coderabbitai[bot]4d ago

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 09d54479142e36eff876383beacfbd37b33b5908 and 3e0ddf60190f35ac8ff98db4c672210dc1aaa4ac.

Files selected for processing: 2
  • shared/core/core.test.ts
  • shared/core/schema/json/path.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Walkthrough

getPath now returns undefined for 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 3e0dd

No merge-blocking issue is evident, so this change is mergeable pending normal checks.

Pre-merge checks: 4
Check nameStatusExplanation
Description CheckCheck skipped - CodeRabbit’s high-level summary is enabled.
Title checkThe title clearly and concisely describes the main change: inherited members are treated as absent during JSONPath lookup.
Linked Issues checkIssue #121 requires 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…
Out of Scope Changes checkThe 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 h…
Full details: Linked Issues check

Explanation

Issue #121 requires 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 #121 is 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.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.