diff --git a/.goreleaser.yaml b/.goreleaser.yaml index abc54679..e7345bc2 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -28,7 +28,8 @@ builds: flags: - -trimpath ldflags: - - -s -w -X github.com/localstack/lstk/internal/version.version={{ .Version }} + # bundlesExtensions flips to true with the release that ships bundled extensions. + - -s -w -X github.com/localstack/lstk/internal/version.version={{ .Version }} -X github.com/localstack/lstk/internal/version.bundlesExtensions=false archives: - id: lstk diff --git a/CLAUDE.md b/CLAUDE.md index e6e44c6d..61697571 100644 --- a/CLAUDE.md +++ b/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 diff --git a/cmd/extension.go b/cmd/extension.go index ebfdda4d..fa3332c4 100644 --- a/cmd/extension.go +++ b/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.DetectMissingBundleFor(name); 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 diff --git a/internal/update/extract.go b/internal/update/extract.go index f46d052d..c856acf8 100644 --- a/internal/update/extract.go +++ b/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 the set the archive carries as one unit: lstk, +// the bundled-extensions binary 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,125 @@ 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 members present at the extracted archive root, +// lstk last: a failure before that final rename leaves a working lstk to re-run +// with. Anything else there (completions, manpages) is not installed. +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 || !entry.Type().IsRegular() { + continue + } + mode := os.FileMode(0o755) + switch name { + case bundledBinaryName(goos): + case descriptionsFileName: + mode = 0o644 + 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 +} + +// 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 +236,10 @@ 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 a "binary" holding a path string. +// Extracted modes do not matter: copyFile applies each member's mode. + func extractTarGz(archivePath, destDir string) error { f, err := os.Open(archivePath) if err != nil { @@ -100,12 +262,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 @@ -146,6 +309,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 } @@ -169,6 +335,10 @@ func extractZip(archivePath, destDir string) error { 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 { @@ -176,12 +346,20 @@ func copyFile(src, dst string, mode os.FileMode) error { } 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) } diff --git a/internal/update/extract_test.go b/internal/update/extract_test.go new file mode 100644 index 00000000..220276e3 --- /dev/null +++ b/internal/update/extract_test.go @@ -0,0 +1,419 @@ +package update + +import ( + "archive/tar" + "archive/zip" + "compress/gzip" + "os" + "path/filepath" + goruntime "runtime" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Both release archive formats, since the two extractors are separate code. +var archiveFormats = []string{"tar.gz", "zip"} + +type archiveEntry struct { + name string + body string + mode os.FileMode + link string // when set, a symlink entry pointing at link (never shipped; for extractor tests) +} + +func lstkBinaryName() string { return exeName("lstk", goruntime.GOOS) } +func bundleName() string { return bundledBinaryName(goruntime.GOOS) } + +func buildArchive(t *testing.T, format string, entries []archiveEntry) string { + t.Helper() + path := filepath.Join(t.TempDir(), "archive."+format) + f, err := os.Create(path) + require.NoError(t, err) + defer func() { require.NoError(t, f.Close()) }() + + if format == "zip" { + zw := zip.NewWriter(f) + for _, e := range entries { + hdr := &zip.FileHeader{Name: e.name, Method: zip.Deflate} + body := e.body + if e.link != "" { + hdr.SetMode(e.mode | os.ModeSymlink) + body = e.link + } else { + hdr.SetMode(e.mode) + } + w, err := zw.CreateHeader(hdr) + require.NoError(t, err) + _, err = w.Write([]byte(body)) + require.NoError(t, err) + } + require.NoError(t, zw.Close()) + return path + } + gw := gzip.NewWriter(f) + tw := tar.NewWriter(gw) + for _, e := range entries { + if e.link != "" { + require.NoError(t, tw.WriteHeader(&tar.Header{Name: e.name, Mode: int64(e.mode), Linkname: e.link, Typeflag: tar.TypeSymlink})) + continue + } + require.NoError(t, tw.WriteHeader(&tar.Header{Name: e.name, Mode: int64(e.mode), Size: int64(len(e.body)), Typeflag: tar.TypeReg})) + _, err := tw.Write([]byte(e.body)) + require.NoError(t, err) + } + require.NoError(t, tw.Close()) + require.NoError(t, gw.Close()) + return path +} + +// newInstallDir writes files into a fresh directory and returns it with the +// path of the lstk binary. Everything but the descriptions file is 0755. +func newInstallDir(t *testing.T, files map[string]string) (dir, exePath string) { + t.Helper() + dir = t.TempDir() + for name, body := range files { + mode := os.FileMode(0o755) + if name == descriptionsFileName { + mode = 0o644 + } + require.NoError(t, os.WriteFile(filepath.Join(dir, name), []byte(body), mode)) + } + return dir, filepath.Join(dir, lstkBinaryName()) +} + +func requireFileContent(t *testing.T, path, want string) { + t.Helper() + got, err := os.ReadFile(path) + require.NoError(t, err, "expected %s to exist", path) + assert.Equal(t, want, string(got), "content of %s", path) +} + +func requireExecutable(t *testing.T, path string) { + t.Helper() + info, err := os.Stat(path) + require.NoError(t, err) + if goruntime.GOOS != "windows" { + assert.NotZero(t, info.Mode().Perm()&0o111, "%s should be executable", path) + } +} + +func requireAbsent(t *testing.T, path string) { + t.Helper() + _, err := os.Lstat(path) + assert.True(t, os.IsNotExist(err), "%s should not exist", path) +} + +func requireNoStagingLeftovers(t *testing.T, dir string) { + t.Helper() + entries, err := os.ReadDir(dir) + require.NoError(t, err) + for _, e := range entries { + if filepath.Ext(e.Name()) == stagingSuffix && e.Type().IsRegular() { + t.Errorf("staging leftover: %s", e.Name()) + } + } +} + +// TestExtractAndReplaceInstallsArchiveSet covers the archive shapes the +// updater has to handle, in both formats. The set is lstk, the +// bundled-extensions binary and the descriptions file; anything else at the +// archive root is ignored, and files beside lstk are never deleted. +func TestExtractAndReplaceInstallsArchiveSet(t *testing.T) { + t.Parallel() + toml := descriptionsFileName + cases := []struct { + name string + installed map[string]string + archive []archiveEntry + want map[string]string // content under the real names after the update + executable []string + absent []string // must not have been installed + }{ + { + name: "the whole set replaces all three members", + installed: map[string]string{lstkBinaryName(): "old lstk", bundleName(): "old bundle", toml: "doctor = \"old\"\n"}, + archive: []archiveEntry{ + {name: lstkBinaryName(), body: "new lstk", mode: 0o755}, + {name: bundleName(), body: "new bundle", mode: 0o755}, + {name: toml, body: "doctor = \"new\"\ndeploy = \"new\"\n", mode: 0o644}, + }, + want: map[string]string{lstkBinaryName(): "new lstk", bundleName(): "new bundle", toml: "doctor = \"new\"\ndeploy = \"new\"\n"}, + executable: []string{lstkBinaryName(), bundleName()}, + }, + { + name: "the bundle is added to a pre-bundling install", + installed: map[string]string{lstkBinaryName(): "old lstk"}, + archive: []archiveEntry{ + {name: lstkBinaryName(), body: "new lstk", mode: 0o755}, + {name: bundleName(), body: "bundle", mode: 0o755}, + {name: toml, body: "deploy = \"Deploy\"\n", mode: 0o644}, + }, + want: map[string]string{lstkBinaryName(): "new lstk", bundleName(): "bundle", toml: "deploy = \"Deploy\"\n"}, + executable: []string{bundleName()}, + }, + { + name: "bundle without a toml still installs", + installed: map[string]string{lstkBinaryName(): "old lstk"}, + archive: []archiveEntry{ + {name: lstkBinaryName(), body: "new lstk", mode: 0o755}, + {name: bundleName(), body: "bundle", mode: 0o755}, + }, + want: map[string]string{lstkBinaryName(): "new lstk", bundleName(): "bundle"}, + }, + { + // The pre-bundling and rollback shape: previously installed files stay. + name: "lstk-only archive replaces lstk and keeps the rest", + installed: map[string]string{lstkBinaryName(): "old lstk", bundleName(): "bundle", toml: "doctor = \"Doctor\"\n", "lstk-mine": "user extension"}, + archive: []archiveEntry{{name: lstkBinaryName(), body: "new lstk", mode: 0o755}}, + want: map[string]string{lstkBinaryName(): "new lstk", bundleName(): "bundle", toml: "doctor = \"Doctor\"\n", "lstk-mine": "user extension"}, + }, + { + // Release archives carry completions and manpages too; an executable + // lstk-* file is not part of the set either (decision 7b). + name: "other files at the archive root are not installed", + installed: map[string]string{lstkBinaryName(): "old lstk"}, + archive: []archiveEntry{ + {name: lstkBinaryName(), body: "new lstk", mode: 0o755}, + {name: exeName("lstk-alpha", goruntime.GOOS), body: "standalone extension", mode: 0o755}, + {name: "lstk-notes.txt", body: "notes", mode: 0o644}, + {name: "README.md", body: "readme", mode: 0o644}, + }, + want: map[string]string{lstkBinaryName(): "new lstk"}, + absent: []string{exeName("lstk-alpha", goruntime.GOOS), "lstk-notes.txt", "README.md"}, + }, + } + for _, tc := range cases { + for _, format := range archiveFormats { + t.Run(tc.name+"/"+format, func(t *testing.T) { + t.Parallel() + dir, exePath := newInstallDir(t, tc.installed) + require.NoError(t, extractAndReplace(buildArchive(t, format, tc.archive), exePath, format)) + for name, body := range tc.want { + requireFileContent(t, filepath.Join(dir, name), body) + } + for _, name := range tc.executable { + requireExecutable(t, filepath.Join(dir, name)) + } + for _, name := range tc.absent { + requireAbsent(t, filepath.Join(dir, name)) + } + requireNoStagingLeftovers(t, dir) + }) + } + } +} + +func TestExtractAndReplaceRejectsArchiveWithoutLstk(t *testing.T) { + t.Parallel() + dir, exePath := newInstallDir(t, map[string]string{lstkBinaryName(): "old lstk"}) + archive := buildArchive(t, "tar.gz", []archiveEntry{{name: bundleName(), body: "bundle", mode: 0o755}}) + err := extractAndReplace(archive, exePath, "tar.gz") + require.ErrorContains(t, err, "binary not found in archive") + requireFileContent(t, exePath, "old lstk") + requireNoStagingLeftovers(t, dir) +} + +// A failure while staging leaves the installation untouched and no staging +// files behind. Here the toml's staging path is blocked by a directory after +// the bundle was already staged (members stage in name order, lstk last). +func TestExtractAndReplaceStagingFailureLeavesInstallUntouched(t *testing.T) { + t.Parallel() + dir, exePath := newInstallDir(t, map[string]string{lstkBinaryName(): "old lstk", bundleName(): "old bundle", descriptionsFileName: "old toml"}) + require.NoError(t, os.MkdirAll(filepath.Join(dir, descriptionsFileName+stagingSuffix), 0o755)) + archive := buildArchive(t, "tar.gz", []archiveEntry{ + {name: lstkBinaryName(), body: "new lstk", mode: 0o755}, + {name: bundleName(), body: "new bundle", mode: 0o755}, + {name: descriptionsFileName, body: "new toml", mode: 0o644}, + }) + err := extractAndReplace(archive, exePath, "tar.gz") + require.ErrorContains(t, err, descriptionsFileName) + require.ErrorContains(t, err, "move it out of the way") + requireFileContent(t, exePath, "old lstk") + requireFileContent(t, filepath.Join(dir, bundleName()), "old bundle") + requireFileContent(t, filepath.Join(dir, descriptionsFileName), "old toml") + requireNoStagingLeftovers(t, dir) +} + +// Staging must never write through a symlink left at a staging path: that +// would destroy the target and install the link as the member. +func TestExtractAndReplaceRefusesSymlinkSquatter(t *testing.T) { + t.Parallel() + if goruntime.GOOS == "windows" { + t.Skip("os.Symlink needs elevation on Windows") + } + dir, exePath := newInstallDir(t, map[string]string{lstkBinaryName(): "old lstk"}) + target := filepath.Join(t.TempDir(), "precious") + require.NoError(t, os.WriteFile(target, []byte("precious data"), 0o644)) + require.NoError(t, os.Symlink(target, filepath.Join(dir, bundleName()+stagingSuffix))) + archive := buildArchive(t, "tar.gz", []archiveEntry{ + {name: lstkBinaryName(), body: "new lstk", mode: 0o755}, + {name: bundleName(), body: "bundle", mode: 0o755}, + }) + err := extractAndReplace(archive, exePath, "tar.gz") + require.ErrorContains(t, err, bundleName()) + requireFileContent(t, target, "precious data") + requireFileContent(t, exePath, "old lstk") + requireAbsent(t, filepath.Join(dir, bundleName())) +} + +// A regular file appearing at a staging path after cleanup means another +// update is running; it must not be truncated. +func TestStageMembersRefusesExistingStagingFile(t *testing.T) { + t.Parallel() + dir := t.TempDir() + dest := filepath.Join(dir, bundledBinaryBaseName) + src := filepath.Join(t.TempDir(), "src") + require.NoError(t, os.WriteFile(src, []byte("bundle"), 0o755)) + require.NoError(t, os.WriteFile(dest+stagingSuffix, []byte("another update's bytes"), 0o755)) + require.Error(t, stageMembers([]updateMember{{src: src, dest: dest, mode: 0o755}})) + requireFileContent(t, dest+stagingSuffix, "another update's bytes") +} + +func TestExtractAndReplaceCleansLeftoverStagingFiles(t *testing.T) { + t.Parallel() + dir, exePath := newInstallDir(t, map[string]string{lstkBinaryName(): "old lstk"}) + for _, name := range []string{lstkBinaryName(), bundleName(), "gone"} { + require.NoError(t, os.WriteFile(filepath.Join(dir, name+stagingSuffix), []byte("crashed"), 0o755)) + } + archive := buildArchive(t, "tar.gz", []archiveEntry{ + {name: lstkBinaryName(), body: "new lstk", mode: 0o755}, + {name: bundleName(), body: "bundle", mode: 0o755}, + }) + require.NoError(t, extractAndReplace(archive, exePath, "tar.gz")) + requireFileContent(t, exePath, "new lstk") + requireFileContent(t, filepath.Join(dir, bundleName()), "bundle") + requireAbsent(t, filepath.Join(dir, "gone")) + requireNoStagingLeftovers(t, dir) +} + +// Committing lstk last: a member that fails to commit leaves lstk on the +// previous version. +func TestExtractAndReplaceCommitFailureKeepsPreviousLstk(t *testing.T) { + t.Parallel() + dir, exePath := newInstallDir(t, map[string]string{lstkBinaryName(): "old lstk"}) + blocked := filepath.Join(dir, bundleName()) + require.NoError(t, os.MkdirAll(blocked, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(blocked, "occupied"), []byte("x"), 0o644)) + archive := buildArchive(t, "tar.gz", []archiveEntry{ + {name: lstkBinaryName(), body: "new lstk", mode: 0o755}, + {name: bundleName(), body: "bundle", mode: 0o755}, + }) + err := extractAndReplace(archive, exePath, "tar.gz") + require.ErrorContains(t, err, bundleName()) + requireFileContent(t, exePath, "old lstk") +} + +// The install path is data, not a glob pattern. +func TestExtractAndReplaceWorksInGlobMetacharacterDir(t *testing.T) { + t.Parallel() + for _, dirName := range []string{"we[ird", "we[ir]d", "sta*rs", "quest?ion"} { + t.Run(dirName, func(t *testing.T) { + t.Parallel() + dir := filepath.Join(t.TempDir(), dirName) + require.NoError(t, os.MkdirAll(dir, 0o755)) + exePath := filepath.Join(dir, lstkBinaryName()) + require.NoError(t, os.WriteFile(exePath, []byte("old lstk"), 0o755)) + leftover := filepath.Join(dir, bundleName()+stagingSuffix) + require.NoError(t, os.WriteFile(leftover, []byte("crashed"), 0o755)) + archive := buildArchive(t, "tar.gz", []archiveEntry{{name: lstkBinaryName(), body: "new lstk", mode: 0o755}}) + require.NoError(t, extractAndReplace(archive, exePath, "tar.gz")) + requireFileContent(t, exePath, "new lstk") + requireAbsent(t, leftover) + }) + } +} + +func TestExtractAndReplacePreservesSpecialModeBits(t *testing.T) { + t.Parallel() + if goruntime.GOOS == "windows" { + t.Skip("no Unix mode bits on Windows") + } + _, exePath := newInstallDir(t, map[string]string{lstkBinaryName(): "old lstk"}) + require.NoError(t, os.Chmod(exePath, 0o755|os.ModeSetgid)) + if info, err := os.Stat(exePath); err != nil || info.Mode()&os.ModeSetgid == 0 { + t.Skip("filesystem does not support setgid on files") + } + archive := buildArchive(t, "tar.gz", []archiveEntry{{name: lstkBinaryName(), body: "new lstk", mode: 0o755}}) + require.NoError(t, extractAndReplace(archive, exePath, "tar.gz")) + info, err := os.Stat(exePath) + require.NoError(t, err) + assert.NotZero(t, info.Mode()&os.ModeSetgid, "setgid must survive the update, got %v", info.Mode()) +} + +// The Windows shape, exercised from any host through the goos parameter: zip, +// ".exe" names, and every existing member moved to ".old" before the rename. +func TestReplaceSetWindows(t *testing.T) { + t.Parallel() + t.Run("whole set with .exe names", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + exePath := filepath.Join(dir, "lstk.exe") + require.NoError(t, os.WriteFile(exePath, []byte("old lstk"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "bundled-extensions.exe"), []byte("old bundle"), 0o755)) + archive := buildArchive(t, "zip", []archiveEntry{ + {name: "lstk.exe", body: "new lstk", mode: 0o755}, + {name: "bundled-extensions.exe", body: "new bundle", mode: 0o755}, + {name: "bundled-extensions", body: "no .exe: not the Windows member", mode: 0o755}, + {name: "lstk-alpha.exe", body: "not part of the set", mode: 0o755}, + {name: descriptionsFileName, body: "deploy = \"Deploy\"\n", mode: 0o644}, + }) + require.NoError(t, replaceSet(archive, exePath, "zip", "windows")) + requireFileContent(t, exePath, "new lstk") + requireFileContent(t, filepath.Join(dir, "lstk.exe.old"), "old lstk") + requireFileContent(t, filepath.Join(dir, "bundled-extensions.exe"), "new bundle") + requireFileContent(t, filepath.Join(dir, "bundled-extensions.exe.old"), "old bundle") + requireFileContent(t, filepath.Join(dir, descriptionsFileName), "deploy = \"Deploy\"\n") + requireAbsent(t, filepath.Join(dir, "bundled-extensions")) + requireAbsent(t, filepath.Join(dir, "lstk-alpha.exe")) + requireNoStagingLeftovers(t, dir) + }) + t.Run("lstk-only archive refreshes .old and keeps installed members", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + exePath := filepath.Join(dir, "lstk.exe") + require.NoError(t, os.WriteFile(exePath, []byte("old lstk"), 0o755)) + require.NoError(t, os.WriteFile(exePath+".old", []byte("older lstk"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "bundled-extensions.exe"), []byte("installed bundle"), 0o755)) + archive := buildArchive(t, "zip", []archiveEntry{{name: "lstk.exe", body: "new lstk", mode: 0o755}}) + require.NoError(t, replaceSet(archive, exePath, "zip", "windows")) + requireFileContent(t, exePath, "new lstk") + requireFileContent(t, exePath+".old", "old lstk") + requireFileContent(t, filepath.Join(dir, "bundled-extensions.exe"), "installed bundle") + }) +} + +// When the final rename fails after lstk.exe was moved aside, it is moved back. +func TestCommitRestoresRunningBinaryOnWindowsRenameFailure(t *testing.T) { + t.Parallel() + dest := filepath.Join(t.TempDir(), "lstk.exe") + require.NoError(t, os.WriteFile(dest, []byte("old lstk"), 0o755)) + m := updateMember{dest: dest, mode: 0o755} // no staging file: the rename fails + require.Error(t, m.commit("windows")) + requireFileContent(t, dest, "old lstk") + requireAbsent(t, dest+".old") +} + +// A zip symlink entry extracted as a file would be a "binary" holding a path +// string, which discoverMembers would then install as the bundle. +func TestExtractorsSkipSymlinkEntries(t *testing.T) { + t.Parallel() + for _, format := range archiveFormats { + t.Run(format, func(t *testing.T) { + t.Parallel() + archive := buildArchive(t, format, []archiveEntry{ + {name: lstkBinaryName(), body: "new lstk", mode: 0o755}, + {name: bundledBinaryBaseName, mode: 0o755, link: lstkBinaryName()}, + }) + dest := t.TempDir() + if format == "zip" { + require.NoError(t, extractZip(archive, dest)) + } else { + require.NoError(t, extractTarGz(archive, dest)) + } + requireAbsent(t, filepath.Join(dest, bundledBinaryBaseName)) + requireFileContent(t, filepath.Join(dest, lstkBinaryName()), "new lstk") + }) + } +} diff --git a/internal/update/github_test.go b/internal/update/github_test.go index 8af7321d..b9eb8ea1 100644 --- a/internal/update/github_test.go +++ b/internal/update/github_test.go @@ -1,10 +1,6 @@ package update import ( - "archive/tar" - "archive/zip" - "bytes" - "compress/gzip" "context" "crypto/sha256" "encoding/hex" @@ -20,44 +16,24 @@ import ( // makeReleaseArchive builds an archive in the format updateBinary expects for // the current GOOS (zip on windows, tar.gz elsewhere) containing a single -// binary entry at the archive root. +// binary entry at the archive root. It delegates to buildArchive +// (extract_test.go) so the package has exactly one archive builder and the +// checksum tests and the set-replacement tests cannot drift onto different +// archive shapes. func makeReleaseArchive(t *testing.T, binaryContent string) []byte { t.Helper() - var buf bytes.Buffer + format := "tar.gz" if goruntime.GOOS == "windows" { - zw := zip.NewWriter(&buf) - w, err := zw.Create("lstk.exe") - if err != nil { - t.Fatal(err) - } - if _, err := w.Write([]byte(binaryContent)); err != nil { - t.Fatal(err) - } - if err := zw.Close(); err != nil { - t.Fatal(err) - } - return buf.Bytes() - } - gw := gzip.NewWriter(&buf) - tw := tar.NewWriter(gw) - if err := tw.WriteHeader(&tar.Header{ - Name: "lstk", - Mode: 0o755, - Size: int64(len(binaryContent)), - Typeflag: tar.TypeReg, - }); err != nil { - t.Fatal(err) + format = "zip" } - if _, err := tw.Write([]byte(binaryContent)); err != nil { - t.Fatal(err) - } - if err := tw.Close(); err != nil { - t.Fatal(err) - } - if err := gw.Close(); err != nil { + path := buildArchive(t, format, []archiveEntry{ + {name: exeName("lstk", goruntime.GOOS), body: binaryContent, mode: 0o755}, + }) + data, err := os.ReadFile(path) + if err != nil { t.Fatal(err) } - return buf.Bytes() + return data } // fakeExecutable writes a stand-in for the running binary into its own temp diff --git a/internal/update/reinstall.go b/internal/update/reinstall.go new file mode 100644 index 00000000..4294cebd --- /dev/null +++ b/internal/update/reinstall.go @@ -0,0 +1,74 @@ +package update + +import ( + "fmt" + "os" + "path/filepath" + goruntime "runtime" + + "github.com/localstack/lstk/internal/version" +) + +// MissingBundle describes a binary install of a bundling release whose bundled +// extensions are not beside lstk: what the pre-bundling updater leaves behind +// when it installs a bundling release. Homebrew and npm replace the whole +// package, so they never end up here. +type MissingBundle struct { + Dir string // install directory that should hold the bundle + Reinstall string // what restores the complete set +} + +const reinstallInstruction = "download the latest release from https://github.com/localstack/lstk/releases/latest" + +// Summary is the one-sentence explanation shown wherever the state is reported. +func (m MissingBundle) Summary() string { + return fmt.Sprintf("This lstk release ships bundled extensions, but none are installed in %s.", m.Dir) +} + +// DetectMissingBundle reports whether the running lstk is a release build +// (version.BundlesExtensions), installed as a plain binary, with no bundle +// beside it. Dev builds never report one. +func DetectMissingBundle() (MissingBundle, bool) { + if !version.BundlesExtensions() { + return MissingBundle{}, false + } + info := DetectInstallMethod() + if info.ResolvedPath == "" { + return MissingBundle{}, false + } + return detectMissingBundle(info, goruntime.GOOS) +} + +// DetectMissingBundleFor is DetectMissingBundle limited to the commands the +// bundle provided at the time of the transition, so a typo never earns a +// reinstall hint. Retire it with the hint once every supported release ships +// the set-wise updater. +func DetectMissingBundleFor(command string) (MissingBundle, bool) { + switch command { + case "deploy", "doctor": + return DetectMissingBundle() + } + return MissingBundle{}, false +} + +func detectMissingBundle(info InstallInfo, goos string) (MissingBundle, bool) { + if info.Method != InstallBinary { + return MissingBundle{}, false + } + dir := filepath.Dir(info.ResolvedPath) + if !bundleMissing(dir, goos) { + return MissingBundle{}, false + } + return MissingBundle{Dir: dir, Reinstall: reinstallInstruction}, true +} + +// bundleMissing is true only when neither set member exists. A binary without +// its toml is a different, corrupt state, left to the extension resolver. +func bundleMissing(dir, goos string) bool { + for _, name := range []string{bundledBinaryName(goos), descriptionsFileName} { + if _, err := os.Lstat(filepath.Join(dir, name)); err == nil { + return false + } + } + return true +} diff --git a/internal/update/reinstall_test.go b/internal/update/reinstall_test.go new file mode 100644 index 00000000..dcc6a20a --- /dev/null +++ b/internal/update/reinstall_test.go @@ -0,0 +1,57 @@ +package update + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestBundleMissing(t *testing.T) { + cases := []struct { + name string + files []string + goos string + want bool + }{ + {name: "nothing beside lstk", goos: "linux", want: true}, + {name: "binary present", files: []string{"bundled-extensions"}, goos: "linux"}, + {name: "toml present", files: []string{"lstk-extensions.toml"}, goos: "linux"}, + {name: "both present", files: []string{"bundled-extensions", "lstk-extensions.toml"}, goos: "linux"}, + {name: "windows binary present", files: []string{"bundled-extensions.exe"}, goos: "windows"}, + {name: "unix binary name does not count on windows", files: []string{"bundled-extensions"}, goos: "windows", want: true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + for _, f := range tc.files { + require.NoError(t, os.WriteFile(filepath.Join(dir, f), []byte("x"), 0o644)) + } + assert.Equal(t, tc.want, bundleMissing(dir, tc.goos)) + }) + } +} + +// Only a plain binary install can be left without its bundle: Homebrew and npm +// replace the whole package. +func TestDetectMissingBundleByInstallMethod(t *testing.T) { + dir := t.TempDir() + exe := filepath.Join(dir, "lstk") + + mb, ok := detectMissingBundle(InstallInfo{Method: InstallBinary, ResolvedPath: exe}, "linux") + require.True(t, ok) + assert.Equal(t, dir, mb.Dir) + assert.Contains(t, mb.Reinstall, "https://github.com/localstack/lstk/releases/latest") + assert.Contains(t, mb.Summary(), dir) + + for _, m := range []InstallMethod{InstallHomebrew, InstallNPM} { + _, ok := detectMissingBundle(InstallInfo{Method: m, ResolvedPath: exe}, "linux") + assert.False(t, ok, m.String()) + } + + require.NoError(t, os.WriteFile(filepath.Join(dir, "lstk-extensions.toml"), nil, 0o644)) + _, ok = detectMissingBundle(InstallInfo{Method: InstallBinary, ResolvedPath: exe}, "linux") + assert.False(t, ok) +} diff --git a/internal/update/update.go b/internal/update/update.go index da399bdb..fc55138d 100644 --- a/internal/update/update.go +++ b/internal/update/update.go @@ -1,3 +1,27 @@ +// Package update implements lstk's self-update: checking GitHub for a newer +// release and applying it through whichever mechanism installed lstk (Homebrew, +// npm, or replacing the binary in place). +// +// On the binary channel an update installs the whole set a release archive +// carries (lstk, the bundled-extensions binary, lstk-extensions.toml) with a +// stage-then-commit scheme (extract.go). The guarantees to preserve: +// +// 1. A file under its real name is never truncated or half-written: content +// is only ever written to a fresh staging file and renamed into place. +// 2. An interrupted update is repaired by re-running `lstk update`: nothing +// commits until everything is staged, leftovers are cleaned first, and +// lstk commits last, so a working lstk always remains. Windows caveat: a +// crash between renaming lstk.exe aside and renaming the new one in leaves +// no lstk.exe; rename lstk.exe.old back by hand. +// 3. Nothing is deleted: only the members the archive carries are written, so +// an archive without the bundle (a rollback) replaces lstk alone. +// +// An archive carrying only lstk installs exactly as before bundling existed. +// +// The pre-bundling updater installs a bundling release with only its lstk +// binary. A release build that then finds neither bundle member beside itself +// (DetectMissingBundle) points the user at a reinstall, from the +// unknown-command error and from an up-to-date `lstk update`. package update import ( @@ -44,7 +68,11 @@ func Update(ctx context.Context, sink output.Sink, checkOnly bool, githubToken s if err != nil { return err } - if !available || checkOnly { + if !available { + warnIfBundleMissing(sink) + return nil + } + if checkOnly { return nil } @@ -58,6 +86,19 @@ func Update(ctx context.Context, sink output.Sink, checkOnly bool, githubToken s return nil } +// warnIfBundleMissing tells a current install that lacks its bundle that no +// update will bring it: the state the pre-bundling updater leaves behind. +func warnIfBundleMissing(sink output.Sink) { + missing, ok := DetectMissingBundle() + if !ok { + return + } + sink.Emit(output.MessageEvent{ + Severity: output.SeverityWarning, + Text: missing.Summary() + " Reinstall lstk: " + missing.Reinstall, + }) +} + // applyUpdate detects the current install method and performs the update, // returning its canonical name ("homebrew"/"npm"/"binary") on success. func applyUpdate(ctx context.Context, sink output.Sink, latest, githubToken string) (string, error) { diff --git a/internal/version/version.go b/internal/version/version.go index e2a1f478..bea989ec 100644 --- a/internal/version/version.go +++ b/internal/version/version.go @@ -4,4 +4,11 @@ package version // because the linker can only modify variables at link time. var version = "dev" +// bundlesExtensions is "true" on release builds, which ship the +// bundled-extensions binary and lstk-extensions.toml beside lstk. +var bundlesExtensions = "false" + func Version() string { return version } + +// BundlesExtensions reports whether this build expects a bundle beside it. +func BundlesExtensions() bool { return bundlesExtensions == "true" } diff --git a/openspec/changes/add-bundled-extension-distribution/specs/extension-bundling-distribution/spec.md b/openspec/changes/add-bundled-extension-distribution/specs/extension-bundling-distribution/spec.md index db9d5dcc..62f91af4 100644 --- a/openspec/changes/add-bundled-extension-distribution/specs/extension-bundling-distribution/spec.md +++ b/openspec/changes/add-bundled-extension-distribution/specs/extension-bundling-distribution/spec.md @@ -63,7 +63,15 @@ Bundled extensions are payload rather than a precondition in one sense only: **a - **WHEN** a user on a pre-bundling lstk runs `lstk update` and the latest release bundles extensions - **THEN** the update succeeds using the in-the-field updater (which replaces only the lstk binary and ignores the archive's extra members) - **AND** the install is left with an incomplete set, since that updater predates bundling and cannot be made to fail -- **AND** the incomplete set is repaired by the next `lstk update`, which SHALL NOT wait for a newer release to become available +- **AND** until the next release's `lstk update` (now the set-wise updater) installs the complete set, lstk points the user at a reinstall: an unknown command and an up-to-date `lstk update` both say the release ships bundled extensions that are not installed, and name the reinstall command for the install method (accepted one-release gap; a same-version repair was considered and dropped as too much machinery in PR #482) + +#### Scenario: Release build without its bundle hints at a reinstall + +- **WHEN** a release build (goreleaser stamps `version.bundlesExtensions=true`) installed as a plain binary finds neither `bundled-extensions` nor `lstk-extensions.toml` beside the running lstk +- **THEN** `lstk deploy` and `lstk doctor` (the bundled commands at the time of the transition) add to their unknown-command error that the release ships bundled extensions which are not installed in that directory, plus a "Reinstall lstk:" action pointing at the releases download page +- **AND** any other unknown command, a typo included, reports only the plain unknown-command error +- **AND** an `lstk update` that finds no newer version emits the same text as a warning (a `warnings` entry under `--json`) +- **AND** a dev build, a Homebrew or npm install, or an install with either member present, shows nothing extra #### Scenario: Pre-bundling lstk updates via Homebrew or npm @@ -81,13 +89,6 @@ Bundled extensions are payload rather than a precondition in one sense only: **a - **THEN** the update fails with an error naming the member that failed - **AND** the installation is left on its previous version rather than reporting success with an incomplete set -#### Scenario: Incomplete bundled set is repaired when lstk is already current - -- **WHEN** `lstk update` runs on an install whose lstk binary is already the latest version but whose bundled set is incomplete -- **THEN** the update SHALL NOT report "already up to date" -- **AND** it installs the missing members of the set -- **AND** on the binary channel, previously installed bundled extensions remain in place and still run - ### Requirement: Bundle provenance is resolved once, recorded, and verified Each lstk release SHALL ship exactly one extensions bundle, identified by a version file committed to the lstk repository. That file SHALL default to `latest` — the newest published bundle — and SHALL accept an explicit release tag to lock a build to one bundle. The release process SHALL resolve the value to a concrete tag once per build and use that resolved tag for every subsequent step, and SHALL record it in the published release notes so the mapping from an lstk version to an extensions bundle survives independently of build-log retention. It SHALL download the resolved bundle's prebuilt binaries and descriptions file from the private extensions repository's release assets, SHALL verify every downloaded asset against the bundle's checksum manifest before staging (hard fail on a missing or mismatching manifest), and SHALL fail when a bundled extension lacks a binary for any lstk target platform not explicitly allow-listed as unsupported. The credential used is read-only and scoped to the private extensions repository. diff --git a/openspec/changes/add-bundled-extension-distribution/tasks.md b/openspec/changes/add-bundled-extension-distribution/tasks.md index 272942a0..0bd8c9d3 100644 --- a/openspec/changes/add-bundled-extension-distribution/tasks.md +++ b/openspec/changes/add-bundled-extension-distribution/tasks.md @@ -4,13 +4,14 @@ Today `internal/update/extract.go` extracts the downloaded archive and replaces exactly one file: the `lstk` binary. Once archives also contain extension binaries (`lstk-deploy`, …) and the descriptions file (`lstk-extensions.toml`), the updater has to replace all of them — without ever leaving a half-written file if the update is interrupted. The approach: copy the new files into the install directory under temporary names first (`lstk-deploy.lstk-new`), and only when every copy has succeeded, rename each one over the real name. Renames within a directory are instant and atomic, so nobody can ever run a half-copied binary. -- [ ] 1.1 In `internal/update/extract.go`, build the list of files to replace by looking at the extracted archive root: the lstk binary (`lstk` / `lstk.exe`), every executable file named `lstk-*`, and `lstk-extensions.toml`. If the archive contains only `lstk` (all current releases, and any future rollback), the list has one entry and the updater must behave exactly as it does today. -- [ ] 1.2 Before doing anything else, delete any leftover `*.lstk-new` files in the install directory. These can only exist if a previous update crashed partway through; cleaning them up is what makes "just run `lstk update` again" always repair an interrupted update. -- [ ] 1.3 Copy phase: copy each file from the list into the install directory (the directory of the running executable) under the temporary name `.lstk-new`, and make binaries executable (0755). If any copy fails (disk full, permissions, …), delete the `.lstk-new` files and return an error — the existing installation must be completely untouched. -- [ ] 1.4 Rename phase: once all copies succeeded, rename each `.lstk-new` to ``. Rename the extensions and the toml first and the lstk binary **last**, so if the process dies mid-way the user still has a working lstk and a re-run finishes the job. Keep two existing behaviors as-is: on Windows, the running `lstk.exe` is first moved aside to `lstk.exe.old` (you cannot rename over a running exe there — this applies only to lstk itself, extensions aren't running during an update); and the cross-device copy fallback for installs where rename fails. If a rename fails, stop and return an error naming the file that failed — never report success with only part of the set installed. Renaming lstk last is what makes that safe: any failure before the final rename leaves the user on their previous, complete version. -- [ ] 1.5 Write the resulting guarantee into the package documentation so it survives future refactors: (a) a file visible under its real name is never truncated or half-written, (b) an interrupted update is fixed by re-running `lstk update`, (c) the updater never **deletes** an `lstk-*` file that isn't in the new archive — it can't tell a dropped bundled extension from a file the user put there themselves (design Decision 4 has the full reasoning). -- [ ] 1.6 Re-introduce `internal/update/extract_test.go` with tests that build small tar.gz/zip archives on the fly and cover each behavior above: an archive with lstk + two extensions + toml replaces all of them; an update that introduces a brand-new extension installs it; an archive with only `lstk` reproduces today's behavior; a failed copy leaves the installation untouched; leftover `.lstk-new` files from a fake earlier crash get cleaned up; an `lstk-*` file NOT present in the archive is left alone; a rename failure partway through returns an error and leaves lstk on its previous version; and the Windows zip/`.exe` variant works. -- [ ] 1.7 Repair an incomplete set even when the binary is already current. Anyone crossing the transition on the binary channel gets the new lstk with no extensions, because the updater that ran was their old one which ignores the archive's extra files. They cannot fix that by updating again: `applyUpdate` always jumps straight to the newest release, so they are already on it, and `Check` in `internal/update/update.go` reports "already up to date" until another release ships — leaving them without extensions for up to a week. Make `lstk update` compare the installed set against the set the release is expected to contain, and re-run the install when a member is missing, instead of short-circuiting on the version alone. What the expected set is depends on design Decision 7 — under (b) it is the command list in `lstk-extensions.toml`, under (a) it needs a shipped list, because a directory cannot testify to its own completeness. Cover it in `extract_test.go`/`update_test.go`: a current binary with a missing member installs the member; a current binary with a complete set still reports up to date. +- [x] 1.1 In `internal/update/extract.go`, build the list of files to replace by looking at the extracted archive root: the lstk binary (`lstk` / `lstk.exe`), every executable file named `lstk-*`, and `lstk-extensions.toml`. If the archive contains only `lstk` (all current releases, and any future rollback), the list has one entry and the updater must behave exactly as it does today. +- [x] 1.2 Before doing anything else, delete any leftover `*.lstk-new` files in the install directory. These can only exist if a previous update crashed partway through; cleaning them up is what makes "just run `lstk update` again" always repair an interrupted update. +- [x] 1.3 Copy phase: copy each file from the list into the install directory (the directory of the running executable) under the temporary name `.lstk-new`, and make binaries executable (0755). If any copy fails (disk full, permissions, …), delete the `.lstk-new` files and return an error — the existing installation must be completely untouched. +- [x] 1.4 Rename phase: once all copies succeeded, rename each `.lstk-new` to ``. Rename the extensions and the toml first and the lstk binary **last**, so if the process dies mid-way the user still has a working lstk and a re-run finishes the job. Keep two existing behaviors as-is: on Windows, the running `lstk.exe` is first moved aside to `lstk.exe.old` (you cannot rename over a running exe there — this applies only to lstk itself, extensions aren't running during an update); and the cross-device copy fallback for installs where rename fails. If a rename fails, stop and return an error naming the file that failed — never report success with only part of the set installed. Renaming lstk last is what makes that safe: any failure before the final rename leaves the user on their previous, complete version. +- [x] 1.5 Write the resulting guarantee into the package documentation so it survives future refactors: (a) a file visible under its real name is never truncated or half-written, (b) an interrupted update is fixed by re-running `lstk update`, (c) the updater never **deletes** an `lstk-*` file that isn't in the new archive — it can't tell a dropped bundled extension from a file the user put there themselves (design Decision 4 has the full reasoning). +- [x] 1.6 Re-introduce `internal/update/extract_test.go` with tests that build small tar.gz/zip archives on the fly and cover each behavior above: an archive with lstk + two extensions + toml replaces all of them; an update that introduces a brand-new extension installs it; an archive with only `lstk` reproduces today's behavior; a failed copy leaves the installation untouched; leftover `.lstk-new` files from a fake earlier crash get cleaned up; an `lstk-*` file NOT present in the archive is left alone; a rename failure partway through returns an error and leaves lstk on its previous version; and the Windows zip/`.exe` variant works. +- [x] 1.7 Reinstall hint instead of a repair (decided in review of PR #482: a same-version re-download was too much machinery for a one-release gap). goreleaser stamps `version.bundlesExtensions=true`; when a release build finds neither bundle member beside lstk, the unknown-command error and an up-to-date `lstk update` say so and point at the releases download page (`update.DetectMissingBundleFor`). Review of PR #482 narrowed it: only `deploy` and `doctor` get the hint (never a typo) and only plain binary installs (Homebrew and npm replace the whole package). +- [x] 1.8 Review of PR #482: `lstk-*` files are no longer set members. Under decision 7(b) an archive carries exactly `lstk`, `bundled-extensions` and `lstk-extensions.toml`; anything else at the archive root (completions, manpages, a stray `lstk-*` file) is not installed. The 1.1 and 1.6 wording above predates that call. ## 2. The release-time check that descriptions match binaries diff --git a/test/integration/reinstall_hint_test.go b/test/integration/reinstall_hint_test.go new file mode 100644 index 00000000..cfc628d5 --- /dev/null +++ b/test/integration/reinstall_hint_test.go @@ -0,0 +1,162 @@ +package integration_test + +import ( + "context" + "encoding/json" + "os" + "os/exec" + "path/filepath" + "runtime" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// buildBundlingLstk builds lstk the way a bundling release does, stamped with +// version.bundlesExtensions=true, so the binary expects a bundle beside it. +func buildBundlingLstk(t *testing.T, ctx context.Context, version, outPath string) { + t.Helper() + buildLstkWithLdflags(t, ctx, + versionLdflag(version)+" -X github.com/localstack/lstk/internal/version.bundlesExtensions=true", outPath) +} + +func platformExe(base string) string { + if runtime.GOOS == "windows" { + return base + ".exe" + } + return base +} + +// writeBundledSet puts a stand-in bundled-extensions binary and descriptions +// file beside lstk, the layout a complete bundling install has. +func writeBundledSet(t *testing.T, dir string) { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(dir, platformExe("bundled-extensions")), []byte("stand-in"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "lstk-extensions.toml"), []byte("doctor = \"Diagnose your setup\"\n"), 0o644)) +} + +// Path shapes the install-method detection recognises. +var ( + homebrewShape = filepath.Join("Caskroom", "lstk", "1.0.0") + npmShape = filepath.Join("node_modules", "@localstack", "lstk_test", "bin") +) + +func TestUnknownCommandHintsReinstallWhenBundleMissing(t *testing.T) { + t.Parallel() + ctx := testContext(t) + + cases := []struct { + name string + bundling bool + withBundle bool + shape string // subdirectory layout under the temp dir + command string + wantHint bool + }{ + {name: "bundling release without its bundle, deploy", bundling: true, command: "deploy", wantHint: true}, + {name: "bundling release without its bundle, doctor", bundling: true, command: "doctor", wantHint: true}, + {name: "bundling release without its bundle, a typo", bundling: true, command: "strt"}, + {name: "bundling release with its bundle", bundling: true, withBundle: true, command: "nosuchcmd"}, + {name: "pre-bundling release", command: "deploy"}, + {name: "Homebrew layout is never hinted", bundling: true, shape: homebrewShape, command: "deploy"}, + {name: "npm layout is never hinted", bundling: true, shape: npmShape, command: "deploy"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + installDir := filepath.Join(t.TempDir(), tc.shape) + require.NoError(t, os.MkdirAll(installDir, 0o755)) + lstk := filepath.Join(installDir, platformExe("lstk")) + if tc.bundling { + buildBundlingLstk(t, ctx, "0.0.2", lstk) + } else { + buildLstkWithVersion(t, ctx, "0.0.2", lstk) + } + if tc.withBundle { + writeBundledSet(t, installDir) + } + + cmd := exec.CommandContext(ctx, lstk, tc.command) + cmd.Env = testEnvWithHome(t.TempDir(), "") + out, err := cmd.CombinedOutput() + require.Error(t, err, "an unknown command must still exit non-zero: %s", out) + assert.Contains(t, string(out), `unknown command "`+tc.command+`"`) + + if !tc.wantHint { + assert.NotContains(t, string(out), "Reinstall lstk") + return + } + resolvedDir, err := filepath.EvalSymlinks(installDir) + require.NoError(t, err) + assert.Contains(t, string(out), "bundled extensions") + assert.Contains(t, string(out), resolvedDir) + assert.Contains(t, string(out), "Reinstall lstk") + assert.Contains(t, string(out), "https://github.com/localstack/lstk/releases") + assert.NotContains(t, string(out), "brew") + assert.NotContains(t, string(out), "npm") + }) + } +} + +func TestUpdateUpToDateWarnsWhenBundleMissing(t *testing.T) { + t.Parallel() + ctx := testContext(t) + srv := mockGitHubReleaseServer(t, "v0.0.2", nil) + + t.Run("bundle missing: plain and JSON output carry the warning", func(t *testing.T) { + t.Parallel() + installDir := t.TempDir() + lstk := filepath.Join(installDir, platformExe("lstk")) + buildBundlingLstk(t, ctx, "0.0.2", lstk) + + cmd := exec.CommandContext(ctx, lstk, "update", "--non-interactive") + cmd.Env = mockGitHubEnv(t, srv) + out, err := cmd.CombinedOutput() + require.NoError(t, err, "lstk update failed: %s", out) + assert.Contains(t, string(out), "Already up to date") + assert.Contains(t, string(out), "bundled extensions") + assert.Contains(t, string(out), "Reinstall lstk") + + cmd = exec.CommandContext(ctx, lstk, "update", "--check", "--json") + cmd.Env = mockGitHubEnv(t, srv) + out, err = cmd.CombinedOutput() + require.NoError(t, err, "lstk update --check --json failed: %s", out) + var envelope struct { + Warnings []struct { + Code string `json:"code"` + Message string `json:"message"` + } `json:"warnings"` + } + require.NoError(t, json.Unmarshal(out, &envelope), "not a JSON envelope: %s", out) + require.Len(t, envelope.Warnings, 1) + assert.Contains(t, envelope.Warnings[0].Message, "Reinstall lstk") + }) + + for _, tc := range []struct { + name string + withBundle bool + shape string + }{ + {name: "bundle present: no warning", withBundle: true}, + {name: "Homebrew layout: no warning", shape: homebrewShape}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + installDir := filepath.Join(t.TempDir(), tc.shape) + require.NoError(t, os.MkdirAll(installDir, 0o755)) + lstk := filepath.Join(installDir, platformExe("lstk")) + buildBundlingLstk(t, ctx, "0.0.2", lstk) + if tc.withBundle { + writeBundledSet(t, installDir) + } + + cmd := exec.CommandContext(ctx, lstk, "update", "--non-interactive") + cmd.Env = mockGitHubEnv(t, srv) + out, err := cmd.CombinedOutput() + require.NoError(t, err, "lstk update failed: %s", out) + assert.Contains(t, string(out), "Already up to date") + assert.NotContains(t, string(out), "Reinstall lstk") + }) + } +} diff --git a/test/integration/update_test.go b/test/integration/update_test.go index 683eb5ba..e3851ae1 100644 --- a/test/integration/update_test.go +++ b/test/integration/update_test.go @@ -439,11 +439,20 @@ func npmPlatformPackage() string { // buildLstkWithVersion builds the lstk binary from the repo root with the // given version stamped in, writing it to outPath. func buildLstkWithVersion(t *testing.T, ctx context.Context, version, outPath string) { + t.Helper() + buildLstkWithLdflags(t, ctx, versionLdflag(version), outPath) +} + +func versionLdflag(version string) string { + return "-X github.com/localstack/lstk/internal/version.version=" + version +} + +func buildLstkWithLdflags(t *testing.T, ctx context.Context, ldflags, outPath string) { t.Helper() repoRoot, err := filepath.Abs("../..") require.NoError(t, err) buildCmd := exec.CommandContext(ctx, "go", "build", - "-ldflags", "-X github.com/localstack/lstk/internal/version.version="+version, + "-ldflags", ldflags, "-o", outPath, ".", ) @@ -462,32 +471,50 @@ func releaseAssetName(ver string) string { return fmt.Sprintf("lstk_%s_%s_%s.%s", ver, runtime.GOOS, runtime.GOARCH, ext) } +// releaseMember is one file at the root of a release archive. +type releaseMember struct { + name string + body []byte + mode os.FileMode +} + // packageReleaseArchive wraps binary bytes into the release archive format the // updater extracts: a tar.gz (zip on Windows) with a single executable entry. func packageReleaseArchive(t *testing.T, binaryName string, binary []byte) []byte { + t.Helper() + return packageReleaseArchiveWith(t, []releaseMember{{name: binaryName, body: binary, mode: 0o755}}) +} + +// packageReleaseArchiveWith builds a release archive carrying the given members +// at its root, so a test can express a case as "an archive containing X". +func packageReleaseArchiveWith(t *testing.T, members []releaseMember) []byte { t.Helper() var buf bytes.Buffer if runtime.GOOS == "windows" { zw := zip.NewWriter(&buf) - hdr := &zip.FileHeader{Name: binaryName, Method: zip.Deflate} - hdr.SetMode(0o755) - w, err := zw.CreateHeader(hdr) - require.NoError(t, err) - _, err = w.Write(binary) - require.NoError(t, err) + for _, m := range members { + hdr := &zip.FileHeader{Name: m.name, Method: zip.Deflate} + hdr.SetMode(m.mode) + w, err := zw.CreateHeader(hdr) + require.NoError(t, err) + _, err = w.Write(m.body) + require.NoError(t, err) + } require.NoError(t, zw.Close()) return buf.Bytes() } gw := gzip.NewWriter(&buf) tw := tar.NewWriter(gw) - require.NoError(t, tw.WriteHeader(&tar.Header{ - Name: binaryName, - Mode: 0o755, - Size: int64(len(binary)), - Typeflag: tar.TypeReg, - })) - _, err := tw.Write(binary) - require.NoError(t, err) + for _, m := range members { + require.NoError(t, tw.WriteHeader(&tar.Header{ + Name: m.name, + Mode: int64(m.mode), + Size: int64(len(m.body)), + Typeflag: tar.TypeReg, + })) + _, err := tw.Write(m.body) + require.NoError(t, err) + } require.NoError(t, tw.Close()) require.NoError(t, gw.Close()) return buf.Bytes() @@ -670,3 +697,56 @@ func TestUpdateBinaryMockGitHubMissingChecksums(t *testing.T) { require.NoError(t, err) assert.Empty(t, leftovers, "aborted update must not leave temp files behind") } + +// TestUpdateBinaryInstallsBundledSet: a version bump whose archive carries the +// bundle lands lstk, the bundled binary and the descriptions file together, +// executable, with no staging leftovers. +func TestUpdateBinaryInstallsBundledSet(t *testing.T) { + t.Parallel() + ctx := testContext(t) + + binaryName, bundled := "lstk", "bundled-extensions" + if runtime.GOOS == "windows" { + binaryName, bundled = "lstk.exe", "bundled-extensions.exe" + } + installDir := t.TempDir() + oldBinary := filepath.Join(installDir, binaryName) + buildLstkWithVersion(t, ctx, "0.0.1", oldBinary) + newBinary := filepath.Join(t.TempDir(), binaryName) + buildLstkWithVersion(t, ctx, "0.0.2", newBinary) + newBytes, err := os.ReadFile(newBinary) + require.NoError(t, err) + + archive := packageReleaseArchiveWith(t, []releaseMember{ + {name: binaryName, body: newBytes, mode: 0o755}, + {name: bundled, body: []byte("multi-call extensions binary"), mode: 0o755}, + {name: "lstk-extensions.toml", body: []byte("doctor = \"Diagnose your setup\"\n"), mode: 0o644}, + }) + sum := sha256.Sum256(archive) + assetName := releaseAssetName("0.0.2") + srv := mockGitHubReleaseServer(t, "v0.0.2", map[string][]byte{ + "checksums.txt": []byte(fmt.Sprintf("%s %s\n", hex.EncodeToString(sum[:]), assetName)), + assetName: archive, + }) + + updateCmd := exec.CommandContext(ctx, oldBinary, "update", "--non-interactive") + updateCmd.Env = mockGitHubEnv(t, srv) + out, err := updateCmd.CombinedOutput() + require.NoError(t, err, "lstk update failed: %s", string(out)) + assert.Contains(t, string(out), "Updated to") + + verOut, err := exec.CommandContext(ctx, oldBinary, "--version").CombinedOutput() + require.NoError(t, err) + assert.Contains(t, string(verOut), "0.0.2") + info, err := os.Stat(filepath.Join(installDir, bundled)) + require.NoError(t, err, "the bundled binary should have been installed") + if runtime.GOOS != "windows" { + assert.NotZero(t, info.Mode().Perm()&0o111, "the bundled binary should be executable") + } + toml, err := os.ReadFile(filepath.Join(installDir, "lstk-extensions.toml")) + require.NoError(t, err) + assert.Equal(t, "doctor = \"Diagnose your setup\"\n", string(toml)) + leftovers, err := filepath.Glob(filepath.Join(installDir, "*.lstk-new")) + require.NoError(t, err) + assert.Empty(t, leftovers) +}