From b627db761b2aab654b36ae50a2478e08cda83d45 Mon Sep 17 00:00:00 2001 From: xavix-yo Date: Mon, 20 Jul 2026 23:04:24 +0100 Subject: [PATCH] scope file ops (ws, api, tools) to per-user context --- crates/core-api/src/events.rs | 10 +-- crates/core-api/src/user_fs.rs | 59 ++++++++++++++++ crates/skald-core/src/tools/fs/mod.rs | 23 +++++++ crates/skald-core/src/tools/show_file.rs | 35 +++++++--- src/frontend/api/file_watch.rs | 48 +++++++++---- src/frontend/api/files.rs | 88 ++++++++++++++++-------- src/frontend/api/ws.rs | 1 + 7 files changed, 206 insertions(+), 58 deletions(-) diff --git a/crates/core-api/src/events.rs b/crates/core-api/src/events.rs index f4af363..e1bb435 100644 --- a/crates/core-api/src/events.rs +++ b/crates/core-api/src/events.rs @@ -149,10 +149,12 @@ pub enum ServerEvent { FileChanged { path: String, }, - /// Ask the frontend to open a file for the user. Behaves like - /// `window.openFile(path)`: navigates to the file viewer page for markdown / - /// text / images, or opens an HTML file in a new browser tab. Emitted by - /// the future `show_file_to_user` interface tool (not wired yet). + /// Ask the frontend to open a file for the user, via `window.openFile(path)`: + /// the file-viewer page renders every kind (markdown / text / images / SVG / + /// PDF / LaTeX compiled server-side, and HTML live in an origin-isolated + /// iframe). Emitted by the `show_file_to_user` interface tool; `path` is the + /// caller's canonical **agent path** (`~/…`, `shared/{X}/…`, `projects/…`), + /// which the viewer fetches back through `GET /api/file`. OpenFile { path: String, }, diff --git a/crates/core-api/src/user_fs.rs b/crates/core-api/src/user_fs.rs index 1c3f745..81991c9 100644 --- a/crates/core-api/src/user_fs.rs +++ b/crates/core-api/src/user_fs.rs @@ -161,6 +161,65 @@ impl UserFs { let stripped = strip_home_prefix(agent_path); normalize(&self.container_home.join(stripped)) } + + /// Reverse of [`to_container`](Self::to_container) for an already-absolute path: + /// map a **container-absolute** path (`/root/…`, `/root/shared/{X}/…`, + /// `/root/projects/{O}/{S}/…`) back to the agent vocabulary. Shared and project + /// mounts nest *under* `container_home`, so they are matched **first** — otherwise + /// `/root/shared/X` would strip against the home base and mis-route. + /// + /// Returns `None` when `abs` lies outside every one of this user's container mounts + /// (i.e. it points outside their view) — the caller rejects it fail-closed. Purely + /// lexical: no membership check, no filesystem access. + pub fn container_to_agent(&self, abs: &Path) -> Option { + let abs = normalize(abs); + for m in &self.shared { + if let Ok(tail) = abs.strip_prefix(&m.container) { + return Some(agent_join(&format!("shared/{}", m.name), tail)); + } + } + for m in &self.projects { + if let Ok(tail) = abs.strip_prefix(&m.container) { + return Some(agent_join(&format!("projects/{}/{}", m.owner_username, m.slug), tail)); + } + } + abs.strip_prefix(&self.container_home) + .ok() + .map(|tail| agent_join("~", tail)) + } + + /// Normalize any path arriving from the show-file / file-viewer surface into a + /// **canonical agent path** the UI can display and echo back: a relative or `~/…` + /// path is cleaned and rooted (`report.md` → `~/report.md`, `shared/X/y` and + /// `projects/O/S/y` keep their root); a container-absolute path is reverse-mapped + /// via [`container_to_agent`](Self::container_to_agent). + /// + /// Returns `None` only for an absolute path outside every container mount — the + /// caller rejects it fail-closed. Purely lexical (`.`/`..` collapse, `..` clamps at + /// the root); membership + on-disk containment are enforced later, in skald-core. + pub fn to_agent_display(&self, input: &str) -> Option { + let p = Path::new(input); + if p.is_absolute() { + return self.container_to_agent(p); + } + let cleaned = normalize(Path::new(strip_home_prefix(input))); + let cleaned = cleaned.to_string_lossy().replace('\\', "/"); + let root = cleaned.split('/').next().unwrap_or(""); + if root == "shared" || root == "projects" { + Some(cleaned) + } else if cleaned.is_empty() { + Some("~".to_string()) + } else { + Some(format!("~/{cleaned}")) + } + } +} + +/// Join an agent-path base (`~`, `shared/X`, `projects/O/S`) with a tail relative to +/// the mount, normalizing separators. An empty tail yields the bare base. +fn agent_join(base: &str, tail: &Path) -> String { + let t = tail.to_string_lossy().replace('\\', "/"); + if t.is_empty() { base.to_string() } else { format!("{base}/{t}") } } /// Strips a leading `~/`, bare `~`, or `./` so what remains is relative to the home. diff --git a/crates/skald-core/src/tools/fs/mod.rs b/crates/skald-core/src/tools/fs/mod.rs index 7f66acb..21dfb4a 100644 --- a/crates/skald-core/src/tools/fs/mod.rs +++ b/crates/skald-core/src/tools/fs/mod.rs @@ -212,6 +212,29 @@ pub(crate) fn resolve_host_path(fs: &UserFs, agent_path: &str) -> Result Result<(PathBuf, String)> { + if classify_memory(input).is_some() { + anyhow::bail!("memory notes can't be opened in the file viewer: {input}"); + } + let agent = fs.to_agent_display(input) + .ok_or_else(|| anyhow::anyhow!("path is outside your workspace: {input}"))?; + let host = resolve_host_path(fs, &agent)?; + Ok((host, agent)) +} + /// Rewrites the `path` argument of a physical fs-tool call to the resolved absolute /// host path, so the on-disk `execute` (which takes absolute paths as-is) acts on /// the caller's per-user workspace rather than the process working directory. diff --git a/crates/skald-core/src/tools/show_file.rs b/crates/skald-core/src/tools/show_file.rs index cb1789e..61d6103 100644 --- a/crates/skald-core/src/tools/show_file.rs +++ b/crates/skald-core/src/tools/show_file.rs @@ -2,22 +2,28 @@ use std::sync::Arc; use serde_json::{Value, json}; +use core_api::user_fs::SharedFs; + use crate::chat_hub::ChatHub; use crate::events::{GlobalEvent, ServerEvent}; use crate::session::handler::{InterfaceTool, ToolFuture}; use crate::tools::fs; use crate::tools::tool_names::SHOW_FILE_TO_USER; -/// Build a `show_file_to_user` InterfaceTool bound to a `ChatHub` and a source. +/// Build a `show_file_to_user` InterfaceTool bound to a `ChatHub`, a source and the +/// caller's [`SharedFs`] (their per-user filesystem view). /// /// Injected only for SPA clients (web copilot + mobile) at the WebSocket entry /// point, so Telegram — which has its own `send_attachment` — never sees it. /// -/// When called, it emits a `ServerEvent::OpenFile` to the source's connected -/// clients. The frontend routes it: HTML opens in a new browser tab, everything -/// else (Markdown / code / raster images / SVG / PDF / LaTeX — which is compiled -/// to PDF server-side) opens in the file-viewer page. -pub fn make_tool(hub: Arc, source: String) -> InterfaceTool { +/// The path is resolved through the caller's own workspace (`resolve_view_path`): +/// `~/…`, `shared/{X}/…`, `projects/{O}/{S}/…`, a bare relative path, or a +/// container-absolute `/root/…` — anything outside the container view is refused. +/// It then emits a `ServerEvent::OpenFile` carrying the **canonical agent path**, so +/// the file-viewer page fetches the same file back through `/api/file` (which applies +/// the identical per-user resolution). The frontend renders every kind in the viewer +/// (HTML live in an origin-isolated iframe; LaTeX compiled to PDF server-side). +pub fn make_tool(hub: Arc, source: String, fs: SharedFs) -> InterfaceTool { let definition = json!({ "type": "function", "function": { @@ -41,7 +47,10 @@ pub fn make_tool(hub: Arc, source: String) -> InterfaceTool { "properties": { "path": { "type": "string", - "description": "Path of the file to show. Relative to the project root, or absolute." + "description": "Path of the file to show, in your own workspace: relative to your \ + home (e.g. `report.md` or `~/report.md`), a `shared//…` or \ + `projects///…` path, or an absolute container path \ + (`/root/…`). Paths outside your workspace are refused." } }, "required": ["path"] @@ -52,20 +61,24 @@ pub fn make_tool(hub: Arc, source: String) -> InterfaceTool { let handler = Arc::new(move |args: Value| -> ToolFuture { let hub = Arc::clone(&hub); let source = source.clone(); + let fs = fs.clone(); Box::pin(async move { let path = args["path"] .as_str() .ok_or_else(|| anyhow::anyhow!("show_file_to_user: missing required parameter 'path'"))?; - let abs = fs::resolve(path)?; + // Resolve against the caller's workspace snapshot: gives the host path to + // stat and the canonical agent path the viewer will fetch back. + let user_fs = fs.load(); + let (abs, display) = fs::resolve_view_path(user_fs.as_ref(), path) + .map_err(|e| anyhow::anyhow!("show_file_to_user: {e}"))?; if !abs.exists() { - anyhow::bail!("show_file_to_user: file not found: {path}"); + anyhow::bail!("show_file_to_user: file not found: {display}"); } if abs.is_dir() { - anyhow::bail!("show_file_to_user: '{path}' is a directory, not a file"); + anyhow::bail!("show_file_to_user: '{display}' is a directory, not a file"); } - let display = fs::relativize_for_display(path); hub.emit(GlobalEvent { source: Some(source), session_id: None, diff --git a/src/frontend/api/file_watch.rs b/src/frontend/api/file_watch.rs index c1cf1ab..60b74ad 100644 --- a/src/frontend/api/file_watch.rs +++ b/src/frontend/api/file_watch.rs @@ -17,11 +17,13 @@ //! { "type": "error", "path": "...", "error": "..." } // watch install failed //! ``` //! -//! `path` is the original user-supplied string (relative or absolute) — it -//! round-trips unchanged so the client can match it against the path it asked -//! to watch. The backend resolves it to an absolute path via `fs_tools::resolve` -//! (same path model as `GET /api/file`), so absolute paths are used as-is and -//! relative paths resolve against Skald's process CWD (the data root). +//! `path` is the original client-supplied string (an agent path — `~/…`, +//! `shared/{X}/…`, `projects/{O}/{S}/…` — or a container-absolute `/root/…`) — it +//! round-trips unchanged so the client can match it against the path it asked to +//! watch. The backend resolves it to an absolute host path via +//! `fs_tools::resolve_view_path` scoped to the connection's authenticated user +//! (same path model as `GET /api/file`); a path outside that user's workspace is +//! refused fail-closed. //! //! One OS watcher per watched file per connection (no cross-connection //! sharing). On disconnect every watcher is dropped and the OS resources are @@ -49,12 +51,14 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; use axum::{ + Extension, extract::{ State, ws::{Message, WebSocket, WebSocketUpgrade}, }, response::IntoResponse, }; +use core_api::user_fs::SharedFs; use notify::{Config, RecommendedWatcher, RecursiveMode, Watcher}; use serde::Deserialize; use serde_json::json; @@ -64,11 +68,14 @@ use tracing::info; use skald_core::skald::Skald; use skald_core::tools::fs as fs_tools; +use super::guard::AuthUser; + pub async fn handler( - ws: WebSocketUpgrade, - State(skald): State>, + ws: WebSocketUpgrade, + Extension(auth): Extension, + State(skald): State>, ) -> impl IntoResponse { - ws.on_upgrade(move |socket| handle_socket(socket, skald)) + ws.on_upgrade(move |socket| handle_socket(socket, skald, auth.user_id)) } #[derive(Deserialize)] @@ -77,8 +84,21 @@ struct ClientMsg { path: String, } -async fn handle_socket(mut socket: WebSocket, skald: Arc) { - info!("file-watch WS connected"); +async fn handle_socket(mut socket: WebSocket, skald: Arc, user_id: String) { + // Resolve the caller's per-user filesystem view once at connect; every + // subscribe resolves its path against the current snapshot (`fs.load()`), so a + // membership change is picked up without dropping the connection. A missing + // context means the database re-locked — report and close. + let fs: SharedFs = match skald.user_context(&user_id).await { + Some(ctx) => ctx.fs.clone(), + None => { + let _ = send_json(&mut socket, + json!({ "type": "error", "error": "session expired — please log in again" }) + ).await; + return; + } + }; + info!(user = %user_id, "file-watch WS connected"); // Single mpsc into which every watcher callback forwards via an unbounded // sender (unbounded so the sync callback never blocks). @@ -112,7 +132,7 @@ async fn handle_socket(mut socket: WebSocket, skald: Arc) { ).await; continue; } - match install_watcher(&parsed.path, &change_tx, &mut watchers, &skald) { + match install_watcher(&parsed.path, &change_tx, &mut watchers, &skald, &fs) { Ok(()) => { let _ = send_json(&mut socket, json!({ "type": "subscribed", "path": parsed.path }) @@ -155,7 +175,7 @@ async fn handle_socket(mut socket: WebSocket, skald: Arc) { // path so they reflect the current dependency graph. if is_latex_path(&p) { if watchers.remove(&p).is_some() { - let _ = install_watcher(&p, &change_tx, &mut watchers, &skald); + let _ = install_watcher(&p, &change_tx, &mut watchers, &skald, &fs); } } } @@ -180,8 +200,10 @@ fn install_watcher( change_tx: &mpsc::UnboundedSender, watchers: &mut HashMap>, skald: &Skald, + fs: &SharedFs, ) -> Result<(), String> { - let abs: PathBuf = fs_tools::resolve(user_path).map_err(|e| e.to_string())?; + let (abs, _agent): (PathBuf, String) = + fs_tools::resolve_view_path(fs.load().as_ref(), user_path).map_err(|e| e.to_string())?; let paths_to_watch: Vec = if is_latex_path(user_path) { skald.latex_compiler().watch_paths_for(&abs) diff --git a/src/frontend/api/files.rs b/src/frontend/api/files.rs index 2d610a7..ff4d350 100644 --- a/src/frontend/api/files.rs +++ b/src/frontend/api/files.rs @@ -1,7 +1,7 @@ use std::path::Path; use axum::{ - Json, + Extension, Json, extract::{Query, State}, http::{HeaderValue, StatusCode, header}, response::{IntoResponse, Response}, @@ -13,6 +13,8 @@ use skald_core::skald::Skald; use skald_core::latex::CompileError; use skald_core::tools::fs as fs_tools; use super::ApiError; +use super::guard::AuthUser; +use super::require_context; #[derive(Serialize)] pub struct FileEntry { @@ -20,19 +22,25 @@ pub struct FileEntry { pub name: String, } -pub async fn list_files(State(_state): State>) -> Result>, ApiError> { - let root = fs_tools::resolve(".")?; +pub async fn list_files( + State(state): State>, + Extension(auth): Extension, +) -> Result>, ApiError> { + // Scoped to the caller's own home; entries are returned as agent paths (`~/…`) + // so they round-trip through `GET /api/file` unchanged. + let ctx = require_context(&state, &auth.user_id).await?; + let root = ctx.fs.load().home_host.clone(); let mut paths: Vec = Vec::new(); walk(&root, &root, &mut paths)?; paths.sort(); let entries = paths .into_iter() - .map(|p| { - let name = Path::new(&p) + .map(|rel| { + let name = Path::new(&rel) .file_stem() - .map_or_else(|| p.clone(), |s| s.to_string_lossy().to_string()); - FileEntry { path: p, name } + .map_or_else(|| rel.clone(), |s| s.to_string_lossy().to_string()); + FileEntry { path: format!("~/{rel}"), name } }) .collect(); Ok(Json(entries)) @@ -64,12 +72,18 @@ pub struct FileQuery { /// with the textual `latexmk` log in the body, so the caller can fall back to /// showing the raw source. pub async fn get_file( - State(state): State>, - Query(q): Query, + State(state): State>, + Extension(auth): Extension, + Query(q): Query, ) -> Response { - let abs = match fs_tools::resolve(&q.path) { - Ok(p) => p, - Err(_) => return (StatusCode::BAD_REQUEST, format!("Invalid path: {}", q.path)).into_response(), + let ctx = match require_context(&state, &auth.user_id).await { + Ok(c) => c, + Err(e) => return e.into_response(), + }; + let user_fs = ctx.fs.load(); + let abs = match fs_tools::resolve_view_path(user_fs.as_ref(), &q.path) { + Ok((abs, _)) => abs, + Err(e) => return (StatusCode::BAD_REQUEST, format!("Invalid path: {e}")).into_response(), }; if q.compile_latex && is_latex(&q.path) { @@ -220,12 +234,15 @@ pub struct CreatePayload { } pub async fn create_file( - State(_state): State>, - Json(body): Json, + State(state): State>, + Extension(auth): Extension, + Json(body): Json, ) -> Result { - let abs = fs_tools::resolve(&body.path)?; + let ctx = require_context(&state, &auth.user_id).await?; + let (abs, display) = fs_tools::resolve_view_path(ctx.fs.load().as_ref(), &body.path) + .map_err(|e| ApiError::bad_request(e.to_string()))?; if abs.exists() { - return Err(anyhow::anyhow!("File already exists: {}", body.path).into()); + return Err(anyhow::anyhow!("File already exists: {display}").into()); } if let Some(parent) = abs.parent() { std::fs::create_dir_all(parent)?; @@ -235,12 +252,15 @@ pub async fn create_file( } pub async fn save_file( - State(_state): State>, - Json(body): Json, + State(state): State>, + Extension(auth): Extension, + Json(body): Json, ) -> Result { - let abs = fs_tools::resolve(&body.path)?; + let ctx = require_context(&state, &auth.user_id).await?; + let (abs, display) = fs_tools::resolve_view_path(ctx.fs.load().as_ref(), &body.path) + .map_err(|e| ApiError::bad_request(e.to_string()))?; if !abs.exists() { - return Err(anyhow::anyhow!("File not found: {}", body.path).into()); + return Err(anyhow::anyhow!("File not found: {display}").into()); } std::fs::write(&abs, &body.content)?; Ok(StatusCode::NO_CONTENT) @@ -253,16 +273,21 @@ pub struct RenamePayload { } pub async fn rename_file( - State(_state): State>, - Json(body): Json, + State(state): State>, + Extension(auth): Extension, + Json(body): Json, ) -> Result { - let old_abs = fs_tools::resolve(&body.old_path)?; - let new_abs = fs_tools::resolve(&body.new_path)?; + let ctx = require_context(&state, &auth.user_id).await?; + let fs = ctx.fs.load(); + let (old_abs, old_disp) = fs_tools::resolve_view_path(fs.as_ref(), &body.old_path) + .map_err(|e| ApiError::bad_request(e.to_string()))?; + let (new_abs, new_disp) = fs_tools::resolve_view_path(fs.as_ref(), &body.new_path) + .map_err(|e| ApiError::bad_request(e.to_string()))?; if !old_abs.exists() { - return Err(anyhow::anyhow!("File not found: {}", body.old_path).into()); + return Err(anyhow::anyhow!("File not found: {old_disp}").into()); } if new_abs.exists() { - return Err(anyhow::anyhow!("File already exists: {}", body.new_path).into()); + return Err(anyhow::anyhow!("File already exists: {new_disp}").into()); } if let Some(parent) = new_abs.parent() { std::fs::create_dir_all(parent)?; @@ -272,12 +297,15 @@ pub async fn rename_file( } pub async fn delete_file( - State(_state): State>, - Query(q): Query, + State(state): State>, + Extension(auth): Extension, + Query(q): Query, ) -> Result { - let abs = fs_tools::resolve(&q.path)?; + let ctx = require_context(&state, &auth.user_id).await?; + let (abs, display) = fs_tools::resolve_view_path(ctx.fs.load().as_ref(), &q.path) + .map_err(|e| ApiError::bad_request(e.to_string()))?; if !abs.exists() { - return Err(anyhow::anyhow!("File non trovato: {}", q.path).into()); + return Err(anyhow::anyhow!("File not found: {display}").into()); } std::fs::remove_file(&abs)?; Ok(StatusCode::NO_CONTENT) diff --git a/src/frontend/api/ws.rs b/src/frontend/api/ws.rs index 858c99a..860511c 100644 --- a/src/frontend/api/ws.rs +++ b/src/frontend/api/ws.rs @@ -400,6 +400,7 @@ async fn handle_socket(mut socket: WebSocket, skald: Arc, source: String, skald_core::tools::show_file::make_tool( Arc::clone(&chat_hub), source.clone(), + ctx.fs.clone(), ), ], ..Default::default()