[FIX] security: the KeePass password leaves the command line too

Same exposure as the master password, same fix. kdbx_manager put the Odoo
password straight into the web_login command; /proc/<pid>/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/<pid>/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
This commit is contained in:
Mathieu Benoit 2026-08-23 00:25:03 -04:00
parent 6cdbd528f3
commit 3f97b0a49c
5 changed files with 102 additions and 30 deletions

View file

@ -341,7 +341,11 @@ else
--email "$ADMIN_EMAIL" --must-change-password=false \ --email "$ADMIN_EMAIL" --must-change-password=false \
--config "$CONF" >/dev/null \ --config "$CONF" >/dev/null \
|| die "création de l'administrateur impossible" || 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 fi
# --- 9. Résumé ------------------------------------------------------------- # --- 9. Résumé -------------------------------------------------------------

View file

@ -32,7 +32,19 @@ def fill_parser(parser):
group_login.add_argument( group_login.add_argument(
"--default_password_auth", "--default_password_auth",
default="admin", 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 = ( email_auth = (
default_email_auth if default_email_auth else config.default_email_auth default_email_auth if default_email_auth else config.default_email_auth
) )
# L'environnement l'emporte : /proc/<pid>/cmdline est lisible par tout
# utilisateur de la machine, /proc/<pid>/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 = ( pass_auth = (
default_password_auth (pass_env and os.environ.get(pass_env))
if default_password_auth or default_password_auth
else config.default_password_auth or config.default_password_auth
) )
courriel_input.clear() courriel_input.clear()
mot_de_passe_input.clear() mot_de_passe_input.clear()
@ -100,9 +116,11 @@ def run(
) )
error_button.click() error_button.click()
# Remplissez le courriel et le mot de passe # Les valeurs RÉSOLUES, pas celles du config : la reprise
courriel_input.send_keys(config.default_email_auth) # renvoyait le défaut « admin » dès qu'un identifiant avait été
mot_de_passe_input.send_keys(config.default_password_auth) # fourni autrement, et échouait sans dire pourquoi.
courriel_input.send_keys(email_auth)
mot_de_passe_input.send_keys(pass_auth)
connexion_button.click() connexion_button.click()
else: else:

View file

@ -91,12 +91,24 @@ class KdbxManager:
def get_extra_command_user( def get_extra_command_user(
self, kdbx_key: str | list | None 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/<pid>/cmdline est lisible par tout
utilisateur de la machine, /proc/<pid>/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 = [] values = []
env = {}
if kdbx_key: if kdbx_key:
kp = self.get_kdbx() kp = self.get_kdbx()
if not kp: if not kp:
return "" return "", {}
if type(kdbx_key) is not list: if type(kdbx_key) is not list:
kdbx_keys = [kdbx_key] kdbx_keys = [kdbx_key]
else: else:
@ -111,13 +123,14 @@ class KdbxManager:
odoo_password = entry.password odoo_password = entry.password
except AttributeError: except AttributeError:
_logger.error(f"Cannot find password from keys {key}") _logger.error(f"Cannot find password from keys {key}")
var = f"EL_WEB_LOGIN_PWD_{len(values)}"
env[var] = odoo_password
values.append( values.append(
" --default_email_auth" " --default_email_auth"
f" {odoo_user} --default_password_auth" f" {odoo_user} --default_password_auth_env {var}"
f" '{odoo_password}'"
) )
if len(values) == 0: if len(values) == 0:
return "" return "", {}
elif len(values) == 1: elif len(values) == 1:
return values[0] return values[0], env
return values return values, env

View file

@ -510,14 +510,19 @@ class TODO:
odoo_user = instance.get("user") odoo_user = instance.get("user")
odoo_password = instance.get("password") 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: 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: elif odoo_user and odoo_password:
web_login_env = {"EL_WEB_LOGIN_PWD_0": odoo_password}
extra_cmd_web_login = ( extra_cmd_web_login = (
f" --default_email_auth {odoo_user} --default_password_auth" f" --default_email_auth {odoo_user}"
f" '{odoo_password}'" " --default_password_auth_env EL_WEB_LOGIN_PWD_0"
) )
else: else:
extra_cmd_web_login = "" extra_cmd_web_login = ""
@ -538,7 +543,9 @@ class TODO:
if exec_run_db: if exec_run_db:
db_name = instance.get("database") db_name = instance.get("database")
self.prompt_execute_selenium_and_run_db( 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") bash_command = instance.get("bash_command")
@ -554,7 +561,9 @@ class TODO:
command = instance.get("command") command = instance.get("command")
if command: if command:
self.prompt_execute_selenium( 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") callback = instance.get("callback")
@ -11401,16 +11410,20 @@ class TODO:
) )
def prompt_execute_selenium_and_run_db( 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" cmd_server = f"./run.sh -d {db_name};bash"
self.execute.exec_command_live(cmd_server) self.execute.exec_command_live(cmd_server)
cmd_client = ( cmd_client = (
f"sleep 3;./script/selenium/web_login.py{extra_cmd_web_login};bash" 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 = [] commands = []
if not command: if not command:
cmd = "./script/selenium/web_login.py" cmd = "./script/selenium/web_login.py"
@ -11423,13 +11436,16 @@ class TODO:
else: else:
commands.append(cmd + extra_cmd_web_login) commands.append(cmd + extra_cmd_web_login)
env = web_login_env or None
if len(commands) == 1: 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: elif len(commands) > 1:
new_cmd = "parallel ::: " new_cmd = "parallel ::: "
for i, cmd in enumerate(commands): for i, cmd in enumerate(commands):
new_cmd += f' "sleep {1 * i};{cmd}"' 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): def crash_diagnostic(self, e):
# TODO show message at start if os.path.exists(ERROR_LOG_PATH) # TODO show message at start if os.path.exists(ERROR_LOG_PATH)

View file

@ -403,21 +403,42 @@ class TestTestMenuDispatch(unittest.TestCase):
class TestKdbxGetExtraCommandUser(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/<pid>/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): def test_empty_kdbx_key(self):
todo = TODO() todo = TODO()
result = todo.kdbx_manager.get_extra_command_user("") result = todo.kdbx_manager.get_extra_command_user("")
self.assertEqual(result, "") self.assertEqual(result, ("", {}))
def test_none_kdbx_key(self): def test_none_kdbx_key(self):
todo = TODO() todo = TODO()
result = todo.kdbx_manager.get_extra_command_user(None) result = todo.kdbx_manager.get_extra_command_user(None)
self.assertEqual(result, "") self.assertEqual(result, ("", {}))
def test_kdbx_not_available(self): def test_kdbx_not_available(self):
todo = TODO() todo = TODO()
todo.kdbx_manager.get_kdbx = MagicMock(return_value=None) todo.kdbx_manager.get_kdbx = MagicMock(return_value=None)
result = todo.kdbx_manager.get_extra_command_user("some_key") 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): class TestSetupClaudeCommit(unittest.TestCase):