search

guacsec / guac

GUAC aggregates software security metadata into a high fidelity graph database.
https://guac.sh
Apache License 2.0
1.27k stars 173 forks source link

[bug] toposort Loop Not Ending when Graph is Finished #1132

Closed nathannaveen closed 1 year ago

nathannaveen commented 1 year ago

Describe the bug

On line 30 of pkg/guacanalytics/toposort.go:

https://github.com/guacsec/guac/blob/bc5c042429c7fa93f993cfef3a7ad2b5f4b1059c/pkg/guacanalytics/toposort.go#L30

I think that it shouldn't be numNodes <= totalNodes, and should actually be numNodes < totalNodes. The reason for this being on line 41:

https://github.com/guacsec/guac/blob/bc5c042429c7fa93f993cfef3a7ad2b5f4b1059c/pkg/guacanalytics/toposort.go#L41-L43

We are checking for whether len(foundIDs) == 0. If numNodes == totalNodes then we have gone through all the nodes and have no more nodes to find, so len(foundIDs) will equal 0.

I am not sure whether this is intentional because the error message is fmt.Errorf("error: cycle detected"), even though in this case there will be no cycle.

nathannaveen commented 1 year ago

cc @rmetzman

rmetzman commented 1 year ago

https://github.com/guacsec/guac/pull/1129 This PR fixes the issue but it does not apply the change with the len(parents) from your other fix because the PR hasn't been merged yet.

nathannaveen commented 1 year ago

This gets fixed with https://github.com/guacsec/guac/pull/1129.