From 6ad814d1a24bca2a87ea99787ffed7a704110436 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sun, 28 Sep 2025 18:47:45 +0100 Subject: [PATCH 01/17] Extract process helpers from runner module --- src/runner.rs | 65 ++++--------------------------------- src/runner/process.rs | 75 +++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 79 insertions(+), 61 deletions(-) diff --git a/src/runner.rs b/src/runner.rs index 59af4204..f73fc6f6 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -9,9 +9,8 @@ use crate::{ir::BuildGraph, manifest, ninja_gen}; use anyhow::{Context, Result}; use serde_json; use std::borrow::Cow; -use std::fs; -use std::path::{Path, PathBuf}; -use tempfile::{Builder, NamedTempFile}; +use std::path::Path; +use tempfile::NamedTempFile; use tracing::{debug, info}; /// Default Ninja executable to invoke. @@ -85,7 +84,7 @@ pub fn run(cli: &Cli) -> Result<()> { Commands::Build(args) => handle_build(cli, &args), Commands::Manifest { file } => { let ninja = generate_ninja(cli)?; - write_ninja_file(&file, &ninja)?; + process::write_ninja_file(&file, &ninja)?; Ok(()) } Commands::Clean => { @@ -123,10 +122,10 @@ fn handle_build(cli: &Cli, args: &BuildArgs) -> Result<()> { let build_path: Cow; let mut tmp_file: Option = None; if let Some(path) = &args.emit { - write_ninja_file(path, &ninja)?; + process::write_ninja_file(path, &ninja)?; build_path = Cow::Borrowed(path.as_path()); } else { - let tmp = create_temp_ninja_file(&ninja)?; + let tmp = process::create_temp_ninja_file(&ninja)?; tmp_file = Some(tmp); build_path = Cow::Borrowed( tmp_file @@ -136,7 +135,7 @@ fn handle_build(cli: &Cli, args: &BuildArgs) -> Result<()> { ); } - let program = resolve_ninja_program(); + let program = process::resolve_ninja_program(); run_ninja(program.as_path(), cli, build_path.as_ref(), &targets).with_context(|| { format!( "running {} with build file {}", @@ -148,52 +147,6 @@ fn handle_build(cli: &Cli, args: &BuildArgs) -> Result<()> { Ok(()) } -/// Create a temporary Ninja file on disk containing `content`. -/// -/// # Errors -/// -/// Returns an error if the file cannot be created or written. -/// -/// # Examples -/// ```ignore -/// use netsuke::runner::{create_temp_ninja_file, NinjaContent}; -/// let tmp = create_temp_ninja_file(&NinjaContent::new("".into())).unwrap(); -/// assert!(tmp.path().to_string_lossy().ends_with(".ninja")); -/// ``` -fn create_temp_ninja_file(content: &NinjaContent) -> Result { - let tmp = Builder::new() - .prefix("netsuke.") - .suffix(".ninja") - .tempfile() - .context("create temp file")?; - write_ninja_file(tmp.path(), content)?; - Ok(tmp) -} - -/// Write `content` to `path` and log the file's location. -/// -/// # Errors -/// -/// Returns an error if the file cannot be written. -/// -/// # Examples -/// ```ignore -/// let content = NinjaContent::new("rule cc\n".to_string()); -/// write_ninja_file(Path::new("out.ninja"), &content).unwrap(); -/// ``` -fn write_ninja_file(path: &Path, content: &NinjaContent) -> Result<()> { - // Ensure the parent directory exists; guard against empty components so we - // do not attempt to create the current directory on some platforms. - if let Some(parent) = path.parent().filter(|p| !p.as_os_str().is_empty()) { - fs::create_dir_all(parent) - .with_context(|| format!("failed to create parent directory {}", parent.display()))?; - } - fs::write(path, content.as_str()) - .with_context(|| format!("failed to write Ninja file to {}", path.display()))?; - info!("Generated Ninja file at {}", path.display()); - Ok(()) -} - /// Generate the Ninja manifest string from the Netsuke manifest referenced by `cli`. /// /// # Errors @@ -242,9 +195,3 @@ fn resolve_manifest_path(cli: &Cli) -> std::path::PathBuf { .as_ref() .map_or_else(|| cli.file.clone(), |dir| dir.join(&cli.file)) } - -/// Determine which Ninja executable to invoke. -#[must_use] -fn resolve_ninja_program() -> PathBuf { - std::env::var_os(NINJA_ENV).map_or_else(|| PathBuf::from(NINJA_PROGRAM), PathBuf::from) -} diff --git a/src/runner/process.rs b/src/runner/process.rs index dbb4216d..c0ded5dc 100644 --- a/src/runner/process.rs +++ b/src/runner/process.rs @@ -1,9 +1,13 @@ -use super::BuildTargets; +use super::{BuildTargets, NinjaContent, NINJA_PROGRAM}; use crate::cli::Cli; +use anyhow::{Context, Result as AnyResult}; +use ninja_env::NINJA_ENV; +use tempfile::{Builder, NamedTempFile}; use std::{ + env, fs, io::{self, BufRead, BufReader, Write}, - path::Path, + path::{Path, PathBuf}, process::{Command, Stdio}, thread, }; @@ -44,6 +48,19 @@ pub mod doc { pub fn redact_sensitive_args(args: &[CommandArg]) -> Vec { super::redact_sensitive_args(args) } + + pub fn create_temp_ninja_file( + content: &super::NinjaContent, + ) -> super::AnyResult { + super::create_temp_ninja_file(content) + } + + pub fn write_ninja_file( + path: &super::Path, + content: &super::NinjaContent, + ) -> super::AnyResult<()> { + super::write_ninja_file(path, content) + } } /// Check if `arg` contains a sensitive keyword. @@ -112,6 +129,60 @@ pub(crate) fn redact_sensitive_args(args: &[CommandArg]) -> Vec { args.iter().map(redact_argument).collect() } +/// Create a temporary Ninja file on disk containing `content`. +/// +/// # Errors +/// +/// Returns an error if the file cannot be created or written. +/// +/// # Examples +/// ```ignore +/// use netsuke::runner::doc::create_temp_ninja_file; +/// use netsuke::runner::NinjaContent; +/// let tmp = create_temp_ninja_file(&NinjaContent::new("".into())).unwrap(); +/// assert!(tmp.path().to_string_lossy().ends_with(".ninja")); +/// ``` +pub(super) fn create_temp_ninja_file(content: &NinjaContent) -> AnyResult { + let tmp = Builder::new() + .prefix("netsuke.") + .suffix(".ninja") + .tempfile() + .context("create temp file")?; + write_ninja_file(tmp.path(), content)?; + Ok(tmp) +} + +/// Write `content` to `path` and log the file's location. +/// +/// # Errors +/// +/// Returns an error if the file cannot be written. +/// +/// # Examples +/// ```ignore +/// use std::path::Path; +/// use netsuke::runner::doc::write_ninja_file; +/// use netsuke::runner::NinjaContent; +/// let content = NinjaContent::new("rule cc\n".to_string()); +/// write_ninja_file(Path::new("out.ninja"), &content).unwrap(); +/// ``` +pub(super) fn write_ninja_file(path: &Path, content: &NinjaContent) -> AnyResult<()> { + if let Some(parent) = path.parent().filter(|p| !p.as_os_str().is_empty()) { + fs::create_dir_all(parent) + .with_context(|| format!("failed to create parent directory {}", parent.display()))?; + } + fs::write(path, content.as_str()) + .with_context(|| format!("failed to write Ninja file to {}", path.display()))?; + info!("Generated Ninja file at {}", path.display()); + Ok(()) +} + +/// Determine which Ninja executable to invoke. +#[must_use] +pub(super) fn resolve_ninja_program() -> PathBuf { + env::var_os(NINJA_ENV).map_or_else(|| PathBuf::from(NINJA_PROGRAM), PathBuf::from) +} + /// Invoke the Ninja executable with the provided CLI settings. /// /// The function forwards the job count and working directory to Ninja, From 000ccfd7180d1d505f35cbe1db2f96ac58e46de9 Mon Sep 17 00:00:00 2001 From: Leynos Date: Mon, 29 Sep 2025 11:14:10 +0100 Subject: [PATCH 02/17] Refine runner process doc helpers Replace the doc module's forwarding helpers with direct re-exports and make the underlying functions visible for doctests. Add unit and behavioural tests that exercise the Ninja file helpers directly to cover the new process utilities. --- src/runner/process.rs | 88 ++++++++++++++++++++++--------------------- tests/runner_tests.rs | 25 +++++++++++- 2 files changed, 69 insertions(+), 44 deletions(-) diff --git a/src/runner/process.rs b/src/runner/process.rs index c0ded5dc..d20f87bc 100644 --- a/src/runner/process.rs +++ b/src/runner/process.rs @@ -1,16 +1,15 @@ -use super::{BuildTargets, NinjaContent, NINJA_PROGRAM}; +use super::{BuildTargets, NINJA_PROGRAM, NinjaContent}; use crate::cli::Cli; use anyhow::{Context, Result as AnyResult}; use ninja_env::NINJA_ENV; -use tempfile::{Builder, NamedTempFile}; use std::{ - env, - fs, + env, fs, io::{self, BufRead, BufReader, Write}, path::{Path, PathBuf}, process::{Command, Stdio}, thread, }; +use tempfile::{Builder, NamedTempFile}; use tracing::info; #[derive(Debug, Clone)] @@ -30,37 +29,10 @@ impl CommandArg { // testing surface without exporting them in release builds. #[doc(hidden)] pub mod doc { - pub use super::CommandArg; - - #[must_use] - pub fn contains_sensitive_keyword(arg: &CommandArg) -> bool { - super::contains_sensitive_keyword(arg) - } - #[must_use] - pub fn is_sensitive_arg(arg: &CommandArg) -> bool { - super::is_sensitive_arg(arg) - } - #[must_use] - pub fn redact_argument(arg: &CommandArg) -> CommandArg { - super::redact_argument(arg) - } - #[must_use] - pub fn redact_sensitive_args(args: &[CommandArg]) -> Vec { - super::redact_sensitive_args(args) - } - - pub fn create_temp_ninja_file( - content: &super::NinjaContent, - ) -> super::AnyResult { - super::create_temp_ninja_file(content) - } - - pub fn write_ninja_file( - path: &super::Path, - content: &super::NinjaContent, - ) -> super::AnyResult<()> { - super::write_ninja_file(path, content) - } + pub use super::{ + CommandArg, contains_sensitive_keyword, create_temp_ninja_file, is_sensitive_arg, + redact_argument, redact_sensitive_args, resolve_ninja_program, write_ninja_file, + }; } /// Check if `arg` contains a sensitive keyword. @@ -71,12 +43,12 @@ pub mod doc { /// assert!(contains_sensitive_keyword(&CommandArg::new("token=abc".into()))); /// assert!(!contains_sensitive_keyword(&CommandArg::new("path=/tmp".into()))); /// ``` -pub(crate) fn contains_sensitive_keyword(arg: &CommandArg) -> bool { +#[must_use] +pub fn contains_sensitive_keyword(arg: &CommandArg) -> bool { let lower = arg.as_str().to_lowercase(); lower.contains("password") || lower.contains("token") || lower.contains("secret") } -/// Determine whether the argument should be redacted. /// Determine whether the argument should be redacted. /// /// # Examples @@ -85,7 +57,8 @@ pub(crate) fn contains_sensitive_keyword(arg: &CommandArg) -> bool { /// assert!(is_sensitive_arg(&CommandArg::new("password=123".into()))); /// assert!(!is_sensitive_arg(&CommandArg::new("file=readme".into()))); /// ``` -pub(crate) fn is_sensitive_arg(arg: &CommandArg) -> bool { +#[must_use] +pub fn is_sensitive_arg(arg: &CommandArg) -> bool { contains_sensitive_keyword(arg) } @@ -101,7 +74,8 @@ pub(crate) fn is_sensitive_arg(arg: &CommandArg) -> bool { /// let arg = CommandArg::new("path=/tmp".into()); /// assert_eq!(redact_argument(&arg).as_str(), "path=/tmp"); /// ``` -pub(crate) fn redact_argument(arg: &CommandArg) -> CommandArg { +#[must_use] +pub fn redact_argument(arg: &CommandArg) -> CommandArg { if is_sensitive_arg(arg) { let redacted = arg.as_str().split_once('=').map_or_else( || "***REDACTED***".to_string(), @@ -125,7 +99,8 @@ pub(crate) fn redact_argument(arg: &CommandArg) -> CommandArg { /// let redacted = redact_sensitive_args(&args); /// assert_eq!(redacted[1].as_str(), "token=***REDACTED***"); /// ``` -pub(crate) fn redact_sensitive_args(args: &[CommandArg]) -> Vec { +#[must_use] +pub fn redact_sensitive_args(args: &[CommandArg]) -> Vec { args.iter().map(redact_argument).collect() } @@ -142,7 +117,7 @@ pub(crate) fn redact_sensitive_args(args: &[CommandArg]) -> Vec { /// let tmp = create_temp_ninja_file(&NinjaContent::new("".into())).unwrap(); /// assert!(tmp.path().to_string_lossy().ends_with(".ninja")); /// ``` -pub(super) fn create_temp_ninja_file(content: &NinjaContent) -> AnyResult { +pub fn create_temp_ninja_file(content: &NinjaContent) -> AnyResult { let tmp = Builder::new() .prefix("netsuke.") .suffix(".ninja") @@ -166,7 +141,7 @@ pub(super) fn create_temp_ninja_file(content: &NinjaContent) -> AnyResult AnyResult<()> { +pub fn write_ninja_file(path: &Path, content: &NinjaContent) -> AnyResult<()> { if let Some(parent) = path.parent().filter(|p| !p.as_os_str().is_empty()) { fs::create_dir_all(parent) .with_context(|| format!("failed to create parent directory {}", parent.display()))?; @@ -179,7 +154,7 @@ pub(super) fn write_ninja_file(path: &Path, content: &NinjaContent) -> AnyResult /// Determine which Ninja executable to invoke. #[must_use] -pub(super) fn resolve_ninja_program() -> PathBuf { +pub fn resolve_ninja_program() -> PathBuf { env::var_os(NINJA_ENV).map_or_else(|| PathBuf::from(NINJA_PROGRAM), PathBuf::from) } @@ -264,3 +239,30 @@ pub fn run_ninja( )) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn create_temp_ninja_file_persists_contents() { + let content = NinjaContent::new(String::from("rule cc")); + let file = create_temp_ninja_file(&content).expect("create temp file"); + let written = std::fs::read_to_string(file.path()).expect("read temp file"); + assert_eq!(written, content.as_str()); + assert!(file.path().to_string_lossy().ends_with(".ninja")); + } + + #[test] + fn write_ninja_file_creates_parent_directories() { + let temp = tempfile::tempdir().expect("create temp dir"); + let nested = temp.path().join("nested").join("build.ninja"); + let content = NinjaContent::new(String::from("build all: phony")); + + write_ninja_file(&nested, &content).expect("write ninja file"); + + let written = std::fs::read_to_string(&nested).expect("read nested file"); + assert_eq!(written, content.as_str()); + assert!(nested.parent().expect("parent path").exists()); + } +} diff --git a/tests/runner_tests.rs b/tests/runner_tests.rs index 347e8ab7..99e5b308 100644 --- a/tests/runner_tests.rs +++ b/tests/runner_tests.rs @@ -1,5 +1,5 @@ use netsuke::cli::{BuildArgs, Cli, Commands}; -use netsuke::runner::{BuildTargets, run, run_ninja}; +use netsuke::runner::{BuildTargets, NinjaContent, doc, run, run_ninja}; use rstest::{fixture, rstest}; use serial_test::serial; use std::path::{Path, PathBuf}; @@ -51,6 +51,29 @@ fn test_manifest() -> (tempfile::TempDir, PathBuf) { (temp, manifest_path) } +#[test] +fn create_temp_ninja_file_writes_contents() { + let content = NinjaContent::new(String::from("rule cc")); + let file = doc::create_temp_ninja_file(&content).expect("create temp file"); + + let written = std::fs::read_to_string(file.path()).expect("read temp file"); + assert_eq!(written, content.as_str()); + assert!(file.path().to_string_lossy().ends_with(".ninja")); +} + +#[test] +fn write_ninja_file_creates_directories() { + let temp = tempfile::tempdir().expect("create temp dir"); + let nested = temp.path().join("nested").join("build.ninja"); + let content = NinjaContent::new(String::from("build all: phony")); + + doc::write_ninja_file(&nested, &content).expect("write ninja file"); + + let written = std::fs::read_to_string(&nested).expect("read nested file"); + assert_eq!(written, content.as_str()); + assert!(nested.parent().expect("parent path").exists()); +} + #[test] fn run_exits_with_manifest_error_on_invalid_version() { let temp = tempfile::tempdir().expect("temp dir"); From 8e193d9e6630342980f15c730ef9ec8202e537c8 Mon Sep 17 00:00:00 2001 From: Leynos Date: Mon, 29 Sep 2025 11:14:16 +0100 Subject: [PATCH 03/17] Document runner process refactor Detail that process orchestration now lives in src/runner/process.rs so the design guide matches the current module layout. --- docs/netsuke-design.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index ef66bdc7..9f8e6ca6 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -1598,10 +1598,13 @@ The CLI is implemented using clap's derive API in `src/cli.rs`. Clap's `default_value_t` attribute marks `Build` as the default subcommand, so invoking `netsuke` with no explicit command still triggers a build. CLI execution and dispatch live in `src/runner.rs`, keeping `main.rs` focused on -parsing. The working directory flag mirrors Ninja's `-C` option but is resolved -internally; Netsuke changes directory before spawning Ninja rather than -forwarding the flag. Error scenarios are validated using clap's `ErrorKind` -enumeration in unit tests and via Cucumber steps for behavioural coverage. +parsing. Process management, Ninja invocation, argument redaction, and the +temporary file helpers reside in `src/runner/process.rs`, allowing the runner +entry point to delegate low-level concerns. The working directory flag mirrors +Ninja's `-C` option but is resolved internally; Netsuke changes directory before +spawning Ninja rather than forwarding the flag. Error scenarios are validated +using clap's `ErrorKind` enumeration in unit tests and via Cucumber steps for +behavioural coverage. ### 8.5 Manual Pages From 31761a9a20c04909bba8bd094911f5d333e9b26f Mon Sep 17 00:00:00 2001 From: Leynos Date: Mon, 29 Sep 2025 13:20:12 +0100 Subject: [PATCH 04/17] Gate runner doc helpers for tests --- src/runner.rs | 15 +++++---------- src/runner/process.rs | 2 +- tests/runner_tests.rs | 25 +------------------------ 3 files changed, 7 insertions(+), 35 deletions(-) diff --git a/src/runner.rs b/src/runner.rs index f73fc6f6..53e33789 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -19,7 +19,7 @@ pub const NINJA_PROGRAM: &str = "ninja"; pub use ninja_env::NINJA_ENV; mod process; -#[doc(hidden)] +#[cfg(any(test, doctest))] pub use process::doc; pub use process::run_ninja; @@ -120,19 +120,15 @@ fn handle_build(cli: &Cli, args: &BuildArgs) -> Result<()> { // duration of the Ninja invocation. Borrow the emitted path when provided // to avoid unnecessary allocation. let build_path: Cow; - let mut tmp_file: Option = None; + let _tmp_file_guard: Option; if let Some(path) = &args.emit { process::write_ninja_file(path, &ninja)?; build_path = Cow::Borrowed(path.as_path()); + _tmp_file_guard = None; } else { let tmp = process::create_temp_ninja_file(&ninja)?; - tmp_file = Some(tmp); - build_path = Cow::Borrowed( - tmp_file - .as_ref() - .expect("temporary Ninja file should exist") - .path(), - ); + build_path = Cow::Owned(tmp.path().to_path_buf()); + _tmp_file_guard = Some(tmp); } let program = process::resolve_ninja_program(); @@ -143,7 +139,6 @@ fn handle_build(cli: &Cli, args: &BuildArgs) -> Result<()> { build_path.display() ) })?; - drop(tmp_file); Ok(()) } diff --git a/src/runner/process.rs b/src/runner/process.rs index d20f87bc..307869bb 100644 --- a/src/runner/process.rs +++ b/src/runner/process.rs @@ -27,7 +27,7 @@ impl CommandArg { // Public helpers for doctests only. This exposes internal helpers as a stable // testing surface without exporting them in release builds. -#[doc(hidden)] +#[cfg(any(test, doctest))] pub mod doc { pub use super::{ CommandArg, contains_sensitive_keyword, create_temp_ninja_file, is_sensitive_arg, diff --git a/tests/runner_tests.rs b/tests/runner_tests.rs index 99e5b308..347e8ab7 100644 --- a/tests/runner_tests.rs +++ b/tests/runner_tests.rs @@ -1,5 +1,5 @@ use netsuke::cli::{BuildArgs, Cli, Commands}; -use netsuke::runner::{BuildTargets, NinjaContent, doc, run, run_ninja}; +use netsuke::runner::{BuildTargets, run, run_ninja}; use rstest::{fixture, rstest}; use serial_test::serial; use std::path::{Path, PathBuf}; @@ -51,29 +51,6 @@ fn test_manifest() -> (tempfile::TempDir, PathBuf) { (temp, manifest_path) } -#[test] -fn create_temp_ninja_file_writes_contents() { - let content = NinjaContent::new(String::from("rule cc")); - let file = doc::create_temp_ninja_file(&content).expect("create temp file"); - - let written = std::fs::read_to_string(file.path()).expect("read temp file"); - assert_eq!(written, content.as_str()); - assert!(file.path().to_string_lossy().ends_with(".ninja")); -} - -#[test] -fn write_ninja_file_creates_directories() { - let temp = tempfile::tempdir().expect("create temp dir"); - let nested = temp.path().join("nested").join("build.ninja"); - let content = NinjaContent::new(String::from("build all: phony")); - - doc::write_ninja_file(&nested, &content).expect("write ninja file"); - - let written = std::fs::read_to_string(&nested).expect("read nested file"); - assert_eq!(written, content.as_str()); - assert!(nested.parent().expect("parent path").exists()); -} - #[test] fn run_exits_with_manifest_error_on_invalid_version() { let temp = tempfile::tempdir().expect("temp dir"); From 1afb52cc6ec79ef79fc08e05f6b8a9c1e61ad0f8 Mon Sep 17 00:00:00 2001 From: Leynos Date: Mon, 29 Sep 2025 13:20:31 +0100 Subject: [PATCH 05/17] Refine runner redaction and file helpers Tighten the redaction utilities to only match key=value secrets, add capability-based Ninja file writers, and cover the behaviour with focused tests. Update the runner design doc wrapping changed by fmt. --- docs/netsuke-design.md | 8 +-- src/runner/process.rs | 152 +++++++++++++++++++++++++++++++++++------ 2 files changed, 136 insertions(+), 24 deletions(-) diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index 9f8e6ca6..9a88bca9 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -1601,10 +1601,10 @@ execution and dispatch live in `src/runner.rs`, keeping `main.rs` focused on parsing. Process management, Ninja invocation, argument redaction, and the temporary file helpers reside in `src/runner/process.rs`, allowing the runner entry point to delegate low-level concerns. The working directory flag mirrors -Ninja's `-C` option but is resolved internally; Netsuke changes directory before -spawning Ninja rather than forwarding the flag. Error scenarios are validated -using clap's `ErrorKind` enumeration in unit tests and via Cucumber steps for -behavioural coverage. +Ninja's `-C` option but is resolved internally; Netsuke changes directory +before spawning Ninja rather than forwarding the flag. Error scenarios are +validated using clap's `ErrorKind` enumeration in unit tests and via Cucumber +steps for behavioural coverage. ### 8.5 Manual Pages diff --git a/src/runner/process.rs b/src/runner/process.rs index 307869bb..7c785990 100644 --- a/src/runner/process.rs +++ b/src/runner/process.rs @@ -1,6 +1,8 @@ use super::{BuildTargets, NINJA_PROGRAM, NinjaContent}; use crate::cli::Cli; -use anyhow::{Context, Result as AnyResult}; +use anyhow::{Context, Result as AnyResult, anyhow}; +use camino::{Utf8Path, Utf8PathBuf}; +use cap_std::{ambient_authority, fs as cap_fs}; use ninja_env::NINJA_ENV; use std::{ env, fs, @@ -31,10 +33,26 @@ impl CommandArg { pub mod doc { pub use super::{ CommandArg, contains_sensitive_keyword, create_temp_ninja_file, is_sensitive_arg, - redact_argument, redact_sensitive_args, resolve_ninja_program, write_ninja_file, + redact_argument, redact_sensitive_args, resolve_ninja_program, resolve_ninja_program_utf8, + write_ninja_file, write_ninja_file_utf8, }; } +fn is_sensitive_key(key: &str) -> bool { + const SENSITIVE_KEYS: [&str; 7] = [ + "password", + "token", + "secret", + "api_key", + "apikey", + "auth", + "authorization", + ]; + SENSITIVE_KEYS + .iter() + .any(|candidate| key.eq_ignore_ascii_case(candidate)) +} + /// Check if `arg` contains a sensitive keyword. /// /// # Examples @@ -45,8 +63,9 @@ pub mod doc { /// ``` #[must_use] pub fn contains_sensitive_keyword(arg: &CommandArg) -> bool { - let lower = arg.as_str().to_lowercase(); - lower.contains("password") || lower.contains("token") || lower.contains("secret") + arg.as_str() + .split_once('=') + .is_some_and(|(key, _)| is_sensitive_key(key.trim())) } /// Determine whether the argument should be redacted. @@ -77,14 +96,13 @@ pub fn is_sensitive_arg(arg: &CommandArg) -> bool { #[must_use] pub fn redact_argument(arg: &CommandArg) -> CommandArg { if is_sensitive_arg(arg) { - let redacted = arg.as_str().split_once('=').map_or_else( - || "***REDACTED***".to_string(), - |(key, _)| format!("{key}=***REDACTED***"), - ); - CommandArg::new(redacted) - } else { - arg.clone() + if let Some((key, _)) = arg.as_str().split_once('=') { + let trimmed = key.trim(); + return CommandArg::new(format!("{trimmed}=***REDACTED***")); + } + return CommandArg::new(String::from("***REDACTED***")); } + arg.clone() } /// Redact sensitive information from all `args`. @@ -127,6 +145,49 @@ pub fn create_temp_ninja_file(content: &NinjaContent) -> AnyResult AnyResult<()> { + if let Some(parent) = path.parent().filter(|p| !p.as_str().is_empty()) { + dir.create_dir_all(parent.as_str()) + .with_context(|| format!("failed to create parent directory {parent}"))?; + } + let mut file = dir + .create(path.as_str()) + .with_context(|| format!("failed to create Ninja file at {path}"))?; + file.write_all(content.as_str().as_bytes()) + .with_context(|| format!("failed to write Ninja file to {path}"))?; + Ok(()) +} + +fn derive_dir_and_relative(path: &Utf8Path) -> AnyResult<(cap_fs::Dir, Utf8PathBuf)> { + if path.is_relative() { + let dir = cap_fs::Dir::open_ambient_dir(".", ambient_authority()) + .context("open ambient directory")?; + return Ok((dir, path.to_owned())); + } + + let mut ancestors = path.ancestors(); + ancestors.next(); + let base = ancestors + .find(|candidate| candidate.exists()) + .ok_or_else(|| anyhow!("no existing ancestor for {path}"))?; + let relative = path + .strip_prefix(base) + .context("derive relative Ninja path")? + .to_owned(); + let dir = cap_fs::Dir::open_ambient_dir(base.as_std_path(), ambient_authority()) + .with_context(|| format!("open ancestor directory {base}"))?; + Ok((dir, relative)) +} + /// Write `content` to `path` and log the file's location. /// /// # Errors @@ -142,20 +203,28 @@ pub fn create_temp_ninja_file(content: &NinjaContent) -> AnyResult AnyResult<()> { - if let Some(parent) = path.parent().filter(|p| !p.as_os_str().is_empty()) { - fs::create_dir_all(parent) - .with_context(|| format!("failed to create parent directory {}", parent.display()))?; - } - fs::write(path, content.as_str()) - .with_context(|| format!("failed to write Ninja file to {}", path.display()))?; - info!("Generated Ninja file at {}", path.display()); + let utf8_path = + Utf8Path::from_path(path).ok_or_else(|| anyhow!("non-UTF-8 path is not supported"))?; + let (dir, relative) = derive_dir_and_relative(utf8_path)?; + write_ninja_file_utf8(&dir, &relative, content)?; + info!("Generated Ninja file at {utf8_path}"); Ok(()) } +#[must_use] +pub fn resolve_ninja_program_utf8() -> Utf8PathBuf { + env::var_os(NINJA_ENV) + .and_then(|value| { + let path = PathBuf::from(value); + Utf8PathBuf::from_path_buf(path).ok() + }) + .unwrap_or_else(|| Utf8PathBuf::from(NINJA_PROGRAM)) +} + /// Determine which Ninja executable to invoke. #[must_use] pub fn resolve_ninja_program() -> PathBuf { - env::var_os(NINJA_ENV).map_or_else(|| PathBuf::from(NINJA_PROGRAM), PathBuf::from) + resolve_ninja_program_utf8().into() } /// Invoke the Ninja executable with the provided CLI settings. @@ -244,6 +313,33 @@ pub fn run_ninja( mod tests { use super::*; + #[test] + fn contains_sensitive_keyword_only_flags_known_keys() { + let token = CommandArg::new(String::from("token=abc")); + assert!(contains_sensitive_keyword(&token)); + + let positional = CommandArg::new(String::from("secrets.yml")); + assert!(!contains_sensitive_keyword(&positional)); + + let path_arg = CommandArg::new(String::from("path=/tmp/secrets.yml")); + assert!(!contains_sensitive_keyword(&path_arg)); + + let spaced = CommandArg::new(String::from(" PASSWORD = value ")); + assert!(contains_sensitive_keyword(&spaced)); + } + + #[test] + fn redact_argument_preserves_non_sensitive_pairs() { + let redacted = redact_argument(&CommandArg::new(String::from("auth = token123"))); + assert_eq!(redacted.as_str(), "auth=***REDACTED***"); + + let untouched = redact_argument(&CommandArg::new(String::from("path=/var/secrets"))); + assert_eq!(untouched.as_str(), "path=/var/secrets"); + + let positional = redact_argument(&CommandArg::new(String::from("secret"))); + assert_eq!(positional.as_str(), "secret"); + } + #[test] fn create_temp_ninja_file_persists_contents() { let content = NinjaContent::new(String::from("rule cc")); @@ -254,7 +350,23 @@ mod tests { } #[test] - fn write_ninja_file_creates_parent_directories() { + fn write_ninja_file_utf8_creates_parent_directories() { + let temp = tempfile::tempdir().expect("create temp dir"); + let dir = + cap_fs::Dir::open_ambient_dir(temp.path(), ambient_authority()).expect("open temp dir"); + let nested = Utf8PathBuf::from("nested/build.ninja"); + let content = NinjaContent::new(String::from("build all: phony")); + + write_ninja_file_utf8(&dir, &nested, &content).expect("write ninja file"); + + let nested_path = temp.path().join("nested").join("build.ninja"); + let written = std::fs::read_to_string(&nested_path).expect("read nested file"); + assert_eq!(written, content.as_str()); + assert!(nested_path.parent().expect("parent path").exists()); + } + + #[test] + fn write_ninja_file_handles_absolute_paths() { let temp = tempfile::tempdir().expect("create temp dir"); let nested = temp.path().join("nested").join("build.ninja"); let content = NinjaContent::new(String::from("build all: phony")); From cffc2b6a25f2762ebe7b561860dcec4e6accd8df Mon Sep 17 00:00:00 2001 From: Leynos Date: Mon, 29 Sep 2025 16:58:19 +0100 Subject: [PATCH 06/17] Document process helpers and drop unused import --- src/runner.rs | 1 - src/runner/process.rs | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/runner.rs b/src/runner.rs index 53e33789..589f78ad 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -7,7 +7,6 @@ use crate::cli::{BuildArgs, Cli, Commands}; use crate::{ir::BuildGraph, manifest, ninja_gen}; use anyhow::{Context, Result}; -use serde_json; use std::borrow::Cow; use std::path::Path; use tempfile::NamedTempFile; diff --git a/src/runner/process.rs b/src/runner/process.rs index 7c785990..b5219538 100644 --- a/src/runner/process.rs +++ b/src/runner/process.rs @@ -1,3 +1,5 @@ +//! Process helpers for Ninja file lifecycle, argument redaction, and subprocess I/O. +//! Internal to `runner`; public API is defined in `runner.rs`. use super::{BuildTargets, NINJA_PROGRAM, NinjaContent}; use crate::cli::Cli; use anyhow::{Context, Result as AnyResult, anyhow}; From 387256c79e9f3fb902365f2231bc1e28ce8f080d Mon Sep 17 00:00:00 2001 From: Leynos Date: Mon, 29 Sep 2025 16:58:46 +0100 Subject: [PATCH 07/17] Gate runner doc helpers to doctests --- src/runner.rs | 2 +- src/runner/process.rs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/runner.rs b/src/runner.rs index 589f78ad..e9cdbb5e 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -18,7 +18,7 @@ pub const NINJA_PROGRAM: &str = "ninja"; pub use ninja_env::NINJA_ENV; mod process; -#[cfg(any(test, doctest))] +#[cfg(doctest)] pub use process::doc; pub use process::run_ninja; diff --git a/src/runner/process.rs b/src/runner/process.rs index b5219538..ceb0d68f 100644 --- a/src/runner/process.rs +++ b/src/runner/process.rs @@ -31,7 +31,7 @@ impl CommandArg { // Public helpers for doctests only. This exposes internal helpers as a stable // testing surface without exporting them in release builds. -#[cfg(any(test, doctest))] +#[cfg(doctest)] pub mod doc { pub use super::{ CommandArg, contains_sensitive_keyword, create_temp_ninja_file, is_sensitive_arg, @@ -58,7 +58,7 @@ fn is_sensitive_key(key: &str) -> bool { /// Check if `arg` contains a sensitive keyword. /// /// # Examples -/// ``` +/// ```ignore /// # use netsuke::runner::doc::{CommandArg, contains_sensitive_keyword}; /// assert!(contains_sensitive_keyword(&CommandArg::new("token=abc".into()))); /// assert!(!contains_sensitive_keyword(&CommandArg::new("path=/tmp".into()))); @@ -73,7 +73,7 @@ pub fn contains_sensitive_keyword(arg: &CommandArg) -> bool { /// Determine whether the argument should be redacted. /// /// # Examples -/// ``` +/// ```ignore /// # use netsuke::runner::doc::{CommandArg, is_sensitive_arg}; /// assert!(is_sensitive_arg(&CommandArg::new("password=123".into()))); /// assert!(!is_sensitive_arg(&CommandArg::new("file=readme".into()))); @@ -88,7 +88,7 @@ pub fn is_sensitive_arg(arg: &CommandArg) -> bool { /// Sensitive values are replaced with `***REDACTED***`, preserving keys. /// /// # Examples -/// ``` +/// ```ignore /// # use netsuke::runner::doc::{CommandArg, redact_argument}; /// let arg = CommandArg::new("token=abc".into()); /// assert_eq!(redact_argument(&arg).as_str(), "token=***REDACTED***"); @@ -110,7 +110,7 @@ pub fn redact_argument(arg: &CommandArg) -> CommandArg { /// Redact sensitive information from all `args`. /// /// # Examples -/// ``` +/// ```ignore /// # use netsuke::runner::doc::{CommandArg, redact_sensitive_args}; /// let args = vec![ /// CommandArg::new("ninja".into()), From ef14ef59de62455d6ab5656080fd4fc2404d4b37 Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 30 Sep 2025 07:37:34 +0100 Subject: [PATCH 08/17] Harden Ninja helper durability Flush Ninja file writes, probe ancestors via capabilities, and cover the runner documentation surface with targeted tests. Document the NINJA_ENV override to resolve review feedback. --- docs/netsuke-design.md | 5 ++++ src/runner/process.rs | 66 +++++++++++++++++++++++++++++++++++------- 2 files changed, 60 insertions(+), 11 deletions(-) diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index 9a88bca9..4867f918 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -1606,6 +1606,11 @@ before spawning Ninja rather than forwarding the flag. Error scenarios are validated using clap's `ErrorKind` enumeration in unit tests and via Cucumber steps for behavioural coverage. +The Ninja executable may be overridden via the `NINJA_ENV` environment +variable. For example, `NINJA_ENV=/opt/ninja/bin/ninja netsuke build` forces +Netsuke to execute the specified binary while preserving the default when the +variable is unset or invalid. + ### 8.5 Manual Pages The CLI definition doubles as the source for user documentation. A build script diff --git a/src/runner/process.rs b/src/runner/process.rs index ceb0d68f..7cf6bfb1 100644 --- a/src/runner/process.rs +++ b/src/runner/process.rs @@ -7,7 +7,9 @@ use camino::{Utf8Path, Utf8PathBuf}; use cap_std::{ambient_authority, fs as cap_fs}; use ninja_env::NINJA_ENV; use std::{ - env, fs, + env, + ffi::OsString, + fs, io::{self, BufRead, BufReader, Write}, path::{Path, PathBuf}, process::{Command, Stdio}, @@ -166,6 +168,10 @@ pub fn write_ninja_file_utf8( .with_context(|| format!("failed to create Ninja file at {path}"))?; file.write_all(content.as_str().as_bytes()) .with_context(|| format!("failed to write Ninja file to {path}"))?; + file.flush() + .with_context(|| format!("failed to flush Ninja file at {path}"))?; + file.sync_all() + .with_context(|| format!("failed to sync Ninja file at {path}"))?; Ok(()) } @@ -177,16 +183,18 @@ fn derive_dir_and_relative(path: &Utf8Path) -> AnyResult<(cap_fs::Dir, Utf8PathB } let mut ancestors = path.ancestors(); - ancestors.next(); - let base = ancestors - .find(|candidate| candidate.exists()) + ancestors.next(); // skip the full path + let (base, dir) = ancestors + .find_map(|candidate| { + cap_fs::Dir::open_ambient_dir(candidate.as_str(), ambient_authority()) + .ok() + .map(|dir| (candidate.to_owned(), dir)) + }) .ok_or_else(|| anyhow!("no existing ancestor for {path}"))?; let relative = path - .strip_prefix(base) + .strip_prefix(&base) .context("derive relative Ninja path")? .to_owned(); - let dir = cap_fs::Dir::open_ambient_dir(base.as_std_path(), ambient_authority()) - .with_context(|| format!("open ancestor directory {base}"))?; Ok((dir, relative)) } @@ -213,9 +221,11 @@ pub fn write_ninja_file(path: &Path, content: &NinjaContent) -> AnyResult<()> { Ok(()) } -#[must_use] -pub fn resolve_ninja_program_utf8() -> Utf8PathBuf { - env::var_os(NINJA_ENV) +fn resolve_ninja_program_utf8_with(mut read_env: F) -> Utf8PathBuf +where + F: FnMut(&str) -> Option, +{ + read_env(NINJA_ENV) .and_then(|value| { let path = PathBuf::from(value); Utf8PathBuf::from_path_buf(path).ok() @@ -223,6 +233,11 @@ pub fn resolve_ninja_program_utf8() -> Utf8PathBuf { .unwrap_or_else(|| Utf8PathBuf::from(NINJA_PROGRAM)) } +#[must_use] +pub fn resolve_ninja_program_utf8() -> Utf8PathBuf { + resolve_ninja_program_utf8_with(|key| env::var_os(key)) +} + /// Determine which Ninja executable to invoke. #[must_use] pub fn resolve_ninja_program() -> PathBuf { @@ -344,9 +359,15 @@ mod tests { #[test] fn create_temp_ninja_file_persists_contents() { + use std::io::Read; + let content = NinjaContent::new(String::from("rule cc")); let file = create_temp_ninja_file(&content).expect("create temp file"); - let written = std::fs::read_to_string(file.path()).expect("read temp file"); + let mut reopened = file.reopen().expect("reopen temp file"); + let mut written = String::new(); + reopened + .read_to_string(&mut written) + .expect("read temp file"); assert_eq!(written, content.as_str()); assert!(file.path().to_string_lossy().ends_with(".ninja")); } @@ -379,4 +400,27 @@ mod tests { assert_eq!(written, content.as_str()); assert!(nested.parent().expect("parent path").exists()); } + + #[test] + fn resolve_ninja_program_utf8_prefers_env_override() { + let resolved = resolve_ninja_program_utf8_with(|_| Some(OsString::from("/opt/ninja"))); + assert_eq!(resolved, Utf8PathBuf::from("/opt/ninja")); + } + + #[test] + fn resolve_ninja_program_utf8_defaults_without_override() { + let resolved = resolve_ninja_program_utf8_with(|_| None); + assert_eq!(resolved, Utf8PathBuf::from(NINJA_PROGRAM)); + } + + #[cfg(unix)] + #[test] + fn resolve_ninja_program_utf8_ignores_invalid_utf8_override() { + use std::os::unix::ffi::OsStringExt; + + let resolved = resolve_ninja_program_utf8_with(|_| { + Some(OsString::from_vec(vec![0xff, b'n', b'i', b'n', b'j', b'a'])) + }); + assert_eq!(resolved, Utf8PathBuf::from(NINJA_PROGRAM)); + } } From 52ce62a0d5b07d4d8199fb581fd9a14aaae68933 Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 30 Sep 2025 07:37:40 +0100 Subject: [PATCH 09/17] Canonicalize Ninja paths with cap-std Resolve build and working directory paths using cap-std handles while preserving the prior fallback for missing build files. Rework run_ninja logging to favour UTF-8 paths and add a helper that returns canonicalised UTF-8 paths. Update the manifest resolution to return Utf8PathBuf so generation uses UTF-8-safe paths. --- src/runner.rs | 21 +++++++---- src/runner/process.rs | 82 +++++++++++++++++++++++++++++++++++++------ 2 files changed, 86 insertions(+), 17 deletions(-) diff --git a/src/runner.rs b/src/runner.rs index e9cdbb5e..b0d9f594 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -7,6 +7,7 @@ use crate::cli::{BuildArgs, Cli, Commands}; use crate::{ir::BuildGraph, manifest, ninja_gen}; use anyhow::{Context, Result}; +use camino::Utf8PathBuf; use std::borrow::Cow; use std::path::Path; use tempfile::NamedTempFile; @@ -163,8 +164,8 @@ fn handle_build(cli: &Cli, args: &BuildArgs) -> Result<()> { /// ``` fn generate_ninja(cli: &Cli) -> Result { let manifest_path = resolve_manifest_path(cli); - let manifest = manifest::from_path(&manifest_path) - .with_context(|| format!("loading manifest at {}", manifest_path.display()))?; + let manifest = manifest::from_path(manifest_path.as_std_path()) + .with_context(|| format!("loading manifest at {manifest_path}"))?; if tracing::enabled!(tracing::Level::DEBUG) { let ast_json = serde_json::to_string_pretty(&manifest).context("serialising manifest")?; debug!("AST:\n{ast_json}"); @@ -181,11 +182,17 @@ fn generate_ninja(cli: &Cli) -> Result { /// use crate::cli::Cli; /// use crate::runner::resolve_manifest_path; /// let cli = Cli { file: "Netsukefile".into(), directory: None, jobs: None, verbose: false, command: None }; -/// assert!(resolve_manifest_path(&cli).ends_with("Netsukefile")); +/// assert!(resolve_manifest_path(&cli).as_str().ends_with("Netsukefile")); /// ``` #[must_use] -fn resolve_manifest_path(cli: &Cli) -> std::path::PathBuf { - cli.directory - .as_ref() - .map_or_else(|| cli.file.clone(), |dir| dir.join(&cli.file)) +fn resolve_manifest_path(cli: &Cli) -> Utf8PathBuf { + let file = + Utf8PathBuf::from_path_buf(cli.file.clone()).expect("manifest path must be valid UTF-8"); + if let Some(dir) = &cli.directory { + let base = Utf8PathBuf::from_path_buf(dir.clone()) + .expect("manifest directory must be valid UTF-8"); + base.join(file) + } else { + file + } } diff --git a/src/runner/process.rs b/src/runner/process.rs index 7cf6bfb1..2b63c150 100644 --- a/src/runner/process.rs +++ b/src/runner/process.rs @@ -9,8 +9,7 @@ use ninja_env::NINJA_ENV; use std::{ env, ffi::OsString, - fs, - io::{self, BufRead, BufReader, Write}, + io::{self, BufRead, BufReader, ErrorKind, Write}, path::{Path, PathBuf}, process::{Command, Stdio}, thread, @@ -266,28 +265,44 @@ pub fn run_ninja( ) -> io::Result<()> { let mut cmd = Command::new(program); if let Some(dir) = &cli.directory { - let dir = fs::canonicalize(dir)?; - cmd.current_dir(dir); + let canonical = canonicalize_utf8_path(dir.as_path())?; + cmd.current_dir(canonical.as_std_path()); } if let Some(jobs) = cli.jobs { cmd.arg("-j").arg(jobs.to_string()); } - let build_file_path = build_file - .canonicalize() - .unwrap_or_else(|_| build_file.to_path_buf()); - cmd.arg("-f").arg(&build_file_path); + let build_file_path = canonicalize_utf8_path(build_file).or_else(|_| { + Utf8PathBuf::from_path_buf(build_file.to_path_buf()).map_err(|_| { + io::Error::new( + ErrorKind::InvalidData, + format!( + "build file path {} is not valid UTF-8", + build_file.display() + ), + ) + }) + })?; + cmd.arg("-f").arg(build_file_path.as_std_path()); cmd.args(targets.as_slice()); cmd.stdout(Stdio::piped()); cmd.stderr(Stdio::piped()); - let program = cmd.get_program().to_string_lossy().into_owned(); + let program_path = PathBuf::from(cmd.get_program()); + let program_display = Utf8PathBuf::from_path_buf(program_path.clone()).map_or_else( + |_| program_path.to_string_lossy().into_owned(), + Utf8PathBuf::into_string, + ); let args: Vec = cmd .get_args() .map(|a| CommandArg::new(a.to_string_lossy().into_owned())) .collect(); let redacted_args = redact_sensitive_args(&args); let arg_strings: Vec<&str> = redacted_args.iter().map(CommandArg::as_str).collect(); - info!("Running command: {} {}", program, arg_strings.join(" ")); + info!( + "Running command: {} {}", + program_display, + arg_strings.join(" ") + ); let mut child = cmd.spawn()?; let stdout = child.stdout.take().expect("child stdout"); @@ -326,6 +341,53 @@ pub fn run_ninja( } } +fn canonicalize_utf8_path(path: &Path) -> io::Result { + let utf8 = Utf8Path::from_path(path).ok_or_else(|| { + io::Error::new( + ErrorKind::InvalidData, + format!("path {} is not valid UTF-8", path.display()), + ) + })?; + + let convert = |buf: PathBuf, reference: &Utf8Path| { + Utf8PathBuf::from_path_buf(buf).map_err(|_| { + io::Error::new( + ErrorKind::InvalidData, + format!("canonical path for {reference} is not valid UTF-8"), + ) + }) + }; + + if utf8.as_str().is_empty() || utf8 == Utf8Path::new(".") { + let dir = cap_fs::Dir::open_ambient_dir(".", ambient_authority())?; + let resolved = dir.canonicalize(Path::new("."))?; + return convert(resolved, Utf8Path::new(".")); + } + + if utf8.parent().is_none() && utf8.file_name().is_none() { + return Ok(utf8.to_path_buf()); + } + + if utf8.is_relative() { + let dir = cap_fs::Dir::open_ambient_dir(".", ambient_authority())?; + let resolved = dir.canonicalize(utf8.as_std_path())?; + return convert(resolved, utf8); + } + + let parent = utf8.parent().unwrap_or_else(|| Utf8Path::new("/")); + let handle = cap_fs::Dir::open_ambient_dir(parent.as_std_path(), ambient_authority())?; + let relative = utf8.strip_prefix(parent).unwrap_or(utf8); + let resolved = handle.canonicalize(relative.as_std_path())?; + let canonical = convert(resolved, relative)?; + if canonical.is_absolute() { + Ok(canonical) + } else { + let mut absolute = parent.to_path_buf(); + absolute.push(&canonical); + Ok(absolute) + } +} + #[cfg(test)] mod tests { use super::*; From aebf44bb7b108fb919af346adc23034dfeee2fec Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 30 Sep 2025 07:37:48 +0100 Subject: [PATCH 10/17] Strengthen temp Ninja file coverage --- src/runner/process.rs | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/runner/process.rs b/src/runner/process.rs index 2b63c150..1006fd8b 100644 --- a/src/runner/process.rs +++ b/src/runner/process.rs @@ -420,18 +420,33 @@ mod tests { } #[test] - fn create_temp_ninja_file_persists_contents() { - use std::io::Read; + fn create_temp_ninja_file_supports_reopen() { + use std::io::{Read, Seek, SeekFrom}; let content = NinjaContent::new(String::from("rule cc")); let file = create_temp_ninja_file(&content).expect("create temp file"); + let mut reopened = file.reopen().expect("reopen temp file"); let mut written = String::new(); reopened .read_to_string(&mut written) - .expect("read temp file"); + .expect("read reopened temp file"); assert_eq!(written, content.as_str()); + + // Metadata queries must succeed while the original handle is alive so + // Windows-style sharing semantics remain intact. + let metadata = std::fs::metadata(file.path()).expect("query temp file metadata"); + assert_eq!(metadata.len(), content.as_str().len() as u64); assert!(file.path().to_string_lossy().ends_with(".ninja")); + + reopened + .seek(SeekFrom::Start(0)) + .expect("rewind reopened temp file"); + written.clear(); + reopened + .read_to_string(&mut written) + .expect("re-read reopened temp file"); + assert_eq!(written, content.as_str()); } #[test] From 6d1038d16dfa95f5ad030874ae56e1da30d1677e Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 30 Sep 2025 08:27:22 +0100 Subject: [PATCH 11/17] Refactor canonicalize path helpers --- src/runner/process.rs | 50 ++++++++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/src/runner/process.rs b/src/runner/process.rs index 1006fd8b..eca0a6f6 100644 --- a/src/runner/process.rs +++ b/src/runner/process.rs @@ -349,36 +349,43 @@ fn canonicalize_utf8_path(path: &Path) -> io::Result { ) })?; - let convert = |buf: PathBuf, reference: &Utf8Path| { - Utf8PathBuf::from_path_buf(buf).map_err(|_| { - io::Error::new( - ErrorKind::InvalidData, - format!("canonical path for {reference} is not valid UTF-8"), - ) - }) - }; - if utf8.as_str().is_empty() || utf8 == Utf8Path::new(".") { - let dir = cap_fs::Dir::open_ambient_dir(".", ambient_authority())?; - let resolved = dir.canonicalize(Path::new("."))?; - return convert(resolved, Utf8Path::new(".")); + return canonicalize_current_dir(); } if utf8.parent().is_none() && utf8.file_name().is_none() { - return Ok(utf8.to_path_buf()); + return canonicalize_root_path(utf8); } if utf8.is_relative() { - let dir = cap_fs::Dir::open_ambient_dir(".", ambient_authority())?; - let resolved = dir.canonicalize(utf8.as_std_path())?; - return convert(resolved, utf8); + return canonicalize_relative_path(utf8); } + canonicalize_absolute_path(utf8) +} + +fn canonicalize_current_dir() -> io::Result { + let dir = cap_fs::Dir::open_ambient_dir(".", ambient_authority())?; + let resolved = dir.canonicalize(Path::new("."))?; + convert_path_to_utf8(resolved, Utf8Path::new(".")) +} + +fn canonicalize_root_path(utf8: &Utf8Path) -> io::Result { + Ok(utf8.to_path_buf()) +} + +fn canonicalize_relative_path(utf8: &Utf8Path) -> io::Result { + let dir = cap_fs::Dir::open_ambient_dir(".", ambient_authority())?; + let resolved = dir.canonicalize(utf8.as_std_path())?; + convert_path_to_utf8(resolved, utf8) +} + +fn canonicalize_absolute_path(utf8: &Utf8Path) -> io::Result { let parent = utf8.parent().unwrap_or_else(|| Utf8Path::new("/")); let handle = cap_fs::Dir::open_ambient_dir(parent.as_std_path(), ambient_authority())?; let relative = utf8.strip_prefix(parent).unwrap_or(utf8); let resolved = handle.canonicalize(relative.as_std_path())?; - let canonical = convert(resolved, relative)?; + let canonical = convert_path_to_utf8(resolved, relative)?; if canonical.is_absolute() { Ok(canonical) } else { @@ -388,6 +395,15 @@ fn canonicalize_utf8_path(path: &Path) -> io::Result { } } +fn convert_path_to_utf8(buf: PathBuf, reference: &Utf8Path) -> io::Result { + Utf8PathBuf::from_path_buf(buf).map_err(|_| { + io::Error::new( + ErrorKind::InvalidData, + format!("canonical path for {reference} is not valid UTF-8"), + ) + }) +} + #[cfg(test)] mod tests { use super::*; From 16aad6f0bfbdb6cb619548bcecda08197a227e3e Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 30 Sep 2025 08:58:16 +0100 Subject: [PATCH 12/17] Wrap root canonicalization at call site Update canonicalize_root_path to return a bare Utf8PathBuf and wrap its caller so clippy no longer flags an unnecessary Result. --- src/runner/process.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/runner/process.rs b/src/runner/process.rs index eca0a6f6..f1b41aa8 100644 --- a/src/runner/process.rs +++ b/src/runner/process.rs @@ -354,7 +354,7 @@ fn canonicalize_utf8_path(path: &Path) -> io::Result { } if utf8.parent().is_none() && utf8.file_name().is_none() { - return canonicalize_root_path(utf8); + return Ok(canonicalize_root_path(utf8)); } if utf8.is_relative() { @@ -370,8 +370,8 @@ fn canonicalize_current_dir() -> io::Result { convert_path_to_utf8(resolved, Utf8Path::new(".")) } -fn canonicalize_root_path(utf8: &Utf8Path) -> io::Result { - Ok(utf8.to_path_buf()) +fn canonicalize_root_path(utf8: &Utf8Path) -> Utf8PathBuf { + utf8.to_path_buf() } fn canonicalize_relative_path(utf8: &Utf8Path) -> io::Result { From f871b5c81034dc0ac20e5e96f3db413f7af7d3e3 Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 30 Sep 2025 12:27:28 +0100 Subject: [PATCH 13/17] Refactor Ninja helpers for share-safe temp writes --- src/runner/process.rs | 61 +++++++++++++++++++++++++++++-------------- 1 file changed, 41 insertions(+), 20 deletions(-) diff --git a/src/runner/process.rs b/src/runner/process.rs index f1b41aa8..d28a402d 100644 --- a/src/runner/process.rs +++ b/src/runner/process.rs @@ -139,12 +139,19 @@ pub fn redact_sensitive_args(args: &[CommandArg]) -> Vec { /// assert!(tmp.path().to_string_lossy().ends_with(".ninja")); /// ``` pub fn create_temp_ninja_file(content: &NinjaContent) -> AnyResult { - let tmp = Builder::new() + let mut tmp = Builder::new() .prefix("netsuke.") .suffix(".ninja") .tempfile() .context("create temp file")?; - write_ninja_file(tmp.path(), content)?; + tmp.as_file_mut() + .write_all(content.as_str().as_bytes()) + .context("write temp Ninja file")?; + tmp.as_file_mut().flush().context("flush temp Ninja file")?; + tmp.as_file_mut() + .sync_all() + .context("sync temp Ninja file")?; + info!("Generated temporary Ninja file at {}", tmp.path().display()); Ok(tmp) } @@ -243,26 +250,12 @@ pub fn resolve_ninja_program() -> PathBuf { resolve_ninja_program_utf8().into() } -/// Invoke the Ninja executable with the provided CLI settings. -/// -/// The function forwards the job count and working directory to Ninja, -/// specifies the temporary build file, and streams its standard output and -/// error back to the user. -/// -/// # Errors -/// -/// Returns an [`io::Error`] if the Ninja process fails to spawn or reports a -/// non-zero exit status. -/// -/// # Panics -/// -/// Panics if the child's output streams cannot be captured. -pub fn run_ninja( +fn build_ninja_command( program: &Path, cli: &Cli, build_file: &Path, targets: &BuildTargets<'_>, -) -> io::Result<()> { +) -> io::Result { let mut cmd = Command::new(program); if let Some(dir) = &cli.directory { let canonical = canonicalize_utf8_path(dir.as_path())?; @@ -284,9 +277,10 @@ pub fn run_ninja( })?; cmd.arg("-f").arg(build_file_path.as_std_path()); cmd.args(targets.as_slice()); - cmd.stdout(Stdio::piped()); - cmd.stderr(Stdio::piped()); + Ok(cmd) +} +fn log_ninja_command(cmd: &Command) { let program_path = PathBuf::from(cmd.get_program()); let program_display = Utf8PathBuf::from_path_buf(program_path.clone()).map_or_else( |_| program_path.to_string_lossy().into_owned(), @@ -303,6 +297,33 @@ pub fn run_ninja( program_display, arg_strings.join(" ") ); +} + +/// Invoke the Ninja executable with the provided CLI settings. +/// +/// The function forwards the job count and working directory to Ninja, +/// specifies the temporary build file, and streams its standard output and +/// error back to the user. +/// +/// # Errors +/// +/// Returns an [`io::Error`] if the Ninja process fails to spawn or reports a +/// non-zero exit status. +/// +/// # Panics +/// +/// Panics if the child's output streams cannot be captured. +pub fn run_ninja( + program: &Path, + cli: &Cli, + build_file: &Path, + targets: &BuildTargets<'_>, +) -> io::Result<()> { + let mut cmd = build_ninja_command(program, cli, build_file, targets)?; + cmd.stdout(Stdio::piped()); + cmd.stderr(Stdio::piped()); + + log_ninja_command(&cmd); let mut child = cmd.spawn()?; let stdout = child.stdout.take().expect("child stdout"); From db1861c12ca4053d65f0573f5299e4b93059dcc9 Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 30 Sep 2025 22:02:56 +0100 Subject: [PATCH 14/17] Enable runner process doctests --- src/runner/process.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/runner/process.rs b/src/runner/process.rs index d28a402d..ec364b4c 100644 --- a/src/runner/process.rs +++ b/src/runner/process.rs @@ -59,7 +59,8 @@ fn is_sensitive_key(key: &str) -> bool { /// Check if `arg` contains a sensitive keyword. /// /// # Examples -/// ```ignore +/// ``` +/// # #[cfg(doctest)] /// # use netsuke::runner::doc::{CommandArg, contains_sensitive_keyword}; /// assert!(contains_sensitive_keyword(&CommandArg::new("token=abc".into()))); /// assert!(!contains_sensitive_keyword(&CommandArg::new("path=/tmp".into()))); @@ -74,7 +75,8 @@ pub fn contains_sensitive_keyword(arg: &CommandArg) -> bool { /// Determine whether the argument should be redacted. /// /// # Examples -/// ```ignore +/// ``` +/// # #[cfg(doctest)] /// # use netsuke::runner::doc::{CommandArg, is_sensitive_arg}; /// assert!(is_sensitive_arg(&CommandArg::new("password=123".into()))); /// assert!(!is_sensitive_arg(&CommandArg::new("file=readme".into()))); @@ -89,7 +91,8 @@ pub fn is_sensitive_arg(arg: &CommandArg) -> bool { /// Sensitive values are replaced with `***REDACTED***`, preserving keys. /// /// # Examples -/// ```ignore +/// ``` +/// # #[cfg(doctest)] /// # use netsuke::runner::doc::{CommandArg, redact_argument}; /// let arg = CommandArg::new("token=abc".into()); /// assert_eq!(redact_argument(&arg).as_str(), "token=***REDACTED***"); @@ -132,7 +135,8 @@ pub fn redact_sensitive_args(args: &[CommandArg]) -> Vec { /// Returns an error if the file cannot be created or written. /// /// # Examples -/// ```ignore +/// ``` +/// # #[cfg(doctest)] /// use netsuke::runner::doc::create_temp_ninja_file; /// use netsuke::runner::NinjaContent; /// let tmp = create_temp_ninja_file(&NinjaContent::new("".into())).unwrap(); From c3b87ef456a0fdb1b72fe946472fb15509f9eab9 Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 30 Sep 2025 22:03:01 +0100 Subject: [PATCH 15/17] Write temp Ninja files via open handles --- src/runner/process.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/runner/process.rs b/src/runner/process.rs index ec364b4c..a12fb561 100644 --- a/src/runner/process.rs +++ b/src/runner/process.rs @@ -114,7 +114,8 @@ pub fn redact_argument(arg: &CommandArg) -> CommandArg { /// Redact sensitive information from all `args`. /// /// # Examples -/// ```ignore +/// ``` +/// # #[cfg(doctest)] /// # use netsuke::runner::doc::{CommandArg, redact_sensitive_args}; /// let args = vec![ /// CommandArg::new("ninja".into()), @@ -148,13 +149,14 @@ pub fn create_temp_ninja_file(content: &NinjaContent) -> AnyResult AnyResult<(cap_fs::Dir, Utf8PathB /// Returns an error if the file cannot be written. /// /// # Examples -/// ```ignore +/// ``` /// use std::path::Path; /// use netsuke::runner::doc::write_ninja_file; /// use netsuke::runner::NinjaContent; From d2bc875a5578b9a177aa185729cab42795a2d285 Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 30 Sep 2025 22:03:06 +0100 Subject: [PATCH 16/17] Refactor Ninja runner command execution --- src/runner/process.rs | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/src/runner/process.rs b/src/runner/process.rs index a12fb561..65998068 100644 --- a/src/runner/process.rs +++ b/src/runner/process.rs @@ -11,7 +11,7 @@ use std::{ ffi::OsString, io::{self, BufRead, BufReader, ErrorKind, Write}, path::{Path, PathBuf}, - process::{Command, Stdio}, + process::{Child, Command, ExitStatus, Stdio}, thread, }; use tempfile::{Builder, NamedTempFile}; @@ -256,13 +256,12 @@ pub fn resolve_ninja_program() -> PathBuf { resolve_ninja_program_utf8().into() } -fn build_ninja_command( - program: &Path, +fn configure_ninja_command( + cmd: &mut Command, cli: &Cli, build_file: &Path, targets: &BuildTargets<'_>, -) -> io::Result { - let mut cmd = Command::new(program); +) -> io::Result<()> { if let Some(dir) = &cli.directory { let canonical = canonicalize_utf8_path(dir.as_path())?; cmd.current_dir(canonical.as_std_path()); @@ -283,10 +282,12 @@ fn build_ninja_command( })?; cmd.arg("-f").arg(build_file_path.as_std_path()); cmd.args(targets.as_slice()); - Ok(cmd) + cmd.stdout(Stdio::piped()); + cmd.stderr(Stdio::piped()); + Ok(()) } -fn log_ninja_command(cmd: &Command) { +fn log_command_execution(cmd: &Command) { let program_path = PathBuf::from(cmd.get_program()); let program_display = Utf8PathBuf::from_path_buf(program_path.clone()).map_or_else( |_| program_path.to_string_lossy().into_owned(), @@ -325,13 +326,15 @@ pub fn run_ninja( build_file: &Path, targets: &BuildTargets<'_>, ) -> io::Result<()> { - let mut cmd = build_ninja_command(program, cli, build_file, targets)?; - cmd.stdout(Stdio::piped()); - cmd.stderr(Stdio::piped()); - - log_ninja_command(&cmd); + let mut cmd = Command::new(program); + configure_ninja_command(&mut cmd, cli, build_file, targets)?; + log_command_execution(&cmd); + let child = cmd.spawn()?; + let status = spawn_and_stream_output(child)?; + check_exit_status(status) +} - let mut child = cmd.spawn()?; +fn spawn_and_stream_output(mut child: Child) -> io::Result { let stdout = child.stdout.take().expect("child stdout"); let stderr = child.stderr.take().expect("child stderr"); @@ -353,7 +356,10 @@ pub fn run_ninja( let status = child.wait()?; let _ = out_handle.join(); let _ = err_handle.join(); + Ok(status) +} +fn check_exit_status(status: ExitStatus) -> io::Result<()> { if status.success() { Ok(()) } else { From 189671af83fcdff85fe9401400a64513e10f8b5c Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 30 Sep 2025 23:37:57 +0100 Subject: [PATCH 17/17] Split runner process helpers into submodules --- src/runner/process.rs | 378 ++------------------------------ src/runner/process/file_io.rs | 146 ++++++++++++ src/runner/process/paths.rs | 70 ++++++ src/runner/process/redaction.rs | 135 ++++++++++++ 4 files changed, 370 insertions(+), 359 deletions(-) create mode 100644 src/runner/process/file_io.rs create mode 100644 src/runner/process/paths.rs create mode 100644 src/runner/process/redaction.rs diff --git a/src/runner/process.rs b/src/runner/process.rs index 65998068..be18936b 100644 --- a/src/runner/process.rs +++ b/src/runner/process.rs @@ -1,10 +1,9 @@ //! Process helpers for Ninja file lifecycle, argument redaction, and subprocess I/O. //! Internal to `runner`; public API is defined in `runner.rs`. -use super::{BuildTargets, NINJA_PROGRAM, NinjaContent}; + +use super::{BuildTargets, NINJA_PROGRAM}; use crate::cli::Cli; -use anyhow::{Context, Result as AnyResult, anyhow}; -use camino::{Utf8Path, Utf8PathBuf}; -use cap_std::{ambient_authority, fs as cap_fs}; +use camino::Utf8PathBuf; use ninja_env::NINJA_ENV; use std::{ env, @@ -14,21 +13,22 @@ use std::{ process::{Child, Command, ExitStatus, Stdio}, thread, }; -use tempfile::{Builder, NamedTempFile}; use tracing::info; -#[derive(Debug, Clone)] -pub struct CommandArg(String); -impl CommandArg { - #[must_use] - pub fn new(arg: String) -> Self { - Self(arg) - } - #[must_use] - pub fn as_str(&self) -> &str { - &self.0 - } -} +mod file_io; +mod paths; +mod redaction; + +pub use file_io::*; +pub use paths::*; +// Re-export redaction helpers for doctests without leaking unused imports in release builds. +#[cfg_attr( + not(doctest), + allow(unused_imports, reason = "retain doctest re-exports") +)] +pub use redaction::*; + +use redaction::{CommandArg, redact_sensitive_args}; // Public helpers for doctests only. This exposes internal helpers as a stable // testing surface without exporting them in release builds. @@ -41,198 +41,6 @@ pub mod doc { }; } -fn is_sensitive_key(key: &str) -> bool { - const SENSITIVE_KEYS: [&str; 7] = [ - "password", - "token", - "secret", - "api_key", - "apikey", - "auth", - "authorization", - ]; - SENSITIVE_KEYS - .iter() - .any(|candidate| key.eq_ignore_ascii_case(candidate)) -} - -/// Check if `arg` contains a sensitive keyword. -/// -/// # Examples -/// ``` -/// # #[cfg(doctest)] -/// # use netsuke::runner::doc::{CommandArg, contains_sensitive_keyword}; -/// assert!(contains_sensitive_keyword(&CommandArg::new("token=abc".into()))); -/// assert!(!contains_sensitive_keyword(&CommandArg::new("path=/tmp".into()))); -/// ``` -#[must_use] -pub fn contains_sensitive_keyword(arg: &CommandArg) -> bool { - arg.as_str() - .split_once('=') - .is_some_and(|(key, _)| is_sensitive_key(key.trim())) -} - -/// Determine whether the argument should be redacted. -/// -/// # Examples -/// ``` -/// # #[cfg(doctest)] -/// # use netsuke::runner::doc::{CommandArg, is_sensitive_arg}; -/// assert!(is_sensitive_arg(&CommandArg::new("password=123".into()))); -/// assert!(!is_sensitive_arg(&CommandArg::new("file=readme".into()))); -/// ``` -#[must_use] -pub fn is_sensitive_arg(arg: &CommandArg) -> bool { - contains_sensitive_keyword(arg) -} - -/// Redact sensitive information in a single argument. -/// -/// Sensitive values are replaced with `***REDACTED***`, preserving keys. -/// -/// # Examples -/// ``` -/// # #[cfg(doctest)] -/// # use netsuke::runner::doc::{CommandArg, redact_argument}; -/// let arg = CommandArg::new("token=abc".into()); -/// assert_eq!(redact_argument(&arg).as_str(), "token=***REDACTED***"); -/// let arg = CommandArg::new("path=/tmp".into()); -/// assert_eq!(redact_argument(&arg).as_str(), "path=/tmp"); -/// ``` -#[must_use] -pub fn redact_argument(arg: &CommandArg) -> CommandArg { - if is_sensitive_arg(arg) { - if let Some((key, _)) = arg.as_str().split_once('=') { - let trimmed = key.trim(); - return CommandArg::new(format!("{trimmed}=***REDACTED***")); - } - return CommandArg::new(String::from("***REDACTED***")); - } - arg.clone() -} - -/// Redact sensitive information from all `args`. -/// -/// # Examples -/// ``` -/// # #[cfg(doctest)] -/// # use netsuke::runner::doc::{CommandArg, redact_sensitive_args}; -/// let args = vec![ -/// CommandArg::new("ninja".into()), -/// CommandArg::new("token=abc".into()), -/// ]; -/// let redacted = redact_sensitive_args(&args); -/// assert_eq!(redacted[1].as_str(), "token=***REDACTED***"); -/// ``` -#[must_use] -pub fn redact_sensitive_args(args: &[CommandArg]) -> Vec { - args.iter().map(redact_argument).collect() -} - -/// Create a temporary Ninja file on disk containing `content`. -/// -/// # Errors -/// -/// Returns an error if the file cannot be created or written. -/// -/// # Examples -/// ``` -/// # #[cfg(doctest)] -/// use netsuke::runner::doc::create_temp_ninja_file; -/// use netsuke::runner::NinjaContent; -/// let tmp = create_temp_ninja_file(&NinjaContent::new("".into())).unwrap(); -/// assert!(tmp.path().to_string_lossy().ends_with(".ninja")); -/// ``` -pub fn create_temp_ninja_file(content: &NinjaContent) -> AnyResult { - let mut tmp = Builder::new() - .prefix("netsuke.") - .suffix(".ninja") - .tempfile() - .context("create temp file")?; - { - let handle = tmp.as_file_mut(); - handle - .write_all(content.as_str().as_bytes()) - .context("write temp ninja file")?; - handle.flush().context("flush temp ninja file")?; - handle.sync_all().context("sync temp ninja file")?; - } - info!("Generated temporary Ninja file at {}", tmp.path().display()); - Ok(tmp) -} - -/// Write `content` to `path` within `dir`. -/// -/// # Errors -/// -/// Returns an error if the parent directories cannot be created or the file cannot be written. -pub fn write_ninja_file_utf8( - dir: &cap_fs::Dir, - path: &Utf8Path, - content: &NinjaContent, -) -> AnyResult<()> { - if let Some(parent) = path.parent().filter(|p| !p.as_str().is_empty()) { - dir.create_dir_all(parent.as_str()) - .with_context(|| format!("failed to create parent directory {parent}"))?; - } - let mut file = dir - .create(path.as_str()) - .with_context(|| format!("failed to create Ninja file at {path}"))?; - file.write_all(content.as_str().as_bytes()) - .with_context(|| format!("failed to write Ninja file to {path}"))?; - file.flush() - .with_context(|| format!("failed to flush Ninja file at {path}"))?; - file.sync_all() - .with_context(|| format!("failed to sync Ninja file at {path}"))?; - Ok(()) -} - -fn derive_dir_and_relative(path: &Utf8Path) -> AnyResult<(cap_fs::Dir, Utf8PathBuf)> { - if path.is_relative() { - let dir = cap_fs::Dir::open_ambient_dir(".", ambient_authority()) - .context("open ambient directory")?; - return Ok((dir, path.to_owned())); - } - - let mut ancestors = path.ancestors(); - ancestors.next(); // skip the full path - let (base, dir) = ancestors - .find_map(|candidate| { - cap_fs::Dir::open_ambient_dir(candidate.as_str(), ambient_authority()) - .ok() - .map(|dir| (candidate.to_owned(), dir)) - }) - .ok_or_else(|| anyhow!("no existing ancestor for {path}"))?; - let relative = path - .strip_prefix(&base) - .context("derive relative Ninja path")? - .to_owned(); - Ok((dir, relative)) -} - -/// Write `content` to `path` and log the file's location. -/// -/// # Errors -/// -/// Returns an error if the file cannot be written. -/// -/// # Examples -/// ``` -/// use std::path::Path; -/// use netsuke::runner::doc::write_ninja_file; -/// use netsuke::runner::NinjaContent; -/// let content = NinjaContent::new("rule cc\n".to_string()); -/// write_ninja_file(Path::new("out.ninja"), &content).unwrap(); -/// ``` -pub fn write_ninja_file(path: &Path, content: &NinjaContent) -> AnyResult<()> { - let utf8_path = - Utf8Path::from_path(path).ok_or_else(|| anyhow!("non-UTF-8 path is not supported"))?; - let (dir, relative) = derive_dir_and_relative(utf8_path)?; - write_ninja_file_utf8(&dir, &relative, content)?; - info!("Generated Ninja file at {utf8_path}"); - Ok(()) -} - fn resolve_ninja_program_utf8_with(mut read_env: F) -> Utf8PathBuf where F: FnMut(&str) -> Option, @@ -250,7 +58,6 @@ pub fn resolve_ninja_program_utf8() -> Utf8PathBuf { resolve_ninja_program_utf8_with(|key| env::var_os(key)) } -/// Determine which Ninja executable to invoke. #[must_use] pub fn resolve_ninja_program() -> PathBuf { resolve_ninja_program_utf8().into() @@ -374,158 +181,11 @@ fn check_exit_status(status: ExitStatus) -> io::Result<()> { } } -fn canonicalize_utf8_path(path: &Path) -> io::Result { - let utf8 = Utf8Path::from_path(path).ok_or_else(|| { - io::Error::new( - ErrorKind::InvalidData, - format!("path {} is not valid UTF-8", path.display()), - ) - })?; - - if utf8.as_str().is_empty() || utf8 == Utf8Path::new(".") { - return canonicalize_current_dir(); - } - - if utf8.parent().is_none() && utf8.file_name().is_none() { - return Ok(canonicalize_root_path(utf8)); - } - - if utf8.is_relative() { - return canonicalize_relative_path(utf8); - } - - canonicalize_absolute_path(utf8) -} - -fn canonicalize_current_dir() -> io::Result { - let dir = cap_fs::Dir::open_ambient_dir(".", ambient_authority())?; - let resolved = dir.canonicalize(Path::new("."))?; - convert_path_to_utf8(resolved, Utf8Path::new(".")) -} - -fn canonicalize_root_path(utf8: &Utf8Path) -> Utf8PathBuf { - utf8.to_path_buf() -} - -fn canonicalize_relative_path(utf8: &Utf8Path) -> io::Result { - let dir = cap_fs::Dir::open_ambient_dir(".", ambient_authority())?; - let resolved = dir.canonicalize(utf8.as_std_path())?; - convert_path_to_utf8(resolved, utf8) -} - -fn canonicalize_absolute_path(utf8: &Utf8Path) -> io::Result { - let parent = utf8.parent().unwrap_or_else(|| Utf8Path::new("/")); - let handle = cap_fs::Dir::open_ambient_dir(parent.as_std_path(), ambient_authority())?; - let relative = utf8.strip_prefix(parent).unwrap_or(utf8); - let resolved = handle.canonicalize(relative.as_std_path())?; - let canonical = convert_path_to_utf8(resolved, relative)?; - if canonical.is_absolute() { - Ok(canonical) - } else { - let mut absolute = parent.to_path_buf(); - absolute.push(&canonical); - Ok(absolute) - } -} - -fn convert_path_to_utf8(buf: PathBuf, reference: &Utf8Path) -> io::Result { - Utf8PathBuf::from_path_buf(buf).map_err(|_| { - io::Error::new( - ErrorKind::InvalidData, - format!("canonical path for {reference} is not valid UTF-8"), - ) - }) -} - #[cfg(test)] mod tests { use super::*; - - #[test] - fn contains_sensitive_keyword_only_flags_known_keys() { - let token = CommandArg::new(String::from("token=abc")); - assert!(contains_sensitive_keyword(&token)); - - let positional = CommandArg::new(String::from("secrets.yml")); - assert!(!contains_sensitive_keyword(&positional)); - - let path_arg = CommandArg::new(String::from("path=/tmp/secrets.yml")); - assert!(!contains_sensitive_keyword(&path_arg)); - - let spaced = CommandArg::new(String::from(" PASSWORD = value ")); - assert!(contains_sensitive_keyword(&spaced)); - } - - #[test] - fn redact_argument_preserves_non_sensitive_pairs() { - let redacted = redact_argument(&CommandArg::new(String::from("auth = token123"))); - assert_eq!(redacted.as_str(), "auth=***REDACTED***"); - - let untouched = redact_argument(&CommandArg::new(String::from("path=/var/secrets"))); - assert_eq!(untouched.as_str(), "path=/var/secrets"); - - let positional = redact_argument(&CommandArg::new(String::from("secret"))); - assert_eq!(positional.as_str(), "secret"); - } - - #[test] - fn create_temp_ninja_file_supports_reopen() { - use std::io::{Read, Seek, SeekFrom}; - - let content = NinjaContent::new(String::from("rule cc")); - let file = create_temp_ninja_file(&content).expect("create temp file"); - - let mut reopened = file.reopen().expect("reopen temp file"); - let mut written = String::new(); - reopened - .read_to_string(&mut written) - .expect("read reopened temp file"); - assert_eq!(written, content.as_str()); - - // Metadata queries must succeed while the original handle is alive so - // Windows-style sharing semantics remain intact. - let metadata = std::fs::metadata(file.path()).expect("query temp file metadata"); - assert_eq!(metadata.len(), content.as_str().len() as u64); - assert!(file.path().to_string_lossy().ends_with(".ninja")); - - reopened - .seek(SeekFrom::Start(0)) - .expect("rewind reopened temp file"); - written.clear(); - reopened - .read_to_string(&mut written) - .expect("re-read reopened temp file"); - assert_eq!(written, content.as_str()); - } - - #[test] - fn write_ninja_file_utf8_creates_parent_directories() { - let temp = tempfile::tempdir().expect("create temp dir"); - let dir = - cap_fs::Dir::open_ambient_dir(temp.path(), ambient_authority()).expect("open temp dir"); - let nested = Utf8PathBuf::from("nested/build.ninja"); - let content = NinjaContent::new(String::from("build all: phony")); - - write_ninja_file_utf8(&dir, &nested, &content).expect("write ninja file"); - - let nested_path = temp.path().join("nested").join("build.ninja"); - let written = std::fs::read_to_string(&nested_path).expect("read nested file"); - assert_eq!(written, content.as_str()); - assert!(nested_path.parent().expect("parent path").exists()); - } - - #[test] - fn write_ninja_file_handles_absolute_paths() { - let temp = tempfile::tempdir().expect("create temp dir"); - let nested = temp.path().join("nested").join("build.ninja"); - let content = NinjaContent::new(String::from("build all: phony")); - - write_ninja_file(&nested, &content).expect("write ninja file"); - - let written = std::fs::read_to_string(&nested).expect("read nested file"); - assert_eq!(written, content.as_str()); - assert!(nested.parent().expect("parent path").exists()); - } + use camino::Utf8PathBuf; + use std::ffi::OsString; #[test] fn resolve_ninja_program_utf8_prefers_env_override() { diff --git a/src/runner/process/file_io.rs b/src/runner/process/file_io.rs new file mode 100644 index 00000000..8b8bede0 --- /dev/null +++ b/src/runner/process/file_io.rs @@ -0,0 +1,146 @@ +//! File creation helpers for the Ninja runner. +//! Handles temporary build files and writes to capability-based directories. + +use crate::runner::NinjaContent; +use anyhow::{Context, Result as AnyResult, anyhow}; +use camino::{Utf8Path, Utf8PathBuf}; +use cap_std::{ambient_authority, fs as cap_fs}; +use std::io::Write; +use std::path::Path; +use tempfile::{Builder, NamedTempFile}; +use tracing::info; + +pub fn create_temp_ninja_file(content: &NinjaContent) -> AnyResult { + let mut tmp = Builder::new() + .prefix("netsuke.") + .suffix(".ninja") + .tempfile() + .context("create temp file")?; + { + let handle = tmp.as_file_mut(); + handle + .write_all(content.as_str().as_bytes()) + .context("write temp ninja file")?; + handle.flush().context("flush temp ninja file")?; + handle.sync_all().context("sync temp ninja file")?; + } + info!("Generated temporary Ninja file at {}", tmp.path().display()); + Ok(tmp) +} + +pub fn write_ninja_file_utf8( + dir: &cap_fs::Dir, + path: &Utf8Path, + content: &NinjaContent, +) -> AnyResult<()> { + if let Some(parent) = path.parent().filter(|p| !p.as_str().is_empty()) { + dir.create_dir_all(parent.as_str()) + .with_context(|| format!("failed to create parent directory {parent}"))?; + } + let mut file = dir + .create(path.as_str()) + .with_context(|| format!("failed to create Ninja file at {path}"))?; + file.write_all(content.as_str().as_bytes()) + .with_context(|| format!("failed to write Ninja file to {path}"))?; + file.flush() + .with_context(|| format!("failed to flush Ninja file at {path}"))?; + file.sync_all() + .with_context(|| format!("failed to sync Ninja file at {path}"))?; + Ok(()) +} + +fn derive_dir_and_relative(path: &Utf8Path) -> AnyResult<(cap_fs::Dir, Utf8PathBuf)> { + if path.is_relative() { + let dir = cap_fs::Dir::open_ambient_dir(".", ambient_authority()) + .context("open ambient directory")?; + return Ok((dir, path.to_owned())); + } + + let mut ancestors = path.ancestors(); + ancestors.next(); + let (base, dir) = ancestors + .find_map(|candidate| { + cap_fs::Dir::open_ambient_dir(candidate.as_str(), ambient_authority()) + .ok() + .map(|dir| (candidate.to_owned(), dir)) + }) + .ok_or_else(|| anyhow!("no existing ancestor for {path}"))?; + let relative = path + .strip_prefix(&base) + .context("derive relative Ninja path")? + .to_owned(); + Ok((dir, relative)) +} + +pub fn write_ninja_file(path: &Path, content: &NinjaContent) -> AnyResult<()> { + let utf8_path = + Utf8Path::from_path(path).ok_or_else(|| anyhow!("non-UTF-8 path is not supported"))?; + let (dir, relative) = derive_dir_and_relative(utf8_path)?; + write_ninja_file_utf8(&dir, &relative, content)?; + info!("Generated Ninja file at {utf8_path}"); + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::runner::NinjaContent; + use camino::Utf8PathBuf; + use cap_std::{ambient_authority, fs as cap_fs}; + use std::io::{Read, Seek, SeekFrom}; + + #[test] + fn create_temp_ninja_file_supports_reopen() { + let content = NinjaContent::new(String::from("rule cc")); + let file = create_temp_ninja_file(&content).expect("create temp file"); + + let mut reopened = file.reopen().expect("reopen temp file"); + let mut written = String::new(); + reopened + .read_to_string(&mut written) + .expect("read reopened temp file"); + assert_eq!(written, content.as_str()); + + let metadata = std::fs::metadata(file.path()).expect("query temp file metadata"); + assert_eq!(metadata.len(), content.as_str().len() as u64); + assert!(file.path().to_string_lossy().ends_with(".ninja")); + + reopened + .seek(SeekFrom::Start(0)) + .expect("rewind reopened temp file"); + written.clear(); + reopened + .read_to_string(&mut written) + .expect("re-read reopened temp file"); + assert_eq!(written, content.as_str()); + } + + #[test] + fn write_ninja_file_utf8_creates_parent_directories() { + let temp = tempfile::tempdir().expect("create temp dir"); + let dir = + cap_fs::Dir::open_ambient_dir(temp.path(), ambient_authority()).expect("open temp dir"); + let nested = Utf8PathBuf::from("nested/build.ninja"); + let content = NinjaContent::new(String::from("build all: phony")); + + write_ninja_file_utf8(&dir, &nested, &content).expect("write ninja file"); + + let nested_path = temp.path().join("nested").join("build.ninja"); + let written = std::fs::read_to_string(&nested_path).expect("read nested file"); + assert_eq!(written, content.as_str()); + assert!(nested_path.parent().expect("parent path").exists()); + } + + #[test] + fn write_ninja_file_handles_absolute_paths() { + let temp = tempfile::tempdir().expect("create temp dir"); + let nested = temp.path().join("nested").join("build.ninja"); + let content = NinjaContent::new(String::from("build all: phony")); + + write_ninja_file(&nested, &content).expect("write ninja file"); + + let written = std::fs::read_to_string(&nested).expect("read nested file"); + assert_eq!(written, content.as_str()); + assert!(nested.parent().expect("parent path").exists()); + } +} diff --git a/src/runner/process/paths.rs b/src/runner/process/paths.rs new file mode 100644 index 00000000..02928809 --- /dev/null +++ b/src/runner/process/paths.rs @@ -0,0 +1,70 @@ +//! Path resolution helpers for the Ninja runner. +//! Canonicalises UTF-8 paths via capability-based handles. + +use camino::{Utf8Path, Utf8PathBuf}; +use cap_std::{ambient_authority, fs as cap_fs}; +use std::io::{self, ErrorKind}; +use std::path::{Path, PathBuf}; + +pub fn canonicalize_utf8_path(path: &Path) -> io::Result { + let utf8 = Utf8Path::from_path(path).ok_or_else(|| { + io::Error::new( + ErrorKind::InvalidData, + format!("path {} is not valid UTF-8", path.display()), + ) + })?; + + if utf8.as_str().is_empty() || utf8 == Utf8Path::new(".") { + return canonicalize_current_dir(); + } + + if utf8.parent().is_none() && utf8.file_name().is_none() { + return Ok(canonicalize_root_path(utf8)); + } + + if utf8.is_relative() { + return canonicalize_relative_path(utf8); + } + + canonicalize_absolute_path(utf8) +} + +fn canonicalize_current_dir() -> io::Result { + let dir = cap_fs::Dir::open_ambient_dir(".", ambient_authority())?; + let resolved = dir.canonicalize(Path::new("."))?; + convert_path_to_utf8(resolved, Utf8Path::new(".")) +} + +fn canonicalize_root_path(utf8: &Utf8Path) -> Utf8PathBuf { + utf8.to_path_buf() +} + +fn canonicalize_relative_path(utf8: &Utf8Path) -> io::Result { + let dir = cap_fs::Dir::open_ambient_dir(".", ambient_authority())?; + let resolved = dir.canonicalize(utf8.as_std_path())?; + convert_path_to_utf8(resolved, utf8) +} + +fn canonicalize_absolute_path(utf8: &Utf8Path) -> io::Result { + let parent = utf8.parent().unwrap_or_else(|| Utf8Path::new("/")); + let handle = cap_fs::Dir::open_ambient_dir(parent.as_std_path(), ambient_authority())?; + let relative = utf8.strip_prefix(parent).unwrap_or(utf8); + let resolved = handle.canonicalize(relative.as_std_path())?; + let canonical = convert_path_to_utf8(resolved, relative)?; + if canonical.is_absolute() { + Ok(canonical) + } else { + let mut absolute = parent.to_path_buf(); + absolute.push(&canonical); + Ok(absolute) + } +} + +fn convert_path_to_utf8(buf: PathBuf, reference: &Utf8Path) -> io::Result { + Utf8PathBuf::from_path_buf(buf).map_err(|_| { + io::Error::new( + ErrorKind::InvalidData, + format!("canonical path for {reference} is not valid UTF-8"), + ) + }) +} diff --git a/src/runner/process/redaction.rs b/src/runner/process/redaction.rs new file mode 100644 index 00000000..3ea52772 --- /dev/null +++ b/src/runner/process/redaction.rs @@ -0,0 +1,135 @@ +//! Argument redaction helpers for the Ninja runner. +//! Provides the `CommandArg` wrapper used by doctests and logging. + +#[derive(Debug, Clone)] +pub struct CommandArg(String); +impl CommandArg { + #[must_use] + pub fn new(arg: String) -> Self { + Self(arg) + } + #[must_use] + pub fn as_str(&self) -> &str { + &self.0 + } +} + +fn is_sensitive_key(key: &str) -> bool { + const SENSITIVE_KEYS: [&str; 7] = [ + "password", + "token", + "secret", + "api_key", + "apikey", + "auth", + "authorization", + ]; + SENSITIVE_KEYS + .iter() + .any(|candidate| key.eq_ignore_ascii_case(candidate)) +} + +/// Check if `arg` contains a sensitive keyword. +/// +/// # Examples +/// ``` +/// # #[cfg(doctest)] +/// # use netsuke::runner::doc::{CommandArg, contains_sensitive_keyword}; +/// assert!(contains_sensitive_keyword(&CommandArg::new("token=abc".into()))); +/// assert!(!contains_sensitive_keyword(&CommandArg::new("path=/tmp".into()))); +/// ``` +#[must_use] +pub fn contains_sensitive_keyword(arg: &CommandArg) -> bool { + arg.as_str() + .split_once('=') + .is_some_and(|(key, _)| is_sensitive_key(key.trim())) +} + +/// Determine whether the argument should be redacted. +/// +/// # Examples +/// ``` +/// # #[cfg(doctest)] +/// # use netsuke::runner::doc::{CommandArg, is_sensitive_arg}; +/// assert!(is_sensitive_arg(&CommandArg::new("password=123".into()))); +/// assert!(!is_sensitive_arg(&CommandArg::new("file=readme".into()))); +/// ``` +#[must_use] +pub fn is_sensitive_arg(arg: &CommandArg) -> bool { + contains_sensitive_keyword(arg) +} + +/// Redact sensitive information in a single argument. +/// +/// Sensitive values are replaced with `***REDACTED***`, preserving keys. +/// +/// # Examples +/// ``` +/// # #[cfg(doctest)] +/// # use netsuke::runner::doc::{CommandArg, redact_argument}; +/// let arg = CommandArg::new("token=abc".into()); +/// assert_eq!(redact_argument(&arg).as_str(), "token=***REDACTED***"); +/// let arg = CommandArg::new("path=/tmp".into()); +/// assert_eq!(redact_argument(&arg).as_str(), "path=/tmp"); +/// ``` +#[must_use] +pub fn redact_argument(arg: &CommandArg) -> CommandArg { + if is_sensitive_arg(arg) { + if let Some((key, _)) = arg.as_str().split_once('=') { + let trimmed = key.trim(); + return CommandArg::new(format!("{trimmed}=***REDACTED***")); + } + return CommandArg::new(String::from("***REDACTED***")); + } + arg.clone() +} + +/// Redact sensitive information from all `args`. +/// +/// # Examples +/// ``` +/// # #[cfg(doctest)] +/// # use netsuke::runner::doc::{CommandArg, redact_sensitive_args}; +/// let args = vec![ +/// CommandArg::new("ninja".into()), +/// CommandArg::new("token=abc".into()), +/// ]; +/// let redacted = redact_sensitive_args(&args); +/// assert_eq!(redacted[1].as_str(), "token=***REDACTED***"); +/// ``` +#[must_use] +pub fn redact_sensitive_args(args: &[CommandArg]) -> Vec { + args.iter().map(redact_argument).collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn contains_sensitive_keyword_only_flags_known_keys() { + let token = CommandArg::new(String::from("token=abc")); + assert!(contains_sensitive_keyword(&token)); + + let positional = CommandArg::new(String::from("secrets.yml")); + assert!(!contains_sensitive_keyword(&positional)); + + let path_arg = CommandArg::new(String::from("path=/tmp/secrets.yml")); + assert!(!contains_sensitive_keyword(&path_arg)); + + let spaced = CommandArg::new(String::from(" PASSWORD = value ")); + assert!(contains_sensitive_keyword(&spaced)); + } + + #[test] + fn redact_argument_preserves_non_sensitive_pairs() { + let redacted = redact_argument(&CommandArg::new(String::from("auth = token123"))); + assert_eq!(redacted.as_str(), "auth=***REDACTED***"); + + let untouched = redact_argument(&CommandArg::new(String::from("path=/var/secrets"))); + assert_eq!(untouched.as_str(), "path=/var/secrets"); + + let positional = redact_argument(&CommandArg::new(String::from("secret"))); + assert_eq!(positional.as_str(), "secret"); + } +}