Implement an UnsafeEval binding

[jasnell]

This adds a new UnsafeEval binding APi that is intended only for use with workerd and only when the --experimental flag is specified. See the included test for an example on how to use it.

console.log(env.unsafe.eval('1+1'));  // 2

const fn = env.unsafe.newFunction('return m', 'foo', 'm');
console.log(fn(1));  // 1
console.log(fn.name);  // foo
console.log(fn.length); // 1 argument

/cc @mrbbot @IgorMinar

[mrbbot]

This is very exciting! 👍 From a DevProd perspective, this looks great, but probably want some more runtime eyes on this. 😃

[ohodson]

lgtm

[kentonv]

@jasnell Is this call actually necessary? Since we're not actually invoking the JS eval API in this code -- we're calling the C++ APIs directly -- I suspect there's no need to enable eval here?

[jasnell]

This is specifically to allow for cases where the script being eval'd itself contains nested regular eval() expressions, e.g.

env.unsafe.eval('eval(1)');

While this specific example is a bit silly, it's not unlikely that nested evals could be included in the script passed into the unsafe.eval(...)

[IgorMinar]

I didn't realize that that was the case @jasnell. I think it's undesirable to recursively allow eval to be used by unprivileged code.

We use dynamic execution in a specialty/privileged code that powers local development workflows/ Outside of this specialty code, we don't want people to use eval as that could user provided code to run locally but not in production.

I propose that we remove this recursive feature. If the privileged code has a need to expose eval to userland code, it can do so via globalThis.eval = env.unsafe.eval before evaling user's code.

[jasnell]

globalThis.eval = env.unsafe.eval wouldn't actually work directly, that would end up with a "Illegal invocation" error, but you can get close with globalThis.eval = function(code) { return env.unsafe.eval(code); }. But this is not a pattern that we should ever recommend either way.

We use dynamic execution in a specialty/privileged code that powers local development workflows/ Outside of this specialty code, we don't want people to use eval as that could user provided code to run locally but not in production.

I'm not sure how this would allow use of eval outside of this scope? The regular eval() would only be allowed within the script passed into env.unsafe.eval(...), which is only enabled with the binding that only exists in the local runtime. There's nothing here that would allow that to be used in production and nothing that would allow eval() to be used outside of this specific binding.

For instance, even with the binding enabled, the following still throws an error:

eval('1+1');  // throws!

But

env.unsafe.eval('eval("1+1")');  // works

Even if I tried something silly like the following, trying to break out of the restriction we still get an appropriate error.

const fn = env.unsafe.eval(`
function foo() { eval('1+1'); }
foo;
`);

fn();  // throws!

[mrbbot]

I think I agree with @IgorMinar here. Users could feasibly try to use eval()/new Function() in the top-level. Passing this user code to env.unsafe.eval would succeed in local development/testing, where it would fail when deployed.

As a concrete example, imagine a user wanted to use Ajv for JSON-schema validation. They build the validator in the top-level so they can reuse it across functions. This won't work when deployed, as new Function() is disabled, but would work if the user code was passed directly to env.unsafe.eval().

// ...bundled Ajv somehow
const ajv = new Ajv();

const schema = {
  type: "object",
  properties: {
    key: { type: "string" },
  },
  required: ["key"]
};
const validate = ajv.compile(schema); // fails when deployed because `new Function()` disallowed

addEventListener("fetch", (event) => {
  const valid = validate({ key: "thing" });
  if (valid) { ... } else { ... }
});

...whereas the following would pass...

env.unsafe.eval(`
// ...
const validate = ajv.compile(schema);
// ...
`)

Admittedly, the user code would probably be wrapped in a function() { ... } like your last code snippet, so this wouldn't be an issue. Even so, I'm not sure this should be the default behaviour.

[jasnell]

I'm not sure I understand why this is a problem. Using Ajv like this at the top level, the user would still need to call unsafe.eval(...) to use it, and we would only allow for eval(...) and new Function(...) within the synchronous execution of that call. ANY worker code that is using unsafe.eval(...), whether it contains a nested eval or not, will not work when deployed to production because we cannot deploy a worker with the unsafe binding at all -- it simply doesn't and won't exist outside of the local workerd environment.

[IgorMinar]

Yes, that's a good example.

If we wanted user code to have unrestricted access to eval then we could have implemented this feature as a workerd flag which makes globalThis.eval available. We chose to restrict access to eval by making it available via a binding instead, so I'd expect any recursive eval if really needed, to access the functionality via the binding and not global.

Since dynamic code evaluation is only meant to be used by development tooling (test runner code, dev server code) I think we should be intentional about not easily exposing it to user code running within this development tooling.

[brr53]

Is eval available yet in Cloudflare workers? Would love to use it / enable it for use. @mrbbot @IgorMinar

[jasnell]

Since dynamic code evaluation is only meant to be used by development tooling (test runner code, dev server code) I think we should be intentional about not easily exposing it to user code running within this development tooling.

I'm a bit confused. How is allowing env.unsafe.eval('eval("1+1")') less safe than allowing env.unsafe.eval('1+1')?

[IgorMinar]

@jasnell the goal is not "safety" but about preventing accidental code interoperability. Accidental use of eval in applications will result in some apps running locally, but breaking in production, which is bad DX.

Our use of eval in Workers tooling is for dev-only code that wraps the application code to make it run during local development workflows - this wrapper code will never run in production as it only enables hot module reloading and other dev-only features.

Since our code runs in the same VM as the application code we need a way to access eval but ideally without polluting the runtime globals with eval. Application code could be using eval directly, or indirectly via 3rd party libraries/generated code, without developers knowing about it, and learning about it only when the production deployment fails for an app that ran well locally.

You could argue that exposing eval via binding is not a high bar to overcome, but in that case you are using a non-standard API which is behind an experimental flag both of which reduce the likelihood of surprises in production.