Skip to content

fix(cli): refuse cross-origin redirects so credentials stay with the configured server #137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
TristanSpeakEasy wants to merge 2 commits into
base: main
Choose a base branch
from
+250 −3
Open

fix(cli): refuse cross-origin redirects so credentials stay with the configured server #137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1ec4edc
fix(cli): refuse cross-origin redirects so credentials stay with the …
TristanSpeakEasy Oct 23, 2026
9809f79
fix(cli): treat explicit default ports as the same redirect origin
TristanSpeakEasy Oct 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .changesets/1792715944-523a54e4.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"
1 change: 1 addition & 0 deletions templates/perms.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
32 changes: 31 additions & 1 deletion templates/templates/cli/auxiliary/internal/client/client.go.stmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
{{- addImport "internal/config" true -}}
{{- addImport "net" -}}
{{- addImport "net/http" -}}
{{- addImport "net/url" -}}
{{- addImport "strings" -}}
{{- addImport "sync" -}}
{{- addImport "time" -}}
{{- addImport "fmt" -}}
Expand Down Expand Up @@ -83,7 +85,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}

@cubic-dev-ai cubic-dev-ai Bot Oct 23, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Test mode bypasses this redirect policy: NewClient replaces the protected client with TestHTTPClient, whose underlying client follows cross-origin redirects and can forward API-key headers. Apply the same policy to the test client and cover the path through NewClient.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At templates/templates/cli/auxiliary/internal/client/client.go.stmpl, line 87:

<comment>Test mode bypasses this redirect policy: `NewClient` replaces the protected client with `TestHTTPClient`, whose underlying client follows cross-origin redirects and can forward API-key headers. Apply the same policy to the test client and cover the path through `NewClient`.</comment>

<file context>
@@ -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
</file context>
Fix with cubic

@TristanSpeakEasy TristanSpeakEasy Oct 23, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving this one as is. TestHTTPClient is only constructed when SPEAKEASY_TEST_NAME is set by the generated test harness, and it already bypasses the production transport (phase timeouts) entirely, so it is not a path an end user's CLI takes. The redirect policy is covered directly by TestSameOriginRedirect / TestSameOriginRedirectOrigins.

if testClient := testclient.NewTestHTTPClient(); testClient != nil {
httpClient = testClient
}
Expand All @@ -97,6 +99,34 @@ 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 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 != "" {
Expand Down
86 changes: 86 additions & 0 deletions templates/templates/cli/auxiliary/internal/client/client_test.go.stmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
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)
Comment thread
TristanSpeakEasy marked this conversation as resolved.
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)
}
}

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)
}
}
}
4 changes: 3 additions & 1 deletion zSDKs/sdk-cli/.speakeasy/gen.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

32 changes: 31 additions & 1 deletion zSDKs/sdk-cli/internal/client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@ import (
"github.com/spf13/cobra"
"net"
"net/http"
"net/url"
"openapi/internal/config"
"openapi/internal/flagutil"
"openapi/internal/sdk"
"openapi/internal/sdk/models/components"
"openapi/internal/sdk/retry"
"openapi/internal/testclient"
"strconv"
"strings"
"sync"
"time"
)
Expand Down Expand Up @@ -71,7 +73,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
}
Expand All @@ -80,6 +82,34 @@ 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 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 != "" {
Expand Down
88 changes: 88 additions & 0 deletions zSDKs/sdk-cli/internal/client/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// 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)
}
}

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)
}
}
}
Loading