fix: quota evaluation rules not working properly (#9033)

This patch is mainly intended to fix forgejo/forgejo#7721, and to fix forgejo/forgejo#9019.

It also changes the evaluation of 0 limits to prevent all writes, instead of allowing one write and then failing on subsequent writes after the limit has been exceeded.  This matches the expectation of the existing tests, and I believe it will better match the expectations of users.

Tests have been updated accordingly where necessary, and some additional test coverage added.

The fixes in this PR depend on each other in order for the quota system to function correctly, so I'm submitting them as a single PR instead of individually.

## Test Cases

### Quota subjects not covered by their parent subjects

Before enabling quotas, create a test user and test repository for that user.

Enable quotas, and set a default total to some large value.  (Do not use unit suffixes forgejo/forgejo#8996)

```ini
[quota]
ENABLED = true

[quota.default]
TOTAL = 1073741824
```

With the test user, navigate to "Storage overview" and verify that the quota group "Global quota" is the only group listed, containing the rule "Default", and displays the configured limit, and that the limit has not been exceeded (eg. `42 MiB / 1 GiB`).

The default quota rule has the subject `size:all`, so any write action should be allowed.

#### Attempt to create a new repository.

Expected result: Repository is created.
Actual result: Error 413, You have exhausted your quota.

#### Attempt to create a new file in the existing repository.

Expected result: File is created.
Actual result: Error 413, You have exhausted your quota.

#### Create an issue on the test repository, and attempt to upload an image to the issue.

Expected result: Image is uploaded.
Actual Result: Quota exceeded. Displays error message: `JavaScript promise rejection: can't access property "submitted", oi[ji.uuid] is undefined. Open browser console to see more details.`

### Unlimited quota rules incorrectly allow all writes

With quotas enabled, [Use the API](https://forgejo.org/docs/latest/admin/advanced/quota/#advanced-usage-via-api) to create a quota group containing a single rule with a subject of `sizelfs`, and a limit of `-1` (Unlimited).  Add the test user to this group.

```json
{
  "name": "git-lfs-unlimited",
  "rules": [
    {
      "name": "git-lfs-unlimited",
      "limit": -1,
      "subjects": ["sizelfs"]
    }
  ]
}
```

With the test user, navigate to "Storage overview" and verify that the user has been added to this group, that it is the only group the user is assigned to, and that the rule limit displays as "Unlimited".

The user should only have the ability to write to Git LFS storage, all other writes should be denied.

#### Attempt to create a new repository.

Expected result: Error 413, You have exhausted your quota.
Actual result: Repository is created.

#### Attempt to create a new file in the test repository.

Expected result: Error 413, You have exhausted your quota.
Actual result: File is created.

#### Create an issue on the test repository, and attempt to upload an image to the issue.

Expected Result: Quota exceeded.
Actual result: Image is uploaded.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9033
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Brook Miles <brook@noreply.codeberg.org>
Co-committed-by: Brook Miles <brook@noreply.codeberg.org>

models/quota/default.go

@@ -7,7 +7,7 @@ import (
 	"forgejo.org/modules/setting"
 )
 
-func EvaluateDefault(used Used, forSubject LimitSubject) (bool, int64) {
+func EvaluateDefault(used Used, forSubject LimitSubject) bool {
 	groups := GroupList{
 		&Group{
 			Name: "builtin-default-group",

models/quota/group.go

@@ -5,7 +5,6 @@ package quota
 
 import (
 	"context"
-	"math"
 
 	"forgejo.org/models/db"
 	user_model "forgejo.org/models/user"
@@ -179,78 +178,40 @@ func (g *Group) RemoveRuleByName(ctx context.Context, ruleName string) error {
 	return committer.Commit()
 }
 
-var affectsMap = map[LimitSubject]LimitSubjects{
+// Group.Evaluate returns whether the group contains a matching rule for the subject
-	LimitSubjectSizeAll: {
+// and if so, whether the group allows the action given the size used
-		LimitSubjectSizeReposAll,
+func (g *Group) Evaluate(used Used, forSubject LimitSubject) (match, allow bool) {
-		LimitSubjectSizeGitLFS,
-		LimitSubjectSizeAssetsAll,
-	},
-	LimitSubjectSizeReposAll: {
-		LimitSubjectSizeReposPublic,
-		LimitSubjectSizeReposPrivate,
-	},
-	LimitSubjectSizeAssetsAll: {
-		LimitSubjectSizeAssetsAttachmentsAll,
-		LimitSubjectSizeAssetsArtifacts,
-		LimitSubjectSizeAssetsPackagesAll,
-	},
-	LimitSubjectSizeAssetsAttachmentsAll: {
-		LimitSubjectSizeAssetsAttachmentsIssues,
-		LimitSubjectSizeAssetsAttachmentsReleases,
-	},
-}
-
-// Evaluate returns whether the size used is acceptable for the topic if a rule
-// was found, and returns the smallest limit of all applicable rules or the
-// first limit found to be unacceptable for the size used.
-func (g *Group) Evaluate(used Used, forSubject LimitSubject) (bool, bool, int64) {
-	var found bool
-	foundLimit := int64(math.MaxInt64)
 	for _, rule := range g.Rules {
-		ok, has := rule.Evaluate(used, forSubject)
+		ruleMatch, ruleAllow := rule.Evaluate(used, forSubject)
-		if has {
+		if ruleMatch {
-			if !ok {
+			// evaluation stops as soon as we find a matching rule that denies the action
-				return false, true, rule.Limit
+			if !ruleAllow {
+				return true, false
 			}
-			found = true
+
-			foundLimit = min(foundLimit, rule.Limit)
+			match = true
+			allow = true
 		}
 	}
 
-	if !found {
+	return match, allow
-		// If Evaluation for forSubject did not succeed, try evaluating against
-		// subjects below
-
-		for _, subject := range affectsMap[forSubject] {
-			ok, has, limit := g.Evaluate(used, subject)
-			if has {
-				if !ok {
-					return false, true, limit
-				}
-				found = true
-				foundLimit = min(foundLimit, limit)
-			}
-		}
-	}
-
-	return true, found, foundLimit
 }
 
-// Evaluate returns if the used size is acceptable for the subject and the
+// GroupList.Evaluate returns whether the grouplist allows the action given the size used
-// lowest limit that is acceptable for the subject.
+func (gl *GroupList) Evaluate(used Used, forSubject LimitSubject) (pass bool) {
-func (gl *GroupList) Evaluate(used Used, forSubject LimitSubject) (bool, int64) {
 	// If there are no groups, use the configured defaults:
 	if gl == nil || len(*gl) == 0 {
 		return EvaluateDefault(used, forSubject)
 	}
 
 	for _, group := range *gl {
-		ok, has, limit := group.Evaluate(used, forSubject)
+		groupMatch, groupAllow := group.Evaluate(used, forSubject)
-		if has && ok {
+		if groupMatch && groupAllow {
-			return true, limit
+			// evaluation stops as soon as we find a matching group that allows the action
+			return true
 		}
 	}
-	return false, 0
+	return false
 }
 
 func GetGroupByName(ctx context.Context, name string) (*Group, error) {

models/quota/quota.go

@@ -32,6 +32,6 @@ func EvaluateForUser(ctx context.Context, userID int64, subject LimitSubject) (b
 		return false, err
 	}
 
-	acceptable, _ := groups.Evaluate(*used, subject)
+	allow := groups.Evaluate(*used, subject)
-	return acceptable, nil
+	return allow, nil
 }

models/quota/quota_group_test.go

@@ -4,15 +4,16 @@
 package quota_test
 
 import (
-	"math"
 	"testing"
 
 	quota_model "forgejo.org/models/quota"
+	"forgejo.org/modules/setting"
+	"forgejo.org/modules/test"
 
 	"github.com/stretchr/testify/assert"
 )
 
-func TestQuotaGroupAllRulesMustPass(t *testing.T) {
+func TestQuotaGroupAllRulesMustAllow(t *testing.T) {
 	unlimitedRule := quota_model.Rule{
 		Limit: -1,
 		Subjects: quota_model.LimitSubjects{
@@ -35,12 +36,11 @@ func TestQuotaGroupAllRulesMustPass(t *testing.T) {
 	used := quota_model.Used{}
 	used.Size.Repos.Public = 1024
 
-	// Within a group, *all* rules must pass. Thus, if we have a deny-all rule,
+	// Within a group, *all* matching rules must allow. Thus, if we have a deny-all rule,
-	// and an unlimited rule, that will always fail.
+	// and an unlimited rule, the deny rule wins.
-	ok, has, limit := group.Evaluate(used, quota_model.LimitSubjectSizeAll)
+	match, allow := group.Evaluate(used, quota_model.LimitSubjectSizeAll)
-	assert.True(t, has)
+	assert.True(t, match)
-	assert.False(t, ok)
+	assert.False(t, allow)
-	assert.EqualValues(t, 0, limit)
 }
 
 func TestQuotaGroupRuleScenario1(t *testing.T) {
@@ -68,25 +68,21 @@ func TestQuotaGroupRuleScenario1(t *testing.T) {
 	used.Size.Assets.Packages.All = 256
 	used.Size.Git.LFS = 16
 
-	ok, has, limit := group.Evaluate(used, quota_model.LimitSubjectSizeAssetsAttachmentsReleases)
+	match, allow := group.Evaluate(used, quota_model.LimitSubjectSizeAssetsAttachmentsReleases)
-	assert.True(t, has, "size:assets:attachments:releases is covered")
+	assert.True(t, match, "size:assets:attachments:releases is covered")
-	assert.True(t, ok, "size:assets:attachments:releases passes")
+	assert.True(t, allow, "size:assets:attachments:releases is allowed")
-	assert.EqualValues(t, 1024, limit)
 
-	ok, has, limit = group.Evaluate(used, quota_model.LimitSubjectSizeAssetsPackagesAll)
+	match, allow = group.Evaluate(used, quota_model.LimitSubjectSizeAssetsPackagesAll)
-	assert.True(t, has, "size:assets:packages:all is covered")
+	assert.True(t, match, "size:assets:packages:all is covered")
-	assert.True(t, ok, "size:assets:packages:all passes")
+	assert.True(t, allow, "size:assets:packages:all is allowed")
-	assert.EqualValues(t, 1024, limit)
 
-	ok, has, limit = group.Evaluate(used, quota_model.LimitSubjectSizeGitLFS)
+	match, allow = group.Evaluate(used, quota_model.LimitSubjectSizeGitLFS)
-	assert.True(t, has, "size:git:lfs is covered")
+	assert.True(t, match, "size:git:lfs is covered")
-	assert.False(t, ok, "size:git:lfs fails")
+	assert.False(t, allow, "size:git:lfs is denied")
-	assert.EqualValues(t, 0, limit)
 
-	ok, has, limit = group.Evaluate(used, quota_model.LimitSubjectSizeAll)
+	match, allow = group.Evaluate(used, quota_model.LimitSubjectSizeAll)
-	assert.True(t, has, "size:all is covered")
+	assert.False(t, match, "size:all is not covered")
-	assert.False(t, ok, "size:all fails")
+	assert.False(t, allow, "size:all is not allowed (not covered)")
-	assert.EqualValues(t, 0, limit)
 }
 
 func TestQuotaGroupRuleCombination(t *testing.T) {
@@ -114,31 +110,23 @@ func TestQuotaGroupRuleCombination(t *testing.T) {
 		},
 	}
 
-	// Git LFS isn't covered by any rule
+	// Git LFS does not match any rule
-	_, has, limit := group.Evaluate(used, quota_model.LimitSubjectSizeGitLFS)
+	match, allow := group.Evaluate(used, quota_model.LimitSubjectSizeGitLFS)
-	assert.False(t, has)
+	assert.False(t, match)
-	assert.EqualValues(t, math.MaxInt, limit)
+	assert.False(t, allow)
 
-	// repos:all is covered, and is passing
+	// repos:all has a matching rule and is allowed
-	ok, has, limit := group.Evaluate(used, quota_model.LimitSubjectSizeReposAll)
+	match, allow = group.Evaluate(used, quota_model.LimitSubjectSizeReposAll)
-	assert.True(t, has)
+	assert.True(t, match)
-	assert.True(t, ok)
+	assert.True(t, allow)
-	assert.EqualValues(t, 4096, limit)
 
-	// packages:all is covered, and is failing
+	// packages:all has a matching rule and is denied
-	ok, has, limit = group.Evaluate(used, quota_model.LimitSubjectSizeAssetsPackagesAll)
+	match, allow = group.Evaluate(used, quota_model.LimitSubjectSizeAssetsPackagesAll)
-	assert.True(t, has)
+	assert.True(t, match)
-	assert.False(t, ok)
+	assert.False(t, allow)
-	assert.EqualValues(t, 0, limit)
-
-	// size:all is covered, and is failing (due to packages:all being over quota)
-	ok, has, limit = group.Evaluate(used, quota_model.LimitSubjectSizeAll)
-	assert.True(t, has, "size:all should be covered")
-	assert.False(t, ok, "size:all should fail")
-	assert.EqualValues(t, 0, limit)
 }
 
-func TestQuotaGroupListsRequireOnlyOnePassing(t *testing.T) {
+func TestQuotaGroupListsRequireOnlyOneAllow(t *testing.T) {
 	unlimitedRule := quota_model.Rule{
 		Limit: -1,
 		Subjects: quota_model.LimitSubjects{
@@ -168,13 +156,12 @@ func TestQuotaGroupListsRequireOnlyOnePassing(t *testing.T) {
 	used := quota_model.Used{}
 	used.Size.Repos.Public = 1024
 
-	// In a group list, if any group passes, the entire evaluation passes.
+	// In a group list, an action is allowed if any group matches and allows it.
-	ok, limit := groups.Evaluate(used, quota_model.LimitSubjectSizeAll)
+	allow := groups.Evaluate(used, quota_model.LimitSubjectSizeAll)
-	assert.True(t, ok)
+	assert.True(t, allow)
-	assert.EqualValues(t, -1, limit)
 }
 
-func TestQuotaGroupListAllFailing(t *testing.T) {
+func TestQuotaGroupListAllDeny(t *testing.T) {
 	denyRule := quota_model.Rule{
 		Limit: 0,
 		Subjects: quota_model.LimitSubjects{
@@ -204,18 +191,38 @@ func TestQuotaGroupListAllFailing(t *testing.T) {
 	used := quota_model.Used{}
 	used.Size.Repos.Public = 2048
 
-	ok, limit := groups.Evaluate(used, quota_model.LimitSubjectSizeAll)
+	allow := groups.Evaluate(used, quota_model.LimitSubjectSizeAll)
-	assert.False(t, ok)
+	assert.False(t, allow)
-	assert.EqualValues(t, 0, limit)
 }
 
-func TestQuotaGroupListEmpty(t *testing.T) {
+// An empty group list should result in the use of the built in Default
+// group: size:all defaulting to unlimited
+func TestQuotaDefaultGroup(t *testing.T) {
 	groups := quota_model.GroupList{}
 
 	used := quota_model.Used{}
 	used.Size.Repos.Public = 2048
 
-	ok, limit := groups.Evaluate(used, quota_model.LimitSubjectSizeAll)
+	testSets := []struct {
-	assert.True(t, ok)
+		name        string
-	assert.EqualValues(t, -1, limit)
+		limit       int64
+		expectAllow bool
+	}{
+		{"unlimited", -1, true},
+		{"limit-allow", 1024 * 1024, true},
+		{"limit-deny", 1024, false},
+	}
+
+	for _, testSet := range testSets {
+		t.Run(testSet.name, func(t *testing.T) {
+			defer test.MockVariableValue(&setting.Quota.Default.Total, testSet.limit)()
+
+			for subject := quota_model.LimitSubjectFirst; subject <= quota_model.LimitSubjectLast; subject++ {
+				t.Run(subject.String(), func(t *testing.T) {
+					allow := groups.Evaluate(used, subject)
+					assert.Equal(t, testSet.expectAllow, allow)
+				})
+			}
+		})
+	}
 }

models/quota/quota_rule_test.go

@@ -83,15 +83,27 @@ func assertEvaluation(t *testing.T, rule quota_model.Rule, used quota_model.Used
 	t.Helper()
 
 	t.Run(subject.String(), func(t *testing.T) {
-		ok, has := rule.Evaluate(used, subject)
+		match, allow := rule.Evaluate(used, subject)
-		assert.True(t, has)
+		assert.True(t, match)
-		assert.Equal(t, expected, ok)
+		assert.Equal(t, expected, allow)
 	})
 }
 
-func TestQuotaRuleNoEvaluation(t *testing.T) {
+func TestQuotaRuleNoMatch(t *testing.T) {
+	testSets := []struct {
+		name  string
+		limit int64
+	}{
+		{"unlimited", -1},
+		{"limit-0", 0},
+		{"limit-1k", 1024},
+		{"limit-1M", 1024 * 1024},
+	}
+
+	for _, testSet := range testSets {
+		t.Run(testSet.name, func(t *testing.T) {
 			rule := quota_model.Rule{
-		Limit: 1024,
+				Limit: testSet.limit,
 				Subjects: quota_model.LimitSubjects{
 					quota_model.LimitSubjectSizeAssetsAttachmentsAll,
 				},
@@ -99,12 +111,15 @@ func TestQuotaRuleNoEvaluation(t *testing.T) {
 			used := quota_model.Used{}
 			used.Size.Repos.Public = 4096
 
-	_, has := rule.Evaluate(used, quota_model.LimitSubjectSizeReposAll)
+			match, allow := rule.Evaluate(used, quota_model.LimitSubjectSizeReposAll)
 
 			// We have a rule for "size:assets:attachments:all", and query for
-	// "size:repos:all". We don't cover that subject, so the evaluation returns
+			// "size:repos:all". We don't cover that subject, so the rule does not match
-	// with no rules found.
+			// regardless of the limit.
-	assert.False(t, has)
+			assert.False(t, match)
+			assert.False(t, allow)
+		})
+	}
 }
 
 func TestQuotaRuleDirectEvaluation(t *testing.T) {
@@ -129,13 +144,12 @@ func TestQuotaRuleDirectEvaluation(t *testing.T) {
 	}
 
 	t.Run("limit:0", func(t *testing.T) {
-		// With limit:0, nothing used is fine.
+		// With limit:0, any usage will fail evaluation, including 0
 		t.Run("used:0", func(t *testing.T) {
 			for subject := quota_model.LimitSubjectFirst; subject <= quota_model.LimitSubjectLast; subject++ {
-				runTest(t, subject, 0, 0, true)
+				runTest(t, subject, 0, 0, false)
 			}
 		})
-		// With limit:0, any usage will fail evaluation
 		t.Run("used:512", func(t *testing.T) {
 			for subject := quota_model.LimitSubjectFirst; subject <= quota_model.LimitSubjectLast; subject++ {
 				runTest(t, subject, 0, 512, false)
@@ -170,14 +184,6 @@ func TestQuotaRuleDirectEvaluation(t *testing.T) {
 }
 
 func TestQuotaRuleCombined(t *testing.T) {
-	rule := quota_model.Rule{
-		Limit: 1024,
-		Subjects: quota_model.LimitSubjects{
-			quota_model.LimitSubjectSizeGitLFS,
-			quota_model.LimitSubjectSizeAssetsAttachmentsReleases,
-			quota_model.LimitSubjectSizeAssetsPackagesAll,
-		},
-	}
 	used := quota_model.Used{
 		Size: quota_model.UsedSize{
 			Repos: quota_model.UsedSizeRepos{
@@ -198,107 +204,112 @@ func TestQuotaRuleCombined(t *testing.T) {
 		},
 	}
 
-	expectationMap := map[quota_model.LimitSubject]bool{
+	expectMatch := map[quota_model.LimitSubject]bool{
-		quota_model.LimitSubjectSizeGitLFS:                    false,
+		quota_model.LimitSubjectSizeGitLFS:                    true,
-		quota_model.LimitSubjectSizeAssetsAttachmentsReleases: false,
+		quota_model.LimitSubjectSizeAssetsAttachmentsReleases: true,
-		quota_model.LimitSubjectSizeAssetsPackagesAll:         false,
+		quota_model.LimitSubjectSizeAssetsPackagesAll:         true,
+	}
+
+	testSets := []struct {
+		name        string
+		limit       int64
+		expectAllow bool
+	}{
+		{"unlimited", -1, true},
+		{"limit-allow", 1024 * 1024, true},
+		{"limit-deny", 1024, false},
+	}
+
+	for _, testSet := range testSets {
+		t.Run(testSet.name, func(t *testing.T) {
+			rule := quota_model.Rule{
+				Limit: testSet.limit,
+				Subjects: quota_model.LimitSubjects{
+					quota_model.LimitSubjectSizeGitLFS,
+					quota_model.LimitSubjectSizeAssetsAttachmentsReleases,
+					quota_model.LimitSubjectSizeAssetsPackagesAll,
+				},
 			}
 
 			for subject := quota_model.LimitSubjectFirst; subject <= quota_model.LimitSubjectLast; subject++ {
 				t.Run(subject.String(), func(t *testing.T) {
-			evalOk, evalHas := rule.Evaluate(used, subject)
+					match, allow := rule.Evaluate(used, subject)
-			expected, expectedHas := expectationMap[subject]
 
-			assert.Equal(t, expectedHas, evalHas)
+					assert.Equal(t, expectMatch[subject], match)
-			if expectedHas {
+					if expectMatch[subject] {
-				assert.Equal(t, expected, evalOk)
+						assert.Equal(t, testSet.expectAllow, allow)
+					} else {
+						assert.False(t, allow)
+					}
+				})
 			}
 		})
 	}
 }
 
 func TestQuotaRuleSizeAll(t *testing.T) {
-	runTests := func(t *testing.T, rule quota_model.Rule, expected bool) {
+	type Test struct {
-		t.Helper()
+		name        string
-
+		limit       int64
-		subject := quota_model.LimitSubjectSizeAll
+		expectAllow bool
-
-		t.Run("used:0", func(t *testing.T) {
-			used := quota_model.Used{}
-
-			assertEvaluation(t, rule, used, subject, true)
-		})
-
-		t.Run("used:some-each", func(t *testing.T) {
-			used := makeFullyUsed()
-
-			assertEvaluation(t, rule, used, subject, expected)
-		})
-
-		t.Run("used:some", func(t *testing.T) {
-			used := makePartiallyUsed()
-
-			assertEvaluation(t, rule, used, subject, expected)
-		})
 	}
 
-	// With all limits set to 0, evaluation always fails if usage > 0
+	usedSets := []struct {
-	t.Run("rule:0", func(t *testing.T) {
+		name     string
+		used     quota_model.Used
+		testSets []Test
+	}{
+		{
+			"empty",
+			quota_model.Used{},
+			[]Test{
+				{"unlimited", -1, true},
+				{"limit-1M", 1024 * 1024, true},
+				{"limit-5k", 5 * 1024, true},
+				{"limit-0", 0, false},
+			},
+		},
+		{
+			"partial",
+			makePartiallyUsed(),
+			[]Test{
+				{"unlimited", -1, true},
+				{"limit-1M", 1024 * 1024, true},
+				{"limit-5k", 5 * 1024, true},
+				{"limit-0", 0, false},
+			},
+		},
+		{
+			"full",
+			makeFullyUsed(),
+			[]Test{
+				{"unlimited", -1, true},
+				{"limit-1M", 1024 * 1024, true},
+				{"limit-5k", 5 * 1024, false},
+				{"limit-0", 0, false},
+			},
+		},
+	}
+
+	for _, usedSet := range usedSets {
+		t.Run(usedSet.name, func(t *testing.T) {
+			testSets := usedSet.testSets
+			used := usedSet.used
+
+			for _, testSet := range testSets {
+				t.Run(testSet.name, func(t *testing.T) {
 					rule := quota_model.Rule{
-			Limit: 0,
+						Limit: testSet.limit,
 						Subjects: quota_model.LimitSubjects{
 							quota_model.LimitSubjectSizeAll,
 						},
 					}
 
-		runTests(t, rule, false)
+					match, allow := rule.Evaluate(used, quota_model.LimitSubjectSizeAll)
+					assert.True(t, match)
+					assert.Equal(t, testSet.expectAllow, allow)
 				})
-
-	// With no limits, evaluation always succeeds
-	t.Run("rule:unlimited", func(t *testing.T) {
-		rule := quota_model.Rule{
-			Limit: -1,
-			Subjects: quota_model.LimitSubjects{
-				quota_model.LimitSubjectSizeAll,
-			},
 			}
-
-		runTests(t, rule, true)
 		})
-
-	// With a specific, very generous limit, evaluation succeeds if the limit isn't exhausted
-	t.Run("rule:generous", func(t *testing.T) {
-		rule := quota_model.Rule{
-			Limit: 102400,
-			Subjects: quota_model.LimitSubjects{
-				quota_model.LimitSubjectSizeAll,
-			},
 	}
-
-		runTests(t, rule, true)
-
-		t.Run("limit exhaustion", func(t *testing.T) {
-			used := quota_model.Used{
-				Size: quota_model.UsedSize{
-					Repos: quota_model.UsedSizeRepos{
-						Public: 204800,
-					},
-				},
-			}
-
-			assertEvaluation(t, rule, used, quota_model.LimitSubjectSizeAll, false)
-		})
-	})
-
-	// With a specific, small limit, evaluation fails
-	t.Run("rule:limited", func(t *testing.T) {
-		rule := quota_model.Rule{
-			Limit: 512,
-			Subjects: quota_model.LimitSubjects{
-				quota_model.LimitSubjectSizeAll,
-			},
-		}
-
-		runTests(t, rule, false)
-	})
 }

models/quota/rule.go

@@ -16,6 +16,21 @@ type Rule struct {
 	Subjects LimitSubjects `json:"subjects,omitempty"`
 }
 
+var subjectToParent = map[LimitSubject]LimitSubject{
+	LimitSubjectSizeGitAll:                    LimitSubjectSizeAll,
+	LimitSubjectSizeGitLFS:                    LimitSubjectSizeGitAll,
+	LimitSubjectSizeReposAll:                  LimitSubjectSizeGitAll,
+	LimitSubjectSizeReposPublic:               LimitSubjectSizeReposAll,
+	LimitSubjectSizeReposPrivate:              LimitSubjectSizeReposAll,
+	LimitSubjectSizeAssetsAll:                 LimitSubjectSizeAll,
+	LimitSubjectSizeAssetsAttachmentsAll:      LimitSubjectSizeAssetsAll,
+	LimitSubjectSizeAssetsAttachmentsIssues:   LimitSubjectSizeAssetsAttachmentsAll,
+	LimitSubjectSizeAssetsAttachmentsReleases: LimitSubjectSizeAssetsAttachmentsAll,
+	LimitSubjectSizeAssetsArtifacts:           LimitSubjectSizeAssetsAll,
+	LimitSubjectSizeAssetsPackagesAll:         LimitSubjectSizeAssetsAll,
+	LimitSubjectSizeWiki:                      LimitSubjectSizeAssetsAll,
+}
+
 func (r *Rule) TableName() string {
 	return "quota_rule"
 }
@@ -36,18 +51,25 @@ func (r Rule) Sum(used Used) int64 {
 	return sum
 }
 
-func (r Rule) Evaluate(used Used, forSubject LimitSubject) (bool, bool) {
+func (r Rule) Evaluate(used Used, forSubject LimitSubject) (match, allow bool) {
-	// If there's no limit, short circuit out
-	if r.Limit == -1 {
-		return true, true
-	}
-
-	// If the rule does not cover forSubject, bail out early
 	if !slices.Contains(r.Subjects, forSubject) {
+		// this rule does not match the subject being tested
+		parent := subjectToParent[forSubject]
+		if parent != LimitSubjectNone {
+			return r.Evaluate(used, parent)
+		}
 		return false, false
 	}
 
-	return r.Sum(used) <= r.Limit, true
+	match = true
+
+	if r.Limit == -1 {
+		// Unlimited, any value is allowed
+		allow = true
+	} else {
+		allow = r.Sum(used) < r.Limit
+	}
+	return match, allow
 }
 
 func (r *Rule) Edit(ctx context.Context, limit *int64, subjects *LimitSubjects) (*Rule, error) {

routers/api/v1/api.go

@@ -1193,7 +1193,7 @@ func Routes() *web.Route {
 					m.Get("", repo.ListBranches)
 					m.Get("/*", repo.GetBranch)
 					m.Delete("/*", reqToken(), reqRepoWriter(unit.TypeCode), mustNotBeArchived, repo.DeleteBranch)
-					m.Post("", reqToken(), reqRepoWriter(unit.TypeCode), mustNotBeArchived, bind(api.CreateBranchRepoOption{}), context.EnforceQuotaAPI(quota_model.LimitSubjectSizeGitAll, context.QuotaTargetRepo), repo.CreateBranch)
+					m.Post("", reqToken(), reqRepoWriter(unit.TypeCode), mustNotBeArchived, bind(api.CreateBranchRepoOption{}), context.EnforceQuotaAPI(quota_model.LimitSubjectSizeReposAll, context.QuotaTargetRepo), repo.CreateBranch)
 					m.Patch("/*", reqToken(), reqRepoWriter(unit.TypeCode), mustNotBeArchived, bind(api.UpdateBranchRepoOption{}), repo.UpdateBranch)
 				}, context.ReferencesGitRepo(), reqRepoReader(unit.TypeCode))
 				m.Group("/branch_protections", func() {

tests/e2e/e2e_test.go

@@ -20,6 +20,7 @@ import (
 	"forgejo.org/modules/graceful"
 	"forgejo.org/modules/log"
 	"forgejo.org/modules/setting"
+	"forgejo.org/modules/test"
 	"forgejo.org/modules/testlogger"
 	"forgejo.org/modules/util"
 	"forgejo.org/modules/web"
@@ -37,7 +38,6 @@ func TestMain(m *testing.M) {
 	defer cancel()
 
 	tests.InitTest(true)
-	setting.Quota.Enabled = true
 	initChangedFiles()
 	testE2eWebRoutes = routers.NormalRoutes()
 
@@ -103,6 +103,11 @@ func TestE2e(t *testing.T) {
 		}
 
 		t.Run(testname, func(t *testing.T) {
+			if testname == "user-settings.test.e2e" {
+				defer test.MockVariableValue(&setting.Quota.Enabled, true)()
+				defer test.MockVariableValue(&testE2eWebRoutes, routers.NormalRoutes())()
+			}
+
 			// Default 2 minute timeout
 			onForgejoRun(t, func(*testing.T, *url.URL) {
 				defer DeclareGitRepos(t)()

tests/e2e/fixtures/quota_group_rule_mapping.yml

@@ -6,7 +6,7 @@
 -
   id: 1002
   group_name: trusted-user
-  rule_name: "all:assets"
+  rule_name: "repos:all"
 
 -
   id: 1003

tests/e2e/fixtures/quota_rule.yml

@@ -4,9 +4,9 @@
   subjects: [6]
 
 -
-  name: "all:assets"
+  name: "repos:all"
   limit: -1
-  subjects: [7]
+  subjects: [2]
 
 - name: "Multi subjects"
   limit: 5000000000

tests/integration/api_quota_use_test.go

@@ -187,16 +187,6 @@ func (e *quotaEnv) SetupWithMultipleQuotaRules(t *testing.T) {
 	cleaner = createQuotaGroup(t, "default")
 	e.cleanups = append(e.cleanups, cleaner)
 
-	// Create three rules: all, repo-size, and asset-size
-	zero := int64(0)
-	ruleAll := api.CreateQuotaRuleOptions{
-		Name:     "all",
-		Limit:    &zero,
-		Subjects: []string{"size:all"},
-	}
-	cleaner = createQuotaRule(t, ruleAll)
-	e.cleanups = append(e.cleanups, cleaner)
-
 	fifteenMb := int64(1024 * 1024 * 15)
 	ruleRepoSize := api.CreateQuotaRuleOptions{
 		Name:     "repo-size",
@@ -215,8 +205,6 @@ func (e *quotaEnv) SetupWithMultipleQuotaRules(t *testing.T) {
 	e.cleanups = append(e.cleanups, cleaner)
 
 	// Add these rules to the group
-	cleaner = e.AddRuleToGroup(t, "default", "all")
-	e.cleanups = append(e.cleanups, cleaner)
 	cleaner = e.AddRuleToGroup(t, "default", "repo-size")
 	e.cleanups = append(e.cleanups, cleaner)
 	cleaner = e.AddRuleToGroup(t, "default", "asset-size")
@@ -1414,7 +1402,6 @@ func TestAPIQuotaUserBasics(t *testing.T) {
 
 				// Temporarily disable quota checking
 				defer env.SetRuleLimit(t, "repo-size", -1)()
-				defer env.SetRuleLimit(t, "all", -1)()
 
 				// Create a branch
 				req := NewRequestWithJSON(t, "POST", env.APIPathForRepo("/branches"), api.CreateBranchRepoOption{
@@ -1424,7 +1411,6 @@ func TestAPIQuotaUserBasics(t *testing.T) {
 
 				// Set the limit back. No need to defer, the first one will set it
 				// back to the correct value.
-				env.SetRuleLimit(t, "all", 0)
 				env.SetRuleLimit(t, "repo-size", 0)
 
 				// Deleting a branch does not incur quota enforcement