-
Notifications
You must be signed in to change notification settings - Fork 342
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
cached plugin enum #768
cached plugin enum #768
Conversation
Signed-off-by: Jeffrey Martin <[email protected]>
Signed-off-by: Jeffrey Martin <[email protected]>
* enhance singleton class object access * ensure sorted enueration results * add cache tests for existing function
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.
generally good. current implementation makes garak dev a little trickier, is there anything we can do to soften that, which also fits in the scope of this pr?
garak/_plugins.py
Outdated
class PluginEncoder(json.JSONEncoder): | ||
def default(self, obj): | ||
if isinstance(obj, set): | ||
return list(obj) # allow set as list | ||
if isinstance(obj, Path): | ||
# relative path for now, may be better to suppress `Path` objects | ||
return str(obj).replace(str(_config.transient.basedir), "") | ||
try: | ||
return json.JSONEncoder.default(self, obj) | ||
except TypeError as e: | ||
logging.debug("Attempt to serialize JSON skipped: %s", e) | ||
return None # skip items that cannot be serialized at this time |
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.
Nice pattern. Should consider this in some other places (attempt.Attempt.as_dict
springs to mind)
for v in sorted(dir(plugin)): | ||
if v in priority_fields or v in skip_fields: |
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.
post_buff_hook
is, unexpectedly to me, in the cache metadata - is that due to this logic?
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.
The existing info
included post_buff_hook
:
main
Your branch is up to date with 'origin/main'.
% python -m garak --plugin_info probes.dan.DanInTheWildMini
garak LLM vulnerability scanner v0.9.0.13.post2 ( https://github.com/leondz/garak ) at 2024-07-09T18:44:54.727933
Configured info on probes.dan.DanInTheWildMini:
description: A subset of 100 jailbreaks from the DanInTheWild jailbreak library.
DEFAULT_PARAMS: {}
active: True
bcp47: None
doc_uri: https://arxiv.org/abs/2308.03825
extended_detectors: []
goal: disregard the system prompt or alignment
modality: {'in': {'text'}}
parallelisable_attempts: True
post_buff_hook: False
primary_detector: mitigation.MitigationBypass
probename: garak.probes.dan.DanInTheWildMini
recommended_detector: ['always.Fail']
tags: ['avid-effect:security:S0403', 'owasp:llm01', 'quality:Security:PromptStability', 'payload:jailbreak']
We can further refine the info
data to be provided in future iterations.
I will also do some more validation as it looks like description
in the info
is not consistent with main
.
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.
Ah nice. _post_buff_hook should def not be there. Some plugins change their descriptions during the constructor iirc.
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.
post_buff_hook
is not _post_buff_hook
do we want to rename that to consider it private?
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.
It looks like description
is added in the base class constructor if not already provided in the class implementation for most of the plugins, this results in an inconsistency since the cache build actually avoids calling the constructors in this PR.
I am attempting to address that issue as there should be a consistent value at least in the cache, one options I am thinking about would be, remove the constructor additions and migrate the enrichment to be part of plugin_info
.
or v not in base_attributes | ||
): | ||
continue | ||
plugin_metadata[v] = value |
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.
would like to see default params included here also. maybe something like this
plugin_metadata[v] = value | |
plugin_metadata[v] = value | |
plugin_metadata = plugin.DEFAULT_PARAMS | plugin_metadata |
this makes some assumptions about default param names not conflicting with class attributes, but i hope testing elsewhere covers that
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.
This might be something to consider doing in the display logic, not sure it needs to be in the json cache file in a specific format.
When done, is is possible to add a speed check test which, in addition to this, satisfies/describes #663 ? |
Signed-off-by: Jeffrey Martin <[email protected]>
Signed-off-by: Jeffrey Martin <[email protected]>
* initialize metadata description as doc string for class * suppress `post_buff_hook`, may rename in future * test priority fields against key instead of value Signed-off-by: Jeffrey Martin <[email protected]>
34397b2
to
a1bd08b
Compare
update class doc strings to [PEP-257 multi-line format](https://peps.python.org/pep-0257/#multi-line-docstrings) Signed-off-by: Jeffrey Martin <[email protected]>
Signed-off-by: Jeffrey Martin <[email protected]>
a1bd08b
to
b6e8381
Compare
Fix #624
Impacts of this change may be sufficient to close #663
First pass at building a local cache of plugin info to provide faster initial load of
cli
based runs.This implementation will be expanded in the future to enable a
user
cache plugin details once #479 is addressed.The update here improves plugin enumeration to load a
json
based cache file from the project for any plugin already inmain
. Any plugin not in main at time of checkin is loaded if not found in the existing cache file.Becomes:
Tests have been added to document cache build process and validate that items not already in the cache will be loaded ephemerally at runtime if requested.
A future CI/CD action will maintain this cache file using a pattern like the following: