Closely related to this is a never-ending import (a malicious server sending a never-ending stream of data).

Likely need a max import size as well as timeouts.

@jasnell agreed, but there are multiple concepts of granularity here. Graph size vs individual source text size. I think we have to solve for both either by mitigating or scoping out. I'm thinking for this topic we are really talking about mitigations on the individual level, but the infinite recursion might be better for the strongly connected static graph mitigations. I think at least for the initial approach to size, byte limits might be enough as laid out in https://github.com/bmeck/node/tree/policy-max-body . Timeouts are more complex.

How can that happen?

A server could dynamically generate a payload that ~= import "import.meta.url + 1 ". This would prevent the ESM graph from ever evaluating any code and memory leak until the process dies.

It is possible to do this in CJS already, but with ESM this is a bit different. CJS can write files as they are loaded. ESM must load files after they are written using dynamic import. With the introduction of Loaders and/or 3rd party actors like servers that makes static import also able to cause infinite recursion.

I'm not entirely sure how we want to limit this, could limit the dep graph source size of an import or the total distance of the graph from entry or from the main entry point. When we get to that point if the problem was through static imports it won't be too much of an issue since we can GC the modules prior to linking (linking makes them live for the life of the global unfortunately [cantfix / upstream]). With dynamic imports, we couldn't reclaim that lost memory.

We can somewhat do this already with URL redirects, see a policy with:

{
  "scopes": {
    "file:": {
       "integrity": true,
       "dependencies": {
           "https://example.com/foo": "file://cache/https/example.com/foo"
       }
    }
  }
}

That would not cover the HTTP redirect semantics, leak the wrong import.meta.url, and would not respect cache / updating. It does bring up some questions though about what the update semantics should be for a cache. Should it ever be automatic? I somewhat lean on "no it should always be a manual step to sync the cache".

I agree the cache update should be manual. I think we need a good CLI to work with it though.

• We already have integrity checks via policies, but the use cases of HTTPS imports also can lean towards automatic updating which wouldn't work with integrity checks.

HTTPS has other means of granting trust such as certificate verification of various types.

• Support for HPKP and/or Certificate Transparency would rely on a similar need for out of band persistence as policies but does allow for ensuring that the entity that provided the previous source text is the same as the current source text.
• Root CAs are already configurable but not able to be configured solely for import purposes.

This is really important to provide safety against social engineering attacks, e.g. left-pad and event-stream.

@mcollina those attacks would already be mitigated by policies as they exist today. Is there a specific aspect of those that you are seeking to guard against and should it be outside of the scope of HTTPS itself?

I'm not sure I understand. My understanding / expectation is it must be possible to do dynamic fetching of source texts over the network. I think trying to state it must be either always online or offline isn't really an easy topic since if we allow only one it invalidates some of the use cases of HTTPS imports.

that's how Deno does it and it's what makes more sense to me as well, I just wanted to clarify because a few of the comments on the PR seemed to assume it was only dynamic imports.

WHATWG already only allows only same domain from http/https schemes. I think the presumption is that https: could load node:, but I actually think that isn't really a priority as many/most modules don't actually need direct access to things like fs:. I do think in general that it is a security concern, but not a new attack vector. Without really strong reasons I do think requiring https: to have those kinds of node: modules dynamically injected as params or something seems fine to me.

node_modules lives on file: so it would be prevented by nature of being cross origin and not allowing https: to file:.

Browsers have a very clear origin to compare against, how would that work from the server? For reference this is also an ongoing conversation in Deno denoland/deno#4981

I don't think we should use Agent, it is mutable and means that an application could manipulate it to intercept things it shouldn't

I think it should have an Agent which is kept private for esm.

we could, but it cannot reuse the implementation currently in core

Also, it might be a good idea to require a flag (or use an external utility) to enable remote fetching, which will be used to enable remote fetching and initialise the web_modules folder.

Without the flag, any https:// import will only consult the local web_modules folder.

At the very least, there should be a way to disable remote lookup and only consult the local web_modules folder.

Visible caching on disk is a must, on that we agree, but why create yet another folder when reusing node_modules/ would work just fine?

Visible caching on disk is a must, on that we agree, but why create yet another folder when reusing node_modules/ would work just fine?

The recommendation for node_modules is to not commit that to the repo. web_modules should be committed to repo. Using two folders makes this simpler.

Currently policies allow rewriting URLs, would allowing wildcard URL rewriting like export templates be enough?

Also, can this be done as a follow on during experimentation?

I like the node_modules/.https/ - seems like a nice way to make the old and the new play together nicely

Unfortunately we can't put this in scope until we have some newer JS features; this likely has to wait for Realms or some such feature. We looked into policies and other mechanisms around limiting it for ESM in a few things in the past like: #26334. We need a way to introduce new global scopes w/o creating new intrinsics for things like Array which we don't have a way to do so currently. Users could replace globalThis.process with a getter that does a callsite check like in that and the other PR we attempted to get through, but the overall performance hit is not negligible. Other ways to limit it for now would be up to source transforms detecting usage of the globals and replacing it with a corresponding import/require.

Can we move it one week further? I'm off that week.

bumped, updated to new link

Filled

Per discussions above and video, network access would not be enabled by default when running node. During experimental it will be enabled automatically under the flag while offline cache is worked out since the flag is the opt-in.

Thanks, not all of us have time to watch videos, and well-maintained high-level summaries would help a lot in ensuring adequate onboarding for those familiarizing.

Is the flag a CLI flag with the intention to unflag at some point? Is this not something that can be integrated into policy?

Is the flag a CLI flag with the intention to unflag at some point? Is this not something that can be integrated into policy?

Various features were punted and behavior under the flag will change to accomodate them; the flag as it exists likely won't be unflagged until policy integration etc. is done but even with policy integration features like manipulating how redirects are expected to work for HTTP are not possible currently. We do have a POC at https://github.com/bmeck/local-fs-https-imports but it doesn't properly mimic various nuances of web based loading.

I've banned everything except network dependencies in the PR for now.

Note that depending on how the main application is started (notably via a CommonJS entry point), it's still possible to access the node: scheme using globalThis.process.mainModule.require().

@targos yup, this is known and we tried to fix Buffer/process so they were not implicit globals in ESM but failed to do so due to a perf concern. So... ambient ways to mitigate it are left up to users and not covered by things like policies. A lot of mitigations are about limiting how things can be accessed so they can properly be mitigated, and unfortunately not mitigating them in core itself. Removing data: for example means that you have to grab them via globals, or be passed them somehow via dependency injection. It removes import from the problem areas.

CC: @jasnell from same doc (woops)

How does the spec prevent us from intercepting the call before it hits the cache?

@targos it states that import() and import share the cache and deterministically share the result (this enforced by V8 source text module local cache, not the global cache which node controls). If one succeeds, the other must also succeed in perpetuity for the same source text and cache key (URL + import assertions these days [may grow]). So if you do import "fs"; dynamic import feeds through to FinishDynamicImport which feeds into the same mechanism as static imports (HostResolveImportedModule) which has the restriction: "Each time this operation is called with a specific referencingScriptOrModule, specifier pair as arguments it must return the same Module Record instance if it completes normally."

There's quite a lot of rationale behind supporting this in core. It is possible to do with a loader, but that would likely result in divergent behaviour, security vulnerabilities, etc.