Refact cwhub: simplify tree scan and dependency checks (#2600)

* method rename: GetInstalledItemsAsString() -> GetInstalledItemNames()
* use path package
* Comments and method names
* Extract method Item.setVersionState() from Hub.itemVisit()
* refact localSync(), itemVisit() etc.
* fix check for cyclic dependencies, with test

cmd/crowdsec-cli/capi.go

@@ -156,7 +156,7 @@ func NewCapiStatusCmd() *cobra.Command {
 				return err
 			}
 
-			scenarios, err := hub.GetInstalledItemsAsString(cwhub.SCENARIOS)
+			scenarios, err := hub.GetInstalledItemNames(cwhub.SCENARIOS)
 			if err != nil {
 				return fmt.Errorf("failed to get scenarios: %w", err)
 			}

cmd/crowdsec-cli/console.go

@@ -76,7 +76,7 @@ After running this command your will need to validate the enrollment in the weba
 				return err
 			}
 
-			scenarios, err := hub.GetInstalledItemsAsString(cwhub.SCENARIOS)
+			scenarios, err := hub.GetInstalledItemNames(cwhub.SCENARIOS)
 			if err != nil {
 				return fmt.Errorf("failed to get installed scenarios: %s", err)
 			}

cmd/crowdsec-cli/item_suggest.go

@@ -61,7 +61,7 @@ func compInstalledItems(itemType string, args []string, toComplete string) ([]st
 		return nil, cobra.ShellCompDirectiveDefault
 	}
 
-	items, err := hub.GetInstalledItemsAsString(itemType)
+	items, err := hub.GetInstalledItemNames(itemType)
 	if err != nil {
 		cobra.CompDebugln(fmt.Sprintf("list installed %s err: %s", itemType, err), true)
 		return nil, cobra.ShellCompDirectiveDefault

cmd/crowdsec-cli/lapi.go

@@ -43,7 +43,7 @@ func runLapiStatus(cmd *cobra.Command, args []string) error {
 		log.Fatal(err)
 	}
 
-	scenarios, err := hub.GetInstalledItemsAsString(cwhub.SCENARIOS)
+	scenarios, err := hub.GetInstalledItemNames(cwhub.SCENARIOS)
 	if err != nil {
 		log.Fatalf("failed to get scenarios : %s", err)
 	}

cmd/crowdsec-cli/support.go

@@ -174,7 +174,7 @@ func collectAPIStatus(login string, password string, endpoint string, prefix str
 	if err != nil {
 		return []byte(fmt.Sprintf("cannot parse API URL: %s", err))
 	}
-	scenarios, err := hub.GetInstalledItemsAsString(cwhub.SCENARIOS)
+	scenarios, err := hub.GetInstalledItemNames(cwhub.SCENARIOS)
 	if err != nil {
 		return []byte(fmt.Sprintf("could not collect scenarios: %s", err))
 	}

cmd/crowdsec/output.go

@@ -71,7 +71,7 @@ func runOutput(input chan types.Event, overflow chan types.Event, buckets *leaky
 	var cache []types.RuntimeAlert
 	var cacheMutex sync.Mutex
 
-	scenarios, err := hub.GetInstalledItemsAsString(cwhub.SCENARIOS)
+	scenarios, err := hub.GetInstalledItemNames(cwhub.SCENARIOS)
 	if err != nil {
 		return fmt.Errorf("loading list of installed hub scenarios: %w", err)
 	}
@@ -94,7 +94,7 @@ func runOutput(input chan types.Event, overflow chan types.Event, buckets *leaky
 		URL:            apiURL,
 		PapiURL:        papiURL,
 		VersionPrefix:  "v1",
-		UpdateScenario: func() ([]string, error) {return hub.GetInstalledItemsAsString(cwhub.SCENARIOS)},
+		UpdateScenario: func() ([]string, error) {return hub.GetInstalledItemNames(cwhub.SCENARIOS)},
 	})
 	if err != nil {
 		return fmt.Errorf("new client api: %w", err)

pkg/cwhub/dataset.go

@@ -18,6 +18,7 @@ type DataSet struct {
 	Data []types.DataSource `yaml:"data,omitempty"`
 }
 
+// downloadFile downloads a file and writes it to disk, with no hash verification
 func downloadFile(url string, destPath string) error {
 	log.Debugf("downloading %s in %s", url, destPath)
 
@@ -37,6 +38,7 @@ func downloadFile(url string, destPath string) error {
 	}
 	defer file.Close()
 
+	// avoid reading the whole file in memory
 	_, err = io.Copy(file, resp.Body)
 	if err != nil {
 		return err
@@ -49,8 +51,8 @@ func downloadFile(url string, destPath string) error {
 	return nil
 }
 
-// downloadData downloads the data files for an item
+// downloadDataSet downloads all the data files for an item
-func downloadData(dataFolder string, force bool, reader io.Reader) error {
+func downloadDataSet(dataFolder string, force bool, reader io.Reader) error {
 	dec := yaml.NewDecoder(reader)
 
 	for {

pkg/cwhub/enable.go

@@ -10,14 +10,15 @@ import (
 	log "github.com/sirupsen/logrus"
 )
 
-// installLink returns the location of the symlink to the actual config file (eg. /etc/crowdsec/collections/xyz.yaml)
+// installLink returns the location of the symlink to the downloaded config file
-func (i *Item) installLink() string {
+// (eg. /etc/crowdsec/collections/xyz.yaml)
+func (i *Item) installLinkPath() string {
 	return filepath.Join(i.hub.local.InstallDir, i.Type, i.Stage, i.FileName)
 }
 
 // makeLink creates a symlink between the actual config file at hub.HubDir and hub.ConfigDir
 func (i *Item) createInstallLink() error {
-	dest, err := filepath.Abs(i.installLink())
+	dest, err := filepath.Abs(i.installLinkPath())
 	if err != nil {
 		return err
 	}
@@ -102,8 +103,9 @@ func (i *Item) purge() error {
 	return nil
 }
 
+// removeInstallLink removes the symlink to the downloaded content
 func (i *Item) removeInstallLink() error {
-	syml, err := filepath.Abs(i.installLink())
+	syml, err := filepath.Abs(i.installLinkPath())
 	if err != nil {
 		return err
 	}
@@ -143,13 +145,13 @@ func (i *Item) removeInstallLink() error {
 	return nil
 }
 
-// disable removes the symlink to the downloaded content, also removes the content if purge is true
+// disable removes the install link, and optionally the downloaded content
 func (i *Item) disable(purge bool, force bool) error {
 	// XXX: should return the number of disabled/purged items to inform the upper layer whether to reload or not
 	err := i.removeInstallLink()
 	if os.IsNotExist(err) {
 		if !purge && !force {
-			return fmt.Errorf("link %s does not exist (override with --force or --purge)", i.installLink())
+			return fmt.Errorf("link %s does not exist (override with --force or --purge)", i.installLinkPath())
 		}
 	} else if err != nil {
 		return err

pkg/cwhub/helpers.go

@@ -52,20 +52,46 @@ func (i *Item) Install(force bool, downloadOnly bool) error {
 	return nil
 }
 
-// allDependencies return a list of all dependencies and sub-dependencies of the item
+// allDependencies returns a list of all (direct or indirect) dependencies of the item
-func (i *Item) allDependencies() []*Item {
+func (i *Item) allDependencies() ([]*Item, error) {
-	var deps []*Item
+	var collectSubItems func(item *Item, visited map[*Item]bool, result *[]*Item) error
 
-	for _, dep := range i.SubItems() {
+	collectSubItems = func(item *Item, visited map[*Item]bool, result *[]*Item) error {
-		if dep == i {
+		if item == nil {
-			log.Errorf("circular dependency detected: %s depends on %s", dep.Name, i.Name)
+			return nil
-			continue
 		}
 
-		deps = append(deps, dep.allDependencies()...)
+		if visited[item] {
+			return nil
 		}
 
-	return append(deps, i)
+		visited[item] = true
+
+		for _, subItem := range item.SubItems() {
+			if subItem == i {
+				return fmt.Errorf("circular dependency detected: %s depends on %s", item.Name, i.Name)
+			}
+
+			*result = append(*result, subItem)
+
+			err := collectSubItems(subItem, visited, result)
+			if err != nil {
+				return err
+			}
+		}
+
+		return nil
+	}
+
+	ret := []*Item{}
+	visited := map[*Item]bool{}
+
+	err := collectSubItems(i, visited, &ret)
+	if err != nil {
+		return nil, err
+	}
+
+	return ret, nil
 }
 
 // Remove disables the item, optionally removing the downloaded content
@@ -85,15 +111,18 @@ func (i *Item) Remove(purge bool, force bool) (bool, error) {
 
 	removed := false
 
-	allDeps := i.allDependencies()
+	allDeps, err := i.allDependencies()
+	if err != nil {
+		return false, err
+	}
 
 	for _, sub := range i.SubItems() {
 		if !sub.Installed {
 			continue
 		}
 
-		// if the other collection(s) are direct or indirect dependencies of the current one, it's good to go
+		// if the sub depends on a collection that is not a direct or indirect dependency
-		// log parent collections
+		// of the current item, it is not removed
 		for _, subParent := range sub.parentCollections() {
 			if subParent == i {
 				continue
@@ -113,8 +142,7 @@ func (i *Item) Remove(purge bool, force bool) (bool, error) {
 		removed = removed || subRemoved
 	}
 
-	err := i.disable(purge, force)
+	if err = i.disable(purge, force); err != nil {
-	if err != nil {
 		return false, fmt.Errorf("while removing %s: %w", i.Name, err)
 	}
 
@@ -171,7 +199,7 @@ func (i *Item) Upgrade(force bool) (bool, error) {
 	return updated, nil
 }
 
-// downloadLatest will download the latest version of Item to the tdir directory
+// downloadLatest downloads the latest version of the item to the hub directory
 func (i *Item) downloadLatest(overwrite bool, updateOnly bool) error {
 	// XXX: should return the path of the downloaded file (taken from download())
 	log.Debugf("Downloading %s %s", i.Type, i.Name)
@@ -314,7 +342,7 @@ func (i *Item) download(overwrite bool) error {
 	i.Tainted = false
 	i.UpToDate = true
 
-	if err = downloadData(i.hub.local.InstallDataDir, overwrite, bytes.NewReader(body)); err != nil {
+	if err = downloadDataSet(i.hub.local.InstallDataDir, overwrite, bytes.NewReader(body)); err != nil {
 		return fmt.Errorf("while downloading data for %s: %w", i.FileName, err)
 	}
 
@@ -332,7 +360,7 @@ func (i *Item) DownloadDataIfNeeded(force bool) error {
 
 	defer itemFile.Close()
 
-	if err = downloadData(i.hub.local.InstallDataDir, force, itemFile); err != nil {
+	if err = downloadDataSet(i.hub.local.InstallDataDir, force, itemFile); err != nil {
 		return fmt.Errorf("while downloading data for %s: %w", itemFilePath, err)
 	}
 

pkg/cwhub/hub.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"os"
+	"path"
 	"strings"
 
 	log "github.com/sirupsen/logrus"
@@ -15,8 +16,6 @@ type Hub struct {
 	Items    HubItems
 	local    *csconfig.LocalHubCfg
 	remote   *RemoteHubCfg
-	skippedLocal   int
-	skippedTainted int
 	Warnings []string
 }
 
@@ -82,8 +81,7 @@ func (h *Hub) parseIndex() error {
 			}
 
 			item.Type = itemType
-			x := strings.Split(item.RemotePath, "/")
+			item.FileName = path.Base(item.RemotePath)
-			item.FileName = x[len(x)-1]
 
 			item.logMissingSubItems()
 		}
@@ -95,6 +93,8 @@ func (h *Hub) parseIndex() error {
 // ItemStats returns total counts of the hub items
 func (h *Hub) ItemStats() []string {
 	loaded := ""
+	local := 0
+	tainted := 0
 
 	for _, itemType := range ItemTypes {
 		if len(h.Items[itemType]) == 0 {
@@ -102,11 +102,20 @@ func (h *Hub) ItemStats() []string {
 		}
 
 		loaded += fmt.Sprintf("%d %s, ", len(h.Items[itemType]), itemType)
+
+		for _, item := range h.Items[itemType] {
+			if item.IsLocal() {
+				local++
+			}
+
+			if item.Tainted {
+				tainted++
+			}
+		}
 	}
 
 	loaded = strings.Trim(loaded, ", ")
 	if loaded == "" {
-		// empty hub
 		loaded = "0 items"
 	}
 
@@ -114,8 +123,8 @@ func (h *Hub) ItemStats() []string {
 		fmt.Sprintf("Loaded: %s", loaded),
 	}
 
-	if h.skippedLocal > 0 || h.skippedTainted > 0 {
+	if local > 0 || tainted > 0 {
-		ret = append(ret, fmt.Sprintf("Unmanaged items: %d local, %d tainted", h.skippedLocal, h.skippedTainted))
+		ret = append(ret, fmt.Sprintf("Unmanaged items: %d local, %d tainted", local, tainted))
 	}
 
 	return ret

pkg/cwhub/items.go

@@ -186,6 +186,7 @@ func (i *Item) logMissingSubItems() {
 	}
 }
 
+// parentCollections returns the list of items (collections) that have this item as a direct dependency
 func (i *Item) parentCollections() []*Item {
 	ret := make([]*Item, 0)
 
@@ -281,8 +282,8 @@ func (h *Hub) GetItem(itemType string, itemName string) *Item {
 	return h.GetItemMap(itemType)[itemName]
 }
 
-// GetItemNames returns the list of item (full) names for a given type
+// GetItemNames returns the list of (full) item names for a given type
-// ie. for parsers: crowdsecurity/apache2 crowdsecurity/nginx
+// ie. for collections: crowdsecurity/apache2 crowdsecurity/nginx
 // The names can be used to retrieve the item with GetItem()
 func (h *Hub) GetItemNames(itemType string) []string {
 	m := h.GetItemMap(itemType)
@@ -335,8 +336,8 @@ func (h *Hub) GetInstalledItems(itemType string) ([]*Item, error) {
 	return retItems, nil
 }
 
-// GetInstalledItemsAsString returns the names of the installed items
+// GetInstalledItemNames returns the names of the installed items
-func (h *Hub) GetInstalledItemsAsString(itemType string) ([]string, error) {
+func (h *Hub) GetInstalledItemNames(itemType string) ([]string, error) {
 	items, err := h.GetInstalledItems(itemType)
 	if err != nil {
 		return nil, err

pkg/cwhub/items_test.go

@@ -36,7 +36,10 @@ func TestItemStatus(t *testing.T) {
 	}
 
 	stats := hub.ItemStats()
-	require.Equal(t, []string{"Loaded: 2 parsers, 1 scenarios, 3 collections"}, stats)
+	require.Equal(t, []string{
+		"Loaded: 2 parsers, 1 scenarios, 3 collections",
+		"Unmanaged items: 3 local, 0 tainted",
+	}, stats)
 }
 
 func TestGetters(t *testing.T) {

pkg/cwhub/remote.go

@@ -17,6 +17,7 @@ type RemoteHubCfg struct {
 	IndexPath   string
 }
 
+// urlTo builds the URL to download a file from the remote hub
 func (r *RemoteHubCfg) urlTo(remotePath string) (string, error) {
 	if r == nil {
 		return "", ErrNilRemoteHub

pkg/cwhub/sync.go

@@ -19,21 +19,18 @@ func isYAMLFileName(path string) bool {
 	return strings.HasSuffix(path, ".yaml") || strings.HasSuffix(path, ".yml")
 }
 
-func handleSymlink(path string) (string, error) {
+// linkTarget returns the target of a symlink, or empty string if it's dangling
+func linkTarget(path string) (string, error) {
 	hubpath, err := os.Readlink(path)
 	if err != nil {
-		return "", fmt.Errorf("unable to read symlink of %s", path)
+		return "", fmt.Errorf("unable to read symlink: %s", path)
-	}
-	// the symlink target doesn't exist, user might have removed ~/.hub/hub/...yaml without deleting /etc/crowdsec/....yaml
-	_, err = os.Lstat(hubpath)
-	if os.IsNotExist(err) {
-		log.Infof("%s is a symlink to %s that doesn't exist, deleting symlink", path, hubpath)
-		// remove the symlink
-		if err = os.Remove(path); err != nil {
-			return "", fmt.Errorf("failed to unlink %s: %w", path, err)
 	}
 
-		// ignore this file
+	log.Tracef("symlink %s -> %s", path, hubpath)
+
+	_, err = os.Lstat(hubpath)
+	if os.IsNotExist(err) {
+		log.Infof("link target does not exist: %s -> %s", path, hubpath)
 		return "", nil
 	}
 
@@ -57,15 +54,15 @@ func getSHA256(filepath string) (string, error) {
 }
 
 type itemFileInfo struct {
+	inhub   bool
 	fname   string
 	stage   string
 	ftype   string
 	fauthor string
 }
 
-func (h *Hub) getItemInfo(path string) (itemFileInfo, bool, error) {
+func (h *Hub) getItemFileInfo(path string) (*itemFileInfo, error) {
-	ret := itemFileInfo{}
+	var ret *itemFileInfo
-	inhub := false
 
 	hubDir := h.local.HubDir
 	installDir := h.local.InstallDir
@@ -78,37 +75,41 @@ func (h *Hub) getItemInfo(path string) (itemFileInfo, bool, error) {
 	if strings.HasPrefix(path, hubDir) {
 		log.Tracef("in hub dir")
 
-		inhub = true
 		//.../hub/parsers/s00-raw/crowdsec/skip-pretag.yaml
 		//.../hub/scenarios/crowdsec/ssh_bf.yaml
 		//.../hub/profiles/crowdsec/linux.yaml
 		if len(subs) < 4 {
-			return itemFileInfo{}, false, fmt.Errorf("path is too short: %s (%d)", path, len(subs))
+			return nil, fmt.Errorf("path is too short: %s (%d)", path, len(subs))
 		}
 
-		ret.fname = subs[len(subs)-1]
+		ret = &itemFileInfo{
-		ret.fauthor = subs[len(subs)-2]
+			inhub:   true,
-		ret.stage = subs[len(subs)-3]
+			fname:   subs[len(subs)-1],
-		ret.ftype = subs[len(subs)-4]
+			fauthor: subs[len(subs)-2],
+			stage:   subs[len(subs)-3],
+			ftype:   subs[len(subs)-4],
+		}
 	} else if strings.HasPrefix(path, installDir) { // we're in install /etc/crowdsec/<type>/...
 		log.Tracef("in install dir")
 		if len(subs) < 3 {
-			return itemFileInfo{}, false, fmt.Errorf("path is too short: %s (%d)", path, len(subs))
+			return nil, fmt.Errorf("path is too short: %s (%d)", path, len(subs))
 		}
 		///.../config/parser/stage/file.yaml
 		///.../config/postoverflow/stage/file.yaml
 		///.../config/scenarios/scenar.yaml
 		///.../config/collections/linux.yaml //file is empty
-		ret.fname = subs[len(subs)-1]
+		ret = &itemFileInfo{
-		ret.stage = subs[len(subs)-2]
+			inhub:   false,
-		ret.ftype = subs[len(subs)-3]
+			fname:   subs[len(subs)-1],
-		ret.fauthor = ""
+			stage:   subs[len(subs)-2],
+			ftype:   subs[len(subs)-3],
+			fauthor: "",
+		}
 	} else {
-		return itemFileInfo{}, false, fmt.Errorf("file '%s' is not from hub '%s' nor from the configuration directory '%s'", path, hubDir, installDir)
+		return nil, fmt.Errorf("file '%s' is not from hub '%s' nor from the configuration directory '%s'", path, hubDir, installDir)
 	}
 
 	log.Tracef("stage:%s ftype:%s", ret.stage, ret.ftype)
-	// log.Infof("%s -> name:%s stage:%s", path, fname, stage)
 
 	if ret.stage == SCENARIOS {
 		ret.ftype = SCENARIOS
@@ -118,15 +119,15 @@ func (h *Hub) getItemInfo(path string) (itemFileInfo, bool, error) {
 		ret.stage = ""
 	} else if ret.ftype != PARSERS && ret.ftype != POSTOVERFLOWS {
 		// it's a PARSER / POSTOVERFLOW with a stage
-		return itemFileInfo{}, inhub, fmt.Errorf("unknown configuration type for file '%s'", path)
+		return nil, fmt.Errorf("unknown configuration type for file '%s'", path)
 	}
 
 	log.Tracef("CORRECTED [%s] by [%s] in stage [%s] of type [%s]", ret.fname, ret.fauthor, ret.stage, ret.ftype)
 
-	return ret, inhub, nil
+	return ret, nil
 }
 
-// sortedVersions returns the input data, sorted in reverse order by semver
+// sortedVersions returns the input data, sorted in reverse order (new, old) by semver
 func sortedVersions(raw []string) ([]string, error) {
 	vs := make([]*semver.Version, len(raw))
 
@@ -149,66 +150,10 @@ func sortedVersions(raw []string) ([]string, error) {
 	return ret, nil
 }
 
-func (h *Hub) itemVisit(path string, f os.DirEntry, err error) error {
+func newLocalItem(h *Hub, path string, info *itemFileInfo) *Item {
-	local := false
-	hubpath := ""
-
-	if err != nil {
-		log.Debugf("while syncing hub dir: %s", err)
-		// there is a path error, we ignore the file
-		return nil
-	}
-
-	path, err = filepath.Abs(path)
-	if err != nil {
-		return err
-	}
-
-	// we only care about files
-	if f == nil || f.IsDir() {
-		return nil
-	}
-
-	if !isYAMLFileName(f.Name()) {
-		return nil
-	}
-
-	info, inhub, err := h.getItemInfo(path)
-	if err != nil {
-		return err
-	}
-
-	/*
-		we can encounter 'collections' in the form of a symlink:
-		/etc/crowdsec/.../collections/linux.yaml -> ~/.hub/hub/collections/.../linux.yaml
-		when the collection is installed, both files are created
-	*/
-	// non symlinks are local user files or hub files
-	if f.Type()&os.ModeSymlink == 0 {
-		local = true
-
-		log.Tracef("%s isn't a symlink", path)
-	} else {
-		hubpath, err = handleSymlink(path)
-		if err != nil {
-			return err
-		}
-		log.Tracef("%s points to %s", path, hubpath)
-
-		if hubpath == "" {
-			// ignore this file
-			return nil
-		}
-	}
-
-	// if it's not a symlink and not in hub, it's a local file, don't bother
-	if local && !inhub {
-		log.Tracef("%s is a local file, skip", path)
-		h.skippedLocal++
-
 	_, fileName := filepath.Split(path)
 
-		h.Items[info.ftype][info.fname] = &Item{
+	return &Item{
 		hub:       h,
 		Name:      info.fname,
 		Stage:     info.stage,
@@ -218,30 +163,70 @@ func (h *Hub) itemVisit(path string, f os.DirEntry, err error) error {
 		UpToDate:  true,
 		FileName:  fileName,
 	}
+}
+
+func (h *Hub) itemVisit(path string, f os.DirEntry, err error) error {
+	hubpath := ""
+
+	if err != nil {
+		log.Debugf("while syncing hub dir: %s", err)
+		// there is a path error, we ignore the file
+		return nil
+	}
+
+	// only happens if the current working directory was removed (!)
+	path, err = filepath.Abs(path)
+	if err != nil {
+		return err
+	}
+
+	// we only care about YAML files
+	if f == nil || f.IsDir() || !isYAMLFileName(f.Name()) {
+		return nil
+	}
+
+	info, err := h.getItemFileInfo(path)
+	if err != nil {
+		return err
+	}
+
+	// non symlinks are local user files or hub files
+	if f.Type()&os.ModeSymlink == 0 {
+		log.Tracef("%s is not a symlink", path)
+
+		if !info.inhub {
+			log.Tracef("%s is a local file, skip", path)
+			h.Items[info.ftype][info.fname] = newLocalItem(h, path, info)
 
 			return nil
 		}
+	} else {
+		hubpath, err = linkTarget(path)
+		if err != nil {
+			return err
+		}
+
+		if hubpath == "" {
+			// target does not exist, the user might have removed the file
+			// or switched to a hub branch without it
+			return nil
+		}
+	}
 
 	// try to find which configuration item it is
 	log.Tracef("check [%s] of %s", info.fname, info.ftype)
 
-	match := false
-
 	for name, item := range h.Items[info.ftype] {
-		log.Tracef("check [%s] vs [%s]: %s", info.fname, item.RemotePath, info.ftype+"/"+info.stage+"/"+info.fname+".yaml")
-
 		if info.fname != item.FileName {
-			log.Tracef("%s != %s (filename)", info.fname, item.FileName)
 			continue
 		}
 
-		// wrong stage
 		if item.Stage != info.stage {
 			continue
 		}
 
 		// if we are walking hub dir, just mark present files as downloaded
-		if inhub {
+		if info.inhub {
 			// wrong author
 			if info.fauthor != item.Author {
 				continue
@@ -262,66 +247,9 @@ func (h *Hub) itemVisit(path string, f os.DirEntry, err error) error {
 			continue
 		}
 
-		sha, err := getSHA256(path)
+		err := item.setVersionState(path, info.inhub)
 		if err != nil {
-			log.Fatalf("Failed to get sha of %s: %v", path, err)
+			return err
-		}
-
-		// let's reverse sort the versions to deal with hash collisions (#154)
-		versions := make([]string, 0, len(item.Versions))
-		for k := range item.Versions {
-			versions = append(versions, k)
-		}
-
-		versions, err = sortedVersions(versions)
-		if err != nil {
-			return fmt.Errorf("while syncing %s %s: %w", info.ftype, info.fname, err)
-		}
-
-		for _, version := range versions {
-			if item.Versions[version].Digest != sha {
-				continue
-			}
-
-			// we got an exact match, update struct
-
-			item.Downloaded = true
-			item.LocalHash = sha
-
-			if !inhub {
-				log.Tracef("found exact match for %s, version is %s, latest is %s", item.Name, version, item.Version)
-				item.LocalPath = path
-				item.LocalVersion = version
-				item.Tainted = false
-				// if we're walking the hub, present file doesn't means installed file
-				item.Installed = true
-			}
-
-			if version == item.Version {
-				log.Tracef("%s is up-to-date", item.Name)
-				item.UpToDate = true
-			}
-
-			match = true
-
-			break
-		}
-
-		if !match {
-			log.Tracef("got tainted match for %s: %s", item.Name, path)
-
-			h.skippedTainted++
-
-			// the file and the stage is right, but the hash is wrong, it has been tainted by user
-			if !inhub {
-				item.LocalPath = path
-				item.Installed = true
-			}
-
-			item.UpToDate = false
-			item.LocalVersion = "?"
-			item.Tainted = true
-			item.LocalHash = sha
 		}
 
 		h.Items[info.ftype][name] = item
@@ -365,11 +293,13 @@ func (h *Hub) checkSubItems(v *Item) error {
 
 		if sub.Tainted {
 			v.Tainted = true
+			// XXX: improve msg
 			return fmt.Errorf("tainted %s %s, tainted", sub.Type, sub.Name)
 		}
 
 		if !sub.Installed && v.Installed {
 			v.Tainted = true
+			// XXX: improve msg
 			return fmt.Errorf("missing %s %s, tainted", sub.Type, sub.Name)
 		}
 
@@ -388,9 +318,8 @@ func (h *Hub) checkSubItems(v *Item) error {
 	return nil
 }
 
-func (h *Hub) syncDir(dir string) ([]string, error) {
+// syncDir scans a directory for items, and updates the Hub state accordingly
-	warnings := []string{}
+func (h *Hub) syncDir(dir string) error {
-
 	// For each, scan PARSERS, POSTOVERFLOWS, SCENARIOS and COLLECTIONS last
 	for _, scan := range ItemTypes {
 		// cpath: top-level item directory, either downloaded or installed items.
@@ -408,11 +337,31 @@ func (h *Hub) syncDir(dir string) ([]string, error) {
 		}
 
 		if err = filepath.WalkDir(cpath, h.itemVisit); err != nil {
-			return warnings, err
+			return err
 		}
 	}
 
+	return nil
+}
+
+// localSync updates the hub state with downloaded, installed and local items
+func (h *Hub) localSync() error {
+	err := h.syncDir(h.local.InstallDir)
+	if err != nil {
+		return fmt.Errorf("failed to scan %s: %w", h.local.InstallDir, err)
+	}
+
+	if err = h.syncDir(h.local.HubDir); err != nil {
+		return fmt.Errorf("failed to scan %s: %w", h.local.HubDir, err)
+	}
+
+	warnings := make([]string, 0)
+
 	for _, item := range h.Items[COLLECTIONS] {
+		if _, err := item.allDependencies(); err != nil {
+			return err
+		}
+
 		if !item.Installed {
 			continue
 		}
@@ -434,27 +383,69 @@ func (h *Hub) syncDir(dir string) ([]string, error) {
 		log.Debugf("installed (%s) - status: %d | installed: %s | latest: %s | full: %+v", item.Name, vs, item.LocalVersion, item.Version, item.Versions)
 	}
 
-	return warnings, nil
+	h.Warnings = warnings
-}
+
-
+	return nil
-// Updates the info from HubInit() with the local state
+}
-func (h *Hub) localSync() error {
+
-	h.skippedLocal = 0
+func (i *Item) setVersionState(path string, inhub bool) error {
-	h.skippedTainted = 0
+	var err error
-	h.Warnings = []string{}
+
-
+	i.LocalHash, err = getSHA256(path)
-	warnings, err := h.syncDir(h.local.InstallDir)
+	if err != nil {
-	if err != nil {
+		return fmt.Errorf("failed to get sha256 of %s: %w", path, err)
-		return fmt.Errorf("failed to scan %s: %w", h.local.InstallDir, err)
+	}
-	}
+
-
+	// let's reverse sort the versions to deal with hash collisions (#154)
-	h.Warnings = append(h.Warnings, warnings...)
+	versions := make([]string, 0, len(i.Versions))
-
+	for k := range i.Versions {
-	if warnings, err = h.syncDir(h.local.HubDir); err != nil {
+		versions = append(versions, k)
-		return fmt.Errorf("failed to scan %s: %w", h.local.HubDir, err)
+	}
-	}
+
-
+	versions, err = sortedVersions(versions)
-	h.Warnings = append(h.Warnings, warnings...)
+	if err != nil {
+		return fmt.Errorf("while syncing %s %s: %w", i.Type, i.FileName, err)
+	}
+
+	i.LocalVersion = "?"
+
+	for _, version := range versions {
+		if i.Versions[version].Digest == i.LocalHash {
+			i.LocalVersion = version
+			break
+		}
+	}
+
+	if i.LocalVersion == "?" {
+		log.Tracef("got tainted match for %s: %s", i.Name, path)
+
+		if !inhub {
+			i.LocalPath = path
+			i.Installed = true
+		}
+
+		i.UpToDate = false
+		i.Tainted = true
+
+		return nil
+	}
+
+	// we got an exact match, update struct
+
+	i.Downloaded = true
+
+	if !inhub {
+		log.Tracef("found exact match for %s, version is %s, latest is %s", i.Name, i.LocalVersion, i.Version)
+		i.LocalPath = path
+		i.Tainted = false
+		// if we're walking the hub, present file doesn't means installed file
+		i.Installed = true
+	}
+
+	if i.LocalVersion == i.Version {
+		log.Tracef("%s is up-to-date", i.Name)
+		i.UpToDate = true
+	}
 
 	return nil
 }

test/bats/20_hub_collections_dep.bats

@@ -112,3 +112,13 @@ teardown() {
     rune -0 cscli parsers list -o json
     rune -0 jq -e '.parsers | length == 0' <(output)
 }
+
+@test "cscli collections (dependencies IV: looper)" {
+    hub_dep=$(jq <"$INDEX_PATH" '. * {collections:{"crowdsecurity/sshd":{collections:["crowdsecurity/linux"]}}}')
+    echo "$hub_dep" >"$INDEX_PATH"
+
+    rune -1 cscli hub list
+    assert_stderr --partial "circular dependency detected"
+    rune -1 wait-for "${CROWDSEC}"
+    assert_stderr --partial "circular dependency detected"
+}