chore: safety improvements on file upload

service/internal/executor/arguments.go

@@ -79,12 +79,19 @@ func parseExecSegment(arg string, templateArgs map[string]any, entity *entities.
 	return tpl.ParseTemplateWithActionContext(arg, entity, templateArgs)
 }
 
+func uploadRegistryFromExecutor(req *ExecutionRequest) *fileupload.Registry {
+	if req == nil || req.executor == nil {
+		return nil
+	}
+	return req.executor.UploadRegistry
+}
+
 func validateArguments(req *ExecutionRequest) error {
-	action := req.Binding.Action
-	if action == nil {
-		return fmt.Errorf("action is nil")
+	action, err := execRequestAction(req)
+	if err != nil {
+		return err
 	}
-	reg := req.executor.UploadRegistry
+	reg := uploadRegistryFromExecutor(req)
 	bindingID := req.Binding.ID
 	for _, arg := range action.Arguments {
 		if err := typecheckActionArgument(&arg, req.Arguments[arg.Name], action, reg, bindingID); err != nil {

service/internal/executor/arguments_upload.go

@@ -81,7 +81,7 @@ func applyConsumedStagedFile(req *ExecutionRequest, arg *config.ActionArgument,
 	req.Arguments[arg.Name] = staged.Path
 	req.FileArgData[arg.Name] = &tpl.FileUpload{
 		TmpName:  staged.Path,
-		Name:     staged.OriginalName,
+		Name:     fileupload.SanitizeUploadFilename(staged.OriginalName),
 		MimeType: staged.MimeType,
 		Size:     staged.Size,
 	}

service/internal/fileupload/registry.go

@@ -1,6 +1,7 @@
 package fileupload
 
 import (
+	"context"
 	"crypto/rand"
 	"encoding/hex"
 	"fmt"
@@ -29,6 +30,9 @@ type Registry struct {
 	pending map[string]*pendingEntry
 	cfg     *config.Config
 	baseDir string
+
+	pruneMu     sync.Mutex
+	pruneCancel context.CancelFunc
 }
 
 type pendingEntry struct {
@@ -59,15 +63,39 @@ func NewRegistry(cfg *config.Config) (*Registry, error) {
 
 // StartPeriodicPrune runs a background loop that removes pending uploads past their TTL.
 // Without this, staged files are only deleted when some other registry operation runs prune.
+// Call Stop to end the loop. Starting again replaces any previous loop.
 func (r *Registry) StartPeriodicPrune() {
-	go r.periodicPruneLoop()
+	ctx, cancel := context.WithCancel(context.Background())
+	r.pruneMu.Lock()
+	if r.pruneCancel != nil {
+		r.pruneCancel()
+	}
+	r.pruneCancel = cancel
+	r.pruneMu.Unlock()
+	go r.periodicPruneLoop(ctx)
+}
+
+// Stop cancels the periodic prune goroutine started by StartPeriodicPrune. It is safe to call
+// multiple times or when pruning was never started.
+func (r *Registry) Stop() {
+	r.pruneMu.Lock()
+	defer r.pruneMu.Unlock()
+	if r.pruneCancel != nil {
+		r.pruneCancel()
+		r.pruneCancel = nil
+	}
 }
 
-func (r *Registry) periodicPruneLoop() {
+func (r *Registry) periodicPruneLoop(ctx context.Context) {
 	ticker := time.NewTicker(30 * time.Second)
 	defer ticker.Stop()
-	for range ticker.C {
-		r.pruneExpired()
+	for {
+		select {
+		case <-ctx.Done():
+			return
+		case <-ticker.C:
+			r.pruneExpired()
+		}
 	}
 }
 
@@ -289,13 +317,35 @@ func newUploadToken() (string, error) {
 	return hex.EncodeToString(b), nil
 }
 
-func sanitizeFilename(name string) string {
-	base := filepath.Base(name)
-	if base == "." || base == string(filepath.Separator) {
+const uploadFilenameSafeRunes = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789.-_"
+
+func finalizeSanitizedUploadFilename(out string) string {
+	if out == "" || out == "." {
 		return "upload"
 	}
-	if len(base) > 255 {
-		base = base[:255]
+	if len(out) > 255 {
+		return out[:255]
 	}
-	return base
+	return out
+}
+
+func sanitizeFilename(name string) string {
+	base := filepath.Base(name)
+	var b strings.Builder
+	b.Grow(len(base))
+	for _, r := range base {
+		if r < 128 && strings.ContainsRune(uploadFilenameSafeRunes, r) {
+			b.WriteByte(byte(r))
+		} else {
+			b.WriteByte('_')
+		}
+	}
+	return finalizeSanitizedUploadFilename(b.String())
+}
+
+// SanitizeUploadFilename normalizes a client file name for safe use in shell commands and action templates.
+// It is applied when staging uploads; call again when building template context if the value may not
+// have passed through staging.
+func SanitizeUploadFilename(name string) string {
+	return sanitizeFilename(name)
 }

service/internal/fileupload/registry_test.go

@@ -2,6 +2,7 @@ package fileupload
 
 import (
 	"os"
+	"strings"
 	"testing"
 	"time"
 
@@ -62,3 +63,30 @@ func assertPendingGone(t *testing.T, r *Registry) {
 		t.Fatal("expected pending entry deleted")
 	}
 }
+
+func TestSanitizeUploadFilenameShellSafe(t *testing.T) {
+	t.Parallel()
+	cases := []struct {
+		in   string
+		want string
+	}{
+		{"normal.txt", "normal.txt"},
+		{"My-Document_2.pdf", "My-Document_2.pdf"},
+		{"", "upload"},
+		{".", "upload"},
+		{"../../../etc/passwd", "passwd"},
+		{"foo;rm -rf /", "foo_rm_-rf_"},
+		{"a$b`x$(y)", "a_b_x__y_"},
+		{"x\ny\tz", "x_y_z"},
+		{"a|b&c>d<e", "a_b_c_d_e"},
+		{`a\b`, "a_b"},
+		{`'quote"`, "_quote_"},
+		{strings.Repeat("n", 300), strings.Repeat("n", 255)},
+	}
+	for _, tc := range cases {
+		got := SanitizeUploadFilename(tc.in)
+		if got != tc.want {
+			t.Errorf("SanitizeUploadFilename(%q) = %q, want %q", tc.in, got, tc.want)
+		}
+	}
+}

service/internal/tpl/templates.go

@@ -43,6 +43,7 @@ type generalTemplateContext struct {
 
 // FileUpload is exposed in action templates as .Arguments.<name> for type file_upload.
 // TmpName is the absolute path of the staged file on the server (similar to PHP's tmp_name).
+// Name is normalized to shell-safe ASCII (see fileupload.SanitizeUploadFilename) before template rendering.
 type FileUpload struct {
 	TmpName  string
 	Name     string