[CUS-10700] Nlp to execute ssh shell commands using java library.#340
Merged
Conversation
📝 WalkthroughWalkthroughAdds a new Maven module implementing SSH command execution via SSHJ across multiple platforms, plus utilities to convert text to an image and upload files to S3 presigned URLs; introduces SDK properties and build configuration. Changes
Sequence DiagramsequenceDiagram
actor User
participant Action as SSHCommandExecutionSSHJ
participant SSH as SSH Server
participant ImgConv as StringToImageConverter
participant Uploader as ImageComparisonUtils
participant S3 as S3 Storage
User->>Action: execute()
Action->>SSH: connect(host,port,user,password)
SSH-->>Action: connected
Action->>SSH: execute(wrapped command)
SSH-->>Action: stdout/stderr (+ exit status)
Action->>Action: store output in RunTimeData
alt step has screenshot URL
Action->>ImgConv: convert(output) -> File
ImgConv-->>Action: tempFile
Action->>Uploader: uploadFile(s3SignedURL, tempFilePath)
Uploader->>S3: HTTP PUT presigned URL
S3-->>Uploader: HTTP 200/201
Uploader-->>Action: upload success
Action->>Action: delete tempFile
end
Action-->>User: return SUCCESS/FAILED
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
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)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (4)
ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/util/StringToImageConverter.java (1)
74-79: Temp files fromconvertToFileaccumulate until JVM shutdown.
File.createTempFilecreates files that persist. The caller (SSHCommandExecutionSSHJ) usesdeleteOnExit()but never callsdelete(). In a long-running server process, temporary PNG files will accumulate in the temp directory. The caller shoulddelete()immediately after use, withdeleteOnExit()only as a safety net.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/util/StringToImageConverter.java` around lines 74 - 79, The convertToFile method currently creates persistent temp PNGs; modify convertToFile (in StringToImageConverter) to call tempFile.deleteOnExit() as a safety net and update the caller (SSHCommandExecutionSSHJ) to explicitly call tempFile.delete() immediately after the file is consumed (while still optionally keeping deleteOnExit()). Ensure convertToFile still returns the File so the caller can delete it, and add a short comment documenting that the caller must perform immediate deletion to avoid accumulating temp files.ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/util/ImageComparisonUtils.java (1)
16-23:WebDriver driverfield is stored but never used.The constructor accepts and stores a
WebDriverreference, but no method in this class uses it. This creates an unnecessary coupling and holds a reference to a heavy object.Proposed fix — remove unused field
public class ImageComparisonUtils { - WebDriver driver; Logger logger; - public ImageComparisonUtils(WebDriver driver, Logger logger) { - this.driver = driver; + public ImageComparisonUtils(Logger logger) { this.logger = logger; }Note: This would also require updating the call site in
SSHCommandExecutionSSHJ.java(Line 114).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/util/ImageComparisonUtils.java` around lines 16 - 23, The ImageComparisonUtils class stores an unused WebDriver field and accepts it in the constructor (field: WebDriver driver; constructor: ImageComparisonUtils(WebDriver driver, Logger logger)); remove the field and change the constructor to only accept Logger (ImageComparisonUtils(Logger logger)), remove the WebDriver import, and update all call sites that construct ImageComparisonUtils (e.g., the instantiation in SSHCommandExecutionSSHJ) to stop passing a WebDriver and instead pass only the Logger so the code compiles and no unused heavy reference is kept.ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/web/SSHCommandExecutionSSHJ.java (2)
122-126: Usedelete()instead ofdeleteOnExit()for immediate cleanup.
deleteOnExit()only removes the file when the JVM shuts down. In a long-running server, temp files will accumulate. Since the upload is already complete at this point, delete immediately and keepdeleteOnExit()only as a fallback in the try block before the upload.Proposed fix
} finally { if (screenshotFile != null && screenshotFile.exists()) { - screenshotFile.deleteOnExit(); + if (!screenshotFile.delete()) { + screenshotFile.deleteOnExit(); + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/web/SSHCommandExecutionSSHJ.java` around lines 122 - 126, The finally block currently calls screenshotFile.deleteOnExit(), which defers removal until JVM shutdown; change this to screenshotFile.delete() so temp files are removed immediately after use (referencing the screenshotFile variable in SSHCommandExecutionSSHJ). Also, as a safety net add screenshotFile.deleteOnExit() earlier (before the upload attempt) so the file is still scheduled for JVM-exit cleanup if immediate deletion fails.
128-128:setSuccessMessageincludes full command output — may be excessively large.If the SSH command produces megabytes of output, this will be stored entirely in the success message. Consider truncating it.
Proposed fix
- setSuccessMessage("Output is: " + output.toString()); + String outputStr = output.toString(); + String truncated = outputStr.length() > 2000 + ? outputStr.substring(0, 2000) + "... [truncated]" + : outputStr; + setSuccessMessage("Output is: " + truncated);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/web/SSHCommandExecutionSSHJ.java` at line 128, The current call setSuccessMessage("Output is: " + output.toString()) may store arbitrarily large command output; change it to truncate the output to a safe maximum before sending to setSuccessMessage (e.g. define an int MAX_OUTPUT_LENGTH = 1000, convert output to a String via output.toString(), then if length > MAX_OUTPUT_LENGTH call setSuccessMessage("Output (truncated): " + outStr.substring(0, MAX_OUTPUT_LENGTH) + "…") else pass the whole string), and ensure the full output is preserved elsewhere (e.g. logged via logger.info/debug or written to a temporary/artifact file) rather than embedded entirely in the success message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ssh_shell_command_executor_using_sshj/pom.xml`:
- Around line 42-46: The TestNG dependency in the pom (artifactId "testng")
lacks a test scope so it will be included in the shaded JAR; update the TestNG
dependency block by adding a <scope>test</scope> element for the dependency on
org.testng:testng to ensure it is only used for tests and not bundled into the
runtime artifact.
- Around line 59-82: The pom currently mixes jackson-annotations (2.13.0) and
jackson-databind (2.12.3) which risks runtime incompatibilities; update the
jackson dependency versions so all Jackson modules use the same compatible
release (e.g., set both jackson-annotations and jackson-databind to 2.21.0), and
if there are other Jackson artifacts in the POM (jackson-core, jackson-.*) align
those as well to 2.21.0 to ensure a consistent Jackson BOM across the project.
In
`@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/util/ImageComparisonUtils.java`:
- Around line 32-33: The debug logs in ImageComparisonUtils currently print the
full s3SignedURL (logger.debug lines referencing s3SignedURL and localPath)
which leaks presigned URL credentials; change these logger.debug calls to avoid
printing the full URL — instead log a sanitized identifier such as the S3 bucket
and object key or a masked URL (e.g., strip query parameters) and keep localPath
if needed; update the logging statements that reference s3SignedURL to log only
the bucket/key or a masked representation and leave localPath logging unchanged
or minimal.
- Around line 31-54: The uploadFile method currently ignores the HttpResponse
from executing the HttpPut; update uploadFile to inspect the response status
(e.g., response.getStatusLine().getStatusCode()) after
httpclient.execute(httpPut) and only return true for successful 2xx codes,
otherwise log the status code and response body (or reason phrase) and return
false; ensure you still catch and log exceptions as before and reference the
HttpResponse variable named response and the uploadFile method when making the
change.
In
`@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/util/StringToImageConverter.java`:
- Around line 34-65: The convert(...) method can throw HeadlessException on
servers without a display; to fix it, at the start of
StringToImageConverter.convert(String) check
java.awt.GraphicsEnvironment.isHeadless() and when true call
System.setProperty("java.awt.headless", "true") (or otherwise ensure the JVM is
in headless mode) before any BufferedImage/Graphics2D/Font/FontMetrics usage, so
BufferedImage/Graphics2D creation and text rendering will not fail in CI/server
environments.
In
`@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/web/SSHCommandExecutionSSHJ.java`:
- Around line 86-103: The current logic in SSHCommandExecutionSSHJ reads
cmd.getInputStream() fully and only then reads cmd.getErrorStream(), which can
deadlock for large outputs; update the command execution to consume stdout and
stderr concurrently (for example spawn two threads/tasks or use an executor to
stream-copy both cmd.getInputStream() and cmd.getErrorStream() into buffers
simultaneously) and then wait for cmd.join(...) and the two reader tasks to
finish before checking cmd.getExitStatus(); refer to the cmd variable and the
surrounding command-execution method in SSHCommandExecutionSSHJ to locate where
to replace the sequential Reader reads with concurrent stream gobblers and
ensure thread-safe aggregation into the final output.
- Around line 77-82: The code in SSHCommandExecutionSSHJ currently uses
PromiscuousVerifier (ssh.addHostKeyVerifier(new PromiscuousVerifier())), which
disables host key verification; change this to validate the server host key
against a configurable fingerprint or known-hosts entry: add a new test-data
parameter (e.g., hostFingerprint or knownHosts) and in the
SSHCommandExecutionSSHJ connection logic replace PromiscuousVerifier with a
HostKeyVerifier implementation that checks the remote key's fingerprint against
that parameter (or uses SSHClient.loadKnownHosts()/KnownHostsVerifier if
knownHosts provided), and fail the connection if verification does not match;
document the new parameter in the class comments and ensure
ssh.addHostKeyVerifier is only called with the secure verifier.
- Around line 135-142: The finally block in SSHCommandExecutionSSHJ currently
closes session and ssh in the same try, so if session.close() throws,
ssh.disconnect() is skipped; change the cleanup to close each resource in its
own try-catch (e.g., try { if (session != null) session.close(); } catch
(IOException e) { logger.info(...); } and then a separate try { if (ssh != null)
ssh.disconnect(); } catch (IOException e) { logger.info(...); }) so both
session.close() and ssh.disconnect() are attempted independently and both
exceptions are logged.
- Around line 78-80: The SSHClient connection can hang because no connect
timeout is set in SSHCommandExecutionSSHJ; before calling ssh.connect(host,
port) on the SSHClient instance (variable ssh), set a connect timeout (e.g. call
ssh.setConnectTimeout(timeoutMs) with a sensible default like 10_000 or read
from configuration) so the connect attempt will fail fast on
unreachable/firewalled hosts; place this call in the same method (in
SSHCommandExecutionSSHJ) immediately before ssh.connect(...) and ensure the
timeout value is configurable and non-zero.
- Line 64: The code in SSHCommandExecutionSSHJ that does int port =
Integer.parseInt(portNumber.getValue().toString()); can throw an uncaught
NumberFormatException for non-numeric input; wrap the parse in validation and
error handling: first check portNumber.getValue() is non-null and matches a
numeric pattern (or use Integer.parseInt inside a try/catch), validate the
parsed value is within 1–65535, and on failure throw or return a clear,
user-friendly error (or log via the existing logger) instead of letting
NumberFormatException propagate; update the parsing site (portNumber variable
usage in SSHCommandExecutionSSHJ) to implement this safe-parse and error message
behavior.
In
`@ssh_shell_command_executor_using_sshj/src/main/resources/testsigma-sdk.properties`:
- Line 1: The committed file contains a hardcoded JWT value under the property
key "testsigma-sdk.api.key" which must be removed: revoke the exposed token
immediately, delete the properties file from the repo, purge it from Git history
(e.g., git filter-repo or BFG) and add the filename to .gitignore; then change
the configuration to read testsigma-sdk.api.key from an environment variable or
CI secret at runtime/CI (update the code that reads the property to fall back to
ENV if present) and ensure CI/CD secrets injection is configured to supply the
API key instead of committing it.
---
Nitpick comments:
In
`@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/util/ImageComparisonUtils.java`:
- Around line 16-23: The ImageComparisonUtils class stores an unused WebDriver
field and accepts it in the constructor (field: WebDriver driver; constructor:
ImageComparisonUtils(WebDriver driver, Logger logger)); remove the field and
change the constructor to only accept Logger (ImageComparisonUtils(Logger
logger)), remove the WebDriver import, and update all call sites that construct
ImageComparisonUtils (e.g., the instantiation in SSHCommandExecutionSSHJ) to
stop passing a WebDriver and instead pass only the Logger so the code compiles
and no unused heavy reference is kept.
In
`@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/util/StringToImageConverter.java`:
- Around line 74-79: The convertToFile method currently creates persistent temp
PNGs; modify convertToFile (in StringToImageConverter) to call
tempFile.deleteOnExit() as a safety net and update the caller
(SSHCommandExecutionSSHJ) to explicitly call tempFile.delete() immediately after
the file is consumed (while still optionally keeping deleteOnExit()). Ensure
convertToFile still returns the File so the caller can delete it, and add a
short comment documenting that the caller must perform immediate deletion to
avoid accumulating temp files.
In
`@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/web/SSHCommandExecutionSSHJ.java`:
- Around line 122-126: The finally block currently calls
screenshotFile.deleteOnExit(), which defers removal until JVM shutdown; change
this to screenshotFile.delete() so temp files are removed immediately after use
(referencing the screenshotFile variable in SSHCommandExecutionSSHJ). Also, as a
safety net add screenshotFile.deleteOnExit() earlier (before the upload attempt)
so the file is still scheduled for JVM-exit cleanup if immediate deletion fails.
- Line 128: The current call setSuccessMessage("Output is: " +
output.toString()) may store arbitrarily large command output; change it to
truncate the output to a safe maximum before sending to setSuccessMessage (e.g.
define an int MAX_OUTPUT_LENGTH = 1000, convert output to a String via
output.toString(), then if length > MAX_OUTPUT_LENGTH call
setSuccessMessage("Output (truncated): " + outStr.substring(0,
MAX_OUTPUT_LENGTH) + "…") else pass the whole string), and ensure the full
output is preserved elsewhere (e.g. logged via logger.info/debug or written to a
temporary/artifact file) rather than embedded entirely in the success message.
Comment on lines
+42
to
+46
| <dependency> | ||
| <groupId>org.testng</groupId> | ||
| <artifactId>testng</artifactId> | ||
| <version>6.14.3</version> | ||
| </dependency> |
There was a problem hiding this comment.
TestNG dependency is missing <scope>test</scope>.
Without test scope, TestNG (and its transitive dependencies) will be bundled into the shaded JAR. This bloats the artifact and may cause classpath conflicts at runtime.
Proposed fix
<dependency>
<groupId>org.testng</groupId>
<artifactId>testng</artifactId>
<version>6.14.3</version>
+ <scope>test</scope>
</dependency>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <dependency> | |
| <groupId>org.testng</groupId> | |
| <artifactId>testng</artifactId> | |
| <version>6.14.3</version> | |
| </dependency> | |
| <dependency> | |
| <groupId>org.testng</groupId> | |
| <artifactId>testng</artifactId> | |
| <version>6.14.3</version> | |
| <scope>test</scope> | |
| </dependency> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ssh_shell_command_executor_using_sshj/pom.xml` around lines 42 - 46, The
TestNG dependency in the pom (artifactId "testng") lacks a test scope so it will
be included in the shaded JAR; update the TestNG dependency block by adding a
<scope>test</scope> element for the dependency on org.testng:testng to ensure it
is only used for tests and not bundled into the runtime artifact.
Comment on lines
+59
to
+82
| <dependency> | ||
| <groupId>com.fasterxml.jackson.core</groupId> | ||
| <artifactId>jackson-annotations</artifactId> | ||
| <version>2.13.0</version> | ||
| </dependency> | ||
|
|
||
|
|
||
| <!-- SSHJ - alternative SSH library for command execution --> | ||
| <dependency> | ||
| <groupId>com.hierynomus</groupId> | ||
| <artifactId>sshj</artifactId> | ||
| <version>0.38.0</version> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.apache.commons</groupId> | ||
| <artifactId>commons-lang3</artifactId> | ||
| <version>3.14.0</version> | ||
| </dependency> | ||
|
|
||
| <dependency> | ||
| <groupId>com.fasterxml.jackson.core</groupId> | ||
| <artifactId>jackson-databind</artifactId> | ||
| <version>2.12.3</version> <!-- Make sure to use the latest version --> | ||
| </dependency> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
latest Jackson versions 2025 2026
💡 Result:
As of Feb 18, 2026, the latest FasterXML Jackson versions published to Maven Central are:
Jackson 3.x (new major line)
- Latest stable: 3.0.4 (published 2026-01-21/22) [1]
- Latest pre-release: 3.1.0-rc1 (published 2026-01-28) [1]
Jackson 2.x (current stable line)
- Latest stable: 2.21.0 (published 2026-01-18) [2]
- Previous stable: 2.20.2 (published 2026-01-19) [2]
Jackson 2.18 (LTS line)
- Latest 2.18 LTS patch: 2.18.5 (released 2025-10-27) [3]
Sources: Maven Central directory listings for tools.jackson:jackson-bom (3.x) and com.fasterxml.jackson:jackson-bom (2.x), plus FasterXML’s release wiki for 2.18 LTS patches. [1] [2] [3]
🏁 Script executed:
# Check if we can find any Jackson version references in the repository
rg -i "jackson" --type xml -nRepository: testsigmahq/testsigma-addons
Length of output: 1735
Align Jackson versions to prevent runtime incompatibilities.
Jackson components must use matching minor versions to avoid NoSuchMethodError or IncompatibleClassChangeError at runtime. Currently, jackson-annotations (2.13.0) and jackson-databind (2.12.3) are misaligned. Update both to the same version—preferably the latest stable 2.21.0 (January 2026), or 3.0.4 if upgrading to the new major line is acceptable.
Proposed fix — align to Jackson 2.21.0
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-annotations</artifactId>
- <version>2.13.0</version>
+ <version>2.21.0</version>
</dependency>
<!-- SSHJ - alternative SSH library for command execution -->
...
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
- <version>2.12.3</version> <!-- Make sure to use the latest version -->
+ <version>2.21.0</version>
</dependency>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <dependency> | |
| <groupId>com.fasterxml.jackson.core</groupId> | |
| <artifactId>jackson-annotations</artifactId> | |
| <version>2.13.0</version> | |
| </dependency> | |
| <!-- SSHJ - alternative SSH library for command execution --> | |
| <dependency> | |
| <groupId>com.hierynomus</groupId> | |
| <artifactId>sshj</artifactId> | |
| <version>0.38.0</version> | |
| </dependency> | |
| <dependency> | |
| <groupId>org.apache.commons</groupId> | |
| <artifactId>commons-lang3</artifactId> | |
| <version>3.14.0</version> | |
| </dependency> | |
| <dependency> | |
| <groupId>com.fasterxml.jackson.core</groupId> | |
| <artifactId>jackson-databind</artifactId> | |
| <version>2.12.3</version> <!-- Make sure to use the latest version --> | |
| </dependency> | |
| <dependency> | |
| <groupId>com.fasterxml.jackson.core</groupId> | |
| <artifactId>jackson-annotations</artifactId> | |
| <version>2.21.0</version> | |
| </dependency> | |
| <!-- SSHJ - alternative SSH library for command execution --> | |
| <dependency> | |
| <groupId>com.hierynomus</groupId> | |
| <artifactId>sshj</artifactId> | |
| <version>0.38.0</version> | |
| </dependency> | |
| <dependency> | |
| <groupId>org.apache.commons</groupId> | |
| <artifactId>commons-lang3</artifactId> | |
| <version>3.14.0</version> | |
| </dependency> | |
| <dependency> | |
| <groupId>com.fasterxml.jackson.core</groupId> | |
| <artifactId>jackson-databind</artifactId> | |
| <version>2.21.0</version> | |
| </dependency> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ssh_shell_command_executor_using_sshj/pom.xml` around lines 59 - 82, The pom
currently mixes jackson-annotations (2.13.0) and jackson-databind (2.12.3) which
risks runtime incompatibilities; update the jackson dependency versions so all
Jackson modules use the same compatible release (e.g., set both
jackson-annotations and jackson-databind to 2.21.0), and if there are other
Jackson artifacts in the POM (jackson-core, jackson-.*) align those as well to
2.21.0 to ensure a consistent Jackson BOM across the project.
Comment on lines
+31
to
+54
| public boolean uploadFile(String s3SignedURL, String localPath) { | ||
| logger.debug("s3SignedURL - " + s3SignedURL); | ||
| logger.debug("localPath - " + localPath); | ||
| boolean localUrlExists = new File(localPath).exists(); | ||
| if (localUrlExists) { | ||
| logger.info(String.format("Uploading test asset to storage, presigned-URL:%s, localFilePath:%s", s3SignedURL, localPath)); | ||
| try (CloseableHttpClient httpclient = HttpClients.custom().setDefaultRequestConfig(config).build()) { | ||
| HttpPut httpPut = new HttpPut(s3SignedURL); | ||
|
|
||
| File file = new File(localPath); | ||
| HttpEntity entity = EntityBuilder.create().setFile(file).build(); | ||
| httpPut.setEntity(entity); | ||
| HttpResponse response = httpclient.execute(httpPut); | ||
| logger.info("Upload completed"); | ||
| return true; | ||
| } catch (Exception e) { | ||
| logger.info("Exception while uploading custom screenshot to s3: " + ExceptionUtils.getStackTrace(e)); | ||
| return false; | ||
| } | ||
| } else { | ||
| logger.info("Local path does not exist"); | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
HTTP response status code is never checked — uploads may fail silently.
Line 43 executes the PUT but the response is unused. A 403 (expired presigned URL) or 500 from S3 will still return true, misleading the caller into thinking the upload succeeded.
Proposed fix — check response status
HttpResponse response = httpclient.execute(httpPut);
- logger.info("Upload completed");
- return true;
+ int statusCode = response.getStatusLine().getStatusCode();
+ if (statusCode >= 200 && statusCode < 300) {
+ logger.info("Upload completed successfully, status: " + statusCode);
+ return true;
+ } else {
+ logger.info("Upload failed with HTTP status: " + statusCode);
+ return false;
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/util/ImageComparisonUtils.java`
around lines 31 - 54, The uploadFile method currently ignores the HttpResponse
from executing the HttpPut; update uploadFile to inspect the response status
(e.g., response.getStatusLine().getStatusCode()) after
httpclient.execute(httpPut) and only return true for successful 2xx codes,
otherwise log the status code and response body (or reason phrase) and return
false; ensure you still catch and log exceptions as before and reference the
HttpResponse variable named response and the uploadFile method when making the
change.
Comment on lines
+32
to
+33
| logger.debug("s3SignedURL - " + s3SignedURL); | ||
| logger.debug("localPath - " + localPath); |
There was a problem hiding this comment.
Presigned S3 URL logged at debug level — leaks temporary credentials.
S3 presigned URLs contain authentication tokens (signature, access key). Logging them — even at debug level — risks credential exposure in log aggregation systems. Log a sanitized form or just the bucket/key path instead.
Proposed fix
- logger.debug("s3SignedURL - " + s3SignedURL);
+ logger.debug("s3SignedURL - " + (s3SignedURL != null ? s3SignedURL.split("\\?")[0] : "null"));
logger.debug("localPath - " + localPath);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| logger.debug("s3SignedURL - " + s3SignedURL); | |
| logger.debug("localPath - " + localPath); | |
| logger.debug("s3SignedURL - " + (s3SignedURL != null ? s3SignedURL.split("\\?")[0] : "null")); | |
| logger.debug("localPath - " + localPath); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/util/ImageComparisonUtils.java`
around lines 32 - 33, The debug logs in ImageComparisonUtils currently print the
full s3SignedURL (logger.debug lines referencing s3SignedURL and localPath)
which leaks presigned URL credentials; change these logger.debug calls to avoid
printing the full URL — instead log a sanitized identifier such as the S3 bucket
and object key or a masked URL (e.g., strip query parameters) and keep localPath
if needed; update the logging statements that reference s3SignedURL to log only
the bucket/key or a masked representation and leave localPath logging unchanged
or minimal.
Comment on lines
+34
to
+65
| public static BufferedImage convert(String text) { | ||
| String safeText = text == null ? "" : text; | ||
| int fontSize = computeFontSize(safeText.length()); | ||
| Font font = new Font(FONT_NAME, Font.PLAIN, fontSize); | ||
|
|
||
| FontMetrics fm = getFontMetrics(font); | ||
| List<String> lines = wrapLines(safeText, fm, MAX_IMAGE_WIDTH - 2 * PADDING); | ||
| int lineHeight = fm.getHeight(); | ||
| int ascent = fm.getAscent(); | ||
| int imageWidth = Math.max(MIN_IMAGE_WIDTH, computeTextWidth(lines, fm) + 2 * PADDING); | ||
| int imageHeight = Math.max(40, lines.size() * lineHeight + 2 * PADDING); | ||
|
|
||
| BufferedImage image = new BufferedImage(imageWidth, imageHeight, BufferedImage.TYPE_INT_RGB); | ||
| Graphics2D g = image.createGraphics(); | ||
| try { | ||
| g.setRenderingHint(RenderingHints.KEY_ANTIALIASING, RenderingHints.VALUE_ANTIALIAS_ON); | ||
| g.setRenderingHint(RenderingHints.KEY_TEXT_ANTIALIASING, RenderingHints.VALUE_TEXT_ANTIALIAS_ON); | ||
| g.setColor(Color.WHITE); | ||
| g.fillRect(0, 0, imageWidth, imageHeight); | ||
| g.setColor(Color.BLACK); | ||
| g.setFont(font); | ||
|
|
||
| int y = PADDING + ascent; | ||
| for (String line : lines) { | ||
| g.drawString(line, PADDING, y); | ||
| y += lineHeight; | ||
| } | ||
| } finally { | ||
| g.dispose(); | ||
| } | ||
| return image; | ||
| } |
There was a problem hiding this comment.
AWT Graphics2D may fail in headless server environments.
This code uses BufferedImage and Graphics2D for text rendering, which requires either a display or headless mode. In server/CI environments (where this addon likely runs), this will throw a HeadlessException unless java.awt.headless=true is set.
Consider adding at the top of convert():
Proposed fix — ensure headless mode
public static BufferedImage convert(String text) {
+ System.setProperty("java.awt.headless", "true");
String safeText = text == null ? "" : text;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public static BufferedImage convert(String text) { | |
| String safeText = text == null ? "" : text; | |
| int fontSize = computeFontSize(safeText.length()); | |
| Font font = new Font(FONT_NAME, Font.PLAIN, fontSize); | |
| FontMetrics fm = getFontMetrics(font); | |
| List<String> lines = wrapLines(safeText, fm, MAX_IMAGE_WIDTH - 2 * PADDING); | |
| int lineHeight = fm.getHeight(); | |
| int ascent = fm.getAscent(); | |
| int imageWidth = Math.max(MIN_IMAGE_WIDTH, computeTextWidth(lines, fm) + 2 * PADDING); | |
| int imageHeight = Math.max(40, lines.size() * lineHeight + 2 * PADDING); | |
| BufferedImage image = new BufferedImage(imageWidth, imageHeight, BufferedImage.TYPE_INT_RGB); | |
| Graphics2D g = image.createGraphics(); | |
| try { | |
| g.setRenderingHint(RenderingHints.KEY_ANTIALIASING, RenderingHints.VALUE_ANTIALIAS_ON); | |
| g.setRenderingHint(RenderingHints.KEY_TEXT_ANTIALIASING, RenderingHints.VALUE_TEXT_ANTIALIAS_ON); | |
| g.setColor(Color.WHITE); | |
| g.fillRect(0, 0, imageWidth, imageHeight); | |
| g.setColor(Color.BLACK); | |
| g.setFont(font); | |
| int y = PADDING + ascent; | |
| for (String line : lines) { | |
| g.drawString(line, PADDING, y); | |
| y += lineHeight; | |
| } | |
| } finally { | |
| g.dispose(); | |
| } | |
| return image; | |
| } | |
| public static BufferedImage convert(String text) { | |
| System.setProperty("java.awt.headless", "true"); | |
| String safeText = text == null ? "" : text; | |
| int fontSize = computeFontSize(safeText.length()); | |
| Font font = new Font(FONT_NAME, Font.PLAIN, fontSize); | |
| FontMetrics fm = getFontMetrics(font); | |
| List<String> lines = wrapLines(safeText, fm, MAX_IMAGE_WIDTH - 2 * PADDING); | |
| int lineHeight = fm.getHeight(); | |
| int ascent = fm.getAscent(); | |
| int imageWidth = Math.max(MIN_IMAGE_WIDTH, computeTextWidth(lines, fm) + 2 * PADDING); | |
| int imageHeight = Math.max(40, lines.size() * lineHeight + 2 * PADDING); | |
| BufferedImage image = new BufferedImage(imageWidth, imageHeight, BufferedImage.TYPE_INT_RGB); | |
| Graphics2D g = image.createGraphics(); | |
| try { | |
| g.setRenderingHint(RenderingHints.KEY_ANTIALIASING, RenderingHints.VALUE_ANTIALIAS_ON); | |
| g.setRenderingHint(RenderingHints.KEY_TEXT_ANTIALIASING, RenderingHints.VALUE_TEXT_ANTIALIAS_ON); | |
| g.setColor(Color.WHITE); | |
| g.fillRect(0, 0, imageWidth, imageHeight); | |
| g.setColor(Color.BLACK); | |
| g.setFont(font); | |
| int y = PADDING + ascent; | |
| for (String line : lines) { | |
| g.drawString(line, PADDING, y); | |
| y += lineHeight; | |
| } | |
| } finally { | |
| g.dispose(); | |
| } | |
| return image; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/util/StringToImageConverter.java`
around lines 34 - 65, The convert(...) method can throw HeadlessException on
servers without a display; to fix it, at the start of
StringToImageConverter.convert(String) check
java.awt.GraphicsEnvironment.isHeadless() and when true call
System.setProperty("java.awt.headless", "true") (or otherwise ensure the JVM is
in headless mode) before any BufferedImage/Graphics2D/Font/FontMetrics usage, so
BufferedImage/Graphics2D creation and text rendering will not fail in CI/server
environments.
Comment on lines
+77
to
+82
| try { | ||
| ssh = new SSHClient(); | ||
| ssh.addHostKeyVerifier(new PromiscuousVerifier()); | ||
| ssh.connect(host, port); | ||
| ssh.authPassword(user, password); | ||
| logger.info("SSH session connected (SSHJ)."); |
There was a problem hiding this comment.
PromiscuousVerifier disables SSH host key verification — vulnerable to MITM attacks.
Accepting any host key means a man-in-the-middle can intercept the SSH connection, capturing credentials and command output. While this may be pragmatic for test automation against ephemeral hosts, it should at minimum be documented as a known risk, and ideally the host fingerprint should be configurable via a test data parameter.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/web/SSHCommandExecutionSSHJ.java`
around lines 77 - 82, The code in SSHCommandExecutionSSHJ currently uses
PromiscuousVerifier (ssh.addHostKeyVerifier(new PromiscuousVerifier())), which
disables host key verification; change this to validate the server host key
against a configurable fingerprint or known-hosts entry: add a new test-data
parameter (e.g., hostFingerprint or knownHosts) and in the
SSHCommandExecutionSSHJ connection logic replace PromiscuousVerifier with a
HostKeyVerifier implementation that checks the remote key's fingerprint against
that parameter (or uses SSHClient.loadKnownHosts()/KnownHostsVerifier if
knownHosts provided), and fail the connection if verification does not match;
document the new parameter in the class comments and ensure
ssh.addHostKeyVerifier is only called with the secure verifier.
Comment on lines
+78
to
+80
| ssh = new SSHClient(); | ||
| ssh.addHostKeyVerifier(new PromiscuousVerifier()); | ||
| ssh.connect(host, port); |
There was a problem hiding this comment.
No connection timeout on ssh.connect() — can hang indefinitely.
If the target host is unreachable or firewalled, ssh.connect() will block with the default socket timeout (infinite). Set a connect timeout before calling connect().
Proposed fix — add before connect
ssh = new SSHClient();
ssh.addHostKeyVerifier(new PromiscuousVerifier());
+ ssh.setConnectTimeout(30_000); // 30 seconds
+ ssh.setTimeout(30_000); // read timeout
ssh.connect(host, port);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ssh = new SSHClient(); | |
| ssh.addHostKeyVerifier(new PromiscuousVerifier()); | |
| ssh.connect(host, port); | |
| ssh = new SSHClient(); | |
| ssh.addHostKeyVerifier(new PromiscuousVerifier()); | |
| ssh.setConnectTimeout(30_000); // 30 seconds | |
| ssh.setTimeout(30_000); // read timeout | |
| ssh.connect(host, port); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/web/SSHCommandExecutionSSHJ.java`
around lines 78 - 80, The SSHClient connection can hang because no connect
timeout is set in SSHCommandExecutionSSHJ; before calling ssh.connect(host,
port) on the SSHClient instance (variable ssh), set a connect timeout (e.g. call
ssh.setConnectTimeout(timeoutMs) with a sensible default like 10_000 or read
from configuration) so the connect attempt will fail fast on
unreachable/firewalled hosts; place this call in the same method (in
SSHCommandExecutionSSHJ) immediately before ssh.connect(...) and ensure the
timeout value is configurable and non-zero.
Comment on lines
+86
to
+103
| StringBuilder output = new StringBuilder(); | ||
| try (Reader reader = new InputStreamReader(cmd.getInputStream())) { | ||
| char[] buf = new char[1024]; | ||
| int n; | ||
| while ((n = reader.read(buf)) != -1) { | ||
| output.append(buf, 0, n); | ||
| } | ||
| } | ||
| cmd.join(30, TimeUnit.SECONDS); | ||
| if (cmd.getExitStatus() != null && cmd.getExitStatus() != 0) { | ||
| try (Reader errReader = new InputStreamReader(cmd.getErrorStream())) { | ||
| char[] buf = new char[1024]; | ||
| int n; | ||
| while ((n = errReader.read(buf)) != -1) { | ||
| output.append(buf, 0, n); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Reading stdout fully before stderr can deadlock on large outputs.
The pattern reads all of stdout (Lines 87–93), then joins (Line 94), then reads stderr (Lines 96–102). If the remote command produces enough output to fill the SSH channel's window/buffer on either stream, the command will block waiting for the buffer to drain, while this code blocks waiting for EOF on the other stream — a classic deadlock.
Consider reading stdout and stderr concurrently (e.g., in separate threads), or use SSHJ's cmd.getInputStream() merged with cmd.getErrorStream() if ordering is acceptable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/web/SSHCommandExecutionSSHJ.java`
around lines 86 - 103, The current logic in SSHCommandExecutionSSHJ reads
cmd.getInputStream() fully and only then reads cmd.getErrorStream(), which can
deadlock for large outputs; update the command execution to consume stdout and
stderr concurrently (for example spawn two threads/tasks or use an executor to
stream-copy both cmd.getInputStream() and cmd.getErrorStream() into buffers
simultaneously) and then wait for cmd.join(...) and the two reader tasks to
finish before checking cmd.getExitStatus(); refer to the cmd variable and the
surrounding command-execution method in SSHCommandExecutionSSHJ to locate where
to replace the sequential Reader reads with concurrent stream gobblers and
ensure thread-safe aggregation into the final output.
Comment on lines
+135
to
+142
| } finally { | ||
| try { | ||
| if (session != null) session.close(); | ||
| if (ssh != null) ssh.disconnect(); | ||
| } catch (IOException e) { | ||
| logger.info("Error closing SSHJ resources: " + e.getMessage()); | ||
| } | ||
| } |
There was a problem hiding this comment.
If session.close() throws, ssh.disconnect() is skipped — connection leak.
Both resources should be closed independently. Use separate try-catch blocks or a utility method to ensure both are cleaned up.
Proposed fix
} finally {
- try {
- if (session != null) session.close();
- if (ssh != null) ssh.disconnect();
- } catch (IOException e) {
- logger.info("Error closing SSHJ resources: " + e.getMessage());
- }
+ if (session != null) {
+ try { session.close(); } catch (IOException e) {
+ logger.info("Error closing SSHJ session: " + e.getMessage());
+ }
+ }
+ if (ssh != null) {
+ try { ssh.disconnect(); } catch (IOException e) {
+ logger.info("Error disconnecting SSHJ client: " + e.getMessage());
+ }
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| } finally { | |
| try { | |
| if (session != null) session.close(); | |
| if (ssh != null) ssh.disconnect(); | |
| } catch (IOException e) { | |
| logger.info("Error closing SSHJ resources: " + e.getMessage()); | |
| } | |
| } | |
| } finally { | |
| if (session != null) { | |
| try { session.close(); } catch (IOException e) { | |
| logger.info("Error closing SSHJ session: " + e.getMessage()); | |
| } | |
| } | |
| if (ssh != null) { | |
| try { ssh.disconnect(); } catch (IOException e) { | |
| logger.info("Error disconnecting SSHJ client: " + e.getMessage()); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/web/SSHCommandExecutionSSHJ.java`
around lines 135 - 142, The finally block in SSHCommandExecutionSSHJ currently
closes session and ssh in the same try, so if session.close() throws,
ssh.disconnect() is skipped; change the cleanup to close each resource in its
own try-catch (e.g., try { if (session != null) session.close(); } catch
(IOException e) { logger.info(...); } and then a separate try { if (ssh != null)
ssh.disconnect(); } catch (IOException e) { logger.info(...); }) so both
session.close() and ssh.disconnect() are attempted independently and both
exceptions are logged.
| @@ -0,0 +1 @@ | |||
| testsigma-sdk.api.key=eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiIyNDBkMjhiNS0xNmUwLThlNmYtOWQ0ZS05MjYxMGNiZTcyYzciLCJ1bmlxdWVJZCI6IjU5NzgiLCJpZGVudGl0eUFjY291bnRVVUlkIjoiNDMifQ.eX8QdtK6wAKQJ2rI3Em8VbztUA2m06VFbSLJ35vsNsiljS7yrqGjIZ3TjxdXL3dvscC-ZcU21ugKIjqyWmstkQ No newline at end of file | |||
There was a problem hiding this comment.
🚨 Hardcoded JWT API key committed to source control — secret leak.
This file contains a real JWT token (eyJhbGciOiJIUzUxMiJ9.eyJzdWIi...) that will be visible in the public repository history. This is a serious security vulnerability:
- Revoke this token immediately — it's already exposed in the PR diff.
- Remove this file from the repository and purge it from Git history (e.g.,
git filter-repo). - Use environment variables or a CI/CD secrets mechanism to inject the API key at build/test time, not a committed properties file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@ssh_shell_command_executor_using_sshj/src/main/resources/testsigma-sdk.properties`
at line 1, The committed file contains a hardcoded JWT value under the property
key "testsigma-sdk.api.key" which must be removed: revoke the exposed token
immediately, delete the properties file from the repo, purge it from Git history
(e.g., git filter-repo or BFG) and add the filename to .gitignore; then change
the configuration to read testsigma-sdk.api.key from an environment variable or
CI secret at runtime/CI (update the code that reads the property to fall back to
ENV if present) and ensure CI/CD secrets injection is configured to supply the
API key instead of committing it.
vignesh-testsigma
approved these changes
Feb 19, 2026
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (6)
ssh_shell_command_executor_using_sshj/pom.xml (2)
42-46: TestNG dependency is missing<scope>test</scope>.This was already flagged — without test scope, TestNG and its transitive dependencies will be bundled into the shaded JAR, bloating the artifact and risking classpath conflicts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ssh_shell_command_executor_using_sshj/pom.xml` around lines 42 - 46, The TestNG dependency declared (groupId org.testng, artifactId testng, version 6.14.3) is missing a test scope; update the pom.xml dependency block for org.testng:testng to include <scope>test</scope> so TestNG and its transitive dependencies are not included in the shaded/packaged artifact.
59-82: Jackson version mismatch betweenjackson-annotations(2.13.0) andjackson-databind(2.12.3).This was already flagged — mismatched minor versions can cause
NoSuchMethodErrororIncompatibleClassChangeErrorat runtime. Align both to the same release.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ssh_shell_command_executor_using_sshj/pom.xml` around lines 59 - 82, The pom declares jackson-annotations:2.13.0 and jackson-databind:2.12.3 which is a mismatched Jackson minor version and can cause runtime errors; update the Jackson artifacts so they use the same release—either change the jackson-databind dependency version to 2.13.0 to match jackson-annotations, or upgrade both jackson-annotations and jackson-databind (and any other Jackson modules) to the same newer stable version; target the artifactIds jackson-annotations and jackson-databind in the pom.xml when making the change.ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/android/SSHCommandExecutionSSHJ.java (1)
60-143: Same issues as flagged in the iOS variant apply here.The
PromiscuousVerifiersecurity concern, resource leak infinally, screenshot failure propagation,deleteOnExitusage, and potential NPE oncommandSeparator.getValue()all apply identically to this file. Please apply the same fixes here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/android/SSHCommandExecutionSSHJ.java` around lines 60 - 143, The execute() method uses PromiscuousVerifier (replace with proper host key verification or make it configurable), dereferences commandSeparator.getValue() without null checks (guard commandSeparator and its value before using or default safely), and incorrectly handles resources and screenshot errors: ensure ssh and session are closed safely using try-with-resources or null-check close in finally, propagate/handle failures from StringToImageConverter.convertToFile and ImageComparisonUtils.uploadFile instead of swallowing them, and avoid deleteOnExit() by deleting the temp screenshotFile immediately after upload/usage; update references in this method (PromiscuousVerifier, commandSeparator, wrapInLoginShell, session, ssh, StringToImageConverter.convertToFile, ImageComparisonUtils.uploadFile, screenshotFile) accordingly and set runTimeData/setErrorMessage consistently on failures.ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/windowsAdvanced/SSHCommandExecutionSSHJ.java (1)
58-142: Same issues as flagged in the iOS variant apply here.The
PromiscuousVerifiersecurity concern, resource leak infinally, screenshot failure propagation,deleteOnExitusage, and potential NPE oncommandSeparator.getValue()all apply identically to this file. Please apply the same fixes here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/windowsAdvanced/SSHCommandExecutionSSHJ.java` around lines 58 - 142, The execute() method uses PromiscuousVerifier (replace with a secure HostKeyVerifier implementation), dereferences commandSeparator.getValue() without null checks (guard commandSeparator and its getValue() before calling toString()), and mismanages resources and error handling: ensure SSHClient ssh and Session session are closed safely (use try-with-resources or null-checked close inside finally and handle IOExceptions), propagate screenshot upload failures to the step result (if ImageComparisonUtils.uploadFile(...) returns false, setErrorMessage and return Result.FAILED), and avoid deleteOnExit for temporary files created by StringToImageConverter.convertToFile (delete immediately with Files.deleteIfExists or schedule try-finally deletion after upload). Reference symbols: PromiscuousVerifier, commandSeparator.getValue(), execute(), ssh, session, StringToImageConverter.convertToFile, ImageComparisonUtils.uploadFile, setErrorMessage.ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/mobileWeb/SSHCommandExecutionSSHJ.java (1)
60-143: Same issues as flagged in the iOS variant apply here.The
PromiscuousVerifiersecurity concern, resource leak infinally, screenshot failure propagation,deleteOnExitusage, and potential NPE oncommandSeparator.getValue()all apply identically to this file. Please apply the same fixes here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/mobileWeb/SSHCommandExecutionSSHJ.java` around lines 60 - 143, The execute() method repeats the same issues: replace the insecure PromiscuousVerifier usage in SSHClient setup with a proper HostKeyVerifier (or load known_hosts) instead of new PromiscuousVerifier; make commandSeparator access null-safe (check commandSeparator != null && commandSeparator.getValue() != null before calling toString); avoid deleteOnExit and instead delete the temp screenshot file immediately with Files.deleteIfExists(screenshotFile.toPath()) in a finally block; ensure SSHClient and Session are closed robustly (use try-with-resources or null-check close with separate try/catch to avoid resource leaks rather than swallowing exceptions in the outer finally); and do not silently swallow failures from StringToImageConverter.convertToFile or uploadFile — catch/handle those exceptions and set failure result or at least log and attach the error to the step (use StringToImageConverter.convertToFile and ImageComparisonUtils.uploadFile references to locate the code to change).ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/salesforce/SSHCommandExecutionSSHJ.java (1)
61-144: Same issues as flagged in the iOS variant apply here.The
PromiscuousVerifiersecurity concern, resource leak infinally, screenshot failure propagation,deleteOnExitusage, and potential NPE oncommandSeparator.getValue()all apply identically to this file. Please apply the same fixes here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/salesforce/SSHCommandExecutionSSHJ.java` around lines 61 - 144, The execute() method uses PromiscuousVerifier (replace with proper host key verification or configurable verifier), reads commandSeparator.getValue() without null checks (null-safe check commandSeparator and its getValue() before calling toString()), leaks resources and swallows close errors (ensure session and SSHClient are closed in a safe try-with-resources or explicit close with null checks and log full stack traces on IOException instead of ignoring), avoid deleteOnExit for screenshotFile (delete immediately after upload attempt and handle failures), and propagate or record screenshot conversion/upload failures from StringToImageConverter.convertToFile and ImageComparisonUtils.uploadFile instead of silently continuing; update logic around screenshotFile to delete in finally only after ensuring upload completed/handled and log or setErrorMessage when conversion/upload fails (references: execute(), PromiscuousVerifier, commandSeparator, session, ssh, StringToImageConverter.convertToFile, ImageComparisonUtils.uploadFile, screenshotFile, deleteOnExit).
🧹 Nitpick comments (1)
ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/ios/SSHCommandExecutionSSHJ.java (1)
1-149: Massive code duplication across 6 platform-specific classes.The
execute()method, field declarations,wrapInLoginShell(), and all internal logic are identical across Android, iOS, Web, MobileWeb, Salesforce, and WindowsAdvanced variants. The only differences are:
- The base class (
IOSAction,AndroidAction,WebAction,WindowsAdvancedAction)- The
ApplicationTypeenum value- The log message string
Consider extracting the SSH execution logic into a shared helper class (e.g.,
SSHCommandExecutor) that each platform variant delegates to. This would make bug fixes (like the ones flagged above) a single-point change rather than requiring updates in 6 files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/ios/SSHCommandExecutionSSHJ.java` around lines 1 - 149, The execute() implementation, fields (hostName, portNumber, userName, userPassword, commands, commandSeparator, storeVariable, runTimeData), and helper wrapInLoginShell() in SSHCommandExecutionSSHJ are duplicated across platform-specific classes; extract the SSH logic into a shared helper class (e.g., SSHCommandExecutor) that exposes a method like executeCommands(Host, Port, User, Password, Commands, CommandSeparator, RunTimeData, TestStepResult, Logger, Driver) and a static helper wrapInLoginShell(String), move all SSH setup/teardown, output collection, runtime-data setting and screenshot handling into that class, then update SSHCommandExecutionSSHJ.execute() to call SSHCommandExecutor.executeCommands(...) and keep only platform-specific bits (base class type, ApplicationType and the log message) in the platform class.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/ios/SSHCommandExecutionSSHJ.java`:
- Around line 78-82: Replace the insecure PromiscuousVerifier usage in
SSHCommandExecutionSSHJ: instead of unconditionally calling
ssh.addHostKeyVerifier(new PromiscuousVerifier()) inside the try block, add
support for a configurable known_hosts path or a boolean flag to enable strict
host-key checking and load host keys into SSHClient (refer to
ssh.addHostKeyVerifier and SSHClient), and if strict checking is disabled emit a
prominent warning to logs that host-key verification is turned off; make the
known_hosts path an optional parameter or config so callers can opt into secure
verification across all platform variants.
- Around line 122-126: In SSHCommandExecutionSSHJ replace the use of
screenshotFile.deleteOnExit() inside the finally block with an immediate
deletion call (screenshotFile.delete()) so temporary screenshot files are
cleaned up immediately; locate the finally block that references screenshotFile
(in methods that capture screenshots) and change deleteOnExit() to delete(),
optionally check the boolean result and log a warning via the class logger if
deletion fails to aid debugging.
- Around line 68-71: The current logic for building commandSeparatorStr and
allCommands calls commandSeparator.getValue().toString() and
commands.getValue().toString() without guarding against getValue() returning
null; update the code around the symbols commandSeparator, commandSeparatorStr,
commands, and allCommands to first check that commandSeparator.getValue() and
commands.getValue() are non-null (or use Optional) before calling toString(),
and fall back to the default separator "&&" and empty/validated commands string
if null so no NPE is thrown when getValue() is null.
- Around line 135-142: In SSHCommandExecutionSSHJ's finally block, close session
and ssh independently so one failing close doesn't skip the other: replace the
single try that calls session.close() then ssh.disconnect() with separate
try-catch blocks (or individual try-finally) for session.close() and for
ssh.disconnect(), each checking null and logging IOExceptions, ensuring
ssh.disconnect() always runs even if session.close() throws.
- Around line 109-126: The screenshot creation/upload block (starting at
StringToImageConverter.convertToFile(output)) can throw and currently propagates
to the outer catch, failing the whole SSH command; wrap the entire screenshot
workflow (StringToImageConverter.convertToFile, ImageComparisonUtils
imageComparisonUtils and uploadFile, and the deleteOnExit cleanup) in its own
try-catch that catches Exception, logs the error via logger.debug/error with
context (e.g., "screenshot creation/upload failed") and ensures any created
screenshotFile is cleaned up in a finally inside that block so execution
continues to treat the SSH command as successful even if image generation/upload
fails.
---
Duplicate comments:
In `@ssh_shell_command_executor_using_sshj/pom.xml`:
- Around line 42-46: The TestNG dependency declared (groupId org.testng,
artifactId testng, version 6.14.3) is missing a test scope; update the pom.xml
dependency block for org.testng:testng to include <scope>test</scope> so TestNG
and its transitive dependencies are not included in the shaded/packaged
artifact.
- Around line 59-82: The pom declares jackson-annotations:2.13.0 and
jackson-databind:2.12.3 which is a mismatched Jackson minor version and can
cause runtime errors; update the Jackson artifacts so they use the same
release—either change the jackson-databind dependency version to 2.13.0 to match
jackson-annotations, or upgrade both jackson-annotations and jackson-databind
(and any other Jackson modules) to the same newer stable version; target the
artifactIds jackson-annotations and jackson-databind in the pom.xml when making
the change.
In
`@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/android/SSHCommandExecutionSSHJ.java`:
- Around line 60-143: The execute() method uses PromiscuousVerifier (replace
with proper host key verification or make it configurable), dereferences
commandSeparator.getValue() without null checks (guard commandSeparator and its
value before using or default safely), and incorrectly handles resources and
screenshot errors: ensure ssh and session are closed safely using
try-with-resources or null-check close in finally, propagate/handle failures
from StringToImageConverter.convertToFile and ImageComparisonUtils.uploadFile
instead of swallowing them, and avoid deleteOnExit() by deleting the temp
screenshotFile immediately after upload/usage; update references in this method
(PromiscuousVerifier, commandSeparator, wrapInLoginShell, session, ssh,
StringToImageConverter.convertToFile, ImageComparisonUtils.uploadFile,
screenshotFile) accordingly and set runTimeData/setErrorMessage consistently on
failures.
In
`@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/mobileWeb/SSHCommandExecutionSSHJ.java`:
- Around line 60-143: The execute() method repeats the same issues: replace the
insecure PromiscuousVerifier usage in SSHClient setup with a proper
HostKeyVerifier (or load known_hosts) instead of new PromiscuousVerifier; make
commandSeparator access null-safe (check commandSeparator != null &&
commandSeparator.getValue() != null before calling toString); avoid deleteOnExit
and instead delete the temp screenshot file immediately with
Files.deleteIfExists(screenshotFile.toPath()) in a finally block; ensure
SSHClient and Session are closed robustly (use try-with-resources or null-check
close with separate try/catch to avoid resource leaks rather than swallowing
exceptions in the outer finally); and do not silently swallow failures from
StringToImageConverter.convertToFile or uploadFile — catch/handle those
exceptions and set failure result or at least log and attach the error to the
step (use StringToImageConverter.convertToFile and
ImageComparisonUtils.uploadFile references to locate the code to change).
In
`@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/salesforce/SSHCommandExecutionSSHJ.java`:
- Around line 61-144: The execute() method uses PromiscuousVerifier (replace
with proper host key verification or configurable verifier), reads
commandSeparator.getValue() without null checks (null-safe check
commandSeparator and its getValue() before calling toString()), leaks resources
and swallows close errors (ensure session and SSHClient are closed in a safe
try-with-resources or explicit close with null checks and log full stack traces
on IOException instead of ignoring), avoid deleteOnExit for screenshotFile
(delete immediately after upload attempt and handle failures), and propagate or
record screenshot conversion/upload failures from
StringToImageConverter.convertToFile and ImageComparisonUtils.uploadFile instead
of silently continuing; update logic around screenshotFile to delete in finally
only after ensuring upload completed/handled and log or setErrorMessage when
conversion/upload fails (references: execute(), PromiscuousVerifier,
commandSeparator, session, ssh, StringToImageConverter.convertToFile,
ImageComparisonUtils.uploadFile, screenshotFile, deleteOnExit).
In
`@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/windowsAdvanced/SSHCommandExecutionSSHJ.java`:
- Around line 58-142: The execute() method uses PromiscuousVerifier (replace
with a secure HostKeyVerifier implementation), dereferences
commandSeparator.getValue() without null checks (guard commandSeparator and its
getValue() before calling toString()), and mismanages resources and error
handling: ensure SSHClient ssh and Session session are closed safely (use
try-with-resources or null-checked close inside finally and handle
IOExceptions), propagate screenshot upload failures to the step result (if
ImageComparisonUtils.uploadFile(...) returns false, setErrorMessage and return
Result.FAILED), and avoid deleteOnExit for temporary files created by
StringToImageConverter.convertToFile (delete immediately with
Files.deleteIfExists or schedule try-finally deletion after upload). Reference
symbols: PromiscuousVerifier, commandSeparator.getValue(), execute(), ssh,
session, StringToImageConverter.convertToFile, ImageComparisonUtils.uploadFile,
setErrorMessage.
---
Nitpick comments:
In
`@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/ios/SSHCommandExecutionSSHJ.java`:
- Around line 1-149: The execute() implementation, fields (hostName, portNumber,
userName, userPassword, commands, commandSeparator, storeVariable, runTimeData),
and helper wrapInLoginShell() in SSHCommandExecutionSSHJ are duplicated across
platform-specific classes; extract the SSH logic into a shared helper class
(e.g., SSHCommandExecutor) that exposes a method like executeCommands(Host,
Port, User, Password, Commands, CommandSeparator, RunTimeData, TestStepResult,
Logger, Driver) and a static helper wrapInLoginShell(String), move all SSH
setup/teardown, output collection, runtime-data setting and screenshot handling
into that class, then update SSHCommandExecutionSSHJ.execute() to call
SSHCommandExecutor.executeCommands(...) and keep only platform-specific bits
(base class type, ApplicationType and the log message) in the platform class.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
ssh_shell_command_executor_using_sshj/pom.xmlssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/android/SSHCommandExecutionSSHJ.javassh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/ios/SSHCommandExecutionSSHJ.javassh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/mobileWeb/SSHCommandExecutionSSHJ.javassh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/salesforce/SSHCommandExecutionSSHJ.javassh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/windowsAdvanced/SSHCommandExecutionSSHJ.java
Comment on lines
+68
to
+71
| String commandSeparatorStr = commandSeparator != null && !commandSeparator.getValue().toString().isEmpty() | ||
| ? commandSeparator.getValue().toString() | ||
| : "&&"; | ||
| String allCommands = commands.getValue().toString().replace(commandSeparatorStr, "&&"); |
There was a problem hiding this comment.
Potential NPE when commandSeparator is non-null but getValue() returns null.
Line 68 checks commandSeparator != null but then calls commandSeparator.getValue().toString() — if getValue() returns null, this throws NPE. Also on Line 71, if commands.getValue() returns null the same issue occurs. This applies to all platform variants.
Proposed fix
- String commandSeparatorStr = commandSeparator != null && !commandSeparator.getValue().toString().isEmpty()
+ String commandSeparatorStr = commandSeparator != null && commandSeparator.getValue() != null
+ && !commandSeparator.getValue().toString().isEmpty()
? commandSeparator.getValue().toString()
: "&&";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| String commandSeparatorStr = commandSeparator != null && !commandSeparator.getValue().toString().isEmpty() | |
| ? commandSeparator.getValue().toString() | |
| : "&&"; | |
| String allCommands = commands.getValue().toString().replace(commandSeparatorStr, "&&"); | |
| String commandSeparatorStr = commandSeparator != null && commandSeparator.getValue() != null | |
| && !commandSeparator.getValue().toString().isEmpty() | |
| ? commandSeparator.getValue().toString() | |
| : "&&"; | |
| String allCommands = commands.getValue().toString().replace(commandSeparatorStr, "&&"); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/ios/SSHCommandExecutionSSHJ.java`
around lines 68 - 71, The current logic for building commandSeparatorStr and
allCommands calls commandSeparator.getValue().toString() and
commands.getValue().toString() without guarding against getValue() returning
null; update the code around the symbols commandSeparator, commandSeparatorStr,
commands, and allCommands to first check that commandSeparator.getValue() and
commands.getValue() are non-null (or use Optional) before calling toString(),
and fall back to the default separator "&&" and empty/validated commands string
if null so no NPE is thrown when getValue() is null.
Comment on lines
+78
to
+82
| try { | ||
| ssh = new SSHClient(); | ||
| ssh.addHostKeyVerifier(new PromiscuousVerifier()); | ||
| ssh.connect(host, port); | ||
| ssh.authPassword(user, password); |
There was a problem hiding this comment.
PromiscuousVerifier disables SSH host-key verification — MITM risk.
This addon is intended to be published as PUBLIC. Using PromiscuousVerifier means any host key is silently accepted, making the connection vulnerable to man-in-the-middle attacks. An attacker on the network path can intercept credentials and command output.
At minimum, consider adding a known_hosts file path as an optional parameter, or log a prominent warning that host-key verification is disabled. This applies to all platform variants (Android, iOS, Web, MobileWeb, Salesforce, WindowsAdvanced).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/ios/SSHCommandExecutionSSHJ.java`
around lines 78 - 82, Replace the insecure PromiscuousVerifier usage in
SSHCommandExecutionSSHJ: instead of unconditionally calling
ssh.addHostKeyVerifier(new PromiscuousVerifier()) inside the try block, add
support for a configurable known_hosts path or a boolean flag to enable strict
host-key checking and load host keys into SSHClient (refer to
ssh.addHostKeyVerifier and SSHClient), and if strict checking is disabled emit a
prominent warning to logs that host-key verification is turned off; make the
known_hosts path an optional parameter or config so callers can opt into secure
verification across all platform variants.
Comment on lines
+109
to
+126
| File screenshotFile = null; | ||
| try { | ||
| screenshotFile = StringToImageConverter.convertToFile(output); | ||
| String s3Url = testStepResult != null ? testStepResult.getScreenshotUrl() : null; | ||
| if (s3Url != null && !s3Url.isEmpty()) { | ||
| ImageComparisonUtils imageComparisonUtils = new ImageComparisonUtils(driver, logger); | ||
| boolean uploadResult = imageComparisonUtils.uploadFile(s3Url, screenshotFile.getAbsolutePath()); | ||
| if (!uploadResult) { | ||
| logger.debug("Error uploading custom screenshot to S3; step result may not show the output image."); | ||
| } else { | ||
| logger.debug("Custom screenshot (command output) uploaded successfully."); | ||
| } | ||
| } | ||
| } finally { | ||
| if (screenshotFile != null && screenshotFile.exists()) { | ||
| screenshotFile.deleteOnExit(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Screenshot failure will fail the entire SSH command execution.
If StringToImageConverter.convertToFile(output) throws an exception on Line 111 (e.g., for very large output or rendering issues), it propagates to the outer catch (Exception e) block on Line 130, causing the step to return Result.FAILED — even though the SSH command itself completed successfully and the output was captured.
Wrap the screenshot block in its own try-catch that logs the error and continues to the success path. This applies to all platform variants.
Proposed fix
- File screenshotFile = null;
- try {
- screenshotFile = StringToImageConverter.convertToFile(output);
- String s3Url = testStepResult != null ? testStepResult.getScreenshotUrl() : null;
- if (s3Url != null && !s3Url.isEmpty()) {
- ImageComparisonUtils imageComparisonUtils = new ImageComparisonUtils(driver, logger);
- boolean uploadResult = imageComparisonUtils.uploadFile(s3Url, screenshotFile.getAbsolutePath());
- if (!uploadResult) {
- logger.debug("Error uploading custom screenshot to S3; step result may not show the output image.");
- } else {
- logger.debug("Custom screenshot (command output) uploaded successfully.");
+ try {
+ File screenshotFile = null;
+ try {
+ screenshotFile = StringToImageConverter.convertToFile(output);
+ String s3Url = testStepResult != null ? testStepResult.getScreenshotUrl() : null;
+ if (s3Url != null && !s3Url.isEmpty()) {
+ ImageComparisonUtils imageComparisonUtils = new ImageComparisonUtils(driver, logger);
+ boolean uploadResult = imageComparisonUtils.uploadFile(s3Url, screenshotFile.getAbsolutePath());
+ if (!uploadResult) {
+ logger.debug("Error uploading custom screenshot to S3; step result may not show the output image.");
+ } else {
+ logger.debug("Custom screenshot (command output) uploaded successfully.");
+ }
}
- }
- } finally {
- if (screenshotFile != null && screenshotFile.exists()) {
- screenshotFile.deleteOnExit();
+ } finally {
+ if (screenshotFile != null && screenshotFile.exists()) {
+ screenshotFile.delete();
+ }
}
+ } catch (Exception screenshotEx) {
+ logger.debug("Failed to generate/upload custom screenshot: " + screenshotEx.getMessage());
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/ios/SSHCommandExecutionSSHJ.java`
around lines 109 - 126, The screenshot creation/upload block (starting at
StringToImageConverter.convertToFile(output)) can throw and currently propagates
to the outer catch, failing the whole SSH command; wrap the entire screenshot
workflow (StringToImageConverter.convertToFile, ImageComparisonUtils
imageComparisonUtils and uploadFile, and the deleteOnExit cleanup) in its own
try-catch that catches Exception, logs the error via logger.debug/error with
context (e.g., "screenshot creation/upload failed") and ensures any created
screenshotFile is cleaned up in a finally inside that block so execution
continues to treat the SSH command as successful even if image generation/upload
fails.
Comment on lines
+122
to
+126
| } finally { | ||
| if (screenshotFile != null && screenshotFile.exists()) { | ||
| screenshotFile.deleteOnExit(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Use delete() instead of deleteOnExit() for temp files.
deleteOnExit() only schedules deletion at JVM shutdown. In a long-running test agent, temporary screenshot files will accumulate on disk for the lifetime of the process. Since you're already in a finally block, use screenshotFile.delete() for immediate cleanup. This applies to all platform variants.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/ios/SSHCommandExecutionSSHJ.java`
around lines 122 - 126, In SSHCommandExecutionSSHJ replace the use of
screenshotFile.deleteOnExit() inside the finally block with an immediate
deletion call (screenshotFile.delete()) so temporary screenshot files are
cleaned up immediately; locate the finally block that references screenshotFile
(in methods that capture screenshots) and change deleteOnExit() to delete(),
optionally check the boolean result and log a warning via the class logger if
deletion fails to aid debugging.
Comment on lines
+135
to
+142
| } finally { | ||
| try { | ||
| if (session != null) session.close(); | ||
| if (ssh != null) ssh.disconnect(); | ||
| } catch (IOException e) { | ||
| logger.info("Error closing SSHJ resources: " + e.getMessage()); | ||
| } | ||
| } |
There was a problem hiding this comment.
If session.close() throws, ssh.disconnect() is skipped — resource leak.
When session.close() throws an IOException, the catch block catches it and ssh.disconnect() is never called, leaking the SSH connection. Close each resource independently. This applies to all platform variants.
Proposed fix
} finally {
- try {
- if (session != null) session.close();
- if (ssh != null) ssh.disconnect();
- } catch (IOException e) {
- logger.info("Error closing SSHJ resources: " + e.getMessage());
+ if (session != null) {
+ try { session.close(); } catch (IOException e) {
+ logger.info("Error closing SSHJ session: " + e.getMessage());
+ }
+ }
+ if (ssh != null) {
+ try { ssh.disconnect(); } catch (IOException e) {
+ logger.info("Error disconnecting SSHJ client: " + e.getMessage());
+ }
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| } finally { | |
| try { | |
| if (session != null) session.close(); | |
| if (ssh != null) ssh.disconnect(); | |
| } catch (IOException e) { | |
| logger.info("Error closing SSHJ resources: " + e.getMessage()); | |
| } | |
| } | |
| } finally { | |
| if (session != null) { | |
| try { session.close(); } catch (IOException e) { | |
| logger.info("Error closing SSHJ session: " + e.getMessage()); | |
| } | |
| } | |
| if (ssh != null) { | |
| try { ssh.disconnect(); } catch (IOException e) { | |
| logger.info("Error disconnecting SSHJ client: " + e.getMessage()); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@ssh_shell_command_executor_using_sshj/src/main/java/com/testsigma/addons/ios/SSHCommandExecutionSSHJ.java`
around lines 135 - 142, In SSHCommandExecutionSSHJ's finally block, close
session and ssh independently so one failing close doesn't skip the other:
replace the single try that calls session.close() then ssh.disconnect() with
separate try-catch blocks (or individual try-finally) for session.close() and
for ssh.disconnect(), each checking null and logging IOExceptions, ensuring
ssh.disconnect() always runs even if session.close() throws.
vignesh-testsigma
approved these changes
Feb 24, 2026
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.
please review this addon and publish as PUBLIC
Addon name : ssh_shell_command_executor_using_sshj
Addon accont: https://jarvis.testsigma.com/ui/tenants/2817/addons
Jira: https://testsigma.atlassian.net/browse/CUS-10700
fix
Summary by CodeRabbit
New Features
Chores