From 4f59e3e418cf88d6d6fd76b3fc9f147395cdea69 Mon Sep 17 00:00:00 2001 From: silverwind <2021+silverwind@noreply.gitea.com> Date: Fri, 13 Nov 2026 09:33:32 +0000 Subject: [PATCH] fix: support `cache-mode` and make low-trust runs cache read-only (#1412) Low-trust runs like `pull_request_target` on the default branch could poison caches that `push` runs restore. As on GitHub, they now only restore unless `cache-mode` allows saving. This and the Gitea change can be deployed in any order. Refs: https://gitea.com/gitea/actionslib/pulls/33 https://github.com/go-gitea/gitea/pull/40343 _Written by Claude (Opus 5.5)._ Reviewed-on: https://gitea.com/gitea/runner/pulls/1412 Reviewed-by: bircni Co-authored-by: silverwind <2021+silverwind@noreply.gitea.com> --- act/artifactcache/handler.go | 33 +++++++++++++++++++++++---- act/artifactcache/handler_test.go | 33 ++++++++++++++++++++++----- act/artifactcache/handler_v2.go | 9 +++++--- act/artifactcache/handler_v2_test.go | 24 +++++++++++++++++++ go.mod | 2 +- go.sum | 4 ++-- internal/app/run/runner.go | 24 +++++++++++++++---- internal/app/run/runner_cache_test.go | 24 +++++++++++++++---- 8 files changed, 127 insertions(+), 26 deletions(-) diff --git a/act/artifactcache/handler.go b/act/artifactcache/handler.go index 540ea3ed..c1a7e94e 100644 --- a/act/artifactcache/handler.go +++ b/act/artifactcache/handler.go @@ -62,6 +62,7 @@ type JobCredential struct { Ref string `json:"ref"` BaseRef string `json:"base_ref"` DefaultRef string `json:"default_ref"` + CacheMode string `json:"cache_mode"` // empty allows everything // Results is the instance whose artifact service this server forwards for the job, and // InsecureTLS how the runner reaches it; see results.go. @@ -218,10 +219,10 @@ func StartHandler(opts Options) (*Handler, error) { } router := http.NewServeMux() - router.HandleFunc("GET "+apiPath+"/cache", h.bearerAuth(h.find)) - router.HandleFunc("POST "+apiPath+"/caches", h.bearerAuth(h.reserve)) - router.HandleFunc("PATCH "+apiPath+"/caches/{id}", h.bearerAuth(h.upload)) - router.HandleFunc("POST "+apiPath+"/caches/{id}", h.bearerAuth(h.commit)) + router.HandleFunc("GET "+apiPath+"/cache", h.cacheModeAuth(false, h.find)) + router.HandleFunc("POST "+apiPath+"/caches", h.cacheModeAuth(true, h.reserve)) + router.HandleFunc("PATCH "+apiPath+"/caches/{id}", h.cacheModeAuth(true, h.upload)) + router.HandleFunc("POST "+apiPath+"/caches/{id}", h.cacheModeAuth(true, h.commit)) router.HandleFunc("POST "+apiPath+"/clean", h.bearerAuth(h.clean)) // Artifact GET is signed via query-string HMAC because @actions/cache // does not attach Authorization when downloading archiveLocation. @@ -956,6 +957,30 @@ func (h *Handler) bearerAuth(handler http.HandlerFunc) http.HandlerFunc { } } +// cacheModeAuth is bearerAuth refusing what the job's cache-mode forbids, in the shapes the toolkit reports as a warning. +func (h *Handler) cacheModeAuth(write bool, handler http.HandlerFunc) http.HandlerFunc { + return h.bearerAuth(func(w http.ResponseWriter, r *http.Request) { + mode := credFromContext(r.Context()).CacheMode + access, forbidden := "read", "write-only" + if write { + access, forbidden = "write", "read" + } + if mode != "none" && mode != forbidden { + handler(w, r) + return + } + message := fmt.Sprintf("cache %s denied: the job's cache-mode is %s", access, mode) + switch { + case !strings.HasPrefix(r.URL.Path, cacheServiceV2Path): + h.responseJSON(w, r, http.StatusForbidden, map[string]any{"message": message}) + case write: + h.responseJSON(w, r, http.StatusOK, map[string]any{"ok": false, "message": message}) + default: + h.twirpError(w, r, twirpDenied, errors.New(message)) + } + }) +} + func (h *Handler) unauthorized(w http.ResponseWriter, r *http.Request, err error) { if strings.HasPrefix(r.URL.Path, cacheServiceV2Path) { h.twirpError(w, r, twirpUnauthenticated, err) diff --git a/act/artifactcache/handler_test.go b/act/artifactcache/handler_test.go index c27e9e1b..7b88da13 100644 --- a/act/artifactcache/handler_test.go +++ b/act/artifactcache/handler_test.go @@ -1003,7 +1003,7 @@ func TestHandler_UnregisterRevokes(t *testing.T) { } } -func TestHandler_CrossRepoIsolation(t *testing.T) { +func TestHandler_CrossRepoAndCacheModeIsolation(t *testing.T) { t.Parallel() dir := filepath.Join(t.TempDir(), "artifactcache") handler, err := StartHandler(Options{Dir: dir}) @@ -1012,6 +1012,8 @@ func TestHandler_CrossRepoIsolation(t *testing.T) { handler.RegisterJob("token-a", JobCredential{Repo: "owner/repoA"}) handler.RegisterJob("token-b", JobCredential{Repo: "owner/repoB"}) handler.RegisterJob("token-a-pull", JobCredential{Repo: "owner/repoA", Ref: "refs/pull/1/head"}) + handler.RegisterJob("token-a-read", JobCredential{Repo: "owner/repoA", CacheMode: "read"}) + handler.RegisterJob("token-a-write-only", JobCredential{Repo: "owner/repoA", CacheMode: "write-only"}) base := handler.ExternalURL() + apiPath key := "shared-key" @@ -1041,12 +1043,31 @@ func TestHandler_CrossRepoIsolation(t *testing.T) { resp.Body.Close() assert.Equal(t, http.StatusNoContent, resp.StatusCode) - resp, err = clientA.Get(fmt.Sprintf("%s/cache?keys=%s&version=%s", base, key, version)) - require.NoError(t, err) - resp.Body.Close() - assert.Equal(t, http.StatusOK, resp.StatusCode) + clientRead := &http.Client{Transport: &bearerTransport{token: "token-a-read"}} + for _, client := range []*http.Client{clientA, clientRead} { + resp, err = client.Get(fmt.Sprintf("%s/cache?keys=%s&version=%s", base, key, version)) + require.NoError(t, err) + resp.Body.Close() + assert.Equal(t, http.StatusOK, resp.StatusCode) + } - for _, client := range []*http.Client{clientB, {Transport: &bearerTransport{token: "token-a-pull"}}} { + requireDenied := func(prefix string, do func() (*http.Response, error)) { + resp, err := do() + require.NoError(t, err) + defer resp.Body.Close() + denied := map[string]string{} + require.NoError(t, json.UnmarshalRead(resp.Body, &denied)) + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + assert.True(t, strings.HasPrefix(denied["message"], prefix), denied["message"]) + } + requireDenied("cache write denied:", func() (*http.Response, error) { + return clientRead.Post(base+"/caches", "application/json", strings.NewReader(`{"key":"new-key","version":"`+version+`"}`)) + }) + requireDenied("cache read denied:", func() (*http.Response, error) { + return (&http.Client{Transport: &bearerTransport{token: "token-a-write-only"}}).Get(fmt.Sprintf("%s/cache?keys=%s&version=%s", base, key, version)) + }) + + for _, client := range []*http.Client{clientB, {Transport: &bearerTransport{token: "token-a-pull"}}, clientRead} { req, err = http.NewRequest(http.MethodPatch, fmt.Sprintf("%s/caches/%d", base, id), bytes.NewReader([]byte("poison"))) require.NoError(t, err) req.Header.Set("Content-Range", "bytes 0-5/*") diff --git a/act/artifactcache/handler_v2.go b/act/artifactcache/handler_v2.go index b7ef6a32..87bdd747 100644 --- a/act/artifactcache/handler_v2.go +++ b/act/artifactcache/handler_v2.go @@ -40,12 +40,13 @@ const ( twirpInternal = "internal" twirpUnavailable = "unavailable" twirpUnauthenticated = "unauthenticated" + twirpDenied = "permission_denied" ) func (h *Handler) registerV2Routes(router *http.ServeMux) { - router.HandleFunc("POST "+cacheServiceV2Path+"/CreateCacheEntry", h.bearerAuth(h.v2CreateCacheEntry)) - router.HandleFunc("POST "+cacheServiceV2Path+"/FinalizeCacheEntryUpload", h.bearerAuth(h.v2FinalizeCacheEntryUpload)) - router.HandleFunc("POST "+cacheServiceV2Path+"/GetCacheEntryDownloadURL", h.bearerAuth(h.v2GetCacheEntryDownloadURL)) + router.HandleFunc("POST "+cacheServiceV2Path+"/CreateCacheEntry", h.cacheModeAuth(true, h.v2CreateCacheEntry)) + router.HandleFunc("POST "+cacheServiceV2Path+"/FinalizeCacheEntryUpload", h.cacheModeAuth(true, h.v2FinalizeCacheEntryUpload)) + router.HandleFunc("POST "+cacheServiceV2Path+"/GetCacheEntryDownloadURL", h.cacheModeAuth(false, h.v2GetCacheEntryDownloadURL)) router.HandleFunc("PUT "+blobPath+"/{id}", h.signedAuth(blobUploadPurpose, h.v2UploadBlob)) } @@ -267,6 +268,8 @@ func (h *Handler) twirpError(w http.ResponseWriter, r *http.Request, code string status = http.StatusServiceUnavailable case twirpUnauthenticated: status = http.StatusUnauthorized + case twirpDenied: + status = http.StatusForbidden } h.responseJSON(w, r, status, map[string]any{"code": code, "msg": err.Error()}) } diff --git a/act/artifactcache/handler_v2_test.go b/act/artifactcache/handler_v2_test.go index 2c2b76db..6f3afe39 100644 --- a/act/artifactcache/handler_v2_test.go +++ b/act/artifactcache/handler_v2_test.go @@ -227,6 +227,30 @@ func TestCacheServiceV2Lookups(t *testing.T) { assert.NotEmpty(t, got["archiveLocation"]) }) + t.Run("a read-only job only restores and a write-only one only saves", func(t *testing.T) { + handler.RegisterJob("read-only", JobCredential{Repo: testRepo, CacheMode: "read"}) + readOnlyClient := &http.Client{Transport: &bearerTransport{token: "read-only"}} + + assert.Equal(t, true, v2Call(t, handler, readOnlyClient, "GetCacheEntryDownloadURL", map[string]any{"key": "deps-abc", "version": "v1"})["ok"]) + for _, method := range []string{"CreateCacheEntry", "FinalizeCacheEntryUpload"} { + got := v2Call(t, handler, readOnlyClient, method, map[string]any{"key": "deps-new", "version": "v1", "size_bytes": 1}) + assert.Equal(t, false, got["ok"], method) + assert.True(t, strings.HasPrefix(got["message"].(string), "cache write denied:"), method) + } + + handler.RegisterJob("write-only", JobCredential{Repo: testRepo, CacheMode: "write-only"}) + writeOnlyClient := &http.Client{Transport: &bearerTransport{token: "write-only"}} + assert.Equal(t, true, v2Call(t, handler, writeOnlyClient, "CreateCacheEntry", map[string]any{"key": "deps-write-only", "version": "v1"})["ok"]) + resp, err := writeOnlyClient.Post(handler.ExternalURL()+cacheServiceV2Path+"/GetCacheEntryDownloadURL", "application/json", strings.NewReader(`{"key":"deps-abc","version":"v1"}`)) + require.NoError(t, err) + defer resp.Body.Close() + got := map[string]string{} + require.NoError(t, json.UnmarshalRead(resp.Body, &got)) + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + assert.Equal(t, twirpDenied, got["code"]) + assert.True(t, strings.HasPrefix(got["msg"], "cache read denied:"), got["msg"]) + }) + t.Run("another repository sees nothing", func(t *testing.T) { handler.RegisterJob("other-runtime-token", JobCredential{Repo: "other/repo"}) otherClient := &http.Client{Transport: &bearerTransport{token: "other-runtime-token"}} diff --git a/go.mod b/go.mod index e3e8e917..93741f06 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ toolchain go1.27.3 require ( connectrpc.com/connect/v2 v2.0.2 - gitea.dev/actionslib v1.5.2 + gitea.dev/actionslib v1.6.0 github.com/bmatcuk/doublestar/v4 v4.10.4 github.com/coder/websocket v1.8.15 github.com/containerd/errdefs v1.0.0 diff --git a/go.sum b/go.sum index 4b89a60c..0f2f15e2 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,7 @@ connectrpc.com/connect/v2 v2.0.2 h1:PoPDmOlhc46elsuCdKCKTxjNBb3HJlKF+s/sP2oucPw= connectrpc.com/connect/v2 v2.0.2/go.mod h1:N32OLrstcPUzkykFynnefFro3+xzSdOrZlK2jHmS/xo= -gitea.dev/actionslib v1.5.2 h1:PSSUd+4jIjI+ycqo62+BRgAWkBnzjgeGQAcI3ZqH7LY= -gitea.dev/actionslib v1.5.2/go.mod h1:i8wUQMnvGwT13f2re2ZTMCFL/TyauQLeOb1L4ngjtPk= +gitea.dev/actionslib v1.6.0 h1:1xKhGyJzL9CYHJ+ESqFdxetbAU+2U7YIEuPYtsS3Fuk= +gitea.dev/actionslib v1.6.0/go.mod h1:i8wUQMnvGwT13f2re2ZTMCFL/TyauQLeOb1L4ngjtPk= github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY= github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= diff --git a/internal/app/run/runner.go b/internal/app/run/runner.go index 13c8f8cc..f58d6a5c 100644 --- a/internal/app/run/runner.go +++ b/internal/app/run/runner.go @@ -543,7 +543,14 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report. // is removed when the task finishes, so a leaked token stops working as // soon as the job ends rather than remaining valid for the runner's // lifetime. - revokeCache, resultsURL := r.registerCacheForTask(giteaRuntimeToken, cacheCredential(preset, cacheURL), reporter) + cred := cacheCredential(preset, cacheURL, cmp.Or(job.CacheMode, workflow.CacheMode)) + if cacheURL != "" { + envs["ACTIONS_CACHE_MODE"] = cred.CacheMode // @actions/cache skips what the mode forbids + if cred.CacheMode != "write" { + reporter.Logf("Cache mode: %s", cred.CacheMode) + } + } + revokeCache, resultsURL := r.registerCacheForTask(giteaRuntimeToken, cred, reporter) defer revokeCache() eventJSON, err := json.Marshal(preset.Event) @@ -728,17 +735,24 @@ func (r *Runner) registerCacheForTask(token string, cred artifactcache.JobCreden } // cacheCredential scopes the job's cache entries by ref, as GitHub does. -func cacheCredential(preset *model.GithubContext, publicURL string) artifactcache.JobCredential { +func cacheCredential(preset *model.GithubContext, publicURL, cacheMode string) artifactcache.JobCredential { ref := preset.Ref - if strings.HasPrefix(preset.EventName, "pull_request") && preset.EventName != "pull_request_target" { - number, _ := preset.Event["number"].(float64) + if number, ok := preset.Event["number"].(float64); ok && strings.HasPrefix(preset.EventName, "pull_request") && preset.EventName != "pull_request_target" { ref = fmt.Sprintf("refs/pull/%d/head", int64(number)) // review events take github.ref from the reviewer's commit_id } + defaultRef := branchRef(model.AsString(model.NestedMapLookup(preset.Event, "repository", "default_branch"))) + if cacheMode == "" { // only trusted events save caches for the default branch + cacheMode = "write" + if ref == defaultRef && !slices.Contains([]string{"push", "workflow_dispatch", "schedule", "delete", "registry_package"}, preset.EventName) { + cacheMode = "read" + } + } return artifactcache.JobCredential{ Repo: preset.Repository, Ref: ref, BaseRef: branchRef(preset.BaseRef), - DefaultRef: branchRef(model.AsString(model.NestedMapLookup(preset.Event, "repository", "default_branch"))), + DefaultRef: defaultRef, + CacheMode: cacheMode, PublicURL: publicURL, } } diff --git a/internal/app/run/runner_cache_test.go b/internal/app/run/runner_cache_test.go index b33b2db7..d7fd854e 100644 --- a/internal/app/run/runner_cache_test.go +++ b/internal/app/run/runner_cache_test.go @@ -120,15 +120,29 @@ func TestCacheCredential(t *testing.T) { Event: map[string]any{"repository": repository, "number": float64(3)}, } assert.Equal(t, artifactcache.JobCredential{ - Repo: "owner/repo", Ref: "refs/pull/3/head", BaseRef: "refs/heads/release", DefaultRef: "refs/heads/main", PublicURL: "http://cache", - }, cacheCredential(review, "http://cache")) + Repo: "owner/repo", Ref: "refs/pull/3/head", BaseRef: "refs/heads/release", DefaultRef: "refs/heads/main", CacheMode: "write", PublicURL: "http://cache", + }, cacheCredential(review, "http://cache", "")) review.EventName, review.Ref = "pull_request_target", "refs/heads/release" - assert.Equal(t, "refs/heads/release", cacheCredential(review, "").Ref) + assert.Equal(t, "refs/heads/release", cacheCredential(review, "", "").Ref) + assert.Equal(t, "write", cacheCredential(review, "", "").CacheMode) assert.Equal(t, artifactcache.JobCredential{ - Repo: "owner/repo", Ref: "refs/heads/feature", DefaultRef: "refs/heads/main", - }, cacheCredential(&model.GithubContext{Repository: "owner/repo", Ref: "refs/heads/feature", Event: map[string]any{"repository": repository}}, "")) + Repo: "owner/repo", Ref: "refs/heads/feature", DefaultRef: "refs/heads/main", CacheMode: "write", + }, cacheCredential(&model.GithubContext{Repository: "owner/repo", Ref: "refs/heads/feature", Event: map[string]any{"repository": repository}}, "", "")) + + for _, tc := range [][3]string{ + {"pull_request_target", "", "read"}, + {"issue_comment", "", "read"}, + {"pull_request_comment", "", "read"}, + {"push", "", "write"}, + {"issue_comment", "write", "write"}, + {"push", "none", "none"}, + } { + cred := cacheCredential(&model.GithubContext{EventName: tc[0], Ref: "refs/heads/main", Event: map[string]any{"repository": repository}}, "", tc[1]) + assert.Equal(t, "refs/heads/main", cred.Ref, tc) + assert.Equal(t, tc[2], cred.CacheMode, tc) + } } func TestRunner_ExternalCacheServer_RegisterRevoke(t *testing.T) {