Aliases are never checked in Helm

@@ -17,6 +17,7 @@ package chart
 
 import (
 	"path/filepath"
+	"regexp"
 	"strings"
 )
 
@@ -26,6 +27,9 @@ const APIVersionV1 = "v1"
 // APIVersionV2 is the API version number for version 2.
 const APIVersionV2 = "v2"
 
+// aliasNameFormat defines the characters that are legal in an alias name.
+var aliasNameFormat = regexp.MustCompile("^[a-zA-Z0-9_-]+$")
+
 // Chart is a helm package that contains metadata, a default config, zero or more
 // optionally parameterizable templates, and zero or more charts (dependencies).
 type Chart struct {

@@ -15,9 +15,16 @@ limitations under the License.
 
 package chart
 
+import "fmt"
+
 // ValidationError represents a data validation error.
 type ValidationError string
 
 func (v ValidationError) Error() string {
 	return "validation: " + string(v)
 }
+
+// ValidationErrorf takes a message and formatting options and creates a ValidationError
+func ValidationErrorf(msg string, args ...interface{}) ValidationError {
+	return ValidationError(fmt.Sprintf(msg, args...))
+}

@@ -81,6 +81,15 @@ func (md *Metadata) Validate() error {
 	if !isValidChartType(md.Type) {
 		return ValidationError("chart.metadata.type must be application or library")
 	}
+
+	// Aliases need to be validated here to make sure that the alias name does
+	// not contain any illegal characters.
+	for _, dependency := range md.Dependencies {
+		if err := validateDependency(dependency); err != nil {
+			return err
+		}
+	}
+
 	// TODO validate valid semver here?
 	return nil
 }
@@ -92,3 +101,13 @@ func isValidChartType(in string) bool {
 	}
 	return false
 }
+
+// validateDependency checks for common problems with the dependency datastructure in
+// the chart. This check must be done at load time before the dependency's charts are
+// loaded.
+func validateDependency(dep *Dependency) error {
+	if len(dep.Alias) > 0 && !aliasNameFormat.MatchString(dep.Alias) {
+		return ValidationErrorf("dependency %q has disallowed characters in the alias", dep.Name)
+	}
+	return nil
+}

@@ -48,12 +48,60 @@ func TestValidate(t *testing.T) {
 			&Metadata{Name: "test", APIVersion: "v2", Version: "1.0", Type: "application"},
 			nil,
 		},
+		{
+			&Metadata{
+				Name:       "test",
+				APIVersion: "v2",
+				Version:    "1.0",
+				Type:       "application",
+				Dependencies: []*Dependency{
+					{Name: "dependency", Alias: "legal-alias"},
+				},
+			},
+			nil,
+		},
+		{
+			&Metadata{
+				Name:       "test",
+				APIVersion: "v2",
+				Version:    "1.0",
+				Type:       "application",
+				Dependencies: []*Dependency{
+					{Name: "bad", Alias: "illegal alias"},
+				},
+			},
+			ValidationError("dependency \"bad\" has disallowed characters in the alias"),
+		},
 	}
 
 	for _, tt := range tests {
 		result := tt.md.Validate()
 		if result != tt.err {
-			t.Errorf("expected %s, got %s", tt.err, result)
+			t.Errorf("expected '%s', got '%s'", tt.err, result)
+		}
+	}
+}
+
+func TestValidateDependency(t *testing.T) {
+	dep := &Dependency{
+		Name: "example",
+	}
+	for value, shouldFail := range map[string]bool{
+		"abcdefghijklmenopQRSTUVWXYZ-0123456780_": false,
+		"-okay":      false,
+		"_okay":      false,
+		"- bad":      true,
+		" bad":       true,
+		"bad\nvalue": true,
+		"bad ":       true,
+		"bad$":       true,
+	} {
+		dep.Alias = value
+		res := validateDependency(dep)
+		if res != nil && !shouldFail {
+			t.Errorf("Failed on case %q", dep.Alias)
+		} else if res == nil && shouldFail {
+			t.Errorf("Expected failure for %q", dep.Alias)
 		}
 	}
 }

@@ -17,7 +17,9 @@ package chartutil
 
 import (
 	"errors"
+	"fmt"
 	"log"
+	"regexp"
 	"strings"
 	"time"
 
@@ -219,6 +221,9 @@ func ProcessRequirementsTags(reqs *Requirements, cvals Values) {
 
 }
 
+// Validate alias names against this regexp
+var aliasRegexp = regexp.MustCompile("^[a-zA-Z0-9-_]+$")
+
 func getAliasDependency(charts []*chart.Chart, aliasChart *Dependency) *chart.Chart {
 	var chartFound chart.Chart
 	for _, existingChart := range charts {
@@ -237,6 +242,11 @@ func getAliasDependency(charts []*chart.Chart, aliasChart *Dependency) *chart.Ch
 		chartFound = *existingChart
 		newMetadata := *existingChart.Metadata
 		if aliasChart.Alias != "" {
+			// Make sure Alias is well-formed
+			if !aliasRegexp.MatchString(aliasChart.Alias) {
+				fmt.Printf("Invalid alias in dependency %q. Skipping.", aliasChart.Name)
+				continue
+			}
 			newMetadata.Name = aliasChart.Alias
 		}
 		chartFound.Metadata = &newMetadata
@@ -286,6 +296,9 @@ func doProcessRequirementsEnabled(c *chart.Chart, v *chart.Config, path string)
 			chartDependencies = append(chartDependencies, chartDependency)
 		}
 		if req.Alias != "" {
+			if !aliasRegexp.MatchString(req.Alias) {
+				return fmt.Errorf("illegal alias name in %q", req.Name)
+			}
 			req.Name = req.Alias
 		}
 	}

@@ -370,11 +370,19 @@ func TestGetAliasDependency(t *testing.T) {
 	}
 
 	// Failure case
+	resetName := req.Dependencies[0].Name
 	req.Dependencies[0].Name = "something-else"
 	if aliasChart := getAliasDependency(c.Dependencies, req.Dependencies[0]); aliasChart != nil {
 		t.Fatalf("expected no chart but got %s", aliasChart.Metadata.Name)
 	}
 
+	// Add a bad alias name
+	req.Dependencies[0].Name = resetName
+	req.Dependencies[0].Alias = "$foobar"
+	if aliasChart := getAliasDependency(c.Dependencies, req.Dependencies[0]); aliasChart != nil {
+		t.Fatalf("expected no chart but got %s", aliasChart.Metadata.Name)
+	}
+
 	req.Dependencies[0].Version = "something else which is not in the compatible range"
 	if version.IsCompatibleRange(req.Dependencies[0].Version, aliasChart.Metadata.Version) {
 		t.Fatalf("Dependency chart version which is not in the compatible range should cause a failure other than a success ")
@@ -516,3 +524,16 @@ func TestDependentChartsWithSomeSubchartsSpecifiedInRequirements(t *testing.T) {
 	}
 
 }
+
+func TestAliasRegexp(t *testing.T) {
+	for name, shouldPass := range map[string]bool{
+		"abcdefghijklmnopqrstuvwxyzABCDEFG0987654321_-": true,
+		"$foo":     false,
+		"bar$":     false,
+		"foo\nbar": false,
+	} {
+		if aliasRegexp.MatchString(name) != shouldPass {
+			t.Errorf("name %q failed to pass its test", name)
+		}
+	}
+}

@@ -16,6 +16,7 @@ limitations under the License.
 package plugin // import "k8s.io/helm/pkg/plugin"
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -141,13 +142,22 @@ func LoadAll(basedir string) ([]*Plugin, error) {
 		return plugins, nil
 	}
 
+	loaded := map[string]bool{}
 	for _, yaml := range matches {
 		dir := filepath.Dir(yaml)
 		p, err := LoadDir(dir)
+		pname := p.Metadata.Name
 		if err != nil {
 			return plugins, err
 		}
+
+		if _, ok := loaded[pname]; ok {
+			fmt.Fprintf(os.Stderr, "A plugin named %q already exists. Skipping.", pname)
+			continue
+		}
+
 		plugins = append(plugins, p)
+		loaded[pname] = true
 	}
 	return plugins, nil
 }

@@ -137,6 +137,7 @@ func TestLoadAll(t *testing.T) {
 		t.Fatalf("Could not load %q: %s", basedir, err)
 	}
 
+	// This would fail if the duplicate plugin were loaded.
 	if l := len(plugs); l != 3 {
 		t.Fatalf("expected 3 plugins, found %d", l)
 	}

@@ -0,0 +1,13 @@
+#!/bin/bash
+
+echo "Hello from a Helm plugin"
+
+echo "PARAMS"
+echo $*
+
+echo "ENVIRONMENT"
+echo $TILLER_HOST
+echo $HELM_HOME
+
+$HELM_BIN --host $TILLER_HOST ls --all
+

@@ -0,0 +1,11 @@
+name: "hello"
+version: "0.1.0"
+usage: "usage"
+description: |-
+  description
+command: "$HELM_PLUGIN_SELF/hello.sh"
+useTunnel: true
+ignoreFlags: true
+install: "echo installing..."
+hooks:
+  install: "echo installing..."

@@ -30,6 +30,7 @@ import (
 
 	"github.com/Masterminds/semver"
 	"github.com/ghodss/yaml"
+	yaml2 "gopkg.in/yaml.v2"
 
 	"k8s.io/helm/pkg/chartutil"
 	"k8s.io/helm/pkg/proto/hapi/chart"
@@ -83,6 +84,14 @@ type IndexFile struct {
 	PublicKeys []string                 `json:"publicKeys,omitempty"`
 }
 
+// IndexValidation is used to validate the integrity of an index file
+type IndexValidation struct {
+	APIVersion string                 `yaml:"apiVersion"`
+	Generated  time.Time              `yaml:"generated"`
+	Entries    map[string]interface{} `yaml:"entries"`
+	PublicKeys []string               `yaml:"publicKeys,omitempty"`
+}
+
 // NewIndexFile initializes an index.
 func NewIndexFile() *IndexFile {
 	return &IndexFile{
@@ -283,9 +292,14 @@ func IndexDirectory(dir, baseURL string) (*IndexFile, error) {
 // This will fail if API Version is not set (ErrNoAPIVersion) or if the unmarshal fails.
 func loadIndex(data []byte) (*IndexFile, error) {
 	i := &IndexFile{}
+	if err := validateIndex(data); err != nil {
+		return i, err
+	}
+
 	if err := yaml.Unmarshal(data, i); err != nil {
 		return i, err
 	}
+
 	i.SortEntries()
 	if i.APIVersion == "" {
 		// When we leave Beta, we should remove legacy support and just
@@ -296,6 +310,16 @@ func loadIndex(data []byte) (*IndexFile, error) {
 	return i, nil
 }
 
+// validateIndex validates that the index is well-formed.
+func validateIndex(data []byte) error {
+	// This is done ONLY for validation. We need to use ghodss/yaml for the actual parsing.
+	validation := &IndexValidation{}
+	if err := yaml2.UnmarshalStrict(data, validation); err != nil {
+		return err
+	}
+	return nil
+}
+
 // unversionedEntry represents a deprecated pre-Alpha.5 format.
 //
 // This will be removed prior to v2.0.0

@@ -422,3 +422,19 @@ func TestIndexAdd(t *testing.T) {
 		t.Errorf("Expected http://example.com/charts/deis-0.1.0.tgz, got %s", i.Entries["deis"][0].URLs[0])
 	}
 }
+
+const mockDuplicateIndex = `
+entries:
+  foo: {}
+  bar: {}
+  baz: {}
+  bar: {}
+`
+
+func TestValidateIndex(t *testing.T) {
+	expect := `key "bar" already set in map`
+	err := validateIndex([]byte(mockDuplicateIndex))
+	if strings.Contains(expect, err.Error()) {
+		t.Errorf("Unexpected error: %s", err)
+	}
+}