Skip to content

Commit

Permalink
Use bad json in slackwebhooks (#2193)
Browse files Browse the repository at this point in the history 
* 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 <ahrav.dutta@trufflesec.com>
  • Loading branch information
@rosecodym @ahrav
rosecodym and ahrav committed Dec 11, 2023
1 parent 61c7d52 commit 405f356
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 51 deletions.
20 changes: 14 additions & 6 deletions pkg/detectors/slackwebhook/slackwebhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
67 changes: 22 additions & 45 deletions pkg/detectors/slackwebhook/slackwebhook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(),
Expand All @@ -51,95 +53,70 @@ 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,
},
},
wantErr: false,
wantVerificationErr: false,
},
{
name: "found, verified, 400 no_text",
s: Scanner{client: common.ConstantResponseHttpClient(400, "no_text")},
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", secret)),
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: "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: "deleted webhook",
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", 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,
},
},
wantErr: false,
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"),
verify: true,
},
want: nil,
wantErr: false,
wantVerificationErr: false,
},
{
name: "found, would be verified if not for timeout",
s: Scanner{client: common.SaneHttpClientTimeOut(1 * time.Microsecond)},
args: args{
ctx: context.Background(),
data: []byte(fmt.Sprintf("You can find a slackwebhook secret %s within", secret)),
data: []byte(fmt.Sprintf("You can find a slackwebhook secret %s within", inactiveSecret)),
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,
wantVerificationErr: false,
},
{
name: "found, verified but unexpected api surface",
s: Scanner{client: common.ConstantResponseHttpClient(404, "")},
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)),
Expand All @@ -148,15 +125,16 @@ 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: false,
},
},
wantErr: false,
wantVerificationErr: true,
},
{
name: "found, account disabled",
s: Scanner{client: common.ConstantResponseHttpClient(400, disabledAccountResponse)},
name: "timeout",
s: Scanner{client: common.SaneHttpClientTimeOut(1 * time.Microsecond)},
args: args{
ctx: context.Background(),
data: []byte(fmt.Sprintf("You can find a slackwebhook secret %s within", secret)),
Expand All @@ -165,6 +143,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: false,
},
},
Expand Down Expand Up @@ -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{}
Expand Down

0 comments on commit 405f356

Please sign in to comment.