search

RafaelGSS / is-my-node-vulnerable

package that checks if your Node.js installation is vulnerable to known security vulnerabilities
MIT License
180 stars 6 forks source link

feat: warn if not an actively supported Node.js version #5

Closed srl295 closed 1 year ago

srl295 commented 1 year ago

Fixes: #2 Ref: https://github.com/nodejs/security-wg/issues/871

Example output:

Not an actively supported Node.js version: v17.9.1
srl295 commented 1 year ago

Looks good! Could you please include a test for it (look at test.js)?

Do you want some type of API? Or to have it always return true as vulnerable for out of active major versions ?

ljharb commented 1 year ago

You'll still need to remove the node: prefixes if you don't want it to crash in unsupported nodes.

RafaelGSS commented 1 year ago

Looks good! Could you please include a test for it (look at test.js)?

Do you want some type of API? Or to have it always return true as vulnerable for out of active major versions ?

true as vulnerable for end-of-life versions should be enough

srl295 commented 1 year ago

Looks good! Could you please include a test for it (look at test.js)?

Do you want some type of API? Or to have it always return true as vulnerable for out of active major versions ?

true as vulnerable for end-of-life versions should be enough

Hey, I really think this is a great tool and appreciate your work on security… I'm still puzzled as to why end of life keeps getting mentioned when that's not the only case…

RafaelGSS commented 1 year ago

Hey, I really think this is a great tool and appreciate your work on security… I'm still puzzled as to why end of life keeps getting mentioned when that's not the only case…

I'm not sure what you mean. This tool will cover all the cases (after this PR). The only request here is to update the message and include a test for this, that's all. Active release lines will be supported, end-of-life will show "Danger", and vulnerable versions will also show "Danger". That covers pretty much everything, uh?

ljharb commented 1 year ago

I'd suggest showing a different message for EOL - iow, "danger" for when you are KNOWN to be in danger, and maybe "risk" when you are LIKELY to be in danger but we don't know for sure?

srl295 commented 1 year ago

Hey, I really think this is a great tool and appreciate your work on security… I'm still puzzled as to why end of life keeps getting mentioned when that's not the only case…

I'm not sure what you mean. This tool will cover all the cases (after this PR). The only request here is to update the message and include a test for this, that's all. Active release lines will be supported, end-of-life will show "Danger", and vulnerable versions will also show "Danger". That covers pretty much everything, uh?

Node 20 was the example I thought of, if someone DID call this with node v20 for some reason it would be incorrect to say it was 'EOL'.

srl295 commented 1 year ago

I'd suggest showing a different message for EOL - iow, "danger" for when you are KNOWN to be in danger, and maybe "risk" when you are LIKELY to be in danger but we don't know for sure?

Yes. This exactly. We don't know, which is different than EOL.

ljharb commented 1 year ago

or maybe "UNKNOWN"

srl295 commented 1 year ago

so we get this now: (when #6 is merged)

You may be at risk. v15.14.0 is not an actively supported Node.js version, so unable to check vulnerabilities.

(14 ALL GOOD)

v13.14.0 is end-of-life. There are high chances of being vulnerable. Please upgrade it.

v12.22.12 is end-of-life. There are high chances of being vulnerable. Please upgrade it.
RafaelGSS commented 1 year ago

@srl295 v15 is end-of-life as well, it must have the same message as v13 and v12. See https://github.com/nodejs/Release#end-of-life-releases

srl295 commented 1 year ago

PTAL. tests

srl295 commented 1 year ago

@srl295 v15 is end-of-life as well, it must have the same message as v13 and v12. See https://github.com/nodejs/Release#end-of-life-releases

not clear that it must.. v13 was never an LTS. But in any event, how would I tell? Should I check if it's less than the MAX supported major ? i.e. < 19 and not supported therefore EOL? That would only show 'unknown' for a v20.

ljharb commented 1 year ago

a non-LTS version is EOL the instant the next major comes out; it's supported while it's the latest major tho.

srl295 commented 1 year ago

a non-LTS version is EOL the instant the next major comes out; it's supported while it's the latest major tho.

does the end: field from nv reflect this properly? that's what i'm using, for example it says v15 is EOL 2021-06-01T00:00:00.000Z… but v16 released on 2021-04-20

how would you recommend improvement here?

srl295 commented 1 year ago

it now reports v15 as EOL and danger.

RafaelGSS commented 1 year ago

@srl295 would you like me to include a commit in your patch? I'm planning to release the package today with this change

srl295 commented 1 year ago

@srl295 would you like me to include a commit in your patch? I'm planning to release the package today with this change

Go ahead - I wasn't able to patch further yet

RafaelGSS commented 1 year ago

You haven't allowed maintainer changes in your PR, so I had to create a new PR. I've included you as the author. Thank you @srl295!

RafaelGSS commented 1 year ago

For reference, when opening a PR you can let the maintainers update your fork branch by selecting a box in the right corner. See: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork

srl295 commented 1 year ago

For reference, when opening a PR you can let the maintainers update your fork branch by selecting a box in the right corner. See: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork

I meant to check that, was an accident

ljharb commented 1 year ago

It's checked by default.