Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces command-line password redaction in process_handler.py to prevent credentials from being logged in runtime snapshots, along with corresponding unit tests. The review feedback highlights a security vulnerability where passwords containing spaces may not be fully redacted when using the proposed regular expression on a space-joined string. To address this, the reviewer recommends refactoring the redaction logic to process argument lists directly, which would also allow removing the regex and the re module import, and suggests adding test coverage for arguments containing spaces.
831f09a to
0902932
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces password redaction for command-line arguments in process snapshots to prevent sensitive credentials from being logged. It defines a regular expression to identify password flags and implements a redact_command_line function, which is applied within get_runtime_snapshot. Comprehensive unit tests have also been added to verify various redaction scenarios. Feedback suggests adding a guard check in redact_command_line to robustly handle None or empty inputs.
|
|
||
| # Matches a password flag whose value must be redacted before logging. The value | ||
| # is either passed as the next argument (e.g. `-password value`) or inline after | ||
| # `=` or `:` (e.g. `--password=value`, `/password:value`). |
There was a problem hiding this comment.
nit:
Can you also add an example where this regex will match a cli flag that has both a username and password?
When a hang is detected, terminate_hung_threads() logs a snapshot of all running processes with their full command lines. If any process was started with a password on its command line, that password ends up in the logs in plain text. Redact the value of password flags in get_runtime_snapshot().
0902932 to
97a8e29
Compare
When a hang is detected, terminate_hung_threads() logs a snapshot of all running processes with their full command lines. If any process was started with a password on its command line, that password ends up in the logs in plain text.
Redact the value of password flags in get_runtime_snapshot().