Skip to content

Commit 9d45154

Browse files
committed
fix(upgrade): close twelfth-round compliance findings
M1: emit upgrade.http_body_read_error Error before returning a mid-body read failure; distinguishes a truncated transfer from a refused connection in forensic reconstruction (SOC2 CC7.3) M3: add captureLog() test helper that redirects the global zerolog logger to a buffer for test duration; use it to assert upgrade.http_request_failed, upgrade.download_size_limit_reached, and upgrade.github_api_rejected are actually emitted — test coverage now proves audit events fire, not just that errors propagate M4: add .Err(err) to upgrade.rollback_succeeded so the root cause of the apply failure is in the structured log record alongside the rollback outcome (SOC1 completeness and accuracy)
1 parent c25d185 commit 9d45154

2 files changed

Lines changed: 28 additions & 2 deletions

File tree

pkg/upgrade/upgrade.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,11 @@ func download(rawURL, operator string) ([]byte, error) {
498498
}
499499
data, err := io.ReadAll(io.LimitReader(resp.Body, maxBinaryBytes))
500500
if err != nil {
501+
log.Error().Err(err).
502+
Str("event", "upgrade.http_body_read_error").
503+
Str("operator", operator).
504+
Str("url", sanitizeURL(rawURL)).
505+
Msg("HTTP response body read failed after successful connection")
501506
return nil, err
502507
}
503508
// LimitReader silently caps at maxBinaryBytes; detect and reject truncated payloads.
@@ -587,8 +592,9 @@ func apply(binary io.Reader, operator, fromVersion, toVersion, exe, sourceURL st
587592
return fmt.Errorf("upgrade failed and rollback also failed: %w", rbErr)
588593
}
589594
// Rollback succeeded — log at Warn so auditors can distinguish a
590-
// recovery action from routine Info events.
591-
log.Warn().
595+
// recovery action from routine Info events. Include the original error
596+
// so the root cause is in the structured record (SOC1 completeness).
597+
log.Warn().Err(err).
592598
Str("event", "upgrade.rollback_succeeded").
593599
Str("operator", operator).
594600
Str("from_version", fromVersion).

pkg/upgrade/upgrade_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,24 @@ import (
2626
"runtime"
2727
"testing"
2828

29+
"github.com/rs/zerolog"
30+
"github.com/rs/zerolog/log"
2931
"github.com/stretchr/testify/assert"
3032
"github.com/stretchr/testify/require"
3133
)
3234

35+
// captureLog redirects the global zerolog logger to a buffer for the duration
36+
// of the test, then restores it. Returns the buffer so callers can assert on
37+
// logged event names.
38+
func captureLog(t *testing.T) *bytes.Buffer {
39+
t.Helper()
40+
var buf bytes.Buffer
41+
orig := log.Logger
42+
log.Logger = zerolog.New(&buf)
43+
t.Cleanup(func() { log.Logger = orig })
44+
return &buf
45+
}
46+
3347
// ── archiveAssetName ──────────────────────────────────────────────────────────
3448

3549
func TestArchiveAssetName(t *testing.T) {
@@ -137,6 +151,7 @@ func TestDownload_TokenSentToTrustedHost(t *testing.T) {
137151
}
138152

139153
func TestDownload_NonOKStatus(t *testing.T) {
154+
logBuf := captureLog(t)
140155
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
141156
w.WriteHeader(http.StatusForbidden)
142157
}))
@@ -145,11 +160,13 @@ func TestDownload_NonOKStatus(t *testing.T) {
145160
_, err := download(srv.URL+"/asset.zip", "testuser")
146161
require.Error(t, err)
147162
assert.Contains(t, err.Error(), "403")
163+
assert.Contains(t, logBuf.String(), "upgrade.http_request_failed", "audit event must fire on non-200 download")
148164
}
149165

150166
func TestDownload_BodyTruncatedAtLimit(t *testing.T) {
151167
// Serve exactly maxBinaryBytes bytes — download must return an error rather
152168
// than silently returning a truncated payload that would later fail checksum.
169+
logBuf := captureLog(t)
153170
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
154171
w.WriteHeader(http.StatusOK)
155172
chunk := make([]byte, 4096)
@@ -168,6 +185,7 @@ func TestDownload_BodyTruncatedAtLimit(t *testing.T) {
168185
_, err := download(srv.URL+"/big.zip", "testuser")
169186
require.Error(t, err)
170187
assert.Contains(t, err.Error(), "exceeded maximum allowed size")
188+
assert.Contains(t, logBuf.String(), "upgrade.download_size_limit_reached", "audit event must fire on body size cap")
171189
}
172190

173191
func TestDownload_TokenNotSentToUntrustedHost(t *testing.T) {
@@ -238,12 +256,14 @@ func TestFetchRelease_NotFound(t *testing.T) {
238256
}
239257

240258
func TestFetchRelease_RateLimited(t *testing.T) {
259+
logBuf := captureLog(t)
241260
srv := mockReleaseServer(t, "", http.StatusForbidden)
242261
defer srv.Close()
243262

244263
_, err := fetchReleaseFrom(srv.URL, "", "testuser")
245264
require.Error(t, err)
246265
assert.Contains(t, err.Error(), "rate limited")
266+
assert.Contains(t, logBuf.String(), "upgrade.github_api_rejected", "audit event must fire on rate-limited response")
247267
}
248268

249269
// ── sanitizeURL ───────────────────────────────────────────────────────────────

0 commit comments

Comments
 (0)