From cf51d3c888d0950b2e8d52cbf36af209b209c440 Mon Sep 17 00:00:00 2001 From: wejdross Date: Mon, 9 Mar 2026 17:14:50 +0100 Subject: [PATCH] fix: enforce package quota against package owner, not uploader (#11442) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What is broken Quota on packages is not enforced when pushing to an organisation. `enforcePackagesQuota()` calls `EvaluateForUser(ctx.Doer.ID, ...)` — it checks how much space the **uploader** personally owns, not the org being pushed to. Since packages accumulate under `package.owner_id = org_id`, the uploader always shows 0 bytes used and the check always passes. This also means site admins bypass quota entirely when pushing to orgs (they get the service-layer admin bypass on top of the 0-byte measurement). OCI/container routes (`/v2/...`) have the same problem but worse — `enforcePackagesQuota()` was not called on them at all. ## Fix Check quota against `ctx.Package.Owner.ID` instead of `ctx.Doer.ID`. The package owner (the org or user being pushed to) is already available via `ctx.Package.Owner`, populated by `PackageAssignment()` before this middleware runs. For individual user namespaces nothing changes — `ctx.Package.Owner` is the user themselves. Also wired `enforcePackagesQuota()` into the missing OCI upload routes: `InitiateUploadBlob`, `UploadBlob`, `EndUploadBlob`, `UploadManifest` — both in the named `/{image}` group and the wildcard `/*` handler. ## Tested Kind cluster, org `maw2` with 1 GiB quota, 2.6 GiB of container images already pushed: - pushing a generic package to `maw2` as SA user → was 201, now 413 - pushing a generic package to `maw2` as `gitea_admin` → was 201, now 413 - initiating OCI blob upload to `maw2` as SA user → was 202, now 413 - pushing to own user namespace within quota → still 201 Co-authored-by: azhluwi Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11442 Reviewed-by: Andreas Ahlenstorf Reviewed-by: Mathieu Fenniak Co-authored-by: wejdross Co-committed-by: wejdross --- routers/api/packages/api.go | 33 ++++- tests/integration/api_quota_use_test.go | 163 +++++++++++++++++++++++- 2 files changed, 190 insertions(+), 6 deletions(-) diff --git a/routers/api/packages/api.go b/routers/api/packages/api.go index 2676a6309d..800e3514d9 100644 --- a/routers/api/packages/api.go +++ b/routers/api/packages/api.go @@ -94,7 +94,14 @@ func reqPackageAccess(accessMode perm.AccessMode) func(ctx *context.Context) { func enforcePackagesQuota() func(ctx *context.Context) { return func(ctx *context.Context) { - ok, err := quota_model.EvaluateForUser(ctx, ctx.Doer.ID, quota_model.LimitSubjectSizeAssetsPackagesAll) + // Evaluate quota against the package owner (org or user the package is pushed to), + // not the uploader (ctx.Doer). This enables org-level quota: all members uploading + // to an org consume from the org's quota group, not their own personal quota. + ownerID := ctx.Doer.ID + if ctx.Package != nil { + ownerID = ctx.Package.Owner.ID + } + ok, err := quota_model.EvaluateForUser(ctx, ownerID, quota_model.LimitSubjectSizeAssetsPackagesAll) if err != nil { log.Error("quota_model.EvaluateForUser: %v", err) ctx.Error(http.StatusInternalServerError, "Error checking quota") @@ -793,11 +800,11 @@ func ContainerRoutes() *web.Route { r.Group("/{username}", func() { r.Group("/{image}", func() { r.Group("/blobs/uploads", func() { - r.Post("", container.InitiateUploadBlob) + r.Post("", enforcePackagesQuota(), container.InitiateUploadBlob) r.Group("/{uuid}", func() { r.Get("", container.GetUploadBlob) - r.Patch("", container.UploadBlob) - r.Put("", container.EndUploadBlob) + r.Patch("", enforcePackagesQuota(), container.UploadBlob) + r.Put("", enforcePackagesQuota(), container.EndUploadBlob) r.Delete("", container.CancelUploadBlob) }) }, reqPackageAccess(perm.AccessModeWrite)) @@ -807,7 +814,7 @@ func ContainerRoutes() *web.Route { r.Delete("", reqPackageAccess(perm.AccessModeWrite), container.DeleteBlob) }) r.Group("/manifests/{reference}", func() { - r.Put("", reqPackageAccess(perm.AccessModeWrite), container.UploadManifest) + r.Put("", reqPackageAccess(perm.AccessModeWrite), enforcePackagesQuota(), container.UploadManifest) r.Head("", container.HeadManifest) r.Get("", container.GetManifest) r.Delete("", reqPackageAccess(perm.AccessModeWrite), container.DeleteManifest) @@ -843,6 +850,10 @@ func ContainerRoutes() *web.Route { return } + enforcePackagesQuota()(ctx) + if ctx.Written() { + return + } container.InitiateUploadBlob(ctx) return } @@ -875,8 +886,16 @@ func ContainerRoutes() *web.Route { if isGet { container.GetUploadBlob(ctx) } else if isPatch { + enforcePackagesQuota()(ctx) + if ctx.Written() { + return + } container.UploadBlob(ctx) } else if isPut { + enforcePackagesQuota()(ctx) + if ctx.Written() { + return + } container.EndUploadBlob(ctx) } else { container.CancelUploadBlob(ctx) @@ -926,6 +945,10 @@ func ContainerRoutes() *web.Route { return } if isPut { + enforcePackagesQuota()(ctx) + if ctx.Written() { + return + } container.UploadManifest(ctx) } else { container.DeleteManifest(ctx) diff --git a/tests/integration/api_quota_use_test.go b/tests/integration/api_quota_use_test.go index 1d1cf6062c..1ca44cc17e 100644 --- a/tests/integration/api_quota_use_test.go +++ b/tests/integration/api_quota_use_test.go @@ -5,6 +5,7 @@ package integration import ( "bytes" + "crypto/sha256" "encoding/base64" "fmt" "io" @@ -402,7 +403,7 @@ func testAPIQuotaEnforcement(t *testing.T) { t.Run("#/orgs/{org}/repos", func(t *testing.T) { defer tests.PrintCurrentTest(t)() - defer env.SetRuleLimit(t, "all", 0) + defer env.SetRuleLimit(t, "all", 0)() assertCreateRepo := func(t *testing.T, orgName, repoName string, expectedStatus int) func() { t.Helper() @@ -1285,6 +1286,166 @@ func testAPIQuotaEnforcement(t *testing.T) { env.User.Session.MakeRequest(t, req, http.StatusNoContent) }) }) + + // verify that package upload quota is evaluated against the package owner, not the uploader + t.Run("#/packages/{org}/quota-enforcement-against-owner", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + // Ensure the user's own quota is unlimited for this block; prior tests may have left it at 0. + defer env.SetRuleLimit(t, "all", -1)() + + t.Run("upload to limited org is rejected", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + // Verify the user has quota remaining, so rejection is due to org quota + req := NewRequest(t, "GET", "/api/v1/user/quota/check?subject=size:assets:packages:all").AddTokenAuth(env.User.Token) + resp := env.User.Session.MakeRequest(t, req, http.StatusOK) + var quotaOK bool + DecodeJSON(t, resp, "aOK) + assert.True(t, quotaOK, "user must have quota remaining before the rejection test") + + body := strings.NewReader("forgejo is awesome") + req = NewRequestWithBody(t, "PUT", + fmt.Sprintf("/api/packages/%s/generic/org-quota-test/1.0.0/file.txt", env.Orgs.Limited.Name), + body, + ).AddTokenAuth(env.User.Token) + env.User.Session.MakeRequest(t, req, http.StatusRequestEntityTooLarge) + }) + + t.Run("upload to unlimited org succeeds even when user quota is zero", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + defer env.SetRuleLimit(t, "all", 0)() + + // Verify the user's quota is zero before the upload + req := NewRequest(t, "GET", "/api/v1/user/quota/check?subject=size:assets:packages:all").AddTokenAuth(env.User.Token) + resp := env.User.Session.MakeRequest(t, req, http.StatusOK) + var quotaOK bool + DecodeJSON(t, resp, "aOK) + assert.False(t, quotaOK, "user must have no quota before the upload test") + + body := strings.NewReader("forgejo is awesome") + req = NewRequestWithBody(t, "PUT", + fmt.Sprintf("/api/packages/%s/generic/org-quota-test/1.0.0/file.txt", env.Orgs.Unlimited.Name), + body, + ).AddTokenAuth(env.User.Token) + env.User.Session.MakeRequest(t, req, http.StatusCreated) + + env.WithoutQuota(t, func() { + req := NewRequestf(t, "DELETE", "/api/v1/packages/%s/generic/org-quota-test/1.0.0", env.Orgs.Unlimited.Name). + AddTokenAuth(env.User.Token) + env.User.Session.MakeRequest(t, req, http.StatusNoContent) + }) + }) + }) + + t.Run("#/v2/{org}/container/quota-enforcement-against-owner", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + // Ensure the user's own quota is unlimited for this block; prior tests may have left it at 0. + defer env.SetRuleLimit(t, "all", -1)() + + type tokenResponse struct { + Token string `json:"token"` + } + + getContainerToken := func(t *testing.T, username string) string { + t.Helper() + req := NewRequest(t, "GET", fmt.Sprintf("%sv2/token", setting.AppURL)). + AddBasicAuth(username) + resp := MakeRequest(t, req, http.StatusOK) + var tr tokenResponse + DecodeJSON(t, resp, &tr) + return fmt.Sprintf("Bearer %s", tr.Token) + } + + blobContent := []byte("quota-container-blob") + blobDigest := fmt.Sprintf("sha256:%x", sha256.Sum256(blobContent)) + configContent := `{}` + configDigest := fmt.Sprintf("sha256:%x", sha256.Sum256([]byte(configContent))) + manifestContent := fmt.Sprintf(`{"schemaVersion":2,"mediaType":"application/vnd.docker.distribution.manifest.v2+json","config":{"mediaType":"application/vnd.docker.container.image.v1+json","digest":%q,"size":%d},"layers":[{"mediaType":"application/vnd.docker.image.rootfs.diff.tar.gzip","digest":%q,"size":%d}]}`, + configDigest, len(configContent), blobDigest, len(blobContent)) + + userToken := getContainerToken(t, env.User.User.Name) + + t.Run("upload to limited org is rejected", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + // Verify the user has quota remaining, so rejection is due to org quota + req := NewRequest(t, "GET", "/api/v1/user/quota/check?subject=size:assets:packages:all").AddTokenAuth(env.User.Token) + resp := env.User.Session.MakeRequest(t, req, http.StatusOK) + var quotaOK bool + DecodeJSON(t, resp, "aOK) + assert.True(t, quotaOK, "user must have quota remaining before the rejection test") + + image := fmt.Sprintf("%sv2/%s/quota-test-img", setting.AppURL, env.Orgs.Limited.Name) + + req = NewRequestWithBody(t, "POST", fmt.Sprintf("%s/blobs/uploads?digest=%s", image, blobDigest), bytes.NewReader(blobContent)). + AddTokenAuth(userToken) + MakeRequest(t, req, http.StatusRequestEntityTooLarge) + + req = NewRequest(t, "POST", fmt.Sprintf("%s/blobs/uploads", image)). + AddTokenAuth(userToken) + resp = MakeRequest(t, req, http.StatusRequestEntityTooLarge) + + // chunked upload: initiate returns 413, so patch/put are never reached + // this is the result we expect, otherwise it's very hard to create multi-tenant environment + _ = resp + }) + + t.Run("upload to unlimited org succeeds even when user quota is zero", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + defer env.SetRuleLimit(t, "all", 0)() + + // Verify the user's quota is zero before the upload + req := NewRequest(t, "GET", "/api/v1/user/quota/check?subject=size:assets:packages:all").AddTokenAuth(env.User.Token) + resp := env.User.Session.MakeRequest(t, req, http.StatusOK) + var quotaOK bool + DecodeJSON(t, resp, "aOK) + assert.False(t, quotaOK, "user must have no quota before the upload test") + + image := fmt.Sprintf("%sv2/%s/quota-test-img", setting.AppURL, env.Orgs.Unlimited.Name) + + // monolithic blob upload + req = NewRequestWithBody(t, "POST", fmt.Sprintf("%s/blobs/uploads?digest=%s", image, blobDigest), bytes.NewReader(blobContent)). + AddTokenAuth(userToken) + MakeRequest(t, req, http.StatusCreated) + + req = NewRequestWithBody(t, "POST", fmt.Sprintf("%s/blobs/uploads?digest=%s", image, configDigest), strings.NewReader(configContent)). + AddTokenAuth(userToken) + MakeRequest(t, req, http.StatusCreated) + + // chunked blob upload (patch + put) + req = NewRequest(t, "POST", fmt.Sprintf("%s/blobs/uploads", image)). + AddTokenAuth(userToken) + resp = MakeRequest(t, req, http.StatusAccepted) + + uploadURL := resp.Header().Get("Location") + contentRange := fmt.Sprintf("0-%d", len(blobContent)-1) + req = NewRequestWithBody(t, "PATCH", setting.AppURL+uploadURL[1:], bytes.NewReader(blobContent)). + AddTokenAuth(userToken). + SetHeader("Content-Range", contentRange) + resp = MakeRequest(t, req, http.StatusAccepted) + + uploadURL = resp.Header().Get("Location") + req = NewRequest(t, "PUT", fmt.Sprintf("%s?digest=%s", setting.AppURL+uploadURL[1:], blobDigest)). + AddTokenAuth(userToken) + MakeRequest(t, req, http.StatusCreated) + + // upload manifest + req = NewRequestWithBody(t, "PUT", fmt.Sprintf("%s/manifests/v1", image), strings.NewReader(manifestContent)). + AddTokenAuth(userToken). + SetHeader("Content-Type", "application/vnd.docker.distribution.manifest.v2+json") + MakeRequest(t, req, http.StatusCreated) + + // delete manifest + env.WithoutQuota(t, func() { + manifestDigest := fmt.Sprintf("sha256:%x", sha256.Sum256([]byte(manifestContent))) + req := NewRequest(t, "DELETE", fmt.Sprintf("%s/manifests/%s", image, manifestDigest)). + AddTokenAuth(userToken) + MakeRequest(t, req, http.StatusAccepted) + }) + }) + }) } func TestAPIQuotaOrgQuotaQuery(t *testing.T) {