From 1ec4edcb3dcbdec150a1ba92d8cf6b996ea43609 Mon Sep 17 00:00:00 2001 From: Tristan Cartledge Date: Fri, 23 Oct 2026 10:40:33 +1000 Subject: [PATCH 1/2] fix(cli): refuse cross-origin redirects so credentials stay with the configured server --- .changesets/1792715944-523a54e4.yaml | 10 ++++ templates/perms.go | 1 + .../auxiliary/internal/client/client.go.stmpl | 17 +++++- .../internal/client/client_test.go.stmpl | 56 ++++++++++++++++++ zSDKs/sdk-cli/.speakeasy/gen.lock | 4 +- zSDKs/sdk-cli/internal/client/client.go | 17 +++++- zSDKs/sdk-cli/internal/client/client_test.go | 58 +++++++++++++++++++ 7 files changed, 160 insertions(+), 3 deletions(-) create mode 100644 .changesets/1792715944-523a54e4.yaml create mode 100644 templates/templates/cli/auxiliary/internal/client/client_test.go.stmpl create mode 100644 zSDKs/sdk-cli/internal/client/client_test.go diff --git a/.changesets/1792715944-523a54e4.yaml b/.changesets/1792715944-523a54e4.yaml new file mode 100644 index 000000000..39cf7bb52 --- /dev/null +++ b/.changesets/1792715944-523a54e4.yaml @@ -0,0 +1,10 @@ +id: 1792715944-523a54e4 +features: + - core +targets: + - cli +type: fix +bump: patch +description: refuse cross-origin redirects so credentials are only sent to the configured server +author: TristanSpeakEasy +date: "2026-10-23" diff --git a/templates/perms.go b/templates/perms.go index 3bd7b0c7e..29a0436db 100644 --- a/templates/perms.go +++ b/templates/perms.go @@ -24,6 +24,7 @@ var fileMode = map[string]fs.FileMode{ "templates/cli/auxiliary/internal": 0o20000000755, "templates/cli/auxiliary/internal/client": 0o20000000755, "templates/cli/auxiliary/internal/client/client.go.stmpl": 0o644, + "templates/cli/auxiliary/internal/client/client_test.go.stmpl": 0o644, "templates/cli/auxiliary/internal/client/diagnostics.go.stmpl": 0o644, "templates/cli/auxiliary/internal/clierrors": 0o20000000755, "templates/cli/auxiliary/internal/clierrors/clierrors.go.stmpl": 0o644, diff --git a/templates/templates/cli/auxiliary/internal/client/client.go.stmpl b/templates/templates/cli/auxiliary/internal/client/client.go.stmpl index e3ed8967c..f124d5db7 100644 --- a/templates/templates/cli/auxiliary/internal/client/client.go.stmpl +++ b/templates/templates/cli/auxiliary/internal/client/client.go.stmpl @@ -6,6 +6,7 @@ {{- addImport "internal/config" true -}} {{- addImport "net" -}} {{- addImport "net/http" -}} +{{- addImport "strings" -}} {{- addImport "sync" -}} {{- addImport "time" -}} {{- addImport "fmt" -}} @@ -83,7 +84,7 @@ func NewClient(cmd *cobra.Command, allowedSecurityFields ...string) (*sdk.{{temp // Diagnostics and test client composition. // Order: test client (innermost) → diagnostics wrapper (outermost). - var httpClient HTTPClient = &http.Client{Transport: newPhaseBoundedTransport(cmd)} + var httpClient HTTPClient = &http.Client{Transport: newPhaseBoundedTransport(cmd), CheckRedirect: SameOriginRedirect} if testClient := testclient.NewTestHTTPClient(); testClient != nil { httpClient = testClient } @@ -97,6 +98,20 @@ func NewClient(cmd *cobra.Command, allowedSecurityFields ...string) (*sdk.{{temp {{- end}} } +// SameOriginRedirect refuses redirects to another scheme or host: net/http +// forwards custom credential headers (API keys) and replays 307/308 bodies +// across origins. +func SameOriginRedirect(req *http.Request, via []*http.Request) error { + if len(via) >= 10 { + return fmt.Errorf("stopped after 10 redirects") + } + from := via[0].URL + if req.URL.Scheme != from.Scheme || !strings.EqualFold(req.URL.Host, from.Host) { + return fmt.Errorf("refusing to follow a redirect from %s://%s to %s://%s: credentials are only sent to the configured server", from.Scheme, from.Host, req.URL.Scheme, req.URL.Host) + } + return nil +} + func newPhaseBoundedTransport(cmd *cobra.Command) http.RoundTripper { var phase time.Duration if s := resolveStringFlag(cmd, "timeout"); s != "" { diff --git a/templates/templates/cli/auxiliary/internal/client/client_test.go.stmpl b/templates/templates/cli/auxiliary/internal/client/client_test.go.stmpl new file mode 100644 index 000000000..9980f1f71 --- /dev/null +++ b/templates/templates/cli/auxiliary/internal/client/client_test.go.stmpl @@ -0,0 +1,56 @@ +package client + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func TestSameOriginRedirect(t *testing.T) { + var leakedKey string + other := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + leakedKey = r.Header.Get("x-api-key") + w.WriteHeader(http.StatusOK) + })) + defer other.Close() + + origin := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/cross": + http.Redirect(w, r, other.URL+"/landed", http.StatusTemporaryRedirect) + case "/same": + http.Redirect(w, r, "/landed", http.StatusFound) + default: + w.WriteHeader(http.StatusOK) + } + })) + defer origin.Close() + + httpClient := &http.Client{CheckRedirect: SameOriginRedirect} + do := func(path string) (*http.Response, error) { + req, err := http.NewRequest(http.MethodPost, origin.URL+path, strings.NewReader("{}")) + if err != nil { + t.Fatal(err) + } + req.Header.Set("x-api-key", "secret") + return httpClient.Do(req) + } + + res, err := do("/same") + if err != nil { + t.Fatalf("same-origin redirect: %v", err) + } + _ = res.Body.Close() + if res.StatusCode != http.StatusOK { + t.Fatalf("same-origin redirect status = %d", res.StatusCode) + } + + _, err = do("/cross") + if err == nil || !strings.Contains(err.Error(), "refusing to follow a redirect") { + t.Fatalf("cross-origin redirect error = %v", err) + } + if leakedKey != "" { + t.Fatalf("credential header reached the redirect target: %q", leakedKey) + } +} diff --git a/zSDKs/sdk-cli/.speakeasy/gen.lock b/zSDKs/sdk-cli/.speakeasy/gen.lock index 5b420f96d..336671486 100644 --- a/zSDKs/sdk-cli/.speakeasy/gen.lock +++ b/zSDKs/sdk-cli/.speakeasy/gen.lock @@ -224,7 +224,9 @@ trackedFiles: internal/cli/whoami.go: last_write_checksum: sha1:ad27d6ccfed17dd213951f981030b010ababbf79 internal/client/client.go: - last_write_checksum: sha1:58db154dfc3b80e998471223136423d3ffc1ca92 + last_write_checksum: sha1:378561deb3e868f01d2a5790617ef584e74f2fd2 + internal/client/client_test.go: + last_write_checksum: sha1:10098cd4d1439ddfc4e54b2263aecb664100faed internal/client/diagnostics.go: last_write_checksum: sha1:30b37c632ff875e00e6bce57caa64050dff89fcf internal/clierrors/clierrors.go: diff --git a/zSDKs/sdk-cli/internal/client/client.go b/zSDKs/sdk-cli/internal/client/client.go index 4f54648e9..d3c1d2fff 100644 --- a/zSDKs/sdk-cli/internal/client/client.go +++ b/zSDKs/sdk-cli/internal/client/client.go @@ -15,6 +15,7 @@ import ( "openapi/internal/sdk/retry" "openapi/internal/testclient" "strconv" + "strings" "sync" "time" ) @@ -71,7 +72,7 @@ func NewClient(cmd *cobra.Command, allowedSecurityFields ...string) (*sdk.SDK, e // Diagnostics and test client composition. // Order: test client (innermost) → diagnostics wrapper (outermost). - var httpClient HTTPClient = &http.Client{Transport: newPhaseBoundedTransport(cmd)} + var httpClient HTTPClient = &http.Client{Transport: newPhaseBoundedTransport(cmd), CheckRedirect: SameOriginRedirect} if testClient := testclient.NewTestHTTPClient(); testClient != nil { httpClient = testClient } @@ -80,6 +81,20 @@ func NewClient(cmd *cobra.Command, allowedSecurityFields ...string) (*sdk.SDK, e return sdk.New(sdkOpts...), nil } +// SameOriginRedirect refuses redirects to another scheme or host: net/http +// forwards custom credential headers (API keys) and replays 307/308 bodies +// across origins. +func SameOriginRedirect(req *http.Request, via []*http.Request) error { + if len(via) >= 10 { + return fmt.Errorf("stopped after 10 redirects") + } + from := via[0].URL + if req.URL.Scheme != from.Scheme || !strings.EqualFold(req.URL.Host, from.Host) { + return fmt.Errorf("refusing to follow a redirect from %s://%s to %s://%s: credentials are only sent to the configured server", from.Scheme, from.Host, req.URL.Scheme, req.URL.Host) + } + return nil +} + func newPhaseBoundedTransport(cmd *cobra.Command) http.RoundTripper { var phase time.Duration if s := resolveStringFlag(cmd, "timeout"); s != "" { diff --git a/zSDKs/sdk-cli/internal/client/client_test.go b/zSDKs/sdk-cli/internal/client/client_test.go new file mode 100644 index 000000000..a6ecbfcee --- /dev/null +++ b/zSDKs/sdk-cli/internal/client/client_test.go @@ -0,0 +1,58 @@ +// Code generated by Speakeasy (https://speakeasy.com). DO NOT EDIT. + +package client + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func TestSameOriginRedirect(t *testing.T) { + var leakedKey string + other := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + leakedKey = r.Header.Get("x-api-key") + w.WriteHeader(http.StatusOK) + })) + defer other.Close() + + origin := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/cross": + http.Redirect(w, r, other.URL+"/landed", http.StatusTemporaryRedirect) + case "/same": + http.Redirect(w, r, "/landed", http.StatusFound) + default: + w.WriteHeader(http.StatusOK) + } + })) + defer origin.Close() + + httpClient := &http.Client{CheckRedirect: SameOriginRedirect} + do := func(path string) (*http.Response, error) { + req, err := http.NewRequest(http.MethodPost, origin.URL+path, strings.NewReader("{}")) + if err != nil { + t.Fatal(err) + } + req.Header.Set("x-api-key", "secret") + return httpClient.Do(req) + } + + res, err := do("/same") + if err != nil { + t.Fatalf("same-origin redirect: %v", err) + } + _ = res.Body.Close() + if res.StatusCode != http.StatusOK { + t.Fatalf("same-origin redirect status = %d", res.StatusCode) + } + + _, err = do("/cross") + if err == nil || !strings.Contains(err.Error(), "refusing to follow a redirect") { + t.Fatalf("cross-origin redirect error = %v", err) + } + if leakedKey != "" { + t.Fatalf("credential header reached the redirect target: %q", leakedKey) + } +} From 9809f79677c0952cd214d6f48fb64d3bd5c4784e Mon Sep 17 00:00:00 2001 From: Tristan Cartledge Date: Fri, 23 Oct 2026 11:47:23 +1000 Subject: [PATCH 2/2] fix(cli): treat explicit default ports as the same redirect origin --- .../auxiliary/internal/client/client.go.stmpl | 17 ++++++++++- .../internal/client/client_test.go.stmpl | 30 +++++++++++++++++++ zSDKs/sdk-cli/.speakeasy/gen.lock | 4 +-- zSDKs/sdk-cli/internal/client/client.go | 17 ++++++++++- zSDKs/sdk-cli/internal/client/client_test.go | 30 +++++++++++++++++++ 5 files changed, 94 insertions(+), 4 deletions(-) diff --git a/templates/templates/cli/auxiliary/internal/client/client.go.stmpl b/templates/templates/cli/auxiliary/internal/client/client.go.stmpl index f124d5db7..a7a561cf9 100644 --- a/templates/templates/cli/auxiliary/internal/client/client.go.stmpl +++ b/templates/templates/cli/auxiliary/internal/client/client.go.stmpl @@ -6,6 +6,7 @@ {{- addImport "internal/config" true -}} {{- addImport "net" -}} {{- addImport "net/http" -}} +{{- addImport "net/url" -}} {{- addImport "strings" -}} {{- addImport "sync" -}} {{- addImport "time" -}} @@ -106,12 +107,26 @@ func SameOriginRedirect(req *http.Request, via []*http.Request) error { return fmt.Errorf("stopped after 10 redirects") } from := via[0].URL - if req.URL.Scheme != from.Scheme || !strings.EqualFold(req.URL.Host, from.Host) { + if urlOrigin(req.URL) != urlOrigin(from) { return fmt.Errorf("refusing to follow a redirect from %s://%s to %s://%s: credentials are only sent to the configured server", from.Scheme, from.Host, req.URL.Scheme, req.URL.Host) } return nil } +func urlOrigin(u *url.URL) string { + scheme := strings.ToLower(u.Scheme) + port := u.Port() + if port == "" { + switch scheme { + case "https": + port = "443" + case "http": + port = "80" + } + } + return scheme + "://" + net.JoinHostPort(strings.ToLower(u.Hostname()), port) +} + func newPhaseBoundedTransport(cmd *cobra.Command) http.RoundTripper { var phase time.Duration if s := resolveStringFlag(cmd, "timeout"); s != "" { diff --git a/templates/templates/cli/auxiliary/internal/client/client_test.go.stmpl b/templates/templates/cli/auxiliary/internal/client/client_test.go.stmpl index 9980f1f71..7fa879278 100644 --- a/templates/templates/cli/auxiliary/internal/client/client_test.go.stmpl +++ b/templates/templates/cli/auxiliary/internal/client/client_test.go.stmpl @@ -54,3 +54,33 @@ func TestSameOriginRedirect(t *testing.T) { t.Fatalf("credential header reached the redirect target: %q", leakedKey) } } + +func TestSameOriginRedirectOrigins(t *testing.T) { + for _, tc := range []struct { + from, to string + allowed bool + }{ + {"https://api.example.com/a", "https://api.example.com/b", true}, + {"https://api.example.com/a", "https://API.example.com:443/b", true}, + {"http://api.example.com:80/a", "http://api.example.com/b", true}, + {"https://api.example.com/a", "http://api.example.com/b", false}, + {"https://api.example.com/a", "https://api.example.com:8443/b", false}, + {"https://api.example.com/a", "https://other.example.com/b", false}, + } { + from, err := http.NewRequest(http.MethodGet, tc.from, nil) + if err != nil { + t.Fatal(err) + } + to, err := http.NewRequest(http.MethodGet, tc.to, nil) + if err != nil { + t.Fatal(err) + } + err = SameOriginRedirect(to, []*http.Request{from}) + if tc.allowed && err != nil { + t.Errorf("%s -> %s: unexpected error %v", tc.from, tc.to, err) + } + if !tc.allowed && err == nil { + t.Errorf("%s -> %s: redirect was allowed", tc.from, tc.to) + } + } +} diff --git a/zSDKs/sdk-cli/.speakeasy/gen.lock b/zSDKs/sdk-cli/.speakeasy/gen.lock index 336671486..31d0820b3 100644 --- a/zSDKs/sdk-cli/.speakeasy/gen.lock +++ b/zSDKs/sdk-cli/.speakeasy/gen.lock @@ -224,9 +224,9 @@ trackedFiles: internal/cli/whoami.go: last_write_checksum: sha1:ad27d6ccfed17dd213951f981030b010ababbf79 internal/client/client.go: - last_write_checksum: sha1:378561deb3e868f01d2a5790617ef584e74f2fd2 + last_write_checksum: sha1:461c08574c9a79efebb38c51b5ba8fbb407dc9c8 internal/client/client_test.go: - last_write_checksum: sha1:10098cd4d1439ddfc4e54b2263aecb664100faed + last_write_checksum: sha1:b34318212a7159e4d235f846c9f86c7f2a93b4c3 internal/client/diagnostics.go: last_write_checksum: sha1:30b37c632ff875e00e6bce57caa64050dff89fcf internal/clierrors/clierrors.go: diff --git a/zSDKs/sdk-cli/internal/client/client.go b/zSDKs/sdk-cli/internal/client/client.go index d3c1d2fff..d1aedd042 100644 --- a/zSDKs/sdk-cli/internal/client/client.go +++ b/zSDKs/sdk-cli/internal/client/client.go @@ -8,6 +8,7 @@ import ( "github.com/spf13/cobra" "net" "net/http" + "net/url" "openapi/internal/config" "openapi/internal/flagutil" "openapi/internal/sdk" @@ -89,12 +90,26 @@ func SameOriginRedirect(req *http.Request, via []*http.Request) error { return fmt.Errorf("stopped after 10 redirects") } from := via[0].URL - if req.URL.Scheme != from.Scheme || !strings.EqualFold(req.URL.Host, from.Host) { + if urlOrigin(req.URL) != urlOrigin(from) { return fmt.Errorf("refusing to follow a redirect from %s://%s to %s://%s: credentials are only sent to the configured server", from.Scheme, from.Host, req.URL.Scheme, req.URL.Host) } return nil } +func urlOrigin(u *url.URL) string { + scheme := strings.ToLower(u.Scheme) + port := u.Port() + if port == "" { + switch scheme { + case "https": + port = "443" + case "http": + port = "80" + } + } + return scheme + "://" + net.JoinHostPort(strings.ToLower(u.Hostname()), port) +} + func newPhaseBoundedTransport(cmd *cobra.Command) http.RoundTripper { var phase time.Duration if s := resolveStringFlag(cmd, "timeout"); s != "" { diff --git a/zSDKs/sdk-cli/internal/client/client_test.go b/zSDKs/sdk-cli/internal/client/client_test.go index a6ecbfcee..5e50e12f2 100644 --- a/zSDKs/sdk-cli/internal/client/client_test.go +++ b/zSDKs/sdk-cli/internal/client/client_test.go @@ -56,3 +56,33 @@ func TestSameOriginRedirect(t *testing.T) { t.Fatalf("credential header reached the redirect target: %q", leakedKey) } } + +func TestSameOriginRedirectOrigins(t *testing.T) { + for _, tc := range []struct { + from, to string + allowed bool + }{ + {"https://api.example.com/a", "https://api.example.com/b", true}, + {"https://api.example.com/a", "https://API.example.com:443/b", true}, + {"http://api.example.com:80/a", "http://api.example.com/b", true}, + {"https://api.example.com/a", "http://api.example.com/b", false}, + {"https://api.example.com/a", "https://api.example.com:8443/b", false}, + {"https://api.example.com/a", "https://other.example.com/b", false}, + } { + from, err := http.NewRequest(http.MethodGet, tc.from, nil) + if err != nil { + t.Fatal(err) + } + to, err := http.NewRequest(http.MethodGet, tc.to, nil) + if err != nil { + t.Fatal(err) + } + err = SameOriginRedirect(to, []*http.Request{from}) + if tc.allowed && err != nil { + t.Errorf("%s -> %s: unexpected error %v", tc.from, tc.to, err) + } + if !tc.allowed && err == nil { + t.Errorf("%s -> %s: redirect was allowed", tc.from, tc.to) + } + } +}