From fb1c0e31c809b814a8c0dd459e63d27c7a66035d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Nov 2026 20:21:35 +0000 Subject: [PATCH 1/3] Render RFD PDFs in Asciidoctor safe mode with a minimal environment The asciidoctor-pdf command line defaults to the unsafe safe mode, so RFD content could include:: arbitrary files (absolute or parent-relative paths) or point pdf-theme at a YAML file elsewhere on disk and have its contents embedded in the published PDF. The renderer also inherited the processor's full environment, including any configuration supplied through it. Run asciidoctor-pdf with `-S safe` and the render directory as the base dir so includes and relative images are jailed to it, lock the pdf-theme, pdf-themesdir and pdf-fontsdir attributes (asciidoctor-pdf does not jail them) so documents cannot override them, and pass the child only the environment variables needed to locate Ruby, Node and Chromium. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Q5Ej2cHBc7wgaksc9BPkai --- rfd-processor/src/content/asciidoc.rs | 66 +++++++++++++++++++++------ 1 file changed, 51 insertions(+), 15 deletions(-) diff --git a/rfd-processor/src/content/asciidoc.rs b/rfd-processor/src/content/asciidoc.rs index a5f9e740..5e72c323 100644 --- a/rfd-processor/src/content/asciidoc.rs +++ b/rfd-processor/src/content/asciidoc.rs @@ -48,22 +48,58 @@ async fn render_pdf( .await?; write_file(&shim_path, MERMAID_CONFIG_SHIM.as_bytes()).await?; + // RFD content is untrusted. The CLI defaults to the unsafe safe mode, so run + // in safe mode jailed to the content directory, and lock the theme and font + // attributes (which asciidoctor-pdf does not jail) so the document cannot + // point them elsewhere on disk. The renderer also only receives the + // environment it needs to locate Ruby, Node and Chromium. let mut command = Command::new("asciidoctor-pdf"); - command.current_dir(content_dir.clone()).args([ - "-o", - "-", - "-r", - "base64", - "-r", - "asciidoctor-mermaid/pdf", - "-r", - shim_path.to_str().unwrap(), - "-a", - "reproducible", - "-a", - "source-highlighter=rouge", - file_path.to_str().unwrap(), - ]); + command + .current_dir(content_dir.clone()) + .env_clear() + .envs(std::env::vars_os().filter(|(key, _)| { + key.to_str().is_some_and(|key| { + ["PATH", "HOME", "TMPDIR", "LANG", "TZ"].contains(&key) + || [ + "LC_", + "GEM_", + "BUNDLE_", + "RUBY", + "NODE_", + "PUPPETEER_", + "XDG_", + ] + .iter() + .any(|prefix| key.starts_with(prefix)) + }) + })) + .args([ + "-S", + "safe", + "-B", + content_dir.to_str().unwrap(), + "-a", + "pdf-theme!", + "-a", + "pdf-themesdir!", + "-a", + "pdf-fontsdir!", + ]) + .args([ + "-o", + "-", + "-r", + "base64", + "-r", + "asciidoctor-mermaid/pdf", + "-r", + shim_path.to_str().unwrap(), + "-a", + "reproducible", + "-a", + "source-highlighter=rouge", + file_path.to_str().unwrap(), + ]); let cmd_output = tokio::task::spawn_blocking(move || command.output()).await?; From 7f983e0fca63ec9e97df13b7505fa95cf97c0439 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Nov 2026 20:29:09 +0000 Subject: [PATCH 2/3] Keep downloaded supporting documents inside the render directory download_supporting_documents built each local path by removing every occurrence of `rfd/NNNN` from the repository path with str::replace. Neighbouring components could collapse into `..` (for example `rfd/0123/.rfd/0123./evil` became `../evil`), letting an RFD committer write arbitrary files outside the per-render temp directory when the UpdatePdfs action ran. Strip only the leading RFD directory, component by component, and skip any document whose remaining path is empty or contains anything other than normal components, so the joined path always stays under the render directory. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Q5Ej2cHBc7wgaksc9BPkai --- rfd-processor/src/content/mod.rs | 83 ++++++++++++++++++++++++++++---- 1 file changed, 74 insertions(+), 9 deletions(-) diff --git a/rfd-processor/src/content/mod.rs b/rfd-processor/src/content/mod.rs index 4ed05e41..056beaca 100644 --- a/rfd-processor/src/content/mod.rs +++ b/rfd-processor/src/content/mod.rs @@ -2,7 +2,13 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -use std::{borrow::Cow, env, io, path::PathBuf, str::Utf8Error, string::FromUtf8Error}; +use std::{ + borrow::Cow, + env, io, + path::{Component, Path, PathBuf}, + str::Utf8Error, + string::FromUtf8Error, +}; use async_trait::async_trait; use base64::DecodeError; @@ -149,14 +155,15 @@ impl<'a> RenderableRfd<'a> { .await?; for document in documents { - let document_path = storage_path.join( - document - .path - .replace(dir.trim_start_matches('/'), "") - .trim_start_matches('/'), - ); - - let path = document_path; + let Some(relative_path) = supporting_document_path(&dir, &document.path) else { + tracing::warn!( + document.path, + "Skipping supporting document with unsafe path" + ); + continue; + }; + + let path = storage_path.join(relative_path); write_file(&path, &decode_base64(&document.content)?).await?; tracing::info!(?path, "Wrote supporting document",); @@ -194,6 +201,19 @@ impl<'a> RenderableRfd<'a> { } } +/// Path of a supporting document relative to the RFD directory `dir`. Returns `None` unless the +/// repository path lies under `dir` and the remainder is made up only of normal components, so the +/// result always stays inside the render directory it is joined onto. +fn supporting_document_path<'a>(dir: &str, repo_path: &'a str) -> Option<&'a Path> { + let relative = Path::new(repo_path).strip_prefix(dir).ok()?; + let is_contained = relative.components().next().is_some() + && relative + .components() + .all(|component| matches!(component, Component::Normal(_))); + + is_contained.then_some(relative) +} + impl<'a> RfdDocument for RenderableRfd<'a> { fn get_title(&self) -> Option<&str> { RfdDocument::get_title(&self.content) @@ -287,3 +307,48 @@ pub enum RfdOutputError { #[error(transparent)] Io(#[from] std::io::Error), } + +#[cfg(test)] +mod tests { + use std::path::Path; + + use super::supporting_document_path; + + #[test] + fn test_supporting_document_path_keeps_rfd_relative_path() { + assert_eq!( + supporting_document_path("rfd/0123", "rfd/0123/example.csv"), + Some(Path::new("example.csv")) + ); + assert_eq!( + supporting_document_path("rfd/0123", "rfd/0123/images/rfd/0123/diagram.svg"), + Some(Path::new("images/rfd/0123/diagram.svg")) + ); + } + + #[test] + fn test_supporting_document_path_rejects_escaping_paths() { + // Removing every occurrence of `rfd/0123` would turn this into `../../evil`. + let spliced = "rfd/0123/.rfd/0123./.rfd/0123./evil"; + assert_eq!( + spliced.replace("rfd/0123", "").trim_start_matches('/'), + "../../evil" + ); + assert_eq!( + supporting_document_path("rfd/0123", spliced), + Some(Path::new(".rfd/0123./.rfd/0123./evil")) + ); + + assert_eq!( + supporting_document_path("rfd/0123", "rfd/0123/../evil"), + None + ); + assert_eq!( + supporting_document_path("rfd/0123", "rfd/0123/a/../../evil"), + None + ); + assert_eq!(supporting_document_path("rfd/0123", "/etc/passwd"), None); + assert_eq!(supporting_document_path("rfd/0123", "rfd/0124/evil"), None); + assert_eq!(supporting_document_path("rfd/0123", "rfd/0123"), None); + } +} From 16ef6436915ac3f0ef9afd2bedfb85fec35326ca Mon Sep 17 00:00:00 2001 From: Peter Tripp Date: Wed, 11 Nov 2026 15:41:54 -0500 Subject: [PATCH 3/3] clarity --- rfd-processor/src/content/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rfd-processor/src/content/mod.rs b/rfd-processor/src/content/mod.rs index 056beaca..c937218f 100644 --- a/rfd-processor/src/content/mod.rs +++ b/rfd-processor/src/content/mod.rs @@ -201,12 +201,12 @@ impl<'a> RenderableRfd<'a> { } } -/// Path of a supporting document relative to the RFD directory `dir`. Returns `None` unless the -/// repository path lies under `dir` and the remainder is made up only of normal components, so the -/// result always stays inside the render directory it is joined onto. +/// Returns `repo_path` relative to the RFD directory `dir`, +/// or `None` if it is not strictly inside `dir` (e.g. it contains `..`). +/// The result is safe to join onto the render directory. fn supporting_document_path<'a>(dir: &str, repo_path: &'a str) -> Option<&'a Path> { let relative = Path::new(repo_path).strip_prefix(dir).ok()?; - let is_contained = relative.components().next().is_some() + let is_contained = !relative.as_os_str().is_empty() && relative .components() .all(|component| matches!(component, Component::Normal(_)));