Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

Attempt exposing isDirect and types #301

Closed
wants to merge 3 commits into from
Closed

Attempt exposing isDirect and types #301

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bc5d3d3
Attempt exposing isDirect and types
naugtur Jul 9, 2021
ba28a87
lib/vuln.js to use isProjectRoot
naugtur Jul 10, 2021
daaf5c9
reuse edges, update snapshots
naugtur Jul 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion lib/vuln.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,15 @@ class Vuln {
}

toJSON () {
// sort so that they're always in a consistent order
const edgesIn = [...this.nodes.values()].flatMap(n => [...n.edgesIn])
const isDirect = edgesIn.some(e => e.from.isProjectRoot)
const types = [...new Set(edgesIn.map(a => a.type))].sort()
Comment on lines +97 to +99
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A node might be "optional", but not have any optional edgesIn. Or it might have optional edgesIn, but not actually be optional.

For example:

root -> (a, b) OPTIONAL(c)
a -> OPTIONAL(d)
b -> (d)
c -> (e)

In this dep graph,

  • a is not an optional dep, but has an optional dep on d
  • but b has a production dep on d, so d is not optional
  • c is an optional dep of the root project, and nothing else, so it's optional.
  • e is a production dep, but a production dep of an optional dep, so it is optional.

Is the intention to show all flags on any nodes in the set, or all nodes in the set? Eg, if there's one node in the tree that is optional, and another that isn't, should types include 'optional'?

Suggested change
const edgesIn = [...this.nodes.values()].flatMap(n => [...n.edgesIn])
const isDirect = edgesIn.some(e => e.from.isProjectRoot)
const types = [...new Set(edgesIn.map(a => a.type))].sort()
// this will add the flag to `types` if _any_ dep has the flag set
const allTypes = ['dev', 'devOptional', 'optional', 'peer']
const types = new Set()
// if the semantic should be _all_ deps must have it, do this instead:
// const types = new Set(allTypes)
let isDirect = false
for (const node of this.nodes.values()) {
for (const t of allTypes) {
if (node[t])
types.add(t)
// if the semantic should be _all_ deps must have the flag set, do this instead:
// if (!node[t])
// types.delete(t)
}
// it's a direct dep if the root project or any workspace depends directly on it
// since workspace projects are "ours" in the same sense as the root project
// only need to check if we haven't already found one.
if (!isDirect) {
for (const { from } of node.edgesIn) {
if (from.isProjectRoot || from.isWorkspace) {
isDirect = true
break
}
}
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually only needed dev, so I'm reluctant to include this much complexity here for figuring out types.
I'm going to split this PR so I can finish isDirect and get it merged and use it, then work on types.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extracted here: #323


return {
name: this.name,
severity: this.severity,
isDirect,
types,
// just loop over the advisories, since via is only Vuln objects,
// and calculated advisories have all the info we need
via: [...this.advisories].map(v => v.type === 'metavuln' ? v.dependency : {
Expand Down
Loading