Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -1356,7 +1356,7 @@ winapp cert generate [options]

**Options:**

- `--manifest <Package.appxmanifest>` - Extract publisher information from Package.appxmanifest
- `--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>`
- `--output <path>` - Output certificate file path (supports absolute and relative paths)
- `--password <password>` - Certificate password (default: "password")
Expand Down
75 changes: 75 additions & 0 deletions src/winapp-CLI/WinApp.Cli.Tests/CertGenerateCommandTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,55 @@ public async Task EmptyPassword_NonJson_ReturnsError()
Assert.IsFalse(File.Exists(pfxPath), "No certificate should be created for an empty password.");
}

// ── explicit --manifest publisher resolution (issue #839) ───────────

[TestMethod]
public async Task ExplicitManifest_MissingPublisher_NonJson_ReturnsError()
{
// A named --manifest that has no usable Identity/@Publisher must fail with an actionable
// error instead of silently falling back to the OS user name (issue #839).
var command = GetRequiredService<CertGenerateCommand>();
var manifestPath = Path.Combine(_tempDirectory.FullName, "NoPublisher.appxmanifest");
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed
await File.WriteAllTextAsync(manifestPath, """
<?xml version="1.0" encoding="utf-8"?>
<Package xmlns="http://schemas.microsoft.com/appx/manifest/foundation/windows10">
<Identity Name="FlowHarnessApp" Version="1.0.0.0" />
</Package>
""");
var pfxPath = Path.Combine(_tempDirectory.FullName, "no-publisher.pfx");
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed

var exitCode = await ParseAndInvokeWithCaptureAsync(
command, ["--manifest", manifestPath, "--output", pfxPath, "--password", "testpw"]);

Assert.AreEqual(1, exitCode);
StringAssert.Contains(ConsoleStdErr.ToString(), "Could not extract the publisher from the manifest");
Assert.IsFalse(File.Exists(pfxPath), "No certificate should be created when the manifest has no publisher.");
}

[TestMethod]
public async Task ExplicitManifest_PublisherOnly_UsesManifestPublisher()
{
// A partially-complete manifest (valid Identity/@Publisher, no Applications element) is common
// mid-development. The certificate must use that publisher, not the OS user name (issue #839).
var command = GetRequiredService<CertGenerateCommand>();
var manifestPath = Path.Combine(_tempDirectory.FullName, "Partial.appxmanifest");
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed
await File.WriteAllTextAsync(manifestPath, """
<?xml version="1.0" encoding="utf-8"?>
<Package xmlns="http://schemas.microsoft.com/appx/manifest/foundation/windows10">
<Identity Name="FlowHarnessApp" Publisher="CN=FlowHarnessPublisher, O=Fabrikam Inc, C=US" Version="1.0.0.0" />
</Package>
""");
var pfxPath = Path.Combine(_tempDirectory.FullName, "partial.pfx");
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed

var exitCode = await ParseAndInvokeWithCaptureAsync(
command, ["--manifest", manifestPath, "--output", pfxPath, "--password", "testpw"]);

Assert.AreEqual(0, exitCode, "A manifest with a valid publisher but no Applications element must still succeed.");
Assert.IsTrue(File.Exists(pfxPath), "The certificate should be created.");
using var cert = System.Security.Cryptography.X509Certificates.X509CertificateLoader.LoadPkcs12FromFile(pfxPath, "testpw");
StringAssert.Contains(cert.Subject, "FlowHarnessPublisher");
}

[TestMethod]
public void OutputOption_AcceptsPlainFileName()
{
Expand Down Expand Up @@ -425,4 +474,30 @@ public async Task EmptyPassword_Json_OutputsJsonError()
StringAssert.Contains(errorProp.GetString(), "password cannot be empty");
Assert.IsFalse(File.Exists(pfxPath));
}

[TestMethod]
public async Task ExplicitManifest_MissingPublisher_Json_OutputsSingleJsonError()
{
// The VS Code extension passes --manifest with --json. A manifest that can't yield a publisher
// must produce exactly one structured error document — not empty stdout, and not two documents
// (issue #839). JsonDocument.Parse over the full output verifies it is a single JSON document.
var command = GetRequiredService<CertGenerateCommand>();
var manifestPath = Path.Combine(_tempDirectory.FullName, "NoPublisher.appxmanifest");
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed
await File.WriteAllTextAsync(manifestPath, """
<?xml version="1.0" encoding="utf-8"?>
<Package xmlns="http://schemas.microsoft.com/appx/manifest/foundation/windows10">
<Identity Name="FlowHarnessApp" Version="1.0.0.0" />
</Package>
""");
var pfxPath = Path.Combine(_tempDirectory.FullName, "no-publisher-json.pfx");
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed

var exitCode = await ParseAndInvokeWithCaptureAsync(
command, ["--manifest", manifestPath, "--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(), "Could not extract the publisher from the manifest");
Assert.IsFalse(File.Exists(pfxPath));
}
}
74 changes: 74 additions & 0 deletions src/winapp-CLI/WinApp.Cli.Tests/MsixServicePackagingTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,80 @@ public async Task ParseAppxManifestFromPathAsync_ValidManifest_ReturnsIdentity()
Assert.AreEqual("MyApp", result.ApplicationId);
}

// ---- ExtractPublisherFromPathAsync --------------------------------------------

[TestMethod]
public async Task ExtractPublisherFromPathAsync_NonexistentFile_ThrowsFileNotFoundException()
{
var missing = new FileInfo(Path.Combine(_tempDirectory.FullName, "does-not-exist.xml"));
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed

var ex = await Assert.ThrowsExactlyAsync<FileNotFoundException>(
() => MsixService.ExtractPublisherFromPathAsync(missing, TestContext.CancellationToken));

StringAssert.Contains(ex.Message, "AppX manifest not found");
}

[TestMethod]
public async Task ExtractPublisherFromPathAsync_ManifestWithoutApplications_ReturnsPublisher()
{
// A partially-complete manifest (valid Identity/@Publisher, no Applications/Name) must still
// yield the publisher — unlike ParseAppxManifestFromPathAsync, which requires an Application
// element (issue #839).
const string manifest =
"""
<?xml version="1.0" encoding="utf-8"?>
<Package xmlns="http://schemas.microsoft.com/appx/manifest/foundation/windows10">
<Identity Name="FlowHarnessApp" Publisher="CN=FlowHarnessPublisher, O=Fabrikam Inc, C=US" Version="1.0.0.0" />
</Package>
""";
var manifestPath = new FileInfo(Path.Combine(_tempDirectory.FullName, "Partial.appxmanifest"));
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed
await File.WriteAllTextAsync(manifestPath.FullName, manifest, TestContext.CancellationToken);

var publisher = await MsixService.ExtractPublisherFromPathAsync(manifestPath, TestContext.CancellationToken);

Assert.AreEqual("CN=FlowHarnessPublisher, O=Fabrikam Inc, C=US", publisher);
}

[TestMethod]
public async Task ExtractPublisherFromPathAsync_MissingPublisher_ThrowsInvalidOperationException()
{
const string manifest =
"""
<?xml version="1.0" encoding="utf-8"?>
<Package xmlns="http://schemas.microsoft.com/appx/manifest/foundation/windows10">
<Identity Name="FlowHarnessApp" Version="1.0.0.0" />
</Package>
""";
var manifestPath = new FileInfo(Path.Combine(_tempDirectory.FullName, "NoPublisher.appxmanifest"));
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed
await File.WriteAllTextAsync(manifestPath.FullName, manifest, TestContext.CancellationToken);

var ex = await Assert.ThrowsExactlyAsync<InvalidOperationException>(
() => MsixService.ExtractPublisherFromPathAsync(manifestPath, TestContext.CancellationToken));

StringAssert.Contains(ex.Message, "Publisher");
}

[TestMethod]
public async Task ExtractPublisherFromPathAsync_EmptyPublisher_ThrowsInvalidOperationException()
{
// An empty/whitespace Publisher would otherwise slip through and surface a generic downstream
// "Publisher name cannot be empty" error rather than the actionable manifest guidance.
const string manifest =
"""
<?xml version="1.0" encoding="utf-8"?>
<Package xmlns="http://schemas.microsoft.com/appx/manifest/foundation/windows10">
<Identity Name="FlowHarnessApp" Publisher=" " Version="1.0.0.0" />
</Package>
""";
var manifestPath = new FileInfo(Path.Combine(_tempDirectory.FullName, "EmptyPublisher.appxmanifest"));
await File.WriteAllTextAsync(manifestPath.FullName, manifest, TestContext.CancellationToken);

var ex = await Assert.ThrowsExactlyAsync<InvalidOperationException>(
() => MsixService.ExtractPublisherFromPathAsync(manifestPath, TestContext.CancellationToken));

StringAssert.Contains(ex.Message, "Publisher");
}

// ---- CopyManifestReferencedFiles ----------------------------------------------

[TestMethod]
Expand Down
24 changes: 24 additions & 0 deletions src/winapp-CLI/WinApp.Cli/Commands/CertGenerateCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,30 @@ public override async Task<int> InvokeAsync(ParseResult parseResult, Cancellatio
return 1;
}

// When --manifest is named explicitly (and no --publisher overrides it), the caller is
// asking the certificate to match that manifest's Identity/@Publisher. Resolve it up front
// so a manifest that can't yield a publisher fails with a clear error here — before the
// status task — instead of silently falling back to the system default and producing a
// certificate that can never match the manifest (issue #839).
if (manifestPath != null && string.IsNullOrWhiteSpace(publisher))
{
try
{
publisher = await MsixService.ExtractPublisherFromPathAsync(manifestPath, cancellationToken);
}
catch (Exception ex)
Comment thread
zateutsch marked this conversation as resolved.
Outdated
{
var message = $"Could not extract the publisher from the manifest '{manifestPath}': {ex.Message}. " +
"Fix the manifest's Identity Publisher attribute, or pass --publisher explicitly.";
if (json)
{
return JsonErrorOutput.Write(ansiConsole, message);
}
logger.LogError("{UISymbol} {Message}", UiSymbols.Error, message);
return 1;
}
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed
}

CertificateService.CertificateResult? certResult = null;

var returnCode = await statusService.ExecuteWithStatusAsync("Generating development certificate...", async (taskContext, ct) =>
Expand Down
33 changes: 33 additions & 0 deletions src/winapp-CLI/WinApp.Cli/Services/MsixService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,39 @@ public static async Task<MsixIdentityResult> ParseAppxManifestFromPathAsync(File
return ParseAppxManifestAsync(appxManifestContent);
}

/// <summary>
/// Extracts only the <c>Identity/@Publisher</c> value from an AppX manifest file.
/// Unlike <see cref="ParseAppxManifestFromPathAsync"/>, this does not require the manifest to
/// declare a package Name or an Application element, so it succeeds on the partially-complete
/// manifests that are common mid-development, as long as a publisher is present.
/// </summary>
/// <param name="appxManifestPath">Path to the appxmanifest.xml file</param>
/// <param name="cancellationToken">Cancellation token</param>
/// <returns>The publisher distinguished name from <c>Identity/@Publisher</c></returns>
/// <exception cref="FileNotFoundException">Thrown when the manifest file is not found</exception>
/// <exception cref="InvalidOperationException">Thrown when the manifest is invalid or missing the Identity Publisher attribute</exception>
public static async Task<string> ExtractPublisherFromPathAsync(FileInfo appxManifestPath, CancellationToken cancellationToken = default)
{
if (!appxManifestPath.Exists)
{
throw new FileNotFoundException($"AppX manifest not found at: {appxManifestPath}");
}

var appxManifestContent = await File.ReadAllTextAsync(appxManifestPath.FullName, Encoding.UTF8, cancellationToken);
var doc = AppxManifestDocument.Parse(appxManifestContent);

var identity = doc.GetIdentityElement()
?? throw new InvalidOperationException("No Identity element found in AppX manifest");

var publisher = identity.Attribute("Publisher")?.Value;
if (string.IsNullOrWhiteSpace(publisher))
{
throw new InvalidOperationException("AppX manifest Identity element is missing the required Publisher attribute");
}

return publisher;
}

/// <summary>
/// Parses an AppX manifest content and extracts the package identity information
/// </summary>
Expand Down
Loading