Skip to main content

max / makenotwork

bento: collect over a sync transport, not inline scp Artifact collection built its own transport inline -- `scp -r` remote, `cp -vR` local -- duplicating what SshExec/LocalExec already do with rsync, outside the abstraction that exists to prevent exactly that. No capability gate, no checksum, no resume, and two tools' worth of behavior to keep straight. The fix is NOT to route collect through the host's executor. A mac host's executor is AgentRpc, whose /pull is confined to a narrow pull_root (/Users/max/Dist) -- the confinement that stops an allow-listed caller reading ~/.tauri/passwords.env. Artifacts are built in the repo checkout, outside that root, so that route either 404s or forces pull_root wide enough to undo the confinement. So bento now holds two transports per host: an EXEC transport (may be the agent) and a SYNC transport (never the agent -- always SshExec/LocalExec). A mac host signs over AgentRpc and is collected from over ssh. The agent is an execution transport; bulk data moves over rsync. AgentRpc's refusals already said so. Adds Executor::pull_glob for the collect shape (a recipe knows `bundle/msi/*.msi`, not filenames). Expansion is per-transport and not uniform: SshExec lets the remote login shell expand, LocalExec expands in-process since rsync is spawned without a shell. Zero matches errors on both -- a collect that quietly gathers nothing ships an empty release. Verified against mbp, not just in tests: rsync expands the remote glob (2 of 3 files, correct ones), and a zero-match exits 12. topology::host_ssh and RecipeCtx::host_ssh drop out entirely -- they existed only to feed the inline scp. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-16 17:24 UTC
Commit: d4cb24d0bc8f7f1689f27c4044d9358bde480167
Parent: b0a97aa
16 files changed, +301 insertions, -60 deletions
M bento/daemon/Cargo.lock +7
@@ -659,6 +659,12 @@ dependencies = [
659 659 ]
660 660
661 661 [[package]]
662 + name = "glob"
663 + version = "0.3.3"
664 + source = "registry+https://github.com/rust-lang/crates.io-index"
665 + checksum = "0cc23270f6e1808e30a928bdc84dea0b9b4136a8bc82338574f23baf47bbd280"
666 +
667 + [[package]]
662 668 name = "hashbag"
663 669 version = "0.1.13"
664 670 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -1303,6 +1309,7 @@ dependencies = [
1303 1309 "anyhow",
1304 1310 "async-trait",
1305 1311 "futures-util",
1312 + "glob",
1306 1313 "reqwest",
1307 1314 "serde",
1308 1315 "serde_json",
M bento/daemon/src/engine.rs +30 -38
@@ -19,11 +19,9 @@ use crate::ota::{OtaRegistry, PublishAuthority, Release};
19 19 use crate::state::ExecutorMap;
20 20 use anyhow::{Context as _, Result};
21 21 use ops_core::live_log::LiveLog;
22 - use ops_core::remote::RemoteHost;
23 - use ops_exec::{Action, ObserveKind, Step as OpStep};
22 + use ops_exec::{Action, Executor, ObserveKind, Step as OpStep, SyncOpts};
24 23 use rhai::{Engine, EvalAltResult, Map};
25 24 use sqlx::SqlitePool;
26 - use std::collections::HashMap;
27 25 use std::path::{Path, PathBuf};
28 26 use std::sync::atomic::{AtomicBool, Ordering};
29 27 use std::sync::{Arc, Mutex};
@@ -80,9 +78,10 @@ pub struct RecipeCtx {
80 78 /// through these — the transport (local / ssh / in-session agent) and the
81 79 /// capability gate are the executor's, not the engine's.
82 80 pub execs: Arc<ExecutorMap>,
83 - /// Host name -> ssh string, used only by `collect` to name the remote scp
84 - /// source for a daemon-local artifact pull. Not an execution path.
85 - pub host_ssh: HashMap<String, String>,
81 + /// Sync transport per build host, used only by `collect` to pull artifacts
82 + /// back. Never the agent, even for an agent host — see `state::build_sync`.
83 + /// Not an execution path.
84 + pub syncs: Arc<ExecutorMap>,
86 85 pub pool: SqlitePool,
87 86 pub events: EventTx,
88 87 pub cfg: Arc<Config>,
@@ -114,7 +113,7 @@ impl RecipeCtx {
114 113 repo: String,
115 114 target_run_id: i64,
116 115 execs: Arc<ExecutorMap>,
117 - host_ssh: HashMap<String, String>,
116 + syncs: Arc<ExecutorMap>,
118 117 pool: SqlitePool,
119 118 events: EventTx,
120 119 cfg: Arc<Config>,
@@ -130,7 +129,7 @@ impl RecipeCtx {
130 129 repo,
131 130 target_run_id,
132 131 execs,
133 - host_ssh,
132 + syncs,
134 133 pool,
135 134 events,
136 135 cfg,
@@ -310,8 +309,11 @@ impl RecipeCtx {
310 309 }
311 310
312 311 /// The ssh string for `name` (for `collect`'s remote scp source).
313 - fn host_ssh(&self, name: &str) -> Result<String> {
314 - self.host_ssh
312 + /// The transport that moves artifacts off `name`. Distinct from
313 + /// [`RecipeCtx::exec_for`]'s executor: an agent host signs over `AgentRpc`
314 + /// but is collected from over ssh (`state::build_sync`).
315 + fn host_sync(&self, name: &str) -> Result<Arc<dyn Executor>> {
316 + self.syncs
315 317 .get(name)
316 318 .cloned()
317 319 .ok_or_else(|| anyhow::anyhow!("unknown build host `{name}` (not in topology)"))
@@ -610,36 +612,26 @@ impl RecipeCtx {
610 612 fn collect(self: &Arc<Self>, host: &str, glob: &str, app: &str, version: &str) -> Result<()> {
611 613 let dest = self.cfg.dist_root.join(app).join(version);
612 614 let dest_s = dest.to_string_lossy().into_owned();
613 - let ssh = self.host_ssh(host)?;
614 - let is_local = ssh == "local" || ssh.is_empty();
615 - // Both branches run the transfer through `sh -c`, and both need the
616 - // wildcard (`*?[]`) to reach the shell live — so the glob is left
617 - // UNQUOTED in both and validated against shell metacharacters instead.
618 - // (Quoting it on the local side, as before, neutered the wildcard so
619 - // `cp '*.dmg'` matched nothing.) Path/wildcard chars are allowed; command
620 - // metacharacters are not.
615 + // The glob reaches a remote login shell intact (that's what expands it),
616 + // so command metacharacters stay barred. Path/wildcard chars are fine.
617 + // Not a privilege boundary — a recipe already runs arbitrary shell via
618 + // `sh_ok` — but it keeps a malformed pattern from becoming a command.
621 619 anyhow::ensure!(
622 620 !glob.chars().any(|c| matches!(c, ';' | '&' | '|' | '$' | '`' | '\'' | '"' | '\\' | ' ' | '\n' | '(' | ')' | '<' | '>')),
623 621 "collect glob `{glob}` contains shell metacharacters"
624 622 );
625 - let d = ops_core::remote::sh_quote(&dest_s);
626 - let cmd = if is_local {
627 - format!("mkdir -p {d} && cp -vR {glob} {d}/")
628 - } else {
629 - format!(
630 - "mkdir -p {d} && scp -r {flags} {tgt}:{glob} {d}/",
631 - flags = ops_core::remote::SSH_FLAGS.join(" "),
632 - tgt = ops_core::remote::sh_quote(&ssh),
633 - )
634 - };
635 - // The daemon always runs the transfer itself (local cp or local scp),
636 - // regardless of which host built the artifact.
637 - let sink = self.ensure_step()?;
638 - let local = RemoteHost::new("local");
639 - let out = self.rt.block_on(async move { local.run_streaming(&cmd, sink).await })?;
640 - if !out.success() {
641 - anyhow::bail!("collect failed (exit {:?})", out.status.code());
642 - }
623 + std::fs::create_dir_all(&dest)
624 + .with_context(|| format!("creating collect dest {dest_s}"))?;
625 + // The SYNC transport, not the host's exec executor: artifacts move over
626 + // ssh/rsync even from an agent host, whose `/pull` is confined to a
627 + // narrow `pull_root` that deliberately excludes the repo checkout these
628 + // artifacts are built in (see `state::build_sync`). The daemon still
629 + // runs the transfer itself, as it always has.
630 + let sync = self.host_sync(host)?;
631 + let opts = SyncOpts::precompressed();
632 + self.rt
633 + .block_on(async { sync.pull_glob(glob, &dest, &opts).await })
634 + .with_context(|| format!("collect {glob} from `{host}`"))?;
643 635 // Best-effort size accounting for the event.
644 636 events::emit(
645 637 &self.events,
@@ -963,8 +955,8 @@ mod tests {
963 955 "fw13".into(),
964 956 "/tmp".into(),
965 957 1,
966 - Arc::new(HashMap::new()),
967 - HashMap::new(),
958 + Arc::new(std::collections::HashMap::new()),
959 + Arc::new(std::collections::HashMap::new()),
968 960 pool,
969 961 crate::events::channel(),
970 962 cfg,
M bento/daemon/src/main.rs +2
@@ -56,6 +56,7 @@ async fn main() -> Result<()> {
56 56 }
57 57
58 58 let executors = Arc::new(state::build_executors(&topo));
59 + let syncs = Arc::new(state::build_syncs(&topo));
59 60 let app_state = state::AppState {
60 61 pool,
61 62 topo,
@@ -64,6 +65,7 @@ async fn main() -> Result<()> {
64 65 events: events::channel(),
65 66 ota: Arc::new(ota::OtaRegistry::standard(mnw_base_url())),
66 67 executors,
68 + syncs,
67 69 active: Arc::new(tokio::sync::Mutex::new(HashMap::new())),
68 70 api_token,
69 71 };
M bento/daemon/src/routes.rs +2
@@ -344,6 +344,7 @@ targets = ["linux/x86_64"]
344 344 )
345 345 .unwrap();
346 346 let executors = Arc::new(crate::state::build_executors(&topo));
347 + let syncs = Arc::new(crate::state::build_syncs(&topo));
347 348 AppState {
348 349 pool,
349 350 topo: Arc::new(topo),
@@ -352,6 +353,7 @@ targets = ["linux/x86_64"]
352 353 events: crate::events::channel(),
353 354 ota: Arc::new(OtaRegistry::standard("https://makenot.work")),
354 355 executors,
356 + syncs,
355 357 active: Arc::new(Mutex::new(HashMap::new())),
356 358 api_token: None,
357 359 }
M bento/daemon/src/runner.rs +7 -1
@@ -202,7 +202,7 @@ async fn run_target(
202 202 repo,
203 203 target_run_id,
204 204 state.executors.clone(),
205 - state.topo.host_ssh(),
205 + state.syncs.clone(),
206 206 state.pool.clone(),
207 207 state.events.clone(),
208 208 state.cfg.clone(),
@@ -438,6 +438,7 @@ targets = ["linux/x86_64"]
438 438 .unwrap();
439 439
440 440 let executors = Arc::new(crate::state::build_executors(&topo));
441 + let syncs = Arc::new(crate::state::build_syncs(&topo));
441 442 let state = AppState {
442 443 pool: pool.clone(),
443 444 topo: Arc::new(topo),
@@ -446,6 +447,7 @@ targets = ["linux/x86_64"]
446 447 events: crate::events::channel(),
447 448 ota: Arc::new(OtaRegistry::standard("https://makenot.work")),
448 449 executors,
450 + syncs,
449 451 active: Arc::new(Mutex::new(HashMap::new())),
450 452 api_token: None,
451 453 };
@@ -538,6 +540,7 @@ targets = ["linux/x86_64"]
538 540 ))
539 541 .unwrap();
540 542 let executors = Arc::new(crate::state::build_executors(&topo));
543 + let syncs = Arc::new(crate::state::build_syncs(&topo));
541 544 let state = AppState {
542 545 pool: pool.clone(),
543 546 topo: Arc::new(topo),
@@ -546,6 +549,7 @@ targets = ["linux/x86_64"]
546 549 events: crate::events::channel(),
547 550 ota: Arc::new(OtaRegistry::standard("https://makenot.work")),
548 551 executors,
552 + syncs,
549 553 active: Arc::new(Mutex::new(HashMap::new())),
550 554 api_token: None,
551 555 };
@@ -638,6 +642,7 @@ targets = ["linux/x86_64"]
638 642 .unwrap();
639 643
640 644 let executors = Arc::new(crate::state::build_executors(&topo));
645 + let syncs = Arc::new(crate::state::build_syncs(&topo));
641 646 let state = AppState {
642 647 pool: pool.clone(),
643 648 topo: Arc::new(topo),
@@ -646,6 +651,7 @@ targets = ["linux/x86_64"]
646 651 events: crate::events::channel(),
647 652 ota: Arc::new(OtaRegistry::standard("https://makenot.work")),
648 653 executors,
654 + syncs,
649 655 active: Arc::new(Mutex::new(HashMap::new())),
650 656 api_token: None,
651 657 };
M bento/daemon/src/state.rs +79 -5
@@ -43,8 +43,12 @@ pub struct AppState {
43 43 pub prom: PrometheusHandle,
44 44 pub events: EventTx,
45 45 pub ota: Arc<OtaRegistry>,
46 - /// One capability-scoped [`Executor`] per build host, from the topology.
46 + /// One capability-scoped [`Executor`] per build host, from the topology —
47 + /// the transport that *runs steps* (may be the in-session agent).
47 48 pub executors: Arc<ExecutorMap>,
49 + /// One transport per build host for *moving artifacts* off it. Never the
50 + /// agent — see [`build_sync`].
51 + pub syncs: Arc<ExecutorMap>,
48 52 /// Bearer token required on the build-triggering routes (`/build`,
49 53 /// `/retry`). Sourced from `BENTO_API_TOKEN` (systemd EnvironmentFile).
50 54 /// `None` = unauthenticated, which main() permits only on a loopback bind
@@ -58,9 +62,12 @@ pub struct AppState {
58 62 pub active: ActiveBuilds,
59 63 }
60 64
61 - /// Build one host's executor: `LocalExec` for `ssh = "local"`, `AgentRpc` for an
62 - /// agent-transport host (macOS in-session signing), `SshExec` otherwise — each
63 - /// granted exactly the host's declared capabilities.
65 + /// Build one host's EXEC executor: `LocalExec` for `ssh = "local"`, `AgentRpc`
66 + /// for an agent-transport host (macOS in-session signing), `SshExec` otherwise —
67 + /// each granted exactly the host's declared capabilities.
68 + ///
69 + /// This is the transport for *running steps*. Moving artifacts uses
70 + /// [`build_sync`] instead; see it for why the two are not the same thing.
64 71 pub fn build_executor(host: &Host) -> Arc<dyn Executor> {
65 72 let caps = CapabilitySet::from_tokens(&host.actuate, &host.observe);
66 73 match host.transport {
@@ -76,11 +83,40 @@ pub fn build_executor(host: &Host) -> Arc<dyn Executor> {
76 83 }
77 84 }
78 85
79 - /// Build the full host name -> executor map from the topology.
86 + /// Build one host's SYNC transport — how the daemon moves *artifacts* off it.
87 + /// Always `LocalExec`/`SshExec`, **never `AgentRpc`, even for an agent host.**
88 + ///
89 + /// The agent is an *execution* transport: it exists so codesign runs in the Aqua
90 + /// session where the Developer ID key is usable (design §7 "THE WALL"). It is
91 + /// deliberately a poor artifact mover — `/pull` is confined to the agent's
92 + /// narrow `pull_root` (mbp: `/Users/max/Dist`), which is what keeps an
93 + /// allow-listed caller from reading `~/.tauri/passwords.env`. Build artifacts
94 + /// live in the *repo checkout*, outside that root, so routing collect through
95 + /// the agent would either 404 or force `pull_root` wide enough to undo the
96 + /// confinement. `AgentRpc::push_dir`/`pull_glob` say the same thing in their
97 + /// refusals: bulk data moves over ssh/rsync, not the agent.
98 + ///
99 + /// So a mac host has two transports at once: `AgentRpc` to sign, `SshExec` to
100 + /// fetch what it signed. Both reach the same box; only the privilege differs.
101 + pub fn build_sync(host: &Host) -> Arc<dyn Executor> {
102 + let caps = CapabilitySet::from_tokens(&host.actuate, &host.observe);
103 + if host.ssh == "local" || host.ssh.is_empty() {
104 + Arc::new(LocalExec::new(caps))
105 + } else {
106 + Arc::new(SshExec::new(host.ssh.clone(), caps))
107 + }
108 + }
109 +
110 + /// Build the full host name -> exec-executor map from the topology.
80 111 pub fn build_executors(topo: &Topology) -> ExecutorMap {
81 112 topo.hosts.iter().map(|h| (h.name.clone(), build_executor(h))).collect()
82 113 }
83 114
115 + /// Build the full host name -> sync-transport map from the topology.
116 + pub fn build_syncs(topo: &Topology) -> ExecutorMap {
117 + topo.hosts.iter().map(|h| (h.name.clone(), build_sync(h))).collect()
118 + }
119 +
84 120 #[cfg(test)]
85 121 mod tests {
86 122 use super::*;
@@ -104,6 +140,44 @@ mod tests {
104 140 assert!(!exec.capabilities().permits(&Action::Sign));
105 141 }
106 142
143 + /// The load-bearing property of the two-plane split: a mac host signs over
144 + /// the agent but is COLLECTED FROM over ssh. If this ever regresses to one
145 + /// transport, collect hits `AgentRpc::pull_glob` (refused by design) or
146 + /// forces `pull_root` wide enough to expose `~/.tauri/passwords.env`.
147 + #[tokio::test]
148 + async fn agent_host_syncs_over_ssh_never_the_agent() {
149 + let h = host(
150 + "[[host]]\nname = \"mbp\"\nssh = \"mbp\"\ntargets = [\"macos/aarch64\"]\n\
151 + transport = \"agent\"\nagent_url = \"http://mbp:8765\"\n\
152 + actuate = [\"build\", \"sign\", \"notarize\", \"staple\"]",
153 + );
154 + // The sync transport must not be the agent. AgentRpc refuses pull_glob
155 + // by design, so a non-refusing error proves we got an ssh transport.
156 + let sync = build_sync(&h);
157 + let err = sync
158 + .pull_glob("/nonexistent/*.dmg", std::path::Path::new("/tmp"), &Default::default())
159 + .await
160 + .expect_err("nothing matches, so this must error either way");
161 + assert!(
162 + !err.to_string().contains("unsupported by design"),
163 + "sync transport for an agent host must NOT be AgentRpc: {err}"
164 + );
165 + // ...while the exec transport for the same host still is the agent.
166 + let exec = build_executor(&h);
167 + let err = exec
168 + .pull_glob("/x/*.dmg", std::path::Path::new("/tmp"), &Default::default())
169 + .await
170 + .expect_err("AgentRpc has no glob form");
171 + assert!(err.to_string().contains("unsupported by design"), "exec plane is the agent: {err}");
172 + }
173 +
174 + #[test]
175 + fn local_host_syncs_locally() {
176 + let h = host("[[host]]\nname = \"fw13\"\nssh = \"local\"\ntargets = [\"linux/x86_64\"]");
177 + // A local host's sync transport still carries the host's grant.
178 + assert!(build_sync(&h).capabilities().permits(&Action::Build));
179 + }
180 +
107 181 #[test]
108 182 fn agent_host_executor_permits_sign() {
109 183 // The mac host: agent transport, widened grant. Proves AgentRpc is
M bento/daemon/src/topology.rs -15
@@ -145,13 +145,6 @@ impl Topology {
145 145 self.hosts.iter().find(|h| h.targets.contains(&target))
146 146 }
147 147
148 - /// Map of host name -> ssh string, for `collect`'s daemon-local scp source.
149 - /// (Command execution goes through the capability-scoped executors built in
150 - /// `state::build_executors`, not through this map.)
151 - pub fn host_ssh(&self) -> HashMap<String, String> {
152 - self.hosts.iter().map(|h| (h.name.clone(), h.ssh.clone())).collect()
153 - }
154 -
155 148 pub fn app(&self, app: &AppId) -> Option<&AppConfig> {
156 149 self.app.get(app.as_str())
157 150 }
@@ -208,14 +201,6 @@ targets = ["windows/x86_64"]
208 201 }
209 202
210 203 #[test]
211 - fn host_ssh_maps_names_to_ssh_strings() {
212 - let t = load(SAMPLE).unwrap();
213 - let ssh = t.host_ssh();
214 - assert_eq!(ssh["fw13"], "local");
215 - assert_eq!(ssh["mbp"], "mbp");
216 - }
217 -
218 - #[test]
219 204 fn capability_defaults_make_a_build_host() {
220 205 let t = load(SAMPLE).unwrap();
221 206 let fw13 = t.hosts.iter().find(|h| h.name == "fw13").unwrap();
M bento/driver/Cargo.lock +7
@@ -250,6 +250,12 @@ dependencies = [
250 250 ]
251 251
252 252 [[package]]
253 + name = "glob"
254 + version = "0.3.3"
255 + source = "registry+https://github.com/rust-lang/crates.io-index"
256 + checksum = "0cc23270f6e1808e30a928bdc84dea0b9b4136a8bc82338574f23baf47bbd280"
257 +
258 + [[package]]
253 259 name = "hashbrown"
254 260 version = "0.15.5"
255 261 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -603,6 +609,7 @@ dependencies = [
603 609 "anyhow",
604 610 "async-trait",
605 611 "futures-util",
612 + "glob",
606 613 "reqwest",
607 614 "serde",
608 615 "serde_json",
M bento/tui/Cargo.lock +7
@@ -741,6 +741,12 @@ dependencies = [
741 741 ]
742 742
743 743 [[package]]
744 + name = "glob"
745 + version = "0.3.3"
746 + source = "registry+https://github.com/rust-lang/crates.io-index"
747 + checksum = "0cc23270f6e1808e30a928bdc84dea0b9b4136a8bc82338574f23baf47bbd280"
748 +
749 + [[package]]
744 750 name = "hashbag"
745 751 version = "0.1.13"
746 752 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -1417,6 +1423,7 @@ dependencies = [
1417 1423 "anyhow",
1418 1424 "async-trait",
1419 1425 "futures-util",
1426 + "glob",
1420 1427 "reqwest",
1421 1428 "serde",
1422 1429 "serde_json",
M sando/Cargo.lock +7
@@ -665,6 +665,12 @@ dependencies = [
665 665 ]
666 666
667 667 [[package]]
668 + name = "glob"
669 + version = "0.3.3"
670 + source = "registry+https://github.com/rust-lang/crates.io-index"
671 + checksum = "0cc23270f6e1808e30a928bdc84dea0b9b4136a8bc82338574f23baf47bbd280"
672 +
673 + [[package]]
668 674 name = "hashbag"
669 675 version = "0.1.13"
670 676 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -1334,6 +1340,7 @@ version = "0.1.0"
1334 1340 dependencies = [
1335 1341 "anyhow",
1336 1342 "async-trait",
1343 + "glob",
1337 1344 "serde",
1338 1345 "thiserror 2.0.18",
1339 1346 "tokio",
M shared/ops-core/Cargo.lock +7
@@ -428,6 +428,12 @@ dependencies = [
428 428 ]
429 429
430 430 [[package]]
431 + name = "glob"
432 + version = "0.3.3"
433 + source = "registry+https://github.com/rust-lang/crates.io-index"
434 + checksum = "0cc23270f6e1808e30a928bdc84dea0b9b4136a8bc82338574f23baf47bbd280"
435 +
436 + [[package]]
431 437 name = "hashbrown"
432 438 version = "0.15.5"
433 439 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -833,6 +839,7 @@ version = "0.1.0"
833 839 dependencies = [
834 840 "anyhow",
835 841 "async-trait",
842 + "glob",
836 843 "serde",
837 844 "thiserror",
838 845 "tokio",
M shared/ops-exec/Cargo.lock +7
@@ -287,6 +287,12 @@ dependencies = [
287 287 ]
288 288
289 289 [[package]]
290 + name = "glob"
291 + version = "0.3.3"
292 + source = "registry+https://github.com/rust-lang/crates.io-index"
293 + checksum = "0cc23270f6e1808e30a928bdc84dea0b9b4136a8bc82338574f23baf47bbd280"
294 +
295 + [[package]]
290 296 name = "hashbrown"
291 297 version = "0.15.5"
292 298 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -660,6 +666,7 @@ dependencies = [
660 666 "async-trait",
661 667 "axum",
662 668 "futures-util",
669 + "glob",
663 670 "reqwest",
664 671 "serde",
665 672 "serde_json",
M shared/ops-exec/Cargo.toml +1
@@ -28,6 +28,7 @@ axum = { version = "0.8.8", optional = true }
28 28 tokio-stream = { version = "0.1", optional = true }
29 29 tracing-subscriber = { version = "0.3.22", features = ["env-filter"], optional = true }
30 30 toml = { version = "0.8", optional = true }
31 + glob = "0.3.3"
31 32
32 33 [[bin]]
33 34 name = "ops-agent"
M shared/ops-exec/src/executor.rs +13
@@ -100,6 +100,19 @@ pub trait Executor: Send + Sync {
100 100 /// into `local`). See [`Executor::pull_file`] for why this is separate.
101 101 async fn pull_dir(&self, remote: &Path, local: &Path, opts: &SyncOpts) -> Result<()>;
102 102
103 + /// Pull every file matching a shell GLOB into the directory `local_dir`.
104 + ///
105 + /// The artifact-collection shape: a recipe knows `…/bundle/msi/*.msi`, not
106 + /// the exact filenames. Matching zero files is an error — a collect that
107 + /// silently gathers nothing is how an empty release ships.
108 + ///
109 + /// `remote_glob` is a `str`, not a `Path`, because it is a *pattern*: the
110 + /// wildcard must survive to whatever expands it, and `Path` invites callers
111 + /// to `join`/normalize it. Expansion differs per transport — the ssh
112 + /// transport lets the REMOTE shell expand, the local one expands in-process
113 + /// — so implementations must not assume a shell is involved.
114 + async fn pull_glob(&self, remote_glob: &str, local_dir: &Path, opts: &SyncOpts) -> Result<()>;
115 +
103 116 /// Push the contents of a local directory to the host (rsync into `remote`).
104 117 ///
105 118 /// Directory-only: both rsync transports append a trailing slash, and
M shared/ops-exec/src/rpc.rs +11
@@ -187,6 +187,17 @@ impl Executor for AgentRpc {
187 187 )
188 188 }
189 189
190 + async fn pull_glob(&self, _remote_glob: &str, _local_dir: &Path, _opts: &SyncOpts) -> Result<()> {
191 + // `/pull` takes one concrete path and has no expansion surface — and
192 + // giving it one would mean the agent globbing its own filesystem on a
193 + // caller's behalf. Artifact collection over the agent is per-file.
194 + anyhow::bail!(
195 + "AgentRpc::pull_glob is unsupported by design; the agent serves one concrete \
196 + path per /pull — resolve the glob on the host and use pull_file, or collect \
197 + with SshExec/rsync"
198 + )
199 + }
200 +
190 201 async fn push_dir(&self, _local: &Path, _remote: &Path, _opts: &SyncOpts) -> Result<()> {
191 202 // The agent transport is for in-session *execution*. Bulk data movement
192 203 // onto the host uses SshExec/rsync or a `git pull` step inside the
M shared/ops-exec/src/transport.rs +114 -1
@@ -68,6 +68,18 @@ fn validate_env_names(step: &Step) -> Result<()> {
68 68 /// is the `ssh` argv rsync should transport over — [`RemoteHost::ssh_args`],
69 69 /// so the port and flags match the exec path exactly; `None` = a local rsync.
70 70 fn rsync_command(src: &str, dst: &str, ssh_args: Option<Vec<String>>, opts: &SyncOpts) -> Command {
71 + rsync_command_multi(std::slice::from_ref(&src.to_string()), dst, ssh_args, opts)
72 + }
73 +
74 + /// As [`rsync_command`], but with several sources into one destination — what a
75 + /// glob expands to. `srcs` are passed as separate argv entries, never joined,
76 + /// so a path is never re-split on whitespace.
77 + fn rsync_command_multi(
78 + srcs: &[String],
79 + dst: &str,
80 + ssh_args: Option<Vec<String>>,
81 + opts: &SyncOpts,
82 + ) -> Command {
71 83 let mut rsync = Command::new("rsync");
72 84 // Kill the transfer if the caller's future is dropped (e.g. a promote whose
73 85 // HTTP handler was cancelled by a client disconnect). Without this the
@@ -92,10 +104,26 @@ fn rsync_command(src: &str, dst: &str, ssh_args: Option<Vec<String>>, opts: &Syn
92 104 if let Some(args) = ssh_args {
93 105 rsync.arg("-e").arg(format!("ssh {}", args.join(" ")));
94 106 }
95 - rsync.arg(src).arg(dst);
107 + rsync.args(srcs).arg(dst);
96 108 rsync
97 109 }
98 110
111 + /// Expand a glob against the local filesystem, for the transports that have no
112 + /// remote shell to do it. Returns the matches sorted (deterministic argv), and
113 + /// errors when nothing matches — a collect that quietly gathers zero files is
114 + /// how an empty release ships.
115 + fn expand_glob_locally(pattern: &str) -> Result<Vec<String>> {
116 + let paths = glob::glob(pattern).with_context(|| format!("bad glob pattern `{pattern}`"))?;
117 + let mut out: Vec<String> = Vec::new();
118 + for entry in paths {
119 + let path = entry.with_context(|| format!("reading glob match for `{pattern}`"))?;
120 + out.push(path.to_string_lossy().into_owned());
121 + }
122 + anyhow::ensure!(!out.is_empty(), "glob `{pattern}` matched no files");
123 + out.sort();
124 + Ok(out)
125 + }
126 +
99 127 async fn run_rsync(mut cmd: Command, what: &str) -> Result<()> {
100 128 let out = cmd.output().await.with_context(|| format!("spawning rsync ({what})"))?;
101 129 anyhow::ensure!(
@@ -139,6 +167,16 @@ impl Executor for LocalExec {
139 167 run_rsync(rsync_command(&src, &local.to_string_lossy(), None, opts), "pull_dir(local)").await
140 168 }
141 169
170 + async fn pull_glob(&self, remote_glob: &str, local_dir: &Path, opts: &SyncOpts) -> Result<()> {
171 + // No remote shell to expand for us, and `rsync` is spawned directly (no
172 + // shell), so expand in-process. This is the half of `pull_glob` that
173 + // differs from the ssh transport, and the reason the trait's docs warn
174 + // against assuming a shell.
175 + let matches = expand_glob_locally(remote_glob)?;
176 + let dst = format!("{}/", local_dir.display());
177 + run_rsync(rsync_command_multi(&matches, &dst, None, opts), "pull_glob(local)").await
178 + }
179 +
142 180 async fn push_dir(&self, local: &Path, remote: &Path, opts: &SyncOpts) -> Result<()> {
143 181 let src = format!("{}/", local.display());
144 182 run_rsync(rsync_command(&src, &remote.to_string_lossy(), None, opts), "push_dir(local)").await
@@ -195,6 +233,17 @@ impl Executor for SshExec {
195 233 run_rsync(rsync_command(&src, &local.to_string_lossy(), ssh, opts), "pull_dir(ssh)").await
196 234 }
197 235
236 + async fn pull_glob(&self, remote_glob: &str, local_dir: &Path, opts: &SyncOpts) -> Result<()> {
237 + // The REMOTE shell expands this one: rsync hands an un-`--protect-args`
238 + // remote path to the login shell on the far side, so the wildcard must
239 + // reach it intact — hence no quoting here. rsync exits non-zero when the
240 + // pattern matches nothing, which is the error we want.
241 + let src = format!("{}:{}", self.host.ssh_target(), remote_glob);
242 + let dst = format!("{}/", local_dir.display());
243 + let ssh = Some(self.host.ssh_args());
244 + run_rsync(rsync_command(&src, &dst, ssh, opts), "pull_glob(ssh)").await
245 + }
246 +
198 247 async fn push_dir(&self, local: &Path, remote: &Path, opts: &SyncOpts) -> Result<()> {
199 248 let src = format!("{}/", local.display());
200 249 let dst = format!("{}:{}/", self.host.ssh_target(), remote.display());
@@ -338,6 +387,70 @@ mod tests {
338 387 assert!(!out.join("f.txt").exists(), "pull_file does not flatten; that's pull_dir");
339 388 }
340 389
390 + #[tokio::test]
391 + async fn local_pull_glob_gathers_matches_and_ignores_the_rest() {
392 + let dir = tempfile::tempdir().unwrap();
393 + let src = dir.path().join("bundle");
394 + let out = dir.path().join("dist");
395 + tokio::fs::create_dir_all(&src).await.unwrap();
396 + tokio::fs::create_dir_all(&out).await.unwrap();
397 + tokio::fs::write(src.join("a.msi"), b"A").await.unwrap();
398 + tokio::fs::write(src.join("b.msi"), b"B").await.unwrap();
399 + tokio::fs::write(src.join("notes.txt"), b"N").await.unwrap();
400 +
401 + let exec = LocalExec::new(CapabilitySet::default());
402 + let pattern = format!("{}/*.msi", src.display());
403 + exec.pull_glob(&pattern, &out, &SyncOpts::default()).await.unwrap();
404 + assert_eq!(tokio::fs::read(out.join("a.msi")).await.unwrap(), b"A");
405 + assert_eq!(tokio::fs::read(out.join("b.msi")).await.unwrap(), b"B");
406 + assert!(!out.join("notes.txt").exists(), "glob must not gather non-matches");
407 + }
408 +
409 + /// A collect that silently gathers nothing is how an empty release ships.
410 + #[tokio::test]
411 + async fn local_pull_glob_errors_when_nothing_matches() {
412 + let dir = tempfile::tempdir().unwrap();
413 + let out = dir.path().join("dist");
414 + tokio::fs::create_dir_all(&out).await.unwrap();
415 + let exec = LocalExec::new(CapabilitySet::default());
416 + let pattern = format!("{}/nope/*.msi", dir.path().display());
417 + let err = exec.pull_glob(&pattern, &out, &SyncOpts::default()).await.unwrap_err();
418 + assert!(err.to_string().contains("matched no files"), "{err}");
419 + }
420 +
421 + /// An exact path is a glob with no wildcard — recipes resolve globs
422 + /// host-side (`ls -t | head -1`) and pass concrete paths, so this is the
423 + /// common case, not an edge case.
424 + #[tokio::test]
425 + async fn local_pull_glob_accepts_a_concrete_path() {
426 + let dir = tempfile::tempdir().unwrap();
427 + let out = dir.path().join("dist");
428 + tokio::fs::create_dir_all(&out).await.unwrap();
429 + let f = dir.path().join("GoingsOn.dmg");
430 + tokio::fs::write(&f, b"DMG").await.unwrap();
431 + let exec = LocalExec::new(CapabilitySet::default());
432 + exec.pull_glob(&f.to_string_lossy(), &out, &SyncOpts::default()).await.unwrap();
433 + assert_eq!(tokio::fs::read(out.join("GoingsOn.dmg")).await.unwrap(), b"DMG");
434 + }
435 +
436 + #[test]
437 + fn ssh_pull_glob_leaves_the_wildcard_for_the_remote_shell() {
438 + // rsync hands an un---protect-args remote path to the far-side login
439 + // shell; quoting it here would defeat the expansion this depends on.
440 + let host = RemoteHost::new("windows-x86");
441 + let cmd = rsync_command(
442 + &format!("{}:{}", host.ssh_target(), "/c/build/bundle/msi/*.msi"),
443 + "/dist/",
444 + Some(host.ssh_args()),
445 + &SyncOpts::default(),
446 + );
447 + let args = render_args(&cmd);
448 + assert!(
449 + args.iter().any(|a| a == "windows-x86:/c/build/bundle/msi/*.msi"),
450 + "wildcard reaches the remote intact: {args:?}"
451 + );
452 + }
453 +
341 454 #[test]
342 455 fn sync_opts_flags_are_opt_out() {
343 456 // Default = the historical `-az --partial`.