fix(migrations): pin pg_temp last in migration search_path#13
Merged
Conversation
Each migration was applied with `SET LOCAL search_path TO <schema>`, which left pg_temp at its implicit highest-priority position. A temporary object could shadow the schema-qualified objects a migration references while it runs with elevated DDL privileges — a privilege-escalation vector. Migrations now run with `SET LOCAL search_path TO <schema>, pg_temp`, pinning the temporary-object schema to the lowest priority. The statement is built by a new `migration_search_path_stmt` helper covered by a unit test. Bumps the crate to 0.1.35 and updates migration docs.
tjgreen42
force-pushed
the
fix/migration-search-path-pg-temp
branch
from
533420b to
1967c53
Compare
Address code-review feedback: the per-session nature of pg_temp means there is no live privilege-escalation path for the trusted SQL the migration runner executes today, so reframe the source comments and CHANGELOG as defense-in-depth following the PostgreSQL search_path guidance (CVE-2018-1058). Also document why pg_catalog is intentionally left implicit in migration_search_path_stmt.
Schema-qualified references cannot be shadowed by search_path; the actual risk is to the unqualified object references migrations rely on the search_path to resolve (e.g. CREATE TABLE instances, CREATE INDEX ... ON orchestrator_queue). Reword the helper/call-site comments, CHANGELOG, and migrations README to say "unqualified references" instead of "schema-qualified objects".
…al gap Address remaining code-review feedback on the pg_temp search_path hardening: - Add an #[ignore]-gated behavioral test (tests/migration_search_path_tests.rs) that proves the runner's `SET LOCAL search_path TO <schema>, pg_temp` resolves an unqualified reference to the schema object, and demonstrates that the legacy statement (pg_temp implicit first) is shadowed by a temp object. The existing unit test only locks the emitted string; this verifies its PostgreSQL semantics. - Document the residual gap in `migration_search_path_stmt`: a name existing only in pg_temp still resolves there, so migrations should reference only objects they define in <schema>. - Note in the CHANGELOG that existing deployments need no re-migration. - Correct the unit-test comment to say "unqualified" (not "schema-qualified") references, matching the corrected threat model. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
pinodeca
approved these changes
Jun 1, 2026
pinodeca
left a comment
pinodeca
left a comment
Contributor
There was a problem hiding this comment.
Approving - feel free to override my recommendations. Opus 4.7 said "ship it", so maybe it's fine, but my prompt was simply "review this feature branch".
The assert_eq! on the full statement already covers the pg_temp ordering, so the additional ends_with check was redundant. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Per repo release procedure (prompts/publish-crate.md), the Cargo.toml version bump and a versioned CHANGELOG heading are part of the dedicated publish step, not feature PRs. Revert to 0.1.34 and move the migration search_path security note under an [Unreleased] section so it is picked up at the next release. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
Each duroxide migration was applied with
SET LOCAL search_path TO <schema>, which leftpg_tempat its implicit highest-priority position. Migrations reference objects by unqualified name (e.g.CREATE TABLE instances,CREATE INDEX ... ON orchestrator_queue) and rely on thesearch_pathto resolve them to the target schema. Withpg_tempfirst, a same-named temporary object could be resolved instead of the intended schema object while the migration runs with elevated DDL privileges.Migrations now run with
SET LOCAL search_path TO <schema>, pg_temp, pinning the temporary-object schema to the lowest priority so it can no longer shadow unqualified references. (Schema-qualified references were never at risk.)This is defense-in-depth following the PostgreSQL
search_pathhardening guidance (CVE-2018-1058):pg_tempis per-session, so there is no live escalation path for the trusted, in-repo SQL the runner executes today — but it protects against future reuse of the migration connection orSECURITY DEFINERmigrations.Changes
src/migrations.rs: append, pg_temp; extract the statement into a testablemigration_search_path_stmthelper with an explanatory comment (incl. whypg_catalogis left implicit).migration_search_path_pins_pg_temp_last.migrations/README.mdand the0002migration comment.CHANGELOG.md:## [0.1.35]Security entry;Cargo.tomlbumped0.1.34 → 0.1.35.Notes
schema_nameis already validated (^[A-Za-z_][A-Za-z0-9_]*$) at provider construction (0.1.34), so direct interpolation remains safe.Verification
cargo fmt --checkcleancargo test --lib→ 39 passed (incl. the new test)cargo clippy --libclean (one pre-existing unrelated warning, untouched)