Proposal to reduce inactive collaborator duration

[anonrig]

The current security incidents around Linux made me realize that we should look into Node.js organization from a different perspective.

The current inactive collaborator duration is 18 months (1.5 years). Collaborators have write access to the repository, access to collaborator private repository and can run CI at any time (and run any code on our infrastructure).

The requirement to keep collaborator status is:

1. Approve a pull-request that landed to main branch
2. Author and land a pull-request to main branch at least once in 18 months

I'd like to reduce the inactive collaborator duration to 6 months or 12 months.

My reasonings are:

• People who have not contributed or reviewed or landed a pull-request for more than 6 months has a high chance to not do it for the remainder of 18 months. Having them as "dormant" creates a security risk.
• Due to the complexity of Node.js and the quantity of commits we land every year (around 2300~ commits in 2023 if I'm not mistaken), it's doesn't seem possible for a collaborator to review a pull-request even though they have not contributed for a really long time, and the validity of that review (or block) might or might not be correct.
• Even though we have Slack channels that we can track/monitor of edge cases like force-pushes, landing pull-requests without changes, it's possible that a security incident might occur, in an area we didn't think of. From a security perspective each member of Node.js organization is a risk similar to what's happening right now in Linux ecosystem. This might be harsh to hear, but in reality we all know in our heart that having access to CI (through request-ci or custom tasks) pose serious security risk.
• Throughout my Node.js contributing experience (referencing the past 2-3 years), I've seen contributors who never contributed block certain pull-requests that frustrate Node.js contributors, and even resulted in several resignations.

My logic might be flawed, and 6 months or 12 months might not be the correct duration, but I think we have the obligation (to the users of Node.js) to think about collaborator membership and security of it soon.

I believe that we should at least discuss/think about this in the short term.

cc @nodejs/tsc

[joyeecheung]

Back in 2018 at one collaboration summit I proposed to revoke the commit bit if it has not been used to commit anything in 3 months: https://github.com/joyeecheung/talks/blob/master/node_collab_summit_201805/core_collaboartors_status_and_scope.pdf (when I investigated in 2018, ~50% of the collaborators would be affected if we did that). Collaborators could request to go back if they started sending PR again.

Didn't follow up because there were some pushbacks.

[benjamingr]

Collaborators have write access to the repository, access to collaborator private repository and can run CI at any time (and run any code on our infrastructure).

As a counter point I don't think this was ever abused. Releasing and security issues which are the riskier bits both require different steps from being a collaborator (e.g. release team, tsc) that have shorter inactivity deadlines and removing people doesn't buy us too much as a result.

Before we do this, I want to make sure it doesn't alienate people with periods of inactivity - can we run a query to check what active collaborators had a period of inactivity and returned from such a period? (Not sure how hard it is to check since a bunch of people mostly review)

[MoLow]

+1 on changing this.
maybe we can make re-nomination easier. for example, requiring more than a single collaborator to block it

based on https://github.com/nodejs/node/blob/main/tools/find-inactive-collaborators.mjs
if we change to 12 months of inactivity:

Inactive collaborators (10):

* Anto Aravinth
* Ash Cripps
* Bradley Farias
* Daniel Bevenius
* Gus Caplan
* Ian Sutherland
* Brian White
* Ricky Zhou
* Ujjwal Sharma
* Masashi Hirano

if we change to months of inactivity:

Inactive collaborators (19):

* Anto Aravinth
* Ash Cripps
* Bradley Farias
* Сковорода Никита Андреевич
* Daniel Bevenius
* Gus Caplan
* Feng Yu
* Ian Sutherland
* Yoshiki Kurihara
* Mestery
* Alba Mendez
* Brian White
* Ricky Zhou
* Ujjwal Sharma
* Masashi Hirano
* Tiancheng "Timothy" Gu
* Vladimir de Turckheim
* Khaidi Chu
* Yash Ladha

Note Ian Sutherland has already passed the 18 months inactivity period: nodejs/node#52300

[mcollina]

I don't think reducing the inactivity period will reduce the risk of a rogue collaborators.

We should document how we are doing this (code reviews, release reviews, minimum 2 weeks before a commit goes to a LTS release, a tight group of releasers).

[UlisesGascon]

I don't think reducing the inactivity period will reduce the risk of a rogue collaborators.

Strongly Agree

Collaborators have write access to the repository, access to collaborator private repository and can run CI at any time (and run any code on our infrastructure).

While write access grants certain privileges, there are other control measures in place that limit the capabilities for hostile operations.

Firstly, there are two CI systems (https://ci.nodejs.org/ and https://ci-release.nodejs.org/) with different Project-based Matrix Authorization Strategies.

In the ci, collaborators are limited to the following:

Reference

While in the ci-release, collaborators are not allowed at all, as it is limited to Jenkins administrators (sub-group) and releasers.

On the other hand, even if a malicious collaborator is capable of landing commits abusing write access, there are certain ways to detect it, like the OSSF Scorecard already in place that the Security WG reviews every two weeks (example). But I strongly believe that before that, any TC or collaborator will detect that "unexpected change" in the main branch or some alerts in Slack will bring our attention.

Moreover, even if this change goes unnoticed, the commit won't be released automatically to the Node.js users. As with any release in Node.js, we are required to follow these 20 steps. As part of these steps, the releasers will first cherry-pick the commits based on certain criteria and then elaborate a PR that anyone can review (see). These PRs tend to be open for days and are quite well-reviewed and commented by the collaborators.

My two cents, as @mcollina said, the key here is to document and review these protections and processes that we have already in place to prevent bad things (accidental stuff, human errors, malicious actors...) 😊

[MoLow]

the benefit of this suggestion might not be huge, but if it is going to affect ±20 people there is not a big downside either

[richardlau]

One of the hopes of the commit queue was to perhaps reduce the need to grant so many people write access to the repository.

[mhdawson]

@UlisesGascon thanks for providing the outline of some of the things that we do that reduce the risk of a rogue collaborator.

If we do make a change I think 6 months is too short but would be ok with 1 year for inactivity.

[anonrig]

If we do make a change I think 6 months is too short but would be ok with 1 year for inactivity.

@mhdawson Why do you think a year is too short? Can you elaborate?

[anonrig]

While write access grants certain privileges, there are other control measures in place that limit the capabilities for hostile operations.

@UlisesGascon Having write access (meaning executing/running CI jobs) gives users to run any arbitrary code on the proposed infrastructure. We also talked about non-collaborators being/not being on the release team for example due to similar concerns. So, I don't quite understand how write access can be preventable with other control measures.

[mhdawson]

@anonrig I can easily see people taking a break (for example to care for a new child etc.) who will come back getting removed by the 6 month check. A 12 month check makes this a lot less likely in my opinion.