Repository navigation
fix: make dict-entry secret redaction case-insensitive - #4508
Conversation
|
👋 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. |
|
🚦 CI is currently failing on this PR's latest commit. Please fix the failing checks before OpenHands reviews it - this is re-checked automatically once you push a new commit. (A maintainer can also request This is an automated check - no AI was used to generate this comment. |
The two dictionary-entry patterns in redact_text_secrets only matched
uppercase key names, so text like {'api_key': 's3cr3t'} passed through
unredacted while the module's is_secret_key helper matches
case-insensitively. Add re.IGNORECASE to both substitutions so
lowercase and mixed-case keys containing key/secret/token/password are
redacted, matching the documented behavior.
Fixes OpenHands#4505
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ce1fc2a to
f798777
Compare
|
🚦 CI is currently failing on this PR's latest commit. Please fix the failing checks before OpenHands reviews it - this is re-checked automatically once you push a new commit. (A maintainer can also request This is an automated check - no AI was used to generate this comment. |
…ixes Port from OpenHands/software-agent-sdk#4508: their dict-entry secret redaction was uppercase-only and leaked mixed-case keys (UserPassword, sessionToken). Apply the same case-insensitive treatment to the Python mapping-repr pass: a casefolded credential suffix (apikey/token/secret/ password/passwd/credential) now qualifies a key, while metadata names (TOKEN_COUNT, password_policy, tokenizer) stay untouched. (cherry picked from commit bbcaad4)
…ixes Port from OpenHands/software-agent-sdk#4508: their dict-entry secret redaction was uppercase-only and leaked mixed-case keys (UserPassword, sessionToken). Apply the same case-insensitive treatment to the Python mapping-repr pass: a casefolded credential suffix (apikey/token/secret/ password/passwd/credential) now qualifies a key, while metadata names (TOKEN_COUNT, password_policy, tokenizer) stay untouched.
…ixes Port from OpenHands/software-agent-sdk#4508: their dict-entry secret redaction was uppercase-only and leaked mixed-case keys (UserPassword, sessionToken). Apply the same case-insensitive treatment to the Python mapping-repr pass: a casefolded credential suffix (apikey/token/secret/ password/passwd/credential) now qualifies a key, while metadata names (TOKEN_COUNT, password_policy, tokenizer) stay untouched.
HUMAN:
I reviewed the two-flag regex diff and the failing-then-passing pytest output for the new regression tests before submitting.
AGENT:
Why
The two dictionary-entry substitutions in
redact_text_secrets(openhands-sdk/openhands/sdk/utils/redact.py) only matched uppercase key names ([A-Z_]*(?:KEY|SECRET|TOKEN|PASSWORD)[A-Z_]*), so text like{'api_key': 's3cr3t'}or'UserPassword': 'p@ssw0rd'passed through unredacted. The same module'sis_secret_keymatches case-insensitively, and the function's docstring does not restrict the key categories to uppercase.redact_text_secretsruns on ACP stdout/stderr lines and error details before they reach logs, so lowercase secret keys in process output leaked.Summary
flags=re.IGNORECASEto both dict-entry substitutions inredact_text_secrets; quote handling, value boundaries, URL/header/API-key-literal behavior, and uppercase matching are untouchedTestRedactTextSecretsDictKeysregression tests: the issue's exact reproduction (verified failing before the fix, passing after), uppercase keys still redacted, non-sensitive entries unchangedIssue Number
Fixes #4505
How to Test
38 tests pass on this branch. Before the fix,
test_redacts_lowercase_and_mixed_case_dict_keysfails with the secrets left in the output, reproducing the issue exactly.End-to-end check through the ACP logging path (
acp_agent.pypasses raw output lines throughredact_text_secretsbefore logging). Running the same line through the public API onmainvs this branch:Also ran
tests/sdk/utils/test_command.py(aredact_text_secretscaller): 12 passed.ruff checkandruff format --checkclean on both changed files.Video/Screenshots
Not a GUI change; the before/after log output above shows the behavior change.
Design Doc
Not needed for a two-flag fix.
Type
Notes
The module docstring lists
OpenHands/runtime-apias carrying a partial copy of this file; that copy may want the same flag if it shares these patterns.🤖 Generated with Claude Code