fix(inspect): handle Proxy trap exceptions when walking prototype chain

[robobun]

What does this PR do?

Fixes a null pointer dereference in JSC__JSValue__forEachPropertyImpl (used by Bun.inspect / console.log formatting) when the object being inspected has a Proxy in its prototype chain.

Root cause

In the slow-path property enumeration loop:

1. object->getPropertySlot(globalObject, property, slot) can throw (e.g. a Proxy forwards [[Get]] to a native getter that throws). When it throws, JSC returns false from getPropertySlot.
2. The code did if (!getPropertySlot(...)) continue; — skipping the CLEAR_IF_EXCEPTION that came after the continue, so the exception stayed pending.
3. After the property loop, iterating->getPrototype(globalObject) is called. With an exception pending (or with a Proxy getPrototypeOf trap that throws), this returns an empty JSValue.
4. .getObject() on an empty JSValue calls asCell()->getObject() on a null pointer.

Fix

• Clear the exception from getPropertySlot before the continue (matching what forEachPropertyOrdered already does).
• Clear exceptions from getPrototype and null-check the result before calling .getObject(), both in the slow-path iteration and in the initial fast-path prototype lookup.

How did you verify your code works?

Minimal repro that crashed before and now works:

const proto = Object.create(Object.prototype, {
  foo: { get: () => 1, enumerable: true, configurable: true },
  bar: { get() { throw new Error("boom"); }, enumerable: true, configurable: true },
  baz: { get: () => 2, enumerable: true, configurable: true },
});
const obj = Object.create(new Proxy(proto, {}));
Bun.inspect(obj); // used to SIGABRT (UBSAN null deref), now prints { foo: 1, baz: 2 }

Added regression tests to test/js/bun/util/inspect.test.js covering both the throwing-getter case and the throwing-getPrototypeOf case. Both tests crash the old binary with the same UBSAN report and pass on this branch.

Found by Fuzzilli (fingerprint 6913eddd71ce20aa).

[robobun]

^{Updated 8:28 AM PT - May 4th, 2026}

❌ @robobun, your commit c4eb9e3 has some failures in Build #51237 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 30099

That installs a local version of the PR into your bun-30099 executable, so you can run:

bun-30099 --bun

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4743ada2-dbf4-4ed4-a263-a6c352c0e431

📥 Commits

Reviewing files that changed from the base of the PR and between b331b36 and a1425c5.

📒 Files selected for processing (2)

• src/bun.js/bindings/bindings.cpp
• test/js/bun/util/inspect.test.js

---

Walkthrough

Property enumeration in Bun.inspect() now explicitly suppresses exceptions arising from Proxy traps during prototype-chain traversal and property lookup. Test coverage is extended with fixtures that trigger these exception paths.

Changes

Exception Handling in Property Enumeration

Layer / File(s)	Summary
Core Logic src/bun.js/bindings/bindings.cpp	JSC__JSValue__forEachPropertyImpl clears exceptions after getPrototypeOf calls during prototype traversal, captures property slot lookup results and clears resulting exceptions, and skips properties when lookups fail.
Test Coverage test/js/bun/util/inspect.test.js	Two new proxy fixture cases added: one with a throwing getter on a prototype-chain object, and one with a throwing getPrototypeOf proxy trap, to verify Bun.inspect(await input()) handles such exceptions without crashing.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: fixing exception handling for Proxy traps during prototype chain traversal in the inspect function.
Description check	✅ Passed	The PR description thoroughly covers both required sections with clear explanations of the root cause, the fix applied, and detailed verification steps including minimal repro and regression tests.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Review rate limit: 4/5 reviews remaining, refill in 12 minutes.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

This PR may be a duplicate of:

1. fix(inspect): don't crash when a Proxy in the prototype chain throws #29642 - Adds CLEAR_IF_EXCEPTION after getPrototype and reorders getPropertySlot/CLEAR_IF_EXCEPTION in forEachPropertyImpl — nearly identical fix
2. Fix null JSCell deref in Bun lazy property callbacks and forEachProperty #29671 - Adds CLEAR_IF_EXCEPTION after getPrototype in forEachPropertyImpl plus broader lazy property callback fixes
3. inspect: handle throwing Proxy getPrototypeOf in forEachProperty #29814 - Adds CLEAR_IF_EXCEPTION after getPrototype and reorders getPropertySlot/CLEAR_IF_EXCEPTION in forEachPropertyImpl — virtually identical
4. Fix null deref in forEachProperty with Proxy in prototype chain #29816 - Same CLEAR_IF_EXCEPTION + null check pattern after getPrototype and same getPropertySlot reorder in forEachPropertyImpl
5. inspect: clear exceptions when walking Proxy prototype chain #29845 - Same CLEAR_IF_EXCEPTION + null check after getPrototype and same getPropertySlot reorder in forEachPropertyImpl — nearly identical

🤖 Generated with Claude Code

[claude]

🟣 Note: the same getPrototype(globalObject).getObject() null-deref pattern still exists at src/bun.js/bindings/napi.cpp:1837, reachable via napi_get_all_property_names with napi_key_include_prototypes + an enumerable/writable/configurable filter on an object with a throwing Proxy in its prototype chain. This is pre-existing (the PR doesn't touch napi.cpp), but it's the exact same root cause and might be worth applying the same CLEAR_IF_EXCEPTION + null-check there as a follow-up.

Extended reasoning...

What the bug is

This PR correctly fixes the null dereference in JSC__JSValue__forEachPropertyImpl by splitting iterating->getPrototype(globalObject).getObject() into two steps, clearing any pending exception, and null-checking the returned JSValue before calling .getObject(). However, the identical unsafe pattern still exists in src/bun.js/bindings/napi.cpp:

// src/bun.js/bindings/napi.cpp:1833-1842
if (key_mode == napi_key_include_prototypes) {
    // Climb up the prototype chain to find inherited properties
    JSObject* current_object = object;
    while (!current_object->getOwnPropertyDescriptor(globalObject, key.toPropertyKey(globalObject), desc)) {
        JSObject* proto = current_object->getPrototype(globalObject).getObject();  // <-- same pattern
        if (!proto) {
            break;
        }
        current_object = proto;
    }
}

How it manifests

There are two ways for getPrototype() to return an empty JSValue here, both via Proxy traps:

1. Throwing getOwnPropertyDescriptor trap: The while-loop condition calls current_object->getOwnPropertyDescriptor(...). If current_object is a Proxy whose getOwnPropertyDescriptor trap throws, JSC returns false and leaves the exception pending. The loop body then calls current_object->getPrototype(globalObject) with a pending exception, which returns an empty JSValue.
2. Throwing getPrototypeOf trap: If the loop condition returns false cleanly but current_object is a Proxy whose getPrototypeOf trap throws, getPrototype() itself throws and returns an empty JSValue.

In both cases, .getObject() is then called on an empty JSValue. An empty JSValue is encoded as 0, which has no NotCellMask bits set, so isCell() returns true, asCell() returns nullptr, and nullptr->getObject() reads m_type from a null pointer — the same UBSAN null deref this PR fixes in bindings.cpp. The if (!proto) break; on the next line doesn't help because the dereference has already happened inside .getObject().

Why existing code doesn't prevent it

The NAPI_RETURN_IF_EXCEPTION(env) at line 1823 only guards the initial key collection (allPropertyKeys / ownPropertyKeys). The per-key descriptor loop at lines 1829-1845 has no RETURN_IF_EXCEPTION, CLEAR_IF_EXCEPTION, or empty-value check around getOwnPropertyDescriptor or getPrototype, so exceptions thrown by Proxy traps during the prototype walk are neither observed nor cleared.

Step-by-step proof

1. A native addon calls napi_get_all_property_names(env, obj, napi_key_include_prototypes, napi_key_enumerable, napi_key_numbers_to_strings, &result) where obj has new Proxy(proto, { getPrototypeOf() { throw new Error('boom'); } }) somewhere in its prototype chain.
2. allPropertyKeys collects keys from the chain (this may or may not throw depending on trap order; assume it succeeds and returns at least one key not present as an own property of obj).
3. Because key_filter & filter_by_any_descriptor is truthy, we enter the filter loop. For a key not owned by obj, getOwnPropertyDescriptor returns false and we enter the while-loop body.
4. object->getPrototype(globalObject) walks to the Proxy and invokes its getPrototypeOf trap, which throws. getPrototype returns an empty JSValue.
5. .getObject() → isCell() on encoded 0 → true → asCell() → nullptr → nullptr->getObject() reads JSCell::m_type at offset from null → UBSAN null deref / crash.

(The throwing-getOwnPropertyDescriptor variant is even simpler: trap throws → returns false → loop body runs with pending exception → getPrototype returns empty → same crash.)

Impact and fix

This is reachable from any native addon that calls napi_get_all_property_names with napi_key_include_prototypes and an enumerable/writable/configurable filter on user-supplied objects. The fix is the same as what this PR applies in bindings.cpp:

JSValue protoValue = current_object->getPrototype(globalObject);
CLEAR_IF_EXCEPTION(scope);  // or RETURN_IF_EXCEPTION if NAPI should propagate
JSObject* proto = protoValue ? protoValue.getObject() : nullptr;
if (!proto) break;

plus an exception check after getOwnPropertyDescriptor in the loop condition.

Relationship to this PR

This PR does not touch napi.cpp, add callers to it, or otherwise interact with this code path — so this is strictly a pre-existing issue and should not block merging. Mentioning it only because it is the exact same root cause fixed here, found by the same fuzzer pattern, and the author may want to apply the same fix in a follow-up.

[robobun]

CI failure on darwin-14-x64-test-bun is test/cli/run/no-orphans.test.ts timing out (pre-existing macOS flake, fix in #30100). test/js/bun/util/inspect.test.js passed on all platforms including that one (74 pass / 0 fail).

[claude]

LGTM — small, defensive fix with regression tests; the napi.cpp note from my earlier review is a pre-existing issue and not a blocker.

Extended reasoning...

Overview

This PR touches two files: src/jsc/bindings/bindings.cpp (~10 net lines changed inside JSC__JSValue__forEachPropertyImpl) and test/js/bun/util/inspect.test.js (two new crash-test fixtures). The C++ change does three things: (1) clears exceptions after the initial fast-path getPrototype call, (2) reorders the slow-path getPropertySlot so CLEAR_IF_EXCEPTION runs before the continue instead of after, and (3) splits the slow-path iterating->getPrototype(globalObject).getObject() into separate steps with an exception clear and null-check. All three follow the existing CLEAR_IF_EXCEPTION(scope) pattern already used throughout this function.

Security risks

None introduced. This is strictly a hardening change — it converts a fuzzer-found null-pointer dereference (UBSAN abort) in Bun.inspect/console.log into graceful handling. Exceptions are intentionally swallowed here because forEachProperty is used for debugging/formatting output, where best-effort enumeration is the established behavior (the function already clears getter exceptions in several other places).

Level of scrutiny

Low-to-moderate. The change is small, mechanical, and confined to a single function whose purpose is best-effort property enumeration for inspection. The root-cause analysis in the PR description is precise and the fix matches it exactly. I verified against the current source that scope is a DECLARE_TOP_EXCEPTION_SCOPE in scope at all three insertion points, and that the null-check on proto correctly guards .getObject() (an empty JSValue is falsy, so the ternary short-circuits before the dereference).

Other factors

• Regression tests added to the existing "crash testing" fixture array in inspect.test.js, covering both the throwing-getter-via-Proxy case and the throwing-getPrototypeOf-trap case. CI confirms inspect.test.js passed 74/74 on all platforms; the only CI failure is an unrelated pre-existing macOS flake (no-orphans.test.ts).
• My earlier inline note about napi.cpp:1837 is explicitly a pre-existing issue in code this PR doesn't touch — it's a follow-up suggestion, not a blocker.
• The duplicate-PR bot flagged five near-identical PRs; that's a dedup/workflow concern for maintainers and doesn't affect the correctness of this change.

[claude]

LGTM — targeted crash fix with regression tests; the napi.cpp note from my earlier comment is pre-existing and non-blocking.

Extended reasoning...

Overview

This PR fixes a null-pointer dereference in JSC__JSValue__forEachPropertyImpl (the C++ backing for Bun.inspect/console.log property enumeration) when a Proxy in the prototype chain throws from a trap. Three small changes in src/jsc/bindings/bindings.cpp: (1) clear exceptions after the fast-path getPrototype call, (2) reorder CLEAR_IF_EXCEPTION before the continue in the slow-path property loop so a throwing getPropertySlot doesn't leave a pending exception, and (3) split getPrototype(...).getObject() into a null-checked two-step in the slow-path prototype walk. Two regression fixtures are added to the existing "crash testing" block in test/js/bun/util/inspect.test.js.

Security risks

None. The change is purely defensive — it adds exception clearing and a null check on a code path that previously crashed. It does not introduce new data flows, parsing, or trust boundaries; if anything it removes a user-triggerable abort.

Level of scrutiny

Moderate. This touches JSC bindings C++, but the change is mechanical and follows the established CLEAR_IF_EXCEPTION(scope) pattern already used throughout this function (and matches what the sibling forEachPropertyOrdered already does for getPropertySlot). The diff is ~10 lines of logic, strictly narrowing behavior from "crash" to "skip and continue", with no change to the non-exceptional path.

Other factors

• The new test fixtures exercise both failure modes (throwing getter via Proxy forwarding, and throwing getPrototypeOf trap) and pass on all CI platforms; the only CI failure is an unrelated pre-existing macOS flake in no-orphans.test.ts.
• I previously left a 🟣 note that the same getPrototype(...).getObject() pattern exists in napi.cpp — that is pre-existing, out of scope for this PR, and explicitly non-blocking.
• The duplicate-PR bot flagged several near-identical fixes; that's a triage/process matter and doesn't affect the correctness of this change.
• No new commits with code changes since my prior review (only CI retriggers).

[robobun]

Build #51237: all 16 failures are Expired (Buildkite queue capacity — jobs timed out waiting for agents after the src/ restructure invalidated caches). No build or test failures; the test jobs that ran all passed. Needs a retry once the queue clears.