Skip to content

Conversation

@LanceMcCarthy
Copy link

@LanceMcCarthy LanceMcCarthy commented Sep 29, 2025

Description:

Added new detector for Telerik and Kendo License Keys (https://www.telerik.com). These JWTs are secrets which contain license information that must not be in source code.

This detector does everything locally by checking only the JWT's header for the typ attribute for our unique value "Telerik License Key".

Optimized Performance

There are no remote API calls to perform the detector's work, so I've also optimized the detector code and tests by removing httpClient usage for the following benefits:

✅ Reduced Dependencies, Cleaner Code, Better Performance: No HTTP client overhead (mocked or not)
✅ Simpler Testing: Tests are more focused on actual JWT validation logic
✅ All Tests Pass: Both unit tests and integration tests work perfectly (and the benchmarks look good too)

Tests

In my tests, there are two hard coded JWTs secret and inactiveSecret. This is nothing to be concerned about, they are sample values generated specifically for long term testing support and are not secrets.

One has the expected header value to verify a confirmed positive hit, while the other is like any other normal JWT, and does not trigger a false positive.

image

Note

If these test keys trigger your secret-detection workflow, please let me know how to add an ignore for them. Alternatively, I can switch back to the GCP approach, but that was giving me nothing but headaches.

Checklist:

  • ✅ Tests passing (make test-community)?
    • image
  • Lint passing (make lint this requires golangci-lint)?
    • I attempted to run this, but it seems to be an outdated instruction as there's a default flag not accepted. Might be outdated tooling in the GitHub Codespace, I can rerun with additional instructions.
    • image

[EDIT] Spelling corrections, and added links/note about the test keys.

✅ Reduced Dependencies: No longer depends on HTTP client libraries
✅ Cleaner Code: Removed unused HTTP client logic
✅ Better Performance: No HTTP client initialization overhead
✅ Simpler Testing: Tests are more focused on actual JWT validation logic
✅ All Tests Pass: Both unit tests and integration tests work perfectly
@LanceMcCarthy LanceMcCarthy requested review from a team as code owners September 29, 2025 18:59
@CLAassistant
Copy link

CLAassistant commented Sep 29, 2025

CLA assistant check
All committers have signed the CLA.

@LanceMcCarthy
Copy link
Author

Hi Team,

A couple notes/questions:

  • Once it's time for merge, please consider squash to avoid the mess. I have a few commits in there just so I could move between my local environment to GH Codespaces.
  • I'd like to ask how detector updates work. For example, if we introduce new key types, can I update this detector. If yes, are they versioned or just continuous improvement?

Thanks!

@LanceMcCarthy LanceMcCarthy changed the title New Detector: TelerikLicenseKey New Detector: TelerikLicenseKey [1039] Sep 29, 2025
@shahzadhaider1
Copy link
Contributor

Hey @LanceMcCarthy, thanks for the contribution! We’ll review the PR and share feedback if needed.

Regarding detector updates: if a secret type is deprecated and no longer works for authentication, we update the existing detector. But if a new key type is introduced while the old one still works, we create a new version (v2) of the detector that supports the new key type, and move the old type into v1. That way, we can maintain both versions of the detector side by side.

For more info: https://github.com/trufflesecurity/trufflehog/blob/main/hack/docs/Adding_Detectors_external.md

@LanceMcCarthy
Copy link
Author

Thank you for the prompt follow up and answers @shahzadhaider1! I was eagerly awaiting the results of the checks because I couldn't run the linter manually, but the two linter workflows/check look good :)

My remaining worry was if the 'scan for secrets` check would throw a false positive because of my test JWTs. Is there a reason they were skipped, do I need the code review first?

@LanceMcCarthy
Copy link
Author

Hi @shahzadhaider1 I just wanted to hop in and see if there's anything I can do to help speed things up.

I also suspect that by the time this is ready, I'll need to change the detector ID as newer detectors are added before this is merged. Let me know how you guys prefer to handle this; do I push a commit with n+1 or do you do it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants