Skip to content

Detect Organization ID to pass into AnalysisInfo for Atlassian Detector #4480

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Oct 9, 2025
Merged

Detect Organization ID to pass into AnalysisInfo for Atlassian Detector #4480

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
02d946b
detect organization id and pass into analysis info
mustansir14 Sep 30, 2025
0db029b
Merge branch 'main' into INS-9-capture-organization-id-in-atlassian-a…
shahzadhaider1 Oct 2, 2025
ab267c6
Merge branch 'main' into INS-9-capture-organization-id-in-atlassian-a…
amanfcp Oct 3, 2025
1ad2d85
add non integration tests for analysis info, examples of credentials …
mustansir14 Oct 6, 2025
907cae0
Merge branch 'INS-9-capture-organization-id-in-atlassian-analyzer-to-…
mustansir14 Oct 6, 2025
f39a6d8
switch from tabular tests to separate tests
mustansir14 Oct 8, 2025
66cb725
move gock setup outside test functions
mustansir14 Oct 8, 2025
6d52a0d
Merge branch 'main' into INS-9-capture-organization-id-in-atlassian-a…
mustansir14 Oct 8, 2025
b28c45e
Merge branch 'main' into INS-9-capture-organization-id-in-atlassian-a…
kashifkhan0771 Oct 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 45 additions & 20 deletions pkg/detectors/atlassian/v2/atlassian.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@ var _ detectors.Versioner = (*Scanner)(nil)
var (
defaultClient = common.SaneHttpClient()
// Make sure that your group is surrounded in boundary characters such as below to reduce false positives.

// Example: ATCTT3xFfGN0GsZNgOGrQSHSnxiJVi00oHlRicyM0yMNuKCBfw6qOHVcCy4Hm89GnclGb_W-1qAkxqCn5XbuyoX54bNhpK5yFKGFR7ocV6FByvL_P9Sb3tFnbUg3T3I3S_RGCBLMSN7Nsa4GJv8JEJ6bzvDmX-oJ8AnrazMU-zZ5hb-u3t2ERew=366BFE3A
keyPat = regexp.MustCompile(`\b(ATCTT3xFfG[A-Za-z0-9+/=_-]+=[A-Za-z0-9]{8})\b`)
Copy link
Contributor

@martinlocklear martinlocklear Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we include an example of the type of strings we expect to match on here in the inline comments? I think it will make it easier for future folks to understand the intent of the regex here. On that note - unless I'm missing it - there's not already test coverage verifying these regexes. We should add that.

Copy link
Contributor Author

@mustansir14 mustansir14 Oct 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea. There are unit tests verifiying the regex of the key, I'll make changes to include the organization_id as well

// Example: 123e4567-e89b-12d3-a456-426614174000
organizationIdPat = regexp.MustCompile(detectors.PrefixRegex([]string{"org", "id"}) + `\b([0-9a-z]{8}-[0-9a-z]{4}-[0-9a-z]{4}-[0-9a-z]{4}-[0-9a-z]{12})\b`)
)

// Keywords are used for efficiently pre-filtering chunks.
Expand All @@ -58,31 +62,52 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result
uniqueMatches[match[1]] = struct{}{}
}

for match := range uniqueMatches {
s1 := detectors.Result{
DetectorType: detectorspb.DetectorType_Atlassian,
Raw: []byte(match),
ExtraData: map[string]string{
"rotation_guide": "https://howtorotate.com/docs/tutorials/atlassian/",
"version": fmt.Sprintf("%d", s.Version()),
},
}
uniqueOrgIdMatches := make(map[string]struct{})
for _, match := range organizationIdPat.FindAllStringSubmatch(dataStr, -1) {
uniqueOrgIdMatches[match[1]] = struct{}{}
}
if len(uniqueOrgIdMatches) == 0 {
// we only need an org ID to pass into AnalysisInfo
// if we don't find one, we can still verify the key
// we can add a dummy entry here just to make sure a result is returned
uniqueOrgIdMatches[""] = struct{}{}
}

if verify {
client := s.client
if client == nil {
client = defaultClient
for match := range uniqueMatches {
for orgId := range uniqueOrgIdMatches {
s1 := detectors.Result{
DetectorType: detectorspb.DetectorType_Atlassian,
Raw: []byte(match),
ExtraData: map[string]string{
"rotation_guide": "https://howtorotate.com/docs/tutorials/atlassian/",
"version": fmt.Sprintf("%d", s.Version()),
},
}

isVerified, orgResponse, verificationErr := verifyMatch(ctx, client, match)
s1.Verified = isVerified
if orgResponse != nil && len(orgResponse.Data) > 0 {
s1.ExtraData["Organization"] = orgResponse.Data[0].Attributes.Name
if verify {
client := s.client
if client == nil {
client = defaultClient
}

isVerified, orgResponse, verificationErr := verifyMatch(ctx, client, match)
s1.Verified = isVerified
if orgResponse != nil && len(orgResponse.Data) > 0 {
s1.ExtraData["Organization"] = orgResponse.Data[0].Attributes.Name
}
s1.SetVerificationError(verificationErr, match)
if s1.Verified {
s1.AnalysisInfo = map[string]string{
"key": match,
}
if orgId != "" {
s1.AnalysisInfo["organization_id"] = orgId
Copy link
Contributor

@martinlocklear martinlocklear Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd really love to see non-integration test coverage that verifies that this value is appropriately included (given the right pre-conditions).

Copy link
Contributor Author

@mustansir14 mustansir14 Oct 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will add.

}
Comment on lines +103 to +105
Copy link
Contributor

@shahzadhaider1 shahzadhaider1 Oct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible that some secrets may not actually belong to the orgId we’re sending here? From what I see, we’re always passing the orgId regardless of whether the secret is tied to that organization. How can we be sure that this orgId will work with this secret in the analyzer?
Does that approach make sense in this context?

Copy link
Contributor Author

@mustansir14 mustansir14 Oct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be possible that the secret might not belong to the orgId, and in that case it won't work with the secret in the analyzer. But that's only with Scoped API keys though, for Unscoped (Full Access) keys, we do not use the organization_id in the analyzer, irrespective of it being provided or not.

For scoped keys, passing an incorrect organization_id is the same as not passing an organization_id, as it will error out in both cases. So there's really no benefit in validating that the organization_id belongs to the secret.

}
}
s1.SetVerificationError(verificationErr, match)
}

results = append(results, s1)
results = append(results, s1)
}
}

return
Expand Down
2 changes: 1 addition & 1 deletion pkg/detectors/atlassian/v2/atlassian_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func TestAtlassian_FromChunk(t *testing.T) {
t.Fatalf("wantVerificationError = %v, verification error = %v", tt.wantVerificationErr, got[i].VerificationError())
}
}
ignoreOpts := cmpopts.IgnoreFields(detectors.Result{}, "Raw", "verificationError", "ExtraData")
ignoreOpts := cmpopts.IgnoreFields(detectors.Result{}, "Raw", "verificationError", "ExtraData", "primarySecret", "AnalysisInfo")
if diff := cmp.Diff(got, tt.want, ignoreOpts); diff != "" {
t.Errorf("Atlassian.FromData() %s diff: (-got +want)\n%s", tt.name, diff)
}
Expand Down
97 changes: 97 additions & 0 deletions pkg/detectors/atlassian/v2/atlassian_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@ package atlassian

import (
"context"
"net/http"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/require"
"github.com/trufflesecurity/trufflehog/v3/pkg/common"
"github.com/trufflesecurity/trufflehog/v3/pkg/detectors"
"github.com/trufflesecurity/trufflehog/v3/pkg/engine/ahocorasick"
"gopkg.in/h2non/gock.v1"
)

func TestAtlassian_Pattern(t *testing.T) {
Expand Down Expand Up @@ -79,3 +82,97 @@ func TestAtlassian_Pattern(t *testing.T) {
})
}
}

func TestAtlassian_AnalysisInfo(t *testing.T) {
client := common.SaneHttpClient()
d := Scanner{client: client}
ahoCorasickCore := ahocorasick.NewAhoCorasickCore([]detectors.Detector{d})
tests := []struct {
Copy link
Contributor

@martinlocklear martinlocklear Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the tests! I think there's some room to make these a little more maintainable though.

In general I think that tabular tests are problematic from a maintenance standpoint. (tabular tests are tests that use a test runner to iterate over a list of "test case" or "tests" data structures). Using anonymous functions as a setup step for these tests is also another code smell for me.

Let's go ahead and split out each of these test cases into separate first class tests and include documentation about specifically what the goal of each of the tests is. (This is hugely helpful for the reader to understand what's being tested).

Copy link
Contributor Author

@mustansir14 mustansir14 Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the tests to separate functions.

name string
input string
gockSetup func()
want []map[string]string
}{
{
name: "key and organization_id both present",
input: `
[INFO] Sending request to the atlassian API
[DEBUG] Using Key=ATCTT3xFfGN0GsZNgOGrQSHSnxiJVi00oHlRicyM0yMNuKCBfw6qOHVcCy4Hm89GnclGb_W-1qAkxqCn5XbuyoX54bNhpK5yFKGFR7ocV6FByvL_P9Sb3tFnbUg3T3I3S_RGCBLMSN7Nsa4GJv8JEJ6bzvDmX-oJ8AnrazMU-zZ5hb-u3t2ERew=366BFE3A
[DEBUG] Using Organization ID=123j4567-e89b-12d3-a456-426614174000
[INFO] Response received: 200 OK
`,
gockSetup: func() {
gock.New("https://api.atlassian.com").
Get("/admin/v1/orgs").
MatchHeader("Accept", "application/json").
MatchHeader("Authorization", "Bearer ATCTT3xFfGN0GsZNgOGrQSHSnxiJVi00oHlRicyM0yMNuKCBfw6qOHVcCy4Hm89GnclGb_W-1qAkxqCn5XbuyoX54bNhpK5yFKGFR7ocV6FByvL_P9Sb3tFnbUg3T3I3S_RGCBLMSN7Nsa4GJv8JEJ6bzvDmX-oJ8AnrazMU-zZ5hb-u3t2ERew=366BFE3A").
Reply(http.StatusOK).
JSON(map[string]any{
"Data": []map[string]any{},
})
},
want: []map[string]string{
{
"key": "ATCTT3xFfGN0GsZNgOGrQSHSnxiJVi00oHlRicyM0yMNuKCBfw6qOHVcCy4Hm89GnclGb_W-1qAkxqCn5XbuyoX54bNhpK5yFKGFR7ocV6FByvL_P9Sb3tFnbUg3T3I3S_RGCBLMSN7Nsa4GJv8JEJ6bzvDmX-oJ8AnrazMU-zZ5hb-u3t2ERew=366BFE3A",
"organization_id": "123j4567-e89b-12d3-a456-426614174000",
},
},
},
{
name: "key only",
input: `
[INFO] Sending request to the atlassian API
[DEBUG] Using Key=ATCTT3xFfGN0GsZNgOGrQSHSnxiJVi00oHlRicyM0yMNuKCBfw6qOHVcCy4Hm89GnclGb_W-1qAkxqCn5XbuyoX54bNhpK5yFKGFR7ocV6FByvL_P9Sb3tFnbUg3T3I3S_RGCBLMSN7Nsa4GJv8JEJ6bzvDmX-oJ8AnrazMU-zZ5hb-u3t2ERew=366BFE3A
[INFO] Response received: 200 OK
`,
gockSetup: func() {
gock.New("https://api.atlassian.com").
Get("/admin/v1/orgs").
MatchHeader("Accept", "application/json").
MatchHeader("Authorization", "Bearer ATCTT3xFfGN0GsZNgOGrQSHSnxiJVi00oHlRicyM0yMNuKCBfw6qOHVcCy4Hm89GnclGb_W-1qAkxqCn5XbuyoX54bNhpK5yFKGFR7ocV6FByvL_P9Sb3tFnbUg3T3I3S_RGCBLMSN7Nsa4GJv8JEJ6bzvDmX-oJ8AnrazMU-zZ5hb-u3t2ERew=366BFE3A").
Reply(http.StatusOK).
JSON(map[string]any{
"Data": []map[string]any{},
})
},
want: []map[string]string{
{
"key": "ATCTT3xFfGN0GsZNgOGrQSHSnxiJVi00oHlRicyM0yMNuKCBfw6qOHVcCy4Hm89GnclGb_W-1qAkxqCn5XbuyoX54bNhpK5yFKGFR7ocV6FByvL_P9Sb3tFnbUg3T3I3S_RGCBLMSN7Nsa4GJv8JEJ6bzvDmX-oJ8AnrazMU-zZ5hb-u3t2ERew=366BFE3A",
},
},
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
defer gock.Off()
defer gock.RestoreClient(client)
gock.InterceptClient(client)
test.gockSetup()

matchedDetectors := ahoCorasickCore.FindDetectorMatches([]byte(test.input))
if len(matchedDetectors) == 0 {
t.Errorf("test %q failed: expected keywords %v to be found in the input", test.name, d.Keywords())
return
}

results, err := d.FromData(context.Background(), true, []byte(test.input))
require.NoError(t, err)

if len(results) != len(test.want) {
t.Errorf("mismatch in result count: expected %d, got %d", len(test.want), len(results))
return
}

for i, r := range results {
if r.AnalysisInfo == nil {
t.Errorf("expected analysis info to be populated")
return
}

if diff := cmp.Diff(test.want[i], r.AnalysisInfo); diff != "" {
t.Errorf("%s diff: (-want +got)\n%s", test.name, diff)
}
}
})
}
}