diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json new file mode 100644 index 00000000000..c9de19da28c --- /dev/null +++ b/.devcontainer/devcontainer.json @@ -0,0 +1,24 @@ +{ + "image": "mcr.microsoft.com/devcontainers/go:1.19", + "features": { + "ghcr.io/devcontainers/features/sshd:1": {} + }, + "remoteUser": "vscode", + "customizations": { + "vscode": { + "extensions": [ + "golang.go" + ], + "settings": { + "go.toolsManagement.checkForUpdates": "local", + "go.useLanguageServer": true, + "go.gopath": "/go" + } + } + }, + "runArgs": [ + "--cap-add=SYS_PTRACE", + "--security-opt", + "seccomp=unconfined" + ] +} diff --git a/.github/SECURITY.md b/.github/SECURITY.md index 27170f56408..15f909b715b 100644 --- a/.github/SECURITY.md +++ b/.github/SECURITY.md @@ -1,3 +1,14 @@ -If you discover a security issue in this repository, please submit it through the [GitHub Security Bug Bounty](https://hackerone.com/github). +GitHub takes the security of our software products and services seriously, including the open source code repositories managed through our GitHub organizations, such as [cli](https://github.com/cli). + +If you believe you have found a security vulnerability in GitHub CLI, you can report it to us in one of two ways: + +* Report it to this repository directly using [private vulnerability reporting][]. Such reports are not eligible for a bounty reward. + +* Submit the report through [HackerOne][] to be eligible for a bounty reward. + +**Please do not report security vulnerabilities through public GitHub issues, discussions, or pull requests.** Thanks for helping make GitHub safe for everyone. + + [private vulnerability reporting]: https://github.com/cli/cli/security/advisories + [HackerOne]: https://hackerone.com/github diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 00000000000..1a850c9b3bc --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,15 @@ +version: 2 +updates: +- package-ecosystem: gomod + directory: "/" + schedule: + interval: "daily" + ignore: + - dependency-name: "*" + update-types: + - version-update:semver-minor + - version-update:semver-major +- package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "daily" diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 9e3848c26a2..4aa93810d14 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -2,23 +2,32 @@ name: Code Scanning on: push: + branches: [trunk] pull_request: + branches: [trunk] + paths-ignore: + - '**/*.md' schedule: - cron: "0 0 * * 0" +permissions: + actions: read # for github/codeql-action/init to get workflow details + contents: read # for actions/checkout to fetch code + security-events: write # for github/codeql-action/analyze to upload SARIF results + jobs: CodeQL-Build: runs-on: ubuntu-latest steps: - name: Check out code - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Initialize CodeQL - uses: github/codeql-action/init@v1 + uses: github/codeql-action/init@v2 with: languages: go queries: security-and-quality - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v1 + uses: github/codeql-action/analyze@v2 diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 226050ea90a..1a3a3331885 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -1,5 +1,9 @@ name: Tests on: [push, pull_request] + +permissions: + contents: read + jobs: build: strategy: @@ -9,13 +13,21 @@ jobs: runs-on: ${{ matrix.os }} steps: - - name: Set up Go 1.16 - uses: actions/setup-go@v2 + - name: Set up Go 1.19 + uses: actions/setup-go@v4 with: - go-version: 1.16 + go-version: 1.19 - name: Check out code - uses: actions/checkout@v2 + uses: actions/checkout@v3 + + - name: Restore Go modules cache + uses: actions/cache@v3 + with: + path: ~/go/pkg/mod + key: go-${{ runner.os }}-${{ hashFiles('go.mod') }} + restore-keys: | + go-${{ runner.os }}- - name: Download dependencies run: go mod download diff --git a/.github/workflows/issueauto.yml b/.github/workflows/issueauto.yml new file mode 100644 index 00000000000..40c4b36e700 --- /dev/null +++ b/.github/workflows/issueauto.yml @@ -0,0 +1,24 @@ +name: Issue Automation +on: + issues: + types: [opened] + +permissions: + contents: none + issues: write + +jobs: + issue-auto: + runs-on: ubuntu-latest + steps: + - name: label incoming issue + env: + GH_REPO: ${{ github.repository }} + GH_TOKEN: ${{ secrets.AUTOMATION_TOKEN }} + ISSUENUM: ${{ github.event.issue.number }} + ISSUEAUTHOR: ${{ github.event.issue.user.login }} + run: | + if ! gh api orgs/cli/public_members/$ISSUEAUTHOR --silent 2>/dev/null + then + gh issue edit $ISSUENUM --add-label "needs-triage" + fi \ No newline at end of file diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 78d94c78e5d..dd1fede0dec 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -11,25 +11,36 @@ on: - go.mod - go.sum +permissions: + contents: read + jobs: lint: runs-on: ubuntu-latest steps: - - name: Set up Go 1.16 - uses: actions/setup-go@v2 + - name: Set up Go 1.19 + uses: actions/setup-go@v4 with: - go-version: 1.16 + go-version: 1.19 - name: Check out code - uses: actions/checkout@v2 + uses: actions/checkout@v3 + + - name: Restore Go modules cache + uses: actions/cache@v3 + with: + path: ~/go/pkg/mod + key: go-${{ runner.os }}-${{ hashFiles('go.mod') }} + restore-keys: | + go-${{ runner.os }}- - name: Verify dependencies run: | go mod verify go mod download - LINT_VERSION=1.39.0 + LINT_VERSION=1.50.1 curl -fsSL https://github.com/golangci/golangci-lint/releases/download/v${LINT_VERSION}/golangci-lint-${LINT_VERSION}-linux-amd64.tar.gz | \ tar xz --strip-components 1 --wildcards \*/golangci-lint mkdir -p bin && mv golangci-lint bin/ diff --git a/.github/workflows/prauto.yml b/.github/workflows/prauto.yml index 20a88b31eb3..2596fa76c0e 100644 --- a/.github/workflows/prauto.yml +++ b/.github/workflows/prauto.yml @@ -2,6 +2,12 @@ name: PR Automation on: pull_request_target: types: [ready_for_review, opened, reopened] + +permissions: + contents: none + issues: write + pull-requests: write + jobs: pr-auto: runs-on: ubuntu-latest @@ -15,6 +21,7 @@ jobs: PRNUM: ${{ github.event.pull_request.number }} PRHEAD: ${{ github.event.pull_request.head.label }} PRAUTHOR: ${{ github.event.pull_request.user.login }} + PR_AUTHOR_TYPE: ${{ github.event.pull_request.user.type }} if: "!github.event.pull_request.draft" run: | commentPR () { @@ -42,8 +49,12 @@ jobs: ' -f colID="$(colID "Needs review")" -f prID="$PRID" } - if gh api orgs/cli/public_members/$PRAUTHOR --silent 2>/dev/null + if [ "$PR_AUTHOR_TYPE" = "Bot" ] || gh api orgs/cli/public_members/$PRAUTHOR --silent 2>/dev/null then + if [ "$PR_AUTHOR_TYPE" != "Bot" ] + then + gh pr edit $PRNUM --add-assignee $PRAUTHOR + fi if ! errtext="$(addToBoard 2>&1)" then cat <<<"$errtext" >&2 @@ -55,6 +66,8 @@ jobs: exit 0 fi + gh pr edit $PRNUM --add-label "external" + if [ "$PRHEAD" = "cli:trunk" ] then closePR diff --git a/.github/workflows/releases.yml b/.github/workflows/releases.yml index 6f9f6547ab4..606327059f7 100644 --- a/.github/workflows/releases.yml +++ b/.github/workflows/releases.yml @@ -5,30 +5,52 @@ on: tags: - "v*" +permissions: + contents: write # publishing releases + repository-projects: write # move cards between columns + jobs: goreleaser: - runs-on: ubuntu-latest + runs-on: ubuntu-20.04 steps: - name: Checkout - uses: actions/checkout@v2 - - name: Set up Go 1.16 - uses: actions/setup-go@v2 + uses: actions/checkout@v3 + with: + fetch-depth: 0 + - name: Set up Go 1.19 + uses: actions/setup-go@v4 with: - go-version: 1.16 + go-version: 1.19 - name: Generate changelog + id: changelog + run: | + echo "::set-output name=tag-name::${GITHUB_REF#refs/tags/}" + gh api repos/$GITHUB_REPOSITORY/releases/generate-notes \ + -f tag_name="${GITHUB_REF#refs/tags/}" \ + -f target_commitish=trunk \ + -q .body > CHANGELOG.md + env: + GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}} + - name: Install osslsigncode + run: sudo apt-get install -y osslsigncode + - name: Obtain signing cert run: | - echo "GORELEASER_CURRENT_TAG=${GITHUB_REF#refs/tags/}" >> $GITHUB_ENV - git fetch --unshallow - script/changelog | tee CHANGELOG.md + cert="$(mktemp -t cert.XXX)" + base64 -d <<<"$CERT_CONTENTS" > "$cert" + echo "CERT_FILE=$cert" >> $GITHUB_ENV + env: + CERT_CONTENTS: ${{ secrets.WINDOWS_CERT_PFX }} - name: Run GoReleaser - uses: goreleaser/goreleaser-action@v2 + uses: goreleaser/goreleaser-action@v4 with: - version: v0.174.1 + version: "~1.13.1" args: release --release-notes=CHANGELOG.md env: GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}} + GORELEASER_CURRENT_TAG: ${{steps.changelog.outputs.tag-name}} + CERT_PASSWORD: ${{secrets.WINDOWS_CERT_PASSWORD}} - name: Checkout documentation site - uses: actions/checkout@v2 + uses: actions/checkout@v3 with: repository: github/cli.github.com path: site @@ -55,16 +77,15 @@ jobs: api-write --silent projects/columns/cards/$card/moves -f position=top -F column_id=$DONE_COLUMN done echo "moved ${#cards[@]} cards to the Done column" - - name: Install packaging dependencies run: sudo apt-get install -y rpm reprepro - name: Set up GPG run: | - gpg --import --no-tty --batch --yes < script/pubkey.asc + echo "${{secrets.GPG_PUBKEY}}" | base64 -d | gpg --import --no-tty --batch --yes echo "${{secrets.GPG_KEY}}" | base64 -d | gpg --import --no-tty --batch --yes echo "allow-preset-passphrase" > ~/.gnupg/gpg-agent.conf gpg-connect-agent RELOADAGENT /bye - echo "${{secrets.GPG_PASSPHRASE}}" | /usr/lib/gnupg2/gpg-preset-passphrase --preset 867DAD5051270B843EF54F6186FA10E3A1D22DC5 + echo "${{secrets.GPG_PASSPHRASE}}" | /usr/lib/gnupg2/gpg-preset-passphrase --preset "${{secrets.GPG_KEYGRIP}}" - name: Sign RPMs run: | cp script/rpmmacros ~/.rpmmacros @@ -80,6 +101,9 @@ jobs: popd - name: Run reprepro env: + # We are no longer adding to the distribution list. + # All apt distributions should use "stable" according to our install documentation. + # In the future we will remove legacy distributions listed here. RELEASES: "cosmic eoan disco groovy focal stable oldstable testing sid unstable buster bullseye stretch jessie bionic trusty precise xenial hirsute impish kali-rolling" run: | mkdir -p upload @@ -113,7 +137,7 @@ jobs: runs-on: windows-latest steps: - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Download gh.exe id: download_exe shell: bash @@ -123,34 +147,33 @@ jobs: unzip -o *.zip && rm -v *.zip env: GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}} - - name: Install go-msi - run: choco install -y "go-msi" - name: Prepare PATH - shell: bash - run: | - echo "$WIX\\bin" >> $GITHUB_PATH - echo "C:\\Program Files\\go-msi" >> $GITHUB_PATH + id: setupmsbuild + uses: microsoft/setup-msbuild@v1.3.1 - name: Build MSI id: buildmsi shell: bash env: ZIP_FILE: ${{ steps.download_exe.outputs.zip }} + MSBUILD_PATH: ${{ steps.setupmsbuild.outputs.msbuildPath }} run: | - mkdir -p build - msi="$(basename "$ZIP_FILE" ".zip").msi" - printf "::set-output name=msi::%s\n" "$msi" - go-msi make --msi "$PWD/$msi" --out "$PWD/build" --version "${GITHUB_REF#refs/tags/}" + name="$(basename "$ZIP_FILE" ".zip")" + version="$(echo -e ${GITHUB_REF#refs/tags/v} | sed s/-.*$//)" + "${MSBUILD_PATH}\MSBuild.exe" ./build/windows/gh.wixproj -p:SourceDir="$PWD" -p:OutputPath="$PWD" -p:OutputName="$name" -p:ProductVersion="$version" - name: Obtain signing cert id: obtain_cert + shell: bash + run: | + base64 -d <<<"$CERT_CONTENTS" > ./cert.pfx + printf "::set-output name=cert-file::%s\n" ".\\cert.pfx" env: - DESKTOP_CERT_TOKEN: ${{ secrets.DESKTOP_CERT_TOKEN }} - run: .\script\setup-windows-certificate.ps1 + CERT_CONTENTS: ${{ secrets.WINDOWS_CERT_PFX }} - name: Sign MSI env: CERT_FILE: ${{ steps.obtain_cert.outputs.cert-file }} EXE_FILE: ${{ steps.buildmsi.outputs.msi }} - GITHUB_CERT_PASSWORD: ${{ secrets.GITHUB_CERT_PASSWORD }} - run: .\script\sign.ps1 -Certificate $env:CERT_FILE -Executable $env:EXE_FILE + CERT_PASSWORD: ${{ secrets.WINDOWS_CERT_PASSWORD }} + run: .\script\signtool sign /d "GitHub CLI" /f $env:CERT_FILE /p $env:CERT_PASSWORD /fd sha256 /tr http://timestamp.digicert.com /v $env:EXE_FILE - name: Upload MSI shell: bash run: | @@ -167,14 +190,14 @@ jobs: DISCUSSION_CATEGORY: General GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}} - name: Bump homebrew-core formula - uses: mislav/bump-homebrew-formula-action@v1 + uses: mislav/bump-homebrew-formula-action@v2 if: "!contains(github.ref, '-')" # skip prereleases with: formula-name: gh env: COMMITTER_TOKEN: ${{ secrets.UPLOAD_GITHUB_TOKEN }} - name: Checkout scoop bucket - uses: actions/checkout@v2 + uses: actions/checkout@v3 with: repository: cli/scoop-gh path: scoop-gh @@ -197,18 +220,3 @@ jobs: GIT_AUTHOR_NAME: cli automation GIT_COMMITTER_EMAIL: noreply@github.com GIT_AUTHOR_EMAIL: noreply@github.com - - name: Bump Winget manifest - shell: pwsh - env: - WINGETCREATE_VERSION: v0.2.0.29-preview - GITHUB_TOKEN: ${{ secrets.UPLOAD_GITHUB_TOKEN }} - run: | - $tagname = $env:GITHUB_REF.Replace("refs/tags/", "") - $version = $tagname.Replace("v", "") - $url = "https://github.com/cli/cli/releases/download/${tagname}/gh_${version}_windows_amd64.msi" - iwr https://github.com/microsoft/winget-create/releases/download/${env:WINGETCREATE_VERSION}/wingetcreate.exe -OutFile wingetcreate.exe - - .\wingetcreate.exe update GitHub.cli --url $url --version $version - if ($version -notmatch "-") { - .\wingetcreate.exe submit .\manifests\g\GitHub\cli\${version}\ --token $env:GITHUB_TOKEN - } diff --git a/.goreleaser.yml b/.goreleaser.yml index 4c5f62a0609..d67d023aee1 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -8,7 +8,7 @@ release: before: hooks: - go mod tidy - - make manpages + - make manpages GH_VERSION={{.Version}} builds: - <<: &build_defaults @@ -16,10 +16,9 @@ builds: main: ./cmd/gh ldflags: - -s -w -X github.com/cli/cli/v2/internal/build.Version={{.Version}} -X github.com/cli/cli/v2/internal/build.Date={{time "2006-01-02"}} - - -X main.updaterEnabled=cli/cli id: macos goos: [darwin] - goarch: [amd64] + goarch: [amd64, arm64] - <<: *build_defaults id: linux @@ -31,7 +30,11 @@ builds: - <<: *build_defaults id: windows goos: [windows] - goarch: [386, amd64] + goarch: [386, amd64, arm64] + hooks: + post: + - cmd: ./script/sign-windows-executable.sh '{{ .Path }}' + output: false archives: - id: nix @@ -57,7 +60,7 @@ nfpms: - license: MIT maintainer: GitHub homepage: https://github.com/cli/cli - bindir: /usr/bin + bindir: /usr dependencies: - git description: GitHub’s official command line tool. diff --git a/.vscode/settings.json b/.vscode/settings.json deleted file mode 100644 index 99a52d6d6e0..00000000000 --- a/.vscode/settings.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "search.exclude": { - "vendor/**": true - } -} \ No newline at end of file diff --git a/CODEOWNERS b/CODEOWNERS new file mode 100644 index 00000000000..4b902931306 --- /dev/null +++ b/CODEOWNERS @@ -0,0 +1,5 @@ +* @cli/code-reviewers + +pkg/cmd/codespace/ @cli/codespaces +pkg/liveshare/ @cli/codespaces +internal/codespaces/ @cli/codespaces diff --git a/README.md b/README.md index 71630a5ccd2..c9572246618 100644 --- a/README.md +++ b/README.md @@ -8,18 +8,20 @@ GitHub CLI is available for repositories hosted on GitHub.com and GitHub Enterpr ## Documentation -[See the manual][manual] for setup and usage instructions. +For [installation options see below](#installation), for usage instructions [see the manual][manual]. ## Contributing If anything feels off, or if you feel that some functionality is missing, please check out the [contributing page][contributing]. There you will find instructions for sharing your feedback, building the tool locally, and submitting pull requests to the project. +If you are a hubber and are interested in shipping new commands for the CLI, check out our [doc on internal contributions][intake-doc]. + ## Installation ### macOS -`gh` is available via [Homebrew][], [MacPorts][], [Conda][], and as a downloadable binary from the [releases page][]. +`gh` is available via [Homebrew][], [MacPorts][], [Conda][], [Spack][], and as a downloadable binary from the [releases page][]. #### Homebrew @@ -41,21 +43,34 @@ If anything feels off, or if you feel that some functionality is missing, please Additional Conda installation options available on the [gh-feedstock page](https://github.com/conda-forge/gh-feedstock#installing-gh). +#### Spack + +| Install: | Upgrade: | +| ------------------ | ---------------------------------------- | +| `spack install gh` | `spack uninstall gh && spack install gh` | + ### Linux & BSD -`gh` is available via [Homebrew](#homebrew), [Conda](#Conda), and as downloadable binaries from the [releases page][]. +`gh` is available via: +- [our Debian and RPM repositories](./docs/install_linux.md); +- community-maintained repositories in various Linux distros; +- OS-agnostic package managers such as [Homebrew](#homebrew), [Conda](#conda), and [Spack](#spack); and +- our [releases page][] as precompiled binaries. -For instructions on specific distributions and package managers, see [Linux & BSD installation](./docs/install_linux.md). +For more information, see [Linux & BSD installation](./docs/install_linux.md). ### Windows -`gh` is available via [WinGet][], [scoop][], [Chocolatey][], [Conda](#Conda), and as downloadable MSI. +`gh` is available via [WinGet][], [scoop][], [Chocolatey][], [Conda](#conda), and as downloadable MSI. #### WinGet | Install: | Upgrade: | | ------------------- | --------------------| -| `winget install gh` | `winget upgrade gh` | +| `winget install --id GitHub.cli` | `winget upgrade --id GitHub.cli` | + +> **Note** +> The Windows installer modifes your PATH. When using Windows Terminal, you will need to **open a new window** for the changes to take affect. (Simply opening a new tab will _not_ be sufficient.) #### scoop @@ -73,6 +88,16 @@ For instructions on specific distributions and package managers, see [Linux & BS MSI installers are available for download on the [releases page][]. +### Codespaces + +To add GitHub CLI to your codespace, add the following to your [devcontainer file](https://docs.github.com/en/codespaces/setting-up-your-project-for-codespaces/adding-features-to-a-devcontainer-file): + +```json +"features": { + "ghcr.io/devcontainers/features/github-cli:1": {} +} +``` + ### GitHub Actions GitHub CLI comes pre-installed in all [GitHub-Hosted Runners](https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners). @@ -99,8 +124,10 @@ tool. Check out our [more detailed explanation][gh-vs-hub] to learn more. [scoop]: https://scoop.sh [Chocolatey]: https://chocolatey.org [Conda]: https://docs.conda.io/en/latest/ +[Spack]: https://spack.io [releases page]: https://github.com/cli/cli/releases/latest [hub]: https://github.com/github/hub [contributing]: ./.github/CONTRIBUTING.md [gh-vs-hub]: ./docs/gh-vs-hub.md [build from source]: ./docs/source.md +[intake-doc]: ./docs/working-with-us.md diff --git a/api/cache.go b/api/cache.go deleted file mode 100644 index fc9d1c5ba2a..00000000000 --- a/api/cache.go +++ /dev/null @@ -1,179 +0,0 @@ -package api - -import ( - "bufio" - "bytes" - "crypto/sha256" - "errors" - "fmt" - "io" - "io/ioutil" - "net/http" - "os" - "path/filepath" - "strings" - "sync" - "time" -) - -func NewCachedClient(httpClient *http.Client, cacheTTL time.Duration) *http.Client { - cacheDir := filepath.Join(os.TempDir(), "gh-cli-cache") - return &http.Client{ - Transport: CacheResponse(cacheTTL, cacheDir)(httpClient.Transport), - } -} - -func isCacheableRequest(req *http.Request) bool { - if strings.EqualFold(req.Method, "GET") || strings.EqualFold(req.Method, "HEAD") { - return true - } - - if strings.EqualFold(req.Method, "POST") && (req.URL.Path == "/graphql" || req.URL.Path == "/api/graphql") { - return true - } - - return false -} - -func isCacheableResponse(res *http.Response) bool { - return res.StatusCode < 500 && res.StatusCode != 403 -} - -// CacheResponse produces a RoundTripper that caches HTTP responses to disk for a specified amount of time -func CacheResponse(ttl time.Duration, dir string) ClientOption { - fs := fileStorage{ - dir: dir, - ttl: ttl, - mu: &sync.RWMutex{}, - } - - return func(tr http.RoundTripper) http.RoundTripper { - return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) { - if !isCacheableRequest(req) { - return tr.RoundTrip(req) - } - - key, keyErr := cacheKey(req) - if keyErr == nil { - if res, err := fs.read(key); err == nil { - res.Request = req - return res, nil - } - } - - res, err := tr.RoundTrip(req) - if err == nil && keyErr == nil && isCacheableResponse(res) { - _ = fs.store(key, res) - } - return res, err - }} - } -} - -func copyStream(r io.ReadCloser) (io.ReadCloser, io.ReadCloser) { - b := &bytes.Buffer{} - nr := io.TeeReader(r, b) - return ioutil.NopCloser(b), &readCloser{ - Reader: nr, - Closer: r, - } -} - -type readCloser struct { - io.Reader - io.Closer -} - -func cacheKey(req *http.Request) (string, error) { - h := sha256.New() - fmt.Fprintf(h, "%s:", req.Method) - fmt.Fprintf(h, "%s:", req.URL.String()) - fmt.Fprintf(h, "%s:", req.Header.Get("Accept")) - fmt.Fprintf(h, "%s:", req.Header.Get("Authorization")) - - if req.Body != nil { - var bodyCopy io.ReadCloser - req.Body, bodyCopy = copyStream(req.Body) - defer bodyCopy.Close() - if _, err := io.Copy(h, bodyCopy); err != nil { - return "", err - } - } - - digest := h.Sum(nil) - return fmt.Sprintf("%x", digest), nil -} - -type fileStorage struct { - dir string - ttl time.Duration - mu *sync.RWMutex -} - -func (fs *fileStorage) filePath(key string) string { - if len(key) >= 6 { - return filepath.Join(fs.dir, key[0:2], key[2:4], key[4:]) - } - return filepath.Join(fs.dir, key) -} - -func (fs *fileStorage) read(key string) (*http.Response, error) { - cacheFile := fs.filePath(key) - - fs.mu.RLock() - defer fs.mu.RUnlock() - - f, err := os.Open(cacheFile) - if err != nil { - return nil, err - } - defer f.Close() - - stat, err := f.Stat() - if err != nil { - return nil, err - } - - age := time.Since(stat.ModTime()) - if age > fs.ttl { - return nil, errors.New("cache expired") - } - - body := &bytes.Buffer{} - _, err = io.Copy(body, f) - if err != nil { - return nil, err - } - - res, err := http.ReadResponse(bufio.NewReader(body), nil) - return res, err -} - -func (fs *fileStorage) store(key string, res *http.Response) error { - cacheFile := fs.filePath(key) - - fs.mu.Lock() - defer fs.mu.Unlock() - - err := os.MkdirAll(filepath.Dir(cacheFile), 0755) - if err != nil { - return err - } - - f, err := os.OpenFile(cacheFile, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) - if err != nil { - return err - } - defer f.Close() - - var origBody io.ReadCloser - if res.Body != nil { - origBody, res.Body = copyStream(res.Body) - defer res.Body.Close() - } - err = res.Write(f) - if origBody != nil { - res.Body = origBody - } - return err -} diff --git a/api/cache_test.go b/api/cache_test.go deleted file mode 100644 index f4a6a756ee4..00000000000 --- a/api/cache_test.go +++ /dev/null @@ -1,89 +0,0 @@ -package api - -import ( - "bytes" - "fmt" - "io" - "io/ioutil" - "net/http" - "path/filepath" - "testing" - "time" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func Test_CacheResponse(t *testing.T) { - counter := 0 - fakeHTTP := funcTripper{ - roundTrip: func(req *http.Request) (*http.Response, error) { - counter += 1 - body := fmt.Sprintf("%d: %s %s", counter, req.Method, req.URL.String()) - status := 200 - if req.URL.Path == "/error" { - status = 500 - } - return &http.Response{ - StatusCode: status, - Body: ioutil.NopCloser(bytes.NewBufferString(body)), - }, nil - }, - } - - cacheDir := filepath.Join(t.TempDir(), "gh-cli-cache") - httpClient := NewHTTPClient(ReplaceTripper(fakeHTTP), CacheResponse(time.Minute, cacheDir)) - - do := func(method, url string, body io.Reader) (string, error) { - req, err := http.NewRequest(method, url, body) - if err != nil { - return "", err - } - res, err := httpClient.Do(req) - if err != nil { - return "", err - } - defer res.Body.Close() - resBody, err := ioutil.ReadAll(res.Body) - if err != nil { - err = fmt.Errorf("ReadAll: %w", err) - } - return string(resBody), err - } - - var res string - var err error - - res, err = do("GET", "http://example.com/path", nil) - require.NoError(t, err) - assert.Equal(t, "1: GET http://example.com/path", res) - res, err = do("GET", "http://example.com/path", nil) - require.NoError(t, err) - assert.Equal(t, "1: GET http://example.com/path", res) - - res, err = do("GET", "http://example.com/path2", nil) - require.NoError(t, err) - assert.Equal(t, "2: GET http://example.com/path2", res) - - res, err = do("POST", "http://example.com/path2", nil) - require.NoError(t, err) - assert.Equal(t, "3: POST http://example.com/path2", res) - - res, err = do("POST", "http://example.com/graphql", bytes.NewBufferString(`hello`)) - require.NoError(t, err) - assert.Equal(t, "4: POST http://example.com/graphql", res) - res, err = do("POST", "http://example.com/graphql", bytes.NewBufferString(`hello`)) - require.NoError(t, err) - assert.Equal(t, "4: POST http://example.com/graphql", res) - - res, err = do("POST", "http://example.com/graphql", bytes.NewBufferString(`hello2`)) - require.NoError(t, err) - assert.Equal(t, "5: POST http://example.com/graphql", res) - - res, err = do("GET", "http://example.com/error", nil) - require.NoError(t, err) - assert.Equal(t, "6: GET http://example.com/error", res) - res, err = do("GET", "http://example.com/error", nil) - require.NoError(t, err) - assert.Equal(t, "7: GET http://example.com/error", res) -} diff --git a/api/client.go b/api/client.go index a5741a42eb9..0447051ccef 100644 --- a/api/client.go +++ b/api/client.go @@ -1,112 +1,36 @@ package api import ( - "bytes" + "context" "encoding/json" + "errors" "fmt" "io" - "io/ioutil" "net/http" - "net/url" "regexp" "strings" "github.com/cli/cli/v2/internal/ghinstance" - "github.com/henvic/httpretty" - "github.com/shurcooL/graphql" + "github.com/cli/go-gh" + ghAPI "github.com/cli/go-gh/pkg/api" ) -// ClientOption represents an argument to NewClient -type ClientOption = func(http.RoundTripper) http.RoundTripper - -// NewHTTPClient initializes an http.Client -func NewHTTPClient(opts ...ClientOption) *http.Client { - tr := http.DefaultTransport - for _, opt := range opts { - tr = opt(tr) - } - return &http.Client{Transport: tr} -} +const ( + accept = "Accept" + authorization = "Authorization" + cacheTTL = "X-GH-CACHE-TTL" + graphqlFeatures = "GraphQL-Features" + features = "merge_queue" + userAgent = "User-Agent" +) -// NewClient initializes a Client -func NewClient(opts ...ClientOption) *Client { - client := &Client{http: NewHTTPClient(opts...)} - return client -} +var linkRE = regexp.MustCompile(`<([^>]+)>;\s*rel="([^"]+)"`) -// NewClientFromHTTP takes in an http.Client instance func NewClientFromHTTP(httpClient *http.Client) *Client { client := &Client{http: httpClient} return client } -// AddHeader turns a RoundTripper into one that adds a request header -func AddHeader(name, value string) ClientOption { - return func(tr http.RoundTripper) http.RoundTripper { - return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) { - if req.Header.Get(name) == "" { - req.Header.Add(name, value) - } - return tr.RoundTrip(req) - }} - } -} - -// AddHeaderFunc is an AddHeader that gets the string value from a function -func AddHeaderFunc(name string, getValue func(*http.Request) (string, error)) ClientOption { - return func(tr http.RoundTripper) http.RoundTripper { - return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) { - if req.Header.Get(name) != "" { - return tr.RoundTrip(req) - } - value, err := getValue(req) - if err != nil { - return nil, err - } - if value != "" { - req.Header.Add(name, value) - } - return tr.RoundTrip(req) - }} - } -} - -// VerboseLog enables request/response logging within a RoundTripper -func VerboseLog(out io.Writer, logTraffic bool, colorize bool) ClientOption { - logger := &httpretty.Logger{ - Time: true, - TLS: false, - Colors: colorize, - RequestHeader: logTraffic, - RequestBody: logTraffic, - ResponseHeader: logTraffic, - ResponseBody: logTraffic, - Formatters: []httpretty.Formatter{&httpretty.JSONFormatter{}}, - MaxResponseBody: 10000, - } - logger.SetOutput(out) - logger.SetBodyFilter(func(h http.Header) (skip bool, err error) { - return !inspectableMIMEType(h.Get("Content-Type")), nil - }) - return logger.RoundTripper -} - -// ReplaceTripper substitutes the underlying RoundTripper with a custom one -func ReplaceTripper(tr http.RoundTripper) ClientOption { - return func(http.RoundTripper) http.RoundTripper { - return tr - } -} - -type funcTripper struct { - roundTrip func(*http.Request) (*http.Response, error) -} - -func (tr funcTripper) RoundTrip(req *http.Request) (*http.Response, error) { - return tr.roundTrip(req) -} - -// Client facilitates making HTTP requests to the GitHub API type Client struct { http *http.Client } @@ -115,221 +39,236 @@ func (c *Client) HTTP() *http.Client { return c.http } -type graphQLResponse struct { - Data interface{} - Errors []GraphQLError -} - -// GraphQLError is a single error returned in a GraphQL response type GraphQLError struct { - Type string - Message string - // Path []interface // mixed strings and numbers -} - -// GraphQLErrorResponse contains errors returned in a GraphQL response -type GraphQLErrorResponse struct { - Errors []GraphQLError -} - -func (gr GraphQLErrorResponse) Error() string { - errorMessages := make([]string, 0, len(gr.Errors)) - for _, e := range gr.Errors { - errorMessages = append(errorMessages, e.Message) - } - return fmt.Sprintf("GraphQL error: %s", strings.Join(errorMessages, "\n")) + ghAPI.GQLError } -// HTTPError is an error returned by a failed API call type HTTPError struct { - StatusCode int - RequestURL *url.URL - Message string - OAuthScopes string - Errors []HTTPErrorItem + ghAPI.HTTPError + scopesSuggestion string } -type HTTPErrorItem struct { - Message string - Resource string - Field string - Code string +func (err HTTPError) ScopesSuggestion() string { + return err.scopesSuggestion } -func (err HTTPError) Error() string { - if msgs := strings.SplitN(err.Message, "\n", 2); len(msgs) > 1 { - return fmt.Sprintf("HTTP %d: %s (%s)\n%s", err.StatusCode, msgs[0], err.RequestURL, msgs[1]) - } else if err.Message != "" { - return fmt.Sprintf("HTTP %d: %s (%s)", err.StatusCode, err.Message, err.RequestURL) - } - return fmt.Sprintf("HTTP %d (%s)", err.StatusCode, err.RequestURL) -} - -// GraphQL performs a GraphQL request and parses the response +// GraphQL performs a GraphQL request using the query string and parses the response into data receiver. If there are errors in the response, +// GraphQLError will be returned, but the receiver will also be partially populated. func (c Client) GraphQL(hostname string, query string, variables map[string]interface{}, data interface{}) error { - reqBody, err := json.Marshal(map[string]interface{}{"query": query, "variables": variables}) + opts := clientOptions(hostname, c.http.Transport) + opts.Headers[graphqlFeatures] = features + gqlClient, err := gh.GQLClient(&opts) if err != nil { return err } + return handleResponse(gqlClient.Do(query, variables, data)) +} - req, err := http.NewRequest("POST", ghinstance.GraphQLEndpoint(hostname), bytes.NewBuffer(reqBody)) +// Mutate performs a GraphQL mutation based on a struct and parses the response with the same struct as the receiver. If there are errors in the response, +// GraphQLError will be returned, but the receiver will also be partially populated. +func (c Client) Mutate(hostname, name string, mutation interface{}, variables map[string]interface{}) error { + opts := clientOptions(hostname, c.http.Transport) + opts.Headers[graphqlFeatures] = features + gqlClient, err := gh.GQLClient(&opts) if err != nil { return err } + return handleResponse(gqlClient.Mutate(name, mutation, variables)) +} - req.Header.Set("Content-Type", "application/json; charset=utf-8") - - resp, err := c.http.Do(req) +// Query performs a GraphQL query based on a struct and parses the response with the same struct as the receiver. If there are errors in the response, +// GraphQLError will be returned, but the receiver will also be partially populated. +func (c Client) Query(hostname, name string, query interface{}, variables map[string]interface{}) error { + opts := clientOptions(hostname, c.http.Transport) + opts.Headers[graphqlFeatures] = features + gqlClient, err := gh.GQLClient(&opts) if err != nil { return err } - defer resp.Body.Close() - - return handleResponse(resp, data) + return handleResponse(gqlClient.Query(name, query, variables)) } -func graphQLClient(h *http.Client, hostname string) *graphql.Client { - return graphql.NewClient(ghinstance.GraphQLEndpoint(hostname), h) +// QueryWithContext performs a GraphQL query based on a struct and parses the response with the same struct as the receiver. If there are errors in the response, +// GraphQLError will be returned, but the receiver will also be partially populated. +func (c Client) QueryWithContext(ctx context.Context, hostname, name string, query interface{}, variables map[string]interface{}) error { + opts := clientOptions(hostname, c.http.Transport) + opts.Headers[graphqlFeatures] = features + gqlClient, err := gh.GQLClient(&opts) + if err != nil { + return err + } + return handleResponse(gqlClient.QueryWithContext(ctx, name, query, variables)) } // REST performs a REST request and parses the response. func (c Client) REST(hostname string, method string, p string, body io.Reader, data interface{}) error { - req, err := http.NewRequest(method, restURL(hostname, p), body) + opts := clientOptions(hostname, c.http.Transport) + restClient, err := gh.RESTClient(&opts) if err != nil { return err } + return handleResponse(restClient.Do(method, p, body, data)) +} - req.Header.Set("Content-Type", "application/json; charset=utf-8") +func (c Client) RESTWithNext(hostname string, method string, p string, body io.Reader, data interface{}) (string, error) { + opts := clientOptions(hostname, c.http.Transport) + restClient, err := gh.RESTClient(&opts) + if err != nil { + return "", err + } - resp, err := c.http.Do(req) + resp, err := restClient.Request(method, p, body) if err != nil { - return err + return "", err } defer resp.Body.Close() success := resp.StatusCode >= 200 && resp.StatusCode < 300 if !success { - return HandleHTTPError(resp) + return "", HandleHTTPError(resp) } if resp.StatusCode == http.StatusNoContent { - return nil + return "", nil } - b, err := ioutil.ReadAll(resp.Body) + b, err := io.ReadAll(resp.Body) if err != nil { - return err + return "", err } + err = json.Unmarshal(b, &data) if err != nil { - return err + return "", err } - return nil -} - -func restURL(hostname string, pathOrURL string) string { - if strings.HasPrefix(pathOrURL, "https://") || strings.HasPrefix(pathOrURL, "http://") { - return pathOrURL + var next string + for _, m := range linkRE.FindAllStringSubmatch(resp.Header.Get("Link"), -1) { + if len(m) > 2 && m[2] == "next" { + next = m[1] + } } - return ghinstance.RESTPrefix(hostname) + pathOrURL + + return next, nil } -func handleResponse(resp *http.Response, data interface{}) error { - success := resp.StatusCode >= 200 && resp.StatusCode < 300 +// HandleHTTPError parses a http.Response into a HTTPError. +func HandleHTTPError(resp *http.Response) error { + return handleResponse(ghAPI.HandleHTTPError(resp)) +} - if !success { - return HandleHTTPError(resp) +// handleResponse takes a ghAPI.HTTPError or ghAPI.GQLError and converts it into an +// HTTPError or GraphQLError respectively. +func handleResponse(err error) error { + if err == nil { + return nil } - body, err := ioutil.ReadAll(resp.Body) - if err != nil { - return err + var restErr ghAPI.HTTPError + if errors.As(err, &restErr) { + return HTTPError{ + HTTPError: restErr, + scopesSuggestion: generateScopesSuggestion(restErr.StatusCode, + restErr.Headers.Get("X-Accepted-Oauth-Scopes"), + restErr.Headers.Get("X-Oauth-Scopes"), + restErr.RequestURL.Hostname()), + } } - gr := &graphQLResponse{Data: data} - err = json.Unmarshal(body, &gr) - if err != nil { - return err + var gqlErr ghAPI.GQLError + if errors.As(err, &gqlErr) { + return GraphQLError{ + GQLError: gqlErr, + } } - if len(gr.Errors) > 0 { - return &GraphQLErrorResponse{Errors: gr.Errors} - } - return nil + return err } -func HandleHTTPError(resp *http.Response) error { - httpError := HTTPError{ - StatusCode: resp.StatusCode, - RequestURL: resp.Request.URL, - OAuthScopes: resp.Header.Get("X-Oauth-Scopes"), - } +// ScopesSuggestion is an error messaging utility that prints the suggestion to request additional OAuth +// scopes in case a server response indicates that there are missing scopes. +func ScopesSuggestion(resp *http.Response) string { + return generateScopesSuggestion(resp.StatusCode, + resp.Header.Get("X-Accepted-Oauth-Scopes"), + resp.Header.Get("X-Oauth-Scopes"), + resp.Request.URL.Hostname()) +} - if !jsonTypeRE.MatchString(resp.Header.Get("Content-Type")) { - httpError.Message = resp.Status - return httpError +// EndpointNeedsScopes adds additional OAuth scopes to an HTTP response as if they were returned from the +// server endpoint. This improves HTTP 4xx error messaging for endpoints that don't explicitly list the +// OAuth scopes they need. +func EndpointNeedsScopes(resp *http.Response, s string) *http.Response { + if resp.StatusCode >= 400 && resp.StatusCode < 500 { + oldScopes := resp.Header.Get("X-Accepted-Oauth-Scopes") + resp.Header.Set("X-Accepted-Oauth-Scopes", fmt.Sprintf("%s, %s", oldScopes, s)) } + return resp +} - body, err := ioutil.ReadAll(resp.Body) - if err != nil { - httpError.Message = err.Error() - return httpError +func generateScopesSuggestion(statusCode int, endpointNeedsScopes, tokenHasScopes, hostname string) string { + if statusCode < 400 || statusCode > 499 || statusCode == 422 { + return "" } - var parsedBody struct { - Message string `json:"message"` - Errors []json.RawMessage - } - if err := json.Unmarshal(body, &parsedBody); err != nil { - return httpError + if tokenHasScopes == "" { + return "" } - var messages []string - if parsedBody.Message != "" { - messages = append(messages, parsedBody.Message) + gotScopes := map[string]struct{}{} + for _, s := range strings.Split(tokenHasScopes, ",") { + s = strings.TrimSpace(s) + gotScopes[s] = struct{}{} + + // Certain scopes may be grouped under a single "top-level" scope. The following branch + // statements include these grouped/implied scopes when the top-level scope is encountered. + // See https://docs.github.com/en/developers/apps/building-oauth-apps/scopes-for-oauth-apps. + if s == "repo" { + gotScopes["repo:status"] = struct{}{} + gotScopes["repo_deployment"] = struct{}{} + gotScopes["public_repo"] = struct{}{} + gotScopes["repo:invite"] = struct{}{} + gotScopes["security_events"] = struct{}{} + } else if s == "user" { + gotScopes["read:user"] = struct{}{} + gotScopes["user:email"] = struct{}{} + gotScopes["user:follow"] = struct{}{} + } else if s == "codespace" { + gotScopes["codespace:secrets"] = struct{}{} + } else if strings.HasPrefix(s, "admin:") { + gotScopes["read:"+strings.TrimPrefix(s, "admin:")] = struct{}{} + gotScopes["write:"+strings.TrimPrefix(s, "admin:")] = struct{}{} + } else if strings.HasPrefix(s, "write:") { + gotScopes["read:"+strings.TrimPrefix(s, "write:")] = struct{}{} + } } - for _, raw := range parsedBody.Errors { - switch raw[0] { - case '"': - var errString string - _ = json.Unmarshal(raw, &errString) - messages = append(messages, errString) - httpError.Errors = append(httpError.Errors, HTTPErrorItem{Message: errString}) - case '{': - var errInfo HTTPErrorItem - _ = json.Unmarshal(raw, &errInfo) - msg := errInfo.Message - if errInfo.Code != "" && errInfo.Code != "custom" { - msg = fmt.Sprintf("%s.%s %s", errInfo.Resource, errInfo.Field, errorCodeToMessage(errInfo.Code)) - } - if msg != "" { - messages = append(messages, msg) - } - httpError.Errors = append(httpError.Errors, errInfo) + + for _, s := range strings.Split(endpointNeedsScopes, ",") { + s = strings.TrimSpace(s) + if _, gotScope := gotScopes[s]; s == "" || gotScope { + continue } + return fmt.Sprintf( + "This API operation needs the %[1]q scope. To request it, run: gh auth refresh -h %[2]s -s %[1]s", + s, + ghinstance.NormalizeHostname(hostname), + ) } - httpError.Message = strings.Join(messages, "\n") - return httpError + return "" } -func errorCodeToMessage(code string) string { - // https://docs.github.com/en/rest/overview/resources-in-the-rest-api#client-errors - switch code { - case "missing", "missing_field": - return "is missing" - case "invalid", "unprocessable": - return "is invalid" - case "already_exists": - return "already exists" - default: - return code +func clientOptions(hostname string, transport http.RoundTripper) ghAPI.ClientOptions { + // AuthToken, and Headers are being handled by transport, + // so let go-gh know that it does not need to resolve them. + opts := ghAPI.ClientOptions{ + AuthToken: "none", + Headers: map[string]string{ + authorization: "", + }, + Host: hostname, + SkipDefaultHeaders: true, + Transport: transport, + LogIgnoreEnv: true, } -} - -var jsonTypeRE = regexp.MustCompile(`[/+]json($|;)`) - -func inspectableMIMEType(t string) bool { - return strings.HasPrefix(t, "text/") || jsonTypeRE.MatchString(t) + return opts } diff --git a/api/client_test.go b/api/client_test.go index 50665fddbeb..0f36e3f6972 100644 --- a/api/client_test.go +++ b/api/client_test.go @@ -3,20 +3,25 @@ package api import ( "bytes" "errors" - "io/ioutil" + "io" "net/http" + "net/http/httptest" "testing" "github.com/cli/cli/v2/pkg/httpmock" + "github.com/cli/cli/v2/pkg/iostreams" "github.com/stretchr/testify/assert" ) +func newTestClient(reg *httpmock.Registry) *Client { + client := &http.Client{} + httpmock.ReplaceTripper(client, reg) + return NewClientFromHTTP(client) +} + func TestGraphQL(t *testing.T) { http := &httpmock.Registry{} - client := NewClient( - ReplaceTripper(http), - AddHeader("Authorization", "token OTOKEN"), - ) + client := newTestClient(http) vars := map[string]interface{}{"name": "Mona"} response := struct { @@ -35,40 +40,44 @@ func TestGraphQL(t *testing.T) { assert.Equal(t, "hubot", response.Viewer.Login) req := http.Requests[0] - reqBody, _ := ioutil.ReadAll(req.Body) + reqBody, _ := io.ReadAll(req.Body) assert.Equal(t, `{"query":"QUERY","variables":{"name":"Mona"}}`, string(reqBody)) - assert.Equal(t, "token OTOKEN", req.Header.Get("Authorization")) } func TestGraphQLError(t *testing.T) { - http := &httpmock.Registry{} - client := NewClient(ReplaceTripper(http)) + reg := &httpmock.Registry{} + client := newTestClient(reg) response := struct{}{} - http.Register( + reg.Register( httpmock.GraphQL(""), httpmock.StringResponse(` { "errors": [ - {"message":"OH NO"}, - {"message":"this is fine"} + { + "type": "NOT_FOUND", + "message": "OH NO", + "path": ["repository", "issue"] + }, + { + "type": "ACTUALLY_ITS_FINE", + "message": "this is fine", + "path": ["repository", "issues", 0, "comments"] + } ] } `), ) err := client.GraphQL("github.com", "", nil, &response) - if err == nil || err.Error() != "GraphQL error: OH NO\nthis is fine" { + if err == nil || err.Error() != "GraphQL: OH NO (repository.issue), this is fine (repository.issues.0.comments)" { t.Fatalf("got %q", err.Error()) } } func TestRESTGetDelete(t *testing.T) { http := &httpmock.Registry{} - - client := NewClient( - ReplaceTripper(http), - ) + client := newTestClient(http) http.Register( httpmock.REST("DELETE", "applications/CLIENTID/grant"), @@ -82,7 +91,7 @@ func TestRESTGetDelete(t *testing.T) { func TestRESTWithFullURL(t *testing.T) { http := &httpmock.Registry{} - client := NewClient(ReplaceTripper(http)) + client := newTestClient(http) http.Register( httpmock.REST("GET", "api/v3/user/repos"), @@ -102,13 +111,13 @@ func TestRESTWithFullURL(t *testing.T) { func TestRESTError(t *testing.T) { fakehttp := &httpmock.Registry{} - client := NewClient(ReplaceTripper(fakehttp)) + client := newTestClient(fakehttp) fakehttp.Register(httpmock.MatchAny, func(req *http.Request) (*http.Response, error) { return &http.Response{ Request: req, StatusCode: 422, - Body: ioutil.NopCloser(bytes.NewBufferString(`{"message": "OH NO"}`)), + Body: io.NopCloser(bytes.NewBufferString(`{"message": "OH NO"}`)), Header: map[string][]string{ "Content-Type": {"application/json; charset=utf-8"}, }, @@ -126,7 +135,6 @@ func TestRESTError(t *testing.T) { } if httpErr.Error() != "HTTP 422: OH NO (https://api.github.com/repos/branch)" { t.Errorf("got %q", httpErr.Error()) - } } @@ -138,7 +146,7 @@ func TestHandleHTTPError_GraphQL502(t *testing.T) { resp := &http.Response{ Request: req, StatusCode: 502, - Body: ioutil.NopCloser(bytes.NewBufferString(`{ "data": null, "errors": [{ "message": "Something went wrong" }] }`)), + Body: io.NopCloser(bytes.NewBufferString(`{ "data": null, "errors": [{ "message": "Something went wrong" }] }`)), Header: map[string][]string{"Content-Type": {"application/json"}}, } err = HandleHTTPError(resp) @@ -146,3 +154,105 @@ func TestHandleHTTPError_GraphQL502(t *testing.T) { t.Errorf("got error: %v", err) } } + +func TestHTTPError_ScopesSuggestion(t *testing.T) { + makeResponse := func(s int, u, haveScopes, needScopes string) *http.Response { + req, err := http.NewRequest("GET", u, nil) + if err != nil { + t.Fatal(err) + } + return &http.Response{ + Request: req, + StatusCode: s, + Body: io.NopCloser(bytes.NewBufferString(`{}`)), + Header: map[string][]string{ + "Content-Type": {"application/json"}, + "X-Oauth-Scopes": {haveScopes}, + "X-Accepted-Oauth-Scopes": {needScopes}, + }, + } + } + + tests := []struct { + name string + resp *http.Response + want string + }{ + { + name: "has necessary scopes", + resp: makeResponse(404, "https://api.github.com/gists", "repo, gist, read:org", "gist"), + want: ``, + }, + { + name: "normalizes scopes", + resp: makeResponse(404, "https://api.github.com/orgs/ORG/discussions", "admin:org, write:discussion", "read:org, read:discussion"), + want: ``, + }, + { + name: "no scopes on endpoint", + resp: makeResponse(404, "https://api.github.com/user", "repo", ""), + want: ``, + }, + { + name: "missing a scope", + resp: makeResponse(404, "https://api.github.com/gists", "repo, read:org", "gist, delete_repo"), + want: `This API operation needs the "gist" scope. To request it, run: gh auth refresh -h github.com -s gist`, + }, + { + name: "server error", + resp: makeResponse(500, "https://api.github.com/gists", "repo", "gist"), + want: ``, + }, + { + name: "no scopes on token", + resp: makeResponse(404, "https://api.github.com/gists", "", "gist, delete_repo"), + want: ``, + }, + { + name: "http code is 422", + resp: makeResponse(422, "https://api.github.com/gists", "", "gist"), + want: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + httpError := HandleHTTPError(tt.resp) + if got := httpError.(HTTPError).ScopesSuggestion(); got != tt.want { + t.Errorf("HTTPError.ScopesSuggestion() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestHTTPHeaders(t *testing.T) { + var gotReq *http.Request + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotReq = r + w.WriteHeader(http.StatusNoContent) + })) + defer ts.Close() + + ios, _, _, stderr := iostreams.Test() + httpClient, err := NewHTTPClient(HTTPClientOptions{ + AppVersion: "v1.2.3", + Config: tinyConfig{ts.URL[7:] + ":oauth_token": "MYTOKEN"}, + Log: ios.ErrOut, + SkipAcceptHeaders: false, + }) + assert.NoError(t, err) + client := NewClientFromHTTP(httpClient) + + err = client.REST(ts.URL, "GET", ts.URL+"/user/repos", nil, nil) + assert.NoError(t, err) + + wantHeader := map[string]string{ + "Accept": "application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview", + "Authorization": "token MYTOKEN", + "Content-Type": "application/json; charset=utf-8", + "User-Agent": "GitHub CLI v1.2.3", + } + for name, value := range wantHeader { + assert.Equal(t, value, gotReq.Header.Get(name), name) + } + assert.Equal(t, "", stderr.String()) +} diff --git a/api/export_pr.go b/api/export_pr.go index 29a5c4a639a..c2ddb427709 100644 --- a/api/export_pr.go +++ b/api/export_pr.go @@ -5,7 +5,7 @@ import ( "strings" ) -func (issue *Issue) ExportData(fields []string) *map[string]interface{} { +func (issue *Issue) ExportData(fields []string) map[string]interface{} { v := reflect.ValueOf(issue).Elem() data := map[string]interface{}{} @@ -19,16 +19,24 @@ func (issue *Issue) ExportData(fields []string) *map[string]interface{} { data[f] = issue.Labels.Nodes case "projectCards": data[f] = issue.ProjectCards.Nodes + case "projectItems": + items := make([]map[string]interface{}, 0, len(issue.ProjectItems.Nodes)) + for _, n := range issue.ProjectItems.Nodes { + items = append(items, map[string]interface{}{ + "title": n.Project.Title, + }) + } + data[f] = items default: sf := fieldByName(v, f) data[f] = sf.Interface() } } - return &data + return data } -func (pr *PullRequest) ExportData(fields []string) *map[string]interface{} { +func (pr *PullRequest) ExportData(fields []string) map[string]interface{} { v := reflect.ValueOf(pr).Elem() data := map[string]interface{}{} @@ -38,7 +46,30 @@ func (pr *PullRequest) ExportData(fields []string) *map[string]interface{} { data[f] = pr.HeadRepository case "statusCheckRollup": if n := pr.StatusCheckRollup.Nodes; len(n) > 0 { - data[f] = n[0].Commit.StatusCheckRollup.Contexts.Nodes + checks := make([]interface{}, 0, len(n[0].Commit.StatusCheckRollup.Contexts.Nodes)) + for _, c := range n[0].Commit.StatusCheckRollup.Contexts.Nodes { + if c.TypeName == "CheckRun" { + checks = append(checks, map[string]interface{}{ + "__typename": c.TypeName, + "name": c.Name, + "workflowName": c.CheckSuite.WorkflowRun.Workflow.Name, + "status": c.Status, + "conclusion": c.Conclusion, + "startedAt": c.StartedAt, + "completedAt": c.CompletedAt, + "detailsUrl": c.DetailsURL, + }) + } else { + checks = append(checks, map[string]interface{}{ + "__typename": c.TypeName, + "context": c.Context, + "state": c.State, + "targetUrl": c.TargetURL, + "startedAt": c.CreatedAt, + }) + } + } + data[f] = checks } else { data[f] = nil } @@ -73,8 +104,18 @@ func (pr *PullRequest) ExportData(fields []string) *map[string]interface{} { data[f] = pr.Labels.Nodes case "projectCards": data[f] = pr.ProjectCards.Nodes + case "projectItems": + items := make([]map[string]interface{}, 0, len(pr.ProjectItems.Nodes)) + for _, n := range pr.ProjectItems.Nodes { + items = append(items, map[string]interface{}{ + "title": n.Project.Title, + }) + } + data[f] = items case "reviews": data[f] = pr.Reviews.Nodes + case "latestReviews": + data[f] = pr.LatestReviews.Nodes case "files": data[f] = pr.Files.Nodes case "reviewRequests": @@ -102,7 +143,7 @@ func (pr *PullRequest) ExportData(fields []string) *map[string]interface{} { } } - return &data + return data } func fieldByName(v reflect.Value, field string) reflect.Value { diff --git a/api/export_pr_test.go b/api/export_pr_test.go index dde730884bd..e7d93007855 100644 --- a/api/export_pr_test.go +++ b/api/export_pr_test.go @@ -75,6 +75,30 @@ func TestIssue_ExportData(t *testing.T) { } `), }, + { + name: "project items", + fields: []string{"projectItems"}, + inputJSON: heredoc.Doc(` + { "projectItems": { "nodes": [ + { + "id": "PVTI_id", + "project": { + "id": "PVT_id", + "title": "Some Project" + } + } + ] } } + `), + outputJSON: heredoc.Doc(` + { + "projectItems": [ + { + "title": "Some Project" + } + ] + } + `), + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -140,11 +164,19 @@ func TestPullRequest_ExportData(t *testing.T) { { "__typename": "CheckRun", "name": "mycheck", + "checkSuite": {"workflowRun": {"workflow": {"name": "myworkflow"}}}, "status": "COMPLETED", "conclusion": "SUCCESS", "startedAt": "2020-08-31T15:44:24+02:00", "completedAt": "2020-08-31T15:45:24+02:00", "detailsUrl": "http://example.com/details" + }, + { + "__typename": "StatusContext", + "context": "mycontext", + "state": "SUCCESS", + "createdAt": "2020-08-31T15:44:24+02:00", + "targetUrl": "http://example.com/details" } ] } } } } ] } } @@ -155,11 +187,19 @@ func TestPullRequest_ExportData(t *testing.T) { { "__typename": "CheckRun", "name": "mycheck", + "workflowName": "myworkflow", "status": "COMPLETED", "conclusion": "SUCCESS", "startedAt": "2020-08-31T15:44:24+02:00", "completedAt": "2020-08-31T15:45:24+02:00", "detailsUrl": "http://example.com/details" + }, + { + "__typename": "StatusContext", + "context": "mycontext", + "state": "SUCCESS", + "startedAt": "2020-08-31T15:44:24+02:00", + "targetUrl": "http://example.com/details" } ] } @@ -178,7 +218,14 @@ func TestPullRequest_ExportData(t *testing.T) { enc := json.NewEncoder(&buf) enc.SetIndent("", "\t") require.NoError(t, enc.Encode(exported)) - assert.Equal(t, tt.outputJSON, buf.String()) + + var gotData interface{} + dec = json.NewDecoder(&buf) + require.NoError(t, dec.Decode(&gotData)) + var expectData interface{} + require.NoError(t, json.Unmarshal([]byte(tt.outputJSON), &expectData)) + + assert.Equal(t, expectData, gotData) }) } } diff --git a/api/export_repo.go b/api/export_repo.go index 8d4e669ad96..a07246ab928 100644 --- a/api/export_repo.go +++ b/api/export_repo.go @@ -4,7 +4,7 @@ import ( "reflect" ) -func (repo *Repository) ExportData(fields []string) *map[string]interface{} { +func (repo *Repository) ExportData(fields []string) map[string]interface{} { v := reflect.ValueOf(repo).Elem() data := map[string]interface{}{} @@ -38,7 +38,7 @@ func (repo *Repository) ExportData(fields []string) *map[string]interface{} { } } - return &data + return data } func miniRepoExport(r *Repository) map[string]interface{} { diff --git a/api/http_client.go b/api/http_client.go new file mode 100644 index 00000000000..3fb83974eba --- /dev/null +++ b/api/http_client.go @@ -0,0 +1,133 @@ +package api + +import ( + "fmt" + "io" + "net/http" + "strings" + "time" + + "github.com/cli/cli/v2/internal/ghinstance" + "github.com/cli/cli/v2/utils" + "github.com/cli/go-gh" + ghAPI "github.com/cli/go-gh/pkg/api" +) + +type tokenGetter interface { + Token(string) (string, string) +} + +type HTTPClientOptions struct { + AppVersion string + CacheTTL time.Duration + Config tokenGetter + EnableCache bool + Log io.Writer + LogColorize bool + SkipAcceptHeaders bool +} + +func NewHTTPClient(opts HTTPClientOptions) (*http.Client, error) { + // Provide invalid host, and token values so gh.HTTPClient will not automatically resolve them. + // The real host and token are inserted at request time. + clientOpts := ghAPI.ClientOptions{ + Host: "none", + AuthToken: "none", + LogIgnoreEnv: true, + } + + if debugEnabled, debugValue := utils.IsDebugEnabled(); debugEnabled { + clientOpts.Log = opts.Log + clientOpts.LogColorize = opts.LogColorize + clientOpts.LogVerboseHTTP = strings.Contains(debugValue, "api") + } + + headers := map[string]string{ + userAgent: fmt.Sprintf("GitHub CLI %s", opts.AppVersion), + } + if opts.SkipAcceptHeaders { + headers[accept] = "" + } + clientOpts.Headers = headers + + if opts.EnableCache { + clientOpts.EnableCache = opts.EnableCache + clientOpts.CacheTTL = opts.CacheTTL + } + + client, err := gh.HTTPClient(&clientOpts) + if err != nil { + return nil, err + } + + if opts.Config != nil { + client.Transport = AddAuthTokenHeader(client.Transport, opts.Config) + } + + client.Transport = AddASCIISanitizer(client.Transport) + + return client, nil +} + +func NewCachedHTTPClient(httpClient *http.Client, ttl time.Duration) *http.Client { + newClient := *httpClient + newClient.Transport = AddCacheTTLHeader(httpClient.Transport, ttl) + return &newClient +} + +// AddCacheTTLHeader adds an header to the request telling the cache that the request +// should be cached for a specified amount of time. +func AddCacheTTLHeader(rt http.RoundTripper, ttl time.Duration) http.RoundTripper { + return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) { + // If the header is already set in the request, don't overwrite it. + if req.Header.Get(cacheTTL) == "" { + req.Header.Set(cacheTTL, ttl.String()) + } + return rt.RoundTrip(req) + }} +} + +// AddAuthToken adds an authentication token header for the host specified by the request. +func AddAuthTokenHeader(rt http.RoundTripper, cfg tokenGetter) http.RoundTripper { + return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) { + // If the header is already set in the request, don't overwrite it. + if req.Header.Get(authorization) == "" { + hostname := ghinstance.NormalizeHostname(getHost(req)) + if token, _ := cfg.Token(hostname); token != "" { + req.Header.Set(authorization, fmt.Sprintf("token %s", token)) + } + } + return rt.RoundTrip(req) + }} +} + +// ExtractHeader extracts a named header from any response received by this client and, +// if non-blank, saves it to dest. +func ExtractHeader(name string, dest *string) func(http.RoundTripper) http.RoundTripper { + return func(tr http.RoundTripper) http.RoundTripper { + return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) { + res, err := tr.RoundTrip(req) + if err == nil { + if value := res.Header.Get(name); value != "" { + *dest = value + } + } + return res, err + }} + } +} + +type funcTripper struct { + roundTrip func(*http.Request) (*http.Response, error) +} + +func (tr funcTripper) RoundTrip(req *http.Request) (*http.Response, error) { + return tr.roundTrip(req) +} + +func getHost(r *http.Request) string { + if r.Host != "" { + return r.Host + } + return r.URL.Hostname() +} diff --git a/pkg/cmd/factory/http_test.go b/api/http_client_test.go similarity index 68% rename from pkg/cmd/factory/http_test.go rename to api/http_client_test.go index 1505d1b65c3..ee59dbfc19c 100644 --- a/pkg/cmd/factory/http_test.go +++ b/api/http_client_test.go @@ -1,4 +1,4 @@ -package factory +package api import ( "fmt" @@ -16,7 +16,7 @@ import ( func TestNewHTTPClient(t *testing.T) { type args struct { - config configGetter + config tokenGetter appVersion string setAccept bool } @@ -24,6 +24,8 @@ func TestNewHTTPClient(t *testing.T) { name string args args envDebug string + setGhDebug bool + envGhDebug string host string wantHeader map[string]string wantStderr string @@ -80,8 +82,9 @@ func TestNewHTTPClient(t *testing.T) { appVersion: "v1.2.3", setAccept: true, }, - host: "github.com", - envDebug: "api", + host: "github.com", + envDebug: "api", + setGhDebug: false, wantHeader: map[string]string{ "authorization": "token MYTOKEN", "user-agent": "GitHub CLI v1.2.3", @@ -94,11 +97,45 @@ func TestNewHTTPClient(t *testing.T) { > Host: github.com > Accept: application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview > Authorization: token ████████████████████ + > Content-Type: application/json; charset=utf-8 + > Time-Zone: > User-Agent: GitHub CLI v1.2.3 - + + < HTTP/1.1 204 No Content + < Date: