Skip to content
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.

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

Correctly validating new real-world OpenAI API Key format, relaxing negative tests #3330

Closed
wants to merge 3 commits into from

Conversation

nstielau
Copy link

@nstielau nstielau commented Aug 8, 2024

Why are these changes needed?

Using a real-world OpenAI API key, which successfully works with the OpenAI hosted models, I encountered a warning message that the API key was not valid. This red-herring lead to slow debugging, as the issue was in fact a space character in an agent name rather than anything to do with the API key.

Approach

I've removed two negative tests. Writing a validation implementation that passes these tests is possible, but in my opinion it is preferable to have a simple validation function that doesn't cover all possibilities. It seems like 99.9% of the time, a user would copy/paste their API Key, and the updated function should catch common positive and negative tests.

Additionally, in my opinion, false positives that log the warning message incorrectly are as bad as false negatives which would not indicate the API Key when it is not working.

Reviewers

@wrfly and @AaronWard PTAL TY!

Related issue number

n/a
See #3078 for last changes

Checks

Copy link

gitguardian bot commented Aug 8, 2024

⚠️ GitGuardian has uncovered 6 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

Since your pull request originates from a forked repository, GitGuardian is not able to associate the secrets uncovered with secret incidents on your GitGuardian dashboard.
Skipping this check run and merging your pull request will create secret incidents on your GitGuardian dashboard.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
12853598 Triggered Generic High Entropy Secret f5522a9 test/oai/test_utils.py View secret
- - Generic High Entropy Secret f5522a9 test/oai/test_utils.py View secret
- - Generic High Entropy Secret f5522a9 test/oai/test_utils.py View secret
12853600 Triggered Generic High Entropy Secret f5522a9 test/oai/test_utils.py View secret
12853601 Triggered Generic High Entropy Secret f5522a9 test/oai/test_utils.py View secret
12853602 Triggered Generic High Entropy Secret f5522a9 test/oai/test_utils.py View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secrets safely. Learn here the best practices.
  3. Revoke and rotate these secrets.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@nstielau
Copy link
Author

nstielau commented Aug 8, 2024

@microsoft-github-policy-service agree

@AcePeaX
Copy link

AcePeaX commented Aug 16, 2024

This PR would normally fix this issue #3345. I'm facing the same problem and can't wait for this PR to be merged 👍 .

@nstielau
Copy link
Author

Thanks for connecting to that issue @AcePeed.

@AaronWard
Copy link
Collaborator

@sonichi Can you assign me as a reviewer?

@rseymour
Copy link

Also facing same issue a valid API key format is out of the hands of this library and OpenAI returns an error when one is valid.

@buddycat
Copy link

The updated regex still rejects my valid OpenAI api_key. It would need to be adjusted to allow for multiple underscores, possibly like ^sk-(?!.--)([a-zA-Z0-9]+(?:-[a-zA-Z0-9_]+))$

@nstielau
Copy link
Author

@buddycat can you give a test-case assertion for your key?

@nstielau
Copy link
Author

Haha, FYI, I tried to get autogen agent to fix this. I tweaked the code execution to track changes in git. You can view the iterations here: https://github.githistory.xyz/nstielau/autogen_iterations/blob/main/ai_regex_quiz__temp_0.95.py

It couldn't solve for the current test cases with my new test case :/

@buddycat
Copy link

Here's a test-case that should be true:

sk-proj-111111111122222aaaaaaaa_7777hhhhh_111111111122222aaaaaaa_99900000fggfffhgg

@jackgerrits jackgerrits mentioned this pull request Sep 25, 2024
3 tasks
@jackgerrits
Copy link
Member

Superseded by #3569

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