Make alert messages in issue more helpful #43
Conversation
Signed-off-by: Daniel Bartholomae <[email protected]>
Signed-off-by: Daniel Bartholomae <[email protected]>
| msg = `${msg} ${TAB} **New Alerts** ${NXT_LINE}`; | ||
| site.alerts.forEach((alert) => { | ||
| msg = msg + TAB + `${BULLET} **${alert.name}** [${alert.pluginid}] total: ${alert.instances.length}: ${NXT_LINE}` | ||
| msg = msg + TAB + `${BULLET} **${alert.riskdesc}: ${alert.name}** [[${alert.pluginid}]](https://www.zaproxy.org/docs/alerts/${alert.pluginid}/) total: ${alert.instances.length}: ${NXT_LINE}` |
There was a problem hiding this comment.
It's preferable to use riskcode and confidence and probably better if it's explicit what's what. (That requires mapping the IDs to English though.)
There was a problem hiding this comment.
@thc202 happy to do that - where can I find documentation on these values and what they map to?
There was a problem hiding this comment.
(For context: I'm currently working on moving the repo to TypeScript, add tests, and do some other clean up, and would like to either merge the PR very soon, e.g. if there are only few changes that I can do right away, or close this PR and create a new one in the new structure)
There was a problem hiding this comment.
@thc202 happy to do that - where can I find documentation on these values and what they map to?
There was a problem hiding this comment.
Thanks, I think that's exactly what I need! :)
I'll close this PR, open the refactoring PR, and will add a new feature PR afterwards.
|
Will be redone after refactoring the code behind it[ |
3 participants
This PR makes alert messages in issues more helpful by adding the risk description and a link to the alert description to it.
Why?
The issue created is currently not that helpful, as it has a full list of alerts but does not allow triaging
Alternatives?
I also thought about grouping alerts by risk level instead of just adding the risk description to each. This would be a better user experience from my view point, as we could sort by riskcode and have the highest priority risks on top, but since it's a bigger change I didn't want to do it without first getting some feedback from the maintainers. If this makes sense, I'm happy to either adapt this PR or create an additional one.
What else?
Please note that this PR would affect all actions using it, as this package is used by all of them. As a next steps, the actions would need to be updated to rely on the new version of this library.
Also, I'm unsure how to best test this. It looks like a small change, and
alert.riskdescis part of the JSON example, but someone who has more experience with ZAP should check (or tell me how to check) that this is correct.I didn't find any information on how to best contribute and couldn't open an issue. Since the change is small, I directly created this PR. I'm not mad if it can't get merged and there is a different way to contribute, but would appreciate pointers.