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

Multiple slashes can bypass null exports path #44316

Closed
privatenumber opened this issue Aug 20, 2022 · 3 comments · Fixed by #44477
Closed

Multiple slashes can bypass null exports path #44316

privatenumber opened this issue Aug 20, 2022 · 3 comments · Fixed by #44477
Labels
loaders Issues and PRs related to ES module loaders

Comments

@privatenumber
Copy link
Contributor

privatenumber commented Aug 20, 2022

Version

v16.17.0

Platform

macOS 12.4

Subsystem

No response

Description

Setting null on an export path can be used to exclude private directories from being exposed (Node.js docs):

// ./node_modules/package/package.json
{
    "exports": {
        "./*": "./*",
        "./internal/*": null
    }
}

Although this works to prevent an import like this:

import 'package/internal/file.js'

It can be circumvented by using multiple slashes:

import 'package//internal/file.js'

In UNIX, multiple consecutive slashes are treated as a single slash.

What steps will reproduce the bug?

StackBlitz reproduction

https://stackblitz.com/edit/node-auughm?file=index.js

Reproduction repo link

https:/privatenumber/issue-reproductions/tree/master/reproductions/nodejs/node/44316

Steps

  1. Create the following files:

node_modules/package/package.json:

{
	"exports": {
		"./*": "./*",
		"./internal/*": null
	}
}

node_modules/package/internal/file.js:

console.log('Accessed internal file!')

index.mjs:

import('package/internal/file.js').catch(() => {
	console.log('Fail: Single slash import')
})

import('package//internal/file.js').then(() => {
	console.log('Success: Double slash import')
})
  1. Run node ./index.mjs

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

Always. No environmental conditions. Just multiple slashes in import paths.

What is the expected behavior?

For import 'package//internal/file.js' to not resolve.

What do you see instead?

It resolves.

Additional information

No response

@daeyeon daeyeon added the loaders Issues and PRs related to ES module loaders label Aug 21, 2022
@daeyeon
Copy link
Member

daeyeon commented Aug 21, 2022

I'm able to reproduce on v19.0.0-pre.

@daeyeon
Copy link
Member

daeyeon commented Aug 21, 2022

Seeing packageResolve() and packageExportsResolve(), the packageSubpath string should begin with any key string of packageConfig.exports to properly find the bestmatch and bestMatchSubpath. It doesn't seem to use a normalized path. Maybe that's intended or a bug.

let bestMatch = '';
let bestMatchSubpath;
const keys = ObjectGetOwnPropertyNames(exports);
for (let i = 0; i < keys.length; i++) {
const key = keys[i];
const patternIndex = StringPrototypeIndexOf(key, '*');
if (patternIndex !== -1 &&
StringPrototypeStartsWith(packageSubpath,
StringPrototypeSlice(key, 0, patternIndex))) {

/cc @nodejs/loaders @nodejs/modules

@aduh95
Copy link
Contributor

aduh95 commented Aug 21, 2022

It doesn't seem to use a normalized path. Maybe that's intended or a bug.

I think that's intended, IIRC everything related to "exports" is a URL (or URL pattern), not a path.

That being said, I agree that we should fix this, using one or more slashes should give the same result.

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
3 participants