[APMS-18584] Fix authentication obfuscation on HELLO and MIGRATE verbs by vpellan · Pull Request #5350 · DataDog/dd-trace-rb

vpellan · 2026-02-11T17:10:37Z

What does this PR do?

This PR fixes the leaking of sensitive data in HELLO and MIGRATE verbs on redis instrumentation

Motivation:

Customer had sensitive data that was leaked by HELLO. It was introduced in Redis 6, however MIGRATE can also uses AUTH since 4.0.7.

Change log entry

Yes. Add sensitive-data obfuscation for Redis HELLO and MIGRATE verbs

Additional Notes:

I tried to implement something future-proof, unfortunately, to keep compatibility with Redis < 6, we must support the requirepass mechanism. Which means that sometimes AUTH will only have 1 arg, and sometimes 2. And in the case of MIGRATE, it can be followed by KEYS, which we do not want to remove. So the solution implemented is not future-proof, and we should keep track of new redis versions to ensure we do not miss new verbs that uses AUTH.

How to test the change?

CI

github-actions · 2026-02-11T17:11:18Z

Typing analysis

Ignored files

This PR clears 1 ignored file. It increases the percentage of typed files from 42.43% to 42.54% (+0.11%).

Ignored files (+0-1)

✅ Cleared:

lib/datadog/tracing/contrib/redis/quantize.rb

Note: Ignored files are excluded from the next sections.

Untyped methods

This PR introduces 2 untyped methods and 2 partially typed methods. It decreases the percentage of typed methods from 60.11% to 60.0% (-0.11%).

Untyped methods (+2-0)

❌ Introduced:

sig/datadog/tracing/contrib/redis/quantize.rbs:26
└── def self?.format_arg: (untyped arg) -> untyped
sig/datadog/tracing/contrib/redis/quantize.rbs:34
└── def self?.resolve_command_args: (untyped command_args) -> untyped

Partially typed methods (+2-0)

❌ Introduced:

sig/datadog/tracing/contrib/redis/quantize.rbs:28
└── def self?.format_command_args: (untyped command_args) -> ("AUTH ?" | untyped)
sig/datadog/tracing/contrib/redis/quantize.rbs:30
└── def self?.get_verb: (untyped command_args) -> (nil | untyped | ::String)

Untyped other declarations

This PR introduces 1 untyped other declaration. It increases the percentage of typed other declarations from 77.03% to 77.11% (+0.08%).

Untyped other declarations (+1-0)

❌ Introduced:

sig/datadog/tracing/contrib/redis/quantize.rbs:24
└── MULTI_VERB_COMMANDS: untyped

If you believe a method or an attribute is rightfully untyped or partially typed, you can add # untyped:accept on the line before the definition to remove it from the stats.

datadog-official · 2026-02-11T17:27:53Z

✅ Tests

🎉 All green!

❄️ No new flaky tests detected
🧪 All tests passed

🎯 Code Coverage
• Patch Coverage: 100.00%
• Overall Coverage: 95.15% (+0.00%)

View detailed report

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: e8458a1 | Docs | Datadog PR Page | Was this helpful? Give us feedback!}

pr-commenter · 2026-02-11T17:30:23Z

Benchmarks

Benchmark execution time: 2026-02-11 18:24:20

Comparing candidate commit e8458a1 in PR branch vpellan/APMS-18584-redis-hello-credentials with baseline commit ab36dce in branch master.

Found 3 performance improvements and 0 performance regressions! Performance is the same for 41 metrics, 2 unstable metrics.

scenario:tracing - 100 span trace - no writer

🟩 throughput [+17.814op/s; +18.560op/s] or [+5.946%; +6.195%]

scenario:tracing - Propagation - Datadog

🟩 throughput [+3165.391op/s; +3239.704op/s] or [+11.005%; +11.263%]

scenario:tracing - Tracing.log_correlation

🟩 throughput [+9110.849op/s; +9419.657op/s] or [+9.106%; +9.414%]

y9v · 2026-02-11T20:11:14Z

-            AUTH_COMMANDS.include?(verb)
+            case verb
+            when *AUTH_COMMANDS
+              return %w[AUTH ?]


is this intended to change "AUTH ?" string to ["AUTH", "?"] array?

It does return an array ["AUTH", "?"] but then format_command_args will turn it into a string

y9v · 2026-02-11T20:14:52Z

          def format_command_args(command_args)
            command_args = resolve_command_args(command_args)
-            return 'AUTH ?' if auth_command?(command_args)
+            command_args = obfuscate_auth_args(command_args)


we were returning a string before for an auth command, now for the auth command and for hello and migrate commands we are duping and mutating all command args - could we avoid duping the whole array?

I think there is no way around creating a nearly complete copy of the array one way or another but maybe constructing the new array one element at a time is faster than duping and then removing an element? Don't know how important this optimization is though.

I tried to edit the array in-place but it was causing issues as arrays coming from redis are frozen. I will run some benchmarks to see what's the faster solution

p-datadog · 2026-02-12T03:15:31Z

    module Contrib
      module Redis
        module Quantize
+          type command = ::Array[::String | ::Symbol | command]


This says that command can be an infinitely nested array? Is that right?

p-datadog · 2026-02-12T03:21:32Z

            verb = command_args.first.to_s
-            AUTH_COMMANDS.include?(verb)
+            case verb
+            when *AUTH_COMMANDS


This handles commands in all upper case and all lower case, but according to https://redis.io/docs/latest/commands/command/ the commands are case insensitive. Given that this code deals with redaction I think it should e.g. lowercase the commands prior to matching rather than only handling all upper or all lower case (which, I assume, is the current behavior of supported ruby redis clients?).

I do agree with Oleg, but from pure performance perspective, commands need to go through normalization process first, that would allow us to drop upper/lower case difference.

In addition this will allow us to stop deconstructing arrays of 2 elements every time (at this point we can just list values with comma).

p-datadog · 2026-02-12T03:28:39Z

+            end
+
+            context 'with keys' do
+              let(:args) { %w[MIGRATE 127.0.0.1 6379 "" 0 5000 AUTH dog KEYS foo] }


I think there should also be tests where "auth" is used as the value, e.g.:

MIGRATE 127.0.0.1 6379 "" 0 5000 AUTH AUTH KEYS AUTH MIGRATE 127.0.0.1 6379 "AUTH" 0 5000 AUTH dog KEYS foo

Can the IP address be a hostname? If so that should also ideally be tested with "AUTH" as value.

Strech · 2026-02-12T09:49:27Z

@codex Analyze obfuscation method for performance and memory footprint. Look for bugs and potential issues.

chatgpt-codex-connector · 2026-02-12T09:54:32Z

Summary

Updated Redis obfuscation logic to normalize command matching case-insensitively (upcase) and use fixed keyword constants (AUTH, HELLO, MIGRATE, KEYS) so redaction behavior is consistent for mixed-case inputs. This directly addresses the review concern that Redis commands are case-insensitive. lib/datadog/tracing/contrib/redis/quantize.rbL16-L20 lib/datadog/tracing/contrib/redis/quantize.rbL67-L73
Reworked HELLO/MIGRATE redaction to build a new result array in a single pass instead of dup + delete_at, reducing mutation overhead and avoiding unnecessary whole-array mutation patterns while keeping the same output shape. lib/datadog/tracing/contrib/redis/quantize.rbL76-L87 lib/datadog/tracing/contrib/redis/quantize.rbL100-L117
Fixed a potential bug in MIGRATE parsing by restricting AUTH detection to option positions (after the 6 required MIGRATE args, before KEYS), so payload values equal to "AUTH" are not misinterpreted as the auth option. lib/datadog/tracing/contrib/redis/quantize.rbL89-L99
Added focused tests for the exact risky edge cases called out in review: mixed-case commands/keywords, payload values containing AUTH, and hostname usage in MIGRATE variants. spec/datadog/tracing/contrib/redis/quantize_spec.rbL138-L142 spec/datadog/tracing/contrib/redis/quantize_spec.rbL170-L174 spec/datadog/tracing/contrib/redis/quantize_spec.rbL190-L194 spec/datadog/tracing/contrib/redis/quantize_spec.rbL210-L220
Tightened the RBS definitions to remove the recursive “infinitely nested” command type, type constants more precisely, and narrow method signatures to concrete return types. sig/datadog/tracing/contrib/redis/quantize.rbsL6-L37

Committed: 857abf97 on branch work.

PR created: Follow-up PR title: “Follow-up: harden Redis AUTH obfuscation matching and edge cases”.

Testing

✅ bundle exec rspec spec/datadog/tracing/contrib/redis/quantize_spec.rb
⚠️ curl -L --max-time 15 https://redis.io/docs/latest/commands/command/ | rg -i "case insensitive|case-sensitive|insensitive" -n (blocked by environment network policy: CONNECT tunnel failed, response 403)

View task →

p-datadog · 2026-02-12T19:01:01Z

So many links in the codex comment, but none to the PR it made?

p-datadog · 2026-02-12T19:02:44Z

I think also this PR should use "redaction"? "Obfuscation" means something quite a bit different to me (for example, it should transform different values to different values, replacing all input with a fixed value - the question mark in this case - is not what obfuscation does)

p-datadog · 2026-02-12T19:03:24Z

I am not seeing the PR codex made at all.

Strech · 2026-02-13T09:17:09Z

@p-datadog There is a button you have to press inside the Codex View Task

vpellan · 2026-03-16T11:58:29Z

Closing as the fix has been implemented on the agent (https://github.com/DataDog/datadog-agent/releases/tag/7.75.4)

vpellan added 2 commits February 11, 2026 17:43

Fix authentication obfuscation on HELLO and MIGRATE verbs

05ab4e3

Activate steep for redis/quantize.rb

3ca379f

vpellan requested review from a team as code owners February 11, 2026 17:10

vpellan requested review from mabdinur and marcotc February 11, 2026 17:10

github-actions Bot added integrations Involves tracing integrations tracing labels Feb 11, 2026

Fix obfuscation on frozen array

e8458a1

y9v reviewed Feb 11, 2026

View reviewed changes

p-datadog reviewed Feb 12, 2026

View reviewed changes

vpellan closed this Mar 16, 2026

Conversation

vpellan commented Feb 11, 2026

Uh oh!

github-actions Bot commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Typing analysis

Ignored files

Untyped methods

Untyped other declarations

Uh oh!

datadog-official Bot commented Feb 11, 2026 • edited by datadog-datadog-prod-us1 Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pr-commenter Bot commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks

scenario:tracing - 100 span trace - no writer

scenario:tracing - Propagation - Datadog

scenario:tracing - Tracing.log_correlation

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

p-datadog Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Strech commented Feb 12, 2026

Uh oh!

chatgpt-codex-connector Bot commented Feb 12, 2026

Summary

Testing

Uh oh!

p-datadog commented Feb 12, 2026

Uh oh!

p-datadog commented Feb 12, 2026

Uh oh!

p-datadog commented Feb 12, 2026

Uh oh!

Strech commented Feb 13, 2026

Uh oh!

vpellan commented Mar 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions Bot commented Feb 11, 2026 •

edited

Loading

datadog-official Bot commented Feb 11, 2026 •

edited by datadog-datadog-prod-us1 Bot

Loading

pr-commenter Bot commented Feb 11, 2026 •

edited

Loading

p-datadog Feb 12, 2026 •

edited

Loading