From 2e1651b619fb4ebeb66838b24bd5e3f89c118091 Mon Sep 17 00:00:00 2001 From: "G.Reijn" <26114636+Gijsreyn@users.noreply.github.com> Date: Tue, 21 Jul 2026 03:32:00 +0200 Subject: [PATCH 1/2] fix: add support for preserving quoted group names in sshd_config --- resources/sshdconfig/src/get.rs | 69 +++++++++++++++++++ .../sshdconfig/tests/sshdconfig.get.tests.ps1 | 32 +++++++++ 2 files changed, 101 insertions(+) diff --git a/resources/sshdconfig/src/get.rs b/resources/sshdconfig/src/get.rs index b1504b890..70b2ced43 100644 --- a/resources/sshdconfig/src/get.rs +++ b/resources/sshdconfig/src/get.rs @@ -17,6 +17,7 @@ use crate::canonical_properties::CanonicalProperty; use crate::error::SshdConfigError; use crate::inputs::{CommandInfo, SSHD_CONFIG_FILEPATH}; use crate::parser::parse_text_to_map; +use crate::repeat_keyword::MULTI_ARG_KEYWORDS_SPACE_SEP; use crate::util::{ build_command_info, extract_sshd_defaults, @@ -133,6 +134,12 @@ pub fn get_sshd_settings(cmd_info: &CommandInfo, is_get: bool) -> Result Result, explicit_settings: &Map) { + for keyword in MULTI_ARG_KEYWORDS_SPACE_SEP { + if let Some(value) = explicit_settings.get(keyword) { + result.insert((*keyword).to_string(), value.clone()); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + #[test] + fn overrides_split_group_list_with_quoted_file_value() { + // sshd -T stripped the quotes and split "openssh users" into two entries. + let mut result = Map::new(); + result.insert("allowgroups".to_string(), json!(["administrators", "openssh", "users"])); + + // The raw config file parse preserved the quoted grouping. + let mut explicit_settings = Map::new(); + explicit_settings.insert("allowgroups".to_string(), json!(["administrators", "openssh users"])); + + prefer_explicit_space_sep_lists(&mut result, &explicit_settings); + + assert_eq!( + result.get("allowgroups").unwrap(), + &json!(["administrators", "openssh users"]) + ); + } + + #[test] + fn leaves_keyword_absent_from_file_untouched() { + // allowgroups is present in sshd -T output but not explicitly set in the config file. + let mut result = Map::new(); + result.insert("allowgroups".to_string(), json!(["administrators", "openssh", "users"])); + + let explicit_settings = Map::new(); + + prefer_explicit_space_sep_lists(&mut result, &explicit_settings); + + assert_eq!( + result.get("allowgroups").unwrap(), + &json!(["administrators", "openssh", "users"]) + ); + } + + #[test] + fn leaves_non_space_sep_keyword_untouched() { + // port is not a space-separated list keyword and must not be overridden. + let mut result = Map::new(); + result.insert("port".to_string(), json!([22])); + + let mut explicit_settings = Map::new(); + explicit_settings.insert("port".to_string(), json!([2222])); + + prefer_explicit_space_sep_lists(&mut result, &explicit_settings); + + assert_eq!(result.get("port").unwrap(), &json!([22])); + } +} diff --git a/resources/sshdconfig/tests/sshdconfig.get.tests.ps1 b/resources/sshdconfig/tests/sshdconfig.get.tests.ps1 index d2408cc0a..0ca0fb57e 100644 --- a/resources/sshdconfig/tests/sshdconfig.get.tests.ps1 +++ b/resources/sshdconfig/tests/sshdconfig.get.tests.ps1 @@ -41,6 +41,12 @@ PasswordAuthentication no "@ $TestConfigPathWithInclude = Join-Path $TestDrive 'test_sshd_config_include' $configWithInclude | Set-Content -Path $TestConfigPathWithInclude + $configWithQuotedGroups = @" +PasswordAuthentication no +AllowGroups administrators "openssh users" +"@ + $TestConfigPathWithQuotedGroups = Join-Path $TestDrive 'test_sshd_config_quoted_groups' + $configWithQuotedGroups | Set-Content -Path $TestConfigPathWithQuotedGroups } AfterAll { @@ -53,6 +59,9 @@ PasswordAuthentication no if (Test-Path $TestConfigPathWithInclude) { Remove-Item -Path $TestConfigPathWithInclude -Force -ErrorAction SilentlyContinue } + if (Test-Path $TestConfigPathWithQuotedGroups) { + Remove-Item -Path $TestConfigPathWithQuotedGroups -Force -ErrorAction SilentlyContinue + } } It ' command ' -TestCases @( @@ -142,6 +151,29 @@ PasswordAuthentication no Remove-Item -Path $stderrFile -Force -ErrorAction SilentlyContinue } + It ' command preserves quoted group names containing spaces' -TestCases @( + @{ Command = 'get' } + @{ Command = 'export' } + ) { + param($Command) + + $inputData = @{ + sshd_config_filepath = $TestConfigPathWithQuotedGroups + } | ConvertTo-Json + + if ($Command -eq 'get') { + $result = sshdconfig $Command --input $inputData -s sshd-config 2>$null | ConvertFrom-Json + } + else { + $result = sshdconfig $Command --input $inputData 2>$null | ConvertFrom-Json + } + + # "openssh users" must remain a single entry rather than being split into "openssh" and "users". + $result.AllowGroups.Count | Should -Be 2 + $result.AllowGroups[0] | Should -Be "administrators" + $result.AllowGroups[1] | Should -Be "openssh users" + } + It 'Should fail without creating target config when file does not exist' { $nonExistentPath = Join-Path $TestDrive 'nonexistent_sshd_config' From 4f80db678d183249103593438289a65349686c31 Mon Sep 17 00:00:00 2001 From: "G.Reijn" <26114636+Gijsreyn@users.noreply.github.com> Date: Thu, 30 Jul 2026 06:18:16 +0200 Subject: [PATCH 2/2] Update with remarks Tess --- resources/sshdconfig/locales/en-us.toml | 2 +- resources/sshdconfig/src/get.rs | 73 ++++++++++++---- resources/sshdconfig/src/parser.rs | 83 ++++++++++++++++++- .../sshdconfig/tests/sshdconfig.get.tests.ps1 | 35 ++++++++ 4 files changed, 175 insertions(+), 18 deletions(-) diff --git a/resources/sshdconfig/locales/en-us.toml b/resources/sshdconfig/locales/en-us.toml index 5c6901a49..ae79fb88f 100644 --- a/resources/sshdconfig/locales/en-us.toml +++ b/resources/sshdconfig/locales/en-us.toml @@ -51,12 +51,12 @@ schema = "Schema command:" set = "Set command: '%{input}'" [parser] +combinedMultiArgValue = "combined multiple arguments into a single value for keyword '%{keyword}'" failedToParse = "failed to parse: '%{input}'" failedToParseAsArray = "value is not an array" failedToParseNode = "failed to parse '%{input}'" failedToParseRoot = "failed to parse root: '%{input}'" invalidConfig = "invalid config: '%{input}'" -invalidMultiArgNode = "multi-arg node '%{input}' is not valid" keyNotFound = "key '%{key}' not found" keyNotRepeatable = "key '%{key}' is not repeatable" missingCriteriaInMatch = "missing criteria field in match block: '%{input}'" diff --git a/resources/sshdconfig/src/get.rs b/resources/sshdconfig/src/get.rs index 70b2ced43..016aa5785 100644 --- a/resources/sshdconfig/src/get.rs +++ b/resources/sshdconfig/src/get.rs @@ -17,7 +17,6 @@ use crate::canonical_properties::CanonicalProperty; use crate::error::SshdConfigError; use crate::inputs::{CommandInfo, SSHD_CONFIG_FILEPATH}; use crate::parser::parse_text_to_map; -use crate::repeat_keyword::MULTI_ARG_KEYWORDS_SPACE_SEP; use crate::util::{ build_command_info, extract_sshd_defaults, @@ -134,11 +133,11 @@ pub fn get_sshd_settings(cmd_info: &CommandInfo, is_get: bool) -> Result Result, explicit_settings: &Map) { - for keyword in MULTI_ARG_KEYWORDS_SPACE_SEP { - if let Some(value) = explicit_settings.get(keyword) { - result.insert((*keyword).to_string(), value.clone()); +fn prefer_explicit_values_with_spaces(result: &mut Map, explicit_settings: &Map) { + for (keyword, value) in explicit_settings { + if contains_whitespace(value) { + result.insert(keyword.clone(), value.clone()); } } } +/// Whether the value, or any value nested within it, is a string containing whitespace. +fn contains_whitespace(value: &Value) -> bool { + match value { + Value::String(s) => s.contains(char::is_whitespace), + Value::Array(items) => items.iter().any(contains_whitespace), + Value::Object(map) => map.values().any(contains_whitespace), + _ => false + } +} + #[cfg(test)] mod tests { use super::*; @@ -202,7 +211,7 @@ mod tests { let mut explicit_settings = Map::new(); explicit_settings.insert("allowgroups".to_string(), json!(["administrators", "openssh users"])); - prefer_explicit_space_sep_lists(&mut result, &explicit_settings); + prefer_explicit_values_with_spaces(&mut result, &explicit_settings); assert_eq!( result.get("allowgroups").unwrap(), @@ -210,6 +219,38 @@ mod tests { ); } + #[test] + fn overrides_single_value_keyword_with_quoted_file_value() { + let mut result = Map::new(); + result.insert("banner".to_string(), json!("c:\\program files\\ssh\\sample_banner.txt")); + + let mut explicit_settings = Map::new(); + explicit_settings.insert("banner".to_string(), json!("C:\\Program Files\\ssh\\sample_banner.txt")); + + prefer_explicit_values_with_spaces(&mut result, &explicit_settings); + + assert_eq!( + result.get("banner").unwrap(), + &json!("C:\\Program Files\\ssh\\sample_banner.txt") + ); + } + + #[test] + fn overrides_nested_value_with_spaces() { + let mut result = Map::new(); + result.insert("subsystem".to_string(), json!([{"name": "sftp", "value": "c:/program files/openssh/sftp-server.exe"}])); + + let mut explicit_settings = Map::new(); + explicit_settings.insert("subsystem".to_string(), json!([{"name": "sftp", "value": "C:/Program Files/OpenSSH/sftp-server.exe"}])); + + prefer_explicit_values_with_spaces(&mut result, &explicit_settings); + + assert_eq!( + result.get("subsystem").unwrap(), + &json!([{"name": "sftp", "value": "C:/Program Files/OpenSSH/sftp-server.exe"}]) + ); + } + #[test] fn leaves_keyword_absent_from_file_untouched() { // allowgroups is present in sshd -T output but not explicitly set in the config file. @@ -218,7 +259,7 @@ mod tests { let explicit_settings = Map::new(); - prefer_explicit_space_sep_lists(&mut result, &explicit_settings); + prefer_explicit_values_with_spaces(&mut result, &explicit_settings); assert_eq!( result.get("allowgroups").unwrap(), @@ -227,16 +268,18 @@ mod tests { } #[test] - fn leaves_non_space_sep_keyword_untouched() { - // port is not a space-separated list keyword and must not be overridden. + fn leaves_value_without_spaces_untouched() { let mut result = Map::new(); result.insert("port".to_string(), json!([22])); + result.insert("allowgroups".to_string(), json!(["administrators"])); let mut explicit_settings = Map::new(); explicit_settings.insert("port".to_string(), json!([2222])); + explicit_settings.insert("allowgroups".to_string(), json!(["openssh"])); - prefer_explicit_space_sep_lists(&mut result, &explicit_settings); + prefer_explicit_values_with_spaces(&mut result, &explicit_settings); assert_eq!(result.get("port").unwrap(), &json!([22])); + assert_eq!(result.get("allowgroups").unwrap(), &json!(["administrators"])); } } diff --git a/resources/sshdconfig/src/parser.rs b/resources/sshdconfig/src/parser.rs index 3958672c8..cf4e4dd95 100644 --- a/resources/sshdconfig/src/parser.rs +++ b/resources/sshdconfig/src/parser.rs @@ -266,10 +266,11 @@ fn parse_arguments_node(arg_node: tree_sitter::Node, input: &str, input_bytes: & let mut vec: Vec = Vec::new(); let is_vec = keyword_info.is_multi_arg(); - // if there is more than one argument, but a vector is not expected for the keyword, throw an error let children: Vec<_> = arg_node.named_children(&mut cursor).collect(); + if children.len() > 1 && !is_vec { - return Err(SshdConfigError::ParserError(t!("parser.invalidMultiArgNode", input = input).to_string())); + debug!("{}", t!("parser.combinedMultiArgValue", keyword = &keyword_info.name).to_string()); + return Ok(Value::String(combine_args(&children, input, input_bytes)?)); } for node in &children { @@ -333,6 +334,33 @@ fn parse_arguments_node(arg_node: tree_sitter::Node, input: &str, input_bytes: & } } +/// Combine multiple argument nodes into a single string value. +/// +/// The grammar cannot include spaces in a `string` token, so an unquoted value containing spaces is +/// split into several argument nodes. The separator that appeared between two arguments in the +/// source text is preserved, so a comma-separated value is not silently rewritten as a +/// space-separated one. Quote characters sit between the argument nodes and are dropped, matching +/// how a quoted value that parsed as a single argument is handled. +fn combine_args(children: &[tree_sitter::Node], input: &str, input_bytes: &[u8]) -> Result { + let mut combined = String::new(); + let mut previous_end: Option = None; + + for node in children { + if node.is_error() { + return Err(SshdConfigError::ParserError(t!("parser.failedToParseNode", input = input).to_string())); + } + if let Some(end) = previous_end { + let between = input.get(end..node.start_byte()).unwrap_or_default(); + combined.push_str(if between.contains(',') { "," } else { " " }); + } + combined.push_str(node.utf8_text(input_bytes)?.trim()); + previous_end = Some(node.end_byte()); + } + + // Unescape backslashes (sshd -T escapes them on Windows) + Ok(unescape_backslashes(&combined)) +} + /// Parse arguments node for match criteria, always returning an array. /// Match criteria are always comma-separated and should be arrays. fn parse_arguments_node_as_array(arg_node: tree_sitter::Node, input: &str, input_bytes: &[u8], _keyword_info: &KeywordInfo) -> Result { @@ -487,6 +515,57 @@ mod tests { assert_eq!(allowgroups[1], Value::String("developers".to_string())); } + #[test] + fn single_value_keyword_with_spaces() { + // sshd -T prints Banner "C:\Program Files\ssh\sample_banner.txt" without the quotes. + let input = "banner c:\\program files\\ssh\\sample_banner.txt\r\n"; + let result: Map = parse_text_to_map(input).unwrap(); + assert_eq!( + result.get("banner").unwrap(), + &Value::String("c:\\program files\\ssh\\sample_banner.txt".to_string()) + ); + } + + #[test] + fn single_value_keyword_with_quotes_matches_unquoted() { + let quoted = parse_text_to_map("banner \"c:\\program files\\ssh\\sample_banner.txt\"\r\n").unwrap(); + let unquoted = parse_text_to_map("banner c:\\program files\\ssh\\sample_banner.txt\r\n").unwrap(); + assert_eq!(quoted.get("banner").unwrap(), unquoted.get("banner").unwrap()); + } + + #[test] + fn single_value_keyword_preserves_comma_separator() { + // logverbose is not a known multi-arg keyword, but its value is comma-separated. + let input = "logverbose kex.c:*:1,monitor.c:*\r\n"; + let result: Map = parse_text_to_map(input).unwrap(); + assert_eq!( + result.get("logverbose").unwrap(), + &Value::String("kex.c:*:1,monitor.c:*".to_string()) + ); + } + + #[test] + fn single_value_keyword_preserves_mixed_separators() { + let input = "forcecommand /usr/bin/cmd -o a,b -x\r\n"; + let result: Map = parse_text_to_map(input).unwrap(); + assert_eq!( + result.get("forcecommand").unwrap(), + &Value::String("/usr/bin/cmd -o a,b -x".to_string()) + ); + } + + #[test] + fn single_value_keyword_with_spaces_roundtrip() { + use crate::formatter::write_config_map_to_text; + + let input = "banner \"/etc/ssh/sample banner.txt\"\n"; + let parsed = parse_text_to_map(input).unwrap(); + let formatted = write_config_map_to_text(&parsed).unwrap(); + let reparsed = parse_text_to_map(&formatted).unwrap(); + + assert_eq!(parsed.get("banner").unwrap(), reparsed.get("banner").unwrap()); + } + #[test] fn err_multiarg_repeated_keyword() { let input = "hostkeyalgorithms ssh-ed25519-cert-v01@openssh.com\r\n hostkeyalgorithms ecdsa-sha2-nistp256-cert-v01@openssh.com\r\n"; diff --git a/resources/sshdconfig/tests/sshdconfig.get.tests.ps1 b/resources/sshdconfig/tests/sshdconfig.get.tests.ps1 index 0ca0fb57e..b3c7e1edd 100644 --- a/resources/sshdconfig/tests/sshdconfig.get.tests.ps1 +++ b/resources/sshdconfig/tests/sshdconfig.get.tests.ps1 @@ -47,6 +47,14 @@ AllowGroups administrators "openssh users" "@ $TestConfigPathWithQuotedGroups = Join-Path $TestDrive 'test_sshd_config_quoted_groups' $configWithQuotedGroups | Set-Content -Path $TestConfigPathWithQuotedGroups + $BannerPath = Join-Path $TestDrive 'Sample Banner.txt' + 'welcome' | Set-Content -Path $BannerPath + $configWithQuotedPath = @" +PasswordAuthentication no +Banner "$BannerPath" +"@ + $TestConfigPathWithQuotedPath = Join-Path $TestDrive 'test_sshd_config_quoted_path' + $configWithQuotedPath | Set-Content -Path $TestConfigPathWithQuotedPath } AfterAll { @@ -62,6 +70,12 @@ AllowGroups administrators "openssh users" if (Test-Path $TestConfigPathWithQuotedGroups) { Remove-Item -Path $TestConfigPathWithQuotedGroups -Force -ErrorAction SilentlyContinue } + if (Test-Path $TestConfigPathWithQuotedPath) { + Remove-Item -Path $TestConfigPathWithQuotedPath -Force -ErrorAction SilentlyContinue + } + if (Test-Path $BannerPath) { + Remove-Item -Path $BannerPath -Force -ErrorAction SilentlyContinue + } } It ' command ' -TestCases @( @@ -174,6 +188,27 @@ AllowGroups administrators "openssh users" $result.AllowGroups[1] | Should -Be "openssh users" } + It ' command preserves a single-value keyword whose path contains spaces' -TestCases @( + @{ Command = 'get' } + @{ Command = 'export' } + ) { + param($Command) + + $inputData = @{ + sshd_config_filepath = $TestConfigPathWithQuotedPath + } | ConvertTo-Json + + if ($Command -eq 'get') { + $result = sshdconfig $Command --input $inputData -s sshd-config 2>$null | ConvertFrom-Json + } + else { + $result = sshdconfig $Command --input $inputData 2>$null | ConvertFrom-Json + } + + $LASTEXITCODE | Should -Be 0 + $result.Banner | Should -Be $BannerPath + } + It 'Should fail without creating target config when file does not exist' { $nonExistentPath = Join-Path $TestDrive 'nonexistent_sshd_config'