Reject malformed publisher DNs in cert generate (#845)

docs/cli-schema.json

@@ -284,7 +284,7 @@
               "recursive": false
             },
             "--publisher": {
-              "description": "Publisher distinguished name (DN) for the generated certificate (e.g., CN=MyCompany or OU=Team, O=Corp, C=US). If not specified, will be inferred from manifest. Bare names are auto-wrapped as CN=<name>.",
+              "description": "Publisher distinguished name (DN) for the generated certificate (e.g., CN=MyCompany or OU=Team, O=Corp, C=US). Components must be single-valued and comma-separated; multi-valued '+' RDNs, ';' separators, and backslashes are not supported. If not specified, will be inferred from manifest. Bare names are auto-wrapped as CN=<name>.",
               "hidden": false,
               "valueType": "System.String",
               "hasDefaultValue": false,
@@ -1968,7 +1968,7 @@
               "recursive": false
             },
             "--publisher-name": {
-              "description": "Publisher distinguished name (DN) (default: CN=<current user>). Accepts any valid X.500 DN; bare names are auto-wrapped as CN=<name>.",
+              "description": "Publisher distinguished name (DN) (default: CN=<current user>). Accepts an X.500 DN with single-valued, comma-separated components (multi-valued '+' RDNs, ';' separators, and backslashes are not supported); bare names are auto-wrapped as CN=<name>.",
               "hidden": false,
               "valueType": "System.String",
               "hasDefaultValue": false,

docs/usage.md

@@ -556,7 +556,7 @@ winapp manifest generate [directory] [options]
 **Options:**
 
 - `--package-name <name>` - Package name (default: folder name)
-- `--publisher-name <name>` - Publisher distinguished name (default: CN=\<current user\>). Accepts any valid X.500 DN; bare names are auto-wrapped as CN=\<name\>.
+- `--publisher-name <name>` - Publisher distinguished name (default: CN=\<current user\>). Accepts an X.500 DN with single-valued, comma-separated components (multi-valued `+` RDNs and backslashes are not supported); bare names are auto-wrapped as CN=\<name\>.
 - `--version <version>` - Version (default: "1.0.0.0")
 - `--description <text>` - Description (default: "My Application")
 - `--entrypoint <path>` - Entry point executable or script
@@ -1357,7 +1357,7 @@ winapp cert generate [options]
 **Options:**
 
 - `--manifest <Package.appxmanifest>` - Extract the certificate publisher from the manifest's `Identity/@Publisher`. Only the publisher is required, so a partially-complete manifest still works. If the manifest has no usable publisher, the command fails instead of substituting a default, so the certificate can never silently mismatch the manifest.
-- `--publisher <name>` - Publisher for the certificate. Accepts a full X.500 distinguished name (e.g., `CN=Contoso, O=Contoso Ltd, C=US`) or a bare name which is automatically wrapped as `CN=<name>`
+- `--publisher <name>` - Publisher for the certificate. Accepts a full X.500 distinguished name (e.g., `CN=Contoso, O=Contoso Ltd, C=US`) or a bare name which is automatically wrapped as `CN=<name>`. Components must be single-valued and comma-separated; multi-valued RDNs (`CN=Foo+OU=Bar`) and backslashes are not supported because the MSIX manifest publisher cannot represent them. A malformed distinguished name (e.g. `CN=` or `CN=A,,O=B`) is rejected with a non-zero exit and an error naming the problem, rather than producing a certificate that can never match the manifest publisher.
 - `--output <path>` - Output certificate file path (supports absolute and relative paths)
 - `--password <password>` - Certificate password (default: "password")
 - `--valid-days <valid-days>` - Number of days the certificate is valid (default: 365)

plugins/winapp/com.github.copilot/agents/winapp.agent.md

@@ -126,7 +126,7 @@ Need to know whether a Windows/WinRT API exists, or what a type/enum actually of
 
 2. **The key prerequisite is `appxmanifest.xml`, not `winapp.yaml`.** Most winapp commands (`package`, `create-debug-identity`, `sign`, `cert generate --manifest`) need an `appxmanifest.xml`. If one doesn't exist, guide the user to run `winapp init` or `winapp manifest generate`. A project does **not** need `winapp.yaml` to use winapp — `winapp.yaml` is only needed for SDK version management via `restore`/`update`. For SDK build tools, winapp resolves versions via a fallback chain: `winapp.yaml` → `.csproj` NuGet package references (e.g., `Microsoft.Windows.SDK.BuildTools`) → latest available version in the NuGet cache. This means any project with the right NuGet packages (common in .NET) can use winapp commands without ever running `init`, as long as it has an `appxmanifest.xml`.
 
-3. **Publisher must match between cert and manifest.** The `Publisher` field in `appxmanifest.xml` must exactly match the certificate subject distinguished name. Any valid X.500 DN is supported (e.g., `CN=YourName` or `OU=Team, O=Corp, C=US`). Use `winapp cert generate --manifest ./appxmanifest.xml` to auto-infer the correct publisher. If there's a mismatch, signing and installation will fail.
+3. **Publisher must match between cert and manifest.** The `Publisher` field in `appxmanifest.xml` must exactly match the certificate subject distinguished name. Most X.500 DNs are supported (e.g., `CN=YourName` or `OU=Team, O=Corp, C=US`), but components must be single-valued and comma-separated — multi-valued RDNs (`CN=Foo+OU=Bar`) and backslashes are rejected because the MSIX manifest publisher cannot represent them. Use `winapp cert generate --manifest ./appxmanifest.xml` to auto-infer the correct publisher. If there's a mismatch, signing and installation will fail.
 
 4. **`cert install` requires administrator elevation.** Always warn the user that `winapp cert install` must be run in an elevated (administrator) terminal. Without this, the certificate won't be trusted and MSIX installation will fail.
 
@@ -267,7 +267,7 @@ with `winapp target snapshot sandbox`; it does not create a VM. Consult
 **When to use:** When you need a development certificate to sign MSIX packages or executables.
 **Key options:**
 - `--manifest <path>` — auto-infer publisher from manifest (recommended)
-- `--publisher "CN=..."` — set publisher DN explicitly (any valid X.500 DN; bare names auto-wrapped as CN=\<name\>)
+- `--publisher "CN=..."` — set publisher DN explicitly (single-valued, comma-separated components; multi-valued `+` RDNs and backslashes unsupported; bare names auto-wrapped as CN=\<name\>)
 - `--output <path>` — output PFX path (default: `devcert.pfx`)
 - `--password <pwd>` — PFX password (default: `password`)
 - `--valid-days <n>` — certificate validity period (default: 365)

plugins/winapp/skills/winapp-signing/SKILL.md

@@ -17,7 +17,7 @@ Use this skill when:
 
 ## Key concepts
 
-**Publisher matching:** The publisher in your certificate must exactly match the `Publisher` attribute in `Package.appxmanifest`. Any valid X.500 distinguished name is supported (e.g., `CN=MyCompany` or `OU=Team, O=Corp, C=US`). Use `--manifest` when generating to auto-match.
+**Publisher matching:** The publisher in your certificate must exactly match the `Publisher` attribute in `Package.appxmanifest`. Most X.500 distinguished names are supported (e.g., `CN=MyCompany` or `OU=Team, O=Corp, C=US`), but components must be single-valued and comma-separated — multi-valued RDNs (`CN=Foo+OU=Bar`) and backslashes aren't supported because the MSIX manifest publisher cannot represent them. Use `--manifest` when generating to auto-match.
 
 **Dev vs. production certs:** `winapp cert generate` creates self-signed certificates for **local testing only**. For production distribution (Microsoft Store or enterprise), obtain a certificate from a trusted Certificate Authority.
 

src/winapp-CLI/WinApp.Cli.Tests/CertGenerateCommandTests.cs

@@ -119,6 +119,41 @@ await File.WriteAllTextAsync(manifestPath, """
         Assert.IsFalse(File.Exists(pfxPath), "No certificate should be created when the operation is canceled.");
     }
 
+    [TestMethod]
+    [DataRow("CN=", DisplayName = "Empty CN value")]
+    [DataRow("=Contoso", DisplayName = "Leading equals")]
+    [DataRow("CN=A,,O=B", DisplayName = "Unparseable DN")]
+    public async Task MalformedPublisher_NonJson_ReturnsErrorWithoutGenerating(string publisher)
+    {
+        var command = GetRequiredService<CertGenerateCommand>();
+        var pfxPath = Path.Join(_tempDirectory.FullName, "malformed-publisher.pfx");
+
+        var exitCode = await ParseAndInvokeWithCaptureAsync(
+            command, ["--publisher", publisher, "--output", pfxPath, "--password", "testpw"]);
+
+        Assert.AreEqual(1, exitCode, "A malformed --publisher must be rejected before generating a PFX.");
+        StringAssert.Contains(ConsoleStdErr.ToString(), "Publisher");
+        Assert.IsFalse(File.Exists(pfxPath), "No certificate should be created for a malformed publisher.");
+    }
+
+    [TestMethod]
+    [DataRow("", DisplayName = "Empty string")]
+    [DataRow("   ", DisplayName = "Whitespace only")]
+    public async Task ExplicitEmptyPublisher_NonJson_ReturnsErrorWithoutGenerating(string publisher)
+    {
+        // An explicitly supplied empty --publisher must fail loudly rather than silently fall back
+        // to the inferred/default publisher and generate a certificate for the wrong identity.
+        var command = GetRequiredService<CertGenerateCommand>();
+        var pfxPath = Path.Join(_tempDirectory.FullName, "empty-publisher.pfx");
+
+        var exitCode = await ParseAndInvokeWithCaptureAsync(
+            command, ["--publisher", publisher, "--output", pfxPath, "--password", "testpw"]);
+
+        Assert.AreEqual(1, exitCode, "An explicitly empty --publisher must be rejected.");
+        StringAssert.Contains(ConsoleStdErr.ToString(), "Publisher name cannot be empty");
+        Assert.IsFalse(File.Exists(pfxPath), "No certificate should be created for an empty publisher.");
+    }
+
     [TestMethod]
     public void OutputOption_AcceptsPlainFileName()
     {
@@ -502,6 +537,22 @@ public async Task EmptyPassword_Json_OutputsJsonError()
         Assert.IsFalse(File.Exists(pfxPath));
     }
 
+    [TestMethod]
+    public async Task MalformedPublisher_Json_OutputsJsonError()
+    {
+        var command = GetRequiredService<CertGenerateCommand>();
+        var pfxPath = Path.Join(_tempDirectory.FullName, "malformed-publisher-json.pfx");
+
+        var exitCode = await ParseAndInvokeWithCaptureAsync(
+            command, ["--publisher", "CN=A,,O=B", "--output", pfxPath, "--password", "testpw", "--json"]);
+
+        Assert.AreEqual(1, exitCode);
+        var root = JsonDocument.Parse(TestAnsiConsole.Output.Trim()).RootElement;
+        Assert.IsTrue(root.TryGetProperty("error", out var errorProp), "JSON error output should contain 'error' property");
+        StringAssert.Contains(errorProp.GetString(), "distinguished name");
+        Assert.IsFalse(File.Exists(pfxPath));
+    }
+
     [TestMethod]
     public async Task ExplicitManifest_MissingPublisher_Json_OutputsSingleJsonError()
     {

src/winapp-CLI/WinApp.Cli.Tests/PublisherDnHelperTests.cs

@@ -73,6 +73,123 @@ public void Normalize_RejectsEmptyInput(string input)
         Assert.ThrowsExactly<ArgumentException>(() => PublisherDnHelper.Normalize(input));
     }
 
+    [TestMethod]
+    [DataRow("CN=", "CN", DisplayName = "Empty CN value")]
+    [DataRow("CN=A, O=", "O", DisplayName = "Empty trailing component value")]
+    public void Normalize_RejectsEmptyValuedComponent(string input, string componentName)
+    {
+        var ex = Assert.ThrowsExactly<ArgumentException>(() => PublisherDnHelper.Normalize(input));
+        StringAssert.Contains(ex.Message, componentName, "The error must name the offending empty component.");
+    }
+
+    [TestMethod]
+    [DataRow("=Contoso", DisplayName = "Leading equals")]
+    [DataRow("CN=A,,O=B", DisplayName = "Empty RDN between components")]
+    [DataRow("OID.2.5.4.3=A,,O=B", DisplayName = "OID.-prefixed malformed DN")]
+    [DataRow("2.5.4.3=A,,O=B", DisplayName = "Bare-OID malformed DN")]
+    public void Normalize_RejectsMalformedDnAttempt(string input)
+    {
+        // Inputs that look like a DN (start with an attribute assignment, incl. an OID or "OID."
+        // prefix) but do not parse must be rejected rather than silently wrapped as a literal CN.
+        var ex = Assert.ThrowsExactly<ArgumentException>(() => PublisherDnHelper.Normalize(input));
+        StringAssert.Contains(ex.Message, "distinguished name");
+    }
+
+    [TestMethod]
+    [DataRow("CN=Contoso\\Bar", DisplayName = "Literal backslash in a DN value")]
+    [DataRow("CN=Contoso\\, Inc", DisplayName = "X.500 escaped comma")]
+    [DataRow("Contoso\\Bar", DisplayName = "Backslash in a bare name")]
+    public void Normalize_RejectsBackslash(string input)
+    {
+        // The MSIX manifest publisher type cannot represent a backslash, so any publisher containing
+        // one can never match Identity/@Publisher and is rejected up front.
+        var ex = Assert.ThrowsExactly<ArgumentException>(() => PublisherDnHelper.Normalize(input));
+        StringAssert.Contains(ex.Message, "backslash");
+    }
+
+    [TestMethod]
+    [DataRow("CN=A+O=", DisplayName = "Multi-valued RDN with an empty value")]
+    [DataRow("CN=Foo+OU=Bar", DisplayName = "Multi-valued RDN with values")]
+    public void Normalize_RejectsMultiValuedRdn(string input)
+    {
+        // A multi-valued RDN (a+b) is never used for a publisher and can hide an empty value, so it
+        // is rejected outright rather than accepted.
+        var ex = Assert.ThrowsExactly<ArgumentException>(() => PublisherDnHelper.Normalize(input));
+        StringAssert.Contains(ex.Message, "multi-valued");
+    }
+
+    [TestMethod]
+    [DataRow("R&D = Team", DisplayName = "Ampersand and spaces around equals")]
+    [DataRow("Contoso (a=b)", DisplayName = "Equals inside parentheses")]
+    public void Normalize_BareNameContainingEquals_WrapsAsCn(string input)
+    {
+        // A plain name whose first '=' is preceded by non-attribute text is a bare name, not a DN
+        // attempt, so it is wrapped as CN=<name> (the documented behavior) rather than rejected.
+        var result = PublisherDnHelper.Normalize(input);
+        Assert.IsTrue(PublisherDnHelper.IsDistinguishedName(result), $"Result should be a valid DN: {result}");
+        StringAssert.StartsWith(result, "CN=");
+    }
+
+    [TestMethod]
+    [DataRow("CN=A;O=B", DisplayName = "Semicolon-separated components")]
+    [DataRow("CN=A ; O=B", DisplayName = "Semicolon separator with spaces")]
+    public void Normalize_RejectsSemicolonSeparator(string input)
+    {
+        // .NET treats an unquoted ';' as an RDN separator, but the raw string keeps the literal ';',
+        // so the certificate (re-parsed to commas) and the manifest (literal ';') diverge. Reject it.
+        var ex = Assert.ThrowsExactly<ArgumentException>(() => PublisherDnHelper.Normalize(input));
+        StringAssert.Contains(ex.Message, "';'");
+    }
+
+    [TestMethod]
+    public void Normalize_SemicolonInsideQuotedValue_IsAccepted()
+    {
+        // A ';' inside a quoted value is data, not a separator, and round-trips as a single CN.
+        var result = PublisherDnHelper.Normalize("CN=\"A;B\"");
+        Assert.IsTrue(PublisherDnHelper.IsDistinguishedName(result), $"Result should be a valid DN: {result}");
+    }
+
+    [TestMethod]
+    [DataRow("CN=A,", DisplayName = "Trailing comma")]
+    [DataRow("CN=A+", DisplayName = "Trailing plus")]
+    [DataRow("CN=A, ", DisplayName = "Trailing comma with whitespace")]
+    [DataRow("CN=A,O=B,", DisplayName = "Trailing comma after multiple components")]
+    public void Normalize_RejectsTrailingSeparator(string input)
+    {
+        // A trailing ',' or '+' parses but is dropped when the certificate is encoded (CN=A, -> CN=A),
+        // while the manifest keeps it, so the two diverge. Reject it up front.
+        var ex = Assert.ThrowsExactly<ArgumentException>(() => PublisherDnHelper.Normalize(input));
+        StringAssert.Contains(ex.Message, "stray");
+    }
+
+    [TestMethod]
+    public void Normalize_CommaInsideQuotedValue_IsNotATrailingSeparator()
+    {
+        // The comma here is data inside the quoted value, and the value does not end with an unquoted
+        // separator, so it is accepted.
+        var result = PublisherDnHelper.Normalize("CN=\"A,\"");
+        Assert.IsTrue(PublisherDnHelper.IsDistinguishedName(result), $"Result should be a valid DN: {result}");
+    }
+
+    [TestMethod]
+    public void TryNormalize_ValidBareName_WrapsAsCn()
+    {
+        Assert.IsTrue(PublisherDnHelper.TryNormalize("Contoso", out var normalized, out var error));
+        Assert.AreEqual("CN=Contoso", normalized);
+        Assert.IsNull(error);
+    }
+
+    [TestMethod]
+    [DataRow("CN=", DisplayName = "Empty CN value")]
+    [DataRow("CN=A,,O=B", DisplayName = "Unparseable DN")]
+    [DataRow("CN=A+O=", DisplayName = "Multi-valued RDN with an empty value")]
+    public void TryNormalize_MalformedInput_ReturnsFalseWithMessage(string input)
+    {
+        Assert.IsFalse(PublisherDnHelper.TryNormalize(input, out var normalized, out var error));
+        Assert.IsNull(normalized);
+        Assert.IsFalse(string.IsNullOrWhiteSpace(error), "A user-facing error message must be provided.");
+    }
+
     [TestMethod]
     public void Normalize_PreservesInternalQuotes()
     {

src/winapp-CLI/WinApp.Cli/Commands/CertGenerateCommand.cs

@@ -30,7 +30,7 @@ static CertGenerateCommand()
     {
         PublisherOption = new Option<string>("--publisher")
         {
-            Description = "Publisher distinguished name (DN) for the generated certificate (e.g., CN=MyCompany or OU=Team, O=Corp, C=US). If not specified, will be inferred from manifest. Bare names are auto-wrapped as CN=<name>."
+            Description = "Publisher distinguished name (DN) for the generated certificate (e.g., CN=MyCompany or OU=Team, O=Corp, C=US). Components must be single-valued and comma-separated; multi-valued '+' RDNs, ';' separators, and backslashes are not supported. If not specified, will be inferred from manifest. Bare names are auto-wrapped as CN=<name>."
         };
         ManifestOption = new Option<FileInfo>("--manifest")
         {
@@ -154,6 +154,20 @@ public override async Task<int> InvokeAsync(ParseResult parseResult, Cancellatio
                     return 1;
                 }
             }
+            // Otherwise validate an explicit publisher up front so a malformed distinguished name — or
+            // an explicitly empty value — fails with a clear, actionable message instead of silently
+            // generating a certificate that can never match the manifest Identity/@Publisher.
+            // `publisher` is null only when --publisher was omitted (inference then applies); a
+            // supplied-but-empty value must still be rejected rather than fall through to a default.
+            else if (publisher is not null && !PublisherDnHelper.TryNormalize(publisher, out _, out var publisherError))
+            {
+                if (json)
+                {
+                    return JsonErrorOutput.Write(ansiConsole, publisherError);
+                }
+                logger.LogError("{UISymbol} {Message}", UiSymbols.Error, publisherError);
+                return 1;
+            }
 
             CertificateService.CertificateResult? certResult = null;
 

src/winapp-CLI/WinApp.Cli/Commands/ManifestGenerateCommand.cs

@@ -39,7 +39,7 @@ static ManifestGenerateCommand()
 
         PublisherNameOption = new Option<string>("--publisher-name")
         {
-            Description = "Publisher distinguished name (DN) (default: CN=<current user>). Accepts any valid X.500 DN; bare names are auto-wrapped as CN=<name>."
+            Description = "Publisher distinguished name (DN) (default: CN=<current user>). Accepts an X.500 DN with single-valued, comma-separated components (multi-valued '+' RDNs, ';' separators, and backslashes are not supported); bare names are auto-wrapped as CN=<name>."
         };
 
         VersionOption = new Option<string>("--version")