From 16e6acab24281b8e85bcec8e057eb1eb4ed054a5 Mon Sep 17 00:00:00 2001
From: Anders Eknert <anders@styra.com>
Date: Wed, 16 Oct 2024 13:45:51 +0200
Subject: [PATCH] Improve error messages for incorrect capabilities version

As reported in #1202 (thanks @geirs73!)

Note however that this does not address the Windows path loading
issue from the same issue though.

Signed-off-by: Anders Eknert <anders@styra.com>
---
 pkg/config/config.go      | 15 ++++++++++++---
 pkg/config/config_test.go | 30 ++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/pkg/config/config.go b/pkg/config/config.go
index 68e01e22..a59614c6 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -340,7 +340,7 @@ type marshallingIntermediary struct {
 	Capabilities struct {
 		From struct {
 			Engine  string `yaml:"engine"`
-			Version string `yaml:"version"`
+			Version any    `yaml:"version"`
 			File    string `yaml:"file"`
 			URL     string `yaml:"url"`
 		} `yaml:"from"`
@@ -368,7 +368,7 @@ func (config *Config) UnmarshalYAML(value *yaml.Node) error {
 		return fmt.Errorf("unmarshalling config failed %w", err)
 	}
 
-	// this call will walk the rule config and load and defaults into the config
+	// this call will walk the rule config and load defaults into the config
 	if err := extractDefaults(config, &result); err != nil {
 		return fmt.Errorf("extracting defaults failed: %w", err)
 	}
@@ -411,7 +411,16 @@ func (config *Config) UnmarshalYAML(value *yaml.Node) error {
 	}
 
 	if capabilitiesEngine != "" {
-		capabilitiesURL = "regal:///capabilities/" + capabilitiesEngine + "/" + capabilitiesEngineVersion
+		version, ok := capabilitiesEngineVersion.(string)
+		if !ok {
+			return errors.New("capabilities: from.version must be a string")
+		}
+
+		if capabilitiesEngine == capabilitiesEngineOPA && !strings.HasPrefix(version, "v") {
+			return errors.New("capabilities: from.version must be a valid OPA version (with a 'v' prefix)")
+		}
+
+		capabilitiesURL = "regal:///capabilities/" + capabilitiesEngine + "/" + version
 	}
 
 	if capabilitiesFile != "" {
diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go
index 1dd16eb6..20e775bb 100644
--- a/pkg/config/config_test.go
+++ b/pkg/config/config_test.go
@@ -394,3 +394,33 @@ func TestUnmarshalConfigDefaultCapabilities(t *testing.T) {
 		}
 	}
 }
+
+func TestUnmarshalConfigWithNumericOPAVersion(t *testing.T) {
+	t.Parallel()
+
+	bs := []byte(`
+capabilities:
+  from:
+    engine: opa
+    version: 68
+`)
+	if err := yaml.Unmarshal(bs, &Config{}); err == nil ||
+		err.Error() != "capabilities: from.version must be a string" {
+		t.Errorf("expected error, got %v", err)
+	}
+}
+
+func TestUnmarshalConfigWithMissingVPrefixOPAVersion(t *testing.T) {
+	t.Parallel()
+
+	bs := []byte(`
+capabilities:
+  from:
+    engine: opa
+    version: 0.68.0
+`)
+	if err := yaml.Unmarshal(bs, &Config{}); err == nil ||
+		err.Error() != "capabilities: from.version must be a valid OPA version (with a 'v' prefix)" {
+		t.Errorf("expected error, got %v", err)
+	}
+}