From e192e0a50ad9f6582fdb2c98aa6fe2e7ed513875 Mon Sep 17 00:00:00 2001 From: "Steve Lee (POWERSHELL HE/HIM) (from Dev Box)" Date: Thu, 13 Aug 2026 13:51:52 -0700 Subject: [PATCH 1/3] Add Windows environment variable resource Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- Cargo.lock | 11 + Cargo.toml | 3 + .../environment_variable/.project.data.json | 14 + resources/environment_variable/Cargo.toml | 18 ++ .../environment_variable.dsc.resource.json | 124 ++++++++ .../environment_variable/locales/en-us.toml | 31 ++ .../environment_variable/src/environment.rs | 278 ++++++++++++++++++ resources/environment_variable/src/main.rs | 112 +++++++ resources/environment_variable/src/types.rs | 169 +++++++++++ .../tests/environment_variable_get.tests.ps1 | 93 ++++++ .../tests/environment_variable_set.tests.ps1 | 212 +++++++++++++ 11 files changed, 1065 insertions(+) create mode 100644 resources/environment_variable/.project.data.json create mode 100644 resources/environment_variable/Cargo.toml create mode 100644 resources/environment_variable/environment_variable.dsc.resource.json create mode 100644 resources/environment_variable/locales/en-us.toml create mode 100644 resources/environment_variable/src/environment.rs create mode 100644 resources/environment_variable/src/main.rs create mode 100644 resources/environment_variable/src/types.rs create mode 100644 resources/environment_variable/tests/environment_variable_get.tests.ps1 create mode 100644 resources/environment_variable/tests/environment_variable_set.tests.ps1 diff --git a/Cargo.lock b/Cargo.lock index 80f251607..b2137d8d9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1067,6 +1067,17 @@ dependencies = [ "encoding_rs", ] +[[package]] +name = "environment_variable" +version = "0.1.0" +dependencies = [ + "dsc-lib-registry", + "dsc-lib-security_context", + "rust-i18n", + "serde", + "serde_json", +] + [[package]] name = "equivalent" version = "1.0.2" diff --git a/Cargo.toml b/Cargo.toml index 65eccb184..5a9ef1bac 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,6 +25,7 @@ members = [ "resources/windows_firewall", "resources/windows_service", "resources/WindowsUpdate", + "resources/environment_variable", "tools/dsctest", "tools/test_group_resource", "xtask", @@ -57,6 +58,7 @@ default-members = [ "resources/windows_firewall", "resources/windows_service", "resources/WindowsUpdate", + "resources/environment_variable", "tools/dsctest", "tools/test_group_resource", "xtask", @@ -90,6 +92,7 @@ Windows = [ "resources/windows_firewall", "resources/windows_service", "resources/WindowsUpdate", + "resources/environment_variable", "tools/dsctest", "tools/test_group_resource", "xtask", diff --git a/resources/environment_variable/.project.data.json b/resources/environment_variable/.project.data.json new file mode 100644 index 000000000..c6d1a4526 --- /dev/null +++ b/resources/environment_variable/.project.data.json @@ -0,0 +1,14 @@ +{ + "Name": "environment_variable", + "Kind": "Resource", + "IsRust": true, + "SupportedPlatformOS": "Windows", + "Binaries": [ + "environment_variable" + ], + "CopyFiles": { + "Windows": [ + "environment_variable.dsc.resource.json" + ] + } +} diff --git a/resources/environment_variable/Cargo.toml b/resources/environment_variable/Cargo.toml new file mode 100644 index 000000000..bf49187a4 --- /dev/null +++ b/resources/environment_variable/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "environment_variable" +version = "0.1.0" +edition = "2024" + +[package.metadata.i18n] +available-locales = ["en-us"] +default-locale = "en-us" +load-path = "locales" + +[dependencies] +rust-i18n = { workspace = true } +serde = { workspace = true } +serde_json = { workspace = true } + +[target.'cfg(windows)'.dependencies] +dsc-lib-registry = { workspace = true } +dsc-lib-security_context = { workspace = true } diff --git a/resources/environment_variable/environment_variable.dsc.resource.json b/resources/environment_variable/environment_variable.dsc.resource.json new file mode 100644 index 000000000..dda741366 --- /dev/null +++ b/resources/environment_variable/environment_variable.dsc.resource.json @@ -0,0 +1,124 @@ +{ + "$schema": "https://aka.ms/dsc/schemas/v3/bundled/resource/manifest.json", + "type": "Microsoft.Windows/EnvironmentVariableList", + "description": "Manage user and machine environment variables stored in the Windows registry.", + "tags": [ + "Windows", + "Environment" + ], + "version": "0.1.0", + "get": { + "executable": "environment_variable", + "args": [ + "get", + { + "jsonInputArg": "--input", + "mandatory": true + } + ] + }, + "set": { + "executable": "environment_variable", + "args": [ + "set", + { + "jsonInputArg": "--input", + "mandatory": true + } + ], + "implementsPretest": false, + "handlesExist": true, + "return": "state" + }, + "exitCodes": { + "0": "Success", + "1": "Invalid arguments", + "2": "Invalid input", + "3": "Environment variable resource error", + "4": "Elevation required: Setting or removing AllUsers environment variables requires an elevated process" + }, + "schema": { + "embedded": { + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "Windows Environment Variable List", + "description": "Manage user and machine environment variables stored in the Windows registry.", + "type": "object", + "additionalProperties": false, + "required": [ + "environmentVariables" + ], + "properties": { + "environmentVariables": { + "type": "array", + "title": "Environment variables", + "description": "The environment variables to get or set.", + "minItems": 1, + "items": { + "type": "object", + "additionalProperties": false, + "required": [ + "name" + ], + "not": { + "required": [ + "value", + "pathValue" + ] + }, + "properties": { + "scope": { + "type": "string", + "title": "Scope", + "description": "The registry scope for the environment variable.", + "default": "CurrentUser", + "enum": [ + "AllUsers", + "CurrentUser" + ] + }, + "name": { + "type": "string", + "title": "Name", + "description": "The environment variable name.", + "minLength": 1 + }, + "value": { + "type": "string", + "title": "Value", + "description": "The environment variable value." + }, + "pathValue": { + "type": "array", + "title": "Path value", + "description": "The semicolon-delimited environment variable value represented as path entries.", + "items": { + "type": "string", + "minLength": 1, + "pattern": "^[^;]+$" + } + }, + "pathAction": { + "type": "string", + "title": "Path action", + "description": "How pathValue entries are combined with the current value.", + "writeOnly": true, + "default": "clobber", + "enum": [ + "prepend", + "append", + "clobber" + ] + }, + "_exist": { + "type": "boolean", + "title": "Exists", + "description": "Whether the environment variable should exist. Set to false to remove it.", + "default": true + } + } + } + } + } + } + } +} diff --git a/resources/environment_variable/locales/en-us.toml b/resources/environment_variable/locales/en-us.toml new file mode 100644 index 000000000..b3b3251fc --- /dev/null +++ b/resources/environment_variable/locales/en-us.toml @@ -0,0 +1,31 @@ +_version = 1 + +[main] +missingOperation = "Missing operation. Usage: environment_variable get --input | set --input " +unknownOperation = "Unknown operation: '%{operation}'. Expected: get or set" +missingInput = "Missing --input argument" +missingInputValue = "Missing value for --input argument" +invalidJson = "Invalid JSON input: %{error}" +serializeError = "Failed to serialize resource output: %{error}" +windowsOnly = "The Microsoft.Windows/EnvironmentVariableList resource is only supported on Windows" +registryError = "Failed to access environment variable '%{name}' in scope '%{scope}': %{error}" + +[validation] +emptyList = "The environmentVariables array must contain at least one environment variable" +emptyName = "Environment variable name must not be empty" +invalidName = "Environment variable name '%{name}' contains an invalid null character" +duplicate = "Environment variable '%{name}' is specified more than once in scope '%{scope}'" +valueConflict = "Environment variable '%{name}' cannot specify both value and pathValue" +pathActionWithoutValue = "Environment variable '%{name}' can only specify pathAction with pathValue" +invalidPathEntry = "Environment variable '%{name}' has a pathValue entry that is empty or contains a semicolon or null character" +missingValue = "Environment variable '%{name}' must specify value or pathValue when _exist is true" + +[get] +readError = "Failed to read environment variable '%{name}' in scope '%{scope}': %{error}" +unsupportedType = "Environment variable '%{name}' in scope '%{scope}' uses an unsupported registry value type; expected REG_SZ or REG_EXPAND_SZ" + +[set] +elevationRequired = "Setting or removing AllUsers environment variables requires elevation. Run DSC from an elevated process." +readError = "Failed to read environment variable '%{name}' in scope '%{scope}' before setting it: %{error}" +writeError = "Failed to set environment variable '%{name}' in scope '%{scope}': %{error}" +removeError = "Failed to remove environment variable '%{name}' in scope '%{scope}': %{error}" diff --git a/resources/environment_variable/src/environment.rs b/resources/environment_variable/src/environment.rs new file mode 100644 index 000000000..952d51356 --- /dev/null +++ b/resources/environment_variable/src/environment.rs @@ -0,0 +1,278 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +use crate::types::{EnvironmentVariable, EnvironmentVariableList, PathAction, Scope}; +use dsc_lib_registry::{RegistryHelper, config::RegistryValueData}; +use dsc_lib_security_context::{SecurityContext, get_security_context}; +use rust_i18n::t; +use std::collections::HashSet; + +const CURRENT_USER_KEY: &str = r"HKCU\Environment"; +const ALL_USERS_KEY: &str = r"HKLM\SYSTEM\CurrentControlSet\Control\Session Manager\Environment"; + +#[derive(Debug)] +pub enum EnvironmentError { + ElevationRequired, + Resource(String), +} + +impl EnvironmentError { + pub fn is_elevation_required(&self) -> bool { + matches!(self, Self::ElevationRequired) + } +} + +impl std::fmt::Display for EnvironmentError { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::ElevationRequired => formatter.write_str(&t!("set.elevationRequired")), + Self::Resource(message) => formatter.write_str(message), + } + } +} + +impl std::error::Error for EnvironmentError {} + +pub fn get_variables( + input: &EnvironmentVariableList, +) -> Result { + let environment_variables = input + .environment_variables + .iter() + .map(get_variable) + .collect::, _>>()?; + + Ok(EnvironmentVariableList { + environment_variables, + }) +} + +pub fn set_variables( + input: &EnvironmentVariableList, +) -> Result { + if input + .environment_variables + .iter() + .any(|variable| variable.scope == Scope::AllUsers) + && get_security_context() != SecurityContext::Admin + { + return Err(EnvironmentError::ElevationRequired); + } + + let mut environment_variables = Vec::with_capacity(input.environment_variables.len()); + for variable in &input.environment_variables { + let helper = registry_helper(variable, None)?; + if !variable.exist.unwrap_or(true) { + helper + .remove() + .map_err(|error| operation_error("set.removeError", variable, &error))?; + environment_variables.push(EnvironmentVariable { + scope: variable.scope, + name: variable.name.clone(), + value: None, + path_value: None, + path_action: None, + exist: Some(false), + }); + continue; + } + + let current_data = helper + .get() + .map_err(|error| operation_error("set.readError", variable, &error))? + .value_data; + let desired_value = desired_value(variable, current_data.as_ref()); + let value_data = registry_data(&desired_value, current_data.as_ref()); + registry_helper(variable, Some(value_data))? + .set() + .map_err(|error| operation_error("set.writeError", variable, &error))?; + environment_variables.push(get_variable(variable)?); + } + + Ok(EnvironmentVariableList { + environment_variables, + }) +} + +fn get_variable(variable: &EnvironmentVariable) -> Result { + let state = registry_helper(variable, None)? + .get() + .map_err(|error| operation_error("get.readError", variable, &error))?; + + if state.exist == Some(false) { + return Ok(EnvironmentVariable { + scope: variable.scope, + name: variable.name.clone(), + value: None, + path_value: None, + path_action: None, + exist: Some(false), + }); + } + + let value = match state.value_data { + Some(RegistryValueData::String(value) | RegistryValueData::ExpandString(value)) => value, + Some(_) => { + return Err(EnvironmentError::Resource( + t!( + "get.unsupportedType", + name = variable.name.as_str(), + scope = variable.scope.to_string() + ) + .to_string(), + )); + } + None => String::new(), + }; + + let (value, path_value) = if variable.path_value.is_some() { + (None, Some(split_path(&value))) + } else { + (Some(value), None) + }; + + Ok(EnvironmentVariable { + scope: variable.scope, + name: variable.name.clone(), + value, + path_value, + path_action: None, + exist: Some(true), + }) +} + +fn registry_helper( + variable: &EnvironmentVariable, + value_data: Option, +) -> Result { + RegistryHelper::new( + key_path(variable.scope), + Some(variable.name.clone()), + value_data, + ) + .map_err(|error| operation_error("main.registryError", variable, &error)) +} + +fn key_path(scope: Scope) -> &'static str { + match scope { + Scope::AllUsers => ALL_USERS_KEY, + Scope::CurrentUser => CURRENT_USER_KEY, + } +} + +fn desired_value( + variable: &EnvironmentVariable, + current_data: Option<&RegistryValueData>, +) -> String { + if let Some(value) = &variable.value { + return value.clone(); + } + + let desired = variable.path_value.as_deref().unwrap_or_default(); + let existing = match current_data { + Some(RegistryValueData::String(value) | RegistryValueData::ExpandString(value)) => { + split_path(value) + } + _ => Vec::new(), + }; + + merge_path(&existing, desired, variable.path_action.unwrap_or_default()).join(";") +} + +fn registry_data(value: &str, current_data: Option<&RegistryValueData>) -> RegistryValueData { + if matches!(current_data, Some(RegistryValueData::ExpandString(_))) || value.contains('%') { + RegistryValueData::ExpandString(value.to_string()) + } else { + RegistryValueData::String(value.to_string()) + } +} + +fn split_path(value: &str) -> Vec { + value + .split(';') + .filter(|entry| !entry.is_empty()) + .map(str::to_string) + .collect() +} + +fn merge_path(existing: &[String], desired: &[String], action: PathAction) -> Vec { + let mut values = match action { + PathAction::Prepend => desired.iter().chain(existing).cloned().collect::>(), + PathAction::Append => { + let desired_keys = desired + .iter() + .map(|entry| entry.to_lowercase()) + .collect::>(); + existing + .iter() + .filter(|entry| !desired_keys.contains(&entry.to_lowercase())) + .chain(desired) + .cloned() + .collect::>() + } + PathAction::Clobber => desired.to_vec(), + }; + + let mut seen = HashSet::new(); + values.retain(|entry| seen.insert(entry.to_lowercase())); + values +} + +fn operation_error( + key: &str, + variable: &EnvironmentVariable, + error: &impl std::fmt::Display, +) -> EnvironmentError { + EnvironmentError::Resource( + t!( + key, + name = variable.name.as_str(), + scope = variable.scope.to_string(), + error = error.to_string() + ) + .to_string(), + ) +} + +#[cfg(test)] +mod tests { + use super::{merge_path, split_path}; + use crate::types::PathAction; + + #[test] + fn prepends_and_deduplicates_case_insensitively() { + let existing = vec!["C:\\Existing".to_string(), "C:\\Shared".to_string()]; + let desired = vec!["c:\\shared".to_string(), "C:\\New".to_string()]; + + assert_eq!( + merge_path(&existing, &desired, PathAction::Prepend), + vec!["c:\\shared", "C:\\New", "C:\\Existing"] + ); + } + + #[test] + fn appends_entries_at_the_end() { + let existing = vec!["C:\\Shared".to_string(), "C:\\Existing".to_string()]; + let desired = vec!["c:\\shared".to_string(), "C:\\New".to_string()]; + + assert_eq!( + merge_path(&existing, &desired, PathAction::Append), + vec!["C:\\Existing", "c:\\shared", "C:\\New"] + ); + } + + #[test] + fn clobber_deduplicates_desired_entries() { + let desired = vec!["C:\\One".to_string(), "c:\\one".to_string()]; + + assert_eq!( + merge_path(&[], &desired, PathAction::Clobber), + vec!["C:\\One"] + ); + } + + #[test] + fn splitting_omits_empty_path_segments() { + assert_eq!(split_path("C:\\One;;C:\\Two;"), vec!["C:\\One", "C:\\Two"]); + } +} diff --git a/resources/environment_variable/src/main.rs b/resources/environment_variable/src/main.rs new file mode 100644 index 000000000..20b79cd7a --- /dev/null +++ b/resources/environment_variable/src/main.rs @@ -0,0 +1,112 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +mod types; + +#[cfg(windows)] +mod environment; + +use rust_i18n::t; +use std::process::exit; +use types::{EnvironmentVariableList, Operation}; + +rust_i18n::i18n!("locales", fallback = "en-us"); + +const EXIT_SUCCESS: i32 = 0; +const EXIT_INVALID_ARGS: i32 = 1; +const EXIT_INVALID_INPUT: i32 = 2; +const EXIT_RESOURCE_ERROR: i32 = 3; +const EXIT_ELEVATION_REQUIRED: i32 = 4; + +fn write_error(message: &str) { + eprintln!("{}", serde_json::json!({ "error": message })); +} + +fn print_json(value: &impl serde::Serialize) { + match serde_json::to_string(value) { + Ok(json) => println!("{json}"), + Err(error) => { + write_error(&t!("main.serializeError", error = error.to_string())); + exit(EXIT_RESOURCE_ERROR); + } + } +} + +fn require_input(input_json: Option, operation: Operation) -> EnvironmentVariableList { + let Some(json) = input_json else { + write_error(&t!("main.missingInput")); + exit(EXIT_INVALID_ARGS); + }; + + let input: EnvironmentVariableList = match serde_json::from_str(&json) { + Ok(value) => value, + Err(error) => { + write_error(&t!("main.invalidJson", error = error.to_string())); + exit(EXIT_INVALID_INPUT); + } + }; + + if let Err(error) = input.validate(operation) { + write_error(&error); + exit(EXIT_INVALID_INPUT); + } + + input +} + +#[cfg(not(windows))] +fn main() { + write_error(&t!("main.windowsOnly")); + exit(EXIT_RESOURCE_ERROR); +} + +#[cfg(windows)] +fn main() { + let args: Vec = std::env::args().collect(); + if args.len() < 2 { + write_error(&t!("main.missingOperation")); + exit(EXIT_INVALID_ARGS); + } + + let operation = args[1].as_str(); + let input_json = parse_input_arg(&args); + + let result = match operation { + "get" => environment::get_variables(&require_input(input_json, Operation::Get)), + "set" => environment::set_variables(&require_input(input_json, Operation::Set)), + _ => { + write_error(&t!("main.unknownOperation", operation = operation)); + exit(EXIT_INVALID_ARGS); + } + }; + + match result { + Ok(value) => { + print_json(&value); + exit(EXIT_SUCCESS); + } + Err(error) => { + write_error(&error.to_string()); + exit(if error.is_elevation_required() { + EXIT_ELEVATION_REQUIRED + } else { + EXIT_RESOURCE_ERROR + }); + } + } +} + +fn parse_input_arg(args: &[String]) -> Option { + let mut index = 2; + while index < args.len() { + if args[index] == "--input" || args[index] == "-i" { + if index + 1 < args.len() { + return Some(args[index + 1].clone()); + } + write_error(&t!("main.missingInputValue")); + exit(EXIT_INVALID_ARGS); + } + index += 1; + } + None +} diff --git a/resources/environment_variable/src/types.rs b/resources/environment_variable/src/types.rs new file mode 100644 index 000000000..4cb708349 --- /dev/null +++ b/resources/environment_variable/src/types.rs @@ -0,0 +1,169 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +use rust_i18n::t; +use serde::{Deserialize, Serialize}; +use std::collections::HashSet; + +#[derive(Debug, Clone, Copy)] +pub enum Operation { + Get, + Set, +} + +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash, Serialize, Deserialize)] +pub enum Scope { + AllUsers, + #[default] + CurrentUser, +} + +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub enum PathAction { + Prepend, + Append, + #[default] + Clobber, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct EnvironmentVariableList { + pub environment_variables: Vec, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct EnvironmentVariable { + #[serde(default)] + pub scope: Scope, + pub name: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub value: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub path_value: Option>, + #[serde(default, skip_serializing)] + pub path_action: Option, + #[serde(rename = "_exist", skip_serializing_if = "Option::is_none")] + pub exist: Option, +} + +impl EnvironmentVariableList { + pub fn validate(&self, operation: Operation) -> Result<(), String> { + if self.environment_variables.is_empty() { + return Err(t!("validation.emptyList").to_string()); + } + + let mut identities = HashSet::new(); + for variable in &self.environment_variables { + variable.validate(operation)?; + let identity = (variable.scope, variable.name.to_lowercase()); + if !identities.insert(identity) { + return Err(t!( + "validation.duplicate", + name = variable.name.as_str(), + scope = variable.scope.to_string() + ) + .to_string()); + } + } + + Ok(()) + } +} + +impl EnvironmentVariable { + fn validate(&self, operation: Operation) -> Result<(), String> { + if self.name.is_empty() { + return Err(t!("validation.emptyName").to_string()); + } + if self.name.contains('\0') { + return Err(t!("validation.invalidName", name = self.name.as_str()).to_string()); + } + if self.value.is_some() && self.path_value.is_some() { + return Err(t!("validation.valueConflict", name = self.name.as_str()).to_string()); + } + if self.path_action.is_some() && self.path_value.is_none() { + return Err(t!( + "validation.pathActionWithoutValue", + name = self.name.as_str() + ) + .to_string()); + } + if let Some(entries) = &self.path_value + && entries + .iter() + .any(|entry| entry.is_empty() || entry.contains(';') || entry.contains('\0')) + { + return Err(t!("validation.invalidPathEntry", name = self.name.as_str()).to_string()); + } + if matches!(operation, Operation::Set) + && self.exist.unwrap_or(true) + && self.value.is_none() + && self.path_value.is_none() + { + return Err(t!("validation.missingValue", name = self.name.as_str()).to_string()); + } + + Ok(()) + } +} + +impl std::fmt::Display for Scope { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::AllUsers => write!(formatter, "AllUsers"), + Self::CurrentUser => write!(formatter, "CurrentUser"), + } + } +} + +#[cfg(test)] +mod tests { + use super::{EnvironmentVariable, EnvironmentVariableList, Operation, PathAction, Scope}; + + fn variable(name: &str) -> EnvironmentVariable { + EnvironmentVariable { + scope: Scope::CurrentUser, + name: name.to_string(), + value: Some("value".to_string()), + path_value: None, + path_action: None, + exist: None, + } + } + + #[test] + fn rejects_duplicate_identity_case_insensitively() { + let mut second = variable("TEST_NAME"); + second.scope = Scope::CurrentUser; + let list = EnvironmentVariableList { + environment_variables: vec![variable("Test_Name"), second], + }; + + assert!(list.validate(Operation::Set).is_err()); + } + + #[test] + fn allows_same_name_in_different_scopes() { + let mut second = variable("Test_Name"); + second.scope = Scope::AllUsers; + let list = EnvironmentVariableList { + environment_variables: vec![variable("Test_Name"), second], + }; + + assert!(list.validate(Operation::Set).is_ok()); + } + + #[test] + fn rejects_path_action_without_path_value() { + let mut input = variable("Test_Name"); + input.path_action = Some(PathAction::Append); + let list = EnvironmentVariableList { + environment_variables: vec![input], + }; + + assert!(list.validate(Operation::Set).is_err()); + } +} diff --git a/resources/environment_variable/tests/environment_variable_get.tests.ps1 b/resources/environment_variable/tests/environment_variable_get.tests.ps1 new file mode 100644 index 000000000..37e6953cf --- /dev/null +++ b/resources/environment_variable/tests/environment_variable_get.tests.ps1 @@ -0,0 +1,93 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +Describe 'Microsoft.Windows/EnvironmentVariableList get operation' -Skip:(!$IsWindows) { + BeforeAll { + $resourceType = 'Microsoft.Windows/EnvironmentVariableList' + $testName = "DSC_Environment_Get_$([guid]::NewGuid().ToString('N'))" + $testValue = 'C:\DSC\First;C:\DSC\Second' + [Environment]::SetEnvironmentVariable( + $testName, + $testValue, + [EnvironmentVariableTarget]::User) + } + + AfterAll { + [Environment]::SetEnvironmentVariable( + $testName, + $null, + [EnvironmentVariableTarget]::User) + } + + It 'Gets a CurrentUser variable using the default scope' { + $json = @{ + environmentVariables = @( + @{ name = $testName } + ) + } | ConvertTo-Json -Compress -Depth 5 + + $out = $json | dsc resource get -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + $result = ($out | ConvertFrom-Json).actualState.environmentVariables[0] + + $result.scope | Should -BeExactly 'CurrentUser' + $result.name | Should -BeExactly $testName + $result.value | Should -BeExactly $testValue + $result._exist | Should -BeTrue + $result.PSObject.Properties.Name | Should -Not -Contain 'pathAction' + } + + It 'Gets a variable as pathValue when pathValue is requested' { + $json = @{ + environmentVariables = @( + @{ + name = $testName + pathValue = @() + } + ) + } | ConvertTo-Json -Compress -Depth 5 + + $out = $json | dsc resource get -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + $result = ($out | ConvertFrom-Json).actualState.environmentVariables[0] + + ($result.pathValue | ConvertTo-Json -Compress) | + Should -BeExactly '["C:\\DSC\\First","C:\\DSC\\Second"]' + $result.PSObject.Properties.Name | Should -Not -Contain 'value' + } + + It 'Returns _exist false for a missing variable' { + $missingName = "DSC_Environment_Missing_$([guid]::NewGuid().ToString('N'))" + $json = @{ + environmentVariables = @( + @{ name = $missingName } + ) + } | ConvertTo-Json -Compress -Depth 5 + + $out = $json | dsc resource get -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + $result = ($out | ConvertFrom-Json).actualState.environmentVariables[0] + + $result.name | Should -BeExactly $missingName + $result._exist | Should -BeFalse + $result.PSObject.Properties.Name | Should -Not -Contain 'value' + } + + It 'Gets multiple variables in input order' { + $missingName = "DSC_Environment_Missing_$([guid]::NewGuid().ToString('N'))" + $json = @{ + environmentVariables = @( + @{ name = $testName } + @{ name = $missingName } + ) + } | ConvertTo-Json -Compress -Depth 5 + + $out = $json | dsc resource get -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + $result = ($out | ConvertFrom-Json).actualState.environmentVariables + + $result.Count | Should -Be 2 + $result[0].name | Should -BeExactly $testName + $result[1].name | Should -BeExactly $missingName + } +} diff --git a/resources/environment_variable/tests/environment_variable_set.tests.ps1 b/resources/environment_variable/tests/environment_variable_set.tests.ps1 new file mode 100644 index 000000000..f479704bf --- /dev/null +++ b/resources/environment_variable/tests/environment_variable_set.tests.ps1 @@ -0,0 +1,212 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +Describe 'Microsoft.Windows/EnvironmentVariableList set operation' -Skip:(!$IsWindows) { + BeforeDiscovery { + $isAdmin = if ($IsWindows) { + $identity = [Security.Principal.WindowsIdentity]::GetCurrent() + $principal = [Security.Principal.WindowsPrincipal]$identity + $principal.IsInRole([Security.Principal.WindowsBuiltInRole]::Administrator) + } + else { + $false + } + } + + BeforeAll { + $resourceType = 'Microsoft.Windows/EnvironmentVariableList' + $namePrefix = "DSC_Environment_Set_$([guid]::NewGuid().ToString('N'))" + $testNames = @( + "${namePrefix}_Scalar" + "${namePrefix}_Path" + "${namePrefix}_First" + "${namePrefix}_Second" + ) + } + + AfterEach { + foreach ($name in $testNames) { + [Environment]::SetEnvironmentVariable( + $name, + $null, + [EnvironmentVariableTarget]::User) + } + } + + It 'Sets a scalar value with CurrentUser and _exist defaults' { + $json = @{ + environmentVariables = @( + @{ + name = $testNames[0] + value = 'DSC scalar value' + } + ) + } | ConvertTo-Json -Compress -Depth 5 + + $out = $json | dsc resource set -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + $result = ($out | ConvertFrom-Json).afterState.environmentVariables[0] + + $result.scope | Should -BeExactly 'CurrentUser' + $result.value | Should -BeExactly 'DSC scalar value' + $result._exist | Should -BeTrue + [Environment]::GetEnvironmentVariable( + $testNames[0], + [EnvironmentVariableTarget]::User) | Should -BeExactly 'DSC scalar value' + } + + It 'Clobbers a path value by default and removes duplicate entries case-insensitively' { + [Environment]::SetEnvironmentVariable( + $testNames[1], + 'C:\Old', + [EnvironmentVariableTarget]::User) + $json = @{ + environmentVariables = @( + @{ + name = $testNames[1] + pathValue = @('C:\One', 'c:\one', 'C:\Two') + } + ) + } | ConvertTo-Json -Compress -Depth 5 + + $out = $json | dsc resource set -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + $result = ($out | ConvertFrom-Json).afterState.environmentVariables[0] + + ($result.pathValue | ConvertTo-Json -Compress) | + Should -BeExactly '["C:\\One","C:\\Two"]' + [Environment]::GetEnvironmentVariable( + $testNames[1], + [EnvironmentVariableTarget]::User) | Should -BeExactly 'C:\One;C:\Two' + } + + It 'Prepends path entries and moves an existing duplicate to the front' { + [Environment]::SetEnvironmentVariable( + $testNames[1], + 'C:\Existing;C:\Shared', + [EnvironmentVariableTarget]::User) + $json = @{ + environmentVariables = @( + @{ + name = $testNames[1] + pathValue = @('c:\shared', 'C:\New') + pathAction = 'prepend' + } + ) + } | ConvertTo-Json -Compress -Depth 5 + + $out = $json | dsc resource set -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + $result = ($out | ConvertFrom-Json).afterState.environmentVariables[0] + + ($result.pathValue | ConvertTo-Json -Compress) | + Should -BeExactly '["c:\\shared","C:\\New","C:\\Existing"]' + } + + It 'Appends path entries and moves an existing duplicate to the end' { + [Environment]::SetEnvironmentVariable( + $testNames[1], + 'C:\Shared;C:\Existing', + [EnvironmentVariableTarget]::User) + $json = @{ + environmentVariables = @( + @{ + name = $testNames[1] + pathValue = @('c:\shared', 'C:\New') + pathAction = 'append' + } + ) + } | ConvertTo-Json -Compress -Depth 5 + + $out = $json | dsc resource set -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + $result = ($out | ConvertFrom-Json).afterState.environmentVariables[0] + + ($result.pathValue | ConvertTo-Json -Compress) | + Should -BeExactly '["C:\\Existing","c:\\shared","C:\\New"]' + } + + It 'Removes a variable when _exist is false' { + [Environment]::SetEnvironmentVariable( + $testNames[0], + 'remove me', + [EnvironmentVariableTarget]::User) + $json = @{ + environmentVariables = @( + @{ + name = $testNames[0] + _exist = $false + } + ) + } | ConvertTo-Json -Compress -Depth 5 + + $out = $json | dsc resource set -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + $result = ($out | ConvertFrom-Json).afterState.environmentVariables[0] + + $result._exist | Should -BeFalse + [Environment]::GetEnvironmentVariable( + $testNames[0], + [EnvironmentVariableTarget]::User) | Should -BeNullOrEmpty + } + + It 'Sets multiple variables in one request' { + $json = @{ + environmentVariables = @( + @{ + name = $testNames[2] + value = 'first' + } + @{ + name = $testNames[3] + value = 'second' + } + ) + } | ConvertTo-Json -Compress -Depth 5 + + $out = $json | dsc resource set -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + $result = ($out | ConvertFrom-Json).afterState.environmentVariables + + $result.Count | Should -Be 2 + $result[0].value | Should -BeExactly 'first' + $result[1].value | Should -BeExactly 'second' + } + + It 'Rejects value and pathValue together' { + $json = @{ + environmentVariables = @( + @{ + name = $testNames[0] + value = 'value' + pathValue = @('C:\Path') + } + ) + } | ConvertTo-Json -Compress -Depth 5 + + $out = $json | dsc resource set -r $resourceType -f - 2>&1 + $LASTEXITCODE | Should -Not -Be 0 + $out | Should -Match 'value.*pathValue' + } + + It 'Returns an actionable elevation error for AllUsers' -Skip:$isAdmin { + $machineName = "${namePrefix}_Machine" + $json = @{ + environmentVariables = @( + @{ + scope = 'AllUsers' + name = $machineName + value = 'requires elevation' + } + ) + } | ConvertTo-Json -Compress -Depth 5 + + $out = $json | dsc resource set -r $resourceType -f - 2>&1 + + $LASTEXITCODE | Should -Not -Be 0 + $out | Should -Match 'elevation' + [Environment]::GetEnvironmentVariable( + $machineName, + [EnvironmentVariableTarget]::Machine) | Should -BeNullOrEmpty + } +} From 37349b9db660b14a0f7cd4e019efa314d92c7503 Mon Sep 17 00:00:00 2001 From: "Steve Lee (POWERSHELL HE/HIM) (from Dev Box)" Date: Thu, 13 Aug 2026 14:05:53 -0700 Subject: [PATCH 2/3] Include environment resource in Windows package Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/skills/create-dsc-resource/SKILL.md | 4 +++- data.build.json | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/skills/create-dsc-resource/SKILL.md b/.github/skills/create-dsc-resource/SKILL.md index b2209ae32..2dd310b3d 100644 --- a/.github/skills/create-dsc-resource/SKILL.md +++ b/.github/skills/create-dsc-resource/SKILL.md @@ -20,6 +20,7 @@ Management tasks or operations are specific to the resource type, but may includ - **Resource manifest**: A JSON file that defines the resource type name, supported operations (including executable and arguments), and JSON schema for input parameters - **Dependency management**: All crates must be listed in Cargo.toml specifying to use workspace dependencies. The root level Cargo.toml should be updated to include the new crates or associated to the DSC resource project. - **Project files**: A `.project.data.json` file in the root of the project folder defines properties of the project and non-code files to include during build +- **Release packaging**: Add every resource binary and manifest to the applicable platform list under `PackageFiles` in the root `data.build.json`. Project discovery alone does not include a resource in released packages. - **Localization**: For Rust-based resources, all user-facing strings must use `rust-i18n` for internationalization. For script-based resources (such as PowerShell), follow the existing localization and string-handling patterns used by those scripts or any repository-specific localization guidance. - **Copyright headers**: Every source file must start with the copyright header: ``` @@ -37,6 +38,7 @@ Management tasks or operations are specific to the resource type, but may includ - Create a resource manifest JSON file named `.dsc.resource.json` in the same directory using `./resources/windows_service/windows_service.dsc.resource.json` as an example - Create a `.project.data.json` file in the root of the resource project directory - Create a `locales/en-us.toml` file for localized strings +- Add the resource executable and manifest to the applicable `PackageFiles` platform list in the root `data.build.json` (for example, `.exe` and `.dsc.resource.json` under `PackageFiles.Windows`) ### 2. .project.data.json @@ -261,6 +263,7 @@ someError = "Failed to do something: %{error}" #### Build and Deployment +- Verify the root `data.build.json` lists the resource binary and every manifest under each applicable `PackageFiles` platform. A `.project.data.json` entry controls building and copying artifacts but does not by itself add those files to a released package. - The resource should be built using `build.ps1 -project ` from the root of the repository, which will handle building the Rust code and ensure it is found in PATH for testing ## What-If support @@ -560,4 +563,3 @@ When asked to add what-if to a new resource, perform these steps in order: 7. Add `whatIf*` localized strings under `[_helper]` and `args.configArgsWhatIfHelp` in `locales/en-us.toml`. 8. Create `.config.whatif.tests.ps1` (and the list variant if applicable) following the test template; cover create, update, delete-via-`_exist`, and `delete -w`. 9. Build with `./build.ps1 -project ` and run the new Pester file. - diff --git a/data.build.json b/data.build.json index 9a6d58d03..96e753c28 100644 --- a/data.build.json +++ b/data.build.json @@ -74,6 +74,8 @@ "dsc-bicep-ext.exe", "dscecho.exe", "echo.dsc.resource.json", + "environment_variable.exe", + "environment_variable.dsc.resource.json", "assertion.dsc.resource.json", "featureondemand.dsc.resource.json", "group.dsc.resource.json", From dcd912742c924e35e5313ccbc3e767322646022c Mon Sep 17 00:00:00 2001 From: "Steve Lee (POWERSHELL HE/HIM) (from Dev Box)" Date: Thu, 13 Aug 2026 16:39:16 -0700 Subject: [PATCH 3/3] Fix environment resource localization audit Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../environment_variable/src/environment.rs | 48 +++++++++++++------ 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/resources/environment_variable/src/environment.rs b/resources/environment_variable/src/environment.rs index 952d51356..2185aeec5 100644 --- a/resources/environment_variable/src/environment.rs +++ b/resources/environment_variable/src/environment.rs @@ -16,6 +16,15 @@ pub enum EnvironmentError { Resource(String), } +#[derive(Clone, Copy)] +enum OperationError { + Registry, + GetRead, + SetRead, + SetWrite, + SetRemove, +} + impl EnvironmentError { pub fn is_elevation_required(&self) -> bool { matches!(self, Self::ElevationRequired) @@ -65,7 +74,7 @@ pub fn set_variables( if !variable.exist.unwrap_or(true) { helper .remove() - .map_err(|error| operation_error("set.removeError", variable, &error))?; + .map_err(|error| operation_error(OperationError::SetRemove, variable, &error))?; environment_variables.push(EnvironmentVariable { scope: variable.scope, name: variable.name.clone(), @@ -79,13 +88,13 @@ pub fn set_variables( let current_data = helper .get() - .map_err(|error| operation_error("set.readError", variable, &error))? + .map_err(|error| operation_error(OperationError::SetRead, variable, &error))? .value_data; let desired_value = desired_value(variable, current_data.as_ref()); let value_data = registry_data(&desired_value, current_data.as_ref()); registry_helper(variable, Some(value_data))? .set() - .map_err(|error| operation_error("set.writeError", variable, &error))?; + .map_err(|error| operation_error(OperationError::SetWrite, variable, &error))?; environment_variables.push(get_variable(variable)?); } @@ -97,7 +106,7 @@ pub fn set_variables( fn get_variable(variable: &EnvironmentVariable) -> Result { let state = registry_helper(variable, None)? .get() - .map_err(|error| operation_error("get.readError", variable, &error))?; + .map_err(|error| operation_error(OperationError::GetRead, variable, &error))?; if state.exist == Some(false) { return Ok(EnvironmentVariable { @@ -150,7 +159,7 @@ fn registry_helper( Some(variable.name.clone()), value_data, ) - .map_err(|error| operation_error("main.registryError", variable, &error)) + .map_err(|error| operation_error(OperationError::Registry, variable, &error)) } fn key_path(scope: Scope) -> &'static str { @@ -219,19 +228,28 @@ fn merge_path(existing: &[String], desired: &[String], action: PathAction) -> Ve } fn operation_error( - key: &str, + operation: OperationError, variable: &EnvironmentVariable, error: &impl std::fmt::Display, ) -> EnvironmentError { - EnvironmentError::Resource( - t!( - key, - name = variable.name.as_str(), - scope = variable.scope.to_string(), - error = error.to_string() - ) - .to_string(), - ) + let name = variable.name.as_str(); + let scope = variable.scope.to_string(); + let error = error.to_string(); + let message = match operation { + OperationError::Registry => t!( + "main.registryError", + name = name, + scope = scope, + error = error + ), + OperationError::GetRead => t!("get.readError", name = name, scope = scope, error = error), + OperationError::SetRead => t!("set.readError", name = name, scope = scope, error = error), + OperationError::SetWrite => t!("set.writeError", name = name, scope = scope, error = error), + OperationError::SetRemove => { + t!("set.removeError", name = name, scope = scope, error = error) + } + }; + EnvironmentError::Resource(message.to_string()) } #[cfg(test)]