<fix>[storage]: desensitize ExternalPrimaryStorage config and addonInfo#3536
Open
ZStack-Robot wants to merge 1 commit into5.5.12from
Open
<fix>[storage]: desensitize ExternalPrimaryStorage config and addonInfo#3536ZStack-Robot wants to merge 1 commit into5.5.12from
ZStack-Robot wants to merge 1 commit into5.5.12from
Conversation
Resolves: ZSTAC-80664 Change-Id: I11e572e7d71d970853302cc33f5843637155a595
概览新增 变更
代码审查工作量估评🎯 3 (中等) | ⏱️ ~22 分钟 诗句
🚥 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)
📝 Coding Plan
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugin/zbs/src/main/java/org/zstack/storage/zbs/MdsInfo.java (1)
24-39:⚠️ Potential issue | 🟡 Minor
equals()/hashCode()包含transient字段password，可能导致序列化后行为不一致。由于
password被标记为transient，在 JSON 序列化/反序列化后该字段会变为null。这意味着：
- 序列化前相等的两个
MdsInfo对象，反序列化后可能不再相等- 如果将
MdsInfo存入HashSet或作为HashMap的 key，反序列化后可能无法正确查找建议从
equals()和hashCode()中移除password字段，仅使用addr、port、username进行比较。🛠️ 建议修改
`@Override` public boolean equals(Object obj) { if (this == obj) return true; if (obj == null || getClass() != obj.getClass()) return false; MdsInfo other = (MdsInfo) obj; return Objects.equals(addr, other.addr) && Objects.equals(port, other.port) && - Objects.equals(username, other.username) && - Objects.equals(password, other.password); + Objects.equals(username, other.username); } `@Override` public int hashCode() { - return Objects.hash(addr, port, username, password); + return Objects.hash(addr, port, username); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/zbs/src/main/java/org/zstack/storage/zbs/MdsInfo.java` around lines 24 - 39, The equals()/hashCode() implementations in class MdsInfo currently include the transient field password which will be null after JSON deserialize; update MdsInfo.equals(Object) and MdsInfo.hashCode() to exclude password and only compare/hash addr, port and username (i.e. remove any reference to password in both methods) so object equality and hashing remain stable across serialization.
🧹 Nitpick comments (1)
header/src/main/java/org/zstack/header/storage/addon/primary/ExternalPrimaryStorageInventory.java (1)
26-28:Class.newInstance()已在 Java 9+ 中被弃用。建议使用
clz.getDeclaredConstructor().newInstance()替代，以避免弃用警告和潜在的兼容性问题。♻️ 建议修改
try { - ExternalPrimaryStorageConfig instance = clz.newInstance(); + ExternalPrimaryStorageConfig instance = clz.getDeclaredConstructor().newInstance(); configClassRegistry.put(instance.getIdentity(), clz); } catch (Exception e) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/storage/addon/primary/ExternalPrimaryStorageInventory.java` around lines 26 - 28, Replace the deprecated clz.newInstance() call in ExternalPrimaryStorageInventory with clz.getDeclaredConstructor().newInstance(): instantiate ExternalPrimaryStorageConfig via clz.getDeclaredConstructor().newInstance(), then continue to register instance.getIdentity() into configClassRegistry as before; also update the surrounding exception handling to catch/declare ReflectiveOperationException (or the specific NoSuchMethodException, InstantiationException, IllegalAccessException, InvocationTargetException) so compilation and runtime behavior remain correct.
🤖 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/zbs/src/main/java/org/zstack/storage/zbs/Config.java`:
- Line 28: 当前用于掩码 URI 中凭据的正则 URI_CREDENTIAL_PATTERN =
Pattern.compile(":[^:@]*@") 无法处理密码中含 ':' 或 '@' 的情况；请改为不依赖字符排除的实现：在 Config.java
中移除或替换该正则，改用基于 MdsUri 解析逻辑的显式定位——使用 split(":", 2) 找到用户/密码起始位置并用 lastIndexOf("@")
找到凭据结束位置，然后用这两个索引之间的子串替换为掩码，或将正则改为匹配从第一个 ':' 到最后一个 '@'（非贪婪/明确定位）的模式以确保像
user:pass:word@host 和 user:pass@word@host 这类情况都能正确掩码。
---
Outside diff comments:
In `@plugin/zbs/src/main/java/org/zstack/storage/zbs/MdsInfo.java`:
- Around line 24-39: The equals()/hashCode() implementations in class MdsInfo
currently include the transient field password which will be null after JSON
deserialize; update MdsInfo.equals(Object) and MdsInfo.hashCode() to exclude
password and only compare/hash addr, port and username (i.e. remove any
reference to password in both methods) so object equality and hashing remain
stable across serialization.
---
Nitpick comments:
In
`@header/src/main/java/org/zstack/header/storage/addon/primary/ExternalPrimaryStorageInventory.java`:
- Around line 26-28: Replace the deprecated clz.newInstance() call in
ExternalPrimaryStorageInventory with clz.getDeclaredConstructor().newInstance():
instantiate ExternalPrimaryStorageConfig via
clz.getDeclaredConstructor().newInstance(), then continue to register
instance.getIdentity() into configClassRegistry as before; also update the
surrounding exception handling to catch/declare ReflectiveOperationException (or
the specific NoSuchMethodException, InstantiationException,
IllegalAccessException, InvocationTargetException) so compilation and runtime
behavior remain correct.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
Run ID: 7665dddd-7950-434d-84bc-85c029431843
📒 Files selected for processing (5)
header/src/main/java/org/zstack/header/storage/addon/primary/ExternalPrimaryStorageConfig.javaheader/src/main/java/org/zstack/header/storage/addon/primary/ExternalPrimaryStorageInventory.javaplugin/zbs/src/main/java/org/zstack/storage/zbs/Config.javaplugin/zbs/src/main/java/org/zstack/storage/zbs/MdsInfo.javatest/src/test/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsPrimaryStorageCase.groovy
| public Pool() {} | ||
| } | ||
|
|
||
| private static final Pattern URI_CREDENTIAL_PATTERN = Pattern.compile(":[^:@]*@"); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 查找 MdsUri 类的实现，确认密码中特殊字符的处理方式
ast-grep --pattern $'class MdsUri {
$$$
}'
# 搜索密码编码相关的处理逻辑
rg -n "encode|decode|URLEncoder|URLDecoder" --type java -g "**/zbs/**"Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
# 搜索 MdsUri 相关的所有文件
find . -type f -name "*MdsUri*" -o -name "*Uri*" | grep -i zbs | head -20Repository: MatheMatrix/zstack
Length of output: 123
🏁 Script executed:
# 搜索包含 mdsUrl 或 URI 的代码
rg -n "mdsUrl|MdsUri" --type java -A 3 -B 3 | head -100Repository: MatheMatrix/zstack
Length of output: 9709
🏁 Script executed:
# 查看 Config.java 文件的完整内容
find . -type f -path "**/zbs/**/Config.java" | head -5Repository: MatheMatrix/zstack
Length of output: 123
🏁 Script executed:
# 搜索正则表达式的使用情况
rg -n "URI_CREDENTIAL_PATTERN" --type javaRepository: MatheMatrix/zstack
Length of output: 367
🏁 Script executed:
# 查看 desensitize 方法的调用情况和上下文
rg -n "desensitize" --type java -A 5 -B 5 | head -80Repository: MatheMatrix/zstack
Length of output: 7898
🏁 Script executed:
# 查看 MdsUri.java 的完整内容，特别是密码处理
cat -n plugin/zbs/src/main/java/org/zstack/storage/zbs/MdsUri.java | head -150Repository: MatheMatrix/zstack
Length of output: 5924
🏁 Script executed:
# 查看 Config.java 的完整内容
cat -n plugin/zbs/src/main/java/org/zstack/storage/zbs/Config.javaRepository: MatheMatrix/zstack
Length of output: 2872
🏁 Script executed:
# 搜索是否有任何 URLEncoder 或类似的编码处理
rg -n "URLEncoder|URLDecoder|encode|decode" plugin/zbs/src/main/java/org/zstack/storage/zbs/ -A 3 -B 3Repository: MatheMatrix/zstack
Length of output: 44
正则表达式无法正确处理密码中包含特殊字符的情况。
MdsUri 的解析逻辑支持密码中包含 : 和 @ 字符（通过 split(":", 2) 和 lastIndexOf("@")），但当前正则 :[^:@]*@ 无法匹配这些情况：
user:pass:word@host- 密码含:，正则无法匹配user:pass@word@host- 只匹配:pass@，留下word@host未掩码
应修改正则或采用其他方式来正确掩码含特殊字符的密码。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/zbs/src/main/java/org/zstack/storage/zbs/Config.java` at line 28,
当前用于掩码 URI 中凭据的正则 URI_CREDENTIAL_PATTERN = Pattern.compile(":[^:@]*@") 无法处理密码中含
':' 或 '@' 的情况；请改为不依赖字符排除的实现：在 Config.java 中移除或替换该正则，改用基于 MdsUri 解析逻辑的显式定位——使用
split(":", 2) 找到用户/密码起始位置并用 lastIndexOf("@")
找到凭据结束位置，然后用这两个索引之间的子串替换为掩码，或将正则改为匹配从第一个 ':' 到最后一个 '@'（非贪婪/明确定位）的模式以确保像
user:pass:word@host 和 user:pass@word@host 这类情况都能正确掩码。
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.
问题
ZSTAC-80664: ZBS主存储
addonInfo.mdsInfos[].password和config.mdsUrls凭证未脱敏。方案
1. addonInfo 不再存储密码
MdsInfo.password加transient，密码只保留在config.mdsUrls2. config 通过 desensitize 接口脱敏
ExternalPrimaryStorageConfig接口（getIdentity()+desensitize()）ExternalPrimaryStorageInventory自动扫描子类，构建 inventory 时调用desensitize()脱敏 configConfig实现接口，将root:password@host脱敏为root:*****@host3. 对插件开发者
只需 Config 类实现
ExternalPrimaryStorageConfig，ZStack 自动扫描注册，零额外代码。改动文件
ExternalPrimaryStorageConfig.java— 新增接口ExternalPrimaryStorageInventory.java— 自动扫描 + desensitize 调用，移除 ye.zou 的手动脱敏Config.java— 实现接口MdsInfo.java— password 加 transient + @nologgingZbsPrimaryStorageCase.groovy— 断言移除 password测试
Resolves: ZSTAC-80664
sync from gitlab !9395