Skip to content

Conversation

ankushgoel27
Copy link
Contributor

Description:
This PR adds the detector to detect the Grafana cloud API detector. Opened before in PR #1865

How to create a token is described here - https://grafana.com/docs/grafana-cloud/developer-resources/api-reference/cloud-api/

grafana

Checklist:

  • Tests passing (make test-community)?
  • Lint passing (make lint this requires golangci-lint)?

Sorry, something went wrong.

@ankushgoel27 ankushgoel27 requested a review from a team as a code owner October 24, 2023 07:45
@dustin-decker dustin-decker added the Hacktoberfest-Detector-Competition-New Apply this label if you are adding a new detector for the detector competition label Oct 24, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
var (
defaultClient = common.SaneHttpClient()
// Make sure that your group is surrounded in boundary characters such as below to reduce false positives.
keyPat = regexp.MustCompile(`(glc_[A-Za-z0-9+\/]{50,150}\={0,2})`)
Copy link
Contributor

Choose a reason for hiding this comment

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

we should add a prefix, and word bounds
regexp.MustCompile(detectors.PrefixRegex([]string{"grafana"}) + `\b(glc_[A-Za-z0-9+\/]{50,150}\={0,2})\b`)

Copy link
Contributor

Choose a reason for hiding this comment

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

also, what made you add \={0,2}? i was not able to generate a token with that format

Copy link
Contributor

@rgmz rgmz Oct 30, 2023

Choose a reason for hiding this comment

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

I agree with word bounds, but I'd argue that a prefix isn't necessary due to the token's glc_ prefix. Adding "grafana" as a prefix will result in valid secrets being missed, in part because of #2035.

also, what made you add ={0,2}? i was not able to generate a token with that format

The part after glc_ is a base64-encoded JWK, so = at the end is possible even if unlikely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes @rgmz you are correct. adding word bound /b is causing issue due the presence of == sign in the secret. Also , its base64 encoded so == presence is possible. also glc_ is unique in itself so i didn't include the prefix regex of "grafana"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if i include the /b word boundary and the token contains an = sign. the regex is not picking it up and only picks up the part before it missing the = sign all together from the secret detection.

Copy link
Contributor

Choose a reason for hiding this comment

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

makes sense, let's omit the prefix. i don't understand why the word bounds causes the = to not be picked up, but you're right that is indeed the case. i think it still makes sense to add at the beginning at least

Copy link
Contributor Author

Choose a reason for hiding this comment

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

its because "=" is part of the word boundary char so it's not picked up. I added the /b at the beginning of the regex.

Copy link
Contributor

Choose a reason for hiding this comment

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

i don't understand why the word bounds causes the = to not be picked up

@ankushgoel27 is correct.

Word boundaries (\b) match the following conditions:

  • ^[a-zA-Z0-9_] (start followed by word character)
  • [a-zA-Z0-9_]$ (word character followed by end)
  • [^a-zA-Z0-9_][a-zA-Z0-9_] (non-word character followed by word character)
  • [a-zA-Z0-9_][^a-zA-Z0-9_] (word character followed by non-word character)

Hence, the = at the end isn't picked up because it is treated as a word boundary itself.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Copy link
Contributor

@0x1 0x1 left a comment

Choose a reason for hiding this comment

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

ty for the submission

@0x1 0x1 merged commit b95ed3b into trufflesecurity:main Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Hacktoberfest-Detector-Competition-New Apply this label if you are adding a new detector for the detector competition
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants