You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hop.apache.org by "zanarellidev (via GitHub)" <gi...@apache.org> on 2026/08/25 18:23:33 UTC
[PR] feat(action): support hidden arguments in Shell action to prevent credential leaks in execution logs (#4935) (hop)
zanarellidev opened a new pull request, #8094:
URL: https://github.com/apache/hop/pull/8094
<!--
Thank you for contributing to Apache Hop! Please read the following before submitting your pull request.
-->
### Description
Fixes #4935.
When running Shell actions with arguments (e.g. API keys, database credentials, passwords, or tokens), Hop currently logs the raw execution command with all arguments visible in the execution log at `BASIC` log level. This creates a security risk / credential exposure in log sinks, execution history, and UI logs.
This PR adds support for **Hidden Arguments** in the Shell Action:
1. **POJO & Metadata**: Added static inner class `ActionShell.ShellArgument` with `value` and `hidden` properties.
2. **Backward Compatibility**: Fully backward compatible with legacy pipeline/workflow XML definitions (via legacy string parsing in `loadXml`).
3. **Secure Log Masking**: Replaces hidden arguments with `***` in execution log outputs (`logBasic`), while preserving raw unmasked argument values for the operating system process execution via `ProcessBuilder`.
4. **GUI / Dialog**: Added a `Hidden (Y/N)` CCombo column in `ActionShellDialog` for easy GUI configuration.
5. **Tests**: Added unit tests in `WorkflowActionShellLoadSaveTest.java` covering `ShellArgument` serialization and hidden flag behavior. All 8 tests in the module pass cleanly.
### Testing
- [x] Unit tests pass via `mvn test`
- [x] Spotless code format applied
------------------------
Thank you for your review!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(action): support hidden arguments in Shell action to prevent credential leaks in execution logs (#4935) (hop)
Posted by "mattcasters (via GitHub)" <gi...@apache.org>.
mattcasters commented on PR #8094:
URL: https://github.com/apache/hop/pull/8094#issuecomment-5438484376
Thanks a lot @zanarellidev! Great work. I took the liberty of adding an integration test and fixing/cleaning up the Shell action dialog.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(action): support hidden arguments in Shell action to prevent credential leaks in execution logs (#4935) (hop)
Posted by "hansva (via GitHub)" <gi...@apache.org>.
hansva merged PR #8094:
URL: https://github.com/apache/hop/pull/8094
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(action): support hidden arguments in Shell action to prevent credential leaks in execution logs (#4935) (hop)
Posted by "zanarellidev (via GitHub)" <gi...@apache.org>.
zanarellidev commented on PR #8094:
URL: https://github.com/apache/hop/pull/8094#issuecomment-5438628899
Wow Matt, thank you so much! That really made my day! It's a true honor to contribute to Apache Hop alongside such incredible people.
Thank you as well for the extra polish on the dialog and adding the integration test.
Wishing you all the best, take care and God bless!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(action): support hidden arguments in Shell action to prevent credential leaks in execution logs (#4935) (hop)
Posted by "zanarellidev (via GitHub)" <gi...@apache.org>.
zanarellidev commented on PR #8094:
URL: https://github.com/apache/hop/pull/8094#issuecomment-5431005715
Hi @mattcasters, thank you for the review!
I have updated the PR according to your feedback:
1. **Removed `loadXml()`** from `ActionShell.java`.
2. **Implemented `ILegacyXml`**:
- `ShellArgument` implements `ILegacyXml` to parse values from previous Hop XMLs (`<arguments><argument>value</argument></arguments>`).
- `ActionShell` implements `ILegacyXml` to handle older numbered tags (`<argument0>val0</argument0>`).
3. **Simplified the diff**: Reverted duplicate command-building branches in `executeShell`, isolating log masking into a clean `buildLogCommand` helper.
4. **Backward Compatibility Tests**: Added comprehensive tests in `WorkflowActionShellLoadSaveTest` verifying:
- Backward compatibility loading old numbered arguments (`<argument0>`, `<argument1>`).
- Backward compatibility loading previous `<arguments><argument>val</argument></arguments>` format.
- Serialization & round-trip with the new structure (`<value>`, `<hidden>`).
- Command masking in execution logs.
All tests and spotless checks pass cleanly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(action): support hidden arguments in Shell action to prevent credential leaks in execution logs (#4935) (hop)
Posted by "mattcasters (via GitHub)" <gi...@apache.org>.
mattcasters commented on PR #8094:
URL: https://github.com/apache/hop/pull/8094#issuecomment-5430826896
Please remove method `loadXml()` and assure backward compatibility with an integration test.
If needed, implement an ILegacyXml method.
The code change should be very simple and for some reason it's hard to see what is going on.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org