Merge pull request #1835 from tgstation/BarGoesUpInFlames [APIDeploy]…

…[NugetDeploy]

Guard against users using relative pathing to set the project name to the .dme in the repository

build/Version.props

@@ -3,12 +3,12 @@
   <!-- Integration tests will ensure they match across the board -->
   <Import Project="WebpanelVersion.props" />
   <PropertyGroup>
-    <TgsCoreVersion>6.7.0</TgsCoreVersion>
+    <TgsCoreVersion>6.8.0</TgsCoreVersion>
     <TgsConfigVersion>5.1.0</TgsConfigVersion>
-    <TgsApiVersion>10.5.0</TgsApiVersion>
+    <TgsApiVersion>10.6.0</TgsApiVersion>
     <TgsCommonLibraryVersion>7.0.0</TgsCommonLibraryVersion>
-    <TgsApiLibraryVersion>13.5.0</TgsApiLibraryVersion>
-    <TgsClientVersion>15.5.0</TgsClientVersion>
+    <TgsApiLibraryVersion>13.6.0</TgsApiLibraryVersion>
+    <TgsClientVersion>15.6.0</TgsClientVersion>
     <TgsDmapiVersion>7.1.3</TgsDmapiVersion>
     <TgsInteropVersion>5.9.0</TgsInteropVersion>
     <TgsHostWatchdogVersion>1.4.1</TgsHostWatchdogVersion>

src/Tgstation.Server.Api/Models/ErrorCode.cs

@@ -651,5 +651,11 @@ public enum ErrorCode : uint
 		/// </summary>
 		[Description("Could not create dump as dotnet diagnostics threw an exception!")]
 		DotnetDiagnosticsFailure,
+
+		/// <summary>
+		/// The configured .dme could not be found.
+		/// </summary>
+		[Description("Could not load configured .dme due to it being outside the deployment directory! This should be a relative path.")]
+		DeploymentWrongDme,
 	}
 }

src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs

@@ -607,6 +607,9 @@ await eventConsumer.HandleEvent(
 				else
 				{
 					var targetDme = ioManager.ConcatPath(outputDirectory, String.Join('.', job.DmeName, DmeExtension));
+					if (!await ioManager.PathIsChildOf(outputDirectory, targetDme, cancellationToken))
+						throw new JobException(ErrorCode.DeploymentWrongDme);
+
 					var targetDmeExists = await ioManager.FileExists(targetDme, cancellationToken);
 					if (!targetDmeExists)
 						throw new JobException(ErrorCode.DeploymentMissingDme);

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

@@ -103,8 +103,8 @@ public InstanceController(
 			IInstanceManager instanceManager,
 			IJobManager jobManager,
 			IIOManager ioManager,
-			IPortAllocator portAllocator,
 			IPlatformIdentifier platformIdentifier,
+			IPortAllocator portAllocator,
 			IPermissionsUpdateNotifyee permissionsUpdateNotifyee,
 			IOptions<GeneralConfiguration> generalConfigurationOptions,
 			IOptions<SwarmConfiguration> swarmConfigurationOptions,
@@ -150,77 +150,52 @@ public async ValueTask<IActionResult> Create([FromBody] InstanceCreateRequest mo
 			if (earlyOut != null)
 				return earlyOut;
 
-			var unNormalizedPath = model.Path;
-			var targetInstancePath = NormalizePath(unNormalizedPath);
+			var targetInstancePath = NormalizePath(model.Path!);
 			model.Path = targetInstancePath;
 
-			var installationDirectoryPath = NormalizePath(DefaultIOManager.CurrentDirectory);
-
-			bool InstanceIsChildOf(string otherPath)
-			{
-				if (!targetInstancePath.StartsWith(otherPath, StringComparison.Ordinal))
-					return false;
-
-				bool sameLength = targetInstancePath.Length == otherPath.Length;
-				char dirSeparatorChar = targetInstancePath.ToCharArray()[Math.Min(otherPath.Length, targetInstancePath.Length - 1)];
-				return sameLength
-					|| dirSeparatorChar == Path.DirectorySeparatorChar
-					|| dirSeparatorChar == Path.AltDirectorySeparatorChar;
-			}
-
-			if (InstanceIsChildOf(installationDirectoryPath))
+			var installationDirectoryPath = DefaultIOManager.CurrentDirectory;
+			if (await ioManager.PathIsChildOf(installationDirectoryPath, targetInstancePath, cancellationToken))
 				return Conflict(new ErrorMessageResponse(ErrorCode.InstanceAtConflictingPath));
 
 			// Validate it's not a child of any other instance
-			ulong countOfOtherInstances = 0;
-			using (var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken))
-			{
-				var newCancellationToken = cts.Token;
-				try
-				{
-					await DatabaseContext
-						.Instances
-						.AsQueryable()
-						.Where(x => x.SwarmIdentifer == swarmConfiguration.Identifier)
-						.Select(x => new Models.Instance
-						{
-							Path = x.Path,
-						})
-						.ForEachAsync(
-							otherInstance =>
-							{
-								if (++countOfOtherInstances >= generalConfiguration.InstanceLimit)
-									earlyOut ??= Conflict(new ErrorMessageResponse(ErrorCode.InstanceLimitReached));
-								else if (InstanceIsChildOf(otherInstance.Path!))
-									earlyOut ??= Conflict(new ErrorMessageResponse(ErrorCode.InstanceAtConflictingPath));
-
-								if (earlyOut != null && !newCancellationToken.IsCancellationRequested)
-									cts.Cancel();
-							},
-							newCancellationToken);
-				}
-				catch (OperationCanceledException)
+			var instancePaths = await DatabaseContext
+				.Instances
+				.AsQueryable()
+				.Where(x => x.SwarmIdentifer == swarmConfiguration.Identifier)
+				.Select(x => new Models.Instance
 				{
-					cancellationToken.ThrowIfCancellationRequested();
-				}
-			}
+					Path = x.Path,
+				})
+				.ToListAsync(cancellationToken);
 
-			if (earlyOut != null)
-				return earlyOut;
+			if ((instancePaths.Count + 1) >= generalConfiguration.InstanceLimit)
+				return Conflict(new ErrorMessageResponse(ErrorCode.InstanceLimitReached));
+
+			var instancePathChecks = instancePaths
+				.Select(otherInstance => ioManager.PathIsChildOf(otherInstance.Path!, targetInstancePath, cancellationToken))
+				.ToArray();
+
+			await Task.WhenAll(instancePathChecks);
+
+			if (instancePathChecks.Any(task => task.Result))
+				return Conflict(new ErrorMessageResponse(ErrorCode.InstanceAtConflictingPath));
 
 			// Last test, ensure it's in the list of valid paths
-			if (!(generalConfiguration.ValidInstancePaths?
-				.Select(path => NormalizePath(path))
-				.Any(path => InstanceIsChildOf(path)) ?? true))
+			var pathChecks = generalConfiguration.ValidInstancePaths?
+				.Select(path => ioManager.PathIsChildOf(path, targetInstancePath, cancellationToken))
+				.ToArray()
+				?? Enumerable.Empty<Task<bool>>();
+			await Task.WhenAll(pathChecks);
+			if (!pathChecks.All(task => task.Result))
 				return BadRequest(new ErrorMessageResponse(ErrorCode.InstanceNotAtWhitelistedPath));
 
 			async ValueTask<bool> DirExistsAndIsNotEmpty()
 			{
-				if (!await ioManager.DirectoryExists(model.Path, cancellationToken))
+				if (!await ioManager.DirectoryExists(targetInstancePath, cancellationToken))
 					return false;
 
-				var filesTask = ioManager.GetFiles(model.Path, cancellationToken);
-				var dirsTask = ioManager.GetDirectories(model.Path, cancellationToken);
+				var filesTask = ioManager.GetFiles(targetInstancePath, cancellationToken);
+				var dirsTask = ioManager.GetDirectories(targetInstancePath, cancellationToken);
 
 				var files = await filesTask;
 				var dirs = await dirsTask;
@@ -230,8 +205,8 @@ async ValueTask<bool> DirExistsAndIsNotEmpty()
 
 			var dirExistsTask = DirExistsAndIsNotEmpty();
 			bool attached = false;
-			if (await ioManager.FileExists(model.Path, cancellationToken) || await dirExistsTask)
-				if (!await ioManager.FileExists(ioManager.ConcatPath(model.Path, InstanceAttachFileName), cancellationToken))
+			if (await ioManager.FileExists(targetInstancePath, cancellationToken) || await dirExistsTask)
+				if (!await ioManager.FileExists(ioManager.ConcatPath(targetInstancePath, InstanceAttachFileName), cancellationToken))
 					return Conflict(new ErrorMessageResponse(ErrorCode.InstanceAtExistingPath));
 				else
 					attached = true;
@@ -248,7 +223,7 @@ async ValueTask<bool> DirExistsAndIsNotEmpty()
 				try
 				{
 					// actually reserve it now
-					await ioManager.CreateDirectory(unNormalizedPath, cancellationToken);
+					await ioManager.CreateDirectory(targetInstancePath, cancellationToken);
 					await ioManager.DeleteFile(ioManager.ConcatPath(targetInstancePath, InstanceAttachFileName), cancellationToken);
 				}
 				catch
@@ -397,13 +372,13 @@ bool CheckModified<T>(Expression<Func<Api.Models.Instance, T>> expression, Insta
 			}
 
 			string? originalModelPath = null;
-			string? rawPath = null;
+			string? normalizedPath = null;
 			var originalOnline = originalModel.Online!.Value;
 			if (model.Path != null)
 			{
-				rawPath = NormalizePath(model.Path);
+				normalizedPath = NormalizePath(model.Path);
 
-				if (rawPath != originalModel.Path)
+				if (normalizedPath != originalModel.Path)
 				{
 					if (!userRights.HasFlag(InstanceManagerRights.Relocate))
 						return Forbid();
@@ -415,7 +390,7 @@ bool CheckModified<T>(Expression<Func<Api.Models.Instance, T>> expression, Insta
 						return Conflict(new ErrorMessageResponse(ErrorCode.InstanceAtExistingPath));
 
 					originalModelPath = originalModel.Path;
-					originalModel.Path = rawPath;
+					originalModel.Path = normalizedPath;
 				}
 			}
 
@@ -505,7 +480,7 @@ await WithComponentInstanceNullable(
 			var moving = originalModelPath != null;
 			if (moving)
 			{
-				var description = $"Move instance ID {originalModel.Id} from {originalModelPath} to {rawPath}";
+				var description = $"Move instance ID {originalModel.Id} from {originalModelPath} to {normalizedPath}";
 				var job = Job.Create(JobCode.Move, AuthenticationContext.User, originalModel, InstanceManagerRights.Relocate);
 				job.Description = description;
 
@@ -823,8 +798,7 @@ InstancePermissionSet InstanceAdminPermissionSet(InstancePermissionSet? permissi
 				return null;
 
 			path = ioManager.ResolvePath(path);
-			if (platformIdentifier.IsWindows)
-				path = path.ToUpperInvariant().Replace('\\', '/');
+			path = platformIdentifier.NormalizePath(path);
 
 			return path;
 		}

src/Tgstation.Server.Host/Database/DatabaseSeeder.cs

@@ -51,6 +51,11 @@ sealed class DatabaseSeeder : IDatabaseSeeder
 		/// </summary>
 		readonly DatabaseConfiguration databaseConfiguration;
 
+		/// <summary>
+		/// The <see cref="SwarmConfiguration"/> for the <see cref="DatabaseSeeder"/>.
+		/// </summary>
+		readonly SwarmConfiguration swarmConfiguration;
+
 		/// <summary>
 		/// Add a default system <see cref="User"/> to a given <paramref name="databaseContext"/>.
 		/// </summary>
@@ -83,20 +88,23 @@ static User SeedSystemUser(IDatabaseContext databaseContext, User? tgsUser = nul
 		/// <param name="platformIdentifier">The value of <see cref="platformIdentifier"/>.</param>
 		/// <param name="generalConfigurationOptions">The <see cref="IOptions{TOptions}"/> containing the value of <see cref="generalConfiguration"/>.</param>
 		/// <param name="databaseConfigurationOptions">The <see cref="IOptions{TOptions}"/> containing the value of <see cref="databaseConfiguration"/>.</param>
+		/// <param name="swarmConfigurationOptions">The <see cref="IOptions{TOptions}"/> containing the value of <see cref="swarmConfiguration"/>.</param>
 		/// <param name="databaseLogger">The value of <see cref="databaseLogger"/>.</param>
 		/// <param name="logger">The value of <see cref="logger"/>.</param>
 		public DatabaseSeeder(
 			ICryptographySuite cryptographySuite,
 			IPlatformIdentifier platformIdentifier,
 			IOptions<GeneralConfiguration> generalConfigurationOptions,
 			IOptions<DatabaseConfiguration> databaseConfigurationOptions,
+			IOptions<SwarmConfiguration> swarmConfigurationOptions,
 			ILogger<DatabaseContext> databaseLogger,
 			ILogger<DatabaseSeeder> logger)
 		{
 			this.cryptographySuite = cryptographySuite ?? throw new ArgumentNullException(nameof(cryptographySuite));
 			this.platformIdentifier = platformIdentifier ?? throw new ArgumentNullException(nameof(platformIdentifier));
 			databaseConfiguration = databaseConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(databaseConfigurationOptions));
 			generalConfiguration = generalConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(generalConfigurationOptions));
+			swarmConfiguration = swarmConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(swarmConfigurationOptions));
 			this.databaseLogger = databaseLogger ?? throw new ArgumentNullException(nameof(databaseLogger));
 			this.logger = logger ?? throw new ArgumentNullException(nameof(logger));
 		}
@@ -223,16 +231,14 @@ async ValueTask SanitizeDatabase(IDatabaseContext databaseContext, CancellationT
 				}
 			}
 
-			if (platformIdentifier.IsWindows)
-			{
-				// normalize backslashes to forward slashes
-				var allInstances = await databaseContext
-					.Instances
-					.AsQueryable()
-					.ToListAsync(cancellationToken);
-				foreach (var instance in allInstances)
-					instance.Path = instance.Path!.Replace('\\', '/');
-			}
+			// normalize backslashes to forward slashes
+			var allInstances = await databaseContext
+				.Instances
+				.AsQueryable()
+				.Where(instance => instance.SwarmIdentifer == swarmConfiguration.Identifier)
+				.ToListAsync(cancellationToken);
+			foreach (var instance in allInstances)
+				instance.Path = platformIdentifier.NormalizePath(instance.Path!.Replace('\\', '/'));
 
 			if (generalConfiguration.ByondTopicTimeout != 0)
 			{

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

@@ -358,6 +358,33 @@ public Task<DateTimeOffset> GetLastModified(string path, CancellationToken cance
 			DefaultBufferSize,
 			true);
 
+		/// <inheritdoc />
+		public Task<bool> PathIsChildOf(string parentPath, string childPath, CancellationToken cancellationToken) => Task.Factory.StartNew(
+			() =>
+			{
+				parentPath = ResolvePath(parentPath);
+				childPath = ResolvePath(childPath);
+
+				if (parentPath == childPath)
+					return true;
+
+				// https://stackoverflow.com/questions/5617320/given-full-path-check-if-path-is-subdirectory-of-some-other-path-or-otherwise?lq=1
+				var di1 = new DirectoryInfo(parentPath);
+				var di2 = new DirectoryInfo(childPath);
+				while (di2.Parent != null)
+				{
+					if (di2.Parent.FullName == di1.FullName)
+						return true;
+
+					di2 = di2.Parent;
+				}
+
+				return false;
+			},
+			cancellationToken,
+			BlockingTaskCreationOptions,
+			TaskScheduler.Current);
+
 		/// <summary>
 		/// Copies a directory from <paramref name="src"/> to <paramref name="dest"/>.
 		/// </summary>

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

@@ -45,6 +45,15 @@ public interface IIOManager
 		/// <returns><see langword="true"/> if <paramref name="path"/> contains a '..' accessor, <see langword="false"/> otherwise.</returns>
 		bool PathContainsParentAccess(string path);
 
+		/// <summary>
+		/// Check if a given <paramref name="parentPath"/> is a parent of a given <paramref name="parentPath"/>.
+		/// </summary>
+		/// <param name="parentPath">The parent path.</param>
+		/// <param name="childPath">The child path.</param>
+		/// <param name="cancellationToken">The <see cref="CancellationToken"/> for the operation.</param>
+		/// <returns>A <see cref="Task{TResult}"/> resulting in <see langword="true"/> if <paramref name="childPath"/> is a child of <paramref name="parentPath"/> or they are equivalent.</returns>
+		Task<bool> PathIsChildOf(string parentPath, string childPath, CancellationToken cancellationToken);
+
 		/// <summary>
 		/// Copies a directory from <paramref name="src"/> to <paramref name="dest"/>.
 		/// </summary>
@@ -68,7 +77,7 @@ ValueTask CopyDirectory(
 		/// </summary>
 		/// <param name="path">The file to check for existence.</param>
 		/// <param name="cancellationToken">The <see cref="CancellationToken"/> for the operation.</param>
-		/// <returns>A <see cref="Task"/> resulting in <see langword="true"/> if the file at <paramref name="path"/> exists, <see langword="false"/> otherwise.</returns>
+		/// <returns>A <see cref="Task{TResult}"/> resulting in <see langword="true"/> if the file at <paramref name="path"/> exists, <see langword="false"/> otherwise.</returns>
 		Task<bool> FileExists(string path, CancellationToken cancellationToken);
 
 		/// <summary>

src/Tgstation.Server.Host/System/IPlatformIdentifier.cs

@@ -17,5 +17,12 @@ public interface IPlatformIdentifier
 		/// The extension of executable script files for the system.
 		/// </summary>
 		string ScriptFileExtension { get; }
+
+		/// <summary>
+		/// Normalize a path for consistency.
+		/// </summary>
+		/// <param name="path">The path to normalize.</param>
+		/// <returns>The normalized path.</returns>
+		string NormalizePath(string path);
 	}
 }

src/Tgstation.Server.Host/System/PlatformIdentifier.cs

@@ -1,4 +1,5 @@
-using System.Runtime.InteropServices;
+using System;
+using System.Runtime.InteropServices;
 using System.Runtime.Versioning;
 
 namespace Tgstation.Server.Host.System
@@ -21,5 +22,15 @@ public PlatformIdentifier()
 			IsWindows = RuntimeInformation.IsOSPlatform(OSPlatform.Windows);
 			ScriptFileExtension = IsWindows ? "bat" : "sh";
 		}
+
+		/// <inheritdoc />
+		public string NormalizePath(string path)
+		{
+			ArgumentNullException.ThrowIfNull(path);
+			if (IsWindows)
+				path = path.Replace('\\', '/');
+
+			return path;
+		}
 	}
 }