From 9d7ed129508b9f2e013b5e6ae88a2f8763ec521c Mon Sep 17 00:00:00 2001
From: mmetc <92726601+mmetc@users.noreply.github.com>
Date: Fri, 10 Nov 2023 10:25:29 +0100
Subject: [PATCH] Refact cwhub (#2586)

* Inspect item: always show tainted, installed, etc. when false
* cleanup, comments, unused stuff
* download collection content after downloading dependencies, avoid duplicate call
* Return instances from Item.SubItems()
* shorter i/o code
* inline / simplify getData()
* Handle timeout connections when downloading from hub or data
---
 cmd/crowdsec-cli/hubtest.go         |  2 +-
 cmd/crowdsec-cli/item_metrics.go    | 21 ++-----
 cmd/crowdsec-cli/itemcommands.go    |  5 ++
 cmd/crowdsec-cli/items.go           |  2 +-
 cmd/crowdsec-cli/require/branch.go  | 19 +++---
 pkg/csconfig/cscli.go               |  2 -
 pkg/cwhub/cwhub.go                  |  9 +++
 pkg/cwhub/cwhub_test.go             |  6 +-
 pkg/cwhub/dataset.go                | 40 +++---------
 pkg/cwhub/enable.go                 | 33 +++-------
 pkg/cwhub/enable_test.go            | 11 ++--
 pkg/cwhub/helpers.go                | 46 +++++---------
 pkg/cwhub/hub.go                    |  8 +--
 pkg/cwhub/items.go                  | 98 +++++++++++++++++++----------
 pkg/cwhub/remote.go                 |  6 +-
 pkg/cwhub/sync.go                   | 26 ++++----
 test/bats/20_hub.bats               |  4 +-
 test/bats/20_hub_collections.bats   |  4 +-
 test/bats/20_hub_items.bats         |  2 +-
 test/bats/20_hub_parsers.bats       |  5 +-
 test/bats/20_hub_postoverflows.bats |  4 +-
 test/bats/20_hub_scenarios.bats     |  5 +-
 22 files changed, 162 insertions(+), 196 deletions(-)

diff --git a/cmd/crowdsec-cli/hubtest.go b/cmd/crowdsec-cli/hubtest.go
index 5052c1332..8b574c3ee 100644
--- a/cmd/crowdsec-cli/hubtest.go
+++ b/cmd/crowdsec-cli/hubtest.go
@@ -136,7 +136,7 @@ cscli hubtest create my-scenario-test --parsers crowdsecurity/nginx --scenarios
 			}
 
 			configFilePath := filepath.Join(testPath, "config.yaml")
-			fd, err := os.OpenFile(configFilePath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0666)
+			fd, err := os.Create(configFilePath)
 			if err != nil {
 				return fmt.Errorf("open: %s", err)
 			}
diff --git a/cmd/crowdsec-cli/item_metrics.go b/cmd/crowdsec-cli/item_metrics.go
index 4442a332e..51b652abc 100644
--- a/cmd/crowdsec-cli/item_metrics.go
+++ b/cmd/crowdsec-cli/item_metrics.go
@@ -18,8 +18,7 @@ import (
 	"github.com/crowdsecurity/crowdsec/pkg/cwhub"
 )
 
-// XXX: this should not need hub?
-func ShowMetrics(hub *cwhub.Hub, hubItem *cwhub.Item) error {
+func ShowMetrics(hubItem *cwhub.Item) error {
 	switch hubItem.Type {
 	case cwhub.PARSERS:
 		metrics := GetParserMetric(csConfig.Cscli.PrometheusUrl, hubItem.Name)
@@ -28,25 +27,13 @@ func ShowMetrics(hub *cwhub.Hub, hubItem *cwhub.Item) error {
 		metrics := GetScenarioMetric(csConfig.Cscli.PrometheusUrl, hubItem.Name)
 		scenarioMetricsTable(color.Output, hubItem.Name, metrics)
 	case cwhub.COLLECTIONS:
-		for _, parserName := range hubItem.Parsers {
-			metrics := GetParserMetric(csConfig.Cscli.PrometheusUrl, parserName)
-			parserMetricsTable(color.Output, parserName, metrics)
-		}
-		for _, scenarioName := range hubItem.Scenarios {
-			metrics := GetScenarioMetric(csConfig.Cscli.PrometheusUrl, scenarioName)
-			scenarioMetricsTable(color.Output, scenarioName, metrics)
-		}
-		for _, collName := range hubItem.Collections {
-			subColl := hub.GetItem(cwhub.COLLECTIONS, collName)
-			if subColl == nil {
-				return fmt.Errorf("unable to retrieve sub-collection '%s' from '%s'", collName, hubItem.Name)
-			}
-			if err := ShowMetrics(hub, subColl); err != nil {
+		for _, sub := range hubItem.SubItems() {
+			if err := ShowMetrics(sub); err != nil {
 				return err
 			}
 		}
 	default:
-		log.Errorf("item of type '%s' is unknown", hubItem.Type)
+		// no metrics for this item type
 	}
 	return nil
 }
diff --git a/cmd/crowdsec-cli/itemcommands.go b/cmd/crowdsec-cli/itemcommands.go
index b6beac1f4..3880afc14 100644
--- a/cmd/crowdsec-cli/itemcommands.go
+++ b/cmd/crowdsec-cli/itemcommands.go
@@ -211,6 +211,7 @@ func itemsInstallRunner(it hubItemType) func(cmd *cobra.Command, args []string)
 				}
 
 				log.Errorf(msg)
+
 				continue
 			}
 
@@ -309,6 +310,7 @@ func itemsRemoveRunner(it hubItemType) func(cmd *cobra.Command, args []string) e
 		}
 
 		removed := 0
+
 		for _, itemName := range args {
 			item := hub.GetItem(it.name, itemName)
 			if item == nil {
@@ -318,6 +320,7 @@ func itemsRemoveRunner(it hubItemType) func(cmd *cobra.Command, args []string) e
 			if !force && len(item.BelongsToCollections) > 0 {
 				log.Warningf("%s belongs to collections: %s", item.Name, item.BelongsToCollections)
 				log.Warningf("Run 'sudo cscli %s remove %s --force' if you want to force remove this %s", item.Type, item.Name, it.singular)
+
 				continue
 			}
 
@@ -390,6 +393,7 @@ func itemsUpgradeRunner(it hubItemType) func(cmd *cobra.Command, args []string)
 			}
 
 			updated := 0
+
 			for _, item := range items {
 				didUpdate, err := item.Upgrade(force)
 				if err != nil {
@@ -414,6 +418,7 @@ func itemsUpgradeRunner(it hubItemType) func(cmd *cobra.Command, args []string)
 		}
 
 		updated := 0
+
 		for _, itemName := range args {
 			item := hub.GetItem(it.name, itemName)
 			if item == nil {
diff --git a/cmd/crowdsec-cli/items.go b/cmd/crowdsec-cli/items.go
index 6d9136183..6d84582db 100644
--- a/cmd/crowdsec-cli/items.go
+++ b/cmd/crowdsec-cli/items.go
@@ -157,7 +157,7 @@ func InspectItem(hub *cwhub.Hub, item *cwhub.Item, showMetrics bool) error {
 
 	if csConfig.Cscli.Output == "human" && showMetrics {
 		fmt.Printf("\nCurrent metrics: \n")
-		if err := ShowMetrics(hub, item); err != nil {
+		if err := ShowMetrics(item); err != nil {
 			return err
 		}
 	}
diff --git a/cmd/crowdsec-cli/require/branch.go b/cmd/crowdsec-cli/require/branch.go
index 14d482796..b82f34a27 100644
--- a/cmd/crowdsec-cli/require/branch.go
+++ b/cmd/crowdsec-cli/require/branch.go
@@ -10,32 +10,33 @@ import (
 	"github.com/crowdsecurity/crowdsec/pkg/csconfig"
 )
 
-func chooseBranch(cfg *csconfig.Config, logger *log.Logger) string {
+func chooseBranch(cfg *csconfig.Config) string {
+	// this was set from config.yaml or flag
 	if cfg.Cscli.HubBranch != "" {
-		logger.Debugf("Hub override from config: branch '%s'", cfg.Cscli.HubBranch)
+		log.Debugf("Hub override from config: branch '%s'", cfg.Cscli.HubBranch)
 		return cfg.Cscli.HubBranch
 	}
 
 	latest, err := cwversion.Latest()
 	if err != nil {
-		logger.Warningf("Unable to retrieve latest crowdsec version: %s, using hub branch 'master'", err)
+		log.Warningf("Unable to retrieve latest crowdsec version: %s, using hub branch 'master'", err)
 		return "master"
 	}
 
 	csVersion := cwversion.VersionStrip()
 	if csVersion == latest {
-		logger.Debugf("Latest crowdsec version (%s), using hub branch 'master'", csVersion)
+		log.Debugf("Latest crowdsec version (%s), using hub branch 'master'", csVersion)
 		return "master"
 	}
 
 	// if current version is greater than the latest we are in pre-release
 	if semver.Compare(csVersion, latest) == 1 {
-		logger.Debugf("Your current crowdsec version seems to be a pre-release (%s), using hub branch 'master'", csVersion)
+		log.Debugf("Your current crowdsec version seems to be a pre-release (%s), using hub branch 'master'", csVersion)
 		return "master"
 	}
 
 	if csVersion == "" {
-		logger.Warning("Crowdsec version is not set, using hub branch 'master'")
+		log.Warning("Crowdsec version is not set, using hub branch 'master'")
 		return "master"
 	}
 
@@ -49,11 +50,7 @@ func chooseBranch(cfg *csconfig.Config, logger *log.Logger) string {
 // HubBranch sets the branch (in cscli config) and returns its value
 // It can be "master", or the branch corresponding to the current crowdsec version, or the value overridden in config/flag
 func HubBranch(cfg *csconfig.Config) string {
-	// XXX: we want to be able to suppress logs
-	// to avoid being too noisy in some commands
-	logger := log.StandardLogger()
-
-	branch := chooseBranch(cfg, logger)
+	branch := chooseBranch(cfg)
 
 	cfg.Cscli.HubBranch = branch
 
diff --git a/pkg/csconfig/cscli.go b/pkg/csconfig/cscli.go
index 6ecce4a53..2a3fa7df3 100644
--- a/pkg/csconfig/cscli.go
+++ b/pkg/csconfig/cscli.go
@@ -21,8 +21,6 @@ func (c *Config) loadCSCLI() error {
 		c.Cscli = &CscliCfg{}
 	}
 
-	// XXX: HubBranch default should be set here and fed to HubCfg?
-
 	if c.Prometheus.ListenAddr != "" && c.Prometheus.ListenPort != 0 {
 		c.Cscli.PrometheusUrl = fmt.Sprintf("http://%s:%d/metrics", c.Prometheus.ListenAddr, c.Prometheus.ListenPort)
 	}
diff --git a/pkg/cwhub/cwhub.go b/pkg/cwhub/cwhub.go
index 8a68c97a5..fa59d63da 100644
--- a/pkg/cwhub/cwhub.go
+++ b/pkg/cwhub/cwhub.go
@@ -3,3 +3,12 @@
 // This includes retrieving the index, the items to install (parsers, scenarios, data files...)
 // and managing the dependencies and taints.
 package cwhub
+
+import (
+	"net/http"
+	"time"
+)
+
+var hubClient = &http.Client{
+	Timeout: 10 * time.Second,
+}
diff --git a/pkg/cwhub/cwhub_test.go b/pkg/cwhub/cwhub_test.go
index 3e1bb57d1..af19b93ee 100644
--- a/pkg/cwhub/cwhub_test.go
+++ b/pkg/cwhub/cwhub_test.go
@@ -79,14 +79,14 @@ func envSetup(t *testing.T) *Hub {
 	setResponseByPath()
 	log.SetLevel(log.DebugLevel)
 
-	defaultTransport := http.DefaultClient.Transport
+	defaultTransport := hubClient.Transport
 
 	t.Cleanup(func() {
-		http.DefaultClient.Transport = defaultTransport
+		hubClient.Transport = defaultTransport
 	})
 
 	// Mock the http client
-	http.DefaultClient.Transport = newMockTransport()
+	hubClient.Transport = newMockTransport()
 
 	hub := testHub(t, true)
 
diff --git a/pkg/cwhub/dataset.go b/pkg/cwhub/dataset.go
index 2b3629a97..a0e710ebd 100644
--- a/pkg/cwhub/dataset.go
+++ b/pkg/cwhub/dataset.go
@@ -21,7 +21,7 @@ type DataSet struct {
 func downloadFile(url string, destPath string) error {
 	log.Debugf("downloading %s in %s", url, destPath)
 
-	resp, err := http.DefaultClient.Get(url)
+	resp, err := hubClient.Get(url)
 	if err != nil {
 		return fmt.Errorf("while downloading %s: %w", url, err)
 	}
@@ -31,18 +31,13 @@ func downloadFile(url string, destPath string) error {
 		return fmt.Errorf("bad http code %d for %s", resp.StatusCode, url)
 	}
 
-	body, err := io.ReadAll(resp.Body)
-	if err != nil {
-		return fmt.Errorf("while downloading %s: %w", url, err)
-	}
-
-	file, err := os.OpenFile(destPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0o644)
+	file, err := os.Create(destPath)
 	if err != nil {
 		return err
 	}
 	defer file.Close()
 
-	_, err = file.Write(body)
+	_, err = io.Copy(file, resp.Body)
 	if err != nil {
 		return err
 	}
@@ -54,19 +49,6 @@ func downloadFile(url string, destPath string) error {
 	return nil
 }
 
-func getData(data []types.DataSource, dataDir string) error {
-	for _, dataS := range data {
-		destPath := filepath.Join(dataDir, dataS.DestPath)
-		log.Infof("downloading data '%s' in '%s'", dataS.SourceURL, destPath)
-
-		if err := downloadFile(dataS.SourceURL, destPath); err != nil {
-			return err
-		}
-	}
-
-	return nil
-}
-
 // downloadData downloads the data files for an item
 func downloadData(dataFolder string, force bool, reader io.Reader) error {
 	dec := yaml.NewDecoder(reader)
@@ -82,17 +64,15 @@ func downloadData(dataFolder string, force bool, reader io.Reader) error {
 			return fmt.Errorf("while reading file: %w", err)
 		}
 
-		download := false
-
 		for _, dataS := range data.Data {
-			if _, err := os.Stat(filepath.Join(dataFolder, dataS.DestPath)); os.IsNotExist(err) {
-				download = true
-			}
-		}
+			destPath := filepath.Join(dataFolder, dataS.DestPath)
 
-		if download || force {
-			if err := getData(data.Data, dataFolder); err != nil {
-				return fmt.Errorf("while getting data: %w", err)
+			if _, err := os.Stat(destPath); os.IsNotExist(err) || force {
+				log.Infof("downloading data '%s' in '%s'", dataS.SourceURL, destPath)
+
+				if err := downloadFile(dataS.SourceURL, destPath); err != nil {
+					return fmt.Errorf("while getting data: %w", err)
+				}
 			}
 		}
 	}
diff --git a/pkg/cwhub/enable.go b/pkg/cwhub/enable.go
index e0c20628f..4886899fd 100644
--- a/pkg/cwhub/enable.go
+++ b/pkg/cwhub/enable.go
@@ -42,12 +42,7 @@ func (i *Item) enable() error {
 
 	// install sub-items if any
 	for _, sub := range i.SubItems() {
-		val, ok := i.hub.Items[sub.Type][sub.Name]
-		if !ok {
-			return fmt.Errorf("required %s %s of %s doesn't exist, abort", sub.Type, sub.Name, i.Name)
-		}
-
-		if err := val.enable(); err != nil {
+		if err := sub.enable(); err != nil {
 			return fmt.Errorf("while installing %s: %w", sub.Name, err)
 		}
 	}
@@ -102,8 +97,7 @@ func (i *Item) disable(purge bool, force bool) error {
 	// already disabled, noop unless purge
 	if !i.Installed {
 		if purge {
-			err = i.purge()
-			if err != nil {
+			if err = i.purge(); err != nil {
 				return err
 			}
 		}
@@ -121,29 +115,22 @@ func (i *Item) disable(purge bool, force bool) error {
 
 	// disable sub-items if any - it's a collection
 	for _, sub := range i.SubItems() {
-		// XXX: we do this already when syncing, do we really need to do consistency checks here and there?
-		val, ok := i.hub.Items[sub.Type][sub.Name]
-		if !ok {
-			log.Errorf("Referred %s %s in collection %s doesn't exist.", sub.Type, sub.Name, i.Name)
-			continue
-		}
-
 		// check if the item doesn't belong to another collection before removing it
-		toRemove := true
+		removeSub := true
 
-		for _, collection := range val.BelongsToCollections {
+		for _, collection := range sub.BelongsToCollections {
 			if collection != i.Name {
-				toRemove = false
+				removeSub = false
 				break
 			}
 		}
 
-		if toRemove {
-			if err = val.disable(purge, force); err != nil {
+		if removeSub {
+			if err = sub.disable(purge, force); err != nil {
 				return fmt.Errorf("while disabling %s: %w", sub.Name, err)
 			}
 		} else {
-			log.Infof("%s was not removed because it belongs to another collection", val.Name)
+			log.Infof("%s was not removed because it belongs to another collection", sub.Name)
 		}
 	}
 
@@ -180,7 +167,6 @@ func (i *Item) disable(purge bool, force bool) error {
 			return fmt.Errorf("%s isn't managed by hub", i.Name)
 		}
 
-		// remove the symlink
 		if err = os.Remove(syml); err != nil {
 			return fmt.Errorf("while removing symlink: %w", err)
 		}
@@ -191,8 +177,7 @@ func (i *Item) disable(purge bool, force bool) error {
 	i.Installed = false
 
 	if purge {
-		err = i.purge()
-		if err != nil {
+		if err = i.purge(); err != nil {
 			return err
 		}
 	}
diff --git a/pkg/cwhub/enable_test.go b/pkg/cwhub/enable_test.go
index 771edd8a7..9173024a4 100644
--- a/pkg/cwhub/enable_test.go
+++ b/pkg/cwhub/enable_test.go
@@ -32,13 +32,10 @@ func testInstall(hub *Hub, t *testing.T, item *Item) {
 func testTaint(hub *Hub, t *testing.T, item *Item) {
 	assert.False(t, hub.Items[item.Type][item.Name].Tainted, "%s should not be tainted", item.Name)
 
-	f, err := os.OpenFile(item.LocalPath, os.O_APPEND|os.O_WRONLY, 0600)
-	require.NoError(t, err, "failed to open %s (%s)", item.LocalPath, item.Name)
-
-	defer f.Close()
-
-	_, err = f.WriteString("tainted")
-	require.NoError(t, err, "failed to write to %s (%s)", item.LocalPath, item.Name)
+	// truncate the file
+	f, err := os.Create(item.LocalPath)
+	require.NoError(t, err)
+	f.Close()
 
 	// Local sync and check status
 	err = hub.localSync()
diff --git a/pkg/cwhub/helpers.go b/pkg/cwhub/helpers.go
index 774a7386a..9eeb80088 100644
--- a/pkg/cwhub/helpers.go
+++ b/pkg/cwhub/helpers.go
@@ -127,24 +127,8 @@ 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)
 
-	if !i.HasSubItems() {
-		if !i.Installed && updateOnly && i.Downloaded {
-			log.Debugf("skipping upgrade of %s: not installed", i.Name)
-			return nil
-		}
-
-		// XXX:
-		return i.download(overwrite)
-	}
-
-	// collection
 	for _, sub := range i.SubItems() {
-		val, ok := i.hub.Items[sub.Type][sub.Name]
-		if !ok {
-			return fmt.Errorf("required %s %s of %s doesn't exist, abort", sub.Type, sub.Name, i.Name)
-		}
-
-		if !val.Installed && updateOnly && val.Downloaded {
+		if !sub.Installed && updateOnly && sub.Downloaded {
 			log.Debugf("skipping upgrade of %s: not installed", i.Name)
 			continue
 		}
@@ -155,26 +139,31 @@ func (i *Item) downloadLatest(overwrite bool, updateOnly bool) error {
 		if sub.HasSubItems() {
 			log.Tracef("collection, recurse")
 
-			if err := val.downloadLatest(overwrite, updateOnly); err != nil {
-				return fmt.Errorf("while downloading %s: %w", val.Name, err)
+			if err := sub.downloadLatest(overwrite, updateOnly); err != nil {
+				return fmt.Errorf("while downloading %s: %w", sub.Name, err)
 			}
 		}
 
-		downloaded := val.Downloaded
+		downloaded := sub.Downloaded
 
-		if err := val.download(overwrite); err != nil {
-			return fmt.Errorf("while downloading %s: %w", val.Name, err)
+		if err := sub.download(overwrite); err != nil {
+			return fmt.Errorf("while downloading %s: %w", sub.Name, err)
 		}
 
 		// We need to enable an item when it has been added to a collection since latest release of the collection.
-		// We check if val.Downloaded is false because maybe the item has been disabled by the user.
-		if !val.Installed && !downloaded {
-			if err := val.enable(); err != nil {
-				return fmt.Errorf("enabling '%s': %w", val.Name, err)
+		// We check if sub.Downloaded is false because maybe the item has been disabled by the user.
+		if !sub.Installed && !downloaded {
+			if err := sub.enable(); err != nil {
+				return fmt.Errorf("enabling '%s': %w", sub.Name, err)
 			}
 		}
 	}
 
+	if !i.Installed && updateOnly && i.Downloaded {
+		log.Debugf("skipping upgrade of %s: not installed", i.Name)
+		return nil
+	}
+
 	if err := i.download(overwrite); err != nil {
 		return fmt.Errorf("failed to download item: %w", err)
 	}
@@ -203,7 +192,7 @@ func (i *Item) download(overwrite bool) error {
 		}
 	}
 
-	resp, err := http.DefaultClient.Get(url)
+	resp, err := hubClient.Get(url)
 	if err != nil {
 		return fmt.Errorf("while downloading %s: %w", url, err)
 	}
@@ -263,11 +252,10 @@ func (i *Item) download(overwrite bool) error {
 		log.Infof("%s: OK", i.Name)
 	}
 
-	f, err := os.OpenFile(tdir+"/"+i.RemotePath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o644)
+	f, err := os.Create(tdir + "/" + i.RemotePath)
 	if err != nil {
 		return fmt.Errorf("while opening file: %w", err)
 	}
-
 	defer f.Close()
 
 	_, err = f.Write(body)
diff --git a/pkg/cwhub/hub.go b/pkg/cwhub/hub.go
index 2d4d7bc85..1fc8e40ad 100644
--- a/pkg/cwhub/hub.go
+++ b/pkg/cwhub/hub.go
@@ -94,13 +94,7 @@ func (h *Hub) parseIndex() error {
 			x := strings.Split(item.RemotePath, "/")
 			item.FileName = x[len(x)-1]
 
-			// if it's a collection, check its sub-items are present
-			// XXX should be done later, maybe report all missing at once?
-			for _, sub := range item.SubItems() {
-				if _, ok := h.Items[sub.Type][sub.Name]; !ok {
-					log.Errorf("Referred %s %s in collection %s doesn't exist.", sub.Type, sub.Name, item.Name)
-				}
-			}
+			item.logMissingSubItems()
 		}
 	}
 
diff --git a/pkg/cwhub/items.go b/pkg/cwhub/items.go
index bbd2864e0..e9a1b8d4b 100644
--- a/pkg/cwhub/items.go
+++ b/pkg/cwhub/items.go
@@ -6,6 +6,7 @@ import (
 
 	"github.com/Masterminds/semver/v3"
 	"github.com/enescakir/emoji"
+	log "github.com/sirupsen/logrus"
 )
 
 const (
@@ -32,8 +33,8 @@ type HubItems map[string]map[string]*Item
 // by comparing the hash of each version to the local file.
 // If the item does not match any known version, it is considered tainted.
 type ItemVersion struct {
-	Digest     string `json:"digest,omitempty"`     // meow
-	Deprecated bool   `json:"deprecated,omitempty"` // XXX: do we keep this?
+	Digest     string `json:"digest,omitempty"` // meow
+	Deprecated bool   `json:"deprecated,omitempty"`
 }
 
 // Item represents an object managed in the hub. It can be a parser, scenario, collection..
@@ -60,10 +61,10 @@ type Item struct {
 	LocalPath    string `json:"local_path,omitempty" yaml:"local_path,omitempty"` // the local path relative to ${CFG_DIR}
 	LocalVersion string `json:"local_version,omitempty"`
 	LocalHash    string `json:"local_hash,omitempty"` // the local meow
-	Installed    bool   `json:"installed,omitempty"`  // XXX: should we remove omitempty from bool fields?
-	Downloaded   bool   `json:"downloaded,omitempty"`
-	UpToDate     bool   `json:"up_to_date,omitempty"`
-	Tainted      bool   `json:"tainted,omitempty"` // has it been locally modified?
+	Installed    bool   `json:"installed"`
+	Downloaded   bool   `json:"downloaded"`
+	UpToDate     bool   `json:"up_to_date"`
+	Tainted      bool   `json:"tainted"` // has it been locally modified?
 
 	// if it's a collection, it can have sub items
 	Parsers       []string `json:"parsers,omitempty"       yaml:"parsers,omitempty"`
@@ -72,19 +73,10 @@ type Item struct {
 	Collections   []string `json:"collections,omitempty"   yaml:"collections,omitempty"`
 }
 
-type SubItem struct {
-	Type string
-	Name string
-}
-
 func (i *Item) HasSubItems() bool {
 	return i.Type == COLLECTIONS
 }
 
-func (i *SubItem) HasSubItems() bool {
-	return i.Type == COLLECTIONS
-}
-
 func (i *Item) IsLocal() bool {
 	return i.Installed && !i.Downloaded
 }
@@ -97,7 +89,7 @@ func (i Item) MarshalJSON() ([]byte, error) {
 
 	return json.Marshal(&struct {
 		Alias
-		Local bool `json:"local"` // XXX: omitempty?
+		Local bool `json:"local"`
 	}{
 		Alias: Alias(i),
 		Local: i.IsLocal(),
@@ -119,39 +111,79 @@ func (i Item) MarshalYAML() (interface{}, error) {
 	}, nil
 }
 
-// SubItems returns the list of sub items for a given item (typically a collection)
-func (i *Item) SubItems() []SubItem {
-	sub := make([]SubItem,
-		len(i.Parsers)+
-			len(i.PostOverflows)+
-			len(i.Scenarios)+
-			len(i.Collections))
-
-	n := 0
+// SubItems returns a slice of sub-item pointers, excluding the ones that were not found
+func (i *Item) SubItems() []*Item {
+	sub := make([]*Item, 0)
 
 	for _, name := range i.Parsers {
-		sub[n] = SubItem{Type: PARSERS, Name: name}
-		n++
+		s := i.hub.GetItem(PARSERS, name)
+		if s == nil {
+			continue
+		}
+
+		sub = append(sub, s)
 	}
 
 	for _, name := range i.PostOverflows {
-		sub[n] = SubItem{Type: POSTOVERFLOWS, Name: name}
-		n++
+		s := i.hub.GetItem(POSTOVERFLOWS, name)
+		if s == nil {
+			continue
+		}
+
+		sub = append(sub, s)
 	}
 
 	for _, name := range i.Scenarios {
-		sub[n] = SubItem{Type: SCENARIOS, Name: name}
-		n++
+		s := i.hub.GetItem(SCENARIOS, name)
+		if s == nil {
+			continue
+		}
+
+		sub = append(sub, s)
 	}
 
 	for _, name := range i.Collections {
-		sub[n] = SubItem{Type: COLLECTIONS, Name: name}
-		n++
+		s := i.hub.GetItem(COLLECTIONS, name)
+		if s == nil {
+			continue
+		}
+
+		sub = append(sub, s)
 	}
 
 	return sub
 }
 
+func (i *Item) logMissingSubItems() {
+	if !i.HasSubItems() {
+		return
+	}
+
+	for _, subName := range i.Parsers {
+		if i.hub.GetItem(PARSERS, subName) == nil {
+			log.Errorf("can't find %s in %s, required by %s", subName, PARSERS, i.Name)
+		}
+	}
+
+	for _, subName := range i.Scenarios {
+		if i.hub.GetItem(SCENARIOS, subName) == nil {
+			log.Errorf("can't find %s in %s, required by %s", subName, SCENARIOS, i.Name)
+		}
+	}
+
+	for _, subName := range i.PostOverflows {
+		if i.hub.GetItem(POSTOVERFLOWS, subName) == nil {
+			log.Errorf("can't find %s in %s, required by %s", subName, POSTOVERFLOWS, i.Name)
+		}
+	}
+
+	for _, subName := range i.Collections {
+		if i.hub.GetItem(COLLECTIONS, subName) == nil {
+			log.Errorf("can't find %s in %s, required by %s", subName, COLLECTIONS, i.Name)
+		}
+	}
+}
+
 // Status returns the status of the item as a string and an emoji
 // ie. "enabled,update-available" and emoji.Warning
 func (i *Item) Status() (string, emoji.Emoji) {
diff --git a/pkg/cwhub/remote.go b/pkg/cwhub/remote.go
index 9e0c2c04d..ad97a4efb 100644
--- a/pkg/cwhub/remote.go
+++ b/pkg/cwhub/remote.go
@@ -22,6 +22,7 @@ func (r *RemoteHubCfg) urlTo(remotePath string) (string, error) {
 		return "", ErrNilRemoteHub
 	}
 
+	// the template must contain two string placeholders
 	if fmt.Sprintf(r.URLTemplate, "%s", "%s") != r.URLTemplate {
 		return "", fmt.Errorf("invalid URL template '%s'", r.URLTemplate)
 	}
@@ -40,7 +41,7 @@ func (r *RemoteHubCfg) downloadIndex(localPath string) error {
 		return fmt.Errorf("failed to build hub index request: %w", err)
 	}
 
-	resp, err := http.DefaultClient.Get(url)
+	resp, err := hubClient.Get(url)
 	if err != nil {
 		return fmt.Errorf("failed http request for hub index: %w", err)
 	}
@@ -69,8 +70,7 @@ func (r *RemoteHubCfg) downloadIndex(localPath string) error {
 		return nil
 	}
 
-	file, err := os.OpenFile(localPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o644)
-
+	file, err := os.Create(localPath)
 	if err != nil {
 		return fmt.Errorf("while opening hub index file: %w", err)
 	}
diff --git a/pkg/cwhub/sync.go b/pkg/cwhub/sync.go
index e9855027f..fe47342ca 100644
--- a/pkg/cwhub/sync.go
+++ b/pkg/cwhub/sync.go
@@ -129,6 +129,7 @@ func (h *Hub) getItemInfo(path string) (itemFileInfo, bool, error) {
 // sortedVersions returns the input data, sorted in reverse order by semver
 func sortedVersions(raw []string) ([]string, error) {
 	vs := make([]*semver.Version, len(raw))
+
 	for idx, r := range raw {
 		v, err := semver.NewVersion(r)
 		if err != nil {
@@ -180,7 +181,7 @@ func (h *Hub) itemVisit(path string, f os.DirEntry, err error) error {
 	}
 
 	/*
-		we can encounter 'collections' in the form of a symlink :
+		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
 	*/
@@ -350,42 +351,37 @@ func (h *Hub) checkSubItems(v *Item) error {
 	log.Tracef("checking submembers of %s installed:%t", v.Name, v.Installed)
 
 	for _, sub := range v.SubItems() {
-		subItem, ok := h.Items[sub.Type][sub.Name]
-		if !ok {
-			return fmt.Errorf("referred %s %s in collection %s doesn't exist", sub.Type, sub.Name, v.Name)
-		}
-
-		log.Tracef("check %s installed:%t", subItem.Name, subItem.Installed)
+		log.Tracef("check %s installed:%t", sub.Name, sub.Installed)
 
 		if !v.Installed {
 			continue
 		}
 
-		if err := h.checkSubItems(subItem); err != nil {
-			if subItem.Tainted {
+		if err := h.checkSubItems(sub); err != nil {
+			if sub.Tainted {
 				v.Tainted = true
 			}
 
-			return fmt.Errorf("sub collection %s is broken: %w", subItem.Name, err)
+			return fmt.Errorf("sub collection %s is broken: %w", sub.Name, err)
 		}
 
-		if subItem.Tainted {
+		if sub.Tainted {
 			v.Tainted = true
 			return fmt.Errorf("tainted %s %s, tainted", sub.Type, sub.Name)
 		}
 
-		if !subItem.Installed && v.Installed {
+		if !sub.Installed && v.Installed {
 			v.Tainted = true
 			return fmt.Errorf("missing %s %s, tainted", sub.Type, sub.Name)
 		}
 
-		if !subItem.UpToDate {
+		if !sub.UpToDate {
 			v.UpToDate = false
 			return fmt.Errorf("outdated %s %s", sub.Type, sub.Name)
 		}
 
-		if !slices.Contains(subItem.BelongsToCollections, v.Name) {
-			subItem.BelongsToCollections = append(subItem.BelongsToCollections, v.Name)
+		if !slices.Contains(sub.BelongsToCollections, v.Name) {
+			sub.BelongsToCollections = append(sub.BelongsToCollections, v.Name)
 		}
 
 		log.Tracef("checking for %s - tainted:%t uptodate:%t", sub.Name, v.Tainted, v.UpToDate)
diff --git a/test/bats/20_hub.bats b/test/bats/20_hub.bats
index f6c1c87cf..0a45b0839 100644
--- a/test/bats/20_hub.bats
+++ b/test/bats/20_hub.bats
@@ -66,8 +66,8 @@ teardown() {
     new_hub=$(jq <"$HUB_DIR/.index.json" 'del(.parsers."crowdsecurity/smb-logs") | del (.scenarios."crowdsecurity/mysql-bf")')
     echo "$new_hub" >"$HUB_DIR/.index.json"
     rune -0 cscli hub list --error
-    assert_stderr --partial "Referred parsers crowdsecurity/smb-logs in collection crowdsecurity/smb doesn't exist."
-    assert_stderr --partial "Referred scenarios crowdsecurity/mysql-bf in collection crowdsecurity/mysql doesn't exist."
+    assert_stderr --partial "can't find crowdsecurity/smb-logs in parsers, required by crowdsecurity/smb"
+    assert_stderr --partial "can't find crowdsecurity/mysql-bf in scenarios, required by crowdsecurity/mysql"
 }
 
 @test "cscli hub update" {
diff --git a/test/bats/20_hub_collections.bats b/test/bats/20_hub_collections.bats
index 8452fc666..b527e3905 100644
--- a/test/bats/20_hub_collections.bats
+++ b/test/bats/20_hub_collections.bats
@@ -185,7 +185,7 @@ teardown() {
     rune -0 cscli collections inspect crowdsecurity/sshd -o json
     rune -0 jq -c '[.type, .name, .author, .path, .installed]' <(output)
     # XXX: .installed is missing -- not false
-    assert_json '["collections","crowdsecurity/sshd","crowdsecurity","collections/crowdsecurity/sshd.yaml",null]'
+    assert_json '["collections","crowdsecurity/sshd","crowdsecurity","collections/crowdsecurity/sshd.yaml",false]'
 
     # one item, raw
     rune -0 cscli collections inspect crowdsecurity/sshd -o raw
@@ -211,7 +211,7 @@ teardown() {
     # multiple items, json
     rune -0 cscli collections inspect crowdsecurity/sshd crowdsecurity/smb -o json
     rune -0 jq -sc '[.[] | [.type, .name, .author, .path, .installed]]' <(output)
-    assert_json '[["collections","crowdsecurity/sshd","crowdsecurity","collections/crowdsecurity/sshd.yaml",null],["collections","crowdsecurity/smb","crowdsecurity","collections/crowdsecurity/smb.yaml",null]]'
+    assert_json '[["collections","crowdsecurity/sshd","crowdsecurity","collections/crowdsecurity/sshd.yaml",false],["collections","crowdsecurity/smb","crowdsecurity","collections/crowdsecurity/smb.yaml",false]]'
 
     # multiple items, raw
     rune -0 cscli collections inspect crowdsecurity/sshd crowdsecurity/smb -o raw
diff --git a/test/bats/20_hub_items.bats b/test/bats/20_hub_items.bats
index 6ac19bf90..80e0934c6 100644
--- a/test/bats/20_hub_items.bats
+++ b/test/bats/20_hub_items.bats
@@ -53,7 +53,7 @@ teardown() {
     rune -0 cscli collections inspect crowdsecurity/sshd -o json
     # XXX: is this supposed to be tainted or up to date?
     rune -0 jq -c '[.local_version,.up_to_date,.tainted]' <(output)
-    assert_json '["1.10",null,null]'
+    assert_json '["1.10",false,false]'
 }
 
 @test "hub index with invalid (non semver) version numbers" {
diff --git a/test/bats/20_hub_parsers.bats b/test/bats/20_hub_parsers.bats
index 05088cde9..544f03aef 100644
--- a/test/bats/20_hub_parsers.bats
+++ b/test/bats/20_hub_parsers.bats
@@ -187,8 +187,7 @@ teardown() {
     # one item, json
     rune -0 cscli parsers inspect crowdsecurity/sshd-logs -o json
     rune -0 jq -c '[.type, .stage, .name, .author, .path, .installed]' <(output)
-    # XXX: .installed is missing -- not false
-    assert_json '["parsers","s01-parse","crowdsecurity/sshd-logs","crowdsecurity","parsers/s01-parse/crowdsecurity/sshd-logs.yaml",null]'
+    assert_json '["parsers","s01-parse","crowdsecurity/sshd-logs","crowdsecurity","parsers/s01-parse/crowdsecurity/sshd-logs.yaml",false]'
 
     # one item, raw
     rune -0 cscli parsers inspect crowdsecurity/sshd-logs -o raw
@@ -215,7 +214,7 @@ teardown() {
     # multiple items, json
     rune -0 cscli parsers inspect crowdsecurity/sshd-logs crowdsecurity/whitelists -o json
     rune -0 jq -sc '[.[] | [.type, .stage, .name, .author, .path, .installed]]' <(output)
-    assert_json '[["parsers","s01-parse","crowdsecurity/sshd-logs","crowdsecurity","parsers/s01-parse/crowdsecurity/sshd-logs.yaml",null],["parsers","s02-enrich","crowdsecurity/whitelists","crowdsecurity","parsers/s02-enrich/crowdsecurity/whitelists.yaml",null]]'
+    assert_json '[["parsers","s01-parse","crowdsecurity/sshd-logs","crowdsecurity","parsers/s01-parse/crowdsecurity/sshd-logs.yaml",false],["parsers","s02-enrich","crowdsecurity/whitelists","crowdsecurity","parsers/s02-enrich/crowdsecurity/whitelists.yaml",false]]'
 
     # multiple items, raw
     rune -0 cscli parsers inspect crowdsecurity/sshd-logs crowdsecurity/whitelists -o raw
diff --git a/test/bats/20_hub_postoverflows.bats b/test/bats/20_hub_postoverflows.bats
index 7e3714de9..9f766a2ad 100644
--- a/test/bats/20_hub_postoverflows.bats
+++ b/test/bats/20_hub_postoverflows.bats
@@ -189,7 +189,7 @@ teardown() {
     rune -0 cscli postoverflows inspect crowdsecurity/rdns -o json
     rune -0 jq -c '[.type, .stage, .name, .author, .path, .installed]' <(output)
     # XXX: .installed is missing -- not false
-    assert_json '["postoverflows","s00-enrich","crowdsecurity/rdns","crowdsecurity","postoverflows/s00-enrich/crowdsecurity/rdns.yaml",null]'
+    assert_json '["postoverflows","s00-enrich","crowdsecurity/rdns","crowdsecurity","postoverflows/s00-enrich/crowdsecurity/rdns.yaml",false]'
 
     # one item, raw
     rune -0 cscli postoverflows inspect crowdsecurity/rdns -o raw
@@ -216,7 +216,7 @@ teardown() {
     # multiple items, json
     rune -0 cscli postoverflows inspect crowdsecurity/rdns crowdsecurity/cdn-whitelist -o json
     rune -0 jq -sc '[.[] | [.type, .stage, .name, .author, .path, .installed]]' <(output)
-    assert_json '[["postoverflows","s00-enrich","crowdsecurity/rdns","crowdsecurity","postoverflows/s00-enrich/crowdsecurity/rdns.yaml",null],["postoverflows","s01-whitelist","crowdsecurity/cdn-whitelist","crowdsecurity","postoverflows/s01-whitelist/crowdsecurity/cdn-whitelist.yaml",null]]'
+    assert_json '[["postoverflows","s00-enrich","crowdsecurity/rdns","crowdsecurity","postoverflows/s00-enrich/crowdsecurity/rdns.yaml",false],["postoverflows","s01-whitelist","crowdsecurity/cdn-whitelist","crowdsecurity","postoverflows/s01-whitelist/crowdsecurity/cdn-whitelist.yaml",false]]'
 
     # multiple items, raw
     rune -0 cscli postoverflows inspect crowdsecurity/rdns crowdsecurity/cdn-whitelist -o raw
diff --git a/test/bats/20_hub_scenarios.bats b/test/bats/20_hub_scenarios.bats
index eb3193823..7cbbb0104 100644
--- a/test/bats/20_hub_scenarios.bats
+++ b/test/bats/20_hub_scenarios.bats
@@ -187,8 +187,7 @@ teardown() {
     # one item, json
     rune -0 cscli scenarios inspect crowdsecurity/ssh-bf -o json
     rune -0 jq -c '[.type, .name, .author, .path, .installed]' <(output)
-    # XXX: .installed is missing -- not false
-    assert_json '["scenarios","crowdsecurity/ssh-bf","crowdsecurity","scenarios/crowdsecurity/ssh-bf.yaml",null]'
+    assert_json '["scenarios","crowdsecurity/ssh-bf","crowdsecurity","scenarios/crowdsecurity/ssh-bf.yaml",false]'
 
     # one item, raw
     rune -0 cscli scenarios inspect crowdsecurity/ssh-bf -o raw
@@ -214,7 +213,7 @@ teardown() {
     # multiple items, json
     rune -0 cscli scenarios inspect crowdsecurity/ssh-bf crowdsecurity/telnet-bf -o json
     rune -0 jq -sc '[.[] | [.type, .name, .author, .path, .installed]]' <(output)
-    assert_json '[["scenarios","crowdsecurity/ssh-bf","crowdsecurity","scenarios/crowdsecurity/ssh-bf.yaml",null],["scenarios","crowdsecurity/telnet-bf","crowdsecurity","scenarios/crowdsecurity/telnet-bf.yaml",null]]'
+    assert_json '[["scenarios","crowdsecurity/ssh-bf","crowdsecurity","scenarios/crowdsecurity/ssh-bf.yaml",false],["scenarios","crowdsecurity/telnet-bf","crowdsecurity","scenarios/crowdsecurity/telnet-bf.yaml",false]]'
 
     # multiple items, raw
     rune -0 cscli scenarios inspect crowdsecurity/ssh-bf crowdsecurity/telnet-bf -o raw