From 7e8433d0ea470dabf15571878226ccc347db826d Mon Sep 17 00:00:00 2001
From: Katrina Rogan <katroganGH@gmail.com>
Date: Mon, 13 Mar 2023 09:21:15 -0700
Subject: [PATCH] Validate user-provided entity versions are url safe (#537)

---
 flyteadmin/pkg/manager/impl/validation/validation.go     | 8 ++++++++
 .../pkg/manager/impl/validation/validation_test.go       | 9 +++++++++
 2 files changed, 17 insertions(+)

diff --git a/flyteadmin/pkg/manager/impl/validation/validation.go b/flyteadmin/pkg/manager/impl/validation/validation.go
index 60afa58235..e6c7cfae2f 100644
--- a/flyteadmin/pkg/manager/impl/validation/validation.go
+++ b/flyteadmin/pkg/manager/impl/validation/validation.go
@@ -1,6 +1,7 @@
 package validation
 
 import (
+	"net/url"
 	"strconv"
 	"strings"
 
@@ -22,6 +23,9 @@ var entityToResourceType = map[common.Entity]core.ResourceType{
 	common.LaunchPlan: core.ResourceType_LAUNCH_PLAN,
 }
 
+// See https://www.rfc-editor.org/rfc/rfc3986#section-2.2
+var uriReservedChars = "!*'();:@&=+$,/?#[]"
+
 func ValidateEmptyStringField(field, fieldName string) error {
 	if field == "" {
 		return shared.GetMissingArgumentError(fieldName)
@@ -132,6 +136,10 @@ func ValidateVersion(version string) error {
 	if err := ValidateEmptyStringField(version, shared.Version); err != nil {
 		return err
 	}
+	sanitizedVersion := url.QueryEscape(version)
+	if !strings.EqualFold(sanitizedVersion, version) {
+		return errors.NewFlyteAdminErrorf(codes.InvalidArgument, "version [%s] must be url safe, cannot contains chars [%s]", version, uriReservedChars)
+	}
 	return nil
 }
 
diff --git a/flyteadmin/pkg/manager/impl/validation/validation_test.go b/flyteadmin/pkg/manager/impl/validation/validation_test.go
index 6707e7f781..80830ce033 100644
--- a/flyteadmin/pkg/manager/impl/validation/validation_test.go
+++ b/flyteadmin/pkg/manager/impl/validation/validation_test.go
@@ -1,6 +1,7 @@
 package validation
 
 import (
+	"fmt"
 	"testing"
 	"time"
 
@@ -174,6 +175,14 @@ func TestValidateDescriptionEntityListRequest(t *testing.T) {
 func TestValidateVersion(t *testing.T) {
 	err := ValidateVersion("")
 	assert.EqualError(t, err, "missing version")
+
+	t.Run("url safe versions only", func(t *testing.T) {
+		assert.NoError(t, ValidateVersion("Foo123"))
+		for _, reservedChar := range uriReservedChars {
+			invalidVersion := fmt.Sprintf("foo%c", reservedChar)
+			assert.NotNil(t, ValidateVersion(invalidVersion))
+		}
+	})
 }
 
 func TestValidateListTaskRequest(t *testing.T) {