JENKINS-62208 Prevent file contents from being printed during transfers#175
Conversation
Updated the defineRemote method to accept an enableInteraction flag, disabling interaction (and thus log piping) during file transfers in put and get methods. This prevents file contents from being printed to the logs, addressing a potential security and verbosity issue.
There was a problem hiding this comment.
Pull request overview
This PR addresses JENKINS-62208 by preventing file contents from being printed to Jenkins logs during SSH file transfers. The fix adds an enableInteraction flag to the defineRemote method, which when set to false, disables the interaction block that pipes output to logs.
- Added optional
enableInteractionparameter todefineRemotemethod with backward-compatible default value oftrue - Modified
putandgetmethods to explicitly disable interaction during file transfers - Updated interaction block setup to be conditional based on the
enableInteractionflag
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Introduce security-focused tests in GetStepTest and PutStepTest to ensure that file transfer operations (get/put) do not leak file contents or sensitive information to the Jenkins console. These tests validate that the SSHService methods are called and that the security fix disabling interaction blocks during file transfers is effective.
|
@kirk-fitz Would you be able to verif this incremental is working properly on your Jenkins? https://repo.jenkins-ci.org/incrementals/org/jenkins-ci/plugins/ssh-steps/2.0.89.vccd94d21c78a_/ |
|
@nrayapati glad to see some movement on it, not sure if you got to look at my previous pr It was not complete, but maybe you got inspiration from it, I will review and test over the coming days |
Thanks for your earlier work on #158! I actually approached this differently - rather than filtering at runtime based on SCP mode and verbose flag, I disabled the interaction block entirely for all file transfer operations (both SCP and SFTP) at definition time via the enableInteraction parameter. This is more generic and prevents the issue at the source for sshGet, sshPut, and any future file transfer steps. The interaction block now only captures output for commands and scripts where users actually want to see the output. If anyone needs file transfer debugging, they can still use the remote.logLevel setting to enable groovy-ssh's internal logging separately. Looking forward to your review and testing feedback! :) |
|
Thanks for the detailed explanation, Your approach is cleaner and more robust. My PR was really a narrow fix aimed at suppressing the SCP output I was personally hitting, and it didn’t address SFTP or future transfer steps, nor did it come with tests. |
kirk-fitz
left a comment
kirk-fitz
left a comment
There was a problem hiding this comment.
Changes look good, have tested locally
Thank you very much! |
Updated the defineRemote method to accept an enableInteraction flag, disabling interaction (and thus log piping) during file transfers in put and get methods. This prevents file contents from being printed to the logs, addressing a potential security and verbosity issue.
Description
See JENKINS-62208.
Submitter checklist
Reviewer checklist