-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DownloadContents can't download files from root of repo #2480
Comments
So was this a regression caused by a change to Do you want to create a PR to fix this, @abatilo , or would you like me to open up this issue to other contributors to this repo to address? |
Hi @gmlewis, I think I actually poorly chose my words. I shouldn't have said "as of v47.1.0" but instead it's more that I found this behavior while using |
Thank you, @abatilo . This would be a great PR for any new contributor to this repo or a new Go developer. Feel free to volunteer for any issue and the issue can be assigned to you so that others don't attempt to duplicate the work. Please check out our CONTRIBUTING.md guide to get started. (In particular, please remember to Thank you! |
I took a look into this tonight because I'd like to get involved in the project. Based on the I was able to construct a minimal working example of this as follows:
package main
import (
"golang.org/x/oauth2"
"github.com/google/go-github/github"
"context"
"fmt"
)
func main() {
ctx := context.Background()
ts := oauth2.StaticTokenSource(
&oauth2.Token{AccessToken: "<YOUR-GITHUB-PAT-HERE>"})
tc := oauth2.NewClient(ctx, ts)
client := github.NewClient(tc)
const (
owner = "google"
repo = "go-github"
)
filepath := "LICENSE"
rc, _, err := client.Repositories.DownloadContents(ctx, owner, repo, filepath, nil)
if err != nil {
fmt.Errorf("Error was: %s\n", err)
}
defer rc.Close()
byteCount := 100
buf := make([]byte, byteCount)
n, err := rc.Read(buf)
if err != nil {
fmt.Errorf("Error while reading into buffer: %s\n", err)
}
fmt.Printf("Contents of first %d bytes are: %s\n", byteCount, buf[:n])
}
diff --git a/github/repos_contents.go b/github/repos_contents.go
index be58fd5..d4a2319 100644
--- a/github/repos_contents.go
+++ b/github/repos_contents.go
@@ -127,13 +127,16 @@ func (s *RepositoriesService) GetReadme(ctx context.Context, owner, repo string,
// code to verify the content is from a successful response.
func (s *RepositoriesService) DownloadContents(ctx context.Context, owner, repo, filepath string, opts *RepositoryContentGetOptions) (io.ReadCloser, *Response, error) {
dir := path.Dir(filepath)
+ fmt.Printf("Dir is: %s\n", dir)
filename := path.Base(filepath)
+ fmt.Printf("Filename is: %s\n", filename)
_, dirContents, resp, err := s.GetContents(ctx, owner, repo, dir, opts)
if err != nil {
return nil, resp, err
}
for _, contents := range dirContents {
+ fmt.Printf("\tName of found content item is: %s\n", *contents.Name)
if *contents.Name == filename {
if contents.DownloadURL == nil || *contents.DownloadURL == "" {
return nil, resp, fmt.Errorf("no download link found for %s", filepath) Because of the behavior of Running the code above, I got the result:
which is the first 100 bytes of Doing the same test on the
So, it seems like specifying a filename in the repo root will still behave properly; the API call will just consider I've never participated in this project before, and I tried to make sure I understood the context before commenting, but if I've totally misunderstood something here, please let me know! I'm just trying to help and learn a little Golang! 😄 |
Thank you, @zaataylor , for the detailed analysis and excellent write-up... It is greatly appreciated! I believe that you have demonstrated that this is working as intended. I will leave this issue open for a short while in case there are any other comments, but otherwise I believe this issue can be closed. |
Hi @zaataylor, your solution demonstrates accessing things at the root of the repo for a GitHub Personal Access Token which does not have the same fine grained per file permissions that a GitHub App might request. Maybe this issue needs to be re-worded to be specific about the file path permissions that you can set on a GitHub App. |
@abatilo - is it not possible to give the GitHub App the appropriate permissions needed to access the root of the repo? If that is truly the case, then I think that you need to contact GitHub tech support and ask how to accomplish this in a GitHub App. |
It's possible. I discovered this because I am building an app to do things based on CODEOWNERS, which can be in 3 locations. There's no actual need for the app to request permissions or have any ability to read files in the root of the repo and my guess is that security minded organizations would prefer that. I do still think that the "correct" solution here is to change the I may have time this week to open a PR with that change and see if the proposed solution is alright! |
Hello, may I try to take a look on this? |
Sorry, @petersondmg , I should have assigned this to @abatilo from the previous comment. Let's wait to hear back what @abatilo discovers. |
I did NOT have the time when I had hoped to. Please feel free to assign to @petersondmg! I appreciate it though @gmlewis |
No problem, @abatilo - thank you for the note, and best wishes to you. @petersondmg - it is yours. |
Hello To generate a Github App Token I did: 1 - generated a private key for the app It was able to list files on the root and download the file. Should I try a different approach? @gmlewis @abatilo Thanks. |
Thank you, @petersondmg ! |
Hi @petersondmg. If you configure the GitHub App with the |
I believe this issue can be closed. Do you agree, @abatilo and @petersondmg ? |
Any news on the proposed solution? How can we help? |
Hi there, was there any progress made on this? |
GitHub Apps have the ability to request access to individual files which is great for fine-grained access.
However, as of
v47.1.0
, this will prevent someone from being able to download a file from the root repository because of the wayDownloadContents
implements directory scanning first:go-github/github/repos_contents.go
Lines 129 to 134 in 8a4bdb5
The first thing that the function does is get just the directory name with
path.Dir
but if you try to download a file like justCODEOWNERS
which is a valid location for aCODEOWNERS
file, then the first request to the API will create a URL like so:At the moment, there's no way to add the root directory to the list of single file paths. You cannot add a blank path and adding just
/
doesn't work either.The text was updated successfully, but these errors were encountered: