diff --git a/updater.go b/updater.go index 9ead82a..532c35d 100644 --- a/updater.go +++ b/updater.go @@ -53,9 +53,13 @@ type Config struct { PinnedVersion string // SkipAttestation disables SLSA attestation verification of - // checksums.txt. Intended for test environments where the test - // repos do not have real attestations. Default (false) enables - // verification in production. + // checksums.txt. This is the ONLY way to bypass attestation: when + // false (the default) and the gh CLI is absent, verification fails + // closed and the update is refused. Intended for test environments + // where the test repos do not have real attestations. Leaving it + // false in production keeps provenance verification mandatory. + // The SHA256 checksums.txt match is always enforced regardless of + // this flag. SkipAttestation bool // StatePath, when non-empty, points to a JSON control file @@ -403,8 +407,8 @@ func (u *Updater) applyUpdate(release *GitHubRelease) error { // actions/attest-build-provenance@v2 (PILOT-120, PR #166). // This closes the "attacker publishes matched fake binary + // fake checksums.txt" gap — the attestation ties checksums.txt - // to the trusted CI workflow. Graceful skip when gh CLI is - // unavailable (operator directive: not every environment has it). + // to the trusted CI workflow. Fails closed when the gh CLI is + // unavailable unless SkipAttestation is explicitly set. if err := u.verifyChecksumsAttestation(checksumsPath); err != nil { return fmt.Errorf("checksums attestation verification failed: %w", err) } @@ -756,27 +760,35 @@ func (u *Updater) signalDaemonRestartLinux() { // binary + fake checksums.txt" gap — the attestation ties checksums.txt to // the trusted CI workflow identity. // -// If gh is not on PATH, we log a warning and return nil (graceful skip per -// operator directive: not every environment has gh installed). If gh is -// present but verification fails, we return an error — the checksums file -// cannot be trusted. +// This gate FAILS CLOSED: if the gh CLI is not on PATH, verification returns +// an error and the update does not proceed. The only way to skip attestation +// is to set Config.SkipAttestation explicitly — there is no implicit skip. +// This is deliberate: the prior "gh absent => pass" behaviour silently +// disabled the entire provenance gate on headless production hosts (install.sh +// never installs gh), collapsing auto-update integrity back to "anyone with +// GitHub repo-write access can ship a malicious release." func (u *Updater) verifyChecksumsAttestation(checksumsPath string) error { if u.config.SkipAttestation { - slog.Debug("skipping attestation verification (SkipAttestation=true)") + slog.Warn("SLSA attestation verification disabled (SkipAttestation=true) — checksums provenance is NOT verified") return nil } return verifyChecksumsAttestationFn(u.config.Repo, checksumsPath) } -// verifyChecksumsAttestationFn is the default attestation verification -// implementation. Tests may replace it to avoid requiring a real GitHub -// repo with SLSA attestations. -var verifyChecksumsAttestationFn = func(repo, checksumsPath string) error { +// verifyChecksumsAttestationFn is the attestation verification implementation +// used by the updater. Tests may replace it to avoid requiring a real GitHub +// repo with SLSA attestations; they restore realVerifyChecksumsAttestationFn +// to exercise the production path. +var verifyChecksumsAttestationFn = realVerifyChecksumsAttestationFn + +// realVerifyChecksumsAttestationFn is the production attestation check. It +// fails closed when gh is absent: callers only reach this function when +// SkipAttestation is false, so a missing gh means provenance cannot be +// established and the update must be refused. +func realVerifyChecksumsAttestationFn(repo, checksumsPath string) error { ghPath, err := exec.LookPath("gh") if err != nil { - slog.Warn("gh CLI not found — skipping attestation verification; install gh for full provenance guarantee", - "install_url", "https://cli.github.com/") - return nil + return fmt.Errorf("gh CLI required for SLSA attestation verification; install gh (https://cli.github.com/) or set SkipAttestation to bypass: %w", err) } cmd := exec.Command(ghPath, "attestation", "verify", checksumsPath, "--repo", repo) diff --git a/zz_checksums_required_bug_test.go b/zz_checksums_required_bug_test.go index 7f69651..712a762 100644 --- a/zz_checksums_required_bug_test.go +++ b/zz_checksums_required_bug_test.go @@ -196,3 +196,96 @@ func TestApplyUpdate_PassesWithValidChecksums(t *testing.T) { t.Fatalf("green-path install failed: %v", err) } } + +// TestApplyUpdate_SkipAttestationStillRequiresChecksum verifies that +// SkipAttestation=true bypasses ONLY the SLSA attestation check — the +// mandatory SHA256 checksums.txt match is still enforced. A matching +// archive installs; a tampered archive is refused. The real attestation +// function is restored so the test proves SkipAttestation is the live +// bypass, not the TestMain stub. +func TestApplyUpdate_SkipAttestationStillRequiresChecksum(t *testing.T) { + // Not parallel: restores the package-level verifyChecksumsAttestationFn. + origFn := verifyChecksumsAttestationFn + verifyChecksumsAttestationFn = realVerifyChecksumsAttestationFn + defer func() { verifyChecksumsAttestationFn = origFn }() + + archiveName := fmt.Sprintf("pilot-%s-%s.tar.gz", runtime.GOOS, runtime.GOARCH) + + newServer := func(t *testing.T, archiveContent []byte, checksumsContent string) *httptest.Server { + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/download/" + archiveName: + w.Write(archiveContent) + case "/download/checksums.txt": + w.Write([]byte(checksumsContent)) + default: + http.NotFound(w, r) + } + })) + } + + newRelease := func(srvURL string) *GitHubRelease { + return &GitHubRelease{ + TagName: "v1.1.0", + Assets: []GitHubAsset{ + {Name: archiveName, BrowserDownloadURL: srvURL + "/download/" + archiveName}, + {Name: "checksums.txt", BrowserDownloadURL: srvURL + "/download/checksums.txt"}, + }, + } + } + + t.Run("matching checksum installs", func(t *testing.T) { + installDir := filepath.Join(t.TempDir(), "bin") + os.MkdirAll(installDir, 0755) + os.WriteFile(filepath.Join(installDir, ".pilot-version"), []byte("v1.0.0\n"), 0644) + + archivePath := filepath.Join(t.TempDir(), archiveName) + createTestTarGz(t, archivePath, map[string]string{"daemon": "good-content"}) + archiveContent, _ := os.ReadFile(archivePath) + hash := sha256.Sum256(archiveContent) + checksumsContent := fmt.Sprintf("%x %s\n", hash, archiveName) + + srv := newServer(t, archiveContent, checksumsContent) + defer srv.Close() + + u := &Updater{ + config: Config{InstallDir: installDir, SkipAttestation: true}, + client: srv.Client(), + stopCh: make(chan struct{}), + exitFn: func(int) {}, + } + if err := u.applyUpdate(newRelease(srv.URL)); err != nil { + t.Fatalf("SkipAttestation=true with matching checksum should install: %v", err) + } + }) + + t.Run("checksum mismatch is refused", func(t *testing.T) { + installDir := filepath.Join(t.TempDir(), "bin") + os.MkdirAll(installDir, 0755) + os.WriteFile(filepath.Join(installDir, ".pilot-version"), []byte("v1.0.0\n"), 0644) + + archivePath := filepath.Join(t.TempDir(), archiveName) + createTestTarGz(t, archivePath, map[string]string{"daemon": "good-content"}) + archiveContent, _ := os.ReadFile(archivePath) + // Checksum for DIFFERENT content — simulates a tampered archive. + wrongHash := sha256.Sum256([]byte("attacker-controlled")) + checksumsContent := fmt.Sprintf("%x %s\n", wrongHash, archiveName) + + srv := newServer(t, archiveContent, checksumsContent) + defer srv.Close() + + u := &Updater{ + config: Config{InstallDir: installDir, SkipAttestation: true}, + client: srv.Client(), + stopCh: make(chan struct{}), + exitFn: func(int) {}, + } + err := u.applyUpdate(newRelease(srv.URL)) + if err == nil { + t.Fatal("SkipAttestation=true must still enforce SHA256: mismatch should fail") + } + if !strings.Contains(err.Error(), "checksum") { + t.Errorf("expected checksum verification error, got: %v", err) + } + }) +} diff --git a/zz_coverage_test.go b/zz_coverage_test.go index d7e1f3c..d42219e 100644 --- a/zz_coverage_test.go +++ b/zz_coverage_test.go @@ -787,23 +787,56 @@ func TestArchiveToInstallMapping(t *testing.T) { } } -// TestVerifyChecksumsAttestation_GhNotInstalled verifies graceful -// skip when gh CLI is not on PATH. +// TestVerifyChecksumsAttestation_GhNotInstalled verifies the gate FAILS +// CLOSED when the gh CLI is not on PATH. The previous behaviour (silent +// pass) turned the SLSA provenance gate into a no-op on every headless +// production host, since install.sh never installs gh. func TestVerifyChecksumsAttestation_GhNotInstalled(t *testing.T) { - t.Parallel() + // Not parallel: mutates the process-wide PATH. // Temporarily unset PATH so LookPath fails. origPath := os.Getenv("PATH") os.Setenv("PATH", "") defer os.Setenv("PATH", origPath) - err := verifyChecksumsAttestationFn("test/repo", "/nonexistent/checksums.txt") - if err != nil { - t.Errorf("expected graceful skip when gh not installed, got error: %v", err) + // Call the real implementation directly — TestMain stubs the package + // variable to a nil-returning func for the rest of the suite. + err := realVerifyChecksumsAttestationFn("test/repo", "/nonexistent/checksums.txt") + if err == nil { + t.Fatal("expected error when gh is absent (fail closed), got nil") + } + if !strings.Contains(err.Error(), "gh CLI required") { + t.Errorf("error should explain gh is required, got: %v", err) + } +} + +// TestVerifyChecksumsAttestation_FailsClosedWithoutSkip verifies that the +// wrapper refuses the update when gh is absent and SkipAttestation is false. +func TestVerifyChecksumsAttestation_FailsClosedWithoutSkip(t *testing.T) { + // Not parallel: mutates the process-wide PATH and the package-level + // verifyChecksumsAttestationFn. + origFn := verifyChecksumsAttestationFn + verifyChecksumsAttestationFn = realVerifyChecksumsAttestationFn + defer func() { verifyChecksumsAttestationFn = origFn }() + + origPath := os.Getenv("PATH") + os.Setenv("PATH", "") + defer os.Setenv("PATH", origPath) + + u := New(Config{ + InstallDir: t.TempDir(), + Repo: "test/repo", + SkipAttestation: false, + }) + + err := u.verifyChecksumsAttestation("/nonexistent/checksums.txt") + if err == nil { + t.Fatal("expected fail-closed error when gh absent and SkipAttestation=false, got nil") } } -// TestVerifyChecksumsAttestation_SkipConfig verifies the config-driven skip. +// TestVerifyChecksumsAttestation_SkipConfig verifies the config-driven skip +// is the only way to bypass attestation. func TestVerifyChecksumsAttestation_SkipConfig(t *testing.T) { t.Parallel() diff --git a/zz_test.go b/zz_test.go index 007dafd..dae630d 100644 --- a/zz_test.go +++ b/zz_test.go @@ -18,10 +18,12 @@ import ( "time" ) -// TestMain disables attestation verification for all tests — test repos -// don't have real GitHub SLSA attestations, and the test environment may -// not have the gh CLI installed. A dedicated TestVerifyChecksumsAttestation -// test exercises the real function in isolation. +// TestMain stubs attestation verification for the bulk of the suite — +// test repos don't have real GitHub SLSA attestations, and the test +// environment may not have the gh CLI installed. Tests that need the +// production fail-closed behaviour restore realVerifyChecksumsAttestationFn +// explicitly (see TestVerifyChecksumsAttestation_* and +// TestApplyUpdate_SkipAttestationStillRequiresChecksum). func TestMain(m *testing.M) { verifyChecksumsAttestationFn = func(repo, checksumsPath string) error { return nil