Skip to content

fix(cache): let a cache mask its keys in the log - #160

Open
cosmin-staicu wants to merge 1 commit into
mainfrom
fix/log-keys-redaction
Open

fix(cache): let a cache mask its keys in the log#160
cosmin-staicu wants to merge 1 commit into
mainfrom
fix/log-keys-redaction

Conversation

@cosmin-staicu

@cosmin-staicu cosmin-staicu commented Sep 8, 2026

Copy link
Copy Markdown
Member

Closes #129.

Problem

Cache keys are written to logs, several of them at Warning. For the application's own caches that is usually harmless. For the IDistributedCache adapter it is not: those keys are the consumer's and can be secrets, such as the ASP.NET Core session id. A Redis blip raised the inner-cache warning paths and wrote session identifiers to the log (CWE-532).

Change

  • ICacheOptions.LogKeys, default true, on InMemoryCacheOptions, InMemoryRedisCacheOptions and RedisCacheOptions. The interface member has a default implementation, so hand-written ICacheOptions implementers keep compiling.
  • Masking keeps the prefix and hides the key. A masked key keeps whatever the key strategy composed in front of the caller's key and shows the first three characters of the key followed by ****: d:ses**** at the cache layer, myapp:dh:d:ses**** at the Redis layer. Each layer learns its prefix by composing a probe key through its own strategy; a custom strategy whose output does not end with the key yields no prefix, and the whole value is masked, since nothing in it is known to be safe. The Redis layer adds the cache-key prefix on top of its own. The adapter hands its own d: prefix to the tier copies it configures, because it neutralizes their key strategy.
  • LoggedKey, an internal struct whose ToString does the masking. The LoggerMessage generator only calls ToString when the level is enabled, so a masked Trace site on a hot path costs nothing when Trace is off.
  • Every generated log method that took a CacheKey or RedisKey now takes a LoggedKey and is named Log…Core; a same-named Log…(CacheKey) wrapper beside it applies the cache's switch. The 100-odd call sites are untouched, and a new call site cannot bypass the switch: a raw key does not convert to LoggedKey. Applied mechanically to MultilayerCache (22 methods), MultilayerHashCache (16), RedisCache (5), RedisHashCache (3) and CacheEventPublisher (1); the four joined-keys sites go through the same helper. Not done via CacheKey.ToString(), which the issue rules out because the implicit string conversion is used to compose physical keys.
  • The adapter sets LogKeys = false on the private copies of its tier's options, next to the existing CacheKeyStrategy neutralization, and masks the key in its own two messages.

Tests

  • LoggedKeyTests: verbatim, prefix kept and key masked, case-insensitive prefix match, whole-value masking without a matching prefix, short keys, prefix learning for the Redis and cache-key strategies including one that does not keep the key as a tail, join.
  • KeyLoggingTests: real caches through the container with a Trace-level capturing logger. With the switch off, no line contains the key and lines show sec****; with it on, the reverse. Same for the hash cache. The distributed adapter on the InMemory tier logs d:Sec**** even with the tier's own switch left at its default.
  • RedisCacheTests and RedisHashCacheTests: the miss line follows the switch, using the mocked logger.
  • UiPathDistributedCacheTests: the failed-write warning asserts the masked key and the absence of the raw one.

Docs: LogKeys rows in the three provider tables, the distributed-cache note rewritten from "keys appear in logs" to what now happens, appsettings.all.json gains the setting, changelog under Fixed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01V9jRarnMLeZRZ5rYySRhkh

@github-actions github-actions Bot added the needs-cla-review A maintainer should assess whether a signed CLA is required (see CONTRIBUTING.md) label Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown

🔎 Maintainer heads-up: automated triage flagged this PR as potentially material, so it may need a signed CLA in addition to the DCO sign-off.

Strong signals

  • adds public API surface (PublicAPI.Unshipped.txt in src/UiPath.Caching)

This is advisory only — the bot does not decide. Please judge against the CLA criteria (material, product-critical, patent-sensitive, corporate contributor, broad commercial use). Note that thresholds can be gamed by splitting PRs, so use your judgement.

  • If a CLA is needed → add the cla-required label (a contributor comment with signing steps is posted automatically).
  • If it is not needed → replace needs-cla-review with cla-not-required so later pushes don't re-flag it.

@cosmin-staicu
cosmin-staicu force-pushed the fix/log-keys-redaction branch 7 times, most recently from f7b63fa to 5a8db68 Compare September 8, 2026 16:31
ICacheOptions.LogKeys (default true) on the three options classes. Off,
every log line that named a key keeps the prefix the key strategy
composed and masks the caller's key to its first three characters
(myapp:s:cos****); a key not under a recognizable prefix is masked whole.
Each layer learns its prefix by composing a probe key through its own
strategy, the Redis layer adds the cache-key prefix on top, and the
adapter hands its own prefix to the tier copies it configures. Masking
runs only when the line is written. The generated log methods take a
LoggedKey and keep a CacheKey/RedisKey wrapper beside them, so no call
site changes and a raw key cannot reach them by accident.
AddDistributedCache turns the switch off for its own provider and masks
the key in its own two messages, because IDistributedCache keys are the
consumer's and can be secrets.

Closes #129

Signed-off-by: Cosmin Staicu <cosmin.staicu@uipath.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V9jRarnMLeZRZ5rYySRhkh
@cosmin-staicu cosmin-staicu changed the title fix(cache): let a cache keep its keys out of the log fix(cache): let a cache mask its keys in the log Sep 8, 2026
@sonarqubecloud

sonarqubecloud Bot commented Sep 8, 2026

Copy link
Copy Markdown

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

Labels

needs-cla-review A maintainer should assess whether a signed CLA is required (see CONTRIBUTING.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cache keys are logged at Warning level; distributed-cache keys can be secrets (session ids)

1 participant