fix: stop artifact uploads depending on the cache server reaching Gitea (#1216)

Cache v2 makes the cache server the `ACTIONS_RESULTS_URL` origin, so artifact calls arrived there and were proxied on to Gitea, failing whenever it could not reach the instance.

- Artifact calls are answered with a redirect, so the cache server opens no connection to Gitea. A scheme change or an untrusted instance is still proxied, but there the cache server is the runner itself, which already reaches Gitea.
- Failures answer in twirp, not an empty `502` that clients report as `Unexpected end of JSON input`.
- `cache.v2: false` really points artifacts at Gitea now.
- Cache reservations are bound to the job that made them, so two jobs saving one key cannot commit against each other's upload, and a retry after a lost answer no longer fails a saved entry.
- The toolkit patch, which edits the GitHub-host check out of an action's bundle, was left in the shared checkout where a job running with `runner.patch_actions: false` could inherit it. It is put back after the job's copy.
- `exec` names an origin for the cache v2 it advertises, and masks its runtime token.

Behaviour changes: `no_proxy` no longer exempts `cache.external_server`, and `cache.enabled: false` also stops external registration.

Fixes https://gitea.com/gitea/runner/issues/1208
Fixes https://gitea.com/gitea/runner/issues/1211

Assisted by Claude (Opus 5).

Reviewed-on: https://gitea.com/gitea/runner/pulls/1216
Reviewed-by: bircni <bircni@icloud.com>
Co-authored-by: silverwind <me@silverwind.io>

act/artifactcache/results_test.go

@@ -4,6 +4,8 @@
 package artifactcache
 
 import (
+	"cmp"
+	"encoding/json/v2"
 	"io"
 	"net/http"
 	"net/http/httptest"
@@ -14,8 +16,6 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// The artifact half is forwarded under the Host Gitea knows itself by, so the URLs it hands back
-// still point at Gitea, and nothing else is proxied.
 func TestFrontResultsService(t *testing.T) {
 	var gotHost, gotPath, gotProto string
 	gitea := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -23,34 +23,112 @@ func TestFrontResultsService(t *testing.T) {
 		_, _ = io.WriteString(w, `{"ok":true}`)
 	}))
 	defer gitea.Close()
+	selfSigned := httptest.NewTLSServer(gitea.Config.Handler)
+	defer selfSigned.Close()
 
 	handler, err := StartHandler(Options{Dir: t.TempDir(), OutboundIP: "127.0.0.1"})
 	require.NoError(t, err)
 	defer handler.Close()
-	const token = "forward-token"
+	const token, rpc = "forward-token", artifactServicePath + "CreateArtifact"
 
-	client := &http.Client{Transport: &bearerTransport{token: token}}
-	post := func(path string) int {
+	noRedirect := func(token string) *http.Client {
+		return &http.Client{
+			Transport:     &bearerTransport{token: token},
+			CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse },
+		}
+	}
+	post := func(path string) (int, map[string]string) {
 		req, err := http.NewRequestWithContext(t.Context(), http.MethodPost, handler.ExternalURL()+path, nil)
 		require.NoError(t, err)
-		resp, err := client.Do(req)
+		resp, err := noRedirect(token).Do(req)
 		require.NoError(t, err)
-		resp.Body.Close()
-		return resp.StatusCode
+		defer resp.Body.Close()
+		body := map[string]string{}
+		_ = json.UnmarshalRead(resp.Body, &body)
+		return resp.StatusCode, body
 	}
 
-	assert.Equal(t, http.StatusNotFound, post(artifactServicePath+"CreateArtifact"),
-		"an unregistered token is forwarded nowhere")
+	status, body := post(rpc)
+	assert.Equal(t, http.StatusUnauthorized, status, "an unregistered token is told so, not left to parse a 404 page")
+	assert.Equal(t, twirpUnauthenticated, body["code"])
+	assert.Contains(t, body["msg"], "bearer")
+	assert.NotEmpty(t, body["msg"])
 
-	defer handler.RegisterJob(token, JobCredential{Repo: "owner/repo", Results: gitea.URL})()
+	defer handler.RegisterJob(token, JobCredential{Repo: "owner/repo", Results: gitea.URL, PublicURL: "https://cache.example"})()
 
-	assert.Equal(t, http.StatusOK, post(artifactServicePath+"CreateArtifact"))
+	status, _ = post(rpc)
+	require.Equal(t, http.StatusOK, status, "a registered https public URL onto an http instance must proxy, not redirect")
 	assert.Equal(t, strings.TrimPrefix(gitea.URL, "http://"), gotHost, "Gitea must see the host it mints its URLs from")
 	assert.Empty(t, gotProto, "a forwarded scheme would make an https Gitea mint http URLs")
-	assert.Equal(t, artifactServicePath+"CreateArtifact", gotPath)
+	assert.Equal(t, rpc, gotPath)
 
 	gotPath = ""
-	assert.Equal(t, http.StatusNotFound, post("/twirp/github.actions.results.api.v1.OtherService/Do"))
-	assert.Equal(t, http.StatusNotFound, post("/api/v1/repos/owner/repo"))
+	status, _ = post("/twirp/github.actions.results.api.v1.OtherService/Do")
+	assert.Equal(t, http.StatusNotFound, status)
+	status, _ = post("/api/v1/repos/owner/repo")
+	assert.Equal(t, http.StatusNotFound, status)
 	assert.Empty(t, gotPath, "only the artifact service is forwarded")
+
+	tests := []struct {
+		name           string
+		cred           JobCredential
+		forwardedProto string
+		reqQuery       string
+		want           int
+		location       string
+		wantMsg        string
+	}{
+		{name: "an instance the job reaches itself", cred: JobCredential{Results: gitea.URL}, want: http.StatusTemporaryRedirect},
+		{name: "a sub-path instance keeps its prefix", cred: JobCredential{Results: gitea.URL + "/sub"}, want: http.StatusTemporaryRedirect},
+		{name: "an instance with no certificate to distrust", cred: JobCredential{Results: gitea.URL, InsecureTLS: true}, want: http.StatusTemporaryRedirect},
+		{
+			name: "both queries survive", cred: JobCredential{Results: gitea.URL + "?tenant=one"}, reqQuery: "?run=7",
+			want: http.StatusTemporaryRedirect, location: gitea.URL + rpc + "?tenant=one&run=7",
+		},
+		{
+			name: "a trusted https instance is reached by the job itself",
+			cred: JobCredential{Results: selfSigned.URL, PublicURL: "https://cache.example"}, want: http.StatusTemporaryRedirect,
+		},
+		{
+			name: "the job does not share this server's disregard for the certificate",
+			cred: JobCredential{Results: selfSigned.URL, PublicURL: "https://cache.example", InsecureTLS: true}, want: http.StatusOK,
+		},
+		{name: "an http job is not redirected to an https instance", cred: JobCredential{Results: selfSigned.URL}, want: http.StatusInternalServerError},
+		{
+			name: "a runner too old to send its public URL leaves the terminator to say so",
+			cred: JobCredential{Results: gitea.URL}, forwardedProto: "https", want: http.StatusOK,
+		},
+		{
+			name: "a scheme the job could not use is refused before it is sent one",
+			cred: JobCredential{Results: "ftp://gitea.example"}, want: http.StatusInternalServerError, wantMsg: "unusable",
+		},
+		{
+			name: "an address with no host is refused before it is sent one",
+			cred: JobCredential{Results: "http://:3000"}, want: http.StatusInternalServerError, wantMsg: "unusable",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			defer handler.RegisterJob(tt.name, tt.cred)()
+			req, err := http.NewRequestWithContext(t.Context(), http.MethodPost, handler.ExternalURL()+rpc+tt.reqQuery, nil)
+			require.NoError(t, err)
+			if tt.forwardedProto != "" {
+				req.Header.Set("X-Forwarded-Proto", tt.forwardedProto)
+			}
+			resp, err := noRedirect(tt.name).Do(req)
+			require.NoError(t, err)
+			defer resp.Body.Close()
+
+			require.Equal(t, tt.want, resp.StatusCode)
+			switch tt.want {
+			case http.StatusTemporaryRedirect:
+				assert.Equal(t, cmp.Or(tt.location, tt.cred.Results+rpc), resp.Header.Get("Location"))
+			case http.StatusInternalServerError:
+				body := map[string]string{}
+				require.NoError(t, json.UnmarshalRead(resp.Body, &body))
+				assert.Equal(t, twirpInternal, body["code"])
+				assert.Contains(t, body["msg"], cmp.Or(tt.wantMsg, tt.cred.Results))
+			}
+		})
+	}
 }