Escape label names when writing to disk.

cmd/pullrequest-init/disk.go

@@ -19,6 +19,7 @@ package main
 import (
 	"encoding/json"
 	"io/ioutil"
+	"net/url"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -37,6 +38,8 @@ import (
 // /workspace/<resource>/head.json
 // /workspace/<resource>/base.json
 
+// Filenames for labels and statuses are URL encoded for safety.
+
 // ToDisk converts a PullRequest object to an on-disk representation at the specified path.
 func ToDisk(pr *PullRequest, path string) error {
 	labelsPath := filepath.Join(path, "labels")
@@ -92,7 +95,8 @@ func commentsToDisk(path string, comments []*Comment) error {
 
 func labelsToDisk(path string, labels []*Label) error {
 	for _, l := range labels {
-		labelPath := filepath.Join(path, l.Text)
+		name := url.QueryEscape(l.Text)
+		labelPath := filepath.Join(path, name)
 		if err := ioutil.WriteFile(labelPath, []byte{}, 0700); err != nil {
 			return err
 		}
@@ -102,7 +106,8 @@ func labelsToDisk(path string, labels []*Label) error {
 
 func statusToDisk(path string, statuses []*Status) error {
 	for _, s := range statuses {
-		statusPath := filepath.Join(path, s.ID+".json")
+		statusName := url.QueryEscape(s.ID) + ".json"
+		statusPath := filepath.Join(path, statusName)
 		b, err := json.Marshal(s)
 		if err != nil {
 			return err
@@ -191,7 +196,11 @@ func labelsFromDisk(path string) ([]*Label, error) {
 	}
 	labels := []*Label{}
 	for _, fi := range fis {
-		labels = append(labels, &Label{fi.Name()})
+		text, err := url.QueryUnescape(fi.Name())
+		if err != nil {
+			return nil, err
+		}
+		labels = append(labels, &Label{Text: text})
 	}
 	return labels, nil
 }

cmd/pullrequest-init/disk_test.go

@@ -19,8 +19,11 @@ package main
 import (
 	"encoding/json"
 	"io/ioutil"
+	"net/url"
 	"os"
 	"path/filepath"
+	"reflect"
+	"sort"
 	"strconv"
 	"testing"
 
@@ -48,6 +51,12 @@ func TestToDisk(t *testing.T) {
 				Description: "foobar",
 				URL:         "https://foo.bar",
 			},
+			{
+				ID:          "cla/foo",
+				Code:        Success,
+				Description: "bazbat",
+				URL:         "https://baz.bat",
+			},
 		},
 		Comments: []*Comment{
 			{
@@ -59,6 +68,7 @@ func TestToDisk(t *testing.T) {
 		Labels: []*Label{
 			{Text: "help"},
 			{Text: "me"},
+			{Text: "foo/bar"},
 		},
 	}
 
@@ -111,7 +121,11 @@ func TestToDisk(t *testing.T) {
 	}
 	labels := map[string]struct{}{}
 	for _, fi := range fis {
-		labels[fi.Name()] = struct{}{}
+		text, err := url.QueryUnescape(fi.Name())
+		if err != nil {
+			t.Errorf("Error decoding label text: %s", fi.Name())
+		}
+		labels[text] = struct{}{}
 	}
 
 	for _, l := range tektonPr.Labels {
@@ -198,7 +212,7 @@ func TestFromDisk(t *testing.T) {
 	if err := os.MkdirAll(filepath.Join(d, "labels"), 0750); err != nil {
 		t.Fatal(err)
 	}
-	labels := []string{"hey", "you"}
+	labels := []string{"hey", "you", "size%2Flgtm"}
 	for _, l := range labels {
 		if err := ioutil.WriteFile(filepath.Join(d, l), []byte{}, 0700); err != nil {
 			t.Fatal(err)
@@ -264,7 +278,8 @@ func TestFromDisk(t *testing.T) {
 		labelsMap[l] = struct{}{}
 	}
 	for _, l := range pr.Labels {
-		if diff := cmp.Diff(labelsMap[l.Text], &l); diff != "" {
+		key := url.QueryEscape(l.Text)
+		if diff := cmp.Diff(labelsMap[key], &l); diff != "" {
 			t.Errorf("Get labels: -want +got: %s", diff)
 		}
 	}
@@ -291,3 +306,215 @@ func readAndUnmarshal(t *testing.T, p string, v interface{}) {
 		t.Fatal(err)
 	}
 }
+
+func Test_labelsToDisk(t *testing.T) {
+	type args struct {
+		labels []*Label
+	}
+	tests := []struct {
+		name      string
+		args      args
+		wantFiles []string
+	}{
+		{
+			name: "single label",
+			args: args{
+				labels: []*Label{
+					{Text: "foo"},
+				},
+			},
+			wantFiles: []string{
+				"foo",
+			},
+		},
+		{
+			name: "multiple labels",
+			args: args{
+				labels: []*Label{
+					{Text: "foo"},
+					{Text: "bar"},
+				},
+			},
+			wantFiles: []string{
+				"foo",
+				"bar",
+			},
+		},
+		{
+			name: "complex labels",
+			args: args{
+				labels: []*Label{
+					{Text: "foo/bar"},
+					{Text: "help wanted"},
+					{Text: "simple"},
+				},
+			},
+			wantFiles: []string{
+				"foo%2Fbar",
+				"help+wanted",
+				"simple",
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			d, err := ioutil.TempDir("", "")
+			if err != nil {
+				t.Fatalf("Error creating temp dir: %s", err)
+			}
+			defer os.RemoveAll(d)
+			if err := labelsToDisk(d, tt.args.labels); err != nil {
+				t.Errorf("labelsToDisk() error = %v", err)
+			}
+			for _, f := range tt.wantFiles {
+				if _, err := os.Stat(filepath.Join(d, f)); err != nil {
+					t.Errorf("expected file %s to exist", f)
+				}
+			}
+		})
+	}
+}
+
+func Test_statusToDisk(t *testing.T) {
+	type args struct {
+		statuses []*Status
+	}
+	tests := []struct {
+		name      string
+		args      args
+		wantFiles []string
+	}{
+		{
+			name: "single status",
+			args: args{
+				statuses: []*Status{
+					{ID: "foo"},
+				},
+			},
+			wantFiles: []string{
+				"foo.json",
+			},
+		},
+		{
+			name: "multiple statuses",
+			args: args{
+				statuses: []*Status{
+					{ID: "foo"},
+					{ID: "bar"},
+				},
+			},
+			wantFiles: []string{
+				"foo.json",
+				"bar.json",
+			},
+		},
+		{
+			name: "complex statuses",
+			args: args{
+				statuses: []*Status{
+					{ID: "foo/bar"},
+					{ID: "help wanted"},
+					{ID: "simple"},
+				},
+			},
+			wantFiles: []string{
+				"foo%2Fbar.json",
+				"help+wanted.json",
+				"simple.json",
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			d, err := ioutil.TempDir("", "")
+			if err != nil {
+				t.Fatalf("Error creating temp dir: %s", err)
+			}
+			defer os.RemoveAll(d)
+			if err := statusToDisk(d, tt.args.statuses); err != nil {
+				t.Errorf("statusToDisk() error = %v", err)
+			}
+			for _, f := range tt.wantFiles {
+				if _, err := os.Stat(filepath.Join(d, f)); err != nil {
+					t.Errorf("expected file %s to exist", f)
+				}
+			}
+		})
+	}
+}
+
+func Test_labelsFromDisk(t *testing.T) {
+	type args struct {
+		fileNames []string
+	}
+	tests := []struct {
+		name string
+		args args
+		want []Label
+	}{
+		{
+			name: "single label",
+			args: args{
+				fileNames: []string{"foo"},
+			},
+			want: []Label{
+				{Text: "foo"},
+			},
+		},
+		{
+			name: "multiple labels",
+			args: args{
+				fileNames: []string{"foo", "bar"},
+			},
+			want: []Label{
+				{Text: "foo"},
+				{Text: "bar"},
+			},
+		},
+		{
+			name: "complex labels",
+			args: args{
+				fileNames: []string{"foo%2Fbar", "bar+bat"},
+			},
+			want: []Label{
+				{Text: "foo/bar"},
+				{Text: "bar bat"},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			d, err := ioutil.TempDir("", "")
+			if err != nil {
+				t.Fatalf("Error creating temp dir: %s", err)
+			}
+			defer os.RemoveAll(d)
+
+			for _, l := range tt.args.fileNames {
+				if err := ioutil.WriteFile(filepath.Join(d, l), []byte{}, 0700); err != nil {
+					t.Errorf("Error creating label: %s", err)
+				}
+			}
+			got, err := labelsFromDisk(d)
+			if err != nil {
+				t.Errorf("labelsFromDisk() error = %v", err)
+			}
+
+			derefed := []Label{}
+			for _, l := range got {
+				derefed = append(derefed, *l)
+			}
+
+			sort.Slice(derefed, func(i, j int) bool {
+				return derefed[i].Text < derefed[j].Text
+			})
+			sort.Slice(tt.want, func(i, j int) bool {
+				return tt.want[i].Text < tt.want[j].Text
+			})
+
+			if !reflect.DeepEqual(derefed, tt.want) {
+				t.Errorf("labelsFromDisk() = %v, want %v", derefed, tt.want)
+			}
+		})
+	}
+}

examples/taskruns/taskrun-github-pr-yaml

@@ -0,0 +1,51 @@
+apiVersion: tekton.dev/v1alpha1
+kind: PipelineResource
+metadata:
+  name: tekton-pr
+spec:
+  type: pullRequest
+  params:
+  - name: url
+    # I just picked a random PR. The first couple didn't have any interesting comments or labels.
+    value: https://github.com/tektoncd/pipeline/pull/100
+---
+apiVersion: tekton.dev/v1alpha1
+kind: Task
+metadata:
+  name: dump-pr-task
+spec:
+  inputs:
+    resources:
+    - name: pr
+      type: pullRequest
+  steps:
+  - name: dump-workspace
+    image: ubuntu
+    command: ["sh"]
+    args: ["-c", "find $(inputs.resources.pr.path)/* -type f | xargs tail -n +1"]
+  - name: ensure-approved
+    image: ubuntu
+    command: ["/bin/bash"]
+    args:
+    - -c
+    - |
+      if [ -f "$(inputs.resources.pr.path)/labels/approved" ]; then
+        echo "PR is approved!"
+      else
+        echo "PR is not approved!"
+        exit 1
+      fi
+---
+apiVersion: tekton.dev/v1alpha1
+kind: TaskRun
+metadata:
+  name: dump-pr-task-run
+spec:
+  inputs:
+    resources:
+    - name: pr
+      resourceRef:
+        name: tekton-pr
+  taskRef:
+    name: dump-pr-task
+---