fix: github token getter

[wuhuizuo]

This pull request includes several changes to the change-insight/lib/github/common.go file to enhance security and improve error handling. The most important changes include the addition of environment variable support for GitHub tokens and improved error handling for API calls.

Enhancements to security:

• change-insight/lib/github/common.go: Replaced the hardcoded GitHub token with a function that retrieves the token from the environment variable GITHUB_TOKEN.

Improvements to error handling:

• change-insight/lib/github/common.go: Added error handling for unexpected status codes in the apiCall function, logging the status code and returning an error if it is not http.StatusOK.

Currently, the sub app that changed in this PR is planed to be deprecated.

[ti-chi-bot]

I have already done a preliminary review for you, and I hope to help you do a better job.

Based on the pull request title and diff, it seems that the key change in this pull request is to fix the GitHub token getter in the common.go file. Specifically, the hardcoded token value has been replaced with a call to os.Getenv("GITHUB_TOKEN") to dynamically retrieve the token from the environment.

One potential problem with this change is that it assumes that the GITHUB_TOKEN environment variable is always set and contains a valid token. If this is not the case, the API calls made by this code may fail.

To address this issue, you could consider adding some additional error handling to the getGitHubToken function to check for the presence of the environment variable and return an error if it is not set. Additionally, you could add some checks in the apiCall function to ensure that the retrieved token is valid and that the API call was successful.

Overall, this seems like a reasonable change to the code and should improve its reliability and security.

[ti-chi-bot]

I have already done a preliminary review for you, and I hope to help you do a better job.

Based on the pull request's title, it seems that the change only involves fixing the GitHub token getter. However, the PR description is empty, so it's hard to tell what prompted this change.

The diff shows that the previous hard-coded GitHub token was removed, and a new function getGitHubToken() was added to retrieve the token from an environment variable GITHUB_TOKEN. This is a good practice because exposing secrets like a GitHub token in code is a security risk.

However, there are some potential problems with this pull request:

• The getGitHubToken() function is missing an opening curly brace, which will cause a compilation error.
• The fmt package is not imported, so the fmt.Errorf() call will also cause a compilation error.
• There is no error handling when retrieving the token from the environment variable. If the variable is not set, the function will return an empty string and the Authorization header will be set to token, which will result in a 401 Unauthorized response from GitHub.

To fix these problems, the code should be updated as follows:

diff --git a/change-insight/lib/github/common.go b/change-insight/lib/github/common.go
index 4b60eb8..e4d31e9 100644
--- a/change-insight/lib/github/common.go
+++ b/change-insight/lib/github/common.go
@@ -3,12 +3,14 @@ package github
 import (
 	"log"
 	"net/http"
+	"os"
+	"fmt"
 )
 
 type Repo struct {
 	Org  string // eg: pingcap
 	Repo string // eg: tidb
 }
 
-func getGitHubToken() string
+func getGitHubToken() string {
 	return os.Getenv("GITHUB_TOKEN")
 }
 
@@ -25,17 +27,32 @@ type labelInfo struct {
 	Description string `json:"description"`
 }
 
-var token string = "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"
 var apiURL string = "https://api.github.com/repos/"
 
 func apiCall(apiUrl string) (*http.Response, error) {
 	req, err := http.NewRequest("GET", apiUrl, nil)
 	if err != nil {
-		log.Printf("request init error: %s\n", err.Error())
+		log.Printf("request init error: %s\n", err)
 		return nil, err
 	}
 
 	req.Header.Set("Accept", "application/vnd.github+json")
-	req.Header.Set("Authorization", "token "+token)
+	token := getGitHubToken()
+	if token == "" {
+		return nil, fmt.Errorf("GITHUB_TOKEN environment variable not set")
+	}
+	req.Header.Set("Authorization", "token " + token)
+	
+	resp, err := http.DefaultClient.Do(req)
+	if err != nil {
+		log.Printf("request error: %s\n", err)
+		return nil, err
+	}
+	
+	if resp.StatusCode != http.StatusOK {
+		log.Printf("unexpected status code: %d\n", resp.StatusCode)
+		return resp, fmt.Errorf("unexpected status code: %d", resp.StatusCode)
+	}
+	
+	return resp, nil
 }

In summary, the pull request is a good change that removes a hard-coded secret from the code. However, it needs some fixes to compile and handle errors correctly.

[ti-chi-bot]

I have already done a preliminary review for you, and I hope to help you do a better job.

Based on the pull request title and diff, it seems that the change is related to the GitHub API token. The getGitHubToken() function has been added to retrieve the token from the environment variable GITHUB_TOKEN. Additionally, the token variable has been removed from the code, and the function now uses the new getGitHubToken() function to set the Authorization header for the HTTP request instead of the static token value.

As for potential problems, there are a couple of things to note:

• The function getGitHubToken() assumes that the environment variable GITHUB_TOKEN is always set and returns an empty string if it is not. This could cause issues if the token is not set or not set correctly.
• There is no error handling for the case where the Authorization header cannot be set, which could result in unexpected behavior.

To fix these issues, I would suggest modifying the getGitHubToken() function to return an error if the GITHUB_TOKEN environment variable is not set. Additionally, I would add error handling to the apiCall() function to handle cases where the Authorization header cannot be set.

Here is an example of how the getGitHubToken() function could be modified:

func getGitHubToken() (string, error) {
	token := os.Getenv("GITHUB_TOKEN")
	if token == "" {
		return "", fmt.Errorf("GITHUB_TOKEN environment variable is not set")
	}
	return token, nil
}

And here is an example of how the apiCall() function could be modified to handle errors:

func apiCall(apiUrl string) (*http.Response, error) {
	token, err := getGitHubToken()
	if err != nil {
		log.Printf("error getting GitHub token: %s\n", err.Error())
		return nil, err
	}

	req, err := http.NewRequest("GET", apiUrl, nil)
	if err != nil {
		log.Printf("request init error: %s\n", err.Error())
		return nil, err
	}

	req.Header.Set("Accept", "application/vnd.github+json")
	req.Header.Set("Authorization", "token "+token)

	resp, err := http.DefaultClient.Do(req)
	if err != nil {
		log.Printf("request error: %s\n", err.Error())
		return nil, err
	}

	if resp.StatusCode != http.StatusOK {
		log.Printf("unexpected status code: %d\n", resp.StatusCode)
		return resp, fmt.Errorf("unexpected status code: %d", resp.StatusCode)
	}

	return resp, nil
}

[wuhuizuo]

/approve

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wuhuizuo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• change-insight/OWNERS [wuhuizuo]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment