diff --git a/.editorconfig b/.editorconfig index 108a8200..0962e7ad 100644 --- a/.editorconfig +++ b/.editorconfig @@ -96,6 +96,21 @@ dotnet_style_prefer_simplified_interpolation = true:suggestion # CA1822: Mark members as static dotnet_diagnostic.CA1822.severity = silent dotnet_style_namespace_match_folder = true:error +dotnet_style_predefined_type_for_locals_parameters_members = true:silent +dotnet_style_readonly_field = true:suggestion +dotnet_code_quality_unused_parameters = all:suggestion +dotnet_style_allow_statement_immediately_after_block_experimental = true:silent +dotnet_style_allow_multiple_blank_lines_experimental = true:silent +dotnet_style_require_accessibility_modifiers = for_non_interface_members:silent +dotnet_style_predefined_type_for_member_access = true:silent +dotnet_style_parentheses_in_other_operators = never_if_unnecessary:silent +dotnet_style_parentheses_in_relational_binary_operators = always_for_clarity:silent +dotnet_style_parentheses_in_other_binary_operators = always_for_clarity:silent +dotnet_style_parentheses_in_arithmetic_binary_operators = always_for_clarity:silent +dotnet_style_qualification_for_event = false:silent +dotnet_style_qualification_for_method = false:silent +dotnet_style_qualification_for_property = false:silent +dotnet_style_qualification_for_field = false:silent [*.cs] csharp_indent_labels = one_less_than_current csharp_using_directive_placement = outside_namespace:silent @@ -112,4 +127,43 @@ csharp_style_expression_bodied_indexers = true:silent csharp_style_expression_bodied_accessors = true:silent csharp_style_expression_bodied_lambdas = true:silent csharp_style_expression_bodied_local_functions = false:silent -csharp_prefer_static_local_function = true:suggestion \ No newline at end of file +csharp_prefer_static_local_function = true:suggestion +csharp_style_throw_expression = true:suggestion +csharp_style_unused_value_expression_statement_preference = discard_variable:silent +csharp_style_unused_value_assignment_preference = discard_variable:suggestion +csharp_style_deconstructed_variable_declaration = true:suggestion +csharp_style_inlined_variable_declaration = true:suggestion +csharp_style_prefer_utf8_string_literals = true:suggestion +csharp_style_prefer_tuple_swap = true:suggestion +csharp_style_implicit_object_creation_when_type_is_apparent = true:suggestion +csharp_style_prefer_range_operator = true:suggestion +csharp_style_prefer_index_operator = true:suggestion +csharp_style_prefer_local_over_anonymous_function = true:suggestion +csharp_prefer_simple_default_expression = true:suggestion +csharp_style_prefer_null_check_over_type_check = true:suggestion +csharp_style_conditional_delegate_call = true:suggestion +csharp_style_allow_blank_line_after_token_in_arrow_expression_clause_experimental = true:silent +csharp_style_allow_blank_line_after_token_in_conditional_expression_experimental = true:silent +csharp_style_allow_blank_line_after_colon_in_constructor_initializer_experimental = true:silent +csharp_style_allow_blank_lines_between_consecutive_braces_experimental = true:silent +csharp_style_allow_embedded_statements_on_same_line_experimental = true:silent +csharp_style_prefer_readonly_struct_member = true:suggestion +csharp_style_prefer_readonly_struct = true:suggestion +csharp_style_prefer_pattern_matching = true:silent +csharp_style_prefer_switch_expression = true:suggestion +csharp_style_pattern_matching_over_is_with_cast_check = true:suggestion +csharp_style_prefer_extended_property_pattern = true:suggestion +csharp_style_prefer_not_pattern = true:suggestion +csharp_style_pattern_matching_over_as_with_null_check = true:suggestion +csharp_style_var_elsewhere = false:silent +csharp_style_var_when_type_is_apparent = false:silent +csharp_style_var_for_built_in_types = false:silent + +# CS8604: Possible null reference argument. +dotnet_diagnostic.CS8604.severity = error + +# CS8600: Converting null literal or possible null value to non-nullable type. +dotnet_diagnostic.CS8600.severity = error + +# CS8602: Dereference of a possibly null reference. +dotnet_diagnostic.CS8602.severity = error diff --git a/Agent.Installer.Win/Agent.Installer.Win.csproj b/Agent.Installer.Win/Agent.Installer.Win.csproj index cc763414..3bfc656c 100644 --- a/Agent.Installer.Win/Agent.Installer.Win.csproj +++ b/Agent.Installer.Win/Agent.Installer.Win.csproj @@ -15,8 +15,8 @@ true true - 10 - enable + 10 + enable win;win-x64;win10-x64;win-x64;win10-x86; diff --git a/Agent.Installer.Win/ViewModels/MainWindowViewModel.cs b/Agent.Installer.Win/ViewModels/MainWindowViewModel.cs index 5b04f78d..c4977af8 100644 --- a/Agent.Installer.Win/ViewModels/MainWindowViewModel.cs +++ b/Agent.Installer.Win/ViewModels/MainWindowViewModel.cs @@ -392,7 +392,7 @@ public class MainWindowViewModel : ViewModelBase return; } - var embeddedData = await _embeddedDataReader.TryGetEmbeddedData(filePath); + var embeddedData = await _embeddedDataReader.TryGetEmbeddedData(filePath!); if (embeddedData is null || embeddedData == EmbeddedServerData.Empty) { diff --git a/Agent/Interfaces/IScriptingShell.cs b/Agent/Interfaces/IScriptingShell.cs new file mode 100644 index 00000000..267c5ff6 --- /dev/null +++ b/Agent/Interfaces/IScriptingShell.cs @@ -0,0 +1,11 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Remotely.Agent.Interfaces; +public interface IScriptingShell +{ + bool IsDisposed { get; } +} diff --git a/Agent/Program.cs b/Agent/Program.cs index d6a8b3eb..027bd27e 100644 --- a/Agent/Program.cs +++ b/Agent/Program.cs @@ -23,9 +23,6 @@ namespace Remotely.Agent; public class Program { - [Obsolete("Remove this when all services are in DI behind interfaces.")] - public static IServiceProvider? Services { get; set; } - public static async Task Main(string[] args) { try @@ -39,8 +36,6 @@ public class Program await host.StartAsync(); - Services = host.Services; - await Init(host.Services); await host.WaitForShutdownAsync(); @@ -105,6 +100,7 @@ public class Program services.AddScoped(); services.AddScoped(); services.AddSingleton(); + services.AddSingleton(); if (OperatingSystem.IsWindows()) { diff --git a/Agent/Services/AgentHubConnection.cs b/Agent/Services/AgentHubConnection.cs index 0aad376c..39d81a61 100644 --- a/Agent/Services/AgentHubConnection.cs +++ b/Agent/Services/AgentHubConnection.cs @@ -42,6 +42,7 @@ public class AgentHubConnection : IAgentHubConnection, IDisposable private readonly ILogger _logger; private readonly IFileLogsManager _fileLogsManager; private readonly IHostApplicationLifetime _appLifetime; + private readonly IScriptingShellFactory _scriptingShellFactory; private readonly IScriptExecutor _scriptExecutor; private readonly IUninstaller _uninstaller; private readonly IUpdater _updater; @@ -63,6 +64,7 @@ public class AgentHubConnection : IAgentHubConnection, IDisposable IWakeOnLanService wakeOnLanService, IFileLogsManager fileLogsManager, IHostApplicationLifetime appLifetime, + IScriptingShellFactory scriptingShellFactory, ILogger logger) { _configService = configService; @@ -77,6 +79,7 @@ public class AgentHubConnection : IAgentHubConnection, IDisposable _logger = logger; _fileLogsManager = fileLogsManager; _appLifetime = appLifetime; + _scriptingShellFactory = scriptingShellFactory; } public bool IsConnected => _hubConnection?.State == HubConnectionState.Connected; @@ -395,7 +398,7 @@ public class AgentHubConnection : IAgentHubConnection, IDisposable { try { - var session = PsCoreShell.GetCurrent(senderConnectionId); + var session = _scriptingShellFactory.GetOrCreatePsCoreShell(senderConnectionId); var completion = session.GetCompletions(inputText, currentIndex, forward); var completionModel = completion.ToPwshCompletion(); await _hubConnection.InvokeAsync("ReturnPowerShellCompletions", completionModel, intent, senderConnectionId).ConfigureAwait(false); diff --git a/Agent/Services/ExternalScriptingShell.cs b/Agent/Services/ExternalScriptingShell.cs index abad72a4..d65f4da7 100644 --- a/Agent/Services/ExternalScriptingShell.cs +++ b/Agent/Services/ExternalScriptingShell.cs @@ -1,9 +1,7 @@ -using Immense.RemoteControl.Shared.Extensions; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging; +using Remotely.Agent.Interfaces; using Remotely.Shared.Dtos; using Remotely.Shared.Enums; -using Remotely.Shared.Models; using System; using System.Collections.Concurrent; using System.Diagnostics; @@ -14,7 +12,7 @@ using System.Timers; namespace Remotely.Agent.Services; -public interface IExternalScriptingShell +public interface IExternalScriptingShell : IDisposable, IScriptingShell { Process? ShellProcess { get; } Task Init(ScriptingShell shell, string shellProcessName, string lineEnding, string connectionId); @@ -28,6 +26,7 @@ public class ExternalScriptingShell : IExternalScriptingShell private readonly ILogger _logger; private readonly ManualResetEvent _outputDone = new(false); private readonly SemaphoreSlim _writeLock = new(1, 1); + private bool _disposedValue; private string _errorOut = string.Empty; private string _lastInputID = string.Empty; private string _lineEnding = Environment.NewLine; @@ -39,7 +38,6 @@ public class ExternalScriptingShell : IExternalScriptingShell private string? _senderConnectionId; private ScriptingShell _shell; private string _standardOut = string.Empty; - public ExternalScriptingShell( IConfigService configService, ILogger logger) @@ -48,38 +46,14 @@ public class ExternalScriptingShell : IExternalScriptingShell _logger = logger; } + public bool IsDisposed => _disposedValue; + public Process? ShellProcess { get; private set; } - - // TODO: Turn into cache and factory. - public static async Task GetCurrent(ScriptingShell shell, string senderConnectionId) + public void Dispose() { - if (_sessions.TryGetValue($"{shell}-{senderConnectionId}", out var session) && - session.ShellProcess?.HasExited != true) - { - return session; - } - else - { - session = Program.Services.GetRequiredService(); - - switch (shell) - { - case ScriptingShell.WinPS: - await session.Init(shell, "powershell.exe", "\r\n", senderConnectionId); - break; - case ScriptingShell.Bash: - await session.Init(shell, "bash", "\n", senderConnectionId); - break; - case ScriptingShell.CMD: - await session.Init(shell, "cmd.exe", "\r\n", senderConnectionId); - break; - default: - throw new ArgumentException($"Unknown external scripting shell type: {shell}"); - } - _sessions.AddOrUpdate($"{shell}-{senderConnectionId}", session, (id, b) => session); - return session; - } + Dispose(disposing: true); + GC.SuppressFinalize(this); } public async Task Init(ScriptingShell shell, string shellProcessName, string lineEnding, string connectionId) @@ -187,6 +161,19 @@ public class ExternalScriptingShell : IExternalScriptingShell return GeneratePartialResult(input, sw.Elapsed); } + protected virtual void Dispose(bool disposing) + { + if (!_disposedValue) + { + if (disposing) + { + ShellProcess?.Dispose(); + } + + _disposedValue = true; + } + } + private ScriptResultDto GenerateCompletedResult(string input, TimeSpan runtime) { return new ScriptResultDto() diff --git a/Agent/Services/PsCoreShell.cs b/Agent/Services/PsCoreShell.cs index 4e264f58..4b255579 100644 --- a/Agent/Services/PsCoreShell.cs +++ b/Agent/Services/PsCoreShell.cs @@ -1,5 +1,6 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Remotely.Agent.Interfaces; using Remotely.Shared.Dtos; using Remotely.Shared.Models; using System; @@ -11,7 +12,7 @@ using System.Threading.Tasks; namespace Remotely.Agent.Services; -public interface IPsCoreShell +public interface IPsCoreShell : IDisposable, IScriptingShell { string? SenderConnectionId { get; set; } @@ -21,11 +22,11 @@ public interface IPsCoreShell public class PsCoreShell : IPsCoreShell { - private static readonly ConcurrentDictionary _sessions = new(); private readonly IConfigService _configService; private readonly ConnectionInfo _connectionInfo; private readonly ILogger _logger; private readonly PowerShell _powershell; + private bool _disposedValue; private CommandCompletion? _lastCompletion; private string? _lastInputText; @@ -49,22 +50,13 @@ public class PsCoreShell : IPsCoreShell _powershell.Invoke(); } + public bool IsDisposed => _disposedValue; public string? SenderConnectionId { get; set; } - // TODO: Turn into cache and factory. - public static IPsCoreShell GetCurrent(string senderConnectionId) + public void Dispose() { - if (_sessions.TryGetValue(senderConnectionId, out var session)) - { - return session; - } - else - { - session = Program.Services.GetRequiredService(); - session.SenderConnectionId = senderConnectionId; - _sessions.AddOrUpdate(senderConnectionId, session, (id, b) => session); - return session; - } + Dispose(disposing: true); + GC.SuppressFinalize(this); } public CommandCompletion GetCompletions(string inputText, int currentIndex, bool? forward) @@ -150,4 +142,16 @@ public class PsCoreShell : IPsCoreShell }; } } + + protected virtual void Dispose(bool disposing) + { + if (!_disposedValue) + { + if (disposing) + { + _powershell.Dispose(); + } + _disposedValue = true; + } + } } diff --git a/Agent/Services/ScriptExecutor.cs b/Agent/Services/ScriptExecutor.cs index ee470e71..cecdeb78 100644 --- a/Agent/Services/ScriptExecutor.cs +++ b/Agent/Services/ScriptExecutor.cs @@ -25,11 +25,16 @@ public interface IScriptExecutor public class ScriptExecutor : IScriptExecutor { private readonly IConfigService _configService; + private readonly IScriptingShellFactory _scriptingShellFactory; private readonly ILogger _logger; - public ScriptExecutor(IConfigService configService, ILogger logger) + public ScriptExecutor( + IConfigService configService, + IScriptingShellFactory scriptingShellFactory, + ILogger logger) { _configService = configService; + _scriptingShellFactory = scriptingShellFactory; _logger = logger; } @@ -161,15 +166,15 @@ public class ScriptExecutor : IScriptExecutor switch (shell) { case ScriptingShell.PSCore: - return await PsCoreShell - .GetCurrent(terminalSessionId) + return await _scriptingShellFactory + .GetOrCreatePsCoreShell(terminalSessionId) .WriteInput(command); case ScriptingShell.WinPS: if (EnvironmentHelper.IsWindows) { - var instance = await ExternalScriptingShell - .GetCurrent(ScriptingShell.WinPS, terminalSessionId); + var instance = await _scriptingShellFactory + .GetOrCreateExternalShell(ScriptingShell.WinPS, terminalSessionId); return await instance.WriteInput(command, timeout); } @@ -178,8 +183,8 @@ public class ScriptExecutor : IScriptExecutor case ScriptingShell.CMD: if (EnvironmentHelper.IsWindows) { - var instance = await ExternalScriptingShell - .GetCurrent(ScriptingShell.CMD, terminalSessionId); + var instance = await _scriptingShellFactory + .GetOrCreateExternalShell(ScriptingShell.CMD, terminalSessionId); return await instance.WriteInput(command, timeout); } @@ -187,8 +192,8 @@ public class ScriptExecutor : IScriptExecutor case ScriptingShell.Bash: { - var instance = await ExternalScriptingShell - .GetCurrent(ScriptingShell.Bash, terminalSessionId); + var instance = await _scriptingShellFactory + .GetOrCreateExternalShell(ScriptingShell.Bash, terminalSessionId); return await instance.WriteInput(command, timeout); } diff --git a/Agent/Services/ScriptingShellFactory.cs b/Agent/Services/ScriptingShellFactory.cs new file mode 100644 index 00000000..a80adb9e --- /dev/null +++ b/Agent/Services/ScriptingShellFactory.cs @@ -0,0 +1,94 @@ +using Microsoft.Extensions.Caching.Memory; +using Microsoft.Extensions.DependencyInjection; +using Remotely.Shared.Enums; +using System; +using System.Threading.Tasks; + +namespace Remotely.Agent.Services; + +public interface IScriptingShellFactory +{ + Task GetOrCreateExternalShell(ScriptingShell shell, string senderConnectionId); + IPsCoreShell GetOrCreatePsCoreShell(string senderConnectionId); +} + +internal class ScriptingShellFactory : IScriptingShellFactory +{ + private readonly MemoryCache _sessionCache = new(new MemoryCacheOptions()); + private readonly IServiceProvider _serviceProvider; + + public ScriptingShellFactory(IServiceProvider serviceProvider) + { + _serviceProvider = serviceProvider; + } + public IPsCoreShell GetOrCreatePsCoreShell(string senderConnectionId) + { + if (_sessionCache.TryGetValue(senderConnectionId, out var result) && + result is IPsCoreShell shell && + !shell.IsDisposed) + { + return shell; + } + + shell = _serviceProvider.GetRequiredService(); + shell.SenderConnectionId = senderConnectionId; + _sessionCache.Set(senderConnectionId, shell, GetEntryOptions()); + return shell; + } + + public async Task GetOrCreateExternalShell( + ScriptingShell shell, + string senderConnectionId) + { + if (_sessionCache.TryGetValue($"{shell}-{senderConnectionId}", out var result) && + result is IExternalScriptingShell scriptingShell && + !scriptingShell.IsDisposed) + { + return scriptingShell; + } + + var session = _serviceProvider.GetRequiredService(); + + switch (shell) + { + case ScriptingShell.WinPS: + await session.Init(shell, "powershell.exe", "\r\n", senderConnectionId); + break; + case ScriptingShell.Bash: + await session.Init(shell, "bash", "\n", senderConnectionId); + break; + case ScriptingShell.CMD: + await session.Init(shell, "cmd.exe", "\r\n", senderConnectionId); + break; + default: + throw new ArgumentException($"Unknown external scripting shell type: {shell}"); + } + + _sessionCache.Set($"{shell}-{senderConnectionId}", session, GetEntryOptions()); + return session; + } + + private MemoryCacheEntryOptions GetEntryOptions() + { + var options = new MemoryCacheEntryOptions() + { + SlidingExpiration = TimeSpan.FromMinutes(10), + }; + options.PostEvictionCallbacks.Add(new PostEvictionCallbackRegistration() + { + EvictionCallback = (key, value, reason, state) => + { + if (value is IDisposable disposable) + { + try + { + disposable.Dispose(); + } + catch { } + } + } + }); + + return options; + } +} diff --git a/Shared/Utilities/Logger.cs b/Shared/Utilities/Logger.cs deleted file mode 100644 index b56087f3..00000000 --- a/Shared/Utilities/Logger.cs +++ /dev/null @@ -1,172 +0,0 @@ -using Remotely.Shared.Enums; -using Remotely.Shared.Services; -using Remotely.Shared.Utilities; -using System; -using System.Diagnostics; -using System.IO; -using System.Linq; -using System.Runtime.CompilerServices; -using System.Runtime.InteropServices; -using System.Threading; -using System.Threading.Tasks; - -namespace Remotely.Shared.Utilities; - -[Obsolete("Please use ILogger via dependency injection.")] -public static class Logger -{ - private static string? _logDir; - - private static string LogDir - { - get - { - if (!string.IsNullOrWhiteSpace(_logDir)) - { - return _logDir; - } - - if (OperatingSystem.IsWindows()) - { - _logDir = Directory.CreateDirectory(Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData), "Remotely", "Logs")).FullName; - } - else - { - _logDir = Directory.CreateDirectory("/var/log/remotely").FullName; - } - return _logDir; - } - } - - private static string LogPath => Path.Combine(LogDir, $"LogFile_{DateTime.Now:yyyy-MM-dd}.log"); - private static SemaphoreSlim WriteLock { get; } = new(1, 1); - public static void Debug(string message, [CallerMemberName] string callerName = "") - { - try - { - WriteLock.Wait(); - - if (EnvironmentHelper.IsDebug) - { - CheckLogFileExists(); - - File.AppendAllText(LogPath, $"{DateTimeOffset.Now:yyyy-MM-dd HH:mm:ss.fff}\t[Debug]\t[{callerName}]\t{message}{Environment.NewLine}"); - } - - System.Diagnostics.Debug.WriteLine(message); - } - catch { } - finally - { - WriteLock.Release(); - } - } - - public static void DeleteLogs() - { - try - { - WriteLock.Wait(); - - if (File.Exists(LogPath)) - { - File.Delete(LogPath); - } - } - catch { } - finally - { - WriteLock.Release(); - } - } - - public static async Task ReadAllLogs() - { - try - { - WriteLock.Wait(); - - CheckLogFileExists(); - - return await File.ReadAllBytesAsync(LogPath); - } - catch (Exception ex) - { - Write(ex); - return Array.Empty(); - } - finally - { - WriteLock.Release(); - } - } - - public static void Write(string message, EventType eventType = EventType.Info, [CallerMemberName] string callerName = "") - { - try - { - WriteLock.Wait(); - - CheckLogFileExists(); - File.AppendAllText(LogPath, $"{DateTimeOffset.Now:yyyy-MM-dd HH:mm:ss.fff}\t[{eventType}]\t[{callerName}]\t{message}{Environment.NewLine}"); - Console.WriteLine(message); - } - catch { } - finally - { - WriteLock.Release(); - } - } - - public static void Write(Exception ex, EventType eventType = EventType.Error, [CallerMemberName] string callerName = "") - { - try - { - WriteLock.Wait(); - - CheckLogFileExists(); - - var exception = ex; - - while (exception != null) - { - File.AppendAllText(LogPath, $"{DateTimeOffset.Now:yyyy-MM-dd HH:mm:ss.fff}\t[{eventType}]\t[{callerName}]\t{exception.Message}\t{exception.StackTrace}\t{exception.Source}{Environment.NewLine}"); - Console.WriteLine(exception.Message); - exception = exception.InnerException; - } - } - catch { } - finally - { - WriteLock.Release(); - } - } - - public static void Write(Exception ex, string message, EventType eventType = EventType.Error, [CallerMemberName] string callerName = "") - { - Write(message, eventType, callerName); - Write(ex, eventType, callerName); - } - - private static void CheckLogFileExists() - { - if (!File.Exists(LogPath)) - { - File.Create(LogPath).Close(); - if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) - { - Process.Start("sudo", $"chmod 775 {LogPath}").WaitForExit(); - } - } - if (File.Exists(LogPath)) - { - var fi = new FileInfo(LogPath); - while (fi.Length > 1000000) - { - var content = File.ReadAllLines(LogPath); - File.WriteAllLines(LogPath, content.Skip(10)); - fi = new FileInfo(LogPath); - } - } - } -} diff --git a/Shared/Utilities/PasswordGenerator.cs b/Shared/Utilities/PasswordGenerator.cs deleted file mode 100644 index 372682a0..00000000 --- a/Shared/Utilities/PasswordGenerator.cs +++ /dev/null @@ -1,15 +0,0 @@ -using System; -using System.Security.Cryptography; - -namespace Remotely.Shared.Utilities; - -public static class PasswordGenerator -{ - public static string GeneratePassword(int size) - { - var buffer = new byte[size]; - var rng = RandomNumberGenerator.Create(); - rng.GetNonZeroBytes(buffer); - return Convert.ToBase64String(buffer).Replace("=", ""); - } -}