diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9e56bb94..43099747 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -2,20 +2,15 @@ on: [push, pull_request] name: Test jobs: test: - strategy: - matrix: - go-version: [1.14.x, 1.15.x] - os: [ubuntu-latest, macos-latest] - runs-on: ${{ matrix.os }} + runs-on: ubuntu-latest steps: + - name: Checkout code + uses: actions/checkout@v4 - name: Install Go - uses: actions/setup-go@v2 + uses: actions/setup-go@v5 with: - go-version: ${{ matrix.go-version }} - - name: Checkout code - uses: actions/checkout@v2 + go-version-file: go.mod - name: Install Task run: curl -sL https://taskfile.dev/install.sh | sh - name: Run CI Task run: ./bin/task ci - diff --git a/common.go b/common.go index 27a60eb1..18b80b79 100644 --- a/common.go +++ b/common.go @@ -6,13 +6,29 @@ Common functions import ( "encoding/json" "log" + "strings" ) +func RedactSecrets(s Source, str string) string { + for _, secret := range []string{ + s.AccessToken, + s.GitCryptKey, + s.OdAdvanced.VaultApproleSecretId, + s.OdAdvanced.DataDogApiKey, + s.OdAdvanced.DataDogAppKey, + } { + if secret != "" { + str = strings.ReplaceAll(str, secret, "REDACTED") + } + } + return str +} + // print the request json coming in to "in|out|check" func PrintDebugInput(s Source, obj any) { if s.OdAdvanced.Debug { jsonBytes, _ := json.Marshal(obj) - log.Printf("input jsonStr : %s\n", string(jsonBytes)) + log.Printf("input jsonStr : %s\n", RedactSecrets(s, string(jsonBytes))) log.Printf("Debig Tip1: run this docker image locally: docker run -it --entrypoint=/bin/sh opendoor/telia-oss-github-pr-resource:dev\n") log.Printf("Debug Tip2: save the above jsonStr to /tmp/request.json\n") log.Printf("Debug Tip3: cd /opt/resource && cat /tmp/request.json | <./in . |./out .|./check>\n") @@ -22,7 +38,7 @@ func PrintDebugInput(s Source, obj any) { func PrintDebugOutput(s Source, obj any) { if s.OdAdvanced.Debug { jsonBytes, _ := json.Marshal(obj) - log.Printf("output jsonStr : %s\n", string(jsonBytes)) + log.Printf("output jsonStr : %s\n", RedactSecrets(s, string(jsonBytes))) } } diff --git a/common_test.go b/common_test.go new file mode 100644 index 00000000..d67c041d --- /dev/null +++ b/common_test.go @@ -0,0 +1,40 @@ +package resource_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + resource "github.com/telia-oss/github-pr-resource" +) + +func TestRedactSecrets(t *testing.T) { + source := resource.Source{ + AccessToken: "ghp_abc123secret", + GitCryptKey: "cryptkey123", + OdAdvanced: resource.OdAdvanced{ + VaultApproleSecretId: "vault-secret-id", + DataDogApiKey: "dd-api-key", + DataDogAppKey: "dd-app-key", + }, + } + + input := `{"access_token":"ghp_abc123secret","git_crypt_key":"cryptkey123",` + + `"vault_approle_secret_id":"vault-secret-id","datadog_api_key":"dd-api-key",` + + `"datadog_app_key":"dd-app-key","repository":"halter/repo"}` + + ` fatal: unable to access 'https://x-oauth-basic:ghp_abc123secret@github.com/halter/repo'` + + output := resource.RedactSecrets(source, input) + + assert.NotContains(t, output, "ghp_abc123secret") + assert.NotContains(t, output, "cryptkey123") + assert.NotContains(t, output, "vault-secret-id") + assert.NotContains(t, output, "dd-api-key") + assert.NotContains(t, output, "dd-app-key") + assert.Contains(t, output, "halter/repo") + assert.Contains(t, output, "REDACTED") +} + +func TestRedactSecretsEmptySource(t *testing.T) { + input := "no secrets here" + assert.Equal(t, input, resource.RedactSecrets(resource.Source{}, input)) +} diff --git a/git.go b/git.go index 6b232ce0..ac296071 100644 --- a/git.go +++ b/git.go @@ -115,7 +115,7 @@ func (g *GitClient) Pull(uri, branch string, depth int, submodules bool, fetchTa } if err := g.command("git", "remote", "add", "origin", endpoint).Run(); err != nil { - return fmt.Errorf("setting 'origin' remote to '%s' failed: %w", endpoint, err) + return fmt.Errorf("setting 'origin' remote failed: %w", err) } args := []string{"pull", "origin", branch} diff --git a/github.go b/github.go index 4425ff9a..b7c0d601 100644 --- a/github.go +++ b/github.go @@ -10,7 +10,6 @@ import ( "net/http" "net/url" "os" - "os/exec" "path" "path/filepath" "strconv" @@ -78,7 +77,7 @@ func NewGithubClient(s *Source) (*GithubClient, error) { skipAccessToken = true } } - log.Printf("current AccessToken : %s_REDACTED\n", s.AccessToken[0:10]) + log.Printf("current AccessToken type : %s\n", tokenTypePrefix(s.AccessToken)) log.Printf("If the AccessToken starts with 'ghp_', it is a GitHub Personal token\n") log.Printf("If the AccessToken starts with 'ghs_', it is a GitHub App token - which has a higher rateLimit and is more secure\n") if skipAccessToken { @@ -87,7 +86,7 @@ func NewGithubClient(s *Source) (*GithubClient, error) { log.Printf("There is a problem with vault %s\n", err) return nil, err } - log.Printf("new AccessToken : %s_REDACTED\n", s.AccessToken[0:10]) + log.Printf("new AccessToken type : %s\n", tokenTypePrefix(s.AccessToken)) PrintCurrentRateLimit(*s) } else { coreRemaining, graphqlRemaining, _ := getRateLimit(*s) @@ -108,13 +107,13 @@ func NewGithubClient(s *Source) (*GithubClient, error) { log.Printf("setting AccessToken to first element in AccessTokenAdditional\n") // TODO altho we are passing a list of AccessTokenAdditional, we will only consider the first element as it is already sorted // by highest remaining ... in the future consider the rest of the list, altho this TODO is a low priority - log.Printf("old AccessToken : %s_REDACTED\n", s.AccessToken[0:10]) + log.Printf("old AccessToken type : %s\n", tokenTypePrefix(s.AccessToken)) s.AccessToken, err = getAccessTokenFromVault(*s) if err != nil { log.Printf("There is a problem with vault %s\n", err) return nil, err } - log.Printf("new AccessToken : %s_REDACTED\n", s.AccessToken[0:10]) + log.Printf("new AccessToken type : %s\n", tokenTypePrefix(s.AccessToken)) PrintCurrentRateLimit(*s) } else { log.Printf("there is sufficient minRemaining : %d rateLimit. No need to use AccessTokenAdditional\n", minRemaining) @@ -619,29 +618,45 @@ func parseRepository(s string) (string, string, error) { return parts[0], parts[1], nil } +func tokenTypePrefix(token string) string { + if i := strings.Index(token, "_"); i >= 0 { + return token[:i+1] + } + return "unknown" +} + /* returns rateLimit for core and rateLimit for graphql i.e. github ratelimit has sections for different resources */ func getRateLimit(source Source) (int, int, error) { - command := fmt.Sprintf("curl -s https://api.github.com/rate_limit -H \"Authorization: token %s\" > rateLimit.json", source.AccessToken) - _, err := exec.Command("sh", "-c", command).Output() + req, err := http.NewRequest("GET", "https://api.github.com/rate_limit", nil) if err != nil { - return 0, 0, fmt.Errorf("getRateLimit curl error : %w", err) + return 0, 0, fmt.Errorf("getRateLimit request error : %w", err) } - command = "cat rateLimit.json | jq -r '.resources.core.remaining'" - coreRemaining, err := exec.Command("sh", "-c", command).Output() + req.Header.Set("Authorization", "token "+source.AccessToken) + resp, err := http.DefaultClient.Do(req) if err != nil { - return 0, 0, fmt.Errorf("getRateLimit jq error : %w", err) - } - coreRemainingInt, _ := strconv.Atoi(strings.TrimSpace(string(coreRemaining))) - command = "cat rateLimit.json | jq -r '.resources.graphql.remaining'" - graphqlRemaining, err := exec.Command("sh", "-c", command).Output() - if err != nil { - return 0, 0, fmt.Errorf("getRateLimit jq error : %w", err) - } - graphqlRemainingInt, _ := strconv.Atoi(strings.TrimSpace(string(graphqlRemaining))) - return coreRemainingInt, graphqlRemainingInt, nil + return 0, 0, fmt.Errorf("getRateLimit http error : %w", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return 0, 0, fmt.Errorf("getRateLimit unexpected status : %s", resp.Status) + } + var rateLimit struct { + Resources struct { + Core struct { + Remaining int `json:"remaining"` + } `json:"core"` + Graphql struct { + Remaining int `json:"remaining"` + } `json:"graphql"` + } `json:"resources"` + } + if err := json.NewDecoder(resp.Body).Decode(&rateLimit); err != nil { + return 0, 0, fmt.Errorf("getRateLimit decode error : %w", err) + } + return rateLimit.Resources.Core.Remaining, rateLimit.Resources.Graphql.Remaining, nil } func PrintCurrentRateLimit(source Source) { diff --git a/in.go b/in.go index f5f5986d..e9548491 100644 --- a/in.go +++ b/in.go @@ -136,7 +136,7 @@ func Get(request GetRequest, github Github, git Git, outputDir string) (*GetResp log.Printf("Performing merge abort ...") command = fmt.Sprintf("cd %s && git merge --abort 2>&1", outputDir) out, _ = exec.Command("sh", "-c", command).CombinedOutput() - outTrim = strings.TrimSpace(string(out)) + outTrim = RedactSecrets(request.Source, strings.TrimSpace(string(out))) log.Printf("command : %s returned: %s\n", command, outTrim) request.Params.GitDepth *= 2 @@ -157,7 +157,7 @@ func Get(request GetRequest, github Github, git Git, outputDir string) (*GetResp // shouldn't this have been written in Bash? command = fmt.Sprintf("cd %s && git pull --depth %d origin %s", outputDir, request.Params.GitDepth, pull.BaseRefName) out, cmdErr = exec.Command("sh", "-c", command).CombinedOutput() - outTrim = strings.TrimSpace(string(out)) + outTrim = RedactSecrets(request.Source, strings.TrimSpace(string(out))) log.Printf("command : %s returned: %s\n", command, outTrim) if cmdErr != nil { log.Printf("commandErr : %s", cmdErr) @@ -167,7 +167,7 @@ func Get(request GetRequest, github Github, git Git, outputDir string) (*GetResp command = getFetchCommand(git, outputDir, pull.Number, request.Params.GitDepth, false) commandRedacted := getFetchCommand(git, outputDir, pull.Number, request.Params.GitDepth, true) out, cmdErr = exec.Command("sh", "-c", command).CombinedOutput() - outTrim = strings.TrimSpace(string(out)) + outTrim = RedactSecrets(request.Source, strings.TrimSpace(string(out))) if cmdErr != nil { log.Printf("commandErr : %s, command : %s\n", cmdErr, commandRedacted) } @@ -178,7 +178,7 @@ func Get(request GetRequest, github Github, git Git, outputDir string) (*GetResp log.Printf("END merge") if err != nil { log.Printf("merge failed after depth of %d (maxDepth : %d), returning err %s\n", request.Params.GitDepth, MaxGitDepth, err) - err = Wrap(err, outTrim+errBuffer.String()) + err = Wrap(err, RedactSecrets(request.Source, outTrim+errBuffer.String())) return nil, err } case "checkout": diff --git a/in_test.go b/in_test.go index 3ae45a44..bcc71124 100644 --- a/in_test.go +++ b/in_test.go @@ -35,10 +35,10 @@ func TestGet(t *testing.T) { AccessToken: "oauthtoken", }, version: resource.Version{ - PR: "pr1", - Commit: "commit1", - ChangedDate: time.Time{}, - State: githubv4.PullRequestStateOpen, + PR: "pr1", + Commit: "commit1", + ChangedDate: time.Time{}, + State: githubv4.PullRequestStateOpen, }, parameters: resource.GetParameters{GitDepth: resource.DefaultGitDepth}, pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen, []resource.StatusContext{}), @@ -53,10 +53,10 @@ func TestGet(t *testing.T) { GitCryptKey: "gitcryptkey", }, version: resource.Version{ - PR: "pr1", - Commit: "commit1", - ChangedDate: time.Time{}, - State: githubv4.PullRequestStateOpen, + PR: "pr1", + Commit: "commit1", + ChangedDate: time.Time{}, + State: githubv4.PullRequestStateOpen, }, parameters: resource.GetParameters{GitDepth: resource.DefaultGitDepth}, pullRequest: createTestPR(1, "master", false, false, 0, nil, false, githubv4.PullRequestStateOpen, []resource.StatusContext{}), @@ -70,10 +70,10 @@ func TestGet(t *testing.T) { AccessToken: "oauthtoken", }, version: resource.Version{ - PR: "pr1", - Commit: "commit1", - ChangedDate: time.Time{}, - State: githubv4.PullRequestStateOpen, + PR: "pr1", + Commit: "commit1", + ChangedDate: time.Time{}, + State: githubv4.PullRequestStateOpen, }, parameters: resource.GetParameters{ IntegrationTool: "rebase", @@ -90,10 +90,10 @@ func TestGet(t *testing.T) { AccessToken: "oauthtoken", }, version: resource.Version{ - PR: "pr1", - Commit: "commit1", - ChangedDate: time.Time{}, - State: githubv4.PullRequestStateOpen, + PR: "pr1", + Commit: "commit1", + ChangedDate: time.Time{}, + State: githubv4.PullRequestStateOpen, }, parameters: resource.GetParameters{ IntegrationTool: "checkout", @@ -110,10 +110,10 @@ func TestGet(t *testing.T) { AccessToken: "oauthtoken", }, version: resource.Version{ - PR: "pr1", - Commit: "commit1", - ChangedDate: time.Time{}, - State: githubv4.PullRequestStateOpen, + PR: "pr1", + Commit: "commit1", + ChangedDate: time.Time{}, + State: githubv4.PullRequestStateOpen, }, parameters: resource.GetParameters{ GitDepth: 2, @@ -129,10 +129,10 @@ func TestGet(t *testing.T) { AccessToken: "oauthtoken", }, version: resource.Version{ - PR: "pr1", - Commit: "commit1", - ChangedDate: time.Time{}, - State: githubv4.PullRequestStateOpen, + PR: "pr1", + Commit: "commit1", + ChangedDate: time.Time{}, + State: githubv4.PullRequestStateOpen, }, parameters: resource.GetParameters{ ListChangedFiles: true,