From 6b0bdc5eebece47882daa32a4a28d1532f24a1de Mon Sep 17 00:00:00 2001 From: mmetc <92726601+mmetc@users.noreply.github.com> Date: Tue, 28 Nov 2023 23:51:51 +0100 Subject: [PATCH] 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 | 8 +- cmd/crowdsec-cli/itemcommands.go | 5 +- cmd/crowdsec-cli/utils.go | 5 +- pkg/cwhub/cwhub.go | 8 + pkg/cwhub/dataset.go | 63 +++++- pkg/cwhub/enable.go | 190 ------------------ pkg/cwhub/hub.go | 79 ++++++++ pkg/cwhub/{items.go => item.go} | 161 +++++++-------- pkg/cwhub/{items_test.go => item_test.go} | 0 pkg/cwhub/iteminstall.go | 70 +++++++ .../{enable_test.go => iteminstall_test.go} | 4 +- pkg/cwhub/itemlink.go | 80 ++++++++ pkg/cwhub/itemremove.go | 139 +++++++++++++ pkg/cwhub/{helpers.go => itemupgrade.go} | 154 +------------- .../{helpers_test.go => itemupgrade_test.go} | 0 pkg/cwhub/sync.go | 2 +- test/bats/20_hub.bats | 7 + test/bats/20_hub_collections.bats | 7 +- test/bats/20_hub_items.bats | 36 +++- test/bats/20_hub_parsers.bats | 7 +- test/bats/20_hub_postoverflows.bats | 7 +- test/bats/20_hub_scenarios.bats | 7 +- 22 files changed, 583 insertions(+), 456 deletions(-) delete mode 100644 pkg/cwhub/enable.go rename pkg/cwhub/{items.go => item.go} (80%) rename pkg/cwhub/{items_test.go => item_test.go} (100%) create mode 100644 pkg/cwhub/iteminstall.go rename pkg/cwhub/{enable_test.go => iteminstall_test.go} (98%) create mode 100644 pkg/cwhub/itemlink.go create mode 100644 pkg/cwhub/itemremove.go rename pkg/cwhub/{helpers.go => itemupgrade.go} (63%) rename pkg/cwhub/{helpers_test.go => itemupgrade_test.go} (100%) diff --git a/cmd/crowdsec-cli/item_suggest.go b/cmd/crowdsec-cli/item_suggest.go index e9db3b7b9..af7211e80 100644 --- a/cmd/crowdsec-cli/item_suggest.go +++ b/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) } diff --git a/cmd/crowdsec-cli/itemcommands.go b/cmd/crowdsec-cli/itemcommands.go index 6b9476d02..6ec53681d 100644 --- a/cmd/crowdsec-cli/itemcommands.go +++ b/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()) } diff --git a/cmd/crowdsec-cli/utils.go b/cmd/crowdsec-cli/utils.go index eb7fb51e0..362a8942f 100644 --- a/cmd/crowdsec-cli/utils.go +++ b/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 } diff --git a/pkg/cwhub/cwhub.go b/pkg/cwhub/cwhub.go index ff34bed59..9ce091fad 100644 --- a/pkg/cwhub/cwhub.go +++ b/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) + }) +} diff --git a/pkg/cwhub/dataset.go b/pkg/cwhub/dataset.go index e624436c8..a2c8479d2 100644 --- a/pkg/cwhub/dataset.go +++ b/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) } diff --git a/pkg/cwhub/enable.go b/pkg/cwhub/enable.go deleted file mode 100644 index a8f46c6b3..000000000 --- a/pkg/cwhub/enable.go +++ /dev/null @@ -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 -} diff --git a/pkg/cwhub/hub.go b/pkg/cwhub/hub.go index ff1c3cf15..1cff0a21b 100644 --- a/pkg/cwhub/hub.go +++ b/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 +} diff --git a/pkg/cwhub/items.go b/pkg/cwhub/item.go similarity index 80% rename from pkg/cwhub/items.go rename to pkg/cwhub/item.go index 9fc528590..9c3ec8cd2 100644 --- a/pkg/cwhub/items.go +++ b/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) - }) -} diff --git a/pkg/cwhub/items_test.go b/pkg/cwhub/item_test.go similarity index 100% rename from pkg/cwhub/items_test.go rename to pkg/cwhub/item_test.go diff --git a/pkg/cwhub/iteminstall.go b/pkg/cwhub/iteminstall.go new file mode 100644 index 000000000..038e5471d --- /dev/null +++ b/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 +} diff --git a/pkg/cwhub/enable_test.go b/pkg/cwhub/iteminstall_test.go similarity index 98% rename from pkg/cwhub/enable_test.go rename to pkg/cwhub/iteminstall_test.go index 35e56915b..36ba7c11d 100644 --- a/pkg/cwhub/enable_test.go +++ b/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 diff --git a/pkg/cwhub/itemlink.go b/pkg/cwhub/itemlink.go new file mode 100644 index 000000000..2a925826c --- /dev/null +++ b/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 +} diff --git a/pkg/cwhub/itemremove.go b/pkg/cwhub/itemremove.go new file mode 100644 index 000000000..cba5a5904 --- /dev/null +++ b/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 +} diff --git a/pkg/cwhub/helpers.go b/pkg/cwhub/itemupgrade.go similarity index 63% rename from pkg/cwhub/helpers.go rename to pkg/cwhub/itemupgrade.go index fc8a98d43..5e7c6f71a 100644 --- a/pkg/cwhub/helpers.go +++ b/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 -} - -// 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) - 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.IsLocal() { + log.Infof("not upgrading %s: local item", i.Name) + return false, nil + } + 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 { diff --git a/pkg/cwhub/helpers_test.go b/pkg/cwhub/itemupgrade_test.go similarity index 100% rename from pkg/cwhub/helpers_test.go rename to pkg/cwhub/itemupgrade_test.go diff --git a/pkg/cwhub/sync.go b/pkg/cwhub/sync.go index bc64cd01c..7dc03e093 100644 --- a/pkg/cwhub/sync.go +++ b/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 } diff --git a/test/bats/20_hub.bats b/test/bats/20_hub.bats index f0a8fa4dc..3f2ce8c4b 100644 --- a/test/bats/20_hub.bats +++ b/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" +} diff --git a/test/bats/20_hub_collections.bats b/test/bats/20_hub_collections.bats index f49d0e24b..5e5b43a9e 100644 --- a/test/bats/20_hub_collections.bats +++ b/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 diff --git a/test/bats/20_hub_items.bats b/test/bats/20_hub_items.bats index 3fb38c663..dd78c0cf4 100644 --- a/test/bats/20_hub_items.bats +++ b/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 '[]' +} diff --git a/test/bats/20_hub_parsers.bats b/test/bats/20_hub_parsers.bats index c780457b3..71a1f933a 100644 --- a/test/bats/20_hub_parsers.bats +++ b/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 diff --git a/test/bats/20_hub_postoverflows.bats b/test/bats/20_hub_postoverflows.bats index 55c384942..de4b1e8a5 100644 --- a/test/bats/20_hub_postoverflows.bats +++ b/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 diff --git a/test/bats/20_hub_scenarios.bats b/test/bats/20_hub_scenarios.bats index bf033c2f9..9c441057a 100644 --- a/test/bats/20_hub_scenarios.bats +++ b/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