Reject foreign-host URLs in api command to prevent token leak

parsePath now validates absolute URLs against the configured base URL host:
a path is accepted only if it's relative or an absolute Basecamp URL on the
same host (path extracted, account segment dropped — the SDK re-prefixes the
configured account). Absolute URLs on any other host are rejected before any
network call, so the SDK never attaches the Authorization: Bearer header to a
foreign host. A stray leading slash ('/https://evil/…') and mixed-case schemes
('HTTPS://…') are normalized first so neither can smuggle an absolute URL past
the host check. Error propagated through get/post/put/delete; api_test.go
covers same-host extraction, query preservation, and foreign-host rejection.

Author: jeremy

internal/commands/api.go

@@ -3,6 +3,7 @@ package commands
 import (
 	"encoding/json"
 	"fmt"
+	"net/url"
 	"regexp"
 	"strings"
 
@@ -47,7 +48,10 @@ func newAPIGetCmd() *cobra.Command {
 				return err
 			}
 
-			path := parsePath(args[0])
+			path, err := parsePath(args[0], app.Config.BaseURL)
+			if err != nil {
+				return err
+			}
 			resp, err := app.Account().Get(cmd.Context(), path)
 			if err != nil {
 				return convertSDKError(err)
@@ -85,7 +89,10 @@ func newAPIPostCmd() *cobra.Command {
 				return err
 			}
 
-			path := parsePath(args[0])
+			path, err := parsePath(args[0], app.Config.BaseURL)
+			if err != nil {
+				return err
+			}
 
 			// Parse JSON data
 			var body any
@@ -134,7 +141,10 @@ func newAPIPutCmd() *cobra.Command {
 				return err
 			}
 
-			path := parsePath(args[0])
+			path, err := parsePath(args[0], app.Config.BaseURL)
+			if err != nil {
+				return err
+			}
 
 			// Parse JSON data
 			var body any
@@ -176,7 +186,10 @@ func newAPIDeleteCmd() *cobra.Command {
 				return err
 			}
 
-			path := parsePath(args[0])
+			path, err := parsePath(args[0], app.Config.BaseURL)
+			if err != nil {
+				return err
+			}
 			resp, err := app.Account().Delete(cmd.Context(), path)
 			if err != nil {
 				return err
@@ -201,20 +214,47 @@ func apiPathArgs(cmd *cobra.Command, args []string) error {
 	return nil
 }
 
-// parsePath extracts and normalizes the API path.
-// Handles full URLs and relative paths. The leading slash is stripped because
-// the SDK's accountPath and buildURL both add one — keeping it here would
-// double-slash and, on Windows, MSYS/Git Bash converts /path to C:\...\path.
-func parsePath(input string) string {
-	urlPattern := regexp.MustCompile(`^https?://[^/]+/[0-9]+(/.*)`)
-	if matches := urlPattern.FindStringSubmatch(input); len(matches) > 1 {
-		return matches[1]
+// accountSegmentPattern matches a leading /<account-id>/ segment in an API
+// path so it can be dropped — the SDK re-prefixes the configured account.
+var accountSegmentPattern = regexp.MustCompile(`^/[0-9]+(/.*)$`)
+
+// parsePath normalizes the user-supplied API path against the configured base
+// URL. It accepts relative paths ("projects.json", "/projects.json") and
+// absolute Basecamp URLs whose host matches baseURL — from which the path is
+// extracted and a leading /<account-id> segment dropped (the SDK re-prefixes
+// the configured account). Absolute URLs on ANY other host are rejected so the
+// bearer token is never attached to a request bound for a foreign host.
+//
+// A stray leading slash and a mixed-case scheme are normalized first so neither
+// "/https://evil/…" nor "HTTPS://evil/…" can smuggle an absolute URL past the
+// host check (URL schemes are case-insensitive per RFC 3986 §3.1).
+func parsePath(input, baseURL string) (string, error) {
+	candidate := strings.TrimPrefix(input, "/")
+	lower := strings.ToLower(candidate)
+	if strings.HasPrefix(lower, "http://") || strings.HasPrefix(lower, "https://") {
+		u, err := url.Parse(candidate)
+		if err != nil || u.Host == "" {
+			return "", output.ErrUsage("invalid API URL: " + input)
+		}
+		base, baseErr := url.Parse(baseURL)
+		if baseErr != nil || base.Host == "" || !strings.EqualFold(u.Host, base.Host) {
+			return "", output.ErrUsage("API path must be relative or a Basecamp URL on the configured host; refusing to send credentials to " + input)
+		}
+		// Same host: use the path (+ query), dropping a leading /<account-id>.
+		path := u.EscapedPath()
+		if m := accountSegmentPattern.FindStringSubmatch(path); m != nil {
+			path = m[1]
+		}
+		if u.RawQuery != "" {
+			path += "?" + u.RawQuery
+		}
+		return path, nil
 	}
 
-	// Strip leading slash — the SDK prefixes the account path.
-	input = strings.TrimPrefix(input, "/")
-
-	return input
+	// Relative path — return without the leading slash; the SDK prefixes the
+	// account path (keeping it here would double-slash and, on Windows,
+	// MSYS/Git Bash converts /path to C:\...\path).
+	return candidate, nil
 }
 
 // apiSummary generates a summary from the API response.

internal/commands/api_test.go

@@ -8,6 +8,8 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+const testBaseURL = "https://3.basecampapi.com"
+
 func TestParsePath(t *testing.T) {
 	tests := []struct {
 		input string
@@ -17,13 +19,51 @@ func TestParsePath(t *testing.T) {
 		{"/projects.json", "projects.json"},
 		{"buckets/123/todos/456.json", "buckets/123/todos/456.json"},
 		{"/buckets/123/todos/456.json", "buckets/123/todos/456.json"},
+		// Same-host absolute URLs: extract the path, dropping the account segment.
 		{"https://3.basecampapi.com/999/projects.json", "/projects.json"},
 		{"https://3.basecampapi.com/12345/buckets/1/todos/2.json", "/buckets/1/todos/2.json"},
+		// Same host but no account segment — accepted, path used as-is.
+		{"https://3.basecampapi.com/projects.json", "/projects.json"},
+		// Query strings are preserved.
+		{"https://3.basecampapi.com/999/projects.json?page=2", "/projects.json?page=2"},
+		// Schemes are case-insensitive (RFC 3986): a same-host uppercase scheme
+		// still extracts the path.
+		{"HTTPS://3.basecampapi.com/999/projects.json", "/projects.json"},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.input, func(t *testing.T) {
-			assert.Equal(t, tt.want, parsePath(tt.input))
+			got, err := parsePath(tt.input, testBaseURL)
+			require.NoError(t, err)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
+
+// TestParsePathRejectsForeignHosts guards against bearer-token exfiltration: an
+// absolute URL whose host differs from the configured base URL must be rejected
+// before it reaches the SDK, which would otherwise attach the Authorization
+// header and send credentials to the foreign host. Covers mixed-case schemes
+// and a leading-slash smuggling attempt.
+func TestParsePathRejectsForeignHosts(t *testing.T) {
+	bad := []string{
+		"https://evil.com/projects.json",
+		"http://evil.com/projects.json",
+		"https://evil.com/",
+		"https://evil.com",
+		"https://evil.example/999/projects.json", // foreign host, numeric form
+		"http://127.0.0.1:9999/projects.json",
+		"HTTPS://evil.example/projects.json",        // uppercase scheme
+		"HtTpS://evil.example/projects.json",        // mixed-case scheme
+		"/https://evil.example/projects.json",       // leading-slash smuggling
+		"/HTTPS://evil.example/projects.json",       // leading slash + uppercase
+	}
+
+	for _, input := range bad {
+		t.Run(input, func(t *testing.T) {
+			got, err := parsePath(input, testBaseURL)
+			require.Error(t, err)
+			assert.Empty(t, got)
 		})
 	}
 }