Skip to content

<fix>[core]: add SshFileChecker #4649

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
zstack-robot-2 wants to merge 1 commit into
base: 4.8.0
Choose a base branch
from
Open

<fix>[core]: add SshFileChecker #4649

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 99 additions & 0 deletions core/src/main/java/org/zstack/core/ansible/SshFileChecker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
package org.zstack.core.ansible;

import org.apache.logging.log4j.util.Strings;
import org.zstack.utils.Utils;
import org.zstack.utils.logging.CLogger;
import org.zstack.utils.ssh.Ssh;
import org.zstack.utils.ssh.SshResult;

import java.io.File;

public class SshFileChecker implements AnsibleChecker {
private static final CLogger logger = Utils.getLogger(SshFileChecker.class);

private String filePath;
private String username;
private String password;
private String privateKey;
private String targetIp;
private int sshPort = 22;

@Override
public boolean needDeploy() {
if (Strings.isEmpty(filePath)) {
return false;
}

Ssh ssh = new Ssh();
ssh.setUsername(username).setPrivateKey(privateKey).setPassword(password).setPort(sshPort).setHostname(targetIp);

try {
String dirPath = new File(filePath).getParent();

ssh.command(String.format("if [ -d %s ] && [ -f %s ]; then exit 0; else exit 1; fi", dirPath, filePath));

@coderabbitai coderabbitai Bot Jul 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win

对远端路径进行 POSIX shell 转义，避免远程命令注入。

Line 33 直接将可通过 setFilePath() 设置的值拼入 shell；包含分号、命令替换或空格的路径可执行额外远端命令，普通含空格路径也会误判。请对每个参数做单引号转义后再构造命令。

建议修改 
-            ssh.command(String.format("if [ -d %s ] && [ -f %s ]; then exit 0; else exit 1; fi", dirPath, filePath));
+            ssh.command(String.format("if [ -d %s ] && [ -f %s ]; then exit 0; else exit 1; fi",
+                    quoteForPosixShell(dirPath), quoteForPosixShell(filePath)));
private static String quoteForPosixShell(String value) {
    return "'" + value.replace("'", "'\"'\"'") + "'";
}
📝 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.command(String.format("if [ -d %s ] && [ -f %s ]; then exit 0; else exit 1; fi", dirPath, filePath));
ssh.command(String.format("if [ -d %s ] && [ -f %s ]; then exit 0; else exit 1; fi",
quoteForPosixShell(dirPath), quoteForPosixShell(filePath)));
private static String quoteForPosixShell(String value) {
return "'" + value.replace("'", "'\"'\"'") + "'";
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@core/src/main/java/org/zstack/core/ansible/SshFileChecker.java` at line 33,
Update the command construction in SshFileChecker to POSIX-shell-quote both
dirPath and filePath before interpolation, using the standard single-quote
escaping that safely handles embedded quotes, spaces, substitutions, and shell
metacharacters. Add or reuse a helper such as quoteForPosixShell and preserve
the existing directory/file checks.

SshResult ret = ssh.run();
if (ret.getReturnCode() != 0) {
return true;
}

ssh.reset();
} finally {
ssh.close();
}

return false;
}

@Override
public void deleteDestFile() {
// do nothing.
}
Comment on lines +48 to +50

@coderabbitai coderabbitai Bot Jul 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

失败后删除远端残留文件。

AnsibleRunner.cleanup() 会在 playbook 失败后调用此方法，而 needDeploy() 只要发现该文件存在就返回 false。当前空实现会保留可能不完整的 filePath，使下一次调用错误跳过部署。请使用相同 SSH 配置以安全转义的路径执行远端文件删除（不删除父目录），并记录清理失败。

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@core/src/main/java/org/zstack/core/ansible/SshFileChecker.java` around lines
48 - 50, Implement deleteDestFile() in SshFileChecker to remove the remote
filePath after a failed deployment, reusing the existing SSH configuration and
safely escaping the path without deleting its parent directory. Record any
cleanup failure while preserving the method’s current contract.


public String getUsername() {
return username;
}

public void setUsername(String username) {
this.username = username;
}

public String getPassword() {
return password;
}

public void setPassword(String password) {
this.password = password;
}

public String getPrivateKey() {
return privateKey;
}

public void setPrivateKey(String privateKey) {
this.privateKey = privateKey;
}

public int getSshPort() {
return sshPort;
}

public void setSshPort(int sshPort) {
this.sshPort = sshPort;
}

public String getTargetIp() {
return targetIp;
}

public void setTargetIp(String targetIp) {
this.targetIp = targetIp;
}

public String getFilePath() {
return filePath;
}

public void setFilePath(String filePath) {
this.filePath = filePath;
}
}