From 1820167d687f596aab5b9228ed3c8f93965b735a Mon Sep 17 00:00:00 2001 From: Guillermo Rojo Date: Thu, 8 Oct 2026 15:01:40 +0200 Subject: [PATCH] fix: copies done whole and neutralised as Odoo does - Each copy (refresh, replica, duplicate database, local restore) is one staged step: seed, identity, Odoo's own neutralize.sql, the catalogue, the check and the hand-over; a failure drops only the database the step created. A staging comment marks it until the hand-over, and a leftover of an interrupted run is replaced instead of refused. - The role gets CONNECT only at the hand-over (an Odoo with db_name set connects by name); CREATE on public is taken from PUBLIC. - A local restore goes as a duplication (no service stop); a remote one creates, restores and neutralises in one step that drops the database and restarts the service on failure; --no-comments; the service starts after the files. - On 16.0-19.0 every installed module's data/neutralize.sql runs first, read from the checkout as the instance user, run as the owner. - Catalogue, checked against Odoo 14.0-19.0 and OCA l10n-spain: delete rules, optional labels, Spanish EDI per version, VERI*FACTU and TicketBAI (Odoo and OCA), EDI proxy (14-16 parameter, MY/GR), web push, cloud storage, certificates, SMS, calendars per table, IAP long tokens, mail host pointed nowhere, a login sink, cli and EDI checks, citations. --- CHANGELOG.md | 20 ++ docs/operations/instance-management.md | 49 ++- instance_manager/i18n.py | 7 + instance_manager/neutralise.py | 211 ++++++++--- instance_manager/system.py | 13 + instance_manager/workflows/backup_restore.py | 329 +++++++++++++----- instance_manager/workflows/common.py | 17 + .../2026-10-08-copies-done-whole/proposal.md | 52 +++ .../specs/data-backup-restore/spec.md | 255 ++++++++++++++ .../2026-10-08-copies-done-whole/tasks.md | 11 + openspec/specs/data-backup-restore/spec.md | 61 +++- tests/test_backup_restore.py | 8 +- tests/test_copies_neutralise.py | 115 ++++++ tests/test_neutralise.py | 9 +- tools/verify_neutralisation.py | 31 +- tools/verify_privilege.py | 41 +++ 16 files changed, 1066 insertions(+), 163 deletions(-) create mode 100644 openspec/changes/archive/2026-10-08-copies-done-whole/proposal.md create mode 100644 openspec/changes/archive/2026-10-08-copies-done-whole/specs/data-backup-restore/spec.md create mode 100644 openspec/changes/archive/2026-10-08-copies-done-whole/tasks.md create mode 100644 tests/test_copies_neutralise.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ebd8d3e..f5979e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,26 @@ All notable changes to this project are documented here. The format is based on ### Fixed +- **A neutralised copy could still reach the outside.** + - Several rules named columns some versions lack, so they were skipped silently and left those copies in + production: + - SII and TicketBAI on 14–17; + - the EDI proxy on 14–16; + - Google Calendar on 15–17; + - IAP on 18–19. + - Other integrations were not covered at all: VERI\*FACTU, web push, cloud storage, certificates, SMS, and the + Malaysian and Greek EDI. + - A mail sink cloned from a `cli` server still sent through `odoo.conf`. + + On 16–19 a copy now first runs Odoo's own `neutralize.sql` for every installed module. Then the tool's rules, + now checked against each version's sources, cover 12–19 and the OCA modules. +- **A failed restore or duplication left a half-made copy.** It could be armed and visible to the instance's + cron worker, the service stayed stopped, and every retry was refused. Each copy is now one step: done whole, or + its database is dropped and the service started again. A local restore is now done like a duplication, without + stopping the service. An interrupted copy is recognised and replaced. +- The copy's role can connect to it only once it is handed over: an Odoo with `db_name` set could reach the copy + before it was neutralised. + - **Root shell and superuser exposure.** - The certificate paths and the venv package names reached a root shell unescaped, so a file or package named `$(…)` ran as root. diff --git a/docs/operations/instance-management.md b/docs/operations/instance-management.md index 37ce41e..44d2044 100644 --- a/docs/operations/instance-management.md +++ b/docs/operations/instance-management.md @@ -100,29 +100,40 @@ for a refresh in place): - **Copied (new UUID on target)** — the target gets its own `database.uuid`, `database.secret` and creation date, as Odoo's own copy does. - **Moved (keep UUID)** — the target keeps its identity: the database "moves". -- **Neutralize** (recommended) — the copy cannot act as production. It follows Odoo's own `neutralize.sql` - (16.0–19.0) on every version from 12 to 19, plus the OCA modules a Spanish or queue-based instance runs: +- **Neutralize** (recommended) — the copy cannot act as production. On 16.0–19.0 it first runs Odoo's own + neutralisation: the `neutralize.sql` of every module installed in the copy, read from the instance's + checkout, including payment terminals and foreign EDIs. Then, on every version from 12 to 19, it applies the + tool's own rules, which follow Odoo's and add the OCA modules a Spanish or queue-based instance runs: - crons off, except Odoo's autovacuum and queue_job's cleanup; queued jobs held; - - outgoing mail servers off with their credentials dropped, fetchmail off, templates' fixed servers cleared, - and one active **mail sink** (`invalid:1025`) so Odoo never falls back to the `smtp_server` of `odoo.conf`; + - mail: + - outgoing mail servers are switched off, their credentials dropped and their host pointed nowhere; + - fetchmail is off and templates' fixed servers are cleared; + - one active **mail sink** (`invalid:1025`, logging in) stays, so Odoo never falls back to the + `smtp_server` of `odoo.conf`; - payment providers, external carriers and their production mode, OAuth providers; - - Google and Microsoft calendar tokens, webhooks, IAP accounts; - - SII, TicketBAI, EDI proxy and Peppol in test mode; + - Google and Microsoft calendar tokens, webhooks, IAP accounts, web push keys and devices, cloud storage, + certificate and SMS passwords; + - test mode for the tax and invoicing links: + - Spain: SII, TicketBAI and VERI*FACTU, both Odoo's and OCA's; + - the EDI proxy, Peppol, and the Malaysian and Greek EDI; - website domain and CDN cleared, `web.base.url` pointing at the target, the "neutralised" banner and flag. - The rules, each with its source, are in `instance_manager/neutralise.py`. They run as the copy's owner role, - not as the PostgreSQL superuser, so code the source database carries (a trigger, a function) gets no more - rights than its own role has. + The rules, each with its source, are in `instance_manager/neutralise.py`. Everything runs as the copy's owner + role, not as the PostgreSQL superuser, so code the source database carries (a trigger, a function) gets no + more rights than its own role has. A copy is never visible to a running Odoo before it is neutralised. An Odoo's cron worker lists every database -its role owns, whatever the `dbfilter`, so a duplication goes this way: +its role owns, whatever the `dbfilter`, and one with `db_name` set connects to it by name. So the copy is owned +by postgres, and closed to every role, until it is handed over. The whole copy is **one step**: if any part +fails, the database it created is dropped, and nothing half-made is left behind. ```mermaid flowchart LR seed[("Copy the database
owned by postgres")] --> identity["Copied mode:
new identity"] - identity --> neutralise["Neutralise
one statement"] + identity --> odoo["Odoo's own
neutralize.sql"] + odoo --> neutralise["The tool's rules
one statement"] neutralise --> check{"Anything left
that acts outside?"} - check -- yes --> stop(["Stop the plan"]) + check -- yes --> stop(["Drop the copy,
stop the plan"]) check -- no --> handover["Hand the database
to the target role"] handover --> start["Start the
target service"] classDef step fill:#dbeafe,stroke:#2563eb,color:#1e3a8a @@ -132,16 +143,22 @@ flowchart LR classDef data fill:#ede9fe,stroke:#7c3aed,color:#4c1d95 class identity,start step class check ask - class neutralise,handover guard + class odoo,neutralise,handover guard class stop stop class seed data ``` -A **restore** creates the database with the credentials you give, so it stops the instance's service while the -copy is restored and neutralised, and starts it again afterwards. +A **restore** into a local database goes the same way, restored as the instance's role; the service keeps +running. Into a remote database it uses the credentials you give: the service is stopped while the copy is +restored and neutralised, and started again afterwards — also if the restore fails, which drops the database it +created. A module update switches crons back on. **Check a copy is neutralised** (in *Status & health*) lists, read-only, -what in a database can still act on the outside, rule by rule, and whether the mail sink is in place. +what in a database can still act on the outside, rule by rule, whether the mail sink is in place, and any +`cli` mail server. + +A copy an interrupted run left behind (owned by postgres, marked as unfinished) is recognised and replaced by +the next restore or duplication; any other database is never taken for one. Restore refuses to overwrite an existing target **database**. An existing target **filestore** needs an explicit overwrite, and is then moved aside to `.replaced-`, not deleted; the restored files are diff --git a/instance_manager/i18n.py b/instance_manager/i18n.py index 21afd5a..93dd33a 100644 --- a/instance_manager/i18n.py +++ b/instance_manager/i18n.py @@ -974,4 +974,11 @@ def tf(template: str, *args: object, **kwargs: object) -> str: 'Control characters are not allowed.': 'No se admiten caracteres de control.', 'Ignored (not a package): {}': 'Ignorado (no es un paquete): {}', 'Invalid service name. Use letters, digits and @ . _ : -': 'Nombre de servicio no válido. Usa letras, dígitos y @ . _ : -', + "Run Odoo's own neutralize.sql of every installed module (16.0-19.0)": 'Ejecutar el neutralize.sql de Odoo de cada módulo instalado (16.0-19.0)', + 'Close the copy to every role until it is handed over (revoke PUBLIC connect)': 'Cerrar la copia a todos los roles hasta el traspaso (revocar CONNECT a PUBLIC)', + 'Restore {} into {}, neutralise it and hand it over (undone if a step fails)': 'Restaurar {} en {}, neutralizarla y traspasarla (se deshace si falla un paso)', + 'Copy {} into {}, neutralise it and hand it over (undone if a step fails)': 'Copiar {} en {}, neutralizarla y traspasarla (se deshace si falla un paso)', + 'Restore the dump as role {}': 'Restaurar el dump como rol {}', + "'cli' servers send through odoo.conf's smtp_server": "los servidores 'cli' envían por el smtp_server de odoo.conf", + 'The copy failed: its unfinished database {} is dropped.': 'La copia ha fallado: se elimina su base de datos a medias {}.', } diff --git a/instance_manager/neutralise.py b/instance_manager/neutralise.py index 01f6276..74f9c3a 100644 --- a/instance_manager/neutralise.py +++ b/instance_manager/neutralise.py @@ -6,11 +6,14 @@ tax integration in production mode is one click from a real transaction, and the copy calls webhooks, calendars and IAP services as production. -Odoo's own ``neutralize`` exists only from 16.0 and knows nothing of OCA -modules. These rules follow it (``data/neutralize.sql`` of each module, cited -per rule) and cover 12.0-19.0 and the OCA modules a Spanish or queue-based -instance runs: each rule is guarded by the existence of the table and every -column it touches, so one catalogue serves every version. It is one-way, like +Odoo's own ``neutralize`` exists only from 16.0 and knows nothing of older +versions. A copy of 16.0-19.0 first runs it — every installed module's +``data/neutralize.sql``, read from the instance's checkout — and then these +rules, which also serve 12.0-15.0: they follow Odoo's SQL (cited per rule) and +add the OCA modules a Spanish or queue-based instance runs (SII, VERI*FACTU, +TicketBAI, queue_job) and what Odoo leaves (the mail host, ``cli`` servers). +Each rule is guarded by the existence of its table and every column it touches, +so one catalogue serves every version; the check after it uses the same rules. It is one-way, like Odoo's: a copy is not given back to production, and recording the values it replaces would keep production's mail passwords inside the copy. @@ -21,7 +24,7 @@ from __future__ import annotations -from dataclasses import dataclass +from dataclasses import dataclass, replace _ODOO = "odoo {} {}/data/neutralize.sql" @@ -52,22 +55,24 @@ class Keep: @dataclass(frozen=True) class Rule: - """Set ``sets`` on the rows of ``table`` (alias ``t``) matching ``where``. + """Set ``sets`` on the rows of ``table`` (alias ``t``) matching ``where`` — or, + with ``delete``, delete them (a secret or a queued call has no harmless value). - A rule applies only where its table and every column it names exist.""" + A rule applies only where its table and every column it names exist. ``label`` + only names rows in the check; a table without it is still neutralised.""" id: str source: str table: str where: str - sets: tuple[tuple[str, str], ...] + sets: tuple[tuple[str, str], ...] = () requires: tuple[str, ...] = () label: str = "" + delete: bool = False @property def columns(self) -> tuple[str, ...]: - return tuple(dict.fromkeys(("id", *self.requires, *(c for c, _ in self.sets), - *((self.label,) if self.label else ())))) + return tuple(dict.fromkeys(("id", *self.requires, *(c for c, _ in self.sets)))) def _keep_ids() -> str: @@ -80,39 +85,99 @@ def _param(key: str) -> str: return f"t.key = '{key}'" +def _params(*keys: str) -> str: + return "t.key IN (" + ", ".join(f"'{k}'" for k in keys) + ")" + + +_OCA_ES = "OCA/l10n-spain {} {}" + CATALOGUE: tuple[Rule, ...] = ( Rule("crons", _ODOO.format("16.0-19.0", "odoo/addons/base"), "ir_cron", f"t.active AND {_keep_ids()}", (("active", "false"),), label="cron_name"), + # 12.0-15.0 still send through an archived server a message or template names, + # so the host goes nowhere too; 17.0-19.0 drop the credentials as Odoo does. Rule("mail-servers", _ODOO.format("16.0-19.0", "odoo/addons/base") - + " (deactivated, credentials dropped)", - "ir_mail_server", f"t.name <> '{MAIL_SINK_NAME}' AND (t.active OR t.smtp_user IS NOT NULL " - "OR t.smtp_pass IS NOT NULL)", - (("active", "false"), ("smtp_user", "NULL"), ("smtp_pass", "NULL")), label="name"), - Rule("fetchmail", "odoo 12.0-19.0 addons/fetchmail (mail/models/fetchmail.py from 17.0)", + + " (deactivated; credentials dropped as 17.0-19.0 do; host pointed nowhere)", + "ir_mail_server", f"t.name <> '{MAIL_SINK_NAME}'", + (("active", "false"), ("smtp_user", "NULL"), ("smtp_pass", "NULL"), + ("smtp_host", f"'{MAIL_SINK_HOST}'")), label="name"), + Rule("fetchmail", "odoo 12.0-19.0 addons/fetchmail (in addons/mail from 16.0; " + "neutralize.sql 16.0-19.0)", "fetchmail_server", "t.active", (("active", "false"),), label="name"), Rule("mail-template-server", _ODOO.format("16.0-19.0", "addons/mail"), "mail_template", "t.mail_server_id IS NOT NULL", (("mail_server_id", "NULL"),)), + Rule("web-push-keys", _ODOO.format("17.0-19.0", "addons/mail"), + "ir_config_parameter", _params("mail.web_push_vapid_private_key", + "mail.web_push_vapid_public_key", "mail.sfu_server_key"), + requires=("key",), label="key", delete=True), + Rule("web-push-devices", "odoo 17.0 addons/mail/data/neutralize.sql (mail_partner_device)", + "mail_partner_device", "true", delete=True), + Rule("web-push-queued", "odoo 17.0 addons/mail/data/neutralize.sql (mail_notification_web_push)", + "mail_notification_web_push", "true", delete=True), + Rule("push-devices", _ODOO.format("18.0-19.0", "addons/mail") + " (mail_push_device)", + "mail_push_device", "true", delete=True), + Rule("push-queued", _ODOO.format("18.0-19.0", "addons/mail") + " (mail_push)", + "mail_push", "true", delete=True), Rule("queued-jobs", "OCA/queue queue_job/job.py (pending, enqueued and started are what " "the job runner executes; held as failed, which it never picks up)", "queue_job", "t.state IN ('pending', 'enqueued', 'started', 'wait_dependencies')", (("state", "'failed'"),), label="name"), - Rule("sii-oca", "OCA/l10n-spain l10n_es_aeat_sii(_oca)/models/res_company.py", + Rule("sii-oca", _OCA_ES.format("12.0-18.0", "l10n_es_aeat_sii(_oca)/models/res_company.py"), "res_company", "t.sii_enabled OR NOT coalesce(t.sii_test, false)", (("sii_enabled", "false"), ("sii_test", "true")), label="name"), - Rule("sii-odoo", _ODOO.format("16.0-19.0", "addons/l10n_es_edi_sii"), + Rule("spain-edi-odoo", "odoo 14.0-17.0 addons/l10n_es_edi_sii (neutralize.sql 16.0-17.0; " + "l10n_es_edi_tbai 16.0-17.0 shares the field)", + "res_company", "NOT coalesce(t.l10n_es_edi_test_env, false)", + (("l10n_es_edi_test_env", "true"),), label="name"), + Rule("sii-odoo", _ODOO.format("18.0-19.0", "addons/l10n_es_edi_sii"), "res_company", "NOT coalesce(t.l10n_es_sii_test_env, false)", (("l10n_es_sii_test_env", "true"),), label="name"), - Rule("ticketbai", _ODOO.format("17.0-19.0", "addons/l10n_es_edi_tbai"), + Rule("ticketbai", _ODOO.format("18.0-19.0", "addons/l10n_es_edi_tbai"), "res_company", "NOT coalesce(t.l10n_es_tbai_test_env, false)", (("l10n_es_tbai_test_env", "true"),), label="name"), - Rule("edi-proxy", _ODOO.format("16.0-19.0", "addons/account_edi_proxy_client"), + Rule("verifactu-odoo", _ODOO.format("17.0-19.0", "addons/l10n_es_edi_verifactu"), + "res_company", "NOT coalesce(t.l10n_es_edi_verifactu_test_environment, false)", + (("l10n_es_edi_verifactu_test_environment", "true"),), label="name"), + Rule("verifactu-oca", _OCA_ES.format("14.0-18.0", "l10n_es_verifactu_oca/data/neutralize.sql"), + "res_company", "NOT coalesce(t.verifactu_test, false)", + (("verifactu_test", "true"),), label="name"), + Rule("ticketbai-oca", _OCA_ES.format("13.0-16.0", "l10n_es_ticketbai_api/models/res_company.py"), + "res_company", "t.tbai_enabled AND NOT coalesce(t.tbai_test_enabled, false)", + (("tbai_test_enabled", "true"),), requires=("tbai_enabled",), label="name"), + Rule("edi-proxy", _ODOO.format("17.0-19.0", "addons/account_edi_proxy_client"), "account_edi_proxy_client_user", "t.edi_mode = 'prod' AND t.proxy_type NOT IN ('l10n_my_edi', 'l10n_gr_edi')", (("edi_mode", "CASE WHEN t.proxy_type IN ('l10n_it_edi', 'peppol', 'nemhandel', " "'pdp') THEN 'demo' ELSE 'test' END"),), requires=("proxy_type",)), + Rule("edi-proxy-my-gr", _ODOO.format("17.0-19.0", "addons/l10n_my_edi") + + "; " + _ODOO.format("18.0-19.0", "addons/l10n_gr_edi_e_invoo"), + "account_edi_proxy_client_user", + "t.active AND t.proxy_type IN ('l10n_my_edi', 'l10n_gr_edi')", + (("active", "false"),), requires=("proxy_type",)), + Rule("my-edi-mode", _ODOO.format("17.0-19.0", "addons/l10n_my_edi"), + "res_company", "t.l10n_my_edi_mode IS DISTINCT FROM 'test'", + (("l10n_my_edi_mode", "'test'"),), label="name"), + Rule("gr-edi", _ODOO.format("18.0-19.0", "addons/l10n_gr_edi"), + "res_company", "NOT coalesce(t.l10n_gr_edi_test_env, false)", + (("l10n_gr_edi_test_env", "true"),), label="name"), Rule("peppol-mode", _ODOO.format("17.0-19.0", "addons/account_peppol"), "ir_config_parameter", f"{_param('account_peppol.edi.mode')} AND t.value <> 'demo'", (("value", "'demo'"),), requires=("key",), label="key"), + Rule("certificate-password", _ODOO.format("18.0-19.0", "addons/certificate"), + "certificate_certificate", "t.pkcs12_password IS NOT NULL", + (("pkcs12_password", "'dummy'"),), label="name"), + Rule("certificate-key-password", _ODOO.format("18.0-19.0", "addons/certificate"), + "certificate_key", "t.password IS NOT NULL", (("password", "'dummy'"),), label="name"), + Rule("sms-twilio", _ODOO.format("18.0-19.0", "addons/sms_twilio"), + "res_company", "t.sms_twilio_auth_token IS NOT NULL", + (("sms_twilio_auth_token", "'dummytoken'"),), label="name"), + Rule("cloud-storage", _ODOO.format("18.0-19.0", "addons/cloud_storage(_azure, _google)"), + "ir_config_parameter", _params( + "cloud_storage_provider", "cloud_storage_azure_account_name", + "cloud_storage_azure_container_name", "cloud_storage_azure_tenant_id", + "cloud_storage_azure_client_id", "cloud_storage_azure_client_secret", + "cloud_storage_google_bucket_name", "cloud_storage_google_account_info"), + requires=("key",), label="key", delete=True), Rule("payment-provider", _ODOO.format("16.0-19.0", "addons/payment"), "payment_provider", "t.state NOT IN ('test', 'disabled')", (("state", "'disabled'"),), label="code"), @@ -129,28 +194,44 @@ def _param(key: str) -> str: (("active", "false"),), requires=("delivery_type",)), Rule("oauth", _ODOO.format("16.0-19.0", "addons/auth_oauth"), "auth_oauth_provider", "t.enabled", (("enabled", "false"),)), - Rule("google-calendar-users", "odoo 12.0-15.0 addons/google_calendar/models/res_users.py", + Rule("google-calendar-users", "odoo 12.0-14.0 addons/google_calendar/models/res_users.py", "res_users", "t.google_calendar_rtoken IS NOT NULL OR t.google_calendar_token IS NOT NULL", (("google_calendar_rtoken", "NULL"), ("google_calendar_token", "NULL"))), - Rule("google-calendar-settings", _ODOO.format("16.0-19.0", "addons/google_calendar"), + Rule("google-calendar-credentials", _ODOO.format("16.0-17.0", "addons/google_calendar") + + " (the table exists from 15.0)", + "google_calendar_credentials", "t.calendar_rtoken IS NOT NULL OR t.calendar_token IS NOT NULL", + (("calendar_rtoken", "NULL"), ("calendar_token", "NULL"), ("synchronization_stopped", "true"))), + Rule("google-calendar-settings", _ODOO.format("18.0-19.0", "addons/google_calendar"), "res_users_settings", "t.google_calendar_rtoken IS NOT NULL OR t.google_calendar_token IS NOT NULL", (("google_calendar_rtoken", "NULL"), ("google_calendar_token", "NULL"), ("google_synchronization_stopped", "true"))), - Rule("microsoft-calendar-users", _ODOO.format("16.0-19.0", "addons/microsoft_calendar"), + Rule("microsoft-calendar-users", "odoo 14.0-19.0 addons/microsoft_account " + "(neutralize.sql of microsoft_calendar 16.0-19.0)", "res_users", "t.microsoft_calendar_token IS NOT NULL OR t.microsoft_calendar_rtoken IS NOT NULL", (("microsoft_calendar_token", "NULL"), ("microsoft_calendar_rtoken", "NULL"))), - Rule("microsoft-calendar-settings", _ODOO.format("16.0-19.0", "addons/microsoft_calendar"), + Rule("microsoft-calendar-stopped", _ODOO.format("16.0", "addons/microsoft_calendar") + + " (the field exists 15.0-16.0)", + "res_users", "NOT coalesce(t.microsoft_synchronization_stopped, false)", + (("microsoft_synchronization_stopped", "true"),)), + Rule("microsoft-calendar-credentials", _ODOO.format("17.0", "addons/microsoft_calendar"), + "microsoft_calendar_credentials", "t.calendar_sync_token IS NOT NULL", + (("calendar_sync_token", "NULL"), ("synchronization_stopped", "true"))), + Rule("microsoft-calendar-settings", _ODOO.format("18.0-19.0", "addons/microsoft_calendar"), "res_users_settings", "t.microsoft_calendar_sync_token IS NOT NULL", (("microsoft_calendar_sync_token", "NULL"), ("microsoft_synchronization_stopped", "true"))), Rule("webhooks", _ODOO.format("17.0-19.0", "odoo/addons/base"), "ir_act_server", "t.state = 'webhook' AND t.webhook_url IS DISTINCT FROM 'neutralised: webhook disabled'", (("webhook_url", "'neutralised: webhook disabled'"),), requires=("state",), label="name"), + # Tokens longer than 33 characters are replaced whole, as 17.0-19.0 do: the + # suffix alone would leave a usable secret in the copy. Rule("iap", _ODOO.format("16.0-19.0", "addons/iap"), "iap_account", "t.account_token NOT LIKE '%+disabled'", - (("account_token", "regexp_replace(t.account_token, '(\\+.*)?$', '+disabled')"),), + (("account_token", "CASE WHEN length(t.account_token) <= 33 THEN " + "regexp_replace(t.account_token, '(\\+.*)?$', '+disabled') " + "ELSE 'dummy_value+disabled' END"),), label="service_name"), Rule("website-domain", _ODOO.format("16.0-19.0", "addons/website"), "website", "t.domain IS NOT NULL", (("domain", "NULL"),), label="name"), @@ -171,6 +252,11 @@ def _param(key: str) -> str: (("value", "'True'"),), requires=("key",), label="key"), ) +#: Odoo 14.0-16.0 read the EDI proxy's mode from a parameter, absent meaning +#: production (account_edi_proxy_client/models/account_edi_proxy_user.py +#: ``_get_demo_state``); 16.0's neutralize.sql sets it. 17.0 has ``edi_mode``. +EDI_DEMO_PARAM = "account_edi_proxy_client.demo" + def _lit(text: str) -> str: return "'" + text.replace("'", "''") + "'" @@ -191,28 +277,57 @@ def _guard(rule: Rule) -> str: def _armed(rule: Rule, local_url: str) -> str: + if rule.delete: + return "true" return " OR ".join(f"t.{c} IS DISTINCT FROM {_value(e, local_url)}" for c, e in rule.sets) +def _apply(rule: Rule, local_url: str) -> str: + if rule.delete: + return f"DELETE FROM {rule.table} t WHERE ({rule.where}); " + return (f"UPDATE {rule.table} t SET " + + ", ".join(f"{c} = {_value(e, local_url)}" for c, e in rule.sets) + + f" WHERE ({rule.where}) AND ({_armed(rule, local_url)}); ") + + +_AUTH_COL = ("(SELECT count(*) = 1 FROM pg_attribute a JOIN pg_class c ON c.oid = a.attrelid " + "JOIN pg_namespace s ON s.oid = c.relnamespace WHERE s.nspname = current_schema() " + "AND c.relname = 'ir_mail_server' AND a.attname = 'smtp_authentication' " + "AND NOT a.attisdropped)") + +# The EDI proxy of 14.0-16.0: its table, and no edi_mode column (17.0 has one). +_EDI_PARAM_ERA = ( + "(to_regclass('account_edi_proxy_client_user') IS NOT NULL AND NOT EXISTS (SELECT 1 FROM " + "pg_attribute a JOIN pg_class c ON c.oid = a.attrelid JOIN pg_namespace s ON s.oid = " + "c.relnamespace WHERE s.nspname = current_schema() AND c.relname = " + "'account_edi_proxy_client_user' AND a.attname = 'edi_mode' AND NOT a.attisdropped))" +) +# Production is a missing parameter or 'prod'; any other value is demo or test. +_EDI_PARAM_SAFE = f"EXISTS (SELECT 1 FROM ir_config_parameter WHERE key = '{EDI_DEMO_PARAM}' AND value <> 'prod')" + + def _mail_sink_block() -> str: """One active outgoing server pointing nowhere. Without it Odoo falls back to ``smtp_server`` in odoo.conf (``localhost:25`` by default), and a local MTA relays real mail. Cloned from an existing row when there is one, so the columns each version requires are filled; inserted otherwise.""" name, host, port = _lit(MAIL_SINK_NAME), _lit(MAIL_SINK_HOST), MAIL_SINK_PORT - auth_col = ("(SELECT count(*) = 1 FROM pg_attribute a JOIN pg_class c ON c.oid = a.attrelid " - "WHERE c.relname = 'ir_mail_server' AND a.attname = 'smtp_authentication' " - "AND NOT a.attisdropped)") + auth_col = _AUTH_COL + # 'login' (17.0-19.0): a server set to 'cli' sends through odoo.conf's + # smtp_server whatever its host says. A key the version lacks is ignored by + # jsonb_populate_record. return ( "IF to_regclass('ir_mail_server') IS NOT NULL THEN " f"IF EXISTS (SELECT 1 FROM ir_mail_server WHERE name = {name}) THEN " f"UPDATE ir_mail_server SET active = true, smtp_host = {host}, smtp_port = {port}, " f"smtp_user = NULL, smtp_pass = NULL, sequence = 1 WHERE name = {name}; " + f"IF {auth_col} THEN UPDATE ir_mail_server SET smtp_authentication = 'login' WHERE name = {name}; END IF; " "ELSIF EXISTS (SELECT 1 FROM ir_mail_server) THEN " "INSERT INTO ir_mail_server SELECT (jsonb_populate_record(NULL::ir_mail_server, " "to_jsonb(src) || jsonb_build_object('id', nextval(pg_get_serial_sequence('ir_mail_server', " f"'id')), 'name', {name}, 'smtp_host', {host}, 'smtp_port', {port}, " "'smtp_encryption', 'none', 'smtp_user', NULL, 'smtp_pass', NULL, 'sequence', 1, " + "'smtp_authentication', 'login', " "'active', true))).* FROM (SELECT * FROM ir_mail_server ORDER BY id LIMIT 1) src; " f"ELSIF {auth_col} THEN " "INSERT INTO ir_mail_server (name, smtp_host, smtp_port, smtp_encryption, sequence, active, " @@ -225,11 +340,11 @@ def _mail_sink_block() -> str: def apply_sql(local_url: str) -> str: """Neutralise the connected database. One statement, so it applies whole or not at all; repeatable.""" - blocks = "".join( - f"IF {_guard(rule)} THEN UPDATE {rule.table} t SET " - + ", ".join(f"{c} = {_value(e, local_url)}" for c, e in rule.sets) - + f" WHERE ({rule.where}) AND ({_armed(rule, local_url)}); END IF; " - for rule in CATALOGUE + blocks = "".join(f"IF {_guard(rule)} THEN {_apply(rule, local_url)}END IF; " for rule in CATALOGUE) + edi = ( + f"IF {_EDI_PARAM_ERA} THEN INSERT INTO ir_config_parameter (key, value) VALUES " + f"('{EDI_DEMO_PARAM}', 'true') ON CONFLICT (key) DO UPDATE SET value = 'true' " + "WHERE ir_config_parameter.value = 'prod'; END IF; " ) # Nested, not `AND`: PL/pgSQL plans the whole condition, so a query on a table # that is not there fails even behind a false `to_regclass` test. @@ -239,7 +354,7 @@ def apply_sql(local_url: str) -> str: "INSERT INTO ir_config_parameter (key, value) VALUES ('database.is_neutralized', 'True'); " "END IF; END IF; " ) - return f"DO $$ BEGIN {blocks}{_mail_sink_block()}{flag}END $$;" + return f"DO $$ BEGIN {blocks}{edi}{_mail_sink_block()}{flag}END $$;" def guard_sql(local_url: str) -> str: @@ -254,7 +369,12 @@ def guard_sql(local_url: str) -> str: sink = ( "IF to_regclass('ir_mail_server') IS NOT NULL THEN IF NOT EXISTS (SELECT 1 FROM ir_mail_server " f"WHERE active AND name = {_lit(MAIL_SINK_NAME)} AND smtp_host = {_lit(MAIL_SINK_HOST)}) THEN " - "found := found || 'mail-sink (missing) '; END IF; END IF; " + "found := found || 'mail-sink (missing) '; END IF; " + f"IF {_AUTH_COL} THEN IF EXISTS (SELECT 1 FROM ir_mail_server WHERE active AND " + "smtp_authentication = 'cli') THEN found := found || 'mail-server (cli: sends through " + "odoo.conf) '; END IF; END IF; END IF; " + f"IF {_EDI_PARAM_ERA} THEN IF NOT {_EDI_PARAM_SAFE} THEN " + "found := found || 'edi-proxy-demo (missing) '; END IF; END IF; " ) # Every rule looks in the current schema: if Odoo's tables are not there, no rule # applied and the check above saw nothing, so it must not pass. @@ -281,14 +401,27 @@ def columns_sql() -> str: def applicable(existing: set[tuple[str, str]]) -> list[Rule]: - """The rules whose table and columns exist, given ``(table, column)`` pairs.""" - return [r for r in CATALOGUE if all((r.table, c) in existing for c in r.columns)] + """The rules whose table and columns exist, given ``(table, column)`` pairs; a + label the table lacks is dropped (the check then names rows by id).""" + return [ + r if not r.label or (r.table, r.label) in existing else replace(r, label="") + for r in CATALOGUE if all((r.table, c) in existing for c in r.columns) + ] + + +def edi_param_armed(existing: set[tuple[str, str]]) -> bool: + """Whether the check must also look at the EDI proxy's demo parameter.""" + return ("account_edi_proxy_client_user", "id") in existing and ( + "account_edi_proxy_client_user", "edi_mode") not in existing -def check_sql(rules: list[Rule], local_url: str) -> str: +def check_sql(rules: list[Rule], local_url: str, edi_param: bool = False) -> str: """One row per rule with something still armed: rule, count, up to five labels. - Only ``SELECT``; pass only applicable rules.""" + Only ``SELECT``; pass only applicable rules, and ``edi_param`` from + :func:`edi_param_armed`.""" parts = [] + if edi_param: + parts.append(f"SELECT 'edi-proxy-demo', '1', 'missing' WHERE NOT {_EDI_PARAM_SAFE}") for rule in rules: label = f"t.{rule.label}::text" if rule.label else "t.id::text" parts.append( diff --git a/instance_manager/system.py b/instance_manager/system.py index 0d91bdd..e6de873 100644 --- a/instance_manager/system.py +++ b/instance_manager/system.py @@ -161,6 +161,19 @@ def database_exists(db_name: str) -> bool: return _local_psql_value(f"SELECT 1 FROM pg_database WHERE datname = {sql_literal(db_name)}") == "1" +def database_comment(db_name: str) -> str: + """The comment on ``db_name`` on the local server ("" when none or unreadable).""" + return _local_psql_value( + "SELECT coalesce(shobj_description(oid, 'pg_database'), '') FROM pg_database " + f"WHERE datname = {sql_literal(db_name)}" + ) or "" + + +def local_postgres_available() -> bool: + """True when the local server answers as postgres (``sudo -u postgres``).""" + return _local_psql_value("SELECT 1") == "1" + + def database_owner(db_name: str) -> str | None: """The role owning ``db_name`` on the local server, or None when it does not exist (or the server cannot be queried).""" diff --git a/instance_manager/workflows/backup_restore.py b/instance_manager/workflows/backup_restore.py index 3dfe688..8758403 100644 --- a/instance_manager/workflows/backup_restore.py +++ b/instance_manager/workflows/backup_restore.py @@ -9,6 +9,7 @@ from .. import neutralise from ..i18n import t, tf from ..models import ( + LOCAL_DB_HOSTS, POSTGRES_IDENTIFIER_RE, InstanceConfig, branch_error, @@ -36,9 +37,11 @@ ) from ..system import ( Command, + database_comment, database_exists, database_owner, detect_nginx_version, + local_postgres_available, path_exists, pg_env, read_odoo_conf, @@ -48,10 +51,12 @@ from ..ui import level_text, render_table from .common import ( DbCredentials, + _addons_paths, _ask_db_credentials, _backup_dir_error, _execute_plan, _filestore_path, + _instance_db_user, _odoo_conf_candidates, _own_filestore_commands, _pick_db_name, @@ -115,6 +120,7 @@ def _post_db_mode_commands( local_url: str, env: dict[str, str] | None = None, role: str = "", + odoo_sql_reader: InstanceConfig | None = None, ) -> list[Command]: """Apply Odoo migration semantics to an already-restored target database. @@ -142,6 +148,8 @@ def _post_db_mode_commands( ) ) if neutralize: + if odoo_sql_reader is not None: + commands.append(_odoo_neutralize_command(psql_target, role, odoo_sql_reader, env)) commands += [ Command( 'Neutralise the copy (crons, mail, payment, EDI/SII, calendars, webhooks, IAP, base URL)', @@ -157,37 +165,116 @@ def _post_db_mode_commands( return commands +#: Marks a copy this tool is still making (owned by postgres, not yet handed +#: over): a leftover of an interrupted run is recognised and may be replaced. +STAGING_COMMENT = "odoo-instance-manager: unfinished copy" + +# What Odoo's own `neutralize` runs: the data/neutralize.sql of every module in +# these states (odoo/modules/neutralize.py, 16.0-19.0), in install order. +_INSTALLED_MODULES_SQL = ( + "SELECT name FROM ir_module_module WHERE state IN ('installed', 'to upgrade', 'to remove') " + "ORDER BY id" +) +# Run as the instance user over its addons paths: the first module directory found +# wins, as Odoo's file_open does. +_READ_NEUTRALIZE_SQL = ( + 'while read -r m; do for d in "$@"; do f="$d/$m/data/neutralize.sql"; ' + "if [ -f \"$f\" ]; then printf -- '-- %s\\n' \"$m\"; cat -- \"$f\"; printf ';\\n'; break; fi; " + "done; done" +) + + +def _odoo_neutralize_command( + psql_target: str, role: str, reader: InstanceConfig, env: dict[str, str] | None +) -> Command: + """Odoo's own neutralisation (16.0-19.0): every installed module's + ``data/neutralize.sql``, read from ``reader``'s checkout as its user and run in + one transaction as the copy's owner — payment terminals, foreign EDIs and the + rest the catalogue does not name. Before 16.0 no module ships one, and nothing + runs. Names MUST be safe.""" + psql = f"{psql_target} -X -q -v ON_ERROR_STOP=1" + prefix = f'SET ROLE "{role}"; SET search_path = public; ' if role else "" + paths = " ".join(_quote(path) for path in _addons_paths(reader)) + script = "\n".join([ + "set -eo pipefail", + f"mods=$({psql} -tA -c {_quote(prefix + _INSTALLED_MODULES_SQL)} | grep -E '^[a-z0-9_]+$' || true)", + 'sql=$(printf \'%s\\n\' $mods | ' + + _as_instance_user(reader, _READ_NEUTRALIZE_SQL) + f" _ {paths})", + 'echo "Odoo\'s own neutralize.sql: $(grep -c \'^-- \' <<<"$sql" || true) module(s)"', + f'{{ printf \'%s\\n\' {_quote(prefix)}; printf \'%s\\n\' "$sql"; }} | {psql} -1', + ]) + return Command("Run Odoo's own neutralize.sql of every installed module (16.0-19.0)", script, + env=dict(env or {})) + + def _hand_over_commands(target_db: str, target_owner: str) -> list[Command]: - """Give the seeded database to its role. Until now postgres owned it, and an - Odoo lists only the databases its own role owns (``list_dbs``: ``datdba`` is - the connected role) — so no cron worker could start on the copy before it was - neutralised. Names MUST be safe (:func:`_is_safe_db_name`).""" + """Give the seeded database to its role: owner, ``CONNECT`` and the end of the + staging mark. Until now postgres owned it and the role could not connect — an + Odoo lists only the databases its role owns (``list_dbs``: ``datdba``), and one + with ``db_name`` set connects by name — so no cron worker could start on the + copy before it was neutralised. ``CREATE`` on ``public`` is taken from + ``PUBLIC`` (PostgreSQL 14 and older grant it). Names MUST be safe.""" return [ Command( tf('Hand the database over to role {}', target_owner), "sudo -u postgres psql -X -q -d postgres -v ON_ERROR_STOP=1 -c " - f"{_quote(f'ALTER DATABASE "{target_db}" OWNER TO "{target_owner}";')}", + + _quote( + f'ALTER DATABASE "{target_db}" OWNER TO "{target_owner}"; ' + f'GRANT CONNECT ON DATABASE "{target_db}" TO "{target_owner}"; ' + f'COMMENT ON DATABASE "{target_db}" IS NULL;' + ) + + f" && sudo -u postgres psql -X -q -d {_quote(target_db)} -v ON_ERROR_STOP=1 -c " + + _quote("REVOKE CREATE ON SCHEMA public FROM PUBLIC;"), ) ] -def _lock_db_access_commands(db_name: str, owner: str) -> list[Command]: - """Restrict a database to its owner: revoke ``CONNECT`` from ``PUBLIC`` and grant - it to the owning role, so an Odoo role cannot reach **other** instances' - databases. Names MUST be safe (:func:`_is_safe_db_name`).""" - psql = "sudo -u postgres psql -d postgres -v ON_ERROR_STOP=1" +def _lock_db_access_commands(db_name: str) -> list[Command]: + """Restrict a new copy: no ``CONNECT`` for ``PUBLIC``, so neither another + instance's role nor its own can connect before the hand-over grants it to the + owner. Names MUST be safe (:func:`_is_safe_db_name`).""" + psql = "sudo -u postgres psql -X -q -d postgres -v ON_ERROR_STOP=1" return [ Command( - 'Restrict DB access to its owner (revoke PUBLIC connect)', - f'{psql} -c \'REVOKE CONNECT ON DATABASE "{db_name}" FROM PUBLIC;\'', - ), - Command( - 'Grant DB connect to the owner role', - f'{psql} -c \'GRANT CONNECT ON DATABASE "{db_name}" TO "{owner}";\'', + 'Close the copy to every role until it is handed over (revoke PUBLIC connect)', + f"{psql} -c " + _quote( + f'REVOKE CONNECT ON DATABASE "{db_name}" FROM PUBLIC; ' + f"COMMENT ON DATABASE \"{db_name}\" IS '{STAGING_COMMENT}';" + ), ), ] +def _staged_copy_command( + description: str, target_db: str, create: Command, steps: list[Command], drop: str, on_failure: str = "", +) -> Command: + """``create`` (which makes ``target_db``) and ``steps`` as one step: if any of + them fails or is interrupted, the database this step created is dropped — an + unfinished copy is never left behind, armed or blocking a retry — and + ``on_failure`` runs (a service to start again). A database that existed before + is never dropped: ``create`` fails on it first.""" + def block(command: Command, text: str) -> list[str]: + return [f"echo {_quote('== ' + t(command.description))}", f"(\n{text}\n)"] + + head = [ + "set -e", + "created=", + "finish() { rc=$?; if [ \"$rc\" -ne 0 ]; then " + f"if [ -n \"$created\" ]; then echo {_quote(tf('The copy failed: its unfinished database {} is dropped.', target_db))} >&2; " + f"{drop} || true; fi; " + (f"{on_failure}; " if on_failure else "") + "fi; }", + "trap finish EXIT", + ] + lines, shown = list(head), list(head) + lines += block(create, create.command) + ["created=1"] + shown += block(create, create.shown) + ["created=1"] + env: dict[str, str] = dict(create.env) + for step in steps: + lines += block(step, step.command) + shown += block(step, step.shown) + env.update(step.env) + return Command(description, "\n".join(lines), env=env, display="\n".join(shown)) + + def _template_copy_script(source_db: str, target_db: str) -> str: """``createdb -T`` needs no other session on the template, and a running Odoo reconnects at once: so new connections to the source are refused first, its @@ -223,6 +310,7 @@ def _seed_db_commands(source_db: str, target_db: str, target_owner: str, method: 'Seed target DB via template copy (source closed to new sessions meanwhile)', _template_copy_script(source_db, target_db), ), + *_lock_db_access_commands(target_db), ] else: commands = [ @@ -230,18 +318,8 @@ def _seed_db_commands(source_db: str, target_db: str, target_owner: str, method: 'Create the empty target DB (owned by postgres until it is handed over)', f"sudo -u postgres createdb {_quote(target_db)}", ), - # The restore runs as the target role while postgres still owns the - # database: from PostgreSQL 15 only the owner may create in `public`, and a - # trusted extension (Odoo 18 creates pg_trgm) needs CREATE on the database. - # Handing the database over later makes the role its owner anyway. - Command( - tf('Let role {} create in the new DB until it is handed over', target_owner), - f"sudo -u postgres psql -X -q -v ON_ERROR_STOP=1 -d {_quote(target_db)} -c " - + _quote( - f'GRANT CREATE ON DATABASE "{target_db}" TO "{target_owner}"; ' - f'GRANT ALL ON SCHEMA public TO "{target_owner}";' - ), - ), + *_lock_db_access_commands(target_db), + _grant_create_command(target_db, target_owner), Command( 'Seed target DB via pg_dump | pg_restore (re-owned by the target role)', "set -o pipefail; " @@ -249,10 +327,48 @@ def _seed_db_commands(source_db: str, target_db: str, target_owner: str, method: f"sudo -u postgres pg_restore -d {_quote(target_db)} --no-owner --role={_quote(target_owner)} --no-privileges", ), ] - commands.extend(_lock_db_access_commands(target_db, target_owner)) return commands +def _grant_create_command(target_db: str, target_owner: str) -> Command: + # The restore runs as the target role while postgres still owns the database: + # from PostgreSQL 15 only the owner may create in `public`, and a trusted + # extension (Odoo creates pg_trgm from 16) needs CREATE on the database. Handing + # the database over later makes the role its owner anyway. + return Command( + tf('Let role {} create in the new DB until it is handed over', target_owner), + f"sudo -u postgres psql -X -q -v ON_ERROR_STOP=1 -d {_quote(target_db)} -c " + + _quote( + f'GRANT CREATE ON DATABASE "{target_db}" TO "{target_owner}"; ' + f'GRANT ALL ON SCHEMA public TO "{target_owner}";' + ), + ) + + +def _is_staging_leftover(db_name: str) -> bool: + """A copy an interrupted run of this tool left (owned by postgres, marked).""" + return database_owner(db_name) == "postgres" and database_comment(db_name) == STAGING_COMMENT + + +def _local_copy_command( + description: str, target_db: str, target_owner: str, seed: list[Command], migration_mode: str, + neutralize: bool, local_url: str, reader: InstanceConfig, on_failure: str = "", +) -> Command: + """Seed, give the copy its identity, neutralise and check it as its owner, and + hand it over — one staged step on the local server (see + :func:`_staged_copy_command`). Names MUST be safe.""" + steps = [ + *seed[1:], + *_post_db_mode_commands(_psql_target_local(target_db), migration_mode, neutralize, local_url, + role=target_owner, odoo_sql_reader=reader), + *_hand_over_commands(target_db, target_owner), + ] + return _staged_copy_command( + description, target_db, seed[0], steps, + f"sudo -u postgres dropdb --if-exists --force {_quote(target_db)}", on_failure, + ) + + def _drop_db_commands(target_db: str) -> list[Command]: """Drop ``target_db`` on the local server (superuser). ``--force`` (PostgreSQL 13+, older than every supported distribution's server) terminates its sessions @@ -369,13 +485,14 @@ def _restore_backup( return cached neutralize = ask_bool('Neutralize the target?', True) - creds = _ask_db_credentials(config.instance, cached) - db_host, db_port, db_user, db_password = creds.host, creds.port, creds.user, creds.password - + creds = cached commands: list[Command] = [] + restart: Command | None = None if restore_mode in {'Database only', 'Database + Filestore'}: - if database_exists(target_db): + conf = read_odoo_conf(config.odoo_conf_file) + local = (conf.get("db_host", "").strip().lower() in LOCAL_DB_HOSTS) and local_postgres_available() + if database_exists(target_db) and not _is_staging_leftover(target_db): print(level_text("ERROR", tf('The target DB already exists: {}', target_db))) return creds @@ -384,43 +501,72 @@ def _restore_backup( if not dump_file: print(level_text("INFO", 'Operation cancelled.')) return creds - # The instance's own role owns what it creates, so its cron worker would see - # the copy as soon as it exists: the service waits until it is neutralised. - stop_service = neutralize and service_active(config.odoo_service) - if stop_service: - commands.append( + url = _local_url(_config_with_port(config, conf)) + description = tf('Restore {} into {}, neutralise it and hand it over (undone if a step fails)', + os.path.basename(dump_file), target_db) + if local: + # As a duplication does: created by postgres, restored as the instance's + # role, invisible to every Odoo until it is neutralised and handed over — + # the running service needs no stop. + owner = _instance_db_user(config) + if not POSTGRES_IDENTIFIER_RE.fullmatch(owner): + print(level_text("ERROR", tf('Invalid db_user in {}: {}', config.odoo_conf_file, owner))) + return creds + if database_exists(target_db): + commands.extend(_drop_db_commands(target_db)) + seed = [ + Command('Create the empty target DB (owned by postgres until it is handed over)', + f"sudo -u postgres createdb {_quote(target_db)}"), + *_lock_db_access_commands(target_db), + _grant_create_command(target_db, owner), Command( - 'Stop the Odoo service while the copy is restored and neutralised', - f"systemctl stop {_quote(config.odoo_service)}", + tf('Restore the dump as role {}', owner), + f"sudo -u postgres pg_restore -d {_quote(target_db)} --no-owner --no-privileges " + f"--no-comments --role={_quote(owner)} < {_quote(dump_file)}", + ), + ] + commands.append(_local_copy_command( + description, target_db, owner, seed, migration_mode, neutralize, url, config, + )) + else: + creds = _ask_db_credentials(config.instance, cached) + db_host, db_port, db_user, db_password = creds.host, creds.port, creds.user, creds.password + connect = f"-h {_quote(db_host)} -p {int(db_port)} -U {_quote(db_user)}" + # The role restoring owns what it creates, so its cron worker would see the + # copy as soon as it exists: the service waits until it is neutralised, and + # is started again whatever happens. + stop_service = neutralize and service_active(config.odoo_service) + start = f"systemctl start {_quote(config.odoo_service)}" + if stop_service: + commands.append( + Command( + 'Stop the Odoo service while the copy is restored and neutralised', + f"systemctl stop {_quote(config.odoo_service)}", + ) ) + restart = Command('Start the Odoo service again', start) + create = Command( + 'Create target DB', + f"createdb {connect} -O {_quote(db_user)} {_quote(target_db)}", + env=pg_env(db_password), ) - commands.extend( - [ - Command( - 'Create 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), - ), + steps = [ Command( 'Restore the dump into the target DB', - 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)}", + f"pg_restore {connect} -d {_quote(target_db)} --no-owner --no-privileges --no-comments " + f"{_quote(dump_file)}", env=pg_env(db_password), ), + *_post_db_mode_commands( + _psql_target(db_host, db_port, db_user, db_password, target_db), + migration_mode, neutralize, url, env=pg_env(db_password), odoo_sql_reader=config, + ), ] - ) - commands.extend( - _post_db_mode_commands( - _psql_target(db_host, db_port, db_user, db_password, target_db), - migration_mode, - neutralize, - _local_url(_config_with_port(config, read_odoo_conf(config.odoo_conf_file))), - env=pg_env(db_password), - ) - ) - if stop_service: - commands.append( - Command('Start the Odoo service again', f"systemctl start {_quote(config.odoo_service)}") - ) + commands.append(_staged_copy_command( + description, target_db, create, steps, + f"dropdb --if-exists {connect} {_quote(target_db)}", + start if stop_service else "", + )) if restore_mode in {'Filestore only', 'Database + Filestore'}: print(level_text("INFO", 'Select a filestore backup file (.tar.gz)')) @@ -467,6 +613,10 @@ def _restore_backup( ) commands.extend(_own_filestore_commands(config, target_db)) + # Started again once the files are in place too. + if restart is not None: + commands.append(restart) + if not confirm_with_phrase( 'Sensitive restore action detected.', f"RESTORE {config.instance}", @@ -613,6 +763,9 @@ def _existing_target_error(target_db: str, target_owner: str) -> str | None: owner = database_owner(target_db) if owner is None or owner == target_owner: return None + # A copy this tool left unfinished (an interrupted run): its own, replaceable. + if owner == "postgres" and database_comment(target_db) == STAGING_COMMENT: + return None return tf( 'The database {} belongs to role {}, not to the target role {}: it is not the target\'s database and will not be dropped.', target_db, @@ -659,23 +812,22 @@ def _plan_refresh_target( tf('Target instance {} exists — refreshing it in place from {}.', target_config.instance, source_db), ) ) + start = f"systemctl start {_quote(target_config.odoo_service)}" commands: list[Command] = [ Command('Stop the target Odoo service', f"systemctl stop {_quote(target_config.odoo_service)} || true"), ] commands.extend(_drop_db_commands(target_db)) - commands.extend(_seed_db_commands(source_db, target_db, target_owner, method)) - commands.extend(_post_db_mode_commands( - _psql_target_local(target_db), migration_mode, neutralize, - _local_url(_config_with_port(target_config, existing)), role=target_owner, - )) - commands.extend(_hand_over_commands(target_db, target_owner)) if duplicate_filestore: commands.extend( _filestore_copy_commands(source_config, source_db, target_config, target_db) ) - commands.append( - Command('Start the target Odoo service', f"systemctl start {_quote(target_config.odoo_service)}") - ) + commands.append(_local_copy_command( + tf('Copy {} into {}, neutralise it and hand it over (undone if a step fails)', source_db, target_db), + target_db, target_owner, _seed_db_commands(source_db, target_db, target_owner, method), + migration_mode, neutralize, _local_url(_config_with_port(target_config, existing)), source_config, + on_failure=start, + )) + commands.append(Command('Start the target Odoo service', start)) return commands @@ -691,7 +843,8 @@ def _plan_replica_target( duplicate_filestore: bool, ) -> list[Command] | None: """Provision a brand-new target instance seeded from the source.""" - if database_exists(target_db): + leftover = database_exists(target_db) and _is_staging_leftover(target_db) + if database_exists(target_db) and not leftover: print(level_text("ERROR", tf('Target DB already exists: {}', target_db))) return None # The replica's database belongs to its own new role, never the source's. @@ -770,10 +923,9 @@ def _plan_replica_target( ) ) - commands: list[Command] = [] + commands: list[Command] = _drop_db_commands(target_db) if leftover else [] commands.extend(_plan_runtime(target_config)) commands.extend(plan_ensure_db_role(target_config)) - commands.extend(_seed_db_commands(source_db, target_db, target_db, method)) commands.extend(plan_odoo_base_setup(target_config, service_autostart=True, start_now=False)) commands.extend(wkhtmltopdf_plan) if replicate_packages: @@ -782,11 +934,12 @@ def _plan_replica_target( commands.extend( _filestore_copy_commands(source_config, source_db, target_config, target_db) ) - commands.extend(_post_db_mode_commands( - _psql_target_local(target_db), migration_mode, neutralize, _local_url(target_config), - role=target_db, + # The database last, in one step, once the instance around it is built. + commands.append(_local_copy_command( + tf('Copy {} into {}, neutralise it and hand it over (undone if a step fails)', source_db, target_db), + target_db, target_db, _seed_db_commands(source_db, target_db, target_db, method), + migration_mode, neutralize, _local_url(target_config), source_config, )) - commands.extend(_hand_over_commands(target_db, target_db)) commands.append( Command('Start the target Odoo service', f"systemctl start {_quote(target_config.odoo_service)}") ) @@ -871,16 +1024,16 @@ def _duplicate_database( commands: list[Command] = [] if overwrite: commands.extend(_drop_db_commands(target_db)) - commands.extend(_seed_db_commands(source_db, target_db, target_owner, method)) - commands.extend(_post_db_mode_commands( - _psql_target_local(target_db), migration_mode, neutralize, - _local_url(_config_with_port(config, existing)), role=target_owner, - )) - commands.extend(_hand_over_commands(target_db, target_owner)) + # Files first: the database is handed over, visible to Odoo, only when complete. if duplicate_filestore: commands.extend( _filestore_copy_commands(config, source_db, config, target_db) ) + commands.append(_local_copy_command( + tf('Copy {} into {}, neutralise it and hand it over (undone if a step fails)', source_db, target_db), + target_db, target_owner, _seed_db_commands(source_db, target_db, target_owner, method), + migration_mode, neutralize, _local_url(_config_with_port(config, existing)), config, + )) if not confirm_with_phrase( 'Sensitive duplication action detected.', @@ -1012,7 +1165,8 @@ def _check_neutralisation( 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, env=env) + armed = run(f"{psql} {_quote(neutralise.check_sql(rules, url, neutralise.edi_param_armed(existing)))}", # type: ignore[arg-type] + check=False, env=env) rows = [line.split("|", 2) for line in armed.stdout.splitlines() if line.count("|") >= 2] sink = run( f"{psql} " @@ -1025,6 +1179,11 @@ def _check_neutralisation( ) if sink.stdout.strip() != "1": rows.append(["mail-sink", "0", t('missing: mail can leave through odoo.conf\'s smtp_server')]) + if ("ir_mail_server", "smtp_authentication") in existing: + cli = run(f"{psql} " + _quote("SELECT count(*) FROM ir_mail_server WHERE active AND smtp_authentication = 'cli'"), + check=False, env=env) + if cli.stdout.strip() not in {"", "0"}: + rows.append(["mail-server-cli", cli.stdout.strip(), t("'cli' servers send through odoo.conf's smtp_server")]) if armed.returncode != 0: print(level_text("ERROR", armed.stderr.strip() or 'Could not read the database.')) return creds diff --git a/instance_manager/workflows/common.py b/instance_manager/workflows/common.py index ff1e531..167f4bc 100644 --- a/instance_manager/workflows/common.py +++ b/instance_manager/workflows/common.py @@ -9,6 +9,7 @@ import datetime import os +import re import shlex from dataclasses import dataclass @@ -181,6 +182,22 @@ def _probe_databases_for_management(instance: str) -> tuple[str, str | None, lis return db_name, db_error, listed_dbs +_ADDONS_DIR_RE = re.compile(r"/[A-Za-z0-9/._-]{1,250}") + + +def _addons_paths(config: InstanceConfig) -> list[str]: + """Where the instance's modules live: its odoo.conf ``addons_path`` plus the two + directories Odoo itself always adds (``odoo/addons`` and ``addons`` of the + checkout), each an absolute path of plain characters.""" + paths = [f"{config.odoo_home}/odoo/odoo/addons", f"{config.odoo_home}/odoo/addons"] + for conf_path in _odoo_conf_candidates(config.instance): + value = read_odoo_conf(conf_path).get("addons_path", "") + if value: + paths += [item.strip().rstrip("/") for item in value.split(",")] + break + return [p for p in dict.fromkeys(paths) if _ADDONS_DIR_RE.fullmatch(p) and ".." not in p.split("/")] + + def _resolve_data_dir(config: InstanceConfig) -> str: """The instance's data dir: the one the config being built names (a new instance has no odoo.conf on disk yet), else ``data_dir`` from its odoo.conf (current or diff --git a/openspec/changes/archive/2026-10-08-copies-done-whole/proposal.md b/openspec/changes/archive/2026-10-08-copies-done-whole/proposal.md new file mode 100644 index 0000000..b84f0c9 --- /dev/null +++ b/openspec/changes/archive/2026-10-08-copies-done-whole/proposal.md @@ -0,0 +1,52 @@ +# Copies done whole, neutralised as Odoo does + +## Why + +The second audit ran restores and duplications into failures, and read the neutralisation catalogue against +the Odoo 14.0-19.0 and OCA l10n-spain sources. + +- **A failed restore** left an armed copy. The plan stopped the service, created the database owned by the + instance's role (so visible to its cron worker), and stopped at the first error, such as a dump whose extension + comment the role may not set. The service stayed stopped, and a retry was refused because the target existed. +- **A failed duplication** left a postgres-owned database that blocked every retry ("belongs to role + postgres"). +- **The copy's role got `CONNECT` when it was seeded.** An Odoo with `db_name` set connects by name, without + listing, so it could reach the copy before it was neutralised. +- **Rules that named columns some versions lack were skipped silently**, leaving those copies in production: + - SII and TicketBAI on 14.0-17.0 use `l10n_es_edi_test_env`; + - the EDI proxy on 14.0-16.0 reads a parameter; + - Google Calendar tokens on 15.0-17.0 live in `google_calendar_credentials`; + - IAP's label column is not stored on 18.0-19.0. +- **Uncovered:** + - VERI*FACTU (Odoo and OCA), OCA TicketBAI, web push keys and devices, cloud storage, certificate and SMS + passwords, and the Malaysian and Greek EDI; + - a mail sink cloned from a `cli` server, which sends through `odoo.conf`; + - archived mail servers that 12.0-15.0 still use when a message names them. +- **Wrong citations:** the versions given for fetchmail, mail credentials, Microsoft settings and the Google + rules. + +## What changes + +- **One step per copy.** The seed, identity, neutralisation, check and hand-over form one staged step on the + local server. A failure drops only the database that step created; the target filestore is copied before it. +- **Hand-over:** the copy is marked with a staging comment until it is handed over, and the hand-over grants + `CONNECT` and takes `CREATE` on `public` from `PUBLIC`. A marked leftover is replaced, never refused. +- **Restore.** + - A local restore goes the same way, restored as the instance's role, with no service stop. + - A remote restore creates, restores and neutralises in one step. If any part fails, it drops the database and + restarts the service. + - Both use `--no-comments`, and the service starts after the files are restored. +- **Odoo's own `neutralize.sql`** of every installed module runs first on 16.0-19.0. It is read from the + instance's checkout as its user, and runs as the copy's owner. +- **The catalogue** gains `DELETE` rules, optional labels, and rules per version for everything listed above. The + mail sink logs in with `login`, the check rejects `cli` servers, and the citations are corrected. + +## Impact + +- Specs: `data-backup-restore`. +- Code: `neutralise.py`, `system.py`, `workflows/{backup_restore,common}.py`. +- Tests: `tests/test_copies_neutralise.py`. +- Verification: + - `tools/verify_privilege.py` (CI) runs the staged copy: success, failure, an existing target, and a leftover; + - `tools/verify_neutralisation.py` (real Odoo) checks Odoo's own SQL, the EDI demo parameter and the web push + key. diff --git a/openspec/changes/archive/2026-10-08-copies-done-whole/specs/data-backup-restore/spec.md b/openspec/changes/archive/2026-10-08-copies-done-whole/specs/data-backup-restore/spec.md new file mode 100644 index 0000000..532ea79 --- /dev/null +++ b/openspec/changes/archive/2026-10-08-copies-done-whole/specs/data-backup-restore/spec.md @@ -0,0 +1,255 @@ +## MODIFIED Requirements + +### Requirement: Restore + +Restoring SHALL create the target database from a selected dump and/or restore a filestore archive, refusing to +clobber an existing target database, and require phrase confirmation before applying. The pre-restore +existence check for the target database is evaluated against the **local** PostgreSQL server. An existing +target filestore SHALL be moved aside, never deleted, and the restored files SHALL belong to the instance user. +When the instance's database is local, the restore SHALL go the way a duplication does: created by postgres, +restored as the instance's role (`--role`), neutralised and handed over in one step, with no service stop. On a +remote server it SHALL create, restore and neutralise in one step that drops the database it created and starts +the service again when any part fails; the service is otherwise started again once the files are restored too. +Comments are not restored (`--no-comments`): a dump's comment on an extension fails for a role that does not own +it. + +#### Scenario: Existing target database blocks restore + +- **WHEN** the restore includes the database and the target database already exists (other than an unfinished + copy of this tool) +- **THEN** the operation reports the conflict and stops without executing + +#### Scenario: A failed restore leaves no armed copy + +- **WHEN** a step of the database restore fails (the dump does not load, the neutralisation fails) +- **THEN** the database the restore created is dropped and, if the service was stopped for it, started again + +#### Scenario: Target-database existence check is local-only + +- **WHEN** the operator targets a remote database host for the restore +- **THEN** the pre-restore existence check runs against the local server and may not detect a remote + collision; the subsequent `createdb` still fails safely if the remote target exists + +#### Scenario: Existing target filestore requires explicit overwrite + +- **WHEN** the restore includes the filestore and the target filestore exists +- **THEN** the operator must confirm overwrite; on confirmation the previous filestore is moved to + `.replaced-` before extracting, otherwise the restore is cancelled + +#### Scenario: Restored files belong to the instance user + +- **WHEN** a filestore archive is extracted +- **THEN** tar runs as the instance user (root only reads the archive), restores neither the archive's owners + nor its permission bits, and the plan gives the files to the + instance user: the whole data dir when it is the instance's own (`/var/lib/odoo/` or inside its + home), only that database's filestore when the data dir may be shared + +#### Scenario: Restore is phrase-confirmed + +- **WHEN** the restore plan is assembled +- **THEN** it executes only after the operator types the exact `RESTORE ` phrase + +### Requirement: Duplication + +The tool SHALL duplicate a source instance's database — and optionally its filestore — into a target, +end-to-end and existence-aware, requiring phrase confirmation and validating the source and target database +names as safe before using them in SQL. The target instance SHALL differ from the source instance, and the +target database SHALL differ from the source database. + +The operator SHALL choose the database copy method: a fast PostgreSQL **template** copy, or a robust +**`pg_dump | pg_restore --no-owner`** that reassigns ownership to the target role. A template copy keeps the +source's role as owner of every table, so the tool SHALL refuse it when the target role differs from the source +database's owner (always the case for a new replica). The template copy SHALL refuse new connections to the +source, terminate its sessions, copy, and reopen the source whatever the outcome. + +When the **target instance does not exist**, duplication SHALL provision it fully before seeding: create the +target role, run the base setup (system user, home, Odoo checkout at the source's version, virtualenv, +`odoo.conf`, systemd unit) **without starting the service**, optionally configure Nginx, seed the database and +filestore, then start the service. The replica SHALL follow the **same production-hardening prompts as a fresh +install** (secrets with informed choice, `list_db`, `dbfilter`, worker sizing, `db_sslmode`, wkhtmltopdf) with +auto-suggested non-colliding internal ports, and — when it fronts Nginx — SHALL require a domain that is **not +already served by another vhost** (or the replica would be unreachable behind a shared `server_name`). It SHALL +also offer to **replicate the source venv's Python packages** into the target venv, so addon dependencies the +source installed beyond `requirements.txt` are present in the replica. + +When the **target instance already exists**, duplication SHALL refresh it in place: stop the target service, +drop and recreate its database from the source and replace its filestore, apply the migration semantics, and +restart — **without** recreating the target's config, service, or system user. It SHALL drop an existing +target database only when that database belongs to the target's own role (read from its `odoo.conf`) and the +operator explicitly confirms the overwrite. + +Migration semantics (copied vs moved, neutralize) SHALL apply in both cases, and the duplicated filestore SHALL +be placed under the **target** instance's data directory — the one its configuration names, also for a replica +whose `odoo.conf` is not written yet — and owned by the target system user (the whole data dir when it is the +target's own, only the copied filestore otherwise). A filestore already at the target SHALL be moved aside to +`.replaced-`, never deleted, and the copy SHALL run only when the source filestore exists. +A target that is not a whole instance of this tool (home and `odoo.conf`) SHALL be provisioned only under a free +name, and never refreshed. Every database the tool seeds SHALL have its access +**restricted to its owner** — `CONNECT` revoked from `PUBLIC` when it is created, granted to the owning role at +the hand-over, and `CREATE` on `public` taken from `PUBLIC` — so an instance's database role cannot reach other +instances' databases. The database SHALL be copied after the rest of the target (files, and for a replica the +whole instance), in one step that drops it if any part fails. Local drops SHALL use `dropdb --force`, which closes the +database's sessions and refuses new ones in one statement. + +#### Scenario: Replica replicates the source venv Python packages + +- **WHEN** a replica is provisioned and the operator opts to replicate packages +- **THEN** the plan installs the source venv's packages into the target venv, so the replica has the same addon + Python dependencies as the source: the source's `pip freeze` runs as the source user, only exact + `name==version` lines are kept (in a private temporary directory) and shown, the install runs as the target + user, and the step fails when either fails + +#### Scenario: New target is provisioned and seeded as a replica + +- **WHEN** the target instance does not exist +- **THEN** the plan creates the target role, runs the same production-hardening prompts as a fresh install with + auto-suggested non-colliding ports, provisions the target (base setup without starting, optional Nginx), + seeds the database and filestore from the source with the `pg_dump` copy, and then starts the target service + +#### Scenario: Replica domain must not collide with another vhost + +- **WHEN** the replica is configured to front Nginx and the chosen domain is already a `server_name` in an + enabled vhost +- **THEN** the tool rejects it and re-prompts for a different domain, so the replica is reachable + +#### Scenario: Existing target is refreshed in place + +- **WHEN** the target instance already exists +- **THEN** the plan stops the target service, drops and recreates its database from the source, replaces its + filestore, applies the migration semantics, and restarts the service without recreating its config or unit + +#### Scenario: A database that is not the target's is never dropped + +- **WHEN** the target database exists and belongs to a role other than the target's, or the target equals the + source instance or the source database +- **THEN** the tool refuses with a descriptive error and builds no plan + +#### Scenario: Overwriting the target's database is confirmed + +- **WHEN** the target database exists and belongs to the target's role +- **THEN** the operator must explicitly confirm that it will be dropped and replaced before the plan is built + +#### Scenario: Operator selects the database copy method + +- **WHEN** the duplication plan is assembled +- **THEN** the operator chooses a template copy or a `pg_dump | pg_restore --no-owner` copy, and the plan uses + the selected method, refusing a template copy across roles + +#### Scenario: Template copy frees the source; dump copy leaves it untouched + +- **WHEN** the template method is chosen +- **THEN** the plan blocks and terminates the source's sessions before the copy and re-enables them afterward + even on failure; **WHEN** the dump method is chosen, the source is read live without terminating its sessions + +#### Scenario: Migration semantics and phrase confirmation + +- **WHEN** duplication runs +- **THEN** copied/moved and neutralize semantics are applied to the target and execution proceeds only after + the operator types the exact `DUPLICATE ` phrase for a replica, or `REPLACE ` for a refresh, + whose data it replaces + +#### Scenario: Seeded database is restricted to its owner + +- **WHEN** the tool seeds a target database +- **THEN** the plan revokes `CONNECT` on that database from `PUBLIC` when it is created and grants it to the + owning role only at the hand-over, so no role — its own included — connects to it before it is neutralised + +#### Scenario: Target data dir is owned by the target user + +- **WHEN** the filestore is copied into the target data directory (created as root) +- **THEN** the plan chowns the target's own data directory to the target system user, so Odoo can create its + `sessions` and `filestore` entries; in a data directory that may be shared, only the copied filestore + +#### Scenario: A previous target filestore is moved aside + +- **WHEN** the target filestore already exists +- **THEN** it is moved to `.replaced-` and the copy lands in the target, not nested in it + +#### Scenario: Unsafe database name is rejected + +- **WHEN** the source or target database name is not a safe name +- **THEN** the plan is not built and the operation reports the problem instead of interpolating it into SQL + +### Requirement: Copied vs moved database semantics + +Restore and duplication SHALL apply Odoo migration semantics: in "copied" mode give the target its own +`database.uuid`, `database.secret` and `database.create_date`, as Odoo's own copy does; when neutralisation is +requested, first run Odoo's own neutralisation — the `data/neutralize.sql` of every module installed in the copy +(states `installed`, `to upgrade`, `to remove`), read as the instance user from its addons paths, as Odoo +16.0-19.0's `neutralize` does — then apply every rule of the neutralisation catalogue (Odoo's `neutralize.sql` 16.0-19.0, extended to +12.0-19.0 and the OCA modules it lists), each guarded by the existence of its table and columns, as one statement +that stops the plan on failure, and then verify that nothing can still act on the outside, failing the plan when +something can. On the local server these statements SHALL run as the copy's owner role with +`search_path = public` (`pg_catalog` is still searched first), never as the superuser: a trigger or function the source database carries +then runs with that role's rights. The neutralisation SHALL leave exactly one active outgoing mail server, pointing at a host that +does not resolve and logging in (`login`, never `cli`, which sends through `odoo.conf`), so Odoo never falls back +to the `smtp_server` of `odoo.conf`, and SHALL drop production's SMTP credentials from the copy and point every +other server at that host (12.0-15.0 still send through an archived server a message names). + +#### Scenario: Copied mode regenerates the database UUID + +- **WHEN** the operator selects the copied mode +- **THEN** the plan writes a fresh `database.uuid`, `database.secret` and `database.create_date` into + `ir_config_parameter` on the target database + +#### Scenario: Neutralization deactivates automation in the target + +- **WHEN** the operator opts to neutralize the target +- **THEN** the plan deactivates crons (except Odoo's autovacuum and queue_job's cleanup), mail servers (dropping + their credentials), fetchmail, payment providers, external carriers and their production mode, OAuth + providers, calendar tokens (Google and Microsoft, in every table they lived in 12.0-19.0), webhooks, IAP tokens, + web push keys and devices, cloud storage settings, certificate and SMS passwords, the EDI proxy (its + 14.0-16.0 parameter included), the Spanish SII, TicketBAI and VERI*FACTU of Odoo and OCA, Peppol and the + Malaysian and Greek EDI, and queued jobs; points `web.base.url` at the target, adds the mail sink, and sets + `database.is_neutralized` + +#### Scenario: A failed or incomplete neutralisation stops the plan + +- **WHEN** the neutralisation statement fails, or the check finds something that can still act on the outside +- **THEN** the step fails and the plan stops; no step tolerates its own failure + +#### Scenario: Code in the copy runs with its owner's rights + +- **WHEN** the copied database has a trigger that tries to make its owner a superuser +- **THEN** the trigger fails for lack of privilege, the step fails, and the role is unchanged + +#### Scenario: A check that sees no Odoo table fails + +- **WHEN** the check runs where the current schema holds no Odoo table (`ir_cron`) +- **THEN** it fails instead of passing because no rule applied + +### Requirement: A copy stays invisible until neutralised + +A database seeded on the local server SHALL be owned by `postgres` until its migration semantics are applied, and +only then handed to its role: an Odoo lists only the databases its own role owns, so no cron worker can start on +the copy before it is neutralised, and its role is granted `CONNECT` only then, so one with `db_name` set cannot +connect by name either. Until it is handed over the copy SHALL carry a staging comment, by which a copy an +interrupted run left behind is recognised and may be replaced; any other database owned by postgres is never +taken for one. A restore to a remote server that neutralises SHALL stop the instance's running service before +creating the database and start it again afterwards, whether or not the restore succeeded. + +#### Scenario: The copy is handed over after neutralisation + +- **WHEN** a database is duplicated +- **THEN** it is created by postgres, neutralised, and only then `ALTER DATABASE … OWNER TO` the target role + +#### Scenario: A restore keeps the service stopped meanwhile + +- **WHEN** a database is restored to a remote server with neutralisation while the instance's service runs +- **THEN** the plan stops the service first and starts it again after the files, or at once if the restore fails + +#### Scenario: An interrupted copy can be replaced + +- **WHEN** the target database is a copy this tool left unfinished (owned by postgres, with the staging comment) +- **THEN** a new duplication or restore drops and replaces it instead of refusing + +### Requirement: Neutralisation check + +The tool SHALL offer a read-only check of a database that lists, per rule, what can still act on the outside, +whether the mail sink is in place, any active `cli` mail server, and, on 14.0-16.0, whether the EDI proxy's demo +parameter is set. + +#### Scenario: An armed database is reported + +- **WHEN** the operator checks a database whose crons or mail servers are active +- **THEN** the tool lists each such rule with its row count and examples, and changes nothing diff --git a/openspec/changes/archive/2026-10-08-copies-done-whole/tasks.md b/openspec/changes/archive/2026-10-08-copies-done-whole/tasks.md new file mode 100644 index 0000000..da0233a --- /dev/null +++ b/openspec/changes/archive/2026-10-08-copies-done-whole/tasks.md @@ -0,0 +1,11 @@ +# Tasks + +- [x] 1. data-backup-restore delta; `openspec validate --specs` (1.4.1) passes. +- [x] 2. Catalogue: delete rules, optional labels, Spanish EDI per version, VERI*FACTU and TicketBAI (Odoo, OCA), + EDI proxy (17-19, 14-16 parameter, MY/GR), web push, cloud storage, certificates, SMS, calendars per table, + IAP long tokens, mail host and `login` sink, `cli` and EDI checks, citations. +- [x] 3. Odoo's own neutralize.sql step (16.0-19.0), read as the instance user, run as the owner. +- [x] 4. Staged copy: one step, dropped on failure, staging comment, CONNECT at hand-over, leftovers replaced. +- [x] 5. Restore: local as a duplication, remote atomic with the service restarted; `--no-comments`. +- [x] 6. Order: target files before the database; replica database after the instance. +- [x] 7. Tests and verification (unit, `verify_privilege`, `verify_neutralisation` on Odoo 14 and 18). diff --git a/openspec/specs/data-backup-restore/spec.md b/openspec/specs/data-backup-restore/spec.md index d072be3..9c112f2 100644 --- a/openspec/specs/data-backup-restore/spec.md +++ b/openspec/specs/data-backup-restore/spec.md @@ -56,12 +56,24 @@ Restoring SHALL create the target database from a selected dump and/or restore a clobber an existing target database, and require phrase confirmation before applying. The pre-restore existence check for the target database is evaluated against the **local** PostgreSQL server. An existing target filestore SHALL be moved aside, never deleted, and the restored files SHALL belong to the instance user. +When the instance's database is local, the restore SHALL go the way a duplication does: created by postgres, +restored as the instance's role (`--role`), neutralised and handed over in one step, with no service stop. On a +remote server it SHALL create, restore and neutralise in one step that drops the database it created and starts +the service again when any part fails; the service is otherwise started again once the files are restored too. +Comments are not restored (`--no-comments`): a dump's comment on an extension fails for a role that does not own +it. #### Scenario: Existing target database blocks restore -- **WHEN** the restore includes the database and the target database already exists +- **WHEN** the restore includes the database and the target database already exists (other than an unfinished + copy of this tool) - **THEN** the operation reports the conflict and stops without executing +#### Scenario: A failed restore leaves no armed copy + +- **WHEN** a step of the database restore fails (the dump does not load, the neutralisation fails) +- **THEN** the database the restore created is dropped and, if the service was stopped for it, started again + #### Scenario: Target-database existence check is local-only - **WHEN** the operator targets a remote database host for the restore @@ -123,8 +135,10 @@ target's own, only the copied filestore otherwise). A filestore already at the t `.replaced-`, never deleted, and the copy SHALL run only when the source filestore exists. A target that is not a whole instance of this tool (home and `odoo.conf`) SHALL be provisioned only under a free name, and never refreshed. Every database the tool seeds SHALL have its access -**restricted to its owner** — `CONNECT` revoked from `PUBLIC` and granted to the owning role — so an instance's -database role cannot reach other instances' databases. Local drops SHALL use `dropdb --force`, which closes the +**restricted to its owner** — `CONNECT` revoked from `PUBLIC` when it is created, granted to the owning role at +the hand-over, and `CREATE` on `public` taken from `PUBLIC` — so an instance's database role cannot reach other +instances' databases. The database SHALL be copied after the rest of the target (files, and for a replica the +whole instance), in one step that drops it if any part fails. Local drops SHALL use `dropdb --force`, which closes the database's sessions and refuses new ones in one statement. #### Scenario: Replica replicates the source venv Python packages @@ -187,8 +201,8 @@ database's sessions and refuses new ones in one statement. #### Scenario: Seeded database is restricted to its owner - **WHEN** the tool seeds a target database -- **THEN** the plan revokes `CONNECT` on that database from `PUBLIC` and grants it to the owning role, so other - instances' roles cannot connect to it +- **THEN** the plan revokes `CONNECT` on that database from `PUBLIC` when it is created and grants it to the + owning role only at the hand-over, so no role — its own included — connects to it before it is neutralised #### Scenario: Target data dir is owned by the target user @@ -210,14 +224,17 @@ database's sessions and refuses new ones in one statement. Restore and duplication SHALL apply Odoo migration semantics: in "copied" mode give the target its own `database.uuid`, `database.secret` and `database.create_date`, as Odoo's own copy does; when neutralisation is -requested, apply every rule of the neutralisation catalogue (Odoo's `neutralize.sql` 16.0-19.0, extended to +requested, first run Odoo's own neutralisation — the `data/neutralize.sql` of every module installed in the copy +(states `installed`, `to upgrade`, `to remove`), read as the instance user from its addons paths, as Odoo +16.0-19.0's `neutralize` does — then apply every rule of the neutralisation catalogue (Odoo's `neutralize.sql` 16.0-19.0, extended to 12.0-19.0 and the OCA modules it lists), each guarded by the existence of its table and columns, as one statement that stops the plan on failure, and then verify that nothing can still act on the outside, failing the plan when something can. On the local server these statements SHALL run as the copy's owner role with `search_path = public` (`pg_catalog` is still searched first), never as the superuser: a trigger or function the source database carries then runs with that role's rights. The neutralisation SHALL leave exactly one active outgoing mail server, pointing at a host that -does not resolve, so Odoo never falls back to the `smtp_server` of `odoo.conf`, and SHALL drop production's SMTP -credentials from the copy. +does not resolve and logging in (`login`, never `cli`, which sends through `odoo.conf`), so Odoo never falls back +to the `smtp_server` of `odoo.conf`, and SHALL drop production's SMTP credentials from the copy and point every +other server at that host (12.0-15.0 still send through an archived server a message names). #### Scenario: Copied mode regenerates the database UUID @@ -230,8 +247,11 @@ credentials from the copy. - **WHEN** the operator opts to neutralize the target - **THEN** the plan deactivates crons (except Odoo's autovacuum and queue_job's cleanup), mail servers (dropping their credentials), fetchmail, payment providers, external carriers and their production mode, OAuth - providers, calendar tokens, webhooks, IAP tokens, EDI/SII/TicketBAI/Peppol production modes and queued jobs, - points `web.base.url` at the target, adds the mail sink, and sets `database.is_neutralized` + providers, calendar tokens (Google and Microsoft, in every table they lived in 12.0-19.0), webhooks, IAP tokens, + web push keys and devices, cloud storage settings, certificate and SMS passwords, the EDI proxy (its + 14.0-16.0 parameter included), the Spanish SII, TicketBAI and VERI*FACTU of Odoo and OCA, Peppol and the + Malaysian and Greek EDI, and queued jobs; points `web.base.url` at the target, adds the mail sink, and sets + `database.is_neutralized` #### Scenario: A failed or incomplete neutralisation stops the plan @@ -310,8 +330,11 @@ for its version on this host, and SHALL get its own data dir `/var/lib/odoo/ None: for method in ("dump", "template"): joined = "\n".join(c.command for c in _seed_db_commands("prod", "dev", "dev", method)) self.assertIn('REVOKE CONNECT ON DATABASE "dev" FROM PUBLIC;', joined) - self.assertIn('GRANT CONNECT ON DATABASE "dev" TO "dev";', joined) + # Its own role connects only once it is handed over, neutralised. + self.assertNotIn('GRANT CONNECT', joined) class DropDbCommandsTests(unittest.TestCase): @@ -144,7 +145,10 @@ def test_moved_without_neutralize_is_empty(self) -> None: def test_hand_over_gives_the_database_to_its_role(self) -> None: [cmd] = backup_restore._hand_over_commands("dev", "dev") - self.assertIn('ALTER DATABASE "dev" OWNER TO "dev";', shlex.split(cmd.command)[-1]) + words = shlex.split(cmd.command) + self.assertIn('ALTER DATABASE "dev" OWNER TO "dev"; GRANT CONNECT ON DATABASE "dev" TO "dev"; ' + 'COMMENT ON DATABASE "dev" IS NULL;', words) + self.assertIn("REVOKE CREATE ON SCHEMA public FROM PUBLIC;", words) class FilestoreCopyTests(unittest.TestCase): diff --git a/tests/test_copies_neutralise.py b/tests/test_copies_neutralise.py new file mode 100644 index 0000000..4a730d1 --- /dev/null +++ b/tests/test_copies_neutralise.py @@ -0,0 +1,115 @@ +"""Copies: the catalogue's new rules, Odoo's own neutralize.sql, and the staged +copy that is done whole or dropped.""" + +from __future__ import annotations + +import unittest +from unittest import mock + +from instance_manager import neutralise +from instance_manager.models import InstanceConfig +from instance_manager.system import Command +from instance_manager.workflows import backup_restore + +URL = "http://127.0.0.1:8069" + + +def _rule(rule_id: str) -> neutralise.Rule: + return next(r for r in neutralise.CATALOGUE if r.id == rule_id) + + +class CatalogueTests(unittest.TestCase): + def test_spanish_test_modes_per_version(self) -> None: + # Fields as each version's res_company declares them (odoo 14.0-19.0, OCA l10n-spain). + self.assertEqual(_rule("spain-edi-odoo").sets, (("l10n_es_edi_test_env", "true"),)) + self.assertEqual(_rule("sii-odoo").sets, (("l10n_es_sii_test_env", "true"),)) + self.assertEqual(_rule("verifactu-odoo").sets, (("l10n_es_edi_verifactu_test_environment", "true"),)) + self.assertEqual(_rule("verifactu-oca").sets, (("verifactu_test", "true"),)) + self.assertEqual(_rule("ticketbai-oca").sets, (("tbai_test_enabled", "true"),)) + + def test_secrets_without_a_harmless_value_are_deleted(self) -> None: + sql = neutralise.apply_sql(URL) + self.assertIn("DELETE FROM ir_config_parameter t WHERE (t.key IN ('mail.web_push_vapid_private_key'", sql) + self.assertIn("DELETE FROM mail_push_device t WHERE (true)", sql) + self.assertIn("'cloud_storage_azure_client_secret'", sql) + + def test_mail_servers_point_nowhere_and_the_sink_logs_in(self) -> None: + self.assertIn(("smtp_host", "'invalid'"), _rule("mail-servers").sets) + sql = neutralise.apply_sql(URL) + self.assertIn("'smtp_authentication', 'login'", sql) + self.assertIn("UPDATE ir_mail_server SET smtp_authentication = 'login'", sql) + self.assertIn("smtp_authentication = 'cli'", neutralise.guard_sql(URL)) + + def test_the_edi_proxy_parameter_of_14_to_16(self) -> None: + sql = neutralise.apply_sql(URL) + self.assertIn("INSERT INTO ir_config_parameter (key, value) VALUES ('account_edi_proxy_client.demo', 'true')", + sql) + self.assertIn("edi-proxy-demo (missing)", neutralise.guard_sql(URL)) + old = {("account_edi_proxy_client_user", "id")} + self.assertTrue(neutralise.edi_param_armed(old)) + self.assertFalse(neutralise.edi_param_armed(old | {("account_edi_proxy_client_user", "edi_mode")})) + self.assertIn("'edi-proxy-demo'", neutralise.check_sql([], URL, edi_param=True)) + + def test_long_iap_tokens_are_replaced_whole(self) -> None: + [(column, expr)] = _rule("iap").sets + self.assertEqual(column, "account_token") + self.assertIn("length(t.account_token) <= 33", expr) + self.assertIn("'dummy_value+disabled'", expr) + + def test_a_rule_without_its_label_still_applies(self) -> None: + existing = {("iap_account", "id"), ("iap_account", "account_token")} + [rule] = [r for r in neutralise.applicable(existing) if r.id == "iap"] + self.assertEqual(rule.label, "") + self.assertIn("t.id::text", neutralise.check_sql([rule], URL)) + + +class OdooNeutralizeTests(unittest.TestCase): + def test_reads_the_checkout_as_its_user_and_runs_as_the_owner(self) -> None: + reader = InstanceConfig(instance="prod") + with mock.patch.object(backup_restore, "_addons_paths", return_value=["/opt/odoo/prod/odoo/addons"]): + command = backup_restore._odoo_neutralize_command( + backup_restore._psql_target_local("copy"), "dev", reader, None).command + self.assertIn("ir_module_module WHERE state IN (", command) + self.assertIn("to upgrade", command) + self.assertIn("sudo -u prod -H bash -c", command) + self.assertIn("/opt/odoo/prod/odoo/addons", command) + self.assertIn('SET ROLE "dev"; SET search_path = public;', command) + self.assertTrue(command.rstrip().endswith("-v ON_ERROR_STOP=1 -1")) + + def test_it_runs_before_the_catalogue(self) -> None: + with mock.patch.object(backup_restore, "_addons_paths", return_value=[]): + steps = backup_restore._post_db_mode_commands( + "psql", "Moved (keep UUID)", True, URL, role="dev", odoo_sql_reader=InstanceConfig(instance="p")) + self.assertIn("neutralize.sql", steps[0].description) + self.assertIn("Neutralise the copy", steps[1].description) + + +class StagedCopyTests(unittest.TestCase): + def test_only_a_database_it_created_is_dropped(self) -> None: + command = backup_restore._staged_copy_command( + "copy", "dev", Command("Create", "createdb dev"), [Command("Fill", "false")], "dropdb dev", + "systemctl start shop").command + lines = command.splitlines() + self.assertEqual(lines[0], "set -e") + self.assertLess(lines.index("createdb dev"), lines.index("created=1")) + self.assertIn('if [ -n "$created" ]', command) + self.assertIn("systemctl start shop", command) + + def test_the_copy_is_closed_until_the_hand_over(self) -> None: + seed = backup_restore._seed_db_commands("prod", "dev", "dev", "dump") + joined = "\n".join(c.command for c in seed) + self.assertIn(backup_restore.STAGING_COMMENT, joined) + self.assertNotIn("GRANT CONNECT", joined) + + def test_the_whole_copy_is_one_step(self) -> None: + with mock.patch.object(backup_restore, "_addons_paths", return_value=[]): + command = backup_restore._local_copy_command( + "copy", "dev", "dev", backup_restore._seed_db_commands("prod", "dev", "dev", "dump"), + "Copied (new UUID on target)", True, URL, InstanceConfig(instance="prod")).command + for part in ("createdb dev", "pg_restore", "database.uuid", "neutralize.sql", "can still act", + 'OWNER TO "dev"', "dropdb --if-exists --force dev"): + self.assertIn(part, command, part) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_neutralise.py b/tests/test_neutralise.py index b5d7cb4..5c72067 100644 --- a/tests/test_neutralise.py +++ b/tests/test_neutralise.py @@ -32,7 +32,7 @@ def setUp(self) -> None: def test_one_statement_guarded_per_rule(self) -> None: self.assertTrue(self.sql.startswith("DO $$") and self.sql.endswith("END $$;")) - self.assertEqual(self.sql.count("FROM pg_attribute a JOIN pg_class c"), len(neutralise.CATALOGUE) + 1) + self.assertEqual(self.sql.count("JOIN pg_namespace n ON n.oid"), len(neutralise.CATALOGUE)) def test_housekeeping_cron_stays(self) -> None: self.assertIn("('base', 'autovacuum_job')", self.sql) @@ -64,10 +64,13 @@ def test_check_sql_only_selects(self) -> None: self.assertEqual(sql.count("SELECT 'crons'"), 1) def test_applicable_needs_every_column(self) -> None: - existing = {("ir_cron", "id"), ("ir_cron", "active")} + existing = {("ir_cron", "id")} self.assertEqual(neutralise.applicable(existing), []) + # The label only names rows in the check: without it the rule still applies. + existing.add(("ir_cron", "active")) + self.assertEqual([(r.id, r.label) for r in neutralise.applicable(existing)], [("crons", "")]) existing.add(("ir_cron", "cron_name")) - self.assertEqual([r.id for r in neutralise.applicable(existing)], ["crons"]) + self.assertEqual([(r.id, r.label) for r in neutralise.applicable(existing)], [("crons", "cron_name")]) if __name__ == "__main__": diff --git a/tools/verify_neutralisation.py b/tools/verify_neutralisation.py index 9bd1c19..aea73d5 100644 --- a/tools/verify_neutralisation.py +++ b/tools/verify_neutralisation.py @@ -34,6 +34,7 @@ from verify_data_safety import Cluster, _postgres_bindir, _write_exe # noqa: E402 +from instance_manager.models import InstanceConfig # noqa: E402 from instance_manager.workflows import backup_restore # noqa: E402 FAILURES: list[str] = [] @@ -69,6 +70,13 @@ def _arm(cluster: Cluster, db: str, major: int) -> None: "UPDATE delivery_carrier SET prod_environment = true", "UPDATE auth_oauth_provider SET enabled = true", ] + if major >= 17: + statements.append("INSERT INTO ir_config_parameter (key, value) VALUES " + "('mail.web_push_vapid_private_key', 'production-push-key')") + else: + # Installed as 'demo'; production sets 'prod' (or removes it). + statements.append("UPDATE ir_config_parameter SET value = 'prod' " + "WHERE key = 'account_edi_proxy_client.demo'") if major >= 16: statements.append("UPDATE payment_provider SET state = 'enabled' WHERE code = 'demo' OR id = " "(SELECT min(id) FROM payment_provider)") @@ -93,7 +101,8 @@ def _visible_to(cluster: Cluster, role: str, db: str) -> bool: def _one(major: int, python: str, clone: Path, cluster: Cluster, env: dict[str, str], root: Path) -> None: src, dst = f"prod{major}", f"copy{major}" data = root / f"data{major}" - modules = "base,mail,payment,delivery,auth_oauth,iap" + (",fetchmail" if major <= 16 else "") + modules = ("base,mail,payment,delivery,auth_oauth,iap,account_edi_proxy_client" + + (",fetchmail" if major <= 16 else "")) print(f"info Odoo {major}: installing {modules} (a few minutes)", flush=True) result = _odoo(python, clone, cluster, data, src, "-i", modules, "--without-demo=all", "--stop-after-init", "--no-http") @@ -108,9 +117,13 @@ def _one(major: int, python: str, clone: Path, cluster: Cluster, env: dict[str, check(f"Odoo {major}: the guard refuses the armed source", result.returncode != 0 and "can still act on the outside" in result.stderr, result.stderr) + # Odoo's own neutralize.sql is read from this checkout, as the copy's reader. + reader = InstanceConfig(instance="dev") + backup_restore._addons_paths = lambda config: [f"{clone}/odoo/addons", f"{clone}/addons"] plan = (backup_restore._seed_db_commands(src, dst, "dev", "dump") + backup_restore._post_db_mode_commands( - backup_restore._psql_target_local(dst), "Copied (new UUID on target)", True, URL, role="dev")) + backup_restore._psql_target_local(dst), "Copied (new UUID on target)", True, URL, role="dev", + odoo_sql_reader=reader)) for command in plan: result = subprocess.run(["bash", "-c", command.command], capture_output=True, text=True, env=env) output = result.stdout + result.stderr @@ -143,6 +156,20 @@ def value(db: str, sql: str) -> str: value(dst, "SELECT count(*) FROM ir_mail_server WHERE smtp_pass IS NOT NULL") == "0") check(f"Odoo {major}: the copy's base URL is local", value(dst, "SELECT value FROM ir_config_parameter WHERE key = 'web.base.url'") == URL) + if major >= 16: + check(f"Odoo {major}: Odoo's own neutralize.sql ran (its mail sink is there)", + value(dst, "SELECT count(*) FROM ir_mail_server WHERE name = 'neutralization - disable emails'") != "0") + if major <= 16: + check(f"Odoo {major}: the EDI proxy is in demo mode", + value(dst, "SELECT value FROM ir_config_parameter WHERE key = 'account_edi_proxy_client.demo'") + not in {"", "prod"}, + value(dst, "SELECT coalesce(to_regclass('account_edi_proxy_client_user')::text, 'no table') || ' / ' " + "|| (SELECT string_agg(name || ':' || state, ',') FROM ir_module_module " + "WHERE name LIKE 'account_edi%')")) + if major >= 17: + check(f"Odoo {major}: the web push key is gone from the copy", + value(dst, "SELECT count(*) FROM ir_config_parameter WHERE key = " + "'mail.web_push_vapid_private_key'") == "0") script = ( "Mail = env['ir.mail_server'].sudo()\n" diff --git a/tools/verify_privilege.py b/tools/verify_privilege.py index 43cfd34..9d01471 100644 --- a/tools/verify_privilege.py +++ b/tools/verify_privilege.py @@ -28,6 +28,7 @@ import sys import tempfile from pathlib import Path +from unittest import mock HERE = Path(__file__).resolve().parent sys.path.insert(0, str(HERE.parent)) @@ -258,10 +259,50 @@ def _neutralise_as_owner(root: Path) -> None: result = _run(check_step.command, env) check("the check fails where it sees no Odoo table, instead of passing on nothing", result.returncode != 0 and "no Odoo tables" in result.stderr, result.stderr) + _staged_copies(cluster, env) finally: cluster.stop() +def _staged_copy(source: str, target: str) -> str: + reader = InstanceConfig(instance="dev") + return backup_restore._local_copy_command( + "copy", target, "dev", backup_restore._seed_db_commands(source, target, "dev", "dump"), + "Moved (keep UUID)", False, "http://127.0.0.1:8069", reader, + ).command + + +def _staged_copies(cluster: Cluster, env: dict[str, str]) -> None: + """The copy is one step: done whole, or its database is dropped.""" + def exists(db: str) -> bool: + return cluster.value(f"SELECT count(*) FROM pg_database WHERE datname = '{db}'") == "1" + + result = _run(_staged_copy("copy_plain", "staged_ok"), env) + check("a staged copy succeeds", result.returncode == 0, result.stderr) + state = cluster.value("SELECT pg_get_userbyid(datdba) || ' ' || has_database_privilege('dev', 'staged_ok', " + "'CONNECT') || ' ' || coalesce(shobj_description(oid, 'pg_database'), '-') " + "FROM pg_database WHERE datname = 'staged_ok'") + check("and is handed over: owned by its role, connectable, unmarked", state == "dev true -", state) + + result = _run(_staged_copy("no_such_source", "staged_fail"), env) + check("a copy that fails partway fails the step", result.returncode != 0) + check("and leaves no unfinished database behind", not exists("staged_fail"), result.stderr) + + cluster.value('CREATE DATABASE "already_there"') + result = _run(_staged_copy("copy_plain", "already_there"), env) + check("a target that already exists is never dropped by the copy", + result.returncode != 0 and exists("already_there"), result.stderr) + + cluster.value('CREATE DATABASE "left_over"') + cluster.value(f"COMMENT ON DATABASE \"left_over\" IS '{backup_restore.STAGING_COMMENT}'") + with mock.patch.dict(os.environ, env): + check("an interrupted run's copy is recognised as its own", backup_restore._is_staging_leftover("left_over") + and backup_restore._existing_target_error("left_over", "dev") is None) + check("a database merely owned by postgres is not", + not backup_restore._is_staging_leftover("already_there") + and backup_restore._existing_target_error("already_there", "dev") is not None) + + def main() -> int: with tempfile.TemporaryDirectory(prefix="oim-verify-privilege-") as tmp: root = Path(tmp)