Merge pull request tgstation#1358 from tgstation/FixRepoCheckoutPerms…

… [TGSDeploy] Fix RepositoryRights.SetSha and SetReference being able to be bypassed. Fix repo tracking error
MarkSuckerberg · Feb 3, 2022 · 255cbb9 · 255cbb9
2 parents f5d00ce + 7f64243
commit 255cbb9
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 6 deletions.
diff --git a/build/Version.props b/build/Version.props
@@ -3,7 +3,7 @@
   <!-- Integration tests will ensure they match across the board -->
   <Import Project="ControlPanelVersion.props" />
   <PropertyGroup>
-    <TgsCoreVersion>4.16.0</TgsCoreVersion>
+    <TgsCoreVersion>4.16.1</TgsCoreVersion>
     <TgsConfigVersion>4.1.0</TgsConfigVersion>
     <TgsApiVersion>9.3.0</TgsApiVersion>
     <TgsApiLibraryVersion>9.3.1</TgsApiLibraryVersion>

diff --git a/src/Tgstation.Server.Host/Components/Repository/Repository.cs b/src/Tgstation.Server.Host/Components/Repository/Repository.cs
@@ -830,15 +830,18 @@ void RunCheckout() => commands.Checkout(
 			{
 				// Maybe (likely) a remote?
 				var remoteName = $"origin/{committish}";
-				var potentialBranch = libGitRepo.Branches.FirstOrDefault(
+				var remoteBranch = libGitRepo.Branches.FirstOrDefault(
 					branch => branch.FriendlyName.Equals(remoteName, StringComparison.Ordinal));
 				cancellationToken.ThrowIfCancellationRequested();
 
-				if (potentialBranch == default)
+				if (remoteBranch == default)
 					throw;
 
-				logger.LogDebug("Creating local branch for {0}...", potentialBranch.FriendlyName);
-				libGitRepo.CreateBranch(committish, potentialBranch.Tip);
+				logger.LogDebug("Creating local branch for {0}...", remoteBranch.FriendlyName);
+				var branch = libGitRepo.CreateBranch(committish, remoteBranch.Tip);
+
+				libGitRepo.Branches.Update(branch, branchUpdate => branchUpdate.TrackedBranch = remoteBranch.CanonicalName);
+
 				cancellationToken.ThrowIfCancellationRequested();
 
 				RunCheckout();

diff --git a/src/Tgstation.Server.Host/Controllers/RepositoryController.cs b/src/Tgstation.Server.Host/Controllers/RepositoryController.cs
@@ -371,7 +371,9 @@ bool CheckModified<T>(Expression<Func<Api.Models.Internal.RepositorySettings, T>
 				|| CheckModified(x => x.ShowTestMergeCommitters, RepositoryRights.ChangeTestMergeCommits)
 				|| CheckModified(x => x.PostTestMergeComment, RepositoryRights.ChangeTestMergeCommits)
 				|| CheckModified(x => x.UpdateSubmodules, RepositoryRights.ChangeSubmoduleUpdate)
-				|| (model.UpdateFromOrigin == true && !userRights.HasFlag(RepositoryRights.UpdateBranch)))
+				|| (model.UpdateFromOrigin == true && !userRights.HasFlag(RepositoryRights.UpdateBranch))
+				|| (model.CheckoutSha != null && !userRights.HasFlag(RepositoryRights.SetSha))
+				|| (model.Reference != null && model.UpdateFromOrigin != true && !userRights.HasFlag(RepositoryRights.SetReference))) // don't care if it's the same reference, we want to forbid them before starting the job
 				return Forbid();
 
 			if (model.AccessToken?.Length == 0 && model.AccessUser?.Length == 0)