feat(templates): survey variables valiation on backend

api-docs.yml

@@ -819,8 +819,16 @@ definitions:
         type: string
       environment:
         type: string
+        description: >
+          JSON object with the values of the template survey variables. Only
+          variables declared in `survey_vars` are accepted, and their values must
+          match the declared type, unless the template enables
+          `allow_any_vars_in_task`.
       secret:
         type: string
+        description: >
+          JSON object with the values of the template survey variables of type
+          `secret`. Write only, never returned by the API.
       arguments:
         type: string
       git_branch:
@@ -947,6 +955,13 @@ definitions:
       allow_override_args_in_task:
         type: boolean
         example: false
+      allow_any_vars_in_task:
+        type: boolean
+        example: false
+        description: >
+          Allows a task to set variables which are not declared in `survey_vars`.
+          Disabled by default: such variables override the template environment
+          and are passed to the app with the highest precedence.
       limit:
         type: string
         example: ''
@@ -1013,6 +1028,13 @@ definitions:
       allow_override_args_in_task:
         type: boolean
         example: false
+      allow_any_vars_in_task:
+        type: boolean
+        example: false
+        description: >
+          Allows a task to set variables which are not declared in `survey_vars`.
+          Disabled by default: such variables override the template environment
+          and are passed to the app with the highest precedence.
       suppress_success_alerts:
         type: boolean
       app:
@@ -3428,6 +3450,12 @@ paths:
                 type: string
               environment:
                 type: string
+                example: '{}'
+                description: >
+                  JSON object with the values of the template survey variables.
+                  Only variables declared in `survey_vars` are accepted, and their
+                  values must match the declared type, unless the template enables
+                  `allow_any_vars_in_task`.
               limit:
                 type: string
               git_branch:

api/projects/tasks.go

@@ -48,6 +48,17 @@ func (c *TaskController) AddTask(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// By default a task may carry only the variables declared in the template
+	// survey, no matter who sends it: task variables override the template
+	// environment and are passed to the app with the highest precedence, so
+	// undeclared keys change settings the template author never exposed — see
+	// db.Task.ValidateSurveyVars. Templates whose tasks legitimately need
+	// arbitrary variables opt in with AllowAnyVarsInTask.
+	if err = taskObj.ValidateSurveyVars(tpl); err != nil {
+		helpers.WriteError(w, err)
+		return
+	}
+
 	newTask, err := taskPool(r).AddTask(
 		taskObj,
 		&user.ID,

db/Migration.go

@@ -133,6 +133,7 @@ func GetMigrations(dialect string) []Migration {
 		{Version: "2.19.11"},
 		{Version: "2.20.0"},
 		{Version: "2.20.1"},
+		{Version: "2.20.2"},
 	}
 
 	return append(initScripts, commonScripts...)

db/Task.go

@@ -7,6 +7,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/semaphoreui/semaphore/pkg/common_errors"
 	"github.com/semaphoreui/semaphore/pkg/git"
 	"github.com/semaphoreui/semaphore/pkg/tz"
 
@@ -197,6 +198,89 @@ func (task *Task) ValidateNewTask(template Template) error {
 	return task.ExtractParams(params)
 }
 
+// parseTaskVars parses a task-supplied variable payload (Task.Environment or
+// Task.Secret) into a map. Numbers are decoded as json.Number so integer survey
+// variables can be checked without float rounding.
+func parseTaskVars(payload string, field string) (map[string]any, error) {
+	res := make(map[string]any)
+
+	if payload == "" {
+		return res, nil
+	}
+
+	dec := json.NewDecoder(strings.NewReader(payload))
+	dec.UseNumber()
+
+	if err := dec.Decode(&res); err != nil {
+		return nil, common_errors.NewValidationError("task " + field + " must be a JSON object")
+	}
+
+	return res, nil
+}
+
+// ValidateSurveyVars ensures that the variables supplied with the task are
+// declared in the template survey and match their declared type.
+//
+// Task variables are merged over the template environment and reach the app as
+// extra variables (--extra-vars for Ansible, -var for Terraform apps, arguments
+// for shell apps), where they take precedence over everything the template
+// author configured. Undeclared keys therefore let the requester change
+// settings the template never exposed — for Ansible that includes connection
+// options such as ansible_ssh_common_args, whose ProxyCommand runs on the
+// Semaphore server itself. Nobody may send them by default, which is also all
+// the UI ever sends; a template opts in to arbitrary variables with
+// AllowAnyVarsInTask.
+func (task *Task) ValidateSurveyVars(template Template) error {
+	if template.AllowAnyVarsInTask {
+		return nil
+	}
+
+	env, err := parseTaskVars(task.Environment, "environment")
+	if err != nil {
+		return err
+	}
+
+	secrets, err := parseTaskVars(task.Secret, "secret")
+	if err != nil {
+		return err
+	}
+
+	for name, value := range env {
+		v := template.GetSurveyVar(name)
+
+		if v == nil {
+			return undeclaredSurveyVarError(name)
+		}
+
+		// Secret variables belong to the secret payload, which is stored
+		// encrypted and masked in logs; accepting them here would persist the
+		// value in plaintext on the task row.
+		if v.Type == SurveyVarSecret {
+			return common_errors.NewValidationError(
+				"survey variable " + name + " must be sent in the task secret")
+		}
+
+		if err = v.ValidateValue(value); err != nil {
+			return err
+		}
+	}
+
+	for name := range secrets {
+		v := template.GetSurveyVar(name)
+
+		if v == nil || v.Type != SurveyVarSecret {
+			return undeclaredSurveyVarError(name)
+		}
+	}
+
+	return nil
+}
+
+func undeclaredSurveyVarError(name string) error {
+	return common_errors.NewValidationError(
+		"variable " + name + " is not declared in the template survey")
+}
+
 func (task *TaskWithTpl) Fill(d Store) error {
 	if task.BuildTaskID != nil {
 		build, err := d.GetTask(task.ProjectID, *task.BuildTaskID)

db/Task_test.go

@@ -0,0 +1,141 @@
+package db
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func surveyTemplate() Template {
+	return Template{
+		SurveyVars: []SurveyVar{
+			{Name: "app_version", Type: SurveyVarStr},
+			{Name: "replicas", Type: SurveyVarInt},
+			{Name: "stage", Type: SurveyVarEnum, Values: []SurveyVarEnumValue{
+				{Name: "Dev", Value: "dev"},
+				{Name: "Prod", Value: "prod"},
+			}},
+			{Name: "notes", Type: SurveyVarText},
+			{Name: "token", Type: SurveyVarSecret},
+		},
+	}
+}
+
+func TestTask_ValidateSurveyVars(t *testing.T) {
+	tests := []struct {
+		name        string
+		environment string
+		secret      string
+		errContains string
+	}{
+		{name: "no variables"},
+		{name: "declared variables", environment: `{"app_version":"1.2.3","replicas":3,"stage":"prod","notes":"hi"}`},
+		{name: "declared secret", secret: `{"token":"s3cret"}`},
+		{name: "integer as string", environment: `{"replicas":"3"}`},
+		{name: "empty value of typed var", environment: `{"replicas":"","stage":""}`},
+		{name: "null value of typed var", environment: `{"replicas":null,"stage":null}`},
+
+		{
+			name:        "undeclared variable",
+			environment: `{"ansible_ssh_common_args":"-o ProxyCommand=/bin/sh"}`,
+			errContains: "ansible_ssh_common_args is not declared",
+		},
+		{
+			name:        "undeclared variable next to a declared one",
+			environment: `{"app_version":"1.2.3","ansible_connection":"local"}`,
+			errContains: "ansible_connection is not declared",
+		},
+		{
+			name:        "undeclared secret",
+			secret:      `{"ansible_ssh_common_args":"-o ProxyCommand=/bin/sh"}`,
+			errContains: "ansible_ssh_common_args is not declared",
+		},
+		{
+			name:        "declared non secret variable sent as secret",
+			secret:      `{"app_version":"1.2.3"}`,
+			errContains: "app_version is not declared",
+		},
+		{
+			name:        "secret variable sent as environment",
+			environment: `{"token":"s3cret"}`,
+			errContains: "token must be sent in the task secret",
+		},
+		{
+			name:        "non integer value",
+			environment: `{"replicas":"3; rm -rf /"}`,
+			errContains: "replicas must be an integer",
+		},
+		{
+			name:        "fractional value of integer var",
+			environment: `{"replicas":1.5}`,
+			errContains: "replicas must be an integer",
+		},
+		{
+			name:        "value outside of enum",
+			environment: `{"stage":"staging"}`,
+			errContains: "stage has a value which is not allowed",
+		},
+		{
+			name:        "environment is not an object",
+			environment: `["app_version"]`,
+			errContains: "task environment must be a JSON object",
+		},
+		{
+			name:        "secret is not an object",
+			secret:      `"token"`,
+			errContains: "task secret must be a JSON object",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			task := Task{Environment: tt.environment, Secret: tt.secret}
+
+			err := task.ValidateSurveyVars(surveyTemplate())
+
+			if tt.errContains == "" {
+				assert.NoError(t, err)
+				return
+			}
+
+			require.Error(t, err)
+			assert.ErrorContains(t, err, tt.errContains)
+		})
+	}
+}
+
+func TestTask_ValidateSurveyVars_TemplateWithoutSurvey(t *testing.T) {
+	task := Task{Environment: `{"anything":"value"}`}
+
+	err := task.ValidateSurveyVars(Template{})
+
+	assert.ErrorContains(t, err, "anything is not declared")
+}
+
+func TestTask_ValidateSurveyVars_AllowAnyVarsInTask(t *testing.T) {
+	task := Task{
+		Environment: `{"ansible_ssh_common_args":"-o ProxyCommand=/bin/sh","replicas":"many"}`,
+		Secret:      `{"undeclared":"value"}`,
+	}
+
+	tpl := surveyTemplate()
+	tpl.AllowAnyVarsInTask = true
+
+	assert.NoError(t, task.ValidateSurveyVars(tpl))
+
+	// The very same task is rejected while the template does not opt in.
+	tpl.AllowAnyVarsInTask = false
+	assert.Error(t, task.ValidateSurveyVars(tpl))
+}
+
+func TestTemplate_GetSurveyVar(t *testing.T) {
+	tpl := surveyTemplate()
+
+	v := tpl.GetSurveyVar("stage")
+
+	require.NotNil(t, v)
+	assert.Equal(t, SurveyVarEnum, v.Type)
+	assert.Nil(t, tpl.GetSurveyVar("STAGE"))
+	assert.Nil(t, tpl.GetSurveyVar("unknown"))
+}