Stop leaking access token and other secrets in logs

.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
-

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)))
 	}
 }
 

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))
+}

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}

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) {

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":

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,