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

Proposal: Smaller Publishing Teams on crates.io #21

Merged
merged 2 commits into from
Oct 21, 2020

Conversation

michaelkirk
Copy link
Member

@michaelkirk michaelkirk commented Oct 15, 2020

Proposal: Smaller Publishing Teams on crates.io

As the organization grows, georust crate owners should restrict crates.io publishing permissions to a team with only the specific maintainers of the crate rather than the entire org.

This is a proposal, so please let me know what you think, including whether you support going forward with this, whether you think we should leave things as they are, or whether you have alternative suggestion or amendments.

If there seems to be broad support and no blocking objections, I'll plan to merge this in a week, and create issues as TODO's in each of our repos. Since it's an opt-in crate-by-crate process, my hope is that it won't be pulling the rug out from under anyone.

For the tldr; you can just read the GUIDELINES.md diff, but if you want more background and justification, read on!

Background (as I understand it)

In the beginning, we had our little georust, and it was a very small thing — just a handful of contributors and repositories. In terms of authorization, there is only one GitHub team, georust/core, and every new member is added to it. This team has read/write/admin access to every georust repository.

This inclusive approach has helped interested folks to go beyond just the feature or bugfixes they were originally focused on, and graduate to a role of more holistic ownership - triaging issues, reviewing and merging other contributor's PRs, etc. It's arguable that these permissions could or should be further refined, but that is not the scope of this proposal.

Typically, when someone installs a crate using cargo it is being pulled down from crates.io, the default repository of rust packages. When a release is ready, a crate owner will publish their crate to cargo. Though crates.io is a separate entity from GitHub, it has some faculties for integrating with GitHub teams for authorizing who can publish a crate.

Several of our crates have gone through ownership transitions. For whatever reason the original author loses interest or time or whatever to maintain the library. That's OK! It's common to feel guilty about this, but really the fact that you were generous enough to donate your time in the past should not bind you to donate more of your time now.

In a perfect world, any ownership transition will be an explict action at a well defined convenient time, and the already burdended previous owner will somehow prioritize the effort of identifying a new owner and transferring the project to them. But because the world we live in is not perfect, transitions are usually gradual and not explicit, maybe you fully intend to get back to it, but just haven't found the time to review that critical bug-fix PR that's been open a couple months... for this reason (almost) all georust crate owners have added the georust/core team as a "team owner" on crates.io. This serves a dual purpose - one is simply that it's a convenient way to give collaboraters publishing access, but it also serves as a sort of insurance that, should something happen, a georust crate doesn't have to become abandoned.

Fast forward to today. As of writing this, the organization has had 1,139 commits in the last year across 23 active repositories. The organization (and the georust/core team) has grown to 47 members.

To restate incase you missed it, the current situation is that any of the 47 georust members can publish any crate in the org, and thus control what software runs when a downstream user cargo installs.

One thing about GitHub is that it's fairly transparent. Let's say someone inadvertently pushes their work-in-progress branch to georust/geo@master. Oops, that's certainly not good, but it's relatively recoverable. The commit is visible on GitHub, other developers can see it when they pull down the repo - we can revert the commit, talk to the errant developer and move on with our lives. This proposal doesn't intend to change anything about that process.

Now compare that scenario to someone publishing a bad crate to crates.io. The specific changes will be less visible, and even after a fix is published, anyone who had installed the bad version will continue to use the bad version until they explicitly cargo update again, or even longer if they or one of their dependencies have unwittingly pinned to the bad version.

Granted, the same concern applies to folks installing crates directly from GitHub; They too could install a bad version, but I would argue the stakes are lower. For one, most people aren't installing via GitHub in the first place. But secondly, I would also argue that our downstream users have a broader assumption of scrutiny, stability, and quality for software published to crates.io vs. whatever bleeding edge code is available from the current head of master.

I think we have an opportunity to mitigate a future catastrophe by decoupling repository management permissions from publishing permissions while having a low probability of negatively impacting anyone's actual day-to-day workflow. My hunch is that some of the 47 people in georust don't even realize they have this ability nor want it. It would be a tragedy if one of them inadvertently or otherwise published or removed a crate when they shouldn't have.

Proposal Specifics

In order to maintain the publishing redundancy that we achieve with the current georust/core team, each crate should have its own, specific team, e.g. georust/geo-publishers, georust/geojson-publishers.

GitHub team "maintainers" can add/remove team members. I think it makes sense for the crate owners to also be the team maintainers of their publishing team, that way they can add/remove people from their publishing team as they see fit. Whether you prefer to add individual users via cargo owner --add some_user or by adding them to their georust/foo-publishers group is up to them. Consider that in the former case, some_user will be able to edit the crate owners. This could be desirable if you would want this person to succeed you as maintainer in the event that you become inactive.

So specifically, if this is approved, crate owners would need to:

  1. Go to https://github.com/orgs/georust/new-team and create a new team with a
    name like "$foo-publishers" and comment "crates.io publishing". Because you created the team,
    you will automatically be a team maintainer. You should add other members/maintainers to the
    publishers group as they see fit.

  2. Update the crates.io publishing authoriziations

    cargo owner --remove github:georust:core
    cargo owner --add github:georust:$foo-publishers

For example, for the crate I own,
geographiclib-rs, I made the geographiclib-rs-publishers team then ran:

cargo owner --remove github:georust:core
cargo owner --add github:georust:geographiclib-rs-publishers

To streamline maintenance, closely related crates might choose to reuse the same publisher team - e.g. geo,
geo-types, geo-postgis, which all live in the same repository, could all choose to use the georust/geo-publishers team. Similarly, while geos and geos-sys are separate repositories, it would probably makes sense for them to share a single georust/geos-publishers team.

Resiliency

As mentioned, team members cannot add/remove people to the team unless they have the "maintainer" role. But beyond that, the organization's owners, currently Corey Farwell (@frewsxcv) and Stephen Hügel (@urschrei), can also edit any team within the organization. I think this is a desirable feature, because it allows further redundancy if the original project maintainer(s) lose the interest/ability/time to maintain a project in the org. It's similar to why we have historically added georust/core as a crate owner, but with much more limited exposure.

Pros

Greatly reduces the number of people who have access to publish each crate.

Maintains a measure of resiliency for georust crate maintainence - if a crate gets abandoned for whatever reason, the GH organization owners can appoint new publishers.

Maintains the permissive model we have with respect to who-can-do-what on GitHub. (If you'd like to see this change consider making a proposal!)

Since it involves adding a new team and leaving the old team as-is this should be less disruptive for crates, who can make the switch when they are ready.

Cons

I'm curious about what downsides you see. The major downside I see is the introduction of more bureaucracy and a more complicated permission models. "Everyone can do everything" is expedient and easy to understand, but I think on balance, this is a better approach for an organization our size (and growing).

For what it's worth, I'd be personally willing to shepherd/audit the individual crates and repositories under the organization through the initial process. Probably by creating an issue in each repository referencing this proposal with instructions and being available for support / questions.

Additionally, I'd like to include an abridged version of this in georust/meta as documentation for how to onboard projects into georust.

The inverse of the "pro", that this is non-disruptive, is that since there's no forcing function, crates might never make the switch. To some extent, if that's their perogative, well 🤷‍♂️. We can always revisit in say, 6 months.

Alternatives Considered

Since almost all crates already have georust/core as an owner, we could move everyone out of the georust/core to a new georust/contributors (or somesuch) and declare georust/core is only a "publishing recovery group" for when a crate is abandoned. This would be a quick centralized change and most individual crates wouldn't need to do anything.

However:

  1. I think some users are utilizing the georust/core permissions to be able to publish.
    This would break things for them at a potentially inconvenient time, rather than letting them opt in when ready.
  2. crates.io doesn't allow "owning teams" to themselves edit the ownership list, so this approach is kind of a dead end,
    since any publisher added to this group would then also become a a publisher for every other crate - just as things
    are now. I did open a feature request with crates.io which would
    open up other possibilities in the future.

Edit: Fixed typo

@michaelkirk
Copy link
Member Author

Hiya @georust/core - please weigh in on my proposal for all georust crates to adopt publishing teams with a more limited scope.

@magnusuMET
Copy link
Member

@michaelkirk Great proposal! I like the security aspect of this, as the contents of crates on crates.io is not transparent. Usually one has to crawl through the repository and hope the release is tagged and consistent with what is published, or sift through the downloaded crate. There is also no public log over which user/group made the last commit, only the date at which it was published.

It would be very easy for a rogue user to publish a patch bump on a crate with their malicious code. This could possibly not be detected at all (if minor or major bump), or just be seen as an oddity when the developer finds a patch version is already in use.

On the alternative side we could grandfather existing users to georust/core, and put new users in georust/contributors. Individual users may then choose to leave core for contributors on a voluntary basis (or forced after e.g. 6 months).

@michaelkirk
Copy link
Member Author

Thanks for taking the time to review and give feedback @magnusuMET

On the alternative side we could grandfather existing users to georust/core, and put new users in georust/contributors. Individual users may then choose to leave core for contributors on a voluntary basis (or forced after e.g. 6 months).

That would help mitigate the "rogue committer", but the other key part is to mitigate abandoned crates by allowing GH team maintainers (which includes owners of the georust GH org) a way to add/remove publishers to the GH team if the original maintainer becomes inaccessible without properly transferring their crate.

If we continue using only the one publishing team shared by all crates, then there's no way to add someone to it without also giving them publishing rights to all our other crates - the same situation we're trying to get out of today.

@rmanoka
Copy link

rmanoka commented Oct 16, 2020

Good proposal @michaelkirk; I support the general idea.

For the sake of reducing bureaucracy, would it make sense to combine the concerns of publishing, and repo administration (like merging to master)? Essentially the same as what you're saying, but we have a georust/$crate-core for each crate, and they are in charge of merging PRs into master, as well as publishing. The rationale is that all your concerns also apply to merging to master (but may be with lesser severity). Currently, anyone from georust/core can merge PRs into master, which is also not ideal.

@michaelkirk
Copy link
Member Author

Good proposal @michaelkirk; I support the general idea.

For the sake of reducing bureaucracy, would it make sense to combine the concerns of publishing, and repo administration (like merging to master)? Essentially the same as what you're saying, but we have a georust/$crate-core for each crate, and they are in charge of merging PRs into master, as well as publishing. The rationale is that all your concerns also apply to merging to master (but may be with lesser severity). Currently, anyone from georust/core can merge PRs into master, which is also not ideal.

Thanks for taking the time to weigh in @rmanoka.

I'd prefer to have any changes to GH contribution access be a separate discussion. Even though I am personally in favor of restricting repository contribution more than it currently is, I intentionally left it outside the scope of this proposal in order to focus on publishing.

Reasoning:

I see restricting publishing as the biggest concern and wanted to address it quickly and uncontroversially. Repository contribution permissions are potentially a little more complicated. For example, I personally don't think that all contributors should automatically and necessarily be publishers.

There are reasons to have relatively permissive access to contribution that don't apply to publishing:

  • for more day-to-day tasks, like issue mgmt, it's helpful to have more people with access
  • having access encourages people to take more ownership of crates

Conversely, there are reasons to have relatively strict control of publishing which don't necessarily apply to contribution.

  • publishing actions are less transparent
  • mistakes have higher ramifications

And if we do want to further restrict GH contribution access, that can be a quick and centralized change, since, unlike crates.io, on GH we have organization admins who have access to make changes on all projects.

@michaelkirk
Copy link
Member Author

If there seems to be broad support and no blocking objections, I'll plan to merge this in a week, and create issues as TODO's in each of our repos. Since it's an opt-in crate-by-crate process, my hope is that it won't be pulling the rug out from under anyone.

Screen Shot 2020-10-20 at 9 50 26 AM

Since there's been some substantial support shown for the proposal, and no objections, I'll plan to merge this tomorrow unless I hear otherwise. Please weigh in today if you haven't already.

Copy link
Member

@frewsxcv frewsxcv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you so much for putting in the time and effort to write up this great proposal @michaelkirk 🙇

Copy link
Member

@urschrei urschrei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@michaelkirk michaelkirk merged commit 5b4cbdb into master Oct 21, 2020
@michaelkirk
Copy link
Member Author

michaelkirk commented Oct 21, 2020

Ok @georust/core - I've merged the publishing teams proposal.

Next steps for crate owners

Step 1: Create your publishing team

Go to https://github.com/orgs/georust/new-team and enter a name like _my-crate_-publishers. For example, geojson-publishers and use the description crates.io publishing.

Add any additional publishers to the team at this point. If you mark them as team maintainers, they will be able to edit the team themselves in the future.

Step 2: Update crate owners

cd my-crate
cargo owner --add github:georust:my-crate-publishers

If you had previously granted another team, like all of "georust/core" publishing permissions, it should be revoked so that only the smallest group necessary has publishing access.

cargo owner --remove github:georust:core

If your crate had additional folks publishing, be sure that they're either a crates.io user-owner or add them to your new georust/my-crate-publishing team on GitHub.

Step 3: Double check

cargo owner --list
  • Everyone there who should be?
  • Includes your publishing team?
  • No one extra?

Next steps for me

In another week I'll follow up with an audit and reach out to any crate owners who haven't made these changes, or feel free to proactively reach out before then if you have trouble with the process.

@magnusuMET
Copy link
Member

@michaelkirk It seems the syntax is cargo owner --add github:georust:my-crate-publishers instead of georust/my-crate-publishers

@michaelkirk
Copy link
Member Author

🤦 - thanks @magnusuMET. I updated the docs.

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

Successfully merging this pull request may close these issues.

None yet

5 participants