[gobby-cli-#177] fix: reject postgres bootstrap refs

joshwilhelmi · joshwilhelmi · commit 528827a5988d · 2026-05-23T20:23:14.000-05:00
diff --git a/README.md b/README.md
@@ -92,13 +92,9 @@ setups can use `GOBBY_FALKORDB_HOST`, `GOBBY_FALKORDB_PORT`, and
 
 `gcode` 0.8.0+ uses the migrated Gobby PostgreSQL hub. It asks the local daemon
 broker for the hub DSN first. If the daemon is unavailable, it falls back to
-explicit non-keychain sources: `GCODE_DATABASE_URL`, `GOBBY_POSTGRES_DSN`,
-`~/.gobby/gcode.yaml` `database_url`, inline bootstrap `database_url`, then
-supported `database_url_ref` values. Bootstrap fallback requires
-`hub_backend: postgres`. It never reads the native OS keyring directly. For
-explicit daemonless setups, use inline `database_url`.
-If macOS keeps asking for Keychain authorization, check `which -a gcode`; stale
-binaries from before `0.8.4` can still read Keychain directly.
+explicit fallback sources: `GCODE_DATABASE_URL`, `GOBBY_POSTGRES_DSN`,
+`~/.gobby/gcode.yaml` `database_url`, then bootstrap `database_url`.
+Bootstrap fallback requires `hub_backend: postgres`.
 Installing from source or crates.io requires Rust 1.88+.
 
 ### From source
diff --git a/crates/gcode/README.md b/crates/gcode/README.md
@@ -97,14 +97,10 @@ setups can use `GOBBY_FALKORDB_HOST`, `GOBBY_FALKORDB_PORT`, and
 
 Runtime indexing/search requires a migrated Gobby PostgreSQL hub. gcode
 asks the local daemon broker for the hub DSN first. If the daemon is
-unavailable, it falls back to explicit non-keychain sources:
+unavailable, it falls back to explicit fallback sources:
 `GCODE_DATABASE_URL`, `GOBBY_POSTGRES_DSN`, `~/.gobby/gcode.yaml`
-`database_url`, inline bootstrap `database_url`, then supported
-`database_url_ref` values. Bootstrap fallback requires `hub_backend: postgres`.
-gcode never reads the native OS keyring directly. For explicit daemonless
-setups, use inline `database_url`.
-If macOS keeps asking for Keychain authorization, check `which -a gcode`; stale
-binaries from before `0.8.4` can still read Keychain directly.
+`database_url`, then bootstrap `database_url`. Bootstrap fallback requires
+`hub_backend: postgres`.
 
 ### With Gobby
 
diff --git a/crates/gcode/src/db.rs b/crates/gcode/src/db.rs
@@ -7,8 +7,6 @@ use serde::Deserialize;
 
 use crate::schema;
 
-const POSTGRES_DATABASE_URL_REF: &str = "keyring:gobby:postgres_database_url";
-const DAEMON_POSTGRES_DATABASE_URL_REF: &str = "daemon:gobby:postgres_database_url";
 const GCODE_DATABASE_URL_ENV: &str = "GCODE_DATABASE_URL";
 const GOBBY_POSTGRES_DSN_ENV: &str = "GOBBY_POSTGRES_DSN";
 const GCODE_CONFIG_FILENAME: &str = "gcode.yaml";
@@ -24,7 +22,6 @@ struct BrokerDatabaseUrlResponse {
 struct BootstrapDatabase {
     hub_backend: String,
     database_url: Option<String>,
-    database_url_ref: Option<String>,
 }
 
 /// Return Gobby home, respecting `GOBBY_HOME` when the daemon was configured with it.
@@ -43,9 +40,8 @@ pub fn bootstrap_path() -> anyhow::Result<PathBuf> {
 
 /// Resolve the PostgreSQL hub DSN from explicit overrides or Gobby bootstrap config.
 ///
-/// gcode intentionally has no local database fallback. Keyring-backed bootstraps resolve
-/// through the long-lived daemon broker so short-lived gcode processes never touch the OS
-/// keyring directly.
+/// gcode intentionally has no local database fallback. It asks the long-lived daemon
+/// broker first, then falls back to explicit DSN sources for daemonless operation.
 pub fn resolve_database_url() -> anyhow::Result<String> {
     let home = gobby_home()?;
     resolve_database_url_from_sources(
@@ -83,7 +79,7 @@ fn resolve_database_url_from_sources(
         )
     })?;
     let bootstrap = parse_bootstrap_database(&contents)?;
-    resolve_database_url_from_bootstrap(&bootstrap, || broker_resolver(&path))
+    resolve_database_url_from_bootstrap(&bootstrap)
 }
 
 fn resolve_database_url_from_env(
@@ -144,18 +140,21 @@ fn parse_bootstrap_database(contents: &str) -> anyhow::Result<BootstrapDatabase>
         }
     };
 
+    let database_url_ref = get_string("database_url_ref")?;
+    if database_url_ref.is_some() {
+        bail!(
+            "database_url_ref is no longer supported in bootstrap.yaml; store the local PostgreSQL DSN in database_url"
+        );
+    }
+
     Ok(BootstrapDatabase {
         hub_backend: get_string("hub_backend")?
             .context("bootstrap.yaml must include `hub_backend: postgres`")?,
         database_url: get_string("database_url")?,
-        database_url_ref: get_string("database_url_ref")?,
     })
 }
 
-fn resolve_database_url_from_bootstrap(
-    bootstrap: &BootstrapDatabase,
-    broker_resolver: impl Fn() -> anyhow::Result<String>,
-) -> anyhow::Result<String> {
+fn resolve_database_url_from_bootstrap(bootstrap: &BootstrapDatabase) -> anyhow::Result<String> {
     if bootstrap.hub_backend != "postgres" {
         bail!(
             "gcode requires `hub_backend: postgres` in bootstrap.yaml. Current hub_backend is `{}`. Configure the Gobby PostgreSQL hub before running gcode.",
@@ -167,23 +166,7 @@ fn resolve_database_url_from_bootstrap(
         return Ok(database_url.to_string());
     }
 
-    if let Some(database_url_ref) = bootstrap.database_url_ref.as_deref() {
-        parse_database_url_ref(database_url_ref)?;
-        let database_url = broker_resolver().with_context(|| {
-            format!(
-                "failed to resolve database_url_ref {database_url_ref} through the local Gobby daemon broker. gcode does not read the OS keyring directly; start the Gobby daemon or use inline database_url in bootstrap.yaml for daemonless/manual setups"
-            )
-        })?;
-        let database_url = database_url.trim().to_string();
-        if database_url.is_empty() {
-            bail!("database_url broker response for {database_url_ref} was empty");
-        }
-        return Ok(database_url);
-    }
-
-    bail!(
-        "hub_backend=postgres requires `database_url_ref: {POSTGRES_DATABASE_URL_REF}`, `database_url_ref: {DAEMON_POSTGRES_DATABASE_URL_REF}`, or an inline `database_url`"
-    )
+    bail!("hub_backend=postgres requires `database_url` in bootstrap.yaml")
 }
 
 fn non_empty_trimmed(value: Option<String>) -> Option<String> {
@@ -232,16 +215,6 @@ fn request_broker_database_url(daemon_url: &str, token: &str) -> anyhow::Result<
     Ok(database_url)
 }
 
-fn parse_database_url_ref(database_url_ref: &str) -> anyhow::Result<()> {
-    let parts: Vec<&str> = database_url_ref.splitn(3, ':').collect();
-    match parts.as_slice() {
-        ["keyring" | "daemon", "gobby", "postgres_database_url"] => Ok(()),
-        _ => bail!(
-            "database_url_ref must be {POSTGRES_DATABASE_URL_REF} or {DAEMON_POSTGRES_DATABASE_URL_REF}"
-        ),
-    }
-}
-
 pub fn connect_readwrite(database_url: &str) -> anyhow::Result<Client> {
     connect(database_url)
 }
@@ -281,15 +254,10 @@ mod tests {
     use std::net::TcpListener;
     use std::thread;
 
-    fn bootstrap(
-        hub_backend: &str,
-        database_url: Option<&str>,
-        database_url_ref: Option<&str>,
-    ) -> BootstrapDatabase {
+    fn bootstrap(hub_backend: &str, database_url: Option<&str>) -> BootstrapDatabase {
         BootstrapDatabase {
             hub_backend: hub_backend.to_string(),
             database_url: database_url.map(str::to_string),
-            database_url_ref: database_url_ref.map(str::to_string),
         }
     }
 
@@ -413,126 +381,47 @@ mod tests {
     }
 
     #[test]
-    fn postgres_bootstrap_resolves_keyring_ref_through_broker() {
-        let resolved = resolve_database_url_from_bootstrap(
-            &bootstrap("postgres", None, Some(POSTGRES_DATABASE_URL_REF)),
-            || Ok("postgresql://broker/db".to_string()),
-        )
-        .expect("resolve ref");
-
-        assert_eq!(resolved, "postgresql://broker/db");
-    }
-
-    #[test]
-    fn postgres_bootstrap_resolves_daemon_ref_through_broker() {
-        let resolved = resolve_database_url_from_bootstrap(
-            &bootstrap("postgres", None, Some(DAEMON_POSTGRES_DATABASE_URL_REF)),
-            || Ok("postgresql://broker/db".to_string()),
-        )
-        .expect("resolve ref");
+    fn postgres_bootstrap_accepts_inline_url() {
+        let resolved = resolve_database_url_from_bootstrap(&bootstrap(
+            "postgres",
+            Some("postgresql://inline/db"),
+        ))
+        .expect("resolve inline url");
 
-        assert_eq!(resolved, "postgresql://broker/db");
+        assert_eq!(resolved, "postgresql://inline/db");
     }
 
     #[test]
-    fn postgres_bootstrap_ref_fails_clearly_when_broker_fails() {
-        let err = resolve_database_url_from_bootstrap(
-            &bootstrap("postgres", None, Some(POSTGRES_DATABASE_URL_REF)),
-            || bail!("broker unavailable"),
+    fn postgres_bootstrap_rejects_database_url_ref() {
+        let err = parse_bootstrap_database(
+            "hub_backend: postgres\n\
+             database_url_ref: deprecated\n",
         )
-        .expect_err("broker failure must fail");
+        .expect_err("database_url_ref must fail");
 
         let message = err.to_string();
-        assert!(message.contains("local Gobby daemon broker"));
-        assert!(message.contains("does not read the OS keyring directly"));
-    }
-
-    #[test]
-    fn postgres_bootstrap_fails_when_broker_token_is_missing() {
-        let home = tempfile::tempdir().expect("temp home");
-        let bootstrap_path = write_bootstrap(home.path(), 60887);
-
-        let err = resolve_database_url_from_bootstrap(
-            &bootstrap("postgres", None, Some(POSTGRES_DATABASE_URL_REF)),
-            || resolve_brokered_database_url_at(home.path(), &bootstrap_path),
-        )
-        .expect_err("missing broker token must fail");
-
-        assert!(err.to_string().contains("local Gobby daemon broker"));
-    }
-
-    #[test]
-    fn postgres_bootstrap_fails_when_broker_is_unreachable() {
-        let home = tempfile::tempdir().expect("temp home");
-        std::fs::write(home.path().join(LOCAL_CLI_TOKEN_FILENAME), "token\n").expect("write token");
-        let bootstrap_path = write_bootstrap(home.path(), unused_local_port());
-
-        let err = resolve_database_url_from_bootstrap(
-            &bootstrap("postgres", None, Some(POSTGRES_DATABASE_URL_REF)),
-            || resolve_brokered_database_url_at(home.path(), &bootstrap_path),
-        )
-        .expect_err("unreachable broker must fail");
-
-        assert!(err.to_string().contains("local Gobby daemon broker"));
-    }
-
-    #[test]
-    fn postgres_bootstrap_fails_after_broker_401() {
-        assert_broker_http_failure_fails("401 Unauthorized", r#"{"error":"bad token"}"#);
-    }
-
-    #[test]
-    fn postgres_bootstrap_fails_after_broker_403() {
-        assert_broker_http_failure_fails("403 Forbidden", r#"{"error":"forbidden"}"#);
-    }
-
-    #[test]
-    fn postgres_bootstrap_fails_after_broker_non_2xx() {
-        assert_broker_http_failure_fails("503 Service Unavailable", r#"{"error":"unavailable"}"#);
-    }
-
-    #[test]
-    fn postgres_bootstrap_fails_after_broker_invalid_json() {
-        assert_broker_http_failure_fails("200 OK", "not json");
-    }
-
-    #[test]
-    fn postgres_bootstrap_fails_after_broker_empty_database_url() {
-        assert_broker_http_failure_fails("200 OK", r#"{"database_url":"  "}"#);
-    }
-
-    #[test]
-    fn postgres_bootstrap_prefers_inline_url_over_ref() {
-        let resolved = resolve_database_url_from_bootstrap(
-            &bootstrap(
-                "postgres",
-                Some("postgresql://inline/db"),
-                Some(POSTGRES_DATABASE_URL_REF),
-            ),
-            || unreachable!("broker should not be used when inline url is present"),
-        )
-        .expect("resolve inline url");
-
-        assert_eq!(resolved, "postgresql://inline/db");
+        assert!(message.contains("database_url_ref is no longer supported"));
+        assert!(message.contains("database_url"));
     }
 
     #[test]
-    fn postgres_bootstrap_accepts_inline_url() {
-        let resolved = resolve_database_url_from_bootstrap(
-            &bootstrap("postgres", Some("postgresql://inline/db"), None),
-            || unreachable!("broker should not be used"),
+    fn postgres_bootstrap_rejects_database_url_ref_even_with_inline_url() {
+        let err = parse_bootstrap_database(
+            "hub_backend: postgres\n\
+             database_url: postgresql://inline/db\n\
+             database_url_ref: deprecated\n",
         )
-        .expect("resolve inline url");
+        .expect_err("database_url_ref must fail");
 
-        assert_eq!(resolved, "postgresql://inline/db");
+        assert!(
+            err.to_string()
+                .contains("database_url_ref is no longer supported")
+        );
     }
 
     #[test]
     fn non_postgres_bootstrap_fails_clearly() {
-        let err =
-            resolve_database_url_from_bootstrap(&bootstrap("legacy-local", None, None), || {
-                unreachable!("broker should not be used")
-            })
+        let err = resolve_database_url_from_bootstrap(&bootstrap("legacy-local", None))
             .expect_err("non-postgres backend must fail");
 
         let message = err.to_string();
@@ -550,41 +439,24 @@ mod tests {
 
     #[test]
     fn missing_postgres_dsn_fails_clearly() {
-        let err = resolve_database_url_from_bootstrap(&bootstrap("postgres", None, None), || {
-            unreachable!("broker should not be used")
-        })
-        .expect_err("missing dsn must fail");
+        let err = resolve_database_url_from_bootstrap(&bootstrap("postgres", None))
+            .expect_err("missing dsn must fail");
 
-        assert!(err.to_string().contains("database_url_ref"));
-    }
-
-    #[test]
-    fn unsupported_database_url_ref_fails_clearly() {
-        let err = resolve_database_url_from_bootstrap(
-            &bootstrap(
-                "postgres",
-                None,
-                Some("keyring:other:postgres_database_url"),
-            ),
-            || unreachable!("broker should not be used for unsupported refs"),
-        )
-        .expect_err("unsupported ref must fail");
-
-        assert!(err.to_string().contains(POSTGRES_DATABASE_URL_REF));
+        assert!(err.to_string().contains("database_url"));
     }
 
     #[test]
     fn parse_bootstrap_database_reads_postgres_fields() {
         let parsed = parse_bootstrap_database(
             "hub_backend: postgres\n\
-             database_url_ref: daemon:gobby:postgres_database_url\n",
+             database_url: postgresql://inline/db\n",
         )
         .expect("parse bootstrap");
 
         assert_eq!(parsed.hub_backend, "postgres");
         assert_eq!(
-            parsed.database_url_ref.as_deref(),
-            Some(DAEMON_POSTGRES_DATABASE_URL_REF)
+            parsed.database_url.as_deref(),
+            Some("postgresql://inline/db")
         );
     }
 
@@ -760,22 +632,4 @@ mod tests {
         });
         (format!("http://{addr}"), handle)
     }
-
-    fn assert_broker_http_failure_fails(status: &str, body: &str) {
-        let (daemon_url, request) = spawn_http_response(http_response(status, body));
-
-        let err = resolve_database_url_from_bootstrap(
-            &bootstrap("postgres", None, Some(POSTGRES_DATABASE_URL_REF)),
-            || request_broker_database_url(&daemon_url, "token"),
-        )
-        .expect_err("broker failure must fail");
-        let _ = request.join().expect("read request");
-
-        assert!(err.to_string().contains("local Gobby daemon broker"));
-    }
-
-    fn unused_local_port() -> u16 {
-        let listener = TcpListener::bind("127.0.0.1:0").expect("bind unused local port");
-        listener.local_addr().expect("local addr").port()
-    }
 }
diff --git a/docs/guides/gcode-development-guide.md b/docs/guides/gcode-development-guide.md
@@ -77,11 +77,10 @@ A new dedicated module that owns all `git` shell-out logic for project-root dete
 `src/db.rs` asks the local daemon broker for PostgreSQL DSNs first using
 `POST /api/local/runtime/database-url` with `X-Gobby-Local-Token` from
 `local_cli_token` and a 3s timeout. If the broker is unavailable, gcode falls
-back to explicit non-keychain sources: `GCODE_DATABASE_URL`,
-`GOBBY_POSTGRES_DSN`, `$GOBBY_HOME/gcode.yaml` `database_url`, then
+back to explicit fallback sources: `GCODE_DATABASE_URL`, `GOBBY_POSTGRES_DSN`,
+`$GOBBY_HOME/gcode.yaml` `database_url`, then
 `$GOBBY_HOME/bootstrap.yaml` inline `database_url`. Bootstrap still requires
-`hub_backend: postgres` when used. gcode never reads the native OS keyring
-directly.
+`hub_backend: postgres` when used. Bootstrap `database_url_ref` is rejected.
 `connect_readwrite()` and `connect_readonly()` both return a synchronous
 `postgres::Client`; PostgreSQL permissions decide actual access.
 
diff --git a/docs/guides/gcode-user-guide.md b/docs/guides/gcode-user-guide.md
