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

node.js resolver may arbitrarily remap modules to different sources #4

Closed
devsnek opened this issue Oct 24, 2019 · 8 comments
Closed

Comments

@devsnek
Copy link
Member

devsnek commented Oct 24, 2019

node has loaders which can arbitrarily redirect module resolution, which could then fail these checks. Since this isn't part of node's security model anyway, should this proposal just be optional for hosts to enforce?

@devsnek devsnek changed the title node.js tooling may arbitrarily replace modules node.js resolver may arbitrarily remap modules to different sources Oct 24, 2019
@xtuc
Copy link
Member

xtuc commented Oct 24, 2019

Hosts are always free to not, but it would mean the developer will not be able to rely on these checks (which is what this proposal is about). On the other hand it might be wanted during tests.

Import-maps would be another occurence of arbitrary remaping.

@littledan
Copy link
Member

Are file types resolved before or after remapping?

@devsnek
Copy link
Member Author

devsnek commented Oct 29, 2019

@littledan after. asking for ./config.json might be turned into /tmp/config_wrapper.mjs (or a module that isn't even on the filesystem).

And to be explicit, this is a feature for node, not a bug/antipattern/security issue.

@littledan
Copy link
Member

This is really interesting background. Let's make sure to document it somewhere in the repository.

@justinfagnani
Copy link

which could then fail these checks.

That's the whole point of the checks. If I have an Node application that loads JSON over the network, I want to assert that it's actually JSON and not script that could do anything to the user's machine.

@devsnek
Copy link
Member Author

devsnek commented Nov 1, 2019

@justinfagnani your local code is doing the remapping. if a loader is loaded it can just exfiltrate your harddrive using fs and http. no need to hack your json imports.

@littledan
Copy link
Member

Coming back to @devsnek 's original point:

should this proposal just be optional for hosts to enforce?

I'd like to conclude: yes. While all hosts are required to support JSON modules using with type: "json", this proposal doesn't prohibit hosts from also exposing other kinds of JSON modules.

Is this sufficient to close the issue, or are there further aspects to discuss?

@nicolo-ribaudo
Copy link
Member

I'm closing this issue, per #4 (comment). Hosts are free to provide JSON imports without the attributes that indicates that it must be actually JSON.

Please reopen in https://github.com/tc39/proposal-json-modules if you'd prefer to continue the discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants