Added support for Auditing Issues/Vulnerabilities in SSC#919
Added support for Auditing Issues/Vulnerabilities in SSC#919jmadhur87 wants to merge 6 commits intofortify:dev/v3.xfrom
Conversation
…87/fcli into mjain6/SSCAuditingUpdate
SangameshV
left a comment
SangameshV
left a comment
There was a problem hiding this comment.
Hi @jmadhur87 , pleae have a look at the comments and share your inputs.
| List<SSCIssueIdentifier> issues = fetchIssueRevisionsFromSSC(unirest, appVersionId, issueIds); | ||
|
|
||
| if (StringUtils.isNotBlank(assignUser)) { | ||
| executeAssignUserRequest(unirest, appVersionId, issues, assignUser); |
There was a problem hiding this comment.
Is it required to pass unirest here? Can make use of the class level unirestInstance.
| if (StringUtils.isNotBlank(assignUser)) { | ||
| executeAssignUserRequest(unirest, appVersionId, issues, assignUser); | ||
| if (isAuditRequired()) { | ||
| issues = fetchIssueRevisionsFromSSC(unirest, appVersionId, issueIds); |
There was a problem hiding this comment.
Refer to the already given comment.
Is it required to pass unirest here? Can make use of the class-level unirestInstance.
| } | ||
|
|
||
| if (isAuditRequired()) { | ||
| executeAuditRequest(unirest, appVersionId, issues); |
There was a problem hiding this comment.
Same comment as before.
| } | ||
| } | ||
|
|
||
| private boolean isAuditRequired() { |
There was a problem hiding this comment.
Since you are updating the issue to either suppress it, assign a user, add a comment or update the custom tags, AND nt just auditing, it makes more sense to change the function name to isUpdateRequired.
| result.put("suppressed", suppress); | ||
| } | ||
|
|
||
| ArrayNode resultsArray = JsonHelper.getObjectMapper().createArrayNode(); |
There was a problem hiding this comment.
Why create an arrayNode when you can return the already-created ObjectNode result? Are we returning an array or a single object node?
| @Option(names = {"--assign-user"}) | ||
| private String assignUser; | ||
|
|
||
| private UnirestInstance unirestInstance; |
There was a problem hiding this comment.
As discussed before, command classes shouldn't have any instance fields, other than those annotation with picocli annotations like @Option or @Mixin. Please remove the unirestInstance field, and just pass to the methods that need it. If necessary, you can create a separate class that takes UnirestInstance as a constructor parameter, and have that class implement most of the logic, i.e.., in the getJsonNode method you could do something like return new MyHelper(unirest).performWork();. But only do this if such an extra class makes things significantly simpler.
3 participants
feat: Added support for Auditing Issues/Vulnerabilities in SSC using FCLI
Introduce new SSC issue update CLI command to support updating SSC issues from the CLI.
Supports options: --issue-ids (required), --custom-tags / -t (TAG=VALUE, comma-separated), --suppress (true|false), --comment (text), and --assign-user (username or id).