No actionable comments were generated in the recent review. 🎉

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 1a5b5d4d-18c1-46b5-bd6c-221bd023583e

Reviewing files that changed from the base of the PR and between c4df088 and 8e35998.

• NativeScript/runtime/ArgConverter.h
• NativeScript/runtime/ArgConverter.mm
• NativeScript/runtime/ClassBuilder.mm
• NativeScript/runtime/MetadataBuilder.mm
• TestRunner/app/tests/ProxyReceiverTests.js

• NativeScript/runtime/ArgConverter.mm
• NativeScript/runtime/MetadataBuilder.mm
• TestRunner/app/tests/ProxyReceiverTests.js

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

Walkthrough

The runtime unwraps proxies during native value conversion, receiver dispatch, wrapper lookup, and native counterpart release. It adds fallback context lookup for objects and callbacks. Tests cover proxied native receivers and arguments, revoked proxies, invalid targets, callback results, and proxy traps.

Changes

Proxy Runtime Handling

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant JavaScriptCaller
  participant MethodCallback
  participant ResolveProxyReceiver
  participant NativeMethod
  JavaScriptCaller->>MethodCallback: invoke method through proxy
  MethodCallback->>ResolveProxyReceiver: resolve receiver
  ResolveProxyReceiver-->>MethodCallback: return target or throw TypeError
  MethodCallback->>NativeMethod: invoke with resolved receiver

Suggested reviewers: nathanwalker

Merge Risk: ⚪ Minimal · up to 8e359

The reviewed changes have no established behavior that blocks merging; normal project checks remain appropriate.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to c4df0

Native object validation remains intact, but the new revoked-proxy recovery behavior can leave large callback results incompletely initialized. This creates a data-integrity and possible stale-data exposure risk within the application process. Some callback exception paths remain insufficiently established.

Retained concerns

• Medium · security · inferred: The new revoked-callback-result recovery state does not preserve complete aggregate return initialization. A successful callback returning a revoked Proxy becomes an empty value, and SetValue clears only one ffi_arg before returning. For a larger declared struct, the remaining bytes are untouched, potentially propagating stale data into native code. The same clearing defect predates this PR for null and undefined; the changed contract extends it to revoked results rather than introducing a new privilege boundary.

Security Blast Radius

• inferred — The demonstrated boundary surface is JavaScript execution with access to native wrappers or registered native callbacks in the hosting application. Aggregate return corruption could affect native consumers within that process. No inspected evidence establishes additional tenant, service, credential, or environment authority.

Security Findings and Attack Paths

• inferred — A JavaScript callback supplying a revoked Proxy can complete successfully, be normalized to empty, and return after only one ABI word is cleared. A native caller expecting a larger struct can receive untouched trailing bytes. Stale-data disclosure is a potential consequence, not a demonstrated exploit; null and undefined already exposed the underlying clearing defect at the PR base.

Trust Boundaries and Controls

• inferred — Receiver unwrapping aliases an already-held native wrapper rather than granting native identity to an arbitrary target. Callback context fallback stays within the supplied isolate and its cache; runtime initialization and worker creation provide counterevidence to cross-worker authority transfer. Authority equivalence for additional embedding-created contexts remains unestablished.

Resilience and Maintainability Implications

• observed — Returned native block and function-pointer V8 callbacks invoke argument conversion without a local NativeScriptException catch. The new revoked-argument helper throws that exception type. Other conversion throws already existed at the base, so incomplete containment is not established as a wholly new security condition; complete outer recovery remains unresolved.

Hardening Proposals

• proposed — Make revoked and nil callback recovery type-aware and initialize the complete ABI return storage for aggregate types. Validate successful revoked-result recovery with a struct larger than ffi_arg, including repetition after a nonzero prior result.
• proposed — Establish a local exception-to-JavaScript boundary for returned native callables, covering revoked arguments and existing conversion errors before native invocation. Validate that rejection remains catchable and that neither native execution nor unsupported unwinding occurs.

❌ Failed checks (1 warning)

Explanation

Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 5 files. (3 skipped: 3 unsupported.)

• Fix all pre-merge checks with AI

• Commit to this branch
• Create a new PR

• Commit to this branch
• Create a new PR

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

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

• X
• Mastodon
• Reddit
• LinkedIn

A rabbit checks each proxy trail
Through nested paths, both broad and frail
The native call meets its true target
Revoked trails stop before they start
Null results leave the bytes all clear
The test suite hops to check each case

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