Running FCLI actions fail if you are using a non-default session

kadraman commented 5 months ago

Current Behavior

Running the following action with named session fails as it appears to be looking for default session:

>fcli ssc session login -t YTVmOTllMGUtMjg2Yy00... --session cli-test
>fcli ssc action run check-policy --appversion IWA:main --session cli-test
com.fortify.cli.common.action.runner.ActionRunner$StepProcessingException: Error processing:
  ActionStepFcli(args=TemplateExpression(ssc issue count --av ${parameters.appversion.id} --by FOLDER), name=countsByFolder, forEach=null)
        at com.fortify.cli.common.action.runner.ActionRunner$ActionStepsProcessor.processStep(ActionRunner.java:416)
        at com.fortify.cli.common.action.runner.ActionRunner$ActionStepsProcessor.lambda$processStepEntries$0(ActionRunner.java:395)
        at java.base@21.0.2/java.util.ArrayList.forEach(ArrayList.java:1596)
        at com.fortify.cli.common.action.runner.ActionRunner$ActionStepsProcessor.processStepEntries(ActionRunner.java:395)
        at com.fortify.cli.common.action.runner.ActionRunner$ActionStepsProcessor.processStep(ActionRunner.java:383)
        at java.base@21.0.2/java.util.ArrayList.forEach(ArrayList.java:1596)
        at com.fortify.cli.common.action.runner.ActionRunner$ActionStepsProcessor.processSteps(ActionRunner.java:371)
        at com.fortify.cli.common.action.runner.ActionRunner$ActionStepsProcessor.processSteps(ActionRunner.java:367)
        at com.fortify.cli.common.action.runner.ActionRunner.run(ActionRunner.java:164)
        at com.fortify.cli.common.action.cli.cmd.AbstractActionRunCommand.run(AbstractActionRunCommand.java:71)
        at com.fortify.cli.common.action.cli.cmd.AbstractActionRunCommand.call(AbstractActionRunCommand.java:62)
        at com.fortify.cli.common.action.cli.cmd.AbstractActionRunCommand.call(AbstractActionRunCommand.java:39)
        at picocli.CommandLine.executeUserObject(CommandLine.java:2118)
        at picocli.CommandLine$RunLast.executeUserObjectOfLastSubcommandWithSameParent(CommandLine.java:2538)
        at picocli.CommandLine$RunLast.handle(CommandLine.java:2530)
        at picocli.CommandLine$RunLast.handle(CommandLine.java:2492)
        at picocli.CommandLine$AbstractParseResultHandler.execute(CommandLine.java:2350)
        at picocli.CommandLine$RunLast.execute(CommandLine.java:2494)
        at picocli.CommandLine.execute(CommandLine.java:2247)
        at com.fortify.cli.app.runner.DefaultFortifyCLIRunner.run(DefaultFortifyCLIRunner.java:49)
        at com.fortify.cli.app.FortifyCLI.execute(FortifyCLI.java:38)
        at com.fortify.cli.app.FortifyCLI.main(FortifyCLI.java:32)
        at java.base@21.0.2/java.lang.invoke.LambdaForm$DMH/sa346b79c.invokeStaticInit(LambdaForm$DMH)
Caused by: java.lang.RuntimeException: Error executing
        at com.fortify.cli.common.util.OutputCollector.collectOutput(OutputCollector.java:37)
        at com.fortify.cli.common.cli.util.FcliCommandExecutor.execute(FcliCommandExecutor.java:64)
        at com.fortify.cli.common.cli.util.FcliCommandExecutor.execute(FcliCommandExecutor.java:75)
        at com.fortify.cli.common.action.runner.ActionRunner$ActionStepsProcessor.processFcliStep(ActionRunner.java:653)
        at com.fortify.cli.common.action.runner.ActionRunner$ActionStepsProcessor.processStep(ActionRunner.java:410)
        ... 22 more
Caused by: com.fortify.cli.common.action.runner.ActionRunner$StepProcessingException: Fcli command threw an exception
        at com.fortify.cli.common.cli.util.FcliCommandExecutor._execute(FcliCommandExecutor.java:83)
        at com.fortify.cli.common.cli.util.FcliCommandExecutor.lambda$execute$0(FcliCommandExecutor.java:64)
        at com.fortify.cli.common.util.OutputCollector.collectOutput(OutputCollector.java:32)
        ... 26 more
Caused by: picocli.CommandLine$ExecutionException: Error while calling command (com.fortify.cli.ssc.issue.cli.cmd.SSCIssueCountCommand@daafc8d): java.lang.IllegalStateException: SSC session 'default' not found, please login first
        at picocli.CommandLine.executeUserObject(CommandLine.java:2127)
        at picocli.CommandLine$RunLast.executeUserObjectOfLastSubcommandWithSameParent(CommandLine.java:2538)
        at picocli.CommandLine$RunLast.handle(CommandLine.java:2530)
        at picocli.CommandLine$RunLast.handle(CommandLine.java:2492)
        at picocli.CommandLine$AbstractParseResultHandler.execute(CommandLine.java:2350)
        at picocli.CommandLine$RunLast.execute(CommandLine.java:2494)
        at com.fortify.cli.common.cli.util.FcliCommandExecutor._execute(FcliCommandExecutor.java:81)
        ... 28 more
Caused by: java.lang.IllegalStateException: SSC session 'default' not found, please login first
        at com.fortify.cli.common.session.helper.AbstractSessionHelper.checkSessionExists(AbstractSessionHelper.java:136)
        at com.fortify.cli.common.session.helper.AbstractSessionHelper.get(AbstractSessionHelper.java:43)
        at com.fortify.cli.ssc._common.session.cli.mixin.SSCUnirestInstanceSupplierMixin.getSessionDescriptor(SSCUnirestInstanceSupplierMixin.java:25)
        at com.fortify.cli.ssc._common.session.cli.mixin.SSCUnirestInstanceSupplierMixin.getSessionDescriptor(SSCUnirestInstanceSupplierMixin.java:22)
        at com.fortify.cli.common.session.cli.mixin.AbstractSessionDescriptorSupplierMixin.getSessionDescriptor(AbstractSessionDescriptorSupplierMixin.java:24)
        at com.fortify.cli.common.session.cli.mixin.AbstractSessionUnirestInstanceSupplierMixin.getUnirestInstance(AbstractSessionUnirestInstanceSupplierMixin.java:27)
        at com.fortify.cli.ssc._common.output.cli.cmd.AbstractSSCOutputCommand.getUnirestInstance(AbstractSSCOutputCommand.java:32)
        at com.fortify.cli.ssc._common.output.cli.cmd.AbstractSSCBaseRequestOutputCommand.getBaseRequest(AbstractSSCBaseRequestOutputCommand.java:23)
        at com.fortify.cli.common.output.cli.cmd.AbstractOutputCommand.call(AbstractOutputCommand.java:31)
        at com.fortify.cli.common.output.cli.cmd.AbstractOutputCommand.call(AbstractOutputCommand.java:22)
        at picocli.CommandLine.executeUserObject(CommandLine.java:2118)
        ... 34 more

Expected Behavior

Running with default session works:

>fcli ssc session login -t YTVm...
>fcli ssc action run check-policy --appversion IWA:main
PASS: No critical issues allowed
FAIL: No new issues allowed
PASS: No new critical issues allowed

Steps To Reproduce

No response

Environment

- OS: Windows 10
- SSC: 24.2.0
- fcli: 2.4.0

Anything else?

No response

rsenden commented 5 months ago

@kadraman, thanks for reporting. The session name given on the action run command line will be used for REST calls made directly from the action (using requests keyword), although I don't think I've ever explicitly tested this scenario.

The problem in this case is that the fcli keyword is used to invoke another fcli command like fcli ssc issue count, and this command isn't aware of the session name that was passed on the action run command. We need to think about the best way to make any fcli commands invoked from an action aware of the session name with which the action itself was invoked.

Several potential ways to fix this come to mind:

Automatically have the action framework pass the --session option to any (relevant) fcli commands invoked from an action. We'd probably need to check whether the command being invoked supports --session and is in the same module as the action itself; potentially an action could invoke fcli tool, fcli util or fcli config commands that don't accept --session, or potentially an SSC action could invoke fcli sc-sast or fcli sc-dast modules that have their own sessions. We should probably also check that --session wasn't explicitly specified in fcli statement in the action file.
Upon action run command execution, set an fcli.env.FCLI_DEFAULT_<module>_SESSION system property (and potentially unset on action completion) with the given session name; this should be picked up as the default session name for any <module> commands invoked from the action. See EnvHelper::env() and FortifyCLIDefaultValueProvider::resolve() on how such a system property would be resolved as an FCLI_DEFAULT environment variable.
Upon action run command execution, store the provided session/name as a ThreadLocal, and have the session framework check whether this ThreadLocal has a value.
Same as previous, but use a static variable.

I think #2 is easiest to implement, and the session name will be automatically applied to relevant commands in the same module as the action itself only.

Note that this is closely related to #547.

MikeTheSnowman commented 5 months ago

Hey @rsenden . Would adding an optional --<module>-session option to the action templates be a viable option? ie: --ssc-session, --sc-sast-session, --fod-session

rsenden commented 5 months ago

Hi @MikeTheSnowman, until we add full pipeline support (#549), I don't think it will be very common that any action will invoke both fcli ssc and fcli sc-sast commands for example, also because actions explicitly live in either ssc or fod module and we don't have action support in sc-sast and sc-dast modules (yet).

The action run command currently integrates with standard ssc/fod session management, using the same --session option to specify session name as all other ssc/fod commands. For now, I think this is good enough, we only need to implement some mechanism to pass that session/name to other fcli commands invoked from the action. If there are any actions that do need to invoke sc-sast commands from an SSC action, the action itself could potentially take a --sc-sast-session option and explicitly pass that option value to any fcli sc-sast commands invoked by that action.

Ideally, as described in #551, we'd implement functionality to share sessions between SSC and SC-SAST/SC-DAST modules, which would greatly simplify the ability to invoke commands for all 3 modules (whether or not they all get integrated under the ssc module) from an action. Then, only actions that do some kind of integration between SSC and FoD would require both an SSC and an FoD session, but this could again be explicitly handled by the action itself,

kadraman commented 5 months ago

OK great, yes agreed #2 looks like it would be a good interim solution.

fortify / fcli