Unit Test Cases Added for ZAP#4290
Merged
Merged
Conversation
Contributor
WalkthroughAdds a new public Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant Service as ZapProxyService
participant Client as IZapClient (ZapClientWrapper)
Note over Caller,Service: Start active scan → poll → complete
Caller->>Service: PerformActiveScan(url,...)
Service->>Client: Scan(url, recurse, inscopeonly, policy, method, postdata, contextId)
Client-->>Service: scanId
loop poll until status == "100"
Service->>Client: ScanStatus(scanId)
Client-->>Service: status
end
Service-->>Caller: scanId (completed)
sequenceDiagram
autonumber
actor Caller
participant Service as ZapProxyService
participant Client as IZapClient (ZapClientWrapper)
Note over Caller,Service: Retrieve alerts summary for evaluation
Caller->>Service: GetAlertSummary(targetUrl)
Service->>Client: AlertsSummary(targetUrl)
Client-->>Service: IApiResponse (summary)
Service-->>Caller: parsed alert summary / evaluation result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs (5)
67-79: Async method uses Thread.Sleep and has no await — fixes needed (also removes build warning)Replace Thread.Sleep with await Task.Delay to avoid blocking and eliminate CS1998. Consider a timeout to prevent indefinite waits.
- public async Task WaitTillPassiveScanCompleted() + public async Task WaitTillPassiveScanCompleted() { try { IApiResponse apiResponse = _zapClient.RecordsToScan(); string tempVal = ((ApiResponseElement)apiResponse).Value; while (!tempVal.Equals("0")) { - Thread.Sleep(1000); + await Task.Delay(1000).ConfigureAwait(false); apiResponse = _zapClient.RecordsToScan(); tempVal = ((ApiResponseElement)apiResponse).Value; } }Optional: add a cancellation token/timeout in a follow-up.
200-219: Method contradicts its XML comment; exceptions are swallowedThe summary says it throws when the URL isn't added, but the catch logs and returns. This can lead to ActiveScan continuing on a bad setup.
- public void AddUrlToScanTree(string siteToTest) + public void AddUrlToScanTree(string siteToTest) { try { _zapClient.AccessUrl(siteToTest, "false"); var urls = GetUrlsFromScanTree(siteToTest); if (urls.Contains(siteToTest)) { Reporter.ToLog(eLogLevel.INFO, $"{siteToTest} has been added to scan tree"); } else { - throw new InvalidOperationException($"{siteToTest} not added to scan tree, active scan will not be possible"); + throw new InvalidOperationException($"{siteToTest} not added to scan tree, active scan will not be possible"); } } catch (Exception ex) { - Reporter.ToLog(eLogLevel.ERROR, $"Failed to add {siteToTest} to scan tree: {ex.Message}", ex); + Reporter.ToLog(eLogLevel.ERROR, $"Failed to add {siteToTest} to scan tree: {ex.Message}", ex); + throw; } }If you prefer not to throw, change the summary and return a bool indicating success.
279-309: EvaluateScanResultWeb should only fail on disallowed alerts with count > 0; also guard null listCurrently it fails even when count == 0 and will NRE if allowedAlertNames is null.
- foreach (var alertEntry in alertSummary.Dictionary) + foreach (var alertEntry in alertSummary.Dictionary) { string alertName = alertEntry.Key; string valueString = alertEntry.Value is ApiResponseElement element ? element.Value : alertEntry.Value?.ToString(); - if (!int.TryParse(valueString, out int count)) + if (!int.TryParse(valueString, out int count)) { continue; } - if (!allowedAlertNames.Any(a => a.Value == alertName)) + if (count > 0 && !(allowedAlertNames?.Any(a => a.Value == alertName) ?? false)) { testPassed = false; } }(Used retrieved learnings: keep alert names as-is; case sensitivity rules unchanged here.)
102-108: XML doc says “string” but method returns int?Fix the XML comment to match the signature and behavior.
- /// <returns>The port number as string, or null if not found.</returns> + /// <returns>The port number as int?, or null if not found.</returns>
256-270: Optional: make active-scan wait non-blocking and include progressSwap Thread.Sleep for Task.Delay and log the numeric percentage to reduce log noise and allow responsiveness.
If you choose to keep it sync, at least log the current percentage:
- Reporter.ToLog(eLogLevel.INFO, "Active scan is in progress"); + Reporter.ToLog(eLogLevel.INFO, $"Active scan is in progress: {status}%");
📜 Review details
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.
📒 Files selected for processing (4)
Ginger/GingerCoreNET/External/ZAP/IZapClient.cs(1 hunks)Ginger/GingerCoreNET/External/ZAP/ZapClientWrapper.cs(1 hunks)Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs(12 hunks)Ginger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-08-25T09:20:17.608Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4281
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:1958-1970
Timestamp: 2025-08-25T09:20:17.608Z
Learning: In Ginger security testing, passive ZAP scans should be invoked with an empty testURL (ActSecurityTesting.ExecutePassiveZapScan("", act)) to aggregate alerts across the entire ZAP session. Passing a URL scopes results to that base URL. Only the active scan path should pass Driver.Url, with null/empty checks. Context: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs :: ActSecurity. Confirmed by AmanPrasad43 in PR #4281.
Applied to files:
Ginger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.csGinger/GingerCoreNET/External/ZAP/IZapClient.csGinger/GingerCoreNET/External/ZAP/ZapProxyService.cs
📚 Learning: 2025-08-25T08:48:11.915Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4281
File: Ginger/GingerCore/GingerCore.csproj:538-538
Timestamp: 2025-08-25T08:48:11.915Z
Learning: In the Ginger codebase, the OWASPZAPDotNetAPI package reference in GingerCore.csproj is required for architectural reasons, even when direct usage is not immediately visible in the codebase. This was confirmed by AmanPrasad43 during the security testing feature implementation.
Applied to files:
Ginger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.csGinger/GingerCoreNET/External/ZAP/ZapClientWrapper.csGinger/GingerCoreNET/External/ZAP/IZapClient.csGinger/GingerCoreNET/External/ZAP/ZapProxyService.cs
📚 Learning: 2025-08-28T09:27:18.793Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4286
File: Ginger/Ginger/Drivers/DriversConfigsEditPages/WebAgentConfigEditPage.xaml.cs:556-571
Timestamp: 2025-08-28T09:27:18.793Z
Learning: In Ginger's WebAgentConfigEditPage.xaml.cs ZAP configuration, when ZAP Security Testing is enabled, the manual proxy UI should remain enabled/accessible even though ZAP will override the proxy settings in the background. This allows users to see and modify their proxy configurations for transparency and future use when ZAP is disabled. Confirmed by AmanPrasad43 in PR #4286.
Applied to files:
Ginger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.csGinger/GingerCoreNET/External/ZAP/ZapProxyService.cs
📚 Learning: 2025-06-16T10:37:13.073Z
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.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs
📚 Learning: 2025-08-25T09:30:08.106Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4281
File: Ginger/GingerCoreCommon/External/Configurations/ZAPConfiguration.cs:37-58
Timestamp: 2025-08-25T09:30:08.106Z
Learning: In the Ginger codebase, URL validation in configuration classes like ZAPConfiguration should not throw exceptions for invalid URLs. The team prefers simple normalization (like trimming trailing slashes) over strict validation with exception throwing. This was confirmed by AmanPrasad43 in PR #4281 for the ZAPConfiguration.ZAPUrl property setter.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs
📚 Learning: 2025-04-25T13:29:45.059Z
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.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs
📚 Learning: 2025-08-29T09:35:45.994Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4286
File: Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs:287-309
Timestamp: 2025-08-29T09:35:45.994Z
Learning: In Ginger ZAP security testing, alert names should preserve their original formatting with spaces and hyphens. When comparing alert names in EvaluateScanResultAPI and similar methods, use StringComparison.OrdinalIgnoreCase for case-insensitive matching but do not normalize by removing spaces or hyphens, as alert names like "False Positive" need to remain as separate words. Confirmed by AmanPrasad43 in PR #4286.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs
🧬 Code graph analysis (2)
Ginger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs (2)
Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs (13)
ZapProxyService(16-369)ZapProxyService(27-39)ZapProxyService(42-45)GetPortFromUrl(108-148)GetHostFromUrl(155-193)IsZapRunning(52-65)GenerateZapReport(86-100)List(225-230)List(351-367)AddUrlToScanTree(200-219)PerformActiveScan(236-250)EvaluateScanResultWeb(279-309)EvaluateScanResultAPI(311-341)Ginger/GingerCoreNET/External/ZAP/ZapClientWrapper.cs (8)
IApiResponse(14-14)IApiResponse(15-15)IApiResponse(17-17)IApiResponse(18-18)IApiResponse(19-21)IApiResponse(23-23)IApiResponse(24-24)IApiResponse(26-31)
Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs (2)
Ginger/GingerCoreCommon/External/Configurations/ZAPConfiguration.cs (1)
ZAPConfiguration(5-71)Ginger/GingerCoreNET/External/ZAP/ZapClientWrapper.cs (10)
ZapClientWrapper(5-32)ZapClientWrapper(9-12)IApiResponse(14-14)IApiResponse(15-15)IApiResponse(17-17)IApiResponse(18-18)IApiResponse(19-21)IApiResponse(23-23)IApiResponse(24-24)IApiResponse(26-31)
🔇 Additional comments (4)
Ginger/GingerCoreNET/External/ZAP/ZapClientWrapper.cs (4)
14-16: LGTM: simple pass-throughs for version/pscan are correct
23-24: LGTM: status and alerts summary mapping looks correct
7-12: Dispose pattern (if ClientApi is disposable)
If ClientApi implements IDisposable, wrap the wrapper with IDisposable and dispose it. If not, ignore.Would you like me to confirm the type’s disposability and generate the minimal implementation?
1-1: Package reference present and consistent
GingerCoreNET.csproj includes<PackageReference Include="OWASPZAPDotNetAPI" Version="2.12.0" />, matching GingerCore.csproj—no changes needed.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
♻️ Duplicate comments (1)
Ginger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs (1)
138-151: Skip: polling verification already discussed earlier in the PRPrior review suggested verifying ScanStatus polling; author confirmed coverage via SetupSequence. No further action.
📜 Review details
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.
📒 Files selected for processing (2)
Ginger/GingerCoreNET/External/ZAP/ZapClientWrapper.cs(1 hunks)Ginger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs(1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4281
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:1958-1970
Timestamp: 2025-08-25T09:20:17.608Z
Learning: In Ginger security testing, passive ZAP scans should be invoked with an empty testURL (ActSecurityTesting.ExecutePassiveZapScan("", act)) to aggregate alerts across the entire ZAP session. Passing a URL scopes results to that base URL. Only the active scan path should pass Driver.Url, with null/empty checks. Context: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs :: ActSecurity. Confirmed by AmanPrasad43 in PR #4281.
📚 Learning: 2025-09-03T17:35:32.098Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4290
File: Ginger/GingerCoreNET/External/ZAP/ZapClientWrapper.cs:17-21
Timestamp: 2025-09-03T17:35:32.098Z
Learning: In the Ginger ZAP integration, the OWASPZAPDotNetAPI library requires string values ("true"/"false") for boolean parameters in methods like AccessUrl, Scan, etc. The ZapClientWrapper should maintain this string-based interface to stay consistent with the underlying ZAP library's API requirements rather than adding boolean overloads.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapClientWrapper.cs
📚 Learning: 2025-08-25T08:48:11.915Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4281
File: Ginger/GingerCore/GingerCore.csproj:538-538
Timestamp: 2025-08-25T08:48:11.915Z
Learning: In the Ginger codebase, the OWASPZAPDotNetAPI package reference in GingerCore.csproj is required for architectural reasons, even when direct usage is not immediately visible in the codebase. This was confirmed by AmanPrasad43 during the security testing feature implementation.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapClientWrapper.csGinger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs
📚 Learning: 2025-09-03T17:37:45.916Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4290
File: Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs:56-59
Timestamp: 2025-09-03T17:37:45.916Z
Learning: In the Ginger ZAP integration, the ZAP library's version API returns a string value, which is why the code uses ToString() to convert the IApiResponse to string format in the IsZapRunning method in ZapProxyService.cs.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapClientWrapper.csGinger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs
📚 Learning: 2025-09-03T17:37:09.236Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4290
File: Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs:34-39
Timestamp: 2025-09-03T17:37:09.236Z
Learning: In Ginger's ZAP integration, when GetPortFromUrl() returns null (URL without port), avoid using hardcoded fallback ports as they may conflict with other applications. Instead, require explicit port specification in the ZAP configuration and provide clear error messages when port is missing.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapClientWrapper.csGinger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs
📚 Learning: 2025-08-25T09:30:08.106Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4281
File: Ginger/GingerCoreCommon/External/Configurations/ZAPConfiguration.cs:37-58
Timestamp: 2025-08-25T09:30:08.106Z
Learning: In the Ginger codebase, URL validation in configuration classes like ZAPConfiguration should not throw exceptions for invalid URLs. The team prefers simple normalization (like trimming trailing slashes) over strict validation with exception throwing. This was confirmed by AmanPrasad43 in PR #4281 for the ZAPConfiguration.ZAPUrl property setter.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapClientWrapper.csGinger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs
📚 Learning: 2025-08-28T09:27:18.793Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4286
File: Ginger/Ginger/Drivers/DriversConfigsEditPages/WebAgentConfigEditPage.xaml.cs:556-571
Timestamp: 2025-08-28T09:27:18.793Z
Learning: In Ginger's WebAgentConfigEditPage.xaml.cs ZAP configuration, when ZAP Security Testing is enabled, the manual proxy UI should remain enabled/accessible even though ZAP will override the proxy settings in the background. This allows users to see and modify their proxy configurations for transparency and future use when ZAP is disabled. Confirmed by AmanPrasad43 in PR #4286.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapClientWrapper.csGinger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs
📚 Learning: 2025-08-25T09:20:17.608Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4281
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:1958-1970
Timestamp: 2025-08-25T09:20:17.608Z
Learning: In Ginger security testing, passive ZAP scans should be invoked with an empty testURL (ActSecurityTesting.ExecutePassiveZapScan("", act)) to aggregate alerts across the entire ZAP session. Passing a URL scopes results to that base URL. Only the active scan path should pass Driver.Url, with null/empty checks. Context: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs :: ActSecurity. Confirmed by AmanPrasad43 in PR #4281.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapClientWrapper.csGinger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs
📚 Learning: 2025-08-29T09:35:45.994Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4286
File: Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs:287-309
Timestamp: 2025-08-29T09:35:45.994Z
Learning: In Ginger ZAP security testing, alert names should preserve their original formatting with spaces and hyphens. When comparing alert names in EvaluateScanResultAPI and similar methods, use StringComparison.OrdinalIgnoreCase for case-insensitive matching but do not normalize by removing spaces or hyphens, as alert names like "False Positive" need to remain as separate words. Confirmed by AmanPrasad43 in PR #4286.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapClientWrapper.csGinger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs
📚 Learning: 2025-09-03T17:39:51.349Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4290
File: Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs:343-349
Timestamp: 2025-09-03T17:39:51.349Z
Learning: The user AmanPrasad43 initially claimed that ApiResponseSet in the ZAP library is a string type, but the ZAP .NET API library source code and the existing code patterns in ZapProxyService.cs show that ApiResponseSet has a Dictionary property that should be used instead of ToString() for parsing alert summaries. The Dictionary property provides structured access to alert data with key-value pairs.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapClientWrapper.cs
📚 Learning: 2025-01-30T07:38:05.310Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4072
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActWebSmartSyncHandler.cs:137-150
Timestamp: 2025-01-30T07:38:05.310Z
Learning: In the Ginger automation framework, element existence checks using polling with 100ms delay is an acceptable approach, as confirmed by the maintainers.
Applied to files:
Ginger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs
🧬 Code graph analysis (2)
Ginger/GingerCoreNET/External/ZAP/ZapClientWrapper.cs (1)
Ginger/GingerCoreNET/External/ZAP/IZapClient.cs (8)
IApiResponse(7-7)IApiResponse(8-8)IApiResponse(9-9)IApiResponse(10-10)IApiResponse(11-11)IApiResponse(12-13)IApiResponse(14-14)IApiResponse(15-15)
Ginger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs (3)
Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs (13)
ZapProxyService(16-369)ZapProxyService(27-39)ZapProxyService(42-45)GetPortFromUrl(108-148)GetHostFromUrl(155-193)IsZapRunning(52-65)GenerateZapReport(86-100)List(225-230)List(351-367)AddUrlToScanTree(200-219)PerformActiveScan(236-250)EvaluateScanResultWeb(279-309)EvaluateScanResultAPI(311-341)Ginger/GingerCoreNET/External/ZAP/ZapClientWrapper.cs (8)
IApiResponse(14-14)IApiResponse(15-15)IApiResponse(17-17)IApiResponse(18-18)IApiResponse(19-21)IApiResponse(23-23)IApiResponse(24-24)IApiResponse(26-31)Ginger/GingerCoreNET/External/ZAP/IZapClient.cs (8)
IApiResponse(7-7)IApiResponse(8-8)IApiResponse(9-9)IApiResponse(10-10)IApiResponse(11-11)IApiResponse(12-13)IApiResponse(14-14)IApiResponse(15-15)
🔇 Additional comments (9)
Ginger/GingerCoreNET/External/ZAP/ZapClientWrapper.cs (4)
9-12: Constructor delegation looks goodSimple, focused initialization; aligns with upstream validation strategy noted earlier.
14-16: Core version and passive-scan delegates are correctSignatures and targets match the ZAP API.
23-24: Status and alerts summary delegates are correctGood coverage of common queries.
26-31: Bug fix confirmed: GenerateReport now forwards theme/pattern/displayParameter ordering matches the underlying API; prevents silent option loss.
Ginger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs (5)
21-26: Good DI setup for testabilityUsing IZapClient with a TestInitialize to create the SUT is clean and enables focused unit tests.
57-65: LGTM: Positive IsZapRunning pathUsing ApiResponseElement and ToString() matches the service behavior for version detection (per team learning about Version API returning string).
113-122: LGTM: Negative path correctly asserts InvalidOperationExceptionGood failure coverage when URL is not added to the scan tree.
173-184: LGTM: Positive Web evaluation path coveredConfirms allowed-only alerts return true.
75-82: Optional: test passive scan evaluation with empty target URL (session-wide aggregation)Team guidance says passive scans aggregate across the session when target URL is empty. Add a test ensuring EvaluateScanResultWeb handles empty string target and forwards it to AlertsSummary.
+ [TestMethod] + public void EvaluateScanResultWeb_WithEmptyTarget_UsesSessionAggregation() + { + var alertSummary = new ApiResponseSet("alerts", new Dictionary<string, IApiResponse>()); + _mockZap.Setup(c => c.AlertsSummary("")).Returns(alertSummary); + var allowed = new ObservableList<OperationValues>(); + var result = _service.EvaluateScanResultWeb("", allowed); + Assert.IsTrue(result); + _mockZap.Verify(c => c.AlertsSummary(""), Times.Once); + }Also applies to: 153-171
⛔ Skipped due to learnings
Learnt from: AmanPrasad43 PR: Ginger-Automation/Ginger#4281 File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:1958-1970 Timestamp: 2025-08-25T09:20:17.608Z Learning: In Ginger security testing, passive ZAP scans should be invoked with an empty testURL (ActSecurityTesting.ExecutePassiveZapScan("", act)) to aggregate alerts across the entire ZAP session. Passing a URL scopes results to that base URL. Only the active scan path should pass Driver.Url, with null/empty checks. Context: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs :: ActSecurity. Confirmed by AmanPrasad43 in PR #4281.Learnt from: AmanPrasad43 PR: Ginger-Automation/Ginger#4286 File: Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs:287-309 Timestamp: 2025-08-29T09:35:45.994Z Learning: In Ginger ZAP security testing, alert names should preserve their original formatting with spaces and hyphens. When comparing alert names in EvaluateScanResultAPI and similar methods, use StringComparison.OrdinalIgnoreCase for case-insensitive matching but do not normalize by removing spaces or hyphens, as alert names like "False Positive" need to remain as separate words. Confirmed by AmanPrasad43 in PR #4286.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs (5)
67-84: Async method uses Thread.Sleep; can block threads and hang on bad valuesUse Task.Delay in async loop; also guard against unexpected non-numeric responses.
- public async Task WaitTillPassiveScanCompleted() + public async Task WaitTillPassiveScanCompleted() { try { IApiResponse apiResponse = _zapClient.RecordsToScan(); - string tempVal = ((ApiResponseElement)apiResponse).Value; - while (!tempVal.Equals("0")) + string tempVal = ((ApiResponseElement)apiResponse).Value; + while (!string.Equals(tempVal, "0", StringComparison.Ordinal)) { - Thread.Sleep(1000); + await Task.Delay(1000).ConfigureAwait(false); apiResponse = _zapClient.RecordsToScan(); tempVal = ((ApiResponseElement)apiResponse).Value; + if (!int.TryParse(tempVal, out _)) + { + throw new InvalidOperationException($"Unexpected passive-scan counter: '{tempVal}'"); + } } } catch (Exception ex) { throw new InvalidOperationException("Error during passive scan: " + ex.Message, ex); } }
102-108: XML doc mismatch: return type isn't "string"Method returns
int?, not string. Fix the comment.- /// <returns>The port number as string, or null if not found.</returns> + /// <returns>The port number, or null if not found.</returns>
171-177: Normalize IPv6 host to bracketless form for non-URL inputsWhen input is "[::1]:8080", current branch returns "[::1]" (with brackets). Client libraries typically expect "::1". Align with Uri.Host behavior.
- if (idx > 0) - { - return url.Substring(0, idx + 1); - } + if (idx > 0) + { + var hostWithBrackets = url.Substring(0, idx + 1); + return hostWithBrackets.Trim('[', ']'); + }
250-264: Active scan polling: add timeout/cancellation and avoid blockingConsider async polling with timeout to prevent indefinite waits on network issues.
- private void WaitTillActiveScanIsCompleted(string scanId) + private async Task WaitTillActiveScanIsCompletedAsync(string scanId, TimeSpan? timeout = null) { - var apiResponse = _zapClient.ScanStatus(scanId); - string status = ((ApiResponseElement)apiResponse).Value; - - while (!status.Equals("100")) - { - Thread.Sleep(1000); - apiResponse = _zapClient.ScanStatus(scanId); - status = ((ApiResponseElement)apiResponse).Value; - Reporter.ToLog(eLogLevel.INFO, "Active scan is in progress"); - } + var deadline = DateTime.UtcNow + (timeout ?? TimeSpan.FromMinutes(10)); + while (true) + { + var apiResponse = _zapClient.ScanStatus(scanId); + string status = ((ApiResponseElement)apiResponse).Value; + if (string.Equals(status, "100", StringComparison.Ordinal)) break; + if (DateTime.UtcNow >= deadline) + throw new TimeoutException($"Active scan {scanId} did not complete before timeout."); + await Task.Delay(1000).ConfigureAwait(false); + Reporter.ToLog(eLogLevel.INFO, "Active scan is in progress"); + } Reporter.ToLog(eLogLevel.INFO, "Active scan has completed"); }Call site example:
- WaitTillActiveScanIsCompleted(scanId); + _ = WaitTillActiveScanIsCompletedAsync(scanId).GetAwaiter().GetResult();
291-296: Web evaluation should be case-insensitive (keep delimiters)
Use OrdinalIgnoreCase like API path; preserves spaces/hyphens as required.- if (!allowedAlertNames.Any(a => a.Value == alertName)) + if (!(allowedAlertNames?.Any(a => + string.Equals(a.Value, alertName, StringComparison.OrdinalIgnoreCase)) ?? false)) { testPassed = false; }
♻️ Duplicate comments (3)
Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs (3)
29-36: Don't cast nullable port; fail fast with a clear messageGetPortFromUrl can return null;
(int)nullthrows "Nullable object must have a value." Throw a descriptive error instead, per team decision to require explicit port in config.Apply:
- _zapHost = GetHostFromUrl(ValueExpression.PasswordCalculation(zAPConfiguration.ZAPUrl)); - _zapPort = (int)GetPortFromUrl(ValueExpression.PasswordCalculation(zAPConfiguration.ZAPUrl)); + _zapHost = GetHostFromUrl(ValueExpression.PasswordCalculation(zAPConfiguration.ZAPUrl)); + var parsedPort = GetPortFromUrl(ValueExpression.PasswordCalculation(zAPConfiguration.ZAPUrl)); + if (parsedPort == null) + { + throw new InvalidOperationException($"ZAP URL must include an explicit port. Current value: '{zAPConfiguration.ZAPUrl}'"); + } + _zapPort = parsedPort.Value;
56-59: Prefer typed check over ToString() for version (null-safe)Cast to ApiResponseElement and check Value; safer and clearer.
- IApiResponse version = _zapClient.Version(); - return !string.IsNullOrEmpty(version.ToString()); + IApiResponse version = _zapClient.Version(); + if (version is ApiResponseElement v) return !string.IsNullOrEmpty(v.Value); + return !string.IsNullOrEmpty(version?.ToString());
337-343: EvaluateScanResult should compute from counts, not ToString()Parse ApiResponseSet.Dictionary and return true only if any count > 0.
- var summaryResponse = _zapClient.AlertsSummary(targetUrl); - var alertSummary = (ApiResponseSet)summaryResponse; - return !string.IsNullOrEmpty(alertSummary.ToString()); + var summaryResponse = _zapClient.AlertsSummary(targetUrl); + var alertSummary = (ApiResponseSet)summaryResponse; + foreach (var kv in alertSummary.Dictionary) + { + var valueString = kv.Value is ApiResponseElement e ? e.Value : kv.Value?.ToString(); + if (int.TryParse(valueString, out var count) && count > 0) return true; + } + return false;
📜 Review details
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.
📒 Files selected for processing (3)
Ginger/GingerCoreNET/External/ZAP/ZapClientWrapper.cs(1 hunks)Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs(12 hunks)Ginger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs(1 hunks)
🧰 Additional context used
🧠 Learnings (16)
📓 Common learnings
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4281
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:1958-1970
Timestamp: 2025-08-25T09:20:17.608Z
Learning: In Ginger security testing, passive ZAP scans should be invoked with an empty testURL (ActSecurityTesting.ExecutePassiveZapScan("", act)) to aggregate alerts across the entire ZAP session. Passing a URL scopes results to that base URL. Only the active scan path should pass Driver.Url, with null/empty checks. Context: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs :: ActSecurity. Confirmed by AmanPrasad43 in PR #4281.
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4290
File: Ginger/GingerCoreNET/External/ZAP/ZapClientWrapper.cs:17-21
Timestamp: 2025-09-03T17:35:32.098Z
Learning: In the Ginger ZAP integration, the OWASPZAPDotNetAPI library requires string values ("true"/"false") for boolean parameters in methods like AccessUrl, Scan, etc. The ZapClientWrapper should maintain this string-based interface to stay consistent with the underlying ZAP library's API requirements rather than adding boolean overloads.
📚 Learning: 2025-09-03T17:35:32.098Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4290
File: Ginger/GingerCoreNET/External/ZAP/ZapClientWrapper.cs:17-21
Timestamp: 2025-09-03T17:35:32.098Z
Learning: In the Ginger ZAP integration, the OWASPZAPDotNetAPI library requires string values ("true"/"false") for boolean parameters in methods like AccessUrl, Scan, etc. The ZapClientWrapper should maintain this string-based interface to stay consistent with the underlying ZAP library's API requirements rather than adding boolean overloads.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapProxyService.csGinger/GingerCoreNET/External/ZAP/ZapClientWrapper.cs
📚 Learning: 2025-09-03T17:37:45.916Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4290
File: Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs:56-59
Timestamp: 2025-09-03T17:37:45.916Z
Learning: In the Ginger ZAP integration, the ZAP library's version API returns a string value, which is why the code uses ToString() to convert the IApiResponse to string format in the IsZapRunning method in ZapProxyService.cs.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapProxyService.csGinger/GingerCoreNET/External/ZAP/ZapClientWrapper.csGinger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs
📚 Learning: 2025-08-28T09:27:18.793Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4286
File: Ginger/Ginger/Drivers/DriversConfigsEditPages/WebAgentConfigEditPage.xaml.cs:556-571
Timestamp: 2025-08-28T09:27:18.793Z
Learning: In Ginger's WebAgentConfigEditPage.xaml.cs ZAP configuration, when ZAP Security Testing is enabled, the manual proxy UI should remain enabled/accessible even though ZAP will override the proxy settings in the background. This allows users to see and modify their proxy configurations for transparency and future use when ZAP is disabled. Confirmed by AmanPrasad43 in PR #4286.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapProxyService.csGinger/GingerCoreNET/External/ZAP/ZapClientWrapper.csGinger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs
📚 Learning: 2025-09-03T17:37:09.236Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4290
File: Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs:34-39
Timestamp: 2025-09-03T17:37:09.236Z
Learning: In Ginger's ZAP integration, when GetPortFromUrl() returns null (URL without port), avoid using hardcoded fallback ports as they may conflict with other applications. Instead, require explicit port specification in the ZAP configuration and provide clear error messages when port is missing.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapProxyService.csGinger/GingerCoreNET/External/ZAP/ZapClientWrapper.csGinger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs
📚 Learning: 2025-08-25T09:30:08.106Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4281
File: Ginger/GingerCoreCommon/External/Configurations/ZAPConfiguration.cs:37-58
Timestamp: 2025-08-25T09:30:08.106Z
Learning: In the Ginger codebase, URL validation in configuration classes like ZAPConfiguration should not throw exceptions for invalid URLs. The team prefers simple normalization (like trimming trailing slashes) over strict validation with exception throwing. This was confirmed by AmanPrasad43 in PR #4281 for the ZAPConfiguration.ZAPUrl property setter.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapProxyService.csGinger/GingerCoreNET/External/ZAP/ZapClientWrapper.csGinger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs
📚 Learning: 2025-08-25T09:27:30.249Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4281
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:1433-1441
Timestamp: 2025-08-25T09:27:30.249Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs, SetProxy(dynamic options) should not force options.AcceptInsecureCertificates = false. Only set AcceptInsecureCertificates = true when UseZAP is enabled to avoid breaking setups that permit self-signed/dev certificates.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs
📚 Learning: 2025-08-25T09:20:17.608Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4281
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:1958-1970
Timestamp: 2025-08-25T09:20:17.608Z
Learning: In Ginger security testing, passive ZAP scans should be invoked with an empty testURL (ActSecurityTesting.ExecutePassiveZapScan("", act)) to aggregate alerts across the entire ZAP session. Passing a URL scopes results to that base URL. Only the active scan path should pass Driver.Url, with null/empty checks. Context: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs :: ActSecurity. Confirmed by AmanPrasad43 in PR #4281.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapProxyService.csGinger/GingerCoreNET/External/ZAP/ZapClientWrapper.csGinger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs
📚 Learning: 2025-09-04T05:43:12.360Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4290
File: Ginger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs:66-73
Timestamp: 2025-09-04T05:43:12.360Z
Learning: In Ginger's ZAP integration, the ZAP API Version() method always returns a valid IApiResponse object and cannot return null, so null checks in unit tests for Version() are not valid test cases.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapProxyService.csGinger/GingerCoreNET/External/ZAP/ZapClientWrapper.csGinger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs
📚 Learning: 2025-08-29T09:35:45.994Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4286
File: Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs:287-309
Timestamp: 2025-08-29T09:35:45.994Z
Learning: In Ginger ZAP security testing, alert names should preserve their original formatting with spaces and hyphens. When comparing alert names in EvaluateScanResultAPI and similar methods, use StringComparison.OrdinalIgnoreCase for case-insensitive matching but do not normalize by removing spaces or hyphens, as alert names like "False Positive" need to remain as separate words. Confirmed by AmanPrasad43 in PR #4286.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapProxyService.csGinger/GingerCoreNET/External/ZAP/ZapClientWrapper.csGinger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs
📚 Learning: 2025-09-03T17:39:51.349Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4290
File: Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs:343-349
Timestamp: 2025-09-03T17:39:51.349Z
Learning: The user AmanPrasad43 initially claimed that ApiResponseSet in the ZAP library is a string type, but the ZAP .NET API library source code and the existing code patterns in ZapProxyService.cs show that ApiResponseSet has a Dictionary property that should be used instead of ToString() for parsing alert summaries. The Dictionary property provides structured access to alert data with key-value pairs.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapProxyService.csGinger/GingerCoreNET/External/ZAP/ZapClientWrapper.cs
📚 Learning: 2025-08-25T08:48:11.915Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4281
File: Ginger/GingerCore/GingerCore.csproj:538-538
Timestamp: 2025-08-25T08:48:11.915Z
Learning: In the Ginger codebase, the OWASPZAPDotNetAPI package reference in GingerCore.csproj is required for architectural reasons, even when direct usage is not immediately visible in the codebase. This was confirmed by AmanPrasad43 during the security testing feature implementation.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapProxyService.csGinger/GingerCoreNET/External/ZAP/ZapClientWrapper.csGinger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs
📚 Learning: 2025-06-16T10:37:13.073Z
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.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs
📚 Learning: 2025-04-25T13:29:45.059Z
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.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs
📚 Learning: 2025-07-16T14:17:56.429Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4254
File: Ginger/GingerCoreNET/GeneralLib/ElementWrapperInfo.cs:89-93
Timestamp: 2025-07-16T14:17:56.429Z
Learning: In Ginger/GingerCoreNET/GeneralLib/ElementWrapperInfo.cs, the user prashelke prefers to keep the current casing for properties `insideIframe` and `insideShadowDOM` in the `ElementwrapperContext` class rather than changing them to PascalCase.
Applied to files:
Ginger/GingerCoreNET/External/ZAP/ZapClientWrapper.cs
📚 Learning: 2025-01-30T07:38:05.310Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4072
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActWebSmartSyncHandler.cs:137-150
Timestamp: 2025-01-30T07:38:05.310Z
Learning: In the Ginger automation framework, element existence checks using polling with 100ms delay is an acceptable approach, as confirmed by the maintainers.
Applied to files:
Ginger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs
🧬 Code graph analysis (3)
Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs (4)
Ginger/GingerCoreCommon/External/Configurations/ZAPConfiguration.cs (1)
ZAPConfiguration(5-71)Ginger/GingerCoreNET/ValueExpressionLib/ValueExpression.cs (1)
PasswordCalculation(2039-2064)Ginger/GingerCoreNET/External/ZAP/ZapClientWrapper.cs (10)
ZapClientWrapper(5-33)ZapClientWrapper(9-12)IApiResponse(14-14)IApiResponse(15-15)IApiResponse(18-18)IApiResponse(19-19)IApiResponse(20-22)IApiResponse(24-24)IApiResponse(25-25)IApiResponse(27-32)Ginger/GingerCoreNET/External/ZAP/IZapClient.cs (8)
IApiResponse(7-7)IApiResponse(8-8)IApiResponse(9-9)IApiResponse(10-10)IApiResponse(11-11)IApiResponse(12-13)IApiResponse(14-14)IApiResponse(15-15)
Ginger/GingerCoreNET/External/ZAP/ZapClientWrapper.cs (1)
Ginger/GingerCoreNET/External/ZAP/IZapClient.cs (8)
IApiResponse(7-7)IApiResponse(8-8)IApiResponse(9-9)IApiResponse(10-10)IApiResponse(11-11)IApiResponse(12-13)IApiResponse(14-14)IApiResponse(15-15)
Ginger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs (3)
Ginger/GingerCoreNET/External/ZAP/ZapProxyService.cs (14)
ZapProxyService(16-363)ZapProxyService(27-39)ZapProxyService(42-45)GetPortFromUrl(108-148)GetHostFromUrl(155-193)IsZapRunning(52-65)Task(67-84)GenerateZapReport(86-100)List(219-224)List(345-361)AddUrlToScanTree(200-213)PerformActiveScan(230-244)EvaluateScanResultWeb(273-303)EvaluateScanResultAPI(305-335)Ginger/GingerCoreNET/External/ZAP/ZapClientWrapper.cs (8)
IApiResponse(14-14)IApiResponse(15-15)IApiResponse(18-18)IApiResponse(19-19)IApiResponse(20-22)IApiResponse(24-24)IApiResponse(25-25)IApiResponse(27-32)Ginger/GingerCoreNET/External/ZAP/IZapClient.cs (8)
IApiResponse(7-7)IApiResponse(8-8)IApiResponse(9-9)IApiResponse(10-10)IApiResponse(11-11)IApiResponse(12-13)IApiResponse(14-14)IApiResponse(15-15)
🔇 Additional comments (7)
Ginger/GingerCoreNETUnitTest/ZAP/ZapProxyServiceTests.cs (5)
21-26: Good DI setup for service under testConstructor-injected IZapClient mock keeps tests fast and isolated.
76-86: Nice polling verification for passive scanSetupSequence + Verify ensures loop behavior without flakiness.
110-127: Solid positive-path assertion for AddUrlToScanTreeVerifies both AccessUrl and Urls were invoked as expected.
190-201: Positive Web evaluation test adds valuable coverageCovers allowed-alerts-only success path.
236-264: Alert summary tests are thorough (content and edge cases)Checks tuple contents and ignores non-integer counts—great.
Ginger/GingerCoreNET/External/ZAP/ZapClientWrapper.cs (2)
17-23: Doc note for string booleans is helpfulKeeps wrapper aligned with ZAP API expectations.
27-32: GenerateReport now forwards all options correctlyPassing theme/pattern/display fixes silent option loss.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thank you for your contribution.
Before submitting this PR, please make sure:
Summary by CodeRabbit