Merge pull request #1842 from dbcli/RW/allow-leading-hyphen-on-cli-passwords

Allow CLI passwords starting with dash characters

Author: rolandwalker

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

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',
         )

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]:

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'),
     [

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