fix: PRs cache artifacts separate from other runs

act/artifactcache/handler.go

@@ -181,11 +181,19 @@ func (h *Handler) find(w http.ResponseWriter, r *http.Request, params httprouter
 	}
 	defer db.Close()
 
-	cache, err := findCache(db, repo, keys, version)
+	cache, err := findCache(db, repo, keys, version, rundata.WriteIsolationKey)
 	if err != nil {
 		h.responseJSON(w, r, 500, err)
 		return
 	}
+	// If read was scoped to WriteIsolationKey and didn't find anything, we can fallback to the non-isolated cache read
+	if cache == nil && rundata.WriteIsolationKey != "" {
+		cache, err = findCache(db, repo, keys, version, "")
+		if err != nil {
+			h.responseJSON(w, r, 500, err)
+			return
+		}
+	}
 	if cache == nil {
 		h.responseJSON(w, r, 204)
 		return
@@ -236,6 +244,7 @@ func (h *Handler) reserve(w http.ResponseWriter, r *http.Request, params httprou
 	cache.CreatedAt = now
 	cache.UsedAt = now
 	cache.Repo = repo
+	cache.WriteIsolationKey = rundata.WriteIsolationKey
 	if err := insertCache(db, cache); err != nil {
 		h.responseJSON(w, r, 500, err)
 		return
@@ -275,6 +284,10 @@ func (h *Handler) upload(w http.ResponseWriter, r *http.Request, params httprout
 		h.responseJSON(w, r, 500, fmt.Errorf("cache repo is not valid"))
 		return
 	}
+	if cache.WriteIsolationKey != rundata.WriteIsolationKey {
+		h.responseJSON(w, r, 403, fmt.Errorf("cache authorized for write isolation %q, but attempting to operate on %q", rundata.WriteIsolationKey, cache.WriteIsolationKey))
+		return
+	}
 
 	if cache.Complete {
 		h.responseJSON(w, r, 400, fmt.Errorf("cache %v %q: already complete", cache.ID, cache.Key))
@@ -326,6 +339,10 @@ func (h *Handler) commit(w http.ResponseWriter, r *http.Request, params httprout
 		h.responseJSON(w, r, 500, fmt.Errorf("cache repo is not valid"))
 		return
 	}
+	if cache.WriteIsolationKey != rundata.WriteIsolationKey {
+		h.responseJSON(w, r, 403, fmt.Errorf("cache authorized for write isolation %q, but attempting to operate on %q", rundata.WriteIsolationKey, cache.WriteIsolationKey))
+		return
+	}
 
 	if cache.Complete {
 		h.responseJSON(w, r, 400, fmt.Errorf("cache %v %q: already complete", cache.ID, cache.Key))
@@ -386,6 +403,11 @@ func (h *Handler) get(w http.ResponseWriter, r *http.Request, params httprouter.
 		h.responseJSON(w, r, 500, fmt.Errorf("cache repo is not valid"))
 		return
 	}
+	// reads permitted against caches w/ the same isolation key, or no isolation key
+	if cache.WriteIsolationKey != rundata.WriteIsolationKey && cache.WriteIsolationKey != "" {
+		h.responseJSON(w, r, 403, fmt.Errorf("cache authorized for write isolation %q, but attempting to operate on %q", rundata.WriteIsolationKey, cache.WriteIsolationKey))
+		return
+	}
 
 	if err := h.useCache(id); err != nil {
 		h.responseJSON(w, r, 500, fmt.Errorf("cache useCache: %w", err))
@@ -417,7 +439,7 @@ func (h *Handler) middleware(handler httprouter.Handle) httprouter.Handle {
 }
 
 // if not found, return (nil, nil) instead of an error.
-func findCache(db *bolthold.Store, repo string, keys []string, version string) (*Cache, error) {
+func findCache(db *bolthold.Store, repo string, keys []string, version, writeIsolationKey string) (*Cache, error) {
 	cache := &Cache{}
 	for _, prefix := range keys {
 		// if a key in the list matches exactly, don't return partial matches
@@ -425,6 +447,7 @@ func findCache(db *bolthold.Store, repo string, keys []string, version string) (
 			bolthold.Where("Repo").Eq(repo).Index("Repo").
 				And("Key").Eq(prefix).
 				And("Version").Eq(version).
+				And("WriteIsolationKey").Eq(writeIsolationKey).
 				And("Complete").Eq(true).
 				SortBy("CreatedAt").Reverse()); err == nil || !errors.Is(err, bolthold.ErrNotFound) {
 			if err != nil {
@@ -441,6 +464,7 @@ func findCache(db *bolthold.Store, repo string, keys []string, version string) (
 			bolthold.Where("Repo").Eq(repo).Index("Repo").
 				And("Key").RegExp(re).
 				And("Version").Eq(version).
+				And("WriteIsolationKey").Eq(writeIsolationKey).
 				And("Complete").Eq(true).
 				SortBy("CreatedAt").Reverse()); err != nil {
 			if errors.Is(err, bolthold.ErrNotFound) {
@@ -644,5 +668,6 @@ func runDataFromHeaders(r *http.Request) cacheproxy.RunData {
 		RunNumber:          r.Header.Get("Forgejo-Cache-RunNumber"),
 		Timestamp:          r.Header.Get("Forgejo-Cache-Timestamp"),
 		RepositoryMAC:      r.Header.Get("Forgejo-Cache-MAC"),
+		WriteIsolationKey:  r.Header.Get("Forgejo-Cache-WriteIsolationKey"),
 	}
 }

act/artifactcache/model.go

@@ -32,4 +32,5 @@ type Cache struct {
 	Complete          bool   `json:"complete"`
 	UsedAt            int64  `json:"usedAt"`
 	CreatedAt         int64  `json:"createdAt"`
+	WriteIsolationKey string `json:"writeIsolationKey"`
 }

act/cacheproxy/handler.go

@@ -51,15 +51,17 @@ type RunData struct {
 	RunNumber          string
 	Timestamp          string
 	RepositoryMAC      string
+	WriteIsolationKey  string
 }
 
-func (h *Handler) CreateRunData(fullName, runNumber, timestamp string) RunData {
+func (h *Handler) CreateRunData(fullName, runNumber, timestamp, writeIsolationKey string) RunData {
 	mac := computeMac(h.cacheSecret, fullName, runNumber, timestamp)
 	return RunData{
 		RepositoryFullName: fullName,
 		RunNumber:          runNumber,
 		Timestamp:          timestamp,
 		RepositoryMAC:      mac,
+		WriteIsolationKey:  writeIsolationKey,
 	}
 }
 
@@ -152,6 +154,9 @@ func (h *Handler) newReverseProxy(targetHost string) (*httputil.ReverseProxy, er
 			r.Out.Header.Set("Forgejo-Cache-Timestamp", runData.Timestamp)
 			r.Out.Header.Set("Forgejo-Cache-MAC", runData.RepositoryMAC)
 			r.Out.Header.Set("Forgejo-Cache-Host", h.ExternalURL())
+			if runData.WriteIsolationKey != "" {
+				r.Out.Header.Set("Forgejo-Cache-WriteIsolationKey", runData.WriteIsolationKey)
+			}
 		},
 	}
 	return proxy, nil

internal/app/run/runner.go

@@ -265,8 +265,23 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report.
 
 	// Register the run with the cacheproxy and modify the CACHE_URL
 	if r.cacheProxy != nil {
+		writeIsolationKey := ""
+
+		// When performing an action on an event from a PR, provide a "write isolation key" to the cache. The generated
+		// ACTIONS_CACHE_URL will be able to read the cache, and write to a cache, but its writes will be isolated to
+		// future runs of the PR's workflows and won't be shared with other pull requests or actions. This is a security
+		// measure to prevent a malicious pull request from poisoning the cache with secret-stealing code which would
+		// later be executed on another action.
+		if taskContext["event_name"].GetStringValue() == "pull_request" || taskContext["event_name"].GetStringValue() == "pull_request_target" {
+			// Ensure that `Ref` has the expected format so that we don't end up with a useless write isolation key
+			if !strings.HasPrefix(preset.Ref, "refs/pull/") {
+				return fmt.Errorf("write isolation key: expected preset.Ref to be refs/pull/..., but was %q", preset.Ref)
+			}
+			writeIsolationKey = preset.Ref
+		}
+
 		timestamp := strconv.FormatInt(time.Now().Unix(), 10)
-		cacheRunData := r.cacheProxy.CreateRunData(preset.Repository, preset.RunID, timestamp)
+		cacheRunData := r.cacheProxy.CreateRunData(preset.Repository, preset.RunID, timestamp, writeIsolationKey)
 		cacheRunID, err := r.cacheProxy.AddRun(cacheRunData)
 		if err == nil {
 			defer func() { _ = r.cacheProxy.RemoveRun(cacheRunID) }()