Skip to content

Commit 29edf37

Browse files
committed
fix: harden path handling and permissions in manual upgrade
- Normalize paths with filepath.Clean() in IsManualInvocation to handle trailing slashes and .. components - Add directory boundary check to prevent staging-test matching staging - Use update.DirPermissions (0700) instead of 0755 for staging directory to match security posture of automatic upgrades - Skip auto-download in dry-run mode to fix integration tests
1 parent 0276213 commit 29edf37

File tree

3 files changed

+478
-0
lines changed

3 files changed

+478
-0
lines changed

cmd/upgrade/autodownload.go

Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,196 @@
1+
package upgrade
2+
3+
import (
4+
"context"
5+
"errors"
6+
"fmt"
7+
"log/slog"
8+
"os"
9+
"path/filepath"
10+
"strings"
11+
"syscall"
12+
"time"
13+
14+
"hostlink/app/services/agentstate"
15+
"hostlink/app/services/requestsigner"
16+
"hostlink/app/services/updatecheck"
17+
"hostlink/app/services/updatedownload"
18+
"hostlink/config/appconf"
19+
"hostlink/internal/httpclient"
20+
"hostlink/internal/update"
21+
)
22+
23+
// Sentinel errors for auto-download failures
24+
var (
25+
ErrUpdateCheckFailed = errors.New("update check failed")
26+
ErrDownloadFailed = errors.New("download failed")
27+
ErrExtractFailed = errors.New("extract failed")
28+
)
29+
30+
// UpdateCheckerInterface abstracts the update check client for testing.
31+
type UpdateCheckerInterface interface {
32+
Check() (*updatecheck.UpdateInfo, error)
33+
}
34+
35+
// DownloaderInterface abstracts the download functionality for testing.
36+
type DownloaderInterface interface {
37+
DownloadAndVerify(ctx context.Context, url, destPath, sha256 string) error
38+
}
39+
40+
// ExtractorInterface abstracts the tarball extraction for testing.
41+
type ExtractorInterface interface {
42+
Extract(tarPath, destPath string) error
43+
}
44+
45+
// AutoDownloader handles automatic download of the latest version.
46+
type AutoDownloader struct {
47+
UpdateChecker UpdateCheckerInterface
48+
Downloader DownloaderInterface
49+
Extractor ExtractorInterface
50+
StagingDir string
51+
}
52+
53+
// DownloadLatestIfNeeded checks for updates and downloads the latest version if available.
54+
// Returns the path to the staged binary, or empty string if no update is available.
55+
func (ad *AutoDownloader) DownloadLatestIfNeeded(ctx context.Context) (string, error) {
56+
// Check for updates
57+
info, err := ad.UpdateChecker.Check()
58+
if err != nil {
59+
return "", fmt.Errorf("%w: %v", ErrUpdateCheckFailed, err)
60+
}
61+
62+
if !info.UpdateAvailable {
63+
return "", nil
64+
}
65+
66+
// Create staging directory if it doesn't exist
67+
if err := os.MkdirAll(ad.StagingDir, update.DirPermissions); err != nil {
68+
return "", fmt.Errorf("failed to create staging directory: %w", err)
69+
}
70+
71+
// Download tarball
72+
tarballPath := filepath.Join(ad.StagingDir, "hostlink.tar.gz")
73+
if err := ad.Downloader.DownloadAndVerify(ctx, info.AgentURL, tarballPath, info.AgentSHA256); err != nil {
74+
return "", fmt.Errorf("%w: %v", ErrDownloadFailed, err)
75+
}
76+
77+
// Extract binary
78+
binaryPath := filepath.Join(ad.StagingDir, "hostlink")
79+
if err := ad.Extractor.Extract(tarballPath, binaryPath); err != nil {
80+
return "", fmt.Errorf("%w: %v", ErrExtractFailed, err)
81+
}
82+
83+
return binaryPath, nil
84+
}
85+
86+
// IsManualInvocation returns true if the upgrade command was invoked manually
87+
// (e.g., from /usr/bin/hostlink) rather than spawned by selfupdatejob from staging.
88+
func IsManualInvocation(selfPath, installPath, stagingDir string) bool {
89+
// Normalize all paths to handle trailing slashes, .., etc.
90+
selfPath = filepath.Clean(selfPath)
91+
installPath = filepath.Clean(installPath)
92+
stagingDir = filepath.Clean(stagingDir)
93+
94+
// If running from the install path, it's manual
95+
if selfPath == installPath {
96+
return true
97+
}
98+
99+
// If running from staging directory, it's spawned by selfupdatejob
100+
// Add separator to ensure directory boundary matching
101+
// e.g., /var/lib/staging should NOT match /var/lib/staging-test
102+
stagingDirWithSep := stagingDir + string(filepath.Separator)
103+
if strings.HasPrefix(selfPath, stagingDirWithSep) {
104+
return false
105+
}
106+
107+
// Any other path is considered manual (e.g., /tmp/hostlink for testing)
108+
return true
109+
}
110+
111+
// realDownloader wraps updatedownload.Downloader to implement DownloaderInterface.
112+
type realDownloader struct {
113+
d *updatedownload.Downloader
114+
}
115+
116+
func (r *realDownloader) DownloadAndVerify(ctx context.Context, url, destPath, sha256 string) error {
117+
_, err := r.d.DownloadAndVerify(ctx, url, destPath, sha256)
118+
return err
119+
}
120+
121+
// realExtractor wraps update.InstallBinary to implement ExtractorInterface.
122+
type realExtractor struct{}
123+
124+
func (r *realExtractor) Extract(tarPath, destPath string) error {
125+
return update.InstallBinary(tarPath, destPath)
126+
}
127+
128+
// NewAutoDownloaderConfig holds configuration for creating an AutoDownloader.
129+
type NewAutoDownloaderConfig struct {
130+
StagingDir string
131+
Logger *slog.Logger
132+
}
133+
134+
// NewAutoDownloader creates an AutoDownloader with real dependencies.
135+
// It loads agent ID and control plane URL from the environment/config files.
136+
func NewAutoDownloader(cfg NewAutoDownloaderConfig) (*AutoDownloader, error) {
137+
// Load agent ID from state file
138+
state := agentstate.New(appconf.AgentStatePath())
139+
if err := state.Load(); err != nil {
140+
return nil, fmt.Errorf("failed to load agent state: %w (is the agent registered?)", err)
141+
}
142+
agentID := state.GetAgentID()
143+
if agentID == "" {
144+
return nil, errors.New("agent not registered: run hostlink first to register")
145+
}
146+
147+
// Get control plane URL
148+
controlPlaneURL := appconf.ControlPlaneURL()
149+
if controlPlaneURL == "" {
150+
return nil, errors.New("control plane URL not configured")
151+
}
152+
153+
// Create request signer
154+
signer, err := requestsigner.New(appconf.AgentPrivateKeyPath(), agentID)
155+
if err != nil {
156+
return nil, fmt.Errorf("failed to create request signer: %w", err)
157+
}
158+
159+
// Create HTTP client with agent headers
160+
client := httpclient.NewClient(30 * time.Second)
161+
162+
// Create update checker
163+
checker, err := updatecheck.New(client, controlPlaneURL, agentID, signer)
164+
if err != nil {
165+
return nil, fmt.Errorf("failed to create update checker: %w", err)
166+
}
167+
168+
// Create downloader
169+
downloader := updatedownload.NewDownloader(updatedownload.DefaultDownloadConfig())
170+
171+
if cfg.Logger != nil {
172+
cfg.Logger.Info("auto-downloader initialized",
173+
"agent_id", agentID,
174+
"control_plane_url", controlPlaneURL,
175+
)
176+
}
177+
178+
return &AutoDownloader{
179+
UpdateChecker: checker,
180+
Downloader: &realDownloader{d: downloader},
181+
Extractor: &realExtractor{},
182+
StagingDir: cfg.StagingDir,
183+
}, nil
184+
}
185+
186+
// ExecStagedBinary replaces the current process with the staged binary.
187+
// This is used to hand off execution to the newly downloaded binary.
188+
// The function never returns on success (process is replaced).
189+
func ExecStagedBinary(stagedBinary string, args []string) error {
190+
// Prepend the binary path as argv[0]
191+
argv := append([]string{stagedBinary}, args...)
192+
193+
// Replace current process with the staged binary
194+
// This never returns on success
195+
return syscall.Exec(stagedBinary, argv, os.Environ())
196+
}

0 commit comments

Comments
 (0)