Skip to content

fix(redis): avoid sensitive content in deserialization logs - #72

Merged
yuluo-yx merged 2 commits into
agentic-ai-java:mainfrom
logicwu0:fix/54-redis-safe-deserialization-logs
Oct 30, 2026
Merged

yuluo-yx merged 2 commits into
agentic-ai-java:mainfrom
logicwu0:fix/54-redis-safe-deserialization-logs

Conversation

@logicwu0

Copy link
Copy Markdown
Contributor

Describe what this PR does / why we need it

Redis chat-memory deserialization logs currently expose stored message text through ERROR log arguments, DEBUG JSON dumps, field-conversion exception messages, and unrecognized message-type values. Exception causes and stack traces can also contain source data even when the first-level log template is sanitized.

This PR removes those content-bearing diagnostics while retaining useful error categories and input length. It does not change damaged-record handling.

Does this pull request fix one issue?

Fixes #54

Describe how you did it

  • Log only input character count and exception class for message-level deserialization errors; do not attach the parser exception or its cause chain.
  • Remove complete-message DEBUG logging.
  • Keep field names and exception classes in metadata/tool-call/tool-response conversion warnings, without exception messages.
  • Do not echo untrusted message-type values in fallback warnings.
  • Add synthetic-marker tests that inspect formatted messages, log arguments, and throwable proxies at INFO and DEBUG, including valid bodies, metadata, tool arguments/results, malformed JSON/fields, and existing defensive fallback branches.
  • Add real Redis read tests for Jedis, Lettuce, and Redisson, checking that valid messages are still returned, malformed records retain the existing filtering behavior, stored records remain untouched by reads, and diagnostics contain no sensitive marker.
  • Document the diagnostic changes, correlation guidance, and handling of existing sensitive log archives in the module README.

Describe how to verify it

With an isolated standalone Redis on port 16380:

mvn -B -ntp -pl memory-repository/argi-model-chat-memory-repository-redis -am \
  -Dtest=RedisDeserializationLoggingTest,RedisDeserializationLoggingIntegrationTest \
  -Dredis.test.port=16380 -Dcheckstyle.skip=false test

The integration suite can alternatively start a Redis container when Docker is available. It deletes only its uniquely named test keys and never flushes a database.

Verified locally with JDK 21 and Redis 7.0.15:

  • Before the production fix: 21 of 22 offline cases failed on sensitive logging, and all 6 real Redis cases failed.
  • After the fix: all 28 new cases passed without skips (22 offline and 6 real-backend cases).
  • A fresh-target module run reported 85 tests, zero failures/errors, and 57 skips. The skipped cases are the pre-existing Docker-based suites, including TLS tests; Docker is unavailable locally.
  • Checkstyle reported zero violations and git diff --check passed.

Special notes for reviews

Five files change: two production classes, two test classes, and the existing README. This branch is based on main and does not include the separate key-prefix fix in #71.

No changes to serialization, public APIs, stored records, or damaged-message read policy are included. Actual malformed-field errors in the current Jackson version are still rejected; the existing IllegalArgumentException fallback branches are tested separately with synthetic failures.

Applications parsing the previous log templates should update their queries. There is no new message-body logging opt-in, raw conversation-ID label, or content hash. Existing request/trace context can be used for correlation. Standalone Redis verification does not certify TLS or Redis Cluster behavior.

Remove stored message bodies, untrusted type values, and exception details from deserialization diagnostics. Preserve existing read behavior and cover INFO/DEBUG logs with offline and real Redis regression tests.

Fixes agentic-ai-java#54

@yuluo-yx yuluo-yx 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.

lgtm 👍

@yuluo-yx
yuluo-yx merged commit 4b54224 into agentic-ai-java:main Oct 30, 2026
4 checks passed
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.

[BUG] Redis 聊天记忆反序列化日志应避免输出完整消息正文

2 participants