Problem/Motivation
Following issue #3500259: Employees of TATA Consultancy Services are bulk posting requests to gain (co-)maintainer access to modules has shown how easy it is to become a (co-) maintainer of a module. In a few instances, an unverified new user with next to no track record was added as a maintainer to a module, some of which have 35K+ installations.
This raises the concerns of a man in the middle/supply chain attack where malicious code is being added and releases without anyone really noticing. (Reference: https://cloud.google.com/blog/topics/threat-intelligence/supply-chain-no...). The difference being it's not the account of an maintainer which is being highjacked, but the actor is just being added as a maintainer.
Steps to reproduce
https://www.drupal.org/project/image_url_formatter/issues/3496527
Proposed resolution
This should be the outcome of the discussion here.
Remaining tasks
Propose a policy change or technical change to at least make it less easy for this to happen.
User interface changes
TBD
API changes
TBD
Data model changes
TBD
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | Screenshot_20250116_145348.png | 36.21 KB | mcdruid |
Comments
Comment #2
bramdriesenComment #3
gregglesRe-titling. It seems important to focus on the goal (safer additions) than a potential solution (harder additions). It's possible a solution will be that it is somewhat harder to be added as co-maintainer, but just making it harder alone doesn't make it safer.
Comment #4
smustgrave commentedMaybe a checklist could help?
1. Are they verified
2. Are they part of a company on the community radar for concern?
And there may need to be some governance around if the DA can remove someone. If a maintainer isn’t paying attention and adds such a user, like your example, and they do something that impacts 35K sites. DA should remove them till criteria is met.
Comment #5
mcdruid commentedIf it's technically possible, we should disallow adding users that are not yet verified as a maintainer of a project.
Comment #6
gregglesThere are docs pages already, we could definitely add some suggested items to the template. I'd be more +1 to the idea of using "git verified" as a hurdle if that process were more efficient and transparent to end users (e.g. a quiz, being able to apply with significant MRs to core/contrib).
Comment #7
mcdruid commentedJust to be clear, by "not yet verified" I don't (necessarily) mean git verified; I was thinking more about this "NEW" status:
I'm not sure how that's reflected in d.o's schema but looks like the proper term is "unconfirmed":
https://www.drupal.org/drupalorg/docs/user-accounts/become-a-confirmed-user
Adding a user with that status as a maintainer seems fairly bonkers, to use a technical term.
Comment #8
bramdriesenRe #4
I don't think that's really a trustable check. I can create a new account and register myself as an Acquia employee on Drupal.org. There is no check involved here. I remember seeing an issue about this somewhere to allow orgs to enable a setting that they have to verify users. But I don't think it ever went further as just an idea. Might have been a slack discussion as well.
And Re #7 I think the users in the example listed in the IS actually already got "confirmed" by someone (probably from their org) as the green button to confirm no longer shows.
I believe anyone with the git vetter role can verify users.And some info about the Community role:
Source: https://www.drupal.org/node/2455913
Comment #9
gregglesAh, that makes sense. +1 to that as a base level.
Personally I've generally asked applicants to go through the queue and post patches to the things they are likely to fix first, to provide feedback on proposed feature issues of whether an idea fits with the philosophy of the module or not, to do reviews of patches other people have provided, to create a "next release" plan issue and highlight what they would commit in that next release. Those create more material for the current maintainer to at least skim over before granting access. They would also get the person past being "new" without using that specifically as the barrier.
Comment #10
cmlaraFiling as a child of #3452326: [META] Increase Security of Project Ownership Transfer Process
Regarding #4 and #8
#3402577: Add ability for organizations to remove people was opened to require orgs to approve users, it was rejected by Drupa Infra and narrowed to only allowing them to self delete users.
Comment #11
bramdriesenThis hasn't really anything to do with project ownership transfer, which is only really used for abandoned modules if I'm not mistaken. But it for sure is related 🙂.
I'm following #3402577: Add ability for organizations to remove people so also related, but no the issue I had in the top of my mind 🙃 I'll check tomorrow if I can find it.