diff --git a/crates-cli/yaak-cli/src/commands/mcp.rs b/crates-cli/yaak-cli/src/commands/mcp.rs index 97e064128..4dac043d3 100644 --- a/crates-cli/yaak-cli/src/commands/mcp.rs +++ b/crates-cli/yaak-cli/src/commands/mcp.rs @@ -5,7 +5,9 @@ use rand::RngExt; use std::net::{Ipv4Addr, SocketAddr}; use std::path::PathBuf; use std::sync::Arc; -use yaak::send::{SendHttpRequestWithPluginsParams, send_http_request_with_plugins}; +use yaak::send::{ + SendHttpRequestWithPluginsParams, SendRequestSource, send_http_request_with_plugins, +}; use yaak_crypto::manager::EncryptionManager; use yaak_http::manager::HttpConnectionManager; use yaak_mcp::{HostResult, McpHost, WorkspaceInfo}; @@ -215,6 +217,8 @@ impl McpHost for CliMcpHost { query_manager: &self.query_manager, blob_manager: &self.blob_manager, request, + // The MCP server loads the request by id and passes the row on as it is + request_source: SendRequestSource::Stored, environment_id: environment_id.as_deref(), update_source: UpdateSource::Sync, cookie_jar_id, diff --git a/crates-cli/yaak-cli/src/plugin_events.rs b/crates-cli/yaak-cli/src/plugin_events.rs index b4f773a10..5adbc6e58 100644 --- a/crates-cli/yaak-cli/src/plugin_events.rs +++ b/crates-cli/yaak-cli/src/plugin_events.rs @@ -14,7 +14,9 @@ use yaak::plugin_events::{ GroupedPluginEvent, HostRequest, SharedPluginEventContext, handle_shared_plugin_event, }; use yaak::response_body::FileResponseBodyStore; -use yaak::send::{SendHttpRequestWithPluginsParams, send_http_request_with_plugins}; +use yaak::send::{ + SendHttpRequestWithPluginsParams, SendRequestSource, send_http_request_with_plugins, +}; use yaak_crypto::manager::EncryptionManager; use yaak_http::cookies::get_cookie_value_from_jar; use yaak_http::manager::HttpConnectionManager; @@ -255,6 +257,8 @@ async fn build_plugin_reply( query_manager: &host_context.query_manager, blob_manager: &host_context.blob_manager, request: http_request, + // A plugin may have rendered it, so it is versioned from the stored row + request_source: SendRequestSource::Supplied, environment_id: environment_id.as_deref(), update_source: UpdateSource::Plugin, cookie_jar_id, diff --git a/crates-tauri/yaak-app-client/src/http_request.rs b/crates-tauri/yaak-app-client/src/http_request.rs index b8a63b9c8..1fe29a613 100644 --- a/crates-tauri/yaak-app-client/src/http_request.rs +++ b/crates-tauri/yaak-app-client/src/http_request.rs @@ -8,7 +8,10 @@ use std::sync::Arc; use std::time::Instant; use tauri::{AppHandle, Manager, Runtime, WebviewWindow}; use tokio::sync::watch::Receiver; -use yaak::send::{ResponseBody, SendHttpRequestWithPluginsParams, send_http_request_with_plugins}; +use yaak::send::{ + ResponseBody, SendHttpRequestWithPluginsParams, SendRequestSource, + send_http_request_with_plugins, +}; use yaak_crypto::manager::EncryptionManager; use yaak_http::manager::HttpConnectionManager; use yaak_models::models::{CookieJar, Environment, HttpRequest, HttpResponse, HttpResponseState}; @@ -70,6 +73,7 @@ pub struct SentHttpRequest { pub async fn send_http_request( window: &WebviewWindow, unrendered_request: &HttpRequest, + request_source: SendRequestSource, og_response: &HttpResponse, environment: Option, cookie_jar: Option, @@ -78,6 +82,7 @@ pub async fn send_http_request( send_http_request_with_context( window, unrendered_request, + request_source, og_response, environment, cookie_jar, @@ -90,6 +95,7 @@ pub async fn send_http_request( pub async fn send_http_request_with_context( window: &WebviewWindow, unrendered_request: &HttpRequest, + request_source: SendRequestSource, og_response: &HttpResponse, environment: Option, cookie_jar: Option, @@ -106,6 +112,7 @@ pub async fn send_http_request_with_context( let result = send_http_request_inner( window, unrendered_request, + request_source, environment, cookie_jar, cancelled_rx, @@ -141,6 +148,7 @@ pub async fn send_http_request_with_context( async fn send_http_request_inner( window: &WebviewWindow, unrendered_request: &HttpRequest, + request_source: SendRequestSource, environment: Option, cookie_jar: Option, cancelled_rx: &Receiver, @@ -159,6 +167,7 @@ async fn send_http_request_inner( query_manager: app_handle.db_manager().inner(), blob_manager: app_handle.blob_manager().inner(), request: unrendered_request.clone(), + request_source, environment_id: environment_id.as_deref(), update_source: response_ctx.update_source.clone(), cookie_jar_id, diff --git a/crates-tauri/yaak-app-client/src/lib.rs b/crates-tauri/yaak-app-client/src/lib.rs index 395173558..cdea95d1d 100644 --- a/crates-tauri/yaak-app-client/src/lib.rs +++ b/crates-tauri/yaak-app-client/src/lib.rs @@ -28,7 +28,7 @@ use tauri_plugin_log::{Builder, Target, TargetKind, log}; use tokio::sync::Mutex; use tokio::task::block_in_place; use tokio::time; -use yaak::send::ResponseBody; +use yaak::send::{ResponseBody, SendRequestSource}; use yaak_commands::resolve::resolve_grpc_request; use yaak_commands::responses::locate_response_body; use yaak_common::command::new_checked_command; @@ -973,7 +973,16 @@ async fn cmd_send_ephemeral_request( }); let sent = - send_http_request(&window, &request, &response, environment, cookie_jar, &mut cancel_rx) + // No id, so no version either way; it is not a stored row + send_http_request( + &window, + &request, + SendRequestSource::Supplied, + &response, + environment, + cookie_jar, + &mut cancel_rx, + ) .await?; // Blanking the request id above is what makes this send unsaved, so the @@ -1129,9 +1138,12 @@ async fn cmd_send_http_request( None => None, }; + // `request` is the row loaded above, so the version is taken from it rather than + // re-read once plugins have started, which an edit could beat let r = match send_http_request( &window, &request, + SendRequestSource::Stored, &response, environment, cookie_jar, diff --git a/crates-tauri/yaak-app-client/src/plugin_events.rs b/crates-tauri/yaak-app-client/src/plugin_events.rs index 5a079a51c..34ea4a079 100644 --- a/crates-tauri/yaak-app-client/src/plugin_events.rs +++ b/crates-tauri/yaak-app-client/src/plugin_events.rs @@ -19,6 +19,7 @@ use yaak::plugin_events::{ GroupedPluginEvent, HostRequest, SharedPluginEventContext, handle_shared_plugin_event, }; use yaak::response_body::FileResponseBodyStore; +use yaak::send::SendRequestSource; use yaak_crypto::manager::EncryptionManager; use yaak_http::cookies::get_cookie_value_from_jar; use yaak_models::models::{HttpRequest, HttpResponse, Plugin}; @@ -486,6 +487,8 @@ pub(crate) async fn send_http_request_in_window( send_http_request_with_context( window, &http_request, + // A plugin may have rendered it, so it is versioned from the stored row + SendRequestSource::Supplied, &http_response, environment, cookie_jar, diff --git a/crates/yaak/src/send.rs b/crates/yaak/src/send.rs index 43925c7b2..22c9ed2a9 100644 --- a/crates/yaak/src/send.rs +++ b/crates/yaak/src/send.rs @@ -27,6 +27,7 @@ use yaak_models::models::{ HttpResponseEvent, HttpResponseEventData, HttpResponseHeader, HttpResponseState, ProxySetting, ProxySettingAuth, ResolvedHttpRequestSettings, }; +use yaak_models::queries::any_request::AnyRequest; use yaak_models::query_manager::QueryManager; use yaak_models::render::render_http_request; use yaak_models::util::{UpdateSource, generate_prefixed_id}; @@ -312,6 +313,9 @@ pub struct SendHttpRequestWithPluginsParams<'a> { pub query_manager: &'a QueryManager, pub blob_manager: &'a BlobManager, pub request: HttpRequest, + /// Whether `request` is the stored row as loaded, which decides what its version is + /// taken from; see [`SendRequestSource`]. + pub request_source: SendRequestSource, pub environment_id: Option<&'a str>, pub update_source: UpdateSource, pub cookie_jar_id: Option, @@ -404,10 +408,25 @@ impl HttpSendRuntimeConfig { } } +/// Where the request a send was handed came from, which decides what its version is +/// taken from. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum SendRequestSource { + /// The stored row, exactly as this send loaded it. The version is taken from this + /// copy, with no second read, so the response names what was actually sent even if + /// the row is edited before the send resolves. + Stored, + /// Something that may differ from the stored row, such as a request a plugin + /// rendered before sending. The version is read from the stored row by id instead, + /// so rendered values (secrets included) never reach history. + Supplied, +} + /// Resolve every database-backed input a send needs, in one pass. pub fn resolve_send_inputs( query_manager: &QueryManager, request: &HttpRequest, + request_source: SendRequestSource, environment_id: Option<&str>, cookies: Option>, ) -> Result { @@ -438,23 +457,31 @@ pub fn resolve_send_inputs( cookie_store: cookies.map(CookieStore::from_cookies), // Every host reaches sending through this function — the desktop, the CLI, // plugin-triggered sends — so every response gets a version without each of them - // remembering to ask for one. The version is of the stored row, not of `request`: - // what a send holds may already be resolved or rendered (a plugin can send a - // request it rendered itself), and neither is what a restore should put back. - version_id: snapshot_request_for_send(query_manager, &request.id), + // remembering to ask for one. It is always of stored content, never of a resolved + // or rendered copy, which is not what a restore should put back. + version_id: snapshot_request_for_send(query_manager, request, request_source), }) } -/// Capture the stored request in its own transaction. Like the snapshot itself, a -/// failure to get the write connection is not worth failing the send over. -fn snapshot_request_for_send(query_manager: &QueryManager, request_id: &str) -> Option { +/// Capture the request in its own transaction. Like the snapshot itself, a failure to +/// get the write connection is not worth failing the send over. +fn snapshot_request_for_send( + query_manager: &QueryManager, + request: &HttpRequest, + source: SendRequestSource, +) -> Option { // An unsaved request has nothing to version; don't take the writer for it - if request_id.is_empty() { + if request.id.is_empty() { return None; } query_manager .with_tx(|tx| { - Ok::<_, yaak_models::error::Error>(tx.snapshot_stored_request_for_send(request_id)) + Ok::<_, yaak_models::error::Error>(match source { + SendRequestSource::Stored => { + tx.snapshot_request_for_send(&AnyRequest::HttpRequest(request.clone())) + } + SendRequestSource::Supplied => tx.snapshot_stored_request_for_send(&request.id), + }) }) .unwrap_or_else(|err| { warn!("Failed to snapshot request before send: {err}"); @@ -494,6 +521,7 @@ pub async fn send_http_request_by_id_with_plugins( query_manager: params.query_manager, blob_manager: params.blob_manager, request, + request_source: SendRequestSource::Stored, environment_id: params.environment_id, update_source: params.update_source, cookie_jar_id: params.cookie_jar_id, @@ -517,6 +545,7 @@ pub async fn send_http_request_with_plugins( let inputs = resolve_send_inputs( params.query_manager, ¶ms.request, + params.request_source, params.environment_id, cookie_jar.as_ref().map(|jar| jar.cookies.clone()), )?; @@ -575,6 +604,7 @@ pub async fn send_http_request_by_id( let inputs = resolve_send_inputs( params.query_manager, &request, + SendRequestSource::Stored, params.environment_id, cookie_jar.as_ref().map(|jar| jar.cookies.clone()), )?; @@ -1505,8 +1535,17 @@ mod tests { }) .expect("Failed to seed request"); - let first = stored_send(&query_manager, &blob_manager, temp_dir.path(), &request).await; - let second = stored_send(&query_manager, &blob_manager, temp_dir.path(), &request).await; + let send = || { + stored_send( + &query_manager, + &blob_manager, + temp_dir.path(), + &request, + SendRequestSource::Stored, + ) + }; + let first = send().await; + let second = send().await; let version_id = first.version_id.clone().expect("a stored send must record a version"); assert_eq!( @@ -1563,7 +1602,14 @@ mod tests { headers: vec![header("Bearer s3cret")], ..stored.clone() }; - let response = stored_send(&query_manager, &blob_manager, temp_dir.path(), &rendered).await; + let response = stored_send( + &query_manager, + &blob_manager, + temp_dir.path(), + &rendered, + SendRequestSource::Supplied, + ) + .await; let version_id = response.version_id.expect("a stored request must record a version"); let version = @@ -1578,6 +1624,54 @@ mod tests { ); } + /// An edit can land between a send loading its request and resolving it (the + /// desktop awaits plugin startup in between). The response must still name the + /// content that was sent, not the edit. + #[tokio::test] + async fn an_edit_between_loading_and_sending_is_not_what_gets_versioned() { + let (query_manager, blob_manager, temp_dir) = seed_send_storage(); + let stored = query_manager + .with_tx(|tx| { + tx.upsert_http_request( + &HttpRequest { + workspace_id: "wk_test".to_string(), + url: "http://localhost/sent".to_string(), + ..Default::default() + }, + &UpdateSource::Sync, + ) + }) + .expect("Failed to seed request"); + + // The send has loaded `stored`; now the row changes underneath it + let loaded = query_manager.connect().get_http_request(&stored.id).unwrap(); + query_manager + .with_tx(|tx| { + tx.upsert_http_request( + &HttpRequest { url: "http://localhost/edited".to_string(), ..loaded.clone() }, + &UpdateSource::Sync, + ) + }) + .expect("Failed to edit request"); + + let response = stored_send( + &query_manager, + &blob_manager, + temp_dir.path(), + &loaded, + SendRequestSource::Stored, + ) + .await; + + assert_eq!(response.url, "http://localhost/sent"); + let version_id = response.version_id.expect("a stored send must record a version"); + let version = + query_manager.connect().get_model_version(&version_id).expect("Failed to load version"); + assert_eq!(version.document.get("url").unwrap(), "http://localhost/sent"); + // And the live request, which now holds the edit, shows as changed + assert!(!query_manager.connect().request_matches_version(&version).unwrap()); + } + /// A request with an id but no stored row has nothing to version, and /// resolving its send must not fail for it. #[test] @@ -1590,8 +1684,14 @@ mod tests { ..Default::default() }; - let inputs = resolve_send_inputs(&query_manager, &transient, None, None) - .expect("Failed to resolve send inputs"); + let inputs = resolve_send_inputs( + &query_manager, + &transient, + SendRequestSource::Supplied, + None, + None, + ) + .expect("Failed to resolve send inputs"); assert_eq!(inputs.version_id, None); assert!(query_manager.connect().list_model_versions("rq_not_stored").unwrap().is_empty()); @@ -1602,9 +1702,10 @@ mod tests { blob_manager: &BlobManager, response_dir: &std::path::Path, request: &HttpRequest, + source: SendRequestSource, ) -> HttpResponse { let executor = StubExecutor { body: b"hello world" }; - let inputs = resolve_send_inputs(query_manager, request, None, None) + let inputs = resolve_send_inputs(query_manager, request, source, None, None) .expect("Failed to resolve send inputs"); send_http_request(SendHttpRequestParams { inputs,