3d6e47e Address public TLS review findings · gordon

M docs/cli/tls.md +4 -4


@@ -38,8 +38,8 @@ gordon tls status --remote https://gordon.mydomain.com --token $TOKEN
38	38		Public TLS / ACME Status
39	39
40	40		ACME: enabled
41		-	Configured Mode: acme
42		-	Effective Mode: acme-staging
	41	+	Configured Mode: auto
	42	+	Effective Mode: http-01
43	43		Reason: configured
44	44		Token Source: env
45	45

@@ -62,8 +62,8 @@ Errors
62	62		```json
63	63		{
64	64		"acme_enabled": true,
65		-	"configured_mode": "acme",
66		-	"effective_mode": "acme-staging",
	65	+	"configured_mode": "auto",
	66	+	"effective_mode": "http-01",
67	67		"selection_reason": "configured",
68	68		"token_source": "env",
69	69		"certificates": [


@@ -38,8 +38,8 @@ gordon tls status --remote https://gordon.mydomain.com --token $TOKEN
38		Public TLS / ACME Status	38		Public TLS / ACME Status
39			39
40		ACME: enabled	40		ACME: enabled
41	-	Configured Mode: acme	41	+	Configured Mode: auto
42	-	Effective Mode: acme-staging	42	+	Effective Mode: http-01
43		Reason: configured	43		Reason: configured
44		Token Source: env	44		Token Source: env
45			45

@@ -62,8 +62,8 @@ Errors
62		```json	62		```json
63		{	63		{
64		"acme_enabled": true,	64		"acme_enabled": true,
65	-	"configured_mode": "acme",	65	+	"configured_mode": "auto",
66	-	"effective_mode": "acme-staging",	66	+	"effective_mode": "http-01",
67		"selection_reason": "configured",	67		"selection_reason": "configured",
68		"token_source": "env",	68		"token_source": "env",
69		"certificates": [	69		"certificates": [

M internal/adapters/in/http/acme/handler.go +5 -1


@@ -36,8 +36,12 @@ func (h Handler) ServeHTTP(w http.ResponseWriter, r http.Request) {
36	36		return
37	37		}
38	38
	39	+	if !strings.HasPrefix(r.URL.Path, Prefix) {
	40	+	http.NotFound(w, r)
	41	+	return
	42	+	}
39	43		token := strings.TrimPrefix(r.URL.Path, Prefix)
40		-	if !strings.HasPrefix(r.URL.Path, Prefix) \|\| !safeToken(token) {
	44	+	if !safeToken(token) {
41	45		http.NotFound(w, r)
42	46		return
43	47		}

 		return
 	}
+	if !strings.HasPrefix(r.URL.Path, Prefix) {
+		http.NotFound(w, r)
+		return
+	}
 	token := strings.TrimPrefix(r.URL.Path, Prefix)
-	if !strings.HasPrefix(r.URL.Path, Prefix) || !safeToken(token) {
+	if !safeToken(token) {
 		http.NotFound(w, r)
 		return
 	}

M internal/adapters/in/http/admin/handler.go +4 -1


@@ -1924,7 +1924,10 @@ func (h Handler) handleTLSStatus(w http.ResponseWriter, r http.Request) {
1924	1924		}
1925	1925
1926	1926		if h.publicTLSSvc == nil {
1927		-	h.sendJSON(w, http.StatusOK, dto.TLSStatusResponse{})
	1927	+	h.sendJSON(w, http.StatusOK, dto.TLSStatusResponse{
	1928	+	ACMEEnabled: false,
	1929	+	SelectionReason: "public TLS service not configured",
	1930	+	})
1928	1931		return
1929	1932		}
1930	1933

 	}
 	if h.publicTLSSvc == nil {
-		h.sendJSON(w, http.StatusOK, dto.TLSStatusResponse{})
+		h.sendJSON(w, http.StatusOK, dto.TLSStatusResponse{
+			ACMEEnabled:     false,
+			SelectionReason: "public TLS service not configured",
+		})
 		return
 	}

M internal/adapters/in/http/admin/handler_test.go +8 -6


@@ -2591,14 +2591,16 @@ func TestHandler_TLSStatus_RequiresStatusReadScope(t *testing.T) {
2591	2591		d.PublicTLSSvc = inmocks.NewMockPublicTLSService(t)
2592	2592		})
2593	2593
2594		-	req := httptest.NewRequest("GET", "/admin/tls/status", nil)
2595		-	req = req.WithContext(ctxWithScopes("admin:config:write"))
2596		-	rec := httptest.NewRecorder()
	2594	+	server := newScopedTestServer(t, handler, "admin:config:write")
	2595	+	resp, err := http.Get(server.URL + "/admin/tls/status")
	2596	+	require.NoError(t, err)
	2597	+	defer resp.Body.Close()
2597	2598
2598		-	handler.ServeHTTP(rec, req)
	2599	+	body, err := io.ReadAll(resp.Body)
	2600	+	require.NoError(t, err)
2599	2601
2600		-	assert.Equal(t, http.StatusForbidden, rec.Code)
2601		-	assert.Contains(t, rec.Body.String(), "insufficient permissions for status:read")
	2602	+	assert.Equal(t, http.StatusForbidden, resp.StatusCode)
	2603	+	assert.Contains(t, string(body), "insufficient permissions for status:read")
2602	2604		}
2603	2605
2604	2606		func TestHandler_TLSStatus_GETReturnsJSON(t *testing.T) {


@@ -2591,14 +2591,16 @@ func TestHandler_TLSStatus_RequiresStatusReadScope(t *testing.T) {
2591		d.PublicTLSSvc = inmocks.NewMockPublicTLSService(t)	2591		d.PublicTLSSvc = inmocks.NewMockPublicTLSService(t)
2592		})	2592		})
2593			2593
2594	-	req := httptest.NewRequest("GET", "/admin/tls/status", nil)	2594	+	server := newScopedTestServer(t, handler, "admin:config:write")
2595	-	req = req.WithContext(ctxWithScopes("admin:config:write"))	2595	+	resp, err := http.Get(server.URL + "/admin/tls/status")
2596	-	rec := httptest.NewRecorder()	2596	+	require.NoError(t, err)
			2597	+	defer resp.Body.Close()
2597			2598
2598	-	handler.ServeHTTP(rec, req)	2599	+	body, err := io.ReadAll(resp.Body)
			2600	+	require.NoError(t, err)
2599			2601
2600	-	assert.Equal(t, http.StatusForbidden, rec.Code)	2602	+	assert.Equal(t, http.StatusForbidden, resp.StatusCode)
2601	-	assert.Contains(t, rec.Body.String(), "insufficient permissions for status:read")	2603	+	assert.Contains(t, string(body), "insufficient permissions for status:read")
2602		}	2604		}
2603			2605
2604		func TestHandler_TLSStatus_GETReturnsJSON(t *testing.T) {	2606		func TestHandler_TLSStatus_GETReturnsJSON(t *testing.T) {

M internal/adapters/out/acmelego/cloudflare.go +21 -5


@@ -42,7 +42,9 @@ type CloudflareZoneResolver struct {
42	42		baseURL string
43	43		client *http.Client
44	44
45		-	mu sync.RWMutex
	45	+	mu sync.RWMutex
	46	+	// cache uses a small fixed maximum size; eviction scans the map to keep the
	47	+	// resolver simple because defaultCacheMaxSize is intentionally low.
46	48		cache map[string]zoneCacheEntry
47	49		}
48	50

@@ -50,10 +52,12 @@ type zoneCacheEntry struct {
50	52		zone out.CloudflareZone
51	53		err error
52	54		createdAt time.Time
	55	+	ttl time.Duration
53	56		}
54	57
55	58		const (
56	59		defaultCacheTTL = 10 * time.Minute
	60	+	defaultErrorCacheTTL = time.Minute
57	61		defaultCloudflareTimeout = 10 * time.Second
58	62		defaultCacheMaxSize = 100
59	63		defaultMaxResponseSize = 1 << 20 // 1 MB

@@ -149,14 +153,21 @@ func (r *CloudflareZoneResolver) FindZone(ctx context.Context, domainName string
149	153
150	154		func (r *CloudflareZoneResolver) cached(name string) (zoneCacheEntry, bool) {
151	155		r.mu.RLock()
152		-	defer r.mu.RUnlock()
153	156		entry, ok := r.cache[name]
154	157		if !ok {
	158	+	r.mu.RUnlock()
155	159		return entry, false
156	160		}
157		-	if time.Since(entry.createdAt) > defaultCacheTTL {
	161	+	if time.Since(entry.createdAt) > entry.ttl {
	162	+	r.mu.RUnlock()
	163	+	r.mu.Lock()
	164	+	if current, ok := r.cache[name]; ok && current.createdAt.Equal(entry.createdAt) {
	165	+	delete(r.cache, name)
	166	+	}
	167	+	r.mu.Unlock()
158	168		return zoneCacheEntry{}, false
159	169		}
	170	+	r.mu.RUnlock()
160	171		return entry, true
161	172		}
162	173

@@ -164,7 +175,8 @@ func (r *CloudflareZoneResolver) storeCache(name string, zone out.CloudflareZone
164	175		r.mu.Lock()
165	176		defer r.mu.Unlock()
166	177		if len(r.cache) >= defaultCacheMaxSize {
167		-	// Evict oldest entry
	178	+	// Evict oldest entry with an O(n) scan. The cache is deliberately small
	179	+	// (defaultCacheMaxSize), so this stays simple without an LRU list.
168	180		var oldestKey string
169	181		var oldestTime time.Time
170	182		for k, v := range r.cache {

@@ -175,7 +187,11 @@ func (r *CloudflareZoneResolver) storeCache(name string, zone out.CloudflareZone
175	187		}
176	188		delete(r.cache, oldestKey)
177	189		}
178		-	r.cache[name] = zoneCacheEntry{zone: zone, err: err, createdAt: time.Now()}
	190	+	ttl := defaultCacheTTL
	191	+	if err != nil {
	192	+	ttl = defaultErrorCacheTTL
	193	+	}
	194	+	r.cache[name] = zoneCacheEntry{zone: zone, err: err, createdAt: time.Now(), ttl: ttl}
179	195		}
180	196
181	197		// findZoneByName queries the Cloudflare API for an active zone with the exact given name.

 	baseURL string
 	client  *http.Client
-	mu    sync.RWMutex
+	mu sync.RWMutex
+	// cache uses a small fixed maximum size; eviction scans the map to keep the
+	// resolver simple because defaultCacheMaxSize is intentionally low.
 	cache map[string]zoneCacheEntry
 }
 	zone      out.CloudflareZone
 	err       error
 	createdAt time.Time
+	ttl       time.Duration
 }
 const (
 	defaultCacheTTL          = 10 * time.Minute
+	defaultErrorCacheTTL     = time.Minute
 	defaultCloudflareTimeout = 10 * time.Second
 	defaultCacheMaxSize      = 100
 	defaultMaxResponseSize   = 1 << 20 // 1 MB
 func (r *CloudflareZoneResolver) cached(name string) (zoneCacheEntry, bool) {
 	r.mu.RLock()
-	defer r.mu.RUnlock()
 	entry, ok := r.cache[name]
 	if !ok {
+		r.mu.RUnlock()
 		return entry, false
 	}
-	if time.Since(entry.createdAt) > defaultCacheTTL {
+	if time.Since(entry.createdAt) > entry.ttl {
+		r.mu.RUnlock()
+		r.mu.Lock()
+		if current, ok := r.cache[name]; ok && current.createdAt.Equal(entry.createdAt) {
+			delete(r.cache, name)
+		}
+		r.mu.Unlock()
 		return zoneCacheEntry{}, false
 	}
+	r.mu.RUnlock()
 	return entry, true
 }
 	r.mu.Lock()
 	defer r.mu.Unlock()
 	if len(r.cache) >= defaultCacheMaxSize {
-		// Evict oldest entry
+		// Evict oldest entry with an O(n) scan. The cache is deliberately small
+		// (defaultCacheMaxSize), so this stays simple without an LRU list.
 		var oldestKey string
 		var oldestTime time.Time
 		for k, v := range r.cache {
 		}
 		delete(r.cache, oldestKey)
 	}
-	r.cache[name] = zoneCacheEntry{zone: zone, err: err, createdAt: time.Now()}
+	ttl := defaultCacheTTL
+	if err != nil {
+		ttl = defaultErrorCacheTTL
+	}
+	r.cache[name] = zoneCacheEntry{zone: zone, err: err, createdAt: time.Now(), ttl: ttl}
 }
 // findZoneByName queries the Cloudflare API for an active zone with the exact given name.

M internal/adapters/out/acmelego/cloudflare_test.go +11 -0


@@ -3,6 +3,7 @@ package acmelego
3	3		import (
4	4		"context"
5	5		"encoding/json"
	6	+	"errors"
6	7		"net/http"
7	8		"net/http/httptest"
8	9		"testing"

@@ -136,6 +137,16 @@ func TestCloudflareZoneResolverNoZoneFound(t *testing.T) {
136	137		assert.Contains(t, err.Error(), "no active zone found")
137	138		}
138	139
	140	+	func TestCloudflareZoneResolverCachesErrorsWithShortTTL(t *testing.T) {
	141	+	resolver := NewCloudflareZoneResolver("token")
	142	+	resolver.storeCache("example.com", out.CloudflareZone{}, errors.New("temporary failure"))
	143	+
	144	+	entry, ok := resolver.cached("example.com")
	145	+	require.True(t, ok)
	146	+	assert.Error(t, entry.err)
	147	+	assert.Equal(t, defaultErrorCacheTTL, entry.ttl)
	148	+	}
	149	+
139	150		func TestCloudflareZoneResolverUnsuccessfulAPI(t *testing.T) {
140	151		server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
141	152		w.Header().Set("Content-Type", "application/json")


@@ -3,6 +3,7 @@ package acmelego
3	import (	3		import (
4	"context"	4		"context"
5	"encoding/json"	5		"encoding/json"
		6	+	"errors"
6	"net/http"	7		"net/http"
7	"net/http/httptest"	8		"net/http/httptest"
8	"testing"	9		"testing"

@@ -136,6 +137,16 @@ func TestCloudflareZoneResolverNoZoneFound(t *testing.T) {
136	assert.Contains(t, err.Error(), "no active zone found")	137		assert.Contains(t, err.Error(), "no active zone found")
137	}	138		}
138		139
		140	+	func TestCloudflareZoneResolverCachesErrorsWithShortTTL(t *testing.T) {
		141	+	resolver := NewCloudflareZoneResolver("token")
		142	+	resolver.storeCache("example.com", out.CloudflareZone{}, errors.New("temporary failure"))
		143	+
		144	+	entry, ok := resolver.cached("example.com")
		145	+	require.True(t, ok)
		146	+	assert.Error(t, entry.err)
		147	+	assert.Equal(t, defaultErrorCacheTTL, entry.ttl)
		148	+	}
		149	+
139	func TestCloudflareZoneResolverUnsuccessfulAPI(t *testing.T) {	150		func TestCloudflareZoneResolverUnsuccessfulAPI(t *testing.T) {
140	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {	151		server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
141	w.Header().Set("Content-Type", "application/json")	152		w.Header().Set("Content-Type", "application/json")

M internal/adapters/out/acmelego/http_provider.go +7 -5

@@ -1,7 +1,11 @@
1	1		// Package acmelego implements ACME certificate operations using the lego library.
2	2		package acmelego
3	3
4		-	import "github.com/bnema/gordon/internal/boundaries/out"
	4	+	import (
	5	+	"fmt"
	6	+
	7	+	"github.com/bnema/gordon/internal/boundaries/out"
	8	+	)
5	9
6	10		// HTTPProvider adapts an out.HTTPChallengeSink to the lego challenge.Provider
7	11		// interface for HTTP-01 challenge solving.

@@ -15,11 +19,9 @@ func NewHTTPProvider(sink out.HTTPChallengeSink) *HTTPProvider {
15	19		}
16	20
17	21		// Present stores the key authorization for the given token via the sink.
18		-	// If the sink is nil, it returns nil (no-op) to allow graceful degradation
19		-	// in test or misconfigured scenarios.
20	22		func (p *HTTPProvider) Present(_domain, token, keyAuth string) error {
21	23		if p.sink == nil {
22		-	return nil
	24	+	return fmt.Errorf("http-01 sink is nil: cannot Present")
23	25		}
24	26		p.sink.Present(token, keyAuth)
25	27		return nil

@@ -28,7 +30,7 @@ func (p *HTTPProvider) Present(_domain, token, keyAuth string) error {
28	30		// CleanUp removes the key authorization for the given token via the sink.
29	31		func (p *HTTPProvider) CleanUp(_domain, token, _keyAuth string) error {
30	32		if p.sink == nil {
31		-	return nil
	33	+	return fmt.Errorf("http-01 sink is nil: cannot CleanUp")
32	34		}
33	35		p.sink.CleanUp(token)
34	36		return nil

 // Package acmelego implements ACME certificate operations using the lego library.
 package acmelego
-import "github.com/bnema/gordon/internal/boundaries/out"
+import (
+	"fmt"
+	"github.com/bnema/gordon/internal/boundaries/out"
+)
 // HTTPProvider adapts an out.HTTPChallengeSink to the lego challenge.Provider
 // interface for HTTP-01 challenge solving.
 }
 // Present stores the key authorization for the given token via the sink.
-// If the sink is nil, it returns nil (no-op) to allow graceful degradation
-// in test or misconfigured scenarios.
 func (p *HTTPProvider) Present(_domain, token, keyAuth string) error {
 	if p.sink == nil {
-		return nil
+		return fmt.Errorf("http-01 sink is nil: cannot Present")
 	}
 	p.sink.Present(token, keyAuth)
 	return nil
 // CleanUp removes the key authorization for the given token via the sink.
 func (p *HTTPProvider) CleanUp(_domain, token, _keyAuth string) error {
 	if p.sink == nil {
-		return nil
+		return fmt.Errorf("http-01 sink is nil: cannot CleanUp")
 	}
 	p.sink.CleanUp(token)
 	return nil

M internal/adapters/out/acmelego/http_provider_test.go +11 -22


@@ -4,43 +4,32 @@ import (
4	4		"testing"
5	5
6	6		"github.com/stretchr/testify/assert"
7		-	)
8		-
9		-	// challengeSink is a test sink that stores challenge tokens in a map.
10		-	type challengeSink struct {
11		-	values map[string]string
12		-	}
13		-
14		-	func (s *challengeSink) Present(token, keyAuth string) {
15		-	if s.values == nil {
16		-	s.values = make(map[string]string)
17		-	}
18		-	s.values[token] = keyAuth
19		-	}
20	7
21		-	func (s *challengeSink) CleanUp(token string) {
22		-	delete(s.values, token)
23		-	}
	8	+	outmocks "github.com/bnema/gordon/internal/boundaries/out/mocks"
	9	+	)
24	10
25	11		func TestHTTPProviderStoresAndCleansChallenge(t *testing.T) {
26		-	sink := &challengeSink{}
27		-	provider := NewHTTPProvider(sink)
	12	+	mockSink := outmocks.NewMockHTTPChallengeSink(t)
	13	+	mockSink.EXPECT().Present("token", "key-auth").Return()
	14	+	mockSink.EXPECT().CleanUp("token").Return()
	15	+
	16	+	provider := NewHTTPProvider(mockSink)
28	17
29	18		err := provider.Present("example.com", "token", "key-auth")
30	19		assert.NoError(t, err)
31		-	assert.Equal(t, "key-auth", sink.values["token"])
32	20
33	21		err = provider.CleanUp("example.com", "token", "key-auth")
34	22		assert.NoError(t, err)
35		-	assert.Empty(t, sink.values["token"])
36	23		}
37	24
38	25		func TestHTTPProviderNilSink(t *testing.T) {
39	26		provider := NewHTTPProvider(nil)
40	27
41	28		err := provider.Present("example.com", "token", "key-auth")
42		-	assert.NoError(t, err)
	29	+	assert.Error(t, err)
	30	+	assert.Contains(t, err.Error(), "sink is nil")
43	31
44	32		err = provider.CleanUp("example.com", "token", "key-auth")
45		-	assert.NoError(t, err)
	33	+	assert.Error(t, err)
	34	+	assert.Contains(t, err.Error(), "sink is nil")
46	35		}

 	"testing"
 	"github.com/stretchr/testify/assert"
-)
-// challengeSink is a test sink that stores challenge tokens in a map.
-type challengeSink struct {
-	values map[string]string
-}
-func (s *challengeSink) Present(token, keyAuth string) {
-	if s.values == nil {
-		s.values = make(map[string]string)
-	}
-	s.values[token] = keyAuth
-}
-func (s *challengeSink) CleanUp(token string) {
+	outmocks "github.com/bnema/gordon/internal/boundaries/out/mocks"
-	delete(s.values, token)
+)
-}
 func TestHTTPProviderStoresAndCleansChallenge(t *testing.T) {
-	sink := &challengeSink{}
+	mockSink := outmocks.NewMockHTTPChallengeSink(t)
-	provider := NewHTTPProvider(sink)
+	mockSink.EXPECT().Present("token", "key-auth").Return()
+	mockSink.EXPECT().CleanUp("token").Return()
+	provider := NewHTTPProvider(mockSink)
 	err := provider.Present("example.com", "token", "key-auth")
 	assert.NoError(t, err)
-	assert.Equal(t, "key-auth", sink.values["token"])
 	err = provider.CleanUp("example.com", "token", "key-auth")
 	assert.NoError(t, err)
-	assert.Empty(t, sink.values["token"])
 }
 func TestHTTPProviderNilSink(t *testing.T) {
 	provider := NewHTTPProvider(nil)
 	err := provider.Present("example.com", "token", "key-auth")
-	assert.NoError(t, err)
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "sink is nil")
 	err = provider.CleanUp("example.com", "token", "key-auth")
-	assert.NoError(t, err)
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "sink is nil")
 }

M internal/adapters/out/acmelego/issuer.go +9 -7


@@ -8,6 +8,7 @@ import (
8	8		"encoding/json"
9	9		"fmt"
10	10		"net/http"
	11	+	"strings"
11	12		"sync"
12	13		"time"
13	14

@@ -63,6 +64,9 @@ func NewIssuer(cfg Config) (*Issuer, error) {
63	64		if cfg.Store == nil {
64	65		return nil, fmt.Errorf("acmelego: %w", domain.ErrCertificateStoreRequired)
65	66		}
	67	+	if cfg.CADirectoryURL != "" && !strings.HasPrefix(cfg.CADirectoryURL, "https://") {
	68	+	return nil, fmt.Errorf("acmelego: CADirectoryURL must use HTTPS, got %q", cfg.CADirectoryURL)
	69	+	}
66	70		if cfg.Challenge == "" {
67	71		cfg.Challenge = domain.ACMEChallengeHTTP01
68	72		}

@@ -110,13 +114,13 @@ func (i Issuer) Renew(ctx context.Context, cert out.StoredCertificate) (out.St
110	114		return nil, err
111	115		}
112	116
113		-	domain := ""
	117	+	domainName := ""
114	118		if len(cert.Names) > 0 {
115		-	domain = cert.Names[0]
	119	+	domainName = cert.Names[0]
116	120		}
117	121
118	122		legoCert := certificate.Resource{
119		-	Domain: domain,
	123	+	Domain: domainName,
120	124		PrivateKey: cert.PrivateKeyPEM,
121	125		Certificate: cert.CertPEM,
122	126		IssuerCertificate: cert.ChainPEM,

@@ -168,10 +172,8 @@ func (i *Issuer) ensureClient(ctx context.Context) error {
168	172		// Set up challenge providers
169	173		switch i.cfg.Challenge {
170	174		case domain.ACMEChallengeHTTP01:
171		-	if i.cfg.HTTPChallengeSink != nil {
172		-	if err := client.Challenge.SetHTTP01Provider(NewHTTPProvider(i.cfg.HTTPChallengeSink)); err != nil {
173		-	return fmt.Errorf("set http-01 provider: %w", err)
174		-	}
	175	+	if err := client.Challenge.SetHTTP01Provider(NewHTTPProvider(i.cfg.HTTPChallengeSink)); err != nil {
	176	+	return fmt.Errorf("set http-01 provider: %w", err)
175	177		}
176	178		case domain.ACMEChallengeCloudflareDNS01:
177	179		cfCfg := cloudflare.NewDefaultConfig()

 	"encoding/json"
 	"fmt"
 	"net/http"
+	"strings"
 	"sync"
 	"time"
 	if cfg.Store == nil {
 		return nil, fmt.Errorf("acmelego: %w", domain.ErrCertificateStoreRequired)
 	}
+	if cfg.CADirectoryURL != "" && !strings.HasPrefix(cfg.CADirectoryURL, "https://") {
+		return nil, fmt.Errorf("acmelego: CADirectoryURL must use HTTPS, got %q", cfg.CADirectoryURL)
+	}
 	if cfg.Challenge == "" {
 		cfg.Challenge = domain.ACMEChallengeHTTP01
 	}
 		return nil, err
 	}
-	domain := ""
+	domainName := ""
 	if len(cert.Names) > 0 {
-		domain = cert.Names[0]
+		domainName = cert.Names[0]
 	}
 	legoCert := certificate.Resource{
-		Domain:            domain,
+		Domain:            domainName,
 		PrivateKey:        cert.PrivateKeyPEM,
 		Certificate:       cert.CertPEM,
 		IssuerCertificate: cert.ChainPEM,
 	// Set up challenge providers
 	switch i.cfg.Challenge {
 	case domain.ACMEChallengeHTTP01:
-		if i.cfg.HTTPChallengeSink != nil {
+		if err := client.Challenge.SetHTTP01Provider(NewHTTPProvider(i.cfg.HTTPChallengeSink)); err != nil {
-			if err := client.Challenge.SetHTTP01Provider(NewHTTPProvider(i.cfg.HTTPChallengeSink)); err != nil {
+			return fmt.Errorf("set http-01 provider: %w", err)
-				return fmt.Errorf("set http-01 provider: %w", err)
-			}
 		}
 	case domain.ACMEChallengeCloudflareDNS01:
 		cfCfg := cloudflare.NewDefaultConfig()

M internal/adapters/out/acmelego/issuer_test.go +22 -0


@@ -69,6 +69,28 @@ func TestNewIssuerValidatesCloudflareToken(t *testing.T) {
69	69		assert.ErrorIs(t, err, domain.ErrCloudflareTokenMissing)
70	70		}
71	71
	72	+	func TestNewIssuerRejectsNonHTTPSURL(t *testing.T) {
	73	+	_, err := NewIssuer(Config{
	74	+	Email: "test@example.com",
	75	+	Challenge: domain.ACMEChallengeHTTP01,
	76	+	Store: outmocks.NewMockCertificateStore(t),
	77	+	HTTPChallengeSink: outmocks.NewMockHTTPChallengeSink(t),
	78	+	CADirectoryURL: "http://acme.test/directory",
	79	+	})
	80	+	require.Error(t, err)
	81	+	assert.Contains(t, err.Error(), "must use HTTPS")
	82	+
	83	+	// HTTPS should be accepted
	84	+	_, err = NewIssuer(Config{
	85	+	Email: "test@example.com",
	86	+	Challenge: domain.ACMEChallengeHTTP01,
	87	+	Store: outmocks.NewMockCertificateStore(t),
	88	+	HTTPChallengeSink: outmocks.NewMockHTTPChallengeSink(t),
	89	+	CADirectoryURL: "https://acme.test/directory",
	90	+	})
	91	+	require.NoError(t, err)
	92	+	}
	93	+
72	94		func TestNewIssuerValidWithDefaults(t *testing.T) {
73	95		issuer, err := NewIssuer(Config{
74	96		Email: "test@example.com",


@@ -69,6 +69,28 @@ func TestNewIssuerValidatesCloudflareToken(t *testing.T) {
69	assert.ErrorIs(t, err, domain.ErrCloudflareTokenMissing)	69		assert.ErrorIs(t, err, domain.ErrCloudflareTokenMissing)
70	}	70		}
71		71
		72	+	func TestNewIssuerRejectsNonHTTPSURL(t *testing.T) {
		73	+	_, err := NewIssuer(Config{
		74	+	Email: "test@example.com",
		75	+	Challenge: domain.ACMEChallengeHTTP01,
		76	+	Store: outmocks.NewMockCertificateStore(t),
		77	+	HTTPChallengeSink: outmocks.NewMockHTTPChallengeSink(t),
		78	+	CADirectoryURL: "http://acme.test/directory",
		79	+	})
		80	+	require.Error(t, err)
		81	+	assert.Contains(t, err.Error(), "must use HTTPS")
		82	+
		83	+	// HTTPS should be accepted
		84	+	_, err = NewIssuer(Config{
		85	+	Email: "test@example.com",
		86	+	Challenge: domain.ACMEChallengeHTTP01,
		87	+	Store: outmocks.NewMockCertificateStore(t),
		88	+	HTTPChallengeSink: outmocks.NewMockHTTPChallengeSink(t),
		89	+	CADirectoryURL: "https://acme.test/directory",
		90	+	})
		91	+	require.NoError(t, err)
		92	+	}
		93	+
72	func TestNewIssuerValidWithDefaults(t *testing.T) {	94		func TestNewIssuerValidWithDefaults(t *testing.T) {
73	issuer, err := NewIssuer(Config{	95		issuer, err := NewIssuer(Config{
74	Email: "test@example.com",	96		Email: "test@example.com",

M internal/adapters/out/acmestore/store.go +5 -4


@@ -263,9 +263,10 @@ func writeCertificateFiles(dir string, cert out.StoredCertificate) error {
263	263		}
264	264
265	265		// Lock acquires an exclusive lock using a lock file at <root>/.lock.
266		-	// The lock is released by calling the returned unlock function.
267		-	// If the lock file already exists, Lock returns an error.
268		-	// Context cancellation is respected before attempting the lock.
	266	+	// It is fail-fast and non-blocking: if the lock file already exists, Lock
	267	+	// returns an error immediately without waiting. Context cancellation is
	268	+	// respected before attempting the lock acquisition. The returned unlock
	269	+	// function removes the lock file to clean up.
269	270		func (s *Store) Lock(ctx context.Context) (func() error, error) {
270	271		select {
271	272		case <-ctx.Done():

@@ -296,7 +297,7 @@ func (s *Store) Lock(ctx context.Context) (func() error, error) {
296	297
297	298		// safeID rejects IDs that could cause path traversal.
298	299		func safeID(id string) bool {
299		-	return id != "" && !strings.Contains(id, "/") && !strings.Contains(id, "\\") && !strings.Contains(id, "..") && !strings.Contains(id, "\x00")
	300	+	return id != "" && !strings.HasPrefix(id, ".") && !strings.Contains(id, "/") && !strings.Contains(id, "\\") && !strings.Contains(id, "..") && !strings.Contains(id, "\x00")
300	301		}
301	302
302	303		// writeAtomic writes data to path atomically by writing to a temporary file

 }
 // Lock acquires an exclusive lock using a lock file at <root>/.lock.
-// The lock is released by calling the returned unlock function.
+// It is fail-fast and non-blocking: if the lock file already exists, Lock
-// If the lock file already exists, Lock returns an error.
+// returns an error immediately without waiting. Context cancellation is
-// Context cancellation is respected before attempting the lock.
+// respected before attempting the lock acquisition. The returned unlock
+// function removes the lock file to clean up.
 func (s *Store) Lock(ctx context.Context) (func() error, error) {
 	select {
 	case <-ctx.Done():
 // safeID rejects IDs that could cause path traversal.
 func safeID(id string) bool {
-	return id != "" && !strings.Contains(id, "/") && !strings.Contains(id, "\\") && !strings.Contains(id, "..") && !strings.Contains(id, "\x00")
+	return id != "" && !strings.HasPrefix(id, ".") && !strings.Contains(id, "/") && !strings.Contains(id, "\\") && !strings.Contains(id, "..") && !strings.Contains(id, "\x00")
 }
 // writeAtomic writes data to path atomically by writing to a temporary file

M internal/adapters/out/acmestore/store_test.go +11 -7


@@ -128,14 +128,18 @@ func TestStoreRejectsUnsafeID(t *testing.T) {
128	128		store, err := New(root)
129	129		require.NoError(t, err)
130	130
131		-	cert := out.StoredCertificate{
132		-	ID: "../escape",
133		-	PrivateKeyPEM: []byte("key"),
	131	+	for _, id := range []string{"../escape", ".", ".hidden"} {
	132	+	t.Run(id, func(t *testing.T) {
	133	+	cert := out.StoredCertificate{
	134	+	ID: id,
	135	+	PrivateKeyPEM: []byte("key"),
	136	+	}
	137	+
	138	+	err = store.Save(ctx, cert)
	139	+	require.Error(t, err)
	140	+	assert.Contains(t, err.Error(), "unsafe certificate id")
	141	+	})
134	142		}
135		-
136		-	err = store.Save(ctx, cert)
137		-	require.Error(t, err)
138		-	assert.Contains(t, err.Error(), "unsafe certificate id")
139	143		}
140	144
141	145		func TestStoreSaveLoadAccount(t *testing.T) {

 	store, err := New(root)
 	require.NoError(t, err)
-	cert := out.StoredCertificate{
+	for _, id := range []string{"../escape", ".", ".hidden"} {
-		ID:            "../escape",
+		t.Run(id, func(t *testing.T) {
-		PrivateKeyPEM: []byte("key"),
+			cert := out.StoredCertificate{
+				ID:            id,
+				PrivateKeyPEM: []byte("key"),
+			}
+			err = store.Save(ctx, cert)
+			require.Error(t, err)
+			assert.Contains(t, err.Error(), "unsafe certificate id")
+		})
 	}
-	err = store.Save(ctx, cert)
-	require.Error(t, err)
-	assert.Contains(t, err.Error(), "unsafe certificate id")
 }
 func TestStoreSaveLoadAccount(t *testing.T) {

M internal/adapters/out/secrets/public_tls_resolver.go +8 -1


@@ -122,8 +122,15 @@ func readFileWithTimeout(ctx context.Context, path string, timeout time.Duration
122	122
123	123		ch := make(chan result, 1)
124	124		go func() {
	125	+	// os.ReadFile is not context-cancellable. The buffered channel ensures
	126	+	// this goroutine can finish without blocking if readCtx expires first;
	127	+	// for local config files this trade-off is acceptable, though reads on
	128	+	// slow NFS/FUSE mounts may continue after cancellation.
125	129		data, err := os.ReadFile(path)
126		-	ch <- result{data: data, err: err}
	130	+	select {
	131	+	case ch <- result{data: data, err: err}:
	132	+	case <-readCtx.Done():
	133	+	}
127	134		}()
128	135
129	136		select {

 	ch := make(chan result, 1)
 	go func() {
+		// os.ReadFile is not context-cancellable. The buffered channel ensures
+		// this goroutine can finish without blocking if readCtx expires first;
+		// for local config files this trade-off is acceptable, though reads on
+		// slow NFS/FUSE mounts may continue after cancellation.
 		data, err := os.ReadFile(path)
-		ch <- result{data: data, err: err}
+		select {
+		case ch <- result{data: data, err: err}:
+		case <-readCtx.Done():
+		}
 	}()
 	select {

M internal/app/migrate_env_test.go +2 -2


@@ -32,7 +32,7 @@ func requirePass(t *testing.T) {
32	32		}
33	33		}
34	34
35		-	func cleanupPassDomain(_ *testing.T, domainName string, keys []string) {
	35	+	func cleanupPassDomain(domainName string, keys []string) {
36	36		safeDomain, err := domain.SanitizeDomainForEnvFile(domainName)
37	37		if err != nil {
38	38		return

@@ -60,7 +60,7 @@ func TestMigrateEnvFile(t *testing.T) {
60	60
61	61		domainName := fmt.Sprintf("test.%d.example.com", time.Now().UnixNano())
62	62		keys := []string{"API_KEY", "DB_PASSWORD"}
63		-	defer cleanupPassDomain(t, domainName, keys)
	63	+	defer cleanupPassDomain(domainName, keys)
64	64
65	65		envContent := "API_KEY=secret123\nDB_PASSWORD=pass456\n"
66	66		envFileName := domainName + ".env"


@@ -32,7 +32,7 @@ func requirePass(t *testing.T) {
32		}	32		}
33		}	33		}
34			34
35	-	func cleanupPassDomain(_ *testing.T, domainName string, keys []string) {	35	+	func cleanupPassDomain(domainName string, keys []string) {
36		safeDomain, err := domain.SanitizeDomainForEnvFile(domainName)	36		safeDomain, err := domain.SanitizeDomainForEnvFile(domainName)
37		if err != nil {	37		if err != nil {
38		return	38		return

@@ -60,7 +60,7 @@ func TestMigrateEnvFile(t *testing.T) {
60			60
61		domainName := fmt.Sprintf("test.%d.example.com", time.Now().UnixNano())	61		domainName := fmt.Sprintf("test.%d.example.com", time.Now().UnixNano())
62		keys := []string{"API_KEY", "DB_PASSWORD"}	62		keys := []string{"API_KEY", "DB_PASSWORD"}
63	-	defer cleanupPassDomain(t, domainName, keys)	63	+	defer cleanupPassDomain(domainName, keys)
64			64
65		envContent := "API_KEY=secret123\nDB_PASSWORD=pass456\n"	65		envContent := "API_KEY=secret123\nDB_PASSWORD=pass456\n"
66		envFileName := domainName + ".env"	66		envFileName := domainName + ".env"

M internal/domain/errors.go +9 -0


@@ -91,4 +91,13 @@ var (
91	91		ErrAttachmentImageEmpty = errors.New("attachment image cannot be empty")
92	92		ErrAttachmentTargetEmpty = errors.New("attachment target (domain or group) cannot be empty")
93	93		ErrAttachmentNotDeployed = errors.New("configured attachment is not deployed; run gordon deploy to create it")
	94	+
	95	+	// TLS / ACME errors
	96	+	ErrACMEDisabled = errors.New("acme disabled")
	97	+	ErrACMEEmailRequired = errors.New("acme email required")
	98	+	ErrACMEChallengeInvalid = errors.New("acme challenge invalid")
	99	+	ErrCloudflareTokenMissing = errors.New("cloudflare api token missing")
	100	+	ErrCertificateStoreRequired = errors.New("certificate store required")
	101	+	ErrHTTPChallengeSinkRequired = errors.New("http challenge sink required")
	102	+	ErrTLSRouteNotCovered = errors.New("tls route not covered by public certificate")
94	103		)


@@ -91,4 +91,13 @@ var (
91	ErrAttachmentImageEmpty = errors.New("attachment image cannot be empty")	91		ErrAttachmentImageEmpty = errors.New("attachment image cannot be empty")
92	ErrAttachmentTargetEmpty = errors.New("attachment target (domain or group) cannot be empty")	92		ErrAttachmentTargetEmpty = errors.New("attachment target (domain or group) cannot be empty")
93	ErrAttachmentNotDeployed = errors.New("configured attachment is not deployed; run gordon deploy to create it")	93		ErrAttachmentNotDeployed = errors.New("configured attachment is not deployed; run gordon deploy to create it")
		94	+
		95	+	// TLS / ACME errors
		96	+	ErrACMEDisabled = errors.New("acme disabled")
		97	+	ErrACMEEmailRequired = errors.New("acme email required")
		98	+	ErrACMEChallengeInvalid = errors.New("acme challenge invalid")
		99	+	ErrCloudflareTokenMissing = errors.New("cloudflare api token missing")
		100	+	ErrCertificateStoreRequired = errors.New("certificate store required")
		101	+	ErrHTTPChallengeSinkRequired = errors.New("http challenge sink required")
		102	+	ErrTLSRouteNotCovered = errors.New("tls route not covered by public certificate")
94	)	103		)

M internal/domain/public_tls.go +0 -11

@@ -1,7 +1,6 @@
1	1		package domain
2	2
3	3		import (
4		-	"errors"
5	4		"fmt"
6	5		"strings"
7	6		"time"

@@ -15,16 +14,6 @@ const (
15	14		ACMEChallengeCloudflareDNS01 ACMEChallengeMode = "cloudflare-dns-01"
16	15		)
17	16
18		-	var (
19		-	ErrACMEDisabled = errors.New("acme disabled")
20		-	ErrACMEEmailRequired = errors.New("acme email required")
21		-	ErrACMEChallengeInvalid = errors.New("acme challenge invalid")
22		-	ErrCloudflareTokenMissing = errors.New("cloudflare api token missing")
23		-	ErrCertificateStoreRequired = errors.New("certificate store required")
24		-	ErrHTTPChallengeSinkRequired = errors.New("http challenge sink required")
25		-	ErrTLSRouteNotCovered = errors.New("tls route not covered by public certificate")
26		-	)
27		-
28	17		func ParseACMEChallengeMode(value string) (ACMEChallengeMode, error) {
29	18		switch strings.TrimSpace(value) {
30	19		case "", string(ACMEChallengeAuto):

@@ -1,7 +1,6 @@
1		package domain	1	package domain
2			2
3		import (	3	import (
4	-	"errors"
5		"fmt"	4	"fmt"
6		"strings"	5	"strings"
7		"time"	6	"time"

@@ -15,16 +14,6 @@ const (
15		ACMEChallengeCloudflareDNS01 ACMEChallengeMode = "cloudflare-dns-01"	14	ACMEChallengeCloudflareDNS01 ACMEChallengeMode = "cloudflare-dns-01"
16		)	15	)
17			16
18	-	var (
19	-	ErrACMEDisabled = errors.New("acme disabled")
20	-	ErrACMEEmailRequired = errors.New("acme email required")
21	-	ErrACMEChallengeInvalid = errors.New("acme challenge invalid")
22	-	ErrCloudflareTokenMissing = errors.New("cloudflare api token missing")
23	-	ErrCertificateStoreRequired = errors.New("certificate store required")
24	-	ErrHTTPChallengeSinkRequired = errors.New("http challenge sink required")
25	-	ErrTLSRouteNotCovered = errors.New("tls route not covered by public certificate")
26	-	)
27	-
28		func ParseACMEChallengeMode(value string) (ACMEChallengeMode, error) {	17	func ParseACMEChallengeMode(value string) (ACMEChallengeMode, error) {
29		switch strings.TrimSpace(value) {	18	switch strings.TrimSpace(value) {
30		case "", string(ACMEChallengeAuto):	19	case "", string(ACMEChallengeAuto):

M internal/usecase/publictls/config.go +4 -7


@@ -3,7 +3,6 @@ package publictls
3	3		import (
4	4		"context"
5	5		"fmt"
6		-	"strings"
7	6
8	7		"github.com/bnema/gordon/internal/boundaries/out"
9	8		"github.com/bnema/gordon/internal/domain"

@@ -117,11 +116,10 @@ func resolveOptionalToken(ctx context.Context, resolver out.SecretResolver) (str
117	116		if err != nil {
118	117		return "", domain.ACMETokenSourceNone
119	118		}
120		-	token := strings.TrimSpace(sv.Value)
121		-	if token == "" {
	119	+	if sv.Value == "" {
122	120		return "", domain.ACMETokenSourceNone
123	121		}
124		-	return token, sv.Source
	122	+	return sv.Value, sv.Source
125	123		}
126	124
127	125		// resolveToken resolves the Cloudflare token via the resolver, propagating errors.

@@ -134,9 +132,8 @@ func resolveToken(ctx context.Context, resolver out.SecretResolver) (string, dom
134	132		if err != nil {
135	133		return "", domain.ACMETokenSourceNone, err
136	134		}
137		-	token := strings.TrimSpace(sv.Value)
138		-	if token == "" {
	135	+	if sv.Value == "" {
139	136		return "", domain.ACMETokenSourceNone, nil
140	137		}
141		-	return token, sv.Source, nil
	138	+	return sv.Value, sv.Source, nil
142	139		}

 import (
 	"context"
 	"fmt"
-	"strings"
 	"github.com/bnema/gordon/internal/boundaries/out"
 	"github.com/bnema/gordon/internal/domain"
 	if err != nil {
 		return "", domain.ACMETokenSourceNone
 	}
-	token := strings.TrimSpace(sv.Value)
+	if sv.Value == "" {
-	if token == "" {
 		return "", domain.ACMETokenSourceNone
 	}
-	return token, sv.Source
+	return sv.Value, sv.Source
 }
 // resolveToken resolves the Cloudflare token via the resolver, propagating errors.
 	if err != nil {
 		return "", domain.ACMETokenSourceNone, err
 	}
-	token := strings.TrimSpace(sv.Value)
+	if sv.Value == "" {
-	if token == "" {
 		return "", domain.ACMETokenSourceNone, nil
 	}
-	return token, sv.Source, nil
+	return sv.Value, sv.Source, nil
 }

M internal/usecase/publictls/http01.go +5 -1


@@ -31,8 +31,12 @@ func (c *HTTP01Challenges) Present(token, keyAuth string) {
31	31		c.data[token] = keyAuth
32	32		}
33	33
34		-	// CleanUp removes the stored key authorization for the given token.
	34	+	// CleanUp removes the stored key authorization for the given token. It silently
	35	+	// ignores unsafe tokens (see safeHTTP01Token).
35	36		func (c *HTTP01Challenges) CleanUp(token string) {
	37	+	if !safeHTTP01Token(token) {
	38	+	return
	39	+	}
36	40		c.mu.Lock()
37	41		defer c.mu.Unlock()
38	42		delete(c.data, token)

 	c.data[token] = keyAuth
 }
-// CleanUp removes the stored key authorization for the given token.
+// CleanUp removes the stored key authorization for the given token. It silently
+// ignores unsafe tokens (see safeHTTP01Token).
 func (c *HTTP01Challenges) CleanUp(token string) {
+	if !safeHTTP01Token(token) {
+		return
+	}
 	c.mu.Lock()
 	defer c.mu.Unlock()
 	delete(c.data, token)

M internal/usecase/publictls/renewal.go +5 -4


@@ -100,18 +100,19 @@ func (s *Service) renewDueCertificates(ctx context.Context, now time.Time) error
100	100		}
101	101		defer func() { _ = unlock() }()
102	102
103		-	// Collect due certs under lock, then release before making external calls.
	103	+	// Collect due certs under lock, copying values before releasing the lock so
	104	+	// renewal uses a stable snapshot even if the cache changes concurrently.
104	105		s.mu.Lock()
105		-	due := make([]*out.StoredCertificate, 0)
	106	+	due := make([]out.StoredCertificate, 0)
106	107		for _, cert := range s.certs {
107	108		if ShouldRenew(*cert, now) {
108		-	due = append(due, cert)
	109	+	due = append(due, *cert)
109	110		}
110	111		}
111	112		s.mu.Unlock()
112	113
113	114		for _, cert := range due {
114		-	renewed, err := s.deps.Issuer.Renew(ctx, *cert)
	115	+	renewed, err := s.deps.Issuer.Renew(ctx, cert)
115	116		if err != nil {
116	117		s.mu.Lock()
117	118		s.lastErr[cert.ID] = fmt.Sprintf("renew: %v", err)

 	}
 	defer func() { _ = unlock() }()
-	// Collect due certs under lock, then release before making external calls.
+	// Collect due certs under lock, copying values before releasing the lock so
+	// renewal uses a stable snapshot even if the cache changes concurrently.
 	s.mu.Lock()
-	due := make([]*out.StoredCertificate, 0)
+	due := make([]out.StoredCertificate, 0)
 	for _, cert := range s.certs {
 		if ShouldRenew(*cert, now) {
-			due = append(due, cert)
+			due = append(due, *cert)
 		}
 	}
 	s.mu.Unlock()
 	for _, cert := range due {
-		renewed, err := s.deps.Issuer.Renew(ctx, *cert)
+		renewed, err := s.deps.Issuer.Renew(ctx, cert)
 		if err != nil {
 			s.mu.Lock()
 			s.lastErr[cert.ID] = fmt.Sprintf("renew: %v", err)

M internal/usecase/publictls/service.go +7 -2


@@ -9,9 +9,10 @@ import (
9	9		"sync"
10	10		"time"
11	11
	12	+	"github.com/bnema/zerowrap"
	13	+
12	14		"github.com/bnema/gordon/internal/boundaries/out"
13	15		"github.com/bnema/gordon/internal/domain"
14		-	"github.com/bnema/zerowrap"
15	16		)
16	17
17	18		// RouteSource provides routes from which certificate targets are derived.

@@ -182,7 +183,11 @@ func (s *Service) Reconcile(ctx context.Context) error {
182	183		if err != nil {
183	184		return fmt.Errorf("acquire store lock: %w", err)
184	185		}
185		-	defer func() { _ = unlock() }()
	186	+	defer func() {
	187	+	if err := unlock(); err != nil {
	188	+	log.Warn().Err(err).Msg("failed to release store lock")
	189	+	}
	190	+	}()
186	191
187	192		// Under mu: update requiredHosts (with target names) and compute missing targets.
188	193		s.mu.Lock()

 	"sync"
 	"time"
+	"github.com/bnema/zerowrap"
 	"github.com/bnema/gordon/internal/boundaries/out"
 	"github.com/bnema/gordon/internal/domain"
-	"github.com/bnema/zerowrap"
 )
 // RouteSource provides routes from which certificate targets are derived.
 	if err != nil {
 		return fmt.Errorf("acquire store lock: %w", err)
 	}
-	defer func() { _ = unlock() }()
+	defer func() {
+		if err := unlock(); err != nil {
+			log.Warn().Err(err).Msg("failed to release store lock")
+		}
+	}()
 	// Under mu: update requiredHosts (with target names) and compute missing targets.
 	s.mu.Lock()

M internal/usecase/publictls/service_test.go +13 -14


@@ -42,17 +42,17 @@ type fakeRoutes struct {
42	42		func (f *fakeRoutes) GetRoutes(_ context.Context) []domain.Route {
43	43		f.mu.Lock()
44	44		defer f.mu.Unlock()
45		-	out := make([]domain.Route, len(f.routes))
46		-	copy(out, f.routes)
47		-	return out
	45	+	routesCopy := make([]domain.Route, len(f.routes))
	46	+	copy(routesCopy, f.routes)
	47	+	return routesCopy
48	48		}
49	49
50	50		func (f *fakeRoutes) GetExternalRoutes() map[string]string {
51	51		f.mu.Lock()
52	52		defer f.mu.Unlock()
53		-	out := make(map[string]string, len(f.external))
54		-	maps.Copy(out, f.external)
55		-	return out
	53	+	externalCopy := make(map[string]string, len(f.external))
	54	+	maps.Copy(externalCopy, f.external)
	55	+	return externalCopy
56	56		}
57	57
58	58		type mockIssuerRecorder struct {

@@ -426,13 +426,6 @@ func TestServiceGetCertificateReturnsErrTLSRouteNotCovered(t *testing.T) {
426	426		assert.Contains(t, err.Error(), "app.example.com")
427	427		}
428	428
429		-	// brokenZoneResolver always returns an error.
430		-	type brokenZoneResolver struct{}
431		-
432		-	func (brokenZoneResolver) FindZone(_ context.Context, host string) (out.CloudflareZone, error) {
433		-	return out.CloudflareZone{}, fmt.Errorf("zone resolver unavailable for %s", host)
434		-	}
435		-
436	429		func TestServiceReconcileDNS01BrokenResolverReturnsErrTLSRouteNotCovered(t *testing.T) {
437	430		ctx := context.Background()
438	431

@@ -444,6 +437,12 @@ func TestServiceReconcileDNS01BrokenResolverReturnsErrTLSRouteNotCovered(t *test
444	437		issuer, _ := newMockPublicCertificateIssuer(t, nil, nil)
445	438		store, _ := newMockCertificateStore(t)
446	439
	440	+	// Use generated mock that always returns an error.
	441	+	zoneResolver := outmocks.NewMockCloudflareZoneResolver(t)
	442	+	zoneResolver.EXPECT().FindZone(mock.Anything, "app.example.com").Return(
	443	+	out.CloudflareZone{}, fmt.Errorf("zone resolver unavailable for app.example.com"),
	444	+	)
	445	+
447	446		cfg := Config{
448	447		Enabled: true,
449	448		Email: "admin@example.com",

@@ -457,7 +456,7 @@ func TestServiceReconcileDNS01BrokenResolverReturnsErrTLSRouteNotCovered(t *test
457	456		Routes: routes,
458	457		Issuer: issuer,
459	458		Store: store,
460		-	ZoneResolver: brokenZoneResolver{}, // broken: always fails
	459	+	ZoneResolver: zoneResolver, // generated mock that always fails
461	460		Challenges: NewHTTP01Challenges(),
462	461		Effective: EffectiveChallenge{
463	462		ConfiguredMode: domain.ACMEChallengeCloudflareDNS01,

 func (f *fakeRoutes) GetRoutes(_ context.Context) []domain.Route {
 	f.mu.Lock()
 	defer f.mu.Unlock()
-	out := make([]domain.Route, len(f.routes))
+	routesCopy := make([]domain.Route, len(f.routes))
-	copy(out, f.routes)
+	copy(routesCopy, f.routes)
-	return out
+	return routesCopy
 }
 func (f *fakeRoutes) GetExternalRoutes() map[string]string {
 	f.mu.Lock()
 	defer f.mu.Unlock()
-	out := make(map[string]string, len(f.external))
+	externalCopy := make(map[string]string, len(f.external))
-	maps.Copy(out, f.external)
+	maps.Copy(externalCopy, f.external)
-	return out
+	return externalCopy
 }
 type mockIssuerRecorder struct {
 	assert.Contains(t, err.Error(), "app.example.com")
 }
-// brokenZoneResolver always returns an error.
-type brokenZoneResolver struct{}
-func (brokenZoneResolver) FindZone(_ context.Context, host string) (out.CloudflareZone, error) {
-	return out.CloudflareZone{}, fmt.Errorf("zone resolver unavailable for %s", host)
-}
 func TestServiceReconcileDNS01BrokenResolverReturnsErrTLSRouteNotCovered(t *testing.T) {
 	ctx := context.Background()
 	issuer, _ := newMockPublicCertificateIssuer(t, nil, nil)
 	store, _ := newMockCertificateStore(t)
+	// Use generated mock that always returns an error.
+	zoneResolver := outmocks.NewMockCloudflareZoneResolver(t)
+	zoneResolver.EXPECT().FindZone(mock.Anything, "app.example.com").Return(
+		out.CloudflareZone{}, fmt.Errorf("zone resolver unavailable for app.example.com"),
+	)
 	cfg := Config{
 		Enabled:   true,
 		Email:     "admin@example.com",
 		Routes:       routes,
 		Issuer:       issuer,
 		Store:        store,
-		ZoneResolver: brokenZoneResolver{}, // broken: always fails
+		ZoneResolver: zoneResolver, // generated mock that always fails
 		Challenges:   NewHTTP01Challenges(),
 		Effective: EffectiveChallenge{
 			ConfiguredMode: domain.ACMEChallengeCloudflareDNS01,

M internal/usecase/publictls/status.go +25 -9


@@ -119,7 +119,8 @@ func (s *Service) findCoverageLocked(routeDomain string, managedCerts []domain.M
119	119		case domain.TLSCertificateStatusExpired, domain.TLSCertificateStatusMissing, domain.TLSCertificateStatusError:
120	120		continue
121	121		}
122		-	// Double-check expiry from the raw stored cert.
	122	+	// Double-check the raw stored cert expiry to guard against stale
	123	+	// ManagedCertificate.Status values that may not reflect real expiry.
123	124		if sc, ok := s.certs[mc.ID]; ok {
124	125		if !sc.NotAfter.IsZero() && !now.Before(sc.NotAfter) {
125	126		continue

@@ -130,12 +131,26 @@ func (s *Service) findCoverageLocked(routeDomain string, managedCerts []domain.M
130	131		return false, ""
131	132		}
132	133
133		-	// keyEqValuePattern matches key=value patterns for known sensitive keys
134		-	// in a case-insensitive manner.
135		-	var keyEqValuePattern = regexp.MustCompile(`(?i)(token\|secret\|key\|password\|pass\|api_key\|apikey\|auth)=[^\s]+`)
	134	+	// Sensitive key names that may appear in error messages.
	135	+	var sensitiveKeys = `(token\|secret\|key\|password\|pass\|api_key\|apikey\|auth)`
	136	+
	137	+	// Patterns for redacting sensitive values in common formats.
	138	+	var (
	139	+	// key=value (unquoted, case-insensitive)
	140	+	keyEqValueRe = regexp.MustCompile(`(?i)` + sensitiveKeys + `=[^\s"']+`)
	141	+	// key="value" (double-quoted)
	142	+	keyEqQuotedRe = regexp.MustCompile(`(?i)` + sensitiveKeys + `="[^"]*"`)
	143	+	// key='value' (single-quoted)
	144	+	keyEqSingleQuotedRe = regexp.MustCompile(`(?i)` + sensitiveKeys + `='[^']*'`)
	145	+	// JSON "key": "value"
	146	+	jsonKeyValueRe = regexp.MustCompile(`(?i)"` + sensitiveKeys + `"\s:\s"[^"]*"`)
	147	+	// YAML key: value (start of line or after indent)
	148	+	yamlKeyValueRe = regexp.MustCompile(`(?i)(?m)^\s` + sensitiveKeys + `\s:\s+\S+`)
	149	+	)
136	150
137	151		// sanitizeError removes obvious sensitive strings from error messages.
138		-	// It redacts key=value patterns for known sensitive keys while preserving
	152	+	// It redacts key=value, key="value", key='value', JSON "key": "value",
	153	+	// and YAML key: value patterns for known sensitive keys while preserving
139	154		// safe context. It does not blanket-redact messages just because they
140	155		// contain the word "secret" or similar.
141	156		func sanitizeError(err string) string {

@@ -143,9 +158,10 @@ func sanitizeError(err string) string {
143	158		return ""
144	159		}
145	160
146		-	result := keyEqValuePattern.ReplaceAllString(err, "$1=redacted")
147		-	if result == "" {
148		-	return "certificate error"
149		-	}
	161	+	result := keyEqValueRe.ReplaceAllString(err, "$1=redacted")
	162	+	result = keyEqQuotedRe.ReplaceAllString(result, `$1="redacted"`)
	163	+	result = keyEqSingleQuotedRe.ReplaceAllString(result, "$1='redacted'")
	164	+	result = jsonKeyValueRe.ReplaceAllString(result, `"$1":"redacted"`)
	165	+	result = yamlKeyValueRe.ReplaceAllString(result, "$1: redacted")
150	166		return result
151	167		}


@@ -119,7 +119,8 @@ func (s *Service) findCoverageLocked(routeDomain string, managedCerts []domain.M
119		case domain.TLSCertificateStatusExpired, domain.TLSCertificateStatusMissing, domain.TLSCertificateStatusError:	119		case domain.TLSCertificateStatusExpired, domain.TLSCertificateStatusMissing, domain.TLSCertificateStatusError:
120		continue	120		continue
121		}	121		}
122	-	// Double-check expiry from the raw stored cert.	122	+	// Double-check the raw stored cert expiry to guard against stale
			123	+	// ManagedCertificate.Status values that may not reflect real expiry.
123		if sc, ok := s.certs[mc.ID]; ok {	124		if sc, ok := s.certs[mc.ID]; ok {
124		if !sc.NotAfter.IsZero() && !now.Before(sc.NotAfter) {	125		if !sc.NotAfter.IsZero() && !now.Before(sc.NotAfter) {
125		continue	126		continue

@@ -130,12 +131,26 @@ func (s *Service) findCoverageLocked(routeDomain string, managedCerts []domain.M
130		return false, ""	131		return false, ""
131		}	132		}
132			133
133	-	// keyEqValuePattern matches key=value patterns for known sensitive keys	134	+	// Sensitive key names that may appear in error messages.
134	-	// in a case-insensitive manner.	135	+	var sensitiveKeys = `(token\|secret\|key\|password\|pass\|api_key\|apikey\|auth)`
135	-	var keyEqValuePattern = regexp.MustCompile(`(?i)(token\|secret\|key\|password\|pass\|api_key\|apikey\|auth)=[^\s]+`)	136	+
			137	+	// Patterns for redacting sensitive values in common formats.
			138	+	var (
			139	+	// key=value (unquoted, case-insensitive)
			140	+	keyEqValueRe = regexp.MustCompile(`(?i)` + sensitiveKeys + `=[^\s"']+`)
			141	+	// key="value" (double-quoted)
			142	+	keyEqQuotedRe = regexp.MustCompile(`(?i)` + sensitiveKeys + `="[^"]*"`)
			143	+	// key='value' (single-quoted)
			144	+	keyEqSingleQuotedRe = regexp.MustCompile(`(?i)` + sensitiveKeys + `='[^']*'`)
			145	+	// JSON "key": "value"
			146	+	jsonKeyValueRe = regexp.MustCompile(`(?i)"` + sensitiveKeys + `"\s:\s"[^"]*"`)
			147	+	// YAML key: value (start of line or after indent)
			148	+	yamlKeyValueRe = regexp.MustCompile(`(?i)(?m)^\s` + sensitiveKeys + `\s:\s+\S+`)
			149	+	)
136			150
137		// sanitizeError removes obvious sensitive strings from error messages.	151		// sanitizeError removes obvious sensitive strings from error messages.
138	-	// It redacts key=value patterns for known sensitive keys while preserving	152	+	// It redacts key=value, key="value", key='value', JSON "key": "value",
			153	+	// and YAML key: value patterns for known sensitive keys while preserving
139		// safe context. It does not blanket-redact messages just because they	154		// safe context. It does not blanket-redact messages just because they
140		// contain the word "secret" or similar.	155		// contain the word "secret" or similar.
141		func sanitizeError(err string) string {	156		func sanitizeError(err string) string {

@@ -143,9 +158,10 @@ func sanitizeError(err string) string {
143		return ""	158		return ""
144		}	159		}
145			160
146	-	result := keyEqValuePattern.ReplaceAllString(err, "$1=redacted")	161	+	result := keyEqValueRe.ReplaceAllString(err, "$1=redacted")
147	-	if result == "" {	162	+	result = keyEqQuotedRe.ReplaceAllString(result, `$1="redacted"`)
148	-	return "certificate error"	163	+	result = keyEqSingleQuotedRe.ReplaceAllString(result, "$1='redacted'")
149	-	}	164	+	result = jsonKeyValueRe.ReplaceAllString(result, `"$1":"redacted"`)
			165	+	result = yamlKeyValueRe.ReplaceAllString(result, "$1: redacted")
150		return result	166		return result
151		}	167		}

A internal/usecase/publictls/status_test.go +77 -0

@@ -0,0 +1,77 @@
	1	+	package publictls
	2	+
	3	+	import (
	4	+	"testing"
	5	+
	6	+	"github.com/stretchr/testify/assert"
	7	+	)
	8	+
	9	+	func TestSanitizeError_Empty(t *testing.T) {
	10	+	assert.Equal(t, "", sanitizeError(""))
	11	+	}
	12	+
	13	+	func TestSanitizeError_NoSensitiveData(t *testing.T) {
	14	+	msg := "failed to obtain certificate: acme server returned 429"
	15	+	assert.Equal(t, msg, sanitizeError(msg))
	16	+	}
	17	+
	18	+	func TestSanitizeError_KeyEqValue(t *testing.T) {
	19	+	result := sanitizeError("token=sk-secret-goes-here provider said invalid")
	20	+	assert.NotContains(t, result, "sk-secret-goes-here")
	21	+	assert.Contains(t, result, "token=redacted")
	22	+	}
	23	+
	24	+	func TestSanitizeError_KeyEqQuotedValue(t *testing.T) {
	25	+	result := sanitizeError(`secret="my-secret-value" is invalid`)
	26	+	assert.NotContains(t, result, "my-secret-value")
	27	+	assert.Contains(t, result, `secret="redacted"`)
	28	+	}
	29	+
	30	+	func TestSanitizeError_KeyEqSingleQuotedValue(t *testing.T) {
	31	+	result := sanitizeError(`key='my-api-key-12345' not found`)
	32	+	assert.NotContains(t, result, "my-api-key-12345")
	33	+	assert.Contains(t, result, "key='redacted'")
	34	+	}
	35	+
	36	+	func TestSanitizeError_JSONKeyValue(t *testing.T) {
	37	+	result := sanitizeError(`{"token":"s3kr1t","domain":"example.com"}`)
	38	+	assert.NotContains(t, result, "s3kr1t")
	39	+	assert.Contains(t, result, `"token":"redacted"`)
	40	+	// Non-sensitive fields should be preserved
	41	+	assert.Contains(t, result, `"domain":"example.com"`)
	42	+	}
	43	+
	44	+	func TestSanitizeError_YAMLKeyValue(t *testing.T) {
	45	+	result := sanitizeError("configuration error:\n api_key: abcdef123456\n password: hunter2")
	46	+	assert.NotContains(t, result, "abcdef123456")
	47	+	assert.NotContains(t, result, "hunter2")
	48	+	assert.Contains(t, result, "api_key: redacted")
	49	+	assert.Contains(t, result, "password: redacted")
	50	+	// Non-sensitive context preserved
	51	+	assert.Contains(t, result, "configuration error:")
	52	+	}
	53	+
	54	+	func TestSanitizeError_CaseInsensitive(t *testing.T) {
	55	+	result := sanitizeError("Token=sk-live-abc123 and SECRET=top-secret-value")
	56	+	assert.NotContains(t, result, "sk-live-abc123")
	57	+	assert.NotContains(t, result, "top-secret-value")
	58	+	assert.Contains(t, result, "Token=redacted")
	59	+	assert.Contains(t, result, "SECRET=redacted")
	60	+	}
	61	+
	62	+	func TestSanitizeError_MixedFormats(t *testing.T) {
	63	+	result := sanitizeError(`token=plain, secret="quoted", auth='single-quoted', and json {"password":"p4ss"}`)
	64	+	assert.NotContains(t, result, "plain")
	65	+	assert.NotContains(t, result, "quoted")
	66	+	assert.NotContains(t, result, "single-quoted")
	67	+	assert.NotContains(t, result, "p4ss")
	68	+	assert.Contains(t, result, "token=redacted")
	69	+	assert.Contains(t, result, `secret="redacted"`)
	70	+	assert.Contains(t, result, "auth='redacted'")
	71	+	assert.Contains(t, result, `"password":"redacted"`)
	72	+	}
	73	+
	74	+	func TestSanitizeError_PreservesSafeContext(t *testing.T) {
	75	+	msg := "certificate for example.com failed: acme error 403"
	76	+	assert.Equal(t, msg, sanitizeError(msg))
	77	+	}

@@ -0,0 +1,77 @@
	1	+	package publictls
	2	+
	3	+	import (
	4	+	"testing"
	5	+
	6	+	"github.com/stretchr/testify/assert"
	7	+	)
	8	+
	9	+	func TestSanitizeError_Empty(t *testing.T) {
	10	+	assert.Equal(t, "", sanitizeError(""))
	11	+	}
	12	+
	13	+	func TestSanitizeError_NoSensitiveData(t *testing.T) {
	14	+	msg := "failed to obtain certificate: acme server returned 429"
	15	+	assert.Equal(t, msg, sanitizeError(msg))
	16	+	}
	17	+
	18	+	func TestSanitizeError_KeyEqValue(t *testing.T) {
	19	+	result := sanitizeError("token=sk-secret-goes-here provider said invalid")
	20	+	assert.NotContains(t, result, "sk-secret-goes-here")
	21	+	assert.Contains(t, result, "token=redacted")
	22	+	}
	23	+
	24	+	func TestSanitizeError_KeyEqQuotedValue(t *testing.T) {
	25	+	result := sanitizeError(`secret="my-secret-value" is invalid`)
	26	+	assert.NotContains(t, result, "my-secret-value")
	27	+	assert.Contains(t, result, `secret="redacted"`)
	28	+	}
	29	+
	30	+	func TestSanitizeError_KeyEqSingleQuotedValue(t *testing.T) {
	31	+	result := sanitizeError(`key='my-api-key-12345' not found`)
	32	+	assert.NotContains(t, result, "my-api-key-12345")
	33	+	assert.Contains(t, result, "key='redacted'")
	34	+	}
	35	+
	36	+	func TestSanitizeError_JSONKeyValue(t *testing.T) {
	37	+	result := sanitizeError(`{"token":"s3kr1t","domain":"example.com"}`)
	38	+	assert.NotContains(t, result, "s3kr1t")
	39	+	assert.Contains(t, result, `"token":"redacted"`)
	40	+	// Non-sensitive fields should be preserved
	41	+	assert.Contains(t, result, `"domain":"example.com"`)
	42	+	}
	43	+
	44	+	func TestSanitizeError_YAMLKeyValue(t *testing.T) {
	45	+	result := sanitizeError("configuration error:\n api_key: abcdef123456\n password: hunter2")
	46	+	assert.NotContains(t, result, "abcdef123456")
	47	+	assert.NotContains(t, result, "hunter2")
	48	+	assert.Contains(t, result, "api_key: redacted")
	49	+	assert.Contains(t, result, "password: redacted")
	50	+	// Non-sensitive context preserved
	51	+	assert.Contains(t, result, "configuration error:")
	52	+	}
	53	+
	54	+	func TestSanitizeError_CaseInsensitive(t *testing.T) {
	55	+	result := sanitizeError("Token=sk-live-abc123 and SECRET=top-secret-value")
	56	+	assert.NotContains(t, result, "sk-live-abc123")
	57	+	assert.NotContains(t, result, "top-secret-value")
	58	+	assert.Contains(t, result, "Token=redacted")
	59	+	assert.Contains(t, result, "SECRET=redacted")
	60	+	}
	61	+
	62	+	func TestSanitizeError_MixedFormats(t *testing.T) {
	63	+	result := sanitizeError(`token=plain, secret="quoted", auth='single-quoted', and json {"password":"p4ss"}`)
	64	+	assert.NotContains(t, result, "plain")
	65	+	assert.NotContains(t, result, "quoted")
	66	+	assert.NotContains(t, result, "single-quoted")
	67	+	assert.NotContains(t, result, "p4ss")
	68	+	assert.Contains(t, result, "token=redacted")
	69	+	assert.Contains(t, result, `secret="redacted"`)
	70	+	assert.Contains(t, result, "auth='redacted'")
	71	+	assert.Contains(t, result, `"password":"redacted"`)
	72	+	}
	73	+
	74	+	func TestSanitizeError_PreservesSafeContext(t *testing.T) {
	75	+	msg := "certificate for example.com failed: acme error 403"
	76	+	assert.Equal(t, msg, sanitizeError(msg))
	77	+	}

M internal/usecase/publictls/targets.go +3 -0


@@ -117,6 +117,9 @@ func deriveDNS01Targets(ctx context.Context, hosts []string, resolver out.Cloudf
117	117		}
118	118
119	119		// wildcardBase determines the wildcard base for a host within a zone.
	120	+	// This is used by deriveDNS01Targets to group hosts under a common
	121	+	// wildcard certificate (e.g. example.com + *.example.com). Hosts that
	122	+	// share the same wildcard base are combined into a single DNS-01 target.
120	123		//
121	124		// If host == zone, returns zone.
122	125		// If host is a direct child of zone, returns zone.


@@ -117,6 +117,9 @@ func deriveDNS01Targets(ctx context.Context, hosts []string, resolver out.Cloudf
117	}	117		}
118		118
119	// wildcardBase determines the wildcard base for a host within a zone.	119		// wildcardBase determines the wildcard base for a host within a zone.
		120	+	// This is used by deriveDNS01Targets to group hosts under a common
		121	+	// wildcard certificate (e.g. example.com + *.example.com). Hosts that
		122	+	// share the same wildcard base are combined into a single DNS-01 target.
120	//	123		//
121	// If host == zone, returns zone.	124		// If host == zone, returns zone.
122	// If host is a direct child of zone, returns zone.	125		// If host is a direct child of zone, returns zone.