From 3f97b0a49c497616755a0526dc5abfc779107a08 Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Sun, 23 Aug 2026 00:25:03 -0400 Subject: [PATCH] [FIX] security: the KeePass password leaves the command line too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same exposure as the master password, same fix. kdbx_manager put the Odoo password straight into the web_login command; /proc//cmdline is readable by every user on the machine, and no downstream filter reaches that. The command now carries the NAME of an environment variable, never the value. One name per entry, because several credentials go out in a single "parallel" call and a single variable could not tell them apart. get_extra_command_user therefore returns (fragments, variables), and the two call sites hand the variables to exec_command_live, which already merged an environment. Two things found on the way. web_login re-sent config.default_password_auth when it retried after dismissing a modal, ignoring whatever the caller had passed -- the retry silently fell back to "admin". And install_forgejo printed the admin password back to the terminal, hence into the install log and any CI capture; its own header already documents the default. A test pins the guarantee: the fragment must not contain the password. --- FR --- Même exposition que pour le mot de passe maître, même correctif. kdbx_manager mettait le mot de passe Odoo directement dans la commande web_login ; /proc//cmdline est lisible par tout utilisateur de la machine, et aucun filtre en aval ne l'atteint. La commande porte désormais le NOM d'une variable d'environnement, jamais la valeur. Un nom par entrée, car plusieurs identifiants partent dans un seul appel « parallel » et une variable unique ne saurait les distinguer. get_extra_command_user rend donc (fragments, variables), et les deux appelants confient les variables à exec_command_live, qui fusionnait déjà un environnement. Deux trouvailles en chemin. web_login renvoyait config.default_password_auth à la reprise après une modale, ignorant ce que l'appelant avait fourni — la reprise retombait en silence sur « admin ». Et install_forgejo réaffichait le mot de passe administrateur, donc dans le journal d'installation et toute capture de CI ; son propre en-tête documente déjà le défaut. Un test verrouille la garantie : le fragment ne doit pas porter le secret. Assisted-by: Claude Opus 5 --- script/forgejo/install_forgejo.sh | 6 ++++- script/selenium/web_login.py | 32 +++++++++++++++++++------ script/todo/kdbx_manager.py | 27 +++++++++++++++------ script/todo/todo.py | 40 +++++++++++++++++++++---------- test/test_todo.py | 27 ++++++++++++++++++--- 5 files changed, 102 insertions(+), 30 deletions(-) diff --git a/script/forgejo/install_forgejo.sh b/script/forgejo/install_forgejo.sh index d2f873b..ca312c3 100755 --- a/script/forgejo/install_forgejo.sh +++ b/script/forgejo/install_forgejo.sh @@ -341,7 +341,11 @@ else --email "$ADMIN_EMAIL" --must-change-password=false \ --config "$CONF" >/dev/null \ || die "création de l'administrateur impossible" - say "${Green}administrateur créé : $ADMIN_USER / $ADMIN_PASSWORD${Color_Off}" + # Le mot de passe n'est PAS réaffiché : cette sortie part dans les + # journaux d'installation et dans toute capture de CI. Celui qui a + # posé FORGEJO_ADMIN_PASSWORD le connaît déjà ; les autres ont le + # défaut, documenté en tête de ce fichier. + say "${Green}administrateur créé : $ADMIN_USER${Color_Off}" fi # --- 9. Résumé ------------------------------------------------------------- diff --git a/script/selenium/web_login.py b/script/selenium/web_login.py index e26fd67..d747711 100755 --- a/script/selenium/web_login.py +++ b/script/selenium/web_login.py @@ -32,7 +32,19 @@ def fill_parser(parser): group_login.add_argument( "--default_password_auth", default="admin", - help="Password to use to authenticate with admin.", + help=( + "Password to use to authenticate with admin. Prefer" + " --default_password_auth_env: a value given here travels" + " through argv, which every user on the machine can read." + ), + ) + group_login.add_argument( + "--default_password_auth_env", + default=None, + help=( + "NAME of an environment variable holding the password. Only" + " the name reaches the command line; the value never does." + ), ) @@ -71,10 +83,14 @@ def run( email_auth = ( default_email_auth if default_email_auth else config.default_email_auth ) + # L'environnement l'emporte : /proc//cmdline est lisible par tout + # utilisateur de la machine, /proc//environ par son seul + # propriétaire. Le nom de la variable, lui, n'est pas un secret. + pass_env = getattr(config, "default_password_auth_env", None) pass_auth = ( - default_password_auth - if default_password_auth - else config.default_password_auth + (pass_env and os.environ.get(pass_env)) + or default_password_auth + or config.default_password_auth ) courriel_input.clear() mot_de_passe_input.clear() @@ -100,9 +116,11 @@ def run( ) error_button.click() - # Remplissez le courriel et le mot de passe - courriel_input.send_keys(config.default_email_auth) - mot_de_passe_input.send_keys(config.default_password_auth) + # Les valeurs RÉSOLUES, pas celles du config : la reprise + # renvoyait le défaut « admin » dès qu'un identifiant avait été + # fourni autrement, et échouait sans dire pourquoi. + courriel_input.send_keys(email_auth) + mot_de_passe_input.send_keys(pass_auth) connexion_button.click() else: diff --git a/script/todo/kdbx_manager.py b/script/todo/kdbx_manager.py index 8a798f6..3b7ba59 100644 --- a/script/todo/kdbx_manager.py +++ b/script/todo/kdbx_manager.py @@ -91,12 +91,24 @@ class KdbxManager: def get_extra_command_user( self, kdbx_key: str | list | None - ) -> str | list: + ) -> tuple[str | list, dict]: + """(fragments de commande, variables d'environnement à poser). + + Le mot de passe ne rejoint PAS la ligne de commande : seul le NOM + d'une variable y figure. /proc//cmdline est lisible par tout + utilisateur de la machine, /proc//environ par son seul + propriétaire — et un mot de passe KeePass n'a rien à faire dans la + liste des processus. + + Un nom par entrée : plusieurs identifiants partent dans UNE seule + commande « parallel », donc une variable unique ne suffirait pas. + """ values = [] + env = {} if kdbx_key: kp = self.get_kdbx() if not kp: - return "" + return "", {} if type(kdbx_key) is not list: kdbx_keys = [kdbx_key] else: @@ -111,13 +123,14 @@ class KdbxManager: odoo_password = entry.password except AttributeError: _logger.error(f"Cannot find password from keys {key}") + var = f"EL_WEB_LOGIN_PWD_{len(values)}" + env[var] = odoo_password values.append( " --default_email_auth" - f" {odoo_user} --default_password_auth" - f" '{odoo_password}'" + f" {odoo_user} --default_password_auth_env {var}" ) if len(values) == 0: - return "" + return "", {} elif len(values) == 1: - return values[0] - return values + return values[0], env + return values, env diff --git a/script/todo/todo.py b/script/todo/todo.py index 04f9964..ef32dc5 100755 --- a/script/todo/todo.py +++ b/script/todo/todo.py @@ -510,14 +510,19 @@ class TODO: odoo_user = instance.get("user") odoo_password = instance.get("password") + # Le mot de passe voyage par l'environnement, jamais par argv : la + # ligne de commande est lisible par tout utilisateur de la machine. + web_login_env = {} if kdbx_key: - extra_cmd_web_login = self.kdbx_manager.get_extra_command_user( - kdbx_key - ) + ( + extra_cmd_web_login, + web_login_env, + ) = self.kdbx_manager.get_extra_command_user(kdbx_key) elif odoo_user and odoo_password: + web_login_env = {"EL_WEB_LOGIN_PWD_0": odoo_password} extra_cmd_web_login = ( - f" --default_email_auth {odoo_user} --default_password_auth" - f" '{odoo_password}'" + f" --default_email_auth {odoo_user}" + " --default_password_auth_env EL_WEB_LOGIN_PWD_0" ) else: extra_cmd_web_login = "" @@ -538,7 +543,9 @@ class TODO: if exec_run_db: db_name = instance.get("database") self.prompt_execute_selenium_and_run_db( - db_name, extra_cmd_web_login=extra_cmd_web_login + db_name, + extra_cmd_web_login=extra_cmd_web_login, + web_login_env=web_login_env, ) bash_command = instance.get("bash_command") @@ -554,7 +561,9 @@ class TODO: command = instance.get("command") if command: self.prompt_execute_selenium( - command=command, extra_cmd_web_login=extra_cmd_web_login + command=command, + extra_cmd_web_login=extra_cmd_web_login, + web_login_env=web_login_env, ) callback = instance.get("callback") @@ -11401,16 +11410,20 @@ class TODO: ) def prompt_execute_selenium_and_run_db( - self, db_name, extra_cmd_web_login="" + self, db_name, extra_cmd_web_login="", web_login_env=None ): cmd_server = f"./run.sh -d {db_name};bash" self.execute.exec_command_live(cmd_server) cmd_client = ( f"sleep 3;./script/selenium/web_login.py{extra_cmd_web_login};bash" ) - self.execute.exec_command_live(cmd_client) + self.execute.exec_command_live( + cmd_client, new_env=web_login_env or None + ) - def prompt_execute_selenium(self, command=None, extra_cmd_web_login=""): + def prompt_execute_selenium( + self, command=None, extra_cmd_web_login="", web_login_env=None + ): commands = [] if not command: cmd = "./script/selenium/web_login.py" @@ -11423,13 +11436,16 @@ class TODO: else: commands.append(cmd + extra_cmd_web_login) + env = web_login_env or None if len(commands) == 1: - self.execute.exec_command_live(commands[0]) + self.execute.exec_command_live(commands[0], new_env=env) elif len(commands) > 1: new_cmd = "parallel ::: " for i, cmd in enumerate(commands): new_cmd += f' "sleep {1 * i};{cmd}"' - self.execute.exec_command_live(new_cmd) + # « parallel » hérite de l'environnement, et chaque entrée lit + # SA variable : un nom par identifiant, d'où EL_WEB_LOGIN_PWD_N. + self.execute.exec_command_live(new_cmd, new_env=env) def crash_diagnostic(self, e): # TODO show message at start if os.path.exists(ERROR_LOG_PATH) diff --git a/test/test_todo.py b/test/test_todo.py index e98c47e..5900f63 100644 --- a/test/test_todo.py +++ b/test/test_todo.py @@ -403,21 +403,42 @@ class TestTestMenuDispatch(unittest.TestCase): class TestKdbxGetExtraCommandUser(unittest.TestCase): + """La fonction rend (fragments, variables d'environnement). + + Le mot de passe ne doit JAMAIS revenir dans les fragments : ils + deviennent une ligne de commande, que tout utilisateur de la machine + peut lire dans /proc//cmdline. Seul le NOM d'une variable y a sa + place, et c'est ce que le dernier test verrouille. + """ + def test_empty_kdbx_key(self): todo = TODO() result = todo.kdbx_manager.get_extra_command_user("") - self.assertEqual(result, "") + self.assertEqual(result, ("", {})) def test_none_kdbx_key(self): todo = TODO() result = todo.kdbx_manager.get_extra_command_user(None) - self.assertEqual(result, "") + self.assertEqual(result, ("", {})) def test_kdbx_not_available(self): todo = TODO() todo.kdbx_manager.get_kdbx = MagicMock(return_value=None) result = todo.kdbx_manager.get_extra_command_user("some_key") - self.assertEqual(result, "") + self.assertEqual(result, ("", {})) + + def test_password_never_reaches_the_command_line(self): + todo = TODO() + entry = MagicMock(username="odoo", password="s3cr3t") + kp = MagicMock() + kp.find_entries_by_title = MagicMock(return_value=entry) + todo.kdbx_manager.get_kdbx = MagicMock(return_value=kp) + fragment, env = todo.kdbx_manager.get_extra_command_user("une_cle") + self.assertNotIn("s3cr3t", fragment) + self.assertIn( + "--default_password_auth_env EL_WEB_LOGIN_PWD_0", fragment + ) + self.assertEqual(env, {"EL_WEB_LOGIN_PWD_0": "s3cr3t"}) class TestSetupClaudeCommit(unittest.TestCase):