-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Container scanning #23
Conversation
@angrymeir If you will find some time take a look at this change :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As always, it's a true pleasure to browse through the code you write! :)
secscanner2junit/vulnerability.py
Outdated
return self.__identifiers[0].get_name() | ||
|
||
def get_output(self): | ||
return json.dumps(self._raw_object, indent=4) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With json.dumps
, the report (see below) is a bit difficult to read for humans (due to formatting), especially compared to the SAST report.
I'd suggest, we change the output to yaml, this way the report is easier to read for the human eye :)
import yaml
...
return yaml.dump(self._raw_object, width=1000, ident=4,sort_keys=False)
# Explanation:
# *width* needs to be set, as yaml otherwise inserts linebreaks after 80 characters which messes up the description and the urls
# *sort_keys* is True by default, which moves the description to the top and the links to the middle of the report
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are the BOSS!!! Super idea!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, then once this is changed, we can merge :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done :)
Btw: You basically solved #11 with this PR Only thing thats missing is versioning...So to decide whether we want to support multiple report versions or only the latest. And if so, a mechanism to check for breaking changes in the report formats ^^ (But thats a story for another day) |
Lastly I was reading about overenginnering and maybe we are to early in development process to define such hard to implement system? |
Fixes #22
Introduce severity in name #16
Some small steps forward #11
I've proposed new object oriented model to represent vulnerability adjust to reports version 15.0.2