diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index f7baeb0650b..ceef3d54661 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -2077,8 +2077,8 @@ func buildSignature(endp string, expires, artifactID int64) []byte { return actions.BuildSignature("api", endp, strconv.FormatInt(expires, 10), strconv.FormatInt(artifactID, 10)) } -func buildDownloadRawEndpoint(repo *repo_model.Repository, artifactID int64) string { - return fmt.Sprintf("api/v1/repos/%s/%s/actions/artifacts/%d/zip/raw", url.PathEscape(repo.OwnerName), url.PathEscape(repo.Name), artifactID) +func buildDownloadRawEndpoint(ownerName, repoName string, artifactID int64) string { + return fmt.Sprintf("api/v1/repos/%s/%s/actions/artifacts/%d/zip/raw", url.PathEscape(ownerName), url.PathEscape(repoName), artifactID) } func buildSigURL(ctx go_context.Context, endPoint string, artifactID int64) string { @@ -2139,7 +2139,7 @@ func DownloadArtifact(ctx *context.APIContext) { // @actions/toolkit asserts a 302 for the artifact download, so we have to build a signed URL and redirect to it // TODO: a perma link to the code for reference - redirectURL := buildSigURL(ctx, buildDownloadRawEndpoint(ctx.Repo.Repository, art.ID), art.ID) + redirectURL := buildSigURL(ctx, buildDownloadRawEndpoint(ctx.Repo.Repository.OwnerName, ctx.Repo.Repository.Name, art.ID), art.ID) ctx.Redirect(redirectURL, http.StatusFound) return } @@ -2150,7 +2150,22 @@ func DownloadArtifact(ctx *context.APIContext) { // DownloadArtifactRaw Downloads a specific artifact for a workflow run directly. func DownloadArtifactRaw(ctx *context.APIContext) { // it doesn't use repoAssignment middleware, so it needs to prepare the repo and check permission (sig) by itself - repo, err := repo_model.GetRepositoryByOwnerAndName(ctx, ctx.PathParam("username"), ctx.PathParam("reponame")) + ownerName, repoName := ctx.PathParam("username"), ctx.PathParam("reponame") + query := ctx.Req.URL.Query() + sigBytes, _ := base64.RawURLEncoding.DecodeString(query.Get("sig")) + expires, _ := strconv.ParseInt(query.Get("expires"), 10, 64) + artifactID := ctx.PathParamInt64("artifact_id") + + if !hmac.Equal(sigBytes, buildSignature(buildDownloadRawEndpoint(ownerName, repoName, artifactID), expires, artifactID)) { + ctx.APIErrorNotFound() + return + } + if time.Unix(expires, 0).Before(time.Now()) { + ctx.APIError(http.StatusUnauthorized, "Error link expired") + return + } + + repo, err := repo_model.GetRepositoryByOwnerAndName(ctx, ownerName, repoName) if err != nil { if errors.Is(err, util.ErrNotExist) { ctx.APIErrorNotFound() @@ -2164,22 +2179,6 @@ func DownloadArtifactRaw(ctx *context.APIContext) { return } - sigStr := ctx.Req.URL.Query().Get("sig") - expiresStr := ctx.Req.URL.Query().Get("expires") - sigBytes, _ := base64.RawURLEncoding.DecodeString(sigStr) - expires, _ := strconv.ParseInt(expiresStr, 10, 64) - - expectedSig := buildSignature(buildDownloadRawEndpoint(repo, art.ID), expires, art.ID) - if !hmac.Equal(sigBytes, expectedSig) { - ctx.APIError(http.StatusUnauthorized, "Error unauthorized") - return - } - t := time.Unix(expires, 0) - if t.Before(time.Now()) { - ctx.APIError(http.StatusUnauthorized, "Error link expired") - return - } - // if artifacts status is not uploaded-confirmed, treat it as not found if art.Status == actions_model.ArtifactStatusExpired { ctx.APIError(http.StatusNotFound, "Artifact has expired") diff --git a/tests/integration/api_actions_artifact_v4_test.go b/tests/integration/api_actions_artifact_v4_test.go index 16f8f6f4dad..5acc58669bf 100644 --- a/tests/integration/api_actions_artifact_v4_test.go +++ b/tests/integration/api_actions_artifact_v4_test.go @@ -662,10 +662,25 @@ func TestActionsArtifactV4DownloadSinglePublicApi(t *testing.T) { body := strings.Repeat("D", 1024) assert.Equal(t, body, resp.Body.String()) - // confirm artifact can not be downloaded without query - req = NewRequestWithBody(t, "GET", blobLocation, nil) + req = NewRequestWithBody(t, "GET", blobLocation, nil).AddTokenAuth(token) req.URL.RawQuery = "" - _ = MakeRequest(t, req, http.StatusUnauthorized) + notFoundResp := MakeRequest(t, req, http.StatusNotFound) + + otherRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + require.NotEqual(t, repo.ID, otherRepo.ID) + for _, testCase := range []struct { + name string + path string + }{ + {"other repository", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d/zip/raw", otherRepo.FullName(), listResp.Entries[0].ID)}, + {"missing artifact", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/0/zip/raw", repo.FullName())}, + {"missing repository", fmt.Sprintf("/api/v1/repos/%s/does-not-exist/actions/artifacts/%d/zip/raw", repo.OwnerName, listResp.Entries[0].ID)}, + } { + t.Run(testCase.name, func(t *testing.T) { + invalidResp := MakeRequest(t, NewRequestWithBody(t, "GET", testCase.path, nil), http.StatusNotFound) + assert.Equal(t, notFoundResp.Body.String(), invalidResp.Body.String()) + }) + } } func TestActionsArtifactV4DownloadSinglePublicApiPrivateRepo(t *testing.T) { @@ -703,7 +718,7 @@ func TestActionsArtifactV4DownloadSinglePublicApiPrivateRepo(t *testing.T) { // confirm artifact can not be downloaded without query req = NewRequestWithBody(t, "GET", blobLocation, nil) req.URL.RawQuery = "" - _ = MakeRequest(t, req, http.StatusUnauthorized) + _ = MakeRequest(t, req, http.StatusNotFound) } func TestActionsArtifactV4ListAndGetPublicApi(t *testing.T) { @@ -783,20 +798,6 @@ func TestActionsArtifactV4DownloadArtifactCorrectRepoFound(t *testing.T) { MakeRequest(t, req, http.StatusFound) } -func TestActionsArtifactV4DownloadRawArtifactCorrectRepoMissingSignatureUnauthorized(t *testing.T) { - defer prepareTestEnvActionsArtifacts(t)() - - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) - session := loginUser(t, user.Name) - token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) - - // confirm cannot use the raw artifact endpoint even with a correct access token - req := NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d/zip/raw", repo.FullName(), 22), nil). - AddTokenAuth(token) - MakeRequest(t, req, http.StatusUnauthorized) -} - func TestActionsArtifactV4Delete(t *testing.T) { defer prepareTestEnvActionsArtifacts(t)()