release: v0.19.8 with durable runtime ownership and verified acquisition

agents/manager/downloader.go

@@ -4,6 +4,8 @@ import (
 	"archive/tar"
 	"compress/gzip"
 	"context"
+	"crypto/sha256"
+	"encoding/hex"
 	"fmt"
 	"io"
 	"net"
@@ -86,40 +88,28 @@ var agentDownloadClient = &http.Client{
 	},
 }
 
-// fetchRelease issues the asset GET under the caller's context, so cancelling
-// it aborts the transfer at any point — including mid-body, which no timeout
-// on the request can cover.
-//
-// It does not check the host: callers must pass a URL they have already put
-// through ValidURL, as Download does. The check lives there rather than here
-// because the tests that cover the stall and cancellation paths have to reach
-// a local server.
-func fetchRelease(ctx context.Context, releaseURL string) (*http.Response, error) {
-	req, err := http.NewRequestWithContext(ctx, http.MethodGet, releaseURL, nil)
-	if err != nil {
-		return nil, err
-	}
-	return agentDownloadClient.Do(req)
-}
-
-// writeArchive streams body into f and closes it. extractTarGz reopens the
-// archive by path, so the write handle must not outlive the copy — every
-// return here closes it, including the ones that fail.
-func writeArchive(f *os.File, body io.Reader, size int64) error {
+// writeArchive streams body into f, digesting it on the way, and closes it.
+// extractTarGz reopens the archive by path, so the write handle must not
+// outlive the copy — every return here closes it, including the ones that fail.
+func writeArchive(f *os.File, body io.Reader, size int64) (digest string, err error) {
 	defer func() { _ = f.Close() }()
 
 	bar := pb.Full.Start64(size)
 	bar.Set(pb.Bytes, true) // Display in bytes instead of default kilobytes
 
-	if _, err := io.Copy(bar.NewProxyWriter(f), body); err != nil {
-		return err
+	hash := sha256.New()
+	if _, err := io.Copy(io.MultiWriter(bar.NewProxyWriter(f), hash), body); err != nil {
+		return "", err
 	}
 	bar.Finish()
 
 	// Closed here rather than only by the defer: a write error that surfaces
 	// at close must reach the caller instead of appearing after the archive
 	// has already been extracted.
-	return f.Close()
+	if err := f.Close(); err != nil {
+		return "", err
+	}
+	return hex.EncodeToString(hash.Sum(nil)), nil
 }
 
 func Download(ctx context.Context, p *resources.Agent) error {
@@ -141,11 +131,11 @@ func Download(ctx context.Context, p *resources.Agent) error {
 	w.Info(fmt.Sprintf("Downloading agent %s", p.Identifier()))
 	w.Debug("downloading", wool.Field("agent", p.Identifier()), wool.Field("url", releaseURL).Debug())
 
-	body, size, err := openAgentRelease(ctx, releaseURL)
+	asset, err := openAgentRelease(ctx, releaseURL)
 	if err != nil {
 		return w.Wrapf(err, "cannot download agent")
 	}
-	defer body.Close()
+	defer asset.Body.Close()
 
 	tmp, err := os.CreateTemp("", "agent-*.tar.gz")
 	if err != nil {
@@ -158,9 +148,17 @@ func Download(ctx context.Context, p *resources.Agent) error {
 			w.Error("cannot remove temp file", wool.ErrField(err))
 		}
 	}(tmp.Name())
-	if err = writeArchive(tmp, body, size); err != nil {
+	archiveDigest, err := writeArchive(tmp, asset.Body, asset.Size)
+	if err != nil {
 		return w.Wrapf(err, "cannot copy agent")
 	}
+	warning, err := checkArchiveDigest(p.Identifier(), asset, archiveDigest)
+	if err != nil {
+		return w.Wrap(err)
+	}
+	if warning != "" {
+		w.Warn(warning, wool.Field("url", releaseURL))
+	}
 
 	tmpDir, err := os.MkdirTemp("", "agent-*")
 	if err != nil {
@@ -196,6 +194,20 @@ func Download(ctx context.Context, p *resources.Agent) error {
 	return nil
 }
 
+// checkArchiveDigest holds downloaded bytes to the digest the release records
+// for them, before anything is extracted. Without a digest nothing can be held,
+// and that is said rather than passed over: the archive is then trusted on its
+// name alone, which is the state every install was in before this check.
+func checkArchiveDigest(identifier string, asset *ReleaseAsset, archiveDigest string) (warning string, err error) {
+	switch {
+	case asset.Digest == "":
+		return "release publishes no digest for this archive; installing it unverified", nil
+	case archiveDigest != asset.Digest:
+		return "", fmt.Errorf("refusing to install %s: the downloaded archive is sha256:%s but the release's %s record says sha256:%s", identifier, archiveDigest, asset.DigestSource, asset.Digest)
+	}
+	return "", nil
+}
+
 // extractTarGz decompresses gzip then extracts the tar archive at src into
 // dst. Replaces mholt/archiver.Unarchive (which had two unpatched path
 // traversal CVEs). Only .tar.gz is supported — this is all GitHub release

agents/manager/downloader_test.go

@@ -14,6 +14,7 @@ import (
 	"time"
 
 	"github.com/codefly-dev/core/resources"
+	"github.com/google/go-github/v89/github"
 	"github.com/stretchr/testify/require"
 )
 
@@ -166,19 +167,25 @@ func TestFetchReleaseAbortsWhenContextIsCancelledBeforeHeaders(t *testing.T) {
 	// request it never returns at all, and this must report that as a failure
 	// rather than hang the package until the suite-wide timeout.
 	type result struct {
-		resp *http.Response
+		resp io.ReadCloser
 		err  error
 	}
 	returned := make(chan result, 1)
 	go func() {
-		resp, err := fetchRelease(ctx, server.URL)
+		endpoint := server.URL + "/"
+		client, err := newGitHubReleaseClient().Clone(github.WithURLs(&endpoint, &endpoint))
+		if err != nil {
+			returned <- result{err: err}
+			return
+		}
+		resp, _, err := client.Repositories.DownloadReleaseAsset(ctx, "owner", "repo", 1, agentDownloadClient)
 		returned <- result{resp, err}
 	}()
 
 	select {
 	case got := <-returned:
 		if got.err == nil {
-			got.resp.Body.Close()
+			got.resp.Close()
 			t.Fatal("a server that never answers must not return a response")
 		}
 		if !errors.Is(got.err, context.DeadlineExceeded) {
@@ -199,21 +206,24 @@ func TestFetchReleaseAbortsWhenContextIsCancelledMidTransfer(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	resp, err := fetchRelease(ctx, server.URL)
+	endpoint := server.URL + "/"
+	client, err := newGitHubReleaseClient().Clone(github.WithURLs(&endpoint, &endpoint))
+	require.NoError(t, err)
+	resp, _, err := client.Repositories.DownloadReleaseAsset(ctx, "owner", "repo", 1, agentDownloadClient)
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer resp.Body.Close()
+	defer resp.Close()
 
 	first := make([]byte, 1)
-	if _, err := io.ReadFull(resp.Body, first); err != nil {
+	if _, err := io.ReadFull(resp, first); err != nil {
 		t.Fatalf("expected the transfer to start: %v", err)
 	}
 
 	cancel()
 	failed := make(chan error, 1)
 	go func() {
-		_, err := io.Copy(io.Discard, resp.Body)
+		_, err := io.Copy(io.Discard, resp)
 		failed <- err
 	}()
 	select {
@@ -315,7 +325,7 @@ func TestWriteArchiveClosesItsHandle(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if err := writeArchive(f, strings.NewReader(payload), int64(len(payload))); err != nil {
+	if _, err := writeArchive(f, strings.NewReader(payload), int64(len(payload))); err != nil {
 		t.Fatal(err)
 	}
 	if err := f.Close(); !errors.Is(err, os.ErrClosed) {
@@ -338,11 +348,29 @@ func TestWriteArchiveClosesItsHandleWhenTheTransferFails(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	err = writeArchive(f, errReader{err: context.Canceled}, 1024)
+	_, err = writeArchive(f, errReader{err: context.Canceled}, 1024)
 	if !errors.Is(err, context.Canceled) {
 		t.Fatalf("got %v, want the transfer failure to reach the caller", err)
 	}
 	if err := f.Close(); !errors.Is(err, os.ErrClosed) {
 		t.Fatalf("second Close returned %v — a failed transfer leaked the descriptor", err)
 	}
 }
+
+// TestCheckArchiveDigestHoldsBytesToTheReleaseRecord: a digest mismatch is a
+// refusal naming both digests and whose record was used; no digest at all is a
+// warning that says the install is unverified, never a silent pass.
+func TestCheckArchiveDigestHoldsBytesToTheReleaseRecord(t *testing.T) {
+	warning, err := checkArchiveDigest("codefly.dev/parse:0.1.1", &ReleaseAsset{Digest: "aa", DigestSource: DigestFromChecksums}, "aa")
+	require.NoError(t, err)
+	require.Empty(t, warning)
+
+	_, err = checkArchiveDigest("codefly.dev/parse:0.1.1", &ReleaseAsset{Digest: "aa", DigestSource: DigestFromChecksums}, "bb")
+	require.ErrorContains(t, err, "refusing to install codefly.dev/parse:0.1.1")
+	require.ErrorContains(t, err, "sha256:bb")
+	require.ErrorContains(t, err, "checksums record says sha256:aa")
+
+	warning, err = checkArchiveDigest("codefly.dev/parse:0.1.1", &ReleaseAsset{}, "bb")
+	require.NoError(t, err)
+	require.Contains(t, warning, "unverified")
+}

agents/manager/manager.go

@@ -20,24 +20,25 @@ import (
 // API access is capped at 60 requests/hour per IP, so resolving "latest" for
 // several agents can flakily 403; a token raises the cap to 5000/hour.
 func newGitHubReleaseClient() *github.Client {
-	token := githubReleaseToken()
-	var options []github.ClientOptionsFunc
-	if token == "" {
-		client, err := github.NewClient()
-		if err != nil {
-			panic(fmt.Sprintf("configure GitHub release client: %v", err))
-		}
-		return client
+	token := GitHubReleaseToken()
+	options := []github.ClientOptionsFunc{github.WithHTTPClient(agentDownloadClient)}
+	if token != "" {
+		options = append(options, github.WithAuthToken(token))
 	}
-	options = append(options, github.WithAuthToken(token))
 	client, err := github.NewClient(options...)
 	if err != nil {
 		panic(fmt.Sprintf("configure authenticated GitHub release client: %v", err))
 	}
 	return client
 }
 
-func githubReleaseToken() string {
+// GitHubReleaseToken is the one credential rule the agent loader applies when
+// it downloads a release: GITHUB_TOKEN, else GH_TOKEN, else nothing. It never
+// consults a `gh` CLI login, because the hosts that install agents — CI runners
+// and hosted builds — have none. A caller that checks whether a release is
+// installable must check with exactly this credential: verifying with a wider
+// one establishes reachability for the laptop and nothing else.
+func GitHubReleaseToken() string {
 	if token := strings.TrimSpace(os.Getenv("GITHUB_TOKEN")); token != "" {
 		return token
 	}