diff --git a/build/Version.props b/build/Version.props index 80aeac2c88..0099619aac 100644 --- a/build/Version.props +++ b/build/Version.props @@ -3,12 +3,12 @@ - 6.7.0 + 6.8.0 5.1.0 - 10.5.0 + 10.6.0 7.0.0 - 13.5.0 - 15.5.0 + 13.6.0 + 15.6.0 7.1.3 5.9.0 1.4.1 diff --git a/src/Tgstation.Server.Api/Models/ErrorCode.cs b/src/Tgstation.Server.Api/Models/ErrorCode.cs index 7dc134f905..9e1fea2fe9 100644 --- a/src/Tgstation.Server.Api/Models/ErrorCode.cs +++ b/src/Tgstation.Server.Api/Models/ErrorCode.cs @@ -651,5 +651,11 @@ namespace Tgstation.Server.Api.Models /// [Description("Could not create dump as dotnet diagnostics threw an exception!")] DotnetDiagnosticsFailure, + + /// + /// The configured .dme could not be found. + /// + [Description("Could not load configured .dme due to it being outside the deployment directory! This should be a relative path.")] + DeploymentWrongDme, } } diff --git a/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs b/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs index 6890dc25e5..e923f77305 100644 --- a/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs +++ b/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs @@ -607,6 +607,9 @@ namespace Tgstation.Server.Host.Components.Deployment 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); diff --git a/src/Tgstation.Server.Host/Controllers/InstanceController.cs b/src/Tgstation.Server.Host/Controllers/InstanceController.cs index 85f37f0f84..b56d5a9a7a 100644 --- a/src/Tgstation.Server.Host/Controllers/InstanceController.cs +++ b/src/Tgstation.Server.Host/Controllers/InstanceController.cs @@ -103,8 +103,8 @@ namespace Tgstation.Server.Host.Controllers IInstanceManager instanceManager, IJobManager jobManager, IIOManager ioManager, - IPortAllocator portAllocator, IPlatformIdentifier platformIdentifier, + IPortAllocator portAllocator, IPermissionsUpdateNotifyee permissionsUpdateNotifyee, IOptions generalConfigurationOptions, IOptions swarmConfigurationOptions, @@ -150,77 +150,52 @@ namespace Tgstation.Server.Host.Controllers 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 + var instancePaths = await DatabaseContext + .Instances + .AsQueryable() + .Where(x => x.SwarmIdentifer == swarmConfiguration.Identifier) + .Select(x => new Models.Instance { - 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)); + Path = x.Path, + }) + .ToListAsync(cancellationToken); - if (earlyOut != null && !newCancellationToken.IsCancellationRequested) - cts.Cancel(); - }, - newCancellationToken); - } - catch (OperationCanceledException) - { - cancellationToken.ThrowIfCancellationRequested(); - } - } + if ((instancePaths.Count + 1) >= generalConfiguration.InstanceLimit) + return Conflict(new ErrorMessageResponse(ErrorCode.InstanceLimitReached)); - if (earlyOut != null) - return earlyOut; + 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>(); + await Task.WhenAll(pathChecks); + if (!pathChecks.All(task => task.Result)) return BadRequest(new ErrorMessageResponse(ErrorCode.InstanceNotAtWhitelistedPath)); async ValueTask 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 @@ namespace Tgstation.Server.Host.Controllers 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 @@ namespace Tgstation.Server.Host.Controllers 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 @@ namespace Tgstation.Server.Host.Controllers } 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 @@ namespace Tgstation.Server.Host.Controllers return Conflict(new ErrorMessageResponse(ErrorCode.InstanceAtExistingPath)); originalModelPath = originalModel.Path; - originalModel.Path = rawPath; + originalModel.Path = normalizedPath; } } @@ -505,7 +480,7 @@ namespace Tgstation.Server.Host.Controllers 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 @@ namespace Tgstation.Server.Host.Controllers return null; path = ioManager.ResolvePath(path); - if (platformIdentifier.IsWindows) - path = path.ToUpperInvariant().Replace('\\', '/'); + path = platformIdentifier.NormalizePath(path); return path; } diff --git a/src/Tgstation.Server.Host/Database/DatabaseSeeder.cs b/src/Tgstation.Server.Host/Database/DatabaseSeeder.cs index c88c7a5e0d..694fff0c9c 100644 --- a/src/Tgstation.Server.Host/Database/DatabaseSeeder.cs +++ b/src/Tgstation.Server.Host/Database/DatabaseSeeder.cs @@ -51,6 +51,11 @@ namespace Tgstation.Server.Host.Database /// readonly DatabaseConfiguration databaseConfiguration; + /// + /// The for the . + /// + readonly SwarmConfiguration swarmConfiguration; + /// /// Add a default system to a given . /// @@ -83,6 +88,7 @@ namespace Tgstation.Server.Host.Database /// The value of . /// The containing the value of . /// The containing the value of . + /// The containing the value of . /// The value of . /// The value of . public DatabaseSeeder( @@ -90,6 +96,7 @@ namespace Tgstation.Server.Host.Database IPlatformIdentifier platformIdentifier, IOptions generalConfigurationOptions, IOptions databaseConfigurationOptions, + IOptions swarmConfigurationOptions, ILogger databaseLogger, ILogger logger) { @@ -97,6 +104,7 @@ namespace Tgstation.Server.Host.Database 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 @@ namespace Tgstation.Server.Host.Database } } - 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) { diff --git a/src/Tgstation.Server.Host/IO/DefaultIOManager.cs b/src/Tgstation.Server.Host/IO/DefaultIOManager.cs index 0ce1d50f45..b392bb7654 100644 --- a/src/Tgstation.Server.Host/IO/DefaultIOManager.cs +++ b/src/Tgstation.Server.Host/IO/DefaultIOManager.cs @@ -358,6 +358,33 @@ namespace Tgstation.Server.Host.IO DefaultBufferSize, true); + /// + public Task 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); + /// /// Copies a directory from to . /// diff --git a/src/Tgstation.Server.Host/IO/IIOManager.cs b/src/Tgstation.Server.Host/IO/IIOManager.cs index e14f15cf29..13a29310e1 100644 --- a/src/Tgstation.Server.Host/IO/IIOManager.cs +++ b/src/Tgstation.Server.Host/IO/IIOManager.cs @@ -45,6 +45,15 @@ namespace Tgstation.Server.Host.IO /// if contains a '..' accessor, otherwise. bool PathContainsParentAccess(string path); + /// + /// Check if a given is a parent of a given . + /// + /// The parent path. + /// The child path. + /// The for the operation. + /// A resulting in if is a child of or they are equivalent. + Task PathIsChildOf(string parentPath, string childPath, CancellationToken cancellationToken); + /// /// Copies a directory from to . /// @@ -68,7 +77,7 @@ namespace Tgstation.Server.Host.IO /// /// The file to check for existence. /// The for the operation. - /// A resulting in if the file at exists, otherwise. + /// A resulting in if the file at exists, otherwise. Task FileExists(string path, CancellationToken cancellationToken); /// diff --git a/src/Tgstation.Server.Host/System/IPlatformIdentifier.cs b/src/Tgstation.Server.Host/System/IPlatformIdentifier.cs index 805b93b97d..b22173d2c6 100644 --- a/src/Tgstation.Server.Host/System/IPlatformIdentifier.cs +++ b/src/Tgstation.Server.Host/System/IPlatformIdentifier.cs @@ -17,5 +17,12 @@ namespace Tgstation.Server.Host.System /// The extension of executable script files for the system. /// string ScriptFileExtension { get; } + + /// + /// Normalize a path for consistency. + /// + /// The path to normalize. + /// The normalized path. + string NormalizePath(string path); } } diff --git a/src/Tgstation.Server.Host/System/PlatformIdentifier.cs b/src/Tgstation.Server.Host/System/PlatformIdentifier.cs index 22931b97ce..afa9d607e1 100644 --- a/src/Tgstation.Server.Host/System/PlatformIdentifier.cs +++ b/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 @@ namespace Tgstation.Server.Host.System IsWindows = RuntimeInformation.IsOSPlatform(OSPlatform.Windows); ScriptFileExtension = IsWindows ? "bat" : "sh"; } + + /// + public string NormalizePath(string path) + { + ArgumentNullException.ThrowIfNull(path); + if (IsWindows) + path = path.Replace('\\', '/'); + + return path; + } } } diff --git a/tests/Tgstation.Server.Tests/Live/Instance/DeploymentTest.cs b/tests/Tgstation.Server.Tests/Live/Instance/DeploymentTest.cs index fe32e9e196..1b20cd560c 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/DeploymentTest.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/DeploymentTest.cs @@ -1,4 +1,6 @@ using System; +using System.IO; +using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -200,6 +202,27 @@ namespace Tgstation.Server.Tests.Live.Instance deployJob = await dreamMakerClient.Compile(cancellationToken); await WaitForJob(deployJob, 40, true, ErrorCode.DeploymentMissingDme, cancellationToken); + // set to an absolute path that does exist + var tempFile = Path.GetTempFileName().Replace('\\', '/'); + try + { + // for testing purposes, assume same drive for windows + var relativePath = $"../../{String.Join("/", instanceClient.Metadata.Path.Replace('\\', '/').Where(pathChar => pathChar == '/').Select(x => ".."))}{tempFile.Substring(tempFile.IndexOf('/'))}"; + var dmePath = $"{tempFile}.dme"; + File.Move(tempFile, dmePath); + tempFile = dmePath; + await dreamMakerClient.Update(new DreamMakerRequest + { + ProjectName = relativePath + }, cancellationToken); + deployJob = await dreamMakerClient.Compile(cancellationToken); + await WaitForJob(deployJob, 40, true, ErrorCode.DeploymentWrongDme, cancellationToken); + } + finally + { + File.Delete(tempFile); + } + // check that we can change the visibility await vpTest;