feat(agent-installer): add Agent Tunnel configuration dialog#1789
Conversation
Let maintainers know that an action is required on their side
|
a521b61 to
460aeab
Compare
Adds an optional Agent Tunnel wizard step to the Devolutions Agent installer so admins can enroll the agent in a Gateway QUIC tunnel as part of MSI install (UI or unattended). Surfaces three MSI public properties for unattended installs: - AGENT_TUNNEL_ENROLLMENT_STRING (dgw-enroll:v1:<base64> from DVLS/Hub/Gateway) - AGENT_TUNNEL_ADVERTISE_SUBNETS (CSV CIDR; empty = none) - AGENT_TUNNEL_ADVERTISE_DOMAINS (CSV DNS suffixes; empty = auto-detect only) Wires a new deferred elevated custom action (EnrollAgentTunnel) that runs Before StartServices when AGENT_TUNNEL_FEATURE is being installed. It base64-decodes the enrollment payload, shells out to `devolutions-agent.exe enroll <url> <token> <name> [subnets]` with a 60s timeout, and redacts the token in the session log. Advertise domains are persisted by patching `Tunnel.AdvertiseDomains` in agent.json post-enrollment, matching the agreed direction that domain config lives in the file rather than as a CLI flag. The Tunnel feature itself is opt-in (isEnabled:false, allowChange:true); the dialog is skipped when the feature isn't selected. An empty enrollment string also skips tunnel setup, allowing the installer to be used without touching the tunnel.
WixSharp's runtime dialog loader threw at AgentTunnelDialog init (MSI 1603) because the tableLayoutPanel had RowCount=8 but the new gateway URL controls were placed at rows 8/9/10.
…add Agent name field - Propagate AGENT_TUNNEL_* properties to deferred CA via Secure MSI Property declarations + explicit UsesProperties string. The deferred CA was previously seeing empty values because the wizard-set properties never crossed the UAC boundary. - Treat empty enrollment string as install failure (was silent skip). EnrollAgentTunnel CA now returns ActionResult.Failure and surfaces session.Message(InstallMessage.Error, ...) on the empty case and on enrollment timeout, non-zero exit, and exception paths. - Add optional Agent name field to AgentTunnelDialog. Resolution order at install time: dialog value > JWT jet_agent_name claim > computer name. Avoids "missing required --name" failures when the JWT lacks the claim. - Update Wizard.ShouldSkip-gated dialog so blank enrollment is blocked at UI validation (previously the dialog let users click Next on empty).
Captures the root cause behind silent enrollment-success-but-no-tunnel failures we hit during integration testing, the constraints we've confirmed with the team, and the proposed redesign: - Decouple Gateway's cryptographic identity (server cert SAN) from its network reachability (the host agents dial). Replace single conf.hostname with AgentTunnel.AdvertisedNames (multi-SAN, label-able). - Agent derives its QUIC endpoint from the host it enrolled through (jet_gw_url) + a quic_port returned by the gateway, instead of accepting whatever hostname the gateway dictates. - Gateway validates enrollment URL host against AdvertisedNames upfront, with a structured 400 response carrying error/message/help. - New agent.exe verify-tunnel subcommand wired into the MSI CA so install success means the tunnel is actually up, not just that a cert was written. Errors expose a structured kind/detail/next_step triple. - DVLS enrollment-string UI becomes a dropdown over AdvertisedNames (refreshed from gateway diagnostics) instead of a free-text URL box. Includes a 9-entry error catalog with operator-facing next-step text, non-goals (single-use enforcement, gateway farms — deferred), migration path, and a 5-PR implementation plan. Includes Codex's review.
The dialog's optional "Gateway URL (advanced)" textbox encouraged a deployment pattern the Gateway explicitly doesn't support: pointing the agent at the Gateway via a different host/IP than the canonical externally-reachable `Hostname` configured on the Gateway side. Per the team's design, a Gateway has one canonical hostname (the one its HTTPS cert validates against); multiple agent-facing identities are not supported. Removed: - `AGENT_TUNNEL_GATEWAY_URL` MSI public property (Program.cs, AgentProperties.cs) - `--gateway` argument forwarding in CA.EnrollAgentTunnel (CustomActions.cs) - gatewayUrl textbox + label + hint label from AgentTunnelDialog (.cs + .Designer.cs), reduced TableLayoutPanel RowCount from 14 to 11 - `AgentTunnelDlgGatewayUrlLabel` / `AgentTunnelDlgGatewayUrlHint` localization strings (en-us + fr-fr .wxl) The JWT's `jet_gw_url` claim remains the sole source of truth for the agent's enrollment endpoint and persisted tunnel identity. The CLI `--gateway` flag on `agent.exe up` is still available for dev/operator scenarios as an enrollment-transport override only (identity stays anchored to the JWT host).
Align user-facing strings with the project's UI conventions and remove implementation-specific terminology so the installer reads cleanly to an operator who doesn't (and shouldn't have to) know about transport protocols, token formats, or which Devolutions product minted the enrollment string. - Drop "QUIC" from feature description and dialog title/subtitle. The agent tunnel may be implemented over QUIC today; that's not something the operator running the installer needs to think about. - Drop "JWT" from labels, hints, and error messages. "Enrollment string" is the product term we already use and is self-explanatory. - Drop "Devolutions Server, Hub, or Gateway" enumeration. The enrollment string originates from whatever orchestration tool the operator runs (could be DVLS, Hub, a rebranded distribution, or a future third-party integration). Replace with "your gateway operator" — neutral, accurate, doesn't lock in product names. - Drop the JWT-shape and base64url-decodability checks in `AgentTunnelDialog.DoValidate`. The dialog now only checks the enrollment string is non-empty; structural validation happens at the gateway and the gateway's error reaches the operator verbatim through the InstallMessage.Error surface. Avoids the dialog half-validating implementation details that may evolve. - Tighten dialog title from "Agent Tunnel Configuration" to "Agent Tunnel" to match the noun-phrase pattern in the rest of the wxl catalog (e.g. "Destination Folder", "External URL", "Certificate"). - Normalize casing to Title Case for labels (matching existing "Advertise Subnets:" / "Advertise Domains:"). Hints stay sentence case ending with a period. - Update Agent Name hint to describe the fallback chain in plain English without leaking JWT internals. - Reword the enrollment-timeout failure to prompt the operator to check Gateway reachability, which is the actual recovery action. Same edits applied to the French resource file.
2185192 to
e06e67d
Compare
Address review notes against PR #1789: - Subnets now follow the same post-enrollment config-write pattern as domains, so the only mandatory enroll CLI flag is --enrollment-string (with optional --name when the wizard collects one). Renames the helper to WriteTunnelAdvertisementsToConfig, extracts a small SplitCsv helper, and updates the call site. The agent CLI's --advertise-subnets flag is untouched; we just stop using it from the MSI custom action. - DevolutionsAgent_fr-fr.wxl: regroup the new strings to match the en-us catalog. FeatureAgentTunnel* sits with other Feature strings; AgentTunnelDlg* moves into a new <!-- dialogs.agentTunnel --> section near the bottom. - Program.cs: fix mixed-case "Software\..." registry path; align with the surrounding "SOFTWARE\..." entries. - CustomActions.cs: brace single-line if statements, expand the process.Kill() try/catch, add `using System.Text;` and drop the inline `System.Text.Encoding.UTF8` qualification. - AgentTunnelDialog.cs: document why the empty Back/Next/Cancel overrides exist (WixSharp ManagedForm reflects on the leaf dialog type).
The design proposal lives outside this installer PR's scope — it covers Gateway-side cryptographic identity vs network reachability and spans devolutions-gateway, devolutions-agent, and DVLS. The installer change (PR #1789) is independent and ships now; the design discussion tracks separately so reviewers of #1789 aren't asked to evaluate a cross-stack architecture document alongside an MSI dialog. The doc itself is preserved on the `review/agent-tunnel-identity-all` local branch (snapshot commit `d1055bbc`) for the eventual follow-up PRs.
26f23c0
into
master
There was a problem hiding this comment.
Pull request overview
Adds an optional Agent Tunnel feature to the Windows MSI (WixSharp ManagedUI) that, when selected, introduces a new wizard dialog to collect tunnel enrollment inputs and runs a deferred elevated custom action to enroll the installed agent into a Gateway tunnel and patch tunnel advertisement settings into agent.json.
Changes:
- Introduces a new MSI feature (
F.Tunnel) and hooks it into the existing Agent feature tree. - Adds an
AgentTunnelDialogwizard step (skipped unless the feature is selected) and new public MSI properties for unattended installs. - Adds a deferred elevated custom action (
EnrollAgentTunnel) to rundevolutions-agent.exe up --enrollment-string ...and persist advertise subnets/domains into%ProgramData%\Devolutions\Agent\agent.json.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| package/AgentWindowsManaged/Resources/Features.cs | Adds AGENT_TUNNEL_FEATURE and attaches it under AGENT_FEATURE. |
| package/AgentWindowsManaged/Resources/DevolutionsAgent_en-us.wxl | Adds en-us localization strings for the new feature + dialog labels/hints. |
| package/AgentWindowsManaged/Resources/DevolutionsAgent_fr-fr.wxl | Adds fr-fr localization strings for the new feature + dialog labels/hints. |
| package/AgentWindowsManaged/Properties/AgentProperties.cs | Declares new public MSI properties for enrollment string/name/subnets/domains. |
| package/AgentWindowsManaged/Program.cs | Anchors the new feature with a registry component; declares tunnel properties as Secure (and enrollment string Hidden). |
| package/AgentWindowsManaged/Dialogs/Wizard.cs | Inserts AgentTunnelDialog into the wizard sequence and adds skip logic based on ADDLOCAL. |
| package/AgentWindowsManaged/Dialogs/AgentTunnelDialog.Designer.cs | New UI layout for enrollment string/name/subnets/domains. |
| package/AgentWindowsManaged/Dialogs/AgentTunnelDialog.cs | Wires UI fields to MSI properties and adds minimal validation. |
| package/AgentWindowsManaged/Actions/CustomActions.cs | Implements EnrollAgentTunnel and config patching into agent.json. |
| package/AgentWindowsManaged/Actions/AgentActions.cs | Schedules the deferred elevated EnrollAgentTunnel CA before StartServices. |
Files not reviewed (1)
- package/AgentWindowsManaged/Dialogs/AgentTunnelDialog.Designer.cs: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| if (enrollmentString.Length == 0) | ||
| { | ||
| return Fail("An enrollment string is required. Paste the enrollment string provided by your gateway operator, or deselect the Agent Tunnel feature."); |
There was a problem hiding this comment.
Not acceptable
There was a problem hiding this comment.
Hi — quick correction on this one. We initially acted on this suggestion and switched the empty-string case to a no-op (skip), but we've since reverted it: an empty enrollment string keeps failing the install intentionally.
The "non-tunnel / unattended deployments" concern doesn't actually apply here. EnrollAgentTunnel only runs under the AGENT_TUNNEL_FEATURE.BeingInstall() condition, and the Agent Tunnel feature is opt-in (Level 2 — not installed by default). A deployment that doesn't select the tunnel feature never reaches this code, so it can't trigger the failure. The only way to hit the empty-string branch is to explicitly select the Agent Tunnel feature (ADDLOCAL=...,F.Tunnel or ALL) while omitting the enrollment string — which is a misconfiguration we'd rather surface than silently skip.
(The genuinely separate bug nearby — ADDLOCAL=ALL wrongly skipping the dialog — is fixed in #1804.)
| if (string.IsNullOrWhiteSpace(enrollmentString.Text)) | ||
| { | ||
| ShowValidationErrorString("An enrollment string is required. Paste the enrollment string provided by your gateway operator, or go back and deselect the Agent Tunnel feature."); | ||
| return false; |
| if (!process.WaitForExit(60_000)) | ||
| { | ||
| try | ||
| { | ||
| process.Kill(); |
| // (see Wizard.ShouldSkip), so an enrollment string is required here. | ||
| // Structural validation of the string itself happens server-side at | ||
| // enrollment time — surface that gateway error verbatim rather than | ||
| // half-validating implementation details (signature, encoding, etc.) | ||
| // here. |
| return Fail("Agent tunnel enrollment timed out. Verify your Devolutions Gateway is reachable from this machine."); | ||
| } |
| string arguments = $"up --enrollment-string \"{enrollmentString}\""; | ||
| if (resolvedName.Length != 0) | ||
| { | ||
| arguments += $" --name \"{resolvedName}\""; | ||
| } |
| { | ||
| Execute = Execute.deferred, | ||
| Impersonate = false, | ||
| // Deferred CAs only see properties bubbled through CustomActionData. The Set_<CA>_Props |
There was a problem hiding this comment.
This is just documented Windows Installer, I'd remove this comment
| Impersonate = false, | ||
| // Deferred CAs only see properties bubbled through CustomActionData. The Set_<CA>_Props | ||
| // immediate action expands [PROP] for each entry below before the deferred CA runs. | ||
| UsesProperties = string.Join(";", new[] |
There was a problem hiding this comment.
There's a helper function that provides error checking, use e.g.
UsesProperties = UseProperties(new [] { AgentProperties.installId })
| AgentProperties.AgentTunnelAdvertiseDomains, | ||
| AgentProperties.InstallDir, | ||
| }.Select(p => $"{p}=[{p}]")), | ||
| }; |
There was a problem hiding this comment.
I'm not exactly sure what is being passed in these properties, but are you certain it doesn't/won't contain a semi-colon? Semi-colons break CustomActionData and need special handling. This is a known Windows Installer quirk. If you're sure it's ok, that's fine; if you're not I can explain how to fix it.
Summary
Adds an optional Agent Tunnel wizard step to the Devolutions Agent MSI installer so admins can enrol the agent in a Gateway QUIC tunnel during install — UI or unattended.
Three text fields in the dialog: enrolment string, advertise subnets, advertise domains. The feature is opt-in (
isEnabled: false, allowChange: true); the dialog is skipped when the feature isn't selected, and an empty enrolment string skips tunnel setup even if the feature is on, so the installer remains usable for non-tunnel deployments.Smoke-tested end-to-end against a local DVLS + Gateway + Agent stack: MSI built clean, agent enrolled successfully, RDP TCP traffic routed through the agent tunnel to
IT-HELP-DC:3389.MSI public properties (unattended install)
Custom action
EnrollAgentTunnelis deferred + elevated +Impersonate=false, runsBefore StartServices, gated byFeatures.AGENT_TUNNEL_FEATURE.BeingInstall(). It:dgw-enroll:v1:prefix, strips whitespace, pads base64url, decodes JSON payload (api_base_url,enrollment_token, optionalname).devolutions-agent.exe enroll <url> <token> <name> [subnets]with a 60s timeout; on timeout the child is killed.***) symmetrically across the command-line log and child stdout/stderr.Tunnel.AdvertiseDomainsof%ProgramData%\Devolutions\Agent\agent.json. This matches the design that advertise domains live in config, not on the CLI (replaces the closed feat(agent): --advertise-domains CLI flag #1774).Depends on
/jet/tunnel/enroll)Both merged to master 2026-05-20.
Test plan
CNDL1138/CNDL1006warnings from existing code, not introduced here)dgw-enroll:v1:prefix → validation error in dialogIT-HELP-DC:3389succeedsReview history
Pre-review and codex-review passes folded into the single commit. Notable points addressed:
JObject,Value<string>(),IsNullOrWhiteSpacevalidation)WaitForExit(60_000)return-value check with child kill on timeoutInstallDirDlg*strings leaked into en-us