Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 75 additions & 5 deletions crates/buzz-provider-deploy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,37 @@
//! desktop resolves `~/.buzz`; a headless daemon may have none) and, for a
//! caller acting on a signed launch bundle, pinning the expected binary
//! digest via [`provider_deploy_pinned`] — see that function's doc for why.
//!
//! # The child's environment
//!
//! Every entry point takes an `env: Option<&HashMap<String, String>>`. `None`
//! (every caller before `buzz-waker`'s dynamic multi-tenant enrolment)
//! leaves the child's environment exactly as `std::process::Command`
//! defaults it — the parent process's own environment, untouched — so this
//! parameter changed no existing caller's behavior when it was added.
//!
//! **`Some(map)` is isolation, not an overlay.** The child's environment is
//! cleared ([`std::process::Command::env_clear`]), then
//! [`tenant_child_environment_baseline`] is applied (today: `HOME` — required
//! by `buzz-backend-sprites::credentials::resolve` before it even checks
//! `SPRITE_TOKEN`, for its keychain fallback and `~/.sprites` metadata dir —
//! and `PATH`, which that same binary's own provisioning code needs to
//! resolve `bash` by relative name), then `map`'s keys are layered on top,
//! overriding any baseline variable with the same name. This stops the
//! child from also seeing whatever else the daemon's own environment
//! carries (`WAKER_IDENTITY_NSEC`, another tenant's already-resolved
//! provider credential, proxy/TLS controls) — the gap the original overlay-
//! only version of this parameter left open, since in `buzz-waker`'s
//! multi-tenant model the tenant's own bundle authorizes which provider
//! binary/digest runs, so an unisolated child could otherwise read and
//! exfiltrate secrets across the shared-daemon boundary.
//!
//! `TENANT_BASELINE_VARS` is deliberately small and reviewed per addition,
//! not "whatever the parent happens to have" — growing it back to the full
//! parent environment would silently undo the isolation this exists for.

use sha2::{Digest, Sha256};
use std::collections::HashMap;
use std::io::{BufReader, Read, Write};
use std::path::{Path, PathBuf};
use std::sync::mpsc;
Expand All @@ -29,6 +58,27 @@ const STDERR_CAP: usize = 65536;
const STDOUT_CAP: usize = 1_048_576; // 1 MB
const PROVIDER_PROTOCOL_VERSION: u64 = 1;

/// Variables kept from this process's own environment when a tenant-scoped
/// `env` overlay is supplied — see the module doc's The child's environment
/// section for why each one is here. Provider-agnostic on purpose: this
/// crate has no notion of which provider binary it is invoking, so it keeps
/// the same small baseline regardless of provider, rather than branching on
/// provider identity.
const TENANT_BASELINE_VARS: &[&str] = &["HOME", "PATH"];

/// Build the fixed baseline applied under a tenant-scoped `env` overlay,
/// read fresh from this process's own environment (never from `env` itself).
fn tenant_child_environment_baseline() -> HashMap<String, String> {
TENANT_BASELINE_VARS
.iter()
.filter_map(|&key| {
std::env::var(key)
.ok()
.map(|value| (key.to_string(), value))
})
.collect()
}

/// On Windows, a console-subsystem child gets a fresh, briefly-visible
/// console window per invocation unless `CREATE_NO_WINDOW` is set. A pure
/// no-op on non-Windows platforms, so callers can call this unconditionally.
Expand Down Expand Up @@ -102,7 +152,8 @@ fn validate_provider_info(info: &serde_json::Value) -> Result<(), String> {
/// `workdir` is the child's working directory (`None` inherits the caller's
/// own CWD) — callers with a notion of a stable agent home (the desktop's
/// `~/.buzz`) should resolve and pass it; a caller without one may pass
/// `None`.
/// `None`. `Some` isolates the child's environment — see the module doc's
/// The child's environment section.
///
/// Reader threads stream lines/chunks over channels so the caller can receive
/// data as it arrives and time-box the wait. No `read_to_end` — if a provider
Expand All @@ -119,6 +170,7 @@ pub fn invoke_provider(
request: &serde_json::Value,
timeout: Duration,
workdir: Option<&Path>,
env: Option<&HashMap<String, String>>,
) -> Result<serde_json::Value, String> {
let request_bytes = format!(
"{}\n",
Expand All @@ -129,6 +181,11 @@ pub fn invoke_provider(
if let Some(workdir) = workdir {
cmd.current_dir(workdir);
}
if let Some(env) = env {
cmd.env_clear();
cmd.envs(tenant_child_environment_baseline());
cmd.envs(env);
}
configure_no_window(&mut cmd);
let mut child = cmd
.stdin(std::process::Stdio::piped())
Expand Down Expand Up @@ -563,7 +620,8 @@ fn stage_provider(
/// Deploy through one immutable staged copy: negotiate protocol v1 before the
/// secret-bearing request, then invoke deploy on those exact same bytes.
///
/// `workdir` is passed straight through to [`invoke_provider`] — see its doc.
/// `workdir` and `env` are passed straight through to [`invoke_provider`] —
/// see its doc.
///
/// # Errors
/// See [`invoke_provider`] and [`stage_provider`] — every path returns a
Expand All @@ -573,8 +631,9 @@ pub fn provider_deploy(
agent: &serde_json::Value,
provider_config: &serde_json::Value,
workdir: Option<&Path>,
env: Option<&HashMap<String, String>>,
) -> Result<ProviderDeployOutcome, String> {
deploy(binary, agent, provider_config, workdir, None)
deploy(binary, agent, provider_config, workdir, env, None)
}

/// Like [`provider_deploy`], but refuses to run unless the staged binary's
Expand All @@ -595,27 +654,32 @@ pub fn provider_deploy(
/// # Errors
/// A digest mismatch is reported before any process is spawned. Otherwise
/// see [`provider_deploy`].
#[allow(clippy::too_many_arguments)]
pub fn provider_deploy_pinned(
binary: &Path,
agent: &serde_json::Value,
provider_config: &serde_json::Value,
workdir: Option<&Path>,
env: Option<&HashMap<String, String>>,
expected_sha256_hex: &str,
) -> Result<ProviderDeployOutcome, String> {
deploy(
binary,
agent,
provider_config,
workdir,
env,
Some(expected_sha256_hex),
)
}

#[allow(clippy::too_many_arguments)]
fn deploy(
binary: &Path,
agent: &serde_json::Value,
provider_config: &serde_json::Value,
workdir: Option<&Path>,
env: Option<&HashMap<String, String>>,
expected_sha256_hex: Option<&str>,
) -> Result<ProviderDeployOutcome, String> {
let (_directory, staged, digest, _execution_guard) = stage_provider(binary)?;
Expand All @@ -633,7 +697,13 @@ fn deploy(
"op": "info",
"request_id": uuid::Uuid::new_v4().to_string(),
});
let info = invoke_provider(&staged, &info_request, Duration::from_secs(10), workdir)?;
let info = invoke_provider(
&staged,
&info_request,
Duration::from_secs(10),
workdir,
env,
)?;
validate_provider_info(&info)?;

let request = serde_json::json!({
Expand All @@ -642,7 +712,7 @@ fn deploy(
"agent": agent,
"provider_config": provider_config,
});
let resp = invoke_provider(&staged, &request, Duration::from_secs(600), workdir)?;
let resp = invoke_provider(&staged, &request, Duration::from_secs(600), workdir, env)?;
let agent_id = resp["agent_id"]
.as_str()
.map(String::from)
Expand Down
139 changes: 139 additions & 0 deletions crates/buzz-provider-deploy/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ esac"#,
&serde_json::json!({}),
&serde_json::json!({}),
None,
None,
)
.expect("staged deploy");
assert_eq!(outcome.agent_id, "remote-1");
Expand Down Expand Up @@ -208,6 +209,7 @@ esac"#
&serde_json::json!({}),
&serde_json::json!({}),
None,
None,
)
.expect("staged deploy");
assert_eq!(outcome.fresh_generation, parsed, "wire value {wire_value}");
Expand Down Expand Up @@ -257,6 +259,7 @@ esac"#,
&serde_json::json!({}),
&serde_json::json!({}),
None,
None,
)
.expect("deploy from immutable staged copy")
.agent_id;
Expand Down Expand Up @@ -304,6 +307,7 @@ esac"#,
&serde_json::json!({}),
&serde_json::json!({}),
None,
None,
)
.expect("deploy from immutable staged copy")
.agent_id;
Expand Down Expand Up @@ -342,6 +346,7 @@ esac"#,
&serde_json::json!({"private_key_nsec": "nsec1must-not-cross"}),
&serde_json::json!({}),
None,
None,
)
.unwrap_err();
assert!(error.contains("protocol version 2"), "{error}");
Expand All @@ -365,6 +370,7 @@ printf '%s\n' '{"ok":true,"version":"1.0.0"}'"#,
&serde_json::json!({}),
&serde_json::json!({}),
None,
None,
)
.unwrap_err();
assert!(
Expand All @@ -391,6 +397,7 @@ fn provider_deploy_pinned_refuses_a_digest_mismatch_before_any_negotiation() {
&serde_json::json!({"private_key_nsec": "nsec1must-not-cross"}),
&serde_json::json!({}),
None,
None,
&"f".repeat(64),
)
.unwrap_err();
Expand Down Expand Up @@ -424,12 +431,144 @@ esac"#,
&serde_json::json!({}),
&serde_json::json!({}),
None,
None,
&expected,
)
.expect("digest matched, deploy proceeds");
assert_eq!(outcome.agent_id, "pinned-1");
}

/// The headline case `env` exists for: a caller's overlay must actually
/// reach the child process, not just be accepted and silently dropped.
#[cfg(unix)]
#[test]
fn provider_deploy_env_overlay_reaches_the_child_process() {
let directory = tempfile::tempdir().unwrap();
let provider = directory.path().join("provider");
write_test_provider(
&provider,
r#"read request
case "$request" in
*\"op\":\"info\"*) printf '%s\n' '{"ok":true,"name":"test","version":"1.0.0","protocol_version":1,"description":"test provider","config_schema":{}}' ;;
*\"op\":\"deploy\"*) printf '{"ok":true,"agent_id":"%s"}\n' "$TEST_TENANT_TOKEN" ;;
esac"#,
);

let mut env = std::collections::HashMap::new();
env.insert(
"TEST_TENANT_TOKEN".to_string(),
"sprt-tenant-abc".to_string(),
);

let outcome = provider_deploy(
&provider,
&serde_json::json!({}),
&serde_json::json!({}),
None,
Some(&env),
)
.expect("deploy with an env overlay");
assert_eq!(
outcome.agent_id, "sprt-tenant-abc",
"the child must see the overlaid variable"
);
}

/// `Some(map)` overrides an inherited variable of the same name — the
/// module doc's own contract, and the property `buzz-waker` relies on if a
/// baseline var and a tenant var ever collide.
#[cfg(unix)]
#[test]
fn provider_deploy_env_overlay_overrides_an_inherited_variable() {
let directory = tempfile::tempdir().unwrap();
let provider = directory.path().join("provider");
write_test_provider(
&provider,
r#"read request
case "$request" in
*\"op\":\"info\"*) printf '%s\n' '{"ok":true,"name":"test","version":"1.0.0","protocol_version":1,"description":"test provider","config_schema":{}}' ;;
*\"op\":\"deploy\"*) printf '{"ok":true,"agent_id":"%s"}\n' "$TEST_TENANT_TOKEN" ;;
esac"#,
);

// SAFETY: test-only, single-threaded at this point in the process
// (no other test in this crate reads or writes this exact variable).
unsafe {
std::env::set_var("TEST_TENANT_TOKEN", "inherited-from-parent");
}
let mut env = std::collections::HashMap::new();
env.insert(
"TEST_TENANT_TOKEN".to_string(),
"overlaid-value".to_string(),
);

let outcome = provider_deploy(
&provider,
&serde_json::json!({}),
&serde_json::json!({}),
None,
Some(&env),
)
.expect("deploy with an env overlay");
assert_eq!(outcome.agent_id, "overlaid-value");

unsafe {
std::env::remove_var("TEST_TENANT_TOKEN");
}
}

/// A tenant-scoped `env` overlay must isolate the child from this process's
/// own environment, not merely overlay on top of it — an unrelated inherited
/// variable (standing in for a daemon secret like `WAKER_IDENTITY_NSEC` or
/// another tenant's already-resolved provider credential) must not reach the
/// child, even though it is never mentioned in `env` or in
/// `TENANT_BASELINE_VARS`.
#[cfg(unix)]
#[test]
fn provider_deploy_env_overlay_clears_unrelated_inherited_variables() {
let directory = tempfile::tempdir().unwrap();
let provider = directory.path().join("provider");
write_test_provider(
&provider,
r#"read request
case "$request" in
*\"op\":\"info\"*) printf '%s\n' '{"ok":true,"name":"test","version":"1.0.0","protocol_version":1,"description":"test provider","config_schema":{}}' ;;
*\"op\":\"deploy\"*)
if [ -z "${TEST_SHOULD_NOT_LEAK:-}" ]; then
printf '{"ok":true,"agent_id":"cleared"}\n'
else
printf '{"ok":true,"agent_id":"leaked"}\n'
fi
;;
esac"#,
);

// SAFETY: test-only, single-threaded at this point in the process
// (no other test in this crate reads or writes this exact variable).
unsafe {
std::env::set_var("TEST_SHOULD_NOT_LEAK", "daemon-secret");
}
let env = std::collections::HashMap::new();

let outcome = provider_deploy(
&provider,
&serde_json::json!({}),
&serde_json::json!({}),
None,
Some(&env),
)
.expect("deploy with an empty tenant env overlay");
assert_eq!(
outcome.agent_id, "cleared",
"a variable inherited from this process but absent from both the tenant overlay \
and TENANT_BASELINE_VARS must not reach the child"
);

unsafe {
std::env::remove_var("TEST_SHOULD_NOT_LEAK");
}
}

#[test]
fn provider_info_requires_the_complete_flat_wire_shape() {
let complete = serde_json::json!({
Expand Down
Loading
Loading