Skip to content

fix(installer): stop granting Users read access to the data folder - #2190

Merged
irvingouj@Devolutions (irvingoujAtDevolution) merged 1 commit into
masterfrom
fix/recordings-acl
Nov 9, 2026
Merged

irvingouj@Devolutions (irvingoujAtDevolution) merged 1 commit into
masterfrom
fix/recordings-acl

Conversation

@irvingoujAtDevolution

@irvingoujAtDevolution irvingouj@Devolutions (irvingoujAtDevolution) commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

The installer gave BUILTIN\Users read access to %ProgramData%\Devolutions\Gateway, and everything in it inherits that, including recordings and server.key. Only SYSTEM, admins and the service account need it, so this drops the Users entry from PROGRAM_DATA_SDDL_TEMPLATE.

Existing installs get fixed on upgrade: install/upgrade/repair already reset the DACL of the folder and everything below it.

Heads-up: non-admins can't read gateway.json or the logs anymore.

Tested

  • New DataFolderTests.OnlySystemAdministratorsAndTheServiceAccountCanReadTheData (red before, green after), full installer unit tests pass.
  • Lab: upgraded a 26.3.0 Gateway with this MSI. A non-admin local user lost read access to existing and new recordings and server.key. The service still writes recordings and serves them through /jet/jrec/pull, and admins can still read/write.

Issue: DGW-504

🤖 Generated with Claude Code

Copilot AI balanced review requested due to automatic review settings November 9, 2026 15:34
@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown

Let maintainers know that an action is required on their side

  • Add the label release-required Please cut a new release (Devolutions Gateway, Devolutions Agent, Jetsocat, PowerShell module) when you request a maintainer to cut a new release (Devolutions Gateway, Devolutions Agent, Jetsocat, PowerShell module)

  • Add the label release-blocker Follow-up is required before cutting a new release if a follow-up is required before cutting a new release

  • Add the label publish-required Please publish libraries (`Devolutions.Gateway.Utils`, OpenAPI clients, etc) when you request a maintainer to publish libraries (Devolutions.Gateway.Utils, OpenAPI clients, etc.)

  • Add the label publish-blocker Follow-up is required before publishing libraries if a follow-up is required before publishing libraries

Copilot AI left a comment

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.

🟢 Approval recommended

No active review issues remain for the data-folder access restriction and its associated regression coverage.

No open issues 🎉

What changed in this PR

Restricts access to Gateway’s data folder to SYSTEM, administrators, and the service account, protecting recordings and private keys from non-admin users.

Changes:

  • Removes the BUILTIN\Users read-access entry from the data-folder security template and updates its documentation.
  • Adds regression coverage checking allowed readers for the data-folder and users-database templates.
File Description
package/​WindowsManaged/​Resources/​Includes.cs Removes Users read access and documents the restricted permissions.
package/​WindowsManaged.UnitTests/​DataFolderTests.cs Verifies that only SYSTEM, administrators, and the service account receive read access.

Review effort: Balanced

The installer gave BUILTIN\Users read access to %ProgramData%\Devolutions\Gateway,
and everything the Gateway stores there inherited it. Only SYSTEM, administrators
and the service account now get access.

Install, upgrade and repair already reset the data folder's DACL and the entries
below it, so existing files lose the entry on upgrade.

Issue: DGW-504

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@irvingoujAtDevolution
irvingouj@Devolutions (irvingoujAtDevolution) merged commit ebbaff8 into master Nov 9, 2026
53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants