don't persist password-change SQL to history file

Leveraging the new is_password_change() detection, avoid persisting
password-change statements to the history file, but leave them available
for navigation in the current session.

Wrap the sqlglot.tokenize() call for is_password_change() in a try
block, since it could throw if given invalid SQL.

Add and improve tests for sql_utils.py.

Author: rolandwalker

changelog.md

@@ -8,6 +8,7 @@ Features
 * Allow more characters in passwords read from a file.
 * Show sponsors and contributors separately in startup messages.
 * Add support for expired password (sandbox) mode (#440)
+* Don't persist password-change statements to history file.
 
 
 Bug Fixes

mycli/packages/ptoolkit/history.py

@@ -3,6 +3,8 @@
 
 from prompt_toolkit.history import FileHistory
 
+from mycli.packages.sql_utils import is_password_change
+
 _StrOrBytesPath = Union[str, bytes, os.PathLike]
 
 
@@ -15,6 +17,13 @@ def __init__(self, filename: _StrOrBytesPath) -> None:
         self.filename = filename
         super().__init__(filename)
 
+    def append_string(self, string: str) -> None:
+        "Add string to the history."
+        self._loaded_strings.insert(0, string)
+        if is_password_change(string):
+            return
+        self.store_string(string)
+
     def load_history_with_timestamp(self) -> list[tuple[str, str]]:
         """
         Load history entries along with their timestamps.

mycli/packages/sql_utils.py

@@ -485,7 +485,11 @@ def classify_sandbox_statement(text: str) -> tuple[str | None, str | None]:
     if not stripped:
         return ('quit', None)
 
-    tokens = list(sqlglot.tokenize(stripped, dialect='mysql'))
+    try:
+        tokens = list(sqlglot.tokenize(stripped, dialect='mysql'))
+    except sqlglot.errors.TokenError:
+        tokens = []
+
     if not tokens:
         return ('quit', None)
 

test/pytests/test_ptoolkit_history.py

@@ -2,6 +2,7 @@
 
 from pathlib import Path
 
+from mycli.packages.ptoolkit import history as history_module
 from mycli.packages.ptoolkit.history import FileHistoryWithTimestamp
 
 
@@ -13,6 +14,29 @@ def test_file_history_with_timestamp_sets_filename(tmp_path: Path) -> None:
     assert history.filename == history_path
 
 
+def test_append_string_caches_and_stores_non_password_statement(tmp_path: Path, monkeypatch) -> None:
+    history = FileHistoryWithTimestamp(tmp_path / 'history.txt')
+    stored: list[str] = []
+    monkeypatch.setattr(history, 'store_string', stored.append)
+
+    history.append_string('SELECT 1')
+
+    assert history._loaded_strings[0] == 'SELECT 1'
+    assert stored == ['SELECT 1']
+
+
+def test_append_string_does_not_store_password_change(tmp_path: Path, monkeypatch) -> None:
+    history = FileHistoryWithTimestamp(tmp_path / 'history.txt')
+    stored: list[str] = []
+    monkeypatch.setattr(history, 'store_string', stored.append)
+    monkeypatch.setattr(history_module, 'is_password_change', lambda string: True)
+
+    history.append_string("SET PASSWORD = 'secret'")
+
+    assert history._loaded_strings[0] == "SET PASSWORD = 'secret'"
+    assert stored == []
+
+
 def test_load_history_with_timestamp_returns_empty_when_file_is_missing(tmp_path: Path) -> None:
     history = FileHistoryWithTimestamp(tmp_path / 'missing-history.txt')
 

test/pytests/test_sql_utils.py

@@ -1,5 +1,7 @@
 # type: ignore
 
+from types import SimpleNamespace
+
 import pytest
 import sqlparse
 from sqlparse.sql import Identifier, IdentifierList, Token, TokenList
@@ -563,6 +565,98 @@ def split(self):
     assert need_completion_reset('ignored') is False
 
 
+def test_classify_sandbox_statement_treats_token_error_as_quit(monkeypatch):
+    def raise_token_error(*_args, **_kwargs):
+        raise sql_utils.sqlglot.errors.TokenError('bad token')
+
+    monkeypatch.setattr(sql_utils.sqlglot, 'tokenize', raise_token_error)
+
+    assert sql_utils.classify_sandbox_statement('`') == ('quit', None)
+
+
+def test_classify_sandbox_statement_treats_empty_tokens_as_quit(monkeypatch):
+    monkeypatch.setattr(sql_utils.sqlglot, 'tokenize', lambda *_args, **_kwargs: [])
+
+    assert sql_utils.classify_sandbox_statement('ignored') == ('quit', None)
+
+
+def test_find_password_after_eq_returns_none_for_non_string_token() -> None:
+    token_type = sql_utils.sqlglot.tokens.TokenType
+    tokens = [
+        SimpleNamespace(token_type=token_type.EQ, text='='),
+        SimpleNamespace(token_type=token_type.VAR, text='CURRENT_USER'),
+    ]
+
+    assert sql_utils._find_password_after_eq(tokens) is None
+
+
+@pytest.mark.parametrize(
+    ('text', 'expected'),
+    [
+        ('', ('quit', None)),
+        ('  ', ('quit', None)),
+        ('quit', ('quit', None)),
+        ('exit', ('quit', None)),
+        ('\\q', ('quit', None)),
+        ("ALTER USER 'root'@'localhost' IDENTIFIED BY 'new'", ('alter_user', 'new')),
+        ('ALTER USER root IDENTIFIED WITH mysql_native_password', ('alter_user', None)),
+        ("SET PASSWORD = 'newpass'", ('set_password', 'newpass')),
+        ('SELECT 1', (None, None)),
+    ],
+)
+def test_classify_sandbox_statement(text: str, expected: tuple[str | None, str | None]) -> None:
+    assert sql_utils.classify_sandbox_statement(text) == expected
+
+
+@pytest.mark.parametrize(
+    ('text', 'expected'),
+    [
+        ('', True),
+        ('  ', True),
+        ("ALTER USER 'root'@'localhost' IDENTIFIED BY 'new'", True),
+        ('alter user root identified by "pw"', True),
+        ("SET PASSWORD = 'newpass'", True),
+        ("set password = 'newpass'", True),
+        ('quit', True),
+        ('exit', True),
+        ('\\q', True),
+        ('SELECT 1', False),
+        ('DROP TABLE t', False),
+        ('USE mydb', False),
+        ('SHOW DATABASES', False),
+    ],
+)
+def test_is_sandbox_allowed(text: str, expected: bool) -> None:
+    assert sql_utils.is_sandbox_allowed(text) is expected
+
+
+@pytest.mark.parametrize(
+    ('text', 'expected'),
+    [
+        ("ALTER USER 'root'@'localhost' IDENTIFIED BY 'new'", True),
+        ("SET PASSWORD = 'newpass'", True),
+        ('SELECT 1', False),
+        ('quit', False),
+    ],
+)
+def test_is_password_change(text: str, expected: bool) -> None:
+    assert sql_utils.is_password_change(text) is expected
+
+
+@pytest.mark.parametrize(
+    ('text', 'expected'),
+    [
+        ("ALTER USER 'root'@'localhost' IDENTIFIED BY 'newpass'", 'newpass'),
+        ("SET PASSWORD = 'secret123'", 'secret123'),
+        ("ALTER USER root IDENTIFIED BY 'p@ss w0rd!'", 'p@ss w0rd!'),
+        ('ALTER USER root IDENTIFIED WITH mysql_native_password', None),
+        ('SELECT 1', None),
+    ],
+)
+def test_extract_new_password(text: str, expected: str | None) -> None:
+    assert sql_utils.extract_new_password(text) == expected
+
+
 @pytest.mark.parametrize(
     ('status_plain', 'expected'),
     [