refactor: introduce primordials #10939
Conversation
This commit introduces primordials to deno_core. Primordials are a frozen set of all intrinsic objects in the runtime. They are not vulnerable to prototype pollution.
lucacasonato
force-pushed
the
primordials
branch
from
2b896b5
to
d63ce7f
Compare
|
For what it's worth this is one of the ugliest things we have in the Node.js codebase and I'd caution against relying on primordials for security. Please be very mindful when you consider taking this sort of burden upon yourself as a platform. |
|
@benjamingr I don't disagree, but there are some web platform tests we can not get to pass without using primordials or re-architechting the entire system to use only native v8 bindins. Example of a failing test: https://staging.wpt.fyi/results/fetch/api/response/response-stream-with-broken-then.any.html?product=deno&product=chrome. The issue is that the glue between our native code (the ops) and our runtime internals are implemented completely as JS. They need to behave as if they are native objects however, because that is what happens in the browser. We do not intend to use primordials for security - all our access control is and still will be managed by native code, and we do now and will continue to consider all JS code untrusted. |
|
That's unfortunate, reasonable and fair. As far as I know attempts to solve this (better) at the platform level were stopped - so I'm not sure what else you can do. |
| @@ -1732,7 +1732,8 @@ pub mod tests { | |||
|
|
|||
| #[test] | |||
| fn test_heap_limits() { | |||
| let create_params = v8::Isolate::create_params().heap_limits(0, 20 * 1024); | |||
| let create_params = | |||
| v8::Isolate::create_params().heap_limits(0, 3 * 1024 * 1024); | |||
There was a problem hiding this comment.
Note: required startup heap limits have increased.
There was a problem hiding this comment.
V8 is really inefficient on first startup (because of the interpreter). You can see in the benchmarks that the snapshot (so baseline heap size of Deno) has not really increased.
@magurotuna how involved would be adding a lint rule like this to |
|
@bnoordhuis do you have any objections to this PR? |
There was a problem hiding this comment.
LGTM
@lucacasonato could you later open an issue for tracking using primordials in extensions and runtime/js so we can share migration work?
Looks like rather complicated but not impossible. Let me try to implement it :) |
@bnoordhuis why? |
|
@benjamingr Robustness in the presence of monkey patching. Same reason it was a good idea in Node. :-) |
This commit introduces primordials to deno_core. Primordials are a
frozen set of all intrinsic objects in the runtime. They are not
vulnerable to prototype pollution.
This commit just migrates deno_core to use primordials. Followup commits can
introduce primordials to the rest of the runtime.
Node has a eslint plugin that validates that all globals that are used in a
given script use primordials. We should consider adding a plugin for this to
dlint too. See https://github.com/nodejs/node/blob/master/tools/eslint-rules/prefer-primordials.js
Towards #10756