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

monorepo-symlink-test #312

Closed
roberthockley opened this issue Sep 4, 2023 · 5 comments
Closed

monorepo-symlink-test #312

roberthockley opened this issue Sep 4, 2023 · 5 comments

Comments

@roberthockley
Copy link

Hi,

It seems that resolve has a dependancy on monorepo-symlink-test. I see it in node_modules/resolve/test/resolver/multirepo/package.json.

The monorepo-symlink-test module has been identified as having a critical security risk:

https://security.snyk.io/vuln/SNYK-JS-MONOREPOSYMLINKTEST-5865510

Are there plans to remove this dependancy, or swap it out with something else?

@spanagiot
Copy link

From what I see and understand they include the source code directly in the test folder and do not depend on NPM. The source code of the included library is here
https://github.com/browserify/resolve/blob/main/test/resolver/multirepo/packages/package-a/index.js
https://github.com/browserify/resolve/blob/main/test/resolver/multirepo/packages/package-b/index.js
and I don't see anything strange.
Please correct me if I'm wrong

@cichelero
Copy link

It seems this is just a confusion. The monorepo-symlink-test dependency is not being used by this package, but there is a package.json in the test folder that has the same name, which generates the confusion:

"name": "monorepo-symlink-test",

I think the simplest solution is to simply remove the name of the test package. However a cleaner solution is not to ship the test folder with the NPM package.

@ljharb
Copy link
Member

ljharb commented Sep 4, 2023

Because it's a private package that just coincidentally has the same name as the malicious one, it is a false positive - so whatever tool is flagging this repo is broken, and you should strongly reconsider using a tool that is this naive about npm package names.

Duplicate of #303. Duplicate of #291. Duplicate of #288. Duplicate of #304. Duplicate of #305. Duplicate of #306. Duplicate of #309. Duplicate of #310. Duplicate of #311.

Tests must be shipped with packages so that npm explore foo && npm install && npm test always works.

@ljharb ljharb closed this as not planned Won't fix, can't repro, duplicate, stale Sep 4, 2023
@cichelero
Copy link

Tests must be shipped with packages so that npm explore foo && npm install && npm test always works.

@ljharb I don't understand why this is required for a package. Can you please explain a bit more?

@ljharb
Copy link
Member

ljharb commented Sep 11, 2023

@cichelero so that i can debug an installed package by running its tests, whether i have internet or not, and even if the github repo has been deleted (for example, substack deleted his github recently and a thousand repos vanished).

ljharb added a commit that referenced this issue Oct 10, 2023
…d security scanners

Fixes #319.
Fixes #318.
Fixes #317.
Fixes #314.
Closes #313.
Fixes #312.
Fixes #311.
Fixes #310.
Fixes #309.
Fixes #306.
Fixes #305.
Fixes #304.
Fixes #303.
Fixes #291.
Fixes #288.
ljharb added a commit that referenced this issue Oct 10, 2023
    Fixes #319.
    Fixes #318.
    Fixes #317.
    Fixes #314.
    Closes #313.
    Fixes #312.
    Fixes #311.
    Fixes #310.
    Fixes #309.
    Fixes #306.
    Fixes #305.
    Fixes #304.
    Fixes #303.
    Fixes #291.
    Fixes #288.
ljharb added a commit that referenced this issue Oct 10, 2023
    Fixes #319.
    Fixes #318.
    Fixes #317.
    Fixes #314.
    Closes #313.
    Fixes #312.
    Fixes #311.
    Fixes #310.
    Fixes #309.
    Fixes #306.
    Fixes #305.
    Fixes #304.
    Fixes #303.
    Fixes #291.
    Fixes #288.
elliotttf added a commit to elliotttf/knex that referenced this issue Jun 21, 2024
This replaces the dependency on resolve-from with node's native
`require.resolve` method.

If this and gulpjs/rechoir#47 are merged, a false positive related to
[resolve's `monorepo-symlink-test`](browserify/resolve#312)
file will be resolved.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

4 participants