Merge pull request #1088 from thaJeztah/fix_empty_stackname_validation

Validate stack-names for empty values

cli/command/stack/swarm/common.go

@@ -2,6 +2,9 @@ package swarm
 
 import (
 	"context"
+	"fmt"
+	"strings"
+	"unicode"
 
 	"github.com/docker/cli/cli/compose/convert"
 	"github.com/docker/cli/opts"
@@ -48,3 +51,28 @@ func getStackSecrets(ctx context.Context, apiclient client.APIClient, namespace
 func getStackConfigs(ctx context.Context, apiclient client.APIClient, namespace string) ([]swarm.Config, error) {
 	return apiclient.ConfigList(ctx, types.ConfigListOptions{Filters: getStackFilter(namespace)})
 }
+
+// validateStackName checks if the provided string is a valid stack name (namespace).
+//
+// It currently only does a rudimentary check if the string is empty, or consists
+// of only whitespace and quoting characters.
+func validateStackName(namespace string) error {
+	v := strings.TrimFunc(namespace, quotesOrWhitespace)
+	if len(v) == 0 {
+		return fmt.Errorf("invalid stack name: %q", namespace)
+	}
+	return nil
+}
+
+func validateStackNames(namespaces []string) error {
+	for _, ns := range namespaces {
+		if err := validateStackName(ns); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func quotesOrWhitespace(r rune) bool {
+	return unicode.IsSpace(r) || r == '"' || r == '\''
+}

cli/command/stack/swarm/deploy.go

@@ -24,6 +24,9 @@ const (
 func RunDeploy(dockerCli command.Cli, opts options.Deploy) error {
 	ctx := context.Background()
 
+	if err := validateStackName(opts.Namespace); err != nil {
+		return err
+	}
 	if err := validateResolveImageFlag(dockerCli, &opts); err != nil {
 		return err
 	}

cli/command/stack/swarm/deploy_bundlefile.go

@@ -16,6 +16,9 @@ import (
 )
 
 func deployBundle(ctx context.Context, dockerCli command.Cli, opts options.Deploy) error {
+	if err := validateStackName(opts.Namespace); err != nil {
+		return err
+	}
 	bundle, err := loadBundlefile(dockerCli.Err(), opts.Namespace, opts.Bundlefile)
 	if err != nil {
 		return err

cli/command/stack/swarm/deploy_composefile.go

@@ -18,6 +18,9 @@ import (
 )
 
 func deployCompose(ctx context.Context, dockerCli command.Cli, opts options.Deploy) error {
+	if err := validateStackName(opts.Namespace); err != nil {
+		return err
+	}
 	config, err := loader.LoadComposefile(dockerCli, opts)
 	if err != nil {
 		return err

cli/command/stack/swarm/deploy_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"testing"
 
+	"github.com/docker/cli/cli/command/stack/options"
 	"github.com/docker/cli/cli/compose/convert"
 	"github.com/docker/cli/internal/test"
 	"github.com/docker/docker/api/types"
@@ -26,6 +27,15 @@ func TestPruneServices(t *testing.T) {
 	assert.Check(t, is.DeepEqual(buildObjectIDs([]string{objectName("foo", "remove")}), client.removedServices))
 }
 
+func TestDeployWithEmptyName(t *testing.T) {
+	ctx := context.Background()
+	client := &fakeClient{}
+	dockerCli := test.NewFakeCli(client)
+
+	err := deployCompose(ctx, dockerCli, options.Deploy{Namespace: "'   '", Prune: true})
+	assert.Check(t, is.Error(err, `invalid stack name: "'   '"`))
+}
+
 // TestServiceUpdateResolveImageChanged tests that the service's
 // image digest, and "ForceUpdate" is preserved if the image did not change in
 // the compose file

cli/command/stack/swarm/ps.go

@@ -13,19 +13,21 @@ import (
 
 // RunPS is the swarm implementation of docker stack ps
 func RunPS(dockerCli command.Cli, opts options.PS) error {
-	namespace := opts.Namespace
-	client := dockerCli.Client()
-	ctx := context.Background()
+	if err := validateStackName(opts.Namespace); err != nil {
+		return err
+	}
 
 	filter := getStackFilterFromOpt(opts.Namespace, opts.Filter)
 
+	ctx := context.Background()
+	client := dockerCli.Client()
 	tasks, err := client.TaskList(ctx, types.TaskListOptions{Filters: filter})
 	if err != nil {
 		return err
 	}
 
 	if len(tasks) == 0 {
-		return fmt.Errorf("nothing found in stack: %s", namespace)
+		return fmt.Errorf("nothing found in stack: %s", opts.Namespace)
 	}
 
 	format := opts.Format

cli/command/stack/swarm/ps_test.go

@@ -0,0 +1,18 @@
+package swarm
+
+import (
+	"testing"
+
+	"github.com/docker/cli/cli/command/stack/options"
+	"github.com/docker/cli/internal/test"
+	"github.com/gotestyourself/gotestyourself/assert"
+	is "github.com/gotestyourself/gotestyourself/assert/cmp"
+)
+
+func TestRunPSWithEmptyName(t *testing.T) {
+	client := &fakeClient{}
+	dockerCli := test.NewFakeCli(client)
+
+	err := RunPS(dockerCli, options.PS{Namespace: "'   '"})
+	assert.Check(t, is.Error(err, `invalid stack name: "'   '"`))
+}

cli/command/stack/swarm/remove.go

@@ -16,12 +16,15 @@ import (
 
 // RunRemove is the swarm implementation of docker stack remove
 func RunRemove(dockerCli command.Cli, opts options.Remove) error {
-	namespaces := opts.Namespaces
+	if err := validateStackNames(opts.Namespaces); err != nil {
+		return err
+	}
+
 	client := dockerCli.Client()
 	ctx := context.Background()
 
 	var errs []string
-	for _, namespace := range namespaces {
+	for _, namespace := range opts.Namespaces {
 		services, err := getStackServices(ctx, client, namespace)
 		if err != nil {
 			return err

cli/command/stack/swarm/remove_test.go

@@ -0,0 +1,18 @@
+package swarm
+
+import (
+	"testing"
+
+	"github.com/docker/cli/cli/command/stack/options"
+	"github.com/docker/cli/internal/test"
+	"github.com/gotestyourself/gotestyourself/assert"
+	is "github.com/gotestyourself/gotestyourself/assert/cmp"
+)
+
+func TestRunRemoveWithEmptyName(t *testing.T) {
+	client := &fakeClient{}
+	dockerCli := test.NewFakeCli(client)
+
+	err := RunRemove(dockerCli, options.Remove{Namespaces: []string{"good", "'   '", "alsogood"}})
+	assert.Check(t, is.Error(err, `invalid stack name: "'   '"`))
+}

cli/command/stack/swarm/services.go

@@ -14,6 +14,9 @@ import (
 
 // RunServices is the swarm implementation of docker stack services
 func RunServices(dockerCli command.Cli, opts options.Services) error {
+	if err := validateStackName(opts.Namespace); err != nil {
+		return err
+	}
 	ctx := context.Background()
 	client := dockerCli.Client()
 

cli/command/stack/swarm/services_test.go

@@ -0,0 +1,18 @@
+package swarm
+
+import (
+	"testing"
+
+	"github.com/docker/cli/cli/command/stack/options"
+	"github.com/docker/cli/internal/test"
+	"github.com/gotestyourself/gotestyourself/assert"
+	is "github.com/gotestyourself/gotestyourself/assert/cmp"
+)
+
+func TestRunServicesWithEmptyName(t *testing.T) {
+	client := &fakeClient{}
+	dockerCli := test.NewFakeCli(client)
+
+	err := RunServices(dockerCli, options.Services{Namespace: "'   '"})
+	assert.Check(t, is.Error(err, `invalid stack name: "'   '"`))
+}