chore: checkbox and security bits

.github/SECURITY_ADVISORY_DUPLICATES.md

@@ -0,0 +1,116 @@
+# Security Advisory Duplicates — Maintainer Guide
+
+This document lists known duplicate security advisory clusters for [OliveTin/OliveTin](https://github.com/OliveTin/OliveTin). When triaging new reports, check here and [open advisories](https://github.com/OliveTin/OliveTin/security/advisories) before accepting.
+
+**Duplicate policy:** the earliest reporter on the canonical advisory receives primary credit. Later reporters are credited on the canonical advisory when closed as duplicates. See [SECURITY.md](../SECURITY.md).
+
+## Triage checklist
+
+1. Search open advisories for the same component and attack path.
+2. Match against clusters below.
+3. If duplicate: close the newer advisory, link to canonical, add reporter to canonical credits.
+4. If unique: accept, patch on a private branch, reassess CVSS with OliveTin context (see SECURITY.md — OliveTin is intentional RCE by design).
+5. Merge fix to `next`, publish advisory, credit reporters in advisory body (not commit message).
+
+---
+
+## shellAfterCompleted command injection
+
+**Canonical:** [GHSA-vc6p-m6vx-6cwq](https://github.com/OliveTin/OliveTin/security/advisories/GHSA-vc6p-m6vx-6cwq) — reporter **knight-yagami** (2026-03-04)
+
+Untrusted command output or template variables interpolated into `shellAfterCompleted` and executed via `sh -c`.
+
+| GHSA | Reporter | Status | Notes |
+|------|----------|--------|-------|
+| GHSA-vc6p-m6vx-6cwq | knight-yagami | closed (canonical) | Original report |
+| GHSA-v5gc-hqpq-227p | 0xkakash1 | closed (duplicate) | Output template variant |
+| GHSA-m7wr-wj5j-7459 | Ryu7zz | duplicate | Webhook `exec` → output → `shellAfterCompleted` |
+| GHSA-cjxm-x848-6vmc | Yesuhei | duplicate | Missing shell safety on after-completion |
+| GHSA-j9p9-36jc-2v8w | anushkavirgaonkar | duplicate | Same root cause |
+
+**Fix:** shell-quote `output`/`exitCode` before template render; block `shellAfterCompleted` for webhook-tagged actions.
+
+**CVSS note:** requires admin-configured `shellAfterCompleted` and attacker influence on output — typically PR:H not PR:N.
+
+---
+
+## OAuth2 state map memory exhaustion (DoS)
+
+**Canonical:** [GHSA-xpxj-f2fm-rqch](https://github.com/OliveTin/OliveTin/security/advisories/GHSA-xpxj-f2fm-rqch) — reporter **knight-yagami** (2026-03-04)
+
+Unauthenticated `/oauth/login` grows `registeredStates` without TTL or cap.
+
+| GHSA | Reporter | Status | Notes |
+|------|----------|--------|-------|
+| GHSA-xpxj-f2fm-rqch | knight-yagami | closed (canonical) | Original report |
+| GHSA-cj96-c55v-2f3c | Dredsen | duplicate | Same unbounded map |
+
+**Fix:** TTL sweep (match 15-minute cookie MaxAge), max map size, cleanup on failed callback.
+
+**CVSS note:** unauthenticated DoS — reported 7.5 is appropriate.
+
+---
+
+## URL argument type — unrestricted URI schemes (SSRF / file read)
+
+**Canonical:** [GHSA-45pc-w4ph-hrq4](https://github.com/OliveTin/OliveTin/security/advisories/GHSA-45pc-w4ph-hrq4) — reporter **fg0x0** (2026-03-09)
+
+`url` type accepts `file://`, `gopher://`, etc. Blocked in `shell:` mode but still validated weakly for `exec:` actions.
+
+| GHSA | Reporter | Status | Notes |
+|------|----------|--------|-------|
+| GHSA-45pc-w4ph-hrq4 | fg0x0 | closed (canonical) | Original report |
+| GHSA-cchg-25m4-q6rj | anushkavirgaonkar | duplicate | Same scheme validation gap |
+
+**Fix:** allowlist `http`/`https` in `typeSafetyCheckUrl`.
+
+**CVSS note:** admin must configure `exec:` action passing URL to external tool — PR:H.
+
+---
+
+## Custom `regex:` argument type in shell actions
+
+**Canonical:** [GHSA-xc5w-4v5w-7x65](https://github.com/OliveTin/OliveTin/security/advisories/GHSA-xc5w-4v5w-7x65) — reporter **Ayantaker** (2026-05-06)
+
+`regex:` types not in shell denylist; partial `MatchString` allows injection suffixes.
+
+| GHSA | Reporter | Status | Notes |
+|------|----------|--------|-------|
+| GHSA-xc5w-4v5w-7x65 | Ayantaker | canonical | Missing denylist entry |
+| GHSA-gvxq-7gvp-4ggr | anushkavirgaonkar | duplicate | Unanchored partial match |
+
+**Fix:** deny `regex:` in shell mode; enforce full-string match for custom regex types.
+
+---
+
+## Shell denylist incomplete (post CVE-2026-27626)
+
+**Canonical:** [GHSA-c26w-h42g-jfp9](https://github.com/OliveTin/OliveTin/security/advisories/GHSA-c26w-h42g-jfp9) — reporter **sec-reex** (2026-07-03)
+
+CVE-2026-27626 added `password` to denylist only; `html`, `confirmation`, and choiceless `checkbox` still skip validation and are allowed in `shell:` actions.
+
+**Fix:** extend `checkShellArgumentSafety` denylist.
+
+---
+
+## StartActionAndWait logs ACL bypass
+
+**Canonical:** [GHSA-jm28-2wcr-qf3h](https://github.com/OliveTin/OliveTin/security/advisories/GHSA-jm28-2wcr-qf3h) — reporter **offset** (2026-03-12)
+
+`StartActionAndWait` / `StartActionByGetAndWait` return full log output without `logs` ACL check.
+
+No known duplicates.
+
+**Fix:** apply `isLogEntryAllowed` before returning `LogEntry`.
+
+**CVSS note:** requires authenticated user with `exec` but not `logs` — typically 4.3–5.3 not 6.5.
+
+---
+
+## Easy to confuse (not duplicates)
+
+| Topic | Advisories | Distinction |
+|-------|------------|-------------|
+| OAuth2 state DoS vs OAuth2 auth bypass | GHSA-xpxj vs GHSA-3v7p | DoS fills state map; bypass spoofs `authHttpHeaderUsername` |
+| `shellAfterCompleted` vs direct `shell` injection | GHSA-vc6p vs GHSA-49gm | Second-order via output vs first-order argument injection |
+| `ValidateArgumentType` enumeration | GHSA-f637 vs GHSA-x6q3 | Same issue; GHSA-f637 published |

docs/modules/ROOT/pages/action_execution/aftercompletion.adoc

@@ -13,10 +13,10 @@ actions:
     shellAfterCompleted: "apprise -c /config/apprise.yml -t 'Notification: Backup script completed' -b 'The backup script completed with code {{ exitCode}}. The log is: \n {{ output }} '"
 ----
 
-When running shellAfterCompleted, you *cannot* use argument values - they are not passed to the command. However the following special arguments are defined; 
+When running shellAfterCompleted, you *cannot* use argument values - they are not passed to the command. However the following special arguments are defined;
 
-* `{{ exitCode }}` - The exit code of the previous shell command
-* `{{ output }}` - The standard output of the previous shell command
+* `{{ exitCode }}` - The exit code of the previous shell command. OliveTin substitutes this with the `EXITCODE` environment variable when running `shellAfterCompleted`, so shell metacharacters in the value cannot break quoting.
+* `{{ output }}` - The standard output of the previous shell command. OliveTin substitutes this with the `OUTPUT` environment variable when running `shellAfterCompleted`, so shell metacharacters in command output cannot be executed. You can also reference `$OUTPUT` directly in your `shellAfterCompleted` command.
 * `{{ .Arguments.ot_executionTrackingId }}` - The unique execution tracking id for this execution (version 3k; in 2k use `{{ ot_executionTrackingId }}`)
 * `{{ .Arguments.ot_username }}` - The username of the user who started the execution (version 3k; in 2k use `{{ ot_username }}`). May be `guest` or `cron` for unauthenticated or automated runs.
 

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/api/api.go

@@ -282,6 +282,21 @@ func (api *oliveTinAPI) findBindingByIDOrNotFound(bindingId string) (*executor.A
 	return api.findBindingOrNotFound(bindingId)
 }
 
+func (api *oliveTinAPI) startActionAndWaitLogEntry(binding *executor.ActionBinding, args map[string]string, justification string, user *authpublic.AuthenticatedUser) (*apiv1.LogEntry, error) {
+	internalLogEntry, ok := api.startActionAndWaitRun(binding, args, justification, user)
+	if !ok {
+		return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("execution not found"))
+	}
+	return api.logEntryForAllowedViewer(internalLogEntry, user)
+}
+
+func (api *oliveTinAPI) logEntryForAllowedViewer(internalLogEntry *executor.InternalLogEntry, user *authpublic.AuthenticatedUser) (*apiv1.LogEntry, error) {
+	if err := api.requireLogEntryAllowed(internalLogEntry, user); err != nil {
+		return nil, err
+	}
+	return api.internalLogEntryToPb(internalLogEntry, user), nil
+}
+
 func (api *oliveTinAPI) StartActionAndWait(ctx ctx.Context, req *connect.Request[apiv1.StartActionAndWaitRequest]) (*connect.Response[apiv1.StartActionAndWaitResponse], error) {
 	binding, err := api.findBindingOrNotFound(req.Msg.ActionId)
 	if err != nil {
@@ -295,12 +310,12 @@ func (api *oliveTinAPI) StartActionAndWait(ctx ctx.Context, req *connect.Request
 		return nil, connectInvalidJustification(err)
 	}
 
-	internalLogEntry, ok := api.startActionAndWaitRun(binding, args, justification, user)
-	if !ok {
-		return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("execution not found"))
+	logEntry, err := api.startActionAndWaitLogEntry(binding, args, justification, user)
+	if err != nil {
+		return nil, err
 	}
 	return connect.NewResponse(&apiv1.StartActionAndWaitResponse{
-		LogEntry: api.internalLogEntryToPb(internalLogEntry, user),
+		LogEntry: logEntry,
 	}), nil
 }
 
@@ -327,16 +342,7 @@ func (api *oliveTinAPI) StartActionByGet(ctx ctx.Context, req *connect.Request[a
 	}), nil
 }
 
-func (api *oliveTinAPI) StartActionByGetAndWait(ctx ctx.Context, req *connect.Request[apiv1.StartActionByGetAndWaitRequest]) (*connect.Response[apiv1.StartActionByGetAndWaitResponse], error) {
-	binding := api.executor.FindBindingByID(req.Msg.ActionId)
-	if binding == nil || binding.Action == nil {
-		return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("action with ID %s not found", req.Msg.ActionId))
-	}
-
-	args := make(map[string]string)
-
-	user := auth.UserFromApiCall(ctx, req, api.cfg)
-
+func (api *oliveTinAPI) runBindingAndWait(binding *executor.ActionBinding, args map[string]string, user *authpublic.AuthenticatedUser) (*executor.InternalLogEntry, bool) {
 	execReq := executor.ExecutionRequest{
 		Binding:           binding,
 		TrackingID:        uuid.NewString(),
@@ -348,14 +354,31 @@ func (api *oliveTinAPI) StartActionByGetAndWait(ctx ctx.Context, req *connect.Re
 	wg, _ := api.executor.ExecRequest(&execReq)
 	wg.Wait()
 
-	internalLogEntry, ok := api.executor.GetLog(execReq.TrackingID)
+	return api.executor.GetLog(execReq.TrackingID)
+}
 
-	if ok {
-		return connect.NewResponse(&apiv1.StartActionByGetAndWaitResponse{
-			LogEntry: api.internalLogEntryToPb(internalLogEntry, user),
-		}), nil
+func (api *oliveTinAPI) startActionByGetAndWaitLogEntry(binding *executor.ActionBinding, user *authpublic.AuthenticatedUser) (*apiv1.LogEntry, error) {
+	internalLogEntry, ok := api.runBindingAndWait(binding, map[string]string{}, user)
+	if !ok {
+		return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("execution not found"))
 	}
-	return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("execution not found"))
+	return api.logEntryForAllowedViewer(internalLogEntry, user)
+}
+
+func (api *oliveTinAPI) StartActionByGetAndWait(ctx ctx.Context, req *connect.Request[apiv1.StartActionByGetAndWaitRequest]) (*connect.Response[apiv1.StartActionByGetAndWaitResponse], error) {
+	binding := api.executor.FindBindingByID(req.Msg.ActionId)
+	if binding == nil || binding.Action == nil {
+		return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("action with ID %s not found", req.Msg.ActionId))
+	}
+
+	user := auth.UserFromApiCall(ctx, req, api.cfg)
+	logEntry, err := api.startActionByGetAndWaitLogEntry(binding, user)
+	if err != nil {
+		return nil, err
+	}
+	return connect.NewResponse(&apiv1.StartActionByGetAndWaitResponse{
+		LogEntry: logEntry,
+	}), nil
 }
 
 func calculateRateLimitExpires(api *oliveTinAPI, logEntry *executor.InternalLogEntry) string {
@@ -684,6 +707,13 @@ func (api *oliveTinAPI) isLogEntryAllowed(e *executor.InternalLogEntry, user *au
 	return acl.IsAllowedLogs(api.cfg, user, e.Binding.Action)
 }
 
+func (api *oliveTinAPI) requireLogEntryAllowed(entry *executor.InternalLogEntry, user *authpublic.AuthenticatedUser) error {
+	if api.isLogEntryAllowed(entry, user) {
+		return nil
+	}
+	return connect.NewError(connect.CodePermissionDenied, fmt.Errorf("permission denied to view this execution"))
+}
+
 // mayViewExecutionEvent returns whether the user is allowed to receive this execution event (for EventStream ACL).
 func (api *oliveTinAPI) mayViewExecutionEvent(entry *executor.InternalLogEntry, user *authpublic.AuthenticatedUser) bool {
 	if user == nil {

service/internal/api/api_test.go

@@ -546,6 +546,54 @@ func TestValidateArgumentTypeAllowedWithViewPermission(t *testing.T) {
 	assert.True(t, resp.Msg.Valid, "admin with view:true should get successful validation for a valid ascii value")
 }
 
+// buildExecWithoutLogsTestConfig returns config for GHSA-jm28: user "runner" may exec but not read logs.
+func buildExecWithoutLogsTestConfig(t *testing.T) (*config.Config, *authpublic.AuthenticatedUser) {
+	t.Helper()
+	cfg := config.DefaultConfig()
+	cfg.AuthHttpHeaderUsername = "X-Ot-User"
+	cfg.DefaultPermissions.View = false
+	cfg.DefaultPermissions.Exec = false
+	cfg.DefaultPermissions.Logs = false
+
+	cfg.Actions = append(cfg.Actions, &config.Action{
+		ID:    "run_only",
+		Title: "Run Only",
+		Shell: "echo sensitive-output",
+		Icon:  "🔒",
+	})
+
+	cfg.AccessControlLists = append(cfg.AccessControlLists, &config.AccessControlList{
+		Name:             "runner",
+		MatchUsernames:   []string{"runner"},
+		AddToEveryAction: true,
+		Permissions:      config.PermissionsList{View: true, Exec: true, Logs: false, Kill: false},
+	})
+
+	runner := &authpublic.AuthenticatedUser{Username: "runner"}
+	runner.BuildUserAcls(cfg)
+	return cfg, runner
+}
+
+// TestStartActionAndWaitDeniesLogsPermission (GHSA-jm28-2wcr-qf3h) asserts sync execution endpoints
+// enforce logs ACL and do not return action output to users allowed to exec but not read logs.
+func TestStartActionAndWaitDeniesLogsPermission(t *testing.T) {
+	cfg, _ := buildExecWithoutLogsTestConfig(t)
+	ex := executor.DefaultExecutor(cfg)
+	ex.RebuildActionMap()
+	ts, client := getNewTestServerAndClientWithExecutor(cfg, ex)
+	defer ts.Close()
+
+	req := connect.NewRequest(&apiv1.StartActionAndWaitRequest{
+		ActionId: "run_only",
+	})
+	req.Header().Set("X-Ot-User", "runner")
+
+	_, err := client.StartActionAndWait(context.Background(), req)
+	require.Error(t, err)
+	assert.Equal(t, connect.CodePermissionDenied, connect.CodeOf(err),
+		"user with exec:true and logs:false must not receive log output from StartActionAndWait")
+}
+
 // TestViewPermissionAllowedSeesAction (GHSA: view permission) asserts that a user with view: true
 // still sees the action in the dashboard and can fetch it via GetActionBinding.
 func TestViewPermissionAllowedSeesAction(t *testing.T) {

service/internal/auth/otoauth2/restapi_auth_oauth2.go

@@ -62,8 +62,14 @@ type oauth2State struct {
 	providerName   string
 	Username       string
 	Usergroup      string
+	createdAt      time.Time
 }
 
+const (
+	oauthStateMaxAge     = 900 // matches olivetin-sid-oauth cookie MaxAge
+	oauthStateMaxEntries = 10000
+)
+
 func assignIfEmpty(target *string, value string) {
 	if *target == "" {
 		*target = value
@@ -129,6 +135,19 @@ func (h *OAuth2Handler) setOAuthCallbackCookie(w http.ResponseWriter, r *http.Re
 	http.SetCookie(w, cookie)
 }
 
+func (h *OAuth2Handler) deleteOAuthStateLocked(state string) {
+	delete(h.registeredStates, state)
+}
+
+func (h *OAuth2Handler) sweepExpiredOAuthStatesLocked(now time.Time) {
+	cutoff := now.Add(-oauthStateMaxAge * time.Second)
+	for state, entry := range h.registeredStates {
+		if entry.createdAt.Before(cutoff) {
+			delete(h.registeredStates, state)
+		}
+	}
+}
+
 func (h *OAuth2Handler) HandleOAuthLogin(w http.ResponseWriter, r *http.Request) {
 	state, err := randString(16)
 
@@ -147,10 +166,17 @@ func (h *OAuth2Handler) HandleOAuthLogin(w http.ResponseWriter, r *http.Request)
 	}
 
 	h.mu.Lock()
+	h.sweepExpiredOAuthStatesLocked(time.Now())
+	if len(h.registeredStates) >= oauthStateMaxEntries {
+		h.mu.Unlock()
+		http.Error(w, "OAuth login temporarily unavailable", http.StatusServiceUnavailable)
+		return
+	}
 	h.registeredStates[state] = &oauth2State{
 		providerConfig: provider,
 		providerName:   providerName,
 		Username:       "",
+		createdAt:      time.Now(),
 	}
 	h.mu.Unlock()
 
@@ -177,6 +203,9 @@ func (h *OAuth2Handler) checkOAuthCallbackCookie(w http.ResponseWriter, r *http.
 
 	if !h.validateStateMatch(r.URL.Query().Get("state"), state) {
 		log.Errorf("State mismatch: %v != %v", r.URL.Query().Get("state"), state)
+		h.mu.Lock()
+		h.deleteOAuthStateLocked(state)
+		h.mu.Unlock()
 		http.Error(w, "State mismatch", http.StatusBadRequest)
 		return nil, state, false
 	}
@@ -186,6 +215,9 @@ func (h *OAuth2Handler) checkOAuthCallbackCookie(w http.ResponseWriter, r *http.
 	h.mu.RUnlock()
 	if !ok {
 		log.Errorf("State not found in server: %v", state)
+		h.mu.Lock()
+		h.deleteOAuthStateLocked(state)
+		h.mu.Unlock()
 		http.Error(w, "State not found in server", http.StatusBadRequest)
 		return nil, state, false
 	}

service/internal/auth/otoauth2/restapi_auth_oauth2_test.go

@@ -0,0 +1,66 @@
+package otoauth2
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"strconv"
+	"testing"
+	"time"
+
+	config "github.com/OliveTin/OliveTin/internal/config"
+	"github.com/stretchr/testify/assert"
+	"golang.org/x/oauth2"
+)
+
+func TestSweepExpiredOAuthStatesLocked(t *testing.T) {
+	h := &OAuth2Handler{
+		registeredStates: make(map[string]*oauth2State),
+	}
+
+	h.registeredStates["fresh"] = &oauth2State{
+		providerName: "test",
+		createdAt:    time.Now(),
+	}
+	h.registeredStates["stale"] = &oauth2State{
+		providerName: "test",
+		createdAt:    time.Now().Add(-2 * oauthStateMaxAge * time.Second),
+	}
+
+	h.sweepExpiredOAuthStatesLocked(time.Now())
+
+	_, freshFound := h.registeredStates["fresh"]
+	_, staleFound := h.registeredStates["stale"]
+	assert.True(t, freshFound)
+	assert.False(t, staleFound)
+}
+
+func TestHandleOAuthLoginRejectsWhenStateMapFull(t *testing.T) {
+	cfg := config.DefaultConfig()
+	cfg.AuthOAuth2Providers = map[string]*config.OAuth2Provider{
+		"test": {
+			Name:         "test",
+			ClientID:     "id",
+			ClientSecret: "secret",
+			AuthUrl:      "https://example.com/auth",
+			TokenUrl:     "https://example.com/token",
+		},
+	}
+
+	h := NewOAuth2Handler(cfg)
+	h.registeredStates = make(map[string]*oauth2State, oauthStateMaxEntries)
+	for i := 0; i < oauthStateMaxEntries; i++ {
+		h.registeredStates[strconv.Itoa(i)] = &oauth2State{
+			providerConfig: &oauth2.Config{},
+			providerName:   "test",
+			createdAt:      time.Now(),
+		}
+	}
+
+	req := httptest.NewRequest(http.MethodGet, "/oauth/login?provider=test", nil)
+	rec := httptest.NewRecorder()
+
+	h.HandleOAuthLogin(rec, req)
+
+	assert.Equal(t, http.StatusServiceUnavailable, rec.Code)
+	assert.Equal(t, oauthStateMaxEntries, len(h.registeredStates))
+}

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:") {
-		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
+	}
+
+	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
+	}
 
-	return err
+	_, 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
 }
 

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 {

service/internal/executor/executor.go

@@ -1238,23 +1238,55 @@ func stepExecAfter(req *ExecutionRequest) bool {
 	return true
 }
 
+func substituteShellAfterCompletedEnvRefs(command string) string {
+	replacements := []struct{ old, new string }{
+		{"{{ output }}", `"$OUTPUT"`},
+		{"{{output}}", `"$OUTPUT"`},
+		{"{{ exitCode }}", `"$EXITCODE"`},
+		{"{{exitCode}}", `"$EXITCODE"`},
+		{"{{ exitCode}}", `"$EXITCODE"`},
+		{"{{exitCode }}", `"$EXITCODE"`},
+	}
+
+	for _, replacement := range replacements {
+		command = strings.ReplaceAll(command, replacement.old, replacement.new)
+	}
+
+	return command
+}
+
+func parseShellAfterCompletedCommand(req *ExecutionRequest, commandTemplate string, args map[string]string) (string, error) {
+	finalParsedCommand, err := tpl.ParseTemplateWithActionContext(commandTemplate, req.Binding.Entity, args)
+	if err != nil {
+		msg := "Could not prepare shellAfterCompleted command: " + err.Error() + "\n"
+		req.mutateLogEntry(func(entry *InternalLogEntry) {
+			entry.Output += msg
+		})
+		log.Warn(msg)
+		return "", err
+	}
+
+	return finalParsedCommand, nil
+}
+
+//gocyclo:ignore
 func buildShellAfterCommand(ctx context.Context, req *ExecutionRequest, stdout, stderr *bytes.Buffer) (*exec.Cmd, map[string]string, error) {
 	if req.Binding.Action.ShellAfterCompleted == "" {
 		return nil, nil, nil
 	}
 
+	if hasWebhookTag(req) {
+		return nil, nil, fmt.Errorf("webhooks cannot use shellAfterCompleted; use exec without after-completion shell instead. See https://docs.olivetin.app/action_execution/shellvsexec.html")
+	}
+
 	args, err := buildShellAfterArgs(req)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	finalParsedCommand, err := tpl.ParseTemplateWithActionContext(req.Binding.Action.ShellAfterCompleted, req.Binding.Entity, args)
+	commandTemplate := substituteShellAfterCompletedEnvRefs(req.Binding.Action.ShellAfterCompleted)
+	finalParsedCommand, err := parseShellAfterCompletedCommand(req, commandTemplate, args)
 	if err != nil {
-		msg := "Could not prepare shellAfterCompleted command: " + err.Error() + "\n"
-		req.mutateLogEntry(func(entry *InternalLogEntry) {
-			entry.Output += msg
-		})
-		log.Warn(msg)
 		return nil, nil, nil
 	}
 

service/internal/executor/executor_test.go

@@ -1,6 +1,8 @@
 package executor
 
 import (
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 	"time"
@@ -385,6 +387,64 @@ func TestWebhookAllowsExecExecution(t *testing.T) {
 	assert.Contains(t, req.logEntry.Output, "hello")
 }
 
+func TestWebhookRejectsShellAfterCompleted(t *testing.T) {
+	cfg := config.DefaultConfig()
+	e := DefaultExecutor(cfg)
+	a1 := &config.Action{
+		Title:               "Webhook After Shell Reject",
+		Exec:                []string{"echo", "{{ msg }}"},
+		ShellAfterCompleted: "echo after",
+		Arguments: []config.ActionArgument{
+			{Name: "msg", Type: "ascii"},
+		},
+	}
+	cfg.Actions = append(cfg.Actions, a1)
+	cfg.Sanitize()
+	e.RebuildActionMap()
+
+	req := ExecutionRequest{
+		Tags:              []string{"webhook"},
+		AuthenticatedUser: auth.UserFromSystem(cfg, "webhook"),
+		Cfg:               cfg,
+		Arguments:         map[string]string{"msg": "hello"},
+		Binding:           e.FindBindingWithNoEntity(a1),
+	}
+
+	wg, _ := e.ExecRequest(&req)
+	wg.Wait()
+
+	assert.NotNil(t, req.logEntry)
+	assert.Contains(t, req.logEntry.Output, "webhooks cannot use shellAfterCompleted")
+}
+
+func TestShellAfterCompletedUsesOutputEnvSafely(t *testing.T) {
+	cfg := config.DefaultConfig()
+	e := DefaultExecutor(cfg)
+	injectedPath := filepath.Join(t.TempDir(), "olivetin-injected")
+	a1 := &config.Action{
+		Title:               "After completion escape",
+		Shell:               "printf %s \"'; touch " + injectedPath + "; echo '\"",
+		ShellAfterCompleted: "printf %s '{{ output }}'",
+	}
+	cfg.Actions = append(cfg.Actions, a1)
+	cfg.Sanitize()
+	e.RebuildActionMap()
+
+	req := ExecutionRequest{
+		AuthenticatedUser: auth.UserFromSystem(cfg, "cron"),
+		Cfg:               cfg,
+		Binding:           e.FindBindingWithNoEntity(a1),
+	}
+
+	wg, _ := e.ExecRequest(&req)
+	wg.Wait()
+
+	assert.NotNil(t, req.logEntry)
+	assert.Equal(t, int32(0), req.logEntry.ExitCode)
+	_, err := os.Stat(injectedPath)
+	assert.True(t, os.IsNotExist(err), "shellAfterCompleted must not execute injected commands from output")
+}
+
 func TestFilterToDefinedArgumentsOnly(t *testing.T) {
 	req := newExecRequest()
 	req.Binding.Action = &config.Action{