WKS-3614 - Escape worker API path elements and validate worker keys

commands/common/cmd_api.go

@@ -68,7 +68,16 @@ func CallWorkerAPI(c model.IntFlagProvider, params APICallParams) error {
 		apiVersion = params.APIVersion
 	}
 
-	apiEndpoint := fmt.Sprintf("%sworker/api/v%d/%s", utils.AddTrailingSlashIfNeeded(params.ServerURL), apiVersion, strings.Join(params.Path, "/"))
+	escapedPath := make([]string, len(params.Path))
+	for i, p := range params.Path {
+		// PathEscape leaves dot-segments untouched, so they must be rejected.
+		if p == "." || p == ".." {
+			return apiError(http.StatusBadRequest, "invalid path element '%s' in worker API call", p)
+		}
+		escapedPath[i] = url.PathEscape(p)
+	}
+
+	apiEndpoint := fmt.Sprintf("%sworker/api/v%d/%s", utils.AddTrailingSlashIfNeeded(params.ServerURL), apiVersion, strings.Join(escapedPath, "/"))
 
 	q := url.Values{}
 

commands/common/cmd_api_test.go

@@ -6,6 +6,7 @@ package common
 import (
 	"encoding/json"
 	"net/http"
+	"net/http/httptest"
 	"testing"
 	"time"
 
@@ -137,3 +138,51 @@ func TestCallWorkerAPI(t *testing.T) {
 		})
 	}
 }
+
+func TestCallWorkerAPI_PathEscaping(t *testing.T) {
+	tests := []struct {
+		name    string
+		path    []string
+		wantURI string
+		wantErr string
+	}{
+		{name: "ticket case", path: []string{"workers", "../x?y"}, wantURI: "/worker/api/v1/workers/..%2Fx%3Fy?projectKey=p"},
+		{
+			name:    "slashes",
+			path:    []string{"workers", "x/../../../../artifactory/api/repositories/libs-release-local"},
+			wantURI: "/worker/api/v1/workers/x%2F..%2F..%2F..%2F..%2Fartifactory%2Fapi%2Frepositories%2Flibs-release-local?projectKey=p",
+		},
+		{name: "hash", path: []string{"execute", "a#b"}, wantURI: "/worker/api/v1/execute/a%23b?projectKey=p"},
+		{name: "plain key", path: []string{"workers", "wk-1"}, wantURI: "/worker/api/v1/workers/wk-1?projectKey=p"},
+		{name: "constant path", path: []string{"scaffold", "tsconfig"}, wantURI: "/worker/api/v1/scaffold/tsconfig?projectKey=p"},
+		{name: "double dot", path: []string{"workers", ".."}, wantErr: "invalid path element '..' in worker API call"},
+		{name: "single dot", path: []string{"workers", "."}, wantErr: "invalid path element '.' in worker API call"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var gotURI string
+			srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				gotURI = r.RequestURI
+				w.WriteHeader(http.StatusOK)
+			}))
+			defer srv.Close()
+
+			err := CallWorkerAPI(IntFlagMap{}, APICallParams{
+				Method:     http.MethodGet,
+				ServerURL:  srv.URL,
+				Path:       tt.path,
+				ProjectKey: "p",
+				OkStatuses: []int{http.StatusOK},
+			})
+
+			if tt.wantErr != "" {
+				assert.ErrorContains(t, err, tt.wantErr)
+				assert.Empty(t, gotURI, "no request should be sent")
+				return
+			}
+			require.NoError(t, err)
+			assert.Equal(t, tt.wantURI, gotURI)
+		})
+	}
+}

commands/common/cmd_commons.go

@@ -32,6 +32,12 @@ func ExtractProjectAndKeyFromCommandContext(c stringFlagAware, args []string, mi
 
 	if len(args) > 0 && len(args) > minArguments {
 		workerKey = args[0]
+		// An empty argument falls back to the manifest name, which is validated there.
+		if workerKey != "" {
+			if err := ValidateWorkerKey(workerKey); err != nil {
+				return "", "", err
+			}
+		}
 	}
 
 	if workerKey == "" || projectKey == "" {
@@ -43,7 +49,13 @@ func ExtractProjectAndKeyFromCommandContext(c stringFlagAware, args []string, mi
 			return workerKey, projectKey, nil
 		}
 
-		if err = ValidateManifest(manifest, nil); err != nil {
+		// The manifest name is only used (and must be valid) when it is the worker key.
+		toValidate := *manifest
+		if workerKey != "" {
+			toValidate.Name = workerKey
+		}
+
+		if err = ValidateManifest(&toValidate, nil); err != nil {
 			return "", "", err
 		}
 

commands/common/cmd_commons_test.go

@@ -97,6 +97,8 @@ func Test_extractProjectAndKeyFromCommandContext(t *testing.T) {
 		minArguments int
 		onlyGeneric  bool
 		eventType    string
+		skipManifest bool
+		manifestName string
 		assert       func(t *testing.T, manifestWorkerKey, manifestProjectKey, extractedWorkerKey, extractedProjectKey string, err error)
 	}{
 		{
@@ -146,6 +148,55 @@ func Test_extractProjectAndKeyFromCommandContext(t *testing.T) {
 				assert.Equal(t, manifestProjectKey, extractedProjectKey)
 			},
 		},
+		{
+			name:         "empty worker key arg falls back to manifest",
+			c:            mockStringFlagAware{},
+			args:         []string{""},
+			minArguments: 0,
+			assert: func(t *testing.T, manifestWorkerKey, manifestProjectKey, extractedWorkerKey, extractedProjectKey string, err error) {
+				require.NoError(t, err)
+				assert.Equal(t, manifestWorkerKey, extractedWorkerKey)
+				assert.Equal(t, manifestProjectKey, extractedProjectKey)
+			},
+		},
+		{
+			name: "invalid worker key from args",
+			c:    mockStringFlagAware{},
+			args: []string{"../x"},
+			assert: func(t *testing.T, _, _, extractedWorkerKey, _ string, err error) {
+				assert.EqualError(t, err, "invalid worker key '../x': must start with a letter")
+				assert.Empty(t, extractedWorkerKey)
+			},
+		},
+		{
+			name:         "invalid worker key from args without manifest",
+			c:            mockStringFlagAware{},
+			args:         []string{"../x"},
+			skipManifest: true,
+			assert: func(t *testing.T, _, _, extractedWorkerKey, _ string, err error) {
+				assert.EqualError(t, err, "invalid worker key '../x': must start with a letter")
+				assert.Empty(t, extractedWorkerKey)
+			},
+		},
+		{
+			name:         "invalid worker key from manifest",
+			c:            mockStringFlagAware{},
+			manifestName: "x?y",
+			assert: func(t *testing.T, _, _, _, _ string, err error) {
+				assert.EqualError(t, err, "invalid manifest: invalid worker key 'x?y'")
+			},
+		},
+		{
+			name:         "valid key from args ignores invalid manifest name",
+			c:            mockStringFlagAware{},
+			args:         []string{"good-key"},
+			manifestName: "my.worker",
+			assert: func(t *testing.T, _, manifestProjectKey, extractedWorkerKey, extractedProjectKey string, err error) {
+				require.NoError(t, err)
+				assert.Equal(t, "good-key", extractedWorkerKey)
+				assert.Equal(t, manifestProjectKey, extractedProjectKey)
+			},
+		},
 	}
 
 	for _, tt := range tests {
@@ -164,7 +215,13 @@ func Test_extractProjectAndKeyFromCommandContext(t *testing.T) {
 				mf.Action = "GENERIC_EVENT"
 			}
 
-			require.NoError(t, SaveManifest(&mf, dir))
+			if tt.manifestName != "" {
+				mf.Name = tt.manifestName
+			}
+
+			if !tt.skipManifest {
+				require.NoError(t, SaveManifest(&mf, dir))
+			}
 
 			workerKey, projectKey, err := ExtractProjectAndKeyFromCommandContext(tt.c, tt.args, tt.minArguments, tt.onlyGeneric)
 

commands/common/manifest.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"regexp"
 
 	"github.com/jfrog/jfrog-cli-platform-services/model"
 	"github.com/jfrog/jfrog-client-go/utils/log"
@@ -94,11 +95,33 @@ func ReadSourceCode(mf *model.Manifest) (string, error) {
 	return string(sourceBytes), nil
 }
 
+// workerKeyPattern mirrors the server rule (platform-extension, workerValidator.service.ts, assertWorkerKeyIsValid).
+var workerKeyPattern = regexp.MustCompile(`^[A-Za-z][A-Za-z0-9_\\-]+$`)
+
+// ValidateWorkerKey checks the worker key against the same rules as the Workers service.
+func ValidateWorkerKey(key string) error {
+	switch {
+	case len(key) < 2:
+		return fmt.Errorf("invalid worker key '%s': must be at least 2 chars long", key)
+	case len(key) > 40:
+		return fmt.Errorf("invalid worker key '%s': must be lower than 40 chars long", key)
+	case (key[0] < 'a' || key[0] > 'z') && (key[0] < 'A' || key[0] > 'Z'):
+		return fmt.Errorf("invalid worker key '%s': must start with a letter", key)
+	case !workerKeyPattern.MatchString(key):
+		return fmt.Errorf("invalid worker key '%s'", key)
+	}
+	return nil
+}
+
 func ValidateManifest(mf *model.Manifest, actionsMeta ActionsMetadata) error {
 	if mf.Name == "" {
 		return invalidManifestErr("missing name")
 	}
 
+	if err := ValidateWorkerKey(mf.Name); err != nil {
+		return invalidManifestErr(err.Error())
+	}
+
 	if mf.SourceCodePath == "" {
 		return invalidManifestErr("missing source code path")
 	}

commands/common/manifest_test.go

@@ -9,6 +9,7 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
+	"strings"
 	"testing"
 
 	"github.com/jfrog/jfrog-cli-platform-services/model"
@@ -222,6 +223,26 @@ func TestManifest_Validate(t *testing.T) {
 				assert.EqualError(t, err, invalidManifestErr("missing name").Error())
 			},
 		},
+		{
+			name:        "invalid name",
+			actionsMeta: sampleActions,
+			manifest: patchedManifestSample(func(mf *model.Manifest) {
+				mf.Name = "../x?y"
+			}),
+			assert: func(t *testing.T, err error) {
+				assert.EqualError(t, err, invalidManifestErr("invalid worker key '../x?y': must start with a letter").Error())
+			},
+		},
+		{
+			name:        "name too long",
+			actionsMeta: sampleActions,
+			manifest: patchedManifestSample(func(mf *model.Manifest) {
+				mf.Name = "a" + strings.Repeat("b", 40)
+			}),
+			assert: func(t *testing.T, err error) {
+				assert.ErrorContains(t, err, "must be lower than 40 chars long")
+			},
+		},
 		{
 			name:        "missing source code path",
 			actionsMeta: sampleActions,
@@ -500,3 +521,51 @@ func patchedManifestSample(patch func(mf *model.Manifest)) *model.Manifest {
 	patch(&patched)
 	return &patched
 }
+
+func TestValidateWorkerKey(t *testing.T) {
+	const (
+		minMsg   = "must be at least 2 chars long"
+		maxMsg   = "must be lower than 40 chars long"
+		startMsg = "must start with a letter"
+	)
+
+	tests := []struct {
+		key     string
+		wantErr string // empty means valid, otherwise the full expected error
+	}{
+		{key: "ab"},
+		{key: "a" + strings.Repeat("b", 39)},
+		{key: "my-worker"},
+		{key: "wk_1"},
+		{key: "A-b_C-9"},
+		{key: `a\b`},
+		{key: "worker-1234567890-init"},
+		{key: "a", wantErr: "invalid worker key 'a': " + minMsg},
+		{key: "", wantErr: "invalid worker key '': " + minMsg},
+		{key: "a" + strings.Repeat("b", 40), wantErr: "invalid worker key '" + "a" + strings.Repeat("b", 40) + "': " + maxMsg},
+		{key: "1worker", wantErr: "invalid worker key '1worker': " + startMsg},
+		{key: "-worker", wantErr: "invalid worker key '-worker': " + startMsg},
+		{key: "_w", wantErr: "invalid worker key '_w': " + startMsg},
+		{key: "..", wantErr: "invalid worker key '..': " + startMsg},
+		{key: "../x?y", wantErr: "invalid worker key '../x?y': " + startMsg},
+		{key: "ab/cd", wantErr: "invalid worker key 'ab/cd'"},
+		{key: "x?y", wantErr: "invalid worker key 'x?y'"},
+		{key: "a#b", wantErr: "invalid worker key 'a#b'"},
+		{key: "a.b", wantErr: "invalid worker key 'a.b'"},
+		{key: "my worker", wantErr: "invalid worker key 'my worker'"},
+		{key: "a%2F", wantErr: "invalid worker key 'a%2F'"},
+		{key: "wé", wantErr: "invalid worker key 'wé'"},
+		{key: "ab\n", wantErr: "invalid worker key 'ab\n'"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.key, func(t *testing.T) {
+			err := ValidateWorkerKey(tt.key)
+			if tt.wantErr == "" {
+				assert.NoError(t, err)
+			} else {
+				assert.EqualError(t, err, tt.wantErr)
+			}
+		})
+	}
+}

commands/remove_cmd_test.go

@@ -45,6 +45,12 @@ func TestRemoveCommand(t *testing.T) {
 				WithWorkers(&model.WorkerDetails{Key: "wk-1"}).
 				WithDeleteEndpoint(),
 		},
+		{
+			name:           "fails on invalid worker key",
+			commandArgs:    []string{"../x"},
+			serverBehavior: common.NewServerStub(t),
+			assert:         common.AssertOutputError("invalid worker key '../x': must start with a letter"),
+		},
 		{
 			name:        "fails if timeout exceeds",
 			commandArgs: []string{"--" + model.FlagTimeout, "500"},