Merge pull request #1224 from SteveL-MSFT/exe-validation

Enable resource manifests to have relative paths

Author: SteveL-MSFT

dsc/tests/dsc_discovery.tests.ps1

@@ -245,4 +245,55 @@ Describe 'tests for resource discovery' {
             $env:DSC_RESOURCE_PATH = $oldPath
         }
     }
+
+    It 'Resource manifest using relative path to exe: <path>' -TestCases @(
+        @{ path = '../dscecho'; success = $true }
+        @{ path = '../foo/dscecho'; success = $false }
+    ) {
+        param($path, $success)
+        $manifest = @"
+{
+    "`$schema": "https://aka.ms/dsc/schemas/v3/bundled/resource/manifest.json",
+    "type": "Microsoft.DSC.Debug/Echo",
+    "version": "1.0.0",
+    "description": "Echo resource for testing and debugging purposes",
+    "get": {
+        "executable": "$path",
+        "args": [
+            {
+                "jsonInputArg": "--input",
+                "mandatory": true
+            }
+        ]
+    },
+    "schema": {
+        "command": {
+            "executable": "$path"
+        }
+    }
+}
+"@
+        $dscEcho = Get-Command dscecho -ErrorAction Stop
+        # copy to testdrive
+        Copy-Item -Path "$($dscEcho.Source)" -Destination $testdrive
+        # create manifest in subfolder
+        $subfolder = Join-Path $testdrive 'subfolder'
+        New-Item -Path $subfolder -ItemType Directory -Force | Out-Null
+        Set-Content -Path (Join-Path $subfolder 'test.dsc.resource.json') -Value $manifest
+
+        try {
+            $env:DSC_RESOURCE_PATH = $subfolder
+            $out = dsc resource get -r 'Microsoft.DSC.Debug/Echo' -i '{"output":"RelativePathTest"}' 2> "$testdrive/error.txt" | ConvertFrom-Json
+            if ($success) {
+                $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw -Path "$testdrive/error.txt")
+                $out.actualState.output | Should -BeExactly 'RelativePathTest'
+            } else {
+                $LASTEXITCODE | Should -Be 2 -Because (Get-Content -Raw -Path "$testdrive/error.txt")
+                (Get-Content -Raw -Path "$testdrive/error.txt") | Should -Match "ERROR.*?Executable '\.\./foo/dscecho(\.exe)?' not found"
+            }
+        }
+        finally {
+            $env:DSC_RESOURCE_PATH = $null
+        }
+    }
 }

dsc/tests/dsc_extension_discover.tests.ps1

@@ -138,7 +138,7 @@ Describe 'Discover extension tests' {
             $out = dsc -l warn resource list 2> $TestDrive/error.log | ConvertFrom-Json
             $LASTEXITCODE | Should -Be 0
             $out.Count | Should -BeGreaterThan 0
-            (Get-Content -Path "$TestDrive/error.log" -Raw) | Should -BeLike "*WARN Extension 'Microsoft.Windows.Appx/Discover' failed to discover resources: Command: Operation program not found for executable 'powershell'*" -Because (Get-Content -Path "$TestDrive/error.log" -Raw | Out-String)
+            (Get-Content -Path "$TestDrive/error.log" -Raw) | Should -BeLike "*WARN Extension 'Microsoft.Windows.Appx/Discover' failed to discover resources: Command: Operation Executable 'powershell' not found*" -Because (Get-Content -Path "$TestDrive/error.log" -Raw | Out-String)
         } finally {
             $env:PATH = $oldPath
         }

lib/dsc-lib/locales/en-us.toml

@@ -698,3 +698,5 @@ failedToGetExePath = "Can't get 'dsc' executable path"
 settingNotFound = "Setting '%{name}' not found"
 failedToAbsolutizePath = "Failed to absolutize path '%{path}'"
 invalidExitCodeKey = "Invalid exit code key '%{key}'"
+executableNotFoundInWorkingDirectory = "Executable '%{executable}' not found with working directory '%{cwd}'"
+executableNotFound = "Executable '%{executable}' not found"

lib/dsc-lib/src/discovery/command_discovery.rs

@@ -20,14 +20,13 @@ use serde::Deserialize;
 use std::{collections::{BTreeMap, HashMap, HashSet}, sync::{LazyLock, RwLock}};
 use std::env;
 use std::ffi::OsStr;
-use std::fs;
+use std::fs::{create_dir_all, read, read_to_string, write};
 use std::path::{Path, PathBuf};
 use std::str::FromStr;
 use tracing::{debug, info, trace, warn};
-use which::which;
 
 use crate::util::get_setting;
-use crate::util::get_exe_path;
+use crate::util::{canonicalize_which, get_exe_path};
 
 const DSC_EXTENSION_EXTENSIONS: [&str; 3] = [".dsc.extension.json", ".dsc.extension.yaml", ".dsc.extension.yml"];
 const DSC_MANIFEST_LIST_EXTENSIONS: [&str; 3] = [".dsc.manifests.json", ".dsc.manifests.yaml", ".dsc.manifests.yml"];
@@ -621,7 +620,7 @@ fn insert_resource(resources: &mut BTreeMap<String, Vec<DscResource>>, resource:
 ///
 /// * Returns a `DscError` if the manifest could not be loaded or parsed.
 pub fn load_manifest(path: &Path) -> Result<Vec<ImportedManifest>, DscError> {
-    let contents = fs::read_to_string(path)?;
+    let contents = read_to_string(path)?;
     let file_name_lowercase = path.file_name().and_then(OsStr::to_str).unwrap_or("").to_lowercase();
     let extension_is_json = path.extension().is_some_and(|ext| ext.eq_ignore_ascii_case("json"));
     if DSC_RESOURCE_EXTENSIONS.iter().any(|ext| file_name_lowercase.ends_with(ext)) {
@@ -711,38 +710,38 @@ fn load_resource_manifest(path: &Path, manifest: &ResourceManifest) -> Result<Ds
 
     let mut capabilities: Vec<Capability> = vec![];
     if let Some(get) = &manifest.get {
-        verify_executable(&manifest.resource_type, "get", &get.executable);
+        verify_executable(&manifest.resource_type, "get", &get.executable, path.parent().unwrap());
         capabilities.push(Capability::Get);
     }
     if let Some(set) = &manifest.set {
-        verify_executable(&manifest.resource_type, "set", &set.executable);
+        verify_executable(&manifest.resource_type, "set", &set.executable, path.parent().unwrap());
         capabilities.push(Capability::Set);
         if set.handles_exist == Some(true) {
             capabilities.push(Capability::SetHandlesExist);
         }
     }
     if let Some(what_if) = &manifest.what_if {
-        verify_executable(&manifest.resource_type, "what_if", &what_if.executable);
+        verify_executable(&manifest.resource_type, "what_if", &what_if.executable, path.parent().unwrap());
         capabilities.push(Capability::WhatIf);
     }
     if let Some(test) = &manifest.test {
-        verify_executable(&manifest.resource_type, "test", &test.executable);
+        verify_executable(&manifest.resource_type, "test", &test.executable, path.parent().unwrap());
         capabilities.push(Capability::Test);
     }
     if let Some(delete) = &manifest.delete {
-        verify_executable(&manifest.resource_type, "delete", &delete.executable);
+        verify_executable(&manifest.resource_type, "delete", &delete.executable, path.parent().unwrap());
         capabilities.push(Capability::Delete);
     }
     if let Some(export) = &manifest.export {
-        verify_executable(&manifest.resource_type, "export", &export.executable);
+        verify_executable(&manifest.resource_type, "export", &export.executable, path.parent().unwrap());
         capabilities.push(Capability::Export);
     }
     if let Some(resolve) = &manifest.resolve {
-        verify_executable(&manifest.resource_type, "resolve", &resolve.executable);
+        verify_executable(&manifest.resource_type, "resolve", &resolve.executable, path.parent().unwrap());
         capabilities.push(Capability::Resolve);
     }
     if let Some(SchemaKind::Command(command)) = &manifest.schema {
-        verify_executable(&manifest.resource_type, "schema", &command.executable);
+        verify_executable(&manifest.resource_type, "schema", &command.executable, path.parent().unwrap());
     }
 
     let resource = DscResource {
@@ -768,15 +767,15 @@ fn load_extension_manifest(path: &Path, manifest: &ExtensionManifest) -> Result<
 
     let mut capabilities: Vec<dscextension::Capability> = vec![];
     if let Some(discover) = &manifest.discover {
-        verify_executable(&manifest.r#type, "discover", &discover.executable);
+        verify_executable(&manifest.r#type, "discover", &discover.executable, path.parent().unwrap());
         capabilities.push(dscextension::Capability::Discover);
     }
     if let Some(secret) = &manifest.secret {
-        verify_executable(&manifest.r#type, "secret", &secret.executable);
+        verify_executable(&manifest.r#type, "secret", &secret.executable, path.parent().unwrap());
         capabilities.push(dscextension::Capability::Secret);
     }
     let import_extensions = if let Some(import) = &manifest.import {
-        verify_executable(&manifest.r#type, "import", &import.executable);
+        verify_executable(&manifest.r#type, "import", &import.executable, path.parent().unwrap());
         capabilities.push(dscextension::Capability::Import);
         if import.file_extensions.is_empty() {
             warn!("{}", t!("discovery.commandDiscovery.importExtensionsEmpty", extension = manifest.r#type));
@@ -803,8 +802,8 @@ fn load_extension_manifest(path: &Path, manifest: &ExtensionManifest) -> Result<
     Ok(extension)
 }
 
-fn verify_executable(resource: &str, operation: &str, executable: &str) {
-    if which(executable).is_err() {
+fn verify_executable(resource: &str, operation: &str, executable: &str, directory: &Path) {
+    if canonicalize_which(executable, Some(directory.to_string_lossy().as_ref())).is_err() {
         info!("{}", t!("discovery.commandDiscovery.executableNotFound", resource = resource, operation = operation, executable = executable));
     }
 }
@@ -839,8 +838,8 @@ fn save_adapted_resources_lookup_table(lookup_table: &HashMap<String, String>)
 
         let path = std::path::Path::new(&file_path);
         if let Some(prefix) = path.parent() {
-            if fs::create_dir_all(prefix).is_ok()  {
-                if fs::write(file_path.clone(), lookup_table_json).is_err() {
+            if create_dir_all(prefix).is_ok()  {
+                if write(file_path.clone(), lookup_table_json).is_err() {
                     info!("Unable to write lookup_table file {file_path:?}");
                 }
             } else {
@@ -858,7 +857,7 @@ fn load_adapted_resources_lookup_table() -> HashMap<String, String>
 {
     let file_path = get_lookup_table_file_path();
 
-    let lookup_table: HashMap<String, String> = match fs::read(file_path.clone()){
+    let lookup_table: HashMap<String, String> = match read(file_path.clone()){
         Ok(data) => { serde_json::from_slice(&data).unwrap_or_default() },
         Err(_) => { HashMap::new() }
     };

lib/dsc-lib/src/dscerror.rs

@@ -26,6 +26,9 @@ pub enum DscError {
     #[error("{t} '{0}' [{t2} {1}] {t3}: {2}", t = t!("dscerror.commandResource"), t2 = t!("dscerror.exitCode"), t3 = t!("dscerror.manifestDescription"))]
     CommandExitFromManifest(String, i32, String),
 
+    #[error("{0}")]
+    CommandNotFound(String),
+
     #[error("{t} {0} {t2} '{1}'", t = t!("dscerror.commandOperation"), t2 = t!("dscerror.forExecutable"))]
     CommandOperation(String, String),
 

lib/dsc-lib/src/dscresources/command_resource.rs

@@ -7,7 +7,7 @@ use rust_i18n::t;
 use serde::Deserialize;
 use serde_json::{Map, Value};
 use std::{collections::HashMap, env, process::Stdio};
-use crate::configure::{config_doc::ExecutionKind, config_result::{ResourceGetResult, ResourceTestResult}};
+use crate::{configure::{config_doc::ExecutionKind, config_result::{ResourceGetResult, ResourceTestResult}}, util::canonicalize_which};
 use crate::dscerror::DscError;
 use super::{dscresource::{get_diff, redact}, invoke_result::{ExportResult, GetResult, ResolveResult, SetResult, TestResult, ValidateResult, ResourceGetResponse, ResourceSetResponse, ResourceTestResponse, get_in_desired_state}, resource_manifest::{ArgKind, InputKind, Kind, ResourceManifest, ReturnKind, SchemaKind}};
 use tracing::{error, warn, info, debug, trace};
@@ -763,6 +763,7 @@ fn convert_hashmap_string_keys_to_i32(input: Option<&HashMap<String, String>>) -
 #[allow(clippy::implicit_hasher)]
 pub fn invoke_command(executable: &str, args: Option<Vec<String>>, input: Option<&str>, cwd: Option<&str>, env: Option<HashMap<String, String>>, exit_codes: Option<&HashMap<String, String>>) -> Result<(i32, String, String), DscError> {
     let exit_codes = convert_hashmap_string_keys_to_i32(exit_codes)?;
+    let executable = canonicalize_which(executable, cwd)?;
 
     tokio::runtime::Builder::new_multi_thread().enable_all().build().unwrap().block_on(
         async {
@@ -771,7 +772,7 @@ pub fn invoke_command(executable: &str, args: Option<Vec<String>>, input: Option
                 trace!("{}", t!("dscresources.commandResource.commandCwd", cwd = cwd));
             }
 
-            match run_process_async(executable, args, input, cwd, env, exit_codes.as_ref()).await {
+            match run_process_async(&executable, args, input, cwd, env, exit_codes.as_ref()).await {
                 Ok((code, stdout, stderr)) => {
                     Ok((code, stdout, stderr))
                 },

lib/dsc-lib/src/util.rs

@@ -6,12 +6,13 @@ use rust_i18n::t;
 use serde_json::Value;
 use std::{
     fs,
-    fs::File,
+    fs::{canonicalize, File},
     io::BufReader,
     path::{Path, PathBuf},
     env,
 };
 use tracing::debug;
+use which::which;
 
 pub struct DscSettingValue {
     pub setting:  Value,
@@ -232,6 +233,25 @@ pub fn resource_id(type_name: &str, name: &str) -> String {
     result
 }
 
+pub fn canonicalize_which(executable: &str, cwd: Option<&str>) -> Result<String, DscError> {
+    // Use PathBuf to handle path separators robustly
+    let mut executable_path = PathBuf::from(executable);
+    if cfg!(target_os = "windows") && executable_path.extension().is_none() {
+        executable_path.set_extension("exe");
+    }
+    if which(executable).is_err() {
+        if let Some(cwd) = cwd {
+            let cwd_path = Path::new(cwd);
+            if let Ok(canonical_path) = canonicalize(cwd_path.join(&executable_path)) {
+                return Ok(canonical_path.to_string_lossy().to_string());
+            }
+            return Err(DscError::CommandOperation(t!("util.executableNotFoundInWorkingDirectory", executable = &executable, cwd = cwd_path.to_string_lossy()).to_string(), executable_path.to_string_lossy().to_string()));
+        }
+        return Err(DscError::CommandOperation(t!("util.executableNotFound", executable = &executable).to_string(), executable.to_string()));
+    }
+    Ok(executable.to_string())
+}
+
 #[macro_export]
 macro_rules! locked_is_empty {
     ($lockable:expr) => {{