Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4100
File: Ginger/Ginger/ApplicationModelsLib/APIModels/APIModelWizard/AddAPIModelExtraConfigsPage.xaml:28-30
Timestamp: 2025-02-14T07:02:48.416Z
Learning: In AddAPIModelExtraConfigsPage.xaml, the WireMock mapping checkbox (xWireMockMappingToggle) is initially disabled and only gets enabled after successful validation of WireMock configuration and connection test in the CheckWireMockConfiguration method.

Learnt from: prashelke
PR: Ginger-Automation/Ginger#4067
File: Ginger/GingerCoreCommon/Repository/SolutionRepository.cs:55-55
Timestamp: 2025-01-21T11:43:12.379Z
Learning: In Ginger's SolutionRepository, the path @"ExecutionResults\GingerExecutionResults.db" needs to be explicitly excluded in mSolutionPathsToAvoid list, even though its parent directory @"ExecutionResults\" is also excluded. This specific file exclusion serves a distinct purpose and should not be considered redundant.

Learnt from: prashelke
PR: Ginger-Automation/Ginger#4232
File: Ginger/GingerCoreNET/ActionsLib/UI/VisualTesting/VRTAnalyzer.cs:19-22
Timestamp: 2025-06-16T10:37:13.073Z
Learning: In Ginger codebase, both `using amdocs.ginger.GingerCoreNET;` and `using Amdocs.Ginger.CoreNET;` are valid and serve different purposes. The first (lowercase) contains the WorkSpace class and related workspace functionality, while the second (proper case) contains drivers like GenericAppiumDriver and other core functionality. Both may be required in files that use types from both namespaces.

Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4188
File: Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs:22-23
Timestamp: 2025-04-25T13:29:45.059Z
Learning: The `using Amdocs.Ginger.Repository;` statement in Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs is necessary as it provides access to the `ObservableList<>` class which is used throughout the file for collections of RunSetConfig, AnalyzerItemBase, and ApplicationPOMModel objects.

Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4188
File: Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs:22-23
Timestamp: 2025-04-25T13:29:45.059Z
Learning: The `using Amdocs.Ginger.Repository;` statement in Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs is necessary as it imports the ObservableList<> class which is used throughout the file.

Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4137
File: Ginger/Ginger/SourceControl/SourceControlProjectsPage.xaml.cs:21-21
Timestamp: 2025-03-20T11:10:30.816Z
Learning: The `Amdocs.Ginger.Common.SourceControlLib` namespace is required in files that reference the `GingerSolution` class for source control operations in the Ginger automation framework.

Learnt from: prashelke
PR: Ginger-Automation/Ginger#4232
File: Ginger/GingerCoreNET/ActionsLib/UI/VisualTesting/VRTAnalyzer.cs:19-22
Timestamp: 2025-06-16T09:52:40.873Z
Learning: The `GenericAppiumDriver` class is located in the `Amdocs.Ginger.CoreNET` namespace, not in a namespace that matches its file path structure. File paths don't always correspond to namespace declarations in this codebase.

Ginger/Ginger/Ginger.csproj (1)

725-769: Prior request resolved: no OWASPZAPDotNetAPI PackageReference in Ginger UI

Confirming the UI project no longer directly references OWASPZAPDotNetAPI. This aligns with the architectural guidance to keep the package in GingerCore/GingerCoreNET only and rely on transitive usage.

Ginger/Ginger/Drivers/DriversConfigsEditPages/WebServicesDriverEditPage.xaml (2)

7-8: Verify xmlns:local resolves MultiBooleanToVisibilityConverter at runtime

Ensure MultiBooleanToVisibilityConverter actually lives in Ginger.Actions.ActionConversion; otherwise StaticResource lookup will fail. If it lives in a common converters namespace, point xmlns accordingly.

---

154-179: Wire ZAP controls to driver params and fix label punctuation/casing

The controls render but won’t persist without bindings unless code‑behind handles it. Also remove extra spaces before colons and normalize casing.

-                    <Label Content="Security Testing Configurations" Style="{StaticResource $HorizontalExpanderLabelStyle}"/>
+                    <Label Content="Security testing configurations" Style="{StaticResource $HorizontalExpanderLabelStyle}"/>

-                        <CheckBox x:Name="xEnableZAPCheckBox" Content="Enable Security Testing" Style="{StaticResource $CeckBoxStyle}" />
+                        <CheckBox x:Name="xEnableZAPCheckBox"
+                                  Content="Enable security testing"
+                                  Style="{StaticResource $CeckBoxStyle}"/>

-                            <Label Content="Scan type : " Style="{StaticResource $LabelStyle}" Grid.Column="0" Grid.Row="0" HorizontalAlignment="Left"/>
+                            <Label Content="Scan type:" Style="{StaticResource $LabelStyle}" Grid.Column="0" Grid.Row="0" HorizontalAlignment="Left"/>
-                            <ComboBox x:Name="xScanTypeComboBox" IsEditable="False" Style="{StaticResource $FlatInputComboBoxStyle}" Grid.Column="1" Grid.Row="0" Width="120" HorizontalAlignment="Left"/>
+                            <ComboBox x:Name="xScanTypeComboBox"
+                                      IsEditable="False"
+                                      Style="{StaticResource $FlatInputComboBoxStyle}"
+                                      Grid.Column="1" Grid.Row="0" Width="120" HorizontalAlignment="Left"/>

-                            <Label Content="Failure Vulnerability Threshold : " Style="{StaticResource $LabelStyle}" Grid.Column="0" Grid.Row="1" HorizontalAlignment="Left"/>
+                            <Label Content="Failure vulnerability threshold:" Style="{StaticResource $LabelStyle}" Grid.Column="0" Grid.Row="1" HorizontalAlignment="Left"/>
-                            <ComboBox x:Name="xVulnerabilityTypeComboBox" IsEditable="False" Style="{StaticResource $FlatInputComboBoxStyle}" Grid.Column="1" Grid.Row="1" Width="120" HorizontalAlignment="Left"/>
+                            <ComboBox x:Name="xVulnerabilityTypeComboBox"
+                                      IsEditable="False"
+                                      Style="{StaticResource $FlatInputComboBoxStyle}"
+                                      Grid.Column="1" Grid.Row="1" Width="120" HorizontalAlignment="Left"/>

If you prefer XAML bindings (instead of code‑behind), I can provide a binding diff for UseSecurityTesting/ZapScanTypeSetting/ZapVulnerabilitySetting.

Ginger/GingerCoreCommon/ReporterLib/UserMsgsPool.cs (1)

830-831: Polish the ZAP connection messages (grammar + punctuation)

Aligns with style used elsewhere (“connection failed/successful.”), adds period, and clarifies instruction.

-                { eUserMsgKey.ZAPConnectionFail, new UserMsg(eUserMsgType.ERROR, "ZAP Connection Info", "ZAP Connection Failed, Please check credentials/check error logs", eUserMsgOption.OK, eUserMsgSelection.None) },
-                { eUserMsgKey.ZAPConnectionSuccess, new UserMsg(eUserMsgType.INFO, "ZAP Connection Info", "ZAP Connection Successful", eUserMsgOption.OK, eUserMsgSelection.None) },
+                { eUserMsgKey.ZAPConnectionFail, new UserMsg(eUserMsgType.ERROR, "ZAP Connection Info", "ZAP connection failed. Please check credentials or review error logs.", eUserMsgOption.OK, eUserMsgSelection.None) },
+                { eUserMsgKey.ZAPConnectionSuccess, new UserMsg(eUserMsgType.INFO, "ZAP Connection Info", "ZAP connection successful.", eUserMsgOption.OK, eUserMsgSelection.None) },

Ginger/Ginger/Actions/ActionEditPages/ActSecurityTestingEditPage.xaml.cs (4)

61-64: Default/selection should use SelectedValue, not Text

Setting Text won’t select the item; it also fights the SelectedValue binding.

-            GingerCore.General.FillComboFromEnumType(xScanTypeComboBox, typeof(eScanType), null);
-            xScanTypeComboBox.Text = GingerCore.General.GetEnumValueDescription(typeof(eScanType), eScanType.Active);
-            GingerCore.GeneralLib.BindingHandler.ObjFieldBinding(xScanTypeComboBox, ComboBox.SelectedValueProperty, mAct, ActSecurityTesting.Fields.ScanType);
+            GingerCore.General.FillComboFromEnumType(xScanTypeComboBox, typeof(eScanType), null);
+            xScanTypeComboBox.SelectedValue = mAct.ScanType != default ? mAct.ScanType : eScanType.Active;
+            GingerCore.GeneralLib.BindingHandler.ObjFieldBinding(xScanTypeComboBox, ComboBox.SelectedValueProperty, mAct, ActSecurityTesting.Fields.ScanType);

---

82-91: SelectionChanged reads wrong type from SelectedValue

FillComboFromEnumType sets SelectedValuePath="Value", so SelectedValue is eScanType, not string.

-        private void xScanTypeComboBox_SelectionChanged(object sender, SelectionChangedEventArgs e)
-        {
-            if (xScanTypeComboBox.SelectedValue is string selectedScanType)
-            {
-                if (Enum.TryParse(typeof(ActSecurityTesting.eScanType), selectedScanType, out var scanTypeEnum))
-                {
-                    mAct.ScanType = (ActSecurityTesting.eScanType)scanTypeEnum;
-                }
-            }
-        }
+        private void xScanTypeComboBox_SelectionChanged(object sender, SelectionChangedEventArgs e)
+        {
+            if (xScanTypeComboBox.SelectedValue is eScanType scanType)
+            {
+                mAct.ScanType = scanType;
+            }
+        }

Or remove the handler and rely on the TwoWay SelectedValue binding.

---

67-80: Harden platform resolution (null/empty safety)

Avoid potential NullReference/IndexOutOfRange when BF/CurrentActivity/TargetApplication or ApplicationPlatforms are null/empty.

-        private ePlatformType GetActionPlatform()
-        {
-            ePlatformType platform;
-            if (mAct.Context != null && (Context.GetAsContext(mAct.Context)).BusinessFlow != null)
-            {
-                string targetapp = (Context.GetAsContext(mAct.Context)).BusinessFlow.CurrentActivity.TargetApplication;
-                platform = WorkSpace.Instance.Solution.ApplicationPlatforms.FirstOrDefault(x => x.AppName == targetapp).Platform;
-            }
-            else
-            {
-                platform = WorkSpace.Instance.Solution.ApplicationPlatforms[0].Platform;
-            }
-            return platform;
-        }
+        private ePlatformType GetActionPlatform()
+        {
+            var solution = WorkSpace.Instance?.Solution;
+            var appPlatforms = solution?.ApplicationPlatforms;
+
+            if (mAct.Context != null)
+            {
+                var ctx = Context.GetAsContext(mAct.Context);
+                var bf = ctx?.BusinessFlow;
+                var ta = bf?.CurrentActivity?.TargetApplication;
+                var match = appPlatforms?.FirstOrDefault(x => !string.IsNullOrEmpty(ta) && x.AppName == ta);
+                if (match != null)
+                {
+                    return match.Platform;
+                }
+            }
+            return (appPlatforms != null && appPlatforms.Any())
+                ? appPlatforms[0].Platform
+                : ePlatformType.Web; // safe default
+        }

---

96-105: Severity order: put Informational before FalsePositive

Match common order and enum: High, Medium, Low, Informational, FalsePositive.

             ObservableList<OperationValues> SeverityList =
             [
                 new OperationValues() { Value = nameof(ActSecurityTesting.eAlertTypes.High) },
                 new OperationValues() { Value = nameof(ActSecurityTesting.eAlertTypes.Medium) },
                 new OperationValues() { Value = nameof(ActSecurityTesting.eAlertTypes.Low) },
-                new OperationValues() { Value = nameof(ActSecurityTesting.eAlertTypes.FalsePositive) },
-                new OperationValues() { Value = nameof(ActSecurityTesting.eAlertTypes.Informational) },
+                new OperationValues() { Value = nameof(ActSecurityTesting.eAlertTypes.Informational) },
+                new OperationValues() { Value = nameof(ActSecurityTesting.eAlertTypes.FalsePositive) },
             ];

Ginger/Ginger/Drivers/DriversConfigsEditPages/WebServicesDriverEditPage.xaml.cs (1)

139-154: Make ZAP scan/vulnerability combos enable/disable with the “Enable ZAP” checkbox

Improves UX and prevents selecting ZAP options when ZAP is off. Initialize state on load and toggle on check/uncheck.

Apply this diff after the existing bindings in BindConfigurations:

             BindingHandler.ObjFieldBinding(xVulnerabilityTypeComboBox, ComboBox.ToolTipProperty, zapVulnerability, nameof(DriverConfigParam.Description));

+            // Enable/disable ZAP-related combos based on the "Enable ZAP" checkbox
+            xEnableZAPCheckBox.Checked += (_, __) =>
+            {
+                xScanTypeComboBox.IsEnabled = true;
+                xVulnerabilityTypeComboBox.IsEnabled = true;
+            };
+            xEnableZAPCheckBox.Unchecked += (_, __) =>
+            {
+                xScanTypeComboBox.IsEnabled = false;
+                xVulnerabilityTypeComboBox.IsEnabled = false;
+            };
+            // Initialize current enabled state from the parameter value
+            if (bool.TryParse(useZAP.Value, out var zapEnabled))
+            {
+                xScanTypeComboBox.IsEnabled = zapEnabled;
+                xVulnerabilityTypeComboBox.IsEnabled = zapEnabled;
+            }

Ginger/Ginger/ExternalConfigurations/ZAPConfigurationPage.xaml (3)

73-93: Correct “About ZAP” attribution and phrasing

ZAP is an OWASP project, not by Checkmarx. Also use “man-in-the-middle” phrasing.

Apply this diff:

-                                        Zed Attack Proxy (ZAP) by Checkmarx is a free, open-source penetration testing tool. ZAP is
+                                        Zed Attack Proxy (ZAP) by the OWASP community is a free, open-source penetration testing tool. ZAP is
@@
-                                        At its core, ZAP is what is known as a “manipulator-in-the-middle proxy.” It stands between
+                                        At its core, ZAP is a “man-in-the-middle” proxy. It stands between

---

41-47: Fix misleading ToolTips (“GingerOps Account URL” / “Client Secret”)

These ToolTips are incorrect for ZAP configuration and may confuse users. Update to ZAP-specific wording.

Apply this diff:

-                    <Activities:UCValueExpression x:Name="xZAPURLTextBox" Grid.Column="1" Grid.Row="0" HorizontalAlignment="Left" VerticalAlignment="Center" ToolTip="GingerOps Account URL" Margin="10,0,0,0"  Width="400"/>
+                    <Activities:UCValueExpression x:Name="xZAPURLTextBox" Grid.Column="1" Grid.Row="0" HorizontalAlignment="Left" VerticalAlignment="Center" ToolTip="ZAP API URL (e.g., http://127.0.0.1:8080)" Margin="10,0,0,0"  Width="400"/>
@@
-                    <Activities:UCValueExpression x:Name="xZAPAPIkeyTextBox" Grid.Column="1" Grid.Row="1" HorizontalAlignment="Left" VerticalAlignment="Center" ToolTip="Client Secret" Margin="10,0,0,0" Width="400" LostKeyboardFocus="xZAPAPIkeyTextBox_LostKeyboardFocus"/>
+                    <Activities:UCValueExpression x:Name="xZAPAPIkeyTextBox" Grid.Column="1" Grid.Row="1" HorizontalAlignment="Left" VerticalAlignment="Center" ToolTip="ZAP API Key" Margin="10,0,0,0" Width="400" LostKeyboardFocus="xZAPAPIkeyTextBox_LostKeyboardFocus"/>

---

51-51: Change loader visibility to Collapsed in ZAPConfigurationPage.xaml.cs

The code-behind for ZAPConfigurationPage currently sets the processing‐image Visibility to Hidden, which contradicts its initial Collapsed state and can introduce unwanted layout gaps. Please update these occurrences to use Visibility.Collapsed instead.

• File: Ginger/Ginger/ExternalConfigurations/ZAPConfigurationPage.xaml.cs
– Line 111: xProcessingImage.Visibility = Visibility.Hidden;
– (Ensure no other Hidden usages remain in this file.)

Suggested diff:

--- a/Ginger/Ginger/ExternalConfigurations/ZAPConfigurationPage.xaml.cs
+++ b/Ginger/Ginger/ExternalConfigurations/ZAPConfigurationPage.xaml.cs
@@ 108,5
-               this.Dispatcher.Invoke(() =>
-               {
-                   xProcessingImage.Visibility = Visibility.Hidden;
-               });
+               this.Dispatcher.Invoke(() =>
+               {
+                   xProcessingImage.Visibility = Visibility.Collapsed;
+               });

Ginger/GingerCoreNET/Drivers/WebServicesDriver/WebServicesDriver.cs (2)

435-435: Optional: resolve variables before using EndPointURL

Use GetInputParamCalculatedValue to ensure variables are substituted.

Apply this one-line change if you agree:

-                        string endpointUrl = mActWebAPI.GetInputParamValue(ActWebAPIBase.Fields.EndPointURL);
+                        string endpointUrl = mActWebAPI.GetInputParamCalculatedValue(ActWebAPIBase.Fields.EndPointURL);

---

431-457: Fix: incorrect enum comparison, redundant checks, and lost AlertList in active path

• string.Equals on an enum is a compile/runtime bug. Use direct enum comparison.
• Inner else-if duplicates the same condition; simplify.
• Active path creates mActSecTest with AlertList but never passes it to HandleAPISecurityTesting, losing the threshold. Pass it through.

Apply this diff:

-                if (this.UseSecurityTesting)
-                {
-                    if (string.Equals(this.mZapScanTypeSetting, eZapScanType.Passive))
-                    {
-                        string endpointUrl = mActWebAPI.GetInputParamValue(ActWebAPIBase.Fields.EndPointURL);
-                        if (string.IsNullOrWhiteSpace(endpointUrl))
-                        {
-                            Reporter.ToLog(eLogLevel.WARN, "ZAP scan skipped: endpoint URL is empty");
-                        }
-                        else if (this.mZapScanTypeSetting == eZapScanType.Passive)
-                        {
-                            var mActSecTest = new ActSecurityTesting
-                            {
-                                AlertList = BuildAlertListFromThreshold(mZapVulnerabilitySetting)
-                            };
-                            mActSecTest.ExecutePassiveZapScan(endpointUrl, mActWebAPI);
-                        }
-                    }
-                    else
-                    {
-                        var mActSecTest = new ActSecurityTesting
-                        {
-                            AlertList = BuildAlertListFromThreshold(mZapVulnerabilitySetting)
-                        };
-                        HandleAPISecurityTesting(mActWebAPI);
-                    }
-                }
+                if (this.UseSecurityTesting)
+                {
+                    if (this.mZapScanTypeSetting == eZapScanType.Passive)
+                    {
+                        string endpointUrl = mActWebAPI.GetInputParamValue(ActWebAPIBase.Fields.EndPointURL);
+                        if (string.IsNullOrWhiteSpace(endpointUrl))
+                        {
+                            Reporter.ToLog(eLogLevel.WARN, "ZAP scan skipped: endpoint URL is empty");
+                        }
+                        else
+                        {
+                            var mActSecTest = new ActSecurityTesting
+                            {
+                                AlertList = BuildAlertListFromThreshold(mZapVulnerabilitySetting)
+                            };
+                            mActSecTest.ExecutePassiveZapScan(endpointUrl, mActWebAPI);
+                        }
+                    }
+                    else
+                    {
+                        var mActSecTest = new ActSecurityTesting
+                        {
+                            AlertList = BuildAlertListFromThreshold(mZapVulnerabilitySetting)
+                        };
+                        HandleAPISecurityTesting(mActWebAPI, mActSecTest);
+                    }
+                }

Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (4)

412-415: Document UseSecurityTesting precedence and security implications.

Add an XML doc so users/devs understand that enabling this overrides existing proxy settings and accepts ZAP’s MITM certificate.

-        [UserConfigured]
+        /// <summary>
+        /// Route browser traffic via OWASP ZAP for security testing.
+        /// When enabled, this takes precedence over Proxy, ProxyAutoConfigUrl and AutoDetect by forcing a Manual proxy
+        /// to the configured ZAP endpoint for the session, and sets AcceptInsecureCertificates=true so ZAP's MITM
+        /// certificate is trusted. Disable to restore normal proxy behavior.
+        /// </summary>
+        [UserConfigured]
         [UserConfiguredDefault("false")]
         [UserConfiguredDescription("Allow ZAP to Perform Security Testing")]
         public bool UseSecurityTesting { get; set; }

---

3581-3593: SetAttributeUsingJs should use setAttribute and fail clearly on bad input (no silent pass).

Current code treats the left side as a JS property, which breaks for data-* and many attributes, and it returns early with only a debug log.

-                {
-                    e = LocateElement(act);
-                    char[] delimit = new char[] { '=' };
-                    string insertval = act.GetInputParamCalculatedValue("Value");
-                    string[] vals = insertval.Split(delimit, 2);
-                    if (vals.Length != 2)
-                    {
-                        Reporter.ToLog(eLogLevel.DEBUG, @"Input string should be in the format : attribute=value");
-                        return;
-                    }
-                        ((IJavaScriptExecutor)Driver).ExecuteScript("arguments[0]." + vals[0] + "=arguments[1]", e, vals[1]);
-                }
+                {
+                    e = LocateElement(act);
+                    if (e == null)
+                    {
+                        act.Error = $"Error: Element not found - {act.LocateBy} {act.LocateValueCalculated}";
+                        act.Status = Amdocs.Ginger.CoreNET.Execution.eRunStatus.Failed;
+                        break;
+                    }
+                    string input = act.GetInputParamCalculatedValue("Value") ?? string.Empty;
+                    var parts = input.Split(new[] { '=' }, 2, StringSplitOptions.None);
+                    if (parts.Length != 2 || string.IsNullOrWhiteSpace(parts[0]))
+                    {
+                        act.Error = "Input must be in the format attribute=value";
+                        act.Status = Amdocs.Ginger.CoreNET.Execution.eRunStatus.Failed;
+                        break;
+                    }
+                    string attribute = parts[0].Trim();
+                    string value = parts[1];
+                    ((IJavaScriptExecutor)Driver).ExecuteScript(
+                        "arguments[0].setAttribute(arguments[1], arguments[2]);",
+                        e, attribute, value);
+                }

---

4602-4602: Improve error log with locator context for faster triage.

The current log is too generic; include element name and active locators.

-                Reporter.ToLog(eLogLevel.ERROR, $"Failed to locate element", ex);
+                Reporter.ToLog(
+                    eLogLevel.ERROR,
+                    $"Failed to locate element. Element='{currentPOMElementInfo?.ElementName}', " +
+                    $"ActiveLocators='{string.Join(", ", currentPOMElementInfo?.Locators?.Where(l => l.Active).Select(l => $"{l.LocateBy}=\"{l.LocateValue}\"") ?? Enumerable.Empty<string>())}'",
+                    ex);

---

1957-1968: Harden ActSecurity: null-safe URL and clear failure for Active scan; keep Passive unscoped.

Accessing Driver.Url can throw; Active scan requires a concrete URL. Passive scan intentionally uses "" to aggregate across the ZAP session, per team guidance.

-        private void ActSecurity(ActSecurityTesting act)
-        {
-            string testURL = Driver.Url;
-            if (act.ScanType == ActSecurityTesting.eScanType.Active)
-            {
-                act.ExecuteActiveZapScan(testURL);
-            }
-            else
-            {
-                act.ExecutePassiveZapScan("", act);
-            }
-        }
+        private void ActSecurity(ActSecurityTesting act)
+        {
+            string testURL = string.Empty;
+            try { testURL = Driver?.Url; } catch { /* ignore */ }
+
+            if (act.ScanType == ActSecurityTesting.eScanType.Active)
+            {
+                if (string.IsNullOrWhiteSpace(testURL))
+                {
+                    act.Error = "Active scan requires a current URL, but none was detected.";
+                    act.Status = Amdocs.Ginger.CoreNET.Execution.eRunStatus.Failed;
+                    return;
+                }
+                act.ExecuteActiveZapScan(testURL);
+            }
+            else
+            {
+                // Intentionally unscoped to aggregate alerts across the entire ZAP session
+                act.ExecutePassiveZapScan("", act);
+            }
+        }

Ginger/Ginger/ExternalConfigurations/ZAPConfigurationPage.xaml.cs (5)

61-66: Fix required-field validation: wrong control and message; add URL+API key checks

The required-field rule is attached to the API key textbox but the message refers to the URL. Attach the correct rules to the correct controls and add API key validation too.

 private void ApplyValidationRules()
 {
-    // check if fields have been populated (font-end validation)
-    xZAPAPIkeyTextBox.ValueTextBox.AddValidationRule(new ValidateEmptyValue("ZAP URL cannot be empty"));
+    // basic front-end validation
+    xZAPURLTextBox.ValueTextBox.AddValidationRule(new ValidateEmptyValue("ZAP URL cannot be empty"));
+    xZAPAPIkeyTextBox.ValueTextBox.AddValidationRule(new ValidateEmptyValue("ZAP API Key cannot be empty"));
 }

---

86-90: Treat whitespace-only input as empty

Use IsNullOrWhiteSpace so inputs with only spaces fail validation.

- return string.IsNullOrEmpty(zAPConfiguration.ZAPUrl)
-     || string.IsNullOrEmpty(zAPConfiguration.ZAPApiKey);
+ return string.IsNullOrWhiteSpace(zAPConfiguration.ZAPUrl)
+     || string.IsNullOrWhiteSpace(zAPConfiguration.ZAPApiKey);

---

107-121: Avoid unnecessary dispatcher marshaling and use Collapsed for hiding

Show/Hide run on UI thread callers; always invoking Dispatcher is redundant. Also prefer Visibility.Collapsed for layout.

- public void HideLoader()
- {
-     this.Dispatcher.Invoke(() =>
-     {
-         xProcessingImage.Visibility = Visibility.Hidden;
-     });
- }
+ public void HideLoader()
+ {
+     Action setHidden = () => xProcessingImage.Visibility = Visibility.Collapsed;
+     if (Dispatcher.CheckAccess()) setHidden();
+     else Dispatcher.Invoke(setHidden);
+ }
 
- public void ShowLoader()
- {
-     this.Dispatcher.Invoke(() =>
-     {
-         xProcessingImage.Visibility = Visibility.Visible;
-     });
- }
+ public void ShowLoader()
+ {
+     Action setVisible = () => xProcessingImage.Visibility = Visibility.Visible;
+     if (Dispatcher.CheckAccess()) setVisible();
+     else Dispatcher.Invoke(setVisible);
+ }

---

123-129: Trim before encrypting to avoid persisting accidental whitespace

Minor UX hardening; use trimmed input for VE/encryption checks.

- if (!EncryptionHandler.IsStringEncrypted(xZAPAPIkeyTextBox.ValueTextBox.Text))
+ var raw = xZAPAPIkeyTextBox.ValueTextBox.Text?.Trim();
+ if (!EncryptionHandler.IsStringEncrypted(raw))
 {
-    xZAPAPIkeyTextBox.ValueTextBox.Text = ValueExpression.IsThisAValueExpression(xZAPAPIkeyTextBox.ValueTextBox.Text) ? xZAPAPIkeyTextBox.ValueTextBox.Text : EncryptionHandler.EncryptwithKey(xZAPAPIkeyTextBox.ValueTextBox.Text);
+    xZAPAPIkeyTextBox.ValueTextBox.Text =
+        ValueExpression.IsThisAValueExpression(raw) ? raw : EncryptionHandler.EncryptwithKey(raw);
 }

---

131-164: Harden HTTP: reuse HttpClient, URL normalize, URL-encode apiKey, and faster timeout

Prevents socket exhaustion, handles stray spaces/trailing slash, and avoids breaking on special characters in apiKey.

- private async Task<bool> TestZAPConnectionAsync(string zapUrl, string apiKey)
+ private async Task<bool> TestZAPConnectionAsync(string zapUrl, string apiKey)
 {
     try
     {
-        // Ensure URL does not end with a slash
-        if (zapUrl.EndsWith('/'))
-        {
-            zapUrl = zapUrl.TrimEnd('/');
-        }
-
-        string testUrl = $"{zapUrl}/JSON/core/view/version/?apikey={apiKey}";
-
-        using HttpClient client = new HttpClient();
-        var response = await client.GetAsync(testUrl);
+        zapUrl = (zapUrl ?? string.Empty).Trim().TrimEnd('/');
+        var encodedKey = Uri.EscapeDataString(apiKey ?? string.Empty);
+        string testUrl = $"{zapUrl}/JSON/core/view/version/?apikey={encodedKey}";
+        var response = await s_http.GetAsync(testUrl);
 
         if (!response.IsSuccessStatusCode)
         {
             Reporter.ToLog(eLogLevel.ERROR, $"ZAP connection failed. Status code: {response.StatusCode}");
             return false;
         }
 
         string content = await response.Content.ReadAsStringAsync();
 
         // Optionally, check if content contains "version" field
         bool hasVersion = content.Contains("version");
         Reporter.ToLog(eLogLevel.DEBUG, $"ZAP response contains 'version': {hasVersion}");
         return hasVersion;
     }
     catch (Exception ex)
     {
         Reporter.ToLog(eLogLevel.ERROR, $"Exception during ZAP connection test: {ex.Message}", ex);
         return false;
     }
 }

Add this static client near the top of the class:

private static readonly HttpClient s_http = new HttpClient { Timeout = TimeSpan.FromSeconds(10) };

Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs (7)

50-67: Method marked async but blocks thread; make it synchronous or truly async

Current implementation has no awaits and uses Thread.Sleep. Either remove async or implement proper async with Task.Delay + CancellationToken. Given call sites are synchronous, prefer a synchronous API.

Option A — make it synchronous (minimal change):

-public async Task WaitTillPassiveScanCompleted()
+public void WaitTillPassiveScanCompleted()
 {
     try
     {
         IApiResponse apiResponse = _zapClient.pscan.recordsToScan();
         string tempVal = ((ApiResponseElement)apiResponse).Value;
         while (!tempVal.Equals("0"))
         {
-            Thread.Sleep(1000);
+            Thread.Sleep(1000);
             apiResponse = _zapClient.pscan.recordsToScan();
             tempVal = ((ApiResponseElement)apiResponse).Value;
         }
     }
     catch (Exception ex)
     {
         throw new InvalidOperationException("Error during passive scan: " + ex.Message, ex);
     }
 }

Option B — true async (requires updating all callers):

-public async Task WaitTillPassiveScanCompleted()
+public async Task WaitTillPassiveScanCompletedAsync(CancellationToken ct = default)
 {
     try
     {
         IApiResponse apiResponse = _zapClient.pscan.recordsToScan();
         string tempVal = ((ApiResponseElement)apiResponse).Value;
         while (!tempVal.Equals("0"))
         {
-            Thread.Sleep(1000);
+            await Task.Delay(1000, ct);
             apiResponse = _zapClient.pscan.recordsToScan();
             tempVal = ((ApiResponseElement)apiResponse).Value;
         }
     }
     catch (Exception ex)
     {
         throw new InvalidOperationException("Error during passive scan: " + ex.Message, ex);
     }
 }

---

69-83: Report generation: specify file extension and validate inputs

Passing an empty extension may save files without .html. Provide "html". Consider guarding null/empty args.

 _zapClient.reports.generate(
-    "Security Test Report", "traditional-html", "", "Security scan report", "", siteToTest, "", "", "",
-    reportfilename, "", reportDir, ""
+    "Security Test Report", "traditional-html", "", "Security scan report",
+    "", siteToTest, "", "", "",
+    reportfilename, "html", reportDir, ""
 );

---

85-131: Doc fix and safer parsing: avoid “as string”; default to ZAP port when missing

Comments say “as string” but return type is int?. Also handle missing port by returning 8080 only when the caller requests default; since the constructor uses GetPortFromUrl(..) ?? 8080, current change to docs suffices.

-/// <returns>The port number as string, or null if not found.</returns>
+/// <returns>The port number, or null if not found.</returns>

Optional robustness for non-URL host:port:

- var parts = url.Split(':');
+ var parts = url.Trim().Split(':');

---

133-176: Host parsing docs and IPv6 handling nits

Align XML docs with return type and improve trivial trimming.

-/// <returns>The host as string, or null if not found.</returns>
+/// <returns>The host name, or null if not found.</returns>
...
- var parts = url.Split(':');
+ var parts = url.Trim().Split(':');

---

239-253: Active scan loop: include percentage and consider async later

At minimum, log percentage to aid troubleshooting. True async remains recommended for UI/throughput.

- Reporter.ToLog(eLogLevel.INFO, "Active scan is in progress");
+ Reporter.ToLog(eLogLevel.INFO, $"Active scan is in progress: {status}%");

---

262-284: EvaluateScanResult: handle null allowed list and ignore zero-count entries

Avoids false failures and NREs.

- if (!allowedAlertNames.Any(a => a.Value == alertName))
- {
-     testPassed = false;
-}
+ if (count <= 0)
+ {
+     continue;
+ }
+ if (allowedAlertNames == null || !allowedAlertNames.Any(a => a.Value == alertName))
+ {
+     testPassed = false;
+ }

---

286-292: EvaluateScanResult (no allowed list): ToString check is unreliable

Parse counts and only pass if total is zero.

- var alertSummary = (ApiResponseSet)summaryResponse;
- return !string.IsNullOrEmpty(alertSummary.ToString());
+ var alertSummary = (ApiResponseSet)summaryResponse;
+ foreach (var kv in alertSummary.Dictionary)
+ {
+     var val = kv.Value is ApiResponseElement el ? el.Value : kv.Value?.ToString();
+     if (int.TryParse(val, out int count) && count > 0)
+     {
+         return false;
+     }
+ }
+ return true;

Ginger/GingerCoreNET/ActionsLib/UI/Web/ActSecurityTesting.cs (5)

269-312: Passive path: null-safe EvaluateScanResult and align with session-wide passive scan

When aggregating passive alerts, pass empty base URL to get the entire session summary (per team guidance). Also, the current call can NRE if AlertList is null.

- zapProxyService.WaitTillPassiveScanCompleted();
-
- ProcessResultAsync(zapProxyService, act, testURL);
- AddZapAlertOutputValues(zapProxyService, act, testURL);
- isPassed = zapProxyService.EvaluateScanResult(testURL, AlertList);
+ zapProxyService.WaitTillPassiveScanCompleted();
+ ProcessResult(zapProxyService, act, string.Empty);
+ AddZapAlertOutputValues(zapProxyService, act, string.Empty);
+ isPassed = AlertList != null
+     ? zapProxyService.EvaluateScanResult(string.Empty, AlertList)
+     : zapProxyService.EvaluateScanResult(string.Empty);

Note: This follows the learning that passive scans should use an empty testURL to aggregate alerts across the ZAP session.

---

369-388: Method is synchronous; rename accordingly and use TestArtifactsFolder; fix timestamp var

Name suggests async but there’s no async work. Also use a consistent artifacts root and a standard timestamp.

- private void ProcessResultAsync(ZapProxyService zps, Act _act, string testURL)
+ private void ProcessResult(ZapProxyService zps, Act _act, string testURL)
 {
-
-    string path = String.Empty;
-
-    string solutionDir = WorkSpace.Instance.Solution.Folder;
-    string folderPath = Path.Combine(solutionDir, "ExecutionResults", "Artifacts", "ZAPReport");
+    string path = string.Empty;
+    string baseArtifacts = WorkSpace.Instance.TestArtifactsFolder ?? WorkSpace.Instance.Solution.Folder;
+    string folderPath = Path.Combine(baseArtifacts, "ExecutionResults", "Artifacts", "ZAPReport");
     // Ensure the directory exists
     if (!System.IO.Directory.Exists(folderPath))
     {
         System.IO.Directory.CreateDirectory(folderPath);
     }
-    string DatetimeFormate = DateTime.Now.ToString("ddMMyyyy_HHmmssfff");
-    string reportname = $"{_act.ItemName}_SecurityTestingReport_{DatetimeFormate}.html";
+    string timestamp = DateTime.Now.ToString("yyyyMMdd_HHmmssfff");
+    string reportname = $"{_act.ItemName}_SecurityTestingReport_{timestamp}.html";
     path = Path.Combine(folderPath, reportname);
 
     zps.GenerateZapReport(testURL, folderPath, reportname);
     Act.AddArtifactToAction(Path.GetFileName(path), _act, path);
     _act.AddOrUpdateReturnParamActual(ParamName: "Security Testing Report", ActualValue: path);
 }

Update call sites at Lines 222, 281, 329 accordingly (see related diffs).

---

390-409: Output values: don’t overload Path as totals; add a separate “Total Vulnerabilities” param

Keeps semantics clear and makes totals easier to consume in reports.

- act.AddNewReturnParams = true;
-
- act.AddOrUpdateReturnParamActualWithPath(
-     "Vulnerabilities Summary",
-     vulnerabilitiesSummary,
-     totalAlerts.ToString()
- );
+ act.AddNewReturnParams = true;
+ act.AddOrUpdateReturnParamActual("Vulnerabilities Summary", vulnerabilitiesSummary);
+ act.AddOrUpdateReturnParamActual("Total Vulnerabilities", totalAlerts.ToString());

---

115-188: Declared properties (ScanType, PullReport, AlertItems) aren’t used in execution paths

Either wire these into flows (e.g., respect ScanType and PullReport) or remove to avoid confusion and dead state.

Example wiring (high level):

• Use ScanType to pick between ExecuteActiveZapScan vs ExecutePassiveZapScan in a single Execute method.
• Use PullReport to skip ProcessResult when false.

---

192-208: Lazy loading of ZAPConfiguration: consider passing to ZapProxyService for clarity

Service currently fetches from repository again; pass the selected configuration or pre-resolved URL/apiKey for testability and fewer implicit dependencies.

- ZapProxyService zapProxyService = new ZapProxyService();
+ ZapProxyService zapProxyService = new ZapProxyService();
+ // Optional: if service adds an overload accepting host/port/apiKey, derive them here from ZAPConfiguration