New fork resolving vulnerabilities and incorporating most current PRs about node-static HOT 33 OPEN

I'm well aware of supply chain attacks.. but basically a big part of that responsibility is on users auditing their dependencies. I did my duty of making sure @brettz9 is a legitimate contributor to the open source community (which he clearly is), and so I think he will do a great job. I wouldn't have given write access to someone with no credentials or github history!

from node-static.

but basically a big part of that responsibility is on users auditing their dependencies.

Indeed @cloudhead you are right about all that, and @brettz9 is probably very good maintainer for this project! Happy you guys found each other :)

My remark was actually unspecific to you guys as a person or this situation, and I felt similarly it being my duty in such a case to remind all participants about needed trust concerns of such a handover. Which is done, and I agree everything seems all fine here 👍

from node-static.

@cloudhead I did it! I converted all history since 2018 (i.e. everything @brettz9 pushed) into a linear tree. You can take my cleaned history with:

git checkout -b zarel-master master
git pull https://github.com/Zarel/node-static.git zarel-master
git checkout master
git reset --hard 6efac07ba8c01

You can then go ahead and do an interactive rebase with git rebase -i 83aac2e to clean the history to your liking.

from node-static.

@brettz9 thanks! I'll push a new release now

@cloudhead the last release on NPM appears to be 0.7.11 from 3 years ago. I was wondering if you could push a new release now? It would be really nice to have a version without the DoS vulnerability.

from node-static.

not sure what @cloudhead s view on it is. (i have done stuff on this code, but am not a maintainer)

maybe he should officialise the lack of maintainership on this project?

if not hand over the repo here - I guess making clear this repo is not maintained anymore - so that other people who want to continue based on the work of this fork for example can gather crowd around them with a clear direction?

i guess anyone can just use the published fork dist packages, but some clear direction where this is going and who to follow or who not is the point @bacloud14 ?

from node-static.

Ok, this sounds good. I would then propose that I give you write access to merge this fork into master, and make any fixes that may be needed in the future, given you have the time/capacity. We can then release this as version 0.8.

from node-static.

If you want, we could also bump engines to Node 12 now though as it has reached end-of-life.

from node-static.

bring our tests up to 100% coverage so we can be more confident that with any new features/fixes come no new problems.

that would be a nice summertime 2-day code sprint or so :) need to see if i can shove it in.

from node-static.

I went ahead and amended/rebased until before the last merge. If you want to go back further, it would be great if you could handle that. Thanks!

from node-static.

Let's get ESM and the cli changes in before publishing

from node-static.

sad that this stays unmerged.

from node-static.

Agreed. I cannot contribute on this in anyway. I just notified maintainer's because frankly, the guy made what seemed to me a big fork.

from node-static.

Hey, what would be everyone's preference here? The problem is I don't have the capacity to review such a big change. I'd be happy to pass on maintainership to someone. Does this fork have any breaking API changes? (besides the "engines" change)

from node-static.

Thanks for your reply, @cloudhead .

Hey, what would be everyone's preference here? The problem is I don't have the capacity to review such a big change.

For anyone wishing to review, most of the changes were a series of merges for PRs already here and some non-API-changing refactoring. But I understand there were a number of merges, and I created however inevitable diff noise with the refactoring.

I'd be happy to pass on maintainership to someone.

If no one else is interested, I can accept, but knowing that:

1. I don't have plans to very actively develop it, perhaps just responding to bug fixes with tests or only as feasible for me to review others' PRs (beyond those already reviewed and merged).
2. I have some health issues which make me fickle in energy
3. While I was careful in the changes I made (which besides some refactoring were mostly just merges of others' work that I reviewed), and added nyc to give us an idea of areas where we need coverage, we don't have full testing coverage, and I can't claim deep familiarity already with all the internals.

Does this fork have any breaking API changes? (besides the "engines" change)

No. It uses ES6+ features, so for someone ignoring the stated engines requirement, it might not work for that reason as well as due to the dependency updates and additions having different Node requirements, but there should be no breaking API changes affecting users except those earlier Node users.

Of course with all the changes, e.g., the CLI parsing engine switch, it's always possible there could be some subtle differences, but it's been working for me in my projects, and if there really was any breakage accidentally introduced, my intent would be to fix it.

from node-static.

Thanks for that, @cloudhead

I've made the tweaks I think I'm ready to make at this point, so if you want to add the 0.8.0 release, I think we'd be ready with these steps (which I mention given that they involve files which weren't there before):

1. Update package.json version
2. Update new package-lock.json version
3. Update version at top of new CHANGES.md

Note that there still is an advisory per https://www.npmjs.com/advisories/1206/versions which I wasn't able to replicate (on the contrary, it seemed already fixed), so barring any test case, I'm not sure what we can do there, but the release should at least address the other reported vulnerabilities.

from node-static.

Ok nice @cloudhead @brettz9 :)

I don't want to be the devils advocate here, but

I'd be happy to pass on maintainership to someone.

made me remind that story 2-3 years ago (how fast we forget these things as well right ...?)

where some guy took over maintainership of a package from an overworked open-source dev

and eventually used it to inject a Trojan if I remember well, into some crypto-currency platforms backend

I am still not sure what the FLOSS & NPM communitys take on this is tbh.

from node-static.

see

• https://blog.npmjs.org/post/182828408610/the-security-risks-of-changing-package-owners
• https://blog.npmjs.org/post/180565383195/details-about-the-event-stream-incident

from node-static.

There are no guarantees that even the regular owner of a project won't suddenly turn into a malicious person. One either has to:

1. Trust the management of a project
2. Speak up if you think that a new maintainer such as myself is suddenly going to play malicious games. (Though in my defense, you might question whether the lead contributor of a well-downloaded project (eslint-plugin-jsdoc) is so likely to do so.)
3. Peg your projects to the old version, or
4. Review the changes yourself since the last commit you know to be working.

from node-static.

@cloudhead : I bumped to Node 12. Anything else you want for a release?

from node-static.

@brettz9 Let me know if you need any additional hands on doing some quick codework.

from node-static.

Thanks, @tchakabam . Feel free to take up any issue, or in some ways, better yet, bring our tests up to 100% coverage so we can be more confident that with any new features/fixes come no new problems.

Due to my own limitations, I can't do very much, but hope I can at least review well-explained PRs.

from node-static.

Hi @cloudhead ,

Since the work is already done, I'm a bit eager to move on with this and get this out there for the community if that is at all possible.

If you have the time to do whatever setup or final reviewing you might want to do, I think it'd be nice to get these vulnerabilities fixed up for the community.

Do you think setting up semantic-release might free you up from having to worry about future releases or is there anything left you'd like to do before a release?

from node-static.

@brettz9 thanks! I'll push a new release now

Do you think setting up semantic-release might free you up from having to worry about future releases or is there anything left you'd like to do before a release?

I looked through the commit history, and wonder why some of the commits have a dash ("-") in front of them? We should fix that before using something like semantic-release especially to make sure the changelog is rendered properly.

from node-static.

Hadn't thought about building up the past history with such tooling, but makes sense. (In my non-semantic-release projects, I've liked to use them to set off my sometimes adding multiple items to a single commit.)

I can rebase the history to remove this (as well as convert the multi-entry commits to perhaps list the most important item, with the lesser items in the footer), though such rebasing would break master for anyone who's forked it.

from node-static.

Yeah, I think that's a small price to pay for the history to be consistent, also happy to do the formatting if you don't have time.

from node-static.

Ok, with ESM and CLI changes added, I think it should be ready AFAIC.

I'm not feeling inclined to add all kinds of tests for the binary files now, but:

1. I should be able to quickly resolve if anything comes up with the new CLI processing
2. I at least added what I think should be a convenient testing utility whereby one can await a Promise which executes the binary and ceases waiting once a condition may be met (e.g., the accumulated stdout includes "serving ..."), then executes and awaits an action as part of the test (e.g., a fetch to the server) before resolving to an object against which assertions can be made (about the resolution of the action (e.g., a fetch response) and/or the stdout). I added one example. Hopefully this may encourage us to check the binary more over time as well as complete coverage of the programmatic API.

(I probably should have checked for other such testing utilities, but haven't come across one, and I think the API is pretty clean.)

from node-static.

I've also added a GitHub Action file, if you wanted to enable workflows in the repo Settings, @cloudhead .

from node-static.

@brettz9 please pay attention to your commit messages, eg. this doesn't work as a commit message: c8a003a

I'm trying to go through the history to fix everything but somehow it's causing issues..

from node-static.

Ah, ok, apologies, I'm used to using my own format a lot of times. I think maybe test should be testing if you plan to use semantic-release. I might also have left some line breaks but I don't think semantic-release has a problem with those even though some commit checkers do.

from node-static.

I'm ok if we don't follow semantic-release to the letter, but at least the commits should be formatted according to the way git works, ie. it shouldn't break git log --oneline for example, and the commit title and description should be clearly separated, otherwise it's hard to read for humans too.

from node-static.

I barely use the command line for Git, but will try to remember to adhere to it. Alternatively, it should be doable as per https://github.com/semantic-release/semantic-release#commit-message-format , to add commitizen or commitlint to enforce valid commit messages before they are committed (including possibly via husky).

from node-static.

I've tried to clean up the history but unfortunately because of all the merges it's very very difficult to rebase (even with the help of some pretty advanced git commands). So I'm left with two choices:

1. Squash the new commits from the fork into one giant commit
2. Leave everything as-is, do nothing

Neither is ideal for me, but (1) would at least mark a clear separation between the old and the new node-static and it would be easy to follow the change. We would of course lose the history, but since it's the history that is not particularly usable as-is, I don't see that as a major downside.

from node-static.

@cloudhead I should be able to do it for you! Please just hold on a moment.

from node-static.