From 9fc9af1f6707057628cf21a8aa5deacd5819c8cf Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Mon, 19 Oct 2026 10:13:28 -0400 Subject: [PATCH 01/19] feat(dgw): search session logs across recordings Add `POST /jet/jrec/search`, so DVLS can search the `.slog` logs of recordings without downloading every log. The body is `{ "query": "...", "recordings": "all" | ["", ...] }`, guarded by a new `gateway.recordings.search` scope. The existing `gateway.recordings.read` only lists IDs, so reusing it would let every read token see log contents. The search reads the `.slog` files and AI analysis logs listed in `recording.json`, with the rules of the log viewer's parser: a case-insensitive substring over the shown fields and parameters, entries the viewer drops are skipped, and long strings match on their shown part only. Hits are the original lines. A search stops at 1000 hits, 16 MiB of hits or 256 MiB read, and reports it with `limitReached`. `api/jrec.rs` moves to `api/jrec/mod.rs` to hold the `search` submodule. Issue: DGW-463 Co-Authored-By: Claude Opus 5.5 (1M context) --- devolutions-gateway/Cargo.toml | 2 +- .../src/api/{jrec.rs => jrec/mod.rs} | 5 +- devolutions-gateway/src/api/jrec/search.rs | 787 ++++++++++++++++++ devolutions-gateway/src/extract.rs | 18 + devolutions-gateway/src/openapi.rs | 6 + devolutions-gateway/src/token.rs | 2 + testsuite/tests/cli/dgw/mod.rs | 1 + testsuite/tests/cli/dgw/recording_search.rs | 201 +++++ .../JsonSerializationTests.cs | 10 + .../src/AccessScope.cs | 1 + 10 files changed, 1031 insertions(+), 2 deletions(-) rename devolutions-gateway/src/api/{jrec.rs => jrec/mod.rs} (99%) create mode 100644 devolutions-gateway/src/api/jrec/search.rs create mode 100644 testsuite/tests/cli/dgw/recording_search.rs diff --git a/devolutions-gateway/Cargo.toml b/devolutions-gateway/Cargo.toml index 6e360fc24c..25e65c256b 100644 --- a/devolutions-gateway/Cargo.toml +++ b/devolutions-gateway/Cargo.toml @@ -50,7 +50,7 @@ kdc = "0.1" # Serialization serde = { version = "1", features = ["derive"] } -serde_json = "1" +serde_json = { version = "1", features = ["raw_value"] } serde_urlencoded = "0.7" serde-querystring = "0.3.0" diff --git a/devolutions-gateway/src/api/jrec.rs b/devolutions-gateway/src/api/jrec/mod.rs similarity index 99% rename from devolutions-gateway/src/api/jrec.rs rename to devolutions-gateway/src/api/jrec/mod.rs index 95f2ce7101..32f4b7d143 100644 --- a/devolutions-gateway/src/api/jrec.rs +++ b/devolutions-gateway/src/api/jrec/mod.rs @@ -1,3 +1,5 @@ +pub(crate) mod search; + use std::fs; use std::io::{self, Seek as _, Write as _}; use std::net::SocketAddr; @@ -12,7 +14,7 @@ use axum::extract::ws::WebSocket; use axum::extract::{self, ConnectInfo, Query, State, WebSocketUpgrade}; use axum::http::header::{CONTENT_DISPOSITION, CONTENT_TYPE, HeaderValue}; use axum::response::Response; -use axum::routing::{delete, get}; +use axum::routing::{delete, get, post}; use axum::{Json, Router}; use bytes::Bytes; use cadeau::xmf; @@ -59,6 +61,7 @@ pub fn make_router(state: DgwState) -> Router { .route("/play", get(get_player)) .route("/play/{*path}", get(get_player)) .route("/shadow/{id}", get(shadow_recording)) + .route("/search", post(search::search_recordings)) .with_state(state) } diff --git a/devolutions-gateway/src/api/jrec/search.rs b/devolutions-gateway/src/api/jrec/search.rs new file mode 100644 index 0000000000..289dc928b0 --- /dev/null +++ b/devolutions-gateway/src/api/jrec/search.rs @@ -0,0 +1,787 @@ +use std::collections::{HashMap, HashSet}; +use std::{fmt, io}; + +use axum::Json; +use axum::extract::State; +use camino::{Utf8Path, Utf8PathBuf}; +use serde::de::{self, MapAccess, Visitor}; +use serde_json::Value; +use serde_json::value::RawValue; +use tokio::fs::File; +use tokio::io::{AsyncBufReadExt as _, BufReader}; +use uuid::Uuid; + +use super::is_safe_recording_file_name; +use crate::DgwState; +use crate::artifacts::JrecArtifacts; +use crate::extract::RecordingsSearchScope; +use crate::http::HttpError; +use crate::token::RecordingFileType; + +/// Most hits returned by one search. +const MAX_HITS: usize = 1_000; + +/// Most bytes of matching entries returned by one search. +const MAX_HIT_BYTES: usize = 16 * 1024 * 1024; + +/// Most log bytes read by one search, so that searching every recording stays bounded. +const MAX_SCANNED_BYTES: u64 = 256 * 1024 * 1024; + +// The log viewers read a `.slog` file through the log parser, which drops or truncates some entries. +// The search applies the same limits, so that it finds what the viewers show and nothing else. +// https://github.com/Devolutions/devolutions-gateway/blob/master/webapp/packages/session-recording-log/src/parser.ts + +const MAX_LINE_BYTES: usize = 256 * 1024; +const MAX_SCANNED_LINES: usize = 20_000; +const MAX_PARSED_ENTRIES: usize = 10_000; +const MAX_RETAINED_BYTES: usize = 8 * 1024 * 1024; +/// Counted in UTF-16 code units, like JavaScript string lengths. +const MAX_STRING_LENGTH: usize = 4_096; +const MAX_PARAMETER_COUNT: usize = 200; +const MAX_OBJECT_DEPTH: usize = 8; +/// `Number.MAX_SAFE_INTEGER` in JavaScript. +const MAX_SAFE_INTEGER: f64 = 9_007_199_254_740_991.0; + +/// Optional entry fields searched by the query, in addition to `timestamp` and `description`. +const SEARCHED_OPTIONAL_FIELDS: [&str; 4] = ["object", "actor", "host", "sessionType"]; + +#[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] +#[derive(Deserialize)] +pub(crate) struct RecordingSearchRequest { + /// Text to search for. + query: String, + /// The recordings to search. + recordings: RecordingSelection, +} + +/// `"all"`, or a list of recording IDs. +/// +/// A repeated ID is searched once, and an empty list searches nothing. +#[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] +#[derive(Deserialize)] +#[serde(untagged)] +pub(crate) enum RecordingSelection { + All(AllRecordings), + Listed(Vec), +} + +#[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] +#[derive(Deserialize)] +#[serde(rename_all = "lowercase")] +pub(crate) enum AllRecordings { + All, +} + +#[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +pub(crate) struct RecordingSearchResponse { + /// Matching entries, newest recording first when searching all recordings, otherwise in the requested order. + hits: Vec, + /// The search stopped before reading every log, so more entries may match. + limit_reached: bool, +} + +#[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +pub(crate) struct RecordingSearchHit { + recording_id: Uuid, + /// Name of the `.slog` file holding the entry, as listed in `recording.json`. + file_name: String, + /// The log entry exactly as recorded. + #[cfg_attr(feature = "openapi", schema(value_type = Object))] + entry: Box, +} + +/// Searches the Session Recording Log (`.slog`) files of recordings stored on this instance +/// +/// The query is a case-insensitive substring, matched against the same entry fields as the log viewers: +/// `timestamp`, `description`, `object`, `actor`, `host`, `sessionType`, and the parameter names and values. +/// Entries the log viewers do not show are not searched, and long strings are matched on the part the viewers show. +/// Recordings without a readable `recording.json` and IDs with no recording on this instance give no hits. +#[cfg_attr(feature = "openapi", utoipa::path( + post, + operation_id = "SearchRecordings", + tag = "Jrec", + path = "/jet/jrec/search", + request_body(content = RecordingSearchRequest, description = "JSON-encoded search request", content_type = "application/json"), + responses( + (status = 200, description = "Matching log entries", body = RecordingSearchResponse), + (status = 400, description = "Bad request"), + (status = 401, description = "Invalid or missing authorization token"), + (status = 403, description = "Insufficient permissions"), + ), + security(("scope_token" = ["gateway.recordings.search"])), +))] +pub(crate) async fn search_recordings( + State(DgwState { conf_handle, .. }): State, + _scope: RecordingsSearchScope, + Json(request): Json, +) -> Result, HttpError> { + let text = SearchText::new(&request.query).ok_or_else(|| HttpError::bad_request().msg("empty search text"))?; + + let recording_root = conf_handle.get_conf().recording_path.clone(); + + let recordings = match request.recordings { + RecordingSelection::All(AllRecordings::All) => all_recording_logs_newest_first(&recording_root) + .await + .map_err(HttpError::internal().with_msg("failed recording listing").err())?, + RecordingSelection::Listed(recording_ids) => { + let mut seen = HashSet::new(); + let mut recordings = Vec::with_capacity(recording_ids.len()); + for recording_id in recording_ids + .into_iter() + .filter(|recording_id| seen.insert(*recording_id)) + { + let recording_dir = recording_root.join(recording_id.to_string()); + recordings.extend(read_recording_logs(recording_dir, recording_id).await); + } + recordings + } + }; + + let mut search = Search::new(text); + + for recording in recordings { + for file_name in recording.file_names { + let path = recording.dir.join(&file_name); + + match search.search_file(recording.recording_id, file_name, &path).await { + Ok(()) => {} + // A recording can be deleted while it is searched. + Err(error) if error.kind() == io::ErrorKind::NotFound => { + debug!(%path, "Recording log listed in the manifest is missing"); + } + Err(error) => warn!(%path, %error, "Failed to search a recording log"), + } + + if search.limit_reached { + return Ok(Json(search.into_response())); + } + } + } + + Ok(Json(search.into_response())) +} + +/// Search text in lowercase, the case entries are compared in. +struct SearchText(String); + +impl SearchText { + fn new(query: &str) -> Option { + let text = query.trim().to_lowercase(); + (!text.is_empty()).then_some(Self(text)) + } + + fn is_in(&self, value: &str) -> bool { + visible_text(value).to_lowercase().contains(&self.0) + } +} + +struct Search { + text: SearchText, + hits: Vec, + hit_bytes: usize, + scanned_bytes: u64, + limit_reached: bool, +} + +impl Search { + fn new(text: SearchText) -> Self { + Self { + text, + hits: Vec::new(), + hit_bytes: 0, + scanned_bytes: 0, + limit_reached: false, + } + } + + fn into_response(self) -> RecordingSearchResponse { + RecordingSearchResponse { + hits: self.hits, + limit_reached: self.limit_reached, + } + } + + /// Adds the matching entries of one log, reading it the way the log parser does. + async fn search_file(&mut self, recording_id: Uuid, file_name: String, path: &Utf8Path) -> io::Result<()> { + let mut reader = BufReader::new(File::open(path).await?); + let mut line = Vec::new(); + let mut scanned_lines = 0; + let mut parsed_entries = 0; + let mut retained_bytes = 0; + + while let Some(read) = read_line(&mut reader, &mut line).await? { + // UNTESTED: the scanned-bytes budget stops the search and reports `limitReached`. + // It keeps a search of every recording from reading an unbounded amount of logs. + // When changing it, keep `limit_reached` set whenever logs are left unread. + self.scanned_bytes += read.bytes; + if MAX_SCANNED_BYTES < self.scanned_bytes { + self.limit_reached = true; + return Ok(()); + } + + scanned_lines += 1; + if MAX_SCANNED_LINES < scanned_lines { + break; + } + + if !read.too_long && line.iter().all(u8::is_ascii_whitespace) { + continue; + } + + if MAX_PARSED_ENTRIES <= parsed_entries { + break; + } + + if read.too_long { + continue; + } + + let is_match = match viewer_entry(&self.text, &line) { + ViewerEntry::Hidden => continue, + ViewerEntry::Match => true, + ViewerEntry::NoMatch => false, + }; + + if MAX_RETAINED_BYTES < retained_bytes + line.len() { + break; + } + retained_bytes += line.len(); + parsed_entries += 1; + + if is_match && !self.push(recording_id, &file_name, &line) { + return Ok(()); + } + } + + Ok(()) + } + + /// Returns `false` when the hit limits are reached. + fn push(&mut self, recording_id: Uuid, file_name: &str, line: &[u8]) -> bool { + // UNTESTED: the hit-bytes budget stops the search and reports `limitReached`. + // It bounds the response size, which the hit count alone does not, because a line can be large. + // When changing it, keep both the hit count and the hit bytes bounded. + if self.hits.len() == MAX_HITS || MAX_HIT_BYTES < self.hit_bytes + line.len() { + self.limit_reached = true; + return false; + } + + // The line was parsed as JSON already. + let Ok(entry) = serde_json::from_slice::>(line) else { + return true; + }; + + self.hit_bytes += line.len(); + self.hits.push(RecordingSearchHit { + recording_id, + file_name: file_name.to_owned(), + entry, + }); + + true + } +} + +/// How the log viewers see the entry on one line, given the search text. +#[derive(Debug, PartialEq, Eq)] +enum ViewerEntry { + /// The viewers drop the entry. + Hidden, + /// The viewers show the entry, and the search text is in it. + Match, + /// The viewers show the entry, and the search text is not in it. + NoMatch, +} + +fn viewer_entry(text: &SearchText, line: &[u8]) -> ViewerEntry { + return shown_entry_matches(text, line).map_or(ViewerEntry::Hidden, |is_match| match is_match { + true => ViewerEntry::Match, + false => ViewerEntry::NoMatch, + }); + + fn shown_entry_matches(text: &SearchText, line: &[u8]) -> Option { + let Ok(Value::Object(fields)) = serde_json::from_slice::(line) else { + return None; + }; + + if exceeds_max_object_depth(&fields) { + return None; + } + + let event = fields.get("event")?.as_str()?; + if MAX_STRING_LENGTH < event.encode_utf16().count() { + return None; + } + + let timestamp = fields.get("timestamp")?.as_str()?; + let description = fields.get("description")?.as_str()?; + + let seq = fields.get("seq")?.as_f64()?; + if !(seq.fract() == 0.0 && (0.0..=MAX_SAFE_INTEGER).contains(&seq)) { + return None; + } + + let in_fields = [timestamp, description] + .into_iter() + .chain( + SEARCHED_OPTIONAL_FIELDS + .iter() + .filter_map(|field| fields.get(*field)?.as_str()), + ) + .any(|value| text.is_in(value)); + + if in_fields { + return Some(true); + } + + if !fields.get("parameters").is_some_and(Value::is_object) { + return Some(false); + } + + // A `serde_json::Map` sorts its keys, and which parameters the viewers keep depends on their order. + #[derive(Deserialize)] + struct LineParameters { + parameters: OrderedObject, + } + + let Ok(LineParameters { parameters }) = serde_json::from_slice(line) else { + return Some(false); + }; + + let mut kept_names = HashSet::new(); + + let in_parameters = parameters + .0 + .iter() + .take(MAX_PARAMETER_COUNT) + .filter_map(|(name, value)| Some((visible_text(name), value.as_str()?))) + .filter(|(name, _)| kept_names.insert(*name)) + .any(|(name, value)| text.is_in(name) || text.is_in(value)); + + Some(in_parameters) + } +} + +fn exceeds_max_object_depth(fields: &serde_json::Map) -> bool { + let mut stack: Vec<(&Value, usize)> = fields.values().map(|value| (value, 2)).collect(); + + while let Some((value, depth)) = stack.pop() { + match value { + Value::Array(_) | Value::Object(_) if MAX_OBJECT_DEPTH < depth => return true, + Value::Array(items) => stack.extend(items.iter().map(|child| (child, depth + 1))), + Value::Object(fields) => stack.extend(fields.values().map(|child| (child, depth + 1))), + _ => {} + } + } + + false +} + +/// The part of a string the log viewers keep: its first [`MAX_STRING_LENGTH`] UTF-16 code units. +fn visible_text(value: &str) -> &str { + let mut code_units = 0; + + for (index, character) in value.char_indices() { + code_units += character.len_utf16(); + + if MAX_STRING_LENGTH < code_units { + return &value[..index]; + } + } + + value +} + +/// A JSON object's entries in the order JavaScript enumerates them after `JSON.parse`. +/// +/// Integer-like names come first in ascending order, then the other names in written order. +/// A repeated name keeps its first position and its last value. +struct OrderedObject(Vec<(String, Value)>); + +impl<'de> serde::Deserialize<'de> for OrderedObject { + fn deserialize>(deserializer: D) -> Result { + struct OrderedObjectVisitor; + + impl<'de> Visitor<'de> for OrderedObjectVisitor { + type Value = OrderedObject; + + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("a JSON object") + } + + fn visit_map>(self, mut map: A) -> Result { + let mut entries: Vec<(String, Value)> = Vec::new(); + let mut positions: HashMap = HashMap::new(); + + while let Some((name, value)) = map.next_entry::()? { + match positions.get(&name) { + Some(&position) => entries[position].1 = value, + None => { + positions.insert(name.clone(), entries.len()); + entries.push((name, value)); + } + } + } + + // A stable sort keeps the written order among the other names. + entries.sort_by_key(|(name, _)| array_index(name).map_or((1, 0), |index| (0, index))); + + Ok(OrderedObject(entries)) + } + } + + return deserializer.deserialize_map(OrderedObjectVisitor); + + /// The array index a property name stands for in JavaScript: a canonical decimal below `u32::MAX`. + fn array_index(name: &str) -> Option { + let index = name.parse::().ok()?; + (index < u32::MAX && index.to_string() == name).then_some(index) + } + } +} + +/// The `.slog` files of one recording. +struct RecordingLogs { + recording_id: Uuid, + dir: Utf8PathBuf, + start_time: i64, + file_names: Vec, +} + +/// Lists the recording logs and the log artifacts, such as the AI analysis, from `recording.json`. +async fn read_recording_logs(dir: Utf8PathBuf, recording_id: Uuid) -> Option { + #[derive(Deserialize)] + #[serde(rename_all = "camelCase")] + struct Manifest { + start_time: i64, + files: Vec, + #[serde(default)] + artifacts: JrecArtifacts, + } + + #[derive(Deserialize)] + #[serde(rename_all = "camelCase")] + struct ManifestFile { + file_name: String, + } + + let path = dir.join("recording.json"); + + // Every search reads every manifest again, so a failure here is not worth more than a debug event. + let json = tokio::fs::read(&path) + .await + .inspect_err(|error| debug!(%path, %error, "Failed to read a recording manifest")) + .ok()?; + + let manifest: Manifest = serde_json::from_slice(&json) + .inspect_err(|error| debug!(%path, %error, "Invalid recording manifest")) + .ok()?; + + let file_names = manifest + .files + .into_iter() + .map(|file| file.file_name) + .chain(manifest.artifacts.into_file_names()) + .filter(|file_name| { + is_safe_recording_file_name(file_name) + && Utf8Path::new(file_name) + .extension() + .and_then(RecordingFileType::from_extension) + == Some(RecordingFileType::SessionRecordingLog) + }) + .collect(); + + Some(RecordingLogs { + recording_id, + dir, + start_time: manifest.start_time, + file_names, + }) +} + +/// Logs of every recording on this instance, newest first by manifest start time. +async fn all_recording_logs_newest_first(recording_root: &Utf8Path) -> anyhow::Result> { + let mut read_dir = match tokio::fs::read_dir(recording_root).await { + Ok(read_dir) => read_dir, + // No recording has been made yet. + Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(Vec::new()), + Err(error) => return Err(error.into()), + }; + + let mut recordings = Vec::new(); + + while let Some(entry) = read_dir.next_entry().await? { + let Some(recording_id) = entry.file_name().to_str().and_then(|name| Uuid::parse_str(name).ok()) else { + continue; + }; + + // A recording deleted while listing is skipped rather than failing the search. + if !entry.file_type().await.is_ok_and(|file_type| file_type.is_dir()) { + continue; + } + + let Ok(dir) = Utf8PathBuf::try_from(entry.path()) else { + continue; + }; + + recordings.extend(read_recording_logs(dir, recording_id).await); + } + + recordings.sort_unstable_by(|a, b| b.start_time.cmp(&a.start_time)); + + Ok(recordings) +} + +struct LineRead { + /// Bytes consumed, line break included. + bytes: u64, + /// The line is longer than [`MAX_LINE_BYTES`]; it is then read as empty. + too_long: bool, +} + +/// Reads the next line into `line`, without its line break, and returns `None` at the end of the file. +/// +/// The reader never holds more than [`MAX_LINE_BYTES`] of one line in memory. +async fn read_line(reader: &mut BufReader, line: &mut Vec) -> io::Result> { + line.clear(); + let mut read = LineRead { + bytes: 0, + too_long: false, + }; + + loop { + let available = reader.fill_buf().await?; + if available.is_empty() { + return Ok((read.bytes != 0).then_some(read)); + } + + let line_break = available.iter().position(|&byte| byte == b'\n'); + let chunk = &available[..line_break.unwrap_or(available.len())]; + + if !read.too_long { + if line.len() + chunk.len() <= MAX_LINE_BYTES { + line.extend_from_slice(chunk); + } else { + read.too_long = true; + line.clear(); + } + } + + let consumed = line_break.map_or(available.len(), |index| index + 1); + reader.consume(consumed); + read.bytes += u64::try_from(consumed).expect("buffer length fits in u64"); + + if line_break.is_some() { + if line.last() == Some(&b'\r') { + line.pop(); + } + + return Ok(Some(read)); + } + } +} + +#[cfg(test)] +mod tests { + use rstest::rstest; + + use super::*; + + fn search_text(query: &str) -> SearchText { + SearchText::new(query).expect("non-empty query") + } + + fn matches(query: &str, line: &str) -> ViewerEntry { + viewer_entry(&search_text(query), line.as_bytes()) + } + + #[rstest] + #[case::description("created user", ViewerEntry::Match)] + #[case::object("desk ott", ViewerEntry::Match)] + #[case::field_name("description", ViewerEntry::NoMatch)] + #[case::event("session", ViewerEntry::NoMatch)] + fn matches_field_values_ignoring_case(#[case] query: &str, #[case] expected: ViewerEntry) { + let line = r#"{"timestamp":"t","seq":1,"event":"session.action","description":"Created User","object":"Help Desk Ottawa"}"#; + + assert_eq!(matches(query, line), expected); + } + + #[rstest] + #[case::no_seq(r#"{"timestamp":"t","event":"e","description":"hit"}"#)] + #[case::negative_seq(r#"{"timestamp":"t","seq":-1,"event":"e","description":"hit"}"#)] + #[case::unsafe_seq(r#"{"timestamp":"t","seq":9007199254740992,"event":"e","description":"hit"}"#)] + #[case::fractional_seq(r#"{"timestamp":"t","seq":1.5,"event":"e","description":"hit"}"#)] + #[case::number_timestamp(r#"{"timestamp":1,"seq":1,"event":"e","description":"hit"}"#)] + #[case::no_event(r#"{"timestamp":"t","seq":1,"description":"hit"}"#)] + #[case::no_description(r#"{"timestamp":"t","seq":1,"event":"e","object":"hit"}"#)] + #[case::too_deep(r#"{"timestamp":"t","seq":1,"event":"e","description":"hit","x":[[[[[[[[1]]]]]]]]}"#)] + #[case::not_an_object(r#"["hit"]"#)] + fn skips_entries_the_viewers_drop(#[case] line: &str) { + assert_eq!(matches("hit", line), ViewerEntry::Hidden); + } + + #[test] + fn keeps_an_entry_at_the_max_depth() { + let line = r#"{"timestamp":"t","seq":1,"event":"e","description":"hit","x":[[[[[[[1]]]]]]]}"#; + + assert_eq!(matches("hit", line), ViewerEntry::Match); + } + + #[test] + fn matches_only_the_visible_part_of_long_strings() { + let description = format!("{}tail", "a".repeat(MAX_STRING_LENGTH)); + let line = format!(r#"{{"timestamp":"t","seq":1,"event":"e","description":"{description}"}}"#); + + assert_eq!(matches("tail", &line), ViewerEntry::NoMatch); + assert_eq!(matches("aaa", &line), ViewerEntry::Match); + } + + #[rstest] + #[case::name("new name", ViewerEntry::Match)] + #[case::value("hull", ViewerEntry::Match)] + #[case::non_string_value("count", ViewerEntry::NoMatch)] + fn matches_string_parameters_only(#[case] query: &str, #[case] expected: ViewerEntry) { + let line = + r#"{"timestamp":"t","seq":1,"event":"e","description":"d","parameters":{"New name":"Hull","Count":2}}"#; + + assert_eq!(matches(query, line), expected); + } + + #[test] + fn matches_the_first_parameters_in_written_order() { + let mut parameters: Vec = (0..MAX_PARAMETER_COUNT).map(|i| format!(r#""z{i}":"v""#)).collect(); + parameters.push(r#""a-late":"late""#.to_owned()); + let line = format!( + r#"{{"timestamp":"t","seq":1,"event":"e","description":"d","parameters":{{{}}}}}"#, + parameters.join(",") + ); + + assert_eq!(matches("z199", &line), ViewerEntry::Match); + assert_eq!(matches("late", &line), ViewerEntry::NoMatch); + } + + #[test] + fn keeps_the_first_parameter_whose_name_collides_after_truncation() { + let name = "n".repeat(MAX_STRING_LENGTH); + let line = format!( + r#"{{"timestamp":"t","seq":1,"event":"e","description":"d","parameters":{{"{name}x":"first","{name}y":"second"}}}}"# + ); + + assert_eq!(matches("first", &line), ViewerEntry::Match); + assert_eq!(matches("second", &line), ViewerEntry::NoMatch); + } + + #[test] + fn orders_parameters_like_javascript() { + let line = r#"{"timestamp":"t","seq":1,"event":"e","description":"d","parameters":{"b":"1","10":"2","a":"3","2":"4","01":"5","b":"6"}}"#; + + #[derive(Deserialize)] + struct LineParameters { + parameters: OrderedObject, + } + + let LineParameters { parameters } = serde_json::from_str(line).expect("parameters"); + let entries: Vec<_> = parameters + .0 + .iter() + .map(|(name, value)| (name.as_str(), value.as_str().expect("string value"))) + .collect(); + + assert_eq!(entries, [("2", "4"), ("10", "2"), ("b", "6"), ("a", "3"), ("01", "5")]); + } + + #[rstest] + #[case::all(r#""all""#, Some(None))] + #[case::listed( + r#"["00000000-0000-0000-0000-000000000002","00000000-0000-0000-0000-000000000001"]"#, + Some(Some(vec![Uuid::from_u128(2), Uuid::from_u128(1)])) + )] + #[case::empty_list("[]", Some(Some(Vec::new())))] + #[case::other_word(r#""some""#, None)] + #[case::all_in_a_list(r#"["all"]"#, None)] + fn parses_the_recording_selection(#[case] recordings: &str, #[case] expected: Option>>) { + let request = format!(r#"{{"query":"q","recordings":{recordings}}}"#); + + let actual = serde_json::from_str::(&request) + .ok() + .map(|request| match request.recordings { + RecordingSelection::All(AllRecordings::All) => None, + RecordingSelection::Listed(recording_ids) => Some(recording_ids), + }); + + assert_eq!(actual, expected); + } + + async fn search_lines(content: &str) -> RecordingSearchResponse { + let dir = tempfile::tempdir().expect("temp dir"); + let path = Utf8PathBuf::from_path_buf(dir.path().join("recording-0.slog")).expect("UTF-8 path"); + std::fs::write(&path, content).expect("write log"); + + let mut search = Search::new(search_text("hit")); + search + .search_file(Uuid::nil(), "recording-0.slog".to_owned(), &path) + .await + .expect("search log"); + + search.into_response() + } + + fn entry_line(seq: usize, description: &str) -> String { + format!(r#"{{"timestamp":"t","seq":{seq},"event":"e","description":"{description}"}}"#) + } + + #[tokio::test] + async fn returns_entries_as_recorded() { + let line = r#"{"timestamp":"t","seq":1,"event":"e","description":"hit","parameters":{"b":"1","a":"2"}}"#; + + let response = search_lines(&format!("{line}\r\n")).await; + + assert_eq!(response.hits.len(), 1); + assert_eq!(response.hits[0].entry.get(), line); + } + + #[tokio::test] + async fn stops_at_the_hit_limit() { + let content: String = (0..=MAX_HITS) + .map(|seq| format!("{}\n", entry_line(seq, "hit"))) + .collect(); + + let response = search_lines(&content).await; + + assert_eq!(response.hits.len(), MAX_HITS); + assert!(response.limit_reached); + } + + #[tokio::test] + async fn skips_lines_longer_than_the_limit() { + let long_line = entry_line(1, &format!("hit{}", "x".repeat(MAX_LINE_BYTES))); + let content = format!( + "{}\n{long_line}\n{}", + entry_line(0, "hit first"), + entry_line(2, "hit last") + ); + + let response = search_lines(&content).await; + + let seqs: Vec<_> = response + .hits + .iter() + .map(|hit| serde_json::from_str::(hit.entry.get()).expect("JSON entry")["seq"].clone()) + .collect(); + assert_eq!(seqs, [serde_json::json!(0), serde_json::json!(2)]); + } + + #[tokio::test] + async fn stops_after_the_scanned_line_limit() { + let mut content = "\n".repeat(MAX_SCANNED_LINES); + content.push_str(&entry_line(0, "hit")); + + let response = search_lines(&content).await; + + assert!(response.hits.is_empty()); + } +} diff --git a/devolutions-gateway/src/extract.rs b/devolutions-gateway/src/extract.rs index b22ee04611..a000e46cf1 100644 --- a/devolutions-gateway/src/extract.rs +++ b/devolutions-gateway/src/extract.rs @@ -321,6 +321,24 @@ where } } +#[derive(Clone, Copy)] +pub struct RecordingsSearchScope; + +impl FromRequestParts for RecordingsSearchScope +where + S: Send + Sync, +{ + type Rejection = HttpError; + + async fn from_request_parts(parts: &mut Parts, state: &S) -> Result { + match ScopeToken::from_request_parts(parts, state).await?.0.scope { + AccessScope::Wildcard => Ok(Self), + AccessScope::RecordingsSearch => Ok(Self), + _ => Err(HttpError::forbidden().msg("invalid scope for route")), + } + } +} + #[derive(Clone, Copy)] pub struct UpdateScope; diff --git a/devolutions-gateway/src/openapi.rs b/devolutions-gateway/src/openapi.rs index 5052f67046..1881ac1fe0 100644 --- a/devolutions-gateway/src/openapi.rs +++ b/devolutions-gateway/src/openapi.rs @@ -24,6 +24,7 @@ use crate::config::dto::{DataEncoding, PubKeyFormat, Subscriber}; crate::api::jrec::list_recordings, crate::api::jrec::pull_recording_session, crate::api::jrec::pull_recording_file, + crate::api::jrec::search::search_recordings, crate::api::webapp::sign_app_token, crate::api::webapp::sign_session_token, crate::api::update::get_update_products, @@ -58,6 +59,11 @@ use crate::config::dto::{DataEncoding, PubKeyFormat, Subscriber}; ConfigPatch, crate::api::jrl::JrlInfo, crate::api::jrec::DeleteManyResult, + crate::api::jrec::search::RecordingSearchRequest, + crate::api::jrec::search::RecordingSelection, + crate::api::jrec::search::AllRecordings, + crate::api::jrec::search::RecordingSearchResponse, + crate::api::jrec::search::RecordingSearchHit, crate::token::AccessScope, crate::api::webapp::AppTokenSignRequest, crate::api::webapp::AppTokenContentType, diff --git a/devolutions-gateway/src/token.rs b/devolutions-gateway/src/token.rs index d7aefef907..c922869a58 100644 --- a/devolutions-gateway/src/token.rs +++ b/devolutions-gateway/src/token.rs @@ -511,6 +511,8 @@ pub enum AccessScope { AgentDelete, #[serde(rename = "gateway.agent.read")] AgentRead, + #[serde(rename = "gateway.recordings.search")] + RecordingsSearch, } #[derive(Clone, Serialize, Deserialize)] diff --git a/testsuite/tests/cli/dgw/mod.rs b/testsuite/tests/cli/dgw/mod.rs index 66adf332e3..5bd4e0c49d 100644 --- a/testsuite/tests/cli/dgw/mod.rs +++ b/testsuite/tests/cli/dgw/mod.rs @@ -3,6 +3,7 @@ mod cli_args; mod heartbeat; mod kdc_proxy; mod preflight; +mod recording_search; mod subscriber; mod tls_anchoring; mod traffic_audit; diff --git a/testsuite/tests/cli/dgw/recording_search.rs b/testsuite/tests/cli/dgw/recording_search.rs new file mode 100644 index 0000000000..396c73aabc --- /dev/null +++ b/testsuite/tests/cli/dgw/recording_search.rs @@ -0,0 +1,201 @@ +//! Recording log search API tests. + +use std::path::Path; + +use anyhow::Context as _; +use base64::Engine as _; +use testsuite::cli::start_dgw; +use testsuite::dgw_config::{DgwConfig, DgwConfigHandle}; +use uuid::Uuid; + +const OLDER_RECORDING: Uuid = Uuid::from_u128(1); +const NEWER_RECORDING: Uuid = Uuid::from_u128(2); + +/// Unsigned scope token; token validation is disabled in these tests. +fn scope_token(scope: &str) -> String { + let b64 = base64::engine::general_purpose::URL_SAFE_NO_PAD; + let header = serde_json::json!({ "typ": "JWT", "alg": "RS256" }); + let payload = serde_json::json!({ + "type": "scope", + "jti": Uuid::new_v4().to_string(), + "iat": 1_733_669_999, + "nbf": 1_733_669_999, + "exp": 3_331_553_599i64, + "scope": scope, + }); + + format!( + "{}.{}.{}", + b64.encode(header.to_string()), + b64.encode(payload.to_string()), + b64.encode("signature-not-checked"), + ) +} + +fn write_recording(root: &Path, id: Uuid, manifest: serde_json::Value, files: &[(&str, &str)]) -> anyhow::Result<()> { + let dir = root.join(id.to_string()); + std::fs::create_dir_all(&dir)?; + std::fs::write(dir.join("recording.json"), manifest.to_string())?; + for (name, content) in files { + std::fs::write(dir.join(name), content)?; + } + Ok(()) +} + +/// Two recordings: an older one recorded as a log next to a video, and a newer one whose log is an AI analysis artifact. +/// The video, and a log outside the recording folder that the older manifest points to, hold the searched text too, +/// to show that only the logs of the recording are searched. +fn write_recordings(root: &Path) -> anyhow::Result<()> { + std::fs::write( + root.join("outside.slog"), + r#"{"timestamp":"2026-07-15T16:15:35.140Z","seq":0,"event":"session.action","description":"Created User"}"#, + )?; + + write_recording( + root, + OLDER_RECORDING, + serde_json::json!({ + "sessionId": OLDER_RECORDING.to_string(), + "startTime": 100, + "duration": 10, + "files": [ + { "fileName": "recording-0.slog", "startTime": 100, "duration": 10 }, + { "fileName": "recording-1.webm", "startTime": 100, "duration": 10 }, + { "fileName": "../outside.slog", "startTime": 100, "duration": 10 }, + ], + }), + &[ + ( + "recording-0.slog", + concat!( + r#"{"timestamp":"2026-07-15T16:15:35.140Z","seq":0,"event":"session.start","description":"Session started"}"#, + "\n", + r#"{"timestamp":"2026-07-15T16:15:41.054Z","seq":1,"event":"session.action","description":"Created User","object":"Help Desk Ottawa"}"#, + "\n", + ), + ), + ("recording-1.webm", "Created User"), + ], + )?; + + write_recording( + root, + NEWER_RECORDING, + serde_json::json!({ + "sessionId": NEWER_RECORDING.to_string(), + "startTime": 200, + "duration": 10, + "files": [{ "fileName": "recording-0.webm", "startTime": 200, "duration": 10 }], + "artifacts": { "ai-analysis": [{ "fileName": "ai-analysis-0.slog" }] }, + }), + &[( + "ai-analysis-0.slog", + r#"{"timestamp":"2026-07-16T09:00:01.000Z","seq":1,"event":"session.action","description":"Opened Users","parameters":{"Created user":"Bob"}}"#, + )], + ) +} + +/// Starts Gateway on the recordings above; the returned folder and process must outlive the test requests. +async fn start_with_recordings() -> anyhow::Result<(tempfile::TempDir, DgwConfigHandle, tokio::process::Child)> { + let recordings = tempfile::tempdir()?; + write_recordings(recordings.path())?; + + let config_handle = DgwConfig::builder() + .disable_token_validation(true) + .recording_path(recordings.path().to_owned()) + .build() + .init()?; + let process = start_dgw(&config_handle).await?; + + Ok((recordings, config_handle, process)) +} + +async fn search(http_port: u16, scope: &str, request: serde_json::Value) -> anyhow::Result { + reqwest::Client::new() + .post(format!("http://127.0.0.1:{http_port}/jet/jrec/search")) + .bearer_auth(scope_token(scope)) + .json(&request) + .send() + .await + .context("send search request") +} + +/// `(recordingId, fileName, entry.seq)` of each hit. +fn hit_summaries(body: &serde_json::Value) -> anyhow::Result> { + body["hits"] + .as_array() + .context("hits")? + .iter() + .map(|hit| { + Ok(( + hit["recordingId"].as_str().context("recordingId")?.to_owned(), + hit["fileName"].as_str().context("fileName")?.to_owned(), + hit["entry"]["seq"].as_u64().context("entry.seq")?, + )) + }) + .collect() +} + +#[tokio::test] +async fn searches_the_logs_of_every_recording_newest_first() -> anyhow::Result<()> { + let (_recordings, config_handle, _process) = start_with_recordings().await?; + + let response = search( + config_handle.http_port(), + "gateway.recordings.search", + serde_json::json!({ "query": "created USER", "recordings": "all" }), + ) + .await?; + assert_eq!(response.status(), reqwest::StatusCode::OK); + + let body: serde_json::Value = response.json().await?; + assert_eq!( + hit_summaries(&body)?, + [ + (NEWER_RECORDING.to_string(), "ai-analysis-0.slog".to_owned(), 1), + (OLDER_RECORDING.to_string(), "recording-0.slog".to_owned(), 1), + ] + ); + assert_eq!(body["limitReached"], false); + + Ok(()) +} + +#[tokio::test] +async fn searches_each_listed_recording_once() -> anyhow::Result<()> { + let (_recordings, config_handle, _process) = start_with_recordings().await?; + + let response = search( + config_handle.http_port(), + "gateway.recordings.search", + serde_json::json!({ + "query": "created user", + "recordings": [OLDER_RECORDING.to_string(), OLDER_RECORDING.to_string()], + }), + ) + .await?; + assert_eq!(response.status(), reqwest::StatusCode::OK); + + let body: serde_json::Value = response.json().await?; + assert_eq!( + hit_summaries(&body)?, + [(OLDER_RECORDING.to_string(), "recording-0.slog".to_owned(), 1)] + ); + + Ok(()) +} + +#[tokio::test] +async fn rejects_a_token_without_the_search_scope() -> anyhow::Result<()> { + let (_recordings, config_handle, _process) = start_with_recordings().await?; + + let response = search( + config_handle.http_port(), + "gateway.recordings.read", + serde_json::json!({ "query": "created", "recordings": "all" }), + ) + .await?; + assert_eq!(response.status(), reqwest::StatusCode::FORBIDDEN); + + Ok(()) +} diff --git a/utils/dotnet/Devolutions.Gateway.Utils.Tests/JsonSerializationTests.cs b/utils/dotnet/Devolutions.Gateway.Utils.Tests/JsonSerializationTests.cs index eeb30c306a..871dc7a23e 100644 --- a/utils/dotnet/Devolutions.Gateway.Utils.Tests/JsonSerializationTests.cs +++ b/utils/dotnet/Devolutions.Gateway.Utils.Tests/JsonSerializationTests.cs @@ -219,6 +219,16 @@ public void ScopeClaimsAgentRead() Assert.Equal(EXPECTED, result); } + [Fact] + public void ScopeClaimsRecordingsSearch() + { + const string EXPECTED = """{"scope":"gateway.recordings.search","jet_gw_id":"ccbaad3f-4627-4666-8bb5-cb6a1a7db815"}"""; + + var claims = new ScopeClaims(gatewayId, AccessScope.GatewayRecordingsSearch); + string result = JsonSerializer.Serialize(claims); + Assert.Equal(EXPECTED, result); + } + [Fact] public void EnrollmentClaimsAllFields() { diff --git a/utils/dotnet/Devolutions.Gateway.Utils/src/AccessScope.cs b/utils/dotnet/Devolutions.Gateway.Utils/src/AccessScope.cs index bc953a2b15..880b866253 100644 --- a/utils/dotnet/Devolutions.Gateway.Utils/src/AccessScope.cs +++ b/utils/dotnet/Devolutions.Gateway.Utils/src/AccessScope.cs @@ -30,6 +30,7 @@ internal AccessScope(string value) public static AccessScope GatewayNetMonitorDrain = new AccessScope("gateway.net.monitor.drain"); public static AccessScope GatewayAgentDelete = new AccessScope("gateway.agent.delete"); public static AccessScope GatewayAgentRead = new AccessScope("gateway.agent.read"); + public static AccessScope GatewayRecordingsSearch = new AccessScope("gateway.recordings.search"); public override string? ToString() { From ce252c824f1b21aa3057145d6d7bf8badc262853 Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Mon, 19 Oct 2026 10:52:09 -0400 Subject: [PATCH 02/19] fix(dgw): measure search log lines like the log viewer The log viewer strips a trailing `\r` before it checks a line against the 256 KiB limit, so a CRLF line of exactly 256 KiB is shown there. The search now strips it first too, also on a last line without `\n`. Mark the search invariants with `INVARIANT:` and give each recording selection case its own test. Issue: DGW-463 Co-Authored-By: Claude Opus 5.5 (1M context) --- devolutions-gateway/src/api/jrec/search.rs | 108 ++++++++++++++------- 1 file changed, 72 insertions(+), 36 deletions(-) diff --git a/devolutions-gateway/src/api/jrec/search.rs b/devolutions-gateway/src/api/jrec/search.rs index 289dc928b0..b0ec9ba4dd 100644 --- a/devolutions-gateway/src/api/jrec/search.rs +++ b/devolutions-gateway/src/api/jrec/search.rs @@ -265,12 +265,12 @@ impl Search { // UNTESTED: the hit-bytes budget stops the search and reports `limitReached`. // It bounds the response size, which the hit count alone does not, because a line can be large. // When changing it, keep both the hit count and the hit bytes bounded. - if self.hits.len() == MAX_HITS || MAX_HIT_BYTES < self.hit_bytes + line.len() { + if MAX_HITS <= self.hits.len() || MAX_HIT_BYTES < self.hit_bytes + line.len() { self.limit_reached = true; return false; } - // The line was parsed as JSON already. + // INVARIANT: `line` was parsed as JSON already, so this never skips a hit. let Ok(entry) = serde_json::from_slice::>(line) else { return true; }; @@ -367,6 +367,7 @@ fn viewer_entry(text: &SearchText, line: &[u8]) -> ViewerEntry { } fn exceeds_max_object_depth(fields: &serde_json::Map) -> bool { + // INVARIANT: `depth` is how deep the value sits, counting the entry object as depth 1. let mut stack: Vec<(&Value, usize)> = fields.values().map(|value| (value, 2)).collect(); while let Some((value, depth)) = stack.pop() { @@ -545,8 +546,11 @@ struct LineRead { /// Reads the next line into `line`, without its line break, and returns `None` at the end of the file. /// -/// The reader never holds more than [`MAX_LINE_BYTES`] of one line in memory. +/// INVARIANT: `line` never holds more than [`MAX_LINE_BYTES`] and a trailing `\r`. async fn read_line(reader: &mut BufReader, line: &mut Vec) -> io::Result> { + // The log parser strips a trailing `\r` before it measures the line. + const MAX_BUFFERED_BYTES: usize = MAX_LINE_BYTES + 1; + line.clear(); let mut read = LineRead { bytes: 0, @@ -556,14 +560,18 @@ async fn read_line(reader: &mut BufReader, line: &mut Vec) -> io::Resu loop { let available = reader.fill_buf().await?; if available.is_empty() { - return Ok((read.bytes != 0).then_some(read)); + if read.bytes == 0 { + return Ok(None); + } + + break; } let line_break = available.iter().position(|&byte| byte == b'\n'); let chunk = &available[..line_break.unwrap_or(available.len())]; if !read.too_long { - if line.len() + chunk.len() <= MAX_LINE_BYTES { + if line.len() + chunk.len() <= MAX_BUFFERED_BYTES { line.extend_from_slice(chunk); } else { read.too_long = true; @@ -576,13 +584,20 @@ async fn read_line(reader: &mut BufReader, line: &mut Vec) -> io::Resu read.bytes += u64::try_from(consumed).expect("buffer length fits in u64"); if line_break.is_some() { - if line.last() == Some(&b'\r') { - line.pop(); - } - - return Ok(Some(read)); + break; } } + + if line.last() == Some(&b'\r') { + line.pop(); + } + + if MAX_LINE_BYTES < line.len() { + read.too_long = true; + line.clear(); + } + + Ok(Some(read)) } #[cfg(test)] @@ -595,7 +610,7 @@ mod tests { SearchText::new(query).expect("non-empty query") } - fn matches(query: &str, line: &str) -> ViewerEntry { + fn viewer_entry_of(query: &str, line: &str) -> ViewerEntry { viewer_entry(&search_text(query), line.as_bytes()) } @@ -607,7 +622,7 @@ mod tests { fn matches_field_values_ignoring_case(#[case] query: &str, #[case] expected: ViewerEntry) { let line = r#"{"timestamp":"t","seq":1,"event":"session.action","description":"Created User","object":"Help Desk Ottawa"}"#; - assert_eq!(matches(query, line), expected); + assert_eq!(viewer_entry_of(query, line), expected); } #[rstest] @@ -621,14 +636,14 @@ mod tests { #[case::too_deep(r#"{"timestamp":"t","seq":1,"event":"e","description":"hit","x":[[[[[[[[1]]]]]]]]}"#)] #[case::not_an_object(r#"["hit"]"#)] fn skips_entries_the_viewers_drop(#[case] line: &str) { - assert_eq!(matches("hit", line), ViewerEntry::Hidden); + assert_eq!(viewer_entry_of("hit", line), ViewerEntry::Hidden); } #[test] fn keeps_an_entry_at_the_max_depth() { let line = r#"{"timestamp":"t","seq":1,"event":"e","description":"hit","x":[[[[[[[1]]]]]]]}"#; - assert_eq!(matches("hit", line), ViewerEntry::Match); + assert_eq!(viewer_entry_of("hit", line), ViewerEntry::Match); } #[test] @@ -636,8 +651,8 @@ mod tests { let description = format!("{}tail", "a".repeat(MAX_STRING_LENGTH)); let line = format!(r#"{{"timestamp":"t","seq":1,"event":"e","description":"{description}"}}"#); - assert_eq!(matches("tail", &line), ViewerEntry::NoMatch); - assert_eq!(matches("aaa", &line), ViewerEntry::Match); + assert_eq!(viewer_entry_of("tail", &line), ViewerEntry::NoMatch); + assert_eq!(viewer_entry_of("aaa", &line), ViewerEntry::Match); } #[rstest] @@ -648,7 +663,7 @@ mod tests { let line = r#"{"timestamp":"t","seq":1,"event":"e","description":"d","parameters":{"New name":"Hull","Count":2}}"#; - assert_eq!(matches(query, line), expected); + assert_eq!(viewer_entry_of(query, line), expected); } #[test] @@ -660,8 +675,8 @@ mod tests { parameters.join(",") ); - assert_eq!(matches("z199", &line), ViewerEntry::Match); - assert_eq!(matches("late", &line), ViewerEntry::NoMatch); + assert_eq!(viewer_entry_of("z199", &line), ViewerEntry::Match); + assert_eq!(viewer_entry_of("late", &line), ViewerEntry::NoMatch); } #[test] @@ -671,8 +686,8 @@ mod tests { r#"{{"timestamp":"t","seq":1,"event":"e","description":"d","parameters":{{"{name}x":"first","{name}y":"second"}}}}"# ); - assert_eq!(matches("first", &line), ViewerEntry::Match); - assert_eq!(matches("second", &line), ViewerEntry::NoMatch); + assert_eq!(viewer_entry_of("first", &line), ViewerEntry::Match); + assert_eq!(viewer_entry_of("second", &line), ViewerEntry::NoMatch); } #[test] @@ -694,26 +709,36 @@ mod tests { assert_eq!(entries, [("2", "4"), ("10", "2"), ("b", "6"), ("a", "3"), ("01", "5")]); } + fn parse_recordings(recordings: &str) -> serde_json::Result { + let request = format!(r#"{{"query":"q","recordings":{recordings}}}"#); + + serde_json::from_str::(&request).map(|request| request.recordings) + } + + #[test] + fn parses_all_recordings() { + let recordings = parse_recordings(r#""all""#).expect("valid selection"); + + assert!(matches!(recordings, RecordingSelection::All(AllRecordings::All))); + } + #[rstest] - #[case::all(r#""all""#, Some(None))] - #[case::listed( + #[case::ids( r#"["00000000-0000-0000-0000-000000000002","00000000-0000-0000-0000-000000000001"]"#, - Some(Some(vec![Uuid::from_u128(2), Uuid::from_u128(1)])) + vec![Uuid::from_u128(2), Uuid::from_u128(1)] )] - #[case::empty_list("[]", Some(Some(Vec::new())))] - #[case::other_word(r#""some""#, None)] - #[case::all_in_a_list(r#"["all"]"#, None)] - fn parses_the_recording_selection(#[case] recordings: &str, #[case] expected: Option>>) { - let request = format!(r#"{{"query":"q","recordings":{recordings}}}"#); + #[case::empty("[]", Vec::new())] + fn parses_listed_recordings(#[case] recordings: &str, #[case] expected: Vec) { + let recordings = parse_recordings(recordings).expect("valid selection"); - let actual = serde_json::from_str::(&request) - .ok() - .map(|request| match request.recordings { - RecordingSelection::All(AllRecordings::All) => None, - RecordingSelection::Listed(recording_ids) => Some(recording_ids), - }); + assert!(matches!(recordings, RecordingSelection::Listed(recording_ids) if recording_ids == expected)); + } - assert_eq!(actual, expected); + #[rstest] + #[case::other_word(r#""some""#)] + #[case::all_in_a_list(r#"["all"]"#)] + fn rejects_other_recording_selections(#[case] recordings: &str) { + assert!(parse_recordings(recordings).is_err()); } async fn search_lines(content: &str) -> RecordingSearchResponse { @@ -775,6 +800,17 @@ mod tests { assert_eq!(seqs, [serde_json::json!(0), serde_json::json!(2)]); } + #[tokio::test] + async fn keeps_a_line_at_the_limit_ending_with_crlf() { + let padding = MAX_LINE_BYTES - entry_line(0, "hit").len(); + let line = entry_line(0, &format!("hit{}", "x".repeat(padding))); + assert_eq!(line.len(), MAX_LINE_BYTES); + + let response = search_lines(&format!("{line}\r\n")).await; + + assert_eq!(response.hits.len(), 1); + } + #[tokio::test] async fn stops_after_the_scanned_line_limit() { let mut content = "\n".repeat(MAX_SCANNED_LINES); From 047406413c7f3793a169a8bae58c9f8bd7d18214 Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Mon, 19 Oct 2026 11:34:46 -0400 Subject: [PATCH 03/19] fix(dgw): inline "all" in the recording selection schema The derived schema put `"all"` in an `AllRecordings` schema of its own, and the C# client generator (7.9.0) turns that `oneOf` member into a constructor that does not compile. The schema now lists `"all"` inline, like `MonitoringProbeType`; the JSON is unchanged. Also test the line limit one byte over it and on a last line ending with `\r`, and keep `INVARIANT:` for definitions only. Issue: DGW-463 Co-Authored-By: Claude Opus 5.5 (1M context) --- devolutions-gateway/src/api/jrec/search.rs | 63 +++++++++++++++++++--- devolutions-gateway/src/openapi.rs | 1 - 2 files changed, 57 insertions(+), 7 deletions(-) diff --git a/devolutions-gateway/src/api/jrec/search.rs b/devolutions-gateway/src/api/jrec/search.rs index b0ec9ba4dd..4cdbe937a2 100644 --- a/devolutions-gateway/src/api/jrec/search.rs +++ b/devolutions-gateway/src/api/jrec/search.rs @@ -57,7 +57,6 @@ pub(crate) struct RecordingSearchRequest { /// `"all"`, or a list of recording IDs. /// /// A repeated ID is searched once, and an empty list searches nothing. -#[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] #[derive(Deserialize)] #[serde(untagged)] pub(crate) enum RecordingSelection { @@ -65,13 +64,42 @@ pub(crate) enum RecordingSelection { Listed(Vec), } -#[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] #[derive(Deserialize)] #[serde(rename_all = "lowercase")] pub(crate) enum AllRecordings { All, } +// The derived schema puts `"all"` in its own enum schema, which version 7.9.0 of the C# client generator turns into +// a `oneOf` constructor that does not compile. +#[cfg(feature = "openapi")] +impl<'__s> utoipa::ToSchema<'__s> for RecordingSelection { + fn schema() -> (&'__s str, utoipa::openapi::RefOr) { + use utoipa::openapi::schema::{ + ArrayBuilder, KnownFormat, ObjectBuilder, OneOfBuilder, SchemaFormat, SchemaType, + }; + + let all = ObjectBuilder::new() + .schema_type(SchemaType::String) + .enum_values(Some(["all"])); + + let listed = ArrayBuilder::new().items( + ObjectBuilder::new() + .schema_type(SchemaType::String) + .format(Some(SchemaFormat::KnownFormat(KnownFormat::Uuid))), + ); + + let schema = OneOfBuilder::new() + .item(all) + .item(listed) + .description(Some( + "`\"all\"`, or a list of recording IDs.\n\nA repeated ID is searched once, and an empty list searches nothing.", + )); + + ("RecordingSelection", schema.into()) + } +} + #[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] #[derive(Serialize)] #[serde(rename_all = "camelCase")] @@ -270,7 +298,7 @@ impl Search { return false; } - // INVARIANT: `line` was parsed as JSON already, so this never skips a hit. + // `viewer_entry` parsed the line as JSON already, so this never skips a hit. let Ok(entry) = serde_json::from_slice::>(line) else { return true; }; @@ -546,7 +574,7 @@ struct LineRead { /// Reads the next line into `line`, without its line break, and returns `None` at the end of the file. /// -/// INVARIANT: `line` never holds more than [`MAX_LINE_BYTES`] and a trailing `\r`. +/// INVARIANT: on return, `line` holds at most [`MAX_LINE_BYTES`] bytes. async fn read_line(reader: &mut BufReader, line: &mut Vec) -> io::Result> { // The log parser strips a trailing `\r` before it measures the line. const MAX_BUFFERED_BYTES: usize = MAX_LINE_BYTES + 1; @@ -729,9 +757,11 @@ mod tests { )] #[case::empty("[]", Vec::new())] fn parses_listed_recordings(#[case] recordings: &str, #[case] expected: Vec) { - let recordings = parse_recordings(recordings).expect("valid selection"); + let RecordingSelection::Listed(recording_ids) = parse_recordings(recordings).expect("valid selection") else { + panic!("expected a list of recording IDs"); + }; - assert!(matches!(recordings, RecordingSelection::Listed(recording_ids) if recording_ids == expected)); + assert_eq!(recording_ids, expected); } #[rstest] @@ -811,6 +841,27 @@ mod tests { assert_eq!(response.hits.len(), 1); } + #[tokio::test] + async fn keeps_a_last_line_at_the_limit_ending_with_cr() { + let padding = MAX_LINE_BYTES - entry_line(0, "hit").len(); + let line = entry_line(0, &format!("hit{}", "x".repeat(padding))); + + let response = search_lines(&format!("{line}\r")).await; + + assert_eq!(response.hits.len(), 1); + } + + #[tokio::test] + async fn skips_a_line_one_byte_over_the_limit() { + let padding = MAX_LINE_BYTES + 1 - entry_line(0, "hit").len(); + let line = entry_line(0, &format!("hit{}", "x".repeat(padding))); + assert_eq!(line.len(), MAX_LINE_BYTES + 1); + + let response = search_lines(&format!("{line}\n")).await; + + assert!(response.hits.is_empty()); + } + #[tokio::test] async fn stops_after_the_scanned_line_limit() { let mut content = "\n".repeat(MAX_SCANNED_LINES); diff --git a/devolutions-gateway/src/openapi.rs b/devolutions-gateway/src/openapi.rs index 1881ac1fe0..b2711e9c9c 100644 --- a/devolutions-gateway/src/openapi.rs +++ b/devolutions-gateway/src/openapi.rs @@ -61,7 +61,6 @@ use crate::config::dto::{DataEncoding, PubKeyFormat, Subscriber}; crate::api::jrec::DeleteManyResult, crate::api::jrec::search::RecordingSearchRequest, crate::api::jrec::search::RecordingSelection, - crate::api::jrec::search::AllRecordings, crate::api::jrec::search::RecordingSearchResponse, crate::api::jrec::search::RecordingSearchHit, crate::token::AccessScope, From 9d0e9c8c0e37704dcfd24cad9aa0e5635aa5b31d Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Tue, 20 Oct 2026 17:04:17 -0400 Subject: [PATCH 04/19] fix(dgw): read at most 16 MiB of logs per search Scanning costs about 16 ms per MiB of logs, so a 256 MiB budget kept one request busy for about 4 seconds. A search now reads at most 16 MiB. Every hit is a line the search has read, so the read budget bounds the response too, and the separate 16 MiB hit-bytes limit goes away. Issue: DGW-463 Co-Authored-By: Claude Opus 5.5 --- devolutions-gateway/src/api/jrec/search.rs | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/devolutions-gateway/src/api/jrec/search.rs b/devolutions-gateway/src/api/jrec/search.rs index 4cdbe937a2..773df685d2 100644 --- a/devolutions-gateway/src/api/jrec/search.rs +++ b/devolutions-gateway/src/api/jrec/search.rs @@ -21,11 +21,8 @@ use crate::token::RecordingFileType; /// Most hits returned by one search. const MAX_HITS: usize = 1_000; -/// Most bytes of matching entries returned by one search. -const MAX_HIT_BYTES: usize = 16 * 1024 * 1024; - /// Most log bytes read by one search, so that searching every recording stays bounded. -const MAX_SCANNED_BYTES: u64 = 256 * 1024 * 1024; +const MAX_SCANNED_BYTES: u64 = 16 * 1024 * 1024; // The log viewers read a `.slog` file through the log parser, which drops or truncates some entries. // The search applies the same limits, so that it finds what the viewers show and nothing else. @@ -210,7 +207,6 @@ impl SearchText { struct Search { text: SearchText, hits: Vec, - hit_bytes: usize, scanned_bytes: u64, limit_reached: bool, } @@ -220,7 +216,6 @@ impl Search { Self { text, hits: Vec::new(), - hit_bytes: 0, scanned_bytes: 0, limit_reached: false, } @@ -243,7 +238,7 @@ impl Search { while let Some(read) = read_line(&mut reader, &mut line).await? { // UNTESTED: the scanned-bytes budget stops the search and reports `limitReached`. - // It keeps a search of every recording from reading an unbounded amount of logs. + // It bounds the logs one search reads, and the response too, since every hit is a line it read. // When changing it, keep `limit_reached` set whenever logs are left unread. self.scanned_bytes += read.bytes; if MAX_SCANNED_BYTES < self.scanned_bytes { @@ -288,12 +283,9 @@ impl Search { Ok(()) } - /// Returns `false` when the hit limits are reached. + /// Returns `false` when the hit limit is reached. fn push(&mut self, recording_id: Uuid, file_name: &str, line: &[u8]) -> bool { - // UNTESTED: the hit-bytes budget stops the search and reports `limitReached`. - // It bounds the response size, which the hit count alone does not, because a line can be large. - // When changing it, keep both the hit count and the hit bytes bounded. - if MAX_HITS <= self.hits.len() || MAX_HIT_BYTES < self.hit_bytes + line.len() { + if MAX_HITS <= self.hits.len() { self.limit_reached = true; return false; } @@ -303,7 +295,6 @@ impl Search { return true; }; - self.hit_bytes += line.len(); self.hits.push(RecordingSearchHit { recording_id, file_name: file_name.to_owned(), From 9fcadd61a2832b225ecb8907edc3cd3aed3fd48f Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Tue, 20 Oct 2026 21:17:53 -0400 Subject: [PATCH 05/19] feat(dgw): search the raw text of slog artifacts Search no longer reads `.slog` lines the way the log viewer's parser does. It matches the query, case-insensitively, against the raw text of each line, so Gateway does not need to understand a log to search it and any kind of artifact can be searched. It reads only the `.slog` files listed under `artifacts` in `recording.json`. A hit is now the line as a string, with its line number and byte offset, since a line is not guaranteed to be JSON. A line over 256 KiB is matched and returned on its first 256 KiB, marked `truncated`, and the rest of it is skipped within the read budget, so a log without line breaks cannot make one request read all of it. The request takes an optional `startTime` range of RFC 3339 times, matched on the start time in `recording.json`, with `from` included and `to` excluded. Recordings that start together are ordered by ID, so the order is the same on every request. Issue: DGW-463 Co-Authored-By: Claude Opus 5.5 --- devolutions-gateway/Cargo.toml | 4 +- devolutions-gateway/src/api/jrec/search.rs | 780 ++++++++------------ testsuite/tests/cli/dgw/recording_search.rs | 104 ++- 3 files changed, 385 insertions(+), 503 deletions(-) diff --git a/devolutions-gateway/Cargo.toml b/devolutions-gateway/Cargo.toml index 25e65c256b..ee26e0adb5 100644 --- a/devolutions-gateway/Cargo.toml +++ b/devolutions-gateway/Cargo.toml @@ -50,7 +50,7 @@ kdc = "0.1" # Serialization serde = { version = "1", features = ["derive"] } -serde_json = { version = "1", features = ["raw_value"] } +serde_json = "1" serde_urlencoded = "0.7" serde-querystring = "0.3.0" @@ -65,7 +65,7 @@ cfg-if = "1.0" url = { version = "2.5", features = ["serde"] } ulid = { version = "1", features = ["serde"] } uuid = { version = "1.23", features = ["v4", "serde"] } -time = { version = "0.3", default-features = false, features = ["std", "serde", "formatting"] } +time = { version = "0.3", default-features = false, features = ["std", "serde", "formatting", "parsing"] } parking_lot = "0.12" anyhow = "1.0" thiserror = "2" diff --git a/devolutions-gateway/src/api/jrec/search.rs b/devolutions-gateway/src/api/jrec/search.rs index 773df685d2..6e74fcc2b3 100644 --- a/devolutions-gateway/src/api/jrec/search.rs +++ b/devolutions-gateway/src/api/jrec/search.rs @@ -1,12 +1,10 @@ -use std::collections::{HashMap, HashSet}; -use std::{fmt, io}; +use std::collections::HashSet; +use std::io; use axum::Json; use axum::extract::State; use camino::{Utf8Path, Utf8PathBuf}; -use serde::de::{self, MapAccess, Visitor}; -use serde_json::Value; -use serde_json::value::RawValue; +use time::OffsetDateTime; use tokio::fs::File; use tokio::io::{AsyncBufReadExt as _, BufReader}; use uuid::Uuid; @@ -21,34 +19,25 @@ use crate::token::RecordingFileType; /// Most hits returned by one search. const MAX_HITS: usize = 1_000; -/// Most log bytes read by one search, so that searching every recording stays bounded. +/// Log bytes one search reads before it stops, so that searching every recording takes a bounded time. +/// +/// A search reads past it by about [`MAX_LINE_BYTES`] at most, to finish reading the start of a line. const MAX_SCANNED_BYTES: u64 = 16 * 1024 * 1024; -// The log viewers read a `.slog` file through the log parser, which drops or truncates some entries. -// The search applies the same limits, so that it finds what the viewers show and nothing else. -// https://github.com/Devolutions/devolutions-gateway/blob/master/webapp/packages/session-recording-log/src/parser.ts - +/// Bytes of a line that are matched and returned; the rest of a longer line is skipped. const MAX_LINE_BYTES: usize = 256 * 1024; -const MAX_SCANNED_LINES: usize = 20_000; -const MAX_PARSED_ENTRIES: usize = 10_000; -const MAX_RETAINED_BYTES: usize = 8 * 1024 * 1024; -/// Counted in UTF-16 code units, like JavaScript string lengths. -const MAX_STRING_LENGTH: usize = 4_096; -const MAX_PARAMETER_COUNT: usize = 200; -const MAX_OBJECT_DEPTH: usize = 8; -/// `Number.MAX_SAFE_INTEGER` in JavaScript. -const MAX_SAFE_INTEGER: f64 = 9_007_199_254_740_991.0; - -/// Optional entry fields searched by the query, in addition to `timestamp` and `description`. -const SEARCHED_OPTIONAL_FIELDS: [&str; 4] = ["object", "actor", "host", "sessionType"]; #[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] #[derive(Deserialize)] +#[serde(rename_all = "camelCase")] pub(crate) struct RecordingSearchRequest { /// Text to search for. query: String, /// The recordings to search. recordings: RecordingSelection, + /// Only search the recordings that started in this range. + #[serde(default)] + start_time: Option, } /// `"all"`, or a list of recording IDs. @@ -97,13 +86,37 @@ impl<'__s> utoipa::ToSchema<'__s> for RecordingSelection { } } +/// A range of recording start times, as written in `recording.json`, that includes `from` and excludes `to`. +/// +/// A missing end leaves that side of the range open. +#[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] +#[derive(Deserialize)] +pub(crate) struct StartTimeRange { + /// Earliest start time searched. + #[serde(default, with = "time::serde::rfc3339::option")] + from: Option, + /// Recordings that start at this time or later are not searched. + #[serde(default, with = "time::serde::rfc3339::option")] + to: Option, +} + +impl StartTimeRange { + /// Whether a recording that started at `start_time`, in Unix seconds, is in the range. + fn contains(&self, start_time: i64) -> bool { + let start_time = i128::from(start_time) * 1_000_000_000; + + self.from.is_none_or(|from| from.unix_timestamp_nanos() <= start_time) + && self.to.is_none_or(|to| start_time < to.unix_timestamp_nanos()) + } +} + #[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] #[derive(Serialize)] #[serde(rename_all = "camelCase")] pub(crate) struct RecordingSearchResponse { - /// Matching entries, newest recording first when searching all recordings, otherwise in the requested order. + /// Matching lines, newest recording first when searching all recordings, otherwise in the requested order. hits: Vec, - /// The search stopped before reading every log, so more entries may match. + /// The search stopped before reading every log, so more lines may match. limit_reached: bool, } @@ -112,18 +125,24 @@ pub(crate) struct RecordingSearchResponse { #[serde(rename_all = "camelCase")] pub(crate) struct RecordingSearchHit { recording_id: Uuid, - /// Name of the `.slog` file holding the entry, as listed in `recording.json`. + /// Name of the `.slog` artifact holding the line, as listed in `recording.json`. file_name: String, - /// The log entry exactly as recorded. - #[cfg_attr(feature = "openapi", schema(value_type = Object))] - entry: Box, + /// Number of the line in the file, counting from 1. + line_number: u64, + /// Byte offset of the line in the file. + offset: u64, + /// The line's text without its line break; bytes that are not UTF-8 read as U+FFFD. + line: String, + /// The line is longer than 256 KiB, so `line` holds only its start, and the file holds the rest. + truncated: bool, } -/// Searches the Session Recording Log (`.slog`) files of recordings stored on this instance +/// Searches the `.slog` artifacts, such as AI analysis logs, of recordings stored on this instance /// -/// The query is a case-insensitive substring, matched against the same entry fields as the log viewers: -/// `timestamp`, `description`, `object`, `actor`, `host`, `sessionType`, and the parameter names and values. -/// Entries the log viewers do not show are not searched, and long strings are matched on the part the viewers show. +/// The query is a case-insensitive substring, matched against the raw text of each line. +/// Gateway does not interpret the logs, so any part of a line can match, field names included. +/// Only the files listed under `artifacts` in `recording.json` with the `slog` type are searched, +/// and a line longer than 256 KiB is matched on its first 256 KiB only. /// Recordings without a readable `recording.json` and IDs with no recording on this instance give no hits. #[cfg_attr(feature = "openapi", utoipa::path( post, @@ -132,7 +151,7 @@ pub(crate) struct RecordingSearchHit { path = "/jet/jrec/search", request_body(content = RecordingSearchRequest, description = "JSON-encoded search request", content_type = "application/json"), responses( - (status = 200, description = "Matching log entries", body = RecordingSearchResponse), + (status = 200, description = "Matching log lines", body = RecordingSearchResponse), (status = 400, description = "Bad request"), (status = 401, description = "Invalid or missing authorization token"), (status = 403, description = "Insufficient permissions"), @@ -148,7 +167,7 @@ pub(crate) async fn search_recordings( let recording_root = conf_handle.get_conf().recording_path.clone(); - let recordings = match request.recordings { + let mut recordings = match request.recordings { RecordingSelection::All(AllRecordings::All) => all_recording_logs_newest_first(&recording_root) .await .map_err(HttpError::internal().with_msg("failed recording listing").err())?, @@ -166,11 +185,15 @@ pub(crate) async fn search_recordings( } }; + if let Some(start_time) = &request.start_time { + recordings.retain(|recording| start_time.contains(recording.start_time)); + } + let mut search = Search::new(text); for recording in recordings { - for file_name in recording.file_names { - let path = recording.dir.join(&file_name); + for file_name in &recording.file_names { + let path = recording.dir.join(file_name); match search.search_file(recording.recording_id, file_name, &path).await { Ok(()) => {} @@ -190,7 +213,7 @@ pub(crate) async fn search_recordings( Ok(Json(search.into_response())) } -/// Search text in lowercase, the case entries are compared in. +/// Search text in lowercase, the case lines are compared in. struct SearchText(String); impl SearchText { @@ -199,8 +222,8 @@ impl SearchText { (!text.is_empty()).then_some(Self(text)) } - fn is_in(&self, value: &str) -> bool { - visible_text(value).to_lowercase().contains(&self.0) + fn is_in(&self, line: &str) -> bool { + line.to_lowercase().contains(&self.0) } } @@ -228,243 +251,60 @@ impl Search { } } - /// Adds the matching entries of one log, reading it the way the log parser does. - async fn search_file(&mut self, recording_id: Uuid, file_name: String, path: &Utf8Path) -> io::Result<()> { + /// Adds the lines of one log that hold the search text. + async fn search_file(&mut self, recording_id: Uuid, file_name: &str, path: &Utf8Path) -> io::Result<()> { let mut reader = BufReader::new(File::open(path).await?); let mut line = Vec::new(); - let mut scanned_lines = 0; - let mut parsed_entries = 0; - let mut retained_bytes = 0; - - while let Some(read) = read_line(&mut reader, &mut line).await? { - // UNTESTED: the scanned-bytes budget stops the search and reports `limitReached`. - // It bounds the logs one search reads, and the response too, since every hit is a line it read. - // When changing it, keep `limit_reached` set whenever logs are left unread. - self.scanned_bytes += read.bytes; - if MAX_SCANNED_BYTES < self.scanned_bytes { + let mut line_number = 0; + let mut offset = 0; + + loop { + if MAX_SCANNED_BYTES <= self.scanned_bytes { self.limit_reached = true; return Ok(()); } - scanned_lines += 1; - if MAX_SCANNED_LINES < scanned_lines { - break; - } - - if !read.too_long && line.iter().all(u8::is_ascii_whitespace) { - continue; - } - - if MAX_PARSED_ENTRIES <= parsed_entries { - break; - } - - if read.too_long { - continue; - } - - let is_match = match viewer_entry(&self.text, &line) { - ViewerEntry::Hidden => continue, - ViewerEntry::Match => true, - ViewerEntry::NoMatch => false, + let Some(read) = read_line(&mut reader, &mut line).await? else { + return Ok(()); }; + self.scanned_bytes += read.bytes; + line_number += 1; + let line_offset = offset; + offset += read.bytes; + + let text = String::from_utf8_lossy(&line); + if self.text.is_in(&text) { + if MAX_HITS <= self.hits.len() { + self.limit_reached = true; + return Ok(()); + } - if MAX_RETAINED_BYTES < retained_bytes + line.len() { - break; - } - retained_bytes += line.len(); - parsed_entries += 1; - - if is_match && !self.push(recording_id, &file_name, &line) { - return Ok(()); + self.hits.push(RecordingSearchHit { + recording_id, + file_name: file_name.to_owned(), + line_number, + offset: line_offset, + line: text.into_owned(), + truncated: read.truncated, + }); } - } - - Ok(()) - } - - /// Returns `false` when the hit limit is reached. - fn push(&mut self, recording_id: Uuid, file_name: &str, line: &[u8]) -> bool { - if MAX_HITS <= self.hits.len() { - self.limit_reached = true; - return false; - } - // `viewer_entry` parsed the line as JSON already, so this never skips a hit. - let Ok(entry) = serde_json::from_slice::>(line) else { - return true; - }; - - self.hits.push(RecordingSearchHit { - recording_id, - file_name: file_name.to_owned(), - entry, - }); - - true - } -} - -/// How the log viewers see the entry on one line, given the search text. -#[derive(Debug, PartialEq, Eq)] -enum ViewerEntry { - /// The viewers drop the entry. - Hidden, - /// The viewers show the entry, and the search text is in it. - Match, - /// The viewers show the entry, and the search text is not in it. - NoMatch, -} + if read.rest_unread { + let budget = MAX_SCANNED_BYTES.saturating_sub(self.scanned_bytes); + let skipped = skip_rest_of_line(&mut reader, budget).await?; + self.scanned_bytes += skipped.bytes; + offset += skipped.bytes; -fn viewer_entry(text: &SearchText, line: &[u8]) -> ViewerEntry { - return shown_entry_matches(text, line).map_or(ViewerEntry::Hidden, |is_match| match is_match { - true => ViewerEntry::Match, - false => ViewerEntry::NoMatch, - }); - - fn shown_entry_matches(text: &SearchText, line: &[u8]) -> Option { - let Ok(Value::Object(fields)) = serde_json::from_slice::(line) else { - return None; - }; - - if exceeds_max_object_depth(&fields) { - return None; - } - - let event = fields.get("event")?.as_str()?; - if MAX_STRING_LENGTH < event.encode_utf16().count() { - return None; - } - - let timestamp = fields.get("timestamp")?.as_str()?; - let description = fields.get("description")?.as_str()?; - - let seq = fields.get("seq")?.as_f64()?; - if !(seq.fract() == 0.0 && (0.0..=MAX_SAFE_INTEGER).contains(&seq)) { - return None; - } - - let in_fields = [timestamp, description] - .into_iter() - .chain( - SEARCHED_OPTIONAL_FIELDS - .iter() - .filter_map(|field| fields.get(*field)?.as_str()), - ) - .any(|value| text.is_in(value)); - - if in_fields { - return Some(true); - } - - if !fields.get("parameters").is_some_and(Value::is_object) { - return Some(false); - } - - // A `serde_json::Map` sorts its keys, and which parameters the viewers keep depends on their order. - #[derive(Deserialize)] - struct LineParameters { - parameters: OrderedObject, - } - - let Ok(LineParameters { parameters }) = serde_json::from_slice(line) else { - return Some(false); - }; - - let mut kept_names = HashSet::new(); - - let in_parameters = parameters - .0 - .iter() - .take(MAX_PARAMETER_COUNT) - .filter_map(|(name, value)| Some((visible_text(name), value.as_str()?))) - .filter(|(name, _)| kept_names.insert(*name)) - .any(|(name, value)| text.is_in(name) || text.is_in(value)); - - Some(in_parameters) - } -} - -fn exceeds_max_object_depth(fields: &serde_json::Map) -> bool { - // INVARIANT: `depth` is how deep the value sits, counting the entry object as depth 1. - let mut stack: Vec<(&Value, usize)> = fields.values().map(|value| (value, 2)).collect(); - - while let Some((value, depth)) = stack.pop() { - match value { - Value::Array(_) | Value::Object(_) if MAX_OBJECT_DEPTH < depth => return true, - Value::Array(items) => stack.extend(items.iter().map(|child| (child, depth + 1))), - Value::Object(fields) => stack.extend(fields.values().map(|child| (child, depth + 1))), - _ => {} - } - } - - false -} - -/// The part of a string the log viewers keep: its first [`MAX_STRING_LENGTH`] UTF-16 code units. -fn visible_text(value: &str) -> &str { - let mut code_units = 0; - - for (index, character) in value.char_indices() { - code_units += character.len_utf16(); - - if MAX_STRING_LENGTH < code_units { - return &value[..index]; - } - } - - value -} - -/// A JSON object's entries in the order JavaScript enumerates them after `JSON.parse`. -/// -/// Integer-like names come first in ascending order, then the other names in written order. -/// A repeated name keeps its first position and its last value. -struct OrderedObject(Vec<(String, Value)>); - -impl<'de> serde::Deserialize<'de> for OrderedObject { - fn deserialize>(deserializer: D) -> Result { - struct OrderedObjectVisitor; - - impl<'de> Visitor<'de> for OrderedObjectVisitor { - type Value = OrderedObject; - - fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - formatter.write_str("a JSON object") - } - - fn visit_map>(self, mut map: A) -> Result { - let mut entries: Vec<(String, Value)> = Vec::new(); - let mut positions: HashMap = HashMap::new(); - - while let Some((name, value)) = map.next_entry::()? { - match positions.get(&name) { - Some(&position) => entries[position].1 = value, - None => { - positions.insert(name.clone(), entries.len()); - entries.push((name, value)); - } - } + if !skipped.finished { + self.limit_reached = true; + return Ok(()); } - - // A stable sort keeps the written order among the other names. - entries.sort_by_key(|(name, _)| array_index(name).map_or((1, 0), |index| (0, index))); - - Ok(OrderedObject(entries)) } } - - return deserializer.deserialize_map(OrderedObjectVisitor); - - /// The array index a property name stands for in JavaScript: a canonical decimal below `u32::MAX`. - fn array_index(name: &str) -> Option { - let index = name.parse::().ok()?; - (index < u32::MAX && index.to_string() == name).then_some(index) - } } } -/// The `.slog` files of one recording. +/// The `.slog` artifacts of one recording. struct RecordingLogs { recording_id: Uuid, dir: Utf8PathBuf, @@ -472,23 +312,16 @@ struct RecordingLogs { file_names: Vec, } -/// Lists the recording logs and the log artifacts, such as the AI analysis, from `recording.json`. +/// Lists the `.slog` artifacts of a recording, such as its AI analysis, from `recording.json`. async fn read_recording_logs(dir: Utf8PathBuf, recording_id: Uuid) -> Option { #[derive(Deserialize)] #[serde(rename_all = "camelCase")] struct Manifest { start_time: i64, - files: Vec, #[serde(default)] artifacts: JrecArtifacts, } - #[derive(Deserialize)] - #[serde(rename_all = "camelCase")] - struct ManifestFile { - file_name: String, - } - let path = dir.join("recording.json"); // Every search reads every manifest again, so a failure here is not worth more than a debug event. @@ -502,10 +335,9 @@ async fn read_recording_logs(dir: Utf8PathBuf, recording_id: Uuid) -> Option Option anyhow::Result> { let mut read_dir = match tokio::fs::read_dir(recording_root).await { Ok(read_dir) => read_dir, @@ -551,7 +383,8 @@ async fn all_recording_logs_newest_first(recording_root: &Utf8Path) -> anyhow::R recordings.extend(read_recording_logs(dir, recording_id).await); } - recordings.sort_unstable_by(|a, b| b.start_time.cmp(&a.start_time)); + // Ordering by ID too keeps the order of recordings that start together the same on every request. + recordings.sort_unstable_by(|a, b| (b.start_time, b.recording_id).cmp(&(a.start_time, a.recording_id))); Ok(recordings) } @@ -559,307 +392,306 @@ async fn all_recording_logs_newest_first(recording_root: &Utf8Path) -> anyhow::R struct LineRead { /// Bytes consumed, line break included. bytes: u64, - /// The line is longer than [`MAX_LINE_BYTES`]; it is then read as empty. - too_long: bool, + /// The line is longer than [`MAX_LINE_BYTES`], so `line` holds only its start. + truncated: bool, + /// The rest of a truncated line, up to its line break, is still to be read. + rest_unread: bool, } /// Reads the next line into `line`, without its line break, and returns `None` at the end of the file. /// +/// A line longer than [`MAX_LINE_BYTES`] is cut to its start, at a character boundary. +/// /// INVARIANT: on return, `line` holds at most [`MAX_LINE_BYTES`] bytes. async fn read_line(reader: &mut BufReader, line: &mut Vec) -> io::Result> { - // The log parser strips a trailing `\r` before it measures the line. - const MAX_BUFFERED_BYTES: usize = MAX_LINE_BYTES + 1; + // Room for a line of exactly `MAX_LINE_BYTES` and its `\r\n`, so that it is not taken for a longer one. + const MAX_BUFFERED_BYTES: usize = MAX_LINE_BYTES + 2; line.clear(); - let mut read = LineRead { - bytes: 0, - too_long: false, - }; + let mut bytes = 0; + let mut ended = false; loop { let available = reader.fill_buf().await?; if available.is_empty() { - if read.bytes == 0 { + if bytes == 0 { return Ok(None); } + ended = true; break; } - let line_break = available.iter().position(|&byte| byte == b'\n'); - let chunk = &available[..line_break.unwrap_or(available.len())]; + let room = MAX_BUFFERED_BYTES - line.len(); + let window = &available[..available.len().min(room)]; + let line_break = window.iter().position(|&byte| byte == b'\n'); + line.extend_from_slice(&window[..line_break.unwrap_or(window.len())]); - if !read.too_long { - if line.len() + chunk.len() <= MAX_BUFFERED_BYTES { - line.extend_from_slice(chunk); - } else { - read.too_long = true; - line.clear(); - } - } - - let consumed = line_break.map_or(available.len(), |index| index + 1); + let consumed = line_break.map_or(window.len(), |index| index + 1); reader.consume(consumed); - read.bytes += u64::try_from(consumed).expect("buffer length fits in u64"); + bytes += u64::try_from(consumed).expect("buffer length fits in u64"); if line_break.is_some() { + ended = true; + break; + } + + if line.len() == MAX_BUFFERED_BYTES { break; } } - if line.last() == Some(&b'\r') { + if ended && line.last() == Some(&b'\r') { line.pop(); } - if MAX_LINE_BYTES < line.len() { - read.too_long = true; - line.clear(); + let truncated = MAX_LINE_BYTES < line.len(); + if truncated { + line.truncate(MAX_LINE_BYTES); + line.truncate(len_without_cut_character(line)); } - Ok(Some(read)) + Ok(Some(LineRead { + bytes, + truncated, + rest_unread: !ended, + })) } -#[cfg(test)] -mod tests { - use rstest::rstest; - - use super::*; - - fn search_text(query: &str) -> SearchText { - SearchText::new(query).expect("non-empty query") +/// Length of `line` without a UTF-8 character that is cut at its end. +fn len_without_cut_character(line: &[u8]) -> usize { + match std::str::from_utf8(line) { + Err(error) if error.error_len().is_none() => error.valid_up_to(), + _ => line.len(), } +} - fn viewer_entry_of(query: &str, line: &str) -> ViewerEntry { - viewer_entry(&search_text(query), line.as_bytes()) - } +struct Skipped { + /// Bytes consumed, line break included. + bytes: u64, + /// The line break or the end of the file was reached. + finished: bool, +} - #[rstest] - #[case::description("created user", ViewerEntry::Match)] - #[case::object("desk ott", ViewerEntry::Match)] - #[case::field_name("description", ViewerEntry::NoMatch)] - #[case::event("session", ViewerEntry::NoMatch)] - fn matches_field_values_ignoring_case(#[case] query: &str, #[case] expected: ViewerEntry) { - let line = r#"{"timestamp":"t","seq":1,"event":"session.action","description":"Created User","object":"Help Desk Ottawa"}"#; +/// Consumes the rest of a line up to its line break, reading at most `budget` bytes, +/// so that a log without line breaks cannot make one search read all of it. +async fn skip_rest_of_line(reader: &mut BufReader, budget: u64) -> io::Result { + let mut bytes = 0; - assert_eq!(viewer_entry_of(query, line), expected); - } + while bytes < budget { + let available = reader.fill_buf().await?; + if available.is_empty() { + return Ok(Skipped { bytes, finished: true }); + } - #[rstest] - #[case::no_seq(r#"{"timestamp":"t","event":"e","description":"hit"}"#)] - #[case::negative_seq(r#"{"timestamp":"t","seq":-1,"event":"e","description":"hit"}"#)] - #[case::unsafe_seq(r#"{"timestamp":"t","seq":9007199254740992,"event":"e","description":"hit"}"#)] - #[case::fractional_seq(r#"{"timestamp":"t","seq":1.5,"event":"e","description":"hit"}"#)] - #[case::number_timestamp(r#"{"timestamp":1,"seq":1,"event":"e","description":"hit"}"#)] - #[case::no_event(r#"{"timestamp":"t","seq":1,"description":"hit"}"#)] - #[case::no_description(r#"{"timestamp":"t","seq":1,"event":"e","object":"hit"}"#)] - #[case::too_deep(r#"{"timestamp":"t","seq":1,"event":"e","description":"hit","x":[[[[[[[[1]]]]]]]]}"#)] - #[case::not_an_object(r#"["hit"]"#)] - fn skips_entries_the_viewers_drop(#[case] line: &str) { - assert_eq!(viewer_entry_of("hit", line), ViewerEntry::Hidden); - } + let room = usize::try_from(budget - bytes).unwrap_or(usize::MAX); + let window = &available[..available.len().min(room)]; + let line_break = window.iter().position(|&byte| byte == b'\n'); - #[test] - fn keeps_an_entry_at_the_max_depth() { - let line = r#"{"timestamp":"t","seq":1,"event":"e","description":"hit","x":[[[[[[[1]]]]]]]}"#; + let consumed = line_break.map_or(window.len(), |index| index + 1); + reader.consume(consumed); + bytes += u64::try_from(consumed).expect("buffer length fits in u64"); - assert_eq!(viewer_entry_of("hit", line), ViewerEntry::Match); + if line_break.is_some() { + return Ok(Skipped { bytes, finished: true }); + } } - #[test] - fn matches_only_the_visible_part_of_long_strings() { - let description = format!("{}tail", "a".repeat(MAX_STRING_LENGTH)); - let line = format!(r#"{{"timestamp":"t","seq":1,"event":"e","description":"{description}"}}"#); + Ok(Skipped { bytes, finished: false }) +} - assert_eq!(viewer_entry_of("tail", &line), ViewerEntry::NoMatch); - assert_eq!(viewer_entry_of("aaa", &line), ViewerEntry::Match); - } +#[cfg(test)] +mod tests { + use rstest::rstest; - #[rstest] - #[case::name("new name", ViewerEntry::Match)] - #[case::value("hull", ViewerEntry::Match)] - #[case::non_string_value("count", ViewerEntry::NoMatch)] - fn matches_string_parameters_only(#[case] query: &str, #[case] expected: ViewerEntry) { - let line = - r#"{"timestamp":"t","seq":1,"event":"e","description":"d","parameters":{"New name":"Hull","Count":2}}"#; + use super::*; - assert_eq!(viewer_entry_of(query, line), expected); - } + async fn search_log(query: &str, content: &[u8]) -> Search { + let dir = tempfile::tempdir().expect("temp dir"); + let path = Utf8PathBuf::from_path_buf(dir.path().join("ai-analysis-0.slog")).expect("UTF-8 path"); + std::fs::write(&path, content).expect("write log"); - #[test] - fn matches_the_first_parameters_in_written_order() { - let mut parameters: Vec = (0..MAX_PARAMETER_COUNT).map(|i| format!(r#""z{i}":"v""#)).collect(); - parameters.push(r#""a-late":"late""#.to_owned()); - let line = format!( - r#"{{"timestamp":"t","seq":1,"event":"e","description":"d","parameters":{{{}}}}}"#, - parameters.join(",") - ); + let mut search = Search::new(SearchText::new(query).expect("non-empty query")); + search + .search_file(Uuid::nil(), "ai-analysis-0.slog", &path) + .await + .expect("search log"); - assert_eq!(viewer_entry_of("z199", &line), ViewerEntry::Match); - assert_eq!(viewer_entry_of("late", &line), ViewerEntry::NoMatch); + search } - #[test] - fn keeps_the_first_parameter_whose_name_collides_after_truncation() { - let name = "n".repeat(MAX_STRING_LENGTH); - let line = format!( - r#"{{"timestamp":"t","seq":1,"event":"e","description":"d","parameters":{{"{name}x":"first","{name}y":"second"}}}}"# - ); - - assert_eq!(viewer_entry_of("first", &line), ViewerEntry::Match); - assert_eq!(viewer_entry_of("second", &line), ViewerEntry::NoMatch); + fn hit_lines(search: &Search) -> Vec<(u64, &str)> { + search + .hits + .iter() + .map(|hit| (hit.line_number, hit.line.as_str())) + .collect() } - #[test] - fn orders_parameters_like_javascript() { - let line = r#"{"timestamp":"t","seq":1,"event":"e","description":"d","parameters":{"b":"1","10":"2","a":"3","2":"4","01":"5","b":"6"}}"#; - - #[derive(Deserialize)] - struct LineParameters { - parameters: OrderedObject, - } + #[rstest] + #[case::value_in_another_case("created USER")] + #[case::field_name("description")] + #[case::event_name("session.action")] + #[tokio::test] + async fn matches_any_part_of_the_raw_line(#[case] query: &str) { + let line = r#"{"seq":1,"event":"session.action","description":"Created User"}"#; - let LineParameters { parameters } = serde_json::from_str(line).expect("parameters"); - let entries: Vec<_> = parameters - .0 - .iter() - .map(|(name, value)| (name.as_str(), value.as_str().expect("string value"))) - .collect(); + let search = search_log(query, line.as_bytes()).await; - assert_eq!(entries, [("2", "4"), ("10", "2"), ("b", "6"), ("a", "3"), ("01", "5")]); + assert_eq!(hit_lines(&search), [(1, line)]); } - fn parse_recordings(recordings: &str) -> serde_json::Result { - let request = format!(r#"{{"query":"q","recordings":{recordings}}}"#); + #[tokio::test] + async fn returns_a_line_that_is_not_json() { + let search = search_log("sess", br#"{"seq":12,"event":"sess"#).await; - serde_json::from_str::(&request).map(|request| request.recordings) + assert_eq!(hit_lines(&search), [(1, r#"{"seq":12,"event":"sess"#)]); } - #[test] - fn parses_all_recordings() { - let recordings = parse_recordings(r#""all""#).expect("valid selection"); + #[tokio::test] + async fn gives_the_line_number_and_byte_offset_of_each_hit() { + let search = search_log("hit", b"first\r\nsecond hit\r\nthird\r\nfourth hit\n").await; - assert!(matches!(recordings, RecordingSelection::All(AllRecordings::All))); + let positions: Vec<_> = search + .hits + .iter() + .map(|hit| (hit.line_number, hit.offset, hit.line.as_str())) + .collect(); + assert_eq!(positions, [(2, 7, "second hit"), (4, 26, "fourth hit")]); } - #[rstest] - #[case::ids( - r#"["00000000-0000-0000-0000-000000000002","00000000-0000-0000-0000-000000000001"]"#, - vec![Uuid::from_u128(2), Uuid::from_u128(1)] - )] - #[case::empty("[]", Vec::new())] - fn parses_listed_recordings(#[case] recordings: &str, #[case] expected: Vec) { - let RecordingSelection::Listed(recording_ids) = parse_recordings(recordings).expect("valid selection") else { - panic!("expected a list of recording IDs"); - }; + #[tokio::test] + async fn keeps_a_line_at_the_limit_ending_with_crlf_whole() { + let line = format!("hit{}", "x".repeat(MAX_LINE_BYTES - 3)); - assert_eq!(recording_ids, expected); - } + let search = search_log("hit", format!("{line}\r\n").as_bytes()).await; - #[rstest] - #[case::other_word(r#""some""#)] - #[case::all_in_a_list(r#"["all"]"#)] - fn rejects_other_recording_selections(#[case] recordings: &str) { - assert!(parse_recordings(recordings).is_err()); + assert_eq!(search.hits.len(), 1); + assert_eq!(search.hits[0].line, line); + assert!(!search.hits[0].truncated); } - async fn search_lines(content: &str) -> RecordingSearchResponse { - let dir = tempfile::tempdir().expect("temp dir"); - let path = Utf8PathBuf::from_path_buf(dir.path().join("recording-0.slog")).expect("UTF-8 path"); - std::fs::write(&path, content).expect("write log"); + #[tokio::test] + async fn cuts_a_longer_line_and_keeps_reading_after_it() { + let content = format!("hit{}\nhit again\n", "x".repeat(MAX_LINE_BYTES)); - let mut search = Search::new(search_text("hit")); - search - .search_file(Uuid::nil(), "recording-0.slog".to_owned(), &path) - .await - .expect("search log"); + let search = search_log("hit", content.as_bytes()).await; - search.into_response() + assert_eq!(search.hits.len(), 2); + assert_eq!(search.hits[0].line.len(), MAX_LINE_BYTES); + assert!(search.hits[0].truncated); + assert_eq!(hit_lines(&search)[1], (2, "hit again")); } - fn entry_line(seq: usize, description: &str) -> String { - format!(r#"{{"timestamp":"t","seq":{seq},"event":"e","description":"{description}"}}"#) + #[tokio::test] + async fn does_not_match_past_the_first_256_kib_of_a_line() { + let content = format!("{}hit\n", "x".repeat(MAX_LINE_BYTES)); + + let search = search_log("hit", content.as_bytes()).await; + + assert!(search.hits.is_empty()); } #[tokio::test] - async fn returns_entries_as_recorded() { - let line = r#"{"timestamp":"t","seq":1,"event":"e","description":"hit","parameters":{"b":"1","a":"2"}}"#; + async fn cuts_a_long_line_on_a_character_boundary() { + // "€" is 3 bytes, so the limit falls 1 byte into a character. + let content = format!("hit{}", "€".repeat(MAX_LINE_BYTES / 3)); - let response = search_lines(&format!("{line}\r\n")).await; + let search = search_log("hit", content.as_bytes()).await; - assert_eq!(response.hits.len(), 1); - assert_eq!(response.hits[0].entry.get(), line); + assert_eq!(search.hits[0].line.len(), MAX_LINE_BYTES - 1); + assert!(!search.hits[0].line.contains('\u{FFFD}')); } #[tokio::test] async fn stops_at_the_hit_limit() { - let content: String = (0..=MAX_HITS) - .map(|seq| format!("{}\n", entry_line(seq, "hit"))) - .collect(); + let content = "hit\n".repeat(MAX_HITS + 1); - let response = search_lines(&content).await; + let search = search_log("hit", content.as_bytes()).await; - assert_eq!(response.hits.len(), MAX_HITS); - assert!(response.limit_reached); + assert_eq!(search.hits.len(), MAX_HITS); + assert!(search.limit_reached); } #[tokio::test] - async fn skips_lines_longer_than_the_limit() { - let long_line = entry_line(1, &format!("hit{}", "x".repeat(MAX_LINE_BYTES))); - let content = format!( - "{}\n{long_line}\n{}", - entry_line(0, "hit first"), - entry_line(2, "hit last") - ); + async fn stops_inside_a_line_without_breaks_at_the_read_budget() { + let budget = usize::try_from(MAX_SCANNED_BYTES).expect("budget fits in usize"); + let mut content = b"hit\n".to_vec(); + content.resize(content.len() + budget + 1024 * 1024, b'x'); + content.extend_from_slice(b"\nhit\n"); - let response = search_lines(&content).await; + let search = search_log("hit", &content).await; - let seqs: Vec<_> = response - .hits - .iter() - .map(|hit| serde_json::from_str::(hit.entry.get()).expect("JSON entry")["seq"].clone()) - .collect(); - assert_eq!(seqs, [serde_json::json!(0), serde_json::json!(2)]); + assert_eq!(hit_lines(&search), [(1, "hit")]); + assert!(search.limit_reached); + assert!(search.scanned_bytes <= MAX_SCANNED_BYTES); } - #[tokio::test] - async fn keeps_a_line_at_the_limit_ending_with_crlf() { - let padding = MAX_LINE_BYTES - entry_line(0, "hit").len(); - let line = entry_line(0, &format!("hit{}", "x".repeat(padding))); - assert_eq!(line.len(), MAX_LINE_BYTES); + const SECOND: i128 = 1_000_000_000; - let response = search_lines(&format!("{line}\r\n")).await; + fn at(unix_nanos: i128) -> Option { + Some(OffsetDateTime::from_unix_timestamp_nanos(unix_nanos).expect("valid time")) + } - assert_eq!(response.hits.len(), 1); + #[rstest] + #[case::from_is_included(at(100 * SECOND), None, true)] + #[case::from_a_fraction_later(at(100 * SECOND + SECOND / 2), None, false)] + #[case::to_is_excluded(None, at(100 * SECOND), false)] + #[case::to_a_fraction_later(None, at(100 * SECOND + 1), true)] + #[case::open(None, None, true)] + fn keeps_start_times_from_from_up_to_to( + #[case] from: Option, + #[case] to: Option, + #[case] contains: bool, + ) { + assert_eq!(StartTimeRange { from, to }.contains(100), contains); } - #[tokio::test] - async fn keeps_a_last_line_at_the_limit_ending_with_cr() { - let padding = MAX_LINE_BYTES - entry_line(0, "hit").len(); - let line = entry_line(0, &format!("hit{}", "x".repeat(padding))); + fn parse_request(fields: &str) -> serde_json::Result { + serde_json::from_str(&format!(r#"{{"query":"q",{fields}}}"#)) + } - let response = search_lines(&format!("{line}\r")).await; + #[test] + fn parses_start_times_with_fractions_and_offsets() { + let request = parse_request( + r#""recordings":"all","startTime":{"from":"2026-10-10T00:00:00.000Z","to":"2026-10-10T02:00:00+02:00"}"#, + ) + .expect("valid request"); - assert_eq!(response.hits.len(), 1); + let start_time = request.start_time.expect("start time range"); + assert!(start_time.from.is_some()); + assert_eq!(start_time.from, start_time.to); } - #[tokio::test] - async fn skips_a_line_one_byte_over_the_limit() { - let padding = MAX_LINE_BYTES + 1 - entry_line(0, "hit").len(); - let line = entry_line(0, &format!("hit{}", "x".repeat(padding))); - assert_eq!(line.len(), MAX_LINE_BYTES + 1); - - let response = search_lines(&format!("{line}\n")).await; + #[test] + fn parses_all_recordings() { + let request = parse_request(r#""recordings":"all""#).expect("valid request"); - assert!(response.hits.is_empty()); + assert!(matches!( + request.recordings, + RecordingSelection::All(AllRecordings::All) + )); } - #[tokio::test] - async fn stops_after_the_scanned_line_limit() { - let mut content = "\n".repeat(MAX_SCANNED_LINES); - content.push_str(&entry_line(0, "hit")); + #[rstest] + #[case::ids( + r#"["00000000-0000-0000-0000-000000000002","00000000-0000-0000-0000-000000000001"]"#, + vec![Uuid::from_u128(2), Uuid::from_u128(1)] + )] + #[case::empty("[]", Vec::new())] + fn parses_listed_recordings(#[case] recordings: &str, #[case] expected: Vec) { + let request = parse_request(&format!(r#""recordings":{recordings}"#)).expect("valid request"); - let response = search_lines(&content).await; + let RecordingSelection::Listed(recording_ids) = request.recordings else { + panic!("expected a list of recording IDs"); + }; + assert_eq!(recording_ids, expected); + } - assert!(response.hits.is_empty()); + #[rstest] + #[case::other_word(r#""some""#)] + #[case::all_in_a_list(r#"["all"]"#)] + fn rejects_other_recording_selections(#[case] recordings: &str) { + assert!(parse_request(&format!(r#""recordings":{recordings}"#)).is_err()); } } diff --git a/testsuite/tests/cli/dgw/recording_search.rs b/testsuite/tests/cli/dgw/recording_search.rs index 396c73aabc..12106aadd1 100644 --- a/testsuite/tests/cli/dgw/recording_search.rs +++ b/testsuite/tests/cli/dgw/recording_search.rs @@ -42,9 +42,25 @@ fn write_recording(root: &Path, id: Uuid, manifest: serde_json::Value, files: &[ Ok(()) } -/// Two recordings: an older one recorded as a log next to a video, and a newer one whose log is an AI analysis artifact. -/// The video, and a log outside the recording folder that the older manifest points to, hold the searched text too, -/// to show that only the logs of the recording are searched. +/// A recording whose only log is the AI analysis artifact `ai-analysis-0.slog` holding `log`. +fn write_analysis_recording(root: &Path, id: Uuid, start_time: i64, log: &str) -> anyhow::Result<()> { + write_recording( + root, + id, + serde_json::json!({ + "sessionId": id.to_string(), + "startTime": start_time, + "duration": 10, + "files": [{ "fileName": "recording-0.webm", "startTime": start_time, "duration": 10 }], + "artifacts": { "ai-analysis": [{ "fileName": "ai-analysis-0.slog" }] }, + }), + &[("ai-analysis-0.slog", log)], + ) +} + +/// Two recordings whose AI analysis logs hold the searched text. +/// The older one also holds it in a recorded `.slog`, in its video, and in a log outside the recording folder that +/// its manifest points to, to show that only the `.slog` artifacts of a recording are searched. fn write_recordings(root: &Path) -> anyhow::Result<()> { std::fs::write( root.join("outside.slog"), @@ -61,12 +77,19 @@ fn write_recordings(root: &Path) -> anyhow::Result<()> { "files": [ { "fileName": "recording-0.slog", "startTime": 100, "duration": 10 }, { "fileName": "recording-1.webm", "startTime": 100, "duration": 10 }, - { "fileName": "../outside.slog", "startTime": 100, "duration": 10 }, ], + "artifacts": { + "ai-analysis": [{ "fileName": "ai-analysis-0.slog" }, { "fileName": "../outside.slog" }], + }, }), &[ ( "recording-0.slog", + r#"{"timestamp":"2026-07-15T16:15:41.054Z","seq":1,"event":"session.action","description":"Created User"}"#, + ), + ("recording-1.webm", "Created User"), + ( + "ai-analysis-0.slog", concat!( r#"{"timestamp":"2026-07-15T16:15:35.140Z","seq":0,"event":"session.start","description":"Session started"}"#, "\n", @@ -74,32 +97,21 @@ fn write_recordings(root: &Path) -> anyhow::Result<()> { "\n", ), ), - ("recording-1.webm", "Created User"), ], )?; - write_recording( + write_analysis_recording( root, NEWER_RECORDING, - serde_json::json!({ - "sessionId": NEWER_RECORDING.to_string(), - "startTime": 200, - "duration": 10, - "files": [{ "fileName": "recording-0.webm", "startTime": 200, "duration": 10 }], - "artifacts": { "ai-analysis": [{ "fileName": "ai-analysis-0.slog" }] }, - }), - &[( - "ai-analysis-0.slog", - r#"{"timestamp":"2026-07-16T09:00:01.000Z","seq":1,"event":"session.action","description":"Opened Users","parameters":{"Created user":"Bob"}}"#, - )], + 200, + r#"{"timestamp":"2026-07-16T09:00:01.000Z","seq":1,"event":"session.action","description":"Opened Users","parameters":{"Created user":"Bob"}}"#, ) } -/// Starts Gateway on the recordings above; the returned folder and process must outlive the test requests. -async fn start_with_recordings() -> anyhow::Result<(tempfile::TempDir, DgwConfigHandle, tokio::process::Child)> { - let recordings = tempfile::tempdir()?; - write_recordings(recordings.path())?; - +/// Starts Gateway on `recordings`; the returned folder and process must outlive the test requests. +async fn start_on( + recordings: tempfile::TempDir, +) -> anyhow::Result<(tempfile::TempDir, DgwConfigHandle, tokio::process::Child)> { let config_handle = DgwConfig::builder() .disable_token_validation(true) .recording_path(recordings.path().to_owned()) @@ -110,6 +122,14 @@ async fn start_with_recordings() -> anyhow::Result<(tempfile::TempDir, DgwConfig Ok((recordings, config_handle, process)) } +/// Starts Gateway on the recordings above. +async fn start_with_recordings() -> anyhow::Result<(tempfile::TempDir, DgwConfigHandle, tokio::process::Child)> { + let recordings = tempfile::tempdir()?; + write_recordings(recordings.path())?; + + start_on(recordings).await +} + async fn search(http_port: u16, scope: &str, request: serde_json::Value) -> anyhow::Result { reqwest::Client::new() .post(format!("http://127.0.0.1:{http_port}/jet/jrec/search")) @@ -120,7 +140,7 @@ async fn search(http_port: u16, scope: &str, request: serde_json::Value) -> anyh .context("send search request") } -/// `(recordingId, fileName, entry.seq)` of each hit. +/// `(recordingId, fileName, lineNumber)` of each hit. fn hit_summaries(body: &serde_json::Value) -> anyhow::Result> { body["hits"] .as_array() @@ -130,14 +150,14 @@ fn hit_summaries(body: &serde_json::Value) -> anyhow::Result anyhow::Result<()> { +async fn searches_the_slog_artifacts_of_every_recording_newest_first() -> anyhow::Result<()> { let (_recordings, config_handle, _process) = start_with_recordings().await?; let response = search( @@ -153,7 +173,7 @@ async fn searches_the_logs_of_every_recording_newest_first() -> anyhow::Result<( hit_summaries(&body)?, [ (NEWER_RECORDING.to_string(), "ai-analysis-0.slog".to_owned(), 1), - (OLDER_RECORDING.to_string(), "recording-0.slog".to_owned(), 1), + (OLDER_RECORDING.to_string(), "ai-analysis-0.slog".to_owned(), 2), ] ); assert_eq!(body["limitReached"], false); @@ -179,7 +199,37 @@ async fn searches_each_listed_recording_once() -> anyhow::Result<()> { let body: serde_json::Value = response.json().await?; assert_eq!( hit_summaries(&body)?, - [(OLDER_RECORDING.to_string(), "recording-0.slog".to_owned(), 1)] + [(OLDER_RECORDING.to_string(), "ai-analysis-0.slog".to_owned(), 2)] + ); + + Ok(()) +} + +#[tokio::test] +async fn searches_only_the_recordings_started_in_the_time_range() -> anyhow::Result<()> { + let recordings = tempfile::tempdir()?; + for (id, start_time) in [(1, 100), (2, 200), (3, 300)] { + write_analysis_recording(recordings.path(), Uuid::from_u128(id), start_time, "Created User")?; + } + let (_recordings, config_handle, _process) = start_on(recordings).await?; + + // From the start time 200 up to, but not including, 300. + let response = search( + config_handle.http_port(), + "gateway.recordings.search", + serde_json::json!({ + "query": "created user", + "recordings": "all", + "startTime": { "from": "1970-01-01T00:03:20Z", "to": "1970-01-01T00:05:00Z" }, + }), + ) + .await?; + assert_eq!(response.status(), reqwest::StatusCode::OK); + + let body: serde_json::Value = response.json().await?; + assert_eq!( + hit_summaries(&body)?, + [(Uuid::from_u128(2).to_string(), "ai-analysis-0.slog".to_owned(), 1)] ); Ok(()) From 632ee1f4dd2ec30af051b7a1a56bd33756c787cb Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Tue, 20 Oct 2026 21:18:12 -0400 Subject: [PATCH 06/19] docs(dgw): state the recording search intent Record what the search reads, how it matches, what a hit holds, who may search, and its limits, with the reason behind each. Issue: DGW-463 Co-Authored-By: Claude Opus 5.5 --- .../src/api/jrec/search.intent.md | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 devolutions-gateway/src/api/jrec/search.intent.md diff --git a/devolutions-gateway/src/api/jrec/search.intent.md b/devolutions-gateway/src/api/jrec/search.intent.md new file mode 100644 index 0000000000..7aaa5274ed --- /dev/null +++ b/devolutions-gateway/src/api/jrec/search.intent.md @@ -0,0 +1,35 @@ +## What is searched + +Search reads only the files listed under `artifacts` in `recording.json` whose type is `slog`. +Recording files listed under `files` are not searched, a `.slog` recording included. + +Search does not interpret log content: it matches the query case-insensitively against the raw text of each line, so that any kind of artifact, present or future, can be searched. +Per-field search, such as matching an entry's `description` only, is not supported, since it would need Gateway to parse what producers write. + +## Hits + +A hit is a line that holds the query. +It carries the recording ID, the file name, the line number counting from 1, the byte offset of the line, and the line itself as a string. +Gateway does not check that a line is JSON: a `.slog` is NDJSON by format, but a line holds whatever its producer pushed, such as a half-written last line while the push is live. +A line longer than 256 KiB is matched on its first 256 KiB and returned cut, with `truncated: true`; the caller fetches the file for the whole line. + +## Access + +A valid token with the `gateway.recordings.search` scope is the only check, and DVLS decides who gets one. +Search has its own scope because `gateway.recordings.read` only lists recording IDs and must not reveal log contents. + +## Request + +`recordings` is `"all"` or a list of recording IDs, and stays open to new forms added without breaking existing callers. +`"all"` orders recordings newest first by start time, then by ID, so that the order is the same on every request; a list keeps its order and searches a repeated ID once. + +The optional `startTime` range takes RFC 3339 date-times and keeps a recording when `from <= startTime < to`, using the start time in `recording.json`, never anything inside a log. +A missing end leaves that side of the range open. + +## Limits + +One search reads about 16 MiB of logs at most and returns at most 1,000 hits, so that searching every recording takes a bounded time. + +## Later + +Regular expressions or wildcards, a total hit count, and combining words are wanted later, and the request shape must not preclude them. From 7d69f98a449e094d512100d7ebf8320715e747ba Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Tue, 20 Oct 2026 21:34:24 -0400 Subject: [PATCH 07/19] fix(dgw): list the start time range in the OpenAPI schemas The search request refers to `StartTimeRange`, but the schema was not registered, so the generated document held a dangling reference and the client generators rejected it. Issue: DGW-463 Co-Authored-By: Claude Opus 5.5 --- devolutions-gateway/src/openapi.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/devolutions-gateway/src/openapi.rs b/devolutions-gateway/src/openapi.rs index b2711e9c9c..e023be1436 100644 --- a/devolutions-gateway/src/openapi.rs +++ b/devolutions-gateway/src/openapi.rs @@ -61,6 +61,7 @@ use crate::config::dto::{DataEncoding, PubKeyFormat, Subscriber}; crate::api::jrec::DeleteManyResult, crate::api::jrec::search::RecordingSearchRequest, crate::api::jrec::search::RecordingSelection, + crate::api::jrec::search::StartTimeRange, crate::api::jrec::search::RecordingSearchResponse, crate::api::jrec::search::RecordingSearchHit, crate::token::AccessScope, From ce74751745d0b55cba994b653c32319a0408e1f4 Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Tue, 20 Oct 2026 23:07:44 -0400 Subject: [PATCH 08/19] fix(dgw): match the search query as sent and fix limit edges The query is no longer trimmed, so a hit is a line that holds the query as sent; only an empty query is rejected. A long line is now cut at a character boundary even when an invalid byte comes earlier in it. `limitReached` stays false when the read budget runs out exactly at the end of the last log, since nothing is left unread. Manifest read failures are traced rather than logged at debug level, because they repeat on every search. Tests now cover recordings that start together, a log listed in the manifest but gone, and a recording folder without a manifest. The search schemas get descriptions. Issue: DGW-463 Co-Authored-By: Claude Opus 5.5 --- devolutions-gateway/src/api/jrec/search.rs | 96 +++++++++++++++------ testsuite/tests/cli/dgw/recording_search.rs | 9 +- 2 files changed, 79 insertions(+), 26 deletions(-) diff --git a/devolutions-gateway/src/api/jrec/search.rs b/devolutions-gateway/src/api/jrec/search.rs index 6e74fcc2b3..573841cc12 100644 --- a/devolutions-gateway/src/api/jrec/search.rs +++ b/devolutions-gateway/src/api/jrec/search.rs @@ -27,11 +27,12 @@ const MAX_SCANNED_BYTES: u64 = 16 * 1024 * 1024; /// Bytes of a line that are matched and returned; the rest of a longer line is skipped. const MAX_LINE_BYTES: usize = 256 * 1024; +/// What to search for, and in which recordings. #[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] #[derive(Deserialize)] #[serde(rename_all = "camelCase")] pub(crate) struct RecordingSearchRequest { - /// Text to search for. + /// Text to search for, ignoring case; leading and trailing spaces are part of it. query: String, /// The recordings to search. recordings: RecordingSelection, @@ -110,6 +111,7 @@ impl StartTimeRange { } } +/// The lines that hold the search text. #[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] #[derive(Serialize)] #[serde(rename_all = "camelCase")] @@ -120,10 +122,12 @@ pub(crate) struct RecordingSearchResponse { limit_reached: bool, } +/// A line that holds the search text. #[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] #[derive(Serialize)] #[serde(rename_all = "camelCase")] pub(crate) struct RecordingSearchHit { + /// The recording whose artifact holds the line. recording_id: Uuid, /// Name of the `.slog` artifact holding the line, as listed in `recording.json`. file_name: String, @@ -218,8 +222,7 @@ struct SearchText(String); impl SearchText { fn new(query: &str) -> Option { - let text = query.trim().to_lowercase(); - (!text.is_empty()).then_some(Self(text)) + (!query.is_empty()).then(|| Self(query.to_lowercase())) } fn is_in(&self, line: &str) -> bool { @@ -260,7 +263,7 @@ impl Search { loop { if MAX_SCANNED_BYTES <= self.scanned_bytes { - self.limit_reached = true; + self.limit_reached = !reader.fill_buf().await?.is_empty(); return Ok(()); } @@ -324,14 +327,14 @@ async fn read_recording_logs(dir: Utf8PathBuf, recording_id: Uuid) -> Option, line: &mut Vec) -> io::Resu let truncated = MAX_LINE_BYTES < line.len(); if truncated { line.truncate(MAX_LINE_BYTES); - line.truncate(len_without_cut_character(line)); + + let cut_character = line.utf8_chunks().last().map_or(0, |chunk| { + let invalid = chunk.invalid(); + let is_incomplete = std::str::from_utf8(invalid).is_err_and(|error| error.error_len().is_none()); + if is_incomplete { invalid.len() } else { 0 } + }); + line.truncate(line.len() - cut_character); } Ok(Some(LineRead { @@ -458,14 +467,6 @@ async fn read_line(reader: &mut BufReader, line: &mut Vec) -> io::Resu })) } -/// Length of `line` without a UTF-8 character that is cut at its end. -fn len_without_cut_character(line: &[u8]) -> usize { - match std::str::from_utf8(line) { - Err(error) if error.error_len().is_none() => error.valid_up_to(), - _ => line.len(), - } -} - struct Skipped { /// Bytes consumed, line break included. bytes: u64, @@ -478,12 +479,16 @@ struct Skipped { async fn skip_rest_of_line(reader: &mut BufReader, budget: u64) -> io::Result { let mut bytes = 0; - while bytes < budget { + loop { let available = reader.fill_buf().await?; if available.is_empty() { return Ok(Skipped { bytes, finished: true }); } + if budget <= bytes { + return Ok(Skipped { bytes, finished: false }); + } + let room = usize::try_from(budget - bytes).unwrap_or(usize::MAX); let window = &available[..available.len().min(room)]; let line_break = window.iter().position(|&byte| byte == b'\n'); @@ -496,8 +501,6 @@ async fn skip_rest_of_line(reader: &mut BufReader, budget: u64) -> io::Res return Ok(Skipped { bytes, finished: true }); } } - - Ok(Skipped { bytes, finished: false }) } #[cfg(test)] @@ -541,6 +544,13 @@ mod tests { assert_eq!(hit_lines(&search), [(1, line)]); } + #[tokio::test] + async fn keeps_the_spaces_of_the_query() { + let search = search_log("user ", br#"{"name":"username"}"#).await; + + assert!(search.hits.is_empty()); + } + #[tokio::test] async fn returns_a_line_that_is_not_json() { let search = search_log("sess", br#"{"seq":12,"event":"sess"#).await; @@ -592,15 +602,19 @@ mod tests { assert!(search.hits.is_empty()); } + #[rstest] + #[case::valid_text(b"hit".as_slice())] + #[case::after_an_invalid_byte(b"\xFFhi".as_slice())] #[tokio::test] - async fn cuts_a_long_line_on_a_character_boundary() { - // "€" is 3 bytes, so the limit falls 1 byte into a character. - let content = format!("hit{}", "€".repeat(MAX_LINE_BYTES / 3)); + async fn cuts_a_long_line_on_a_character_boundary(#[case] start: &[u8]) { + // Both starts are 3 bytes and "€" is 3 bytes, so the limit falls 1 byte into a character. + let mut content = start.to_vec(); + content.extend_from_slice("€".repeat(MAX_LINE_BYTES / 3).as_bytes()); - let search = search_log("hit", content.as_bytes()).await; + let search = search_log("hi", &content).await; - assert_eq!(search.hits[0].line.len(), MAX_LINE_BYTES - 1); - assert!(!search.hits[0].line.contains('\u{FFFD}')); + assert!(search.hits[0].truncated); + assert!(search.hits[0].line.ends_with('€')); } #[tokio::test] @@ -627,6 +641,38 @@ mod tests { assert!(search.scanned_bytes <= MAX_SCANNED_BYTES); } + #[rstest] + #[case::with_a_final_line_break(b"\n".as_slice())] + #[case::without_one(b"".as_slice())] + #[tokio::test] + async fn reaches_no_limit_when_the_read_budget_ends_with_the_log(#[case] end: &[u8]) { + let budget = usize::try_from(MAX_SCANNED_BYTES).expect("budget fits in usize"); + let mut content = b"hit\n".to_vec(); + content.resize(budget - end.len(), b'x'); + content.extend_from_slice(end); + + let search = search_log("hit", &content).await; + + assert_eq!(hit_lines(&search), [(1, "hit")]); + assert!(!search.limit_reached); + } + + #[tokio::test] + async fn lists_recordings_that_start_together_highest_id_first() { + let root = tempfile::tempdir().expect("temp dir"); + for id in 1..=3 { + let dir = root.path().join(Uuid::from_u128(id).to_string()); + std::fs::create_dir(&dir).expect("create recording folder"); + std::fs::write(dir.join("recording.json"), r#"{"startTime":100}"#).expect("write manifest"); + } + let root = Utf8Path::from_path(root.path()).expect("UTF-8 path"); + + let recordings = all_recording_logs_newest_first(root).await.expect("list recordings"); + + let ids: Vec<_> = recordings.iter().map(|recording| recording.recording_id).collect(); + assert_eq!(ids, [3, 2, 1].map(Uuid::from_u128)); + } + const SECOND: i128 = 1_000_000_000; fn at(unix_nanos: i128) -> Option { diff --git a/testsuite/tests/cli/dgw/recording_search.rs b/testsuite/tests/cli/dgw/recording_search.rs index 12106aadd1..13f8e4f5f1 100644 --- a/testsuite/tests/cli/dgw/recording_search.rs +++ b/testsuite/tests/cli/dgw/recording_search.rs @@ -61,11 +61,14 @@ fn write_analysis_recording(root: &Path, id: Uuid, start_time: i64, log: &str) - /// Two recordings whose AI analysis logs hold the searched text. /// The older one also holds it in a recorded `.slog`, in its video, and in a log outside the recording folder that /// its manifest points to, to show that only the `.slog` artifacts of a recording are searched. +/// Its manifest also lists a log that is gone, and a third recording folder has no manifest, to show that neither +/// fails the search. fn write_recordings(root: &Path) -> anyhow::Result<()> { std::fs::write( root.join("outside.slog"), r#"{"timestamp":"2026-07-15T16:15:35.140Z","seq":0,"event":"session.action","description":"Created User"}"#, )?; + std::fs::create_dir(root.join(Uuid::from_u128(3).to_string()))?; write_recording( root, @@ -79,7 +82,11 @@ fn write_recordings(root: &Path) -> anyhow::Result<()> { { "fileName": "recording-1.webm", "startTime": 100, "duration": 10 }, ], "artifacts": { - "ai-analysis": [{ "fileName": "ai-analysis-0.slog" }, { "fileName": "../outside.slog" }], + "ai-analysis": [ + { "fileName": "ai-analysis-1.slog" }, + { "fileName": "ai-analysis-0.slog" }, + { "fileName": "../outside.slog" }, + ], }, }), &[ From 27db4ed9cff171f90ab0d2e87494aca1c18d7d4b Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Tue, 20 Oct 2026 23:32:36 -0400 Subject: [PATCH 09/19] fix(dgw): fold the case of a sigma and search each log once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `str::to_lowercase` turns a capital sigma that ends a word into a final sigma, so the query "ΧΡΗΣ" missed the line "ΧΡΗΣΤΗΣ". Each character is now lowercased on its own and a final sigma is read as a sigma, so case never decides whether a line holds the query. A manifest that lists the same log twice no longer gets it searched twice. A missing log is traced rather than logged at debug level, since it repeats on every search while the manifest lists it. Issue: DGW-463 Co-Authored-By: Claude Opus 5.5 --- devolutions-gateway/src/api/jrec/search.rs | 47 ++++++++++++++++++++-- 1 file changed, 43 insertions(+), 4 deletions(-) diff --git a/devolutions-gateway/src/api/jrec/search.rs b/devolutions-gateway/src/api/jrec/search.rs index 573841cc12..ccadc24ddd 100644 --- a/devolutions-gateway/src/api/jrec/search.rs +++ b/devolutions-gateway/src/api/jrec/search.rs @@ -203,7 +203,7 @@ pub(crate) async fn search_recordings( Ok(()) => {} // A recording can be deleted while it is searched. Err(error) if error.kind() == io::ErrorKind::NotFound => { - debug!(%path, "Recording log listed in the manifest is missing"); + trace!(%path, "Recording log listed in the manifest is missing"); } Err(error) => warn!(%path, %error, "Failed to search a recording log"), } @@ -217,19 +217,30 @@ pub(crate) async fn search_recordings( Ok(Json(search.into_response())) } -/// Search text in lowercase, the case lines are compared in. +/// Search text with its case folded, the form lines are compared in. struct SearchText(String); impl SearchText { fn new(query: &str) -> Option { - (!query.is_empty()).then(|| Self(query.to_lowercase())) + (!query.is_empty()).then(|| Self(fold_case(query))) } fn is_in(&self, line: &str) -> bool { - line.to_lowercase().contains(&self.0) + fold_case(line).contains(&self.0) } } +/// Lowercases each character on its own, with the final sigma read as a sigma. +/// +/// `str::to_lowercase` turns a capital sigma that ends a word into a final sigma, +/// so a query ending in "Σ" would not match the same letters inside a longer word. +fn fold_case(text: &str) -> String { + text.chars() + .flat_map(char::to_lowercase) + .map(|character| if character == 'ς' { 'σ' } else { character }) + .collect() +} + struct Search { text: SearchText, hits: Vec, @@ -337,6 +348,8 @@ async fn read_recording_logs(dir: Utf8PathBuf, recording_id: Uuid) -> Option Option Date: Tue, 20 Oct 2026 23:53:28 -0400 Subject: [PATCH 10/19] fix(dgw): list each recording once and lowercase lines quickly Searching all recordings now skips folders not named by the hyphenated recording ID, the only spelling Gateway writes, so a copy under another spelling of the same ID is not searched as the same recording twice. Lines are lowercased with `str::to_lowercase` again, keeping its fast path for ASCII, and every final sigma then becomes a middle sigma. The schema now states that `query` is not empty and that `lineNumber` starts at 1. Issue: DGW-463 Co-Authored-By: Claude Opus 5.5 --- devolutions-gateway/src/api/jrec/search.rs | 55 ++++++++++++++++------ 1 file changed, 40 insertions(+), 15 deletions(-) diff --git a/devolutions-gateway/src/api/jrec/search.rs b/devolutions-gateway/src/api/jrec/search.rs index ccadc24ddd..3e80711cf6 100644 --- a/devolutions-gateway/src/api/jrec/search.rs +++ b/devolutions-gateway/src/api/jrec/search.rs @@ -33,6 +33,7 @@ const MAX_LINE_BYTES: usize = 256 * 1024; #[serde(rename_all = "camelCase")] pub(crate) struct RecordingSearchRequest { /// Text to search for, ignoring case; leading and trailing spaces are part of it. + #[cfg_attr(feature = "openapi", schema(min_length = 1))] query: String, /// The recordings to search. recordings: RecordingSelection, @@ -132,6 +133,7 @@ pub(crate) struct RecordingSearchHit { /// Name of the `.slog` artifact holding the line, as listed in `recording.json`. file_name: String, /// Number of the line in the file, counting from 1. + #[cfg_attr(feature = "openapi", schema(minimum = 1))] line_number: u64, /// Byte offset of the line in the file. offset: u64, @@ -217,28 +219,30 @@ pub(crate) async fn search_recordings( Ok(Json(search.into_response())) } -/// Search text with its case folded, the form lines are compared in. +/// Search text in the lowercase form lines are compared in. struct SearchText(String); impl SearchText { fn new(query: &str) -> Option { - (!query.is_empty()).then(|| Self(fold_case(query))) + (!query.is_empty()).then(|| Self(lowercase_for_matching(query))) } fn is_in(&self, line: &str) -> bool { - fold_case(line).contains(&self.0) + lowercase_for_matching(line).contains(&self.0) } } -/// Lowercases each character on its own, with the final sigma read as a sigma. +/// Lowercases `text`, with every sigma in its middle form. /// /// `str::to_lowercase` turns a capital sigma that ends a word into a final sigma, /// so a query ending in "Σ" would not match the same letters inside a longer word. -fn fold_case(text: &str) -> String { - text.chars() - .flat_map(char::to_lowercase) - .map(|character| if character == 'ς' { 'σ' } else { character }) - .collect() +fn lowercase_for_matching(text: &str) -> String { + let lowercase = text.to_lowercase(); + if lowercase.contains('ς') { + lowercase.replace('ς', "σ") + } else { + lowercase + } } struct Search { @@ -384,7 +388,13 @@ async fn all_recording_logs_newest_first(recording_root: &Utf8Path) -> anyhow::R let mut recordings = Vec::new(); while let Some(entry) = read_dir.next_entry().await? { - let Some(recording_id) = entry.file_name().to_str().and_then(|name| Uuid::parse_str(name).ok()) else { + // Gateway names a recording folder by the hyphenated ID, and a copy under another spelling of the same ID + // would be searched as the same recording twice. + let Some(recording_id) = entry.file_name().to_str().and_then(|name| { + Uuid::parse_str(name) + .ok() + .filter(|recording_id| recording_id.to_string() == name) + }) else { continue; }; @@ -696,11 +706,11 @@ mod tests { assert_eq!(recording.file_names, ["ai-analysis-0.slog"]); } - #[tokio::test] - async fn lists_recordings_that_start_together_highest_id_first() { + /// Lists the recordings of a root holding a folder per name, each with a manifest that starts at the same time. + async fn list_recording_folders(names: impl IntoIterator) -> Vec { let root = tempfile::tempdir().expect("temp dir"); - for id in 1..=3 { - let dir = root.path().join(Uuid::from_u128(id).to_string()); + for name in names { + let dir = root.path().join(name); std::fs::create_dir(&dir).expect("create recording folder"); std::fs::write(dir.join("recording.json"), r#"{"startTime":100}"#).expect("write manifest"); } @@ -708,10 +718,25 @@ mod tests { let recordings = all_recording_logs_newest_first(root).await.expect("list recordings"); - let ids: Vec<_> = recordings.iter().map(|recording| recording.recording_id).collect(); + recordings.iter().map(|recording| recording.recording_id).collect() + } + + #[tokio::test] + async fn lists_recordings_that_start_together_highest_id_first() { + let ids = list_recording_folders((1..=3).map(|id| Uuid::from_u128(id).to_string())).await; + assert_eq!(ids, [3, 2, 1].map(Uuid::from_u128)); } + #[tokio::test] + async fn lists_only_folders_named_by_the_hyphenated_id() { + let id = Uuid::from_u128(1); + + let ids = list_recording_folders([id.to_string(), id.simple().to_string(), id.braced().to_string()]).await; + + assert_eq!(ids, [id]); + } + const SECOND: i128 = 1_000_000_000; fn at(unix_nanos: i128) -> Option { From 56c3533eb42223c49b7c157cf7082879c25542e1 Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Wed, 21 Oct 2026 09:14:25 -0400 Subject: [PATCH 11/19] feat(dgw): reject unknown fields in a recording search A search request, and its `startTime` range, now fail on a field they do not know. An option added later, such as a regular expression mode, then fails on a Gateway that predates it instead of being dropped while a plain substring search runs. Issue: DGW-463 Co-Authored-By: Claude Opus 5.5 --- devolutions-gateway/src/api/jrec/search.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/devolutions-gateway/src/api/jrec/search.rs b/devolutions-gateway/src/api/jrec/search.rs index 3e80711cf6..cc5be5b45b 100644 --- a/devolutions-gateway/src/api/jrec/search.rs +++ b/devolutions-gateway/src/api/jrec/search.rs @@ -28,9 +28,11 @@ const MAX_SCANNED_BYTES: u64 = 16 * 1024 * 1024; const MAX_LINE_BYTES: usize = 256 * 1024; /// What to search for, and in which recordings. +/// +/// Unknown fields are rejected, so that an option added later is never silently ignored by a Gateway that predates it. #[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] #[derive(Deserialize)] -#[serde(rename_all = "camelCase")] +#[serde(rename_all = "camelCase", deny_unknown_fields)] pub(crate) struct RecordingSearchRequest { /// Text to search for, ignoring case; leading and trailing spaces are part of it. #[cfg_attr(feature = "openapi", schema(min_length = 1))] @@ -93,6 +95,7 @@ impl<'__s> utoipa::ToSchema<'__s> for RecordingSelection { /// A missing end leaves that side of the range open. #[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] #[derive(Deserialize)] +#[serde(deny_unknown_fields)] pub(crate) struct StartTimeRange { /// Earliest start time searched. #[serde(default, with = "time::serde::rfc3339::option")] @@ -804,4 +807,11 @@ mod tests { fn rejects_other_recording_selections(#[case] recordings: &str) { assert!(parse_request(&format!(r#""recordings":{recordings}"#)).is_err()); } + + #[rstest] + #[case::in_the_request(r#""recordings":"all","mode":"regex""#)] + #[case::in_the_start_time(r#""recordings":"all","startTime":{"until":"2026-10-10T00:00:00Z"}"#)] + fn rejects_unknown_fields(#[case] fields: &str) { + assert!(parse_request(fields).is_err()); + } } From 8acff8fff3d3070ddeca325c2ef38576d9d76266 Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Wed, 21 Oct 2026 12:54:25 -0400 Subject: [PATCH 12/19] feat(dgw): search the string values of JSON log lines Matching the raw text of a line made a field name such as "description" match every line, and missed text a JSON writer escapes: "DOMAIN\user" is stored as "DOMAIN\user". A line holding a JSON object is now matched against its string values, at any depth and once unescaped. Any other line, such as a half-written last line or one cut at 256 KiB, is still matched as raw text, so non-JSON artifacts stay searchable. Issue: DGW-463 Co-Authored-By: Claude Opus 5.5 --- devolutions-gateway/src/api/jrec/search.rs | 88 ++++++++++++++++++--- testsuite/tests/cli/dgw/recording_search.rs | 2 +- 2 files changed, 78 insertions(+), 12 deletions(-) diff --git a/devolutions-gateway/src/api/jrec/search.rs b/devolutions-gateway/src/api/jrec/search.rs index cc5be5b45b..83830acb24 100644 --- a/devolutions-gateway/src/api/jrec/search.rs +++ b/devolutions-gateway/src/api/jrec/search.rs @@ -4,6 +4,7 @@ use std::io; use axum::Json; use axum::extract::State; use camino::{Utf8Path, Utf8PathBuf}; +use serde_json::Value; use time::OffsetDateTime; use tokio::fs::File; use tokio::io::{AsyncBufReadExt as _, BufReader}; @@ -148,8 +149,10 @@ pub(crate) struct RecordingSearchHit { /// Searches the `.slog` artifacts, such as AI analysis logs, of recordings stored on this instance /// -/// The query is a case-insensitive substring, matched against the raw text of each line. -/// Gateway does not interpret the logs, so any part of a line can match, field names included. +/// The query is a case-insensitive substring. +/// In a line holding a JSON object, it is matched against the string values of the object at any depth, as they read +/// once unescaped, so field names, numbers and booleans never match. +/// Any other line, such as one still being written, is matched as raw text. /// Only the files listed under `artifacts` in `recording.json` with the `slog` type are searched, /// and a line longer than 256 KiB is matched on its first 256 KiB only. /// Recordings without a readable `recording.json` and IDs with no recording on this instance give no hits. @@ -222,7 +225,7 @@ pub(crate) async fn search_recordings( Ok(Json(search.into_response())) } -/// Search text in the lowercase form lines are compared in. +/// Search text in the lowercase form text is compared in. struct SearchText(String); impl SearchText { @@ -230,8 +233,30 @@ impl SearchText { (!query.is_empty()).then(|| Self(lowercase_for_matching(query))) } - fn is_in(&self, line: &str) -> bool { - lowercase_for_matching(line).contains(&self.0) + /// Whether `line` holds the search text. + /// + /// In a JSON object, only the string values count, once unescaped, so that a field name never matches and + /// `DOMAIN\user` matches the `"DOMAIN\\user"` a JSON writer stores. + /// Any other line is compared as raw text, so that a line of plain text that happens to parse as JSON, such as + /// `2026`, still matches. + fn is_in(&self, line: &[u8]) -> bool { + match serde_json::from_slice(line) { + Ok(object @ Value::Object(_)) => self.is_in_strings_of(&object), + _ => self.is_in_text(&String::from_utf8_lossy(line)), + } + } + + fn is_in_strings_of(&self, value: &Value) -> bool { + match value { + Value::String(text) => self.is_in_text(text), + Value::Array(values) => values.iter().any(|value| self.is_in_strings_of(value)), + Value::Object(fields) => fields.values().any(|value| self.is_in_strings_of(value)), + Value::Null | Value::Bool(_) | Value::Number(_) => false, + } + } + + fn is_in_text(&self, text: &str) -> bool { + lowercase_for_matching(text).contains(&self.0) } } @@ -293,8 +318,7 @@ impl Search { let line_offset = offset; offset += read.bytes; - let text = String::from_utf8_lossy(&line); - if self.text.is_in(&text) { + if self.text.is_in(&line) { if MAX_HITS <= self.hits.len() { self.limit_reached = true; return Ok(()); @@ -305,7 +329,7 @@ impl Search { file_name: file_name.to_owned(), line_number, offset: line_offset, - line: text.into_owned(), + line: String::from_utf8_lossy(&line).into_owned(), truncated: read.truncated, }); } @@ -560,17 +584,59 @@ mod tests { #[rstest] #[case::value_in_another_case("created USER")] - #[case::field_name("description")] #[case::event_name("session.action")] + #[case::nested_value("ottawa/hull")] #[tokio::test] - async fn matches_any_part_of_the_raw_line(#[case] query: &str) { - let line = r#"{"seq":1,"event":"session.action","description":"Created User"}"#; + async fn matches_the_string_values_of_a_json_line(#[case] query: &str) { + let line = r#"{"seq":1,"event":"session.action","description":"Created User","parameters":{"New name":"Help Desk Ottawa/Hull"}}"#; + + let search = search_log(query, line.as_bytes()).await; + + assert_eq!(hit_lines(&search), [(1, line)]); + } + + #[rstest] + #[case::field_name("description")] + #[case::nested_field_name("new name")] + #[case::number("12")] + #[case::boolean("true")] + #[tokio::test] + async fn does_not_match_the_field_names_or_other_values_of_a_json_line(#[case] query: &str) { + let line = r#"{"seq":12,"done":true,"description":"Created User","parameters":{"New name":"Bob"}}"#; + + let search = search_log(query, line.as_bytes()).await; + assert!(search.hits.is_empty()); + } + + #[rstest] + #[case::backslash(r"IT-HELP\Administrator", r#"{"description":"Reset IT-HELP\\Administrator"}"#)] + #[case::quote(r#""net user""#, r#"{"description":"Ran \"net user\""}"#)] + #[case::unicode_escape("créé", r#"{"description":"Utilisateur créé"}"#)] + #[tokio::test] + async fn matches_the_text_of_a_json_string_once_unescaped(#[case] query: &str, #[case] line: &str) { let search = search_log(query, line.as_bytes()).await; assert_eq!(hit_lines(&search), [(1, line)]); } + #[tokio::test] + async fn matches_a_line_of_plain_text_that_parses_as_a_json_number() { + let search = search_log("2026", b"2026").await; + + assert_eq!(hit_lines(&search), [(1, "2026")]); + } + + #[tokio::test] + async fn matches_a_json_object_cut_at_the_line_limit_as_raw_text() { + let line = format!(r#"{{"description":"{}"}}"#, "x".repeat(MAX_LINE_BYTES)); + + let search = search_log("description", line.as_bytes()).await; + + assert_eq!(search.hits.len(), 1); + assert!(search.hits[0].truncated); + } + #[rstest] #[case::query_ending_in_a_capital_sigma("ΧΡΗΣ", "ΧΡΗΣΤΗΣ")] #[case::line_ending_in_a_capital_sigma("σ", "ΟΣ")] diff --git a/testsuite/tests/cli/dgw/recording_search.rs b/testsuite/tests/cli/dgw/recording_search.rs index 13f8e4f5f1..150b54c60d 100644 --- a/testsuite/tests/cli/dgw/recording_search.rs +++ b/testsuite/tests/cli/dgw/recording_search.rs @@ -111,7 +111,7 @@ fn write_recordings(root: &Path) -> anyhow::Result<()> { root, NEWER_RECORDING, 200, - r#"{"timestamp":"2026-07-16T09:00:01.000Z","seq":1,"event":"session.action","description":"Opened Users","parameters":{"Created user":"Bob"}}"#, + r#"{"timestamp":"2026-07-16T09:00:01.000Z","seq":1,"event":"session.action","description":"Opened Users","parameters":{"Action":"Created user"}}"#, ) } From 2b9313932dc1390844eded338c92a0b20e19ed5e Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Wed, 21 Oct 2026 13:53:19 -0400 Subject: [PATCH 13/19] docs(dgw): state the value matching rule in the search intent Benoit's review moved search to matching the string values of JSON lines, which the intent still described as raw-text matching. The intent now gives the rule and its reasons, says why line numbers count from 1, names the provisioner rather than DVLS as the party that decides who gets a search token, and marks choosing artifact kinds or other sources as wanted later. Issue: DGW-463 Co-Authored-By: Claude Opus 5.5 --- .../src/api/jrec/search.intent.md | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/devolutions-gateway/src/api/jrec/search.intent.md b/devolutions-gateway/src/api/jrec/search.intent.md index 7aaa5274ed..c5b7bace1d 100644 --- a/devolutions-gateway/src/api/jrec/search.intent.md +++ b/devolutions-gateway/src/api/jrec/search.intent.md @@ -3,19 +3,25 @@ Search reads only the files listed under `artifacts` in `recording.json` whose type is `slog`. Recording files listed under `files` are not searched, a `.slog` recording included. -Search does not interpret log content: it matches the query case-insensitively against the raw text of each line, so that any kind of artifact, present or future, can be searched. -Per-field search, such as matching an entry's `description` only, is not supported, since it would need Gateway to parse what producers write. +A line holding a JSON object is matched case-insensitively against the object's string values, at any depth and once unescaped; field names, numbers, booleans and null never match. +Any other line is matched case-insensitively against its raw text, so that any kind of artifact, present or future, can be searched. +A line cut at 256 KiB follows the same rule: what was read is matched by its string values when it parses as a JSON object, and as raw text otherwise. + +Values are matched rather than raw text because users search the text they see: raw matching made a field name such as `description` match every line, and missed text that JSON escapes, such as `DOMAIN\user` stored as `DOMAIN\\user`. +The rule never depends on field names, so a producer can change the fields it writes without any change to search. +Per-field search, such as matching an entry's `description` only, is not supported: the fields a producer writes are unknown to Gateway, which only cares about values. ## Hits A hit is a line that holds the query. -It carries the recording ID, the file name, the line number counting from 1, the byte offset of the line, and the line itself as a string. -Gateway does not check that a line is JSON: a `.slog` is NDJSON by format, but a line holds whatever its producer pushed, such as a half-written last line while the push is live. +A hit carries the recording ID, the file name, the line number counting from 1, the byte offset of the line, and the raw line itself as a string. +Line numbers count from 1 like the slog package in `webapp/packages/session-recording-log`, so that a hit and that package's own search give a line the same number. +Gateway does not require a line to be JSON: a `.slog` is NDJSON by format, but a line holds whatever its producer pushed, such as a half-written last line while the push is live. A line longer than 256 KiB is matched on its first 256 KiB and returned cut, with `truncated: true`; the caller fetches the file for the whole line. ## Access -A valid token with the `gateway.recordings.search` scope is the only check, and DVLS decides who gets one. +A valid token with the `gateway.recordings.search` scope is the only check, and the provisioner that issues the token decides who gets one. Search has its own scope because `gateway.recordings.read` only lists recording IDs and must not reveal log contents. ## Request @@ -33,3 +39,4 @@ One search reads about 16 MiB of logs at most and returns at most 1,000 hits, so ## Later Regular expressions or wildcards, a total hit count, and combining words are wanted later, and the request shape must not preclude them. +Choosing which artifact kinds to search, or searching other sources such as the `.slog` recordings under `files`, is wanted later; the request shape must not preclude it either. From 862d0683deea45bf3b57459bbff05b2e38f94ccb Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Wed, 21 Oct 2026 13:56:20 -0400 Subject: [PATCH 14/19] test(dgw): check unescaping, arrays and null in recording search MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The unicode escape case held a literal "é", so it passed with raw matching too; it now holds the JSON escape it is named after. New cases cover a string inside an array, which must match, and a null value, which must not. The docs also name null among the values that never match. Issue: DGW-463 Co-Authored-By: Claude Opus 5.5 --- devolutions-gateway/src/api/jrec/search.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/devolutions-gateway/src/api/jrec/search.rs b/devolutions-gateway/src/api/jrec/search.rs index 83830acb24..147db6b6c4 100644 --- a/devolutions-gateway/src/api/jrec/search.rs +++ b/devolutions-gateway/src/api/jrec/search.rs @@ -151,7 +151,7 @@ pub(crate) struct RecordingSearchHit { /// /// The query is a case-insensitive substring. /// In a line holding a JSON object, it is matched against the string values of the object at any depth, as they read -/// once unescaped, so field names, numbers and booleans never match. +/// once unescaped, so field names, numbers, booleans and null never match. /// Any other line, such as one still being written, is matched as raw text. /// Only the files listed under `artifacts` in `recording.json` with the `slog` type are searched, /// and a line longer than 256 KiB is matched on its first 256 KiB only. @@ -225,7 +225,7 @@ pub(crate) async fn search_recordings( Ok(Json(search.into_response())) } -/// Search text in the lowercase form text is compared in. +/// The query, lowercased the same way as the text it is compared with. struct SearchText(String); impl SearchText { @@ -586,9 +586,10 @@ mod tests { #[case::value_in_another_case("created USER")] #[case::event_name("session.action")] #[case::nested_value("ottawa/hull")] + #[case::value_in_an_array("audit trail")] #[tokio::test] async fn matches_the_string_values_of_a_json_line(#[case] query: &str) { - let line = r#"{"seq":1,"event":"session.action","description":"Created User","parameters":{"New name":"Help Desk Ottawa/Hull"}}"#; + let line = r#"{"seq":1,"event":"session.action","description":"Created User","parameters":{"New name":"Help Desk Ottawa/Hull"},"tags":["Audit trail"]}"#; let search = search_log(query, line.as_bytes()).await; @@ -600,9 +601,10 @@ mod tests { #[case::nested_field_name("new name")] #[case::number("12")] #[case::boolean("true")] + #[case::null("null")] #[tokio::test] async fn does_not_match_the_field_names_or_other_values_of_a_json_line(#[case] query: &str) { - let line = r#"{"seq":12,"done":true,"description":"Created User","parameters":{"New name":"Bob"}}"#; + let line = r#"{"seq":12,"done":true,"actor":null,"description":"Created User","parameters":{"New name":"Bob"}}"#; let search = search_log(query, line.as_bytes()).await; @@ -612,7 +614,7 @@ mod tests { #[rstest] #[case::backslash(r"IT-HELP\Administrator", r#"{"description":"Reset IT-HELP\\Administrator"}"#)] #[case::quote(r#""net user""#, r#"{"description":"Ran \"net user\""}"#)] - #[case::unicode_escape("créé", r#"{"description":"Utilisateur créé"}"#)] + #[case::unicode_escape("créé", r#"{"description":"Utilisateur cr\u00e9\u00e9"}"#)] #[tokio::test] async fn matches_the_text_of_a_json_string_once_unescaped(#[case] query: &str, #[case] line: &str) { let search = search_log(query, line.as_bytes()).await; From 4918b94bc2284300c7c11ad45cfb034ff2f29f49 Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Wed, 21 Oct 2026 14:12:32 -0400 Subject: [PATCH 15/19] test(dgw): pin that a cut line still parses in recording search No test told apart the rule that a cut line is matched by its string values when what was read parses from the rejected rule that never parses a cut line. A cut object followed by spaces now must not match its field name. The docs say a hit matches the search text rather than holds it, since a field name holding the query is not a hit. Issue: DGW-463 Co-Authored-By: Claude Opus 5.5 --- devolutions-gateway/src/api/jrec/search.rs | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/devolutions-gateway/src/api/jrec/search.rs b/devolutions-gateway/src/api/jrec/search.rs index 147db6b6c4..a3965fcb50 100644 --- a/devolutions-gateway/src/api/jrec/search.rs +++ b/devolutions-gateway/src/api/jrec/search.rs @@ -116,7 +116,7 @@ impl StartTimeRange { } } -/// The lines that hold the search text. +/// The lines that match the search text. #[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] #[derive(Serialize)] #[serde(rename_all = "camelCase")] @@ -127,7 +127,7 @@ pub(crate) struct RecordingSearchResponse { limit_reached: bool, } -/// A line that holds the search text. +/// A line that matches the search text. #[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] #[derive(Serialize)] #[serde(rename_all = "camelCase")] @@ -233,7 +233,7 @@ impl SearchText { (!query.is_empty()).then(|| Self(lowercase_for_matching(query))) } - /// Whether `line` holds the search text. + /// Whether `line` matches the search text. /// /// In a JSON object, only the string values count, once unescaped, so that a field name never matches and /// `DOMAIN\user` matches the `"DOMAIN\\user"` a JSON writer stores. @@ -297,7 +297,7 @@ impl Search { } } - /// Adds the lines of one log that hold the search text. + /// Adds the lines of one log that match the search text. async fn search_file(&mut self, recording_id: Uuid, file_name: &str, path: &Utf8Path) -> io::Result<()> { let mut reader = BufReader::new(File::open(path).await?); let mut line = Vec::new(); @@ -630,7 +630,7 @@ mod tests { } #[tokio::test] - async fn matches_a_json_object_cut_at_the_line_limit_as_raw_text() { + async fn matches_a_cut_line_that_no_longer_parses_as_raw_text() { let line = format!(r#"{{"description":"{}"}}"#, "x".repeat(MAX_LINE_BYTES)); let search = search_log("description", line.as_bytes()).await; @@ -639,6 +639,16 @@ mod tests { assert!(search.hits[0].truncated); } + #[tokio::test] + async fn matches_a_cut_line_by_its_string_values_when_what_was_read_parses() { + let mut line = br#"{"description":"x"}"#.to_vec(); + line.resize(MAX_LINE_BYTES + 1, b' '); + + let search = search_log("description", &line).await; + + assert!(search.hits.is_empty()); + } + #[rstest] #[case::query_ending_in_a_capital_sigma("ΧΡΗΣ", "ΧΡΗΣΤΗΣ")] #[case::line_ending_in_a_capital_sigma("σ", "ΟΣ")] From 8da08dfe1309facfd5cdcf9498c6ebb77cf9c3c3 Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Wed, 21 Oct 2026 14:21:32 -0400 Subject: [PATCH 16/19] test(dgw): match an escaped value in a cut line in recording search The cut line test only asserted that nothing matched, so it also passed if the line was never matched. It now searches an escaped value that only a parsed line can match, and checks the hit is cut. The endpoint docs say that a line over 256 KiB follows the same rule on its first 256 KiB, so a cut JSON object is usually raw text. Issue: DGW-463 Co-Authored-By: Claude Opus 5.5 --- devolutions-gateway/src/api/jrec/search.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/devolutions-gateway/src/api/jrec/search.rs b/devolutions-gateway/src/api/jrec/search.rs index a3965fcb50..64c591ef1e 100644 --- a/devolutions-gateway/src/api/jrec/search.rs +++ b/devolutions-gateway/src/api/jrec/search.rs @@ -153,8 +153,9 @@ pub(crate) struct RecordingSearchHit { /// In a line holding a JSON object, it is matched against the string values of the object at any depth, as they read /// once unescaped, so field names, numbers, booleans and null never match. /// Any other line, such as one still being written, is matched as raw text. -/// Only the files listed under `artifacts` in `recording.json` with the `slog` type are searched, -/// and a line longer than 256 KiB is matched on its first 256 KiB only. +/// A line longer than 256 KiB is matched on its first 256 KiB only, by the same rule, so a cut JSON object is +/// usually matched as raw text. +/// Only the files listed under `artifacts` in `recording.json` with the `slog` type are searched. /// Recordings without a readable `recording.json` and IDs with no recording on this instance give no hits. #[cfg_attr(feature = "openapi", utoipa::path( post, @@ -641,12 +642,13 @@ mod tests { #[tokio::test] async fn matches_a_cut_line_by_its_string_values_when_what_was_read_parses() { - let mut line = br#"{"description":"x"}"#.to_vec(); + let mut line = br#"{"description":"DOMAIN\\user"}"#.to_vec(); line.resize(MAX_LINE_BYTES + 1, b' '); - let search = search_log("description", &line).await; + let search = search_log(r"DOMAIN\user", &line).await; - assert!(search.hits.is_empty()); + assert_eq!(search.hits.len(), 1); + assert!(search.hits[0].truncated); } #[rstest] From 3ee0497404afd38a333d2a2ee62fc1f0ea6afab4 Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Wed, 21 Oct 2026 14:27:03 -0400 Subject: [PATCH 17/19] docs(dgw): wrap the cut line sentence at the line limit Issue: DGW-463 Co-Authored-By: Claude Opus 5.5 --- devolutions-gateway/src/api/jrec/search.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/devolutions-gateway/src/api/jrec/search.rs b/devolutions-gateway/src/api/jrec/search.rs index 64c591ef1e..8bb5200861 100644 --- a/devolutions-gateway/src/api/jrec/search.rs +++ b/devolutions-gateway/src/api/jrec/search.rs @@ -153,8 +153,8 @@ pub(crate) struct RecordingSearchHit { /// In a line holding a JSON object, it is matched against the string values of the object at any depth, as they read /// once unescaped, so field names, numbers, booleans and null never match. /// Any other line, such as one still being written, is matched as raw text. -/// A line longer than 256 KiB is matched on its first 256 KiB only, by the same rule, so a cut JSON object is -/// usually matched as raw text. +/// A line longer than 256 KiB is matched on its first 256 KiB only, by the same rule, so a cut JSON object is usually +/// matched as raw text. /// Only the files listed under `artifacts` in `recording.json` with the `slog` type are searched. /// Recordings without a readable `recording.json` and IDs with no recording on this instance give no hits. #[cfg_attr(feature = "openapi", utoipa::path( From 880576ae38876a8b62bb764dafb3d63607839302 Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Wed, 21 Oct 2026 14:32:46 -0400 Subject: [PATCH 18/19] docs(dgw): state the search intent reasons as rationale The reason for matching values described what raw matching did, and "a hit is a line that holds the query" was not true of a JSON line holding the query only in a field name. Issue: DGW-463 Co-Authored-By: Claude Opus 5.5 --- devolutions-gateway/src/api/jrec/search.intent.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/devolutions-gateway/src/api/jrec/search.intent.md b/devolutions-gateway/src/api/jrec/search.intent.md index c5b7bace1d..864975d463 100644 --- a/devolutions-gateway/src/api/jrec/search.intent.md +++ b/devolutions-gateway/src/api/jrec/search.intent.md @@ -7,13 +7,13 @@ A line holding a JSON object is matched case-insensitively against the object's Any other line is matched case-insensitively against its raw text, so that any kind of artifact, present or future, can be searched. A line cut at 256 KiB follows the same rule: what was read is matched by its string values when it parses as a JSON object, and as raw text otherwise. -Values are matched rather than raw text because users search the text they see: raw matching made a field name such as `description` match every line, and missed text that JSON escapes, such as `DOMAIN\user` stored as `DOMAIN\\user`. +Values are matched rather than raw text because users search the text they see: raw matching would make a field name such as `description` match every line, and would miss text that JSON escapes, such as `DOMAIN\user` stored as `DOMAIN\\user`. The rule never depends on field names, so a producer can change the fields it writes without any change to search. Per-field search, such as matching an entry's `description` only, is not supported: the fields a producer writes are unknown to Gateway, which only cares about values. ## Hits -A hit is a line that holds the query. +A hit is a line that matches the query. A hit carries the recording ID, the file name, the line number counting from 1, the byte offset of the line, and the raw line itself as a string. Line numbers count from 1 like the slog package in `webapp/packages/session-recording-log`, so that a hit and that package's own search give a line the same number. Gateway does not require a line to be JSON: a `.slog` is NDJSON by format, but a line holds whatever its producer pushed, such as a half-written last line while the push is live. From ca3fb61c4cffd44ea4cedfee890c7afe0be6db2b Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Wed, 21 Oct 2026 14:36:19 -0400 Subject: [PATCH 19/19] style(dgw): wrap a recording search test line as rustfmt does Issue: DGW-463 Co-Authored-By: Claude Opus 5.5 --- devolutions-gateway/src/api/jrec/search.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/devolutions-gateway/src/api/jrec/search.rs b/devolutions-gateway/src/api/jrec/search.rs index 8bb5200861..ea7609801a 100644 --- a/devolutions-gateway/src/api/jrec/search.rs +++ b/devolutions-gateway/src/api/jrec/search.rs @@ -605,7 +605,8 @@ mod tests { #[case::null("null")] #[tokio::test] async fn does_not_match_the_field_names_or_other_values_of_a_json_line(#[case] query: &str) { - let line = r#"{"seq":12,"done":true,"actor":null,"description":"Created User","parameters":{"New name":"Bob"}}"#; + let line = + r#"{"seq":12,"done":true,"actor":null,"description":"Created User","parameters":{"New name":"Bob"}}"#; let search = search_log(query, line.as_bytes()).await;