Repository navigation
fix: 3.9.1 follow-up for startup_parameters - #250
Merged
Merged
Conversation
vadv
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
added 7 commits
May 13, 2026 13:19
…n overlay drift A static pool registered for the same (db, user) identifier holds `empty_overlay_hash()`. A concurrent auth_query passthrough login that fetched a non-empty overlay used to take the hash-mismatch branch in both fast and slow paths: drop_dynamic_pool is a no-op for static pools, and new_pools.remove(&identifier) does not check is_dynamic_pool. The result was a static pool silently replaced by a dynamic auth_query pool with operator-defined backend_auth and startup_parameters swapped for the passthrough version. Add should_rebuild_for_overlay_drift(live_hash, fetched_hash, is_dynamic). The rebuild decision now requires both an overlay-hash mismatch AND that the live pool is dynamic. Both fast and slow paths consult the same predicate; backend_auth is refreshed only when the hash matches.
…rtup_parameters PostgreSQL GUC lookup is case-insensitive, and `canonicalize_param_name` lowercases non-tracked keys before they reach the wire. The reserved protocol-extension prefix `_pq_.` was checked case-sensitively, so a configured or auth_query-supplied key like `_PQ_.foo` passed `validate_key`, then got lowercased on the cascade output and arrived at the backend as `_pq_.foo`. That bypassed the documented guard against injecting protocol-extension parameters. Tighten the prefix check to `eq_ignore_ascii_case` on the first five bytes; both `validate` and `validate_entry` (auth_query JSON path) now reject `_PQ_.`, `_Pq_.`, and any other casing.
…_sources `SHOW STARTUP_PARAMETERS` and `/api/pools` previously showed raw operator-written keys, while the wire-ready cascade canonicalised them via `cascade_canonical_keys`. A pool that wrote `TimeZone` over a general `timezone` showed both rows in the read surface, and the wire compare in `effective_startup_parameters_with_sources` then flagged one variant as `dropped_due_to_budget` or `stale`. That pointed the operator at RELOAD/refetch even though the runtime had merged the cascade correctly. Canonicalise inside `resolve_with_sources` with the same function as the runtime. Layer precedence is preserved because later inserts at the same canonical key replace earlier ones, mirroring the original raw-key behaviour.
The new `@web-ui` BDD job in `.github/workflows/bdd-tests.yml` first runs in this branch and exposed that 13 of the 14 scenarios used `And output contains "..."`. cucumber-rs found no matching step definition, marked every assertion as skipped, and failed the suite. The new `/api/config` restart-required scenario never executed. Rewrite the steps as `And the command output should contain "..."`, the phrasing declared by `command_output_should_contain` in `tests/bdd/shell_helper.rs:303`. Scenario behaviour is unchanged.
…apshot Client startup used to compute the operator-managed key set with a second `POOLS` global lookup after `authenticate` returned `server_parameters`. A RELOAD or auth_query overlay refetch between the two reads could leave the filter inspecting one snapshot while `server_parameters` came from another. The visible symptom: `ParameterStatus` messages forwarded to the client carry values for keys that the backend session already has pinned by `startup_parameters`. Have `authenticate` return both via a new `AuthOutcome` struct. The static path snapshots the key set from the pool that produced `server_parameters`; the auth_query path does the same for the shared (dedicated mode) and dynamic (passthrough mode) pools. `OperatorManagedKeys = Arc<HashSet<String>>` keeps the signature readable and preserves the existing `Arc` zero-copy clone.
…us-rules paths Three documentation and CI tweaks from codex review: - `dashboard-validation.yml` listens on `monitoring/prometheus-rules/**` so a rules-only change does not bypass smoke and ground-truth validation. - The per-user `startup_parameters` tutorial now spells out that changes follow `auth_query.cache_ttl` rather than the next reconnect, and lists the levers operators have to roll out immediately. - The generated `startup_parameters` reference describes the runtime reject path (SQLSTATE 53400) instead of the drop-and-continue wording. The `fields.yaml` source of truth is updated alongside the reference text in `general.md`.
…eaks The Phase 6 fields.yaml change replaced the drop-and-continue wording with the SQLSTATE 53400 reject behaviour. The committed reference configs (`pg_doorman.toml`, `pg_doorman.yaml`) embed those descriptions verbatim, so the in-tree copies fell out of sync and the matches-file guard tests in `app::generate::annotated::tests` failed in Library Tests.
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Close the gaps that surfaced in the first deployment cycle of
startup_parametersfrom 3.9.0: SQLSTATE choice, ParameterStatus correctness, auth_query refetch races, RBAC on the new API surfaces, and alert rules.What operators see
SQLSTATE 53400(configuration_limit_exceeded). The previous code used54000. Alert rules and log filters keyed on54000for this case need to switch.PgDoormanStartupParameterPgRejectionships asseverity: warning(wascriticalin 3.9.0). Cascade overflow stayscritical. Re-check on-call routing if it pages by severity.ParameterStatusmessages forwarded to the client no longer overwrite operator-managed GUCs, so the value the client reads matches what the backend actually has.auth_queryaccepts astartup_parameterscolumn typed as nativejsonorjsonb; the::textcast that earlier deployments needed is gone./api/configand/api/poolsshow literalstartup_parametersvalues only toAdmin. SSO readers get the masked view consistent with the rest of the read-only surface./api/configmarksgeneral.host,general.port,web.host,web.port(plusworker_threads,unix_socket_dir,backlog) as restart-required so the SPA stops offering RELOAD on fields that need a restart.auth_querypayloads, dedicated-mode drops, and SSO credentials arriving over insecure transport.Why
The 3.9.0 rollout exposed three classes of issue. First, the rejection SQLSTATE conflated a pooler-side budget decision with a backend statement-cancellation code, which made log triage and alert keying ambiguous. Second, the dynamic pool path had two real races: a
ParameterStatusoverlay leaked across reauthentication, and concurrent first-auth callers could observe a pool created against a now-staleauth_queryrow. Third, the new read APIs leaked literal GUC values to SSO readers and presented bind-address fields as if they were reloadable.The cached merge in the pool removes per-checkout allocation of the merged map and its budget decision; checkout now reads the precomputed result instead of recalculating it.
Risk and rollout
SQLSTATE 54000for the pooler-side budget case must move to53400. Severity ofPgDoormanStartupParameterPgRejectiondropped towarning; pager routing keyed oncriticalfor that alert will stop paging./api/configand/api/poolsreaders: SSO callers no longer see literalstartup_parametervalues; onlyAdmindoes. Anonymous and SSO surfaces stay the same shape, only the values are masked.general->pool->auth_query) are unchanged.jsonbauth_querycolumn applied without::text, and/api/configreturning restart-required for bind-address fields.Test plan
startup_parameters.featureandweb-ui.featurescenarios pass on this branch.startup-parameters.feature(jsonb column) andweb-ui.feature(bind-address restart-required) pass undermake test-bdd.cargo testandcargo clippy --all-targets -- --deny warningsare green.SQLSTATE 54000switched to53400before deploying 3.9.1.