Dpx 693 pr 2 teach lstk update to handle the bundled extensions

.goreleaser.yaml

@@ -28,7 +28,7 @@ builds:
     flags:
       - -trimpath
     ldflags:
-      - -s -w -X github.com/localstack/lstk/internal/version.version={{ .Version }}
+      - -s -w -X github.com/localstack/lstk/internal/version.version={{ .Version }} -X github.com/localstack/lstk/internal/version.bundlesExtensions=true
 
 archives:
   - id: lstk

CLAUDE.md

@@ -70,7 +70,7 @@ Notes:
   - `terminal/` - Plain-mode terminal helpers (spinner, TTY detection)
   - `tracing/` - OpenTelemetry setup (`LSTK_OTEL=1`)
   - `ui/` - Bubble Tea views for interactive output
-  - `update/` - Self-update logic: version check via GitHub API, binary/Homebrew/npm update paths, archive extraction; the binary path verifies the downloaded archive's SHA-256 against the release's `checksums.txt` before replacing the executable (hard fail on missing/malformed manifest or mismatch)
+  - `update/` - Self-update logic: version check via GitHub API, binary/Homebrew/npm update paths, archive extraction; the binary path verifies the downloaded archive's SHA-256 against the release's `checksums.txt` before replacing the executable (hard fail on missing/malformed manifest or mismatch); a release build (`version.BundlesExtensions`, stamped by goreleaser) that finds no bundle beside lstk points the user at a reinstall, from the unknown-command error in `cmd/extension.go` and from an up-to-date `lstk update` (`update.DetectMissingBundle`)
   - `validate/` - Reusable input validators for user-supplied CLI values (pod names, env var names, auth tokens) rejecting malformed/hostile input (control chars, path traversal, percent-encoding, shell metacharacters)
   - `version/` - Version info
   - `volume/` - `lstk volume` domain logic

cmd/extension.go

@@ -17,6 +17,7 @@ import (
 	"github.com/localstack/lstk/internal/output"
 	"github.com/localstack/lstk/internal/runtime"
 	"github.com/localstack/lstk/internal/telemetry"
+	"github.com/localstack/lstk/internal/update"
 	"github.com/spf13/cobra"
 )
 
@@ -48,10 +49,15 @@ func dispatchExtension(ctx context.Context, cfg *env.Env, tel *telemetry.Client,
 	if err != nil {
 		if errors.Is(err, extension.ErrNotFound) {
 			// Errors go to stderr, like Cobra's own unknown-command output.
-			output.NewPlainSink(os.Stderr).Emit(output.ErrorEvent{
+			ev := output.ErrorEvent{
 				Title:   fmt.Sprintf("unknown command %q for lstk", name),
 				Actions: []output.ErrorAction{{Label: "See help:", Value: "lstk -h"}},
-			})
+			}
+			if missing, ok := update.DetectMissingBundle(); ok {
+				ev.Summary = missing.Summary()
+				ev.Actions = append(ev.Actions, output.ErrorAction{Label: "Reinstall lstk:", Value: missing.Reinstall})
+			}
+			output.NewPlainSink(os.Stderr).Emit(ev)
 			return output.NewSilentError(fmt.Errorf("unknown command %q for lstk", name))
 		}
 		return err

internal/update/extract.go

@@ -10,9 +10,82 @@ import (
 	"path/filepath"
 	goruntime "runtime"
 	"strings"
+
+	"github.com/localstack/lstk/internal/extension"
 )
 
+// stagingSuffix marks a member copied into the install directory but not yet
+// renamed over its final name. Staging in the destination directory makes every
+// commit an intra-directory rename: atomic, and never cross-device.
+const stagingSuffix = ".lstk-new"
+
+const descriptionsFileName = extension.DescriptionsFileName
+
+// bundledBinaryBaseName is the multi-call binary providing every bundled
+// extension; it is the one set member that does not match "lstk-*".
+// TODO(dpx-692): alias from extension.BundledBinaryName once that branch lands.
+const bundledBinaryBaseName = "bundled-extensions"
+
+func exeName(base, goos string) string {
+	if goos == "windows" {
+		return base + ".exe"
+	}
+	return base
+}
+
+func bundledBinaryName(goos string) string { return exeName(bundledBinaryBaseName, goos) }
+
+// updateMember is one file of the set an update installs.
+type updateMember struct {
+	src  string      // path inside the extracted archive
+	dest string      // final path in the install directory
+	mode os.FileMode // mode to install with
+}
+
+func (m updateMember) staging() string { return m.dest + stagingSuffix }
+
+// commit renames the staged copy over the final name. On Windows a running
+// executable can be renamed but not replaced, so an existing member is moved to
+// ".old" first (lstk.exe itself, or a bundled extension the user is running);
+// the ".old" is removed by the next update's commit.
+func (m updateMember) commit(goos string) error {
+	movedAside := ""
+	if goos == "windows" {
+		oldPath := m.dest + ".old"
+		if err := os.Remove(oldPath); err != nil && !os.IsNotExist(err) {
+			return fmt.Errorf("cannot remove old binary %s: %w", oldPath, err)
+		}
+		switch err := os.Rename(m.dest, oldPath); {
+		case err == nil:
+			movedAside = oldPath
+		case os.IsNotExist(err):
+		default:
+			return fmt.Errorf("cannot move %s aside: %w", filepath.Base(m.dest), err)
+		}
+	}
+	if err := os.Rename(m.staging(), m.dest); err != nil {
+		if movedAside != "" {
+			if rerr := os.Rename(movedAside, m.dest); rerr != nil {
+				return fmt.Errorf("%w (restoring the previous file also failed: %v; rename %s back to %s by hand)",
+					err, rerr, movedAside, m.dest)
+			}
+		}
+		return err
+	}
+	return nil
+}
+
+// extractAndReplace installs every member the archive carries as one unit:
+// lstk, the bundled-extensions binary, any lstk-* binaries, and the
+// descriptions file. An archive carrying only lstk is a set of size one. A
+// member that fails to stage or commit fails the whole update, naming it.
 func extractAndReplace(archivePath, exePath, format string) error {
+	return replaceSet(archivePath, exePath, format, goruntime.GOOS)
+}
+
+// replaceSet takes the platform as a parameter so the Windows naming and
+// move-aside rules are testable on any host (unit tests run on Linux in CI).
+func replaceSet(archivePath, exePath, format, goos string) error {
 	dir, err := os.MkdirTemp("", "lstk-extract-*")
 	if err != nil {
 		return err
@@ -30,40 +103,142 @@ func extractAndReplace(archivePath, exePath, format string) error {
 		}
 	}
 
-	binaryName := "lstk"
-	if goruntime.GOOS == "windows" {
-		binaryName = "lstk.exe"
+	members, err := discoverMembers(dir, exePath, goos)
+	if err != nil {
+		return err
 	}
+	if err := removeStagingFiles(filepath.Dir(exePath)); err != nil {
+		return err
+	}
+	if err := stageMembers(members); err != nil {
+		return err
+	}
+	return commitMembers(members, goos)
+}
 
-	newBinary := filepath.Join(dir, binaryName)
+// discoverMembers lists the set at the extracted archive root, lstk last: a
+// failure before that final rename leaves a working lstk to re-run with.
+func discoverMembers(extractDir, exePath, goos string) ([]updateMember, error) {
+	binaryName := exeName("lstk", goos)
+	newBinary := filepath.Join(extractDir, binaryName)
 	if _, err := os.Stat(newBinary); err != nil {
-		return fmt.Errorf("binary not found in archive: %w", err)
+		return nil, fmt.Errorf("binary not found in archive: %w", err)
+	}
+	exeInfo, err := os.Stat(exePath)
+	if err != nil {
+		return nil, err
+	}
+	entries, err := os.ReadDir(extractDir)
+	if err != nil {
+		return nil, err
 	}
 
-	info, err := os.Stat(exePath)
+	destDir := filepath.Dir(exePath)
+	var members []updateMember
+	for _, entry := range entries {
+		name := entry.Name()
+		if name == binaryName {
+			continue
+		}
+		info, err := entry.Info()
+		if err != nil {
+			return nil, err
+		}
+		mode := os.FileMode(0o755)
+		switch {
+		case name == bundledBinaryName(goos):
+		case name == descriptionsFileName:
+			mode = 0o644
+		case isExtensionEntry(name, info, goos):
+		default:
+			continue
+		}
+		members = append(members, updateMember{
+			src:  filepath.Join(extractDir, name),
+			dest: filepath.Join(destDir, name),
+			mode: mode,
+		})
+	}
+	// Destination and mode come from the running binary: the user may have
+	// installed it under another name or with special bits (setgid), and the
+	// pre-bundling updater preserved both.
+	return append(members, updateMember{src: newBinary, dest: exePath, mode: exeInfo.Mode()}), nil
+}
+
+// isExtensionEntry accepts executable "lstk-*" regular files. On Windows only
+// ".exe" counts: an installer must not let the user's PATHEXT decide what an
+// archive installs, unlike the resolver (extension.scanDir), which honours it.
+func isExtensionEntry(name string, info os.FileInfo, goos string) bool {
+	if !strings.HasPrefix(name, extension.NamePrefix) || !info.Mode().IsRegular() {
+		return false
+	}
+	if goos == "windows" {
+		return strings.EqualFold(filepath.Ext(name), ".exe")
+	}
+	return info.Mode().Perm()&0o111 != 0
+}
+
+// removeStagingFiles deletes regular ".lstk-new" files left by an interrupted
+// update. Matching is by literal suffix, not a glob: the path is user data and
+// may contain glob metacharacters. Non-regular files are left for stageMembers
+// to refuse.
+func removeStagingFiles(dir string) error {
+	entries, err := os.ReadDir(dir)
 	if err != nil {
 		return err
 	}
-
-	// On Windows, a running executable cannot be overwritten but can be renamed.
-	// Move it out of the way first so we can place the new binary at the original path.
-	if goruntime.GOOS == "windows" {
-		oldPath := exePath + ".old"
-		// Clean up leftover from a previous update; ignore error if it doesn't exist.
-		if err := os.Remove(oldPath); err != nil && !os.IsNotExist(err) {
-			return fmt.Errorf("cannot remove old binary %s: %w", oldPath, err)
+	for _, entry := range entries {
+		if !strings.HasSuffix(entry.Name(), stagingSuffix) || !entry.Type().IsRegular() {
+			continue
 		}
-		if err := os.Rename(exePath, oldPath); err != nil {
-			return fmt.Errorf("cannot move running binary: %w", err)
+		path := filepath.Join(dir, entry.Name())
+		if err := os.Remove(path); err != nil {
+			return fmt.Errorf("cannot remove leftover staging file %s: %w", path, err)
 		}
 	}
+	return nil
+}
 
-	if err := os.Rename(newBinary, exePath); err != nil {
-		// Cross-device rename: fall back to copy
-		return copyFile(newBinary, exePath, info.Mode())
+// stageMembers copies every member to its staging name; on failure it removes
+// what it staged and leaves the installation untouched. Anything already at a
+// staging path is refused: a regular file means another update is running, and
+// writing through a symlink or directory would damage the user's files.
+func stageMembers(members []updateMember) error {
+	staged := make([]string, 0, len(members))
+	unstage := func() {
+		for _, path := range staged {
+			_ = os.Remove(path)
+		}
+	}
+	for _, m := range members {
+		path := m.staging()
+		if info, err := os.Lstat(path); err == nil {
+			unstage()
+			if info.Mode().IsRegular() {
+				return fmt.Errorf("cannot stage %s: %s already exists; is another lstk update running?", filepath.Base(m.dest), path)
+			}
+			return fmt.Errorf("cannot stage %s: %s exists and is not a regular file; move it out of the way and re-run lstk update", filepath.Base(m.dest), path)
+		}
+		if err := copyFile(m.src, path, m.mode); err != nil {
+			_ = os.Remove(path)
+			unstage()
+			return fmt.Errorf("cannot stage %s in %s: %w (the update needs write permission in this directory)",
+				filepath.Base(m.dest), filepath.Dir(m.dest), err)
+		}
+		staged = append(staged, path)
 	}
+	return nil
+}
 
-	return os.Chmod(exePath, info.Mode())
+// commitMembers renames each staged file into place, stopping at the first
+// failure. Uncommitted staging files are left for the next run to clean up.
+func commitMembers(members []updateMember, goos string) error {
+	for _, m := range members {
+		if err := m.commit(goos); err != nil {
+			return fmt.Errorf("cannot install %s: %w", filepath.Base(m.dest), err)
+		}
+	}
+	return nil
 }
 
 func safePath(destDir, name string) (string, error) {
@@ -78,6 +253,11 @@ func safePath(destDir, name string) (string, error) {
 	return target, nil
 }
 
+// The extractors skip symlink entries: release archives ship none, and a zip
+// symlink extracted as a file would be an "executable" holding a path string.
+// Modes are restored with Chmod because OpenFile's mode is umask-masked and
+// discoverMembers keys extension discovery off the exec bits.
+
 func extractTarGz(archivePath, destDir string) error {
 	f, err := os.Open(archivePath)
 	if err != nil {
@@ -100,12 +280,13 @@ func extractTarGz(archivePath, destDir string) error {
 		if err != nil {
 			return err
 		}
-
 		target, err := safePath(destDir, hdr.Name)
 		if err != nil {
 			return err
 		}
 		switch hdr.Typeflag {
+		case tar.TypeSymlink, tar.TypeLink:
+			continue
 		case tar.TypeDir:
 			if err := os.MkdirAll(target, 0o755); err != nil {
 				return err
@@ -123,6 +304,9 @@ func extractTarGz(archivePath, destDir string) error {
 				return err
 			}
 			_ = out.Close()
+			if err := os.Chmod(target, hdr.FileInfo().Mode().Perm()); err != nil {
+				return err
+			}
 		}
 	}
 	return nil
@@ -146,6 +330,9 @@ func extractZip(archivePath, destDir string) error {
 			}
 			continue
 		}
+		if f.Mode()&os.ModeSymlink != 0 {
+			continue
+		}
 		if err := os.MkdirAll(filepath.Dir(target), 0o755); err != nil {
 			return err
 		}
@@ -165,23 +352,38 @@ func extractZip(archivePath, destDir string) error {
 		}
 		_ = out.Close()
 		_ = rc.Close()
+		if err := os.Chmod(target, f.Mode().Perm()); err != nil {
+			return err
+		}
 	}
 	return nil
 }
 
+// copyFile copies src to a new file at dst (O_EXCL: never overwrites, never
+// follows a symlink), syncs, and applies mode with Chmod so umask and the
+// special bits are handled. A close error is reported: a full disk shows up
+// there.
 func copyFile(src, dst string, mode os.FileMode) error {
 	in, err := os.Open(src)
 	if err != nil {
 		return err
 	}
 	defer func() { _ = in.Close() }()
 
-	out, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, mode)
+	out, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_EXCL, mode.Perm())
 	if err != nil {
 		return err
 	}
-	defer func() { _ = out.Close() }()
-
-	_, err = io.Copy(out, in)
-	return err
+	if _, err := io.Copy(out, in); err != nil {
+		_ = out.Close()
+		return err
+	}
+	if err := out.Sync(); err != nil {
+		_ = out.Close()
+		return err
+	}
+	if err := out.Close(); err != nil {
+		return err
+	}
+	return os.Chmod(dst, mode)
 }