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

Disable Jenkins CI / git node request CI when no approvals #3696

Open
RafaelGSS opened this issue Apr 30, 2024 · 5 comments
Open

Disable Jenkins CI / git node request CI when no approvals #3696

RafaelGSS opened this issue Apr 30, 2024 · 5 comments

Comments

@RafaelGSS
Copy link
Member

Recently, I received the following message: nodejs/node#52761 (comment) and that's great! However, it doesn't cover all the cases such as requesting it through https://ci.nodejs.org/ or git node. As a security measure, how hard would be to include both in these rules?

cc: @nodejs/security-wg

@aduh95
Copy link
Contributor

aduh95 commented Apr 30, 2024

That doesn't seem desirable, it's useful to be able to start CI runs – that's even a reason often mentioned when suggesting onboarding new collaborators.

@MoLow
Copy link
Member

MoLow commented May 1, 2024

Well ci.nodejs.org and git node uses your own GitHub credentials, which are validated and authorized within Jenkins.
however, the https://github.com/nodejs/node/labels/request-ci label uses the GitHub bot credentials - so it makes sense to only limit that

@joyeecheung
Copy link
Member

joyeecheung commented May 1, 2024

I am -1 because that means no CI can be run to verify that it even works across the platform without an approval. And "approving to land" vs "approving to run in the CI" are completely different things. It also means to get a change that needs to handle platform specifics working, you have to get a PR up and going which may attract unwanted comments in a WIP that's not ready for review (in the issue tracker, even if you put [WIP] and "this is not ready for review" loud and clear in the PR, people still come in to comment on obvious TODOs that are just there because it's WIP), and get spammed constantly by the bot about the CI runs when you iterate over and over on platform failures (which, again, make the thread less useful because it's full of bot noises).

@joyeecheung
Copy link
Member

Also, how much security does this offer? Is it assuming that collaborators can get their GitHub account compromised to start Jenkins? In that case they can also work around it by pushing to some old PR (opened by others) that no one pays attention to, approving the change themselves, and start a CI on it. Or create a fake account to open a PR, and approve it using the collaborator account to run the CI on it.

@mcollina
Copy link
Member

mcollina commented May 1, 2024

I personally think we should revert the changes for the reasons highlighted in

nodejs/node-core-utils#801

I don’t want to wait for another collaborator to LGTM my changes to be able to run them on Windows, AIX, ARM or other odd environments. This will slow us down significantly.

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

5 participants