Open
Conversation
Walkthrough为备份存储主机新增一组文件操作消息与回复类型，并在 Ceph 插件、KVM、SDK、测试模拟器和路径校验工具中实现相应处理、模拟端点与校验逻辑以支持上传/下载/删除/解压/升级/进度/取消等操作。 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MessageBus
participant BackupStorageBase
participant CephBackupStorageBase
participant CephAgent
Client->>MessageBus: UploadFileToBackupStorageHostMsg
MessageBus->>BackupStorageBase: handleLocalMessage()
BackupStorageBase->>CephBackupStorageBase: handle(UploadFileToBackupStorageHostMsg)
CephBackupStorageBase->>CephBackupStorageBase: validateUrl(), selectMonByUuid()
alt scheme == "upload"
CephBackupStorageBase->>CephAgent: POST /ceph/file/upload (UploadFileCmd, timeout rgba(0,128,0,0.5))
else
CephBackupStorageBase->>CephAgent: POST /ceph/file/download (DownloadFileCmd, timeout rgba(0,128,0,0.5))
end
CephAgent-->>CephBackupStorageBase: Upload/Download Response
CephBackupStorageBase-->>Client: UploadFileToBackupStorageHostReply
sequenceDiagram
participant Client
participant MessageBus
participant BackupStorageBase
participant CephBackupStorageBase
participant CephAgent
Client->>MessageBus: GetFileDownloadProgressFromBackupStorageHostMsg
MessageBus->>BackupStorageBase: handleLocalMessage()
BackupStorageBase->>CephBackupStorageBase: handle(GetFileDownloadProgressFromBackupStorageHostMsg)
CephBackupStorageBase->>CephBackupStorageBase: validateTaskUuid(), selectMonByUuid()
CephBackupStorageBase->>CephAgent: POST /ceph/file/progress (GetDownloadFileProgressCmd)
CephAgent-->>CephBackupStorageBase: GetDownloadFileProgressResponse
CephBackupStorageBase-->>Client: GetFileDownloadProgressFromBackupStorageHostReply
sequenceDiagram
participant Client
participant MessageBus
participant BackupStorageBase
participant CephBackupStorageBase
participant CephAgent
Client->>MessageBus: SoftwareUpgradePackageDeployMsg
MessageBus->>BackupStorageBase: handleLocalMessage()
BackupStorageBase->>CephBackupStorageBase: handle(SoftwareUpgradePackageDeployMsg)
CephBackupStorageBase->>CephBackupStorageBase: validatePaths(), validateSshUser(), selectMonByUuid()
CephBackupStorageBase->>CephAgent: POST /ceph/upgrade/deploy (SoftwareUpgradePackageCmd)
CephAgent-->>CephBackupStorageBase: SoftwareUpgradePackageResponse
CephBackupStorageBase-->>Client: SoftwareUpgradePackageDeployReply
估计代码审查工作量🎯 4 (复杂) | ⏱️ ~45 分钟 诗
🚥 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 docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.42.1)plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaComment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
sdk/src/main/java/org/zstack/sdk/AdditionalLicenseType.java (1)
3-9: 可选建议：枚举常量命名未遵循大写规范。所有枚举常量（
edge、zstone、zcex、zsv、zmigrate）使用小写命名，但根据编码规范，常量应使用全大写并用下划线分隔单词。不过，这是整个枚举中的既有模式，新增的
zmigrate与现有代码保持了一致性。如需修改，建议作为独立的重构任务统一处理所有常量的命名，避免仅修改部分常量造成的不一致。编码规范依据：常量命名：全部大写，使用下划线分隔单词。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/src/main/java/org/zstack/sdk/AdditionalLicenseType.java` around lines 3 - 9, The enum AdditionalLicenseType currently uses lowercase constants (edge, zstone, zcex, zsv, zmigrate); rename them to follow constant naming conventions (EDGE, ZSTONE, ZCEX, ZSV, ZMIGRATE) and update any code that references these enum names (parsing, serialization/deserialization, switches, tests) to the new identifiers; if breaking changes are a concern, perform this as a coordinated refactor (or add `@JsonProperty` annotations / a compatibility mapper where enums are deserialized from lowercase strings) to preserve runtime behavior while standardizing the names.utils/src/main/java/org/zstack/utils/path/RemotePathValidator.java (1)
10-10: 建议将工具类设为不可实例化。
RemotePathValidator仅包含静态方法，建议加私有构造函数（可选优化，避免误用）。♻️ 建议修改
-public class RemotePathValidator { +public class RemotePathValidator { + private RemotePathValidator() { + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@utils/src/main/java/org/zstack/utils/path/RemotePathValidator.java` at line 10, RemotePathValidator only contains static methods and should not be instantiable; add a private no-arg constructor to the RemotePathValidator class (e.g., private RemotePathValidator() {}) to prevent accidental instantiation and make the intent explicit while leaving existing static methods unchanged.plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageSimulator.java (1)
230-307: 建议统一新增接口的cmd记录策略。当前新增方法都反序列化了
cmd，但未像本类其他 handler 一样写入config进行留痕。建议统一记录（或明确移除未使用局部变量），提升测试可断言性与维护性。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageSimulator.java` around lines 230 - 307, Several new handlers in CephBackupStorageSimulator (getLocalFileSize, fileDownload, fileUpload, fileDownloadProgress, deleteFiles, unzipFile, softwareUpgradePackageDeploy) deserialize a cmd but do not record it to the simulator's config like other handlers; either persist the deserialized cmd into the simulator's config map (e.g., config.put("lastGetLocalFileSizeCmd", cmd) using a unique key per handler) so tests can assert calls, or explicitly remove the unused local cmd variable if you intend not to record; update each referenced method in CephBackupStorageSimulator accordingly to follow the same recording pattern used elsewhere in the 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
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java`:
- Around line 2256-2260: The HTTP requests sent via new
CephBackupStorageMonBase(mon).httpCall(...) for UnzipFileCmd (and similarly
DeleteFilesCmd, GetDownloadFileProgressCmd, SoftwareUpgradePackageCmd,
CancelCommand) are missing uuid/fsid because HttpCaller.prepareCmd(...) isn’t
executed; fix by ensuring each command is initialized with the agent identity
before calling httpCall — either call the existing
HttpCaller.prepareCmd(monBase, cmd) (or equivalent prepare/initialize method) or
add a small shared init helper that sets cmd.uuid and cmd.fsid from the
CephBackupStorageMonBase/mon, then use that initialized command in the httpCall;
update the UnzipFileCmd path and the other flagged call sites (and consider
extracting a common initCommand(CephBackupStorageMonBase, AgentCommand) used by
all five places).
- Around line 2159-2167: The current guard skips validation for an empty
installPath because of the "!isEmpty()" check, allowing "" to be sent to the
agent; instead, call RemotePathValidator.validateRemotePath whenever
msg.getInstallPath() is non-null and let validateRemotePath reject empty
strings. In CephBackupStorageBase, replace the conditional that checks both null
and isEmpty with a single null check on msg.getInstallPath() and run String
pathErr = RemotePathValidator.validateRemotePath(msg.getInstallPath(),
"installPath"); then keep the existing reply.setError(operr(pathErr));
bus.reply(msg, reply); return; behavior when pathErr != null.
- Around line 732-755: The SoftwareUpgradePackageCmd currently carries
targetHostSshPassword (Base64) over HTTP; remove/avoid sending reversible
secrets and instead use key-based auth or a credential reference: modify
SoftwareUpgradePackageCmd to drop targetHostSshPassword and add either
targetHostSshPrivateKey (private-key content, still `@NoLogging`) or, preferably,
targetHostCredentialId (a reference ID so the agent fetches the secret from a
vault/credential store); update the agent protocol/handler that consumes
SoftwareUpgradePackageCmd to resolve the credential locally (or require
TLS-protected transport) and ensure any remaining secret fields are annotated
with `@NoLogging` and never serialized in cleartext over non-TLS channels.
---
Nitpick comments:
In
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageSimulator.java`:
- Around line 230-307: Several new handlers in CephBackupStorageSimulator
(getLocalFileSize, fileDownload, fileUpload, fileDownloadProgress, deleteFiles,
unzipFile, softwareUpgradePackageDeploy) deserialize a cmd but do not record it
to the simulator's config like other handlers; either persist the deserialized
cmd into the simulator's config map (e.g., config.put("lastGetLocalFileSizeCmd",
cmd) using a unique key per handler) so tests can assert calls, or explicitly
remove the unused local cmd variable if you intend not to record; update each
referenced method in CephBackupStorageSimulator accordingly to follow the same
recording pattern used elsewhere in the class.
In `@sdk/src/main/java/org/zstack/sdk/AdditionalLicenseType.java`:
- Around line 3-9: The enum AdditionalLicenseType currently uses lowercase
constants (edge, zstone, zcex, zsv, zmigrate); rename them to follow constant
naming conventions (EDGE, ZSTONE, ZCEX, ZSV, ZMIGRATE) and update any code that
references these enum names (parsing, serialization/deserialization, switches,
tests) to the new identifiers; if breaking changes are a concern, perform this
as a coordinated refactor (or add `@JsonProperty` annotations / a compatibility
mapper where enums are deserialized from lowercase strings) to preserve runtime
behavior while standardizing the names.
In `@utils/src/main/java/org/zstack/utils/path/RemotePathValidator.java`:
- Line 10: RemotePathValidator only contains static methods and should not be
instantiable; add a private no-arg constructor to the RemotePathValidator class
(e.g., private RemotePathValidator() {}) to prevent accidental instantiation and
make the intent explicit while leaving existing static methods unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d45c1e3a-ac05-4751-8374-473058cdd5fd
⛔ Files ignored due to path filters (2)
build/pom.xmlis excluded by!**/*.xmlconf/errorCodes/ZMigratePlugin.xmlis excluded by!**/*.xml
📒 Files selected for processing (34)
header/src/main/java/org/zstack/header/storage/backup/CancelDownloadFileOnBackupStorageHostMsg.javaheader/src/main/java/org/zstack/header/storage/backup/CancelDownloadFileOnBackupStorageHostReply.javaheader/src/main/java/org/zstack/header/storage/backup/DeleteFilesOnBackupStorageHostMsg.javaheader/src/main/java/org/zstack/header/storage/backup/DeleteFilesOnBackupStorageHostReply.javaheader/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressFromBackupStorageHostMsg.javaheader/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressFromBackupStorageHostReply.javaheader/src/main/java/org/zstack/header/storage/backup/SoftwareUpgradePackageDeployMsg.javaheader/src/main/java/org/zstack/header/storage/backup/SoftwareUpgradePackageDeployReply.javaheader/src/main/java/org/zstack/header/storage/backup/UnzipFileOnBackupStorageHostMsg.javaheader/src/main/java/org/zstack/header/storage/backup/UnzipFileOnBackupStorageHostReply.javaheader/src/main/java/org/zstack/header/storage/backup/UploadFileToBackupStorageHostMsg.javaheader/src/main/java/org/zstack/header/storage/backup/UploadFileToBackupStorageHostReply.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageSimulator.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javasdk/src/main/java/org/zstack/sdk/AdditionalLicenseType.javasdk/src/main/java/org/zstack/sdk/softwarePackage/header/CleanUpgradeSoftwarePackageAction.javasdk/src/main/java/org/zstack/sdk/softwarePackage/header/CleanUpgradeSoftwarePackageResult.javasdk/src/main/java/org/zstack/sdk/softwarePackage/header/UploadAndExecuteSoftwareUpgradePackageAction.javasdk/src/main/java/org/zstack/sdk/softwarePackage/header/UploadAndExecuteSoftwareUpgradePackageResult.javasdk/src/main/java/org/zstack/sdk/softwarePackage/header/UploadSoftwarePackageToBackupStorageAction.javasdk/src/main/java/org/zstack/sdk/softwarePackage/header/UploadSoftwarePackageToBackupStorageResult.javasdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateGatewayVmInstancesAction.javasdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateGatewayVmInstancesResult.javasdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateImagesAction.javasdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateImagesResult.javasdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateInfosAction.javasdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateInfosResult.javastorage/src/main/java/org/zstack/storage/backup/BackupStorageBase.javatestlib/src/main/java/org/zstack/testlib/ApiHelper.groovytestlib/src/main/java/org/zstack/testlib/CephBackupStorageSpec.groovytestlib/src/main/java/org/zstack/testlib/KVMSimulator.groovyutils/src/main/java/org/zstack/utils/path/RemotePathValidator.java
Comment on lines
+732
to
+755
| /** | ||
| * Command to deploy a software upgrade package from a Ceph backup storage host | ||
| * to a target host via SCP/SSH. | ||
| * | ||
| * NOTE: targetHostSshPassword is the Base64-encoded password (not plaintext), | ||
| * and is annotated with @NoLogging so it will never appear in logs. | ||
| * The HTTP transport to the Ceph agent is within the trusted management network. | ||
| * | ||
| * TODO(security): The SSH password is transmitted over HTTP (management network) as Base64. | ||
| * While @NoLogging prevents log leakage and the management network is considered trusted, | ||
| * this is still vulnerable to network sniffing. Consider migrating to HTTPS transport | ||
| * or pushing password handling down to the agent side (e.g., vault/key-based auth). | ||
| */ | ||
| public static class SoftwareUpgradePackageCmd extends AgentCommand implements HasThreadContext, Serializable { | ||
| public String upgradePackagePath; | ||
| public String upgradePackageTargetPath; | ||
| public String upgradeScriptPath; | ||
| public String softwareType; | ||
| public int targetHostSshPort; | ||
| public String targetHostSshUsername; | ||
| @NoLogging | ||
| public String targetHostSshPassword; | ||
| public String targetHostIp; | ||
| } |
There was a problem hiding this comment.
不要把 SSH 密码仅做 Base64 后走 HTTP。
@NoLogging 只能防日志泄漏，不能保护传输链路。这个 PR 新增的 targetHostSshPassword 在管理网络上仍然是可逆明文，抓包即可还原。
至少应改成密钥认证 / 凭据引用，或者在 Ceph agent 通道启用 TLS 后再传输这类秘密。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java`
around lines 732 - 755, The SoftwareUpgradePackageCmd currently carries
targetHostSshPassword (Base64) over HTTP; remove/avoid sending reversible
secrets and instead use key-based auth or a credential reference: modify
SoftwareUpgradePackageCmd to drop targetHostSshPassword and add either
targetHostSshPrivateKey (private-key content, still `@NoLogging`) or, preferably,
targetHostCredentialId (a reference ID so the agent fetches the secret from a
vault/credential store); update the agent protocol/handler that consumes
SoftwareUpgradePackageCmd to resolve the credential locally (or require
TLS-protected transport) and ensure any remaining secret fields are annotated
with `@NoLogging` and never serialized in cleartext over non-TLS channels.
Comment on lines
+2256
to
+2260
| UnzipFileCmd cmd = new UnzipFileCmd(); | ||
| cmd.installPath = msg.getInstallPath(); | ||
|
|
||
| CephBackupStorageMonBase monBase = new CephBackupStorageMonBase(mon); | ||
| monBase.httpCall(UNZIP_FILE_PATH, cmd, UnzipFileResponse.class, new ReturnValueCompletion<UnzipFileResponse>(msg) { |
There was a problem hiding this comment.
这些定向 MON 的请求没有填充 uuid/fsid。
HttpCaller.prepareCmd() 只会在 httpCall(...) 包装器里执行；这里直接走 new CephBackupStorageMonBase(mon).httpCall(...)，发出去的 AgentCommand 会缺少 uuid 和 fsid。上面的 handle(GetImageDownloadProgressMsg) 已经手工补了这两个字段，当前这 5 条新路径都会受影响。
🛠️ 最小修复示例
UnzipFileCmd cmd = new UnzipFileCmd();
+ cmd.uuid = self.getUuid();
+ cmd.fsid = getSelf().getFsid();
cmd.installPath = msg.getInstallPath();DeleteFilesCmd、GetDownloadFileProgressCmd、SoftwareUpgradePackageCmd、CancelCommand 也需要同样处理；更稳妥的是抽一个公共初始化方法，避免后续再漏。
Also applies to: 2302-2306, 2339-2343, 2435-2446, 2479-2483
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java`
around lines 2256 - 2260, The HTTP requests sent via new
CephBackupStorageMonBase(mon).httpCall(...) for UnzipFileCmd (and similarly
DeleteFilesCmd, GetDownloadFileProgressCmd, SoftwareUpgradePackageCmd,
CancelCommand) are missing uuid/fsid because HttpCaller.prepareCmd(...) isn’t
executed; fix by ensuring each command is initialized with the agent identity
before calling httpCall — either call the existing
HttpCaller.prepareCmd(monBase, cmd) (or equivalent prepare/initialize method) or
add a small shared init helper that sets cmd.uuid and cmd.fsid from the
CephBackupStorageMonBase/mon, then use that initialized command in the httpCall;
update the UnzipFileCmd path and the other flagged call sites (and consider
extracting a common initCommand(CephBackupStorageMonBase, AgentCommand) used by
all five places).
MatheMatrix
force-pushed
the
sync/tao.gan/zmigrate-ZSV-11449@@4
branch
2 times, most recently
from
ed52637 to
28ab5c5
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java (2)
732-755:⚠️ Potential issue | 🟠 Major不要继续在命令体里传递可逆 SSH 密码。
targetHostSshPassword目前仍是可逆敏感信息；@NoLogging只能防日志泄漏，无法防链路抓包。这里保留为 TODO 但仍在真实调用路径上，风险未消除。建议改为凭据引用（credentialId）或强制 TLS 通道后再传输秘密。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java` around lines 732 - 755, SoftwareUpgradePackageCmd currently carries a reversible SSH password in the field targetHostSshPassword which `@NoLogging` cannot protect from network sniffing; remove this field from SoftwareUpgradePackageCmd and replace it with a credential reference (e.g., targetHostCredentialId) or a SSH key identifier, update any consumers of SoftwareUpgradePackageCmd to resolve the secret from the credential store (or require an HTTPS/TLS-backed agent endpoint) and ensure the agent obtains the actual secret locally rather than having the management plane transmit plaintext/Base64 credentials.
2441-2453:⚠️ Potential issue | 🔴 Critical定向到指定 MON 的两个新请求仍缺少
uuid/fsid初始化。这两处直接使用
new CephBackupStorageMonBase(mon).httpCall(...)，不会经过HttpCaller.prepareCmd()；当前SoftwareUpgradePackageCmd和CancelCommand未设置cmd.uuid/cmd.fsid，会导致 agent 侧识别信息缺失。🛠️ 最小修复建议
SoftwareUpgradePackageCmd cmd = new SoftwareUpgradePackageCmd(); + cmd.uuid = self.getUuid(); + cmd.fsid = getSelf().getFsid(); cmd.upgradePackagePath = msg.getUpgradePackagePath(); cmd.upgradePackageTargetPath = msg.getUpgradePackageTargetPath(); cmd.softwareType = msg.getSoftwareType(); @@ CancelCommand cmd = new CancelCommand(); + cmd.uuid = self.getUuid(); + cmd.fsid = getSelf().getFsid(); cmd.setCancellationApiId(msg.getCancellationApiId());Also applies to: 2485-2490
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java` around lines 2441 - 2453, The targeted-MON HTTP calls using new CephBackupStorageMonBase(mon).httpCall(...) for SoftwareUpgradePackageCmd and CancelCommand are missing required identification fields (cmd.uuid / cmd.fsid) because they skip HttpCaller.prepareCmd(); fix by initializing those fields before the call: either invoke the same prepareCmd(...) logic used for non-directed calls or explicitly set cmd.uuid = <backupStorageUuid> and cmd.fsid = <cephFsid> on the SoftwareUpgradePackageCmd and CancelCommand instances prior to calling CephBackupStorageMonBase.httpCall; reference CephBackupStorageMonBase, SoftwareUpgradePackageCmd, CancelCommand and HttpCaller.prepareCmd to locate where to apply the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java`:
- Around line 732-755: SoftwareUpgradePackageCmd currently carries a reversible
SSH password in the field targetHostSshPassword which `@NoLogging` cannot protect
from network sniffing; remove this field from SoftwareUpgradePackageCmd and
replace it with a credential reference (e.g., targetHostCredentialId) or a SSH
key identifier, update any consumers of SoftwareUpgradePackageCmd to resolve the
secret from the credential store (or require an HTTPS/TLS-backed agent endpoint)
and ensure the agent obtains the actual secret locally rather than having the
management plane transmit plaintext/Base64 credentials.
- Around line 2441-2453: The targeted-MON HTTP calls using new
CephBackupStorageMonBase(mon).httpCall(...) for SoftwareUpgradePackageCmd and
CancelCommand are missing required identification fields (cmd.uuid / cmd.fsid)
because they skip HttpCaller.prepareCmd(); fix by initializing those fields
before the call: either invoke the same prepareCmd(...) logic used for
non-directed calls or explicitly set cmd.uuid = <backupStorageUuid> and cmd.fsid
= <cephFsid> on the SoftwareUpgradePackageCmd and CancelCommand instances prior
to calling CephBackupStorageMonBase.httpCall; reference
CephBackupStorageMonBase, SoftwareUpgradePackageCmd, CancelCommand and
HttpCaller.prepareCmd to locate where to apply the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2865d2c8-b8d8-44b6-a1ce-ea57291ed9a3
⛔ Files ignored due to path filters (2)
build/pom.xmlis excluded by!**/*.xmlconf/errorCodes/ZMigratePlugin.xmlis excluded by!**/*.xml
📒 Files selected for processing (34)
header/src/main/java/org/zstack/header/storage/backup/CancelDownloadFileOnBackupStorageHostMsg.javaheader/src/main/java/org/zstack/header/storage/backup/CancelDownloadFileOnBackupStorageHostReply.javaheader/src/main/java/org/zstack/header/storage/backup/DeleteFilesOnBackupStorageHostMsg.javaheader/src/main/java/org/zstack/header/storage/backup/DeleteFilesOnBackupStorageHostReply.javaheader/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressFromBackupStorageHostMsg.javaheader/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressFromBackupStorageHostReply.javaheader/src/main/java/org/zstack/header/storage/backup/SoftwareUpgradePackageDeployMsg.javaheader/src/main/java/org/zstack/header/storage/backup/SoftwareUpgradePackageDeployReply.javaheader/src/main/java/org/zstack/header/storage/backup/UnzipFileOnBackupStorageHostMsg.javaheader/src/main/java/org/zstack/header/storage/backup/UnzipFileOnBackupStorageHostReply.javaheader/src/main/java/org/zstack/header/storage/backup/UploadFileToBackupStorageHostMsg.javaheader/src/main/java/org/zstack/header/storage/backup/UploadFileToBackupStorageHostReply.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageSimulator.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javasdk/src/main/java/org/zstack/sdk/AdditionalLicenseType.javasdk/src/main/java/org/zstack/sdk/softwarePackage/header/CleanUpgradeSoftwarePackageAction.javasdk/src/main/java/org/zstack/sdk/softwarePackage/header/CleanUpgradeSoftwarePackageResult.javasdk/src/main/java/org/zstack/sdk/softwarePackage/header/UploadAndExecuteSoftwareUpgradePackageAction.javasdk/src/main/java/org/zstack/sdk/softwarePackage/header/UploadAndExecuteSoftwareUpgradePackageResult.javasdk/src/main/java/org/zstack/sdk/softwarePackage/header/UploadSoftwarePackageToBackupStorageAction.javasdk/src/main/java/org/zstack/sdk/softwarePackage/header/UploadSoftwarePackageToBackupStorageResult.javasdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateGatewayVmInstancesAction.javasdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateGatewayVmInstancesResult.javasdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateImagesAction.javasdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateImagesResult.javasdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateInfosAction.javasdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateInfosResult.javastorage/src/main/java/org/zstack/storage/backup/BackupStorageBase.javatestlib/src/main/java/org/zstack/testlib/ApiHelper.groovytestlib/src/main/java/org/zstack/testlib/CephBackupStorageSpec.groovytestlib/src/main/java/org/zstack/testlib/KVMSimulator.groovyutils/src/main/java/org/zstack/utils/path/RemotePathValidator.java
✅ Files skipped from review due to trivial changes (16)
- header/src/main/java/org/zstack/header/storage/backup/CancelDownloadFileOnBackupStorageHostReply.java
- sdk/src/main/java/org/zstack/sdk/softwarePackage/header/UploadAndExecuteSoftwareUpgradePackageResult.java
- header/src/main/java/org/zstack/header/storage/backup/SoftwareUpgradePackageDeployReply.java
- sdk/src/main/java/org/zstack/sdk/softwarePackage/header/CleanUpgradeSoftwarePackageResult.java
- sdk/src/main/java/org/zstack/sdk/softwarePackage/header/UploadSoftwarePackageToBackupStorageResult.java
- sdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateImagesResult.java
- sdk/src/main/java/org/zstack/sdk/AdditionalLicenseType.java
- header/src/main/java/org/zstack/header/storage/backup/UnzipFileOnBackupStorageHostReply.java
- header/src/main/java/org/zstack/header/storage/backup/CancelDownloadFileOnBackupStorageHostMsg.java
- sdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateInfosResult.java
- header/src/main/java/org/zstack/header/storage/backup/UploadFileToBackupStorageHostMsg.java
- header/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressFromBackupStorageHostMsg.java
- sdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateGatewayVmInstancesResult.java
- header/src/main/java/org/zstack/header/storage/backup/DeleteFilesOnBackupStorageHostReply.java
- header/src/main/java/org/zstack/header/storage/backup/UploadFileToBackupStorageHostReply.java
- header/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressFromBackupStorageHostReply.java
🚧 Files skipped from review as they are similar to previous changes (6)
- plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java
- header/src/main/java/org/zstack/header/storage/backup/UnzipFileOnBackupStorageHostMsg.java
- plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageSimulator.java
- storage/src/main/java/org/zstack/storage/backup/BackupStorageBase.java
- testlib/src/main/java/org/zstack/testlib/CephBackupStorageSpec.groovy
- header/src/main/java/org/zstack/header/storage/backup/SoftwareUpgradePackageDeployMsg.java
APIImpact Resolves: ZSV-11449 Change-Id: I656979726c736267776c7262716969726874716d
MatheMatrix
force-pushed
the
sync/tao.gan/zmigrate-ZSV-11449@@4
branch
from
28ab5c5 to
da0f633
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java (2)
2441-2452:⚠️ Potential issue | 🔴 Critical这两条定向 MON 的请求还没补
uuid/fsid。这里直接走
new CephBackupStorageMonBase(mon).httpCall(...)，不会经过HttpCaller.prepareCmd()。UnzipFileCmd、DeleteFilesCmd、GetDownloadFileProgressCmd前面已经手工补了身份字段，但SoftwareUpgradePackageCmd和CancelCommand这两处仍然是空的，agent 侧拿不到 backup storage 标识。🛠️ 最小修复示例
SoftwareUpgradePackageCmd cmd = new SoftwareUpgradePackageCmd(); + cmd.uuid = self.getUuid(); + cmd.fsid = getSelf().getFsid(); cmd.upgradePackagePath = msg.getUpgradePackagePath(); cmd.upgradePackageTargetPath = msg.getUpgradePackageTargetPath(); cmd.softwareType = msg.getSoftwareType(); cmd.upgradeScriptPath = msg.getUpgradeScriptPath(); cmd.targetHostSshPort = msg.getTargetHostSshPort(); @@ CancelCommand cmd = new CancelCommand(); + cmd.uuid = self.getUuid(); + cmd.fsid = getSelf().getFsid(); cmd.setCancellationApiId(msg.getCancellationApiId());更稳一点的话，建议抽一个公共初始化方法，避免后面再漏新的直连 MON 命令。
Also applies to: 2485-2489
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java` around lines 2441 - 2452, SoftwareUpgradePackageCmd (and the sibling CancelCommand call later) are missing the backup storage identity fields (uuid/fsid) because using new CephBackupStorageMonBase(mon).httpCall(...) skips HttpCaller.prepareCmd(); fix by populating the identity on the cmd before the httpCall — either call HttpCaller.prepareCmd(cmd, backupStorage) or set cmd.uuid and cmd.fsid manually (same approach used for UnzipFileCmd/DeleteFilesCmd/GetDownloadFileProgressCmd), and to avoid future omissions extract a small helper (e.g., prepareMonCmd(cmd, backupStorage) or CephBackupStorageMonBase.prepareCmd(...)) and use it for all direct MON calls including SoftwareUpgradePackageCmd and CancelCommand.
732-755:⚠️ Potential issue | 🟠 Major不要继续把 SSH 密码作为可逆内容下发给 agent。
@NoLogging只能避免日志泄漏，不能保护传输链路；targetHostSshPassword这里仍然会随 HTTP 请求离开管理节点。Base64 不是保密措施，抓包即可还原，建议改成凭据引用/密钥认证，或者先把 agent 通道升级到 TLS。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java` around lines 732 - 755, The SoftwareUpgradePackageCmd currently carries a reversible SSH password in the targetHostSshPassword field which is transmitted over HTTP; remove this plaintext/password field and replace it with a non-reversible credential reference (e.g., targetHostCredentialId) or a public-key-based artifact (e.g., targetHostSshKeyId or boolean useAgentKey) and update all usages of SoftwareUpgradePackageCmd/AgentCommand to look up the secret from the vault/credential store on the agent side or to use TLS-mutual auth; ensure serialization and deserialization reflect the new field, remove reliance on Base64 password transport, and update codepaths that set or read targetHostSshPassword to instead pass only the credential ID or key reference and perform actual secret retrieval/auth on the agent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageSimulator.java`:
- Around line 244-247: In the fileDownload handler in
CephBackupStorageSimulator, populate the DownloadFileResponse.format field
instead of leaving it null: set rsp.format from the incoming request (e.g., use
the request/entity format field such as entity.format or entity.getFormat())
and, if that value is null/empty, default to a stable value like "raw"; ensure
you update the DownloadFileResponse assignment (rsp.format) before calling
reply(entity, rsp).
In
`@sdk/src/main/java/org/zstack/sdk/softwarePackage/header/UploadAndExecuteSoftwareUpgradePackageAction.java`:
- Around line 28-29: The uuid field in
UploadAndExecuteSoftwareUpgradePackageAction is marked as a path identifier but
currently allows empty strings (annotation has nonempty = false and emptyString
= true); update the `@Param` annotation on the uuid field so it rejects empty
strings and enforces non-empty values (e.g., set nonempty = true and emptyString
= false) to validate the path variable at SDK boundary.
In `@utils/src/main/java/org/zstack/utils/path/RemotePathValidator.java`:
- Around line 61-63: RemotePathValidator currently only blocks exact matches
against PROTECTED_PATHS (using normalized), allowing protected directories'
children like "/etc/passwd" through; update the check in the validation method
(where PROTECTED_PATHS and normalized are used) to reject any path that is the
protected path or a descendant of it by testing normalized.equals(p) ||
normalized.startsWith(p + "/") for each p in PROTECTED_PATHS (ensure proper
normalization/trim of trailing slashes before this comparison) and return the
same formatted message using paramName and path when matched.
---
Duplicate comments:
In
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java`:
- Around line 2441-2452: SoftwareUpgradePackageCmd (and the sibling
CancelCommand call later) are missing the backup storage identity fields
(uuid/fsid) because using new CephBackupStorageMonBase(mon).httpCall(...) skips
HttpCaller.prepareCmd(); fix by populating the identity on the cmd before the
httpCall — either call HttpCaller.prepareCmd(cmd, backupStorage) or set cmd.uuid
and cmd.fsid manually (same approach used for
UnzipFileCmd/DeleteFilesCmd/GetDownloadFileProgressCmd), and to avoid future
omissions extract a small helper (e.g., prepareMonCmd(cmd, backupStorage) or
CephBackupStorageMonBase.prepareCmd(...)) and use it for all direct MON calls
including SoftwareUpgradePackageCmd and CancelCommand.
- Around line 732-755: The SoftwareUpgradePackageCmd currently carries a
reversible SSH password in the targetHostSshPassword field which is transmitted
over HTTP; remove this plaintext/password field and replace it with a
non-reversible credential reference (e.g., targetHostCredentialId) or a
public-key-based artifact (e.g., targetHostSshKeyId or boolean useAgentKey) and
update all usages of SoftwareUpgradePackageCmd/AgentCommand to look up the
secret from the vault/credential store on the agent side or to use TLS-mutual
auth; ensure serialization and deserialization reflect the new field, remove
reliance on Base64 password transport, and update codepaths that set or read
targetHostSshPassword to instead pass only the credential ID or key reference
and perform actual secret retrieval/auth on the agent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3bbc2acd-0b8d-414d-9b63-45f2ff2ae945
⛔ Files ignored due to path filters (2)
build/pom.xmlis excluded by!**/*.xmlconf/errorCodes/ZMigratePlugin.xmlis excluded by!**/*.xml
📒 Files selected for processing (34)
header/src/main/java/org/zstack/header/storage/backup/CancelDownloadFileOnBackupStorageHostMsg.javaheader/src/main/java/org/zstack/header/storage/backup/CancelDownloadFileOnBackupStorageHostReply.javaheader/src/main/java/org/zstack/header/storage/backup/DeleteFilesOnBackupStorageHostMsg.javaheader/src/main/java/org/zstack/header/storage/backup/DeleteFilesOnBackupStorageHostReply.javaheader/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressFromBackupStorageHostMsg.javaheader/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressFromBackupStorageHostReply.javaheader/src/main/java/org/zstack/header/storage/backup/SoftwareUpgradePackageDeployMsg.javaheader/src/main/java/org/zstack/header/storage/backup/SoftwareUpgradePackageDeployReply.javaheader/src/main/java/org/zstack/header/storage/backup/UnzipFileOnBackupStorageHostMsg.javaheader/src/main/java/org/zstack/header/storage/backup/UnzipFileOnBackupStorageHostReply.javaheader/src/main/java/org/zstack/header/storage/backup/UploadFileToBackupStorageHostMsg.javaheader/src/main/java/org/zstack/header/storage/backup/UploadFileToBackupStorageHostReply.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageSimulator.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javasdk/src/main/java/org/zstack/sdk/AdditionalLicenseType.javasdk/src/main/java/org/zstack/sdk/softwarePackage/header/CleanUpgradeSoftwarePackageAction.javasdk/src/main/java/org/zstack/sdk/softwarePackage/header/CleanUpgradeSoftwarePackageResult.javasdk/src/main/java/org/zstack/sdk/softwarePackage/header/UploadAndExecuteSoftwareUpgradePackageAction.javasdk/src/main/java/org/zstack/sdk/softwarePackage/header/UploadAndExecuteSoftwareUpgradePackageResult.javasdk/src/main/java/org/zstack/sdk/softwarePackage/header/UploadSoftwarePackageToBackupStorageAction.javasdk/src/main/java/org/zstack/sdk/softwarePackage/header/UploadSoftwarePackageToBackupStorageResult.javasdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateGatewayVmInstancesAction.javasdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateGatewayVmInstancesResult.javasdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateImagesAction.javasdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateImagesResult.javasdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateInfosAction.javasdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateInfosResult.javastorage/src/main/java/org/zstack/storage/backup/BackupStorageBase.javatestlib/src/main/java/org/zstack/testlib/ApiHelper.groovytestlib/src/main/java/org/zstack/testlib/CephBackupStorageSpec.groovytestlib/src/main/java/org/zstack/testlib/KVMSimulator.groovyutils/src/main/java/org/zstack/utils/path/RemotePathValidator.java
✅ Files skipped from review due to trivial changes (14)
- sdk/src/main/java/org/zstack/sdk/AdditionalLicenseType.java
- header/src/main/java/org/zstack/header/storage/backup/CancelDownloadFileOnBackupStorageHostReply.java
- header/src/main/java/org/zstack/header/storage/backup/SoftwareUpgradePackageDeployReply.java
- sdk/src/main/java/org/zstack/sdk/softwarePackage/header/CleanUpgradeSoftwarePackageResult.java
- sdk/src/main/java/org/zstack/sdk/softwarePackage/header/UploadAndExecuteSoftwareUpgradePackageResult.java
- header/src/main/java/org/zstack/header/storage/backup/DeleteFilesOnBackupStorageHostReply.java
- sdk/src/main/java/org/zstack/sdk/softwarePackage/header/UploadSoftwarePackageToBackupStorageResult.java
- header/src/main/java/org/zstack/header/storage/backup/UnzipFileOnBackupStorageHostMsg.java
- header/src/main/java/org/zstack/header/storage/backup/UnzipFileOnBackupStorageHostReply.java
- header/src/main/java/org/zstack/header/storage/backup/DeleteFilesOnBackupStorageHostMsg.java
- header/src/main/java/org/zstack/header/storage/backup/UploadFileToBackupStorageHostReply.java
- sdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateInfosResult.java
- sdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateImagesResult.java
- header/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressFromBackupStorageHostReply.java
🚧 Files skipped from review as they are similar to previous changes (7)
- testlib/src/main/java/org/zstack/testlib/CephBackupStorageSpec.groovy
- header/src/main/java/org/zstack/header/storage/backup/UploadFileToBackupStorageHostMsg.java
- header/src/main/java/org/zstack/header/storage/backup/CancelDownloadFileOnBackupStorageHostMsg.java
- sdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateGatewayVmInstancesResult.java
- header/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressFromBackupStorageHostMsg.java
- header/src/main/java/org/zstack/header/storage/backup/SoftwareUpgradePackageDeployMsg.java
- sdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateGatewayVmInstancesAction.java
Comment on lines
+244
to
+247
| DownloadFileResponse rsp = new DownloadFileResponse(); | ||
| rsp.md5sum = "d41d8cd98f00b204e9800998ecf8427e"; | ||
| rsp.size = 0; | ||
| reply(entity, rsp); |
There was a problem hiding this comment.
补齐 fileDownload 的 format 占位值。
DownloadFileResponse 的契约里有 format，而上游会把它直接透传到 reply。这里始终返回 null，模拟器和真实实现不一致，相关用例也拿不到稳定的格式值。
🛠️ 建议修改
DownloadFileResponse rsp = new DownloadFileResponse();
rsp.md5sum = "d41d8cd98f00b204e9800998ecf8427e";
rsp.size = 0;
+ rsp.format = "qcow2";
reply(entity, rsp);📝 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
| DownloadFileResponse rsp = new DownloadFileResponse(); | |
| rsp.md5sum = "d41d8cd98f00b204e9800998ecf8427e"; | |
| rsp.size = 0; | |
| reply(entity, rsp); | |
| DownloadFileResponse rsp = new DownloadFileResponse(); | |
| rsp.md5sum = "d41d8cd98f00b204e9800998ecf8427e"; | |
| rsp.size = 0; | |
| rsp.format = "qcow2"; | |
| reply(entity, rsp); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageSimulator.java`
around lines 244 - 247, In the fileDownload handler in
CephBackupStorageSimulator, populate the DownloadFileResponse.format field
instead of leaving it null: set rsp.format from the incoming request (e.g., use
the request/entity format field such as entity.format or entity.getFormat())
and, if that value is null/empty, default to a stable value like "raw"; ensure
you update the DownloadFileResponse assignment (rsp.format) before calling
reply(entity, rsp).
Comment on lines
+28
to
+29
| @Param(required = true, nonempty = false, nullElements = false, emptyString = true, noTrim = false) | ||
| public java.lang.String uuid; |
There was a problem hiding this comment.
uuid 作为路径变量不应允许空字符串。
Line 28-29 当前配置会放宽主标识约束，可能产生非法路径。建议在 SDK 侧尽早拦截。
🔧 建议修复
- `@Param`(required = true, nonempty = false, nullElements = false, emptyString = true, noTrim = false)
+ `@Param`(required = true, nonempty = true, nullElements = false, emptyString = false, noTrim = false)
public java.lang.String uuid;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@sdk/src/main/java/org/zstack/sdk/softwarePackage/header/UploadAndExecuteSoftwareUpgradePackageAction.java`
around lines 28 - 29, The uuid field in
UploadAndExecuteSoftwareUpgradePackageAction is marked as a path identifier but
currently allows empty strings (annotation has nonempty = false and emptyString
= true); update the `@Param` annotation on the uuid field so it rejects empty
strings and enforces non-empty values (e.g., set nonempty = true and emptyString
= false) to validate the path variable at SDK boundary.
Comment on lines
+61
to
+63
| if (PROTECTED_PATHS.contains(normalized)) { | ||
| return String.format("%s [%s] targets a protected system directory", paramName, path); | ||
| } |
There was a problem hiding this comment.
受保护目录判断过窄，子路径会被放行。
Line 61-63 只拦截“目录本身”，不会拦截其子路径（如 /etc/passwd）。这会削弱安全边界。
🔧 建议修复
- // Reject protected system directories as direct targets
- if (PROTECTED_PATHS.contains(normalized)) {
- return String.format("%s [%s] targets a protected system directory", paramName, path);
- }
+ // Reject protected system directories and their descendants
+ for (String protectedPath : PROTECTED_PATHS) {
+ if (normalized.equals(protectedPath)
+ || (!"/".equals(protectedPath) && normalized.startsWith(protectedPath + "/"))) {
+ return String.format("%s [%s] targets a protected system directory", paramName, path);
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@utils/src/main/java/org/zstack/utils/path/RemotePathValidator.java` around
lines 61 - 63, RemotePathValidator currently only blocks exact matches against
PROTECTED_PATHS (using normalized), allowing protected directories' children
like "/etc/passwd" through; update the check in the validation method (where
PROTECTED_PATHS and normalized are used) to reject any path that is the
protected path or a descendant of it by testing normalized.equals(p) ||
normalized.startsWith(p + "/") for each p in PROTECTED_PATHS (ensure proper
normalization/trim of trailing slashes before this comparison) and return the
same formatted message using paramName and path when matched.
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.
APIImpact
Resolves: ZSV-11449
Change-Id: I656979726c736267776c7262716969726874716d
sync from gitlab !9660