fix(secrets): keep passwords out of process arguments, previews and errors

CHANGELOG.md

@@ -31,6 +31,16 @@ All notable changes to this project are documented here. The format is based on
 
 ### Fixed
 
+- **Passwords were visible to every local user.** Each database password went into the command text
+  (`PGPASSWORD=… psql`), so it was in the arguments of the running step, which `ps` shows any user (another
+  instance's Odoo user included), and it was printed in the plan preview and in a failed step's error, as were
+  `odoo.conf`'s passwords. Steps now carry secrets in their environment, SQL with a password goes to psql on
+  stdin, files are written atomically with their content in the environment, the preview masks secrets, and a
+  failure names the step. Database probes give up after 10 seconds instead of minutes.
+- The addon dependency check reported packages declared by their distribution name (`python-stdnum`) as
+  missing; it now asks by distribution name first, as Odoo does. The health check now reaches a Unix-socket
+  database (`db_host = False`), a failed export no longer crashes the CLI, and the server report's database
+  list no longer matches by name prefix.
 - **The v1.2.0 wheel did not run**: it left out `instance_manager.workflows`. Packages are now found, and CI and
   the release build the wheel and import it from a clean venv; the release fails when the changelog has no
   section for the tag.

CONTRIBUTING.md

@@ -50,6 +50,7 @@ python3 tools/verify_data_safety.py  # run the generated backup/retention/delete
 python3 tools/verify_install_runtime.py --clone 14=/path/odoo-14.0 --clone 16=/path/odoo-16.0
 python3 tools/verify_neutralisation.py --odoo 18=/path/venv/bin/python:/path/odoo-18.0
 python3 tools/verify_ops_configs.py    # nginx -t and fail2ban on the generated config
+python3 tools/verify_secrets.py        # secrets in no process argument, preview or error
 ```
 
 `tools/verify_data_safety.py` is not part of CI: it executes the generated scheduled-backup script, the

docs/architecture.md

@@ -66,7 +66,10 @@ flowchart LR
     apply -- error during install --> cleanup["Best-effort<br/>cleanup residues"]
 ```
 
-- **Commands** are `(description, command)` pairs (`system.Command`). A plan is just a `list[Command]`.
+- **Commands** are `system.Command(description, command, env, display)`. A plan is just a `list[Command]`.
+  Secrets never go in `command`: a password or a file holding one travels in `env` (the step's environment,
+  readable only by root, unlike its arguments, which `ps` shows every user), and `display` is what the preview
+  shows instead, with the secrets masked. A failed step is reported by its description.
 - `preview_commands` renders the whole plan before anything runs; `apply_commands` runs it, stopping on error
   (and re-raising so install flows can clean up).
 - Destructive actions add `confirm_with_phrase` — the operator must type an exact phrase naming the

instance_manager/i18n.py

@@ -380,6 +380,9 @@ def tf(template: str, *args: object, **kwargs: object) -> str:
     "No se pudo detectar el puerto SSH (sshd -T / ssh.socket).": "Could not detect the SSH port (sshd -T / ssh.socket).",
     "SSH no escucha en {}: activar UFW bloquearía las sesiones SSH nuevas. ¿Continuar igualmente?": "SSH does not listen on {}: enabling UFW would lock new SSH sessions out. Continue anyway?",
     "No existe la regla #{}.": "There is no rule #{}.",
+    "No se pudo escribir {}: {}": "Could not write {}: {}",
+    "Fallo al ejecutar: {}": "Failed running: {}",
+    "Escribir {} (modo {})": "Write {} (mode {})",
     "Regla": "Rule",
     "Filas": "Rows",
     "Ejemplos": "Examples",

instance_manager/planners.py

@@ -6,7 +6,7 @@
 from . import support
 from .i18n import tf
 from .models import InstanceConfig, host_cidr
-from .system import Command
+from .system import Command, mask, pg_env
 
 
 def _sql_literal(value: str) -> str:
@@ -43,14 +43,16 @@ def _db_role_create_if_missing_sql(config: InstanceConfig) -> str:
     )
 
 
-def _db_connectivity_check_command(config: InstanceConfig) -> str:
+def _db_connectivity_check(config: InstanceConfig) -> Command:
+    """Log in as the instance's role, the way Odoo will (TCP for a local host)."""
     check_host = config.db_host
     if _is_local_db_host(check_host):
         check_host = "127.0.0.1"
-    return (
-        f"PGPASSWORD={shlex.quote(config.db_password)} "
-        f"psql -h {shlex.quote(check_host)} -p {config.db_port} -U {shlex.quote(config.db_user)} "
-        "-d postgres -tAc \"SELECT 1;\" >/dev/null"
+    return Command(
+        'Validate DB user login',
+        f"psql -X -h {shlex.quote(check_host)} -p {int(config.db_port)} -U {shlex.quote(config.db_user)} "
+        "-d postgres -tAc 'SELECT 1;' >/dev/null",
+        env=pg_env(config.db_password),
     )
 
 
@@ -158,7 +160,11 @@ def plan_update_instance_config(
     an Odoo that needs it pinned. A new DB password is set on the local role too, or
     the next start could not connect. The service is restarted when it runs, since
     ``systemctl start`` on a running unit loads nothing."""
-    commands = write_text_file_command(config.odoo_conf_file, render_merged_odoo_conf(config, existing), "640")
+    commands = write_text_file_command(
+        config.odoo_conf_file, render_merged_odoo_conf(config, existing), "640",
+        secrets=(config.odoo_admin_passwd, config.db_password,
+                 *(value for key, value in existing.items() if "pass" in key or "secret" in key)),
+    )
     commands.append(
         Command("Owner config Odoo", f"chown root:{shlex.quote(config.odoo_user)} {shlex.quote(config.odoo_conf_file)}")
     )
@@ -168,9 +174,9 @@ def plan_update_instance_config(
     if new_db_password and not config.is_remote_db_host:
         sql = f"ALTER ROLE {config.db_user} WITH PASSWORD '{_sql_literal(config.db_password)}';"
         commands.append(
-            Command(
+            _psql_stdin_command(
+                "sudo -u postgres psql -X -q -v ON_ERROR_STOP=1", sql, (config.db_password,),
                 'Set the new password on the local PostgreSQL role',
-                f"sudo -u postgres psql -X -q -v ON_ERROR_STOP=1 -c {shlex.quote(sql)}",
             )
         )
     if restart:
@@ -408,20 +414,36 @@ def staged_files_command(files: list[tuple[str, str, str]], validate: str) -> st
 
 
 def write_text_file_command(
-    target_path: str, content: str, mode: str = "640"
+    target_path: str, content: str, mode: str = "640", secrets: tuple[str, ...] = ()
 ) -> list[Command]:
+    """Write ``content`` to ``target_path`` atomically with ``mode``: a private
+    temporary file in the same directory, chmod, then rename — the file is never
+    seen half-written or with the wrong mode. The content travels in the step's
+    environment, so a file holding secrets (odoo.conf) is not in any process's
+    arguments, and the preview shows it with ``secrets`` masked."""
+    target = shlex.quote(target_path)
+    command = (
+        f'umask 077 && tmp=$(mktemp "$(dirname {target})/.oim-write.XXXXXX") && '
+        f'printf \'%s\' "$OIM_FILE_CONTENT" > "$tmp" && chmod {mode} "$tmp" && mv -f "$tmp" {target}'
+    )
+    body = content if content.endswith("\n") else content + "\n"
     return [
         Command(
-            description=tf("Write {}", target_path),
-            command=f"cat > '{target_path}' <<'EOF'\n{content}\nEOF",
-        ),
-        Command(
-            description=tf('Permissions {} on {}', mode, target_path),
-            command=f"chmod {mode} '{target_path}'",
-        ),
+            description=tf('Write {} (mode {})', target_path, mode),
+            command=command,
+            env={"OIM_FILE_CONTENT": body},
+            display=f"{command}\n--- {target_path} ---\n{mask(body, secrets)}",
+        )
     ]
 
 
+def _psql_stdin_command(psql: str, sql: str, secrets: tuple[str, ...], description: str) -> Command:
+    """``sql`` fed to ``psql`` on stdin from the step's environment: a statement
+    holding a password is in no process's arguments, and the preview masks it."""
+    command = f'{psql} <<<"$OIM_SQL"'
+    return Command(description, command, env={"OIM_SQL": sql}, display=f"{command}\n{mask(sql, secrets)}")
+
+
 def _logrotate_content(
     config: InstanceConfig,
     frequency: str,
@@ -768,7 +790,8 @@ def plan_odoo_base_setup(
 
     commands.extend(
         write_text_file_command(
-            config.odoo_conf_file, _odoo_conf_content(config), "640"
+            config.odoo_conf_file, _odoo_conf_content(config), "640",
+            secrets=(config.odoo_admin_passwd, config.db_password),
         )
     )
     commands.extend(
@@ -951,14 +974,11 @@ def plan_db_setup(
         ),
         Command('Enable and start PostgreSQL', "systemctl enable --now postgresql"),
         *_postgres_floor_commands(config),
-        Command(
+        _psql_stdin_command(
+            "sudo -u postgres psql -X -v ON_ERROR_STOP=1", role_sql, (config.db_password,),
             'Ensure PostgreSQL role (create if missing)',
-            f"sudo -u postgres psql -v ON_ERROR_STOP=1 -c {shlex.quote(role_sql)}",
-        ),
-        Command(
-            'Validate DB user login',
-            _db_connectivity_check_command(config),
         ),
+        _db_connectivity_check(config),
     ]
 
     if ensure_remote_access:
@@ -988,9 +1008,9 @@ def plan_ensure_db_role(config: InstanceConfig) -> list[Command]:
     if _is_local_db_host(config.db_host):
         role_sql = _db_role_create_if_missing_sql(config)
         commands.append(
-            Command(
+            _psql_stdin_command(
+                "sudo -u postgres psql -X -v ON_ERROR_STOP=1", role_sql, (config.db_password,),
                 'Ensure local PostgreSQL role (create if missing)',
-                f"sudo -u postgres psql -v ON_ERROR_STOP=1 -c {shlex.quote(role_sql)}",
             )
         )
     else:
@@ -1001,12 +1021,7 @@ def plan_ensure_db_role(config: InstanceConfig) -> list[Command]:
             )
         )
 
-    commands.append(
-        Command(
-            'Validate DB user login',
-            _db_connectivity_check_command(config),
-        )
-    )
+    commands.append(_db_connectivity_check(config))
     return commands
 
 

instance_manager/system.py

@@ -6,25 +6,61 @@
 import shlex
 import shutil
 import subprocess
-from dataclasses import dataclass
+from dataclasses import dataclass, field
 
 from .i18n import t, tf
 from .ui import level_text, style, title, wrap_plain_block
 
 
 @dataclass
 class Command:
+    """One plan step. ``env`` carries what must not be in the command text —
+    passwords, file contents holding secrets — into the step's environment, which
+    only root can read (``/proc/<pid>/environ``), unlike a process's arguments,
+    which every local user sees in ``ps``. ``display`` is what the preview shows
+    instead of ``command`` when the step was written for it (secrets masked)."""
+
     description: str
     command: str
+    env: dict[str, str] = field(default_factory=dict)
+    display: str = ""
+
+    @property
+    def shown(self) -> str:
+        return self.display or self.command
+
+
+# A remote PostgreSQL that drops packets would otherwise hold a probe (and the
+# menu) until the TCP connect gives up, minutes later.
+PG_CONNECT_TIMEOUT = "10"
+
+
+def pg_env(password: str) -> dict[str, str]:
+    """The environment a libpq client takes its password from: never its argv."""
+    return {"PGPASSWORD": password or "", "PGCONNECT_TIMEOUT": PG_CONNECT_TIMEOUT}
+
+
+def mask(text: str, secrets: tuple[str, ...] | list[str]) -> str:
+    """``text`` with every non-empty secret replaced by ``********``."""
+    for secret in sorted((s for s in secrets if s), key=len, reverse=True):
+        text = text.replace(secret, "********")
+    return text
+
+
+def _environment(env: dict[str, str] | None) -> dict[str, str] | None:
+    return {**os.environ, **env} if env else None
 
 
-def run(command: str, check: bool = False) -> subprocess.CompletedProcess[str]:
+def run(
+    command: str, check: bool = False, env: dict[str, str] | None = None
+) -> subprocess.CompletedProcess[str]:
     result = subprocess.run(
         ["bash", "-lc", command],
         capture_output=True,
         text=True,
         encoding="utf-8",
         errors="replace",
+        env=_environment(env),
     )
     if check and result.returncode != 0:
         raise RuntimeError(
@@ -36,7 +72,7 @@ def run(command: str, check: bool = False) -> subprocess.CompletedProcess[str]:
     return result
 
 
-def run_streaming(command: str) -> subprocess.CompletedProcess[str]:
+def run_streaming(command: str, env: dict[str, str] | None = None) -> subprocess.CompletedProcess[str]:
     """Run a command, forwarding its output live while also capturing it.
 
     stdlib-only (``subprocess.Popen``): stderr is merged into stdout so combined
@@ -53,6 +89,7 @@ def run_streaming(command: str) -> subprocess.CompletedProcess[str]:
         encoding="utf-8",
         errors="replace",
         bufsize=1,
+        env=_environment(env),
     )
     captured: list[str] = []
     assert process.stdout is not None
@@ -139,19 +176,21 @@ def preview_commands(commands: list[Command]) -> None:
     body_width = max(20, shutil.get_terminal_size((100, 24)).columns - len(indent))
     for index, item in enumerate(commands, start=1):
         print(f"\n{style(f'[{index:02d}]', 'blue', 'bold')} {t(item.description)}")
-        for chunk in wrap_plain_block(item.command, body_width):
+        for chunk in wrap_plain_block(item.shown, body_width):
             print(style(f"{indent}{chunk}", "dim"))
 
 
 def apply_commands(commands: list[Command], stop_on_error: bool = True) -> None:
     for index, item in enumerate(commands, start=1):
         print(f"\n{style(f'[{index}/{len(commands)}]', 'blue', 'bold')} {t(item.description)}")
         # Stream output live so long steps (apt/pip/pg_restore) aren't silent.
-        result = run_streaming(item.command)
+        result = run_streaming(item.command, item.env)
         if result.returncode != 0:
             print(level_text("ERROR", tf('Command finished with code {}.', result.returncode)))
             if stop_on_error:
-                raise RuntimeError(f"Failed running: {item.command}")
+                # The description, not the command: the command may hold what the
+                # preview masked.
+                raise RuntimeError(tf('Failed running: {}', t(item.description)))
 
 
 def list_dirs(base_path: str) -> list[str]:
@@ -310,7 +349,6 @@ def list_databases(
     instance shows only its own databases. A name prefix is not used: instance
     ``shop`` would otherwise list ``shop2`` and ``shop_eu``, and ``_`` is a LIKE
     wildcard."""
-    quoted_password = shlex.quote(db_password)
     quoted_host = shlex.quote(db_host)
     quoted_user = shlex.quote(db_user)
     if owner and re.fullmatch(r"[A-Za-z0-9_.-]{1,63}", owner):
@@ -323,10 +361,10 @@ def list_databases(
     else:
         select = "SELECT datname FROM pg_database WHERE datistemplate = false ORDER BY datname;"
     query_cmd = (
-        f"PGPASSWORD={quoted_password} psql -h {quoted_host} -p {db_port} -U {quoted_user} "
-        f'-d postgres -tA -c "{select}"'
+        f"psql -X -h {quoted_host} -p {int(db_port)} -U {quoted_user} "
+        f"-d postgres -tA -c {shlex.quote(select)}"
     )
-    result = run(query_cmd, check=False)
+    result = run(query_cmd, check=False, env=pg_env(db_password))
     if result.returncode != 0:
         error_text = (
             result.stderr.strip()

instance_manager/workflows/addons.py

@@ -10,7 +10,7 @@
 from ..i18n import tf
 from ..models import InstanceConfig
 from ..prompts import ask_bool, ask_text
-from ..system import read_odoo_conf, run
+from ..system import pg_env, read_odoo_conf, run
 from ..ui import level_tag, level_text, render_table, strip_ansi, title
 from .common import DbCredentials, _ask_db_credentials, _quote
 
@@ -99,10 +99,10 @@ def _installed_modules(creds: DbCredentials, db_name: str) -> dict[str, tuple[st
     """name -> (state, installed_version) from ir_module_module, or empty on error."""
     sql = "SELECT name, state, coalesce(latest_version, '') FROM ir_module_module"
     cmd = (
-        f"PGPASSWORD={_quote(creds.password)} psql -h {_quote(creds.host)} -p {creds.port} "
+        f"psql -X -h {_quote(creds.host)} -p {int(creds.port)} "
         f"-U {_quote(creds.user)} -d {_quote(db_name)} -tAF'|' -c {_quote(sql)}"
     )
-    result = run(cmd, check=False)
+    result = run(cmd, check=False, env=pg_env(creds.password))
     installed: dict[str, tuple[str, str]] = {}
     if result.returncode != 0:
         return installed
@@ -160,14 +160,26 @@ def _collect_python_deps(config: InstanceConfig) -> dict[str, set[str]]:
     return deps
 
 
+# Odoo's own check (odoo/modules/module.py, check_python_external_dependency):
+# the distribution name first — what a manifest is meant to name, e.g.
+# `python-stdnum` or `pdfminer.six` — then, failing that, an importable module.
+_DEPENDENCY_PROBE = (
+    "import importlib, importlib.metadata as m, sys\n"
+    "name = sys.argv[1]\n"
+    "try:\n    m.version(name)\n"
+    "except m.PackageNotFoundError:\n    importlib.import_module(name)\n"
+)
+
+
 def _venv_can_import(config: InstanceConfig, module: str) -> bool:
-    """True if ``module`` imports in the instance venv (run as the instance user)."""
-    if not re.fullmatch(r"[A-Za-z0-9_.]+", module):
+    """True if ``module`` is installed in the instance venv, as Odoo checks it: by
+    distribution name, else by import (run as the instance user)."""
+    if not re.fullmatch(r"[A-Za-z0-9_.-]+", module):
         return False
     venv_python = f"{config.odoo_home}/venv/bin/python"
     cmd = (
         f"sudo -u {_quote(config.odoo_user)} {_quote(venv_python)} "
-        f"-c {_quote('import ' + module)} >/dev/null 2>&1"
+        f"-c {_quote(_DEPENDENCY_PROBE)} {_quote(module)} >/dev/null 2>&1"
     )
     return run(cmd, check=False).returncode == 0
 
@@ -254,10 +266,13 @@ def _maybe_export_inventory(config: InstanceConfig, sections: list[str]) -> None
     default_path = f"./reports/addons_{config.instance}_{now}.txt"
     export_path = ask_text('Inventory export path', default_path, required=True)
 
-    export_dir = os.path.dirname(export_path) or "."
-    os.makedirs(export_dir, exist_ok=True)
     header = tf('Addon inventory: {}', config.instance)
-    with open(export_path, "w", encoding="utf-8") as file_handle:
-        file_handle.write(f"{header}\n\n" + "\n\n".join(sections) + "\n")
+    try:
+        os.makedirs(os.path.dirname(export_path) or ".", exist_ok=True)
+        with open(export_path, "w", encoding="utf-8") as file_handle:
+            file_handle.write(f"{header}\n\n" + "\n\n".join(sections) + "\n")
+    except OSError as error:
+        print(level_text("ERROR", tf('Could not write {}: {}', export_path, error.strerror or error)))
+        return
 
     print(level_text("OK", tf('Inventory exported to: {}', export_path)))