diff --git a/cmd/kosli/assertPRGithub.go b/cmd/kosli/assertPRGithub.go index 8979281ef..22745898a 100644 --- a/cmd/kosli/assertPRGithub.go +++ b/cmd/kosli/assertPRGithub.go @@ -40,7 +40,7 @@ func newAssertPullRequestGithubCmd(out io.Writer) *cobra.Command { Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { o.retriever = ghUtils.NewGithubRetrieverFunc(githubFlagsValues.Token, githubFlagsValues.BaseURL, - githubFlagsValues.Org, githubFlagsValues.Repository) + githubFlagsValues.Org, githubFlagsValues.Repository, global.Debug) return o.run(args) }, } diff --git a/cmd/kosli/assertPRGithub_test.go b/cmd/kosli/assertPRGithub_test.go index c846fa481..2bd2a412c 100644 --- a/cmd/kosli/assertPRGithub_test.go +++ b/cmd/kosli/assertPRGithub_test.go @@ -20,7 +20,7 @@ func (suite *AssertPRGithubCommandTestSuite) SetupTest() { suite.commitWithPR = "480e5a00379a52b8e184d6815080242a878ca295" suite.commitWithNoPR = "7d1db1c8b7e71ee0ce369f1b722cc8844d3a7af6" - ghUtils.NewGithubRetrieverFunc = func(token, baseURL, org, repository string) types.PRRetriever { + ghUtils.NewGithubRetrieverFunc = func(token, baseURL, org, repository string, debug bool) types.PRRetriever { return &ghUtils.FakeGitHubClient{ PRsByCommit: map[string][]*types.PREvidence{ suite.commitWithPR: {{URL: "https://github.com/kosli-dev/cli/pull/1", State: "MERGED"}}, diff --git a/cmd/kosli/attestPRGithub.go b/cmd/kosli/attestPRGithub.go index a4c345c70..9e96449d0 100644 --- a/cmd/kosli/attestPRGithub.go +++ b/cmd/kosli/attestPRGithub.go @@ -143,7 +143,7 @@ func newAttestGithubPRCmd(out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") o.retriever = ghUtils.NewGithubRetrieverFunc(githubFlagsValues.Token, githubFlagsValues.BaseURL, - githubFlagsValues.Org, o.repoName) + githubFlagsValues.Org, o.repoName, global.Debug) return o.run(args) }, } diff --git a/cmd/kosli/attestPRGithub_test.go b/cmd/kosli/attestPRGithub_test.go index 0e899c4a0..c888a35f1 100644 --- a/cmd/kosli/attestPRGithub_test.go +++ b/cmd/kosli/attestPRGithub_test.go @@ -33,7 +33,7 @@ func (suite *AttestGithubPRCommandTestSuite) SetupTest() { suite.commitWithNoPR, err = gv.ResolveRevision("HEAD~1") require.NoError(suite.T(), err) - ghUtils.NewGithubRetrieverFunc = func(token, baseURL, org, repository string) types.PRRetriever { + ghUtils.NewGithubRetrieverFunc = func(token, baseURL, org, repository string, debug bool) types.PRRetriever { return &ghUtils.FakeGitHubClient{ PRsByCommit: map[string][]*types.PREvidence{ suite.commitWithPR: {{URL: "https://github.com/kosli-dev/cli/pull/1", State: "MERGED"}}, diff --git a/internal/github/github.go b/internal/github/github.go index ea2e5176e..3719fbd64 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -1,10 +1,13 @@ package github import ( + "bytes" "context" "fmt" "io" + "net/http" "net/url" + "os" "strings" "time" @@ -21,6 +24,7 @@ type GithubConfig struct { BaseURL string Org string Repository string + Debug bool // Sleep is called between retries in PREvidenceByPRNumber. Defaults to // time.Sleep when nil. Override in tests to avoid real delays. Sleep func(time.Duration) @@ -34,7 +38,7 @@ type GithubFlagsTempValueHolder struct { } // NewGithubConfig returns a new GithubConfig -func NewGithubConfig(token, baseURL, org, repository string) *GithubConfig { +func NewGithubConfig(token, baseURL, org, repository string, debug bool) *GithubConfig { return &GithubConfig{ Token: token, BaseURL: baseURL, @@ -42,6 +46,7 @@ func NewGithubConfig(token, baseURL, org, repository string) *GithubConfig { // repository name must be extracted if a user is using default value from ${GITHUB_REPOSITORY} // because the value is in the format of "org/repository" Repository: extractRepoName(repository), + Debug: debug, } } @@ -52,12 +57,21 @@ func extractRepoName(fullRepositoryName string) string { return repository } -// NewGithubClientFromToken returns Github client with a token and context -func NewGithubClientFromToken(ctx context.Context, ghToken string, baseURL string) (*gh.Client, error) { +// NewGithubClientFromToken returns Github client with a token and context. +// When debug is true the underlying transport is wrapped so every HTTP +// request and response (REST and GraphQL) is dumped to stderr. +func NewGithubClientFromToken(ctx context.Context, ghToken string, baseURL string, debug bool) (*gh.Client, error) { ts := oauth2.StaticTokenSource( &oauth2.Token{AccessToken: ghToken}, ) tc := oauth2.NewClient(ctx, ts) + if debug { + base := tc.Transport + if base == nil { + base = http.DefaultTransport + } + tc.Transport = &debugTransport{base: base, out: os.Stderr} + } if baseURL != "" { client, err := gh.NewEnterpriseClient(baseURL, baseURL, tc) if err != nil { @@ -68,6 +82,100 @@ func NewGithubClientFromToken(ctx context.Context, ghToken string, baseURL strin return gh.NewClient(tc), nil } +// debugTransport is an http.RoundTripper that logs each request and +// response to its writer. Used when --debug is enabled to make GitHub +// auth/permission failures debuggable. +type debugTransport struct { + base http.RoundTripper + out io.Writer +} + +// logf writes debug output and intentionally ignores write errors — +// failures writing to stderr are not actionable from a debug transport. +func (d *debugTransport) logf(format string, args ...any) { + _, _ = fmt.Fprintf(d.out, format, args...) +} + +func (d *debugTransport) RoundTrip(req *http.Request) (*http.Response, error) { + d.logf("[debug-github] --> %s %s\n", req.Method, req.URL) + for k, vs := range req.Header { + for _, v := range vs { + d.logf("[debug-github] %s: %s\n", k, redactSensitiveHeader(k, v)) + } + } + if req.Body != nil { + bodyBytes, err := io.ReadAll(req.Body) + _ = req.Body.Close() + if err != nil { + d.logf("[debug-github] \n", err) + } else { + if len(bodyBytes) > 0 { + d.logf("[debug-github] body: %s\n", string(bodyBytes)) + } + req.Body = io.NopCloser(bytes.NewReader(bodyBytes)) + } + } + + resp, err := d.base.RoundTrip(req) + if err != nil { + d.logf("[debug-github] <-- transport error: %v\n", err) + return resp, err + } + d.logf("[debug-github] <-- %d %s (%s %s)\n", resp.StatusCode, resp.Status, req.Method, req.URL) + for k, vs := range resp.Header { + for _, v := range vs { + d.logf("[debug-github] %s: %s\n", k, redactSensitiveHeader(k, v)) + } + } + if resp.Body != nil { + respBytes, err := io.ReadAll(resp.Body) + _ = resp.Body.Close() + if err != nil { + d.logf("[debug-github] \n", err) + resp.Body = io.NopCloser(bytes.NewReader(nil)) + } else { + if len(respBytes) > 0 { + d.logf("[debug-github] body: %s\n", string(respBytes)) + } + resp.Body = io.NopCloser(bytes.NewReader(respBytes)) + } + } + return resp, nil +} + +// redactSensitiveHeader hides credentials in headers that commonly carry +// them so debug output is safe to paste into bug reports. Authorization +// keeps the last 4 chars of the token so the user can spot truncation +// or whitespace issues; cookie/proxy-auth values are fully replaced. +func redactSensitiveHeader(name, value string) string { + switch strings.ToLower(name) { + case "authorization": + return redactAuthHeader(value) + case "cookie", "set-cookie", "proxy-authorization": + return "***" + default: + return value + } +} + +// redactAuthHeader hides all but the last 4 chars of the credential so +// debug output is safe to paste into bug reports while still letting the +// user spot truncation/whitespace issues. +func redactAuthHeader(v string) string { + parts := strings.SplitN(v, " ", 2) + if len(parts) != 2 { + if len(v) <= 4 { + return "***" + } + return "***" + v[len(v)-4:] + } + tok := parts[1] + if len(tok) <= 4 { + return parts[0] + " ***" + } + return parts[0] + " ***" + tok[len(tok)-4:] +} + func graphqlEndpoint(baseURL string) string { if baseURL == "" || baseURL == "https://api.github.com" { return "https://api.github.com/graphql" @@ -84,8 +192,8 @@ func (c *GithubConfig) ProviderAndLabel() (string, string) { // parameters. It can be replaced in tests to inject a FakeGitHubClient. var NewGithubRetrieverFunc = defaultNewGithubRetriever -func defaultNewGithubRetriever(token, baseURL, org, repository string) types.PRRetriever { - return NewGithubConfig(token, baseURL, org, repository) +func defaultNewGithubRetriever(token, baseURL, org, repository string, debug bool) types.PRRetriever { + return NewGithubConfig(token, baseURL, org, repository, debug) } // ResetGithubRetrieverFunc restores NewGithubRetrieverFunc to its default. @@ -226,7 +334,7 @@ func buildPREvidence( func (c *GithubConfig) PREvidenceByPRNumber(prNumber int) (*types.PREvidence, error) { ctx := context.Background() - ghClient, err := NewGithubClientFromToken(ctx, c.Token, c.BaseURL) + ghClient, err := NewGithubClientFromToken(ctx, c.Token, c.BaseURL, c.Debug) if err != nil { return nil, err } @@ -313,7 +421,7 @@ func (c *GithubConfig) PREvidenceForCommitV2(commit string) ([]*types.PREvidence ctx := context.Background() pullRequestsEvidence := []*types.PREvidence{} - ghClient, err := NewGithubClientFromToken(ctx, c.Token, c.BaseURL) + ghClient, err := NewGithubClientFromToken(ctx, c.Token, c.BaseURL, c.Debug) if err != nil { return pullRequestsEvidence, err } @@ -423,7 +531,7 @@ func (c *GithubConfig) PREvidenceForCommitV1(commit string) ([]*types.PREvidence // PullRequestsForCommit returns a list of pull requests for a specific commit func (c *GithubConfig) PullRequestsForCommit(commit string) ([]*gh.PullRequest, error) { ctx := context.Background() - client, err := NewGithubClientFromToken(ctx, c.Token, c.BaseURL) + client, err := NewGithubClientFromToken(ctx, c.Token, c.BaseURL, c.Debug) if err != nil { return []*gh.PullRequest{}, err } @@ -437,7 +545,7 @@ func (c *GithubConfig) PullRequestsForCommit(commit string) ([]*gh.PullRequest, func (c *GithubConfig) GetPullRequestApprovers(number int) ([]string, error) { approvers := []string{} ctx := context.Background() - client, err := NewGithubClientFromToken(ctx, c.Token, c.BaseURL) + client, err := NewGithubClientFromToken(ctx, c.Token, c.BaseURL, c.Debug) if err != nil { return approvers, err } diff --git a/internal/github/github_contract_test.go b/internal/github/github_contract_test.go index cf9534f67..0169229f9 100644 --- a/internal/github/github_contract_test.go +++ b/internal/github/github_contract_test.go @@ -175,6 +175,7 @@ func TestGitHubContract_RealGitHub(t *testing.T) { "", "kosli-dev", "cli", + false, ) // commitUnknown is a validly-formatted SHA that does not exist in kosli-dev/cli. @@ -212,6 +213,7 @@ func TestGitHubContract_RealGitHub_PRByNumber(t *testing.T) { "", "kosli-dev", "cli", + false, ) runPRByNumberContractTests(t, config, testHelpers.GithubPRNumber()) diff --git a/internal/github/github_test.go b/internal/github/github_test.go index 8e9bb9d0c..3223b1781 100644 --- a/internal/github/github_test.go +++ b/internal/github/github_test.go @@ -35,7 +35,7 @@ func (suite *GithubTestSuite) TestNewGithubClientFromToken() { }, } { suite.Run(t.name, func() { - client, err := NewGithubClientFromToken(context.Background(), t.token, t.baseURL) + client, err := NewGithubClientFromToken(context.Background(), t.token, t.baseURL, false) require.NoErrorf(suite.T(), err, "was NOT expecting error but got: %s", err) require.NotNilf(suite.T(), client, "client should not be nil") }) @@ -129,3 +129,102 @@ func TestPREvidenceByPRNumber_ReturnsErrorAfterAllRetriesExhausted(t *testing.T) require.Error(t, err) require.Equal(t, 4, *attempts, "should have made 1 initial attempt + 3 retries") } + +func TestRedactAuthHeader(t *testing.T) { + for _, tc := range []struct { + name string + in string + want string + }{ + { + name: "scheme + long token keeps last 4", + in: "Bearer ghp_abcdef1234567890ABCD", + want: "Bearer ***ABCD", + }, + { + name: "scheme + short token is fully redacted", + in: "token ab", + want: "token ***", + }, + { + name: "scheme + 4-char token is fully redacted", + in: "Bearer abcd", + want: "Bearer ***", + }, + { + name: "no scheme, long value keeps last 4", + in: "nospaceshort", + want: "***hort", + }, + { + name: "no scheme, short value is fully redacted", + in: "abc", + want: "***", + }, + { + name: "empty value is fully redacted", + in: "", + want: "***", + }, + } { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.want, redactAuthHeader(tc.in)) + }) + } +} + +func TestRedactSensitiveHeader(t *testing.T) { + for _, tc := range []struct { + name string + headerName string + value string + want string + }{ + { + name: "authorization is redacted via redactAuthHeader", + headerName: "Authorization", + value: "Bearer ghp_abcdef1234567890ABCD", + want: "Bearer ***ABCD", + }, + { + name: "authorization match is case-insensitive", + headerName: "AUTHORIZATION", + value: "Bearer ghp_abcdef1234567890ABCD", + want: "Bearer ***ABCD", + }, + { + name: "cookie is fully redacted", + headerName: "Cookie", + value: "session=abc; csrf=xyz", + want: "***", + }, + { + name: "set-cookie is fully redacted", + headerName: "Set-Cookie", + value: "session=abc; Path=/; HttpOnly", + want: "***", + }, + { + name: "proxy-authorization is fully redacted", + headerName: "Proxy-Authorization", + value: "Basic dXNlcjpwYXNz", + want: "***", + }, + { + name: "non-sensitive header is returned unchanged", + headerName: "Content-Type", + value: "application/json", + want: "application/json", + }, + { + name: "x-oauth-scopes is intentionally not redacted", + headerName: "X-OAuth-Scopes", + value: "repo, read:org", + want: "repo, read:org", + }, + } { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.want, redactSensitiveHeader(tc.headerName, tc.value)) + }) + } +}