Various Configuration fixups

docs/API.dox

@@ -418,11 +418,19 @@ To create, write, and delete files use the following request
 
 I POST "/Config" @ref Tgstation.Server.Api.Models.ConfigurationFile => @ref Tgstation.Server.Api.Models.ConfigurationFile
 
-When creating a file, only @ref Tgstation.Server.Api.Models.ConfigurationFile.Path and @ref Tgstation.Server.Api.Models.ConfigurationFile.Content should be specified. Any necessary preceeding directories will be created if possible.
+When creating a file, only @ref Tgstation.Server.Api.Models.ConfigurationFile.Path and @ref Tgstation.Server.Api.Models.ConfigurationFile.Content should be specified
 
 If the file already exists, the @ref Tgstation.Server.Api.Models.ConfigurationFile.LastReadHash field must also be present with the last version recieved from the server for that file. If this does not match at the time of the request, 409 will be returned, indicating the file has changed since it was last viewed by the client.
 
-To delete a file set @ref Tgstation.Server.Api.Models.ConfigurationFile.Content to null in the request. If deleting a file leaves a directory empty, they too will be deleted.
+To delete a file set @ref Tgstation.Server.Api.Models.ConfigurationFile.Content to null in the request.
+
+
+To delete an empty directory use the following request
+
+I DELETE "/Config" @ref Tgstation.Server.Api.Models.ConfigurationFile => OK
+
+Where the request @ref Tgstation.Server.Api.Models.ConfigurationFile.Path is set to the directory to delete
+
 
 @subsection api_dog Watchdog
 

src/Tgstation.Server.Client/ApiClient.cs

@@ -197,6 +197,9 @@ namespace Tgstation.Server.Client
 		/// <inheritdoc />
 		public Task Delete(string route, long instanceId, CancellationToken cancellationToken) => RunRequest<object>(route, null, HttpMethod.Delete, instanceId, cancellationToken);
 
+		/// <inheritdoc />
+		public Task Delete<TBody>(string route, TBody body, long instanceId, CancellationToken cancellationToken) => RunRequest<object>(route, body, HttpMethod.Delete, instanceId, cancellationToken);
+
 		/// <inheritdoc />
 		public Task<TResult> Delete<TResult>(string route, long instanceId, CancellationToken cancellationToken) => RunRequest<TResult>(route, null, HttpMethod.Delete, instanceId, cancellationToken);
 

src/Tgstation.Server.Client/Components/ConfigurationClient.cs

@@ -19,6 +19,20 @@ namespace Tgstation.Server.Client.Components
 		/// </summary>
 		readonly Instance instance;
 
+		/// <summary>
+		/// Sanitize a <see cref="ConfigurationFile"/> path for use in a GET <see cref="Uri"/>
+		/// </summary>
+		/// <param name="path">The path to sanitize</param>
+		/// <returns>The sanitized path</returns>
+		static string SanitizeGetPath(string path)
+		{
+			if (path == null)
+				path = String.Empty;
+			if (path.Length == 0 || path[0] != '/')
+				path = '/' + path;
+			return path;
+		}
+
 		/// <summary>
 		/// Construct a <see cref="ConfigurationClient"/>
 		/// </summary>
@@ -31,22 +45,17 @@ namespace Tgstation.Server.Client.Components
 		}
 
 		/// <inheritdoc />
-		public Task DeleteEmptyDirectory(string directory, CancellationToken cancellationToken) => apiClient.Delete(Routes.Configuration + directory ?? throw new ArgumentNullException(nameof(directory)), instance.Id, cancellationToken);
+		public Task DeleteEmptyDirectory(ConfigurationFile directory, CancellationToken cancellationToken) => apiClient.Delete(Routes.Configuration, directory, instance.Id, cancellationToken);
 
 		/// <inheritdoc />
-		public Task<IReadOnlyList<ConfigurationFile>> List(string directory, CancellationToken cancellationToken)
-		{
-			if (directory == null)
-				directory = String.Empty;
-			return apiClient.Read<IReadOnlyList<ConfigurationFile>>(Routes.List(Routes.Configuration) + directory, instance.Id, cancellationToken);
-		}
+		public Task<IReadOnlyList<ConfigurationFile>> List(string directory, CancellationToken cancellationToken) => apiClient.Read<IReadOnlyList<ConfigurationFile>>(Routes.List(Routes.Configuration) + SanitizeGetPath(directory), instance.Id, cancellationToken);
 
 		/// <inheritdoc />
 		public Task<ConfigurationFile> Read(ConfigurationFile file, CancellationToken cancellationToken)
 		{
 			if (file == null)
 				throw new ArgumentNullException(nameof(file));
-			return apiClient.Read<ConfigurationFile>(Routes.Configuration + file.Path, instance.Id, cancellationToken);
+			return apiClient.Read<ConfigurationFile>(Routes.Configuration + SanitizeGetPath(file.Path), instance.Id, cancellationToken);
 		}
 
 		/// <inheritdoc />

src/Tgstation.Server.Client/Components/IConfigurationClient.cs

@@ -37,9 +37,9 @@ namespace Tgstation.Server.Client.Components
 		/// <summary>
 		/// Delete an empty <paramref name="directory"/>
 		/// </summary>
-		/// <param name="directory">The path to directory to delete</param>
+		/// <param name="directory">The <see cref="ConfigurationFile"/> representing the directory to delete</param>
 		/// <param name="cancellationToken">The <see cref="CancellationToken"/> for the operation</param>
 		/// <returns>A <see cref="Task"/> representing the running operation</returns>
-		Task DeleteEmptyDirectory(string directory, CancellationToken cancellationToken);
+		Task DeleteEmptyDirectory(ConfigurationFile directory, CancellationToken cancellationToken);
 	}
 }

src/Tgstation.Server.Client/IApiClient.cs

@@ -35,6 +35,7 @@ namespace Tgstation.Server.Client
 		Task<TResult> Read<TResult>(string route, long instanceId, CancellationToken cancellationToken);
 		Task<TResult> Update<TBody, TResult>(string route, TBody body, long instanceId, CancellationToken cancellationToken);
 		Task Delete(string route, long instanceId, CancellationToken cancellationToken);
+		Task Delete<TBody>(string route, TBody body, long instanceId, CancellationToken cancellationToken);
 		Task<TResult> Delete<TResult>(string route, long instanceId, CancellationToken cancellationToken);
 	}
 }

src/Tgstation.Server.Host/Components/StaticFiles/Configuration.cs

@@ -144,6 +144,8 @@ namespace Tgstation.Server.Host.Components.StaticFiles
 			var nullOrEmptyCheck = String.IsNullOrEmpty(configurationRelativePath);
 			if (nullOrEmptyCheck)
 				configurationRelativePath = ".";
+			if (configurationRelativePath[0] == Path.DirectorySeparatorChar || configurationRelativePath[0] == Path.AltDirectorySeparatorChar)
+				configurationRelativePath = '.' + configurationRelativePath;
 			var resolved = ioManager.ResolvePath(configurationRelativePath);
 			var local = !nullOrEmptyCheck ? ioManager.ResolvePath(".") : null;
 			if (!nullOrEmptyCheck && resolved.Length < local.Length) //.. fuccbois

src/Tgstation.Server.Host/Controllers/ConfigurationController.cs

@@ -2,6 +2,7 @@
 using Microsoft.Extensions.Logging;
 using System;
 using System.IO;
+using System.Linq;
 using System.Net;
 using System.Threading;
 using System.Threading.Tasks;
@@ -9,6 +10,7 @@ using Tgstation.Server.Api;
 using Tgstation.Server.Api.Models;
 using Tgstation.Server.Api.Rights;
 using Tgstation.Server.Host.Components;
+using Tgstation.Server.Host.IO;
 using Tgstation.Server.Host.Models;
 using Tgstation.Server.Host.Security;
 
@@ -25,29 +27,39 @@ namespace Tgstation.Server.Host.Controllers
 		/// </summary>
 		readonly IInstanceManager instanceManager;
 
+		/// <summary>
+		/// The <see cref="IIOManager"/> for the <see cref="ConfigurationController"/>
+		/// </summary>
+		readonly IIOManager ioManager;
+
 		/// <summary>
 		/// Construct a <see cref="UserController"/>
 		/// </summary>
 		/// <param name="databaseContext">The <see cref="IDatabaseContext"/> for the <see cref="ApiController"/></param>
 		/// <param name="authenticationContextFactory">The <see cref="IAuthenticationContextFactory"/> for the <see cref="ApiController"/></param>
 		/// <param name="instanceManager">The value of <see cref="instanceManager"/></param>
+		/// <param name="ioManager">The value of <see cref="ioManager"/></param>
 		/// <param name="logger">The <see cref="ILogger"/> for the <see cref="ApiController"/></param>
-		public ConfigurationController(IDatabaseContext databaseContext, IAuthenticationContextFactory authenticationContextFactory, IInstanceManager instanceManager, ILogger<ConfigurationController> logger) : base(databaseContext, authenticationContextFactory, logger, true)
+		public ConfigurationController(IDatabaseContext databaseContext, IAuthenticationContextFactory authenticationContextFactory, IInstanceManager instanceManager, IIOManager ioManager, ILogger<ConfigurationController> logger) : base(databaseContext, authenticationContextFactory, logger, true)
 		{
 			this.instanceManager = instanceManager ?? throw new ArgumentNullException(nameof(instanceManager));
+			this.ioManager = ioManager ?? throw new ArgumentNullException(nameof(ioManager));
 		}
 
 		/// <summary>
-		/// If a <see cref="ForbidResult"/> should be returned from actions due to conflicts with one or both of the <see cref="Api.Models.Instance.ConfigurationType"/> or the <see cref="IAuthenticationContext.SystemIdentity"/>
+		/// If a <see cref="ForbidResult"/> should be returned from actions due to conflicts with one or both of the <see cref="Api.Models.Instance.ConfigurationType"/> or the <see cref="IAuthenticationContext.SystemIdentity"/> or a given <paramref name="path"/> tries to access parent directories
 		/// </summary>
+		/// <param name="path">The path to validate if any</param>
 		/// <returns><see langword="true"/> if a <see cref="ForbidResult"/> should be returned, <see langword="false"/> otherwise</returns>
-		bool ForbidDueToModeConflicts() => Instance.ConfigurationType == ConfigurationType.Disallowed || (Instance.ConfigurationType == ConfigurationType.SystemIdentityWrite && AuthenticationContext.SystemIdentity == null);
+		bool ForbidDueToModeConflicts(string path) => Instance.ConfigurationType == ConfigurationType.Disallowed || (Instance.ConfigurationType == ConfigurationType.SystemIdentityWrite && AuthenticationContext.SystemIdentity == null) || (path != null && ioManager.PathContainsParentAccess(path));
 
 		/// <inheritdoc />
 		[TgsAuthorize(ConfigurationRights.Write)]
 		public override async Task<IActionResult> Update([FromBody] ConfigurationFile model, CancellationToken cancellationToken)
 		{
-			if (ForbidDueToModeConflicts())
+			if (model == null)
+				throw new ArgumentNullException(nameof(model));
+			if (ForbidDueToModeConflicts(model.Path))
 				return Forbid();
 
 			var config = instanceManager.GetInstance(Instance).Configuration;
@@ -88,7 +100,7 @@ namespace Tgstation.Server.Host.Controllers
 		[TgsAuthorize(ConfigurationRights.Read)]
 		public async Task<IActionResult> File(string filePath, CancellationToken cancellationToken)
 		{
-			if (ForbidDueToModeConflicts())
+			if (ForbidDueToModeConflicts(filePath))
 				return Forbid();
 
 			try
@@ -123,7 +135,7 @@ namespace Tgstation.Server.Host.Controllers
 		[TgsAuthorize(ConfigurationRights.List)]
 		public async Task<IActionResult> Directory(string directoryPath, CancellationToken cancellationToken)
 		{
-			if (ForbidDueToModeConflicts())
+			if (ForbidDueToModeConflicts(directoryPath))
 				return Forbid();
 
 			try
@@ -152,7 +164,10 @@ namespace Tgstation.Server.Host.Controllers
 		[TgsAuthorize(ConfigurationRights.Write)]
 		public override async Task<IActionResult> Create([FromBody] ConfigurationFile model, CancellationToken cancellationToken)
 		{
-			if (ForbidDueToModeConflicts())
+			if (model == null)
+				throw new ArgumentNullException(nameof(model));
+
+			if (ForbidDueToModeConflicts(model.Path))
 				return Forbid();
 
 			try
@@ -169,16 +184,19 @@ namespace Tgstation.Server.Host.Controllers
 			}
 		}
 		
-		[HttpDelete("{*directoryPath}")]
+		[HttpDelete]
 		[TgsAuthorize(ConfigurationRights.Delete)]
-		public async Task<IActionResult> Delete(string directoryPath, CancellationToken cancellationToken)
+		public async Task<IActionResult> Delete([FromBody] ConfigurationFile directory, CancellationToken cancellationToken)
 		{
-			if (ForbidDueToModeConflicts())
+			if (directory == null)
+				throw new ArgumentNullException(nameof(directory));
+
+			if (ForbidDueToModeConflicts(directory.Path))
 				return Forbid();
 
 			try
 			{
-				return await instanceManager.GetInstance(Instance).Configuration.DeleteDirectory(directoryPath, AuthenticationContext.SystemIdentity, cancellationToken).ConfigureAwait(false) ? (IActionResult)Ok() : Conflict(new ErrorMessage
+				return await instanceManager.GetInstance(Instance).Configuration.DeleteDirectory(directory.Path, AuthenticationContext.SystemIdentity, cancellationToken).ConfigureAwait(false) ? (IActionResult)Ok() : Conflict(new ErrorMessage
 				{
 					Message = "Directory not empty!"
 				});

src/Tgstation.Server.Host/IO/DefaultIOManager.cs

@@ -310,5 +310,8 @@ namespace Tgstation.Server.Host.IO
 			using (var archive = new ZipArchive(ms, ZipArchiveMode.Read))
 				archive.ExtractToDirectory(path);
 		}, cancellationToken, TaskCreationOptions.LongRunning, TaskScheduler.Current);
+
+		/// <inheritdoc />
+		public bool PathContainsParentAccess(string path) => path?.Split(new[] { Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar }).Any(x => x == "..") ?? throw new ArgumentNullException(nameof(path));
 	}
 }

src/Tgstation.Server.Host/IO/IIOManager.cs

@@ -31,6 +31,13 @@ namespace Tgstation.Server.Host.IO
 		/// <returns>The file name portion of <paramref name="path"/></returns>
 		string GetFileNameWithoutExtension(string path);
 
+		/// <summary>
+		/// Check if a <paramref name="path"/> contains the '..' parent directory accessor
+		/// </summary>
+		/// <param name="path">The path to check</param>
+		/// <returns><see langword="true"/> if <paramref name="path"/> contains a '..' accessor, <see langword="false"/> otherwise</returns>
+		bool PathContainsParentAccess(string path);
+
 		/// <summary>
 		/// Copies a directory from <paramref name="src"/> to <paramref name="dest"/>
 		/// </summary>

src/Tgstation.Server.Host/IO/SynchronousIOManager.cs

@@ -104,7 +104,7 @@ namespace Tgstation.Server.Host.IO
 				using (var sha1 = new SHA1Managed())
 #pragma warning restore CA5350 // Do not use insecure cryptographic algorithm SHA1.
 				{
-					string GetSha1(byte[] dataToHash) => dataToHash.Length != 0 ? String.Join("", sha1.ComputeHash(dataToHash).Select(b => b.ToString("x2", CultureInfo.InvariantCulture))) : null;
+					string GetSha1(byte[] dataToHash) => dataToHash != null && dataToHash.Length != 0 ? String.Join("", sha1.ComputeHash(dataToHash).Select(b => b.ToString("x2", CultureInfo.InvariantCulture))) : null;
 					var originalSha1 = GetSha1(originalBytes);
 					if (originalSha1 != sha1InOut)
 					{