Accept password values that start with a dash (#1752)

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

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(

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