Skip to content

Intercept: the mandatory checkReceiver guard silently disables interceptors #117

Description

@velzie

ScramjetClient.Intercept takes an optional checkReceiver and, when it is absent, refuses the whole declaration if any prototype member carries IDL argument types:

// client.ts, Intercept()
if (!checkReceiver) {
  for (const key of Reflect_ownKeys(descs)) {
    if (isConstructorMember(desc.value)) continue;
    if (memberValidator(this.box, desc.value) ||
        memberValidator(this.box, desc.set, true)) {
      throw new Error(`Intercept(${classname}.${String(key)}) requires a native receiver check before IDL conversion`);

No call site in the repo has ever passed a second argument — 19 declarations on develop, 99 on feat/idl-cleanup, zero with a checkReceiver. The throw is caught by loadModules' dbg.error("failed to install scramjet module", err), and because it aborts the module function, every declaration after the offending one in the same file dies with it.

Impact on develop

10 of 19 declarations are refused at install:

module refused on also lost
dom/CookieStore.ts @Arguments on CookieStore.get() —
dom/cookie.ts @Type on set Document.cookie —
dom/history.ts @Arguments on History.pushState() —
dom/performance.ts @Arguments on PerformanceEntry.toJSON() the other 4 declarations
shared/opfs.ts @Arguments() on StorageManager.getDirectory() FileSystemHandle
shared/worker.ts @Arguments on Worklet.addModule() —

The runway failures on develop are a readout of exactly this: ckjar-* and cookies-* (cookie/CookieStore), location-history-pushstate-url-forms and regression-1b988b4-displaced-history-pushstate (history), platform-performance-observer and platform-resource-timing-own-entries (performance). None of them fail on 060552a4, which predates #110.

Why the guard exists, and why it over-fires

The intent is sound. WebIDL checks the receiver's brand before converting arguments, and conversion invokes page code (toString, valueOf, Symbol.iterator). Intercept runs validate(args) before the interceptor body, so a body that brand-checks itself is already too late — the page's toString has run before the TypeError a browser throws first. checkReceiver is the only hook ahead of validate.

The predicate, though, asks "does this member declare IDL arguments at all" rather than "can this member's conversion run page code". That splits three ways:

  1. @Arguments() with no types — spurious. compileIDLValidator(box, []) returns a closure that reads nothing, coerces nothing, and cannot invoke page code: required is 0 and the loop has no iterations. There is no conversion to order against the brand check. This is what kills StorageManager.getDirectory, PerformanceEntry.toJSON, Performance.getEntries, and (on the storage branch) IDBFactory.databases, CacheStorage.keys and Storage.clear — the majority of refusals.

  2. Real argument types — genuine, but a fingerprint. Storage.prototype.setItem.call({}, {toString(){ ping() }}, "v") runs ping() before throwing Illegal invocation; a browser throws first. Observable, worth fixing, but the call fails either way with the same error. It is a detection vector, not a privilege boundary.

  3. Promise-returning interfaces — genuine, but unsatisfiable. Cache, CacheStorage and CookieStore expose only promise-returning operations, which reject rather than throw on a bad receiver. Nothing can meet checkReceiver's "synchronously invoke … throwing for an invalid receiver" contract, so the only options are to drop the IDL declarations or not install.

Even where a sync check exists it is not mechanical. For Storage the receiver the page holds is a wrapper Proxy, and internal slots do not forward through a Proxy, so new client.native.Storage(r).length throws for the legitimate receiver — it has to go through that module's own unwrap table first.

The calibration is backwards

The guard's failure mode is refusing to install, and for the storage-shaped modules not installing means no origin scoping at all: cross-origin localStorage reads and writes, cross-origin IndexedDB enumeration, cross-origin cache-entry reads. It trades an ordering fingerprint for a real cross-origin data leak, silently.

There is also a name collision that makes this easy to miss: the eslint rule scramjet-core/intercept-brand-check and the runtime checkReceiver parameter are unrelated mechanisms. Disabling the lint rule — as dom/storage.ts does, with a comment arguing the in-body checks suffice — does nothing for the runtime guard.

Suggested fix

  • Narrow the predicate to parameters whose conversion can actually invoke page code. An empty list cannot; neither can a purely numeric or boolean signature.
  • Accept an async brand check, so promise-returning interfaces have a way to comply.
  • Make a bad declaration fail the build rather than vanish into a dbg.error at runtime. This shape of bug has now shipped across three merged PRs without being noticed.
  • Then add checkReceiver to the declarations that genuinely need it, and re-enable the refusal.

The throw itself has been removed from develop as a stopgap so the affected interceptors install again; the ordering imprecision in categories 2 and 3 remains and is what this issue tracks.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions