search

tc39 / proposal-shadowrealm

ECMAScript Proposal, specs, and reference implementation for Realms
https://tc39.es/proposal-shadowrealm/
1.43k stars 67 forks source link

Adding explainer for error.message and censoring error.stack #381

Closed caridy closed 1 year ago

caridy commented 1 year ago

This PR introduces an extended explainer for error.message and error.stack.

bathos commented 1 year ago

If lowercase names represent incubator realm functions and uppercase those of one shadow realm in a call stack like a → B → c → D → e and e includes code introspecting new Error().stack (note nothing is thrown), does it “see” a-c-e or a-B-c-D-e now? It seems like the text here does not say for certain, but it might imply the latter? I may be misunderstanding, though.

(FWIW if e can now observe B and D’s interleaved presence at will like that, ShadowRealm no longer satisfies my use cases.)

mhofman commented 1 year ago

The intent was to allow a-B-c-D-e. The assumption is that the incubator realm is in a position to wrap ShadowRealm it constructs anyway, so it'd be able to stitch stacks.

I am curious about your use case. Would you be able to share why an incubator realm shouldn't be able to automatically introspect these stack frames? We were trying to strike a balance, and heard feedback that requiring things like telemetry libraries in the incubator realm to recursively wrap the ShadowRealm constructors to stitch stacks was too onerous.

bathos commented 1 year ago

Would you be able to share why an incubator realm shouldn't be able to automatically introspect these stack frames?

Implementation and virtualization of “native” Web IDL constructs. The incubator realm (or other realms that share first-class object values with it) includes the bootstrapping code / interfaces over for the implementation layer but also arbitrary consumer code; the ShadowRealm is providing encapsulation for the implementation code, not encapsulation for evaluation of arbitrary or otherwise “untrusted” (or whatever term is accepted — you know what I mean I’m sure) code.

heard feedback that requiring things like telemetry libraries in the incubator realm to recursively wrap the ShadowRealm constructors to stitch stacks was too onerous.

It is not the exposure that is the problem but the conditions of exposure: it would need to occur only when a genuine throw completion crossed the boundary. If the error were thrown from the shadow, it would be no problem if the stack included all frames because it would represent a real error in the virtualized implementation layer. When the shadow wishes to create an error that will be exposed as a thrown value in the incubator realm, it must make an explicit call to create that error and throw it there, and that error should not expose internal frames because it is part of correct operation of the implementation code (e.g. a DOMException or argument-conversion TypeError being thrown as specified).

Because of the possibility of “re-entrancy” especially, but also the fact that serialized stacks cannot currently be reliably parsed (e.g. function names with newlines in them in v8), we would have no way to reliably derive a-c-e from a-B-c-D-e before exposure even though the opportunity for manual redaction theoretically exists. Disconnected iframes already handle this scenario right. The implementation of ShadowRealm I’ve worked with in d8 prior to this change has as well, but it seems like that may no longer be true.

caridy commented 1 year ago

If lowercase names represent incubator realm functions and uppercase those of one shadow realm in a call stack like a → B → c → D → e and e includes code introspecting new Error().stack (note nothing is thrown), does it “see” a-c-e or a-B-c-D-e now? It seems like the text here does not say for certain, but it might imply the latter? I may be misunderstanding, though.

@bathos It is the latter a-B-c-D-e but only if the incubator realm is not a ShadowRealm (say it is a window). If, on the other hand, the incubator realm is a ShadowRealm, e.g.: Window->ShadowRealm_X->ShadowRealm_Y, then only code in Window can see the full stack, and therefore e was evaluated in ShadowRealm_X, and the stack property will contain a-c-e because the other frames do not belong to the realm associated to the ShadowRealm instance were e is evaluated. It is about individual stack frames, and they associated Realm only when observed from ShadowRealms.

caridy commented 1 year ago

The new introduction is very well-written, though it doesn't match the new spec text. I guess we could just delete the normative text on message and encourage engines to do something like this, more or less. The censorship goal will require consensus.

Agreed! I have updated the spec to remove those steps, and I have added a lot more details here about the suggested process.