Skip to content
Open
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
98 changes: 98 additions & 0 deletions crates/openshell-cli/src/ssh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,17 @@ const FORWARD_LISTENER_PROBE_INTERVAL: Duration = Duration::from_millis(50);
/// grace period.
const FORWARD_LISTENER_CONNECT_TIMEOUT: Duration = Duration::from_millis(200);

const HOST_TOOL_LINKER_ENV: &[&str] = &[
"DYLD_FALLBACK_LIBRARY_PATH",
"DYLD_INSERT_LIBRARIES",
"DYLD_LIBRARY_PATH",
"LD_AUDIT",
"LD_LIBRARY_PATH",
"LD_PRELOAD",
"LIBRARY_PATH",
"NIX_LD_LIBRARY_PATH",
];

#[derive(Clone, Copy, Debug)]
pub enum Editor {
Vscode,
Expand Down Expand Up @@ -130,6 +141,7 @@ async fn ssh_session_config(
&session.token,
gateway_name,
);
let proxy_command = proxy_command_with_preserved_environment(proxy_command);

Ok(SshSessionConfig {
proxy_command,
Expand All @@ -146,6 +158,7 @@ fn ssh_base_command(proxy_command: &str) -> Command {
std::env::var("OPENSHELL_SSH_LOG_LEVEL").unwrap_or_else(|_| "ERROR".to_string());

let mut command = Command::new("ssh");
sanitize_host_tool_environment(&mut command);
command
.arg("-o")
.arg(format!("ProxyCommand={proxy_command}"))
Expand All @@ -168,6 +181,30 @@ fn ssh_base_command(proxy_command: &str) -> Command {
command
}

fn sanitize_host_tool_environment(command: &mut Command) {
for key in HOST_TOOL_LINKER_ENV {
command.env_remove(key);
}
}

fn proxy_command_with_preserved_environment(proxy_command: String) -> String {
let assignments = HOST_TOOL_LINKER_ENV
.iter()
.filter_map(|key| {
std::env::var_os(key).map(|value| {
let value = value.to_string_lossy();
format!("{key}={}", shell_escape(&value))
})
})
.collect::<Vec<_>>();

if assignments.is_empty() {
proxy_command
} else {
format!("env {} {proxy_command}", assignments.join(" "))
}
}

#[cfg(unix)]
const TRANSIENT_TTY_SIGNALS: &[Signal] = &[Signal::SIGINT, Signal::SIGQUIT, Signal::SIGTERM];

Expand Down Expand Up @@ -1633,6 +1670,67 @@ mod tests {
use super::*;
use crate::TEST_ENV_LOCK;

#[test]
fn ssh_base_command_removes_host_linker_environment() {
let command = ssh_base_command("openshell ssh-proxy");
let removed_keys = command
.get_envs()
.filter(|(_, value)| value.is_none())
.map(|(key, _)| key.to_string_lossy().into_owned())
.collect::<Vec<_>>();

for key in HOST_TOOL_LINKER_ENV {
assert!(
removed_keys.iter().any(|removed| removed == key),
"expected ssh command to remove {key}"
);
}
}

#[test]
fn proxy_command_preserves_linker_environment_for_proxy_child() {
let env = HOST_TOOL_LINKER_ENV
.iter()
.map(|key| {
(
*key,
(*key == "LD_LIBRARY_PATH").then_some("/nix/store/z3 lib:/opt/lib"),
)
})
.collect::<Vec<_>>();
temp_env::with_vars(env, || {
let proxy_command =
proxy_command_with_preserved_environment("openshell ssh-proxy".to_string());

assert!(
proxy_command.contains("LD_LIBRARY_PATH='/nix/store/z3 lib:/opt/lib'"),
"unexpected proxy command: {proxy_command}"
);
assert!(
proxy_command.starts_with("env "),
"unexpected proxy command: {proxy_command}"
);
assert!(
proxy_command.ends_with(" openshell ssh-proxy"),
"unexpected proxy command: {proxy_command}"
);
});
}

#[test]
fn proxy_command_is_unchanged_without_linker_environment() {
let env = HOST_TOOL_LINKER_ENV
.iter()
.map(|key| (*key, None::<&str>))
.collect::<Vec<_>>();
temp_env::with_vars(env, || {
let proxy_command =
proxy_command_with_preserved_environment("openshell ssh-proxy".to_string());

assert_eq!(proxy_command, "openshell ssh-proxy");
});
}

#[test]
fn upsert_host_block_appends_when_missing() {
let input = "Host existing\n HostName example.com\n";
Expand Down
9 changes: 5 additions & 4 deletions crates/openshell-driver-docker/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,11 @@ The Docker driver bind-mounts a host-side Linux `openshell-sandbox` binary into
each sandbox container. Resolution order is:

1. `supervisor_bin` in `[openshell.drivers.docker]`.
2. A sibling `openshell-sandbox` next to the running `openshell-gateway` binary.
3. A local Linux cargo target build for the Docker daemon architecture.
4. `supervisor_image` in `[openshell.drivers.docker]`, or the
release-matched default supervisor image, extracting `/openshell-sandbox`.
2. `supervisor_image` in `[openshell.drivers.docker]`, extracting
`/openshell-sandbox` from that image.
3. A sibling `openshell-sandbox` next to the running `openshell-gateway` binary.
4. A local Linux cargo target build for the Docker daemon architecture.
5. The release-matched default supervisor image, extracting `/openshell-sandbox`.

Release and Docker-image gateway builds bake the matching supervisor image tag
into the binary at compile time. The default Docker supervisor image is not
Expand Down
103 changes: 68 additions & 35 deletions crates/openshell-driver-docker/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ const DOCKER_NETWORK_DRIVER: &str = "bridge";

/// Default image holding the Linux `openshell-sandbox` binary. The gateway
/// pulls this image and extracts the binary to a host-side cache when no
/// explicit `supervisor_bin` override or local build is available.
/// explicit `supervisor_bin`, configured `supervisor_image`, sibling binary,
/// or local build is available.
const DEFAULT_DOCKER_SUPERVISOR_IMAGE_REPO: &str = "ghcr.io/nvidia/openshell/supervisor";

/// Return the default `ghcr.io/nvidia/openshell/supervisor:<tag>` reference
Expand Down Expand Up @@ -155,10 +156,9 @@ pub struct DockerComputeConfig {
/// Optional override for the Linux `openshell-sandbox` binary mounted into containers.
pub supervisor_bin: Option<PathBuf>,

/// Optional override for the image the gateway pulls to extract the
/// Linux `openshell-sandbox` binary when no explicit binary path or
/// local build is available. Defaults to
/// `ghcr.io/nvidia/openshell/supervisor:<gateway-image-tag>`.
/// Optional image used to extract the Linux `openshell-sandbox` binary.
/// Ignored when `supervisor_bin` is set. See `resolve_supervisor_bin` for
/// the full resolution order.
pub supervisor_image: Option<String>,

/// Host-side CA certificate for Docker sandbox mTLS.
Expand Down Expand Up @@ -2948,56 +2948,89 @@ fn normalize_docker_arch(arch: &str) -> String {
}
}

pub(crate) async fn resolve_supervisor_bin(
docker: &Docker,
#[derive(Debug, Eq, PartialEq)]
enum SupervisorBinSource {
Binary(PathBuf),
Image(String),
}

fn resolve_supervisor_bin_source(
docker_config: &DockerComputeConfig,
daemon_arch: &str,
) -> CoreResult<PathBuf> {
current_exe: Option<&Path>,
target_candidates: &[PathBuf],
) -> CoreResult<SupervisorBinSource> {
// Tier 1: explicit supervisor_bin in [openshell.drivers.docker].
if let Some(path) = docker_config.supervisor_bin.clone() {
let path = canonicalize_existing_file(&path, "docker supervisor binary")?;
validate_linux_elf_binary(&path)?;
return Ok(path);
return Ok(SupervisorBinSource::Binary(path));
}

// Tier 2: explicit supervisor_image in [openshell.drivers.docker].
// A configured image should be the source of truth even when a local
// developer build is present under target/.
if let Some(image) = docker_config.supervisor_image.clone() {
return Ok(SupervisorBinSource::Image(image));
}
Comment on lines +2969 to +2974

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a comment here:

/// Optional override for the image the gateway pulls to extract the
/// Linux `openshell-sandbox` binary when no explicit binary path or
/// local build is available. Defaults to
/// `ghcr.io/nvidia/openshell/supervisor:<gateway-image-tag>`.
pub supervisor_image: Option<String>,
that should be updated after changing order in which the supervisor bin is resolved.

Could be something like:

/// Optional image used to extract the Linux `openshell-sandbox` binary.
/// Ignored when `supervisor_bin` is set. See `resolve_supervisor_bin` for
/// the full resolution order.
pub supervisor_image: Option<String>,

It could make sense to validate the config and only allow either supervisor_bin or supervisor_image, so both cannot be set. But that would be a breaking change.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, it would be good to have a test for this new resolution order.

@elezar elezar Jun 23, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could make sense to validate the config and only allow either supervisor_bin or supervisor_image, so both cannot be set. But that would be a breaking change.

If we think that will be clearer, then breaking now probably makes sense. Having these settings be exclusive does reduce the questions around resolution precedence.

I don't want to do that in this PR though. If we think that's a direction we want to go, then let's make that change in a separate PR so that the possibly breaking change is more clearly surfaced.


// Tier 2: sibling `openshell-sandbox` next to the running gateway
// Tier 3: sibling `openshell-sandbox` next to the running gateway
// (release artifact layout). Linux-only because the sibling must be a
// Linux ELF to bind-mount into a Linux container.
if cfg!(target_os = "linux") {
let current_exe = std::env::current_exe()
.map_err(|err| Error::config(format!("failed to resolve current executable: {err}")))?;
if let Some(parent) = current_exe.parent() {
let sibling = parent.join("openshell-sandbox");
if sibling.is_file() {
let path = canonicalize_existing_file(&sibling, "docker supervisor binary")?;
if validate_linux_elf_binary(&path).is_ok() {
return Ok(path);
}
if cfg!(target_os = "linux")
&& let Some(current_exe) = current_exe
&& let Some(parent) = current_exe.parent()
{
let sibling = parent.join("openshell-sandbox");
if sibling.is_file() {
let path = canonicalize_existing_file(&sibling, "docker supervisor binary")?;
if validate_linux_elf_binary(&path).is_ok() {
return Ok(SupervisorBinSource::Binary(path));
}
}
}

// Tier 3: local cargo target build (developer workflow). Preferred
// over a registry pull when available because it matches whatever the
// developer just built.
let target_candidates = linux_supervisor_candidates(daemon_arch);
for candidate in &target_candidates {
// Tier 4: local cargo target build (developer workflow). Preferred
// over the default registry image when available because it matches
// whatever the developer just built.
for candidate in target_candidates {
if candidate.is_file() {
let path = canonicalize_existing_file(candidate, "docker supervisor binary")?;
if validate_linux_elf_binary(&path).is_ok() {
return Ok(path);
return Ok(SupervisorBinSource::Binary(path));
}
}
}

// Tier 4: pull the supervisor image from a registry and extract the
// binary to a host-side cache keyed by image content digest. This is
// the default path for released gateway binaries.
let image = docker_config
.supervisor_image
.clone()
.unwrap_or_else(default_docker_supervisor_image);
extract_supervisor_bin_from_image(docker, &image).await
// Tier 5: pull the release-matched default supervisor image and extract
// the binary to a host-side cache keyed by image content digest.
Ok(SupervisorBinSource::Image(default_docker_supervisor_image()))
}

pub(crate) async fn resolve_supervisor_bin(
docker: &Docker,
docker_config: &DockerComputeConfig,
daemon_arch: &str,
) -> CoreResult<PathBuf> {
let current_exe =
if cfg!(target_os = "linux")
&& docker_config.supervisor_bin.is_none()
&& docker_config.supervisor_image.is_none()
{
Some(std::env::current_exe().map_err(|err| {
Error::config(format!("failed to resolve current executable: {err}"))
})?)
} else {
None
};
let target_candidates = linux_supervisor_candidates(daemon_arch);

match resolve_supervisor_bin_source(docker_config, current_exe.as_deref(), &target_candidates)?
{
SupervisorBinSource::Binary(path) => Ok(path),
SupervisorBinSource::Image(image) => {
extract_supervisor_bin_from_image(docker, &image).await
}
}
}

fn linux_supervisor_candidates(daemon_arch: &str) -> Vec<PathBuf> {
Expand Down
30 changes: 30 additions & 0 deletions crates/openshell-driver-docker/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1711,6 +1711,36 @@ fn default_docker_supervisor_image_uses_nvidia_ghcr_repo() {
);
}

#[test]
fn configured_supervisor_image_takes_precedence_over_local_binaries() {
let tempdir = TempDir::new().unwrap();
let bin_dir = tempdir.path().join("bin");
fs::create_dir_all(&bin_dir).unwrap();
let current_exe = bin_dir.join("openshell-gateway");
let sibling = bin_dir.join("openshell-sandbox");
fs::write(&current_exe, b"gateway").unwrap();
fs::write(&sibling, b"\x7fELFsibling").unwrap();

let local_build = tempdir.path().join("target/openshell-sandbox");
fs::create_dir_all(local_build.parent().unwrap()).unwrap();
fs::write(&local_build, b"\x7fELFlocal").unwrap();

let source = resolve_supervisor_bin_source(
&DockerComputeConfig {
supervisor_image: Some("example.com/openshell/supervisor:test".to_string()),
..Default::default()
},
Some(&current_exe),
&[local_build],
)
.unwrap();

assert_eq!(
source,
SupervisorBinSource::Image("example.com/openshell/supervisor:test".to_string())
);
}

#[test]
fn docker_supervisor_image_tag_prefers_explicit_build_tags() {
assert_eq!(
Expand Down
1 change: 1 addition & 0 deletions docs/reference/gateway-config.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ sandbox_namespace = "docker-dev"
grpc_endpoint = "https://host.openshell.internal:17670"
# Skip the image-pull-and-extract step by pointing at a locally built binary.
supervisor_bin = "/usr/local/libexec/openshell/openshell-sandbox"
# When supervisor_bin is omitted, Docker extracts /openshell-sandbox from this image.
supervisor_image = "ghcr.io/nvidia/openshell/supervisor:latest"
guest_tls_ca = "/etc/openshell/certs/ca.pem"
guest_tls_cert = "/etc/openshell/certs/client.pem"
Expand Down
Loading
Loading