Skip to content

[ZSTAC-65483][4.8.38] Mask invalid userdata tag#4304

Open
MatheMatrix wants to merge 1 commit into
4.8.38from
sync/ye.zou/fix/ZSTAC-65483-4.8.38
Open

[ZSTAC-65483][4.8.38] Mask invalid userdata tag#4304
MatheMatrix wants to merge 1 commit into
4.8.38from
sync/ye.zou/fix/ZSTAC-65483-4.8.38

Conversation

@MatheMatrix

Copy link
Copy Markdown
Owner

Summary

Fix HTTP 500 when querying userdata system tags with Rest.maskSensitiveInfo enabled and malformed userdata content.

Root Cause

VmSystemTags.UserdataTagOutputHandler decoded userdata and parsed it as YAML without handling SnakeYAML runtime parse errors. If the userdata looked like cloud-config but had invalid indentation, the sensitive-output masking path threw during query response serialization.

Change

  • Catch YAML parsing failures in userdata sensitive tag masking.
  • Replace the whole userdata token with ***** when parsing fails, avoiding both the API 500 and plaintext exposure.
  • Add focused JUnit coverage for malformed userdata and valid chpasswd.list structured masking.

Verification

Ran in PR Docker through scripts/verify-case-docker.sh:

cd /zstack && mvn -pl compute,testlib -am -DskipTests -DskipJacoco=true install
cd /zstack/test && mvn test -Dtest=org.zstack.test.userdata.TestUserdataTagOutputHandler -Dsurefire.useFile=false -DskipJacoco=true

Result:

Tests run: 2, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

Risk

Scope is limited to userdata sensitive tag output masking. Full CI pending.

Resolves: ZSTAC-65483

sync from gitlab !10262

Malformed userdata can break the sensitive-output masking path when Rest.maskSensitiveInfo is enabled. The handler now falls back to replacing the whole userdata token if YAML parsing fails, preserving query success without exposing plaintext.

Constraint: ZStack 4.8.38 verification must run inside PR docker instead of host Maven
Rejected: Return original tag on YAML parse failure | would keep plaintext userdata visible when masking is enabled
Confidence: high
Scope-risk: narrow
Tested: Docker verify-case: mvn -pl compute,testlib -am -DskipTests -DskipJacoco=true install; mvn test -Dtest=org.zstack.test.userdata.TestUserdataTagOutputHandler -Dsurefire.useFile=false -DskipJacoco=true
Not-tested: Full CI pipeline

Resolves: ZSTAC-65483

Change-Id: Icd82dcc2f8ec8972e6b238f9e9a63186facfe1a3
@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown

Review Change Stack

概览

VmSystemTags.UserdataTagOutputHandler.desensitizeTag 中,对 yaml.load(userdata) 调用增加 try/catch,捕获 RuntimeException 时将当前 token 值替换为 "*****" 并跳过后续处理。同步新增单元测试类覆盖畸形与正常 cloud-config 两种场景。

变更内容

userdata 脱敏异常处理与测试

Layer / File(s) 说明
yaml.load 异常处理及单元测试
compute/src/main/java/org/zstack/compute/vm/VmSystemTags.java, test/src/test/java/org/zstack/test/userdata/TestUserdataTagOutputHandler.java
desensitizeTagyaml.load 包裹在 try/catch 中,RuntimeException 时将 token 值置为 "*****"continue;新增测试类通过反射注入敏感注解,分别断言畸形 userdata 被完整替换为 "*****" 以及结构化 cloud-config 保留结构但敏感字段被掩码。

估计代码审查难度

🎯 2 (Simple) | ⏱️ ~10 minutes

小诗

🐇 YAML 若报错,莫慌莫停顿,
星号五颗闪,秘密不外露。
兔子跳一跳,异常轻轻绕,
测试把关好,脱敏稳如山。✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed 标题准确反映了主要变化:修复无效userdata标签的掩码处理,对应于PR的核心目标——处理格式错误的userdata内容时避免HTTP 500错误。
Description check ✅ Passed 描述充分关联到变更集,清晰阐述了问题根源、修复方案和验证步骤,与文件变更内容保持一致。
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sync/ye.zou/fix/ZSTAC-65483-4.8.38

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with 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.

Inline comments:
In
`@test/src/test/java/org/zstack/test/userdata/TestUserdataTagOutputHandler.java`:
- Line 12: The test class `TestUserdataTagOutputHandler` does not follow the
naming convention which requires test classes to end with `Test` or `Case`
suffix. Rename the class from `TestUserdataTagOutputHandler` to
`UserdataTagOutputHandlerTest` and also rename the file accordingly from
`TestUserdataTagOutputHandler.java` to `UserdataTagOutputHandlerTest.java` to
maintain consistency with the coding guidelines and standard test naming
conventions.
🪄 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: 8a8e8df7-41aa-431a-b74a-740e38390784

📥 Commits

Reviewing files that changed from the base of the PR and between 9c57b7b and b705ba9.

📒 Files selected for processing (2)
  • compute/src/main/java/org/zstack/compute/vm/VmSystemTags.java
  • test/src/test/java/org/zstack/test/userdata/TestUserdataTagOutputHandler.java

import java.util.HashMap;
import java.util.Map;

public class TestUserdataTagOutputHandler {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

测试类命名未满足约定后缀。

TestUserdataTagOutputHandler 未以 TestCase 结尾,建议重命名为 UserdataTagOutputHandlerTest(并同步文件名),以保持测试命名一致性。
As per coding guidelines, “测试类需要以 TestCase 结尾”。

🤖 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
`@test/src/test/java/org/zstack/test/userdata/TestUserdataTagOutputHandler.java`
at line 12, The test class `TestUserdataTagOutputHandler` does not follow the
naming convention which requires test classes to end with `Test` or `Case`
suffix. Rename the class from `TestUserdataTagOutputHandler` to
`UserdataTagOutputHandlerTest` and also rename the file accordingly from
`TestUserdataTagOutputHandler.java` to `UserdataTagOutputHandlerTest.java` to
maintain consistency with the coding guidelines and standard test naming
conventions.

Source: Coding guidelines

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants