endojs / endo

Endo is a distributed secure JavaScript sandbox, based on SES
Apache License 2.0
825 stars 71 forks source link

daemon: `eval`-mediated worker names do not resolve #2021

Open rekmarks opened 8 months ago

rekmarks commented 8 months ago

Describe the bug

When pet-naming a worker that is returned from an eval formula, you cannot later use that name to specify a --worker to the daemon. Doing so will result in the following error: Cannot deliver "evaluate" to target; typeof target is "undefined".

Steps to reproduce

Using the INFO feature of #1916:

> endo make counter.js --name counter
  Object [Alleged: Counter] {}
> endo eval 'E(INFO).lookup("counter")' INFO --name inspector
  Object [Alleged: Inspector (make-bundle ... )]
> endo eval 'E(inspector).lookup("worker")' inspector --name cworker
  Object [Alleged: EndoWorker] {}
> endo eval 'E(counter).incr()' counter --worker cworker
  CapTP cli exception: (RemoteTypeError(error:captp:Endo#20001)#1)
  RemoteTypeError(error:captp:Endo#20001)#1: Cannot deliver "evaluate" to target; typeof target is "undefined"

Expected behavior

As above except:

...
> endo eval 'E(counter).incr()' counter --worker cworker
  1
kriskowal commented 8 months ago

Design: we should go back to keeping an internal a WeakMap from worker handles to their corresponding bootstrap objects and stop using the internal facet trick. The WeakMap will work for workers even if laundered through an eval formula.

rekmarks commented 8 months ago

I believe this is another instance of the same problem (originally reported in #2074):

Steps to reproduce

> endo make counter.js --name counter

> endo eval 'E(counter).incr()' counter
1

> endo eval 'E(counter).incr()' counter
2

> endo eval 'foo' foo:INFO.counter.worker -n main-worker
Object [Alleged: EndoWorker] {}

// This should cancel the worker, but actually cancels a lookup formula instead.
// The bug is that this should, but does not, propagate to the worker (and then the counter).
> endo cancel main-worker

> endo eval 'E(counter).incr()' counter
3

Expected Behavior

> endo make counter.js --name counter

> endo eval 'E(counter).incr()' counter
1

> endo eval 'E(counter).incr()' counter
2

> endo eval 'foo' foo:INFO.counter.worker -n main-worker
Object [Alleged: EndoWorker] {}

> endo cancel main-worker

> endo eval 'E(counter).incr()' counter
1
rekmarks commented 7 months ago

We believed that this was fixed by #2092. Rather than fixing the bug, however, that PR concealed it by always creating a new incarnation of eval formula workers, even if they already existed. It's back to the drawing board with this one.