FEAT: Enhanced HTML view #25

kevinwright · 2018-09-28T19:47:55Z

Added packageManager tags
All sections can now be filtered
improved de-duplicated view of paths using graphviz

See ticket SC-6251

- Added packageManager tags - All sections can now be filtered - improved de-duplicated view of paths using graphviz See ticket SC-6251

…ilure on node 4

… logic

gjvis

Loads of inline questions, as I don't have a huge amount of context!

Also: the graph is interesting, but is quite a deviation from what was there before (and this is in use already at customers). Perhaps it should be opt-in, or there should be a toggle for graph or text-list visualisation of the vulnerable paths. This is particularly important for PDFs generated from the output as the graph doesn't wrap.

gjvis · 2018-10-01T13:32:30Z

src/lib/handlebars-directory.ts

@@ -0,0 +1,78 @@
+
+// originally forked from https://github.com/reykjavikingur/node-handlebars-directory
+// under the terms of the ISC licence:


was something changed in the copied code? Perhaps better to fork to the snyk organisation and make the change there (and contribute it back?)

This is because it failed under node 4. I raised a ticket, but the author refused to consider Node 4 support as valuable

Were their actual changes needed? If we're going to use this library I think its better to fork the repo (npm can pull in a dep directly from a github form)

gjvis · 2018-10-01T13:39:58Z

src/lib/snyk-to-html.ts

+import {loadSource} from './loader';
+
+import Viz = require('viz.js');
+import { Module, render } from 'viz.js/full.render.js';


inconsistent brace spacing (linting should've picked it up - perhaps the rules on the repo are out of date)

same again on line 14 below

gjvis · 2018-10-01T14:35:25Z

src/lib/hbars.ts

@@ -0,0 +1,55 @@
+import Handlebars = require('handlebars');
+import {HandlebarsDirectory} from './handlebars-directory';


what's the reason for introducing this (copied-and-pasted) dependency over the previous implementation?

See above, see also build failures in travis

gjvis · 2018-10-01T14:39:04Z

template/test-report.hbs

+          {{> checkbox id="proof-of-concept" label="Proof of Concept"}}
+          {{> checkbox id="remediation" label="Remediation"}}
+          {{> checkbox id="disclosure-timeline" label="Disclosure Timeline"}}
+          {{> checkbox id="references" label="References"}}


Is this an exhaustive list of possible sections?

Similarly is the data subject to change? Should it be dynamic? (perhaps not sensibly possible with the data we to hand)

Very possible, and definitely something I considered. Main issue is that it would almost certainly need to be client side and generated out of the DOM - given how much of this comes out of markdown. Non-trivial enough that I had to consider time constraints

gjvis · 2018-10-01T14:39:33Z

template/test-report.hbs

@@ -47,6 +54,21 @@

  <div class="layout-stacked__content">
    <div class="layout-container--short" style="padding-top: 35px;">   
+        <div class="filter-box">
+          <b>filter</b>


Capital F?

gjvis · 2018-10-01T14:56:10Z

template/test-report.hbs

+          <hr/>
+          {{> checkbox id="severity--high" label="High Severity"}}
+          {{> checkbox id="severity--medium" label="Medium Severity"}}
+          {{> checkbox id="severity--low" label="Low Severity"}}


I believe "critical" and "info" severities are being introduced, similar to the previous comment – perhaps this should this be dynamic?

Ultimately we'd want priorities to be listed (and ordered) in the underlying JSON, or they still need to be hardcoded SOMEWHERE. I already raised that for discussion elsewhere

gjvis · 2018-10-01T15:23:31Z

template/test-report.vuln-card.hbs

-            {{/each}}
-        </ul><!-- .list-paths -->
-
-    </div><!-- .card__section -->


Why have we removed all of this remediation advice?

gjvis · 2018-10-01T15:32:53Z

template/test-report.hbs

@@ -47,6 +54,21 @@

  <div class="layout-stacked__content">
    <div class="layout-container--short" style="padding-top: 35px;">   
+        <div class="filter-box">
+          <b>filter</b>
+          <hr/>


Hmm, also - this top list isn't really a list of filters, but rather a list of sections to display. Perhaps they should be labelled independently to the severity filters below.

The original thinking was filtering of the output as a whole, not precious about terminology though. I'm open to alternativr terms

gjvis · 2018-10-01T15:33:31Z

template/test-report.hbs

@@ -47,6 +54,21 @@

  <div class="layout-stacked__content">
    <div class="layout-container--short" style="padding-top: 35px;">   
+        <div class="filter-box">


This should probably be excluded from printed output, I believe snyk-to-html is used to generate PDFs a fair amount

Interesting... differing output for print is a new req though, and would demand a new param. Mixed feelings about doing that in the same PR

lili2311

After getting more info on the requirements these are issues we are trying to address:

Users would like to see less info in this report
It would be nice to filter license vs vuln

How users use this currently:

users are likely generating this as PDF
users are also using this in their build/test pipeline to generate test artefacts

Based on this we should make changes as following:
Filters:

in order to not break users pdfs remove or exclude from print the Filters
add ability to filter license vs issue

Smaller summary

keep the current standard vuln card view, please either remove the graph or make it a toggle. Again this should not amend the PDF output to what users expect today
Remediations are per path so we can't just show 1, each remediation should be shown under the path:

Tags - the package manager tags are great but please use the current style and add them on on the left of the card like so:

lili2311 · 2018-10-01T16:19:01Z

src/lib/snyk-to-html.ts

    id: vuln.id,
    title: vuln.title,
    name: vuln.name,
    info: vuln.info || 'No information available.',
    severity: vuln.severity,
    severityValue: severityMap[vuln.severity],
    description: vuln.description || 'No description available.',
+    isLicense: (vuln.type === 'license'),


why do we have isLicense? type is already either license or vulnerability

lili2311 · 2018-10-01T16:19:53Z

src/lib/snyk-to-html.ts

+    .then(svg => {
+      r.metadata.graph = svg;
+    });
+    // r.metadata.graph = mkGraph(r.list);


please remove is this is unused

lili2311 · 2018-10-01T16:21:27Z

template/inline-css.hbs

@@ -275,6 +299,79 @@
    padding: 0.5em;
  }

+.filter-box {


Assume these is just copy pasted from snyk-ui? maybe let's reference where it came from with a comment so it is easier to just override in the future

adrukh · 2019-07-29T11:20:27Z

Closing as stale

FEAT: Enhanced HTML view

0c4ec9c

- Added packageManager tags - All sections can now be filtered - improved de-duplicated view of paths using graphviz See ticket SC-6251

kevinwright force-pushed the feature/report-summary-and-filtering branch from f00ce20 to 507ed5a Compare September 28, 2018 19:49

chore: removed a couple of unused deps

9758511

kevinwright force-pushed the feature/report-summary-and-filtering branch from 507ed5a to 9758511 Compare September 28, 2018 19:51

kevinwright requested review from gjvis and adrukh September 28, 2018 19:56

chore: copied in source of handlebars-directory to avoid a runtime fa…

1789bdd

…ilure on node 4

kevinwright force-pushed the feature/report-summary-and-filtering branch from 6b222bd to 1789bdd Compare September 28, 2018 21:17

kevinwright added 4 commits September 28, 2018 22:33

chore: typeface in path diagrams now inherited from surrounding document

30c2028

chore: added a soupçon of documentation to the markdown restructuring…

6a16a1b

… logic

chore: sexy css for group and version display in the from graph

717a242

feat: added severity filtering

7026b99

gjvis reviewed Oct 1, 2018

View reviewed changes

lili2311 requested changes Oct 1, 2018

View reviewed changes

lili2311 reviewed Oct 1, 2018

View reviewed changes

adrukh closed this Jul 29, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FEAT: Enhanced HTML view #25

FEAT: Enhanced HTML view #25

kevinwright commented Sep 28, 2018

gjvis left a comment •

edited

Loading

gjvis Oct 1, 2018

kevinwright Oct 1, 2018

gjvis Oct 2, 2018

gjvis Oct 1, 2018

gjvis Oct 1, 2018

gjvis Oct 1, 2018

kevinwright Oct 1, 2018

gjvis Oct 1, 2018

kevinwright Oct 1, 2018

gjvis Oct 1, 2018

gjvis Oct 1, 2018

kevinwright Oct 1, 2018 •

edited

Loading

gjvis Oct 1, 2018

gjvis Oct 1, 2018

kevinwright Oct 1, 2018

gjvis Oct 1, 2018

kevinwright Oct 1, 2018

lili2311 left a comment

lili2311 Oct 1, 2018

lili2311 Oct 1, 2018

lili2311 Oct 1, 2018

adrukh commented Jul 29, 2019

		@@ -0,0 +1,78 @@

		// originally forked from https://github.com/reykjavikingur/node-handlebars-directory
		// under the terms of the ISC licence:

		@@ -0,0 +1,55 @@
		import Handlebars = require('handlebars');
		import {HandlebarsDirectory} from './handlebars-directory';

FEAT: Enhanced HTML view #25

FEAT: Enhanced HTML view #25

Conversation

kevinwright commented Sep 28, 2018

gjvis left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevinwright Oct 1, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lili2311 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adrukh commented Jul 29, 2019

gjvis left a comment •

edited

Loading

kevinwright Oct 1, 2018 •

edited

Loading