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

Use native options of dependencies when available #19

Open
trivikr opened this issue Oct 20, 2024 · 7 comments
Open

Use native options of dependencies when available #19

trivikr opened this issue Oct 20, 2024 · 7 comments

Comments

@trivikr
Copy link
Member

trivikr commented Oct 20, 2024

Is your feature request related to a problem? Please describe.

Since is-my-node-vulnerable is officially recommended by Node.js, it would be better to have as less dependencies as possible.

Describe the solution you'd like

Use native options of dependencies when available

Additional context

Discussion on Twitter https://x.com/styfle/status/1847469788236255562

@RafaelGSS
Copy link
Member

undici is an official Node.js dependency. I don't see why changing it to node:https will change things. If undici is vulnerable, Node.js is vulnerable (also npx). A different and more reasonable solution would be that we won't need to download a module making npx is-my-node-vulnerable fast, but it possible uses cache.

@trivikr
Copy link
Member Author

trivikr commented Oct 20, 2024

The feature request, and the discussion on Twitter, is not about the vulnerabilities the Node.js version being tested - but the ones used by is-my-node-vulnerable module.

@RafaelGSS
Copy link
Member

Yes, I understand. However, I'm still not convinced that removing undici in favor of node:https will enhance security. Upgrading undici might be a better option.

@trivikr
Copy link
Member Author

trivikr commented Oct 20, 2024

Yes, I understand. But upgrading undici, or any other dependencies, may conflict with the minimum Node.js version which is-my-node-vulnerable aims to support.

Ideally, this module should support some EOL Node.js versions, as it's aimed to inform users whether their Node.js version is vulnerable. But it's dependencies may (and likely) drop support for EOL Node.js versions sooner.


There's no package.json engines field for this module, and I couldn't find minimum supported Node.js version in README. As per #6 (comment), I think this module aims to support Node.js 12+. Undici 6.x supoports Node.js 18+, for example https://github.com/nodejs/undici/blob/e218fc61eda46da8784e0cedcaa88cd7e84dee99/package.json#L137-L139

RafaelGSS added a commit that referenced this issue Oct 22, 2024
RafaelGSS added a commit that referenced this issue Oct 22, 2024
RafaelGSS added a commit that referenced this issue Oct 22, 2024
RafaelGSS added a commit that referenced this issue Oct 22, 2024
RafaelGSS added a commit that referenced this issue Nov 1, 2024
* feat: add compatibility to Node.js >= 0.12

For retro-compatibility lovers

* doc: add note about supported Node.js versions

* fixup! feat: add compatibility to Node.js >= 0.12

* chore: drop cli-colors

Refs: #19

* fixup! extra forward slashes

Co-authored-by: Trivikram Kamat <[email protected]>

* fixup! feat: add compatibility to Node.js >= 0.12

---------

Co-authored-by: Trivikram Kamat <[email protected]>
RafaelGSS added a commit that referenced this issue Nov 1, 2024
@styfle
Copy link
Member

styfle commented Nov 1, 2024

I just realized that you're bundling all the code with ncc before publishing to npm, therefore you could move all dependencies to devDependencies and it will still work.

https://github.com/RafaelGSS/is-my-node-vulnerable/blob/9dad0803755768664fa913c7956353208e82cded/package.json#L14

@RafaelGSS
Copy link
Member

The dist is only used for github actions, when users attempt to npx is-my-node-vulnerable it's supposed to install package.dependencies otherwise it will fail. Am I missing something?

RafaelGSS added a commit that referenced this issue Nov 1, 2024
trivikr pushed a commit to trivikr/is-my-node-vulnerable that referenced this issue Nov 1, 2024
trivikr pushed a commit to trivikr/is-my-node-vulnerable that referenced this issue Nov 1, 2024
@styfle
Copy link
Member

styfle commented Nov 2, 2024

I see. In that case, moving the GitHub Actions usage into its own package might be better.

RafaelGSS added a commit that referenced this issue Nov 3, 2024
* chore: remove debug dependency (#25)

Part of: #19

* Use response.pipe instead of stream.pipeline

* Add error handler for fetchCoreIndex and call end()

* Remove variable req in getCoreIndex()

---------

Co-authored-by: Rafael Gonzaga <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants