Skip to content

Commit 51be952

Browse files
elezardrew
authored andcommitted
test(compute): cover external driver socket contract
Signed-off-by: Evan Lezar <elezar@nvidia.com>
1 parent 9033294 commit 51be952

2 files changed

Lines changed: 108 additions & 1 deletion

File tree

.agents/skills/debug-openshell-cluster/SKILL.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ journalctl -u <driver-service> --no-pager --lines=200
8787
journalctl -u openshell-gateway --no-pager --lines=200
8888
```
8989

90-
Custom names use `[openshell.drivers.<name>].socket_path`. A launch-time `--compute-driver-socket` override may also use `docker`, `podman`, `kubernetes`, or `vm`; the endpoint then takes precedence over built-in construction. The socket must be accessible only to the intended gateway identity. Check gateway logs for connection errors, `GetCapabilities` failures, or an unexpected advertised driver name. The advertised name is diagnostic metadata; negotiated features control optional behavior. The gateway does not create or supervise operator-supplied driver processes or sockets.
90+
Custom names use `[openshell.drivers.<name>].socket_path`. A launch-time `--compute-driver-socket` override may also use `docker`, `podman`, `kubernetes`, or `vm`; the endpoint then takes precedence over built-in construction. First-party standalone drivers require the socket parent directory to be owned by the driver's effective UID, force its mode to `0700`, create the socket with mode `0600`, and accept only peers with that same UID. Check the parent and socket separately with `stat`; a gateway running under a different UID cannot connect even when filesystem permissions or group membership would otherwise allow it. Operator-supplied drivers must provide equivalent access control appropriate to their implementation. Check gateway logs for connection errors, `GetCapabilities` failures, or an unexpected advertised driver name. The advertised name is diagnostic metadata; negotiated features control optional behavior. The gateway does not create or supervise operator-supplied driver processes or sockets.
9191

9292
For configured gateway interceptors, inspect `[[openshell.gateway.interceptors]]`, their Unix or network endpoints, and gateway startup logs:
9393

crates/openshell-core/src/external_driver_socket.rs

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,3 +123,110 @@ impl Stream for SameUidUnixIncoming {
123123
}
124124
}
125125
}
126+
127+
#[cfg(test)]
128+
mod tests {
129+
use std::os::unix::fs::{PermissionsExt, symlink};
130+
131+
use tokio_stream::StreamExt;
132+
133+
use super::*;
134+
135+
#[tokio::test]
136+
async fn bind_private_creates_private_socket_and_cleanup_removes_it() {
137+
let temp_dir = tempfile::tempdir().expect("create temporary directory");
138+
let socket_dir = temp_dir.path().join("driver");
139+
let socket_path = socket_dir.join("compute.sock");
140+
141+
let listener = bind_private(&socket_path).expect("bind private socket");
142+
143+
let directory_mode = std::fs::metadata(&socket_dir)
144+
.expect("stat socket directory")
145+
.permissions()
146+
.mode()
147+
& 0o777;
148+
let socket_mode = std::fs::metadata(&socket_path)
149+
.expect("stat socket")
150+
.permissions()
151+
.mode()
152+
& 0o777;
153+
assert_eq!(directory_mode, 0o700);
154+
assert_eq!(socket_mode, 0o600);
155+
156+
drop(listener);
157+
let cleanup = SocketCleanup::new(socket_path.clone());
158+
drop(cleanup);
159+
assert!(!socket_path.exists());
160+
}
161+
162+
#[test]
163+
fn bind_private_rejects_symlinked_parent() {
164+
let temp_dir = tempfile::tempdir().expect("create temporary directory");
165+
let actual_dir = temp_dir.path().join("actual");
166+
let linked_dir = temp_dir.path().join("linked");
167+
std::fs::create_dir(&actual_dir).expect("create socket directory");
168+
symlink(&actual_dir, &linked_dir).expect("create directory symlink");
169+
170+
let err = bind_private(&linked_dir.join("compute.sock"))
171+
.expect_err("symlinked socket parent must be rejected");
172+
173+
assert!(err.contains("must be a directory, not a symlink"));
174+
}
175+
176+
#[test]
177+
fn bind_private_rejects_existing_non_socket() {
178+
let temp_dir = tempfile::tempdir().expect("create temporary directory");
179+
let socket_path = temp_dir.path().join("compute.sock");
180+
std::fs::write(&socket_path, b"not a socket").expect("create conflicting file");
181+
182+
let err = bind_private(&socket_path).expect_err("non-socket path must be rejected");
183+
184+
assert!(err.contains("is not an owned Unix socket"));
185+
assert_eq!(
186+
std::fs::read(&socket_path).expect("read conflicting file"),
187+
b"not a socket"
188+
);
189+
}
190+
191+
#[tokio::test]
192+
async fn bind_private_replaces_existing_owned_socket() {
193+
let temp_dir = tempfile::tempdir().expect("create temporary directory");
194+
let socket_path = temp_dir.path().join("compute.sock");
195+
let stale_listener =
196+
std::os::unix::net::UnixListener::bind(&socket_path).expect("bind stale Unix socket");
197+
drop(stale_listener);
198+
199+
let listener = bind_private(&socket_path).expect("replace owned Unix socket");
200+
201+
assert!(
202+
std::fs::symlink_metadata(&socket_path)
203+
.expect("stat replacement socket")
204+
.file_type()
205+
.is_socket()
206+
);
207+
drop(listener);
208+
}
209+
210+
#[tokio::test]
211+
async fn same_uid_incoming_accepts_current_user() {
212+
let temp_dir = tempfile::tempdir().expect("create temporary directory");
213+
let socket_path = temp_dir.path().join("compute.sock");
214+
let listener = bind_private(&socket_path).expect("bind private socket");
215+
let mut incoming = SameUidUnixIncoming::new(listener);
216+
217+
let client = UnixStream::connect(&socket_path)
218+
.await
219+
.expect("connect to private socket");
220+
let accepted = incoming
221+
.next()
222+
.await
223+
.expect("incoming stream ended")
224+
.expect("accept same-UID client");
225+
226+
assert_eq!(
227+
accepted.peer_cred().expect("read client credentials").uid(),
228+
rustix::process::geteuid().as_raw()
229+
);
230+
drop(client);
231+
}
232+
}

0 commit comments

Comments
 (0)