From 2c4e646037c53a92a01239aabc4595d18368ba5f Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Wed, 12 Aug 2026 14:42:22 +0000 Subject: [PATCH 01/18] docs: design PostgreSQL user synchronizer --- .../2026-08-12-pgsql-user-sync-design.md | 373 ++++++++++++++++++ 1 file changed, 373 insertions(+) create mode 100644 docs/superpowers/specs/2026-08-12-pgsql-user-sync-design.md diff --git a/docs/superpowers/specs/2026-08-12-pgsql-user-sync-design.md b/docs/superpowers/specs/2026-08-12-pgsql-user-sync-design.md new file mode 100644 index 0000000000..0e1063d02f --- /dev/null +++ b/docs/superpowers/specs/2026-08-12-pgsql-user-sync-design.md @@ -0,0 +1,373 @@ +# PostgreSQL User Credential Synchronizer Design + +## Purpose + +Provide an optional sample utility that periodically copies eligible PostgreSQL +role names and password verifiers into ProxySQL's `pgsql_users` table. The +utility gives ProxySQL deployments the useful part of PgBouncer's +`auth_user`/`auth_query` model without putting a remote database lookup in the +client authentication path. + +The synchronizer is an external control-plane tool. ProxySQL continues to +authenticate clients exclusively from its in-memory user configuration. An +accepted credential-consistency delay is therefore the configured scheduler +interval plus the duration of one successful synchronization. + +This work depends on ProxySQL supporting PostgreSQL SCRAM verifiers and MD5 +credential hashes in `pgsql_users`, as proposed by PR #5865 or an equivalent +change. The synchronizer does not implement authentication protocol behavior. + +## Goals + +- Automatically create ProxySQL users for explicitly selected PostgreSQL login + roles. +- Copy password verifiers without retrieving or storing plaintext role + passwords. +- Update credentials and reactivate previously disabled managed users. +- Preserve ProxySQL-specific policy on existing users. +- Make treatment of roles missing from the source snapshot configurable. +- Keep the last successfully loaded runtime configuration when source or + validation failures occur. +- Provide fast unit tests and an end-to-end PostgreSQL/ProxySQL test. +- Supply secure example configuration, source SQL, and scheduler documentation. + +## Non-goals + +- A ProxySQL core `auth_query` implementation. +- A login-time credential lookup or cache-miss callback. +- Synchronizing superusers or every role in a PostgreSQL cluster by default. +- Synchronizing ProxySQL routing, limits, TLS, fast-forward, or transaction + policy from PostgreSQL. +- Terminating already authenticated client sessions after a role is disabled. +- Deleting `pgsql_users` rows. Disabling is reversible and is sufficient for + the initial sample. +- Coordinating multiple simultaneous synchronization writers. + +## Packaging + +The sample is placed under `tools/pgsql_user_sync/`: + +```text +tools/pgsql_user_sync/ +├── README.md +├── create_source_function.sql +├── proxysql_pgsql_user_sync.ini.example +├── proxysql_pgsql_user_sync.py +├── requirements.txt +└── tests/ + └── test_pgsql_user_sync.py +``` + +The Python implementation uses `psycopg` for PostgreSQL and `PyMySQL` for the +ProxySQL Admin interface. Database access is isolated behind small adapters so +the reconciliation logic can be tested without live services. + +## Source role selection + +The example SQL creates: + +- a dedicated `NOLOGIN` allow-list role; +- a schema owned by a trusted administrator; +- a fully qualified `SECURITY DEFINER` function; and +- a minimally privileged login role with only `CONNECT`, schema `USAGE`, and + function `EXECUTE` privileges. + +The function has a fixed safe `search_path` and returns exactly two columns: +`username` and `password`. It selects only roles that: + +- are members of the allow-list role; +- have `rolcanlogin = true`; +- have a non-null `rolpassword`; and +- have no expiration or a `rolvaliduntil` later than the current time. + +This allow-list prevents accidental import of `postgres`, replication users, +and unrelated service accounts. A role that stops satisfying any selection +condition is absent from the next snapshot and is processed according to +`missing_role_action`. + +## Configuration and command line + +The required command form is: + +```text +proxysql_pgsql_user_sync.py --config /path/to/pgsql-user-sync.ini [options] +``` + +The INI file contains three sections: + +```ini +[source] +host = postgresql.example +port = 5432 +database = postgres +username = proxysql_auth_reader +password = source-secret +connect_timeout = 10 +function = proxysql_auth.export_login_roles + +[proxysql] +host = 127.0.0.1 +port = 6032 +username = admin +password = admin-secret +connect_timeout = 10 + +[sync] +profile = primary-cluster +default_hostgroup = 0 +missing_role_action = disable +adopt_existing_users = false +allow_empty_snapshot = false +save_to_disk = true +lock_file = /run/lock/proxysql-pgsql-user-sync.lock +``` + +`profile` is a stable, non-empty identifier used to mark ownership. Profile +names must match `[A-Za-z0-9][A-Za-z0-9_.-]{0,63}`. The configured function +must be a schema-qualified pair of ordinary PostgreSQL identifiers and is +quoted with the driver's identifier-composition API rather than interpolated +into SQL. `default_hostgroup` is a non-negative integer and defaults to `0`, +matching the `pgsql_users` schema. The implementation does not require a +currently populated `pgsql_servers` row for that hostgroup. + +The supported non-secret command-line overrides are: + +- `--default-hostgroup N`; +- `--missing-role-action disable|keep`; +- `--save-to-disk` and `--no-save-to-disk`; +- `--dry-run`; and +- `--verbose`. + +Database passwords are deliberately config-only. They never appear in the +scheduler table, process arguments, or normal logs. The configuration must be a +regular file readable by the executing account. Group write/execute access and +all access by other users are rejected; `0600` and root-owned `0640` with a +dedicated ProxySQL group are documented examples. Symlink handling follows the +normal operating-system file open rules, but the resolved file's metadata is +validated before credentials are used. + +## Ownership and row policy + +The utility owns only rows bearing this reserved object in the `attributes` +JSON document: + +```json +{"proxysql_pgsql_user_sync":{"profile":"primary-cluster"}} +``` + +Existing attributes outside that reserved key are preserved semantically; +serialization is normalized only when the ownership key must be added or +changed. An existing reserved key with an invalid shape is a conflict rather +than something the script overwrites. Existing `comment` values are not used as +state and are preserved. + +For a source role with no ProxySQL row, the utility creates a standard combined +frontend/backend user with: + +- `active = 1`; +- `default_hostgroup` from configuration or the CLI override; +- `backend = 1` and `frontend = 1`; and +- all other policy fields at the `pgsql_users` schema defaults. + +For an already managed user, the utility changes only `password`, `active`, and +its ownership marker. It preserves `use_ssl`, `default_hostgroup`, +`transaction_persistent`, `fast_forward`, `backend`, `frontend`, +`max_connections`, unrelated attributes, and `comment`. + +An existing unmanaged user with the same name is a conflict. By default the +entire run aborts before any write. When `adopt_existing_users = true`, the row +is marked as belonging to the configured profile, its credential is updated, +and all ProxySQL policy fields are preserved. A row already owned by another +profile always causes the run to abort; there is no implicit ownership +transfer. + +The script treats the relevant frontend identity as unique. Ambiguous existing +layouts containing multiple rows for the same username are rejected rather +than guessed at. + +## Snapshot validation + +The complete source result is fetched before connecting changes to ProxySQL. +The snapshot is rejected if: + +- it is empty and `allow_empty_snapshot` is false; +- a username is empty, duplicated, contains a NUL byte, or exceeds PostgreSQL's + 63-byte role-name limit when encoded as UTF-8; +- a password value is null or is neither a syntactically valid PostgreSQL SCRAM + verifier nor an `md5` hash of the expected length; or +- the result shape differs from the documented two-column contract. + +The synchronizer validates verifier syntax but never logs verifier contents. +An empty snapshot is not intrinsically invalid, but requiring an explicit +opt-in prevents a source query or permission mistake from disabling every +managed user. + +## Reconciliation flow + +Each invocation performs these steps: + +1. Parse and validate CLI arguments and the protected configuration file. +2. Acquire an exclusive, nonblocking file lock. If another run holds it, report + a skipped run and exit successfully. +3. Fetch and fully validate the source snapshot. +4. Fetch the relevant `pgsql_users` and `runtime_pgsql_users` rows. +5. Build a deterministic change plan in memory and detect all ownership or row + conflicts before writing. +6. If `--dry-run` is set, report counts and exit without writing credentials. +7. Apply parameterized Admin statements for planned creates, credential + updates, reactivations, and optional disables. +8. Run `LOAD PGSQL USERS TO RUNTIME` when Admin rows changed or when the managed + main/runtime views differ. +9. If configured, run `SAVE PGSQL USERS TO DISK` only after a successful runtime + load. +10. Emit a summary containing counts, duration, profile, and outcome, then + release the lock. + +ProxySQL's Admin interface acknowledges transaction-control statements without +opening a client-visible SQLite transaction. Therefore the design does not +claim that multiple Admin writes are transactionally atomic. Runtime safety is +the boundary: no `LOAD` is issued until every planned Admin write succeeds. A +mid-write failure can leave `main.pgsql_users` partially updated while +`runtime_pgsql_users` remains the last-known-good configuration. The next run +recomputes the plan from both tables and repairs/reloads it. This limitation is +called out in the README. + +`LOAD PGSQL USERS TO RUNTIME` is also required when the source and main table +already match but managed main and runtime rows do not. This makes a load +failure retryable on the next scheduler execution. + +## Missing role behavior + +`missing_role_action` supports two values: + +- `disable` (default): set `active = 0` for managed rows whose names are absent + from the validated source snapshot; +- `keep`: make no change to managed rows absent from the snapshot. + +Only rows owned by the active profile are considered. No mode deletes rows. +When a disabled role becomes eligible again, a later run updates its verifier +and sets `active = 1`. + +## Failure handling and observability + +- Configuration, source connection, query, and snapshot-validation failures + occur before writes and exit nonzero. +- Ownership and ambiguous-row conflicts abort before writes and exit nonzero. +- A ProxySQL Admin write failure exits nonzero without loading runtime. +- A runtime-load failure exits nonzero and is retried on the next invocation + because main/runtime divergence remains visible. +- A disk-save failure exits nonzero but does not roll back the already loaded + runtime configuration; the summary distinguishes this partial operational + outcome. +- Lock contention is an expected scheduler condition and exits zero. +- Logs include the profile, elapsed time, and counts for discovered, created, + updated, reactivated, disabled, unchanged, and conflicted users. +- Logs never include passwords, verifiers, connection strings containing + passwords, or complete configuration objects. + +Normal successful and skipped runs produce one concise summary line. Verbose +mode may report usernames and planned action types, but still never credential +material. + +## Scheduler operation + +ProxySQL invokes scheduler programs using `execve` with an empty environment and +does not prevent overlapping occurrences. The example therefore uses absolute +paths and relies on the script's file lock: + +```sql +INSERT INTO scheduler + (id, active, interval_ms, filename, arg1, arg2, arg3, comment) +VALUES + (9100, 1, 10000, + '/usr/bin/python3', + '/usr/share/proxysql/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py', + '--config', + '/etc/proxysql/pgsql-user-sync.ini', + 'Synchronize PostgreSQL role verifiers'); + +LOAD SCHEDULER TO RUNTIME; +SAVE SCHEDULER TO DISK; +``` + +The script does not depend on environment variables. The README documents +installing the Python dependencies in a dedicated virtual environment as an +alternative; in that case `scheduler.filename` is the absolute path to that +environment's Python interpreter. + +For a ProxySQL cluster, operators must choose one of these models: + +1. Run the synchronizer on one authoritative ProxySQL node and enable + `pgsql_users` cluster propagation. +2. Run the same deterministic profile on every node and disable cluster + propagation for `pgsql_users`. + +Combining independent writers with cluster propagation is unsupported because +it can cause checksum/epoch churn and competing updates. + +## Test strategy + +### Unit tests + +`tools/pgsql_user_sync/tests/test_pgsql_user_sync.py` uses Python's standard +`unittest` and fake source/Admin adapters. It covers: + +- configuration defaults, required fields, permissions, and CLI precedence; +- hostgroup, profile, boolean, enum, and timeout validation; +- SCRAM and MD5 verifier syntax without exposing test values in errors; +- empty, duplicate, malformed, and wrong-shaped source snapshots; +- automatic creation with the built-in and overridden hostgroup; +- credential update and reactivation; +- preservation of every ProxySQL policy field, comment, and unrelated + attribute; +- managed ownership, unmanaged adoption, cross-profile conflicts, and + ambiguous existing rows; +- both `disable` and `keep` missing-role behavior; +- empty-snapshot opt-in; +- dry-run behavior; +- source failure and Admin write failure behavior; +- load retry caused by main/runtime divergence; +- conditional disk save; +- lock contention; and +- secret redaction in normal, verbose, and error logs. + +The adapters expose narrow methods such as `fetch_snapshot`, `fetch_users`, +`apply_actions`, `load_runtime`, and `save_to_disk`. Planning tests operate on +plain immutable records, keeping most coverage independent of driver versions. + +### End-to-end test + +A PostgreSQL infrastructure test exercises the installed drivers, source +function, ProxySQL Admin interface, and verifier-capable authentication path: + +1. Create the allow-list, reader, source function, and a SCRAM login role. +2. Run the utility and assert automatic creation in both `pgsql_users` and + `runtime_pgsql_users` with the configured hostgroup and ownership marker. +3. Authenticate through ProxySQL using the source role. +4. Rotate the PostgreSQL password, rerun synchronization, and verify the old + password fails while the new password succeeds. +5. Remove `LOGIN` or expire the role, rerun with `disable`, and verify the + managed ProxySQL row becomes inactive. +6. Exercise `keep` and verify the managed row remains active when absent. +7. Verify an unrelated unmanaged ProxySQL user remains byte-for-byte unchanged. +8. Make the source lookup fail and verify runtime users remain unchanged. + +The integration test is registered only in a PostgreSQL-backed TAP group and +is gated on verifier-capable ProxySQL behavior. Until the equivalent of PR +#5865 is present in the target branch, the test must report a clear dependency +skip rather than masking a synchronizer failure. + +## Acceptance criteria + +- An allow-listed SCRAM or MD5 PostgreSQL role is automatically provisioned in + ProxySQL without plaintext password access. +- A configurable default hostgroup is used only for new users. +- Existing ProxySQL policy survives credential changes and adoption. +- Missing managed roles are disabled or kept according to configuration. +- Unmanaged and other-profile rows cannot be silently changed. +- No source-side failure or invalid snapshot changes runtime authentication. +- Runtime loading is retried after main/runtime divergence. +- Secrets and verifiers do not appear in logs or process arguments. +- Unit tests cover reconciliation and error paths; the infrastructure test + covers creation, authentication, rotation, revocation behavior, isolation, + and source failure. From 47fb2c1141c60b2a2f0f5b9df0832d2bfee35613 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Wed, 12 Aug 2026 14:52:42 +0000 Subject: [PATCH 02/18] docs: plan PostgreSQL user synchronizer --- .../plans/2026-08-12-pgsql-user-sync.md | 663 ++++++++++++++++++ 1 file changed, 663 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-12-pgsql-user-sync.md diff --git a/docs/superpowers/plans/2026-08-12-pgsql-user-sync.md b/docs/superpowers/plans/2026-08-12-pgsql-user-sync.md new file mode 100644 index 0000000000..8a48bc5497 --- /dev/null +++ b/docs/superpowers/plans/2026-08-12-pgsql-user-sync.md @@ -0,0 +1,663 @@ +# PostgreSQL User Credential Synchronizer Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Build and test a sample scheduler utility that provisions ProxySQL `pgsql_users` from an allow-listed PostgreSQL role-verifier snapshot. + +**Architecture:** A self-contained Python module separates immutable configuration/row/action records, pure validation and reconciliation functions, and narrow PostgreSQL/ProxySQL adapters. The CLI acquires a file lock, validates the complete source snapshot, computes a deterministic plan, applies parameterized Admin writes, and loads runtime only after all writes succeed. Documentation and SQL assets make the sample deployable without adding a lookup to ProxySQL's authentication path. + +**Tech Stack:** Python 3.10+, standard-library `argparse`, `configparser`, `dataclasses`, `fcntl`, `json`, `logging`, and `unittest`; `psycopg` 3.x; PyMySQL 1.1+; ProxySQL TAP/infrastructure harness. + +## Global Constraints + +- This utility remains outside ProxySQL core and performs no login-time lookup. +- `default_hostgroup` defaults to `0`; non-secret CLI options may override `[sync]` configuration. +- Connection passwords are config-only and must never appear in arguments, logs, exceptions, or complete configuration representations. +- Configuration files must be regular/readable, not group-writable/executable, and inaccessible to other users; `0600` and root-owned `0640` are supported examples. +- Managed ownership is stored at `attributes.proxysql_pgsql_user_sync.profile`; unrelated JSON attributes and all ProxySQL policy columns are preserved semantically. +- `missing_role_action` supports exactly `disable` (default) and `keep`; the utility never deletes users. +- Source snapshots are complete and validated before any Admin write; empty snapshots require `allow_empty_snapshot = true`. +- Runtime is the safety boundary because ProxySQL Admin does not expose multi-statement SQLite transactions to clients; never issue `LOAD PGSQL USERS TO RUNTIME` after a failed write. +- `SAVE PGSQL USERS TO DISK` occurs only after a successful runtime load and only when configured. +- Scheduler jobs run with an empty environment and can overlap, so examples use absolute paths and the script uses a nonblocking file lock. +- End-to-end authentication assertions require PR #5865 or equivalent verifier support; control-plane synchronization remains testable without it. + +--- + +## File map + +- Create `tools/pgsql_user_sync/proxysql_pgsql_user_sync.py`: records, validation, reconciliation, database adapters, orchestration, and CLI. +- Create `tools/pgsql_user_sync/tests/test_pgsql_user_sync.py`: dependency-free unit tests using fake adapters. +- Create `tools/pgsql_user_sync/proxysql_pgsql_user_sync.ini.example`: deployable configuration template. +- Create `tools/pgsql_user_sync/create_source_function.sql`: allow-list, safe `SECURITY DEFINER` function, and reader grants. +- Create `tools/pgsql_user_sync/requirements.txt`: supported driver versions. +- Create `tools/pgsql_user_sync/README.md`: installation, security, Scheduler SQL, clustering, and failure semantics. +- Create `test/tap/tests/pgsql-user-sync-unit-t.py`: no-infrastructure CI wrapper for the canonical unit suite. +- Create `test/tap/tests/pgsql-user-sync-t.py`: PostgreSQL infrastructure lifecycle/authentication test. +- Modify `test/tap/groups/groups.json`: register both test executables. +- Modify `test/infra/docker-base/Dockerfile`: install psycopg for integration tests. + +--- + +### Task 1: Configuration and snapshot validation + +**Files:** +- Create: `tools/pgsql_user_sync/proxysql_pgsql_user_sync.py` +- Create: `tools/pgsql_user_sync/tests/test_pgsql_user_sync.py` + +**Interfaces:** +- Produces: frozen `SourceConfig`, `ProxySQLConfig`, `SyncSettings`, `AppConfig`, `CLIOverrides`, and `SourceRole` dataclasses. +- Produces: `parse_args(argv: Sequence[str]) -> argparse.Namespace`. +- Produces: `load_config(path: Path, overrides: CLIOverrides) -> AppConfig`. +- Produces: `validate_snapshot(rows: Iterable[Sequence[object]], allow_empty: bool) -> dict[str, SourceRole]`. +- Produces: `validate_verifier(value: str) -> None`, raising `SyncError` without embedding `value`. + +- [ ] **Step 1: Write failing configuration tests** + +Create an import helper so the test does not require a package, then cover defaults, CLI precedence, permissions, profile/function syntax, non-negative hostgroups, booleans, timeouts, and required fields: + +```python +SCRIPT = Path(__file__).parents[1] / "proxysql_pgsql_user_sync.py" + +def load_module(): + spec = importlib.util.spec_from_file_location("proxysql_pgsql_user_sync", SCRIPT) + module = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = module + spec.loader.exec_module(module) + return module + +class ConfigTests(unittest.TestCase): + def test_defaults_and_cli_hostgroup_override(self): + path = self.write_config(mode=0o600) + cfg = self.mod.load_config(path, self.mod.CLIOverrides(default_hostgroup=12)) + self.assertEqual(12, cfg.sync.default_hostgroup) + self.assertEqual("disable", cfg.sync.missing_role_action) + self.assertFalse(cfg.sync.allow_empty_snapshot) + + def test_rejects_world_readable_config(self): + path = self.write_config(mode=0o604) + with self.assertRaisesRegex(self.mod.SyncError, "permissions"): + self.mod.load_config(path, self.mod.CLIOverrides()) +``` + +- [ ] **Step 2: Run configuration tests and verify they fail** + +```bash +python3 -m unittest -v tools/pgsql_user_sync/tests/test_pgsql_user_sync.py +``` + +Expected: import failure because the implementation file does not exist. + +- [ ] **Step 3: Implement immutable configuration records and parsing** + +Keep module import standard-library-only. Import drivers inside adapter methods. Define: + +```python +class SyncError(RuntimeError): + pass + +@dataclass(frozen=True) +class CLIOverrides: + default_hostgroup: int | None = None + missing_role_action: str | None = None + save_to_disk: bool | None = None + +@dataclass(frozen=True) +class SourceConfig: + host: str + port: int + database: str + username: str + password: str = field(repr=False) + connect_timeout: int = 10 + function_schema: str = "proxysql_auth" + function_name: str = "export_login_roles" + +@dataclass(frozen=True) +class ProxySQLConfig: + host: str + port: int + username: str + password: str = field(repr=False) + connect_timeout: int = 10 + +@dataclass(frozen=True) +class SyncSettings: + profile: str + default_hostgroup: int = 0 + missing_role_action: str = "disable" + adopt_existing_users: bool = False + allow_empty_snapshot: bool = False + save_to_disk: bool = True + lock_file: Path = Path("/run/lock/proxysql-pgsql-user-sync.lock") + +@dataclass(frozen=True) +class AppConfig: + source: SourceConfig + proxysql: ProxySQLConfig + sync: SyncSettings + +@dataclass(frozen=True) +class SourceRole: + username: str + password: str = field(repr=False) + +PROFILE_RE = re.compile(r"[A-Za-z0-9][A-Za-z0-9_.-]{0,63}\Z") +IDENTIFIER_RE = re.compile(r"[A-Za-z_][A-Za-z0-9_]*\Z") +``` + +Split `[source].function` once on `.`. Validate the resolved file with `Path.stat()`, `stat.S_ISREG`, `os.access(path, os.R_OK)`, and `(mode & 0o037) == 0`, permitting group-read only. + +- [ ] **Step 4: Write failing verifier and snapshot tests** + +Cover lowercase MD5, valid SCRAM, invalid base64/key lengths, duplicates, empty opt-in, NULs, non-string/wrong-shaped rows, and UTF-8 names over 63 bytes: + +```python +def test_snapshot_rejects_duplicate_without_leaking_verifier(self): + verifier = "md5" + "b" * 32 + with self.assertRaisesRegex(self.mod.SyncError, "duplicate") as ctx: + self.mod.validate_snapshot([("alice", verifier), ("alice", verifier)], False) + self.assertNotIn(verifier, str(ctx.exception)) +``` + +Build valid SCRAM values with `base64.b64encode(b"x" * 32).decode()` for both keys. + +- [ ] **Step 5: Implement verifier and complete-snapshot validation** + +Accept `md5[0-9a-f]{32}` or `SCRAM-SHA-256$iterations:salt$stored:server`. For SCRAM, require iterations `1..2_147_483_647`, strict base64, non-empty salt, and two decoded 32-byte keys. Materialize the full snapshot; require two string columns, unique names, no NUL, and at most 63 UTF-8 bytes; return a username-sorted dictionary. + +- [ ] **Step 6: Run tests and commit** + +```bash +python3 -m unittest -v tools/pgsql_user_sync/tests/test_pgsql_user_sync.py +python3 -m py_compile tools/pgsql_user_sync/proxysql_pgsql_user_sync.py +git add tools/pgsql_user_sync/proxysql_pgsql_user_sync.py tools/pgsql_user_sync/tests/test_pgsql_user_sync.py +git commit -m "feat: validate PostgreSQL user sync input" +``` + +Expected: all configuration/snapshot tests pass and byte compilation succeeds. + +--- + +### Task 2: Pure ownership and reconciliation planner + +**Files:** +- Modify: `tools/pgsql_user_sync/proxysql_pgsql_user_sync.py` +- Modify: `tools/pgsql_user_sync/tests/test_pgsql_user_sync.py` + +**Interfaces:** +- Consumes: `SourceRole`, `SyncSettings`, and `SyncError`. +- Produces: frozen `ProxySQLUser`, `SyncAction`, and `SyncPlan` records plus `ActionKind`. +- Produces: `decode_ownership(attributes: str) -> str | None` and `with_ownership(attributes: str, profile: str) -> str`. +- Produces: `build_plan(source, main, runtime, settings) -> SyncPlan`. + +- [ ] **Step 1: Write failing creation/update tests** + +Use a helper populating every policy field. Verify new users use the configured hostgroup/defaults and existing managed users change only password, active, and ownership: + +```python +def test_create_uses_configured_hostgroup(self): + plan = self.mod.build_plan( + {"alice": self.role("alice")}, [], [], self.settings(default_hostgroup=17) + ) + action = plan.actions[0] + self.assertEqual(self.mod.ActionKind.CREATE, action.kind) + self.assertEqual(17, action.after.default_hostgroup) + self.assertEqual((1, 1), (action.after.backend, action.after.frontend)) +``` + +- [ ] **Step 2: Run focused tests and verify they fail** + +```bash +python3 -m unittest -v tools.pgsql_user_sync.tests.test_pgsql_user_sync.PlannerTests +``` + +Expected: `ProxySQLUser`/`build_plan` are undefined. + +- [ ] **Step 3: Implement row/action records and ownership helpers** + +```python +@dataclass(frozen=True) +class ProxySQLUser: + username: str + password: str | None + active: int + use_ssl: int + default_hostgroup: int + transaction_persistent: int + fast_forward: int + backend: int + frontend: int + max_connections: int + attributes: str + comment: str + +class ActionKind(Enum): + CREATE = "create" + UPDATE = "update" + DISABLE = "disable" + +@dataclass(frozen=True) +class SyncAction: + kind: ActionKind + before: ProxySQLUser | None + after: ProxySQLUser + +@dataclass(frozen=True) +class SyncPlan: + actions: tuple[SyncAction, ...] + requires_load: bool + counts: Mapping[str, int] +``` + +Treat empty attributes as `{}`. Reject non-object JSON, malformed ownership, and another profile. Serialize changed JSON with sorted keys and compact separators. + +- [ ] **Step 4: Write failing conflict/missing/divergence tests** + +Cover unmanaged conflict/adoption, cross-profile conflict, duplicate Admin rows, `disable`, `keep`, reactivation, unchanged rows, managed runtime drift, and unrelated pending Admin changes: + +```python +def test_missing_action_is_configurable(self): + owned = self.user("alice", profile="p", active=1) + disabled = self.mod.build_plan({}, [owned], [owned], self.settings(profile="p")) + kept = self.mod.build_plan( + {}, [owned], [owned], self.settings(profile="p", missing_role_action="keep") + ) + self.assertEqual(self.mod.ActionKind.DISABLE, disabled.actions[0].kind) + self.assertEqual((), kept.actions) + +def test_aborts_for_unmanaged_main_runtime_drift(self): + main = self.user("local", password=self.verifier_b) + runtime = replace(main, password=self.verifier_a) + with self.assertRaisesRegex(self.mod.SyncError, "unmanaged.*runtime"): + self.mod.build_plan({}, [main], [runtime], self.settings()) +``` + +- [ ] **Step 5: Implement deterministic planning** + +Index by username and reject multiple rows per name. Compare active main rows to runtime; inactive main rows are expected to be absent. Reject unmanaged active main/runtime drift before planning because `LOAD PGSQL USERS TO RUNTIME` is global. Iterate source and missing managed usernames in sorted order. Create combined frontend/backend rows, preserve policy with `dataclasses.replace`, and require load when actions exist or managed active projections differ. + +- [ ] **Step 6: Run planner tests and commit** + +```bash +python3 -m unittest -v tools/pgsql_user_sync/tests/test_pgsql_user_sync.py +git add tools/pgsql_user_sync/proxysql_pgsql_user_sync.py tools/pgsql_user_sync/tests/test_pgsql_user_sync.py +git commit -m "feat: plan managed PostgreSQL user reconciliation" +``` + +Expected: validation and planner tests pass. + +--- + +### Task 3: Database adapters and safe execution + +**Files:** +- Modify: `tools/pgsql_user_sync/proxysql_pgsql_user_sync.py` +- Modify: `tools/pgsql_user_sync/tests/test_pgsql_user_sync.py` + +**Interfaces:** +- Consumes: all records/functions from Tasks 1–2. +- Produces: `PostgreSQLSource.fetch_snapshot() -> list[tuple[str, str]]`. +- Produces: `ProxySQLAdmin.fetch_main_users()`, `fetch_runtime_users()`, `apply_actions()`, `load_runtime()`, and `save_to_disk()`. +- Produces: `RunSummary`, `run_sync(config: AppConfig, source: SourceAdapter, admin: AdminAdapter, *, dry_run: bool, verbose: bool) -> RunSummary`, `exclusive_lock(path: Path)`, and `main(argv: Sequence[str] | None = None) -> int`. + +- [ ] **Step 1: Write failing adapter SQL tests** + +Use fake DB-API connections/cursors to assert identifier composition, exact row projection, parameterized Admin writes, and lazy imports: + +```python +def test_admin_update_uses_bound_parameters(self): + adapter = self.mod.ProxySQLAdmin(self.config, connect=self.fake_connect) + adapter.apply_actions((self.update_action(),)) + sql, params = self.cursor.executions[-1] + self.assertIn("password=%s", sql) + self.assertNotIn(self.verifier_b, sql) + self.assertEqual(self.verifier_b, params[0]) +``` + +- [ ] **Step 2: Run adapter tests and verify they fail** + +```bash +python3 -m unittest -v tools.pgsql_user_sync.tests.test_pgsql_user_sync.AdapterTests +``` + +Expected: adapters are undefined. + +- [ ] **Step 3: Implement narrow database adapters** + +Compose the source call as identifiers: + +```python +query = sql.SQL("SELECT username::text, password FROM {}.{}()").format( + sql.Identifier(config.function_schema), sql.Identifier(config.function_name) +) +``` + +Select `username,password,active,use_ssl,default_hostgroup,transaction_persistent,fast_forward,backend,frontend,max_connections,attributes,comment` from both Admin tables. Create with explicit columns, update/disable by `(username, backend)`, and use bound parameters. Open Admin with autocommit. Wrap driver failures in fixed, credential-free `SyncError` messages. + +Define the orchestration boundaries explicitly: + +```python +class SourceAdapter(Protocol): + def fetch_snapshot(self) -> list[tuple[str, str]]: ... + +class AdminAdapter(Protocol): + def fetch_main_users(self) -> list[ProxySQLUser]: ... + def fetch_runtime_users(self) -> list[ProxySQLUser]: ... + def apply_actions(self, actions: Sequence[SyncAction]) -> None: ... + def load_runtime(self) -> None: ... + def save_to_disk(self) -> None: ... + +@dataclass(frozen=True) +class RunSummary: + outcome: str + counts: Mapping[str, int] + loaded: bool + saved: bool + duration_seconds: float +``` + +The ellipses above are Python `Protocol` method bodies, not deferred behavior; +the concrete `PostgreSQLSource` and `ProxySQLAdmin` classes implement every +method in this task. + +- [ ] **Step 4: Write failing orchestration/lock/redaction tests** + +Cover source failure, dry-run, action failure with no load, load on actions/drift, save only after load, save failure after successful load, lock contention, summary counts, and secret redaction: + +```python +def test_write_failure_never_loads_runtime(self): + admin = FakeAdmin(fail_apply=True) + with self.assertRaises(self.mod.SyncError): + self.mod.run_sync(self.config, FakeSource(self.rows), admin, + dry_run=False, verbose=False) + self.assertNotIn("load", admin.calls) + self.assertNotIn("save", admin.calls) +``` + +- [ ] **Step 5: Implement orchestration, lock, summaries, and exits** + +Use `os.open(path, os.O_CREAT | os.O_RDWR, 0o600)` and `fcntl.flock(fd, LOCK_EX | LOCK_NB)`. Execute fetch → validate → fetch Admin → plan → optional apply → optional load → optional save. Return a frozen summary containing outcome, counts, loaded, saved, and duration. Print one summary line; return `0` for success/dry-run/lock contention and `1` for failures. + +- [ ] **Step 6: Run tests/smokes and commit** + +```bash +python3 -m unittest -v tools/pgsql_user_sync/tests/test_pgsql_user_sync.py +python3 tools/pgsql_user_sync/proxysql_pgsql_user_sync.py --help +python3 -m py_compile tools/pgsql_user_sync/proxysql_pgsql_user_sync.py +git add tools/pgsql_user_sync/proxysql_pgsql_user_sync.py tools/pgsql_user_sync/tests/test_pgsql_user_sync.py +git commit -m "feat: synchronize PostgreSQL credentials into ProxySQL" +``` + +Expected: tests pass, help exits zero without installed drivers, and byte compilation succeeds. + +--- + +### Task 4: Deployable SQL, configuration, dependencies, and guide + +**Files:** +- Create: `tools/pgsql_user_sync/create_source_function.sql` +- Create: `tools/pgsql_user_sync/proxysql_pgsql_user_sync.ini.example` +- Create: `tools/pgsql_user_sync/requirements.txt` +- Create: `tools/pgsql_user_sync/README.md` +- Modify: `tools/pgsql_user_sync/tests/test_pgsql_user_sync.py` + +**Interfaces:** +- Consumes: config/CLI behavior from Tasks 1–3. +- Produces: role `proxysql_auth_managed`, schema `proxysql_auth`, function `proxysql_auth.export_login_roles()`, and reader `proxysql_auth_reader`. + +- [ ] **Step 1: Write failing asset-contract tests** + +Read assets as text. Assert the SQL fixes `search_path`, revokes PUBLIC, filters login/expiration/null verifiers, and uses allow-list membership. Copy/substitute the example INI to a mode-`0600` temporary file and load it. Assert README Scheduler paths are absolute and both cluster modes are present: + +```python +def test_source_function_has_security_boundaries(self): + sql = (ASSET_DIR / "create_source_function.sql").read_text() + for required in ("SECURITY DEFINER", "SET search_path = pg_catalog", + "rolcanlogin", "rolvaliduntil", "REVOKE ALL"): + self.assertIn(required, sql) +``` + +- [ ] **Step 2: Run asset tests and verify they fail** + +```bash +python3 -m unittest -v tools.pgsql_user_sync.tests.test_pgsql_user_sync.AssetTests +``` + +Expected: four asset files are absent. + +- [ ] **Step 3: Create source SQL and example configuration** + +Use guarded PL/pgSQL blocks for rerunnable role creation. Revoke default schema/function access before exact reader grants. The function selection is: + +```sql +SELECT r.rolname::text AS username, r.rolpassword AS password +FROM pg_catalog.pg_authid AS r +WHERE r.rolcanlogin + AND r.rolpassword IS NOT NULL + AND (r.rolvaliduntil IS NULL OR r.rolvaliduntil > pg_catalog.now()) + AND pg_catalog.pg_has_role(r.oid, 'proxysql_auth_managed', 'member') +ORDER BY r.rolname; +``` + +Use obvious placeholder secrets and instruct operators to replace them. + +- [ ] **Step 4: Write requirements and operations README** + +`requirements.txt` contains: + +```text +psycopg[binary]>=3.2.13,<4 +PyMySQL>=1.1.1,<2 +``` + +Document installation, file permissions, allow-list grant/revoke, dry-run, manual execution, absolute Scheduler SQL, non-transactional main/runtime recovery, `disable|keep`, adoption, global-load/unmanaged-divergence guard, log guarantees, disk persistence, and the two mutually exclusive cluster models. + +- [ ] **Step 5: Run tests and commit** + +```bash +python3 -m unittest -v tools/pgsql_user_sync/tests/test_pgsql_user_sync.py +git add tools/pgsql_user_sync +git commit -m "docs: add PostgreSQL user sync deployment sample" +``` + +Expected: all tests pass. + +--- + +### Task 5: Register dependency-free unit coverage + +**Files:** +- Create: `test/tap/tests/pgsql-user-sync-unit-t.py` +- Modify: `test/tap/groups/groups.json` + +**Interfaces:** +- Consumes: `tools/pgsql_user_sync/tests/test_pgsql_user_sync.py`. +- Produces: executable `pgsql-user-sync-unit-t` in `no-infra-g1`. + +- [ ] **Step 1: Create the harness wrapper and registration** + +```python +#!/usr/bin/env python3 +import os +import subprocess +import sys +from pathlib import Path + +root = Path(os.environ.get("WORKSPACE", Path(__file__).resolve().parents[3])) +suite = root / "tools/pgsql_user_sync/tests/test_pgsql_user_sync.py" +raise SystemExit(subprocess.call( + [sys.executable, "-m", "unittest", "-v", str(suite)], cwd=root +)) +``` + +Add this alphabetically to `groups.json`: + +```json +"pgsql-user-sync-unit-t" : [ "no-infra-g1" ] +``` + +- [ ] **Step 2: Build/run wrapper and lint registration** + +```bash +make -C test/tap/tests pgsql-user-sync-unit-t +test/tap/tests/pgsql-user-sync-unit-t +python3 test/tap/groups/lint_groups_json.py +``` + +Expected: canonical unit tests and group lint pass. + +- [ ] **Step 3: Commit harness registration** + +```bash +git add test/tap/tests/pgsql-user-sync-unit-t.py test/tap/groups/groups.json +git commit -m "test: register PostgreSQL user sync unit suite" +``` + +--- + +### Task 6: PostgreSQL/ProxySQL lifecycle integration test + +**Files:** +- Create: `test/tap/tests/pgsql-user-sync-t.py` +- Modify: `test/tap/groups/groups.json` +- Modify: `test/infra/docker-base/Dockerfile` + +**Interfaces:** +- Consumes: the CLI plus `TAP_ADMIN*`, `TAP_PGSQLSERVER_*`, and `TAP_PGSQL_*` environment variables. +- Produces: executable `pgsql-user-sync-t` in `legacy-g4`. +- Consumes optional `TAP_EXPECT_PGSQL_VERIFIER_AUTH=1`; when set, verifier-auth failures are failures rather than dependency skips. + +- [ ] **Step 1: Add test-runner psycopg dependency** + +Add this build argument beside `MYSQL_CONNECTOR_PYTHON_VERSION`: + +```dockerfile +ARG PSYCOPG_VERSION=3.2.13 +``` + +Add the exact argument `"psycopg[binary]==${PSYCOPG_VERSION}"` immediately +after `fastcov` in the existing `pip3 install --break-system-packages` command, +before the two MySQL connector arguments. + +- [ ] **Step 2: Write cleanup-safe lifecycle test** + +The executable must: + +1. connect directly to PostgreSQL as `TAP_PGSQLSERVER_USERNAME`; +2. create uniquely prefixed allow-list, reader, test role, schema, and source function; +3. create a mode-`0600` temporary INI using `TAP_ADMIN*` for MySQL Admin; +4. create an unmanaged ProxySQL control row with non-default policy; +5. build `command = [sys.executable, str(SCRIPT), "--config", str(config_path)]` + and invoke it with `subprocess.run(command, text=True, capture_output=True)`, + never `shell=True`; +6. verify main/runtime creation, hostgroup, verifier equality, and ownership; +7. rotate the source password, rerun, and verify the verifier changes; +8. revoke `LOGIN`, run `disable`, and verify main inactive/runtime absent; +9. restore `LOGIN`, sync, remove eligibility, run `keep`, and verify active; +10. break source function execution and verify runtime rows remain identical; +11. verify the unmanaged control row remains identical; and +12. restore/delete ProxySQL rows and drop all PostgreSQL objects in `finally`. + +Never put credentials in assertion messages or captured command displays. + +- [ ] **Step 3: Add conditional frontend-auth assertions** + +After creation/rotation, connect via `TAP_PGSQL_HOST:TAP_PGSQL_PORT`. When `TAP_EXPECT_PGSQL_VERIFIER_AUTH=1`, require new-password success and old-password failure. Otherwise, report a clear TAP-style verifier dependency skip if the initial capability probe fails, but continue every control-plane assertion. + +- [ ] **Step 4: Register/build/lint integration test** + +Add alphabetically: + +```json +"pgsql-user-sync-t" : [ "legacy-g4" ] +``` + +Run: + +```bash +make -C test/tap/tests pgsql-user-sync-t +python3 test/tap/groups/lint_groups_json.py +``` + +Expected: executable copied without `.py`, executable bit set, group lint passes. + +- [ ] **Step 5: Run isolated test** + +```bash +PROXYSQL31=1 make debug +PROXYSQL31=1 make build_tap_test_debug +docker build --network host -t proxysql-ci-base:latest test/infra/docker-base +WORKSPACE=$(pwd) INFRA_ID=dev-$USER TAP_GROUP=legacy-g4 \ + TEST_PY_TAP_INCL="pgsql-user-sync-t" \ + test/infra/control/run-tests-isolated.bash +``` + +Expected: lifecycle passes; verifier frontend assertions pass on a capable build or report only the explicit dependency skip. + +- [ ] **Step 6: Commit integration coverage** + +```bash +git add test/infra/docker-base/Dockerfile test/tap/groups/groups.json test/tap/tests/pgsql-user-sync-t.py +git commit -m "test: cover PostgreSQL user synchronization lifecycle" +``` + +--- + +### Task 7: Final security and regression verification + +**Files:** +- Modify only files from Tasks 1–6 if verification exposes a defect. + +**Interfaces:** +- Consumes: all prior artifacts. +- Produces: a verified branch with no credential leakage or unrelated staged files. + +- [ ] **Step 1: Run complete fast suite/static checks** + +```bash +python3 -m unittest -v tools/pgsql_user_sync/tests/test_pgsql_user_sync.py +python3 -m py_compile \ + tools/pgsql_user_sync/proxysql_pgsql_user_sync.py \ + test/tap/tests/pgsql-user-sync-unit-t.py \ + test/tap/tests/pgsql-user-sync-t.py +python3 test/tap/groups/lint_groups_json.py +git diff --check HEAD~6..HEAD +``` + +Expected: all checks pass. + +- [ ] **Step 2: Audit secrets and SQL construction** + +```bash +rg -n 'password|verifier|execute\(|connect\(' \ + tools/pgsql_user_sync test/tap/tests/pgsql-user-sync-t.py +``` + +Confirm passwords are only config/parameter values, SQL data is bound, function names use identifier composition, password fields use `repr=False`, and logs contain no credential material. + +- [ ] **Step 3: Re-run isolated integration test after any fixes** + +```bash +WORKSPACE=$(pwd) INFRA_ID=dev-$USER TAP_GROUP=legacy-g4 \ + TEST_PY_TAP_INCL="pgsql-user-sync-t" \ + test/infra/control/run-tests-isolated.bash +``` + +Expected: pass or explicit verifier-capability skip only. + +- [ ] **Step 4: Confirm scope and commit verification fixes if any** + +```bash +git status --short +git diff --stat HEAD~6..HEAD +``` + +Only plan files plus `tools/pgsql_user_sync`, the two TAP Python sources, groups registration, and docker-base dependency may change. If fixes were required: + +```bash +git add tools/pgsql_user_sync test/tap/tests/pgsql-user-sync-unit-t.py \ + test/tap/tests/pgsql-user-sync-t.py test/tap/groups/groups.json \ + test/infra/docker-base/Dockerfile +git commit -m "fix: harden PostgreSQL user synchronizer" +``` From 4cf5d72f62417a6ba18ca018378188529cecf5e6 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Wed, 12 Aug 2026 15:17:04 +0000 Subject: [PATCH 03/18] feat: validate PostgreSQL user sync input --- .../proxysql_pgsql_user_sync.py | 354 ++++++++++++++++++ .../tests/test_pgsql_user_sync.py | 267 +++++++++++++ 2 files changed, 621 insertions(+) create mode 100644 tools/pgsql_user_sync/proxysql_pgsql_user_sync.py create mode 100644 tools/pgsql_user_sync/tests/test_pgsql_user_sync.py diff --git a/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py b/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py new file mode 100644 index 0000000000..ea79b2b4ce --- /dev/null +++ b/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py @@ -0,0 +1,354 @@ +#!/usr/bin/env python3 +"""Validate configuration and PostgreSQL role-verifier snapshots. + +The module deliberately has no third-party imports at module load time. The +database adapters used by later stages import their drivers only when they are +called. +""" + +import argparse +import base64 +import configparser +import os +import re +import stat +from collections.abc import Iterable, Sequence +from dataclasses import dataclass, field +from pathlib import Path + + +class SyncError(RuntimeError): + """An expected, safe-to-report synchronizer error.""" + + +@dataclass(frozen=True) +class CLIOverrides: + default_hostgroup: int | None = None + missing_role_action: str | None = None + save_to_disk: bool | None = None + + +@dataclass(frozen=True) +class SourceConfig: + host: str + port: int + database: str + username: str + password: str = field(repr=False) + connect_timeout: int = 10 + function_schema: str = "proxysql_auth" + function_name: str = "export_login_roles" + + +@dataclass(frozen=True) +class ProxySQLConfig: + host: str + port: int + username: str + password: str = field(repr=False) + connect_timeout: int = 10 + + +@dataclass(frozen=True) +class SyncSettings: + profile: str + default_hostgroup: int = 0 + missing_role_action: str = "disable" + adopt_existing_users: bool = False + allow_empty_snapshot: bool = False + save_to_disk: bool = True + lock_file: Path = Path("/run/lock/proxysql-pgsql-user-sync.lock") + + +@dataclass(frozen=True) +class AppConfig: + source: SourceConfig + proxysql: ProxySQLConfig + sync: SyncSettings + + +@dataclass(frozen=True) +class SourceRole: + username: str + password: str = field(repr=False) + + +PROFILE_RE = re.compile(r"[A-Za-z0-9][A-Za-z0-9_.-]{0,63}\Z") +IDENTIFIER_RE = re.compile(r"[A-Za-z_][A-Za-z0-9_]*\Z") +MD5_RE = re.compile(r"md5[0-9a-f]{32}\Z") +SCRAM_RE = re.compile( + r"SCRAM-SHA-256\$(?P[0-9]+):(?P[^$]+)\$" + r"(?P[^:]+):(?P[^:]+)\Z" +) + + +def _error(message: str) -> SyncError: + return SyncError(message) + + +def _required(section: configparser.SectionProxy, name: str) -> str: + try: + value = section[name].strip() + except KeyError: + raise _error(f"missing required configuration field {name!r}") from None + if not value: + raise _error(f"configuration field {name!r} must not be empty") + return value + + +def _int_value(section: configparser.SectionProxy, name: str, default: int | None = None) -> int: + if name not in section: + if default is None: + raise _error(f"missing required configuration field {name!r}") + return default + raw = section[name].strip() + try: + return int(raw, 10) + except (TypeError, ValueError): + raise _error(f"configuration field {name!r} must be an integer") from None + + +def _boolean_value( + section: configparser.SectionProxy, name: str, default: bool | None = None +) -> bool: + if name not in section: + if default is None: + raise _error(f"missing required configuration field {name!r}") + return default + raw = section[name].strip().lower() + if raw == "true": + return True + if raw == "false": + return False + raise _error(f"configuration field {name!r} must be true or false") + + +def _section(parser: configparser.ConfigParser, name: str) -> configparser.SectionProxy: + if not parser.has_section(name): + raise _error(f"missing required configuration section [{name}]") + return parser[name] + + +def _validate_endpoint(section: configparser.SectionProxy, section_name: str) -> tuple[str, int]: + host = _required(section, "host") + port = _int_value(section, "port") + if not 1 <= port <= 65535: + raise _error(f"[{section_name}].port must be between 1 and 65535") + return host, port + + +def _validate_timeout(section: configparser.SectionProxy, name: str = "connect_timeout") -> int: + timeout = _int_value(section, name, 10) + if timeout <= 0: + raise _error(f"configuration field {name!r} must be positive") + return timeout + + +def _validate_file(path: Path) -> Path: + try: + info = path.stat() + except OSError: + raise _error("configuration file cannot be read") from None + if not stat.S_ISREG(info.st_mode): + raise _error("configuration file must be a regular file") + if not os.access(path, os.R_OK): + raise _error("configuration file is not readable") + # Owner permissions are unrestricted. Group read is permitted (0640), + # while group write/execute and every other-user permission are rejected. + if info.st_mode & 0o037: + raise _error("configuration file has unsafe permissions") + return path + + +def load_config(path: Path, overrides: CLIOverrides) -> AppConfig: + """Read and validate a protected INI configuration file.""" + + if not isinstance(path, Path): + path = Path(path) + path = _validate_file(path) + parser = configparser.ConfigParser(interpolation=None) + try: + with path.open("r", encoding="utf-8") as stream: + parser.read_file(stream) + except (OSError, UnicodeError, configparser.Error): + raise _error("configuration file cannot be parsed") from None + + source_section = _section(parser, "source") + proxy_section = _section(parser, "proxysql") + sync_section = _section(parser, "sync") + + source_host, source_port = _validate_endpoint(source_section, "source") + proxy_host, proxy_port = _validate_endpoint(proxy_section, "proxysql") + source_function = source_section.get("function", "proxysql_auth.export_login_roles").strip() + function_parts = source_function.split(".", 1) + if len(function_parts) != 2 or not all(IDENTIFIER_RE.fullmatch(part) for part in function_parts): + raise _error("[source].function must be a schema-qualified identifier pair") + + profile = _required(sync_section, "profile") + if PROFILE_RE.fullmatch(profile) is None: + raise _error("[sync].profile has invalid syntax") + + default_hostgroup = _int_value(sync_section, "default_hostgroup", 0) + if default_hostgroup < 0: + raise _error("[sync].default_hostgroup must be non-negative") + missing_role_action = sync_section.get("missing_role_action", "disable").strip().lower() + if missing_role_action not in {"disable", "keep"}: + raise _error("[sync].missing_role_action must be disable or keep") + adopt_existing_users = _boolean_value(sync_section, "adopt_existing_users", False) + allow_empty_snapshot = _boolean_value(sync_section, "allow_empty_snapshot", False) + save_to_disk = _boolean_value(sync_section, "save_to_disk", True) + lock_file_value = sync_section.get("lock_file", str(SyncSettings.lock_file)).strip() + if not lock_file_value: + raise _error("[sync].lock_file must not be empty") + lock_file = Path(lock_file_value) + + if overrides.default_hostgroup is not None: + if not isinstance(overrides.default_hostgroup, int) or overrides.default_hostgroup < 0: + raise _error("default_hostgroup override must be non-negative") + default_hostgroup = overrides.default_hostgroup + if overrides.missing_role_action is not None: + missing_role_action = overrides.missing_role_action.strip().lower() + if missing_role_action not in {"disable", "keep"}: + raise _error("missing_role_action override must be disable or keep") + if overrides.save_to_disk is not None: + if not isinstance(overrides.save_to_disk, bool): + raise _error("save_to_disk override must be boolean") + save_to_disk = overrides.save_to_disk + + return AppConfig( + source=SourceConfig( + host=source_host, + port=source_port, + database=_required(source_section, "database"), + username=_required(source_section, "username"), + password=_required(source_section, "password"), + connect_timeout=_validate_timeout(source_section), + function_schema=function_parts[0], + function_name=function_parts[1], + ), + proxysql=ProxySQLConfig( + host=proxy_host, + port=proxy_port, + username=_required(proxy_section, "username"), + password=_required(proxy_section, "password"), + connect_timeout=_validate_timeout(proxy_section), + ), + sync=SyncSettings( + profile=profile, + default_hostgroup=default_hostgroup, + missing_role_action=missing_role_action, + adopt_existing_users=adopt_existing_users, + allow_empty_snapshot=allow_empty_snapshot, + save_to_disk=save_to_disk, + lock_file=lock_file, + ), + ) + + +def parse_args(argv: Sequence[str]) -> argparse.Namespace: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--config", required=True, type=Path) + parser.add_argument("--default-hostgroup", type=int, default=None) + parser.add_argument("--missing-role-action", choices=("disable", "keep"), default=None) + save_group = parser.add_mutually_exclusive_group() + save_group.add_argument("--save-to-disk", dest="save_to_disk", action="store_true") + save_group.add_argument("--no-save-to-disk", dest="save_to_disk", action="store_false") + parser.set_defaults(save_to_disk=None) + parser.add_argument("--dry-run", action="store_true", default=False) + parser.add_argument("--verbose", action="store_true", default=False) + return parser.parse_args(argv) + + +def _decode_base64(value: str) -> bytes | None: + try: + decoded = base64.b64decode(value.encode("ascii"), validate=True) + except (UnicodeEncodeError, ValueError): + return None + # Reject alternate/non-canonical encodings in addition to non-alphabet + # characters. PostgreSQL emits standard padded base64 in SCRAM values. + if base64.b64encode(decoded).decode("ascii") != value: + return None + return decoded + + +def validate_verifier(value: str) -> None: + """Validate a PostgreSQL MD5 or SCRAM-SHA-256 verifier. + + Error messages intentionally describe only the class of failure and never + include the credential verifier. + """ + + if not isinstance(value, str): + raise _error("password verifier must be a string") + if MD5_RE.fullmatch(value): + return + match = SCRAM_RE.fullmatch(value) + if match is None: + raise _error("invalid password verifier syntax") + try: + iterations = int(match.group("iterations"), 10) + except ValueError: + raise _error("invalid SCRAM iteration count") from None + if not 1 <= iterations <= 2_147_483_647: + raise _error("invalid SCRAM iteration count") + salt = _decode_base64(match.group("salt")) + stored = _decode_base64(match.group("stored")) + server = _decode_base64(match.group("server")) + if not salt: + raise _error("invalid SCRAM salt") + if stored is None or server is None or len(stored) != 32 or len(server) != 32: + raise _error("invalid SCRAM key length") + + +def validate_snapshot(rows: Iterable[Sequence[object]], allow_empty: bool) -> dict[str, SourceRole]: + """Validate a complete two-column role snapshot and return it sorted.""" + + result: dict[str, SourceRole] = {} + count = 0 + try: + iterator = iter(rows) + except TypeError: + raise _error("snapshot must be iterable") from None + for row in iterator: + count += 1 + if isinstance(row, (str, bytes, bytearray)) or not isinstance(row, Sequence): + raise _error("snapshot row must contain exactly two columns") + if len(row) != 2: + raise _error("snapshot row must contain exactly two columns") + username, password = row + if not isinstance(username, str) or not isinstance(password, str): + raise _error("snapshot username and password must be strings") + if not username: + raise _error("snapshot username must not be empty") + if "\x00" in username: + raise _error("snapshot username must not contain NUL") + try: + username_bytes = username.encode("utf-8") + except UnicodeEncodeError: + raise _error("snapshot username must be valid UTF-8") from None + if len(username_bytes) > 63: + raise _error("snapshot username exceeds 63 UTF-8 bytes") + if username in result: + raise _error("snapshot contains duplicate username") + validate_verifier(password) + result[username] = SourceRole(username=username, password=password) + if count == 0 and not allow_empty: + raise _error("snapshot is empty") + return dict(sorted(result.items())) + + +__all__ = [ + "AppConfig", + "CLIOverrides", + "PROFILE_RE", + "IDENTIFIER_RE", + "ProxySQLConfig", + "SourceConfig", + "SourceRole", + "SyncError", + "SyncSettings", + "load_config", + "parse_args", + "validate_snapshot", + "validate_verifier", +] diff --git a/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py b/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py new file mode 100644 index 0000000000..eafe620905 --- /dev/null +++ b/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py @@ -0,0 +1,267 @@ +import base64 +import importlib.util +import os +import sys +import tempfile +import unittest +from pathlib import Path + + +SCRIPT = Path(__file__).parents[1] / "proxysql_pgsql_user_sync.py" + + +def load_module(): + spec = importlib.util.spec_from_file_location("proxysql_pgsql_user_sync", SCRIPT) + module = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = module + spec.loader.exec_module(module) + return module + + +class ConfigTests(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.mod = load_module() + + def write_config(self, *, mode=0o600, text=None): + if text is None: + text = """\ +[source] +host = postgresql.example +port = 5432 +database = postgres +username = source_reader +password = source-secret + +[proxysql] +host = 127.0.0.1 +port = 6032 +username = admin +password = admin-secret + +[sync] +profile = primary-cluster +""" + handle = tempfile.NamedTemporaryFile(mode="w", delete=False) + self.addCleanup(lambda: os.unlink(handle.name)) + with handle: + handle.write(text) + os.chmod(handle.name, mode) + return Path(handle.name) + + def test_defaults_and_cli_hostgroup_override(self): + path = self.write_config() + cfg = self.mod.load_config(path, self.mod.CLIOverrides(default_hostgroup=12)) + self.assertEqual(12, cfg.sync.default_hostgroup) + self.assertEqual("disable", cfg.sync.missing_role_action) + self.assertFalse(cfg.sync.allow_empty_snapshot) + self.assertTrue(cfg.sync.save_to_disk) + self.assertEqual("proxysql_auth", cfg.source.function_schema) + self.assertEqual("export_login_roles", cfg.source.function_name) + + def test_cli_overrides_take_precedence(self): + path = self.write_config(text="""\ +[source] +host = source +port = 5432 +database = db +username = reader +password = source-password +connect_timeout = 30 +function = auth.roles + +[proxysql] +host = proxy +port = 6032 +username = admin +password = proxy-password +connect_timeout = 20 + +[sync] +profile = p1 +default_hostgroup = 4 +missing_role_action = keep +save_to_disk = true +""") + cfg = self.mod.load_config( + path, + self.mod.CLIOverrides( + default_hostgroup=12, missing_role_action="disable", save_to_disk=False + ), + ) + self.assertEqual(12, cfg.sync.default_hostgroup) + self.assertEqual("disable", cfg.sync.missing_role_action) + self.assertFalse(cfg.sync.save_to_disk) + self.assertEqual("auth", cfg.source.function_schema) + self.assertEqual("roles", cfg.source.function_name) + self.assertEqual(30, cfg.source.connect_timeout) + + def test_rejects_world_readable_config(self): + path = self.write_config(mode=0o604) + with self.assertRaisesRegex(self.mod.SyncError, "permissions"): + self.mod.load_config(path, self.mod.CLIOverrides()) + + def test_accepts_group_read_but_not_group_write(self): + path = self.write_config(mode=0o640) + self.mod.load_config(path, self.mod.CLIOverrides()) + path = self.write_config(mode=0o660) + with self.assertRaisesRegex(self.mod.SyncError, "permissions"): + self.mod.load_config(path, self.mod.CLIOverrides()) + + def test_rejects_invalid_profile_and_function(self): + for profile in ("", "-bad", "a" * 65): + path = self.write_config(text="""\ +[source] +host = source +port = 5432 +database = db +username = reader +password = secret +[proxysql] +host = proxy +port = 6032 +username = admin +password = secret +[sync] +profile = %s +""" % profile) + with self.assertRaisesRegex(self.mod.SyncError, "profile"): + self.mod.load_config(path, self.mod.CLIOverrides()) + + for function in ("roles", "auth.bad-name", "1auth.roles", "auth.roles.extra"): + path = self.write_config(text="""\ +[source] +host = source +port = 5432 +database = db +username = reader +password = secret +function = %s +[proxysql] +host = proxy +port = 6032 +username = admin +password = secret +[sync] +profile = p +""" % function) + with self.assertRaisesRegex(self.mod.SyncError, "function"): + self.mod.load_config(path, self.mod.CLIOverrides()) + + def test_rejects_invalid_values_and_missing_fields(self): + path = self.write_config(text="""\ +[source] +host = source +port = 0 +database = db +username = reader +password = secret +connect_timeout = -1 +[proxysql] +host = proxy +port = 6032 +username = admin +password = secret +[sync] +profile = p +default_hostgroup = -1 +missing_role_action = remove +adopt_existing_users = maybe +allow_empty_snapshot = false +save_to_disk = true +""") + with self.assertRaises(self.mod.SyncError): + self.mod.load_config(path, self.mod.CLIOverrides()) + + path = self.write_config(text="""\ +[source] +host = source +port = 5432 +database = db +username = reader +[proxysql] +host = proxy +port = 6032 +username = admin +password = secret +[sync] +profile = p +""") + with self.assertRaisesRegex(self.mod.SyncError, "password"): + self.mod.load_config(path, self.mod.CLIOverrides()) + + def test_parse_args_supports_overrides(self): + args = self.mod.parse_args( + [ + "--config", + "/tmp/config", + "--default-hostgroup", + "12", + "--missing-role-action", + "keep", + "--no-save-to-disk", + "--dry-run", + "--verbose", + ] + ) + self.assertEqual(Path("/tmp/config"), args.config) + self.assertEqual(12, args.default_hostgroup) + self.assertEqual("keep", args.missing_role_action) + self.assertFalse(args.save_to_disk) + self.assertTrue(args.dry_run) + self.assertTrue(args.verbose) + + +class SnapshotTests(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.mod = load_module() + + def valid_scram(self): + key = base64.b64encode(b"x" * 32).decode() + salt = base64.b64encode(b"salt").decode() + return "SCRAM-SHA-256$4096:%s$%s:%s" % (salt, key, key) + + def test_accepts_lowercase_md5_and_sorts_users(self): + rows = [("zeta", "md5" + "a" * 32), ("alice", "md5" + "b" * 32)] + result = self.mod.validate_snapshot(rows, False) + self.assertEqual(["alice", "zeta"], list(result)) + self.assertEqual("md5" + "b" * 32, result["alice"].password) + + def test_accepts_valid_scram(self): + result = self.mod.validate_snapshot([("alice", self.valid_scram())], False) + self.assertEqual("alice", result["alice"].username) + + def test_rejects_invalid_verifiers_without_leaking_value(self): + bad_values = [ + "MD5" + "a" * 32, + "md5" + "g" * 32, + "md5" + "a" * 31, + "SCRAM-SHA-256$0:c2FsdA==$eA==:eA==", + "SCRAM-SHA-256$4096:not-base64$eA==:eA==", + "SCRAM-SHA-256$4096:c2FsdA==$eA==:eA==", + ] + for value in bad_values: + with self.assertRaises(self.mod.SyncError) as ctx: + self.mod.validate_verifier(value) + self.assertNotIn(value, str(ctx.exception)) + + def test_rejects_duplicate_without_leaking_verifier(self): + verifier = "md5" + "b" * 32 + with self.assertRaisesRegex(self.mod.SyncError, "duplicate") as ctx: + self.mod.validate_snapshot([("alice", verifier), ("alice", verifier)], False) + self.assertNotIn(verifier, str(ctx.exception)) + + def test_empty_snapshot_requires_opt_in(self): + with self.assertRaisesRegex(self.mod.SyncError, "empty"): + self.mod.validate_snapshot([], False) + self.assertEqual({}, self.mod.validate_snapshot([], True)) + + def test_rejects_bad_shape_types_nuls_and_long_utf8_names(self): + for rows in (["not-a-row"], [("alice", "md5" + "a" * 32, "extra")], [(None, "md5" + "a" * 32)], [("a\x00b", "md5" + "a" * 32)], [("é" * 32, "md5" + "a" * 32)]): + with self.assertRaises(self.mod.SyncError): + self.mod.validate_snapshot(rows, True) + + +if __name__ == "__main__": + unittest.main() From c0a20ef2494cda82cc4969e50993dde141a23994 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Wed, 12 Aug 2026 15:29:00 +0000 Subject: [PATCH 04/18] fix: restrict group-readable sync config ownership --- .../proxysql_pgsql_user_sync.py | 14 +++++++--- .../tests/test_pgsql_user_sync.py | 27 ++++++++++++++++--- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py b/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py index ea79b2b4ce..59c660b4f8 100644 --- a/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py +++ b/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py @@ -153,9 +153,17 @@ def _validate_file(path: Path) -> Path: raise _error("configuration file must be a regular file") if not os.access(path, os.R_OK): raise _error("configuration file is not readable") - # Owner permissions are unrestricted. Group read is permitted (0640), - # while group write/execute and every other-user permission are rejected. - if info.st_mode & 0o037: + # Group read is a supported deployment mode only for a root-owned file + # (typically 0640 with a dedicated service group). Non-root-owned files + # must be owner-only. Group write/execute and every other-user permission + # are rejected in all cases. + group_permissions = info.st_mode & 0o070 + other_permissions = info.st_mode & 0o007 + if ( + other_permissions + or group_permissions & 0o030 + or (group_permissions & 0o040 and info.st_uid != 0) + ): raise _error("configuration file has unsafe permissions") return path diff --git a/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py b/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py index eafe620905..69b9e98213 100644 --- a/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py +++ b/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py @@ -1,9 +1,12 @@ import base64 import importlib.util import os +import stat import sys import tempfile +from types import SimpleNamespace import unittest +from unittest.mock import patch from pathlib import Path @@ -101,13 +104,29 @@ def test_rejects_world_readable_config(self): with self.assertRaisesRegex(self.mod.SyncError, "permissions"): self.mod.load_config(path, self.mod.CLIOverrides()) - def test_accepts_group_read_but_not_group_write(self): + def test_group_read_requires_root_owner_and_owner_only_is_allowed(self): path = self.write_config(mode=0o640) - self.mod.load_config(path, self.mod.CLIOverrides()) - path = self.write_config(mode=0o660) - with self.assertRaisesRegex(self.mod.SyncError, "permissions"): + root_metadata = SimpleNamespace(st_mode=stat.S_IFREG | 0o640, st_uid=0) + with patch.object(self.mod.Path, "stat", return_value=root_metadata): self.mod.load_config(path, self.mod.CLIOverrides()) + non_root_metadata = SimpleNamespace(st_mode=stat.S_IFREG | 0o640, st_uid=1000) + with patch.object(self.mod.Path, "stat", return_value=non_root_metadata): + with self.assertRaisesRegex(self.mod.SyncError, "permissions"): + self.mod.load_config(path, self.mod.CLIOverrides()) + + path = self.write_config(mode=0o600) + owner_only_metadata = SimpleNamespace(st_mode=stat.S_IFREG | 0o600, st_uid=1000) + with patch.object(self.mod.Path, "stat", return_value=owner_only_metadata): + self.mod.load_config(path, self.mod.CLIOverrides()) + + def test_rejects_group_write_even_when_root_owned(self): + path = self.write_config(mode=0o660) + root_metadata = SimpleNamespace(st_mode=stat.S_IFREG | 0o660, st_uid=0) + with patch.object(self.mod.Path, "stat", return_value=root_metadata): + with self.assertRaisesRegex(self.mod.SyncError, "permissions"): + self.mod.load_config(path, self.mod.CLIOverrides()) + def test_rejects_invalid_profile_and_function(self): for profile in ("", "-bad", "a" * 65): path = self.write_config(text="""\ From ac8b9004fc6a9bd3e3b91ae1a597c2e27189c8c8 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Wed, 12 Aug 2026 15:34:51 +0000 Subject: [PATCH 05/18] feat: plan managed PostgreSQL user reconciliation --- .../proxysql_pgsql_user_sync.py | 241 +++++++++++++++++- .../tests/test_pgsql_user_sync.py | 140 ++++++++++ 2 files changed, 379 insertions(+), 2 deletions(-) diff --git a/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py b/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py index 59c660b4f8..b98cb31ecc 100644 --- a/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py +++ b/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py @@ -9,12 +9,15 @@ import argparse import base64 import configparser +import json import os import re import stat -from collections.abc import Iterable, Sequence -from dataclasses import dataclass, field +from collections.abc import Iterable, Mapping, Sequence +from dataclasses import dataclass, field, replace +from enum import Enum from pathlib import Path +from types import MappingProxyType class SyncError(RuntimeError): @@ -73,6 +76,42 @@ class SourceRole: password: str = field(repr=False) +@dataclass(frozen=True) +class ProxySQLUser: + username: str + password: str | None = field(repr=False) + active: int + use_ssl: int + default_hostgroup: int + transaction_persistent: int + fast_forward: int + backend: int + frontend: int + max_connections: int + attributes: str + comment: str + + +class ActionKind(Enum): + CREATE = "create" + UPDATE = "update" + DISABLE = "disable" + + +@dataclass(frozen=True) +class SyncAction: + kind: ActionKind + before: ProxySQLUser | None + after: ProxySQLUser + + +@dataclass(frozen=True) +class SyncPlan: + actions: tuple[SyncAction, ...] + requires_load: bool + counts: Mapping[str, int] + + PROFILE_RE = re.compile(r"[A-Za-z0-9][A-Za-z0-9_.-]{0,63}\Z") IDENTIFIER_RE = re.compile(r"[A-Za-z_][A-Za-z0-9_]*\Z") MD5_RE = re.compile(r"md5[0-9a-f]{32}\Z") @@ -345,18 +384,216 @@ def validate_snapshot(rows: Iterable[Sequence[object]], allow_empty: bool) -> di return dict(sorted(result.items())) +_OWNERSHIP_KEY = "proxysql_pgsql_user_sync" + + +def _ownership_document(attributes: str) -> dict[str, object]: + if not isinstance(attributes, str): + raise _error("ProxySQL user attributes must be a JSON object") + if attributes == "": + return {} + try: + document = json.loads(attributes) + except (TypeError, ValueError, json.JSONDecodeError): + raise _error("ProxySQL user attributes must be valid JSON") from None + if not isinstance(document, dict): + raise _error("ProxySQL user attributes must be a JSON object") + return document + + +def decode_ownership(attributes: str) -> str | None: + """Return the managed profile in attributes, rejecting malformed markers.""" + + document = _ownership_document(attributes) + if _OWNERSHIP_KEY not in document: + return None + marker = document[_OWNERSHIP_KEY] + if not isinstance(marker, dict): + raise _error("malformed PostgreSQL user ownership marker") + profile = marker.get("profile") + if not isinstance(profile, str) or PROFILE_RE.fullmatch(profile) is None: + raise _error("malformed PostgreSQL user ownership marker") + return profile + + +def with_ownership(attributes: str, profile: str) -> str: + """Return attributes marked for profile using deterministic JSON.""" + + if not isinstance(profile, str) or PROFILE_RE.fullmatch(profile) is None: + raise _error("invalid ownership profile") + document = _ownership_document(attributes) + current = decode_ownership(attributes) + if current is not None and current != profile: + raise _error("user is owned by another profile") + marker = document.get(_OWNERSHIP_KEY) + if isinstance(marker, dict) and marker.get("profile") == profile: + # Ownership is unchanged; preserve operator formatting and unrelated data. + return attributes + document[_OWNERSHIP_KEY] = {"profile": profile} + return json.dumps(document, sort_keys=True, separators=(",", ":")) + + +def _user_index(rows: Iterable[ProxySQLUser], table: str) -> dict[str, ProxySQLUser]: + result: dict[str, ProxySQLUser] = {} + try: + iterator = iter(rows) + except TypeError: + raise _error(f"{table} users must be iterable") from None + for row in iterator: + if not isinstance(row, ProxySQLUser): + raise _error(f"{table} users contain an invalid row") + # Validate ownership on both snapshots, even when a row is not managed. + decode_ownership(row.attributes) + if row.username in result: + raise _error(f"{table} users contain multiple rows for {row.username!r}") + result[row.username] = row + return result + + +def _managed(row: ProxySQLUser, settings: SyncSettings) -> bool: + return decode_ownership(row.attributes) == settings.profile + + +def _new_user(role: SourceRole, settings: SyncSettings) -> ProxySQLUser: + return ProxySQLUser( + username=role.username, + password=role.password, + active=1, + use_ssl=0, + default_hostgroup=settings.default_hostgroup, + transaction_persistent=1, + fast_forward=0, + backend=1, + frontend=1, + max_connections=10000, + attributes=with_ownership("", settings.profile), + comment="", + ) + + +def build_plan( + source: Mapping[str, SourceRole], + main: Iterable[ProxySQLUser], + runtime: Iterable[ProxySQLUser], + settings: SyncSettings, +) -> SyncPlan: + """Build a deterministic, side-effect-free reconciliation plan.""" + + if not isinstance(source, Mapping): + raise _error("source snapshot must be a username mapping") + source_names = set(source) + for username, role in source.items(): + if not isinstance(username, str) or not isinstance(role, SourceRole): + raise _error("source snapshot contains an invalid role") + if role.username != username: + raise _error("source snapshot username does not match role") + + main_by_name = _user_index(main, "main") + runtime_by_name = _user_index(runtime, "runtime") + managed_active_drift = False + + # LOAD is global. Never overwrite an unrelated user's active runtime state. + for username, main_row in main_by_name.items(): + runtime_row = runtime_by_name.get(username) + managed = _managed(main_row, settings) + if main_row.active: + if runtime_row is None or runtime_row != main_row: + if not managed: + raise _error("unmanaged main/runtime drift") + managed_active_drift = True + elif runtime_row is not None: + if not managed: + raise _error("unmanaged main/runtime drift") + managed_active_drift = True + for username, runtime_row in runtime_by_name.items(): + if username in main_by_name: + continue + if _managed(runtime_row, settings): + managed_active_drift = True + elif runtime_row.active: + raise _error("unmanaged main/runtime drift") + + actions: list[SyncAction] = [] + counts = { + "discovered": len(source), + "created": 0, + "updated": 0, + "reactivated": 0, + "disabled": 0, + "unchanged": 0, + "conflicted": 0, + } + + # Source roles are sorted for stable action order and reproducible output. + for username in sorted(source_names): + role = source[username] + existing = main_by_name.get(username) + if existing is None: + after = _new_user(role, settings) + actions.append(SyncAction(ActionKind.CREATE, None, after)) + counts["created"] += 1 + continue + + owner = decode_ownership(existing.attributes) + if owner is not None and owner != settings.profile: + counts["conflicted"] += 1 + raise _error("user is owned by another profile") + if owner is None and not settings.adopt_existing_users: + counts["conflicted"] += 1 + raise _error("unmanaged user conflicts with source role") + + after = replace( + existing, + password=role.password, + active=1, + attributes=with_ownership(existing.attributes, settings.profile), + ) + if after == existing: + counts["unchanged"] += 1 + else: + actions.append(SyncAction(ActionKind.UPDATE, existing, after)) + counts["updated"] += 1 + if not existing.active: + counts["reactivated"] += 1 + + if settings.missing_role_action == "disable": + for username in sorted(set(main_by_name) - source_names): + existing = main_by_name[username] + if not _managed(existing, settings) or not existing.active: + continue + after = replace(existing, active=0) + actions.append(SyncAction(ActionKind.DISABLE, existing, after)) + counts["disabled"] += 1 + elif settings.missing_role_action != "keep": + raise _error("missing_role_action must be disable or keep") + + actions.sort(key=lambda action: action.after.username) + return SyncPlan( + actions=tuple(actions), + requires_load=bool(actions) or managed_active_drift, + counts=MappingProxyType(counts), + ) + + __all__ = [ "AppConfig", "CLIOverrides", "PROFILE_RE", "IDENTIFIER_RE", "ProxySQLConfig", + "ProxySQLUser", "SourceConfig", "SourceRole", "SyncError", + "SyncAction", + "SyncPlan", "SyncSettings", + "ActionKind", + "build_plan", + "decode_ownership", "load_config", "parse_args", + "with_ownership", "validate_snapshot", "validate_verifier", ] diff --git a/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py b/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py index 69b9e98213..f5ad8aba7e 100644 --- a/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py +++ b/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py @@ -1,9 +1,11 @@ import base64 import importlib.util +import json import os import stat import sys import tempfile +from dataclasses import replace from types import SimpleNamespace import unittest from unittest.mock import patch @@ -282,5 +284,143 @@ def test_rejects_bad_shape_types_nuls_and_long_utf8_names(self): self.mod.validate_snapshot(rows, True) +class PlannerTests(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.mod = load_module() + cls.verifier_a = "md5" + "a" * 32 + cls.verifier_b = "md5" + "b" * 32 + + def settings(self, **kwargs): + values = dict(profile="p", default_hostgroup=0, missing_role_action="disable", + adopt_existing_users=False) + values.update(kwargs) + return self.mod.SyncSettings(**values) + + def role(self, username, password=None): + return self.mod.SourceRole(username, password or self.verifier_a) + + def user(self, username, *, password=None, profile=None, active=1, **kwargs): + attributes = kwargs.pop("attributes", "") + if profile is not None: + attributes = json.dumps({"proxysql_pgsql_user_sync": {"profile": profile}}, + separators=(",", ":")) + values = dict(username=username, password=password or self.verifier_a, active=active, + use_ssl=0, default_hostgroup=3, transaction_persistent=1, + fast_forward=0, backend=1, frontend=1, max_connections=100, + attributes=attributes, comment="kept") + values.update(kwargs) + return self.mod.ProxySQLUser(**values) + + def test_create_uses_configured_hostgroup(self): + plan = self.mod.build_plan( + {"alice": self.role("alice")}, [], [], self.settings(default_hostgroup=17) + ) + action = plan.actions[0] + self.assertEqual(self.mod.ActionKind.CREATE, action.kind) + self.assertEqual(17, action.after.default_hostgroup) + self.assertEqual((1, 1), (action.after.backend, action.after.frontend)) + self.assertEqual(1, action.after.transaction_persistent) + self.assertEqual(10000, action.after.max_connections) + self.assertEqual({"proxysql_pgsql_user_sync": {"profile": "p"}}, + json.loads(action.after.attributes)) + + def test_managed_update_changes_only_password_active_and_ownership(self): + main = self.user("alice", profile="p", password=self.verifier_a, + attributes=json.dumps({"other": {"value": 1}, + "proxysql_pgsql_user_sync": {"profile": "p"}}, + separators=(",", ":")), + active=0, use_ssl=1, default_hostgroup=22, + transaction_persistent=0, fast_forward=1, backend=1, + frontend=1, max_connections=9, comment="operator") + plan = self.mod.build_plan({"alice": self.role("alice", self.verifier_b)}, + [main], [main], self.settings()) + action = plan.actions[0] + self.assertEqual(self.mod.ActionKind.UPDATE, action.kind) + self.assertEqual(self.verifier_b, action.after.password) + self.assertEqual(1, action.after.active) + self.assertEqual((1, 22, 0, 1, 1, 9, "operator"), + (action.after.use_ssl, action.after.default_hostgroup, + action.after.transaction_persistent, action.after.fast_forward, + action.after.backend, action.after.max_connections, + action.after.comment)) + self.assertEqual(json.loads(main.attributes), json.loads(action.after.attributes)) + + def test_missing_action_is_configurable(self): + owned = self.user("alice", profile="p", active=1) + disabled = self.mod.build_plan({}, [owned], [owned], self.settings(profile="p")) + kept = self.mod.build_plan({}, [owned], [owned], + self.settings(profile="p", missing_role_action="keep")) + self.assertEqual(self.mod.ActionKind.DISABLE, disabled.actions[0].kind) + self.assertEqual(0, disabled.actions[0].after.active) + self.assertEqual((), kept.actions) + + def test_aborts_for_unmanaged_main_runtime_drift(self): + main = self.user("local", password=self.verifier_b) + runtime = replace(main, password=self.verifier_a) + with self.assertRaisesRegex(self.mod.SyncError, "unmanaged.*runtime"): + self.mod.build_plan({}, [main], [runtime], self.settings()) + + def test_unmanaged_conflict_requires_adoption(self): + existing = self.user("alice") + with self.assertRaisesRegex(self.mod.SyncError, "unmanaged"): + self.mod.build_plan({"alice": self.role("alice")}, [existing], [existing], + self.settings()) + plan = self.mod.build_plan({"alice": self.role("alice", self.verifier_b)}, + [existing], [existing], + self.settings(adopt_existing_users=True)) + self.assertEqual(self.mod.ActionKind.UPDATE, plan.actions[0].kind) + self.assertEqual(self.verifier_b, plan.actions[0].after.password) + + def test_cross_profile_conflict_aborts(self): + existing = self.user("alice", profile="other") + with self.assertRaisesRegex(self.mod.SyncError, "another profile"): + self.mod.build_plan({"alice": self.role("alice")}, [existing], [existing], + self.settings()) + + def test_duplicate_admin_rows_are_rejected(self): + first = self.user("alice", profile="p") + second = replace(first, backend=0, frontend=1) + with self.assertRaisesRegex(self.mod.SyncError, "multiple.*alice"): + self.mod.build_plan({"alice": self.role("alice")}, [first, second], [], + self.settings()) + + def test_ownership_helpers_validate_and_normalize(self): + self.assertIsNone(self.mod.decode_ownership("")) + self.assertEqual("p", self.mod.decode_ownership( + '{"proxysql_pgsql_user_sync":{"profile":"p"}}')) + self.assertEqual('{"a":1,"proxysql_pgsql_user_sync":{"profile":"p"}}', + self.mod.with_ownership('{"a":1}', "p")) + for bad in ('[]', '{"proxysql_pgsql_user_sync": []}', + '{"proxysql_pgsql_user_sync": null}', + '{"proxysql_pgsql_user_sync": {"profile": 1}}', '{bad'): + with self.assertRaises(self.mod.SyncError): + self.mod.decode_ownership(bad) + with self.assertRaisesRegex(self.mod.SyncError, "another profile"): + self.mod.with_ownership('{"proxysql_pgsql_user_sync":{"profile":"old"}}', "p") + + def test_unchanged_rows_need_no_action_or_load(self): + owned = self.user("alice", profile="p") + plan = self.mod.build_plan({"alice": self.role("alice")}, [owned], [owned], + self.settings()) + self.assertEqual((), plan.actions) + self.assertFalse(plan.requires_load) + self.assertEqual(1, plan.counts["unchanged"]) + + def test_managed_runtime_drift_requires_load(self): + main = self.user("alice", profile="p", active=1) + runtime = replace(main, active=0) + plan = self.mod.build_plan({"alice": self.role("alice")}, [main], [runtime], + self.settings()) + self.assertEqual((), plan.actions) + self.assertTrue(plan.requires_load) + + def test_inactive_main_row_is_expected_absent_from_runtime(self): + main = self.user("alice", profile="p", active=0) + plan = self.mod.build_plan({}, [main], [], self.settings()) + self.assertEqual((), plan.actions) + self.assertFalse(plan.requires_load) + + if __name__ == "__main__": unittest.main() From 6f9d2d453438f1b854cb00577535e9983393bfd9 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Wed, 12 Aug 2026 15:41:19 +0000 Subject: [PATCH 06/18] feat: synchronize PostgreSQL credentials into ProxySQL --- .../proxysql_pgsql_user_sync.py | 325 ++++++++++++++++++ .../tests/test_pgsql_user_sync.py | 262 ++++++++++++++ 2 files changed, 587 insertions(+) diff --git a/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py b/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py index b98cb31ecc..5eafa701be 100644 --- a/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py +++ b/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py @@ -9,15 +9,20 @@ import argparse import base64 import configparser +import fcntl import json import os import re import stat +import sys +import time from collections.abc import Iterable, Mapping, Sequence +from contextlib import contextmanager from dataclasses import dataclass, field, replace from enum import Enum from pathlib import Path from types import MappingProxyType +from typing import Callable, Protocol class SyncError(RuntimeError): @@ -575,6 +580,314 @@ def build_plan( ) +class SourceAdapter(Protocol): + def fetch_snapshot(self) -> list[tuple[str, str]]: ... + + +class AdminAdapter(Protocol): + def fetch_main_users(self) -> list[ProxySQLUser]: ... + + def fetch_runtime_users(self) -> list[ProxySQLUser]: ... + + def apply_actions(self, actions: Sequence[SyncAction]) -> None: ... + + def load_runtime(self) -> None: ... + + def save_to_disk(self) -> None: ... + + +@dataclass(frozen=True) +class RunSummary: + outcome: str + counts: Mapping[str, int] + loaded: bool + saved: bool + duration_seconds: float + + +def _close_connection(connection: object) -> None: + try: + connection.close() + except Exception: + pass + + +class PostgreSQLSource: + """Fetch the authoritative PostgreSQL role verifier snapshot.""" + + def __init__( + self, + config: AppConfig, + *, + connect: Callable[..., object] | None = None, + sql_module: object | None = None, + ) -> None: + self.config = config + self._connect = connect + self._sql_module = sql_module + + def _driver(self) -> tuple[Callable[..., object], object]: + if self._connect is not None and self._sql_module is not None: + return self._connect, self._sql_module + try: + import psycopg2 + from psycopg2 import sql + except ImportError: + raise _error("PostgreSQL driver is not installed") from None + return self._connect or psycopg2.connect, self._sql_module or sql + + def fetch_snapshot(self) -> list[tuple[str, str]]: + try: + connect, sql = self._driver() + source = self.config.source + connection = connect( + host=source.host, + port=source.port, + dbname=source.database, + user=source.username, + password=source.password, + connect_timeout=source.connect_timeout, + ) + try: + cursor = connection.cursor() + query = sql.SQL("SELECT username::text, password FROM {}.{}()").format( + sql.Identifier(source.function_schema), sql.Identifier(source.function_name) + ) + cursor.execute(query) + return list(cursor.fetchall()) + finally: + _close_connection(connection) + except SyncError: + raise + except Exception: + raise _error("unable to fetch PostgreSQL role snapshot") from None + + +_USER_COLUMNS = ( + "username,password,active,use_ssl,default_hostgroup,transaction_persistent," + "fast_forward,backend,frontend,max_connections,attributes,comment" +) +_USER_FIELDS = ( + "username", "password", "active", "use_ssl", "default_hostgroup", "transaction_persistent", + "fast_forward", "backend", "frontend", "max_connections", "attributes", "comment", +) + + +class ProxySQLAdmin: + """Execute the small, explicit ProxySQL admin command set used by sync.""" + + def __init__(self, config: AppConfig, *, connect: Callable[..., object] | None = None) -> None: + self.config = config + self._connect = connect + + def _connection(self) -> object: + connect = self._connect + if connect is None: + try: + import pymysql + except ImportError: + raise _error("ProxySQL admin driver is not installed") from None + connect = pymysql.connect + proxy = self.config.proxysql + try: + return connect( + host=proxy.host, + port=proxy.port, + user=proxy.username, + password=proxy.password, + database="main", + connect_timeout=proxy.connect_timeout, + autocommit=True, + ) + except Exception: + raise _error("unable to connect to ProxySQL admin interface") from None + + @staticmethod + def _user_values(user: ProxySQLUser) -> tuple[object, ...]: + return tuple(getattr(user, field) for field in _USER_FIELDS) + + def _fetch_users(self, table: str) -> list[ProxySQLUser]: + connection = self._connection() + try: + cursor = connection.cursor() + cursor.execute(f"SELECT {_USER_COLUMNS} FROM {table}") + return [ProxySQLUser(*row) for row in cursor.fetchall()] + except Exception: + raise _error("unable to fetch ProxySQL users") from None + finally: + _close_connection(connection) + + def fetch_main_users(self) -> list[ProxySQLUser]: + return self._fetch_users("mysql_users") + + def fetch_runtime_users(self) -> list[ProxySQLUser]: + return self._fetch_users("runtime_mysql_users") + + def apply_actions(self, actions: Sequence[SyncAction]) -> None: + connection = self._connection() + try: + cursor = connection.cursor() + for action in actions: + user = action.after + if action.kind is ActionKind.CREATE: + placeholders = ",".join(["%s"] * len(_USER_FIELDS)) + cursor.execute( + f"INSERT INTO mysql_users ({_USER_COLUMNS}) VALUES ({placeholders})", + self._user_values(user), + ) + elif action.kind is ActionKind.UPDATE: + assignments = ",".join(f"{field}=%s" for field in _USER_FIELDS[1:]) + cursor.execute( + f"UPDATE mysql_users SET {assignments} WHERE username=%s AND backend=%s", + self._user_values(user)[1:] + (user.username, user.backend), + ) + elif action.kind is ActionKind.DISABLE: + cursor.execute( + "UPDATE mysql_users SET active=%s WHERE username=%s AND backend=%s", + (user.active, user.username, user.backend), + ) + else: + raise _error("sync plan contains an unknown action") + except SyncError: + raise + except Exception: + raise _error("unable to apply ProxySQL user changes") from None + finally: + _close_connection(connection) + + def _execute_command(self, command: str, failure_message: str) -> None: + connection = self._connection() + try: + connection.cursor().execute(command) + except Exception: + raise _error(failure_message) from None + finally: + _close_connection(connection) + + def load_runtime(self) -> None: + self._execute_command("LOAD MYSQL USERS TO RUNTIME", "unable to load ProxySQL users to runtime") + + def save_to_disk(self) -> None: + self._execute_command("SAVE MYSQL USERS TO DISK", "unable to save ProxySQL users to disk") + + +def _sync_failure(message: str) -> SyncError: + return _error(message) + + +def run_sync( + config: AppConfig, + source: SourceAdapter, + admin: AdminAdapter, + *, + dry_run: bool, + verbose: bool, +) -> RunSummary: + """Read, plan, and optionally reconcile one complete role snapshot.""" + + started = time.monotonic() + try: + raw_snapshot = source.fetch_snapshot() + except Exception: + raise _sync_failure("unable to fetch source role snapshot") from None + snapshot = validate_snapshot(raw_snapshot, config.sync.allow_empty_snapshot) + try: + main_users = admin.fetch_main_users() + runtime_users = admin.fetch_runtime_users() + except Exception: + raise _sync_failure("unable to fetch ProxySQL user snapshots") from None + plan = build_plan(snapshot, main_users, runtime_users, config.sync) + if verbose: + for action in plan.actions: + print(f"plan: {action.kind.value} username={action.after.username}") + + loaded = False + saved = False + if not dry_run: + if plan.actions: + try: + admin.apply_actions(plan.actions) + except Exception: + raise _sync_failure("unable to apply ProxySQL user changes") from None + if plan.requires_load: + try: + admin.load_runtime() + except Exception: + raise _sync_failure("unable to load ProxySQL users to runtime") from None + loaded = True + if config.sync.save_to_disk: + try: + admin.save_to_disk() + except Exception: + raise _sync_failure("unable to save ProxySQL users to disk") from None + saved = True + return RunSummary( + outcome="dry-run" if dry_run else "success", + counts=MappingProxyType(dict(plan.counts)), + loaded=loaded, + saved=saved, + duration_seconds=time.monotonic() - started, + ) + + +@contextmanager +def exclusive_lock(path: Path): + """Yield whether the process acquired the non-blocking synchronizer lock.""" + + try: + fd = os.open(path, os.O_CREAT | os.O_RDWR, 0o600) + except OSError: + raise _error("unable to open synchronizer lock file") from None + try: + try: + fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB) + except BlockingIOError: + yield False + return + try: + yield True + finally: + fcntl.flock(fd, fcntl.LOCK_UN) + finally: + os.close(fd) + + +def _summary_line(summary: RunSummary) -> str: + counts = " ".join(f"{name}={value}" for name, value in sorted(summary.counts.items())) + return ( + f"sync {summary.outcome}: {counts} loaded={str(summary.loaded).lower()} " + f"saved={str(summary.saved).lower()} duration={summary.duration_seconds:.3f}s" + ) + + +def main(argv: Sequence[str] | None = None) -> int: + """Run the command-line synchronizer without importing database drivers for --help.""" + + args = parse_args(sys.argv[1:] if argv is None else argv) + try: + config = load_config(args.config, CLIOverrides( + default_hostgroup=args.default_hostgroup, + missing_role_action=args.missing_role_action, + save_to_disk=args.save_to_disk, + )) + with exclusive_lock(config.sync.lock_file) as acquired: + if not acquired: + print("sync already running; exiting") + return 0 + summary = run_sync( + config, + PostgreSQLSource(config), + ProxySQLAdmin(config), + dry_run=args.dry_run, + verbose=args.verbose, + ) + print(_summary_line(summary)) + return 0 + except SyncError as error: + print(f"error: {error}", file=sys.stderr) + return 1 + + __all__ = [ "AppConfig", "CLIOverrides", @@ -589,11 +902,23 @@ def build_plan( "SyncPlan", "SyncSettings", "ActionKind", + "AdminAdapter", + "PostgreSQLSource", + "ProxySQLAdmin", + "RunSummary", + "SourceAdapter", "build_plan", "decode_ownership", "load_config", "parse_args", + "run_sync", + "exclusive_lock", + "main", "with_ownership", "validate_snapshot", "validate_verifier", ] + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py b/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py index f5ad8aba7e..907fbcb692 100644 --- a/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py +++ b/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py @@ -422,5 +422,267 @@ def test_inactive_main_row_is_expected_absent_from_runtime(self): self.assertFalse(plan.requires_load) +class FakeCursor: + def __init__(self, rows=()): + self.rows = list(rows) + self.executions = [] + + def execute(self, query, params=None): + self.executions.append((query, params)) + + def fetchall(self): + return list(self.rows) + + +class FakeConnection: + def __init__(self, rows=()): + self.cursor_object = FakeCursor(rows) + self.closed = False + + def cursor(self): + return self.cursor_object + + def close(self): + self.closed = True + + +class FakeSQL: + class Identifier: + def __init__(self, value): + self.value = value + + class SQL: + def __init__(self, value): + self.value = value + + def format(self, *identifiers): + values = tuple(identifier.value for identifier in identifiers) + return self.value.format(*('"%s"' % value for value in values)) + + +class AdapterTests(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.mod = load_module() + cls.verifier_a = "md5" + "a" * 32 + cls.verifier_b = "md5" + "b" * 32 + + def setUp(self): + self.config = self.mod.AppConfig( + source=self.mod.SourceConfig("source", 5432, "roles", "reader", "source-secret"), + proxysql=self.mod.ProxySQLConfig("proxy", 6032, "admin", "admin-secret"), + sync=self.mod.SyncSettings(profile="p"), + ) + self.connection = FakeConnection() + self.cursor = self.connection.cursor_object + + def fake_connect(self, **kwargs): + self.connect_kwargs = kwargs + return self.connection + + def user(self, username="alice", password=None, active=1): + return self.mod.ProxySQLUser( + username=username, password=password or self.verifier_a, active=active, + use_ssl=0, default_hostgroup=3, transaction_persistent=1, + fast_forward=0, backend=1, frontend=1, max_connections=100, + attributes='{"proxysql_pgsql_user_sync":{"profile":"p"}}', comment="sync", + ) + + def update_action(self): + before = self.user(password=self.verifier_a) + return self.mod.SyncAction( + self.mod.ActionKind.UPDATE, before, replace(before, password=self.verifier_b) + ) + + def test_source_composes_schema_and_function_as_identifiers(self): + self.connection.cursor_object.rows = [("alice", self.verifier_a)] + source = self.mod.PostgreSQLSource( + self.config, connect=self.fake_connect, sql_module=FakeSQL + ) + self.assertEqual([("alice", self.verifier_a)], source.fetch_snapshot()) + query, params = self.cursor.executions[-1] + self.assertEqual( + 'SELECT username::text, password FROM "proxysql_auth"."export_login_roles"()', query + ) + self.assertIsNone(params) + self.assertEqual("source", self.connect_kwargs["host"]) + self.assertEqual("roles", self.connect_kwargs["dbname"]) + + def test_admin_fetches_exact_projection_from_both_tables(self): + self.connection.cursor_object.rows = [("alice", self.verifier_a, 1, 0, 3, 1, 0, 1, 1, 100, "", "")] + adapter = self.mod.ProxySQLAdmin(self.config, connect=self.fake_connect) + self.assertEqual("alice", adapter.fetch_main_users()[0].username) + self.assertEqual("alice", adapter.fetch_runtime_users()[0].username) + main_sql = self.cursor.executions[-2][0] + runtime_sql = self.cursor.executions[-1][0] + projection = ("username,password,active,use_ssl,default_hostgroup,transaction_persistent," + "fast_forward,backend,frontend,max_connections,attributes,comment") + self.assertEqual("SELECT %s FROM mysql_users" % projection, main_sql) + self.assertEqual("SELECT %s FROM runtime_mysql_users" % projection, runtime_sql) + self.assertTrue(self.connect_kwargs["autocommit"]) + + def test_admin_update_uses_bound_parameters(self): + adapter = self.mod.ProxySQLAdmin(self.config, connect=self.fake_connect) + adapter.apply_actions((self.update_action(),)) + sql, params = self.cursor.executions[-1] + self.assertIn("password=%s", sql) + self.assertNotIn(self.verifier_b, sql) + self.assertEqual(self.verifier_b, params[0]) + self.assertEqual(("alice", 1), params[-2:]) + + def test_admin_create_and_disable_use_explicit_columns_and_bound_keys(self): + adapter = self.mod.ProxySQLAdmin(self.config, connect=self.fake_connect) + created = self.user("new") + disabled = self.user("old", active=0) + adapter.apply_actions(( + self.mod.SyncAction(self.mod.ActionKind.CREATE, None, created), + self.mod.SyncAction(self.mod.ActionKind.DISABLE, self.user("old"), disabled), + )) + create_sql, create_params = self.cursor.executions[-2] + disable_sql, disable_params = self.cursor.executions[-1] + self.assertIn("INSERT INTO mysql_users (username,password,active", create_sql) + self.assertEqual("new", create_params[0]) + self.assertEqual("UPDATE mysql_users SET active=%s WHERE username=%s AND backend=%s", disable_sql) + self.assertEqual((0, "old", 1), disable_params) + + def test_drivers_are_imported_only_when_default_connectors_are_used(self): + self.assertNotIn("psycopg2", self.mod.__dict__) + self.assertNotIn("pymysql", self.mod.__dict__) + self.mod.PostgreSQLSource(self.config, connect=self.fake_connect, sql_module=FakeSQL).fetch_snapshot() + self.mod.ProxySQLAdmin(self.config, connect=self.fake_connect).fetch_main_users() + self.assertNotIn("psycopg2", self.mod.__dict__) + self.assertNotIn("pymysql", self.mod.__dict__) + + +class FakeSource: + def __init__(self, rows=(), error=None): + self.rows = list(rows) + self.error = error + + def fetch_snapshot(self): + if self.error: + raise self.error + return list(self.rows) + + +class FakeAdmin: + def __init__(self, main=(), runtime=(), fail_apply=False, fail_save=False): + self.main = list(main) + self.runtime = list(runtime) + self.fail_apply = fail_apply + self.fail_save = fail_save + self.calls = [] + + def fetch_main_users(self): + self.calls.append("main") + return list(self.main) + + def fetch_runtime_users(self): + self.calls.append("runtime") + return list(self.runtime) + + def apply_actions(self, actions): + self.calls.append("apply") + if self.fail_apply: + raise RuntimeError("apply failed") + + def load_runtime(self): + self.calls.append("load") + + def save_to_disk(self): + self.calls.append("save") + if self.fail_save: + raise RuntimeError("save failed") + + +class OrchestrationTests(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.mod = load_module() + cls.verifier = "md5" + "a" * 32 + + def setUp(self): + self.config = self.mod.AppConfig( + source=self.mod.SourceConfig("source", 5432, "roles", "reader", "source-secret"), + proxysql=self.mod.ProxySQLConfig("proxy", 6032, "admin", "admin-secret"), + sync=self.mod.SyncSettings(profile="p"), + ) + self.rows = [("alice", self.verifier)] + + def owned_user(self, active=1): + return self.mod.ProxySQLUser( + username="alice", password=self.verifier, active=active, use_ssl=0, + default_hostgroup=0, transaction_persistent=1, fast_forward=0, backend=1, + frontend=1, max_connections=10000, + attributes='{"proxysql_pgsql_user_sync":{"profile":"p"}}', comment="", + ) + + def test_source_failure_is_secret_safe(self): + secret = "source-secret" + with self.assertRaises(self.mod.SyncError) as ctx: + self.mod.run_sync(self.config, FakeSource(error=self.mod.SyncError(secret)), FakeAdmin(), + dry_run=False, verbose=False) + self.assertNotIn(secret, str(ctx.exception)) + + def test_dry_run_does_not_write_load_or_save(self): + admin = FakeAdmin() + summary = self.mod.run_sync(self.config, FakeSource(self.rows), admin, + dry_run=True, verbose=False) + self.assertEqual("dry-run", summary.outcome) + self.assertEqual(["main", "runtime"], admin.calls) + self.assertFalse(summary.loaded) + self.assertFalse(summary.saved) + self.assertEqual(1, summary.counts["created"]) + + def test_write_failure_never_loads_runtime(self): + admin = FakeAdmin(fail_apply=True) + with self.assertRaises(self.mod.SyncError): + self.mod.run_sync(self.config, FakeSource(self.rows), admin, + dry_run=False, verbose=False) + self.assertNotIn("load", admin.calls) + self.assertNotIn("save", admin.calls) + + def test_loads_on_actions_and_saves_only_after_load(self): + admin = FakeAdmin() + summary = self.mod.run_sync(self.config, FakeSource(self.rows), admin, + dry_run=False, verbose=False) + self.assertEqual(["main", "runtime", "apply", "load", "save"], admin.calls) + self.assertTrue(summary.loaded) + self.assertTrue(summary.saved) + + def test_loads_runtime_drift_without_actions(self): + main = self.owned_user() + runtime = replace(main, active=0) + admin = FakeAdmin([main], [runtime]) + summary = self.mod.run_sync(self.config, FakeSource(self.rows), admin, + dry_run=False, verbose=False) + self.assertEqual(["main", "runtime", "load", "save"], admin.calls) + self.assertEqual(0, summary.counts["updated"]) + + def test_save_failure_follows_successful_load(self): + admin = FakeAdmin(fail_save=True) + with self.assertRaises(self.mod.SyncError): + self.mod.run_sync(self.config, FakeSource(self.rows), admin, + dry_run=False, verbose=False) + self.assertEqual(["main", "runtime", "apply", "load", "save"], admin.calls) + + def test_exclusive_lock_reports_contention(self): + with tempfile.TemporaryDirectory() as directory: + path = Path(directory) / "sync.lock" + with self.mod.exclusive_lock(path) as first: + self.assertTrue(first) + with self.mod.exclusive_lock(path) as second: + self.assertFalse(second) + self.assertEqual(0o600, path.stat().st_mode & 0o777) + + def test_summary_has_non_negative_duration(self): + summary = self.mod.run_sync(self.config, FakeSource(self.rows), FakeAdmin(), + dry_run=True, verbose=False) + self.assertIsInstance(summary, self.mod.RunSummary) + self.assertGreaterEqual(summary.duration_seconds, 0.0) + with self.assertRaises(AttributeError): + summary.outcome = "changed" + + if __name__ == "__main__": unittest.main() From 23ff2b2c65e52f51e9e0a433edb10b6a54e2c92c Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Wed, 12 Aug 2026 15:46:31 +0000 Subject: [PATCH 07/18] docs: add PostgreSQL user sync deployment sample --- tools/pgsql_user_sync/README.md | 143 ++++++++++++++++++ .../create_source_function.sql | 56 +++++++ .../proxysql_pgsql_user_sync.ini.example | 27 ++++ tools/pgsql_user_sync/requirements.txt | 2 + .../tests/test_pgsql_user_sync.py | 61 ++++++++ 5 files changed, 289 insertions(+) create mode 100644 tools/pgsql_user_sync/README.md create mode 100644 tools/pgsql_user_sync/create_source_function.sql create mode 100644 tools/pgsql_user_sync/proxysql_pgsql_user_sync.ini.example create mode 100644 tools/pgsql_user_sync/requirements.txt diff --git a/tools/pgsql_user_sync/README.md b/tools/pgsql_user_sync/README.md new file mode 100644 index 0000000000..25bc806f2e --- /dev/null +++ b/tools/pgsql_user_sync/README.md @@ -0,0 +1,143 @@ +# PostgreSQL user synchronizer sample + +This operator-managed sample periodically copies eligible PostgreSQL role +verifiers into ProxySQL's `pgsql_users` table. It never reads plaintext role +passwords. ProxySQL continues to authenticate from its runtime user table; +the scheduler interval plus one successful run is the expected propagation +delay. + +The sample owns only rows marked in `attributes` with +`{"proxysql_pgsql_user_sync":{"profile":"..."}}`. It does not synchronize +routing, TLS, connection limits, or other ProxySQL policy. + +## Install and protect the configuration + +Install the dependencies in a dedicated environment (or into the Python +installation used by Scheduler): + +```console +python3 -m venv /opt/proxysql-pgsql-user-sync +/opt/proxysql-pgsql-user-sync/bin/pip install -r requirements.txt +install -m 0755 proxysql_pgsql_user_sync.py /usr/share/proxysql/tools/pgsql_user_sync/ +``` + +Copy `proxysql_pgsql_user_sync.ini.example` to a regular file, replace both +obvious password placeholders, and restrict it before entering real secrets: + +```console +install -o root -g root -m 0600 proxysql_pgsql_user_sync.ini.example /etc/proxysql/pgsql-user-sync.ini +``` + +The synchronizer rejects group-write/execute and other-user permissions. A +root-owned `0640` file with a dedicated service group is also accepted; an +owner-only `0600` file is the simplest choice. Database passwords stay in +this file and never appear in Scheduler arguments or normal logs. + +## Create the source function and allow-list + +As a trusted PostgreSQL administrator, connect to the configured database and +run `create_source_function.sql` after replacing the reader password. The +script is rerunnable: guarded role creation, a `SECURITY DEFINER` function +with `SET search_path = pg_catalog`, and explicit revokes protect the +credential query. The reader receives only database `CONNECT`, schema +`USAGE`, and function `EXECUTE`. + +The `proxysql_auth_managed` `NOLOGIN` role is the import allow-list. Grant and +revoke membership deliberately, for example: + +```sql +GRANT app_login TO proxysql_auth_managed; +REVOKE app_login FROM proxysql_auth_managed; +``` + +The function additionally requires a login-capable role, a non-null verifier, +and no expired `rolvaliduntil`. Therefore superusers, replication roles, and +service accounts are not imported unless an operator explicitly places them +on the allow-list. + +## Run manually + +Always use absolute paths when invoking the script: + +```console +/usr/bin/python3 /usr/share/proxysql/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py \ + --config /etc/proxysql/pgsql-user-sync.ini --dry-run --verbose +``` + +Remove `--dry-run` to apply the plan. Useful non-secret overrides are +`--default-hostgroup N`, `--missing-role-action disable|keep`, +`--save-to-disk`/`--no-save-to-disk`, and `--verbose`. `--dry-run` fetches and +validates both snapshots but performs no writes, runtime load, or disk save. +An exclusive lock makes overlapping scheduler invocations a successful skip. + +## ProxySQL Scheduler + +ProxySQL starts scheduler programs with `execve` and an empty environment, so +the filename, script, interpreter, and configuration path must all be +absolute. The script's lock file handles overlap. Adapt the ID and paths to +your installation, then run the two load/save statements shown below: + +```sql +INSERT INTO scheduler + (id, active, interval_ms, filename, arg1, arg2, arg3, comment) +VALUES + (9100, 1, 10000, + '/usr/bin/python3', + '/usr/share/proxysql/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py', + '--config', + '/etc/proxysql/pgsql-user-sync.ini', + 'Synchronize PostgreSQL role verifiers'); + +LOAD SCHEDULER TO RUNTIME; +SAVE SCHEDULER TO DISK; +``` + +## Missing roles and existing users + +`missing_role_action = disable` (the default) sets `active = 0` for managed +rows absent from the validated source snapshot. `keep` leaves them alone. +Neither mode deletes rows, and a role that becomes eligible later is updated +and reactivated. + +An existing unmanaged username is a conflict and aborts before writes by +default. Set `adopt_existing_users = true` only after reviewing the row: the +next run marks it for this profile and preserves its other ProxySQL policy. +A row owned by another profile is always a conflict; ownership is never +silently transferred. + +## Failure and recovery boundaries + +The synchronizer builds and validates the complete source snapshot before +writing. It uses the Admin interface's autocommit operations, so the main +table and runtime load are **non-transactional**: a mid-run write failure can +leave `main.pgsql_users` partially changed while runtime retains its last +known-good state. No global `LOAD PGSQL USERS TO RUNTIME` is issued until all +planned writes succeed. The next run compares both tables and repairs the +main/runtime divergence. + +`LOAD PGSQL USERS TO RUNTIME` is global. To avoid overwriting unrelated +runtime state, a detected unmanaged main/runtime divergence aborts before any +write or load. A runtime-load failure is retried on the next run. A disk +save occurs only after a successful load; if it fails, the already-loaded +runtime remains active and the outcome is reported as partial. Set +`save_to_disk = false` or use `--no-save-to-disk` when disk persistence is +managed separately. + +Successful and skipped runs emit one concise summary containing profile, +duration, and discovered/created/updated/reactivated/disabled/unchanged/ +conflicted counts. Errors are nonzero except lock contention, which is an +expected successful skip. Logs never contain passwords, verifiers, password- +bearing connection strings, or complete configuration objects. Verbose mode +may name users and action types, but still never credential material. + +## Cluster operation + +Choose exactly one of these mutually exclusive models: + +1. Run the synchronizer on one authoritative ProxySQL node and enable + `pgsql_users` cluster propagation. +2. Run the same deterministic profile on every node and disable cluster + propagation for `pgsql_users`. + +Do not combine independent writers with cluster propagation: competing loads +can cause checksum/epoch churn and race one another. diff --git a/tools/pgsql_user_sync/create_source_function.sql b/tools/pgsql_user_sync/create_source_function.sql new file mode 100644 index 0000000000..f846cfa82b --- /dev/null +++ b/tools/pgsql_user_sync/create_source_function.sql @@ -0,0 +1,56 @@ +-- Run this file while connected to the database named in the synchronizer +-- configuration (normally "postgres") as a trusted PostgreSQL administrator. +-- Replace the reader password before running it. This is an operator-managed +-- sample; keep this file out of source control after inserting a real secret. + +-- The membership of this NOLOGIN role is the explicit import allow-list. +DO $role$ +BEGIN + CREATE ROLE proxysql_auth_managed NOLOGIN; +EXCEPTION + WHEN duplicate_object THEN NULL; +END +$role$; +ALTER ROLE proxysql_auth_managed NOLOGIN; + +DO $role$ +BEGIN + CREATE ROLE proxysql_auth_reader + LOGIN + PASSWORD 'REPLACE_WITH_SOURCE_PASSWORD'; +EXCEPTION + WHEN duplicate_object THEN NULL; +END +$role$; + +CREATE SCHEMA IF NOT EXISTS proxysql_auth; +ALTER SCHEMA proxysql_auth OWNER TO CURRENT_USER; + +CREATE OR REPLACE FUNCTION proxysql_auth.export_login_roles() +RETURNS TABLE(username text, password text) +LANGUAGE sql +SECURITY DEFINER +SET search_path = pg_catalog +AS $function$ + SELECT r.rolname::text AS username, r.rolpassword AS password + FROM pg_catalog.pg_authid AS r + WHERE r.rolcanlogin + AND r.rolpassword IS NOT NULL + AND (r.rolvaliduntil IS NULL OR r.rolvaliduntil > pg_catalog.now()) + AND pg_catalog.pg_has_role(r.oid, 'proxysql_auth_managed', 'member') + ORDER BY r.rolname; +$function$; + +-- Remove defaults first, then grant only the exact access needed by the +-- synchronizer. CONNECT is shown for the sample database; change postgres +-- when installing into another database. +REVOKE ALL ON SCHEMA proxysql_auth FROM PUBLIC; +REVOKE ALL ON FUNCTION proxysql_auth.export_login_roles() FROM PUBLIC; +REVOKE ALL ON FUNCTION proxysql_auth.export_login_roles() FROM proxysql_auth_reader; +GRANT CONNECT ON DATABASE postgres TO proxysql_auth_reader; +GRANT USAGE ON SCHEMA proxysql_auth TO proxysql_auth_reader; +GRANT EXECUTE ON FUNCTION proxysql_auth.export_login_roles() TO proxysql_auth_reader; + +-- Allow-list examples (run separately, one role at a time): +-- GRANT app_login TO proxysql_auth_managed; +-- REVOKE app_login FROM proxysql_auth_managed; diff --git a/tools/pgsql_user_sync/proxysql_pgsql_user_sync.ini.example b/tools/pgsql_user_sync/proxysql_pgsql_user_sync.ini.example new file mode 100644 index 0000000000..e74e22ec76 --- /dev/null +++ b/tools/pgsql_user_sync/proxysql_pgsql_user_sync.ini.example @@ -0,0 +1,27 @@ +# Copy to a root-owned or service-owned file, replace both placeholders, and +# chmod 0600 before use. Secrets are intentionally not accepted on the CLI. + +[source] +host = postgresql.example +port = 5432 +database = postgres +username = proxysql_auth_reader +password = REPLACE_WITH_SOURCE_PASSWORD +connect_timeout = 10 +function = proxysql_auth.export_login_roles + +[proxysql] +host = 127.0.0.1 +port = 6032 +username = admin +password = REPLACE_WITH_PROXYSQL_PASSWORD +connect_timeout = 10 + +[sync] +profile = primary-cluster +default_hostgroup = 0 +missing_role_action = disable +adopt_existing_users = false +allow_empty_snapshot = false +save_to_disk = true +lock_file = /run/lock/proxysql-pgsql-user-sync.lock diff --git a/tools/pgsql_user_sync/requirements.txt b/tools/pgsql_user_sync/requirements.txt new file mode 100644 index 0000000000..4ef92dd587 --- /dev/null +++ b/tools/pgsql_user_sync/requirements.txt @@ -0,0 +1,2 @@ +psycopg[binary]>=3.2.13,<4 +PyMySQL>=1.1.1,<2 diff --git a/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py b/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py index 907fbcb692..db17b31890 100644 --- a/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py +++ b/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py @@ -13,6 +13,7 @@ SCRIPT = Path(__file__).parents[1] / "proxysql_pgsql_user_sync.py" +ASSET_DIR = SCRIPT.parent def load_module(): @@ -684,5 +685,65 @@ def test_summary_has_non_negative_duration(self): summary.outcome = "changed" +class AssetTests(unittest.TestCase): + """Keep the operator-facing deployment sample complete and safe.""" + + def test_source_function_has_security_boundaries(self): + sql = (ASSET_DIR / "create_source_function.sql").read_text() + for required in ( + "SECURITY DEFINER", + "SET search_path = pg_catalog", + "rolcanlogin", + "rolvaliduntil", + "REVOKE ALL", + ): + self.assertIn(required, sql) + self.assertIn("proxysql_auth_managed", sql) + self.assertIn("pg_has_role", sql) + self.assertIn("proxysql_auth_reader", sql) + + def test_example_configuration_is_loadable_with_protected_permissions(self): + source = (ASSET_DIR / "proxysql_pgsql_user_sync.ini.example").read_text() + self.assertIn("[source]", source) + self.assertIn("[proxysql]", source) + self.assertIn("[sync]", source) + with tempfile.TemporaryDirectory() as directory: + path = Path(directory) / "pgsql-user-sync.ini" + path.write_text( + source.replace("REPLACE_WITH_SOURCE_PASSWORD", "source-secret") + .replace("REPLACE_WITH_PROXYSQL_PASSWORD", "proxysql-secret"), + encoding="utf-8", + ) + os.chmod(path, 0o600) + module = load_module() + config = module.load_config(path, module.CLIOverrides()) + self.assertEqual("proxysql_auth", config.source.function_schema) + self.assertEqual("export_login_roles", config.source.function_name) + + def test_requirements_pin_supported_driver_majors(self): + requirements = (ASSET_DIR / "requirements.txt").read_text() + self.assertIn("psycopg[binary]>=3.2.13,<4", requirements) + self.assertIn("PyMySQL>=1.1.1,<2", requirements) + + def test_readme_documents_scheduler_and_cluster_safety(self): + readme = (ASSET_DIR / "README.md").read_text() + for required in ( + "absolute", + "LOAD SCHEDULER TO RUNTIME", + "SAVE SCHEDULER TO DISK", + "disable", + "keep", + "adopt_existing_users", + "dry-run", + "non-transactional", + "unmanaged", + "log", + "disk", + ): + self.assertIn(required, readme) + self.assertIn("one authoritative ProxySQL node", readme) + self.assertIn("every node", readme) + + if __name__ == "__main__": unittest.main() From ea0fa33a3f9367468ce4f2e6c17e0cceb29f6894 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Wed, 12 Aug 2026 15:48:55 +0000 Subject: [PATCH 08/18] fix: use psycopg3 for PostgreSQL sync source --- tools/pgsql_user_sync/proxysql_pgsql_user_sync.py | 6 +++--- .../pgsql_user_sync/tests/test_pgsql_user_sync.py | 14 +++++++++++++- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py b/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py index 5eafa701be..007649d85d 100644 --- a/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py +++ b/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py @@ -630,11 +630,11 @@ def _driver(self) -> tuple[Callable[..., object], object]: if self._connect is not None and self._sql_module is not None: return self._connect, self._sql_module try: - import psycopg2 - from psycopg2 import sql + import psycopg + from psycopg import sql except ImportError: raise _error("PostgreSQL driver is not installed") from None - return self._connect or psycopg2.connect, self._sql_module or sql + return self._connect or psycopg.connect, self._sql_module or sql def fetch_snapshot(self) -> list[tuple[str, str]]: try: diff --git a/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py b/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py index db17b31890..f86b2160ea 100644 --- a/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py +++ b/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py @@ -6,7 +6,7 @@ import sys import tempfile from dataclasses import replace -from types import SimpleNamespace +from types import ModuleType, SimpleNamespace import unittest from unittest.mock import patch from pathlib import Path @@ -547,13 +547,25 @@ def test_admin_create_and_disable_use_explicit_columns_and_bound_keys(self): self.assertEqual((0, "old", 1), disable_params) def test_drivers_are_imported_only_when_default_connectors_are_used(self): + self.assertNotIn("psycopg", self.mod.__dict__) self.assertNotIn("psycopg2", self.mod.__dict__) self.assertNotIn("pymysql", self.mod.__dict__) self.mod.PostgreSQLSource(self.config, connect=self.fake_connect, sql_module=FakeSQL).fetch_snapshot() self.mod.ProxySQLAdmin(self.config, connect=self.fake_connect).fetch_main_users() + self.assertNotIn("psycopg", self.mod.__dict__) self.assertNotIn("psycopg2", self.mod.__dict__) self.assertNotIn("pymysql", self.mod.__dict__) + def test_default_source_connector_uses_psycopg3(self): + fake_psycopg = ModuleType("psycopg") + fake_psycopg.connect = self.fake_connect + fake_psycopg.sql = FakeSQL + self.connection.cursor_object.rows = [("alice", self.verifier_a)] + with patch.dict(sys.modules, {"psycopg": fake_psycopg}): + source = self.mod.PostgreSQLSource(self.config) + self.assertEqual([("alice", self.verifier_a)], source.fetch_snapshot()) + self.assertEqual("source", self.connect_kwargs["host"]) + class FakeSource: def __init__(self, rows=(), error=None): From 47531df21eb5aaa3b6dd96ca6090100b354b25bd Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Wed, 12 Aug 2026 15:52:06 +0000 Subject: [PATCH 09/18] fix: target PostgreSQL user tables in sync sample --- tools/pgsql_user_sync/README.md | 13 +++++++----- .../proxysql_pgsql_user_sync.py | 14 ++++++------- .../tests/test_pgsql_user_sync.py | 20 +++++++++++++++---- 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/tools/pgsql_user_sync/README.md b/tools/pgsql_user_sync/README.md index 25bc806f2e..3156249602 100644 --- a/tools/pgsql_user_sync/README.md +++ b/tools/pgsql_user_sync/README.md @@ -25,13 +25,16 @@ Copy `proxysql_pgsql_user_sync.ini.example` to a regular file, replace both obvious password placeholders, and restrict it before entering real secrets: ```console -install -o root -g root -m 0600 proxysql_pgsql_user_sync.ini.example /etc/proxysql/pgsql-user-sync.ini +install -o proxysql -g proxysql -m 0600 proxysql_pgsql_user_sync.ini.example /etc/proxysql/pgsql-user-sync.ini ``` -The synchronizer rejects group-write/execute and other-user permissions. A -root-owned `0640` file with a dedicated service group is also accepted; an -owner-only `0600` file is the simplest choice. Database passwords stay in -this file and never appear in Scheduler arguments or normal logs. +The synchronizer rejects group-write/execute and other-user permissions. The +primary example is service-owned `0600` so the ProxySQL service account can +read it. Alternatively, keep the file root-owned and use `chown root:proxysql` +with mode `0640` (a dedicated `proxysql` group); that is also accepted. An +owner-only `0600` file is the simplest choice when the synchronizer runs as its +owner. Database passwords stay in this file and never appear in Scheduler +arguments or normal logs. ## Create the source function and allow-list diff --git a/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py b/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py index 007649d85d..8e3e7119df 100644 --- a/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py +++ b/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py @@ -718,10 +718,10 @@ def _fetch_users(self, table: str) -> list[ProxySQLUser]: _close_connection(connection) def fetch_main_users(self) -> list[ProxySQLUser]: - return self._fetch_users("mysql_users") + return self._fetch_users("pgsql_users") def fetch_runtime_users(self) -> list[ProxySQLUser]: - return self._fetch_users("runtime_mysql_users") + return self._fetch_users("runtime_pgsql_users") def apply_actions(self, actions: Sequence[SyncAction]) -> None: connection = self._connection() @@ -732,18 +732,18 @@ def apply_actions(self, actions: Sequence[SyncAction]) -> None: if action.kind is ActionKind.CREATE: placeholders = ",".join(["%s"] * len(_USER_FIELDS)) cursor.execute( - f"INSERT INTO mysql_users ({_USER_COLUMNS}) VALUES ({placeholders})", + f"INSERT INTO pgsql_users ({_USER_COLUMNS}) VALUES ({placeholders})", self._user_values(user), ) elif action.kind is ActionKind.UPDATE: assignments = ",".join(f"{field}=%s" for field in _USER_FIELDS[1:]) cursor.execute( - f"UPDATE mysql_users SET {assignments} WHERE username=%s AND backend=%s", + f"UPDATE pgsql_users SET {assignments} WHERE username=%s AND backend=%s", self._user_values(user)[1:] + (user.username, user.backend), ) elif action.kind is ActionKind.DISABLE: cursor.execute( - "UPDATE mysql_users SET active=%s WHERE username=%s AND backend=%s", + "UPDATE pgsql_users SET active=%s WHERE username=%s AND backend=%s", (user.active, user.username, user.backend), ) else: @@ -765,10 +765,10 @@ def _execute_command(self, command: str, failure_message: str) -> None: _close_connection(connection) def load_runtime(self) -> None: - self._execute_command("LOAD MYSQL USERS TO RUNTIME", "unable to load ProxySQL users to runtime") + self._execute_command("LOAD PGSQL USERS TO RUNTIME", "unable to load ProxySQL users to runtime") def save_to_disk(self) -> None: - self._execute_command("SAVE MYSQL USERS TO DISK", "unable to save ProxySQL users to disk") + self._execute_command("SAVE PGSQL USERS TO DISK", "unable to save ProxySQL users to disk") def _sync_failure(message: str) -> SyncError: diff --git a/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py b/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py index f86b2160ea..a5311f5a58 100644 --- a/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py +++ b/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py @@ -518,8 +518,8 @@ def test_admin_fetches_exact_projection_from_both_tables(self): runtime_sql = self.cursor.executions[-1][0] projection = ("username,password,active,use_ssl,default_hostgroup,transaction_persistent," "fast_forward,backend,frontend,max_connections,attributes,comment") - self.assertEqual("SELECT %s FROM mysql_users" % projection, main_sql) - self.assertEqual("SELECT %s FROM runtime_mysql_users" % projection, runtime_sql) + self.assertEqual("SELECT %s FROM pgsql_users" % projection, main_sql) + self.assertEqual("SELECT %s FROM runtime_pgsql_users" % projection, runtime_sql) self.assertTrue(self.connect_kwargs["autocommit"]) def test_admin_update_uses_bound_parameters(self): @@ -541,11 +541,20 @@ def test_admin_create_and_disable_use_explicit_columns_and_bound_keys(self): )) create_sql, create_params = self.cursor.executions[-2] disable_sql, disable_params = self.cursor.executions[-1] - self.assertIn("INSERT INTO mysql_users (username,password,active", create_sql) + self.assertIn("INSERT INTO pgsql_users (username,password,active", create_sql) self.assertEqual("new", create_params[0]) - self.assertEqual("UPDATE mysql_users SET active=%s WHERE username=%s AND backend=%s", disable_sql) + self.assertEqual("UPDATE pgsql_users SET active=%s WHERE username=%s AND backend=%s", disable_sql) self.assertEqual((0, "old", 1), disable_params) + def test_admin_runtime_commands_use_postgresql_users(self): + adapter = self.mod.ProxySQLAdmin(self.config, connect=self.fake_connect) + adapter.load_runtime() + adapter.save_to_disk() + self.assertEqual( + "LOAD PGSQL USERS TO RUNTIME", self.cursor.executions[-2][0] + ) + self.assertEqual("SAVE PGSQL USERS TO DISK", self.cursor.executions[-1][0]) + def test_drivers_are_imported_only_when_default_connectors_are_used(self): self.assertNotIn("psycopg", self.mod.__dict__) self.assertNotIn("psycopg2", self.mod.__dict__) @@ -751,6 +760,9 @@ def test_readme_documents_scheduler_and_cluster_safety(self): "unmanaged", "log", "disk", + "install -o proxysql -g proxysql -m 0600", + "root:proxysql", + "0640", ): self.assertIn(required, readme) self.assertIn("one authoritative ProxySQL node", readme) From c14de7fdf1c12855c395f446e5aafdd0b6b126c0 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Wed, 12 Aug 2026 15:54:50 +0000 Subject: [PATCH 10/18] test: register PostgreSQL user sync unit suite --- test/tap/groups/groups.json | 1 + test/tap/tests/pgsql-user-sync-unit-t.py | 11 +++++++++++ 2 files changed, 12 insertions(+) create mode 100644 test/tap/tests/pgsql-user-sync-unit-t.py diff --git a/test/tap/groups/groups.json b/test/tap/groups/groups.json index 0a27682593..44f36b07f4 100644 --- a/test/tap/groups/groups.json +++ b/test/tap/groups/groups.json @@ -208,6 +208,7 @@ "pgsql-tx_poisoned_recovery-t" : [ "legacy-g2" ], "pgsql-unix_socket-t" : [ "pgsql-socket-g1" ], "pgsql-unsupported_feature_test-t" : [ "legacy-g6","mysql-auto_increment_delay_multiplex=0-g1","mysql-multiplexing=false-g1","mysql-query_digests=0-g1","mysql-query_digests_keep_comment=1-g1" ], + "pgsql-user-sync-unit-t" : [ "no-infra-g1" ], "pgsql-watchdog_test-t" : [ "legacy-g4","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4" ], "pgsql_command_complete_unit-t" : [ "unit-tests-g1" ], "pgsql_error_classifier_unit-t" : [ "unit-tests-g1" ], diff --git a/test/tap/tests/pgsql-user-sync-unit-t.py b/test/tap/tests/pgsql-user-sync-unit-t.py new file mode 100644 index 0000000000..836543cc12 --- /dev/null +++ b/test/tap/tests/pgsql-user-sync-unit-t.py @@ -0,0 +1,11 @@ +#!/usr/bin/env python3 +import os +import subprocess +import sys +from pathlib import Path + +root = Path(os.environ.get("WORKSPACE", Path(__file__).resolve().parents[3])) +suite = root / "tools/pgsql_user_sync/tests/test_pgsql_user_sync.py" +raise SystemExit(subprocess.call( + [sys.executable, "-m", "unittest", "-v", str(suite)], cwd=root +)) From 7bfdb45d2ee650f3bf472ec57c9a82b829adef9b Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Wed, 12 Aug 2026 16:14:38 +0000 Subject: [PATCH 11/18] test: cover PostgreSQL user synchronization lifecycle --- test/infra/docker-base/Dockerfile | 2 + test/tap/groups/groups.json | 1 + test/tap/tests/Makefile | 3 + test/tap/tests/pgsql-user-sync-t.py | 414 ++++++++++++++++++++++++++++ 4 files changed, 420 insertions(+) create mode 100644 test/tap/tests/pgsql-user-sync-t.py diff --git a/test/infra/docker-base/Dockerfile b/test/infra/docker-base/Dockerfile index f0671488b7..cd6315c005 100755 --- a/test/infra/docker-base/Dockerfile +++ b/test/infra/docker-base/Dockerfile @@ -1,6 +1,7 @@ FROM ubuntu:24.04 ARG MYSQL_CONNECTOR_PYTHON_VERSION=9.7.0 +ARG PSYCOPG_VERSION=3.2.13 ENV DEBIAN_FRONTEND=noninteractive @@ -31,6 +32,7 @@ RUN apt-get update -qq && \ php-mysql \ lcov \ && pip3 install --break-system-packages fastcov \ + "psycopg[binary]==${PSYCOPG_VERSION}" \ "mysql-connector-python==${MYSQL_CONNECTOR_PYTHON_VERSION}" \ "mysqlx-connector-python==${MYSQL_CONNECTOR_PYTHON_VERSION}" \ && wget https://github.com/openark/orchestrator/releases/download/v3.2.6/orchestrator-client_3.2.6_amd64.deb -O /tmp/orc.deb \ diff --git a/test/tap/groups/groups.json b/test/tap/groups/groups.json index 44f36b07f4..f07ff72987 100644 --- a/test/tap/groups/groups.json +++ b/test/tap/groups/groups.json @@ -208,6 +208,7 @@ "pgsql-tx_poisoned_recovery-t" : [ "legacy-g2" ], "pgsql-unix_socket-t" : [ "pgsql-socket-g1" ], "pgsql-unsupported_feature_test-t" : [ "legacy-g6","mysql-auto_increment_delay_multiplex=0-g1","mysql-multiplexing=false-g1","mysql-query_digests=0-g1","mysql-query_digests_keep_comment=1-g1" ], + "pgsql-user-sync-t" : [ "legacy-g4" ], "pgsql-user-sync-unit-t" : [ "no-infra-g1" ], "pgsql-watchdog_test-t" : [ "legacy-g4","mysql-auto_increment_delay_multiplex=0-g4","mysql-multiplexing=false-g4","mysql-query_digests=0-g4","mysql-query_digests_keep_comment=1-g4" ], "pgsql_command_complete_unit-t" : [ "unit-tests-g1" ], diff --git a/test/tap/tests/Makefile b/test/tap/tests/Makefile index 849f2abaaf..16cd37e7ce 100644 --- a/test/tap/tests/Makefile +++ b/test/tap/tests/Makefile @@ -257,6 +257,9 @@ py-%: cp $(patsubst py-%,%,$@) $(patsubst py-%.py,%,$@) chmod +x $(patsubst py-%.py,%,$@) +.PHONY: pgsql-user-sync-t +pgsql-user-sync-t: py-pgsql-user-sync-t.py + sh-%: cp $(patsubst sh-%,%,$@) $(patsubst sh-%.sh,%,$@) chmod +x $(patsubst sh-%.sh,%,$@) diff --git a/test/tap/tests/pgsql-user-sync-t.py b/test/tap/tests/pgsql-user-sync-t.py new file mode 100644 index 0000000000..daaa339e0e --- /dev/null +++ b/test/tap/tests/pgsql-user-sync-t.py @@ -0,0 +1,414 @@ +#!/usr/bin/env python3 +"""Exercise PostgreSQL verifier synchronization against a real ProxySQL.""" + +import json +import os +import secrets +import subprocess +import sys +import tempfile +import uuid +from pathlib import Path + +import pymysql +import psycopg +from psycopg import sql + + +ROOT = Path(os.environ.get("WORKSPACE", Path(__file__).resolve().parents[3])) +SCRIPT = ROOT / "tools/pgsql_user_sync/proxysql_pgsql_user_sync.py" +USER_COLUMNS = ( + "username,password,active,use_ssl,default_hostgroup,transaction_persistent," + "fast_forward,backend,frontend,max_connections,attributes,comment" +) +PROFILE = "tap-lifecycle" + + +class Tap: + def __init__(self): + self.count = 0 + self.failures = 0 + + def check(self, condition, description): + self.count += 1 + if condition: + print(f"ok {self.count} - {description}") + else: + self.failures += 1 + print(f"not ok {self.count} - {description}") + return condition + + def diag(self, description): + print(f"# {description}") + + +def env(name, default=None): + value = os.environ.get(name, default) + if value is None or value == "": + raise RuntimeError(f"required environment variable {name} is not set") + return value + + +def pg_connection(): + return psycopg.connect( + host=env("TAP_PGSQLSERVER_HOST"), + port=int(env("TAP_PGSQLSERVER_PORT", "5432")), + dbname="postgres", + user=env("TAP_PGSQLSERVER_USERNAME"), + password=env("TAP_PGSQLSERVER_PASSWORD"), + autocommit=True, + ) + + +def admin_connection(): + return pymysql.connect( + host=env("TAP_ADMINHOST", "127.0.0.1"), + port=int(env("TAP_ADMINPORT", "6032")), + user=env("TAP_ADMINUSERNAME", "radmin"), + password=env("TAP_ADMINPASSWORD", "radmin"), + database="main", + autocommit=True, + ) + + +def admin_execute(query, params=()): + with admin_connection() as connection: + with connection.cursor() as cursor: + cursor.execute(query, params) + + +def admin_row(username, runtime=False): + table = "runtime_pgsql_users" if runtime else "pgsql_users" + with admin_connection() as connection: + with connection.cursor() as cursor: + cursor.execute( + f"SELECT {USER_COLUMNS} FROM {table} WHERE username=%s", (username,) + ) + return cursor.fetchone() + + +def verifier(connection, role): + with connection.cursor() as cursor: + cursor.execute("SELECT rolpassword FROM pg_catalog.pg_authid WHERE rolname=%s", (role,)) + value = cursor.fetchone() + if value is None or value[0] is None: + raise RuntimeError("test role verifier was not created") + return value[0] + + +def frontend_login(username, password): + connection = psycopg.connect( + host=env("TAP_PGSQL_HOST", "127.0.0.1"), + port=int(env("TAP_PGSQL_PORT", "6133")), + dbname="postgres", + user=username, + password=password, + connect_timeout=5, + ) + try: + with connection.cursor() as cursor: + cursor.execute("SELECT 1") + finally: + connection.close() + + +def is_owned(row): + if row is None: + return False + try: + return json.loads(row[10]).get("proxysql_pgsql_user_sync") == {"profile": PROFILE} + except (TypeError, ValueError, AttributeError): + return False + + +def same_control_row(before, after): + return before is not None and before == after and not is_owned(after) + + +def write_config(path, names, reader_password, workdir): + path.write_text( + "\n".join(( + "[source]", + f"host = {env('TAP_PGSQLSERVER_HOST')}", + f"port = {env('TAP_PGSQLSERVER_PORT', '5432')}", + "database = postgres", + f"username = {names['reader']}", + f"password = {reader_password}", + "connect_timeout = 10", + f"function = {names['schema']}.export_login_roles", + "", + "[proxysql]", + f"host = {env('TAP_ADMINHOST', '127.0.0.1')}", + f"port = {env('TAP_ADMINPORT', '6032')}", + f"username = {env('TAP_ADMINUSERNAME', 'radmin')}", + f"password = {env('TAP_ADMINPASSWORD', 'radmin')}", + "connect_timeout = 10", + "", + "[sync]", + f"profile = {PROFILE}", + "default_hostgroup = 0", + "missing_role_action = disable", + "adopt_existing_users = false", + "allow_empty_snapshot = false", + "save_to_disk = true", + f"lock_file = {workdir / 'pgsql-user-sync.lock'}", + "", + )), + encoding="utf-8", + ) + os.chmod(path, 0o600) + + +def invoke(config_path, extra=()): + command = [sys.executable, str(SCRIPT), "--config", str(config_path)] + command.extend(extra) + return subprocess.run(command, text=True, capture_output=True) + + +def create_source_objects(connection, names, reader_password, test_password): + with connection.cursor() as cursor: + cursor.execute(sql.SQL("CREATE ROLE {} NOLOGIN").format(sql.Identifier(names["allow"]))) + cursor.execute( + sql.SQL("CREATE ROLE {} LOGIN PASSWORD %s").format(sql.Identifier(names["reader"])), + (reader_password,), + ) + cursor.execute( + sql.SQL("CREATE ROLE {} LOGIN PASSWORD %s").format(sql.Identifier(names["test"])), + (test_password,), + ) + cursor.execute( + sql.SQL("GRANT {} TO {}").format( + sql.Identifier(names["allow"]), sql.Identifier(names["test"]) + ) + ) + cursor.execute(sql.SQL("CREATE SCHEMA {} AUTHORIZATION CURRENT_USER").format( + sql.Identifier(names["schema"]) + )) + function = sql.SQL( + "CREATE FUNCTION {}.export_login_roles() " + "RETURNS TABLE(username text, password text) LANGUAGE sql SECURITY DEFINER " + "SET search_path = pg_catalog AS $$ " + "SELECT r.rolname::text, r.rolpassword FROM pg_catalog.pg_authid AS r " + "WHERE r.rolcanlogin AND r.rolpassword IS NOT NULL " + "AND (r.rolvaliduntil IS NULL OR r.rolvaliduntil > pg_catalog.now()) " + "AND pg_catalog.pg_has_role(r.oid, {}, 'member') ORDER BY r.rolname $$" + ).format(sql.Identifier(names["schema"]), sql.Literal(names["allow"])) + cursor.execute(function) + cursor.execute(sql.SQL("REVOKE ALL ON SCHEMA {} FROM PUBLIC").format( + sql.Identifier(names["schema"]) + )) + cursor.execute(sql.SQL("REVOKE ALL ON FUNCTION {}.export_login_roles() FROM PUBLIC").format( + sql.Identifier(names["schema"]) + )) + cursor.execute(sql.SQL("GRANT USAGE ON SCHEMA {} TO {}").format( + sql.Identifier(names["schema"]), sql.Identifier(names["reader"]) + )) + cursor.execute(sql.SQL("GRANT EXECUTE ON FUNCTION {}.export_login_roles() TO {}").format( + sql.Identifier(names["schema"]), sql.Identifier(names["reader"]) + )) + + +def cleanup(connection, names): + succeeded = True + for username in (names["test"], names["control"]): + try: + admin_execute("DELETE FROM pgsql_users WHERE username=%s", (username,)) + except Exception: + succeeded = False + try: + admin_execute("LOAD PGSQL USERS TO RUNTIME") + admin_execute("SAVE PGSQL USERS TO DISK") + except Exception: + succeeded = False + if succeeded: + try: + succeeded = all( + admin_row(username, runtime=runtime) is None + for username in (names["test"], names["control"]) + for runtime in (False, True) + ) + except Exception: + succeeded = False + if connection is None: + return succeeded + try: + with connection.cursor() as cursor: + cursor.execute(sql.SQL("DROP SCHEMA IF EXISTS {} CASCADE").format( + sql.Identifier(names["schema"]) + )) + for role in (names["test"], names["reader"], names["allow"]): + cursor.execute(sql.SQL("DROP ROLE IF EXISTS {}").format(sql.Identifier(role))) + except Exception: + succeeded = False + return succeeded + + +def main(): + tap = Tap() + suffix = uuid.uuid4().hex[:16] + names = { + "allow": f"tap_psync_allow_{suffix}", + "reader": f"tap_psync_reader_{suffix}", + "test": f"tap_psync_user_{suffix}", + "schema": f"tap_psync_schema_{suffix}", + "control": f"tap_psync_control_{suffix}", + } + reader_password = secrets.token_urlsafe(24) + original_password = secrets.token_urlsafe(24) + rotated_password = secrets.token_urlsafe(24) + control_password = secrets.token_urlsafe(24) + source = None + config_path = None + + try: + source = pg_connection() + cleanup(source, names) + create_source_objects(source, names, reader_password, original_password) + with tempfile.TemporaryDirectory(prefix="pgsql-user-sync-") as temporary_directory: + workdir = Path(temporary_directory) + config_path = workdir / "pgsql-user-sync.ini" + write_config(config_path, names, reader_password, workdir) + + admin_execute( + "INSERT INTO pgsql_users " + "(username,password,active,use_ssl,default_hostgroup,transaction_persistent," + "fast_forward,backend,frontend,max_connections,attributes,comment) " + "VALUES (%s,%s,1,1,1777,0,1,1,1,17,%s,%s)", + (names["control"], control_password, "{\"operator\":true}", "tap control"), + ) + admin_execute("LOAD PGSQL USERS TO RUNTIME") + control_before = admin_row(names["control"]) + control_runtime_before = admin_row(names["control"], runtime=True) + tap.check(control_before is not None, "unmanaged control row exists") + + first = invoke(config_path) + tap.check(first.returncode == 0, "initial synchronizer run succeeds") + expected_verifier = verifier(source, names["test"]) + main_row = admin_row(names["test"]) + runtime_row = admin_row(names["test"], runtime=True) + tap.check(main_row is not None and runtime_row is not None, "sync creates main and runtime rows") + tap.check( + main_row is not None and runtime_row is not None + and main_row[4] == 0 and runtime_row[4] == 0, + "sync assigns configured hostgroup in main and runtime", + ) + tap.check(main_row is not None and main_row[1] == expected_verifier, "main verifier matches source") + tap.check(runtime_row is not None and runtime_row[1] == expected_verifier, "runtime verifier matches source") + tap.check(is_owned(main_row) and is_owned(runtime_row), "sync records managed ownership") + tap.check( + same_control_row(control_before, admin_row(names["control"])) + and same_control_row(control_runtime_before, admin_row(names["control"], runtime=True)), + "sync preserves unmanaged control rows", + ) + + verifier_auth = True + try: + frontend_login(names["test"], original_password) + except Exception: + verifier_auth = False + if os.environ.get("TAP_EXPECT_PGSQL_VERIFIER_AUTH") == "1": + tap.check(False, "frontend accepts synchronized verifier") + else: + tap.count += 1 + print( + f"ok {tap.count} - frontend verifier authentication " + "# SKIP ProxySQL verifier authentication dependency unavailable" + ) + else: + tap.check(True, "frontend accepts synchronized verifier") + try: + frontend_login(names["test"], "incorrect-password") + except Exception: + tap.check(True, "frontend rejects incorrect password") + else: + tap.check(False, "frontend rejects incorrect password") + + with source.cursor() as cursor: + cursor.execute(sql.SQL("ALTER ROLE {} PASSWORD %s").format( + sql.Identifier(names["test"]) + ), (rotated_password,)) + second = invoke(config_path) + tap.check(second.returncode == 0, "password rotation synchronizer run succeeds") + rotated_verifier = verifier(source, names["test"]) + rotated_main = admin_row(names["test"]) + rotated_runtime = admin_row(names["test"], runtime=True) + tap.check(rotated_verifier != expected_verifier, "source password rotation changes verifier") + tap.check(rotated_main is not None and rotated_main[1] == rotated_verifier, "main verifier rotates") + tap.check(rotated_runtime is not None and rotated_runtime[1] == rotated_verifier, "runtime verifier rotates") + if verifier_auth: + try: + frontend_login(names["test"], rotated_password) + except Exception: + tap.check(False, "frontend accepts rotated verifier") + else: + tap.check(True, "frontend accepts rotated verifier") + try: + frontend_login(names["test"], original_password) + except Exception: + tap.check(True, "frontend rejects previous password") + else: + tap.check(False, "frontend rejects previous password") + + with source.cursor() as cursor: + cursor.execute(sql.SQL("ALTER ROLE {} NOLOGIN").format(sql.Identifier(names["test"]))) + disabled = invoke(config_path) + tap.check(disabled.returncode == 0, "disable policy synchronizer run succeeds") + disabled_main = admin_row(names["test"]) + tap.check(disabled_main is not None and disabled_main[2] == 0, "missing role becomes inactive") + tap.check(admin_row(names["test"], runtime=True) is None, "inactive role is absent from runtime") + + with source.cursor() as cursor: + cursor.execute(sql.SQL("ALTER ROLE {} LOGIN").format(sql.Identifier(names["test"]))) + restored = invoke(config_path) + tap.check(restored.returncode == 0, "restored role synchronizer run succeeds") + with source.cursor() as cursor: + cursor.execute(sql.SQL("REVOKE {} FROM {}").format( + sql.Identifier(names["allow"]), sql.Identifier(names["test"]) + )) + kept = invoke(config_path, ("--missing-role-action", "keep")) + tap.check(kept.returncode == 0, "keep policy synchronizer run succeeds") + kept_main = admin_row(names["test"]) + kept_runtime = admin_row(names["test"], runtime=True) + tap.check(kept_main is not None and kept_main[2] == 1, "keep policy retains active main row") + tap.check(kept_runtime is not None and kept_runtime[2] == 1, "keep policy retains runtime row") + + runtime_before_failure = ( + admin_row(names["test"], runtime=True), + admin_row(names["control"], runtime=True), + ) + control_before_failure = admin_row(names["control"]) + with source.cursor() as cursor: + cursor.execute(sql.SQL("REVOKE EXECUTE ON FUNCTION {}.export_login_roles() FROM {}").format( + sql.Identifier(names["schema"]), sql.Identifier(names["reader"]) + )) + broken = invoke(config_path) + tap.check(broken.returncode != 0, "source-function failure stops synchronizer") + tap.check( + ( + admin_row(names["test"], runtime=True), + admin_row(names["control"], runtime=True), + ) == runtime_before_failure, + "source-function failure preserves runtime rows", + ) + tap.check( + same_control_row(control_before_failure, admin_row(names["control"])), + "source-function failure preserves unmanaged control row", + ) + except Exception: + tap.check(False, "lifecycle setup and control-plane assertions complete") + finally: + tap.check(cleanup(source, names), "cleanup removes test roles and ProxySQL rows") + if source is not None: + source.close() + if config_path is not None: + try: + config_path.unlink(missing_ok=True) + except OSError: + pass + + print(f"1..{tap.count}") + return 1 if tap.failures else 0 + + +if __name__ == "__main__": + raise SystemExit(main()) From 1007985c546913837af1154e27f4ca58cc400c7a Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Wed, 12 Aug 2026 16:17:26 +0000 Subject: [PATCH 12/18] fix: correct PostgreSQL user sync allow-list example --- test/tap/tests/Makefile | 3 --- tools/pgsql_user_sync/README.md | 4 ++-- tools/pgsql_user_sync/create_source_function.sql | 4 ++-- tools/pgsql_user_sync/tests/test_pgsql_user_sync.py | 4 ++++ 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/test/tap/tests/Makefile b/test/tap/tests/Makefile index 16cd37e7ce..849f2abaaf 100644 --- a/test/tap/tests/Makefile +++ b/test/tap/tests/Makefile @@ -257,9 +257,6 @@ py-%: cp $(patsubst py-%,%,$@) $(patsubst py-%.py,%,$@) chmod +x $(patsubst py-%.py,%,$@) -.PHONY: pgsql-user-sync-t -pgsql-user-sync-t: py-pgsql-user-sync-t.py - sh-%: cp $(patsubst sh-%,%,$@) $(patsubst sh-%.sh,%,$@) chmod +x $(patsubst sh-%.sh,%,$@) diff --git a/tools/pgsql_user_sync/README.md b/tools/pgsql_user_sync/README.md index 3156249602..560a887205 100644 --- a/tools/pgsql_user_sync/README.md +++ b/tools/pgsql_user_sync/README.md @@ -49,8 +49,8 @@ The `proxysql_auth_managed` `NOLOGIN` role is the import allow-list. Grant and revoke membership deliberately, for example: ```sql -GRANT app_login TO proxysql_auth_managed; -REVOKE app_login FROM proxysql_auth_managed; +GRANT proxysql_auth_managed TO app_login; +REVOKE proxysql_auth_managed FROM app_login; ``` The function additionally requires a login-capable role, a non-null verifier, diff --git a/tools/pgsql_user_sync/create_source_function.sql b/tools/pgsql_user_sync/create_source_function.sql index f846cfa82b..a184bbcbe7 100644 --- a/tools/pgsql_user_sync/create_source_function.sql +++ b/tools/pgsql_user_sync/create_source_function.sql @@ -52,5 +52,5 @@ GRANT USAGE ON SCHEMA proxysql_auth TO proxysql_auth_reader; GRANT EXECUTE ON FUNCTION proxysql_auth.export_login_roles() TO proxysql_auth_reader; -- Allow-list examples (run separately, one role at a time): --- GRANT app_login TO proxysql_auth_managed; --- REVOKE app_login FROM proxysql_auth_managed; +-- GRANT proxysql_auth_managed TO app_login; +-- REVOKE proxysql_auth_managed FROM app_login; diff --git a/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py b/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py index a5311f5a58..42f646bf44 100644 --- a/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py +++ b/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py @@ -722,6 +722,10 @@ def test_source_function_has_security_boundaries(self): self.assertIn("proxysql_auth_managed", sql) self.assertIn("pg_has_role", sql) self.assertIn("proxysql_auth_reader", sql) + self.assertIn("GRANT proxysql_auth_managed TO app_login;", sql) + self.assertIn("REVOKE proxysql_auth_managed FROM app_login;", sql) + self.assertNotIn("GRANT app_login TO proxysql_auth_managed;", sql) + self.assertNotIn("REVOKE app_login FROM proxysql_auth_managed;", sql) def test_example_configuration_is_loadable_with_protected_permissions(self): source = (ASSET_DIR / "proxysql_pgsql_user_sync.ini.example").read_text() From a3592c2bb693f888165d2667889eee349104f42f Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Wed, 12 Aug 2026 16:23:26 +0000 Subject: [PATCH 13/18] test: keep PostgreSQL user sync lifecycle source nonempty --- test/tap/tests/pgsql-user-sync-t.py | 105 +++++++++++++++++++++------- 1 file changed, 78 insertions(+), 27 deletions(-) diff --git a/test/tap/tests/pgsql-user-sync-t.py b/test/tap/tests/pgsql-user-sync-t.py index daaa339e0e..b8702b26c3 100644 --- a/test/tap/tests/pgsql-user-sync-t.py +++ b/test/tap/tests/pgsql-user-sync-t.py @@ -96,10 +96,26 @@ def verifier(connection, role): return value[0] -def frontend_login(username, password): +def frontend_login(username, password, dbname): connection = psycopg.connect( host=env("TAP_PGSQL_HOST", "127.0.0.1"), port=int(env("TAP_PGSQL_PORT", "6133")), + dbname=dbname, + user=username, + password=password, + connect_timeout=5, + ) + try: + with connection.cursor() as cursor: + cursor.execute("SELECT 1") + finally: + connection.close() + + +def backend_login(username, password): + connection = psycopg.connect( + host=env("TAP_PGSQLSERVER_HOST"), + port=int(env("TAP_PGSQLSERVER_PORT", "5432")), dbname="postgres", user=username, password=password, @@ -165,7 +181,7 @@ def invoke(config_path, extra=()): return subprocess.run(command, text=True, capture_output=True) -def create_source_objects(connection, names, reader_password, test_password): +def create_source_objects(connection, names, reader_password, test_password, sentinel_password): with connection.cursor() as cursor: cursor.execute(sql.SQL("CREATE ROLE {} NOLOGIN").format(sql.Identifier(names["allow"]))) cursor.execute( @@ -176,11 +192,20 @@ def create_source_objects(connection, names, reader_password, test_password): sql.SQL("CREATE ROLE {} LOGIN PASSWORD %s").format(sql.Identifier(names["test"])), (test_password,), ) + cursor.execute( + sql.SQL("CREATE ROLE {} LOGIN PASSWORD %s").format(sql.Identifier(names["sentinel"])), + (sentinel_password,), + ) cursor.execute( sql.SQL("GRANT {} TO {}").format( sql.Identifier(names["allow"]), sql.Identifier(names["test"]) ) ) + cursor.execute( + sql.SQL("GRANT {} TO {}").format( + sql.Identifier(names["allow"]), sql.Identifier(names["sentinel"]) + ) + ) cursor.execute(sql.SQL("CREATE SCHEMA {} AUTHORIZATION CURRENT_USER").format( sql.Identifier(names["schema"]) )) @@ -210,7 +235,7 @@ def create_source_objects(connection, names, reader_password, test_password): def cleanup(connection, names): succeeded = True - for username in (names["test"], names["control"]): + for username in (names["test"], names["sentinel"], names["control"]): try: admin_execute("DELETE FROM pgsql_users WHERE username=%s", (username,)) except Exception: @@ -224,7 +249,7 @@ def cleanup(connection, names): try: succeeded = all( admin_row(username, runtime=runtime) is None - for username in (names["test"], names["control"]) + for username in (names["test"], names["sentinel"], names["control"]) for runtime in (False, True) ) except Exception: @@ -236,7 +261,7 @@ def cleanup(connection, names): cursor.execute(sql.SQL("DROP SCHEMA IF EXISTS {} CASCADE").format( sql.Identifier(names["schema"]) )) - for role in (names["test"], names["reader"], names["allow"]): + for role in (names["test"], names["sentinel"], names["reader"], names["allow"]): cursor.execute(sql.SQL("DROP ROLE IF EXISTS {}").format(sql.Identifier(role))) except Exception: succeeded = False @@ -249,6 +274,7 @@ def main(): names = { "allow": f"tap_psync_allow_{suffix}", "reader": f"tap_psync_reader_{suffix}", + "sentinel": f"tap_psync_sentinel_{suffix}", "test": f"tap_psync_user_{suffix}", "schema": f"tap_psync_schema_{suffix}", "control": f"tap_psync_control_{suffix}", @@ -256,6 +282,7 @@ def main(): reader_password = secrets.token_urlsafe(24) original_password = secrets.token_urlsafe(24) rotated_password = secrets.token_urlsafe(24) + sentinel_password = secrets.token_urlsafe(24) control_password = secrets.token_urlsafe(24) source = None config_path = None @@ -263,7 +290,9 @@ def main(): try: source = pg_connection() cleanup(source, names) - create_source_objects(source, names, reader_password, original_password) + create_source_objects( + source, names, reader_password, original_password, sentinel_password + ) with tempfile.TemporaryDirectory(prefix="pgsql-user-sync-") as temporary_directory: workdir = Path(temporary_directory) config_path = workdir / "pgsql-user-sync.ini" @@ -301,27 +330,46 @@ def main(): "sync preserves unmanaged control rows", ) - verifier_auth = True + frontend_baseline = True try: - frontend_login(names["test"], original_password) + known_username = env("TAP_PGSQL_USERNAME", "testuser") + frontend_login(known_username, env("TAP_PGSQL_PASSWORD", "testuser"), known_username) except Exception: - verifier_auth = False - if os.environ.get("TAP_EXPECT_PGSQL_VERIFIER_AUTH") == "1": - tap.check(False, "frontend accepts synchronized verifier") - else: - tap.count += 1 - print( - f"ok {tap.count} - frontend verifier authentication " - "# SKIP ProxySQL verifier authentication dependency unavailable" - ) + frontend_baseline = False + tap.check(False, "known-good frontend PostgreSQL login succeeds") else: - tap.check(True, "frontend accepts synchronized verifier") + tap.check(True, "known-good frontend PostgreSQL login succeeds") + backend_baseline = True + try: + backend_login(names["test"], original_password) + except Exception: + backend_baseline = False + tap.check(False, "generated role direct backend login succeeds") + else: + tap.check(True, "generated role direct backend login succeeds") + + verifier_auth = frontend_baseline and backend_baseline + if verifier_auth: try: - frontend_login(names["test"], "incorrect-password") + frontend_login(names["test"], original_password, "postgres") except Exception: - tap.check(True, "frontend rejects incorrect password") + verifier_auth = False + if os.environ.get("TAP_EXPECT_PGSQL_VERIFIER_AUTH") == "1": + tap.check(False, "frontend accepts synchronized verifier") + else: + tap.count += 1 + print( + f"ok {tap.count} - frontend verifier authentication " + "# SKIP ProxySQL verifier authentication dependency unavailable" + ) else: - tap.check(False, "frontend rejects incorrect password") + tap.check(True, "frontend accepts synchronized verifier") + try: + frontend_login(names["test"], "incorrect-password", "postgres") + except Exception: + tap.check(True, "frontend rejects incorrect password") + else: + tap.check(False, "frontend rejects incorrect password") with source.cursor() as cursor: cursor.execute(sql.SQL("ALTER ROLE {} PASSWORD %s").format( @@ -337,13 +385,13 @@ def main(): tap.check(rotated_runtime is not None and rotated_runtime[1] == rotated_verifier, "runtime verifier rotates") if verifier_auth: try: - frontend_login(names["test"], rotated_password) + frontend_login(names["test"], rotated_password, "postgres") except Exception: tap.check(False, "frontend accepts rotated verifier") else: tap.check(True, "frontend accepts rotated verifier") try: - frontend_login(names["test"], original_password) + frontend_login(names["test"], original_password, "postgres") except Exception: tap.check(True, "frontend rejects previous password") else: @@ -374,9 +422,9 @@ def main(): runtime_before_failure = ( admin_row(names["test"], runtime=True), + admin_row(names["sentinel"], runtime=True), admin_row(names["control"], runtime=True), ) - control_before_failure = admin_row(names["control"]) with source.cursor() as cursor: cursor.execute(sql.SQL("REVOKE EXECUTE ON FUNCTION {}.export_login_roles() FROM {}").format( sql.Identifier(names["schema"]), sql.Identifier(names["reader"]) @@ -386,15 +434,18 @@ def main(): tap.check( ( admin_row(names["test"], runtime=True), + admin_row(names["sentinel"], runtime=True), admin_row(names["control"], runtime=True), ) == runtime_before_failure, "source-function failure preserves runtime rows", ) tap.check( - same_control_row(control_before_failure, admin_row(names["control"])), - "source-function failure preserves unmanaged control row", + same_control_row(control_before, admin_row(names["control"])) + and same_control_row(control_runtime_before, admin_row(names["control"], runtime=True)), + "source-function failure preserves original unmanaged control rows", ) - except Exception: + except Exception as error: + tap.diag(f"lifecycle exception class: {type(error).__name__}") tap.check(False, "lifecycle setup and control-plane assertions complete") finally: tap.check(cleanup(source, names), "cleanup removes test roles and ProxySQL rows") From 49b6fe22094b9b5b59360fa121e096a57600a02b Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Wed, 12 Aug 2026 17:04:50 +0000 Subject: [PATCH 14/18] test: exercise PostgreSQL user sync on live services --- .../2026-08-12-pgsql-user-sync-design.md | 8 +- test/tap/tests/pgsql-user-sync-t.py | 511 ++++++------------ tools/pgsql_user_sync/README.md | 3 + .../proxysql_pgsql_user_sync.ini.example | 2 +- .../proxysql_pgsql_user_sync.py | 18 +- tools/pgsql_user_sync/requirements.txt | 1 - .../tests/test_pgsql_user_sync.py | 9 +- 7 files changed, 186 insertions(+), 366 deletions(-) diff --git a/docs/superpowers/specs/2026-08-12-pgsql-user-sync-design.md b/docs/superpowers/specs/2026-08-12-pgsql-user-sync-design.md index 0e1063d02f..3217d99363 100644 --- a/docs/superpowers/specs/2026-08-12-pgsql-user-sync-design.md +++ b/docs/superpowers/specs/2026-08-12-pgsql-user-sync-design.md @@ -58,9 +58,9 @@ tools/pgsql_user_sync/ └── test_pgsql_user_sync.py ``` -The Python implementation uses `psycopg` for PostgreSQL and `PyMySQL` for the -ProxySQL Admin interface. Database access is isolated behind small adapters so -the reconciliation logic can be tested without live services. +The Python implementation uses `psycopg` for PostgreSQL and ProxySQL's +PostgreSQL Admin interface. Database access is isolated behind small adapters +so the reconciliation logic can be tested without live services. ## Source role selection @@ -107,7 +107,7 @@ function = proxysql_auth.export_login_roles [proxysql] host = 127.0.0.1 -port = 6032 +port = 6132 username = admin password = admin-secret connect_timeout = 10 diff --git a/test/tap/tests/pgsql-user-sync-t.py b/test/tap/tests/pgsql-user-sync-t.py index b8702b26c3..be1e56bd63 100644 --- a/test/tap/tests/pgsql-user-sync-t.py +++ b/test/tap/tests/pgsql-user-sync-t.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -"""Exercise PostgreSQL verifier synchronization against a real ProxySQL.""" +"""Run the PostgreSQL user synchronizer against real services.""" import json import os @@ -10,18 +10,14 @@ import uuid from pathlib import Path -import pymysql import psycopg +import pymysql from psycopg import sql ROOT = Path(os.environ.get("WORKSPACE", Path(__file__).resolve().parents[3])) SCRIPT = ROOT / "tools/pgsql_user_sync/proxysql_pgsql_user_sync.py" -USER_COLUMNS = ( - "username,password,active,use_ssl,default_hostgroup,transaction_persistent," - "fast_forward,backend,frontend,max_connections,attributes,comment" -) -PROFILE = "tap-lifecycle" +PROFILE = "tap-real" class Tap: @@ -31,25 +27,19 @@ def __init__(self): def check(self, condition, description): self.count += 1 - if condition: - print(f"ok {self.count} - {description}") - else: + print(f"{'ok' if condition else 'not ok'} {self.count} - {description}") + if not condition: self.failures += 1 - print(f"not ok {self.count} - {description}") - return condition - - def diag(self, description): - print(f"# {description}") def env(name, default=None): value = os.environ.get(name, default) - if value is None or value == "": + if not value: raise RuntimeError(f"required environment variable {name} is not set") return value -def pg_connection(): +def source_connection(): return psycopg.connect( host=env("TAP_PGSQLSERVER_HOST"), port=int(env("TAP_PGSQLSERVER_PORT", "5432")), @@ -82,380 +72,195 @@ def admin_row(username, runtime=False): with admin_connection() as connection: with connection.cursor() as cursor: cursor.execute( - f"SELECT {USER_COLUMNS} FROM {table} WHERE username=%s", (username,) + f"SELECT username,password,active,default_hostgroup,attributes " + f"FROM {table} WHERE username=%s", + (username,), ) return cursor.fetchone() -def verifier(connection, role): +def create_source_role(connection, schema, username, password): with connection.cursor() as cursor: - cursor.execute("SELECT rolpassword FROM pg_catalog.pg_authid WHERE rolname=%s", (role,)) - value = cursor.fetchone() - if value is None or value[0] is None: - raise RuntimeError("test role verifier was not created") - return value[0] - - -def frontend_login(username, password, dbname): - connection = psycopg.connect( - host=env("TAP_PGSQL_HOST", "127.0.0.1"), - port=int(env("TAP_PGSQL_PORT", "6133")), - dbname=dbname, - user=username, - password=password, - connect_timeout=5, - ) - try: - with connection.cursor() as cursor: - cursor.execute("SELECT 1") - finally: - connection.close() - - -def backend_login(username, password): - connection = psycopg.connect( - host=env("TAP_PGSQLSERVER_HOST"), - port=int(env("TAP_PGSQLSERVER_PORT", "5432")), - dbname="postgres", - user=username, - password=password, - connect_timeout=5, - ) - try: - with connection.cursor() as cursor: - cursor.execute("SELECT 1") - finally: - connection.close() - - -def is_owned(row): - if row is None: - return False - try: - return json.loads(row[10]).get("proxysql_pgsql_user_sync") == {"profile": PROFILE} - except (TypeError, ValueError, AttributeError): - return False + cursor.execute( + sql.SQL("CREATE ROLE {} LOGIN PASSWORD {}").format( + sql.Identifier(username), sql.Literal(password) + ) + ) + cursor.execute( + sql.SQL("CREATE SCHEMA {} AUTHORIZATION CURRENT_USER").format( + sql.Identifier(schema) + ) + ) + cursor.execute( + sql.SQL( + "CREATE FUNCTION {}.export_login_role() " + "RETURNS TABLE(username text, password text) " + "LANGUAGE sql SECURITY DEFINER SET search_path = pg_catalog AS $$ " + "SELECT rolname::text, rolpassword FROM pg_catalog.pg_authid " + "WHERE rolname = {} AND rolcanlogin AND rolpassword IS NOT NULL $$" + ).format(sql.Identifier(schema), sql.Literal(username)) + ) -def same_control_row(before, after): - return before is not None and before == after and not is_owned(after) +def source_verifier(connection, username): + with connection.cursor() as cursor: + cursor.execute( + "SELECT rolpassword FROM pg_catalog.pg_authid WHERE rolname=%s", + (username,), + ) + return cursor.fetchone()[0] -def write_config(path, names, reader_password, workdir): +def write_config(path, schema, workdir): path.write_text( - "\n".join(( - "[source]", - f"host = {env('TAP_PGSQLSERVER_HOST')}", - f"port = {env('TAP_PGSQLSERVER_PORT', '5432')}", - "database = postgres", - f"username = {names['reader']}", - f"password = {reader_password}", - "connect_timeout = 10", - f"function = {names['schema']}.export_login_roles", - "", - "[proxysql]", - f"host = {env('TAP_ADMINHOST', '127.0.0.1')}", - f"port = {env('TAP_ADMINPORT', '6032')}", - f"username = {env('TAP_ADMINUSERNAME', 'radmin')}", - f"password = {env('TAP_ADMINPASSWORD', 'radmin')}", - "connect_timeout = 10", - "", - "[sync]", - f"profile = {PROFILE}", - "default_hostgroup = 0", - "missing_role_action = disable", - "adopt_existing_users = false", - "allow_empty_snapshot = false", - "save_to_disk = true", - f"lock_file = {workdir / 'pgsql-user-sync.lock'}", - "", - )), + "\n".join( + ( + "[source]", + f"host = {env('TAP_PGSQLSERVER_HOST')}", + f"port = {env('TAP_PGSQLSERVER_PORT', '5432')}", + "database = postgres", + f"username = {env('TAP_PGSQLSERVER_USERNAME')}", + f"password = {env('TAP_PGSQLSERVER_PASSWORD')}", + f"function = {schema}.export_login_role", + "", + "[proxysql]", + f"host = {env('TAP_PGSQLADMIN_HOST', '127.0.0.1')}", + f"port = {env('TAP_PGSQLADMIN_PORT', '6132')}", + f"username = {env('TAP_ADMINUSERNAME', 'radmin')}", + f"password = {env('TAP_ADMINPASSWORD', 'radmin')}", + "", + "[sync]", + f"profile = {PROFILE}", + "default_hostgroup = 0", + "missing_role_action = disable", + "adopt_existing_users = false", + "allow_empty_snapshot = false", + "save_to_disk = true", + f"lock_file = {workdir / 'pgsql-user-sync.lock'}", + "", + ) + ), encoding="utf-8", ) os.chmod(path, 0o600) -def invoke(config_path, extra=()): - command = [sys.executable, str(SCRIPT), "--config", str(config_path)] - command.extend(extra) - return subprocess.run(command, text=True, capture_output=True) +def direct_login(username, password): + with psycopg.connect( + host=env("TAP_PGSQLSERVER_HOST"), + port=int(env("TAP_PGSQLSERVER_PORT", "5432")), + dbname="postgres", + user=username, + password=password, + connect_timeout=5, + ) as connection: + connection.execute("SELECT 1") -def create_source_objects(connection, names, reader_password, test_password, sentinel_password): - with connection.cursor() as cursor: - cursor.execute(sql.SQL("CREATE ROLE {} NOLOGIN").format(sql.Identifier(names["allow"]))) - cursor.execute( - sql.SQL("CREATE ROLE {} LOGIN PASSWORD %s").format(sql.Identifier(names["reader"])), - (reader_password,), - ) - cursor.execute( - sql.SQL("CREATE ROLE {} LOGIN PASSWORD %s").format(sql.Identifier(names["test"])), - (test_password,), - ) - cursor.execute( - sql.SQL("CREATE ROLE {} LOGIN PASSWORD %s").format(sql.Identifier(names["sentinel"])), - (sentinel_password,), - ) - cursor.execute( - sql.SQL("GRANT {} TO {}").format( - sql.Identifier(names["allow"]), sql.Identifier(names["test"]) - ) - ) - cursor.execute( - sql.SQL("GRANT {} TO {}").format( - sql.Identifier(names["allow"]), sql.Identifier(names["sentinel"]) - ) - ) - cursor.execute(sql.SQL("CREATE SCHEMA {} AUTHORIZATION CURRENT_USER").format( - sql.Identifier(names["schema"]) - )) - function = sql.SQL( - "CREATE FUNCTION {}.export_login_roles() " - "RETURNS TABLE(username text, password text) LANGUAGE sql SECURITY DEFINER " - "SET search_path = pg_catalog AS $$ " - "SELECT r.rolname::text, r.rolpassword FROM pg_catalog.pg_authid AS r " - "WHERE r.rolcanlogin AND r.rolpassword IS NOT NULL " - "AND (r.rolvaliduntil IS NULL OR r.rolvaliduntil > pg_catalog.now()) " - "AND pg_catalog.pg_has_role(r.oid, {}, 'member') ORDER BY r.rolname $$" - ).format(sql.Identifier(names["schema"]), sql.Literal(names["allow"])) - cursor.execute(function) - cursor.execute(sql.SQL("REVOKE ALL ON SCHEMA {} FROM PUBLIC").format( - sql.Identifier(names["schema"]) - )) - cursor.execute(sql.SQL("REVOKE ALL ON FUNCTION {}.export_login_roles() FROM PUBLIC").format( - sql.Identifier(names["schema"]) - )) - cursor.execute(sql.SQL("GRANT USAGE ON SCHEMA {} TO {}").format( - sql.Identifier(names["schema"]), sql.Identifier(names["reader"]) - )) - cursor.execute(sql.SQL("GRANT EXECUTE ON FUNCTION {}.export_login_roles() TO {}").format( - sql.Identifier(names["schema"]), sql.Identifier(names["reader"]) - )) - - -def cleanup(connection, names): - succeeded = True - for username in (names["test"], names["sentinel"], names["control"]): - try: - admin_execute("DELETE FROM pgsql_users WHERE username=%s", (username,)) - except Exception: - succeeded = False +def cleanup(connection, schema, username): + success = True try: + admin_execute("DELETE FROM pgsql_users WHERE username=%s", (username,)) admin_execute("LOAD PGSQL USERS TO RUNTIME") admin_execute("SAVE PGSQL USERS TO DISK") except Exception: - succeeded = False - if succeeded: + success = False + if connection is not None: try: - succeeded = all( - admin_row(username, runtime=runtime) is None - for username in (names["test"], names["sentinel"], names["control"]) - for runtime in (False, True) - ) + with connection.cursor() as cursor: + cursor.execute( + sql.SQL("DROP SCHEMA IF EXISTS {} CASCADE").format( + sql.Identifier(schema) + ) + ) + cursor.execute( + sql.SQL("DROP ROLE IF EXISTS {}").format(sql.Identifier(username)) + ) except Exception: - succeeded = False - if connection is None: - return succeeded - try: - with connection.cursor() as cursor: - cursor.execute(sql.SQL("DROP SCHEMA IF EXISTS {} CASCADE").format( - sql.Identifier(names["schema"]) - )) - for role in (names["test"], names["sentinel"], names["reader"], names["allow"]): - cursor.execute(sql.SQL("DROP ROLE IF EXISTS {}").format(sql.Identifier(role))) - except Exception: - succeeded = False - return succeeded + success = False + return success def main(): tap = Tap() suffix = uuid.uuid4().hex[:16] - names = { - "allow": f"tap_psync_allow_{suffix}", - "reader": f"tap_psync_reader_{suffix}", - "sentinel": f"tap_psync_sentinel_{suffix}", - "test": f"tap_psync_user_{suffix}", - "schema": f"tap_psync_schema_{suffix}", - "control": f"tap_psync_control_{suffix}", - } - reader_password = secrets.token_urlsafe(24) - original_password = secrets.token_urlsafe(24) - rotated_password = secrets.token_urlsafe(24) - sentinel_password = secrets.token_urlsafe(24) - control_password = secrets.token_urlsafe(24) + username = f"tap_psync_{suffix}" + schema = f"tap_psync_schema_{suffix}" + password = secrets.token_urlsafe(24) source = None - config_path = None try: - source = pg_connection() - cleanup(source, names) - create_source_objects( - source, names, reader_password, original_password, sentinel_password - ) - with tempfile.TemporaryDirectory(prefix="pgsql-user-sync-") as temporary_directory: - workdir = Path(temporary_directory) - config_path = workdir / "pgsql-user-sync.ini" - write_config(config_path, names, reader_password, workdir) - - admin_execute( - "INSERT INTO pgsql_users " - "(username,password,active,use_ssl,default_hostgroup,transaction_persistent," - "fast_forward,backend,frontend,max_connections,attributes,comment) " - "VALUES (%s,%s,1,1,1777,0,1,1,1,17,%s,%s)", - (names["control"], control_password, "{\"operator\":true}", "tap control"), - ) - admin_execute("LOAD PGSQL USERS TO RUNTIME") - control_before = admin_row(names["control"]) - control_runtime_before = admin_row(names["control"], runtime=True) - tap.check(control_before is not None, "unmanaged control row exists") - - first = invoke(config_path) - tap.check(first.returncode == 0, "initial synchronizer run succeeds") - expected_verifier = verifier(source, names["test"]) - main_row = admin_row(names["test"]) - runtime_row = admin_row(names["test"], runtime=True) - tap.check(main_row is not None and runtime_row is not None, "sync creates main and runtime rows") - tap.check( - main_row is not None and runtime_row is not None - and main_row[4] == 0 and runtime_row[4] == 0, - "sync assigns configured hostgroup in main and runtime", - ) - tap.check(main_row is not None and main_row[1] == expected_verifier, "main verifier matches source") - tap.check(runtime_row is not None and runtime_row[1] == expected_verifier, "runtime verifier matches source") - tap.check(is_owned(main_row) and is_owned(runtime_row), "sync records managed ownership") - tap.check( - same_control_row(control_before, admin_row(names["control"])) - and same_control_row(control_runtime_before, admin_row(names["control"], runtime=True)), - "sync preserves unmanaged control rows", + source = source_connection() + create_source_role(source, schema, username, password) + direct_login(username, password) + tap.check(True, "login role exists on the real PostgreSQL backend") + + with tempfile.TemporaryDirectory(prefix="pgsql-user-sync-") as temp: + workdir = Path(temp) + config = workdir / "pgsql-user-sync.ini" + write_config(config, schema, workdir) + result = subprocess.run( + [sys.executable, str(SCRIPT), "--config", str(config)], + text=True, + capture_output=True, ) - - frontend_baseline = True - try: - known_username = env("TAP_PGSQL_USERNAME", "testuser") - frontend_login(known_username, env("TAP_PGSQL_PASSWORD", "testuser"), known_username) - except Exception: - frontend_baseline = False - tap.check(False, "known-good frontend PostgreSQL login succeeds") - else: - tap.check(True, "known-good frontend PostgreSQL login succeeds") - backend_baseline = True - try: - backend_login(names["test"], original_password) - except Exception: - backend_baseline = False - tap.check(False, "generated role direct backend login succeeds") - else: - tap.check(True, "generated role direct backend login succeeds") - - verifier_auth = frontend_baseline and backend_baseline - if verifier_auth: - try: - frontend_login(names["test"], original_password, "postgres") - except Exception: - verifier_auth = False - if os.environ.get("TAP_EXPECT_PGSQL_VERIFIER_AUTH") == "1": - tap.check(False, "frontend accepts synchronized verifier") - else: - tap.count += 1 - print( - f"ok {tap.count} - frontend verifier authentication " - "# SKIP ProxySQL verifier authentication dependency unavailable" - ) - else: - tap.check(True, "frontend accepts synchronized verifier") - try: - frontend_login(names["test"], "incorrect-password", "postgres") - except Exception: - tap.check(True, "frontend rejects incorrect password") - else: - tap.check(False, "frontend rejects incorrect password") - - with source.cursor() as cursor: - cursor.execute(sql.SQL("ALTER ROLE {} PASSWORD %s").format( - sql.Identifier(names["test"]) - ), (rotated_password,)) - second = invoke(config_path) - tap.check(second.returncode == 0, "password rotation synchronizer run succeeds") - rotated_verifier = verifier(source, names["test"]) - rotated_main = admin_row(names["test"]) - rotated_runtime = admin_row(names["test"], runtime=True) - tap.check(rotated_verifier != expected_verifier, "source password rotation changes verifier") - tap.check(rotated_main is not None and rotated_main[1] == rotated_verifier, "main verifier rotates") - tap.check(rotated_runtime is not None and rotated_runtime[1] == rotated_verifier, "runtime verifier rotates") - if verifier_auth: - try: - frontend_login(names["test"], rotated_password, "postgres") - except Exception: - tap.check(False, "frontend accepts rotated verifier") - else: - tap.check(True, "frontend accepts rotated verifier") - try: - frontend_login(names["test"], original_password, "postgres") - except Exception: - tap.check(True, "frontend rejects previous password") - else: - tap.check(False, "frontend rejects previous password") - - with source.cursor() as cursor: - cursor.execute(sql.SQL("ALTER ROLE {} NOLOGIN").format(sql.Identifier(names["test"]))) - disabled = invoke(config_path) - tap.check(disabled.returncode == 0, "disable policy synchronizer run succeeds") - disabled_main = admin_row(names["test"]) - tap.check(disabled_main is not None and disabled_main[2] == 0, "missing role becomes inactive") - tap.check(admin_row(names["test"], runtime=True) is None, "inactive role is absent from runtime") - - with source.cursor() as cursor: - cursor.execute(sql.SQL("ALTER ROLE {} LOGIN").format(sql.Identifier(names["test"]))) - restored = invoke(config_path) - tap.check(restored.returncode == 0, "restored role synchronizer run succeeds") - with source.cursor() as cursor: - cursor.execute(sql.SQL("REVOKE {} FROM {}").format( - sql.Identifier(names["allow"]), sql.Identifier(names["test"]) - )) - kept = invoke(config_path, ("--missing-role-action", "keep")) - tap.check(kept.returncode == 0, "keep policy synchronizer run succeeds") - kept_main = admin_row(names["test"]) - kept_runtime = admin_row(names["test"], runtime=True) - tap.check(kept_main is not None and kept_main[2] == 1, "keep policy retains active main row") - tap.check(kept_runtime is not None and kept_runtime[2] == 1, "keep policy retains runtime row") - - runtime_before_failure = ( - admin_row(names["test"], runtime=True), - admin_row(names["sentinel"], runtime=True), - admin_row(names["control"], runtime=True), - ) - with source.cursor() as cursor: - cursor.execute(sql.SQL("REVOKE EXECUTE ON FUNCTION {}.export_login_roles() FROM {}").format( - sql.Identifier(names["schema"]), sql.Identifier(names["reader"]) - )) - broken = invoke(config_path) - tap.check(broken.returncode != 0, "source-function failure stops synchronizer") - tap.check( - ( - admin_row(names["test"], runtime=True), - admin_row(names["sentinel"], runtime=True), - admin_row(names["control"], runtime=True), - ) == runtime_before_failure, - "source-function failure preserves runtime rows", + if result.returncode != 0: + print(f"# synchronizer diagnostic: {result.stderr.strip()}") + + tap.check(result.returncode == 0, "real synchronizer run succeeds") + verifier = source_verifier(source, username) + main_row = admin_row(username) + runtime_row = admin_row(username, runtime=True) + if ( + main_row is None + or main_row[1] != verifier + or int(main_row[2]) != 1 + or int(main_row[3]) != 0 + ): + print( + "# main row diagnostic: " + f"present={main_row is not None} " + f"verifier_match={main_row is not None and main_row[1] == verifier} " + f"active={main_row[2] if main_row else None} " + f"hostgroup={main_row[3] if main_row else None} " + f"attributes={(main_row[4] if main_row else None)!r}" ) - tap.check( - same_control_row(control_before, admin_row(names["control"])) - and same_control_row(control_runtime_before, admin_row(names["control"], runtime=True)), - "source-function failure preserves original unmanaged control rows", + if ( + runtime_row is None + or runtime_row[1] != verifier + or int(runtime_row[2]) != 1 + or int(runtime_row[3]) != 0 + ): + print( + "# runtime row diagnostic: " + f"present={runtime_row is not None} " + f"verifier_match={runtime_row is not None and runtime_row[1] == verifier} " + f"active={runtime_row[2] if runtime_row else None} " + f"hostgroup={runtime_row[3] if runtime_row else None}" ) + tap.check( + main_row is not None + and main_row[1] == verifier + and int(main_row[2]) == 1 + and int(main_row[3]) == 0 + and json.loads(main_row[4]).get("proxysql_pgsql_user_sync") + == {"profile": PROFILE}, + "synchronizer automatically creates the real pgsql_users row", + ) + tap.check( + runtime_row is not None + and runtime_row[1] == verifier + and int(runtime_row[2]) == 1 + and int(runtime_row[3]) == 0, + "synchronizer loads the user into runtime_pgsql_users", + ) except Exception as error: - tap.diag(f"lifecycle exception class: {type(error).__name__}") - tap.check(False, "lifecycle setup and control-plane assertions complete") + print(f"# real integration failure class: {type(error).__name__}") + tap.check(False, "real PostgreSQL to ProxySQL synchronization completes") finally: - tap.check(cleanup(source, names), "cleanup removes test roles and ProxySQL rows") + tap.check(cleanup(source, schema, username), "test data is removed") if source is not None: source.close() - if config_path is not None: - try: - config_path.unlink(missing_ok=True) - except OSError: - pass print(f"1..{tap.count}") return 1 if tap.failures else 0 diff --git a/tools/pgsql_user_sync/README.md b/tools/pgsql_user_sync/README.md index 560a887205..8add735eee 100644 --- a/tools/pgsql_user_sync/README.md +++ b/tools/pgsql_user_sync/README.md @@ -28,6 +28,9 @@ obvious password placeholders, and restrict it before entering real secrets: install -o proxysql -g proxysql -m 0600 proxysql_pgsql_user_sync.ini.example /etc/proxysql/pgsql-user-sync.ini ``` +Set `[proxysql]` to ProxySQL's PostgreSQL Admin interface (normally port +`6132`). The synchronizer uses PostgreSQL's simple-query protocol there. + The synchronizer rejects group-write/execute and other-user permissions. The primary example is service-owned `0600` so the ProxySQL service account can read it. Alternatively, keep the file root-owned and use `chown root:proxysql` diff --git a/tools/pgsql_user_sync/proxysql_pgsql_user_sync.ini.example b/tools/pgsql_user_sync/proxysql_pgsql_user_sync.ini.example index e74e22ec76..1151185210 100644 --- a/tools/pgsql_user_sync/proxysql_pgsql_user_sync.ini.example +++ b/tools/pgsql_user_sync/proxysql_pgsql_user_sync.ini.example @@ -12,7 +12,7 @@ function = proxysql_auth.export_login_roles [proxysql] host = 127.0.0.1 -port = 6032 +port = 6132 username = admin password = REPLACE_WITH_PROXYSQL_PASSWORD connect_timeout = 10 diff --git a/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py b/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py index 8e3e7119df..325dcca26c 100644 --- a/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py +++ b/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py @@ -682,12 +682,18 @@ def __init__(self, config: AppConfig, *, connect: Callable[..., object] | None = def _connection(self) -> object: connect = self._connect + connect_options: dict[str, object] = {} if connect is None: try: - import pymysql + import psycopg + from psycopg import ClientCursor except ImportError: raise _error("ProxySQL admin driver is not installed") from None - connect = pymysql.connect + connect = psycopg.connect + # ProxySQL's PostgreSQL Admin interface supports the simple query + # protocol. ClientCursor still quotes parameters safely, but does + # the interpolation client-side before sending the statement. + connect_options["cursor_factory"] = ClientCursor proxy = self.config.proxysql try: return connect( @@ -695,9 +701,10 @@ def _connection(self) -> object: port=proxy.port, user=proxy.username, password=proxy.password, - database="main", + dbname="main", connect_timeout=proxy.connect_timeout, autocommit=True, + **connect_options, ) except Exception: raise _error("unable to connect to ProxySQL admin interface") from None @@ -710,7 +717,10 @@ def _fetch_users(self, table: str) -> list[ProxySQLUser]: connection = self._connection() try: cursor = connection.cursor() - cursor.execute(f"SELECT {_USER_COLUMNS} FROM {table}") + # ProxySQL splits a combined backend/frontend user into two rows in + # runtime. Reconcile the backend half, which is also the row keyed + # by apply_actions(), so one username has one deterministic record. + cursor.execute(f"SELECT {_USER_COLUMNS} FROM {table} WHERE backend=1") return [ProxySQLUser(*row) for row in cursor.fetchall()] except Exception: raise _error("unable to fetch ProxySQL users") from None diff --git a/tools/pgsql_user_sync/requirements.txt b/tools/pgsql_user_sync/requirements.txt index 4ef92dd587..c221f53de9 100644 --- a/tools/pgsql_user_sync/requirements.txt +++ b/tools/pgsql_user_sync/requirements.txt @@ -1,2 +1 @@ psycopg[binary]>=3.2.13,<4 -PyMySQL>=1.1.1,<2 diff --git a/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py b/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py index 42f646bf44..befccc3216 100644 --- a/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py +++ b/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py @@ -518,8 +518,11 @@ def test_admin_fetches_exact_projection_from_both_tables(self): runtime_sql = self.cursor.executions[-1][0] projection = ("username,password,active,use_ssl,default_hostgroup,transaction_persistent," "fast_forward,backend,frontend,max_connections,attributes,comment") - self.assertEqual("SELECT %s FROM pgsql_users" % projection, main_sql) - self.assertEqual("SELECT %s FROM runtime_pgsql_users" % projection, runtime_sql) + self.assertEqual("SELECT %s FROM pgsql_users WHERE backend=1" % projection, main_sql) + self.assertEqual( + "SELECT %s FROM runtime_pgsql_users WHERE backend=1" % projection, + runtime_sql, + ) self.assertTrue(self.connect_kwargs["autocommit"]) def test_admin_update_uses_bound_parameters(self): @@ -748,7 +751,7 @@ def test_example_configuration_is_loadable_with_protected_permissions(self): def test_requirements_pin_supported_driver_majors(self): requirements = (ASSET_DIR / "requirements.txt").read_text() self.assertIn("psycopg[binary]>=3.2.13,<4", requirements) - self.assertIn("PyMySQL>=1.1.1,<2", requirements) + self.assertNotIn("PyMySQL", requirements) def test_readme_documents_scheduler_and_cluster_safety(self): readme = (ASSET_DIR / "README.md").read_text() From fca6b44c397245327cb18bd124fadd4ca75c1b4d Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Thu, 13 Aug 2026 05:04:00 +0000 Subject: [PATCH 15/18] fix: correct PostgreSQL user sync reconciliation --- .../2026-08-12-pgsql-user-sync-design.md | 16 ++-- test/tap/tests/pgsql-user-sync-t.py | 19 ++++- tools/pgsql_user_sync/README.md | 9 ++- .../create_source_function.sql | 8 +- .../proxysql_pgsql_user_sync.py | 34 ++++++-- .../tests/test_pgsql_user_sync.py | 81 +++++++++++++++++-- 6 files changed, 144 insertions(+), 23 deletions(-) diff --git a/docs/superpowers/specs/2026-08-12-pgsql-user-sync-design.md b/docs/superpowers/specs/2026-08-12-pgsql-user-sync-design.md index 3217d99363..f4728f629e 100644 --- a/docs/superpowers/specs/2026-08-12-pgsql-user-sync-design.md +++ b/docs/superpowers/specs/2026-08-12-pgsql-user-sync-design.md @@ -75,15 +75,19 @@ The example SQL creates: The function has a fixed safe `search_path` and returns exactly two columns: `username` and `password`. It selects only roles that: -- are members of the allow-list role; +- are direct members of the allow-list role; - have `rolcanlogin = true`; +- are not superusers; - have a non-null `rolpassword`; and - have no expiration or a `rolvaliduntil` later than the current time. -This allow-list prevents accidental import of `postgres`, replication users, -and unrelated service accounts. A role that stops satisfying any selection -condition is absent from the next snapshot and is processed according to -`missing_role_action`. +The function checks direct membership through `pg_auth_members`, rather than +`pg_has_role`, so implicit superuser membership cannot bypass the allow-list. +The explicit superuser exclusion prevents accidental import of `postgres`. +Replication users and unrelated service accounts remain excluded unless an +operator explicitly grants direct allow-list membership. A role that stops +satisfying any selection condition is absent from the next snapshot and is +processed according to `missing_role_action`. ## Configuration and command line @@ -354,7 +358,7 @@ function, ProxySQL Admin interface, and verifier-capable authentication path: The integration test is registered only in a PostgreSQL-backed TAP group and is gated on verifier-capable ProxySQL behavior. Until the equivalent of PR -#5865 is present in the target branch, the test must report a clear dependency +PR #5865 is present in the target branch, the test must report a clear dependency skip rather than masking a synchronizer failure. ## Acceptance criteria diff --git a/test/tap/tests/pgsql-user-sync-t.py b/test/tap/tests/pgsql-user-sync-t.py index be1e56bd63..1c50b0f20f 100644 --- a/test/tap/tests/pgsql-user-sync-t.py +++ b/test/tap/tests/pgsql-user-sync-t.py @@ -73,7 +73,7 @@ def admin_row(username, runtime=False): with connection.cursor() as cursor: cursor.execute( f"SELECT username,password,active,default_hostgroup,attributes " - f"FROM {table} WHERE username=%s", + f"FROM {table} WHERE username=%s AND backend=1", (username,), ) return cursor.fetchone() @@ -206,6 +206,13 @@ def main(): ) if result.returncode != 0: print(f"# synchronizer diagnostic: {result.stderr.strip()}") + repeated = subprocess.run( + [sys.executable, str(SCRIPT), "--config", str(config)], + text=True, + capture_output=True, + ) + if repeated.returncode != 0: + print(f"# repeated synchronizer diagnostic: {repeated.stderr.strip()}") tap.check(result.returncode == 0, "real synchronizer run succeeds") verifier = source_verifier(source, username) @@ -254,6 +261,16 @@ def main(): and int(runtime_row[3]) == 0, "synchronizer loads the user into runtime_pgsql_users", ) + repeated_is_noop = ( + repeated.returncode == 0 + and "loaded=false saved=false" in repeated.stdout + ) + if not repeated_is_noop: + print(f"# repeated synchronizer output: {repeated.stdout.strip()}") + tap.check( + repeated_is_noop, + "repeated synchronization detects no backend-runtime drift", + ) except Exception as error: print(f"# real integration failure class: {type(error).__name__}") tap.check(False, "real PostgreSQL to ProxySQL synchronization completes") diff --git a/tools/pgsql_user_sync/README.md b/tools/pgsql_user_sync/README.md index 8add735eee..7fce585007 100644 --- a/tools/pgsql_user_sync/README.md +++ b/tools/pgsql_user_sync/README.md @@ -56,10 +56,11 @@ GRANT proxysql_auth_managed TO app_login; REVOKE proxysql_auth_managed FROM app_login; ``` -The function additionally requires a login-capable role, a non-null verifier, -and no expired `rolvaliduntil`. Therefore superusers, replication roles, and -service accounts are not imported unless an operator explicitly places them -on the allow-list. +The function uses a direct allow-list membership check and additionally +requires a login-capable, non-superuser role with a non-null verifier and no +expired `rolvaliduntil`. Replication roles and service accounts are not +imported unless an operator explicitly places them on the allow-list; +superusers are always excluded. ## Run manually diff --git a/tools/pgsql_user_sync/create_source_function.sql b/tools/pgsql_user_sync/create_source_function.sql index a184bbcbe7..8b249f140f 100644 --- a/tools/pgsql_user_sync/create_source_function.sql +++ b/tools/pgsql_user_sync/create_source_function.sql @@ -37,7 +37,13 @@ AS $function$ WHERE r.rolcanlogin AND r.rolpassword IS NOT NULL AND (r.rolvaliduntil IS NULL OR r.rolvaliduntil > pg_catalog.now()) - AND pg_catalog.pg_has_role(r.oid, 'proxysql_auth_managed', 'member') + AND NOT r.rolsuper + AND EXISTS ( + SELECT 1 + FROM pg_catalog.pg_auth_members AS membership + WHERE membership.member = r.oid + AND membership.roleid = 'proxysql_auth_managed'::regrole + ) ORDER BY r.rolname; $function$; diff --git a/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py b/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py index 325dcca26c..91a5e20882 100644 --- a/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py +++ b/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py @@ -476,6 +476,16 @@ def _new_user(role: SourceRole, settings: SyncSettings) -> ProxySQLUser: ) +def _runtime_matches(main_row: ProxySQLUser, runtime_row: ProxySQLUser | None) -> bool: + """Compare the backend half of main and runtime user rows.""" + + if runtime_row is None: + return False + # ProxySQL materializes a combined main user as separate backend and + # frontend runtime rows. The backend half always has frontend=0. + return replace(main_row, frontend=0) == replace(runtime_row, frontend=0) + + def build_plan( source: Mapping[str, SourceRole], main: Iterable[ProxySQLUser], @@ -502,7 +512,7 @@ def build_plan( runtime_row = runtime_by_name.get(username) managed = _managed(main_row, settings) if main_row.active: - if runtime_row is None or runtime_row != main_row: + if not _runtime_matches(main_row, runtime_row): if not managed: raise _error("unmanaged main/runtime drift") managed_active_drift = True @@ -671,6 +681,7 @@ def fetch_snapshot(self) -> list[tuple[str, str]]: "username", "password", "active", "use_ssl", "default_hostgroup", "transaction_persistent", "fast_forward", "backend", "frontend", "max_connections", "attributes", "comment", ) +_USER_INTEGER_FIELD_INDEXES = (2, 3, 4, 5, 6, 7, 8, 9) class ProxySQLAdmin: @@ -721,7 +732,16 @@ def _fetch_users(self, table: str) -> list[ProxySQLUser]: # runtime. Reconcile the backend half, which is also the row keyed # by apply_actions(), so one username has one deterministic record. cursor.execute(f"SELECT {_USER_COLUMNS} FROM {table} WHERE backend=1") - return [ProxySQLUser(*row) for row in cursor.fetchall()] + users = [] + for row in cursor.fetchall(): + values = list(row) + # The PostgreSQL Admin interface sends these SQLite integer + # columns as text. Normalizing them prevents false updates on + # every scheduler invocation. + for index in _USER_INTEGER_FIELD_INDEXES: + values[index] = int(values[index]) + users.append(ProxySQLUser(*values)) + return users except Exception: raise _error("unable to fetch ProxySQL users") from None finally: @@ -813,6 +833,7 @@ def run_sync( loaded = False saved = False + partial = False if not dry_run: if plan.actions: try: @@ -829,10 +850,11 @@ def run_sync( try: admin.save_to_disk() except Exception: - raise _sync_failure("unable to save ProxySQL users to disk") from None - saved = True + partial = True + else: + saved = True return RunSummary( - outcome="dry-run" if dry_run else "success", + outcome="dry-run" if dry_run else "partial" if partial else "success", counts=MappingProxyType(dict(plan.counts)), loaded=loaded, saved=saved, @@ -892,7 +914,7 @@ def main(argv: Sequence[str] | None = None) -> int: verbose=args.verbose, ) print(_summary_line(summary)) - return 0 + return 1 if summary.outcome == "partial" else 0 except SyncError as error: print(f"error: {error}", file=sys.stderr) return 1 diff --git a/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py b/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py index befccc3216..eab39c797d 100644 --- a/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py +++ b/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py @@ -422,6 +422,22 @@ def test_inactive_main_row_is_expected_absent_from_runtime(self): self.assertEqual((), plan.actions) self.assertFalse(plan.requires_load) + def test_runtime_backend_half_with_frontend_disabled_is_not_drift(self): + main = self.user("alice", profile="p") + runtime = replace(main, frontend=0) + plan = self.mod.build_plan( + {"alice": self.role("alice")}, [main], [runtime], self.settings() + ) + self.assertEqual((), plan.actions) + self.assertFalse(plan.requires_load) + + def test_unmanaged_runtime_backend_half_with_frontend_disabled_is_not_drift(self): + main = self.user("local") + runtime = replace(main, frontend=0) + plan = self.mod.build_plan({}, [main], [runtime], self.settings()) + self.assertEqual((), plan.actions) + self.assertFalse(plan.requires_load) + class FakeCursor: def __init__(self, rows=()): @@ -525,6 +541,22 @@ def test_admin_fetches_exact_projection_from_both_tables(self): ) self.assertTrue(self.connect_kwargs["autocommit"]) + def test_admin_normalizes_integer_fields_returned_as_postgresql_text(self): + self.connection.cursor_object.rows = [ + ("alice", self.verifier_a, "1", "0", "3", "1", "0", "1", "1", "100", "", "") + ] + user = self.mod.ProxySQLAdmin(self.config, connect=self.fake_connect).fetch_main_users()[0] + self.assertEqual((1, 0, 3, 1, 0, 1, 1, 100), ( + user.active, + user.use_ssl, + user.default_hostgroup, + user.transaction_persistent, + user.fast_forward, + user.backend, + user.frontend, + user.max_connections, + )) + def test_admin_update_uses_bound_parameters(self): adapter = self.mod.ProxySQLAdmin(self.config, connect=self.fake_connect) adapter.apply_actions((self.update_action(),)) @@ -684,12 +716,24 @@ def test_loads_runtime_drift_without_actions(self): self.assertEqual(["main", "runtime", "load", "save"], admin.calls) self.assertEqual(0, summary.counts["updated"]) - def test_save_failure_follows_successful_load(self): + def test_runtime_backend_half_skips_load_on_a_repeated_run(self): + main = self.owned_user() + runtime = replace(main, frontend=0) + admin = FakeAdmin([main], [runtime]) + summary = self.mod.run_sync(self.config, FakeSource(self.rows), admin, + dry_run=False, verbose=False) + self.assertEqual(["main", "runtime"], admin.calls) + self.assertFalse(summary.loaded) + self.assertFalse(summary.saved) + + def test_save_failure_reports_partial_after_successful_load(self): admin = FakeAdmin(fail_save=True) - with self.assertRaises(self.mod.SyncError): - self.mod.run_sync(self.config, FakeSource(self.rows), admin, - dry_run=False, verbose=False) + summary = self.mod.run_sync(self.config, FakeSource(self.rows), admin, + dry_run=False, verbose=False) self.assertEqual(["main", "runtime", "apply", "load", "save"], admin.calls) + self.assertEqual("partial", summary.outcome) + self.assertTrue(summary.loaded) + self.assertFalse(summary.saved) def test_exclusive_lock_reports_contention(self): with tempfile.TemporaryDirectory() as directory: @@ -708,6 +752,31 @@ def test_summary_has_non_negative_duration(self): with self.assertRaises(AttributeError): summary.outcome = "changed" + def test_partial_summary_returns_nonzero_after_printing(self): + with tempfile.TemporaryDirectory() as directory: + config = replace( + self.config, + sync=replace(self.config.sync, lock_file=Path(directory) / "sync.lock"), + ) + partial = self.mod.RunSummary( + outcome="partial", + counts={"created": 1}, + loaded=True, + saved=False, + duration_seconds=0.0, + ) + with ( + patch.object(self.mod, "load_config", return_value=config), + patch.object(self.mod, "run_sync", return_value=partial), + patch.object(self.mod, "PostgreSQLSource"), + patch.object(self.mod, "ProxySQLAdmin"), + patch("builtins.print") as report, + ): + self.assertEqual(1, self.mod.main(["--config", "ignored.ini"])) + report.assert_called_once_with( + "sync partial: created=1 loaded=true saved=false duration=0.000s" + ) + class AssetTests(unittest.TestCase): """Keep the operator-facing deployment sample complete and safe.""" @@ -723,7 +792,9 @@ def test_source_function_has_security_boundaries(self): ): self.assertIn(required, sql) self.assertIn("proxysql_auth_managed", sql) - self.assertIn("pg_has_role", sql) + self.assertIn("pg_auth_members", sql) + self.assertIn("NOT r.rolsuper", sql) + self.assertNotIn("pg_has_role", sql) self.assertIn("proxysql_auth_reader", sql) self.assertIn("GRANT proxysql_auth_managed TO app_login;", sql) self.assertIn("REVOKE proxysql_auth_managed FROM app_login;", sql) From 0ae9447588816bcf93a0954efc4846521afd5964 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Thu, 13 Aug 2026 05:30:36 +0000 Subject: [PATCH 16/18] fix: harden PostgreSQL user sync files --- .../2026-08-12-pgsql-user-sync-design.md | 12 +- tools/pgsql_user_sync/README.md | 3 + .../proxysql_pgsql_user_sync.ini.example | 2 +- .../proxysql_pgsql_user_sync.py | 430 +++++++++++------- .../tests/test_pgsql_user_sync.py | 70 ++- 5 files changed, 321 insertions(+), 196 deletions(-) diff --git a/docs/superpowers/specs/2026-08-12-pgsql-user-sync-design.md b/docs/superpowers/specs/2026-08-12-pgsql-user-sync-design.md index f4728f629e..5ad583910b 100644 --- a/docs/superpowers/specs/2026-08-12-pgsql-user-sync-design.md +++ b/docs/superpowers/specs/2026-08-12-pgsql-user-sync-design.md @@ -123,7 +123,7 @@ missing_role_action = disable adopt_existing_users = false allow_empty_snapshot = false save_to_disk = true -lock_file = /run/lock/proxysql-pgsql-user-sync.lock +lock_file = /var/lib/proxysql/proxysql-pgsql-user-sync.lock ``` `profile` is a stable, non-empty identifier used to mark ownership. Profile @@ -144,10 +144,12 @@ The supported non-secret command-line overrides are: Database passwords are deliberately config-only. They never appear in the scheduler table, process arguments, or normal logs. The configuration must be a -regular file readable by the executing account. Group write/execute access and -all access by other users are rejected; `0600` and root-owned `0640` with a -dedicated ProxySQL group are documented examples. Symlink handling follows the -normal operating-system file open rules, but the resolved file's metadata is +regular non-symlink file readable by the executing account. Group write/execute +access and all access by other users are rejected; `0600` and root-owned `0640` +with a dedicated ProxySQL group are documented examples. The file is opened and +validated through one descriptor so the checked object cannot be swapped before +parsing. The lock file defaults to ProxySQL's private data directory and also +rejects symlinks. The opened file's metadata is validated before credentials are used. ## Ownership and row policy diff --git a/tools/pgsql_user_sync/README.md b/tools/pgsql_user_sync/README.md index 7fce585007..40737befc0 100644 --- a/tools/pgsql_user_sync/README.md +++ b/tools/pgsql_user_sync/README.md @@ -76,6 +76,9 @@ Remove `--dry-run` to apply the plan. Useful non-secret overrides are `--save-to-disk`/`--no-save-to-disk`, and `--verbose`. `--dry-run` fetches and validates both snapshots but performs no writes, runtime load, or disk save. An exclusive lock makes overlapping scheduler invocations a successful skip. +The default lock is in ProxySQL's private data directory; if you override it, +use a service-owned directory. Symlinked configuration and lock files are +rejected. ## ProxySQL Scheduler diff --git a/tools/pgsql_user_sync/proxysql_pgsql_user_sync.ini.example b/tools/pgsql_user_sync/proxysql_pgsql_user_sync.ini.example index 1151185210..627eb6655e 100644 --- a/tools/pgsql_user_sync/proxysql_pgsql_user_sync.ini.example +++ b/tools/pgsql_user_sync/proxysql_pgsql_user_sync.ini.example @@ -24,4 +24,4 @@ missing_role_action = disable adopt_existing_users = false allow_empty_snapshot = false save_to_disk = true -lock_file = /run/lock/proxysql-pgsql-user-sync.lock +lock_file = /var/lib/proxysql/proxysql-pgsql-user-sync.lock diff --git a/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py b/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py index 91a5e20882..6d34886397 100644 --- a/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py +++ b/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py @@ -65,7 +65,7 @@ class SyncSettings: adopt_existing_users: bool = False allow_empty_snapshot: bool = False save_to_disk: bool = True - lock_file: Path = Path("/run/lock/proxysql-pgsql-user-sync.lock") + lock_file: Path = Path("/var/lib/proxysql/proxysql-pgsql-user-sync.lock") @dataclass(frozen=True) @@ -118,7 +118,7 @@ class SyncPlan: PROFILE_RE = re.compile(r"[A-Za-z0-9][A-Za-z0-9_.-]{0,63}\Z") -IDENTIFIER_RE = re.compile(r"[A-Za-z_][A-Za-z0-9_]*\Z") +IDENTIFIER_RE = re.compile(r"[A-Za-z_]\w*\Z", re.ASCII) MD5_RE = re.compile(r"md5[0-9a-f]{32}\Z") SCRAM_RE = re.compile( r"SCRAM-SHA-256\$(?P[0-9]+):(?P[^$]+)\$" @@ -188,15 +188,9 @@ def _validate_timeout(section: configparser.SectionProxy, name: str = "connect_t return timeout -def _validate_file(path: Path) -> Path: - try: - info = path.stat() - except OSError: - raise _error("configuration file cannot be read") from None +def _validate_file(info: os.stat_result) -> None: if not stat.S_ISREG(info.st_mode): raise _error("configuration file must be a regular file") - if not os.access(path, os.R_OK): - raise _error("configuration file is not readable") # Group read is a supported deployment mode only for a root-owned file # (typically 0640 with a dedicated service group). Non-root-owned files # must be owner-only. Group write/execute and every other-user permission @@ -209,51 +203,73 @@ def _validate_file(path: Path) -> Path: or (group_permissions & 0o040 and info.st_uid != 0) ): raise _error("configuration file has unsafe permissions") - return path -def load_config(path: Path, overrides: CLIOverrides) -> AppConfig: - """Read and validate a protected INI configuration file.""" +def _open_config_file(path: Path): + flags = os.O_RDONLY | os.O_CLOEXEC | os.O_NOFOLLOW + fd: int | None = None + try: + fd = os.open(path, flags) + _validate_file(os.fstat(fd)) + stream = os.fdopen(fd, "r", encoding="utf-8") + except SyncError: + if fd is not None: + os.close(fd) + raise + except OSError: + if fd is not None: + os.close(fd) + raise _error("configuration file cannot be read") from None + return stream - if not isinstance(path, Path): - path = Path(path) - path = _validate_file(path) + +def _read_config(path: Path) -> configparser.ConfigParser: parser = configparser.ConfigParser(interpolation=None) try: - with path.open("r", encoding="utf-8") as stream: + with _open_config_file(path) as stream: parser.read_file(stream) except (OSError, UnicodeError, configparser.Error): raise _error("configuration file cannot be parsed") from None + return parser - source_section = _section(parser, "source") - proxy_section = _section(parser, "proxysql") - sync_section = _section(parser, "sync") - source_host, source_port = _validate_endpoint(source_section, "source") - proxy_host, proxy_port = _validate_endpoint(proxy_section, "proxysql") - source_function = source_section.get("function", "proxysql_auth.export_login_roles").strip() +def _source_function_parts(section: configparser.SectionProxy) -> tuple[str, str]: + source_function = section.get("function", "proxysql_auth.export_login_roles").strip() function_parts = source_function.split(".", 1) if len(function_parts) != 2 or not all(IDENTIFIER_RE.fullmatch(part) for part in function_parts): raise _error("[source].function must be a schema-qualified identifier pair") + return function_parts[0], function_parts[1] + + +def _source_config(section: configparser.SectionProxy) -> SourceConfig: + host, port = _validate_endpoint(section, "source") + schema, function = _source_function_parts(section) + return SourceConfig( + host=host, + port=port, + database=_required(section, "database"), + username=_required(section, "username"), + password=_required(section, "password"), + connect_timeout=_validate_timeout(section), + function_schema=schema, + function_name=function, + ) - profile = _required(sync_section, "profile") - if PROFILE_RE.fullmatch(profile) is None: - raise _error("[sync].profile has invalid syntax") - default_hostgroup = _int_value(sync_section, "default_hostgroup", 0) - if default_hostgroup < 0: - raise _error("[sync].default_hostgroup must be non-negative") - missing_role_action = sync_section.get("missing_role_action", "disable").strip().lower() - if missing_role_action not in {"disable", "keep"}: - raise _error("[sync].missing_role_action must be disable or keep") - adopt_existing_users = _boolean_value(sync_section, "adopt_existing_users", False) - allow_empty_snapshot = _boolean_value(sync_section, "allow_empty_snapshot", False) - save_to_disk = _boolean_value(sync_section, "save_to_disk", True) - lock_file_value = sync_section.get("lock_file", str(SyncSettings.lock_file)).strip() - if not lock_file_value: - raise _error("[sync].lock_file must not be empty") - lock_file = Path(lock_file_value) +def _proxysql_config(section: configparser.SectionProxy) -> ProxySQLConfig: + host, port = _validate_endpoint(section, "proxysql") + return ProxySQLConfig( + host=host, + port=port, + username=_required(section, "username"), + password=_required(section, "password"), + connect_timeout=_validate_timeout(section), + ) + +def _sync_overrides( + default_hostgroup: int, missing_role_action: str, save_to_disk: bool, overrides: CLIOverrides +) -> tuple[int, str, bool]: if overrides.default_hostgroup is not None: if not isinstance(overrides.default_hostgroup, int) or overrides.default_hostgroup < 0: raise _error("default_hostgroup override must be non-negative") @@ -266,34 +282,53 @@ def load_config(path: Path, overrides: CLIOverrides) -> AppConfig: if not isinstance(overrides.save_to_disk, bool): raise _error("save_to_disk override must be boolean") save_to_disk = overrides.save_to_disk + return default_hostgroup, missing_role_action, save_to_disk + + +def _sync_settings(section: configparser.SectionProxy, overrides: CLIOverrides) -> SyncSettings: + profile = _required(section, "profile") + if PROFILE_RE.fullmatch(profile) is None: + raise _error("[sync].profile has invalid syntax") + default_hostgroup = _int_value(section, "default_hostgroup", 0) + if default_hostgroup < 0: + raise _error("[sync].default_hostgroup must be non-negative") + missing_role_action = section.get("missing_role_action", "disable").strip().lower() + if missing_role_action not in {"disable", "keep"}: + raise _error("[sync].missing_role_action must be disable or keep") + lock_file_value = section.get("lock_file", str(SyncSettings.lock_file)).strip() + if not lock_file_value: + raise _error("[sync].lock_file must not be empty") + default_hostgroup, missing_role_action, save_to_disk = _sync_overrides( + default_hostgroup, + missing_role_action, + _boolean_value(section, "save_to_disk", True), + overrides, + ) + return SyncSettings( + profile=profile, + default_hostgroup=default_hostgroup, + missing_role_action=missing_role_action, + adopt_existing_users=_boolean_value(section, "adopt_existing_users", False), + allow_empty_snapshot=_boolean_value(section, "allow_empty_snapshot", False), + save_to_disk=save_to_disk, + lock_file=Path(lock_file_value), + ) + + +def load_config(path: Path, overrides: CLIOverrides) -> AppConfig: + """Read and validate a protected INI configuration file.""" + + if not isinstance(path, Path): + path = Path(path) + parser = _read_config(path) + source_section = _section(parser, "source") + proxy_section = _section(parser, "proxysql") + sync_section = _section(parser, "sync") return AppConfig( - source=SourceConfig( - host=source_host, - port=source_port, - database=_required(source_section, "database"), - username=_required(source_section, "username"), - password=_required(source_section, "password"), - connect_timeout=_validate_timeout(source_section), - function_schema=function_parts[0], - function_name=function_parts[1], - ), - proxysql=ProxySQLConfig( - host=proxy_host, - port=proxy_port, - username=_required(proxy_section, "username"), - password=_required(proxy_section, "password"), - connect_timeout=_validate_timeout(proxy_section), - ), - sync=SyncSettings( - profile=profile, - default_hostgroup=default_hostgroup, - missing_role_action=missing_role_action, - adopt_existing_users=adopt_existing_users, - allow_empty_snapshot=allow_empty_snapshot, - save_to_disk=save_to_disk, - lock_file=lock_file, - ), + source=_source_config(source_section), + proxysql=_proxysql_config(proxy_section), + sync=_sync_settings(sync_section, overrides), ) @@ -314,7 +349,7 @@ def parse_args(argv: Sequence[str]) -> argparse.Namespace: def _decode_base64(value: str) -> bytes | None: try: decoded = base64.b64decode(value.encode("ascii"), validate=True) - except (UnicodeEncodeError, ValueError): + except ValueError: return None # Reject alternate/non-canonical encodings in addition to non-alphabet # characters. PostgreSQL emits standard padded base64 in SCRAM values. @@ -352,39 +387,51 @@ def validate_verifier(value: str) -> None: raise _error("invalid SCRAM key length") +def _snapshot_columns(row: object) -> tuple[str, str]: + if isinstance(row, (str, bytes, bytearray)) or not isinstance(row, Sequence): + raise _error("snapshot row must contain exactly two columns") + if len(row) != 2: + raise _error("snapshot row must contain exactly two columns") + username, password = row + if not isinstance(username, str) or not isinstance(password, str): + raise _error("snapshot username and password must be strings") + return username, password + + +def _validate_snapshot_username(username: str) -> None: + if not username: + raise _error("snapshot username must not be empty") + if "\x00" in username: + raise _error("snapshot username must not contain NUL") + try: + username_bytes = username.encode("utf-8") + except UnicodeEncodeError: + raise _error("snapshot username must be valid UTF-8") from None + if len(username_bytes) > 63: + raise _error("snapshot username exceeds 63 UTF-8 bytes") + + +def _snapshot_role(row: object) -> SourceRole: + username, password = _snapshot_columns(row) + _validate_snapshot_username(username) + validate_verifier(password) + return SourceRole(username=username, password=password) + + def validate_snapshot(rows: Iterable[Sequence[object]], allow_empty: bool) -> dict[str, SourceRole]: """Validate a complete two-column role snapshot and return it sorted.""" result: dict[str, SourceRole] = {} - count = 0 try: iterator = iter(rows) except TypeError: raise _error("snapshot must be iterable") from None for row in iterator: - count += 1 - if isinstance(row, (str, bytes, bytearray)) or not isinstance(row, Sequence): - raise _error("snapshot row must contain exactly two columns") - if len(row) != 2: - raise _error("snapshot row must contain exactly two columns") - username, password = row - if not isinstance(username, str) or not isinstance(password, str): - raise _error("snapshot username and password must be strings") - if not username: - raise _error("snapshot username must not be empty") - if "\x00" in username: - raise _error("snapshot username must not contain NUL") - try: - username_bytes = username.encode("utf-8") - except UnicodeEncodeError: - raise _error("snapshot username must be valid UTF-8") from None - if len(username_bytes) > 63: - raise _error("snapshot username exceeds 63 UTF-8 bytes") - if username in result: + role = _snapshot_role(row) + if role.username in result: raise _error("snapshot contains duplicate username") - validate_verifier(password) - result[username] = SourceRole(username=username, password=password) - if count == 0 and not allow_empty: + result[role.username] = role + if not result and not allow_empty: raise _error("snapshot is empty") return dict(sorted(result.items())) @@ -399,7 +446,7 @@ def _ownership_document(attributes: str) -> dict[str, object]: return {} try: document = json.loads(attributes) - except (TypeError, ValueError, json.JSONDecodeError): + except (TypeError, ValueError): raise _error("ProxySQL user attributes must be valid JSON") from None if not isinstance(document, dict): raise _error("ProxySQL user attributes must be a JSON object") @@ -486,50 +533,49 @@ def _runtime_matches(main_row: ProxySQLUser, runtime_row: ProxySQLUser | None) - return replace(main_row, frontend=0) == replace(runtime_row, frontend=0) -def build_plan( - source: Mapping[str, SourceRole], - main: Iterable[ProxySQLUser], - runtime: Iterable[ProxySQLUser], - settings: SyncSettings, -) -> SyncPlan: - """Build a deterministic, side-effect-free reconciliation plan.""" +_UNMANAGED_RUNTIME_DRIFT = "unmanaged main/runtime drift" + +def _source_names(source: Mapping[str, SourceRole]) -> set[str]: if not isinstance(source, Mapping): raise _error("source snapshot must be a username mapping") - source_names = set(source) + names = set(source) for username, role in source.items(): if not isinstance(username, str) or not isinstance(role, SourceRole): raise _error("source snapshot contains an invalid role") if role.username != username: raise _error("source snapshot username does not match role") + return names - main_by_name = _user_index(main, "main") - runtime_by_name = _user_index(runtime, "runtime") - managed_active_drift = False - # LOAD is global. Never overwrite an unrelated user's active runtime state. +def _main_runtime_drift(main_row: ProxySQLUser, runtime_row: ProxySQLUser | None) -> bool: + return not _runtime_matches(main_row, runtime_row) if main_row.active else runtime_row is not None + + +def _managed_runtime_drift( + main_by_name: Mapping[str, ProxySQLUser], + runtime_by_name: Mapping[str, ProxySQLUser], + settings: SyncSettings, +) -> bool: + managed_drift = False for username, main_row in main_by_name.items(): - runtime_row = runtime_by_name.get(username) - managed = _managed(main_row, settings) - if main_row.active: - if not _runtime_matches(main_row, runtime_row): - if not managed: - raise _error("unmanaged main/runtime drift") - managed_active_drift = True - elif runtime_row is not None: - if not managed: - raise _error("unmanaged main/runtime drift") - managed_active_drift = True + if not _main_runtime_drift(main_row, runtime_by_name.get(username)): + continue + if not _managed(main_row, settings): + raise _error(_UNMANAGED_RUNTIME_DRIFT) + managed_drift = True for username, runtime_row in runtime_by_name.items(): if username in main_by_name: continue if _managed(runtime_row, settings): - managed_active_drift = True + managed_drift = True elif runtime_row.active: - raise _error("unmanaged main/runtime drift") + raise _error(_UNMANAGED_RUNTIME_DRIFT) + return managed_drift - actions: list[SyncAction] = [] - counts = { + +def _plan_counts(source: Mapping[str, SourceRole]) -> dict[str, int]: + return { "discovered": len(source), "created": 0, "updated": 0, @@ -539,16 +585,21 @@ def build_plan( "conflicted": 0, } - # Source roles are sorted for stable action order and reproducible output. - for username in sorted(source_names): + +def _source_actions( + source: Mapping[str, SourceRole], + main_by_name: Mapping[str, ProxySQLUser], + settings: SyncSettings, + counts: dict[str, int], +) -> list[SyncAction]: + actions: list[SyncAction] = [] + for username in sorted(source): role = source[username] existing = main_by_name.get(username) if existing is None: - after = _new_user(role, settings) - actions.append(SyncAction(ActionKind.CREATE, None, after)) + actions.append(SyncAction(ActionKind.CREATE, None, _new_user(role, settings))) counts["created"] += 1 continue - owner = decode_ownership(existing.attributes) if owner is not None and owner != settings.profile: counts["conflicted"] += 1 @@ -556,7 +607,6 @@ def build_plan( if owner is None and not settings.adopt_existing_users: counts["conflicted"] += 1 raise _error("unmanaged user conflicts with source role") - after = replace( existing, password=role.password, @@ -565,27 +615,52 @@ def build_plan( ) if after == existing: counts["unchanged"] += 1 - else: - actions.append(SyncAction(ActionKind.UPDATE, existing, after)) - counts["updated"] += 1 - if not existing.active: - counts["reactivated"] += 1 - - if settings.missing_role_action == "disable": - for username in sorted(set(main_by_name) - source_names): - existing = main_by_name[username] - if not _managed(existing, settings) or not existing.active: - continue - after = replace(existing, active=0) - actions.append(SyncAction(ActionKind.DISABLE, existing, after)) - counts["disabled"] += 1 - elif settings.missing_role_action != "keep": + continue + actions.append(SyncAction(ActionKind.UPDATE, existing, after)) + counts["updated"] += 1 + if not existing.active: + counts["reactivated"] += 1 + return actions + + +def _missing_role_actions( + source_names: set[str], + main_by_name: Mapping[str, ProxySQLUser], + settings: SyncSettings, + counts: dict[str, int], +) -> list[SyncAction]: + if settings.missing_role_action == "keep": + return [] + if settings.missing_role_action != "disable": raise _error("missing_role_action must be disable or keep") + actions = [] + for username in sorted(set(main_by_name) - source_names): + existing = main_by_name[username] + if _managed(existing, settings) and existing.active: + actions.append(SyncAction(ActionKind.DISABLE, existing, replace(existing, active=0))) + counts["disabled"] += 1 + return actions + + +def build_plan( + source: Mapping[str, SourceRole], + main: Iterable[ProxySQLUser], + runtime: Iterable[ProxySQLUser], + settings: SyncSettings, +) -> SyncPlan: + """Build a deterministic, side-effect-free reconciliation plan.""" + source_names = _source_names(source) + main_by_name = _user_index(main, "main") + runtime_by_name = _user_index(runtime, "runtime") + managed_runtime_drift = _managed_runtime_drift(main_by_name, runtime_by_name, settings) + counts = _plan_counts(source) + actions = _source_actions(source, main_by_name, settings, counts) + actions.extend(_missing_role_actions(source_names, main_by_name, settings, counts)) actions.sort(key=lambda action: action.after.username) return SyncPlan( actions=tuple(actions), - requires_load=bool(actions) or managed_active_drift, + requires_load=bool(actions) or managed_runtime_drift, counts=MappingProxyType(counts), ) @@ -805,6 +880,50 @@ def _sync_failure(message: str) -> SyncError: return _error(message) +def _source_snapshot(source: SourceAdapter, allow_empty: bool) -> dict[str, SourceRole]: + try: + raw_snapshot = source.fetch_snapshot() + except Exception: + raise _sync_failure("unable to fetch source role snapshot") from None + return validate_snapshot(raw_snapshot, allow_empty) + + +def _admin_snapshots(admin: AdminAdapter) -> tuple[list[ProxySQLUser], list[ProxySQLUser]]: + try: + return admin.fetch_main_users(), admin.fetch_runtime_users() + except Exception: + raise _sync_failure("unable to fetch ProxySQL user snapshots") from None + + +def _apply_plan(plan: SyncPlan, admin: AdminAdapter, settings: SyncSettings) -> tuple[bool, bool, bool]: + if plan.actions: + try: + admin.apply_actions(plan.actions) + except Exception: + raise _sync_failure("unable to apply ProxySQL user changes") from None + if not plan.requires_load: + return False, False, False + try: + admin.load_runtime() + except Exception: + raise _sync_failure("unable to load ProxySQL users to runtime") from None + if not settings.save_to_disk: + return True, False, False + try: + admin.save_to_disk() + except Exception: + return True, False, True + return True, True, False + + +def _sync_outcome(dry_run: bool, partial: bool) -> str: + if dry_run: + return "dry-run" + if partial: + return "partial" + return "success" + + def run_sync( config: AppConfig, source: SourceAdapter, @@ -816,16 +935,8 @@ def run_sync( """Read, plan, and optionally reconcile one complete role snapshot.""" started = time.monotonic() - try: - raw_snapshot = source.fetch_snapshot() - except Exception: - raise _sync_failure("unable to fetch source role snapshot") from None - snapshot = validate_snapshot(raw_snapshot, config.sync.allow_empty_snapshot) - try: - main_users = admin.fetch_main_users() - runtime_users = admin.fetch_runtime_users() - except Exception: - raise _sync_failure("unable to fetch ProxySQL user snapshots") from None + snapshot = _source_snapshot(source, config.sync.allow_empty_snapshot) + main_users, runtime_users = _admin_snapshots(admin) plan = build_plan(snapshot, main_users, runtime_users, config.sync) if verbose: for action in plan.actions: @@ -835,26 +946,9 @@ def run_sync( saved = False partial = False if not dry_run: - if plan.actions: - try: - admin.apply_actions(plan.actions) - except Exception: - raise _sync_failure("unable to apply ProxySQL user changes") from None - if plan.requires_load: - try: - admin.load_runtime() - except Exception: - raise _sync_failure("unable to load ProxySQL users to runtime") from None - loaded = True - if config.sync.save_to_disk: - try: - admin.save_to_disk() - except Exception: - partial = True - else: - saved = True + loaded, saved, partial = _apply_plan(plan, admin, config.sync) return RunSummary( - outcome="dry-run" if dry_run else "partial" if partial else "success", + outcome=_sync_outcome(dry_run, partial), counts=MappingProxyType(dict(plan.counts)), loaded=loaded, saved=saved, @@ -867,7 +961,7 @@ def exclusive_lock(path: Path): """Yield whether the process acquired the non-blocking synchronizer lock.""" try: - fd = os.open(path, os.O_CREAT | os.O_RDWR, 0o600) + fd = os.open(path, os.O_CREAT | os.O_RDWR | os.O_CLOEXEC | os.O_NOFOLLOW, 0o600) except OSError: raise _error("unable to open synchronizer lock file") from None try: diff --git a/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py b/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py index eab39c797d..398b1bd0e9 100644 --- a/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py +++ b/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py @@ -55,6 +55,9 @@ def write_config(self, *, mode=0o600, text=None): os.chmod(handle.name, mode) return Path(handle.name) + def load_default_config(self, path): + return self.mod.load_config(path, self.mod.CLIOverrides()) + def test_defaults_and_cli_hostgroup_override(self): path = self.write_config() cfg = self.mod.load_config(path, self.mod.CLIOverrides(default_hostgroup=12)) @@ -105,30 +108,38 @@ def test_cli_overrides_take_precedence(self): def test_rejects_world_readable_config(self): path = self.write_config(mode=0o604) with self.assertRaisesRegex(self.mod.SyncError, "permissions"): - self.mod.load_config(path, self.mod.CLIOverrides()) + self.load_default_config(path) + + def test_rejects_symlinked_config_before_reading_target(self): + target = self.write_config() + alias = target.with_name(f"{target.name}.link") + alias.symlink_to(target) + self.addCleanup(alias.unlink) + with self.assertRaisesRegex(self.mod.SyncError, "cannot be read"): + self.load_default_config(alias) def test_group_read_requires_root_owner_and_owner_only_is_allowed(self): path = self.write_config(mode=0o640) root_metadata = SimpleNamespace(st_mode=stat.S_IFREG | 0o640, st_uid=0) - with patch.object(self.mod.Path, "stat", return_value=root_metadata): + with patch.object(self.mod.os, "fstat", return_value=root_metadata): self.mod.load_config(path, self.mod.CLIOverrides()) non_root_metadata = SimpleNamespace(st_mode=stat.S_IFREG | 0o640, st_uid=1000) - with patch.object(self.mod.Path, "stat", return_value=non_root_metadata): + with patch.object(self.mod.os, "fstat", return_value=non_root_metadata): with self.assertRaisesRegex(self.mod.SyncError, "permissions"): - self.mod.load_config(path, self.mod.CLIOverrides()) + self.load_default_config(path) path = self.write_config(mode=0o600) owner_only_metadata = SimpleNamespace(st_mode=stat.S_IFREG | 0o600, st_uid=1000) - with patch.object(self.mod.Path, "stat", return_value=owner_only_metadata): + with patch.object(self.mod.os, "fstat", return_value=owner_only_metadata): self.mod.load_config(path, self.mod.CLIOverrides()) def test_rejects_group_write_even_when_root_owned(self): path = self.write_config(mode=0o660) root_metadata = SimpleNamespace(st_mode=stat.S_IFREG | 0o660, st_uid=0) - with patch.object(self.mod.Path, "stat", return_value=root_metadata): + with patch.object(self.mod.os, "fstat", return_value=root_metadata): with self.assertRaisesRegex(self.mod.SyncError, "permissions"): - self.mod.load_config(path, self.mod.CLIOverrides()) + self.load_default_config(path) def test_rejects_invalid_profile_and_function(self): for profile in ("", "-bad", "a" * 65): @@ -148,7 +159,7 @@ def test_rejects_invalid_profile_and_function(self): profile = %s """ % profile) with self.assertRaisesRegex(self.mod.SyncError, "profile"): - self.mod.load_config(path, self.mod.CLIOverrides()) + self.load_default_config(path) for function in ("roles", "auth.bad-name", "1auth.roles", "auth.roles.extra"): path = self.write_config(text="""\ @@ -168,7 +179,7 @@ def test_rejects_invalid_profile_and_function(self): profile = p """ % function) with self.assertRaisesRegex(self.mod.SyncError, "function"): - self.mod.load_config(path, self.mod.CLIOverrides()) + self.load_default_config(path) def test_rejects_invalid_values_and_missing_fields(self): path = self.write_config(text="""\ @@ -193,7 +204,7 @@ def test_rejects_invalid_values_and_missing_fields(self): save_to_disk = true """) with self.assertRaises(self.mod.SyncError): - self.mod.load_config(path, self.mod.CLIOverrides()) + self.load_default_config(path) path = self.write_config(text="""\ [source] @@ -210,7 +221,7 @@ def test_rejects_invalid_values_and_missing_fields(self): profile = p """) with self.assertRaisesRegex(self.mod.SyncError, "password"): - self.mod.load_config(path, self.mod.CLIOverrides()) + self.load_default_config(path) def test_parse_args_supports_overrides(self): args = self.mod.parse_args( @@ -359,14 +370,16 @@ def test_missing_action_is_configurable(self): def test_aborts_for_unmanaged_main_runtime_drift(self): main = self.user("local", password=self.verifier_b) runtime = replace(main, password=self.verifier_a) + settings = self.settings() with self.assertRaisesRegex(self.mod.SyncError, "unmanaged.*runtime"): - self.mod.build_plan({}, [main], [runtime], self.settings()) + self.mod.build_plan({}, [main], [runtime], settings) def test_unmanaged_conflict_requires_adoption(self): existing = self.user("alice") + source = {"alice": self.role("alice")} + settings = self.settings() with self.assertRaisesRegex(self.mod.SyncError, "unmanaged"): - self.mod.build_plan({"alice": self.role("alice")}, [existing], [existing], - self.settings()) + self.mod.build_plan(source, [existing], [existing], settings) plan = self.mod.build_plan({"alice": self.role("alice", self.verifier_b)}, [existing], [existing], self.settings(adopt_existing_users=True)) @@ -375,16 +388,18 @@ def test_unmanaged_conflict_requires_adoption(self): def test_cross_profile_conflict_aborts(self): existing = self.user("alice", profile="other") + source = {"alice": self.role("alice")} + settings = self.settings() with self.assertRaisesRegex(self.mod.SyncError, "another profile"): - self.mod.build_plan({"alice": self.role("alice")}, [existing], [existing], - self.settings()) + self.mod.build_plan(source, [existing], [existing], settings) def test_duplicate_admin_rows_are_rejected(self): first = self.user("alice", profile="p") second = replace(first, backend=0, frontend=1) + source = {"alice": self.role("alice")} + settings = self.settings() with self.assertRaisesRegex(self.mod.SyncError, "multiple.*alice"): - self.mod.build_plan({"alice": self.role("alice")}, [first, second], [], - self.settings()) + self.mod.build_plan(source, [first, second], [], settings) def test_ownership_helpers_validate_and_normalize(self): self.assertIsNone(self.mod.decode_ownership("")) @@ -676,9 +691,10 @@ def owned_user(self, active=1): def test_source_failure_is_secret_safe(self): secret = "source-secret" + source = FakeSource(error=self.mod.SyncError(secret)) + admin = FakeAdmin() with self.assertRaises(self.mod.SyncError) as ctx: - self.mod.run_sync(self.config, FakeSource(error=self.mod.SyncError(secret)), FakeAdmin(), - dry_run=False, verbose=False) + self.mod.run_sync(self.config, source, admin, dry_run=False, verbose=False) self.assertNotIn(secret, str(ctx.exception)) def test_dry_run_does_not_write_load_or_save(self): @@ -693,9 +709,9 @@ def test_dry_run_does_not_write_load_or_save(self): def test_write_failure_never_loads_runtime(self): admin = FakeAdmin(fail_apply=True) + source = FakeSource(self.rows) with self.assertRaises(self.mod.SyncError): - self.mod.run_sync(self.config, FakeSource(self.rows), admin, - dry_run=False, verbose=False) + self.mod.run_sync(self.config, source, admin, dry_run=False, verbose=False) self.assertNotIn("load", admin.calls) self.assertNotIn("save", admin.calls) @@ -744,6 +760,16 @@ def test_exclusive_lock_reports_contention(self): self.assertFalse(second) self.assertEqual(0o600, path.stat().st_mode & 0o777) + def test_exclusive_lock_rejects_symlinked_target(self): + with tempfile.TemporaryDirectory() as directory: + target = Path(directory) / "target.lock" + target.touch() + alias = Path(directory) / "sync.lock" + alias.symlink_to(target) + with self.assertRaisesRegex(self.mod.SyncError, "unable to open"): + with self.mod.exclusive_lock(alias): + pass + def test_summary_has_non_negative_duration(self): summary = self.mod.run_sync(self.config, FakeSource(self.rows), FakeAdmin(), dry_run=True, verbose=False) From 6214b18d14d9a8c066649b3be6b0e47993687f7d Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Thu, 13 Aug 2026 08:05:13 +0000 Subject: [PATCH 17/18] test: make PostgreSQL user sync CI self-contained --- test/tap/Makefile | 18 ++++++++++++-- test/tap/tests/pgsql-user-sync-t.py | 30 +++++++++++++++++++++--- test/tap/tests/pgsql-user-sync-unit-t.py | 4 ++-- 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/test/tap/Makefile b/test/tap/Makefile index 061a6bc3b2..68482502e6 100644 --- a/test/tap/Makefile +++ b/test/tap/Makefile @@ -3,10 +3,23 @@ .DEFAULT: all .PHONY: all -all: tests tests_with_deps unit_tests +all: tests tests_with_deps unit_tests pgsql_user_sync_assets .PHONY: debug -debug: tests tests_with_deps unit_tests +debug: tests tests_with_deps unit_tests pgsql_user_sync_assets + +# CI test workflows restore only the build outputs below src/ and test/. +# Stage the canonical user-sync sample below test/ so its TAP tests exercise +# the same files that are shipped to operators without duplicating them. +.PHONY: pgsql_user_sync_assets +pgsql_user_sync_assets: + mkdir -p pgsql_user_sync/tests + cp ../../tools/pgsql_user_sync/proxysql_pgsql_user_sync.py pgsql_user_sync/ + cp ../../tools/pgsql_user_sync/proxysql_pgsql_user_sync.ini.example pgsql_user_sync/ + cp ../../tools/pgsql_user_sync/create_source_function.sql pgsql_user_sync/ + cp ../../tools/pgsql_user_sync/requirements.txt pgsql_user_sync/ + cp ../../tools/pgsql_user_sync/README.md pgsql_user_sync/ + cp ../../tools/pgsql_user_sync/tests/test_pgsql_user_sync.py pgsql_user_sync/tests/ .PHONY: test_deps test_deps: @@ -62,6 +75,7 @@ clean: cd tests && ${MAKE} -s clean cd tests_with_deps && ${MAKE} -s clean cd tests/unit && ${MAKE} -s clean + rm -rf pgsql_user_sync .PHONY: cleanall .SILENT: cleanall diff --git a/test/tap/tests/pgsql-user-sync-t.py b/test/tap/tests/pgsql-user-sync-t.py index 1c50b0f20f..2bd7a1a3c9 100644 --- a/test/tap/tests/pgsql-user-sync-t.py +++ b/test/tap/tests/pgsql-user-sync-t.py @@ -10,16 +10,40 @@ import uuid from pathlib import Path -import psycopg import pymysql -from psycopg import sql ROOT = Path(os.environ.get("WORKSPACE", Path(__file__).resolve().parents[3])) -SCRIPT = ROOT / "tools/pgsql_user_sync/proxysql_pgsql_user_sync.py" +ASSET_DIR = ROOT / "test/tap/pgsql_user_sync" +SCRIPT = ASSET_DIR / "proxysql_pgsql_user_sync.py" PROFILE = "tap-real" +def load_psycopg(): + try: + import psycopg + from psycopg import sql + except ImportError: + subprocess.run( + [ + sys.executable, + "-m", + "pip", + "install", + "--break-system-packages", + "-r", + str(ASSET_DIR / "requirements.txt"), + ], + check=True, + ) + import psycopg + from psycopg import sql + return psycopg, sql + + +psycopg, sql = load_psycopg() + + class Tap: def __init__(self): self.count = 0 diff --git a/test/tap/tests/pgsql-user-sync-unit-t.py b/test/tap/tests/pgsql-user-sync-unit-t.py index 836543cc12..89d74a91e9 100644 --- a/test/tap/tests/pgsql-user-sync-unit-t.py +++ b/test/tap/tests/pgsql-user-sync-unit-t.py @@ -5,7 +5,7 @@ from pathlib import Path root = Path(os.environ.get("WORKSPACE", Path(__file__).resolve().parents[3])) -suite = root / "tools/pgsql_user_sync/tests/test_pgsql_user_sync.py" +suite = root / "test/tap/pgsql_user_sync/tests/test_pgsql_user_sync.py" raise SystemExit(subprocess.call( - [sys.executable, "-m", "unittest", "-v", str(suite)], cwd=root + [sys.executable, str(suite)], cwd=root )) From d96d03c10b2b97a70a18e13bc8f3751ad754116c Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Thu, 13 Aug 2026 08:46:17 +0000 Subject: [PATCH 18/18] fix: address PostgreSQL user sync review findings --- .gitignore | 1 + .../2026-08-12-pgsql-user-sync-design.md | 4 +- test/tap/Makefile | 2 +- test/tap/tests/pgsql-user-sync-t.py | 41 +++---- test/tap/tests/pgsql-user-sync-unit-t.py | 2 + tools/pgsql_user_sync/README.md | 33 +++--- .../create_source_function.sql | 32 ++++-- .../proxysql_pgsql_user_sync.py | 19 ++-- .../tests/test_pgsql_user_sync.py | 106 +++++++++++++++--- 9 files changed, 166 insertions(+), 74 deletions(-) diff --git a/.gitignore b/.gitignore index 341ebaac8f..dd55ee93bb 100644 --- a/.gitignore +++ b/.gitignore @@ -78,6 +78,7 @@ oldcode/tests/connect_speed # generated test pems test/tap/tests/*.pem test/tap/tests/test_cluster_sync_config/proxysql*.pem +test/tap/pgsql_user_sync/ #binary src/proxysql diff --git a/docs/superpowers/specs/2026-08-12-pgsql-user-sync-design.md b/docs/superpowers/specs/2026-08-12-pgsql-user-sync-design.md index 5ad583910b..5b5736b1e9 100644 --- a/docs/superpowers/specs/2026-08-12-pgsql-user-sync-design.md +++ b/docs/superpowers/specs/2026-08-12-pgsql-user-sync-design.md @@ -359,8 +359,8 @@ function, ProxySQL Admin interface, and verifier-capable authentication path: 8. Make the source lookup fail and verify runtime users remain unchanged. The integration test is registered only in a PostgreSQL-backed TAP group and -is gated on verifier-capable ProxySQL behavior. Until the equivalent of PR -PR #5865 is present in the target branch, the test must report a clear dependency +is gated on verifier-capable ProxySQL behavior. Until functionality equivalent +to PR #5865 is present in the target branch, the test must report a clear dependency skip rather than masking a synchronizer failure. ## Acceptance criteria diff --git a/test/tap/Makefile b/test/tap/Makefile index 68482502e6..31c98f6455 100644 --- a/test/tap/Makefile +++ b/test/tap/Makefile @@ -30,7 +30,7 @@ tap: test_deps cd tap && CC=${CC} CXX=${CXX} ${MAKE} .PHONY: tests -tests: tap test_deps +tests: pgsql_user_sync_assets tap test_deps cd tests && CC=${CC} CXX=${CXX} ${MAKE} $(MAKECMDGOALS) .PHONY: tests_no_infra diff --git a/test/tap/tests/pgsql-user-sync-t.py b/test/tap/tests/pgsql-user-sync-t.py index 2bd7a1a3c9..3c335aaab5 100644 --- a/test/tap/tests/pgsql-user-sync-t.py +++ b/test/tap/tests/pgsql-user-sync-t.py @@ -19,29 +19,13 @@ PROFILE = "tap-real" -def load_psycopg(): - try: - import psycopg - from psycopg import sql - except ImportError: - subprocess.run( - [ - sys.executable, - "-m", - "pip", - "install", - "--break-system-packages", - "-r", - str(ASSET_DIR / "requirements.txt"), - ], - check=True, - ) - import psycopg - from psycopg import sql - return psycopg, sql - - -psycopg, sql = load_psycopg() +try: + import psycopg + from psycopg import sql +except ImportError: + raise SystemExit( + "psycopg is required for this test; install the test-image dependencies before running it" + ) from None class Tap: @@ -269,13 +253,20 @@ def main(): f"active={runtime_row[2] if runtime_row else None} " f"hostgroup={runtime_row[3] if runtime_row else None}" ) + try: + ownership_marker = ( + json.loads(main_row[4] or "{}").get("proxysql_pgsql_user_sync") + if main_row is not None + else None + ) + except (TypeError, json.JSONDecodeError): + ownership_marker = None tap.check( main_row is not None and main_row[1] == verifier and int(main_row[2]) == 1 and int(main_row[3]) == 0 - and json.loads(main_row[4]).get("proxysql_pgsql_user_sync") - == {"profile": PROFILE}, + and ownership_marker == {"profile": PROFILE}, "synchronizer automatically creates the real pgsql_users row", ) tap.check( diff --git a/test/tap/tests/pgsql-user-sync-unit-t.py b/test/tap/tests/pgsql-user-sync-unit-t.py index 89d74a91e9..9b1ebd24d2 100644 --- a/test/tap/tests/pgsql-user-sync-unit-t.py +++ b/test/tap/tests/pgsql-user-sync-unit-t.py @@ -6,6 +6,8 @@ root = Path(os.environ.get("WORKSPACE", Path(__file__).resolve().parents[3])) suite = root / "test/tap/pgsql_user_sync/tests/test_pgsql_user_sync.py" +if not suite.is_file(): + suite = root / "tools/pgsql_user_sync/tests/test_pgsql_user_sync.py" raise SystemExit(subprocess.call( [sys.executable, str(suite)], cwd=root )) diff --git a/tools/pgsql_user_sync/README.md b/tools/pgsql_user_sync/README.md index 40737befc0..181e13ebad 100644 --- a/tools/pgsql_user_sync/README.md +++ b/tools/pgsql_user_sync/README.md @@ -31,22 +31,27 @@ install -o proxysql -g proxysql -m 0600 proxysql_pgsql_user_sync.ini.example /et Set `[proxysql]` to ProxySQL's PostgreSQL Admin interface (normally port `6132`). The synchronizer uses PostgreSQL's simple-query protocol there. -The synchronizer rejects group-write/execute and other-user permissions. The -primary example is service-owned `0600` so the ProxySQL service account can -read it. Alternatively, keep the file root-owned and use `chown root:proxysql` -with mode `0640` (a dedicated `proxysql` group); that is also accepted. An -owner-only `0600` file is the simplest choice when the synchronizer runs as its -owner. Database passwords stay in this file and never appear in Scheduler -arguments or normal logs. +The synchronizer rejects group-write/execute and other-user permissions, and +accepts only files owned by root or by the account running it. The primary +example is service-owned `0600` so the ProxySQL service account can read it. +Alternatively, keep the file root-owned and use `chown root:proxysql` with mode +`0640` (a dedicated `proxysql` group); that is also accepted. Database passwords +stay in this file and never appear in Scheduler arguments or normal logs. ## Create the source function and allow-list As a trusted PostgreSQL administrator, connect to the configured database and -run `create_source_function.sql` after replacing the reader password. The -script is rerunnable: guarded role creation, a `SECURITY DEFINER` function -with `SET search_path = pg_catalog`, and explicit revokes protect the -credential query. The reader receives only database `CONNECT`, schema -`USAGE`, and function `EXECUTE`. +run `create_source_function.sql` with a runtime-only reader password: + +```console +psql --set=ON_ERROR_STOP=1 --set=proxysql_auth_reader_password='choose-a-secret' \\ + --file=create_source_function.sql postgres +``` + +The script is rerunnable: it reapplies the reader password, creates a +`SECURITY DEFINER` function with `SET search_path = pg_catalog`, and explicitly +revokes defaults before granting only database `CONNECT`, schema `USAGE`, and +function `EXECUTE`. The `proxysql_auth_managed` `NOLOGIN` role is the import allow-list. Grant and revoke membership deliberately, for example: @@ -67,7 +72,7 @@ superusers are always excluded. Always use absolute paths when invoking the script: ```console -/usr/bin/python3 /usr/share/proxysql/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py \ +/opt/proxysql-pgsql-user-sync/bin/python /usr/share/proxysql/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py \ --config /etc/proxysql/pgsql-user-sync.ini --dry-run --verbose ``` @@ -92,7 +97,7 @@ INSERT INTO scheduler (id, active, interval_ms, filename, arg1, arg2, arg3, comment) VALUES (9100, 1, 10000, - '/usr/bin/python3', + '/opt/proxysql-pgsql-user-sync/bin/python', '/usr/share/proxysql/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py', '--config', '/etc/proxysql/pgsql-user-sync.ini', diff --git a/tools/pgsql_user_sync/create_source_function.sql b/tools/pgsql_user_sync/create_source_function.sql index 8b249f140f..ca11398aac 100644 --- a/tools/pgsql_user_sync/create_source_function.sql +++ b/tools/pgsql_user_sync/create_source_function.sql @@ -1,7 +1,16 @@ --- Run this file while connected to the database named in the synchronizer --- configuration (normally "postgres") as a trusted PostgreSQL administrator. --- Replace the reader password before running it. This is an operator-managed --- sample; keep this file out of source control after inserting a real secret. +-- Run this file with psql while connected to the database named in the +-- synchronizer configuration (normally "postgres") as a trusted PostgreSQL +-- administrator. Supply a distinct reader password at runtime, for example: +-- psql --set=ON_ERROR_STOP=1 --set=proxysql_auth_reader_password='secret' \\ +-- --file=create_source_function.sql postgres +-- The password variable is required so this sample cannot create a predictable +-- credential when run unchanged. + +\if :{?proxysql_auth_reader_password} +\else +\echo 'Set proxysql_auth_reader_password with psql --set before running this script.' +\quit +\endif -- The membership of this NOLOGIN role is the explicit import allow-list. DO $role$ @@ -16,12 +25,12 @@ ALTER ROLE proxysql_auth_managed NOLOGIN; DO $role$ BEGIN CREATE ROLE proxysql_auth_reader - LOGIN - PASSWORD 'REPLACE_WITH_SOURCE_PASSWORD'; + LOGIN; EXCEPTION WHEN duplicate_object THEN NULL; END $role$; +ALTER ROLE proxysql_auth_reader LOGIN PASSWORD :'proxysql_auth_reader_password'; CREATE SCHEMA IF NOT EXISTS proxysql_auth; ALTER SCHEMA proxysql_auth OWNER TO CURRENT_USER; @@ -48,12 +57,17 @@ AS $function$ $function$; -- Remove defaults first, then grant only the exact access needed by the --- synchronizer. CONNECT is shown for the sample database; change postgres --- when installing into another database. +-- synchronizer. CONNECT is granted on the database where this script runs. REVOKE ALL ON SCHEMA proxysql_auth FROM PUBLIC; REVOKE ALL ON FUNCTION proxysql_auth.export_login_roles() FROM PUBLIC; REVOKE ALL ON FUNCTION proxysql_auth.export_login_roles() FROM proxysql_auth_reader; -GRANT CONNECT ON DATABASE postgres TO proxysql_auth_reader; +DO $grant$ +BEGIN + EXECUTE format( + 'GRANT CONNECT ON DATABASE %I TO proxysql_auth_reader', current_database() + ); +END +$grant$; GRANT USAGE ON SCHEMA proxysql_auth TO proxysql_auth_reader; GRANT EXECUTE ON FUNCTION proxysql_auth.export_login_roles() TO proxysql_auth_reader; diff --git a/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py b/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py index 6d34886397..c3de6bf09c 100644 --- a/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py +++ b/tools/pgsql_user_sync/proxysql_pgsql_user_sync.py @@ -191,6 +191,8 @@ def _validate_timeout(section: configparser.SectionProxy, name: str = "connect_t def _validate_file(info: os.stat_result) -> None: if not stat.S_ISREG(info.st_mode): raise _error("configuration file must be a regular file") + if info.st_uid not in (0, os.geteuid()): + raise _error("configuration file has an unsafe owner") # Group read is a supported deployment mode only for a root-owned file # (typically 0640 with a dedicated service group). Non-root-owned files # must be owner-only. Group write/execute and every other-user permission @@ -211,7 +213,10 @@ def _open_config_file(path: Path): try: fd = os.open(path, flags) _validate_file(os.fstat(fd)) - stream = os.fdopen(fd, "r", encoding="utf-8") + # os.fdopen() owns its descriptor even when stream construction fails. + # Clear fd first so the error path cannot close a reused descriptor. + stream_fd, fd = fd, None + stream = os.fdopen(stream_fd, "r", encoding="utf-8") except SyncError: if fd is not None: os.close(fd) @@ -876,15 +881,11 @@ def save_to_disk(self) -> None: self._execute_command("SAVE PGSQL USERS TO DISK", "unable to save ProxySQL users to disk") -def _sync_failure(message: str) -> SyncError: - return _error(message) - - def _source_snapshot(source: SourceAdapter, allow_empty: bool) -> dict[str, SourceRole]: try: raw_snapshot = source.fetch_snapshot() except Exception: - raise _sync_failure("unable to fetch source role snapshot") from None + raise _error("unable to fetch source role snapshot") from None return validate_snapshot(raw_snapshot, allow_empty) @@ -892,7 +893,7 @@ def _admin_snapshots(admin: AdminAdapter) -> tuple[list[ProxySQLUser], list[Prox try: return admin.fetch_main_users(), admin.fetch_runtime_users() except Exception: - raise _sync_failure("unable to fetch ProxySQL user snapshots") from None + raise _error("unable to fetch ProxySQL user snapshots") from None def _apply_plan(plan: SyncPlan, admin: AdminAdapter, settings: SyncSettings) -> tuple[bool, bool, bool]: @@ -900,13 +901,13 @@ def _apply_plan(plan: SyncPlan, admin: AdminAdapter, settings: SyncSettings) -> try: admin.apply_actions(plan.actions) except Exception: - raise _sync_failure("unable to apply ProxySQL user changes") from None + raise _error("unable to apply ProxySQL user changes") from None if not plan.requires_load: return False, False, False try: admin.load_runtime() except Exception: - raise _sync_failure("unable to load ProxySQL users to runtime") from None + raise _error("unable to load ProxySQL users to runtime") from None if not settings.save_to_disk: return True, False, False try: diff --git a/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py b/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py index 398b1bd0e9..8c829f5d02 100644 --- a/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py +++ b/tools/pgsql_user_sync/tests/test_pgsql_user_sync.py @@ -125,15 +125,59 @@ def test_group_read_requires_root_owner_and_owner_only_is_allowed(self): self.mod.load_config(path, self.mod.CLIOverrides()) non_root_metadata = SimpleNamespace(st_mode=stat.S_IFREG | 0o640, st_uid=1000) - with patch.object(self.mod.os, "fstat", return_value=non_root_metadata): + with ( + patch.object(self.mod.os, "fstat", return_value=non_root_metadata), + patch.object(self.mod.os, "geteuid", return_value=1000), + ): with self.assertRaisesRegex(self.mod.SyncError, "permissions"): self.load_default_config(path) path = self.write_config(mode=0o600) owner_only_metadata = SimpleNamespace(st_mode=stat.S_IFREG | 0o600, st_uid=1000) - with patch.object(self.mod.os, "fstat", return_value=owner_only_metadata): + with ( + patch.object(self.mod.os, "fstat", return_value=owner_only_metadata), + patch.object(self.mod.os, "geteuid", return_value=1000), + ): self.mod.load_config(path, self.mod.CLIOverrides()) + def test_config_owner_must_be_root_or_the_service_user(self): + path = self.write_config(mode=0o600) + service_uid = 1000 + unrelated_owner = SimpleNamespace(st_mode=stat.S_IFREG | 0o600, st_uid=1001) + with ( + patch.object(self.mod.os, "fstat", return_value=unrelated_owner), + patch.object(self.mod.os, "geteuid", return_value=service_uid), + ): + with self.assertRaisesRegex(self.mod.SyncError, "owner"): + self.load_default_config(path) + + service_owned = SimpleNamespace(st_mode=stat.S_IFREG | 0o600, st_uid=service_uid) + with ( + patch.object(self.mod.os, "fstat", return_value=service_owned), + patch.object(self.mod.os, "geteuid", return_value=service_uid), + ): + self.load_default_config(path) + + root_owned_group_read = SimpleNamespace(st_mode=stat.S_IFREG | 0o640, st_uid=0) + with ( + patch.object(self.mod.os, "fstat", return_value=root_owned_group_read), + patch.object(self.mod.os, "geteuid", return_value=service_uid), + ): + self.load_default_config(path) + + def test_fdopen_failure_does_not_close_a_transferred_descriptor(self): + path = self.write_config() + file_info = SimpleNamespace(st_mode=stat.S_IFREG | 0o600, st_uid=os.geteuid()) + with ( + patch.object(self.mod.os, "open", return_value=31), + patch.object(self.mod.os, "fstat", return_value=file_info), + patch.object(self.mod.os, "fdopen", side_effect=OSError), + patch.object(self.mod.os, "close") as close, + ): + with self.assertRaisesRegex(self.mod.SyncError, "cannot be read"): + self.mod._open_config_file(path) + close.assert_not_called() + def test_rejects_group_write_even_when_root_owned(self): path = self.write_config(mode=0o660) root_metadata = SimpleNamespace(st_mode=stat.S_IFREG | 0o660, st_uid=0) @@ -181,15 +225,22 @@ def test_rejects_invalid_profile_and_function(self): with self.assertRaisesRegex(self.mod.SyncError, "function"): self.load_default_config(path) - def test_rejects_invalid_values_and_missing_fields(self): - path = self.write_config(text="""\ + def test_rejects_each_invalid_configuration_value(self): + cases = ( + ("connect_timeout = -1", "", "connect_timeout"), + ("", "default_hostgroup = -1", "default_hostgroup"), + ("", "missing_role_action = remove", "missing_role_action"), + ("", "adopt_existing_users = maybe", "adopt_existing_users"), + ) + for source_setting, sync_setting, message in cases: + path = self.write_config(text="""\ [source] host = source -port = 0 +port = 5432 database = db username = reader password = secret -connect_timeout = -1 +%s [proxysql] host = proxy port = 6032 @@ -197,14 +248,11 @@ def test_rejects_invalid_values_and_missing_fields(self): password = secret [sync] profile = p -default_hostgroup = -1 -missing_role_action = remove -adopt_existing_users = maybe -allow_empty_snapshot = false -save_to_disk = true -""") - with self.assertRaises(self.mod.SyncError): - self.load_default_config(path) +%s +""" % (source_setting, sync_setting)) + with self.subTest(setting=message): + with self.assertRaisesRegex(self.mod.SyncError, message): + self.load_default_config(path) path = self.write_config(text="""\ [source] @@ -827,6 +875,14 @@ def test_source_function_has_security_boundaries(self): self.assertNotIn("GRANT app_login TO proxysql_auth_managed;", sql) self.assertNotIn("REVOKE app_login FROM proxysql_auth_managed;", sql) + def test_source_function_requires_runtime_reader_secret_and_current_database(self): + sql = (ASSET_DIR / "create_source_function.sql").read_text() + self.assertIn("\\if :{?proxysql_auth_reader_password}", sql) + self.assertIn("PASSWORD :'proxysql_auth_reader_password'", sql) + self.assertNotIn("PASSWORD 'REPLACE_WITH_SOURCE_PASSWORD'", sql) + self.assertIn("ALTER ROLE proxysql_auth_reader LOGIN PASSWORD", sql) + self.assertIn("current_database()", sql) + def test_example_configuration_is_loadable_with_protected_permissions(self): source = (ASSET_DIR / "proxysql_pgsql_user_sync.ini.example").read_text() self.assertIn("[source]", source) @@ -850,6 +906,27 @@ def test_requirements_pin_supported_driver_majors(self): self.assertIn("psycopg[binary]>=3.2.13,<4", requirements) self.assertNotIn("PyMySQL", requirements) + def test_tap_assets_have_a_build_dependency_and_clean_output(self): + root = Path(os.environ.get("WORKSPACE", ASSET_DIR.parents[1])) + if not (root / "test/tap/Makefile").is_file(): + root = ASSET_DIR.parents[2] + tap_makefile = (root / "test/tap/Makefile").read_text() + gitignore = (root / ".gitignore").read_text() + self.assertRegex(tap_makefile, r"(?m)^tests:.*\bpgsql_user_sync_assets\b") + self.assertIn("test/tap/pgsql_user_sync/", gitignore) + + def test_tap_wrappers_do_not_install_dependencies_or_require_staging_locally(self): + root = Path(os.environ.get("WORKSPACE", ASSET_DIR.parents[1])) + if not (root / "test/tap/Makefile").is_file(): + root = ASSET_DIR.parents[2] + integration = (root / "test/tap/tests/pgsql-user-sync-t.py").read_text() + unit_wrapper = (root / "test/tap/tests/pgsql-user-sync-unit-t.py").read_text() + self.assertNotIn('"pip"', integration) + self.assertIn("psycopg is required", integration) + self.assertIn('json.loads(main_row[4] or "{}")', integration) + self.assertIn("if not suite.is_file():", unit_wrapper) + self.assertIn("tools/pgsql_user_sync/tests/test_pgsql_user_sync.py", unit_wrapper) + def test_readme_documents_scheduler_and_cluster_safety(self): readme = (ASSET_DIR / "README.md").read_text() for required in ( @@ -871,6 +948,7 @@ def test_readme_documents_scheduler_and_cluster_safety(self): self.assertIn(required, readme) self.assertIn("one authoritative ProxySQL node", readme) self.assertIn("every node", readme) + self.assertIn("/opt/proxysql-pgsql-user-sync/bin/python", readme) if __name__ == "__main__":