Artifacts download api for artifact actions v4

cmd/migrate_storage.go

@@ -196,7 +196,7 @@ func migrateActionsLog(ctx context.Context, dstStorage storage.ObjectStorage) er
 
 func migrateActionsArtifacts(ctx context.Context, dstStorage storage.ObjectStorage) error {
 	return db.Iterate(ctx, nil, func(ctx context.Context, artifact *actions_model.ActionArtifact) error {
-		if artifact.Status == int64(actions_model.ArtifactStatusExpired) {
+		if artifact.Status == actions_model.ArtifactStatusExpired {
 			return nil
 		}
 

models/actions/artifact.go

@@ -48,7 +48,7 @@ type ActionArtifact struct {
 	ContentEncoding    string             // The content encoding of the artifact
 	ArtifactPath       string             `xorm:"index unique(runid_name_path)"` // The path to the artifact when runner uploads it
 	ArtifactName       string             `xorm:"index unique(runid_name_path)"` // The name of the artifact when runner uploads it
-	Status             int64              `xorm:"index"`                         // The status of the artifact, uploading, expired or need-delete
+	Status             ArtifactStatus     `xorm:"index"`                         // The status of the artifact, uploading, expired or need-delete
 	CreatedUnix        timeutil.TimeStamp `xorm:"created"`
 	UpdatedUnix        timeutil.TimeStamp `xorm:"updated index"`
 	ExpiredUnix        timeutil.TimeStamp `xorm:"index"` // The time when the artifact will be expired
@@ -68,7 +68,7 @@ func CreateArtifact(ctx context.Context, t *ActionTask, artifactName, artifactPa
 			RepoID:       t.RepoID,
 			OwnerID:      t.OwnerID,
 			CommitSHA:    t.CommitSHA,
-			Status:       int64(ArtifactStatusUploadPending),
+			Status:       ArtifactStatusUploadPending,
 			ExpiredUnix:  timeutil.TimeStamp(time.Now().Unix() + timeutil.Day*expiredDays),
 		}
 		if _, err := db.GetEngine(ctx).Insert(artifact); err != nil {
@@ -177,18 +177,18 @@ func ListPendingDeleteArtifacts(ctx context.Context, limit int) ([]*ActionArtifa
 
 // SetArtifactExpired sets an artifact to expired
 func SetArtifactExpired(ctx context.Context, artifactID int64) error {
-	_, err := db.GetEngine(ctx).Where("id=? AND status = ?", artifactID, ArtifactStatusUploadConfirmed).Cols("status").Update(&ActionArtifact{Status: int64(ArtifactStatusExpired)})
+	_, err := db.GetEngine(ctx).Where("id=? AND status = ?", artifactID, ArtifactStatusUploadConfirmed).Cols("status").Update(&ActionArtifact{Status: ArtifactStatusExpired})
 	return err
 }
 
 // SetArtifactNeedDelete sets an artifact to need-delete, cron job will delete it
 func SetArtifactNeedDelete(ctx context.Context, runID int64, name string) error {
-	_, err := db.GetEngine(ctx).Where("run_id=? AND artifact_name=? AND status = ?", runID, name, ArtifactStatusUploadConfirmed).Cols("status").Update(&ActionArtifact{Status: int64(ArtifactStatusPendingDeletion)})
+	_, err := db.GetEngine(ctx).Where("run_id=? AND artifact_name=? AND status = ?", runID, name, ArtifactStatusUploadConfirmed).Cols("status").Update(&ActionArtifact{Status: ArtifactStatusPendingDeletion})
 	return err
 }
 
 // SetArtifactDeleted sets an artifact to deleted
 func SetArtifactDeleted(ctx context.Context, artifactID int64) error {
-	_, err := db.GetEngine(ctx).ID(artifactID).Cols("status").Update(&ActionArtifact{Status: int64(ArtifactStatusDeleted)})
+	_, err := db.GetEngine(ctx).ID(artifactID).Cols("status").Update(&ActionArtifact{Status: ArtifactStatusDeleted})
 	return err
 }

routers/api/actions/artifacts_chunks.go

@@ -292,7 +292,7 @@ func mergeChunksForArtifact(ctx *ArtifactContext, chunks []*chunkFileItem, st st
 	}
 
 	artifact.StoragePath = storagePath
-	artifact.Status = int64(actions.ArtifactStatusUploadConfirmed)
+	artifact.Status = actions.ArtifactStatusUploadConfirmed
 	if err := actions.UpdateArtifactByID(ctx, artifact.ID, artifact); err != nil {
 		return fmt.Errorf("update artifact error: %v", err)
 	}

routers/api/v1/api.go

@@ -1409,6 +1409,7 @@ func Routes() *web.Router {
 		}, tokenRequiresScopes(auth_model.AccessTokenScopeCategoryRepository))
 
 		// Artifacts direct download endpoint authenticates via signed url
+		// it is protected by the "sig" parameter (to help to access private repo), so no need to use other middlewares
 		m.Get("/repos/{username}/{reponame}/actions/artifacts/{artifact_id}/zip/raw", repo.DownloadArtifactRaw)
 
 		// Notifications (requires notifications scope)

routers/api/v1/repo/action.go

@@ -17,10 +17,10 @@ import (
 
 	actions_model "code.gitea.io/gitea/models/actions"
 	"code.gitea.io/gitea/models/db"
+	repo_model "code.gitea.io/gitea/models/repo"
 	secret_model "code.gitea.io/gitea/models/secret"
 	"code.gitea.io/gitea/modules/actions"
 	"code.gitea.io/gitea/modules/httplib"
-	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/util"
@@ -1028,8 +1028,8 @@ func GetArtifact(ctx *context.APIContext) {
 	//   "404":
 	//     "$ref": "#/responses/notFound"
 
-	art, ok := getArtifactByID(ctx)
-	if !ok {
+	art := getArtifactByPathParam(ctx, ctx.Repo.Repository)
+	if ctx.Written() {
 		return
 	}
 
@@ -1043,7 +1043,7 @@ func GetArtifact(ctx *context.APIContext) {
 		return
 	}
 	// v3 not supported due to not having one unique id
-	ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found"))
+	ctx.Error(http.StatusNotFound, "GetArtifact", "Artifact not found")
 }
 
 // DeleteArtifact Deletes a specific artifact for a workflow run.
@@ -1077,21 +1077,21 @@ func DeleteArtifact(ctx *context.APIContext) {
 	//   "404":
 	//     "$ref": "#/responses/notFound"
 
-	art, ok := getArtifactByID(ctx)
-	if !ok {
+	art := getArtifactByPathParam(ctx, ctx.Repo.Repository)
+	if ctx.Written() {
 		return
 	}
 
 	if actions.IsArtifactV4(art) {
 		if err := actions_model.SetArtifactNeedDelete(ctx, art.RunID, art.ArtifactName); err != nil {
-			ctx.Error(http.StatusInternalServerError, err.Error(), err)
+			ctx.Error(http.StatusInternalServerError, "DeleteArtifact", err)
 			return
 		}
 		ctx.Status(http.StatusNoContent)
 		return
 	}
 	// v3 not supported due to not having one unique id
-	ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found"))
+	ctx.Error(http.StatusNotFound, "DeleteArtifact", "Artifact not found")
 }
 
 func buildSignature(endp, expires string, artifactID int64) []byte {
@@ -1102,10 +1102,14 @@ func buildSignature(endp, expires string, artifactID int64) []byte {
 	return mac.Sum(nil)
 }
 
-func buildSigURL(ctx go_context.Context, endp string, artifactID int64) string {
+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 buildSigURL(ctx go_context.Context, endPoint string, artifactID int64) string {
+	// endPoint is a path like "api/v1/repos/owner/repo/actions/artifacts/1/zip/raw"
 	expires := time.Now().Add(60 * time.Minute).Format("2006-01-02 15:04:05.999999999 -0700 MST")
-	uploadURL := strings.TrimSuffix(httplib.GuessCurrentAppURL(ctx), "/") +
-		"/" + endp + "?sig=" + base64.URLEncoding.EncodeToString(buildSignature(endp, expires, artifactID)) + "&expires=" + url.QueryEscape(expires)
+	uploadURL := httplib.GuessCurrentAppURL(ctx) + endPoint + "?sig=" + base64.URLEncoding.EncodeToString(buildSignature(endPoint, expires, artifactID)) + "&expires=" + url.QueryEscape(expires)
 	return uploadURL
 }
 
@@ -1140,14 +1144,14 @@ func DownloadArtifact(ctx *context.APIContext) {
 	//   "404":
 	//     "$ref": "#/responses/notFound"
 
-	art, ok := getArtifactByID(ctx)
-	if !ok {
+	art := getArtifactByPathParam(ctx, ctx.Repo.Repository)
+	if ctx.Written() {
 		return
 	}
 
 	// if artifacts status is not uploaded-confirmed, treat it as not found
-	if art.Status == int64(actions_model.ArtifactStatusExpired) {
-		ctx.Error(http.StatusNotFound, "artifact has expired", fmt.Errorf("artifact has expired"))
+	if art.Status == actions_model.ArtifactStatusExpired {
+		ctx.Error(http.StatusNotFound, "DownloadArtifact", "Artifact has expired")
 		return
 	}
 	ctx.Resp.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%s.zip; filename*=UTF-8''%s.zip", url.PathEscape(art.ArtifactName), art.ArtifactName))
@@ -1158,79 +1162,85 @@ func DownloadArtifact(ctx *context.APIContext) {
 			return
 		}
 		if err != nil {
-			ctx.Error(http.StatusInternalServerError, err.Error(), err)
+			ctx.Error(http.StatusInternalServerError, "DownloadArtifactV4ServeDirectOnly", err)
 			return
 		}
 
-		rurl := buildSigURL(ctx, "api/v1/repos/"+url.PathEscape(ctx.Repo.Repository.OwnerName)+"/"+url.PathEscape(ctx.Repo.Repository.Name)+"/actions/artifacts/"+fmt.Sprintf("%d", art.ID)+"/zip/raw", art.ID)
-		ctx.Redirect(rurl, http.StatusFound)
+		redirectURL := buildSigURL(ctx, buildDownloadRawEndpoint(ctx.Repo.Repository, art.ID), art.ID)
+		ctx.Redirect(redirectURL, http.StatusFound)
 		return
 	}
 	// v3 not supported due to not having one unique id
-	ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found"))
+	ctx.Error(http.StatusNotFound, "DownloadArtifact", "Artifact not found")
 }
 
 // DownloadArtifactRaw Downloads a specific artifact for a workflow run directly.
 func DownloadArtifactRaw(ctx *context.APIContext) {
-	username := ctx.PathParam("username")
-	reponame := ctx.PathParam("reponame")
-	artifactID := ctx.PathParamInt64("artifact_id")
-	sig := ctx.Req.URL.Query().Get("sig")
+	// 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"))
+	if err != nil {
+		if errors.Is(err, util.ErrNotExist) {
+			ctx.NotFound()
+		} else {
+			ctx.InternalServerError(err)
+		}
+		return
+	}
+	art := getArtifactByPathParam(ctx, repo)
+	if ctx.Written() {
+		return
+	}
+
+	sigStr := ctx.Req.URL.Query().Get("sig")
 	expires := ctx.Req.URL.Query().Get("expires")
-	dsig, _ := base64.URLEncoding.DecodeString(sig)
+	sigBytes, _ := base64.URLEncoding.DecodeString(sigStr)
 
-	endp := "api/v1/repos/" + url.PathEscape(username) + "/" + url.PathEscape(reponame) + "/actions/artifacts/" + fmt.Sprintf("%d", artifactID) + "/zip/raw"
-	expecedsig := buildSignature(endp, expires, artifactID)
-	if !hmac.Equal(dsig, expecedsig) {
-		log.Error("Error unauthorized")
-		ctx.Error(http.StatusUnauthorized, "Error unauthorized", nil)
+	expectedSig := buildSignature(buildDownloadRawEndpoint(repo, art.ID), expires, art.ID)
+	if !hmac.Equal(sigBytes, expectedSig) {
+		ctx.Error(http.StatusUnauthorized, "DownloadArtifactRaw", "Error unauthorized")
 		return
 	}
 	t, err := time.Parse("2006-01-02 15:04:05.999999999 -0700 MST", expires)
 	if err != nil || t.Before(time.Now()) {
-		log.Error("Error link expired")
-		ctx.Error(http.StatusUnauthorized, "Error link expired", nil)
-		return
-	}
-
-	art, ok := getArtifactByID(ctx)
-	if !ok {
+		ctx.Error(http.StatusUnauthorized, "DownloadArtifactRaw", "Error link expired")
 		return
 	}
 
 	// if artifacts status is not uploaded-confirmed, treat it as not found
-	if art.Status == int64(actions_model.ArtifactStatusExpired) {
-		ctx.Error(http.StatusNotFound, "artifact has expired", fmt.Errorf("artifact has expired"))
+	if art.Status == actions_model.ArtifactStatusExpired {
+		ctx.Error(http.StatusNotFound, "DownloadArtifactRaw", "Artifact has expired")
 		return
 	}
 	ctx.Resp.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%s.zip; filename*=UTF-8''%s.zip", url.PathEscape(art.ArtifactName), art.ArtifactName))
 
 	if actions.IsArtifactV4(art) {
 		err := actions.DownloadArtifactV4(ctx.Base, art)
 		if err != nil {
-			ctx.Error(http.StatusInternalServerError, err.Error(), err)
+			ctx.Error(http.StatusInternalServerError, "DownloadArtifactV4", err)
 			return
 		}
 		return
 	}
 	// v3 not supported due to not having one unique id
-	ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found"))
+	ctx.Error(http.StatusNotFound, "DownloadArtifactRaw", "artifact not found")
 }
 
 // Try to get the artifact by ID and check access
-func getArtifactByID(ctx *context.APIContext) (*actions_model.ActionArtifact, bool) {
+func getArtifactByPathParam(ctx *context.APIContext, repo *repo_model.Repository) *actions_model.ActionArtifact {
 	artifactID := ctx.PathParamInt64("artifact_id")
 
 	art, ok, err := db.GetByID[actions_model.ActionArtifact](ctx, artifactID)
 	if err != nil {
-		ctx.Error(http.StatusInternalServerError, err.Error(), err)
-		return nil, false
+		ctx.Error(http.StatusInternalServerError, "getArtifactByPathParam", err)
+		return nil
 	}
 	// if artifacts status is not uploaded-confirmed, treat it as not found
-	// ctx.Repo.Repository is nil for the raw download endpoint that checked this already
-	if !ok || ctx.Repo != nil && ctx.Repo.Repository != nil && (art.RepoID != ctx.Repo.Repository.ID || art.OwnerID != ctx.Repo.Repository.OwnerID) || art.Status != int64(actions_model.ArtifactStatusUploadConfirmed) && art.Status != int64(actions_model.ArtifactStatusExpired) {
-		ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found"))
-		return nil, false
+	// FIXME: is the OwnerID check right? What if a repo is transferred to a new owner?
+	if !ok ||
+		(art.RepoID != repo.ID || art.OwnerID != repo.OwnerID) ||
+		art.Status != actions_model.ArtifactStatusUploadConfirmed && art.Status != actions_model.ArtifactStatusExpired {
+		ctx.Error(http.StatusNotFound, "getArtifactByPathParam", "artifact not found")
+		return nil
 	}
-	return art, true
+	return art
 }

routers/web/repo/actions/view.go

@@ -668,7 +668,7 @@ func ArtifactsDownloadView(ctx *context_module.Context) {
 
 	// if artifacts status is not uploaded-confirmed, treat it as not found
 	for _, art := range artifacts {
-		if art.Status != int64(actions_model.ArtifactStatusUploadConfirmed) {
+		if art.Status != actions_model.ArtifactStatusUploadConfirmed {
 			ctx.Error(http.StatusNotFound, "artifact not found")
 			return
 		}

services/convert/convert.go

@@ -237,7 +237,7 @@ func ToActionArtifact(repo *repo_model.Repository, art *actions_model.ActionArti
 		ID:                 art.ID,
 		Name:               art.ArtifactName,
 		SizeInBytes:        art.FileSize,
-		Expired:            art.Status == int64(actions_model.ArtifactStatusExpired),
+		Expired:            art.Status == actions_model.ArtifactStatusExpired,
 		URL:                url,
 		ArchiveDownloadURL: url + "/zip",
 		CreatedAt:          art.CreatedUnix.AsLocalTime(),

tests/integration/api_actions_artifact_v4_test.go

@@ -536,9 +536,10 @@ func TestActionsArtifactV4DownloadRawArtifactMismatchedRepoOwnerMissingSignature
 	token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
 
 	// confirm artifacts of wrong owner or repo is not visible
+	// TODO: is this test right? `zip/raw` endpoint doesn't use the 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)
+	MakeRequest(t, req, http.StatusNotFound)
 }
 
 func TestActionsArtifactV4DownloadRawArtifactCorrectRepoOwnerMissingSignatureUnauthorized(t *testing.T) {