📢 แจ้งเตือน: valkey io/valkey/pull/

madolson · 2026-08-03T21:41:40Z

In an earlier commit we didn't properly reset the redaction bitmap during exec, and used the wrong one for lua scripts. This fixes that to properly redact commands. Added new regression tests and all existing tests still pass.

Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>

coderabbitai · 2026-08-03T21:42:01Z

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: b7629007-74f9-4467-a93b-05d54fd3be24

📥 Commits

Reviewing files that changed from the base of the PR and between a945648 and 1d6fe5d.

📒 Files selected for processing (1)

src/server.c

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

src/server.c

📝 Walkthrough

Walkthrough

Command logging now resolves script command identity from the calling client, preserves executing-client metrics, and resets argument-redaction state before each command. Tests cover redaction across transactions and scripted MIGRATE AUTH2 execution.

Changes

Command log correctness

Layer / File(s)	Summary
Caller identity and execution metrics `src/commandlog.c`	Command log entries resolve identity from the script caller while retaining metrics and redaction context from the executing client.
Redaction state reset and validation `src/server.c`, `tests/unit/commandlog.tcl`	`call()` clears the redaction bitmap before each command. Tests verify transaction state isolation and scripted `MIGRATE AUTH2` redaction.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

valkey-io/valkey#4267: Both changes modify command-log behavior and tests, although they address different concerns.
valkey-io/valkey#4316: Both changes address redaction bitmap state between commands at different execution points.

Suggested reviewers: zuiderkwast, rjd15372

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the primary change: resetting the redaction bitmap between commands in a transaction.
Description check	✅ Passed	The description accurately covers the redaction fix, Lua script handling, regression tests, and test status.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

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.}

codecov · 2026-08-03T22:06:54Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.90%. Comparing base (f75e2d5) to head (1d6fe5d).
⚠️ Report is 8 commits behind head on unstable.

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #4323      +/-   ##
============================================
+ Coverage     76.87%   76.90%   +0.03%     
============================================
  Files           162      162              
  Lines         81497    81515      +18     
============================================
+ Hits          62653    62693      +40     
+ Misses        18844    18822      -22

Files with missing lines	Coverage Δ
src/commandlog.c	`95.68% <100.00%> (-0.11%)`	⬇️
src/server.c	`89.49% <100.00%> (+<0.01%)`	⬆️

... and 23 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.

Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>

rainsupreme

LGTM overall!

Note the other paths where a client executes several commands without an intervening resetClient(): RM_Call sequences on a reused module temp client and a script's server.call() chain, not just queued MULTI commands. Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>

) `unit/commandlog` fails on unstable at 917de6a: ``` *** [err]: COMMANDLOG slow - Redaction does not leak to later commands in a MULTI in tests/unit/commandlog.tcl Expected 'set foo bar' to be equal to 'exec' (context: type eval line 12 cmd {assert_equal {set foo bar} [lindex [lindex $slowlog_resp 0] 3]} proc ::test) ``` #4267 dropped `SKIP_COMMANDLOG` from EXEC (`src/commands/exec.json:12`), so EXEC is now logged after the commands it ran, which puts it at entry 0 of the newest-first `COMMANDLOG GET`. The redaction test at `tests/unit/commandlog.tcl:212`, added by #4323, still reads entry 0 and gets the EXEC instead of the SET. Neither PR was rebased on the other, so this only broke on merge and not in either PR's CI. The log after `MULTI; ACL SETUSER commandlog-test-user +get; SET foo bar; EXEC`: | Entry | Command | |---|---| | 0 | `exec` | | 1 | `set foo bar` | | 2 | `acl setuser (redacted) (redacted)` | Read the SET from entry 1 instead. The other tests #4267 touched already assert entry 0 is `exec` and entry 1 is the inner command, so this matches. Also assert entries 0 and 2 rather than only the SET. The point of the test is that the ACL SETUSER redaction stops at the ACL SETUSER, and asserting entry 2 is redacted is what keeps it from passing if redaction breaks entirely. Asserting entry 0 is `exec` makes the index arithmetic fail loudly next time the entry order changes, instead of silently comparing against the wrong entry the way it just did. ## Testing `unit/commandlog` goes from 29 passed, 1 failed to 30 passed. `unit/slowlog` and `unit/multi` are unchanged at 100 passed. Reverting the `src/` hunks of 917de6a confirms that commit is the trigger. The new entry-0 assert fails, along with the two EXEC tests 917de6a added: ``` *** [err]: COMMANDLOG slow - Redaction does not leak to later commands in a MULTI in tests/unit/commandlog.tcl Expected 'exec' to be equal to 'set foo bar' (context: type eval line 13 cmd {assert_equal {exec} [lindex [lindex $slowlog_resp 0] 3]} proc ::test) *** [err]: COMMANDLOG slow - EXEC is logged alongside slow inner commands in tests/unit/commandlog.tcl Expected '1' to be equal to '2' (context: type eval line 8 cmd {assert_equal [r commandlog len slow] 2} proc ::test) *** [err]: COMMANDLOG slow - EXEC records total transaction time when inner commands are individually fast in tests/unit/commandlog.tcl Expected '0' to be equal to '1' (context: type eval line 9 cmd {assert_equal [r commandlog len slow] 1} proc ::test) ``` *This was generated by AI but verified, with love, by a human.* Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>

In an earlier commit we didn't properly reset the redaction bitmap during exec, and used the wrong one for lua scripts. This fixes that to properly redact commands. Added new regression tests and all existing tests still pass. --------- Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>

This PR contains the following updates: | Package | Update | Change | |---|---|---| | [valkey/valkey](https://github.com/valkey-io/valkey) | patch | `9.1.1` → `9.1.2` | --- > ⚠️ **Warning** > > Some dependencies could not be looked up. Check the [Dependency Dashboard](issues/6) for more information. --- ### Release Notes <details> <summary>valkey-io/valkey (valkey/valkey)</summary> ### [`v9.1.2`](https://github.com/valkey-io/valkey/releases/tag/9.1.2) [Compare Source](valkey-io/valkey@9.1.1...9.1.2) ## Valkey 9.1.2 - Released Mon 31 August 2026 Upgrade urgency SECURITY: This release includes security fixes we recommend you apply as soon as possible. ##### Security Fixes - GHSA-jcj7-v34w-v9vv: Fix a use-after-free in RDMA connection handling that could allow an authenticated client to crash the server using CLIENT KILL. Only affects servers built with USE\_RDMA and configured with an RDMA listener ([#4534](valkey-io/valkey#4534)) - GHSA-fq2f-crmw-q97r: Fix an unauthenticated use-after-free of the Lua interpreter state, caused by a process-global script debugger command table that cached a raw pointer to a freed interpreter and was never invalidated ([#4574](valkey-io/valkey#4574)) ##### Bug Fixes - Fix a double-free crash when a module timer callback stops its own timer with `ValkeyModule_StopTimer` by [@quanyeyang](https://github.com/quanyeyang) ([#4211](valkey-io/valkey#4211)) - Fix torn RESP3 push frames when a client publishes to a channel it is also subscribed to, which could desync client libraries by [@quanyeyang](https://github.com/quanyeyang) ([#4253](valkey-io/valkey#4253)) - Listpacks are now always validated on RDB load and RESTORE, preventing deferred assertion crashes; `sanitize-dump-payload` and its ACL flags become no-ops by [@jjuleslasarte](https://github.com/jjuleslasarte) ([#3721](valkey-io/valkey#3721)) - Fix crashes, hangs, and CPU spinning when the RDMA transport is used together with I/O threads by [@quanyeyang](https://github.com/quanyeyang) ([#3611](valkey-io/valkey#3611)) - RESET now clears the `CLIENT IMPORT-SOURCE` flag, so reused pooled connections return to normal expiration semantics by [@tjade273](https://github.com/tjade273) ([#3973](valkey-io/valkey#3973)) - Truncate a partially written MULTI block from the AOF on short read, preventing loss of newer writes after a later restart by [@chzhoo](https://github.com/chzhoo) ([#4342](valkey-io/valkey#4342)) - Fix an ACL bypass where duplicate STORE/STOREDIST options let GEORADIUS write or delete keys outside the user's permitted patterns by [@tjade273](https://github.com/tjade273) ([#3971](valkey-io/valkey#3971)) - Fix command log redaction leaking between commands in a MULTI transaction and missing for commands executed from scripts by [@madolson](https://github.com/madolson) ([#4323](valkey-io/valkey#4323)) - Fix a use-after-free crash when a module's cluster message type is received after the module is unloaded by [@enjoy-binbin](https://github.com/enjoy-binbin) ([#4360](valkey-io/valkey#4360)) - Fix out-of-bounds access for cluster module message type 255, which is now a valid, dispatchable message type by [@enjoy-binbin](https://github.com/enjoy-binbin) ([#4410](valkey-io/valkey#4410)) - AOF loading no longer performs ACL checks on replayed commands, preventing silent data loss when the default user is disabled by [@lukepalmer](https://github.com/lukepalmer) ([#3984](valkey-io/valkey#3984)) - Fix a client memory accounting leak on replicas that inflated the `mem_clients_normal` INFO field after primary disconnections by [@enjoy-binbin](https://github.com/enjoy-binbin) ([#4395](valkey-io/valkey#4395)) - Fix a permanent client deadlock when a blocking command like BLPOP is followed by a partially delivered pipelined command by [@foobar](https://github.com/foobar) ([#4531](valkey-io/valkey#4531)) - HGETEX now requires write permission on the key, closing an ACL gap that let read-only users change field TTLs or delete fields by [@ranshid](https://github.com/ranshid) ([#4576](valkey-io/valkey#4576)) - Compare the whole TLS certificate CN during authentication, so an embedded NUL can no longer impersonate another ACL user by [@madolson](https://github.com/madolson) ([#4577](valkey-io/valkey#4577)) - Fix atomic slot migration failures with I/O threads by not offloading the export job's writes while snapshotting by [@satheeshaGowda](https://github.com/satheeshaGowda) ([#4104](valkey-io/valkey#4104)) - Reject invalid slot import ranges when loading an RDB, so corrupted files can no longer create bad migration jobs by [@enjoy-binbin](https://github.com/enjoy-binbin) ([#4229](valkey-io/valkey#4229)) - Reject RDB slot import records with an invalid job name length, preventing an out-of-bounds read at startup by [@quanyeyang](https://github.com/quanyeyang) ([#4210](valkey-io/valkey#4210)) - MOVE and COPY now check ACL access to the current database, so users can no longer exfiltrate keys from an unauthorized DB by [@cjx-zar](https://github.com/cjx-zar) ([#4155](valkey-io/valkey#4155)) - Fix a crash on COPY with a trailing DB option during slot migration, and block cross-DB COPY regardless of option order by [@madolson](https://github.com/madolson) ([#4301](valkey-io/valkey#4301)) - Fix a server panic when pipelined commands with invalid arity reach the key prefetcher with I/O threads enabled by [@madolson](https://github.com/madolson) ([#4302](valkey-io/valkey#4302)) - HPERSIST, HTTL, HPTTL, HEXPIRETIME, and HPEXPIRETIME now return a syntax error when the FIELDS keyword is missing by [@cjx-zar](https://github.com/cjx-zar) ([#4300](valkey-io/valkey#4300)) - Fix a race between TLS I/O-thread writes and reads that could leave slot migration export jobs stuck until timeout by [@jjuleslasarte](https://github.com/jjuleslasarte) ([#4320](valkey-io/valkey#4320)) - Fix a signed overflow that let very large hash field expiration times (e.g. via HPEXPIREAT) crash the server by [@ranshid](https://github.com/ranshid) ([#4312](valkey-io/valkey#4312)) - Fix a frozen monotonic clock on hosts with unsynchronized TSC that stopped background tasks and key expiration by [@quanyeyang](https://github.com/quanyeyang) ([#4346](valkey-io/valkey#4346)) - Fix a stack overflow crash when retrying a failed TLS write with a large reply by [@murphyjacob4](https://github.com/murphyjacob4) ([#4307](valkey-io/valkey#4307)) - Fix the `--check-system` clocksource check to skip hosts using a hardware clock and suggest only actually available clocksources by [@quanyeyang](https://github.com/quanyeyang) ([#4272](valkey-io/valkey#4272)) - Fix an assertion failure with I/O threads when a blocked client's pending command was processed again before unblocking by [@quanyeyang](https://github.com/quanyeyang) ([#4376](valkey-io/valkey#4376)) - Sentinel no longer loads the built-in Lua scripting engine, removing a spurious warning at startup by [@enjoy-binbin](https://github.com/enjoy-binbin) ([#4327](valkey-io/valkey#4327)) - Validate channel, message, and module payload lengths in cluster bus packets, preventing forged packets from crashing nodes by [@tjade273](https://github.com/tjade273) ([#3972](valkey-io/valkey#3972)) - Harden stream validation on RDB load and RESTORE so crafted payloads can no longer crash the server on later commands by [@madolson](https://github.com/madolson) ([#3922](valkey-io/valkey#3922)) - Reject stream payloads with mismatched live/deleted record counts, preventing XDEL from destroying unaccounted entries by [@roshkhatri](https://github.com/roshkhatri) ([#4381](valkey-io/valkey#4381)) - Skip unnecessary post-read processing with I/O threads on socket and TLS connections, restoring small-payload throughput by [@quanyeyang](https://github.com/quanyeyang) ([#4401](valkey-io/valkey#4401)) - Fix a use-after-free crash when serving clients blocked on the same key if one client is freed during processing by [@quanyeyang](https://github.com/quanyeyang) ([#4212](valkey-io/valkey#4212)) - Avoid an unneeded client lookup per write completion with I/O threads on socket and TLS connections, improving pipelined throughput by [@dgershko](https://github.com/dgershko) ([#4440](valkey-io/valkey#4440)) - Fix CLUSTER SLOT-STATS ORDERBY returning wrong ordering when slot counters differ by more than 2^31 by [@jzy1688](https://github.com/jzy1688) ([#4459](valkey-io/valkey#4459)) - Fix slot migration failures with I/O threads and TLS by keeping the export job's ACK reads on the main thread while snapshotting by [@satheeshaGowda](https://github.com/satheeshaGowda) ([#4559](valkey-io/valkey#4559)) **Full Changelog**: <valkey-io/valkey@9.1.1...9.1.2> </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate CLI](https://github.com/renovatebot/renovate).  Reviewed-on: https://forge.superkooka.com/Superkooka/infra-gitops/pulls/31

Clear the redaction bitmap between commands in a transaction

6dc42a9

Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>

madolson requested review from rjd15372 and zuiderkwast August 3, 2026 21:41

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

github-actions Bot assigned madolson Aug 3, 2026

madolson added this to Valkey 9.1 Aug 3, 2026

madolson moved this to In Progress in Valkey 9.1 Aug 3, 2026

madolson commented Aug 3, 2026

View reviewed changes

Comment thread tests/unit/commandlog.tcl Resolved

Apply suggestion from @madolson

a945648

Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>

rainsupreme approved these changes Aug 3, 2026

View reviewed changes

Comment thread src/server.c Resolved

Comment thread tests/unit/commandlog.tcl Resolved

enjoy-binbin approved these changes Aug 4, 2026

View reviewed changes

zuiderkwast approved these changes Aug 7, 2026

View reviewed changes

madolson merged commit 875696f into valkey-io:unstable Aug 12, 2026
64 checks passed

github-project-automation Bot moved this from In Progress to To be backported in Valkey 9.1 Aug 12, 2026

madolson mentioned this pull request Aug 13, 2026

Tests: EXEC is now entry 0 of the MULTI redaction commandlog test #4404

Merged

valkeyrie-ops Bot mentioned this pull request Aug 15, 2026

[backport] Backport sweep for 9.1 #4225

Merged

valkeyrie-ops Bot moved this from To be backported to Done in Valkey 9.1 Aug 30, 2026

github-actions Bot mentioned this pull request Sep 21, 2026

Update Helm release valkey to v2.3.4 claytono/infra#2989

Merged

1 task

Navigation Menu

Repository navigation

Clear the redaction bitmap between commands in a transaction - #4323

Clear the redaction bitmap between commands in a transaction#4323
madolson merged 3 commits into
valkey-io:unstablefrom
madolson:fix-redact-bitmap-multi

madolson commented Aug 3, 2026

Uh oh!

coderabbitai Bot commented Aug 3, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

codecov Bot commented Aug 3, 2026 •

edited

Loading

Uh oh!

Uh oh!

rainsupreme left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

madolson commented Aug 3, 2026

Uh oh!

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

Uh oh!

Walkthrough

Changes

Uh oh!

codecov Bot commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

rainsupreme left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coderabbitai Bot commented Aug 3, 2026 •

edited

Loading

codecov Bot commented Aug 3, 2026 •

edited

Loading