diff --git a/pkg/api/ws_dispatcher_test.go b/pkg/api/ws_dispatcher_test.go index 60d16ad8c..03be52e97 100644 --- a/pkg/api/ws_dispatcher_test.go +++ b/pkg/api/ws_dispatcher_test.go @@ -585,8 +585,14 @@ func TestWebSocketRunJobStartsMethodTimeoutAtExecution(t *testing.T) { tt.method, )), } - time.Sleep(25 * time.Millisecond) - require.Error(t, enqueuedCtx.Err(), "pre-existing enqueue-time context should be expired") + // Waited for rather than slept past: a deadline is recorded by a timer + // goroutine, so under load the wall clock passes the deadline well + // before anything runs to set the error. + select { + case <-enqueuedCtx.Done(): + case <-time.After(5 * time.Second): + t.Fatal("pre-existing enqueue-time context should be expired") + } beforeRun := time.Now() d.runJob(job) diff --git a/pkg/cli/cli.go b/pkg/cli/cli.go index 1fbff6e78..826b0445d 100644 --- a/pkg/cli/cli.go +++ b/pkg/cli/cli.go @@ -98,7 +98,7 @@ func SetupFlags() *Flags { "send method and params to API and print response", ), Version: flag.Bool( - "version", + config.VersionFlagName, false, "print version and exit", ), @@ -166,7 +166,9 @@ func (f *Flags) Pre(pl platforms.Platform) { flag.Parse() if *f.Version { - _, _ = fmt.Printf("Zaparoo v%s (%s)\n", config.AppVersion, pl.ID()) + // config.VersionLine, not a literal: the self-update probe compares a + // staged binary's output against it, so the two must not drift. + _, _ = fmt.Printf("%s\n", config.VersionLine(config.AppVersion, pl.ID())) os.Exit(0) } } diff --git a/pkg/config/app.go b/pkg/config/app.go index 13a5e53d8..3b164be3d 100644 --- a/pkg/config/app.go +++ b/pkg/config/app.go @@ -20,6 +20,7 @@ package config import ( + "fmt" "strings" "time" ) @@ -56,4 +57,25 @@ const ( CacheDir = "cache" LogUploadURL = "https://logs.zaparoo.org/" MinFreeDiskBytes = 500 * 1024 * 1024 // 500 MB + + // VersionFlagName is the flag that prints VersionLine and exits. The + // self-update probe passes it to a binary it has just downloaded, so the + // name is part of the same frozen contract as the line itself. + VersionFlagName = "version" ) + +// VersionLine is the line the version flag prints, and the line the self-update +// probe looks for in a staged binary's output. +// +// It is a compatibility surface between releases, not a cosmetic string. The +// probe runs in the binary that is already installed and checks what the +// incoming one prints, so it is always the *older* build that decides whether a +// newer release is acceptable. Changing this text would make every device +// already in the field reject the release that changed it, and every release +// after that, with no way to fix it from the new release's side. Both the +// producer and the probe read it from here so they cannot drift, and the probe +// matches this as one line of output rather than the whole stream so that +// adding another line elsewhere stays harmless. +func VersionLine(version, platformID string) string { + return fmt.Sprintf("Zaparoo v%s (%s)", version, platformID) +} diff --git a/pkg/config/app_test.go b/pkg/config/app_test.go new file mode 100644 index 000000000..1befa390b --- /dev/null +++ b/pkg/config/app_test.go @@ -0,0 +1,50 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package config + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// TestVersionLine_IsFrozen is a tripwire, not a restatement of the code. The +// self-update probe runs in the binary a device already has installed and +// compares what a freshly downloaded one prints against this text, so the older +// build is always the one judging the newer. Editing the format would make +// every device in the field refuse the release that changed it and every +// release after it, unrecoverably from the new release's side. +// +// If this test fails, the change is a compatibility break, not a typo fix. +func TestVersionLine_IsFrozen(t *testing.T) { + t.Parallel() + + assert.Equal(t, "Zaparoo v2.11.0 (mister)", VersionLine("2.11.0", "mister")) + assert.Equal(t, "Zaparoo v2.11.0-beta4 (linux)", VersionLine("2.11.0-beta4", "linux")) +} + +// TestVersionFlagName_IsFrozen guards the other half of the same contract: the +// probe invokes a downloaded binary with this flag, so an installed build can +// only ask a future one for its version by the name it knows today. +func TestVersionFlagName_IsFrozen(t *testing.T) { + t.Parallel() + + assert.Equal(t, "version", VersionFlagName) +} diff --git a/pkg/service/updater/extract.go b/pkg/service/updater/extract.go new file mode 100644 index 000000000..19609d0cd --- /dev/null +++ b/pkg/service/updater/extract.go @@ -0,0 +1,364 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +// Extraction here pulls rather than unpacks. It walks a bounded number of +// archive members, ignores everything that is not a regular file, and copies +// the one member it wants into a path this package chose. No name out of the +// archive ever reaches the filesystem, which is what removes zip-slip, "..", +// absolute paths and duplicate names as things to get right: there is no code +// path where an archive decides where a byte lands. + +package updater + +import ( + "archive/tar" + "archive/zip" + "compress/gzip" + "context" + "crypto/sha256" + "crypto/subtle" + "encoding/hex" + "errors" + "fmt" + "io" + "os" + "regexp" + "strings" + + "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/updater/otameta" +) + +const ( + // semverPattern matches a version inside an archive member name. + semverPattern = `(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)` + + `(?:-((?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?` + + `(?:\+([0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?` +) + +// ctxReader ends a read once the context is done. The archive walks need it +// because they are otherwise uninterruptible: a gzip stream cannot seek, so tar +// has to inflate every member it skips on the way past, and a cancelled staging +// attempt would keep decompressing tens of megabytes on a device that is trying +// to shut down. +type ctxReader struct { + ctx context.Context + source io.Reader +} + +func (r *ctxReader) Read(p []byte) (int, error) { + if err := r.ctx.Err(); err != nil { + return 0, fmt.Errorf("reading the update archive: %w", err) + } + //nolint:wrapcheck // a reader wrapper has to pass io.EOF and the source's errors through unchanged + return r.source.Read(p) +} + +// stagedSink is where a member copied out of an archive lands. *os.File is the +// only implementation outside tests; the interface is here so a destination that +// fails mid-write can be exercised without filling a real disk. +type stagedSink interface { + io.Writer + Sync() error + Close() error +} + +// errWriter remembers the destination's own failures. io.Copy fuses the two +// directions into a single error value, and here they mean opposite things: a +// read that fails is the archive's problem, a write that fails is the device's. +type errWriter struct { + dest io.Writer + err error +} + +func (w *errWriter) Write(p []byte) (int, error) { + n, err := w.dest.Write(p) + if err != nil { + w.err = err + } + //nolint:wrapcheck // a writer wrapper has to pass the destination's error through unchanged + return n, err +} + +// extractBinary opens the archive once, proves the bytes sitting on disk are +// still the ones the signed manifest describes, and copies the executable out of +// that same open file. +func (s *stager) extractBinary(ctx context.Context, archivePath, ext string, want []byte, destPath string) error { + //nolint:gosec // the path is built by this package inside its own staging directory + f, err := os.Open(archivePath) + if err != nil { + return fmt.Errorf("opening the update archive: %w", err) + } + defer closeQuietly(f, "update archive") + + size, err := verifyOpenArchive(ctx, f, want) + if err != nil { + return err + } + // Only the tar path reads sequentially; zip addresses the handle directly and + // does not care where the offset is. Rewinding both keeps that an + // implementation detail of the format rather than of this function. + if _, err := f.Seek(0, io.SeekStart); err != nil { + return fmt.Errorf("rewinding the update archive: %w", err) + } + + switch ext { + case otameta.ArchiveExtTarGz: + return s.extractFromTarGz(ctx, f, destPath) + case otameta.ArchiveExtZip: + return s.extractFromZip(ctx, f, size, destPath) + default: + return fmt.Errorf("%w: %q is not an archive type this build unpacks", ErrArchiveRejected, ext) + } +} + +// verifyOpenArchive re-checks the archive against the manifest digest, reading +// through the handle extraction is about to use. +// +// The download already hashed these bytes on their way to disk. This hashes them +// on the way back off it, which is a different claim: in between there is a +// close, and the storage these devices run on has been observed acknowledging an +// fsync and later returning zeroed pages, so "the bytes that arrived were right" +// does not establish "the bytes about to be installed are right". Reading +// through the same open file rather than re-opening by path is what ties the two +// reads to one inode. +// +// Without this the .tar.gz platforms have no integrity check on the second read +// at all: tar carries no member checksum, and the walk stops at the +// end-of-archive marker without ever driving the gzip stream to its trailer. The +// .zip platforms get per-member CRC32 for free, which is weaker than this and +// covers only the member that is read. +func verifyOpenArchive(ctx context.Context, f *os.File, want []byte) (int64, error) { + digest := sha256.New() + size, err := io.Copy(digest, &ctxReader{ctx: ctx, source: f}) + if err != nil { + return 0, fmt.Errorf("re-reading the update archive: %w", err) + } + + got := digest.Sum(nil) + if subtle.ConstantTimeCompare(got, want) != 1 { + return 0, fmt.Errorf("%w: the archive on disk hashes to %s, the manifest declares %s", + ErrChecksumMismatch, hex.EncodeToString(got), hex.EncodeToString(want)) + } + return size, nil +} + +func (s *stager) extractFromTarGz(ctx context.Context, f *os.File, destPath string) error { + gz, err := gzip.NewReader(&ctxReader{ctx: ctx, source: f}) + if err != nil { + if ctxErr := ctx.Err(); ctxErr != nil { + return fmt.Errorf("reading the update archive was cancelled: %w", ctxErr) + } + return fmt.Errorf("%w: reading the update archive: %w", ErrArchiveRejected, err) + } + defer closeQuietly(gz, "update archive decompressor") + + // The member count bounds how many entries the walk visits; this bounds how + // much content getting to them can cost. A skipped member still has to be + // inflated in full, because gzip cannot seek past one, so without a ceiling + // here a bomb ahead of the binary would run unbounded. + inflated := &io.LimitedReader{R: gz, N: s.maxInflatedBytes + 1} + overBudget := func() bool { return inflated.N <= 0 } + + tr := tar.NewReader(inflated) + found := false + members := 0 + for { + header, nextErr := tr.Next() + if errors.Is(nextErr, io.EOF) { + break + } + if nextErr != nil { + // Caller intent first. ErrArchiveRejected is a permanent verdict on the + // release, so returning it for what was really a shutdown would condemn + // a build nothing had actually found fault with. + if ctxErr := ctx.Err(); ctxErr != nil { + return fmt.Errorf("reading the update archive was cancelled: %w", ctxErr) + } + if overBudget() { + return fmt.Errorf("%w: more than %d bytes of content in it", + ErrArchiveRejected, s.maxInflatedBytes) + } + return fmt.Errorf("%w: reading the update archive: %w", ErrArchiveRejected, nextErr) + } + + members++ + if members > maxArchiveMembers { + return fmt.Errorf("%w: more than %d members", ErrArchiveRejected, maxArchiveMembers) + } + + // Regular files only. A symlink, hardlink, device node, fifo or + // directory entry is skipped rather than reasoned about. + if header.Typeflag != tar.TypeReg || !s.wantsMember(header.Name) { + continue + } + if found { + return fmt.Errorf("%w: more than one %s", ErrArchiveRejected, s.binaryName) + } + if header.Size > s.maxFileBytes { + return fmt.Errorf("%w: %s declares %d bytes, over the %d byte limit", + ErrArchiveRejected, header.Name, header.Size, s.maxFileBytes) + } + if copyErr := copyStagedFile(ctx, tr, destPath, s.maxFileBytes); copyErr != nil { + // copyStagedFile already reports a cancellation as one, so only the + // budget needs separating out here. + if ctx.Err() == nil && overBudget() { + return fmt.Errorf("%w: more than %d bytes of content in it", + ErrArchiveRejected, s.maxInflatedBytes) + } + return copyErr + } + found = true + } + + if !found { + return fmt.Errorf("%w: no %s in it", ErrArchiveRejected, s.binaryName) + } + return nil +} + +func (s *stager) extractFromZip(ctx context.Context, f *os.File, size int64, destPath string) error { + // No total-content ceiling here, unlike the tar walk: a zip member that is + // not wanted is never opened, so nothing but the binary is ever inflated and + // maxFileBytes already bounds that. + r, err := zip.NewReader(f, size) + if err != nil { + return fmt.Errorf("%w: reading the update archive: %w", ErrArchiveRejected, err) + } + + if len(r.File) > maxArchiveMembers { + return fmt.Errorf("%w: more than %d members", ErrArchiveRejected, maxArchiveMembers) + } + + found := false + for _, member := range r.File { + if ctxErr := ctx.Err(); ctxErr != nil { + return fmt.Errorf("reading the update archive: %w", ctxErr) + } + // Regular files only, same as the tar walk: the mode bits are what + // carry a zip symlink, and a directory entry is not a file. + if !member.Mode().IsRegular() || !s.wantsMember(member.Name) { + continue + } + if found { + return fmt.Errorf("%w: more than one %s", ErrArchiveRejected, s.binaryName) + } + if declared := member.FileInfo().Size(); declared > s.maxFileBytes { + return fmt.Errorf("%w: %s declares %d bytes, over the %d byte limit", + ErrArchiveRejected, member.Name, declared, s.maxFileBytes) + } + + rc, openErr := member.Open() + if openErr != nil { + return fmt.Errorf("%w: reading %s: %w", ErrArchiveRejected, member.Name, openErr) + } + copyErr := copyStagedFile(ctx, rc, destPath, s.maxFileBytes) + closeQuietly(rc, "update archive member") + if copyErr != nil { + return copyErr + } + found = true + } + + if !found { + return fmt.Errorf("%w: no %s in it", ErrArchiveRejected, s.binaryName) + } + return nil +} + +// wantsMember reports whether a member is the binary being staged. It has to be +// the member's whole name: release archives are flat, so a nested path claiming +// to be the binary is not something a genuine build produces. +func (s *stager) wantsMember(name string) bool { + if strings.ContainsAny(name, `/\`) { + return false + } + return matchExecutableName(s.binaryName, s.goos, s.goarch, name) +} + +// matchExecutableName reports whether an archive member names the executable +// cmd: the bare name, optionally carrying a version and an os/arch pair, with +// an optional .exe. It reimplements the rule go-selfupdate applies inside its +// own decompressor, because nothing here goes through that decompressor and the +// rule decides which file gets installed. +func matchExecutableName(cmd, goos, goarch, target string) bool { + base := strings.TrimSuffix(cmd, ".exe") + if base == "" { + return false + } + // Every part is quoted, so a name with punctuation in it stays a name. The + // MiSTer builds are called zaparoo.sh, which without quoting would match + // anything in that position. + pattern := regexp.MustCompile(fmt.Sprintf( + `^%s([_-]v?%s)?([_-]%s[_-]%s)?(\.exe)?$`, + regexp.QuoteMeta(base), + semverPattern, + regexp.QuoteMeta(goos), + regexp.QuoteMeta(goarch), + )) + return pattern.MatchString(target) +} + +// copyStagedFile writes an archive member to a path this package chose. The +// limit is enforced against the bytes that actually arrive rather than the size +// the archive declares, so a header that lies about its size is caught mid-copy +// instead of being trusted to fill a disk. +func copyStagedFile(ctx context.Context, src io.Reader, destPath string, limit int64) error { + //nolint:gosec // the path is built by this package inside its own staging directory + f, err := os.OpenFile(destPath, os.O_WRONLY|os.O_CREATE|os.O_EXCL, stagedFilePerm) + if err != nil { + return fmt.Errorf("creating the staged update binary: %w", err) + } + return copyIntoSink(ctx, src, f, limit) +} + +// copyIntoSink is copyStagedFile with the destination already open. It owns +// flushing and closing it, whichever way the copy goes. +func copyIntoSink(ctx context.Context, src io.Reader, dest stagedSink, limit int64) error { + sink := &errWriter{dest: dest} + limited := io.LimitReader(&ctxReader{ctx: ctx, source: src}, limit+1) + written, copyErr := io.Copy(sink, limited) + syncErr := dest.Sync() + closeErr := dest.Close() + + switch { + case copyErr != nil && ctx.Err() != nil: + return fmt.Errorf("extracting the update binary was cancelled: %w", ctx.Err()) + case copyErr != nil && sink.err != nil: + // The device failed, not the release. ErrArchiveRejected is a verdict on + // the build itself, so a full or failing SD card must not earn one: the + // flush below already reports the same physical fault as an ordinary + // error, and so does the download's own write path. + return fmt.Errorf("writing the staged update binary: %w", sink.err) + case copyErr != nil: + return fmt.Errorf("%w: extracting the update binary: %w", ErrArchiveRejected, copyErr) + case syncErr != nil: + return fmt.Errorf("flushing the staged update binary to disk: %w", syncErr) + case closeErr != nil: + return fmt.Errorf("closing the staged update binary: %w", closeErr) + } + + if written > limit { + return fmt.Errorf("%w: the update binary is larger than the %d byte limit", + ErrArchiveRejected, limit) + } + if written == 0 { + return fmt.Errorf("%w: the update binary is empty", ErrArchiveRejected) + } + return nil +} diff --git a/pkg/service/updater/extract_test.go b/pkg/service/updater/extract_test.go new file mode 100644 index 000000000..dfd8b05f5 --- /dev/null +++ b/pkg/service/updater/extract_test.go @@ -0,0 +1,1010 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package updater + +import ( + "archive/tar" + "archive/zip" + "bytes" + "compress/gzip" + "context" + "crypto/sha256" + "encoding/hex" + "errors" + "fmt" + "io" + "io/fs" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/updater/otameta" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// tarMember describes one entry to write into a test tar. The size in the +// header always matches the body, because tar.Writer refuses to let a header +// lie about it; the guard against an oversized declared size is exercised by +// lowering the stager's limit instead. +type tarMember struct { + name string + linkname string + body []byte + mode int64 + typeflag byte +} + +// zipMember describes one entry to write into a test zip. The mode carries what +// a zip uses instead of a type flag: a symlink or a directory is a mode bit. +type zipMember struct { + name string + body []byte + mode fs.FileMode +} + +func writeTarGz(t *testing.T, path string, members []tarMember) { + t.Helper() + + var buf bytes.Buffer + gz, err := gzip.NewWriterLevel(&buf, gzip.BestSpeed) + require.NoError(t, err) + tw := tar.NewWriter(gz) + + for _, m := range members { + header := &tar.Header{ + Name: m.name, + Linkname: m.linkname, + Typeflag: m.typeflag, + Mode: m.mode, + } + if header.Typeflag == 0 { + header.Typeflag = tar.TypeReg + } + if header.Mode == 0 { + header.Mode = 0o644 + } + if header.Typeflag == tar.TypeReg { + header.Size = int64(len(m.body)) + } + require.NoError(t, tw.WriteHeader(header)) + if header.Typeflag == tar.TypeReg && len(m.body) > 0 { + _, writeErr := tw.Write(m.body) + require.NoError(t, writeErr) + } + } + + require.NoError(t, tw.Close()) + require.NoError(t, gz.Close()) + require.NoError(t, os.WriteFile(path, buf.Bytes(), 0o600)) +} + +func writeZip(t *testing.T, path string, members []zipMember) { + t.Helper() + + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + + for _, m := range members { + header := &zip.FileHeader{Name: m.name, Method: zip.Deflate} + mode := m.mode + if mode == 0 { + mode = 0o644 + } + header.SetMode(mode) + w, err := zw.CreateHeader(header) + require.NoError(t, err) + if len(m.body) > 0 { + _, writeErr := w.Write(m.body) + require.NoError(t, writeErr) + } + } + + require.NoError(t, zw.Close()) + require.NoError(t, os.WriteFile(path, buf.Bytes(), 0o600)) +} + +// extractHarness lays a tree out so an escape is visible: the archive, the +// payload directory extraction is allowed to write into, and a sibling holding +// a file that must never change. +type extractHarness struct { + stager *stager + base string + archivePath string + destPath string + payloadDir string +} + +func newExtractHarness(t *testing.T, ext string) *extractHarness { + t.Helper() + + base := t.TempDir() + payloadDir := filepath.Join(base, "staging", payloadSubdir) + require.NoError(t, os.MkdirAll(payloadDir, 0o750)) + require.NoError(t, os.MkdirAll(filepath.Join(base, "archive"), 0o750)) + require.NoError(t, os.MkdirAll(filepath.Join(base, "outside"), 0o750)) + require.NoError(t, os.WriteFile( + filepath.Join(base, "outside", "sentinel"), []byte("untouched"), 0o600)) + + return &extractHarness{ + stager: &stager{ + binaryName: "zaparoo", + goos: "linux", + goarch: "amd64", + maxFileBytes: maxStagedFileBytes, + maxInflatedBytes: maxArchiveInflatedBytes, + }, + base: base, + archivePath: filepath.Join(base, "archive", "release"+ext), + destPath: filepath.Join(payloadDir, "zaparoo"), + payloadDir: payloadDir, + } +} + +// extract runs extraction and asserts the only thing it changed is inside the +// payload directory, which is the whole point of naming the destination +// ourselves rather than unpacking archive-supplied paths. +func (h *extractHarness) extract(t *testing.T, ext string) error { + t.Helper() + + // Hashed as the archive stands right now, so a test that deliberately + // corrupts one still exercises the format handling instead of stopping at the + // on-disk digest check. TestExtractBinary_RejectsBytesChangedOnDisk covers + // that check on its own. + return h.extractWithDigest(t, ext, archiveDigest(t, h.archivePath)) +} + +func (h *extractHarness) extractWithDigest(t *testing.T, ext string, want []byte) error { + t.Helper() + + before := snapshotOutside(t, h.base, h.payloadDir) + err := h.stager.extractBinary(context.Background(), h.archivePath, ext, want, h.destPath) + assert.Equal(t, before, snapshotOutside(t, h.base, h.payloadDir), + "extraction wrote outside the payload directory") + return err +} + +func archiveDigest(t *testing.T, path string) []byte { + t.Helper() + + body, err := os.ReadFile(path) //nolint:gosec // test path under t.TempDir + require.NoError(t, err) + sum := sha256.Sum256(body) + return sum[:] +} + +// snapshotOutside records the whole tree except the payload directory. +func snapshotOutside(t *testing.T, base, payloadDir string) map[string]string { + t.Helper() + + found := make(map[string]string) + err := filepath.WalkDir(base, func(path string, d fs.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + if path == payloadDir { + return fs.SkipDir + } + rel, relErr := filepath.Rel(base, path) + if relErr != nil { + return fmt.Errorf("relative path of %s: %w", path, relErr) + } + if d.IsDir() { + found[rel] = "dir" + return nil + } + body, readErr := os.ReadFile(path) //nolint:gosec // test tree under t.TempDir + if readErr != nil { + return fmt.Errorf("reading %s: %w", path, readErr) + } + sum := sha256.Sum256(body) + found[rel] = hex.EncodeToString(sum[:]) + return nil + }) + require.NoError(t, err) + return found +} + +func TestMatchExecutableName(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + cmd string + goos string + goarch string + target string + want bool + }{ + {name: "bare name", cmd: "zaparoo", goos: "linux", goarch: "amd64", target: "zaparoo", want: true}, + { + name: "version with underscore", cmd: "zaparoo", goos: "linux", goarch: "amd64", + target: "zaparoo_2.11.0", want: true, + }, + { + name: "version with dash", cmd: "zaparoo", goos: "linux", goarch: "amd64", + target: "zaparoo-2.11.0", want: true, + }, + { + name: "version with v prefix", cmd: "zaparoo", goos: "linux", goarch: "amd64", + target: "zaparoo_v2.11.0", want: true, + }, + { + name: "prerelease version", cmd: "zaparoo", goos: "linux", goarch: "amd64", + target: "zaparoo-2.11.0-beta.1", want: true, + }, + { + name: "os and arch", cmd: "zaparoo", goos: "linux", goarch: "amd64", + target: "zaparoo_linux_amd64", want: true, + }, + { + name: "version and os and arch", cmd: "zaparoo", goos: "linux", goarch: "amd64", + target: "zaparoo_2.11.0_linux_amd64", want: true, + }, + { + name: "exe suffix accepted", cmd: "zaparoo", goos: "linux", goarch: "amd64", + target: "zaparoo.exe", want: true, + }, + { + name: "windows binary", cmd: "Zaparoo.exe", goos: "windows", goarch: "amd64", + target: "Zaparoo.exe", want: true, + }, + { + name: "windows binary without extension", cmd: "Zaparoo.exe", goos: "windows", goarch: "amd64", + target: "Zaparoo", want: true, + }, + { + name: "match is case sensitive", cmd: "Zaparoo.exe", goos: "windows", goarch: "amd64", + target: "zaparoo.exe", want: false, + }, + { + name: "mister binary keeps its sh name", cmd: "zaparoo.sh", goos: "linux", goarch: "arm", + target: "zaparoo.sh", want: true, + }, + { + name: "mister binary with version", cmd: "zaparoo.sh", goos: "linux", goarch: "arm", + target: "zaparoo.sh_2.11.0", want: true, + }, + // The dot in zaparoo.sh has to stay a dot. Without QuoteMeta it would + // match any character in that position. + { + name: "dot in the name is literal", cmd: "zaparoo.sh", goos: "linux", goarch: "arm", + target: "zaparoosh", want: false, + }, + { + name: "mister binary is not the plain name", cmd: "zaparoo.sh", goos: "linux", goarch: "arm", + target: "zaparoo", want: false, + }, + { + name: "sh suffix is not the plain binary", cmd: "zaparoo", goos: "linux", goarch: "amd64", + target: "zaparoo.sh", want: false, + }, + { + name: "other os", cmd: "zaparoo", goos: "linux", goarch: "amd64", + target: "zaparoo_windows_amd64", want: false, + }, + { + name: "other arch", cmd: "zaparoo", goos: "linux", goarch: "amd64", + target: "zaparoo_linux_arm64", want: false, + }, + // arm must not match an arm64 archive member. + { + name: "arm does not match arm64", cmd: "zaparoo", goos: "linux", goarch: "arm", + target: "zaparoo_linux_arm64", want: false, + }, + {name: "licence", cmd: "zaparoo", goos: "linux", goarch: "amd64", target: "LICENSE.txt", want: false}, + {name: "trailing junk", cmd: "zaparoo", goos: "linux", goarch: "amd64", target: "zaparoo2", want: false}, + {name: "nested path", cmd: "zaparoo", goos: "linux", goarch: "amd64", target: "bin/zaparoo", want: false}, + {name: "empty target", cmd: "zaparoo", goos: "linux", goarch: "amd64", target: "", want: false}, + {name: "empty command", cmd: "", goos: "linux", goarch: "amd64", target: "zaparoo", want: false}, + {name: "command is only an extension", cmd: ".exe", goos: "windows", goarch: "amd64", target: "", want: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := matchExecutableName(tt.cmd, tt.goos, tt.goarch, tt.target) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestWantsMember_RejectsAnythingWithAPathInIt(t *testing.T) { + t.Parallel() + + s := &stager{binaryName: "zaparoo", goos: "linux", goarch: "amd64"} + + assert.True(t, s.wantsMember("zaparoo")) + assert.False(t, s.wantsMember("bin/zaparoo")) + assert.False(t, s.wantsMember(`bin\zaparoo`)) + assert.False(t, s.wantsMember("../zaparoo")) + assert.False(t, s.wantsMember("/zaparoo")) + assert.False(t, s.wantsMember("./zaparoo")) +} + +func TestExtractFromTarGz_TakesOnlyTheBinary(t *testing.T) { + t.Parallel() + + h := newExtractHarness(t, otameta.ArchiveExtTarGz) + binary := []byte("this stands in for the executable") + writeTarGz(t, h.archivePath, []tarMember{ + {name: "LICENSE.txt", body: []byte("gpl")}, + {name: "README.txt", body: []byte("readme")}, + {name: "zaparoo", body: binary, mode: 0o755}, + }) + + require.NoError(t, h.extract(t, otameta.ArchiveExtTarGz)) + + got, err := os.ReadFile(h.destPath) //nolint:gosec // test path under t.TempDir + require.NoError(t, err) + assert.Equal(t, binary, got) + + // The licence and readme are in the archive and are deliberately not + // written: extraction pulls what it wants, it does not unpack. + entries, err := os.ReadDir(h.payloadDir) + require.NoError(t, err) + require.Len(t, entries, 1) + assert.Equal(t, "zaparoo", entries[0].Name()) +} + +func TestExtractFromZip_TakesOnlyTheBinary(t *testing.T) { + t.Parallel() + + h := newExtractHarness(t, otameta.ArchiveExtZip) + binary := []byte("this stands in for the executable") + writeZip(t, h.archivePath, []zipMember{ + {name: "LICENSE.txt", body: []byte("gpl")}, + {name: "README.txt", body: []byte("readme")}, + {name: "zaparoo", body: binary, mode: 0o755}, + }) + + require.NoError(t, h.extract(t, otameta.ArchiveExtZip)) + + got, err := os.ReadFile(h.destPath) //nolint:gosec // test path under t.TempDir + require.NoError(t, err) + assert.Equal(t, binary, got) + + entries, err := os.ReadDir(h.payloadDir) + require.NoError(t, err) + require.Len(t, entries, 1) + assert.Equal(t, "zaparoo", entries[0].Name()) +} + +func TestExtractFromTarGz_VersionedMemberName(t *testing.T) { + t.Parallel() + + h := newExtractHarness(t, otameta.ArchiveExtTarGz) + binary := []byte("versioned") + writeTarGz(t, h.archivePath, []tarMember{ + {name: "zaparoo_2.11.0_linux_amd64", body: binary, mode: 0o755}, + }) + + require.NoError(t, h.extract(t, otameta.ArchiveExtTarGz)) + + // It landed under the name this package chose, not the one in the archive. + got, err := os.ReadFile(h.destPath) //nolint:gosec // test path under t.TempDir + require.NoError(t, err) + assert.Equal(t, binary, got) +} + +func TestExtractBinary_UnknownExtension(t *testing.T) { + t.Parallel() + + h := newExtractHarness(t, ".7z") + require.NoError(t, os.WriteFile(h.archivePath, []byte("not an archive"), 0o600)) + + err := h.extract(t, ".7z") + require.ErrorIs(t, err, ErrArchiveRejected) + assert.NoFileExists(t, h.destPath) +} + +// TestExtractBinary_RejectsBytesChangedOnDisk is the claim the download's own +// hash cannot make. That one proves the bytes that arrived were right; this one +// proves the bytes about to be installed are. Between the two there is a close, +// and the storage these devices run on has been seen acknowledging an fsync and +// later handing back zeroed pages, so the archive is re-read through the handle +// extraction is about to use rather than trusted. +func TestExtractBinary_RejectsBytesChangedOnDisk(t *testing.T) { + t.Parallel() + + for _, ext := range []string{otameta.ArchiveExtTarGz, otameta.ArchiveExtZip} { + t.Run(ext, func(t *testing.T) { + t.Parallel() + + h := newExtractHarness(t, ext) + writeArchive(t, h.archivePath, ext, "zaparoo", []byte("the binary")) + + // What the download verified, before the disk changed under it. + want := archiveDigest(t, h.archivePath) + + body, err := os.ReadFile(h.archivePath) //nolint:gosec // test path under t.TempDir + require.NoError(t, err) + body[len(body)/2] ^= 0xff + //nolint:gosec // G703: test path under t.TempDir + require.NoError(t, os.WriteFile(h.archivePath, body, 0o600)) + + err = h.extractWithDigest(t, ext, want) + require.ErrorIs(t, err, ErrChecksumMismatch) + assert.NoFileExists(t, h.destPath, "a binary was staged out of an archive that no longer verifies") + }) + } +} + +// TestExtractBinary_StopsOnCancellation asserts a cancelled staging attempt gets +// no further than the digest re-read, and is not blamed on the archive. +func TestExtractBinary_StopsOnCancellation(t *testing.T) { + t.Parallel() + + for _, ext := range []string{otameta.ArchiveExtTarGz, otameta.ArchiveExtZip} { + t.Run(ext, func(t *testing.T) { + t.Parallel() + + h := newExtractHarness(t, ext) + writeArchive(t, h.archivePath, ext, "zaparoo", []byte("the binary")) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + err := h.stager.extractBinary(ctx, h.archivePath, ext, archiveDigest(t, h.archivePath), h.destPath) + require.ErrorIs(t, err, context.Canceled) + // Nothing was wrong with the archive, so nothing may say there was. + require.NotErrorIs(t, err, ErrChecksumMismatch) + require.NotErrorIs(t, err, ErrArchiveRejected) + assert.NoFileExists(t, h.destPath) + }) + } +} + +// TestExtractWalk_StopsOnCancellation covers the walk itself, which is the +// stretch that is otherwise uninterruptible: a gzip stream cannot be seeked past, +// so tar inflates every member it skips, and a shutdown part-way through would +// keep decompressing tens of megabytes on a device trying to stop. The walks are +// called directly because reaching them past the digest read needs a context that +// is live for one and dead for the other. +func TestExtractWalk_StopsOnCancellation(t *testing.T) { + t.Parallel() + + tests := []struct { + walk func(*extractHarness, context.Context, *os.File, int64) error + ext string + }{ + { + ext: otameta.ArchiveExtTarGz, + walk: func(h *extractHarness, ctx context.Context, f *os.File, _ int64) error { + return h.stager.extractFromTarGz(ctx, f, h.destPath) + }, + }, + { + ext: otameta.ArchiveExtZip, + walk: func(h *extractHarness, ctx context.Context, f *os.File, size int64) error { + return h.stager.extractFromZip(ctx, f, size, h.destPath) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.ext, func(t *testing.T) { + t.Parallel() + + h := newExtractHarness(t, tt.ext) + writeArchive(t, h.archivePath, tt.ext, "zaparoo", []byte("the binary")) + + f, err := os.Open(h.archivePath) //nolint:gosec // test path under t.TempDir + require.NoError(t, err) + t.Cleanup(func() { _ = f.Close() }) + info, err := f.Stat() + require.NoError(t, err) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + err = tt.walk(h, ctx, f, info.Size()) + require.ErrorIs(t, err, context.Canceled) + // ErrArchiveRejected is a permanent verdict on the release. A walk that + // was interrupted reached no verdict at all, so it must not return one. + require.NotErrorIs(t, err, ErrArchiveRejected) + assert.NoFileExists(t, h.destPath) + }) + } +} + +// TestExtractFromTarGz_InflateBudget bounds what reaching the binary can be made +// to cost. The member cap alone does not: a skipped member still has to be +// inflated in full, so content ahead of the binary is work the walk cannot +// decline. +func TestExtractFromTarGz_InflateBudget(t *testing.T) { + t.Parallel() + + h := newExtractHarness(t, otameta.ArchiveExtTarGz) + // Compresses to almost nothing on the wire and to well over the budget once + // inflated, which is the shape of the attack. + writeTarGz(t, h.archivePath, []tarMember{ + {name: "filler.bin", body: make([]byte, 64<<10)}, + {name: "zaparoo", body: []byte("the binary"), mode: 0o755}, + }) + h.stager.maxInflatedBytes = 4 << 10 + + err := h.extract(t, otameta.ArchiveExtTarGz) + require.ErrorIs(t, err, ErrArchiveRejected) + assert.Contains(t, err.Error(), "bytes of content in it") + assert.NoFileExists(t, h.destPath) +} + +// TestExtractFromZip_NeedsNoInflateBudget is the other half of that: a zip member +// the walk does not want is never opened, so the same filler costs nothing and +// must not be refused. +func TestExtractFromZip_NeedsNoInflateBudget(t *testing.T) { + t.Parallel() + + h := newExtractHarness(t, otameta.ArchiveExtZip) + binary := []byte("the binary") + writeZip(t, h.archivePath, []zipMember{ + {name: "filler.bin", body: make([]byte, 64<<10)}, + {name: "zaparoo", body: binary, mode: 0o755}, + }) + h.stager.maxInflatedBytes = 4 << 10 + + require.NoError(t, h.extract(t, otameta.ArchiveExtZip)) + + got, err := os.ReadFile(h.destPath) //nolint:gosec // test path under t.TempDir + require.NoError(t, err) + assert.Equal(t, binary, got) +} + +// writeArchive writes a minimal release archive in either format. +func writeArchive(t *testing.T, path, ext, memberName string, binary []byte) { + t.Helper() + + switch ext { + case otameta.ArchiveExtTarGz: + writeTarGz(t, path, []tarMember{{name: memberName, body: binary, mode: 0o755}}) + case otameta.ArchiveExtZip: + writeZip(t, path, []zipMember{{name: memberName, body: binary, mode: 0o755}}) + default: + t.Fatalf("unsupported archive extension %q", ext) + } +} + +func TestExtractFromTarGz_HostileMembers(t *testing.T) { + t.Parallel() + + binary := []byte("the binary") + + crowded := make([]tarMember, 0, maxArchiveMembers+1) + for i := range maxArchiveMembers { + crowded = append(crowded, tarMember{name: fmt.Sprintf("filler-%d", i), body: []byte("x")}) + } + crowded = append(crowded, tarMember{name: "zaparoo", body: binary}) + + tests := []struct { + name string + wantMsg string + members []tarMember + limit int64 + }{ + { + name: "traversal name", + members: []tarMember{ + {name: "../../etc/passwd", body: []byte("root:x:0:0")}, + {name: "LICENSE.txt", body: []byte("gpl")}, + }, + wantMsg: "no zaparoo in it", + }, + { + name: "absolute name", + members: []tarMember{ + {name: "/etc/passwd", body: []byte("root:x:0:0")}, + }, + wantMsg: "no zaparoo in it", + }, + { + name: "nested binary", + members: []tarMember{ + {name: "bin/zaparoo", body: binary}, + }, + wantMsg: "no zaparoo in it", + }, + { + name: "symlink standing in for the binary", + members: []tarMember{ + {name: "zaparoo", linkname: "/etc/passwd", typeflag: tar.TypeSymlink}, + }, + wantMsg: "no zaparoo in it", + }, + { + name: "hardlink standing in for the binary", + members: []tarMember{ + {name: "zaparoo", linkname: "LICENSE.txt", typeflag: tar.TypeLink}, + {name: "LICENSE.txt", body: []byte("gpl")}, + }, + wantMsg: "no zaparoo in it", + }, + { + name: "character device", + members: []tarMember{ + {name: "zaparoo", typeflag: tar.TypeChar}, + }, + wantMsg: "no zaparoo in it", + }, + { + name: "block device", + members: []tarMember{ + {name: "zaparoo", typeflag: tar.TypeBlock}, + }, + wantMsg: "no zaparoo in it", + }, + { + name: "fifo", + members: []tarMember{ + {name: "zaparoo", typeflag: tar.TypeFifo}, + }, + wantMsg: "no zaparoo in it", + }, + { + name: "directory using the binary name", + members: []tarMember{ + {name: "zaparoo", typeflag: tar.TypeDir, mode: 0o755}, + }, + wantMsg: "no zaparoo in it", + }, + { + name: "no binary at all", + members: []tarMember{ + {name: "LICENSE.txt", body: []byte("gpl")}, + {name: "README.txt", body: []byte("readme")}, + }, + wantMsg: "no zaparoo in it", + }, + { + name: "two different names both match", + members: []tarMember{ + {name: "zaparoo", body: binary}, + {name: "zaparoo_linux_amd64", body: []byte("a different binary")}, + }, + wantMsg: "more than one zaparoo", + }, + { + name: "duplicate names", + members: []tarMember{ + {name: "zaparoo", body: binary}, + {name: "zaparoo", body: []byte("a different binary")}, + }, + wantMsg: "more than one zaparoo", + }, + { + name: "declared size over the limit", + members: []tarMember{ + {name: "zaparoo", body: bytes.Repeat([]byte("x"), 128)}, + }, + limit: 16, + wantMsg: "over the 16 byte limit", + }, + { + name: "too many members", + members: crowded, + wantMsg: fmt.Sprintf("more than %d members", maxArchiveMembers), + }, + { + name: "empty binary", + members: []tarMember{ + {name: "zaparoo", body: nil}, + }, + wantMsg: "the update binary is empty", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + h := newExtractHarness(t, otameta.ArchiveExtTarGz) + if tt.limit > 0 { + h.stager.maxFileBytes = tt.limit + } + writeTarGz(t, h.archivePath, tt.members) + + err := h.extract(t, otameta.ArchiveExtTarGz) + require.ErrorIs(t, err, ErrArchiveRejected) + assert.Contains(t, err.Error(), tt.wantMsg) + }) + } +} + +func TestExtractFromZip_HostileMembers(t *testing.T) { + t.Parallel() + + binary := []byte("the binary") + + crowded := make([]zipMember, 0, maxArchiveMembers+1) + for i := range maxArchiveMembers { + crowded = append(crowded, zipMember{name: fmt.Sprintf("filler-%d", i), body: []byte("x")}) + } + crowded = append(crowded, zipMember{name: "zaparoo", body: binary}) + + tests := []struct { + name string + wantMsg string + members []zipMember + limit int64 + }{ + { + name: "traversal name", + members: []zipMember{ + {name: "../../etc/passwd", body: []byte("root:x:0:0")}, + }, + wantMsg: "no zaparoo in it", + }, + { + name: "windows traversal name", + members: []zipMember{ + {name: `..\zaparoo`, body: binary}, + }, + wantMsg: "no zaparoo in it", + }, + { + name: "absolute name", + members: []zipMember{ + {name: "/etc/passwd", body: []byte("root:x:0:0")}, + }, + wantMsg: "no zaparoo in it", + }, + { + name: "nested binary", + members: []zipMember{ + {name: "bin/zaparoo", body: binary}, + }, + wantMsg: "no zaparoo in it", + }, + { + name: "symlink standing in for the binary", + members: []zipMember{ + {name: "zaparoo", body: []byte("/etc/passwd"), mode: fs.ModeSymlink | 0o777}, + }, + wantMsg: "no zaparoo in it", + }, + { + name: "directory using the binary name", + members: []zipMember{ + {name: "zaparoo", mode: fs.ModeDir | 0o755}, + }, + wantMsg: "no zaparoo in it", + }, + { + name: "no binary at all", + members: []zipMember{ + {name: "LICENSE.txt", body: []byte("gpl")}, + }, + wantMsg: "no zaparoo in it", + }, + { + name: "two different names both match", + members: []zipMember{ + {name: "zaparoo", body: binary}, + {name: "zaparoo_linux_amd64", body: []byte("a different binary")}, + }, + wantMsg: "more than one zaparoo", + }, + { + name: "duplicate names", + members: []zipMember{ + {name: "zaparoo", body: binary}, + {name: "zaparoo", body: []byte("a different binary")}, + }, + wantMsg: "more than one zaparoo", + }, + { + name: "declared size over the limit", + members: []zipMember{ + {name: "zaparoo", body: bytes.Repeat([]byte("x"), 128)}, + }, + limit: 16, + wantMsg: "over the 16 byte limit", + }, + { + name: "too many members", + members: crowded, + wantMsg: fmt.Sprintf("more than %d members", maxArchiveMembers), + }, + { + name: "empty binary", + members: []zipMember{ + {name: "zaparoo", body: nil}, + }, + wantMsg: "the update binary is empty", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + h := newExtractHarness(t, otameta.ArchiveExtZip) + if tt.limit > 0 { + h.stager.maxFileBytes = tt.limit + } + writeZip(t, h.archivePath, tt.members) + + err := h.extract(t, otameta.ArchiveExtZip) + require.ErrorIs(t, err, ErrArchiveRejected) + assert.Contains(t, err.Error(), tt.wantMsg) + }) + } +} + +func TestExtractFromTarGz_CorruptArchive(t *testing.T) { + t.Parallel() + + t.Run("not gzip at all", func(t *testing.T) { + t.Parallel() + + h := newExtractHarness(t, otameta.ArchiveExtTarGz) + require.NoError(t, os.WriteFile(h.archivePath, []byte("this is not gzip"), 0o600)) + + err := h.extract(t, otameta.ArchiveExtTarGz) + require.ErrorIs(t, err, ErrArchiveRejected) + assert.NoFileExists(t, h.destPath) + }) + + t.Run("truncated tar inside valid gzip", func(t *testing.T) { + t.Parallel() + + h := newExtractHarness(t, otameta.ArchiveExtTarGz) + full := filepath.Join(t.TempDir(), "full.tar.gz") + writeTarGz(t, full, []tarMember{ + {name: "LICENSE.txt", body: bytes.Repeat([]byte("gpl"), 4096)}, + {name: "zaparoo", body: bytes.Repeat([]byte("bin"), 4096)}, + }) + body, err := os.ReadFile(full) //nolint:gosec // test path under t.TempDir + require.NoError(t, err) + //nolint:gosec // G703: test path under t.TempDir + require.NoError(t, os.WriteFile(h.archivePath, body[:len(body)/2], 0o600)) + + err = h.extract(t, otameta.ArchiveExtTarGz) + require.ErrorIs(t, err, ErrArchiveRejected) + }) +} + +func TestExtractFromZip_CorruptArchive(t *testing.T) { + t.Parallel() + + h := newExtractHarness(t, otameta.ArchiveExtZip) + require.NoError(t, os.WriteFile(h.archivePath, []byte("this is not a zip"), 0o600)) + + err := h.extract(t, otameta.ArchiveExtZip) + require.ErrorIs(t, err, ErrArchiveRejected) + assert.NoFileExists(t, h.destPath) +} + +// TestCopyStagedFile_LimitsBytesThatArrive covers the backstop no archive header +// can talk its way past: the limit is applied to what actually turns up, so a +// member that streams more than it declared is still caught. +func TestCopyStagedFile_LimitsBytesThatArrive(t *testing.T) { + t.Parallel() + + dest := filepath.Join(t.TempDir(), "zaparoo") + err := copyStagedFile(context.Background(), strings.NewReader(strings.Repeat("x", 64)), dest, 16) + + require.ErrorIs(t, err, ErrArchiveRejected) + assert.Contains(t, err.Error(), "larger than the 16 byte limit") +} + +func TestCopyStagedFile_ExactLimitIsAccepted(t *testing.T) { + t.Parallel() + + dest := filepath.Join(t.TempDir(), "zaparoo") + require.NoError(t, copyStagedFile(context.Background(), strings.NewReader("0123456789abcdef"), dest, 16)) + + got, err := os.ReadFile(dest) //nolint:gosec // test path under t.TempDir + require.NoError(t, err) + assert.Equal(t, "0123456789abcdef", string(got)) +} + +func TestCopyStagedFile_RefusesToOverwrite(t *testing.T) { + t.Parallel() + + dest := filepath.Join(t.TempDir(), "zaparoo") + require.NoError(t, os.WriteFile(dest, []byte("already here"), 0o600)) + + err := copyStagedFile(context.Background(), strings.NewReader("new"), dest, 64) + require.Error(t, err) + assert.Contains(t, err.Error(), "creating the staged update binary") + + got, err := os.ReadFile(dest) //nolint:gosec // test path under t.TempDir + require.NoError(t, err) + assert.Equal(t, "already here", string(got)) +} + +func TestCopyStagedFile_ReadError(t *testing.T) { + t.Parallel() + + dest := filepath.Join(t.TempDir(), "zaparoo") + err := copyStagedFile(context.Background(), io.MultiReader( + strings.NewReader("some bytes"), + &failingReader{}, + ), dest, 1024) + + require.ErrorIs(t, err, ErrArchiveRejected) + assert.Contains(t, err.Error(), "extracting the update binary") +} + +// failingReader stands in for an archive member whose stream dies mid-read. +type failingReader struct{} + +func (*failingReader) Read([]byte) (int, error) { + return 0, io.ErrUnexpectedEOF +} + +// TestCopyIntoSink_WriteError is the failure the classification turns on: the +// bytes arriving are fine and the disk underneath is not. A device that has run +// out of space must not have the release condemned for it, so this error carries +// no verdict. +func TestCopyIntoSink_WriteError(t *testing.T) { + t.Parallel() + + sink := &failingSink{err: errDiskFull} + err := copyIntoSink(context.Background(), strings.NewReader(strings.Repeat("x", 64)), sink, 1024) + + require.Error(t, err) + require.NotErrorIs(t, err, ErrArchiveRejected) + require.ErrorIs(t, err, errDiskFull) + assert.Contains(t, err.Error(), "writing the staged update binary") + assert.True(t, sink.closed, "the destination has to be closed either way") +} + +// TestCopyIntoSink_ReadErrorStillRejects is the other half of the same switch: a +// stream that dies mid-member is the archive's problem and does earn a verdict. +func TestCopyIntoSink_ReadErrorStillRejects(t *testing.T) { + t.Parallel() + + sink := &failingSink{} + err := copyIntoSink(context.Background(), io.MultiReader( + strings.NewReader("some bytes"), + &failingReader{}, + ), sink, 1024) + + require.ErrorIs(t, err, ErrArchiveRejected) + assert.Contains(t, err.Error(), "extracting the update binary") +} + +// errDiskFull stands in for the write fault a full card produces, without +// needing a platform-specific errno. +var errDiskFull = errors.New("no space left on device") + +// failingSink stands in for a destination whose writes fail, which is how a full +// or failing card behaves. With err nil it accepts everything. +type failingSink struct { + err error + closed bool +} + +func (s *failingSink) Write(p []byte) (int, error) { + if s.err != nil { + return 0, s.err + } + return len(p), nil +} + +func (*failingSink) Sync() error { return nil } + +func (s *failingSink) Close() error { + s.closed = true + return nil +} diff --git a/pkg/service/updater/otameta/manifest.go b/pkg/service/updater/otameta/manifest.go index 739f5efa3..76096aa8d 100644 --- a/pkg/service/updater/otameta/manifest.go +++ b/pkg/service/updater/otameta/manifest.go @@ -47,8 +47,15 @@ const ( ChannelBeta = "beta" ) -// archiveExts are the extensions release builds package update archives with. -var archiveExts = []string{".tar.gz", ".zip"} +// ArchiveExtTarGz and ArchiveExtZip are the extensions release builds package +// update archives with. They live here because selection decides which one a +// platform gets and extraction has to agree with that decision. +const ( + ArchiveExtTarGz = ".tar.gz" + ArchiveExtZip = ".zip" +) + +var archiveExts = []string{ArchiveExtTarGz, ArchiveExtZip} var ( // ErrNoAsset means no archive in the release is installable here. That is @@ -227,6 +234,18 @@ func isArchiveName(name, base string) bool { return false } +// ArchiveExtension returns the extension of an archive name, which is what +// decides how it is unpacked. The list of extensions lives here so the client +// and the publisher cannot disagree about what an update archive is. +func ArchiveExtension(name string) (string, error) { + for _, ext := range archiveExts { + if strings.HasSuffix(name, ext) { + return ext, nil + } + } + return "", fmt.Errorf("%w: %q is not an update archive", ErrNoAsset, name) +} + // FindRelease returns the release carrying a tag, or nil. func FindRelease(m *Manifest, tag string) *Release { if m == nil { diff --git a/pkg/service/updater/otameta/manifest_test.go b/pkg/service/updater/otameta/manifest_test.go index 7747964cb..2384a0516 100644 --- a/pkg/service/updater/otameta/manifest_test.go +++ b/pkg/service/updater/otameta/manifest_test.go @@ -390,3 +390,34 @@ func TestManifest_ChannelNames(t *testing.T) { assert.Equal(t, "beta", ChannelBeta) assert.True(t, strings.HasPrefix(ArchiveBaseName("linux", "amd64", "2.16.1"), "zaparoo-")) } + +func TestArchiveExtension(t *testing.T) { + t.Parallel() + + for _, tt := range []struct { + name string + want string + }{ + {name: "zaparoo-mister_arm-2.16.1.zip", want: ".zip"}, + {name: "zaparoo-linux_amd64-2.16.1.tar.gz", want: ".tar.gz"}, + // .gz alone is not one of the two, so the suffix check must not treat the + // tail of .tar.gz as a match on its own. + {name: "zaparoo-linux_amd64-2.16.1.gz", want: ""}, + {name: "zaparoo-windows_amd64-2.16.1.exe", want: ""}, + {name: "checksums.txt", want: ""}, + {name: "", want: ""}, + } { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got, err := ArchiveExtension(tt.name) + if tt.want == "" { + require.ErrorIs(t, err, ErrNoAsset) + assert.Empty(t, got) + return + } + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/pkg/service/updater/stage.go b/pkg/service/updater/stage.go new file mode 100644 index 000000000..e65fa7567 --- /dev/null +++ b/pkg/service/updater/stage.go @@ -0,0 +1,741 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package updater + +import ( + "context" + "crypto/sha256" + "crypto/subtle" + "encoding/hex" + "errors" + "fmt" + "io" + "net" + "net/http" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "time" + + "github.com/Masterminds/semver/v3" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/config" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers/tlsroots" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/updater/otameta" + "github.com/rs/zerolog/log" +) + +const ( + // maxArchiveBytes refuses an absurd declared archive size before a byte is + // read. Release archives are 10-16 MB today. + maxArchiveBytes = 256 << 20 + + // maxStagedFileBytes caps one file copied out of an archive. It is enforced + // against the bytes that actually arrive rather than the size the archive + // declares, so a lying header is caught mid-copy rather than trusted. The + // binary is around 45 MB uncompressed today. + maxStagedFileBytes = 256 << 20 + + // maxArchiveMembers bounds how much of an archive is walked. Releases carry + // the binary, a licence, a readme and on one platform a scripts directory, + // so a hundred is generous; the point is that the walk terminates. + maxArchiveMembers = 100 + + // maxArchiveInflatedBytes bounds the total content the walk reads out of one + // archive, across every member including the ones it skips. maxStagedFileBytes + // caps the file that is kept; this caps what getting to it can be made to + // cost. + maxArchiveInflatedBytes = 384 << 20 + + // downloadStallTimeout is how long a transfer may make no progress at all + // before it is abandoned. It deliberately bounds silence rather than total + // duration: a legitimate download to a MiSTer over a slow link can run for + // minutes, and killing that would leave those devices unable to update. + downloadStallTimeout = 90 * time.Second + + // stallChecks is how many times the stall guard looks per timeout. Checking + // more often than the timeout keeps the worst-case detection delay to a + // fraction of it rather than double. + stallChecks = 4 + + // probeTimeout bounds the staged binary's version check. It prints one line + // and exits before opening anything, so a binary that has not answered in + // ten seconds is not going to. + probeTimeout = 10 * time.Second + + // probeWaitDelay is how long the probe waits for the output pipes to close + // after the process itself is gone. Without it a leftover child holding those + // pipes would block the probe past its timeout, which would strand the whole + // update rather than fail it. + probeWaitDelay = 2 * time.Second + + // probeOutputLimit caps how much of the staged binary's output the probe + // keeps. The probe runs a binary that arrived over the network moments ago, + // and one that fails by printing without stopping would otherwise be held + // whole in memory on a device that has a few hundred megabytes of it. Only + // the first line matters here, so keeping a few kilobytes loses nothing the + // error message would have used. + probeOutputLimit = 8 << 10 + + // stagingRemoveAttempts and stagingRemoveDelay bound how long removing a + // staging directory is retried. A staging directory holds a binary this + // process may have just finished executing for the probe, and Windows can keep + // such a file open for a moment after the process itself is gone, which fails + // a single removal with a sharing violation. Retrying costs nothing on the + // platforms where the first attempt always works; on the one where it does + // not, it is the difference between a failed update leaving its whole payload + // on the disk and leaving nothing. + stagingRemoveAttempts = 20 + stagingRemoveDelay = 100 * time.Millisecond + + // stagingSubdir holds one directory per staged version, under the updater's + // own directory rather than anywhere the platform might clean up. + stagingSubdir = "staging" + + // payloadSubdir holds the files pulled out of the archive, all named by this + // package. + payloadSubdir = "payload" + + stagedFilePerm = 0o600 + stagedBinaryPerm = 0o755 +) + +var ( + // ErrNotAnUpgrade means the release is not newer than what is running. It is + // checked here rather than trusted from the check response because that is + // what stops a stale or tampered manifest installing an older build. + ErrNotAnUpgrade = errors.New("release is not newer than the running version") + + // ErrUpgradeFloor means the release declares a minimum version to come from + // and this device is below it, so it has to take an intermediate build + // first. + ErrUpgradeFloor = errors.New("release cannot be installed directly from the running version") + + // ErrArchiveRejected covers everything about the archive that fails a rule: + // a size the manifest will not vouch for, an unreadable or overlong member, + // a missing binary, or two of them. + ErrArchiveRejected = errors.New("release archive was rejected") + + // ErrChecksumMismatch means the bytes that arrived are not the bytes the + // signed manifest describes. + ErrChecksumMismatch = errors.New("release archive does not match the manifest checksum") + + // ErrDownloadStalled means the transfer stopped making progress, either + // because the stall guard saw no bytes for the whole stall timeout or because + // one of the transport's own deadlines ran out waiting for a connect, a + // handshake or the response headers. It is a fault of the network, never of + // the release. + ErrDownloadStalled = errors.New("release archive download stalled") + + // ErrProbeFailed means the staged binary would not run here, or ran and + // disagreed about what version it is. This is the check that keeps a bad + // build from reaching a device with no supervisor to recover it. + ErrProbeFailed = errors.New("staged binary failed its version probe") +) + +// StageOptions describes one staging attempt. +type StageOptions struct { + // Release comes from a manifest whose signature has already been checked. + // The archive and the version are re-derived from it here rather than + // trusting anything passed alongside it. + Release *otameta.Release + // PlatformID is the platform half of the archive name. + PlatformID string + // Arch and OS default to this build's. They are settable so the selection + // and the archive member rules can be tested for platforms other than the + // one running the test. + Arch string + OS string + // TargetPath is the binary that will eventually be replaced. Only its base + // name is read here: it names the archive member to pull out, and the name + // the staged copy is written under. + TargetPath string + // StagingRoot holds one directory per staged version. + StagingRoot string + // CurrentVersion is the version running now, which the release has to beat. + CurrentVersion string +} + +// StagedUpdate is a verified release unpacked into files this process named, +// ready for the install stage to move into place. Nothing outside Dir has been +// touched to produce it. +type StagedUpdate struct { + // Dir is the staging directory holding the archive and the payload. + // Removing it undoes the whole staging attempt. + Dir string + // BinaryPath is the new executable. It came out of an archive whose bytes on + // disk were checked against the signed manifest immediately before it was + // read, and it has answered a version probe on this device. + BinaryPath string + // ArchivePath is the downloaded archive, kept so a later stage can report + // what it installed from. + ArchivePath string + // Version is the release version, without the tag's leading v. + Version string +} + +// assetFetcher retrieves an asset URL. Production hands back the CDN response +// body; tests serve archives from a local server. +type assetFetcher func(ctx context.Context, target string) (io.ReadCloser, error) + +// cappedBuilder keeps the first probeOutputLimit bytes written to it and +// discards the rest. It reports every write as fully consumed, so the process +// on the other end drains normally instead of blocking on a pipe nobody is +// reading. +type cappedBuilder struct { + buf strings.Builder +} + +func (b *cappedBuilder) Write(p []byte) (int, error) { + if room := probeOutputLimit - b.buf.Len(); room > 0 { + // strings.Builder.Write never fails. + _, _ = b.buf.Write(p[:min(room, len(p))]) + } + return len(p), nil +} + +func (b *cappedBuilder) String() string { + return b.buf.String() +} + +// stager holds the resolved settings for one staging attempt. The limits and +// timeouts are fields rather than constants read at the point of use so tests +// can drive the guards without a 256 MB fixture or a 90 second wait. +type stager struct { + fetch assetFetcher + release *otameta.Release + chmod func(string, os.FileMode) error + stagingRoot string + goarch string + binaryName string + goos string + current string + platformID string + maxFileBytes int64 + maxInflatedBytes int64 + stallTimeout time.Duration + probeTimeout time.Duration +} + +// stagingRootFor returns where staged versions live for a data directory. +func stagingRootFor(dataDir string) string { + dir := stateDirFor(dataDir) + if dir == "" { + return "" + } + return filepath.Join(dir, stagingSubdir) +} + +// Stage downloads a release, checks it against the signed manifest, pulls the +// binary out of it and proves that binary runs, without touching the live +// install. Every failure leaves the device exactly as it was. +func Stage(ctx context.Context, opts *StageOptions) (*StagedUpdate, error) { + // tlsroots hands back a transport this operation owns outright, so closing + // it at the end does not affect anything else in the process. + transport := tlsroots.Transport(nil) + transport.ResponseHeaderTimeout = responseHeaderTimeout + defer transport.CloseIdleConnections() + + return stageRelease(ctx, opts, assetFetcherFor(transport)) +} + +// stageRelease is Stage with the transfer injected, so tests can serve an +// archive without a network. +func stageRelease(ctx context.Context, opts *StageOptions, fetch assetFetcher) (*StagedUpdate, error) { + s, err := newStager(opts, fetch) + if err != nil { + return nil, err + } + return s.run(ctx) +} + +func newStager(opts *StageOptions, fetch assetFetcher) (*stager, error) { + if opts == nil { + return nil, errors.New("staging an update needs options") + } + if opts.Release == nil { + return nil, errors.New("staging an update needs a release") + } + if opts.PlatformID == "" { + return nil, errors.New("staging an update needs a platform id") + } + if opts.CurrentVersion == "" { + return nil, errors.New("staging an update needs the running version") + } + if opts.StagingRoot == "" { + return nil, errors.New("staging an update needs a staging directory") + } + if fetch == nil { + return nil, errors.New("staging an update needs a way to fetch the archive") + } + + // The base name is both the member to pull out and the name to write, so a + // target path that does not name a file has nothing to stage. + binaryName := filepath.Base(opts.TargetPath) + if opts.TargetPath == "" || binaryName == "." || binaryName == string(filepath.Separator) { + return nil, fmt.Errorf("staging an update needs the path of the binary to replace, got %q", opts.TargetPath) + } + + s := &stager{ + fetch: fetch, + release: opts.Release, + platformID: opts.PlatformID, + goos: opts.OS, + goarch: opts.Arch, + binaryName: binaryName, + stagingRoot: opts.StagingRoot, + current: opts.CurrentVersion, + maxFileBytes: maxStagedFileBytes, + maxInflatedBytes: maxArchiveInflatedBytes, + stallTimeout: downloadStallTimeout, + probeTimeout: probeTimeout, + chmod: os.Chmod, + } + if s.goos == "" { + s.goos = runtime.GOOS + } + if s.goarch == "" { + s.goarch = runtime.GOARCH + } + return s, nil +} + +func (s *stager) run(ctx context.Context) (*StagedUpdate, error) { + asset, version, err := s.selectArchive() + if err != nil { + return nil, err + } + + // The version has already been through semver parsing, which admits only + // digits, dots, hyphens and alphanumerics, so it cannot name anything but a + // single directory. Asserted rather than argued about, because it is the one + // string out of the manifest that becomes a path. + if version == "." || version == ".." || version != filepath.Base(version) { + return nil, fmt.Errorf("%w: %q cannot name a staging directory", ErrArchiveRejected, version) + } + dir := filepath.Join(s.stagingRoot, version) + + // A previous attempt that died without cleaning up would otherwise collide + // with this one's exclusive file creation. + if rmErr := removeStagingDir(ctx, dir); rmErr != nil { + return nil, fmt.Errorf("clearing previous update staging directory: %w", rmErr) + } + pruneStagingRoot(ctx, s.stagingRoot, version) + //nolint:gosec // G703: the version is asserted above to be a single path element + if mkErr := os.MkdirAll(dir, stateDirPerm); mkErr != nil { + return nil, fmt.Errorf("creating update staging directory: %w", mkErr) + } + + staged, err := s.stageInto(ctx, dir, asset, version) + if err != nil { + // Nothing outside this directory has been written, so discarding it + // leaves no trace of the attempt. + if rmErr := removeStagingDir(ctx, dir); rmErr != nil { + log.Warn().Err(rmErr).Str("dir", dir).Msg("could not remove failed update staging directory") + } + return nil, err + } + + log.Info(). + Str("version", staged.Version). + Str("binary", staged.BinaryPath). + Msg("staged update is verified and runnable") + return staged, nil +} + +// pruneStagingRoot deletes every staged version except the one being staged now. +// +// The failure path below removes this attempt's own directory, but it only runs +// when the process lives long enough to reach it. A power cut or a kill during a +// download leaves a directory that nothing afterwards is looking for: the next +// release computes a different name, so without this the whole ~60 MB of a +// half-staged version stays on the SD card for the life of the device, next to +// the config and the media database. Sweeping on the way in rather than on the +// way out means an orphan is collected by the next attempt whatever killed the +// last one. +// +// Failures are logged and not returned. Being unable to tidy up is not a reason +// to refuse an update. +func pruneStagingRoot(ctx context.Context, root, keep string) { + entries, err := os.ReadDir(root) + if err != nil { + if !errors.Is(err, os.ErrNotExist) { + log.Warn().Err(err).Str("dir", root).Msg("could not read the update staging directory") + } + return + } + + for _, entry := range entries { + if entry.Name() == keep { + continue + } + stale := filepath.Join(root, entry.Name()) + if rmErr := removeStagingDir(ctx, stale); rmErr != nil { + log.Warn().Err(rmErr).Str("dir", stale).Msg("could not remove an orphaned update staging directory") + continue + } + log.Info().Str("dir", stale).Msg("removed an orphaned update staging directory") + } +} + +// removeStagingDir deletes a staging directory, retrying for a bounded time. +// +// See stagingRemoveAttempts for why one attempt is not enough. Sleeping only +// happens when a removal has actually failed, so the ordinary path is a single +// call and never consults the context. Once it is retrying, a cancelled context +// ends it: two seconds of sharing-violation retries is not worth holding up a +// shutdown for, and the directory left behind is collected by the next +// attempt's sweep. +func removeStagingDir(ctx context.Context, dir string) error { + var err error + for attempt := range stagingRemoveAttempts { + if attempt > 0 { + select { + case <-ctx.Done(): + return fmt.Errorf("removing update staging directory %q: %w", dir, ctx.Err()) + case <-time.After(stagingRemoveDelay): + } + } + //nolint:gosec // G703: callers pass a path this package built under its own staging root + if err = os.RemoveAll(dir); err == nil { + return nil + } + } + return fmt.Errorf("removing update staging directory %q: %w", dir, err) +} + +func (s *stager) stageInto( + ctx context.Context, dir string, asset *otameta.Asset, version string, +) (*StagedUpdate, error) { + ext, err := otameta.ArchiveExtension(asset.Name) + if err != nil { + return nil, fmt.Errorf("%w: %w", ErrArchiveRejected, err) + } + wantDigest, err := assetDigest(asset) + if err != nil { + return nil, err + } + + // The archive is written under a name built from the version and extension + // rather than the one the manifest gives it, so no metadata string reaches + // the filesystem even though selection has already constrained it. + archivePath := filepath.Join(dir, otameta.ArchiveBaseName(s.platformID, s.goarch, version)+ext) + if err := s.downloadArchive(ctx, asset, archivePath); err != nil { + return nil, err + } + + payloadDir := filepath.Join(dir, payloadSubdir) + //nolint:gosec // G703: dir is the staging directory this package created, payloadSubdir is a constant + if err := os.MkdirAll(payloadDir, stateDirPerm); err != nil { + return nil, fmt.Errorf("creating update payload directory: %w", err) + } + + binaryPath := filepath.Join(payloadDir, s.binaryName) + if err := s.extractBinary(ctx, archivePath, ext, wantDigest, binaryPath); err != nil { + return nil, err + } + + // Extraction is a long uninterruptible stretch on a slow device, so the probe + // is only meaningful if the caller is still interested in the answer. Without + // this, a shutdown part-way through staging would run the probe on a dead + // context and report a perfectly good build as unrunnable. + if err := ctx.Err(); err != nil { + return nil, fmt.Errorf("staging the update was cancelled: %w", err) + } + + // The exec bit is set here rather than taken from the archive, so an archive + // cannot decide what is executable. + // + // A failure here is not fatal, because on the volumes MiSTer and MiSTeX + // install to it does not mean what it looks like it means. /media/fat is vfat + // or exFAT, which has no mode bits: the exec bit comes from the mount's mask, + // and chmod is either a silent no-op or an outright error depending on which + // driver mounted it. In the error case the file is already executable and + // refusing here would reject a release that runs perfectly well. The probe + // below is what actually decides whether this binary can execute, so let it, + // and record that we could not set the bit ourselves in case the probe then + // fails for a reason that needs this context. + //nolint:gosec // an executable has to be executable; the archive does not get a say in it + if err := s.chmod(binaryPath, stagedBinaryPerm); err != nil { + log.Warn().Err(err). + Str("binary", binaryPath). + Msg("could not set the exec bit on the staged binary; leaving it to the probe") + } + + if err := s.probeBinary(ctx, binaryPath, version); err != nil { + return nil, err + } + + return &StagedUpdate{ + Dir: dir, + BinaryPath: binaryPath, + ArchivePath: archivePath, + Version: version, + }, nil +} + +// selectArchive picks the one archive this device may install from the release +// and refuses anything that is not a step forward. The version assertions are +// made here, against the release's own tag, because the check that offered the +// update is a separate operation whose answer this one does not take on trust. +func (s *stager) selectArchive() (*otameta.Asset, string, error) { + if s.release.Draft { + return nil, "", fmt.Errorf("%w: %s is a draft", ErrArchiveRejected, s.release.TagName) + } + + version := otameta.VersionFromTag(s.release.TagName) + target, err := semver.NewVersion(version) + if err != nil { + return nil, "", fmt.Errorf("%w: release %q has no usable version: %w", + ErrArchiveRejected, s.release.TagName, err) + } + current, err := semver.NewVersion(s.current) + if err != nil { + return nil, "", fmt.Errorf("reading the running version %q: %w", s.current, err) + } + + if !target.GreaterThan(current) { + return nil, "", fmt.Errorf("%w: %s is not newer than %s", ErrNotAnUpgrade, version, s.current) + } + + if s.release.MinUpgradeFrom != "" { + floor, floorErr := semver.NewVersion(s.release.MinUpgradeFrom) + if floorErr != nil { + return nil, "", fmt.Errorf("%w: %s declares an unusable min_upgrade_from %q: %w", + ErrArchiveRejected, s.release.TagName, s.release.MinUpgradeFrom, floorErr) + } + if current.LessThan(floor) { + return nil, "", fmt.Errorf("%w: %s needs %s or newer first, running %s", + ErrUpgradeFloor, version, s.release.MinUpgradeFrom, s.current) + } + } + + asset, err := otameta.SelectAsset(s.release, s.platformID, s.goarch) + if err != nil { + return nil, "", fmt.Errorf("selecting the update archive: %w", err) + } + return asset, version, nil +} + +// downloadArchive streams the archive to disk, hashing as it goes, and accepts +// it only if both the length and the digest are exactly what the signed +// manifest says. The body is never read into memory. +func (s *stager) downloadArchive(ctx context.Context, asset *otameta.Asset, dest string) error { + if asset.Size <= 0 { + return fmt.Errorf("%w: the manifest declares no size for %s", ErrArchiveRejected, asset.Name) + } + if asset.Size > maxArchiveBytes { + return fmt.Errorf("%w: the manifest declares %d bytes for %s, over the %d byte limit", + ErrArchiveRejected, asset.Size, asset.Name, maxArchiveBytes) + } + want, err := assetDigest(asset) + if err != nil { + return err + } + + stallCtx, guard := newStallGuard(ctx, s.stallTimeout) + defer guard.stop() + + body, err := s.fetch(stallCtx, asset.URL) + if err != nil { + // Classified the same way a mid-body failure is. Before the first byte the + // transport usually decides first: the dial, the TLS handshake and the + // response headers each carry a deadline of their own, and all three are + // shorter than the stall timeout, so classifyTransferErr maps a timeout + // from any of them onto the same stall verdict. The guard covers the case + // none of those deadlines can see, where every hop of a redirect chain + // answers inside its own budget and the transfer as a whole still goes + // nowhere. + return s.classifyTransferErr(ctx, guard, err, 0) + } + defer closeQuietly(body, "update archive response") + + //nolint:gosec // the path is built by this package inside its own staging directory + f, err := os.OpenFile(dest, os.O_WRONLY|os.O_CREATE|os.O_EXCL, stagedFilePerm) + if err != nil { + return fmt.Errorf("creating the update archive file: %w", err) + } + + // One byte past the declared size, so an archive of exactly that length is + // accepted and a longer one is detected rather than silently truncated into + // something that would then fail the digest for the wrong reason. + digest := sha256.New() + written, copyErr := io.Copy(io.MultiWriter(f, digest), guard.reader(io.LimitReader(body, asset.Size+1))) + syncErr := f.Sync() + closeErr := f.Close() + + switch { + case copyErr != nil: + return s.classifyTransferErr(ctx, guard, copyErr, written) + case syncErr != nil: + return fmt.Errorf("flushing the update archive to disk: %w", syncErr) + case closeErr != nil: + return fmt.Errorf("closing the update archive: %w", closeErr) + } + + if written != asset.Size { + return fmt.Errorf("%w: %s is %d bytes, the manifest declares %d", + ErrArchiveRejected, asset.Name, written, asset.Size) + } + if subtle.ConstantTimeCompare(digest.Sum(nil), want) != 1 { + return fmt.Errorf("%w: %s hashes to %s, the manifest declares %s", + ErrChecksumMismatch, asset.Name, hex.EncodeToString(digest.Sum(nil)), asset.SHA256) + } + + log.Debug().Str("archive", asset.Name).Int64("bytes", written).Msg("update archive verified") + return nil +} + +// assetDigest decodes the digest the signed manifest gives for an asset. Both +// the download and the read that feeds extraction check against it, so it is +// decoded in one place. +func assetDigest(asset *otameta.Asset) ([]byte, error) { + want, err := hex.DecodeString(asset.SHA256) + if err != nil || len(want) != sha256.Size { + return nil, fmt.Errorf("%w: the manifest has no usable sha256 for %s", ErrArchiveRejected, asset.Name) + } + return want, nil +} + +// classifyTransferErr tells the three ways a transfer can fail apart. Both the +// request and the body read cancel through the same guarded context, so the +// error either side hands back says only "cancelled" and the reason has to come +// from which of the two cancelled it. Caller intent is checked first: a caller +// who gave up mid-stall is not reporting a network fault. +func (s *stager) classifyTransferErr( + ctx context.Context, guard *stallGuard, err error, written int64, +) error { + var netErr net.Error + switch { + case ctx.Err() != nil: + return fmt.Errorf("update archive download was cancelled after %d bytes: %w", written, ctx.Err()) + case guard.tripped(): + return fmt.Errorf("%w after %d bytes with no progress for %s", + ErrDownloadStalled, written, s.stallTimeout) + case errors.As(err, &netErr) && netErr.Timeout(): + // A deadline the transport owns rather than one the guard watches: the + // connect, the handshake or the response headers gave up. Silence the + // guard never gets to see, but silence all the same, and dead versus slow + // is the distinction this sentinel exists to draw. + return fmt.Errorf("%w after %d bytes: %w", ErrDownloadStalled, written, err) + default: + return fmt.Errorf("downloading the update archive: %w", err) + } +} + +// probeBinary runs the staged binary's version flag. It is the load-bearing +// check for the platforms with no supervisor: the binary has to demonstrably +// execute here, and agree about what it is, before anything replaces the one +// that is currently working. It catches a wrong architecture, a libc mismatch, +// a missing shared library, an exec bit a vfat mount dropped, a noexec mount, +// and a version that disagrees with the manifest. +func (s *stager) probeBinary(ctx context.Context, binaryPath, version string) error { + probeCtx, cancel := context.WithTimeout(ctx, s.probeTimeout) + defer cancel() + + //nolint:gosec // the path is a file this process just created inside its own staging directory + cmd := exec.CommandContext(probeCtx, binaryPath, "-"+config.VersionFlagName) + var stdout, stderr cappedBuilder + cmd.Stdout = &stdout + cmd.Stderr = &stderr + // Killing the process is not enough to unblock Wait if it left a child + // holding these pipes. This runs a binary that was downloaded seconds ago, + // so the timeout has to bound the call and not just the process. + cmd.WaitDelay = probeWaitDelay + + runErr := cmd.Run() + if runErr != nil { + switch { + case ctx.Err() != nil: + // The caller gave up. Reporting that as a failed probe would + // condemn a build that was never actually judged. + return fmt.Errorf("update probe was cancelled: %w", ctx.Err()) + case errors.Is(probeCtx.Err(), context.DeadlineExceeded): + return fmt.Errorf("%w: no answer within %s", ErrProbeFailed, s.probeTimeout) + default: + return fmt.Errorf("%w: %w (stderr: %s)", ErrProbeFailed, runErr, clip(stderr.String(), 256)) + } + } + + // One matching line, not the whole stream: the comparison is made by the + // build that is already installed against one that did not exist when it + // shipped, so a future release that prints something extra alongside its + // version must not be judged unrunnable for it. + want := config.VersionLine(version, s.platformID) + if !hasLine(stdout.String(), want) { + return fmt.Errorf("%w: printed %q, expected a line reading %q", + ErrProbeFailed, clip(stdout.String(), 256), want) + } + + log.Debug().Str("binary", binaryPath).Msg("staged binary answered its version probe") + return nil +} + +// assetFetcherFor returns a fetcher backed by an HTTP transport. +func assetFetcherFor(transport *http.Transport) assetFetcher { + return func(ctx context.Context, target string) (io.ReadCloser, error) { + // No client deadline: the archive is the one response whose size is not + // bounded by a small constant, so total duration is the caller's context + // to bound and the stall guard is what tells slow apart from dead. + client := &http.Client{Transport: transport} + req, err := http.NewRequestWithContext(ctx, http.MethodGet, target, http.NoBody) + if err != nil { + return nil, fmt.Errorf("creating the update archive request: %w", err) + } + + res, err := client.Do(req) //nolint:bodyclose // the body is the return value; the caller closes it + if err != nil { + return nil, fmt.Errorf("requesting the update archive: %w", err) + } + if res.StatusCode != http.StatusOK { + closeQuietly(res.Body, "update archive response") + return nil, fmt.Errorf("the update archive request failed with status %d", res.StatusCode) + } + return res.Body, nil + } +} + +func closeQuietly(c io.Closer, what string) { + if err := c.Close(); err != nil { + log.Debug().Err(err).Msgf("closing %s", what) + } +} + +// hasLine reports whether any line of out is exactly want. A trailing carriage +// return is ignored so output that has been through a CRLF channel still +// matches. +func hasLine(out, want string) bool { + for line := range strings.SplitSeq(out, "\n") { + if strings.TrimSuffix(line, "\r") == want { + return true + } + } + return false +} + +// clip shortens a string for an error message. +func clip(s string, limit int) string { + trimmed := strings.TrimSpace(s) + if len(trimmed) <= limit { + return trimmed + } + return trimmed[:limit] + "…" +} diff --git a/pkg/service/updater/stage_test.go b/pkg/service/updater/stage_test.go new file mode 100644 index 000000000..43d9b47b7 --- /dev/null +++ b/pkg/service/updater/stage_test.go @@ -0,0 +1,1199 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package updater + +import ( + "bytes" + "context" + "crypto/sha256" + "encoding/hex" + "errors" + "fmt" + "io" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "runtime" + "strconv" + "strings" + "testing" + "time" + + "github.com/ZaparooProject/zaparoo-core/v2/pkg/config" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/updater/otameta" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + // testStageVersion is compiled into the fake binary, so every test that + // reaches the probe stages this version. + testStageVersion = "2.11.0" + testStagePlatform = "linux" + testStageArch = "amd64" + testCurrentVersion = "2.10.1" +) + +// fakeBinaryTemplate is a stand-in release binary. It answers the version probe +// the way the real one does, and picks a way to misbehave from its own file +// name, which is the only channel available: the probe inherits this process's +// environment and staging deletes anything placed alongside the binary, so +// neither an env var nor a sidecar file can select a behaviour per test. +const fakeBinaryTemplate = `package main + +import ( + "os" + "path/filepath" + "strings" + "time" +) + +func main() { + switch strings.ToLower(strings.TrimSuffix(filepath.Base(os.Args[0]), ".exe")) { + case "zaparoo-fail": + os.Stderr.WriteString("error while loading shared libraries: libz.so.1\n") + os.Exit(1) + case "zaparoo-wrong": + os.Stdout.WriteString(__WRONG__) + case "zaparoo-chatty": + os.Stderr.WriteString("warning: config file not found, using defaults\n") + os.Stdout.WriteString("some future build says something here first\n") + os.Stdout.WriteString(__GOOD__) + case "zaparoo-hang": + time.Sleep(10 * time.Minute) + default: + os.Stdout.WriteString(__GOOD__) + } +} +` + +var ( + fakeBinaryPath string + errFakeBinary error +) + +func TestMain(m *testing.M) { + dir, err := os.MkdirTemp("", "zaparoo-updater-fake") + if err == nil { + defer func() { _ = os.RemoveAll(dir) }() + fakeBinaryPath, errFakeBinary = buildFakeBinary(dir) + } else { + errFakeBinary = err + } + m.Run() +} + +// buildFakeBinary compiles the stand-in release binary once for the package. +// Nothing else can honestly test the probe: it has to be a real executable, +// and the failures worth catching are a process that will not start, one that +// answers with the wrong version, and one that never answers. +func buildFakeBinary(dir string) (string, error) { + // Built from config.VersionLine rather than a literal of its own, so this + // fixture cannot pass while a real release binary would fail the probe. + source := strings.NewReplacer( + "__GOOD__", strconv.Quote(config.VersionLine(testStageVersion, testStagePlatform)+"\n"), + "__WRONG__", strconv.Quote(config.VersionLine("0.0.1", testStagePlatform)+"\n"), + ).Replace(fakeBinaryTemplate) + + if err := os.WriteFile(filepath.Join(dir, "main.go"), []byte(source), 0o600); err != nil { + return "", fmt.Errorf("writing the fake binary source: %w", err) + } + gomod := "module zaparoofake\n\ngo 1.21\n" + if err := os.WriteFile(filepath.Join(dir, "go.mod"), []byte(gomod), 0o600); err != nil { + return "", fmt.Errorf("writing the fake binary go.mod: %w", err) + } + + out := filepath.Join(dir, testBinaryName("fake")) + //nolint:gosec // a fixed command compiling a source file this test just wrote + cmd := exec.CommandContext(context.Background(), "go", "build", "-o", out, ".") + cmd.Dir = dir + cmd.Env = append(os.Environ(), "GOWORK=off", "GOFLAGS=", "CGO_ENABLED=0") + if combined, buildErr := cmd.CombinedOutput(); buildErr != nil { + return "", fmt.Errorf("building the fake binary: %w (%s)", buildErr, combined) + } + return out, nil +} + +// testBinaryName adds the extension Windows needs. Without it exec cannot find +// the staged file at all, because Windows resolves even an absolute path +// through PATHEXT. +func testBinaryName(stem string) string { + if runtime.GOOS == "windows" { + return stem + ".exe" + } + return stem +} + +func fakeBinaryBytes(t *testing.T) []byte { + t.Helper() + + require.NoError(t, errFakeBinary, "the fake release binary did not build") + body, err := os.ReadFile(fakeBinaryPath) //nolint:gosec // built by this package into its own temp dir + require.NoError(t, err) + return body +} + +// releaseArchive builds an archive shaped like a real release: the binary plus +// the licence and readme that must not be extracted. +func releaseArchive(t *testing.T, ext, memberName string, binary []byte) []byte { + t.Helper() + + path := filepath.Join(t.TempDir(), "release"+ext) + switch ext { + case otameta.ArchiveExtTarGz: + writeTarGz(t, path, []tarMember{ + {name: "LICENSE.txt", body: []byte("gpl")}, + {name: "README.txt", body: []byte("readme")}, + {name: memberName, body: binary, mode: 0o755}, + }) + case otameta.ArchiveExtZip: + writeZip(t, path, []zipMember{ + {name: "LICENSE.txt", body: []byte("gpl")}, + {name: "README.txt", body: []byte("readme")}, + {name: memberName, body: binary, mode: 0o755}, + }) + default: + t.Fatalf("unsupported archive extension %q", ext) + } + + body, err := os.ReadFile(path) //nolint:gosec // test path under t.TempDir + require.NoError(t, err) + return body +} + +// testArchiveName is the only name a release may publish an archive under for +// this platform and version, which is what binds the archive to the version. +func testArchiveName(version, ext string) string { + return otameta.ArchiveBaseName(testStagePlatform, testStageArch, version) + ext +} + +// servedAsset publishes body over HTTP and describes it exactly as a verified +// manifest would. +func servedAsset(t *testing.T, name string, body []byte) *otameta.Asset { + t.Helper() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Length", strconv.Itoa(len(body))) + _, _ = w.Write(body) + })) + t.Cleanup(srv.Close) + + sum := sha256.Sum256(body) + return &otameta.Asset{ + Name: name, + URL: srv.URL + "/" + name, + SHA256: hex.EncodeToString(sum[:]), + Size: int64(len(body)), + } +} + +func testRelease(tag string, assets ...*otameta.Asset) *otameta.Release { + return &otameta.Release{ + Name: tag, + TagName: tag, + Channel: otameta.ChannelStable, + Assets: assets, + Rollout: 100, + } +} + +func testAssetFetcher(t *testing.T) assetFetcher { + t.Helper() + + transport := &http.Transport{} + t.Cleanup(transport.CloseIdleConnections) + return assetFetcherFor(transport) +} + +// unusedFetcher fails loudly, for the stages that must decide before any bytes +// are requested. +func unusedFetcher(context.Context, string) (io.ReadCloser, error) { + return nil, errors.New("the archive should not have been fetched") +} + +func testStageOptions(t *testing.T, rel *otameta.Release, stem string) *StageOptions { + t.Helper() + + root := t.TempDir() + return &StageOptions{ + Release: rel, + PlatformID: testStagePlatform, + Arch: testStageArch, + // Pinned rather than taken from the host, so one set of fixtures covers + // the archive member rules whatever this test is running on. + OS: "linux", + TargetPath: filepath.Join(root, "install", testBinaryName(stem)), + StagingRoot: filepath.Join(root, "updater", stagingSubdir), + CurrentVersion: testCurrentVersion, + } +} + +func TestNewStager_RequiresItsInputs(t *testing.T) { + t.Parallel() + + valid := &StageOptions{ + Release: testRelease("v2.11.0"), + PlatformID: testStagePlatform, + TargetPath: filepath.Join("install", "zaparoo"), + StagingRoot: filepath.Join("data", "updater", "staging"), + CurrentVersion: testCurrentVersion, + } + + tests := []struct { + mutate func(*StageOptions) + name string + wantMsg string + noFetch bool + }{ + { + name: "no release", + mutate: func(o *StageOptions) { o.Release = nil }, + wantMsg: "needs a release", + }, + { + name: "no platform", + mutate: func(o *StageOptions) { o.PlatformID = "" }, + wantMsg: "needs a platform id", + }, + { + name: "no running version", + mutate: func(o *StageOptions) { o.CurrentVersion = "" }, + wantMsg: "needs the running version", + }, + { + name: "no staging root", + mutate: func(o *StageOptions) { o.StagingRoot = "" }, + wantMsg: "needs a staging directory", + }, + { + name: "no target path", + mutate: func(o *StageOptions) { o.TargetPath = "" }, + wantMsg: "needs the path of the binary to replace", + }, + { + name: "target path is a directory", + mutate: func(o *StageOptions) { o.TargetPath = "." }, + wantMsg: "needs the path of the binary to replace", + }, + { + name: "no fetcher", + mutate: func(*StageOptions) {}, + noFetch: true, + wantMsg: "needs a way to fetch the archive", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + opts := *valid + tt.mutate(&opts) + + fetch := assetFetcher(unusedFetcher) + if tt.noFetch { + fetch = nil + } + + s, err := newStager(&opts, fetch) + require.Error(t, err) + assert.Nil(t, s) + assert.Contains(t, err.Error(), tt.wantMsg) + }) + } +} + +func TestNewStager_DefaultsToThisBuild(t *testing.T) { + t.Parallel() + + s, err := newStager(&StageOptions{ + Release: testRelease("v2.11.0"), + PlatformID: testStagePlatform, + TargetPath: filepath.Join("install", "zaparoo"), + StagingRoot: filepath.Join("data", "updater", "staging"), + CurrentVersion: testCurrentVersion, + }, unusedFetcher) + + require.NoError(t, err) + assert.Equal(t, runtime.GOOS, s.goos) + assert.Equal(t, runtime.GOARCH, s.goarch) + assert.Equal(t, "zaparoo", s.binaryName) +} + +func TestStage_ValidatesBeforeTouchingTheNetwork(t *testing.T) { + t.Parallel() + + staged, err := Stage(context.Background(), &StageOptions{}) + require.Error(t, err) + assert.Nil(t, staged) +} + +// TestSelectArchive covers the assertions that make a stale or tampered +// manifest unable to install something older than what is running. They are +// made here against the release's own tag rather than trusted from whatever +// offered the update. +func TestSelectArchive(t *testing.T) { + t.Parallel() + + asset := func(version string) *otameta.Asset { + return &otameta.Asset{Name: testArchiveName(version, otameta.ArchiveExtTarGz), Size: 1024} + } + + tests := []struct { + wantErr error + build func() *otameta.Release + name string + current string + wantMsg string + wantVersion string + }{ + { + name: "newer release", + build: func() *otameta.Release { + return testRelease("v2.11.0", asset("2.11.0")) + }, + current: "2.10.1", + wantVersion: "2.11.0", + }, + { + name: "draft", + build: func() *otameta.Release { + rel := testRelease("v2.11.0", asset("2.11.0")) + rel.Draft = true + return rel + }, + current: "2.10.1", + wantErr: ErrArchiveRejected, + wantMsg: "is a draft", + }, + { + name: "same version", + build: func() *otameta.Release { + return testRelease("v2.10.1", asset("2.10.1")) + }, + current: "2.10.1", + wantErr: ErrNotAnUpgrade, + }, + { + name: "older version", + build: func() *otameta.Release { + return testRelease("v2.9.0", asset("2.9.0")) + }, + current: "2.10.1", + wantErr: ErrNotAnUpgrade, + }, + { + name: "prerelease of the running version is not an upgrade", + build: func() *otameta.Release { + return testRelease("v2.10.1-beta.1", asset("2.10.1-beta.1")) + }, + current: "2.10.1", + wantErr: ErrNotAnUpgrade, + }, + { + name: "unusable release version", + build: func() *otameta.Release { + return testRelease("vnot-a-version", asset("not-a-version")) + }, + current: "2.10.1", + wantErr: ErrArchiveRejected, + wantMsg: "no usable version", + }, + { + name: "unusable running version", + build: func() *otameta.Release { + return testRelease("v2.11.0", asset("2.11.0")) + }, + current: "not-a-version", + wantMsg: "reading the running version", + }, + { + name: "upgrade floor above the running version", + build: func() *otameta.Release { + rel := testRelease("v2.11.0", asset("2.11.0")) + rel.MinUpgradeFrom = "2.10.2" + return rel + }, + current: "2.10.1", + wantErr: ErrUpgradeFloor, + wantMsg: "needs 2.10.2 or newer first", + }, + { + name: "upgrade floor equal to the running version", + build: func() *otameta.Release { + rel := testRelease("v2.11.0", asset("2.11.0")) + rel.MinUpgradeFrom = "2.10.1" + return rel + }, + current: "2.10.1", + wantVersion: "2.11.0", + }, + { + name: "upgrade floor below the running version", + build: func() *otameta.Release { + rel := testRelease("v2.11.0", asset("2.11.0")) + rel.MinUpgradeFrom = "2.6.0" + return rel + }, + current: "2.10.1", + wantVersion: "2.11.0", + }, + { + name: "unusable upgrade floor", + build: func() *otameta.Release { + rel := testRelease("v2.11.0", asset("2.11.0")) + rel.MinUpgradeFrom = "sometime" + return rel + }, + current: "2.10.1", + wantErr: ErrArchiveRejected, + wantMsg: "unusable min_upgrade_from", + }, + // A manifest claiming a high version while carrying a genuine older + // archive selects nothing: the candidate name is built from the tag, so + // relabelling the release stops it matching its own assets. + { + name: "relabelled release does not match its own archives", + build: func() *otameta.Release { + return testRelease("v99.0.0", asset("2.10.1")) + }, + current: "2.10.1", + wantErr: otameta.ErrNoAsset, + }, + { + name: "no archive for this platform", + build: func() *otameta.Release { + return testRelease("v2.11.0", &otameta.Asset{ + Name: otameta.ArchiveBaseName("windows", "amd64", "2.11.0") + otameta.ArchiveExtZip, + }) + }, + current: "2.10.1", + wantErr: otameta.ErrNoAsset, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + opts := testStageOptions(t, tt.build(), "zaparoo") + opts.CurrentVersion = tt.current + s, err := newStager(opts, unusedFetcher) + require.NoError(t, err) + + got, version, err := s.selectArchive() + if tt.wantVersion != "" { + require.NoError(t, err) + require.NotNil(t, got) + assert.Equal(t, tt.wantVersion, version) + assert.Equal(t, testArchiveName(tt.wantVersion, otameta.ArchiveExtTarGz), got.Name) + return + } + + require.Error(t, err) + assert.Empty(t, version) + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr) + } + if tt.wantMsg != "" { + assert.Contains(t, err.Error(), tt.wantMsg) + } + }) + } +} + +func TestSelectArchive_ArmDoesNotPickUpArm64(t *testing.T) { + t.Parallel() + + rel := testRelease("v2.11.0", &otameta.Asset{ + Name: otameta.ArchiveBaseName("mister", "arm64", "2.11.0") + otameta.ArchiveExtZip, + }) + opts := testStageOptions(t, rel, "zaparoo.sh") + opts.PlatformID = "mister" + opts.Arch = "arm" + + s, err := newStager(opts, unusedFetcher) + require.NoError(t, err) + + _, _, err = s.selectArchive() + require.ErrorIs(t, err, otameta.ErrNoAsset) +} + +func TestDownloadArchive(t *testing.T) { + t.Parallel() + + body := bytes.Repeat([]byte("zaparoo release archive"), 64) + sum := sha256.Sum256(body) + + tests := []struct { + wantErr error + mutate func(*otameta.Asset) + name string + wantMsg string + }{ + { + name: "accepts what the manifest describes", + mutate: func(*otameta.Asset) {}, + }, + { + name: "no declared size", + mutate: func(a *otameta.Asset) { a.Size = 0 }, + wantErr: ErrArchiveRejected, + wantMsg: "declares no size", + }, + { + name: "negative declared size", + mutate: func(a *otameta.Asset) { a.Size = -1 }, + wantErr: ErrArchiveRejected, + wantMsg: "declares no size", + }, + { + name: "declared size over the cap", + mutate: func(a *otameta.Asset) { a.Size = maxArchiveBytes + 1 }, + wantErr: ErrArchiveRejected, + wantMsg: "over the", + }, + { + name: "unusable digest", + mutate: func(a *otameta.Asset) { a.SHA256 = "not hex" }, + wantErr: ErrArchiveRejected, + wantMsg: "no usable sha256", + }, + { + name: "digest of the wrong length", + mutate: func(a *otameta.Asset) { a.SHA256 = hex.EncodeToString(sum[:16]) }, + wantErr: ErrArchiveRejected, + wantMsg: "no usable sha256", + }, + { + name: "digest does not match the bytes", + mutate: func(a *otameta.Asset) { a.SHA256 = strings.Repeat("ab", sha256.Size) }, + wantErr: ErrChecksumMismatch, + }, + { + name: "shorter than declared", + mutate: func(a *otameta.Asset) { a.Size = int64(len(body)) + 10 }, + wantErr: ErrArchiveRejected, + wantMsg: "the manifest declares", + }, + { + name: "longer than declared", + mutate: func(a *otameta.Asset) { a.Size = int64(len(body)) - 10 }, + wantErr: ErrArchiveRejected, + wantMsg: "the manifest declares", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + asset := servedAsset(t, testArchiveName(testStageVersion, otameta.ArchiveExtTarGz), body) + tt.mutate(asset) + + opts := testStageOptions(t, testRelease("v"+testStageVersion, asset), "zaparoo") + s, err := newStager(opts, testAssetFetcher(t)) + require.NoError(t, err) + + dest := filepath.Join(t.TempDir(), asset.Name) + err = s.downloadArchive(context.Background(), asset, dest) + + if tt.wantErr == nil && tt.wantMsg == "" { + require.NoError(t, err) + got, readErr := os.ReadFile(dest) //nolint:gosec // test path under t.TempDir + require.NoError(t, readErr) + assert.Equal(t, body, got) + return + } + + require.Error(t, err) + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr) + } + if tt.wantMsg != "" { + assert.Contains(t, err.Error(), tt.wantMsg) + } + }) + } +} + +func TestDownloadArchive_ServerError(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + })) + t.Cleanup(srv.Close) + + asset := &otameta.Asset{ + Name: testArchiveName(testStageVersion, otameta.ArchiveExtTarGz), + URL: srv.URL + "/missing", + SHA256: strings.Repeat("00", sha256.Size), + Size: 1024, + } + opts := testStageOptions(t, testRelease("v"+testStageVersion, asset), "zaparoo") + s, err := newStager(opts, testAssetFetcher(t)) + require.NoError(t, err) + + dest := filepath.Join(t.TempDir(), asset.Name) + err = s.downloadArchive(context.Background(), asset, dest) + require.Error(t, err) + assert.Contains(t, err.Error(), "404") + assert.NoFileExists(t, dest) +} + +// TestDownloadArchive_Stalls proves the guard bounds silence rather than total +// duration: the server answers, sends a little, and then never sends again. +func TestDownloadArchive_Stalls(t *testing.T) { + t.Parallel() + + body := bytes.Repeat([]byte("x"), 4096) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Length", strconv.Itoa(len(body))) + _, _ = w.Write(body[:8]) + if flusher, ok := w.(http.Flusher); ok { + flusher.Flush() + } + <-r.Context().Done() + })) + t.Cleanup(srv.Close) + + sum := sha256.Sum256(body) + asset := &otameta.Asset{ + Name: testArchiveName(testStageVersion, otameta.ArchiveExtTarGz), + URL: srv.URL + "/archive", + SHA256: hex.EncodeToString(sum[:]), + Size: int64(len(body)), + } + + opts := testStageOptions(t, testRelease("v"+testStageVersion, asset), "zaparoo") + s, err := newStager(opts, testAssetFetcher(t)) + require.NoError(t, err) + s.stallTimeout = 200 * time.Millisecond + + dest := filepath.Join(t.TempDir(), asset.Name) + err = s.downloadArchive(context.Background(), asset, dest) + require.ErrorIs(t, err, ErrDownloadStalled) + assert.Contains(t, err.Error(), "no progress") +} + +func TestDownloadArchive_CallerCancels(t *testing.T) { + t.Parallel() + + body := bytes.Repeat([]byte("x"), 4096) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Length", strconv.Itoa(len(body))) + _, _ = w.Write(body[:8]) + if flusher, ok := w.(http.Flusher); ok { + flusher.Flush() + } + <-r.Context().Done() + })) + t.Cleanup(srv.Close) + + sum := sha256.Sum256(body) + asset := &otameta.Asset{ + Name: testArchiveName(testStageVersion, otameta.ArchiveExtTarGz), + URL: srv.URL + "/archive", + SHA256: hex.EncodeToString(sum[:]), + Size: int64(len(body)), + } + + opts := testStageOptions(t, testRelease("v"+testStageVersion, asset), "zaparoo") + s, err := newStager(opts, testAssetFetcher(t)) + require.NoError(t, err) + + ctx, cancel := context.WithCancel(context.Background()) + go func() { + time.Sleep(100 * time.Millisecond) + cancel() + }() + defer cancel() + + dest := filepath.Join(t.TempDir(), asset.Name) + err = s.downloadArchive(ctx, asset, dest) + require.Error(t, err) + // A caller giving up is not a stall, and must not be reported as one. + require.NotErrorIs(t, err, ErrDownloadStalled) + assert.Contains(t, err.Error(), "cancelled") +} + +// TestDownloadArchive_StallsBeforeTheFirstByte covers the window the guard is +// running in but the body read is not: DNS, the dial, TLS and the response +// headers across every redirect hop. The stall timeout is shortened because the +// transport built for these tests carries no deadlines of its own, which leaves +// the guard as the only thing watching; production gets to the same verdict +// through the transport instead, and TestDownloadArchive_TransportDeadlineStalls +// pins that path at the shipped stall timeout. Either way a caller who has not +// given up must not be told the download was cancelled. +func TestDownloadArchive_StallsBeforeTheFirstByte(t *testing.T) { + t.Parallel() + + // The handler writes nothing, so net/http sends no response at all and the + // client blocks waiting for the headers. + srv := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + <-r.Context().Done() + })) + t.Cleanup(srv.Close) + + asset := &otameta.Asset{ + Name: testArchiveName(testStageVersion, otameta.ArchiveExtTarGz), + URL: srv.URL + "/archive", + SHA256: strings.Repeat("00", sha256.Size), + Size: 4096, + } + opts := testStageOptions(t, testRelease("v"+testStageVersion, asset), "zaparoo") + s, err := newStager(opts, testAssetFetcher(t)) + require.NoError(t, err) + s.stallTimeout = 200 * time.Millisecond + + dest := filepath.Join(t.TempDir(), asset.Name) + err = s.downloadArchive(context.Background(), asset, dest) + require.ErrorIs(t, err, ErrDownloadStalled) + assert.Contains(t, err.Error(), "after 0 bytes") + assert.NoFileExists(t, dest) +} + +// TestDownloadArchive_TransportDeadlineStalls is the same dead network as above +// with nothing about the stager weakened: the shipped stall timeout stands, and +// the deadline that fires is the one the transport owns for the response +// headers. A verdict has to come out of that too, or the sentinel would be +// unreachable for the ordinary case of a link that is up and a server that is +// gone. +func TestDownloadArchive_TransportDeadlineStalls(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + <-r.Context().Done() + })) + t.Cleanup(srv.Close) + + transport := &http.Transport{ResponseHeaderTimeout: 200 * time.Millisecond} + t.Cleanup(transport.CloseIdleConnections) + + asset := &otameta.Asset{ + Name: testArchiveName(testStageVersion, otameta.ArchiveExtTarGz), + URL: srv.URL + "/archive", + SHA256: strings.Repeat("00", sha256.Size), + Size: 4096, + } + opts := testStageOptions(t, testRelease("v"+testStageVersion, asset), "zaparoo") + s, err := newStager(opts, assetFetcherFor(transport)) + require.NoError(t, err) + require.Equal(t, downloadStallTimeout, s.stallTimeout) + + dest := filepath.Join(t.TempDir(), asset.Name) + err = s.downloadArchive(context.Background(), asset, dest) + require.ErrorIs(t, err, ErrDownloadStalled) + // Not the release's fault, so not a verdict on it. + require.NotErrorIs(t, err, ErrArchiveRejected) + assert.Contains(t, err.Error(), "after 0 bytes") + assert.NoFileExists(t, dest) +} + +func TestStageRelease_TarGz(t *testing.T) { + t.Parallel() + + assertStagesCleanly(t, otameta.ArchiveExtTarGz) +} + +func TestStageRelease_Zip(t *testing.T) { + t.Parallel() + + assertStagesCleanly(t, otameta.ArchiveExtZip) +} + +func assertStagesCleanly(t *testing.T, ext string) { + t.Helper() + + binary := fakeBinaryBytes(t) + name := testArchiveName(testStageVersion, ext) + asset := servedAsset(t, name, releaseArchive(t, ext, testBinaryName("zaparoo"), binary)) + opts := testStageOptions(t, testRelease("v"+testStageVersion, asset), "zaparoo") + + staged, err := stageRelease(context.Background(), opts, testAssetFetcher(t)) + require.NoError(t, err) + require.NotNil(t, staged) + + assert.Equal(t, testStageVersion, staged.Version) + assert.Equal(t, filepath.Join(opts.StagingRoot, testStageVersion), staged.Dir) + assert.Equal(t, filepath.Join(staged.Dir, name), staged.ArchivePath) + assert.Equal(t, filepath.Join(staged.Dir, payloadSubdir, testBinaryName("zaparoo")), staged.BinaryPath) + assert.FileExists(t, staged.ArchivePath) + + got, err := os.ReadFile(staged.BinaryPath) //nolint:gosec // path this package built under t.TempDir + require.NoError(t, err) + assert.Equal(t, binary, got) + + // The licence and readme are in the archive and stay there. + entries, err := os.ReadDir(filepath.Join(staged.Dir, payloadSubdir)) + require.NoError(t, err) + assert.Len(t, entries, 1) + + if runtime.GOOS != "windows" { + info, statErr := os.Stat(staged.BinaryPath) + require.NoError(t, statErr) + assert.NotZero(t, info.Mode().Perm()&0o111, "the staged binary is not executable") + } + + // Nothing outside the staging directory was touched. + assert.NoDirExists(t, filepath.Dir(opts.TargetPath)) +} + +func TestStageRelease_ReplacesAStaleStagingDirectory(t *testing.T) { + t.Parallel() + + ext := otameta.ArchiveExtTarGz + asset := servedAsset(t, testArchiveName(testStageVersion, ext), + releaseArchive(t, ext, testBinaryName("zaparoo"), fakeBinaryBytes(t))) + opts := testStageOptions(t, testRelease("v"+testStageVersion, asset), "zaparoo") + + // An attempt that died without cleaning up leaves a directory behind, and + // the archive is created exclusively, so it has to go. + stale := filepath.Join(opts.StagingRoot, testStageVersion) + require.NoError(t, os.MkdirAll(filepath.Join(stale, payloadSubdir), 0o750)) + require.NoError(t, os.WriteFile(filepath.Join(stale, testArchiveName(testStageVersion, ext)), + []byte("half a download"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(stale, "junk"), []byte("junk"), 0o600)) + + staged, err := stageRelease(context.Background(), opts, testAssetFetcher(t)) + require.NoError(t, err) + assert.NoFileExists(t, filepath.Join(staged.Dir, "junk")) +} + +// TestStageRelease_PrunesOrphanedStagingDirectories covers the directory no +// later attempt is looking for. The failure path only runs when the process +// lives to reach it, so a power cut mid-download leaves a version directory that +// the next release, computing a different name, would never touch. +func TestStageRelease_PrunesOrphanedStagingDirectories(t *testing.T) { + t.Parallel() + + ext := otameta.ArchiveExtTarGz + asset := servedAsset(t, testArchiveName(testStageVersion, ext), + releaseArchive(t, ext, testBinaryName("zaparoo"), fakeBinaryBytes(t))) + opts := testStageOptions(t, testRelease("v"+testStageVersion, asset), "zaparoo") + + orphan := filepath.Join(opts.StagingRoot, "2.9.0") + require.NoError(t, os.MkdirAll(filepath.Join(orphan, payloadSubdir), 0o750)) + require.NoError(t, os.WriteFile(filepath.Join(orphan, "half-a-download"), + bytes.Repeat([]byte("x"), 1024), 0o600)) + + staged, err := stageRelease(context.Background(), opts, testAssetFetcher(t)) + require.NoError(t, err) + + assert.NoDirExists(t, orphan, "an orphaned version directory was left on disk") + assert.DirExists(t, staged.Dir, "pruning removed the directory being staged into") +} + +func TestStageRelease_FailureLeavesNothingBehind(t *testing.T) { + t.Parallel() + + ext := otameta.ArchiveExtTarGz + asset := servedAsset(t, testArchiveName(testStageVersion, ext), + releaseArchive(t, ext, testBinaryName("zaparoo"), fakeBinaryBytes(t))) + asset.SHA256 = strings.Repeat("cd", sha256.Size) + opts := testStageOptions(t, testRelease("v"+testStageVersion, asset), "zaparoo") + + staged, err := stageRelease(context.Background(), opts, testAssetFetcher(t)) + require.ErrorIs(t, err, ErrChecksumMismatch) + assert.Nil(t, staged) + assert.NoDirExists(t, filepath.Join(opts.StagingRoot, testStageVersion)) +} + +func TestStageRelease_ArchiveWithoutTheBinary(t *testing.T) { + t.Parallel() + + ext := otameta.ArchiveExtTarGz + path := filepath.Join(t.TempDir(), "release"+ext) + writeTarGz(t, path, []tarMember{{name: "LICENSE.txt", body: []byte("gpl")}}) + body, err := os.ReadFile(path) //nolint:gosec // test path under t.TempDir + require.NoError(t, err) + + asset := servedAsset(t, testArchiveName(testStageVersion, ext), body) + opts := testStageOptions(t, testRelease("v"+testStageVersion, asset), "zaparoo") + + staged, err := stageRelease(context.Background(), opts, testAssetFetcher(t)) + require.ErrorIs(t, err, ErrArchiveRejected) + assert.Nil(t, staged) + assert.NoDirExists(t, filepath.Join(opts.StagingRoot, testStageVersion)) +} + +// TestStageRelease_ProbeFailures is the check that keeps a build which cannot +// run on this device from ever reaching a platform with no supervisor to +// recover it. +func TestStageRelease_ProbeFailures(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + stem string + wantMsg string + binary []byte + probeTimeout time.Duration + }{ + { + name: "exits non-zero", + stem: "zaparoo-fail", + wantMsg: "libz.so.1", + }, + { + name: "answers with the wrong version", + stem: "zaparoo-wrong", + wantMsg: "Zaparoo v0.0.1", + }, + { + name: "never answers", + stem: "zaparoo-hang", + probeTimeout: 300 * time.Millisecond, + wantMsg: "no answer within", + }, + { + name: "is not an executable", + stem: "zaparoo", + binary: bytes.Repeat([]byte("not an executable"), 64), + wantMsg: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + binary := tt.binary + if binary == nil { + binary = fakeBinaryBytes(t) + } + + ext := otameta.ArchiveExtTarGz + member := testBinaryName(tt.stem) + asset := servedAsset(t, testArchiveName(testStageVersion, ext), + releaseArchive(t, ext, member, binary)) + opts := testStageOptions(t, testRelease("v"+testStageVersion, asset), tt.stem) + + s, err := newStager(opts, testAssetFetcher(t)) + require.NoError(t, err) + if tt.probeTimeout > 0 { + s.probeTimeout = tt.probeTimeout + } + + staged, err := s.run(context.Background()) + require.ErrorIs(t, err, ErrProbeFailed) + assert.Nil(t, staged) + if tt.wantMsg != "" { + assert.Contains(t, err.Error(), tt.wantMsg) + } + // Checked immediately, which the code can be held to: these are the + // subtests that exec a staged binary, and removal is retried for long + // enough to outlast an image file the OS has not finished releasing. + assert.NoDirExists(t, filepath.Join(opts.StagingRoot, testStageVersion)) + }) + } +} + +// TestStageRelease_AcceptsProbeOutputBesideTheVersionLine is the compatibility +// half of the probe. The comparison is made by the build already installed +// against one that did not exist when it shipped, so a future release that prints +// a warning alongside its version must not be condemned as unrunnable for it. +func TestStageRelease_AcceptsProbeOutputBesideTheVersionLine(t *testing.T) { + t.Parallel() + + ext := otameta.ArchiveExtTarGz + asset := servedAsset(t, testArchiveName(testStageVersion, ext), + releaseArchive(t, ext, testBinaryName("zaparoo-chatty"), fakeBinaryBytes(t))) + opts := testStageOptions(t, testRelease("v"+testStageVersion, asset), "zaparoo-chatty") + + staged, err := stageRelease(context.Background(), opts, testAssetFetcher(t)) + require.NoError(t, err) + require.NotNil(t, staged) + assert.Equal(t, testStageVersion, staged.Version) +} + +// TestStageRelease_ChmodFailureIsLeftToTheProbe covers the filesystem MiSTer and +// MiSTeX install to, which no test host has: /media/fat is vfat or exFAT, it has +// no mode bits, and depending on which driver mounted it chmod either silently +// does nothing or returns an error while the mount's mask has already made the +// file executable. Refusing on the error would reject a release that runs. +// +// Both halves are asserted together, because the first is only safe while the +// second holds: a chmod error on its own does not condemn the release, and the +// probe still does when the binary genuinely cannot execute. +func TestStageRelease_ChmodFailureIsLeftToTheProbe(t *testing.T) { + t.Parallel() + + chmodFailed := errors.New("operation not supported") + + tests := []struct { + wantErr error + chmod func(*testing.T) func(string, os.FileMode) error + name string + wantSuccess bool + needsModes bool + }{ + { + name: "mask already granted the exec bit", + chmod: func(t *testing.T) func(string, os.FileMode) error { + t.Helper() + return func(path string, mode os.FileMode) error { + // Standing in for the mount mask: the file ends up executable + // without this call being what did it. On a filesystem that has + // mode bits, doing the chmod for real is how that is reproduced. + //nolint:gosec // G703: staging path under t.TempDir + require.NoError(t, os.Chmod(path, mode)) + return chmodFailed + } + }, + wantSuccess: true, + }, + { + name: "nothing made the binary executable", + chmod: func(t *testing.T) func(string, os.FileMode) error { + t.Helper() + return func(string, os.FileMode) error { return chmodFailed } + }, + wantErr: ErrProbeFailed, + needsModes: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + if tt.needsModes && runtime.GOOS == "windows" { + // There is no exec bit to withhold: the staged file runs whether + // the chmod worked or not, so the probe has nothing to catch. + t.Skip("windows has no permission bit for a failed chmod to leave unset") + } + + ext := otameta.ArchiveExtTarGz + asset := servedAsset(t, testArchiveName(testStageVersion, ext), + releaseArchive(t, ext, testBinaryName("zaparoo"), fakeBinaryBytes(t))) + opts := testStageOptions(t, testRelease("v"+testStageVersion, asset), "zaparoo") + s, err := newStager(opts, testAssetFetcher(t)) + require.NoError(t, err) + s.chmod = tt.chmod(t) + + staged, runErr := s.run(context.Background()) + if tt.wantSuccess { + require.NoError(t, runErr) + require.NotNil(t, staged) + assert.Equal(t, testStageVersion, staged.Version) + return + } + require.ErrorIs(t, runErr, tt.wantErr) + assert.Nil(t, staged) + }) + } +} + +// TestProbeBinary_CallerCancellationIsNotAProbeFailure keeps a shutdown from +// condemning a build. A probe that was interrupted never reached a verdict, and +// reporting one would mark a perfectly good release as unrunnable on this device. +func TestProbeBinary_CallerCancellationIsNotAProbeFailure(t *testing.T) { + t.Parallel() + + opts := testStageOptions(t, testRelease("v"+testStageVersion), "zaparoo") + s, err := newStager(opts, unusedFetcher) + require.NoError(t, err) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + go func() { + time.Sleep(150 * time.Millisecond) + cancel() + }() + + err = s.probeBinary(ctx, executableCopy(t, "zaparoo-hang"), testStageVersion) + require.Error(t, err) + require.NotErrorIs(t, err, ErrProbeFailed) + assert.Contains(t, err.Error(), "cancelled") +} + +// executableCopy puts the fake release binary somewhere named stem, which is how +// its behaviour is chosen. +func executableCopy(t *testing.T, stem string) string { + t.Helper() + + path := filepath.Join(t.TempDir(), testBinaryName(stem)) + //nolint:gosec // it has to be executable to be probed at all + require.NoError(t, os.WriteFile(path, fakeBinaryBytes(t), 0o755)) + return path +} + +func TestHasLine(t *testing.T) { + t.Parallel() + + const want = "Zaparoo v2.11.0 (mister)" + + assert.True(t, hasLine(want+"\n", want)) + assert.True(t, hasLine(want, want), "output without a trailing newline") + assert.True(t, hasLine(want+"\r\n", want), "output through a CRLF channel") + assert.True(t, hasLine("warning: something\n"+want+"\n", want)) + assert.True(t, hasLine(want+"\nnote: something\n", want)) + + assert.False(t, hasLine("", want)) + assert.False(t, hasLine("Zaparoo v2.11.0 (linux)\n", want)) + assert.False(t, hasLine("prefix "+want+"\n", want), "a line that merely contains it") + assert.False(t, hasLine(want+" suffix\n", want), "a line that merely starts with it") +} + +func TestStagingRootFor(t *testing.T) { + t.Parallel() + + assert.Empty(t, stagingRootFor("")) + assert.Equal(t, + filepath.Join("data", "updater", stagingSubdir), + stagingRootFor("data")) +} + +func TestClip(t *testing.T) { + t.Parallel() + + assert.Equal(t, "short", clip(" short\n", 32)) + assert.Equal(t, "abc…", clip("abcdef", 3)) +} + +// TestCappedBuilder_KeepsThePrefixAndDrainsTheRest covers the probe's output +// handling: a staged binary that fails by printing without stopping must not be +// able to grow the updater's memory, and must not block on a pipe nobody reads +// either, so every write is reported as consumed whatever happened to it. +func TestCappedBuilder_KeepsThePrefixAndDrainsTheRest(t *testing.T) { + t.Parallel() + + var b cappedBuilder + first := strings.Repeat("a", probeOutputLimit-1) + n, err := b.Write([]byte(first)) + require.NoError(t, err) + assert.Equal(t, len(first), n) + + n, err = b.Write([]byte("bcde")) + require.NoError(t, err) + assert.Equal(t, 4, n, "a partially kept write still has to report every byte consumed") + + n, err = b.Write([]byte(strings.Repeat("f", 1<<20))) + require.NoError(t, err) + assert.Equal(t, 1<<20, n, "a write past the cap still has to report every byte consumed") + + got := b.String() + assert.Len(t, got, probeOutputLimit) + assert.Equal(t, first+"b", got) +} diff --git a/pkg/service/updater/stall.go b/pkg/service/updater/stall.go new file mode 100644 index 000000000..6d2378ac8 --- /dev/null +++ b/pkg/service/updater/stall.go @@ -0,0 +1,137 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package updater + +import ( + "context" + "io" + "sync/atomic" + "time" +) + +// stallGuard cancels a transfer that has stopped making progress, without +// putting a ceiling on one that is merely slow. A download to an SD card over a +// congested link can legitimately take minutes, so a wall-clock timeout would +// make those devices unable to update at all; silence is the thing worth +// bounding. A blocked Read cannot time itself out, so a watcher goroutine +// cancels the request context instead. +type stallGuard struct { + cancel context.CancelFunc + done chan struct{} + // start anchors every measurement the guard makes. It carries a monotonic + // reading, so progress is stored and compared as an elapsed duration since + // this instant rather than as a wall-clock instant. That matters on the + // devices this exists for: MiSTer and MiSTeX have no RTC and get stepped by + // NTP shortly after the network comes up, which is the same window an update + // check and download run in. Measured against the wall clock, a forward step + // would abandon a perfectly healthy transfer and a backward step would + // disable stall detection until the step was worked off. + start time.Time + progress atomic.Int64 + fired atomic.Bool + timeout time.Duration +} + +// newStallGuard returns a context to make the request with and the guard +// watching it. The caller must stop the guard once the body is fully read, +// which also releases the context. +func newStallGuard(parent context.Context, timeout time.Duration) (context.Context, *stallGuard) { + ctx, cancel := context.WithCancel(parent) + g := &stallGuard{ + cancel: cancel, + done: make(chan struct{}), + start: time.Now(), + timeout: timeout, + } + g.touch() + go g.watch(ctx) + return ctx, g +} + +// reader wraps a reader so arriving bytes count as progress. +func (g *stallGuard) reader(r io.Reader) io.Reader { + return &progressReader{guard: g, source: r} +} + +// tripped reports whether the guard is what cancelled the context, which is how +// a stall is told apart from the caller giving up. +func (g *stallGuard) tripped() bool { + return g.fired.Load() +} + +// stop ends the watcher and releases the context. Safe to call more than once. +func (g *stallGuard) stop() { + g.cancel() + <-g.done +} + +// touch records that progress happened now, as nanoseconds since the guard +// started rather than as a clock reading. +func (g *stallGuard) touch() { + g.progress.Store(int64(g.since())) +} + +// since is how long the guard has been running, from its monotonic anchor. +func (g *stallGuard) since() time.Duration { + return time.Since(g.start) +} + +func (g *stallGuard) watch(ctx context.Context) { + defer close(g.done) + + // Checking several times per timeout keeps the worst-case detection delay to + // a fraction of it rather than nearly double. The floor is there because + // time.NewTicker panics on a non-positive interval, and nothing in an update + // is worth taking the service down over. + interval := g.timeout / stallChecks + if interval <= 0 { + interval = time.Millisecond + } + ticker := time.NewTicker(interval) + defer ticker.Stop() + + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + if idle := g.since() - time.Duration(g.progress.Load()); idle >= g.timeout { + g.fired.Store(true) + g.cancel() + return + } + } + } +} + +// progressReader reports every read that produced bytes to its guard. +type progressReader struct { + guard *stallGuard + source io.Reader +} + +func (r *progressReader) Read(p []byte) (int, error) { + n, err := r.source.Read(p) + if n > 0 { + r.guard.touch() + } + //nolint:wrapcheck // a reader wrapper has to pass io.EOF and the source's errors through unchanged + return n, err +} diff --git a/pkg/service/updater/stall_test.go b/pkg/service/updater/stall_test.go new file mode 100644 index 000000000..21147a7a8 --- /dev/null +++ b/pkg/service/updater/stall_test.go @@ -0,0 +1,185 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package updater + +import ( + "context" + "io" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// trickleReader stands in for a slow but healthy transfer: one byte at a time, +// with a pause between, for far longer in total than the stall timeout. +type trickleReader struct { + interval time.Duration + left int +} + +func (r *trickleReader) Read(p []byte) (int, error) { + if len(p) == 0 { + return 0, nil + } + if r.left == 0 { + return 0, io.EOF + } + time.Sleep(r.interval) + r.left-- + p[0] = 'x' + return 1, nil +} + +// emptyReader returns without producing bytes, which must not count as progress. +type emptyReader struct{ calls int } + +func (r *emptyReader) Read([]byte) (int, error) { + r.calls++ + if r.calls > 1 { + return 0, io.EOF + } + return 0, nil +} + +// TestStallGuard_ProgressKeepsASlowTransferAlive is the property the guard +// exists for, and the one nothing else in the package covers: it bounds silence, +// not duration. This transfer runs four times longer than the timeout while +// never going quiet for it, and has to survive. +// +// It is also the regression test for the tracking itself. The guard records +// progress once when it is constructed, so if reads stopped reporting it this +// would fail while a stall test would not notice. +func TestStallGuard_ProgressKeepsASlowTransferAlive(t *testing.T) { + t.Parallel() + + const ( + timeout = 500 * time.Millisecond + interval = 50 * time.Millisecond + total = 40 + ) + + ctx, guard := newStallGuard(context.Background(), timeout) + defer guard.stop() + + n, err := io.Copy(io.Discard, guard.reader(&trickleReader{interval: interval, left: total})) + require.NoError(t, err) + assert.EqualValues(t, total, n) + assert.False(t, guard.tripped(), "a transfer that never went quiet was reported as stalled") + assert.NoError(t, ctx.Err()) +} + +func TestStallGuard_SilenceTrips(t *testing.T) { + t.Parallel() + + ctx, guard := newStallGuard(context.Background(), 200*time.Millisecond) + defer guard.stop() + + select { + case <-ctx.Done(): + case <-time.After(30 * time.Second): + t.Fatal("the guard never fired on a transfer that made no progress") + } + assert.True(t, guard.tripped(), "the guard cancelled without recording that it was the one that did") +} + +// TestStallGuard_CallerCancelIsNotAStall keeps the two reasons a transfer stops +// distinguishable. Both arrive as a cancelled context, and only the guard's own +// record of firing tells them apart. +func TestStallGuard_CallerCancelIsNotAStall(t *testing.T) { + t.Parallel() + + parent, cancel := context.WithCancel(context.Background()) + ctx, guard := newStallGuard(parent, time.Hour) + defer guard.stop() + + cancel() + <-ctx.Done() + assert.False(t, guard.tripped(), "a caller giving up was recorded as a stall") +} + +func TestStallGuard_StopIsIdempotent(t *testing.T) { + t.Parallel() + + _, guard := newStallGuard(context.Background(), time.Hour) + guard.stop() + guard.stop() +} + +// TestStallGuard_TinyTimeoutDoesNotPanic covers the interval floor. Dividing the +// timeout by the check count can reach zero, and time.NewTicker panics on that. +func TestStallGuard_TinyTimeoutDoesNotPanic(t *testing.T) { + t.Parallel() + + ctx, guard := newStallGuard(context.Background(), time.Nanosecond) + defer guard.stop() + + select { + case <-ctx.Done(): + case <-time.After(30 * time.Second): + t.Fatal("the guard never fired") + } + assert.True(t, guard.tripped()) +} + +func TestProgressReader_CountsBytesAndPassesThrough(t *testing.T) { + t.Parallel() + + _, guard := newStallGuard(context.Background(), time.Hour) + defer guard.stop() + + before := guard.progress.Load() + // Long enough that since() has moved on even where the clock is coarse, so + // the assertion below is about the read counting as progress rather than + // about the platform's timer resolution. + time.Sleep(20 * time.Millisecond) + + r := guard.reader(strings.NewReader("zaparoo")) + buf := make([]byte, 4) + n, err := r.Read(buf) + require.NoError(t, err) + assert.Equal(t, 4, n) + assert.Equal(t, "zapa", string(buf)) + assert.Greater(t, guard.progress.Load(), before, + "a read that produced bytes did not count as progress") + + rest, err := io.ReadAll(r) + require.NoError(t, err) + assert.Equal(t, "roo", string(rest)) +} + +func TestProgressReader_EmptyReadIsNotProgress(t *testing.T) { + t.Parallel() + + _, guard := newStallGuard(context.Background(), time.Hour) + defer guard.stop() + + time.Sleep(time.Millisecond) + before := guard.progress.Load() + + r := guard.reader(&emptyReader{}) + n, err := r.Read(make([]byte, 4)) + require.NoError(t, err) + assert.Zero(t, n) + assert.Equal(t, before, guard.progress.Load(), + "a read that produced nothing was counted as progress") +} diff --git a/pkg/ui/tui/searchmedia_test.go b/pkg/ui/tui/searchmedia_test.go index 722594f7e..838b006e9 100644 --- a/pkg/ui/tui/searchmedia_test.go +++ b/pkg/ui/tui/searchmedia_test.go @@ -317,9 +317,13 @@ func TestBuildSearchMedia_AutoloadsMoreResults_Integration(t *testing.T) { runner.SimulateArrowDown() close(scrollDone) }() + // If the prefetch ran on the event loop the key press would block until + // releaseNextPage is closed, so this timeout only has to outlast scheduler + // jitter — it is not a latency budget. A short one false-fails when the + // whole suite is running under the race detector. select { case <-scrollDone: - case <-time.After(100 * time.Millisecond): + case <-time.After(5 * time.Second): close(releaseNextPage) <-scrollDone t.Fatal("scrolling blocked while the next page loaded")