From 682d9b243b406c26b8a9fa4719d3acc5bb57741b Mon Sep 17 00:00:00 2001 From: Atif Ali Date: Wed, 29 Jul 2026 13:21:56 +0000 Subject: [PATCH 1/2] fix(registry/coder/modules/windows-rdp): escape special characters in credentials --- registry/coder/modules/windows-rdp/README.md | 8 +- .../modules/windows-rdp/devolutions-patch.js | 6 +- .../coder/modules/windows-rdp/main.test.ts | 87 ++++++++++++------- registry/coder/modules/windows-rdp/main.tf | 23 ++++- .../powershell-installation-script.tftpl | 6 +- 5 files changed, 87 insertions(+), 43 deletions(-) diff --git a/registry/coder/modules/windows-rdp/README.md b/registry/coder/modules/windows-rdp/README.md index 111e8d7fd..bc5b4d3de 100644 --- a/registry/coder/modules/windows-rdp/README.md +++ b/registry/coder/modules/windows-rdp/README.md @@ -15,7 +15,7 @@ Enable Remote Desktop + a web based client on Windows workspaces, powered by [de module "windows_rdp" { count = data.coder_workspace.me.start_count source = "registry.coder.com/coder/windows-rdp/coder" - version = "1.3.0" + version = "1.3.1" agent_id = coder_agent.main.id } ``` @@ -32,7 +32,7 @@ module "windows_rdp" { module "windows_rdp" { count = data.coder_workspace.me.start_count source = "registry.coder.com/coder/windows-rdp/coder" - version = "1.3.0" + version = "1.3.1" agent_id = coder_agent.main.id } ``` @@ -43,7 +43,7 @@ module "windows_rdp" { module "windows_rdp" { count = data.coder_workspace.me.start_count source = "registry.coder.com/coder/windows-rdp/coder" - version = "1.3.0" + version = "1.3.1" agent_id = coder_agent.main.id } ``` @@ -54,7 +54,7 @@ module "windows_rdp" { module "windows_rdp" { count = data.coder_workspace.me.start_count source = "registry.coder.com/coder/windows-rdp/coder" - version = "1.3.0" + version = "1.3.1" agent_id = coder_agent.main.id devolutions_gateway_version = "2025.2.2" # Specify a specific version } diff --git a/registry/coder/modules/windows-rdp/devolutions-patch.js b/registry/coder/modules/windows-rdp/devolutions-patch.js index 1f231ca3f..0d49d6534 100644 --- a/registry/coder/modules/windows-rdp/devolutions-patch.js +++ b/registry/coder/modules/windows-rdp/devolutions-patch.js @@ -47,8 +47,10 @@ * * All properties should be defined as placeholder templates in the form * VALUE_NAME. The Coder module, when spun up, should then run some logic to - * replace the template slots with actual values. These values should never - * change from within JavaScript itself. + * replace the template slots with actual values. The module JSON-escapes each + * value before injecting it, so characters like backslashes and quotes stay + * intact inside these string literals. These values should never change from + * within JavaScript itself. * * @satisfies {FormFieldEntries} */ diff --git a/registry/coder/modules/windows-rdp/main.test.ts b/registry/coder/modules/windows-rdp/main.test.ts index 80c09fd0d..c9086c44c 100644 --- a/registry/coder/modules/windows-rdp/main.test.ts +++ b/registry/coder/modules/windows-rdp/main.test.ts @@ -35,6 +35,31 @@ function findWindowsRdpScript(state: TerraformState): string | null { return null; } +/** + * Extracts the username and password the module injected into the JS patch + * file. + * + * The values are injected as JSON-escaped content inside double-quoted JS + * string literals, so the matched literals are parsed with JSON.parse to get + * the original values back. The regex stays verbose and pedantic on purpose: + * it validates the structure of the form entries object and, by only matching + * non-quote characters or escape pairs, it cannot overshoot into later content. + */ +function extractFormFieldValues(rdpScript: string): { + username?: string; + password?: string; +} { + const formEntryValuesRe = + /username:\s*\{[\s\S]*?value:\s*(?"(?:[^"\\]|\\.)*")[\s\S]*?password:\s*\{[\s\S]*?value:\s*(?"(?:[^"\\]|\\.)*")/; + + const groups = formEntryValuesRe.exec(rdpScript)?.groups ?? {}; + + return { + username: groups.username && JSON.parse(groups.username), + password: groups.password && JSON.parse(groups.password), + }; +} + /** * @todo It would be nice if we had a way to verify that the Devolutions root * HTML file is modified to include the import for the patched Coder script, @@ -69,27 +94,6 @@ describe("Web RDP", async () => { }); it("Injects Terraform's username and password into the JS patch file", async () => { - /** - * Using a regex as a quick-and-dirty way to get at the username and - * password values. - * - * Tried going through the trouble of extracting out the form entries - * variable from the main output, converting it from Prettier/JS-based JSON - * text to universal JSON text, and exposing it as a parsed JSON value. That - * got to be a bit too much, though. - * - * Regex is a little bit more verbose and pedantic than normal. Want to - * have some basic safety nets for validating the structure of the form - * entries variable after the JS file has had values injected. Even with all - * the wildcard classes set to lazy mode, we want to make sure that they - * don't overshoot and grab too much content. - * - * Written and tested via Regex101 - * @see {@link https://regex101.com/r/UMgQpv/2} - */ - const formEntryValuesRe = - /username:\s*\{[\s\S]*?value:\s*"(?[^"]+)"[\s\S]*?password:\s*\{[\s\S]*?value:\s*"(?[^"]+)"/; - // Test that things work with the default username/password const defaultState = await runTerraformApply( import.meta.dir, @@ -101,11 +105,10 @@ describe("Web RDP", async () => { const defaultRdpScript = findWindowsRdpScript(defaultState); expect(defaultRdpScript).toBeString(); - const defaultResultsGroup = - formEntryValuesRe.exec(defaultRdpScript ?? "")?.groups ?? {}; - - expect(defaultResultsGroup.username).toBe("Administrator"); - expect(defaultResultsGroup.password).toBe("coderRDP!"); + expect(extractFormFieldValues(defaultRdpScript ?? "")).toEqual({ + username: "Administrator", + password: "coderRDP!", + }); // Test that custom usernames/passwords are also forwarded correctly const customAdminUsername = "crouton"; @@ -122,10 +125,34 @@ describe("Web RDP", async () => { const customRdpScript = findWindowsRdpScript(customizedState); expect(customRdpScript).toBeString(); - const customResultsGroup = - formEntryValuesRe.exec(customRdpScript ?? "")?.groups ?? {}; + expect(extractFormFieldValues(customRdpScript ?? "")).toEqual({ + username: customAdminUsername, + password: customAdminPassword, + }); + }); + + it("Preserves special characters in the password", async () => { + // Covers the characters that break naive string interpolation in either the + // JS patch file or the PowerShell installation script. + const specialPassword = "N;JVO*U\\mL^a*P\"'`$&<>|{}[]%@:~"; + + const state = await runTerraformApply(import.meta.dir, { + agent_id: "foo", + admin_password: specialPassword, + }); - expect(customResultsGroup.username).toBe(customAdminUsername); - expect(customResultsGroup.password).toBe(customAdminPassword); + const rdpScript = findWindowsRdpScript(state); + expect(rdpScript).toBeString(); + + // The JS patch file must receive the password verbatim once parsed. + expect(extractFormFieldValues(rdpScript ?? "").password).toBe( + specialPassword, + ); + + // PowerShell single-quoted strings are literal, and a literal single quote + // is escaped by doubling it. + expect(rdpScript).toContain( + `Set-AdminPassword -adminPassword '${specialPassword.replaceAll("'", "''")}'`, + ); }); }); diff --git a/registry/coder/modules/windows-rdp/main.tf b/registry/coder/modules/windows-rdp/main.tf index 3c83d195b..c1035ce18 100644 --- a/registry/coder/modules/windows-rdp/main.tf +++ b/registry/coder/modules/windows-rdp/main.tf @@ -70,22 +70,37 @@ variable "devolutions_gateway_version" { description = "Version of Devolutions Gateway to install. Use 'latest' for the most recent version, or specify a version like '2025.3.2'." } +locals { + # The Devolutions patch script embeds these values inside double-quoted JS + # string literals. jsonencode escapes backslashes, quotes, control characters, + # and HTML-significant characters; the outer quotes are trimmed because the JS + # file supplies its own. + js_admin_username = trimsuffix(trimprefix(jsonencode(var.admin_username), "\""), "\"") + js_admin_password = trimsuffix(trimprefix(jsonencode(var.admin_password), "\""), "\"") + + # The installation script passes these values as PowerShell single-quoted + # strings, which are literal apart from the single quote itself. Doubling the + # single quotes keeps values containing $, backticks, or double quotes intact. + ps_admin_username = replace(var.admin_username, "'", "''") + ps_admin_password = replace(var.admin_password, "'", "''") +} + resource "coder_script" "windows-rdp" { agent_id = var.agent_id display_name = "windows-rdp" icon = "/icon/rdp.svg" script = templatefile("${path.module}/powershell-installation-script.tftpl", { - admin_username = var.admin_username - admin_password = var.admin_password + admin_username = local.ps_admin_username + admin_password = local.ps_admin_password devolutions_gateway_version = var.devolutions_gateway_version # Wanted to have this be in the powershell template file, but Terraform # doesn't allow recursive calls to the templatefile function. Have to feed # results of the JS template replace into the powershell template patch_file_contents = templatefile("${path.module}/devolutions-patch.js", { - CODER_USERNAME = var.admin_username - CODER_PASSWORD = var.admin_password + CODER_USERNAME = local.js_admin_username + CODER_PASSWORD = local.js_admin_password }) }) diff --git a/registry/coder/modules/windows-rdp/powershell-installation-script.tftpl b/registry/coder/modules/windows-rdp/powershell-installation-script.tftpl index 1657b878d..fdd5a1b92 100644 --- a/registry/coder/modules/windows-rdp/powershell-installation-script.tftpl +++ b/registry/coder/modules/windows-rdp/powershell-installation-script.tftpl @@ -6,9 +6,9 @@ function Set-AdminPassword { Import-Module Microsoft.PowerShell.LocalAccounts -ErrorAction SilentlyContinue # Set admin password - Get-LocalUser -Name "${admin_username}" | Set-LocalUser -Password (ConvertTo-SecureString -AsPlainText $adminPassword -Force) + Get-LocalUser -Name '${admin_username}' | Set-LocalUser -Password (ConvertTo-SecureString -AsPlainText $adminPassword -Force) # Enable admin user - Get-LocalUser -Name "${admin_username}" | Enable-LocalUser + Get-LocalUser -Name '${admin_username}' | Enable-LocalUser } function Configure-RDP { @@ -125,7 +125,7 @@ if ($isPatched -eq $null) { } } -Set-AdminPassword -adminPassword "${admin_password}" +Set-AdminPassword -adminPassword '${admin_password}' Configure-RDP Install-DevolutionsGateway Patch-Devolutions-HTML From cfbb20e1562a0d860b445df4649735ab9035772b Mon Sep 17 00:00:00 2001 From: Atif Ali Date: Tue, 11 Aug 2026 15:42:11 +0000 Subject: [PATCH 2/2] fix(registry/coder/modules/windows-rdp): update Devolutions selectors for 2026.2.x The auto-fill script targeted PrimeNG class names and component names that no longer exist on Gateway 2026.2.4, so the form filled but never submitted, and the in-session toolbar was never found. - protocol: p-dropdown is now p-select, and RDP is already the default, so the explicit selection is dropped rather than driving a PrimeNG overlay - submit: select button[type=submit] inside the form instead of matching an exact p-element class list - toolbar: session-toolbar is now floating-session-toolbar, and the close button is an icon button with aria-label Close session - checkboxes: Unicode Keyboard Mode is gone, and dynamic resize moved onto the connection form as enableDisplayControl, already enabled by default - both polls are now bounded and reveal the form with a visible banner when auto-connect fails, instead of looping forever and logging to a console the user never opens - stop logging credential values to the browser console --- .../modules/windows-rdp/devolutions-patch.js | 228 +++++++++++------- .../coder/modules/windows-rdp/main.test.ts | 2 +- 2 files changed, 141 insertions(+), 89 deletions(-) diff --git a/registry/coder/modules/windows-rdp/devolutions-patch.js b/registry/coder/modules/windows-rdp/devolutions-patch.js index 0d49d6534..f2a823d5a 100644 --- a/registry/coder/modules/windows-rdp/devolutions-patch.js +++ b/registry/coder/modules/windows-rdp/devolutions-patch.js @@ -26,11 +26,6 @@ * @typedef {Readonly>} FormFieldEntries */ (function () { - /** - * The communication protocol to set Devolutions to. - */ - const PROTOCOL = "RDP"; - /** * The hostname to use with Devolutions. */ @@ -41,6 +36,35 @@ */ const POLL_INTERVAL_MS = 500; + /** + * How many times each poll retries before giving up and surfacing the + * failure to the user. The Angular app can take upwards of 20 seconds to + * render the form on a cold start, so these ceilings are deliberately + * generous. + */ + const MAX_FORM_POLLS = 240; + const MAX_TOOLBAR_POLLS = 120; + + /** + * Selectors for the Devolutions markup this script drives. + * + * Devolutions builds its UI on PrimeNG, whose component names and class + * lists change between releases. Prefer structural hooks (element type, + * input type, aria-label) over class names, which have already broken once: + * `p-dropdown` became `p-select`, `session-toolbar` became + * `floating-session-toolbar`, and the submit button lost the `p-element` + * class. Where a selector list is used, the newest form comes first and + * older forms are kept as fallbacks. + */ + const selectors = { + form: "web-client-form form", + hostname: "p-autocomplete#hostname input", + submitButton: 'button[type="submit"], p-button button', + displayControl: "input#enableDisplayControl", + sessionToolbar: "floating-session-toolbar, session-toolbar", + closeSession: 'button[aria-label="Close session" i]', + }; + /** * The fields in the Devolutions sign-in form that should be populated with * values from the Coder workspace. @@ -73,6 +97,66 @@ }, }; + /** + * CSS variable that controls whether the sign-in form is visible. Declared + * at this scope so that failure paths can reveal the form again. + */ + const cssOpacityVariableName = "--coder-opacity-multiplier"; + + /** + * Makes the Devolutions sign-in form visible again. + * + * @returns {void} + */ + function revealForm() { + const rootNode = document.querySelector(":root"); + if (rootNode instanceof HTMLHtmlElement) { + rootNode.style.setProperty(cssOpacityVariableName, "1"); + } + } + + /** + * Reveals the form and displays a banner explaining that auto-connect did + * not work. + * + * Without this, a selector that stops matching leaves the user staring at a + * form that silently does nothing, with the explanation buried in a console + * they are unlikely to open. + * + * @param {string} reason + * @returns {void} + */ + function reportAutoConnectFailure(reason) { + log(`Auto-connect failed: $${reason}`); + revealForm(); + + const bannerId = "coder-patch--auto-connect-failure"; + // biome-ignore lint/style/useTemplate: Have to skip interpolation for the main.tf interpolation + if (document.querySelector("#" + bannerId)) { + return; + } + + const banner = document.createElement("div"); + banner.id = bannerId; + banner.textContent = + "Coder could not sign in to this session automatically. " + + "The form below has been filled in, so you can connect manually."; + banner.style.cssText = [ + "position: fixed", + "top: 0", + "left: 0", + "right: 0", + "z-index: 9999", + "padding: 12px 16px", + "background: #fde68a", + "color: #1f2937", + "font-family: sans-serif", + "text-align: center", + ].join(";"); + + document.body.appendChild(banner); + } + /** * This ensures that the Devolutions login form (which by default, always shows * up on screen when the app first launches) stays visually hidden from the user @@ -87,7 +171,6 @@ */ function hideFormForInitialSubmission() { const styleId = "coder-patch--styles-initial-submission"; - const cssOpacityVariableName = "--coder-opacity-multiplier"; /** @type {HTMLStyleElement | null} */ // biome-ignore lint/style/useTemplate: Have to skip interpolation for the main.tf interpolation @@ -147,7 +230,7 @@ // of the rest of the app. Even if the form isn't hidden at the style level, // it will still be covered up. const restoreOpacity = () => { - rootNode.style.setProperty(cssOpacityVariableName, "1"); + revealForm(); }; // If this file gets more complicated, it might make sense to set up the @@ -272,25 +355,11 @@ try { log("Form detected. Starting auto-fill..."); - // By default, RDP is selected. Leaving this here if needed - // in the future. - const protocolTrigger = form.querySelector('p-dropdown[id="protocol"]'); - if (protocolTrigger) { - protocolTrigger.click(); - const protocolOption = document.querySelector( - `li[aria-label="$${PROTOCOL}"]`, - ); - if (protocolOption) { - protocolOption.click(); - log(`Protocol set to $${PROTOCOL}`); - } else { - log("Protocol option not found."); - } - } else { - log("Protocol dropdown trigger not found."); - } + // The protocol control defaults to RDP, so this script does not touch + // it. Selecting it explicitly meant driving a PrimeNG overlay, which is + // exactly the kind of markup that changes between Gateway releases. - const hostnameInput = form.querySelector("p-autocomplete#hostname input"); + const hostnameInput = form.querySelector(selectors.hostname); if (hostnameInput) { await setInputValue(hostnameInput, HOSTNAME); log(`Hostname set to $${HOSTNAME}`); @@ -304,23 +373,31 @@ const input = document.querySelector(querySelector); if (input) { await setInputValue(input, value); - log(`Set $${key} to $${value}`); + log(`Set $${key}`); } else { log(`Input for $${key} not found with selector: $${querySelector}`); } } - const submitButton = form.querySelector( - 'p-button[class="p-element"] button', - ); + // Resizes the remote display to match the browser window. Enabled by + // default on current Gateway versions, so this only has to correct it + // when it is not. + const displayControl = document.querySelector(selectors.displayControl); + if (displayControl && !displayControl.checked) { + displayControl.click(); + log("Enabled display control."); + } + + const submitButton = form.querySelector(selectors.submitButton); if (submitButton && !submitButton.disabled) { submitButton.click(); log("Form submitted."); } else { - log("Submit button not found or disabled."); + reportAutoConnectFailure("submit button not found or disabled"); } } catch (err) { console.error("[Devolutions Patch] Error during form fill:", err); + reportAutoConnectFailure("unexpected error while filling the form"); } } @@ -333,15 +410,10 @@ * @returns {void} */ function attachCloseListener(topBar) { - const buttons = topBar.querySelectorAll("button"); - - const closeButton = Array.from(buttons).find((button) => { - const labelSpan = button.querySelector(".p-button-label"); - return labelSpan && labelSpan.textContent.trim() === "Close Session"; - }); + const closeButton = topBar.querySelector(selectors.closeSession); if (closeButton) { - closeButton.parentElement.addEventListener("click", () => { + closeButton.addEventListener("click", () => { window.close(); }); log("Close listener attached."); @@ -351,79 +423,59 @@ } /** - * Sets the checked state of a checkbox based on its label text. - * Searches all components in the document and identifies the one - * whose label matches the provided `filterText`. Once found, it sets the checkbox - * to the specified `checked` state (true or false) and dispatches a change event - * to ensure any bound listeners (e.g., Angular change detection) are triggered. - * Logs the outcome of the operation for debugging or audit purposes. - * - * @param {string} filterText - The exact label text of the checkbox to target. - * @param {boolean} checked - The desired checked state (true to check, false to uncheck). - * @returns {void} - */ - function setCheckbox(filterText, checked) { - const checkboxes = document.querySelectorAll("p-checkbox"); - - const targetCheckbox = Array.from(checkboxes).find((checkbox) => { - const label = checkbox.querySelector(".p-checkbox-label"); - return label && label.textContent.trim() === filterText; - }); - - if (targetCheckbox) { - const input = targetCheckbox.querySelector('input[type="checkbox"]'); - if (input) { - input.checked = checked; - input.dispatchEvent(new Event("change", { bubbles: true })); - } - log(`$${filterText} set to $${checked}.`); - } else { - log(`$${filterText} checkbox not found in top bar.`); - } - } - - /** - * Continuously polls the DOM for a specific form element. + * Continuously polls the DOM for the sign-in form, up to MAX_FORM_POLLS + * attempts. * - Searches for a
inside a element. * - If found, calls `fillForm(form)` to process it. - * - If not found, logs a retry message and schedules another check after a delay. + * - If the ceiling is reached, surfaces the failure to the user. * + * @param {number} [attempt] * @returns {void} */ - function pollForForm() { - const form = document.querySelector("web-client-form form"); + function pollForForm(attempt = 0) { + const form = document.querySelector(selectors.form); if (form) { fillForm(form); // Start polling for top bar after form is filled pollForSessionToolBar(); - } else { - log("Form not yet available. Retrying..."); - setTimeout(pollForForm, POLL_INTERVAL_MS); + return; } + + if (attempt >= MAX_FORM_POLLS) { + reportAutoConnectFailure("sign-in form never appeared"); + return; + } + + log("Form not yet available. Retrying..."); + setTimeout(() => pollForForm(attempt + 1), POLL_INTERVAL_MS); } /** - * Continuously polls the DOM for a specific form element. - * - Searches for a element. - * - If found, adds another listener to session toolbar - * - If not found, logs a retry message and schedules another check after a delay. + * Continuously polls the DOM for the in-session toolbar, up to + * MAX_TOOLBAR_POLLS attempts. + * - Searches for the floating session toolbar element. + * - If found, attaches the close listener. + * - If the ceiling is reached, stops polling and logs. * + * @param {number} [attempt] * @returns {void} */ - function pollForSessionToolBar() { - const sessionToolBar = document.querySelector("session-toolbar"); + function pollForSessionToolBar(attempt = 0) { + const sessionToolBar = document.querySelector(selectors.sessionToolbar); if (sessionToolBar) { log("Top bar detected. Proceeding with next steps..."); attachCloseListener(sessionToolBar); + return; + } - // Automatically set checkboxes to improve user experience - setCheckbox("Unicode Keyboard Mode", true); - setCheckbox("Dynamic Resize", true); - } else { - log("Top bar not yet available. Retrying..."); - setTimeout(pollForSessionToolBar, POLL_INTERVAL_MS); + if (attempt >= MAX_TOOLBAR_POLLS) { + log("Top bar never appeared. Stopped polling."); + return; } + + log("Top bar not yet available. Retrying..."); + setTimeout(() => pollForSessionToolBar(attempt + 1), POLL_INTERVAL_MS); } /** diff --git a/registry/coder/modules/windows-rdp/main.test.ts b/registry/coder/modules/windows-rdp/main.test.ts index c9086c44c..f34899c52 100644 --- a/registry/coder/modules/windows-rdp/main.test.ts +++ b/registry/coder/modules/windows-rdp/main.test.ts @@ -152,7 +152,7 @@ describe("Web RDP", async () => { // PowerShell single-quoted strings are literal, and a literal single quote // is escaped by doubling it. expect(rdpScript).toContain( - `Set-AdminPassword -adminPassword '${specialPassword.replaceAll("'", "''")}'`, + `Set-AdminPassword -adminPassword '${specialPassword.replace(/'/g, "''")}'`, ); }); });