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.

Sign up for GitHub

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

Jump to bottom

RFC 0045 - Github Trust Levels #45

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

RFC 0045 - Github Trust Levels #45

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 76 additions & 0 deletions rfcs/0045-github-trust-levels.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
# RFC 0045 - Github Trust Levels
* Proposed by: @ahal

# Summary

In Firefox-CI there are currently three trust levels (abbreviated L1, L2 and
L3). These were designed for Gecko, but when we started supporting Github, we
shoehorned the level system on. Currently for a "level 3" project:

1. L1 is used on pull requests
2. L2 is unused
3. L3 is used for pushes

However, it's also possible for a project to be configured as a "level 1" project.
In these projects, both pull requests and pushes use L1 and L3 is unused.

This RFC proposes a redefinition of the Github trust levels, as well as some changes to
ci-config to support it. First, trust levels on Github will be defined as follows:

1. L1 is used for public (e.g untrusted) pull requests.
2. L2 is used for trusted pull requests (e.g with `collaborator` or `public_restricted` policies), as well as
pushes to unprotected or non-release branches.
3. L3 is used for pushes to protected or release branches.
Copy link

Choose a reason for hiding this comment

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

This, imo, is a very different use of the "level" system. I think this will be an interesting discussion that may lead to a re-definition of levels. 😉 Now would be the best time to do that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The mental model I have (and brought to security/risk with me) is that "levels" are traditionally assigned to a branch, and specify both the permissions required to modify the code base and the pool of workers used for building (to provide isolation between the levels).

The above is my definition of level as well. I'm interested to dig into why you think this proposal constitutes a change to that definition. (Though we shouldn't dig in too much, because this whole idea is moot if there isn't a good way to implement it :p)

Copy link

Choose a reason for hiding this comment

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

interesting -- probably a zoom chat -- maybe I'm looking at "target branch" to set level, and you're looking at "source branch". And/or I'm only considering m-c where phabricator is controlling access, and you're looking at "native GitHub" projects as well. (Both need coverage.)


In ci-configuration, Github projects will no longer be configured with a
project wide level. Instead, pushes will use L2 by default and select branches
will be explicitly listed as L3 as necessary.


## Motivation

This change will help solve three problems:

1. Lack of separation between pushes and pull requests on level 1 projects.

Currently PRs and pushes share the same secrets, caches and more for level 1
projects. While these projects are not a security risk, this still
introduces potential for footguns and makes it difficult to implement
certain features properly. Caching in particular suffers.

2. Lack of separation between pull request policies.

There's also no way to differentiate between trusted and untrusted
pull-requests via the level system. E.g, they both share access to the same
secret paths, so care needs to be taken not to accidentally expose a secret
to the public. Having these on separate levels reduces the risk of errors
in ci-config.

3. Gecko migration to Github.

Gecko will be moving to Github sometime in 2024, which means we need to
figure out how to deal with the project twigs (e.g, oak, maple, etc). We
could explicitly fork each one and list them all out as a separate level 2
project (like we do now). But it would be much nicer if these could be
replaced by branches on the main repo. Supporting this will require allowing
different branches to have different levels.
Copy link

Choose a reason for hiding this comment

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

FYI, the reason the mercurial repos uses separate repositories for some branches was not (entirely) about "poor branch semantics in hg". A primary reason was that there was no good method to enforce different levels of write access with the way the hg server operates. The same is true of git, and the only reason the branches could be hosted in the same repository as m-c et all is that neither git nor GitHub will be controlling who's allowed to write to what. Phabricator will enforce all the ACLs.

Regardless of how branches are stored, each branch has a "level" associated with it, and can only be "built" on machinery in a specific pool.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A primary reason was that there was no good method to enforce different levels of write access with the way the hg server operates. The same is true of git

But don't branch protections in Github provide this? You can block people with write access from landing directly to the branch. In the case of Gecko, I assume we'd set up a branch protection that blocks everyone except the Lando bot for branches that are traditionally L3.

Copy link

Choose a reason for hiding this comment

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

Nope -- branch protections are not a git feature, but a GitHub feature. My understanding is no one will be using the GitHub interface directly. But this is a tricky area, and plans may not yet be "firm enough" to really say

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, but branch protections aren't just for the Github interface, they apply to the Git server. So e.g, if you add a branch protection to block pushes on main, then run git push origin main the Git server will reject your push.

So while it's true they aren't a general Git feature, they are a feature of Github's Git servers :)

Copy link

Choose a reason for hiding this comment

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

This approach would need more discussion, as:

  • it may lead to a more complex access model than desired for m-c
  • why not use the gecko-projects approach

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since writing this I learned that using gecko-projects is the plan, so this is a moot point.

But personally I'd prefer branches on the main repo (defaulting to level 1), as that seems more self-serve and easy to set up.

Copy link

Choose a reason for hiding this comment

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

You have greater faith in humanity than I! 😉

Self-serve and shared "free" resources more often leads to recreating the tragedy of the commons in my experience. 😒



# Details

<how will this be implemented? what does it depend on? what are the
compatibility concerns?>

# Open Questions

<what isn't decided yet? remove this section when it is empty, and then go to
the final comment phase>

# Implementation

<once the RFC is decided, these links will provide readers a way to track the
implementation through to completion>

* <link to tracker bug, issue, etc.>
* <...>