From bc502b52f5a2d7a371d038bfaebe11bcfae978a9 Mon Sep 17 00:00:00 2001 From: Ethan <39577870+ethanndickson@users.noreply.github.com> Date: Wed, 6 Aug 2025 13:11:45 +1000 Subject: [PATCH] chore: cherry-pick coder desktop + corporate vpn fixes for 2.24 (#19177) This backports the coder/coder fixes for https://github.com/coder/coder-desktop-windows/issues/147 and https://github.com/coder/coder-desktop-macos/issues/201 to v2.24, and fixes a bug where Coder Desktop logs were duplicated on Windows. 1. https://github.com/coder/coder/pull/19124 (`c64b3c0d8327edbfad4cc7bc373bdb873cd7e26b`) - Required for CI to pass. 2. https://github.com/coder/coder/pull/19125 (`9f2dc72d9a3d9feecf28bc30cd1234394a0e5c37`) - Required for CI to pass. 3. https://github.com/coder/coder/pull/19143 (`914daac2dcf700817d4ae248a6e8014f64654fe7`) - Required for CI to pass. 4. https://github.com/coder/coder/pull/19023 (`618121cba39833db7c1ca4653d7ce7745d5aff57`) - First implementation of fix for https://github.com/coder/coder-desktop-windows/issues/147 5. https://github.com/coder/coder/pull/19069 (`ba0b124a1daaddde590f5839afebe803330981be`) - Partially reverts previous commit, actual fix for https://github.com/coder/coder-desktop-windows/issues/147 6. https://github.com/coder/coder/pull/19052 (`19171e27c9624c435f97d4ae5e3b61c649bc7063`) - Avoid duplicating logs on Coder Desktop Windows 7. https://github.com/coder/coder/pull/19080 (`18b037d0086cd6eca11e6180b36a302badaa5a5f`) - Updates coder/tailscale reference, lets Coder Desktop macOS use the slim binary to run Coder Connect. I've tested my latest build of Coder Desktop against a Coder server running this branch, just as a sanity check. --------- Co-authored-by: Cian Johnston Co-authored-by: Dean Sheather --- .github/workflows/ci.yaml | 7 +++- .github/workflows/release.yaml | 4 +- cli/vpndaemon_darwin.go | 73 ++++++++++++++++++++++++++++++++++ cli/vpndaemon_other.go | 2 +- cli/vpndaemon_windows.go | 5 +-- go.mod | 2 +- go.sum | 4 +- tailnet/conn.go | 14 ++++++- vpn/tunnel.go | 6 --- 9 files changed, 98 insertions(+), 19 deletions(-) create mode 100644 cli/vpndaemon_darwin.go diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 7a0bdbd435..8e442e4a53 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -428,6 +428,11 @@ jobs: - name: Disable Spotlight Indexing if: runner.os == 'macOS' run: | + enabled=$(sudo mdutil -a -s | grep "Indexing enabled" | wc -l) + if [ $enabled -eq 0 ]; then + echo "Spotlight indexing is already disabled" + exit 0 + fi sudo mdutil -a -i off sudo mdutil -X / sudo launchctl bootout system /System/Library/LaunchDaemons/com.apple.metadata.mds.plist @@ -1082,7 +1087,7 @@ jobs: - name: Switch XCode Version uses: maxim-lobanov/setup-xcode@60606e260d2fc5762a71e64e74b2174e8ea3c8bd # v1.6.0 with: - xcode-version: "16.0.0" + xcode-version: "16.1.0" - name: Setup Go uses: ./.github/actions/setup-go diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 1d5ebb5aa2..d032cd3e87 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -60,7 +60,7 @@ jobs: - name: Switch XCode Version uses: maxim-lobanov/setup-xcode@60606e260d2fc5762a71e64e74b2174e8ea3c8bd # v1.6.0 with: - xcode-version: "16.0.0" + xcode-version: "16.1.0" - name: Setup Go uses: ./.github/actions/setup-go @@ -655,7 +655,7 @@ jobs: detached_signature="${binary}.asc" gcloud storage cp "./site/out/bin/${binary}" "gs://releases.coder.com/coder-cli/${version}/${binary}" gcloud storage cp "./site/out/bin/${detached_signature}" "gs://releases.coder.com/coder-cli/${version}/${detached_signature}" - done + done - name: Publish release run: | diff --git a/cli/vpndaemon_darwin.go b/cli/vpndaemon_darwin.go new file mode 100644 index 0000000000..a1b836dd6b --- /dev/null +++ b/cli/vpndaemon_darwin.go @@ -0,0 +1,73 @@ +//go:build darwin + +package cli + +import ( + "golang.org/x/xerrors" + + "cdr.dev/slog" + "github.com/coder/coder/v2/vpn" + "github.com/coder/serpent" +) + +func (r *RootCmd) vpnDaemonRun() *serpent.Command { + var ( + rpcReadFD int64 + rpcWriteFD int64 + ) + + cmd := &serpent.Command{ + Use: "run", + Short: "Run the VPN daemon on macOS.", + Middleware: serpent.Chain( + serpent.RequireNArgs(0), + ), + Options: serpent.OptionSet{ + { + Flag: "rpc-read-fd", + Env: "CODER_VPN_DAEMON_RPC_READ_FD", + Description: "The file descriptor for the pipe to read from the RPC connection.", + Value: serpent.Int64Of(&rpcReadFD), + Required: true, + }, + { + Flag: "rpc-write-fd", + Env: "CODER_VPN_DAEMON_RPC_WRITE_FD", + Description: "The file descriptor for the pipe to write to the RPC connection.", + Value: serpent.Int64Of(&rpcWriteFD), + Required: true, + }, + }, + Handler: func(inv *serpent.Invocation) error { + ctx := inv.Context() + + if rpcReadFD < 0 || rpcWriteFD < 0 { + return xerrors.Errorf("rpc-read-fd (%v) and rpc-write-fd (%v) must be positive", rpcReadFD, rpcWriteFD) + } + if rpcReadFD == rpcWriteFD { + return xerrors.Errorf("rpc-read-fd (%v) and rpc-write-fd (%v) must be different", rpcReadFD, rpcWriteFD) + } + + pipe, err := vpn.NewBidirectionalPipe(uintptr(rpcReadFD), uintptr(rpcWriteFD)) + if err != nil { + return xerrors.Errorf("create bidirectional RPC pipe: %w", err) + } + defer pipe.Close() + + tunnel, err := vpn.NewTunnel(ctx, slog.Make().Leveled(slog.LevelDebug), pipe, + vpn.NewClient(), + vpn.UseOSNetworkingStack(), + vpn.UseAsLogger(), + ) + if err != nil { + return xerrors.Errorf("create new tunnel for client: %w", err) + } + defer tunnel.Close() + + <-ctx.Done() + return nil + }, + } + + return cmd +} diff --git a/cli/vpndaemon_other.go b/cli/vpndaemon_other.go index 2e3e39b1b9..1526efb011 100644 --- a/cli/vpndaemon_other.go +++ b/cli/vpndaemon_other.go @@ -1,4 +1,4 @@ -//go:build !windows +//go:build !windows && !darwin package cli diff --git a/cli/vpndaemon_windows.go b/cli/vpndaemon_windows.go index cf74558ffa..6c2d147da2 100644 --- a/cli/vpndaemon_windows.go +++ b/cli/vpndaemon_windows.go @@ -63,10 +63,7 @@ func (r *RootCmd) vpnDaemonRun() *serpent.Command { defer pipe.Close() logger.Info(ctx, "starting tunnel") - tunnel, err := vpn.NewTunnel(ctx, logger, pipe, vpn.NewClient(), - vpn.UseOSNetworkingStack(), - vpn.UseCustomLogSinks(sinks...), - ) + tunnel, err := vpn.NewTunnel(ctx, logger, pipe, vpn.NewClient(), vpn.UseOSNetworkingStack()) if err != nil { return xerrors.Errorf("create new tunnel for client: %w", err) } diff --git a/go.mod b/go.mod index ad95555900..b440475458 100644 --- a/go.mod +++ b/go.mod @@ -36,7 +36,7 @@ replace github.com/tcnksm/go-httpstat => github.com/coder/go-httpstat v0.0.0-202 // There are a few minor changes we make to Tailscale that we're slowly upstreaming. Compare here: // https://github.com/tailscale/tailscale/compare/main...coder:tailscale:main -replace tailscale.com => github.com/coder/tailscale v1.1.1-0.20250611020837-f14d20d23d8c +replace tailscale.com => github.com/coder/tailscale v1.1.1-0.20250729141742-067f1e5d9716 // This is replaced to include // 1. a fix for a data race: c.f. https://github.com/tailscale/wireguard-go/pull/25 diff --git a/go.sum b/go.sum index 45070b45fd..8496867edf 100644 --- a/go.sum +++ b/go.sum @@ -926,8 +926,8 @@ github.com/coder/serpent v0.10.0 h1:ofVk9FJXSek+SmL3yVE3GoArP83M+1tX+H7S4t8BSuM= github.com/coder/serpent v0.10.0/go.mod h1:cZFW6/fP+kE9nd/oRkEHJpG6sXCtQ+AX7WMMEHv0Y3Q= github.com/coder/ssh v0.0.0-20231128192721-70855dedb788 h1:YoUSJ19E8AtuUFVYBpXuOD6a/zVP3rcxezNsoDseTUw= github.com/coder/ssh v0.0.0-20231128192721-70855dedb788/go.mod h1:aGQbuCLyhRLMzZF067xc84Lh7JDs1FKwCmF1Crl9dxQ= -github.com/coder/tailscale v1.1.1-0.20250611020837-f14d20d23d8c h1:d/qBIi3Ez7KkopRgNtfdvTMqvqBg47d36qVfkd3C5EQ= -github.com/coder/tailscale v1.1.1-0.20250611020837-f14d20d23d8c/go.mod h1:l7ml5uu7lFh5hY28lGYM4b/oFSmuPHYX6uk4RAu23Lc= +github.com/coder/tailscale v1.1.1-0.20250729141742-067f1e5d9716 h1:hi7o0sA+RPBq8Rvvz+hNrC/OTL2897OKREMIRIuQeTs= +github.com/coder/tailscale v1.1.1-0.20250729141742-067f1e5d9716/go.mod h1:l7ml5uu7lFh5hY28lGYM4b/oFSmuPHYX6uk4RAu23Lc= github.com/coder/terraform-config-inspect v0.0.0-20250107175719-6d06d90c630e h1:JNLPDi2P73laR1oAclY6jWzAbucf70ASAvf5mh2cME0= github.com/coder/terraform-config-inspect v0.0.0-20250107175719-6d06d90c630e/go.mod h1:Gz/z9Hbn+4KSp8A2FBtNszfLSdT2Tn/uAKGuVqqWmDI= github.com/coder/terraform-provider-coder/v2 v2.7.1-0.20250623193313-e890833351e2 h1:vtGzECz5CyzuxMODexWdIRxhYLqyTcHafuJpH60PYhM= diff --git a/tailnet/conn.go b/tailnet/conn.go index c3ebd246c5..709d5b2958 100644 --- a/tailnet/conn.go +++ b/tailnet/conn.go @@ -65,7 +65,9 @@ const EnvMagicsockDebugLogging = "CODER_MAGICSOCK_DEBUG_LOGGING" func init() { // Globally disable network namespacing. All networking happens in - // userspace. + // userspace unless the connection is configured to use a TUN. + // NOTE: this exists in init() so it affects all connections (incl. DERP) + // made by tailscale packages by default. netns.SetEnabled(false) // Tailscale, by default, "trims" the set of peers down to ones that we are // "actively" communicating with in an effort to save memory. Since @@ -100,6 +102,7 @@ type Options struct { BlockEndpoints bool Logger slog.Logger ListenPort uint16 + // CaptureHook is a callback that captures Disco packets and packets sent // into the tailnet tunnel. CaptureHook capture.Callback @@ -154,7 +157,14 @@ func NewConn(options *Options) (conn *Conn, err error) { return nil, xerrors.New("At least one IP range must be provided") } - netns.SetEnabled(options.TUNDev != nil) + useNetNS := options.TUNDev != nil + options.Logger.Debug(context.Background(), "network isolation configuration", slog.F("use_netns", useNetNS)) + netns.SetEnabled(useNetNS) + // The Coder soft isolation mode is a workaround to allow Coder Connect to + // connect to Coder servers behind corporate VPNs, and relaxes some of the + // loop protections that come with Tailscale. + // See the comment above the netns function for more details. + netns.SetCoderSoftIsolation(useNetNS) var telemetryStore *TelemetryStore if options.TelemetrySink != nil { diff --git a/vpn/tunnel.go b/vpn/tunnel.go index e4624ac182..30ee56c239 100644 --- a/vpn/tunnel.go +++ b/vpn/tunnel.go @@ -192,12 +192,6 @@ func UseAsLogger() TunnelOption { } } -func UseCustomLogSinks(sinks ...slog.Sink) TunnelOption { - return func(t *Tunnel) { - t.clientLogger = t.clientLogger.AppendSinks(sinks...) - } -} - func WithClock(clock quartz.Clock) TunnelOption { return func(t *Tunnel) { t.clock = clock