-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix that log sensitive infomation in cmd of script #12024
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.
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
base: 4.20
Are you sure you want to change the base?
fix that log sensitive infomation in cmd of script #12024
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.20 #12024 +/- ##
============================================
- Coverage 16.18% 16.18% -0.01%
+ Complexity 13305 13303 -2
============================================
Files 5657 5657
Lines 498466 498485 +19
Branches 60491 60491
============================================
- Hits 80696 80677 -19
- Misses 408789 408828 +39
+ Partials 8981 8980 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15701 |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-14794)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR addresses a security issue where sensitive information (such as passwords) was being logged in plain text within the Script class. The fix introduces a new addSensitive() method to mark specific command arguments as sensitive, ensuring they are masked with "******" in logs and command-line representations.
Key changes:
- Added
sensitiveArgIndicesSet to track which arguments contain sensitive data - Implemented
addSensitive()method for explicitly marking sensitive arguments - Updated all logging statements throughout the
execute()methods to conditionally log sanitized messages when sensitive arguments are present
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 13 comments.
| File | Description |
|---|---|
| Script.java | Core implementation adding sensitive argument tracking and comprehensive logging changes to mask sensitive data |
| ScriptTest.java | Test cases validating that sensitive arguments are properly masked in command-line output |
| LibvirtUpdateHostPasswordCommandWrapper.java | Updated to use addSensitive() for password arguments |
| CitrixUpdateHostPasswordCommandWrapper.java | Updated logging to mask password in debug output |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
…ithub.com/YLChen-007/cloudstack into my-fix-420-mask-script-cmd-sensitive-info
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
I will delete this import. |
Description
This PR fixes that log sensitive infomation in cmd of cloud.utils.script.Script. #12005