Refact pkg/cwhub: fix some known issues and reorganize files (#2616)

* bump gopkg.in/yaml.v3
* test: cannot remove local items with cscli
* test dangling links
* test: cannot install local item with cscli
* pkg/cwhub: reorg (move) functions in files
* allow hub upgrade with local items
* data download: honor Last-Modified header
* fatal -> warning when attempting to remove a local item (allows remove --all)
* cscli...inspect -o yaml|human: rename remote_path -> path
* Correct count of removed items
Still no separate counter for the --purge option, but should be clear enough

cmd/crowdsec-cli/item_suggest.go

@@ -12,10 +12,10 @@ import (
 	"github.com/crowdsecurity/crowdsec/pkg/cwhub"
 )
 
-const MaxDistance = 7
+// suggestNearestMessage returns a message with the most similar item name, if one is found
+func suggestNearestMessage(hub *cwhub.Hub, itemType string, itemName string) string {
+	const maxDistance = 7
 
-// SuggestNearestMessage returns a message with the most similar item name, if one is found
-func SuggestNearestMessage(hub *cwhub.Hub, itemType string, itemName string) string {
 	score := 100
 	nearest := ""
 
@@ -29,7 +29,7 @@ func SuggestNearestMessage(hub *cwhub.Hub, itemType string, itemName string) str
 
 	msg := fmt.Sprintf("can't find '%s' in %s", itemName, itemType)
 
-	if score < MaxDistance {
+	if score < maxDistance {
 		msg += fmt.Sprintf(", did you mean '%s'?", nearest)
 	}
 

cmd/crowdsec-cli/itemcommands.go

@@ -214,7 +214,7 @@ func itemsInstallRunner(it hubItemType) func(cmd *cobra.Command, args []string)
 		for _, name := range args {
 			item := hub.GetItem(it.name, name)
 			if item == nil {
-				msg := SuggestNearestMessage(hub, it.name, name)
+				msg := suggestNearestMessage(hub, it.name, name)
 				if !ignoreError {
 					return fmt.Errorf(msg)
 				}
@@ -319,6 +319,7 @@ func itemsRemoveRunner(it hubItemType) func(cmd *cobra.Command, args []string) e
 					return err
 				}
 				if didRemove {
+					log.Infof("Removed %s", item.Name)
 					removed++
 				}
 			}
@@ -361,6 +362,8 @@ func itemsRemoveRunner(it hubItemType) func(cmd *cobra.Command, args []string) e
 				removed++
 			}
 		}
+
+		log.Infof("Removed %d %s", removed, it.name)
 		if removed > 0 {
 			log.Infof(ReloadMessage())
 		}

cmd/crowdsec-cli/utils.go

@@ -13,14 +13,12 @@ import (
 )
 
 func printHelp(cmd *cobra.Command) {
-	err := cmd.Help()
-	if err != nil {
+	if err := cmd.Help(); err != nil {
 		log.Fatalf("unable to print help(): %s", err)
 	}
 }
 
 func manageCliDecisionAlerts(ip *string, ipRange *string, scope *string, value *string) error {
-
 	/*if a range is provided, change the scope*/
 	if *ipRange != "" {
 		_, _, err := net.ParseCIDR(*ipRange)
@@ -50,7 +48,6 @@ func manageCliDecisionAlerts(ip *string, ipRange *string, scope *string, value *
 }
 
 func getDBClient() (*database.Client, error) {
-	var err error
 	if err := csConfig.LoadAPIServer(); err != nil || csConfig.DisableAPI {
 		return nil, err
 	}

pkg/cwhub/cwhub.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"net/http"
 	"path/filepath"
+	"sort"
 	"strings"
 	"time"
 )
@@ -30,3 +31,10 @@ func safePath(dir, filePath string) (string, error) {
 
 	return absFilePath, nil
 }
+
+// SortItemSlice sorts a slice of items by name, case insensitive.
+func SortItemSlice(items []*Item) {
+	sort.Slice(items, func(i, j int) bool {
+		return strings.ToLower(items[i].Name) < strings.ToLower(items[j].Name)
+	})
+}

pkg/cwhub/dataset.go

@@ -6,9 +6,10 @@ import (
 	"io"
 	"net/http"
 	"os"
+	"time"
 
 	log "github.com/sirupsen/logrus"
-	"gopkg.in/yaml.v2"
+	"gopkg.in/yaml.v3"
 
 	"github.com/crowdsecurity/crowdsec/pkg/types"
 )
@@ -51,6 +52,62 @@ func downloadFile(url string, destPath string) error {
 	return nil
 }
 
+// needsUpdate checks if a data file has to be downloaded (or updated).
+// if the local file doesn't exist, update.
+// if the remote is newer than the local file, update.
+// if the remote has no modification date, but local file has been modified > a week ago, update.
+func needsUpdate(destPath string, url string) bool {
+	fileInfo, err := os.Stat(destPath)
+	switch {
+	case os.IsNotExist(err):
+		return true
+	case err != nil:
+		log.Errorf("while getting %s: %s", destPath, err)
+		return true
+	}
+
+	resp, err := hubClient.Head(url)
+	if err != nil {
+		log.Errorf("while getting %s: %s", url, err)
+		// Head failed, Get would likely fail too -> no update
+		return false
+	}
+	defer resp.Body.Close()
+
+	if resp.StatusCode != http.StatusOK {
+		log.Errorf("bad http code %d for %s", resp.StatusCode, url)
+		return false
+	}
+
+	// update if local file is older than this
+	shelfLife := 7 * 24 * time.Hour
+
+	lastModify := fileInfo.ModTime()
+
+	localIsOld := lastModify.Add(shelfLife).Before(time.Now())
+
+	remoteLastModified := resp.Header.Get("Last-Modified")
+	if remoteLastModified == "" {
+		if localIsOld {
+			log.Infof("no last modified date for %s, but local file is older than %s", url, shelfLife)
+		}
+		return localIsOld
+	}
+
+	lastAvailable, err := time.Parse(time.RFC1123, remoteLastModified)
+	if err != nil {
+		log.Warningf("while parsing last modified date for %s: %s", url, err)
+		return localIsOld
+	}
+
+	if lastModify.Before(lastAvailable) {
+		log.Infof("new version available, updating %s", destPath)
+		return true
+	}
+
+	return false
+}
+
 // downloadDataSet downloads all the data files for an item.
 func downloadDataSet(dataFolder string, force bool, reader io.Reader) error {
 	dec := yaml.NewDecoder(reader)
@@ -72,9 +129,7 @@ func downloadDataSet(dataFolder string, force bool, reader io.Reader) error {
 				return err
 			}
 
-			if _, err := os.Stat(destPath); os.IsNotExist(err) || force {
-				log.Infof("downloading data '%s' in '%s'", dataS.SourceURL, destPath)
-
+			if force || needsUpdate(destPath, dataS.SourceURL) {
 				if err := downloadFile(dataS.SourceURL, destPath); err != nil {
 					return fmt.Errorf("while getting data: %w", err)
 				}

pkg/cwhub/enable.go

@@ -1,190 +0,0 @@
-package cwhub
-
-// Enable/disable items already downloaded
-
-import (
-	"fmt"
-	"os"
-	"path/filepath"
-
-	log "github.com/sirupsen/logrus"
-)
-
-// installPath returns the location of the symlink to the item in the hub, or the path of the item itself if it's local
-// (eg. /etc/crowdsec/collections/xyz.yaml).
-// Raises an error if the path goes outside of the install dir.
-func (i *Item) installPath() (string, error) {
-	p := i.Type
-	if i.Stage != "" {
-		p = filepath.Join(p, i.Stage)
-	}
-
-	return safePath(i.hub.local.InstallDir, filepath.Join(p, i.FileName))
-}
-
-// downloadPath returns the location of the actual config file in the hub
-// (eg. /etc/crowdsec/hub/collections/author/xyz.yaml).
-// Raises an error if the path goes outside of the hub dir.
-func (i *Item) downloadPath() (string, error) {
-	ret, err := safePath(i.hub.local.HubDir, i.RemotePath)
-	if err != nil {
-		return "", err
-	}
-
-	return ret, nil
-}
-
-// makeLink creates a symlink between the actual config file at hub.HubDir and hub.ConfigDir.
-func (i *Item) createInstallLink() error {
-	dest, err := i.installPath()
-	if err != nil {
-		return err
-	}
-
-	destDir := filepath.Dir(dest)
-	if err = os.MkdirAll(destDir, os.ModePerm); err != nil {
-		return fmt.Errorf("while creating %s: %w", destDir, err)
-	}
-
-	if _, err = os.Lstat(dest); !os.IsNotExist(err) {
-		log.Infof("%s already exists.", dest)
-		return nil
-	}
-
-	src, err := i.downloadPath()
-	if err != nil {
-		return err
-	}
-
-	if err = os.Symlink(src, dest); err != nil {
-		return fmt.Errorf("while creating symlink from %s to %s: %w", src, dest, err)
-	}
-
-	return nil
-}
-
-// enable enables the item by creating a symlink to the downloaded content, and also enables sub-items.
-func (i *Item) enable() error {
-	if i.State.Installed {
-		if i.State.Tainted {
-			return fmt.Errorf("%s is tainted, won't enable unless --force", i.Name)
-		}
-
-		if i.IsLocal() {
-			return fmt.Errorf("%s is local, won't enable", i.Name)
-		}
-
-		// if it's a collection, check sub-items even if the collection file itself is up-to-date
-		if i.State.UpToDate && !i.HasSubItems() {
-			log.Tracef("%s is installed and up-to-date, skip.", i.Name)
-			return nil
-		}
-	}
-
-	for _, sub := range i.SubItems() {
-		if err := sub.enable(); err != nil {
-			return fmt.Errorf("while installing %s: %w", sub.Name, err)
-		}
-	}
-
-	if err := i.createInstallLink(); err != nil {
-		return err
-	}
-
-	log.Infof("Enabled %s: %s", i.Type, i.Name)
-	i.State.Installed = true
-
-	return nil
-}
-
-// purge removes the actual config file that was downloaded.
-func (i *Item) purge() error {
-	if !i.State.Downloaded {
-		log.Infof("removing %s: not downloaded -- no need to remove", i.Name)
-		return nil
-	}
-
-	src, err := i.downloadPath()
-	if err != nil {
-		return err
-	}
-
-	if err := os.Remove(src); err != nil {
-		if os.IsNotExist(err) {
-			log.Debugf("%s doesn't exist, no need to remove", src)
-			return nil
-		}
-
-		return fmt.Errorf("while removing file: %w", err)
-	}
-
-	i.State.Downloaded = false
-	log.Infof("Removed source file [%s]: %s", i.Name, src)
-
-	return nil
-}
-
-// removeInstallLink removes the symlink to the downloaded content.
-func (i *Item) removeInstallLink() error {
-	syml, err := i.installPath()
-	if err != nil {
-		return err
-	}
-
-	stat, err := os.Lstat(syml)
-	if err != nil {
-		return err
-	}
-
-	// if it's managed by hub, it's a symlink to csconfig.GConfig.hub.HubDir / ...
-	if stat.Mode()&os.ModeSymlink == 0 {
-		log.Warningf("%s (%s) isn't a symlink, can't disable", i.Name, syml)
-		return fmt.Errorf("%s isn't managed by hub", i.Name)
-	}
-
-	hubpath, err := os.Readlink(syml)
-	if err != nil {
-		return fmt.Errorf("while reading symlink: %w", err)
-	}
-
-	src, err := i.downloadPath()
-	if err != nil {
-		return err
-	}
-
-	if hubpath != src {
-		log.Warningf("%s (%s) isn't a symlink to %s", i.Name, syml, src)
-		return fmt.Errorf("%s isn't managed by hub", i.Name)
-	}
-
-	if err := os.Remove(syml); err != nil {
-		return fmt.Errorf("while removing symlink: %w", err)
-	}
-
-	log.Infof("Removed symlink [%s]: %s", i.Name, syml)
-
-	return nil
-}
-
-// disable removes the install link, and optionally the downloaded content.
-func (i *Item) disable(purge bool, force bool) error {
-	err := i.removeInstallLink()
-	if os.IsNotExist(err) {
-		if !purge && !force {
-			link, _ := i.installPath()
-			return fmt.Errorf("link %s does not exist (override with --force or --purge)", link)
-		}
-	} else if err != nil {
-		return err
-	}
-
-	i.State.Installed = false
-
-	if purge {
-		if err := i.purge(); err != nil {
-			return err
-		}
-	}
-
-	return nil
-}

pkg/cwhub/hub.go

@@ -159,3 +159,82 @@ func (h *Hub) updateIndex() error {
 
 	return nil
 }
+
+// GetItemMap returns the map of items for a given type.
+func (h *Hub) GetItemMap(itemType string) map[string]*Item {
+	return h.Items[itemType]
+}
+
+// GetItem returns an item from hub based on its type and full name (author/name).
+func (h *Hub) GetItem(itemType string, itemName string) *Item {
+	return h.GetItemMap(itemType)[itemName]
+}
+
+// GetItemNames returns a slice of (full) item names for a given type
+// (eg. for collections: crowdsecurity/apache2 crowdsecurity/nginx).
+func (h *Hub) GetItemNames(itemType string) []string {
+	m := h.GetItemMap(itemType)
+	if m == nil {
+		return nil
+	}
+
+	names := make([]string, 0, len(m))
+	for k := range m {
+		names = append(names, k)
+	}
+
+	return names
+}
+
+// GetAllItems returns a slice of all the items of a given type, installed or not.
+func (h *Hub) GetAllItems(itemType string) ([]*Item, error) {
+	items, ok := h.Items[itemType]
+	if !ok {
+		return nil, fmt.Errorf("no %s in the hub index", itemType)
+	}
+
+	ret := make([]*Item, len(items))
+
+	idx := 0
+
+	for _, item := range items {
+		ret[idx] = item
+		idx++
+	}
+
+	return ret, nil
+}
+
+// GetInstalledItems returns a slice of the installed items of a given type.
+func (h *Hub) GetInstalledItems(itemType string) ([]*Item, error) {
+	items, ok := h.Items[itemType]
+	if !ok {
+		return nil, fmt.Errorf("no %s in the hub index", itemType)
+	}
+
+	retItems := make([]*Item, 0)
+
+	for _, item := range items {
+		if item.State.Installed {
+			retItems = append(retItems, item)
+		}
+	}
+
+	return retItems, nil
+}
+
+// GetInstalledItemNames returns the names of the installed items of a given type.
+func (h *Hub) GetInstalledItemNames(itemType string) ([]string, error) {
+	items, err := h.GetInstalledItems(itemType)
+	if err != nil {
+		return nil, err
+	}
+
+	retStr := make([]string, len(items))
+
+	for idx, it := range items {
+		retStr[idx] = it.Name
+	}
+
+	return retStr, nil
+}

pkg/cwhub/items.go → pkg/cwhub/item.go

@@ -3,8 +3,7 @@ package cwhub
 import (
 	"encoding/json"
 	"fmt"
-	"sort"
-	"strings"
+	"path/filepath"
 
 	"github.com/Masterminds/semver/v3"
 	"github.com/enescakir/emoji"
@@ -68,9 +67,9 @@ type Item struct {
 	Author      string   `json:"author,omitempty" yaml:"author,omitempty"`
 	References  []string `json:"references,omitempty" yaml:"references,omitempty"`
 
-	RemotePath string                 `json:"path,omitempty" yaml:"remote_path,omitempty"` // path relative to the base URL eg. /parsers/stage/author/file.yaml
-	Version    string                 `json:"version,omitempty" yaml:"version,omitempty"`  // the last available version
-	Versions   map[string]ItemVersion `json:"versions,omitempty"  yaml:"-"`                // all the known versions
+	RemotePath string                 `json:"path,omitempty" yaml:"path,omitempty"`       // path relative to the base URL eg. /parsers/stage/author/file.yaml
+	Version    string                 `json:"version,omitempty" yaml:"version,omitempty"` // the last available version
+	Versions   map[string]ItemVersion `json:"versions,omitempty"  yaml:"-"`               // all the known versions
 
 	// if it's a collection, it can have sub items
 	Parsers       []string `json:"parsers,omitempty" yaml:"parsers,omitempty"`
@@ -79,6 +78,30 @@ type Item struct {
 	Collections   []string `json:"collections,omitempty" yaml:"collections,omitempty"`
 }
 
+// installPath returns the location of the symlink to the item in the hub, or the path of the item itself if it's local
+// (eg. /etc/crowdsec/collections/xyz.yaml).
+// Raises an error if the path goes outside of the install dir.
+func (i *Item) installPath() (string, error) {
+	p := i.Type
+	if i.Stage != "" {
+		p = filepath.Join(p, i.Stage)
+	}
+
+	return safePath(i.hub.local.InstallDir, filepath.Join(p, i.FileName))
+}
+
+// downloadPath returns the location of the actual config file in the hub
+// (eg. /etc/crowdsec/hub/collections/author/xyz.yaml).
+// Raises an error if the path goes outside of the hub dir.
+func (i *Item) downloadPath() (string, error) {
+	ret, err := safePath(i.hub.local.HubDir, i.RemotePath)
+	if err != nil {
+		return "", err
+	}
+
+	return ret, nil
+}
+
 // HasSubItems returns true if items of this type can have sub-items. Currently only collections.
 func (i *Item) HasSubItems() bool {
 	return i.Type == COLLECTIONS
@@ -225,6 +248,48 @@ func (i *Item) Ancestors() []*Item {
 	return ret
 }
 
+// descendants returns a list of all (direct or indirect) dependencies of the item.
+func (i *Item) descendants() ([]*Item, error) {
+	var collectSubItems func(item *Item, visited map[*Item]bool, result *[]*Item) error
+
+	collectSubItems = func(item *Item, visited map[*Item]bool, result *[]*Item) error {
+		if item == nil {
+			return nil
+		}
+
+		if visited[item] {
+			return nil
+		}
+
+		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
+}
+
 // InstallStatus returns the status of the item as a string and an emoji
 // (eg. "enabled,update-available" and emoji.Warning).
 func (i *Item) InstallStatus() (string, emoji.Emoji) {
@@ -295,89 +360,3 @@ func (i *Item) versionStatus() int {
 func (i *Item) validPath(dirName, fileName string) bool {
 	return (dirName+"/"+fileName == i.Name+".yaml") || (dirName+"/"+fileName == i.Name+".yml")
 }
-
-// GetItemMap returns the map of items for a given type.
-func (h *Hub) GetItemMap(itemType string) map[string]*Item {
-	return h.Items[itemType]
-}
-
-// GetItem returns an item from hub based on its type and full name (author/name).
-func (h *Hub) GetItem(itemType string, itemName string) *Item {
-	return h.GetItemMap(itemType)[itemName]
-}
-
-// GetItemNames returns a slice of (full) item names for a given type
-// (eg. for collections: crowdsecurity/apache2 crowdsecurity/nginx).
-func (h *Hub) GetItemNames(itemType string) []string {
-	m := h.GetItemMap(itemType)
-	if m == nil {
-		return nil
-	}
-
-	names := make([]string, 0, len(m))
-	for k := range m {
-		names = append(names, k)
-	}
-
-	return names
-}
-
-// GetAllItems returns a slice of all the items of a given type, installed or not.
-func (h *Hub) GetAllItems(itemType string) ([]*Item, error) {
-	items, ok := h.Items[itemType]
-	if !ok {
-		return nil, fmt.Errorf("no %s in the hub index", itemType)
-	}
-
-	ret := make([]*Item, len(items))
-
-	idx := 0
-
-	for _, item := range items {
-		ret[idx] = item
-		idx++
-	}
-
-	return ret, nil
-}
-
-// GetInstalledItems returns a slice of the installed items of a given type.
-func (h *Hub) GetInstalledItems(itemType string) ([]*Item, error) {
-	items, ok := h.Items[itemType]
-	if !ok {
-		return nil, fmt.Errorf("no %s in the hub index", itemType)
-	}
-
-	retItems := make([]*Item, 0)
-
-	for _, item := range items {
-		if item.State.Installed {
-			retItems = append(retItems, item)
-		}
-	}
-
-	return retItems, nil
-}
-
-// GetInstalledItemNames returns the names of the installed items of a given type.
-func (h *Hub) GetInstalledItemNames(itemType string) ([]string, error) {
-	items, err := h.GetInstalledItems(itemType)
-	if err != nil {
-		return nil, err
-	}
-
-	retStr := make([]string, len(items))
-
-	for idx, it := range items {
-		retStr[idx] = it.Name
-	}
-
-	return retStr, nil
-}
-
-// SortItemSlice sorts a slice of items by name, case insensitive.
-func SortItemSlice(items []*Item) {
-	sort.Slice(items, func(i, j int) bool {
-		return strings.ToLower(items[i].Name) < strings.ToLower(items[j].Name)
-	})
-}

pkg/cwhub/iteminstall.go

@@ -0,0 +1,70 @@
+package cwhub
+
+import (
+	"fmt"
+
+	log "github.com/sirupsen/logrus"
+)
+
+// enable enables the item by creating a symlink to the downloaded content, and also enables sub-items.
+func (i *Item) enable() error {
+	if i.State.Installed {
+		if i.State.Tainted {
+			return fmt.Errorf("%s is tainted, won't enable unless --force", i.Name)
+		}
+
+		if i.IsLocal() {
+			return fmt.Errorf("%s is local, won't enable", i.Name)
+		}
+
+		// if it's a collection, check sub-items even if the collection file itself is up-to-date
+		if i.State.UpToDate && !i.HasSubItems() {
+			log.Tracef("%s is installed and up-to-date, skip.", i.Name)
+			return nil
+		}
+	}
+
+	for _, sub := range i.SubItems() {
+		if err := sub.enable(); err != nil {
+			return fmt.Errorf("while installing %s: %w", sub.Name, err)
+		}
+	}
+
+	if err := i.createInstallLink(); err != nil {
+		return err
+	}
+
+	log.Infof("Enabled %s: %s", i.Type, i.Name)
+	i.State.Installed = true
+
+	return nil
+}
+
+// Install installs the item from the hub, downloading it if needed.
+func (i *Item) Install(force bool, downloadOnly bool) error {
+	if downloadOnly && i.State.Downloaded && i.State.UpToDate {
+		log.Infof("%s is already downloaded and up-to-date", i.Name)
+
+		if !force {
+			return nil
+		}
+	}
+
+	filePath, err := i.downloadLatest(force, true)
+	if err != nil {
+		return fmt.Errorf("while downloading %s: %w", i.Name, err)
+	}
+
+	if downloadOnly {
+		log.Infof("Downloaded %s to %s", i.Name, filePath)
+		return nil
+	}
+
+	if err := i.enable(); err != nil {
+		return fmt.Errorf("while enabling %s: %w", i.Name, err)
+	}
+
+	log.Infof("Enabled %s", i.Name)
+
+	return nil
+}

pkg/cwhub/enable_test.go → pkg/cwhub/iteminstall_test.go

@@ -63,7 +63,7 @@ func testDisable(hub *Hub, t *testing.T, item *Item) {
 	assert.True(t, hub.Items[item.Type][item.Name].State.Installed, "%s should be installed", item.Name)
 
 	// Remove
-	err := item.disable(false, false)
+	_, err := item.disable(false, false)
 	require.NoError(t, err, "failed to disable %s", item.Name)
 
 	// Local sync and check status
@@ -76,7 +76,7 @@ func testDisable(hub *Hub, t *testing.T, item *Item) {
 	assert.True(t, hub.Items[item.Type][item.Name].State.Downloaded, "%s should still be downloaded", item.Name)
 
 	// Purge
-	err = item.disable(true, false)
+	_, err = item.disable(true, false)
 	require.NoError(t, err, "failed to purge %s", item.Name)
 
 	// Local sync and check status

pkg/cwhub/itemlink.go

@@ -0,0 +1,80 @@
+package cwhub
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+
+	log "github.com/sirupsen/logrus"
+)
+
+// createInstallLink creates a symlink between the actual config file at hub.HubDir and hub.ConfigDir.
+func (i *Item) createInstallLink() error {
+	dest, err := i.installPath()
+	if err != nil {
+		return err
+	}
+
+	destDir := filepath.Dir(dest)
+	if err = os.MkdirAll(destDir, os.ModePerm); err != nil {
+		return fmt.Errorf("while creating %s: %w", destDir, err)
+	}
+
+	if _, err = os.Lstat(dest); !os.IsNotExist(err) {
+		log.Infof("%s already exists.", dest)
+		return nil
+	}
+
+	src, err := i.downloadPath()
+	if err != nil {
+		return err
+	}
+
+	if err = os.Symlink(src, dest); err != nil {
+		return fmt.Errorf("while creating symlink from %s to %s: %w", src, dest, err)
+	}
+
+	return nil
+}
+
+// removeInstallLink removes the symlink to the downloaded content.
+func (i *Item) removeInstallLink() error {
+	syml, err := i.installPath()
+	if err != nil {
+		return err
+	}
+
+	stat, err := os.Lstat(syml)
+	if err != nil {
+		return err
+	}
+
+	// if it's managed by hub, it's a symlink to csconfig.GConfig.hub.HubDir / ...
+	if stat.Mode()&os.ModeSymlink == 0 {
+		log.Warningf("%s (%s) isn't a symlink, can't disable", i.Name, syml)
+		return fmt.Errorf("%s isn't managed by hub", i.Name)
+	}
+
+	hubpath, err := os.Readlink(syml)
+	if err != nil {
+		return fmt.Errorf("while reading symlink: %w", err)
+	}
+
+	src, err := i.downloadPath()
+	if err != nil {
+		return err
+	}
+
+	if hubpath != src {
+		log.Warningf("%s (%s) isn't a symlink to %s", i.Name, syml, src)
+		return fmt.Errorf("%s isn't managed by hub", i.Name)
+	}
+
+	if err := os.Remove(syml); err != nil {
+		return fmt.Errorf("while removing symlink: %w", err)
+	}
+
+	log.Infof("Removed symlink [%s]: %s", i.Name, syml)
+
+	return nil
+}

pkg/cwhub/itemremove.go

@@ -0,0 +1,139 @@
+package cwhub
+
+import (
+	"fmt"
+	"os"
+
+	log "github.com/sirupsen/logrus"
+	"slices"
+)
+
+// purge removes the actual config file that was downloaded.
+func (i *Item) purge() (bool, error) {
+	if !i.State.Downloaded {
+		log.Debugf("removing %s: not downloaded -- no need to remove", i.Name)
+		return false, nil
+	}
+
+	src, err := i.downloadPath()
+	if err != nil {
+		return false, err
+	}
+
+	if err := os.Remove(src); err != nil {
+		if os.IsNotExist(err) {
+			log.Debugf("%s doesn't exist, no need to remove", src)
+			return false, nil
+		}
+
+		return false, fmt.Errorf("while removing file: %w", err)
+	}
+
+	i.State.Downloaded = false
+	log.Infof("Removed source file [%s]: %s", i.Name, src)
+
+	return true, nil
+}
+
+// disable removes the install link, and optionally the downloaded content.
+func (i *Item) disable(purge bool, force bool) (bool, error) {
+	didRemove := true
+
+	err := i.removeInstallLink()
+	if os.IsNotExist(err) {
+		if !purge && !force {
+			link, _ := i.installPath()
+			return false, fmt.Errorf("link %s does not exist (override with --force or --purge)", link)
+		}
+		didRemove = false
+	} else if err != nil {
+		return false, err
+	}
+
+	i.State.Installed = false
+
+	didPurge := false
+	if purge {
+		if didPurge, err = i.purge(); err != nil {
+			return didRemove, err
+		}
+	}
+
+	ret := didRemove || didPurge
+
+	return ret, nil
+}
+
+// Remove disables the item, optionally removing the downloaded content.
+func (i *Item) Remove(purge bool, force bool) (bool, error) {
+	if i.IsLocal() {
+		log.Warningf("%s is a local item, please delete manually", i.Name)
+		return false, nil
+	}
+
+	if i.State.Tainted && !force {
+		return false, fmt.Errorf("%s is tainted, use '--force' to remove", i.Name)
+	}
+
+	if !i.State.Installed && !purge {
+		log.Infof("removing %s: not installed -- no need to remove", i.Name)
+		return false, nil
+	}
+
+	removed := false
+
+	descendants, err := i.descendants()
+	if err != nil {
+		return false, err
+	}
+
+	ancestors := i.Ancestors()
+
+	for _, sub := range i.SubItems() {
+		if !sub.State.Installed {
+			continue
+		}
+
+		// if the sub depends on a collection that is not a direct or indirect dependency
+		// of the current item, it is not removed
+		for _, subParent := range sub.Ancestors() {
+			if !purge && !subParent.State.Installed {
+				continue
+			}
+
+			// the ancestor that would block the removal of the sub item is also an ancestor
+			// of the item we are removing, so we don't want false warnings
+			// (e.g. crowdsecurity/sshd-logs was not removed because it also belongs to crowdsecurity/linux,
+			// while we are removing crowdsecurity/sshd)
+			if slices.Contains(ancestors, subParent) {
+				continue
+			}
+
+			// the sub-item belongs to the item we are removing, but we already knew that
+			if subParent == i {
+				continue
+			}
+
+			if !slices.Contains(descendants, subParent) {
+				log.Infof("%s was not removed because it also belongs to %s", sub.Name, subParent.Name)
+				continue
+			}
+		}
+
+		subRemoved, err := sub.Remove(purge, force)
+		if err != nil {
+			return false, fmt.Errorf("unable to disable %s: %w", i.Name, err)
+		}
+
+		removed = removed || subRemoved
+	}
+
+	didDisable, err := i.disable(purge, force)
+	if err != nil {
+		return false, fmt.Errorf("while removing %s: %w", i.Name, err)
+	}
+
+	removed = removed || didDisable
+
+	return removed, nil
+}

pkg/cwhub/helpers.go → pkg/cwhub/itemupgrade.go

@@ -14,156 +14,17 @@ import (
 
 	"github.com/enescakir/emoji"
 	log "github.com/sirupsen/logrus"
-	"slices"
 )
 
-// Install installs the item from the hub, downloading it if needed.
-func (i *Item) Install(force bool, downloadOnly bool) error {
-	if downloadOnly && i.State.Downloaded && i.State.UpToDate {
-		log.Infof("%s is already downloaded and up-to-date", i.Name)
-
-		if !force {
-			return nil
-		}
-	}
-
-	filePath, err := i.downloadLatest(force, true)
-	if err != nil {
-		return fmt.Errorf("while downloading %s: %w", i.Name, err)
-	}
-
-	if downloadOnly {
-		log.Infof("Downloaded %s to %s", i.Name, filePath)
-		return nil
-	}
-
-	if err := i.enable(); err != nil {
-		return fmt.Errorf("while enabling %s: %w", i.Name, err)
-	}
-
-	log.Infof("Enabled %s", i.Name)
-
-	return nil
-}
-
-// descendants returns a list of all (direct or indirect) dependencies of the item.
-func (i *Item) descendants() ([]*Item, error) {
-	var collectSubItems func(item *Item, visited map[*Item]bool, result *[]*Item) error
-
-	collectSubItems = func(item *Item, visited map[*Item]bool, result *[]*Item) error {
-		if item == nil {
-			return nil
-		}
-
-		if visited[item] {
-			return nil
-		}
-
-		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
-}
+// Upgrade downloads and applies the last version of the item from the hub.
+func (i *Item) Upgrade(force bool) (bool, error) {
+	updated := false
 
-// Remove disables the item, optionally removing the downloaded content.
-func (i *Item) Remove(purge bool, force bool) (bool, error) {
 	if i.IsLocal() {
-		return false, fmt.Errorf("%s isn't managed by hub. Please delete manually", i.Name)
-	}
-
-	if i.State.Tainted && !force {
-		return false, fmt.Errorf("%s is tainted, use '--force' to remove", i.Name)
-	}
-
-	if !i.State.Installed && !purge {
-		log.Infof("removing %s: not installed -- no need to remove", i.Name)
+		log.Infof("not upgrading %s: local item", i.Name)
 		return false, nil
 	}
 
-	removed := false
-
-	descendants, err := i.descendants()
-	if err != nil {
-		return false, err
-	}
-
-	ancestors := i.Ancestors()
-
-	for _, sub := range i.SubItems() {
-		if !sub.State.Installed {
-			continue
-		}
-
-		// if the sub depends on a collection that is not a direct or indirect dependency
-		// of the current item, it is not removed
-		for _, subParent := range sub.Ancestors() {
-			if !purge && !subParent.State.Installed {
-				continue
-			}
-
-			// the ancestor that would block the removal of the sub item is also an ancestor
-			// of the item we are removing, so we don't want false warnings
-			// (e.g. crowdsecurity/sshd-logs was not removed because it also belongs to crowdsecurity/linux,
-			// while we are removing crowdsecurity/sshd)
-			if slices.Contains(ancestors, subParent) {
-				continue
-			}
-
-			// the sub-item belongs to the item we are removing, but we already knew that
-			if subParent == i {
-				continue
-			}
-
-			if !slices.Contains(descendants, subParent) {
-				log.Infof("%s was not removed because it also belongs to %s", sub.Name, subParent.Name)
-				continue
-			}
-		}
-
-		subRemoved, err := sub.Remove(purge, force)
-		if err != nil {
-			return false, fmt.Errorf("unable to disable %s: %w", i.Name, err)
-		}
-
-		removed = removed || subRemoved
-	}
-
-	if err = i.disable(purge, force); err != nil {
-		return false, fmt.Errorf("while removing %s: %w", i.Name, err)
-	}
-
-	removed = true
-
-	return removed, nil
-}
-
-// Upgrade downloads and applies the last version of the item from the hub.
-func (i *Item) Upgrade(force bool) (bool, error) {
-	updated := false
-
 	if !i.State.Downloaded {
 		return false, fmt.Errorf("can't upgrade %s: not installed", i.Name)
 	}
@@ -192,8 +53,6 @@ func (i *Item) Upgrade(force bool) (bool, error) {
 	if !i.State.UpToDate {
 		if i.State.Tainted {
 			log.Warningf("%v %s is tainted, --force to overwrite", emoji.Warning, i.Name)
-		} else if i.IsLocal() {
-			log.Infof("%v %s is local", emoji.Prohibited, i.Name)
 		}
 	} else {
 		// a check on stdout is used while scripting to know if the hub has been upgraded
@@ -296,6 +155,9 @@ func (i *Item) fetch() ([]byte, error) {
 
 // download downloads the item from the hub and writes it to the hub directory.
 func (i *Item) download(overwrite bool) (string, error) {
+	if i.IsLocal() {
+		return "", fmt.Errorf("%s is local, can't download", i.Name)
+	}
 	// if user didn't --force, don't overwrite local, tainted, up-to-date files
 	if !overwrite {
 		if i.State.Tainted {

pkg/cwhub/sync.go

@@ -30,7 +30,7 @@ func linkTarget(path string) (string, error) {
 
 	_, err = os.Lstat(hubpath)
 	if os.IsNotExist(err) {
-		log.Infof("link target does not exist: %s -> %s", path, hubpath)
+		log.Warningf("link target does not exist: %s -> %s", path, hubpath)
 		return "", nil
 	}
 

test/bats/20_hub.bats

@@ -119,3 +119,10 @@ teardown() {
     # this is used by the cron script to know if the hub was updated
     assert_output --partial "updated crowdsecurity/syslog-logs"
 }
+
+@test "cscli hub upgrade (with local items)" {
+    mkdir -p "$CONFIG_DIR/collections"
+    touch "$CONFIG_DIR/collections/foo.yaml"
+    rune -0 cscli hub upgrade
+    assert_stderr --partial "not upgrading foo.yaml: local item"
+}

test/bats/20_hub_collections.bats

@@ -208,7 +208,7 @@ teardown() {
     assert_line 'type: collections'
     assert_line 'name: crowdsecurity/sshd'
     assert_line 'author: crowdsecurity'
-    assert_line 'remote_path: collections/crowdsecurity/sshd.yaml'
+    assert_line 'path: collections/crowdsecurity/sshd.yaml'
     assert_line 'installed: false'
     refute_line --partial 'Current metrics:'
 
@@ -226,7 +226,7 @@ teardown() {
     assert_line 'type: collections'
     assert_line 'name: crowdsecurity/sshd'
     assert_line 'author: crowdsecurity'
-    assert_line 'remote_path: collections/crowdsecurity/sshd.yaml'
+    assert_line 'path: collections/crowdsecurity/sshd.yaml'
     assert_line 'installed: false'
     refute_line --partial 'Current metrics:'
 
@@ -275,8 +275,9 @@ teardown() {
     rune -0 cscli collections remove crowdsecurity/sshd
     assert_stderr --partial 'removing crowdsecurity/sshd: not installed -- no need to remove'
 
-    rune -0 cscli collections remove crowdsecurity/sshd --purge
+    rune -0 cscli collections remove crowdsecurity/sshd --purge --debug
     assert_stderr --partial 'removing crowdsecurity/sshd: not downloaded -- no need to remove'
+    refute_stderr --partial 'Removed source file [crowdsecurity/sshd]'
 
     # install, then remove, check files
     rune -0 cscli collections install crowdsecurity/sshd

test/bats/20_hub_items.bats

@@ -99,7 +99,7 @@ teardown() {
     rune -0 jq -r '.path' <(output)
     rune -0 rm "$HUB_DIR/$(output)"
 
-    rune -0 cscli parsers remove crowdsecurity/syslog-logs --purge
+    rune -0 cscli parsers remove crowdsecurity/syslog-logs --purge --debug
     assert_stderr --partial "removing crowdsecurity/syslog-logs: not downloaded -- no need to remove"
 
     rune -0 cscli parsers remove crowdsecurity/linux --all --error --purge --force
@@ -147,3 +147,37 @@ teardown() {
     rune -0 cscli collections inspect hi-its-me -o json
     rune -0 jq -e '[.installed,.local]==[true,true]' <(output)
 }
+
+@test "a local item cannot be downloaded by cscli" {
+    rune -0 mkdir -p "$CONFIG_DIR/collections"
+    rune -0 touch "$CONFIG_DIR/collections/foobar.yaml"
+    rune -1 cscli collections install foobar.yaml
+    assert_stderr --partial "failed to download item: foobar.yaml is local, can't download"
+    rune -1 cscli collections install foobar.yaml --force
+    assert_stderr --partial "failed to download item: foobar.yaml is local, can't download"
+}
+
+@test "a local item cannot be removed by cscli" {
+    rune -0 mkdir -p "$CONFIG_DIR/collections"
+    rune -0 touch "$CONFIG_DIR/collections/foobar.yaml"
+    rune -0 cscli collections remove foobar.yaml
+    assert_stderr --partial "foobar.yaml is a local item, please delete manually"
+    rune -0 cscli collections remove foobar.yaml --purge
+    assert_stderr --partial "foobar.yaml is a local item, please delete manually"
+    rune -0 cscli collections remove foobar.yaml --force
+    assert_stderr --partial "foobar.yaml is a local item, please delete manually"
+    rune -0 cscli collections remove --all
+    assert_stderr --partial "foobar.yaml is a local item, please delete manually"
+    rune -0 cscli collections remove --all --purge
+    assert_stderr --partial "foobar.yaml is a local item, please delete manually"
+}
+
+@test "a dangling link is reported with a warning" {
+    rune -0 mkdir -p "$CONFIG_DIR/collections"
+    rune -0 ln -s /this/does/not/exist.yaml "$CONFIG_DIR/collections/foobar.yaml"
+    rune -0 cscli hub list
+    assert_stderr --partial "link target does not exist: $CONFIG_DIR/collections/foobar.yaml -> /this/does/not/exist.yaml"
+    rune -0 cscli hub list -o json
+    rune -0 jq '.collections' <(output)
+    assert_json '[]'
+}

test/bats/20_hub_parsers.bats

@@ -209,7 +209,7 @@ teardown() {
     assert_line 'stage: s01-parse'
     assert_line 'name: crowdsecurity/sshd-logs'
     assert_line 'author: crowdsecurity'
-    assert_line 'remote_path: parsers/s01-parse/crowdsecurity/sshd-logs.yaml'
+    assert_line 'path: parsers/s01-parse/crowdsecurity/sshd-logs.yaml'
     assert_line 'installed: false'
     refute_line --partial 'Current metrics:'
 
@@ -228,7 +228,7 @@ teardown() {
     assert_line 'name: crowdsecurity/sshd-logs'
     assert_line 'stage: s01-parse'
     assert_line 'author: crowdsecurity'
-    assert_line 'remote_path: parsers/s01-parse/crowdsecurity/sshd-logs.yaml'
+    assert_line 'path: parsers/s01-parse/crowdsecurity/sshd-logs.yaml'
     assert_line 'installed: false'
     refute_line --partial 'Current metrics:'
 
@@ -277,8 +277,9 @@ teardown() {
     rune -0 cscli parsers remove crowdsecurity/whitelists
     assert_stderr --partial "removing crowdsecurity/whitelists: not installed -- no need to remove"
 
-    rune -0 cscli parsers remove crowdsecurity/whitelists --purge
+    rune -0 cscli parsers remove crowdsecurity/whitelists --purge --debug
     assert_stderr --partial 'removing crowdsecurity/whitelists: not downloaded -- no need to remove'
+    refute_stderr --partial 'Removed source file [crowdsecurity/whitelists]'
 
     # install, then remove, check files
     rune -0 cscli parsers install crowdsecurity/whitelists

test/bats/20_hub_postoverflows.bats

@@ -209,7 +209,7 @@ teardown() {
     assert_line 'stage: s00-enrich'
     assert_line 'name: crowdsecurity/rdns'
     assert_line 'author: crowdsecurity'
-    assert_line 'remote_path: postoverflows/s00-enrich/crowdsecurity/rdns.yaml'
+    assert_line 'path: postoverflows/s00-enrich/crowdsecurity/rdns.yaml'
     assert_line 'installed: false'
     refute_line --partial 'Current metrics:'
 
@@ -228,7 +228,7 @@ teardown() {
     assert_line 'name: crowdsecurity/rdns'
     assert_line 'stage: s00-enrich'
     assert_line 'author: crowdsecurity'
-    assert_line 'remote_path: postoverflows/s00-enrich/crowdsecurity/rdns.yaml'
+    assert_line 'path: postoverflows/s00-enrich/crowdsecurity/rdns.yaml'
     assert_line 'installed: false'
     refute_line --partial 'Current metrics:'
 
@@ -277,8 +277,9 @@ teardown() {
     rune -0 cscli postoverflows remove crowdsecurity/rdns
     assert_stderr --partial 'removing crowdsecurity/rdns: not installed -- no need to remove'
 
-    rune -0 cscli postoverflows remove crowdsecurity/rdns --purge
+    rune -0 cscli postoverflows remove crowdsecurity/rdns --purge --debug
     assert_stderr --partial 'removing crowdsecurity/rdns: not downloaded -- no need to remove'
+    refute_stderr --partial 'Removed source file [crowdsecurity/rdns]'
 
     # install, then remove, check files
     rune -0 cscli postoverflows install crowdsecurity/rdns

test/bats/20_hub_scenarios.bats

@@ -209,7 +209,7 @@ teardown() {
     assert_line 'type: scenarios'
     assert_line 'name: crowdsecurity/ssh-bf'
     assert_line 'author: crowdsecurity'
-    assert_line 'remote_path: scenarios/crowdsecurity/ssh-bf.yaml'
+    assert_line 'path: scenarios/crowdsecurity/ssh-bf.yaml'
     assert_line 'installed: false'
     refute_line --partial 'Current metrics:'
 
@@ -227,7 +227,7 @@ teardown() {
     assert_line 'type: scenarios'
     assert_line 'name: crowdsecurity/ssh-bf'
     assert_line 'author: crowdsecurity'
-    assert_line 'remote_path: scenarios/crowdsecurity/ssh-bf.yaml'
+    assert_line 'path: scenarios/crowdsecurity/ssh-bf.yaml'
     assert_line 'installed: false'
     refute_line --partial 'Current metrics:'
 
@@ -276,8 +276,9 @@ teardown() {
     rune -0 cscli scenarios remove crowdsecurity/ssh-bf
     assert_stderr --partial "removing crowdsecurity/ssh-bf: not installed -- no need to remove"
 
-    rune -0 cscli scenarios remove crowdsecurity/ssh-bf --purge
+    rune -0 cscli scenarios remove crowdsecurity/ssh-bf --purge --debug
     assert_stderr --partial 'removing crowdsecurity/ssh-bf: not downloaded -- no need to remove'
+    refute_stderr --partial 'Removed source file [crowdsecurity/ssh-bf]'
 
     # install, then remove, check files
     rune -0 cscli scenarios install crowdsecurity/ssh-bf