From 3b11fd4140350ce0ccc0ed1b794f26b3138ce9b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Sanz?= Date: Fri, 6 Nov 2026 12:29:20 +0100 Subject: [PATCH 1/2] fix: hand the endpoint path editor to the invoke operations only --- internal/api/serverless/client.go | 1 - internal/api/serverless/tasks.go | 56 ++++----- internal/api/serverless/tasks_test.go | 173 ++++++++++++++++++++++---- 3 files changed, 177 insertions(+), 53 deletions(-) diff --git a/internal/api/serverless/client.go b/internal/api/serverless/client.go index b16ee2a..29b7933 100644 --- a/internal/api/serverless/client.go +++ b/internal/api/serverless/client.go @@ -289,7 +289,6 @@ func newGeneratedClient(apiKey, baseURL string, httpClient gen.HttpRequestDoer) req.Header.Set("Accept", "application/json") return nil }), - gen.WithRequestEditorFn(restoreEndpointPathSeparators), ) if err != nil { // NewClientWithResponses only fails if an option returns an error; diff --git a/internal/api/serverless/tasks.go b/internal/api/serverless/tasks.go index 547fcb4..5273ea4 100644 --- a/internal/api/serverless/tasks.go +++ b/internal/api/serverless/tasks.go @@ -23,38 +23,32 @@ var endpointPathPattern = regexp.MustCompile(`^[a-z]([a-z0-9-]{0,62}[a-z0-9])?(/ const endpointPathMaxLen = 255 -// endpointPathRoutes are the operation paths that carry an endpointPath. -var endpointPathRoutes = []string{"/invoke-async/", "/invoke-sync/"} - -// restoreEndpointPathSeparators puts the slashes of a multi-segment endpoint -// path back into the request URL. The generated client styles a path parameter -// with simple escaping, which percent-encodes those slashes, and the API -// rejects an endpointPath that arrives as %2F. Only the part after the route -// marker is rewritten, so an escaped slash in a configured base URL is left as -// the caller wrote it. -func restoreEndpointPathSeparators(_ context.Context, req *http.Request) error { - if req.URL == nil || req.URL.RawPath == "" { - return nil - } - raw := req.URL.RawPath - cut := -1 - for _, marker := range endpointPathRoutes { - if i := strings.LastIndex(raw, marker); i >= 0 && i+len(marker) > cut { - cut = i + len(marker) +// endpointPathEditor restores the slashes of a multi-segment endpoint path in +// the request URL. The generated client styles a path parameter with simple +// escaping, which turns those slashes into %2F, and the API rejects an +// endpointPath that arrives escaped. +// +// The editor is handed to the two operations that carry an endpointPath, so it +// knows the value to look for. A path it is not given is left alone: `invoke-sync` +// is itself a valid app id, so a route matched by its text would rewrite requests +// that carry no endpoint path at all. +func endpointPathEditor(endpointPath string) gen.RequestEditorFn { + return func(_ context.Context, req *http.Request) error { + if req.URL == nil || req.URL.RawPath == "" { + return nil } - } - if cut < 0 { - return nil - } - head, tail := raw[:cut], raw[cut:] - restored := strings.ReplaceAll(tail, "%2F", "/") - restored = strings.ReplaceAll(restored, "%2f", "/") - if head+restored == req.URL.Path { - req.URL.RawPath = "" + // Path is already decoded, so the parameter we were handed is its last + // segments. Anything else on the URL is not ours to touch. + if !strings.HasSuffix(req.URL.Path, "/"+endpointPath) { + return nil + } + // The escaped parameter carries no slash of its own, so it is whatever + // follows the last one, and the base URL keeps the encoding it came with. + raw := req.URL.RawPath + cut := strings.LastIndex(raw, "/") + 1 + req.URL.RawPath = raw[:cut] + endpointPath return nil } - req.URL.RawPath = head + restored - return nil } // taskNotFoundRetry is how long WaitTask retries getTask 404s. A freshly @@ -103,7 +97,7 @@ func (c *Client) InvokeAsync(ctx context.Context, appID, endpointPath, taskID st resp, err := c.inner.StartAsyncTaskWithResponse(ctx, appID, endpointPath, &gen.StartAsyncTaskParams{ Environment: c.environmentParam(), - }, invocation) + }, invocation, endpointPathEditor(endpointPath)) if err != nil { return nil, fmt.Errorf("invoke async: %w", err) } @@ -152,7 +146,7 @@ func (c *Client) InvokeSync(ctx context.Context, appID, endpointPath, taskID str resp, err := c.innerWithMinTimeout(invokeSyncTimeout).StartSyncTaskWithResponse(ctx, appID, endpointPath, &gen.StartSyncTaskParams{ Environment: c.environmentParam(), - }, invocation) + }, invocation, endpointPathEditor(endpointPath)) if err != nil { return nil, fmt.Errorf("invoke sync: %w", err) } diff --git a/internal/api/serverless/tasks_test.go b/internal/api/serverless/tasks_test.go index 169ff8e..40c9b04 100644 --- a/internal/api/serverless/tasks_test.go +++ b/internal/api/serverless/tasks_test.go @@ -8,6 +8,8 @@ import ( "log/slog" "net/http" "net/http/httptest" + "os" + "path/filepath" "strings" "sync/atomic" "testing" @@ -15,11 +17,13 @@ import ( "github.com/google/uuid" "github.com/runware/runware-cli/internal/api/transport" + "gopkg.in/yaml.v3" ) const ( testTaskID = "7c9e6679-7425-40de-944b-e07fc1f90ae7" testEndpoint = "infer" + testNested = "v1/chat/completions" testTaskJSON = `{"id":"7c9e6679-7425-40de-944b-e07fc1f90ae7","status":"pending","appId":"my-app","endpointPath":"infer","createdAt":"2026-07-30T12:00:00Z"}` testTaskDone = `{"id":"7c9e6679-7425-40de-944b-e07fc1f90ae7","status":"completed","appId":"my-app","endpointPath":"infer","createdAt":"2026-07-30T12:00:00Z","completedAt":"2026-07-30T12:00:05Z","output":{"ok":true}}` testTaskFailed = `{"id":"7c9e6679-7425-40de-944b-e07fc1f90ae7","status":"failed","appId":"my-app","endpointPath":"infer","createdAt":"2026-07-30T12:00:00Z","error":"oom killed"}` @@ -35,20 +39,20 @@ func TestValidateEndpointPath(t *testing.T) { {path: "infer"}, {path: "a"}, {path: "my-endpoint"}, - {path: "v1/chat/completions"}, + {path: testNested}, {path: "v1/models"}, {path: "a/b/c/d/e"}, {path: strings.Repeat("ab/", 84) + "ab"}, {path: "", wantErr: "required"}, {path: "/infer", wantErr: "leading slash"}, {path: "/infer", wantErr: `"infer"`}, - {path: "/v1/chat/completions", wantErr: `"v1/chat/completions"`}, + {path: "/" + testNested, wantErr: `"` + testNested + `"`}, {path: "Infer", wantErr: wantInvalid}, {path: "my_endpoint", wantErr: wantInvalid}, {path: "infer/", wantErr: wantInvalid}, {path: "v1//completions", wantErr: wantInvalid}, {path: "v1/Chat", wantErr: wantInvalid}, - {path: "v1/chat/completions/", wantErr: wantInvalid}, + {path: testNested + "/", wantErr: wantInvalid}, {path: strings.Repeat("ab/", 85) + "ab", wantErr: "over the 255"}, {path: "/", wantErr: "leading slash"}, } @@ -431,27 +435,154 @@ func TestWaitTask_DoesNotPost(t *testing.T) { } } -func TestRestoreEndpointPathSeparators(t *testing.T) { +func TestEndpointPathEditorRestoresTheSeparators(t *testing.T) { cases := []struct { - url string - want string + name string + endpointPath string + url string + want string }{ - {url: "https://example.invalid/v1/apps/my-app/invoke-sync/v1%2Fchat%2Fcompletions", want: "/v1/apps/my-app/invoke-sync/v1/chat/completions"}, - {url: "https://example.invalid/v1/apps/my-app/invoke-async/v1%2fchat%2fcompletions", want: "/v1/apps/my-app/invoke-async/v1/chat/completions"}, - {url: "https://example.invalid/v1/apps/my-app/invoke-sync/infer", want: "/v1/apps/my-app/invoke-sync/infer"}, - {url: "https://proxy.invalid/tenant%2Fname/v1/apps/my-app/invoke-sync/v1%2Fchat%2Fcompletions", want: "/tenant%2Fname/v1/apps/my-app/invoke-sync/v1/chat/completions"}, - {url: "https://proxy.invalid/tenant%2Fname/v1/apps/my-app/tasks/7c9e6679", want: "/tenant%2Fname/v1/apps/my-app/tasks/7c9e6679"}, + { + name: "a nested path", + endpointPath: testNested, + url: "https://example.invalid/v1/apps/my-app/invoke-sync/v1%2Fchat%2Fcompletions", + want: "/v1/apps/my-app/invoke-sync/v1/chat/completions", + }, + { + name: "escaped in lower case", + endpointPath: testNested, + url: "https://example.invalid/v1/apps/my-app/invoke-async/v1%2fchat%2fcompletions", + want: "/v1/apps/my-app/invoke-async/v1/chat/completions", + }, + { + name: "one segment", + endpointPath: "infer", + url: "https://example.invalid/v1/apps/my-app/invoke-sync/infer", + want: "/v1/apps/my-app/invoke-sync/infer", + }, + { + name: "an escaped slash in the base url is left alone", + endpointPath: testNested, + url: "https://proxy.invalid/tenant%2Fname/v1/apps/my-app/invoke-sync/v1%2Fchat%2Fcompletions", + want: "/tenant%2Fname/v1/apps/my-app/invoke-sync/v1/chat/completions", + }, + { + // The editor is given to the two operations that carry an endpoint + // path, and rewrites only that parameter. `invoke-sync` is a valid + // app id, so a route recognized by its text alone would rewrite the + // escaped secret name on this request. + name: "another operation on an app named like the route", + endpointPath: testNested, + url: "https://example.invalid/v1/apps/invoke-sync/secrets/a%2Fb", + want: "/v1/apps/invoke-sync/secrets/a%2Fb", + }, } for _, tc := range cases { - req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, tc.url, nil) - if err != nil { - t.Fatalf("NewRequest(%q): %v", tc.url, err) - } - if err := restoreEndpointPathSeparators(context.Background(), req); err != nil { - t.Fatalf("restoreEndpointPathSeparators(%q): %v", tc.url, err) - } - if got := req.URL.EscapedPath(); got != tc.want { - t.Errorf("EscapedPath for %q = %q, want %q", tc.url, got, tc.want) - } + t.Run(tc.name, func(t *testing.T) { + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, tc.url, nil) + if err != nil { + t.Fatalf("NewRequest(%q): %v", tc.url, err) + } + if err := endpointPathEditor(tc.endpointPath)(context.Background(), req); err != nil { + t.Fatalf("endpointPathEditor(%q) on %q: %v", tc.endpointPath, tc.url, err) + } + if got := req.URL.EscapedPath(); got != tc.want { + t.Errorf("EscapedPath for %q = %q, want %q", tc.url, got, tc.want) + } + }) + } +} + +// endpointPathSchema is the EndpointPath rule the vendored API spec states. +type endpointPathSchema struct { + Pattern string `yaml:"pattern"` + MaxLength int `yaml:"maxLength"` +} + +type specComponents struct { + Schemas map[string]endpointPathSchema `yaml:"schemas"` +} + +type specDocument struct { + Components specComponents `yaml:"components"` +} + +// TestEndpointPathRuleMatchesTheSpec keeps the copy above honest. The bug this +// file fixes was a stale copy of this rule, so a spec change that no table row +// covers has to fail here. +func TestEndpointPathRuleMatchesTheSpec(t *testing.T) { + raw, err := os.ReadFile(filepath.Join("..", "..", "..", "api", "serverless", "openapi.yaml")) + if err != nil { + t.Fatal(err) + } + var spec specDocument + if err := yaml.Unmarshal(raw, &spec); err != nil { + t.Fatal(err) + } + + schema, ok := spec.Components.Schemas["EndpointPath"] + if !ok { + t.Fatal("the spec has no EndpointPath schema") + } + if schema.Pattern == "" { + t.Fatal("the spec has no EndpointPath pattern") + } + if got := endpointPathPattern.String(); got != schema.Pattern { + t.Errorf("endpointPathPattern = %q, the spec says %q", got, schema.Pattern) + } + if schema.MaxLength != endpointPathMaxLen { + t.Errorf("endpointPathMaxLen = %d, the spec says %d", endpointPathMaxLen, schema.MaxLength) + } +} + +// TestInvokeSendsANestedPathUnescaped drives both operations through the +// generated client, which is where the escaping happens. The editor is attached +// per operation, so a call that stops passing it fails here and nowhere else. +func TestInvokeSendsANestedPathUnescaped(t *testing.T) { + cases := []struct { + name string + verb string + invoke func(*Client) error + }{ + { + name: "sync", + verb: "invoke-sync", + invoke: func(c *Client) error { + _, err := c.InvokeSync(context.Background(), testAppID, testNested, testTaskID, nil) + return err + }, + }, + { + name: "async", + verb: "invoke-async", + invoke: func(c *Client) error { + _, err := c.InvokeAsync(context.Background(), testAppID, testNested, testTaskID, nil) + return err + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + want := "/v1/apps/" + testAppID + "/" + tc.verb + "/" + testNested + var got string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + got = r.URL.EscapedPath() + w.Header().Set("Content-Type", "application/json") + if tc.verb == "invoke-async" { + w.WriteHeader(http.StatusAccepted) + } + _, _ = w.Write([]byte(testTaskDone)) + })) + defer srv.Close() + + c := newClient("test-key", srv.URL, slog.Default(), srv.Client()) + if err := tc.invoke(c); err != nil { + t.Fatalf("invoke: %v", err) + } + if got != want { + t.Errorf("the server saw %q, want %q", got, want) + } + }) } } From 34af933f7af76229ed64be9ca7cc66a8e5f7a4f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Sanz?= Date: Fri, 6 Nov 2026 12:29:21 +0100 Subject: [PATCH 2/2] docs: name the nested endpoint path where the readme invokes one --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 5d4c220..af99a06 100644 --- a/README.md +++ b/README.md @@ -203,7 +203,7 @@ runware serverless apps show my-app runware serverless apps endpoints my-app runware serverless apps workers my-app --state live -# Invoke an endpoint (path is a bare segment, e.g. infer) +# Invoke an endpoint (the path apps endpoints reports, e.g. infer or v1/chat/completions) runware serverless apps invoke my-app infer -f payload.json --wait # Logs, events, and usage