From 434e619b13d85a3f852e776c6bf098fa1102ef98 Mon Sep 17 00:00:00 2001 From: Guillermo Rojo Date: Thu, 8 Oct 2026 13:19:48 +0200 Subject: [PATCH] fix(secrets): keep passwords out of process arguments, previews and errors Every database password went into the command text (PGPASSWORD=... psql), so it sat in the running step's arguments, which ps shows every local user, including another instance's Odoo user; the plan preview and a failed step's error printed it again, as they printed odoo.conf's passwords. A step now carries its secrets in Command.env and what the preview shows in Command.display, masked. libpq clients take PGPASSWORD (and a 10 s connect timeout) from the environment, SQL with a password goes to psql on stdin, files are written atomically with their content in the environment, and a failure names the step. Also: dependencies are checked by distribution name first, as Odoo does; the health check reaches a socket database; a failed export no longer crashes; the report's database list no longer matches by prefix. tools/verify_secrets.py runs the steps and looks at ps. --- CHANGELOG.md | 10 ++ CONTRIBUTING.md | 1 + docs/architecture.md | 5 +- instance_manager/i18n.py | 3 + instance_manager/planners.py | 79 +++++----- instance_manager/system.py | 58 ++++++-- instance_manager/workflows/addons.py | 35 +++-- instance_manager/workflows/backup_restore.py | 29 ++-- instance_manager/workflows/common.py | 7 +- instance_manager/workflows/health.py | 7 +- instance_manager/workflows/manage.py | 4 +- instance_manager/workflows/purge.py | 25 ++-- instance_manager/workflows/report.py | 13 +- .../changes/secrets-out-of-sight/proposal.md | 28 ++++ .../specs/addon-inventory/spec.md | 13 ++ .../specs/execution-safety/spec.md | 31 ++++ .../changes/secrets-out-of-sight/tasks.md | 7 + tests/test_db_existence.py | 2 +- tests/test_install_per_version.py | 7 +- tests/test_planners.py | 4 +- tests/test_system.py | 6 +- tests/test_workflow_helpers.py | 8 +- tools/verify_secrets.py | 135 ++++++++++++++++++ 23 files changed, 424 insertions(+), 93 deletions(-) create mode 100644 openspec/changes/secrets-out-of-sight/proposal.md create mode 100644 openspec/changes/secrets-out-of-sight/specs/addon-inventory/spec.md create mode 100644 openspec/changes/secrets-out-of-sight/specs/execution-safety/spec.md create mode 100644 openspec/changes/secrets-out-of-sight/tasks.md create mode 100644 tools/verify_secrets.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e752a3..dd3f210 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9496bd9..14ce644 100644 --- a/CONTRIBUTING.md +++ b/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 diff --git a/docs/architecture.md b/docs/architecture.md index 17c100c..d6d7984 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -66,7 +66,10 @@ flowchart LR apply -- error during install --> cleanup["Best-effort
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 diff --git a/instance_manager/i18n.py b/instance_manager/i18n.py index 2d033e2..0c618d1 100644 --- a/instance_manager/i18n.py +++ b/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", diff --git a/instance_manager/planners.py b/instance_manager/planners.py index e9ee741..4243837 100644 --- a/instance_manager/planners.py +++ b/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 diff --git a/instance_manager/system.py b/instance_manager/system.py index 5bc76f1..94324a7 100644 --- a/instance_manager/system.py +++ b/instance_manager/system.py @@ -6,7 +6,7 @@ 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 @@ -14,17 +14,53 @@ @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//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,7 +176,7 @@ 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")) @@ -147,11 +184,13 @@ 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() diff --git a/instance_manager/workflows/addons.py b/instance_manager/workflows/addons.py index 27fd71d..63efe31 100644 --- a/instance_manager/workflows/addons.py +++ b/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))) diff --git a/instance_manager/workflows/backup_restore.py b/instance_manager/workflows/backup_restore.py index f975af0..4a2d686 100644 --- a/instance_manager/workflows/backup_restore.py +++ b/instance_manager/workflows/backup_restore.py @@ -39,6 +39,7 @@ database_owner, detect_nginx_version, path_exists, + pg_env, read_odoo_conf, run, service_active, @@ -80,11 +81,9 @@ def _is_safe_db_name(name: str) -> bool: def _psql_target( db_host: str, db_port: int, db_user: str, db_password: str, target_db: str ) -> str: - """A psql invocation pointed at ``target_db`` using client credentials.""" - return ( - f"PGPASSWORD={_quote(db_password)} psql -h {_quote(db_host)} -p {db_port} " - f"-U {_quote(db_user)} -d {_quote(target_db)}" - ) + """A psql invocation pointed at ``target_db`` using client credentials; the + password goes in the step's environment (``pg_env``), never in this text.""" + return f"psql -h {_quote(db_host)} -p {int(db_port)} -U {_quote(db_user)} -d {_quote(target_db)}" def _psql_target_local(target_db: str) -> str: @@ -111,6 +110,7 @@ def _post_db_mode_commands( migration_mode: str, neutralize: bool, local_url: str, + env: dict[str, str] | None = None, ) -> list[Command]: """Apply Odoo migration semantics to an already-restored target database. @@ -127,6 +127,7 @@ def _post_db_mode_commands( Command( 'Give the copy its own identity: new database.uuid, database.secret, create date', f"{psql} {_quote(neutralise.copied_identity_sql())}", + env=dict(env or {}), ) ) if neutralize: @@ -134,10 +135,12 @@ def _post_db_mode_commands( Command( 'Neutralise the copy (crons, mail, payment, EDI/SII, calendars, webhooks, IAP, base URL)', f"{psql} {_quote(neutralise.apply_sql(local_url))}", + env=dict(env or {}), ), Command( 'Check nothing in the copy can still act on the outside', f"{psql} {_quote(neutralise.guard_sql(local_url))}", + env=dict(env or {}), ), ] return commands @@ -300,8 +303,9 @@ def _backup_instance( Command( 'Export DB backup (custom format, atomic)', f"umask 077 && TMP={_quote(dump_path + '.partial')} && " - f"PGPASSWORD={_quote(db_password)} pg_dump -h {_quote(db_host)} -p {db_port} -U {_quote(db_user)} -Fc -f \"$TMP\" {_quote(db_name)} && " + f"pg_dump -h {_quote(db_host)} -p {int(db_port)} -U {_quote(db_user)} -Fc -f \"$TMP\" {_quote(db_name)} && " f"mv \"$TMP\" {_quote(dump_path)} || {{ rm -f \"$TMP\"; exit 1; }}", + env=pg_env(db_password), ) ) @@ -378,11 +382,13 @@ def _restore_backup( [ Command( 'Create target DB', - f"PGPASSWORD={_quote(db_password)} createdb -h {_quote(db_host)} -p {db_port} -U {_quote(db_user)} -O {_quote(db_user)} {_quote(target_db)}", + f"createdb -h {_quote(db_host)} -p {int(db_port)} -U {_quote(db_user)} -O {_quote(db_user)} {_quote(target_db)}", + env=pg_env(db_password), ), Command( 'Restore the dump into the target DB', - f"PGPASSWORD={_quote(db_password)} pg_restore -h {_quote(db_host)} -p {db_port} -U {_quote(db_user)} -d {_quote(target_db)} --no-owner --no-privileges {_quote(dump_file)}", + f"pg_restore -h {_quote(db_host)} -p {int(db_port)} -U {_quote(db_user)} -d {_quote(target_db)} --no-owner --no-privileges {_quote(dump_file)}", + env=pg_env(db_password), ), ] ) @@ -392,6 +398,7 @@ def _restore_backup( migration_mode, neutralize, _local_url(_config_with_port(config, read_odoo_conf(config.odoo_conf_file))), + env=pg_env(db_password), ) ) if stop_service: @@ -968,14 +975,15 @@ def _check_neutralisation( print(level_text("ERROR", _INVALID_DB_NAME)) return creds psql = f"{_psql_target(creds.host, creds.port, creds.user, creds.password, db_name)} -X -tA -F '|' -c" - found = run(f"{psql} {_quote(neutralise.columns_sql())}", check=False) + env = pg_env(creds.password) + found = run(f"{psql} {_quote(neutralise.columns_sql())}", check=False, env=env) if found.returncode != 0: print(level_text("ERROR", found.stderr.strip() or 'Could not read the database.')) return creds existing = {tuple(line.split("|", 1)) for line in found.stdout.splitlines() if "|" in line} rules = neutralise.applicable(existing) # type: ignore[arg-type] url = _local_url(_config_with_port(config, read_odoo_conf(config.odoo_conf_file))) - armed = run(f"{psql} {_quote(neutralise.check_sql(rules, url))}", check=False) + armed = run(f"{psql} {_quote(neutralise.check_sql(rules, url))}", check=False, env=env) rows = [line.split("|", 2) for line in armed.stdout.splitlines() if line.count("|") >= 2] sink = run( f"{psql} " @@ -984,6 +992,7 @@ def _check_neutralisation( f"'{neutralise.MAIL_SINK_NAME}' AND smtp_host = '{neutralise.MAIL_SINK_HOST}'" ), check=False, + env=env, ) if sink.stdout.strip() != "1": rows.append(["mail-sink", "0", t('missing: mail can leave through odoo.conf\'s smtp_server')]) diff --git a/instance_manager/workflows/common.py b/instance_manager/workflows/common.py index b5e8515..e05d2fb 100644 --- a/instance_manager/workflows/common.py +++ b/instance_manager/workflows/common.py @@ -21,6 +21,7 @@ list_databases, list_instances, path_exists, + pg_env, preview_commands, read_odoo_conf, require_root_for_apply, @@ -274,10 +275,10 @@ def _database_exists(creds: DbCredentials, db_name: str) -> bool: """ sql = "SELECT 1 FROM pg_database WHERE datname='" + db_name.replace("'", "''") + "'" cmd = ( - f"PGPASSWORD={shlex.quote(creds.password)} psql -h {shlex.quote(creds.host)} " - f"-p {creds.port} -U {shlex.quote(creds.user)} -d postgres -tAc {shlex.quote(sql)}" + f"psql -X -h {shlex.quote(creds.host)} " + f"-p {int(creds.port)} -U {shlex.quote(creds.user)} -d postgres -tAc {shlex.quote(sql)}" ) - result = run(cmd, check=False) + result = run(cmd, check=False, env=pg_env(creds.password)) return result.returncode == 0 and result.stdout.strip() == "1" diff --git a/instance_manager/workflows/health.py b/instance_manager/workflows/health.py index aad2fb9..15e480c 100644 --- a/instance_manager/workflows/health.py +++ b/instance_manager/workflows/health.py @@ -9,6 +9,7 @@ from ..models import InstanceConfig from ..system import ( path_exists, + pg_env, read_odoo_conf, run, service_active, @@ -40,11 +41,13 @@ def _db_probe(config: InstanceConfig, conf: dict[str, str]) -> tuple[bool, str]: db_port = conf.get("db_port", str(config.db_port)) or "5432" db_user = conf.get("db_user", config.db_user) or config.instance db_password = conf.get("db_password", "") + # `db_host = False` (or empty) is Odoo's local Unix socket: psql takes no -h then. + host = "" if db_host.strip().lower() in {"", "false", "none"} else f"-h {_quote(db_host)} " cmd = ( - f"PGPASSWORD={_quote(db_password)} psql -h {_quote(db_host)} -p {_quote(str(db_port))} " + f"psql -X {host}-p {_quote(str(db_port))} " f"-U {_quote(db_user)} -d postgres -tAc 'SELECT 1' >/dev/null 2>&1" ) - ok = run(cmd, check=False).returncode == 0 + ok = run(cmd, check=False, env=pg_env(db_password)).returncode == 0 return ok, f"{db_user}@{db_host}:{db_port}" diff --git a/instance_manager/workflows/manage.py b/instance_manager/workflows/manage.py index c3b3471..6de0b36 100644 --- a/instance_manager/workflows/manage.py +++ b/instance_manager/workflows/manage.py @@ -29,6 +29,7 @@ detect_cpu_count, detect_nginx_version, path_exists, + pg_env, read_odoo_conf, service_active, service_exists, @@ -570,7 +571,8 @@ def _delete_instance( commands.append( Command( 'Delete DB', - f"PGPASSWORD={_quote(db_password)} dropdb --if-exists -h {_quote(db_host)} -p {db_port} -U {_quote(db_user)} {_quote(db_name)}", + f"dropdb --if-exists -h {_quote(db_host)} -p {int(db_port)} -U {_quote(db_user)} {_quote(db_name)}", + env=pg_env(db_password), ) ) diff --git a/instance_manager/workflows/purge.py b/instance_manager/workflows/purge.py index 731108c..60597aa 100644 --- a/instance_manager/workflows/purge.py +++ b/instance_manager/workflows/purge.py @@ -19,6 +19,7 @@ Command, list_instances, path_exists, + pg_env, read_odoo_conf, run, service_exists, @@ -68,10 +69,10 @@ def _resolve_db_admin_access() -> DbAdminSession | None: db_admin_password = ask_secret('DB admin password') probe_cmd = ( - f"PGPASSWORD={_quote(db_admin_password)} psql -h {_quote(db_host)} -p {db_port} " - f"-U {_quote(db_admin_user)} -d postgres -tAc \"SELECT 1;\"" + f"psql -X -h {_quote(db_host)} -p {int(db_port)} " + f"-U {_quote(db_admin_user)} -d postgres -tAc 'SELECT 1;'" ) - probe = run(probe_cmd, check=False) + probe = run(probe_cmd, check=False, env=pg_env(db_admin_password)) if probe.returncode == 0 and "1" in probe.stdout: print(t('[OK] Remote admin connection validated.')) return DbAdminSession("remote", db_host, db_port, db_admin_user, db_admin_password) @@ -90,12 +91,17 @@ def _db_admin_psql_command(session: DbAdminSession, sql: str, psql_flags: str = return f"sudo -u postgres psql -v ON_ERROR_STOP=1 -d postgres {flags}-c {shlex.quote(sql)}" return ( - f"PGPASSWORD={_quote(session.admin_password)} psql -v ON_ERROR_STOP=1 " - f"-h {_quote(session.db_host)} -p {session.db_port} " + f"psql -X -v ON_ERROR_STOP=1 " + f"-h {_quote(session.db_host)} -p {int(session.db_port)} " f"-U {_quote(session.admin_user)} -d postgres {flags}-c {shlex.quote(sql)}" ) +def _admin_env(session: DbAdminSession) -> dict[str, str]: + """The admin password for a remote session, in the environment only.""" + return pg_env(session.admin_password) if session.mode == "remote" else {} + + def _db_admin_dropdb_command(session: DbAdminSession, db_name: str) -> str: if session.mode == "local": # --force (PostgreSQL 13+) closes the sessions and refuses new ones in the @@ -103,8 +109,8 @@ def _db_admin_dropdb_command(session: DbAdminSession, db_name: str) -> str: return f"sudo -u postgres dropdb --if-exists --force {_quote(db_name)}" return ( - f"PGPASSWORD={_quote(session.admin_password)} dropdb --if-exists " - f"-h {_quote(session.db_host)} -p {session.db_port} " + "dropdb --if-exists " + f"-h {_quote(session.db_host)} -p {int(session.db_port)} " f"-U {_quote(session.admin_user)} {_quote(db_name)}" ) @@ -160,7 +166,7 @@ def _prefix_only_databases_sql(instance: str, db_user: str) -> str: def _query_names(session: DbAdminSession, sql: str) -> tuple[list[str], str | None]: query_cmd = _db_admin_psql_command(session, sql, psql_flags="-tA") - result = run(query_cmd, check=False) + result = run(query_cmd, check=False, env=_admin_env(session)) if result.returncode != 0: error_text = result.stderr.strip() or result.stdout.strip() or "Unknown error" return [], error_text @@ -326,12 +332,14 @@ def purge_instance_superuser() -> None: Command( tf('Close active connections of DB {}', db_name), _db_admin_psql_command(session, terminate_sql) + " || true", + env=_admin_env(session), ) ) commands.append( Command( tf('Delete DB {}', db_name), _db_admin_dropdb_command(session, db_name), + env=_admin_env(session), ) ) @@ -349,6 +357,7 @@ def purge_instance_superuser() -> None: Command( tf('Delete PostgreSQL role {} (if it exists)', role), _db_admin_psql_command(session, drop_role_sql) + " || true", + env=_admin_env(session), ) ) diff --git a/instance_manager/workflows/report.py b/instance_manager/workflows/report.py index b211254..f01a67f 100644 --- a/instance_manager/workflows/report.py +++ b/instance_manager/workflows/report.py @@ -466,7 +466,7 @@ def _list_local_postgres_databases(instance: str, db_user: str) -> list[str]: "FROM pg_database d " "JOIN pg_roles r ON d.datdba = r.oid " "WHERE d.datistemplate = false " - f"AND (d.datname LIKE '{instance_literal}%' OR r.rolname = '{db_user_literal}') " + f"AND (d.datname = '{instance_literal}' OR r.rolname = '{db_user_literal}') " "ORDER BY d.datname;" ) result = run( @@ -1092,9 +1092,12 @@ def external_server_report() -> None: default_path = f"./reports/{host}_{now}.txt" export_path = ask_text('Report export path', default_path, required=True) - export_dir = os.path.dirname(export_path) or "." - os.makedirs(export_dir, exist_ok=True) - with open(export_path, "w", encoding="utf-8") as file_handle: - file_handle.write("\n\n".join(report_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("\n\n".join(report_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('Report exported to: {}', export_path))) diff --git a/openspec/changes/secrets-out-of-sight/proposal.md b/openspec/changes/secrets-out-of-sight/proposal.md new file mode 100644 index 0000000..81ca9a6 --- /dev/null +++ b/openspec/changes/secrets-out-of-sight/proposal.md @@ -0,0 +1,28 @@ +# Keep secrets out of sight + +## Why + +Every database password the tool used was written into the command text (`PGPASSWORD=… psql …`), and the +role password into `psql -c "CREATE ROLE … PASSWORD '…'"`. A plan step runs as `bash -lc ""`, so the +password sat in that process's arguments, which any local user — another instance's Odoo user included — reads +with `ps` for as long as the step runs (a long `pg_dump` or `pg_restore`). The same text was printed in the plan +preview (with `odoo.conf`'s `admin_passwd` and `db_password` in its heredoc) and again in the error of a failed +step. A remote server that dropped packets also held a probe, and the menu, until TCP gave up. + +## What changes + +- A plan step carries its secrets in `Command.env` (the step's environment, readable only by root) and what the + preview shows in `Command.display` (secrets masked). Every libpq client takes `PGPASSWORD` from the + environment, with `PGCONNECT_TIMEOUT=10`; SQL holding a password is fed to psql on stdin. +- Files are written atomically (private temporary file, chmod, rename) with their content in the environment; + the preview shows the content with secrets masked. +- A failed step's error names the step, not its command. +- Smaller fixes: the addon dependency check asks by distribution name first, as Odoo does (`python-stdnum` + was reported missing); the health check reaches a Unix-socket database (`db_host = False`); a failed export + no longer crashes the CLI; the server report's database list no longer matches by name prefix. + +## Impact + +- Specs: `execution-safety`, `addon-inventory`. +- Code: `system.py`, `planners.py`, `workflows/{addons,backup_restore,common,health,manage,purge,report}.py`. +- Tests updated; `tools/verify_secrets.py` runs the steps and looks at `ps`. diff --git a/openspec/changes/secrets-out-of-sight/specs/addon-inventory/spec.md b/openspec/changes/secrets-out-of-sight/specs/addon-inventory/spec.md new file mode 100644 index 0000000..f21a318 --- /dev/null +++ b/openspec/changes/secrets-out-of-sight/specs/addon-inventory/spec.md @@ -0,0 +1,13 @@ +## ADDED Requirements + +### Requirement: Python dependencies checked the way Odoo checks them + +The addon inventory SHALL report a declared Python dependency as present when the instance venv has a +distribution of that name, or else can import a module of that name — Odoo's own order +(`check_python_external_dependency`) — so a dependency declared by its distribution name (`python-stdnum`, +`pdfminer.six`) is not reported missing. + +#### Scenario: A distribution name is found + +- **WHEN** an addon declares `python-stdnum` and the venv has that distribution +- **THEN** the inventory reports it present diff --git a/openspec/changes/secrets-out-of-sight/specs/execution-safety/spec.md b/openspec/changes/secrets-out-of-sight/specs/execution-safety/spec.md new file mode 100644 index 0000000..7cd67da --- /dev/null +++ b/openspec/changes/secrets-out-of-sight/specs/execution-safety/spec.md @@ -0,0 +1,31 @@ +## ADDED Requirements + +### Requirement: Secrets out of sight + +A secret — a database password, the master password, a file holding them — SHALL never be part of a plan step's +command text: it SHALL travel in the step's environment (which only root can read), a libpq client SHALL take +its password from `PGPASSWORD` there, and SQL holding a password SHALL be fed to `psql` on stdin. The plan +preview SHALL show such a step with its secrets masked, and a failed step's error SHALL name the step rather than +print its command. Database probes SHALL give up connecting after 10 seconds (`PGCONNECT_TIMEOUT`). + +#### Scenario: A password is not visible to other users while a step runs + +- **WHEN** a step connects to PostgreSQL with a password +- **THEN** the password is in the step's environment and in no process's arguments + +#### Scenario: The preview masks secrets + +- **WHEN** the plan writes `odoo.conf` or sets a role's password +- **THEN** the preview shows the file or statement with `admin_passwd`, `db_password` and the role password as + `********` + +#### Scenario: Files are written atomically + +- **WHEN** a plan writes a file +- **THEN** it writes a private temporary file in the same directory, sets the mode, and renames it over the + target, so the file is never seen half-written or with a wider mode + +#### Scenario: A failure does not print the command + +- **WHEN** a step fails +- **THEN** the error names the step's description diff --git a/openspec/changes/secrets-out-of-sight/tasks.md b/openspec/changes/secrets-out-of-sight/tasks.md new file mode 100644 index 0000000..f0325f0 --- /dev/null +++ b/openspec/changes/secrets-out-of-sight/tasks.md @@ -0,0 +1,7 @@ +# Tasks + +- [x] 1 Spec deltas; `--strict` passes. +- [x] 2 `Command.env`/`display`, `pg_env`, masked preview, error by description. +- [x] 3 Every `PGPASSWORD=` and password SQL moved to the environment / stdin; atomic file writes. +- [x] 4 Dependency check, health socket, export errors, report listing. +- [x] 5 Tests, `tools/verify_secrets.py`, docs, changelog. diff --git a/tests/test_db_existence.py b/tests/test_db_existence.py index cad2d5d..a5028c6 100644 --- a/tests/test_db_existence.py +++ b/tests/test_db_existence.py @@ -17,7 +17,7 @@ def __init__(self, returncode: int, stdout: str) -> None: self.returncode = returncode self.stdout = stdout - def __call__(self, command: str, check: bool = False): + def __call__(self, command: str, check: bool = False, env=None): self.command = command return subprocess.CompletedProcess(command, self.returncode, self.stdout, "") diff --git a/tests/test_install_per_version.py b/tests/test_install_per_version.py index 35312f2..9f8acff 100644 --- a/tests/test_install_per_version.py +++ b/tests/test_install_per_version.py @@ -2,7 +2,6 @@ from __future__ import annotations -import shlex import unittest from instance_manager import support @@ -187,7 +186,11 @@ def test_update_reinstalls_nothing_and_restarts(self) -> None: text = "\n".join(c.command for c in commands) for absent in ("apt-get", "git clone", "pip install"): self.assertNotIn(absent, text) - self.assertIn(shlex.quote("ALTER ROLE shop WITH PASSWORD 'pw';"), text) + alter = next(c for c in commands if "OIM_SQL" in c.command) + self.assertEqual(alter.env["OIM_SQL"], "ALTER ROLE shop WITH PASSWORD 'pw';") + # The password is in no command text and the preview masks it. + self.assertNotIn("'pw'", text) + self.assertNotIn("'pw'", "\n".join(c.shown for c in commands)) self.assertIn("systemctl restart shop", text) def test_no_password_change_no_role_change(self) -> None: diff --git a/tests/test_planners.py b/tests/test_planners.py index 44ed90a..bbf3d60 100644 --- a/tests/test_planners.py +++ b/tests/test_planners.py @@ -51,7 +51,7 @@ def _plan_text(self, **kw: object) -> str: ) defaults.update(kw) cmds = plan_scheduled_backup(_config(), **defaults) # type: ignore[arg-type] - return "\n".join(f"{c.description}\n{c.command}" for c in cmds) + return "\n".join(f"{c.description}\n{c.shown}" for c in cmds) def test_writes_script_service_timer_and_enables(self) -> None: text = self._plan_text() @@ -185,7 +185,7 @@ def test_uses_create_and_postrotate_reopen_not_copytruncate(self) -> None: class PlanLogrotateConfigTests(unittest.TestCase): def _commands_text(self, **kwargs: object) -> str: commands = plan_logrotate_config(_config(), **kwargs) # type: ignore[arg-type] - return "\n".join(f"{c.description} :: {c.command}" for c in commands) + return "\n".join(f"{c.description} :: {c.shown}" for c in commands) def test_default_plan_writes_validates_and_installs(self) -> None: text = self._commands_text() diff --git a/tests/test_system.py b/tests/test_system.py index cb87436..c5a945b 100644 --- a/tests/test_system.py +++ b/tests/test_system.py @@ -55,7 +55,7 @@ class ListDatabasesOwnerScopeTests(unittest.TestCase): def _captured_query(self, **kwargs: object) -> str: captured: dict[str, str] = {} - def fake_run(cmd: str, check: bool = False) -> subprocess.CompletedProcess: + def fake_run(cmd: str, check: bool = False, env=None) -> subprocess.CompletedProcess: captured["cmd"] = cmd return subprocess.CompletedProcess(cmd, 0, "db1\n", "") @@ -64,7 +64,7 @@ def fake_run(cmd: str, check: bool = False) -> subprocess.CompletedProcess: return captured["cmd"] def test_owner_scopes_by_role_and_exact_name(self) -> None: - cmd = self._captured_query(owner="shop") + cmd = shlex.split(self._captured_query(owner="shop"))[-1] self.assertIn("r.rolname = 'shop'", cmd) self.assertIn("d.datname = 'shop'", cmd) # No name prefix: instance `shop` must not list `shop2` or `shop_eu`. @@ -86,7 +86,7 @@ class ProbeQuotingTests(unittest.TestCase): def _captured(self, probe, value: str) -> str: captured: dict[str, str] = {} - def fake_run(cmd: str, check: bool = False) -> subprocess.CompletedProcess: + def fake_run(cmd: str, check: bool = False, env=None) -> subprocess.CompletedProcess: captured["cmd"] = cmd return subprocess.CompletedProcess(cmd, 1, "", "") diff --git a/tests/test_workflow_helpers.py b/tests/test_workflow_helpers.py index 2a74fd8..915ca03 100644 --- a/tests/test_workflow_helpers.py +++ b/tests/test_workflow_helpers.py @@ -8,6 +8,7 @@ from instance_manager.workflows.common import _keep_data_dir_commands from instance_manager.workflows.purge import ( DbAdminSession, + _admin_env, _db_admin_psql_command, _instance_databases_sql, _prefix_only_databases_sql, @@ -66,9 +67,10 @@ def test_flags_inserted_before_c_remote_with_dash_c_in_password(self) -> None: # containing "-c"; the flags path must leave PGPASSWORD intact. cmd = _db_admin_psql_command(self.remote, "SELECT 1;", psql_flags="-tA") self.assertIn("-tA -c", cmd) - # Password with a "-c" substring must survive intact (the old - # .replace("-c", "-tA -c", 1) would have mangled it to "se-tA -cret"). - self.assertIn("PGPASSWORD=se-cret", cmd) + # The password is never in the command text: it goes in the environment. + self.assertNotIn("se-cret", cmd) + self.assertEqual(_admin_env(self.remote)["PGPASSWORD"], "se-cret") + self.assertEqual(_admin_env(self.local), {}) self.assertNotIn("se-tA", cmd) def test_no_flags_has_no_tuples_output(self) -> None: diff --git a/tools/verify_secrets.py b/tools/verify_secrets.py new file mode 100644 index 0000000..b4e7800 --- /dev/null +++ b/tools/verify_secrets.py @@ -0,0 +1,135 @@ +#!/usr/bin/env python3 +"""Check, by running them, that plan steps keep secrets out of sight. + +- a step's password, passed through ``Command.env``, is not in any process's + arguments while it runs (``ps`` shows every user's), while the old form — + ``PGPASSWORD=…`` in the command text — is (negative control); +- ``write_text_file_command`` writes the exact content with the asked mode, leaves + no temporary file, and its preview masks the secrets; +- a failed step's error names the step, not its command; +- the role SQL sent on stdin creates a role with that password, against a + PostgreSQL cluster of our own (skipped without ``initdb`` or as root). + + python3 tools/verify_secrets.py +""" + +from __future__ import annotations + +import os +import stat +import subprocess +import sys +import tempfile +import threading +import time +from pathlib import Path + +HERE = Path(__file__).resolve().parent +sys.path.insert(0, str(HERE.parent)) +sys.path.insert(0, str(HERE)) + +from verify_data_safety import Cluster, _postgres_bindir, _write_exe # noqa: E402 + +from instance_manager import planners, system # noqa: E402 +from instance_manager.models import InstanceConfig # noqa: E402 + +FAILURES: list[str] = [] +SECRET = "S3cr3t-oim-check-7f1c" + + +def check(name: str, ok: bool, detail: str = "") -> None: + print(f"{'ok ' if ok else 'FAIL'} {name}" + (f" — {detail[-400:]}" if detail and not ok else ""), flush=True) + if not ok: + FAILURES.append(name) + + +def _seen_in_ps(start, needle: str) -> bool: + """Start something, then look for ``needle`` in every process's arguments.""" + seen = [] + thread = threading.Thread(target=start) + thread.start() + for _ in range(20): + time.sleep(0.1) + seen.append(needle in subprocess.run(["ps", "-eo", "args"], capture_output=True, text=True).stdout) + thread.join() + return any(seen) + + +def _argv() -> None: + step = system.Command("wait", 'sleep 1.5; test -n "$PGPASSWORD"', env=system.pg_env(SECRET)) + check("a password in the step's environment is not in any process's arguments", + not _seen_in_ps(lambda: system.run(step.command, env=step.env), SECRET)) + old = f"PGPASSWORD={SECRET} sleep 1.5 && true" + check("negative control: PGPASSWORD in the command text is visible in ps", + _seen_in_ps(lambda: system.run(old), SECRET)) + result = system.run(step.command, env=step.env) + check("the step still receives the password", result.returncode == 0, result.stderr) + + +def _files(root: Path) -> None: + config = InstanceConfig(instance="shop") + config.db_password, config.odoo_admin_passwd = SECRET, SECRET + "-master" + content = planners._odoo_conf_content(config) + target = root / "shop.conf" + [command] = planners.write_text_file_command(str(target), content, "640", + secrets=(config.db_password, config.odoo_admin_passwd)) + check("the file content is not in the command text", SECRET not in command.command) + check("the preview shows the file with its secrets masked", + SECRET not in command.shown and "db_password = ********" in command.shown, command.shown) + result = system.run(command.command, env=command.env) + check("the file is written exactly", result.returncode == 0 and target.read_text() == content, result.stderr) + check("with the asked mode", stat.S_IMODE(target.stat().st_mode) == 0o640, oct(target.stat().st_mode)) + check("leaving no temporary file", [p.name for p in root.iterdir()] == ["shop.conf"], + str(list(root.iterdir()))) + + +def _errors() -> None: + step = system.Command("Do the secret thing", f"false {SECRET}", display="false ********") + try: + system.apply_commands([step]) + message = "" + except RuntimeError as error: + message = str(error) + check("a failed step's error names the step, not its command", + "Do the secret thing" in message and SECRET not in message, message) + + +def _role(root: Path) -> None: + bindir = _postgres_bindir() + if bindir is None or os.geteuid() == 0: + print("skip the role check needs initdb and a non-root user") + return + stubs = root / "stubs" + stubs.mkdir() + _write_exe(stubs / "sudo", 'if [ "$1" = "-u" ]; then shift 2; fi\nexec "$@"\n') + cluster = Cluster(root / "pg", bindir) + (root / "pg").mkdir() + try: + cluster.start() + config = InstanceConfig(instance="shop") + config.db_password, config.odoo_admin_passwd = SECRET, "x" + step = next(c for c in planners.plan_ensure_db_role(config) if "OIM_SQL" in c.command) + check("the role SQL is not in the command text", SECRET not in step.command and SECRET not in step.shown) + result = subprocess.run(["bash", "-c", step.command], capture_output=True, text=True, + env={**cluster.env(stubs), **step.env}) + check("the role is created through stdin", result.returncode == 0, result.stderr) + check("with a password set", + cluster.value("SELECT rolpassword IS NOT NULL FROM pg_authid WHERE rolname = 'shop'") == "t") + finally: + cluster.stop() + + +def main() -> int: + with tempfile.TemporaryDirectory(prefix="oim-verify-secrets-") as tmp: + root = Path(tmp) + (root / "files").mkdir() + _argv() + _files(root / "files") + _errors() + _role(root) + print(f"\n{len(FAILURES)} failure(s)" if FAILURES else "\nall checks passed") + return 1 if FAILURES else 0 + + +if __name__ == "__main__": + sys.exit(main())