Desktop: refuse unknown command-line options instead of ignoring them (#836) (#844)

A mistyped or unsupported flag (e.g. --automation-write-roots) used to be
dropped silently, so the app started unconfined. It now exits 2 with
'unknown option', like a bad automation root. A bare -- ends the options.

Adds a desktop control-channel test that file.save / file.saveAs outside the
write root are refused and create nothing, while saves inside succeed, and
documents that the root flags arrived in 0.7.0 (0.6.0 ignores them).
This commit is contained in:
skylar taylor-barrick
2026-10-18 15:20:15 -06:00
committed by GitHub
parent 7fcb193f2a
commit a0d1162976
5 changed files with 56 additions and 5 deletions
+12 -2
View File
@@ -2,7 +2,9 @@
//! [--automation-write-root <dir>]… [files…] [figcraft://join?…]`. An invite link
//! (what the system passes when a `figcraft://` link is opened) joins that session once the window
//! is up; everything else that isn't a flag is a file to open, as a path or a `file://` URL (the
//! Linux desktop entry passes URLs, `%U`, so that it can also hand over invite links).
//! Linux desktop entry passes URLs, `%U`, so that it can also hand over invite links). Any other
//! `--flag` is unknown and stops the app at startup (#836): a mistyped root flag must never leave
//! the app running unconfined. After a bare `--`, every argument is a file.
/// What the app was started with.
#[derive(Debug, Default, PartialEq, Eq)]
@@ -17,6 +19,8 @@ pub struct Args {
/// which is refused at startup.
pub read_roots: Vec<String>,
pub write_roots: Vec<String>,
/// `--flags` the app doesn't know, as given.
pub unknown: Vec<String>,
}
/// An invite link rather than a file (`figcraft:` links, in any case).
@@ -55,6 +59,10 @@ pub fn parse(args: &[String]) -> Args {
let mut out = Args::default();
let mut it = args.iter();
while let Some(a) = it.next() {
if a == "--" {
out.files.extend(it.by_ref().cloned());
break;
}
let (flag, inline) = a.split_once('=').map_or((a.as_str(), None), |(f, v)| (f, Some(v.to_string())));
let roots = match flag {
"--automation-read-root" => Some(&mut out.read_roots),
@@ -69,7 +77,9 @@ pub fn parse(args: &[String]) -> Args {
out.join = Some(a.clone());
} else if let Some(path) = file_url_path(a) {
out.files.push(path);
} else if !a.starts_with("--") {
} else if a.starts_with("--") {
out.unknown.push(a.clone());
} else {
out.files.push(a.clone());
}
}
+14 -1
View File
@@ -9,7 +9,7 @@ fn args(a: &[&str]) -> Vec<String> {
#[test]
fn files_and_the_control_port() {
assert_eq!(parse(&args(&[])), Args::default());
let a = parse(&args(&["--control", "4242", "a.figcraft", "--verbose", "b.svg"]));
let a = parse(&args(&["--control", "4242", "a.figcraft", "b.svg"]));
assert_eq!(a, Args { control: Some(4242), files: args(&["a.figcraft", "b.svg"]), ..Args::default() });
// A missing or bad port is ignored, not taken as a file.
assert_eq!(parse(&args(&["--control"])), Args::default());
@@ -26,6 +26,19 @@ fn automation_roots_repeat_and_are_never_files() {
assert_eq!(parse(&args(&["--automation-write-root"])).write_roots, args(&[""]));
}
#[test]
fn unknown_flags_are_refused_never_ignored() {
// #836: a mistyped root flag (or one an older release lacks) must stop the app, not start it
// unconfined. Both forms, with and without `=`.
let a = parse(&args(&["--automation-write-roots", "out", "--automation-write-dir=out", "--verbose", "a.figcraft"]));
assert_eq!(a.unknown, args(&["--automation-write-roots", "--automation-write-dir=out", "--verbose"]));
assert!(a.write_roots.is_empty());
// Single-dash arguments are left as they were (files); after `--` everything is a file.
let a = parse(&args(&["--control", "1", "--", "--odd.figcraft", "--automation-write-root=x"]));
assert_eq!(a.files, args(&["--odd.figcraft", "--automation-write-root=x"]));
assert!(a.unknown.is_empty() && a.write_roots.is_empty());
}
#[test]
fn invite_links_join_instead_of_opening() {
let link = "figcraft://join?host=192.168.1.20:47900&file=abc&token=AbCdEfGhIjKlMnOpQrStUv";
+8 -1
View File
@@ -10,7 +10,8 @@
//! `--automation-read-root <dir>` / `--automation-write-root <dir>` (each repeatable) confine the
//! files the window and the control channel read and write (`figcraft_engine::file_access`); the
//! app's own folders (settings, recovery copies, installed plugins) and opening the files given on
//! the command line are not. A root that isn't an existing folder stops the app at startup.
//! the command line are not. A root that isn't an existing folder stops the app at startup, and so
//! does any flag the app doesn't know (a mistyped root flag must not start the app unconfined).
#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
#![cfg_attr(all(target_os = "windows", not(debug_assertions)), windows_subsystem = "windows")]
@@ -129,6 +130,12 @@ fn main() -> eframe::Result {
return Ok(());
}
let args = args::parse(&args);
if let Some(flag) = args.unknown.first() {
eprintln!(
"FigCraft: unknown option {flag}\nUsage: figcraft [--control PORT] [--automation-read-root DIR]... [--automation-write-root DIR]... [FILE | figcraft://join?...]..."
);
std::process::exit(2);
}
let roots = match (
args.read_roots.is_empty() && args.write_roots.is_empty(),
figcraft_engine::file_access::Roots::new(&args.read_roots, &args.write_roots),
+18
View File
@@ -57,6 +57,24 @@ fn a_confined_app_writes_only_inside_its_write_root() {
assert!(app.session.execute("file.export", &json!({"format": "svg", "path": p("outside/own.svg")})).is_ok());
}
#[test]
fn a_confined_app_saves_only_inside_its_write_root() {
// #836: saving over the control channel, both as a new file and to the current one.
let (mut app, d) = confined_app("save");
let p = |rel: &str| d.0.join(rel).to_string_lossy().into_owned();
for cmd in ["file.saveAs", "file.save"] {
let e = handle(&mut app, "engine.execute", json!({"command": cmd, "params": {"path": p("outside/a.figcraft")}})).unwrap_err();
assert!(e.contains("automation path rejected"), "{cmd}: {e}");
let e = handle(&mut app, "engine.execute", json!({"command": cmd, "params": {"path": p("out/../outside/b.figcraft")}})).unwrap_err();
assert!(e.contains("automation path rejected"), "{cmd}: {e}");
}
assert_eq!(std::fs::read_dir(d.0.join("outside")).unwrap().count(), 0, "nothing written outside the root");
handle(&mut app, "engine.execute", json!({"command": "file.saveAs", "params": {"path": p("out/in.figcraft")}})).unwrap();
assert!(d.0.join("out/in.figcraft").is_file());
// Saving again goes to the same file, inside the root.
handle(&mut app, "engine.execute", json!({"command": "file.save", "params": {}})).unwrap();
}
#[test]
fn window_captures_are_written_through_the_roots_or_returned() {
let (app, d) = confined_app("capture");
+4 -1
View File
@@ -71,7 +71,10 @@ and what a person does in the window are held to the same folders. Files are rea
read or write root and written only inside a write root; anything else is an ordinary error reply
(`automation path rejected: …`), and recent files outside the roots aren't listed. Paths are
resolved as `docs/mcp.md` › Confining file access describes. A root that isn't an existing folder
stops the app before its window opens. Without the flags nothing changes.
stops the app before its window opens, and so does any option the app doesn't know (a mistyped
flag such as `--automation-write-roots` exits with `unknown option`, never starts unconfined).
Without the flags nothing changes. The flags arrived in FigCraft 0.7.0: 0.6.0 and earlier ignore
them and confine nothing.
Not confined: opening the files given on the command line when the app starts and the documents
the system hands over (Finder, Open With; saving them later is confined like any write), and the