Restrict task names to a strict ASCII whitelist

Task names appear in on-disk checksum paths, terminal log lines and shell completion output. The previous validator only blocked path separators and '..', so a task file could still smuggle in control characters, terminal escapes, shell metacharacters or whitespace. Allow only ASCII letters, digits, '-', '_' and ':' (the namespace separator), and forbid leading/trailing/consecutive ':'.

Author: dgageot

taskfile/parse.go

@@ -12,19 +12,29 @@ import (
 
 const fileName = "gogo.yaml"
 
-// validateTaskName rejects task names containing characters that could escape
-// the on-disk checksum directory or otherwise misbehave when joined into a
-// filesystem path. Namespace separators (':') and ordinary identifier
-// characters are allowed; '/', '\\' and '..' are not.
+// validateTaskName enforces a strict whitelist of characters allowed in task
+// names. Names appear in checksum file paths, log lines, and shell
+// completions; allowing arbitrary characters lets a malicious task file
+// inject control characters, terminal escapes, path components, or shell
+// metacharacters into those contexts.
+//
+// Allowed: ASCII letters, digits, '-', '_', and ':' (the namespace separator).
 func validateTaskName(name string) error {
 	if name == "" {
 		return errors.New("task name must not be empty")
 	}
-	if strings.ContainsAny(name, `/\`) {
-		return fmt.Errorf("task name %q must not contain '/' or '\\'", name)
+	for _, r := range name {
+		switch {
+		case 'a' <= r && r <= 'z':
+		case 'A' <= r && r <= 'Z':
+		case '0' <= r && r <= '9':
+		case r == '-' || r == '_' || r == ':':
+		default:
+			return fmt.Errorf("task name %q contains disallowed character %q (allowed: letters, digits, '-', '_', ':')", name, r)
+		}
 	}
-	if strings.Contains(name, "..") {
-		return fmt.Errorf("task name %q must not contain '..'", name)
+	if strings.HasPrefix(name, ":") || strings.HasSuffix(name, ":") || strings.Contains(name, "::") {
+		return fmt.Errorf("task name %q must not start with, end with, or contain consecutive ':'", name)
 	}
 	return nil
 }

taskfile/parse_test.go

@@ -759,14 +759,34 @@ flatten:
 }
 
 func TestParseRejectsUnsafeTaskNames(t *testing.T) {
-	// Names containing path separators or '..' could be used to escape the
-	// .gogo/checksum/ directory when joined with checksumPath.
+	// Task names appear in checksum file paths, log lines and shell
+	// completions. We allow only letters, digits, '-', '_' and ':'; anything
+	// else — path separators, control characters, terminal escapes, shell
+	// metacharacters — must be rejected at parse time.
 	cases := map[string]string{
-		"slash":      "foo/bar",
-		"backslash":  `foo\bar`,
-		"parent":     "..",
-		"traversal":  "../etc/passwd",
-		"dotdot-mid": "foo..bar",
+		"slash":          "foo/bar",
+		"backslash":      `foo\bar`,
+		"parent":         "..",
+		"traversal":      "../etc/passwd",
+		"dotdot-mid":     "foo..bar",
+		"space":          "foo bar",
+		"newline":        "foo\nbar",
+		"tab":            "foo\tbar",
+		"escape":         "foo\x1b[31mred",
+		"null":           "foo\x00bar",
+		"backtick":       "foo`whoami`",
+		"dollar":         "$(whoami)",
+		"semicolon":      "foo;rm",
+		"pipe":           "foo|rm",
+		"glob":           "foo*",
+		"question":       "foo?",
+		"quote":          `foo"bar`,
+		"singlequote":    "foo'bar",
+		"unicode":        "foo\u202ebar",
+		"leading-colon":  ":build",
+		"trailing-colon": "build:",
+		"double-colon":   "foo::bar",
+		"dot":            "foo.bar",
 	}
 	for label, name := range cases {
 		t.Run(label, func(t *testing.T) {
@@ -782,6 +802,31 @@ func TestParseRejectsUnsafeTaskNames(t *testing.T) {
 	}
 }
 
+func TestParseAcceptsValidTaskNames(t *testing.T) {
+	names := []string{
+		"build",
+		"build-all",
+		"build_all",
+		"_internal",
+		"cli:build",
+		"cli:utils:fmt",
+		"go121",
+		"BuildIt",
+	}
+	for _, name := range names {
+		t.Run(name, func(t *testing.T) {
+			dir := t.TempDir()
+			writeFiles(t, dir, map[string]string{
+				"gogo.yaml": fmt.Sprintf("version: \"1\"\ntasks:\n  %q:\n    cmd: echo hi\n", name),
+			})
+
+			tf, err := Parse(dir)
+			require.NoError(t, err)
+			assert.Contains(t, tf.Tasks, name)
+		})
+	}
+}
+
 func TestLoadWithFlattenRejectsUnsafeTaskName(t *testing.T) {
 	dir := t.TempDir()
 	writeFiles(t, dir, map[string]string{