llm: Pass OtherInfo of Alert to LLM

[njmulsqb]

Overview

Passes otherinfo to LLM for additional context.

Related Issues

fixes zaproxy/zaproxy#8988

[njmulsqb]

I know it currently, doesn't check if otherinfo is present and only pass if thats the case. I am unable to find some good documentation for langchain4j that lists how conditions can be added (and I am not trusting the solution what chatGPT is suggesting me until I cross-verify it with some documentation)

Therefore adding WIP until I find the solution. Please suggest if someone knows already.

[kingthorin]

Thanks for tackling this

[psiinon]

Checkmarx One – Scan Summary & Details – 09d9b3d7-de2b-47cd-97da-2e97e288a731

Great job! No new security vulnerabilities introduced in this pull request

[kingthorin]

@njmulsqb are you still working on this?

[njmulsqb]

Yeah, pretty occupied these days. Will use stringUtils to solve the problem

[kingthorin]

I'm happy to tackle it if you e gotten too bogged down. Just let me know.

[njmulsqb]

I can only manage to do this on coming Friday, if you've time to do it before that than please do.

[kingthorin]

I don't think I can push to this branch, but here's a patch you can apply:

diff --git a/addOns/llm/src/main/java/org/zaproxy/addon/llm/services/LlmAssistant.java b/addOns/llm/src/main/java/org/zaproxy/addon/llm/services/LlmAssistant.java
index 53d5deb6d3..c2973a3624 100644
--- a/addOns/llm/src/main/java/org/zaproxy/addon/llm/services/LlmAssistant.java
+++ b/addOns/llm/src/main/java/org/zaproxy/addon/llm/services/LlmAssistant.java
@@ -26,6 +26,35 @@ import org.zaproxy.addon.llm.communication.Confidence;
 import org.zaproxy.addon.llm.communication.HttpRequestList;
 
 public interface LlmAssistant {
+
+    static final String PRIMARY_SYSTEM_MSG =
+            "You are a web application security expert reviewing potential false positives. Answer only in JSON.";
+    static final String PRIMARY_PROMPT =
+            """
+            Your task is to review the following finding from ZAP (Zed Attack Proxy).
+            The confidence level is a pull down field which allows you to specify how confident you are in the validity of the finding:
+            - 0 if it's False Positive
+            - 1 if it's Low
+            - 2 if it's Medium
+            - 3 if it's High
+
+            The alert is described as follows : {{description}}
+
+            As evidence, the HTTP response contains:
+            ---
+            {{evidence}}
+            ---
+            """;
+
+    static final String PRIMARY_GOAL = "Provide a short consistent explanation of the new score.\n";
+    static final String PRIMARY_PROMPT_WITH_OTHERINFO =
+            PRIMARY_PROMPT
+                    + """
+                    Also, here's some additional information that may be useful for you to reach your conclusion:
+                    ---
+                    {{otherinfo}}
+                    """;
+
     @UserMessage(
             "Given the following OpenAPI definition, generate a list of chained HTTP requests to simulate a real world interaction : {{openapi}} ")
     HttpRequestList extractHttpRequests(String openapi);
@@ -34,26 +63,12 @@ public interface LlmAssistant {
             "As a software architect, and based on your previous answer, generate other potential missing endpoints that are not mentioned in the OpenAPI file. For example, if there is GET /product/1, suggest DELETE /product/1 if it's not mentioned")
     HttpRequestList complete();
 
-    @SystemMessage(
-            "You are a web application security expert reviewing potential false positives. Answer only in JSON.")
-    @UserMessage(
-            "Your task is to review the following finding from ZAP (Zed Attack Proxy).\n"
-                    + "The confidence level is a pull down field which allows you to specify how confident you are in the validity of the finding : \n"
-                    + "- 0 if it's False Positive\n"
-                    + "- 1 if it's Low\n"
-                    + "- 2 if it's Medium\n"
-                    + "- 3 if it's High\n"
-                    + "\n"
-                    + "The alert is described as follows : {{description}}\n"
-                    + "\n"
-                    + "As evidence, the HTTP response contains :\n"
-                    + "---\n"
-                    + "{{evidence}}\n"
-                    + "---\n"
-                    + "Also, here's some additional information that may be useful for you to reach your conclusion"
-                    + "---\n"
-                    + "{{otherinfo}}"
-                    + "Provide a short consistent explanation of the new score.\n")
+    @SystemMessage(PRIMARY_SYSTEM_MSG)
+    @UserMessage(PRIMARY_PROMPT + PRIMARY_GOAL)
+    Confidence review(@V("description") String description, @V("evidence") String evidence);
+
+    @SystemMessage(PRIMARY_SYSTEM_MSG)
+    @UserMessage(PRIMARY_PROMPT_WITH_OTHERINFO + PRIMARY_GOAL)
     Confidence review(
             @V("description") String description,
             @V("evidence") String evidence,
diff --git a/addOns/llm/src/main/java/org/zaproxy/addon/llm/services/LlmCommunicationService.java b/addOns/llm/src/main/java/org/zaproxy/addon/llm/services/LlmCommunicationService.java
index 03a7c07480..c6f6038ac8 100644
--- a/addOns/llm/src/main/java/org/zaproxy/addon/llm/services/LlmCommunicationService.java
+++ b/addOns/llm/src/main/java/org/zaproxy/addon/llm/services/LlmCommunicationService.java
@@ -36,6 +36,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
 import org.apache.commons.httpclient.util.HttpURLConnection;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.parosproxy.paros.Constant;
@@ -170,9 +171,11 @@ public class LlmCommunicationService {
             LOGGER.debug("Reviewing alert : {}", alert.getName());
             LOGGER.debug("Confidence level from ZAP : {}", alert.getConfidence());
             Stats.incCounter("stats.llm.alertreview.call");
-            llmConfidence =
-                    llmAssistant.review(
-                            alert.getDescription(), alert.getEvidence(), alert.getOtherInfo());
+            if (StringUtils.isBlank(alert.getOtherInfo())) {
+                llmConfidence = llmAssistant.review(alert.getDescription(), alert.getEvidence());
+            } else {
+                llmConfidence = llmAssistant.review(alert.getDescription(), alert.getEvidence(), alert.getOtherInfo());
+            }
 
             if (llmConfidence.getLevel() == alert.getConfidence()) {
                 Stats.incCounter("stats.llm.alertreview.result.same");
@@ -209,7 +212,7 @@ public class LlmCommunicationService {
     }
 
     private static boolean isPreviouslyReviewed(Alert alert) {
-        return !alert.getTags().containsKey(AI_REVIEWED_TAG_KEY);
+        return alert.getTags().containsKey(AI_REVIEWED_TAG_KEY);
     }
 
     private static String getUpdatedOtherInfo(Alert alert, Confidence llmConfidence) {

[kingthorin]

I think this is good to go.

[kingthorin]

Superseded by #6653