security: GHSA-xc5w-4v5w-7x65 (HIGH) harden shell argument type safety
Block additional unvalidated argument types from shell actions, enforce full-string custom regex matching, and allowlist http/https URL schemes. Update checkbox integration test to use exec, matching the intended shell vs exec split for choiceless checkbox arguments. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
ec114e95d2
commit
0e45f3b0e3
|
|
@ -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
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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:") {
|
||||
pattern = strings.Replace(argumentType, "regex:", "", 1)
|
||||
if isCustomRegex {
|
||||
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
|
||||
}
|
||||
|
||||
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 {
|
||||
if _, bad := unsafe[arg.Type]; bad {
|
||||
|
||||
for i := range action.Arguments {
|
||||
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
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
Loading…
Reference in New Issue