Skip to content

fix: sanitize AbortSignal in V3 plugin callback args before postMessage#1273

Open
ruyari-cupcake wants to merge 1 commit intokwaroran:mainfrom
ruyari-cupcake:fix/v3-abort-signal-serialization
Open

fix: sanitize AbortSignal in V3 plugin callback args before postMessage#1273
ruyari-cupcake wants to merge 1 commit intokwaroran:mainfrom
ruyari-cupcake:fix/v3-abort-signal-serialization

Conversation

@ruyari-cupcake
Copy link
Copy Markdown

Problem

When a V3 plugin registers a provider via addProvider, the callback is never invoked. Instead, the user sees:

Plugin Error from MyProvider: {}

Root Cause

In factory.ts, the deserializeArgs callback proxy passes innerArgs directly to postMessage. When requestPlugin (in request.ts) calls the provider function as v2Function(args, abortSignal), the AbortSignal object is included in innerArgs.

AbortSignal cannot be structured-cloned (per the HTML spec), so postMessage throws a DataCloneError. Since DOMException has non-enumerable properties, JSON.stringify(error) produces {}, resulting in the unhelpful error message.

Fix

Filter AbortSignal to null in the callback args before postMessage:

ypescript const sanitizedArgs = innerArgs.map(arg => arg instanceof AbortSignal ? null : arg );

This is the minimal change needed. The plugin still receives the full request arguments (prompt_chat, mode, max_tokens, etc.) — only the non-cloneable AbortSignal is replaced with null.

Impact

  • All V3 plugins using addProvider are currently broken by this bug
  • This fix enables V3 plugin providers to work correctly
  • No security implications: AbortSignal is a cancellation signal, not sensitive data
  • Plugins lose abort/cancel functionality, but gain the ability to function at all

AbortSignal cannot be structured-cloned, causing DataCloneError when
the V3 bridge tries to invoke addProvider callbacks via postMessage.

This filters AbortSignal to null in the INVOKE_CALLBACK message args,
allowing plugin provider callbacks to be invoked successfully.
kwaroran added a commit that referenced this pull request Feb 20, 2026
… callbacks (#1276)

## PR Checklist

- Required Checks
    - [x] Have you added type definitions?
    - [x] Have you tested your changes?
    - [x] Have you checked that it won't break any existing features?
- [ ] If your PR uses models[^1], if true, check the following:
    - [ ] Have you checked if it works normally in all models?
- [ ] Have you checked if it works normally in all web, local, and node
hosted versions? If it doesn't, have you blocked it in those versions?
- [x] If your PR is highly ai generated[^2], check the following:
    - [x] Have you understanded what the code does?
    - [x] Have you cleaned up any unnecessary or redundant code?
    - [x] Is is not a huge change?
- We currently do not accept highly ai generated PRs that are large
changes.


[^1]: Modifies the behavior of prompting, requesting or handling
responses from ai models.
[^2]: Almost over 80% of the code is ai generated.

## Summary

Follow-up to #1273. Instead of discarding `AbortSignal` (replacing with
`null`), this PR proxies it across the iframe boundary using an ID-based
message protocol, restoring abort/cancel functionality for V3 plugin
providers.

## Related Issues

- PR #1273 — Initial fix that sanitized `AbortSignal` to `null` to
prevent `DataCloneError`. This PR builds on that foundation.

## Changes

`AbortSignal` is not structured-cloneable (HTML spec), so it cannot be
passed via `postMessage`. PR #1273 solved the crash by replacing it with
`null`, but this meant plugins lost all abort/cancel functionality.

This PR introduces a lightweight proxy protocol within factory.ts
(single file, ~46 lines added):

**Host side** (`SandboxHost.deserializeArgs`):
- `AbortSignal` → serializable `AbortSignalRef` object `{ __type:
'ABORT_SIGNAL_REF', abortId, aborted }`
- Attaches a one-shot `abort` event listener on the real signal that
sends an `ABORT_SIGNAL` message to the guest iframe when triggered

**Guest side** (`GUEST_BRIDGE_SCRIPT`):
- New `abortControllers` registry (Map)
- `INVOKE_CALLBACK` handler deserializes `AbortSignalRef` → local
`AbortController`, stores in registry, returns `controller.signal` to
the plugin callback
- If `aborted: true` in ref, immediately calls `controller.abort()`
- New `ABORT_SIGNAL` message handler: looks up controller by `abortId`,
calls `.abort()`, removes from registry

**Type additions**:
- `'ABORT_SIGNAL'` added to `MsgType` union
- `AbortSignalRef` interface added

### Data flow

```
Host (real AbortSignal)          Guest (reconstructed AbortSignal)
───────────────────────          ────────────────────────────────
AbortSignal                      AbortController + .signal
    │                                 │
    │  INVOKE_CALLBACK                │
    ├──→ { __type: 'ABORT_SIGNAL_REF', ──→ new AbortController()
    │      abortId: 'abc',               abortControllers.set('abc', ctrl)
    │      aborted: false }              return ctrl.signal to plugin
    │                                 │
    │  (user clicks Stop)             │
    │  abort event fires              │
    ├──→ postMessage({                │
    │      type: 'ABORT_SIGNAL', ────→ ctrl.abort()
    │      abortId: 'abc'              abortControllers.delete('abc')
    │    })                           │
```

## Impact

**Backward compatible** — no breaking changes:

- Plugins that **ignore** `abortSignal` (pass it as unused param or
check for null): no behavior change. They received `null` before, now
they receive a real `AbortSignal` that simply never fires if the host
doesn't abort.
- Plugins that **use** `abortSignal` (e.g., `signal: abortSignal` in
fetch, or `.addEventListener('abort', ...)`): these now work correctly,
whereas before they had no abort capability.
- The `ABORT_SIGNAL` message type is ignored by older guest bridge code
that doesn't recognize it, so forward compatibility is maintained.
- No sandbox flags are modified (`allow-scripts allow-modals
allow-downloads` only).
- No host objects or references leak to the guest — only a plain `{
__type, abortId, aborted }` object crosses the boundary.

## Additional Notes

- This change is scoped entirely to factory.ts — no other files are
modified.
- The protocol is unidirectional (host→guest only). Guest-to-host
AbortSignal proxying (e.g., plugin passing `signal` in `nativeFetch`
options) is not covered and would require a separate change. (It will
probably be resolved by #1274)
- The `aborted` field in `AbortSignalRef` handles the edge case where
the signal is already aborted at serialization time.

---
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant