diff --git a/build/Dockerfile b/build/Dockerfile index c1156ae74b..b2f45da8c8 100644 --- a/build/Dockerfile +++ b/build/Dockerfile @@ -69,4 +69,6 @@ COPY --from=build /repo/build/tgs.docker.sh tgs.sh VOLUME ["/config_data", "/tgs_logs", "/app/lib"] +ENV General__ValidInstancePaths__0 /tgs4_instances + ENTRYPOINT ["./tgs.sh"] diff --git a/src/Tgstation.Server.Api/Models/ChatBot.cs b/src/Tgstation.Server.Api/Models/ChatBot.cs index f6560f90ec..afa8aab001 100644 --- a/src/Tgstation.Server.Api/Models/ChatBot.cs +++ b/src/Tgstation.Server.Api/Models/ChatBot.cs @@ -10,7 +10,7 @@ namespace Tgstation.Server.Api.Models /// /// Channels the Discord bot should listen/announce in /// - public List Channels { get; set; } + public ICollection Channels { get; set; } /// /// Validates are correct for the diff --git a/src/Tgstation.Server.Api/Models/ErrorCode.cs b/src/Tgstation.Server.Api/Models/ErrorCode.cs index ecb96a8c53..c1f4c1331f 100644 --- a/src/Tgstation.Server.Api/Models/ErrorCode.cs +++ b/src/Tgstation.Server.Api/Models/ErrorCode.cs @@ -459,5 +459,11 @@ namespace Tgstation.Server.Api.Models /// [Description("Encountered merge conflicts while test merging one or more pull requests!")] RepoTestMergeConflict, + + /// + /// Attempted to create an instance outside of the . + /// + [Description("The new instance's path is not under a white-listed path.")] + InstanceNotAtWhitelistedPath, } } \ No newline at end of file diff --git a/src/Tgstation.Server.Api/Models/Internal/ServerInformation.cs b/src/Tgstation.Server.Api/Models/Internal/ServerInformation.cs index 2b22f8f2f7..101877c917 100644 --- a/src/Tgstation.Server.Api/Models/Internal/ServerInformation.cs +++ b/src/Tgstation.Server.Api/Models/Internal/ServerInformation.cs @@ -1,4 +1,6 @@ -namespace Tgstation.Server.Api.Models.Internal +using System.Collections.Generic; + +namespace Tgstation.Server.Api.Models.Internal { /// /// Base class for . @@ -19,5 +21,10 @@ /// The maximum number of s allowed. /// public uint UserLimit { get; set; } + + /// + /// Limits the locations instances may be created or attached from. + /// + public ICollection ValidInstancePaths { get; set; } } } diff --git a/src/Tgstation.Server.Api/Models/RevisionInformation.cs b/src/Tgstation.Server.Api/Models/RevisionInformation.cs index 0340478d97..9fbe61578a 100644 --- a/src/Tgstation.Server.Api/Models/RevisionInformation.cs +++ b/src/Tgstation.Server.Api/Models/RevisionInformation.cs @@ -13,11 +13,11 @@ namespace Tgstation.Server.Api.Models /// /// The s active in the /// - public List ActiveTestMerges { get; set; } + public ICollection ActiveTestMerges { get; set; } /// /// The s made from the /// - public List CompileJobs { get; set; } + public ICollection CompileJobs { get; set; } } } diff --git a/src/Tgstation.Server.Host/Controllers/HomeController.cs b/src/Tgstation.Server.Host/Controllers/HomeController.cs index c41afa512d..4f08acb9d0 100644 --- a/src/Tgstation.Server.Host/Controllers/HomeController.cs +++ b/src/Tgstation.Server.Host/Controllers/HomeController.cs @@ -126,7 +126,8 @@ namespace Tgstation.Server.Host.Controllers ApiVersion = ApiHeaders.Version, MinimumPasswordLength = generalConfiguration.MinimumPasswordLength, InstanceLimit = generalConfiguration.InstanceLimit, - UserLimit = generalConfiguration.UserLimit + UserLimit = generalConfiguration.UserLimit, + ValidInstancePaths = generalConfiguration.ValidInstancePaths }); // if we are using a browser and the control panel, soft redirect to the app page diff --git a/src/Tgstation.Server.Host/Controllers/InstanceController.cs b/src/Tgstation.Server.Host/Controllers/InstanceController.cs index 981ed2a52a..d207b07d5c 100644 --- a/src/Tgstation.Server.Host/Controllers/InstanceController.cs +++ b/src/Tgstation.Server.Host/Controllers/InstanceController.cs @@ -151,28 +151,24 @@ namespace Tgstation.Server.Host.Controllers var installationDirectoryPath = NormalizePath(DefaultIOManager.CurrentDirectory); - IActionResult CheckInstanceNotChildOf(string conflictingPath) + bool InstanceIsChildOf(string otherPath) { - if (targetInstancePath.StartsWith(conflictingPath, StringComparison.Ordinal)) - { - bool sameLength = targetInstancePath.Length == conflictingPath.Length; - char dirSeparatorChar = targetInstancePath.ToCharArray()[Math.Min(conflictingPath.Length, targetInstancePath.Length - 1)]; - if (sameLength - || dirSeparatorChar == Path.DirectorySeparatorChar - || dirSeparatorChar == Path.AltDirectorySeparatorChar) - return Conflict(new ErrorMessage(ErrorCode.InstanceAtConflictingPath)); - } + if (!targetInstancePath.StartsWith(otherPath, StringComparison.Ordinal)) + return false; - return null; + 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; } - var earlyOut = CheckInstanceNotChildOf(installationDirectoryPath); - if (earlyOut != null) - return earlyOut; - - ulong countOfOtherInstances = 0; + if (InstanceIsChildOf(installationDirectoryPath)) + return Conflict(new ErrorMessage(ErrorCode.InstanceAtConflictingPath)); // Validate it's not a child of any other instance + IActionResult earlyOut = null; + ulong countOfOtherInstances = 0; using (var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken)) { var newCancellationToken = cts.Token; @@ -182,9 +178,9 @@ namespace Tgstation.Server.Host.Controllers otherInstance => { if (++countOfOtherInstances >= generalConfiguration.InstanceLimit) - earlyOut = Conflict(new ErrorMessage(ErrorCode.InstanceLimitReached)); - else - earlyOut = earlyOut ?? CheckInstanceNotChildOf(otherInstance.Path); + earlyOut ??= Conflict(new ErrorMessage(ErrorCode.InstanceLimitReached)); + else if (InstanceIsChildOf(otherInstance.Path)) + earlyOut ??= Conflict(new ErrorMessage(ErrorCode.InstanceAtConflictingPath)); if (earlyOut != null && !newCancellationToken.IsCancellationRequested) cts.Cancel(); @@ -201,6 +197,12 @@ namespace Tgstation.Server.Host.Controllers if (earlyOut != null) return earlyOut; + // Last test, ensure it's in the list of valid paths + if (!(generalConfiguration.ValidInstancePaths? + .Select(path => NormalizePath(path)) + .Any(path => InstanceIsChildOf(path)) ?? true)) + return BadRequest(new ErrorMessage(ErrorCode.InstanceNotAtWhitelistedPath)); + async Task DirExistsAndIsNotEmpty() { if (!await ioManager.DirectoryExists(model.Path, cancellationToken).ConfigureAwait(false)) diff --git a/src/Tgstation.Server.Host/appsettings.json b/src/Tgstation.Server.Host/appsettings.json index a9175e9585..20702e34c5 100644 --- a/src/Tgstation.Server.Host/appsettings.json +++ b/src/Tgstation.Server.Host/appsettings.json @@ -8,7 +8,8 @@ "UseExperimentalWatchdog": false, "UseBasicWatchdogOnWindows": true, "UserLimit": 100, - "InstanceLimit": 10 + "InstanceLimit": 10, + "ValidInstancePaths": null }, "FileLogging": { "Directory": null, diff --git a/tests/Tgstation.Server.Tests/InstanceManagerTest.cs b/tests/Tgstation.Server.Tests/InstanceManagerTest.cs index d83dbe8794..6d69d8e5cc 100644 --- a/tests/Tgstation.Server.Tests/InstanceManagerTest.cs +++ b/tests/Tgstation.Server.Tests/InstanceManagerTest.cs @@ -84,6 +84,15 @@ namespace Tgstation.Server.Tests Path = testNonEmpty }, cancellationToken), ErrorCode.InstanceAtExistingPath).ConfigureAwait(false); + // test can't create instance outside of whitelist + await ApiAssert.ThrowsException(() => instanceManagerClient.CreateOrAttach(new Api.Models.Instance + { + Name = "TestInstanceOutsideOfWhitelist", + Path = Path.Combine(testRootPath, "..", Guid.NewGuid().ToString()), + Online = true, + ChatBotLimit = 1 + }, cancellationToken), ErrorCode.InstanceNotAtWhitelistedPath); + //test basic move Directory.Delete(testNonEmpty); var initialPath = firstTest.Path; diff --git a/tests/Tgstation.Server.Tests/TestingServer.cs b/tests/Tgstation.Server.Tests/TestingServer.cs index 97e1106a8b..2f8943cba5 100644 --- a/tests/Tgstation.Server.Tests/TestingServer.cs +++ b/tests/Tgstation.Server.Tests/TestingServer.cs @@ -58,7 +58,7 @@ namespace Tgstation.Server.Tests Console.WriteLine("WARNING: No GitHub access token configured, test may fail due to rate limits!"); dumpOpenAPISpecpath = !String.IsNullOrEmpty(dumpOpenAPISpecPathEnvVar); - + var args = new List() { String.Format(CultureInfo.InvariantCulture, "Kestrel:EndPoints:Http:Url={0}", UrlString), @@ -67,7 +67,8 @@ namespace Tgstation.Server.Tests String.Format(CultureInfo.InvariantCulture, "Database:DropDatabase={0}", true), String.Format(CultureInfo.InvariantCulture, "General:SetupWizardMode={0}", SetupWizardMode.Never), String.Format(CultureInfo.InvariantCulture, "General:InstanceLimit={0}", 10), - String.Format(CultureInfo.InvariantCulture, "General:UserLimit={0}", 150) + String.Format(CultureInfo.InvariantCulture, "General:UserLimit={0}", 150), + String.Format(CultureInfo.InvariantCulture, "General:ValidInstancePaths:0={0}", Directory) }; if (!String.IsNullOrEmpty(gitHubAccessToken))