Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/kosli/assertPRGithub.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
},
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/kosli/assertPRGithub_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"}},
Expand Down
2 changes: 1 addition & 1 deletion cmd/kosli/attestPRGithub.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
},
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/kosli/attestPRGithub_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"}},
Expand Down
126 changes: 117 additions & 9 deletions internal/github/github.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package github

import (
"bytes"
"context"
"fmt"
"io"
"net/http"
"net/url"
"os"
"strings"
"time"

Expand All @@ -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)
Expand All @@ -34,14 +38,15 @@ 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,
Org: org,
// 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,
}
}

Expand All @@ -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}
Comment thread
ToreMerkely marked this conversation as resolved.
Comment thread
ToreMerkely marked this conversation as resolved.
}
if baseURL != "" {
client, err := gh.NewEnterpriseClient(baseURL, baseURL, tc)
if err != nil {
Expand All @@ -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] <failed to read request body: %v>\n", err)
} else {
if len(bodyBytes) > 0 {
d.logf("[debug-github] body: %s\n", string(bodyBytes))
Comment thread
ToreMerkely marked this conversation as resolved.
}
Comment thread
ToreMerkely marked this conversation as resolved.
req.Body = io.NopCloser(bytes.NewReader(bodyBytes))
}
Comment thread
ToreMerkely marked this conversation as resolved.
}

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] <failed to read response body: %v>\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 "***"
Comment thread
ToreMerkely marked this conversation as resolved.
Comment thread
ToreMerkely marked this conversation as resolved.
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 {
Comment thread
ToreMerkely marked this conversation as resolved.
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"
Expand All @@ -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.
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down
2 changes: 2 additions & 0 deletions internal/github/github_contract_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -212,6 +213,7 @@ func TestGitHubContract_RealGitHub_PRByNumber(t *testing.T) {
"",
"kosli-dev",
"cli",
false,
)

runPRByNumberContractTests(t, config, testHelpers.GithubPRNumber())
Expand Down
101 changes: 100 additions & 1 deletion internal/github/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})
Expand Down Expand Up @@ -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))
})
}
}
Loading