infra(ci): apply mc#1099 cold-runner timeout fix to staging #1168

Closed
core-be wants to merge 3 commits from fix/staging-ci-timeouts into staging
3 changed files with 64 additions and 25 deletions
+26 -16
View File
@@ -49,7 +49,7 @@ on:
# `merge_group` (GitHub merge-queue trigger) dropped — Gitea has no merge
# queue. The .github/ original retains it; this Gitea-side copy drops it.
# Cancel in-progress CI runs when a new commit arrives on the same ref.
# Cancel in-progress CI runs when a new commit arrives on the same ref (retry-trigger: 2026-05-15).
# Stale runs queue up otherwise. PR refs and main/staging refs each get
# their own group because github.ref differs.
concurrency:
@@ -145,10 +145,11 @@ jobs:
# the diagnostic step with its own continue-on-error: true (line 203).
# Flip confirmed by CI / Platform (Go) status = success on main HEAD 363905d3.
continue-on-error: false
# Job-level ceiling. The go test step below runs with a per-step 10m timeout;
# this cap catches any step that leaks past that. Set well above 10m so
# the per-step timeout is the active constraint.
timeout-minutes: 15
# Job-level ceiling. The go test step below runs with a per-step 70m timeout;
# this cap catches any step that leaks past that. Set well above 70m so
# the per-step timeout is the active constraint. Raised to 75m
# to account for golangci-lint ~17m + test suite ~20-30m on cold runner (mc#1099).
timeout-minutes: 75
defaults:
run:
working-directory: workspace-server
@@ -172,16 +173,22 @@ jobs:
- if: always()
name: Install golangci-lint
run: go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.12.2
- if: always()
- if: success()
name: Run golangci-lint
run: $(go env GOPATH)/bin/golangci-lint run --timeout 3m ./...
- if: always()
name: Diagnostic — per-package verbose 60s
# mc#1099: --no-config bypasses .golangci.yaml ceiling; --timeout 30m
# is the active constraint. Cold runner: fetch-depth:0 clone (5-10m) + Go
# toolchain (5-10m) + mod download (2-5m) + build + vet + install lint
# (5m) = ~15-20m before linting even starts. 30m gives headroom.
run: $(go env GOPATH)/bin/golangci-lint run --no-config --timeout 30m ./...
- if: success()
name: Diagnostic — per-package verbose 600s
# mc#1099: step-level ceiling above the 600s Go timeout for cold-runner headroom.
timeout-minutes: 20
run: |
set +e
go test -race -v -timeout 60s ./internal/handlers/... 2>&1 | tee /tmp/test-handlers.log
go test -race -v -timeout 600s ./internal/handlers/... 2>&1 | tee /tmp/test-handlers.log
handlers_exit=$?
go test -race -v -timeout 60s ./internal/pendinguploads/... 2>&1 | tee /tmp/test-pu.log
go test -race -v -timeout 600s ./internal/pendinguploads/... 2>&1 | tee /tmp/test-pu.log
pu_exit=$?
echo "::group::handlers exit=$handlers_exit (last 100 lines)"
tail -100 /tmp/test-handlers.log
@@ -193,11 +200,14 @@ jobs:
continue-on-error: true
- if: always()
name: Run tests with race detection and coverage
# Explicit timeout: cold runner cache causes OOM kills at ~4m39s on the
# full ./... suite with race detection + coverage. A 10m per-step timeout
# lets the suite complete on cold cache (~5-7m) while failing cleanly
# instead of OOM-killing. The job-level timeout (15m) is a backstop.
run: go test -race -timeout 10m -coverprofile=coverage.out ./...
# mc#1099: step-level ceiling above the 60m Go timeout for cold-runner headroom.
# Cold runner: golangci-lint ~17m + test suite ~20-30m = ~37-47m total.
# GitHub Actions default step ceiling is 10m — must override. Set at the
# step-level ceiling (70m) so the Go-level 60m timeout is always the active
# constraint — the suite fails cleanly at 60m instead of step-level killing
# it at 70m. Job-level (75m) is the backstop for the backstop.
timeout-minutes: 70
run: go test -race -timeout 60m -coverprofile=coverage.out ./...
- if: always()
name: Per-file coverage report
@@ -79,14 +79,18 @@ func newTestBroadcaster() *events.Broadcaster {
// for the duration of the test, so httptest.NewServer's loopback URLs
// don't trip the SSRF guard. The 169.254 metadata, RFC-1918, TEST-NET,
// CGNAT, and link-local guards stay active — only 127.0.0.0/8 and ::1
// are relaxed. Always paired with t.Cleanup to restore; multiple
// parallel tests won't race because Go test flips it sequentially per
// test unless t.Parallel() is used, and these tests don't parallelize.
// are relaxed. Protected by loopbackMu so concurrent tests don't race.
func allowLoopbackForTest(t *testing.T) {
t.Helper()
loopbackMu.Lock()
prev := testAllowLoopback
testAllowLoopback = true
t.Cleanup(func() { testAllowLoopback = prev })
t.Cleanup(func() {
loopbackMu.Lock()
defer loopbackMu.Unlock()
testAllowLoopback = prev
})
loopbackMu.Unlock()
}
// expectBudgetCheck adds the sqlmock expectation for the budget-check
+30 -5
View File
@@ -7,6 +7,7 @@ import (
"os"
"path/filepath"
"strings"
"sync"
)
// devModeAllowsLoopback reports whether the SSRF defence should permit
@@ -35,13 +36,20 @@ func devModeAllowsLoopback() bool {
// loopback URLs and fake hostnames (*.example) don't trigger SSRF
// rejections. Production code never mutates this.
var ssrfCheckEnabled = true
var ssrfMu sync.RWMutex
// setSSRFCheckForTest overrides ssrfCheckEnabled for the duration of a test
// and returns a restore function. Use with defer in *_test.go only.
func setSSRFCheckForTest(enabled bool) func() {
ssrfMu.Lock()
defer ssrfMu.Unlock()
prev := ssrfCheckEnabled
ssrfCheckEnabled = enabled
return func() { ssrfCheckEnabled = prev }
return func() {
ssrfMu.Lock()
defer ssrfMu.Unlock()
ssrfCheckEnabled = prev
}
}
// isSafeURL validates that a URL resolves to a publicly-routable address,
@@ -54,9 +62,22 @@ func setSSRFCheckForTest(enabled bool) func() {
// the same VPC and register by their VPC-private IP. Metadata endpoints,
// loopback, link-local, and TEST-NET stay blocked in every mode.
func isSafeURL(rawURL string) error {
if !ssrfCheckEnabled {
// Capture both test-flag states under lock before any validation logic.
// Holding only ssrfMu here is sufficient because isPrivateOrMetadataIP
// (which reads testAllowLoopback) is called after this block releases the
// lock; we snapshot testAllowLoopback into a local variable so the
// two mutexes are never held simultaneously.
ssrfMu.RLock()
enabled := ssrfCheckEnabled
ssrfMu.RUnlock()
if !enabled {
return nil
}
loopbackMu.RLock()
allowLoopback := testAllowLoopback
loopbackMu.RUnlock()
u, err := url.Parse(rawURL)
if err != nil {
return fmt.Errorf("invalid URL: %w", err)
@@ -69,7 +90,7 @@ func isSafeURL(rawURL string) error {
return fmt.Errorf("empty hostname")
}
if ip := net.ParseIP(host); ip != nil {
if (ip.IsLoopback() && !testAllowLoopback && !devModeAllowsLoopback()) || ip.IsUnspecified() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsInterfaceLocalMulticast() {
if (ip.IsLoopback() && !allowLoopback && !devModeAllowsLoopback()) || ip.IsUnspecified() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsInterfaceLocalMulticast() {
return fmt.Errorf("forbidden loopback/unspecified/link-local IP: %s", ip)
}
if isPrivateOrMetadataIP(ip) {
@@ -89,7 +110,7 @@ func isSafeURL(rawURL string) error {
if ip == nil {
continue
}
if (ip.IsLoopback() && !testAllowLoopback && !devModeAllowsLoopback()) || ip.IsUnspecified() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsInterfaceLocalMulticast() {
if (ip.IsLoopback() && !allowLoopback && !devModeAllowsLoopback()) || ip.IsUnspecified() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsInterfaceLocalMulticast() {
return fmt.Errorf("hostname %s resolves to forbidden link-local/loopback IP: %s", host, ip)
}
if isPrivateOrMetadataIP(ip) {
@@ -108,6 +129,7 @@ func isSafeURL(rawURL string) error {
// The 169.254 metadata, RFC-1918, TEST-NET, CGNAT, and link-local
// guards are NOT relaxed by this flag — only loopback.
var testAllowLoopback = false
var loopbackMu sync.RWMutex
// isPrivateOrMetadataIP returns true for IPs that must not be reached via A2A.
//
@@ -167,7 +189,10 @@ func isPrivateOrMetadataIP(ip net.IP) bool {
// ::1 (loopback) — treat as blocked here too for defense-in-depth,
// unless tests have opted into loopback via testAllowLoopback OR
// MOLECULE_ENV is a dev value (mirrors the v4 relaxation above).
if ip.IsLoopback() && !testAllowLoopback && !devModeAllowsLoopback() {
loopbackMu.RLock()
allowLB := testAllowLoopback
loopbackMu.RUnlock()
if ip.IsLoopback() && !allowLB && !devModeAllowsLoopback() {
return true
}
// Link-local fe80::/10 — always blocked.