mirror of
https://github.com/storytold/vectorcraft.git
synced 2026-10-14 19:26:56 -04:00
Fix #832: --automation-read-root and --automation-write-root confine the MCP server's files
vectorcraft-cli mcp and vectorcraft --control take the flags PhotoCraft takes. While roots are in force, every path a command reads (open, place, relink, SVG image links, library and profile loads, plug-ins, preset imports, font searches) must lie inside the read root and every path it writes (save, export, package, Export for Screens and for Web, print to file, captures, library saves) inside the write root. Paths are made absolute, links are followed (a missing file's nearest folder's), and the result is compared by whole components, ignoring case on Windows; `..` below a missing folder, dangling links and device names are refused. A root left out grants none of its access; without the flags nothing changes. The checks live in one place, vectorcraft_engine::file_access, consulted by the engine's file helpers and the app's file services. The headless server confines its whole process; the app confines control requests, the frames carrying input they inject and the background saves they start, leaving the person at the keyboard and the app's own folders alone. The screenshot tool now has the backend write its file. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
1 parent
ab69087d68
commit
9f875050ef
32 files changed
+1353
-136
No files matched your search
@@ -1,7 +1,7 @@
|
||||
//! `vectorcraft-cli`: VectorCraft from the command line.
|
||||
//!
|
||||
//! ```text
|
||||
//! vectorcraft-cli mcp [--connect 127.0.0.1:7979 | --headless]
|
||||
//! vectorcraft-cli mcp [--connect 127.0.0.1:7979 | --headless] [--automation-read-root DIR] [--automation-write-root DIR]
|
||||
//! vectorcraft-cli run [--in FILE] [--cmd id [--params '{json}']]... [--export out.svg]... [--scale 2]
|
||||
//! vectorcraft-cli commands
|
||||
//! vectorcraft-cli convert IN OUT [--scale 2] [--artboard 0 | --range 1-3,5] [--outline-text] [--replace-lossy]
|
||||
@@ -14,6 +14,7 @@
|
||||
#![deny(unsafe_code)]
|
||||
|
||||
use std::io::Write;
|
||||
use std::path::Path;
|
||||
use std::process::ExitCode;
|
||||
|
||||
/// `println!` / `print!` that end the program quietly when stdout is closed
|
||||
@@ -50,6 +51,7 @@ mod prefs_dir;
|
||||
mod system_fonts;
|
||||
|
||||
use serde_json::{Value, json};
|
||||
use vectorcraft_engine::file_access::{self, AutomationRoots};
|
||||
use vectorcraft_mcp::{Backend, DEFAULT_ADDR, Headless, Remote, Server};
|
||||
|
||||
/// stdout went away. A reader that stopped early (a closed pipe) ends the program quietly, as
|
||||
@@ -66,9 +68,13 @@ const USAGE: &str = "\
|
||||
vectorcraft-cli — VectorCraft automation
|
||||
|
||||
USAGE:
|
||||
vectorcraft-cli mcp [--connect ADDR | --headless]
|
||||
vectorcraft-cli mcp [--connect ADDR | --headless] [--automation-read-root DIR] [--automation-write-root DIR]
|
||||
Run the MCP server on stdio. Default: connect to a running app at 127.0.0.1:7979
|
||||
(vectorcraft --control 7979), falling back to a headless in-process session.
|
||||
--automation-read-root / --automation-write-root confine the files the agent's commands read
|
||||
(open, place, relink, library loads…) and write (save, export, package, library saves…) to
|
||||
DIR, links followed; a root left out grants none of its access. They imply --headless: a
|
||||
running app enforces its own (vectorcraft --control PORT --automation-read-root DIR …).
|
||||
|
||||
vectorcraft-cli run [--in FILE] [--cmd ID [--params JSON]]... [--export FILE]... [--scale N]
|
||||
Headless batch: open FILE (any readable format) or start a new document, run commands in
|
||||
@@ -145,18 +151,42 @@ fn main() -> ExitCode {
|
||||
fn mcp(args: &[String]) -> Result<(), String> {
|
||||
let mut connect: Option<String> = None;
|
||||
let mut headless = false;
|
||||
let (mut read_root, mut write_root): (Option<String>, Option<String>) = (None, None);
|
||||
let mut it = args.iter();
|
||||
while let Some(a) = it.next() {
|
||||
match a.as_str() {
|
||||
// The roots also come as `--flag=DIR`, as the other Craft apps take them.
|
||||
let (flag, inline) = match a.split_once('=') {
|
||||
Some((f, v)) if f.starts_with("--automation-") => (f, Some(v.to_string())),
|
||||
_ => (a.as_str(), None),
|
||||
};
|
||||
match flag {
|
||||
"--connect" => connect = Some(it.next().cloned().ok_or("--connect needs an address")?),
|
||||
"--headless" => headless = true,
|
||||
"--automation-read-root" | "--automation-write-root" => {
|
||||
let dir = inline.or_else(|| it.next().cloned()).ok_or_else(|| format!("{flag} needs a folder"))?;
|
||||
let slot = if flag == "--automation-read-root" { &mut read_root } else { &mut write_root };
|
||||
if slot.replace(dir).is_some() {
|
||||
return Err(format!("{flag} is given twice: it takes one folder"));
|
||||
}
|
||||
}
|
||||
other => return Err(format!("unknown mcp option `{other}`")),
|
||||
}
|
||||
}
|
||||
if headless && connect.is_some() {
|
||||
return Err("use either --connect or --headless".into());
|
||||
}
|
||||
let backend: Box<dyn Backend> = if headless {
|
||||
let roots = AutomationRoots::new(read_root.as_deref().map(Path::new), write_root.as_deref().map(Path::new))?;
|
||||
if roots.is_some() && connect.is_some() {
|
||||
return Err("--automation-read-root and --automation-write-root confine the headless server; a running app enforces its own roots: start it with `vectorcraft --control <port> --automation-read-root <dir> --automation-write-root <dir>`"
|
||||
.into());
|
||||
}
|
||||
let backend: Box<dyn Backend> = if let Some(roots) = roots {
|
||||
// Every thread of this process, not only the server's: nothing reaches past the roots.
|
||||
file_access::confine_process(roots.clone());
|
||||
let show = |root: Option<&Path>| root.map_or_else(|| "none".to_string(), |r| r.display().to_string());
|
||||
eprintln!("vectorcraft-cli: files confined: read root {}, write root {}", show(roots.read_root()), show(roots.write_root()));
|
||||
Box::new(Headless::with_document().with_automation_roots(Some(roots)))
|
||||
} else if headless {
|
||||
Box::new(Headless::with_document())
|
||||
} else if let Some(addr) = connect {
|
||||
// Explicit address: fail loudly if the app isn't there.
|
||||
|
||||
@@ -83,6 +83,58 @@ fn mcp_headless_over_stdio() {
|
||||
assert_eq!(replies[3]["result"]["content"][0]["type"], "image");
|
||||
}
|
||||
|
||||
/// `mcp --automation-read-root` / `--automation-write-root` (#832): the flags are checked before the
|
||||
/// server starts, and the process then reads and writes only inside its roots.
|
||||
#[test]
|
||||
fn mcp_automation_roots() {
|
||||
let base = tmp("roots");
|
||||
for d in ["in", "out", "outside"] {
|
||||
std::fs::create_dir_all(base.join(d)).unwrap();
|
||||
}
|
||||
let svg = r#"<svg xmlns="http://www.w3.org/2000/svg" width="40" height="30"><rect width="20" height="10"/></svg>"#;
|
||||
std::fs::write(base.join("in/a.svg"), svg).unwrap();
|
||||
std::fs::write(base.join("outside/b.svg"), svg).unwrap();
|
||||
let dir = |d: &str| base.join(d).to_string_lossy().into_owned();
|
||||
let fails = |args: &[&str], why: &str| {
|
||||
let out = Command::new(BIN).args(args).stdin(Stdio::null()).output().unwrap();
|
||||
let err = String::from_utf8_lossy(&out.stderr);
|
||||
assert!(!out.status.success() && err.contains(why), "{args:?}: {err}");
|
||||
};
|
||||
fails(&["mcp", "--automation-read-root", &dir("missing")], "cannot open the automation read root");
|
||||
fails(&["mcp", "--automation-read-root", &dir("in"), "--automation-read-root", &dir("out")], "given twice");
|
||||
fails(&["mcp", "--automation-write-root"], "needs a folder");
|
||||
fails(&["mcp", "--connect", "127.0.0.1:1", &format!("--automation-write-root={}", dir("out"))], "a running app enforces its own roots");
|
||||
|
||||
let mut child = Command::new(BIN)
|
||||
.args(["mcp", "--automation-read-root", &dir("in"), &format!("--automation-write-root={}", dir("out"))])
|
||||
.stdin(Stdio::piped())
|
||||
.stdout(Stdio::piped())
|
||||
.stderr(Stdio::null())
|
||||
.spawn()
|
||||
.unwrap();
|
||||
let call = |id: u64, name: &str, args: Value| json!({"jsonrpc":"2.0","id":id,"method":"tools/call","params":{"name":name,"arguments":args}});
|
||||
let file = |d: &str, f: &str| base.join(d).join(f).to_string_lossy().into_owned();
|
||||
{
|
||||
let mut stdin = child.stdin.take().unwrap();
|
||||
for m in [
|
||||
call(1, "open_file", json!({"path": file("in", "a.svg")})),
|
||||
call(2, "open_file", json!({"path": file("outside", "b.svg")})),
|
||||
call(3, "export", json!({"path": file("out", "a.png")})),
|
||||
call(4, "export", json!({"path": file("in", "a.png")})),
|
||||
call(5, "save_file", json!({"path": file("outside", "a.vectorcraft")})),
|
||||
] {
|
||||
writeln!(stdin, "{m}").unwrap();
|
||||
}
|
||||
}
|
||||
let out = child.wait_with_output().unwrap();
|
||||
assert!(out.status.success());
|
||||
let replies: Vec<Value> = String::from_utf8(out.stdout).unwrap().lines().map(|l| serde_json::from_str(l).unwrap()).collect();
|
||||
let errors: Vec<bool> = replies.iter().map(|r| r["result"]["isError"] == true).collect();
|
||||
assert_eq!(errors, [false, true, false, true, true], "{replies:?}");
|
||||
assert!(replies[1]["result"]["content"][0]["text"].as_str().unwrap().contains("outside the read root"));
|
||||
assert!(base.join("out/a.png").is_file() && !base.join("in/a.png").exists() && !base.join("outside/a.vectorcraft").exists());
|
||||
}
|
||||
|
||||
/// End-to-end protocol sweep over stdio against the compiled binary. The in-process
|
||||
/// tests drive `Server::handle_line` directly, so the binary's own wiring (argument
|
||||
/// parsing, logger install, stdout pollution, notification order) is untested: this
|
||||
|
||||
@@ -1,13 +1,17 @@
|
||||
//! VectorCraft desktop app.
|
||||
//!
|
||||
//! Usage: `vectorcraft [--control <port>] [--in-window-menus] [files…]`
|
||||
//! Usage: `vectorcraft [--control <port> [--automation-read-root <dir>] [--automation-write-root <dir>]]
|
||||
//! [--in-window-menus] [files…]`
|
||||
//!
|
||||
//! `--in-window-menus` (or `VECTORCRAFT_IN_WINDOW_MENUS=1`) keeps the menus inside the window on
|
||||
//! macOS instead of the macOS menu bar (`mac_menu`).
|
||||
//!
|
||||
//! `--control <port>` (or `VECTORCRAFT_CONTROL_PORT`) starts a localhost JSON-lines control server:
|
||||
//! `{"id":1,"method":"ui.inspect","params":{}}` → `{"id":1,"ok":true,"result":…}`.
|
||||
//! See `vectorcraft_ui_egui::control` for the methods.
|
||||
//! See `vectorcraft_ui_egui::control` for the methods. `--automation-read-root` and
|
||||
//! `--automation-write-root` (or `VECTORCRAFT_AUTOMATION_READ_ROOT` / `_WRITE_ROOT`) confine the
|
||||
//! files its requests read and write (`vectorcraft_engine::file_access`); the person at the
|
||||
//! keyboard isn't confined.
|
||||
#![cfg_attr(all(target_os = "windows", not(debug_assertions)), windows_subsystem = "windows")]
|
||||
|
||||
#[cfg(all(feature = "windows7", any(feature = "wgpu", feature = "accessibility")))]
|
||||
@@ -36,6 +40,7 @@ mod window;
|
||||
|
||||
use vectorcraft_engine::Session;
|
||||
use vectorcraft_engine::cmd::fileio;
|
||||
use vectorcraft_engine::file_access::{self, AutomationRoots};
|
||||
use vectorcraft_ui_egui::graphics::GraphicsLoss;
|
||||
use vectorcraft_ui_egui::picks::PickRequest;
|
||||
use vectorcraft_ui_egui::{ClipboardProbeFactory, FilePick, Services, VectorcraftApp};
|
||||
@@ -51,7 +56,39 @@ struct App {
|
||||
}
|
||||
|
||||
impl eframe::App for App {
|
||||
fn logic(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) {
|
||||
// Each frame runs unconfined: it is the person at the keyboard's. The app confines what the
|
||||
// control channel asks for, and the frames carrying input it injected, to the automation roots
|
||||
// (`VectorcraftApp::with_automation_roots`), which are in force elsewhere in the process.
|
||||
fn logic(&mut self, ctx: &egui::Context, frame: &mut eframe::Frame) {
|
||||
file_access::unconfined(|| self.logic_frame(ctx, frame));
|
||||
}
|
||||
fn raw_input_hook(&mut self, _ctx: &egui::Context, raw: &mut egui::RawInput) {
|
||||
#[cfg(target_os = "macos")]
|
||||
if self.app.services.native_menu.is_some() {
|
||||
mac_menu::raw_input_hook(raw);
|
||||
}
|
||||
file_access::unconfined(|| self.app.raw_input_hook(raw));
|
||||
}
|
||||
fn ui(&mut self, ui: &mut egui::Ui, _frame: &mut eframe::Frame) {
|
||||
file_access::unconfined(|| self.app.ui(ui));
|
||||
#[cfg(target_os = "macos")]
|
||||
if self.app.take_ime_discard() {
|
||||
discard_marked_text();
|
||||
}
|
||||
}
|
||||
#[cfg(not(feature = "windows7"))]
|
||||
fn on_exit(&mut self) {
|
||||
file_access::unconfined(|| save_prefs(&self.app));
|
||||
end_before_teardown(self.frames);
|
||||
}
|
||||
#[cfg(feature = "windows7")]
|
||||
fn on_exit(&mut self, _gl: Option<&eframe::glow::Context>) {
|
||||
file_access::unconfined(|| save_prefs(&self.app));
|
||||
}
|
||||
}
|
||||
|
||||
impl App {
|
||||
fn logic_frame(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) {
|
||||
self.frames = ctx.cumulative_frame_nr();
|
||||
if let Some(why) = self.graphics_loss.take() {
|
||||
// eframe can't give a window a new device: the user saves and starts again.
|
||||
@@ -76,29 +113,6 @@ impl eframe::App for App {
|
||||
ctx.send_viewport_cmd(egui::ViewportCommand::Close);
|
||||
}
|
||||
}
|
||||
fn raw_input_hook(&mut self, _ctx: &egui::Context, raw: &mut egui::RawInput) {
|
||||
#[cfg(target_os = "macos")]
|
||||
if self.app.services.native_menu.is_some() {
|
||||
mac_menu::raw_input_hook(raw);
|
||||
}
|
||||
self.app.raw_input_hook(raw);
|
||||
}
|
||||
fn ui(&mut self, ui: &mut egui::Ui, _frame: &mut eframe::Frame) {
|
||||
self.app.ui(ui);
|
||||
#[cfg(target_os = "macos")]
|
||||
if self.app.take_ime_discard() {
|
||||
discard_marked_text();
|
||||
}
|
||||
}
|
||||
#[cfg(not(feature = "windows7"))]
|
||||
fn on_exit(&mut self) {
|
||||
save_prefs(&self.app);
|
||||
end_before_teardown(self.frames);
|
||||
}
|
||||
#[cfg(feature = "windows7")]
|
||||
fn on_exit(&mut self, _gl: Option<&eframe::glow::Context>) {
|
||||
save_prefs(&self.app);
|
||||
}
|
||||
}
|
||||
|
||||
/// On macOS with accessibility, end the process once the app has saved what it keeps on quitting,
|
||||
@@ -389,12 +403,19 @@ fn main() -> std::process::ExitCode {
|
||||
// VectorCraft's own Fonts folder, which text.addFontFiles copies fonts into.
|
||||
prefs_dir::install_fonts();
|
||||
let mut control_port: Option<u16> = std::env::var("VECTORCRAFT_CONTROL_PORT").ok().and_then(|p| p.parse().ok());
|
||||
let root_env = |name: &str| std::env::var_os(name).filter(|v| !v.is_empty()).map(std::path::PathBuf::from);
|
||||
let mut read_root = root_env("VECTORCRAFT_AUTOMATION_READ_ROOT");
|
||||
let mut write_root = root_env("VECTORCRAFT_AUTOMATION_WRITE_ROOT");
|
||||
let mut files = Vec::new();
|
||||
let mut in_window_menus = std::env::var_os("VECTORCRAFT_IN_WINDOW_MENUS").is_some_and(|v| !v.is_empty() && v != "0");
|
||||
let mut args = std::env::args().skip(1);
|
||||
while let Some(a) = args.next() {
|
||||
match a.as_str() {
|
||||
"--control" => control_port = args.next().and_then(|p| p.parse().ok()),
|
||||
"--automation-read-root" => read_root = args.next().map(std::path::PathBuf::from),
|
||||
"--automation-write-root" => write_root = args.next().map(std::path::PathBuf::from),
|
||||
_ if a.starts_with("--automation-read-root=") => read_root = a.split_once('=').map(|(_, d)| d.into()),
|
||||
_ if a.starts_with("--automation-write-root=") => write_root = a.split_once('=').map(|(_, d)| d.into()),
|
||||
"--in-window-menus" => in_window_menus = true,
|
||||
"--version" => {
|
||||
println!("vectorcraft {}", env!("CARGO_PKG_VERSION"));
|
||||
@@ -417,6 +438,23 @@ fn main() -> std::process::ExitCode {
|
||||
None => logger.no_file(),
|
||||
}
|
||||
}
|
||||
// The roots confine the control channel: a bad one keeps the app from starting rather than
|
||||
// leaving agents unconfined. Without a control port there is nothing to confine.
|
||||
let roots = if control_port.is_none() {
|
||||
if read_root.is_some() || write_root.is_some() {
|
||||
log::warn!("--automation-read-root and --automation-write-root confine the control channel: ignored without --control");
|
||||
}
|
||||
None
|
||||
} else {
|
||||
match AutomationRoots::new(read_root.as_deref(), write_root.as_deref()) {
|
||||
Ok(roots) => roots,
|
||||
Err(e) => {
|
||||
log::error!("{e}");
|
||||
eprintln!("vectorcraft: {e}");
|
||||
return std::process::ExitCode::FAILURE;
|
||||
}
|
||||
}
|
||||
};
|
||||
let saved = read_prefs();
|
||||
let saved_window = saved.as_ref().and_then(|ui| ui.window);
|
||||
#[cfg(feature = "wgpu")]
|
||||
@@ -532,7 +570,13 @@ fn main() -> std::process::ExitCode {
|
||||
app.custom_titlebar = CUSTOM_TITLEBAR;
|
||||
if let Some(port) = control_port {
|
||||
let rx = control_server::start(port, cc.egui_ctx.clone());
|
||||
app = app.with_control(rx);
|
||||
app = app.with_control(rx).with_automation_roots(roots.clone());
|
||||
if let Some(roots) = &roots {
|
||||
// Everywhere the app doesn't say whose work it is (worker threads), the
|
||||
// roots hold: unconfined is only what the person at the keyboard does.
|
||||
file_access::confine_process(roots.clone());
|
||||
log::info!("control channel confined to the automation roots: read {:?}, write {:?}", roots.read_root(), roots.write_root());
|
||||
}
|
||||
}
|
||||
#[cfg(target_os = "macos")]
|
||||
{
|
||||
@@ -544,7 +588,7 @@ fn main() -> std::process::ExitCode {
|
||||
}
|
||||
#[cfg(not(target_os = "macos"))]
|
||||
let _ = in_window_menus;
|
||||
open_files(&mut app, files);
|
||||
file_access::unconfined(|| open_files(&mut app, files));
|
||||
Ok(Box::new(App { app, graphics_loss, graphics_lost: false, frames: 0 }))
|
||||
}),
|
||||
)
|
||||
|
||||
Reference in new issue
Block a user