cli: Crash when fetching encrypted token on Windows: `panic: runtime error: invalid memory address`

Describe the bug

The CLI crashes when I try to download a release with --repo on windows. download without --repo works from the current repo fails in --repo is specified list with --repo seems to work

Steps to reproduce the behavior

gh release download --repo "cli/cli" "v2.26.1" or gh release view--repo "cli/cli" "v2.26.1"

Expected vs actual behavior

I expect the repo to download but instead the command panics

Logs

> gh --version
gh version 2.26.1 (2023-04-04)
https://github.com/cli/cli/releases/tag/v2.26.1
> gh release download --repo "cli/cli" "v2.26.1"
⣾panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x0 addr=0x38 pc=0x489337]

goroutine 26 [running]:
github.com/zalando/go-keyring.windowsKeychain.Get({}, {0xc0003dc410?, 0x3?}, {0x0?, 0xa?})
        /home/runner/go/pkg/mod/github.com/zalando/go-keyring@v0.2.2/keyring_windows.go:21 +0x97
github.com/zalando/go-keyring.Get(...)
        /home/runner/go/pkg/mod/github.com/zalando/go-keyring@v0.2.2/keyring.go:37
github.com/cli/cli/v2/internal/config.(*AuthConfig).TokenFromKeyring(0xfb217e?, {0xfb217e?, 0x1?})
        /home/runner/work/cli/cli/internal/config/config.go:171 +0x53
github.com/cli/cli/v2/internal/config.(*AuthConfig).Token(0xc000526b60, {0xfb217e, 0xa})
        /home/runner/work/cli/cli/internal/config/config.go:135 +0x68
github.com/cli/cli/v2/api.AddAuthTokenHeader.func1(0xc000616000)
        /home/runner/work/cli/cli/api/http_client.go:96 +0xa9
github.com/cli/cli/v2/api.funcTripper.RoundTrip(...)
        /home/runner/work/cli/cli/api/http_client.go:125
github.com/cli/cli/v2/api.AddASCIISanitizer.func1(0xb?)
        /home/runner/work/cli/cli/api/sanitize_ascii.go:26 +0x3a
github.com/cli/cli/v2/api.funcTripper.RoundTrip(...)
        /home/runner/work/cli/cli/api/http_client.go:125
github.com/cli/cli/v2/api.ExtractHeader.func1.1(0x0?)
        /home/runner/work/cli/cli/api/http_client.go:109 +0x47
github.com/cli/cli/v2/api.funcTripper.RoundTrip(...)
        /home/runner/work/cli/cli/api/http_client.go:125
net/http.send(0xc000616000, {0x1ee4380, 0xc00000a760}, {0xf60c80?, 0x2882d9f0b01?, 0x0?})
        /opt/hostedtoolcache/go/1.19.7/x64/src/net/http/client.go:251 +0x5f7
net/http.(*Client).send(0xc00075d4d0, 0xc000616000, {0x0?, 0xc0007b9c50?, 0x0?})
        /opt/hostedtoolcache/go/1.19.7/x64/src/net/http/client.go:175 +0x9b
net/http.(*Client).do(0xc00075d4d0, 0xc000616000)
        /opt/hostedtoolcache/go/1.19.7/x64/src/net/http/client.go:715 +0x8fc
net/http.(*Client).Do(...)
        /opt/hostedtoolcache/go/1.19.7/x64/src/net/http/client.go:581
github.com/cli/cli/v2/pkg/cmd/release/shared.fetchReleasePath({0x1eec4d8, 0xc0005280c0}, 0xc0007b9fa0?, {0xc000039a80?, 0x3?}, {0xc000610000, 0x23})
        /home/runner/work/cli/cli/pkg/cmd/release/shared/fetch.go:206 +0xe9
github.com/cli/cli/v2/pkg/cmd/release/shared.FetchRelease.func1()
        /home/runner/work/cli/cli/pkg/cmd/release/shared/fetch.go:140 +0x1a7
created by github.com/cli/cli/v2/pkg/cmd/release/shared.FetchRelease
        /home/runner/work/cli/cli/pkg/cmd/release/shared/fetch.go:138 +0x15e

About this issue

Original URL
State: closed
Created a year ago
Reactions: 4
Comments: 19 (12 by maintainers)

Commits related to this issue

Workaround for https://github.com/cli/cli/issues/7283 — committed to PathOfBuildingCommunity/PathOfBuilding by Wires77 a year ago

Most upvoted comments

@vilmibm That’s a good call. Since I’m not able to isolate what exactly went wrong in the upstream library, and I wouldn’t know how to patch it, I think the best course of action would be to cache results from keychain library invocations so that they happen at most once per host. I’ll get to it 👍

mislav on Apr 7, 2023

I think this could be some kind of race issue, but I couldn’t make an isolated reproduction case yet.

The gh release download and gh release view commands seem affected because they both invoke FetchRelease(), which in turn kicks off 2 HTTPS requests in parallel: one to the REST API, and one to GraphQL. Each HTTPS request needs an authenticating token, so that gets queried twice (likely in parallel, or in close succession with each other) and that somehow causes wincred.GetGenericCredential to end up in a state where both return values are nil. How this happens, I have no idea yet, but since GetGenericCredential performs Windows syscalls, perhaps that either isn’t parallelism-safe or isn’t safe to invoke in quick succession. I will understand more if I manage to make an isolated case.

The --repo flag turned out to be a red herring. I was able to produce with and without the --repo flag just the same.

mislav on Apr 6, 2023

I have managed to reproduce this! Looking into

mislav on Apr 6, 2023