Merge pull request #31 from bnema/feat/auto-update

fix(update): security hardening for auto-update feature

commit: 8cdf4a4d0c602f452dfea7116961fc74cf66fede
author: bnema · 10mo ago · 2025-12-25 16:54 UTC
committer: GitHub · 10mo ago · 2025-12-25 16:54 UTC
merge of: 87f525c + af4d41b · diff against the first parent
signature: GPG UnverifiedB5690EEEBB952194 The key is unknown

5 files · +540 -10

M internal/application/usecase/apply_update.go +2 -1


@@ -4,6 +4,7 @@ import (
4	4		"context"
5	5		"fmt"
6	6		"os"
	7	+	"path/filepath"
7	8
8	9		"github.com/bnema/dumber/internal/application/port"
9	10		"github.com/bnema/dumber/internal/domain/entity"

@@ -64,7 +65,7 @@ func (uc *ApplyUpdateUseCase) Execute(ctx context.Context, input ApplyUpdateInpu
64	65		}
65	66
66	67		// Create download directory.
67		-	downloadDir := uc.cacheDir + "/updates"
	68	+	downloadDir := filepath.Join(uc.cacheDir, "updates")
68	69		if err := os.MkdirAll(downloadDir, updateDirPerm); err != nil {
69	70		return nil, fmt.Errorf("failed to create download directory: %w", err)
70	71		}

 	"context"
 	"fmt"
 	"os"
+	"path/filepath"
 	"github.com/bnema/dumber/internal/application/port"
 	"github.com/bnema/dumber/internal/domain/entity"
 	}
 	// Create download directory.
-	downloadDir := uc.cacheDir + "/updates"
+	downloadDir := filepath.Join(uc.cacheDir, "updates")
 	if err := os.MkdirAll(downloadDir, updateDirPerm); err != nil {
 		return nil, fmt.Errorf("failed to create download directory: %w", err)
 	}

M internal/cli/cmd/about.go +5 -2


@@ -51,7 +51,10 @@ func checkForUpdates(app *cli.App) error {
51	51
52	52		// Create update checker
53	53		checker := updater.NewGitHubChecker()
54		-	applier := updater.NewApplier("")
	54	+	applier, err := updater.NewApplierFromXDG()
	55	+	if err != nil {
	56	+	return fmt.Errorf("failed to create applier: %w", err)
	57	+	}
55	58
56	59		checkUC := usecase.NewCheckUpdateUseCase(checker, applier, app.BuildInfo)
57	60

@@ -62,7 +65,7 @@ func checkForUpdates(app *cli.App) error {
62	65
63	66		if result.UpdateAvailable {
64	67		fmt.Println()
65		-	fmt.Printf(" ✨ Update available: %s → %s\n", result.CurrentVersion, result.LatestVersion)
	68	+	fmt.Printf(" Update available: %s -> %s\n", result.CurrentVersion, result.LatestVersion)
66	69		fmt.Printf(" %s\n", result.ReleaseURL)
67	70		}
68	71

 	// Create update checker
 	checker := updater.NewGitHubChecker()
-	applier := updater.NewApplier("")
+	applier, err := updater.NewApplierFromXDG()
+	if err != nil {
+		return fmt.Errorf("failed to create applier: %w", err)
+	}
 	checkUC := usecase.NewCheckUpdateUseCase(checker, applier, app.BuildInfo)
 	if result.UpdateAvailable {
 		fmt.Println()
-		fmt.Printf("  ✨ Update available: %s → %s\n", result.CurrentVersion, result.LatestVersion)
+		fmt.Printf("  Update available: %s -> %s\n", result.CurrentVersion, result.LatestVersion)
 		fmt.Printf("     %s\n", result.ReleaseURL)
 	}

M internal/infrastructure/updater/github.go +214 -4


@@ -3,13 +3,17 @@ package updater
3	3
4	4		import (
5	5		"archive/tar"
	6	+	"bufio"
6	7		"compress/gzip"
7	8		"context"
	9	+	"crypto/sha256"
	10	+	"encoding/hex"
8	11		"encoding/json"
9	12		"errors"
10	13		"fmt"
11	14		"io"
12	15		"net/http"
	16	+	"net/url"
13	17		"os"
14	18		"path/filepath"
15	19		"regexp"

@@ -30,6 +34,9 @@ const (
30	34		// Uses version-less filename for stable "latest" download URL.
31	35		downloadURLTemplate = "https://github.com/bnema/dumber/releases/latest/download/dumber_%s_%s.tar.gz"
32	36
	37	+	// Checksums file URL pattern.
	38	+	checksumsURLTemplate = "https://github.com/bnema/dumber/releases/latest/download/checksums.txt"
	39	+
33	40		// HTTP client timeout for API requests.
34	41		apiTimeout = 10 * time.Second
35	42

@@ -41,6 +48,15 @@ const (
41	48
42	49		// Maximum size for extracted binary (100MB) - prevents decompression bombs.
43	50		maxBinarySize = 100 * 1024 * 1024
	51	+
	52	+	// Maximum archive download size (150MB) - prevents unbounded downloads.
	53	+	maxArchiveSize = 150 * 1024 * 1024
	54	+
	55	+	// Minimum binary size (1MB) - prevents truncated/corrupt binaries.
	56	+	minBinarySize = 1 * 1024 * 1024
	57	+
	58	+	// Allowed GitHub download host.
	59	+	allowedDownloadHost = "github.com"
44	60		)
45	61
46	62		// githubRelease represents the GitHub API response for a release.

@@ -193,15 +209,135 @@ func NewGitHubDownloader() *GitHubDownloader {
193	209		}
194	210		}
195	211
	212	+	// validateDownloadURL ensures the URL is a valid GitHub releases URL.
	213	+	func validateDownloadURL(downloadURL string) error {
	214	+	parsed, err := url.Parse(downloadURL)
	215	+	if err != nil {
	216	+	return fmt.Errorf("invalid URL: %w", err)
	217	+	}
	218	+
	219	+	if parsed.Scheme != "https" {
	220	+	return fmt.Errorf("URL must use HTTPS, got %s", parsed.Scheme)
	221	+	}
	222	+
	223	+	if parsed.Host != allowedDownloadHost {
	224	+	return fmt.Errorf("URL must be from %s, got %s", allowedDownloadHost, parsed.Host)
	225	+	}
	226	+
	227	+	// Verify it's a releases download path: /owner/repo/releases/...
	228	+	if !strings.Contains(parsed.Path, "/releases/") {
	229	+	return fmt.Errorf("URL must be a GitHub releases URL")
	230	+	}
	231	+
	232	+	return nil
	233	+	}
	234	+
	235	+	// fetchChecksums downloads the checksums.txt file and returns a map of filename -> sha256.
	236	+	func (g *GitHubDownloader) fetchChecksums(ctx context.Context) (map[string]string, error) {
	237	+	log := logging.FromContext(ctx)
	238	+
	239	+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, checksumsURLTemplate, http.NoBody)
	240	+	if err != nil {
	241	+	return nil, fmt.Errorf("failed to create checksums request: %w", err)
	242	+	}
	243	+	req.Header.Set("User-Agent", "dumber-browser")
	244	+
	245	+	resp, err := g.client.Do(req)
	246	+	if err != nil {
	247	+	return nil, fmt.Errorf("failed to fetch checksums: %w", err)
	248	+	}
	249	+	defer func() { _ = resp.Body.Close() }()
	250	+
	251	+	if resp.StatusCode != http.StatusOK {
	252	+	return nil, fmt.Errorf("checksums fetch failed with status %d", resp.StatusCode)
	253	+	}
	254	+
	255	+	checksums := make(map[string]string)
	256	+	scanner := bufio.NewScanner(resp.Body)
	257	+	for scanner.Scan() {
	258	+	line := strings.TrimSpace(scanner.Text())
	259	+	if line == "" {
	260	+	continue
	261	+	}
	262	+	// Format: "sha256 filename" (two spaces between)
	263	+	parts := strings.Fields(line)
	264	+	if len(parts) >= 2 {
	265	+	hash := parts[0]
	266	+	filename := parts[len(parts)-1] // Last field is filename
	267	+	checksums[filename] = hash
	268	+	}
	269	+	}
	270	+
	271	+	if err := scanner.Err(); err != nil {
	272	+	return nil, fmt.Errorf("failed to parse checksums: %w", err)
	273	+	}
	274	+
	275	+	log.Debug().Int("count", len(checksums)).Msg("fetched checksums")
	276	+	return checksums, nil
	277	+	}
	278	+
	279	+	// verifyChecksum verifies the SHA256 checksum of a file.
	280	+	func verifyChecksum(filePath, expectedHash string) error {
	281	+	file, err := os.Open(filePath)
	282	+	if err != nil {
	283	+	return fmt.Errorf("failed to open file for checksum: %w", err)
	284	+	}
	285	+	defer func() { _ = file.Close() }()
	286	+
	287	+	hash := sha256.New()
	288	+	if _, err := io.Copy(hash, file); err != nil {
	289	+	return fmt.Errorf("failed to compute checksum: %w", err)
	290	+	}
	291	+
	292	+	actualHash := hex.EncodeToString(hash.Sum(nil))
	293	+	if !strings.EqualFold(actualHash, expectedHash) {
	294	+	return fmt.Errorf("checksum mismatch: expected %s, got %s", expectedHash, actualHash)
	295	+	}
	296	+
	297	+	return nil
	298	+	}
	299	+
	300	+	// getExpectedChecksum fetches checksums and returns the hash for the given URL.
	301	+	func (g *GitHubDownloader) getExpectedChecksum(ctx context.Context, downloadURL string) (string, error) {
	302	+	checksums, err := g.fetchChecksums(ctx)
	303	+	if err != nil {
	304	+	return "", fmt.Errorf("failed to fetch checksums: %w", err)
	305	+	}
	306	+
	307	+	parsedURL, err := url.Parse(downloadURL)
	308	+	if err != nil {
	309	+	return "", fmt.Errorf("failed to parse download URL: %w", err)
	310	+	}
	311	+
	312	+	archiveFilename := filepath.Base(parsedURL.Path)
	313	+	expectedHash, ok := checksums[archiveFilename]
	314	+	if !ok {
	315	+	return "", fmt.Errorf("no checksum found for %s", archiveFilename)
	316	+	}
	317	+
	318	+	return expectedHash, nil
	319	+	}
	320	+
196	321		// Download fetches the update archive from the given URL.
197	322		func (g *GitHubDownloader) Download(ctx context.Context, downloadURL, destDir string) (string, error) {
198	323		log := logging.FromContext(ctx)
199	324
	325	+	// Validate the download URL is from GitHub releases.
	326	+	if err := validateDownloadURL(downloadURL); err != nil {
	327	+	return "", fmt.Errorf("invalid download URL: %w", err)
	328	+	}
	329	+
200	330		// Ensure destination directory exists.
201	331		if err := os.MkdirAll(destDir, execPerm); err != nil {
202	332		return "", fmt.Errorf("failed to create download directory: %w", err)
203	333		}
204	334
	335	+	// Fetch and look up expected checksum for this archive.
	336	+	expectedHash, err := g.getExpectedChecksum(ctx, downloadURL)
	337	+	if err != nil {
	338	+	return "", err
	339	+	}
	340	+
205	341		// Create the request.
206	342		req, err := http.NewRequestWithContext(ctx, http.MethodGet, downloadURL, http.NoBody)
207	343		if err != nil {

@@ -221,7 +357,11 @@ func (g *GitHubDownloader) Download(ctx context.Context, downloadURL, destDir st
221	357		return "", fmt.Errorf("download failed with status %d", resp.StatusCode)
222	358		}
223	359
224		-	// Extract filename from URL.
	360	+	// Check Content-Length if available to enforce size limit.
	361	+	if resp.ContentLength > maxArchiveSize {
	362	+	return "", fmt.Errorf("archive too large: %d bytes (max %d)", resp.ContentLength, maxArchiveSize)
	363	+	}
	364	+
225	365		archivePath := filepath.Join(destDir, "dumber_update.tar.gz")
226	366
227	367		// Create the file.

@@ -230,8 +370,9 @@ func (g *GitHubDownloader) Download(ctx context.Context, downloadURL, destDir st
230	370		return "", fmt.Errorf("failed to create archive file: %w", err)
231	371		}
232	372
233		-	// Copy the response body to the file.
234		-	written, err := io.Copy(file, resp.Body)
	373	+	// Use LimitReader to prevent unbounded downloads even if Content-Length is missing/spoofed.
	374	+	limitedReader := io.LimitReader(resp.Body, maxArchiveSize+1)
	375	+	written, err := io.Copy(file, limitedReader)
235	376		if closeErr := file.Close(); closeErr != nil && err == nil {
236	377		err = closeErr
237	378		}

@@ -240,14 +381,70 @@ func (g *GitHubDownloader) Download(ctx context.Context, downloadURL, destDir st
240	381		return "", fmt.Errorf("failed to write archive: %w", err)
241	382		}
242	383
	384	+	// Check if we hit the limit (means file was too large).
	385	+	if written > maxArchiveSize {
	386	+	_ = os.Remove(archivePath)
	387	+	return "", fmt.Errorf("archive exceeds maximum size of %d bytes", maxArchiveSize)
	388	+	}
	389	+
	390	+	// Verify checksum before returning.
	391	+	if err := verifyChecksum(archivePath, expectedHash); err != nil {
	392	+	_ = os.Remove(archivePath)
	393	+	return "", fmt.Errorf("checksum verification failed: %w", err)
	394	+	}
	395	+
	396	+	// Safely truncate hash for logging (SHA256 is 64 chars, but be defensive).
	397	+	displayHash := expectedHash
	398	+	if len(displayHash) > 16 {
	399	+	displayHash = displayHash[:16] + "..."
	400	+	}
	401	+
243	402		log.Debug().
244	403		Int64("bytes", written).
245	404		Str("path", archivePath).
246		-	Msg("download completed")
	405	+	Str("checksum", displayHash).
	406	+	Msg("download completed and verified")
247	407
248	408		return archivePath, nil
249	409		}
250	410
	411	+	// sanitizeTarPath validates and sanitizes a tar header name to prevent path traversal.
	412	+	func sanitizeTarPath(name, destDir string) (string, error) {
	413	+	// Clean the path to remove any . or .. components.
	414	+	cleaned := filepath.Clean(name)
	415	+
	416	+	// Reject absolute paths.
	417	+	if filepath.IsAbs(cleaned) {
	418	+	return "", fmt.Errorf("absolute path not allowed: %s", name)
	419	+	}
	420	+
	421	+	// Reject paths that contain ".." as a path component (not as part of filename).
	422	+	// Split by separator and check each component.
	423	+	for _, part := range strings.Split(cleaned, string(filepath.Separator)) {
	424	+	if part == ".." {
	425	+	return "", fmt.Errorf("path traversal detected: %s", name)
	426	+	}
	427	+	}
	428	+
	429	+	// Join with destination and verify result is within destDir.
	430	+	fullPath := filepath.Join(destDir, cleaned)
	431	+	absDestDir, err := filepath.Abs(destDir)
	432	+	if err != nil {
	433	+	return "", fmt.Errorf("failed to get absolute dest path: %w", err)
	434	+	}
	435	+	absFullPath, err := filepath.Abs(fullPath)
	436	+	if err != nil {
	437	+	return "", fmt.Errorf("failed to get absolute full path: %w", err)
	438	+	}
	439	+
	440	+	// Ensure the resolved path is within destDir.
	441	+	if !strings.HasPrefix(absFullPath, absDestDir+string(filepath.Separator)) && absFullPath != absDestDir {
	442	+	return "", fmt.Errorf("path escapes destination directory: %s", name)
	443	+	}
	444	+
	445	+	return fullPath, nil
	446	+	}
	447	+
251	448		// Extract extracts the dumber binary from the tar.gz archive.
252	449		func (*GitHubDownloader) Extract(ctx context.Context, archivePath, destDir string) (string, error) {
253	450		log := logging.FromContext(ctx)

@@ -279,6 +476,13 @@ func (*GitHubDownloader) Extract(ctx context.Context, archivePath, destDir strin
279	476
280	477		// Look for the dumber binary (inside dumber_VERSION/ directory).
281	478		if header.Typeflag == tar.TypeReg && strings.HasSuffix(header.Name, "/dumber") {
	479	+	// Sanitize the path to prevent path traversal attacks.
	480	+	_, err := sanitizeTarPath(header.Name, destDir)
	481	+	if err != nil {
	482	+	return "", fmt.Errorf("invalid tar entry: %w", err)
	483	+	}
	484	+
	485	+	// We always extract to a fixed name regardless of archive path.
282	486		binaryPath = filepath.Join(destDir, "dumber")
283	487
284	488		outFile, err := os.OpenFile(binaryPath, os.O_CREATE\|os.O_WRONLY\|os.O_TRUNC, execPerm)

@@ -296,6 +500,12 @@ func (*GitHubDownloader) Extract(ctx context.Context, archivePath, destDir strin
296	500		return "", fmt.Errorf("failed to extract binary: %w", err)
297	501		}
298	502
	503	+	// Verify minimum binary size to detect truncated/corrupt files.
	504	+	if written < minBinarySize {
	505	+	_ = os.Remove(binaryPath)
	506	+	return "", fmt.Errorf("binary too small (%d bytes), expected at least %d bytes", written, minBinarySize)
	507	+	}
	508	+
299	509		log.Debug().Int64("bytes", written).Str("path", binaryPath).Msg("extracted binary")
300	510		break
301	511		}


@@ -3,13 +3,17 @@ package updater
3			3
4		import (	4		import (
5		"archive/tar"	5		"archive/tar"
			6	+	"bufio"
6		"compress/gzip"	7		"compress/gzip"
7		"context"	8		"context"
			9	+	"crypto/sha256"
			10	+	"encoding/hex"
8		"encoding/json"	11		"encoding/json"
9		"errors"	12		"errors"
10		"fmt"	13		"fmt"
11		"io"	14		"io"
12		"net/http"	15		"net/http"
			16	+	"net/url"
13		"os"	17		"os"
14		"path/filepath"	18		"path/filepath"
15		"regexp"	19		"regexp"

@@ -30,6 +34,9 @@ const (
30		// Uses version-less filename for stable "latest" download URL.	34		// Uses version-less filename for stable "latest" download URL.
31		downloadURLTemplate = "https://github.com/bnema/dumber/releases/latest/download/dumber_%s_%s.tar.gz"	35		downloadURLTemplate = "https://github.com/bnema/dumber/releases/latest/download/dumber_%s_%s.tar.gz"
32			36
			37	+	// Checksums file URL pattern.
			38	+	checksumsURLTemplate = "https://github.com/bnema/dumber/releases/latest/download/checksums.txt"
			39	+
33		// HTTP client timeout for API requests.	40		// HTTP client timeout for API requests.
34		apiTimeout = 10 * time.Second	41		apiTimeout = 10 * time.Second
35			42

@@ -41,6 +48,15 @@ const (
41			48
42		// Maximum size for extracted binary (100MB) - prevents decompression bombs.	49		// Maximum size for extracted binary (100MB) - prevents decompression bombs.
43		maxBinarySize = 100 * 1024 * 1024	50		maxBinarySize = 100 * 1024 * 1024
			51	+
			52	+	// Maximum archive download size (150MB) - prevents unbounded downloads.
			53	+	maxArchiveSize = 150 * 1024 * 1024
			54	+
			55	+	// Minimum binary size (1MB) - prevents truncated/corrupt binaries.
			56	+	minBinarySize = 1 * 1024 * 1024
			57	+
			58	+	// Allowed GitHub download host.
			59	+	allowedDownloadHost = "github.com"
44		)	60		)
45			61
46		// githubRelease represents the GitHub API response for a release.	62		// githubRelease represents the GitHub API response for a release.

@@ -193,15 +209,135 @@ func NewGitHubDownloader() *GitHubDownloader {
193		}	209		}
194		}	210		}
195			211
			212	+	// validateDownloadURL ensures the URL is a valid GitHub releases URL.
			213	+	func validateDownloadURL(downloadURL string) error {
			214	+	parsed, err := url.Parse(downloadURL)
			215	+	if err != nil {
			216	+	return fmt.Errorf("invalid URL: %w", err)
			217	+	}
			218	+
			219	+	if parsed.Scheme != "https" {
			220	+	return fmt.Errorf("URL must use HTTPS, got %s", parsed.Scheme)
			221	+	}
			222	+
			223	+	if parsed.Host != allowedDownloadHost {
			224	+	return fmt.Errorf("URL must be from %s, got %s", allowedDownloadHost, parsed.Host)
			225	+	}
			226	+
			227	+	// Verify it's a releases download path: /owner/repo/releases/...
			228	+	if !strings.Contains(parsed.Path, "/releases/") {
			229	+	return fmt.Errorf("URL must be a GitHub releases URL")
			230	+	}
			231	+
			232	+	return nil
			233	+	}
			234	+
			235	+	// fetchChecksums downloads the checksums.txt file and returns a map of filename -> sha256.
			236	+	func (g *GitHubDownloader) fetchChecksums(ctx context.Context) (map[string]string, error) {
			237	+	log := logging.FromContext(ctx)
			238	+
			239	+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, checksumsURLTemplate, http.NoBody)
			240	+	if err != nil {
			241	+	return nil, fmt.Errorf("failed to create checksums request: %w", err)
			242	+	}
			243	+	req.Header.Set("User-Agent", "dumber-browser")
			244	+
			245	+	resp, err := g.client.Do(req)
			246	+	if err != nil {
			247	+	return nil, fmt.Errorf("failed to fetch checksums: %w", err)
			248	+	}
			249	+	defer func() { _ = resp.Body.Close() }()
			250	+
			251	+	if resp.StatusCode != http.StatusOK {
			252	+	return nil, fmt.Errorf("checksums fetch failed with status %d", resp.StatusCode)
			253	+	}
			254	+
			255	+	checksums := make(map[string]string)
			256	+	scanner := bufio.NewScanner(resp.Body)
			257	+	for scanner.Scan() {
			258	+	line := strings.TrimSpace(scanner.Text())
			259	+	if line == "" {
			260	+	continue
			261	+	}
			262	+	// Format: "sha256 filename" (two spaces between)
			263	+	parts := strings.Fields(line)
			264	+	if len(parts) >= 2 {
			265	+	hash := parts[0]
			266	+	filename := parts[len(parts)-1] // Last field is filename
			267	+	checksums[filename] = hash
			268	+	}
			269	+	}
			270	+
			271	+	if err := scanner.Err(); err != nil {
			272	+	return nil, fmt.Errorf("failed to parse checksums: %w", err)
			273	+	}
			274	+
			275	+	log.Debug().Int("count", len(checksums)).Msg("fetched checksums")
			276	+	return checksums, nil
			277	+	}
			278	+
			279	+	// verifyChecksum verifies the SHA256 checksum of a file.
			280	+	func verifyChecksum(filePath, expectedHash string) error {
			281	+	file, err := os.Open(filePath)
			282	+	if err != nil {
			283	+	return fmt.Errorf("failed to open file for checksum: %w", err)
			284	+	}
			285	+	defer func() { _ = file.Close() }()
			286	+
			287	+	hash := sha256.New()
			288	+	if _, err := io.Copy(hash, file); err != nil {
			289	+	return fmt.Errorf("failed to compute checksum: %w", err)
			290	+	}
			291	+
			292	+	actualHash := hex.EncodeToString(hash.Sum(nil))
			293	+	if !strings.EqualFold(actualHash, expectedHash) {
			294	+	return fmt.Errorf("checksum mismatch: expected %s, got %s", expectedHash, actualHash)
			295	+	}
			296	+
			297	+	return nil
			298	+	}
			299	+
			300	+	// getExpectedChecksum fetches checksums and returns the hash for the given URL.
			301	+	func (g *GitHubDownloader) getExpectedChecksum(ctx context.Context, downloadURL string) (string, error) {
			302	+	checksums, err := g.fetchChecksums(ctx)
			303	+	if err != nil {
			304	+	return "", fmt.Errorf("failed to fetch checksums: %w", err)
			305	+	}
			306	+
			307	+	parsedURL, err := url.Parse(downloadURL)
			308	+	if err != nil {
			309	+	return "", fmt.Errorf("failed to parse download URL: %w", err)
			310	+	}
			311	+
			312	+	archiveFilename := filepath.Base(parsedURL.Path)
			313	+	expectedHash, ok := checksums[archiveFilename]
			314	+	if !ok {
			315	+	return "", fmt.Errorf("no checksum found for %s", archiveFilename)
			316	+	}
			317	+
			318	+	return expectedHash, nil
			319	+	}
			320	+
196		// Download fetches the update archive from the given URL.	321		// Download fetches the update archive from the given URL.
197		func (g *GitHubDownloader) Download(ctx context.Context, downloadURL, destDir string) (string, error) {	322		func (g *GitHubDownloader) Download(ctx context.Context, downloadURL, destDir string) (string, error) {
198		log := logging.FromContext(ctx)	323		log := logging.FromContext(ctx)
199			324
			325	+	// Validate the download URL is from GitHub releases.
			326	+	if err := validateDownloadURL(downloadURL); err != nil {
			327	+	return "", fmt.Errorf("invalid download URL: %w", err)
			328	+	}
			329	+
200		// Ensure destination directory exists.	330		// Ensure destination directory exists.
201		if err := os.MkdirAll(destDir, execPerm); err != nil {	331		if err := os.MkdirAll(destDir, execPerm); err != nil {
202		return "", fmt.Errorf("failed to create download directory: %w", err)	332		return "", fmt.Errorf("failed to create download directory: %w", err)
203		}	333		}
204			334
			335	+	// Fetch and look up expected checksum for this archive.
			336	+	expectedHash, err := g.getExpectedChecksum(ctx, downloadURL)
			337	+	if err != nil {
			338	+	return "", err
			339	+	}
			340	+
205		// Create the request.	341		// Create the request.
206		req, err := http.NewRequestWithContext(ctx, http.MethodGet, downloadURL, http.NoBody)	342		req, err := http.NewRequestWithContext(ctx, http.MethodGet, downloadURL, http.NoBody)
207		if err != nil {	343		if err != nil {

@@ -221,7 +357,11 @@ func (g *GitHubDownloader) Download(ctx context.Context, downloadURL, destDir st
221		return "", fmt.Errorf("download failed with status %d", resp.StatusCode)	357		return "", fmt.Errorf("download failed with status %d", resp.StatusCode)
222		}	358		}
223			359
224	-	// Extract filename from URL.	360	+	// Check Content-Length if available to enforce size limit.
			361	+	if resp.ContentLength > maxArchiveSize {
			362	+	return "", fmt.Errorf("archive too large: %d bytes (max %d)", resp.ContentLength, maxArchiveSize)
			363	+	}
			364	+
225		archivePath := filepath.Join(destDir, "dumber_update.tar.gz")	365		archivePath := filepath.Join(destDir, "dumber_update.tar.gz")
226			366
227		// Create the file.	367		// Create the file.

@@ -230,8 +370,9 @@ func (g *GitHubDownloader) Download(ctx context.Context, downloadURL, destDir st
230		return "", fmt.Errorf("failed to create archive file: %w", err)	370		return "", fmt.Errorf("failed to create archive file: %w", err)
231		}	371		}
232			372
233	-	// Copy the response body to the file.	373	+	// Use LimitReader to prevent unbounded downloads even if Content-Length is missing/spoofed.
234	-	written, err := io.Copy(file, resp.Body)	374	+	limitedReader := io.LimitReader(resp.Body, maxArchiveSize+1)
			375	+	written, err := io.Copy(file, limitedReader)
235		if closeErr := file.Close(); closeErr != nil && err == nil {	376		if closeErr := file.Close(); closeErr != nil && err == nil {
236		err = closeErr	377		err = closeErr
237		}	378		}

@@ -240,14 +381,70 @@ func (g *GitHubDownloader) Download(ctx context.Context, downloadURL, destDir st
240		return "", fmt.Errorf("failed to write archive: %w", err)	381		return "", fmt.Errorf("failed to write archive: %w", err)
241		}	382		}
242			383
			384	+	// Check if we hit the limit (means file was too large).
			385	+	if written > maxArchiveSize {
			386	+	_ = os.Remove(archivePath)
			387	+	return "", fmt.Errorf("archive exceeds maximum size of %d bytes", maxArchiveSize)
			388	+	}
			389	+
			390	+	// Verify checksum before returning.
			391	+	if err := verifyChecksum(archivePath, expectedHash); err != nil {
			392	+	_ = os.Remove(archivePath)
			393	+	return "", fmt.Errorf("checksum verification failed: %w", err)
			394	+	}
			395	+
			396	+	// Safely truncate hash for logging (SHA256 is 64 chars, but be defensive).
			397	+	displayHash := expectedHash
			398	+	if len(displayHash) > 16 {
			399	+	displayHash = displayHash[:16] + "..."
			400	+	}
			401	+
243		log.Debug().	402		log.Debug().
244		Int64("bytes", written).	403		Int64("bytes", written).
245		Str("path", archivePath).	404		Str("path", archivePath).
246	-	Msg("download completed")	405	+	Str("checksum", displayHash).
			406	+	Msg("download completed and verified")
247			407
248		return archivePath, nil	408		return archivePath, nil
249		}	409		}
250			410
			411	+	// sanitizeTarPath validates and sanitizes a tar header name to prevent path traversal.
			412	+	func sanitizeTarPath(name, destDir string) (string, error) {
			413	+	// Clean the path to remove any . or .. components.
			414	+	cleaned := filepath.Clean(name)
			415	+
			416	+	// Reject absolute paths.
			417	+	if filepath.IsAbs(cleaned) {
			418	+	return "", fmt.Errorf("absolute path not allowed: %s", name)
			419	+	}
			420	+
			421	+	// Reject paths that contain ".." as a path component (not as part of filename).
			422	+	// Split by separator and check each component.
			423	+	for _, part := range strings.Split(cleaned, string(filepath.Separator)) {
			424	+	if part == ".." {
			425	+	return "", fmt.Errorf("path traversal detected: %s", name)
			426	+	}
			427	+	}
			428	+
			429	+	// Join with destination and verify result is within destDir.
			430	+	fullPath := filepath.Join(destDir, cleaned)
			431	+	absDestDir, err := filepath.Abs(destDir)
			432	+	if err != nil {
			433	+	return "", fmt.Errorf("failed to get absolute dest path: %w", err)
			434	+	}
			435	+	absFullPath, err := filepath.Abs(fullPath)
			436	+	if err != nil {
			437	+	return "", fmt.Errorf("failed to get absolute full path: %w", err)
			438	+	}
			439	+
			440	+	// Ensure the resolved path is within destDir.
			441	+	if !strings.HasPrefix(absFullPath, absDestDir+string(filepath.Separator)) && absFullPath != absDestDir {
			442	+	return "", fmt.Errorf("path escapes destination directory: %s", name)
			443	+	}
			444	+
			445	+	return fullPath, nil
			446	+	}
			447	+
251		// Extract extracts the dumber binary from the tar.gz archive.	448		// Extract extracts the dumber binary from the tar.gz archive.
252		func (*GitHubDownloader) Extract(ctx context.Context, archivePath, destDir string) (string, error) {	449		func (*GitHubDownloader) Extract(ctx context.Context, archivePath, destDir string) (string, error) {
253		log := logging.FromContext(ctx)	450		log := logging.FromContext(ctx)

@@ -279,6 +476,13 @@ func (*GitHubDownloader) Extract(ctx context.Context, archivePath, destDir strin
279			476
280		// Look for the dumber binary (inside dumber_VERSION/ directory).	477		// Look for the dumber binary (inside dumber_VERSION/ directory).
281		if header.Typeflag == tar.TypeReg && strings.HasSuffix(header.Name, "/dumber") {	478		if header.Typeflag == tar.TypeReg && strings.HasSuffix(header.Name, "/dumber") {
			479	+	// Sanitize the path to prevent path traversal attacks.
			480	+	_, err := sanitizeTarPath(header.Name, destDir)
			481	+	if err != nil {
			482	+	return "", fmt.Errorf("invalid tar entry: %w", err)
			483	+	}
			484	+
			485	+	// We always extract to a fixed name regardless of archive path.
282		binaryPath = filepath.Join(destDir, "dumber")	486		binaryPath = filepath.Join(destDir, "dumber")
283			487
284		outFile, err := os.OpenFile(binaryPath, os.O_CREATE\|os.O_WRONLY\|os.O_TRUNC, execPerm)	488		outFile, err := os.OpenFile(binaryPath, os.O_CREATE\|os.O_WRONLY\|os.O_TRUNC, execPerm)

@@ -296,6 +500,12 @@ func (*GitHubDownloader) Extract(ctx context.Context, archivePath, destDir strin
296		return "", fmt.Errorf("failed to extract binary: %w", err)	500		return "", fmt.Errorf("failed to extract binary: %w", err)
297		}	501		}
298			502
			503	+	// Verify minimum binary size to detect truncated/corrupt files.
			504	+	if written < minBinarySize {
			505	+	_ = os.Remove(binaryPath)
			506	+	return "", fmt.Errorf("binary too small (%d bytes), expected at least %d bytes", written, minBinarySize)
			507	+	}
			508	+
299		log.Debug().Int64("bytes", written).Str("path", binaryPath).Msg("extracted binary")	509		log.Debug().Int64("bytes", written).Str("path", binaryPath).Msg("extracted binary")
300		break	510		break
301		}	511		}

M internal/infrastructure/updater/github_test.go +298 -0

@@ -1,6 +1,11 @@
1	1		package updater
2	2
3	3		import (
	4	+	"crypto/sha256"
	5	+	"encoding/hex"
	6	+	"os"
	7	+	"path/filepath"
	8	+	"strings"
4	9		"testing"
5	10		)
6	11

@@ -61,3 +66,296 @@ func TestGetArchName(t *testing.T) {
61	66		t.Error("getArchName() returned empty string")
62	67		}
63	68		}
	69	+
	70	+	func TestValidateDownloadURL(t *testing.T) {
	71	+	tests := []struct {
	72	+	name string
	73	+	url string
	74	+	wantErr bool
	75	+	errMsg string
	76	+	}{
	77	+	// Valid URLs
	78	+	{
	79	+	name: "valid latest release URL",
	80	+	url: "https://github.com/bnema/dumber/releases/latest/download/dumber_linux_x86_64.tar.gz",
	81	+	wantErr: false,
	82	+	},
	83	+	{
	84	+	name: "valid versioned release URL",
	85	+	url: "https://github.com/bnema/dumber/releases/download/v0.15.0/dumber_linux_x86_64.tar.gz",
	86	+	wantErr: false,
	87	+	},
	88	+
	89	+	// Invalid scheme
	90	+	{
	91	+	name: "http not allowed",
	92	+	url: "http://github.com/bnema/dumber/releases/latest/download/dumber_linux_x86_64.tar.gz",
	93	+	wantErr: true,
	94	+	errMsg: "must use HTTPS",
	95	+	},
	96	+	{
	97	+	name: "ftp not allowed",
	98	+	url: "ftp://github.com/bnema/dumber/releases/latest/download/file.tar.gz",
	99	+	wantErr: true,
	100	+	errMsg: "must use HTTPS",
	101	+	},
	102	+
	103	+	// Invalid host
	104	+	{
	105	+	name: "wrong host",
	106	+	url: "https://evil.com/bnema/dumber/releases/latest/download/file.tar.gz",
	107	+	wantErr: true,
	108	+	errMsg: "must be from github.com",
	109	+	},
	110	+	{
	111	+	name: "subdomain not allowed",
	112	+	url: "https://raw.github.com/bnema/dumber/releases/latest/download/file.tar.gz",
	113	+	wantErr: true,
	114	+	errMsg: "must be from github.com",
	115	+	},
	116	+	{
	117	+	name: "typosquat domain",
	118	+	url: "https://githuh.com/bnema/dumber/releases/latest/download/file.tar.gz",
	119	+	wantErr: true,
	120	+	errMsg: "must be from github.com",
	121	+	},
	122	+
	123	+	// Invalid path
	124	+	{
	125	+	name: "not a releases URL",
	126	+	url: "https://github.com/bnema/dumber/raw/main/README.md",
	127	+	wantErr: true,
	128	+	errMsg: "must be a GitHub releases URL",
	129	+	},
	130	+	{
	131	+	name: "blob URL not allowed",
	132	+	url: "https://github.com/bnema/dumber/blob/main/cmd/dumber/main.go",
	133	+	wantErr: true,
	134	+	errMsg: "must be a GitHub releases URL",
	135	+	},
	136	+
	137	+	// Malformed URLs
	138	+	{
	139	+	name: "empty URL",
	140	+	url: "",
	141	+	wantErr: true,
	142	+	},
	143	+	{
	144	+	name: "invalid URL format",
	145	+	url: "://not-a-url",
	146	+	wantErr: true,
	147	+	},
	148	+	}
	149	+
	150	+	for _, tt := range tests {
	151	+	t.Run(tt.name, func(t *testing.T) {
	152	+	err := validateDownloadURL(tt.url)
	153	+	if tt.wantErr {
	154	+	if err == nil {
	155	+	t.Errorf("validateDownloadURL(%q) expected error, got nil", tt.url)
	156	+	} else if tt.errMsg != "" && !contains(err.Error(), tt.errMsg) {
	157	+	t.Errorf("validateDownloadURL(%q) error = %q, want to contain %q", tt.url, err.Error(), tt.errMsg)
	158	+	}
	159	+	} else {
	160	+	if err != nil {
	161	+	t.Errorf("validateDownloadURL(%q) unexpected error: %v", tt.url, err)
	162	+	}
	163	+	}
	164	+	})
	165	+	}
	166	+	}
	167	+
	168	+	func TestSanitizeTarPath(t *testing.T) {
	169	+	destDir := "/tmp/extract"
	170	+
	171	+	tests := []struct {
	172	+	name string
	173	+	path string
	174	+	wantErr bool
	175	+	errMsg string
	176	+	wantPath string
	177	+	}{
	178	+	// Valid paths
	179	+	{
	180	+	name: "simple filename",
	181	+	path: "dumber",
	182	+	wantErr: false,
	183	+	wantPath: "/tmp/extract/dumber",
	184	+	},
	185	+	{
	186	+	name: "nested path",
	187	+	path: "dumber_v0.15.0/dumber",
	188	+	wantErr: false,
	189	+	wantPath: "/tmp/extract/dumber_v0.15.0/dumber",
	190	+	},
	191	+	{
	192	+	name: "deep nested path",
	193	+	path: "release/bin/dumber",
	194	+	wantErr: false,
	195	+	wantPath: "/tmp/extract/release/bin/dumber",
	196	+	},
	197	+
	198	+	// Path traversal attempts
	199	+	{
	200	+	name: "parent directory escape",
	201	+	path: "../../../etc/passwd",
	202	+	wantErr: true,
	203	+	errMsg: "path traversal",
	204	+	},
	205	+	{
	206	+	name: "hidden parent escape",
	207	+	path: "dumber_v0.15.0/../../etc/passwd",
	208	+	wantErr: true,
	209	+	errMsg: "path traversal",
	210	+	},
	211	+	{
	212	+	name: "double dot at start of filename",
	213	+	path: "..dumber",
	214	+	wantErr: false, // Valid filename, not a path traversal (.. is not a path component)
	215	+	wantPath: "/tmp/extract/..dumber",
	216	+	},
	217	+	{
	218	+	name: "absolute path unix",
	219	+	path: "/etc/passwd",
	220	+	wantErr: true,
	221	+	errMsg: "absolute path",
	222	+	},
	223	+	{
	224	+	name: "dot dot in middle",
	225	+	path: "foo/../../../bar",
	226	+	wantErr: true,
	227	+	errMsg: "path traversal",
	228	+	},
	229	+
	230	+	// Edge cases
	231	+	{
	232	+	name: "current directory",
	233	+	path: "./dumber",
	234	+	wantErr: false,
	235	+	wantPath: "/tmp/extract/dumber",
	236	+	},
	237	+	{
	238	+	name: "multiple slashes",
	239	+	path: "dumber_v0.15.0//dumber",
	240	+	wantErr: false,
	241	+	wantPath: "/tmp/extract/dumber_v0.15.0/dumber",
	242	+	},
	243	+	}
	244	+
	245	+	for _, tt := range tests {
	246	+	t.Run(tt.name, func(t *testing.T) {
	247	+	result, err := sanitizeTarPath(tt.path, destDir)
	248	+	if tt.wantErr {
	249	+	if err == nil {
	250	+	t.Errorf("sanitizeTarPath(%q, %q) expected error, got nil", tt.path, destDir)
	251	+	} else if tt.errMsg != "" && !contains(err.Error(), tt.errMsg) {
	252	+	t.Errorf("sanitizeTarPath(%q, %q) error = %q, want to contain %q", tt.path, destDir, err.Error(), tt.errMsg)
	253	+	}
	254	+	} else {
	255	+	if err != nil {
	256	+	t.Errorf("sanitizeTarPath(%q, %q) unexpected error: %v", tt.path, destDir, err)
	257	+	} else if tt.wantPath != "" && result != tt.wantPath {
	258	+	t.Errorf("sanitizeTarPath(%q, %q) = %q, want %q", tt.path, destDir, result, tt.wantPath)
	259	+	}
	260	+	}
	261	+	})
	262	+	}
	263	+	}
	264	+
	265	+	func TestVerifyChecksum(t *testing.T) {
	266	+	// Create a temporary file with known content
	267	+	tmpDir := t.TempDir()
	268	+	testFile := filepath.Join(tmpDir, "testfile")
	269	+	testContent := []byte("hello world\n")
	270	+
	271	+	if err := os.WriteFile(testFile, testContent, 0o644); err != nil {
	272	+	t.Fatalf("Failed to create test file: %v", err)
	273	+	}
	274	+
	275	+	// Calculate expected SHA256 hash
	276	+	hash := sha256.Sum256(testContent)
	277	+	expectedHash := hex.EncodeToString(hash[:])
	278	+
	279	+	tests := []struct {
	280	+	name string
	281	+	filePath string
	282	+	hash string
	283	+	wantErr bool
	284	+	errMsg string
	285	+	}{
	286	+	{
	287	+	name: "valid checksum lowercase",
	288	+	filePath: testFile,
	289	+	hash: expectedHash,
	290	+	wantErr: false,
	291	+	},
	292	+	{
	293	+	name: "valid checksum uppercase",
	294	+	filePath: testFile,
	295	+	hash: hex.EncodeToString(hash[:]),
	296	+	wantErr: false,
	297	+	},
	298	+	{
	299	+	name: "invalid checksum",
	300	+	filePath: testFile,
	301	+	hash: "0000000000000000000000000000000000000000000000000000000000000000",
	302	+	wantErr: true,
	303	+	errMsg: "checksum mismatch",
	304	+	},
	305	+	{
	306	+	name: "wrong length hash",
	307	+	filePath: testFile,
	308	+	hash: "abc123",
	309	+	wantErr: true,
	310	+	errMsg: "checksum mismatch",
	311	+	},
	312	+	{
	313	+	name: "file not found",
	314	+	filePath: filepath.Join(tmpDir, "nonexistent"),
	315	+	hash: expectedHash,
	316	+	wantErr: true,
	317	+	errMsg: "failed to open",
	318	+	},
	319	+	}
	320	+
	321	+	for _, tt := range tests {
	322	+	t.Run(tt.name, func(t *testing.T) {
	323	+	err := verifyChecksum(tt.filePath, tt.hash)
	324	+	if tt.wantErr {
	325	+	if err == nil {
	326	+	t.Errorf("verifyChecksum() expected error, got nil")
	327	+	} else if tt.errMsg != "" && !contains(err.Error(), tt.errMsg) {
	328	+	t.Errorf("verifyChecksum() error = %q, want to contain %q", err.Error(), tt.errMsg)
	329	+	}
	330	+	} else {
	331	+	if err != nil {
	332	+	t.Errorf("verifyChecksum() unexpected error: %v", err)
	333	+	}
	334	+	}
	335	+	})
	336	+	}
	337	+	}
	338	+
	339	+	func TestSizeLimits(t *testing.T) {
	340	+	// Verify our size constants are sensible
	341	+	if maxArchiveSize <= 0 {
	342	+	t.Error("maxArchiveSize should be positive")
	343	+	}
	344	+	if maxBinarySize <= 0 {
	345	+	t.Error("maxBinarySize should be positive")
	346	+	}
	347	+	if minBinarySize <= 0 {
	348	+	t.Error("minBinarySize should be positive")
	349	+	}
	350	+	if minBinarySize >= maxBinarySize {
	351	+	t.Errorf("minBinarySize (%d) should be less than maxBinarySize (%d)", minBinarySize, maxBinarySize)
	352	+	}
	353	+	if maxBinarySize >= maxArchiveSize {
	354	+	t.Errorf("maxBinarySize (%d) should be less than maxArchiveSize (%d)", maxBinarySize, maxArchiveSize)
	355	+	}
	356	+	}
	357	+
	358	+	// contains checks if substr is in s (helper for error message checks).
	359	+	func contains(s, substr string) bool {
	360	+	return strings.Contains(s, substr)
	361	+	}

@@ -1,6 +1,11 @@
1	package updater	1		package updater
2		2
3	import (	3		import (
		4	+	"crypto/sha256"
		5	+	"encoding/hex"
		6	+	"os"
		7	+	"path/filepath"
		8	+	"strings"
4	"testing"	9		"testing"
5	)	10		)
6		11

@@ -61,3 +66,296 @@ func TestGetArchName(t *testing.T) {
61	t.Error("getArchName() returned empty string")	66		t.Error("getArchName() returned empty string")
62	}	67		}
63	}	68		}
		69	+
		70	+	func TestValidateDownloadURL(t *testing.T) {
		71	+	tests := []struct {
		72	+	name string
		73	+	url string
		74	+	wantErr bool
		75	+	errMsg string
		76	+	}{
		77	+	// Valid URLs
		78	+	{
		79	+	name: "valid latest release URL",
		80	+	url: "https://github.com/bnema/dumber/releases/latest/download/dumber_linux_x86_64.tar.gz",
		81	+	wantErr: false,
		82	+	},
		83	+	{
		84	+	name: "valid versioned release URL",
		85	+	url: "https://github.com/bnema/dumber/releases/download/v0.15.0/dumber_linux_x86_64.tar.gz",
		86	+	wantErr: false,
		87	+	},
		88	+
		89	+	// Invalid scheme
		90	+	{
		91	+	name: "http not allowed",
		92	+	url: "http://github.com/bnema/dumber/releases/latest/download/dumber_linux_x86_64.tar.gz",
		93	+	wantErr: true,
		94	+	errMsg: "must use HTTPS",
		95	+	},
		96	+	{
		97	+	name: "ftp not allowed",
		98	+	url: "ftp://github.com/bnema/dumber/releases/latest/download/file.tar.gz",
		99	+	wantErr: true,
		100	+	errMsg: "must use HTTPS",
		101	+	},
		102	+
		103	+	// Invalid host
		104	+	{
		105	+	name: "wrong host",
		106	+	url: "https://evil.com/bnema/dumber/releases/latest/download/file.tar.gz",
		107	+	wantErr: true,
		108	+	errMsg: "must be from github.com",
		109	+	},
		110	+	{
		111	+	name: "subdomain not allowed",
		112	+	url: "https://raw.github.com/bnema/dumber/releases/latest/download/file.tar.gz",
		113	+	wantErr: true,
		114	+	errMsg: "must be from github.com",
		115	+	},
		116	+	{
		117	+	name: "typosquat domain",
		118	+	url: "https://githuh.com/bnema/dumber/releases/latest/download/file.tar.gz",
		119	+	wantErr: true,
		120	+	errMsg: "must be from github.com",
		121	+	},
		122	+
		123	+	// Invalid path
		124	+	{
		125	+	name: "not a releases URL",
		126	+	url: "https://github.com/bnema/dumber/raw/main/README.md",
		127	+	wantErr: true,
		128	+	errMsg: "must be a GitHub releases URL",
		129	+	},
		130	+	{
		131	+	name: "blob URL not allowed",
		132	+	url: "https://github.com/bnema/dumber/blob/main/cmd/dumber/main.go",
		133	+	wantErr: true,
		134	+	errMsg: "must be a GitHub releases URL",
		135	+	},
		136	+
		137	+	// Malformed URLs
		138	+	{
		139	+	name: "empty URL",
		140	+	url: "",
		141	+	wantErr: true,
		142	+	},
		143	+	{
		144	+	name: "invalid URL format",
		145	+	url: "://not-a-url",
		146	+	wantErr: true,
		147	+	},
		148	+	}
		149	+
		150	+	for _, tt := range tests {
		151	+	t.Run(tt.name, func(t *testing.T) {
		152	+	err := validateDownloadURL(tt.url)
		153	+	if tt.wantErr {
		154	+	if err == nil {
		155	+	t.Errorf("validateDownloadURL(%q) expected error, got nil", tt.url)
		156	+	} else if tt.errMsg != "" && !contains(err.Error(), tt.errMsg) {
		157	+	t.Errorf("validateDownloadURL(%q) error = %q, want to contain %q", tt.url, err.Error(), tt.errMsg)
		158	+	}
		159	+	} else {
		160	+	if err != nil {
		161	+	t.Errorf("validateDownloadURL(%q) unexpected error: %v", tt.url, err)
		162	+	}
		163	+	}
		164	+	})
		165	+	}
		166	+	}
		167	+
		168	+	func TestSanitizeTarPath(t *testing.T) {
		169	+	destDir := "/tmp/extract"
		170	+
		171	+	tests := []struct {
		172	+	name string
		173	+	path string
		174	+	wantErr bool
		175	+	errMsg string
		176	+	wantPath string
		177	+	}{
		178	+	// Valid paths
		179	+	{
		180	+	name: "simple filename",
		181	+	path: "dumber",
		182	+	wantErr: false,
		183	+	wantPath: "/tmp/extract/dumber",
		184	+	},
		185	+	{
		186	+	name: "nested path",
		187	+	path: "dumber_v0.15.0/dumber",
		188	+	wantErr: false,
		189	+	wantPath: "/tmp/extract/dumber_v0.15.0/dumber",
		190	+	},
		191	+	{
		192	+	name: "deep nested path",
		193	+	path: "release/bin/dumber",
		194	+	wantErr: false,
		195	+	wantPath: "/tmp/extract/release/bin/dumber",
		196	+	},
		197	+
		198	+	// Path traversal attempts
		199	+	{
		200	+	name: "parent directory escape",
		201	+	path: "../../../etc/passwd",
		202	+	wantErr: true,
		203	+	errMsg: "path traversal",
		204	+	},
		205	+	{
		206	+	name: "hidden parent escape",
		207	+	path: "dumber_v0.15.0/../../etc/passwd",
		208	+	wantErr: true,
		209	+	errMsg: "path traversal",
		210	+	},
		211	+	{
		212	+	name: "double dot at start of filename",
		213	+	path: "..dumber",
		214	+	wantErr: false, // Valid filename, not a path traversal (.. is not a path component)
		215	+	wantPath: "/tmp/extract/..dumber",
		216	+	},
		217	+	{
		218	+	name: "absolute path unix",
		219	+	path: "/etc/passwd",
		220	+	wantErr: true,
		221	+	errMsg: "absolute path",
		222	+	},
		223	+	{
		224	+	name: "dot dot in middle",
		225	+	path: "foo/../../../bar",
		226	+	wantErr: true,
		227	+	errMsg: "path traversal",
		228	+	},
		229	+
		230	+	// Edge cases
		231	+	{
		232	+	name: "current directory",
		233	+	path: "./dumber",
		234	+	wantErr: false,
		235	+	wantPath: "/tmp/extract/dumber",
		236	+	},
		237	+	{
		238	+	name: "multiple slashes",
		239	+	path: "dumber_v0.15.0//dumber",
		240	+	wantErr: false,
		241	+	wantPath: "/tmp/extract/dumber_v0.15.0/dumber",
		242	+	},
		243	+	}
		244	+
		245	+	for _, tt := range tests {
		246	+	t.Run(tt.name, func(t *testing.T) {
		247	+	result, err := sanitizeTarPath(tt.path, destDir)
		248	+	if tt.wantErr {
		249	+	if err == nil {
		250	+	t.Errorf("sanitizeTarPath(%q, %q) expected error, got nil", tt.path, destDir)
		251	+	} else if tt.errMsg != "" && !contains(err.Error(), tt.errMsg) {
		252	+	t.Errorf("sanitizeTarPath(%q, %q) error = %q, want to contain %q", tt.path, destDir, err.Error(), tt.errMsg)
		253	+	}
		254	+	} else {
		255	+	if err != nil {
		256	+	t.Errorf("sanitizeTarPath(%q, %q) unexpected error: %v", tt.path, destDir, err)
		257	+	} else if tt.wantPath != "" && result != tt.wantPath {
		258	+	t.Errorf("sanitizeTarPath(%q, %q) = %q, want %q", tt.path, destDir, result, tt.wantPath)
		259	+	}
		260	+	}
		261	+	})
		262	+	}
		263	+	}
		264	+
		265	+	func TestVerifyChecksum(t *testing.T) {
		266	+	// Create a temporary file with known content
		267	+	tmpDir := t.TempDir()
		268	+	testFile := filepath.Join(tmpDir, "testfile")
		269	+	testContent := []byte("hello world\n")
		270	+
		271	+	if err := os.WriteFile(testFile, testContent, 0o644); err != nil {
		272	+	t.Fatalf("Failed to create test file: %v", err)
		273	+	}
		274	+
		275	+	// Calculate expected SHA256 hash
		276	+	hash := sha256.Sum256(testContent)
		277	+	expectedHash := hex.EncodeToString(hash[:])
		278	+
		279	+	tests := []struct {
		280	+	name string
		281	+	filePath string
		282	+	hash string
		283	+	wantErr bool
		284	+	errMsg string
		285	+	}{
		286	+	{
		287	+	name: "valid checksum lowercase",
		288	+	filePath: testFile,
		289	+	hash: expectedHash,
		290	+	wantErr: false,
		291	+	},
		292	+	{
		293	+	name: "valid checksum uppercase",
		294	+	filePath: testFile,
		295	+	hash: hex.EncodeToString(hash[:]),
		296	+	wantErr: false,
		297	+	},
		298	+	{
		299	+	name: "invalid checksum",
		300	+	filePath: testFile,
		301	+	hash: "0000000000000000000000000000000000000000000000000000000000000000",
		302	+	wantErr: true,
		303	+	errMsg: "checksum mismatch",
		304	+	},
		305	+	{
		306	+	name: "wrong length hash",
		307	+	filePath: testFile,
		308	+	hash: "abc123",
		309	+	wantErr: true,
		310	+	errMsg: "checksum mismatch",
		311	+	},
		312	+	{
		313	+	name: "file not found",
		314	+	filePath: filepath.Join(tmpDir, "nonexistent"),
		315	+	hash: expectedHash,
		316	+	wantErr: true,
		317	+	errMsg: "failed to open",
		318	+	},
		319	+	}
		320	+
		321	+	for _, tt := range tests {
		322	+	t.Run(tt.name, func(t *testing.T) {
		323	+	err := verifyChecksum(tt.filePath, tt.hash)
		324	+	if tt.wantErr {
		325	+	if err == nil {
		326	+	t.Errorf("verifyChecksum() expected error, got nil")
		327	+	} else if tt.errMsg != "" && !contains(err.Error(), tt.errMsg) {
		328	+	t.Errorf("verifyChecksum() error = %q, want to contain %q", err.Error(), tt.errMsg)
		329	+	}
		330	+	} else {
		331	+	if err != nil {
		332	+	t.Errorf("verifyChecksum() unexpected error: %v", err)
		333	+	}
		334	+	}
		335	+	})
		336	+	}
		337	+	}
		338	+
		339	+	func TestSizeLimits(t *testing.T) {
		340	+	// Verify our size constants are sensible
		341	+	if maxArchiveSize <= 0 {
		342	+	t.Error("maxArchiveSize should be positive")
		343	+	}
		344	+	if maxBinarySize <= 0 {
		345	+	t.Error("maxBinarySize should be positive")
		346	+	}
		347	+	if minBinarySize <= 0 {
		348	+	t.Error("minBinarySize should be positive")
		349	+	}
		350	+	if minBinarySize >= maxBinarySize {
		351	+	t.Errorf("minBinarySize (%d) should be less than maxBinarySize (%d)", minBinarySize, maxBinarySize)
		352	+	}
		353	+	if maxBinarySize >= maxArchiveSize {
		354	+	t.Errorf("maxBinarySize (%d) should be less than maxArchiveSize (%d)", maxBinarySize, maxArchiveSize)
		355	+	}
		356	+	}
		357	+
		358	+	// contains checks if substr is in s (helper for error message checks).
		359	+	func contains(s, substr string) bool {
		360	+	return strings.Contains(s, substr)
		361	+	}

M internal/ui/coordinator/update.go +21 -3


@@ -3,6 +3,7 @@ package coordinator
3	3		import (
4	4		"context"
5	5		"fmt"
	6	+	"sync"
6	7
7	8		"github.com/jwijenbergh/puregotk/v4/glib"
8	9

@@ -19,6 +20,7 @@ type UpdateCoordinator struct {
19	20		applyUC *usecase.ApplyUpdateUseCase
20	21		toaster *component.Toaster
21	22		config *config.Config
	23	+	mu sync.RWMutex // Protects status and lastInfo
22	24		status entity.UpdateStatus
23	25		lastInfo *usecase.CheckUpdateOutput
24	26		}

@@ -59,21 +61,25 @@ func (c *UpdateCoordinator) checkAsync(ctx context.Context) {
59	61		result, err := c.checkUC.Execute(ctx, usecase.CheckUpdateInput{})
60	62		if err != nil {
61	63		log.Warn().Err(err).Msg("background update check failed")
	64	+	c.mu.Lock()
62	65		c.status = entity.UpdateStatusFailed
	66	+	c.mu.Unlock()
63	67		return
64	68		}
65	69
	70	+	// Update both lastInfo and status atomically to avoid inconsistent reads.
	71	+	c.mu.Lock()
66	72		c.lastInfo = result
67		-
68	73		if !result.UpdateAvailable {
69	74		c.status = entity.UpdateStatusUpToDate
	75	+	c.mu.Unlock()
70	76		log.Debug().
71	77		Str("version", result.CurrentVersion).
72	78		Msg("already on latest version")
73	79		return
74	80		}
75		-
76	81		c.status = entity.UpdateStatusAvailable
	82	+	c.mu.Unlock()
77	83		log.Info().
78	84		Str("current", result.CurrentVersion).
79	85		Str("latest", result.LatestVersion).

@@ -85,7 +91,8 @@ func (c *UpdateCoordinator) checkAsync(ctx context.Context) {
85	91
86	92		// Auto-download if enabled and possible.
87	93		if c.config.Update.AutoDownload && result.CanAutoUpdate {
88		-	c.downloadAsync(ctx, result.DownloadURL)
	94	+	// Spawn download in separate goroutine to not block check completion.
	95	+	go c.downloadAsync(ctx, result.DownloadURL)
89	96		}
90	97		}
91	98

@@ -112,24 +119,31 @@ func (c UpdateCoordinator) showUpdateNotification(ctx context.Context, result
112	119		func (c *UpdateCoordinator) downloadAsync(ctx context.Context, downloadURL string) {
113	120		log := logging.FromContext(ctx)
114	121
	122	+	c.mu.Lock()
115	123		c.status = entity.UpdateStatusDownloading
	124	+	c.mu.Unlock()
116	125
117	126		result, err := c.applyUC.Execute(ctx, usecase.ApplyUpdateInput{
118	127		DownloadURL: downloadURL,
119	128		})
120	129		if err != nil {
121	130		log.Error().Err(err).Msg("update download failed")
	131	+	c.mu.Lock()
122	132		c.status = entity.UpdateStatusFailed
	133	+	c.mu.Unlock()
123	134		c.showToast(ctx, "Update download failed", component.ToastError)
124	135		return
125	136		}
126	137
	138	+	c.mu.Lock()
127	139		if result.Status == entity.UpdateStatusReady {
128	140		c.status = entity.UpdateStatusReady
	141	+	c.mu.Unlock()
129	142		log.Info().Msg("update ready, will apply on exit")
130	143		c.showToast(ctx, "Update ready - applies on exit", component.ToastSuccess)
131	144		} else {
132	145		c.status = entity.UpdateStatusFailed
	146	+	c.mu.Unlock()
133	147		log.Warn().Str("message", result.Message).Msg("update staging failed")
134	148		c.showToast(ctx, result.Message, component.ToastWarning)
135	149		}

@@ -155,11 +169,15 @@ func (c *UpdateCoordinator) FinalizeOnExit(ctx context.Context) error {
155	169
156	170		// Status returns the current update status.
157	171		func (c *UpdateCoordinator) Status() entity.UpdateStatus {
	172	+	c.mu.RLock()
	173	+	defer c.mu.RUnlock()
158	174		return c.status
159	175		}
160	176
161	177		// LastCheckResult returns the result of the last update check.
162	178		func (c UpdateCoordinator) LastCheckResult() usecase.CheckUpdateOutput {
	179	+	c.mu.RLock()
	180	+	defer c.mu.RUnlock()
163	181		return c.lastInfo
164	182		}
165	183

 import (
 	"context"
 	"fmt"
+	"sync"
 	"github.com/jwijenbergh/puregotk/v4/glib"
 	applyUC  *usecase.ApplyUpdateUseCase
 	toaster  *component.Toaster
 	config   *config.Config
+	mu       sync.RWMutex // Protects status and lastInfo
 	status   entity.UpdateStatus
 	lastInfo *usecase.CheckUpdateOutput
 }
 	result, err := c.checkUC.Execute(ctx, usecase.CheckUpdateInput{})
 	if err != nil {
 		log.Warn().Err(err).Msg("background update check failed")
+		c.mu.Lock()
 		c.status = entity.UpdateStatusFailed
+		c.mu.Unlock()
 		return
 	}
+	// Update both lastInfo and status atomically to avoid inconsistent reads.
+	c.mu.Lock()
 	c.lastInfo = result
 	if !result.UpdateAvailable {
 		c.status = entity.UpdateStatusUpToDate
+		c.mu.Unlock()
 		log.Debug().
 			Str("version", result.CurrentVersion).
 			Msg("already on latest version")
 		return
 	}
 	c.status = entity.UpdateStatusAvailable
+	c.mu.Unlock()
 	log.Info().
 		Str("current", result.CurrentVersion).
 		Str("latest", result.LatestVersion).
 	// Auto-download if enabled and possible.
 	if c.config.Update.AutoDownload && result.CanAutoUpdate {
-		c.downloadAsync(ctx, result.DownloadURL)
+		// Spawn download in separate goroutine to not block check completion.
+		go c.downloadAsync(ctx, result.DownloadURL)
 	}
 }
 func (c *UpdateCoordinator) downloadAsync(ctx context.Context, downloadURL string) {
 	log := logging.FromContext(ctx)
+	c.mu.Lock()
 	c.status = entity.UpdateStatusDownloading
+	c.mu.Unlock()
 	result, err := c.applyUC.Execute(ctx, usecase.ApplyUpdateInput{
 		DownloadURL: downloadURL,
 	})
 	if err != nil {
 		log.Error().Err(err).Msg("update download failed")
+		c.mu.Lock()
 		c.status = entity.UpdateStatusFailed
+		c.mu.Unlock()
 		c.showToast(ctx, "Update download failed", component.ToastError)
 		return
 	}
+	c.mu.Lock()
 	if result.Status == entity.UpdateStatusReady {
 		c.status = entity.UpdateStatusReady
+		c.mu.Unlock()
 		log.Info().Msg("update ready, will apply on exit")
 		c.showToast(ctx, "Update ready - applies on exit", component.ToastSuccess)
 	} else {
 		c.status = entity.UpdateStatusFailed
+		c.mu.Unlock()
 		log.Warn().Str("message", result.Message).Msg("update staging failed")
 		c.showToast(ctx, result.Message, component.ToastWarning)
 	}
 // Status returns the current update status.
 func (c *UpdateCoordinator) Status() entity.UpdateStatus {
+	c.mu.RLock()
+	defer c.mu.RUnlock()
 	return c.status
 }
 // LastCheckResult returns the result of the last update check.
 func (c *UpdateCoordinator) LastCheckResult() *usecase.CheckUpdateOutput {
+	c.mu.RLock()
+	defer c.mu.RUnlock()
 	return c.lastInfo
 }