Fix: Unauthorized users can be allowed to view the private pipeline if the logged-in user's scmContext and the pipeline's scmContext are different (scm-github) #194
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yoshwata
changed the title
yoshwata
changed the title
wahapo
approved these changes
Nov 12, 2021
tkyi
approved these changes
Nov 13, 2021
|
🎉 This PR is included in version 11.6.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
tkyi
reviewed
Nov 17, 2021
| @@ -227,6 +227,11 @@ class GithubScm extends Scm { | |||
| privateRepo = scmRepo.privateRepo || false; | |||
| } else { | |||
| try { | |||
| if (scmHost !== this.config.gheHost) { | |||
There was a problem hiding this comment.
there is a bug if we're using open source (and gheHost is not set)
{"level":"error","message":"Failed to getPermissions: Pipeline's scmHost github.com does not match with user's scmHost undefined","stack":"Error: Pipeline's scmHost github.com does not match with user's scmHost undefined\n at GithubScm.lookupScmUri (/Users/tkyi/repos/opensource/screwdriver/node_modules/screwdriver-scm-github/index.js:231:27)\n at GithubScm._getPermissions (/Users/tkyi/repos/opensource/screwdriver/node_modules/screwdriver-scm-github/index.js:865:40)\n at /Users/tkyi/repos/opensource/screwdriver/node_modules/screwdriver-scm-base/index.js:355:25","timestamp":"2021-11-17T00:32:35.390Z"}
There was a problem hiding this comment.
OK, I will add a PR.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context
If multiple ghe instances are available from Screwdriver.cd, you may be able to see private pipelines that shouldn't be visible.
For example, if you have the following ghe instances:
Create a private pipeline
private-1onghe-b. In this case, the user logged in to the Screwdriver in the context ofghe-bwill not be able to viewprivate-1. This access is 404 because this user's privileges cannot look up the target pipeline.The problem occurs when the user's login context is
ghe-a. ThelookUpScmUrifunction does not take into account the pipeline's scmContext at all and uses the user's login context to get the repository information. Therefore, although the repository id is the same asprivate-1, the destination isghe-a. Since this meaningless repository is public, you can see what you shouldn't be able to see after passing authentication.Objective
Check the user's login context just before hitting the API to look up GHE. Accessing a GHE instance in a different context is meaningless and throws an error.
References
PR for scm-gitlab
screwdriver-cd/scm-gitlab#46
License
I confirm that this contribution is made under the terms of the license found in the root directory of this repository's source tree and that I have the authority necessary to make this contribution on behalf of its copyright owner.