From fa12947a64a45e0f2cee4bca301d89c0c4b76685 Mon Sep 17 00:00:00 2001
From: Lukasz Raczylo <lukasz@raczylo.com>
Date: Thu, 13 Aug 2026 16:44:07 +0100
Subject: [PATCH] R64: cap metadata cache grace by configured limits

---
 .../cache/review_r64_metadata_grace_test.go   | 69 +++++++++++++++++++
 internal/cache/typed_cache.go                 | 34 ++++++++-
 2 files changed, 101 insertions(+), 2 deletions(-)
 create mode 100644 internal/cache/review_r64_metadata_grace_test.go

diff --git a/internal/cache/review_r64_metadata_grace_test.go b/internal/cache/review_r64_metadata_grace_test.go
new file mode 100644
index 0000000..b9aba85
--- /dev/null
+++ b/internal/cache/review_r64_metadata_grace_test.go
@@ -0,0 +1,69 @@
+package cache
+
+import (
+	"testing"
+	"time"
+)
+
+// TestMetadataSetGraceCappedByMaxGrace verifies that MetadataCache.Set caps the
+// applied grace period by the configured MaxGracePeriod (and, for
+// security-critical metadata containing an operator-listed field, by
+// SecurityCriticalMaxGracePeriod). Previously only GracePeriod was applied and
+// the caps were configured but never read, so stale discovery metadata
+// (issuer, jwks_uri) could be served fresh beyond the intended bound.
+func TestMetadataSetGraceCappedByMaxGrace(t *testing.T) {
+	t.Run("grace capped by MaxGracePeriod", func(t *testing.T) {
+		base := New(DefaultConfig())
+		defer base.Close()
+
+		mc := NewMetadataCache(base, MetadataConfig{
+			GracePeriod:    time.Hour,
+			MaxGracePeriod: 5 * time.Minute,
+		})
+
+		start := time.Now()
+		baseTTL := time.Minute
+		if err := mc.Set("https://issuer.example", &ProviderMetadata{Issuer: "https://issuer.example"}, baseTTL); err != nil {
+			t.Fatal(err)
+		}
+
+		item, ok := base.items["metadata:https://issuer.example"]
+		if !ok {
+			t.Fatalf("metadata not cached")
+		}
+		// Grace must be capped at 5m, not the full 1h.
+		want := start.Add(baseTTL + 5*time.Minute)
+		if item.ExpiresAt.After(want.Add(2 * time.Second)) {
+			t.Fatalf("grace not capped by MaxGracePeriod: ExpiresAt=%v want ~%v", item.ExpiresAt, want)
+		}
+	})
+
+	t.Run("security-critical metadata uses tighter cap", func(t *testing.T) {
+		base := New(DefaultConfig())
+		defer base.Close()
+
+		mc := NewMetadataCache(base, MetadataConfig{
+			GracePeriod:                    time.Hour,
+			MaxGracePeriod:                 5 * time.Minute,
+			SecurityCriticalMaxGracePeriod: time.Minute,
+			SecurityCriticalFields:         []string{"issuer", "jwks_uri"},
+		})
+
+		start := time.Now()
+		baseTTL := time.Minute
+		// Metadata carries the security-critical issuer field.
+		if err := mc.Set("https://secure.example", &ProviderMetadata{Issuer: "https://secure.example", JWKSUri: "https://secure.example/jwks"}, baseTTL); err != nil {
+			t.Fatal(err)
+		}
+
+		item, ok := base.items["metadata:https://secure.example"]
+		if !ok {
+			t.Fatalf("metadata not cached")
+		}
+		// Grace must be capped at 1m (the security-critical cap), not 5m or 1h.
+		want := start.Add(baseTTL + time.Minute)
+		if item.ExpiresAt.After(want.Add(2 * time.Second)) {
+			t.Fatalf("security-critical grace not capped by SecurityCriticalMaxGracePeriod: ExpiresAt=%v want ~%v", item.ExpiresAt, want)
+		}
+	})
+}
diff --git a/internal/cache/typed_cache.go b/internal/cache/typed_cache.go
index bc50f05..c8d8c34 100644
--- a/internal/cache/typed_cache.go
+++ b/internal/cache/typed_cache.go
@@ -161,6 +161,25 @@ func NewMetadataCache(baseCache *Cache, config MetadataConfig) *MetadataCache {
 	}
 }
 
+// isSecurityCritical reports whether the metadata contains any operator-listed
+// security-critical field (e.g. issuer, jwks_uri) whose staleness window
+// should be capped tighter than ordinary metadata.
+func (mc *MetadataCache) isSecurityCritical(m *ProviderMetadata) bool {
+	for _, f := range mc.config.SecurityCriticalFields {
+		switch f {
+		case "issuer":
+			if m.Issuer != "" {
+				return true
+			}
+		case "jwks_uri":
+			if m.JWKSUri != "" {
+				return true
+			}
+		}
+	}
+	return false
+}
+
 // Set stores provider metadata with grace period support
 func (mc *MetadataCache) Set(providerURL string, metadata *ProviderMetadata, ttl time.Duration) error {
 	if metadata == nil {
@@ -169,9 +188,20 @@ func (mc *MetadataCache) Set(providerURL string, metadata *ProviderMetadata, ttl
 
 	key := "metadata:" + providerURL
 
-	// Apply grace period if configured
+	// Apply grace period if configured, honoring the configured caps. Previously
+	// only GracePeriod was read and applied unboundedly; MaxGracePeriod and
+	// SecurityCriticalMaxGracePeriod were configured but had no effect, so
+	// stale discovery metadata (issuer, jwks_uri) could be served fresh for
+	// longer than the operator's intended bound.
 	if mc.config.GracePeriod > 0 {
-		ttl += mc.config.GracePeriod
+		grace := mc.config.GracePeriod
+		if mc.config.MaxGracePeriod > 0 && grace > mc.config.MaxGracePeriod {
+			grace = mc.config.MaxGracePeriod
+		}
+		if mc.isSecurityCritical(metadata) && mc.config.SecurityCriticalMaxGracePeriod > 0 && grace > mc.config.SecurityCriticalMaxGracePeriod {
+			grace = mc.config.SecurityCriticalMaxGracePeriod
+		}
+		ttl += grace
 	}
 
 	// Store as JSON for consistency