Police OAuth egress with per-provenance address enforcement

go.mod

@@ -8,8 +8,9 @@ require (
 	charm.land/bubbles/v2 v2.1.1
 	charm.land/bubbletea/v2 v2.0.8
 	charm.land/lipgloss/v2 v2.0.6
-	github.com/basecamp/basecamp-sdk/go v0.14.0
+	github.com/basecamp/basecamp-sdk/go v0.15.0
 	github.com/basecamp/cli v0.2.2-0.20260728023309-04e401b12c6c
+	github.com/basecamp/surfguard/go v0.1.0
 	github.com/charmbracelet/bubbles v1.0.0
 	github.com/charmbracelet/glamour v1.0.0
 	github.com/charmbracelet/huh v1.0.0
@@ -25,6 +26,7 @@ require (
 	github.com/yuin/goldmark v1.8.5
 	github.com/zalando/go-keyring v0.2.8
 	golang.org/x/mod v0.40.0
+	golang.org/x/net v0.58.0
 	golang.org/x/sys v0.47.0
 	golang.org/x/text v0.41.0
 	gopkg.in/yaml.v3 v3.0.1
@@ -128,8 +130,7 @@ require (
 	go.opentelemetry.io/otel/metric v1.45.0 // indirect
 	go.opentelemetry.io/otel/trace v1.45.0 // indirect
 	go.yaml.in/yaml/v3 v3.0.4 // indirect
-	golang.org/x/crypto v0.54.0 // indirect
-	golang.org/x/net v0.57.0 // indirect
+	golang.org/x/crypto v0.55.0 // indirect
 	golang.org/x/sync v0.22.0 // indirect
 	golang.org/x/term v0.45.0 // indirect
 	google.golang.org/genproto/googleapis/api v0.0.0-20260526163538-3dc84a4a5aaa // indirect

go.sum

@@ -87,10 +87,12 @@ github.com/aymanbagabas/go-udiff v0.4.1 h1:OEIrQ8maEeDBXQDoGCbbTTXYJMYRCRO1fnodZ
 github.com/aymanbagabas/go-udiff v0.4.1/go.mod h1:0L9PGwj20lrtmEMeyw4WKJ/TMyDtvAoK9bf2u/mNo3w=
 github.com/aymerick/douceur v0.2.0 h1:Mv+mAeH1Q+n9Fr+oyamOlAkUNPWPlA8PPGR0QAaYuPk=
 github.com/aymerick/douceur v0.2.0/go.mod h1:wlT5vV2O3h55X9m7iVYN0TBM0NH/MmbLnd30/FjWUq4=
-github.com/basecamp/basecamp-sdk/go v0.14.0 h1:Jyzmu3ucqwVe+YNC1zWJBOdPjBaTapyZWX7y0cAvaYo=
-github.com/basecamp/basecamp-sdk/go v0.14.0/go.mod h1:BlEtZTW78rOY5geVtKaCiccYT9Jz+QGgPuXLo2pROWk=
+github.com/basecamp/basecamp-sdk/go v0.15.0 h1:Yxp3WM7rZ7PDcXrOTc4dI9EFOqwHojpqVJ/zbdXnoVg=
+github.com/basecamp/basecamp-sdk/go v0.15.0/go.mod h1:00mgcmi89PlnHnLJNwcJjwryo4W5bYJtA2FoqVgHP54=
 github.com/basecamp/cli v0.2.2-0.20260728023309-04e401b12c6c h1:+5sQBl8sqYoD1Qhwsibn8sBCKWPyZ9NDez6mnuo9Afo=
 github.com/basecamp/cli v0.2.2-0.20260728023309-04e401b12c6c/go.mod h1:EK1Dba6DEw8ZAilVBpf/jri3ONDV7LQkLACSDe73f/c=
+github.com/basecamp/surfguard/go v0.1.0 h1:JMo+MZQEOBRqnUylzD0/jS9S42Fp+XKWMG+4LWfu/XE=
+github.com/basecamp/surfguard/go v0.1.0/go.mod h1:y5MWhE5S/CZAPzUOS0LoOYGUVxTeWFByFDssE3/l4b4=
 github.com/blang/semver v3.5.1+incompatible h1:cQNTCjp13qL8KC3Nbxr/y2Bqb63oX6wdnnjpJbkM4JQ=
 github.com/blang/semver v3.5.1+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnwebNt5EWlYSAyrTnjyyk=
 github.com/bmatcuk/doublestar v1.1.1/go.mod h1:UD6OnuiIn0yFxxA2le/rnRU1G4RaI4UvFv1sNto9p6w=
@@ -459,14 +461,14 @@ go.yaml.in/yaml/v2 v2.4.4 h1:tuyd0P+2Ont/d6e2rl3be67goVK4R6deVxCUX5vyPaQ=
 go.yaml.in/yaml/v2 v2.4.4/go.mod h1:gMZqIpDtDqOfM0uNfy0SkpRhvUryYH0Z6wdMYcacYXQ=
 go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc=
 go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg=
-golang.org/x/crypto v0.54.0 h1:YLIA59K4fiNzHzjnZt2tUJQjQtUWfWbeHBqKtk3eScw=
-golang.org/x/crypto v0.54.0/go.mod h1:KWL8ny2AZdGR2cWmzeHrp2azQPGogOv+HeQaVEXC2dk=
+golang.org/x/crypto v0.55.0 h1:+KWHjbgOaAQ66dh/YlkZKHlz9ZUlq61AFirAR9ntP8M=
+golang.org/x/crypto v0.55.0/go.mod h1:uq0V9dE/fzQuJtbnL+2EhWOE63vo164FY8xqEnV9xis=
 golang.org/x/exp v0.0.0-20251023183803-a4bb9ffd2546 h1:mgKeJMpvi0yx/sU5GsxQ7p6s2wtOnGAHZWCHUM4KGzY=
 golang.org/x/exp v0.0.0-20251023183803-a4bb9ffd2546/go.mod h1:j/pmGrbnkbPtQfxEe5D0VQhZC6qKbfKifgD0oM7sR70=
 golang.org/x/mod v0.40.0 h1:hUv+3cXcdRHz08UmSiOob7sadHig73uo5bkXxQ/tvUs=
 golang.org/x/mod v0.40.0/go.mod h1:0/weTWkPWGBikyTWAX3dkjVztMmBA5hM0DH6BElSupE=
-golang.org/x/net v0.57.0 h1:K5+3DljvIuDG9/Jv9rvyMywYNFCQ9RSUY6OOTTkT+tE=
-golang.org/x/net v0.57.0/go.mod h1:KpXc8iv+r3XplLAG/f7Jsf9RPszJzdR0f58q9vGOuEU=
+golang.org/x/net v0.58.0 h1:ynWG7rqYi4ccpTEuPZ2QGWHktVEM9DMCj9yzDE0Q7To=
+golang.org/x/net v0.58.0/go.mod h1:YwCddHnFlT7eLQqVprV19OnhLGtc5xOKgE0RyqgfWAU=
 golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs=
 golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q=
 golang.org/x/sync v0.22.0 h1:SZjpbeLmrCk4xhRSZFNZW5gFUeCeFgjekvI/+gfScek=

internal/appctx/context.go

@@ -9,7 +9,6 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
-	"time"
 
 	"github.com/basecamp/basecamp-sdk/go/pkg/basecamp"
 
@@ -82,34 +81,13 @@ func (a *authAdapter) AccessToken(ctx context.Context) (string, error) {
 	return a.mgr.AccessToken(ctx)
 }
 
-// checkAuthClientRedirect is the CheckRedirect guard for the auth manager's HTTP
-// client (OAuth discovery, token refresh). Refuse to follow redirects for
-// non-idempotent requests: RFC 6749 token endpoints don't legitimately
-// 3xx-redirect POSTs, and because the exchange/refresh requests set GetBody, Go
-// would replay the auth code / refresh_token to the redirect target (only the
-// initial endpoint is origin-validated). Idempotent GET/HEAD requests (e.g.
-// OAuth discovery) carry no credential body, so they may follow redirects
-// normally — blocking those would needlessly fail discovery and force the
-// Launchpad fallback. Still cap the hop count so a looping endpoint fails fast
-// instead of spinning until the client timeout.
-func checkAuthClientRedirect(_ *http.Request, via []*http.Request) error {
-	if len(via) > 0 && via[0].Method != http.MethodGet && via[0].Method != http.MethodHead {
-		return http.ErrUseLastResponse
-	}
-	if len(via) >= 10 {
-		return fmt.Errorf("stopped after 10 redirects")
-	}
-	return nil
-}
-
 // NewApp creates a new App with the given configuration.
 func NewApp(cfg *config.Config) *App {
-	// Create HTTP client for auth manager (OAuth discovery, token refresh).
-	httpClient := &http.Client{
-		Timeout:       30 * time.Second,
-		CheckRedirect: checkAuthClientRedirect,
-	}
-	authMgr := auth.NewManager(cfg, httpClient)
+	// nil client: the auth Manager builds its own per-provenance OAuth
+	// clients (address-policed, redirect-guarded, 30s timeout) — see
+	// internal/auth/client.go. Passing a client here would replace them
+	// wholesale, enforcement included.
+	authMgr := auth.NewManager(cfg, nil)
 
 	// Create observability components
 	// Collector always runs to gather stats; hooks control output verbosity

internal/appctx/context_test.go

@@ -7,9 +7,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"os"
-	"sync/atomic"
 	"testing"
-	"time"
 
 	"github.com/basecamp/basecamp-sdk/go/pkg/basecamp"
 	"github.com/stretchr/testify/assert"
@@ -50,39 +48,6 @@ func TestNewAppSetsCombinedUserAgent(t *testing.T) {
 	require.NoError(t, err)
 }
 
-// TestCheckAuthClientRedirect_StopsLoop verifies the auth client's redirect
-// guard caps idempotent (GET) follows at Go's default 10-hop limit. A looping
-// endpoint would otherwise spin until the 30s client timeout instead of failing
-// fast, since the guard only blocks non-GET/HEAD redirects.
-func TestCheckAuthClientRedirect_StopsLoop(t *testing.T) {
-	var hops atomic.Int32
-	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		hops.Add(1)
-		http.Redirect(w, r, "/", http.StatusFound)
-	}))
-	defer srv.Close()
-
-	client := &http.Client{CheckRedirect: checkAuthClientRedirect, Timeout: 5 * time.Second}
-	req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, srv.URL, nil)
-	require.NoError(t, err)
-	resp, err := client.Do(req)
-	if resp != nil {
-		_ = resp.Body.Close()
-	}
-	require.Error(t, err, "redirect loop must fail rather than hang")
-	assert.Contains(t, err.Error(), "stopped after 10 redirects")
-	assert.LessOrEqual(t, hops.Load(), int32(11), "client must give up around the 10-redirect cap")
-}
-
-// TestCheckAuthClientRedirect_BlocksCredentialPOST verifies a non-GET/HEAD
-// initial request never follows a redirect: the guard returns ErrUseLastResponse
-// so a credential-bearing POST body is not replayed to the redirect target.
-func TestCheckAuthClientRedirect_BlocksCredentialPOST(t *testing.T) {
-	post := &http.Request{Method: http.MethodPost}
-	err := checkAuthClientRedirect(nil, []*http.Request{post})
-	assert.ErrorIs(t, err, http.ErrUseLastResponse)
-}
-
 func TestWithAppAndFromContext(t *testing.T) {
 	cfg := &config.Config{}
 	app := NewApp(cfg)

internal/auth/auth.go

@@ -63,14 +63,43 @@ const (
 
 // Manager handles OAuth authentication.
 type Manager struct {
-	cfg        *config.Config
-	store      *Store
+	cfg   *config.Config
+	store *Store
+
+	// httpClient is the caller-owned injection seam, used by tests to stub
+	// OAuth traffic. When non-nil it carries EVERY OAuth request: it
+	// collapses the per-provenance lanes below and bypasses the SDK's
+	// address enforcement by design (the SDK's "yours, enforcement
+	// included" contract). Production construction (appctx) passes nil and
+	// the Manager builds the per-lane policed clients itself.
 	httpClient *http.Client
 
+	// Per-provenance OAuth egress lanes (see client.go): BC5 traffic rides a
+	// client whose address policy derives from cfg.BaseURL, Launchpad
+	// traffic one derived from launchpadURL(). Built lazily so a malformed
+	// anchor fails the OAuth operation that needed it, cached for the
+	// Manager's lifetime.
+	bc5Lane oauthLane
+	lpLane  oauthLane
+
+	// proxyEnv is the one construction-time proxy-environment snapshot both
+	// lanes share, built under proxyOnce on first lane use.
+	proxyOnce sync.Once
+	proxyEnv  *proxyEnvState
+
+	// Warnf receives transport-policy warnings (a proxy ignored for OAuth
+	// traffic, a malformed opt-out value). Test seam; nil means stderr.
+	Warnf func(format string, args ...any)
+
 	mu sync.Mutex
 }
 
 // NewManager creates a new auth manager.
+//
+// A nil httpClient is the production configuration: OAuth requests ride
+// per-provenance clients that enforce the SDK's address policy at dial time.
+// A non-nil httpClient is caller-owned (test-only in this codebase) and
+// carries every OAuth request as-is — no address policy is applied on top.
 func NewManager(cfg *config.Config, httpClient *http.Client) *Manager {
 	return &Manager{
 		cfg:        cfg,
@@ -245,7 +274,18 @@ func (m *Manager) refreshLocked(ctx context.Context, origin string, creds *Crede
 		}
 	}
 
-	exchanger := oauth.NewExchanger(m.httpClient)
+	// The refresh lane follows the stored credential's provenance: bc5-typed
+	// credentials refresh against a BC5-discovered endpoint, everything else
+	// is Launchpad. OAuthType and TokenEndpoint are persisted independently —
+	// this selects a policy anchor, it does not validate their binding.
+	laneClient, laneErr := m.launchpadClient()
+	if creds.OAuthType == oauthTypeBC5 {
+		laneClient, laneErr = m.bc5Client()
+	}
+	if laneErr != nil {
+		return laneErr
+	}
+	exchanger := oauth.NewExchanger(laneClient)
 
 	req := oauth.RefreshRequest{
 		TokenEndpoint: tokenEndpoint,
@@ -260,7 +300,7 @@ func (m *Manager) refreshLocked(ctx context.Context, origin string, creds *Crede
 
 	token, err := exchanger.Refresh(ctx, req)
 	if err != nil {
-		return output.ErrAPI(0, fmt.Sprintf("token refresh failed: %v", err))
+		return wrapOAuthError("token refresh failed", err)
 	}
 
 	creds.AccessToken = token.AccessToken
@@ -568,9 +608,17 @@ func (m *Manager) loginDevice(ctx context.Context, credKey string, oauthCfg *oau
 		requestedScope = scopeFull
 	}
 
+	// The device-authorization POST and the token polling both ride the BC5
+	// lane client (the SDK carries them on the one WithDeviceHTTPClient
+	// client), so both endpoints the discovery document named are judged by
+	// the policy cfg.BaseURL earned.
+	deviceClient, err := m.bc5Client()
+	if err != nil {
+		return nil, err
+	}
 	devOpts := make([]oauth.DeviceOption, 0, 2+len(opts.deviceOptions))
 	devOpts = append(devOpts,
-		oauth.WithDeviceHTTPClient(m.httpClient),
+		oauth.WithDeviceHTTPClient(deviceClient),
 		oauth.WithDeviceScope(requestedScope),
 	)
 	devOpts = append(devOpts, opts.deviceOptions...)
@@ -706,7 +754,19 @@ func (m *Manager) discoverOAuth(ctx context.Context, log func(string)) (*discove
 		return nil, err
 	}
 
-	discoverer := oauth.NewDiscoverer(m.httpClient)
+	// Both discovery hops ride the BC5 lane client. Hop 2 (the advertised
+	// issuer's metadata) would otherwise ride the SDK's internal default
+	// client, whose policy blocks loopback and knows nothing of the CLI's
+	// local configuration — a local resource's local advertised issuer would
+	// be refused right after hop 1 succeeded. The lane client carries the
+	// per-provenance policy (AllowLoopback iff cfg.BaseURL is local), so
+	// enforcement is preserved in both modes, and the SDK still adds its own
+	// redirect suppression around it.
+	discoveryClient, err := m.bc5Client()
+	if err != nil {
+		return nil, err
+	}
+	discoverer := oauth.NewDiscoverer(discoveryClient, oauth.WithIssuerHTTPClient(discoveryClient))
 	res, err := discoverer.DiscoverFromResource(ctx, origin)
 	if err != nil {
 		// Hard selection failure: propagate unchanged. output.AsError at the
@@ -933,7 +993,13 @@ func (m *Manager) exchangeCode(ctx context.Context, cfg *oauth.Config, code stri
 		return nil, err
 	}
 
-	exchanger := oauth.NewExchanger(m.httpClient)
+	// The web-flow code exchange is Launchpad-provenance traffic (BC5 logins
+	// go through the device flow), so it rides the Launchpad lane client.
+	laneClient, laneErr := m.launchpadClient()
+	if laneErr != nil {
+		return nil, laneErr
+	}
+	exchanger := oauth.NewExchanger(laneClient)
 
 	req := oauth.ExchangeRequest{
 		TokenEndpoint:   cfg.TokenEndpoint,
@@ -946,7 +1012,7 @@ func (m *Manager) exchangeCode(ctx context.Context, cfg *oauth.Config, code stri
 
 	token, err := exchanger.Exchange(ctx, req)
 	if err != nil {
-		return nil, output.ErrAPI(0, fmt.Sprintf("token exchange failed: %v", err))
+		return nil, wrapOAuthError("token exchange failed", err)
 	}
 
 	creds := &Credentials{

internal/auth/auth_test.go

@@ -1344,9 +1344,12 @@ func TestRefreshLocked_LaunchpadSendsClientID(t *testing.T) {
 	assert.Contains(t, body, "client_secret="+launchpadClientSecret)
 }
 
-// guardedClient mirrors the CheckRedirect guard appctx.NewApp installs on the
-// auth manager's HTTP client: non-GET/HEAD redirects are refused so the client
-// never replays a credential-bearing POST body to a redirect target.
+// guardedClient mirrors the CheckRedirect guard the Manager installs on its
+// per-provenance lane clients (checkAuthClientRedirect in client.go):
+// non-GET/HEAD redirects are refused so the client never replays a
+// credential-bearing POST body to a redirect target. Injected here so these
+// tests exercise the guard against live httptest redirects without the lane
+// policy refusing the loopback servers.
 func guardedClient() *http.Client {
 	return &http.Client{
 		Timeout: 30 * time.Second,