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/3] (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/3] (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/3] (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" }] }))); } }