MOSIP-44222: Fixed security issue for PMS in EmailableReport where internal TestNG method names were exposed.

[SradhaMohanty5899]

• Improved security by hiding internal method names
• Fixed PMS scenario grouping
• Prevents NPE in other modules

Summary by CodeRabbit

• Bug Fixes

  • Improved test-case identification in reports with a robust, centralized fallback for deriving test labels—ensures consistent scenario naming, grouping, and navigation even when standard identifiers are missing.

• Refactor

  • Simplified internal report-generation logic and removed unused dependencies to reduce clutter and improve maintainability.

[coderabbitai]

Walkthrough

Replaced ad-hoc method-name usage with a private helper getTestCaseKey(ITestResult) in EmailableReport to derive test-case keys from a TestCaseName attribute, TestCaseDTO parameter, or fallback to the method name. Replaced multiple calls accordingly and removed unused regex imports.

Changes

Cohort / File(s)	Summary
Test Case Key Extraction Refactor apitest-commons/src/main/java/io/mosip/testrig/apirig/report/EmailableReport.java	Added private getTestCaseKey(ITestResult) to derive keys from TestCaseName attribute or parameters, fallback to method name. Replaced several result.getMethod().getMethodName() usages with the helper. Updated writeScenario to read TestCaseName attribute with fallback. Removed unused Matcher/Pattern imports.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 I hopped through code with nimble flair,
I found the name and handled care,
Attributes, DTOs, then methods last,
Keys unified — no more contrast,
A tidy report, a joyful hare. 🥕✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.
Title check	✅ Passed	The title clearly describes the main change: fixing a security issue in EmailableReport where internal TestNG method names were exposed. This aligns well with the changeset which replaces methodName usage with a centralized key extraction mechanism.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

apitest-commons/src/main/java/io/mosip/testrig/apirig/report/EmailableReport.java (1)

780-795: ⚠️ Potential issue | 🟠 Major

Escape test case keys before writing HTML.

getTestCaseKey can come from attributes/DTOs and is now written to HTML without escaping, which can break the report or allow HTML injection. Use Utils.escapeHtml for the displayed value.

✅ Suggested fix

-					String temp = uniqueIdentifier.isEmpty() ? getTestCaseKey(firstResult) : uniqueIdentifier;
+					String temp = uniqueIdentifier.isEmpty() ? getTestCaseKey(firstResult) : uniqueIdentifier;
+					String safeTemp = Utils.escapeHtml(temp);
@@
-					buffer.append("<td style=\"text-align:left;\"><a href=\"#m").append(scenarioIndex).append("\">")
-					.append(temp).append("</a></td>").append("<td style=\"text-align:left;\">")
+					buffer.append("<td style=\"text-align:left;\"><a href=\"#m").append(scenarioIndex).append("\">")
+					.append(safeTemp).append("</a></td>").append("<td style=\"text-align:left;\">")
@@
-						buffer.append("<tr class=\"").append(cssClass).append("\">")
-								.append("<td style=\"text-align:center;\"><a href=\"#m").append(scenarioIndex)
-								.append("\">").append(temp).append("</a></td></tr>");
+						buffer.append("<tr class=\"").append(cssClass).append("\">")
+								.append("<td style=\"text-align:center;\"><a href=\"#m").append(scenarioIndex)
+								.append("\">").append(safeTemp).append("</a></td></tr>");

🤖 Fix all issues with AI agents

In
`@apitest-commons/src/main/java/io/mosip/testrig/apirig/report/EmailableReport.java`:
- Around line 878-882: The fallback logic for TestCaseName only checks for null
so empty or whitespace values still display as blank; update the code around the
testCaseName assignment (the variable testCaseName obtained via
result.getAttribute("TestCaseName")) to treat blank/whitespace as missing by
trimming and/or using an isBlank check and then falling back to
result.getMethod().getMethodName() when testCaseName is null or blank. Ensure
you reference the same variable name testCaseName and the same fallback source
result.getMethod().getMethodName().
- Around line 1374-1387: In getTestCaseKey, guard against null/blank values
returned from TestCaseDTO.getTestCaseName() and fall back to
result.getMethod().getMethodName() if the dto name is null/empty; specifically,
when inspecting result.getParameters() and the first param is a TestCaseDTO,
retrieve dto.getTestCaseName() into a local String (e.g., dtoName), trim and
check it for null/empty before returning it, otherwise continue to the final
fallback. Also rename the local TestCaseName variable to follow Java conventions
(testCaseName).

🧹 Nitpick comments (1)

apitest-commons/src/main/java/io/mosip/testrig/apirig/report/EmailableReport.java (1)

1193-1211: Align sort order with the new test case key.

groupResults now groups by getTestCaseKey, but the list is still sorted by method name. If keys don’t correlate with method names, identical keys might not be contiguous, which can split groups unexpectedly. Consider sorting by the same key you group by.

♻️ Suggested refactor (RESULT_COMPARATOR)

 		protected static final Comparator<ITestResult> RESULT_COMPARATOR = new Comparator<ITestResult>() {
 			`@Override`
 			public int compare(ITestResult o1, ITestResult o2) {
 				int result = o1.getTestClass().getName().compareTo(o2.getTestClass().getName());
 				if (result == 0) {
-					result = o1.getMethod().getMethodName().compareTo(o2.getMethod().getMethodName());
+					String k1 = getTestCaseKey(o1);
+					String k2 = getTestCaseKey(o2);
+					result = k1.compareTo(k2);
+					if (result == 0) {
+						result = o1.getMethod().getMethodName().compareTo(o2.getMethod().getMethodName());
+					}
 				}
 				return result;
 			}
 		};