MOSIP-44099 - Added property dynamically to call testcasedependency#1887
MOSIP-44099 - Added property dynamically to call testcasedependency#1887mohanachandran-s merged 3 commits intomosip:developfrom
Conversation
Signed-off-by: Prathmesh Jadhav <prathmesh.j@cyberpwn.com>
WalkthroughReplaced a public static string field in BaseTestCase with a private static final Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@apitest-commons/src/main/java/io/mosip/testrig/apirig/testrunner/BaseTestCase.java`:
- Around line 199-204: The BASE_CONFIG_PATH constant assumes an IDE project
layout and will fail from a JAR; update getTestCaseInterDependencyPath() to
resolve the config path using the same JAR-vs-IDE logic as
getGlobalResourcePath() (or replace BASE_CONFIG_PATH with a computed path via
getGlobalResourcePath()), so getTestCaseInterDependencyPath() returns
getGlobalResourcePath().resolve("config").resolve("testCaseInterDependency.json").toString()
(and remove or refactor BASE_CONFIG_PATH) to keep behavior consistent with
init() and other methods that use getGlobalResourcePath().
🧹 Nitpick comments (1)
apitest-commons/src/main/java/io/mosip/testrig/apirig/testrunner/BaseTestCase.java (1)
206-212:pluginNameis not sanitized — path traversal risk.If
pluginNamecontains characters like../, the resolved path could escape the intended config directory. Even in a test framework, this is worth guarding against, especially if the value originates from external configuration or user input.Suggested guard
public static String getTestCaseInterDependencyPath(String pluginName) { if (pluginName == null || pluginName.isBlank()) { return getTestCaseInterDependencyPath(); } + if (pluginName.contains("..") || pluginName.contains("/") || pluginName.contains("\\")) { + throw new IllegalArgumentException("Invalid pluginName: " + pluginName); + } return BASE_CONFIG_PATH.resolve("testCaseInterDependency_" + pluginName + ".json").toString(); }
apitest-commons/src/main/java/io/mosip/testrig/apirig/testrunner/BaseTestCase.java
Show resolved
Hide resolved
Signed-off-by: Prathmesh Jadhav <prathmesh.j@cyberpwn.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@apitest-commons/src/main/java/io/mosip/testrig/apirig/testrunner/BaseTestCase.java`:
- Around line 202-208: The getTestCaseInterDependencyPath method concatenates
pluginName into a file name allowing path traversal; validate/sanitize
pluginName (in getTestCaseInterDependencyPath) by rejecting null/blank as before
and additionally checking for forbidden sequences such as "/" "\" or ".." (or
any path-separator characters) and throw an IllegalArgumentException for invalid
names, or alternatively sanitize by stripping those characters, then build the
filename ("testCaseInterDependency_" + sanitizedName + ".json") and resolve it
against BASE_CONFIG_PATH; for extra safety normalize the resolved Path and
assert it startsWith(BASE_CONFIG_PATH) before returning the string.
apitest-commons/src/main/java/io/mosip/testrig/apirig/testrunner/BaseTestCase.java
Show resolved
Hide resolved
Signed-off-by: Prathmesh Jadhav <prathmesh.j@cyberpwn.com>
Added property dynamically to call testcasedependency
Summary by CodeRabbit