Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions host/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,17 +218,28 @@ func installBinary() (string, error) {
}
defer src.Close()

dst, err := os.OpenFile(destPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0755)
if err := replaceBinary(destPath, src, 0755); err != nil {
return "", err
}

return destPath, nil
}

// copyFile writes src to destPath, creating or truncating it. It opens the
// destination before reading src, so if the open fails (for example, a Windows
// sharing violation when destPath is a running executable) it returns without
// consuming src, letting a caller retry the same reader against another path.
func copyFile(destPath string, src io.Reader, perm os.FileMode) error {
dst, err := os.OpenFile(destPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, perm)
if err != nil {
return "", fmt.Errorf("failed to create destination binary: %w", err)
return fmt.Errorf("failed to create destination binary: %w", err)
}
defer dst.Close()

if _, err := io.Copy(dst, src); err != nil {
return "", fmt.Errorf("failed to copy binary: %w", err)
return fmt.Errorf("failed to copy binary: %w", err)
}

return destPath, nil
return nil
}

// writeManifest writes a native messaging host manifest JSON file.
Expand Down
7 changes: 7 additions & 0 deletions host/install_darwin.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"io"
"os"
"path/filepath"
)
Expand Down Expand Up @@ -61,3 +62,9 @@ func browserHasFootprint(target chromiumBrowserTarget) bool {
}

func platformPostInstallFirefox(_ string) error { return nil }

// replaceBinary installs the new host binary at destPath. A running executable
// can be overwritten in place on this platform, so this is a straight copy.
func replaceBinary(destPath string, src io.Reader, perm os.FileMode) error {
return copyFile(destPath, src, perm)
}
7 changes: 7 additions & 0 deletions host/install_linux.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"io"
"os"
"path/filepath"
)
Expand Down Expand Up @@ -60,3 +61,9 @@ func browserHasFootprint(target chromiumBrowserTarget) bool {
}

func platformPostInstallFirefox(_ string) error { return nil }

// replaceBinary installs the new host binary at destPath. A running executable
// can be overwritten in place on this platform, so this is a straight copy.
func replaceBinary(destPath string, src io.Reader, perm os.FileMode) error {
return copyFile(destPath, src, perm)
}
41 changes: 41 additions & 0 deletions host/install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,47 @@ func TestUninstallIsIdempotent(t *testing.T) {
}
}

func TestReplaceBinaryCreatesWhenAbsent(t *testing.T) {
dir := t.TempDir()
dest := filepath.Join(dir, "tailscale-browser-ext")
if err := replaceBinary(dest, strings.NewReader("NEW"), 0o755); err != nil {
t.Fatalf("replaceBinary returned error: %v", err)
}
got, err := os.ReadFile(dest)
if err != nil {
t.Fatalf("read dest: %v", err)
}
if string(got) != "NEW" {
t.Errorf("dest content = %q, want %q", got, "NEW")
}
info, err := os.Stat(dest)
if err != nil {
t.Fatalf("stat dest: %v", err)
}
if info.Mode().Perm()&0o100 == 0 {
t.Errorf("dest is not owner-executable: mode=%v", info.Mode().Perm())
}
}

func TestReplaceBinaryOverwritesAndTruncates(t *testing.T) {
dir := t.TempDir()
dest := filepath.Join(dir, "tailscale-browser-ext")
// Seed with longer content so a non-truncating write would leave a tail.
if err := os.WriteFile(dest, []byte("OLDOLDOLD"), 0o755); err != nil {
t.Fatalf("seed dest: %v", err)
}
if err := replaceBinary(dest, strings.NewReader("NEW"), 0o755); err != nil {
t.Fatalf("replaceBinary returned error: %v", err)
}
got, err := os.ReadFile(dest)
if err != nil {
t.Fatalf("read dest: %v", err)
}
if string(got) != "NEW" {
t.Errorf("dest content = %q, want %q (old content not fully replaced)", got, "NEW")
}
}

func TestInstallChromiumFamilyParentExistedTrueWhenDirPresent(t *testing.T) {
setupTempHome(t)
dirs := chromiumManifestDirs()
Expand Down
45 changes: 45 additions & 0 deletions host/install_windows.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package main

import (
"errors"
"fmt"
"io"
"os"
"path/filepath"
"strings"

"golang.org/x/sys/windows"
"golang.org/x/sys/windows/registry"
)

Expand Down Expand Up @@ -136,3 +139,45 @@ func removeRegistryKey(baseKey registry.Key, path string) error {
}
return nil
}

// replaceBinary installs the new host binary at destPath. On Windows a running
// executable cannot be opened for writing -- the image loader holds it open with
// a share mode that denies writes -- so overwriting the currently-running host
// fails with ERROR_SHARING_VIOLATION. Windows does permit renaming a running
// image, so in that case move the old binary aside and write the new one to the
// original path; the browser launches the updated binary the next time it spawns
// the host. Without this, re-running the helper to update never replaces the
// in-use binary and the extension keeps prompting to update (issue #84).
func replaceBinary(destPath string, src io.Reader, perm os.FileMode) error {
err := copyFile(destPath, src, perm)
if err == nil || !errors.Is(err, windows.ERROR_SHARING_VIOLATION) {
return err
}

// copyFile opens the destination before reading src, so the failed attempt
// above did not consume src; it is still positioned to be written here.
oldPath := destPath + ".old"
if err := os.Rename(destPath, oldPath); err != nil {
return fmt.Errorf("failed to move running binary aside: %w", err)
}
if err := copyFile(destPath, src, perm); err != nil {
// Roll back so the user is not left without a working binary.
_ = os.Rename(oldPath, destPath)
return fmt.Errorf("failed to write new binary after moving the old one aside: %w", err)
}
scheduleOldBinaryCleanup(oldPath)
return nil
}

// scheduleOldBinaryCleanup removes the moved-aside binary. It is usually still
// running (that is why it had to be moved), so deleting it now typically fails;
// in that case schedule it for removal on the next reboot. Best-effort: a
// lingering ".old" file never blocks a successful update.
func scheduleOldBinaryCleanup(oldPath string) {
if err := os.Remove(oldPath); err == nil {
return
}
if p, err := windows.UTF16PtrFromString(oldPath); err == nil {
_ = windows.MoveFileEx(p, nil, windows.MOVEFILE_DELAY_UNTIL_REBOOT)
}
}
92 changes: 92 additions & 0 deletions host/install_windows_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package main

import (
"os"
"path/filepath"
"strings"
"testing"

"golang.org/x/sys/windows"
)

// This file is compiled only on Windows (the _windows suffix is a GOOS build
// constraint), so the Linux/macOS CI run skips it. It is the only place the
// core issue #84 fix -- updating a host binary while it is running -- is
// verified end-to-end, because the running-executable lock cannot be
// reproduced on other platforms.

func TestReplaceBinaryOverwritesUnlocked(t *testing.T) {
dir := t.TempDir()
dest := filepath.Join(dir, "tailscale-browser-ext.exe")
if err := os.WriteFile(dest, []byte("OLDOLDOLD"), 0o755); err != nil {
t.Fatalf("seed dest: %v", err)
}
if err := replaceBinary(dest, strings.NewReader("NEW"), 0o755); err != nil {
t.Fatalf("replaceBinary returned error: %v", err)
}
got, err := os.ReadFile(dest)
if err != nil {
t.Fatalf("read dest: %v", err)
}
if string(got) != "NEW" {
t.Errorf("dest content = %q, want %q", got, "NEW")
}
// When nothing is locked, the fast path writes in place and leaves no
// ".old" sibling.
if _, err := os.Stat(dest + ".old"); !os.IsNotExist(err) {
t.Errorf("unlocked overwrite should not leave a .old file (stat err=%v)", err)
}
}

// TestReplaceBinaryRenamesAsideWhenLocked simulates a running executable: the
// destination is held open with a share mode that denies writes but permits
// rename/delete -- exactly how the Windows image loader holds a running .exe.
// A plain overwrite fails with ERROR_SHARING_VIOLATION; replaceBinary must
// still install the new binary by moving the locked one aside.
func TestReplaceBinaryRenamesAsideWhenLocked(t *testing.T) {
dir := t.TempDir()
dest := filepath.Join(dir, "tailscale-browser-ext.exe")
if err := os.WriteFile(dest, []byte("OLD"), 0o755); err != nil {
t.Fatalf("seed dest: %v", err)
}

closeLock := lockFileLikeRunningExe(t, dest)
defer closeLock()

if err := replaceBinary(dest, strings.NewReader("NEWBINARY"), 0o755); err != nil {
t.Fatalf("replaceBinary under lock returned error: %v", err)
}

got, err := os.ReadFile(dest)
if err != nil {
t.Fatalf("read dest: %v", err)
}
if string(got) != "NEWBINARY" {
t.Errorf("dest content = %q, want %q (rename-aside path did not install the new binary)", got, "NEWBINARY")
}
}

// lockFileLikeRunningExe opens path with GENERIC_READ and a share mode of
// READ|DELETE (no WRITE), mirroring the restriction the loader places on a
// running executable: others may read or rename/delete it, but may not open it
// for writing. Returns a closer for the handle.
func lockFileLikeRunningExe(t *testing.T, path string) func() {
t.Helper()
p, err := windows.UTF16PtrFromString(path)
if err != nil {
t.Fatalf("UTF16PtrFromString: %v", err)
}
h, err := windows.CreateFile(
p,
windows.GENERIC_READ,
windows.FILE_SHARE_READ|windows.FILE_SHARE_DELETE,
nil,
windows.OPEN_EXISTING,
windows.FILE_ATTRIBUTE_NORMAL,
0,
)
if err != nil {
t.Fatalf("CreateFile (simulate running-exe lock): %v", err)
}
return func() { _ = windows.CloseHandle(h) }
}
12 changes: 12 additions & 0 deletions packages/shared/src/popup/views/install-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,18 @@ export function renderInstallFlow(
step2.content.appendChild(step2.label);
step2.content.appendChild(body);
step2.content.appendChild(hint);

// When updating, the previous helper may still be running and connected, so
// the popup can keep showing "Update Available" until the browser respawns
// the host. Tell the user how to force that.
if (opts.mode === "update") {
const restartHint = document.createElement("div");
restartHint.className = "install-step-hint";
restartHint.textContent =
'If it still says "Update Available" after that, fully quit your browser (not just the window) and reopen it.';
step2.content.appendChild(restartHint);
}

steps.appendChild(step2.root);
} else {
// macOS / Linux: need terminal
Expand Down
Loading