From 9604a96cb04839c16286570e521010a746e5dd52 Mon Sep 17 00:00:00 2001 From: "G.Reijn" <26114636+Gijsreyn@users.noreply.github.com> Date: Thu, 8 Oct 2026 06:35:57 +0200 Subject: [PATCH 1/4] (GH-1729) Make nonfunctional secret extension manifests invalid --- CHANGELOG.md | 18 ++ .../schemas/extension/manifest/root.md | 5 +- .../schemas/extension/manifest/secret.md | 28 +- dsc/tests/dsc_extension_secret.tests.ps1 | 62 +++++ lib/dsc-lib/locales/en-us.toml | 6 + .../src/discovery/command_discovery.rs | 3 + lib/dsc-lib/src/extensions/secret.rs | 261 ++++++++++++++++-- .../deprecated/deprecated.dsc.manifests.json | 5 +- tools/dsctest/src/args.rs | 5 +- tools/dsctest/src/main.rs | 4 +- 10 files changed, 361 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ae6e6f41..bc5d85e86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -84,6 +84,23 @@ changes since the last release, see the [diff on GitHub][unreleased]. before applying them. Passing `what_if` with an operation that doesn't support it returns an invalid parameters error. +### Changed + +- Extensions that implement the `secret` capability must now define the `args` property for the + `secret` command with exactly one secret name input argument (`nameArg`) and at most one vault + input argument (`vaultArg`). Previously, an extension could omit the secret name input argument, + which left DSC with no way to tell the extension which secret to retrieve. DSC no longer loads an + extension manifest that defines a nonfunctional `secret` command and logs an informational + message explaining the problem. The extension manifest JSON schema enforces the same + requirements. + +
Related work items + + - Issues: [#1729][#1729] + - PRs: _None_ + +
+ ## [v3.2.2][release-v3.2.2] - 2026-06-16 This section includes a summary of changes for the `3.2.2` release. For the full list of changes @@ -1982,4 +1999,5 @@ Version `3.0.0` is the first generally available release of DSC. [#1557]: https://github.com/PowerShell/DSC/issues/1557 [#1558]: https://github.com/PowerShell/DSC/issues/1558 [#1562]: https://github.com/PowerShell/DSC/issues/1562 +[#1729]: https://github.com/PowerShell/DSC/issues/1729 diff --git a/docs/reference/schemas/extension/manifest/root.md b/docs/reference/schemas/extension/manifest/root.md index 3bcf8eb3f..25b1f68be 100644 --- a/docs/reference/schemas/extension/manifest/root.md +++ b/docs/reference/schemas/extension/manifest/root.md @@ -317,7 +317,10 @@ runtime. When this property is defined, the extension has the `secret` capabilit invoke the extension for the [secret()][05] configuration function. The value of this property must be an object. The object's `executable` property, defining the name -of the command to call, is mandatory. The `args` property is optional. For more information, see +of the command to call, and `args` property, defining the arguments to pass to the command, are +both mandatory. The `args` property must define the secret name input argument exactly once and may +define the vault input argument at most once. DSC doesn't load an extension manifest that defines +the `secret` property without a secret name input argument. For more information, see [DSC extension manifest secret property schema reference][06]. ```yaml diff --git a/docs/reference/schemas/extension/manifest/secret.md b/docs/reference/schemas/extension/manifest/secret.md index 2a159246f..723e77b96 100644 --- a/docs/reference/schemas/extension/manifest/secret.md +++ b/docs/reference/schemas/extension/manifest/secret.md @@ -35,13 +35,14 @@ that extension. The `secret` definition must include these properties: - [executable](#executable) +- [args](#args) ## Properties ### executable The `executable` property defines the name of the command to run. The value must be the name of a -command secretable in the system's `PATH` environment variable or the full path to the command. A +command discoverable in the system's `PATH` environment variable or the full path to the command. A file extension is only required when the command isn't recognizable by the operating system as an executable. @@ -56,15 +57,22 @@ The `args` property defines the list of arguments to pass to the command. Each i can be a string representing a static argument, a [name argument](#name-argument) object, or a [vault argument](#vault-argument) object. -The array should contain exactly one name argument. It may contain a single vault argument and any +The array must contain exactly one name argument. It may contain at most one vault argument and any number of static string arguments. -If the array doesn't define a name argument, DSC can't pass the secret name to the extension. If -the array doesn't define a vault argument, DSC can't pass the vault name to the extension. +Without a name argument, DSC can't pass the secret name to the extension, so the extension can't +retrieve a specific secret. DSC doesn't load an extension manifest that defines the `secret` +property without the `args` property, without a name argument, with more than one name argument, +or with more than one vault argument. When DSC skips a manifest for one of these reasons, it logs +an informational message that explains the problem. Use the `--trace-level info` option, like +`dsc --trace-level info extension list`, to see the message. + +If the array doesn't define a vault argument, DSC can't pass the vault name to the extension, so +the extension can't retrieve a secret from a specific vault. ```yaml Type: array -Required: false +Required: true ItemsType: [string, object(Name or Vault argument)] ``` @@ -79,11 +87,12 @@ Type: string #### Name argument -Defines an argument that receives the path to the file to import. +Defines the argument that receives the name of the secret to retrieve. -DSC passes the value of `nameArg` followed by the name of the secret to retrieve. +DSC passes the value of `nameArg` followed by the name of the secret to retrieve. The `args` array +must define this argument exactly once. -A file argument is defined as a JSON object with the following properties: +A name argument is defined as a JSON object with the following properties: - `nameArg` (required) - The argument to pass before the secret name, like `--secret-name`. @@ -97,7 +106,8 @@ RequiredProperties: [nameArg] Defines an argument that receives the name of a specific vault to retrieve a secret from. DSC passes the value of `vaultArg` followed by the name of the vault when the `secret()` function -specifies a vault. When the function doesn't specify a vault, DSC ignores the vault argument. +specifies a vault. When the function doesn't specify a vault, DSC ignores the vault argument. The +`args` array may define this argument at most once. A vault argument is defined as a JSON object with the following properties: diff --git a/dsc/tests/dsc_extension_secret.tests.ps1 b/dsc/tests/dsc_extension_secret.tests.ps1 index a17659b0e..4b4c3fddd 100644 --- a/dsc/tests/dsc_extension_secret.tests.ps1 +++ b/dsc/tests/dsc_extension_secret.tests.ps1 @@ -200,4 +200,66 @@ Describe 'Tests for the secret() function and extensions' { $env:DSC_RESTRICTED_PATH = $null } } + + It 'Secret extension manifest is not loaded' -TestCases @( + @{ reason = 'without args'; secret = '{ "executable": "dsctest" }'; expectedError = 'missing field *args*' } + @{ reason = 'without a name argument'; secret = '{ "executable": "dsctest", "args": ["no-op"] }'; expectedError = "The 'secret' command doesn't define the secret name input argument" } + @{ reason = 'with multiple name arguments'; secret = '{ "executable": "dsctest", "args": ["no-op", { "nameArg": "--name" }, { "nameArg": "--secret" }] }'; expectedError = "The 'secret' command defines the secret name input argument 2 times" } + @{ reason = 'with multiple vault arguments'; secret = '{ "executable": "dsctest", "args": ["no-op", { "nameArg": "--name" }, { "vaultArg": "--vault" }, { "vaultArg": "--store" }] }'; expectedError = "The 'secret' command defines the vault input argument 2 times" } + ) { + param($secret, $expectedError) + + $manifest = @" +{ + "`$schema": "https://aka.ms/dsc/schemas/v3/bundled/extension/manifest.json", + "type": "Test/SecretInvalid", + "version": "0.1.0", + "description": "Invalid secret extension for testing.", + "secret": $secret +} +"@ + + try { + $env:DSC_RESTRICTED_PATH = $TestDrive + Set-Content -Path "$TestDrive/secretInvalid.dsc.extension.json" -Value $manifest + $out = dsc -l info extension list 2> $TestDrive/error.log | ConvertFrom-Json + $errorLog = Get-Content -Raw -Path $TestDrive/error.log + $LASTEXITCODE | Should -Be 0 -Because $errorLog + @($out).type | Should -Not -Contain 'Test/SecretInvalid' + $errorLog | Should -BeLike "*INFO Failed to load manifest: *$expectedError*" -Because $errorLog + } finally { + $env:DSC_RESTRICTED_PATH = $null + } + } + + It 'Secret extension manifest with a name argument and a vault argument is loaded' { + $manifest = @' +{ + "$schema": "https://aka.ms/dsc/schemas/v3/bundled/extension/manifest.json", + "type": "Test/SecretValid", + "version": "0.1.0", + "description": "Valid secret extension for testing.", + "secret": { + "executable": "dsctest", + "args": [ + "no-op", + { "vaultArg": "--vault" }, + { "nameArg": "--name" } + ] + } +} +'@ + + try { + $env:DSC_RESTRICTED_PATH = $TestDrive + Set-Content -Path "$TestDrive/secretValid.dsc.extension.json" -Value $manifest + $out = dsc extension list 2> $TestDrive/error.log | ConvertFrom-Json + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw -Path $TestDrive/error.log) + @($out).Count | Should -Be 1 + $out.type | Should -BeExactly 'Test/SecretValid' + $out.capabilities | Should -BeExactly @('secret') + } finally { + $env:DSC_RESTRICTED_PATH = $null + } + } } diff --git a/lib/dsc-lib/locales/en-us.toml b/lib/dsc-lib/locales/en-us.toml index 7ec10b494..bafdff849 100644 --- a/lib/dsc-lib/locales/en-us.toml +++ b/lib/dsc-lib/locales/en-us.toml @@ -138,6 +138,7 @@ adaptedResourceFound = "Adapted resource '%{resource}' version %{version} found" executableNotFound = "Executable '%{executable}' not found for operation '%{operation}' for resource '%{resource}'" invalidResourceManifest = "Invalid manifest for resource '%{resource}': %{err}" invalidExtensionManifest = "Invalid manifest for extension '%{resource}': %{err}" +invalidSecretExtensionManifest = "Invalid 'secret' definition for extension '%{extension}' in manifest '%{path}': %{err}" invalidAdaptedResourceManifest = "Invalid manifest for adapted resource '%{resource}': %{err}" invalidManifestList = "Invalid manifest list '%{resource}': %{err}" invalidManifestFile = "Invalid manifest file '%{resource}': %{err}" @@ -303,6 +304,11 @@ manifestImported = "Manifest imported from extension %{extension}" extensionManifestSchemaTitle = "Extension manifest schema URI" extensionManifestSchemaDescription = "Defines the JSON Schema the extension manifest adheres to." +[extensions.secret] +missingNameArg = "The 'secret' command doesn't define the secret name input argument, so DSC can't pass the name of the secret to retrieve to the extension. Define exactly one argument in 'secret.args' as an object with the 'nameArg' property" +multipleNameArgs = "The 'secret' command defines the secret name input argument %{count} times. Define exactly one argument in 'secret.args' as an object with the 'nameArg' property" +multipleVaultArgs = "The 'secret' command defines the vault input argument %{count} times. Define at most one argument in 'secret.args' as an object with the 'vaultArg' property" + [functions] invalidArgType = "Invalid argument type" invalidArguments = "Invalid argument(s)" diff --git a/lib/dsc-lib/src/discovery/command_discovery.rs b/lib/dsc-lib/src/discovery/command_discovery.rs index e7c9eb2a5..3145bbe75 100644 --- a/lib/dsc-lib/src/discovery/command_discovery.rs +++ b/lib/dsc-lib/src/discovery/command_discovery.rs @@ -997,6 +997,9 @@ fn load_extension_manifest(path: &Path, manifest: &ExtensionManifest) -> Result< capabilities.push(dscextension::Capability::Discover); } if let Some(secret) = &manifest.secret { + if let Err(err) = secret.validate_args() { + return Err(DscError::InvalidManifest(t!("discovery.commandDiscovery.invalidSecretExtensionManifest", extension = manifest.r#type, path = path.to_string_lossy(), err = err).to_string())); + } verify_executable(&manifest.r#type, "secret", &secret.executable, path.parent().unwrap()); capabilities.push(dscextension::Capability::Secret); } diff --git a/lib/dsc-lib/src/extensions/secret.rs b/lib/dsc-lib/src/extensions/secret.rs index 6687ca51c..3bd351592 100644 --- a/lib/dsc-lib/src/extensions/secret.rs +++ b/lib/dsc-lib/src/extensions/secret.rs @@ -18,25 +18,29 @@ use crate::{ }; use rust_i18n::t; -use schemars::JsonSchema; +use schemars::{JsonSchema, Schema}; use serde::{Deserialize, Serialize}; +use serde_json::json; use tracing::{debug, warn}; #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] #[serde(untagged)] #[schemars(inline)] pub enum SecretArgKind { - /// The argument is a string. + /// A static string argument to pass to the command, like `get` or `--quiet`. String(String), - /// The argument accepts the secret name. + /// The argument that accepts the secret name. DSC passes the name of the secret to retrieve + /// after this argument. The arguments must define this argument exactly once. Name { - /// The argument that accepts the secret name. + /// The argument that accepts the secret name, like `--name` or `--secret-name`. #[serde(rename = "nameArg")] name_arg: String, }, - /// The argument accepts the vault name. + /// The argument that accepts the vault name. DSC passes the name of the vault after this + /// argument when the `secret()` function specifies a vault. The arguments may define this + /// argument at most once. Vault { - /// The argument that accepts the vault name. + /// The argument that accepts the vault name, like `--vault` or `--vault-name`. #[serde(rename = "vaultArg")] vault_arg: String, }, @@ -45,14 +49,94 @@ pub enum SecretArgKind { #[derive(Debug, Default, Clone, PartialEq, Deserialize, Serialize, JsonSchema, DscRepoSchema)] #[schemars( transform = SecretMethod::transform_export_schema_uris, - transform = SecretMethod::transform_schema_docs + transform = SecretMethod::transform_schema_docs, + transform = SecretMethod::transform_args_constraints )] #[dsc_repo_schema(base_name = "manifest.secret", folder_path = "extension")] pub struct SecretMethod { - /// The command to run to get the state of the resource. + /// The command to run to retrieve a secret. pub executable: String, - /// The arguments to pass to the command to perform a Get. - pub args: Option>, + /// The arguments to pass to the command to retrieve a secret. The arguments must define the + /// secret name input argument exactly once and may define the vault input argument at most + /// once. + pub args: Vec, +} + +impl SecretMethod { + /// Validates that the arguments define the secret name input argument exactly once and the + /// vault input argument at most once. + /// + /// Without the secret name input argument, DSC can't tell the extension which secret to + /// retrieve, so the extension can't function as a secret provider. + /// + /// # Errors + /// + /// Returns [`DscError::InvalidManifest`] when the arguments don't define the secret name input + /// argument, define it more than once, or define the vault input argument more than once. + pub fn validate_args(&self) -> Result<(), DscError> { + let name_arg_count = self.args.iter().filter(|arg| matches!(arg, SecretArgKind::Name { .. })).count(); + let vault_arg_count = self.args.iter().filter(|arg| matches!(arg, SecretArgKind::Vault { .. })).count(); + + if name_arg_count == 0 { + return Err(DscError::InvalidManifest(t!("extensions.secret.missingNameArg").to_string())); + } + if name_arg_count > 1 { + return Err(DscError::InvalidManifest(t!("extensions.secret.multipleNameArgs", count = name_arg_count).to_string())); + } + if vault_arg_count > 1 { + return Err(DscError::InvalidManifest(t!("extensions.secret.multipleVaultArgs", count = vault_arg_count).to_string())); + } + + Ok(()) + } + + /// Adds the validation subschemas that require `args` to define the secret name input + /// argument exactly once and the vault input argument at most once. + /// + /// The subschemas are defined separately so that each failure reports a specific message. + /// The `errorMessage` keyword is only emitted in the VS Code form of the schema. + fn transform_args_constraints(schema: &mut Schema) { + let docs_url = "https://learn.microsoft.com/powershell/dsc/reference/schemas/extension/manifest/secret"; + schema.insert("allOf".to_string(), json!([ + { + "title": "Missing secret name input argument", + "properties": { + "args": { + "errorMessage": format!( + "The `secret` command doesn't define the secret name input argument. If you don't define the secret name input argument, DSC can't pass the secret name to the extension for retrieval. You must define exactly one argument in `secret.args` as a JSON object with the `nameArg` property. For more information, see: {docs_url}" + ), + "contains": { "type": "object", "required": ["nameArg"] }, + "minContains": 1 + } + } + }, + { + "title": "Multiple secret name input arguments", + "properties": { + "args": { + "errorMessage": format!( + "The `secret` command defines the secret name input argument more than once. You must define exactly one argument in `secret.args` as a JSON object with the `nameArg` property and remove the additional secret name input arguments. For more information, see: {docs_url}" + ), + "contains": { "type": "object", "required": ["nameArg"] }, + "maxContains": 1 + } + } + }, + { + "title": "Multiple vault input arguments", + "properties": { + "args": { + "errorMessage": format!( + "The `secret` command defines the vault input argument more than once. You can define at most one argument in `secret.args` as a JSON object with the `vaultArg` property. For more information, see: {docs_url}" + ), + "contains": { "type": "object", "required": ["vaultArg"] }, + "minContains": 0, + "maxContains": 1 + } + } + } + ])); + } } impl DscExtension { @@ -82,13 +166,14 @@ impl DscExtension { let Some(secret) = extension.secret else { return Err(DscError::UnsupportedCapability(self.type_name.to_string(), Capability::Secret.to_string())); }; - let args = process_secret_args(secret.args.as_ref(), name, vault); + secret.validate_args()?; + let args = process_secret_args(&secret.args, name, vault); if let Some(deprecation_message) = extension.deprecation_message.as_ref() { warn!("{}", t!("extensions.dscextension.deprecationMessage", extension = self.type_name, message = deprecation_message)); } let (_exit_code, stdout, _stderr) = invoke_command( &secret.executable, - args, + Some(args), vault, Some(&self.directory), None, @@ -120,30 +205,162 @@ impl DscExtension { } } -fn process_secret_args(args: Option<&Vec>, name: &str, vault: Option<&str>) -> Option> { - let Some(arg_values) = args else { - debug!("{}", t!("dscresources.commandResource.noArgs")); - return None; - }; - +fn process_secret_args(args: &[SecretArgKind], name: &str, vault: Option<&str>) -> Vec { let mut processed_args = Vec::::new(); - for arg in arg_values { + for arg in args { match arg { SecretArgKind::String(s) => { processed_args.push(s.clone()); }, SecretArgKind::Name { name_arg } => { - processed_args.push(name_arg.to_string()); + processed_args.push(name_arg.clone()); processed_args.push(name.to_string()); }, SecretArgKind::Vault { vault_arg } => { if let Some(value) = vault { - processed_args.push(vault_arg.to_string()); + processed_args.push(vault_arg.clone()); processed_args.push(value.to_string()); } }, } } - Some(processed_args) + processed_args +} + +#[cfg(test)] +mod test { + use super::{SecretArgKind, SecretMethod, process_secret_args}; + use crate::dscerror::DscError; + use crate::extensions::extension_manifest::ExtensionManifest; + use crate::schemas::dsc_repo::DscRepoSchema; + use schemars::schema_for; + use serde_json::{Value, json}; + + fn name_arg() -> SecretArgKind { + SecretArgKind::Name { name_arg: "--name".to_string() } + } + + fn vault_arg() -> SecretArgKind { + SecretArgKind::Vault { vault_arg: "--vault".to_string() } + } + + fn secret_method(args: Vec) -> SecretMethod { + SecretMethod { executable: "secret".to_string(), args } + } + + #[test] + fn validate_args_accepts_single_name_arg() { + let method = secret_method(vec![SecretArgKind::String("get".to_string()), name_arg()]); + assert!(method.validate_args().is_ok()); + } + + #[test] + fn validate_args_accepts_single_name_and_vault_arg() { + let method = secret_method(vec![vault_arg(), name_arg()]); + assert!(method.validate_args().is_ok()); + } + + #[test] + fn validate_args_rejects_missing_name_arg() { + let method = secret_method(vec![SecretArgKind::String("get".to_string()), vault_arg()]); + let err = method.validate_args().unwrap_err(); + assert!(matches!(err, DscError::InvalidManifest(_))); + assert!(err.to_string().contains("doesn't define the secret name input argument"), "{err}"); + } + + #[test] + fn validate_args_rejects_empty_args() { + let method = secret_method(vec![]); + assert!(method.validate_args().is_err()); + } + + #[test] + fn validate_args_rejects_multiple_name_args() { + let method = secret_method(vec![name_arg(), name_arg()]); + let err = method.validate_args().unwrap_err(); + assert!(matches!(err, DscError::InvalidManifest(_))); + assert!(err.to_string().contains("defines the secret name input argument 2 times"), "{err}"); + } + + #[test] + fn validate_args_rejects_multiple_vault_args() { + let method = secret_method(vec![name_arg(), vault_arg(), vault_arg()]); + let err = method.validate_args().unwrap_err(); + assert!(matches!(err, DscError::InvalidManifest(_))); + assert!(err.to_string().contains("defines the vault input argument 2 times"), "{err}"); + } + + #[test] + fn process_secret_args_preserves_order_and_inserts_values() { + let args = vec![SecretArgKind::String("get".to_string()), vault_arg(), name_arg(), SecretArgKind::String("--quiet".to_string())]; + let processed = process_secret_args(&args, "apiToken", Some("services")); + assert_eq!(processed, vec!["get", "--vault", "services", "--name", "apiToken", "--quiet"]); + } + + #[test] + fn process_secret_args_omits_vault_arg_without_vault() { + let args = vec![SecretArgKind::String("get".to_string()), name_arg(), vault_arg()]; + let processed = process_secret_args(&args, "apiToken", None); + assert_eq!(processed, vec!["get", "--name", "apiToken"]); + } + + #[test] + fn manifest_without_secret_args_fails_to_deserialize() { + let manifest = json!({ + "$schema": ExtensionManifest::default_schema_id_uri(), + "type": "Test/Secret", + "version": "0.1.0", + "secret": { "executable": "secret" } + }); + let err = serde_json::from_value::(manifest).unwrap_err(); + assert!(err.to_string().contains("missing field `args`"), "{err}"); + } + + #[test] + fn manifest_with_secret_args_deserializes() { + let manifest = json!({ + "$schema": ExtensionManifest::default_schema_id_uri(), + "type": "Test/Secret", + "version": "0.1.0", + "secret": { + "executable": "secret", + "args": ["get", { "nameArg": "--name" }, { "vaultArg": "--vault" }] + } + }); + let manifest = serde_json::from_value::(manifest).unwrap(); + let secret = manifest.secret.unwrap(); + assert_eq!(secret.args, vec![SecretArgKind::String("get".to_string()), name_arg(), vault_arg()]); + assert!(secret.validate_args().is_ok()); + } + + #[test] + fn schema_requires_args_and_constrains_input_arguments() { + let schema = schema_for!(SecretMethod).to_value(); + let required = schema["required"].as_array().unwrap(); + assert!(required.contains(&Value::String("executable".to_string())), "{schema}"); + assert!(required.contains(&Value::String("args".to_string())), "{schema}"); + + let constraints = schema["allOf"].as_array().unwrap(); + assert_eq!(constraints.len(), 3, "{schema}"); + assert_eq!(constraints[0]["properties"]["args"]["minContains"], json!(1)); + assert_eq!(constraints[0]["properties"]["args"]["contains"]["required"], json!(["nameArg"])); + assert_eq!(constraints[1]["properties"]["args"]["maxContains"], json!(1)); + assert_eq!(constraints[1]["properties"]["args"]["contains"]["required"], json!(["nameArg"])); + assert_eq!(constraints[2]["properties"]["args"]["maxContains"], json!(1)); + assert_eq!(constraints[2]["properties"]["args"]["contains"]["required"], json!(["vaultArg"])); + } + + #[test] + fn schema_rejects_args_without_name_arg() { + let schema = schema_for!(SecretMethod).to_value(); + let validator = jsonschema::validator_for(&schema).unwrap(); + + assert!(validator.is_valid(&json!({ "executable": "secret", "args": ["get", { "nameArg": "--name" }] }))); + assert!(validator.is_valid(&json!({ "executable": "secret", "args": [{ "vaultArg": "--vault" }, { "nameArg": "--name" }] }))); + assert!(!validator.is_valid(&json!({ "executable": "secret" }))); + assert!(!validator.is_valid(&json!({ "executable": "secret", "args": ["get"] }))); + assert!(!validator.is_valid(&json!({ "executable": "secret", "args": [{ "nameArg": "--name" }, { "nameArg": "--secret" }] }))); + assert!(!validator.is_valid(&json!({ "executable": "secret", "args": [{ "nameArg": "--name" }, { "vaultArg": "--vault" }, { "vaultArg": "--store" }] }))); + } } diff --git a/tools/dsctest/deprecated/deprecated.dsc.manifests.json b/tools/dsctest/deprecated/deprecated.dsc.manifests.json index ceb5e1ee6..3b76e007b 100644 --- a/tools/dsctest/deprecated/deprecated.dsc.manifests.json +++ b/tools/dsctest/deprecated/deprecated.dsc.manifests.json @@ -66,7 +66,10 @@ "secret": { "executable": "dsctest", "args": [ - "no-op" + "no-op", + { + "nameArg": "--name" + } ] } } diff --git a/tools/dsctest/src/args.rs b/tools/dsctest/src/args.rs index 93d36cd62..fdfb11691 100644 --- a/tools/dsctest/src/args.rs +++ b/tools/dsctest/src/args.rs @@ -135,7 +135,10 @@ pub enum SubCommand { }, #[clap(name = "no-op", about = "Perform no operation, just return success")] - NoOp, + NoOp { + #[clap(name = "args", help = "Arguments that are ignored, allows testing manifests that pass input arguments", trailing_var_arg = true, allow_hyphen_values = true)] + args: Vec, + }, #[clap(name = "operation", about = "Perform an operation")] Operation { diff --git a/tools/dsctest/src/main.rs b/tools/dsctest/src/main.rs index 2363f5b2f..dafeb4d6d 100644 --- a/tools/dsctest/src/main.rs +++ b/tools/dsctest/src/main.rs @@ -244,8 +244,8 @@ fn main() { } String::new() }, - SubCommand::NoOp => { - // do nothing and just return success + SubCommand::NoOp { args: _args } => { + // do nothing and just return success, any arguments are ignored String::new() }, SubCommand::Operation { operation, input } => { From 6fcd53ddae1fc31f2e984358b87a1b01c663c2a1 Mon Sep 17 00:00:00 2001 From: "G.Reijn" <26114636+Gijsreyn@users.noreply.github.com> Date: Thu, 8 Oct 2026 06:35:57 +0200 Subject: [PATCH 2/4] (GH-1729) Make nonfunctional secret extension manifests invalid --- CHANGELOG.md | 18 ++ .../schemas/extension/manifest/root.md | 5 +- .../schemas/extension/manifest/secret.md | 28 +- dsc/tests/dsc_extension_secret.tests.ps1 | 62 +++++ lib/dsc-lib/locales/en-us.toml | 6 + .../src/discovery/command_discovery.rs | 3 + lib/dsc-lib/src/extensions/secret.rs | 261 ++++++++++++++++-- .../deprecated/deprecated.dsc.manifests.json | 5 +- tools/dsctest/src/args.rs | 5 +- tools/dsctest/src/main.rs | 4 +- 10 files changed, 361 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ae6e6f41..bc5d85e86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -84,6 +84,23 @@ changes since the last release, see the [diff on GitHub][unreleased]. before applying them. Passing `what_if` with an operation that doesn't support it returns an invalid parameters error. +### Changed + +- Extensions that implement the `secret` capability must now define the `args` property for the + `secret` command with exactly one secret name input argument (`nameArg`) and at most one vault + input argument (`vaultArg`). Previously, an extension could omit the secret name input argument, + which left DSC with no way to tell the extension which secret to retrieve. DSC no longer loads an + extension manifest that defines a nonfunctional `secret` command and logs an informational + message explaining the problem. The extension manifest JSON schema enforces the same + requirements. + +
Related work items + + - Issues: [#1729][#1729] + - PRs: _None_ + +
+ ## [v3.2.2][release-v3.2.2] - 2026-06-16 This section includes a summary of changes for the `3.2.2` release. For the full list of changes @@ -1982,4 +1999,5 @@ Version `3.0.0` is the first generally available release of DSC. [#1557]: https://github.com/PowerShell/DSC/issues/1557 [#1558]: https://github.com/PowerShell/DSC/issues/1558 [#1562]: https://github.com/PowerShell/DSC/issues/1562 +[#1729]: https://github.com/PowerShell/DSC/issues/1729 diff --git a/docs/reference/schemas/extension/manifest/root.md b/docs/reference/schemas/extension/manifest/root.md index 3bcf8eb3f..25b1f68be 100644 --- a/docs/reference/schemas/extension/manifest/root.md +++ b/docs/reference/schemas/extension/manifest/root.md @@ -317,7 +317,10 @@ runtime. When this property is defined, the extension has the `secret` capabilit invoke the extension for the [secret()][05] configuration function. The value of this property must be an object. The object's `executable` property, defining the name -of the command to call, is mandatory. The `args` property is optional. For more information, see +of the command to call, and `args` property, defining the arguments to pass to the command, are +both mandatory. The `args` property must define the secret name input argument exactly once and may +define the vault input argument at most once. DSC doesn't load an extension manifest that defines +the `secret` property without a secret name input argument. For more information, see [DSC extension manifest secret property schema reference][06]. ```yaml diff --git a/docs/reference/schemas/extension/manifest/secret.md b/docs/reference/schemas/extension/manifest/secret.md index 2a159246f..723e77b96 100644 --- a/docs/reference/schemas/extension/manifest/secret.md +++ b/docs/reference/schemas/extension/manifest/secret.md @@ -35,13 +35,14 @@ that extension. The `secret` definition must include these properties: - [executable](#executable) +- [args](#args) ## Properties ### executable The `executable` property defines the name of the command to run. The value must be the name of a -command secretable in the system's `PATH` environment variable or the full path to the command. A +command discoverable in the system's `PATH` environment variable or the full path to the command. A file extension is only required when the command isn't recognizable by the operating system as an executable. @@ -56,15 +57,22 @@ The `args` property defines the list of arguments to pass to the command. Each i can be a string representing a static argument, a [name argument](#name-argument) object, or a [vault argument](#vault-argument) object. -The array should contain exactly one name argument. It may contain a single vault argument and any +The array must contain exactly one name argument. It may contain at most one vault argument and any number of static string arguments. -If the array doesn't define a name argument, DSC can't pass the secret name to the extension. If -the array doesn't define a vault argument, DSC can't pass the vault name to the extension. +Without a name argument, DSC can't pass the secret name to the extension, so the extension can't +retrieve a specific secret. DSC doesn't load an extension manifest that defines the `secret` +property without the `args` property, without a name argument, with more than one name argument, +or with more than one vault argument. When DSC skips a manifest for one of these reasons, it logs +an informational message that explains the problem. Use the `--trace-level info` option, like +`dsc --trace-level info extension list`, to see the message. + +If the array doesn't define a vault argument, DSC can't pass the vault name to the extension, so +the extension can't retrieve a secret from a specific vault. ```yaml Type: array -Required: false +Required: true ItemsType: [string, object(Name or Vault argument)] ``` @@ -79,11 +87,12 @@ Type: string #### Name argument -Defines an argument that receives the path to the file to import. +Defines the argument that receives the name of the secret to retrieve. -DSC passes the value of `nameArg` followed by the name of the secret to retrieve. +DSC passes the value of `nameArg` followed by the name of the secret to retrieve. The `args` array +must define this argument exactly once. -A file argument is defined as a JSON object with the following properties: +A name argument is defined as a JSON object with the following properties: - `nameArg` (required) - The argument to pass before the secret name, like `--secret-name`. @@ -97,7 +106,8 @@ RequiredProperties: [nameArg] Defines an argument that receives the name of a specific vault to retrieve a secret from. DSC passes the value of `vaultArg` followed by the name of the vault when the `secret()` function -specifies a vault. When the function doesn't specify a vault, DSC ignores the vault argument. +specifies a vault. When the function doesn't specify a vault, DSC ignores the vault argument. The +`args` array may define this argument at most once. A vault argument is defined as a JSON object with the following properties: diff --git a/dsc/tests/dsc_extension_secret.tests.ps1 b/dsc/tests/dsc_extension_secret.tests.ps1 index a17659b0e..4b4c3fddd 100644 --- a/dsc/tests/dsc_extension_secret.tests.ps1 +++ b/dsc/tests/dsc_extension_secret.tests.ps1 @@ -200,4 +200,66 @@ Describe 'Tests for the secret() function and extensions' { $env:DSC_RESTRICTED_PATH = $null } } + + It 'Secret extension manifest is not loaded' -TestCases @( + @{ reason = 'without args'; secret = '{ "executable": "dsctest" }'; expectedError = 'missing field *args*' } + @{ reason = 'without a name argument'; secret = '{ "executable": "dsctest", "args": ["no-op"] }'; expectedError = "The 'secret' command doesn't define the secret name input argument" } + @{ reason = 'with multiple name arguments'; secret = '{ "executable": "dsctest", "args": ["no-op", { "nameArg": "--name" }, { "nameArg": "--secret" }] }'; expectedError = "The 'secret' command defines the secret name input argument 2 times" } + @{ reason = 'with multiple vault arguments'; secret = '{ "executable": "dsctest", "args": ["no-op", { "nameArg": "--name" }, { "vaultArg": "--vault" }, { "vaultArg": "--store" }] }'; expectedError = "The 'secret' command defines the vault input argument 2 times" } + ) { + param($secret, $expectedError) + + $manifest = @" +{ + "`$schema": "https://aka.ms/dsc/schemas/v3/bundled/extension/manifest.json", + "type": "Test/SecretInvalid", + "version": "0.1.0", + "description": "Invalid secret extension for testing.", + "secret": $secret +} +"@ + + try { + $env:DSC_RESTRICTED_PATH = $TestDrive + Set-Content -Path "$TestDrive/secretInvalid.dsc.extension.json" -Value $manifest + $out = dsc -l info extension list 2> $TestDrive/error.log | ConvertFrom-Json + $errorLog = Get-Content -Raw -Path $TestDrive/error.log + $LASTEXITCODE | Should -Be 0 -Because $errorLog + @($out).type | Should -Not -Contain 'Test/SecretInvalid' + $errorLog | Should -BeLike "*INFO Failed to load manifest: *$expectedError*" -Because $errorLog + } finally { + $env:DSC_RESTRICTED_PATH = $null + } + } + + It 'Secret extension manifest with a name argument and a vault argument is loaded' { + $manifest = @' +{ + "$schema": "https://aka.ms/dsc/schemas/v3/bundled/extension/manifest.json", + "type": "Test/SecretValid", + "version": "0.1.0", + "description": "Valid secret extension for testing.", + "secret": { + "executable": "dsctest", + "args": [ + "no-op", + { "vaultArg": "--vault" }, + { "nameArg": "--name" } + ] + } +} +'@ + + try { + $env:DSC_RESTRICTED_PATH = $TestDrive + Set-Content -Path "$TestDrive/secretValid.dsc.extension.json" -Value $manifest + $out = dsc extension list 2> $TestDrive/error.log | ConvertFrom-Json + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw -Path $TestDrive/error.log) + @($out).Count | Should -Be 1 + $out.type | Should -BeExactly 'Test/SecretValid' + $out.capabilities | Should -BeExactly @('secret') + } finally { + $env:DSC_RESTRICTED_PATH = $null + } + } } diff --git a/lib/dsc-lib/locales/en-us.toml b/lib/dsc-lib/locales/en-us.toml index 7ec10b494..bafdff849 100644 --- a/lib/dsc-lib/locales/en-us.toml +++ b/lib/dsc-lib/locales/en-us.toml @@ -138,6 +138,7 @@ adaptedResourceFound = "Adapted resource '%{resource}' version %{version} found" executableNotFound = "Executable '%{executable}' not found for operation '%{operation}' for resource '%{resource}'" invalidResourceManifest = "Invalid manifest for resource '%{resource}': %{err}" invalidExtensionManifest = "Invalid manifest for extension '%{resource}': %{err}" +invalidSecretExtensionManifest = "Invalid 'secret' definition for extension '%{extension}' in manifest '%{path}': %{err}" invalidAdaptedResourceManifest = "Invalid manifest for adapted resource '%{resource}': %{err}" invalidManifestList = "Invalid manifest list '%{resource}': %{err}" invalidManifestFile = "Invalid manifest file '%{resource}': %{err}" @@ -303,6 +304,11 @@ manifestImported = "Manifest imported from extension %{extension}" extensionManifestSchemaTitle = "Extension manifest schema URI" extensionManifestSchemaDescription = "Defines the JSON Schema the extension manifest adheres to." +[extensions.secret] +missingNameArg = "The 'secret' command doesn't define the secret name input argument, so DSC can't pass the name of the secret to retrieve to the extension. Define exactly one argument in 'secret.args' as an object with the 'nameArg' property" +multipleNameArgs = "The 'secret' command defines the secret name input argument %{count} times. Define exactly one argument in 'secret.args' as an object with the 'nameArg' property" +multipleVaultArgs = "The 'secret' command defines the vault input argument %{count} times. Define at most one argument in 'secret.args' as an object with the 'vaultArg' property" + [functions] invalidArgType = "Invalid argument type" invalidArguments = "Invalid argument(s)" diff --git a/lib/dsc-lib/src/discovery/command_discovery.rs b/lib/dsc-lib/src/discovery/command_discovery.rs index e7c9eb2a5..3145bbe75 100644 --- a/lib/dsc-lib/src/discovery/command_discovery.rs +++ b/lib/dsc-lib/src/discovery/command_discovery.rs @@ -997,6 +997,9 @@ fn load_extension_manifest(path: &Path, manifest: &ExtensionManifest) -> Result< capabilities.push(dscextension::Capability::Discover); } if let Some(secret) = &manifest.secret { + if let Err(err) = secret.validate_args() { + return Err(DscError::InvalidManifest(t!("discovery.commandDiscovery.invalidSecretExtensionManifest", extension = manifest.r#type, path = path.to_string_lossy(), err = err).to_string())); + } verify_executable(&manifest.r#type, "secret", &secret.executable, path.parent().unwrap()); capabilities.push(dscextension::Capability::Secret); } diff --git a/lib/dsc-lib/src/extensions/secret.rs b/lib/dsc-lib/src/extensions/secret.rs index 6687ca51c..3bd351592 100644 --- a/lib/dsc-lib/src/extensions/secret.rs +++ b/lib/dsc-lib/src/extensions/secret.rs @@ -18,25 +18,29 @@ use crate::{ }; use rust_i18n::t; -use schemars::JsonSchema; +use schemars::{JsonSchema, Schema}; use serde::{Deserialize, Serialize}; +use serde_json::json; use tracing::{debug, warn}; #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] #[serde(untagged)] #[schemars(inline)] pub enum SecretArgKind { - /// The argument is a string. + /// A static string argument to pass to the command, like `get` or `--quiet`. String(String), - /// The argument accepts the secret name. + /// The argument that accepts the secret name. DSC passes the name of the secret to retrieve + /// after this argument. The arguments must define this argument exactly once. Name { - /// The argument that accepts the secret name. + /// The argument that accepts the secret name, like `--name` or `--secret-name`. #[serde(rename = "nameArg")] name_arg: String, }, - /// The argument accepts the vault name. + /// The argument that accepts the vault name. DSC passes the name of the vault after this + /// argument when the `secret()` function specifies a vault. The arguments may define this + /// argument at most once. Vault { - /// The argument that accepts the vault name. + /// The argument that accepts the vault name, like `--vault` or `--vault-name`. #[serde(rename = "vaultArg")] vault_arg: String, }, @@ -45,14 +49,94 @@ pub enum SecretArgKind { #[derive(Debug, Default, Clone, PartialEq, Deserialize, Serialize, JsonSchema, DscRepoSchema)] #[schemars( transform = SecretMethod::transform_export_schema_uris, - transform = SecretMethod::transform_schema_docs + transform = SecretMethod::transform_schema_docs, + transform = SecretMethod::transform_args_constraints )] #[dsc_repo_schema(base_name = "manifest.secret", folder_path = "extension")] pub struct SecretMethod { - /// The command to run to get the state of the resource. + /// The command to run to retrieve a secret. pub executable: String, - /// The arguments to pass to the command to perform a Get. - pub args: Option>, + /// The arguments to pass to the command to retrieve a secret. The arguments must define the + /// secret name input argument exactly once and may define the vault input argument at most + /// once. + pub args: Vec, +} + +impl SecretMethod { + /// Validates that the arguments define the secret name input argument exactly once and the + /// vault input argument at most once. + /// + /// Without the secret name input argument, DSC can't tell the extension which secret to + /// retrieve, so the extension can't function as a secret provider. + /// + /// # Errors + /// + /// Returns [`DscError::InvalidManifest`] when the arguments don't define the secret name input + /// argument, define it more than once, or define the vault input argument more than once. + pub fn validate_args(&self) -> Result<(), DscError> { + let name_arg_count = self.args.iter().filter(|arg| matches!(arg, SecretArgKind::Name { .. })).count(); + let vault_arg_count = self.args.iter().filter(|arg| matches!(arg, SecretArgKind::Vault { .. })).count(); + + if name_arg_count == 0 { + return Err(DscError::InvalidManifest(t!("extensions.secret.missingNameArg").to_string())); + } + if name_arg_count > 1 { + return Err(DscError::InvalidManifest(t!("extensions.secret.multipleNameArgs", count = name_arg_count).to_string())); + } + if vault_arg_count > 1 { + return Err(DscError::InvalidManifest(t!("extensions.secret.multipleVaultArgs", count = vault_arg_count).to_string())); + } + + Ok(()) + } + + /// Adds the validation subschemas that require `args` to define the secret name input + /// argument exactly once and the vault input argument at most once. + /// + /// The subschemas are defined separately so that each failure reports a specific message. + /// The `errorMessage` keyword is only emitted in the VS Code form of the schema. + fn transform_args_constraints(schema: &mut Schema) { + let docs_url = "https://learn.microsoft.com/powershell/dsc/reference/schemas/extension/manifest/secret"; + schema.insert("allOf".to_string(), json!([ + { + "title": "Missing secret name input argument", + "properties": { + "args": { + "errorMessage": format!( + "The `secret` command doesn't define the secret name input argument. If you don't define the secret name input argument, DSC can't pass the secret name to the extension for retrieval. You must define exactly one argument in `secret.args` as a JSON object with the `nameArg` property. For more information, see: {docs_url}" + ), + "contains": { "type": "object", "required": ["nameArg"] }, + "minContains": 1 + } + } + }, + { + "title": "Multiple secret name input arguments", + "properties": { + "args": { + "errorMessage": format!( + "The `secret` command defines the secret name input argument more than once. You must define exactly one argument in `secret.args` as a JSON object with the `nameArg` property and remove the additional secret name input arguments. For more information, see: {docs_url}" + ), + "contains": { "type": "object", "required": ["nameArg"] }, + "maxContains": 1 + } + } + }, + { + "title": "Multiple vault input arguments", + "properties": { + "args": { + "errorMessage": format!( + "The `secret` command defines the vault input argument more than once. You can define at most one argument in `secret.args` as a JSON object with the `vaultArg` property. For more information, see: {docs_url}" + ), + "contains": { "type": "object", "required": ["vaultArg"] }, + "minContains": 0, + "maxContains": 1 + } + } + } + ])); + } } impl DscExtension { @@ -82,13 +166,14 @@ impl DscExtension { let Some(secret) = extension.secret else { return Err(DscError::UnsupportedCapability(self.type_name.to_string(), Capability::Secret.to_string())); }; - let args = process_secret_args(secret.args.as_ref(), name, vault); + secret.validate_args()?; + let args = process_secret_args(&secret.args, name, vault); if let Some(deprecation_message) = extension.deprecation_message.as_ref() { warn!("{}", t!("extensions.dscextension.deprecationMessage", extension = self.type_name, message = deprecation_message)); } let (_exit_code, stdout, _stderr) = invoke_command( &secret.executable, - args, + Some(args), vault, Some(&self.directory), None, @@ -120,30 +205,162 @@ impl DscExtension { } } -fn process_secret_args(args: Option<&Vec>, name: &str, vault: Option<&str>) -> Option> { - let Some(arg_values) = args else { - debug!("{}", t!("dscresources.commandResource.noArgs")); - return None; - }; - +fn process_secret_args(args: &[SecretArgKind], name: &str, vault: Option<&str>) -> Vec { let mut processed_args = Vec::::new(); - for arg in arg_values { + for arg in args { match arg { SecretArgKind::String(s) => { processed_args.push(s.clone()); }, SecretArgKind::Name { name_arg } => { - processed_args.push(name_arg.to_string()); + processed_args.push(name_arg.clone()); processed_args.push(name.to_string()); }, SecretArgKind::Vault { vault_arg } => { if let Some(value) = vault { - processed_args.push(vault_arg.to_string()); + processed_args.push(vault_arg.clone()); processed_args.push(value.to_string()); } }, } } - Some(processed_args) + processed_args +} + +#[cfg(test)] +mod test { + use super::{SecretArgKind, SecretMethod, process_secret_args}; + use crate::dscerror::DscError; + use crate::extensions::extension_manifest::ExtensionManifest; + use crate::schemas::dsc_repo::DscRepoSchema; + use schemars::schema_for; + use serde_json::{Value, json}; + + fn name_arg() -> SecretArgKind { + SecretArgKind::Name { name_arg: "--name".to_string() } + } + + fn vault_arg() -> SecretArgKind { + SecretArgKind::Vault { vault_arg: "--vault".to_string() } + } + + fn secret_method(args: Vec) -> SecretMethod { + SecretMethod { executable: "secret".to_string(), args } + } + + #[test] + fn validate_args_accepts_single_name_arg() { + let method = secret_method(vec![SecretArgKind::String("get".to_string()), name_arg()]); + assert!(method.validate_args().is_ok()); + } + + #[test] + fn validate_args_accepts_single_name_and_vault_arg() { + let method = secret_method(vec![vault_arg(), name_arg()]); + assert!(method.validate_args().is_ok()); + } + + #[test] + fn validate_args_rejects_missing_name_arg() { + let method = secret_method(vec![SecretArgKind::String("get".to_string()), vault_arg()]); + let err = method.validate_args().unwrap_err(); + assert!(matches!(err, DscError::InvalidManifest(_))); + assert!(err.to_string().contains("doesn't define the secret name input argument"), "{err}"); + } + + #[test] + fn validate_args_rejects_empty_args() { + let method = secret_method(vec![]); + assert!(method.validate_args().is_err()); + } + + #[test] + fn validate_args_rejects_multiple_name_args() { + let method = secret_method(vec![name_arg(), name_arg()]); + let err = method.validate_args().unwrap_err(); + assert!(matches!(err, DscError::InvalidManifest(_))); + assert!(err.to_string().contains("defines the secret name input argument 2 times"), "{err}"); + } + + #[test] + fn validate_args_rejects_multiple_vault_args() { + let method = secret_method(vec![name_arg(), vault_arg(), vault_arg()]); + let err = method.validate_args().unwrap_err(); + assert!(matches!(err, DscError::InvalidManifest(_))); + assert!(err.to_string().contains("defines the vault input argument 2 times"), "{err}"); + } + + #[test] + fn process_secret_args_preserves_order_and_inserts_values() { + let args = vec![SecretArgKind::String("get".to_string()), vault_arg(), name_arg(), SecretArgKind::String("--quiet".to_string())]; + let processed = process_secret_args(&args, "apiToken", Some("services")); + assert_eq!(processed, vec!["get", "--vault", "services", "--name", "apiToken", "--quiet"]); + } + + #[test] + fn process_secret_args_omits_vault_arg_without_vault() { + let args = vec![SecretArgKind::String("get".to_string()), name_arg(), vault_arg()]; + let processed = process_secret_args(&args, "apiToken", None); + assert_eq!(processed, vec!["get", "--name", "apiToken"]); + } + + #[test] + fn manifest_without_secret_args_fails_to_deserialize() { + let manifest = json!({ + "$schema": ExtensionManifest::default_schema_id_uri(), + "type": "Test/Secret", + "version": "0.1.0", + "secret": { "executable": "secret" } + }); + let err = serde_json::from_value::(manifest).unwrap_err(); + assert!(err.to_string().contains("missing field `args`"), "{err}"); + } + + #[test] + fn manifest_with_secret_args_deserializes() { + let manifest = json!({ + "$schema": ExtensionManifest::default_schema_id_uri(), + "type": "Test/Secret", + "version": "0.1.0", + "secret": { + "executable": "secret", + "args": ["get", { "nameArg": "--name" }, { "vaultArg": "--vault" }] + } + }); + let manifest = serde_json::from_value::(manifest).unwrap(); + let secret = manifest.secret.unwrap(); + assert_eq!(secret.args, vec![SecretArgKind::String("get".to_string()), name_arg(), vault_arg()]); + assert!(secret.validate_args().is_ok()); + } + + #[test] + fn schema_requires_args_and_constrains_input_arguments() { + let schema = schema_for!(SecretMethod).to_value(); + let required = schema["required"].as_array().unwrap(); + assert!(required.contains(&Value::String("executable".to_string())), "{schema}"); + assert!(required.contains(&Value::String("args".to_string())), "{schema}"); + + let constraints = schema["allOf"].as_array().unwrap(); + assert_eq!(constraints.len(), 3, "{schema}"); + assert_eq!(constraints[0]["properties"]["args"]["minContains"], json!(1)); + assert_eq!(constraints[0]["properties"]["args"]["contains"]["required"], json!(["nameArg"])); + assert_eq!(constraints[1]["properties"]["args"]["maxContains"], json!(1)); + assert_eq!(constraints[1]["properties"]["args"]["contains"]["required"], json!(["nameArg"])); + assert_eq!(constraints[2]["properties"]["args"]["maxContains"], json!(1)); + assert_eq!(constraints[2]["properties"]["args"]["contains"]["required"], json!(["vaultArg"])); + } + + #[test] + fn schema_rejects_args_without_name_arg() { + let schema = schema_for!(SecretMethod).to_value(); + let validator = jsonschema::validator_for(&schema).unwrap(); + + assert!(validator.is_valid(&json!({ "executable": "secret", "args": ["get", { "nameArg": "--name" }] }))); + assert!(validator.is_valid(&json!({ "executable": "secret", "args": [{ "vaultArg": "--vault" }, { "nameArg": "--name" }] }))); + assert!(!validator.is_valid(&json!({ "executable": "secret" }))); + assert!(!validator.is_valid(&json!({ "executable": "secret", "args": ["get"] }))); + assert!(!validator.is_valid(&json!({ "executable": "secret", "args": [{ "nameArg": "--name" }, { "nameArg": "--secret" }] }))); + assert!(!validator.is_valid(&json!({ "executable": "secret", "args": [{ "nameArg": "--name" }, { "vaultArg": "--vault" }, { "vaultArg": "--store" }] }))); + } } diff --git a/tools/dsctest/deprecated/deprecated.dsc.manifests.json b/tools/dsctest/deprecated/deprecated.dsc.manifests.json index ceb5e1ee6..3b76e007b 100644 --- a/tools/dsctest/deprecated/deprecated.dsc.manifests.json +++ b/tools/dsctest/deprecated/deprecated.dsc.manifests.json @@ -66,7 +66,10 @@ "secret": { "executable": "dsctest", "args": [ - "no-op" + "no-op", + { + "nameArg": "--name" + } ] } } diff --git a/tools/dsctest/src/args.rs b/tools/dsctest/src/args.rs index 93d36cd62..fdfb11691 100644 --- a/tools/dsctest/src/args.rs +++ b/tools/dsctest/src/args.rs @@ -135,7 +135,10 @@ pub enum SubCommand { }, #[clap(name = "no-op", about = "Perform no operation, just return success")] - NoOp, + NoOp { + #[clap(name = "args", help = "Arguments that are ignored, allows testing manifests that pass input arguments", trailing_var_arg = true, allow_hyphen_values = true)] + args: Vec, + }, #[clap(name = "operation", about = "Perform an operation")] Operation { diff --git a/tools/dsctest/src/main.rs b/tools/dsctest/src/main.rs index b195c7e6a..6c078e2b6 100644 --- a/tools/dsctest/src/main.rs +++ b/tools/dsctest/src/main.rs @@ -263,8 +263,8 @@ fn run() -> Result<(), u8> { } String::new() }, - SubCommand::NoOp => { - // do nothing and just return success + SubCommand::NoOp { args: _args } => { + // do nothing and just return success, any arguments are ignored String::new() }, SubCommand::Operation { operation, input } => { From 1f902e20793ab62a2300ddf3b8722d5e78df720f Mon Sep 17 00:00:00 2001 From: "G.Reijn" <26114636+Gijsreyn@users.noreply.github.com> Date: Sun, 11 Oct 2026 14:33:10 +0200 Subject: [PATCH 3/4] (GH-1729) Address review feedback on secret extension validation - Validate secret args during deserialization through a SecretArgs newtype - Define the schema constraints with the schemars attribute and localize them - Reject argument objects with unknown properties - Restore DSC_RESTRICTED_PATH in the new tests --- .../schemas/extension/manifest/secret.md | 20 +- dsc/tests/dsc_extension_secret.tests.ps1 | 7 +- lib/dsc-lib/locales/en-us.toml | 2 +- lib/dsc-lib/locales/schemas.extension.yaml | 37 ++ .../src/discovery/command_discovery.rs | 3 - lib/dsc-lib/src/extensions/secret.rs | 328 +++++++++++------- 6 files changed, 263 insertions(+), 134 deletions(-) create mode 100644 lib/dsc-lib/locales/schemas.extension.yaml diff --git a/docs/reference/schemas/extension/manifest/secret.md b/docs/reference/schemas/extension/manifest/secret.md index 723e77b96..dad2623a1 100644 --- a/docs/reference/schemas/extension/manifest/secret.md +++ b/docs/reference/schemas/extension/manifest/secret.md @@ -63,8 +63,9 @@ number of static string arguments. Without a name argument, DSC can't pass the secret name to the extension, so the extension can't retrieve a specific secret. DSC doesn't load an extension manifest that defines the `secret` property without the `args` property, without a name argument, with more than one name argument, -or with more than one vault argument. When DSC skips a manifest for one of these reasons, it logs -an informational message that explains the problem. Use the `--trace-level info` option, like +with more than one vault argument, or with an item that isn't a string, a name argument, or a +vault argument. When DSC skips a manifest for one of these reasons, it logs an informational +message that explains the problem. Use the `--trace-level info` option, like `dsc --trace-level info extension list`, to see the message. If the array doesn't define a vault argument, DSC can't pass the vault name to the extension, so @@ -96,9 +97,13 @@ A name argument is defined as a JSON object with the following properties: - `nameArg` (required) - The argument to pass before the secret name, like `--secret-name`. +The object must not define any other properties. An object that defines both `nameArg` and +`vaultArg` isn't a valid argument. + ```yaml -Type: object -RequiredProperties: [nameArg] +Type: object +RequiredProperties: [nameArg] +AdditionalProperties: false ``` #### Vault argument @@ -114,9 +119,12 @@ A vault argument is defined as a JSON object with the following properties: - `vaultArg` (required) - The argument to pass before the name of the vault to retrieve a secret from, like `--vault-name`. +The object must not define any other properties. + ```yaml -Type: object -RequiredProperties: [vaultArg] +Type: object +RequiredProperties: [vaultArg] +AdditionalProperties: false ``` diff --git a/dsc/tests/dsc_extension_secret.tests.ps1 b/dsc/tests/dsc_extension_secret.tests.ps1 index 4b4c3fddd..2d22e8205 100644 --- a/dsc/tests/dsc_extension_secret.tests.ps1 +++ b/dsc/tests/dsc_extension_secret.tests.ps1 @@ -206,6 +206,7 @@ Describe 'Tests for the secret() function and extensions' { @{ reason = 'without a name argument'; secret = '{ "executable": "dsctest", "args": ["no-op"] }'; expectedError = "The 'secret' command doesn't define the secret name input argument" } @{ reason = 'with multiple name arguments'; secret = '{ "executable": "dsctest", "args": ["no-op", { "nameArg": "--name" }, { "nameArg": "--secret" }] }'; expectedError = "The 'secret' command defines the secret name input argument 2 times" } @{ reason = 'with multiple vault arguments'; secret = '{ "executable": "dsctest", "args": ["no-op", { "nameArg": "--name" }, { "vaultArg": "--vault" }, { "vaultArg": "--store" }] }'; expectedError = "The 'secret' command defines the vault input argument 2 times" } + @{ reason = 'with an argument that defines both nameArg and vaultArg'; secret = '{ "executable": "dsctest", "args": ["no-op", { "nameArg": "--name", "vaultArg": "--vault" }] }'; expectedError = "Argument 2 of the 'secret' command isn't valid" } ) { param($secret, $expectedError) @@ -219,6 +220,7 @@ Describe 'Tests for the secret() function and extensions' { } "@ + $oldRestrictedPath = $env:DSC_RESTRICTED_PATH try { $env:DSC_RESTRICTED_PATH = $TestDrive Set-Content -Path "$TestDrive/secretInvalid.dsc.extension.json" -Value $manifest @@ -228,7 +230,7 @@ Describe 'Tests for the secret() function and extensions' { @($out).type | Should -Not -Contain 'Test/SecretInvalid' $errorLog | Should -BeLike "*INFO Failed to load manifest: *$expectedError*" -Because $errorLog } finally { - $env:DSC_RESTRICTED_PATH = $null + $env:DSC_RESTRICTED_PATH = $oldRestrictedPath } } @@ -250,6 +252,7 @@ Describe 'Tests for the secret() function and extensions' { } '@ + $oldRestrictedPath = $env:DSC_RESTRICTED_PATH try { $env:DSC_RESTRICTED_PATH = $TestDrive Set-Content -Path "$TestDrive/secretValid.dsc.extension.json" -Value $manifest @@ -259,7 +262,7 @@ Describe 'Tests for the secret() function and extensions' { $out.type | Should -BeExactly 'Test/SecretValid' $out.capabilities | Should -BeExactly @('secret') } finally { - $env:DSC_RESTRICTED_PATH = $null + $env:DSC_RESTRICTED_PATH = $oldRestrictedPath } } } diff --git a/lib/dsc-lib/locales/en-us.toml b/lib/dsc-lib/locales/en-us.toml index bafdff849..b8256b92c 100644 --- a/lib/dsc-lib/locales/en-us.toml +++ b/lib/dsc-lib/locales/en-us.toml @@ -138,7 +138,6 @@ adaptedResourceFound = "Adapted resource '%{resource}' version %{version} found" executableNotFound = "Executable '%{executable}' not found for operation '%{operation}' for resource '%{resource}'" invalidResourceManifest = "Invalid manifest for resource '%{resource}': %{err}" invalidExtensionManifest = "Invalid manifest for extension '%{resource}': %{err}" -invalidSecretExtensionManifest = "Invalid 'secret' definition for extension '%{extension}' in manifest '%{path}': %{err}" invalidAdaptedResourceManifest = "Invalid manifest for adapted resource '%{resource}': %{err}" invalidManifestList = "Invalid manifest list '%{resource}': %{err}" invalidManifestFile = "Invalid manifest file '%{resource}': %{err}" @@ -308,6 +307,7 @@ extensionManifestSchemaDescription = "Defines the JSON Schema the extension mani missingNameArg = "The 'secret' command doesn't define the secret name input argument, so DSC can't pass the name of the secret to retrieve to the extension. Define exactly one argument in 'secret.args' as an object with the 'nameArg' property" multipleNameArgs = "The 'secret' command defines the secret name input argument %{count} times. Define exactly one argument in 'secret.args' as an object with the 'nameArg' property" multipleVaultArgs = "The 'secret' command defines the vault input argument %{count} times. Define at most one argument in 'secret.args' as an object with the 'vaultArg' property" +invalidArg = "Argument %{position} of the 'secret' command isn't valid. Each argument must be a string, an object with only the 'nameArg' property, or an object with only the 'vaultArg' property. Invalid argument: %{arg}" [functions] invalidArgType = "Invalid argument type" diff --git a/lib/dsc-lib/locales/schemas.extension.yaml b/lib/dsc-lib/locales/schemas.extension.yaml new file mode 100644 index 000000000..f6c4d16bc --- /dev/null +++ b/lib/dsc-lib/locales/schemas.extension.yaml @@ -0,0 +1,37 @@ +_version: 2 +schemas: + extension: + manifest: + secret: + args: + constraints: + nameArg: + title: + en-us: Secret name input argument + description: + en-us: >- + The arguments must define exactly one object with the `nameArg` property. + markdownDescription: + en-us: |- + The arguments must define exactly one object with the `nameArg` property. DSC passes + the value of `nameArg` followed by the name of the secret to retrieve. Without this + argument, DSC can't tell the extension which secret to retrieve. + errorMessage: + en-us: >- + The `secret` command must define the secret name input argument exactly once. Define + exactly one argument in `secret.args` as an object with the `nameArg` property. + vaultArg: + title: + en-us: Vault input argument + description: + en-us: >- + The arguments may define at most one object with the `vaultArg` property. + markdownDescription: + en-us: |- + The arguments may define at most one object with the `vaultArg` property. DSC passes + the value of `vaultArg` followed by the name of the vault when the `secret()` function + specifies a vault. + errorMessage: + en-us: >- + The `secret` command defines the vault input argument more than once. Define at most + one argument in `secret.args` as an object with the `vaultArg` property. diff --git a/lib/dsc-lib/src/discovery/command_discovery.rs b/lib/dsc-lib/src/discovery/command_discovery.rs index 3145bbe75..e7c9eb2a5 100644 --- a/lib/dsc-lib/src/discovery/command_discovery.rs +++ b/lib/dsc-lib/src/discovery/command_discovery.rs @@ -997,9 +997,6 @@ fn load_extension_manifest(path: &Path, manifest: &ExtensionManifest) -> Result< capabilities.push(dscextension::Capability::Discover); } if let Some(secret) = &manifest.secret { - if let Err(err) = secret.validate_args() { - return Err(DscError::InvalidManifest(t!("discovery.commandDiscovery.invalidSecretExtensionManifest", extension = manifest.r#type, path = path.to_string_lossy(), err = err).to_string())); - } verify_executable(&manifest.r#type, "secret", &secret.executable, path.parent().unwrap()); capabilities.push(dscextension::Capability::Secret); } diff --git a/lib/dsc-lib/src/extensions/secret.rs b/lib/dsc-lib/src/extensions/secret.rs index 3bd351592..d0af8620a 100644 --- a/lib/dsc-lib/src/extensions/secret.rs +++ b/lib/dsc-lib/src/extensions/secret.rs @@ -14,17 +14,18 @@ use crate::{ }, extension_manifest::ExtensionManifest, }, - schemas::dsc_repo::DscRepoSchema + schemas::dsc_repo::{DscRepoSchema, schema_i18n} }; use rust_i18n::t; -use schemars::{JsonSchema, Schema}; +use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use serde_json::json; +use serde_json::Value; +use std::ops::Deref; use tracing::{debug, warn}; #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] -#[serde(untagged)] +#[serde(untagged, deny_unknown_fields)] #[schemars(inline)] pub enum SecretArgKind { /// A static string argument to pass to the command, like `get` or `--quiet`. @@ -46,36 +47,66 @@ pub enum SecretArgKind { }, } -#[derive(Debug, Default, Clone, PartialEq, Deserialize, Serialize, JsonSchema, DscRepoSchema)] -#[schemars( - transform = SecretMethod::transform_export_schema_uris, - transform = SecretMethod::transform_schema_docs, - transform = SecretMethod::transform_args_constraints -)] -#[dsc_repo_schema(base_name = "manifest.secret", folder_path = "extension")] -pub struct SecretMethod { - /// The command to run to retrieve a secret. - pub executable: String, - /// The arguments to pass to the command to retrieve a secret. The arguments must define the - /// secret name input argument exactly once and may define the vault input argument at most - /// once. - pub args: Vec, -} +/// Defines the arguments to pass to the `secret` command of an extension. +/// +/// The arguments must define the secret name input argument exactly once and may define the vault +/// input argument at most once. Without the secret name input argument, DSC can't tell the +/// extension which secret to retrieve, so the extension can't function as a secret provider. +/// +/// Deserializing arguments that break these rules fails, so a deserialized instance is always +/// valid and DSC doesn't need to validate the arguments again before invoking the extension. +#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] +#[serde(try_from = "Vec")] +#[schemars(inline, !try_from)] +pub struct SecretArgs(Vec); -impl SecretMethod { - /// Validates that the arguments define the secret name input argument exactly once and the - /// vault input argument at most once. +impl SecretArgs { + /// Creates the arguments for the `secret` command from a list of argument definitions. + /// + /// # Arguments /// - /// Without the secret name input argument, DSC can't tell the extension which secret to - /// retrieve, so the extension can't function as a secret provider. + /// * `args` - The argument definitions in the order to pass them to the command. /// /// # Errors /// /// Returns [`DscError::InvalidManifest`] when the arguments don't define the secret name input /// argument, define it more than once, or define the vault input argument more than once. - pub fn validate_args(&self) -> Result<(), DscError> { - let name_arg_count = self.args.iter().filter(|arg| matches!(arg, SecretArgKind::Name { .. })).count(); - let vault_arg_count = self.args.iter().filter(|arg| matches!(arg, SecretArgKind::Vault { .. })).count(); + pub fn new(args: Vec) -> Result { + Self::validate(&args)?; + Ok(Self(args)) + } + + /// Creates the arguments for the `secret` command from the JSON values of a manifest. + /// + /// Each value must be a string, an object with only the `nameArg` property, or an object with + /// only the `vaultArg` property. + /// + /// # Arguments + /// + /// * `values` - The JSON values of the `args` property in the order to pass them to the command. + /// + /// # Errors + /// + /// Returns [`DscError::InvalidManifest`] when a value isn't a valid argument definition or when + /// the arguments break the rules described for [`SecretArgs::new`]. + pub fn from_values(values: Vec) -> Result { + let mut args = Vec::with_capacity(values.len()); + for (index, value) in values.iter().enumerate() { + match SecretArgKind::deserialize(value) { + Ok(arg) => args.push(arg), + Err(_) => { + return Err(DscError::InvalidManifest(t!("extensions.secret.invalidArg", position = index + 1, arg = value).to_string())); + } + } + } + Self::new(args) + } + + /// Validates that the arguments define the secret name input argument exactly once and the + /// vault input argument at most once. + fn validate(args: &[SecretArgKind]) -> Result<(), DscError> { + let name_arg_count = args.iter().filter(|arg| matches!(arg, SecretArgKind::Name { .. })).count(); + let vault_arg_count = args.iter().filter(|arg| matches!(arg, SecretArgKind::Vault { .. })).count(); if name_arg_count == 0 { return Err(DscError::InvalidManifest(t!("extensions.secret.missingNameArg").to_string())); @@ -89,56 +120,67 @@ impl SecretMethod { Ok(()) } +} - /// Adds the validation subschemas that require `args` to define the secret name input - /// argument exactly once and the vault input argument at most once. - /// - /// The subschemas are defined separately so that each failure reports a specific message. - /// The `errorMessage` keyword is only emitted in the VS Code form of the schema. - fn transform_args_constraints(schema: &mut Schema) { - let docs_url = "https://learn.microsoft.com/powershell/dsc/reference/schemas/extension/manifest/secret"; - schema.insert("allOf".to_string(), json!([ - { - "title": "Missing secret name input argument", - "properties": { - "args": { - "errorMessage": format!( - "The `secret` command doesn't define the secret name input argument. If you don't define the secret name input argument, DSC can't pass the secret name to the extension for retrieval. You must define exactly one argument in `secret.args` as a JSON object with the `nameArg` property. For more information, see: {docs_url}" - ), - "contains": { "type": "object", "required": ["nameArg"] }, - "minContains": 1 - } - } - }, - { - "title": "Multiple secret name input arguments", - "properties": { - "args": { - "errorMessage": format!( - "The `secret` command defines the secret name input argument more than once. You must define exactly one argument in `secret.args` as a JSON object with the `nameArg` property and remove the additional secret name input arguments. For more information, see: {docs_url}" - ), - "contains": { "type": "object", "required": ["nameArg"] }, - "maxContains": 1 - } - } - }, - { - "title": "Multiple vault input arguments", - "properties": { - "args": { - "errorMessage": format!( - "The `secret` command defines the vault input argument more than once. You can define at most one argument in `secret.args` as a JSON object with the `vaultArg` property. For more information, see: {docs_url}" - ), - "contains": { "type": "object", "required": ["vaultArg"] }, - "minContains": 0, - "maxContains": 1 - } - } - } - ])); +impl TryFrom> for SecretArgs { + type Error = DscError; + + fn try_from(values: Vec) -> Result { + Self::from_values(values) + } +} + +impl TryFrom> for SecretArgs { + type Error = DscError; + + fn try_from(args: Vec) -> Result { + Self::new(args) } } +impl Deref for SecretArgs { + type Target = [SecretArgKind]; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema, DscRepoSchema)] +#[schemars( + transform = SecretMethod::transform_export_schema_uris, + transform = SecretMethod::transform_schema_docs +)] +#[dsc_repo_schema(base_name = "manifest.secret", folder_path = "extension")] +pub struct SecretMethod { + /// The command to run to retrieve a secret. + pub executable: String, + /// The arguments to pass to the command to retrieve a secret. The arguments must define the + /// secret name input argument exactly once and may define the vault input argument at most + /// once. + #[schemars(extend("allOf" = [ + { + "title": schema_i18n!("args.constraints.nameArg.title"), + "description": schema_i18n!("args.constraints.nameArg.description"), + "markdownDescription": schema_i18n!("args.constraints.nameArg.markdownDescription"), + "errorMessage": schema_i18n!("args.constraints.nameArg.errorMessage"), + "contains": { "type": "object", "required": ["nameArg"] }, + "minContains": 1, + "maxContains": 1, + }, + { + "title": schema_i18n!("args.constraints.vaultArg.title"), + "description": schema_i18n!("args.constraints.vaultArg.description"), + "markdownDescription": schema_i18n!("args.constraints.vaultArg.markdownDescription"), + "errorMessage": schema_i18n!("args.constraints.vaultArg.errorMessage"), + "contains": { "type": "object", "required": ["vaultArg"] }, + "minContains": 0, + "maxContains": 1, + }, + ]))] + pub args: SecretArgs, +} + impl DscExtension { /// Retrieve a secret using the extension. /// @@ -166,7 +208,6 @@ impl DscExtension { let Some(secret) = extension.secret else { return Err(DscError::UnsupportedCapability(self.type_name.to_string(), Capability::Secret.to_string())); }; - secret.validate_args()?; let args = process_secret_args(&secret.args, name, vault); if let Some(deprecation_message) = extension.deprecation_message.as_ref() { warn!("{}", t!("extensions.dscextension.deprecationMessage", extension = self.type_name, message = deprecation_message)); @@ -230,7 +271,7 @@ fn process_secret_args(args: &[SecretArgKind], name: &str, vault: Option<&str>) #[cfg(test)] mod test { - use super::{SecretArgKind, SecretMethod, process_secret_args}; + use super::{SecretArgKind, SecretArgs, SecretMethod, process_secret_args}; use crate::dscerror::DscError; use crate::extensions::extension_manifest::ExtensionManifest; use crate::schemas::dsc_repo::DscRepoSchema; @@ -245,52 +286,78 @@ mod test { SecretArgKind::Vault { vault_arg: "--vault".to_string() } } - fn secret_method(args: Vec) -> SecretMethod { - SecretMethod { executable: "secret".to_string(), args } + fn manifest_with_secret(secret: Value) -> Value { + json!({ + "$schema": ExtensionManifest::default_schema_id_uri(), + "type": "Test/Secret", + "version": "0.1.0", + "secret": secret + }) } #[test] - fn validate_args_accepts_single_name_arg() { - let method = secret_method(vec![SecretArgKind::String("get".to_string()), name_arg()]); - assert!(method.validate_args().is_ok()); + fn new_accepts_single_name_arg() { + let args = SecretArgs::new(vec![SecretArgKind::String("get".to_string()), name_arg()]).unwrap(); + assert_eq!(args.len(), 2); } #[test] - fn validate_args_accepts_single_name_and_vault_arg() { - let method = secret_method(vec![vault_arg(), name_arg()]); - assert!(method.validate_args().is_ok()); + fn new_accepts_single_name_and_vault_arg() { + assert!(SecretArgs::new(vec![vault_arg(), name_arg()]).is_ok()); } #[test] - fn validate_args_rejects_missing_name_arg() { - let method = secret_method(vec![SecretArgKind::String("get".to_string()), vault_arg()]); - let err = method.validate_args().unwrap_err(); + fn new_rejects_missing_name_arg() { + let err = SecretArgs::new(vec![SecretArgKind::String("get".to_string()), vault_arg()]).unwrap_err(); assert!(matches!(err, DscError::InvalidManifest(_))); assert!(err.to_string().contains("doesn't define the secret name input argument"), "{err}"); } #[test] - fn validate_args_rejects_empty_args() { - let method = secret_method(vec![]); - assert!(method.validate_args().is_err()); + fn new_rejects_empty_args() { + assert!(SecretArgs::new(vec![]).is_err()); } #[test] - fn validate_args_rejects_multiple_name_args() { - let method = secret_method(vec![name_arg(), name_arg()]); - let err = method.validate_args().unwrap_err(); + fn new_rejects_multiple_name_args() { + let err = SecretArgs::new(vec![name_arg(), name_arg()]).unwrap_err(); assert!(matches!(err, DscError::InvalidManifest(_))); assert!(err.to_string().contains("defines the secret name input argument 2 times"), "{err}"); } #[test] - fn validate_args_rejects_multiple_vault_args() { - let method = secret_method(vec![name_arg(), vault_arg(), vault_arg()]); - let err = method.validate_args().unwrap_err(); + fn new_rejects_multiple_vault_args() { + let err = SecretArgs::new(vec![name_arg(), vault_arg(), vault_arg()]).unwrap_err(); assert!(matches!(err, DscError::InvalidManifest(_))); assert!(err.to_string().contains("defines the vault input argument 2 times"), "{err}"); } + #[test] + fn from_values_converts_each_argument_kind() { + let args = SecretArgs::from_values(vec![json!("get"), json!({ "nameArg": "--name" }), json!({ "vaultArg": "--vault" })]).unwrap(); + assert_eq!(*args, [SecretArgKind::String("get".to_string()), name_arg(), vault_arg()]); + } + + #[test] + fn from_values_rejects_object_with_name_and_vault_arg() { + let err = SecretArgs::from_values(vec![json!("get"), json!({ "nameArg": "--name", "vaultArg": "--vault" })]).unwrap_err(); + assert!(matches!(err, DscError::InvalidManifest(_))); + assert!(err.to_string().contains("Argument 2 of the 'secret' command isn't valid"), "{err}"); + assert!(err.to_string().contains(r#"{"nameArg":"--name","vaultArg":"--vault"}"#), "{err}"); + } + + #[test] + fn from_values_rejects_object_with_unknown_property() { + let err = SecretArgs::from_values(vec![json!({ "nameArg": "--name", "other": true })]).unwrap_err(); + assert!(err.to_string().contains("Argument 1 of the 'secret' command isn't valid"), "{err}"); + } + + #[test] + fn from_values_rejects_non_string_scalar() { + let err = SecretArgs::from_values(vec![json!(5), json!({ "nameArg": "--name" })]).unwrap_err(); + assert!(err.to_string().contains("Argument 1 of the 'secret' command isn't valid"), "{err}"); + } + #[test] fn process_secret_args_preserves_order_and_inserts_values() { let args = vec![SecretArgKind::String("get".to_string()), vault_arg(), name_arg(), SecretArgKind::String("--quiet".to_string())]; @@ -307,31 +374,40 @@ mod test { #[test] fn manifest_without_secret_args_fails_to_deserialize() { - let manifest = json!({ - "$schema": ExtensionManifest::default_schema_id_uri(), - "type": "Test/Secret", - "version": "0.1.0", - "secret": { "executable": "secret" } - }); + let manifest = manifest_with_secret(json!({ "executable": "secret" })); let err = serde_json::from_value::(manifest).unwrap_err(); assert!(err.to_string().contains("missing field `args`"), "{err}"); } #[test] - fn manifest_with_secret_args_deserializes() { - let manifest = json!({ - "$schema": ExtensionManifest::default_schema_id_uri(), - "type": "Test/Secret", - "version": "0.1.0", - "secret": { - "executable": "secret", - "args": ["get", { "nameArg": "--name" }, { "vaultArg": "--vault" }] - } - }); + fn manifest_without_name_arg_fails_to_deserialize() { + let manifest = manifest_with_secret(json!({ "executable": "secret", "args": ["get"] })); + let err = serde_json::from_value::(manifest).unwrap_err(); + assert!(err.to_string().contains("doesn't define the secret name input argument"), "{err}"); + } + + #[test] + fn manifest_with_multiple_name_args_fails_to_deserialize() { + let manifest = manifest_with_secret(json!({ "executable": "secret", "args": [{ "nameArg": "--name" }, { "nameArg": "--secret" }] })); + let err = serde_json::from_value::(manifest).unwrap_err(); + assert!(err.to_string().contains("defines the secret name input argument 2 times"), "{err}"); + } + + #[test] + fn manifest_with_ambiguous_arg_fails_to_deserialize() { + let manifest = manifest_with_secret(json!({ "executable": "secret", "args": [{ "nameArg": "--name", "vaultArg": "--vault" }] })); + let err = serde_json::from_value::(manifest).unwrap_err(); + assert!(err.to_string().contains("Argument 1 of the 'secret' command isn't valid"), "{err}"); + } + + #[test] + fn manifest_with_secret_args_round_trips() { + let args = json!(["get", { "nameArg": "--name" }, { "vaultArg": "--vault" }]); + let manifest = manifest_with_secret(json!({ "executable": "secret", "args": args })); let manifest = serde_json::from_value::(manifest).unwrap(); let secret = manifest.secret.unwrap(); - assert_eq!(secret.args, vec![SecretArgKind::String("get".to_string()), name_arg(), vault_arg()]); - assert!(secret.validate_args().is_ok()); + assert_eq!(*secret.args, [SecretArgKind::String("get".to_string()), name_arg(), vault_arg()]); + assert_eq!(serde_json::to_value(&secret).unwrap()["args"], args); } #[test] @@ -341,18 +417,25 @@ mod test { assert!(required.contains(&Value::String("executable".to_string())), "{schema}"); assert!(required.contains(&Value::String("args".to_string())), "{schema}"); - let constraints = schema["allOf"].as_array().unwrap(); - assert_eq!(constraints.len(), 3, "{schema}"); - assert_eq!(constraints[0]["properties"]["args"]["minContains"], json!(1)); - assert_eq!(constraints[0]["properties"]["args"]["contains"]["required"], json!(["nameArg"])); - assert_eq!(constraints[1]["properties"]["args"]["maxContains"], json!(1)); - assert_eq!(constraints[1]["properties"]["args"]["contains"]["required"], json!(["nameArg"])); - assert_eq!(constraints[2]["properties"]["args"]["maxContains"], json!(1)); - assert_eq!(constraints[2]["properties"]["args"]["contains"]["required"], json!(["vaultArg"])); + let args = &schema["properties"]["args"]; + assert_eq!(args["type"], json!("array"), "{schema}"); + let constraints = args["allOf"].as_array().unwrap(); + assert_eq!(constraints.len(), 2, "{schema}"); + assert_eq!(constraints[0]["contains"]["required"], json!(["nameArg"])); + assert_eq!(constraints[0]["minContains"], json!(1)); + assert_eq!(constraints[0]["maxContains"], json!(1)); + assert_eq!(constraints[1]["contains"]["required"], json!(["vaultArg"])); + assert_eq!(constraints[1]["minContains"], json!(0)); + assert_eq!(constraints[1]["maxContains"], json!(1)); + for constraint in constraints { + for keyword in ["title", "description", "markdownDescription", "errorMessage"] { + assert!(constraint[keyword].is_string(), "{keyword} missing: {constraint}"); + } + } } #[test] - fn schema_rejects_args_without_name_arg() { + fn schema_validates_input_argument_rules() { let schema = schema_for!(SecretMethod).to_value(); let validator = jsonschema::validator_for(&schema).unwrap(); @@ -362,5 +445,6 @@ mod test { assert!(!validator.is_valid(&json!({ "executable": "secret", "args": ["get"] }))); assert!(!validator.is_valid(&json!({ "executable": "secret", "args": [{ "nameArg": "--name" }, { "nameArg": "--secret" }] }))); assert!(!validator.is_valid(&json!({ "executable": "secret", "args": [{ "nameArg": "--name" }, { "vaultArg": "--vault" }, { "vaultArg": "--store" }] }))); + assert!(!validator.is_valid(&json!({ "executable": "secret", "args": [{ "nameArg": "--name", "vaultArg": "--vault" }] }))); } } From aa9f44380155aa18deee8e07c6dc3a4bcb6bc04d Mon Sep 17 00:00:00 2001 From: "G.Reijn" <26114636+Gijsreyn@users.noreply.github.com> Date: Mon, 12 Oct 2026 23:53:42 +0200 Subject: [PATCH 4/4] Resolve remarks Mikey --- dsc/tests/dsc_extension_secret.tests.ps1 | 3 +- lib/dsc-lib/locales/en-us.toml | 3 +- lib/dsc-lib/locales/schemas.extension.yaml | 53 ++ lib/dsc-lib/src/dscerror.rs | 3 + lib/dsc-lib/src/extensions/secret.rs | 517 ++++++++--------- .../tests/integration/extensions/mod.rs | 7 + .../tests/integration/extensions/secret.rs | 539 ++++++++++++++++++ lib/dsc-lib/tests/integration/main.rs | 1 + .../tests/integration/schemas/schema_for.rs | 1 + 9 files changed, 869 insertions(+), 258 deletions(-) create mode 100644 lib/dsc-lib/tests/integration/extensions/mod.rs create mode 100644 lib/dsc-lib/tests/integration/extensions/secret.rs diff --git a/dsc/tests/dsc_extension_secret.tests.ps1 b/dsc/tests/dsc_extension_secret.tests.ps1 index 2d22e8205..d3aba6633 100644 --- a/dsc/tests/dsc_extension_secret.tests.ps1 +++ b/dsc/tests/dsc_extension_secret.tests.ps1 @@ -179,6 +179,7 @@ Describe 'Tests for the secret() function and extensions' { } It 'Deprecated extension shows message' { + $oldRestrictedPath = $env:DSC_RESTRICTED_PATH try { $dscHome = Split-Path (Get-Command dsc).Source -Parent $env:DSC_RESTRICTED_PATH = (Join-Path -Path $dscHome -ChildPath 'deprecated') + [System.IO.Path]::PathSeparator + $dscHome @@ -197,7 +198,7 @@ Describe 'Tests for the secret() function and extensions' { $LASTEXITCODE | Should -Be 4 (Get-Content -Raw -Path "$TestDrive/error.log") | Should -Match "Extension 'Test/ExtensionDeprecated' is deprecated: This extension is deprecated" -Because (Get-Content -Raw -Path "$TestDrive/error.log") } finally { - $env:DSC_RESTRICTED_PATH = $null + $env:DSC_RESTRICTED_PATH = $oldRestrictedPath } } diff --git a/lib/dsc-lib/locales/en-us.toml b/lib/dsc-lib/locales/en-us.toml index b8256b92c..9f6859d6d 100644 --- a/lib/dsc-lib/locales/en-us.toml +++ b/lib/dsc-lib/locales/en-us.toml @@ -304,7 +304,8 @@ extensionManifestSchemaTitle = "Extension manifest schema URI" extensionManifestSchemaDescription = "Defines the JSON Schema the extension manifest adheres to." [extensions.secret] -missingNameArg = "The 'secret' command doesn't define the secret name input argument, so DSC can't pass the name of the secret to retrieve to the extension. Define exactly one argument in 'secret.args' as an object with the 'nameArg' property" +invalidArgs = "The arguments for the 'secret' command are invalid: %{err}" +missingNameArg ="The 'secret' command doesn't define the secret name input argument, so DSC can't pass the name of the secret to retrieve to the extension. Define exactly one argument in 'secret.args' as an object with the 'nameArg' property" multipleNameArgs = "The 'secret' command defines the secret name input argument %{count} times. Define exactly one argument in 'secret.args' as an object with the 'nameArg' property" multipleVaultArgs = "The 'secret' command defines the vault input argument %{count} times. Define at most one argument in 'secret.args' as an object with the 'vaultArg' property" invalidArg = "Argument %{position} of the 'secret' command isn't valid. Each argument must be a string, an object with only the 'nameArg' property, or an object with only the 'vaultArg' property. Invalid argument: %{arg}" diff --git a/lib/dsc-lib/locales/schemas.extension.yaml b/lib/dsc-lib/locales/schemas.extension.yaml index f6c4d16bc..fc9873286 100644 --- a/lib/dsc-lib/locales/schemas.extension.yaml +++ b/lib/dsc-lib/locales/schemas.extension.yaml @@ -3,7 +3,60 @@ schemas: extension: manifest: secret: + title: + en-us: Secret command + description: + en-us: >- + Defines how to call the extension to retrieve a secret from a vault. + markdownDescription: + en-us: |- + Defines how to call the extension to retrieve a secret from a vault at runtime. When + this property is defined, the extension has the `secret` capability and DSC can invoke + the extension for the [`secret()`][01] configuration function. + + When DSC invokes the `secret` command, it expects the extension to write the secret + value to stdout as a single line. If the extension writes more than one line to stdout, + DSC raises an error. If the extension writes nothing to stdout, DSC treats the secret as + not found for that extension. + + For more information, see the [secret property reference][02]. + + [01]: https://learn.microsoft.com/powershell/dsc/reference/schemas/config/functions/secret + [02]: https://learn.microsoft.com/powershell/dsc/reference/schemas/extension/manifest/secret + executable: + title: + en-us: Executable + description: + en-us: >- + The name of the command to run to retrieve a secret. + markdownDescription: + en-us: |- + Defines the name of the command to run to retrieve a secret. The value must be the + name of a command discoverable in the system's `PATH` environment variable or the + full path to the command. A file extension is only required when the command isn't + recognizable by the operating system as an executable. args: + title: + en-us: Arguments + description: + en-us: >- + The arguments to pass to the command to retrieve a secret. The arguments must define + the secret name input argument exactly once and may define the vault input argument + at most once. + markdownDescription: + en-us: |- + Defines the list of arguments to pass to the command to retrieve a secret. Each item + can be a string representing a static argument, an object with the `nameArg` property, + or an object with the `vaultArg` property. + + The arguments must define exactly one object with the `nameArg` property. DSC passes + the value of `nameArg` followed by the name of the secret to retrieve. Without this + argument, DSC can't tell the extension which secret to retrieve, so DSC doesn't load + the extension manifest. + + The arguments may define at most one object with the `vaultArg` property. DSC passes + the value of `vaultArg` followed by the name of the vault when the `secret()` function + specifies a vault. When the function doesn't specify a vault, DSC omits this argument. constraints: nameArg: title: diff --git a/lib/dsc-lib/src/dscerror.rs b/lib/dsc-lib/src/dscerror.rs index a36cd5c4e..70d75140f 100644 --- a/lib/dsc-lib/src/dscerror.rs +++ b/lib/dsc-lib/src/dscerror.rs @@ -155,6 +155,9 @@ pub enum DscError { #[error("{t}: {0}", t = t!("dscerror.securityContext"))] SecurityContext(String), + #[error(transparent)] + SecretArgs(#[from] crate::extensions::secret::SecretArgsError), + #[error(transparent)] SemVer(#[from] crate::types::SemanticVersionError), diff --git a/lib/dsc-lib/src/extensions/secret.rs b/lib/dsc-lib/src/extensions/secret.rs index d0af8620a..43c5a2fb0 100644 --- a/lib/dsc-lib/src/extensions/secret.rs +++ b/lib/dsc-lib/src/extensions/secret.rs @@ -17,14 +17,17 @@ use crate::{ schemas::dsc_repo::{DscRepoSchema, schema_i18n} }; +use miette::Diagnostic; use rust_i18n::t; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use serde_json::Value; +use std::fmt::{Display, Formatter}; use std::ops::Deref; +use thiserror::Error; use tracing::{debug, warn}; -#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] +#[derive(Debug, Clone, PartialEq, Eq, Hash, Deserialize, Serialize, JsonSchema)] #[serde(untagged, deny_unknown_fields)] #[schemars(inline)] pub enum SecretArgKind { @@ -47,15 +50,92 @@ pub enum SecretArgKind { }, } +// Display the argument definition as it appears in a manifest, like `"get"` or +// `{"nameArg":"--name"}`, so messages show the definition the author wrote. +impl Display for SecretArgKind { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + let json = serde_json::to_string(self).map_err(|_| std::fmt::Error)?; + f.write_str(&json) + } +} + +/// Defines the errors that can occur when creating a [`SecretArgs`] instance. +#[derive(Debug, Clone, PartialEq, Error, Diagnostic)] +pub enum SecretArgsError { + /// Indicates that the arguments for the `secret` command are invalid for one or more reasons. + /// + /// The `errors` field lists every problem found in the arguments, like an argument definition + /// that isn't valid or a missing secret name input argument, so the manifest author can review + /// all of them at once instead of discovering them one at a time. + #[error("{t}", t = t!( + "extensions.secret.invalidArgs", + err = errors.iter().map(ToString::to_string).collect::>().join("; ") + ))] + InvalidArgs { + /// The specific problems found in the arguments. + #[related] + errors: Vec, + }, + + /// Indicates that an argument definition isn't a string, an object with only the `nameArg` + /// property, or an object with only the `vaultArg` property. + /// + /// Validation raises this error in the `errors` field of the main [`InvalidArgs`] error. + /// + /// [`InvalidArgs`]: Self::InvalidArgs + #[error("{t}", t = t!("extensions.secret.invalidArg", position = position, arg = value))] + InvalidArg { + /// The one-based position of the invalid argument in the `args` array. + position: usize, + /// The JSON value of the invalid argument. + value: Value, + }, + + /// Indicates that the arguments don't define the secret name input argument. + /// + /// Validation raises this error in the `errors` field of the main [`InvalidArgs`] error. + /// + /// [`InvalidArgs`]: Self::InvalidArgs + #[error("{t}", t = t!("extensions.secret.missingNameArg"))] + MissingNameArg, + + /// Indicates that the arguments define the secret name input argument more than once. + /// + /// Validation raises this error in the `errors` field of the main [`InvalidArgs`] error. + /// + /// [`InvalidArgs`]: Self::InvalidArgs + #[error("{t}", t = t!("extensions.secret.multipleNameArgs", count = count))] + MultipleNameArgs { + /// The number of secret name input arguments the arguments define. + count: usize, + }, + + /// Indicates that the arguments define the vault input argument more than once. + /// + /// Validation raises this error in the `errors` field of the main [`InvalidArgs`] error. + /// + /// [`InvalidArgs`]: Self::InvalidArgs + #[error("{t}", t = t!("extensions.secret.multipleVaultArgs", count = count))] + MultipleVaultArgs { + /// The number of vault input arguments the arguments define. + count: usize, + }, +} + /// Defines the arguments to pass to the `secret` command of an extension. /// /// The arguments must define the secret name input argument exactly once and may define the vault /// input argument at most once. Without the secret name input argument, DSC can't tell the /// extension which secret to retrieve, so the extension can't function as a secret provider. /// -/// Deserializing arguments that break these rules fails, so a deserialized instance is always -/// valid and DSC doesn't need to validate the arguments again before invoking the extension. -#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] +/// Every constructor validates these rules, and deserializing arguments that break them fails, so +/// an instance is always valid and DSC doesn't need to validate the arguments again before +/// invoking the extension. The type implements [`AsRef`], [`Deref`], and [`IntoIterator`] to +/// enable ergonomically using an instance as the underlying vector, but not [`DerefMut`], because +/// mutating the arguments could break the rules. +/// +/// [`DerefMut`]: std::ops::DerefMut +#[derive(Debug, Clone, PartialEq, Eq, Hash, Deserialize, Serialize, JsonSchema)] #[serde(try_from = "Vec")] #[schemars(inline, !try_from)] pub struct SecretArgs(Vec); @@ -69,11 +149,19 @@ impl SecretArgs { /// /// # Errors /// - /// Returns [`DscError::InvalidManifest`] when the arguments don't define the secret name input - /// argument, define it more than once, or define the vault input argument more than once. - pub fn new(args: Vec) -> Result { - Self::validate(&args)?; - Ok(Self(args)) + /// Returns [`SecretArgsError::InvalidArgs`] when the arguments break any of the rules. The + /// error lists every rule the arguments break: + /// + /// * [`SecretArgsError::MissingNameArg`] when the arguments don't define the secret name input + /// argument. + /// * [`SecretArgsError::MultipleNameArgs`] when the arguments define the secret name input + /// argument more than once. + /// * [`SecretArgsError::MultipleVaultArgs`] when the arguments define the vault input argument + /// more than once. + pub fn from_args(args: Vec) -> Result { + let mut errors = Vec::new(); + Self::validate(&args, &mut errors); + Self::finish(args, errors) } /// Creates the arguments for the `secret` command from the JSON values of a manifest. @@ -87,43 +175,97 @@ impl SecretArgs { /// /// # Errors /// - /// Returns [`DscError::InvalidManifest`] when a value isn't a valid argument definition or when - /// the arguments break the rules described for [`SecretArgs::new`]. - pub fn from_values(values: Vec) -> Result { + /// Returns [`SecretArgsError::InvalidArgs`] when any value isn't a valid argument definition + /// or when the arguments break the rules described for [`SecretArgs::from_args`]. The error + /// lists a [`SecretArgsError::InvalidArg`] for every invalid value followed by every rule the + /// valid argument definitions break. + pub fn from_values(values: Vec) -> Result { + let mut errors = Vec::new(); let mut args = Vec::with_capacity(values.len()); - for (index, value) in values.iter().enumerate() { - match SecretArgKind::deserialize(value) { + for (index, value) in values.into_iter().enumerate() { + match SecretArgKind::deserialize(&value) { Ok(arg) => args.push(arg), - Err(_) => { - return Err(DscError::InvalidManifest(t!("extensions.secret.invalidArg", position = index + 1, arg = value).to_string())); - } + Err(_) => errors.push(SecretArgsError::InvalidArg { position: index + 1, value }), + } + } + Self::validate(&args, &mut errors); + Self::finish(args, errors) + } + + /// Resolves the argument definitions into the arguments to pass to the `secret` command. + /// + /// Static string arguments are passed as-is. The secret name input argument is followed by + /// `name`. The vault input argument is followed by the vault name when `vault` is defined and + /// omitted entirely otherwise. + /// + /// # Arguments + /// + /// * `name` - The name of the secret to retrieve. + /// * `vault` - The name of the vault to retrieve the secret from, if any. + #[must_use] + pub fn to_command_args(&self, name: &str, vault: Option<&str>) -> Vec { + let mut command_args = Vec::::with_capacity(self.0.len() + 2); + for arg in &self.0 { + match arg { + SecretArgKind::String(s) => { + command_args.push(s.clone()); + }, + SecretArgKind::Name { name_arg } => { + command_args.push(name_arg.clone()); + command_args.push(name.to_string()); + }, + SecretArgKind::Vault { vault_arg } => { + if let Some(value) = vault { + command_args.push(vault_arg.clone()); + command_args.push(value.to_string()); + } + }, } } - Self::new(args) + + command_args } - /// Validates that the arguments define the secret name input argument exactly once and the - /// vault input argument at most once. - fn validate(args: &[SecretArgKind]) -> Result<(), DscError> { + /// Collects every rule the argument definitions break into `errors`. + /// + /// The arguments must define the secret name input argument exactly once and the vault input + /// argument at most once. + /// + /// # Arguments + /// + /// * `args` - The argument definitions to validate. + /// * `errors` - The vector to add the detected rule violations to. + fn validate(args: &[SecretArgKind], errors: &mut Vec) { let name_arg_count = args.iter().filter(|arg| matches!(arg, SecretArgKind::Name { .. })).count(); let vault_arg_count = args.iter().filter(|arg| matches!(arg, SecretArgKind::Vault { .. })).count(); - if name_arg_count == 0 { - return Err(DscError::InvalidManifest(t!("extensions.secret.missingNameArg").to_string())); - } - if name_arg_count > 1 { - return Err(DscError::InvalidManifest(t!("extensions.secret.multipleNameArgs", count = name_arg_count).to_string())); + match name_arg_count { + 0 => errors.push(SecretArgsError::MissingNameArg), + 1 => {}, + count => errors.push(SecretArgsError::MultipleNameArgs { count }), } if vault_arg_count > 1 { - return Err(DscError::InvalidManifest(t!("extensions.secret.multipleVaultArgs", count = vault_arg_count).to_string())); + errors.push(SecretArgsError::MultipleVaultArgs { count: vault_arg_count }); } + } - Ok(()) + /// Returns the arguments when no errors were collected and the collected errors otherwise. + /// + /// # Arguments + /// + /// * `args` - The validated argument definitions. + /// * `errors` - The rule violations collected while validating the arguments. + fn finish(args: Vec, errors: Vec) -> Result { + if errors.is_empty() { + Ok(Self(args)) + } else { + Err(SecretArgsError::InvalidArgs { errors }) + } } } impl TryFrom> for SecretArgs { - type Error = DscError; + type Error = SecretArgsError; fn try_from(values: Vec) -> Result { Self::from_values(values) @@ -131,21 +273,77 @@ impl TryFrom> for SecretArgs { } impl TryFrom> for SecretArgs { - type Error = DscError; + type Error = SecretArgsError; fn try_from(args: Vec) -> Result { - Self::new(args) + Self::from_args(args) } } +// Treat a reference to `SecretArgs` as a reference to the underlying `Vec` +impl AsRef> for SecretArgs { + fn as_ref(&self) -> &Vec { + &self.0 + } +} + +// Enable using non-mutating `Vec` methods directly on `SecretArgs` impl Deref for SecretArgs { - type Target = [SecretArgKind]; + type Target = Vec; fn deref(&self) -> &Self::Target { &self.0 } } +// Enable iterating over the underlying `Vec` to use in ergonomic `for` loops +impl IntoIterator for SecretArgs { + type Item = SecretArgKind; + type IntoIter = std::vec::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.0.into_iter() + } +} + +// Enable iterating over references to the argument definitions without consuming the instance +impl<'a> IntoIterator for &'a SecretArgs { + type Item = &'a SecretArgKind; + type IntoIter = std::slice::Iter<'a, SecretArgKind>; + + fn into_iter(self) -> Self::IntoIter { + self.0.iter() + } +} + +// Enable comparing the newtype with the underlying `Vec` in either direction +impl PartialEq> for SecretArgs { + fn eq(&self, other: &Vec) -> bool { + &self.0 == other + } +} + +impl PartialEq for Vec { + fn eq(&self, other: &SecretArgs) -> bool { + self == &other.0 + } +} + +// Enable infallibly converting the newtype into the underlying `Vec` +impl From for Vec { + fn from(args: SecretArgs) -> Self { + args.0 + } +} + +// Display the arguments as they appear in a manifest, like `["get",{"nameArg":"--name"}]` +impl Display for SecretArgs { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + let json = serde_json::to_string(self).map_err(|_| std::fmt::Error)?; + f.write_str(&json) + } +} + #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema, DscRepoSchema)] #[schemars( transform = SecretMethod::transform_export_schema_uris, @@ -154,30 +352,40 @@ impl Deref for SecretArgs { #[dsc_repo_schema(base_name = "manifest.secret", folder_path = "extension")] pub struct SecretMethod { /// The command to run to retrieve a secret. + #[schemars(extend( + "title" = schema_i18n!("executable.title"), + "description" = schema_i18n!("executable.description"), + "markdownDescription" = schema_i18n!("executable.markdownDescription"), + ))] pub executable: String, /// The arguments to pass to the command to retrieve a secret. The arguments must define the /// secret name input argument exactly once and may define the vault input argument at most /// once. - #[schemars(extend("allOf" = [ - { - "title": schema_i18n!("args.constraints.nameArg.title"), - "description": schema_i18n!("args.constraints.nameArg.description"), - "markdownDescription": schema_i18n!("args.constraints.nameArg.markdownDescription"), - "errorMessage": schema_i18n!("args.constraints.nameArg.errorMessage"), - "contains": { "type": "object", "required": ["nameArg"] }, - "minContains": 1, - "maxContains": 1, - }, - { - "title": schema_i18n!("args.constraints.vaultArg.title"), - "description": schema_i18n!("args.constraints.vaultArg.description"), - "markdownDescription": schema_i18n!("args.constraints.vaultArg.markdownDescription"), - "errorMessage": schema_i18n!("args.constraints.vaultArg.errorMessage"), - "contains": { "type": "object", "required": ["vaultArg"] }, - "minContains": 0, - "maxContains": 1, - }, - ]))] + #[schemars(extend( + "title" = schema_i18n!("args.title"), + "description" = schema_i18n!("args.description"), + "markdownDescription" = schema_i18n!("args.markdownDescription"), + "allOf" = [ + { + "title": schema_i18n!("args.constraints.nameArg.title"), + "description": schema_i18n!("args.constraints.nameArg.description"), + "markdownDescription": schema_i18n!("args.constraints.nameArg.markdownDescription"), + "errorMessage": schema_i18n!("args.constraints.nameArg.errorMessage"), + "contains": { "type": "object", "required": ["nameArg"] }, + "minContains": 1, + "maxContains": 1, + }, + { + "title": schema_i18n!("args.constraints.vaultArg.title"), + "description": schema_i18n!("args.constraints.vaultArg.description"), + "markdownDescription": schema_i18n!("args.constraints.vaultArg.markdownDescription"), + "errorMessage": schema_i18n!("args.constraints.vaultArg.errorMessage"), + "contains": { "type": "object", "required": ["vaultArg"] }, + "minContains": 0, + "maxContains": 1, + }, + ], + ))] pub args: SecretArgs, } @@ -208,7 +416,7 @@ impl DscExtension { let Some(secret) = extension.secret else { return Err(DscError::UnsupportedCapability(self.type_name.to_string(), Capability::Secret.to_string())); }; - let args = process_secret_args(&secret.args, name, vault); + let args = secret.args.to_command_args(name, vault); if let Some(deprecation_message) = extension.deprecation_message.as_ref() { warn!("{}", t!("extensions.dscextension.deprecationMessage", extension = self.type_name, message = deprecation_message)); } @@ -245,206 +453,3 @@ impl DscExtension { } } } - -fn process_secret_args(args: &[SecretArgKind], name: &str, vault: Option<&str>) -> Vec { - let mut processed_args = Vec::::new(); - for arg in args { - match arg { - SecretArgKind::String(s) => { - processed_args.push(s.clone()); - }, - SecretArgKind::Name { name_arg } => { - processed_args.push(name_arg.clone()); - processed_args.push(name.to_string()); - }, - SecretArgKind::Vault { vault_arg } => { - if let Some(value) = vault { - processed_args.push(vault_arg.clone()); - processed_args.push(value.to_string()); - } - }, - } - } - - processed_args -} - -#[cfg(test)] -mod test { - use super::{SecretArgKind, SecretArgs, SecretMethod, process_secret_args}; - use crate::dscerror::DscError; - use crate::extensions::extension_manifest::ExtensionManifest; - use crate::schemas::dsc_repo::DscRepoSchema; - use schemars::schema_for; - use serde_json::{Value, json}; - - fn name_arg() -> SecretArgKind { - SecretArgKind::Name { name_arg: "--name".to_string() } - } - - fn vault_arg() -> SecretArgKind { - SecretArgKind::Vault { vault_arg: "--vault".to_string() } - } - - fn manifest_with_secret(secret: Value) -> Value { - json!({ - "$schema": ExtensionManifest::default_schema_id_uri(), - "type": "Test/Secret", - "version": "0.1.0", - "secret": secret - }) - } - - #[test] - fn new_accepts_single_name_arg() { - let args = SecretArgs::new(vec![SecretArgKind::String("get".to_string()), name_arg()]).unwrap(); - assert_eq!(args.len(), 2); - } - - #[test] - fn new_accepts_single_name_and_vault_arg() { - assert!(SecretArgs::new(vec![vault_arg(), name_arg()]).is_ok()); - } - - #[test] - fn new_rejects_missing_name_arg() { - let err = SecretArgs::new(vec![SecretArgKind::String("get".to_string()), vault_arg()]).unwrap_err(); - assert!(matches!(err, DscError::InvalidManifest(_))); - assert!(err.to_string().contains("doesn't define the secret name input argument"), "{err}"); - } - - #[test] - fn new_rejects_empty_args() { - assert!(SecretArgs::new(vec![]).is_err()); - } - - #[test] - fn new_rejects_multiple_name_args() { - let err = SecretArgs::new(vec![name_arg(), name_arg()]).unwrap_err(); - assert!(matches!(err, DscError::InvalidManifest(_))); - assert!(err.to_string().contains("defines the secret name input argument 2 times"), "{err}"); - } - - #[test] - fn new_rejects_multiple_vault_args() { - let err = SecretArgs::new(vec![name_arg(), vault_arg(), vault_arg()]).unwrap_err(); - assert!(matches!(err, DscError::InvalidManifest(_))); - assert!(err.to_string().contains("defines the vault input argument 2 times"), "{err}"); - } - - #[test] - fn from_values_converts_each_argument_kind() { - let args = SecretArgs::from_values(vec![json!("get"), json!({ "nameArg": "--name" }), json!({ "vaultArg": "--vault" })]).unwrap(); - assert_eq!(*args, [SecretArgKind::String("get".to_string()), name_arg(), vault_arg()]); - } - - #[test] - fn from_values_rejects_object_with_name_and_vault_arg() { - let err = SecretArgs::from_values(vec![json!("get"), json!({ "nameArg": "--name", "vaultArg": "--vault" })]).unwrap_err(); - assert!(matches!(err, DscError::InvalidManifest(_))); - assert!(err.to_string().contains("Argument 2 of the 'secret' command isn't valid"), "{err}"); - assert!(err.to_string().contains(r#"{"nameArg":"--name","vaultArg":"--vault"}"#), "{err}"); - } - - #[test] - fn from_values_rejects_object_with_unknown_property() { - let err = SecretArgs::from_values(vec![json!({ "nameArg": "--name", "other": true })]).unwrap_err(); - assert!(err.to_string().contains("Argument 1 of the 'secret' command isn't valid"), "{err}"); - } - - #[test] - fn from_values_rejects_non_string_scalar() { - let err = SecretArgs::from_values(vec![json!(5), json!({ "nameArg": "--name" })]).unwrap_err(); - assert!(err.to_string().contains("Argument 1 of the 'secret' command isn't valid"), "{err}"); - } - - #[test] - fn process_secret_args_preserves_order_and_inserts_values() { - let args = vec![SecretArgKind::String("get".to_string()), vault_arg(), name_arg(), SecretArgKind::String("--quiet".to_string())]; - let processed = process_secret_args(&args, "apiToken", Some("services")); - assert_eq!(processed, vec!["get", "--vault", "services", "--name", "apiToken", "--quiet"]); - } - - #[test] - fn process_secret_args_omits_vault_arg_without_vault() { - let args = vec![SecretArgKind::String("get".to_string()), name_arg(), vault_arg()]; - let processed = process_secret_args(&args, "apiToken", None); - assert_eq!(processed, vec!["get", "--name", "apiToken"]); - } - - #[test] - fn manifest_without_secret_args_fails_to_deserialize() { - let manifest = manifest_with_secret(json!({ "executable": "secret" })); - let err = serde_json::from_value::(manifest).unwrap_err(); - assert!(err.to_string().contains("missing field `args`"), "{err}"); - } - - #[test] - fn manifest_without_name_arg_fails_to_deserialize() { - let manifest = manifest_with_secret(json!({ "executable": "secret", "args": ["get"] })); - let err = serde_json::from_value::(manifest).unwrap_err(); - assert!(err.to_string().contains("doesn't define the secret name input argument"), "{err}"); - } - - #[test] - fn manifest_with_multiple_name_args_fails_to_deserialize() { - let manifest = manifest_with_secret(json!({ "executable": "secret", "args": [{ "nameArg": "--name" }, { "nameArg": "--secret" }] })); - let err = serde_json::from_value::(manifest).unwrap_err(); - assert!(err.to_string().contains("defines the secret name input argument 2 times"), "{err}"); - } - - #[test] - fn manifest_with_ambiguous_arg_fails_to_deserialize() { - let manifest = manifest_with_secret(json!({ "executable": "secret", "args": [{ "nameArg": "--name", "vaultArg": "--vault" }] })); - let err = serde_json::from_value::(manifest).unwrap_err(); - assert!(err.to_string().contains("Argument 1 of the 'secret' command isn't valid"), "{err}"); - } - - #[test] - fn manifest_with_secret_args_round_trips() { - let args = json!(["get", { "nameArg": "--name" }, { "vaultArg": "--vault" }]); - let manifest = manifest_with_secret(json!({ "executable": "secret", "args": args })); - let manifest = serde_json::from_value::(manifest).unwrap(); - let secret = manifest.secret.unwrap(); - assert_eq!(*secret.args, [SecretArgKind::String("get".to_string()), name_arg(), vault_arg()]); - assert_eq!(serde_json::to_value(&secret).unwrap()["args"], args); - } - - #[test] - fn schema_requires_args_and_constrains_input_arguments() { - let schema = schema_for!(SecretMethod).to_value(); - let required = schema["required"].as_array().unwrap(); - assert!(required.contains(&Value::String("executable".to_string())), "{schema}"); - assert!(required.contains(&Value::String("args".to_string())), "{schema}"); - - let args = &schema["properties"]["args"]; - assert_eq!(args["type"], json!("array"), "{schema}"); - let constraints = args["allOf"].as_array().unwrap(); - assert_eq!(constraints.len(), 2, "{schema}"); - assert_eq!(constraints[0]["contains"]["required"], json!(["nameArg"])); - assert_eq!(constraints[0]["minContains"], json!(1)); - assert_eq!(constraints[0]["maxContains"], json!(1)); - assert_eq!(constraints[1]["contains"]["required"], json!(["vaultArg"])); - assert_eq!(constraints[1]["minContains"], json!(0)); - assert_eq!(constraints[1]["maxContains"], json!(1)); - for constraint in constraints { - for keyword in ["title", "description", "markdownDescription", "errorMessage"] { - assert!(constraint[keyword].is_string(), "{keyword} missing: {constraint}"); - } - } - } - - #[test] - fn schema_validates_input_argument_rules() { - let schema = schema_for!(SecretMethod).to_value(); - let validator = jsonschema::validator_for(&schema).unwrap(); - - assert!(validator.is_valid(&json!({ "executable": "secret", "args": ["get", { "nameArg": "--name" }] }))); - assert!(validator.is_valid(&json!({ "executable": "secret", "args": [{ "vaultArg": "--vault" }, { "nameArg": "--name" }] }))); - assert!(!validator.is_valid(&json!({ "executable": "secret" }))); - assert!(!validator.is_valid(&json!({ "executable": "secret", "args": ["get"] }))); - assert!(!validator.is_valid(&json!({ "executable": "secret", "args": [{ "nameArg": "--name" }, { "nameArg": "--secret" }] }))); - assert!(!validator.is_valid(&json!({ "executable": "secret", "args": [{ "nameArg": "--name" }, { "vaultArg": "--vault" }, { "vaultArg": "--store" }] }))); - assert!(!validator.is_valid(&json!({ "executable": "secret", "args": [{ "nameArg": "--name", "vaultArg": "--vault" }] }))); - } -} diff --git a/lib/dsc-lib/tests/integration/extensions/mod.rs b/lib/dsc-lib/tests/integration/extensions/mod.rs new file mode 100644 index 000000000..c42c6e11f --- /dev/null +++ b/lib/dsc-lib/tests/integration/extensions/mod.rs @@ -0,0 +1,7 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Defines integration tests for the public types in the `extensions` module of [`dsc-lib`]. + +#[cfg(test)] +mod secret; diff --git a/lib/dsc-lib/tests/integration/extensions/secret.rs b/lib/dsc-lib/tests/integration/extensions/secret.rs new file mode 100644 index 000000000..3d870ec56 --- /dev/null +++ b/lib/dsc-lib/tests/integration/extensions/secret.rs @@ -0,0 +1,539 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +use dsc_lib::extensions::secret::SecretArgKind; + +/// Returns a static string argument definition. +fn string_arg(value: &str) -> SecretArgKind { + SecretArgKind::String(value.to_string()) +} + +/// Returns the secret name input argument definition `--name`. +fn name_arg() -> SecretArgKind { + SecretArgKind::Name { name_arg: "--name".to_string() } +} + +/// Returns the vault input argument definition `--vault`. +fn vault_arg() -> SecretArgKind { + SecretArgKind::Vault { vault_arg: "--vault".to_string() } +} + +#[cfg(test)] +mod methods { + #[cfg(test)] + mod from_args { + use dsc_lib::extensions::secret::{SecretArgKind, SecretArgs, SecretArgsError}; + use test_case::test_case; + + use super::super::{name_arg, string_arg, vault_arg}; + + #[test_case(vec![name_arg()]; "only name arg")] + #[test_case(vec![string_arg("get"), name_arg()]; "string arg and name arg")] + #[test_case(vec![vault_arg(), name_arg()]; "vault arg and name arg")] + #[test_case(vec![string_arg("get"), name_arg(), vault_arg(), string_arg("--quiet")]; "every argument kind")] + fn valid(args: Vec) { + let secret_args = SecretArgs::from_args(args.clone()).expect("arguments should be valid"); + + assert_eq!(secret_args, args); + } + + #[test_case(vec![] => + SecretArgsError::InvalidArgs { + errors: vec![ + SecretArgsError::MissingNameArg, + ], + }; "empty args" + )] + #[test_case(vec![string_arg("get"), vault_arg()] => + SecretArgsError::InvalidArgs { + errors: vec![ + SecretArgsError::MissingNameArg, + ], + }; "missing name arg" + )] + #[test_case(vec![name_arg(), name_arg()] => + SecretArgsError::InvalidArgs { + errors: vec![ + SecretArgsError::MultipleNameArgs { count: 2 }, + ], + }; "multiple name args" + )] + #[test_case(vec![name_arg(), vault_arg(), vault_arg()] => + SecretArgsError::InvalidArgs { + errors: vec![ + SecretArgsError::MultipleVaultArgs { count: 2 }, + ], + }; "multiple vault args" + )] + #[test_case(vec![name_arg(), name_arg(), vault_arg(), vault_arg()] => + SecretArgsError::InvalidArgs { + errors: vec![ + SecretArgsError::MultipleNameArgs { count: 2 }, + SecretArgsError::MultipleVaultArgs { count: 2 }, + ], + }; "reports all errors in the args" + )] + fn invalid(args: Vec) -> SecretArgsError { + SecretArgs::from_args(args).unwrap_err() + } + } + + #[cfg(test)] + mod from_values { + use dsc_lib::extensions::secret::{SecretArgs, SecretArgsError}; + use serde_json::{json, Value}; + use test_case::test_case; + + use super::super::{name_arg, string_arg, vault_arg}; + + #[test] + fn converts_each_argument_kind() { + let values = vec![json!("get"), json!({ "nameArg": "--name" }), json!({ "vaultArg": "--vault" })]; + + let args = SecretArgs::from_values(values).expect("values should be valid"); + + assert_eq!(args, vec![string_arg("get"), name_arg(), vault_arg()]); + } + + #[test_case(vec![json!("get"), json!({ "nameArg": "--name", "vaultArg": "--vault" })] => + SecretArgsError::InvalidArgs { + errors: vec![ + SecretArgsError::InvalidArg { + position: 2, + value: json!({ "nameArg": "--name", "vaultArg": "--vault" }), + }, + SecretArgsError::MissingNameArg, + ], + }; "object with name arg and vault arg" + )] + #[test_case(vec![json!({ "nameArg": "--name", "other": true })] => + SecretArgsError::InvalidArgs { + errors: vec![ + SecretArgsError::InvalidArg { + position: 1, + value: json!({ "nameArg": "--name", "other": true }), + }, + SecretArgsError::MissingNameArg, + ], + }; "object with unknown property" + )] + #[test_case(vec![json!(5), json!({ "nameArg": "--name" })] => + SecretArgsError::InvalidArgs { + errors: vec![ + SecretArgsError::InvalidArg { + position: 1, + value: json!(5), + }, + ], + }; "non-string scalar" + )] + #[test_case(vec![json!(5), json!(true), json!({ "nameArg": "--name" }), json!({ "nameArg": "--secret" })] => + SecretArgsError::InvalidArgs { + errors: vec![ + SecretArgsError::InvalidArg { + position: 1, + value: json!(5), + }, + SecretArgsError::InvalidArg { + position: 2, + value: json!(true), + }, + SecretArgsError::MultipleNameArgs { count: 2 }, + ], + }; "reports all errors in the values" + )] + fn invalid(values: Vec) -> SecretArgsError { + SecretArgs::from_values(values).unwrap_err() + } + } + + #[cfg(test)] + mod to_command_args { + use dsc_lib::extensions::secret::{SecretArgKind, SecretArgs}; + use test_case::test_case; + + use super::super::{name_arg, string_arg, vault_arg}; + + #[test_case( + vec![string_arg("get"), vault_arg(), name_arg(), string_arg("--quiet")], "apiToken", Some("services") => + vec!["get", "--vault", "services", "--name", "apiToken", "--quiet"]; + "preserves order and inserts the secret and vault names" + )] + #[test_case( + vec![string_arg("get"), name_arg(), vault_arg()], "apiToken", None => + vec!["get", "--name", "apiToken"]; + "omits the vault arg without a vault" + )] + #[test_case( + vec![name_arg()], "apiToken", Some("services") => + vec!["--name", "apiToken"]; + "ignores the vault without a vault arg" + )] + fn resolves(args: Vec, name: &str, vault: Option<&str>) -> Vec { + SecretArgs::from_args(args) + .expect("arguments should be valid") + .to_command_args(name, vault) + } + } +} + +#[cfg(test)] +mod schema { + use std::sync::LazyLock; + + use dsc_lib::extensions::secret::SecretMethod; + use dsc_lib_jsonschema::schema_utility_extensions::SchemaUtilityExtensions; + use jsonschema::Validator; + use regex::Regex; + use schemars::{schema_for, Schema}; + use serde_json::{json, Value}; + use test_case::test_case; + + static SCHEMA: LazyLock = LazyLock::new(|| schema_for!(SecretMethod)); + static VALIDATOR: LazyLock = + LazyLock::new(|| Validator::new((&*SCHEMA).as_value()).unwrap()); + static KEYWORD_PATTERN: LazyLock = + LazyLock::new(|| Regex::new(r"^\w+(\.\w+)+$").expect("pattern is valid")); + + #[test_case("title")] + #[test_case("description")] + #[test_case("markdownDescription")] + fn has_documentation_keyword(keyword: &str) { + let schema = &*SCHEMA; + let value = schema + .get_keyword_as_str(keyword) + .expect(format!("expected keyword '{keyword}' to be defined").as_str()); + + assert!( + !(&*KEYWORD_PATTERN).is_match(value), + "Expected keyword '{keyword}' to be defined in translation, but was set to i18n key '{value}'" + ); + } + + #[test_case("executable", "title")] + #[test_case("executable", "description")] + #[test_case("executable", "markdownDescription")] + #[test_case("args", "title")] + #[test_case("args", "description")] + #[test_case("args", "markdownDescription")] + fn property_has_documentation_keyword(property: &str, keyword: &str) { + let schema = (&*SCHEMA) + .get_property_subschema(property) + .expect(format!("expected property '{property}' to be defined").as_str()); + let value = schema + .get_keyword_as_str(keyword) + .expect(format!("expected keyword '{keyword}' to be defined for property '{property}'").as_str()); + + assert!( + !(&*KEYWORD_PATTERN).is_match(value), + "Expected keyword '{keyword}' for property '{property}' to be defined in translation, but was set to i18n key '{value}'" + ); + } + + #[test_case("executable")] + #[test_case("args")] + fn requires_property(property: &str) { + let required = (&*SCHEMA).get_required_property_names(); + + assert!(required.contains(&property.to_string()), "expected '{property}' to be required, but required properties are {required:?}"); + } + + #[test] + fn constrains_input_arguments() { + let args = (&*SCHEMA) + .get_property_subschema("args") + .expect("expected property 'args' to be defined"); + assert_eq!(args.get_keyword_as_str("type"), Some("array")); + + let constraints = args + .get_keyword_as_array("allOf") + .expect("expected 'allOf' to be defined for property 'args'"); + assert_eq!(constraints.len(), 2, "expected one constraint per input argument rule"); + + assert_eq!(constraints[0]["contains"]["required"], json!(["nameArg"])); + assert_eq!(constraints[0]["minContains"], json!(1)); + assert_eq!(constraints[0]["maxContains"], json!(1)); + assert_eq!(constraints[1]["contains"]["required"], json!(["vaultArg"])); + assert_eq!(constraints[1]["minContains"], json!(0)); + assert_eq!(constraints[1]["maxContains"], json!(1)); + + for constraint in constraints { + for keyword in ["title", "description", "markdownDescription", "errorMessage"] { + let value = constraint[keyword].as_str() + .expect(format!("expected keyword '{keyword}' to be defined for constraint {constraint}").as_str()); + assert!( + !(&*KEYWORD_PATTERN).is_match(value), + "Expected keyword '{keyword}' to be defined in translation, but was set to i18n key '{value}'" + ); + } + } + } + + #[test_case(&json!({ "executable": "secret", "args": ["get", { "nameArg": "--name" }] }) => true; "args with one name arg is valid")] + #[test_case(&json!({ "executable": "secret", "args": [{ "vaultArg": "--vault" }, { "nameArg": "--name" }] }) => true; "args with one name arg and one vault arg is valid")] + #[test_case(&json!({ "executable": "secret", "args": [{ "nameArg": "--name" }] }) => true; "args with only a name arg is valid")] + #[test_case(&json!({ "executable": "secret" }) => false; "missing args is invalid")] + #[test_case(&json!({ "args": [{ "nameArg": "--name" }] }) => false; "missing executable is invalid")] + #[test_case(&json!({ "executable": "secret", "args": [] }) => false; "empty args is invalid")] + #[test_case(&json!({ "executable": "secret", "args": ["get"] }) => false; "args without name arg is invalid")] + #[test_case(&json!({ "executable": "secret", "args": [{ "nameArg": "--name" }, { "nameArg": "--secret" }] }) => false; "args with two name args is invalid")] + #[test_case(&json!({ "executable": "secret", "args": [{ "nameArg": "--name" }, { "vaultArg": "--vault" }, { "vaultArg": "--store" }] }) => false; "args with two vault args is invalid")] + #[test_case(&json!({ "executable": "secret", "args": [{ "nameArg": "--name", "vaultArg": "--vault" }] }) => false; "arg with name arg and vault arg in one object is invalid")] + #[test_case(&json!({ "executable": "secret", "args": [{ "nameArg": "--name", "other": true }] }) => false; "arg with unknown property is invalid")] + #[test_case(&json!({ "executable": "secret", "args": [5, { "nameArg": "--name" }] }) => false; "arg that is not a string or object is invalid")] + fn validation(input_json: &Value) -> bool { + (&*VALIDATOR).validate(input_json).is_ok() + } +} + +#[cfg(test)] +mod serde { + use dsc_lib::extensions::extension_manifest::ExtensionManifest; + use dsc_lib::extensions::secret::SecretArgs; + use dsc_lib::schemas::dsc_repo::DscRepoSchema; + use serde_json::{json, Value}; + use test_case::test_case; + + use super::{name_arg, string_arg, vault_arg}; + + fn manifest_with_secret(secret: Value) -> Value { + json!({ + "$schema": ExtensionManifest::default_schema_id_uri(), + "type": "Test/Secret", + "version": "0.1.0", + "secret": secret + }) + } + + #[test] + fn serializing() { + let args = SecretArgs::from_args(vec![string_arg("get"), name_arg(), vault_arg()]) + .expect("arguments should be valid"); + + let actual = serde_json::to_value(&args).expect("serialization should never fail"); + + pretty_assertions::assert_eq!(actual, json!(["get", { "nameArg": "--name" }, { "vaultArg": "--vault" }])); + } + + #[test_case(json!(["get", { "nameArg": "--name" }]) => matches Ok(_); "array with one name arg is valid")] + #[test_case(json!([{ "vaultArg": "--vault" }, { "nameArg": "--name" }]) => matches Ok(_); "array with one name arg and one vault arg is valid")] + #[test_case(json!([]) => matches Err(_); "empty array raises error")] + #[test_case(json!(["get"]) => matches Err(_); "array without name arg raises error")] + #[test_case(json!([{ "nameArg": "--name" }, { "nameArg": "--secret" }]) => matches Err(_); "array with two name args raises error")] + #[test_case(json!([{ "nameArg": "--name" }, { "vaultArg": "--vault" }, { "vaultArg": "--store" }]) => matches Err(_); "array with two vault args raises error")] + #[test_case(json!([{ "nameArg": "--name", "vaultArg": "--vault" }]) => matches Err(_); "array with name arg and vault arg in one object raises error")] + #[test_case(json!([{ "nameArg": "--name", "other": true }]) => matches Err(_); "array with unknown property in object raises error")] + #[test_case(json!([5, { "nameArg": "--name" }]) => matches Err(_); "array with non-string scalar raises error")] + #[test_case(json!("--name") => matches Err(_); "string value raises error")] + #[test_case(json!({ "nameArg": "--name" }) => matches Err(_); "object value raises error")] + #[test_case(Value::Null => matches Err(_); "null value raises error")] + fn deserializing(value: Value) -> Result { + serde_json::from_value::(value) + } + + #[test_case( + json!({ "executable": "secret" }), + "missing field `args`"; + "manifest without args" + )] + #[test_case( + json!({ "executable": "secret", "args": ["get"] }), + "doesn't define the secret name input argument"; + "manifest without name arg" + )] + #[test_case( + json!({ "executable": "secret", "args": [{ "nameArg": "--name" }, { "nameArg": "--secret" }] }), + "defines the secret name input argument 2 times"; + "manifest with multiple name args" + )] + #[test_case( + json!({ "executable": "secret", "args": [{ "nameArg": "--name" }, { "vaultArg": "--vault" }, { "vaultArg": "--store" }] }), + "defines the vault input argument 2 times"; + "manifest with multiple vault args" + )] + #[test_case( + json!({ "executable": "secret", "args": [{ "nameArg": "--name", "vaultArg": "--vault" }] }), + "Argument 1 of the 'secret' command isn't valid"; + "manifest with ambiguous arg" + )] + fn deserializing_invalid_manifest_explains_problem(secret: Value, expected_message: &str) { + let err = serde_json::from_value::(manifest_with_secret(secret)).unwrap_err(); + + assert!( + err.to_string().contains(expected_message), + "expected error to contain '{expected_message}', but was: {err}" + ); + } + + #[test] + fn deserializing_invalid_manifest_explains_every_problem() { + let secret = json!({ "executable": "secret", "args": [5, { "nameArg": "--name" }, { "nameArg": "--secret" }] }); + + let err = serde_json::from_value::(manifest_with_secret(secret)).unwrap_err(); + + let message = err.to_string(); + assert!(message.contains("Argument 1 of the 'secret' command isn't valid"), "{message}"); + assert!(message.contains("defines the secret name input argument 2 times"), "{message}"); + } + + #[test] + fn round_trips_through_manifest() { + let args = json!(["get", { "nameArg": "--name" }, { "vaultArg": "--vault" }]); + let manifest = manifest_with_secret(json!({ "executable": "secret", "args": args })); + + let manifest = serde_json::from_value::(manifest).expect("manifest should be valid"); + let secret = manifest.secret.expect("secret should be defined"); + + assert_eq!(secret.args, vec![string_arg("get"), name_arg(), vault_arg()]); + pretty_assertions::assert_eq!(serde_json::to_value(&secret).unwrap()["args"], args); + } +} + +#[cfg(test)] +mod traits { + use dsc_lib::extensions::secret::SecretArgs; + + use super::{name_arg, string_arg, vault_arg}; + + /// Returns valid arguments that define every argument kind. + fn args() -> SecretArgs { + SecretArgs::from_args(vec![string_arg("get"), name_arg(), vault_arg()]) + .expect("arguments should be valid") + } + + #[cfg(test)] + mod as_ref { + use dsc_lib::extensions::secret::SecretArgKind; + + use super::args; + + #[test] + fn vec_secret_arg_kind() { + let args = args(); + + let inner: &Vec = args.as_ref(); + + assert_eq!(inner.len(), 3); + } + } + + #[cfg(test)] + mod deref { + use super::{args, super::string_arg}; + + #[test] + fn vec_secret_arg_kind() { + let args = args(); + + assert_eq!(args.len(), 3); + assert_eq!(args.first(), Some(&string_arg("get"))); + assert!(args.iter().any(|arg| arg == &string_arg("get"))); + } + } + + #[cfg(test)] + mod display { + use dsc_lib::extensions::secret::SecretArgKind; + use test_case::test_case; + + use super::{args, super::{name_arg, string_arg, vault_arg}}; + + #[test_case(string_arg("get") => r#""get""#; "string arg displays as JSON string")] + #[test_case(name_arg() => r#"{"nameArg":"--name"}"#; "name arg displays as JSON object")] + #[test_case(vault_arg() => r#"{"vaultArg":"--vault"}"#; "vault arg displays as JSON object")] + fn secret_arg_kind(arg: SecretArgKind) -> String { + arg.to_string() + } + + #[test] + fn secret_args() { + assert_eq!(args().to_string(), r#"["get",{"nameArg":"--name"},{"vaultArg":"--vault"}]"#); + } + } + + #[cfg(test)] + mod from { + use dsc_lib::extensions::secret::SecretArgKind; + + use super::{args, super::{name_arg, string_arg, vault_arg}}; + + #[test] + fn vec_secret_arg_kind() { + let inner: Vec = args().into(); + + assert_eq!(inner, vec![string_arg("get"), name_arg(), vault_arg()]); + } + } + + #[cfg(test)] + mod into_iterator { + use dsc_lib::extensions::secret::SecretArgKind; + + use super::{args, super::{name_arg, string_arg, vault_arg}}; + + #[test] + fn secret_arg_kind() { + let mut collected = Vec::new(); + for arg in args() { + collected.push(arg); + } + + assert_eq!(collected, vec![string_arg("get"), name_arg(), vault_arg()]); + } + + #[test] + fn secret_arg_kind_by_reference() { + let args = args(); + + let mut collected = Vec::<&SecretArgKind>::new(); + for arg in &args { + collected.push(arg); + } + + assert_eq!(collected, vec![&string_arg("get"), &name_arg(), &vault_arg()]); + } + } + + #[cfg(test)] + mod partial_eq { + use super::{args, super::{name_arg, string_arg, vault_arg}}; + + #[test] + fn secret_args_with_vec_secret_arg_kind() { + assert_eq!(args(), vec![string_arg("get"), name_arg(), vault_arg()]); + assert_ne!(args(), vec![name_arg()]); + } + + #[test] + fn vec_secret_arg_kind_with_secret_args() { + assert_eq!(vec![string_arg("get"), name_arg(), vault_arg()], args()); + assert_ne!(vec![name_arg()], args()); + } + } + + #[cfg(test)] + mod try_from { + use dsc_lib::extensions::secret::{SecretArgKind, SecretArgs, SecretArgsError}; + use serde_json::{json, Value}; + + use super::super::{name_arg, string_arg}; + + #[test] + fn vec_secret_arg_kind() { + let valid: Result = vec![string_arg("get"), name_arg()].try_into(); + let invalid: Result = Vec::::new().try_into(); + + assert!(valid.is_ok()); + assert!(invalid.is_err()); + } + + #[test] + fn vec_value() { + let valid: Result = vec![json!("get"), json!({ "nameArg": "--name" })].try_into(); + let invalid: Result = Vec::::new().try_into(); + + assert!(valid.is_ok()); + assert!(invalid.is_err()); + } + } +} diff --git a/lib/dsc-lib/tests/integration/main.rs b/lib/dsc-lib/tests/integration/main.rs index 3a005a0ac..32c32fb0e 100644 --- a/lib/dsc-lib/tests/integration/main.rs +++ b/lib/dsc-lib/tests/integration/main.rs @@ -14,5 +14,6 @@ //! Rust would generate numerous binaries to execute our tests. #[cfg(test)] mod command_resource; +#[cfg(test)] mod extensions; #[cfg(test)] mod schemas; #[cfg(test)] mod types; diff --git a/lib/dsc-lib/tests/integration/schemas/schema_for.rs b/lib/dsc-lib/tests/integration/schemas/schema_for.rs index ebf81f927..8fbdef253 100644 --- a/lib/dsc-lib/tests/integration/schemas/schema_for.rs +++ b/lib/dsc-lib/tests/integration/schemas/schema_for.rs @@ -156,6 +156,7 @@ macro_rules! test_schema_for { #[allow(unused_must_use)] #[cfg(test)] mod secret { test_schema_for!(dsc_lib::extensions::secret::SecretArgKind); + test_schema_for!(dsc_lib::extensions::secret::SecretArgs); test_schema_for!(dsc_lib::extensions::secret::SecretMethod); } }