Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Hazard: hidden public API #1361

Closed
erights opened this issue Aug 3, 2020 · 8 comments
Closed

New Hazard: hidden public API #1361

erights opened this issue Aug 3, 2020 · 8 comments
Assignees
Labels
enhancement New feature or request eventual-send package: eventual-send hygiene Tidying up around the house security tooling repo-wide infrastructure

Comments

@erights
Copy link
Member

erights commented Aug 3, 2020

For example https://github.com/Agoric/agoric-sdk/blob/master/packages/eventual-send/src/index.d.ts does not declare at least the following publicly visible methods

  • HandledPromise.unwrap
  • E.unwrap
  • E.resolve

(See #1360 )

Before we had types and IDE support for types, we would habitually use the implementation as the authoritative source on what API it implemented. With IDE support, our habits and those of our users will more often rely on declared and inferred types, especially since the static checker gives some reasonable (though unsound) degree of fidelity to what we would have gathered from reading the implementation. This fidelity includes checking that at least the declared API is implemented. But the checker is silent on whether there are additional public APIs that do not appear in the declared or inferred types.

For security programming, this may lead to the programmer underestimating the authority that some abstraction provides. Marc Stiegler and I have been through this before for O'Caml, as explained at https://www.hpl.hp.com/techreports/2006/HPL-2006-116.html . To tame O'Caml we had to discover all these undocumented public APIs and turn off at least all the unsafe ones.

So, feature request: How hard would it be for us to statically check that, for an implementation of a declared interface, that the implementation had no public properties that are absent from the declared interface? (Attn @michaelfig )

@erights erights added bug Something isn't working enhancement New feature or request eventual-send package: eventual-send hygiene Tidying up around the house security tooling repo-wide infrastructure and removed bug Something isn't working labels Aug 3, 2020
@michaelfig
Copy link
Member

Not hard to check, with properjs. The eventual send types are done in pure typescript, which hides this mistake.

@erights
Copy link
Member Author

erights commented Aug 3, 2020

How?

@erights
Copy link
Member Author

erights commented Aug 3, 2020

Also, that sounds like it only hides the mistake wrt inter-vat interactions. Which we should still do! But we should also detect this statically to reduce this threat locally.

@michaelfig
Copy link
Member

michaelfig commented Aug 3, 2020

Typescript interface types can have additional members that aren't reflected in the type. Jsdoc @typedef {Object} ... rejects additional members besides those listed in @property ... clauses.

@michaelfig
Copy link
Member

Typescript interface types can have additional members that aren't reflected in the type. Jsdoc @typedef {Object} ... rejects additional members besides those listed in @property ... clauses.

Note that this safety rests on the fact that the JS code is checked against the JSDoc. If there are (unsound) typecasts in the code, this is not the case.

@erights
Copy link
Member Author

erights commented Feb 25, 2021

Hi @michaelfig from your comments above, it sounds like you have a plan. So ok if I assign both of us?

@michaelfig
Copy link
Member

I don't have a plan per se, just mentioning that in our normal style of writing JSDoc interfaces and avoiding explicit casts, we don't have this problem.

@erights
Copy link
Member Author

erights commented Feb 26, 2021

Not having the problem seems an adequate reason to close this ;)

Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request eventual-send package: eventual-send hygiene Tidying up around the house security tooling repo-wide infrastructure
Projects
None yet
Development

No branches or pull requests

2 participants