Geistesblitz: Bors policy enforcement with revocation

Idea

In a world, where bors.tech will be our policy enforcement agent (as oposed to a trusted policy decision agent), we are confronted with the problem, that bors needs to adapt its enforcement not only to changing metadata of the PR itself, but also to changing metadata of it’s head commit in ways that are not subject to race conditions. Currently any policy decision implemented on the Github comment interface via bors delegate= is sticky and does not automatically go away if subsequent malicious commits are pushed to the PR.

While there is a blocking label scheme in bors that could be combined with a Github action that marks any new commit as untrusted (and thereby blocked from merging), this does not seem to afford us adapting a bors delegate= implemented decision including revoking delegation. Fortunately, bors has a very vibrant RFC process, so we should consider to submit a RFC once the use case is completely fleshed out on our side.

Using their RFC process as a community might enable us to improve bors in ways where it is put in a genuine position to consult and enforce policy decisions made by a trusted policy agent all by it’s own over the corresponding interfaces. Such tighter coupling with a policy agent is also a very interesting use case for the broader bors community, and I would wish for our community to earn the credits.

Impact analysis

  • Bors community could benefit from such a diverse mega project such as ours.
  • nix* community could co-evolve with the broader bors community on the subject of matter.
  • The capability to adapt enforcement based on commit metadata seems to be a prerequisite of its own right to fix holes in our (emerging) trust model that’s currently not possible to achieve through an off-the-shelf bors implementation.


2 Likes

I think that Bors supports checking GitHub statuses as a requirement to merge. This means that we could have a policy/merge-allowed status that is a requirement. If we have that then in theory anyone could trigger Bors as merging is not allowed until the policy passes.

Of course we probably still want to have some sort of limitation? At the very least to prevent arbitrary try builds from being triggered. However if we have this in place it could look something like this:

  • Maintainers have Bors permission.
  • We have a bot that automatically delegates to anyone who has already had a PR merged.

Again, at this point access to Bors is a resource concern, not a security concern assuming that Bors reads the GitHub statuses correctly.

To be explicit using stauses gives us revocation via:

  • Statuses can be updated at any time.
  • Statuses are per-commit. So pushing new code will “revoke” permission until the required checks have been made and the new verdict published.

That sounds like a solid suggestion for go/no-go decision enforcement at the commit level and seems currently supported by bors. This is superior to the mentioned blocking label based approach which can have complementary (manual) use.

I think adjusting delegation remains an issue in this scenario.

And by the way, I also think there is currently no bors enforcment of some sort of 4/6/8 eyes principle on PR approval. That also will become relevant for our case.

I think this can be added though via the policy status. The policy engine could look at who has approved the PR (and at what version they approved) and do the right thing.

Your suggestion might even be a better draft for an RFC to the bors community. As I come to understand it an authorization model based on PR metadata is generally flawed (or at least inconvenient) over an authorization model based on commit metadata. Thank you for raising this point.

In this model, we would have to reason about bors delegate at a fundamental level. Currently it’s a fragile model of granting sticky trust. It might have it’s place as a convenience escape hatch among trusted core members, though.

What exactly do you think the RFC would be about? IIUC this is all already supported by Bird.

Might be true, and that would be wonderful. In the context of

I’ll have a closer look in the coming days and report back with some hard reconnaissance.

See https://bors.tech/documentation/ and pr_status. It isn’t explicit in the docs that it ensures that the appropriate commit is the one picked but if not we can probably file that against them as a security issue.

Isn’t this already resolved by using the existing BORS feature of requiring up to date PR reviews? I feel two up to date reviews by a restricted group of those with commit access would be more than enough to ensure malicious bors r+ never run, even after delegation.

1 Like