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

Add CodeQL workflow #858

Closed
wants to merge 16 commits into from
Closed

Add CodeQL workflow #858

wants to merge 16 commits into from

Conversation

sleberknight
Copy link
Member

  • De-lomboks before build
  • Runs extended security and quality suites

* De-lomboks before build
* Runs extended security and quality suites
@sleberknight sleberknight marked this pull request as draft January 29, 2023 15:07
@sleberknight
Copy link
Member Author

Compilation failed because one class (WebTargetHelper) uses Lombok's @Delegate, which (for whatever reason) is not including the delegated methods in the de-lomboked code. This must be something wrong with how the advanced-security/delombok does the delombok, because running a regular delombok (e.g. in IntelliJ or the Lombok Maven plugin, or manually via java -jar lombok.jar delombok ... all produce the expected code. I've skimmed the code of the action and there's nothing obvious, e.g. it doesn't intentionally exclude experimental Lombok features like @Delegate.

@sleberknight sleberknight added the github_actions Pull requests that update Github_actions code label Jan 29, 2023
@sleberknight sleberknight added this to the 2.5.0 milestone Jan 29, 2023
@sleberknight
Copy link
Member Author

This is blocked until #859 is resolved. Then, rebase to this branch.

* Replace Lombok Delegate on the wrapped WebTarget with "real" code
* Make this class implement WebTarget
* Update class-level Javadocs to remove the "Limitations" section
* Fix method javadocs that incorrectly stated "this" is returned
* Fix a few minor grammatical errors in comments

Closes #859
* {@inheritDoc}
*/
@Override
public WebTargetHelper register(Class<?> componentClass, Map<Class<?>, Integer> contracts) {

Check notice

Code scanning / CodeQL

Confusing overloading of methods

Method WebTargetHelper.register(..) could be confused with overloaded method [register](1), since dispatch depends on static types.
* {@inheritDoc}
*/
@Override
public WebTargetHelper register(Class<?> componentClass, Class<?>... contracts) {

Check notice

Code scanning / CodeQL

Confusing overloading of methods

Method WebTargetHelper.register(..) could be confused with overloaded method [register](1), since dispatch depends on static types.
* {@inheritDoc}
*/
@Override
public WebTargetHelper register(Class<?> componentClass, int priority) {

Check notice

Code scanning / CodeQL

Confusing overloading of methods

Method WebTargetHelper.register(..) could be confused with overloaded method [register](1), since dispatch depends on static types.
* {@inheritDoc}
*/
@Override
public WebTargetHelper register(Class<?> componentClass) {

Check notice

Code scanning / CodeQL

Confusing overloading of methods

Method WebTargetHelper.register(..) could be confused with overloaded method [register](1), since dispatch depends on static types.
@sleberknight sleberknight marked this pull request as ready for review January 30, 2023 19:38
@sleberknight sleberknight marked this pull request as draft January 30, 2023 19:39
Fixes #863
Fixes #866
Fixes #878
Sonar failed the last commit, but the auto-merge merged anyway.

(I guess we don't have something correctly set up)
* Change getKeyStore methods so that they return an empty Optional if
  either path or password is null.
* Update Javadocs to match new implementation and explain the intent
  of these methods is to load an existing key store, not create a new one.
* Fix several tests that were asserting that the returned Optional was not
  null instead of it containing a value.

Fixes #861
In logWarning, simply log the type of the object instead of the (user-entered) value.
This provides less information, but avoids the log injection problem.

Fixes #881
Fixes #882
Fix log injection in FilePathValidator and DirectoryPathValidator
by removing the path from the log message. I can't find something
to easily (and with 100% certainty) sanitize the path from all possible
badness, so just removing it entirely.

Add message about the Nul character in FilePathValidator just like the
DirectortPathValidator has. Extracted Nul character check to
package-private method in InternalKiwiValidators.

Fixes #880
Fixes #883
@sleberknight sleberknight marked this pull request as ready for review January 31, 2023 20:37
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sleberknight sleberknight removed this from the 2.5.0 milestone Jan 31, 2023
@sleberknight
Copy link
Member Author

This PR branch is "16 commits ahead, 14 commits behind master" and since I don't want to deal with trying to fix all that, closing this and will create a new PR.

@sleberknight sleberknight deleted the add-codeql-again branch March 14, 2023 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
github_actions Pull requests that update Github_actions code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants