Compare commits

..

2 Commits

Author SHA1 Message Date
jasonwitty 56a2dc372a Reject unknown options in parse_args instead of treating them as the URL
CI / build (ubuntu-latest) (push) Has been cancelled
CI / build (windows-latest) (push) Has been cancelled
An older socktop handed a newer flag (webterm 0.3.9's restricted shell
passing --no-kill to 1.60.1) silently parsed the flag as the positional
websocket URL and offered to overwrite the named profile's URL with the
literal flag text. Unknown options now fail with 'Unknown option' and
exit code 2 (help remains exit 0), with a regression test covering the
exact incident shape.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-24 09:46:20 -07:00
jasonwitty 59320c3bc0 fix formatting 2026-08-24 07:26:16 -07:00
3 changed files with 51 additions and 5 deletions
Generated
+3 -3
View File
@@ -2412,7 +2412,7 @@ dependencies = [
[[package]] [[package]]
name = "socktop" name = "socktop"
version = "1.60.1" version = "1.60.2"
dependencies = [ dependencies = [
"anyhow", "anyhow",
"assert_cmd", "assert_cmd",
@@ -2432,7 +2432,7 @@ dependencies = [
[[package]] [[package]]
name = "socktop_agent" name = "socktop_agent"
version = "1.60.1" version = "1.60.2"
dependencies = [ dependencies = [
"anyhow", "anyhow",
"assert_cmd", "assert_cmd",
@@ -2464,7 +2464,7 @@ dependencies = [
[[package]] [[package]]
name = "socktop_connector" name = "socktop_connector"
version = "1.60.1" version = "1.60.2"
dependencies = [ dependencies = [
"flate2", "flate2",
"futures-util", "futures-util",
+22 -2
View File
@@ -124,6 +124,15 @@ pub(crate) fn parse_args<I: IntoIterator<Item = String>>(args: I) -> Result<Pars
} }
} }
_ => { _ => {
// An unrecognized option must never fall through to the
// positional URL slot: an older binary handed a newer flag
// would otherwise "connect" to the flag text — and offer to
// save it over a named profile's URL.
if arg.starts_with('-') {
return Err(format!(
"Unknown option '{arg}'. Usage: {prog} [--tls-ca CERT_PEM|-t CERT_PEM] [--verify-hostname] [--profile NAME|-P NAME] [--save] [--demo] [--compact] [--no-kill] [ws://HOST:PORT/ws]"
));
}
if url.is_none() { if url.is_none() {
url = Some(arg); url = Some(arg);
} else { } else {
@@ -155,6 +164,11 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
Ok(v) => v, Ok(v) => v,
Err(msg) => { Err(msg) => {
eprintln!("{msg}"); eprintln!("{msg}");
// --help produces the bare usage text and exits cleanly; real
// parse errors must be visible to scripts and CI as a failure.
if !msg.starts_with("Usage:") {
std::process::exit(2);
}
return Ok(()); return Ok(());
} }
}; };
@@ -271,7 +285,12 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
if (1..=names.len()).contains(&idx) { if (1..=names.len()).contains(&idx) {
let name = &names[idx - 1]; let name = &names[idx - 1];
if name == "demo" { if name == "demo" {
return run_demo_mode(parsed.tls_ca.as_deref(), parsed.compact, parsed.no_kill).await; return run_demo_mode(
parsed.tls_ca.as_deref(),
parsed.compact,
parsed.no_kill,
)
.await;
} }
if let Some(entry) = profiles_mut.profiles.get(name) { if let Some(entry) = profiles_mut.profiles.get(name) {
( (
@@ -331,7 +350,8 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
); );
eprintln!("If you don't have an agent running, you can try the demo mode."); eprintln!("If you don't have an agent running, you can try the demo mode.");
if prompt_yes_no("Would you like to start the demo mode now? [Y/n]: ") { if prompt_yes_no("Would you like to start the demo mode now? [Y/n]: ") {
return run_demo_mode(parsed.tls_ca.as_deref(), parsed.compact, parsed.no_kill).await; return run_demo_mode(parsed.tls_ca.as_deref(), parsed.compact, parsed.no_kill)
.await;
} else { } else {
eprintln!("Aborting. You can run 'socktop --help' for usage information."); eprintln!("Aborting. You can run 'socktop --help' for usage information.");
return Ok(()); return Ok(());
+26
View File
@@ -156,3 +156,29 @@ fn test_no_kill_env_var_accepted() {
String::from_utf8_lossy(&out.stderr) String::from_utf8_lossy(&out.stderr)
); );
} }
#[test]
fn test_unknown_option_rejected_not_treated_as_url() {
// Regression guard for the socktop.io incident (Aug 2026): socktop 1.60.1
// parsed the then-unknown --no-kill flag as the positional websocket URL,
// which made it prompt to overwrite the 'local' profile's URL with the
// literal string "--no-kill". Unknown options must fail loudly instead of
// falling through to the URL slot.
let exe = env!("CARGO_BIN_EXE_socktop");
let out = Command::new(exe)
.args(["--not-a-real-flag", "--dry-run", "ws://127.0.0.1:3000/ws"])
.output()
.expect("run socktop with unknown flag");
assert_eq!(
out.status.code(),
Some(2),
"unknown option should exit 2, got: {:?}\nstderr: {}",
out.status.code(),
String::from_utf8_lossy(&out.stderr)
);
let err = String::from_utf8_lossy(&out.stderr);
assert!(
err.contains("Unknown option '--not-a-real-flag'"),
"stderr should name the rejected option\n{err}"
);
}