From 55aa1893d173ea40016cb189e0432b5f5bde7bfc Mon Sep 17 00:00:00 2001
From: mmetc <92726601+mmetc@users.noreply.github.com>
Date: Tue, 25 Mar 2025 11:53:49 +0100
Subject: [PATCH] explicit message for malformed data URL in local items
 (#3537)

---
 debian/migrate-hub.sh       |  4 ++--
 pkg/hubops/colorize.go      |  2 ++
 pkg/hubops/download.go      |  8 ++++++++
 test/bats/20_hub_items.bats | 23 +++++++++++++++++++++++
 4 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/debian/migrate-hub.sh b/debian/migrate-hub.sh
index 877b6f006..fb78b34a1 100755
--- a/debian/migrate-hub.sh
+++ b/debian/migrate-hub.sh
@@ -1,7 +1,7 @@
 #!/usr/bin/env sh
 
 # This script is provided (only in the source distribution) as an ad-hoc solution
-# to migrate an installation from the crowdsec package maintained in the debian repositories
+# to migrate an installation from the crowdsec package maintained in the debian or ubuntu repositories
 # to the official crowdsec repository.
 
 set -eu
@@ -9,7 +9,7 @@ set -eu
 if [ ! -d /var/lib/crowdsec/hub/ ]; then
     echo "You don't have a hub directory to migrate."
     echo
-    echo "Use this script only if you upgrade from the crowdsec package included in the debian repositories."
+    echo "Use this script only if you upgrade from the crowdsec package included in the debian or ubuntu repositories."
     exit 1
 fi
 
diff --git a/pkg/hubops/colorize.go b/pkg/hubops/colorize.go
index 3af2aecab..b988d1e94 100644
--- a/pkg/hubops/colorize.go
+++ b/pkg/hubops/colorize.go
@@ -15,8 +15,10 @@ func colorizeItemName(fullname string) string {
 		bold := color.New(color.Bold)
 		author := parts[0]
 		name := parts[1]
+
 		return author + "/" + bold.Sprint(name)
 	}
+
 	return fullname
 }
 
diff --git a/pkg/hubops/download.go b/pkg/hubops/download.go
index fab514fcd..68b3213c6 100644
--- a/pkg/hubops/download.go
+++ b/pkg/hubops/download.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"net/url"
 	"os"
 	"time"
 
@@ -118,6 +119,13 @@ func downloadDataSet(ctx context.Context, dataFolder string, force bool, reader
 				continue
 			}
 
+			// twopenny validation
+			if u, err := url.Parse(dataS.SourceURL); err != nil {
+				return false, err
+			} else if u.Scheme == "" {
+				return false, fmt.Errorf("a valid URL was expected (note: local items can download data too): %s", dataS.SourceURL)
+			}
+
 			// XXX: check context cancellation
 			destPath, err := cwhub.SafePath(dataFolder, dataS.DestPath)
 			if err != nil {
diff --git a/test/bats/20_hub_items.bats b/test/bats/20_hub_items.bats
index b3f929e19..4bf7a2544 100644
--- a/test/bats/20_hub_items.bats
+++ b/test/bats/20_hub_items.bats
@@ -151,6 +151,29 @@ teardown() {
     assert_output --partial "Nothing to do."
 }
 
+@test "when upgrading the hub, a local item's data will be downloaded" {
+    rune -0 mkdir -p "$CONFIG_DIR/collections"
+    cat >"$CONFIG_DIR"/collections/foobar.yaml <<-EOT
+        data:
+          - source_url: https://localhost:1234/database.mmdb
+            dest_file: database.mmdb
+	EOT
+    rune -1 cscli hub upgrade
+    assert_line "downloading https://localhost:1234/database.mmdb"
+    assert_stderr --partial 'Get "https://localhost:1234/database.mmdb":'
+    assert_stderr --partial 'connect: connection refused'
+
+    # bad link, or local path
+    cat >"$CONFIG_DIR"/collections/foobar.yaml <<-EOT
+        data:
+          - source_url: /tmp/meh
+            dest_file: database.mmdb
+	EOT
+    rune -1 cscli hub upgrade
+    refute_line "downloading /tmp/meh"
+    assert_stderr --partial 'a valid URL was expected (note: local items can download data too): /tmp/meh'
+}
+
 @test "a local item cannot be removed by cscli" {
     rune -0 mkdir -p "$CONFIG_DIR/scenarios"
     rune -0 touch "$CONFIG_DIR/scenarios/foobar.yaml"