From 93ccf41255bb0f65082ad483bce99b02bf2c58ea Mon Sep 17 00:00:00 2001 From: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Date: Fri, 16 Oct 2026 09:53:30 -0400 Subject: [PATCH] fix(seed): refuse connection-string PGDATABASE in local-only admin seed guard psql -d treats URI and key=value values as full connection strings that override -h, so PGDATABASE could route the unarmed fixture writer to a shared database. Accept only a plain database name before either psql path, and extend the refusal test with URI and key/value cases (host=, hostaddr=, service=) plus a plain-name control that must reach psql. Also note in replica_fence that prepared-transaction coverage comes from the retained deferred trigger, not the insert-time app guard. Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> --- crates/buzz-db/src/runtime/replica_fence.rs | 5 +- crates/buzz-db/tests/observability_source.rs | 77 ++++++++++++++++---- scripts/seed-admin-dashboard.sh | 6 ++ 3 files changed, 74 insertions(+), 14 deletions(-) diff --git a/crates/buzz-db/src/runtime/replica_fence.rs b/crates/buzz-db/src/runtime/replica_fence.rs index 06401a5a10..b07a233990 100644 --- a/crates/buzz-db/src/runtime/replica_fence.rs +++ b/crates/buzz-db/src/runtime/replica_fence.rs @@ -639,7 +639,10 @@ async fn sample_writer(writer: &PgPool) -> Result { // prepared they have left `pg_stat_activity` but can still commit // after the token. Their deferred floor guard already ran at PREPARE, // so `pg_prepared_xacts.prepared` bounds their rows exactly like - // `xact_start`; fold it into the same minimum. + // `xact_start`; fold it into the same minimum. Only the retained + // deferred trigger provides that PREPARE-time check: the insert-time + // app guard runs earlier, and no app writer issues PREPARE TRANSACTION. + // Dropping the trigger must keep this case covered. let row = sqlx::query( r#" SELECT diff --git a/crates/buzz-db/tests/observability_source.rs b/crates/buzz-db/tests/observability_source.rs index e7504e5422..913ae9cad6 100644 --- a/crates/buzz-db/tests/observability_source.rs +++ b/crates/buzz-db/tests/observability_source.rs @@ -2207,24 +2207,13 @@ fn admin_seed_refuses_remote_database_before_invoking_psql() { std::fs::set_permissions(&psql, std::fs::Permissions::from_mode(0o755)) .expect("executable stub"); let witness = root.join("psql-was-invoked"); - let cases = [ - ("remote host", "shared-db.example.invalid", "", ""), - ("address override", "localhost", "192.0.2.1", ""), - ("service override", "localhost", "", "production"), - ( - "socket-prefixed host list", - "/tmp,shared-db.example.invalid", - "", - "", - ), - ("host list", "localhost,shared-db.example.invalid", "", ""), - ]; - let results = cases.map(|(name, host, address, service)| { + let run = |host: &str, address: &str, service: &str, database: &str| { let output = std::process::Command::new("/bin/bash") .arg(&script) .env("PGHOST", host) .env("PGHOSTADDR", address) .env("PGSERVICE", service) + .env("PGDATABASE", database) .env("PATH", format!("{}:/usr/bin:/bin", root.display())) .env("SEED_PSQL_WITNESS", &witness) .output() @@ -2233,9 +2222,71 @@ fn admin_seed_refuses_remote_database_before_invoking_psql() { if invoked { std::fs::remove_file(&witness).expect("clear witness"); } + (output, invoked) + }; + // Control: a local host with a plain database name reaches psql, so the + // refusals below come from the guard rather than an unrunnable fixture. + let (_, control_invoked) = run("localhost", "", "", "buzz"); + let cases = [ + ("remote host", "shared-db.example.invalid", "", "", ""), + ("address override", "localhost", "192.0.2.1", "", ""), + ("service override", "localhost", "", "production", ""), + ( + "socket-prefixed host list", + "/tmp,shared-db.example.invalid", + "", + "", + "", + ), + ( + "host list", + "localhost,shared-db.example.invalid", + "", + "", + "", + ), + ( + "database URI", + "localhost", + "", + "", + "postgresql://buzz@shared-db.example.invalid/buzz", + ), + ( + "short database URI", + "localhost", + "", + "", + "postgres://shared-db.example.invalid/buzz", + ), + ( + "database key/value host", + "localhost", + "", + "", + "host=shared-db.example.invalid dbname=buzz", + ), + ( + "database key/value hostaddr", + "localhost", + "", + "", + "hostaddr=192.0.2.1 dbname=buzz", + ), + ( + "database key/value service", + "localhost", + "", + "", + "service=production", + ), + ]; + let results = cases.map(|(name, host, address, service, database)| { + let (output, invoked) = run(host, address, service, database); (name, output, invoked) }); std::fs::remove_dir_all(root).expect("remove scratch fixture"); + assert!(control_invoked, "plain local target did not reach psql"); for (name, output, invoked) in results { assert!(!invoked, "{name} reached psql"); assert!(!output.status.success(), "{name} accepted"); diff --git a/scripts/seed-admin-dashboard.sh b/scripts/seed-admin-dashboard.sh index e015560481..38425b4ff6 100755 --- a/scripts/seed-admin-dashboard.sh +++ b/scripts/seed-admin-dashboard.sh @@ -31,6 +31,12 @@ case "${PGHOST}" in exit 1 ;; esac +# psql -d treats URI and key=value values as full connection strings that +# override -h, so accept only a plain database name. +if [[ ! "${PGDATABASE}" =~ ^[A-Za-z0-9_-]+$ ]]; then + echo "error: admin seed requires a local PostgreSQL target with a plain PGDATABASE name" >&2 + exit 1 +fi if command -v psql >/dev/null 2>&1; then run_psql() {