-
Notifications
You must be signed in to change notification settings - Fork 1.5k
{Image-copy} Mask the value of the --sas-token parameter when displaying it in the terminal
#9516
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -19,7 +19,7 @@ | |||||||||||||
| def run_cli_command(cmd, return_as_json=False): | ||||||||||||||
| try: | ||||||||||||||
| cmd_output = check_output(cmd, stderr=STDOUT, universal_newlines=True) | ||||||||||||||
| logger.debug('command: %s ended with output: %s', cmd, cmd_output) | ||||||||||||||
| logger.debug('command: %s ended with output: %s', _mask_output_token(cmd), cmd_output) | ||||||||||||||
|
|
||||||||||||||
| if return_as_json: | ||||||||||||||
| if cmd_output: | ||||||||||||||
|
|
@@ -40,15 +40,25 @@ def run_cli_command(cmd, return_as_json=False): | |||||||||||||
| raise CLIError("Command returned an unexpected empty string.") | ||||||||||||||
| return cmd_output | ||||||||||||||
| except CalledProcessError as ex: | ||||||||||||||
| logger.error('command failed: %s', cmd) | ||||||||||||||
| logger.error('command failed: %s', _mask_output_token(cmd)) | ||||||||||||||
| logger.error('output: %s', ex.output) | ||||||||||||||
| raise ex | ||||||||||||||
| except Exception as ex: | ||||||||||||||
| logger.error('command ended with an error: %s', cmd) | ||||||||||||||
| logger.error('command ended with an error: %s', _mask_output_token(cmd)) | ||||||||||||||
| logger.error('args: %s', ex.args) | ||||||||||||||
| raise ex | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| def _mask_output_token(cmd): | ||||||||||||||
| output = cmd[:] | ||||||||||||||
| token_param_name = "--sas-token" | ||||||||||||||
| if token_param_name in output: | ||||||||||||||
| idx = output.index(token_param_name) | ||||||||||||||
| output[idx + 1] = "******" | ||||||||||||||
|
Comment on lines
+55
to
+57
|
||||||||||||||
| if token_param_name in output: | |
| idx = output.index(token_param_name) | |
| output[idx + 1] = "******" | |
| for i, arg in enumerate(output): | |
| if arg == token_param_name and i + 1 < len(output): | |
| output[i + 1] = "******" |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new _mask_output_token function lacks unit test coverage. Given that this is a security-related function responsible for masking sensitive SAS tokens in logs, it should have unit tests to verify correct behavior including edge cases like when --sas-token is at the end of the list, when it's missing, or when it appears multiple times.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function has a potential IndexError bug. If
--sas-tokenappears as the last element in the cmd list, accessingoutput[idx + 1]will raise an IndexError. Add a check to ensureidx + 1is within bounds before attempting to mask the token value.