fortify / fcli

fcli is a command-line utility for interacting with various Fortify products
https://fortify.github.io/fcli/
Other
33 stars 22 forks source link

SC-SAST: Controller error when combining `--no-upload` with `--appversion` option #399

Closed rsenden closed 1 year ago

rsenden commented 1 year ago

The fcli --no-upload option will drop the uploadToken parameter from the scan request to ScanCentral SAST Controller. The --no-upload option doesn't change the behavior of the --appversion option; if --appversion is specified, fcli will always send the pvId parameter to the controller, regardless of whether --no-upload was specified.

However, apparently SC-SAST Controller 23.1 (and likely other versions) requires uploadToken to be provided when pvId is included in the scan request, resulting in the controller returning an error if both --no-upload and --appversion options have been specified on the fcli sc-sast scan start command.

After some discussion we came up with 2 approaches to fix this:

  1. Leave the current command structure as is and discard any appversion value if the --no-upload flag is set
  2. Change the --appversion parameter to --upload-to <appversion> and remove the --no-upload flag

Option 1 is more in line with current ScanCentral Client behavior, and would allow for potential future situations where the controller does expect pvId to be present for functionality other than uploading the results, for example to allow users to run a scan without uploading the results if pool_mapping_mode is set to ENFORCED.

However, until such use cases would be actually supported by the controller, keeping both --appversion and --no-upload options could be confusing for users, as to why fcli allows for specifying an application version if upload is disabled.

Option 2 avoids such potential confusion and therefore seems more intuitive from a user perspective.

This issue is related to #393.

rsenden commented 1 year ago

Instead of 'upload', we should consider 'publish' instead as this is more consistent with SC DAST terminology, and also more clear that this refers to publishing the scan results to SSC, as opposed to referring to uploading the scan payload from client to controller.

Some additional considerations for option 1 above:

Taking all of the above into account, the two options for fixing this issue become as follows:

  1. Change --no-upload to --publish (alter option behavior as described above), rename --appversion to --ssc-appversion
  2. Change --appversion option to --publish-to <appversion>, and remove the --no-upload flag