Skip to content
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

[JENKINS-74018] Migrate legacy checkUrl attributes in BapSshTransfer/config.jelly #360

Merged
merged 2 commits into from
Oct 29, 2024

Conversation

yaroslavafenkin
Copy link
Contributor

https://issues.jenkins.io/browse/JENKINS-74018

  • Removed checkUrl attribute from the affected fields and renamed the parameter names in validation methods to let CheckMethod do all the work.
  • Replaced poj:textarea with f:textarea. poj:textarea is provided by https://plugins.jenkins.io/publish-over/ which had it's last release 7 years ago. It doesn't have some of the attributes that modern f:textarea has, and also stands out visually from the rest of the fields. This change is probably not needed for the form validation to work properly without checkUrl attribute, I haven't checked. Let me know if I should revert this to keep the PR clean.

Testing done

Configure an SSH server on the Global Configuration page. Create a new freestyle job, in the Environment section check "Send files or execute commands over SSH before the build starts". In Transfer Set section fill Source files and Exec command fields to trigger the affected validation methods.

Before the change
After the change

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@@ -53,26 +53,26 @@
return FormValidation.validateNonNegativeInteger(value);
}

public FormValidation doCheckSourceFiles(@QueryParameter final String configName, @QueryParameter final String sourceFiles,
public FormValidation doCheckSourceFiles(@QueryParameter final String sourceFilesConfigName, @QueryParameter final String value,

Check warning

Code scanning / Jenkins Security Scan

Stapler: Missing POST/RequirePOST annotation Warning

Potential CSRF vulnerability: If BapSshTransferDescriptor#doCheckSourceFiles connects to user-specified URLs, modifies state, or is expensive to run, it should be annotated with @POST or @RequirePOST
@@ -53,26 +53,26 @@
return FormValidation.validateNonNegativeInteger(value);
}

public FormValidation doCheckSourceFiles(@QueryParameter final String configName, @QueryParameter final String sourceFiles,
public FormValidation doCheckSourceFiles(@QueryParameter final String sourceFilesConfigName, @QueryParameter final String value,

Check warning

Code scanning / Jenkins Security Scan

Stapler: Missing permission check Warning

Potential missing permission check in BapSshTransferDescriptor#doCheckSourceFiles
}

public FormValidation doCheckPatternSeparator(@QueryParameter final String value) {
return BPValidators.validateRegularExpression(value);
}

public FormValidation doCheckExecCommand(@QueryParameter final String sourceFiles, @QueryParameter final String execCommand) {
return checkTransferSet(sourceFiles, execCommand);
public FormValidation doCheckExecCommand(@QueryParameter final String sourceFiles, @QueryParameter final String value) {

Check warning

Code scanning / Jenkins Security Scan

Stapler: Missing permission check Warning

Potential missing permission check in BapSshTransferDescriptor#doCheckExecCommand
}

public FormValidation doCheckPatternSeparator(@QueryParameter final String value) {
return BPValidators.validateRegularExpression(value);
}

public FormValidation doCheckExecCommand(@QueryParameter final String sourceFiles, @QueryParameter final String execCommand) {
return checkTransferSet(sourceFiles, execCommand);
public FormValidation doCheckExecCommand(@QueryParameter final String sourceFiles, @QueryParameter final String value) {

Check warning

Code scanning / Jenkins Security Scan

Stapler: Missing POST/RequirePOST annotation Warning

Potential CSRF vulnerability: If BapSshTransferDescriptor#doCheckExecCommand connects to user-specified URLs, modifies state, or is expensive to run, it should be annotated with @POST or @RequirePOST
@basil basil added the internal label Oct 29, 2024
Copy link
Member

@basil basil left a comment

Choose a reason for hiding this comment

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

Thanks!

@basil basil merged commit d76ba0f into jenkinsci:main Oct 29, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants