Skip to content

Commit 67c603c

Browse files
committed
fix(upgrade): close thirteenth-round compliance findings — stopping loop
H1: capture applyErr before RollbackError unwraps it; add apply_error string field to upgrade.rollback_failed so forensic reconstruction has the root cause alongside the rollback failure (SOC1 completeness) M3: split github_token_present into github_token_present (env var set?) and github_token_forwarded (actually sent in header?) in both fetchReleaseFrom and download; log events for untrusted-host downloads now correctly record that a token existed but was withheld, rather than masking token availability (SOC2 CC6.6 boundary protection telemetry) M4: add comment to TestDownload_TokenSentToTrustedHost explaining why these two tests must not run in parallel (allowedTokenHosts mutation)
1 parent 9d45154 commit 67c603c

2 files changed

Lines changed: 21 additions & 7 deletions

File tree

pkg/upgrade/upgrade.go

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -335,10 +335,12 @@ func fetchReleaseFrom(baseURL, tag, operator string) (*GitHubRelease, error) {
335335
return nil, err
336336
}
337337
req.Header.Set("Accept", "application/vnd.github+json")
338-
tokenPresent := false
339-
if tok := os.Getenv("GITHUB_TOKEN"); tok != "" {
340-
tokenPresent = true
341-
req.Header.Set("Authorization", "Bearer "+tok)
338+
tokenPresent := os.Getenv("GITHUB_TOKEN") != ""
339+
tokenForwarded := false
340+
if tokenPresent {
341+
// GitHub API is always a trusted host — token is always forwarded when present.
342+
tokenForwarded = true
343+
req.Header.Set("Authorization", "Bearer "+os.Getenv("GITHUB_TOKEN"))
342344
}
343345

344346
start := time.Now()
@@ -351,6 +353,7 @@ func fetchReleaseFrom(baseURL, tag, operator string) (*GitHubRelease, error) {
351353
Str("method", http.MethodGet).
352354
Int64("latency_ms", time.Since(start).Milliseconds()).
353355
Bool("github_token_present", tokenPresent).
356+
Bool("github_token_forwarded", tokenForwarded).
354357
Msg("GitHub API network request failed")
355358
return nil, fmt.Errorf("GitHub API request failed: %w", err)
356359
}
@@ -372,6 +375,7 @@ func fetchReleaseFrom(baseURL, tag, operator string) (*GitHubRelease, error) {
372375
Int("status_code", resp.StatusCode).
373376
Int64("latency_ms", time.Since(start).Milliseconds()).
374377
Bool("github_token_present", tokenPresent).
378+
Bool("github_token_forwarded", tokenForwarded).
375379
Str("operator", operator).
376380
Msg("GitHub API response received")
377381

@@ -445,10 +449,12 @@ func download(rawURL, operator string) ([]byte, error) {
445449
return nil, err
446450
}
447451

448-
tokenPresent := false
449-
if tok := os.Getenv("GITHUB_TOKEN"); tok != "" {
452+
tok := os.Getenv("GITHUB_TOKEN")
453+
tokenPresent := tok != ""
454+
tokenForwarded := false
455+
if tokenPresent {
450456
if parsed, err := url.Parse(rawURL); err == nil && allowedTokenHosts[parsed.Hostname()] {
451-
tokenPresent = true
457+
tokenForwarded = true
452458
req.Header.Set("Authorization", "Bearer "+tok)
453459
}
454460
}
@@ -463,6 +469,7 @@ func download(rawURL, operator string) ([]byte, error) {
463469
Str("method", http.MethodGet).
464470
Int64("latency_ms", time.Since(start).Milliseconds()).
465471
Bool("github_token_present", tokenPresent).
472+
Bool("github_token_forwarded", tokenForwarded).
466473
Msg("HTTP network request failed")
467474
return nil, err
468475
}
@@ -484,6 +491,7 @@ func download(rawURL, operator string) ([]byte, error) {
484491
Int("status_code", resp.StatusCode).
485492
Int64("latency_ms", time.Since(start).Milliseconds()).
486493
Bool("github_token_present", tokenPresent).
494+
Bool("github_token_forwarded", tokenForwarded).
487495
Str("operator", operator).
488496
Msg("HTTP response received")
489497

@@ -580,9 +588,11 @@ func apply(binary io.Reader, operator, fromVersion, toVersion, exe, sourceURL st
580588
Msg("binary write commencing via selfupdate")
581589
err := selfupdate.Apply(binary, selfupdate.Options{})
582590
if err != nil {
591+
applyErr := err // preserve original cause before RollbackError unwraps it
583592
if rbErr := selfupdate.RollbackError(err); rbErr != nil {
584593
log.Error().Err(rbErr).
585594
Str("event", "upgrade.rollback_failed").
595+
Str("apply_error", applyErr.Error()).
586596
Str("operator", operator).
587597
Str("from_version", fromVersion).
588598
Str("to_version", toVersion).

pkg/upgrade/upgrade_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,10 @@ func TestExtractBinary_InvalidZip(t *testing.T) {
130130

131131
// ── download (token host allowlist) ──────────────────────────────────────────
132132

133+
// TestDownload_TokenSentToTrustedHost and TestDownload_TokenNotSentToUntrustedHost
134+
// must NOT run in parallel: the former mutates allowedTokenHosts (127.0.0.1 → true)
135+
// which would cause the latter to see the test server as trusted and silently invert
136+
// its assertion. The t.Cleanup restores state, but only after the test completes.
133137
func TestDownload_TokenSentToTrustedHost(t *testing.T) {
134138
var receivedAuth string
135139
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

0 commit comments

Comments
 (0)