Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions devolutions-gateway/openapi/gateway-api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,22 @@ paths:
'200':
description: Recording file
content:
video/webm:
schema:
type: string
format: binary
application/x-asciicast:
schema:
type: string
format: binary
application/x-ndjson:
schema:
type: string
format: binary
application/json:
schema:
type: string
format: binary
application/octet-stream:
schema:
type: string
Expand Down
59 changes: 48 additions & 11 deletions devolutions-gateway/src/api/jrec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,18 @@ pub(crate) async fn pull_recording_session(
("filename" = String, Path, description = "Name of recording file to retrieve"),
),
responses(
(status = 200, description = "Recording file", body = Vec<u8>),
(
status = 200,
description = "Recording file",
body = Vec<u8>,
content_type = [
Comment thread
kristahouse marked this conversation as resolved.
"video/webm",
"application/x-asciicast",
"application/x-ndjson",
"application/json",
"application/octet-stream",
],
),
(status = 400, description = "Bad request"),
(status = 401, description = "Invalid or missing authorization token"),
(status = 403, description = "Insufficient permissions"),
Expand Down Expand Up @@ -609,16 +620,10 @@ where
.await
.map_err(HttpError::internal().err())?;

let content_type = path
.extension()
.and_then(RecordingFileType::from_extension)
.and_then(RecordingFileType::content_type);

if let Some(content_type) = content_type {
response
.headers_mut()
.insert(CONTENT_TYPE, HeaderValue::from_static(content_type));
}
let content_type = recording_file_content_type(&path);
response
.headers_mut()
.insert(CONTENT_TYPE, HeaderValue::from_static(content_type));

Ok(response)
}
Expand All @@ -640,6 +645,17 @@ fn is_safe_recording_file_name(file_name: &str) -> bool {
!file_name.is_empty() && !file_name.contains("..") && !file_name.contains('/') && !file_name.contains('\\')
}

fn recording_file_content_type(path: &Utf8Path) -> &'static str {
if path.file_name() == Some("recording.json") {
return "application/json";
}

path.extension()
.and_then(RecordingFileType::from_extension)
.map(RecordingFileType::content_type)
.unwrap_or("application/octet-stream")
}

/// Immutable package membership for one download attempt.
///
/// `manifest_bytes` are the exact `recording.json` contents used to derive `clip_names`,
Expand Down Expand Up @@ -1016,6 +1032,27 @@ mod tests {
assert!(!is_safe_recording_file_name("a\\b.webm"));
}

#[test]
fn detects_recording_file_content_types() {
let expected_content_types = [
("recording-0.webm", "video/webm"),
("recording-0.trp", "application/octet-stream"),
("recording-0.cast", "application/x-asciicast"),
("recording-0.slog", "application/x-ndjson"),
("recording.json", "application/json"),
("recording-0.bin", "application/octet-stream"),
("recording-0", "application/octet-stream"),
];

for (file_name, expected_content_type) in expected_content_types {
assert_eq!(
recording_file_content_type(Utf8Path::new(file_name)),
expected_content_type,
"unexpected content type for {file_name}"
);
}
}

#[tokio::test]
async fn snapshots_manifest_files_for_zip() {
let dir = tempfile::tempdir().expect("temp dir");
Expand Down
49 changes: 49 additions & 0 deletions devolutions-gateway/src/streaming.intent.md
Comment thread
kristahouse marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# Recording streaming intent
## Scope

These rules apply to the `/shadow` WebSocket streaming path implemented by `streaming.rs`.

This includes:

- recording file-type classification
- streamability decisions
- streaming implementation selection
- terminal input-format selection
They do not define the following:

- JREC push behaviour
- JREC pull behaviour
- artifact storage
- download MIME types
- consumer-side rendering.

## Streaming contract

Only WebM, asciicast, and TRP recording artifacts are accepted by the `/shadow` streaming path.

| Recording file type | Extension | Streaming behaviour |
| --- | --- | --- |
| `WebM` | `.webm` | WebM streaming |
| `Asciicast` | `.cast` | Terminal streaming using asciinema input |
| `TRP` | `.trp` | Terminal streaming using TRP input |
| `SessionRecordingLog` | `.slog` | Explicitly rejected by the `/shadow` streaming path |

- A recognised`RecordingFileType` is not automatically supported by `/shadow` streaming. Each recognised recording file type must have explicitly defined behaviour for the `/shadow` streaming path.
- Files with missing or unrecognised extensions must be rejected before WebSocket streaming begins.

## Architectural invariants

- Recording artifact streaming must use the canonical `RecordingFileType` extension mapping as its source of truth.
- A recording file must be classified once. The resulting `RecordingFileType` must determine:
- if the artifact is supported by the `/shadow` streaming path
- which streaming implementation is used (when applicable)
- which terminal input format is used (when applicable)

- Streaming validation, streamer selection, and terminal input selection must not maintain separate extension mappings or independently compare known recording extensions as raw strings.
- Adding a new `RecordingFileType` requires an explicit decision about whether it is supported by the `/shadow` streaming path and, if supported, how it is streamed.
- A new or unsupported recording file type must not silently fall back to an existing streaming implementation or terminal input format.
## Component boundaries
JREC artifact handling, storage, download content types, and consumer-side rendering are outside the scope of this document.


> **Boundary:** Session Recording Log artifacts are supported elsewhere in Gateway through the JREC recording flow. Their rejection by `/shadow` applies only to the WebSocket streaming path covered by this document.
109 changes: 85 additions & 24 deletions devolutions-gateway/src/streaming.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,11 @@ pub(crate) async fn stream_file(

let path = Arc::new(path.to_owned());
let upgrade_result = match streaming_type {
StreamingType::Terminal => {
StreamingType::Terminal(input_type) => {
let shutdown_notify = Arc::clone(&shutdown_notify);
ws.on_upgrade(move |socket| async move {
if let Err(e) = setup_terminal_streaming(&path, socket, shutdown_notify, when_new_chunk_appended).await
if let Err(e) =
setup_terminal_streaming(&path, input_type, socket, shutdown_notify, when_new_chunk_appended).await
{
error!(error = ?e, "Terminal streaming failed");
}
Expand Down Expand Up @@ -77,34 +78,35 @@ impl terminal_streamer::TerminalStreamSocket for TerminalStreamSocketImpl {
}

enum StreamingType {
Terminal,
Terminal(terminal_streamer::InputStreamType),
WebM,
}

/// Determines streamability from recording type, which is stricter than pull MIME handling.
/// A file may be downloadable but still rejected here when there is no streaming backend.
async fn validate_streaming_file(path: &camino::Utf8Path) -> anyhow::Result<StreamingType> {
let path_extension = path
.extension()
.context("no extension found in the recording file path")?;

info!(?path, extension = ?path_extension, "Streaming file");
if !(path_extension == RecordingFileType::WebM.extension()
|| path_extension == RecordingFileType::Asciicast.extension()
|| path_extension == RecordingFileType::TRP.extension())
{
anyhow::bail!("invalid file type");
}
let file_type =
RecordingFileType::from_extension(path_extension).ok_or_else(|| anyhow::anyhow!("invalid file type"))?;
streaming_type_for_file_type(file_type)
}

if path_extension == RecordingFileType::Asciicast.extension()
|| path_extension == RecordingFileType::TRP.extension()
{
Ok(StreamingType::Terminal)
} else {
Ok(StreamingType::WebM)
fn streaming_type_for_file_type(file_type: RecordingFileType) -> anyhow::Result<StreamingType> {
match file_type {
RecordingFileType::Asciicast => Ok(StreamingType::Terminal(terminal_streamer::InputStreamType::Asciinema)),
RecordingFileType::TRP => Ok(StreamingType::Terminal(terminal_streamer::InputStreamType::Trp)),
RecordingFileType::WebM => Ok(StreamingType::WebM),
RecordingFileType::SessionRecordingLog => anyhow::bail!("invalid file type"),
}
}

async fn setup_terminal_streaming(
path: &camino::Utf8Path,
input_type: terminal_streamer::InputStreamType,
socket: WebSocket,
shutdown_notify: Arc<Notify>,
when_new_chunk_appended: impl Fn() -> tokio::sync::oneshot::Receiver<()> + Send + 'static,
Expand All @@ -127,15 +129,6 @@ async fn setup_terminal_streaming(
.await
.with_context(|| format!("failed to open file: {path:?}"))?;

let path_extension = path
.extension()
.context("no extension found in the recording file path")?;
let input_type = if path_extension == RecordingFileType::Asciicast.extension() {
terminal_streamer::InputStreamType::Asciinema
} else {
terminal_streamer::InputStreamType::Trp
};

terminal_stream(
TerminalStreamSocketImpl(socket),
streaming_file,
Expand Down Expand Up @@ -192,3 +185,71 @@ async fn setup_webm_streaming(
}
}
}

#[cfg(test)]
mod tests {
use super::*;

#[tokio::test]
async fn validates_streaming_behavior_from_file_extension() {
let webm_type = validate_streaming_file(camino::Utf8Path::new("recording-0.webm"))
.await
.expect("webm should be accepted");
assert!(matches!(webm_type, StreamingType::WebM));

let cast_type = validate_streaming_file(camino::Utf8Path::new("recording-0.cast"))
.await
.expect("cast should be accepted");
assert!(matches!(
cast_type,
StreamingType::Terminal(terminal_streamer::InputStreamType::Asciinema)
));

let trp_type = validate_streaming_file(camino::Utf8Path::new("recording-0.trp"))
.await
.expect("trp should be accepted");
assert!(matches!(
trp_type,
StreamingType::Terminal(terminal_streamer::InputStreamType::Trp)
));

assert!(
validate_streaming_file(camino::Utf8Path::new("recording-0.slog"))
.await
.is_err(),
"slog should be rejected for streaming"
);
assert!(
validate_streaming_file(camino::Utf8Path::new("recording-0.bin"))
.await
.is_err(),
"unknown extension should be rejected"
);
assert!(
validate_streaming_file(camino::Utf8Path::new("recording-0"))
.await
.is_err(),
"missing extension should be rejected"
);
Comment thread
kristahouse marked this conversation as resolved.
}

#[test]
fn maps_recording_file_type_to_streaming_type() {
let asciicast_type =
streaming_type_for_file_type(RecordingFileType::Asciicast).expect("asciicast should stream in terminal");
assert!(matches!(
asciicast_type,
StreamingType::Terminal(terminal_streamer::InputStreamType::Asciinema)
));

let trp_type = streaming_type_for_file_type(RecordingFileType::TRP).expect("trp should stream in terminal");
assert!(matches!(
trp_type,
StreamingType::Terminal(terminal_streamer::InputStreamType::Trp)
));

let webm_type = streaming_type_for_file_type(RecordingFileType::WebM).expect("webm should stream as video");
assert!(matches!(webm_type, StreamingType::WebM));
assert!(streaming_type_for_file_type(RecordingFileType::SessionRecordingLog).is_err());
}
}
25 changes: 22 additions & 3 deletions devolutions-gateway/src/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,9 @@ pub enum RecordingFileType {
}

impl RecordingFileType {
pub const WEBM_CONTENT_TYPE: &'static str = "video/webm";
pub const TRP_CONTENT_TYPE: &'static str = "application/octet-stream";
pub const ASCIICAST_CONTENT_TYPE: &'static str = "application/x-asciicast";
pub const SLOG_CONTENT_TYPE: &'static str = "application/x-ndjson";

pub const fn format_name(self) -> &'static str {
Expand Down Expand Up @@ -316,10 +319,12 @@ impl RecordingFileType {
}
}

pub const fn content_type(self) -> Option<&'static str> {
pub const fn content_type(self) -> &'static str {
match self {
RecordingFileType::SessionRecordingLog => Some(Self::SLOG_CONTENT_TYPE),
RecordingFileType::WebM | RecordingFileType::TRP | RecordingFileType::Asciicast => None,
RecordingFileType::WebM => Self::WEBM_CONTENT_TYPE,
RecordingFileType::TRP => Self::TRP_CONTENT_TYPE,
RecordingFileType::Asciicast => Self::ASCIICAST_CONTENT_TYPE,
RecordingFileType::SessionRecordingLog => Self::SLOG_CONTENT_TYPE,
}
}
}
Expand Down Expand Up @@ -1890,4 +1895,18 @@ mod tests {
assert_ne!(claims.jti, Uuid::nil());
assert!(matches!(claims.destination, KdcDestination::Inject { .. }));
}

#[test]
fn recording_file_types_have_concrete_content_types() {
let expected = [
(RecordingFileType::WebM, "video/webm"),
(RecordingFileType::TRP, "application/octet-stream"),
(RecordingFileType::Asciicast, "application/x-asciicast"),
(RecordingFileType::SessionRecordingLog, "application/x-ndjson"),
];

for (recording_file_type, expected_content_type) in expected {
assert_eq!(recording_file_type.content_type(), expected_content_type);
}
}
}
Loading