Closed Bug 1799054 Opened 3 years ago Closed 2 years ago

Jira App requesting updated permissions in Mozilla and Pocket orgs

Categories

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

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: cknowles, Assigned: cknowles)

Details

Attachments

(1 file)

Though the pocket people may update theirs, we got notified from Mozilla and Pocket orgs.

Text reads: "We have updated the contents permissions from 'read-only' to 'read & write.' We have done this so you can create a branch from a Jira Issue."

Screenshot attached.

Flags: needinfo?(asargent)

And Mozilla-Mobile - and Mozilla-services. Frankly at this point, just tell me if there's an org where I shouldn't put this change in.

I'm good with allowed anywhere it's needed. I'm going to NI :hwine to double check that decision, but I can't see any real issue with allowing that and if there is any additional historical context or previous problems faced.

Flags: needinfo?(asargent) → needinfo?(hwine)

First, let me answer Chris' question:
(In reply to Chris Knowles [:cknowles] from comment #1)

And Mozilla-Mobile - and Mozilla-services. Frankly at this point, just tell me if there's an org where I shouldn't put this change in.

We can't do "org by org" permission acceptance -- that way lies madness. (It would be different is we could control which perms we could accept, but we can't, so we won't.) In the worst case, there's a race condition:

  • vendor asks for permission A, and request to seceng is filed
  • seceng approves permission A
  • vendor asks for permission B
  • ghe admin gets around to accepting changes - accepts both A & B :(

So, I think it's good to have the screenshot as in comment 0. If we approve, then the updated permissions can be accepted in every org that presents a matching screenshot. (Andy could have fun scripting-with-image-matching in selenium!)

If a non-matching screenshot is found, then an additional permission has been requested, and a new request to seceng is needed.
Q.E.D.

(In reply to asargent from comment #2)

I can't see any real issue with allowing [content write]

I won't claim it's a real issue, but it's one I want to talk through a bit:

  • write permission allows both content change and content deletion
  • at this moment, I don't know what the Jira UI looks like for this, or how Jira has positioned the feature (beyond the add branch)
  • our "GitHub app approval per repo" policy is set so repo admins explicitly opt in to anything that could change their repository

The most conservative way to address this, and stay aligned with our policy is:

  • get pointer to Jira docs for this new feature
  • get jira admins to explain if the write-required-features can be disabled within Jira
  • write up an announcement
    • explaining the changes and pointing both to both the new docs and jira admins for more questions
    • announce a enable-write-date when we'll start allowing new perms
    • tell admins how to file a ticket to disable Jira on their repo (and promise to do that before enablement starts)
  • somehow make the announcement available to all future repos admins who wish to add the Jira app to their repo

I.e. we can't stop forward progress, but we can make sure repo admins have a chance to make an informed choice.

As I write this up, I realize that I do have a strong feeling about this. Who can add/modify a source code repo as "change permission" is a critical security boundary for any project.

  • Mozilla has spent massive $ to ensure all changes to Firefox source code can be accounted for, and only allowed by vouched individuals
  • I'm not sure how these source code modification events would should up in the GitHub audit log
    • can any Jira user create a branch? Or only if the Jira user also has GitHub permissions (I doubt that it can tell)
    • how is the branch name selected? GitHub branch protection rules are based on regex matches
      • if name is under control of user, there might be a way to gain privilege escalation (e.g. match a "releasable branch" rule set)
      • if name has a fixed prefix (e.g. jira-app-branch-*) what is it? Can we use that to write branch protection rules which make the usage safe?
      • but then, how do we monitor future updates to the app, to ensure it hasn't been hacked (Atlassian doesn't have the purest record)
    • how does the event show up in the audit logs? Can we trace back from write-in-github-by-jira-app to jira-user?
    • are the Jira logs routed to somewhere the SIRT can access?

So, I've convinced myself that we need to pause on acceptance of these permissions until some subset of the concerns above are addressed.

:cknowles - this also means that shouldn't add the app to any new org until this is resolved. I'm not sure the current run book covers a "check for any pending permission update, and hold installation until permissions approved" when the app has already been installed in other orgs. (I sense an agenda item for next week's meeting.)

Keeping NI on myself

Flags: needinfo?(cknowles)

NO app in new org - and Agenda item added for you to work with.

Question - Adding the app to an org is "Check the allow lists in moco-ghe-admin repos, and if it's not there, ask secops" - Jira is NOT in that list, so we're gonna ask - so I'm not concerned about leakage there. However, adding it to a new REPO in an org, is "is it already in the org, if so, GO, add it to the repo" ... I think that wouldn't apply the new settings to the org or the new repo - but I'm not 100% certain. So right now, I'm in "If it's Jira, ask Hal" mode.

Let me know if/how I need to adjust this.

Flags: needinfo?(cknowles)

Over in bug 1805991 it is clear that "if it's jira ask Hal mode" from above was overzealous - per Hal's update in that bug, adding repos to the app in an org that already has jira is fine. NEW orgs continue to be the problem.

Decision: take the update (the update is approved)

Write permissions for actions do not allow modification of repository contents, they only allow those actions designated as "write" in the GitHub App Permissions documentation

The note should be pasted into the request for a new repo to be added, so admins acknowledge the impact. Ideally, we'd notify all current admins of repos with the app enabled, but we don't have the tooling (yet).

Flags: needinfo?(hwine) → needinfo?(cknowles)

Note that per the runbook here, we don't GET a request from a human. Only the GitHub "someone asked for app X in repo or org Y" email to owners of the org.

So, I worry that this process becomes onerous. But hear me out - only comms path I have would be to open a bug to the requestor (assuming we can match GHID to name) and/or the admins of the repo in question. Is that what you'd want?

Flags: needinfo?(hwine)
Flags: needinfo?(cknowles)
Flags: needinfo?(asargent)

No intent to make more work here -- there's no action at the moment for notification on this ticket. Let's take the future comms to a separate thread or agenda item. It need not block this ticket.

so NI :ckowles to approve and close this ticket <tag/>

Flags: needinfo?(hwine)
Flags: needinfo?(cknowles)
Flags: needinfo?(asargent)

buttons pushed in all the orgs that we received emails about...

mozilla-services
mozilla-mobile
mozilla

Note that we did receive an email for pocket but in the intervening months, some other owner over there must have clicked accept.

Status: NEW → RESOLVED
Closed: 2 years ago
Flags: needinfo?(cknowles)
Resolution: --- → FIXED
Assignee: nobody → cknowles
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: