diff --git a/act/artifactcache/handler_test.go b/act/artifactcache/handler_test.go index cf0e11df..ebfdad77 100644 --- a/act/artifactcache/handler_test.go +++ b/act/artifactcache/handler_test.go @@ -369,6 +369,33 @@ func TestHandler(t *testing.T) { require.Equal(t, 404, resp.StatusCode) }) + t.Run("get with bad MAC", func(t *testing.T) { + key := strings.ToLower(t.Name()) + version := "c19da02a2bd7e77277f1ac29ab45c09b7d46b4ee758284e26bb3045ad11d9d20" + content := make([]byte, 100) + _, err := rand.Read(content) + require.NoError(t, err) + + uploadCacheNormally(t, base, key, version, content) + + // Perform the request with the custom `httpClient` which will send correct MAC data + resp, err := httpClient.Get(fmt.Sprintf("%s/cache?keys=%s&version=%s", base, key, version)) + require.NoError(t, err) + require.Equal(t, 200, resp.StatusCode) + + // Perform the same request with incorrect MAC data + req, err := http.NewRequest("GET", fmt.Sprintf("%s/cache?keys=%s&version=%s", base, key, version), nil) + require.NoError(t, err) + req.Header.Set("Forgejo-Cache-Repo", cache_repo) + req.Header.Set("Forgejo-Cache-RunNumber", cache_runnum) + req.Header.Set("Forgejo-Cache-Timestamp", cache_timestamp) + req.Header.Set("Forgejo-Cache-MAC", "33f0e850ba0bdfd2f3e66ff79c1f8004b8226114e3b2e65c229222bb59df0f9d") // ! This is not the correct MAC + req.Header.Set("Forgejo-Cache-Host", handlerExternalUrl) + resp, err = http.Get(fmt.Sprintf("%s/cache?keys=%s&version=%s", base, key, version)) + require.NoError(t, err) + require.Equal(t, 403, resp.StatusCode) + }) + t.Run("get with multiple keys", func(t *testing.T) { version := "c19da02a2bd7e77277f1ac29ab45c09b7d46a4ee758284e26bb3045ad11d9d20" key := strings.ToLower(t.Name()) diff --git a/act/artifactcache/mac.go b/act/artifactcache/mac.go index 1b129b77..16f46c26 100644 --- a/act/artifactcache/mac.go +++ b/act/artifactcache/mac.go @@ -28,7 +28,7 @@ func (h *Handler) validateMac(rundata cacheproxy.RunData) (string, error) { if hmac.Equal([]byte(expectedMAC), []byte(rundata.RepositoryMAC)) { return rundata.RepositoryFullName, nil } - return rundata.RepositoryFullName, ErrValidation + return "", ErrValidation } func validateAge(ts string) bool { diff --git a/act/artifactcache/mac_test.go b/act/artifactcache/mac_test.go index 2087ccc2..b59280c7 100644 --- a/act/artifactcache/mac_test.go +++ b/act/artifactcache/mac_test.go @@ -63,7 +63,7 @@ func TestMac(t *testing.T) { repoName, err := handler.validateMac(rundata) require.Error(t, err) - require.Equal(t, name, repoName) + require.Equal(t, "", repoName) }) t.Run("compute correct mac", func(t *testing.T) { diff --git a/act/cacheproxy/handler.go b/act/cacheproxy/handler.go index 13997ef2..bd047926 100644 --- a/act/cacheproxy/handler.go +++ b/act/cacheproxy/handler.go @@ -78,7 +78,6 @@ func StartHandler(targetHost string, outboundIP string, port uint16, cacheSecret h.logger = logger h.cacheSecret = cacheSecret - // h.runs = make(map[string]RunData) if outboundIP != "" { h.outboundIP = outboundIP @@ -125,9 +124,7 @@ func StartHandler(targetHost string, outboundIP string, port uint16, cacheSecret } func proxyRequestHandler(proxy *httputil.ReverseProxy) func(http.ResponseWriter, *http.Request) { - return func(w http.ResponseWriter, r *http.Request) { - proxy.ServeHTTP(w, r) - } + return proxy.ServeHTTP } func (h *Handler) newReverseProxy(targetHost string) (*httputil.ReverseProxy, error) { @@ -141,13 +138,12 @@ func (h *Handler) newReverseProxy(targetHost string) (*httputil.ReverseProxy, er matches := urlRegex.FindStringSubmatch(r.In.URL.Path) id := matches[1] data, ok := h.runs.Load(id) - var runData = data.(RunData) if !ok { // The ID doesn't exist. - // ! this should probably be handled more gracefully but i can't figure out how - // ! it really shouldn't happen anyway so it's fine for now + h.logger.Warn(fmt.Sprintf("Tried starting a cache proxy with id %s, which does not exist.", id)) return } + var runData = data.(RunData) uri := matches[2] r.SetURL(targetURL) @@ -212,10 +208,7 @@ func (h *Handler) Close() error { } if h.listener != nil { err := h.listener.Close() - if errors.Is(err, net.ErrClosed) { - err = nil - } - if err != nil { + if !errors.Is(err, net.ErrClosed) { retErr = err } h.listener = nil