From 4c3488eb4809ae2acefebb0a7809c94f04a4c58c Mon Sep 17 00:00:00 2001
From: Alex Clemmer <clemmer.alexander@gmail.com>
Date: Thu, 21 Sep 2017 16:17:27 -0700
Subject: [PATCH] Harden `metadata.isValidName`

Currently `metadata.isValidName` will admit names that should be invalid
(e.g., names with spaces, leading '/' characters) and not admit names
that should be valid (e.g., names with '.' characters inside).

This commit moves this function into `metadata/interface.go` and hardens
it against these constraints.
---
 metadata/environment.go | 12 ++----------
 metadata/interface.go   | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/metadata/environment.go b/metadata/environment.go
index 8d66ea8e..8e23beb1 100644
--- a/metadata/environment.go
+++ b/metadata/environment.go
@@ -20,7 +20,6 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"regexp"
 	"strings"
 
 	log "github.com/sirupsen/logrus"
@@ -77,7 +76,7 @@ func (m *manager) createEnvironment(name, uri string, extensionsLibData, k8sLibD
 
 	// ensure environment name does not contain punctuation
 	if !isValidName(name) {
-		return fmt.Errorf("Environment '%s' is not valid; must not contain punctuation or trailing slashes", name)
+		return fmt.Errorf("Environment name '%s' is not valid; must not contain punctuation, spaces, or begin or end with a slash", name)
 	}
 
 	log.Infof("Creating environment '%s' with uri '%s'", name, uri)
@@ -249,7 +248,7 @@ func (m *manager) SetEnvironment(name string, desired *Environment) error {
 
 	// ensure new environment name does not contain punctuation
 	if !isValidName(desired.Name) {
-		return fmt.Errorf("Environment '%s' is not valid; must not contain punctuation or trailing slashes", desired.Name)
+		return fmt.Errorf("Environment name '%s' is not valid; must not contain punctuation, spaces, or begin or end with a slash", name)
 	}
 
 	// If the name has changed, the directory location needs to be moved to
@@ -349,10 +348,3 @@ func (m *manager) environmentExists(name string) (bool, error) {
 
 	return envExists, nil
 }
-
-// regex matcher to ensure environment name does not contain punctuation
-func isValidName(envName string) bool {
-	hasPunctuation := regexp.MustCompile(`[,;.':!()?"{}\[\]*&%@$]+`).MatchString
-	hasTrailingSlashes := regexp.MustCompile(`/+$`).MatchString
-	return !hasPunctuation(envName) && !hasTrailingSlashes(envName)
-}
diff --git a/metadata/interface.go b/metadata/interface.go
index c59c9b77..4d13ed72 100644
--- a/metadata/interface.go
+++ b/metadata/interface.go
@@ -17,6 +17,8 @@ package metadata
 
 import (
 	"os"
+	"regexp"
+	"strings"
 
 	"github.com/spf13/afero"
 )
@@ -85,6 +87,23 @@ func ParseClusterSpec(specFlag string) (ClusterSpec, error) {
 	return parseClusterSpec(specFlag, appFS)
 }
 
+// isValidName returns true if a name (e.g., for an environment) is valid.
+// Broadly, this means it does not contain punctuation, whitespace, leading or
+// trailing slashes.
+func isValidName(name string) bool {
+	// No unicode whitespace is allowed. `Fields` doesn't handle trailing or
+	// leading whitespace.
+	fields := strings.Fields(name)
+	if len(fields) > 1 || len(strings.TrimSpace(name)) != len(name) {
+		return false
+	}
+
+	hasPunctuation := regexp.MustCompile(`[\\,;':!()?"{}\[\]*&%@$]+`).MatchString
+	hasTrailingSlashes := regexp.MustCompile(`/+$`).MatchString
+	hasLeadingSlashes := regexp.MustCompile(`^/+`).MatchString
+	return len(name) != 0 && !hasPunctuation(name) && !hasTrailingSlashes(name) && !hasLeadingSlashes(name)
+}
+
 func init() {
 	appFS = afero.NewOsFs()
 }
-- 
GitLab