Repository navigation
fix(tools): add logging filter to redact secrets from libtmux log output - #4871
Conversation
The libtmux library logs full tmux command strings (including send-keys arguments) to stderr when exceptions occur. This can leak API keys and other secrets that agents send through the terminal. Add a SecretRedactFilter to the libtmux logger that applies redact_api_key_literals() from the SDK's redaction utilities to all log messages before they are emitted. Refs: OpenHands/evaluation#446 Co-authored-by: openhands <openhands@all-hands.dev>
|
👋 This PR needs a couple of things fixed before OpenHands can review it:
Push an update once this is addressed and this check re-runs automatically. This is an automated check - no AI was used to generate this comment. |
Python API breakage checks — ✅ PASSEDResult: ✅ PASSED |
REST API breakage checks (OpenAPI) — ✅ PASSEDResult: ✅ PASSED |
Coverage Report •
|
||||||||||||||||||||
enyst
left a comment
enyst
left a comment
There was a problem hiding this comment.
Feedback from Astra:
Request changes: child loggers bypass the filter.
(I did not verify it myself FWIW)
|
@OpenHands verify that claim Request changes: child loggers bypass the filter. (I did not verify it myself FWIW) |
|
I'm on it! simonrosenberg can track my progress at all-hands.dev |
Co-authored-by: openhands <openhands@all-hands.dev>
|
Verified: the Astra claim is correct. Python logging calls handlers on ancestor loggers for propagated child records, but it does not run filters attached to those ancestor loggers. Fixed in Validation:
I also restored the PR template, added test evidence, and linked the new focused issue #4951. The only remaining PR-description failure is intentionally human-only: @simonrosenberg, please add at least 20 visible characters between There is also an unrelated deprecation-deadline failure for This comment was created by an AI agent (OpenHands) on behalf of the user. |
|
Verified: Astra’s claim is correct. Python logging does not run filters attached to ancestor loggers for propagated child records, and the leaking call comes from Completed:
Validation:
One required action remains: a human must add at least 20 visible characters between There is also an unrelated PR: #4871 |
…put (OpenHands#4871) Co-authored-by: openhands <openhands@all-hands.dev> Co-authored-by: simonrosenberg <157206163+simonrosenberg@users.noreply.github.com>
…put (OpenHands#4871) Co-authored-by: openhands <openhands@all-hands.dev> Co-authored-by: simonrosenberg <157206163+simonrosenberg@users.noreply.github.com>
4 participants
HUMAN:
AGENT:
Why
libtmux.commonlogs the full tmux command string when command execution raises. Commands passed throughsend-keyscan contain API keys or tokens, which can then appear in agent-server stderr logs.Python logging does not run filters attached to an ancestor logger for records emitted by child loggers. Therefore, attaching the filter only to
libtmuxdoes not redact records emitted bylibtmux.common, the source of the leaking exception log.Summary
libtmuxlogger and all loadedlibtmux.*child loggers.TmuxTerminalconstruction, before terminal operations can emit logs.libtmux.commonrecord reaches its handler with the secret replaced by<redacted>.Issue Number
Fixes #4951
Related incident tracking: OpenHands/evaluation#446 and OpenHands/evaluation#383.
How to Test
Ran:
Results:
345 passed, 15 skipped.Video/Screenshots
Not applicable; this changes backend log filtering and is covered by automated tests.
Design Doc
Not applicable for this focused logging fix.
Type
Notes
The review claim was verified directly: a child logger record propagated to an ancestor handler without invoking the ancestor logger's filter. The focused regression test exercises the actual leaking child logger name,
libtmux.common.This PR was created by an AI agent (OpenHands) on behalf of the security scan workflow.
🐳 Agent Server images for this PR — GHCR package, pull/run commands, and all pushed tags (click to expand)
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdknikolaik/python-nodejs:python3.13-nodejs22-slimnikolaik/python-nodejs:python3.13-nodejs22-slimgolang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:bb57854-pythonRun
All tags pushed for this build
About Multi-Architecture Support
bb57854-python) is a multi-arch manifest supporting both amd64 and arm64bb57854-python-amd64) are also available if needed