sapling icon indicating copy to clipboard operation
sapling copied to clipboard

ReviewStack Permission Documentation

Open nairb774 opened this issue 3 years ago • 1 comments

I opened up a conversation about adding ReviewStack to our private organization, and the broad permissions were brought up as curious. My understanding is that ReviewStack is mostly working with PRs and that review process, but there are permissions included for Issues, Wikis, Settings, Webhooks, Deploy keys, and a bit more. I assume most of these either have a good explanation or are possibly being requested for future app expansion(1).

Would it be possible to have the permissions listed out and their need documented? I know it might seem a little over the top, but it could help a bit when the app is being reviewed for policy/security requirements. I don't think the details need to be all that deep, just something to tie the requested permission to the feature or function that makes use of it would go a long way. Yes, I understand that the app is all in-browser - which simplifies a lot - however the permissions are still being requested and that triggered questions.

(1) GH breaking an app when permissions change is a really unfortunate design decision which can make those that plan ahead ask for more permissions than are immediately needed.

nairb774 avatar Dec 15 '22 20:12 nairb774

Hi, author of ReviewStack here. Let me start out by saying: I have no desire to ask for more permissions than are necessary. ReviewStack was designed with a strong focus on security and "least privilege:"

  • I apparently have not communicated this well, but ReviewStack is open source: https://github.com/facebook/sapling/tree/main/addons/reviewstack (see also the reviewstack.dev sibling folder). You are free to/encouraged to run a personal/in-house version of it. https://reviewstack.dev/ exists as a convenience.
  • When work on ReviewStack started, GitHub offered only very coarse-grained ACLs for PATs. My understanding is that changed as of some announcements from their most recent conference: I just haven't had a chance to dig into it yet.
  • reviewstack.dev is hosted on Netlify, which is a static hosting site plus some "extras." A key "extra" is the ability to facilitate OAuth, as OAuth inherently requires a "secret," which is at odds with pure static hosting. Once you go through the OAuth flow to obtain a PAT, all data requests are made directly to GitHub (i.e., they do not go through a proxy) because their API endpoints support CORS.

Yes, admittedly I need to research what is the minimal set of permissions ReviewStack requires to provide the user experience it aims to provide, but hopefully this gives you some confidence that we have gone to great lengths to be as "hands-off" as possible.

As an experiment, you could always mint your own PAT with fewer privileges and jam it into localStorage where ReviewStack expects it and see if anything breaks? I'd be very interested to know what you discover.

bolinfest avatar Dec 16 '22 06:12 bolinfest

I just pushed a new version of https://reviewstack.dev/. While I haven't updated the OAuth scopes yet, you can now enter whatever PAT you want and use github.com as the hostname as if you were logging into a GitHub Enterprise account and it will work as if it were consumer GitHub. You can try a PAT with fewer capabilities and see what breaks!

bolinfest avatar Dec 20 '22 20:12 bolinfest