Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feat] Document on how to import node builtin under --experimental-network-imports #48591

Closed
loynoir opened this issue Jun 29, 2023 · 16 comments
Closed
Labels
loaders Issues and PRs related to ES module loaders

Comments

@loynoir
Copy link

loynoir commented Jun 29, 2023

Version

v20.3.0

Platform

No response

Subsystem

No response

What steps will reproduce the bug?

reproduce.mjs

import { dirname } from 'node:path'

// import { foo } from 'http://git-server-with-correct-mime-type/raw/mod.mjs'
const foo = 42

function bar() {
  void { foo, dirname }
}

export { bar }
$ python -m http.server  &
$ node --experimental-network-imports
> var mod=await import('http://127.0.0.1:8000/reproduce.mjs')
Uncaught:
Error [ERR_NETWORK_IMPORT_DISALLOWED]: import of 'node:path' by http://127.0.0.1:8000/reproduce.mjs is not supported: only relative and absolute specifiers are supported.
    at __node_internal_captureLargerStackTrace (node:internal/errors:496:5)
    at new NodeError (node:internal/errors:405:5)
    at checkIfDisallowedImport (node:internal/modules/esm/resolve:921:13)
    at defaultResolve (node:internal/modules/esm/resolve:1007:23)
    at DefaultModuleLoader.resolve (node:internal/modules/esm/loader:269:12)
    at DefaultModuleLoader.getModuleJob (node:internal/modules/esm/loader:153:32) {
  code: 'ERR_NETWORK_IMPORT_DISALLOWED'
}

How often does it reproduce? Is there a required condition?

Always.

What is the expected behavior? Why is that the expected behavior?

Able to import node:XXX under --experimental-network-imports.

What do you see instead?

Unable to import node:XXX under --experimental-network-imports.

Additional information

Related #42098 (comment)

@loynoir
Copy link
Author

loynoir commented Jun 29, 2023

Yet another error, after convert node:XXX to XXX.

// import { dirname } from 'node:path'
import { dirname } from 'path'
$ node
> var mod=await import('http://127.0.0.1:8000/reproduce.mjs')
Uncaught:
Error [ERR_NETWORK_IMPORT_DISALLOWED]: import of 'path' by http://127.0.0.1:8000/reproduce.mjs is not supported: remote imports cannot import from a local location.
    at __node_internal_captureLargerStackTrace (node:internal/errors:496:5)
    at new NodeError (node:internal/errors:405:5)
    at checkIfDisallowedImport (node:internal/modules/esm/resolve:914:15)
    at defaultResolve (node:internal/modules/esm/resolve:1007:23)
    at DefaultModuleLoader.resolve (node:internal/modules/esm/loader:269:12)
    at DefaultModuleLoader.getModuleJob (node:internal/modules/esm/loader:153:32) {
  code: 'ERR_NETWORK_IMPORT_DISALLOWED'
}

@loynoir loynoir changed the title Unable to import node:XXX under --experimental-network-imports Unable to import node:XXX and {{NODE_BUILTIN}} under --experimental-network-imports Jun 29, 2023
@aduh95
Copy link
Contributor

aduh95 commented Jun 29, 2023

Here's a simpler repro:

import {createServer} from 'node:http';

const server = createServer((req, res) => {
    res.setHeader('Content-Type', 'text/javascript');
    res.end('import"node:path"');
}).listen(() => {
    import(`http://[::1]:${server.address().port}/`).then(console.log, console.error).finally(() => process.exit())
});

I agree the error message needs to be improved, and/or built-in modules should be available (but there might be a good reason for not having them available, not sure).

/cc @nodejs/loaders

@bmeck
Copy link
Member

bmeck commented Jun 29, 2023

Banning loading builtins was intentional due to security concerns.

@VoltrexKeyva VoltrexKeyva added the loaders Issues and PRs related to ES module loaders label Jun 29, 2023
@renhiyama
Copy link

Banning loading builtins was intentional due to security concerns.

So nodejs is trying to just add URL imports without any real reason? Or to convert users from nodejs to deno or what?

I understand the reason why http is disallowed and only https is allowed, but why not allowed to load native modules?

Over 90% of npm modules depends on native node modules.
Source? Trust me bro 😜

Still, there should be a more valid reason than just disabling due to security issues? This really shouldn't be a case considering the authors of deno and nodejs are the same ...

Would nodejs be implementing a similar permissions system and lockfile like deno before enabling this?

@JakobJingleheimer
Copy link
Contributor

I believe there was some discussion in the PR adding support for http imports: #36328

TLDR: Builtins have an enormous amount of system access. A module loaded remotely is subject to several attack vectors that can result in the contents of the module not being what the user expects (the most simple of which is the remote server simply provides something else).

Perhaps after some additional security measures are available (like checksums), this restriction may be lifted.

As it currently stands, I think this issue can be closed as "as designed" ("not planned").


I agree the error message needs to be improved

@aduh95 I'm open to suggestions 🙂

@bmeck
Copy link
Member

bmeck commented Jul 16, 2023 via email

@loynoir
Copy link
Author

loynoir commented Jul 17, 2023

As it currently stands, I think this issue can be closed as "as designed" ("not planned").

Workaround
$ node --experimental-network-imports
Welcome to Node.js v20.4.0.
Type ".help" for more information.
> (node:455897) ExperimentalWarning: Network Imports is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)

> await import('http://self-hosted-git-server/user/repo/raw/commit/929afbdef910c2a845b5de7f21f19010b4b0cc69/src/example.mjs')
Uncaught:
Error [ERR_NETWORK_IMPORT_DISALLOWED]: import of 'http://self-hosted-git-server/user/repo/raw/commit/929afbdef910c2a845b5de7f21f19010b4b0cc69/src/example.mjs' by undefined is not supported: http can only be used to load local resources (use https instead).
    at __node_internal_captureLargerStackTrace (node:internal/errors:496:5)
    at new NodeError (node:internal/errors:405:5) {
  code: 'ERR_NETWORK_IMPORT_DISALLOWED'
}
> 

iptables-workaround.sh

#!/bin/bash
sudo iptables -t nat \
    -A OUTPUT -s 127.0.0.1/32 -p tcp -m comment --comment x_workaround_node_http_import_container -m tcp --dport 11111 -j DNAT --to-destination 172.22.0.3:80
sudo iptables -t nat \
    -A POSTROUTING -o eth2 -m comment --comment x_workaround_node_http_import_container -j MASQUERADE
$ bash iptables-workaround.sh
$ node --experimental-network-imports
Welcome to Node.js v20.4.0.
Type ".help" for more information.
> (node:456666) ExperimentalWarning: Network Imports is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)

> await import('http://127.0.0.1:11111/user/repo/raw/commit/929afbdef910c2a845b5de7f21f19010b4b0cc69/src/example.mjs')
Uncaught:
Error [ERR_NETWORK_IMPORT_DISALLOWED]: import of 'node:path' by http://127.0.0.1:11111/user/repo/raw/commit/929afbdef910c2a845b5de7f21f19010b4b0cc69/src/example.mjs is not supported: only relative and absolute specifiers are supported.
    at __node_internal_captureLargerStackTrace (node:internal/errors:496:5)
    at new NodeError (node:internal/errors:405:5)
    at checkIfDisallowedImport (node:internal/modules/esm/resolve:921:13)
    at defaultResolve (node:internal/modules/esm/resolve:1007:23)
    at DefaultModuleLoader.resolve (node:internal/modules/esm/loader:251:12)
    at DefaultModuleLoader.getModuleJob (node:internal/modules/esm/loader:140:32) {
  code: 'ERR_NETWORK_IMPORT_DISALLOWED'
}

loader-workaround.mjs

const REGX_SELF_HOSTED_GIT_SERVER = /^.*\/raw\/commit\/.*$/

export async function resolve(specifier, context, nextResolve) {
    try {
        if (specifier.startsWith('node:') && REGX_SELF_HOSTED_GIT_SERVER.test(context.parentURL)) {
            return { shortCircuit: true, url: specifier }
        }
    } catch { }

    return await nextResolve(specifier, context)
}
$ node --experimental-network-imports --loader ./loader-workaround.mjs
Welcome to Node.js v20.4.0.
Type ".help" for more information.
> (node:457063) ExperimentalWarning: Custom ESM Loaders is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
(node:457063) ExperimentalWarning: Network Imports is an experimental feature and might change at any time
(node:457063) ExperimentalWarning: Network Imports is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)

> await import('http://127.0.0.1:11111/user/repo/raw/commit/929afbdef910c2a845b5de7f21f19010b4b0cc69/src/example.mjs')
[Module: null prototype] { bar: [Function: bar] }
> 

@loynoir
Copy link
Author

loynoir commented Jul 17, 2023

I think, should control this behavior thru import map standard, not thru loader convention.

#49443

  "scopes": {
    "/scope/dangerous": {
      "node:path": "" as "" | undefined | null | false
    },
    "/scope/trusted": {
      "node:path": "node:path" as string | true
    },

@bmeck
Copy link
Member

bmeck commented Jul 17, 2023

@loynoir node does not currently support import maps and import maps don't support the proper values for things like delegation to the loader https://nodejs.org/dist/latest-v20.x/docs/api/permissions.html#dependency-redirection-using-scopes does support your need without being invalid to import map specification.

@loynoir
Copy link
Author

loynoir commented Jul 17, 2023

@bmeck

Seems interesting.

Is --experimental-policy=policy.json --experimental-network-imports valid yet?

@bmeck
Copy link
Member

bmeck commented Jul 20, 2023

@loynoir policies affect anything through the loader, not sure what you mean valid.

@loynoir
Copy link
Author

loynoir commented Jul 20, 2023

To import node:XXX and {{NODE_BUILTIN}} under --experimental-network-imports, one can use experimental-policy

==> reproduce.mjs <==
import { dirname } from 'node:path'

const foo = 42

function bar() {
    return { foo, dirname }
}

export { bar }

==> policy.json <==
{
  "resources": {
    "http://127.0.0.1:8000/reproduce.mjs": {
      "integrity": "sha384-cZq7X+S/sVvvmCSao4nDKY0nC5t7jub2e3czg3e0C17aTTyWr0+VE8xWrPbLTgo3",
      "dependencies": {
        "node:path": "node:path"
      }
    },
    "file:///tmp/tmp.wH1VN7OzQ9/repl": {
      "dependencies": {
        "http://127.0.0.1:8000/reproduce.mjs": "http://127.0.0.1:8000/reproduce.mjs"
      }
    }
  }
}
$ node --experimental-network-imports --experimental-policy=policy.json
Welcome to Node.js v20.4.0.
Type ".help" for more information.
> (node:654501) ExperimentalWarning: Policies are experimental.
(Use `node --trace-warnings ...` to show where the warning was created)
(node:654501) ExperimentalWarning: Network Imports is an experimental feature and might change at any time

> mod=await import('http://127.0.0.1:8000/reproduce.mjs')
127.0.0.1 - - [20/Jul/2023 15:59:40] "GET /reproduce.mjs HTTP/1.1" 200 -
[Module: null prototype] { bar: [Function: bar] }
> mod.bar().dirname('/a/b')
'/a'

@loynoir loynoir closed this as completed Jul 20, 2023
@loynoir
Copy link
Author

loynoir commented Jul 20, 2023

Would be nice to have better document.

@loynoir loynoir reopened this Jul 20, 2023
@loynoir loynoir changed the title Unable to import node:XXX and {{NODE_BUILTIN}} under --experimental-network-imports [feat] Document on how to import node builtin under --experimental-network-imports Jul 20, 2023
@GeoffreyBooth
Copy link
Member

This is a hacky workaround, not something that we should recommend in the docs. It looks like it might even be a bug or security vulnerability that should be patched, certainly not something to be encouraged.

@bmeck
Copy link
Member

bmeck commented Jul 20, 2023

@GeoffreyBooth this isn't a security vuln from the perspective of mine at least. The ability to include dangerous stuff by redirection still exists even w/o policies by passing in the capabilities (much more like how WASM does things) but policies etc. aren't made to have opinions on redirection of capabilities.

@bmeck
Copy link
Member

bmeck commented Jul 20, 2023

I'd make a separate discussion on if this seems "hacky" since it is just using the accept list mechanism available everywhere. That could be done in a separate PR if the UX/DX should be cleaned up / separated (though that would mean multiple tracking points for these redirects)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
loaders Issues and PRs related to ES module loaders
Projects
None yet
Development

No branches or pull requests

7 participants