Skip to content

<fix>[vm]: validate local storage mount point #4345

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 wants to merge 1 commit into
base: zsv_5.1.0
Choose a base branch
from
+71 −7
Open

<fix>[vm]: validate local storage mount point #4345

Show file tree
Hide file tree
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
66 changes: 59 additions & 7 deletions compute/src/main/java/org/zstack/compute/host/HostApiInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,13 @@
import org.zstack.utils.ShellResult;
import org.zstack.utils.ShellUtils;
import org.zstack.utils.network.NetworkUtils;
import org.zstack.header.storage.primary.PrimaryStorageClusterRefVO;
import org.zstack.header.storage.primary.PrimaryStorageClusterRefVO_;
import org.zstack.header.storage.primary.PrimaryStorageConstants;
import org.zstack.header.storage.primary.PrimaryStorageVO;
import org.zstack.header.storage.primary.PrimaryStorageVO_;

import java.util.List;

import static org.zstack.core.Platform.argerr;
import static org.zstack.core.Platform.operr;
Expand Down Expand Up @@ -152,6 +159,10 @@ private void validate(APIGetPhysicalMachineBlockDevicesMsg msg) {
}

private void validate(APIMountBlockDeviceMsg msg) {
validatePath(msg.getPath());
validateMountPoint(msg.getMountPoint());
validateMountPointNotOccupiedByLocalStorage(msg);

if (msg.getPassword() != null) {
return;
}
Expand All @@ -162,9 +173,6 @@ private void validate(APIMountBlockDeviceMsg msg) {
}
msg.setPassword(proxyHardware.getPassword());
msg.setUsername(msg.getUsername() != null ? msg.getUsername() : proxyHardware.getUsername());

validatePath(msg.getPath());
validateMountPoint(msg.getMountPoint());
}

private void validate(APIUpdateHostnameMsg msg) {
Expand Down Expand Up @@ -215,10 +223,6 @@ private void validateMountPoint(String mountPoint) {
"invalid value detected: '%s'",
SAFE_MOUNT_POINT_PATTERN, mountPoint));
}
if (mountPoint.endsWith("/") && !mountPoint.equals("/")) {
throw new ApiMessageInterceptionException(operr(
"mountPoint should not end with '/' except root directory"));
}
}

private ProxyHardware getProxyHardware(String hostname) {
Expand All @@ -230,4 +234,52 @@ private ProxyHardware getProxyHardware(String hostname) {
}
return null;
}

private String getClusterUuid(String hostname) {
for (ProxyHardwareFactory factory : pluginRgty.getExtensionList(ProxyHardwareFactory.class)) {
String clusterUuid = factory.getClusterUuid(hostname);
if (clusterUuid != null) {
return clusterUuid;
}
}
return null;
}

private void validateMountPointNotOccupiedByLocalStorage(APIMountBlockDeviceMsg msg) {
String clusterUuid = getClusterUuid(msg.getHostName());
if (clusterUuid == null) {
return;
}

List<String> primaryStorageUuids = Q.New(PrimaryStorageClusterRefVO.class)
.select(PrimaryStorageClusterRefVO_.primaryStorageUuid)
.eq(PrimaryStorageClusterRefVO_.clusterUuid, clusterUuid)
.listValues();
if (primaryStorageUuids.isEmpty()) {
return;
}

String mountPoint = normalizeMountPoint(msg.getMountPoint());
List<String> localStorageUrls = Q.New(PrimaryStorageVO.class)
.select(PrimaryStorageVO_.url)
.eq(PrimaryStorageVO_.type, PrimaryStorageConstants.LOCAL_STORAGE_TYPE)
.in(PrimaryStorageVO_.uuid, primaryStorageUuids)
.listValues();

for (String url : localStorageUrls) {
if (!mountPoint.equals(normalizeMountPoint(url))) {
Comment on lines +262 to +270

@coderabbitai coderabbitai Bot Jun 29, 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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="compute/src/main/java/org/zstack/compute/host/HostApiInterceptor.java"

echo "== file line count =="
wc -l "$file"

echo "== relevant slices =="
sed -n '220,320p' "$file" | cat -n

echo "== usages of normalizeMountPoint =="
rg -n "normalizeMountPoint|validateMountPoint|MountPoint" "$file"

Repository: MatheMatrix/zstack

Length of output: 4014

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== search for path normalization helpers =="
rg -n "Paths\.get\(|normalize\(\)|canonicali[sz]e|normalizeMountPoint" compute/src/main/java

echo "== inspect validateMountPoint area =="
sed -n '210,285p' compute/src/main/java/org/zstack/compute/host/HostApiInterceptor.java | cat -n

Repository: MatheMatrix/zstack

Length of output: 4326

补全 mount point 规范化，避免 . 路径段绕过本地主存储占用检查

validateMountPoint 允许 .，但 normalizeMountPoint 只去掉尾部 /；像 /local_ps/./data 这类路径会和 /local_ps/data 比较不相等，却可能落到同一目录。这里应把 msg.getMountPoint() 和已存在的 url 统一做路径归一化后再比对。

🤖 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 `@compute/src/main/java/org/zstack/compute/host/HostApiInterceptor.java` around
lines 262 - 270, The mount point comparison in
HostApiInterceptor.validateMountPoint only trims trailing slashes, so paths
containing "." can bypass the local storage conflict check; update
normalizeMountPoint and the compare flow so both msg.getMountPoint() and each
PrimaryStorageVO_.url are fully normalized before equality checks. Use the
existing validateMountPoint and normalizeMountPoint logic to ensure both values
resolve to the same canonical path representation, then compare the normalized
results in the loop over localStorageUrls.

continue;
}
throw new ApiMessageInterceptionException(argerr(
"url[%s] has been occupied by local primary storage in cluster[uuid:%s]",
msg.getMountPoint(), clusterUuid));
}
}

private static String normalizeMountPoint(String path) {
while (path.length() > 1 && path.endsWith("/")) {
path = path.substring(0, path.length() - 1);
}
return path;
}
}
4 changes: 4 additions & 0 deletions header/src/main/java/org/zstack/header/agent/ProxyHardwareFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,8 @@

public interface ProxyHardwareFactory {
ProxyHardware getProxyHardware(String hostName);

default String getClusterUuid(String hostName) {
return null;
}
}
8 changes: 8 additions & 0 deletions plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1232,4 +1232,12 @@ public ProxyHardware getProxyHardware(String hostName) {
"and host.managementIp = :hostname";
return SQL.New(sql, KVMHostVO.class).param("hostname", hostName).find();
}

@Override
public String getClusterUuid(String hostName) {
return Q.New(HostVO.class)
.select(HostVO_.clusterUuid)
.eq(HostVO_.managementIp, hostName)
.findValue();
}
}