Ginger/GingerCoreNET/Run/RunListenerLib/CenteralizedExecutionLogger/AccountReportApiHandler.cs (2)

273-299: Validation should fail closed: don’t return true on request failure.

Current logic returns true when the call errors, which could allow invalid execution IDs.

Use strict parse and fail closed:

-                    RestResponse response = restClient.Execute(restRequest);
-                    if (response.IsSuccessful)
-                    {
-                        Reporter.ToLog(eLogLevel.DEBUG, $"Successfully validated execution id {message}");
-                        return Convert.ToBoolean(response.Content);
-                    }
-                    else
-                    {
-                        Reporter.ToLog(eLogLevel.ERROR, $"Failed to validate {message} Response: {response.Content}");
-                    }
+                    RestResponse response = restClient.Execute(restRequest);
+                    if (response.IsSuccessful && bool.TryParse(response.Content, out var isValid))
+                    {
+                        Reporter.ToLog(eLogLevel.DEBUG, $"Successfully validated execution id {message}");
+                        return isValid;
+                    }
+                    Reporter.ToLog(eLogLevel.ERROR, $"Failed to validate {message} Response: {response.Content}");
                 }
                 catch (Exception ex)
                 {
                     Reporter.ToLog(eLogLevel.ERROR, $"Exception while validating execution id {message}", ex);
                 }
-                return true;
+                return false;

---

65-71: TLS disabled: RemoteCertificateValidationCallback always returns true.

This bypasses HTTPS validation and is a security risk. Only allow in dev via config or remove override.

Example hardening:

-                        RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true,
+                        // Use default validation; if you must allow insecure for dev, gate it behind a config flag.
+                        // RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => sslPolicyErrors == System.Net.Security.SslPolicyErrors.None,

Ginger/GingerCoreNET/ActionsLib/UI/VisualTesting/MagickAnalyzer.cs (2)

42-56: Dispose MagickImage/diff and avoid use-after-dispose on Bitmap.

• MagickImage and the IMagickImage returned from Compare are IDisposable; not disposing them leaks native memory.
• Inside the using block, ImgToSave is disposed, yet you assign and pass it (mAct.CompareResult, AddScreenShot). That creates a disposed reference risk.

Apply the refactor below to scope-dispose Magick images/diff and hand off live Bitmap instances to mAct:

-            MagickImage magickBaseImg = new MagickImage(General.ImageToByteArray(mAct.baseImage, System.Drawing.Imaging.ImageFormat.Bmp));//Not tested after code change
-            MagickImage magickTargetImg = new MagickImage(General.ImageToByteArray(mAct.targetImage, System.Drawing.Imaging.ImageFormat.Bmp));//Not tested after code change
-
-            double percentageDifference;
+            using (var magickBaseImg = new MagickImage(General.ImageToByteArray(mAct.baseImage, System.Drawing.Imaging.ImageFormat.Bmp))) // Not tested after code change
+            using (var magickTargetImg = new MagickImage(General.ImageToByteArray(mAct.targetImage, System.Drawing.Imaging.ImageFormat.Bmp))) // Not tested after code change
+            {
+                double percentageDifference;

-            ErrorMetric eErrorMetric = ErrorMetric.Fuzz;
-            Enum.TryParse<ErrorMetric>(mAct.GetOrCreateInputParam(ActVisualTesting.Fields.ErrorMetric).Value, out eErrorMetric);
-            IMagickImage diffImg = magickBaseImg.Compare(magickTargetImg, eErrorMetric, Channels.Red, out percentageDifference);
-            percentageDifference = percentageDifference * 100;
-            percentageDifference = Math.Round(percentageDifference, 2);
+                ErrorMetric eErrorMetric = ErrorMetric.Fuzz;
+                Enum.TryParse<ErrorMetric>(mAct.GetOrCreateInputParam(ActVisualTesting.Fields.ErrorMetric).Value, true, out eErrorMetric);
+                using (IMagickImage diffImg = magickBaseImg.Compare(magickTargetImg, eErrorMetric, Channels.Red, out percentageDifference))
+                {
+                    percentageDifference = Math.Round(percentageDifference * 100, 2);

-            TypeConverter tc = TypeDescriptor.GetConverter(typeof(Bitmap));
-            using (Bitmap ImgToSave = (Bitmap)tc.ConvertFrom(diffImg.ToByteArray()))
-            {
-                mAct.CompareResult = ImgToSave;//Not tested after code change
-
-                mAct.AddOrUpdateReturnParamActual("Percentage Difference", percentageDifference + "");
-
-                mAct.AddScreenShot(ImgToSave, "Compare Result");
-            }
+                    TypeConverter tc = TypeDescriptor.GetConverter(typeof(Bitmap));
+                    using (Bitmap imgToSave = (Bitmap)tc.ConvertFrom(diffImg.ToByteArray(MagickFormat.Png)))
+                    {
+                        // Hand off owned clones to mAct; keep local disposed.
+                        mAct.CompareResult = (Bitmap)imgToSave.Clone(); // Not tested after code change
+                        mAct.AddOrUpdateReturnParamActual("Percentage Difference", percentageDifference.ToString(System.Globalization.CultureInfo.InvariantCulture));
+                        mAct.AddScreenShot((Bitmap)imgToSave.Clone(), "Compare Result");
+                    }
+                }
+            }

---

55-62: Persist a live Bitmap; don’t assign a disposed instance.

If CompareResult or AddScreenShot retain the reference, the current using block disposes it prematurely.

Covered by the refactor above (clone before leaving the using block).

Ginger/GingerTest/GeneralLib/VisualCompare.cs (3)

107-121: Dispose MagickImage/diff and the Bitmap; specify a stable output format.

Avoid native leaks and ensure converter recognizes bytes by specifying MagickFormat.

-            MagickImage magickBaseImg = new MagickImage(BaseImageFileName);
-            MagickImage magickTargetImg = new MagickImage(TargetImageFileName);
+            using (var magickBaseImg = new MagickImage(BaseImageFileName))
+            using (var magickTargetImg = new MagickImage(TargetImageFileName))
+            {

-            double percentageDifference;
+            double percentageDifference;

             ErrorMetric EM = ErrorMetric.Fuzz;   // Fuzz !?
-            IMagickImage diffImg = magickBaseImg.Compare(magickTargetImg, EM, Channels.Red, out percentageDifference);
-            percentageDifference = percentageDifference * 100;
-            percentageDifference = Math.Round(percentageDifference, 2);
+            using (IMagickImage diffImg = magickBaseImg.Compare(magickTargetImg, EM, Channels.Red, out percentageDifference))
+            {
+                percentageDifference = Math.Round(percentageDifference * 100, 2);

-            TypeConverter tc = TypeDescriptor.GetConverter(typeof(Bitmap));
-            Bitmap ImgToSave = (Bitmap)tc.ConvertFrom(diffImg.ToByteArray());
-            ImgToSave.Save(ResultFileName);
+                TypeConverter tc = TypeDescriptor.GetConverter(typeof(Bitmap));
+                using (Bitmap imgToSave = (Bitmap)tc.ConvertFrom(diffImg.ToByteArray(MagickFormat.Png)))
+                {
+                    imgToSave.Save(ResultFileName);
+                }
+            }
+            }

---

122-129: Simplify boolean return.

Minor readability tweak.

-            if (percentageDifference == 0)
-            {
-                return true;
-            }
-            else
-            {
-                return false;
-            }
+            return percentageDifference == 0;

---

72-73: Typo in public method names (“Equel”). Consider adding correctly named wrappers.

Leaving old methods for back-compat; add new IsVisualEqual/IsBitmapEqual and mark old as [Obsolete].

I can draft the wrappers if you want this in the same PR.

Also applies to: 105-106

Ginger/GingerCoreNET/Run/RunListenerLib/CenteralizedExecutionLogger/AccountReportApiHandler.cs (1)

281-306: ExecutionIdValidation returns true on error paths — this defeats validation

On network error or non-success HTTP status, the method logs an error but returns true. That can allow invalid executions to pass checks and cascade failures later.

Apply:

@@ public bool ExecutionIdValidation(Guid executionId)
-                try
+                try
                 {
                     RestResponse response = restClient.Execute(restRequest);
                     if (response.IsSuccessful)
                     {
                         Reporter.ToLog(eLogLevel.DEBUG, $"Successfully validated execution id {message}");
                         return Convert.ToBoolean(response.Content);
                     }
                     else
                     {
                         Reporter.ToLog(eLogLevel.ERROR, $"Failed to validate {message} Response: {response.Content}");
-                    }
+                        return false;
+                    }
                 }
                 catch (Exception ex)
                 {
                     Reporter.ToLog(eLogLevel.ERROR, $"Exception while validating execution id {message}", ex);
-                }
-                return true;
+                    return false;
+                }

Ginger/GingerCoreNET/ActionsLib/UI/VisualTesting/MagickAnalyzer.cs (2)

42-63: Dispose Magick.NET images and avoid assigning a disposed Bitmap (plus safer parsing and culture-stable output).

Unmanaged resources are leaked (MagickImage/IMagickImage not disposed). Also, mAct.CompareResult is set to a Bitmap that is disposed at the end of the using block, risking later ObjectDisposedException. Patch also trims/null-guards enum parsing and emits the percentage using invariant culture.

Apply this diff:

-            MagickImage magickBaseImg = new MagickImage(General.ImageToByteArray(mAct.baseImage, System.Drawing.Imaging.ImageFormat.Bmp));//Not tested after code change
-            MagickImage magickTargetImg = new MagickImage(General.ImageToByteArray(mAct.targetImage, System.Drawing.Imaging.ImageFormat.Bmp));//Not tested after code change
-
-            double percentageDifference;
-
-            ErrorMetric eErrorMetric = ErrorMetric.Fuzz;
-            Enum.TryParse<ErrorMetric>(mAct.GetOrCreateInputParam(ActVisualTesting.Fields.ErrorMetric).Value, true, out eErrorMetric);
-
-            IMagickImage diffImg = magickBaseImg.Compare(magickTargetImg, eErrorMetric, Channels.Red, out percentageDifference);
-            percentageDifference = percentageDifference * 100;
-            percentageDifference = Math.Round(percentageDifference, 2);
-
-            TypeConverter tc = TypeDescriptor.GetConverter(typeof(Bitmap));
-            using (Bitmap ImgToSave = (Bitmap)tc.ConvertFrom(diffImg.ToByteArray()))
-            {
-                mAct.CompareResult = ImgToSave;//Not tested after code change
-
-                mAct.AddOrUpdateReturnParamActual("Percentage Difference", percentageDifference + "");
-
-                mAct.AddScreenShot(ImgToSave, "Compare Result");
-            }
+            using (MagickImage magickBaseImg = new MagickImage(General.ImageToByteArray(mAct.baseImage, System.Drawing.Imaging.ImageFormat.Bmp)))
+            using (MagickImage magickTargetImg = new MagickImage(General.ImageToByteArray(mAct.targetImage, System.Drawing.Imaging.ImageFormat.Bmp)))
+            {
+                double percentageDifference;
+                ErrorMetric eErrorMetric = ErrorMetric.Fuzz;
+                Enum.TryParse<ErrorMetric>(mAct.GetOrCreateInputParam(ActVisualTesting.Fields.ErrorMetric).Value?.Trim() ?? string.Empty, true, out eErrorMetric);
+
+                using (IMagickImage diffImg = magickBaseImg.Compare(magickTargetImg, eErrorMetric, Channels.Red, out percentageDifference))
+                {
+                    // Ensure a decoder-friendly format for GDI+
+                    diffImg.Format = MagickFormat.Png;
+
+                    percentageDifference = Math.Round(percentageDifference * 100, 2);
+
+                    TypeConverter tc = TypeDescriptor.GetConverter(typeof(Bitmap));
+                    using (Bitmap resultBmp = (Bitmap)tc.ConvertFrom(diffImg.ToByteArray()))
+                    {
+                        // Keep an owned copy beyond this scope
+                        mAct.CompareResult = (Bitmap)resultBmp.Clone();
+                        mAct.AddOrUpdateReturnParamActual("Percentage Difference", percentageDifference.ToString(System.Globalization.CultureInfo.InvariantCulture));
+                        mAct.AddScreenShot(resultBmp, "Compare Result");
+                    }
+                }
+            }

---

19-24: Add missing import for CultureInfo.

Required by the invariant ToString used in the diff above.

 using ImageMagick;
 using System;
+using System.Globalization;
 using System.ComponentModel;
 using System.Drawing;

Ginger/GingerCoreNET/Run/RunListenerLib/CenteralizedExecutionLogger/AccountReportApiHandler.cs (2)

326-336: Avoid C# 12 collection expressions unless LangVersion ≥ 12.

Use explicit List for broader compatibility.

Apply:

-                        List<string> temp = [];
+                        var temp = new List<string>();
...
-                                temp = [];
+                                temp = new List<string>();

---

401-413: Avoid C# 12 collection expressions unless LangVersion ≥ 12.

Use explicit List for compatibility.

Apply:

-                        List<ArtifactDetails> temp = [];
+                        var temp = new List<ArtifactDetails>();
...
-                                    temp = [];
+                                    temp = new List<ArtifactDetails>();