build
build copied to clipboard
nodejs
Disable Jenkins CI / git node request CI when no approvals
Recently, I received the following message: https://github.com/nodejs/node/pull/52761#issuecomment-2087106560 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
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.
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
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).
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.
I personally think we should revert the changes for the reasons highlighted in
https://github.com/nodejs/node-core-utils/issues/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.
