Skip to main content

max / makenotwork

gates: own the scratch cluster's privileges in code Both gate bugs reduced to preconditions that were satisfied by hand on fw13 and recorded nowhere: sando made SUPERUSER, and a makenotwork NOLOGIN role created so a prod dump's ALTER ... OWNER TO would resolve. Any other box failed one gate at a time with an opaque permissions error. reset_scratch now creates the dump's owner role (NOLOGIN, idempotent) and grants it CREATE on public, so migration_dry_run's restore works on a fresh cluster with no manual SQL. The role name is config (scratch_owner_role, default makenotwork), validated [A-Za-z0-9_]+ at load since it is interpolated into DDL. clean_stale_test_dbs drops foreign-owned mnw_test_* clones instead of skipping them; the ownership filter is what let orphans accumulate and degrade the gate. Startup asserts the scratch role has SUPERUSER or pg_signal_backend and refuses to boot otherwise, naming the SQL to fix it. Not part of --check-config, which stays pure. Not done: pg_dump --no-owner --no-privileges. That dump is prod's disaster-recovery backup, not just the gate's input, and stripping owners there would degrade a real restore to serve a dry-run. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-16 22:24 UTC
Commit: c1fc0900ebde6090d7ffd2adfc53b49ae5b96397
Parent: 2285f61
6 files changed, +248 insertions, -21 deletions
M sando/daemon/sando-daemon.toml +10
@@ -13,7 +13,17 @@ release_root = "./releases"
13 13 # worktree. Safe because builds are serialized. Omit for per-worktree target/.
14 14 cargo_target_dir = "./cargo-target"
15 15 # Dropped and recreated on every migration_dry_run. Leave unset to skip.
16 + # The role must be SUPERUSER on the scratch cluster: the gates reset it, seed the
17 + # dump's owner role into it, and drop stale mnw_test_* clones left by a killed
18 + # run (including foreign-owned ones, which DROP DATABASE refuses without
19 + # superuser). The daemon asserts this at startup and refuses to boot otherwise.
20 + # ALTER ROLE sando SUPERUSER;
16 21 scratch_db_url = "postgres://sando@127.0.0.1/sando_scratch"
22 + # Role that owns the objects in a prod pg_dump. The dump carries `ALTER ... OWNER
23 + # TO <role>` for every object, so it must exist in the scratch cluster before a
24 + # restore; reset_scratch creates it NOLOGIN and grants it CREATE on public, so no
25 + # manual SQL is needed on a fresh box. Must match the prod DB owner.
26 + scratch_owner_role = "makenotwork"
17 27
18 28 # Companion crates built from the same worktree/sha as the server and staged
19 29 # into the release bundle, so a contract-coupled service can't drift out of
M sando/daemon/src/build.rs +1 -1
@@ -118,7 +118,7 @@ pub async fn run(
118 118 }
119 119 if let Some(scratch_url) = cfg.scratch_db_url.as_deref() {
120 120 tracing::info!(sha = %sha.as_str(), "preparing scratch DB schema for sqlx compile-time checks");
121 - crate::gates::reset_scratch(scratch_url).await
121 + crate::gates::reset_scratch(scratch_url, &cfg.scratch_owner_role).await
122 122 .context("scratch DB reset before build")?;
123 123 crate::gates::run_migrator(scratch_url, &server_dir.join("migrations")).await
124 124 .context("applying MNW migrations to scratch DB before build")?;
M sando/daemon/src/config.rs +58 -1
@@ -22,6 +22,16 @@ pub struct Config {
22 22 /// you care about.
23 23 #[serde(default)]
24 24 pub scratch_db_url: Option<String>,
25 + /// Role that owns the restored objects in a prod dump. `pg_dump` emits
26 + /// `ALTER ... OWNER TO <role>` for every object, so the role must exist in
27 + /// the scratch cluster before `migration_dry_run` restores — a superuser
28 + /// connection does not conjure it. `reset_scratch` creates it (NOLOGIN) and
29 + /// grants it CREATE on public, so a fresh box needs no manual SQL.
30 + ///
31 + /// Interpolated into DDL as an identifier, so it is restricted to
32 + /// `[A-Za-z0-9_]+` at load (see `validate`) rather than quoted at use.
33 + #[serde(default = "default_scratch_owner_role")]
34 + pub scratch_owner_role: String,
25 35 /// Loopback port the `boot_smoke` gate tells the staged artifact to bind
26 36 /// (`SANDO_BOOT_SMOKE_PORT`), then probes `GET /health` on. Lets the gate
27 37 /// prove readiness, not just liveness. Fixed rather than ephemeral so the
@@ -105,6 +115,7 @@ pub struct ReleaseEntry {
105 115 }
106 116
107 117 fn default_bin_names() -> Vec<String> { vec!["server".into()] }
118 + fn default_scratch_owner_role() -> String { "makenotwork".into() }
108 119 fn default_logs_root() -> PathBuf { PathBuf::from("/srv/sando/logs") }
109 120 fn default_boot_smoke_port() -> u16 { 18181 }
110 121 fn default_gate_timeout_secs() -> u64 { 2400 }
@@ -119,7 +130,26 @@ impl Config {
119 130 let path = std::env::var("SANDO_CONFIG").unwrap_or_else(|_| "sando-daemon.toml".into());
120 131 let raw = std::fs::read_to_string(&path)
121 132 .with_context(|| format!("reading daemon config at {path}"))?;
122 - Ok(toml::from_str(&raw)?)
133 + let cfg: Self = toml::from_str(&raw)?;
134 + cfg.validate()?;
135 + Ok(cfg)
136 + }
137 +
138 + /// Invariants the deserializer can't express. Runs at load (and so under
139 + /// `--check-config`), never at use — a bad value fails startup once, loudly,
140 + /// rather than at the first gate that happens to touch it.
141 + pub fn validate(&self) -> Result<()> {
142 + anyhow::ensure!(
143 + !self.scratch_owner_role.is_empty()
144 + && self
145 + .scratch_owner_role
146 + .bytes()
147 + .all(|b| b.is_ascii_alphanumeric() || b == b'_'),
148 + "scratch_owner_role must be non-empty and match [A-Za-z0-9_]+ (got {:?}); it is \
149 + interpolated into DDL as a bare identifier",
150 + self.scratch_owner_role,
151 + );
152 + Ok(())
123 153 }
124 154
125 155 #[cfg(test)]
@@ -132,6 +162,7 @@ impl Config {
132 162 workdir: PathBuf::from("/tmp/sando-test-workdir"),
133 163 release_root: PathBuf::from("/tmp/sando-test-release-root"),
134 164 scratch_db_url: None,
165 + scratch_owner_role: default_scratch_owner_role(),
135 166 boot_smoke_port: default_boot_smoke_port(),
136 167 bin_names: vec!["server".into()],
137 168 logs_root: PathBuf::from("/tmp/sando-test-logs"),
@@ -198,6 +229,32 @@ mod tests {
198 229 }
199 230
200 231 #[test]
232 + fn scratch_owner_role_defaults_to_makenotwork() {
233 + let cfg: Config = toml::from_str(MINIMAL).unwrap();
234 + assert_eq!(cfg.scratch_owner_role, "makenotwork");
235 + cfg.validate().unwrap();
236 + }
237 +
238 + #[test]
239 + fn scratch_owner_role_rejects_non_identifiers() {
240 + // It is interpolated into DDL as a bare identifier, so anything outside
241 + // [A-Za-z0-9_]+ must fail at load rather than reach the scratch cluster.
242 + for bad in ["", "mnw-owner", "own er", "own\"er", "x; DROP ROLE sando"] {
243 + let raw = format!("{MINIMAL}\nscratch_owner_role = {bad:?}\n");
244 + let cfg: Config = toml::from_str(&raw).unwrap();
245 + assert!(cfg.validate().is_err(), "should reject {bad:?}");
246 + }
247 + }
248 +
249 + #[test]
250 + fn scratch_owner_role_accepts_a_plain_identifier() {
251 + let raw = format!("{MINIMAL}\nscratch_owner_role = \"app_owner_2\"\n");
252 + let cfg: Config = toml::from_str(&raw).unwrap();
253 + cfg.validate().unwrap();
254 + assert_eq!(cfg.scratch_owner_role, "app_owner_2");
255 + }
256 +
257 + #[test]
201 258 fn build_host_is_required() {
202 259 // No safe default: a config without build_host must not parse, so the
203 260 // no-build-on-prod guard can never be silently skipped.
M sando/daemon/src/gates.rs +169 -19
@@ -10,7 +10,7 @@ use crate::events::{self, Event, EventTx};
10 10 use crate::live_log::LiveLog;
11 11 use crate::outcome::{GateBlocker, GateFailure, GateOutcome, LogRef, PassNote};
12 12 use crate::topology::Gate;
13 - use anyhow::Result;
13 + use anyhow::{Context, Result};
14 14 use chrono::Utc;
15 15 use sqlx::SqlitePool;
16 16 use std::path::PathBuf;
@@ -296,7 +296,7 @@ async fn migration_dry_run(ctx: &GateCtx) -> Result<GateOutcome> {
296 296 };
297 297
298 298 log_buf.extend_from_slice(b"---- reset_scratch ----\n");
299 - if let Err(e) = reset_scratch(db_url).await {
299 + if let Err(e) = reset_scratch(db_url, &ctx.cfg.scratch_owner_role).await {
300 300 let msg = format!("scratch reset: {e}");
301 301 log_buf.extend_from_slice(msg.as_bytes());
302 302 return Ok(finish(
@@ -338,18 +338,30 @@ async fn migration_dry_run(ctx: &GateCtx) -> Result<GateOutcome> {
338 338 }
339 339 }
340 340
341 - pub(crate) async fn reset_scratch(db_url: &str) -> Result<()> {
341 + pub(crate) async fn reset_scratch(db_url: &str, owner_role: &str) -> Result<()> {
342 342 use sqlx::postgres::PgPoolOptions;
343 343 use sqlx::Executor;
344 344 let pool = PgPoolOptions::new().max_connections(1).connect(db_url).await?;
345 - // Drop every non-system schema, not just public — migrations create custom
346 - // schemas (e.g. tower_sessions) that survive `DROP SCHEMA public CASCADE`
347 - // and then collide on the next migration run.
348 - pool.execute(
345 + // `owner_role` is validated `[A-Za-z0-9_]+` at config load, so interpolating
346 + // it into DDL is sound. It still goes through `format('%I')` inside the DO
347 + // block for the quoting Postgres expects on an identifier.
348 + let sql = format!(
349 349 r#"
350 350 DO $$
351 351 DECLARE s text;
352 352 BEGIN
353 + -- The dump restores objects owned by the prod role and re-grants to
354 + -- it (`ALTER ... OWNER TO {owner_role}`), which errors if the role
355 + -- is absent — superuser does not imply the role exists. Create it
356 + -- NOLOGIN: the scratch DB needs the role as an *owner* only, never
357 + -- as a connecting identity. Idempotent, so a re-reset is a no-op.
358 + IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = '{owner_role}') THEN
359 + EXECUTE format('CREATE ROLE %I NOLOGIN', '{owner_role}');
360 + END IF;
361 +
362 + -- Drop every non-system schema, not just public — migrations create
363 + -- custom schemas (e.g. tower_sessions) that survive `DROP SCHEMA
364 + -- public CASCADE` and then collide on the next migration run.
353 365 FOR s IN
354 366 SELECT nspname FROM pg_namespace
355 367 WHERE nspname NOT LIKE 'pg_%'
@@ -362,24 +374,78 @@ pub(crate) async fn reset_scratch(db_url: &str) -> Result<()> {
362 374 -- scratch DB. Without this, the freshly-created public is owned by
363 375 -- the connecting role (sando) with no grant to anyone else, so a
364 376 -- migration's FK/trigger check that Postgres runs as a *restored*
365 - -- prod-owned table's owner (e.g. makenotwork from the backup dump)
377 + -- prod-owned table's owner ({owner_role} from the backup dump)
366 378 -- fails with "permission denied for schema public". Granting to
367 379 -- PUBLIC is role-agnostic and safe here — this DB is disposable and
368 380 -- exists only to dry-run migrations.
369 381 EXECUTE 'GRANT USAGE, CREATE ON SCHEMA public TO PUBLIC';
382 + -- PG15+: the new owner needs CREATE on public in its own right, not
383 + -- only via PUBLIC, for the restore's owner-scoped DDL.
384 + EXECUTE format('GRANT USAGE, CREATE ON SCHEMA public TO %I', '{owner_role}');
370 385 END $$;
371 - "#,
386 + "#
387 + );
388 + pool.execute(sql.as_str()).await?;
389 + pool.close().await;
390 + Ok(())
391 + }
392 +
393 + /// Startup assertion for the scratch cluster: the gates reset it, seed an owner
394 + /// role into it, and drop leftover test databases in it — none of which a plain
395 + /// unprivileged role can do. Historically these preconditions were satisfied by
396 + /// hand on fw13 (`ALTER ROLE sando SUPERUSER`, a hand-created `makenotwork`
397 + /// role) and nothing recorded that, so a rebuild elsewhere would fail one gate
398 + /// at a time with an opaque permissions error. Assert once, at boot, loudly.
399 + ///
400 + /// Not part of `--check-config`: that path is pure by design (no DB, no
401 + /// network), and a green there must mean "this build understands its config",
402 + /// not "the cluster is reachable".
403 + pub async fn preflight_scratch_privileges(db_url: &str) -> Result<()> {
404 + use sqlx::postgres::PgPoolOptions;
405 + let pool = PgPoolOptions::new()
406 + .max_connections(1)
407 + .connect(db_url)
408 + .await
409 + .context("connecting to scratch_db_url for the startup privilege check")?;
410 + let (is_super, can_signal): (bool, bool) = sqlx::query_as(
411 + "SELECT rolsuper, pg_catalog.pg_has_role(current_user, 'pg_signal_backend', 'USAGE')
412 + FROM pg_roles WHERE rolname = current_user",
372 413 )
414 + .fetch_one(&pool)
373 415 .await?;
374 416 pool.close().await;
417 + anyhow::ensure!(
418 + is_super || can_signal,
419 + "the scratch_db_url role has neither SUPERUSER nor pg_signal_backend; migration_dry_run \
420 + and cargo_test cannot reset the scratch DB or clear stale test databases. Grant one:\n \
421 + ALTER ROLE <role> SUPERUSER; -- what fw13 uses\n \
422 + GRANT pg_signal_backend TO <role>; -- narrower: terminate only, cannot drop \
423 + foreign-owned databases",
424 + );
425 + if !is_super {
426 + tracing::warn!(
427 + "scratch role has pg_signal_backend but not SUPERUSER: stale test databases owned by \
428 + another role cannot be dropped, and the scratch owner role cannot be created if absent"
429 + );
430 + }
375 431 Ok(())
376 432 }
377 433
378 434 /// Best-effort cleanup of stale per-test database clones (`mnw_test_<uuid>`)
379 - /// left behind by a previously-killed `cargo_test` run. Only drops databases
380 - /// owned by the connecting role — a foreign-owned leftover can't be dropped
381 - /// without superuser anyway, and the harness namespaces its template per role
382 - /// so one can't wedge us.
435 + /// left behind by a previously-killed `cargo_test` run.
436 + ///
437 + /// Drops **foreign-owned leftovers too**, which is why the daemon asserts
438 + /// SUPERUSER at startup (`preflight_scratch_privileges`): `DROP DATABASE`
439 + /// requires ownership or superuser, and the `WITH (FORCE)` terminate requires
440 + /// superuser or `pg_signal_backend`. Without both, orphans from a run under a
441 + /// different role accumulate and degrade the gate — the failure this cleanup
442 + /// exists to prevent.
443 + ///
444 + /// OPERATIONAL HAZARD: fw13 runs one Postgres cluster shared with local `cargo
445 + /// test` as `max`, so a gate firing mid-local-test will force-drop that run's
446 + /// databases out from under it. That collision is known and tracked separately
447 + /// (give the gate its own cluster); until then, do not run local tests on fw13
448 + /// while a Sando gate is live.
383 449 ///
384 450 /// Deliberately **excludes the template** (`mnw_test_template_*`): the harness
385 451 /// reuses it across runs when it's migration-current (skipping a full
@@ -397,14 +463,13 @@ async fn clean_stale_test_dbs(db_url: &str) {
397 463 return;
398 464 }
399 465 };
400 - // Owned by us OR by a role we're a member of (the shared `mnw_test`
401 - // role): pg_has_role(.., 'USAGE') covers both, and membership is enough
402 - // privilege to DROP. Foreign-owned leftovers are left untouched.
466 + // Every per-test clone, whoever owns it. The ownership filter this used to
467 + // carry is what let foreign-owned orphans pile up; superuser (asserted at
468 + // startup) makes them droppable.
403 469 let names: Vec<(String,)> = sqlx::query_as(
404 470 "SELECT datname FROM pg_database
405 471 WHERE datname LIKE 'mnw_test_%'
406 - AND datname NOT LIKE '%template%'
407 - AND pg_catalog.pg_has_role(current_user, datdba, 'USAGE')",
472 + AND datname NOT LIKE '%template%'",
408 473 )
409 474 .fetch_all(&pool)
410 475 .await
@@ -1086,7 +1151,7 @@ mod tests {
1086 1151 .await.unwrap();
1087 1152 pool.close().await;
1088 1153
1089 - reset_scratch(&url).await.expect("reset_scratch");
1154 + reset_scratch(&url, "makenotwork").await.expect("reset_scratch");
1090 1155
1091 1156 let pool = PgPoolOptions::new().max_connections(1).connect(&url).await.unwrap();
1092 1157 let rows: Vec<(String,)> = sqlx::query_as(
@@ -1101,6 +1166,91 @@ mod tests {
1101 1166 pool.close().await;
1102 1167 }
1103 1168
1169 + /// reset_scratch must leave the dump's owner role existing and able to
1170 + /// create in `public`, because a prod `pg_dump` carries `ALTER ... OWNER TO
1171 + /// <role>` for every object. This was satisfied by a hand-created NOLOGIN
1172 + /// role on fw13; nothing recorded it, so any other box failed
1173 + /// migration_dry_run at the restore with "role does not exist".
1174 + ///
1175 + /// Uses a throwaway role name so it can prove the *creation* path rather
1176 + /// than passing on fw13's pre-existing `makenotwork`. Same
1177 + /// `SANDO_TEST_PG_URL` gate as above; needs a superuser connection.
1178 + #[tokio::test]
1179 + async fn reset_scratch_seeds_the_dump_owner_role_when_absent() {
1180 + let Ok(url) = std::env::var("SANDO_TEST_PG_URL") else {
1181 + eprintln!("skipping: SANDO_TEST_PG_URL not set");
1182 + return;
1183 + };
1184 + use sqlx::Executor;
1185 + use sqlx::postgres::PgPoolOptions;
1186 +
1187 + let role = "sando_test_owner_probe";
1188 + // `DROP ROLE` refuses while the role still holds the grants reset_scratch
1189 + // gave it, so drop what it owns first. Idempotent, and a no-op when the
1190 + // role is absent (the usual case on a first run).
1191 + let drop_role = format!(
1192 + "DO $$ BEGIN
1193 + IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = '{role}') THEN
1194 + EXECUTE 'DROP OWNED BY {role}';
1195 + EXECUTE 'DROP ROLE {role}';
1196 + END IF;
1197 + END $$;"
1198 + );
1199 +
1200 + let pool = PgPoolOptions::new().max_connections(1).connect(&url).await.unwrap();
1201 + pool.execute(drop_role.as_str()).await.unwrap();
1202 + pool.close().await;
1203 +
1204 + reset_scratch(&url, role).await.expect("reset_scratch creates the owner role");
1205 +
1206 + let pool = PgPoolOptions::new().max_connections(1).connect(&url).await.unwrap();
1207 + let (exists, can_login): (bool, bool) = sqlx::query_as(
1208 + "SELECT true, rolcanlogin FROM pg_roles WHERE rolname = $1",
1209 + )
1210 + .bind(role)
1211 + .fetch_one(&pool)
1212 + .await
1213 + .expect("owner role exists after reset");
1214 + assert!(exists);
1215 + assert!(!can_login, "the owner role is an owner only, never a login identity");
1216 +
1217 + // The restore's owner-scoped DDL needs CREATE on public in the role's
1218 + // own right (PG15+ dropped the implicit grant).
1219 + let (has_create,): (bool,) = sqlx::query_as(
1220 + "SELECT pg_catalog.has_schema_privilege($1, 'public', 'CREATE')",
1221 + )
1222 + .bind(role)
1223 + .fetch_one(&pool)
1224 + .await
1225 + .unwrap();
1226 + assert!(has_create, "owner role must be able to create in public");
1227 +
1228 + // Idempotent: a second reset must not error on the now-existing role.
1229 + pool.close().await;
1230 + reset_scratch(&url, role).await.expect("reset_scratch is idempotent");
1231 +
1232 + let pool = PgPoolOptions::new().max_connections(1).connect(&url).await.unwrap();
1233 + pool.execute(drop_role.as_str()).await.unwrap();
1234 + pool.close().await;
1235 + }
1236 +
1237 + /// The preflight must pass against a privileged scratch connection. Guards
1238 + /// the catalog query itself: a wrong column or a `current_user` that matches
1239 + /// no `pg_roles` row would make `fetch_one` error (or, worse, a silently
1240 + /// swapped pair would invert the check) and brick startup for everyone.
1241 + /// `SANDO_TEST_PG_URL` is expected to be a superuser connection, as the
1242 + /// gates require.
1243 + #[tokio::test]
1244 + async fn preflight_passes_on_a_privileged_scratch_connection() {
1245 + let Ok(url) = std::env::var("SANDO_TEST_PG_URL") else {
1246 + eprintln!("skipping: SANDO_TEST_PG_URL not set");
1247 + return;
1248 + };
1249 + preflight_scratch_privileges(&url)
1250 + .await
1251 + .expect("a superuser scratch connection must satisfy the preflight");
1252 + }
1253 +
1104 1254 /// CF4: the restore pipeline must carry `ON_ERROR_STOP=1` (so psql fails on
1105 1255 /// a bad statement instead of exiting 0 on a partial restore) and, for a
1106 1256 /// gzip source, `set -o pipefail` (so a `gunzip` failure on a truncated
M sando/daemon/src/main.rs +9
@@ -46,6 +46,7 @@ fn check_config() -> Result<()> {
46 46 /// hand. Split out so it can be exercised against a fixture config without
47 47 /// touching process env (`Config::load` reads `SANDO_CONFIG`/CWD).
48 48 fn validate_loaded(cfg: &config::Config) -> Result<topology::Topology> {
49 + cfg.validate()?;
49 50 let topo = topology::Topology::load(&cfg.topology_path)?;
50 51 topo.ensure_build_host_not_serving(&cfg.build_host)?;
51 52 Ok(topo)
@@ -85,6 +86,14 @@ async fn run() -> Result<()> {
85 86 sync::sync(&pool, &topo).await?;
86 87 tracing::info!(tiers = topo.tiers.len(), bare = %topo.repo.bare_path, "topology synced");
87 88
89 + // Fail closed on the scratch cluster's privileges before any gate can hit
90 + // them, rather than letting the first migration_dry_run discover it as an
91 + // opaque "permission denied". Unset scratch_db_url is already a per-gate
92 + // Blocked outcome, so there is nothing to assert.
93 + if let Some(scratch_url) = cfg.scratch_db_url.as_deref() {
94 + sando_daemon::gates::preflight_scratch_privileges(scratch_url).await?;
95 + }
96 +
88 97 let prom = metrics::init();
89 98 let addr: SocketAddr = cfg.listen.parse()?;
90 99
M sando/daemon/src/routes/mod.rs +1
@@ -793,6 +793,7 @@ mod tests {
793 793 workdir: PathBuf::from("/tmp/sando-work"),
794 794 release_root: PathBuf::from("/tmp/sando-releases"),
795 795 scratch_db_url: None,
796 + scratch_owner_role: "makenotwork".into(),
796 797 boot_smoke_port: 18181,
797 798 bin_names: vec!["makenotwork".into()],
798 799 logs_root: PathBuf::from("/tmp/sando-logs"),