Closed Bug 1440615 Opened 6 years ago Closed 5 years ago

Assess use of external addon bundlesize in Mozilla's GitHub organization 'mozilla'

Categories

(mozilla.org :: Github: Administration, task)

task
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: emorley, Assigned: hwine)

References

Details

I want to use the bundlesize addon in 'mozilla' for the following reasons:

Below are my answers to your stock questions:

** Which repositories do you want to have access? (all or list)
* https://github.com/mozilla/treeherder
(or ideally all, since it would prevent me/others from having to make similar requests for other repos in the future - and should be safe, since bundlesize only asks for commit status permissions)

** Are any of those repositories private?
No

** Provide link to vendor's description of permissions needed and why
bundlesize tracks the size of generated files (in our case, out webpack bundle), and adds GitHub status checks for them. It only needs the `repo:status` permission (read/writing commit statuses):
https://github.com/siddharthkp/bundlesize/issues/71#issuecomment-315778308

** Provide the Install link for a GitHub app
https://github.com/login/oauth/authorize?scope=repo%3Astatus&client_id=6756cb03a8d6528aca5a

(Filed using the link here: https://wiki.mozilla.org/Github#OAUTH_.28classic.29_Applications)
Blocks: 1440616
Ick. This may be an ugly corner case. Most oauth style addins work fine on public repos without access to private repos. With the current ask, that's fine, but if it changes down the road, it could be a problem.

The best solution is if the provider updates to a "GitHub App" style integration. Lacking that, let me double check that any future permission change would require "re-approval" by the owners.

Looking into that...
Assignee: nobody → hwine
Flags: needinfo?(hwine)
... okay, bad news. There is no "re-approval" workflow when additional scopes are requested. That means there would be no visibility to anyone if the app changed scopes to, e.g. ask for general commit access.

So, "denied" for now. 

I _think_ an acceptable workaround, since this is an opensource app, would be to:
 - fork the repo into mozilla
 - audit the code
 - deploy within mozilla org

This incurs a high maintenance cost. 

:gene - would the "we control the code" approach I suggest above mitigate the concerns we discussed?
Flags: needinfo?(hwine) → needinfo?(gene)
By no visibility, I presume you mean to org admins - and that the concern is that users could be prompted for more permissions when signing up in the future? (And that we understandably don't trust said users to not just click through?)

I agree that ideally this service would be using the new GitHub apps integration, but since it's a free open source project, it's likely a "PR welcome" - and even then hard to do, since it would be tricky to work out the infra parts when we don't have access to them.

I must admit at this point (especially given bug 1410453 too), the thought of moving the treeherder repo to its own org is very tempting. (Not disagreeing with the decision here; just a tricky situation with a 1200-repo org)
(In reply to Ed Morley [:emorley] from comment #3)
> By no visibility, I presume you mean to org admins - and that the concern is
> that users could be prompted for more permissions when signing up in the
> future? (And that we understandably don't trust said users to not just click
> through?)

So just to check I understand - the scenario we're concerned about is this?

1) Org admin approves oauth app
2) I then reopen the approval page and press yes to the commit-status only scope
3) Some time later the app changes, making the prompt ask for more permissions (existing approved access not affected at this point)
4) Another user comes along, gets the oauth prompt asking for more than commit-status scopes, and blindly presses yes, with the org admins not being re-prompted
5) The oauth app now has more than just commit access permissions

For step (5), what repos does the oauth app have access to? I'm presuming one of:
(a) repos for which the user has admin permissions
(b) repos for which the user has write or admin permissions
(c) all repos in the org

If (c), then that's really broken!
Pretty close -- some tweaks:

- I'm not sure (3) results in a user prompt for additional permissions, as I've not seen that myself. Confirmation appreciated.

- Note that (4) can happen anywhere on GitHub -- e.g. you might get prompted in a different organization where such scopes are perfectly reasonable.  The scopes stick to your account, and are not unique per org you're a member of.

- (5) define "access". I you mean update a commit status, that's (b) for both public & private repos. It may also be "list all private repos the user has access to".

Item (4) is the most problematic, imo. It requires users to _always_ think of the impact on their most sensitive repos when using a new OAuth app. E.g. if you were even exploring what 'bundlesize' could do on a public repo in your own GitHub account, it would have access to all the mozilla org repos you do -- once we "allow" it in the org.

We (Gene and I) are working on some reporting and tooling to address this issue, but it's not ready yet.
(In reply to Ed Morley [:emorley] from comment #3)
> I agree that ideally this service would be using the new GitHub apps
> integration, but since it's a free open source project, it's likely a "PR
> welcome" - and even then hard to do, since it would be tricky to work out
> the infra parts when we don't have access to them.

This may not be as hard as you think. Taskcluster converted their OAuth app to a GitHub App fairly quickly.

As for "infrastructure" it may be easy to run our own on this (lambda function behind API gateway).

> I must admit at this point (especially given bug 1410453 too), the thought
> of moving the treeherder repo to its own org is very tempting. (Not
> disagreeing with the decision here; just a tricky situation with a 1200-repo
> org)

Understood, AND we'd like to avoid that. As you note, there are a number of OAuth apps folks would like to use which are impacted by this issue. Gene's work may make this level of risk acceptable -- let's see what he has to say.
> :gene - would the "we control the code" approach I suggest above mitigate the concerns we discussed?

If you mean run our own version of bundlesize on some mozilla owned infrastructure of some kind: Doing so would enable us to assess the actual security controls in place protecting the OAuth tokens that bundlesize would store (whereas with a 3rd party running it we can't know what security controls are in place or how well protected the tokens are), so yes it would mitigate the concerns. Keep in mind that a compromise of this theoretical mozilla infrastructure running bundlesize would still result in a compromise of the repos that have granted it access

> For step (5), what repos does the oauth app have access to? I'm presuming one of:

B

In more detail, after the scenario you describe, the third party would have access to both
* All repos that the first user has read write or admin access to (including repos in the github.com/mozilla org). The third party has the rights described in the scopes it requested to those repos (in your example "commit-status only")
* All repos that the second user has read write or admin access to (including repos in the github.com/mozilla org). The third party has the rights described in the scopes it requested to those repos (in your example "more than just commit access")

So, not C

> Gene's work may make this level of risk acceptable -- let's see what he has to say.

The auditing capabilities which we don't have yet but will would enable the admins of treeherder to know for all of the users that they've granted read write or admin either
* what third parties they've delegated rights to and what scopes those rights are constrained by
* that the user has not reported the rights they've delegated for over N days and you may want to revoke their access as they're not playing nice
Flags: needinfo?(gene)
Thank you for the clarifications :-)

For the pyup.io service there was a new user created to set up the oauth integration, to limit access to just the repositories for which it has been given permissions. I'm presuming this wouldn't help here, since they would be the "first user" (in the comment 7 breakdown) and so not help with limiting the issues caused by the "second user"'s acceptance of new scopes?

That said, it looks perhaps the bundlesize author might already be experimenting with an app:
https://github.com/siddharthkp/bundlesize/issues/203
> For the pyup.io service there was a new user created to set up the oauth integration, to limit access to just the repositories for which it has been given permissions. I'm presuming this wouldn't help here, since they would be the "first user" (in the comment 7 breakdown) and so not help with limiting the issues caused by the "second user"'s acceptance of new scopes?

Indeed, creating a "service user" (a purpose created GitHub user that has specific access to only the repos you want to affect) is somewhat useless for most OAuth integrations since they typically present other users with similar requests for OAuth tokens which just bypasses the constraints built in to the service user. If you have the ability to tell the service to only ever present service user foo with an OAuth scope prompt, then it would help but I've not encountered that functionality before in a third party.

fyi: bundlesize was denied access to private repos.

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.