Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pkg/cwhub: improve error messages #2712

Merged
merged 3 commits into from
Jan 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions cmd/crowdsec-cli/hub.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ func (cli cliHub) upgrade(cmd *cobra.Command, args []string) error {
if err != nil {
return err
}

if didUpdate {
updated++
}
Expand Down Expand Up @@ -191,18 +192,21 @@ func (cli cliHub) types(cmd *cobra.Command, args []string) error {
if err != nil {
return err
}

fmt.Print(string(s))
case "json":
jsonStr, err := json.Marshal(cwhub.ItemTypes)
if err != nil {
return err
}

fmt.Println(string(jsonStr))
case "raw":
for _, itemType := range cwhub.ItemTypes {
fmt.Println(itemType)
}
}

return nil
}

Expand Down
5 changes: 4 additions & 1 deletion cmd/crowdsec-cli/hubappsec.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,11 @@

func NewCLIAppsecRule() *cliItem {
inspectDetail := func(item *cwhub.Item) error {
//Only show the converted rules in human mode
// Only show the converted rules in human mode

Check warning on line 51 in cmd/crowdsec-cli/hubappsec.go

View check run for this annotation

Codecov / codecov/patch

cmd/crowdsec-cli/hubappsec.go#L51

Added line #L51 was not covered by tests
if csConfig.Cscli.Output != "human" {
return nil
}

appsecRule := appsec.AppsecCollectionConfig{}

yamlContent, err := os.ReadFile(item.State.LocalPath)
Expand All @@ -65,11 +66,13 @@

for _, ruleType := range appsec_rule.SupportedTypes() {
fmt.Printf("\n%s format:\n", cases.Title(language.Und, cases.NoLower).String(ruleType))

Check warning on line 69 in cmd/crowdsec-cli/hubappsec.go

View check run for this annotation

Codecov / codecov/patch

cmd/crowdsec-cli/hubappsec.go#L69

Added line #L69 was not covered by tests
for _, rule := range appsecRule.Rules {
convertedRule, _, err := rule.Convert(ruleType, appsecRule.Name)
if err != nil {
return fmt.Errorf("unable to convert rule %s : %s", rule.Name, err)
}

fmt.Println(convertedRule)
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/crowdsec-cli/item_suggest.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ package main

import (
"fmt"
"slices"
"strings"

"github.com/agext/levenshtein"
"github.com/spf13/cobra"
"slices"

"github.com/crowdsecurity/crowdsec/cmd/crowdsec-cli/require"
"github.com/crowdsecurity/crowdsec/pkg/cwhub"
Expand Down
2 changes: 1 addition & 1 deletion cmd/crowdsec-cli/items.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import (
"io"
"os"
"path/filepath"
"slices"
"strings"

"gopkg.in/yaml.v3"
"slices"

"github.com/crowdsecurity/crowdsec/pkg/cwhub"
)
Expand Down
3 changes: 3 additions & 0 deletions pkg/cwhub/dataset.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ func downloadFile(url string, destPath string) error {
// if the remote has no modification date, but local file has been modified > a week ago, update.
func needsUpdate(destPath string, url string, logger *logrus.Logger) bool {
fileInfo, err := os.Stat(destPath)

switch {
case os.IsNotExist(err):
return true
Expand Down Expand Up @@ -89,6 +90,7 @@ func needsUpdate(destPath string, url string, logger *logrus.Logger) bool {
if localIsOld {
logger.Infof("no last modified date for %s, but local file is older than %s", url, shelfLife)
}

return localIsOld
}

Expand Down Expand Up @@ -129,6 +131,7 @@ func downloadDataSet(dataFolder string, force bool, reader io.Reader, logger *lo

if force || needsUpdate(destPath, dataS.SourceURL, logger) {
logger.Debugf("downloading %s in %s", dataS.SourceURL, destPath)

if err := downloadFile(dataS.SourceURL, destPath); err != nil {
return fmt.Errorf("while getting data: %w", err)
}
Expand Down
6 changes: 5 additions & 1 deletion pkg/cwhub/hub.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import (
"io"
"os"
"path"
"slices"
"strings"

"github.com/sirupsen/logrus"
"slices"

"github.com/crowdsecurity/crowdsec/pkg/csconfig"
)
Expand Down Expand Up @@ -97,6 +97,10 @@ func (h *Hub) parseIndex() error {
item.FileName = path.Base(item.RemotePath)

item.logMissingSubItems()

if item.latestHash() == "" {
h.logger.Errorf("invalid hub item %s: latest version missing from index", item.FQName())
}
}
}

Expand Down
1 change: 0 additions & 1 deletion pkg/cwhub/hub_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (

func TestInitHubUpdate(t *testing.T) {
hub := envSetup(t)

remote := &RemoteHubCfg{
URLTemplate: mockURLTemplate,
Branch: "master",
Expand Down
14 changes: 13 additions & 1 deletion pkg/cwhub/item.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import (
"encoding/json"
"fmt"
"path/filepath"
"slices"

"github.com/Masterminds/semver/v3"
"github.com/enescakir/emoji"
"slices"
)

const (
Expand Down Expand Up @@ -440,3 +440,15 @@ func (i *Item) addTaint(sub *Item) {
ancestor.addTaint(sub)
}
}

// latestHash() returns the hash of the latest version of the item.
// if it's missing, the index file has been manually modified or got corrupted.
func (i *Item) latestHash() string {
for k, v := range i.Versions {
if k == i.Version {
return v.Digest
}
}

return ""
}
2 changes: 1 addition & 1 deletion pkg/cwhub/iteminstall.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (i *Item) Install(force bool, downloadOnly bool) error {

filePath, err := i.downloadLatest(force, true)
if err != nil {
return fmt.Errorf("while downloading %s: %w", i.Name, err)
return err
}

if downloadOnly {
Expand Down
1 change: 0 additions & 1 deletion pkg/cwhub/itemremove.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package cwhub
import (
"fmt"
"os"

"slices"
)

Expand Down
20 changes: 15 additions & 5 deletions pkg/cwhub/itemupgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"bytes"
"crypto/sha256"
"encoding/hex"
"errors"
"fmt"
"io"
"net/http"
Expand Down Expand Up @@ -82,14 +83,14 @@
i.hub.logger.Tracef("collection, recurse")

if _, err := sub.downloadLatest(overwrite, updateOnly); err != nil {
return "", fmt.Errorf("while downloading %s: %w", sub.Name, err)
return "", err

Check warning on line 86 in pkg/cwhub/itemupgrade.go

View check run for this annotation

Codecov / codecov/patch

pkg/cwhub/itemupgrade.go#L86

Added line #L86 was not covered by tests
}
}

downloaded := sub.State.Downloaded

if _, err := sub.download(overwrite); err != nil {
return "", fmt.Errorf("while downloading %s: %w", sub.Name, err)
return "", err

Check warning on line 93 in pkg/cwhub/itemupgrade.go

View check run for this annotation

Codecov / codecov/patch

pkg/cwhub/itemupgrade.go#L93

Added line #L93 was not covered by tests
}

// We need to enable an item when it has been added to a collection since latest release of the collection.
Expand All @@ -108,14 +109,18 @@

ret, err := i.download(overwrite)
if err != nil {
return "", fmt.Errorf("failed to download item: %w", err)
return "", err
}

return ret, nil
}

// FetchLatest downloads the latest item from the hub, verifies the hash and returns the content and the used url.
func (i *Item) FetchLatest() ([]byte, string, error) {
if i.latestHash() == "" {
return nil, "", errors.New("latest hash missing from index")
}

url, err := i.hub.remote.urlTo(i.RemotePath)
if err != nil {
return nil, "", fmt.Errorf("failed to build request: %w", err)
Expand Down Expand Up @@ -146,7 +151,7 @@
i.hub.logger.Errorf("Downloaded version doesn't match index, please 'hub update'")
i.hub.logger.Debugf("got %s, expected %s", meow, i.Versions[i.Version].Digest)

return nil, "", fmt.Errorf("invalid download hash for %s", i.Name)
return nil, "", fmt.Errorf("invalid download hash")

Check warning on line 154 in pkg/cwhub/itemupgrade.go

View check run for this annotation

Codecov / codecov/patch

pkg/cwhub/itemupgrade.go#L154

Added line #L154 was not covered by tests
}

return body, url, nil
Expand Down Expand Up @@ -180,7 +185,12 @@

body, url, err := i.FetchLatest()
if err != nil {
return "", fmt.Errorf("while downloading %s: %w", url, err)
what := i.Name
if url != "" {
what += " from " + url
}

Check warning on line 191 in pkg/cwhub/itemupgrade.go

View check run for this annotation

Codecov / codecov/patch

pkg/cwhub/itemupgrade.go#L190-L191

Added lines #L190 - L191 were not covered by tests

return "", fmt.Errorf("while downloading %s: %w", what, err)
}

// all good, install
Expand Down
3 changes: 0 additions & 3 deletions pkg/cwhub/itemupgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
// We expect the new scenario to be installed.
func TestUpgradeItemNewScenarioInCollection(t *testing.T) {
hub := envSetup(t)

item := hub.GetItem(COLLECTIONS, "crowdsecurity/test_collection")

// fresh install of collection
Expand Down Expand Up @@ -65,7 +64,6 @@ func TestUpgradeItemNewScenarioInCollection(t *testing.T) {
// Upgrade should install should not enable/download the disabled scenario.
func TestUpgradeItemInDisabledScenarioShouldNotBeInstalled(t *testing.T) {
hub := envSetup(t)

item := hub.GetItem(COLLECTIONS, "crowdsecurity/test_collection")

// fresh install of collection
Expand Down Expand Up @@ -127,7 +125,6 @@ func getHubOrFail(t *testing.T, local *csconfig.LocalHubCfg, remote *RemoteHubCf
// Upgrade should install and enable the newly added scenario.
func TestUpgradeItemNewScenarioIsInstalledWhenReferencedScenarioIsDisabled(t *testing.T) {
hub := envSetup(t)

item := hub.GetItem(COLLECTIONS, "crowdsecurity/test_collection")

// fresh install of collection
Expand Down
2 changes: 1 addition & 1 deletion pkg/cwhub/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ import (
"io"
"os"
"path/filepath"
"slices"
"sort"
"strings"

"github.com/Masterminds/semver/v3"
"github.com/sirupsen/logrus"
"gopkg.in/yaml.v3"
"slices"
)

func isYAMLFileName(path string) bool {
Expand Down
10 changes: 10 additions & 0 deletions test/bats/20_hub.bats
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,16 @@ teardown() {
assert_output --partial 'crowdsecurity/iptables'
}

@test "cscli hub list (invalid index)" {
new_hub=$(jq <"$INDEX_PATH" '."appsec-rules"."crowdsecurity/vpatch-laravel-debug-mode".version="999"')
echo "$new_hub" >"$INDEX_PATH"
rune -0 cscli hub list --error
assert_stderr --partial "invalid hub item appsec-rules:crowdsecurity/vpatch-laravel-debug-mode: latest version missing from index"

rune -1 cscli appsec-rules install crowdsecurity/vpatch-laravel-debug-mode --force
assert_stderr --partial "error while installing 'crowdsecurity/vpatch-laravel-debug-mode': while downloading crowdsecurity/vpatch-laravel-debug-mode: latest hash missing from index"
}

@test "missing reference in hub index" {
new_hub=$(jq <"$INDEX_PATH" 'del(.parsers."crowdsecurity/smb-logs") | del (.scenarios."crowdsecurity/mysql-bf")')
echo "$new_hub" >"$INDEX_PATH"
Expand Down