From 19b5153958628b96ff7ada8d6809fcb10346cdb4 Mon Sep 17 00:00:00 2001 From: Kush Date: Thu, 9 Jul 2026 11:55:08 -0400 Subject: [PATCH 1/2] fix(auth): allow custom environments via BW_API_URL; harden token parsing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolveEnvironment rejected any environment outside prod/test/uat, which also blocked an environment routed through an explicit BW_API_URL override — regressing that workflow. Permit a non-built-in environment when BW_API_URL is set: an explicit route is not a silent fall-through to prod, so it keeps the guarantee that an unrecognized value never silently resolves to production (the reason validation was added in ddb2632 / 11a4f95). Because a custom-routed environment still hits the production-only messaging host, widen messagingProdOnlyWarning to fire for any non-prod environment, so those users are still warned that sends are real and billable. Harden the OAuth token exchange: a 2xx response whose body is not a JSON object (an HTML proxy interstitial, or a misrouted host returning XML) now produces a clear 'non-JSON response — check your environment and BW_API_URL' error instead of a cryptic 'invalid character <' JSON parse failure. --- internal/auth/oauth.go | 9 +++++++++ internal/auth/oauth_test.go | 24 ++++++++++++++++++++++++ internal/cmdutil/helpers.go | 12 +++++++++++- internal/cmdutil/helpers_test.go | 16 ++++++++++++++-- 4 files changed, 58 insertions(+), 3 deletions(-) diff --git a/internal/auth/oauth.go b/internal/auth/oauth.go index e984b67..113b7e2 100644 --- a/internal/auth/oauth.go +++ b/internal/auth/oauth.go @@ -1,6 +1,7 @@ package auth import ( + "bytes" "encoding/json" "fmt" "io" @@ -82,6 +83,14 @@ func (tm *TokenManager) fetchToken() (string, error) { return "", fmt.Errorf("token exchange failed (HTTP %d): %s", resp.StatusCode, string(body)) } + // A 2xx whose body isn't a JSON object means we reached something other than + // the identity service — typically a proxy interstitial or a misrouted host + // (e.g. an environment pointed at the wrong place). Surface that plainly + // rather than a cryptic JSON parse error on the leading '<'. + if trimmed := bytes.TrimSpace(body); len(trimmed) == 0 || trimmed[0] != '{' { + return "", fmt.Errorf("token endpoint returned a non-JSON response (HTTP %d) — you may be behind a proxy or pointed at the wrong host; check your environment and BW_API_URL", resp.StatusCode) + } + var tr tokenResponse if err := json.Unmarshal(body, &tr); err != nil { return "", fmt.Errorf("parsing token response: %w", err) diff --git a/internal/auth/oauth_test.go b/internal/auth/oauth_test.go index 1d6cfd5..1fc8609 100644 --- a/internal/auth/oauth_test.go +++ b/internal/auth/oauth_test.go @@ -185,3 +185,27 @@ func TestTokenManager_ThreadSafe(t *testing.T) { t.Errorf("concurrent GetToken() error: %v", err) } } + +// TestTokenManager_NonJSON2xx covers a proxy/misroute returning HTTP 200 with +// an HTML/XML body: fetchToken must fail with a legible message, not a raw +// JSON parse error on the leading '<'. +func TestTokenManager_NonJSON2xx(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/html") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("Proxy login")) + })) + defer srv.Close() + + tm := NewTokenManager("client-id", "client-secret", srv.URL) + _, err := tm.GetToken() + if err == nil { + t.Fatal("expected an error for a non-JSON 2xx body, got nil") + } + if strings.Contains(err.Error(), "parsing token response") { + t.Errorf("got raw parse error, want the friendly non-JSON message: %v", err) + } + if !strings.Contains(err.Error(), "non-JSON response") { + t.Errorf("error should explain the non-JSON response, got: %v", err) + } +} diff --git a/internal/cmdutil/helpers.go b/internal/cmdutil/helpers.go index 0e68485..a90c19f 100644 --- a/internal/cmdutil/helpers.go +++ b/internal/cmdutil/helpers.go @@ -34,6 +34,13 @@ func resolveEnvironment(profileEnv string) (string, error) { case "", "prod", "test", "uat": return env, nil default: + // A non-built-in environment is allowed only with an explicit custom + // route via BW_API_URL. Requiring the override preserves the original + // guarantee — an unrecognized value never silently resolves to prod — + // while still supporting hosts the binary does not hardcode. + if os.Getenv("BW_API_URL") != "" { + return env, nil + } return "", fmt.Errorf("unknown environment %q (expected one of: prod, test, uat)", env) } } @@ -227,7 +234,10 @@ func PlatformClient(accountIDOverride string) (*api.Client, string, error) { // no test host). Worded to be accurate for any messaging command (not just // sends). Returns "" when no warning is needed. func messagingProdOnlyWarning(env string) string { - if env == "test" || env == "uat" { + // Warn for any non-production environment, not just test/uat: a custom + // environment (routed via BW_API_URL) still hits the prod messaging host, + // so the user must know sends are real and billable. + if env != "" && env != "prod" { return "Bandwidth Messaging has no test environment — this request hits PRODUCTION regardless of --environment (any sends are real and billable)." } return "" diff --git a/internal/cmdutil/helpers_test.go b/internal/cmdutil/helpers_test.go index 7b2c04b..6fb00e4 100644 --- a/internal/cmdutil/helpers_test.go +++ b/internal/cmdutil/helpers_test.go @@ -92,19 +92,31 @@ func TestResolveEnvironment(t *testing.T) { t.Run("unknown env is an error (no silent prod fall-through)", func(t *testing.T) { EnvironmentOverride = "staging" t.Cleanup(func() { EnvironmentOverride = "" }) + t.Setenv("BW_API_URL", "") if _, err := resolveEnvironment("prod"); err == nil { t.Error("expected error for unknown env, got nil") } }) + t.Run("unknown env is allowed with an explicit BW_API_URL route", func(t *testing.T) { + EnvironmentOverride = "staging" + t.Cleanup(func() { EnvironmentOverride = "" }) + t.Setenv("BW_API_URL", "https://custom.example.com") + got, err := resolveEnvironment("prod") + if err != nil || got != "staging" { + t.Errorf("got %q, err %v; want staging, nil (explicit route opts in)", got, err) + } + }) } func TestMessagingProdOnlyWarning(t *testing.T) { - for _, env := range []string{"test", "uat"} { + // Any non-prod environment must warn — including a custom env reached via + // BW_API_URL, which still hits the prod messaging host. + for _, env := range []string{"test", "uat", "staging"} { if messagingProdOnlyWarning(env) == "" { t.Errorf("expected a warning for env %q", env) } } - for _, env := range []string{"", "prod", "staging"} { + for _, env := range []string{"", "prod"} { if messagingProdOnlyWarning(env) != "" { t.Errorf("expected NO warning for env %q, got one", env) } From 238e5343e598b2b2b19e783300898d750320d436 Mon Sep 17 00:00:00 2001 From: Kush Date: Fri, 10 Jul 2026 09:52:45 -0400 Subject: [PATCH 2/2] fix(auth): reject BW_API_URL without a scheme up front MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A BW_API_URL missing its scheme (e.g. "stage.api.bandwidth.com") reached net/http as the opaque error 'unsupported protocol scheme ""'. ValidateAPIOverride now catches a schemeless or hostless override at both auth entry points — authenticate() and auth login — with an actionable message that names the fix. An unset BW_API_URL stays valid. --- cmd/auth/login.go | 4 ++++ internal/cmdutil/helpers.go | 20 ++++++++++++++++++++ internal/cmdutil/helpers_test.go | 25 +++++++++++++++++++++++++ 3 files changed, 49 insertions(+) diff --git a/cmd/auth/login.go b/cmd/auth/login.go index c6ac6d4..087d99a 100644 --- a/cmd/auth/login.go +++ b/cmd/auth/login.go @@ -116,6 +116,10 @@ func runLogin(cmd *cobra.Command, args []string) error { } } + if err := cmdutil.ValidateAPIOverride(); err != nil { + return err + } + tokenURL := tokenURLForEnvironment(environment) // Step 1: Verify credentials diff --git a/internal/cmdutil/helpers.go b/internal/cmdutil/helpers.go index a90c19f..c05e837 100644 --- a/internal/cmdutil/helpers.go +++ b/internal/cmdutil/helpers.go @@ -3,6 +3,7 @@ package cmdutil import ( "fmt" + "net/url" "os" "strings" @@ -45,6 +46,22 @@ func resolveEnvironment(profileEnv string) (string, error) { } } +// ValidateAPIOverride rejects a BW_API_URL that is set but missing a scheme +// (e.g. "stage.api.bandwidth.com" instead of "https://stage.api.bandwidth.com"). +// A schemeless value reaches net/http as the opaque error "unsupported protocol +// scheme \"\"", which gives no hint at the cause; catch it up front with an +// actionable message. An unset BW_API_URL is valid. +func ValidateAPIOverride() error { + raw := os.Getenv("BW_API_URL") + if raw == "" { + return nil + } + if u, err := url.Parse(raw); err != nil || u.Scheme == "" || u.Host == "" { + return fmt.Errorf("BW_API_URL must be a full URL including scheme, e.g. https://host.example.com (got %q)", raw) + } + return nil +} + // apiHostForEnvironment maps an environment name to its API host. // Non-production environments can be overridden with BW_API_URL. func apiHostForEnvironment(env string) string { @@ -160,6 +177,9 @@ func authenticate(accountIDOverride string) (*auth.TokenManager, string, string, return nil, "", "", err } + if err := ValidateAPIOverride(); err != nil { + return nil, "", "", err + } env, err := resolveEnvironment(p.Environment) if err != nil { return nil, "", "", err diff --git a/internal/cmdutil/helpers_test.go b/internal/cmdutil/helpers_test.go index 6fb00e4..17cb1e5 100644 --- a/internal/cmdutil/helpers_test.go +++ b/internal/cmdutil/helpers_test.go @@ -108,6 +108,31 @@ func TestResolveEnvironment(t *testing.T) { }) } +func TestValidateAPIOverride(t *testing.T) { + t.Run("unset is valid", func(t *testing.T) { + t.Setenv("BW_API_URL", "") + if err := ValidateAPIOverride(); err != nil { + t.Errorf("unset BW_API_URL should be valid, got %v", err) + } + }) + for _, ok := range []string{"https://stage.api.bandwidth.com", "http://localhost:8080", "https://host.example.com/"} { + t.Run("valid "+ok, func(t *testing.T) { + t.Setenv("BW_API_URL", ok) + if err := ValidateAPIOverride(); err != nil { + t.Errorf("BW_API_URL=%q should be valid, got %v", ok, err) + } + }) + } + for _, bad := range []string{"stage.api.bandwidth.com", "api.bandwidth.com/api/v1", " "} { + t.Run("invalid "+bad, func(t *testing.T) { + t.Setenv("BW_API_URL", bad) + if err := ValidateAPIOverride(); err == nil { + t.Errorf("BW_API_URL=%q should be rejected (missing scheme), got nil", bad) + } + }) + } +} + func TestMessagingProdOnlyWarning(t *testing.T) { // Any non-prod environment must warn — including a custom env reached via // BW_API_URL, which still hits the prod messaging host.