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>
This commit is contained in:
jasonwitty
2026-08-24 09:46:20 -07:00
parent 59320c3bc0
commit 56a2dc372a
2 changed files with 40 additions and 0 deletions
+14
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(());
} }
}; };
+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}"
);
}