From b53e27bd380607082323cdcc33a3875dd225e6a9 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Mon, 31 Aug 2026 17:00:04 +0000 Subject: [PATCH 1/2] Fix registry resource what-if for non-existing key (#1692) * Fix registry resource what-if for non-existing key * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Update how exit code is returned to work with code cov tools * Allow coverage threshold override label Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Steve Lee (POWERSHELL HE/HIM) (from Dev Box) Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/workflows/rust.yml | 1 + lib/dsc-lib-registry/locales/en-us.toml | 1 + lib/dsc-lib-registry/src/lib.rs | 16 ++++- lib/dsc-lib/src/dscresources/invoke_result.rs | 1 - resources/registry/src/main.rs | 64 ++++++++++--------- .../tests/registry.config.whatif.tests.ps1 | 17 +++++ 6 files changed, 66 insertions(+), 34 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index ab97e1893..ebe7ba189 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -487,6 +487,7 @@ jobs: steps.coverage.outputs.has_rust_changes == 'true' && steps.coverage.outputs.coverage_failed != 'true' && steps.coverage.outputs.percentage < 70 + && !contains(github.event.pull_request.labels.*.name, 'Ok-CodeCoverage') run: | Write-Error "Code coverage is ${{ steps.coverage.outputs.percentage }}%, which is below the 70% minimum threshold." exit 1 diff --git a/lib/dsc-lib-registry/locales/en-us.toml b/lib/dsc-lib-registry/locales/en-us.toml index 3ab710b3d..0c70cee72 100644 --- a/lib/dsc-lib-registry/locales/en-us.toml +++ b/lib/dsc-lib-registry/locales/en-us.toml @@ -18,6 +18,7 @@ unsupportedValueDataType = "Unsupported registry value data type" whatIfCreateKey = "Key '%{subkey}' not found, would create it" whatIfDeleteValue = "Would delete value '%{value_name}'" whatIfDeleteSubkey = "Would delete subkey '%{subkey_name}'" +whatIfDeleteNonexistingKey = "Key '%{subkey}' not found, would do nothing" [offreg] loadFailed = "Failed to load offreg.dll" diff --git a/lib/dsc-lib-registry/src/lib.rs b/lib/dsc-lib-registry/src/lib.rs index 3cd677c72..12e77c1ac 100644 --- a/lib/dsc-lib-registry/src/lib.rs +++ b/lib/dsc-lib-registry/src/lib.rs @@ -316,6 +316,9 @@ impl RegistryHelper { return self.remove_offline(); } + // Accumulate what-if metadata like set() + let mut what_if_metadata: Vec = Vec::new(); + // For deleting a value, we need SetValue permission (KEY_SET_VALUE). // Try to open with the minimal required permission. // If that fails due to permission, try with AllAccess as a fallback. @@ -323,6 +326,15 @@ impl RegistryHelper { Ok(reg_key) => reg_key, // handle NotFound error Err(RegistryError::RegistryKeyNotFound(_)) => { + if self.what_if { + what_if_metadata.push(t!("registry_helper.whatIfDeleteNonexistingKey", subkey = &self.config.key_path).to_string()); + return Ok(Some(Registry { + key_path: self.config.key_path.clone(), + value_name: self.config.value_name.clone(), + metadata: Some(Metadata { what_if: Some(what_if_metadata) }), + ..Default::default() + })); + } return Ok(None); }, Err(RegistryError::RegistryKey(key::Error::PermissionDenied(_, _))) => { @@ -334,9 +346,6 @@ impl RegistryHelper { Err(e) => return self.handle_error_or_what_if(e), }; - // Accumulate what-if metadata like set() - let mut what_if_metadata: Vec = Vec::new(); - if let Some(value_name) = &self.config.value_name { if self.what_if { what_if_metadata.push(t!("registry_helper.whatIfDeleteValue", value_name = value_name).to_string()); @@ -385,6 +394,7 @@ impl RegistryHelper { Err(e) => return self.handle_error_or_what_if(RegistryError::RegistryKey(e)), } } + Ok(None) } diff --git a/lib/dsc-lib/src/dscresources/invoke_result.rs b/lib/dsc-lib/src/dscresources/invoke_result.rs index 1658f56a0..c8053051f 100644 --- a/lib/dsc-lib/src/dscresources/invoke_result.rs +++ b/lib/dsc-lib/src/dscresources/invoke_result.rs @@ -184,7 +184,6 @@ pub struct ResolveResult { } #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema, DscRepoSchema)] -#[serde(deny_unknown_fields)] #[dsc_repo_schema(base_name = "delete", folder_path = "outputs/resource")] pub struct DeleteResult { /// The return from the resource by the Delete method with what-if simulation. diff --git a/resources/registry/src/main.rs b/resources/registry/src/main.rs index 733dc6862..e9ef6008b 100644 --- a/resources/registry/src/main.rs +++ b/resources/registry/src/main.rs @@ -12,7 +12,7 @@ use clap::Parser; use dsc_lib_registry::{config::Registry, RegistryHelper}; use rust_i18n::t; use schemars::schema_for; -use std::process::exit; +use std::process::ExitCode; use tracing::{error, trace}; use tracing_subscriber::{filter::LevelFilter, prelude::__tracing_subscriber_SubscriberExt, EnvFilter, Layer}; use types::RegistryList; @@ -24,12 +24,12 @@ mod types; rust_i18n::i18n!("locales", fallback = "en-us"); -const EXIT_SUCCESS: i32 = 0; -const EXIT_INVALID_INPUT: i32 = 2; -const EXIT_REGISTRY_ERROR: i32 = 3; +const EXIT_SUCCESS: u8 = 0; +const EXIT_INVALID_INPUT: u8 = 2; +const EXIT_REGISTRY_ERROR: u8 = 3; #[allow(clippy::too_many_lines)] -fn main() { +fn main() -> ExitCode { #[cfg(debug_assertions)] check_debug(); @@ -45,9 +45,9 @@ fn main() { AdapterSubCommand::Set { input, adapted_resource } => { if let Err(e) = adapter_set(&input, &adapted_resource) { error!("{e}"); - exit(EXIT_REGISTRY_ERROR); + return ExitCode::from(EXIT_REGISTRY_ERROR); } - exit(EXIT_SUCCESS); + return ExitCode::from(EXIT_SUCCESS); }, AdapterSubCommand::Export { input, adapted_resource } => { adapter_export(&input, &adapted_resource) @@ -55,7 +55,7 @@ fn main() { AdapterSubCommand::Schema => { let schema = schema_for!(AdaptedRegistryValue); println!("{}", serde_json::to_string(&schema).unwrap()); - exit(EXIT_SUCCESS); + return ExitCode::from(EXIT_SUCCESS); } }; match result { @@ -64,7 +64,7 @@ fn main() { }, Err(err) => { error!("{err}"); - exit(EXIT_INVALID_INPUT); + return ExitCode::from(EXIT_INVALID_INPUT); } } }, @@ -85,13 +85,15 @@ fn main() { ConfigSubCommand::Get{input, list} => { trace!("Get input: {input}"); let mut output = RegistryList { registry_entries: vec![], registry_file_path: None }; - let reg_list = import_input(&input, list); + let Ok(reg_list) = import_input(&input, list) else { + return ExitCode::from(EXIT_INVALID_INPUT); + }; for reg in reg_list.registry_entries { let reg_helper = match RegistryHelper::new_from_registry(®) { Ok(helper) => helper, Err(err) => { error!("{err}"); - exit(EXIT_INVALID_INPUT); + return ExitCode::from(EXIT_INVALID_INPUT); } }; match reg_helper.get() { @@ -101,29 +103,31 @@ fn main() { } else { let json = serde_json::to_string(®_config).unwrap(); println!("{json}"); - exit(EXIT_SUCCESS); + return ExitCode::from(EXIT_SUCCESS); } }, Err(err) => { error!("{err}"); - exit(EXIT_REGISTRY_ERROR); + return ExitCode::from(EXIT_REGISTRY_ERROR); } } } let json = serde_json::to_string(&output).unwrap(); println!("{json}"); - exit(EXIT_SUCCESS); + return ExitCode::from(EXIT_SUCCESS); }, ConfigSubCommand::Set{input, list, what_if} => { trace!("Set input: {input}, what_if: {what_if}"); let mut output = RegistryList { registry_entries: vec![], registry_file_path: None }; - let reg_list = import_input(&input, list); + let Ok(reg_list) = import_input(&input, list) else { + return ExitCode::from(EXIT_INVALID_INPUT); + }; for reg in reg_list.registry_entries { let mut reg_helper = match RegistryHelper::new_from_registry(®) { Ok(helper) => helper, Err(err) => { error!("{err}"); - exit(EXIT_INVALID_INPUT); + return ExitCode::from(EXIT_INVALID_INPUT); } }; if what_if { reg_helper.enable_what_if(); } @@ -136,14 +140,14 @@ fn main() { } else { let json = serde_json::to_string(®_config).unwrap(); println!("{json}"); - exit(EXIT_SUCCESS); + return ExitCode::from(EXIT_SUCCESS); } } }, Ok(None) => {}, Err(err) => { error!("{err}"); - exit(EXIT_REGISTRY_ERROR); + return ExitCode::from(EXIT_REGISTRY_ERROR); } } continue; @@ -156,16 +160,16 @@ fn main() { } else { let json = serde_json::to_string(&config).unwrap(); println!("{json}"); - exit(EXIT_SUCCESS); + return ExitCode::from(EXIT_SUCCESS); } } if !list { - exit(EXIT_SUCCESS); + return ExitCode::from(EXIT_SUCCESS); } }, Err(err) => { error!("{err}"); - exit(EXIT_REGISTRY_ERROR); + return ExitCode::from(EXIT_REGISTRY_ERROR); } } } @@ -173,7 +177,7 @@ fn main() { let json = serde_json::to_string(&output).unwrap(); println!("{json}"); } - exit(EXIT_SUCCESS); + return ExitCode::from(EXIT_SUCCESS); }, ConfigSubCommand::Delete{input, what_if} => { trace!("Delete input: {input}, what_if: {what_if}"); @@ -181,7 +185,7 @@ fn main() { Ok(reg_helper) => reg_helper, Err(err) => { error!("{err}"); - exit(EXIT_INVALID_INPUT); + return ExitCode::from(EXIT_INVALID_INPUT); } }; if what_if { reg_helper.enable_what_if(); } @@ -193,7 +197,7 @@ fn main() { Ok(None) => {}, Err(err) => { error!("{err}"); - exit(EXIT_REGISTRY_ERROR); + return ExitCode::from(EXIT_REGISTRY_ERROR); } } }, @@ -210,10 +214,10 @@ fn main() { }, } - exit(EXIT_SUCCESS); + ExitCode::from(EXIT_SUCCESS) } -fn import_input(input: &str, list: bool) -> RegistryList { +fn import_input(input: &str, list: bool) -> Result { if list { match serde_json::from_str::(input) { Ok(mut reg_list) => { @@ -225,19 +229,19 @@ fn import_input(input: &str, list: bool) -> RegistryList { } } } - reg_list + Ok(reg_list) }, Err(err) => { error!("{err}"); - exit(EXIT_INVALID_INPUT); + Err(ExitCode::from(EXIT_INVALID_INPUT)) } } } else { match serde_json::from_str::(input) { - Ok(reg) => RegistryList { registry_entries: vec![reg], registry_file_path: None }, + Ok(reg) => Ok(RegistryList { registry_entries: vec![reg], registry_file_path: None }), Err(err) => { error!("{err}"); - exit(EXIT_INVALID_INPUT); + Err(ExitCode::from(EXIT_INVALID_INPUT)) } } } diff --git a/resources/registry/tests/registry.config.whatif.tests.ps1 b/resources/registry/tests/registry.config.whatif.tests.ps1 index 4f0cf08b5..42a0c09af 100644 --- a/resources/registry/tests/registry.config.whatif.tests.ps1 +++ b/resources/registry/tests/registry.config.whatif.tests.ps1 @@ -200,4 +200,21 @@ Describe 'registry config whatif tests' { # For delete what-if, payload should only include keyPath (and optionally valueName when deleting a value) ($result.psobject.properties | Where-Object { $_.Name -ne '_metadata' } | Measure-Object).Count | Should -Be 1 } + + + It 'Removing non-existing key' -Skip:(!$IsWindows) { + $after_config_yaml = @' + $schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json + resources: + - name: Reg 1 + type: Microsoft.Windows/Registry + properties: + keyPath: HKCU\1\2\NonExisting + _exist: false +'@ + $out = dsc -l trace config set --what-if --input $after_config_yaml 2>$TestDrive/error.log | ConvertFrom-Json + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Path $TestDrive/error.log -Raw) + $out.results.result[0].afterState.keyPath | Should -BeExactly 'HKCU\1\2\NonExisting' + $out.results.executionInformation.whatIf[0] | Should -Match "Key 'HKCU\\1\\2\\NonExisting' not found, would do nothing" + } } From 968660ddead9dd0900125f4ad488b2d1976d8090 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Sat, 22 Aug 2026 06:53:42 -0700 Subject: [PATCH 2/2] Fix clippy rule violation (#1688) * Fix clippy rule violation * fix build on Windows * Add registry decoder test coverage Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * remove incorrect msrustup env var --------- Co-authored-by: Steve Lee (POWERSHELL HE/HIM) (from Dev Box) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- helpers.build.psm1 | 3 --- lib/dsc-lib-registry/src/lib.rs | 15 +++++++++++++-- lib/dsc-lib/src/functions/int.rs | 9 ++++----- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/helpers.build.psm1 b/helpers.build.psm1 index c129322ce..23f16aab4 100644 --- a/helpers.build.psm1 +++ b/helpers.build.psm1 @@ -388,9 +388,6 @@ function Get-RustUp { Write-Verbose -Verbose "Using msrustup" $rustup = 'msrustup' $channel = 'ms-prod-1.95' - if ($architecture -eq 'current') { - $env:MSRUSTUP_TOOLCHAIN = "$architecture" - } } elseif ($null -ne (Get-Command rustup -CommandType Application -ErrorAction Ignore)) { $rustup = 'rustup' $env:TESTING_FUNCTION_ENV = "lolwhat" diff --git a/lib/dsc-lib-registry/src/lib.rs b/lib/dsc-lib-registry/src/lib.rs index 12e77c1ac..8ac277f89 100644 --- a/lib/dsc-lib-registry/src/lib.rs +++ b/lib/dsc-lib-registry/src/lib.rs @@ -755,7 +755,7 @@ fn convert_value_data_to_offline(value_data: &RegistryValueData) -> Result<(u32, /// Decode a null-terminated UTF-16LE byte slice to a String. fn decode_utf16_bytes(data: &[u8]) -> String { - let u16_slice: Vec = data.chunks_exact(2) + let u16_slice: Vec = data.as_chunks::<2>().0.iter() .map(|chunk| u16::from_le_bytes([chunk[0], chunk[1]])) .collect(); // Strip trailing null @@ -771,7 +771,7 @@ fn encode_utf16_bytes(s: &str) -> Vec { /// Decode REG_MULTI_SZ: double-null-terminated list of null-terminated UTF-16LE strings. fn decode_multi_sz(data: &[u8]) -> Vec { - let u16_slice: Vec = data.chunks_exact(2) + let u16_slice: Vec = data.as_chunks::<2>().0.iter() .map(|chunk| u16::from_le_bytes([chunk[0], chunk[1]])) .collect(); let mut strings = Vec::new(); @@ -799,6 +799,17 @@ fn encode_multi_sz(strings: &[String]) -> Vec { result.iter().flat_map(|&c| c.to_le_bytes()).collect() } +#[test] +fn decode_utf16_bytes_ignores_incomplete_code_unit() { + assert_eq!(decode_utf16_bytes(&[b'A', 0, 0xff]), "A"); +} + +#[test] +fn decode_multi_sz_ignores_incomplete_code_unit() { + let data = [b'A', 0, 0, 0, 0, 0, 0xff]; + assert_eq!(decode_multi_sz(&data), vec!["A"]); +} + #[test] fn get_hklm_key() { let reg_helper = RegistryHelper::new_from_json(r#"{"keyPath":"HKEY_LOCAL_MACHINE"}"#).unwrap(); diff --git a/lib/dsc-lib/src/functions/int.rs b/lib/dsc-lib/src/functions/int.rs index 5297e3aae..c4f81bc25 100644 --- a/lib/dsc-lib/src/functions/int.rs +++ b/lib/dsc-lib/src/functions/int.rs @@ -30,16 +30,15 @@ impl Function for Int { fn invoke(&self, args: &[Value], _context: &Context) -> Result { let arg = &args[0]; - let value: i64; - if arg.is_string() { + let value: i64 = if arg.is_string() { let input = arg.as_str().ok_or(DscError::FunctionArg("int".to_string(), t!("functions.int.invalidInput").to_string()))?; let result = input.parse::().map_err(|_| DscError::FunctionArg("int".to_string(), t!("functions.int.parseStringError").to_string()))?; - value = NumCast::from(result).ok_or(DscError::FunctionArg("int".to_string(), t!("functions.int.castError").to_string()))?; + NumCast::from(result).ok_or(DscError::FunctionArg("int".to_string(), t!("functions.int.castError").to_string()))? } else if arg.is_number() { - value = arg.as_i64().ok_or(DscError::FunctionArg("int".to_string(), t!("functions.int.parseNumError").to_string()))?; + arg.as_i64().ok_or(DscError::FunctionArg("int".to_string(), t!("functions.int.parseNumError").to_string()))? } else { return Err(DscError::FunctionArg("int".to_string(), t!("functions.invalidArgType").to_string())); - } + }; Ok(Value::Number(value.into())) } }