Redacting customer information when hide_user_data_from_log is true in rdb.c, networking.c, debug.c and t_hash by zackcam · Pull Request #3872 · valkey-io/valkey

zackcam · 2026-05-28T21:31:20Z

Removing logging key names when hide_user_data_from_log is true in rdb.c file as well as networking.c, t_hash.c and debug.c files

Made simple if else in order to distinguish for read ability

Signed-off-by: zackcam <zackcam@amazon.com>

coderabbitai · 2026-05-28T21:31:36Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 849eaae9-5448-44d5-92cd-be3566e01ebe

📥 Commits

Reviewing files that changed from the base of the PR and between 0d6fe00 and 175928e.

📒 Files selected for processing (4)

src/debug.c
src/networking.c
src/rdb.c
src/t_hash.c

🚧 Files skipped from review as they are similar to previous changes (4)

src/debug.c
src/networking.c
src/t_hash.c
src/rdb.c

📝 Walkthrough

Walkthrough

This PR redacts user/key/query contents from selected logs when server.hide_user_data_from_log is enabled, adjusting RDB save/load warnings, stream consumer-group reports, runtime debug key prints, networking QB-limit warnings, and a hash listpack diagnostic.

Changes

Logging privacy redaction

Layer / File(s)	Summary
RDB save path: key-version warning `src/rdb.c`	`rdbSaveKeyValuePair()` now emits the key-version incompatibility warning in a generic form when `server.hide_user_data_from_log` is set; otherwise it includes the key string.
RDB load path: empty/duplicate key and stream-cg warnings `src/rdb.c`	RDB loader messages for `RDB_LOAD_ERR_EMPTY_KEY`, duplicated-key detection, and duplicated stream consumer-group name reports now omit the key/group name when `server.hide_user_data_from_log` is enabled.
Runtime debug: found-key context printing `src/debug.c`	`logCurrentClient()` prints a fixed `key 'redacted'` message when `server.hide_user_data_from_log` is set; otherwise it logs the actual key and object debug info.
Networking: query-buffer limit warning `src/networking.c`	`handleQbLimitReached()` no longer logs a `c->querybuf` preview when `server.hide_user_data_from_log` is set; it logs a generic client-closing message instead.
Hash corruption diagnostic `src/t_hash.c`	The `serverLogHexDump` diagnostic for duplicated listpack elements is skipped when `server.hide_user_data_from_log` is enabled; the `serverPanic("Listpack corruption detected")` behavior is unchanged.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main objective: redacting customer information when hide_user_data_from_log is true across multiple files (rdb.c, networking.c, debug.c, and t_hash).
Description check	✅ Passed	The description is directly related to the changeset, explaining the removal of key names from logs when hide_user_data_from_log is enabled and mentioning the implementation approach.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/rdb.c (1)

1221-1226: Request @core-team architectural review.

Changes to rdb.c require architectural review by the core team. As per coding guidelines: "Request @core-team architectural review for changes to cluster*.c, replication.c, rdb.c, or aof.c"

Also applies to: 3465-3471, 3516-3520

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/rdb.c` around lines 1221 - 1226, This change touches rdb.c (lines around
server.hide_user_data_from_log and the serverLog calls) and therefore requires
an explicit `@core-team` architectural review per project guidelines; update the
PR description and/or add a top-level review request so the core team is
notified (mention `@core-team` and note the modified symbols:
server.hide_user_data_from_log and the serverLog warning messages that print
key/db/rdbver) and ensure you include the other affected hunks (around the same
file at the other occurrences referenced) in the review request.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/rdb.c`:
- Around line 1221-1226: This change touches rdb.c (lines around
server.hide_user_data_from_log and the serverLog calls) and therefore requires
an explicit `@core-team` architectural review per project guidelines; update the
PR description and/or add a top-level review request so the core team is
notified (mention `@core-team` and note the modified symbols:
server.hide_user_data_from_log and the serverLog warning messages that print
key/db/rdbver) and ensure you include the other affected hunks (around the same
file at the other occurrences referenced) in the review request.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 059afe18-559e-46ed-9158-61b6a0293769

📥 Commits

Reviewing files that changed from the base of the PR and between 7c3821f and e5e5625.

📒 Files selected for processing (1)

src/rdb.c

coderabbitai · 2026-05-28T21:44:13Z

Actionable comments posted: 0

coderabbitai · 2026-05-28T22:36:56Z

Actionable comments posted: 0

Signed-off-by: zackcam <zackcam@amazon.com>

codecov · 2026-05-29T05:47:36Z

Codecov Report

❌ Patch coverage is 26.08696% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.53%. Comparing base (e4fdae4) to head (175928e).
⚠️ Report is 1 commits behind head on unstable.

Files with missing lines	Patch %	Lines
src/rdb.c	23.07%	10 Missing ⚠️
src/debug.c	0.00%	3 Missing ⚠️
src/t_hash.c	0.00%	3 Missing ⚠️
src/networking.c	75.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #3872      +/-   ##
============================================
- Coverage     76.69%   76.53%   -0.16%     
============================================
  Files           162      162              
  Lines         80679    80694      +15     
============================================
- Hits          61874    61760     -114     
- Misses        18805    18934     +129

Files with missing lines	Coverage Δ
src/networking.c	`92.19% <75.00%> (+0.06%)`	⬆️
src/debug.c	`55.41% <0.00%> (+0.24%)`	⬆️
src/t_hash.c	`95.35% <0.00%> (-0.08%)`	⬇️
src/rdb.c	`76.59% <23.07%> (-0.57%)`	⬇️

... and 13 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…n rdb.c, networking.c, debug.c and t_hash (#3872) Removing logging key names when hide_user_data_from_log is true in rdb.c file as well as networking.c, t_hash.c and debug.c files Made simple if else in order to distinguish for read ability --------- Signed-off-by: zackcam <zackcam@amazon.com>

# Backport sweep for 9.1 Automated cherry-picks from PRs marked "To be backported". ## Applied | Source PR | Title | Detail | |---|---|---| | #3743 | Fix buffered_reply assert in HFE commands with module keyspace notifications | cherry-picked in a prior sweep | | #3766 | Fix flaky block_keyspace_notification test for HGETDEL notify race | cherry-picked in a prior sweep | | #3800 | Fix heap-use-after-free in ACL LOAD when client free is deferred | cherry-picked in a prior sweep | | #3723 | Fix double-finish and RESP reply violation in cluster slot migration | cherry-picked in a prior sweep | | #3872 | Redacting customer information when hide_user_data_from_log is true in rdb.c, networking.c, debug.c and t_hash | cherry-picked in a prior sweep | | #3846 | Fix use-after-free in VM_RegisterClusterMessageReceiver | cherry-picked in a prior sweep | | #3806 | Add ALL_DBS flag to CLUSTER FLUSHSLOT for database-level ACL | cherry-picked in a prior sweep | | #3847 | Harden SENTINEL commands and config rewrite against control-character injection | | | #3801 | Validate every DB clause in COPY against ACL db= permissions | | | #3851 | Replace AUTOMATION_PAT with Valkeyrie Bot GitHub App token | | | #3848 | Fix cluster AUX-field control-character and delimiter injection | | | #3544 | Revert "IO-Threads redesign cleanup work (#3544)" | cherry-picked in a prior sweep | | #3888 | Report exact dbid for COPY in ACL LOG when db= access is denied | conflicts resolved by Claude Code | | #3942 | Fix shard_id format specifier in UPDATE message log | | | #3941 | Avoid random() % 0 undefined behaviour when cluster-node-timeout < 30 | | --- *Generated by valkey-ci-agent using Claude Code.* --------- Signed-off-by: Binbin <binloveplay1314@qq.com> Signed-off-by: Ran Shidlansik <ranshid@amazon.com> Signed-off-by: chx9 <lovelypiska@outlook.com> Signed-off-by: zackcam <zackcam@amazon.com> Signed-off-by: Eran Ifrah <eifrah@amazon.com> Signed-off-by: Jules Lasarte <lasartej@amazon.com> Signed-off-by: jjuleslasarte <jules.lasarte@gmail.com> Signed-off-by: akash kumar <akumdev@amazon.com> Co-authored-by: Binbin <binloveplay1314@qq.com> Co-authored-by: Ran Shidlansik <ranshid@amazon.com> Co-authored-by: lovelypiska <lovelypiska@outlook.com> Co-authored-by: zackcam <zackcam@amazon.com> Co-authored-by: eifrah-aws <eifrah@amazon.com> Co-authored-by: jjuleslasarte <jules.lasarte@gmail.com> Co-authored-by: Jules Lasarte <lasartej@amazon.com> Co-authored-by: Akash Kumar <45854686+akashkgit@users.noreply.github.com>

…n rdb.c, networking.c, debug.c and t_hash (#3872) Removing logging key names when hide_user_data_from_log is true in rdb.c file as well as networking.c, t_hash.c and debug.c files Made simple if else in order to distinguish for read ability --------- Signed-off-by: zackcam <zackcam@amazon.com>

…n rdb.c, networking.c, debug.c and t_hash (valkey-io#3872) (#346) Removing logging key names when hide_user_data_from_log is true in rdb.c file as well as networking.c, t_hash.c and debug.c files Made simple if else in order to distinguish for read ability --------- Signed-off-by: zackcam <zackcam@amazon.com> Co-authored-by: zackcam <zackcam@amazon.com>

Redacting key names in rdb.c when hide_user_data_from_log is true

e5e5625

Signed-off-by: zackcam <zackcam@amazon.com>

github-actions Bot assigned zackcam May 28, 2026

coderabbitai Bot reviewed May 28, 2026

View reviewed changes

zackcam changed the title ~~Redacting key names in rdb.c when hide_user_data_from_log is true~~ Redacting customer information when hide_user_data_from_log is true in rdb.c, netowrking.c and t_hash May 28, 2026

zackcam changed the title ~~Redacting customer information when hide_user_data_from_log is true in rdb.c, netowrking.c and t_hash~~ Redacting customer information when hide_user_data_from_log is true in rdb.c, networking.c and t_hash May 28, 2026

zackcam force-pushed the rdb-redaction branch from 8c6e062 to 0d6fe00 Compare May 28, 2026 22:33

zackcam changed the title ~~Redacting customer information when hide_user_data_from_log is true in rdb.c, networking.c and t_hash~~ Redacting customer information when hide_user_data_from_log is true in rdb.c, networking.c, debug.c and t_hash May 28, 2026

Removing some more logs in t_hash, networking and debug

175928e

Signed-off-by: zackcam <zackcam@amazon.com>

zackcam force-pushed the rdb-redaction branch from 0d6fe00 to 175928e Compare May 28, 2026 23:46

madolson approved these changes May 29, 2026

View reviewed changes

madolson added this to Valkey 9.0, Valkey 9.1, Valkey 8.1 and Valkey 8.0 May 29, 2026

madolson moved this to To be backported in Valkey 8.0 May 29, 2026

madolson moved this to To be backported in Valkey 8.1 May 29, 2026

madolson moved this to To be backported in Valkey 9.0 May 29, 2026

madolson moved this to To be backported in Valkey 9.1 May 29, 2026

madolson added the release-notes This issue should get a line item in the release notes label May 29, 2026

madolson merged commit 2035fb3 into valkey-io:unstable May 29, 2026
100 checks passed

valkeyrie-ops Bot mentioned this pull request May 22, 2026

[backport] Backport sweep for 9.1 #3774

Merged

valkeyrie-ops Bot mentioned this pull request May 21, 2026

[backport] Backport sweep for 8.1 #3737

Merged

valkeyrie-ops Bot mentioned this pull request May 21, 2026

[backport] Backport sweep for 8.0 #3736

Merged

valkeyrie-ops Bot mentioned this pull request May 27, 2026

[backport][validation failed] Backport sweep for 9.0 #3735

Closed

zuiderkwast moved this from To be backported to 8.1.8 (WIP) in Valkey 8.1 Jun 2, 2026

sarthakaggarwal97 moved this from To be backported to Done in Valkey 9.1 Jun 15, 2026

valkeyrie-ops Bot moved this from To be backported to Done in Valkey 8.0 Jun 18, 2026

valkeyrie-ops Bot mentioned this pull request Jun 18, 2026

[backport] Backport sweep for 9.0 #3879

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Redacting customer information when hide_user_data_from_log is true in rdb.c, networking.c, debug.c and t_hash #3872

Redacting customer information when hide_user_data_from_log is true in rdb.c, networking.c, debug.c and t_hash #3872
madolson merged 2 commits into
valkey-io:unstablefrom
zackcam:rdb-redaction

zackcam commented May 28, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented May 28, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot commented May 28, 2026

Uh oh!

coderabbitai Bot commented May 28, 2026

Uh oh!

codecov Bot commented May 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

zackcam commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented May 28, 2026

Uh oh!

coderabbitai Bot commented May 28, 2026

Uh oh!

codecov Bot commented May 29, 2026

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

zackcam commented May 28, 2026 •

edited

Loading

coderabbitai Bot commented May 28, 2026 •

edited

Loading