Skip to content

Increasing visibility of Node.js security patches on node-private #1687

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
RafaelGSS opened this issue Feb 12, 2025 · 8 comments
Closed

Increasing visibility of Node.js security patches on node-private #1687

RafaelGSS opened this issue Feb 12, 2025 · 8 comments

Comments

@RafaelGSS
Copy link
Member

I've been doing security releases for quite a while and to be honest, it's a bit frustrating to not have enough people reviewing the patches before they go out. The reason for that is that reviewing that PR is... time-consuming. One would need to read the HackerOne report and have an understanding of that particular piece of code to review it properly -- despite the fact most TSC members do not have much time to spend on those scenarios.

That said, I wonder if we could find a way to improve the current situation. I believe that using GitHub Advisories for patches can be good as we could invite external people (with context on the particular patch) to review + the report. I just don't know if we can run Jenkins CI on it -- It also needs to be checked by the automation as it expects the PR to be created under node-private.

cc: @nodejs/tsc @nodejs/security

@RafaelGSS
Copy link
Member Author

I think to not change the process too much, we could at least make a procedure to announce PRs that need review before going public at every TSC meeting.

Wdyt?

@mhdawson
Copy link
Member

Mentioning them in the TSC meeting sounds ok to me

@mcollina
Copy link
Member

mcollina commented Feb 22, 2025

The problem is that we can't run CI on GitHub Advisories (at least right now), making them useless for our use case ad they would lead to more work.

Instead, I think we could:

  • expand the people with access to node-private
  • add them as-needed to the hackerone reports

@bnoordhuis
Copy link
Member

The reason for that is that reviewing that PR is... time-consuming

Agreed, fortunately there's an easy solution: pay reviewers for their time.

Basically all problems around here boil down to "we need more people" but that's like the easiest problem in the world to solve: just throw money at it.

People already like hacking on node. The foundation just needs to make it worth their while and not be so stingy.

@ljharb
Copy link
Member

ljharb commented Feb 27, 2025

You may be vastly overestimating how much money the foundation has available - Robin gave a talk on it at the last NodeConf EU - i couldn't find a recording, but https://www.igalia.com/chats/magic-piles-of-money is a podcast where they talk about it.

@jasnell
Copy link
Member

jasnell commented Feb 27, 2025

... The foundation just needs to make it worth their while and not be so stingy.

@ljharb was more diplomatic in his response to this than my initial reaction to this comment. The foundation isn't being stingy. If you want the foundation to be helping to pay for these kinds of things, then there are opportunities for you to help the foundation raise more funds. Money doesn't magically appear.

https://www.youtube.com/watch?v=Yq2hEseP-Ck&list=PLFVadYWYE9opLgYJ7i0j50oIgn6pqBOM7&index=15

@Dustin4444
Copy link

I've been doing security releases for quite a while and to be honest, it's a bit frustrating to not have enough people reviewing the patches before they go out. The reason for that is that reviewing that PR is... time-consuming. One would need to read the HackerOne report and have an understanding of that particular piece of code to review it properly -- despite the fact most TSC members do not have much time to spend on those scenarios.

That said, I wonder if we could find a way to improve the current situation. I believe that using GitHub Advisories for patches can be good as we could invite external people (with context on the particular patch) to review + the report. I just don't know if we can run Jenkins CI on it -- It also needs to be checked by the automation as it expects the PR to be created under node-private.

cc: @nodejs/tsc @nodejs/security

@bnoordhuis
Copy link
Member

You may be vastly overestimating how much money the foundation has available

Quite the opposite; it's half a million more than I remembered! What probably didn't change is that a lot just kind of leaks away.

Consider this: set aside a mere 15% of that $2.9 million and let people bill at $100/hr. That's enough money to allow 10 people to bill 8 hours every week of the year. Imagine how much more work would get done.

RafaelGSS added a commit to RafaelGSS/node that referenced this issue Mar 4, 2025
nodejs-github-bot pushed a commit to nodejs/node that referenced this issue Mar 6, 2025
Refs: nodejs/TSC#1687
PR-URL: #57309
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
aduh95 pushed a commit to nodejs/node that referenced this issue Mar 9, 2025
Refs: nodejs/TSC#1687
PR-URL: #57309
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
aduh95 pushed a commit to nodejs/node that referenced this issue Mar 9, 2025
Refs: nodejs/TSC#1687
PR-URL: #57309
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
RafaelGSS added a commit to nodejs/node that referenced this issue Apr 1, 2025
Refs: nodejs/TSC#1687
PR-URL: #57309
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
RafaelGSS added a commit to nodejs/node that referenced this issue Apr 1, 2025
Refs: nodejs/TSC#1687
PR-URL: #57309
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
aduh95 pushed a commit to nodejs/node that referenced this issue Apr 2, 2025
Refs: nodejs/TSC#1687
PR-URL: #57309
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
aduh95 pushed a commit to nodejs/node that referenced this issue Apr 3, 2025
Refs: nodejs/TSC#1687
PR-URL: #57309
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
RafaelGSS added a commit to nodejs/node that referenced this issue Apr 16, 2025
Refs: nodejs/TSC#1687
PR-URL: #57309
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
RafaelGSS added a commit to nodejs/node that referenced this issue Apr 17, 2025
Refs: nodejs/TSC#1687
PR-URL: #57309
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants