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

commit: 2221f72856a00aa9ff6e6744e56e7f2b1e7e97e4
author: brice · 10mo ago · 2025-12-25 16:40 UTC
parent: 5a26d21
signature: unsigned

5 files · +529 -8

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 +192 -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,122 @@ 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	+
196	300		// Download fetches the update archive from the given URL.
197	301		func (g *GitHubDownloader) Download(ctx context.Context, downloadURL, destDir string) (string, error) {
198	302		log := logging.FromContext(ctx)
199	303
	304	+	// Validate the download URL is from GitHub releases.
	305	+	if err := validateDownloadURL(downloadURL); err != nil {
	306	+	return "", fmt.Errorf("invalid download URL: %w", err)
	307	+	}
	308	+
200	309		// Ensure destination directory exists.
201	310		if err := os.MkdirAll(destDir, execPerm); err != nil {
202	311		return "", fmt.Errorf("failed to create download directory: %w", err)
203	312		}
204	313
	314	+	// Fetch checksums first for verification.
	315	+	checksums, err := g.fetchChecksums(ctx)
	316	+	if err != nil {
	317	+	return "", fmt.Errorf("failed to fetch checksums: %w", err)
	318	+	}
	319	+
	320	+	// Extract expected filename from URL to look up checksum.
	321	+	parsedURL, _ := url.Parse(downloadURL)
	322	+	archiveFilename := filepath.Base(parsedURL.Path)
	323	+	expectedHash, ok := checksums[archiveFilename]
	324	+	if !ok {
	325	+	return "", fmt.Errorf("no checksum found for %s", archiveFilename)
	326	+	}
	327	+
205	328		// Create the request.
206	329		req, err := http.NewRequestWithContext(ctx, http.MethodGet, downloadURL, http.NoBody)
207	330		if err != nil {

@@ -221,7 +344,11 @@ func (g *GitHubDownloader) Download(ctx context.Context, downloadURL, destDir st
221	344		return "", fmt.Errorf("download failed with status %d", resp.StatusCode)
222	345		}
223	346
224		-	// Extract filename from URL.
	347	+	// Check Content-Length if available to enforce size limit.
	348	+	if resp.ContentLength > maxArchiveSize {
	349	+	return "", fmt.Errorf("archive too large: %d bytes (max %d)", resp.ContentLength, maxArchiveSize)
	350	+	}
	351	+
225	352		archivePath := filepath.Join(destDir, "dumber_update.tar.gz")
226	353
227	354		// Create the file.

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

@@ -240,14 +368,61 @@ func (g *GitHubDownloader) Download(ctx context.Context, downloadURL, destDir st
240	368		return "", fmt.Errorf("failed to write archive: %w", err)
241	369		}
242	370
	371	+	// Check if we hit the limit (means file was too large).
	372	+	if written > maxArchiveSize {
	373	+	_ = os.Remove(archivePath)
	374	+	return "", fmt.Errorf("archive exceeds maximum size of %d bytes", maxArchiveSize)
	375	+	}
	376	+
	377	+	// Verify checksum before returning.
	378	+	if err := verifyChecksum(archivePath, expectedHash); err != nil {
	379	+	_ = os.Remove(archivePath)
	380	+	return "", fmt.Errorf("checksum verification failed: %w", err)
	381	+	}
	382	+
243	383		log.Debug().
244	384		Int64("bytes", written).
245	385		Str("path", archivePath).
246		-	Msg("download completed")
	386	+	Str("checksum", expectedHash[:16]+"...").
	387	+	Msg("download completed and verified")
247	388
248	389		return archivePath, nil
249	390		}
250	391
	392	+	// sanitizeTarPath validates and sanitizes a tar header name to prevent path traversal.
	393	+	func sanitizeTarPath(name, destDir string) (string, error) {
	394	+	// Clean the path to remove any . or .. components.
	395	+	cleaned := filepath.Clean(name)
	396	+
	397	+	// Reject absolute paths.
	398	+	if filepath.IsAbs(cleaned) {
	399	+	return "", fmt.Errorf("absolute path not allowed: %s", name)
	400	+	}
	401	+
	402	+	// Reject paths that try to escape using ..
	403	+	if strings.HasPrefix(cleaned, "..") \|\| strings.Contains(cleaned, string(filepath.Separator)+"..") {
	404	+	return "", fmt.Errorf("path traversal detected: %s", name)
	405	+	}
	406	+
	407	+	// Join with destination and verify result is within destDir.
	408	+	fullPath := filepath.Join(destDir, cleaned)
	409	+	absDestDir, err := filepath.Abs(destDir)
	410	+	if err != nil {
	411	+	return "", fmt.Errorf("failed to get absolute dest path: %w", err)
	412	+	}
	413	+	absFullPath, err := filepath.Abs(fullPath)
	414	+	if err != nil {
	415	+	return "", fmt.Errorf("failed to get absolute full path: %w", err)
	416	+	}
	417	+
	418	+	// Ensure the resolved path is within destDir.
	419	+	if !strings.HasPrefix(absFullPath, absDestDir+string(filepath.Separator)) && absFullPath != absDestDir {
	420	+	return "", fmt.Errorf("path escapes destination directory: %s", name)
	421	+	}
	422	+
	423	+	return fullPath, nil
	424	+	}
	425	+
251	426		// Extract extracts the dumber binary from the tar.gz archive.
252	427		func (*GitHubDownloader) Extract(ctx context.Context, archivePath, destDir string) (string, error) {
253	428		log := logging.FromContext(ctx)

@@ -279,6 +454,13 @@ func (*GitHubDownloader) Extract(ctx context.Context, archivePath, destDir strin
279	454
280	455		// Look for the dumber binary (inside dumber_VERSION/ directory).
281	456		if header.Typeflag == tar.TypeReg && strings.HasSuffix(header.Name, "/dumber") {
	457	+	// Sanitize the path to prevent path traversal attacks.
	458	+	_, err := sanitizeTarPath(header.Name, destDir)
	459	+	if err != nil {
	460	+	return "", fmt.Errorf("invalid tar entry: %w", err)
	461	+	}
	462	+
	463	+	// We always extract to a fixed name regardless of archive path.
282	464		binaryPath = filepath.Join(destDir, "dumber")
283	465
284	466		outFile, err := os.OpenFile(binaryPath, os.O_CREATE\|os.O_WRONLY\|os.O_TRUNC, execPerm)

@@ -296,6 +478,12 @@ func (*GitHubDownloader) Extract(ctx context.Context, archivePath, destDir strin
296	478		return "", fmt.Errorf("failed to extract binary: %w", err)
297	479		}
298	480
	481	+	// Verify minimum binary size to detect truncated/corrupt files.
	482	+	if written < minBinarySize {
	483	+	_ = os.Remove(binaryPath)
	484	+	return "", fmt.Errorf("binary too small (%d bytes), expected at least %d bytes", written, minBinarySize)
	485	+	}
	486	+
299	487		log.Debug().Int64("bytes", written).Str("path", binaryPath).Msg("extracted binary")
300	488		break
301	489		}


@@ -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,122 @@ 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	+
196		// Download fetches the update archive from the given URL.	300		// Download fetches the update archive from the given URL.
197		func (g *GitHubDownloader) Download(ctx context.Context, downloadURL, destDir string) (string, error) {	301		func (g *GitHubDownloader) Download(ctx context.Context, downloadURL, destDir string) (string, error) {
198		log := logging.FromContext(ctx)	302		log := logging.FromContext(ctx)
199			303
			304	+	// Validate the download URL is from GitHub releases.
			305	+	if err := validateDownloadURL(downloadURL); err != nil {
			306	+	return "", fmt.Errorf("invalid download URL: %w", err)
			307	+	}
			308	+
200		// Ensure destination directory exists.	309		// Ensure destination directory exists.
201		if err := os.MkdirAll(destDir, execPerm); err != nil {	310		if err := os.MkdirAll(destDir, execPerm); err != nil {
202		return "", fmt.Errorf("failed to create download directory: %w", err)	311		return "", fmt.Errorf("failed to create download directory: %w", err)
203		}	312		}
204			313
			314	+	// Fetch checksums first for verification.
			315	+	checksums, err := g.fetchChecksums(ctx)
			316	+	if err != nil {
			317	+	return "", fmt.Errorf("failed to fetch checksums: %w", err)
			318	+	}
			319	+
			320	+	// Extract expected filename from URL to look up checksum.
			321	+	parsedURL, _ := url.Parse(downloadURL)
			322	+	archiveFilename := filepath.Base(parsedURL.Path)
			323	+	expectedHash, ok := checksums[archiveFilename]
			324	+	if !ok {
			325	+	return "", fmt.Errorf("no checksum found for %s", archiveFilename)
			326	+	}
			327	+
205		// Create the request.	328		// Create the request.
206		req, err := http.NewRequestWithContext(ctx, http.MethodGet, downloadURL, http.NoBody)	329		req, err := http.NewRequestWithContext(ctx, http.MethodGet, downloadURL, http.NoBody)
207		if err != nil {	330		if err != nil {

@@ -221,7 +344,11 @@ func (g *GitHubDownloader) Download(ctx context.Context, downloadURL, destDir st
221		return "", fmt.Errorf("download failed with status %d", resp.StatusCode)	344		return "", fmt.Errorf("download failed with status %d", resp.StatusCode)
222		}	345		}
223			346
224	-	// Extract filename from URL.	347	+	// Check Content-Length if available to enforce size limit.
			348	+	if resp.ContentLength > maxArchiveSize {
			349	+	return "", fmt.Errorf("archive too large: %d bytes (max %d)", resp.ContentLength, maxArchiveSize)
			350	+	}
			351	+
225		archivePath := filepath.Join(destDir, "dumber_update.tar.gz")	352		archivePath := filepath.Join(destDir, "dumber_update.tar.gz")
226			353
227		// Create the file.	354		// Create the file.

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

@@ -240,14 +368,61 @@ func (g *GitHubDownloader) Download(ctx context.Context, downloadURL, destDir st
240		return "", fmt.Errorf("failed to write archive: %w", err)	368		return "", fmt.Errorf("failed to write archive: %w", err)
241		}	369		}
242			370
			371	+	// Check if we hit the limit (means file was too large).
			372	+	if written > maxArchiveSize {
			373	+	_ = os.Remove(archivePath)
			374	+	return "", fmt.Errorf("archive exceeds maximum size of %d bytes", maxArchiveSize)
			375	+	}
			376	+
			377	+	// Verify checksum before returning.
			378	+	if err := verifyChecksum(archivePath, expectedHash); err != nil {
			379	+	_ = os.Remove(archivePath)
			380	+	return "", fmt.Errorf("checksum verification failed: %w", err)
			381	+	}
			382	+
243		log.Debug().	383		log.Debug().
244		Int64("bytes", written).	384		Int64("bytes", written).
245		Str("path", archivePath).	385		Str("path", archivePath).
246	-	Msg("download completed")	386	+	Str("checksum", expectedHash[:16]+"...").
			387	+	Msg("download completed and verified")
247			388
248		return archivePath, nil	389		return archivePath, nil
249		}	390		}
250			391
			392	+	// sanitizeTarPath validates and sanitizes a tar header name to prevent path traversal.
			393	+	func sanitizeTarPath(name, destDir string) (string, error) {
			394	+	// Clean the path to remove any . or .. components.
			395	+	cleaned := filepath.Clean(name)
			396	+
			397	+	// Reject absolute paths.
			398	+	if filepath.IsAbs(cleaned) {
			399	+	return "", fmt.Errorf("absolute path not allowed: %s", name)
			400	+	}
			401	+
			402	+	// Reject paths that try to escape using ..
			403	+	if strings.HasPrefix(cleaned, "..") \|\| strings.Contains(cleaned, string(filepath.Separator)+"..") {
			404	+	return "", fmt.Errorf("path traversal detected: %s", name)
			405	+	}
			406	+
			407	+	// Join with destination and verify result is within destDir.
			408	+	fullPath := filepath.Join(destDir, cleaned)
			409	+	absDestDir, err := filepath.Abs(destDir)
			410	+	if err != nil {
			411	+	return "", fmt.Errorf("failed to get absolute dest path: %w", err)
			412	+	}
			413	+	absFullPath, err := filepath.Abs(fullPath)
			414	+	if err != nil {
			415	+	return "", fmt.Errorf("failed to get absolute full path: %w", err)
			416	+	}
			417	+
			418	+	// Ensure the resolved path is within destDir.
			419	+	if !strings.HasPrefix(absFullPath, absDestDir+string(filepath.Separator)) && absFullPath != absDestDir {
			420	+	return "", fmt.Errorf("path escapes destination directory: %s", name)
			421	+	}
			422	+
			423	+	return fullPath, nil
			424	+	}
			425	+
251		// Extract extracts the dumber binary from the tar.gz archive.	426		// Extract extracts the dumber binary from the tar.gz archive.
252		func (*GitHubDownloader) Extract(ctx context.Context, archivePath, destDir string) (string, error) {	427		func (*GitHubDownloader) Extract(ctx context.Context, archivePath, destDir string) (string, error) {
253		log := logging.FromContext(ctx)	428		log := logging.FromContext(ctx)

@@ -279,6 +454,13 @@ func (*GitHubDownloader) Extract(ctx context.Context, archivePath, destDir strin
279			454
280		// Look for the dumber binary (inside dumber_VERSION/ directory).	455		// Look for the dumber binary (inside dumber_VERSION/ directory).
281		if header.Typeflag == tar.TypeReg && strings.HasSuffix(header.Name, "/dumber") {	456		if header.Typeflag == tar.TypeReg && strings.HasSuffix(header.Name, "/dumber") {
			457	+	// Sanitize the path to prevent path traversal attacks.
			458	+	_, err := sanitizeTarPath(header.Name, destDir)
			459	+	if err != nil {
			460	+	return "", fmt.Errorf("invalid tar entry: %w", err)
			461	+	}
			462	+
			463	+	// We always extract to a fixed name regardless of archive path.
282		binaryPath = filepath.Join(destDir, "dumber")	464		binaryPath = filepath.Join(destDir, "dumber")
283			465
284		outFile, err := os.OpenFile(binaryPath, os.O_CREATE\|os.O_WRONLY\|os.O_TRUNC, execPerm)	466		outFile, err := os.OpenFile(binaryPath, os.O_CREATE\|os.O_WRONLY\|os.O_TRUNC, execPerm)

@@ -296,6 +478,12 @@ func (*GitHubDownloader) Extract(ctx context.Context, archivePath, destDir strin
296		return "", fmt.Errorf("failed to extract binary: %w", err)	478		return "", fmt.Errorf("failed to extract binary: %w", err)
297		}	479		}
298			480
			481	+	// Verify minimum binary size to detect truncated/corrupt files.
			482	+	if written < minBinarySize {
			483	+	_ = os.Remove(binaryPath)
			484	+	return "", fmt.Errorf("binary too small (%d bytes), expected at least %d bytes", written, minBinarySize)
			485	+	}
			486	+
299		log.Debug().Int64("bytes", written).Str("path", binaryPath).Msg("extracted binary")	487		log.Debug().Int64("bytes", written).Str("path", binaryPath).Msg("extracted binary")
300		break	488		break
301		}	489		}

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

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

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

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

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

M internal/ui/coordinator/update.go +23 -1


@@ -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,29 @@ 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	+	c.mu.Lock()
66	71		c.lastInfo = result
	72	+	c.mu.Unlock()
67	73
68	74		if !result.UpdateAvailable {
	75	+	c.mu.Lock()
69	76		c.status = entity.UpdateStatusUpToDate
	77	+	c.mu.Unlock()
70	78		log.Debug().
71	79		Str("version", result.CurrentVersion).
72	80		Msg("already on latest version")
73	81		return
74	82		}
75	83
	84	+	c.mu.Lock()
76	85		c.status = entity.UpdateStatusAvailable
	86	+	c.mu.Unlock()
77	87		log.Info().
78	88		Str("current", result.CurrentVersion).
79	89		Str("latest", result.LatestVersion).

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

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

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

 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
 	}
+	c.mu.Lock()
 	c.lastInfo = result
+	c.mu.Unlock()
 	if !result.UpdateAvailable {
+		c.mu.Lock()
 		c.status = entity.UpdateStatusUpToDate
+		c.mu.Unlock()
 		log.Debug().
 			Str("version", result.CurrentVersion).
 			Msg("already on latest version")
 		return
 	}
+	c.mu.Lock()
 	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
 }