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

Fix CLI Execution from processing results twice #352

Merged
merged 1 commit into from
Jul 21, 2020

Conversation

kmcdon83
Copy link
Contributor

… for both/all scanners will need future consideration

By submitting a PR to this repository, you agree to the terms within the Checkmarx Code of Conduct. Please see the contributing guidelines for how to create and submit a high-quality PR for this repo.

Description

CLI Execution was processing results twice. This change will resolve this.

Logic was removed related to Future processing as the scans were not properly Async compatible. This should be handle in the future.

Testing

Manual testing against GitLab and GitHub were conducted to validate the fix.

Checklist

  • I have added documentation for new/changed functionality in this PR (if applicable). If documentaiton is a Wiki Update, please indicate desired changes within PR MD Comment
  • All active GitHub checks for tests, formatting, and security are passing
  • The correct base branch is being used

…for both/all scanners will need future consideration
@kmcdon83 kmcdon83 added the bug Something isn't working label Jul 21, 2020
@kmcdon83 kmcdon83 requested a review from AvivCx July 21, 2020 13:34
@cxflowtestuser
Copy link
Collaborator

Scan submitted to Checkmarx

@AvivCx AvivCx requested a review from ofersk July 21, 2020 13:35
@kmcdon83 kmcdon83 merged commit 45fe626 into develop Jul 21, 2020
@ofersk
Copy link
Contributor

ofersk commented Jul 21, 2020

I will update my branch with the changes done in this commit.
@kmcdon83 we need to show you the change I'm implementing (basically, moving the logic in this commit to the vulnerabilityScanner)

@ofersk
Copy link
Contributor

ofersk commented Jul 21, 2020

FYI, @AvivCx and @kmcdon83 there is a potential null pointer in the code (in all new functions, fixed on my branch).
if(flowProperties.getEnabledVulnerabilityScanners() == null || ... // <-- implies nullable
if(flowProperties.getEnabledVulnerabilityScanners().contains(SCA_SCANNER)) // <-- null pointer here

@kmcdon83 kmcdon83 deleted the pr-ts-hotfix/sca-local-cli branch August 13, 2020 02:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants