Skip to content

Conversation

@amanfcp
Copy link
Contributor

@amanfcp amanfcp commented Jan 22, 2026

Description:

This PR adds OpenAI Admin Key Detector and enhances the pre-existing OpenAI detector to skip Admin Key verification

Checklist:

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

@amanfcp amanfcp requested a review from a team January 22, 2026 16:27
@amanfcp amanfcp requested review from a team as code owners January 22, 2026 16:27
Copy link
Contributor

@mustansir14 mustansir14 left a comment

Choose a reason for hiding this comment

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

Thanks for this. I have some concerns mentioned in the comments.

Also can we have tests demonstrating that admin keys are not being detected in the original detector and vice-versa?

@amanfcp amanfcp requested a review from mustansir14 January 26, 2026 11:36
Copy link
Contributor

@mustansir14 mustansir14 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@amanfcp
Copy link
Contributor Author

amanfcp commented Feb 2, 2026

I ran additional tests against the corpora datasets. The detector did not appear in any of the outputs, so this looks safe to merge

image image

// Matches: legacy keys (sk-{alnum}T3BlbkFJ...), project keys (sk-proj-...),
// service account keys (sk-svcacct-... or sk-service-...)
// Does NOT match: admin keys (sk-admin-...)
keyPat = regexp.MustCompile(`\b(sk-(?:(?:proj|svcacct|service)-[[:alnum:]_-]+|[a-zA-Z0-9]+)T3BlbkFJ[[:alnum:]_-]+)\b`)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: [:alnum:] and [a-zA-Z0-9] are interchangeable so using either one throughout the regex would be more consistent


// Keywords are used for efficiently pre-filtering chunks.
func (s Scanner) Keywords() []string {
return []string{"sk-admin-"}
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly in openai.go, the keyword could be updated to sk-proj, sk-svcacct, and sk-service.

Copy link
Contributor

@nabeelalam nabeelalam left a comment

Choose a reason for hiding this comment

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

Looks good. Just a couple non-blocking comments.

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.

6 participants