Merge commit from fork

security: GHSA-xc5w-4v5w-7x65 (HIGH) harden shell argument type safety

integration-tests/tests/checkbox/config.yaml

@@ -7,7 +7,9 @@ defaultPopupOnStart: execution-dialog
 
 actions:
   - title: Test checkbox argument
-    shell: "echo 'Checkbox value: {{ confirm }}'"
+    exec:
+      - echo
+      - "Checkbox value: {{ confirm }}"
     icon: ping
     arguments:
       - name: confirm
@@ -15,5 +17,3 @@ actions:
         type: checkbox
         description: "When checked: 1, when unchecked: 0"
         default: false
-
-

service/internal/executor/arguments.go

@@ -314,11 +314,21 @@ func typeSafetyCheckDatetime(value string) error {
 	return nil
 }
 
+func anchorCustomRegexPattern(pattern string) string {
+	if strings.HasPrefix(pattern, "^") && strings.HasSuffix(pattern, "$") {
+		return pattern
+	}
+
+	return "^(?:" + pattern + ")$"
+}
+
 func typeSafetyCheckRegex(name string, value string, argumentType string) error {
 	pattern := ""
+	isCustomRegex := strings.HasPrefix(argumentType, "regex:")
 
-	if strings.HasPrefix(argumentType, "regex:") {
+	if isCustomRegex {
-		pattern = strings.Replace(argumentType, "regex:", "", 1)
+		pattern = strings.TrimPrefix(argumentType, "regex:")
+		pattern = anchorCustomRegexPattern(pattern)
 	} else {
 		found := false
 		pattern, found = typecheckRegex[argumentType]
@@ -345,21 +355,50 @@ func typeSafetyCheckRegex(name string, value string, argumentType string) error
 }
 
 func typeSafetyCheckUrl(value string) error {
-	_, err := url.ParseRequestURI(value)
+	parsed, err := url.ParseRequestURI(value)
-
+	if err != nil {
 		return err
+	}
+
+	scheme := strings.ToLower(parsed.Scheme)
+	if scheme != "http" && scheme != "https" {
+		return fmt.Errorf("url scheme %q is not allowed; only http and https are permitted", parsed.Scheme)
+	}
+
+	return nil
+}
+
+var shellUnsafeArgumentTypes = map[string]struct{}{
+	"url":                       {},
+	"email":                     {},
+	"raw_string_multiline":      {},
+	"very_dangerous_raw_string": {},
+	"password":                  {},
+	"html":                      {},
+	"confirmation":              {},
+}
+
+func isUnsafeShellArgumentType(arg *config.ActionArgument) bool {
+	if strings.HasPrefix(arg.Type, "regex:") {
+		return true
+	}
+
+	_, inMap := shellUnsafeArgumentTypes[arg.Type]
+	return inMap || (arg.Type == "checkbox" && len(arg.Choices) == 0)
 }
 
 func checkShellArgumentSafety(action *config.Action) error {
 	if action.Shell == "" {
 		return nil
 	}
-	unsafe := map[string]struct{}{"url": {}, "email": {}, "raw_string_multiline": {}, "very_dangerous_raw_string": {}, "password": {}}
+
-	for _, arg := range action.Arguments {
+	for i := range action.Arguments {
-		if _, bad := unsafe[arg.Type]; bad {
+		arg := &action.Arguments[i]
+		if isUnsafeShellArgumentType(arg) {
 			return fmt.Errorf("unsafe argument type '%s' cannot be used with Shell execution. Use 'exec' instead. See https://docs.olivetin.app/action_execution/shellvsexec.html", arg.Type)
 		}
 	}
+
 	return nil
 }
 

service/internal/executor/arguments_test.go

@@ -499,13 +499,72 @@ func TestCheckShellArgumentSafetyWithPasswordAndExec(t *testing.T) {
 	assert.Nil(t, err)
 }
 
+func TestCheckShellArgumentSafetyWithHTML(t *testing.T) {
+	a1 := config.Action{
+		Title: "HTML shell",
+		Shell: "echo {{ body }}",
+		Arguments: []config.ActionArgument{
+			{Name: "body", Type: "html"},
+		},
+	}
+
+	err := checkShellArgumentSafety(&a1)
+	assert.NotNil(t, err)
+	assert.Contains(t, err.Error(), "unsafe argument type 'html'")
+}
+
+func TestCheckShellArgumentSafetyWithConfirmation(t *testing.T) {
+	a1 := config.Action{
+		Title: "Confirm shell",
+		Shell: "echo ok",
+		Arguments: []config.ActionArgument{
+			{Name: "agree", Type: "confirmation"},
+		},
+	}
+
+	err := checkShellArgumentSafety(&a1)
+	assert.NotNil(t, err)
+	assert.Contains(t, err.Error(), "unsafe argument type 'confirmation'")
+}
+
+func TestCheckShellArgumentSafetyWithChoicelessCheckbox(t *testing.T) {
+	a1 := config.Action{
+		Title: "Checkbox shell",
+		Shell: "echo {{ flag }}",
+		Arguments: []config.ActionArgument{
+			{Name: "flag", Type: "checkbox"},
+		},
+	}
+
+	err := checkShellArgumentSafety(&a1)
+	assert.NotNil(t, err)
+	assert.Contains(t, err.Error(), "unsafe argument type 'checkbox'")
+}
+
+func TestCheckShellArgumentSafetyWithCustomRegex(t *testing.T) {
+	a1 := config.Action{
+		Title: "Regex shell",
+		Shell: "curl {{ host }}",
+		Arguments: []config.ActionArgument{
+			{Name: "host", Type: "regex:[a-zA-Z0-9.-]+"},
+		},
+	}
+
+	err := checkShellArgumentSafety(&a1)
+	assert.NotNil(t, err)
+	assert.Contains(t, err.Error(), "unsafe argument type 'regex:[a-zA-Z0-9.-]+'")
+}
+
 func TestTypeSafetyCheckUrl(t *testing.T) {
 	assert.Nil(t, TypeSafetyCheck("test1", "http://google.com", "url"), "Test URL: google.com")
 	assert.Nil(t, TypeSafetyCheck("test2", "http://technowax.net:80?foo=bar", "url"), "Test URL: technowax.net with query arguments")
 	assert.Nil(t, TypeSafetyCheck("test3", "http://localhost:80?foo=bar", "url"), "Test URL: localhost with query arguments")
+	assert.Nil(t, TypeSafetyCheck("test7", "https://example.com/path", "url"), "Test URL: https scheme")
 	assert.NotNil(t, TypeSafetyCheck("test4", "http://lo  host:80", "url"), "Test a badly formed URL")
 	assert.NotNil(t, TypeSafetyCheck("test5", "12345", "url"), "Test a badly formed URL")
 	assert.NotNil(t, TypeSafetyCheck("test6", "_!23;", "url"), "Test a badly formed URL")
+	assert.NotNil(t, TypeSafetyCheck("test8", "file:///etc/passwd", "url"), "file:// scheme must be rejected")
+	assert.NotNil(t, TypeSafetyCheck("test9", "gopher://example.com", "url"), "gopher:// scheme must be rejected")
 }
 
 func TestTypeSafetyCheckRegex(t *testing.T) {
@@ -530,6 +589,13 @@ func TestTypeSafetyCheckRegex(t *testing.T) {
 			value:    "James1234",
 			hasError: true,
 		},
+		{
+			name:     "GHSA-gvxq - reject partial regex match",
+			field:    "host",
+			pattern:  "regex:[a-zA-Z0-9.-]+",
+			value:    "example.com; id",
+			hasError: true,
+		},
 	}
 
 	for _, tt := range tests {