-
-
Notifications
You must be signed in to change notification settings - Fork 713
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
LLM Integration #5861
base: main
Are you sure you want to change the base?
LLM Integration #5861
Conversation
All contributors have signed the CLA ✍️ ✅ |
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'll have to finish the rest of my review in a bit, but here's some starting bits.
addOns/llm/src/main/java/org/zaproxy/addon/llm/ExtensionLlm.java
Outdated
Show resolved
Hide resolved
addOns/llm/src/main/java/org/zaproxy/addon/llm/ExtensionLlm.java
Outdated
Show resolved
Hide resolved
addOns/llm/src/main/java/org/zaproxy/addon/llm/ExtensionLlm.java
Outdated
Show resolved
Hide resolved
addOns/llm/src/main/java/org/zaproxy/addon/llm/services/LlmCommunicationService.java
Outdated
Show resolved
Hide resolved
addOns/llm/src/main/java/org/zaproxy/addon/llm/services/LlmCommunicationService.java
Outdated
Show resolved
Hide resolved
addOns/llm/src/main/java/org/zaproxy/addon/llm/services/LlmCommunicationService.java
Outdated
Show resolved
Hide resolved
addOns/llm/src/main/java/org/zaproxy/addon/llm/services/LlmCommunicationService.java
Outdated
Show resolved
Hide resolved
addOns/llm/src/main/java/org/zaproxy/addon/llm/services/LlmCommunicationService.java
Outdated
Show resolved
Hide resolved
addOns/llm/src/main/java/org/zaproxy/addon/llm/ExtensionLlm.java
Outdated
Show resolved
Hide resolved
addOns/llm/src/main/java/org/zaproxy/addon/llm/communication/Confidence.java
Show resolved
Hide resolved
addOns/llm/src/main/java/org/zaproxy/addon/llm/services/LlmAssistant.java
Outdated
Show resolved
Hide resolved
getExtAlert().updateAlert(updatedAlert); | ||
getExtAlert().updateAlertInTree(originalAlert, updatedAlert); | ||
if (alert.getHistoryRef() != null) { | ||
alert.getHistoryRef().updateAlert(updated_alert); | ||
alert.getHistoryRef().updateAlert(updatedAlert); | ||
if (alert.getHistoryRef().getSiteNode() != null) { | ||
// Needed if the same alert was raised on another href for the same | ||
// SiteNode | ||
alert.getHistoryRef().getSiteNode().updateAlert(updated_alert); | ||
alert.getHistoryRef().getSiteNode().updateAlert(updatedAlert); |
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 confused about these conditionals and doing the same action over and over, shouldn't the call hat 167 have done it regardless of the conditions?
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 don't know why, but without this I get this exception when I do alert review for a node of alerts :
Index -1 out of bounds for length 10
java.lang.ArrayIndexOutOfBoundsException: Index -1 out of bounds for length 10
I used the same code from this extension : https://github.com/zaproxy/zap-extensions/blob/main/addOns/alertFilters/src/main/java/org/zaproxy/zap/extension/alertFilters/ExtensionAlertFilters.java#L459
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.
@kingthorin : do you see another way to walk through alerts ?
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 is the "normal" loop we use for reading. I guess part of the problem you're encountering might be timing with the updates. I'd have to play with it and see if your conditions can be simplified or worked around somehow.
https://github.com/zaproxy/community-scripts/blob/main/standalone/Loop%20through%20alerts.js
addOns/llm/src/main/java/org/zaproxy/addon/llm/ui/ImportDialog.java
Outdated
Show resolved
Hide resolved
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.
Should only have the base help and messages.
There's likely a bunch of places swagger should be replaced with openapi. https://swagger.io/blog/api-strategy/difference-between-swagger-and-openapi/ |
Hi TmmmmmR, I started my review on this, but ended up having a few questions, can I propose we setup a meeting between yourself and the 2 reviewers so that to see a demo of the addon? Thank you, |
Hello @yns000, yes of course ! I'm available on slack under the dev-llm channel, my username is temmar. |
addOns/llm/src/main/java/org/zaproxy/addon/llm/services/LlmAssistant.java
Outdated
Show resolved
Hide resolved
addOns/llm/src/main/java/org/zaproxy/addon/llm/services/LlmAssistant.java
Outdated
Show resolved
Hide resolved
addOns/llm/src/main/java/org/zaproxy/addon/llm/services/LlmCommunicationService.java
Outdated
Show resolved
Hide resolved
addOns/llm/src/main/java/org/zaproxy/addon/llm/ui/ImportDialog.java
Outdated
Show resolved
Hide resolved
addOns/llm/src/main/java/org/zaproxy/addon/llm/ui/ImportDialog.java
Outdated
Show resolved
Hide resolved
addOns/llm/src/main/java/org/zaproxy/addon/llm/ui/settings/LlmOptionsPanel.java
Outdated
Show resolved
Hide resolved
addOns/llm/src/main/java/org/zaproxy/addon/llm/ui/settings/LlmOptionsParam.java
Outdated
Show resolved
Hide resolved
addOns/llm/src/main/java/org/zaproxy/addon/llm/utils/HistoryPersister.java
Outdated
Show resolved
Hide resolved
addOns/llm/src/main/javahelp/org/zaproxy/addon/llm/resources/help/contents/about.html
Outdated
Show resolved
Hide resolved
I have read the CLA Document and I hereby sign the CLA. |
Signed-off-by: Abdessamad TEMMAR <[email protected]>
Signed-off-by: Abdessamad TEMMAR <[email protected]>
Signed-off-by: Abdessamad TEMMAR <[email protected]>
Signed-off-by: Abdessamad TEMMAR <[email protected]>
Signed-off-by: Abdessamad TEMMAR <[email protected]>
You'll need to add |
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.
Added some comments which should get the help displayed correctly
PUBLIC "-//Sun Microsystems Inc.//DTD JavaHelp HelpSet Version 2.0//EN" | ||
"http://java.sun.com/products/javahelp/helpset_2_0.dtd"> | ||
<helpset version="2.0" xml:lang="en-GB"> | ||
<title>Simple Example Add-On</title> |
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.
-> LLM Integration (or somethong like that)
<title>Simple Example Add-On</title> | ||
|
||
<maps> | ||
<homeID>top</homeID> |
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.
top -> addon.llm
|
||
<index version="2.0"> | ||
<!-- index entries are merged (sorted) into core index --> | ||
<indexitem text="simple" target="simple" /> |
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.
<indexitem text="llm" target="addon.llm" />
"http://java.sun.com/products/javahelp/map_1_0.dtd"> | ||
|
||
<map version="1.0"> | ||
<mapID target="simple-icon" url="contents/images/cake.png" /> |
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.
all instances of simple
-> llm
. Will need a new icon - I'll look for something suitable 😁 Probably dont need the about.html
page?
<toc version="2.0"> | ||
<tocitem text="ZAP User Guide" tocid="toplevelitem"> | ||
<tocitem text="Add Ons" tocid="addons"> | ||
<tocitem text="Simple Example" image="simple-icon" target="simple"> |
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.
Simple > LLM, simple -> llm and drop the about.html page?
@@ -0,0 +1,23 @@ | |||
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 3.2 Final//EN"> |
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 file is probably not really needed?
Overview
This extension integrates LLM with ZAP and includes two main features:
Related Issues
Specify any related issues or pull requests by linking to them.
Checklist
./gradlew spotlessApply
for code formattingFor more details, please refer to the developer rules and guidelines.