address review feedback

Author: tgauth

sshdconfig/src/args.rs

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-use clap::{Parser, Subcommand};
+use clap::{Parser, Subcommand, ValueEnum};
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 
@@ -14,7 +14,10 @@ pub struct Args {
 #[derive(Subcommand)]
 pub enum Command {
     /// Get default shell, eventually to be used for `sshd_config` and repeatable keywords
-    Get,
+    Get {
+        #[clap(short = 'r', long, hide = true)]
+        resource: Resource,
+    },
     /// Set default shell, eventually to be used for `sshd_config` and repeatable keywords
     Set {
         #[clap(short = 'i', long, help = "input to set in sshd_config")]
@@ -24,8 +27,8 @@ pub enum Command {
     Export,
     Schema {
         // Used to inform which schema to generate
-        #[clap(long, hide = true)]
-        as_global: bool,
+        #[clap(short = 'r', long, hide = true)]
+        resource: Resource,
     },
 }
 
@@ -36,3 +39,9 @@ pub struct DefaultShell {
     pub escape_arguments: Option<bool>,
     pub shell_arguments: Option<Vec<String>>,
 }
+
+#[derive(Clone, Debug, Eq, PartialEq, ValueEnum)]
+pub enum Resource {
+    DefaultShell,
+    SshdConfig,
+}

sshdconfig/src/error.rs

@@ -13,6 +13,8 @@ pub enum SshdConfigError {
     Json(#[from] serde_json::Error),
     #[error("Language: {0}")]
     LanguageError(#[from] tree_sitter::LanguageError),
+    #[error("Not Implemented: {0}")]
+    NotImplemented(String),
     #[error("Parser: {0}")]
     ParserError(String),
     #[error("Parser Int: {0}")]

sshdconfig/src/export.rs

@@ -13,7 +13,7 @@ use crate::util::invoke_sshd_config_validation;
 pub fn invoke_export() -> Result<(), SshdConfigError> {
     let sshd_config_text = invoke_sshd_config_validation()?;
     let sshd_config: serde_json::Map<String, serde_json::Value> = parse_text_to_map(&sshd_config_text)?;
-    let json = serde_json::to_string_pretty(&sshd_config)?;
+    let json = serde_json::to_string(&sshd_config)?;
     println!("{json}");
     Ok(())
 }

sshdconfig/src/get.rs

@@ -4,7 +4,8 @@
 #[cfg(windows)]
 use {
     registry_lib::{config::{Registry, RegistryValueData}, RegistryHelper},
-    crate::args::DefaultShell
+    crate::args::{DefaultShell, Resource},
+    crate::metadata::{DEFAULT_SHELL, DEFAULT_SHELL_CMD_OPTION, DEFAULT_SHELL_ESCAPE_ARGS, REGISTRY_PATH},
 };
 
 use crate::error::SshdConfigError;
@@ -14,56 +15,58 @@ use crate::error::SshdConfigError;
 /// # Errors
 ///
 /// This function will return an error if the desired settings cannot be retrieved.
-pub fn invoke_get() -> Result<(), SshdConfigError> {
-    // TODO: distinguish between get commands for default shell, repeatable keywords, and sshd_config
-    get_default_shell()?;
-    Ok(())
+pub fn invoke_get(resource: &Resource) -> Result<(), SshdConfigError> {
+    match resource {
+        &Resource::DefaultShell => get_default_shell(),
+        &Resource::SshdConfig => Err(SshdConfigError::NotImplemented("get not yet implemented for Microsoft.OpenSSH.SSHD/sshd_config".to_string())),
+    }
 }
 
 #[cfg(windows)]
 fn get_default_shell() -> Result<(), SshdConfigError> {
-    let registry_helper = RegistryHelper::new("HKLM\\SOFTWARE\\OpenSSH", Some("DefaultShell".to_string()), None)?;
+    let registry_helper = RegistryHelper::new(REGISTRY_PATH, Some(DEFAULT_SHELL.to_string()), None)?;
     let default_shell: Registry = registry_helper.get()?;
     let mut shell = None;
     let mut shell_arguments = None;
+    // default_shell is a single string consisting of the shell exe path and, optionally, any arguments
     if let Some(value) = default_shell.value_data {
         match value {
             RegistryValueData::String(s) => {
                 let parts: Vec<&str> = s.split_whitespace().collect();
                 if parts.is_empty() {
-                    return Err(SshdConfigError::InvalidInput("DefaultShell cannot be empty".to_string()));
+                    return Err(SshdConfigError::InvalidInput(format!("{} cannot be empty", DEFAULT_SHELL)));
                 }
                 shell = Some(parts[0].to_string());
                 if parts.len() > 1 {
                     shell_arguments = Some(parts[1..].iter().map(|&s| s.to_string()).collect());
                 }
             }
-            _ => return Err(SshdConfigError::InvalidInput("DefaultShell must be a string".to_string())),
+            _ => return Err(SshdConfigError::InvalidInput(format!("{} must be a string", DEFAULT_SHELL))),
         }
     }
 
-    let registry_helper = RegistryHelper::new("HKLM\\SOFTWARE\\OpenSSH", Some("DefaultShellCommandOption".to_string()), None)?;
+    let registry_helper = RegistryHelper::new(REGISTRY_PATH, Some(DEFAULT_SHELL_CMD_OPTION.to_string()), None)?;
     let option: Registry = registry_helper.get()?;
     let mut cmd_option = None;
     if let Some(value) = option.value_data {
         match value {
             RegistryValueData::String(s) => cmd_option = Some(s),
-            _ => return Err(SshdConfigError::InvalidInput("DefaultShellCommandOption must be a string".to_string())),
+            _ => return Err(SshdConfigError::InvalidInput(format!("{} must be a string", DEFAULT_SHELL_CMD_OPTION))),
         }
     }
 
-    let registry_helper = RegistryHelper::new("HKLM\\SOFTWARE\\OpenSSH", Some("DefaultShellEscapeArguments".to_string()), None)?;
+    let registry_helper = RegistryHelper::new(REGISTRY_PATH, Some(DEFAULT_SHELL_ESCAPE_ARGS.to_string()), None)?;
     let escape_args: Registry = registry_helper.get()?;
     let mut escape_arguments = None;
     if let Some(value) = escape_args.value_data {
         if let RegistryValueData::DWord(b) = value {
             if b == 0 || b == 1 {
                 escape_arguments = if b == 1 { Some(true) } else { Some(false) };
             } else {
-                return Err(SshdConfigError::InvalidInput("DefaultShellEscapeArguments must be a boolean".to_string()));
+                return Err(SshdConfigError::InvalidInput(format!("{} must be a 0 or 1", DEFAULT_SHELL_ESCAPE_ARGS)));
             }
         } else {
-            return Err(SshdConfigError::InvalidInput("DefaultShellEscapeArguments must be a boolean".to_string()));
+            return Err(SshdConfigError::InvalidInput(format!("{} must be a DWord", DEFAULT_SHELL_ESCAPE_ARGS)));
         }
     }
 
@@ -74,12 +77,12 @@ fn get_default_shell() -> Result<(), SshdConfigError> {
         shell_arguments
     };
 
-    let output = serde_json::to_string_pretty(&result)?;
+    let output = serde_json::to_string(&result)?;
     println!("{output}");
     Ok(())
 }
 
 #[cfg(not(windows))]
 pub fn get_default_shell() -> Result<(), SshdConfigError> {
-    Err(SshdConfigError::InvalidInput("Windows registry operations not applicable to this platform".to_string()))
+    Err(SshdConfigError::InvalidInput("Microsoft.OpenSSH.SSHD/Windows is only applicable to Windows".to_string()))
 }

sshdconfig/src/main.rs

@@ -4,7 +4,7 @@
 use clap::{Parser};
 use schemars::schema_for;
 
-use args::{Args, Command, DefaultShell};
+use args::{Args, Command, DefaultShell, Resource};
 use export::invoke_export;
 use get::invoke_get;
 use parser::SshdConfigParser;
@@ -24,15 +24,18 @@ fn main() {
 
     let result = match &args.command {
         Command::Export => invoke_export(),
-        Command::Get => invoke_get(),
+        Command::Get { resource } => invoke_get(resource),
         Command::Set { input } => invoke_set(input),
-        Command::Schema { as_global } => {
-            let schema = if *as_global {
-                schema_for!(DefaultShell)
-            } else {
-                schema_for!(SshdConfigParser)
+        Command::Schema { resource } => {
+            let schema = match resource {
+                Resource::DefaultShell => {
+                    schema_for!(DefaultShell)
+                }
+                Resource::SshdConfig => {
+                    schema_for!(SshdConfigParser)
+                }
             };
-            println!("{}", serde_json::to_string_pretty(&schema).unwrap());
+            println!("{}", serde_json::to_string(&schema).unwrap());
             Ok(())
         }
     };

sshdconfig/src/metadata.rs

@@ -36,3 +36,8 @@ pub const MULTI_ARG_KEYWORDS: [&str; 7] = [
     "macs",
     "pubkeyacceptedalgorithms"
 ];
+
+pub const REGISTRY_PATH: &str = "HKLM\\SOFTWARE\\OpenSSH";
+pub const DEFAULT_SHELL: &str = "DefaultShell";
+pub const DEFAULT_SHELL_CMD_OPTION: &str = "DefaultShellCommandOption";
+pub const DEFAULT_SHELL_ESCAPE_ARGS: &str = "DefaultShellEscapeArguments";

sshdconfig/src/set.rs

@@ -4,7 +4,8 @@
 #[cfg(windows)]
 use {
     std::path::Path,
-    registry_lib::{config::RegistryValueData, RegistryHelper}
+    registry_lib::{config::RegistryValueData, RegistryHelper},
+    crate::metadata::{DEFAULT_SHELL, DEFAULT_SHELL_CMD_OPTION, DEFAULT_SHELL_ESCAPE_ARGS, REGISTRY_PATH},
 };
 
 use crate::args::DefaultShell;
@@ -44,46 +45,46 @@ fn set_default_shell(shell: Option<String>, cmd_option: Option<String>, escape_a
             shell_data = format!("{shell} {args_str}");
         }
 
-        set_registry("DefaultShell", RegistryValueData::String(shell_data))?;
+        set_registry(DEFAULT_SHELL, RegistryValueData::String(shell_data))?;
     } else {
-        remove_registry("DefaultShell")?;
+        remove_registry(DEFAULT_SHELL)?;
     }
 
 
     if let Some(cmd_option) = cmd_option {
-        set_registry("DefaultShellCommandOption", RegistryValueData::String(cmd_option.clone()))?;
+        set_registry(DEFAULT_SHELL_CMD_OPTION, RegistryValueData::String(cmd_option.clone()))?;
     } else {
-        remove_registry("DefaultShellCommandOption")?;
+        remove_registry(DEFAULT_SHELL_CMD_OPTION)?;
     }
 
     if let Some(escape_args) = escape_arguments {
         let mut escape_data = 0;
         if escape_args {
             escape_data = 1;
         }
-        set_registry("DefaultShellEscapeArguments", RegistryValueData::DWord(escape_data))?;
+        set_registry(DEFAULT_SHELL_ESCAPE_ARGS, RegistryValueData::DWord(escape_data))?;
     } else {
-        remove_registry("DefaultShellEscapeArguments")?;
+        remove_registry(DEFAULT_SHELL_ESCAPE_ARGS)?;
     }
 
     Ok(())
 }
 
 #[cfg(not(windows))]
 pub fn set_default_shell(_shell: Option<String>, _cmd_option: Option<String>, _escape_arguments: Option<bool>, _shell_arguments: Option<Vec<String>>) -> Result<(), SshdConfigError> {
-    Err(SshdConfigError::InvalidInput("Windows registry operations not applicable to this platform".to_string()))
+    Err(SshdConfigError::InvalidInput("Microsoft.OpenSSH.SSHD/Windows is only applicable to Windows".to_string()))
 }
 
 #[cfg(windows)]
 fn set_registry(name: &str, data: RegistryValueData) -> Result<(), SshdConfigError> {
-    let registry_helper = RegistryHelper::new("HKLM\\SOFTWARE\\OpenSSH", Some(name.to_string()), Some(data))?;
+    let registry_helper = RegistryHelper::new(REGISTRY_PATH, Some(name.to_string()), Some(data))?;
     registry_helper.set()?;
     Ok(())
 }
 
 #[cfg(windows)]
 fn remove_registry(name: &str) -> Result<(), SshdConfigError> {
-    let registry_helper = RegistryHelper::new("HKLM\\SOFTWARE\\OpenSSH", Some(name.to_string()), None)?;
+    let registry_helper = RegistryHelper::new(REGISTRY_PATH, Some(name.to_string()), None)?;
     registry_helper.remove()?;
     Ok(())
 }

sshdconfig/sshd.dsc.resource.json

@@ -1,6 +1,6 @@
 {
     "$schema": "https://aka.ms/dsc/schemas/v3/bundled/resource/manifest.json",
-    "type": "Microsoft.Windows.OpenSSH/SSHD",
+    "type": "Microsoft.OpenSSH.SSHD/Windows",
     "description": "Manage SSH Server Configuration Global Settings",
     "tags": [
         "Windows"
@@ -9,7 +9,9 @@
     "get": {
         "executable": "sshdconfig",
         "args": [
-            "get"
+            "get",
+            "-r",
+            "default-shell"
         ]
     },
     "set": {
@@ -27,7 +29,8 @@
             "executable": "sshdconfig",
             "args": [
                 "schema",
-                "--as-global"
+                "-r",
+                "default-shell"
             ]
         }
     }

sshdconfig/sshd_config.dsc.resource.json

@@ -13,7 +13,9 @@
         "command": {
             "executable": "sshdconfig",
             "args": [
-                "schema"
+                "schema",
+                "-r",
+                "sshd-config"
             ]
         }
     }