bug(dast): ccp, sshconfig missing

[jagpreetsinghsasan]

Describe the bug

Due to the incorporation of #3578 task, the DAST again got broken as it now requires a sshconfig, ccp input

To Reproduce

The failing CI test can be seen in every PR at the moment

Expected behavior

The DAST scan should run without failing

Logs/Stack traces

https://github.com/hyperledger-cacti/cacti/actions/runs/12153966297/job/33892946268?pr=3659

Additional context

Adding empty sshConfig and connectionProfile` to this line:

cacti/.github/workflows/.dast-nuclei-cmd-api-server.yaml

Line 88 in fbe68c1

run: jq '.plugins += [{ "packageName":"@hyperledger/cactus-plugin-ledger-connector-fabric", "type":"org.hyperledger.cactus.plugin_import_type.LOCAL", "action":"org.hyperledger.cactus.plugin_import_action.INSTALL", "options":{ "packageSrc":"/home/runner/work/cacti/cacti/packages/cactus-plugin-ledger-connector-fabric/", "instanceId":"some-unique-fabric-connector-instance-id", "peerBinary":"/fabric-samples/bin/peer", "connectionProfile":"{}", "dockerBinary":"usr/local/bin/docker","cliContainerEnv":{"CORE_PEER_LOCALMSPID":"Org1MSP","CORE_PEER_ADDRESS":"peer0.org1.example.com:7051","CORE_PEER_MSPCONFIGPATH":"/opt/gopath/src/github.com/hyperledger/fabric/peer/crypto/peerOrganizations/org1.example.com/users/Admin@org1.example.com/msp","CORE_PEER_TLS_ROOTCERT_FILE":"/opt/gopath/src/github.com/hyperledger/fabric/peer/crypto/peerOrganizations/org1.example.com/peers/peer0.org1.example.com/tls/ca.crt","ORDERER_TLS_ROOTCERT_FILE":"/opt/gopath/src/github.com/hyperledger/fabric/peer/crypto/ordererOrganizations/example.com/orderers/orderer.example.com/msp/tlscacerts/tlsca.example.com-cert.pem"},"discoveryOptions":{"enabled":true,"asLocalhost":true}}}] ' .config.json > .config2.json && mv .config2.json .config.json

shall fix the broken test

[raynatopedrajeta]

Hello Team,

Please assign me this task. Thank you!

Rayn

[petermetz]

@jagpreetsinghsasan Once you have a working fix for this (and it is merged onto main) please also make sure to set the DAST scan to be required in the branch protection rules. That way we prevent it breaking again next time a similar change is made.

One more point: The fact that DAST starting breaking with existing configuration is pointing to the idea that maybe we accidentally created a breaking change in the Fabric connector. Could you please double check? New features should not make previously working functionality break with previously valid configuration (unless we are issuing a new major release where breaking changes are allowed). If it turns out that we did make an accidental breaking change then please open an issue to tackle that by way of investigating what can be done where the two main options are:

1. Revert the change for now and schedule it for v3.0.0
2. Refactor the code so that the new code in the connector is backward compatible (if this is possible then it's the preferred way, but it's not always possible to refactor things with perfect backward compatibility)

[raynatopedrajeta]

Hello @petermetz ,

As per my investigation, DAST on other connectors are working since there is no validation for sshConfig

Here is the validation on plugin-ledger-connector-fabric:

Here is the actual error log:

I want to know if sshConfig is a required field or not. If it is not required, maybe we can remove the validation that throws an error when there is no sshConfig.

I created a PR to perform initial investigation together with the proposed solution that I am looking at to solve this issue.

Cheers!
Rayn

[jagpreetsinghsasan]

@petermetz @aldousalvarez @raynatopedrajeta

We have another breaking change when we added an authorization token to the cmd-api-server healthcheck endpoint in #2693 but this didn't get tested in DAST scan as DAST scan was failing due to another error (the besu aio not starting). When we fixed the besu aio error, the DAST scan started failing due to another PR (where we serialized ccp and sshconfig inputs for fabric connector) and while fixing that, we found the auth token missing for /healthcheck endpoint.

So 2 points here,

1. We have to include refactor(cmd-api-server): pull OAuth2 endpoint scopes from openapi.json #2693 as a breaking change for v3.0 release here: chore(release): publish v3.0.0 #3663
2. For the fix of this task, bug(dast): ccp, sshconfig missing #3671, we can remove fabric based breaking changes and remove the DAST scan check for the healthcheck endpoint and that will be the correct solution to this ticket. Also, @raynatopedrajeta please create another ticket to add the healthcheck endpoint again after v3.0 release is done.

[petermetz]

@petermetz @aldousalvarez @raynatopedrajeta

We have another breaking change when we added an authorization token to the cmd-api-server healthcheck endpoint in #2693 but this didn't get tested in DAST scan as DAST scan was failing due to another error (the besu aio not starting). When we fixed the besu aio error, the DAST scan started failing due to another PR (where we serialized ccp and sshconfig inputs for fabric connector) and while fixing that, we found the auth token missing for /healthcheck endpoint.

So 2 points here,

1. We have to include refactor(cmd-api-server): pull OAuth2 endpoint scopes from openapi.json #2693 as a breaking change for v3.0 release here: chore(release): publish v3.0.0 #3663
2. For the fix of this task, bug(dast): ccp, sshconfig missing #3671, we can remove fabric based breaking changes and remove the DAST scan check for the healthcheck endpoint and that will be the correct solution to this ticket. Also, @raynatopedrajeta please create another ticket to add the healthcheck endpoint again after v3.0 release is done.

@jagpreetsinghsasan Oh wow, nice investigation! Agreed on both points.

This is also a great reminder for everyone - myself included - how easy it is to accidentally sneak in a breaking change and not even realize it for months.