From a7180cd5459ca063c14a60965b4487f04c0af439 Mon Sep 17 00:00:00 2001
From: "W. Trevor King" <wking@tremily.us>
Date: Thu, 24 May 2018 13:18:52 -0700
Subject: hooks/1.0.0: Error on empty process.args instead of panicking

The process property is optional [1], which this package already
handled appropriately, although I've added a new test here to guard
against regressions.

The process.args entry is required when process is set [2], and it's
also required to contain at least one entry [3].  The previous
implementation here assumed that would always be satisfied, and
panicked on empty process.args.  With this commit, we avoid the panic
and instead return an error message explaining why the input was
invalid.

[1]: https://github.com/opencontainers/runtime-spec/blame/v1.0.1/config.md#L145
[2]: https://github.com/opencontainers/runtime-spec/blame/v1.0.1/config.md#L157
[3]: https://github.com/opencontainers/runtime-spec/blame/v1.0.1/config.md#L158

Reported-by: Brent Baude <bbaude@redhat.com>
Signed-off-by: W. Trevor King <wking@tremily.us>

Closes: #829
Approved by: mheon
---
 pkg/hooks/1.0.0/when.go      |  3 +++
 pkg/hooks/1.0.0/when_test.go | 42 +++++++++++++++++++++++++++++++++---------
 2 files changed, 36 insertions(+), 9 deletions(-)

(limited to 'pkg/hooks/1.0.0')

diff --git a/pkg/hooks/1.0.0/when.go b/pkg/hooks/1.0.0/when.go
index 3d2a5fd72..c23223ec0 100644
--- a/pkg/hooks/1.0.0/when.go
+++ b/pkg/hooks/1.0.0/when.go
@@ -75,6 +75,9 @@ func (when *When) Match(config *rspec.Spec, annotations map[string]string, hasBi
 	}
 
 	if config.Process != nil {
+		if len(config.Process.Args) == 0 {
+			return false, errors.New("process.args must have at least one entry")
+		}
 		command := config.Process.Args[0]
 		for _, cmdPattern := range when.Commands {
 			match, err := regexp.MatchString(cmdPattern, command)
diff --git a/pkg/hooks/1.0.0/when_test.go b/pkg/hooks/1.0.0/when_test.go
index 9047f4c9f..5a73270ac 100644
--- a/pkg/hooks/1.0.0/when_test.go
+++ b/pkg/hooks/1.0.0/when_test.go
@@ -142,25 +142,33 @@ func TestCommands(t *testing.T) {
 			"^/bin/sh$",
 		},
 	}
-	config := &rspec.Spec{Process: &rspec.Process{}}
+	config := &rspec.Spec{}
 	for _, test := range []struct {
-		name  string
-		args  []string
-		match bool
+		name    string
+		process *rspec.Process
+		match   bool
 	}{
 		{
-			name:  "good",
-			args:  []string{"/bin/sh", "a", "b"},
+			name: "good",
+			process: &rspec.Process{
+				Args: []string{"/bin/sh", "a", "b"},
+			},
 			match: true,
 		},
 		{
-			name:  "extra characters",
-			args:  []string{"/bin/shell", "a", "b"},
+			name: "extra characters",
+			process: &rspec.Process{
+				Args: []string{"/bin/shell", "a", "b"},
+			},
+			match: false,
+		},
+		{
+			name:  "process unset",
 			match: false,
 		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
-			config.Process.Args = test.args
+			config.Process = test.process
 			match, err := when.Match(config, map[string]string{}, false)
 			if err != nil {
 				t.Fatal(err)
@@ -170,6 +178,22 @@ func TestCommands(t *testing.T) {
 	}
 }
 
+func TestCommandsEmptyProcessArgs(t *testing.T) {
+	when := When{
+		Commands: []string{
+			"^/bin/sh$",
+		},
+	}
+	config := &rspec.Spec{
+		Process: &rspec.Process{},
+	}
+	_, err := when.Match(config, map[string]string{}, false)
+	if err == nil {
+		t.Fatal("unexpected success")
+	}
+	assert.Regexp(t, "^process\\.args must have at least one entry$", err.Error())
+}
+
 func TestHasBindMountsAndCommands(t *testing.T) {
 	hasBindMounts := true
 	when := When{
-- 
cgit v1.2.3-54-g00ecf