Skip to content

feat(caching): namespace every Redis key and channel with CachingOptions:KeyPrefix - #1450

Open
maxiar wants to merge 1 commit into
fullstackhero:mainfrom
maxiar:feat/redis-key-prefix-upstream
Open

maxiar wants to merge 1 commit into
fullstackhero:mainfrom
maxiar:feat/redis-key-prefix-upstream

Conversation

@maxiar

@maxiar maxiar commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Every key the framework writes to Redis is unprefixed: cache entries (perm:u:{id}, theme:t:root, theme:default), HybridCache tag invalidations, the DataProtection-Keys ring, idempotency locks and quota:* counters. The SignalR backplane uses a fixed fsh-signalr channel prefix. Two applications built on the kit, or the QA and Production deployments of one app, therefore collide as soon as they point at the same Redis server.

A defaultDatabase=N index in the connection string, which is the usual workaround, only partly helps. It separates keys, but Redis pub/sub is server-wide and ignores the database index. So every app on the server still shares one SignalR backplane. AppHub broadcasts to tenant:{id} and user:{id} groups, and tenant:root exists in every app. A presence or notification broadcast in one app therefore reaches clients connected to another.

Within a shared database, the cache leaks too. An integration test in this PR shows it: when app A calls RemoveByTagAsync("permissions"), app B's permission entries on the same server are evicted as well. Entries for the same logical key (theme:t:root) are read across apps.

Change

This PR adds one setting, CachingOptions:KeyPrefix, and applies it everywhere the framework talks to Redis:

Consumer Before With KeyPrefix = "MyApp:prod"
HybridCache L2 entries + tag records (RedisCacheOptions.InstanceName) theme:t:root MyApp:prod:theme:t:root
Data Protection key ring DataProtection-Keys MyApp:prod:DataProtection-Keys
Idempotency reservation lock (talks to the multiplexer directly) lock:idem:… MyApp:prod:lock:idem:…
Quota counters (QuotaOptions:KeyPrefix overrides, since quota may use its own Redis) quota:… MyApp:prod:quota:…
SignalR backplane channel fsh-signalr MyApp:prod:signalr
  • Normalization. A small shared helper, FSH.Framework.Core.Common.RedisKeyPrefix, normalizes the value to empty or exactly one trailing :. It lives in Core because Caching, Web and Quota all reference Core, while Quota does not reference Caching.
  • Backward compatible. An empty prefix produces exactly today's keys and channel. CacheKeys is unchanged, because the prefix is applied underneath it.
  • Template defaults. The template ships "KeyPrefix": "FSH.Starter" in appsettings.json and "FSH.Starter:prod" in appsettings.Production.json. sourceName renames these per project, so a new scaffold is isolated per app and per environment out of the box.
  • Rollout cost. Setting or changing the prefix on an existing deployment rotates the Data Protection key ring, so issued auth cookies and reset tokens stop validating once. It also starts a cold cache. Projects that upgrade through a template merge should expect that one-time cost.

.agents/rules/caching.md and realtime.md are updated. The docs-site page and changelog entry (golden rule 10) will follow as a PR to fullstackhero/docs once the shape here is agreed.

Verification

dotnet test src/FSH.Starter.slnx   -> every suite passes, including Integration.Tests 793/793 (Testcontainers)

New tests:

  • HybridCacheRedisTests (real Valkey).
    • Two prefixes on one server don't read each other's entries.
    • The raw key in Redis carries the prefix.
    • The Data Protection ring lands under the prefix.
    • Tag invalidation in one app does not evict the other app's entries. As a negative control, I ran this test with both prefixes empty and it fails, which confirms the cross-app eviction exists on main today.
  • RedisKeyPrefixWiringTests.
    • The SignalR channel prefix is fsh-signalr without KeyPrefix and {prefix}:signalr with it.
    • Quota inherits the caching prefix, and its own setting wins when present.
  • IdempotencyEndpointFilterReplayTests. The reservation key starts with the prefix.
  • ShippedConfigurationTests. Development and Production ship distinct, non-empty prefixes.
  • RedisKeyPrefixTests. Normalization cases.

The runs used -p:NuGetAudit=false, because main currently fails restore on the crypto advisories that #1447 fixes.

🤖 Generated with Claude Code

…ons:KeyPrefix

Every key the framework writes to Redis was unprefixed (cache entries, HybridCache
tag invalidations, the Data Protection key ring, idempotency locks, quota counters)
and the SignalR backplane used a fixed `fsh-signalr` channel. Applications or
environments sharing one Redis server collided; a defaultDatabase index does not
help the backplane, because pub/sub ignores the database.

CachingOptions:KeyPrefix (normalized by Core's RedisKeyPrefix) now namespaces all
of them: RedisCacheOptions.InstanceName, the Data Protection list key, the
idempotency reservation, quota counters (QuotaOptions:KeyPrefix overrides) and the
SignalR channel prefix. Empty keeps today's layout. The template ships FSH.Starter /
FSH.Starter:prod, renamed per project by sourceName.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@iammukeshm iammukeshm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @maxiar, this is a good find. The pub/sub point is the part most people miss: defaultDatabase doesn't separate the SignalR backplane, and your tag-invalidation test shows the cache leaking across apps on main today. The BuildingBlocks change is justified here, because every Redis touchpoint lives there. I checked the coverage: health checks, AppHub and the SSE tokens go through IDistributedCache, so InstanceName covers them, and nothing else talks to Redis. It merges cleanly with main, builds with zero warnings, and Framework (360), Caching (31), Architecture (55) and the Valkey HybridCacheRedisTests (7) all pass.

The code is backward compatible with an empty prefix, but the shipped config isn't, and that's the blocker.

  1. The shipped default rotates the Data Protection key ring (critical). With KeyPrefix set in appsettings.json:69 and appsettings.Production.json:44, an existing deployment moves from DataProtection-Keys to FSH.Starter:prod:DataProtection-Keys on upgrade, and the new ring is empty. That's worse than cookies and reset tokens failing once. WebhookSecretProtector stores subscription signing secrets in the database protected with DP, so after the upgrade Unprotect throws and every existing webhook subscription can never sign again. Please ship KeyPrefix empty, make it opt-in, and document the one-line migration (COPY DataProtection-Keys <prefix>:DataProtection-Keys) for anyone turning it on. That also means dropping or flipping Environments_Should_ShipDistinctRedisKeyPrefixes.
  2. The migrator and the API get different prefixes in Production. FSH.Starter.DbMigrator.csproj links appsettings.json and appsettings.Development.json but not appsettings.Production.json. Under compose or Terraform, the migrator would run as FSH.Starter and the API as FSH.Starter:prod. If the default is empty this mostly goes away, but please make sure both read the same value wherever it's set (compose, Terraform, AppHost.cs).
  3. Add a test for the invariant that protects existing deployments. An empty KeyPrefix must give InstanceName == "", the DP key exactly DataProtection-Keys, and the fsh-signalr channel prefix.
  4. Rolling deploys split the backplane. While old pods are on fsh-signalr and new pods are on {prefix}:signalr, cross-pod broadcasts are lost until the rollout finishes. One line in the docs is enough.
  5. Validate the prefix at startup. A { or } becomes a Redis Cluster hash tag and pins every key to one slot, and whitespace is accepted too. Reject both.

One nit: Quota/Extensions.cs:27-29 patches the singleton QuotaOptions but not the bound IOptions<QuotaOptions>, so the two disagree. It's harmless today, but it's easy to trip on later.

Last thing: this needs a companion PR in fullstackhero/docs with a changelog entry and the setting documented on the caching page, plus a mention in deploy/docker/README.md. With the default flipped to empty, this is close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants