Skip to content
Draft
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
1 change: 0 additions & 1 deletion .agents/skills/debug-openshell-cluster/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ Check required Helm deployment secrets:

```bash
kubectl -n openshell get secret \
openshell-ssh-handshake \
openshell-server-tls \
openshell-server-client-ca \
openshell-client-tls
Expand Down
31 changes: 0 additions & 31 deletions crates/openshell-core/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ pub const DEFAULT_SERVER_PORT: u16 = 8080;
/// Default container stop timeout in seconds (SIGTERM → SIGKILL).
pub const DEFAULT_STOP_TIMEOUT_SECS: u32 = 10;

/// Default allowed clock skew for SSH handshake validation, in seconds.
pub const DEFAULT_SSH_HANDSHAKE_SKEW_SECS: u64 = 300;

/// Default Podman bridge network name.
pub const DEFAULT_NETWORK_NAME: &str = "openshell";

Expand Down Expand Up @@ -273,14 +270,6 @@ pub struct Config {
#[serde(default = "default_sandbox_ssh_socket_path")]
pub sandbox_ssh_socket_path: String,

/// Shared secret for gateway-to-sandbox SSH handshake.
#[serde(default)]
pub ssh_handshake_secret: String,

/// Allowed clock skew for SSH handshake validation, in seconds.
#[serde(default = "default_ssh_handshake_skew_secs")]
pub ssh_handshake_skew_secs: u64,

/// TTL for SSH session tokens, in seconds. 0 disables expiry.
#[serde(default = "default_ssh_session_ttl_secs")]
pub ssh_session_ttl_secs: u64,
Expand Down Expand Up @@ -413,8 +402,6 @@ impl Config {
ssh_connect_path: default_ssh_connect_path(),
sandbox_ssh_port: default_sandbox_ssh_port(),
sandbox_ssh_socket_path: default_sandbox_ssh_socket_path(),
ssh_handshake_secret: String::new(),
ssh_handshake_skew_secs: default_ssh_handshake_skew_secs(),
ssh_session_ttl_secs: default_ssh_session_ttl_secs(),
client_tls_secret_name: String::new(),
host_gateway_ip: String::new(),
Expand Down Expand Up @@ -534,20 +521,6 @@ impl Config {
self
}

/// Create a new configuration with the SSH handshake secret.
#[must_use]
pub fn with_ssh_handshake_secret(mut self, secret: impl Into<String>) -> Self {
self.ssh_handshake_secret = secret.into();
self
}

/// Create a new configuration with SSH handshake skew allowance.
#[must_use]
pub const fn with_ssh_handshake_skew_secs(mut self, secs: u64) -> Self {
self.ssh_handshake_skew_secs = secs;
self
}

/// Create a new configuration with the SSH session TTL.
#[must_use]
pub const fn with_ssh_session_ttl_secs(mut self, secs: u64) -> Self {
Expand Down Expand Up @@ -613,10 +586,6 @@ const fn default_sandbox_ssh_port() -> u16 {
DEFAULT_SSH_PORT
}

const fn default_ssh_handshake_skew_secs() -> u64 {
DEFAULT_SSH_HANDSHAKE_SKEW_SECS
}

const fn default_ssh_session_ttl_secs() -> u64 {
86400 // 24 hours
}
Expand Down
2 changes: 0 additions & 2 deletions crates/openshell-driver-kubernetes/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ pub struct KubernetesComputeConfig {
pub supervisor_image_pull_policy: String,
pub grpc_endpoint: String,
pub ssh_socket_path: String,
pub ssh_handshake_secret: String,
pub ssh_handshake_skew_secs: u64,
pub client_tls_secret_name: String,
pub host_gateway_ip: String,
pub enable_user_namespaces: bool,
Expand Down
53 changes: 11 additions & 42 deletions crates/openshell-driver-kubernetes/src/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,6 @@ impl KubernetesComputeDriver {
&self.config.ssh_socket_path
}

pub const fn ssh_handshake_skew_secs(&self) -> u64 {
self.config.ssh_handshake_skew_secs
}

fn watch_api(&self) -> Api<DynamicObject> {
let gvk = GroupVersionKind::gvk(SANDBOX_GROUP, SANDBOX_VERSION, SANDBOX_KIND);
let resource = ApiResource::from_gvk(&gvk);
Expand Down Expand Up @@ -286,10 +282,6 @@ impl KubernetesComputeDriver {
}
}

fn ssh_handshake_secret(&self) -> &str {
&self.config.ssh_handshake_secret
}

pub async fn create_sandbox(&self, sandbox: &Sandbox) -> Result<(), KubernetesDriverError> {
let name = sandbox.name.as_str();
info!(
Expand Down Expand Up @@ -317,8 +309,6 @@ impl KubernetesComputeDriver {
sandbox_name: &sandbox.name,
grpc_endpoint: &self.config.grpc_endpoint,
ssh_socket_path: self.ssh_socket_path(),
ssh_handshake_secret: self.ssh_handshake_secret(),
ssh_handshake_skew_secs: self.ssh_handshake_skew_secs(),
client_tls_secret_name: &self.config.client_tls_secret_name,
host_gateway_ip: &self.config.host_gateway_ip,
enable_user_namespaces: self.config.enable_user_namespaces,
Expand Down Expand Up @@ -938,8 +928,6 @@ struct SandboxPodParams<'a> {
sandbox_name: &'a str,
grpc_endpoint: &'a str,
ssh_socket_path: &'a str,
ssh_handshake_secret: &'a str,
ssh_handshake_skew_secs: u64,
client_tls_secret_name: &'a str,
host_gateway_ip: &'a str,
enable_user_namespaces: bool,
Expand Down Expand Up @@ -1091,8 +1079,6 @@ fn sandbox_template_to_k8s(
params.sandbox_name,
params.grpc_endpoint,
params.ssh_socket_path,
params.ssh_handshake_secret,
params.ssh_handshake_skew_secs,
!params.client_tls_secret_name.is_empty(),
);

Expand Down Expand Up @@ -1245,8 +1231,6 @@ fn build_env_list(
sandbox_name: &str,
grpc_endpoint: &str,
ssh_socket_path: &str,
ssh_handshake_secret: &str,
ssh_handshake_skew_secs: u64,
tls_enabled: bool,
) -> Vec<serde_json::Value> {
let mut env = existing_env.cloned().unwrap_or_default();
Expand All @@ -1258,8 +1242,6 @@ fn build_env_list(
sandbox_name,
grpc_endpoint,
ssh_socket_path,
ssh_handshake_secret,
ssh_handshake_skew_secs,
tls_enabled,
);
env
Expand All @@ -1276,15 +1258,12 @@ fn apply_env_map(

// Required env vars are passed individually for clarity at call sites; grouping into a struct
// would not improve readability for this internal helper.
#[allow(clippy::too_many_arguments)]
fn apply_required_env(
env: &mut Vec<serde_json::Value>,
sandbox_id: &str,
sandbox_name: &str,
grpc_endpoint: &str,
ssh_socket_path: &str,
ssh_handshake_secret: &str,
ssh_handshake_skew_secs: u64,
tls_enabled: bool,
) {
upsert_env(env, "OPENSHELL_SANDBOX_ID", sandbox_id);
Expand All @@ -1294,12 +1273,6 @@ fn apply_required_env(
if !ssh_socket_path.is_empty() {
upsert_env(env, "OPENSHELL_SSH_SOCKET_PATH", ssh_socket_path);
}
upsert_env(env, "OPENSHELL_SSH_HANDSHAKE_SECRET", ssh_handshake_secret);
upsert_env(
env,
"OPENSHELL_SSH_HANDSHAKE_SKEW_SECS",
&ssh_handshake_skew_secs.to_string(),
);
// TLS cert paths for sandbox-to-server mTLS. Only set when TLS is enabled
// and the client TLS secret is mounted into the sandbox pod.
if tls_enabled {
Expand Down Expand Up @@ -1454,29 +1427,27 @@ mod tests {
use prost_types::{Struct, Value, value::Kind};

#[test]
fn apply_required_env_always_injects_ssh_handshake_secret() {
fn apply_required_env_does_not_inject_handshake_env() {
let mut env = Vec::new();
apply_required_env(
&mut env,
"sandbox-1",
"my-sandbox",
"https://endpoint:8080",
"0.0.0.0:2222",
"my-secret-value",
300,
true,
);

let secret_entry = env
.iter()
.find(|e| {
e.get("name").and_then(|v| v.as_str()) == Some("OPENSHELL_SSH_HANDSHAKE_SECRET")
})
.expect("OPENSHELL_SSH_HANDSHAKE_SECRET must be present in env");
assert_eq!(
secret_entry.get("value").and_then(|v| v.as_str()),
Some("my-secret-value")
);
for name in [
"OPENSHELL_SSH_HANDSHAKE_SECRET",
"OPENSHELL_SSH_HANDSHAKE_SKEW_SECS",
] {
assert!(
!env.iter()
.any(|e| e.get("name").and_then(|v| v.as_str()) == Some(name)),
"{name} must not appear in sandbox pod env after handshake-secret removal"
);
}
}

#[test]
Expand Down Expand Up @@ -1614,8 +1585,6 @@ mod tests {
"my-sandbox",
"https://endpoint:8080",
"0.0.0.0:2222",
"secret",
300,
true, // tls_enabled
);

Expand Down
8 changes: 0 additions & 8 deletions crates/openshell-driver-kubernetes/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,6 @@ struct Args {
)]
sandbox_ssh_socket_path: String,

#[arg(long, env = "OPENSHELL_SSH_HANDSHAKE_SECRET")]
ssh_handshake_secret: String,

#[arg(long, env = "OPENSHELL_SSH_HANDSHAKE_SKEW_SECS", default_value_t = 300)]
ssh_handshake_skew_secs: u64,

#[arg(long, env = "OPENSHELL_CLIENT_TLS_SECRET_NAME")]
client_tls_secret_name: Option<String>,

Expand Down Expand Up @@ -87,8 +81,6 @@ async fn main() -> Result<()> {
supervisor_image_pull_policy: args.supervisor_image_pull_policy.unwrap_or_default(),
grpc_endpoint: args.grpc_endpoint.unwrap_or_default(),
ssh_socket_path: args.sandbox_ssh_socket_path,
ssh_handshake_secret: args.ssh_handshake_secret,
ssh_handshake_skew_secs: args.ssh_handshake_skew_secs,
client_tls_secret_name: args.client_tls_secret_name.unwrap_or_default(),
host_gateway_ip: args.host_gateway_ip.unwrap_or_default(),
enable_user_namespaces: args.enable_user_namespaces,
Expand Down
15 changes: 3 additions & 12 deletions crates/openshell-driver-podman/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -214,13 +214,9 @@ via sandbox templates:
- `OPENSHELL_SANDBOX_ID`
- `OPENSHELL_ENDPOINT`
- `OPENSHELL_SSH_SOCKET_PATH`
- `OPENSHELL_SSH_HANDSHAKE_SKEW_SECS`
- `OPENSHELL_CONTAINER_IMAGE`
- `OPENSHELL_SANDBOX_COMMAND`

The `PodmanComputeConfig::Debug` implementation redacts the handshake secret as
`[REDACTED]`.

## Sandbox Lifecycle

### Creation Flow
Expand All @@ -238,18 +234,15 @@ sequenceDiagram
D->>P: pull_image(supervisor, "missing")
D->>P: pull_image(sandbox_image, policy)

D->>P: create_secret(handshake)
Note over D: On failure below, rollback secret

D->>P: create_volume(workspace)
Note over D: On failure below, rollback volume + secret
Note over D: On failure below, rollback volume

D->>P: create_container(spec)
alt Conflict (409)
D->>P: remove_volume + remove_secret
D->>P: remove_volume
D-->>GW: AlreadyExists
end
Note over D: On failure below, rollback container + volume + secret
Note over D: On failure below, rollback container + volume

D->>P: start_container
D-->>GW: Ok
Expand Down Expand Up @@ -299,8 +292,6 @@ Podman resources after out-of-band container removal or label drift.
| `OPENSHELL_GATEWAY_PORT` | `--gateway-port` | `8080` | Gateway port used for endpoint auto-detection by the standalone binary. |
| `OPENSHELL_NETWORK_NAME` | `--network-name` | `openshell` | Podman bridge network name. |
| `OPENSHELL_SANDBOX_SSH_PORT` | `--sandbox-ssh-port` | `2222` | SSH compatibility port inside the container. |
| `OPENSHELL_SSH_HANDSHAKE_SECRET` | `--ssh-handshake-secret` | Required standalone, gateway-generated in-process | Shared secret for the NSSH1 handshake. |
| `OPENSHELL_SSH_HANDSHAKE_SKEW_SECS` | `--ssh-handshake-skew-secs` | `300` | Allowed timestamp skew for SSH handshake validation. |
| `OPENSHELL_SANDBOX_SSH_SOCKET_PATH` | `--sandbox-ssh-socket-path` | `/run/openshell/ssh.sock` | Standalone driver only: supervisor Unix socket path in `PodmanComputeConfig`. In-gateway Podman uses server `config.sandbox_ssh_socket_path`. |
| `OPENSHELL_STOP_TIMEOUT` | `--stop-timeout` | `10` | Container stop timeout in seconds. |
| `OPENSHELL_SUPERVISOR_IMAGE` | `--supervisor-image` | `openshell/supervisor:latest` through the gateway, required standalone | OCI image containing the supervisor binary. |
Expand Down
75 changes: 0 additions & 75 deletions crates/openshell-driver-podman/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -381,23 +381,6 @@ impl PodmanClient {
}
}

/// Perform a versioned HTTP request with a raw byte body (not JSON).
async fn request_raw(
&self,
method: hyper::Method,
path: &str,
content_type: &str,
body: Bytes,
) -> Result<(hyper::StatusCode, Bytes), PodmanApiError> {
let req = Self::build_request(
method,
&format!("/{API_VERSION}{path}"),
Full::new(body),
Some(content_type),
);
self.send_request(req, API_TIMEOUT).await
}

/// POST a JSON body and ignore 409 Conflict (resource already exists).
async fn create_ignore_conflict(&self, path: &str, body: &Value) -> Result<(), PodmanApiError> {
match self
Expand Down Expand Up @@ -550,64 +533,6 @@ impl PodmanClient {
Ok(gateway)
}

// ── Secret operations ────────────────────────────────────────────────

/// Create a Podman secret with the given name and raw value.
///
/// Idempotent: if a secret with the same name already exists it is
/// replaced (delete + recreate) so the value is always up-to-date.
pub async fn create_secret(&self, name: &str, value: &[u8]) -> Result<(), PodmanApiError> {
validate_name(name)?;
let encoded_name = url_encode(name);
let path = format!("/libpod/secrets/create?name={encoded_name}");
let (status, bytes) = self
.request_raw(
hyper::Method::POST,
&path,
"application/octet-stream",
Bytes::copy_from_slice(value),
)
.await?;

match status.as_u16() {
200 | 201 => Ok(()),
409 => {
// Secret already exists — replace it.
self.remove_secret(name).await?;
let (status2, bytes2) = self
.request_raw(
hyper::Method::POST,
&path,
"application/octet-stream",
Bytes::copy_from_slice(value),
)
.await?;
if status2.is_success() {
Ok(())
} else {
Err(error_from_response(status2.as_u16(), &bytes2))
}
}
_ => Err(error_from_response(status.as_u16(), &bytes)),
}
}

/// Remove a Podman secret by name. Idempotent (not-found is ignored).
pub async fn remove_secret(&self, name: &str) -> Result<(), PodmanApiError> {
validate_name(name)?;
match self
.request_ok(
hyper::Method::DELETE,
&format!("/libpod/secrets/{name}"),
None,
)
.await
{
Ok(()) | Err(PodmanApiError::NotFound(_)) => Ok(()),
Err(e) => Err(e),
}
}

// ── Image operations ────────────────────────────────────────────────

/// Pull an image if it is not already present locally.
Expand Down
Loading
Loading