guard against the possibility of no links being provided #69
Conversation
alfredodeza
force-pushed
the
undefined-links
branch
3 times, most recently
from
08160ba
to
27bdc86
Compare
index.js
Outdated
| @@ -221,6 +221,12 @@ function grype_render_rules(vulnerabilities) { | |||
| let ruleID = "ANCHOREVULN_"+v.vulnerability.id+"_"+v.artifact.type+"_"+v.artifact.name+"_"+v.artifact.version; | |||
| if (!ruleIDs.includes(ruleID)) { | |||
| ruleIDs.push(ruleID); | |||
| // Entirely possible to not have any links whatsoever | |||
| var link = v.vulnerability.id; | |||
There was a problem hiding this comment.
Not blocking, but in general var should never be used, and it's safe to use let in its place
There was a problem hiding this comment.
I slipped on this one and have updated it to fix it. I ran tests locally against the reproducer, and it passed
| @@ -221,6 +221,12 @@ function grype_render_rules(vulnerabilities) { | |||
| let ruleID = "ANCHOREVULN_"+v.vulnerability.id+"_"+v.artifact.type+"_"+v.artifact.name+"_"+v.artifact.version; | |||
| if (!ruleIDs.includes(ruleID)) { | |||
| ruleIDs.push(ruleID); | |||
| // Entirely possible to not have any links whatsoever | |||
| var link = v.vulnerability.id; | |||
| if ("links" in v.vulnerability) { | |||
There was a problem hiding this comment.
I haven't seen this style of check before, but that doesn't mean it's wrong. I usually see checks for property's existence in the form of if (typeof v.vulnerability.links !== "undefined") ...
| "markdown": "**Vulnerability "+v.vulnerability.id+"**\n"+ | ||
| "| Severity | Package | Version | Fix Version | Type | Location | Data Namespace | Link |\n"+ | ||
| "| --- | --- | --- | --- | --- | --- | --- | --- |\n"+ | ||
| "|"+v.vulnerability.severity+"|"+v.artifact.name+"|"+v.artifact.version+"|"+"unknown"+"|"+v.artifact.type+"|"+v.artifact.locations[0].path+"|"+"unknown"+"|["+v.vulnerability.id+"]("+v.vulnerability.links[0]+")|\n" | ||
| "|"+v.vulnerability.severity+"|"+v.artifact.name+"|"+v.artifact.version+"|"+"unknown"+"|"+v.artifact.type+"|"+v.artifact.locations[0].path+"|"+"unknown"+"|"+link+"|\n" |
There was a problem hiding this comment.
This whole line might read easier as a template, and with values separated out. For example:
const { severity } = v.vulnerability;
const { name, version, type, locations } = v.artifact;
const { path } = locations[0];
// ...
`|${severity}|${name}|${version}|unknown|${type}|${path}|unknown|${link}|\n`There was a problem hiding this comment.
I agree. I've started using templates as I carve out changes. This whole file needs to use more templates. Do note that I have captured that feedback and created #51 to track it.
Signed-off-by: Alfredo Deza <[email protected]>
alfredodeza
force-pushed
the
undefined-links
branch
from
27bdc86
to
c08e9df
Compare
Closes #68
/cc @jonico