Skip to content

fix: add support for preserving quoted group names in sshd_config - #1639

Open
Gijsreyn wants to merge 2 commits into
PowerShell:mainfrom
Gijsreyn:gh-1508/main/fix-space-sshd
Open

fix: add support for preserving quoted group names in sshd_config#1639
Gijsreyn wants to merge 2 commits into
PowerShell:mainfrom
Gijsreyn:gh-1508/main/fix-space-sshd

Conversation

@Gijsreyn

Copy link
Copy Markdown
Collaborator

PR Summary

Fixes a bug where sshd_config list keywords with quoted, space-containing values (e.g. the Windows group "openssh users") were split into separate entries on export and get.

PR Context

Fixes #1508.

Copilot AI review requested due to automatic review settings July 21, 2026 01:33

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.

Pull request overview

This PR fixes a Windows-specific parsing/export bug in the Microsoft.OpenSSH.SSHD/sshd_config resource where space-containing, quoted list entries (e.g. AllowGroups administrators "openssh users") were incorrectly split into separate items during get and export.

Changes:

  • Added a Pester test that verifies get and export preserve quoted group names containing spaces.
  • Updated get_sshd_settings() to prefer explicitly-parsed config-file values for space-separated multi-arg keywords (avoiding sshd -T quote-stripping/splitting behavior).
  • Added Rust unit tests covering the new override behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
resources/sshdconfig/tests/sshdconfig.get.tests.ps1 Adds coverage ensuring quoted group values remain a single array entry for both get and export.
resources/sshdconfig/src/get.rs Restores correct list semantics by overriding sshd -T-normalized values with explicitly parsed config values for affected keywords.

@Gijsreyn
Gijsreyn force-pushed the gh-1508/main/fix-space-sshd branch from 0faeeaf to 9b7da04 Compare July 21, 2026 03:06
@Gijsreyn

Copy link
Copy Markdown
Collaborator Author

@tgauth - is this something you can take a quick look at?

@tgauth

tgauth commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Thanks for opening this! My initial thoughts are that this issue is applicable to other arguments that can have spaces as well (i.e. regular arguments for paths) and I wonder if we can address those here as well. I'm going to think on it a bit and see if there's a good way to do that, and if not, we can defer it to a future PR.

@Gijsreyn
Gijsreyn force-pushed the gh-1508/main/fix-space-sshd branch from 9b7da04 to b991333 Compare July 22, 2026 04:19
Copilot AI review requested due to automatic review settings July 22, 2026 04:19

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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread resources/sshdconfig/src/get.rs Outdated
@SteveL-MSFT SteveL-MSFT added this to the 3.3-Approved milestone Jul 24, 2026
@michaeltlombardi
michaeltlombardi requested a review from tgauth July 29, 2026 20:29
Comment thread resources/sshdconfig/src/get.rs Outdated
}

fn prefer_explicit_space_sep_lists(result: &mut Map<String, Value>, explicit_settings: &Map<String, Value>) {
for keyword in MULTI_ARG_KEYWORDS_SPACE_SEP {

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.

Ok to cover broader cases, I think there's two changes needed:

  1. Expand this check to be any keyword in explicit_settings that has spaces, rather than only checking for keywords in MULTI_ARG_KEYWORDS_SPACE_SEP
  2. When the Rust parser is parsing keywords from sshd -T, it should not error on "regular, single-value" keywords with a spaced value (i.e. Banner "C:\Program Files\ssh\sample_banner.txt" is output as Banner c:\program files\ssh\sample_banner.txt which seems like Banner has two args "c:\program" and "file\ssh\sample_banner.txt" so the parser errors currently. We should have the parser combine them into a single value)

@Gijsreyn, if you want to leave the PR as-is and then I'll add these changes after, that's fine with me too!

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.

@tgauth - thanks for the feedback! I tried to incorporate it already into this PR. Let me know what you think :)

@Gijsreyn
Gijsreyn force-pushed the gh-1508/main/fix-space-sshd branch from 0161afd to 4f80db6 Compare July 30, 2026 07:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

In Microsoft.OpenSSH.SSHD/sshd_config allowgroups is not processed correctly

4 participants