-
Notifications
You must be signed in to change notification settings - Fork 559
Support NIM API key for NemoGuard JailbreakDetect #1214
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
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1214 +/- ##
========================================
Coverage 68.65% 68.65%
========================================
Files 161 161
Lines 15978 15996 +18
========================================
+ Hits 10969 10982 +13
- Misses 5009 5014 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
cparisien
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok with this and it appears to resolve the Datastax issue around jailbreak server URLs. Will need an additional look from Tim, and I'd like to understand why the Windows python checks are failing.
|
@jeffreyscarpenter let me know if this satisfies your needs and reconciles the discrepancies between this and #1227 |
thank you @erickgalinkin , would you please make sure that these are sufficiently covered? |
|
Thank you @erickgalinkin I can see that the he move from I think this PR also introduces a breaking change:
Before merging, please consider:
(re 1 & 2 see #1214 (comment))
|
…t to use base_url and endpoints. Refactor checks to align with base_uri and api_key_env_var approaches. Add additional error handling and logging. Fix tests to reflect changes. Signed-off-by: Erick Galinkin <egalinkin@nvidia.com>
Signed-off-by: Erick Galinkin <egalinkin@nvidia.com>
Signed-off-by: Erick Galinkin <egalinkin@nvidia.com>
Signed-off-by: Erick Galinkin <egalinkin@nvidia.com>
- Fix TypeError when classifier is None by adding defensive programming - Replace silent failure with clear RuntimeError and descriptive message - Simplify calling code by removing redundant null checks from actions.py and server.py - Update tests to match new function signature and behavior - Add test coverage for new RuntimeError path This resolves the critical bug where check_jailbreak(prompt) would crash with "TypeError: 'NoneType' object is not callable" when EMBEDDING_CLASSIFIER_PATH is not set. Now it raises a clear RuntimeError with guidance on how to fix it.
4ed2d85 to
f640d29
Compare
Pouyanpi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @erickgalinkin, we are good to merge 👍🏻
3002c88 to
b23863b
Compare
|
Rolled back weird changes that got pushed. Thanks @Pouyanpi! |
Update jailbreak detection compatibility for NIM to allow providing an API key.
Tagging @tgasser-nv for review
Description
Allows the use of
integrate.nvidia.comand other NIM deployments that require API key.config.JailbreakDetectionConfigobject to includenim_auth_tokenparameter with a default value ofNoneandnim_classification_pathwith a default value of"/v1/classify".jailbreak_detection.request.jailbreak_nim_requestto acceptnim_auth_tokenandnim_classification_pathparameters and add the"Authorization: Bearer"header if a key is provided.jailbreak_detection.actions.jailbreak_detection_modelto extract thenim_auth_tokenandnim_classification_pathfrom the config and pass them tojailbreak_nim_request.Tested with config values:
nim_url = "https://ai.api.nvidia.com"nim_port=443nim_auth_token=my_tokennim_classification_path="/v1/security/nvidia/nemoguard-jailbreak-detect"Checklist