From eb5ac9cec3f6e6886e8f5f9a67f7ad0640d17a88 Mon Sep 17 00:00:00 2001 From: Mederick Date: Wed, 4 Nov 2026 15:55:17 -0500 Subject: [PATCH 1/6] feat(diagnostics): expose retained Gateway log files Issue: DVLS-15665 --- crates/devolutions-log/src/lib.rs | 64 ++++++++++++ devolutions-gateway/openapi/gateway-api.yaml | 72 +++++++++++++ devolutions-gateway/src/api/diagnostics.rs | 100 ++++++++++++++++++- devolutions-gateway/src/openapi.rs | 3 + 4 files changed, 238 insertions(+), 1 deletion(-) diff --git a/crates/devolutions-log/src/lib.rs b/crates/devolutions-log/src/lib.rs index bd885c3c31..cf59844a8b 100644 --- a/crates/devolutions-log/src/lib.rs +++ b/crates/devolutions-log/src/lib.rs @@ -21,6 +21,13 @@ pub trait StaticLogConfig { const LOG_FILE_PREFIX: &'static str; } +pub struct LogFile { + pub path: std::path::PathBuf, + pub name: String, + pub size: u64, + pub modified: SystemTime, +} + pub struct LoggerGuard { _file_guard: WorkerGuard, _stdio_guard: WorkerGuard, @@ -125,6 +132,63 @@ pub async fn find_latest_log_file(prefix: &Utf8Path) -> anyh most_recent.context("no file found") } +/// Lists the regular log files retained under the configured log path. +pub async fn list_log_files(path: &Utf8Path) -> anyhow::Result> { + let cfg = LogPathCfg::::from_path(path)?; + let mut read_dir = fs::read_dir(cfg.folder).await.context("couldn't read log directory")?; + let mut files = Vec::new(); + + while let Some(entry) = read_dir.next_entry().await.context("couldn't read log entry")? { + let Some(name) = entry.file_name().to_str().map(str::to_owned) else { + continue; + }; + + if !name.starts_with(cfg.prefix) || !name.ends_with(".log") { + continue; + } + + let file_type = match entry.file_type().await { + Ok(file_type) => file_type, + Err(error) => { + warn!(%error, name, "Couldn't read log file type"); + continue; + } + }; + if !file_type.is_file() { + continue; + } + + let metadata = match entry.metadata().await { + Ok(metadata) => metadata, + Err(error) => { + warn!(%error, name, "Couldn't read log metadata"); + continue; + } + }; + let modified = match metadata.modified() { + Ok(modified) => modified, + Err(error) => { + warn!(%error, name, "Couldn't read log modification time"); + continue; + } + }; + files.push(LogFile { + path: entry.path(), + name, + size: metadata.len(), + modified, + }); + } + + files.sort_by(|left, right| { + right + .modified + .cmp(&left.modified) + .then_with(|| right.name.cmp(&left.name)) + }); + Ok(files) +} + /// File deletion task (by age) /// /// Given path is used to filter out by file name prefix. diff --git a/devolutions-gateway/openapi/gateway-api.yaml b/devolutions-gateway/openapi/gateway-api.yaml index f3ed77ab0e..6ebbd083e7 100644 --- a/devolutions-gateway/openapi/gateway-api.yaml +++ b/devolutions-gateway/openapi/gateway-api.yaml @@ -107,6 +107,61 @@ paths: security: - scope_token: - gateway.diagnostics.read + /jet/diagnostics/logs/files: + get: + tags: + - Diagnostics + summary: Lists the log files retained by this Gateway. + operationId: ListLogFiles + responses: + '200': + description: Retained log files + content: + application/json: + schema: + type: array + items: + $ref: '#/components/schemas/LogFileInfo' + '401': + description: Invalid or missing authorization token + '403': + description: Insufficient permissions + '500': + description: Failed to list log files + security: + - scope_token: + - gateway.diagnostics.read + /jet/diagnostics/logs/files/{name}: + get: + tags: + - Diagnostics + summary: Retrieves one retained log file by its listed name. + operationId: GetLogFile + parameters: + - name: name + in: path + description: Name returned by ListLogFiles + required: true + schema: + type: string + responses: + '200': + description: Log file contents + content: + text/plain: + schema: + type: string + '401': + description: Invalid or missing authorization token + '403': + description: Insufficient permissions + '404': + description: Log file no longer exists + '500': + description: Failed to read log file + security: + - scope_token: + - gateway.diagnostics.read /jet/health: get: tags: @@ -2223,6 +2278,23 @@ components: internal_url: type: string description: URL to use on local network + LogFileInfo: + type: object + required: + - name + - size + - modifiedUnixMillis + properties: + name: + type: string + size: + type: integer + format: int64 + minimum: 0 + modifiedUnixMillis: + type: integer + format: int64 + minimum: 0 MonitorDefinition: type: object required: diff --git a/devolutions-gateway/src/api/diagnostics.rs b/devolutions-gateway/src/api/diagnostics.rs index 2bc44a2735..48af766113 100644 --- a/devolutions-gateway/src/api/diagnostics.rs +++ b/devolutions-gateway/src/api/diagnostics.rs @@ -1,4 +1,6 @@ -use axum::extract::State; +use std::time::UNIX_EPOCH; + +use axum::extract::{Path, State}; use axum::response::{IntoResponse as _, Response}; use axum::routing::get; use axum::{Json, Router}; @@ -15,6 +17,8 @@ use crate::log::GatewayLog; pub fn make_router(state: DgwState) -> Router { Router::new() .route("/logs", get(get_logs)) + .route("/logs/files", get(list_log_files)) + .route("/logs/files/{name}", get(get_log_file)) .route("/clock", get(get_clock)) .route("/configuration", get(get_configuration)) .with_state(state) @@ -135,6 +139,100 @@ async fn get_logs( Ok(axum_extra::body::AsyncReadBody::new(file).into_response()) } +#[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +pub(crate) struct LogFileInfo { + name: String, + size: u64, + modified_unix_millis: u64, +} + +/// Lists the log files retained by this Gateway. +#[cfg_attr(feature = "openapi", utoipa::path( + get, + operation_id = "ListLogFiles", + tag = "Diagnostics", + path = "/jet/diagnostics/logs/files", + responses( + (status = 200, description = "Retained log files", body = [LogFileInfo]), + (status = 401, description = "Invalid or missing authorization token"), + (status = 403, description = "Insufficient permissions"), + (status = 500, description = "Failed to list log files"), + ), + security(("scope_token" = ["gateway.diagnostics.read"])), +))] +async fn list_log_files( + State(DgwState { conf_handle, .. }): State, + _scope: DiagnosticsReadScope, +) -> Result>, HttpError> { + let conf = conf_handle.get_conf(); + let files = devolutions_log::list_log_files::(conf.log_file.as_path()) + .await + .map_err(HttpError::internal().err())?; + + Ok(Json( + files + .into_iter() + .map(|file| LogFileInfo { + name: file.name, + size: file.size, + modified_unix_millis: file + .modified + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_millis() + .try_into() + .unwrap_or(u64::MAX), + }) + .collect(), + )) +} + +/// Retrieves one retained log file by its listed name. +#[cfg_attr(feature = "openapi", utoipa::path( + get, + operation_id = "GetLogFile", + tag = "Diagnostics", + path = "/jet/diagnostics/logs/files/{name}", + params(("name" = String, Path, description = "Name returned by ListLogFiles")), + responses( + (status = 200, description = "Log file contents", body = String, content_type = "text/plain"), + (status = 401, description = "Invalid or missing authorization token"), + (status = 403, description = "Insufficient permissions"), + (status = 404, description = "Log file no longer exists"), + (status = 500, description = "Failed to read log file"), + ), + security(("scope_token" = ["gateway.diagnostics.read"])), +))] +async fn get_log_file( + State(DgwState { conf_handle, .. }): State, + Path(name): Path, + _scope: DiagnosticsReadScope, +) -> Result { + let conf = conf_handle.get_conf(); + let files = devolutions_log::list_log_files::(conf.log_file.as_path()) + .await + .map_err(HttpError::internal().err())?; + let file = files + .into_iter() + .find(|file| file.name == name) + .ok_or_else(|| HttpError::not_found().msg("log file not found"))?; + let file = File::open(file.path).await.map_err(|error| { + if error.kind() == std::io::ErrorKind::NotFound { + HttpError::not_found().build(error) + } else { + HttpError::internal().build(error) + } + })?; + + Ok(( + [(axum::http::header::CONTENT_TYPE, "text/plain; charset=utf-8")], + axum_extra::body::AsyncReadBody::new(file), + ) + .into_response()) +} + /// Retrieves a subset of the configuration, for diagnosis purposes. /// /// This route primary function is to help with configuration diagnosis (e.g.: ID mismatch, hostname mismatch, diff --git a/devolutions-gateway/src/openapi.rs b/devolutions-gateway/src/openapi.rs index e1166a9396..d0c73d0a26 100644 --- a/devolutions-gateway/src/openapi.rs +++ b/devolutions-gateway/src/openapi.rs @@ -20,6 +20,8 @@ use crate::provisioner_tasks::recording_ai_analysis::{BadAnswer, Failure, Recove crate::api::sessions::get_sessions, crate::api::session::terminate_session, crate::api::diagnostics::get_logs, + crate::api::diagnostics::list_log_files, + crate::api::diagnostics::get_log_file, crate::api::diagnostics::get_configuration, crate::api::diagnostics::get_clock, crate::api::config::patch_config, @@ -63,6 +65,7 @@ use crate::provisioner_tasks::recording_ai_analysis::{BadAnswer, Failure, Recove Subscriber, crate::api::diagnostics::ConfigDiagnostic, crate::api::diagnostics::ClockDiagnostic, + crate::api::diagnostics::LogFileInfo, SubProvisionerKey, ConfigPatch, crate::api::jrl::JrlInfo, From 8e155a1869b454f864c88b9797c026d77803819f Mon Sep 17 00:00:00 2001 From: Mederick Date: Thu, 5 Nov 2026 14:39:10 -0500 Subject: [PATCH 2/6] fix(diagnostics): reject linked log paths on open Issue: DVLS-15665 Security: yes --- devolutions-gateway/src/api/diagnostics.rs | 85 +++++++++++++++++++++- 1 file changed, 82 insertions(+), 3 deletions(-) diff --git a/devolutions-gateway/src/api/diagnostics.rs b/devolutions-gateway/src/api/diagnostics.rs index 48af766113..58cce9a0cf 100644 --- a/devolutions-gateway/src/api/diagnostics.rs +++ b/devolutions-gateway/src/api/diagnostics.rs @@ -1,10 +1,12 @@ +use std::io; +use std::path::Path as FsPath; use std::time::UNIX_EPOCH; use axum::extract::{Path, State}; use axum::response::{IntoResponse as _, Response}; use axum::routing::get; use axum::{Json, Router}; -use tokio::fs::File; +use tokio::fs::{File, OpenOptions}; use uuid::Uuid; use crate::DgwState; @@ -218,8 +220,8 @@ async fn get_log_file( .into_iter() .find(|file| file.name == name) .ok_or_else(|| HttpError::not_found().msg("log file not found"))?; - let file = File::open(file.path).await.map_err(|error| { - if error.kind() == std::io::ErrorKind::NotFound { + let file = open_regular_log_file(&file.path).await.map_err(|error| { + if error.kind() == io::ErrorKind::NotFound { HttpError::not_found().build(error) } else { HttpError::internal().build(error) @@ -233,6 +235,41 @@ async fn get_log_file( .into_response()) } +async fn open_regular_log_file(path: &FsPath) -> io::Result { + let mut options = OpenOptions::new(); + options.read(true); + + #[cfg(unix)] + options.custom_flags(libc::O_NOFOLLOW | libc::O_NONBLOCK); + + #[cfg(windows)] + options.custom_flags(windows_sys::Win32::Storage::FileSystem::FILE_FLAG_OPEN_REPARSE_POINT); + + let file = options.open(path).await?; + let metadata = file.metadata().await?; + if !metadata.is_file() { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "log path is not a regular file", + )); + } + + #[cfg(windows)] + { + use std::os::windows::fs::MetadataExt as _; + use windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_REPARSE_POINT; + + if metadata.file_attributes() & FILE_ATTRIBUTE_REPARSE_POINT != 0 { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "log path is a reparse point", + )); + } + } + + Ok(file) +} + /// Retrieves a subset of the configuration, for diagnosis purposes. /// /// This route primary function is to help with configuration diagnosis (e.g.: ID mismatch, hostname mismatch, @@ -279,3 +316,45 @@ async fn get_configuration( async fn get_clock() -> Json { Json(ClockDiagnostic::now()) } + +#[cfg(test)] +mod tests { + use super::open_regular_log_file; + + // oracle: regression Devolutions/devolutions-gateway#2185 review comment 4473309953. + #[tokio::test] + async fn replaced_log_file_symlink_is_rejected() { + let dir = tempfile::tempdir().expect("create temporary directory"); + let log_dir = dir.path().join("logs"); + std::fs::create_dir(&log_dir).expect("create log directory"); + let outside = dir.path().join("outside.txt"); + let selected_log = log_dir.join("gateway.log"); + std::fs::write(&outside, b"private data").expect("create outside file"); + std::fs::write(&selected_log, b"log data").expect("create selected log"); + std::fs::remove_file(&selected_log).expect("replace selected log"); + + #[cfg(unix)] + std::os::unix::fs::symlink(&outside, &selected_log).expect("replace log with symlink"); + #[cfg(windows)] + if let Err(error) = std::os::windows::fs::symlink_file(&outside, &selected_log) { + if error.raw_os_error() == Some(1314) { + eprintln!("Windows symlink privilege is unavailable; skipping symlink check"); + return; + } + panic!("replace log with symlink: {error}"); + } + + assert!(open_regular_log_file(&selected_log).await.is_err()); + } + + // oracle: regression Devolutions/devolutions-gateway#2185 review comment 4473309953. + #[tokio::test] + async fn opened_log_handle_must_be_a_regular_file() { + let dir = tempfile::tempdir().expect("create temporary directory"); + let regular_file = dir.path().join("gateway.log"); + std::fs::write(®ular_file, b"log data").expect("create regular file"); + + assert!(open_regular_log_file(®ular_file).await.is_ok()); + assert!(open_regular_log_file(dir.path()).await.is_err()); + } +} From 5d1f75b5d631c222845bfd33c4946bd1dc46580a Mon Sep 17 00:00:00 2001 From: Mederick Date: Thu, 5 Nov 2026 14:42:51 -0500 Subject: [PATCH 3/6] style(diagnostics): satisfy nightly import formatting Issue: DVLS-15665 Changelog: ignore --- devolutions-gateway/src/api/diagnostics.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/devolutions-gateway/src/api/diagnostics.rs b/devolutions-gateway/src/api/diagnostics.rs index 58cce9a0cf..fdb68024a8 100644 --- a/devolutions-gateway/src/api/diagnostics.rs +++ b/devolutions-gateway/src/api/diagnostics.rs @@ -257,6 +257,7 @@ async fn open_regular_log_file(path: &FsPath) -> io::Result { #[cfg(windows)] { use std::os::windows::fs::MetadataExt as _; + use windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_REPARSE_POINT; if metadata.file_attributes() & FILE_ATTRIBUTE_REPARSE_POINT != 0 { From 9cfee51c3f71a6bc956e7bd392c1adcf83c55aed Mon Sep 17 00:00:00 2001 From: Mederick Date: Mon, 9 Nov 2026 12:13:56 -0500 Subject: [PATCH 4/6] fix: avoid stderr in symlink regression test Issue: DVLS-15665 --- devolutions-gateway/src/api/diagnostics.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/devolutions-gateway/src/api/diagnostics.rs b/devolutions-gateway/src/api/diagnostics.rs index fdb68024a8..84486407ac 100644 --- a/devolutions-gateway/src/api/diagnostics.rs +++ b/devolutions-gateway/src/api/diagnostics.rs @@ -339,7 +339,6 @@ mod tests { #[cfg(windows)] if let Err(error) = std::os::windows::fs::symlink_file(&outside, &selected_log) { if error.raw_os_error() == Some(1314) { - eprintln!("Windows symlink privilege is unavailable; skipping symlink check"); return; } panic!("replace log with symlink: {error}"); From 0024bba6a3e0ee17fd4438d965cf6b5bb109c3d3 Mon Sep 17 00:00:00 2001 From: Mederick Date: Mon, 9 Nov 2026 13:33:56 -0500 Subject: [PATCH 5/6] fix(diagnostics): align retained log routes and error handling The latest-log route could select files outside the retained log filter and follow a swapped symlink. Share retained-file selection and guarded opening across both routes, return 404 for unavailable files, preserve signed modification times, and order numeric rotations correctly when timestamps match. Issue: DVLS-15665 --- crates/devolutions-log/src/lib.rs | 56 ++++++------ devolutions-gateway/openapi/gateway-api.yaml | 9 +- devolutions-gateway/src/api/diagnostics.rs | 94 ++++++++++++++++---- 3 files changed, 109 insertions(+), 50 deletions(-) diff --git a/crates/devolutions-log/src/lib.rs b/crates/devolutions-log/src/lib.rs index cf59844a8b..b863ca9b4d 100644 --- a/crates/devolutions-log/src/lib.rs +++ b/crates/devolutions-log/src/lib.rs @@ -21,6 +21,7 @@ pub trait StaticLogConfig { const LOG_FILE_PREFIX: &'static str; } +/// Metadata for a retained log file in the configured log directory. pub struct LogFile { pub path: std::path::PathBuf, pub name: String, @@ -98,38 +99,17 @@ pub fn init( }) } -/// Find latest log file (by age) +/// Finds the newest retained regular log file by modification time. /// -/// Given path is used to filter out by file name prefix. +/// Uses the same file filter and order as [`list_log_files`]. #[instrument] pub async fn find_latest_log_file(prefix: &Utf8Path) -> anyhow::Result { - let cfg = LogPathCfg::::from_path(prefix)?; - - let mut read_dir = fs::read_dir(cfg.folder).await.context("couldn't read directory")?; - - let mut most_recent_time = SystemTime::UNIX_EPOCH; - let mut most_recent = None; - - while let Ok(Some(entry)) = read_dir.next_entry().await { - match entry.file_name().to_str() { - Some(file_name) if file_name.starts_with(cfg.prefix) && file_name.contains("log") => { - debug!(file_name, "Found a log file"); - match entry.metadata().await.and_then(|metadata| metadata.modified()) { - Ok(modified) if modified > most_recent_time => { - most_recent_time = modified; - most_recent = Some(entry.path()); - } - Ok(_) => {} - Err(error) => { - warn!(%error, file_name, "Couldn't retrieve metadata for file"); - } - } - } - _ => continue, - } - } - - most_recent.context("no file found") + list_log_files::(prefix) + .await? + .into_iter() + .next() + .map(|file| file.path) + .context("no file found") } /// Lists the regular log files retained under the configured log path. @@ -184,11 +164,27 @@ pub async fn list_log_files(path: &Utf8Path) -> anyhow::Resu right .modified .cmp(&left.modified) - .then_with(|| right.name.cmp(&left.name)) + .then_with(|| compare_log_file_names(&right.name, &left.name)) }); Ok(files) } +fn compare_log_file_names(left: &str, right: &str) -> std::cmp::Ordering { + let rotations = left + .strip_suffix(".log") + .and_then(|name| name.rsplit_once('.')) + .zip(right.strip_suffix(".log").and_then(|name| name.rsplit_once('.'))); + + if let Some(((left_base, left_rotation), (right_base, right_rotation))) = rotations + && left_base == right_base + && let (Ok(left_rotation), Ok(right_rotation)) = (left_rotation.parse::(), right_rotation.parse::()) + { + return left_rotation.cmp(&right_rotation).then_with(|| left.cmp(right)); + } + + left.cmp(right) +} + /// File deletion task (by age) /// /// Given path is used to filter out by file name prefix. diff --git a/devolutions-gateway/openapi/gateway-api.yaml b/devolutions-gateway/openapi/gateway-api.yaml index 6ebbd083e7..ebd37e3ea9 100644 --- a/devolutions-gateway/openapi/gateway-api.yaml +++ b/devolutions-gateway/openapi/gateway-api.yaml @@ -156,7 +156,7 @@ paths: '403': description: Insufficient permissions '404': - description: Log file no longer exists + description: Log file is no longer available '500': description: Failed to read log file security: @@ -2285,16 +2285,15 @@ components: - size - modifiedUnixMillis properties: + modifiedUnixMillis: + type: integer + format: int64 name: type: string size: type: integer format: int64 minimum: 0 - modifiedUnixMillis: - type: integer - format: int64 - minimum: 0 MonitorDefinition: type: object required: diff --git a/devolutions-gateway/src/api/diagnostics.rs b/devolutions-gateway/src/api/diagnostics.rs index 84486407ac..bbc0004893 100644 --- a/devolutions-gateway/src/api/diagnostics.rs +++ b/devolutions-gateway/src/api/diagnostics.rs @@ -1,6 +1,5 @@ use std::io; use std::path::Path as FsPath; -use std::time::UNIX_EPOCH; use axum::extract::{Path, State}; use axum::response::{IntoResponse as _, Response}; @@ -134,7 +133,7 @@ async fn get_logs( .await .map_err(HttpError::internal().with_msg("latest log file not found").err())?; - let file = File::open(&latest_log_file_path) + let file = open_regular_log_file(&latest_log_file_path) .await .map_err(HttpError::internal().err())?; @@ -147,7 +146,7 @@ async fn get_logs( pub(crate) struct LogFileInfo { name: String, size: u64, - modified_unix_millis: u64, + modified_unix_millis: i64, } /// Lists the log files retained by this Gateway. @@ -179,13 +178,10 @@ async fn list_log_files( .map(|file| LogFileInfo { name: file.name, size: file.size, - modified_unix_millis: file - .modified - .duration_since(UNIX_EPOCH) - .unwrap_or_default() - .as_millis() - .try_into() - .unwrap_or(u64::MAX), + modified_unix_millis: i64::try_from( + time::OffsetDateTime::from(file.modified).unix_timestamp_nanos() / 1_000_000, + ) + .expect("log timestamp fits in i64 milliseconds"), }) .collect(), )) @@ -202,7 +198,7 @@ async fn list_log_files( (status = 200, description = "Log file contents", body = String, content_type = "text/plain"), (status = 401, description = "Invalid or missing authorization token"), (status = 403, description = "Insufficient permissions"), - (status = 404, description = "Log file no longer exists"), + (status = 404, description = "Log file is no longer available"), (status = 500, description = "Failed to read log file"), ), security(("scope_token" = ["gateway.diagnostics.read"])), @@ -221,7 +217,10 @@ async fn get_log_file( .find(|file| file.name == name) .ok_or_else(|| HttpError::not_found().msg("log file not found"))?; let file = open_regular_log_file(&file.path).await.map_err(|error| { - if error.kind() == io::ErrorKind::NotFound { + let unavailable = matches!(error.kind(), io::ErrorKind::NotFound | io::ErrorKind::InvalidData); + #[cfg(unix)] + let unavailable = unavailable || error.raw_os_error() == Some(libc::ELOOP); + if unavailable { HttpError::not_found().build(error) } else { HttpError::internal().build(error) @@ -240,6 +239,8 @@ async fn open_regular_log_file(path: &FsPath) -> io::Result { options.read(true); #[cfg(unix)] + // UNTESTED: A FIFO swapped in after listing must not block this open, so diagnostics stays responsive. + // Check a FIFO replacement before changing O_NONBLOCK. options.custom_flags(libc::O_NOFOLLOW | libc::O_NONBLOCK); #[cfg(windows)] @@ -261,6 +262,8 @@ async fn open_regular_log_file(path: &FsPath) -> io::Result { use windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_REPARSE_POINT; if metadata.file_attributes() & FILE_ATTRIBUTE_REPARSE_POINT != 0 { + // UNTESTED: A reparse-point file must not be streamed, but the test skips on hosts without symlink privilege. + // Exercise a file symlink on a Windows host that can create one before changing this check. return Err(io::Error::new( io::ErrorKind::InvalidData, "log path is a reparse point", @@ -321,8 +324,67 @@ async fn get_clock() -> Json { #[cfg(test)] mod tests { use super::open_regular_log_file; + use crate::log::GatewayLog; + + // oracle: mutant selecting a log-like file without the .log suffix. + #[tokio::test] + async fn latest_log_file_uses_retained_log_filter() { + let dir = tempfile::tempdir().expect("create temporary directory"); + let retained = dir.path().join("gateway.2026-11-09.0.log"); + let other = dir.path().join("gateway.2026-11-09.1.log.tmp"); + std::fs::write(&retained, b"log data").expect("create retained log"); + std::fs::write(&other, b"other data").expect("create other file"); + let older = std::fs::FileTimes::new() + .set_modified(std::time::UNIX_EPOCH + std::time::Duration::from_secs(1_600_000_000)); + let newer = std::fs::FileTimes::new() + .set_modified(std::time::UNIX_EPOCH + std::time::Duration::from_secs(1_600_000_060)); + std::fs::OpenOptions::new() + .write(true) + .open(&retained) + .expect("open retained log") + .set_times(older) + .expect("set retained log time"); + std::fs::OpenOptions::new() + .write(true) + .open(&other) + .expect("open other file") + .set_times(newer) + .expect("set other file time"); + + let log_path = camino::Utf8Path::from_path(dir.path()).expect("UTF-8 log path"); + let latest = devolutions_log::find_latest_log_file::(log_path) + .await + .expect("find latest log"); + assert_eq!(latest, retained); + } + + // oracle: mutant sorting rotation numbers as strings when modification times match. + #[tokio::test] + async fn latest_log_file_orders_numeric_rotations() { + let dir = tempfile::tempdir().expect("create temporary directory"); + let older_rotation = dir.path().join("gateway.2026-11-09.9.log"); + let newer_rotation = dir.path().join("gateway.2026-11-09.10.log"); + std::fs::write(&older_rotation, b"older rotation").expect("create older rotation"); + std::fs::write(&newer_rotation, b"newer rotation").expect("create newer rotation"); + let modified = std::fs::FileTimes::new() + .set_modified(std::time::UNIX_EPOCH + std::time::Duration::from_secs(1_600_000_000)); + for path in [&older_rotation, &newer_rotation] { + std::fs::OpenOptions::new() + .write(true) + .open(path) + .expect("open rotation") + .set_times(modified) + .expect("set rotation time"); + } + + let log_path = camino::Utf8Path::from_path(dir.path()).expect("UTF-8 log path"); + let latest = devolutions_log::find_latest_log_file::(log_path) + .await + .expect("find latest log"); + assert_eq!(latest, newer_rotation); + } - // oracle: regression Devolutions/devolutions-gateway#2185 review comment 4473309953. + // oracle: mutant following a symlink outside the configured log directory. #[tokio::test] async fn replaced_log_file_symlink_is_rejected() { let dir = tempfile::tempdir().expect("create temporary directory"); @@ -338,7 +400,9 @@ mod tests { std::os::unix::fs::symlink(&outside, &selected_log).expect("replace log with symlink"); #[cfg(windows)] if let Err(error) = std::os::windows::fs::symlink_file(&outside, &selected_log) { - if error.raw_os_error() == Some(1314) { + if error.raw_os_error().and_then(|code| u32::try_from(code).ok()) + == Some(windows_sys::Win32::Foundation::ERROR_PRIVILEGE_NOT_HELD) + { return; } panic!("replace log with symlink: {error}"); @@ -347,7 +411,7 @@ mod tests { assert!(open_regular_log_file(&selected_log).await.is_err()); } - // oracle: regression Devolutions/devolutions-gateway#2185 review comment 4473309953. + // oracle: mutant accepting a directory or rejecting a regular log file. #[tokio::test] async fn opened_log_handle_must_be_a_regular_file() { let dir = tempfile::tempdir().expect("create temporary directory"); From 5c820e33242bc133b63c75b128a4bbfbd89139bf Mon Sep 17 00:00:00 2001 From: Mederick Date: Mon, 9 Nov 2026 14:12:05 -0500 Subject: [PATCH 6/6] fix(diagnostics): check scope before log file path Malformed log names could fail extraction before diagnostics scope validation. Check the scope first, document the possible bad request and both not-found cases, and exercise the list and download contract through the shipped Gateway HTTP routes. Issue: DVLS-15665 Security: yes --- devolutions-gateway/openapi/gateway-api.yaml | 4 +- devolutions-gateway/src/api/diagnostics.rs | 5 +- testsuite/tests/cli/dgw/diagnostics.rs | 112 +++++++++++++++++++ testsuite/tests/cli/dgw/mod.rs | 1 + 4 files changed, 119 insertions(+), 3 deletions(-) create mode 100644 testsuite/tests/cli/dgw/diagnostics.rs diff --git a/devolutions-gateway/openapi/gateway-api.yaml b/devolutions-gateway/openapi/gateway-api.yaml index ebd37e3ea9..9bda555e14 100644 --- a/devolutions-gateway/openapi/gateway-api.yaml +++ b/devolutions-gateway/openapi/gateway-api.yaml @@ -151,12 +151,14 @@ paths: text/plain: schema: type: string + '400': + description: Invalid log file name '401': description: Invalid or missing authorization token '403': description: Insufficient permissions '404': - description: Log file is no longer available + description: Log file not found '500': description: Failed to read log file security: diff --git a/devolutions-gateway/src/api/diagnostics.rs b/devolutions-gateway/src/api/diagnostics.rs index bbc0004893..3b4b49c37a 100644 --- a/devolutions-gateway/src/api/diagnostics.rs +++ b/devolutions-gateway/src/api/diagnostics.rs @@ -196,17 +196,18 @@ async fn list_log_files( params(("name" = String, Path, description = "Name returned by ListLogFiles")), responses( (status = 200, description = "Log file contents", body = String, content_type = "text/plain"), + (status = 400, description = "Invalid log file name"), (status = 401, description = "Invalid or missing authorization token"), (status = 403, description = "Insufficient permissions"), - (status = 404, description = "Log file is no longer available"), + (status = 404, description = "Log file not found"), (status = 500, description = "Failed to read log file"), ), security(("scope_token" = ["gateway.diagnostics.read"])), ))] async fn get_log_file( State(DgwState { conf_handle, .. }): State, - Path(name): Path, _scope: DiagnosticsReadScope, + Path(name): Path, ) -> Result { let conf = conf_handle.get_conf(); let files = devolutions_log::list_log_files::(conf.log_file.as_path()) diff --git a/testsuite/tests/cli/dgw/diagnostics.rs b/testsuite/tests/cli/dgw/diagnostics.rs new file mode 100644 index 0000000000..7314816529 --- /dev/null +++ b/testsuite/tests/cli/dgw/diagnostics.rs @@ -0,0 +1,112 @@ +//! Retained Gateway log API tests. + +use anyhow::Context as _; +use base64::Engine as _; +use testsuite::cli::start_dgw; +use testsuite::dgw_config::{DgwConfig, DgwConfigHandle}; +use uuid::Uuid; + +const LOG_NAME: &str = "gateway.contract.log"; +const LOG_CONTENT: &str = "diagnostics route contract\n"; + +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"), + ) +} + +async fn start_with_log_file() -> anyhow::Result<(DgwConfigHandle, tokio::process::Child)> { + let config = DgwConfig::builder().disable_token_validation(true).build().init()?; + std::fs::write(config.config_dir().join(LOG_NAME), LOG_CONTENT).context("write retained log")?; + std::fs::write(config.config_dir().join("gateway.contract.log.tmp"), "ignored") + .context("write non-retained file")?; + let process = start_dgw(&config).await?; + Ok((config, process)) +} + +// oracle: mutant omitting retained files, changing response fields, or returning different download bytes. +#[tokio::test] +async fn lists_and_downloads_retained_log_files() -> anyhow::Result<()> { + let (config, _process) = start_with_log_file().await?; + let client = reqwest::Client::new(); + let base = format!("http://127.0.0.1:{}/jet/diagnostics/logs/files", config.http_port()); + let token = scope_token("gateway.diagnostics.read"); + + let response = client.get(&base).bearer_auth(&token).send().await?; + assert_eq!(response.status(), reqwest::StatusCode::OK); + let files: serde_json::Value = response.json().await?; + let files = files.as_array().context("log list is an array")?; + let log = files + .iter() + .find(|file| file["name"] == LOG_NAME) + .context("fixture log is listed")?; + assert_eq!(log["size"], serde_json::json!(LOG_CONTENT.len())); + assert!(log["modifiedUnixMillis"].as_i64().is_some()); + assert!(!files.iter().any(|file| file["name"] == "gateway.contract.log.tmp")); + + let response = client + .get(format!("{base}/{LOG_NAME}")) + .bearer_auth(&token) + .send() + .await?; + assert_eq!(response.status(), reqwest::StatusCode::OK); + assert_eq!( + response + .headers() + .get(reqwest::header::CONTENT_TYPE) + .context("content type")?, + "text/plain; charset=utf-8" + ); + assert_eq!(response.text().await?, LOG_CONTENT); + + let response = client + .get(format!("{base}/gateway.missing.log")) + .bearer_auth(&token) + .send() + .await?; + assert_eq!(response.status(), reqwest::StatusCode::NOT_FOUND); + + Ok(()) +} + +// oracle: mutant bypassing diagnostics scope or extracting a malformed path before checking scope. +#[tokio::test] +async fn log_routes_enforce_diagnostics_scope_before_path_extraction() -> anyhow::Result<()> { + let (config, _process) = start_with_log_file().await?; + let client = reqwest::Client::new(); + let base = format!("http://127.0.0.1:{}/jet/diagnostics/logs/files", config.http_port()); + let wrong_scope = scope_token("gateway.sessions.read"); + + for url in [base.clone(), format!("{base}/{LOG_NAME}"), format!("{base}/%FF")] { + let response = client.get(&url).bearer_auth(&wrong_scope).send().await?; + assert_eq!(response.status(), reqwest::StatusCode::FORBIDDEN, "{url}"); + } + + let response = client + .get(format!("{base}/%FF")) + .bearer_auth(scope_token("gateway.diagnostics.read")) + .send() + .await?; + assert_eq!(response.status(), reqwest::StatusCode::BAD_REQUEST); + + for url in [base.clone(), format!("{base}/{LOG_NAME}")] { + let response = client.get(&url).send().await?; + assert_eq!(response.status(), reqwest::StatusCode::UNAUTHORIZED, "{url}"); + } + + Ok(()) +} diff --git a/testsuite/tests/cli/dgw/mod.rs b/testsuite/tests/cli/dgw/mod.rs index 85e2b02bcd..b7dfa74cd2 100644 --- a/testsuite/tests/cli/dgw/mod.rs +++ b/testsuite/tests/cli/dgw/mod.rs @@ -1,6 +1,7 @@ mod ai_analysis; mod benign_disconnect; mod cli_args; +mod diagnostics; mod heartbeat; mod jrec_push; mod kdc_proxy;