WIP test(pass-style): demonstrate safe promise loophole for @FUDCo#1781
WIP test(pass-style): demonstrate safe promise loophole for @FUDCo#1781
Conversation
| * ```js | ||
| * function destroyTracking(promise, parent) { | ||
| * trackPromise(promise, parent); | ||
| * trackPromise(promise, parent); |
There was a problem hiding this comment.
Just a drive-by whitespace fix. Otherwise nothing to do with this PR.
| t.is(p4[Symbol.toStringTag], 4); | ||
| t.is(`${p4}`, '[object Object]'); | ||
| const getter = Object.getOwnPropertyDescriptor(p4, Symbol.toStringTag).get; | ||
| t.is(getter(true), 'I am p4'); |
c931b08 to
7d96dc9
Compare
|
@FUDCo because this PR simply demonstrates that endo already has this odd property, you could utilize it for now without waiting for another endo release cycle. See the TODOs added by the PR. |
7d96dc9 to
285c409
Compare
c82f2cf to
3089c82
Compare
3089c82 to
441dbf1
Compare
|
What's the name of the law that all observable behavior, whether documented or not, might get relied on, and thus no longer changeable? If we use this loophole, it'll become one of those, which would be bad. Let's discuss. |
|
For debugging-only purposes, provokes heap exos and well as virtual and durable objects to have a vref-revealing own @@toStringTag property. For virtual and durable objects, this is set by if (LABEL_INSTANCES) {
// This exposes the vref to userspace, which is a confidentiality hazard
// as noted in the CONFIDENTIALITY HAZARD NOTE above.
//
// Aside from that hazard, the frozen string-valued data property is
// safe to expose to userspace without enabling a GC sensor.
// Strings lack identity and cannot be used as keys in WeakMaps.
// If the property were a accessor property, we'd need to
// ```js
// linkToCohort.set(self, getterFunc);
// unweakable.add(getterFunc);
// ```
defineProperty(self, Symbol.toStringTag, {
value: `${proto[Symbol.toStringTag]}#${baseRef}`,
writable: false,
enumerable: false,
configurable: false,
});
}In order for this to work, the endo/packages/pass-style/src/remotable.js Line 217 in 162a916 It seems attractive to similarly have the Since @FUDCo 's need is also debug-only, it further seems attractive to extend the purpose of to label some promises with vrefs and krefs refs, using a similar @@toStringTag own data property. |
I'm really not sure how I feel about this. This particular loophole would actually work just fine for my use case, because the kref for a promise always has the form Another consideration is that our token object for remotables also has a second method that provides the iface string. I'm not sure that's important for promises (presumably they're all, well, promises) but maybe we might use the iface string to indicate what type of thing the promise is allegedly a promise for. But if we wanted that, we'd need a way to add a second method of this sort to the promise token as well. |
Look again. The number is just to pass the safe-promise check as it is. The getter parameter is so we can pass through whatever string we'd like. This would just be until we rev endo. If we rev endo to do #1781 (comment) , then the own @@toStringTag property could be a string-values data property. |
|
OK, I definitely misunderstood this the first time through. Now that I read it through again much more carefully, my weirdness meter has redlined. Let me try again to make sure I understand. I may be being overly detailed in this explication because the behavior in question is too bizarre for me to quite believe this is reality:
I can't even. |
|
Is |
|
I am weirded out by this I guess we don't have pseudo-promises yet we could use for this purpose? AFAIK, @FUDCo's constraint is to pass-through values but they don't have to be functional, right? |
| // TODO Decide whether to close this loophole, which would preclude | ||
| // using it to solve Chip's problem. | ||
| const unknownKeys = keys.filter( | ||
| key => typeof key !== 'symbol' || !hasOwnPropertyOf(Promise.prototype, key), |
There was a problem hiding this comment.
Yeah this really should have been:
| key => typeof key !== 'symbol' || !hasOwnPropertyOf(Promise.prototype, key), | |
| key => { | |
| if (typeof key !== 'symbol') return true; | |
| const protoDesc = getOwnPropertyDescriptor(Promise.prototype, key) | |
| return !(protoDesc && 'set' in protoDesc) | |
| }, |
There was a problem hiding this comment.
Are all the expected async_hook properties necessarily accessors with a setter? If so, yes.
There was a problem hiding this comment.
Yes, they're all accessors and meant to catch assignments (override mistake like taming), so guaranteed to have a set.
Yes |
FWIW, me too!
That was my first thought as well, and where I think the proper fix lies. But @FUDCo explained that he needs something soon, so I came up with this as a first step that happens to work without even revving endo. |
FWIW, me too!
But this is JavaScript!
The check was unaware of Symbol.toStringTag. It was checking all symbol-named properties on All the rest of your comments are essentially correct. |
|
See #1785 |
|
Closing in favor of #1785 |
In #1275 I loosened the safe-promise.js checks used by
passStyleOfin order to accommodate our workaround for the extra symbol-named own properties that Node's currentasync_hooksmight add to promise instances. This checks these possible extra own properties against symbol-named own properties that would otherwise be inherited fromPromise.prototype.However, in doing so I seem to have forgotten that, even without async_hooks,
Promise.prototypealways has an own property namedSymbol.toStringTag. Thus, the safe-promise rules currently allow that, but only if the property's "value" passes further checks. However, apparently by design, this "value" is obtained by a normal property GET (pr[key]), which might invoke a getter, allowing these overriding properties to be accessors. The easiest way for the value to pass the subsequent checks is to be a number.The getter can be a per-instance function that, when called with a truthy argument, returns something else, like the string @FUDCo needs. The normal builtin
toStringuse ofpr[Symbol.toStringTag]expects a string value, which would not pass the test, causing a conflict. However, whenpr[Symbol.toStringTag]is instead a number, the builtintoStringbehavior falls back to printing[object Object]rather than[object Promise], which is harmless enough for the very specialized case in which @FUDCo would use this loophole.