Refact cwhub: version comparison and branch selection (#2581)

* simplify GetItemByPath
* hub: sort version numbers by semver
* replace golang.org/x/mod/semver with github.com/Masterminds/semver/v3 (would not compare correctly)
* fix nil dereference with tainted items
* update tests for collections, postoverflows
* fix nil deref
* don't fallback to master if hub is not found, improve message
* explicit message for unknown version / tainted collections
This commit is contained in:
mmetc 2023-11-08 13:21:59 +01:00 committed by GitHub
parent ad54b99bf9
commit f4b5bcb865
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 207 additions and 81 deletions

View file

@ -1,7 +1,6 @@
package main
import (
"errors"
"fmt"
"github.com/fatih/color"
@ -94,16 +93,7 @@ func runHubUpdate(cmd *cobra.Command, args []string) error {
// don't use require.Hub because if there is no index file, it would fail
hub, err := cwhub.NewHub(local, remote, true)
if err != nil {
// XXX: this should be done when downloading items, too
// but what is the fallback to master actually solving?
if !errors.Is(err, cwhub.ErrIndexNotFound) {
return fmt.Errorf("failed to get Hub index: %w", err)
}
log.Warnf("Could not find index file for branch '%s', using 'master'", remote.Branch)
remote.Branch = "master"
if hub, err = cwhub.NewHub(local, remote, true); err != nil {
return fmt.Errorf("failed to get Hub index after retry: %w", err)
}
return fmt.Errorf("failed to update hub: %w", err)
}
// use LocalSync to get warnings about tainted / outdated items

View file

@ -19,7 +19,7 @@ import (
)
// XXX: this should not need hub?
func ShowMetrics(hub *cwhub.Hub, hubItem *cwhub.Item) {
func ShowMetrics(hub *cwhub.Hub, hubItem *cwhub.Item) error {
switch hubItem.Type {
case cwhub.PARSERS:
metrics := GetParserMetric(csConfig.Cscli.PrometheusUrl, hubItem.Name)
@ -28,24 +28,27 @@ func ShowMetrics(hub *cwhub.Hub, hubItem *cwhub.Item) {
metrics := GetScenarioMetric(csConfig.Cscli.PrometheusUrl, hubItem.Name)
scenarioMetricsTable(color.Output, hubItem.Name, metrics)
case cwhub.COLLECTIONS:
for _, item := range hubItem.Parsers {
metrics := GetParserMetric(csConfig.Cscli.PrometheusUrl, item)
parserMetricsTable(color.Output, item, metrics)
for _, parserName := range hubItem.Parsers {
metrics := GetParserMetric(csConfig.Cscli.PrometheusUrl, parserName)
parserMetricsTable(color.Output, parserName, metrics)
}
for _, item := range hubItem.Scenarios {
metrics := GetScenarioMetric(csConfig.Cscli.PrometheusUrl, item)
scenarioMetricsTable(color.Output, item, metrics)
for _, scenarioName := range hubItem.Scenarios {
metrics := GetScenarioMetric(csConfig.Cscli.PrometheusUrl, scenarioName)
scenarioMetricsTable(color.Output, scenarioName, metrics)
}
for _, item := range hubItem.Collections {
hubItem = hub.GetItem(cwhub.COLLECTIONS, item)
if hubItem == nil {
log.Fatalf("unable to retrieve item '%s' from collection '%s'", item, hubItem.Name)
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 {
return err
}
ShowMetrics(hub, hubItem)
}
default:
log.Errorf("item of type '%s' is unknown", hubItem.Type)
}
return nil
}
// GetParserMetric is a complete rip from prom2json

View file

@ -157,7 +157,9 @@ func InspectItem(hub *cwhub.Hub, item *cwhub.Item, showMetrics bool) error {
if csConfig.Cscli.Output == "human" && showMetrics {
fmt.Printf("\nCurrent metrics: \n")
ShowMetrics(hub, item)
if err := ShowMetrics(hub, item); err != nil {
return err
}
}
return nil

View file

@ -91,7 +91,7 @@ func Hub(c *csconfig.Config, remote *cwhub.RemoteHubCfg) (*cwhub.Hub, error) {
hub, err := cwhub.NewHub(local, remote, false)
if err != nil {
return nil, fmt.Errorf("failed to read Hub index: '%w'. Run 'sudo cscli hub update' to download the index again", err)
return nil, fmt.Errorf("failed to read Hub index: %w. Run 'sudo cscli hub update' to download the index again", err)
}
return hub, nil

View file

@ -2,11 +2,20 @@ package cwhub
import (
"errors"
"fmt"
)
var (
// ErrNilRemoteHub is returned when the remote hub configuration is not provided to the NewHub constructor.
// All attempts to download index or items will return this error.
ErrNilRemoteHub = errors.New("remote hub configuration is not provided. Please report this issue to the developers")
ErrIndexNotFound = errors.New("index not found")
)
type IndexNotFoundError struct {
URL string
Branch string
}
func (e IndexNotFoundError) Error() string {
return fmt.Sprintf("index not found at %s, branch '%s'. Please check the .cscli.hub_branch value if you specified it in config.yaml, or use 'master' if not sure", e.URL, e.Branch)
}

View file

@ -57,7 +57,7 @@ func NewHub(local *csconfig.LocalHubCfg, remote *RemoteHubCfg, downloadIndex boo
}
if _, err := theHub.LocalSync(); err != nil {
return nil, fmt.Errorf("failed to sync hub index: %w", err)
return nil, fmt.Errorf("failed to sync items: %w", err)
}
return theHub, nil

View file

@ -4,8 +4,8 @@ import (
"encoding/json"
"fmt"
"github.com/Masterminds/semver/v3"
"github.com/enescakir/emoji"
"golang.org/x/mod/semver"
)
const (
@ -16,6 +16,13 @@ const (
SCENARIOS = "scenarios"
)
const (
VersionUpToDate = iota
VersionUpdateAvailable
VersionUnknown
VersionFuture
)
// The order is important, as it is used to range over sub-items in collections
var ItemTypes = []string{PARSERS, POSTOVERFLOWS, SCENARIOS, COLLECTIONS}
@ -178,7 +185,23 @@ func (i *Item) Status() (string, emoji.Emoji) {
// versionStatus: semver requires 'v' prefix
func (i *Item) versionStatus() int {
return semver.Compare("v"+i.Version, "v"+i.LocalVersion)
local, err := semver.NewVersion(i.LocalVersion)
if err != nil {
return VersionUnknown
}
// hub versions are already validated while syncing, ignore errors
latest, _ := semver.NewVersion(i.Version)
if local.LessThan(latest) {
return VersionUpdateAvailable
}
if local.Equal(latest) {
return VersionUpToDate
}
return VersionFuture
}
// validPath returns true if the (relative) path is allowed for the item

View file

@ -38,23 +38,16 @@ func itemKey(itemPath string) (string, error) {
}
// GetItemByPath retrieves the item from the hub index based on its path.
// To achieve this it resolves a symlink to find the associated hub item.
func (h *Hub) GetItemByPath(itemType string, itemPath string) (*Item, error) {
itemKey, err := itemKey(itemPath)
if err != nil {
return nil, err
}
// XXX: use GetItem()
m := h.GetItemMap(itemType)
if m == nil {
return nil, fmt.Errorf("item type %s doesn't exist", itemType)
}
v, ok := m[itemKey]
if !ok {
item := h.GetItem(itemType, itemKey)
if item == nil {
return nil, fmt.Errorf("%s not found in %s", itemKey, itemType)
}
return &v, nil
return item, nil
}

View file

@ -53,7 +53,7 @@ func (r *RemoteHubCfg) downloadIndex(localPath string) error {
if resp.StatusCode != http.StatusOK {
if resp.StatusCode == http.StatusNotFound {
return ErrIndexNotFound
return IndexNotFoundError{req.URL.String(), r.Branch}
}
return fmt.Errorf("bad http code %d for %s", resp.StatusCode, req.URL.String())

View file

@ -10,6 +10,7 @@ import (
"sort"
"strings"
"github.com/Masterminds/semver/v3"
log "github.com/sirupsen/logrus"
"slices"
)
@ -32,7 +33,7 @@ func handleSymlink(path string) (string, error) {
return "", fmt.Errorf("failed to unlink %s: %w", path, err)
}
// XXX: is this correct?
// ignore this file
return "", nil
}
@ -125,6 +126,28 @@ func (h *Hub) getItemInfo(path string) (itemFileInfo, bool, error) {
return ret, inhub, nil
}
// sortedVersions returns the input data, sorted in reverse order by semver
func sortedVersions(raw []string) ([]string, error) {
vs := make([]*semver.Version, len(raw))
for i, r := range raw {
v, err := semver.NewVersion(r)
if err != nil {
return nil, fmt.Errorf("%s: %w", r, err)
}
vs[i] = v
}
sort.Sort(sort.Reverse(semver.Collection(vs)))
ret := make([]string, len(vs))
for i, v := range vs {
ret[i] = v.Original()
}
return ret, nil
}
func (h *Hub) itemVisit(path string, f os.DirEntry, err error) error {
var (
local bool
@ -174,7 +197,7 @@ func (h *Hub) itemVisit(path string, f os.DirEntry, err error) error {
log.Tracef("%s points to %s", path, hubpath)
if hubpath == "" {
// XXX: is this correct?
// ignore this file
return nil
}
}
@ -246,13 +269,15 @@ func (h *Hub) itemVisit(path string, f os.DirEntry, err error) error {
}
// let's reverse sort the versions to deal with hash collisions (#154)
// XXX: we sure, lexical sorting?
versions := make([]string, 0, len(item.Versions))
for k := range item.Versions {
versions = append(versions, k)
}
sort.Sort(sort.Reverse(sort.StringSlice(versions)))
versions, err = sortedVersions(versions)
if err != nil {
return fmt.Errorf("while syncing %s %s: %w", info.ftype, info.fname, err)
}
for _, version := range versions {
if item.Versions[version].Digest != sha {
@ -315,7 +340,7 @@ func (h *Hub) CollectDepsCheck(v *Item) error {
return nil
}
if v.versionStatus() != 0 { // not up-to-date
if v.versionStatus() != VersionUpToDate { // not up-to-date
log.Debugf("%s dependencies not checked: not up-to-date", v.Name)
return nil
}
@ -405,15 +430,17 @@ func (h *Hub) SyncDir(dir string) ([]string, error) {
vs := item.versionStatus()
switch vs {
case 0: // latest
case VersionUpToDate: // latest
if err := h.CollectDepsCheck(&item); err != nil {
warnings = append(warnings, fmt.Sprintf("dependency of %s: %s", item.Name, err))
h.Items[COLLECTIONS][name] = item
}
case 1: // not up-to-date
case VersionUpdateAvailable: // not up-to-date
warnings = append(warnings, fmt.Sprintf("update for collection %s available (currently:%s, latest:%s)", item.Name, item.LocalVersion, item.Version))
default: // version is higher than the highest available from hub?
case VersionFuture:
warnings = append(warnings, fmt.Sprintf("collection %s is in the future (currently:%s, latest:%s)", item.Name, item.LocalVersion, item.Version))
case VersionUnknown:
warnings = append(warnings, fmt.Sprintf("collection %s is tainted (latest:%s)", item.Name, item.Version))
}
log.Debugf("installed (%s) - status: %d | installed: %s | latest: %s | full: %+v", item.Name, vs, item.LocalVersion, item.Version, item.Versions)

View file

@ -79,26 +79,35 @@ teardown() {
# XXX: check alphabetical order in human, json, raw
}
@test "cscli collections list [collection]..." {
# non-existent
rune -1 cscli collections install foo/bar
assert_stderr --partial "can't find 'foo/bar' in collections"
# not installed
rune -0 cscli collections list crowdsecurity/smb
assert_output --regexp 'crowdsecurity/smb.*disabled'
# install two items
rune -0 cscli collections install crowdsecurity/sshd crowdsecurity/smb
# list one item
# list an installed item
rune -0 cscli collections list crowdsecurity/sshd
assert_output --partial "crowdsecurity/sshd"
assert_output --regexp "crowdsecurity/sshd"
refute_output --partial "crowdsecurity/smb"
# list multiple items
rune -0 cscli collections list crowdsecurity/sshd crowdsecurity/smb
# list multiple installed and non installed items
rune -0 cscli collections list crowdsecurity/sshd crowdsecurity/smb crowdsecurity/nginx
assert_output --partial "crowdsecurity/sshd"
assert_output --partial "crowdsecurity/smb"
assert_output --partial "crowdsecurity/nginx"
rune -0 cscli collections list crowdsecurity/sshd -o json
rune -0 jq '.collections | length' <(output)
assert_output "1"
rune -0 cscli collections list crowdsecurity/sshd crowdsecurity/smb -o json
rune -0 cscli collections list crowdsecurity/sshd crowdsecurity/smb crowdsecurity/nginx -o json
rune -0 jq '.collections | length' <(output)
assert_output "2"
assert_output "3"
rune -0 cscli collections list crowdsecurity/sshd -o raw
rune -0 grep -vc 'name,status,version,description' <(output)
@ -108,14 +117,6 @@ teardown() {
assert_output "2"
}
@test "cscli collections list [collection]... (not installed / not existing)" {
skip "not implemented yet"
# not installed
rune -1 cscli collections list crowdsecurity/sshd
# not existing
rune -1 cscli collections list blahblah/blahblah
}
@test "cscli collections install [collection]..." {
rune -1 cscli collections install
assert_stderr --partial 'requires at least 1 arg(s), only received 0'

View file

@ -0,0 +1,72 @@
#!/usr/bin/env bats
# vim: ft=bats:list:ts=8:sts=4:sw=4:et:ai:si:
set -u
setup_file() {
load "../lib/setup_file.sh"
./instance-data load
HUB_DIR=$(config_get '.config_paths.hub_dir')
export HUB_DIR
CONFIG_DIR=$(config_get '.config_paths.config_dir')
export CONFIG_DIR
}
teardown_file() {
load "../lib/teardown_file.sh"
}
setup() {
load "../lib/setup.sh"
load "../lib/bats-file/load.bash"
./instance-data load
hub_purge_all
hub_strip_index
}
teardown() {
./instance-crowdsec stop
}
#----------
#
# Tests that don't need to be repeated for each hub type
#
@test "hub versions are correctly sorted during sync" {
# hash of an empty file
sha256_empty="e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
# add two versions with the same hash, that don't sort the same way
# in a lexical vs semver sort. CrowdSec should report the latest version
new_hub=$( \
jq --arg DIGEST "$sha256_empty" <"$HUB_DIR/.index.json" \
'. * {collections:{"crowdsecurity/sshd":{"versions":{"1.2":{"digest":$DIGEST, "deprecated": false}, "1.10": {"digest":$DIGEST, "deprecated": false}}}}}' \
)
echo "$new_hub" >"$HUB_DIR/.index.json"
rune -0 cscli collections install crowdsecurity/sshd
truncate -s 0 "$CONFIG_DIR/collections/sshd.yaml"
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]'
}
@test "hub index with invalid (non semver) version numbers" {
new_hub=$( \
jq <"$HUB_DIR/.index.json" \
'. * {collections:{"crowdsecurity/sshd":{"versions":{"1.2.3.4":{"digest":"foo", "deprecated": false}}}}}' \
)
echo "$new_hub" >"$HUB_DIR/.index.json"
rune -0 cscli collections install crowdsecurity/sshd
rune -1 cscli collections inspect crowdsecurity/sshd --no-metrics
# XXX: we are on the verbose side here...
assert_stderr --partial "failed to read Hub index: failed to sync items: failed to scan $CONFIG_DIR: while syncing collections sshd.yaml: 1.2.3.4: Invalid Semantic Version"
}

View file

@ -79,41 +79,43 @@ teardown() {
# XXX: check alphabetical order in human, json, raw
}
@test "cscli postoverflows list [scenario]..." {
# non-existent
rune -1 cscli postoverflows install foo/bar
assert_stderr --partial "can't find 'foo/bar' in postoverflows"
# not installed
rune -0 cscli postoverflows list crowdsecurity/rdns
assert_output --regexp 'crowdsecurity/rdns.*disabled'
# install two items
rune -0 cscli postoverflows install crowdsecurity/rdns crowdsecurity/cdn-whitelist
# list one item
# list an installed item
rune -0 cscli postoverflows list crowdsecurity/rdns
assert_output --partial "crowdsecurity/rdns"
assert_output --regexp "crowdsecurity/rdns.*enabled"
refute_output --partial "crowdsecurity/cdn-whitelist"
# list multiple items
rune -0 cscli postoverflows list crowdsecurity/rdns crowdsecurity/cdn-whitelist
# list multiple installed and non installed items
rune -0 cscli postoverflows list crowdsecurity/rdns crowdsecurity/cdn-whitelist crowdsecurity/ipv6_to_range
assert_output --partial "crowdsecurity/rdns"
assert_output --partial "crowdsecurity/cdn-whitelist"
assert_output --partial "crowdsecurity/ipv6_to_range"
rune -0 cscli postoverflows list crowdsecurity/rdns -o json
rune -0 jq '.postoverflows | length' <(output)
assert_output "1"
rune -0 cscli postoverflows list crowdsecurity/rdns crowdsecurity/cdn-whitelist -o json
rune -0 cscli postoverflows list crowdsecurity/rdns crowdsecurity/cdn-whitelist crowdsecurity/ipv6_to_range -o json
rune -0 jq '.postoverflows | length' <(output)
assert_output "2"
assert_output "3"
rune -0 cscli postoverflows list crowdsecurity/rdns -o raw
rune -0 grep -vc 'name,status,version,description' <(output)
assert_output "1"
rune -0 cscli postoverflows list crowdsecurity/rdns crowdsecurity/cdn-whitelist -o raw
rune -0 cscli postoverflows list crowdsecurity/rdns crowdsecurity/cdn-whitelist crowdsecurity/ipv6_to_range -o raw
rune -0 grep -vc 'name,status,version,description' <(output)
assert_output "2"
}
@test "cscli postoverflows list [scenario]... (not installed / not existing)" {
skip "not implemented yet"
# not installed
rune -1 cscli postoverflows list crowdsecurity/rdns
# not existing
rune -1 cscli postoverflows list blahblah/blahblah
assert_output "3"
}
@test "cscli postoverflows install [scenario]..." {
@ -157,6 +159,8 @@ teardown() {
assert_file_exists "$CONFIG_DIR/postoverflows/s00-enrich/rdns.yaml"
}
# XXX: test install with --force
# XXX: test install with --ignore
@test "cscli postoverflows inspect [scenario]..." {
rune -1 cscli postoverflows inspect

View file

@ -145,7 +145,6 @@ teardown() {
assert_output --partial 'installed: true'
}
@test "cscli scenarios install [scenario]... (file location and download-only)" {
# simple install
rune -0 cscli scenarios install crowdsecurity/ssh-bf --download-only
@ -159,6 +158,9 @@ teardown() {
assert_file_exists "$CONFIG_DIR/scenarios/ssh-bf.yaml"
}
# XXX: test install with --force
# XXX: test install with --ignore
@test "cscli scenarios inspect [scenario]..." {
rune -1 cscli scenarios inspect