From 1367ed79cead1d01fb577adcf8700b240005d2cf Mon Sep 17 00:00:00 2001 From: Abdul Basit Date: Mon, 14 Apr 2025 18:29:58 +0500 Subject: [PATCH 1/5] handle unexpected status codes and some refactoring --- .../clickuppersonaltoken.go | 47 ++++++++++++------- 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/pkg/detectors/clickuppersonaltoken/clickuppersonaltoken.go b/pkg/detectors/clickuppersonaltoken/clickuppersonaltoken.go index 4502e09ac43a..eef03dfe1290 100644 --- a/pkg/detectors/clickuppersonaltoken/clickuppersonaltoken.go +++ b/pkg/detectors/clickuppersonaltoken/clickuppersonaltoken.go @@ -2,6 +2,7 @@ package clickuppersonaltoken import ( "context" + "fmt" "net/http" "strings" @@ -21,7 +22,7 @@ var ( client = common.SaneHttpClient() // Make sure that your group is surrounded in boundary characters such as below to reduce false positives. - keyPat = regexp.MustCompile(`\b(pk_[0-9]{7,8}_[0-9A-Z]{32})\b`) + keyPat = regexp.MustCompile(`\b(pk_[0-9]{7,9}_[0-9A-Z]{32})\b`) ) // Keywords are used for efficiently pre-filtering chunks. @@ -34,10 +35,12 @@ func (s Scanner) Keywords() []string { func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (results []detectors.Result, err error) { dataStr := string(data) - matches := keyPat.FindAllStringSubmatch(dataStr, -1) + uniqueMatches := make(map[string]struct{}) + for _, match := range keyPat.FindAllStringSubmatch(dataStr, -1) { + uniqueMatches[strings.TrimSpace(match[1])] = struct{}{} + } - for _, match := range matches { - resMatch := strings.TrimSpace(match[1]) + for resMatch, _ := range uniqueMatches { s1 := detectors.Result{ DetectorType: detectorspb.DetectorType_ClickupPersonalToken, @@ -45,18 +48,9 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result } if verify { - req, err := http.NewRequestWithContext(ctx, "GET", "https://api.clickup.com/api/v2/user", nil) - if err != nil { - continue - } - req.Header.Add("Authorization", resMatch) - res, err := client.Do(req) - if err == nil { - defer res.Body.Close() - if res.StatusCode >= 200 && res.StatusCode < 300 { - s1.Verified = true - } - } + isVerified, err := verifyToken(ctx, resMatch) + s1.Verified = isVerified + s1.SetVerificationError(err, resMatch) } results = append(results, s1) @@ -72,3 +66,24 @@ func (s Scanner) Type() detectorspb.DetectorType { func (s Scanner) Description() string { return "ClickUp is a project management tool. Personal tokens can be used to access and modify data within ClickUp on behalf of a user." } + +func verifyToken(ctx context.Context, token string) (bool, error) { + req, err := http.NewRequestWithContext(ctx, "GET", "https://api.clickup.com/api/v2/user", nil) + if err != nil { + return false, err + } + req.Header.Add("Authorization", token) + res, err := client.Do(req) + if err != nil { + return false, err + } + defer res.Body.Close() + switch res.StatusCode { + case http.StatusOK: + return true, nil + case http.StatusUnauthorized: + return false, nil + default: + return false, fmt.Errorf("unexpected status code %d", res.StatusCode) + } +} From 2b2ccad54962fd92bf64474ec598b4c27753ef7b Mon Sep 17 00:00:00 2001 From: Abdul Basit Date: Mon, 14 Apr 2025 18:51:31 +0500 Subject: [PATCH 2/5] reusable http client some code refactoring. --- .../clickuppersonaltoken.go | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/pkg/detectors/clickuppersonaltoken/clickuppersonaltoken.go b/pkg/detectors/clickuppersonaltoken/clickuppersonaltoken.go index eef03dfe1290..88ea46a04f11 100644 --- a/pkg/detectors/clickuppersonaltoken/clickuppersonaltoken.go +++ b/pkg/detectors/clickuppersonaltoken/clickuppersonaltoken.go @@ -3,6 +3,7 @@ package clickuppersonaltoken import ( "context" "fmt" + "io" "net/http" "strings" @@ -13,13 +14,15 @@ import ( "github.com/trufflesecurity/trufflehog/v3/pkg/pb/detectorspb" ) -type Scanner struct{} +type Scanner struct { + client *http.Client +} // Ensure the Scanner satisfies the interface at compile time. var _ detectors.Detector = (*Scanner)(nil) var ( - client = common.SaneHttpClient() + defaultClient = common.SaneHttpClient() // Make sure that your group is surrounded in boundary characters such as below to reduce false positives. keyPat = regexp.MustCompile(`\b(pk_[0-9]{7,9}_[0-9A-Z]{32})\b`) @@ -48,7 +51,11 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result } if verify { - isVerified, err := verifyToken(ctx, resMatch) + client := s.client + if client == nil { + client = defaultClient + } + isVerified, err := verifyToken(ctx, client, resMatch) s1.Verified = isVerified s1.SetVerificationError(err, resMatch) } @@ -67,8 +74,8 @@ func (s Scanner) Description() string { return "ClickUp is a project management tool. Personal tokens can be used to access and modify data within ClickUp on behalf of a user." } -func verifyToken(ctx context.Context, token string) (bool, error) { - req, err := http.NewRequestWithContext(ctx, "GET", "https://api.clickup.com/api/v2/user", nil) +func verifyToken(ctx context.Context, client *http.Client, token string) (bool, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, "https://api.clickup.com/api/v2/user", nil) if err != nil { return false, err } @@ -77,7 +84,11 @@ func verifyToken(ctx context.Context, token string) (bool, error) { if err != nil { return false, err } - defer res.Body.Close() + defer func() { + _, _ = io.Copy(io.Discard, res.Body) + _ = res.Body.Close() + }() + switch res.StatusCode { case http.StatusOK: return true, nil From 4933a306f4c1d04ac01109077cb97dea08d723b6 Mon Sep 17 00:00:00 2001 From: Abdul Basit Date: Tue, 15 Apr 2025 12:05:16 +0500 Subject: [PATCH 3/5] linter fixes. --- pkg/detectors/clickuppersonaltoken/clickuppersonaltoken.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/detectors/clickuppersonaltoken/clickuppersonaltoken.go b/pkg/detectors/clickuppersonaltoken/clickuppersonaltoken.go index 88ea46a04f11..07b0d14520e4 100644 --- a/pkg/detectors/clickuppersonaltoken/clickuppersonaltoken.go +++ b/pkg/detectors/clickuppersonaltoken/clickuppersonaltoken.go @@ -43,8 +43,7 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result uniqueMatches[strings.TrimSpace(match[1])] = struct{}{} } - for resMatch, _ := range uniqueMatches { - + for resMatch := range uniqueMatches { s1 := detectors.Result{ DetectorType: detectorspb.DetectorType_ClickupPersonalToken, Raw: []byte(resMatch), From c9cc757c1e16924ccc3cf6c1264e748f01ec6dda Mon Sep 17 00:00:00 2001 From: Abdul Basit Date: Thu, 17 Apr 2025 17:15:31 +0500 Subject: [PATCH 4/5] added test case for indeterminate verification errors. --- .../clickuppersonaltoken_integration_test.go | 54 ++++++++++++++----- 1 file changed, 41 insertions(+), 13 deletions(-) diff --git a/pkg/detectors/clickuppersonaltoken/clickuppersonaltoken_integration_test.go b/pkg/detectors/clickuppersonaltoken/clickuppersonaltoken_integration_test.go index 8c8bd60f7292..ae40103b6b6d 100644 --- a/pkg/detectors/clickuppersonaltoken/clickuppersonaltoken_integration_test.go +++ b/pkg/detectors/clickuppersonaltoken/clickuppersonaltoken_integration_test.go @@ -9,7 +9,8 @@ import ( "testing" "time" - "github.com/kylelemons/godebug/pretty" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/trufflesecurity/trufflehog/v3/pkg/detectors" "github.com/trufflesecurity/trufflehog/v3/pkg/common" @@ -32,11 +33,12 @@ func TestClickupPersonalToken_FromChunk(t *testing.T) { verify bool } tests := []struct { - name string - s Scanner - args args - want []detectors.Result - wantErr bool + name string + s Scanner + args args + want []detectors.Result + wantErr bool + wantVerificationErr bool }{ { name: "found, verified", @@ -52,7 +54,8 @@ func TestClickupPersonalToken_FromChunk(t *testing.T) { Verified: true, }, }, - wantErr: false, + wantErr: false, + wantVerificationErr: false, }, { name: "found, unverified", @@ -68,7 +71,8 @@ func TestClickupPersonalToken_FromChunk(t *testing.T) { Verified: false, }, }, - wantErr: false, + wantErr: false, + wantVerificationErr: false, }, { name: "not found", @@ -78,14 +82,32 @@ func TestClickupPersonalToken_FromChunk(t *testing.T) { data: []byte("You cannot find the secret within"), verify: true, }, - want: nil, - wantErr: false, + want: nil, + wantErr: false, + wantVerificationErr: false, + }, + { + name: "found verifiable secret, verification failed due to unexpected API response", + s: Scanner{client: common.ConstantResponseHttpClient(404, "")}, + args: args{ + ctx: context.Background(), + data: []byte(fmt.Sprintf("You can find a clickuppersonaltoken secret %s within but not valid", inactiveSecret)), // the secret would satisfy the regex but not pass validation + verify: true, + }, + want: []detectors.Result{ + { + DetectorType: detectorspb.DetectorType_ClickupPersonalToken, + Verified: false, + }, + }, + wantErr: false, + wantVerificationErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s := Scanner{} - got, err := s.FromData(tt.args.ctx, tt.args.verify, tt.args.data) + // s := Scanner{} + got, err := tt.s.FromData(tt.args.ctx, tt.args.verify, tt.args.data) if (err != nil) != tt.wantErr { t.Errorf("ClickupPersonalToken.FromData() error = %v, wantErr %v", err, tt.wantErr) return @@ -94,9 +116,15 @@ func TestClickupPersonalToken_FromChunk(t *testing.T) { if len(got[i].Raw) == 0 { t.Fatalf("no raw secret present: \n %+v", got[i]) } + + if (got[i].VerificationError() != nil) != tt.wantVerificationErr { + t.Fatalf("wantVerificationError = %v, verification error = %v", tt.wantVerificationErr, got[i].VerificationError()) + } got[i].Raw = nil } - if diff := pretty.Compare(got, tt.want); diff != "" { + + ignoreOpts := cmpopts.IgnoreFields(detectors.Result{}, "Raw", "verificationError") + if diff := cmp.Diff(got, tt.want, ignoreOpts); diff != "" { t.Errorf("ClickupPersonalToken.FromData() %s diff: (-got +want)\n%s", tt.name, diff) } }) From be5fb215da3ba8f8a51ddcace01ce3892d8b8927 Mon Sep 17 00:00:00 2001 From: Abdul Basit Date: Thu, 17 Apr 2025 17:55:52 +0500 Subject: [PATCH 5/5] added regex prefix to increase confidence. removed commented code from integration test. --- pkg/detectors/clickuppersonaltoken/clickuppersonaltoken.go | 2 +- .../clickuppersonaltoken_integration_test.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/detectors/clickuppersonaltoken/clickuppersonaltoken.go b/pkg/detectors/clickuppersonaltoken/clickuppersonaltoken.go index 07b0d14520e4..1f5907bf043c 100644 --- a/pkg/detectors/clickuppersonaltoken/clickuppersonaltoken.go +++ b/pkg/detectors/clickuppersonaltoken/clickuppersonaltoken.go @@ -25,7 +25,7 @@ var ( defaultClient = common.SaneHttpClient() // Make sure that your group is surrounded in boundary characters such as below to reduce false positives. - keyPat = regexp.MustCompile(`\b(pk_[0-9]{7,9}_[0-9A-Z]{32})\b`) + keyPat = regexp.MustCompile(detectors.PrefixRegex([]string{"clickup"}) + `\b(pk_[0-9]{7,9}_[0-9A-Z]{32})\b`) ) // Keywords are used for efficiently pre-filtering chunks. diff --git a/pkg/detectors/clickuppersonaltoken/clickuppersonaltoken_integration_test.go b/pkg/detectors/clickuppersonaltoken/clickuppersonaltoken_integration_test.go index ae40103b6b6d..2be96fda4483 100644 --- a/pkg/detectors/clickuppersonaltoken/clickuppersonaltoken_integration_test.go +++ b/pkg/detectors/clickuppersonaltoken/clickuppersonaltoken_integration_test.go @@ -106,7 +106,6 @@ func TestClickupPersonalToken_FromChunk(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // s := Scanner{} got, err := tt.s.FromData(tt.args.ctx, tt.args.verify, tt.args.data) if (err != nil) != tt.wantErr { t.Errorf("ClickupPersonalToken.FromData() error = %v, wantErr %v", err, tt.wantErr)