Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,69 @@ public async Task<ActionResult<List<Branch>>> Branches(string org, string reposi
}
}

/// <summary>
/// Creates a new branch in the repository
/// </summary>
/// <param name="org">Unique identifier of the organisation responsible for the app.</param>
/// <param name="repository">The name of repository</param>
/// <param name="request">The branch creation request</param>
/// <returns>The created branch</returns>
[HttpPost]
[Route("repo/{org}/{repository:regex(^(?!datamodels$)[[a-z]][[a-z0-9-]]{{1,28}}[[a-z0-9]]$)}/branches")]
public async Task<ActionResult<Branch>> CreateBranch(string org, string repository, [FromBody] CreateBranchRequest request)
{
var branch = await _sourceControl.CreateBranch(org, repository, request.BranchName);
return Ok(branch);
}
Comment on lines +403 to +416
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add basic validation for CreateBranch request to avoid null/empty branch names

CreateBranch currently assumes request and request.BranchName are non-null and non-empty; sending an empty body or whitespace branch name will either cause a null-reference or push validation down into _sourceControl.CreateBranch, likely surfacing as a 500. CheckoutBranch already guards this case.

Consider aligning CreateBranch with CheckoutBranch by validating upfront:

-        public async Task<ActionResult<Branch>> CreateBranch(string org, string repository, [FromBody] CreateBranchRequest request)
-        {
-            var branch = await _sourceControl.CreateBranch(org, repository, request.BranchName);
-            return Ok(branch);
-        }
+        public async Task<ActionResult<Branch>> CreateBranch(string org, string repository, [FromBody] CreateBranchRequest request)
+        {
+            if (string.IsNullOrWhiteSpace(request?.BranchName))
+            {
+                return BadRequest("Branch name is required");
+            }
+
+            var branch = await _sourceControl.CreateBranch(org, repository, request.BranchName);
+            return Ok(branch);
+        }

This keeps the API surface consistent and prevents avoidable 500s on bad input.

🤖 Prompt for AI Agents
In src/Designer/backend/src/Designer/Controllers/RepositoryController.cs around
lines 403 to 416, CreateBranch does not validate the request or
request.BranchName and can throw a null-reference or surface a 500 for
empty/whitespace input; update the method to mirror CheckoutBranch by checking
if request is null or string.IsNullOrWhiteSpace(request.BranchName) and return
BadRequest("Invalid branch name") (or the same message used by CheckoutBranch)
before calling _sourceControl.CreateBranch, ensuring consistent API behavior and
avoiding server errors on bad input.


/// <summary>
/// Gets information about the current branch
/// </summary>
/// <param name="org">Unique identifier of the organisation responsible for the app.</param>
/// <param name="repository">The name of repository</param>
/// <returns>Information about the current branch</returns>
[HttpGet]
[Route("repo/{org}/{repository:regex(^(?!datamodels$)[[a-z]][[a-z0-9-]]{{1,28}}[[a-z0-9]]$)}/current-branch")]
public ActionResult<CurrentBranchInfo> GetCurrentBranch(string org, string repository)
{
var branchInfo = _sourceControl.GetCurrentBranch(org, repository);
return Ok(branchInfo);
}

/// <summary>
/// Checks out a specific branch
/// </summary>
/// <param name="org">Unique identifier of the organisation responsible for the app.</param>
/// <param name="repository">The name of repository</param>
/// <param name="request">The checkout request</param>
/// <returns>The updated repository status</returns>
[HttpPost]
[Route("repo/{org}/{repository:regex(^(?!datamodels$)[[a-z]][[a-z0-9-]]{{1,28}}[[a-z0-9]]$)}/checkout")]
public async Task<ActionResult<RepoStatus>> CheckoutBranch(string org, string repository, [FromBody] CheckoutBranchRequest request)
{
if (string.IsNullOrWhiteSpace(request?.BranchName))
{
return BadRequest("Branch name is required");
}

var repoStatus = await _sourceControl.CheckoutBranchWithValidation(org, repository, request.BranchName);
return Ok(repoStatus);
}

/// <summary>
/// Discards all local changes in the repository
/// </summary>
/// <param name="org">Unique identifier of the organisation responsible for the app.</param>
/// <param name="repository">The name of repository</param>
/// <returns>The updated repository status</returns>
[HttpPost]
[Route("repo/{org}/{repository:regex(^(?!datamodels$)[[a-z]][[a-z0-9-]]{{1,28}}[[a-z0-9]]$)}/discard-changes")]
public ActionResult<RepoStatus> DiscardLocalChanges(string org, string repository)
{
var repoStatus = _sourceControl.DiscardLocalChanges(org, repository);
return Ok(repoStatus);
}

/// <summary>
/// Stages a specific file changed in working repository.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
using System;
using Altinn.Studio.Designer.Models;

namespace Altinn.Studio.Designer.Exceptions
{
/// <summary>
/// Exception thrown when attempting to checkout a branch with uncommitted changes
/// </summary>
public class UncommittedChangesException : Exception
{
/// <summary>
/// Gets the uncommitted changes error details
/// </summary>
public UncommittedChangesError ErrorDetails { get; }

/// <summary>
/// Initializes a new instance of the <see cref="UncommittedChangesException"/> class.
/// </summary>
/// <param name="errorDetails">The uncommitted changes error details</param>
public UncommittedChangesException(UncommittedChangesError errorDetails)
: base(errorDetails.Message)
{
ErrorDetails = errorDetails;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,7 @@ public class GitErrorCodes
public const string NonFastForwardError = "GT_01";
public const string RepositoryNotFound = "GT_02";
public const string GiteaSessionExpired = "GT_03";
public const string UncommittedChanges = "GT_04";
public const string BranchNotFound = "GT_05";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Looks like BranchNotFound is not used. I suggest implementing or removing the definition.

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ public override void OnException(ExceptionContext context)
{
context.Result = new ObjectResult(ProblemDetailsUtils.GenerateProblemDetails(context.Exception, GitErrorCodes.GiteaSessionExpired, HttpStatusCode.Unauthorized)) { StatusCode = (int)HttpStatusCode.Unauthorized };
}

if (context.Exception is Exceptions.UncommittedChangesException uncommittedChangesException)
{
context.Result = new ObjectResult(uncommittedChangesException.ErrorDetails) { StatusCode = (int)HttpStatusCode.Conflict };
context.ExceptionHandled = true;
Copy link
Contributor

@ErlingHauan ErlingHauan Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered if the line context.ExceptionHandled = true may prevent locating the exception in debugging situations?

}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System.Text.RegularExpressions;

namespace Altinn.Studio.Designer.Helpers;

public static partial class InputValidator
{

Expand Down
103 changes: 103 additions & 0 deletions src/Designer/backend/src/Designer/Models/BranchModels.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
#nullable disable
namespace Altinn.Studio.Designer.Models
{
/// <summary>
/// Request model for creating a new branch
/// </summary>
public class CreateBranchRequest
{
/// <summary>
/// Gets or sets the name of the branch to create
/// </summary>
public string BranchName { get; set; }
}

/// <summary>
/// Request model for checking out a branch
/// </summary>
public class CheckoutBranchRequest
{
/// <summary>
/// Gets or sets the name of the branch to checkout
/// </summary>
public string BranchName { get; set; }
}

/// <summary>
/// Information about the current branch
/// </summary>
public class CurrentBranchInfo
{
/// <summary>
/// Gets or sets the name of the current branch
/// </summary>
public string BranchName { get; set; }

/// <summary>
/// Gets or sets the SHA of the current commit
/// </summary>
public string CommitSha { get; set; }

/// <summary>
/// Gets or sets whether the branch is tracking a remote branch
/// </summary>
public bool IsTracking { get; set; }

/// <summary>
/// Gets or sets the name of the tracked remote branch
/// </summary>
public string RemoteName { get; set; }
}

/// <summary>
/// Error response for uncommitted changes
/// </summary>
public class UncommittedChangesError
{
/// <summary>
/// Gets or sets the error code
/// </summary>
public string ErrorCode { get; set; } = Filters.Git.GitErrorCodes.UncommittedChanges;

/// <summary>
/// Gets or sets the error type
/// </summary>
public string Error { get; set; }

/// <summary>
/// Gets or sets the error message
/// </summary>
public string Message { get; set; }

/// <summary>
/// Gets or sets the list of uncommitted files
/// </summary>
public System.Collections.Generic.List<UncommittedFile> UncommittedFiles { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: If you add using System.Collections.Generic; at the top of the file, this can be shortened:

Suggested change
public System.Collections.Generic.List<UncommittedFile> UncommittedFiles { get; set; }
public List<UncommittedFile> UncommittedFiles { get; set; }


/// <summary>
/// Gets or sets the current branch name
/// </summary>
public string CurrentBranch { get; set; }

/// <summary>
/// Gets or sets the target branch name
/// </summary>
public string TargetBranch { get; set; }
}

/// <summary>
/// Information about an uncommitted file
/// </summary>
public class UncommittedFile
{
/// <summary>
/// Gets or sets the file path
/// </summary>
public string FilePath { get; set; }

/// <summary>
/// Gets or sets the file status
/// </summary>
public string Status { get; set; }
}
}
5 changes: 5 additions & 0 deletions src/Designer/backend/src/Designer/Models/CommitInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,10 @@ public class CommitInfo
/// Gets or sets the repository name
/// </summary>
public string Repository { get; set; }

/// <summary>
/// Gets or sets the branch name to commit to
/// </summary>
public string BranchName { get; set; }
}
}
1 change: 1 addition & 0 deletions src/Designer/backend/src/Designer/Models/FooterFile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using JetBrains.Annotations;

namespace Altinn.Studio.Designer.Models;

public class FooterFile
{
[JsonPropertyName("$schema")]
Expand Down
5 changes: 5 additions & 0 deletions src/Designer/backend/src/Designer/Models/RepoStatus.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,10 @@ public class RepoStatus
/// Defines if there is any merge conflicts
/// </summary>
public bool HasMergeConflict { get; set; }

/// <summary>
/// The current branch name
/// </summary>
public string CurrentBranch { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,48 @@ private void LogError(Exception ex, string method)
LogError(ex, method, string.Empty, string.Empty);
}

/// <inheritdoc/>
public CurrentBranchInfo GetCurrentBranch(string org, string repository)
{
try
{
return _decoratedService.GetCurrentBranch(org, repository);
}
catch (Exception ex)
{
LogError(ex, "GetCurrentBranch", org, repository);
throw;
}
}

/// <inheritdoc/>
public async Task<RepoStatus> CheckoutBranchWithValidation(string org, string repository, string branchName)
{
try
{
return await _decoratedService.CheckoutBranchWithValidation(org, repository, branchName);
}
catch (Exception ex)
{
LogError(ex, "CheckoutBranchWithValidation", org, repository, repository, branchName);
throw;
}
}

/// <inheritdoc/>
public RepoStatus DiscardLocalChanges(string org, string repository)
{
try
{
return _decoratedService.DiscardLocalChanges(org, repository);
}
catch (Exception ex)
{
LogError(ex, "DiscardLocalChanges", org, repository);
throw;
}
}

private void LogError(Exception ex, string method, string org, string repository)
{
LogError(ex, method, org, repository, repository, string.Empty);
Expand Down
Loading
Loading