From bc914a5ec68c5c36aaf781fc18c7b3a566f432f9 Mon Sep 17 00:00:00 2001 From: barry3406 Date: Sat, 11 Apr 2026 10:08:51 -0700 Subject: [PATCH] accept password values that start with a dash (#1752) click 8.3+ marks any option declared with a flag_value as _flag_needs_value=True, which triggers a parser heuristic that refuses values beginning with a dash. That turned mycli --password=-rocks into 'Error: No such option: -r'. Wrap the command in a _PasswordFriendlyCommand subclass that disables the heuristic on the password option and injects the flag-value sentinel itself when the option is used bare, preserving the prompt-for-password behavior. --- changelog.md | 1 + mycli/main.py | 49 +++++++++++++++++++++++-- test/pytests/test_main.py | 76 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+), 2 deletions(-) diff --git a/changelog.md b/changelog.md index 74d16069..3656f552 100644 --- a/changelog.md +++ b/changelog.md @@ -22,6 +22,7 @@ Bug Fixes * Run empty `--execute` arguments instead of ignoring the flag. * Exit with error when the `--batch` argument is an empty string. * Avoid logging SSH passwords. +* Accept password values that start with a dash, e.g. `--password=-rocks` (#1752). Internal diff --git a/mycli/main.py b/mycli/main.py index ae6ca3c5..72dcfe97 100755 --- a/mycli/main.py +++ b/mycli/main.py @@ -87,7 +87,7 @@ sqlparse.engine.grouping.MAX_GROUPING_DEPTH = None # type: ignore[assignment] sqlparse.engine.grouping.MAX_GROUPING_TOKENS = None # type: ignore[assignment] -EMPTY_PASSWORD_FLAG_SENTINEL = -1 +EMPTY_PASSWORD_FLAG_SENTINEL = '\x00__mycli_empty_password_flag__\x00' class IntOrStringClickParamType(click.ParamType): @@ -1454,7 +1454,52 @@ class CliArgs: ) -@click.command() +class _PasswordFriendlyCommand(click.Command): + """Command subclass that lets --password accept a value starting with '-'. + + Because the password option combines ``is_flag=False`` with ``flag_value`` + (so that a bare ``-p`` / ``--password`` means "prompt for the password"), + click 8.3+ marks the option with ``_flag_needs_value=True``. That flag + triggers a parser heuristic which refuses values beginning with a dash, + turning ``mycli --password=-rocks`` into ``Error: No such option: -r`` + (see dbcli/mycli#1752). + + This subclass disables that heuristic on the password option and instead + injects the flag-value sentinel itself when the option is used bare, + preserving the prompt-for-password behavior. + """ + + _PASSWORD_OPTS = frozenset({'-p', '--pass', '--password'}) + + def parse_args(self, ctx: click.Context, args: list[str]) -> list[str]: + args = self._inject_bare_password_sentinel(args) + for param in self.params: + if isinstance(param, click.Option) and param.name == 'password': + param._flag_needs_value = False + break + return super().parse_args(ctx, args) + + @classmethod + def _inject_bare_password_sentinel(cls, args: list[str]) -> list[str]: + result: list[str] = [] + i = 0 + n = len(args) + while i < n: + token = args[i] + if token in cls._PASSWORD_OPTS: + nxt = args[i + 1] if i + 1 < n else None + bare = nxt is None or (isinstance(nxt, str) and nxt.startswith('-') and len(nxt) > 1) + if bare: + result.append(token) + result.append(EMPTY_PASSWORD_FLAG_SENTINEL) + i += 1 + continue + result.append(token) + i += 1 + return result + + +@click.command(cls=_PasswordFriendlyCommand) @clickdc.adddc('cli_args', CliArgs) @click.version_option(mycli_package.__version__, '--version', '-V', help="Output mycli's version.") def click_entrypoint( diff --git a/test/pytests/test_main.py b/test/pytests/test_main.py index 84019590..c6949415 100644 --- a/test/pytests/test_main.py +++ b/test/pytests/test_main.py @@ -1335,6 +1335,82 @@ def run_query(self, query, new_line=True): assert MockMyCli.connect_args['passwd'] == 'cleartext_password' +@pytest.mark.parametrize( + ('password_args', 'expected'), + [ + # Regression tests for https://github.com/dbcli/mycli/issues/1752: + # a password value starting with '-' used to be reinterpreted as short + # options ("Error: No such option: -r") because click marks the option + # with `_flag_needs_value=True` whenever `flag_value` is set. + (['--password=-rocks'], '-rocks'), + (['--password=-starts-with-dash'], '-starts-with-dash'), + (['--pass=-rocks'], '-rocks'), + (['-p-rocks'], '-rocks'), + # Existing behavior that must not regress. + (['--password=foo'], 'foo'), + (['--password', 'cleartext_password'], 'cleartext_password'), + (['-procks'], 'rocks'), + ], +) +def test_password_option_accepts_dash_prefixed_value(monkeypatch, password_args, expected): + 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.my_cnf = {'client': {}, 'mysqld': {}} + 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_args, + ], + ) + assert result.exit_code == 0, result.output + ' ' + str(result.exception) + assert 'No such option' not in result.output + assert MockMyCli.connect_args['passwd'] == expected + + def test_password_option_overrides_password_file_and_mysql_pwd(monkeypatch): class Formatter: format_name = None