chore: Fix unbounded channel

For the refresh JWKS operation, a worker fiber was reading from a notification channel that had no buffering.

This has two issues. The first issue has to do with memory growth, but the second is more subtle. When we don't supply any bounds, the code needs to be capable to handle concurrent requests and the worker must be able to dedup notifications. While this is what happens now, a future iteration might introduce a bugthat removes the dedup logic.
This commit is contained in:
Miguel da Costa Martins Marcelino
2026-09-17 08:21:12 +00:00
parent a1cb032df2
commit 8985ebd469
2 changed files with 29 additions and 2 deletions
+7 -2
View File
@@ -27,6 +27,9 @@ const (
quickTunnelAuthBrokerJWKSRefreshCooldown = time.Minute
// Three attempts allow one initial request and two transient-failure retries.
quickTunnelAuthBrokerJWKSMaxFetchAttempts = 3
// One pending notification is sufficient because the worker serializes
// refreshes and rechecks the cache before fetching.
quickTunnelAuthBrokerJWKSRefreshNotificationCapacity = 1
// Backoffs of 100ms and 200ms add at most 300ms within the five-second
// refresh budget while giving brief transient failures time to clear.
quickTunnelAuthBrokerJWKSInitialRetryBackoff = 100 * time.Millisecond
@@ -83,7 +86,7 @@ func NewQuickTunnelAuthAssertionValidator() (*QuickTunnelAuthAssertionValidator,
return nil, fmt.Errorf("parse Quick Tunnel authentication broker JWKS URL: %w", err)
}
workerCtx, cancelRefreshWorker := context.WithCancel(context.Background())
workerCtx, cancelRefreshWorker := context.WithCancel(context.Background()) //nolint:gosec
validator := &QuickTunnelAuthAssertionValidator{
jwksURL: *jwksURL,
httpClient: &http.Client{
@@ -93,7 +96,7 @@ func NewQuickTunnelAuthAssertionValidator() (*QuickTunnelAuthAssertionValidator,
},
},
now: time.Now,
refreshNotifications: make(chan quickTunnelAuthBrokerJWKSRefreshNotification),
refreshNotifications: make(chan quickTunnelAuthBrokerJWKSRefreshNotification, quickTunnelAuthBrokerJWKSRefreshNotificationCapacity),
cancelRefreshWorker: cancelRefreshWorker,
refreshWorkerStopped: make(chan struct{}),
}
@@ -162,6 +165,8 @@ func (v *QuickTunnelAuthAssertionValidator) notifyJWKSRefresh(ctx context.Contex
select {
case err := <-notification.result:
return err
case <-v.refreshWorkerStopped:
return errQuickTunnelAuthBrokerJWKSRefreshWorkerStopped
case <-ctx.Done():
return ctx.Err()
}
+22
View File
@@ -37,6 +37,7 @@ func TestNewQuickTunnelAuthAssertionValidator(t *testing.T) {
require.ErrorIs(t, validator.httpClient.CheckRedirect(nil, nil), http.ErrUseLastResponse)
require.NotNil(t, validator.now)
assert.Empty(t, validator.jwks.keySet.Keys)
assert.Equal(t, quickTunnelAuthBrokerJWKSRefreshNotificationCapacity, cap(validator.refreshNotifications))
}
func TestQuickTunnelAuthAssertionValidatorCloseStopsRefreshWorker(t *testing.T) {
@@ -52,6 +53,27 @@ func TestQuickTunnelAuthAssertionValidatorCloseStopsRefreshWorker(t *testing.T)
assert.Nil(t, key)
}
func TestQuickTunnelAuthAssertionValidatorQueuedRefreshObservesStoppedWorker(t *testing.T) {
t.Parallel()
validator := &QuickTunnelAuthAssertionValidator{
refreshNotifications: make(chan quickTunnelAuthBrokerJWKSRefreshNotification, quickTunnelAuthBrokerJWKSRefreshNotificationCapacity),
refreshWorkerStopped: make(chan struct{}),
}
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
defer cancel()
refreshResult := make(chan error, 1)
go func() {
refreshResult <- validator.notifyJWKSRefresh(ctx, "test-key")
}()
require.Eventually(t, func() bool {
return len(validator.refreshNotifications) == quickTunnelAuthBrokerJWKSRefreshNotificationCapacity
}, time.Second, time.Millisecond)
close(validator.refreshWorkerStopped)
require.ErrorIs(t, <-refreshResult, errQuickTunnelAuthBrokerJWKSRefreshWorkerStopped)
}
func TestQuickTunnelAuthAssertionValidatorVerificationKeyCachesAndRefreshes(t *testing.T) {
t.Parallel()