Skip to content

Commit 0c60566

Browse files
authored
feat(shell): named sessions with --name flag (#50)
* feat(shell): named sessions with --name flag (closes #49) Introduces human-readable session names, inspired by tmux, to replace raw PID-based socket paths in the control-channel UX. Launcher changes: - Add --name <slug> to Cli; socket at $TMPDIR/shadi-ctl-<name>.sock - Print "session name: <name>" on startup instead of the raw socket path Shell changes: - shadictl shell --attach <name> resolves to the named socket at startup - /attach accepts a session name OR a full socket path - /sessions displays human-readable names, not raw socket paths - Tab completion for /attach offers session names instead of full paths - /help and usage text updated to mention name support - Prompt short name strips the shadi-ctl- prefix policy_watch helpers: - named_socket_path(name) - name-based socket path (Unix + Windows) - resolve_session_socket(name_or_path) - accepts name or full path - session_name_from_path(path) - display name from socket file stem - sanitize_session_name(name) - allows [a-z0-9_-], replaces other chars Tests: 6 new unit tests for the helpers; 1 new integration test for /attach <name> on a non-existent session name. Signed-off-by: Luca Muscariello <muscariello@ieee.org> * fix(policy_watch): degrade gracefully when getpeereid(2) fails on macOS On macOS 15 Sequoia CI runners, getpeereid(2) returns non-zero in some environments, causing the control socket to reject all connections with "unauthorized control socket peer" and break the terminate_round_trip test with EPIPE. The socket is already protected by 0o600 filesystem permissions set at bind time, so peer-UID verification is defense-in-depth. Degrade gracefully when the syscall itself fails (allow the connection) while still actively denying when getpeereid succeeds but reports a UID mismatch. Signed-off-by: Luca Muscariello <muscariello@ieee.org> * test(shell): cover cmd_attach success/error paths and /attach completion loop Add three targeted unit tests to close the diff coverage gap on PR #50: - given_non_socket_file_when_attach_then_stays_detached: exercises the Err branch of cmd_attach (file exists but query_policy fails). - given_live_socket_when_attach_then_sets_socket: exercises the Ok branch by spinning up a real LivePolicy control socket via start_control_socket, polling until ready, then attaching and asserting self.socket is set. - given_session_file_in_tmpdir_when_completing_attach_then_returns_session_name: exercises the inner loop of the /attach tab-completion handler by placing a shadi-ctl-*.sock marker file in temp_dir(), completing '/attach ', and verifying the session name appears in the candidate list. The marker file is cleaned up (inside catch_unwind) whether the assertion passes or not. Signed-off-by: Luca Muscariello <muscariello@ieee.org> --------- Signed-off-by: Luca Muscariello <muscariello@ieee.org>
1 parent 72ac968 commit 0c60566

8 files changed

Lines changed: 296 additions & 33 deletions

File tree

crates/shadictl/src/cli_types.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,15 @@ pub(crate) struct Cli {
6161
#[arg(long = "watch-policy", action = ArgAction::SetTrue)]
6262
pub(crate) watch_policy: bool,
6363

64+
/// Human-readable name for this sandbox session.
65+
/// When set, the control socket is created at
66+
/// `$TMPDIR/shadi-ctl-<name>.sock` instead of
67+
/// `$TMPDIR/shadi-ctl-<pid>.sock`, making it easy to
68+
/// attach by name: `/attach <name>`.
69+
/// Allowed characters: letters, digits, hyphens, underscores.
70+
#[arg(long = "name", value_name = "NAME")]
71+
pub(crate) session_name: Option<String>,
72+
6473
#[command(subcommand)]
6574
pub(crate) subcommand: Option<Commands>,
6675

@@ -319,6 +328,12 @@ pub(crate) struct ShellArgs {
319328
/// Connect to a running sandbox session by control socket path
320329
#[arg(long = "socket", value_name = "PATH")]
321330
pub(crate) socket: Option<PathBuf>,
331+
332+
/// Connect to a running sandbox session by its human-readable name
333+
/// (as given by --name when launching shadictl).
334+
/// Equivalent to --socket $TMPDIR/shadi-ctl-<name>.sock.
335+
#[arg(long = "attach", value_name = "NAME", conflicts_with = "socket")]
336+
pub(crate) attach: Option<String>,
322337
}
323338

324339
#[derive(Subcommand, Debug)]

crates/shadictl/src/introspection_command.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ fn build_policy_cli(
4646
git_snapshot_dir: None,
4747
git_snapshot_untracked: false,
4848
watch_policy: false,
49+
session_name: None,
4950
subcommand: None,
5051
run_command: Vec::new(),
5152
}

crates/shadictl/src/main_tests.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
git_snapshot_dir: None,
5656
git_snapshot_untracked: false,
5757
watch_policy: false,
58+
session_name: None,
5859
subcommand: None,
5960
run_command: vec!["echo".to_string(), "ok".to_string()],
6061
}

crates/shadictl/src/policy_watch.rs

Lines changed: 122 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,58 @@ pub(crate) fn default_socket_path(pid: u32) -> PathBuf {
117117
PathBuf::from(dir).join(format!("shadi-ctl-{}.sock", pid))
118118
}
119119

120+
/// Resolve a named control socket path.
121+
///
122+
/// Named sockets use `$TMPDIR/shadi-ctl-<name>.sock` instead of the PID-based
123+
/// path, making them stable and human-addressable: any tool can connect by name
124+
/// without needing to discover the PID.
125+
///
126+
/// Only alphanumeric characters, hyphens, and underscores are allowed in the
127+
/// name; other characters are replaced with `-` to keep the filename safe.
128+
#[cfg(unix)]
129+
pub(crate) fn named_socket_path(name: &str) -> PathBuf {
130+
let dir = std::env::var("TMPDIR").unwrap_or_else(|_| "/tmp".to_string());
131+
let slug = sanitize_session_name(name);
132+
PathBuf::from(dir).join(format!("shadi-ctl-{}.sock", slug))
133+
}
134+
135+
/// Resolve the control socket path for a given session name or socket path.
136+
///
137+
/// If `name_or_path` looks like a path (contains `/` or ends with `.sock`) it
138+
/// is used as-is; otherwise it is treated as a session name and resolved via
139+
/// `named_socket_path`.
140+
#[cfg(unix)]
141+
pub(crate) fn resolve_session_socket(name_or_path: &str) -> PathBuf {
142+
if name_or_path.contains('/') || name_or_path.ends_with(".sock") {
143+
PathBuf::from(name_or_path)
144+
} else {
145+
named_socket_path(name_or_path)
146+
}
147+
}
148+
149+
/// Extract the human-readable session name from a socket path.
150+
///
151+
/// `$TMPDIR/shadi-ctl-myagent.sock` → `"myagent"`
152+
/// `$TMPDIR/shadi-ctl-12345.sock` → `"12345"` (PID-based fallback)
153+
pub(crate) fn session_name_from_path(path: &Path) -> String {
154+
path.file_stem()
155+
.and_then(|s| s.to_str())
156+
.and_then(|s| s.strip_prefix("shadi-ctl-"))
157+
.unwrap_or_else(|| path.file_stem().and_then(|s| s.to_str()).unwrap_or("session"))
158+
.to_string()
159+
}
160+
161+
/// Sanitize a session name: keep only alphanumerics, hyphens, and underscores;
162+
/// replace everything else with `-`; truncate to 48 characters.
163+
pub(crate) fn sanitize_session_name(name: &str) -> String {
164+
let slug: String = name
165+
.chars()
166+
.map(|c| if c.is_ascii_alphanumeric() || c == '-' || c == '_' { c } else { '-' })
167+
.collect();
168+
let slug = slug.trim_matches('-');
169+
slug.chars().take(48).collect()
170+
}
171+
120172
/// Resolve the default control socket path for a given PID.
121173
///
122174
/// On Windows this is an `AF_UNIX` socket file (requires Windows 10 1803+).
@@ -126,6 +178,24 @@ pub(crate) fn default_socket_path(pid: u32) -> PathBuf {
126178
PathBuf::from(dir).join(format!("shadi-ctl-{}.sock", pid))
127179
}
128180

181+
/// Resolve a named control socket path (Windows).
182+
#[cfg(windows)]
183+
pub(crate) fn named_socket_path(name: &str) -> PathBuf {
184+
let dir = std::env::var("TEMP").unwrap_or_else(|_| ".".to_string());
185+
let slug = sanitize_session_name(name);
186+
PathBuf::from(dir).join(format!("shadi-ctl-{}.sock", slug))
187+
}
188+
189+
/// Resolve the control socket path for a given session name or socket path (Windows).
190+
#[cfg(windows)]
191+
pub(crate) fn resolve_session_socket(name_or_path: &str) -> PathBuf {
192+
if name_or_path.contains('\\') || name_or_path.contains('/') || name_or_path.ends_with(".sock") {
193+
PathBuf::from(name_or_path)
194+
} else {
195+
named_socket_path(name_or_path)
196+
}
197+
}
198+
129199
// ---------------------------------------------------------------------------
130200
// Listener (AF_UNIX on all platforms)
131201
// ---------------------------------------------------------------------------
@@ -197,7 +267,12 @@ fn authorize_control_peer(stream: &UnixStream) -> Result<(), String> {
197267
let mut gid: libc::gid_t = 0;
198268
let rc = unsafe { libc::getpeereid(stream.as_raw_fd(), &mut uid, &mut gid) };
199269
if rc != 0 {
200-
return Err(std::io::Error::last_os_error().to_string());
270+
// getpeereid(2) failed — this can happen on some macOS versions/environments
271+
// (e.g., macOS 15 Sequoia CI runners). The socket is already protected by
272+
// 0o600 filesystem permissions set at bind time, so we degrade gracefully:
273+
// skip UID verification rather than refusing all connections. We only
274+
// actively deny when the syscall succeeds but reports a UID mismatch.
275+
return Ok(());
201276
}
202277
let current_uid = unsafe { libc::geteuid() };
203278
if uid != current_uid {
@@ -1619,4 +1694,50 @@ mod tests {
16191694

16201695
drop(handle);
16211696
}
1697+
1698+
// ── named session helpers ────────────────────────────────────────────────
1699+
1700+
#[test]
1701+
fn named_socket_path_uses_slug_not_pid() {
1702+
let path = named_socket_path("my-agent");
1703+
let name = path.file_name().unwrap().to_str().unwrap();
1704+
assert_eq!(name, "shadi-ctl-my-agent.sock");
1705+
}
1706+
1707+
#[test]
1708+
fn named_socket_path_sanitizes_special_chars() {
1709+
let path = named_socket_path("my agent/session!");
1710+
let name = path.file_name().unwrap().to_str().unwrap();
1711+
// spaces and slashes become hyphens; exclamation is stripped
1712+
assert!(name.starts_with("shadi-ctl-"));
1713+
assert!(name.ends_with(".sock"));
1714+
assert!(!name.contains(' '));
1715+
assert!(!name.contains('/'));
1716+
}
1717+
1718+
#[test]
1719+
fn session_name_from_path_strips_prefix_and_extension() {
1720+
let path = std::path::PathBuf::from("/tmp/shadi-ctl-codex-session.sock");
1721+
assert_eq!(session_name_from_path(&path), "codex-session");
1722+
}
1723+
1724+
#[test]
1725+
fn session_name_from_path_works_for_pid_sockets() {
1726+
let path = std::path::PathBuf::from("/tmp/shadi-ctl-98765.sock");
1727+
assert_eq!(session_name_from_path(&path), "98765");
1728+
}
1729+
1730+
#[test]
1731+
fn resolve_session_socket_accepts_name() {
1732+
let path = resolve_session_socket("my-agent");
1733+
let name = path.file_name().unwrap().to_str().unwrap();
1734+
assert_eq!(name, "shadi-ctl-my-agent.sock");
1735+
}
1736+
1737+
#[test]
1738+
fn resolve_session_socket_accepts_full_path() {
1739+
let path = resolve_session_socket("/tmp/shadi-ctl-12345.sock");
1740+
assert_eq!(path, std::path::PathBuf::from("/tmp/shadi-ctl-12345.sock"));
1741+
}
16221742
}
1743+

crates/shadictl/src/sandbox_snapshot.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,10 +220,17 @@ pub(crate) fn run_sandboxed_command(
220220
live_net_allowlist: net_allowlist,
221221
}));
222222
let pid = std::process::id();
223-
let sock_path = default_socket_path(pid);
223+
let sock_path = match cli.session_name.as_deref() {
224+
Some(name) => policy_watch::named_socket_path(name),
225+
None => default_socket_path(pid),
226+
};
224227
match start_control_socket(&sock_path, std::sync::Arc::clone(&live)) {
225228
Ok(handle) => {
226-
eprintln!("control socket: {}", handle.path().display());
229+
if let Some(name) = cli.session_name.as_deref() {
230+
eprintln!("session name: {}", name);
231+
} else {
232+
eprintln!("control socket: {}", handle.path().display());
233+
}
227234
control_live = Some(live);
228235
restart_requested = Some(restart_flag);
229236
terminate_requested = Some(terminate_flag);
@@ -1162,6 +1169,7 @@ mod tests {
11621169
git_snapshot_dir: None,
11631170
git_snapshot_untracked: false,
11641171
watch_policy: false,
1172+
session_name: None,
11651173
subcommand: None,
11661174
run_command: vec!["echo".to_string(), "ok".to_string()],
11671175
}

0 commit comments

Comments
 (0)