endojs / endo

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

Expose CapDataShape #1782

Open iomekam opened 1 year ago

iomekam commented 1 year ago

What is the Problem Being Solved?

I recently made a change in agoric/wallet-app where we needed to know the shape of CapData. I was unable to export the type and ended up recreating it within the project.

It'll be nice to have it available within endo for exporting.

Description of the Design

Export CapDataShape

Security Considerations

N/A

Scaling Considerations

N/A

Test Plan

N/A

Upgrade Considerations

N/A

kriskowal commented 1 year ago

attn @erights for design feedback

mhofman commented 1 year ago

I am concerned that code would require to know the CapData shape instead of treating it as a opaque thing to pass to marshal. For issues like #1478 we want the flexibility to change the CapData format, including changing which properties may be present.

mhofman commented 1 year ago

Ugh I didn't realize we even had a pattern to enforce the current CapData shape... this is unfortunate.

turadg commented 1 year ago

require to know the CapData shape instead of treating it as a opaque thing to pass to marshal

We have this in lib-board: https://github.com/Agoric/agoric-sdk/blob/3f47cf03423b46b69851b021a18707b6ce42f056/packages/vats/src/lib-board.js#L23-L30

based on https://github.com/endojs/endo/blob/162a916137b80c59d1f91ce76ef926f57e014657/packages/marshal/src/types.js#L76-L81

Is that type in Marshal inaccurate?

If so, is there anything a client can do to validate that something is CapData?

mhofman commented 1 year ago

Can you clarify what you mean by "validate that something is CapData" ? Something having a { body: string, slot: string[]} shape does not make it CapData. Also I do want in the future to support having things that don't have that shape to be handled by marshal, for example in the shape of {"#body": JSONSerializable, slot: string[]}.

I think at the core I would expect CapData to be M.splitRecord({ slots: M.array() }), that should be forward compatible enough. That still won't validate that the thing is CapData that can be successfully unmarshalled, only a call to marshal decode can do that.

erights commented 1 year ago

... I was unable to export the [CapData] type ...

Any idea why?

(Postponing further responses until I understand this)

turadg commented 1 year ago

Can you clarify what you mean by "validate that something is CapData" ?

Good question. I think the code in lib-board is to satisfy the MarshalI interface guard. It could say M.any() and pass things along to the marshaller to validate but it has the Exo do a shape check first. I think that's a good idea but I would hear an argument against it.

For the case that motivated this issue, where a client tried to validate the shape before passing it to be unserialized, I think you're right that it should leave that to the marshaller.

Regardless, people will be reaching for a CapDataShape and I think Endo should provide it. If that needs to be M.splitRecord({ slots: M.array() }) then fine. If it has to be M.splitRecord({ }) because nothing more can be said, that's also worth having specified.