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

Show security warnings by default (#258) #522

Merged
merged 2 commits into from
Mar 3, 2023

Conversation

jiakuanghe
Copy link
Contributor

@jiakuanghe jiakuanghe commented Feb 1, 2023

Fixes #258

  • 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

@MarkEWaite MarkEWaite changed the title Security warnings will be shown by default (#258) Show security warnings by default (#258) Feb 2, 2023
@MarkEWaite
Copy link
Contributor

Thanks for the pull request!

The "show all security warnings" is too verbose to be enabled by default. The original request from Daniel was to enable security warnings for the mentioned plugins, not for all plugins. I think that would be much better for the user.

When I run it with the following test case it outputs the list of all plugins that have unresolved security issues, even though I mentioned only one of those plugins:

mkdir -p plugins
rm -rf plugins/*
java -jar jenkins-plugin-manager.jar --jenkins-version 2.361.4 --plugins zap -d plugins 2>&1
...  Many lines of output
youtrack-plugin - Arbitrary code execution vulnerability
youtrack-plugin - Credentials stored in plain text
zap - Credentials stored in plain text
zap-pipeline - Content-Security-Policy protection for user content disabled
zephyr-enterprise-test-management - CSRF vulnerability and missing permission check allow SSRF
zephyr-enterprise-test-management - Credentials stored in plain text
zephyr-for-jira-test-management - Credentials stored in plain text
zephyr-for-jira-test-management - CSRF vulnerability and missing permission checks
zos-connector - Password stored in plain text
zulip - Credentials stored in plain text
War not found, installing all plugins: /usr/share/jenkins/jenkins.war

Security warnings:
zap (1.1.0): SECURITY-1041 Credentials stored in plain text https://jenkins.io/security/advisory/2019-04-03/#SECURITY-1041
Done

Could you undo the enablement (and deprecation) of the all security warnings setting?

The rest of the change looks very good.

README.md Outdated Show resolved Hide resolved
…ngs is false, and --view-security-warnings is true by default (jenkinsci#258)
@jiakuanghe
Copy link
Contributor Author

Thanks for your suggestion. I've edited it and pushed it again. For now, the --view-all-security-warnings is false by default, unless the user enables it to prevent too verbose output in the console.

@MarkEWaite MarkEWaite added the enhancement New feature or request label Mar 3, 2023
Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

Reviewed in Docs Office Hours with @freyam and @StackScribe. Tested and saw expected behavior in general cases.

Wondered during review if we should suppress the Security warnings text if there are no warnings. However, this is a good change whether we suppress the warnings or not.

@MarkEWaite MarkEWaite merged commit 4c4a01f into jenkinsci:master Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Security warnings should be shown by default
3 participants