Test commit

Signed-off-by: Denton Gentry <dgentry@tailscale.com>

.github/workflows/coverage.yml

@@ -0,0 +1,30 @@
+name: Code Coverage
+
+on:
+  push:
+    branches:
+      - main
+  pull_request:
+    branches:
+      - '*'
+
+jobs:
+  build:
+    runs-on: ubuntu-latest
+
+    if: "!contains(github.event.head_commit.message, '[ci skip]')"
+
+    steps:
+
+    - uses: actions/checkout@v4
+
+    - name: Set up Go
+      uses: actions/setup-go@v4
+      with:
+        go-version-file: go.mod
+
+    - name: build all
+      run: ./tool/go install ./cmd/...
+
+    - name: Run tests on linux with coverage data
+      run: ./tool/go test -race -covermode atomic -coverprofile=covprofile ./...

client/web/web.go

@@ -21,7 +21,6 @@ import (
 	"slices"
 	"strings"
 	"sync"
-	"sync/atomic"
 	"time"
 
 	"github.com/gorilla/csrf"
@@ -39,7 +38,8 @@ import (
 
 // Server is the backend server for a Tailscale web client.
 type Server struct {
-	lc *tailscale.LocalClient
+	lc      *tailscale.LocalClient
+	timeNow func() time.Time
 
 	devMode     bool
 	tsDebugMode string
@@ -61,8 +61,7 @@ type Server struct {
 	//
 	// The map provides a lookup of the session by cookie value
 	// (browserSession.ID => browserSession).
-	browserSessions  sync.Map
-	controlServerURL atomic.Value // access through getControlServerURL
+	browserSessions sync.Map
 }
 
 const (
@@ -83,7 +82,8 @@ type browserSession struct {
 	ID            string
 	SrcNode       tailcfg.NodeID
 	SrcUser       tailcfg.UserID
-	AuthURL       string // control server URL for user to authenticate the session
+	AuthID        string // from tailcfg.WebClientAuthResponse
+	AuthURL       string // from tailcfg.WebClientAuthResponse
 	Created       time.Time
 	Authenticated bool
 }
@@ -102,7 +102,7 @@ func (s *browserSession) isAuthorized() bool {
 		return false
 	case !s.Authenticated:
 		return false // awaiting auth
-	case s.isExpired(): // TODO: add time field to server?
+	case s.isExpired():
 		return false // expired
 	}
 	return true
@@ -111,7 +111,7 @@ func (s *browserSession) isAuthorized() bool {
 // isExpired reports true if s is expired.
 // 2023-10-05: Sessions expire by default 30 days after creation.
 func (s *browserSession) isExpired() bool {
-	return !s.Created.IsZero() && time.Now().After(s.expires()) // TODO: add time field to server?
+	return !s.Created.IsZero() && time.Now().After(s.expires()) // TODO: use Server.timeNow field
 }
 
 // expires reports when the given session expires.
@@ -132,6 +132,10 @@ type ServerOpts struct {
 	// LocalClient is the tailscale.LocalClient to use for this web server.
 	// If nil, a new one will be created.
 	LocalClient *tailscale.LocalClient
+
+	// TimeNow optionally provides a time function.
+	// time.Now is used as default.
+	TimeNow func() time.Time
 }
 
 // NewServer constructs a new Tailscale web client server.
@@ -143,6 +147,10 @@ func NewServer(opts ServerOpts) (s *Server, cleanup func()) {
 		devMode:    opts.DevMode,
 		lc:         opts.LocalClient,
 		pathPrefix: opts.PathPrefix,
+		timeNow:    opts.TimeNow,
+	}
+	if s.timeNow == nil {
+		s.timeNow = time.Now
 	}
 	s.tsDebugMode = s.debugMode()
 	s.assetsHandler, cleanup = assetsHandler(opts.DevMode)
@@ -284,7 +292,6 @@ var (
 	errNotUsingTailscale = errors.New("not-using-tailscale")
 	errTaggedSource      = errors.New("tagged-source")
 	errNotOwner          = errors.New("not-owner")
-	errFailedAuth        = errors.New("failed-auth")
 )
 
 // getTailscaleBrowserSession retrieves the browser session associated with
@@ -373,7 +380,7 @@ func (s *Server) serveTailscaleAuth(w http.ResponseWriter, r *http.Request) {
 		return
 	case session == nil:
 		// Create a new session.
-		d, err := s.getOrAwaitAuthURL(r.Context(), "", whois.Node.ID)
+		d, err := s.getOrAwaitAuth(r.Context(), "", whois.Node.ID)
 		if err != nil {
 			http.Error(w, err.Error(), http.StatusInternalServerError)
 			return
@@ -387,8 +394,9 @@ func (s *Server) serveTailscaleAuth(w http.ResponseWriter, r *http.Request) {
 			ID:      sid,
 			SrcNode: whois.Node.ID,
 			SrcUser: whois.UserProfile.ID,
+			AuthID:  d.ID,
 			AuthURL: d.URL,
-			Created: time.Now(),
+			Created: s.timeNow(),
 		}
 		s.browserSessions.Store(sid, session)
 		// Set the cookie on browser.
@@ -403,13 +411,13 @@ func (s *Server) serveTailscaleAuth(w http.ResponseWriter, r *http.Request) {
 	case !session.isAuthorized():
 		if r.URL.Query().Get("wait") == "true" {
 			// Client requested we block until user completes auth.
-			d, err := s.getOrAwaitAuthURL(r.Context(), session.AuthURL, whois.Node.ID)
-			if errors.Is(err, errFailedAuth) {
-				http.Error(w, "user is unauthorized", http.StatusUnauthorized)
-				s.browserSessions.Delete(session.ID) // clean up the failed session
-				return
-			} else if err != nil {
-				http.Error(w, err.Error(), http.StatusInternalServerError)
+			d, err := s.getOrAwaitAuth(r.Context(), session.AuthID, whois.Node.ID)
+			if err != nil {
+				http.Error(w, err.Error(), http.StatusUnauthorized)
+				// Clean up the session. Doing this on any error from control
+				// server to avoid the user getting stuck with a bad session
+				// cookie.
+				s.browserSessions.Delete(session.ID)
 				return
 			}
 			if d.Complete {
@@ -447,43 +455,22 @@ func (s *Server) newSessionID() (string, error) {
 	return "", errors.New("too many collisions generating new session; please refresh page")
 }
 
-func (s *Server) getControlServerURL(ctx context.Context) (string, error) {
-	if v := s.controlServerURL.Load(); v != nil {
-		v, _ := v.(string)
-		return v, nil
-	}
-	prefs, err := s.lc.GetPrefs(ctx)
-	if err != nil {
-		return "", err
-	}
-	url := prefs.ControlURLOrDefault()
-	s.controlServerURL.Store(url)
-	return url, nil
-}
-
-// getOrAwaitAuthURL connects to the control server for user auth,
+// getOrAwaitAuth connects to the control server for user auth,
 // with the following behavior:
 //
-//  1. If authURL is provided empty, a new auth URL is created on the
-//     control server and reported back here, which can then be used
-//     to redirect the user on the frontend.
-//  2. If authURL is provided non-empty, the connection to control
-//     blocks until the user has completed the URL. getOrAwaitAuthURL
-//     terminates when either the URL is completed, or ctx is canceled.
-func (s *Server) getOrAwaitAuthURL(ctx context.Context, authURL string, src tailcfg.NodeID) (*tailcfg.WebClientAuthResponse, error) {
-	serverURL, err := s.getControlServerURL(ctx)
-	if err != nil {
-		return nil, err
-	}
+//  1. If authID is provided empty, a new auth URL is created on the control
+//     server and reported back here, which can then be used to redirect the
+//     user on the frontend.
+//  2. If authID is provided non-empty, the connection to control blocks until
+//     the user has completed authenticating the associated auth URL,
+//     or until ctx is canceled.
+func (s *Server) getOrAwaitAuth(ctx context.Context, authID string, src tailcfg.NodeID) (*tailcfg.WebClientAuthResponse, error) {
 	type data struct {
 		ID  string
 		Src tailcfg.NodeID
 	}
 	var b bytes.Buffer
-	if err := json.NewEncoder(&b).Encode(data{
-		ID:  strings.TrimPrefix(authURL, serverURL),
-		Src: src,
-	}); err != nil {
+	if err := json.NewEncoder(&b).Encode(data{ID: authID, Src: src}); err != nil {
 		return nil, err
 	}
 	url := "http://" + apitype.LocalAPIHost + "/localapi/v0/debug-web-client"
@@ -497,11 +484,7 @@ func (s *Server) getOrAwaitAuthURL(ctx context.Context, authURL string, src tail
 	}
 	body, _ := io.ReadAll(resp.Body)
 	resp.Body.Close()
-	if resp.StatusCode == http.StatusUnauthorized {
-		// User completed auth, but control server reported
-		// them unauthorized to manage this node.
-		return nil, errFailedAuth
-	} else if resp.StatusCode != http.StatusOK {
+	if resp.StatusCode != http.StatusOK {
 		return nil, fmt.Errorf("failed request: %s", body)
 	}
 	var authResp *tailcfg.WebClientAuthResponse

client/web/web_test.go

@@ -11,7 +11,6 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
-	"reflect"
 	"strings"
 	"testing"
 	"time"
@@ -19,7 +18,6 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"tailscale.com/client/tailscale"
 	"tailscale.com/client/tailscale/apitype"
-	"tailscale.com/ipn"
 	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/net/memnet"
 	"tailscale.com/tailcfg"
@@ -412,9 +410,14 @@ func TestServeTailscaleAuth(t *testing.T) {
 	defer localapi.Close()
 	go localapi.Serve(lal)
 
+	timeNow := time.Now()
+	oneHourAgo := timeNow.Add(-time.Hour)
+	sixtyDaysAgo := timeNow.Add(-sessionCookieExpiry * 2)
+
 	s := &Server{
 		lc:          &tailscale.LocalClient{Dial: lal.Dial},
 		tsDebugMode: "full",
+		timeNow:     func() time.Time { return timeNow },
 	}
 
 	successCookie := "ts-cookie-success"
@@ -422,7 +425,8 @@ func TestServeTailscaleAuth(t *testing.T) {
 		ID:      successCookie,
 		SrcNode: remoteNode.Node.ID,
 		SrcUser: user.ID,
-		Created: time.Now(),
+		Created: oneHourAgo,
+		AuthID:  testAuthPathSuccess,
 		AuthURL: testControlURL + testAuthPathSuccess,
 	})
 	failureCookie := "ts-cookie-failure"
@@ -430,7 +434,8 @@ func TestServeTailscaleAuth(t *testing.T) {
 		ID:      failureCookie,
 		SrcNode: remoteNode.Node.ID,
 		SrcUser: user.ID,
-		Created: time.Now(),
+		Created: oneHourAgo,
+		AuthID:  testAuthPathError,
 		AuthURL: testControlURL + testAuthPathError,
 	})
 	expiredCookie := "ts-cookie-expired"
@@ -438,7 +443,8 @@ func TestServeTailscaleAuth(t *testing.T) {
 		ID:      expiredCookie,
 		SrcNode: remoteNode.Node.ID,
 		SrcUser: user.ID,
-		Created: time.Now().Add(-sessionCookieExpiry * 2),
+		Created: sixtyDaysAgo,
+		AuthID:  "/a/old-auth-url",
 		AuthURL: testControlURL + "/a/old-auth-url",
 	})
 
@@ -448,19 +454,40 @@ func TestServeTailscaleAuth(t *testing.T) {
 		query         string
 		wantStatus    int
 		wantResp      *authResponse
-		wantNewCookie bool // new cookie generated
+		wantNewCookie bool            // new cookie generated
+		wantSession   *browserSession // session associated w/ cookie at end of request
 	}{
 		{
 			name:          "new-session-created",
 			wantStatus:    http.StatusOK,
 			wantResp:      &authResponse{OK: false, AuthURL: testControlURL + testAuthPath},
 			wantNewCookie: true,
-		}, {
+			wantSession: &browserSession{
+				ID:            "GENERATED_ID", // gets swapped for newly created ID by test
+				SrcNode:       remoteNode.Node.ID,
+				SrcUser:       user.ID,
+				Created:       timeNow,
+				AuthID:        testAuthPath,
+				AuthURL:       testControlURL + testAuthPath,
+				Authenticated: false,
+			},
+		},
+		{
 			name:       "query-existing-incomplete-session",
 			cookie:     successCookie,
 			wantStatus: http.StatusOK,
 			wantResp:   &authResponse{OK: false, AuthURL: testControlURL + testAuthPathSuccess},
-		}, {
+			wantSession: &browserSession{
+				ID:            successCookie,
+				SrcNode:       remoteNode.Node.ID,
+				SrcUser:       user.ID,
+				Created:       oneHourAgo,
+				AuthID:        testAuthPathSuccess,
+				AuthURL:       testControlURL + testAuthPathSuccess,
+				Authenticated: false,
+			},
+		},
+		{
 			name:   "transition-to-successful-session",
 			cookie: successCookie,
 			// query "wait" indicates the FE wants to make
@@ -468,29 +495,70 @@ func TestServeTailscaleAuth(t *testing.T) {
 			query:      "wait=true",
 			wantStatus: http.StatusOK,
 			wantResp:   &authResponse{OK: true},
-		}, {
+			wantSession: &browserSession{
+				ID:            successCookie,
+				SrcNode:       remoteNode.Node.ID,
+				SrcUser:       user.ID,
+				Created:       oneHourAgo,
+				AuthID:        testAuthPathSuccess,
+				AuthURL:       testControlURL + testAuthPathSuccess,
+				Authenticated: true,
+			},
+		},
+		{
 			name:       "query-existing-complete-session",
 			cookie:     successCookie,
 			wantStatus: http.StatusOK,
 			wantResp:   &authResponse{OK: true},
-		}, {
-			name:       "transition-to-failed-session",
-			cookie:     failureCookie,
-			query:      "wait=true",
-			wantStatus: http.StatusUnauthorized,
-			wantResp:   nil,
-		}, {
+			wantSession: &browserSession{
+				ID:            successCookie,
+				SrcNode:       remoteNode.Node.ID,
+				SrcUser:       user.ID,
+				Created:       oneHourAgo,
+				AuthID:        testAuthPathSuccess,
+				AuthURL:       testControlURL + testAuthPathSuccess,
+				Authenticated: true,
+			},
+		},
+		{
+			name:        "transition-to-failed-session",
+			cookie:      failureCookie,
+			query:       "wait=true",
+			wantStatus:  http.StatusUnauthorized,
+			wantResp:    nil,
+			wantSession: nil, // session deleted
+		},
+		{
 			name:          "failed-session-cleaned-up",
 			cookie:        failureCookie,
 			wantStatus:    http.StatusOK,
 			wantResp:      &authResponse{OK: false, AuthURL: testControlURL + testAuthPath},
 			wantNewCookie: true,
-		}, {
+			wantSession: &browserSession{
+				ID:            "GENERATED_ID",
+				SrcNode:       remoteNode.Node.ID,
+				SrcUser:       user.ID,
+				Created:       timeNow,
+				AuthID:        testAuthPath,
+				AuthURL:       testControlURL + testAuthPath,
+				Authenticated: false,
+			},
+		},
+		{
 			name:          "expired-cookie-gets-new-session",
 			cookie:        expiredCookie,
 			wantStatus:    http.StatusOK,
 			wantResp:      &authResponse{OK: false, AuthURL: testControlURL + testAuthPath},
 			wantNewCookie: true,
+			wantSession: &browserSession{
+				ID:            "GENERATED_ID",
+				SrcNode:       remoteNode.Node.ID,
+				SrcUser:       user.ID,
+				Created:       timeNow,
+				AuthID:        testAuthPath,
+				AuthURL:       testControlURL + testAuthPath,
+				Authenticated: false,
+			},
 		},
 	}
 	for _, tt := range tests {
@@ -503,6 +571,8 @@ func TestServeTailscaleAuth(t *testing.T) {
 			s.serveTailscaleAuth(w, r)
 			res := w.Result()
 			defer res.Body.Close()
+
+			// Validate response status/data.
 			if gotStatus := res.StatusCode; tt.wantStatus != gotStatus {
 				t.Errorf("wrong status; want=%v, got=%v", tt.wantStatus, gotStatus)
 			}
@@ -516,19 +586,35 @@ func TestServeTailscaleAuth(t *testing.T) {
 					t.Fatal(err)
 				}
 			}
-			if !reflect.DeepEqual(gotResp, tt.wantResp) {
-				t.Errorf("wrong response; want=%v, got=%v", tt.wantResp, gotResp)
+			if diff := cmp.Diff(gotResp, tt.wantResp); diff != "" {
+				t.Errorf("wrong response; (-got+want):%v", diff)
 			}
+			// Validate cookie creation.
+			sessionID := tt.cookie
 			var gotCookie bool
 			for _, c := range w.Result().Cookies() {
 				if c.Name == sessionCookieName {
 					gotCookie = true
+					sessionID = c.Value
 					break
 				}
 			}
 			if gotCookie != tt.wantNewCookie {
 				t.Errorf("wantNewCookie wrong; want=%v, got=%v", tt.wantNewCookie, gotCookie)
 			}
+			// Validate browser session contents.
+			var gotSesson *browserSession
+			if s, ok := s.browserSessions.Load(sessionID); ok {
+				gotSesson = s.(*browserSession)
+			}
+			if tt.wantSession != nil && tt.wantSession.ID == "GENERATED_ID" {
+				// If requested, swap in the generated session ID before
+				// comparing got/want.
+				tt.wantSession.ID = sessionID
+			}
+			if diff := cmp.Diff(gotSesson, tt.wantSession); diff != "" {
+				t.Errorf("wrong session; (-got+want):%v", diff)
+			}
 		})
 	}
 }
@@ -572,14 +658,6 @@ func mockLocalAPI(t *testing.T, whoIs map[string]*apitype.WhoIsResponse, self fu
 			}
 			w.Header().Set("Content-Type", "application/json")
 			return
-		case "/localapi/v0/prefs":
-			prefs := ipn.Prefs{ControlURL: testControlURL}
-			if err := json.NewEncoder(w).Encode(prefs); err != nil {
-				http.Error(w, err.Error(), http.StatusInternalServerError)
-				return
-			}
-			w.Header().Set("Content-Type", "application/json")
-			return
 		case "/localapi/v0/debug-web-client": // used by TestServeTailscaleAuth
 			type reqData struct {
 				ID  string
@@ -596,7 +674,7 @@ func mockLocalAPI(t *testing.T, whoIs map[string]*apitype.WhoIsResponse, self fu
 			}
 			var resp *tailcfg.WebClientAuthResponse
 			if data.ID == "" {
-				resp = &tailcfg.WebClientAuthResponse{URL: testControlURL + testAuthPath}
+				resp = &tailcfg.WebClientAuthResponse{ID: testAuthPath, URL: testControlURL + testAuthPath}
 			} else if data.ID == testAuthPathSuccess {
 				resp = &tailcfg.WebClientAuthResponse{Complete: true}
 			} else if data.ID == testAuthPathError {

clientupdate/clientupdate.go

@@ -73,9 +73,6 @@ type Arguments struct {
 	//
 	// Leaving this empty is the same as using CurrentTrack.
 	Version string
-	// AppStore forces a local app store check, even if the current binary was
-	// not installed via an app store. TODO(cpalmer): Remove this.
-	AppStore bool
 	// Logf is a logger for update progress messages.
 	Logf logger.Logf
 	// Stdout and Stderr should be used for output instead of os.Stdout and
@@ -182,14 +179,12 @@ func (up *Updater) getUpdateFunction() updateFunction {
 		}
 	case "darwin":
 		switch {
-		case !up.Arguments.AppStore && !version.IsSandboxedMacOS():
-			return nil
-		case !up.Arguments.AppStore && strings.HasSuffix(os.Getenv("HOME"), "/io.tailscale.ipn.macsys/Data"):
-			// TODO(noncombatant): return up.updateMacSys when we figure out why
-			// Sparkle update doesn't work when running "tailscale update".
-			return nil
-		default:
+		case version.IsMacAppStore():
 			return up.updateMacAppStore
+		case version.IsMacSysExt():
+			return up.updateMacSys
+		default:
+			return nil
 		}
 	case "freebsd":
 		return up.updateFreeBSD
@@ -625,55 +620,17 @@ func (up *Updater) updateMacSys() error {
 }
 
 func (up *Updater) updateMacAppStore() error {
-	out, err := exec.Command("defaults", "read", "/Library/Preferences/com.apple.commerce.plist", "AutoUpdate").CombinedOutput()
+	// We can't trigger the update via App Store from the sandboxed app. At
+	// most, we can open the App Store page for them.
+	up.Logf("Please use the App Store to update Tailscale.\nConsider enabling Automatic Updates in the App Store Settings, if you haven't already.\nOpening the Tailscale app page...")
+
+	out, err := exec.Command("open", "https://apps.apple.com/us/app/tailscale/id1475387142").CombinedOutput()
 	if err != nil {
-		return fmt.Errorf("can't check App Store auto-update setting: %w, output: %q", err, string(out))
-	}
-	const on = "1\n"
-	if string(out) != on {
-		up.Logf("NOTE: Automatic updating for App Store apps is turned off. You can change this setting in System Settings (search for ‘update’).")
-	}
-
-	out, err = exec.Command("softwareupdate", "--list").CombinedOutput()
-	if err != nil {
-		return fmt.Errorf("can't check App Store for available updates: %w, output: %q", err, string(out))
-	}
-
-	newTailscale := parseSoftwareupdateList(out)
-	if newTailscale == "" {
-		up.Logf("no Tailscale update available")
-		return nil
-	}
-
-	newTailscaleVer := strings.TrimPrefix(newTailscale, "Tailscale-")
-	if !up.confirm(newTailscaleVer) {
-		return nil
-	}
-
-	cmd := exec.Command("sudo", "softwareupdate", "--install", newTailscale)
-	cmd.Stdout = up.Stdout
-	cmd.Stderr = up.Stderr
-	if err := cmd.Run(); err != nil {
-		return fmt.Errorf("can't install App Store update for Tailscale: %w", err)
+		return fmt.Errorf("can't open the Tailscale page in App Store: %w, output: %q", err, string(out))
 	}
 	return nil
 }
 
-var macOSAppStoreListPattern = regexp.MustCompile(`(?m)^\s+\*\s+Label:\s*(Tailscale-\d[\d\.]+)`)
-
-// parseSoftwareupdateList searches the output of `softwareupdate --list` on
-// Darwin and returns the matching Tailscale package label. If there is none,
-// returns the empty string.
-//
-// See TestParseSoftwareupdateList for example inputs.
-func parseSoftwareupdateList(stdout []byte) string {
-	matches := macOSAppStoreListPattern.FindSubmatch(stdout)
-	if len(matches) < 2 {
-		return ""
-	}
-	return string(matches[1])
-}
-
 // winMSIEnv is the environment variable that, if set, is the MSI file for the
 // update command to install. It's passed like this so we can stop the
 // tailscale.exe process from running before the msiexec process runs and tries

clientupdate/clientupdate_test.go

@@ -84,84 +84,6 @@ func TestUpdateDebianAptSourcesListBytes(t *testing.T) {
 	}
 }
 
-func TestParseSoftwareupdateList(t *testing.T) {
-	tests := []struct {
-		name  string
-		input []byte
-		want  string
-	}{
-		{
-			name: "update-at-end-of-list",
-			input: []byte(`
-	 Software Update Tool
-
-	 Finding available software
-	 Software Update found the following new or updated software:
-			* Label: MacBookAirEFIUpdate2.4-2.4
-					 Title: MacBook Air EFI Firmware Update, Version: 2.4, Size: 3817K, Recommended: YES, Action: restart,
-			* Label: ProAppsQTCodecs-1.0
-					 Title: ProApps QuickTime codecs, Version: 1.0, Size: 968K, Recommended: YES,
-			* Label: Tailscale-1.23.4
-					 Title: The Tailscale VPN, Version: 1.23.4, Size: 1023K, Recommended: YES,
-`),
-			want: "Tailscale-1.23.4",
-		},
-		{
-			name: "update-in-middle-of-list",
-			input: []byte(`
-	 Software Update Tool
-
-	 Finding available software
-	 Software Update found the following new or updated software:
-			* Label: MacBookAirEFIUpdate2.4-2.4
-					 Title: MacBook Air EFI Firmware Update, Version: 2.4, Size: 3817K, Recommended: YES, Action: restart,
-			* Label: Tailscale-1.23.5000
-					 Title: The Tailscale VPN, Version: 1.23.4, Size: 1023K, Recommended: YES,
-			* Label: ProAppsQTCodecs-1.0
-					 Title: ProApps QuickTime codecs, Version: 1.0, Size: 968K, Recommended: YES,
-`),
-			want: "Tailscale-1.23.5000",
-		},
-		{
-			name: "update-not-in-list",
-			input: []byte(`
-	 Software Update Tool
-
-	 Finding available software
-	 Software Update found the following new or updated software:
-			* Label: MacBookAirEFIUpdate2.4-2.4
-					 Title: MacBook Air EFI Firmware Update, Version: 2.4, Size: 3817K, Recommended: YES, Action: restart,
-			* Label: ProAppsQTCodecs-1.0
-					 Title: ProApps QuickTime codecs, Version: 1.0, Size: 968K, Recommended: YES,
-`),
-			want: "",
-		},
-		{
-			name: "decoy-in-list",
-			input: []byte(`
-	 Software Update Tool
-
-	 Finding available software
-	 Software Update found the following new or updated software:
-			* Label: MacBookAirEFIUpdate2.4-2.4
-					 Title: MacBook Air EFI Firmware Update, Version: 2.4, Size: 3817K, Recommended: YES, Action: restart,
-			* Label: Malware-1.0
-					 Title: * Label: Tailscale-0.99.0, Version: 1.0, Size: 968K, Recommended: NOT REALLY TBH,
-`),
-			want: "",
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			got := parseSoftwareupdateList(test.input)
-			if test.want != got {
-				t.Fatalf("got %q, want %q", got, test.want)
-			}
-		})
-	}
-}
-
 func TestUpdateYUMRepoTrack(t *testing.T) {
 	tests := []struct {
 		desc    string

cmd/tailscale/cli/serve_v2.go

@@ -39,15 +39,22 @@ type commandInfo struct {
 }
 
 var serveHelpCommon = strings.TrimSpace(`
-<target> can be a port number (e.g., 3000), a partial URL (e.g., localhost:3000), or a
-full URL including a path (e.g., http://localhost:3000/foo, https+insecure://localhost:3000/foo).
+<target> can be a file, directory, text, or most commonly the location to a service running on the
+local machine. The location to the location service can be expressed as a port number (e.g., 3000),
+a partial URL (e.g., localhost:3000), or a full URL including a path (e.g., http://localhost:3000/foo).
 
 EXAMPLES
-  - Mount a local web server at 127.0.0.1:3000 in the foreground:
-    $ tailscale %s localhost:3000
+  - Expose an HTTP server running at 127.0.0.1:3000 in the foreground:
+    $ tailscale %s 3000
 
-  - Mount a local web server at 127.0.0.1:3000 in the background:
-    $ tailscale %s --bg localhost:3000
+  - Expose an HTTP server running at 127.0.0.1:3000 in the background:
+    $ tailscale %s --bg 3000
+
+  - Expose an HTTPS server with a valid certificate at https://localhost:8443
+    $ tailscale %s https://localhost:8443
+
+  - Expose an HTTPS server with invalid or self-signed certificates at https://localhost:8443
+    $ tailscale %s https+insecure://localhost:8443
 
 For more examples and use cases visit our docs site https://tailscale.com/kb/1247/funnel-serve-use-cases
 `)
@@ -73,7 +80,7 @@ var infoMap = map[serveMode]commandInfo{
 		Name:      "serve",
 		ShortHelp: "Serve content and local servers on your tailnet",
 		LongHelp: strings.Join([]string{
-			"Serve enables you to share a local server securely within your tailnet.\n",
+			"Tailscale Serve enables you to share a local server securely within your tailnet.\n",
 			"To share a local server on the internet, use `tailscale funnel`\n\n",
 		}, "\n"),
 	},
@@ -115,12 +122,12 @@ func newServeV2Command(e *serveEnv, subcmd serveMode) *ffcli.Command {
 		Exec:     e.runServeCombined(subcmd),
 
 		FlagSet: e.newFlags("serve-set", func(fs *flag.FlagSet) {
-			fs.BoolVar(&e.bg, "bg", false, "run the command in the background")
-			fs.StringVar(&e.setPath, "set-path", "", "set a path for a specific target and run in the background")
-			fs.StringVar(&e.https, "https", "", "default; HTTPS listener")
-			fs.StringVar(&e.http, "http", "", "HTTP listener")
-			fs.StringVar(&e.tcp, "tcp", "", "TCP listener")
-			fs.StringVar(&e.tlsTerminatedTCP, "tls-terminated-tcp", "", "TLS terminated TCP listener")
+			fs.BoolVar(&e.bg, "bg", false, "Run the command as a background process")
+			fs.StringVar(&e.setPath, "set-path", "", "Appends the specified path to the base URL for accessing the underlying service")
+			fs.StringVar(&e.https, "https", "", "Expose an HTTPS server at the specified port (default")
+			fs.StringVar(&e.http, "http", "", "Expose an HTTP server at the specified port")
+			fs.StringVar(&e.tcp, "tcp", "", "Expose a TCP forwarder to forward raw TCP packets at the specified port")
+			fs.StringVar(&e.tlsTerminatedTCP, "tls-terminated-tcp", "", "Expose a TCP forwarder to forward TLS-terminated TCP packets at the specified port")
 
 		}),
 		UsageFunc: usageFunc,
@@ -381,7 +388,7 @@ func (e *serveEnv) setServe(sc *ipn.ServeConfig, st *ipnstate.Status, dnsName st
 var (
 	msgFunnelAvailable     = "Available on the internet:"
 	msgServeAvailable      = "Available within your tailnet:"
-	msgRunningInBackground = "Serve started and running in the background."
+	msgRunningInBackground = "%s started and running in the background."
 	msgDisableProxy        = "To disable the proxy, run: tailscale %s --%s=%d off"
 	msgToExit              = "Press Ctrl+C to exit."
 )

cmd/tailscale/cli/update.go

@@ -26,7 +26,6 @@ var updateCmd = &ffcli.Command{
 		fs := newFlagSet("update")
 		fs.BoolVar(&updateArgs.yes, "yes", false, "update without interactive prompts")
 		fs.BoolVar(&updateArgs.dryRun, "dry-run", false, "print what update would do without doing it, or prompts")
-		fs.BoolVar(&updateArgs.appStore, "app-store", false, "HIDDEN: check the App Store for updates, even if this is not an App Store install (for testing only)")
 		// These flags are not supported on several systems that only provide
 		// the latest version of Tailscale:
 		//
@@ -42,11 +41,10 @@ var updateCmd = &ffcli.Command{
 }
 
 var updateArgs struct {
-	yes      bool
-	dryRun   bool
-	appStore bool
-	track    string // explicit track; empty means same as current
-	version  string // explicit version; empty means auto
+	yes     bool
+	dryRun  bool
+	track   string // explicit track; empty means same as current
+	version string // explicit version; empty means auto
 }
 
 func runUpdate(ctx context.Context, args []string) error {
@@ -61,12 +59,11 @@ func runUpdate(ctx context.Context, args []string) error {
 		ver = updateArgs.track
 	}
 	err := clientupdate.Update(clientupdate.Arguments{
-		Version:  ver,
-		AppStore: updateArgs.appStore,
-		Logf:     func(f string, a ...any) { printf(f+"\n", a...) },
-		Stdout:   Stdout,
-		Stderr:   Stderr,
-		Confirm:  confirmUpdate,
+		Version: ver,
+		Logf:    func(f string, a ...any) { printf(f+"\n", a...) },
+		Stdout:  Stdout,
+		Stderr:  Stderr,
+		Confirm: confirmUpdate,
 	})
 	if errors.Is(err, errors.ErrUnsupported) {
 		return errors.New("The 'update' command is not supported on this platform; see https://tailscale.com/s/client-updates")

cmd/tailscaled/depaware.txt

@@ -133,7 +133,6 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
    L    github.com/pierrec/lz4/v4/internal/lz4errors                 from github.com/pierrec/lz4/v4+
    L    github.com/pierrec/lz4/v4/internal/lz4stream                 from github.com/pierrec/lz4/v4
    L    github.com/pierrec/lz4/v4/internal/xxh32                     from github.com/pierrec/lz4/v4/internal/lz4stream
-   W    github.com/pkg/errors                                        from github.com/tailscale/certstore
   LD    github.com/pkg/sftp                                          from tailscale.com/ssh/tailssh
   LD    github.com/pkg/sftp/internal/encoding/ssh/filexfer           from github.com/pkg/sftp
    W 💣 github.com/tailscale/certstore                               from tailscale.com/control/controlclient
@@ -367,7 +366,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/util/testenv                                   from tailscale.com/ipn/ipnlocal+
         tailscale.com/util/uniq                                      from tailscale.com/wgengine/magicsock+
         tailscale.com/util/vizerror                                  from tailscale.com/types/ipproto+
-     💣 tailscale.com/util/winutil                                   from tailscale.com/control/controlclient+
+     💣 tailscale.com/util/winutil                                   from tailscale.com/clientupdate+
    W 💣 tailscale.com/util/winutil/authenticode                      from tailscale.com/util/osdiag+
    W    tailscale.com/util/winutil/policy                            from tailscale.com/ipn/ipnlocal
         tailscale.com/version                                        from tailscale.com/derp+

control/controlclient/sign_supported.go

@@ -1,11 +1,9 @@
 // Copyright (c) Tailscale Inc & AUTHORS
 // SPDX-License-Identifier: BSD-3-Clause
 
-//go:build windows && cgo
+//go:build windows
 
-// darwin,cgo is also supported by certstore but machineCertificateSubject will
-// need to be loaded by a different mechanism, so this is not currently enabled
-// on darwin.
+// darwin,cgo is also supported by certstore but untested, so it is not enabled.
 
 package controlclient
 
@@ -21,7 +19,7 @@ import (
 	"github.com/tailscale/certstore"
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/key"
-	"tailscale.com/util/winutil"
+	"tailscale.com/util/syspolicy"
 )
 
 var getMachineCertificateSubjectOnce struct {
@@ -40,7 +38,7 @@ var getMachineCertificateSubjectOnce struct {
 // Example: "CN=Tailscale Inc Test Root CA,OU=Tailscale Inc Test Certificate Authority,O=Tailscale Inc,ST=ON,C=CA"
 func getMachineCertificateSubject() string {
 	getMachineCertificateSubjectOnce.Do(func() {
-		getMachineCertificateSubjectOnce.v, _ = winutil.GetRegString("MachineCertificateSubject")
+		getMachineCertificateSubjectOnce.v, _ = syspolicy.GetString("MachineCertificateSubject", "")
 	})
 
 	return getMachineCertificateSubjectOnce.v

control/controlclient/sign_unsupported.go

@@ -1,7 +1,7 @@
 // Copyright (c) Tailscale Inc & AUTHORS
 // SPDX-License-Identifier: BSD-3-Clause
 
-//go:build !windows || !cgo
+//go:build !windows
 
 package controlclient
 

go.mod

@@ -58,7 +58,7 @@ require (
 	github.com/prometheus/client_golang v1.17.0
 	github.com/prometheus/common v0.44.0
 	github.com/skip2/go-qrcode v0.0.0-20200617195104-da1b6568686e
-	github.com/tailscale/certstore v0.1.1-0.20220316223106-78d6e1c49d8d
+	github.com/tailscale/certstore v0.1.1-0.20231020161753-77811a65f4ff
 	github.com/tailscale/depaware v0.0.0-20210622194025-720c4b409502
 	github.com/tailscale/goexpect v0.0.0-20210902213824-6e8c725cea41
 	github.com/tailscale/golang-x-crypto v0.0.0-20230713185742-f0b76a10a08e

go.sum

@@ -864,8 +864,8 @@ github.com/subosito/gotenv v1.4.2 h1:X1TuBLAMDFbaTAChgCBLu3DU3UPyELpnF2jjJ2cz/S8
 github.com/subosito/gotenv v1.4.2/go.mod h1:ayKnFf/c6rvx/2iiLrJUk1e6plDbT3edrFNGqEflhK0=
 github.com/t-yuki/gocover-cobertura v0.0.0-20180217150009-aaee18c8195c h1:+aPplBwWcHBo6q9xrfWdMrT9o4kltkmmvpemgIjep/8=
 github.com/t-yuki/gocover-cobertura v0.0.0-20180217150009-aaee18c8195c/go.mod h1:SbErYREK7xXdsRiigaQiQkI9McGRzYMvlKYaP3Nimdk=
-github.com/tailscale/certstore v0.1.1-0.20220316223106-78d6e1c49d8d h1:K3j02b5j2Iw1xoggN9B2DIEkhWGheqFOeDkdJdBrJI8=
-github.com/tailscale/certstore v0.1.1-0.20220316223106-78d6e1c49d8d/go.mod h1:2P+hpOwd53e7JMX/L4f3VXkv1G+33ES6IWZSrkIeWNs=
+github.com/tailscale/certstore v0.1.1-0.20231020161753-77811a65f4ff h1:vnxdYZUJbsSRcIcduDW3DcQqfqaiv4FUgy25q8X+vfI=
+github.com/tailscale/certstore v0.1.1-0.20231020161753-77811a65f4ff/go.mod h1:XrBNfAFN+pwoWuksbFS9Ccxnopa15zJGgXRFN90l3K4=
 github.com/tailscale/depaware v0.0.0-20210622194025-720c4b409502 h1:34icjjmqJ2HPjrSuJYEkdZ+0ItmGQAQ75cRHIiftIyE=
 github.com/tailscale/depaware v0.0.0-20210622194025-720c4b409502/go.mod h1:p9lPsd+cx33L3H9nNoecRRxPssFKUwwI50I3pZ0yT+8=
 github.com/tailscale/goexpect v0.0.0-20210902213824-6e8c725cea41 h1:/V2rCMMWcsjYaYO2MeovLw+ClP63OtXgCF2Y1eb8+Ns=

ipn/ipnlocal/local.go

@@ -14,7 +14,6 @@ import (
 	"maps"
 	"net"
 	"net/http"
-	"net/http/httputil"
 	"net/netip"
 	"net/url"
 	"os"
@@ -268,7 +267,7 @@ type LocalBackend struct {
 	activeWatchSessions set.Set[string]     // of WatchIPN SessionID
 
 	serveListeners     map[netip.AddrPort]*serveListener // addrPort => serveListener
-	serveProxyHandlers sync.Map                          // string (HTTPHandler.Proxy) => *httputil.ReverseProxy
+	serveProxyHandlers sync.Map                          // string (HTTPHandler.Proxy) => *reverseProxy
 
 	// statusLock must be held before calling statusChanged.Wait() or
 	// statusChanged.Broadcast().
@@ -2344,16 +2343,7 @@ func (b *LocalBackend) onClientVersion(v *tailcfg.ClientVersion) {
 	b.mu.Lock()
 	b.lastClientVersion = v
 	b.mu.Unlock()
-	switch runtime.GOOS {
-	case "darwin", "ios":
-		// These auto-update well enough, and we haven't converted the
-		// ClientVersion types to Swift yet, so don't send them in ipn.Notify
-		// messages.
-	default:
-		// But everything else is a Go client and can deal with this field, even
-		// if they ignore it.
-		b.send(ipn.Notify{ClientVersion: v})
-	}
+	b.send(ipn.Notify{ClientVersion: v})
 }
 
 // For testing lazy machine key generation.
@@ -2770,11 +2760,17 @@ func (b *LocalBackend) CheckPrefs(p *ipn.Prefs) error {
 	return b.checkPrefsLocked(p)
 }
 
+// isConfigLocked_Locked reports whether the parsed config file is locked.
+// b.mu must be held.
+func (b *LocalBackend) isConfigLocked_Locked() bool {
+	// TODO(bradfitz,maisem): make this more fine-grained, permit changing
+	// some things if they're not explicitly set in the config. But for now
+	// (2023-10-16), just blanket disable everything.
+	return b.conf != nil && !b.conf.Parsed.Locked.EqualBool(false)
+}
+
 func (b *LocalBackend) checkPrefsLocked(p *ipn.Prefs) error {
-	if b.conf != nil && !b.conf.Parsed.Locked.EqualBool(false) {
-		// TODO(bradfitz,maisem): make this more fine-grained, permit changing
-		// some things if they're not explicitly set in the config. But for now
-		// (2023-10-16), just blanket disable everything.
+	if b.isConfigLocked_Locked() {
 		return errors.New("can't reconfigure tailscaled when using a config file; config file is locked")
 	}
 	var errs []error
@@ -4441,8 +4437,8 @@ func (b *LocalBackend) setServeProxyHandlersLocked() {
 		backend := key.(string)
 		if !backends[backend] {
 			b.logf("serve: closing idle connections to %s", backend)
-			value.(*httputil.ReverseProxy).Transport.(*http.Transport).CloseIdleConnections()
 			b.serveProxyHandlers.Delete(backend)
+			value.(*reverseProxy).close()
 		}
 		return true
 	})

ipn/ipnlocal/serve.go

@@ -23,6 +23,7 @@ import (
 	"strconv"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"time"
 
 	"golang.org/x/net/http2"
@@ -244,6 +245,9 @@ func (b *LocalBackend) setServeConfigLocked(config *ipn.ServeConfig, etag string
 	if config.IsFunnelOn() && prefs.ShieldsUp() {
 		return errors.New("Unable to turn on Funnel while shields-up is enabled")
 	}
+	if b.isConfigLocked_Locked() {
+		return errors.New("can't reconfigure tailscaled when using a config file; config file is locked")
+	}
 
 	nm := b.netMap
 	if nm == nil {
@@ -564,31 +568,52 @@ func (b *LocalBackend) proxyHandlerForBackend(backend string) (http.Handler, err
 // has application/grpc content type header, the connection will be over h2c.
 // Otherwise standard Go http transport will be used.
 type reverseProxy struct {
-	logf     logger.Logf
-	url      *url.URL
-	insecure bool
-	backend  string
-	lb       *LocalBackend
-	// transport for non-h2c backends
-	httpTransport lazy.SyncValue[http.RoundTripper]
-	// transport for h2c backends
-	h2cTransport lazy.SyncValue[http.RoundTripper]
+	logf logger.Logf
+	url  *url.URL
+	// insecure tracks whether the connection to an https backend should be
+	// insecure (i.e because we cannot verify its CA).
+	insecure      bool
+	backend       string
+	lb            *LocalBackend
+	httpTransport lazy.SyncValue[*http.Transport]  // transport for non-h2c backends
+	h2cTransport  lazy.SyncValue[*http2.Transport] // transport for h2c backends
+	// closed tracks whether proxy is closed/currently closing.
+	closed atomic.Bool
+}
+
+// close ensures that any open backend connections get closed.
+func (rp *reverseProxy) close() {
+	rp.closed.Store(true)
+	if h2cT := rp.h2cTransport.Get(func() *http2.Transport {
+		return nil
+	}); h2cT != nil {
+		h2cT.CloseIdleConnections()
+	}
+	if httpTransport := rp.httpTransport.Get(func() *http.Transport {
+		return nil
+	}); httpTransport != nil {
+		httpTransport.CloseIdleConnections()
+	}
 }
 
 func (rp *reverseProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	if closed := rp.closed.Load(); closed {
+		rp.logf("received a request for a proxy that's being closed or has been closed")
+		http.Error(w, "proxy is closed", http.StatusServiceUnavailable)
+		return
+	}
 	p := &httputil.ReverseProxy{Rewrite: func(r *httputil.ProxyRequest) {
 		r.SetURL(rp.url)
 		r.Out.Host = r.In.Host
 		addProxyForwardedHeaders(r)
 		rp.lb.addTailscaleIdentityHeaders(r)
-	},
-	}
+	}}
 
 	// There is no way to autodetect h2c as per RFC 9113
 	// https://datatracker.ietf.org/doc/html/rfc9113#name-starting-http-2.
 	// However, we assume that http:// proxy prefix in combination with the
 	// protoccol being HTTP/2 is sufficient to detect h2c for our needs. Only use this for
-	// gRPC to fix a known problem pf plaintext gRPC backends
+	// gRPC to fix a known problem of plaintext gRPC backends
 	if rp.shouldProxyViaH2C(r) {
 		rp.logf("received a proxy request for plaintext gRPC")
 		p.Transport = rp.getH2CTransport()
@@ -596,13 +621,12 @@ func (rp *reverseProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		p.Transport = rp.getTransport()
 	}
 	p.ServeHTTP(w, r)
-
 }
 
-// getTransport gets transport for http backends. Transport gets created lazily
-// at most once.
-func (rp *reverseProxy) getTransport() http.RoundTripper {
-	return rp.httpTransport.Get(func() http.RoundTripper {
+// getTransport returns the Transport used for regular (non-GRPC) requests
+// to the backend. The Transport gets created lazily, at most once.
+func (rp *reverseProxy) getTransport() *http.Transport {
+	return rp.httpTransport.Get(func() *http.Transport {
 		return &http.Transport{
 			DialContext: rp.lb.dialer.SystemDial,
 			TLSClientConfig: &tls.Config{
@@ -618,10 +642,10 @@ func (rp *reverseProxy) getTransport() http.RoundTripper {
 	})
 }
 
-// getH2CTranport gets transport for h2c backends. Creates it lazily at most
-// once.
-func (rp *reverseProxy) getH2CTransport() http.RoundTripper {
-	return rp.h2cTransport.Get(func() http.RoundTripper {
+// getH2CTransport returns the Transport used for GRPC requests to the backend.
+// The Transport gets created lazily, at most once.
+func (rp *reverseProxy) getH2CTransport() *http2.Transport {
+	return rp.h2cTransport.Get(func() *http2.Transport {
 		return &http2.Transport{
 			AllowHTTP: true,
 			DialTLSContext: func(ctx context.Context, network string, addr string, _ *tls.Config) (net.Conn, error) {

ipn/ipnlocal/serve_test.go

@@ -18,6 +18,7 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
+	"reflect"
 	"strings"
 	"testing"
 	"time"
@@ -446,6 +447,119 @@ func TestServeHTTPProxy(t *testing.T) {
 	}
 }
 
+func Test_reverseProxyConfiguration(t *testing.T) {
+	b := newTestBackend(t)
+	type test struct {
+		backend string
+		path    string
+		// set to false to test that a proxy has been removed
+		shouldExist   bool
+		wantsInsecure bool
+		wantsURL      url.URL
+	}
+	runner := func(name string, tests []test) {
+		t.Logf("running tests for %s", name)
+		host := ipn.HostPort("http://example.ts.net:80")
+		conf := &ipn.ServeConfig{
+			Web: map[ipn.HostPort]*ipn.WebServerConfig{
+				host: {Handlers: map[string]*ipn.HTTPHandler{}},
+			},
+		}
+		for _, tt := range tests {
+			if tt.shouldExist {
+				conf.Web[host].Handlers[tt.path] = &ipn.HTTPHandler{Proxy: tt.backend}
+			}
+		}
+		if err := b.setServeConfigLocked(conf, ""); err != nil {
+			t.Fatal(err)
+		}
+		// test that reverseproxies have been set up as expected
+		for _, tt := range tests {
+			rp, ok := b.serveProxyHandlers.Load(tt.backend)
+			if !tt.shouldExist && ok {
+				t.Errorf("proxy for backend %s should not exist, but it does", tt.backend)
+			}
+			if !tt.shouldExist {
+				continue
+			}
+			parsedRp, ok := rp.(*reverseProxy)
+			if !ok {
+				t.Errorf("proxy for backend %q is not a reverseproxy", tt.backend)
+			}
+			if parsedRp.insecure != tt.wantsInsecure {
+				t.Errorf("proxy for backend %q should be insecure: %v got insecure: %v", tt.backend, tt.wantsInsecure, parsedRp.insecure)
+			}
+			if !reflect.DeepEqual(*parsedRp.url, tt.wantsURL) {
+				t.Errorf("proxy for backend %q should have URL %#+v, got URL %+#v", tt.backend, &tt.wantsURL, parsedRp.url)
+			}
+			if tt.backend != parsedRp.backend {
+				t.Errorf("proxy for backend %q should have backend %q got %q", tt.backend, tt.backend, parsedRp.backend)
+			}
+		}
+	}
+
+	// configure local backend with some proxy backends
+	runner("initial proxy configs", []test{
+		{
+			backend:       "http://example.com/docs",
+			path:          "/example",
+			shouldExist:   true,
+			wantsInsecure: false,
+			wantsURL:      mustCreateURL(t, "http://example.com/docs"),
+		},
+		{
+			backend:       "https://example1.com",
+			path:          "/example1",
+			shouldExist:   true,
+			wantsInsecure: false,
+			wantsURL:      mustCreateURL(t, "https://example1.com"),
+		},
+		{
+			backend:       "https+insecure://example2.com",
+			path:          "/example2",
+			shouldExist:   true,
+			wantsInsecure: true,
+			wantsURL:      mustCreateURL(t, "https://example2.com"),
+		},
+	})
+
+	// reconfigure the local backend with different proxies
+	runner("reloaded proxy configs", []test{
+		{
+			backend:       "http://example.com/docs",
+			path:          "/example",
+			shouldExist:   true,
+			wantsInsecure: false,
+			wantsURL:      mustCreateURL(t, "http://example.com/docs"),
+		},
+		{
+			backend:     "https://example1.com",
+			shouldExist: false,
+		},
+		{
+			backend:     "https+insecure://example2.com",
+			shouldExist: false,
+		},
+		{
+			backend:       "https+insecure://example3.com",
+			path:          "/example3",
+			shouldExist:   true,
+			wantsInsecure: true,
+			wantsURL:      mustCreateURL(t, "https://example3.com"),
+		},
+	})
+
+}
+
+func mustCreateURL(t *testing.T, u string) url.URL {
+	t.Helper()
+	uParsed, err := url.Parse(u)
+	if err != nil {
+		t.Fatalf("failed parsing url: %v", err)
+	}
+	return *uParsed
+}
+
 func newTestBackend(t *testing.T) *LocalBackend {
 	sys := &tsd.System{}
 	e, err := wgengine.NewUserspaceEngine(t.Logf, wgengine.Config{SetSubsystem: sys.Set})
@@ -589,40 +703,21 @@ func TestServeFileOrDirectory(t *testing.T) {
 }
 
 func Test_isGRPCContentType(t *testing.T) {
-	tests := map[string]struct {
+	tests := []struct {
 		contentType string
 		want        bool
 	}{
-		"application/grpc": {
-			contentType: "application/grpc",
-			want:        true,
-		},
-		"application/grpc;": {
-			contentType: "application/grpc;",
-			want:        true,
-		},
-		"application/grpc+": {
-			contentType: "application/grpc+",
-			want:        true,
-		},
-		"application/grpcfoobar": {
-			contentType: "application/grpcfoobar",
-		},
-		"application/text": {
-			contentType: "application/text",
-		},
-		"foobar": {
-			contentType: "foobar",
-		},
-		"no content type": {
-			contentType: "",
-		},
+		{contentType: "application/grpc", want: true},
+		{contentType: "application/grpc;", want: true},
+		{contentType: "application/grpc+", want: true},
+		{contentType: "application/grpcfoobar"},
+		{contentType: "application/text"},
+		{contentType: "foobar"},
+		{contentType: ""},
 	}
-	for name, scenario := range tests {
-		t.Run(name, func(t *testing.T) {
-			if got := isGRPCContentType(scenario.contentType); got != scenario.want {
-				t.Errorf("test case %s failed, isGRPCContentType() = %v, want %v", name, got, scenario.want)
-			}
-		})
+	for _, tt := range tests {
+		if got := isGRPCContentType(tt.contentType); got != tt.want {
+			t.Errorf("isGRPCContentType(%q) = %v, want %v", tt.contentType, got, tt.want)
+		}
 	}
 }

ipn/localapi/localapi.go

@@ -2172,12 +2172,13 @@ func (h *Handler) serveDebugWebClient(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
-	defer resp.Body.Close()
-
-	if _, err := io.Copy(w, resp.Body); err != nil {
-		http.Error(w, err.Error(), http.StatusInternalServerError)
+	body, _ := io.ReadAll(resp.Body)
+	resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		http.Error(w, string(body), resp.StatusCode)
 		return
 	}
+	w.Write(body)
 	w.Header().Set("Content-Type", "application/json")
 }
 

taildrop/delete.go

@@ -27,8 +27,8 @@ const deleteDelay = time.Hour
 type fileDeleter struct {
 	logf  logger.Logf
 	clock tstime.DefaultClock
-	event func(string) // called for certain events; for testing only
 	dir   string
+	event func(string) // called for certain events; for testing only
 
 	mu     sync.Mutex
 	queue  list.List
@@ -46,11 +46,11 @@ type deleteFile struct {
 	inserted time.Time
 }
 
-func (d *fileDeleter) Init(logf logger.Logf, clock tstime.DefaultClock, event func(string), dir string) {
-	d.logf = logf
-	d.clock = clock
-	d.dir = dir
-	d.event = event
+func (d *fileDeleter) Init(m *Manager, eventHook func(string)) {
+	d.logf = m.opts.Logf
+	d.clock = m.opts.Clock
+	d.dir = m.opts.Dir
+	d.event = eventHook
 
 	// From a cold-start, load the list of partial and deleted files.
 	d.byName = make(map[string]*list.Element)
@@ -59,19 +59,30 @@ func (d *fileDeleter) Init(logf logger.Logf, clock tstime.DefaultClock, event fu
 	d.group.Go(func() {
 		d.event("start init")
 		defer d.event("end init")
-		rangeDir(dir, func(de fs.DirEntry) bool {
+		rangeDir(d.dir, func(de fs.DirEntry) bool {
 			switch {
 			case d.shutdownCtx.Err() != nil:
 				return false // terminate early
 			case !de.Type().IsRegular():
 				return true
-			case strings.Contains(de.Name(), partialSuffix):
-				d.Insert(de.Name())
-			case strings.Contains(de.Name(), deletedSuffix):
+			case strings.HasSuffix(de.Name(), partialSuffix):
+				// Only enqueue the file for deletion if there is no active put.
+				nameID := strings.TrimSuffix(de.Name(), partialSuffix)
+				if i := strings.LastIndexByte(nameID, '.'); i > 0 {
+					key := incomingFileKey{ClientID(nameID[i+len("."):]), nameID[:i]}
+					m.incomingFiles.LoadFunc(key, func(_ *incomingFile, loaded bool) {
+						if !loaded {
+							d.Insert(de.Name())
+						}
+					})
+				} else {
+					d.Insert(de.Name())
+				}
+			case strings.HasSuffix(de.Name(), deletedSuffix):
 				// Best-effort immediate deletion of deleted files.
 				name := strings.TrimSuffix(de.Name(), deletedSuffix)
-				if os.Remove(filepath.Join(dir, name)) == nil {
-					if os.Remove(filepath.Join(dir, de.Name())) == nil {
+				if os.Remove(filepath.Join(d.dir, name)) == nil {
+					if os.Remove(filepath.Join(d.dir, de.Name())) == nil {
 						break
 					}
 				}

taildrop/delete_test.go

@@ -67,8 +67,12 @@ func TestDeleter(t *testing.T) {
 	}
 	eventHook := func(event string) { eventsChan <- event }
 
+	var m Manager
 	var fd fileDeleter
-	fd.Init(t.Logf, tstime.DefaultClock{Clock: clock}, eventHook, dir)
+	m.opts.Logf = t.Logf
+	m.opts.Clock = tstime.DefaultClock{Clock: clock}
+	m.opts.Dir = dir
+	fd.Init(&m, eventHook)
 	defer fd.Shutdown()
 	insert := func(name string) {
 		t.Helper()

taildrop/resume.go

@@ -145,7 +145,7 @@ func ResumeReader(r io.Reader, hashNext func() (BlockChecksum, error)) (int64, i
 		}
 
 		// Read the contents of the next block.
-		n, err := io.ReadFull(r, b[:blockSize])
+		n, err := io.ReadFull(r, b[:cs.Size])
 		b = b[:n]
 		if err == io.EOF || err == io.ErrUnexpectedEOF {
 			err = nil

taildrop/retrieve.go

@@ -112,16 +112,15 @@ func (m *Manager) DeleteFile(baseName string) error {
 		err := os.Remove(path)
 		if err != nil && !os.IsNotExist(err) {
 			err = redactError(err)
-			// Put a retry loop around deletes on Windows. Windows
-			// file descriptor closes are effectively asynchronous,
-			// as a bunch of hooks run on/after close, and we can't
-			// necessarily delete the file for a while after close,
-			// as we need to wait for everybody to be done with
-			// it. (on Windows, unlike Unix, a file can't be deleted
-			// if it's open anywhere)
-			// So try a few times but ultimately just leave a
-			// "foo.jpg.deleted" marker file to note that it's
-			// deleted and we clean it up later.
+			// Put a retry loop around deletes on Windows.
+			//
+			// Windows file descriptor closes are effectively asynchronous,
+			// as a bunch of hooks run on/after close,
+			// and we can't necessarily delete the file for a while after close,
+			// as we need to wait for everybody to be done with it.
+			// On Windows, unlike Unix, a file can't be deleted if it's open anywhere.
+			// So try a few times but ultimately just leave a "foo.jpg.deleted"
+			// marker file to note that it's deleted and we clean it up later.
 			if runtime.GOOS == "windows" {
 				if bo == nil {
 					bo = backoff.NewBackoff("delete-retry", logf, 1*time.Second)

taildrop/taildrop.go

@@ -135,7 +135,7 @@ func (opts ManagerOptions) New() *Manager {
 		opts.SendFileNotify = func() {}
 	}
 	m := &Manager{opts: opts}
-	m.deleter.Init(opts.Logf, opts.Clock, func(string) {}, opts.Dir)
+	m.deleter.Init(m, func(string) {})
 	m.emptySince.Store(-1) // invalidate this cache
 	return m
 }
@@ -250,10 +250,6 @@ func (m *Manager) IncomingFiles() []ipn.PartialFile {
 	return files
 }
 
-// redacted is a fake path name we use in errors, to avoid
-// accidentally logging actual filenames anywhere.
-const redacted = "redacted"
-
 type redactedError struct {
 	msg   string
 	inner error
@@ -270,6 +266,7 @@ func (re *redactedError) Unwrap() error {
 func redactString(s string) string {
 	hash := adler32.Checksum([]byte(s))
 
+	const redacted = "redacted"
 	var buf [len(redacted) + len(".12345678")]byte
 	b := append(buf[:0], []byte(redacted)...)
 	b = append(b, '.')

version/prop.go

@@ -47,19 +47,7 @@ var isSandboxedMacOS lazy.SyncValue[bool]
 // and macsys (System Extension) version on macOS, and false for
 // tailscaled-on-macOS.
 func IsSandboxedMacOS() bool {
-	if runtime.GOOS != "darwin" {
-		return false
-	}
-	return isSandboxedMacOS.Get(func() bool {
-		if IsMacSysExt() {
-			return true
-		}
-		exe, err := os.Executable()
-		if err != nil {
-			return false
-		}
-		return filepath.Base(exe) == "io.tailscale.ipn.macsys.network-extension" || strings.HasSuffix(exe, "/Contents/MacOS/Tailscale") || strings.HasSuffix(exe, "/Contents/MacOS/IPNExtension")
-	})
+	return IsMacAppStore() || IsMacSysExt()
 }
 
 var isMacSysExt lazy.SyncValue[bool]
@@ -79,6 +67,23 @@ func IsMacSysExt() bool {
 	})
 }
 
+var isMacAppStore lazy.SyncValue[bool]
+
+// IsMacAppStore whether this binary is from the App Store version of Tailscale
+// for macOS.
+func IsMacAppStore() bool {
+	if runtime.GOOS != "darwin" {
+		return false
+	}
+	return isMacAppStore.Get(func() bool {
+		exe, err := os.Executable()
+		if err != nil {
+			return false
+		}
+		return strings.HasSuffix(exe, "/Contents/MacOS/Tailscale") || strings.HasSuffix(exe, "/Contents/MacOS/IPNExtension")
+	})
+}
+
 var isAppleTV lazy.SyncValue[bool]
 
 // IsAppleTV reports whether this binary is part of the Tailscale network extension for tvOS.