(GH-1729) Make nonfunctional secret extension manifests invalid

[Gijs Reijn (Gijsreyn)]

PR Summary

This change:

• Requires extension manifests that define the secret command to also define its args, with exactly one secret name input argument (nameArg) and at most one vault input argument (vaultArg).
• Validates these rules when DSC loads an extension manifest. A manifest that breaks them is skipped like any other invalid manifest, with an informational message that names the extension, the manifest path, and the specific problem.
• Adds the same rules to the generated JSON schema for the secret command, so editors flag the problem while the manifest is being written. The VS Code form of the schema carries a specific error message for each rule.
• Validates the arguments again right before DSC invokes the extension, as a safeguard for extensions that weren't loaded through discovery.
• Teaches the no-op subcommand of dsctest to accept and ignore arguments, and fixes the deprecated test extension, which was itself a nonfunctional secret extension.
• Adds unit tests for the validation, the argument processing, and the generated schema, and Pester tests that cover each invalid manifest shape and a valid one.
• Updates the reference documentation and the changelog.
• Fixes Make it impossible to accidentally define a nonfunctional secret extension #1729

PR Context

Prior to this change, the args field for the secret command was optional, and nothing checked whether it contained the secret name input argument. An extension could define a secret command that DSC was happy to invoke but could never tell which secret to retrieve: the name was silently dropped from the argument list, and whatever the extension wrote to stdout was treated as the secret. Nothing in the output of dsc extension list or in the trace messages told the author that something was wrong.

The hand-written YAML schema source under schemas/src already described the intended contract: args required, exactly one name argument, at most one vault argument. The published schemas are now generated from the Rust types, though, and those carried none of these constraints. The repository's own deprecated test extension shows how easy it was to get wrong: its secret command passed only no-op to dsctest and nothing else.

The working group chose the first option from the issue, making the contract strict and treating a missing name argument as an error, rather than the second option of appending the secret name as a trailing argument.

The check lives in the loader that turns an extension manifest into a discovered extension, next to the existing checks for the other commands. When it fails, the manifest is rejected the same way as a manifest that doesn't parse, so the behavior is consistent for extension authors: the extension doesn't appear in the list, and running dsc --trace-level info extension list shows why.

Remark on compatibility

This is a breaking change for extension manifests that omit args or the name argument. Those manifests loaded before, but the resulting extension couldn't retrieve a specific secret, so the change only excludes definitions that never worked as intended. The Azure CLI example and the test extensions in this repository were already compliant.

Remark on the message level

The skipped-manifest message is logged at the info level, matching every other invalid manifest. If this case should be louder, a warning from the loader is a one-line change.

Remark on the published schemas

The checked-in schemas under schemas/v3 aren't regenerated in this change. They don't yet contain the secret or import command schemas at all, so regenerating them appears to be a separate release-time step rather than something each change is expected to do.

[Copilot]

🟡 Changes recommended

Validation currently permits an ambiguous argument object, can discard entire manifest lists, and has diagnostic, localization, and test-isolation issues.

5 open findings

• Skip invalid manifest entries without discarding valid resources · New
• Preserve extension type for missing args diagnostics · New
• Reject unknown fields in untagged argument variants · New
• Restore previous DSC_RESTRICTED_PATH value · New
• Localize hard-coded schema titles and validation messages · New

What changed in this PR

Strengthens secret-extension manifests by enforcing required name/vault argument constraints during schema validation, discovery, and invocation.

Changes:

• Adds secret argument validation and schema constraints.
• Updates test tooling, manifests, and automated coverage.
• Documents the stricter contract and compatibility impact.

File	Description
tools/​dsctest/​src/​main.rs	Ignores no-op arguments.
tools/​dsctest/​src/​args.rs	Allows trailing no-op arguments.
tools/​dsctest/​deprecated/​deprecated.dsc.manifests.json	Fixes deprecated secret manifest.
lib/​dsc-lib/​src/​extensions/​secret.rs	Implements validation, invocation safeguards, schema rules, and tests.
lib/​dsc-lib/​src/​discovery/​command_discovery.rs	Rejects invalid secret extensions during discovery.
lib/​dsc-lib/​locales/​en-us.toml	Adds validation diagnostics.
dsc/​tests/​dsc_extension_secret.tests.ps1	Tests valid and invalid manifests.
docs/​reference/​schemas/​extension/​manifest/​secret.md	Documents strict secret arguments.
docs/​reference/​schemas/​extension/​manifest/​root.md	Updates root manifest guidance.
CHANGELOG.md	Records the breaking validation change.

🧠 Review effort: Balanced

---

Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

[Mikey Lombardi (He/Him) (michaeltlombardi)]

Added a few notes on addressing the schema change.

Another option to consider is to define a newtype wrapper around Vec<SecretArgKind>, like (not tested, just quickly scribbled out):

#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)]
#[serde(try_from = "Vec<serde_json::Value>")]
#[schemars(inline, !try_from)]
pub struct SecretArgs(Vec<SecretArgKind>);

impl SecretArgs {
    pub fn from_values(values: Vec<serde_json::Value>) -> Result<Self, DscError> {
        Self::validate(&values)?;

        // Construction logic
    }
    fn validate(values: &Vec<serde_json::Value>) -> Result<(), DscError> {
        // Validation logic for an array of values
    }
}

impl TryFrom<Vec<serde_json::Value>> for SecretArgs {
    type Error = DscError;

    fn try_from(values: Vec<serde_json::Value>) -> Result<Self, Self::Error> {
        SecretArgs::from_values(values)
    }
}

Which lets us pull the validation into the deserialization process and provide meaningful messages. Generally, we want to raise an error when we can't deserialize data because it fails constraints - this way we can just use the deserialized data in the rest of the code without needing to validate it repeatedly.

[Mikey Lombardi (He/Him) (michaeltlombardi)]

Rather than naming this function new(), I think this should be from_args.