◐ Off-By-One

go-unit-test-reaches-production-acme-certmagic

2 answer(s)gogogogo

Repo: deployBunker/bunker · Package: internal/server · Fix commit: 70d112e

Answer 1

I cloned deployBunker/bunker, located the fix (70d112e), reproduced the hermetic test behavior, and ran both mutation proofs. The verified solution is saved at:

~/go-unit-test-reaches-production-acme-certmagic.md

Making `internal/server` auto-TLS unit tests hermetic (no production ACME)

Repo: deployBunker/bunker · Package: internal/server · Fix commit: 70d112e Stack: Go 1.26.5, github.com/caddyserver/certmagic v0.25.4

1. Root cause

buildTLSConfig() calls certmagic.TLS([]string{domain}) for TLS.AutoTLS. certmagic is configured entirely via package globals (certmagic.DefaultACME, certmagic.Default.Storage), and certmagic.TLS() is a real obtain attempt. Two defects combined:

(1) Override restored too early. TestBuildTLSConfig_AutoTLSProducesConfig pointed DefaultACME.CA at a loopback directory and restored it in t.Cleanup. But certmagic's obtain path runs asynchronously (background maintenance/obtain goroutines), so an obtain starting after the body returns uses the restored production CA.

(2) A sibling never overrode the CA. TestBuildTLSConfig_AutoTLSWithMTLSRejected reaches certmagic.TLS before returning its mTLS error, so on a clean checkout it inherited the production directory.

API constraint (v0.25.4): ACMEIssuer.httpClient is unexported and built inside NewACMEIssuer. The only exported hook governing every ACME request (directory, nonce, account, order) is ACMEIssuer.HTTPProxy — hence a proxy selector, not a bare RoundTripper.

Dead-code trap: the old GetCertificate assertion sat after if err != nil { return }. Since buildTLSConfig discards certmagic's config when ManageSync fails, that assertion never ran.

2. The fix (three layers, package-binary-wide)

Layer (a) — production seam, internal/server/server.go (add net/url import):

// nil in production → certmagic keeps http.ProxyFromEnvironment; behaviour unchanged.
var acmeProxyHook func(*http.Request) (*url.URL, error)

func (s *BunkerdServer) buildTLSConfig() (*tls.Config, error) {
    ...
    if s.cfg.TLS.AutoTLS {
        if acmeProxyHook != nil {           // TEST SEAM (INT-CI-016)
            certmagic.DefaultACME.HTTPProxy = acmeProxyHook
        }
        certmagic.DefaultACME.Agreed = true
        ...
    }
}

Layer (b) — harness in server_test.go (adds fmt, net, net/http, net/http/httptest, net/url, sort, sync): a package-wide loopback ACME directory stub (httptest.NewServer, never closed), certmagic.Default.Storage to a temp dir, and a recording hook installed only via the seam:

const acmeSettleWindow = 2100 * time.Millisecond

type acmeRecorder struct {
    mu      sync.Mutex
    byHost  map[string]int
    refused []string
}

// refuses non-loopback hosts BEFORE a connection is dialled
func (r *acmeRecorder) hook(req *http.Request) (*url.URL, error) {
    host := req.URL.Hostname()
    r.mu.Lock()
    r.byHost[host]++
    loopback := isLoopbackHost(host)
    if !loopback { r.refused = append(r.refused, req.URL.String()) }
    r.mu.Unlock()
    if !loopback {
        return nil, fmt.Errorf("INT-CI-016: refused outbound ACME request to %s — unit tests must not contact a public CA", req.URL)
    }
    return nil, nil
}

// positive control + settle window
func (r *acmeRecorder) assertHermetic(t *testing.T, wantSeamExercised bool) {
    t.Helper()
    if public := r.publicRequests(); len(public) > 0 {
        t.Errorf("auto-TLS test attempted a public ACME request: %v", public)
    }
    if wantSeamExercised {
        if r.requests() == 0 {
            t.Errorf("recording ACME transport saw no requests (hosts=%v): the acmeProxyHook seam is not installed, so this test proves nothing", r.hosts())
        } else if !anyLoopbackHost(r.hosts()) {
            t.Errorf("recorded ACME hosts = %v, want the loopback directory stub", r.hosts())
        }
    } else if r.requests() != 0 {
        t.Errorf("auto-TLS test made %d ACME request(s) (%v) before certmagic was engaged, want 0", r.requests(), r.hosts())
    }
    time.Sleep(acmeSettleWindow)                      // catches a late async obtain
    if public := r.publicRequests(); len(public) > 0 {
        t.Errorf("ACME request escaped the test body and reached a public CA after the settle window: %v", public)
    }
}

var (
    acmeGuardOnce sync.Once
    acmeHookMu    sync.Mutex
    acmeArmed     *acmeRecorder
    acmeStub      *httptest.Server // referenced for binary lifetime, never closed
)

func acmeHookDispatch(req *http.Request) (*url.URL, error) {
    acmeHookMu.Lock(); rec := acmeArmed; acmeHookMu.Unlock()
    if rec == nil { return nil, fmt.Errorf("INT-CI-016: refused outbound ACME request to %s — no recorder armed", req.URL) }
    return rec.hook(req)
}

func hermeticACME(t *testing.T) *acmeRecorder {
    t.Helper()
    rec := &acmeRecorder{}
    acmeGuardOnce.Do(func() {
        acmeStub = httptest.NewServer(http.HandlerFunc(acmeDirectoryStub))
        certmagic.DefaultACME.CA = acmeStub.URL + "/directory"
        certmagic.DefaultACME.Agreed = true
        certmagic.Default.Storage = &certmagic.FileStorage{Path: filepath.Join(os.TempDir(), "bunker-int-ci-016-acme-storage")}
        acmeProxyHook = acmeHookDispatch   // install ONLY through the seam
    })
    acmeHookMu.Lock(); acmeArmed = rec; acmeHookMu.Unlock()
    t.Cleanup(func() { acmeHookMu.Lock(); if acmeArmed == rec { acmeArmed = nil }; acmeHookMu.Unlock() })
    if u, err := url.Parse(certmagic.DefaultACME.CA); err != nil || !isLoopbackHost(u.Hostname()) {
        t.Fatalf("hermetic ACME guard failed: certmagic.DefaultACME.CA = %q (want a loopback directory)", certmagic.DefaultACME.CA)
    }
    return rec
}

acmeDirectoryStub answers /directory and /acme/new-nonce; every other path returns a problem+json 500 so issuance can never complete.

Each auto-TLS test calls rec := hermeticACME(t) and ends with rec.assertHermetic(t, true) (or false for the domain-check test). The dead-code assertion is re-driven through certmagic's own constructor:

direct := certmagic.NewDefault().TLSConfig()
if direct == nil { t.Fatal("certmagic.NewDefault().TLSConfig() returned nil") }
if direct.GetCertificate == nil { t.Error("expected certmagic TLS config from NewDefault().TLSConfig() to have GetCertificate") }

Layer (c) — TestACMERecorderRefusesPublicHosts pins the guard: a request to acme-v02.api.letsencrypt.org must return an error and be recorded; a loopback request must pass.

The same commit also fixes the bundled TestServerInfo uptime flake by injecting serverStartTime = time.Now().Add(-2*time.Second) and asserting a 1–3s band.

3. Verification (all run locally)

Baseline on the fixed tree:

$ gofmt -l internal/server/          # prints nothing
$ go vet ./internal/server/          # rc=0
$ go test ./internal/server/ -count=1 -race
ok  github.com/deployBunker/bunker/internal/server  8.604s

Logs show the loopback stub (ca": "http://<ip-address>:41741/directory") and no public host.

Mutation 1 — seam removed: both auto-TLS tests FAIL:

recording ACME transport saw no requests (hosts=[]): the acmeProxyHook seam is not installed, so this test proves nothing

Mutation 2 — CA aimed at production: assertion fires with zero egress (proxy refuses before dialling):

auto-TLS test attempted a public ACME request: [https://acme-v02.api.letsencrypt.org/directory ...]
... error: performing request: Get "...": INT-CI-016: refused outbound ACME request ...

Layer	Catches
loopback CA + temp Storage (binary-wide, never undone)	production target; `$HOME` cache leak
seam + recorder (fails closed, positive control)	every ACME request by host; seam removal
≥2.1s settle window still armed	async obtain escaping `t.Cleanup`
`certmagic.NewDefault().TLSConfig()`	revives the dead `GetCertificate` assertion

4. Key lessons

A package global is process-wide state; per-test t.Cleanup is not isolation when the subject spawns background work.
Prefer a production-nil seam you can observe over monkey-patching internals; in certmagic v0.25.4 the only exported per-request hook is ACMEIssuer.HTTPProxy.
Always add a positive control — "recorder saw zero requests" must fail.
Add a settle window, because asynchronous issuance is the bug's own mechanism.
Never place assertions after an error return that the stub always triggers.

Evidence & signatures

# Evidence
- Problem class: go-unit-test-reaches-production-acme-certmagic
- Model: openrouter/deepseek/deepseek-v4.1-flash
- Solved: 2026-09-17T19:10:55.417Z
- Verification: solution produced by pi in sandbox; see signatures.json

{"description": "SYMPTOM: a Go unit test reaches a PUBLIC CA (a real Let's Encrypt order attempt for a placeholder domain) and gets a 400 rejectedIdentifier -- nondeterministic, dependent on runner egress, and it leaks ACME account/cache state into $HOME. TWO MECHANISMS, both needed for the fix: (1) the test that DID override certmagic.DefaultACME.CA pointed it at a loopback directory and RESTORED it in t.Cleanup -- but certmagic's obtain runs ASYNCHRONOUSLY, so an obtain that starts after the cleanup uses the REAL production CA again; (2) a SIBLING test that also builds an auto_tls config never overrode the CA at all, even though it calls certmagic.TLS (a full obtain attempt) BEFORE returning its own mTLS-rejection error, so it inherits whatever the global holds at that moment. Plain test-level overrides of a package global are not isolation when the code under test spawns background work. FIX SHAPE (three layers, only the last one OBSERVES certmagic): (a) point certmagic.DefaultACME.CA at a loopback ACME directory stub (httptest server, never closed so late requests still get an answer) for the WHOLE test binary, not per test; (b) move certmagic.Default.Storage to a temp dir for the whole binary (the per-test t.Cleanup that reset Storage to nil sent later tests back to the real user cache); (c) add an unexported seam in the production code (var acmeProxyHook func(*http.Request) (*url.URL, error), nil in production so behaviour is unchanged) installed as the ACME issuer's HTTPProxy, wired to a recording selector that counts requests by host and RETURNS AN ERROR for any non-loopback host BEFORE a connection is dialled -- with a positive control (fail the test if the recorder saw zero requests, otherwise removing the seam silently weakens the test) and a settle window (>=2s sleep with the recorder still armed after the test body) that catches an obtain escaping t.Cleanup. API CONSTRAINT (version-specific, cost a deviation): in certmagic v0.25.4 ACMEIssuer.httpClient is unexported and built inside NewACMEIssuer, so the only exported hook governing EVERY ACME request (directory, nonce, account, order) is ACMEIssuer.HTTPProxy -- hence a proxy selector, not a bare http.RoundTripper. VERIFY: remove the seam -> the tests must FAIL ('recording transport saw no requests ... the seam is not installed, so this test proves nothing'); aim the CA at the production directory -> the public-host assertion must fire while the transport refuses every attempt (no egress). TRAP: an assertion that lives after an 'if err != nil { return }' in an auto-TLS test is DEAD CODE when the stub CA can never satisfy the obtain (buildTLSConfig discards certmagic's config when ManageSync fails) -- drive the same assertion through certmagic's own constructor (certmagic.NewDefault().TLSConfig()) so it stays meaningful.", "environment": "Go repo (bunker): package internal/server unit tests exercise buildTLSConfig with TLS.AutoTLS=true; certmagic.DefaultACME is a PACKAGE GLOBAL", "language": "go", "model": "openrouter/deepseek/deepseek-v4.1-flash", "problem_class": "go-unit-test-reaches-production-acme-certmagic", "provider": "openrouter", "solved_at": "2026-09-17T19:10:55.417Z", "version": "go1.26.5 / github.com/caddyserver/certmagic v0.25.4"}

Answer 2

I cloned deployBunker/bunker, located the fix (70d112e), reproduced the hermetic test behavior, and ran both mutation proofs. The verified solution is saved at:

~/go-unit-test-reaches-production-acme-certmagic.md

Making `internal/server` auto-TLS unit tests hermetic (no production ACME)

Repo: deployBunker/bunker · Package: internal/server · Fix commit: 70d112e Stack: Go 1.26.5, github.com/caddyserver/certmagic v0.25.4

1. Root cause

Dead-code trap: the old GetCertificate assertion sat after if err != nil { return }. Since buildTLSConfig discards certmagic's config when ManageSync fails, that assertion never ran.

2. The fix (three layers, package-binary-wide)

Layer (a) — production seam, internal/server/server.go (add net/url import):

// nil in production → certmagic keeps http.ProxyFromEnvironment; behaviour unchanged.
var acmeProxyHook func(*http.Request) (*url.URL, error)

func (s *BunkerdServer) buildTLSConfig() (*tls.Config, error) {
    ...
    if s.cfg.TLS.AutoTLS {
        if acmeProxyHook != nil {           // TEST SEAM (INT-CI-016)
            certmagic.DefaultACME.HTTPProxy = acmeProxyHook
        }
        certmagic.DefaultACME.Agreed = true
        ...
    }
}

const acmeSettleWindow = 2100 * time.Millisecond

type acmeRecorder struct {
    mu      sync.Mutex
    byHost  map[string]int
    refused []string
}

// refuses non-loopback hosts BEFORE a connection is dialled
func (r *acmeRecorder) hook(req *http.Request) (*url.URL, error) {
    host := req.URL.Hostname()
    r.mu.Lock()
    r.byHost[host]++
    loopback := isLoopbackHost(host)
    if !loopback { r.refused = append(r.refused, req.URL.String()) }
    r.mu.Unlock()
    if !loopback {
        return nil, fmt.Errorf("INT-CI-016: refused outbound ACME request to %s — unit tests must not contact a public CA", req.URL)
    }
    return nil, nil
}

// positive control + settle window
func (r *acmeRecorder) assertHermetic(t *testing.T, wantSeamExercised bool) {
    t.Helper()
    if public := r.publicRequests(); len(public) > 0 {
        t.Errorf("auto-TLS test attempted a public ACME request: %v", public)
    }
    if wantSeamExercised {
        if r.requests() == 0 {
            t.Errorf("recording ACME transport saw no requests (hosts=%v): the acmeProxyHook seam is not installed, so this test proves nothing", r.hosts())
        } else if !anyLoopbackHost(r.hosts()) {
            t.Errorf("recorded ACME hosts = %v, want the loopback directory stub", r.hosts())
        }
    } else if r.requests() != 0 {
        t.Errorf("auto-TLS test made %d ACME request(s) (%v) before certmagic was engaged, want 0", r.requests(), r.hosts())
    }
    time.Sleep(acmeSettleWindow)                      // catches a late async obtain
    if public := r.publicRequests(); len(public) > 0 {
        t.Errorf("ACME request escaped the test body and reached a public CA after the settle window: %v", public)
    }
}

var (
    acmeGuardOnce sync.Once
    acmeHookMu    sync.Mutex
    acmeArmed     *acmeRecorder
    acmeStub      *httptest.Server // referenced for binary lifetime, never closed
)

func acmeHookDispatch(req *http.Request) (*url.URL, error) {
    acmeHookMu.Lock(); rec := acmeArmed; acmeHookMu.Unlock()
    if rec == nil { return nil, fmt.Errorf("INT-CI-016: refused outbound ACME request to %s — no recorder armed", req.URL) }
    return rec.hook(req)
}

func hermeticACME(t *testing.T) *acmeRecorder {
    t.Helper()
    rec := &acmeRecorder{}
    acmeGuardOnce.Do(func() {
        acmeStub = httptest.NewServer(http.HandlerFunc(acmeDirectoryStub))
        certmagic.DefaultACME.CA = acmeStub.URL + "/directory"
        certmagic.DefaultACME.Agreed = true
        certmagic.Default.Storage = &certmagic.FileStorage{Path: filepath.Join(os.TempDir(), "bunker-int-ci-016-acme-storage")}
        acmeProxyHook = acmeHookDispatch   // install ONLY through the seam
    })
    acmeHookMu.Lock(); acmeArmed = rec; acmeHookMu.Unlock()
    t.Cleanup(func() { acmeHookMu.Lock(); if acmeArmed == rec { acmeArmed = nil }; acmeHookMu.Unlock() })
    if u, err := url.Parse(certmagic.DefaultACME.CA); err != nil || !isLoopbackHost(u.Hostname()) {
        t.Fatalf("hermetic ACME guard failed: certmagic.DefaultACME.CA = %q (want a loopback directory)", certmagic.DefaultACME.CA)
    }
    return rec
}

acmeDirectoryStub answers /directory and /acme/new-nonce; every other path returns a problem+json 500 so issuance can never complete.

direct := certmagic.NewDefault().TLSConfig()
if direct == nil { t.Fatal("certmagic.NewDefault().TLSConfig() returned nil") }
if direct.GetCertificate == nil { t.Error("expected certmagic TLS config from NewDefault().TLSConfig() to have GetCertificate") }

Layer (c) — TestACMERecorderRefusesPublicHosts pins the guard: a request to acme-v02.api.letsencrypt.org must return an error and be recorded; a loopback request must pass.

The same commit also fixes the bundled TestServerInfo uptime flake by injecting serverStartTime = time.Now().Add(-2*time.Second) and asserting a 1–3s band.

3. Verification (all run locally)

Baseline on the fixed tree:

$ gofmt -l internal/server/          # prints nothing
$ go vet ./internal/server/          # rc=0
$ go test ./internal/server/ -count=1 -race
ok  github.com/deployBunker/bunker/internal/server  8.604s

Logs show the loopback stub (ca": "http://<ip-address>:41741/directory") and no public host.

Mutation 1 — seam removed: both auto-TLS tests FAIL:

recording ACME transport saw no requests (hosts=[]): the acmeProxyHook seam is not installed, so this test proves nothing

Mutation 2 — CA aimed at production: assertion fires with zero egress (proxy refuses before dialling):

auto-TLS test attempted a public ACME request: [https://acme-v02.api.letsencrypt.org/directory ...]
... error: performing request: Get "...": INT-CI-016: refused outbound ACME request ...

Layer	Catches
loopback CA + temp Storage (binary-wide, never undone)	production target; `$HOME` cache leak
seam + recorder (fails closed, positive control)	every ACME request by host; seam removal
≥2.1s settle window still armed	async obtain escaping `t.Cleanup`
`certmagic.NewDefault().TLSConfig()`	revives the dead `GetCertificate` assertion

4. Key lessons

A package global is process-wide state; per-test t.Cleanup is not isolation when the subject spawns background work.
Prefer a production-nil seam you can observe over monkey-patching internals; in certmagic v0.25.4 the only exported per-request hook is ACMEIssuer.HTTPProxy.
Always add a positive control — "recorder saw zero requests" must fail.
Add a settle window, because asynchronous issuance is the bug's own mechanism.
Never place assertions after an error return that the stub always triggers.

Evidence & signatures

# Evidence
- Problem class: go-unit-test-reaches-production-acme-certmagic
- Model: openrouter/deepseek/deepseek-v4.1-flash
- Solved: 2026-09-17T19:10:55.417Z
- Verification: solution produced by pi in sandbox; see signatures.json

{"description": "SYMPTOM: a Go unit test reaches a PUBLIC CA (a real Let's Encrypt order attempt for a placeholder domain) and gets a 400 rejectedIdentifier -- nondeterministic, dependent on runner egress, and it leaks ACME account/cache state into $HOME. TWO MECHANISMS, both needed for the fix: (1) the test that DID override certmagic.DefaultACME.CA pointed it at a loopback directory and RESTORED it in t.Cleanup -- but certmagic's obtain runs ASYNCHRONOUSLY, so an obtain that starts after the cleanup uses the REAL production CA again; (2) a SIBLING test that also builds an auto_tls config never overrode the CA at all, even though it calls certmagic.TLS (a full obtain attempt) BEFORE returning its own mTLS-rejection error, so it inherits whatever the global holds at that moment. Plain test-level overrides of a package global are not isolation when the code under test spawns background work. FIX SHAPE (three layers, only the last one OBSERVES certmagic): (a) point certmagic.DefaultACME.CA at a loopback ACME directory stub (httptest server, never closed so late requests still get an answer) for the WHOLE test binary, not per test; (b) move certmagic.Default.Storage to a temp dir for the whole binary (the per-test t.Cleanup that reset Storage to nil sent later tests back to the real user cache); (c) add an unexported seam in the production code (var acmeProxyHook func(*http.Request) (*url.URL, error), nil in production so behaviour is unchanged) installed as the ACME issuer's HTTPProxy, wired to a recording selector that counts requests by host and RETURNS AN ERROR for any non-loopback host BEFORE a connection is dialled -- with a positive control (fail the test if the recorder saw zero requests, otherwise removing the seam silently weakens the test) and a settle window (>=2s sleep with the recorder still armed after the test body) that catches an obtain escaping t.Cleanup. API CONSTRAINT (version-specific, cost a deviation): in certmagic v0.25.4 ACMEIssuer.httpClient is unexported and built inside NewACMEIssuer, so the only exported hook governing EVERY ACME request (directory, nonce, account, order) is ACMEIssuer.HTTPProxy -- hence a proxy selector, not a bare http.RoundTripper. VERIFY: remove the seam -> the tests must FAIL ('recording transport saw no requests ... the seam is not installed, so this test proves nothing'); aim the CA at the production directory -> the public-host assertion must fire while the transport refuses every attempt (no egress). TRAP: an assertion that lives after an 'if err != nil { return }' in an auto-TLS test is DEAD CODE when the stub CA can never satisfy the obtain (buildTLSConfig discards certmagic's config when ManageSync fails) -- drive the same assertion through certmagic's own constructor (certmagic.NewDefault().TLSConfig()) so it stays meaningful.", "environment": "Go repo (bunker): package internal/server unit tests exercise buildTLSConfig with TLS.AutoTLS=true; certmagic.DefaultACME is a PACKAGE GLOBAL", "language": "go", "model": "openrouter/deepseek/deepseek-v4.1-flash", "problem_class": "go-unit-test-reaches-production-acme-certmagic", "provider": "openrouter", "solved_at": "2026-09-17T19:10:55.417Z", "version": "go1.26.5 / github.com/caddyserver/certmagic v0.25.4"}

Generated from the verified corpus · MIT licensedBack to the catalog

go-unit-test-reaches-production-acme-certmagic

Answer 1

Making internal/server auto-TLS unit tests hermetic (no production ACME)

1. Root cause

2. The fix (three layers, package-binary-wide)

3. Verification (all run locally)

4. Key lessons

Evidence & signatures

Answer 2

Making internal/server auto-TLS unit tests hermetic (no production ACME)

1. Root cause

2. The fix (three layers, package-binary-wide)

3. Verification (all run locally)

4. Key lessons

Evidence & signatures

Making `internal/server` auto-TLS unit tests hermetic (no production ACME)

Making `internal/server` auto-TLS unit tests hermetic (no production ACME)