From ef6ca957b5acea963c12628116c3fcf52ffbb2d2 Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Sat, 9 May 2026 12:27:52 +0000 Subject: [PATCH] fix(artifactcache): preserve cache key case to stop redundant uploads (#947) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary `artifactcache.Handler` was lowercasing cache keys before storing and returning them. This caused actions like `actions/setup-go` to treat every restore as a partial hit and re-upload the cache on every job run. Similar issue: [act#2497](https://github.com/nektos/act/issues/2497) ## Root Cause These actions build cache keys that include `RUNNER_OS` (e.g. `setup-go-Linux-x64-...` See [setup-go/cache-restore.ts](https://github.com/actions/setup-go/blob/78961f6f84d799cd858575bb931c3e51d3b13290/src/cache-restore.ts#L11-L51) ). In `gitea/runner`, `RUNNER_OS` is always `Linux` by default (See https://gitea.com/gitea/runner/search?q=RUNNER_OS). These actions decide whether to save the cache data in their post step using **strict** `===` comparison between the primary key and the key returned from the runner. See [setup-go cache-save.ts](https://github.com/actions/setup-go/blob/78961f6f84d799cd858575bb931c3e51d3b13290/src/cache-save.ts#L44-L86) . |State | Value| |--- | ---| |CachePrimaryKey | `setup-go-Linux-x64-ubuntu-22.04-go-1.24.9-abc123` | |CacheMatchedKey | `setup-go-linux-x64-ubuntu-22.04-go-1.24.9-abc123` | Because the runner's cache server lowercased the stored key, the response carried `setup-go-linux-...` while the action's primary key was `setup-go-Linux-...`. Strict equality failed, then the actions updated same data again. This repeated on every run, wasting disk and bandwidth. The duplicate blobs accumulate until GC . https://gitea.com/gitea/runner/actions/runs/462560/jobs/737401#jobstep-2-15 ![image.png](/attachments/d3487457-1d09-44b5-9937-a0b8cab1bcc5) https://gitea.com/gitea/runner/actions/runs/462560/jobs/737401#jobstep-6-22 ![image.png](/attachments/9217dc71-cb0c-456b-a516-0017458123c7) ## Fix Drop the `strings.ToLower` calls in `find` and `reserve` so the original key case is preserved end-to-end. This fix will invalidate existing "case insensitive" keys. ## Notes The [original act review](https://github.com/nektos/act/pull/1770/changes/BASE..d44b8d15649d9d09d1d891130b8f3962097a81f3#r1177624608) suggested making cache keys case-insensitive because `isExactKeyMatch` compares cache key ignoring case. However, the actions (`setup-go` / `setup-node` / `setup-ruby`) compare with strict `===` rather than `isExactKeyMatch`. --------- Co-authored-by: Nicolas Reviewed-on: https://gitea.com/gitea/runner/pulls/947 Reviewed-by: silverwind <2021+silverwind@noreply.gitea.com> Reviewed-by: Lunny Xiao Reviewed-by: Nicolas Co-authored-by: Zettat123 Co-committed-by: Zettat123 --- act/artifactcache/handler.go | 6 ------ act/artifactcache/handler_test.go | 18 +++++++++++------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/act/artifactcache/handler.go b/act/artifactcache/handler.go index 29b15e8c..c5bb0376 100644 --- a/act/artifactcache/handler.go +++ b/act/artifactcache/handler.go @@ -325,10 +325,6 @@ func (h *Handler) openDB() (*bolthold.Store, error) { func (h *Handler) find(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { cred := credFromContext(r.Context()) keys := strings.Split(r.URL.Query().Get("keys"), ",") - // cache keys are case insensitive - for i, key := range keys { - keys[i] = strings.ToLower(key) - } version := r.URL.Query().Get("version") db, err := h.openDB() @@ -371,8 +367,6 @@ func (h *Handler) reserve(w http.ResponseWriter, r *http.Request, _ httprouter.P h.responseJSON(w, r, 400, err) return } - // cache keys are case insensitive - api.Key = strings.ToLower(api.Key) cache := api.ToCache() cache.Repo = cred.Repo diff --git a/act/artifactcache/handler_test.go b/act/artifactcache/handler_test.go index a3fe16ac..7cca691c 100644 --- a/act/artifactcache/handler_test.go +++ b/act/artifactcache/handler_test.go @@ -459,17 +459,20 @@ func TestHandler(t *testing.T) { assert.Equal(t, contents[except], content) }) - t.Run("case insensitive", func(t *testing.T) { + t.Run("case preserved", func(t *testing.T) { + // Some actions (e.g. actions/setup-go, actions/setup-node) build cache keys that contain mixed-case fragments such as RUNNER_OS=Linux, + // then compare the cacheKey returned by the cache server to their original key with case-sensitive equality to decide whether the + // cache was a complete hit. The server must therefore preserve the original key case. + version := "c19da02a2bd7e77277f1ac29ab45c09b7d46a4ee758284e26bb3045ad11d9d20" - key := strings.ToLower(t.Name()) + key := strings.ToLower(t.Name()) + "_ABC" content := make([]byte, 100) _, err := rand.Read(content) require.NoError(t, err) - uploadCacheNormally(t, base, key+"_ABC", version, content) + uploadCacheNormally(t, base, key, version, content) { - reqKey := key + "_aBc" - resp, err := testClient.Get(fmt.Sprintf("%s/cache?keys=%s&version=%s", base, reqKey, version)) + resp, err := testClient.Get(fmt.Sprintf("%s/cache?keys=%s&version=%s", base, key, version)) require.NoError(t, err) defer resp.Body.Close() require.Equal(t, 200, resp.StatusCode) @@ -480,7 +483,8 @@ func TestHandler(t *testing.T) { }{} require.NoError(t, json.NewDecoder(resp.Body).Decode(&got)) assert.Equal(t, "hit", got.Result) - assert.Equal(t, key+"_abc", got.CacheKey) + assert.Equal(t, key, got.CacheKey) + assert.NotEqual(t, strings.ToLower(key), got.CacheKey) } }) @@ -643,7 +647,7 @@ func uploadCacheNormally(t *testing.T, base, key, version string, content []byte }{} require.NoError(t, json.NewDecoder(resp.Body).Decode(&got)) assert.Equal(t, "hit", got.Result) - assert.Equal(t, strings.ToLower(key), got.CacheKey) + assert.Equal(t, key, got.CacheKey) archiveLocation = got.ArchiveLocation } {