Skip to content

fix(windows-rdp): escape credentials and fix auto-connect on Gateway 2026.2.x - #1034

Merged
matifali merged 8 commits into
mainfrom
fix/windows-rdp-password-escaping
Aug 20, 2026
Merged

fix(windows-rdp): escape credentials and fix auto-connect on Gateway 2026.2.x#1034
matifali merged 8 commits into
mainfrom
fix/windows-rdp-password-escaping

Conversation

@matifali

@matifali matifali commented Jul 29, 2026

Copy link
Copy Markdown
Member

Problem

Two separate defects made the Web RDP session fail, and they compound: the credentials were mangled in transit, and even correct credentials were never submitted.

1. Credentials were interpolated without escaping (#20)

  • devolutions-patch.js embeds the password inside a double-quoted JS string literal (value: "${CODER_PASSWORD}"). JS parses the injected \m as an escape sequence and drops the backslash, so N;JVO*U\mL^a*P becomes N;JVO*UmL^a*P. A " breaks the patch script entirely.
  • powershell-installation-script.tftpl passes the password as a PowerShell double-quoted string (Set-AdminPassword -adminPassword "${admin_password}"), where $, backtick, and " are interpreted. The Windows account then gets a different password than Coder displays.

Both paths end in STATUS_LOGON_FAILURE [0xc000006d], and whether a user hits it depends on which characters their generated password happens to contain.

2. The auto-fill selectors no longer match Gateway 2026.2.x (#1044)

The module defaults to devolutions_gateway_version = "latest". On 2026.2.4 the form fills but never submits, and the in-session toolbar is never found, so the close-window listener never attaches and the poll loops forever.

Changes

Credential escaping

Destination Encoding Handles
JS patch file jsonencode, outer quotes trimmed \, ", control chars, and < > & as \u003c style escapes
PowerShell script single-quoted string, ' doubled $, backtick, ", \, and everything else literally

The JS file keeps a real string literal, so it stays valid JavaScript for Prettier and @ts-check.

Selector updates

Was Now
p-dropdown[id="protocol"] dropped, RDP is already the default and driving a PrimeNG overlay is what broke
p-button[class="p-element"] button button[type="submit"] inside the form
session-toolbar floating-session-toolbar, close button matched by aria-label
setCheckbox("Unicode Keyboard Mode") removed, the option no longer exists
setCheckbox("Dynamic Resize") input#enableDisplayControl on the connection form, corrected only when not already enabled

Both polls are now bounded. On failure the form is revealed with a visible banner instead of looping forever and logging to a console the user never opens. Credential values are no longer logged to the browser console.

Module bumped to 1.3.1.

Validation

Provisioned a real AWS Windows workspace from a copy of aws-windows-desktop, with the module sourced from this branch and admin_password = "Te\st$Pa\"ss'word&<>|1!"`.

Rendered output:

value: "Te\\st$Pa\"ss'w`ord\u0026\u003c\u003e|1!",
Set-AdminPassword -adminPassword 'Te\st$Pa"ss''w`ord&<>|1!'

On the VM, the account password round-trips exactly:

$ctx.ValidateCredentials('Administrator', 'Te\st$Pa"ss''w`ord&<>|1!')   # True
$ctx.ValidateCredentials('Administrator', 'coderRDP!')                  # False

In the browser, unattended, no clicks:

[Devolutions Patch] Form detected. Starting auto-fill...
[Devolutions Patch] Hostname set to localhost
[Devolutions Patch] Set username
[Devolutions Patch] Set password
[Devolutions Patch] Form submitted.
[Devolutions Patch] Top bar detected. Proceeding with next steps...
[Devolutions Patch] Close listener attached.
canvas: 1   floating-session-toolbar: 1   failure banner: 0

ironrdp web::session: Connected!, live Windows desktop, no STATUS_LOGON_FAILURE.

bun test main.test.ts passes (5/5); prettier and terraform fmt are clean.

Note

The same escaping bug exists in amazon-dcv-windows (#1035) and local-windows-rdp (#1043). Both are separate modules with separate releases, so they are filed rather than folded in here.

Caution

devolutions_gateway_version still defaults to latest, which is the root cause of the selector breakage: an upstream UI change silently breaks every workspace using this module. Worth deciding separately whether to pin a known-good default. Tracked in #1044.

Closes #20
Closes #1044

🤖 This PR was created with the help of Coder Agents, and needs a human review. 🧑‍💻

@github-actions

github-actions Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Module Scorecard Check

coder/windows-rdp: 45 → 57

Score improvement: 45 → 57 (+12).

Theme Before After
Presentation & Onboarding 17 / 25 17 / 25
Integration
Credential Hygiene 10 / 20 18 / 20
Restricted-Environment 0 / 20 0 / 20
Engineering Quality 7 / 10 8 / 10
Overall 45 / 100 57 / 100
Full scorecard for this PR
Presentation & Onboarding Credential Hygiene Restricted-Environment Readiness Engineering Quality Overall
17 / 25 18 / 20 0 / 20 8 / 10 57 / 100
Drilldown

Presentation & Onboarding — 17 / 25

Criterion Max Score Notes
Configuration-mode examples 12 12 Multiple provider examples (AWS, Google Cloud) with sensible defaults. Custom version example shows configuration flexibility.
Coder-context framing 8 0 README does not explain what the module adds on top of Coder or how Coder fits in the RDP flow. It names the target tool (Devolutions Gateway) but lacks context about Coder's role.
Visual preview 5 5 Video thumbnail image embedded with link to video demonstration.

Credential Hygiene — 18 / 20

Criterion Max Score Notes
Secrets marked sensitive 16 16 admin_password variable marked sensitive = true. README examples do not inline secrets (use module defaults).
Non-hardcoded auth path 4 2 Module uses local Windows authentication with configurable username/password. No external auth mechanism documented, but credentials are parameterized rather than hardcoded. Partial credit for parameterization.

Restricted-Environment Readiness — 0 / 20

Criterion Max Score Notes
Mirrorable artifact source 10 0 No module variable overrides the Devolutions Gateway download URL. The module installs from PSGallery via Install-Module with no URL override capability. Version can be pinned via devolutions_gateway_version but this is not a URL override.
Bring-your-own binary 5 0 No documented way to skip installation when Devolutions Gateway is pre-installed. The coder_script always runs the installation logic.
Egress transparency 3 0 No dedicated README section enumerating external endpoints. PSGallery and Devolutions endpoints are implicit in code but not documented.
Runs without sudo 2 0 PowerShell script performs system-wide installation (Install-Module for all users), modifies HKLM registry keys, configures Windows services, and modifies Program Files. These operations require administrator privileges. No non-privileged fallback documented or implemented.

Engineering Quality — 8 / 10

Criterion Max Score Notes
Input quality 6 6 Variables have clear descriptions and sensible defaults. share variable includes validation. admin_password properly marked sensitive. devolutions_gateway_version has descriptive documentation.
Test coverage 4 2 TypeScript tests cover password injection and special character handling. Tests verify script generation but not end-to-end RDP functionality. No .tftest.hcl files present. Partial credit for existing test coverage.

Overall — 57 / 100

Raw 43 / 75 → round(43 / 75 × 100) = 57


Scored against SCORECARD.md with claude-sonnet-4-5. Language-model scores are advisory.

… 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
@matifali matifali changed the title fix(windows-rdp): escape special characters in RDP credentials fix(windows-rdp): escape credentials and fix auto-connect on Gateway 2026.2.x Aug 11, 2026
@matifali
matifali marked this pull request as ready for review August 11, 2026 15:51
bpmct added a commit that referenced this pull request Aug 11, 2026
… API (#1046)

Fixes the Module Scorecard Check commenting on modules a PR never
touched, reported on #1034 where a `windows-rdp` PR got a scorecard
comparison for `coder/git-clone`.

Linear:
[REG-74](https://linear.app/codercom/issue/REG-74/unrelated-module-scorecards-on-pr-commnets)

## Root cause

The workflow detected changed modules with `git diff
"${BASE_SHA}"...HEAD`, where `BASE_SHA` is
`github.event.pull_request.base.sha` and HEAD is GitHub's test merge
commit (PR head merged into **current** main). `base.sha` is the base
tip from when the PR was opened, so on PRs whose base has since moved,
the diff includes every module merged to main after the PR branched.
#1034 was opened July 29; `git-clone` changed on main afterwards
(#1037), so a later synchronize run scored it, and score noise made it
look like a regression.

## Change

- Determine changed modules from `GET
/repos/{owner}/{repo}/pulls/{n}/files` (paginated) instead of git
plumbing. The PR files list is the source of truth for what a PR
changes, regardless of how stale the branch is.
- Drop `fetch-depth: 0` from checkout; it existed only for the diff.
- Guard the `grep` so a PR with no module files doesn't fail the step
under `pipefail`.

## Validation

Against #1034 directly: the files API returns only `windows-rdp` paths,
while the old `base.sha` diff on the same PR spans dozens of unrelated
commits including the `git-clone` change.

🤖 Generated with [Coder Agents](https://coder.com/docs/ai-coder/agents)
DevelopmentCats added a commit that referenced this pull request Aug 18, 2026
## Problem

The password was interpolated into two places without escaping it for
the target syntax.

**1. PowerShell double-quoted string** — `configure-rdp.ps1`

```powershell
$password = "${password}"
```

`$`, backtick, and `"` are all interpreted there. Reproduced on a live
AWS Windows workspace with `password = "Te\st$Pa\"ss'w`ord&<>|1!"`: the
entire `Configure RDP`script aborts before doing anything, and the
workspace reports`start_error\`.

```
The string is missing the terminator: '.
    + CategoryInfo          : ParserError: (:) [], ParentContainsErrorRecordException
    + FullyQualifiedErrorId : TerminatorExpectedAtEndOfString
```

**2. Unencoded URL query parameter** — `main.tf`

The same value went into the `coder://` app URL raw, so the rendered URL
on that workspace was:

```
coder://dev.coder.com/v0/open/ws/reg37-rdp/agent/main/rdp?username=Administrator&password=Te\st$Pa"ss'w`ord&<>|1!
```

Coder Desktop receives everything up to the first `&`. A `#` drops the
remainder, `+` decodes as a space, and a bare `%` is an invalid escape.

## Fix

<!-- linear:table-colwidths:400,400 -->
| Destination | Encoding |
| -- | -- |
| PowerShell script | single-quoted string, `'` doubled |
| App URL query | `urlencode()` on both username and password |

Same approach as #1036 for `amazon-dcv-windows` and
#1034 for `windows-rdp`.

## Changes

* `configure-rdp.ps1`: pass credentials as PowerShell single-quoted
strings
* `main.tf`: `urlencode()` both credentials in the app URL
* add `local-windows-rdp.tftest.hcl` covering a password with ``` \ " '
`` ``` \`\` $ & < > | # % +\`
* update 4 assertions in `main.test.ts` that were pinning the old
behavior, for example `password=CustomPass123!` unencoded

Module bumped to `1.0.5`.

## Validation

The new Terraform tests fail without the source fix and pass with it, so
they are not vacuous:

```
run "plan_with_defaults"...           fail
  condition = strcontains(resource.coder_script.rdp_setup.script, "$password = 'coderRDP!'")
run "plan_with_special_characters"... fail
```

With the fix: `2 passed, 0 failed`, `bun test main.test.ts` 10/10,
prettier and `terraform fmt` clean.

> [!NOTE]
> Third and last module in this bug class, after #20 /
#1034 (`windows-rdp`) and #1035 /
#1036 (`amazon-dcv-windows`). Found while dogfooding
#1034 on a real AWS Windows workspace, where this module's
script was the one that actually crashed.

Closes #1043

> 🤖 This PR was created with the help of Coder Agents, and needs a human
review. 🧑‍💻

---------

Co-authored-by: DevCats <christofer@coder.com>

@35C4n0r 35C4n0r left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Image The JS script works as expected, and the changes in main.tf + script LGTM.

@matifali
matifali enabled auto-merge (squash) August 20, 2026 17:07
@matifali
matifali merged commit 866d682 into main Aug 20, 2026
5 checks passed
@matifali
matifali deleted the fix/windows-rdp-password-escaping branch August 20, 2026 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants