endojs / endo

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

feat: support `M.raw()` in method guards #1831

Closed michaelfig closed 10 months ago

michaelfig commented 11 months ago

closes: #1725 closes: #1772 refs: #1724

Description

Complete the #1724 experiment, as well as using the defaultGuards option (originally discussed in https://github.com/endojs/endo/pull/1725#discussion_r1296249277) instead of boolean flags for sloppy and raw.

Security Considerations

The new M.raw() guard is only honoured by arguments and return values in defendSyncMethod, so it should not leak elsewhere as a pattern.

Scaling Considerations

Some additional overhead in redacting raw arguments (in defendSyncArgs), but work is minimized by calculating relevant data structures while wrapping methods before any exos are created (in defendSyncMethod).

Documentation Considerations

Testing Considerations

Upgrade Considerations

Backward compatibility is implemented for interfaceGuards created using the legacy sloppy parameter.

michaelfig commented 10 months ago

All comments are addressed, and CI is passing. Ready for rereview!

michaelfig commented 10 months ago

Even reading the linked issues, I'm not clear on the motivation for introducing this non-Passable "raw" argument/return

Exos used to be tightly coupled to @endo/pass-style, so that it was impossible to define an exo method that either took a non-Passable as an argument or returned a non-Passable. The usage I am working towards is an exo-promise that allows settler.resolve(anyValueAtAll). There is no way to express such a method on an exo class, since the old most lenient argument guard (M.any()) was to harden(anyValueAtAll) and then assert it was passable.

and unknown-method functionality,

We already had the unknown-method functionality called sloppy, and @erights proposed extending it to allow the specification of sloppy-passable methods, as well as sloppy-raw methods. I did the implementation in this PR, even though I have no particular use for the functionality at this point, and prefer the "raw" argument/return annotations since they are stricter.

and am wondering if it might be better to instead establish a convention

I want to be able to create exo objects or facets whose methods can be just as flexible as a vanilla JS function, but have that be an opt-out so that the Exo model of guards and patterns is the default behaviour.

for e.g. conveying mutable objects via thinks

Funny you should mention that. Thunks are non-passable, too!

and/or introduce specific configuration for supporting unknown methods.

The specific configuration is to explicitly specify M.interface('Foo', { meth: M.call().rest(M.raw()).returns(M.raw()) }). Arguably, we could lean in further and do something like M.interface('Foo', { meth: M.callRaw() }) too.

Can you explain the need?

I don't want to make breaking design changes to contracts or libraries that I'm converting to be Exo-objects. Especially when so much of their behaviour fits into the Exo-object model already. M.raw() makes that possible. The unknown method support is tangential but related, and I don't really care as much about it.

gibson042 commented 10 months ago

Even reading the linked issues, I'm not clear on the motivation for introducing this non-Passable "raw" argument/return

Exos used to be tightly coupled to @endo/pass-style, so that it was impossible to define an exo method that either took a non-Passable as an argument or returned a non-Passable. The usage I am working towards is an exo-promise that allows settler.resolve(anyValueAtAll). There is no way to express such a method on an exo class, since the old most lenient argument guard (M.any()) was to harden(anyValueAtAll) and then assert it was passable.

I think that just pushes the question back a level... what is the underlying motivation for non-Passable promise resolution? M.raw() seems like a reasonable way to get a pattern representing literally any value, and defaultGuards "passable" vs. "raw" seems good for its propagation through defineExo{Class,ClassKit}, but I'm still wondering why we need to do anything at all here.

for e.g. conveying mutable objects via thinks

Funny you should mention that. Thunks are non-passable, too!

Are they?

import "@endo/init";
import { Far } from "@endo/far";
import { M, matches } from "@endo/patterns";

const fn = Far("fn", () => "returned");
fn();
// => "returned"
matches(fn, M.remotable());
// => true

I don't want to make breaking design changes to contracts or libraries that I'm converting to be Exo-objects.

Ah, this seems like it's heading in the direction I care about. What is being converted to exo, what is the benefit, and how do they currently interact with non-Passable values?

michaelfig commented 10 months ago
const fn = Far("fn", () => "returned");
fn();
// => "returned"
matches(fn, M.remotable());
// => true

Oh, I misspoke when I said "thunks are non-passable". I meant to say "passable thunks are not consistently supported in SwingSet and other random parts of our system, so I wouldn't count on using them for anything important until they have had significantly more testing."

Ah, this seems like it's heading in the direction I care about. What is being converted to exo, what is the benefit, and how do they currently interact with non-Passable values?

IBC/Network vats and Pegasus are being made durable and upgradable. I'm trying to do a minimal implementation of upgrade-tolerant promise-like remotables, and that's the last in a long line of trying to do promise-like remotables and bashing my head into the ceiling imposed by the passable restrictions. This PR is the result of trying to raise that ceiling so that I can use Zones and Exo-objects for durability as advertised.