You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The exec_sandbox gRPC handler converts structured ExecSandboxRequest fields (command array, environment map, workdir) into a single shell string via build_remote_exec_command, which is then passed through SSH to bash -lc on the sandbox side. This structured-to-unstructured conversion is architecturally weaker than direct structured execution, even with the defensive hardening applied in PR #548 (SEC-002).
The current hardening (null byte / newline rejection, size limits, shell_escape) mitigates practical exploitation, but the fundamental pattern of flattening structured data into a shell string and re-parsing it remains a latent risk surface.
Proposed Design
Replace the shell string protocol with a structured command protocol:
Server side (grpc.rs): Instead of calling build_remote_exec_command to flatten the request into a shell string, serialize the ExecSandboxRequest fields (command array, environment map, workdir) as JSON or protobuf.
Transport: Send the structured payload over the SSH channel (e.g., as a JSON blob followed by a newline, or via a dedicated protobuf message).
Sandbox side (ssh.rs): Parse the structured payload and execute the command using execvp (or Command::new in Rust) directly — no shell interpretation at all. Environment variables are set via the process builder, and workdir is applied via std::env::set_current_dir or Command::current_dir.
This eliminates both shell interpretation layers entirely.
Key considerations
Backward compatibility: the sandbox SSH server needs to support both the old shell string protocol and the new structured protocol during a transition period (version negotiation via the NSSH1 preface, or a separate channel type).
Problem Statement
The
exec_sandboxgRPC handler converts structuredExecSandboxRequestfields (command array, environment map, workdir) into a single shell string viabuild_remote_exec_command, which is then passed through SSH tobash -lcon the sandbox side. This structured-to-unstructured conversion is architecturally weaker than direct structured execution, even with the defensive hardening applied in PR #548 (SEC-002).The current hardening (null byte / newline rejection, size limits,
shell_escape) mitigates practical exploitation, but the fundamental pattern of flattening structured data into a shell string and re-parsing it remains a latent risk surface.Proposed Design
Replace the shell string protocol with a structured command protocol:
grpc.rs): Instead of callingbuild_remote_exec_commandto flatten the request into a shell string, serialize theExecSandboxRequestfields (command array, environment map, workdir) as JSON or protobuf.ssh.rs): Parse the structured payload and execute the command usingexecvp(orCommand::newin Rust) directly — no shell interpretation at all. Environment variables are set via the process builder, andworkdiris applied viastd::env::set_current_dirorCommand::current_dir.This eliminates both shell interpretation layers entirely.
Key considerations
shell_escapeandbuild_remote_exec_commandhardening from PR fix: security hardening batch 1 (SEC-002 through SEC-010) #548 should remain as a fallback path until the transition is complete.Alternatives Considered
envrequests, but many SSH server configs restrict which env vars can be set. Not reliable.Agent Investigation
Codebase analysis from the SEC-002 security review (PR #548):
build_remote_exec_commandatcrates/openshell-server/src/grpc.rs:3424shell_escapeatcrates/openshell-server/src/grpc.rs:3410run_exec_with_russhatcrates/openshell-server/src/grpc.rs:3733sends command viachannel.exec(true, command.as_bytes())crates/openshell-sandbox/src/ssh.rs