Only forcibly generate diagram files for report if missing - use custom diagrams in report

Threagile / threagile

Agile Threat Modeling Toolkit

https://threagile.io

MIT License

577 stars 126 forks source link

Only forcibly generate diagram files for report if missing - use custom diagrams in report #71

Closed softScheck closed 2 weeks ago

softScheck commented 2 months ago

It is not possible to use a self-created diagram for the report because the creation of the diagrams is enforced, even when --generate-data-flow-diagram=0 or --generate-data-asset-diagram=0 is provided. This pull request adds a check to see if the files already exists and allows for skipping the creation of the diagrams in that case.

Therefore, a custom diagram in the build folder can be used and is build into the report.

ezavgorodniy commented 3 weeks ago

For reportPDF variables generateDataFlowDiagram and generateDataAssetsDiagram are always true and proposed changes will never be executed: https://github.com/Threagile/threagile/blame/master/pkg/report/generate.go#L75-L81

As far as I understand you're looking for adding some flag to not generate data flow/data assets diagrams, aren't you? I'd perhaps consider the implementation in this way: https://github.com/Threagile/threagile/commit/7dbcd594e4ee4b979ace6c7bb32d9b902b724eb0 leveraging configuration file. With this implementation you'll need to point to your config.json and run the program with flag --config config.json

softScheck commented 3 weeks ago

Hey,

In my pull request, I modified the generate.go file you pointed out. Specifically, I updated the section where the variables were previously set to true, regardless of the --generate-data-flow-diagram=0 and --generate-data-asset-diagram=0 flags.

Now, the code includes a check to see if the necessary files for building the report already exist. If they do and the variables are set, the generation is skipped.

This approach is more transparent than simply ignoring the --generate-data-flow-diagram=0 and --generate-data-asset-diagram=0 flags without any notification. Additionally, it allows you to place your diagrams in the build folder and have them automatically added to the report.

I think your solution using the config file should be additional to this pull request, as a config will be used to replace setting command line arguments when building the report.

ezavgorodniy commented 2 weeks ago

Sorry, I missread the PR, my fault

Could you please rebase your branch against master branch to contain latest code changes because right now the static checks are failing? Also could you please run go fmt against your changes to make the code more complaining with go code standards (like exclamation mark shall not have space between exclamation mark and variable)? Or even better to move this added bit of code to the function to avoid too much nesting

softScheck commented 2 weeks ago

Rebase is done and format is corrected.

ezavgorodniy commented 2 weeks ago

regarding errors:

unit test failure, you can just re run the code and it will be fine
compilation failures - after reducing amount of violation of dependency of violation principle now report package depend on interface and not on struct https://github.com/Threagile/threagile/blob/master/CONTRIBUTING.md#packages-structure

softScheck commented 2 weeks ago

okay, changed to the now used get methods.

ezavgorodniy commented 2 weeks ago

lgtm thanks for contribution