Skip to main content

max / makenotwork

sando: refuse unprovisioned promotes, harden the companion path guard, drop the stale hook Three Medium findings from the 2026-07-21 audit. /promote never checked whether the target tier was provisioned. Against the shipped topology, POST /promote/c returned 200: the node loop iterated nothing, node_health returned Blocked (warned, not failed), and advance_tier recorded a current_version for a tier that has never received a byte -- /state then reported tier c running a version it does not have. promote_inner now refuses. install-companion.sh tested its arguments with globs, which `..` walks straight out of: /opt/../etc/systemd/system/x.service matches /opt/*, and a src of .../companions/../../../../etc/shadow matches the bundle pattern. Behind the NOPASSWD sudoers wildcard, and being the only control on that grant, that was `install -m 0755` as root to anywhere plus a restart of any unit. Both paths are now realpath -m normalised before the prefix test, and it is the normalised paths that get installed, so there is no gap between check and use. /opt is resolved too, so the test holds if it is ever a symlink. The src guard had the same hole as the dst one the audit named; both are fixed. Four tests execute the real script rather than reasoning about it, covering both traversal payloads, a service name carrying a separator, and -- so the guards cannot be tightened into uselessness -- the exact argument shape Sando sends, which must still get through. deploy/post-receive is deleted. It was a second, divergent copy of the hook that sent no Authorization header, so against a token-configured daemon every push 401'd and no build triggered, swallowed by the hook's own `|| echo`. Bootstrap installed that copy; the daemon overwrote it from the embedded hooks/post-receive on next start, which is exactly why it went unnoticed. Bootstrap no longer installs a hook at all -- sandod owns it.
Author: Max Johnson <me@maxj.phd> · 2026-07-21 21:52 UTC
Commit: 995309fb2024ae38b6635f87294848c83e37e5ff
Parent: 975f92b
6 files changed, +150 insertions, -46 deletions
M sando/daemon/src/deploy.rs +77
@@ -834,6 +834,83 @@ mod tests {
834 834 );
835 835 }
836 836
837 + // ---- install-companion.sh: the node-side guard rails ----
838 +
839 + /// Run the shipped installer script with three args; returns its exit code.
840 + /// Exercises the real file rather than a copy of its logic, because the
841 + /// script is the ONLY control on a NOPASSWD sudo grant.
842 + fn run_installer(src: &str, dst: &str, service: &str) -> i32 {
843 + let script =
844 + std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../deploy/install-companion.sh");
845 + std::process::Command::new("bash")
846 + .arg(&script)
847 + .args([src, dst, service])
848 + .output()
849 + .expect("running install-companion.sh")
850 + .status
851 + .code()
852 + .expect("script exited via signal")
853 + }
854 +
855 + // Guards run before any filesystem write, so these never install anything.
856 + // Exit 3 = refused by a guard; exit 4 = guards passed, src simply absent.
857 + const REFUSED: i32 = 3;
858 + const PASSED_GUARDS: i32 = 4;
859 +
860 + #[test]
861 + fn installer_refuses_a_dst_that_escapes_opt_via_dotdot() {
862 + // `/opt/../etc/...` matches a bare `/opt/*` glob. With the sudoers
863 + // wildcard that meant `install -m 0755` as root to anywhere, plus a
864 + // restart of any unit — so the path must be normalised before the test.
865 + assert_eq!(
866 + run_installer(
867 + "/opt/mnw/releases/1.0.0/companions/mnw-cli",
868 + "/opt/../etc/systemd/system/evil.service",
869 + "mnw-cli.service",
870 + ),
871 + REFUSED,
872 + );
873 + }
874 +
875 + #[test]
876 + fn installer_refuses_a_src_that_escapes_the_bundle_via_dotdot() {
877 + assert_eq!(
878 + run_installer(
879 + "/opt/mnw/releases/1.0.0/companions/../../../../../etc/shadow",
880 + "/opt/mnw-cli/mnw-cli",
881 + "mnw-cli.service",
882 + ),
883 + REFUSED,
884 + );
885 + }
886 +
887 + #[test]
888 + fn installer_accepts_the_real_companion_paths() {
889 + // The guards must not have been tightened into uselessness: the shape
890 + // Sando actually sends has to get past them. It stops at the missing
891 + // src (exit 4), which is proof the guards accepted it.
892 + assert_eq!(
893 + run_installer(
894 + "/opt/mnw/releases/1.0.0/companions/mnw-cli",
895 + "/opt/mnw-cli/mnw-cli",
896 + "mnw-cli.service",
897 + ),
898 + PASSED_GUARDS,
899 + );
900 + }
901 +
902 + #[test]
903 + fn installer_refuses_a_service_name_with_a_path_separator() {
904 + assert_eq!(
905 + run_installer(
906 + "/opt/mnw/releases/1.0.0/companions/mnw-cli",
907 + "/opt/mnw-cli/mnw-cli",
908 + "../../etc/evil.service",
909 + ),
910 + REFUSED,
911 + );
912 + }
913 +
837 914 // ---- install_companion_cmd: shape + quoting ----
838 915
839 916 #[test]
M sando/daemon/src/routes/mod.rs +33
@@ -1772,6 +1772,39 @@ mod tests {
1772 1772 }
1773 1773
1774 1774 #[tokio::test]
1775 + async fn promote_refuses_an_unprovisioned_tier() {
1776 + // Every step of a promote to a node-less tier is a silent no-op that
1777 + // still reports success: the deploy loop iterates nothing and
1778 + // advance_tier records a current_version the tier never received.
1779 + let (mut state, _tmp) = rollback_fixture("1.0.0", "2.0.0").await;
1780 + let mut topo = (*state.topo).clone();
1781 + topo.tiers[1].provisioned = false;
1782 + topo.tiers[1].nodes.clear();
1783 + state.topo = Arc::new(topo);
1784 + let pool = state.pool.clone();
1785 + sqlx::query("UPDATE tier_state SET current_version = '2.0.0' WHERE tier = 'host'")
1786 + .execute(&pool)
1787 + .await
1788 + .unwrap();
1789 +
1790 + let err = promote_inner(state, "a".into(), PromoteBody::default())
1791 + .await
1792 + .expect_err("promoting to an unprovisioned tier must be refused");
1793 + assert!(
1794 + matches!(&err, crate::error::Error::GateBlocked(m) if m.contains("not provisioned")),
1795 + "got: {err:?}",
1796 + );
1797 +
1798 + // And nothing was recorded: the tier keeps whatever it had before.
1799 + let (cur, _) = tier_versions(&pool, "a").await;
1800 + assert_eq!(
1801 + cur.as_deref(),
1802 + Some("2.0.0"),
1803 + "a refused promote must not advance tier_state",
1804 + );
1805 + }
1806 +
1807 + #[tokio::test]
1775 1808 async fn promote_fails_and_flags_the_tier_when_post_deploy_gates_are_red() {
1776 1809 // The deploy reached every node, so tier_state advances (a stale
1777 1810 // current_version would aim a later rollback at the wrong artifact), but
M sando/daemon/src/routes/promotion.rs +11
@@ -27,6 +27,17 @@ pub(super) async fn promote_inner(
27 27 }
28 28 let target = &s.topo.tiers[idx];
29 29 let source = &s.topo.tiers[idx - 1];
30 + // An unprovisioned tier has no nodes, so every step below is a no-op that
31 + // still *looks* like success: the deploy loop iterates nothing, node_health
32 + // returns Blocked, and advance_tier writes a current_version for a tier that
33 + // has never received a byte. /state then reports tier c running a version it
34 + // does not have. Refuse instead.
35 + if !target.provisioned {
36 + return Err(crate::error::Error::GateBlocked(format!(
37 + "tier {} is not provisioned (no nodes); promoting to it would record a version it never received",
38 + target.name,
39 + )));
40 + }
30 41
31 42 // Resolve version: explicit if given, else the source tier's current.
32 43 let version_str = match body.version {
M sando/deploy/bootstrap-sandod-host.sh +10 -6
@@ -256,15 +256,19 @@ else
256 256 log "12/13 skipping sandod build (BUILD_SANDOD=0)"
257 257 fi
258 258
259 - log "13/13 bare mnw.git + post-receive hook"
259 + log "13/13 bare mnw.git"
260 260 if [[ ! -d "$SANDO_HOME/mnw.git" ]]; then
261 261 sudo -u "$SANDO_USER" git init --bare --initial-branch=main "$SANDO_HOME/mnw.git" >/dev/null
262 262 fi
263 - # Install (or refresh) the post-receive hook that POSTs to sandod on push.
264 - # Sourced from the repo so updates here propagate to the next bootstrap run.
265 - install -m 0755 -o "$SANDO_USER" -g "$SANDO_USER" \
266 - "$SCRIPT_DIR/post-receive" \
267 - "$SANDO_HOME/mnw.git/hooks/post-receive"
263 + # No hook is installed here. sandod owns post-receive: ensure_bare_repo() writes
264 + # it from the copy embedded in the binary (include_str! of hooks/post-receive)
265 + # on every start, so the hook can never drift from the daemon that answers it.
266 + #
267 + # Bootstrap used to install its own second copy, deploy/post-receive, which had
268 + # fallen behind: it sent no Authorization header, so against a token-configured
269 + # daemon every push 401'd and no build triggered -- swallowed by the hook's own
270 + # `|| echo 'sando: rebuild trigger failed'`. It self-healed on the next daemon
271 + # start, which is precisely why nobody noticed. That copy is deleted.
268 272
269 273 # Enable services last so a partial bootstrap doesn't leave a service trying
270 274 # to start against an incomplete environment.
M sando/deploy/install-companion.sh +19 -3
@@ -32,14 +32,28 @@ SERVICE="$3"
32 32 # Guard rails: refuse a src outside a release bundle or a dst outside /opt, and a
33 33 # service name that isn't a bare unit. These bound what a caller (already behind
34 34 # the single sudoers grant) can install/restart — defense in depth, not the
35 - # primary control.
35 + # primary control. Being the ONLY control on a NOPASSWD grant, though, they have
36 + # to actually hold.
37 + #
38 + # Normalise BEFORE testing the prefix. These were once glob-only tests against
39 + # the raw arguments, which `..` walks straight out of: `/opt/../etc/systemd/
40 + # system/x.service` matches `/opt/*`, and `.../releases/v/companions/../../../
41 + # ../etc/shadow` matches the src pattern — i.e. `install -m 0755` as root to
42 + # anywhere on the filesystem. `realpath -m` resolves `..` and symlinks without
43 + # requiring the path to exist (the dst usually does not on a first install).
44 + #
45 + # /opt itself is resolved too, so the comparison still holds if it is a symlink.
46 + SRC="$(realpath -m -- "$SRC")"
47 + DST="$(realpath -m -- "$DST")"
48 + OPT_ROOT="$(realpath -m -- /opt)"
49 +
36 50 case "$SRC" in
37 51 */releases/*/companions/*) : ;;
38 52 *) echo "install-companion: refusing src outside a release bundle: $SRC" >&2; exit 3 ;;
39 53 esac
40 54 case "$DST" in
41 - /opt/*) : ;;
42 - *) echo "install-companion: refusing dst outside /opt: $DST" >&2; exit 3 ;;
55 + "$OPT_ROOT"/*) : ;;
56 + *) echo "install-companion: refusing dst outside $OPT_ROOT: $DST" >&2; exit 3 ;;
43 57 esac
44 58 case "$SERVICE" in
45 59 *[/[:space:]]*|"") echo "install-companion: bad service name: $SERVICE" >&2; exit 3 ;;
@@ -52,6 +66,8 @@ if [[ ! -f "$SRC" ]]; then
52 66 exit 4
53 67 fi
54 68
69 + # Installs the normalised paths, not the raw arguments, so there is no gap
70 + # between what was checked and what is written.
55 71 echo "install-companion: installing $SRC -> $DST"
56 72 install -m 0755 "$SRC" "$DST"
57 73
D sando/deploy/post-receive -37
@@ -1,37 +0,0 @@
1 - #!/usr/bin/env bash
2 - # Sando bare-repo post-receive hook.
3 - #
4 - # Installed at <bare repo>/hooks/post-receive by bootstrap-sandod-host.sh.
5 - # Reads each updated ref from stdin (old new ref) and posts the new sha to
6 - # the daemon's /rebuild endpoint. Only the configured deploy branch is
7 - # acted on; pushes to other refs are silently ignored.
8 -
9 - set -euo pipefail
10 -
11 - # Source the operator's sando.env so $SANDO_DAEMON resolves to the tailnet
12 - # listener, not the 127.0.0.1 default. Hooks run in the ssh push context with
13 - # no environment, so this source step is load-bearing. Tolerate missing file
14 - # so the hook still works in a dev clone.
15 - if [[ -f /etc/sando/sando.env ]]; then
16 - # shellcheck disable=SC1091
17 - source /etc/sando/sando.env
18 - fi
19 -
20 - DAEMON_URL="${SANDO_DAEMON:-http://127.0.0.1:7766}"
21 - DEPLOY_BRANCH="${SANDO_BRANCH:-main}"
22 -
23 - while read -r oldsha newsha ref; do
24 - if [[ "$ref" != "refs/heads/$DEPLOY_BRANCH" ]]; then
25 - continue
26 - fi
27 - if [[ "$newsha" == "0000000000000000000000000000000000000000" ]]; then
28 - # Branch deletion; nothing to build.
29 - continue
30 - fi
31 - echo "sando: posting rebuild for $newsha"
32 - curl --silent --show-error --fail \
33 - -X POST "$DAEMON_URL/rebuild" \
34 - -H 'Content-Type: application/json' \
35 - -d "{\"sha\":\"$newsha\"}" \
36 - || echo "sando: rebuild trigger failed; check daemon"
37 - done