Skip to content

Navigation Menu

Sign in
Sign up

Add ClickOnce publish-layout resolution - #1051

Open
dtivel wants to merge 2 commits into
main from
dtivel/clickonce-file-graph
Open

Add ClickOnce publish-layout resolution #1051
dtivel wants to merge 2 commits into
main from
dtivel/clickonce-file-graph

Conversation

@dtivel

@dtivel dtivel commented Aug 9, 2026
edited
Loading

Copy link
Copy Markdown
Collaborator

Part of #1049.

Summary

  • Add dormant ClickOnce publish-layout resolution through:
    • ClickOnceApplicationPublishLayoutResolver
    • ClickOnceDeploymentPublishLayoutResolver
    • ClickOncePayloadResolver
  • Represent the resolved layout with strongly typed models:
    • ResolvedClickOncePublishLayout
    • ResolvedClickOnceDeployment
    • ResolvedClickOnceApplication
    • ResolvedClickOncePayload
    • ResolvedClickOnceAdjacentExecutable
  • Report resolution failures through ClickOncePublishLayoutResolutionException while preserving ordered manifest diagnostics.
  • Read deployment and application manifests through the preservation-safe typed manifest-reader API for every resolution read.
  • Require a deployment manifest model to expose exactly one assembly reference, no file references, and an entry point that is the same non-prerequisite, non-optional reference. Inspect the preserved deployment XML to reject resource-fallback attributes that ManifestUtilities neither models nor preserves. Reject unsupported shapes before file resolution so ManifestUtilities or mage.exe cannot reorder or rewrite them before signing.
  • Reject rooted deployment-entry-point and application-payload target paths before resolution because they cannot be represented beneath a staging root. Preserve relative target paths, including parent traversal, for containment validation against the eventual staging root.
  • Resolve the deployment manifest's referenced application manifest, then resolve physical application payloads using the specified mapping and directory-fallback rules.
  • Preserve each payload's source and manifest reference while snapshotting its target path. ResolvedClickOncePayload validates that the source filename matches the target filename, with or without exactly one additional .deploy suffix, and derives IsFileExtensionMapped from that relationship.
  • Model adjacent executables separately with a constrained setup/launcher kind. ResolvedClickOnceAdjacentExecutable derives its target path from its source filename and validates the filename against its kind.
  • Discover only applicable adjacent setup.exe and unreferenced root-level Launcher.exe files; referenced launchers remain application payloads.
  • Use neutral "deployment manifest" and "application manifest" terminology in user-facing resolution errors.
  • Add coverage for deployment-reference validation, required and optional physical references, multiple versions, malformed and wrong-type manifests, mapping, fallback and diagnostic ordering, adjacent executables, VSTO layouts, target-path validation and preservation, and model invariants.

Intentional scope boundary

This PR resolves and models the publish layout only. It does not register the resolvers with dependency injection and does not change production signing behavior or the CLI surface. Diagnostic text may reference the planned --clickonce-signing-version 2 --no-update-clickonce-manifest option, but the resolvers remain dormant until signer integration adds that option.

Staging, relative-path containment validation, copying, temporary .deploy suffix removal and restoration, manifest metadata updates, signing-operation coordination, and signing remain deferred to the signer-integration work. Resolution rejects rooted target paths but does not rewrite or remap accepted relative paths. Staging must ensure those relative paths remain within the staging directory and fail when a referenced layout cannot be represented safely.

The deployment resolver establishes that Application.Source is the file initially identified by Deployment.ApplicationManifestReference.ResolvedPath. This is a resolution-time postcondition rather than a durable model invariant because staging must redirect that mutable ResolvedPath. The staging implementation must stage Application.Source, redirect the deployment entry point to that exact staged copy, verify that it is contained within the staging directory, and only then update deployment-manifest metadata. At the start of staging, it must capture the deployment entry point's target path. Immediately before deployment-manifest metadata update and serialization, it must fail unless the live entry point is still the resolved ApplicationManifestReference, remains the sole assembly reference with no file references, and its target path still ordinally equals the captured value. It must not reconcile or serialize mutated deployment structure.

For payloads, ResolvedClickOncePayload.TargetPath is an immutable resolution-time snapshot of Reference.TargetPath, not a live projection. Staging must use that snapshot to determine the intended layout, preserve the manifest-relative meaning rather than remapping it, and bind each mutable reference's ResolvedPath to the corresponding contained staged file before updating metadata. Before computing staging destinations or collision claims, and again immediately before updating application-manifest metadata and serializing the manifest, staging must verify with ordinal comparison that each mutable Reference.TargetPath still equals its resolved snapshot. A mismatch must fail staging rather than reconcile or emit an inconsistent manifest.

The deployment-directory payload fallback is a source-discovery mechanism. Signer integration must retain an explicit mapping from every staged file to the original ResolvedClickOncePayload.Source it was staged from and copy signed bytes back to that exact source path. It must not derive copy-back destinations from staging-relative paths or reuse a directory-mirroring copy-back seam that would create a new application-directory file for a deployment-directory fallback source. Integration coverage must verify that a fallback payload is returned to its original source and that no unintended target-relative file is created.

Discovery can also produce two resolved references to one physical file: a root-level setup.exe referenced by the application manifest is classified as both a ResolvedClickOncePayload and a ResolvedClickOnceAdjacentExecutable, with both Source values identifying the same file and potentially the same target path. Signer integration must apply the same source-keyed coordinated signing operation to both references so the file is staged, signed, and copied back exactly once rather than processed as two independent operations.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds dormant ClickOnce file-graph resolution for the version 2 signing pipeline.

Changes:

  • Adds graph models and application/deployment manifest resolvers.
  • Implements payload mapping, fallback resolution, diagnostics, and adjacent executable discovery.
  • Adds comprehensive tests and localized resources.
Show a summary per file
File Description
test/Sign.Core.Test/Tools/ClickOnce/ClickOnceFileGraphResolverTests.cs Tests resolution behavior and edge cases.
test/Sign.Core.Test/Tools/ClickOnce/ClickOnceFileGraphModelTests.cs Tests graph models and validation.
src/Sign.Core/Tools/ClickOnce/ClickOncePayloadFileResolver.cs Resolves referenced payload files.
src/Sign.Core/Tools/ClickOnce/ClickOnceManifestDiagnostic.cs Models manifest diagnostics.
src/Sign.Core/Tools/ClickOnce/ClickOnceFileGraphResolutionException.cs Defines resolution failures.
src/Sign.Core/Tools/ClickOnce/ClickOnceFileGraphEntryKind.cs Defines graph entry categories.
src/Sign.Core/Tools/ClickOnce/ClickOnceFileGraphEntry.cs Models individual graph entries.
src/Sign.Core/Tools/ClickOnce/ClickOnceFileGraph.cs Models resolved ClickOnce graphs.
src/Sign.Core/Tools/ClickOnce/ClickOnceDeployManifestFileGraphResolver.cs Resolves deployment-manifest graphs.
src/Sign.Core/Tools/ClickOnce/ClickOnceApplicationManifestFileGraphResolver.cs Resolves application-manifest graphs.
src/Sign.Core/Resources.resx Adds resolution messages.
src/Sign.Core/Resources.Designer.cs Exposes generated resource properties.
src/Sign.Core/xlf/Resources.zh-Hant.xlf Adds Traditional Chinese localization entries.
src/Sign.Core/xlf/Resources.zh-Hans.xlf Adds Simplified Chinese localization entries.
src/Sign.Core/xlf/Resources.tr.xlf Adds Turkish localization entries.
src/Sign.Core/xlf/Resources.ru.xlf Adds Russian localization entries.
src/Sign.Core/xlf/Resources.pt-BR.xlf Adds Brazilian Portuguese localization entries.
src/Sign.Core/xlf/Resources.pl.xlf Adds Polish localization entries.
src/Sign.Core/xlf/Resources.ko.xlf Adds Korean localization entries.
src/Sign.Core/xlf/Resources.ja.xlf Adds Japanese localization entries.
src/Sign.Core/xlf/Resources.it.xlf Adds Italian localization entries.
src/Sign.Core/xlf/Resources.fr.xlf Adds French localization entries.
src/Sign.Core/xlf/Resources.es.xlf Adds Spanish localization entries.
src/Sign.Core/xlf/Resources.de.xlf Adds German localization entries.
src/Sign.Core/xlf/Resources.cs.xlf Adds Czech localization entries.

Review details

Tip

Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Files not reviewed (1)
  • src/Sign.Core/Resources.Designer.cs: Generated file
  • Files reviewed: 24/25 changed files
  • Comments generated: 0
  • Review effort level: Balanced

dtivel force-pushed the dtivel/clickonce-file-graph branch from 0eebbb8 to 24496c7 Compare August 12, 2026 19:14
dtivel requested a balanced review from Copilot August 12, 2026 19:30

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Files not reviewed (1)
  • src/Sign.Core/Resources.Designer.cs: Generated file
  • Files reviewed: 24/25 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

dtivel force-pushed the dtivel/clickonce-file-graph branch from 24496c7 to d99a714 Compare August 12, 2026 22:43
Base automatically changed from dtivel/clickonce-manifest-foundation to main August 14, 2026 14:06
dtivel force-pushed the dtivel/clickonce-file-graph branch 3 times, most recently from 8e4dc41 to 57f5560 Compare August 15, 2026 19:20
Comment on lines +10 to +11
ClickOnceFileGraphEntry? deploymentManifest,
IDeployManifest? deployManifest,

@kartheekp-ms kartheekp-ms Aug 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we strengthen the file-graph model before merging this PR? I think follow-up PR #1058 demonstrates that the current shape requires consumers to reconstruct invariants that could instead be encoded here.

1. Deployment state can be half-present

ClickOnceFileGraph represents deployment input with two independently nullable properties:

ClickOnceFileGraphEntry? DeploymentManifest
IDeployManifest? DeployManifest

That produces four possible states:

DeploymentManifest DeployManifest Meaning
present present Valid deployment input
absent absent Valid explicit application input
present absent Invalid half-present deployment
absent present Invalid half-present deployment

The constructor currently accepts all four. For example, this compiles:

ClickOnceFileGraph graph = new(
 deploymentManifest: deploymentEntry,
 deployManifest: null,
 applicationManifest,
 applicationManifestModel,
 payloads,
 adjacentExecutables,
 diagnostics);

The inverse also compiles:

ClickOnceFileGraph graph = new(
 deploymentManifest: null,
 deployManifest,
 applicationManifest,
 applicationManifestModel,
 payloads,
 adjacentExecutables,
 diagnostics);

This is already visible in PR #1058's candidate selection:

bool isDeploymentInput = graph.DeploymentManifest is not null;
if (isDeploymentInput)
{
 candidates.Add(
 new StagingCandidate(
 graph.DeploymentManifest!,
 isUpdateInputOnly: false));
}

The stager must choose one nullable property as the authority and use !. Consequently:

  • DeploymentManifest != null and DeployManifest == null is silently treated as deployment input.
  • DeploymentManifest == null and DeployManifest != null is silently treated as application input.

The current resolvers construct valid pairs, but the result type itself does not preserve that invariant for future consumers.

2. Deployment-only adjacent files can exist without a deployment

The model also permits this:

ClickOnceFileGraph graph = new(
 deploymentManifest: null,
 deployManifest: null,
 applicationManifest,
 applicationManifestModel,
 payloads: Array.Empty<ClickOnceFileGraphEntry>(),
 adjacentExecutables: new[]
 {
 new ClickOnceFileGraphEntry(
 setupFile,
 "setup.exe",
 ClickOnceFileGraphEntryKind.Setup)
 },
 diagnostics);

This represents an explicit application-manifest input containing a deployment-only setup.exe.

PR #1058 will actually include that file in default staging because adjacent executables are added independently of isDeploymentInput:

if (mode == ClickOnceSigningMode.Default)
{
 candidates.AddRange(
 graph.AdjacentExecutables.Select(
 entry => new StagingCandidate(
 entry,
 isUpdateInputOnly: false)));
}

The resolvers currently avoid constructing this state, but the aggregate allows it and the next layer assigns it behavior.

3. A generic entry permits contradictory roles

ClickOnceFileGraphEntry accepts any combination of:

Source
TargetPath
Kind
ManifestReference?
MappingAddedSuffix?

For example, this payload can be mislabeled as a setup executable:

ClickOnceFileGraphEntry entry = new(
 payloadFile,
 @"bin\payload.dll",
 ClickOnceFileGraphEntryKind.Setup,
 payloadReference);

PR #1058 uses Kind to determine the target-path base directory:

string basePath =
 graphEntry.Kind == ClickOnceFileGraphEntryKind.Payload
 ? applicationDirectoryPath
 : _rootPath;

The mislabeled payload would therefore be staged relative to the deployment root rather than the application-manifest directory.

Other currently constructible combinations include:

// Payload without the manifest reference that owns it.
new ClickOnceFileGraphEntry(
 payloadFile,
 "payload.dll",
 ClickOnceFileGraphEntryKind.Payload,
 manifestReference: null);
// Setup executable with a manifest reference and mapping suffix.
new ClickOnceFileGraphEntry(
 setupFile,
 "setup.exe",
 ClickOnceFileGraphEntryKind.Setup,
 payloadReference,
 mappingAddedSuffix: ".deploy");
// Entry path disagrees with the manifest reference path.
new ClickOnceFileGraphEntry(
 payloadFile,
 @"bin\payload.dll",
 ClickOnceFileGraphEntryKind.Payload,
 new FileReference
 {
 TargetPath = @"lib\payload.dll"
 });

The follow-up therefore needs additional validation and null-forgiving operations, such as validating that a mapped entry has a reference and later using ManifestReference!.

Suggested model

Would it be safer to represent the fixed hierarchy directly?

internal sealed class ResolvedClickOncePublication
{
 internal ResolvedClickOnceDeployment? Deployment { get; }
 internal ResolvedClickOnceApplication Application { get; }
 internal IReadOnlyList<ClickOnceManifestDiagnostic> Diagnostics { get; }
}
internal sealed class ResolvedClickOnceDeployment
{
 internal FileInfo Source { get; }
 internal IDeployManifest Manifest { get; }
 internal AssemblyReference ApplicationManifestReference { get; }
 internal IReadOnlyList<ClickOnceAdjacentExecutable> AdjacentExecutables { get; }
}
internal sealed class ResolvedClickOnceApplication
{
 internal FileInfo Source { get; }
 internal IApplicationManifest Manifest { get; }
 internal IReadOnlyList<ResolvedClickOncePayload> Payloads { get; }
}
internal sealed class ResolvedClickOncePayload
{
 internal FileInfo Source { get; }
 internal BaseReference Reference { get; }
 internal string ApplicationRelativeTargetPath => Reference.TargetPath;
 internal string? MappingAddedSuffix { get; }
}

This would make the important invariants structural:

  • deployment entry and model are either both present or both absent;
  • adjacent executables cannot exist without a deployment;
  • every payload has a manifest reference;
  • payload paths are explicitly application-relative;
  • the exposed target path cannot disagree with Reference.TargetPath;
  • downstream code does not need Kind branching to recover an object's role.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the detailed feedback, @kartheekp-ms . I adopted the typed hierarchy you suggested and addressed the three invalid model states you identified:

  • ResolvedClickOncePublishLayout has one optional ResolvedClickOnceDeployment and a required ResolvedClickOnceApplication, eliminating independently nullable deployment state.
  • Deployment groups its source, manifest, and application-manifest reference. The constructor requires that reference to be the manifest's actual EntryPoint object.
  • Application groups its source, manifest, and typed payloads.
  • Dedicated payload and adjacent-executable types replace the generic graph entry and broad role enum. Every payload requires its manifest reference, while adjacent executables cannot exist without deployment context.
  • Payloads validate the source/target filename relationship and derive IsFileExtensionMapped; adjacent executables derive their target path from their source filename and validate setup.exe or Launcher.exe against their narrow kind.

I made a few deliberate departures from the suggested shape:

  • I used ResolvedClickOncePublishLayout rather than ResolvedClickOncePublication, because the result describes the resolved on-disk layout.
  • Adjacent executables remain on the top-level layout rather than under Deployment. They are discovered by publish-directory convention rather than referenced by the deployment manifest, but the layout constructor still requires deployment context.
  • ResolvedClickOncePayload.TargetPath is an immutable resolution-time snapshot of Reference.TargetPath, not a live projection. This preserves the target value against which source matching and file-extension mapping were validated, while the underlying ManifestUtilities reference remains mutable.
  • I retained TargetPath rather than ApplicationRelativeTargetPath. It is the manifest's target-path value; rooted values are rejected, but relative traversal such as .. is still permitted pending containment validation.
  • IsFileExtensionMapped is a Boolean rather than a nullable suffix because ClickOnce mapping adds only the fixed .deploy suffix.

I also updated the authoritative signing specification to require fail-fast deployment-reference validation before ResolveFiles. A deployment manifest must contain exactly one assembly dependency and no file references; that dependency must be DeployManifest.EntryPoint itself and must not be prerequisite, optional, or resource-fallback. Resource-fallback attributes are inspected in the preserved XML because ManifestUtilities neither models nor preserves them in its object model. Unsupported structures report the planned --clickonce-signing-version 2 --no-update-clickonce-manifest alternative.

Staging and signing remain outside this PR. The updated PR description records the additional integration requirements; they are not implemented by this resolver change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: be91288d-5106-4c59-a1fa-c5353dccb828
@dtivel dtivel changed the title (削除) Add ClickOnce file graph resolution (削除ここまで) (追記) Add ClickOnce publish-layout resolution (追記ここまで) Sep 6, 2026
Add typed resolved models and enforce deployment, payload, mapping, diagnostic, and target-path invariants. Preserve manifest input, validate unsupported deployment shapes, and expand resolver regression coverage.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: de6a34fe-6989-49f6-a128-ee13323c7c9e

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Its broad manifest-resolution and path-safety behavior warrants final human review despite no unresolved comments.

Review tier: Balanced
Findings: None

Files not reviewed (1)
  • src/Sign.Core/Resources.Designer.cs: Generated file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Balanced
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@kartheekp-ms kartheekp-ms Awaiting requested review from kartheekp-ms

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /