Harden workflow-name and owner-label validation against injection

cmd/account/link_key/link_key.go

@@ -39,8 +39,7 @@ const (
 )
 
 type Inputs struct {
-	// TODO: Add validation for WorkflowOwnerLabel
-	WorkflowOwnerLabel              string `validate:"omitempty"`
+	WorkflowOwnerLabel              string `validate:"omitempty,owner_label" cli:"--owner-label"`
 	WorkflowOwner                   string `validate:"required,workflow_owner"`
 	WorkflowRegistryContractAddress string `validate:"required"`
 	NonInteractive                  bool
@@ -138,7 +137,7 @@ func (h *handler) ResolveInputs(v *viper.Viper) (Inputs, error) {
 	return Inputs{
 		WorkflowOwner:                   h.settings.Workflow.UserWorkflowSettings.WorkflowOwnerAddress,
 		WorkflowRegistryContractAddress: h.environmentSet.WorkflowRegistryAddress,
-		WorkflowOwnerLabel:              v.GetString("owner-label"),
+		WorkflowOwnerLabel:              strings.TrimSpace(v.GetString("owner-label")),
 		NonInteractive:                  v.GetBool(settings.Flags.NonInteractive.Name),
 	}, nil
 }
@@ -185,16 +184,21 @@ func (h *handler) Execute(ctx context.Context, in Inputs) error {
 		label, err := ui.Input(
 			title,
 			ui.WithDefaultValue(defaultLabel),
+			// IsValidOwnerLabel rejects the empty string, so this also enforces
+			// that a label is required.
 			ui.WithValidate(func(s string) error {
-				if strings.TrimSpace(s) == "" {
-					return fmt.Errorf("a label is required")
-				}
-				return nil
+				return validation.IsValidOwnerLabel(strings.TrimSpace(s))
 			}),
 		)
 		if err != nil {
 			return err
 		}
+		// Re-check after the prompt: Execute runs after ValidateInputs, so a
+		// prompt-supplied label would otherwise never reach the validator.
+		label = strings.TrimSpace(label)
+		if err := validation.IsValidOwnerLabel(label); err != nil {
+			return err
+		}
 		in.WorkflowOwnerLabel = label
 	}
 
@@ -425,17 +429,25 @@ func (h *handler) checkIfAlreadyLinked() (bool, error) {
 }
 
 // defaultOwnerLabel derives a suggested label from the authenticated user's
-// email address (the portion before the "@"). Returns "" if no email is available.
+// email address (the portion before the "@"). Returns "" if no email is
+// available, or if the derived value is not a valid owner label — an email
+// local part may contain characters a label may not, and pre-filling the
+// prompt with a value the validator rejects would block submission.
 func (h *handler) defaultOwnerLabel() string {
 	email, err := h.credentials.GetEmail()
 	if err != nil || email == "" {
 		return ""
 	}
 
+	label := email
 	if idx := strings.Index(email, "@"); idx > 0 {
-		return email[:idx]
+		label = email[:idx]
+	}
+
+	if validation.IsValidOwnerLabel(label) != nil {
+		return ""
 	}
-	return email
+	return label
 }
 
 func (h *handler) displayDetails() {

cmd/account/link_key/link_key_test.go

@@ -1,12 +1,58 @@
 package link_key
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
+func TestValidateInputs_OwnerLabel(t *testing.T) {
+	t.Parallel()
+
+	const (
+		validOwner    = "0x0000000000000000000000000000000000000001"
+		validRegistry = "0x0000000000000000000000000000000000000002"
+	)
+
+	tests := []struct {
+		name      string
+		label     string
+		wantError bool
+	}{
+		{name: "valid label", label: "prod owner 1", wantError: false},
+		{name: "valid label with dashes", label: "owner-label-1", wantError: false},
+		{name: "empty label is deferred to the prompt", label: "", wantError: false},
+		{name: "double quote breaks out of yaml scalar", label: `bad"; echo pwned`, wantError: true},
+		{name: "command substitution", label: "bad$(id)", wantError: true},
+		{name: "newline", label: "a\ninjected: true", wantError: true},
+		{name: "leading dash looks like a flag", label: "-rf /", wantError: true},
+		{name: "too long", label: strings.Repeat("a", 65), wantError: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			h := &handler{}
+			err := h.ValidateInputs(Inputs{
+				WorkflowOwnerLabel:              tt.label,
+				WorkflowOwner:                   validOwner,
+				WorkflowRegistryContractAddress: validRegistry,
+			})
+
+			if tt.wantError {
+				require.Error(t, err, "expected label %q to be rejected", tt.label)
+				assert.Contains(t, err.Error(), "--owner-label",
+					"error should name the flag the user passed")
+				assert.False(t, h.validated, "handler must not be marked validated")
+			} else {
+				require.NoError(t, err, "expected label %q to be accepted", tt.label)
+			}
+		})
+	}
+}
+
 func TestNonInteractive_WithoutOwnerLabel_BlocksPrompt(t *testing.T) {
 	t.Parallel()
 	in := Inputs{

cmd/creinit/creinit.go

@@ -167,6 +167,19 @@ func (h *handler) ValidateInputs(inputs Inputs) error {
 	return nil
 }
 
+// validateTemplateWorkflowDirs checks every workflow directory declared by a template manifest.
+// The manifest comes from a remote repository, and each dir is used both as a path segment during
+// scaffolding and as the workflow name substituted into workflow.yaml, so each must satisfy the
+// same rules as a user-supplied workflow name.
+func validateTemplateWorkflowDirs(tmpl *templaterepo.TemplateSummary) error {
+	for _, wf := range tmpl.Workflows {
+		if err := validation.IsValidWorkflowName(wf.Dir); err != nil {
+			return fmt.Errorf("template %q declares an invalid workflow directory %q: %w", tmpl.Name, wf.Dir, err)
+		}
+	}
+	return nil
+}
+
 func (h *handler) Execute(inputs Inputs) error {
 	if !h.validated {
 		return fmt.Errorf("handler inputs not validated")
@@ -338,6 +351,13 @@ func (h *handler) Execute(inputs Inputs) error {
 		return fmt.Errorf("no template selected")
 	}
 
+	// Template manifests are fetched from a remote repo, so their declared workflow dirs are
+	// untrusted input: each becomes a path segment during scaffolding and the workflow name
+	// substituted into workflow.yaml. Check them all before anything is written to disk.
+	if err := validateTemplateWorkflowDirs(selectedTemplate); err != nil {
+		return err
+	}
+
 	// Store for telemetry (flag will be set in RunE)
 	h.selectedTemplateName = selectedTemplate.Name
 

cmd/creinit/creinit_test.go

@@ -1042,3 +1042,94 @@ func TestInitProjectRootFlagFindsExistingProject(t *testing.T) {
 		GetTemplateFileListGo(),
 	)
 }
+
+// maliciousDirTemplate builds a template whose second declared workflow dir is untrusted. Two
+// workflows are declared so the wizard skips the workflow-name step (which would need a TTY),
+// and so the unsafe dir is reached through the multi-workflow scaffolding path.
+func maliciousDirTemplate(name, dir string) templaterepo.TemplateSummary {
+	return templaterepo.TemplateSummary{
+		TemplateMetadata: templaterepo.TemplateMetadata{
+			Kind:        "starter-template",
+			Name:        name,
+			Title:       "Malicious",
+			Description: "Template declaring an unsafe workflow dir",
+			Language:    "go",
+			Category:    "workflow",
+			Author:      "Test",
+			License:     "MIT",
+			Networks:    []string{"ethereum-testnet-sepolia"},
+			Workflows: []templaterepo.WorkflowDirEntry{
+				{Dir: "safe", Description: "safe"},
+				{Dir: dir, Description: "unsafe"},
+			},
+		},
+		Path:   "starter-templates/malicious",
+		Source: templaterepo.RepoSource{Owner: "test", Repo: "templates", Ref: "main"},
+	}
+}
+
+func TestTemplateWorkflowDirIsValidated(t *testing.T) {
+	tests := []struct {
+		name string
+		dir  string
+	}{
+		{name: "parent traversal", dir: "../escape"},
+		{name: "absolute path", dir: "/tmp/escape"},
+		{name: "nested path", dir: "a/b"},
+		{name: "double quote breaks yaml scalar", dir: `a" injected: true`},
+		{name: "newline", dir: "a\ninjected: true"},
+		{name: "command substitution", dir: "a$(id)"},
+		{name: "empty", dir: ""},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			sim := chainsim.NewSimulatedEnvironment(t)
+			defer sim.Close()
+
+			tempDir := t.TempDir()
+			restoreCwd, err := testutil.ChangeWorkingDirectory(tempDir)
+			require.NoError(t, err)
+			defer restoreCwd()
+
+			const templateName = "malicious-template"
+			registry := &mockRegistry{
+				templates: []templaterepo.TemplateSummary{maliciousDirTemplate(templateName, tt.dir)},
+			}
+
+			inputs := Inputs{
+				ProjectName:        "victimProj",
+				TemplateName:       templateName,
+				WorkflowName:       "",
+				RpcURLs:            map[string]string{"ethereum-testnet-sepolia": "https://rpc.example.com"},
+				DeploymentRegistry: "private",
+			}
+
+			h := newHandlerWithRegistry(sim.NewRuntimeContext(), registry)
+			require.NoError(t, h.ValidateInputs(inputs))
+
+			err = h.Execute(inputs)
+			require.Error(t, err, "template dir %q should be rejected", tt.dir)
+			require.Contains(t, err.Error(), "invalid workflow directory")
+
+			// Nothing may be written, in or out of the project root: the check runs before
+			// the project directory is created and before the template is scaffolded.
+			require.False(t, pathExistsForTest(filepath.Join(tempDir, "victimProj")),
+				"project root should not be created when the template is rejected")
+			entries, err := os.ReadDir(tempDir)
+			require.NoError(t, err)
+			require.Empty(t, entries, "no files should be written outside the project root")
+		})
+	}
+}
+
+func TestTemplateWorkflowDirsAcceptValidNames(t *testing.T) {
+	t.Parallel()
+
+	tmpl := testMultiWorkflowTemplate
+	require.NoError(t, validateTemplateWorkflowDirs(&tmpl),
+		"the multi-workflow fixture declares valid dirs and must keep loading")
+
+	single := testGoTemplate
+	require.NoError(t, validateTemplateWorkflowDirs(&single))
+}

cmd/workflow/deploy/deploy.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"strings"
 
 	"github.com/rs/zerolog"
 	"github.com/spf13/cobra"
@@ -38,7 +39,7 @@ type Inputs struct {
 	OutputPath   string `validate:"omitempty,filepath,ascii,max=97" cli:"--output"`
 	WasmPath     string `validate:"omitempty,file,ascii,max=2048" cli:"--wasm"`
 
-	OwnerLabel       string `validate:"omitempty"`
+	OwnerLabel       string `validate:"omitempty,owner_label" cli:"--owner-label"`
 	SkipConfirmation bool
 	NonInteractive   bool
 	// SkipTypeChecks passes --skip-type-checks to cre-compile for TypeScript workflows.
@@ -163,7 +164,7 @@ func (h *handler) ResolveInputs(v *viper.Viper) (Inputs, error) {
 		OutputPath: v.GetString("output"),
 		WasmPath:   v.GetString("wasm"),
 
-		OwnerLabel:       v.GetString("owner-label"),
+		OwnerLabel:       strings.TrimSpace(v.GetString("owner-label")),
 		SkipConfirmation: v.GetBool(settings.Flags.SkipConfirmation.Name),
 		NonInteractive:   v.GetBool(settings.Flags.NonInteractive.Name),
 		SkipTypeChecks:   v.GetBool(cmdcommon.SkipTypeChecksCLIFlag),

cmd/workflow/deploy/deploy_test.go

@@ -100,6 +100,30 @@ func TestWorkflowDeployCommand(t *testing.T) {
 				wantKey:    "Inputs.DonFamily",
 				wantDetail: "DonFamily is a required field",
 			},
+			{
+				name: "Owner Label With Quote Breaks Out Of YAML Scalar",
+				inputs: Inputs{
+					WorkflowName:  "valid_workflow",
+					WorkflowOwner: chainsim.TestAddress,
+					DonFamily:     "test_label",
+					OwnerLabel:    `bad"; echo pwned`,
+				},
+				wantErr:    true,
+				wantKey:    "Inputs.OwnerLabel",
+				wantDetail: `--owner-label must be non-empty, no longer than 64 characters, start with a letter or number, and contain only letters (a-z, A-Z), numbers (0-9), spaces, dots (.), dashes (-), and underscores (_): bad"; echo pwned`,
+			},
+			{
+				name: "Owner Label With Command Substitution",
+				inputs: Inputs{
+					WorkflowName:  "valid_workflow",
+					WorkflowOwner: chainsim.TestAddress,
+					DonFamily:     "test_label",
+					OwnerLabel:    "bad$(id)",
+				},
+				wantErr:    true,
+				wantKey:    "Inputs.OwnerLabel",
+				wantDetail: "--owner-label must be non-empty, no longer than 64 characters, start with a letter or number, and contain only letters (a-z, A-Z), numbers (0-9), spaces, dots (.), dashes (-), and underscores (_): bad$(id)",
+			},
 			{
 				name: "Invalid Binary URL",
 				inputs: Inputs{

cmd/workflow/simulate/simulate.go

@@ -54,7 +54,7 @@ type Inputs struct {
 	SecretsPath  string `validate:"omitempty,file,ascii,max=97"`
 	EngineLogs   bool   `validate:"omitempty" cli:"--engine-logs"`
 	Broadcast    bool   `validate:"-"`
-	WorkflowName string `validate:"required"`
+	WorkflowName string `validate:"required,workflow_name"`
 	// Chain-type-specific fields
 	ChainTypeClients map[string]map[uint64]chain.ChainClient `validate:"omitempty"`
 	ChainTypeKeys    map[string]interface{}                  `validate:"-"`

internal/settings/workflow_settings.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/smartcontractkit/cre-cli/internal/constants"
 	"github.com/smartcontractkit/cre-cli/internal/rpc"
+	"github.com/smartcontractkit/cre-cli/internal/validation"
 )
 
 // GetWorkflowPathFromFile reads workflow-path from a workflow.yaml file (same value deploy/simulate get from Settings).
@@ -137,6 +138,13 @@ func loadWorkflowSettings(logger *zerolog.Logger, v *viper.Viper, cmd *cobra.Com
 	}
 
 	workflowSettings.UserWorkflowSettings.WorkflowName = getSetting(WorkflowNameSettingName)
+	// Validate the name here rather than per-command so every consumer of the setting inherits
+	// the check. Only enforced when set, so commands that run without a workflow.yaml still work.
+	if name := workflowSettings.UserWorkflowSettings.WorkflowName; name != "" {
+		if err := validation.IsValidWorkflowName(name); err != nil {
+			return WorkflowSettings{}, errors.Wrapf(err, "invalid %s in workflow.yaml for target %q", WorkflowNameSettingName, target)
+		}
+	}
 	workflowSettings.WorkflowArtifactSettings.WorkflowPath = getSetting(WorkflowPathSettingName)
 	workflowSettings.WorkflowArtifactSettings.ConfigPath = getSetting(ConfigPathSettingName)
 	workflowSettings.WorkflowArtifactSettings.SecretsPath = getSetting(SecretsPathSettingName)