Conversation
…y in an application
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
| } | ||
|
|
||
| const scope = new Scope(componentName, componentDirectory, configPath, resources, server); | ||
| if (options.applicationContainment) scope.applicationContainment = options.applicationContainment; |
There was a problem hiding this comment.
Why not pass this to Scope constructor?
There was a problem hiding this comment.
I wasn't sure if we really wanted to do this. I created the applicationContainment property in the constructor, based on the config, to ensure it exists. It seems like the constructor has a lot of arguments, and assigning different values is more of an unusual path that seems like it can be handled by property assignment (as is done for the tests). The applicationContainment is now guaranteed to exist on the scope and type safe. But if you still want me to add an argument to the Scope I certainly can.
Co-authored-by: Ethan Arrowood <ethan@arrowood.dev>
Co-authored-by: Ethan Arrowood <ethan@arrowood.dev>
Co-authored-by: Ethan Arrowood <ethan@arrowood.dev>
Co-authored-by: Ethan Arrowood <ethan@arrowood.dev>
0f65288 to
18d0d4f
Compare
cap10morgan
left a comment
There was a problem hiding this comment.
One teeny tiny thing to fix (or not worry about) but looks awesome overall!
Addresses CORE-2463, providing a separate top level scope as containment mechanism for the application code in each application. The primary goal of CORE-2463 is to have distinct top level variables, or exports from
harper, such as the application-specific config, logger, etc. Separate contexts/containment also provides an opportunity to enforce greater security for protection against using modules with system level access, and code execution exploits through prototype traversal and modification.This is configurable, but by default, the node.js's
vmmodule is used to achieve this separate context/top-level-scope/container. We can alternately use endojs/SES'sCompartmentmechanism, which is based on the EcmaScript proposal https://github.com/tc39/proposal-compartments/blob/master/4-compartment.md. From my testing, thevm-based containment is more compatible and usable for more packages. Compartments do have an advantage in that they don't require a set of intrinsics (Object, Array, etc.)This is not applied to (or enforced on) plugins, but plugins are given a "scoped"
importmethod that they can use to load JavaScript in their own scoped context. This is implemented as animportmethod on theScope, and then used by the jsResource module to load the resources module (and could be used elsewhere).Using a separate context can combined with "lockdown", which can freeze intrinsics and "tame" intrinsics to prevent dangerous actions. This is probably the most effective broad mechanism for providing secure, high-integrity JavaScript (enforcing all the things that TS can never do). Unfortunately, it comes at a high price of compatibility. Most popular packages have some violation of changing properties that ends up causing errors with attempts to write to read-only properties. So this is disabled by default. We may want to do some serious consideration about if we do want to do this in some situations though.
This also has an option for whether package dependencies (loaded from
node_modules) should be loaded in the main context (the default) or in the application's context (applicationContainment: true). With lockdown, and using the application's context for 3rd party packages, these packages actually have the luxury of being able to mutate their own intrinsics (Object, etc.), but there are still shared objects because we can't make separate instances/copies of Node's core modules, which can still lead to problems. For example, I believe this will always fail oncelockdown()has been performed: