Skip to main content

max / makenotwork

sando: close self-update/rebuild restart race; test CF2 startup guard (Run 2) Self-update checked active_build then released the guard before triggering the privileged updater, so a /rebuild in the gap could claim the build slot and get SIGKILLed by the restart. Hold the active_build guard across the systemctl trigger so no build can start until the updater is enqueued. Extract the CF2 non-loopback refuse-to-start decision into a pure refuse_unauthenticated_bind predicate and cover its truth table (the startup guard previously had no direct test). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-06-23 00:54 UTC
Commit: 007494ee15d01e7fa807839a8ebc8fba63757b7a
Parent: ff98486
2 files changed, +59 insertions, -18 deletions
M sando/daemon/src/main.rs +37 -1
@@ -4,6 +4,13 @@ use std::net::SocketAddr;
4 4 use std::path::Path;
5 5 use std::sync::Arc;
6 6
7 + /// CF2 fail-closed startup guard: refuse to start when the deploy mutators would
8 + /// be exposed unauthenticated (no token) on a non-loopback bind. A loopback bind
9 + /// without a token is allowed (local-only); any bind with a token is allowed.
10 + fn refuse_unauthenticated_bind(addr: &SocketAddr, has_token: bool) -> bool {
11 + !has_token && !addr.ip().is_loopback()
12 + }
13 +
7 14 #[tokio::main]
8 15 async fn main() -> Result<()> {
9 16 tracing_subscriber::fmt()
@@ -43,7 +50,7 @@ async fn main() -> Result<()> {
43 50 .map(|s| s.trim().to_string())
44 51 .filter(|s| !s.is_empty())
45 52 .map(|s| Arc::from(s.as_str()));
46 - if api_token.is_none() && !addr.ip().is_loopback() {
53 + if refuse_unauthenticated_bind(&addr, api_token.is_some()) {
47 54 anyhow::bail!(
48 55 "SANDO_API_TOKEN is unset but listen={addr} is not loopback; refusing to expose deploy \
49 56 controls unauthenticated. Set SANDO_API_TOKEN (via EnvironmentFile) or bind 127.0.0.1."
@@ -72,3 +79,32 @@ async fn main() -> Result<()> {
72 79 axum::serve(listener, app).await?;
73 80 Ok(())
74 81 }
82 +
83 + #[cfg(test)]
84 + mod tests {
85 + use super::refuse_unauthenticated_bind;
86 + use std::net::SocketAddr;
87 +
88 + fn addr(s: &str) -> SocketAddr {
89 + s.parse().unwrap()
90 + }
91 +
92 + #[test]
93 + fn non_loopback_without_token_is_refused() {
94 + // The exact CF2 posture: tailnet bind, no token.
95 + assert!(refuse_unauthenticated_bind(&addr("100.103.89.95:7766"), false));
96 + assert!(refuse_unauthenticated_bind(&addr("0.0.0.0:7766"), false));
97 + }
98 +
99 + #[test]
100 + fn loopback_without_token_is_allowed() {
101 + assert!(!refuse_unauthenticated_bind(&addr("127.0.0.1:7766"), false));
102 + assert!(!refuse_unauthenticated_bind(&addr("[::1]:7766"), false));
103 + }
104 +
105 + #[test]
106 + fn any_bind_with_token_is_allowed() {
107 + assert!(!refuse_unauthenticated_bind(&addr("100.103.89.95:7766"), true));
108 + assert!(!refuse_unauthenticated_bind(&addr("127.0.0.1:7766"), true));
109 + }
110 + }
M sando/daemon/src/routes.rs +22 -17
@@ -910,8 +910,14 @@ async fn self_update(
910 910 let sha = crate::domain::GitSha::parse(&body.sha)
911 911 .map_err(|e| crate::error::Error::BadRequest(format!("invalid sha: {e}")))?;
912 912
913 - // Don't restart the controller out from under an in-flight server build —
914 - // the restart would SIGKILL it mid-deploy. Make the operator retry once idle.
913 + let unit = self_update_unit(&sha);
914 +
915 + // Don't restart the controller out from under an in-flight server build — the
916 + // restart would SIGKILL it mid-deploy. Hold the active_build slot across the
917 + // trigger, not just the check: releasing it before triggering let a /rebuild
918 + // claim the slot in the gap and start a build the updater then killed (the
919 + // Run 2 TOCTOU). With the guard held, no build can start until the updater is
920 + // enqueued.
915 921 {
916 922 let slot = s.active_build.lock().await;
917 923 if slot.as_ref().is_some_and(|b| !b.handle.is_finished()) {
@@ -919,22 +925,21 @@ async fn self_update(
919 925 "a server build is in flight; retry /self-update once it settles".into(),
920 926 ));
921 927 }
922 - }
923 928
924 - let unit = self_update_unit(&sha);
925 - tracing::warn!(sha = %sha, unit = %unit, "self-update requested; triggering privileged updater");
926 - // `--no-block`: return as soon as the job is enqueued. The build+restart
927 - // outcome lands in `journalctl -u <unit>`; sandod is restarted out from
928 - // under this request, so there is nothing more to await here.
929 - let status = tokio::process::Command::new("systemctl")
930 - .args(["start", "--no-block", &unit])
931 - .status()
932 - .await
933 - .map_err(|e| crate::error::Error::Other(anyhow::anyhow!("spawning systemctl: {e}")))?;
934 - if !status.success() {
935 - return Err(crate::error::Error::Other(anyhow::anyhow!(
936 - "systemctl start {unit} exited {status}; is sando-update@.service installed and the sando-user polkit rule in place?"
937 - )));
929 + tracing::warn!(sha = %sha, unit = %unit, "self-update requested; triggering privileged updater");
930 + // `--no-block`: return as soon as the job is enqueued. The build+restart
931 + // outcome lands in `journalctl -u <unit>`; sandod is restarted out from
932 + // under this request, so there is nothing more to await here.
933 + let status = tokio::process::Command::new("systemctl")
934 + .args(["start", "--no-block", &unit])
935 + .status()
936 + .await
937 + .map_err(|e| crate::error::Error::Other(anyhow::anyhow!("spawning systemctl: {e}")))?;
938 + if !status.success() {
939 + return Err(crate::error::Error::Other(anyhow::anyhow!(
940 + "systemctl start {unit} exited {status}; is sando-update@.service installed and the sando-user polkit rule in place?"
941 + )));
942 + }
938 943 }
939 944 Ok(Json(serde_json::json!({ "accepted": true, "sha": sha.to_string(), "unit": unit })))
940 945 }