Merge pull request #1697 from saimskywalker/issue-1095-roots-refuse-network

M8: commands confined to automation roots reach no server (#1095)

crates/engine/src/assets/library_server.rs

@@ -160,6 +160,12 @@ impl Session {
             Some(Ok(lib)) => Some(lib.manifest.version),
             _ => None,
         };
+        // Confined to automation roots, the server isn't asked (#1095). That is no offline
+        // failure: the last version fetched doesn't stand in for an answer, so `library.updates`
+        // and `library.list` say the check was refused instead of reporting no updates.
+        if let Some(why) = crate::file_access::network_refused() {
+            return Err(EngineError::Other(why));
+        }
         let usage = self.doc().ok().map(|d| {
             let id = d.libraries.enabled.iter().find(|l| l.path == path).map(|l| l.id.clone()).unwrap_or_default();
             usage(d, &id)
@@ -213,7 +219,11 @@ impl Session {
                 std::iter::once((l.path.clone(), LibraryReq::Fetch { have, usage: Some(usage(doc, &l.id)) })).chain(report)
             })
             .collect();
-        if let (false, Some(net)) = (jobs.is_empty(), self.live.link()) {
+        // Confined to automation roots, nothing reaches a server (#1095).
+        if jobs.is_empty() || crate::file_access::network_refused().is_some() {
+            return;
+        }
+        if let Some(net) = self.live.link() {
             net.library_background(jobs);
         }
     }

crates/engine/src/automation/file_access.rs

@@ -218,6 +218,18 @@ pub fn current() -> Option<Arc<Roots>> {
     ROOTS.with(|r| r.borrow().clone())
 }
 
+/// Why this thread's work may not reach the network now, if it may not (#1095): the automation
+/// roots keep an agent to the folders it was given, so while they confine it nothing is published,
+/// hosted, joined or fetched from a server either. The engine reaches the network only through the
+/// live link (`crate::live::Lives`), which asks this first.
+pub fn network_refused() -> Option<String> {
+    current().map(|_| NETWORK_REFUSED.to_string())
+}
+
+/// What a confined command that would reach the network says.
+pub const NETWORK_REFUSED: &str =
+    "automation roots are set: commands don't reach the network (publishing, live sessions, team libraries and plugin directories are off)";
+
 /// The path to use for `access` to `path`: resolved when confined, else as given.
 pub fn allow(path: impl AsRef<Path>, access: Access) -> Result<PathBuf> {
     let path = path.as_ref();

crates/engine/src/cmd/library.rs

@@ -594,6 +594,11 @@ fn check(s: &mut Session, p: &Value) -> Result<Value> {
         }
         None => doc.libraries.enabled.iter().filter(|l| is_server_link(&l.path)).map(|l| l.path.clone()).collect(),
     };
+    // Confined to automation roots, nothing is checked (#1095): say so, and don't remember the
+    // libraries as yours either.
+    if let (false, Some(why)) = (links.is_empty(), crate::file_access::network_refused()) {
+        return Err(EngineError::Disabled(cmd.to_string(), why));
+    }
     for l in &links {
         s.allow_library(l);
     }

crates/engine/src/library_beacon_tests.rs

@@ -115,3 +115,27 @@ fn checking_one_team_library_by_name() {
     assert_eq!(sent(&wire), vec![LINK.to_string()]);
     let _ = std::fs::remove_file(path);
 }
+
+/// Confined to automation roots (#1095), checking says it was refused instead of counting
+/// libraries it never asked about, and doesn't remember them as yours: unconfined later, they are
+/// still not asked by themselves.
+#[test]
+fn a_confined_check_is_refused_before_the_libraries_are_remembered() {
+    let (mut s, wire, path) = opened(Prefs::default(), "confined");
+    let dir = std::env::temp_dir().join(format!("figcraft-beacon-roots-{}", std::process::id()));
+    std::fs::create_dir_all(&dir).unwrap();
+    let roots = Arc::new(crate::file_access::Roots::new(&[], &[dir.to_string_lossy().into_owned()]).unwrap());
+    crate::file_access::confined(Some(&roots), || {
+        for p in [json!({}), json!({"library": "Kit"})] {
+            let e = s.execute("library.check", &p).unwrap_err();
+            assert!(matches!(e, crate::EngineError::Disabled(..)), "{e}");
+            assert!(e.to_string().contains(crate::file_access::NETWORK_REFUSED), "{e}");
+        }
+    });
+    assert!(sent(&wire).is_empty(), "nothing was asked: {:?}", sent(&wire));
+    assert!(!s.library_allowed(LINK) && !s.live.unsaved, "not remembered");
+    s.refresh_server_libraries();
+    assert!(sent(&wire).is_empty());
+    let _ = std::fs::remove_dir_all(&dir);
+    let _ = std::fs::remove_file(path);
+}

crates/engine/src/sharing/background.rs

@@ -128,6 +128,9 @@ impl Session {
     /// answered when it was an earlier one (what it carried then).
     pub(crate) fn net(&mut self, req: NetRequest) -> Result<(NetReply, Option<NetRequest>)> {
         let Some(link) = self.live.link() else { return Err(EngineError::Other(req.unavailable().into())) };
+        if let Some(why) = crate::file_access::network_refused() {
+            return Err(EngineError::Other(why));
+        }
         if let Some(run) = self.background.run.as_mut() {
             let earlier = run.answers.iter().zip(run.used.iter_mut()).find(|((sent, _), used)| !**used && sent.stands_for(&req));
             if let Some(((sent, reply), used)) = earlier {

crates/engine/src/sharing/background/tests.rs

@@ -270,3 +270,65 @@ fn a_link_without_background_requests_is_waited_on() {
     let mut bare = Session::new();
     assert!(bare.start_background("plugin.directory.list", json!({"server": SERVER})).unwrap_err().to_string().contains("desktop app"));
 }
+
+/// Confined to automation roots, commands reach no server (#1095): browsing a plugin directory,
+/// hosting, joining and publishing are refused before the link is asked. Unconfined, the same
+/// request reaches the server as before.
+#[test]
+fn commands_confined_to_automation_roots_reach_no_server() {
+    let dir = std::env::temp_dir().join(format!("figcraft-roots-network-{}", std::process::id()));
+    std::fs::create_dir_all(&dir).unwrap();
+    let roots = Arc::new(crate::file_access::Roots::new(&[], &[dir.to_string_lossy().into_owned()]).unwrap());
+    let (mut s, wire) = session(false);
+    let refused = |r: Result<serde_json::Value, EngineError>| {
+        let e = r.unwrap_err().to_string();
+        assert!(e.contains(crate::file_access::NETWORK_REFUSED), "{e}");
+    };
+    crate::file_access::confined(Some(&roots), || {
+        refused(s.execute("plugin.directory.list", &json!({"server": SERVER})));
+        refused(s.execute("collab.host", &json!({})));
+        refused(s.execute("collab.join", &json!({"link": LINK})));
+        refused(s.execute("collab.publish", &json!({"server": SERVER})));
+    });
+    assert_eq!(wire.lock().unwrap().waited, 0, "the server was never asked");
+    assert!(s.live.iter().next().is_none(), "no session was started");
+    let _ = std::fs::remove_dir_all(&dir);
+
+    let r = s.execute("plugin.directory.list", &json!({"server": SERVER})).unwrap();
+    assert_eq!(r["plugins"][0]["id"], ID);
+    assert_eq!(wire.lock().unwrap().waited, 1);
+    let e = s.execute("collab.host", &json!({})).unwrap_err().to_string();
+    assert!(!e.contains(crate::file_access::NETWORK_REFUSED), "unconfined, the link is asked: {e}");
+}
+
+/// A team library fetched before stays cached, but confined to automation roots (#1095) checking
+/// it for updates is refused, and `library.updates` and `library.list` say so instead of
+/// answering from the cache as if the server had nothing newer.
+#[test]
+fn a_confined_update_check_reports_the_refusal_not_the_cached_library() {
+    let (mut s, wire) = session(false);
+    wire.lock().unwrap().library = Some(published());
+    s.execute("library.add", &json!({"path": LINK})).unwrap();
+    assert!(matches!(s.libraries.cached(LINK), Some(Ok(_))));
+    let waited = wire.lock().unwrap().waited;
+    let dir = std::env::temp_dir().join(format!("figcraft-roots-updates-{}", std::process::id()));
+    std::fs::create_dir_all(&dir).unwrap();
+    let roots = Arc::new(crate::file_access::Roots::new(&[], &[dir.to_string_lossy().into_owned()]).unwrap());
+    crate::file_access::confined(Some(&roots), || {
+        let u = s.execute("library.updates", &json!({})).unwrap();
+        assert_eq!(u["count"], 0);
+        let e = u["errors"][0]["error"].as_str().unwrap();
+        assert!(e.contains(crate::file_access::NETWORK_REFUSED), "{u}");
+        let l = s.execute("library.list", &json!({})).unwrap();
+        let e = l["libraries"][0]["error"].as_str().unwrap();
+        assert!(e.contains(crate::file_access::NETWORK_REFUSED), "{l}");
+    });
+    assert_eq!(wire.lock().unwrap().waited, waited, "the server was never asked");
+    assert!(matches!(s.libraries.cached(LINK), Some(Ok(_))), "the cached library is kept");
+    let _ = std::fs::remove_dir_all(&dir);
+
+    // Unconfined, the same check reaches the server and finds nothing new.
+    let u = s.execute("library.updates", &json!({})).unwrap();
+    assert_eq!((u["count"].clone(), u["errors"].as_array().map(Vec::len)), (json!(0), Some(0)), "{u}");
+    assert_eq!(wire.lock().unwrap().waited, waited + 1);
+}

crates/engine/src/sharing/live.rs

@@ -274,6 +274,9 @@ impl Lives {
     /// session's id, which comes back with what it returned.
     fn start<T>(&mut self, cmd: &str, start: impl FnOnce(&mut dyn LiveLink, SessionId) -> Result<T>) -> Result<(SessionId, T)> {
         let link = self.link.as_deref_mut().ok_or_else(|| unavailable(cmd))?;
+        if let Some(why) = crate::file_access::network_refused() {
+            return Err(EngineError::Disabled(cmd.to_string(), why));
+        }
         let id = self.last.saturating_add(1);
         let out = start(link, id)?;
         self.last = id;

crates/engine/src/sharing/team.rs

@@ -99,7 +99,7 @@ impl Session {
     pub(crate) fn team_refresh(&mut self) -> Result<()> {
         let server = plain_server(&self.prefs.publish_server);
         let public = option_env!("FIGCRAFT_DEFAULT_SERVER").is_none() && same_server(&server, DEFAULT_SERVER);
-        if server.is_empty() || public || self.live.link().is_none() {
+        if server.is_empty() || public || self.live.link().is_none() || crate::file_access::network_refused().is_some() {
             self.live.team = None;
             return Ok(());
         }

docs/control-protocol.md

@@ -93,7 +93,11 @@ read or write root and written only inside a write root; anything else is an ord
 resolved as `docs/mcp.md` › Confining file access describes. A root that isn't an existing folder
 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
+Confined work reaches no FigCraft or plugin server either: hosting, joining, publishing and
+sharing files, team libraries on a server and plugin directories are refused (`automation roots
+are set: commands don't reach the network …`). A root on a network share (a UNC path such as
+`\\server\share\…`, or a mounted network drive) is the exception: files read and written inside it,
+documents saved there included, go to that file server. 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

docs/mcp.md

@@ -93,6 +93,16 @@ Each may be given more than once, as `--flag DIR` or `--flag=DIR`:
   compared by whole names: `/work2/x` is not inside `/work`. On Windows the case comes from the disk (`C:\WORK\x` is
   inside `C:\work`), `\\?\` and UNC (`\\server\share\…`) paths are resolved like drive-letter ones,
   and new names that are devices (`CON`, `NUL`, `COM1`…) or streams (`a.png:x`) are refused.
+- **The network:** a confined call reaches no FigCraft or plugin server, so it can't send a
+  document anywhere through one. Hosting, joining, publishing and sharing a file (`collab.host`,
+  `collab.join`, `collab.publish`, `collab.share` …), team libraries on a server (`library.check`
+  refuses, and `library.updates` / `library.list` report the refusal for each instead of the last
+  version fetched) and plugin directories are refused with `automation roots are set: commands
+  don't reach the network …`. The app started with the flags confines its own window too, so its
+  sharing is off as well. One exception: a root on a network share (a UNC path such as
+  `\\server\share\…`, or a mounted network drive) is still a root, so reading and writing inside
+  it reaches that file server, and saving there sends the document to it. Give only roots you would
+  let the agent's files go to.
 - Not confined: the app's own folders, which no tool names (preferences, Data Recovery copies,
   plugins installed from a plugin directory, the system fonts).
 - `--attach` with either flag is a usage error: the app runs the commands, so only the app can