From 620f1268c05cdfeaa7d4b62498ac2d67e287c17b Mon Sep 17 00:00:00 2001 From: Roland Walker Date: Sat, 11 Apr 2026 16:14:20 -0400 Subject: [PATCH] allow CLI passwords starting with dash characters Allow the argument to --password (and aliases) to start with a dash character, by stopping to pass the "flag_value" parameter to click. Now --password (and aliases) always take a value at the CLI, unless given in the final position, in which case the flag works as a request for an interactive prompt. This retains the mechanism of EMPTY_PASSWORD_FLAG_SENTINEL, just does not use "flag_value" to set the sentinel value. There still remains the special-casing of DSNs after --password flags, which, while not related to the dash-character bug, still defines some special strings which can't be used as CLI password arguments. --- changelog.md | 2 ++ mycli/main.py | 11 ++++--- mycli/packages/cli_utils.py | 15 +++++++-- test/pytests/test_cli_utils.py | 8 +++++ test/pytests/test_main.py | 59 +--------------------------------- 5 files changed, 30 insertions(+), 65 deletions(-) diff --git a/changelog.md b/changelog.md index ddec8fcd..3f565204 100644 --- a/changelog.md +++ b/changelog.md @@ -13,6 +13,7 @@ Breaking Changes * Make `--batch` and `--execute` non-interactive by default. * Migrate `default_character_set` out of the `[main]` section of `~/.myclirc`. * Migrate `ssl_mode` out of the `[main]` section of `~/.myclirc`. +* Limit `--password` without an argument to the final position. Features @@ -23,6 +24,7 @@ Features Bug Fixes --------- * Ensure that `--batch` and `--logfile` files are distinct. +* Allow passwords defined at the CLI to start with dash. Documentation diff --git a/mycli/main.py b/mycli/main.py index 28a7f403..5280c364 100755 --- a/mycli/main.py +++ b/mycli/main.py @@ -4,6 +4,7 @@ from io import TextIOWrapper import os import sys +from textwrap import dedent from typing import Callable import click @@ -78,9 +79,11 @@ class CliArgs: '--password', 'password', type=INT_OR_STRING_CLICK_TYPE, - is_flag=False, - flag_value=EMPTY_PASSWORD_FLAG_SENTINEL, - help='Prompt for (or pass in cleartext) the password to connect to the database.', + help=dedent( + """Password to connect to the database. + Use with a value to set the password at the CLI, or alone in the last position to request a prompt. + """ + ), ) password_file: str | None = clickdc.option( type=click.Path(), @@ -376,7 +379,7 @@ def click_entrypoint( def main() -> int | None: try: result = click_entrypoint.main( - filtered_sys_argv(), + filtered_sys_argv(), # type: ignore[arg-type] standalone_mode=False, # disable builtin exception handling prog_name='mycli', ) diff --git a/mycli/packages/cli_utils.py b/mycli/packages/cli_utils.py index d78b9f37..d90575db 100644 --- a/mycli/packages/cli_utils.py +++ b/mycli/packages/cli_utils.py @@ -1,13 +1,22 @@ from __future__ import annotations import sys +from typing import Sequence +from mycli.constants import EMPTY_PASSWORD_FLAG_SENTINEL + + +def filtered_sys_argv() -> Sequence[str | int]: + args: Sequence[str | int] = sys.argv[1:] + password_flag_forms = ['-p', '--pass', '--password'] -def filtered_sys_argv() -> list[str]: - args = sys.argv[1:] if args == ['-h']: args = ['--help'] - return args + + if args and args[-1] in password_flag_forms: + args = list(args) + [EMPTY_PASSWORD_FLAG_SENTINEL] + + return list(args) def is_valid_connection_scheme(text: str) -> tuple[bool, str | None]: diff --git a/test/pytests/test_cli_utils.py b/test/pytests/test_cli_utils.py index f0709489..9a921214 100644 --- a/test/pytests/test_cli_utils.py +++ b/test/pytests/test_cli_utils.py @@ -2,6 +2,7 @@ import pytest +from mycli.constants import EMPTY_PASSWORD_FLAG_SENTINEL from mycli.packages import cli_utils from mycli.packages.cli_utils import ( filtered_sys_argv, @@ -22,6 +23,13 @@ def test_filtered_sys_argv(monkeypatch, argv, expected): assert filtered_sys_argv() == expected +@pytest.mark.parametrize('password_flag', ['-p', '--pass', '--password']) +def test_filtered_sys_argv_appends_empty_password_sentinel(monkeypatch, password_flag): + monkeypatch.setattr(cli_utils.sys, 'argv', ['mycli', 'database', password_flag]) + + assert filtered_sys_argv() == ['database', password_flag, EMPTY_PASSWORD_FLAG_SENTINEL] + + @pytest.mark.parametrize( ('text', 'is_valid', 'invalid_scheme'), [ diff --git a/test/pytests/test_main.py b/test/pytests/test_main.py index a6a22562..34138293 100644 --- a/test/pytests/test_main.py +++ b/test/pytests/test_main.py @@ -36,11 +36,11 @@ DEFAULT_HOST, DEFAULT_PORT, DEFAULT_USER, + EMPTY_PASSWORD_FLAG_SENTINEL, ER_MUST_CHANGE_PASSWORD_LOGIN, TEST_DATABASE, ) from mycli.main import ( - EMPTY_PASSWORD_FLAG_SENTINEL, INT_OR_STRING_CLICK_TYPE, CliArgs, MyCli, @@ -1199,63 +1199,6 @@ def run_query(self, query, new_line=True): ) -def test_password_flag_uses_sentinel(monkeypatch): - class Formatter: - format_name = None - - class Logger: - def debug(self, *args, **args_dict): - pass - - def warning(self, *args, **args_dict): - pass - - class MockMyCli: - config = { - 'main': {}, - 'alias_dsn': {}, - 'connection': { - 'default_keepalive_ticks': 0, - }, - } - - def __init__(self, **_args): - self.logger = Logger() - self.destructive_warning = False - self.main_formatter = Formatter() - self.redirect_formatter = Formatter() - self.ssl_mode = 'auto' - self.default_keepalive_ticks = 0 - - def connect(self, **args): - MockMyCli.connect_args = args - - def run_query(self, query, new_line=True): - pass - - import mycli.main - - monkeypatch.setattr(mycli.main, 'MyCli', MockMyCli) - runner = CliRunner() - - result = runner.invoke( - mycli.main.click_entrypoint, - args=[ - '--user', - 'user', - '--host', - DEFAULT_HOST, - '--port', - f'{DEFAULT_PORT}', - '--database', - 'database', - '--password', - ], - ) - assert result.exit_code == 0, result.output + ' ' + str(result.exception) - assert MockMyCli.connect_args['passwd'] == EMPTY_PASSWORD_FLAG_SENTINEL - - def test_password_option_uses_cleartext_value(monkeypatch): class Formatter: format_name = None