No remote string becomes a local path (#247)

- IRC METADATA avatars are kept only as web URLs: anybody could publish
  file:// or a bare path and have it drawn as a picture.
- A Matrix invite's room avatar is kept only as an mxc address: the
  inviter writes that state, and anyone can invite anyone.
- Sneedchat's cached attachments are attachments, not file:// links
  written into the message text; rows stored the old way are moved
  across once at startup (scrollback version 2), so the window can take
  no local path from text at all.
- Sign-in QR codes and voice messages are written under the cache's own
  transient folder rather than the system temp directory, which the
  window may no longer read; the Discord QR's name is sanitised like
  Matrix's.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

src/backend/discord/login.rs

@@ -91,7 +91,8 @@ pub(super) async fn run_qr_login(state: &AppState, login_id: &str, reauth_accoun
     // directly. Cleaned up on every exit path (success/error/panic) via
     // this drop guard, including the panic case: unwinding still runs
     // destructors for values already on the stack.
-    let qr_path = std::env::temp_dir().join(format!("nobilis-discord-qr-{login_id}.png"));
+    let safe_id = login_id.replace(|c: char| !c.is_ascii_alphanumeric(), "-");
+    let qr_path = crate::media_cache::transient_dir().join(format!("nobilis-discord-qr-{safe_id}.png"));
     struct RemoveOnDrop(std::path::PathBuf);
     impl Drop for RemoveOnDrop {
         fn drop(&mut self) {

src/backend/matrix/qrlogin.rs

@@ -329,7 +329,7 @@ fn failure(reason: &str) -> Value {
 }
 
 fn qr_path(login_id: &str) -> std::path::PathBuf {
-    std::env::temp_dir().join(format!("moho-matrix-qr-{}.png", login_id.replace(|c: char| !c.is_ascii_alphanumeric(), "-")))
+    crate::media_cache::transient_dir().join(format!("moho-matrix-qr-{}.png", login_id.replace(|c: char| !c.is_ascii_alphanumeric(), "-")))
 }
 
 /// The code as an image. Binary data in the QR code's byte mode, which is

src/backend/matrix/rooms.rs

@@ -215,7 +215,11 @@ pub fn invite_summary(room_id: &str, own_user_id: &str, events: &[&Value]) -> se
                 }
             }
             "m.room.avatar" => {
-                if let Some(u) = event["content"]["url"].as_str().filter(|u| !u.is_empty()) {
+                // Only a Matrix content address. The inviter writes this state
+                // and anyone can invite anyone, so a `file://` or a bare path
+                // here would be a stranger choosing a file on this computer
+                // for the window to load (#247).
+                if let Some(u) = event["content"]["url"].as_str().filter(|u| u.starts_with("mxc://")) {
                     avatar = Some(u.to_string());
                 }
             }

src/backend/sneedchat/media.rs

@@ -478,9 +478,31 @@ pub(super) fn spawn_attachment_resolve(state: AppState, http: http::HttpClient,
             }
         };
 
+        // As an attachment, not written into the text. The text is the
+        // sender's, and the window takes no local path from text at all
+        // (#247) - so a cached file goes where every other protocol puts one,
+        // and the link it replaces leaves the text, as the old rewrite did.
         if let Some(local_url) = local_url {
-            let new_body = body.replace(&matched, &local_url);
-            state.runtime.update_message_body_only(&state, &buffer_id, &msg_id, &new_body);
+            let kind = if matches!(ext.as_str(), "mp4" | "webm" | "mov" | "mkv") { "video" } else { "image" };
+            let page = matched.find("/attachments/").map(|at| format!("https://{KIWIFARMS_CLEARNET_HOST}{}", &matched[at..]));
+            let attachment = crate::model::Attachment {
+                kind: kind.to_string(),
+                filename: Some(format!("{id}.{ext}")),
+                path: Some(local_url),
+                url: page,
+                ..Default::default()
+            };
+            let new_body = body.replace(&matched, "");
+            state.runtime.update_message_body_only(&state, &buffer_id, &msg_id, new_body.trim());
+            let attachments = vec![attachment];
+            if let Err(e) = state.store.update_message_attachments(&buffer_id, &msg_id, &attachments) {
+                tracing::debug!("sneedchat: recording attachment {id}: {e}");
+                return;
+            }
+            state.events.emit(
+                "messageUpdated",
+                serde_json::json!({ "bufferId": buffer_id, "id": msg_id, "edited": false, "attachments": attachments }),
+            );
         }
     });
 }

src/media_cache.rs

@@ -43,6 +43,9 @@ pub const CACHES: &[Cache] = &[
     Cache { dir: "kick-emotes", group: "emotes", max_bytes: 64 * MB },
     Cache { dir: "discord-stickers", group: "emotes", max_bytes: 32 * MB },
     Cache { dir: "discord-sounds", group: "emotes", max_bytes: 16 * MB },
+    // QR codes and voice messages on their way out; normally removed as soon
+    // as they are used, and swept in case something was not.
+    Cache { dir: "transient", group: "media", max_bytes: 16 * MB },
 ];
 
 /// How long a cached file is kept after it was written, whatever the size.
@@ -65,6 +68,18 @@ pub fn dir(name: &str) -> PathBuf {
     root().join(name)
 }
 
+/// Where something drawn once and thrown away is written - a sign-in QR code,
+/// a voice message on its way to being sent - created if need be.
+///
+/// Inside the cache rather than the system temp directory, because the window
+/// is only allowed to read the daemon's named cache folders: the temp
+/// directory as a whole holds every program's files, and was once served.
+pub fn transient_dir() -> PathBuf {
+    let dir = dir("transient");
+    let _ = std::fs::create_dir_all(&dir);
+    dir
+}
+
 static STORE: OnceLock<Arc<Store>> = OnceLock::new();
 static DATA_DIR: OnceLock<PathBuf> = OnceLock::new();
 

src/model.rs

@@ -188,6 +188,17 @@ pub struct SneedChatRoomInfo {
 
 /// Matches Buffer JSON (daemon/nobilis/model.c's nobilis_buffer_json).
 /// `id` = "<accountId>|<name>"; `kind` is "channel"|"dm"|"server".
+/// Whether a URL some other party supplied points at the web.
+///
+/// The test for anything a remote sender controls that will be drawn or
+/// fetched - an avatar, a picture link. Local paths are only ever ones the
+/// daemon wrote itself (#247), so a sender's `file://`, a bare path or any
+/// other scheme is not a URL to pass on.
+pub fn is_web_url(url: &str) -> bool {
+    let lower = url.trim_start().to_ascii_lowercase();
+    (lower.starts_with("https://") || lower.starts_with("http://")) && !url.chars().any(char::is_control)
+}
+
 /// Why a buffer is not receiving, when the account itself is fine.
 ///
 /// A Sneedchat room has its own connection, an IRC channel can refuse the

src/oggopus.rs

@@ -224,7 +224,7 @@ pub fn to_mono(interleaved: &[f32], channels: u16) -> Vec<f32> {
 
 /// Writes the encoded recording somewhere it can be uploaded from.
 pub fn write_temp(bytes: &[u8]) -> Result<std::path::PathBuf> {
-    let dir = std::env::temp_dir().join("moho-voice");
+    let dir = crate::media_cache::transient_dir();
     std::fs::create_dir_all(&dir).with_context(|| format!("creating {}", dir.display()))?;
     let path = dir.join(format!("voice-message-{}.ogg", rand_serial()));
     std::fs::write(&path, bytes).with_context(|| format!("writing {}", path.display()))?;

src/runtime.rs

@@ -3468,6 +3468,11 @@ impl Runtime {
     /// Records one published fact. An absent value clears the key, which is
     /// somebody taking their avatar down rather than never having had one.
     pub fn set_irc_metadata(&self, account_id: &str, nick: &str, key: &str, value: Option<String>) {
+        // Anybody can publish any string as their avatar, and it is drawn as a
+        // picture. Only a web address is kept: a `file://` URL or a bare path
+        // would have the window load a file from this computer on somebody
+        // else's say-so (#247).
+        let value = if key == "avatar" { value.filter(|v| crate::model::is_web_url(v)) } else { value };
         let mut all = self.irc_metadata.lock().unwrap();
         let people = all.entry(account_id.to_string()).or_default();
         let nick = nick.to_ascii_lowercase();

src/store.rs

@@ -297,6 +297,23 @@ impl Store {
             let _ = conn.execute_batch("PRAGMA user_version = 1;");
         }
 
+        // One-time move of Sneedchat's cached attachments out of the text.
+        //
+        // They used to be written into the message body as a `file://` link,
+        // and the window drew any such link as a picture - which let anybody
+        // type one and have the reader's computer load a file of their
+        // choosing (#247). The window now takes no local path from text, and
+        // a cached file is an attachment; rows written before that are moved
+        // across here so their pictures still show.
+        if version < 2 {
+            match move_cached_links_to_attachments(&conn) {
+                Ok(n) if n > 0 => tracing::info!("scrollback: moved {n} cached Sneedchat picture(s) from message text to attachments"),
+                Ok(_) => {}
+                Err(e) => tracing::warn!("scrollback: could not move cached pictures to attachments: {e}"),
+            }
+            let _ = conn.execute_batch("PRAGMA user_version = 2;");
+        }
+
         Ok(Self { conn: Mutex::new(conn) })
     }
 
@@ -505,7 +522,10 @@ impl Store {
         let mut conn = self.conn.lock().unwrap();
         let tx = conn.transaction()?;
         let bodies: Vec<String> = {
-            let mut stmt = tx.prepare("SELECT body FROM messages WHERE buffer_id = ?1")?;
+            // The attachments as well as the text: a cached Sneedchat file is
+            // named by its attachment now, and only in the text in rows
+            // written before that (see the version 2 migration).
+            let mut stmt = tx.prepare("SELECT body || char(10) || attachments FROM messages WHERE buffer_id = ?1")?;
             let rows = stmt.query_map(params![buffer_id], |row| row.get::<_, String>(0))?;
             rows.collect::<std::result::Result<_, _>>()?
         };
@@ -1282,6 +1302,70 @@ impl Store {
     }
 }
 
+
+/// Takes the daemon's own cached-file links out of stored message text and
+/// makes them attachments - see the version 2 migration in `Store::open`.
+///
+/// Only links into the Sneedchat attachment cache, which only the daemon ever
+/// wrote. Any other `file://` in a body was typed by somebody and stays text.
+fn move_cached_links_to_attachments(conn: &Connection) -> Result<usize> {
+    let rows: Vec<(i64, String, String)> = {
+        let mut stmt = conn.prepare(
+            "SELECT id, body, attachments FROM messages WHERE body LIKE '%file://%/sneedchat-attachments/%'",
+        )?;
+        let rows = stmt.query_map([], |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?)))?;
+        rows.collect::<std::result::Result<_, _>>()?
+    };
+    let mut moved = 0;
+    for (id, body, attachments) in rows {
+        let (text, found) = cached_links_in(&body);
+        if found.is_empty() {
+            continue;
+        }
+        let mut list: Vec<crate::model::Attachment> = serde_json::from_str(&attachments).unwrap_or_default();
+        list.extend(found);
+        conn.execute(
+            "UPDATE messages SET body = ?1, attachments = ?2 WHERE id = ?3",
+            params![text, serde_json::to_string(&list)?, id],
+        )?;
+        moved += 1;
+    }
+    Ok(moved)
+}
+
+/// The text without its cached-attachment links, and those links as
+/// attachments.
+fn cached_links_in(body: &str) -> (String, Vec<crate::model::Attachment>) {
+    let mut text = String::with_capacity(body.len());
+    let mut found = Vec::new();
+    let mut rest = body;
+    while let Some(at) = rest.find("file://") {
+        text.push_str(&rest[..at]);
+        let tail = &rest[at..];
+        let end = tail.find(|c: char| c.is_whitespace() || c == '[' || c == ']').unwrap_or(tail.len());
+        let link = &tail[..end];
+        let path = link.trim_start_matches("file://");
+        let parent = std::path::Path::new(path).parent().and_then(|p| p.file_name()).and_then(|n| n.to_str());
+        let name = std::path::Path::new(path).file_name().and_then(|n| n.to_str()).unwrap_or_default();
+        if parent == Some("sneedchat-attachments") && !name.is_empty() {
+            let ext = name.rsplit('.').next().unwrap_or_default().to_ascii_lowercase();
+            found.push(crate::model::Attachment {
+                kind: if matches!(ext.as_str(), "mp4" | "webm" | "mov" | "mkv") { "video" } else { "image" }.to_string(),
+                filename: Some(name.to_string()),
+                path: Some(link.to_string()),
+                ..Default::default()
+            });
+        } else {
+            text.push_str(link);
+        }
+        rest = &tail[end..];
+    }
+    text.push_str(rest);
+    // An [img] that held only the link is now empty; it would draw nothing.
+    let text = text.replace("[img][/img]", "").replace("[IMG][/IMG]", "");
+    (text.trim().to_string(), found)
+}
+
 #[cfg(test)]
 mod tests {
     use super::{drop_missing_local_copies, Store};
@@ -1828,4 +1912,21 @@ mod dedupe_tests {
         assert_eq!(s.get_backlog("acct|#one", 0, 100).unwrap().len(), 1);
         assert_eq!(s.get_backlog("acct|#two", 0, 100).unwrap().len(), 1);
     }
+
+    /// The daemon's own cached-picture links leave the text and become
+    /// attachments; a `file://` somebody typed stays as text.
+    #[test]
+    fn cached_links_move_and_typed_ones_stay() {
+        let body = "look [img]file:///home/a/.cache/nobilis/sneedchat-attachments/9623396.jpeg[/img] and file:///etc/passwd";
+        let (text, found) = cached_links_in(body);
+        assert_eq!(text, "look  and file:///etc/passwd");
+        assert_eq!(found.len(), 1);
+        assert_eq!(found[0].kind, "image");
+        assert_eq!(found[0].filename.as_deref(), Some("9623396.jpeg"));
+        assert_eq!(found[0].path.as_deref(), Some("file:///home/a/.cache/nobilis/sneedchat-attachments/9623396.jpeg"));
+
+        let (same, none) = cached_links_in("nothing cached here");
+        assert_eq!(same, "nothing cached here");
+        assert!(none.is_empty());
+    }
 }