Refuse a request that names more branch patterns than MaxBranchPatternsPerRequest [patch]

GitBranchStateCache.Tests/Configuration/GitBranchStateCacheOptionsValidatorTests.cs

@@ -152,6 +152,20 @@ public void Validate_NonPositiveMaxCachedMergeBases_Fails()
 		Assert.Contains("MaxCachedMergeBases", result.FailureMessage);
 	}
 
+	[TestMethod]
+	[DataRow(0)]
+	[DataRow(-1)]
+	public void Validate_NonPositiveMaxBranchPatternsPerRequest_Fails(int limit)
+	{
+		GitBranchStateCacheOptions options = Valid();
+		options.MaxBranchPatternsPerRequest = limit;
+
+		ValidateOptionsResult result = Validate(options);
+
+		Assert.IsTrue(result.Failed);
+		Assert.Contains("MaxBranchPatternsPerRequest", result.FailureMessage);
+	}
+
 	[TestMethod]
 	public void Validate_NonPositiveMaxConcurrentBranchQueries_Fails()
 	{

GitBranchStateCache.Tests/Integration/StateFlowTests.cs

@@ -258,6 +258,66 @@
 		Assert.AreEqual(HttpStatusCode.BadRequest, response.StatusCode);
 	}
 
+	[TestMethod]
+	public async Task Post_State_MoreBranchPatternsThanTheLimit_Is400BeforeAnyFetch()
+	{
+		// Every pattern is matched against every branch, synchronously, so the count has to be
+		// refused before the mirror is cloned or fetched.
+		await using ServiceFixture fixture = await ServiceFixture.StartAsync(new()
+		{
+			["GitBranchStateCache:MaxBranchPatternsPerRequest"] = "3",
+		});
+		Seed(fixture.Git);
+
+		string[] patterns = ["origin/main", "origin/release/*", "origin/a*", "origin/b*"];
+
+		using HttpResponseMessage response = await fixture.Client.SendAsync(
+			StateRequest(JsonSerializer.Serialize(new { @base = ClientBase, branchPatterns = patterns })));
+		JsonElement body = await ReadAsync(response);
+
+		Assert.AreEqual(HttpStatusCode.BadRequest, response.StatusCode);
+		Assert.Contains("at most 3", body.GetProperty("message").GetString()!);
+		Assert.AreEqual(0, fixture.Git.CountOf("clone"));
+		Assert.AreEqual(0, fixture.Git.CountOf("fetch"));
+	}
+
+	[TestMethod]
+	public async Task Post_State_BranchPatternsAtTheLimit_AreAccepted()
+	{
+		await using ServiceFixture fixture = await ServiceFixture.StartAsync(new()
+		{
+			["GitBranchStateCache:MaxBranchPatternsPerRequest"] = "2",
+		});
+		Seed(fixture.Git);
+
+		string[] patterns = ["origin/main", "origin/release/*"];
+
+		using HttpResponseMessage response = await fixture.Client.SendAsync(
+			StateRequest(JsonSerializer.Serialize(new { @base = ClientBase, branchPatterns = patterns })));
+
+		Assert.AreEqual(HttpStatusCode.OK, response.StatusCode);
+	}
+
+	[TestMethod]
+	public async Task Get_Branches_MoreBranchPatternsThanTheLimit_Is400()
+	{
+		await using ServiceFixture fixture = await ServiceFixture.StartAsync(new()
+		{
+			["GitBranchStateCache:MaxBranchPatternsPerRequest"] = "1",
+		});
+		Seed(fixture.Git);
+
+		using HttpRequestMessage request = new(
+			HttpMethod.Get,
+			"/v1/github/studio/game.git/branches?pattern=origin/main&pattern=origin/release/*");
+		request.Headers.Authorization = AuthenticationHeaderValue.Parse(Credential);
+
+		using HttpResponseMessage response = await fixture.Client.SendAsync(request);
+
+		Assert.AreEqual(HttpStatusCode.BadRequest, response.StatusCode);
+		Assert.AreEqual(0, fixture.Git.CountOf("clone"));
+	}
+
 	[TestMethod]
 	[DataRow("HEAD")]
 	[DataRow("main@{1}")]

GitBranchStateCache.Tool/appsettings.json

@@ -16,6 +16,7 @@
     "MaxCachedMergeBases": 20000,
     "MaxConcurrentBranchQueries": 4,
     "MaxPathsPerRequest": 20000,
+    "MaxBranchPatternsPerRequest": 256,
     "MirrorIdleMaxAge": "30.00:00:00",
     "MaintenanceInterval": "01:00:00"
   }

GitBranchStateCache/Configuration/GitBranchStateCacheOptions.cs

@@ -123,6 +123,15 @@ public sealed class GitBranchStateCacheOptions
 	/// </summary>
 	public int MaxPathsPerRequest { get; set; } = 20_000;
 
+	/// <summary>
+	/// Gets or sets the most branch patterns one request may name.
+	/// </summary>
+	/// <remarks>
+	/// Every pattern is matched against every branch in the mirror, synchronously, so without a limit
+	/// one request can keep a thread busy for minutes. A request over it is refused before any fetch.
+	/// </remarks>
+	public int MaxBranchPatternsPerRequest { get; set; } = 256;
+
 	/// <summary>
 	/// Gets or sets how long a mirror may go unqueried before it is deleted.
 	/// </summary>

GitBranchStateCache/Configuration/GitBranchStateCacheOptionsValidator.cs

@@ -167,6 +167,11 @@ private static void ValidateLimits(GitBranchStateCacheOptions options, List<stri
 		{
 			failures.Add($"{GitBranchStateCacheOptions.SectionName}:MaxPathsPerRequest must be greater than zero.");
 		}
+
+		if (options.MaxBranchPatternsPerRequest <= 0)
+		{
+			failures.Add($"{GitBranchStateCacheOptions.SectionName}:MaxBranchPatternsPerRequest must be greater than zero.");
+		}
 	}
 
 	private static void RequirePositive(TimeSpan value, string setting, List<string> failures)

GitBranchStateCache/Endpoints/BranchStateHandler.cs

@@ -42,7 +42,7 @@
 /// <param name="metrics">Service counters.</param>
 /// <param name="options">The configured options.</param>
 /// <param name="logger">Logger.</param>
 internal sealed class BranchStateHandler(
 	IUpstreamRegistry registry,
 	IRepositoryAllowList allowList,
 	IMirrorStore mirrors,
@@ -141,7 +141,11 @@
 				+ "which 'git rev-parse @{upstream}' produces without touching the network.");
 		}
 
-		if (!TryParsePatterns(request.BranchPatterns, out List<BranchPattern> patterns, out string? patternFailure))
+		if (!TryParsePatterns(
+			request.BranchPatterns,
+			options.Value.MaxBranchPatternsPerRequest,
+			out List<BranchPattern> patterns,
+			out string? patternFailure))
 		{
 			return BadRequest(patternFailure!);
 		}
@@ -407,7 +411,11 @@
 				.Select(pattern => pattern!)
 		];
 
-		if (!TryParsePatterns(requested, out List<BranchPattern> patterns, out string? patternFailure))
+		if (!TryParsePatterns(
+			requested,
+			options.Value.MaxBranchPatternsPerRequest,
+			out List<BranchPattern> patterns,
+			out string? patternFailure))
 		{
 			return BadRequest(patternFailure!);
 		}
@@ -532,6 +540,7 @@
 
 	private static bool TryParsePatterns(
 		IReadOnlyList<string>? requested,
+		int limit,
 		out List<BranchPattern> patterns,
 		out string? failure)
 	{
@@ -544,6 +553,12 @@
 			return false;
 		}
 
+		if (requested.Count > limit)
+		{
+			failure = $"The request named {requested.Count} branch patterns, and at most {limit} may be sent.";
+			return false;
+		}
+
 		foreach (string pattern in requested)
 		{
 			if (!BranchPattern.TryParse(pattern, out BranchPattern? parsed, out string? reason))

README.md

@@ -144,6 +144,7 @@ Resolves wildcard patterns against the mirror's refs and returns names with tip
     "MaxCachedMergeBases": 20000,
     "MaxConcurrentBranchQueries": 4,
     "MaxPathsPerRequest": 20000,
+    "MaxBranchPatternsPerRequest": 256,
     "MirrorIdleMaxAge": "30.00:00:00",
     "Upstreams": {
       "github": {
@@ -165,6 +166,8 @@ Refs younger than `RefsTtl` are answered from as they are. Past it, and for up t
 
 Merge bases are cached by mirror, client base and branch tip, up to `MaxCachedMergeBases`, and diffs by merge base and tip, up to `MaxCachedDiffs`. A request whose answers are all cached starts no git processes beyond reading the refs. One that is not works on up to `MaxConcurrentBranchQueries` branches at a time. Each clone and fetch also writes a commit-graph, which is what keeps an uncached merge base quick on a long history.
 
+A request may name up to `MaxPathsPerRequest` paths and up to `MaxBranchPatternsPerRequest` branch patterns. Every pattern is matched against every branch in the mirror, so a request over either limit is refused with a 400 before anything is fetched.
+
 ## Deployment
 
 Deploy **adjacent to the forge, not on-premises**. Its cost is round trips and its clients are worst served on residential links, which is the opposite placement from an object cache. `deploy/k8s` is a kustomize base: a StatefulSet with a read-write-once volume, a service, an ingress and a configmap. Start at one replica; each replica holds its own mirrors and diff cache, so replicas multiply fetch traffic and disk without improving the hit rate.