From 405f356071d04769a9fc42ef4038aac1cdb9d086 Mon Sep 17 00:00:00 2001 From: Cody Rose Date: Mon, 11 Dec 2023 18:04:55 -0500 Subject: [PATCH] Use bad json in slackwebhooks (#2193) * add rotation guides to SlackWebhook tests * begin cleaning up tests * have slack webhook detector use malformed json * update test secrets --------- Co-authored-by: Ahrav Dutta --- pkg/detectors/slackwebhook/slackwebhook.go | 20 ++- .../slackwebhook/slackwebhook_test.go | 133 ++++++++---------- 2 files changed, 69 insertions(+), 84 deletions(-) diff --git a/pkg/detectors/slackwebhook/slackwebhook.go b/pkg/detectors/slackwebhook/slackwebhook.go index a6e1c7e64..c895c46d9 100644 --- a/pkg/detectors/slackwebhook/slackwebhook.go +++ b/pkg/detectors/slackwebhook/slackwebhook.go @@ -60,7 +60,9 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result client = defaultClient } - payload := strings.NewReader(`{"text": ""}`) + // We don't want to actually send anything to webhooks we find. To verify them without spamming them, we + // send an intentionally malformed message and look for a particular expected error message. + payload := strings.NewReader(`intentionally malformed JSON from Trufflehog scan`) req, err := http.NewRequestWithContext(ctx, "POST", resMatch, payload) if err != nil { continue @@ -76,12 +78,18 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result defer res.Body.Close() - if res.StatusCode >= 200 && res.StatusCode < 300 || (res.StatusCode == 400 && (bytes.Equal(bodyBytes, []byte("no_text")) || bytes.Equal(bodyBytes, []byte("missing_text")))) { + switch { + case res.StatusCode >= http.StatusOK && res.StatusCode < http.StatusMultipleChoices: + // Hopefully this never happens - it means we actually sent something to a channel somewhere. But + // we at least know the secret is verified. s1.Verified = true - } else if res.StatusCode == 401 || res.StatusCode == 403 { - // The secret is determinately not verified (nothing to do) - } else { - err = fmt.Errorf("unexpected HTTP response status %d", res.StatusCode) + case res.StatusCode == http.StatusBadRequest && bytes.Equal(bodyBytes, []byte("invalid_payload")): + s1.Verified = true + case res.StatusCode == http.StatusNotFound: + // Not a real webhook or the owning app's OAuth token has been revoked or the app has been deleted + // You might want to handle this case or log it. + default: + err = fmt.Errorf("unexpected HTTP response status %d: %s", res.StatusCode, bodyBytes) s1.SetVerificationError(err, resMatch) } } else { diff --git a/pkg/detectors/slackwebhook/slackwebhook_test.go b/pkg/detectors/slackwebhook/slackwebhook_test.go index 79511430d..706b5b62e 100644 --- a/pkg/detectors/slackwebhook/slackwebhook_test.go +++ b/pkg/detectors/slackwebhook/slackwebhook_test.go @@ -20,12 +20,14 @@ import ( func TestSlackWebhook_FromChunk(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) defer cancel() - testSecrets, err := common.GetSecret(ctx, "trufflehog-testing", "detectors3") + testSecrets, err := common.GetSecret(ctx, "trufflehog-testing", "detectors5") if err != nil { t.Fatalf("could not get test secrets from GCP: %s", err) } secret := testSecrets.MustGetField("SLACKWEBHOOK_TOKEN") inactiveSecret := testSecrets.MustGetField("SLACKWEBHOOK_INACTIVE") + deletedWebhook := testSecrets.MustGetField("SLACKWEBHOOK_DELETED") + deletedUserActiveWebhook := testSecrets.MustGetField("SLACKWEBHOOK_DELETED_USER_ACTIVE_WEBHOOK") type args struct { ctx context.Context @@ -41,7 +43,7 @@ func TestSlackWebhook_FromChunk(t *testing.T) { wantVerificationErr bool }{ { - name: "found, verified", + name: "active webhook", s: Scanner{}, args: args{ ctx: context.Background(), @@ -51,6 +53,7 @@ func TestSlackWebhook_FromChunk(t *testing.T) { want: []detectors.Result{ { DetectorType: detectorspb.DetectorType_SlackWebhook, + ExtraData: map[string]string{"rotation_guide": "https://howtorotate.com/docs/tutorials/slack-webhook/"}, Verified: true, }, }, @@ -58,50 +61,35 @@ func TestSlackWebhook_FromChunk(t *testing.T) { wantVerificationErr: false, }, { - name: "found, verified, 400 no_text", - s: Scanner{client: common.ConstantResponseHttpClient(400, "no_text")}, - args: args{ - ctx: context.Background(), - data: []byte(fmt.Sprintf("You can find a slackwebhook secret %s within", secret)), - verify: true, - }, - want: []detectors.Result{ - { - DetectorType: detectorspb.DetectorType_SlackWebhook, - Verified: true, - }, - }, - wantErr: false, - wantVerificationErr: false, - }, - { - name: "found, verified, 400 missing_text", - s: Scanner{client: common.ConstantResponseHttpClient(400, "missing_text")}, - args: args{ - ctx: context.Background(), - data: []byte(fmt.Sprintf("You can find a slackwebhook secret %s within", secret)), - verify: true, - }, - want: []detectors.Result{ - { - DetectorType: detectorspb.DetectorType_SlackWebhook, - Verified: true, - }, - }, - wantErr: false, - wantVerificationErr: false, - }, - { - name: "found, unverified", + name: "active webhook created by a deactivated user", s: Scanner{}, args: args{ ctx: context.Background(), - data: []byte(fmt.Sprintf("You can find a slackwebhook secret %s within but not valid", inactiveSecret)), // the secret would satisfy the regex but not pass validation + data: []byte(fmt.Sprintf("You can find a slackwebhook secret %s within", deletedUserActiveWebhook)), verify: true, }, want: []detectors.Result{ { DetectorType: detectorspb.DetectorType_SlackWebhook, + ExtraData: map[string]string{"rotation_guide": "https://howtorotate.com/docs/tutorials/slack-webhook/"}, + Verified: true, + }, + }, + wantErr: false, + wantVerificationErr: false, + }, + { + name: "deleted webhook", + s: Scanner{}, + args: args{ + ctx: context.Background(), + data: []byte(fmt.Sprintf("You can find a slackwebhook secret %s within", deletedWebhook)), + verify: true, + }, + want: []detectors.Result{ + { + DetectorType: detectorspb.DetectorType_SlackWebhook, + ExtraData: map[string]string{"rotation_guide": "https://howtorotate.com/docs/tutorials/slack-webhook/"}, Verified: false, }, }, @@ -109,19 +97,43 @@ func TestSlackWebhook_FromChunk(t *testing.T) { wantVerificationErr: false, }, { - name: "not found", + name: "webhook from app with revoked token", s: Scanner{}, args: args{ ctx: context.Background(), - data: []byte("You cannot find the secret within"), + data: []byte(fmt.Sprintf("You can find a slackwebhook secret %s within", inactiveSecret)), verify: true, }, - want: nil, + want: []detectors.Result{ + { + DetectorType: detectorspb.DetectorType_SlackWebhook, + ExtraData: map[string]string{"rotation_guide": "https://howtorotate.com/docs/tutorials/slack-webhook/"}, + Verified: false, + }, + }, wantErr: false, wantVerificationErr: false, }, { - name: "found, would be verified if not for timeout", + name: "unexpected webhook response", + s: Scanner{client: common.ConstantResponseHttpClient(500, "oh no")}, + args: args{ + ctx: context.Background(), + data: []byte(fmt.Sprintf("You can find a slackwebhook secret %s within", secret)), + verify: true, + }, + want: []detectors.Result{ + { + DetectorType: detectorspb.DetectorType_SlackWebhook, + ExtraData: map[string]string{"rotation_guide": "https://howtorotate.com/docs/tutorials/slack-webhook/"}, + Verified: false, + }, + }, + wantErr: false, + wantVerificationErr: true, + }, + { + name: "timeout", s: Scanner{client: common.SaneHttpClientTimeOut(1 * time.Microsecond)}, args: args{ ctx: context.Background(), @@ -131,40 +143,7 @@ func TestSlackWebhook_FromChunk(t *testing.T) { want: []detectors.Result{ { DetectorType: detectorspb.DetectorType_SlackWebhook, - Verified: false, - }, - }, - wantErr: false, - wantVerificationErr: true, - }, - { - name: "found, verified but unexpected api surface", - s: Scanner{client: common.ConstantResponseHttpClient(404, "")}, - args: args{ - ctx: context.Background(), - data: []byte(fmt.Sprintf("You can find a slackwebhook secret %s within", secret)), - verify: true, - }, - want: []detectors.Result{ - { - DetectorType: detectorspb.DetectorType_SlackWebhook, - Verified: false, - }, - }, - wantErr: false, - wantVerificationErr: true, - }, - { - name: "found, account disabled", - s: Scanner{client: common.ConstantResponseHttpClient(400, disabledAccountResponse)}, - args: args{ - ctx: context.Background(), - data: []byte(fmt.Sprintf("You can find a slackwebhook secret %s within", secret)), - verify: true, - }, - want: []detectors.Result{ - { - DetectorType: detectorspb.DetectorType_SlackWebhook, + ExtraData: map[string]string{"rotation_guide": "https://howtorotate.com/docs/tutorials/slack-webhook/"}, Verified: false, }, }, @@ -195,8 +174,6 @@ func TestSlackWebhook_FromChunk(t *testing.T) { } } -const disabledAccountResponse = `missing_text_or_fallback_or_attachments` - func BenchmarkFromData(benchmark *testing.B) { ctx := context.Background() s := Scanner{}