Skip to content

Commit 043ebe7

Browse files
authored
[test] Add tests for server.nonceCache.evictExpired (#5965)
# Test Coverage Improvement: `nonceCache.evictExpired` ## Function Analyzed - **Package**: `internal/server` - **Function**: `(*nonceCache).evictExpired` - **File**: `internal/server/hmac.go` - **Previous Coverage**: 0% (no direct tests) - **New Coverage**: High (all branches covered) - **Complexity**: Medium ## Why This Function? `evictExpired` is the core garbage-collection routine for the HMAC replay-protection nonce cache. It is called on every HMAC-validated request (from both `checkAndSet` and `seenNonce`) but had **zero direct test coverage**. While the function is exercised indirectly via integration through the middleware tests, no test directly validated its eviction logic, boundary conditions, or interaction behaviour. The function has meaningful branching logic (`now.After(deadline)`) and is security-critical — if eviction is broken, the nonce cache could grow unboundedly or incorrectly retain/remove nonces, weakening replay protection. ## Tests Added - ✅ Empty cache (no-op, no panic) - ✅ All entries expired — all removed - ✅ All entries fresh — none removed - ✅ Mixed expired and fresh — only expired entries evicted - ✅ Exact deadline boundary (`now.After(deadline)` is `false` when equal → entry retained) - ✅ Just-expired entry (one nanosecond past deadline → evicted) - ✅ Large entry count (100 entries, 50 expired / 50 fresh) - ✅ Integration: `checkAndSet` internally triggers eviction of stale entries - ✅ Integration: `seenNonce` internally triggers eviction of stale entries ## Coverage Report ``` Before: 0% direct coverage (exercised only as side-effect of middleware tests) After: All branches of evictExpired directly covered Improvement: Full branch coverage for the eviction logic ``` ## Test File `internal/server/hmac_evict_coverage_test.go` — 9 test functions using table-driven and scenario-based patterns consistent with the existing `hmac_test.go`. --- *Generated by Test Coverage Improver* *Next run will target the next most complex under-tested function* > [!WARNING] > <details> > <summary>Firewall blocked 3 domains</summary> > > The following domains were blocked by the firewall during workflow execution: > > - `goproxy.io` > - `proxy.golang.org` > - `sum.golang.org` >> To allow these domains, add them to the `network.allowed` list in your workflow frontmatter: > > ```yaml > network: > allowed: > - defaults > - "goproxy.io" > - "proxy.golang.org" > - "sum.golang.org" > ``` > > See [Network Configuration](https://github.github.com/gh-aw/reference/network/) for more information. > > </details> > Generated by [Test Coverage Improver](https://github.com/github/gh-aw-mcpg/actions/runs/26048920315/agentic_workflow) · ● 13.7M · [◷](https://github.com/search?q=repo%3Agithub%2Fgh-aw-mcpg+%22gh-aw-workflow-id%3A+test-coverage-improver%22&type=pullrequests) <!-- gh-aw-agentic-workflow: Test Coverage Improver, engine: copilot, version: 1.0.40, model: claude-sonnet-4.6, id: 26048920315, workflow_id: test-coverage-improver, run: https://github.com/github/gh-aw-mcpg/actions/runs/26048920315 --> <!-- gh-aw-workflow-id: test-coverage-improver -->
2 parents 2e642f7 + 4253127 commit 043ebe7

1 file changed

Lines changed: 201 additions & 0 deletions

File tree

Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
package server
2+
3+
import (
4+
"fmt"
5+
"testing"
6+
"time"
7+
8+
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
10+
)
11+
12+
// TestNonceCache_EvictExpired_EmptyCache verifies that calling evictExpired on an
13+
// empty cache is a no-op and does not panic.
14+
func TestNonceCache_EvictExpired_EmptyCache(t *testing.T) {
15+
c := newNonceCache()
16+
assert.Empty(t, c.entries, "cache should start empty")
17+
18+
// Must not panic on empty cache
19+
c.evictExpired(time.Now())
20+
assert.Empty(t, c.entries, "cache should still be empty after eviction")
21+
}
22+
23+
// TestNonceCache_EvictExpired_RemovesExpiredEntries verifies that entries whose
24+
// deadline has passed are deleted.
25+
func TestNonceCache_EvictExpired_RemovesExpiredEntries(t *testing.T) {
26+
c := newNonceCache()
27+
past := time.Now().Add(-1 * time.Second)
28+
29+
// Insert entries with past deadlines directly so they are immediately expired.
30+
c.entries["expired-1"] = past
31+
c.entries["expired-2"] = past.Add(-5 * time.Second)
32+
33+
require.Len(t, c.entries, 2, "should have 2 entries before eviction")
34+
35+
c.evictExpired(time.Now())
36+
37+
assert.Empty(t, c.entries, "all expired entries should be removed")
38+
}
39+
40+
// TestNonceCache_EvictExpired_KeepsFreshEntries verifies that entries whose
41+
// deadline is still in the future are retained.
42+
func TestNonceCache_EvictExpired_KeepsFreshEntries(t *testing.T) {
43+
c := newNonceCache()
44+
future := time.Now().Add(60 * time.Second)
45+
46+
c.entries["fresh-1"] = future
47+
c.entries["fresh-2"] = future.Add(10 * time.Second)
48+
49+
require.Len(t, c.entries, 2, "should have 2 entries before eviction")
50+
51+
c.evictExpired(time.Now())
52+
53+
assert.Len(t, c.entries, 2, "fresh entries should not be removed")
54+
assert.Contains(t, c.entries, "fresh-1")
55+
assert.Contains(t, c.entries, "fresh-2")
56+
}
57+
58+
// TestNonceCache_EvictExpired_MixedEntries verifies that only expired entries are
59+
// removed and fresh entries remain when both exist in the cache.
60+
func TestNonceCache_EvictExpired_MixedEntries(t *testing.T) {
61+
c := newNonceCache()
62+
now := time.Now()
63+
past := now.Add(-1 * time.Second)
64+
future := now.Add(60 * time.Second)
65+
66+
c.entries["expired-a"] = past
67+
c.entries["expired-b"] = past.Add(-10 * time.Second)
68+
c.entries["fresh-a"] = future
69+
c.entries["fresh-b"] = future.Add(30 * time.Second)
70+
71+
require.Len(t, c.entries, 4)
72+
73+
c.evictExpired(now)
74+
75+
assert.Len(t, c.entries, 2, "only fresh entries should remain")
76+
assert.NotContains(t, c.entries, "expired-a")
77+
assert.NotContains(t, c.entries, "expired-b")
78+
assert.Contains(t, c.entries, "fresh-a")
79+
assert.Contains(t, c.entries, "fresh-b")
80+
}
81+
82+
// TestNonceCache_EvictExpired_ExactDeadline verifies the boundary condition where
83+
// the deadline equals the current time. Because evictExpired uses now.After(deadline),
84+
// an entry whose deadline equals `now` (i.e. now.After(deadline) == false) should be
85+
// retained.
86+
func TestNonceCache_EvictExpired_ExactDeadline(t *testing.T) {
87+
c := newNonceCache()
88+
now := time.Now()
89+
90+
// Deadline exactly equals now — now.After(now) is false, so the entry should be kept.
91+
c.entries["exact-deadline"] = now
92+
93+
c.evictExpired(now)
94+
95+
assert.Contains(t, c.entries, "exact-deadline",
96+
"entry at exact deadline should not be evicted (now.After(deadline) is false when equal)")
97+
}
98+
99+
// TestNonceCache_EvictExpired_JustExpired verifies that an entry one nanosecond in the
100+
// past is evicted.
101+
func TestNonceCache_EvictExpired_JustExpired(t *testing.T) {
102+
c := newNonceCache()
103+
now := time.Now()
104+
justBefore := now.Add(-1) // one nanosecond before now
105+
106+
c.entries["just-expired"] = justBefore
107+
108+
c.evictExpired(now)
109+
110+
assert.NotContains(t, c.entries, "just-expired",
111+
"entry one nanosecond in the past should be evicted")
112+
}
113+
114+
// TestNonceCache_EvictExpired_AllExpiredThenFreshAdded verifies that after evicting
115+
// all expired entries, new entries can be added normally.
116+
func TestNonceCache_EvictExpired_AllExpiredThenFreshAdded(t *testing.T) {
117+
c := newNonceCache()
118+
past := time.Now().Add(-1 * time.Second)
119+
120+
c.entries["old-1"] = past
121+
c.entries["old-2"] = past
122+
123+
c.evictExpired(time.Now())
124+
require.Empty(t, c.entries, "all entries should be evicted")
125+
126+
// Now add a fresh entry via the public API
127+
assert.True(t, c.checkAndSet("new-nonce"), "should accept new nonce after full eviction")
128+
assert.Len(t, c.entries, 1)
129+
}
130+
131+
// TestNonceCache_EvictExpired_ManyEntries verifies correctness with a large number
132+
// of entries — half expired, half fresh.
133+
func TestNonceCache_EvictExpired_ManyEntries(t *testing.T) {
134+
c := newNonceCache()
135+
now := time.Now()
136+
past := now.Add(-1 * time.Second)
137+
future := now.Add(60 * time.Second)
138+
139+
const count = 100
140+
for i := range count {
141+
if i%2 == 0 {
142+
c.entries[fmt.Sprintf("expired-%d", i)] = past
143+
} else {
144+
c.entries[fmt.Sprintf("fresh-%d", i)] = future
145+
}
146+
}
147+
148+
require.Len(t, c.entries, count)
149+
150+
c.evictExpired(now)
151+
152+
assert.Len(t, c.entries, count/2, "exactly half the entries (fresh) should remain")
153+
for i := range count {
154+
key := fmt.Sprintf("expired-%d", i)
155+
freshKey := fmt.Sprintf("fresh-%d", i)
156+
if i%2 == 0 {
157+
assert.NotContains(t, c.entries, key, "expired entry should be removed")
158+
} else {
159+
assert.Contains(t, c.entries, freshKey, "fresh entry should remain")
160+
}
161+
}
162+
}
163+
164+
// TestNonceCache_EvictExpired_CalledByCheckAndSet verifies that evictExpired is
165+
// triggered by checkAndSet: entries added with a past deadline are cleaned up on
166+
// the next checkAndSet call.
167+
func TestNonceCache_EvictExpired_CalledByCheckAndSet(t *testing.T) {
168+
c := newNonceCache()
169+
past := time.Now().Add(-1 * time.Second)
170+
171+
// Directly plant an expired entry.
172+
c.entries["stale-nonce"] = past
173+
require.Len(t, c.entries, 1)
174+
175+
// checkAndSet internally calls evictExpired — "stale-nonce" should be cleaned up.
176+
ok := c.checkAndSet("fresh-nonce")
177+
assert.True(t, ok, "fresh nonce should be accepted")
178+
179+
assert.NotContains(t, c.entries, "stale-nonce", "expired entry should be evicted by checkAndSet")
180+
assert.Contains(t, c.entries, "fresh-nonce", "new entry should be present")
181+
}
182+
183+
// TestNonceCache_EvictExpired_CalledBySeenNonce verifies that evictExpired is also
184+
// triggered by seenNonce: expired entries are removed on every read path as well.
185+
func TestNonceCache_EvictExpired_CalledBySeenNonce(t *testing.T) {
186+
c := newNonceCache()
187+
past := time.Now().Add(-1 * time.Second)
188+
future := time.Now().Add(60 * time.Second)
189+
190+
c.entries["stale"] = past
191+
c.entries["live"] = future
192+
193+
require.Len(t, c.entries, 2)
194+
195+
// seenNonce on a non-existent nonce still triggers eviction.
196+
result := c.seenNonce("unknown-nonce")
197+
assert.False(t, result, "unseen nonce should return false")
198+
199+
assert.NotContains(t, c.entries, "stale", "expired entry should be evicted by seenNonce")
200+
assert.Contains(t, c.entries, "live", "live entry should still be present")
201+
}

0 commit comments

Comments
 (0)